From 4a0dfbe53b2191e09a7034ce97e4caf84989d6db Mon Sep 17 00:00:00 2001 From: Adam Reese Date: Thu, 23 Apr 2020 12:20:14 -0700 Subject: [PATCH] fix(pkg/cli): ensure correct configuration from kubeconfig file Bind Helm flags to Kubernetes configuration loader to get a merged config with kubeconfig. Fixes: #7539 Signed-off-by: Adam Reese --- cmd/helm/helm.go | 21 ++++++------- pkg/cli/environment.go | 53 +++++++++++++-------------------- pkg/getter/getter_test.go | 14 +++++---- pkg/getter/httpgetter_test.go | 2 +- pkg/getter/plugingetter_test.go | 17 +++++------ pkg/kube/config.go | 2 ++ pkg/plugin/plugin_test.go | 5 ++-- 7 files changed, 53 insertions(+), 61 deletions(-) diff --git a/cmd/helm/helm.go b/cmd/helm/helm.go index 7e1fcb6e1..fcc7315f5 100644 --- a/cmd/helm/helm.go +++ b/cmd/helm/helm.go @@ -43,9 +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.New() func init() { log.SetFlags(log.Lshortfile) @@ -72,13 +70,16 @@ func main() { actionConfig := new(action.Configuration) cmd := newRootCmd(actionConfig, os.Stdout, os.Args[1:]) - helmDriver := os.Getenv("HELM_DRIVER") - if err := actionConfig.Init(settings.RESTClientGetter(), settings.Namespace(), helmDriver, debug); err != nil { - log.Fatal(err) - } - if helmDriver == "memory" { - loadReleasesInMemory(actionConfig) - } + // 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 { + log.Fatal(err) + } + if helmDriver == "memory" { + loadReleasesInMemory(actionConfig) + } + }) if err := cmd.Execute(); err != nil { debug("%+v", err) diff --git a/pkg/cli/environment.go b/pkg/cli/environment.go index e279331b0..2e64e0810 100644 --- a/pkg/cli/environment.go +++ b/pkg/cli/environment.go @@ -26,21 +26,17 @@ import ( "fmt" "os" "strconv" - "sync" "github.com/spf13/pflag" - "k8s.io/cli-runtime/pkg/genericclioptions" "helm.sh/helm/v3/pkg/helmpath" - "helm.sh/helm/v3/pkg/kube" ) // EnvSettings describes all of the environment settings. type EnvSettings struct { - namespace string - config genericclioptions.RESTClientGetter - configOnce sync.Once + namespace string + config *genericclioptions.ConfigFlags // KubeConfig is the path to the kubeconfig file KubeConfig string @@ -63,8 +59,7 @@ type EnvSettings struct { } func New() *EnvSettings { - - env := EnvSettings{ + env := &EnvSettings{ namespace: os.Getenv("HELM_NAMESPACE"), KubeContext: os.Getenv("HELM_KUBECONTEXT"), KubeToken: os.Getenv("HELM_KUBETOKEN"), @@ -75,7 +70,16 @@ func New() *EnvSettings { RepositoryCache: envOr("HELM_REPOSITORY_CACHE", helmpath.CachePath("repository")), } env.Debug, _ = strconv.ParseBool(os.Getenv("HELM_DEBUG")) - return &env + + // bind to kubernetes config flags + env.config = &genericclioptions.ConfigFlags{ + Namespace: &env.namespace, + Context: &env.KubeContext, + BearerToken: &env.KubeToken, + APIServer: &env.KubeAPIServer, + KubeConfig: &env.KubeConfig, + } + return env } // AddFlags binds flags to the given flagset. @@ -107,42 +111,27 @@ func (s *EnvSettings) EnvVars() map[string]string { "HELM_REPOSITORY_CACHE": s.RepositoryCache, "HELM_REPOSITORY_CONFIG": s.RepositoryConfig, "HELM_NAMESPACE": s.Namespace(), - "HELM_KUBECONTEXT": s.KubeContext, - "HELM_KUBETOKEN": s.KubeToken, - "HELM_KUBEAPISERVER": s.KubeAPIServer, - } + // broken, these are populated from helm flags and not kubeconfig. + "HELM_KUBECONTEXT": s.KubeContext, + "HELM_KUBETOKEN": s.KubeToken, + "HELM_KUBEAPISERVER": s.KubeAPIServer, + } if s.KubeConfig != "" { envvars["KUBECONFIG"] = s.KubeConfig } - return envvars } -//Namespace gets the namespace from the configuration +// Namespace gets the namespace from the configuration func (s *EnvSettings) Namespace() string { - if s.namespace != "" { - return s.namespace - } - - if ns, _, err := s.RESTClientGetter().ToRawKubeConfigLoader().Namespace(); err == nil { + if ns, _, err := s.config.ToRawKubeConfigLoader().Namespace(); err == nil { return ns } return "default" } -//RESTClientGetter gets the kubeconfig from EnvSettings +// RESTClientGetter gets the kubeconfig from EnvSettings func (s *EnvSettings) RESTClientGetter() genericclioptions.RESTClientGetter { - s.configOnce.Do(func() { - clientConfig := kube.GetConfig(s.KubeConfig, s.KubeContext, s.namespace) - if s.KubeToken != "" { - clientConfig.BearerToken = &s.KubeToken - } - if s.KubeAPIServer != "" { - clientConfig.APIServer = &s.KubeAPIServer - } - - s.config = clientConfig - }) return s.config } diff --git a/pkg/getter/getter_test.go b/pkg/getter/getter_test.go index 60eb4738e..79a3338e9 100644 --- a/pkg/getter/getter_test.go +++ b/pkg/getter/getter_test.go @@ -53,9 +53,10 @@ func TestProviders(t *testing.T) { } func TestAll(t *testing.T) { - all := All(&cli.EnvSettings{ - PluginsDirectory: pluginDir, - }) + env := cli.New() + env.PluginsDirectory = pluginDir + + all := All(env) if len(all) != 3 { t.Errorf("expected 3 providers (default plus two plugins), got %d", len(all)) } @@ -66,9 +67,10 @@ func TestAll(t *testing.T) { } func TestByScheme(t *testing.T) { - g := All(&cli.EnvSettings{ - PluginsDirectory: pluginDir, - }) + env := cli.New() + env.PluginsDirectory = pluginDir + + g := All(env) if _, err := g.ByScheme("test"); err != nil { t.Error(err) } diff --git a/pkg/getter/httpgetter_test.go b/pkg/getter/httpgetter_test.go index a1288bf47..4d7ada852 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(new(cli.EnvSettings)).ByScheme("http") + g, err := All(cli.New()).ByScheme("http") if err != nil { t.Fatal(err) } diff --git a/pkg/getter/plugingetter_test.go b/pkg/getter/plugingetter_test.go index 71563e169..a18fa302b 100644 --- a/pkg/getter/plugingetter_test.go +++ b/pkg/getter/plugingetter_test.go @@ -24,9 +24,9 @@ import ( ) func TestCollectPlugins(t *testing.T) { - env := &cli.EnvSettings{ - PluginsDirectory: pluginDir, - } + env := cli.New() + env.PluginsDirectory = pluginDir + p, err := collectPlugins(env) if err != nil { t.Fatal(err) @@ -54,9 +54,8 @@ func TestPluginGetter(t *testing.T) { t.Skip("TODO: refactor this test to work on windows") } - env := &cli.EnvSettings{ - PluginsDirectory: pluginDir, - } + env := cli.New() + env.PluginsDirectory = pluginDir pg := NewPluginGetter("echo", env, "test", ".") g, err := pg() if err != nil { @@ -80,9 +79,9 @@ func TestPluginSubCommands(t *testing.T) { t.Skip("TODO: refactor this test to work on windows") } - env := &cli.EnvSettings{ - PluginsDirectory: pluginDir, - } + env := cli.New() + env.PluginsDirectory = pluginDir + pg := NewPluginGetter("echo -n", env, "test", ".") g, err := pg() if err != nil { diff --git a/pkg/kube/config.go b/pkg/kube/config.go index 624c4a1f7..e00c9acb1 100644 --- a/pkg/kube/config.go +++ b/pkg/kube/config.go @@ -19,6 +19,8 @@ package kube // import "helm.sh/helm/v3/pkg/kube" import "k8s.io/cli-runtime/pkg/genericclioptions" // GetConfig returns a Kubernetes client config. +// +// Deprecated func GetConfig(kubeconfig, context, namespace string) *genericclioptions.ConfigFlags { cf := genericclioptions.NewConfigFlags(true) cf.Namespace = &namespace diff --git a/pkg/plugin/plugin_test.go b/pkg/plugin/plugin_test.go index a869255c4..af0b61846 100644 --- a/pkg/plugin/plugin_test.go +++ b/pkg/plugin/plugin_test.go @@ -305,9 +305,8 @@ func TestSetupEnv(t *testing.T) { name := "pequod" base := filepath.Join("testdata/helmhome/helm/plugins", name) - s := &cli.EnvSettings{ - PluginsDirectory: "testdata/helmhome/helm/plugins", - } + s := cli.New() + s.PluginsDirectory = "testdata/helmhome/helm/plugins" SetupPluginEnv(s, name, base) for _, tt := range []struct {