From c5907a7c712a4c243aef1730f5f1e4862742a4fa Mon Sep 17 00:00:00 2001 From: Arturo Peroni Date: Mon, 31 Aug 2026 17:15:22 +0200 Subject: [PATCH 1/2] fix(chart): guard every hostPath subkey read so --reuse-values renders (backend#2910) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `client/templates` read `.Values.hostPath.` 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).` (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 --- Makefile | 2 +- client/Chart.yaml | 4 +- client/templates/NOTES.txt | 2 +- client/templates/jobs-manager-deployment.yaml | 2 +- client/templates/logs-pvc.yaml | 4 +- client/templates/mysql-deployment.yaml | 2 +- client/templates/mysql-storage-pvc.yaml | 4 +- client/templates/shared-images-pvc.yaml | 4 +- scripts/tests/hostpath-reads-guarded.sh | 87 +++++++++++++++++++ 9 files changed, 99 insertions(+), 12 deletions(-) create mode 100755 scripts/tests/hostpath-reads-guarded.sh diff --git a/Makefile b/Makefile index 1175ae38..46dfd159 100644 --- a/Makefile +++ b/Makefile @@ -350,7 +350,7 @@ lint-warnings: # # `|`-separated because each guard is a multi-word command. One entry per guard, # and this is the only place they are written down. -DRIFT_GUARDS := scripts/gen-manifest.sh --check|scripts/check-facts.sh --check|bash scripts/check-style.sh|bash scripts/tests/check-drift.sh|bash scripts/tests/env-vocabulary-agreement.sh|bash scripts/tests/telemetry-vocabulary-agreement.sh|bash scripts/tests/k3s-components-agreement.sh|bash scripts/tests/collector-class-a-agreement.sh|bash scripts/tests/openshift-scc-coverage.sh|bash scripts/tests/collector-offsets-persisted.sh|bash scripts/tests/node-agents-tenancy.sh|bash scripts/tests/node-agents-pull-secret.sh|bash scripts/tests/telemetry-token-agreement.sh|bash scripts/tests/node-agents-namespace-safety.sh|bash scripts/tests/automount-token-explicit.sh|bash scripts/tests/collector-redaction-floor.sh|bash scripts/tests/collector-redaction-derived.sh|bash scripts/tests/telemetry-token-bootstrap.sh|bash scripts/tests/node-jsonpath-agreement.sh|bash scripts/tests/preflight-not-privilege-gated.sh|bash scripts/tests/kubelet-arg-map-safety.sh|bash scripts/tests/kubelet-config-agreement.sh|bash scripts/tests/kubelet-config-mutations.sh|bash scripts/tests/cronjob-failures-are-readable.sh|bash scripts/tests/release-name-equals-namespace.sh|bash scripts/tests/helm-unittest-error-assertions.sh|bash scripts/tests/helm-unittest-gated.sh|bash scripts/tests/mirror-enumeration-complete.sh|bash scripts/tests/reparent-requires-rotation.sh|bash scripts/tests/edgeuser-drop-readiness-verdicts.sh|bash scripts/tests/regcred-migration-verdicts.sh|bash scripts/tests/client-credentials-have-a-secret-tier.sh|bash scripts/tests/gate-byenv-resolution.sh|bash scripts/tests/gpu-limit-semantics-documented.sh|bash scripts/tests/gate-default-prose-agreement.sh|bash scripts/tests/gate-default-prose-mutations.sh|bash scripts/tests/guards-survive-spaced-paths.sh|bash scripts/tests/auto-upgrade-inflight-vs-wedge.sh +DRIFT_GUARDS := scripts/gen-manifest.sh --check|scripts/check-facts.sh --check|bash scripts/check-style.sh|bash scripts/tests/check-drift.sh|bash scripts/tests/env-vocabulary-agreement.sh|bash scripts/tests/telemetry-vocabulary-agreement.sh|bash scripts/tests/k3s-components-agreement.sh|bash scripts/tests/collector-class-a-agreement.sh|bash scripts/tests/openshift-scc-coverage.sh|bash scripts/tests/collector-offsets-persisted.sh|bash scripts/tests/node-agents-tenancy.sh|bash scripts/tests/node-agents-pull-secret.sh|bash scripts/tests/telemetry-token-agreement.sh|bash scripts/tests/node-agents-namespace-safety.sh|bash scripts/tests/automount-token-explicit.sh|bash scripts/tests/collector-redaction-floor.sh|bash scripts/tests/collector-redaction-derived.sh|bash scripts/tests/telemetry-token-bootstrap.sh|bash scripts/tests/node-jsonpath-agreement.sh|bash scripts/tests/preflight-not-privilege-gated.sh|bash scripts/tests/kubelet-arg-map-safety.sh|bash scripts/tests/kubelet-config-agreement.sh|bash scripts/tests/kubelet-config-mutations.sh|bash scripts/tests/cronjob-failures-are-readable.sh|bash scripts/tests/release-name-equals-namespace.sh|bash scripts/tests/helm-unittest-error-assertions.sh|bash scripts/tests/helm-unittest-gated.sh|bash scripts/tests/mirror-enumeration-complete.sh|bash scripts/tests/reparent-requires-rotation.sh|bash scripts/tests/edgeuser-drop-readiness-verdicts.sh|bash scripts/tests/regcred-migration-verdicts.sh|bash scripts/tests/client-credentials-have-a-secret-tier.sh|bash scripts/tests/gate-byenv-resolution.sh|bash scripts/tests/gpu-limit-semantics-documented.sh|bash scripts/tests/gate-default-prose-agreement.sh|bash scripts/tests/gate-default-prose-mutations.sh|bash scripts/tests/guards-survive-spaced-paths.sh|bash scripts/tests/auto-upgrade-inflight-vs-wedge.sh|bash scripts/tests/hostpath-reads-guarded.sh # EXPORTED, not interpolated. The recipe reads $$DRIFT_GUARDS from the # environment; it used to do `guards='$(DRIFT_GUARDS)'`, which Make expands diff --git a/client/Chart.yaml b/client/Chart.yaml index 65e49efd..517bbae7 100644 --- a/client/Chart.yaml +++ b/client/Chart.yaml @@ -2,8 +2,8 @@ apiVersion: v2 name: client description: A unified Helm chart for tracebloc on AKS, EKS, bare-metal, and OpenShift type: application -version: 1.9.89 -appVersion: "1.9.89" +version: 1.9.90 +appVersion: "1.9.90" keywords: - tracebloc - kubernetes diff --git a/client/templates/NOTES.txt b/client/templates/NOTES.txt index 0197da7e..e252b1e7 100644 --- a/client/templates/NOTES.txt +++ b/client/templates/NOTES.txt @@ -14,7 +14,7 @@ {{ "\033[1;34m" }}Secret:{{ "\033[0m" }} {{ "\033[0;33m" }}{{ include "tracebloc.secretName" . }}{{ "\033[0m" }} {{ "\033[1;34m" }}RBAC scope:{{ "\033[0m" }} {{ "\033[0;33m" }}{{ if ne .Values.clusterScope false }}Cluster{{ else }}Namespace{{ end }}{{ "\033[0m" }} {{ "\033[1;34m" }}Image registry:{{ "\033[0m" }} {{ "\033[1;32m" }}docker.io{{ "\033[0m" }} -{{- if .Values.hostPath.enabled }} +{{- if (default dict .Values.hostPath).enabled }} {{ "\033[1;34m" }}Storage:{{ "\033[0m" }} {{ "\033[0;33m" }}hostPath (bare-metal){{ "\033[0m" }} {{ "\033[1;34m" }}Host dirs:{{ "\033[0m" }} {{ "\033[0;33m" }}/tracebloc/data, /tracebloc/logs, /tracebloc/mysql (on the node){{ "\033[0m" }} {{- else }} diff --git a/client/templates/jobs-manager-deployment.yaml b/client/templates/jobs-manager-deployment.yaml index e3659fea..8330a36f 100644 --- a/client/templates/jobs-manager-deployment.yaml +++ b/client/templates/jobs-manager-deployment.yaml @@ -48,7 +48,7 @@ spec: # It never reaches those spawned writers, so it's all risk and no gain. seccompProfile: type: RuntimeDefault - {{- if .Values.hostPath.enabled }} + {{- if (default dict .Values.hostPath).enabled }} # kubelet does NOT apply fsGroup to hostPath volumes (kubernetes/kubernetes#138411), # so /data/shared AND /data/logs are created root-owned and non-root pods can't write # to them. Two failures this causes: (1) dataset ingest — `tb data ingest` streams files diff --git a/client/templates/logs-pvc.yaml b/client/templates/logs-pvc.yaml index dda191fd..7caf869b 100644 --- a/client/templates/logs-pvc.yaml +++ b/client/templates/logs-pvc.yaml @@ -1,6 +1,6 @@ {{- $name := include "tracebloc.clientLogsPvc" . -}} {{- $storage := include "tracebloc.clientLogsStorage" . -}} -{{- if .Values.hostPath.enabled }} +{{- if (default dict .Values.hostPath).enabled }} apiVersion: v1 kind: PersistentVolume metadata: @@ -33,7 +33,7 @@ metadata: spec: storageClassName: {{ include "tracebloc.storageClassName" . }} accessModes: - - {{ .Values.pvcAccessMode | default (ternary "ReadWriteOnce" "ReadWriteMany" .Values.hostPath.enabled) }} + - {{ .Values.pvcAccessMode | default (ternary "ReadWriteOnce" "ReadWriteMany" ((default dict .Values.hostPath).enabled | default false)) }} resources: requests: storage: {{ $storage }} diff --git a/client/templates/mysql-deployment.yaml b/client/templates/mysql-deployment.yaml index 60c819a2..a1dde23d 100644 --- a/client/templates/mysql-deployment.yaml +++ b/client/templates/mysql-deployment.yaml @@ -37,7 +37,7 @@ spec: fsGroup: 999 fsGroupChangePolicy: "OnRootMismatch" initContainers: - {{- if .Values.hostPath.enabled }} + {{- if (default dict .Values.hostPath).enabled }} # kubelet does not apply fsGroup to hostPath volumes # (kubernetes/kubernetes#138411), so bare-metal installs need a # privileged bootstrap to chown /var/lib/mysql to 999:999. CSI-backed diff --git a/client/templates/mysql-storage-pvc.yaml b/client/templates/mysql-storage-pvc.yaml index 28926162..84bf3fa3 100644 --- a/client/templates/mysql-storage-pvc.yaml +++ b/client/templates/mysql-storage-pvc.yaml @@ -1,6 +1,6 @@ {{- $name := include "tracebloc.mysqlPvc" . -}} {{- $storage := include "tracebloc.mysqlStorage" . -}} -{{- if .Values.hostPath.enabled }} +{{- if (default dict .Values.hostPath).enabled }} apiVersion: v1 kind: PersistentVolume metadata: @@ -33,7 +33,7 @@ metadata: spec: storageClassName: {{ include "tracebloc.storageClassName" . }} accessModes: - - {{ .Values.pvcAccessMode | default (ternary "ReadWriteOnce" "ReadWriteMany" .Values.hostPath.enabled) }} + - {{ .Values.pvcAccessMode | default (ternary "ReadWriteOnce" "ReadWriteMany" ((default dict .Values.hostPath).enabled | default false)) }} resources: requests: storage: {{ $storage }} diff --git a/client/templates/shared-images-pvc.yaml b/client/templates/shared-images-pvc.yaml index cb1d4231..6eae5e1d 100644 --- a/client/templates/shared-images-pvc.yaml +++ b/client/templates/shared-images-pvc.yaml @@ -1,6 +1,6 @@ {{- $name := include "tracebloc.clientDataPvc" . -}} {{- $storage := include "tracebloc.clientDataStorage" . -}} -{{- if .Values.hostPath.enabled }} +{{- if (default dict .Values.hostPath).enabled }} apiVersion: v1 kind: PersistentVolume metadata: @@ -33,7 +33,7 @@ metadata: spec: storageClassName: {{ include "tracebloc.storageClassName" . }} accessModes: - - {{ .Values.pvcAccessMode | default (ternary "ReadWriteOnce" "ReadWriteMany" .Values.hostPath.enabled) }} + - {{ .Values.pvcAccessMode | default (ternary "ReadWriteOnce" "ReadWriteMany" ((default dict .Values.hostPath).enabled | default false)) }} resources: requests: storage: {{ $storage }} diff --git a/scripts/tests/hostpath-reads-guarded.sh b/scripts/tests/hostpath-reads-guarded.sh new file mode 100755 index 00000000..ef9e33cb --- /dev/null +++ b/scripts/tests/hostpath-reads-guarded.sh @@ -0,0 +1,87 @@ +#!/usr/bin/env bash +# +# hostpath-reads-guarded.sh — every read of a `.Values.hostPath` SUBKEY in +# client/templates goes through `(default dict .Values.hostPath)`, so a render +# that inherits a nil `hostPath` survives (backend#2910). +# +# WHY THIS EXISTS, MEASURED. `hostPath` is a map added to values.yaml after the +# first releases shipped. `helm upgrade --reuse-values` from a release that +# predates the key carries no `hostPath`, so `.Values.hostPath` is nil at render +# time. A bare `.Values.hostPath.enabled` then dies with +# "nil pointer evaluating interface {}.enabled" and kills the WHOLE render — +# reproduced on develop with: +# +# helm template t client/ --set hostPath=null \ +# --set storageClass.create=false --set clientId=x --set clientPassword=y +# Error: client/templates/shared-images-pvc.yaml:3 ... nil pointer ... +# +# Nine reads across six templates were bare; the chart's own convention at the +# other sites is `(default dict .Values.hostPath).`, which turns a nil +# hostPath into an empty dict and reads the subkey as a nil-but-not-a-panic. +# +# DERIVED, NOT RESTATED (backend#1729 rule 1). This guard holds NO list of the +# nine files. It greps the tree for the ONE textual signature an unguarded read +# has and a guarded one cannot: the substring `.Values.hostPath.` — a subkey +# read where a `.` immediately follows `hostPath`. The guarded form is +# `(default dict .Values.hostPath).enabled`: a `)` sits between `hostPath` and +# the `.`, so that substring never appears in it. A bare truthiness test +# (`.Values.hostPath` with no subkey) is nil-safe and, having no trailing `.`, +# is likewise not matched. So a template added tomorrow with a new bare read is +# caught tomorrow, and un-guarding any current site reddens this guard — the +# mutation test the acceptance asks for, without a path list to keep in sync. +# +# FAILS CLOSED (backend#1729 rule 3). If the tree the guard reasons about is +# gone — templates dir moved, or every `.Values.hostPath` reference removed — +# the offender grep would find nothing and pass VACUOUSLY. So the denominator +# (any `.Values.hostPath` reference at all) is asserted non-empty first: zero is +# "I cannot see the code I guard", which is a failure, not agreement. +# +# SCOPE. Templates only — that is where `.Values` reads render. `hostPath:` in +# values.yaml / ci values is DATA, not a read, and correctly unmatched. A prose +# comment that spells the unsafe form `.Values.hostPath.enabled` would trip this; +# none do today, and a comment advertising the panicking form is worth rewriting +# anyway. +set -euo pipefail + +cd "$(dirname "${BASH_SOURCE[0]}")/../.." + +DIR="client/templates" +[ -d "$DIR" ] || { echo "FAIL: $DIR not found — cannot check what I cannot read" >&2; exit 1; } + +echo "== hostPath subkey reads are guarded ==" + +# Denominator: every `.Values.hostPath` reference (guarded, bare, or offending). +# grep exits 1 on no-match, and under `errexit`+`pipefail` that aborts the +# `denom=$(…)` assignment BEFORE the count is judged — losing the fail-closed +# diagnostic below (the exit stays 1, but silently). `|| true` on the pipeline +# absorbs the no-match so the explicit count check owns the zero case. Zero +# references means the surface this guard exists for is gone — fail closed +# rather than report green on an empty sweep. +denom=$(grep -rEo '\.Values\.hostPath' "$DIR" | wc -l | tr -d ' ' || true) +if [ "$denom" -eq 0 ]; then + echo "FAIL: no \`.Values.hostPath\` reference in $DIR at all." >&2 + echo " Either the templates moved or hostPath handling was removed — this" >&2 + echo " guard is now blind and must be updated or deleted deliberately, not" >&2 + echo " left to pass vacuously." >&2 + exit 1 +fi + +# Offenders: a subkey read where `.` immediately follows `hostPath`. This is the +# unguarded form and ONLY the unguarded form (see header). +offenders=$(grep -rn --fixed-strings '.Values.hostPath.' "$DIR" || true) + +if [ -n "$offenders" ]; then + echo "" >&2 + echo "FAIL: unguarded \`.Values.hostPath.\` read(s) — these nil-pointer and" >&2 + echo " kill the render under \`helm upgrade --reuse-values\` from a release" >&2 + echo " predating the hostPath key (backend#2910):" >&2 + printf '%s\n' "$offenders" | sed 's/^/ /' >&2 + echo "" >&2 + echo " Route each through the chart convention:" >&2 + echo " .Values.hostPath.enabled -> (default dict .Values.hostPath).enabled" >&2 + echo " (in a ternary/other bool context, append \`| default false\` so the" >&2 + echo " nil subkey coerces to a bool)." >&2 + exit 1 +fi + +echo "hostPath subkey reads are guarded: green — $denom \`.Values.hostPath\` reference(s), none unguarded" From d2eff99035123a11beadd5f45451e7237132ab00 Mon Sep 17 00:00:00 2001 From: Arturo Peroni Date: Mon, 31 Aug 2026 17:19:54 +0200 Subject: [PATCH 2/2] fix(chart): bump to 1.9.91 above develop after merge (backend#2910) 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 --- client/Chart.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/Chart.yaml b/client/Chart.yaml index 517bbae7..d79f4213 100644 --- a/client/Chart.yaml +++ b/client/Chart.yaml @@ -2,8 +2,8 @@ apiVersion: v2 name: client description: A unified Helm chart for tracebloc on AKS, EKS, bare-metal, and OpenShift type: application -version: 1.9.90 -appVersion: "1.9.90" +version: 1.9.91 +appVersion: "1.9.91" keywords: - tracebloc - kubernetes