fix(helm): fix regression with TLS flags/environment variables not being parsed (helm#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 <matt.fisher@microsoft.com>

* fix bug where os.ExpandEnv() on the default value causes differing behaviour

Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com>

* add more context to the TODO/FIXME messages

Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com>
(cherry picked from commit 8be42ba)

Author: Matthew Fisher

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
 }
 

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
 }
 

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
 }
 

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
 }
 

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
 }
 

cmd/helm/get_values.go

@@ -62,6 +62,10 @@ func newGetValuesCmd(client helm.Interface, out io.Writer) *cobra.Command {
 	settings.AddFlagsTLS(f)
 	f.Int32Var(&get.version, "revision", 0, "get the named release with revision")
 	f.BoolVarP(&get.allValues, "all", "a", false, "dump all (computed) values")
+
+	// set defaults from environment
+	settings.InitTLS(f)
+
 	return cmd
 }
 

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)