Workload identity support namespaced service account #39

RichardChen820 · 2024-04-30T06:42:49Z

Workload identity support namespaced service account

linglingye001 · 2024-05-13T05:17:00Z

deploy/templates/deployment.yaml

+        - name: WORKLOAD_IDENTITY_ENABLED
+          value: {{ .Values.workloadIdentity.enabled }}
+        - name: WORKLOAD_IDENTITY_CUSTOM_SERVICE_ACCOUNT_ONLY
+          value: {{ .Values.workloadIdentity.allowCustomServiceAccount }}


It's customServiceAccountOnly in helm-value.yaml file, not allowCustomServiceAccount.

linglingye001 · 2024-05-14T06:35:12Z

internal/loader/configuration_client_manager.go

+	tenantId := serviceAccountObj.Annotations[AnnotationTenantID]
+	getAssertionFunc := newGetAssertionFunc(serviceAccountNamespace, serviceAccountName)
+
+	clientAssertionCredential, err := azidentity.NewClientAssertionCredential(tenantId, clientId, getAssertionFunc, nil)


I am not sure the usage of clientAssertionCredential,for authenticating both service principal and workload identity?

linglingye001 · 2024-05-14T06:44:55Z

internal/loader/configuration_client_manager.go

+			return "", err
+		}
+
+		token, err := kubeClient.CoreV1().ServiceAccounts(serviceAccountNamespace).CreateToken(ctx, serviceAccountName, &authv1.TokenRequest{


Will the service account token expire?

It may expire, but since we reuse the appconfig.client, once the tokenCredential expires, but will try to acquire new AAD token, then it will call the callback to get a new service account token as well, so we do not need to worry about service account expires or not.

linglingye001 · 2024-05-14T06:56:21Z

internal/loader/configuration_client_manager.go

+		return nil, err
+	}
+
+	clientId := serviceAccountObj.ObjectMeta.Annotations[AnnotationClientID]


Can we add the check for AnnotationClientID and AnnotationTenantID? They may not exist.

linglingye001 · 2024-05-17T05:22:04Z

internal/loader/configuration_client_manager.go

+	}
+
+	if _, ok := serviceAccountObj.Annotations[AnnotationTenantID]; ok {
+		tenantId = serviceAccountObj.Annotations[AnnotationTenantID]


What's the difference bewteen the AzureTenantId from pod config and AnnotationTenantID from service account?

You can imagine that the AzureTenantId from pod is global, the AnnotationTenantID is for each specific service account. If users uses the same tenantID for all service account, it just need to use the global one, no need to specify in every service account

I see, thanks

to be honest, I cannot think of a case when a different tenant id is used. Technically it has to be the same tenantId.

I do a bit change to this, hope this make sense to you

linglingye001 · 2024-05-17T05:23:38Z

internal/loader/configuration_client_manager.go

@@ -492,11 +493,22 @@ func newClientAssertionCredential(ctx context.Context, serviceAccountName string
 		return nil, err
 	}

-	clientId := serviceAccountObj.ObjectMeta.Annotations[AnnotationClientID]
-	tenantId := serviceAccountObj.Annotations[AnnotationTenantID]
+	tenantId, ok := os.LookupEnv(strings.ToUpper(AzureTenantId))


Why the tenantId is required?

Good question, the tenantId is there if enable the cluster workload identity, but need confirm it.

Confirmed, once the workload identity is enabled, the tenantId should be there, and the existing way we support for workload identity also relies on this tenantId

If namespaced service account is used, is the value of tenantId same as AnnotationTenantID from service account? If customers need to explicitly set this value, will users from other namespace can get the tenantId through kubectl describe po k8sproviderpodxxxx？

I don't quite get your question. tenantId if for global, each service account could overwrite is by setting AnnotationTenantID

internal/controller/utils.go

juniwang · 2024-05-20T13:11:31Z

internal/controller/utils.go

+
+	if workloadIdentity.ManagedIdentityClientId != nil {
+		if strings.EqualFold(os.Getenv(WorkloadIdentityDisableGlobalServiceAccount), "true") {
+			return loader.NewArgumentError("auth.workloadIdentity.managedIdentityClientId", fmt.Errorf("managedIdentityClientId is not allowed since only custom service account is allowed"))


should we avoid "custom service account"? some like "since global service account is disabled" to keep consistent with the installation flag.

juniwang · 2024-05-20T13:26:56Z

internal/loader/configuration_client_manager.go

+}
+
+func newGetAssertionFunc(serviceAccountNamespace string, serviceAccountName string) func(ctx context.Context) (string, error) {
+	audiences := []string{AzureDefaultAudience}


the name is a little misleading. it's not default audience of Azure. I believe https://management.azure.com is the actual default for control plane. ApiTokenExchangeAudience?

Make sense, updated.

deploy/templates/clusterrole.yaml

deploy/templates/serviceaccount.yaml

* Not running on windows node (#30) * Bump up version to 2.0.0-preview (#42) * Workload identity support namespaced service account (#39) * Workload identity support namespaced service account * feedback * Feedback * Upgrade package (#44) * fix vulnerability * fix go version * update * update * update * update * update * update controller runtime * upgrade azsecrets package * update ci * update patched version * update * revert version * upgrade packages * update ci * revert * deduplicate feature flags (#50) * Treat get settings from one client failure as warning (#48) * Add node affinity and toleration configuration (#46) * K8s provider conformance test plugin (#43) * k8s provider conformance test plugin * rename * remove docker hub dependency * replace curl with wget * add version file * Setup golangci lint action (#51) * setup golangci lint action * fix linting error * update ci * update * add lint in makefile * Add Correlation Context header for extension (#47) * Add Correlation Context header for extension * Add more context * Add Host and RequestType in correlation context * Remove the kv refresh * update extension test plugin conformance file (#52) * added timeout parameter in golintCI (#54) * Revise the error message for selector object verification (#56) * Bump up version to 2.0.0 (#58) * Add data collection section in readme (#57) * Require to opt-in for the global service account (#60) * Require to opt in the global service account * Rename * Fix vulnerability (#68) * fix vulnerability * specify go version in golang lint ci --------- Co-authored-by: Richard chen <[email protected]>

Workload identity support namespaced service account

108c727

RichardChen820 force-pushed the user/junbchen/workloadIdentity branch from 0a4d31f to 108c727 Compare April 30, 2024 07:36

RichardChen820 marked this pull request as ready for review May 13, 2024 03:03

linglingye001 reviewed May 14, 2024

View reviewed changes

feedback

029de80

linglingye001 reviewed May 17, 2024

View reviewed changes

RichardChen820 requested a review from juniwang May 17, 2024 08:12

linglingye001 approved these changes May 20, 2024

View reviewed changes

juniwang reviewed May 20, 2024

View reviewed changes

internal/controller/utils.go Show resolved Hide resolved

juniwang reviewed May 20, 2024

View reviewed changes

RichardChen820 force-pushed the user/junbchen/workloadIdentity branch 3 times, most recently from 8eca86e to 1fcf377 Compare May 22, 2024 09:12

Feedback

29a03e9

RichardChen820 force-pushed the user/junbchen/workloadIdentity branch from 1fcf377 to 29a03e9 Compare May 23, 2024 03:42

juniwang reviewed May 28, 2024

View reviewed changes

deploy/templates/clusterrole.yaml Show resolved Hide resolved

juniwang reviewed May 28, 2024

View reviewed changes

deploy/templates/serviceaccount.yaml Show resolved Hide resolved

juniwang approved these changes Jun 4, 2024

View reviewed changes

RichardChen820 merged commit 74da055 into release/v2/preview Jun 4, 2024
1 check passed

RichardChen820 deleted the user/junbchen/workloadIdentity branch June 4, 2024 06:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Workload identity support namespaced service account #39

Workload identity support namespaced service account #39

RichardChen820 commented Apr 30, 2024

linglingye001 May 13, 2024

RichardChen820 May 17, 2024

linglingye001 May 14, 2024

linglingye001 May 14, 2024

RichardChen820 May 17, 2024 •

edited

Loading

linglingye001 May 14, 2024

linglingye001 May 17, 2024

RichardChen820 May 17, 2024

linglingye001 May 17, 2024

juniwang May 20, 2024

RichardChen1122 May 23, 2024

linglingye001 May 17, 2024

RichardChen820 May 17, 2024

RichardChen820 May 17, 2024 •

edited

Loading

linglingye001 May 17, 2024

RichardChen820 May 20, 2024

juniwang May 20, 2024

RichardChen820 May 21, 2024

juniwang May 20, 2024

RichardChen820 May 21, 2024

Workload identity support namespaced service account #39

Workload identity support namespaced service account #39

Conversation

RichardChen820 commented Apr 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RichardChen820 May 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RichardChen820 May 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RichardChen820 May 17, 2024 •

edited

Loading

RichardChen820 May 17, 2024 •

edited

Loading