Enable secure config multicluster (#1902)

* Parse base64-encoded CAcerts during kubeops initialization.

* Try embedding genericclioptions.ConfigFlags with rest Config.

* Update to store CAFile in a tmp dir on container.

* Remove now unused code.

* Better name for deferFn and directory.

chart/kubeapps/templates/kubeops-deployment.yaml

@@ -54,6 +54,8 @@ spec:
           volumeMounts:
             - name: kubeops-config
               mountPath: /config
+            - name: ca-certs
+              mountPath: /etc/additional-clusters-cafiles
           {{- end }}
           env:
             - name: POD_NAMESPACE
@@ -77,6 +79,8 @@ spec:
         - name: kubeops-config
           configMap:
             name: {{ template "kubeapps.kubeops-config.fullname" . }}
+        - name: ca-certs
+          emptyDir: {}
       {{- end }}
 
 {{- end }}{{/* matches useHelm3 */}}

chart/kubeapps/values.yaml

@@ -696,6 +696,9 @@ featureFlags:
   invalidateCache: true
   operators: false
   # additionalClusters is a WIP feature for multi-cluster support.
+  # The base64-encoded certificateAuthorityData can be obtained from the additional cluster's kube config
+  # file, for example:
+  # kubectl --kubeconfig ~/.kube/kind-config-kubeapps-additional config view --raw -o jsonpath='{.clusters[0].cluster.certificate-authority-data}'
   additionalClusters: []
   # additionalClusters:
   # - name: second-cluster

cmd/kubeops/main.go

@@ -2,13 +2,15 @@ package main
 
 import (
 	"context"
+	"encoding/base64"
 	"encoding/json"
 	"io/ioutil"
 	"net/http"
 	"net/http/httputil"
 	"net/url"
 	"os"
 	"os/signal"
+	"path/filepath"
 	"syscall"
 	"time"
 
@@ -25,6 +27,8 @@ import (
 	"k8s.io/helm/pkg/helm/environment"
 )
 
+const additionalClustersCAFilesPrefix = "/etc/additional-clusters-cafiles"
+
 var (
 	additionalClustersConfigPath string
 	assetsvcURL                  string
@@ -58,10 +62,12 @@ func main() {
 	var additionalClusters map[string]kube.AdditionalClusterConfig
 	if additionalClustersConfigPath != "" {
 		var err error
-		additionalClusters, err = parseAdditionalClusterConfig(additionalClustersConfigPath)
+		var cleanupCAFiles func()
+		additionalClusters, cleanupCAFiles, err = parseAdditionalClusterConfig(additionalClustersConfigPath, additionalClustersCAFilesPrefix)
 		if err != nil {
 			log.Fatalf("unable to parse additional clusters config: %+v", err)
 		}
+		defer cleanupCAFiles()
 	}
 
 	options := handler.Options{
@@ -172,20 +178,42 @@ func main() {
 	os.Exit(0)
 }
 
-func parseAdditionalClusterConfig(path string) (kube.AdditionalClustersConfig, error) {
-	content, err := ioutil.ReadFile(path)
+func parseAdditionalClusterConfig(configPath, caFilesPrefix string) (kube.AdditionalClustersConfig, func(), error) {
+	caFilesDir, err := ioutil.TempDir(caFilesPrefix, "")
+	if err != nil {
+		return nil, func() {}, err
+	}
+	deferFn := func() { os.RemoveAll(caFilesDir) }
+	content, err := ioutil.ReadFile(configPath)
 	if err != nil {
-		return nil, err
+		return nil, deferFn, err
 	}
 
 	var clusterConfigs []kube.AdditionalClusterConfig
 	if err = json.Unmarshal(content, &clusterConfigs); err != nil {
-		return nil, err
+		return nil, deferFn, err
 	}
 
 	configs := kube.AdditionalClustersConfig{}
 	for _, c := range clusterConfigs {
+		// We need to decode the base64-encoded cadata from the input.
+		if c.CertificateAuthorityData != "" {
+			decodedCAData, err := base64.StdEncoding.DecodeString(c.CertificateAuthorityData)
+			if err != nil {
+				return nil, deferFn, err
+			}
+			c.CertificateAuthorityData = string(decodedCAData)
+
+			// We also need a CAFile field because Helm uses the genericclioptions.ConfigFlags
+			// struct which does not support CAData.
+			// https://github.com/kubernetes/cli-runtime/issues/8
+			c.CAFile = filepath.Join(caFilesDir, c.Name)
+			err = ioutil.WriteFile(c.CAFile, decodedCAData, 0644)
+			if err != nil {
+				return nil, deferFn, err
+			}
+		}
 		configs[c.Name] = c
 	}
-	return configs, nil
+	return configs, deferFn, nil
 }

cmd/kubeops/main_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/kubeapps/kubeapps/pkg/kube"
 )
 
@@ -18,31 +19,31 @@ func TestParseAdditionalClusterConfig(t *testing.T) {
 	}{
 		{
 			name:       "parses a single additional cluster",
-			configJSON: `[{"name": "cluster-2", "apiServiceURL": "https://example.com", "certificateAuthorityData": "abcd"}]`,
+			configJSON: `[{"name": "cluster-2", "apiServiceURL": "https://example.com", "certificateAuthorityData": "Y2EtY2VydC1kYXRhCg=="}]`,
 			expectedConfig: kube.AdditionalClustersConfig{
 				"cluster-2": {
 					Name:                     "cluster-2",
 					APIServiceURL:            "https://example.com",
-					CertificateAuthorityData: "abcd",
+					CertificateAuthorityData: "ca-cert-data\n",
 				},
 			},
 		},
 		{
 			name: "parses multiple additional clusters",
 			configJSON: `[
-	{"name": "cluster-2", "apiServiceURL": "https://example.com/cluster-2", "certificateAuthorityData": "abcd"},
-	{"name": "cluster-3", "apiServiceURL": "https://example.com/cluster-3", "certificateAuthorityData": "efgh"}
+	{"name": "cluster-2", "apiServiceURL": "https://example.com/cluster-2", "certificateAuthorityData": "Y2EtY2VydC1kYXRhCg=="},
+	{"name": "cluster-3", "apiServiceURL": "https://example.com/cluster-3", "certificateAuthorityData": "Y2EtY2VydC1kYXRhLWFkZGl0aW9uYWwK"}
 ]`,
 			expectedConfig: kube.AdditionalClustersConfig{
 				"cluster-2": {
 					Name:                     "cluster-2",
 					APIServiceURL:            "https://example.com/cluster-2",
-					CertificateAuthorityData: "abcd",
+					CertificateAuthorityData: "ca-cert-data\n",
 				},
 				"cluster-3": {
 					Name:                     "cluster-3",
 					APIServiceURL:            "https://example.com/cluster-3",
-					CertificateAuthorityData: "efgh",
+					CertificateAuthorityData: "ca-cert-data-additional\n",
 				},
 			},
 		},
@@ -51,20 +52,40 @@ func TestParseAdditionalClusterConfig(t *testing.T) {
 			configJSON:  `[{"name": "cluster-2", "apiServiceURL": "https://example.com", "certificateAuthorityData": "extracomma",}]`,
 			expectedErr: true,
 		},
+		{
+			name:        "errors if any CAData cannot be decoded",
+			configJSON:  `[{"name": "cluster-2", "apiServiceURL": "https://example.com", "certificateAuthorityData": "not-base64-encoded"}]`,
+			expectedErr: true,
+		},
 	}
 
+	ignoreCAFile := cmpopts.IgnoreFields(kube.AdditionalClusterConfig{}, "CAFile")
+
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			path := createConfigFile(t, tc.configJSON)
 			defer os.Remove(path)
 
-			config, err := parseAdditionalClusterConfig(path)
+			config, deferFn, err := parseAdditionalClusterConfig(path, "/tmp")
 			if got, want := err != nil, tc.expectedErr; got != want {
 				t.Errorf("got: %t, want: %t", got, want)
 			}
+			defer deferFn()
+
+			if got, want := config, tc.expectedConfig; !cmp.Equal(want, got, ignoreCAFile) {
+				t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(want, got, ignoreCAFile))
+			}
 
-			if got, want := config, tc.expectedConfig; !cmp.Equal(want, got) {
-				t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(want, got))
+			for clusterName, clusterConfig := range tc.expectedConfig {
+				if clusterConfig.CertificateAuthorityData != "" {
+					fileCAData, err := ioutil.ReadFile(config[clusterName].CAFile)
+					if err != nil {
+						t.Fatalf("%+v", err)
+					}
+					if got, want := string(fileCAData), clusterConfig.CertificateAuthorityData; got != want {
+						t.Errorf("got: %q, want: %q", got, want)
+					}
+				}
 			}
 		})
 	}

docs/user/manifests/kubeapps-local-dev-additional-kind-cluster.yaml

@@ -2,4 +2,6 @@ featureFlags:
   additionalClusters:
    - name: second-cluster
      apiServiceURL: https://172.18.0.3:6443
+     # insecure is set to true only for local dev cluster testing. Always specify the
+     # certificateAuthorityData as documented in teh values.yaml.
      insecure: true

pkg/agent/agent.go

@@ -172,7 +172,7 @@ func NewActionConfig(storageForDriver StorageForDriver, config *rest.Config, cli
 }
 
 // NewConfigFlagsFromCluster returns ConfigFlags with default values set from within cluster.
-func NewConfigFlagsFromCluster(namespace string, clusterConfig *rest.Config) *genericclioptions.ConfigFlags {
+func NewConfigFlagsFromCluster(namespace string, clusterConfig *rest.Config) genericclioptions.RESTClientGetter {
 	impersonateGroup := []string{}
 
 	// CertFile and KeyFile must be nil for the BearerToken to be used for authentication and authorization instead of the pod's service account.

pkg/kube/kube_handler.go

@@ -51,7 +51,14 @@ type AdditionalClusterConfig struct {
 	Name                     string `json:"name"`
 	APIServiceURL            string `json:"apiServiceURL"`
 	CertificateAuthorityData string `json:"certificateAuthorityData,omitempty"`
-	Insecure                 bool   `json:"insecure"`
+	// The genericclioptions.ConfigFlags struct includes only a CAFile field, not
+	// a CAData field.
+	// https://github.com/kubernetes/cli-runtime/issues/8
+	// Embedding genericclioptions.ConfigFlags in a struct which includes the actual rest.Config
+	// and returning that for ToRESTConfig() isn't enough, so we each configured cert out and
+	// include a CAFile field in the config.
+	CAFile   string
+	Insecure bool `json:"insecure"`
 }
 
 // AdditionalClustersConfig is an alias for a map of additional cluster configs.
@@ -77,6 +84,7 @@ func NewClusterConfig(inClusterConfig *rest.Config, token string, cluster string
 	config.TLSClientConfig.Insecure = additionalCluster.Insecure
 	if additionalCluster.CertificateAuthorityData != "" {
 		config.TLSClientConfig.CAData = []byte(additionalCluster.CertificateAuthorityData)
+		config.CAFile = additionalCluster.CAFile
 	}
 	return config, nil
 }

pkg/kube/kube_handler_test.go

@@ -924,6 +924,7 @@ func TestNewClusterConfig(t *testing.T) {
 				"cluster-1": {
 					APIServiceURL:            "https://cluster-1.example.com:7890",
 					CertificateAuthorityData: "ca-file-data",
+					CAFile:                   "/tmp/ca-file-data",
 				},
 			},
 			inClusterConfig: &rest.Config{
@@ -940,6 +941,7 @@ func TestNewClusterConfig(t *testing.T) {
 				BearerTokenFile: "",
 				TLSClientConfig: rest.TLSClientConfig{
 					CAData: []byte("ca-file-data"),
+					CAFile: "/tmp/ca-file-data",
 				},
 			},
 		},

script/deploy-dev.mk

@@ -37,10 +37,6 @@ reset-dev:
 	helm -n kubeapps delete kubeapps || true
 	helm -n dex delete dex || true
 	helm -n ldap delete ldap || true
-	# In case helm installations fail, still delete non-namespaced resources.
-	kubectl delete clusterrole dex kubeapps:controller:apprepository-reader-kubeapps || true
-	kubectl delete clusterrolebinding dex kubeapps:controller:apprepository-reader-kubeapps || true
 	kubectl delete namespace --wait dex ldap kubeapps || true
-	kubectl delete --wait -f ./docs/user/manifests/kubeapps-local-dev-users-rbac.yaml || true
 
 .PHONY: deploy-dex deploy-dev deploy-openldap reset-dev update-apiserver-etc-hosts