fix(chart): guard every hostPath subkey read so --reuse-values renders (backend#2910)

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

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

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 }}

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

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 }}

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

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 }}

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 }}

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).<key>`, 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.<key>\` 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"