feat(chart): fullnameOverride, with the completeness guard that makes it safe (backend#2626)

[LukasWodka]

Why the guard is the deliverable

backend#2621 built this helper, proved the default render byte-identical, and reverted it. The helper is the easy half. The release name appears ~174 times across these templates and is at least six different things, only two of which may follow an override — and a partial routing produces prod-auto-upgrade beside myrel-jobs-manager, which is harder to reason about than a release that is merely badly named.

What moved — including two the ticket didn't list

39 sites across 15 files: names of resources this chart creates, plus two things that reference those names.

• image-refresh-cronjob.yaml DEPLOYMENT_NAME. It names a Deployment this chart creates. Left behind, kubectl set image targets a workload that no longer exists.
• telemetry-collector-configmap.yaml's filelog glob — {ns}_{release}-*/{container}/*.log. It matches pod directories, and pods are named after the DaemonSet. Route the DaemonSet without the glob and the Collector runs, reports healthy, and collects nothing — the exact failure that file already warns about.

What stayed

app.kubernetes.io/instance (Helm convention) · 27 meta.helm.sh/release-name annotations (Helm's bookkeeping) · 3 RELEASE_NAME/RELEASE envs (a Helm identity — helm status, helm rollback) · 4 on-disk paths (a location: renaming orphans a tenant's data rather than moving it).

The ticket predicted my bug and I wrote it anyway

"a sed over .Release.Name catches it on the first pass; mine did."

My first routing pass anchored the env-var exception with $ against a concatenated context string, so it never matched — and the RELEASE_NAME env was routed. backend#2620 re-introduced by the fix for backend#2621, precisely as written. Two more followed:

• $.Release.Name substituted as a plain string clipped $.Release.Namespace to …$)space
• routing ran over the helper's own body, so tracebloc.fullname called itself until helm died

The third is now structural rather than guarded: the helper is inserted after routing, so its body is never a candidate. No exclusion rule to keep in sync.

The guard

scripts/tests/fullname-override-completeness.sh + its assertions module, over every profile in client/ci/*-values.yaml:

1. NO-OP	override == release name renders identically to unset
1b. VERBATIM	with the override unset, resource names carry the release name whole
2. MOVED	with a distinctive override, no resource name still carries the release name — misses reported by name
3. STAYED	every exception still carries it, with the right predicate per class

2 and 3 are a pair: without 3, assertion 2 is satisfied by renaming Helm's own bookkeeping.

Profiles matter, and one render hid it. bm-values.yaml sets hostPath.enabled; the hostPath PVs and the dataset directory exist in no other profile. A single ad-hoc render checked one of the four release-scoped paths and called itself satisfied.

Assertion 1b exists because a mutation survived. Assertion 1 diffs two renders that both pass through the helper, so | trunc N cancels on both sides and is structurally invisible to it — and the mutation was inert anyway for a three-letter release name. The guard's release name is now 38 characters, and 1b reads the default render directly.

Non-determinism is measured, not listed. secrets.yaml mints credentials with randAlphaNum, so the guard renders twice with identical inputs and excludes whatever differs. A hand-kept key list goes stale — and the template's variable names don't even map to the rendered keys ($podTokenSecret renders as POD_TOKEN_SIGNING_SECRET).

Wired into DRIFT_GUARDS, which the required Source-of-truth drift job runs. A guard in a non-required job is advice.

Test plan

• helm unittest ./client at helm 3.15.4 (CI's pin, not my local 4.x) → 36 suites, 631 tests, OK
• 18 name-sensitive shell gates → all pass
• helm lint, shellcheck -S warning -x → clean
• The guard across 4 profiles → 32 assertions OK
• Mutations, each anchor asserted to have applied:

mutation	caught by
route the RELEASE_NAME env	helm-identity assertion
un-route one resource name	MOVED, by name
route the dataset directory	on-disk path assertion
trunc 20 on the default	VERBATIM
upper on the default	VERBATIM
restored	green

Scope note

Per the ticket, this is not urgent and deliberately unhurried — the self-service installer already names releases consistently, so it serves the small hand-managed fleet. Nothing changes for anyone until fullnameOverride is actually set; unset is byte-identical, and that is asserted rather than asserted-in-prose.

Closes tracebloc/backend#2626.

🤖 Generated with Claude Code

---

Note

High Risk
Changes naming and upgrade paths for secrets, MySQL credentials, and hostPath storage; mitigated by template-time refusals and drift guards, but mistaken fullnameOverride on a live release can still strand PVCs/PVs or break installer health checks until backend#2888.

Overview
Introduces fullnameOverride and a tracebloc.fullname helper so chart-created Kubernetes names (Deployments, Secrets, CronJobs, hostPath PV objects, telemetry resources, etc.) can be prefixed independently of the Helm release, while Helm identity (app.kubernetes.io/instance, RELEASE_NAME envs, on-disk host paths) stays on .Release.Name. Dozens of templates and helpers switch from .Release.Name to the helper, including cross-reference sites like image-refresh’s DEPLOYMENT_NAME and the telemetry Collector filelog glob.

Safety on live clusters: secrets.yaml now fails at template time when retained mysql-pvc exists but no Secret exists at the effective name (blocks silent credential re-mint on rename/reinstall), with extra messaging for hostPath PV rename vs bound PVCs; post-install NOTES warn about hostPath PV naming. Telemetry token presence also accepts the pre-override <release>-telemetry-token Secret so renamed releases don’t false-fail when the Collector is enabled.

Enforcement & ops: New drift guard fullname-override-completeness.sh (plus Python assertions) verifies no-op default, full routing, and exceptions; chart tests cover hostPath PV/PVC asymmetry. Bash/PowerShell detect_installed_client reads credentials from <fullnameOverride>-secrets when set. MIGRATIONS.md documents backing up the Secret before uninstall. Chart version 1.9.92 → 1.9.93.

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

[LukasWodka]

bugbot run

[saadqbal]

Good PR, and the framing is the best part — treating the release name as six different things and deciding per class which may follow the override is right, as is rendering every client/ci profile in the guard. Upgrade safety checks out too: tracebloc.fullname is default .Release.Name .Values.fullnameOverride, so an absent key resolves to the release name and --reuse-values is fine, and every selector either is release-independent or renames together with its own object, so there's no immutable-field failure waiting.

Three things before it goes in, and the first two are the same problem — the guard doesn't cover the sites you singled out.

MOVED only reads doc-root metadata.name. Un-routing DEPLOYMENT_NAME and the Collector filelog glob back to .Release.Name renders both broken — DEPLOYMENT_NAME: relname…-jobs-manager against an actual Deployment named zzoverride-jobs-manager — and the guard stays green on all four profiles. Those are precisely the two sites the description calls out as the ones a metadata.name sweep misses, so right now nothing catches them. Walk every string scalar and allowlist the exception classes STAYED already enumerates.

NOTES.txt is still un-routed in three places: {{ .Release.Name }}-jobs-manager (L6), tracebloc-resource-monitor-{{ .Release.Name }} (L24), {{ .Release.Name }}-auto-upgrade (L27) — while L9, L13, L14 and L21 in the same block go through the helpers correctly. The sharpest version of this is seven lines apart: L6 prints rel-jobs-manager and L13 prints zzoverride-jobs-manager, in the same block, under one override. That's the half-routed render your own rationale calls worse than a badly-named release, in the first thing anyone sees after install. helm template doesn't emit NOTES, so the guard structurally can't see it — worth rendering it there too.

Bugbot's PyYAML point is right and slightly worse than it reads: with PyYAML stubbed the guard prints a traceback and then [ERROR] fullnameOverride is incomplete in 4 profile check(s), so a missing dependency reports as a chart defect and sends the reader hunting un-routed names that don't exist. And pyyaml-preflight.bats filters to .sh/.bats, so this is the first .py sidecar to escape the class rule entirely — add .py to that filter while you're in there, or every future sidecar gets the same free pass.

Minor: the schema declares fullnameOverride as a bare string, so --set fullnameOverride='Bad_Name!' templates clean and then fails object-by-object at the API server mid-install. Helm validates the value this replaces as DNS-1123 ≤53 — the substitute should carry the same pattern/maxLength and fail at template time with a name.

[aptracebloc]

Reviewed independently — not adding a second gate, this is blocked on @saadqbal's change-request and I concur with all four of his points after walking the diff and both new guard files. A couple I can confirm are real rather than stylistic:

• The guard's coverage gap is the important one. The MOVED assertion only scans doc-root metadata.name, but the two sites the PR itself flags as subtle — image-refresh DEPLOYMENT_NAME (an env value) and the telemetry filelog glob (a ConfigMap data string) — are correctly routed today yet the guard can't defend them, so a regression that un-routed either renders broken while the guard stays green on all four profiles. Walking every string scalar with an allowlist for the STAYED classes closes it.
• PyYAML preflight is slightly worse than the thread reads: with PyYAML absent the sidecar raises a bare ModuleNotFoundError, the shell else-branch increments failures, and it prints [ERROR] fullnameOverride is incomplete … — a missing dependency misreported as a chart defect. And this is the first .py sidecar, which pyyaml-preflight.bats (filtered to .sh/.bats) doesn't cover.

NOTES.txt L6/L24/L27 still render .Release.Name-based names while L9/L13 use the helpers (and helm template won't emit NOTES, so the guard can't see it), and the schema declares fullnameOverride as a bare string without the DNS-1123 pattern/maxLength Helm enforces on the release name it substitutes for.

Framing and upgrade-safety are strong — the six-class split, rendering every client/ci profile, and the verbatim-default assertion are the right shape. Deferring the verdict to @saadqbal; once his four are addressed this looks close.

— drafted with Claude Code

[LukasWodka]

All four points taken. The first two were the same defect and you were right that it was the important one.

1 + 2 — the guard did not cover the sites this PR singled out

MOVED read doc-root metadata.name only, while STAYED enumerated the exception classes separately. So the two halves disagreed about what the exceptions were, and every name-reference site fell through the gap.

Now one classifier, two callers: classify() walks every string scalar and labels each release-name hit with the exception that licenses it, or None. MOVED is "nothing unlicensed"; STAYED is "every class non-empty and correct". Adding a class cannot weaken MOVED without also adding an obligation to STAYED — which is the property that was missing, rather than a longer list.

Scale of the change: 1,339–1,422 string scalars per profile, against a handful of metadata.name before.

Mutation-proved on exactly the two sites you named:

mutation	before	now
DEPLOYMENT_NAME un-routed to .Release.Name	green	1 unrouted site, all 4 profiles
Collector filelog glob un-routed	green	1 unrouted site, all 4 profiles

3 — NOTES routed, and checked

L6, L24 and L27 routed. Your sharpest version of it is the one I'd have wanted to read first: L6 printing rel-jobs-manager seven lines above L13 printing zzoverride-jobs-manager, under one override.

Assertion 4 now checks it, and getting a render was the awkward part. Measured on the CI-pinned v3.15.4:

helm template …                                  omits NOTES entirely
helm template … --show-only templates/NOTES.txt  "could not find template"
helm install … --dry-run                         "Kubernetes cluster unreachable"
helm install … --dry-run=client                  ALSO needs a cluster on 3.15.4 —
                                                 and WITH a reachable one it failed
                                                 ownership validation against real
                                                 objects, so the guard's verdict
                                                 would depend on whose kubeconfig
                                                 ran it

So it renders a copy of the chart in which NOTES.txt is an ordinary template: real engine, real values, no cluster, same answer on a laptop and on CI. --debug is required because helm refuses to emit output it cannot parse as YAML and NOTES carries ANSI escapes; KUBECONFIG=/dev/null makes the hermeticity a property of the command rather than of the machine. helm exits 1 on that path even while printing the render, so emptiness is the failure signal, not the exit code — noted in the script, because under set -euo pipefail the non-zero exit silently killed the guard after the first profile while I was building it.

4 — PyYAML, and the .py free pass

Named refusal with exit 2, distinct from the 1 that means "the chart is incomplete". Your reading of the severity was right: it printed a traceback and then [ERROR] fullnameOverride is incomplete in 4 profile check(s), so a missing dependency reported itself as a chart defect.

.py added to pyyaml-preflight.bats's filter. extract_python finds python embedded in shell, so a standalone sidecar has no heredoc to find and the filter never opened the file — the first .py in this tree escaped the class rule entirely, and every future one would have. Proven as a pair:

• .py in the filter + guard removed from the sidecar → not ok 1 … does not guard it
• filter reverted + same break → green, which is the free pass you described

5 — the schema

DNS-1123 pattern + maxLength: 53, the same constraints Helm enforces on the value this replaces — so it was a strictly weaker gate than the thing it substitutes for. Verified:

--set fullnameOverride=Bad_Name   → Does not match pattern '^[a-z0-9]([-a-z0-9]*[a-z0-9])?$'
--set fullnameOverride=<54 chars> → String length must be less than or equal to 53
--set fullnameOverride=zzoverride → renders
unset                             → renders

Two weaknesses in my own fix, found by mutation-proving

Worth flagging rather than burying, since both are the class this guard is about:

1. A broadened path allowlist swallowed unrouted sites out of MOVED. CLS_PATH was the only class a non-path could satisfy by accident — the other three demand the value equal a release identity, while "did not follow the override" is true of any unrouted name. Returning CLS_PATH for everything left the guard green. It now asserts its members actually are paths, and the combined mutation reddens.
2. Assertion 4's "no NOTES supplied" refusal was unreachable, so deleting it changed nothing. There is now a self-check that invokes the assertions without the NOTES arguments and requires a non-zero exit — otherwise a future caller could drop the NOTES render and get a green guard that checked three things while documenting four.

Verification

• make drift — 35/35 guards green, all four profiles
• CI=true bats scripts/tests/*.bats — 1568 passing, 0 failures
• shellcheck -S warning -x clean; helm v3.15.4 (the CI pin), not the 4.x on my PATH
• Eight mutations, each anchor asserted applied

[LukasWodka]

bugbot run

[aptracebloc]

Re-reviewed the new commit (6aecef53) against the four points from the last round — all four are addressed, and correctly:

1. Guard coverage gap — fixed, and mutation-proof. classify() now walks every string scalar with an allowlist for the STAYED exceptions, so an un-routed DEPLOYMENT_NAME (not in RELEASE_ENV) or filelog glob (a ConfigMap data blob, not a .path key — CLS_PATH is kept narrow on purpose) falls to None and reddens MOVED. Fail-closed vacuity guards and the "refuse to run without NOTES" self-check are the right shape.
2. NOTES — routed and now checked. L6/SCC/auto-upgrade use the helpers; MySQL Host correctly stays mysql-client. The 4th assertion renders NOTES out-of-band (helm template omits it) so this can't regress silently again.
3. PyYAML preflight — fixed. Named refusal + SystemExit(2) distinct from the incomplete-chart exit 1, and pyyaml-preflight.bats now covers .py sidecars.
4. Schema — fixed (DNS-1123 pattern + maxLength: 53).

Two things still open, neither mine to clear:

• [blocking] bugbot / review is red on a new HIGH: routing tracebloc.secretName through fullnameOverride (_helpers.tpl, pre-existing from 90995f3a) means a rename makes secrets.yaml's existing-Secret lookup miss, so the chart mints fresh credentials while the kept MySQL PVC still holds the old DB — jobs-manager then can't authenticate (login refused / minter CrashLoop). The completeness guard structurally can't catch this: classify() has no exception class for lookup-keyed credential Secrets, so it actually requires the Secret to follow the override. Worth deciding whether the release-Secret belongs in the STAYED set (or whether the lookup should also probe the old name on rename) before this merges.
• [nit] fullnameOverride: "" is now schema-invalid. The pattern requires ≥1 char, but values.yaml documents # fullnameOverride: "" as the example and the template's default .Release.Name .Values.fullnameOverride treats "" as unset — uncommenting the example verbatim aborts the install on a regex mismatch.

Not clearing my end — @saadqbal's change-request is the standing gate and CI is red. Deferring to him + a green run.

— drafted with Claude Code

[LukasWodka]

bugbot run

[saadqbal]

All four of my findings are genuinely fixed, and I checked them by mutation rather than by reading the commit messages. The guard now walks every scalar through a shared classifier — un-routing DEPLOYMENT_NAME reddens naming env[3].value, un-routing the Collector glob reddens naming ConfigMap/.data.config.yaml, and collapsing classify() to return CLS_PATH for everything still reddens via the not-a-path backstop, so the allowlist can't swallow what it was widened to catch. NOTES is rendered and asserted, and un-routing L6 reddens and prints the line. PyYAML absent now exits 2 with the named refusal and the shell says "NOT a verdict on the chart". The schema carries the DNS-1123 pattern and maxLength 53. The notes-probe render is a nice piece of work.

Holding on the rename refusal. A hard fail is the right shape, and I like that it derives the old name from .Release.Name instead of asking the operator for it — the failure mode I was most worried about, a fix that only works when someone hands over the old name, didn't land. But it only covers unset→set. Change the override from one value to another, or drop it from a release that had one, and the lookup misses exactly the same way: four credentials re-mint against the kept MySQL PVC and the upgrade reports deployed. Bugbot has that open as a High on secrets.yaml:43, and I'd derived it from the template independently before seeing the finding.

Rather than guessing one old name, enumerate the namespace and refuse when a Secret this release owns exists under a name that isn't $secretName. app.kubernetes.io/instance still carries the release name by design — it's one of your declared exception classes — so it's a reliable filter, and it covers all three directions for about the same amount of code.

Worth saying why the new assertions stayed green through this: they grep the source, so they're mutation-proof against the text being removed but blind to a scope error that leaves the text intact. That's the limit of that tier rather than a fault in it — putting them there is consistent with how the file treats the other lookup-dependent tiers — but it is why the scope slipped, and it's the same shape as the guard findings we just closed.

Two bits of prose the fix falsified:

values.yaml:1098 still says the chart "ALWAYS emits <release>-secrets", and the pre-create commands below it use that name. With an override set, an operator following them on a genuinely fresh install creates a Secret the chart doesn't read AND trips the new refusal — with remedy text telling them to back up and reinstall a release that was never installed.

values.yaml:492 and the schema description both call changing it on an existing install "a migration, not a config tweak". As of this commit it's refused outright. Worth saying so where operators actually read it.

One small one: fullname-override-completeness.sh never checks command -v python3, where ten sibling guards in scripts/tests do. With python3 missing it prints "fullnameOverride is incomplete in 4 profile check(s)" — the same misdiagnosis we just fixed, arriving from the interpreter half of the rule instead of the module half.

[aptracebloc]

Re-reviewed ea6568dc ("refuse a fullnameOverride rename of a LIVE release"). Asad's original four points remain correctly addressed. The new guard is the right shape — fail-closed, credential-Secret-specific, and it correctly does NOT fire on a fresh install-with-override or a no-op upgrade. But it closes only one of the three rename directions, so the secretName re-mint HIGH is not yet closed. Bugbot has independently re-raised this as a HIGH on this same commit.

The guard keys detection on the un-overridden name {{Release.Name}}-secrets, and the helper makes the Secret name follow the override (tracebloc.fullname = default .Release.Name .Values.fullnameOverride; secretName = {{fullname}}-secrets). So it only detects "installed with no override, then an override is added". The two other renames still re-mint against the kept mysql-pvc (fixed name) and lock the DB out:

• override A → override B. Live Secret is A-secrets. ne $secretName $unoverriddenSecretName is true, but lookup {{Release.Name}}-secrets misses (the live one is A-secrets), so no refusal; the top lookup B-secrets also misses → four randAlphaNum credentials re-minted → helm upgrade deployed, ERROR 1045 Access denied.
• drop the override. $secretName == $unoverriddenSecretName == {{Release.Name}}-secrets, so ne is false and the guard body is skipped entirely; the top lookup still misses the live A-secrets → same silent lockout.

The un-overridden-name key is a proxy for the thing that actually causes the lockout. The invariant is "persisted MySQL credentials exist but the Secret under the current effective name is absent". Keying the refusal off that (e.g. presence of the release's MySQL data alongside a missing current Secret) covers all three directions — and even a reinstall-with-override over a kept PVC after an uninstall, which name-keying also misses.

scripts/tests/client-credentials-have-a-secret-tier.sh assertion 2 currently asserts the guard keys on .Release.Name and calls that "the whole correctness of it" — that assertion cements the incomplete design and will need to change with the fix; no test exercises the A→B or drop directions.

Deferring final clearance to @saadqbal and a green gate.

— drafted with Claude Code

[LukasWodka]

@aptracebloc — your invariant is now the guard. Detail in the thread reply on secrets.yaml; the short version:

The refusal is keyed on persisted MySQL data present + Secret under the current effective name absent, not on any release-derived name. mysql-pvc is a constant with resource-policy: keep, so its presence is "there is a database here with credentials already baked in". All three rename directions plus reinstall-over-a-kept-PVC fall out of that; ordinary upgrades and fresh installs (with or without an override) do not fire.

You were also right that assertion 2 cemented the incomplete design — it passed while two thirds of the class was open. Assertions 2 and 3 now pin the state probe and the absent-Secret gate, and assertion 4 forbids the name key outright, because re-introducing it would reopen both directions with every other assertion still green. Floor 16 → 17.

Two things I verified rather than assumed, since the probe is new: the PVC is unconditional (only the PV is hostPath-gated), and the in-cluster upgrader's namespaced Role is */*/* — worth checking because a forbidden lookup raises rather than returning empty, which would have failed every auto-upgrade.

The remedy also got cheaper and now leads the message: copy the Secret to the new effective name and nothing else has to move, since the PVC name never follows the override.

One caveat I want on the record before clearance: I have not re-run the k3d --dry-run=server matrix against this version. The case table is derived from the two lookups, not measured, and the A→B and drop rows are the ones that deserve a live check. Happy to run it if you'd rather not clear it on derivation.

5 mutations all reddening with anchors asserted applied, make check green, completeness guard green, 631 chart tests, 17/17 assertions.

[LukasWodka]

bugbot run

[LukasWodka]

Bugbot's High is fixed — by the co-driver's change, not mine. Worth saying plainly, because I wrote a competing fix and threw it away.

What I had, and why it lost

I un-routed tracebloc.secretName so a rename could not lose the credentials — reasoning from the chart's own precedent that tracebloc.mysqlPvc is the fixed name mysql-pvc, so the stateful half already doesn't follow the override.

It was untested, and untestable by me: lookup is inert under helm template, --dry-run and every other client-side renderer I have. ea6568d refused the dangerous case instead and measured it against a live cluster with --dry-run=server — the only way to exercise a lookup at all. Then fae5441 re-keyed that refusal on the persisted data after Arturo showed name-keying caught one of three rename directions and missed reinstall-over-a-kept-PVC entirely.

The two designs are incompatible, not additive: with the Secret un-routed, the refusal would compare a name against itself and fire on every override-set upgrade. So mine is discarded rather than layered on.

What I kept: the class

The instance is fixed and client-credentials-have-a-secret-tier.sh holds it. What nothing covered is the class — and this chart has two members:

site	mitigation	asserted by
secrets.yaml	a fail reached by the miss	client-credentials-have-a-secret-tier.sh (fae5441)
tracebloc.telemetryTokenPresent	safe only because it ORs a lookup on the legacy fixed name	nothing

The second was one edit from the same silent shape with no check. Assertion 5 now requires every routed Secret lookup to carry one of the two mitigations in its own file. It does not second-guess which — it's a text-level check and could not have established what --dry-run=server did.

The detector keys on the invariant, not the arithmetic — and that's a lesson from this PR

My first version matched printf "%s-secrets" .Release.Name: the shape the refusal had while it keyed on the un-overridden name. fae5441 improved the refusal, and my detector would have reported it as no refusal at all.

A detector that breaks when the thing it guards gets better is worse than none — it pushes back toward the shape it was written against. It now keys on the property that must hold however the other half is computed: a miss on that lookup reaches a fail.

Which names follow the override is derived transitively by closing include over the helper definitions, so a helper added tomorrow that wraps fullname is covered without anyone remembering. Fails closed three ways: a define parser matching nothing, zero routed sites in a chart known to have two, and zero Secret lookups at all.

Mutation-proved four ways, each anchor asserted applied:

mutation	result
the data-keyed refusal removed	flagged
the refusal no longer negates that lookup	flagged
telemetry legacy-name fallback removed	flagged — the previously unguarded member
routed-var detection stops resolving	fail closed

Verification

On helm v3.15.4 (the CI pin), not the 4.x on my PATH:

• make drift — 35/35 guards green
• helm unittest client — 631/631 in 36 suites
• client-credentials-have-a-secret-tier.sh — 17 assertions OK
• shellcheck -S warning -x clean

No chart files touched, so no version bump.

---

Note on co-driving: two sessions were on this branch. I hit a non-fast-forward, read the other side's fix before writing over it, and rebased onto it — which is how I found out theirs was better. Flagging it so the interleaved commits aren't confusing to read.

[LukasWodka]

bugbot run

[aptracebloc]

Re-review at ddbc4310.

Withdrawing my original block — verified resolved. My HIGH was the secretName re-mint (the rename refusal only closed one of three directions). It's now re-keyed off persisted state (and $mysqlDataPresent (not $existingSecret)); I walked all three directions plus reinstall-over-a-kept-PVC and it holds — tracebloc.mysqlPvc stays the constant mysql-pvc and secretName still follows fullname. That concern is closed for me.

But I'm keeping changes-requested, re-pointed to @saadqbal's denominator finding, which I independently confirmed against the code. The completeness guard renders the four client/ci/*-values.yaml profiles at their defaults, and two name families render in none of them, so the guard walks straight past them:

• egress-enforcement-check.yaml renders only under and networkPolicy.training.enabled (not allowExternalHttps) enforcementProbeHost. No profile sets allowExternalHttps=false — aks/bm/oc leave it defaulting to true, eks has training.enabled=false — so it renders in 0/4 profiles. Its {{ include "tracebloc.fullname" . }}-egress-enforcement-check at :20 is a routed site the guard never sees.
• the registry-Secret family needs dockerRegistry.create=true; the profiles set only username/password/email, never create, so the imagePullSecrets[0].name sites (~13) plus the Secret and Job names render in 0/4 too.

Net: ~15 routed sites sit outside the walk, so the guard's "fullnameOverride routes every resource name" is vacuously true for them — revert the :20 hunk to .Release.Name and it still exits 0. For a completeness guard that's the one failure mode that matters: it passes while under-covering.

Saad's fix is the right shape and keeps the baseline at 0 (the chart is correct at all 15 sites today, so this is a missing tripwire, not a defect) — add to render():
--set networkPolicy.training.enabled=true --set networkPolicy.training.allowExternalHttps=false --set dockerRegistry.create=true --set dockerRegistry.server=https://index.docker.io/v1/ (the server isn't optional — values.schema.json refuses before helm renders without it). Then un-routing any of those sites reddens and names it, same as the jobs-manager control does today.

Everything else I checked is settled: my earlier pathclass/interpreter-loop guard points are fixed by mutation, and CI is green (the rollup FAILURE is a superseded cancelled Helm unit tests run).

— drafted with Claude Code

[saadqbal]

Nothing moved since my last round — head is still ddbc431, which predates that review — so
the denominator blocker stands as written. I re-measured rather than assume: revert
egress-enforcement-check.yaml:20 to .Release.Name and the guard exits 0, 56 OK / 0 ERROR,
still printing "fullnameOverride routes every resource name". Un-route
tracebloc.registrySecretName and the same, silently. Control still holds — un-routing
jobs-manager exits 1 and names the site. Grep says why: no match for allowExternalHttps,
dockerRegistry, or networkPolicy.training anywhere in either guard file.

I went ahead and proved the fix instead of just repeating it. With the four --sets in,
baseline stays 0 — the chart is right at all 15 sites today, so this is a missing tripwire and
not a defect — and both mutations then exit 1 and name the sites, mutation 2 naming all four
imagePullSecrets[0].name holders. One thing I got wrong by omission last round: the gates
have to go into render_notes() as well, or the NOTES probe renders a different chart than
the assertions walk, which would half-fix it and read green.

New since last round: this now conflicts with develop on Makefile and
scripts/manifest.sha256. Please regenerate the manifest from the merged tree rather than
taking a side — it's a checksum over scripts/, and a wholesale --ours/--theirs there
produces something internally well-formed that matches only one side's tree. The drift check
would catch it, but the natural response to a red drift check is to regenerate blindly, which
launders whichever side got dropped.

@aptracebloc — thanks for walking the three rename directions independently, and for
withdrawing your own HIGH rather than leaving it parked. Noting for whoever picks this up:
you kept changes-requested, so once the gates land you'll need to submit an actual approval; a
COMMENTED won't dismiss it.

[LukasWodka]

The denominator hold is closed, and by exactly the fix you both named.

render() now forces the two keys that make the missing families render into all four profiles:

--set networkPolicy.training.enabled=true
--set networkPolicy.training.allowExternalHttps=false
--set dockerRegistry.create=true   (+ server/username/password/email, which the schema requires once create=true)

So egress-enforcement-check.yaml and the registry-Secret family (~15 routed sites — one Job name, one Secret name, thirteen imagePullSecrets[0].name refs) now enter the walk on every profile. Set on all renders (a/b/explicit/override) so the no-op diff still compares like with like.

Baseline stays 0, as you predicted — the chart routes all 15 correctly today, so this is a missing tripwire, not a defect. And the tripwire is proven, not asserted: reverting egress-enforcement-check.yaml:20 back to .Release.Name now reddens and names the site —

[ERROR] 1 value(s) still carry the release name … under fullnameOverride='zzoverride' …
          Job/…-egress-enforcement-check  .metadata.name = '…-egress-enforcement-check'

— where before it exited 0. The jobs-manager control still reddens too, so it's the denominator and not a failed sed.

Rebased onto develop (the DRIFT_GUARDS conflict resolved to keep both hostpath-reads-guarded.sh and fullname-override-completeness.sh; Chart → 1.9.92; manifest regenerated). make check green.

@aptracebloc — your withdrawn HIGH (the secretName re-mint) stays closed; this only touches the guard's denominator. @saadqbal — this is the three --sets you specified.

[LukasWodka]

bugbot run

[aptracebloc]

Re-reviewed at 3cb76d30. My denominator concern is resolved — clearing my block on that basis.

render() now forces the two families that rendered in none of the four CI profiles at defaults: --set networkPolicy.training.enabled=true --set networkPolicy.training.allowExternalHttps=false brings in egress-enforcement-check.yaml, and --set dockerRegistry.create=true --set dockerRegistry.server=… brings in docker-registry-secret.yaml and every imagePullSecrets reference. Verified by rendering all four profiles: the walk now covers 63–66 documents per profile (was ~48), the guard stays at baseline 0 across aks/bm/eks/oc, and the mutation control bites — un-routing tracebloc.registrySecretName reddens the guard naming 14 previously-uncovered sites (the imagePullSecrets refs plus jobs-manager env[15]). That was the vacuous-pass gap; it's closed.

Not approving yet — this is a gate call, not a concern with the code: the branch is in merge conflict, there's still one open Bugbot thread (Medium, "ENV emptiness still fails per profile" on fullname_override_assertions.py), and @saadqbal's change request is standing. Deferring merge-readiness to @saadqbal.

— drafted with Claude Code

[saadqbal]

Denominator blocker is closed — and closed by measurement, not by argument. render() now forces
both families, the oc walk goes 48 -> 66 documents, baseline holds at 56 OK / 0 ERROR, and both
previously-vacuous mutations now exit 1 and name their sites: reverting
egress-enforcement-check.yaml:20 names the Job on all four profiles, and un-routing
tracebloc.registrySecretName names 14 sites including jobs-manager env[15] and the regcred
Secret. Control still reddens. registry.example.com instead of the docker.io value is fine —
the schema accepts it.

I was wrong about render_notes(), and it's worth saying so: I said the gates had to go there too
or it would half-fix while reading green. On this chart it doesn't matter — NOTES.txt references
none of those keys and the gated and ungated renders are byte-identical on all four profiles. It's
a latent-drift consistency nit, not the blocker I implied.

Three things still hold it, none of them the guard:

The branch conflicts, still on Makefile and scripts/manifest.sha256, and develop has moved
again. The Makefile half is the one to be careful with: develop's #933 reformatted DRIFT_GUARDS
to one entry per line and added the empty-entry refusal, while this branch appends to the old
single line. --ours deletes #933's refusal and the reformat; --theirs drops this guard from the
list entirely — a required check that stops existing while every run reports green, which is
precisely what #933's own comment was written to warn about. Take develop's block and add one line.
For the manifest, regenerate from the merged tree with gen-manifest.sh — develop changed
install-client-helm.sh's hash, this branch changed summary.sh and install-k8s.ps1, so they're
disjoint and a side pick yields a well-formed checksum list matching only one tree. --check is
clean at this head, so it's purely a merge artefact.

There's a valid unresolved Bugbot Medium on this head (fullname_override_assertions.py:690), and
I reproduced it rather than relaying it: assert_stayed's empty-class arm demotes only CLS_PATH,
so for CLS_ENV it prints [ERROR] found NO RELEASE_NAME / RELEASE / RELEASE_NAMESPACE env and
sets fail — while three lines later the same run prints that this emptiness is legitimate and
ENVCLASS 0. On aks with autoUpgrade, imageRefresh and sealCheck.storageAssertions all off:
41 documents, 0 identity envs, sidecar exit 1, both messages in one output. Latent and fail-closed
today since all four CI profiles name 5 — but it's a required drift guard refusing a complete
chart. Fix is symmetry: extend the if cls is CLS_PATH demotion to CLS_ENV.

And the assertion-5 fixture is untouched — _mitigations returning {"refusal"} unconditionally
still exits 0 with both selftests green and assertion 5 still printing "all 2 routed Secret
lookup(s) of 2 carry a mitigation". Same for the :308 nit, which now contradicts the case block
34 lines above it in its own file.

@aptracebloc — your 07:50 note says you're clearing your block, but it went in as COMMENTED, so
latestOpinionatedReviews still returns your CHANGES_REQUESTED from 07:16. reviewDecision won't
move until you submit an actual approval.

[LukasWodka]

All three closed. The one that mattered most was your third, which nobody had flagged as blocking.

1. Conflict — resolved exactly as you specified

You were right that the Makefile half is the dangerous one, so I didn't pick a side. Compared the two guard sets programmatically: 39 on develop, 40 here, the single delta being fullname-override-completeness.sh. Took develop's block (keeping #933's reformat and its empty-entry refusal) and appended that one entry as its own line. drift-list-integrity.bats — the guard that polices the guard list — is 9/9.

manifest.sha256 regenerated from the merged tree, not side-picked. --check clean.

2. CLS_ENV — fixed, and the one-line version would have broken the cross-profile assertion

Reproduced before touching it: aks with autoUpgrade, imageRefresh and sealCheck.storageAssertions.enabled all off, 38 documents — the previous code prints [ERROR] found NO RELEASE_NAME … and ENVCLASS 0 in one output.

The trap in "extend the demotion to CLS_ENV": it must not also print ENVCLASS 0 the way CLS_PATH prints PATHCLASS 0. The shell parses head -1, and the identity-env block below prints the real count unconditionally — so a zero here would be read first and the true value discarded, silently turning the cross-profile assertion into one that always sees 0. CLS_PATH can print its count because its two branches are mutually exclusive within that loop; CLS_ENV's belongs to the block below. In the code now, with the reason.

Latent and unpinned, said plainly: all four CI profiles name 5, so disabling the demotion does not redden the suite. The evidence is the reproduction, not a guard. Pinning it needs a fifth profile or a specimen driving assert_stayed's empty arm — say the word and I'll add the specimen.

3. The refusal detector had no specimens at all

This is the finding. _fallback_on_line has three pinning lines; _mitigations had none — which is exactly why your mutation works. return {"refusal"} unconditionally left assertion 5 printing "all 2 routed Secret lookup(s) of 2 carry a mitigation", exit 0, both existing selftests green. The detector that decides whether a routed credential Secret is mitigated could be deleted and nothing in the tree would say so.

Four specimens now, written down rather than hunted for in the chart:

specimen	property
a real refusal (negation of the routed var reaching fail)	must read as refusal
a negation that merely branches	must not — this is the half your mutation fails
a fail reached by negating an unrelated variable	must not — the routed_vars discrimination
a fail below the 4-line window	must not — the window stays bounded

Mutations, all reddening:

mutation	result
your exact return {"refusal"}	3 of 4 halves fire
widen the window 4 → 40	reddens
drop the per-var discrimination	reddens

A fourth came back INERT and its anchor assertion said so rather than reporting a pass — worth mentioning because an inert mutation and real coverage look identical in a log.

The :308 nit

Not addressed in this push. You're right that it now contradicts the case block 34 lines above it in its own file; I'd rather fix it in a pass where I can re-read both together than bundle a prose change into this one. Tell me if you'd rather it landed here.

Evidence

Guard at head: rc=0, 60 OK, 0 ERROR, aks=5 bm=5 eks=5 oc=5 — your 56/0 baseline plus the four new selftest lines. gen-manifest.sh --check clean, drift-list-integrity.bats 9/9.

One thing I deliberately did not do: black reports 14 hunks on fullname_override_assertions.py — but it reports the same 14 on the pre-change file, none inside the added code, and this repo's CI does not run black at all. Reformatting would have buried the fix in an unrelated diff.

Noted on your last point: I can't move reviewDecision for @aptracebloc — that needs their own approval submission.

[LukasWodka]

bugbot run

[LukasWodka]

Bugbot High "Path class misses routed sites" — fixed in b8f5997. Recording the mutation result, because my first version of the fix failed the same way the finding describes.

release_scoped_path_sites now derives the domain from the render with the override UNSET and asserts every site it names is still scoped by the release name — the same shape identity_env_sites uses for envs. Adds a machine-readable PATHSITES count per profile and cross-profile assertion 8, on the same could_not_run vocabulary as 6 and 7 so renaming the marker cannot print NOT CHECKED and exit 0.

The mutation caught my own fix first. I wrote it calling classify() alone — but classify keys CLS_PATH on the doc-path shape (.hostPath.path, or .path with a / value) and never inspects whether the release name is in the value; that filter lives upstream in main()s loop. So it named every hostPath in both renders, the sets came out identical, and the difference was empty by construction. Routing logs-pvc.yaml through tracebloc.fullname left it printing [OK] all 5 and exiting 0.

With the release-scoping test on the value:

rc=1
[ERROR] 1 release-scoped path site(s) present with the override UNSET are no longer
        scoped by the release name under fullnameOverride='zzoverride' — routed away
          PersistentVolume/<NAME>-logs-pv  .spec.hostPath.path

Clean at head (rc=0) and non-vacuous: aks=1 bm=4 eks=1 oc=1. rel is used for both renders deliberately — testing for ovr would pass exactly when the path had been routed.

@saadqbal on your other three:

• Conflict — resolved. The Makefile auto-merged correctly this time: develops one-per-line block with our entry appended, 40 guards, and #933s empty-entry refusal intact (I diffed the guard list both ways against develop rather than eyeballing it). Manifest regenerated from the merged tree, not side-picked. Chart bumped to 1.9.93 — develop had moved to 1.9.92 since your review, so the branch`s existing bump was no longer above it.
• CLS_ENV demotion (:690) — already closed in 0b6fee5 before I got here.
• assertion-5 fixture — also closed in 0b6fee5, and I verified it by running your exact mutation rather than trusting the commit message: _mitigations returning {"refusal"} unconditionally now exits 1 with [ERROR] a negation that merely BRANCHES reads as a refusal on all four profiles.

make drift 40/40, helm unittest 656/656.

[LukasWodka]

bugbot run

[saadqbal]

All three of mine are closed, and I verified each rather than taking the commit message for it.

The CLS_ENV demotion: same aks case (autoUpgrade, imageRefresh, sealCheck.storageAssertions all
off), 41 documents, 0 identity envs — the parent exits 1 with the [ERROR] and the [note] in one
output, this head exits 0 with just the note. And you were right not to print ENVCLASS 0 in that
arm; head -1 would have eaten the real count.

The _mitigations specimens: return {"refusal"} as the first statement now exits 1 with three of
the four biting on all four profiles. Pinning the detector's property rather than the arity is
better than the synthetic source I suggested — the per-file-vs-per-site question stops mattering.

And release_scoped_path_sites is the right answer to Bugbot's routed-sites High: the symmetric
site-level enumeration, with a docstring that names it as the same blind spot one class over. That
is the fix I'd have asked for, already written.

Not approving, for two reasons that aren't your code.

29 checks are still pending at this head, so the gate isn't there yet — I'd rather wait than read
the previous head's green as this one's.

And two of the three Bugbot Highs on this PR were resolved by cursor[bot] itself, not by you
— "Path class misses routed sites" at 08:25 and "Path class fails on cloud profiles" at 11:07,
both with zero replies. The second one post-dates your fix. I'm not treating either resolution as
evidence, so I read the newest myself: it claims the path class is empty on AKS/EKS/OpenShift and
that the guard therefore refuses a complete chart — the same fail-closed shape you just fixed for
CLS_ENV. Your own measured counts are aks=1 bm=4 eks=1 oc=1, all non-empty, so the premise
looks wrong and the auto-resolution may well be harmless here. But worth a line from you saying so
explicitly, because right now the record shows a High about your guard closing with nobody having
answered it. I've filed the general problem as backend#2945.

Nit, still unchanged: line 308 says the preflight "makes 127 unreachable", contradicting the
case "$rc" block 34 lines up that explains at length why it cannot. Code is right, comment isn't.

@aptracebloc — your CHANGES_REQUESTED from 07:16 is still standing. You withdrew your own HIGH and
re-pointed it at my denominator finding, then wrote "clearing my block on that basis" in a
COMMENTED review, which doesn't dismiss it. latestOpinionatedReviews still returns it, so
clearing mine won't flip reviewDecision — that one needs an actual approve from you.

[LukasWodka]

@saadqbal — both of your points, and the second one measured rather than asserted.

The nit is fixed (7db2618). You were right that the code was fine and the comment wasn't, and the contradiction was worse than a stale line: it inverted its own rationale. The case "$rc" block says command -v python3 answers "is there something on PATH by that name", not "does it run" — so a shim that execs a deleted interpreter passes the preflight and exits 127 right here. The comment then claimed the preflight "makes 127 unreachable anyway", i.e. that requiring exactly 1 was belt-and-braces over an impossible code. It is the opposite: 127 is reachable, and refusing to accept it is the entire point, because it is the one status produced by the absence of the thing the assertion invokes. Rewritten to say that, with your review credited.

On the two Bugbot Highs that cursor[bot] closed with zero replies — you're right not to treat those as evidence, and here is the line you asked for.

The newest one claims the release-scoped path class is empty on AKS/EKS/OpenShift and that the guard therefore refuses a complete chart. The premise is false, and the guard says so itself on every run. From this head just now:

-- release-scoped paths per profile: aks=1 bm=4 eks=1 oc=1
-- release-scoped paths present in 4 profile(s) [OK]
-- release-scoped path SITES per profile: aks=1 bm=4 eks=1 oc=1
-- release-scoped path sites present in 4 profile(s) [OK]
[OK] fullnameOverride routes every resource name, and no exception followed it

Non-empty on all four profiles, for both the class and the site-level enumeration, and the run exits 0. The fail-closed shape the finding describes would need an empty set on a cloud profile; there isn't one. So that auto-resolution looks harmless — but harmless because the premise was wrong, not because anyone checked, which is exactly the gap you're pointing at. Recorded here so the trail shows a human-readable answer rather than a High that closed itself.

Thanks for filing the general problem as backend#2945 — the auto-resolution is the part worth fixing, and it isn't specific to this PR.

Still not asking you to approve: @aptracebloc's CHANGES_REQUESTED from 07:16 is standing, and as you note a COMMENTED review saying "clearing my block" doesn't dismiss it — latestOpinionatedReviews still returns it, so reviewDecision needs an actual approve from them to flip.

[LukasWodka]

bugbot run

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

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