From fe952445bdbe492cbc15388b6a125077139d496f Mon Sep 17 00:00:00 2001 From: Matthew Fisher Date: Thu, 8 Aug 2019 11:56:29 -0700 Subject: [PATCH] feat(cmd): put OCI commands behind a feature gate This adds a new `gates` package used for interacting with feature gates. It also marks the OCI registry work as experimental, signalling to users that it is not a stable feature of Helm. Signed-off-by: Matthew Fisher --- cmd/helm/chart.go | 8 ++-- cmd/helm/chart_export.go | 9 ++-- cmd/helm/chart_list.go | 1 + cmd/helm/chart_pull.go | 9 ++-- cmd/helm/chart_push.go | 9 ++-- cmd/helm/chart_remove.go | 1 + cmd/helm/chart_save.go | 9 ++-- cmd/helm/helm.go | 14 ++++++ cmd/helm/registry.go | 8 ++-- cmd/helm/registry_login.go | 9 ++-- cmd/helm/registry_logout.go | 9 ++-- cmd/helm/root.go | 2 +- .../experimental}/registry/authorizer.go | 2 +- .../experimental}/registry/cache.go | 2 +- .../experimental}/registry/client.go | 2 +- .../experimental}/registry/client_test.go | 0 .../experimental}/registry/constants.go | 2 +- .../experimental}/registry/constants_test.go | 0 .../experimental}/registry/reference.go | 2 +- .../experimental}/registry/reference_test.go | 0 .../experimental}/registry/resolver.go | 2 +- pkg/action/action.go | 2 +- pkg/action/action_test.go | 2 +- pkg/action/chart_export.go | 2 +- pkg/action/chart_pull.go | 2 +- pkg/action/chart_push.go | 2 +- pkg/action/chart_remove.go | 2 +- pkg/action/chart_save.go | 2 +- pkg/action/chart_save_test.go | 2 +- pkg/gates/doc.go | 20 ++++++++ pkg/gates/gates.go | 38 +++++++++++++++ pkg/gates/gates_test.go | 47 +++++++++++++++++++ 32 files changed, 176 insertions(+), 45 deletions(-) rename {pkg => internal/experimental}/registry/authorizer.go (90%) rename {pkg => internal/experimental}/registry/cache.go (99%) rename {pkg => internal/experimental}/registry/client.go (98%) rename {pkg => internal/experimental}/registry/client_test.go (100%) rename {pkg => internal/experimental}/registry/constants.go (96%) rename {pkg => internal/experimental}/registry/constants_test.go (100%) rename {pkg => internal/experimental}/registry/reference.go (97%) rename {pkg => internal/experimental}/registry/reference_test.go (100%) rename {pkg => internal/experimental}/registry/resolver.go (90%) create mode 100644 pkg/gates/doc.go create mode 100644 pkg/gates/gates.go create mode 100644 pkg/gates/gates_test.go diff --git a/cmd/helm/chart.go b/cmd/helm/chart.go index 293ab3635..e66e6be3c 100644 --- a/cmd/helm/chart.go +++ b/cmd/helm/chart.go @@ -33,9 +33,11 @@ Example usage: func newChartCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { cmd := &cobra.Command{ - Use: "chart", - Short: "push, pull, tag, or remove Helm charts", - Long: chartHelp, + Use: "chart", + Short: "push, pull, tag, or remove Helm charts", + Long: chartHelp, + Hidden: !FeatureGateOCI.IsEnabled(), + PersistentPreRunE: checkOCIFeatureGate(), } cmd.AddCommand( newChartListCmd(cfg, out), diff --git a/cmd/helm/chart_export.go b/cmd/helm/chart_export.go index 117220f9d..a37d146ea 100644 --- a/cmd/helm/chart_export.go +++ b/cmd/helm/chart_export.go @@ -35,10 +35,11 @@ and check into source control if desired. func newChartExportCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { return &cobra.Command{ - Use: "export [ref]", - Short: "export a chart to directory", - Long: chartExportDesc, - Args: require.MinimumNArgs(1), + Use: "export [ref]", + Short: "export a chart to directory", + Long: chartExportDesc, + Args: require.MinimumNArgs(1), + Hidden: !FeatureGateOCI.IsEnabled(), RunE: func(cmd *cobra.Command, args []string) error { ref := args[0] return action.NewChartExport(cfg).Run(out, ref) diff --git a/cmd/helm/chart_list.go b/cmd/helm/chart_list.go index 868fcd15a..78931603a 100644 --- a/cmd/helm/chart_list.go +++ b/cmd/helm/chart_list.go @@ -36,6 +36,7 @@ func newChartListCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { Aliases: []string{"ls"}, Short: "list all saved charts", Long: chartListDesc, + Hidden: !FeatureGateOCI.IsEnabled(), RunE: func(cmd *cobra.Command, args []string) error { return action.NewChartList(cfg).Run(out) }, diff --git a/cmd/helm/chart_pull.go b/cmd/helm/chart_pull.go index ade952cb8..bb90366cb 100644 --- a/cmd/helm/chart_pull.go +++ b/cmd/helm/chart_pull.go @@ -33,10 +33,11 @@ This will store the chart in the local registry cache to be used later. func newChartPullCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { return &cobra.Command{ - Use: "pull [ref]", - Short: "pull a chart from remote", - Long: chartPullDesc, - Args: require.MinimumNArgs(1), + Use: "pull [ref]", + Short: "pull a chart from remote", + Long: chartPullDesc, + Args: require.MinimumNArgs(1), + Hidden: !FeatureGateOCI.IsEnabled(), RunE: func(cmd *cobra.Command, args []string) error { ref := args[0] return action.NewChartPull(cfg).Run(out, ref) diff --git a/cmd/helm/chart_push.go b/cmd/helm/chart_push.go index 6f5591b97..21fbe0e1f 100644 --- a/cmd/helm/chart_push.go +++ b/cmd/helm/chart_push.go @@ -35,10 +35,11 @@ Must first run "helm chart save" or "helm chart pull". func newChartPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { return &cobra.Command{ - Use: "push [ref]", - Short: "push a chart to remote", - Long: chartPushDesc, - Args: require.MinimumNArgs(1), + Use: "push [ref]", + Short: "push a chart to remote", + Long: chartPushDesc, + Args: require.MinimumNArgs(1), + Hidden: !FeatureGateOCI.IsEnabled(), RunE: func(cmd *cobra.Command, args []string) error { ref := args[0] return action.NewChartPush(cfg).Run(out, ref) diff --git a/cmd/helm/chart_remove.go b/cmd/helm/chart_remove.go index 5ab1d64c3..43e463c6a 100644 --- a/cmd/helm/chart_remove.go +++ b/cmd/helm/chart_remove.go @@ -41,6 +41,7 @@ func newChartRemoveCmd(cfg *action.Configuration, out io.Writer) *cobra.Command Short: "remove a chart", Long: chartRemoveDesc, Args: require.MinimumNArgs(1), + Hidden: !FeatureGateOCI.IsEnabled(), RunE: func(cmd *cobra.Command, args []string) error { ref := args[0] return action.NewChartRemove(cfg).Run(out, ref) diff --git a/cmd/helm/chart_save.go b/cmd/helm/chart_save.go index 1f8c915bb..49c8cb803 100644 --- a/cmd/helm/chart_save.go +++ b/cmd/helm/chart_save.go @@ -36,10 +36,11 @@ not change the item as it exists in the cache. func newChartSaveCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { return &cobra.Command{ - Use: "save [path] [ref]", - Short: "save a chart directory", - Long: chartSaveDesc, - Args: require.MinimumNArgs(2), + Use: "save [path] [ref]", + Short: "save a chart directory", + Long: chartSaveDesc, + Args: require.MinimumNArgs(2), + Hidden: !FeatureGateOCI.IsEnabled(), RunE: func(cmd *cobra.Command, args []string) error { path := args[0] ref := args[1] diff --git a/cmd/helm/helm.go b/cmd/helm/helm.go index b47a91893..75382b089 100644 --- a/cmd/helm/helm.go +++ b/cmd/helm/helm.go @@ -24,6 +24,7 @@ import ( "strings" "sync" + "github.com/spf13/cobra" "github.com/spf13/pflag" "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/klog" @@ -33,11 +34,15 @@ import ( "helm.sh/helm/pkg/action" "helm.sh/helm/pkg/cli" + "helm.sh/helm/pkg/gates" "helm.sh/helm/pkg/kube" "helm.sh/helm/pkg/storage" "helm.sh/helm/pkg/storage/driver" ) +// FeatureGateOCI is the feature gate for checking if `helm chart` and `helm registry` commands should work +const FeatureGateOCI = gates.Gate("HELM_EXPERIMENTAL_OCI") + var ( settings cli.EnvSettings config genericclioptions.RESTClientGetter @@ -134,3 +139,12 @@ func getNamespace() string { func wordSepNormalizeFunc(f *pflag.FlagSet, name string) pflag.NormalizedName { return pflag.NormalizedName(strings.ReplaceAll(name, "_", "-")) } + +func checkOCIFeatureGate() func(_ *cobra.Command, _ []string) error { + return func(_ *cobra.Command, _ []string) error { + if !FeatureGateOCI.IsEnabled() { + return FeatureGateOCI.Error() + } + return nil + } +} diff --git a/cmd/helm/registry.go b/cmd/helm/registry.go index 1ed885c83..2a1e3a03d 100644 --- a/cmd/helm/registry.go +++ b/cmd/helm/registry.go @@ -33,9 +33,11 @@ Example usage: func newRegistryCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { cmd := &cobra.Command{ - Use: "registry", - Short: "login to or logout from a registry", - Long: registryHelp, + Use: "registry", + Short: "login to or logout from a registry", + Long: registryHelp, + Hidden: !FeatureGateOCI.IsEnabled(), + PersistentPreRunE: checkOCIFeatureGate(), } cmd.AddCommand( newRegistryLoginCmd(cfg, out), diff --git a/cmd/helm/registry_login.go b/cmd/helm/registry_login.go index d40b1dd44..d3c0c5b6e 100644 --- a/cmd/helm/registry_login.go +++ b/cmd/helm/registry_login.go @@ -41,10 +41,11 @@ func newRegistryLoginCmd(cfg *action.Configuration, out io.Writer) *cobra.Comman var passwordFromStdinOpt bool cmd := &cobra.Command{ - Use: "login [host]", - Short: "login to a registry", - Long: registryLoginDesc, - Args: require.MinimumNArgs(1), + Use: "login [host]", + Short: "login to a registry", + Long: registryLoginDesc, + Args: require.MinimumNArgs(1), + Hidden: !FeatureGateOCI.IsEnabled(), RunE: func(cmd *cobra.Command, args []string) error { hostname := args[0] diff --git a/cmd/helm/registry_logout.go b/cmd/helm/registry_logout.go index 099f4ee7b..3ec6876b3 100644 --- a/cmd/helm/registry_logout.go +++ b/cmd/helm/registry_logout.go @@ -31,10 +31,11 @@ Remove credentials stored for a remote registry. func newRegistryLogoutCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { return &cobra.Command{ - Use: "logout [host]", - Short: "logout from a registry", - Long: registryLogoutDesc, - Args: require.MinimumNArgs(1), + Use: "logout [host]", + Short: "logout from a registry", + Long: registryLogoutDesc, + Args: require.MinimumNArgs(1), + Hidden: !FeatureGateOCI.IsEnabled(), RunE: func(cmd *cobra.Command, args []string) error { hostname := args[0] return action.NewRegistryLogout(cfg).Run(out, hostname) diff --git a/cmd/helm/root.go b/cmd/helm/root.go index cb69df8f4..fe336057a 100644 --- a/cmd/helm/root.go +++ b/cmd/helm/root.go @@ -25,9 +25,9 @@ import ( "github.com/spf13/cobra" "helm.sh/helm/cmd/helm/require" + "helm.sh/helm/internal/experimental/registry" "helm.sh/helm/pkg/action" "helm.sh/helm/pkg/helmpath" - "helm.sh/helm/pkg/registry" ) const ( diff --git a/pkg/registry/authorizer.go b/internal/experimental/registry/authorizer.go similarity index 90% rename from pkg/registry/authorizer.go rename to internal/experimental/registry/authorizer.go index c601b59d4..783404f6f 100644 --- a/pkg/registry/authorizer.go +++ b/internal/experimental/registry/authorizer.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package registry // import "helm.sh/helm/pkg/registry" +package registry // import "helm.sh/helm/internal/experimental/registry" import ( "github.com/deislabs/oras/pkg/auth" diff --git a/pkg/registry/cache.go b/internal/experimental/registry/cache.go similarity index 99% rename from pkg/registry/cache.go rename to internal/experimental/registry/cache.go index feb9e8069..57bf562fa 100644 --- a/pkg/registry/cache.go +++ b/internal/experimental/registry/cache.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package registry // import "helm.sh/helm/pkg/registry" +package registry // import "helm.sh/helm/internal/experimental/registry" import ( "bytes" diff --git a/pkg/registry/client.go b/internal/experimental/registry/client.go similarity index 98% rename from pkg/registry/client.go rename to internal/experimental/registry/client.go index 844db562d..570b01bd2 100644 --- a/pkg/registry/client.go +++ b/internal/experimental/registry/client.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package registry // import "helm.sh/helm/pkg/registry" +package registry // import "helm.sh/helm/internal/experimental/registry" import ( "context" diff --git a/pkg/registry/client_test.go b/internal/experimental/registry/client_test.go similarity index 100% rename from pkg/registry/client_test.go rename to internal/experimental/registry/client_test.go diff --git a/pkg/registry/constants.go b/internal/experimental/registry/constants.go similarity index 96% rename from pkg/registry/constants.go rename to internal/experimental/registry/constants.go index 6dc46f2c1..24583b5b5 100644 --- a/pkg/registry/constants.go +++ b/internal/experimental/registry/constants.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package registry // import "helm.sh/helm/pkg/registry" +package registry // import "helm.sh/helm/internal/experimental/registry" const ( // HelmChartConfigMediaType is the reserved media type for the Helm chart manifest config diff --git a/pkg/registry/constants_test.go b/internal/experimental/registry/constants_test.go similarity index 100% rename from pkg/registry/constants_test.go rename to internal/experimental/registry/constants_test.go diff --git a/pkg/registry/reference.go b/internal/experimental/registry/reference.go similarity index 97% rename from pkg/registry/reference.go rename to internal/experimental/registry/reference.go index 7a136205f..12abe0260 100644 --- a/pkg/registry/reference.go +++ b/internal/experimental/registry/reference.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package registry // import "helm.sh/helm/pkg/registry" +package registry // import "helm.sh/helm/internal/experimental/registry" import ( "errors" diff --git a/pkg/registry/reference_test.go b/internal/experimental/registry/reference_test.go similarity index 100% rename from pkg/registry/reference_test.go rename to internal/experimental/registry/reference_test.go diff --git a/pkg/registry/resolver.go b/internal/experimental/registry/resolver.go similarity index 90% rename from pkg/registry/resolver.go rename to internal/experimental/registry/resolver.go index fce303c73..716c01f89 100644 --- a/pkg/registry/resolver.go +++ b/internal/experimental/registry/resolver.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package registry // import "helm.sh/helm/pkg/registry" +package registry // import "helm.sh/helm/internal/experimental/registry" import ( "github.com/containerd/containerd/remotes" diff --git a/pkg/action/action.go b/pkg/action/action.go index 7c62a895d..26e8dd945 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -30,10 +30,10 @@ import ( "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" + "helm.sh/helm/internal/experimental/registry" "helm.sh/helm/pkg/chartutil" "helm.sh/helm/pkg/hooks" "helm.sh/helm/pkg/kube" - "helm.sh/helm/pkg/registry" "helm.sh/helm/pkg/release" "helm.sh/helm/pkg/storage" ) diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go index 83346ea58..019d53bde 100644 --- a/pkg/action/action_test.go +++ b/pkg/action/action_test.go @@ -25,10 +25,10 @@ import ( dockerauth "github.com/deislabs/oras/pkg/auth/docker" fakeclientset "k8s.io/client-go/kubernetes/fake" + "helm.sh/helm/internal/experimental/registry" "helm.sh/helm/pkg/chart" "helm.sh/helm/pkg/chartutil" kubefake "helm.sh/helm/pkg/kube/fake" - "helm.sh/helm/pkg/registry" "helm.sh/helm/pkg/release" "helm.sh/helm/pkg/storage" "helm.sh/helm/pkg/storage/driver" diff --git a/pkg/action/chart_export.go b/pkg/action/chart_export.go index 5bfb002de..f477a5235 100644 --- a/pkg/action/chart_export.go +++ b/pkg/action/chart_export.go @@ -20,8 +20,8 @@ import ( "fmt" "io" + "helm.sh/helm/internal/experimental/registry" "helm.sh/helm/pkg/chartutil" - "helm.sh/helm/pkg/registry" ) // ChartExport performs a chart export operation. diff --git a/pkg/action/chart_pull.go b/pkg/action/chart_pull.go index f4388a5ea..7fb7d6011 100644 --- a/pkg/action/chart_pull.go +++ b/pkg/action/chart_pull.go @@ -19,7 +19,7 @@ package action import ( "io" - "helm.sh/helm/pkg/registry" + "helm.sh/helm/internal/experimental/registry" ) // ChartPull performs a chart pull operation. diff --git a/pkg/action/chart_push.go b/pkg/action/chart_push.go index 97ab77fc0..2c867e644 100644 --- a/pkg/action/chart_push.go +++ b/pkg/action/chart_push.go @@ -19,7 +19,7 @@ package action import ( "io" - "helm.sh/helm/pkg/registry" + "helm.sh/helm/internal/experimental/registry" ) // ChartPush performs a chart push operation. diff --git a/pkg/action/chart_remove.go b/pkg/action/chart_remove.go index ae1d93135..c810a395f 100644 --- a/pkg/action/chart_remove.go +++ b/pkg/action/chart_remove.go @@ -19,7 +19,7 @@ package action import ( "io" - "helm.sh/helm/pkg/registry" + "helm.sh/helm/internal/experimental/registry" ) // ChartRemove performs a chart remove operation. diff --git a/pkg/action/chart_save.go b/pkg/action/chart_save.go index 3f8dcf533..cd1bfccb2 100644 --- a/pkg/action/chart_save.go +++ b/pkg/action/chart_save.go @@ -19,8 +19,8 @@ package action import ( "io" + "helm.sh/helm/internal/experimental/registry" "helm.sh/helm/pkg/chart" - "helm.sh/helm/pkg/registry" ) // ChartSave performs a chart save operation. diff --git a/pkg/action/chart_save_test.go b/pkg/action/chart_save_test.go index 609fc79ed..090d4a147 100644 --- a/pkg/action/chart_save_test.go +++ b/pkg/action/chart_save_test.go @@ -20,7 +20,7 @@ import ( "io/ioutil" "testing" - "helm.sh/helm/pkg/registry" + "helm.sh/helm/internal/experimental/registry" ) func chartSaveAction(t *testing.T) *ChartSave { diff --git a/pkg/gates/doc.go b/pkg/gates/doc.go new file mode 100644 index 000000000..762fdb8c6 --- /dev/null +++ b/pkg/gates/doc.go @@ -0,0 +1,20 @@ +/* +Copyright The Helm Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +/*Package gates provides a general tool for working with experimental feature gates. + +This provides convenience methods where the user can determine if certain experimental features are enabled. +*/ +package gates diff --git a/pkg/gates/gates.go b/pkg/gates/gates.go new file mode 100644 index 000000000..fa4853ce7 --- /dev/null +++ b/pkg/gates/gates.go @@ -0,0 +1,38 @@ +/* +Copyright The Helm Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package gates + +import ( + "fmt" + "os" +) + +// Gate is the name of the feature gate. +type Gate string + +// String returns the string representation of this feature gate. +func (g Gate) String() string { + return string(g) +} + +// IsEnabled determines whether a certain feature gate is enabled. +func (g Gate) IsEnabled() bool { + return os.Getenv(string(g)) != "" +} + +func (g Gate) Error() error { + return fmt.Errorf("this feature has been marked as experimental and is not enabled by default. Please set $%s in your environment to use this feature", g.String()) +} diff --git a/pkg/gates/gates_test.go b/pkg/gates/gates_test.go new file mode 100644 index 000000000..2ca5edb67 --- /dev/null +++ b/pkg/gates/gates_test.go @@ -0,0 +1,47 @@ +/* +Copyright The Helm Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package gates + +import ( + "os" + "testing" +) + +const name string = "HELM_EXPERIMENTAL_FEATURE" + +func TestIsEnabled(t *testing.T) { + os.Unsetenv(name) + g := Gate(name) + + if g.IsEnabled() { + t.Errorf("feature gate shows as available, but the environment variable $%s was not set", name) + } + + os.Setenv(name, "1") + + if !g.IsEnabled() { + t.Errorf("feature gate shows as disabled, but the environment variable $%s was set", name) + } +} + +func TestError(t *testing.T) { + os.Unsetenv(name) + g := Gate(name) + + if g.Error().Error() != "this feature has been marked as experimental and is not enabled by default. Please set $HELM_EXPERIMENTAL_FEATURE in your environment to use this feature" { + t.Errorf("incorrect error message. Received %s", g.Error().Error()) + } +}