Repository navigation
fix(chart): mount the service account token when automountServiceAccountToken is false - #6814
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @ashokpariya0! |
|
Hi @ashokpariya0. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
|
@ashokpariya0 please can you rebase this PR? |
…untToken is false Setting automountServiceAccountToken to false removes the token from the pod, but ExternalDNS always talks to the Kubernetes API, so the pod crashes with "open /var/run/secrets/kubernetes.io/serviceaccount/token: no such file or directory". When the value is false, mount a projected token (token, kube-root-ca.crt and namespace, same layout as the kubelet) into the external-dns container only. The webhook sidecar does not get it, which is the usual reason to turn automounting off. If extraVolumeMounts already mounts that path, the chart leaves it alone so existing workarounds keep working. Signed-off-by: Ashok Pariya <ashok.pariya@ibm.com>
9b12124 to
80e22ce
Compare
Done. |
|
/ok-to-test |
|
/lgtm |
Coverage Report for CI Build 43425169554Coverage increased (+0.005%) to 83.476%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
mloiseleur
left a comment
There was a problem hiding this comment.
Thanks, the fix is sound. Small points inline.
| {{- /* ExternalDNS always needs the Kubernetes API. With automounting disabled, mount the token | ||
| into the external-dns container only (not the webhook sidecar), unless the user already | ||
| mounts that path through extraVolumeMounts. */}} |
There was a problem hiding this comment.
The extraVolumeMounts part restates the loop below.
| {{- /* ExternalDNS always needs the Kubernetes API. With automounting disabled, mount the token | |
| into the external-dns container only (not the webhook sidecar), unless the user already | |
| mounts that path through extraVolumeMounts. */}} | |
| {{- /* ExternalDNS always needs the API: mount the token into the external-dns container only, not the webhook sidecar. */}} |
| sources: | ||
| - serviceAccountToken: | ||
| path: token | ||
| expirationSeconds: 3607 |
There was a problem hiding this comment.
3607 looks arbitrary, but it is the value the ServiceAccount admission plugin injects (WarnOnlyBoundTokenExpirationSeconds), and the API server extends exactly this value.
Could you add a short comment so nobody "fixes" it to 3600?
| expirationSeconds: 3607 | |
| {{- /* 3607 like the auto-mounted token: the API server extends this exact value. */}} | |
| expirationSeconds: 3607 |
What does it do ?
When automountServiceAccountToken is false, the chart now mounts a projected service account token into the external-dns container. It’s the same layout the kubelet uses: token, ca.crt from kube-root-ca.crt, and namespace. The webhook sidecar doesn’t get the token. If extraVolumeMounts already mounts /var/run/secrets/kubernetes.io/serviceaccount, the chart adds nothing, so existing workarounds keep working.
Motivation
Fixes #6704
With automountServiceAccountToken set to false, the pod crashes with “open /var/run/secrets/kubernetes.io/serviceaccount/token: no such file or directory”, because ExternalDNS always talks to the Kubernetes API. People turn automounting off to keep the token away from the webhook sidecar, but it broke external-dns itself.
Tested on a v1.36 cluster (s390x, inmemory provider, automountServiceAccountToken=false):
master branch: pod in CrashLoopBackOff with the token error above
this branch: pod Running, log shows “Created Kubernetes client” and “All records are already up to date”
More