fix: configure service-account-issuer in prow CI cluster templates#6288
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6288 +/- ##
=======================================
Coverage 43.84% 43.84%
=======================================
Files 289 289
Lines 25346 25346
=======================================
Hits 11112 11112
Misses 13460 13460
Partials 774 774 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/retest |
3cae9d8 to
06782c2
Compare
06782c2 to
0367def
Compare
Set service-account-issuer: ${SERVICE_ACCOUNT_ISSUER} in the base
cluster template (templates/flavors/base/cluster-template.yaml).
This ensures projected service account tokens are signed with a
discoverable issuer URL. The default value falls back to the
kube-apiserver default (https://kubernetes.default.svc.cluster.local),
so existing deployments are unaffected.
When SERVICE_ACCOUNT_ISSUER is set (e.g., to a public OIDC endpoint),
workload identity flows (CSI drivers, pod identity) will work correctly
because AAD can discover and validate the token issuer.
/kind bug
0367def to
ca6f966
Compare
|
/assign @mboersma |
There was a problem hiding this comment.
/test pull-cluster-api-provider-azure-e2e
(The other failures are "ghosts" from when this was opened against the release-1.23 branch, so we can ignore them.)
This looks good and AFAICT should be safely backward-compatible. But in the PR description you say:
Modified 29 prow CI templates under templates/test/ci/
No user-facing templates are changed
That's not actually true: it changes 50+ templates, many of which are user-facing. I just want to ensure that this was the intention, because if you intended to scope it just to prow CI templates, it doesn't.
Also we could perhaps expand the description of SERVICE_ACCOUNT_ISSUER in docs/book/src/topics/workload-identity.md (line 43 and/or 96). Maybe:
If unset, the cluster template falls back to kubeadm's default
https://kubernetes.default.svc.cluster.local. Set it to your Azure storage container URL
when running `clusterctl generate cluster ...` so projected service account tokens in the
workload cluster are signed with an AAD-discoverable issuer.
Clarify that if SERVICE_ACCOUNT_ISSUER is unset, kubeadm defaults to https://kubernetes.default.svc.cluster.local, and explain when users should set it to their Azure storage container URL.
|
/test pull-cluster-api-provider-azure-e2e |
@mboersma it's done, thanks. |
mboersma
left a comment
There was a problem hiding this comment.
/label tide/merge-method-squash
/lgtm
/approve
/cherry-pick release-1.24
/cherry-pick release-1.23
|
LGTM label has been added. DetailsGit tree hash: fbeaf3bd6ca94394eeccdd8d02412cce29a49152 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mboersma The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/cherry-pick release-1.24 |
|
@mboersma: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
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. |
|
/cherry-pick release-1.23 |
|
@mboersma: #6288 failed to apply on top of branch "release-1.23": DetailsIn response to this:
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. |
|
@mboersma: new pull request created: #6290 DetailsIn response to this:
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. |
…6288) * fix: configure service-account-issuer in cluster templates Set service-account-issuer: ${SERVICE_ACCOUNT_ISSUER} in the base cluster template (templates/flavors/base/cluster-template.yaml). This ensures projected service account tokens are signed with a discoverable issuer URL. The default value falls back to the kube-apiserver default (https://kubernetes.default.svc.cluster.local), so existing deployments are unaffected. When SERVICE_ACCOUNT_ISSUER is set (e.g., to a public OIDC endpoint), workload identity flows (CSI drivers, pod identity) will work correctly because AAD can discover and validate the token issuer. /kind bug * docs: expand SERVICE_ACCOUNT_ISSUER description in workload-identity.md Clarify that if SERVICE_ACCOUNT_ISSUER is unset, kubeadm defaults to https://kubernetes.default.svc.cluster.local, and explain when users should set it to their Azure storage container URL.
What this PR does
Adds
service-account-issuer: ${SERVICE_ACCOUNT_ISSUER:-https://kubernetes.default.svc.cluster.local}to theapiServer.extraArgsin cluster templates that configure kubeadm-based control planes.Why
Cluster templates create workload clusters with
apiServer.extraArgs: {}, causing kube-apiserver to use the default--service-account-issuer=https://kubernetes.default.svc.cluster.local. This is an internal URL that AAD cannot reach from the public internet.When CSI drivers (blob-csi-driver, azurefile-csi-driver) run workload identity e2e tests on these clusters, the projected service account tokens are signed with the internal issuer. AAD rejects them because it cannot fetch the OIDC discovery document, causing mount failures (
mount error(126): Required key not available).Changes
templates/test/ci/, 19 user-facing templates undertemplates/, and 14 dev templates undertemplates/test/dev/)docs/book/src/topics/workload-identity.mdto expand the description ofSERVICE_ACCOUNT_ISSUERhttps://kubernetes.default.svc.cluster.local) preserves backward compatibility — existing clusters withoutSERVICE_ACCOUNT_ISSUERset behave identically to beforeAddresses
Feedback from #6268
/kind bug
Release note: