feat(auth): add Workload Identity Federation (OIDC) support#1424
feat(auth): add Workload Identity Federation (OIDC) support#1424philross wants to merge 2 commits into
Conversation
|
Before I continue working on this feature: Is the introduction of environment variables ( |
|
This PR was marked as stale after 7 days of inactivity and will be closed after another 7 days of further inactivity. If this PR should be kept open, just add a comment, remove the stale label or push new commits to it. |
|
Not stale, @GokceGK could you help route this to the right person/team to get an initial response on the question above? |
|
Hi @philross, thanks for starting this contribution. I will check your question and get back to you asap. Regards, |
|
Hi @philross, you can use env variables, but please align them with the STACKIT Terraform Provider. Regards, |
|
Thanks @GokceGK, According to https://registry.terraform.io/providers/stackitcloud/stackit/latest/docs/guides/workload_identity_federation, |
f0d60b0 to
415910f
Compare
|
This PR was marked as stale after 7 days of inactivity and will be closed after another 7 days of further inactivity. If this PR should be kept open, just add a comment, remove the stale label or push new commits to it. |
|
Not stale |
|
This PR was marked as stale after 7 days of inactivity and will be closed after another 7 days of further inactivity. If this PR should be kept open, just add a comment, remove the stale label or push new commits to it. |
Allows CI/CD pipelines to authenticate without long-lived service account key files by exchanging a short-lived OIDC token for a STACKIT access token.
415910f to
6fdc517
Compare
| }), | ||
| }, | ||
| { | ||
| description: "oidc_mode_no_key_required", |
There was a problem hiding this comment.
This name is confusing here: OIDC is not set but the name implies that it is set
There was a problem hiding this comment.
You are right, the test does not make too much sense as is.
TestParseInput currently validates flag parsing only and does not consider environment variables, which would be needed to test the OIDC setup. Would you prefer that we extend this function/test scope to include env vars, or would it be better to keep parseInput focused on flags and add a separate OIDC-focused test in activate_service_account_test.go ?
There was a problem hiding this comment.
I would keep TestParseInput as is. You already added tests to oidc_test.go to check the correct handling of the oidc flag, right?
| `Authenticate via Workload Identity Federation (OIDC) and print the short-lived access token. Set STACKIT_USE_OIDC=1 and STACKIT_SERVICE_ACCOUNT_EMAIL; no service account key file is required.`, | ||
| "$ STACKIT_USE_OIDC=1 STACKIT_SERVICE_ACCOUNT_EMAIL=ci@sa.stackit.cloud stackit auth activate-service-account --only-print-access-token", | ||
| ), | ||
| ), |
There was a problem hiding this comment.
I checked the TFP implementation against this implementation and found one thing which is different.
The TFP implementation allows to provide a use_oidc in the provider configuration. If this is set this has higher priority (than the environment variable).
1. Explicit configuration, e.g. by setting the field `use_oidc` in the provider block (see example below)
2. Environment variable, e.g. by setting `STACKIT_USE_OIDC`
3. Credentials file
In order to use the same mechanism here we need to add a cli flag as well (if provided -> cli flag has prio, if not environment variable is taken).
Description
Adds Workload Identity Federation (OIDC) support so CI/CD pipelines can authenticate without long-lived service account key files. The CLI exchanges a short-lived OIDC token for a STACKIT access token using the go SDK's WIF flow.
internal/pkg/auth/auth.gowith env-var helpers (STACKIT_USE_OIDC,STACKIT_SERVICE_ACCOUNT_EMAIL,STACKIT_SERVICE_ACCOUNT_FEDERATED_TOKEN)AUTHENTICATION.mdwith setup guiderelates to #1327
Checklist
make fmtmake generate-docs(will be checked by CI)make test(will be checked by CI)make lint(will be checked by CI)