[rbac-manager] Support workload identity #330

jace-ys · 2023-12-08T15:34:56Z

Hey folks 👋🏻

Hope you don't mind this contribution but we'd like to see theatre support workload identity in the rbac-manager instead of using service account keys. I've made the change such that if workload identity is not configured, the rbac-manager will fallback to using service account keys.

This is how we're currently using it with workload identity in our GKE cluster (after removing GOOGLE_APPLICATION_CREDENTIALS):

Same change on our fork: duffelhq#3

# Config Connector CRDs
---
apiVersion: iam.cnrm.cloud.google.com/v1beta1
kind: IAMPolicy
metadata:
  name: theatre-workload-identity-user
  annotations:
    cnrm.cloud.google.com/project-id: duffel-prod
spec:
  bindings:
  - members:
    - serviceAccount:duffel-prod.svc.id.goog[theatre-system/theatre-rbac-manager]
    role: roles/iam.workloadIdentityUser
  - members:
    - serviceAccount:[email protected]
    # Required so that the theatre service account can impersonate itself
    role: roles/iam.serviceAccountTokenCreator
  resourceRef:
    apiVersion: iam.cnrm.cloud.google.com/v1beta1
    kind: IAMServiceAccount
    external: projects/duffel-prod/serviceAccounts/[email protected]

 kubectl annotate serviceaccount theatre-rbac-manager \
    --namespace theatre-system \
    iam.gke.io/[email protected]

jace-ys · 2023-12-08T18:07:16Z

@vinayvinay I think you're the one left in GC that I know..

Any idea who would be best suited to review this? 😁

jace-ys · 2024-08-19T20:30:51Z

Makefile

@@ -68,7 +68,7 @@ manifests: generate
 install-tools:
 	go install github.com/onsi/ginkgo/[email protected]
 	go install sigs.k8s.io/controller-tools/cmd/[email protected]
-	go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest
+	go install sigs.k8s.io/controller-runtime/tools/setup-envtest@release-0.17


Shamelessly stolen from #335 to get my CI tests passing.. 😬

jace-ys · 2024-08-20T09:44:57Z

@mbfisher @bogvak @0x0013 Sorry for the direct tag but wanted to get this merged and saw that you folks have been recently active on this repo!

mbfisher · 2024-08-20T09:49:01Z

@mbfisher @bogvak @0x0013 Sorry for the direct tag but wanted to get this merged and saw that you folks have been recently active on this repo!

I'm a contributor so can't help I'm afraid, Core Infra own this so I'd raise a SPOC ticket with them to get a review

jace-ys · 2024-08-20T09:49:48Z

I'm a contributor so can't help I'm afraid, Core Infra own this so I'd raise a SPOC ticket with them to get a review

Thanks for letting me know! Unfortunately I'm no longer at GC so don't think I'd be able to do that 😅

benwh · 2024-09-09T12:02:59Z

Bumping - it'd be great to get this in!

👋 @ttamimi @VSpike @ijames-gc

0x0013

@jace-ys Thanks, this looks like a valuable addition.

I'm no expert in GCP authentication flow, so I would like to ask some clarifications before I can review, mostly for my own understanding. I've left these in code comments. Apologies if the answers to these are obvious.

0x0013 · 2024-11-25T14:28:36Z

cmd/rbac-manager/main.go

+			Subject:         subject,
+		}
+
+		ts, err := impersonate.CredentialsTokenSource(ctx, config, option.WithCredentials(creds))


Is it necessary for the account to impersonate itself, instead of using the TokenSource already returned by FindDefaultCredentials?

Good question. I'm no expert in GCP auth myself and it's not very well documented or discussed much, but from reading various sources, my understanding is that the federated access token obtained from the GCE
metadata server (via workload identity) is not sufficient for acting as the subject via domain-wide delegation to call Google Workspace Admin APIs.

For delegation to work, we need to sign a JWT with the the "sub" claim set to subject - this happens implicitly through impersonation.

You can kind of see this behaviour in userTokenSource: https://github.com/googleapis/google-api-go-client/blob/af0a938ba1e01e7c2ce7f96fef1e62478f5da784/impersonate/user.go#L96-L103

Whereas you don't see this in computeSource obtained by WID: https://cs.opensource.google/go/x/oauth2/+/refs/tags/v0.24.0:google/google.go;l=270

Hope that make sense!

Also, the changes in this PR were largely inspired / adapted from dexidp/dex#2989

Some useful reading:

https://blog.salrashid.dev/articles/2021/impersonation_and_domain_delegation

https://cloud.google.com/iam/docs/workload-identity-federation#access_management

That said, looks like there have been some new developments since I first opened this PR.. 👀

It looks like it's possible to assign Workspace Admin Groups Reader role to service accounts:

https://support.google.com/a/answer/9807615?sjid=13011041076049488223-AP#:~:text=Assign%20a%20role%20to%20a%20service%20account

https://cloud.google.com/identity/docs/how-to/setup#auth-no-dwd

Which might mean that there's no need for impersonation or domain-wide delegation anymore, which would greatly simplify the auth here (just google.FindDefaultCredentials might be all that's needed) 🤔

But I think that warrants a separate PR as it's a different feature altogether..

0x0013 · 2024-11-25T14:33:00Z

cmd/rbac-manager/main.go

+			return nil, err
+		}
+
+		return directoryv1.NewService(ctx, option.WithTokenSource(ts))


I wonder if the option when using a supplied json credentials (old behavior) could also use the TokenSource from Credentials returned by FindDefaultCredentials?

If so, likely the code could be simplified a little, instead of needing to call NewService with different option for each credential type.

Yeap that's a good point! I didn't change that initially as I didn't want to make too much changes to existing code.

But you're right that we can use the same WithTokenSource option for both the new and old behaviours. That said, it's not TokenSource from the Credentials returned by FindDefaultCredentials but the TokenSource from the jwt.Config, as we need the token source to have the (domain-wide delegation) subject set.

I've updated the PR to use WithTokenSource for both code paths 👍🏻

0x0013 · 2024-12-16T16:08:05Z

@jace-ys it seems the container-builder hasn't built an image for this branch - perhaps it didn't receive the webhook at the time of your last push.

Could you generate a push event on this PR, for example, by pushing an empty commit, to see if it triggers a build? Thanks!

jace-ys · 2024-12-16T16:18:03Z

@jace-ys it seems the container-builder hasn't built an image for this branch - perhaps it didn't receive the webhook at the time of your last push.

Could you generate a push event on this PR, for example, by pushing an empty commit, to see if it triggers a build? Thanks!

Tried that but doesn't look like it triggered a build either! 🙁

0x0013 · 2024-12-16T16:58:18Z

Tried that but doesn't look like it triggered a build either! 🙁

@jace-ys thanks for trying, it looks like container-builder is still not picking it up. I assume this is because the head branch is external from GC.

I'll see if there's anything that can be done about it.

jace-ys marked this pull request as draft December 8, 2023 16:23

jace-ys marked this pull request as ready for review December 8, 2023 17:56

jace-ys mentioned this pull request Dec 11, 2023

Google: Implement groups fetch by default service account from metadata (support for GKE workload identity) dexidp/dex#2989

Merged

jace-ys force-pushed the jace/rbac-manager-workload-identity branch 3 times, most recently from b4d17a9 to 44d5970 Compare August 19, 2024 20:25

jace-ys commented Aug 19, 2024

View reviewed changes

0x0013 reviewed Nov 25, 2024

View reviewed changes

[rbac-manager] Support workload identity

3b04f9d

jace-ys force-pushed the jace/rbac-manager-workload-identity branch from 44d5970 to 2a18959 Compare November 30, 2024 06:30

Address comments

3cdb482

jace-ys force-pushed the jace/rbac-manager-workload-identity branch from 2a18959 to 3cdb482 Compare December 16, 2024 16:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[rbac-manager] Support workload identity #330

[rbac-manager] Support workload identity #330

jace-ys commented Dec 8, 2023 •

edited

Loading

jace-ys commented Dec 8, 2023

jace-ys Aug 19, 2024

jace-ys commented Aug 20, 2024

mbfisher commented Aug 20, 2024

jace-ys commented Aug 20, 2024 •

edited

Loading

benwh commented Sep 9, 2024

0x0013 left a comment

0x0013 Nov 25, 2024

jace-ys Nov 30, 2024 •

edited

Loading

jace-ys Nov 30, 2024

jace-ys Nov 30, 2024

jace-ys Nov 30, 2024

0x0013 Nov 25, 2024

jace-ys Nov 30, 2024 •

edited

Loading

jace-ys Nov 30, 2024 •

edited

Loading

0x0013 commented Dec 16, 2024

jace-ys commented Dec 16, 2024

0x0013 commented Dec 16, 2024

[rbac-manager] Support workload identity #330

Are you sure you want to change the base?

[rbac-manager] Support workload identity #330

Conversation

jace-ys commented Dec 8, 2023 • edited Loading

jace-ys commented Dec 8, 2023

jace-ys Aug 19, 2024

Choose a reason for hiding this comment

jace-ys commented Aug 20, 2024

mbfisher commented Aug 20, 2024

jace-ys commented Aug 20, 2024 • edited Loading

benwh commented Sep 9, 2024

0x0013 left a comment

Choose a reason for hiding this comment

0x0013 Nov 25, 2024

Choose a reason for hiding this comment

jace-ys Nov 30, 2024 • edited Loading

Choose a reason for hiding this comment

jace-ys Nov 30, 2024

Choose a reason for hiding this comment

jace-ys Nov 30, 2024

Choose a reason for hiding this comment

jace-ys Nov 30, 2024

Choose a reason for hiding this comment

0x0013 Nov 25, 2024

Choose a reason for hiding this comment

jace-ys Nov 30, 2024 • edited Loading

Choose a reason for hiding this comment

jace-ys Nov 30, 2024 • edited Loading

Choose a reason for hiding this comment

0x0013 commented Dec 16, 2024

jace-ys commented Dec 16, 2024

0x0013 commented Dec 16, 2024

jace-ys commented Dec 8, 2023 •

edited

Loading

jace-ys commented Aug 20, 2024 •

edited

Loading

jace-ys Nov 30, 2024 •

edited

Loading

jace-ys Nov 30, 2024 •

edited

Loading

jace-ys Nov 30, 2024 •

edited

Loading