fix(telemetry): the token Secret has to precede the flag (backend#2400)

[LukasWodka]

Fixes tracebloc/backend#2400.

The deadlock

The Collector could not be enabled on any edge. Measured on tb-client-dev-templates, chart client-1.9.63 — the published version every edge auto-upgrades to hourly.

enabled=false → jobs-manager has no TELEMETRY_TOKEN_SECRET_* env → it writes no Secret
             → the chart's fail-guard refuses enabled=true because the Secret is absent
             → enabled=false

Neither half was wrong on its own, which is why both passed review. Refusing the release beats CrashLoopBackOff on 200 nodes; an edge with the feature off should make no API calls for it. Nothing tested the pair.

Three gates, not one

All three read telemetryCollector.enabled. Ungating fewer than all three ships a fix that looks applied and does nothing, because sync_telemetry_token_secret is fail-soft — it logs, returns False, never raises — so a 403 on the write is invisible from the release:

gate	file	what its omission costs
the env vars	jobs-manager-deployment.yaml	no coordinates, no write — the one originally named
the Role + RoleBinding	telemetry-token-rbac.yaml	403 on the write, green release, no Secret
the namespace	node-agents-namespace.yaml	404 on an edge with resourceMonitor: false

All three were found exactly where the ticket said.

What is not changed

The fail guard in telemetry-collector-daemonset.yaml is untouched and stays exactly as strict. It was never wrong — its precondition was unreachable. This makes the precondition satisfiable, not optional.

No client-runtime change. sync_telemetry_token_secret already does the right thing the moment the env is present: create-then-patch-on-409, fail-soft, and the token it writes is the backend token jobs-manager already holds (jobs_manager.py:4676 at startup, :4734 on rebind). Nothing about the token's availability ever depended on the flag. (One docstring there now describes the old behaviour; a one-line correction follows in its own PR.)

One deviation from the ticket, with the measurement

The ticket asked for tracebloc.nodeAgentsInUse to be extended so the namespace covers the token sync. The token sync is unconditional, so that third disjunct is a constant true — which makes the whole helper a constant true. Rendered both ways with resourceMonitor=false, enabled=false, that costs:

+ Role/RoleBinding  t-auto-upgrade-node-agents    (DaemonSet rights, no DaemonSets)
+ Role/RoleBinding  t-image-refresh-node-agents   (DaemonSet rights, no DaemonSets)

Widening RBAC as a side effect of a telemetry fix is the opposite direction from backend#1992, so instead:

• nodeAgentsInUse keeps its meaning, narrowed in the docstring to POD-BEARING tenants — which is what its three remaining consumers actually ask (DaemonSet-managing RBAC, and the pull Secret those pods use).
• node-agents-namespace.yaml drops the tenant condition rather than extending it. With a pod-less occupant now always present, the namespace is unconditional and a predicate over it would be a constant true wearing a question mark. Its other two conditions (namespace.create, release-namespace collision) are unchanged and both still bite — pinned by a test.

Same outcome the ticket wanted; no unrelated grant.

The test — the deliverable, not an extra

scripts/tests/telemetry-token-bootstrap.sh, registered in DRIFT_GUARDS (now 16). It asserts the composed property, because a fix that changes a template and tests that template repeats the mistake exactly.

It renders the whole chart with the Collector off and again with it on, projects the bootstrap surface out of both — the writer's env keyed by container, the Role that grants it secrets, the binding that points at that Role, whether the target namespace is created — and asserts the two are identical, while the Collector's own workload stays absent from the off render (so the equality can never be satisfied by shipping a DaemonSet to every node by default).

• Derives, never restates. No Secret name, namespace, key, verb or env var is written down. The coordinates come out of jobs-manager's own environment and every other selection is chased through references from there.
• Fails closed. An empty projection compares equal to an empty projection, so "found nothing" is its own finding.
• Both tenant columns. resourceMonitor defaults to true, so the default alone would pass with the namespace gate still in place.

Mutation results — each of the three re-gated in turn

Anchors asserted applied every time (byte deltas printed), so no mutation was inert:

mutation	guard	helm unittest
re-gate the jobs-manager env	RED — env differs, both columns	RED — 1 failed
re-gate the token Role/RoleBinding	RED — roles differs, both columns	RED — 4 failed
re-gate the namespace	RED — namespace_created differs, resourceMonitor=false column	RED — 1 failed

The namespace mutation reddens only the resourceMonitor=false column — which is precisely why the guard runs both.

helm-unittest changes are inversions of assertions the fix makes false, not additions on top of them; jobs_manager_test.yaml's env[0] positional assertion is switched to select CLIENT_ID by name, since the token block now renders above it.

Verification

helm unittest client   545 passed, 34 suites   (542 on develop)
make drift             all 16 guards green
shellcheck -S warning -x scripts/tests/telemetry-token-bootstrap.sh   clean
gitleaks               clean on the new file (2 pre-existing findings in docs/aks.md, 2024)
helm lint client       0 failed

Chart version and appVersion both bumped to 1.9.65.

The operator sequence this ships

Once 1.9.65 is published, on a clean edge:

1. Nothing. The hourly auto-upgrade pulls the chart; jobs-manager comes up with the coordinates and the RBAC, and writes tracebloc-telemetry-token into tracebloc-node-agents on its next authentication.
2. helm upgrade tracebloc tracebloc/client -n <ns> --reuse-values --set telemetryCollector.enabled=true — one step, and the fail guard now passes because its precondition is satisfied.

A fresh install with telemetryCollector.enabled=true from the start is still refused, correctly: jobs-manager has not run yet, so nothing has written the Secret. Install with it off, let it authenticate once, then flip.

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes Helm RBAC and namespace creation so token Role/RoleBinding and auto-upgrade reach into node-agents even with all pod tenants off. Least-privilege Secret write is unchanged, but every edge now gets extra objects and an unused token Secret write path.

Overview
Fixes a fleet-wide deadlock: the Collector DaemonSet refuses install until its token Secret exists, but jobs-manager only received Secret coordinates and write RBAC when telemetryCollector.enabled was already true.

Bootstrap is now independent of the flag. TELEMETRY_TOKEN_SECRET_* always lands on the jobs-manager writer container, telemetry-token-rbac.yaml always grants create/get/patch on that Secret, and the node-agents Namespace is created whenever namespace.create is set (still skipping a release-namespace collision). The DaemonSet fail guard is unchanged; the Collector workload still does not render when the flag is off.

tracebloc.nodeAgentsInUse stays pod-bearing only so image-refresh and pull Secrets are not granted in an empty namespace. Auto-upgrade’s node-agents Role/RoleBinding instead follows “the chart puts anything there,” so hourly --atomic ticks cannot 403 on the always-rendered token RBAC.

A new drift guard telemetry-token-bootstrap.sh compares off vs on renders of the bootstrap surface (env, Role, binding, namespace) and requires the Collector workload to stay absent when disabled. Chart version 1.9.65.

^{Reviewed by Cursor Bugbot for commit abe4d79. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit d238495. Configure here.}

[saqlainsyed007]

Verified the deadlock and the fix against the chart — a real fleet-wide bug, fixed at the right layer.

The deadlock is real and both halves were individually correct (which is why both passed review and nothing caught the pair): telemetryCollector.enabled=false → jobs-manager gets no TELEMETRY_TOKEN_SECRET_* env → writes no Secret → the daemonset's fail-guard refuses enabled=true while the Secret is absent → stuck at false, on every edge auto-upgrading to client-1.9.63 hourly.

The fix breaks it at the precondition, correctly: the token env, the write-RBAC (Role/RoleBinding jobs-manager needs, or it 403s), and the node-agents-namespace Secret now render unconditionally — the Secret is the flag's precondition, so it must exist before anyone can flip the flag. The daemonset fail-guard keeps its full strength (still refuses release with no Secret), and the Collector mounts the Secret optional: true, so an edge with telemetry off just carries one unused Secret and one idempotent re-auth write — the cost, stated rather than hidden.

The safety implications are handled: making the token Role/RoleBinding render with no pods in the node-agents namespace falsified the old "renders exactly when a pod runs there" claim, so node-agents-namespace-safety.sh is updated to assert the implication that actually holds now, and a new telemetry-token-bootstrap.sh guard is added to DRIFT_GUARDS to pin the unconditional rendering — the pair that was previously untested. CI green (helm tests + all quality/drift guards), no threads. LGTM.