From b25f86ab0193f6e59e68a055041b4d7ca0b957dd Mon Sep 17 00:00:00 2001 From: lucperkins Date: Wed, 29 Apr 2020 17:36:36 -0700 Subject: [PATCH] Create separate Settings struct and update usage Signed-off-by: lucperkins --- cmd/helm/helm.go | 10 ++- cmd/helm/helm_test.go | 4 +- cmd/helm/install.go | 2 +- cmd/helm/lint.go | 2 +- cmd/helm/release_testing.go | 2 +- cmd/helm/upgrade.go | 2 +- internal/completion/complete.go | 2 +- pkg/action/install.go | 2 +- pkg/action/pull.go | 2 +- pkg/cli/{environment.go => settings.go} | 85 ++++++++----------- .../{environment_test.go => settings_test.go} | 8 +- pkg/getter/getter.go | 2 +- pkg/getter/getter_test.go | 4 +- pkg/getter/httpgetter_test.go | 2 +- pkg/getter/plugingetter.go | 6 +- pkg/getter/plugingetter_test.go | 18 ++-- pkg/plugin/installer/http_installer.go | 2 +- pkg/plugin/plugin.go | 2 +- 18 files changed, 71 insertions(+), 86 deletions(-) rename pkg/cli/{environment.go => settings.go} (68%) rename pkg/cli/{environment_test.go => settings_test.go} (91%) diff --git a/cmd/helm/helm.go b/cmd/helm/helm.go index fcc7315f5..9d5f01c3b 100644 --- a/cmd/helm/helm.go +++ b/cmd/helm/helm.go @@ -43,7 +43,7 @@ import ( // 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.New() +var settings = cli.SettingsFromEnv() func init() { log.SetFlags(log.Lshortfile) @@ -72,8 +72,10 @@ func main() { // run when each command's execute method is called cobra.OnInitialize(func() { - helmDriver := os.Getenv("HELM_DRIVER") - if err := actionConfig.Init(settings.RESTClientGetter(), settings.Namespace(), helmDriver, debug); err != nil { + namespace := settings.GetNamespace() + helmDriver := settings.HelmDriver + + if err := actionConfig.Init(settings.RESTClientGetter(), namespace, helmDriver, debug); err != nil { log.Fatal(err) } if helmDriver == "memory" { @@ -141,5 +143,5 @@ func loadReleasesInMemory(actionConfig *action.Configuration) { } } // Must reset namespace to the proper one - mem.SetNamespace(settings.Namespace()) + mem.SetNamespace(settings.GetNamespace()) } diff --git a/cmd/helm/helm_test.go b/cmd/helm/helm_test.go index 9ba9d78fb..1a7c9115c 100644 --- a/cmd/helm/helm_test.go +++ b/cmd/helm/helm_test.go @@ -127,7 +127,7 @@ func executeActionCommandStdinC(store *storage.Storage, in *os.File, cmd string) } if mem, ok := store.Driver.(*driver.Memory); ok { - mem.SetNamespace(settings.Namespace()) + mem.SetNamespace(settings.GetNamespace()) } c, err := root.ExecuteC() @@ -163,7 +163,7 @@ func resetEnv() func() { kv := strings.SplitN(pair, "=", 2) os.Setenv(kv[0], kv[1]) } - settings = cli.New() + settings = cli.SettingsFromEnv() } } diff --git a/cmd/helm/install.go b/cmd/helm/install.go index 21a41b9f9..1e3c628a8 100644 --- a/cmd/helm/install.go +++ b/cmd/helm/install.go @@ -225,7 +225,7 @@ func runInstall(args []string, client *action.Install, valueOpts *values.Options } } - client.Namespace = settings.Namespace() + client.Namespace = settings.GetNamespace() return client.Run(chartRequested, vals) } diff --git a/cmd/helm/lint.go b/cmd/helm/lint.go index fe39a5741..ad2d71822 100644 --- a/cmd/helm/lint.go +++ b/cmd/helm/lint.go @@ -68,7 +68,7 @@ func newLintCmd(out io.Writer) *cobra.Command { } } - client.Namespace = settings.Namespace() + client.Namespace = settings.GetNamespace() vals, err := valueOpts.MergeValues(getter.All(settings)) if err != nil { return err diff --git a/cmd/helm/release_testing.go b/cmd/helm/release_testing.go index 036d96794..3069ee5d4 100644 --- a/cmd/helm/release_testing.go +++ b/cmd/helm/release_testing.go @@ -47,7 +47,7 @@ func newReleaseTestCmd(cfg *action.Configuration, out io.Writer) *cobra.Command Long: releaseTestHelp, Args: require.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - client.Namespace = settings.Namespace() + client.Namespace = settings.GetNamespace() rel, runErr := client.Run(args[0]) // We only return an error if we weren't even able to get the // release, otherwise we keep going so we can print status and logs diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go index c263d32e7..012b6a830 100644 --- a/cmd/helm/upgrade.go +++ b/cmd/helm/upgrade.go @@ -73,7 +73,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { Long: upgradeDesc, Args: require.ExactArgs(2), RunE: func(cmd *cobra.Command, args []string) error { - client.Namespace = settings.Namespace() + client.Namespace = settings.GetNamespace() // Fixes #7002 - Support reading values from STDIN for `upgrade` command // Must load values AFTER determining if we have to call install so that values loaded from stdin are are not read twice diff --git a/internal/completion/complete.go b/internal/completion/complete.go index 545f5b0dd..520bb9a9a 100644 --- a/internal/completion/complete.go +++ b/internal/completion/complete.go @@ -174,7 +174,7 @@ func (d BashCompDirective) string() string { } // NewCompleteCmd add a special hidden command that an be used to request completions -func NewCompleteCmd(settings *cli.EnvSettings, out io.Writer) *cobra.Command { +func NewCompleteCmd(settings *cli.Settings, out io.Writer) *cobra.Command { debug = settings.Debug return &cobra.Command{ Use: fmt.Sprintf("%s [command-line]", CompRequestCmd), diff --git a/pkg/action/install.go b/pkg/action/install.go index 10a9644dd..7d90e54ac 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -613,7 +613,7 @@ OUTER: // - URL // // If 'verify' was set on ChartPathOptions, this will attempt to also verify the chart. -func (c *ChartPathOptions) LocateChart(name string, settings *cli.EnvSettings) (string, error) { +func (c *ChartPathOptions) LocateChart(name string, settings *cli.Settings) (string, error) { name = strings.TrimSpace(name) version := strings.TrimSpace(c.Version) diff --git a/pkg/action/pull.go b/pkg/action/pull.go index ee20bbe83..092128fea 100644 --- a/pkg/action/pull.go +++ b/pkg/action/pull.go @@ -38,7 +38,7 @@ import ( type Pull struct { ChartPathOptions - Settings *cli.EnvSettings // TODO: refactor this out of pkg/action + Settings *cli.Settings // TODO: refactor this out of pkg/action Devel bool Untar bool diff --git a/pkg/cli/environment.go b/pkg/cli/settings.go similarity index 68% rename from pkg/cli/environment.go rename to pkg/cli/settings.go index 2e64e0810..c2e5d189a 100644 --- a/pkg/cli/environment.go +++ b/pkg/cli/settings.go @@ -1,43 +1,18 @@ -/* -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 cli describes the operating environment for the Helm CLI. - -Helm's environment encapsulates all of the service dependencies Helm has. -These dependencies are expressed as interfaces so that alternate implementations -(mocks, etc.) can be easily generated. -*/ package cli import ( "fmt" - "os" - "strconv" - "github.com/spf13/pflag" - "k8s.io/cli-runtime/pkg/genericclioptions" - "helm.sh/helm/v3/pkg/helmpath" + "k8s.io/cli-runtime/pkg/genericclioptions" + "os" + "strconv" ) -// EnvSettings describes all of the environment settings. -type EnvSettings struct { - namespace string - config *genericclioptions.ConfigFlags - +// Settings describes all of the settings required by the Helm client. +type Settings struct { + Namespace string + HelmDriver string // KubeConfig is the path to the kubeconfig file KubeConfig string // KubeContext is the name of the kubeconfig context. @@ -56,11 +31,15 @@ type EnvSettings struct { RepositoryCache string // PluginsDirectory is the path to the plugins directory. PluginsDirectory string + + config *genericclioptions.ConfigFlags } -func New() *EnvSettings { - env := &EnvSettings{ - namespace: os.Getenv("HELM_NAMESPACE"), +// The default Settings struct for the Helm client, largely drawn from environment variables. +func SettingsFromEnv() *Settings { + env := &Settings{ + Namespace: os.Getenv("HELM_NAMESPACE"), + HelmDriver: os.Getenv("HELM_DRIVER"), KubeContext: os.Getenv("HELM_KUBECONTEXT"), KubeToken: os.Getenv("HELM_KUBETOKEN"), KubeAPIServer: os.Getenv("HELM_KUBEAPISERVER"), @@ -73,7 +52,7 @@ func New() *EnvSettings { // bind to kubernetes config flags env.config = &genericclioptions.ConfigFlags{ - Namespace: &env.namespace, + Namespace: &env.Namespace, Context: &env.KubeContext, BearerToken: &env.KubeToken, APIServer: &env.KubeAPIServer, @@ -83,8 +62,8 @@ func New() *EnvSettings { } // AddFlags binds flags to the given flagset. -func (s *EnvSettings) AddFlags(fs *pflag.FlagSet) { - fs.StringVarP(&s.namespace, "namespace", "n", s.namespace, "namespace scope for this request") +func (s *Settings) AddFlags(fs *pflag.FlagSet) { + fs.StringVarP(&s.Namespace, "namespace", "n", s.Namespace, "namespace scope for this request") fs.StringVar(&s.KubeConfig, "kubeconfig", "", "path to the kubeconfig file") fs.StringVar(&s.KubeContext, "kube-context", s.KubeContext, "name of the kubeconfig context to use") fs.StringVar(&s.KubeToken, "kube-token", s.KubeToken, "bearer token used for authentication") @@ -95,6 +74,19 @@ func (s *EnvSettings) AddFlags(fs *pflag.FlagSet) { fs.StringVar(&s.RepositoryCache, "repository-cache", s.RepositoryCache, "path to the file containing cached repository indexes") } +// GetNamespace gets the namespace from the configuration +func (s *Settings) GetNamespace() string { + if ns, _, err := s.config.ToRawKubeConfigLoader().Namespace(); err == nil { + return ns + } + return "default" +} + +// RESTClientGetter gets the kubeconfig from Settings +func (s *Settings) RESTClientGetter() genericclioptions.RESTClientGetter { + return s.config +} + func envOr(name, def string) string { if v, ok := os.LookupEnv(name); ok { return v @@ -102,7 +94,7 @@ func envOr(name, def string) string { return def } -func (s *EnvSettings) EnvVars() map[string]string { +func (s *Settings) EnvVars() map[string]string { envvars := map[string]string{ "HELM_BIN": os.Args[0], "HELM_DEBUG": fmt.Sprint(s.Debug), @@ -110,7 +102,7 @@ func (s *EnvSettings) EnvVars() map[string]string { "HELM_REGISTRY_CONFIG": s.RegistryConfig, "HELM_REPOSITORY_CACHE": s.RepositoryCache, "HELM_REPOSITORY_CONFIG": s.RepositoryConfig, - "HELM_NAMESPACE": s.Namespace(), + "HELM_NAMESPACE": s.GetNamespace(), // broken, these are populated from helm flags and not kubeconfig. "HELM_KUBECONTEXT": s.KubeContext, @@ -123,15 +115,6 @@ func (s *EnvSettings) EnvVars() map[string]string { return envvars } -// Namespace gets the namespace from the configuration -func (s *EnvSettings) Namespace() string { - if ns, _, err := s.config.ToRawKubeConfigLoader().Namespace(); err == nil { - return ns - } - return "default" -} - -// RESTClientGetter gets the kubeconfig from EnvSettings -func (s *EnvSettings) RESTClientGetter() genericclioptions.RESTClientGetter { - return s.config +func (s *Settings) validate() error { + return nil } diff --git a/pkg/cli/environment_test.go b/pkg/cli/settings_test.go similarity index 91% rename from pkg/cli/environment_test.go rename to pkg/cli/settings_test.go index fadc2981e..00387666a 100644 --- a/pkg/cli/environment_test.go +++ b/pkg/cli/settings_test.go @@ -71,15 +71,15 @@ func TestEnvSettings(t *testing.T) { flags := pflag.NewFlagSet("testing", pflag.ContinueOnError) - settings := New() + settings := SettingsFromEnv() settings.AddFlags(flags) flags.Parse(strings.Split(tt.args, " ")) if settings.Debug != tt.debug { t.Errorf("expected debug %t, got %t", tt.debug, settings.Debug) } - if settings.Namespace() != tt.ns { - t.Errorf("expected namespace %q, got %q", tt.ns, settings.Namespace()) + if settings.GetNamespace() != tt.ns { + t.Errorf("expected namespace %q, got %q", tt.ns, settings.GetNamespace()) } if settings.KubeContext != tt.kcontext { t.Errorf("expected kube-context %q, got %q", tt.kcontext, settings.KubeContext) @@ -92,7 +92,7 @@ func resetEnv() func() { origEnv := os.Environ() // ensure any local envvars do not hose us - for e := range New().EnvVars() { + for e := range SettingsFromEnv().EnvVars() { os.Unsetenv(e) } diff --git a/pkg/getter/getter.go b/pkg/getter/getter.go index 4ccc74834..a6c0c6f7e 100644 --- a/pkg/getter/getter.go +++ b/pkg/getter/getter.go @@ -133,7 +133,7 @@ var httpProvider = Provider{ // All finds all of the registered getters as a list of Provider instances. // Currently, the built-in getters and the discovered plugins with downloader // notations are collected. -func All(settings *cli.EnvSettings) Providers { +func All(settings *cli.Settings) Providers { result := Providers{httpProvider} pluginDownloaders, _ := collectPlugins(settings) result = append(result, pluginDownloaders...) diff --git a/pkg/getter/getter_test.go b/pkg/getter/getter_test.go index 79a3338e9..2f737ff86 100644 --- a/pkg/getter/getter_test.go +++ b/pkg/getter/getter_test.go @@ -53,7 +53,7 @@ func TestProviders(t *testing.T) { } func TestAll(t *testing.T) { - env := cli.New() + env := cli.SettingsFromEnv() env.PluginsDirectory = pluginDir all := All(env) @@ -67,7 +67,7 @@ func TestAll(t *testing.T) { } func TestByScheme(t *testing.T) { - env := cli.New() + env := cli.SettingsFromEnv() env.PluginsDirectory = pluginDir g := All(env) diff --git a/pkg/getter/httpgetter_test.go b/pkg/getter/httpgetter_test.go index 4d7ada852..ee2e2caf8 100644 --- a/pkg/getter/httpgetter_test.go +++ b/pkg/getter/httpgetter_test.go @@ -122,7 +122,7 @@ func TestDownload(t *testing.T) { })) defer srv.Close() - g, err := All(cli.New()).ByScheme("http") + g, err := All(cli.SettingsFromEnv()).ByScheme("http") if err != nil { t.Fatal(err) } diff --git a/pkg/getter/plugingetter.go b/pkg/getter/plugingetter.go index 0d13ade57..725468b53 100644 --- a/pkg/getter/plugingetter.go +++ b/pkg/getter/plugingetter.go @@ -30,7 +30,7 @@ import ( // collectPlugins scans for getter plugins. // This will load plugins according to the cli. -func collectPlugins(settings *cli.EnvSettings) (Providers, error) { +func collectPlugins(settings *cli.Settings) (Providers, error) { plugins, err := plugin.FindPlugins(settings.PluginsDirectory) if err != nil { return nil, err @@ -56,7 +56,7 @@ func collectPlugins(settings *cli.EnvSettings) (Providers, error) { // implemented in plugins. type pluginGetter struct { command string - settings *cli.EnvSettings + settings *cli.Settings name string base string opts options @@ -86,7 +86,7 @@ func (p *pluginGetter) Get(href string, options ...Option) (*bytes.Buffer, error } // NewPluginGetter constructs a valid plugin getter -func NewPluginGetter(command string, settings *cli.EnvSettings, name, base string) Constructor { +func NewPluginGetter(command string, settings *cli.Settings, name, base string) Constructor { return func(options ...Option) (Getter, error) { result := &pluginGetter{ command: command, diff --git a/pkg/getter/plugingetter_test.go b/pkg/getter/plugingetter_test.go index a18fa302b..6f5967a29 100644 --- a/pkg/getter/plugingetter_test.go +++ b/pkg/getter/plugingetter_test.go @@ -24,10 +24,10 @@ import ( ) func TestCollectPlugins(t *testing.T) { - env := cli.New() - env.PluginsDirectory = pluginDir + settings := cli.SettingsFromEnv() + settings.PluginsDirectory = pluginDir - p, err := collectPlugins(env) + p, err := collectPlugins(settings) if err != nil { t.Fatal(err) } @@ -54,9 +54,9 @@ func TestPluginGetter(t *testing.T) { t.Skip("TODO: refactor this test to work on windows") } - env := cli.New() - env.PluginsDirectory = pluginDir - pg := NewPluginGetter("echo", env, "test", ".") + settings := cli.SettingsFromEnv() + settings.PluginsDirectory = pluginDir + pg := NewPluginGetter("echo", settings, "test", ".") g, err := pg() if err != nil { t.Fatal(err) @@ -79,10 +79,10 @@ func TestPluginSubCommands(t *testing.T) { t.Skip("TODO: refactor this test to work on windows") } - env := cli.New() - env.PluginsDirectory = pluginDir + settings := cli.SettingsFromEnv() + settings.PluginsDirectory = pluginDir - pg := NewPluginGetter("echo -n", env, "test", ".") + pg := NewPluginGetter("echo -n", settings, "test", ".") g, err := pg() if err != nil { t.Fatal(err) diff --git a/pkg/plugin/installer/http_installer.go b/pkg/plugin/installer/http_installer.go index 629bbec39..00a4ca221 100644 --- a/pkg/plugin/installer/http_installer.go +++ b/pkg/plugin/installer/http_installer.go @@ -79,7 +79,7 @@ func NewHTTPInstaller(source string) (*HTTPInstaller, error) { return nil, err } - get, err := getter.All(new(cli.EnvSettings)).ByScheme("http") + get, err := getter.All(new(cli.Settings)).ByScheme("http") if err != nil { return nil, err } diff --git a/pkg/plugin/plugin.go b/pkg/plugin/plugin.go index 2eb354fca..56b7a85e1 100644 --- a/pkg/plugin/plugin.go +++ b/pkg/plugin/plugin.go @@ -215,7 +215,7 @@ func FindPlugins(plugdirs string) ([]*Plugin, error) { // SetupPluginEnv prepares os.Env for plugins. It operates on os.Env because // the plugin subsystem itself needs access to the environment variables // created here. -func SetupPluginEnv(settings *cli.EnvSettings, name, base string) { +func SetupPluginEnv(settings *cli.Settings, name, base string) { env := settings.EnvVars() env["HELM_PLUGIN_NAME"] = name env["HELM_PLUGIN_DIR"] = base