Use workload identity for azure cli when Federated token file is present #4939

nawazkh · 2024-06-24T23:21:41Z

What type of PR is this?
/kind feature

What this PR does / why we need it:

ensure-azcli.sh will log in using AZURE_WORKLOAD_ID and --federated-token when AZURE_FEDERATED_TOKEN_FILE is available in the env.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
POC as part of https://github.com/kubernetes/test-infra/blob/master/docs/job-migration-todo.md and #4976

Special notes for your reviewer:

Once this PR merges
- We should be updating CAPZ test jobs with preset-azure-cred-wi: "true" preset and dropping any unrelated credential presets.
- Migrate other test jobs to use WI.
cherry-pick candidate

TODOs:

squashed commits
includes documentation
adds unit tests

Release note:

`ensure-azcli.sh` will log in using `AZURE_WORKLOAD_ID` if `AZURE_FEDERATED_TOKEN_FILE` is available in the env.

k8s-ci-robot · 2024-06-24T23:21:43Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

codecov · 2024-06-24T23:28:32Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.24%. Comparing base (5a3f86f) to head (c3f6e88).
Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4939      +/-   ##
==========================================
- Coverage   62.25%   62.24%   -0.02%     
==========================================
  Files         201      201              
  Lines       16912    16912              
==========================================
- Hits        10529    10527       -2     
- Misses       5590     5592       +2     
  Partials      793      793

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

hack/parse-prow-creds.sh

hack/ensure-azcli.sh

mboersma

/lgtm

Seems like the right first step, so as not to break anything. I like your plan of action in the PR description. 👍🏻

k8s-ci-robot · 2024-06-26T16:35:09Z

LGTM label has been added.

Git tree hash: 1ad89fce498c9d1e5d73f4822f4df9d3cc83499e

jackfrancis · 2024-06-26T16:41:32Z

/test ls

k8s-ci-robot · 2024-06-26T16:41:35Z

@jackfrancis: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test pull-cluster-api-provider-azure-apiversion-upgrade
/test pull-cluster-api-provider-azure-build
/test pull-cluster-api-provider-azure-ci-entrypoint
/test pull-cluster-api-provider-azure-e2e
/test pull-cluster-api-provider-azure-e2e-aks
/test pull-cluster-api-provider-azure-test
/test pull-cluster-api-provider-azure-verify

The following commands are available to trigger optional jobs:

/test pull-cluster-api-provider-azure-apidiff
/test pull-cluster-api-provider-azure-capi-e2e
/test pull-cluster-api-provider-azure-conformance
/test pull-cluster-api-provider-azure-conformance-custom-builds
/test pull-cluster-api-provider-azure-conformance-dual-stack-with-ci-artifacts
/test pull-cluster-api-provider-azure-conformance-ipv6-with-ci-artifacts
/test pull-cluster-api-provider-azure-conformance-with-ci-artifacts
/test pull-cluster-api-provider-azure-e2e-optional
/test pull-cluster-api-provider-azure-e2e-workload-upgrade
/test pull-cluster-api-provider-azure-windows-containerd-upstream-with-ci-artifacts-serial-slow
/test pull-cluster-api-provider-azure-windows-custom-builds
/test pull-cluster-api-provider-azure-windows-with-ci-artifacts

Use /test all to run the following jobs that were automatically triggered:

pull-cluster-api-provider-azure-apidiff
pull-cluster-api-provider-azure-build
pull-cluster-api-provider-azure-ci-entrypoint
pull-cluster-api-provider-azure-e2e
pull-cluster-api-provider-azure-e2e-aks
pull-cluster-api-provider-azure-test
pull-cluster-api-provider-azure-verify

In response to this:

/test ls

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

jackfrancis · 2024-06-26T16:42:35Z

/test pull-cluster-api-provider-azure-capi-e2e
/test pull-cluster-api-provider-azure-conformance
/test pull-cluster-api-provider-azure-conformance-custom-builds
/test pull-cluster-api-provider-azure-conformance-dual-stack-with-ci-artifacts
/test pull-cluster-api-provider-azure-conformance-ipv6-with-ci-artifacts
/test pull-cluster-api-provider-azure-conformance-with-ci-artifacts
/test pull-cluster-api-provider-azure-e2e-optional
/test pull-cluster-api-provider-azure-e2e-workload-upgrade
/test pull-cluster-api-provider-azure-windows-custom-builds
/test pull-cluster-api-provider-azure-windows-with-ci-artifacts

mboersma · 2024-06-26T16:49:58Z

This isn't a bugfix per se, but still I think we may want to cherry-pick this change in order to test it through test-infra.

nawazkh · 2024-06-27T17:35:02Z

/test pull-cluster-api-provider-azure-e2e-optional

nawazkh · 2024-07-09T23:33:54Z

/test pull-cluster-api-provider-azure-e2e-with-wi-optional

- use AZURE_FEDERATED_TOKEN_FILE while logging in via wi - overload Azure client ID with Azure workload ID for WI based auth - use auth mode for azure storage commands when using federated identity - SC2086 use unquoted expansion with alt value

- use auth mode for azure storage commands when using federated identity - SC2086 use unquoted expansion with alt value Signed-off-by: James Sturtevant <[email protected]>

nawazkh · 2024-07-10T00:14:36Z

/test pull-cluster-api-provider-azure-e2e-with-wi-optional

nawazkh · 2024-07-10T00:15:52Z

scripts/ci-build-azure-ccm.sh

@@ -69,16 +68,16 @@ main() {
        echo "Building and pushing Linux and Windows amd64 Azure ACR credential provider"
        make -C "${AZURE_CLOUD_PROVIDER_ROOT}" bin/azure-acr-credential-provider bin/azure-acr-credential-provider.exe

-        if [[ "$(az storage container exists --name "${AZURE_BLOB_CONTAINER_NAME}" --query exists --output tsv)" == "false" ]]; then
+        if [[ "$(az storage container exists ${ENABLE_AUTH_MODE_LOGIN:+"--auth-mode login"} --name "${AZURE_BLOB_CONTAINER_NAME}" --query exists --output tsv)" == "false" ]]; then


${ENABLE_AUTH_MODE_LOGIN:+"--auth-mode login"} is the penultimate workaround in the list of exceptions.
SC2086#exceptions

Note that the value of ENABLE_AUTH_MODE_LOGIN immaterial as long as it is set for ${ENABLE_AUTH_MODE_LOGIN:+"--auth-mode login"} evaluate to "--auth-mode login"

nawazkh · 2024-07-10T01:21:38Z

/test pull-cluster-api-provider-azure-windows-custom-builds

nawazkh · 2024-07-10T01:55:16Z

Created and closed the PR kubernetes/test-infra#32938. Adding here for my reference.

jsturtevant · 2024-07-10T17:30:59Z

/lgtm

k8s-ci-robot · 2024-07-10T17:31:05Z

LGTM label has been added.

Git tree hash: 3866d1706d03a0465758caa8c982783d9830e1a8

jsturtevant · 2024-07-10T17:35:41Z

/retitle Use workload identity for azure cli when Federated token file is present

nawazkh · 2024-07-10T17:44:14Z

/unhold

nojnhuh

/lgtm

Thanks @nawazkh!

mboersma

/lgtm

mboersma · 2024-07-10T17:45:29Z

hack/ensure-azcli.sh

+
+    # Use --auth-mode "login" in az storage commands.
+    ENABLE_AUTH_MODE_LOGIN="true"
+    export ENABLE_AUTH_MODE_LOGIN


You can assign and export in one line, just fyi:

export ENABLE_AUTH_MODE_LOGIN="true"

I got a diff to use in the upcoming CAPZ test migration PR :)

Sometimes linter complains about assign + export in one line though!

k8s-ci-robot · 2024-07-10T19:56:20Z

@nawazkh: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-cluster-api-provider-azure-e2e-optional	`ebe5917`	link	false	`/test pull-cluster-api-provider-azure-e2e-optional`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

nawazkh · 2024-07-10T21:10:14Z

Timed out.. :(

 [FAILED] Timed out after 1800.001s.
  Timed out waiting for 3 control plane machines to exist
  Expected
      <int>: 1
  to equal
      <int>: 3
  In [It] at: /home/prow/go/pkg/mod/sigs.k8s.io/cluster-api/[email protected]/framework/controlplane_helpers.go:116 @ 07/10/24 20:48:37.121

/test pull-cluster-api-provider-azure-e2e

Signed-off-by: James Sturtevant <[email protected]>

k8s-ci-robot requested review from mboersma and willie-yao June 24, 2024 23:22

nawazkh changed the title ~~fetch AZURE_WORKLOAD_ID var from prow~~ use AZURE_WORKLOAD_ID var for prow Jun 24, 2024

nawazkh commented Jun 24, 2024

View reviewed changes

hack/parse-prow-creds.sh Outdated Show resolved Hide resolved

jsturtevant mentioned this pull request Jun 25, 2024

Add ability to use workload identity with Azure cli kubernetes-sigs/windows-testing#439

Closed

nawazkh force-pushed the capz_prow_2_wi branch from 9b900ee to 1c24b75 Compare June 25, 2024 18:27

k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 25, 2024

nawazkh marked this pull request as ready for review June 25, 2024 18:30

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 25, 2024

k8s-ci-robot requested a review from jackfrancis June 25, 2024 18:30

nawazkh mentioned this pull request Jun 25, 2024

add Workload ID to CAPZ tests as an env variable kubernetes/test-infra#32837

Merged

jackfrancis reviewed Jun 25, 2024

View reviewed changes

hack/ensure-azcli.sh Outdated Show resolved Hide resolved

nawazkh force-pushed the capz_prow_2_wi branch from 1c24b75 to ebe5917 Compare June 25, 2024 18:56

mboersma approved these changes Jun 26, 2024

View reviewed changes

k8s-ci-robot assigned mboersma Jun 26, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 26, 2024

nawazkh force-pushed the capz_prow_2_wi branch from 70fdd00 to 6b5f483 Compare July 9, 2024 23:33

nawazkh and others added 2 commits July 9, 2024 17:11

Remove storage key for azure storage commands

c3f6e88

- use auth mode for azure storage commands when using federated identity - SC2086 use unquoted expansion with alt value Signed-off-by: James Sturtevant <[email protected]>

nawazkh force-pushed the capz_prow_2_wi branch from 6b5f483 to c3f6e88 Compare July 10, 2024 00:12

nawazkh commented Jul 10, 2024

View reviewed changes

k8s-ci-robot assigned jsturtevant Jul 10, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 10, 2024

k8s-ci-robot changed the title ~~use AZURE_WORKLOAD_ID var for prow~~ Use workload identity for azure cli when Federated token file is present Jul 10, 2024

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 10, 2024

nojnhuh approved these changes Jul 10, 2024

View reviewed changes

k8s-ci-robot assigned nojnhuh Jul 10, 2024

mboersma approved these changes Jul 10, 2024

View reviewed changes

nawazkh mentioned this pull request Jul 10, 2024

migrate presubmit e2e test to use WI kubernetes/test-infra#32942

Merged

k8s-ci-robot merged commit 21479a9 into kubernetes-sigs:main Jul 10, 2024
28 checks passed

nawazkh deleted the capz_prow_2_wi branch July 10, 2024 22:35

jsturtevant added a commit to rtkgupta/windows-testing that referenced this pull request Jul 10, 2024

Use latest from kubernetes-sigs/cluster-api-provider-azure#4939

2d5d9b5

Signed-off-by: James Sturtevant <[email protected]>

jsturtevant mentioned this pull request Jul 11, 2024

Use the env AZURE_STORAGE_AUTH_MODE when using federated auth #4980

Merged

4 tasks

nawazkh mentioned this pull request Jul 12, 2024

Migrate Upstream tests to community infrastructure #4976

Closed

6 tasks

jsturtevant mentioned this pull request Jul 29, 2024

Use federated token to sign-in on new sub kubernetes-sigs/windows-testing#461

Merged

jackfrancis mentioned this pull request Oct 31, 2024

add nawazkh to maintainers #5224

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use workload identity for azure cli when Federated token file is present #4939

Use workload identity for azure cli when Federated token file is present #4939

nawazkh commented Jun 24, 2024 •

edited

Loading

k8s-ci-robot commented Jun 24, 2024

codecov bot commented Jun 24, 2024 •

edited

Loading

mboersma left a comment

k8s-ci-robot commented Jun 26, 2024

jackfrancis commented Jun 26, 2024

k8s-ci-robot commented Jun 26, 2024

jackfrancis commented Jun 26, 2024

mboersma commented Jun 26, 2024

nawazkh commented Jun 27, 2024

nawazkh commented Jul 9, 2024

nawazkh commented Jul 10, 2024

nawazkh Jul 10, 2024

jsturtevant Jul 10, 2024

nawazkh commented Jul 10, 2024

nawazkh commented Jul 10, 2024

jsturtevant commented Jul 10, 2024

k8s-ci-robot commented Jul 10, 2024

jsturtevant commented Jul 10, 2024

nawazkh commented Jul 10, 2024

nojnhuh left a comment

mboersma left a comment

mboersma Jul 10, 2024

nawazkh Jul 10, 2024

jackfrancis Jul 10, 2024

k8s-ci-robot commented Jul 10, 2024 •

edited

Loading

nawazkh commented Jul 10, 2024

Use workload identity for azure cli when Federated token file is present #4939

Use workload identity for azure cli when Federated token file is present #4939

Conversation

nawazkh commented Jun 24, 2024 • edited Loading

k8s-ci-robot commented Jun 24, 2024

codecov bot commented Jun 24, 2024 • edited Loading

Codecov Report

mboersma left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jun 26, 2024

jackfrancis commented Jun 26, 2024

k8s-ci-robot commented Jun 26, 2024

jackfrancis commented Jun 26, 2024

mboersma commented Jun 26, 2024

nawazkh commented Jun 27, 2024

nawazkh commented Jul 9, 2024

nawazkh commented Jul 10, 2024

nawazkh Jul 10, 2024

Choose a reason for hiding this comment

jsturtevant Jul 10, 2024

Choose a reason for hiding this comment

nawazkh commented Jul 10, 2024

nawazkh commented Jul 10, 2024

jsturtevant commented Jul 10, 2024

k8s-ci-robot commented Jul 10, 2024

jsturtevant commented Jul 10, 2024

nawazkh commented Jul 10, 2024

nojnhuh left a comment

Choose a reason for hiding this comment

mboersma left a comment

Choose a reason for hiding this comment

mboersma Jul 10, 2024

Choose a reason for hiding this comment

nawazkh Jul 10, 2024

Choose a reason for hiding this comment

jackfrancis Jul 10, 2024

Choose a reason for hiding this comment

k8s-ci-robot commented Jul 10, 2024 • edited Loading

nawazkh commented Jul 10, 2024

nawazkh commented Jun 24, 2024 •

edited

Loading

codecov bot commented Jun 24, 2024 •

edited

Loading

k8s-ci-robot commented Jul 10, 2024 •

edited

Loading