merge: the substitution check now catches a collapsed doubled dollar
The existing check audited only the inverse — a ${...} undefined or
unescaped — so it passed the manifest that broke prod: Postgres
dollar-quoting under a postBuild.substituteFrom path, which envsubst halves,
delivering 'DO $' and a syntax error on every reconciled run (fixed 16b7fcd).
Measured rather than assumed: Flux 2.4.0 embeds fluxcd/pkg/envsubst (a fork
of drone/envsubst — the brief credited the wrong module), and --strict output
is BYTE-IDENTICAL to non-strict here, so strict mode is blind to this class
entirely. The decisive contrast is two commands on the same stream: kubectl
kustomize emits valid 'DO $$' — which is what the kubectl-apply
verification saw — while the same stream through flux envsubst emits 'DO $'.
357 assertions, 0 unjustified. Legitimate uses genuinely exist (18 ${} escapes
plus two reaching shell comments), so the allowlist is a reviewed opt-in
rather than a bypass: a stale entry fails, a wildcard is matched verbatim and
so fails as stale, a short context or reason is rejected, and a genuine entry
covers only the span it matches — a second occurrence in the same file still
fails. Strongest proof: restoring the real pre-16b7fcd manifest verbatim makes
the guard reject the exact commit that broke production.
This commit is contained in:
jakman85 committed
commit
37eea51f24
3 files changed
+374
-8
No files matched your search
@@ -8,6 +8,18 @@
|
||||
# the empty string while the Kustomization still reports Ready — which emptied Cilium's
|
||||
# k8sServiceHost and broke the CNI on 2/3 dev nodes (DECISIONS-LOG 2026-09-01).
|
||||
#
|
||||
# It is also LOSSY IN THE OTHER DIRECTION, and that is the second incident on this record.
|
||||
# envsubst consumes dollars in PAIRS and emits one per pair, unconditionally and everywhere —
|
||||
# so no substituted path can carry a literal `$$` at all. On 2026-09-13 a ConfigMap under
|
||||
# apps-wave-0 shipped a dollar-quoted PL/pgSQL `DO $$ ... $$;` self-check; Postgres received
|
||||
# a half-quoted `DO $` and every Flux-reconciled run of the nextcloud-db grant reconciler died
|
||||
# with a syntax error, nine Error pods deep before anyone looked (commit 16b7fcd). It passed
|
||||
# review because it was hand-applied with `kubectl apply`, which runs no envsubst: the defect
|
||||
# existed ONLY on the rendered path, and it landed on the assertion whose whole job was to
|
||||
# make a silent regression visible. Nothing here failed — the manifest rendered
|
||||
# "successfully" into something broken, because the check treated any doubled dollar as a
|
||||
# deliberate escape and stopped looking.
|
||||
#
|
||||
# GIVEN every Flux Kustomization under clusters/ that declares postBuild.substituteFrom,
|
||||
# WHEN each `spec.path` is resolved to the exact file set it renders (following
|
||||
# kustomization.yaml `resources:` transitively, not globbing the repo),
|
||||
@@ -15,7 +27,12 @@
|
||||
# or deliberately ESCAPED `$${...}`; no unescaped expansion carries a default
|
||||
# operator on an undefined name (the silent-empty class strict mode does NOT catch);
|
||||
# no unescaped expansion is unparseable by envsubst (which fails the WHOLE
|
||||
# Kustomization); every path rendering a knob actually substitutes cluster-vars;
|
||||
# Kustomization); NO RUN OF TWO OR MORE DOLLARS IS SILENTLY REWRITTEN — the only shape
|
||||
# that passes unremarked is exactly `$${`, the documented total escape, and every other
|
||||
# run (a bare `$$` that cannot survive verbatim, an ODD `$$${...}` that only LOOKS
|
||||
# escaped and in fact expands, an over-counted `$$$${...}`) is named and justified in
|
||||
# `dollar-collapse.allow`, which is itself held to being exact and non-stale;
|
||||
# every path rendering a knob actually substitutes cluster-vars;
|
||||
# every overlay-patched knob has a base default; and the check is wired into the
|
||||
# merge gate as a blocking, unconditionally-run stage.
|
||||
#
|
||||
@@ -26,6 +43,19 @@
|
||||
# only); conformance/run.sh --layer l0 and ci/l0.sh run it directly, and this Chainsaw
|
||||
# Test wraps the SAME script.
|
||||
#
|
||||
# NOT proven, and deliberately NOT proxied: the wider lesson of 2026-09-13 is that the bug
|
||||
# survived review because verification used a path (`kubectl apply`) that runs no
|
||||
# substitution. That property is NOT statically decidable from this repo — which command
|
||||
# someone ran is a fact about a shell session, and git stores the manifest, never the
|
||||
# invocation. Grepping commit prose or runbooks for `kubectl apply` would be trivially
|
||||
# satisfiable by rewording and would fire on the break-glass runbooks whose whole content is
|
||||
# that command, so no such proxy is attempted. The dollar-run assertion is the honest answer
|
||||
# instead: it moves the proof off the verification PATH and onto the SOURCE, where it IS
|
||||
# decidable, so the mangling is caught by reading the manifest however a human chose to try
|
||||
# it. An end-to-end `kustomize build | flux envsubst --strict` per substituted path would be
|
||||
# the real thing, and it is how the arithmetic in check.sh's header was measured — but the L0
|
||||
# CI toolchain installs neither tool, so as a gate stage it would SKIP permanently.
|
||||
#
|
||||
# NOT proven (named in check.sh's header): this is a STATIC scan of the SOURCE manifests
|
||||
# a path includes — a deliberate superset of the post-`kustomize build` stream. It does
|
||||
# not render, does not prove the feature gate is enabled on the live kustomize-controller,
|
||||
@@ -63,13 +93,16 @@ metadata:
|
||||
conformance.home.k3s/intent: >-
|
||||
Every variable a substituted Flux path renders is defined in cluster-vars or
|
||||
escaped, so post-build substitution cannot silently empty a value and enabling
|
||||
StrictPostBuildSubstitutions cannot fail a legitimate manifest.
|
||||
StrictPostBuildSubstitutions cannot fail a legitimate manifest; and no dollar run
|
||||
envsubst rewrites reaches the cluster unreviewed, so content that must arrive
|
||||
verbatim cannot be silently mangled on the Flux-rendered path (16b7fcd).
|
||||
spec:
|
||||
steps:
|
||||
- name: audit-every-substituted-flux-path
|
||||
description: >-
|
||||
Resolve each postBuild.substituteFrom Kustomization to the files it renders and
|
||||
assert every ${...} is defined or escaped, then assert the gate wires it blocking.
|
||||
assert every ${...} is defined or escaped and no dollar run is silently collapsed,
|
||||
then assert the gate wires it blocking.
|
||||
try:
|
||||
- script:
|
||||
content: bash ./check.sh
|
||||
@@ -18,6 +18,32 @@
|
||||
# substituted path renders is either DEFINED or deliberately ESCAPED — and nothing in
|
||||
# the repo asserted that. This check does.
|
||||
#
|
||||
# SECOND INCIDENT, 2026-09-13 — THE INVERSE HAZARD (commit 16b7fcd)
|
||||
# ------------------------------------------------------------------
|
||||
# The first incident was a variable the author wanted EXPANDED that rendered EMPTY. This one
|
||||
# is its mirror: content the author wanted to reach the workload UNTOUCHED that substitution
|
||||
# silently REWROTE. A CronJob reconciler in apps/wave-0/nextcloud-db.yaml asserted its own
|
||||
# convergence in a dollar-quoted PL/pgSQL block —
|
||||
#
|
||||
# DO <dq> BEGIN ... END <dq>; where <dq> is two literal dollars: standard Postgres
|
||||
#
|
||||
# — inside a ConfigMap under apps-wave-0, which declares postBuild.substituteFrom. envsubst
|
||||
# collapses a doubled dollar into ONE dollar, ALWAYS, so Postgres received a half-quoted
|
||||
# `DO <one dollar>` and died with `syntax error at or near "$"`. Three scheduled runs left
|
||||
# nine Error pods in nextcloud-db before anyone looked.
|
||||
#
|
||||
# It PASSED verification because it was hand-applied with `kubectl apply`, which runs NO
|
||||
# envsubst. The defect existed ONLY on the Flux-rendered path. And the damage landed
|
||||
# precisely on the self-check: the GRANT on line 1 still ran, so the privilege stayed
|
||||
# correct and the only thing broken was the assertion meant to make a silent regression
|
||||
# visible — the part whose whole job is to stop a blind spot being quiet.
|
||||
#
|
||||
# THE ASSERTION GAP THIS CLOSES: assertions (1)-(3) below ask whether every `${...}` is
|
||||
# defined, escaped, or parseable. They said NOTHING about a doubled dollar that is not an
|
||||
# escape, because the old classifier treated a `$` run of length >= 2 as "escaped, therefore
|
||||
# fine" and stopped looking. Nothing failed the gate; the manifest rendered "successfully"
|
||||
# into something broken. Assertion (7) is that hole, closed.
|
||||
#
|
||||
# ENVSUBST BEHAVIOUR ASSERTED HERE (empirically verified against the pinned toolchain,
|
||||
# `flux envsubst`, flux 2.4.0 — this is measured behaviour, not assumption):
|
||||
# ${DEFINED} -> substituted with the cluster-vars value.
|
||||
@@ -33,6 +59,66 @@
|
||||
# shell/cloud-init consumer. Escaping is total: it neutralises
|
||||
# even the indexed form.
|
||||
# $VAR, $(cmd) -> left alone by envsubst; they need no escaping.
|
||||
#
|
||||
# DOLLAR-RUN ARITHMETIC — the exact rule, MEASURED, not inferred (see assertion (7))
|
||||
# ---------------------------------------------------------------------------------
|
||||
# envsubst here is `github.com/fluxcd/pkg/envsubst` v1.1.0 — Flux's own maintained FORK of
|
||||
# `github.com/drone/envsubst` ("This package is a fork of drone/envsubst", its README) — and it
|
||||
# is exposed by the `flux envsubst` CLI ("replicate the behavior of the Flux Kustomization
|
||||
# post-build substitutions", its own --help). Named from the binary, not from memory: the
|
||||
# pinned flux 2.4.0 in the dev image embeds the Go module path
|
||||
# `/home/runner/go/pkg/mod/github.com/fluxcd/pkg/[email protected]/parse`
|
||||
# (`grep -ao 'github.com/fluxcd/pkg/envsubst[^ ]*' "$(command -v flux)"`). Its scanner walks
|
||||
# the input left to right; on a `$` it looks at the NEXT byte:
|
||||
# * another `$` -> BOTH are consumed and ONE `$` is emitted. This is UNCONDITIONAL. What
|
||||
# follows the pair is irrelevant — `{`, a letter, a digit, a semicolon,
|
||||
# a newline, end of input. `$$` is not "the escape for `${`"; it is the
|
||||
# escape for `$` itself, everywhere.
|
||||
# * `{` -> the braced expression is parsed and EXPANDED.
|
||||
# * anything else -> the `$` is emitted literally (hence `$VAR` and `$(cmd)` survive).
|
||||
# So a run of N consecutive dollars emits floor(N/2) literal dollars, and when N is ODD the
|
||||
# final dollar is still LIVE. Measured with flux 2.4.0 `flux envsubst`, DEFINED=VALUE — the
|
||||
# left column is authored bytes, the right is what reaches the cluster:
|
||||
#
|
||||
# $ -> $ ${DEFINED} -> VALUE
|
||||
# $$ -> $ $${DEFINED} -> ${DEFINED} <- the escape
|
||||
# $$$ -> $$ $$${DEFINED} -> $VALUE <- ODD: brace LIVE
|
||||
# $$$$ -> $$ $$$${DEFINED} -> $${DEFINED}
|
||||
# $$$$$ -> $$$ $$$$${DEFINED} -> $$VALUE
|
||||
# DO $$ .. $$; -> DO $ .. $; $$${NOPE:-d} -> $d
|
||||
# awk '{print $$2}' -> awk '{print $2}' $${ARR[0]} -> ${ARR[0]} (safe)
|
||||
# $$@ $$< $$^ -> $@ $< $^ $$${ARR[0]} -> HARD PARSE ERROR, whole
|
||||
# Kustomization dies
|
||||
#
|
||||
# TWO CONSEQUENCES THIS CHECK IS BUILT ON:
|
||||
# (a) THERE IS NO WAY TO SHIP A LITERAL `$$` THROUGH A SUBSTITUTED PATH. To make two
|
||||
# dollars arrive you must author FOUR. So any authored `$$` that is meant to reach the
|
||||
# workload verbatim is a BUG, full stop — that is the 2026-09-13 incident, and Postgres
|
||||
# dollar-quoting is its commonest shape (Makefile `$$@`, awk `$$2` and docker-compose
|
||||
# `$${}` are the same hazard arriving from the other direction: there the collapse is
|
||||
# what the author wanted).
|
||||
# (b) `$$$` + brace is NOT "more escaped" than `$$` + brace — it is NOT escaped at all. An
|
||||
# odd run leaves one live dollar, so the brace expands, or, for an inner envsubst
|
||||
# cannot parse, fails the ENTIRE Kustomization (proven above with $$${ARR[0]}, which
|
||||
# aborted the whole stream while $${ARR[0]} on the line before it was fine).
|
||||
# `--strict` reports NONE of this: every row of that table except the parse error was
|
||||
# accepted SILENTLY by `flux envsubst --strict`, exit 0, and its output was BYTE-IDENTICAL to
|
||||
# non-strict (`cmp`). Strict mode cannot see this class, so a source-level assertion is the
|
||||
# only available guard.
|
||||
#
|
||||
# TO REPRODUCE THE TABLE (30 seconds, no cluster) — it is a measurement, so re-measure it
|
||||
# after any FLUX_VERSION bump in infra/devcontainer/versions.env. Open the toolbox with
|
||||
# `./infra/devcontainer/dev.sh` and run these INSIDE it (do NOT pipe into `dev.sh run` from
|
||||
# the host: it starts the container with `-it`, so a piped stdin hangs):
|
||||
# printf '[$$] [$$$] [$${D}] [$$${D}] [DO $$ x $$;]\n' | D=VALUE flux envsubst
|
||||
# -> [$] [$$] [${D}] [$VALUE] [DO $ x $;]
|
||||
# And end-to-end, on the actual delivery path, with the pre-16b7fcd manifest checked out
|
||||
# (`git show 16b7fcd^:apps/wave-0/nextcloud-db.yaml > apps/wave-0/nextcloud-db.yaml`):
|
||||
# kubectl kustomize apps/wave-0 | grep 'DO \$' -> "DO $$" (the kubectl path)
|
||||
# kubectl kustomize apps/wave-0 | flux envsubst --strict | grep 'DO \$'
|
||||
# -> "DO $" (the FLUX path), exit 0
|
||||
# (`kubectl kustomize` IS the kustomize build here; the image ships no standalone binary.)
|
||||
# That two-command contrast IS the 2026-09-13 incident, and is why review missed it.
|
||||
# ${...}, ${a b}, ${ARR[0]} (UNESCAPED) -> HARD PARSE ERROR ("unable to parse
|
||||
# variable name" / "missing closing brace") which fails the
|
||||
# WHOLE Kustomization — every resource in the path, not just
|
||||
@@ -55,6 +141,12 @@
|
||||
# (5) every knob an overlay patches (`/data/KEY`) exists in the BASE ConfigMap —
|
||||
# the "add the knob with a base default BEFORE referencing it" rule that the
|
||||
# incident's interim discipline depends on;
|
||||
# (7) NO DOLLAR RUN IS SILENTLY REWRITTEN: the only run of length >= 2 that passes
|
||||
# unremarked is exactly `$$` immediately followed by `{` — the documented, total,
|
||||
# already-inventoried escape. EVERY other run of >= 2 dollars (a bare `$$`, an odd
|
||||
# `$$$`, an over-counted `$$$$`) is rewritten by envsubst between git and the
|
||||
# container, so it must be named and justified in `dollar-collapse.allow`, a file
|
||||
# this check also holds to being exact and non-stale;
|
||||
# AND (6) this check is LOAD-BEARING: it DECLARES `conformance.home.k3s/scope: whole-repo`,
|
||||
# so ci/l0.sh's `--include-scope whole-repo` runs it on every gate run, and
|
||||
# ci/gate.sh sets the merge block when the l0 stage fails.
|
||||
@@ -79,6 +171,21 @@
|
||||
# kustomize-controller. Both are cluster facts (L2), untested here. Tagging
|
||||
# R-GITOPS-01 claims ONLY the static slice: what git says a Kustomization renders
|
||||
# is fully determined, with no variable silently emptied on the way in.
|
||||
# - It CANNOT prove the thing that actually let 16b7fcd's bug reach a cluster: that a
|
||||
# change was VERIFIED through a substituting path. "It was hand-applied with kubectl" is
|
||||
# a fact about a shell session, not about any tracked file — git records the manifest,
|
||||
# never the command someone ran to try it, so the property is not statically decidable
|
||||
# here and no honest formulation of it exists. Nor is a proxy worth having: grepping
|
||||
# commit prose or runbooks for `kubectl apply` would be trivially satisfiable by
|
||||
# rewording, and would fire on the many legitimate break-glass runbooks whose whole
|
||||
# content is that command. So this check does not attempt it, and does not pretend to.
|
||||
# Assertion (7) answers that incident a different way: it moves the proof off the
|
||||
# verification PATH, which is unknowable, and onto the SOURCE, which is not — the
|
||||
# mangling is now caught by reading the manifest, whichever way a human chose to try it.
|
||||
# (The one decidable half-measure, `kustomize build <path> | flux envsubst --strict` per
|
||||
# substituted path in CI, is already ruled out above: the L0 toolchain installs neither
|
||||
# tool, so the stage would SKIP permanently. The dev image has both, and it is how the
|
||||
# arithmetic table above was measured.)
|
||||
# - It does NOT validate cluster-vars VALUES (that GATEWAY_VIP is inside the LB pool,
|
||||
# that an overlay's LAN block is self-consistent). Presence, not correctness.
|
||||
# - A cluster-vars key nothing references is reported as an ADVISORY, never a failure:
|
||||
@@ -108,6 +215,7 @@ definition of "what a Flux path actually renders" in the harness.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import bisect
|
||||
import os
|
||||
import re
|
||||
import sys
|
||||
@@ -231,9 +339,18 @@ LEN_RE = re.compile(r"^#([A-Za-z_][A-Za-z0-9_]*)$")
|
||||
|
||||
|
||||
def classify(dollars: int, inner: str) -> tuple[str, str] | None:
|
||||
"""Return (severity-tag, remediation) for a hazardous expansion, else None."""
|
||||
if dollars >= 2:
|
||||
return None # escaped: emitted as a literal
|
||||
"""Return (severity-tag, remediation) for a hazardous expansion, else None.
|
||||
|
||||
PARITY, not ">= 2" (the 2026-09-13 correction). envsubst consumes dollars in PAIRS and
|
||||
emits one per pair, so a run of N dollars in front of a brace leaves the brace ESCAPED
|
||||
only when N is EVEN. An ODD run emits floor(N/2) literal dollars and its last dollar is
|
||||
still LIVE, so `$$${X}` expands exactly as `${X}` does — and `$$${ARR[0]}` fails the
|
||||
whole Kustomization where `$${ARR[0]}` is harmless. The old `dollars >= 2 -> return None`
|
||||
called every such run "escaped" and stopped looking. Assertion (7) flags the run itself;
|
||||
this function additionally classifies the still-live expansion an odd run carries.
|
||||
"""
|
||||
if dollars % 2 == 0:
|
||||
return None # fully escaped: emitted as a literal
|
||||
if NAME_RE.match(inner):
|
||||
if inner in KEYS:
|
||||
return None # defined knob — the happy path
|
||||
@@ -268,6 +385,9 @@ def rendered_files(path_spec: str) -> tuple[list[str], bool]:
|
||||
findings: list[str] = []
|
||||
referenced: dict[str, set[str]] = {}
|
||||
inventory: list[str] = []
|
||||
# rel path -> the Kustomization name(s) that render it, for assertion (7)'s second pass.
|
||||
# A file can be rendered by more than one Kustomization; all of them substitute.
|
||||
subst_files: dict[str, set[str]] = {}
|
||||
|
||||
for k in kustomizations:
|
||||
files, exists = rendered_files(k["path"])
|
||||
@@ -281,13 +401,14 @@ for k in kustomizations:
|
||||
text = f.read_text()
|
||||
except (OSError, UnicodeDecodeError):
|
||||
continue
|
||||
subst_files.setdefault(rel, set()).add(k["name"])
|
||||
for lineno, line in enumerate(text.splitlines(), 1):
|
||||
for m in TOKEN_RE.finditer(line):
|
||||
dollars, inner = len(m.group(1)), m.group(2)
|
||||
verdict = classify(dollars, inner)
|
||||
if verdict is None:
|
||||
if dollars >= 2:
|
||||
n_esc += 1
|
||||
if dollars % 2 == 0: # PARITY, see classify(): an odd run is
|
||||
n_esc += 1 # live, not escaped
|
||||
else:
|
||||
n_def += 1
|
||||
referenced.setdefault(re.split(r"[:/#%^,]", inner)[0] or inner,
|
||||
@@ -313,6 +434,156 @@ for k in kustomizations:
|
||||
check(not findings,
|
||||
f"{len(findings)} unescaped, undefined or unparseable expansion(s) in substituted paths")
|
||||
|
||||
# ----------------------------------- (7) no dollar run is silently rewritten on the way out
|
||||
# THE 2026-09-13 CLASS. envsubst consumes dollars in PAIRS and emits one per pair,
|
||||
# unconditionally and everywhere (header: DOLLAR-RUN ARITHMETIC, measured against flux 2.4.0).
|
||||
# So every run of >= 2 dollars in a substituted manifest is REWRITTEN between git and the
|
||||
# container, and from the text alone the two intents are indistinguishable:
|
||||
#
|
||||
# (a) a deliberate escape the author WANTS collapsed — `$${VAR}` -> literal `${VAR}`;
|
||||
# (b) content that must arrive VERBATIM — `DO $$ ... $$;` -> `DO $ ... $;`
|
||||
# i.e. the incident. A BUG.
|
||||
#
|
||||
# THE RULE CHOSEN, AND WHY IT IS NEITHER NOISY NOR VACUOUS.
|
||||
# Exactly ONE shape passes unremarked: a run of EXACTLY TWO dollars immediately followed by
|
||||
# `{`. That is not a guess about intent, it is the escape contract this repo already
|
||||
# documents in clusters/base/apps-vms.yaml, that assertions (1)-(3) already validate the
|
||||
# inside of, and that 18 reviewed occurrences already rely on (unifi cloud-init, the gitea
|
||||
# and nextcloud OIDC bootstrap scripts, two longhorn comments). Failing those would be a
|
||||
# permanently-firing alert on documented, correct intent, and this repo has already been
|
||||
# bitten by one of those.
|
||||
#
|
||||
# EVERY OTHER run of >= 2 dollars FAILS unless it is named in `dollar-collapse.allow`:
|
||||
# * DOLLAR-COLLAPSE — a run not followed by `{`. Cannot survive verbatim: `$$` -> `$`.
|
||||
# This is the incident's exact shape and a blanket bug by default.
|
||||
# * DOLLAR-HALF-ESCAPED — an ODD run >= 3 before `{`. Looks escaped, is not: the last
|
||||
# dollar is live. Occasionally legitimate (`$$${PRICE}` to render a
|
||||
# literal `$` before a value), so it is allowlistable, not banned.
|
||||
# * DOLLAR-OVER-ESCAPED — an EVEN run >= 4 before `{`. Emits N/2 dollars then a literal
|
||||
# brace; nearly always a miscount, sometimes deliberate.
|
||||
#
|
||||
# WHY AN ALLOWLIST AND NOT A BLANKET BAN: a blanket ban was the first choice and was
|
||||
# rejected on evidence, not taste — main carries two real non-braced `$$` occurrences today
|
||||
# (prose inside embedded shell comments), so a blanket ban would have shipped RED. Nor is
|
||||
# the allowlist a hole: an entry is a hand-written edit to a file inside the conformance
|
||||
# test, which is loud in review; it must quote a >= 24-character VERBATIM context, so it can
|
||||
# only ever cover the occurrence it was written for and never a future one that drifts into
|
||||
# the same file; it must carry a >= 40-character reason; and every entry must still MATCH
|
||||
# something, so the allowlist cannot rot into a blanket exemption after the code it excused
|
||||
# is deleted. A wildcard, a bare `$$` context and a stale entry each FAIL.
|
||||
ALLOW_FILE = (ROOT / "conformance" / "tests" / "l0-static"
|
||||
/ "flux-postbuild-substitution" / "dollar-collapse.allow")
|
||||
MIN_CONTEXT = 24
|
||||
MIN_REASON = 40
|
||||
DOLLAR_RUN_RE = re.compile(r"\$+")
|
||||
|
||||
allow_entries: list[dict] = []
|
||||
check(ALLOW_FILE.is_file(),
|
||||
f"{ALLOW_FILE.relative_to(ROOT)} is missing — assertion (7) needs its allowlist to "
|
||||
f"exist even when empty, because an absent file and an empty file are the same green "
|
||||
f"and the next author would not know the mechanism is there")
|
||||
if ALLOW_FILE.is_file():
|
||||
for lineno, raw in enumerate(ALLOW_FILE.read_text().splitlines(), 1):
|
||||
if not raw.strip() or raw.lstrip().startswith("#"):
|
||||
continue
|
||||
parts = [x.strip() for x in raw.split("|")]
|
||||
if len(parts) != 3:
|
||||
check(False, f"dollar-collapse.allow:{lineno}: expected exactly 3 '|'-separated "
|
||||
f"fields (path | verbatim-context | reason), got {len(parts)}")
|
||||
continue
|
||||
allow_entries.append({"line": lineno, "path": parts[0], "ctx": parts[1],
|
||||
"reason": parts[2], "used": 0})
|
||||
|
||||
for e in allow_entries:
|
||||
tag = f"dollar-collapse.allow:{e['line']}"
|
||||
check((ROOT / e["path"]).is_file(),
|
||||
f"{tag}: names '{e['path']}', which is not a file in this repo — a stale entry is "
|
||||
f"a standing exemption for a path someone could recreate")
|
||||
check(len(e["ctx"]) >= MIN_CONTEXT,
|
||||
f"{tag}: context {e['ctx']!r} is {len(e['ctx'])} chars; at least {MIN_CONTEXT} are "
|
||||
f"required so an entry cannot be a near-wildcard that swallows future occurrences")
|
||||
check(DOLLAR_RUN_RE.search(e["ctx"]) is not None
|
||||
and max((len(m.group(0)) for m in DOLLAR_RUN_RE.finditer(e["ctx"])), default=0) >= 2,
|
||||
f"{tag}: context {e['ctx']!r} contains no run of two or more dollars, so it excuses "
|
||||
f"nothing and is either a typo or a decoy")
|
||||
check(len(e["reason"]) >= MIN_REASON,
|
||||
f"{tag}: reason is {len(e['reason'])} chars; at least {MIN_REASON} are required. "
|
||||
f"State what envsubst will actually emit and why that is acceptable — an entry "
|
||||
f"nobody had to explain is an accident wearing an approval")
|
||||
|
||||
RUN_TAGS = {
|
||||
"collapse": ("DOLLAR-COLLAPSE",
|
||||
"envsubst collapses each PAIR of dollars into one, everywhere and "
|
||||
"unconditionally, so this CANNOT reach the workload verbatim (a literal $$ "
|
||||
"needs $$$$ in source). This is the 2026-09-13 nextcloud-db shape: a "
|
||||
"dollar-quoted PL/pgSQL DO block arrived half-quoted and every Flux-"
|
||||
"reconciled run died. Rewrite without dollars, double them again if the "
|
||||
"collapse is what you want, or justify it in dollar-collapse.allow"),
|
||||
"half": ("DOLLAR-HALF-ESCAPED",
|
||||
"an ODD run of dollars before a brace is NOT escaped: floor(N/2) literal "
|
||||
"dollars are emitted and the LAST dollar is still live, so this expansion "
|
||||
"EVALUATES at reconcile time (and if its inner is unparseable it fails the "
|
||||
"entire Kustomization). Use an EVEN run to escape, or justify it in "
|
||||
"dollar-collapse.allow if a literal dollar before a substituted value is "
|
||||
"really what you meant"),
|
||||
"over": ("DOLLAR-OVER-ESCAPED",
|
||||
"an EVEN run of four or more dollars emits N/2 literal dollars followed by a "
|
||||
"LITERAL brace — the value is never substituted and extra dollars appear in "
|
||||
"the rendered manifest. Almost always a miscount of the escape; justify it in "
|
||||
"dollar-collapse.allow if it is not"),
|
||||
}
|
||||
|
||||
run_findings: list[str] = []
|
||||
n_runs_ok = 0
|
||||
for rel in sorted(subst_files):
|
||||
text = (ROOT / rel).read_text(errors="replace")
|
||||
# Absolute offsets, not per-line: the gitea/nextcloud bootstrap scripts are single
|
||||
# 4000-character folded scalars, so "the line" is not a reviewable unit of context here.
|
||||
nl = [i for i, ch in enumerate(text) if ch == "\n"]
|
||||
spans = [] # allowlisted [start, end) regions
|
||||
for e in allow_entries:
|
||||
if e["path"] != rel:
|
||||
continue
|
||||
start = text.find(e["ctx"])
|
||||
while start >= 0:
|
||||
spans.append((start, start + len(e["ctx"]), e))
|
||||
start = text.find(e["ctx"], start + 1)
|
||||
for m in DOLLAR_RUN_RE.finditer(text):
|
||||
n = len(m.group(0))
|
||||
if n < 2:
|
||||
continue
|
||||
braced = text[m.end():m.end() + 1] == "{"
|
||||
if n == 2 and braced:
|
||||
n_runs_ok += 1 # the documented escape; (1)-(3) own it
|
||||
continue
|
||||
covering = [e for (a, b, e) in spans if a <= m.start() and m.end() <= b]
|
||||
if covering:
|
||||
for e in covering:
|
||||
e["used"] += 1
|
||||
continue
|
||||
kind = "collapse" if not braced else ("half" if n % 2 else "over")
|
||||
tag, fix = RUN_TAGS[kind]
|
||||
lineno = bisect.bisect_right(nl, m.start()) + 1
|
||||
ctx = text[max(0, m.start() - 60):m.end() + 60].replace("\n", "\\n")
|
||||
run_findings.append(
|
||||
f" [{tag}] {rel}:{lineno} a run of {n} dollar(s)"
|
||||
f"{' before a brace' if braced else ''}\n"
|
||||
f" rendered by Flux Kustomization(s) {', '.join(sorted(subst_files[rel]))}"
|
||||
f" — postBuild substitution REWRITES it\n"
|
||||
f" {fix}\n"
|
||||
f" | ...{ctx}...")
|
||||
|
||||
check(not run_findings,
|
||||
f"{len(run_findings)} dollar run(s) in substituted paths that envsubst rewrites and "
|
||||
f"nothing justifies (the 2026-09-13 nextcloud-db class)")
|
||||
|
||||
for e in allow_entries:
|
||||
check(e["used"] > 0,
|
||||
f"dollar-collapse.allow:{e['line']}: STALE — its context is not present in "
|
||||
f"{e['path']} any more (or no longer contains a rewritten dollar run), so the entry "
|
||||
f"now excuses nothing. Delete it. A stale allowlist is how an opt-in decays into a "
|
||||
f"blanket exemption for whatever lands in that file next")
|
||||
|
||||
# --------------------------------------------- (5) overlay knobs exist in the base
|
||||
# Parse contract: overlays patch cluster-vars with JSON6902 ops (`- op: replace` /
|
||||
# `path: /data/KEY`) inside an inline `patch: |-` block. The repo rule (cluster-vars.yaml
|
||||
@@ -363,6 +634,10 @@ print("Flux postBuild substitution inventory (per Kustomization):")
|
||||
for line in inventory:
|
||||
print(line)
|
||||
print(f"\ncluster-vars knobs defined: {len(KEYS)} referenced: {len(referenced)}")
|
||||
print(f"dollar runs >=2 in substituted files: {n_runs_ok} documented $${{...}} escapes, "
|
||||
f"{sum(e['used'] for e in allow_entries)} justified by dollar-collapse.allow "
|
||||
f"({len(allow_entries)} entr{'y' if len(allow_entries) == 1 else 'ies'}), "
|
||||
f"{len(run_findings)} unjustified")
|
||||
|
||||
if ADVISORIES:
|
||||
print("\nADVISORIES (not failures):")
|
||||
@@ -374,6 +649,12 @@ if findings:
|
||||
for f in findings:
|
||||
print(f, file=sys.stderr)
|
||||
|
||||
if run_findings:
|
||||
print("\nHAZARDS — dollar runs envsubst rewrites between git and the container:",
|
||||
file=sys.stderr)
|
||||
for f in run_findings:
|
||||
print(f, file=sys.stderr)
|
||||
|
||||
print()
|
||||
if not FAILURES:
|
||||
print(f"check OK: flux-postbuild-substitution — {CHECKS} assertions "
|
||||
|
||||
@@ -0,0 +1,52 @@
|
||||
# =============================================================================
|
||||
# dollar-collapse.allow — the reviewed exceptions to assertion (7) of
|
||||
# conformance/tests/l0-static/flux-postbuild-substitution/check.sh.
|
||||
#
|
||||
# WHY THIS FILE EXISTS
|
||||
# --------------------
|
||||
# Flux's postBuild substitution runs github.com/fluxcd/pkg/envsubst (Flux's fork of
|
||||
# github.com/drone/envsubst; the module path is embedded in the pinned flux 2.4.0
|
||||
# binary, and check.sh's header says how to read it back out) over everything a
|
||||
# substituted Kustomization renders, and that scanner consumes dollars in PAIRS,
|
||||
# emitting ONE dollar per pair — unconditionally, wherever they appear, not only in
|
||||
# front of a brace. A run of N dollars becomes floor(N/2) dollars, and an ODD run
|
||||
# leaves one live dollar behind (so `$$${X}` is NOT escaped; it expands).
|
||||
#
|
||||
# Consequence: there is NO way to ship a literal `$$` through a substituted path.
|
||||
# On 2026-09-13 that broke the nextcloud-db grant reconciler — a dollar-quoted
|
||||
# PL/pgSQL `DO $$ ... $$;` block reached Postgres as `DO $ ... $;` and every
|
||||
# Flux-reconciled run died with `syntax error at or near "$"`. It had passed review
|
||||
# because it was hand-applied with `kubectl apply`, which runs no envsubst at all.
|
||||
# See commit 16b7fcd and check.sh's DOLLAR-RUN ARITHMETIC table (measured against
|
||||
# flux 2.4.0, not inferred).
|
||||
#
|
||||
# From the source text alone, a doubled dollar the author WANTS collapsed and one
|
||||
# that must arrive verbatim are indistinguishable. check.sh therefore accepts exactly
|
||||
# one shape without comment — a run of EXACTLY TWO dollars immediately followed by
|
||||
# `{`, the escape contract clusters/base/apps-vms.yaml already documents and that 18
|
||||
# reviewed occurrences already depend on — and FAILS every other run of two or more
|
||||
# dollars unless it is named here.
|
||||
#
|
||||
# ADDING AN ENTRY IS A DECISION, NOT A MUTE BUTTON
|
||||
# -----------------------------------------------
|
||||
# One entry per line, exactly three `|`-separated fields:
|
||||
#
|
||||
# <repo-relative path> | <verbatim context> | <reason>
|
||||
#
|
||||
# * the context must be a VERBATIM substring of the file AS STORED IN GIT (for a
|
||||
# folded/JSON-ish scalar that means the escaped bytes, e.g. `\n` and `—`
|
||||
# spelled literally), at least 24 characters long, containing the dollar run it
|
||||
# excuses, and containing no `|` — shift the window if a pipe is in the way;
|
||||
# * the reason must be at least 40 characters and should say what envsubst will
|
||||
# actually EMIT and why that is acceptable;
|
||||
# * an entry covers only the dollar runs inside the region its context matches, so
|
||||
# it can never silently cover a new occurrence that lands elsewhere in the file;
|
||||
# * check.sh FAILS a stale entry — one whose context no longer matches, or which
|
||||
# excuses nothing any more. Delete it rather than leaving it to rot into a
|
||||
# blanket exemption for whatever lands in that file next.
|
||||
#
|
||||
# If the collapse would change BEHAVIOUR, this file is the wrong answer. Rewrite the
|
||||
# content without dollars (16b7fcd) or double them again (`$$$$` -> `$$`).
|
||||
# =============================================================================
|
||||
apps/wave-0/gitea.yaml | # NOTE the `$$` escape below \u2014 it is load-bearing. apps-wave-0 declares postBuild. | Prose, not code: this is a comment INSIDE the gitea OIDC bootstrap shell script, and the two dollars are the sentence quoting the escape idiom it goes on to explain. envsubst renders it as "the `$` escape below", which is cosmetically wrong and behaviourally inert — a shell comment. Cannot be written correctly at all: `$$$$` would render `$$` but would then be wrong in the source a reader sees in git, and the file has no non-dollar way to name the idiom. Reviewed and accepted as cosmetic.
|
||||
apps/wave-1/nextcloud.yaml | # NOTE the `$$` escape below \u2014 it is load-bearing. apps-wave-1 declares postBuild. | Identical case to the gitea entry above, in the nextcloud OIDC bootstrap script: a shell comment quoting the escape idiom, rendered by envsubst as "the `$` escape below". Cosmetic only — the collapse happens inside a `#` comment line that no interpreter reads, and the escaped expansion the comment is describing is on a later line and is unaffected. Reviewed and accepted as cosmetic.
|
||||
Reference in new issue
Block a user