CI: workload-identity native #4765
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -108,6 +108,4 @@ capz::util::generate_ssh_key() { | |
| capz::util::ensure_azure_envs() { | ||
| : "${AZURE_SUBSCRIPTION_ID:?Environment variable empty or not defined.}" | ||
| : "${AZURE_TENANT_ID:?Environment variable empty or not defined.}" | ||
|
There was a problem hiding this comment. Isn't client id still required with workload identity? I see it's being kept in other places such as https://github.com/kubernetes-sigs/cluster-api-provider-azure/pull/4765/files#diff-c2ee8653e1d6b85f0aadf87cd438a9250806c052877248442be4d434cbc52425R37 There was a problem hiding this comment. It's not required, no |
||
|
There was a problem hiding this comment. both and There was a problem hiding this comment. here is a possible change set that could address this: https://github.com/kubernetes-sigs/cluster-api-provider-azure/compare/main...jsturtevant:cluster-api-provider-azure:remove-az-cli-cred-checks?expand=1 Let me know if you want to integrate the change into this PR or have me open a separate one There was a problem hiding this comment. The acr change lets local users build the image with out having to manually log in. It fixes an issue we saw when running kubernetes-sigs/windows-testing#430 locally with a test ACR instance There was a problem hiding this comment. Thanks for this! To remove variables in case there are still edge cases from this change, let's do this as a follow-up PR. |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| #!/usr/bin/env bash | ||
| # Copyright 2024 The Kubernetes Authors. | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| set -o errexit | ||
| set -o nounset | ||
| set -o pipefail | ||
|
|
||
| # Install kubectl and kind | ||
| REPO_ROOT=$(dirname "${BASH_SOURCE[0]}")/.. | ||
| # shellcheck source=hack/ensure-azcli.sh | ||
| source "${REPO_ROOT}/hack/ensure-azcli.sh" | ||
|
|
||
| AZWI_RESOURCE_GROUP="${AZWI_RESOURCE_GROUP:-}" | ||
|
|
||
| if [[ -z "${AZWI_RESOURCE_GROUP}" ]]; then | ||
| echo AZWI_RESOURCE_GROUP environment variable must be set | ||
| exit 1 | ||
jackfrancis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| fi | ||
|
|
||
| echo "Cleaning up CI workload-identity infra..." | ||
| az group delete --no-wait -y -n "${AZWI_RESOURCE_GROUP}" | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we use the default kubeadm-generated keys, then I don't think we need
azwiat all.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's correct (planning to do that in a follow-up PR pending priority/cycles)