Repository: helm/helm
Pull request #10944 (2022-05-09 16:32:37)

feat(*): add flags/env for kube api tls overrides
## What this PR does

Add a new flags and associated environment variables to override the TLS Settings used when constructing the Kube Client.

- `--kube-insecure-skip-tls-verify | HELM_KUBEINSECURE_SKIP_TLS_VERIFY`: if true, the kube api server's certificate will not be checked for validity. This will make your HTTPS connections insecure
- `--kube-tls-server-name | HELM_KUBETLS_SERVER_NAME`: server name to use for kube api server certificate validation. If it is not provided, the hostname used to contact the server is used

This PR passes those arguments through the `cli.EnvSettings` to the Kubernetes CLI options it constructs

## Why we need it:

This is necessary for using helm in some situations where direct access to the kuberentes api is not available or is made through a TCP Proxy where TLS server name validation will fail. It's also a feature of `kubectl` which is unmapped to helm, so is currently inaccessible.

### My use-case

When using [HashiCorp Boundary](https://github.com/hashicorp/boundary) to run Helm via [boundary connect -exec=helm](https://www.boundaryproject.io/docs/getting-started/connect-to-target), it creates a TCP proxy on `localhost` to the remote API server. If `localhost` or `127.0.0.1` etc is not part of the Kubernetes API Certificate, then this will fail.  In cases where you have no control over the certificate, such as those managed by a cloud provider (EKS, GKE...) then there is no work around, and so there is no way to run helm commands. 

```
$ boundary connect -target-id=ttcp_abcdEfghIJ -exec=helm -- \
    --kube-apiserver "https://{{boundary.addr}}" ls

Proxy listening information:
  Address:             127.0.0.1
  Connection Limit:    -1
  Expiration:          Mon, 09 May 2022 11:55:41 EDT
  Port:                65422
  Protocol:            tcp
  Session ID:          s_A8qleWlYYF
Error: Kubernetes cluster unreachable: Get "https://127.0.0.1:65422/version": x509: certificate is valid for 172.16.34.35, not 127.0.0.1
```

With this PR, it would now be possible to run helm commands:

```
$ boundary connect -target-id=ttcp_abcdEfghIJ -exec=helm -- \
  --kube-apiserver "https://{{boundary.addr}}" \
  --kube-tls-server-name "C0DECAFEC0DECAFEC0DECAFEC0DECAFE.gr7.us-west-2.eks.amazonaws.com" \
  ls
NAME    NAMESPACE       REVISION        UPDATED STATUS  CHART   APP VERSION
...
```


**Special notes for your reviewer**:

related: #8487
previous attempts: #8622, #9727

**If applicable**:
- [x] this PR contains documentation
- [x] this PR contains unit tests
- [ ] this PR has been tested for backwards compatibility (This should be backwards compatible since these are new flags)


justenwalker: > LGTM. Needs another maintainer’s approval before we can merge. Thanks.

Sure. I've seen @marckhouzam and @hickeyma review features of a similar nature in past PRs - perhaps one of them may be keen on reviewing this as well?

justenwalker: @yxxhero I belive i've taken care of your two review comments:

1. Doc changes to use "Kubernetes API" instead of "kube api" via 939fb4c9d41d847d8419219020598dde11cdf3a5
2. Unit Tests for pkg/cli envOrBool via fe8ff05bb2caa58cd2710db06e1748f618faa181
yxxhero: @justenwalker require squash.
justenwalker: > @justenwalker require squash.

@yxxhero squashed

Patch:
diff --git a/cmd/helm/load_plugins.go b/cmd/helm/load_plugins.go
index a97d49a93..6b67ac28f 100644
--- a/cmd/helm/load_plugins.go
+++ b/cmd/helm/load_plugins.go
@@ -154,7 +154,7 @@ func callPluginExecutable(pluginName string, main string, argv []string, out io.
 func manuallyProcessArgs(args []string) ([]string, []string) {
 	known := []string{}
 	unknown := []string{}
-	kvargs := []string{"--kube-context", "--namespace", "-n", "--kubeconfig", "--kube-apiserver", "--kube-token", "--kube-as-user", "--kube-as-group", "--kube-ca-file", "--registry-config", "--repository-cache", "--repository-config"}
+	kvargs := []string{"--kube-context", "--namespace", "-n", "--kubeconfig", "--kube-apiserver", "--kube-token", "--kube-as-user", "--kube-as-group", "--kube-ca-file", "--registry-config", "--repository-cache", "--repository-config", "--insecure-skip-tls-verify", "--tls-server-name"}
 	knownArg := func(a string) bool {
 		for _, pre := range kvargs {
 			if strings.HasPrefix(a, pre+"=") {
diff --git a/cmd/helm/root.go b/cmd/helm/root.go
index 53b0c0ce8..0230cf5b1 100644
--- a/cmd/helm/root.go
+++ b/cmd/helm/root.go
@@ -67,6 +67,8 @@ Environment variables:
 | $HELM_KUBEASUSER                   | set the Username to impersonate for the operation.                                                |
 | $HELM_KUBECONTEXT                  | set the name of the kubeconfig context.                                                           |
 | $HELM_KUBETOKEN                    | set the Bearer KubeToken used for authentication.                                                 |
+| $HELM_KUBEINSECURE_SKIP_TLS_VERIFY | indicate if the Kubernetes API server's certificate validation should be skipped (insecure)       |
+| $HELM_KUBETLS_SERVER_NAME          | set the server name used to validate the Kubernetes API server certificate                        |
 | $HELM_BURST_LIMIT                  | set the default burst limit in the case the server contains many CRDs (default 100, -1 to disable)|
 
 Helm stores cache, configuration, and data based on the following configuration order:
diff --git a/pkg/cli/environment.go b/pkg/cli/environment.go
index b17172db4..ac3093629 100644
--- a/pkg/cli/environment.go
+++ b/pkg/cli/environment.go
@@ -60,6 +60,12 @@ type EnvSettings struct {
 	KubeAPIServer string
 	// Custom certificate authority file.
 	KubeCaFile string
+	// KubeInsecureSkipTLSVerify indicates if server's certificate will not be checked for validity.
+	// This makes the HTTPS connections insecure
+	KubeInsecureSkipTLSVerify bool
+	// KubeTLSServerName overrides the name to use for server certificate validation.
+	// If it is not provided, the hostname used to contact the server is used
+	KubeTLSServerName string
 	// Debug indicates whether or not Helm is running in Debug mode.
 	Debug bool
 	// RegistryConfig is the path to the registry config file.
@@ -78,19 +84,21 @@ type EnvSettings struct {
 
 func New() *EnvSettings {
 	env := &EnvSettings{
-		namespace:        os.Getenv("HELM_NAMESPACE"),
-		MaxHistory:       envIntOr("HELM_MAX_HISTORY", defaultMaxHistory),
-		KubeContext:      os.Getenv("HELM_KUBECONTEXT"),
-		KubeToken:        os.Getenv("HELM_KUBETOKEN"),
-		KubeAsUser:       os.Getenv("HELM_KUBEASUSER"),
-		KubeAsGroups:     envCSV("HELM_KUBEASGROUPS"),
-		KubeAPIServer:    os.Getenv("HELM_KUBEAPISERVER"),
-		KubeCaFile:       os.Getenv("HELM_KUBECAFILE"),
-		PluginsDirectory: envOr("HELM_PLUGINS", helmpath.DataPath("plugins")),
-		RegistryConfig:   envOr("HELM_REGISTRY_CONFIG", helmpath.ConfigPath("registry/config.json")),
-		RepositoryConfig: envOr("HELM_REPOSITORY_CONFIG", helmpath.ConfigPath("repositories.yaml")),
-		RepositoryCache:  envOr("HELM_REPOSITORY_CACHE", helmpath.CachePath("repository")),
-		BurstLimit:       envIntOr("HELM_BURST_LIMIT", defaultBurstLimit),
+		namespace:                 os.Getenv("HELM_NAMESPACE"),
+		MaxHistory:                envIntOr("HELM_MAX_HISTORY", defaultMaxHistory),
+		KubeContext:               os.Getenv("HELM_KUBECONTEXT"),
+		KubeToken:                 os.Getenv("HELM_KUBETOKEN"),
+		KubeAsUser:                os.Getenv("HELM_KUBEASUSER"),
+		KubeAsGroups:              envCSV("HELM_KUBEASGROUPS"),
+		KubeAPIServer:             os.Getenv("HELM_KUBEAPISERVER"),
+		KubeCaFile:                os.Getenv("HELM_KUBECAFILE"),
+		KubeTLSServerName:         os.Getenv("HELM_KUBETLS_SERVER_NAME"),
+		KubeInsecureSkipTLSVerify: envBoolOr("HELM_KUBEINSECURE_SKIP_TLS_VERIFY", false),
+		PluginsDirectory:          envOr("HELM_PLUGINS", helmpath.DataPath("plugins")),
+		RegistryConfig:            envOr("HELM_REGISTRY_CONFIG", helmpath.ConfigPath("registry/config.json")),
+		RepositoryConfig:          envOr("HELM_REPOSITORY_CONFIG", helmpath.ConfigPath("repositories.yaml")),
+		RepositoryCache:           envOr("HELM_REPOSITORY_CACHE", helmpath.CachePath("repository")),
+		BurstLimit:                envIntOr("HELM_BURST_LIMIT", defaultBurstLimit),
 	}
 	env.Debug, _ = strconv.ParseBool(os.Getenv("HELM_DEBUG"))
 
@@ -103,6 +111,8 @@ func New() *EnvSettings {
 		CAFile:           &env.KubeCaFile,
 		KubeConfig:       &env.KubeConfig,
 		Impersonate:      &env.KubeAsUser,
+		Insecure:         &env.KubeInsecureSkipTLSVerify,
+		TLSServerName:    &env.KubeTLSServerName,
 		ImpersonateGroup: &env.KubeAsGroups,
 		WrapConfigFn: func(config *rest.Config) *rest.Config {
 			config.Burst = env.BurstLimit
@@ -122,6 +132,8 @@ func (s *EnvSettings) AddFlags(fs *pflag.FlagSet) {
 	fs.StringArrayVar(&s.KubeAsGroups, "kube-as-group", s.KubeAsGroups, "group to impersonate for the operation, this flag can be repeated to specify multiple groups.")
 	fs.StringVar(&s.KubeAPIServer, "kube-apiserver", s.KubeAPIServer, "the address and the port for the Kubernetes API server")
 	fs.StringVar(&s.KubeCaFile, "kube-ca-file", s.KubeCaFile, "the certificate authority file for the Kubernetes API server connection")
+	fs.StringVar(&s.KubeTLSServerName, "kube-tls-server-name", s.KubeTLSServerName, "server name to use for Kubernetes API server certificate validation. If it is not provided, the hostname used to contact the server is used")
+	fs.BoolVar(&s.KubeInsecureSkipTLSVerify, "kube-insecure-skip-tls-verify", s.KubeInsecureSkipTLSVerify, "if true, the Kubernetes API server's certificate will not be checked for validity. This will make your HTTPS connections insecure")
 	fs.BoolVar(&s.Debug, "debug", s.Debug, "enable verbose output")
 	fs.StringVar(&s.RegistryConfig, "registry-config", s.RegistryConfig, "path to the registry config file")
 	fs.StringVar(&s.RepositoryConfig, "repository-config", s.RepositoryConfig, "path to the file containing repository names and URLs")
@@ -136,6 +148,18 @@ func envOr(name, def string) string {
 	return def
 }
 
+func envBoolOr(name string, def bool) bool {
+	if name == "" {
+		return def
+	}
+	envVal := envOr(name, strconv.FormatBool(def))
+	ret, err := strconv.ParseBool(envVal)
+	if err != nil {
+		return def
+	}
+	return ret
+}
+
 func envIntOr(name string, def int) int {
 	if name == "" {
 		return def
@@ -172,12 +196,14 @@ func (s *EnvSettings) EnvVars() map[string]string {
 		"HELM_BURST_LIMIT":       strconv.Itoa(s.BurstLimit),
 
 		// broken, these are populated from helm flags and not kubeconfig.
-		"HELM_KUBECONTEXT":   s.KubeContext,
-		"HELM_KUBETOKEN":     s.KubeToken,
-		"HELM_KUBEASUSER":    s.KubeAsUser,
-		"HELM_KUBEASGROUPS":  strings.Join(s.KubeAsGroups, ","),
-		"HELM_KUBEAPISERVER": s.KubeAPIServer,
-		"HELM_KUBECAFILE":    s.KubeCaFile,
+		"HELM_KUBECONTEXT":                  s.KubeContext,
+		"HELM_KUBETOKEN":                    s.KubeToken,
+		"HELM_KUBEASUSER":                   s.KubeAsUser,
+		"HELM_KUBEASGROUPS":                 strings.Join(s.KubeAsGroups, ","),
+		"HELM_KUBEAPISERVER":                s.KubeAPIServer,
+		"HELM_KUBECAFILE":                   s.KubeCaFile,
+		"HELM_KUBEINSECURE_SKIP_TLS_VERIFY": strconv.FormatBool(s.KubeInsecureSkipTLSVerify),
+		"HELM_KUBETLS_SERVER_NAME":          s.KubeTLSServerName,
 	}
 	if s.KubeConfig != "" {
 		envvars["KUBECONFIG"] = s.KubeConfig


Test patch:
diff --git a/cmd/helm/testdata/output/env-comp.txt b/cmd/helm/testdata/output/env-comp.txt
index e14e83472..b7d93c12e 100644
--- a/cmd/helm/testdata/output/env-comp.txt
+++ b/cmd/helm/testdata/output/env-comp.txt
@@ -9,6 +9,8 @@ HELM_KUBEASGROUPS
 HELM_KUBEASUSER
 HELM_KUBECAFILE
 HELM_KUBECONTEXT
+HELM_KUBEINSECURE_SKIP_TLS_VERIFY
+HELM_KUBETLS_SERVER_NAME
 HELM_KUBETOKEN
 HELM_MAX_HISTORY
 HELM_NAMESPACE
diff --git a/pkg/cli/environment_test.go b/pkg/cli/environment_test.go
index d77799c9d..dbf056e3a 100644
--- a/pkg/cli/environment_test.go
+++ b/pkg/cli/environment_test.go
@@ -48,13 +48,15 @@ func TestEnvSettings(t *testing.T) {
 		envvars map[string]string
 
 		// expected values
-		ns, kcontext string
-		debug        bool
-		maxhistory   int
-		kubeAsUser   string
-		kubeAsGroups []string
-		kubeCaFile   string
-		burstLimit   int
+		ns, kcontext  string
+		debug         bool
+		maxhistory    int
+		kubeAsUser    string
+		kubeAsGroups  []string
+		kubeCaFile    string
+		kubeInsecure  bool
+		kubeTLSServer string
+		burstLimit    int
 	}{
 		{
 			name:       "defaults",
@@ -63,38 +65,44 @@ func TestEnvSettings(t *testing.T) {
 			burstLimit: defaultBurstLimit,
 		},
 		{
-			name:         "with flags set",
-			args:         "--debug --namespace=myns --kube-as-user=poro --kube-as-group=admins --kube-as-group=teatime --kube-as-group=snackeaters --kube-ca-file=/tmp/ca.crt --burst-limit 100",
-			ns:           "myns",
-			debug:        true,
-			maxhistory:   defaultMaxHistory,
-			burstLimit:   100,
-			kubeAsUser:   "poro",
-			kubeAsGroups: []string{"admins", "teatime", "snackeaters"},
-			kubeCaFile:   "/tmp/ca.crt",
+			name:          "with flags set",
+			args:          "--debug --namespace=myns --kube-as-user=poro --kube-as-group=admins --kube-as-group=teatime --kube-as-group=snackeaters --kube-ca-file=/tmp/ca.crt --burst-limit 100 --kube-insecure-skip-tls-verify=true --kube-tls-server-name=example.org",
+			ns:            "myns",
+			debug:         true,
+			maxhistory:    defaultMaxHistory,
+			burstLimit:    100,
+			kubeAsUser:    "poro",
+			kubeAsGroups:  []string{"admins", "teatime", "snackeaters"},
+			kubeCaFile:    "/tmp/ca.crt",
+			kubeTLSServer: "example.org",
+			kubeInsecure:  true,
 		},
 		{
-			name:         "with envvars set",
-			envvars:      map[string]string{"HELM_DEBUG": "1", "HELM_NAMESPACE": "yourns", "HELM_KUBEASUSER": "pikachu", "HELM_KUBEASGROUPS": ",,,operators,snackeaters,partyanimals", "HELM_MAX_HISTORY": "5", "HELM_KUBECAFILE": "/tmp/ca.crt", "HELM_BURST_LIMIT": "150"},
-			ns:           "yourns",
-			maxhistory:   5,
-			burstLimit:   150,
-			debug:        true,
-			kubeAsUser:   "pikachu",
-			kubeAsGroups: []string{"operators", "snackeaters", "partyanimals"},
-			kubeCaFile:   "/tmp/ca.crt",
+			name:          "with envvars set",
+			envvars:       map[string]string{"HELM_DEBUG": "1", "HELM_NAMESPACE": "yourns", "HELM_KUBEASUSER": "pikachu", "HELM_KUBEASGROUPS": ",,,operators,snackeaters,partyanimals", "HELM_MAX_HISTORY": "5", "HELM_KUBECAFILE": "/tmp/ca.crt", "HELM_BURST_LIMIT": "150", "HELM_KUBEINSECURE_SKIP_TLS_VERIFY": "true", "HELM_KUBETLS_SERVER_NAME": "example.org"},
+			ns:            "yourns",
+			maxhistory:    5,
+			burstLimit:    150,
+			debug:         true,
+			kubeAsUser:    "pikachu",
+			kubeAsGroups:  []string{"operators", "snackeaters", "partyanimals"},
+			kubeCaFile:    "/tmp/ca.crt",
+			kubeTLSServer: "example.org",
+			kubeInsecure:  true,
 		},
 		{
-			name:         "with flags and envvars set",
-			args:         "--debug --namespace=myns --kube-as-user=poro --kube-as-group=admins --kube-as-group=teatime --kube-as-group=snackeaters --kube-ca-file=/my/ca.crt --burst-limit 175",
-			envvars:      map[string]string{"HELM_DEBUG": "1", "HELM_NAMESPACE": "yourns", "HELM_KUBEASUSER": "pikachu", "HELM_KUBEASGROUPS": ",,,operators,snackeaters,partyanimals", "HELM_MAX_HISTORY": "5", "HELM_KUBECAFILE": "/tmp/ca.crt", "HELM_BURST_LIMIT": "200"},
-			ns:           "myns",
-			debug:        true,
-			maxhistory:   5,
-			burstLimit:   175,
-			kubeAsUser:   "poro",
-			kubeAsGroups: []string{"admins", "teatime", "snackeaters"},
-			kubeCaFile:   "/my/ca.crt",
+			name:          "with flags and envvars set",
+			args:          "--debug --namespace=myns --kube-as-user=poro --kube-as-group=admins --kube-as-group=teatime --kube-as-group=snackeaters --kube-ca-file=/my/ca.crt --burst-limit 175 --kube-insecure-skip-tls-verify=true --kube-tls-server-name=example.org",
+			envvars:       map[string]string{"HELM_DEBUG": "1", "HELM_NAMESPACE": "yourns", "HELM_KUBEASUSER": "pikachu", "HELM_KUBEASGROUPS": ",,,operators,snackeaters,partyanimals", "HELM_MAX_HISTORY": "5", "HELM_KUBECAFILE": "/tmp/ca.crt", "HELM_BURST_LIMIT": "200", "HELM_KUBEINSECURE_SKIP_TLS_VERIFY": "true", "HELM_KUBETLS_SERVER_NAME": "example.org"},
+			ns:            "myns",
+			debug:         true,
+			maxhistory:    5,
+			burstLimit:    175,
+			kubeAsUser:    "poro",
+			kubeAsGroups:  []string{"admins", "teatime", "snackeaters"},
+			kubeCaFile:    "/my/ca.crt",
+			kubeTLSServer: "example.org",
+			kubeInsecure:  true,
 		},
 	}
 
@@ -136,6 +144,89 @@ func TestEnvSettings(t *testing.T) {
 			if tt.burstLimit != settings.BurstLimit {
 				t.Errorf("expected BurstLimit %d, got %d", tt.burstLimit, settings.BurstLimit)
 			}
+			if tt.kubeInsecure != settings.KubeInsecureSkipTLSVerify {
+				t.Errorf("expected kubeInsecure %t, got %t", tt.kubeInsecure, settings.KubeInsecureSkipTLSVerify)
+			}
+			if tt.kubeTLSServer != settings.KubeTLSServerName {
+				t.Errorf("expected kubeTLSServer %q, got %q", tt.kubeTLSServer, settings.KubeTLSServerName)
+			}
+		})
+	}
+}
+
+func TestEnvOrBool(t *testing.T) {
+	const envName = "TEST_ENV_OR_BOOL"
+	tests := []struct {
+		name     string
+		env      string
+		val      string
+		def      bool
+		expected bool
+	}{
+		{
+			name:     "unset with default false",
+			def:      false,
+			expected: false,
+		},
+		{
+			name:     "unset with default true",
+			def:      true,
+			expected: true,
+		},
+		{
+			name:     "blank env with default false",
+			env:      envName,
+			def:      false,
+			expected: false,
+		},
+		{
+			name:     "blank env with default true",
+			env:      envName,
+			def:      true,
+			expected: true,
+		},
+		{
+			name:     "env true with default false",
+			env:      envName,
+			val:      "true",
+			def:      false,
+			expected: true,
+		},
+		{
+			name:     "env false with default true",
+			env:      envName,
+			val:      "false",
+			def:      true,
+			expected: false,
+		},
+		{
+			name:     "env fails parsing with default true",
+			env:      envName,
+			val:      "NOT_A_BOOL",
+			def:      true,
+			expected: true,
+		},
+		{
+			name:     "env fails parsing with default false",
+			env:      envName,
+			val:      "NOT_A_BOOL",
+			def:      false,
+			expected: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.env != "" {
+				t.Cleanup(func() {
+					os.Unsetenv(tt.env)
+				})
+				os.Setenv(tt.env, tt.val)
+			}
+			actual := envBoolOr(tt.env, tt.def)
+			if actual != tt.expected {
+				t.Errorf("expected result %t, got %t", tt.expected, actual)
+			}
 		})
 	}
 }