fix(chart): guard every hostPath subkey read so --reuse-values renders (backend#2910) - #939
Conversation
…s (backend#2910) `client/templates` read `.Values.hostPath.<key>` unguarded in nine places. Under `helm upgrade --reuse-values` from a release predating the `hostPath` key, `.Values.hostPath` is nil and the first such read nil-pointers, killing the whole render (reproduced: `shared-images-pvc.yaml:3`). Route each read through the chart's own convention `(default dict .Values.hostPath).<key>` (already used at six other sites). The three ternary sites also get `| default false` so the nil subkey coerces to the bool `ternary` requires. Add scripts/tests/hostpath-reads-guarded.sh to the required `drift` gate. It DERIVES the site list from the tree — greps client/templates for the literal `.Values.hostPath.`, the one signature a guarded read `(default dict .Values.hostPath).enabled` cannot contain — so a new unguarded read is caught without restating the nine paths (CLAUDE.md rule 1), and un-guarding any site reddens it. Fails closed if the hostPath surface disappears. Bump Chart.yaml 1.9.89 -> 1.9.90 (chart-version-guard). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
develop advanced to 1.9.90 (#929) while this branch was open; the identical bump auto-merged, so the chart-version-guard saw no net change. Bump one patch above develop. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
saadqbal
left a comment
There was a problem hiding this comment.
Nice, careful PR — the "every" in the title holds. 15/15 .Values.hostPath reads across
client/templates (including _helpers.tpl, not just the 9 changed files) are guarded at
head, 0 unguarded, against 9 unguarded on develop. I reproduced the nil-pointer on develop
and the clean 59-manifest render here, and behaviour is unchanged (bm profile still 3 PVs +
ReadWriteOnce). Both subkeys are scalars, so one level of (default dict …) is the right
depth rather than one level short.
One gap, worth a follow-up rather than a block: nothing in CI ever renders with hostPath
absent. I un-guarded shared-images-pvc.yaml:3 and logs-pvc.yaml:36 in turn — helm unittest (643), helm lint --strict and helm template on default values all stayed
green; only the new drift guard reddened. So the guard is doing real work and is genuinely
mutation-proved, but it's a text check standing in for a render. Adding --set hostPath=null as one more cell to the Template render matrix would exercise the actual
failure mode, and would catch what grep can't see:
{{- if (index .Values.hostPath "enabled") }}
guard: green
render: error calling index: index of untyped nil
Two small ones while you're in there. The guard header calls .Values.hostPath. "the ONE
textual signature an unguarded read has and a guarded one cannot" — the second half is
right, the first half the index form above falsifies; worth softening, because in this
repo the guard header is the durable explanation and an over-strong claim there is exactly
what stops the next person adding the render cell. And the body says 1.9.89 -> 1.9.90 and
641 tests, but the diff is 1.9.90 -> 1.9.91 and I get 643 (the Cursor summary has 1.9.91
right).
Three conflicts, and two of them would have silently reverted work: * Makefile DRIFT_GUARDS -- develop moved to one-guard-per-line (#933) while this branch added `jobs-manager-waits-for-mysql.sh` to the old single-line form. UNION merged: develop format plus our guard as its own line. Taking either side alone drops a REQUIRED guard -- ours would have lost `hostpath-reads-guarded.sh`, theirs ours. Verified both are present: 40 green. * jobs-manager-deployment.yaml -- ours is this PR (unconditional initContainers), theirs is the nil-guarded hostPath read from #939/backend#2910. Adopted their guard inside our block; taking ours verbatim would have reverted a fix that keeps `--reuse-values` rendering when the hostPath key is absent entirely. Verified: 3 live hostPath reads, 0 unguarded, and the chart renders with hostPath omitted. * Chart.yaml -- 1.9.92, one patch above develop 1.9.91. Also reworded one comment of mine: `hostpath-reads-guarded.sh` reads RAW text, so my line describing the old gate tripped it by quoting the bare read. Prose breaking a check rather than satisfying one -- the guard cannot tell a comment from code, which is worth fixing in the guard rather than in every comment that needs to name the anti-pattern. Not doing that here: it is develop s guard and belongs in its own change. helm unittest 644/644, drift 40/40. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
What
client/templatesread.Values.hostPath.<key>unguarded in nine places. Underhelm upgrade --reuse-valuesfrom a release predating thehostPathkey,.Values.hostPathis nil and the first unguarded read nil-pointers, killing the whole render.Reproduced on
develop:(The tenth site,
secrets.yaml:91, was already fixed under backend#2626 — one of ten does not fix the render, which is why this was filed separately.)Fix
Route every read through the chart's own convention
(default dict .Values.hostPath).<key>— already used at six other sites. A nilhostPathbecomes an empty dict, and the subkey reads as nil-but-not-a-panic.The nine sites (found by pattern, not line number):
NOTES.txtjobs-manager-deployment.yamllogs-pvc.yamlmysql-deployment.yamlmysql-storage-pvc.yamlshared-images-pvc.yamlThe three PVC
accessModesreads sit inside aternary, which requires abool; the defaulted-dict subkey is nil there, so they also get| default falseto coerce it (matching the| default falsenormalization the already-guardedjobs-managersites use).Guard — derived, not restated
scripts/tests/hostpath-reads-guarded.sh, added to the requireddriftgate, derives the site list from the tree instead of restating the nine paths (CLAUDE.md rule 1). It grepsclient/templatesfor the literal.Values.hostPath.— the one textual signature a guarded read cannot have, because(default dict .Values.hostPath).enabledputs a)betweenhostPathand the.. So:ifsite and aternarysite);.Values.hostPathsurface disappears (templates moved, hostPath removed), the guard fails rather than passing on an empty sweep.Chart version
client/Chart.yaml1.9.89 → 1.9.90 (version+appVersion), per the chart-version-guard.Verification
--set hostPath=null) now renders — 59 manifests,accessModes: ReadWriteMany.hostPath.enabled=true→ 3 PVs +ReadWriteOnce; explicitenabled=false→ no PVs +ReadWriteMany.helm unittest ./client— 641 passed / 36 suites.helm lint --strictandhelm template— clean on all four platforms (aks/bm/eks/oc;bmexerciseshostPath.enabled=true).shellcheck-clean at--severity=error(gate) and--severity=warning(advisory).Closes tracebloc/backend#2910
Note
Low Risk
Nil-safe Helm templating for upgrades with reused values; behavior unchanged when
hostPathis set explicitly, with a drift guard to prevent regressions.Overview
Fixes helm render failures when
hostPathis missing from merged values (e.g.helm upgrade --reuse-valuesfrom a release that predates thehostPathkey). Unguarded.Values.hostPath.<key>reads nil-pointer and aborts the whole template; nine sites across six templates now use(default dict .Values.hostPath).<key>, with| default falseon PVCaccessModesternaries so bool coercion stays safe.Adds
scripts/tests/hostpath-reads-guarded.shto the requiredmake driftlist: it fails on any.Values.hostPath.substring inclient/templates(derived check, fails closed if the surface disappears). Chartversion/appVersionbumped to 1.9.91.Reviewed by Cursor Bugbot for commit d2eff99. Bugbot is set up for automated code reviews on this repo. Configure here.