Enable setting a service token for namespace discovery in other clust…

…ers. (#1921)

chart/kubeapps/values.yaml

@@ -708,5 +708,6 @@ featureFlags:
   # - name: second-cluster
   #   apiServiceURL: https://second-cluster:6443
   #   certificateAuthorityData: LS0tLS1CRUdJ...
+  #   serviceToken: ...
   # ui is a WIP feature for the migration to Clarity design system.
   ui: hex

cmd/kubeops/main_test.go

@@ -19,12 +19,13 @@ func TestParseAdditionalClusterConfig(t *testing.T) {
 	}{
 		{
 			name:       "parses a single additional cluster",
-			configJSON: `[{"name": "cluster-2", "apiServiceURL": "https://example.com", "certificateAuthorityData": "Y2EtY2VydC1kYXRhCg=="}]`,
+			configJSON: `[{"name": "cluster-2", "apiServiceURL": "https://example.com", "certificateAuthorityData": "Y2EtY2VydC1kYXRhCg==", "serviceToken": "abcd"}]`,
 			expectedConfig: kube.AdditionalClustersConfig{
 				"cluster-2": {
 					Name:                     "cluster-2",
 					APIServiceURL:            "https://example.com",
 					CertificateAuthorityData: "ca-cert-data\n",
+					ServiceToken:             "abcd",
 				},
 			},
 		},

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

@@ -5,3 +5,7 @@ featureFlags:
      # insecure is set to true only for local dev cluster testing. Always specify the
      # certificateAuthorityData as documented in teh values.yaml.
      insecure: true
+     # serviceToken is only required if users viewing the additional cluster should be able
+     # to see the namespaces to which they have access in the namespace selector (and should
+     # grant only the required RBAC)
+     serviceToken: ZXlKaGJHY2lPaUpTVXpJMU5pSXNJbXRwWkNJNklsbHpiSEp5TlZwM1QwaG9WSE5PYkhVdE5GQkRablY2TW0wd05rUmtMVmxFWVV4MlZEazNaeTEyUmxFaWZRLmV5SnBjM01pT2lKcmRXSmxjbTVsZEdWekwzTmxjblpwWTJWaFkyTnZkVzUwSWl3aWEzVmlaWEp1WlhSbGN5NXBieTl6WlhKMmFXTmxZV05qYjNWdWRDOXVZVzFsYzNCaFkyVWlPaUprWldaaGRXeDBJaXdpYTNWaVpYSnVaWFJsY3k1cGJ5OXpaWEoyYVdObFlXTmpiM1Z1ZEM5elpXTnlaWFF1Ym1GdFpTSTZJbXQxWW1WaGNIQnpMVzVoYldWemNHRmpaUzFrYVhOamIzWmxjbmt0ZEc5clpXNHRjV295Ym1naUxDSnJkV0psY201bGRHVnpMbWx2TDNObGNuWnBZMlZoWTJOdmRXNTBMM05sY25acFkyVXRZV05qYjNWdWRDNXVZVzFsSWpvaWEzVmlaV0Z3Y0hNdGJtRnRaWE53WVdObExXUnBjMk52ZG1WeWVTSXNJbXQxWW1WeWJtVjBaWE11YVc4dmMyVnlkbWxqWldGalkyOTFiblF2YzJWeWRtbGpaUzFoWTJOdmRXNTBMblZwWkNJNkltVXhaakE1WmpSakxUTTRNemt0TkRJME15MWhZbUptTFRKaU5HWm1OREZrWW1RMllTSXNJbk4xWWlJNkluTjVjM1JsYlRwelpYSjJhV05sWVdOamIzVnVkRHBrWldaaGRXeDBPbXQxWW1WaGNIQnpMVzVoYldWemNHRmpaUzFrYVhOamIzWmxjbmtpZlEuTnh6V2dsUGlrVWpROVQ1NkpWM2xJN1VWTUVSR3J2bklPSHJENkh4dUVwR0luLWFUUzV5Q0pDa3Z0cTF6S3Z3b05sc2MyX0YxaTdFOUxWRGFwbC1UQlhleUN5Rl92S1B1TDF4dTdqZFBMZ1dKT1pQX3JMcXppaDV4ZlkxalFoOHNhdTRZclFJLUtqb3U1UkRRZ0tOQS1BaS1lRlFOZVh2bmlUNlBKYWVkc184V0t3dHRMMC1wdHpYRnBnOFl5dkx6N0U1UWdTR2tjNWpDVXlsS0RvZVRUaVRSOEc2RHFHYkFQQUYwREt0b3MybU9Geno4SlJYNHhoQmdvaUcxVTVmR1g4Z3hnTU1SV0VHRE9kaGMyeXRvcFdRUkRpYmhvaldNS3VDZlNua09zMDRGYTBkYmEwQ0NTbld2a29LZ3Z4QVR5aVVrWm9wV3VpZ1JJNFd5dDkzbXhR 

docs/user/manifests/kubeapps-local-dev-namespace-discovery-rbac.yaml

@@ -0,0 +1,43 @@
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: kubeapps-namespace-discovery
+rules:
+  - apiGroups:
+      - ""
+    resources:
+      - namespaces
+    verbs:
+      - list
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  name: kubeapps-namespace-discovery
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: "kubeapps:kubeops-ns-discovery"
+subjects:
+  - kind: ServiceAccount
+    name: kubeapps-namespace-discovery
+    namespace: default
+---
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: kubeapps-namespace-discovery
+  namespace: default
+secrets:
+- name: kubeapps-namespace-discovery
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: kubeapps-namespace-discovery
+  namespace: default
+  annotations:
+    "kubernetes.io/service-account.name": kubeapps-namespace-discovery
+type: "kubernetes.io/service-account-token"
+data:
+  token: ZXlKaGJHY2lPaUpTVXpJMU5pSXNJbXRwWkNJNklsbHpiSEp5TlZwM1QwaG9WSE5PYkhVdE5GQkRablY2TW0wd05rUmtMVmxFWVV4MlZEazNaeTEyUmxFaWZRLmV5SnBjM01pT2lKcmRXSmxjbTVsZEdWekwzTmxjblpwWTJWaFkyTnZkVzUwSWl3aWEzVmlaWEp1WlhSbGN5NXBieTl6WlhKMmFXTmxZV05qYjNWdWRDOXVZVzFsYzNCaFkyVWlPaUprWldaaGRXeDBJaXdpYTNWaVpYSnVaWFJsY3k1cGJ5OXpaWEoyYVdObFlXTmpiM1Z1ZEM5elpXTnlaWFF1Ym1GdFpTSTZJbXQxWW1WaGNIQnpMVzVoYldWemNHRmpaUzFrYVhOamIzWmxjbmt0ZEc5clpXNHRjV295Ym1naUxDSnJkV0psY201bGRHVnpMbWx2TDNObGNuWnBZMlZoWTJOdmRXNTBMM05sY25acFkyVXRZV05qYjNWdWRDNXVZVzFsSWpvaWEzVmlaV0Z3Y0hNdGJtRnRaWE53WVdObExXUnBjMk52ZG1WeWVTSXNJbXQxWW1WeWJtVjBaWE11YVc4dmMyVnlkbWxqWldGalkyOTFiblF2YzJWeWRtbGpaUzFoWTJOdmRXNTBMblZwWkNJNkltVXhaakE1WmpSakxUTTRNemt0TkRJME15MWhZbUptTFRKaU5HWm1OREZrWW1RMllTSXNJbk4xWWlJNkluTjVjM1JsYlRwelpYSjJhV05sWVdOamIzVnVkRHBrWldaaGRXeDBPbXQxWW1WaGNIQnpMVzVoYldWemNHRmpaUzFrYVhOamIzWmxjbmtpZlEuTnh6V2dsUGlrVWpROVQ1NkpWM2xJN1VWTUVSR3J2bklPSHJENkh4dUVwR0luLWFUUzV5Q0pDa3Z0cTF6S3Z3b05sc2MyX0YxaTdFOUxWRGFwbC1UQlhleUN5Rl92S1B1TDF4dTdqZFBMZ1dKT1pQX3JMcXppaDV4ZlkxalFoOHNhdTRZclFJLUtqb3U1UkRRZ0tOQS1BaS1lRlFOZVh2bmlUNlBKYWVkc184V0t3dHRMMC1wdHpYRnBnOFl5dkx6N0U1UWdTR2tjNWpDVXlsS0RvZVRUaVRSOEc2RHFHYkFQQUYwREt0b3MybU9Geno4SlJYNHhoQmdvaUcxVTVmR1g4Z3hnTU1SV0VHRE9kaGMyeXRvcFdRUkRpYmhvaldNS3VDZlNua09zMDRGYTBkYmEwQ0NTbld2a29LZ3Z4QVR5aVVrWm9wV3VpZ1JJNFd5dDkzbXhR 

pkg/kube/kube_handler.go

@@ -57,7 +57,13 @@ type AdditionalClusterConfig struct {
 	// 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
+	CAFile string
+	// ServiceToken can be configured so that the Kubeapps application itself
+	// has access to get all namespaces on additional clusters, for example. It
+	// should *not* be for reading secrets or similar, but limited to the
+	// required functionality.
+	ServiceToken string
+
 	Insecure bool `json:"insecure"`
 }
 
@@ -186,9 +192,33 @@ func (a *kubeHandler) AsUser(token, cluster string) (handler, error) {
 		log.Errorf("unable to create clientset: %v", err)
 		return nil, err
 	}
+
+	// Just use the service clientset if we're on the default cluster, but otherwise
+	// create a new clientset using a configured service token for a specific cluster.
+	// This is used when requesting the namespaces for a cluster (to populate the selector)
+	// iff the users own credential does not suffice. If a service token is not configured
+	// for the cluster, the namespace selector remains unpopulated.
+	var svcClientset combinedClientsetInterface
+	if cluster == DefaultClusterName {
+		svcClientset = a.svcClientset
+	} else {
+		additionalCluster, ok := a.additionalClustersConfig[cluster]
+		if !ok {
+			return nil, fmt.Errorf("cluster %q has no configuration", cluster)
+		}
+		svcConfig := *config
+		svcConfig.BearerToken = additionalCluster.ServiceToken
+
+		svcClientset, err = a.clientsetForConfig(config)
+		if err != nil {
+			log.Errorf("unable to create clientset: %v", err)
+			return nil, err
+		}
+	}
+
 	return &userHandler{
 		kubeappsNamespace: a.kubeappsNamespace,
-		svcClientset:      a.svcClientset,
+		svcClientset:      svcClientset,
 		clientset:         clientset,
 	}, nil
 }
@@ -552,7 +582,7 @@ func KubeappsSecretNameForRepo(repoName, namespace string) string {
 	return fmt.Sprintf("%s-%s", namespace, secretNameForRepo(repoName))
 }
 
-func filterAllowedNamespaces(userClientset combinedClientsetInterface, namespaces *corev1.NamespaceList) ([]corev1.Namespace, error) {
+func filterAllowedNamespaces(userClientset combinedClientsetInterface, namespaces *corev1.NamespaceList) (*corev1.NamespaceList, error) {
 	allowedNamespaces := []corev1.Namespace{}
 	for _, namespace := range namespaces.Items {
 		res, err := userClientset.AuthorizationV1().SelfSubjectAccessReviews().Create(context.TODO(), &authorizationapi.SelfSubjectAccessReview{
@@ -572,7 +602,8 @@ func filterAllowedNamespaces(userClientset combinedClientsetInterface, namespace
 			allowedNamespaces = append(allowedNamespaces, namespace)
 		}
 	}
-	return allowedNamespaces, nil
+	namespaces.Items = allowedNamespaces
+	return namespaces, nil
 }
 
 // GetNamespaces return the list of namespaces that the user has permission to access
@@ -584,18 +615,23 @@ func (a *userHandler) GetNamespaces() ([]corev1.Namespace, error) {
 		if k8sErrors.IsForbidden(err) {
 			// The user doesn't have permissions to list namespaces, use the current serviceaccount
 			namespaces, err = a.svcClientset.CoreV1().Namespaces().List(context.TODO(), metav1.ListOptions{})
-		}
-		if err != nil {
+			if err != nil && k8sErrors.IsForbidden(err) {
+				// If the configured svcclient doesn't have permission, just return an empty list.
+				return []corev1.Namespace{}, nil
+			}
+
+			// Only if we obtained the namespaces from the svc client do we filter it using
+			// the user clientset.
+			namespaces, err = filterAllowedNamespaces(a.clientset, namespaces)
+			if err != nil {
+				return nil, err
+			}
+		} else {
 			return nil, err
 		}
 	}
 
-	allowedNamespaces, err := filterAllowedNamespaces(a.clientset, namespaces)
-	if err != nil {
-		return nil, err
-	}
-
-	return allowedNamespaces, nil
+	return namespaces.Items, nil
 }
 
 // GetSecret return the a secret from a namespace using a token if given

pkg/kube/kube_handler_test.go

@@ -33,6 +33,7 @@ import (
 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/discovery"
 	fakecoreclientset "k8s.io/client-go/kubernetes/fake"
 	"k8s.io/client-go/rest"
@@ -730,47 +731,72 @@ func TestSecretForRequest(t *testing.T) {
 
 func TestGetNamespaces(t *testing.T) {
 	testCases := []struct {
-		name             string
-		existingNS       []string
-		expectedResponse []corev1.Namespace
-		allowed          bool
+		name               string
+		existingNamespaces []string
+		allowed            bool
+		userClientErr      error
+		svcClientErr       error
+		expectedNamespaces []string
 	}{
 		{
-			name:       "it list namespaces",
-			existingNS: []string{"foo"},
-			expectedResponse: []corev1.Namespace{
-				{
-					ObjectMeta: metav1.ObjectMeta{
-						Name: "foo",
-					},
-				},
-			},
-			allowed: true,
+			name:               "it lists namespaces if the user client returns the namespaces",
+			existingNamespaces: []string{"foo", "bar", "zed"},
+			expectedNamespaces: []string{"foo", "bar", "zed"},
+			allowed:            true,
+		},
+		{
+			name:               "it does not filter the namespaces if returned by the user client",
+			existingNamespaces: []string{"foo", "bar", "zed"},
+			expectedNamespaces: []string{"foo", "bar", "zed"},
+			allowed:            false,
 		},
 		{
-			name:             "it returns an empty list if not allowed",
-			existingNS:       []string{"foo"},
-			expectedResponse: []corev1.Namespace{},
-			allowed:          false,
+			name:               "it lists namespaces if the userclient fails but the service client succeeds",
+			existingNamespaces: []string{"foo"},
+			userClientErr:      k8sErrors.NewForbidden(schema.GroupResource{}, "bang", fmt.Errorf("Bang")),
+			expectedNamespaces: []string{"foo"},
+			allowed:            true,
+		},
+		{
+			name:               "it filters the namespaces if the userclient fails but the service client succeeds",
+			existingNamespaces: []string{"foo"},
+			userClientErr:      k8sErrors.NewForbidden(schema.GroupResource{}, "bang", fmt.Errorf("Bang")),
+			expectedNamespaces: []string{},
+			allowed:            false,
+		},
+		{
+			name:               "it returns an empty list if both the user and service account forbidden",
+			existingNamespaces: []string{"foo"},
+			userClientErr:      k8sErrors.NewForbidden(schema.GroupResource{}, "bang", fmt.Errorf("Bang")),
+			svcClientErr:       k8sErrors.NewForbidden(schema.GroupResource{}, "bang", fmt.Errorf("Bang")),
+			expectedNamespaces: []string{},
+			allowed:            true,
 		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			cs := fakeCombinedClientset{
+			userClientSet := fakeCombinedClientset{
 				fakeapprepoclientset.NewSimpleClientset(),
 				fakecoreclientset.NewSimpleClientset(),
 				&fakeRest.RESTClient{},
 			}
 
-			for _, ns := range tc.existingNS {
-				cs.Clientset.CoreV1().Namespaces().Create(context.TODO(), &corev1.Namespace{
-					ObjectMeta: metav1.ObjectMeta{
-						Name: ns,
-					},
-				}, metav1.CreateOptions{})
+			svcClientSet := fakeCombinedClientset{
+				fakeapprepoclientset.NewSimpleClientset(),
+				fakecoreclientset.NewSimpleClientset(),
+				&fakeRest.RESTClient{},
 			}
 
-			cs.Clientset.Fake.PrependReactor(
+			setClientsetData(userClientSet, tc.existingNamespaces, tc.userClientErr)
+			setClientsetData(svcClientSet, tc.existingNamespaces, tc.svcClientErr)
+
+			// Set whether the userClientSet is allowed to create self subject access reviews.
+			// The handler for the reactor has only the action as input, which does not convey
+			// enough info to be able to decide whether an individual namespace is allowed
+			// (as action.GetNamespace() is always empty as self subject access reviews are
+			// *not* themselves namespaced - the spec includes the namespace but is not contained
+			// in the action). As a result, we can only filter everything or nothing in tests.
+			userClientSet.Clientset.Fake.PrependReactor(
 				"create",
 				"selfsubjectaccessreviews",
 				func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) {
@@ -785,23 +811,48 @@ func TestGetNamespaces(t *testing.T) {
 			)
 
 			handler := kubeHandler{
-				clientsetForConfig: func(*rest.Config) (combinedClientsetInterface, error) { return cs, nil },
+				clientsetForConfig: func(*rest.Config) (combinedClientsetInterface, error) { return userClientSet, nil },
 				kubeappsNamespace:  "kubeapps",
-				svcClientset:       cs,
+				svcClientset:       svcClientSet,
 			}
 
-			namespaces, err := handler.AsSVC().GetNamespaces()
+			userHandler, err := handler.AsUser("token", "default")
+			if err != nil {
+				t.Errorf("Unexpected error %v", err)
+			}
+			namespaces, err := userHandler.GetNamespaces()
 			if err != nil {
 				t.Errorf("Unexpected error %v", err)
 			}
 
-			if !cmp.Equal(namespaces, tc.expectedResponse) {
-				t.Errorf("Unexpected response: %s", cmp.Diff(namespaces, tc.expectedResponse))
+			namespaceNames := []string{}
+			for _, ns := range namespaces {
+				namespaceNames = append(namespaceNames, ns.ObjectMeta.Name)
+			}
+			if !cmp.Equal(namespaceNames, tc.expectedNamespaces) {
+				t.Errorf("Unexpected response: %s", cmp.Diff(namespaces, tc.expectedNamespaces))
 			}
 		})
 	}
 }
 
+// setClientsetData configures the fake clientset with the return and error.
+func setClientsetData(cs fakeCombinedClientset, namespaceNames []string, err error) {
+	namespaces := []corev1.Namespace{}
+	for _, ns := range namespaceNames {
+		namespaces = append(namespaces, corev1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{Name: ns},
+		})
+	}
+	cs.Clientset.Fake.PrependReactor(
+		"list",
+		"namespaces",
+		func(action k8stesting.Action) (bool, runtime.Object, error) {
+			return true, &corev1.NamespaceList{Items: namespaces}, err
+		},
+	)
+}
+
 func TestValidateAppRepository(t *testing.T) {
 	const kubeappsNamespace = "kubeapps"
 	getValidationCliAndReqTests := []struct {

script/cluster-kind.mk

@@ -37,6 +37,7 @@ ${ADDITIONAL_CLUSTER_CONFIG}: devel/dex.crt
 		--retain \
 		--wait 10s
 	kubectl apply --kubeconfig=${ADDITIONAL_CLUSTER_CONFIG} -f ./docs/user/manifests/kubeapps-local-dev-users-rbac.yaml
+	kubectl apply --kubeconfig=${ADDITIONAL_CLUSTER_CONFIG} -f ./docs/user/manifests/kubeapps-local-dev-namespace-discovery-rbac.yaml
 
 additional-cluster-kind: ${ADDITIONAL_CLUSTER_CONFIG}