Skip to content

fix(telemetry): the Collector DaemonSet mounted an unused SA token on every node (backend#2344) - #793

Merged
LukasWodka merged 1 commit into
developfrom
fix/2344-collector-automount-token
Aug 22, 2026
Merged

fix(telemetry): the Collector DaemonSet mounted an unused SA token on every node (backend#2344)#793
LukasWodka merged 1 commit into
developfrom
fix/2344-collector-automount-token

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Fixes the Bugbot Medium raised on the develop -> staging promotion (#789), tracked in backend#2344. Fixed on develop, not on the promotion PR.

What was wrong

templates/telemetry-collector-daemonset.yaml set serviceAccountName but named no automountServiceAccountToken, so it inherited the Kubernetes default of true.

Verified the token is genuinely unused before removing it:

  • telemetry-collector-rbac.yaml renders a ServiceAccount and nothing else — no Role, no ClusterRole. Its own comment says the Collector "never calls the Kubernetes API"; the template agrees, and it: grants no Kubernetes API permissions at all already asserts it.
  • The Role in telemetry-token-rbac.yaml binds jobs-manager's SA, not the Collector's.
  • The ingest token the Collector does read comes from the projected Secret volume at /etc/tracebloc/telemetry (bearertokenauth.filename). Nothing references /var/run/secrets/kubernetes.io/serviceaccount.

So there is no API access to lose. Being a DaemonSet, the unmet cost was the widest one available: a credential nothing reads, mounted on every node of every customer cluster.

What changed

  1. automountServiceAccountToken: false on the pod spec, with a comment saying why it is stated rather than defaulted.
  2. it: does not automount the service account token in tests/telemetry_collector_test.yaml, next to the existing RBAC assertion — the same claim from the pod side.
  3. Chart version/appVersion 1.9.61 -> 1.9.62.

The test is the load-bearing half. An absent value renders as no key rather than as a failure, so without an assertion naming it the DaemonSet could silently drop back to the default and the suite would stay green.

Verification

helm unittest client -> 539 passed, 34 suites, 0 failures
make drift -> all 13 guards green

Mutation-proven both ways, anchor asserted each time:

mutationresult
delete the line entirely1 failed, 46 passedunknown path spec.template.spec.automountServiceAccountToken
flip false -> true1 failed, 46 passedExpected to equal: false / Actual: true

One correction to the ticket, and a follow-up

backend#2344 says the Collector is "the only pod spec in the chart that does not state automountServiceAccountToken". That is wrong — I counted. Of 13 pod specs, 6 state it and 7 do not:

states itsilent
egress-proxy-deployment, requests-proxy-deployment, egress-enforcement-check, egress-reachability-check, storage-assertions-check, ingestor/post-install-jobtelemetry-collector-daemonset (this PR), resource-monitor-daemonset, jobs-manager-deployment, auto-upgrade-cronjob, image-refresh-cronjob, gpu-device-plugin, mysql-deployment

So "state it either way" is a minority practice, not a convention — which also means the chart-level guard the ticket floats cannot be armed in this PR without landing it red, against the arm-while-green rule. Six templates need a deliberate true/false decision first (the three API workloads plausibly true, the two SA-less ones false, resource-monitor needs checking). Filed separately rather than smuggled in here.

Closes backend#2344.


Note

Low Risk
Small hardening that removes an unused SA token from a DaemonSet; no API access or ingest-auth path is changed. Risk is a regression only if something unexpectedly needed that token, which existing RBAC already forbids.

Overview
Stops the telemetry Collector DaemonSet from inheriting Kubernetes’ default automountServiceAccountToken: true. The Collector never calls the API (SA has no Role/ClusterRole; ingest uses a projected Secret), so the default was mounting an unused credential on every node.

Sets automountServiceAccountToken: false on the pod spec and adds a helm-unittest assertion so an absent key cannot silently fall back to the default. Chart version/appVersion 1.9.61 → 1.9.62.

Reviewed by Cursor Bugbot for commit b437b36. Bugbot is set up for automated code reviews on this repo. Configure here.

… every node (backend#2344)
The pod spec omitted automountServiceAccountToken, so it inherited the
Kubernetes default of true. The ServiceAccount exists only as the SCC
binding subject on OpenShift -- telemetry-collector-rbac.yaml gives it no
Role and no ClusterRole, and the ingest token it actually needs is the
projected Secret at /etc/tracebloc/telemetry. Nothing read the mounted
credential, and being a DaemonSet it landed on every node of every
customer cluster.
Also adds the assertion that was missing: with no test naming the key, a
value that is absent renders as no key rather than as a failure, so the
next collector template was free to repeat this. Mutation-proven both
ways -- deleting the line and flipping it to true each redden exactly one
test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodkaLukasWodka self-assigned this Aug 22, 2026
@LukasWodka
LukasWodka merged commit 0985814 into developAug 22, 2026
25 checks passed
@LukasWodka
LukasWodka deleted the fix/2344-collector-automount-token branch August 22, 2026 18:14

@saqlainsyed007saqlainsyed007 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed last pass and held only on Unit tests, which is now green — every check passes, Bugbot clear, no open threads.

Verified: the Collector's ServiceAccount is identity-only (no Role/ClusterRole in telemetry-collector-rbac.yaml; the ingest token comes from the projected Secret at /etc/tracebloc/telemetry, not the SA token), so automountServiceAccountToken: false loses no API access — pure surface reduction, and on a DaemonSet that's every node of every customer cluster. The added helm-unittest assertion is mutation-proof: equal on spec.template.spec.automountServiceAccountToken fails if the key is absent, so dropping the template line reddens it rather than passing vacuously. Chart bump 1.9.61→1.9.62 present. Approving.

@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

/fr-pass

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@LukasWodka@saqlainsyed007