From c70bb071f932d4be894326bd6524e1b07e2647c5 Mon Sep 17 00:00:00 2001 From: shankar0123 Date: Sat, 16 May 2026 22:29:56 +0000 Subject: [PATCH] =?UTF-8?q?fix(helm):=20DEPL-004=20follow-up=20=E2=80=94?= =?UTF-8?q?=20Helm-comment=20block=20for=20tlsConfig=20narrative=20(B3=20c?= =?UTF-8?q?i-guard)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 9155ec9 introduced a YAML `#` comment block above the tlsConfig branch that referenced `{{ if ... }}` and `{{ fail }}` as literal text. Helm's template engine scans for `{{ ... }}` action delimiters everywhere in the source — it does NOT respect YAML `#` comments. So Helm parsed the multi-line sequence {{ if .Values.monitoring. # serviceMonitor.tlsConfig }} as a single template action containing an invalid `#` token, which aborted the WHOLE chart render with: Error: parse error at (certctl/templates/servicemonitor.yaml:51): unexpected <.> in operand That's why all five B3-helm-chart-coherence render modes (default, external-Postgres, production-hardening, sessionAffinity, viaHook) failed simultaneously on f7fcd1e — the parse error fires before any mode-specific values get applied. Fix: replace the YAML `#` block with a Helm `{{- /* ... */ -}}` comment block. Helm strips the comment body before template execution, so descriptive references to `{{ if ... }}` / `{{ fail }}` inside the comment are safe. Also rewrote the `{{ fail }}` message string to drop the inline backtick-quoted `{ insecureSkipVerify: true }` shape (literal `{` could have re-tripped the same scanner) in favor of `insecureSkipVerify=true`. Lesson: descriptive references to Helm template actions inside chart templates MUST live in Helm-comment blocks, never in YAML comments. The G-3-env-docs-drift fix in f7fcd1e is unaffected — this is purely the B3-helm-chart-coherence regression introduced by 9155ec9. --- .../certctl/templates/servicemonitor.yaml | 45 ++++++++++++------- 1 file changed, 29 insertions(+), 16 deletions(-) diff --git a/deploy/helm/certctl/templates/servicemonitor.yaml b/deploy/helm/certctl/templates/servicemonitor.yaml index d338c70..60279d2 100644 --- a/deploy/helm/certctl/templates/servicemonitor.yaml +++ b/deploy/helm/certctl/templates/servicemonitor.yaml @@ -42,25 +42,38 @@ spec: interval: {{ .Values.monitoring.serviceMonitor.interval | default "30s" }} scrapeTimeout: {{ .Values.monitoring.serviceMonitor.scrapeTimeout | default "10s" }} tlsConfig: - # Acquisition-audit DEPL-004 closure (Sprint 6 ACQ, 2026-05-16). - # Pre-Sprint-6 the default was an implicit `insecureSkipVerify: - # true` (template fell through the else-branch). Post-Sprint-6 - # values.yaml ships a real-verify default - # (caFile + serverName matching the chart's existingSecret / - # cert-manager-emitted Secret at /etc/prometheus/secrets/ - # certctl-ca/), so the truthy `{{ if .Values.monitoring. - # serviceMonitor.tlsConfig }}` branch always fires for the - # default install. Operators who genuinely want skipVerify - # back must override with `tlsConfig: { insecureSkipVerify: - # true }` explicitly. Operators who blank tlsConfig entirely - # (`tlsConfig: null` or `tlsConfig: {}`) hit the else-branch - # below and trip the `{{ fail }}` at chart-render time — - # there is no way to inherit the pre-Sprint-6 implicit- - # skipVerify behavior silently. + {{- /* + Acquisition-audit DEPL-004 closure (Sprint 6 ACQ, 2026-05-16). + Pre-Sprint-6 the default was an implicit insecureSkipVerify + true via the template falling through the else branch. + Post-Sprint-6 values.yaml ships a real-verify default + (caFile + serverName matching the chart's existingSecret / + cert-manager-emitted Secret at /etc/prometheus/secrets/ + certctl-ca/), so the truthy if-branch below always fires for + the default install. Operators who want skipVerify back must + override with tlsConfig insecureSkipVerify true explicitly. + Operators who blank tlsConfig entirely (tlsConfig null or + tlsConfig empty-map) hit the else-branch below and trip the + Helm fail directive at chart-render time — there is no way + to inherit the pre-Sprint-6 implicit-skipVerify behavior + silently. + + NOTE: this comment uses Helm's {{- /* ... */ -}} comment + syntax, NOT YAML's # comments. The # form is parsed by YAML + but Helm's template engine still scans for {{ ... }} action + delimiters everywhere in the source text, including inside + YAML comments. Earlier drafts of this block used # comments + that referenced {{ if ... }} and {{ fail }} as descriptive + text — Helm tried to parse those as template actions, hit + invalid # tokens inside the action body, and aborted the + whole chart render. Lesson: descriptive references to + template actions go in Helm-comment blocks, never in YAML + comments. + */ -}} {{- if .Values.monitoring.serviceMonitor.tlsConfig }} {{- toYaml .Values.monitoring.serviceMonitor.tlsConfig | nindent 8 }} {{- else }} - {{- fail "monitoring.serviceMonitor.tlsConfig was explicitly blanked but monitoring.serviceMonitor.enabled=true (Sprint 6 ACQ DEPL-004 closure, 2026-05-16). The values.yaml default ships caFile=/etc/prometheus/secrets/certctl-ca/ca.crt + serverName=certctl-server which matches the existingSecret mount pattern. If your Prometheus pod mounts the CA bundle at a different path, override caFile rather than blanking the block. If you genuinely need skipVerify, set `tlsConfig: { insecureSkipVerify: true }` explicitly — never blank. See docs/operator/helm-deployment.md for the upgrade-path note." }} + {{- fail "monitoring.serviceMonitor.tlsConfig was explicitly blanked but monitoring.serviceMonitor.enabled=true (Sprint 6 ACQ DEPL-004 closure, 2026-05-16). The values.yaml default ships caFile=/etc/prometheus/secrets/certctl-ca/ca.crt + serverName=certctl-server which matches the existingSecret mount pattern. If your Prometheus pod mounts the CA bundle at a different path, override caFile rather than blanking the block. If you genuinely need skipVerify, set tlsConfig insecureSkipVerify=true explicitly — never blank. See docs/operator/helm-deployment.md for the upgrade-path note." }} {{- end }} {{- with .Values.monitoring.serviceMonitor.bearerTokenSecret }} bearerTokenSecret: