Configuration of service account token for additional clusters #5286

castelblanque · 2022-09-02T10:30:58Z

Signed-off-by: Rafa Castelblanque rcastelblanq@vmware.com

Description of the change

This PR adds the ability to set a service token to be used by Kubeapps on additional clusters.
There was the original work done by @XenoAura in #5034.
Due to kubeops being removed, the work has been re-done for the new setup, hence this PR.
Added tests for regression.

Benefits

Service account token applies to operations done against additional clusters.

Possible drawbacks

Security? Service account token will apply to all operations done with the configGetter, not only the ones related to namespaces.

Applicable issues

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>

netlify · 2022-09-02T10:31:09Z

✅ Deploy Preview for kubeapps-dev canceled.

Name	Link
🔨 Latest commit	`1341a35`
🔍 Latest deploy log	https://app.netlify.com/sites/kubeapps-dev/deploys/63174d2680c8db000aab8068

antgamdia

Thanks!

ppbaena · 2022-09-02T10:32:27Z

Thank you @castelblanque!

castelblanque · 2022-09-02T12:46:26Z

After taking a deeper look, it seems that the fix for this is more complex.
The service account for additional clusters should only be used for listing namespaces. See comment here.

With the fix in this PR we are always applying the service token (if any) for calls to additional clusters.
Switching to using this only for listing namespaces requires a change in the signature of the clientGetter function used in the plugin to handle a flag. This will allow to get the client using the service account token or the context token.

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>

castelblanque · 2022-09-07T08:02:14Z

cmd/kubeapps-apis/plugins/resources/v1alpha1/namespaces.go

+	if s.kubeappsCluster != cluster && strings.ToLower(r.GetVerb()) == "list" && strings.ToLower(r.GetResource()) == "namespaces" {
+		// Listing namespaces in additional clusters might involve using the provided service account token
+		typedClient, _, err = s.clusterServiceAccountClientGetter(ctx, cluster)


If actual listing of namespaces ( GetAccessibleNamespaces) makes use of the clusterServiceAccountClientGetter for remote clusters, it makes sense that CanI behaves accordingly.

castelblanque · 2022-09-07T08:15:22Z

script/e2e-test.sh

@@ -299,7 +299,7 @@ if [ "$USE_MULTICLUSTER_OIDC_ENV" = true ]; then
    "--set" "clusters[1].name=second-cluster"
    "--set" "clusters[1].apiServiceURL=https://${ADDITIONAL_CLUSTER_IP}:6443"
    "--set" "clusters[1].insecure=true"
-    "--set" "clusters[1].serviceToken=ZXlKaGJHY2lPaUpTVXpJMU5pSXNJbXRwWkNJNklsbHpiSEp5TlZwM1QwaG9WSE5PYkhVdE5GQkRablY2TW0wd05rUmtMVmxFWVV4MlZEazNaeTEyUmxFaWZRLmV5SnBjM01pT2lKcmRXSmxjbTVsZEdWekwzTmxjblpwWTJWaFkyTnZkVzUwSWl3aWEzVmlaWEp1WlhSbGN5NXBieTl6WlhKMmFXTmxZV05qYjNWdWRDOXVZVzFsYzNCaFkyVWlPaUprWldaaGRXeDBJaXdpYTNWaVpYSnVaWFJsY3k1cGJ5OXpaWEoyYVdObFlXTmpiM1Z1ZEM5elpXTnlaWFF1Ym1GdFpTSTZJbXQxWW1WaGNIQnpMVzVoYldWemNHRmpaUzFrYVhOamIzWmxjbmt0ZEc5clpXNHRjV295Ym1naUxDSnJkV0psY201bGRHVnpMbWx2TDNObGNuWnBZMlZoWTJOdmRXNTBMM05sY25acFkyVXRZV05qYjNWdWRDNXVZVzFsSWpvaWEzVmlaV0Z3Y0hNdGJtRnRaWE53WVdObExXUnBjMk52ZG1WeWVTSXNJbXQxWW1WeWJtVjBaWE11YVc4dmMyVnlkbWxqWldGalkyOTFiblF2YzJWeWRtbGpaUzFoWTJOdmRXNTBMblZwWkNJNkltVXhaakE1WmpSakxUTTRNemt0TkRJME15MWhZbUptTFRKaU5HWm1OREZrWW1RMllTSXNJbk4xWWlJNkluTjVjM1JsYlRwelpYSjJhV05sWVdOamIzVnVkRHBrWldaaGRXeDBPbXQxWW1WaGNIQnpMVzVoYldWemNHRmpaUzFrYVhOamIzWmxjbmtpZlEuTnh6V2dsUGlrVWpROVQ1NkpWM2xJN1VWTUVSR3J2bklPSHJENkh4dUVwR0luLWFUUzV5Q0pDa3Z0cTF6S3Z3b05sc2MyX0YxaTdFOUxWRGFwbC1UQlhleUN5Rl92S1B1TDF4dTdqZFBMZ1dKT1pQX3JMcXppaDV4ZlkxalFoOHNhdTRZclFJLUtqb3U1UkRRZ0tOQS1BaS1lRlFOZVh2bmlUNlBKYWVkc184V0t3dHRMMC1wdHpYRnBnOFl5dkx6N0U1UWdTR2tjNWpDVXlsS0RvZVRUaVRSOEc2RHFHYkFQQUYwREt0b3MybU9Geno4SlJYNHhoQmdvaUcxVTVmR1g4Z3hnTU1SV0VHRE9kaGMyeXRvcFdRUkRpYmhvaldNS3VDZlNua09zMDRGYTBkYmEwQ0NTbld2a29LZ3Z4QVR5aVVrWm9wV3VpZ1JJNFd5dDkzbXhR"
+    "--set" "clusters[1].serviceToken=$(kubectl --context=kind-kubeapps-ci-additional --kubeconfig=${HOME}/.kube/kind-config-kubeapps-ci-additional get secret kubeapps-namespace-discovery -o go-template='{{.data.token | base64decode}}')"


Previously specified service token contained fixed UID for user. The decoded token looks like:

{ "iss": "kubernetes/serviceaccount", "kubernetes.io/serviceaccount/namespace": "default", "kubernetes.io/serviceaccount/secret.name": "kubeapps-namespace-discovery-token-qj2nh", "kubernetes.io/serviceaccount/service-account.name": "kubeapps-namespace-discovery", "kubernetes.io/serviceaccount/service-account.uid": "e1f09f4c-3839-4243-abbf-2b4ff41dbd6a", "sub": "system:serviceaccount:default:kubeapps-namespace-discovery" }

Whenever the SA for kubeapps-namespace-discovery is created, the UID does not match and there is an "invalid claim" auth error.
Switched to dynamically retrieving the token value from the additional cluster.

Ah, good catch!

castelblanque · 2022-09-07T08:16:19Z

script/makefiles/deploy-dev.mk

@@ -38,7 +38,8 @@ deploy-dev-kubeapps:
 	helm --kubeconfig=${CLUSTER_CONFIG} upgrade --install kubeapps ./chart/kubeapps --namespace kubeapps --create-namespace \
 		--values ./site/content/docs/latest/reference/manifests/kubeapps-local-dev-values.yaml \
 		--values ./site/content/docs/latest/reference/manifests/kubeapps-local-dev-auth-proxy-values.yaml \
-		--values ./site/content/docs/latest/reference/manifests/kubeapps-local-dev-additional-kind-cluster.yaml
+		--values ./site/content/docs/latest/reference/manifests/kubeapps-local-dev-additional-kind-cluster.yaml \
+		--set clusters[1].serviceToken=$(shell kubectl --kubeconfig=${ADDITIONAL_CLUSTER_CONFIG} get secret kubeapps-namespace-discovery -o go-template="{{.data.token | base64decode}}")


Service token for additional clusters is only used for listing namespaces, hence the use of kubeapps-namespace-discovery.

antgamdia

Great! Thanks

antgamdia · 2022-09-07T08:28:24Z

script/e2e-test.sh

@@ -299,7 +299,7 @@ if [ "$USE_MULTICLUSTER_OIDC_ENV" = true ]; then
    "--set" "clusters[1].name=second-cluster"
    "--set" "clusters[1].apiServiceURL=https://${ADDITIONAL_CLUSTER_IP}:6443"
    "--set" "clusters[1].insecure=true"
-    "--set" "clusters[1].serviceToken=ZXlKaGJHY2lPaUpTVXpJMU5pSXNJbXRwWkNJNklsbHpiSEp5TlZwM1QwaG9WSE5PYkhVdE5GQkRablY2TW0wd05rUmtMVmxFWVV4MlZEazNaeTEyUmxFaWZRLmV5SnBjM01pT2lKcmRXSmxjbTVsZEdWekwzTmxjblpwWTJWaFkyTnZkVzUwSWl3aWEzVmlaWEp1WlhSbGN5NXBieTl6WlhKMmFXTmxZV05qYjNWdWRDOXVZVzFsYzNCaFkyVWlPaUprWldaaGRXeDBJaXdpYTNWaVpYSnVaWFJsY3k1cGJ5OXpaWEoyYVdObFlXTmpiM1Z1ZEM5elpXTnlaWFF1Ym1GdFpTSTZJbXQxWW1WaGNIQnpMVzVoYldWemNHRmpaUzFrYVhOamIzWmxjbmt0ZEc5clpXNHRjV295Ym1naUxDSnJkV0psY201bGRHVnpMbWx2TDNObGNuWnBZMlZoWTJOdmRXNTBMM05sY25acFkyVXRZV05qYjNWdWRDNXVZVzFsSWpvaWEzVmlaV0Z3Y0hNdGJtRnRaWE53WVdObExXUnBjMk52ZG1WeWVTSXNJbXQxWW1WeWJtVjBaWE11YVc4dmMyVnlkbWxqWldGalkyOTFiblF2YzJWeWRtbGpaUzFoWTJOdmRXNTBMblZwWkNJNkltVXhaakE1WmpSakxUTTRNemt0TkRJME15MWhZbUptTFRKaU5HWm1OREZrWW1RMllTSXNJbk4xWWlJNkluTjVjM1JsYlRwelpYSjJhV05sWVdOamIzVnVkRHBrWldaaGRXeDBPbXQxWW1WaGNIQnpMVzVoYldWemNHRmpaUzFrYVhOamIzWmxjbmtpZlEuTnh6V2dsUGlrVWpROVQ1NkpWM2xJN1VWTUVSR3J2bklPSHJENkh4dUVwR0luLWFUUzV5Q0pDa3Z0cTF6S3Z3b05sc2MyX0YxaTdFOUxWRGFwbC1UQlhleUN5Rl92S1B1TDF4dTdqZFBMZ1dKT1pQX3JMcXppaDV4ZlkxalFoOHNhdTRZclFJLUtqb3U1UkRRZ0tOQS1BaS1lRlFOZVh2bmlUNlBKYWVkc184V0t3dHRMMC1wdHpYRnBnOFl5dkx6N0U1UWdTR2tjNWpDVXlsS0RvZVRUaVRSOEc2RHFHYkFQQUYwREt0b3MybU9Geno4SlJYNHhoQmdvaUcxVTVmR1g4Z3hnTU1SV0VHRE9kaGMyeXRvcFdRUkRpYmhvaldNS3VDZlNua09zMDRGYTBkYmEwQ0NTbld2a29LZ3Z4QVR5aVVrWm9wV3VpZ1JJNFd5dDkzbXhR"
+    "--set" "clusters[1].serviceToken=$(kubectl --context=kind-kubeapps-ci-additional --kubeconfig=${HOME}/.kube/kind-config-kubeapps-ci-additional get secret kubeapps-namespace-discovery -o go-template='{{.data.token | base64decode}}')"


Ah, good catch!

Added logic from #5034

fbc0719

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>

vmwclabot added the cla-not-required label Sep 2, 2022

antgamdia approved these changes Sep 2, 2022

View reviewed changes

This was referenced Sep 2, 2022

Use correct config for additional clusters #5034

Closed

Kubeops cannot return a list of namespaces in additional clusters #5033

Closed

castelblanque self-assigned this Sep 2, 2022

Rafa Castelblanque and others added 6 commits September 5, 2022 13:23

Move the SA token to its own client getter

bac1b1b

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>

Merge branch 'main' into 5033-additional-clusters-sa

a94c3af

Added missing error check

6db967f

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>

Fix for return value

b92af5a

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>

Setting additional SA token dynamically

9ffd5c3

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>

Fix wrong command

1341a35

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>

castelblanque commented Sep 7, 2022

View reviewed changes

castelblanque requested a review from antgamdia September 7, 2022 08:16

antgamdia approved these changes Sep 7, 2022

View reviewed changes

castelblanque merged commit 55bfe30 into main Sep 7, 2022

castelblanque deleted the 5033-additional-clusters-sa branch September 7, 2022 08:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Configuration of service account token for additional clusters #5286

Configuration of service account token for additional clusters #5286

castelblanque commented Sep 2, 2022 •

edited

Loading

netlify bot commented Sep 2, 2022 •

edited

Loading

antgamdia left a comment

ppbaena commented Sep 2, 2022

castelblanque commented Sep 2, 2022 •

edited

Loading

castelblanque Sep 7, 2022

castelblanque Sep 7, 2022

antgamdia Sep 7, 2022

castelblanque Sep 7, 2022

antgamdia left a comment

antgamdia Sep 7, 2022

Configuration of service account token for additional clusters #5286

Configuration of service account token for additional clusters #5286

Conversation

castelblanque commented Sep 2, 2022 • edited Loading

Description of the change

Benefits

Possible drawbacks

Applicable issues

netlify bot commented Sep 2, 2022 • edited Loading

✅ Deploy Preview for kubeapps-dev canceled.

antgamdia left a comment

Choose a reason for hiding this comment

ppbaena commented Sep 2, 2022

castelblanque commented Sep 2, 2022 • edited Loading

castelblanque Sep 7, 2022

Choose a reason for hiding this comment

castelblanque Sep 7, 2022

Choose a reason for hiding this comment

antgamdia Sep 7, 2022

Choose a reason for hiding this comment

castelblanque Sep 7, 2022

Choose a reason for hiding this comment

antgamdia left a comment

Choose a reason for hiding this comment

antgamdia Sep 7, 2022

Choose a reason for hiding this comment

castelblanque commented Sep 2, 2022 •

edited

Loading

netlify bot commented Sep 2, 2022 •

edited

Loading

castelblanque commented Sep 2, 2022 •

edited

Loading