From 8be42bae885a04b4acc242cf420911145b32ee1c Mon Sep 17 00:00:00 2001 From: Matthew Fisher Date: Wed, 19 Sep 2018 11:20:32 -0700 Subject: [PATCH] fix(helm): fix regression with TLS flags/environment variables not being parsed (#4657) * fix(helm): fix regression with TLS flags/envvars This change fixes some of the assumptions made in an earlier commit. Helm's TLS flags and environment variables were not respected because they were parsed well before execution (during settings.AddFlagsTLS()), causing erroneous behaviour at runtime. By re-introducing environment.Init(), Helm can properly parse environment variables at the correct time. One change that had to occur in this PR is the fact that we need to call settings.Init() each time we call settings.AddFlagsTLS(). This is because each command owns its own FlagSet, so we need to parse each flagset to read and propagate the environment variables correctly. I also noticed that we were maintaining two separate variables for each TLS value. Refactoring out some of the older code to all use the settings object makes the code much cleaner to read and fixes an issue where setting a flag or environment variable would propagate to the settings object, but we'd be reading from tlsEnable. I've also added some unit tests to ensure this regression doesn't occur again. Signed-off-by: Matthew Fisher * fix bug where os.ExpandEnv() on the default value causes differing behaviour Signed-off-by: Matthew Fisher * add more context to the TODO/FIXME messages Signed-off-by: Matthew Fisher --- cmd/helm/delete.go | 3 + cmd/helm/get.go | 3 + cmd/helm/get_hooks.go | 4 + cmd/helm/get_manifest.go | 4 + cmd/helm/get_notes.go | 3 + cmd/helm/get_values.go | 4 + cmd/helm/helm.go | 49 ++-- cmd/helm/helm_test.go | 310 +++++++++++++++++++++++ cmd/helm/history.go | 3 + cmd/helm/init.go | 18 ++ cmd/helm/install.go | 3 + cmd/helm/list.go | 3 + cmd/helm/release_testing.go | 3 + cmd/helm/reset.go | 3 + cmd/helm/rollback.go | 3 + cmd/helm/status.go | 3 + cmd/helm/upgrade.go | 3 + cmd/helm/version.go | 3 + pkg/helm/environment/environment.go | 49 ++-- pkg/helm/environment/environment_test.go | 1 + 20 files changed, 429 insertions(+), 46 deletions(-) diff --git a/cmd/helm/delete.go b/cmd/helm/delete.go index b78956ab6..4f52ffdd9 100755 --- a/cmd/helm/delete.go +++ b/cmd/helm/delete.go @@ -85,6 +85,9 @@ func newDeleteCmd(c helm.Interface, out io.Writer) *cobra.Command { f.Int64Var(&del.timeout, "timeout", 300, "time in seconds to wait for any individual Kubernetes operation (like Jobs for hooks)") f.StringVar(&del.description, "description", "", "specify a description for the release") + // set defaults from environment + settings.InitTLS(f) + return cmd } diff --git a/cmd/helm/get.go b/cmd/helm/get.go index 719e0779d..20a4c042f 100644 --- a/cmd/helm/get.go +++ b/cmd/helm/get.go @@ -79,6 +79,9 @@ func newGetCmd(client helm.Interface, out io.Writer) *cobra.Command { cmd.AddCommand(newGetHooksCmd(nil, out)) cmd.AddCommand(newGetNotesCmd(nil, out)) + // set defaults from environment + settings.InitTLS(f) + return cmd } diff --git a/cmd/helm/get_hooks.go b/cmd/helm/get_hooks.go index 1f288245d..310b2ec73 100644 --- a/cmd/helm/get_hooks.go +++ b/cmd/helm/get_hooks.go @@ -60,6 +60,10 @@ func newGetHooksCmd(client helm.Interface, out io.Writer) *cobra.Command { f := cmd.Flags() settings.AddFlagsTLS(f) f.Int32Var(&ghc.version, "revision", 0, "get the named release with revision") + + // set defaults from environment + settings.InitTLS(f) + return cmd } diff --git a/cmd/helm/get_manifest.go b/cmd/helm/get_manifest.go index 206c9d295..1cc7e3543 100644 --- a/cmd/helm/get_manifest.go +++ b/cmd/helm/get_manifest.go @@ -63,6 +63,10 @@ func newGetManifestCmd(client helm.Interface, out io.Writer) *cobra.Command { f := cmd.Flags() settings.AddFlagsTLS(f) f.Int32Var(&get.version, "revision", 0, "get the named release with revision") + + // set defaults from environment + settings.InitTLS(f) + return cmd } diff --git a/cmd/helm/get_notes.go b/cmd/helm/get_notes.go index eaa3bc815..c7c3d7797 100644 --- a/cmd/helm/get_notes.go +++ b/cmd/helm/get_notes.go @@ -63,6 +63,9 @@ func newGetNotesCmd(client helm.Interface, out io.Writer) *cobra.Command { settings.AddFlagsTLS(f) f.Int32Var(&get.version, "revision", 0, "get the notes of the named release with revision") + // set defaults from environment + settings.InitTLS(f) + return cmd } diff --git a/cmd/helm/get_values.go b/cmd/helm/get_values.go index 38eb5f4bb..7cdfa636f 100644 --- a/cmd/helm/get_values.go +++ b/cmd/helm/get_values.go @@ -65,6 +65,10 @@ func newGetValuesCmd(client helm.Interface, out io.Writer) *cobra.Command { f.Int32Var(&get.version, "revision", 0, "get the named release with revision") f.BoolVarP(&get.allValues, "all", "a", false, "dump all (computed) values") f.StringVar(&get.output, "output", "yaml", "output the specified format (json or yaml)") + + // set defaults from environment + settings.InitTLS(f) + return cmd } diff --git a/cmd/helm/helm.go b/cmd/helm/helm.go index 75fa2dc38..7f2bf369a 100644 --- a/cmd/helm/helm.go +++ b/cmd/helm/helm.go @@ -40,13 +40,6 @@ import ( ) var ( - tlsServerName string // overrides the server name used to verify the hostname on the returned certificates from the server. - tlsCaCertFile string // path to TLS CA certificate file - tlsCertFile string // path to TLS certificate file - tlsKeyFile string // path to TLS key file - tlsVerify bool // enable TLS and verify remote certificates - tlsEnable bool // enable TLS - tillerTunnel *kube.Tunnel settings helm_env.EnvSettings ) @@ -87,9 +80,21 @@ func newRootCmd(args []string) *cobra.Command { Long: globalUsage, SilenceUsage: true, PersistentPreRun: func(*cobra.Command, []string) { - tlsCaCertFile = os.ExpandEnv(tlsCaCertFile) - tlsCertFile = os.ExpandEnv(tlsCertFile) - tlsKeyFile = os.ExpandEnv(tlsKeyFile) + if settings.TLSCaCertFile == helm_env.DefaultTLSCaCert || settings.TLSCaCertFile == "" { + settings.TLSCaCertFile = settings.Home.TLSCaCert() + } else { + settings.TLSCaCertFile = os.ExpandEnv(settings.TLSCaCertFile) + } + if settings.TLSCertFile == helm_env.DefaultTLSCert || settings.TLSCertFile == "" { + settings.TLSCertFile = settings.Home.TLSCert() + } else { + settings.TLSCertFile = os.ExpandEnv(settings.TLSCertFile) + } + if settings.TLSKeyFile == helm_env.DefaultTLSKeyFile || settings.TLSKeyFile == "" { + settings.TLSKeyFile = settings.Home.TLSKey() + } else { + settings.TLSKeyFile = os.ExpandEnv(settings.TLSKeyFile) + } }, PersistentPostRun: func(*cobra.Command, []string) { teardown() @@ -143,6 +148,9 @@ func newRootCmd(args []string) *cobra.Command { flags.Parse(args) + // set defaults from environment + settings.Init(flags) + // Find and add plugins loadPlugins(cmd, out) @@ -275,24 +283,15 @@ func newClient() helm.Interface { options := []helm.Option{helm.Host(settings.TillerHost), helm.ConnectTimeout(settings.TillerConnectionTimeout)} if settings.TLSVerify || settings.TLSEnable { - if tlsCaCertFile == "" { - tlsCaCertFile = settings.Home.TLSCaCert() - } - if tlsCertFile == "" { - tlsCertFile = settings.Home.TLSCert() - } - if tlsKeyFile == "" { - tlsKeyFile = settings.Home.TLSKey() - } - debug("Host=%q, Key=%q, Cert=%q, CA=%q\n", tlsServerName, tlsKeyFile, tlsCertFile, tlsCaCertFile) + debug("Host=%q, Key=%q, Cert=%q, CA=%q\n", settings.TLSServerName, settings.TLSKeyFile, settings.TLSCertFile, settings.TLSCaCertFile) tlsopts := tlsutil.Options{ - ServerName: tlsServerName, - KeyFile: tlsKeyFile, - CertFile: tlsCertFile, + ServerName: settings.TLSServerName, + KeyFile: settings.TLSKeyFile, + CertFile: settings.TLSCertFile, InsecureSkipVerify: true, } - if tlsVerify { - tlsopts.CaCertFile = tlsCaCertFile + if settings.TLSVerify { + tlsopts.CaCertFile = settings.TLSCaCertFile tlsopts.InsecureSkipVerify = false } tlscfg, err := tlsutil.ClientConfig(tlsopts) diff --git a/cmd/helm/helm_test.go b/cmd/helm/helm_test.go index c872af8c3..3551eb534 100644 --- a/cmd/helm/helm_test.go +++ b/cmd/helm/helm_test.go @@ -30,6 +30,7 @@ import ( "github.com/spf13/cobra" "k8s.io/helm/pkg/helm" + "k8s.io/helm/pkg/helm/environment" "k8s.io/helm/pkg/helm/helmpath" "k8s.io/helm/pkg/proto/hapi/release" "k8s.io/helm/pkg/repo" @@ -229,6 +230,315 @@ func TestRootCmd(t *testing.T) { } } +func TestTLSFlags(t *testing.T) { + cleanup := resetEnv() + defer cleanup() + + homePath := os.Getenv("HELM_HOME") + if homePath == "" { + homePath = filepath.Join(os.Getenv("HOME"), ".helm") + } + + home := helmpath.Home(homePath) + + tests := []struct { + name string + args []string + envars map[string]string + settings environment.EnvSettings + }{ + { + name: "defaults", + args: []string{"version", "-c"}, + settings: environment.EnvSettings{ + TillerHost: "", + TillerConnectionTimeout: 300, + TillerNamespace: "kube-system", + Home: home, + Debug: false, + KubeContext: "", + KubeConfig: "", + TLSEnable: false, + TLSVerify: false, + TLSServerName: "", + TLSCaCertFile: home.TLSCaCert(), + TLSCertFile: home.TLSCert(), + TLSKeyFile: home.TLSKey(), + }, + }, + { + name: "tls enable", + args: []string{"version", "-c", "--tls"}, + settings: environment.EnvSettings{ + TillerHost: "", + TillerConnectionTimeout: 300, + TillerNamespace: "kube-system", + Home: home, + Debug: false, + KubeContext: "", + KubeConfig: "", + TLSEnable: true, + TLSVerify: false, + TLSServerName: "", + TLSCaCertFile: home.TLSCaCert(), + TLSCertFile: home.TLSCert(), + TLSKeyFile: home.TLSKey(), + }, + }, + { + name: "tls verify", + args: []string{"version", "-c", "--tls-verify"}, + settings: environment.EnvSettings{ + TillerHost: "", + TillerConnectionTimeout: 300, + TillerNamespace: "kube-system", + Home: home, + Debug: false, + KubeContext: "", + KubeConfig: "", + TLSEnable: false, + TLSVerify: true, + TLSServerName: "", + TLSCaCertFile: home.TLSCaCert(), + TLSCertFile: home.TLSCert(), + TLSKeyFile: home.TLSKey(), + }, + }, + { + name: "tls servername", + args: []string{"version", "-c", "--tls-hostname=foo"}, + settings: environment.EnvSettings{ + TillerHost: "", + TillerConnectionTimeout: 300, + TillerNamespace: "kube-system", + Home: home, + Debug: false, + KubeContext: "", + KubeConfig: "", + TLSEnable: false, + TLSVerify: false, + TLSServerName: "foo", + TLSCaCertFile: home.TLSCaCert(), + TLSCertFile: home.TLSCert(), + TLSKeyFile: home.TLSKey(), + }, + }, + { + name: "tls cacert", + args: []string{"version", "-c", "--tls-ca-cert=/foo"}, + settings: environment.EnvSettings{ + TillerHost: "", + TillerConnectionTimeout: 300, + TillerNamespace: "kube-system", + Home: home, + Debug: false, + KubeContext: "", + KubeConfig: "", + TLSEnable: false, + TLSVerify: false, + TLSServerName: "", + TLSCaCertFile: "/foo", + TLSCertFile: home.TLSCert(), + TLSKeyFile: home.TLSKey(), + }, + }, + { + name: "tls cert", + args: []string{"version", "-c", "--tls-cert=/foo"}, + settings: environment.EnvSettings{ + TillerHost: "", + TillerConnectionTimeout: 300, + TillerNamespace: "kube-system", + Home: home, + Debug: false, + KubeContext: "", + KubeConfig: "", + TLSEnable: false, + TLSVerify: false, + TLSServerName: "", + TLSCaCertFile: home.TLSCaCert(), + TLSCertFile: "/foo", + TLSKeyFile: home.TLSKey(), + }, + }, + { + name: "tls key", + args: []string{"version", "-c", "--tls-key=/foo"}, + settings: environment.EnvSettings{ + TillerHost: "", + TillerConnectionTimeout: 300, + TillerNamespace: "kube-system", + Home: home, + Debug: false, + KubeContext: "", + KubeConfig: "", + TLSEnable: false, + TLSVerify: false, + TLSServerName: "", + TLSCaCertFile: home.TLSCaCert(), + TLSCertFile: home.TLSCert(), + TLSKeyFile: "/foo", + }, + }, + { + name: "tls enable envvar", + args: []string{"version", "-c"}, + envars: map[string]string{"HELM_TLS_ENABLE": "true"}, + settings: environment.EnvSettings{ + TillerHost: "", + TillerConnectionTimeout: 300, + TillerNamespace: "kube-system", + Home: home, + Debug: false, + KubeContext: "", + KubeConfig: "", + TLSEnable: true, + TLSVerify: false, + TLSServerName: "", + TLSCaCertFile: home.TLSCaCert(), + TLSCertFile: home.TLSCert(), + TLSKeyFile: home.TLSKey(), + }, + }, + { + name: "tls verify envvar", + args: []string{"version", "-c"}, + envars: map[string]string{"HELM_TLS_VERIFY": "true"}, + settings: environment.EnvSettings{ + TillerHost: "", + TillerConnectionTimeout: 300, + TillerNamespace: "kube-system", + Home: home, + Debug: false, + KubeContext: "", + KubeConfig: "", + TLSEnable: false, + TLSVerify: true, + TLSServerName: "", + TLSCaCertFile: home.TLSCaCert(), + TLSCertFile: home.TLSCert(), + TLSKeyFile: home.TLSKey(), + }, + }, + { + name: "tls servername envvar", + args: []string{"version", "-c"}, + envars: map[string]string{"HELM_TLS_HOSTNAME": "foo"}, + settings: environment.EnvSettings{ + TillerHost: "", + TillerConnectionTimeout: 300, + TillerNamespace: "kube-system", + Home: home, + Debug: false, + KubeContext: "", + KubeConfig: "", + TLSEnable: false, + TLSVerify: false, + TLSServerName: "foo", + TLSCaCertFile: home.TLSCaCert(), + TLSCertFile: home.TLSCert(), + TLSKeyFile: home.TLSKey(), + }, + }, + { + name: "tls cacert envvar", + args: []string{"version", "-c"}, + envars: map[string]string{"HELM_TLS_CA_CERT": "/foo"}, + settings: environment.EnvSettings{ + TillerHost: "", + TillerConnectionTimeout: 300, + TillerNamespace: "kube-system", + Home: home, + Debug: false, + KubeContext: "", + KubeConfig: "", + TLSEnable: false, + TLSVerify: false, + TLSServerName: "", + TLSCaCertFile: "/foo", + TLSCertFile: home.TLSCert(), + TLSKeyFile: home.TLSKey(), + }, + }, + { + name: "tls cert envvar", + args: []string{"version", "-c"}, + envars: map[string]string{"HELM_TLS_CERT": "/foo"}, + settings: environment.EnvSettings{ + TillerHost: "", + TillerConnectionTimeout: 300, + TillerNamespace: "kube-system", + Home: home, + Debug: false, + KubeContext: "", + KubeConfig: "", + TLSEnable: false, + TLSVerify: false, + TLSServerName: "", + TLSCaCertFile: home.TLSCaCert(), + TLSCertFile: "/foo", + TLSKeyFile: home.TLSKey(), + }, + }, + { + name: "tls key envvar", + args: []string{"version", "-c"}, + envars: map[string]string{"HELM_TLS_KEY": "/foo"}, + settings: environment.EnvSettings{ + TillerHost: "", + TillerConnectionTimeout: 300, + TillerNamespace: "kube-system", + Home: home, + Debug: false, + KubeContext: "", + KubeConfig: "", + TLSEnable: false, + TLSVerify: false, + TLSServerName: "", + TLSCaCertFile: home.TLSCaCert(), + TLSCertFile: home.TLSCert(), + TLSKeyFile: "/foo", + }, + }, + } + + // ensure not set locally + tlsEnvvars := []string{ + "HELM_TLS_HOSTNAME", + "HELM_TLS_CA_CERT", + "HELM_TLS_CERT", + "HELM_TLS_KEY", + "HELM_TLS_VERIFY", + "HELM_TLS_ENABLE", + } + + for i := range tlsEnvvars { + os.Unsetenv(tlsEnvvars[i]) + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + + for k, v := range tt.envars { + os.Setenv(k, v) + defer os.Unsetenv(k) + } + + cmd := newRootCmd(tt.args) + cmd.SetOutput(ioutil.Discard) + cmd.SetArgs(tt.args) + cmd.Run = func(*cobra.Command, []string) {} + if err := cmd.Execute(); err != nil { + t.Errorf("unexpected error: %s", err) + } + + if settings != tt.settings { + t.Errorf("expected settings %v, got %v", tt.settings, settings) + } + }) + } +} + func resetEnv() func() { origSettings := settings origEnv := os.Environ() diff --git a/cmd/helm/history.go b/cmd/helm/history.go index 51bc34e75..365346e89 100644 --- a/cmd/helm/history.go +++ b/cmd/helm/history.go @@ -93,6 +93,9 @@ func newHistoryCmd(c helm.Interface, w io.Writer) *cobra.Command { f.UintVar(&his.colWidth, "col-width", 60, "specifies the max column width of output") f.StringVarP(&his.outputFormat, "output", "o", "table", "prints the output in the specified format (json|table|yaml)") + // set defaults from environment + settings.InitTLS(f) + return cmd } diff --git a/cmd/helm/init.go b/cmd/helm/init.go index 4be54e675..b628dc008 100644 --- a/cmd/helm/init.go +++ b/cmd/helm/init.go @@ -70,6 +70,12 @@ var ( // This is the IPv4 loopback, not localhost, because we have to force IPv4 // for Dockerized Helm: https://github.com/kubernetes/helm/issues/1410 localRepositoryURL = "http://127.0.0.1:8879/charts" + tlsServerName string // overrides the server name used to verify the hostname on the returned certificates from the server. + tlsCaCertFile string // path to TLS CA certificate file + tlsCertFile string // path to TLS certificate file + tlsKeyFile string // path to TLS key file + tlsVerify bool // enable TLS and verify remote certificates + tlsEnable bool // enable TLS ) type initCmd struct { @@ -121,6 +127,10 @@ func newInitCmd(out io.Writer) *cobra.Command { f.BoolVar(&i.skipRefresh, "skip-refresh", false, "do not refresh (download) the local repository cache") f.BoolVar(&i.wait, "wait", false, "block until Tiller is running and ready to receive requests") + // TODO: replace TLS flags with pkg/helm/environment.AddFlagsTLS() in Helm 3 + // + // NOTE (bacongobbler): we can't do this in Helm 2 because the flag names differ, and `helm init --tls-ca-cert` + // doesn't conform with the rest of the TLS flag names (should be --tiller-tls-ca-cert in Helm 3) f.BoolVar(&tlsEnable, "tiller-tls", false, "install Tiller with TLS enabled") f.BoolVar(&tlsVerify, "tiller-tls-verify", false, "install Tiller with TLS enabled and to verify remote certificates") f.StringVar(&tlsKeyFile, "tiller-tls-key", "", "path to TLS key file to install with Tiller") @@ -166,6 +176,14 @@ func (i *initCmd) tlsOptions() error { return errors.New("missing required TLS CA file") } } + + // FIXME: remove once we use pkg/helm/environment.AddFlagsTLS() in Helm 3 + settings.TLSEnable = tlsEnable + settings.TLSVerify = tlsVerify + settings.TLSServerName = tlsServerName + settings.TLSCaCertFile = tlsCaCertFile + settings.TLSCertFile = tlsCertFile + settings.TLSKeyFile = tlsKeyFile } return nil } diff --git a/cmd/helm/install.go b/cmd/helm/install.go index 7f84f3355..c5c6b9a49 100644 --- a/cmd/helm/install.go +++ b/cmd/helm/install.go @@ -220,6 +220,9 @@ func newInstallCmd(c helm.Interface, out io.Writer) *cobra.Command { f.BoolVar(&inst.depUp, "dep-up", false, "run helm dependency update before installing the chart") f.StringVar(&inst.description, "description", "", "specify a description for the release") + // set defaults from environment + settings.InitTLS(f) + return cmd } diff --git a/cmd/helm/list.go b/cmd/helm/list.go index 384fca619..3ca3fbbfa 100644 --- a/cmd/helm/list.go +++ b/cmd/helm/list.go @@ -140,6 +140,9 @@ func newListCmd(client helm.Interface, out io.Writer) *cobra.Command { // TODO: Do we want this as a feature of 'helm list'? //f.BoolVar(&list.superseded, "history", true, "show historical releases") + // set defaults from environment + settings.InitTLS(f) + return cmd } diff --git a/cmd/helm/release_testing.go b/cmd/helm/release_testing.go index c7231cf04..f39d9b81f 100644 --- a/cmd/helm/release_testing.go +++ b/cmd/helm/release_testing.go @@ -68,6 +68,9 @@ func newReleaseTestCmd(c helm.Interface, out io.Writer) *cobra.Command { f.Int64Var(&rlsTest.timeout, "timeout", 300, "time in seconds to wait for any individual Kubernetes operation (like Jobs for hooks)") f.BoolVar(&rlsTest.cleanup, "cleanup", false, "delete test pods upon completion") + // set defaults from environment + settings.InitTLS(f) + return cmd } diff --git a/cmd/helm/reset.go b/cmd/helm/reset.go index ffae0a613..eb1dce7f1 100644 --- a/cmd/helm/reset.go +++ b/cmd/helm/reset.go @@ -81,6 +81,9 @@ func newResetCmd(client helm.Interface, out io.Writer) *cobra.Command { f.BoolVarP(&d.force, "force", "f", false, "forces Tiller uninstall even if there are releases installed, or if Tiller is not in ready state. Releases are not deleted.)") f.BoolVar(&d.removeHelmHome, "remove-helm-home", false, "if set deletes $HELM_HOME") + // set defaults from environment + settings.InitTLS(f) + return cmd } diff --git a/cmd/helm/rollback.go b/cmd/helm/rollback.go index a06b205c8..0c46fa818 100644 --- a/cmd/helm/rollback.go +++ b/cmd/helm/rollback.go @@ -87,6 +87,9 @@ func newRollbackCmd(c helm.Interface, out io.Writer) *cobra.Command { f.BoolVar(&rollback.wait, "wait", false, "if set, will wait until all Pods, PVCs, Services, and minimum number of Pods of a Deployment are in a ready state before marking the release as successful. It will wait for as long as --timeout") f.StringVar(&rollback.description, "description", "", "specify a description for the release") + // set defaults from environment + settings.InitTLS(f) + return cmd } diff --git a/cmd/helm/status.go b/cmd/helm/status.go index fe53081a4..b03453adc 100644 --- a/cmd/helm/status.go +++ b/cmd/helm/status.go @@ -81,6 +81,9 @@ func newStatusCmd(client helm.Interface, out io.Writer) *cobra.Command { f.Int32Var(&status.version, "revision", 0, "if set, display the status of the named release with revision") f.StringVarP(&status.outfmt, "output", "o", "", "output the status in the specified format (json or yaml)") + // set defaults from environment + settings.InitTLS(f) + return cmd } diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go index 905a2c175..d05987b8a 100644 --- a/cmd/helm/upgrade.go +++ b/cmd/helm/upgrade.go @@ -177,6 +177,9 @@ func newUpgradeCmd(client helm.Interface, out io.Writer) *cobra.Command { f.MarkDeprecated("disable-hooks", "use --no-hooks instead") + // set defaults from environment + settings.InitTLS(f) + return cmd } diff --git a/cmd/helm/version.go b/cmd/helm/version.go index 1c292e19d..a803a990b 100644 --- a/cmd/helm/version.go +++ b/cmd/helm/version.go @@ -83,6 +83,9 @@ func newVersionCmd(c helm.Interface, out io.Writer) *cobra.Command { f.BoolVar(&version.short, "short", false, "print the version number") f.StringVar(&version.template, "template", "", "template for version string format") + // set defaults from environment + settings.InitTLS(f) + return cmd } diff --git a/pkg/helm/environment/environment.go b/pkg/helm/environment/environment.go index 05d955d69..76348c3bd 100644 --- a/pkg/helm/environment/environment.go +++ b/pkg/helm/environment/environment.go @@ -87,17 +87,6 @@ func (s *EnvSettings) AddFlags(fs *pflag.FlagSet) { fs.BoolVar(&s.Debug, "debug", false, "enable verbose output") fs.StringVar(&s.TillerNamespace, "tiller-namespace", "kube-system", "namespace of Tiller") fs.Int64Var(&s.TillerConnectionTimeout, "tiller-connection-timeout", int64(300), "the duration (in seconds) Helm will wait to establish a connection to tiller") - - envMap := map[string]string{ - "debug": "HELM_DEBUG", - "home": "HELM_HOME", - "host": "HELM_HOST", - "tiller-namespace": "TILLER_NAMESPACE", - } - - for name, envar := range envMap { - setFlagFromEnv(name, envar, fs) - } } // AddFlagsTLS adds the flags for supporting client side TLS to the given flagset. @@ -108,23 +97,38 @@ func (s *EnvSettings) AddFlagsTLS(fs *pflag.FlagSet) { fs.StringVar(&s.TLSKeyFile, "tls-key", DefaultTLSKeyFile, "path to TLS key file") fs.BoolVar(&s.TLSVerify, "tls-verify", DefaultTLSVerify, "enable TLS for request and verify remote") fs.BoolVar(&s.TLSEnable, "tls", DefaultTLSEnable, "enable TLS for request") +} - envMap := map[string]string{ - "tls-hostname": "HELM_TLS_HOSTNAME", - "tls-ca-cert": "HELM_TLS_CA_CERT", - "tls-cert": "HELM_TLS_CERT", - "tls-key": "HELM_TLS_KEY", - "tls-verify": "HELM_TLS_VERIFY", - "tls": "HELM_TLS_ENABLE", +// Init sets values from the environment. +func (s *EnvSettings) Init(fs *pflag.FlagSet) { + for name, envar := range envMap { + setFlagFromEnv(name, envar, fs) } +} - for name, envar := range envMap { +// InitTLS sets TLS values from the environment. +func (s *EnvSettings) InitTLS(fs *pflag.FlagSet) { + for name, envar := range tlsEnvMap { setFlagFromEnv(name, envar, fs) } } -// Init is deprecated; calling `.AddFlags` or `.AddFlagsTLS` directly will set the flags to their default values from the environment, so this is a no-op. -func (s *EnvSettings) Init(fs *pflag.FlagSet) {} +// envMap maps flag names to envvars +var envMap = map[string]string{ + "debug": "HELM_DEBUG", + "home": "HELM_HOME", + "host": "HELM_HOST", + "tiller-namespace": "TILLER_NAMESPACE", +} + +var tlsEnvMap = map[string]string{ + "tls-hostname": "HELM_TLS_HOSTNAME", + "tls-ca-cert": "HELM_TLS_CA_CERT", + "tls-cert": "HELM_TLS_CERT", + "tls-key": "HELM_TLS_KEY", + "tls-verify": "HELM_TLS_VERIFY", + "tls": "HELM_TLS_ENABLE", +} // PluginDirs is the path to the plugin directories. func (s EnvSettings) PluginDirs() string { @@ -134,6 +138,9 @@ func (s EnvSettings) PluginDirs() string { return s.Home.Plugins() } +// setFlagFromEnv looks up and sets a flag if the corresponding environment variable changed. +// if the flag with the corresponding name was set during fs.Parse(), then the environment +// variable is ignored. func setFlagFromEnv(name, envar string, fs *pflag.FlagSet) { if fs.Changed(name) { return diff --git a/pkg/helm/environment/environment_test.go b/pkg/helm/environment/environment_test.go index fb05254ed..675582cf6 100644 --- a/pkg/helm/environment/environment_test.go +++ b/pkg/helm/environment/environment_test.go @@ -123,6 +123,7 @@ func TestEnvSettings(t *testing.T) { flags.Parse(tt.args) settings.Init(flags) + settings.InitTLS(flags) if settings.Home != helmpath.Home(tt.home) { t.Errorf("expected home %q, got %q", tt.home, settings.Home)