docs: research findings — measured Flux substitution semantics + ranked remediation plan
Records three parallel research passes and, importantly, TWO CORRECTIONS.
Correction 1 (mine): StrictPostBuildSubstitutions alone is INSUFFICIENT — it fixes nothing
about ordering. The actual fix is one line: platform.dependsOn: [flux-system]. Verified
against kustomization_controller.go::checkDependencies at our exact version — dependsOn is
REVISION-gated, not merely Ready-gated, so a dependent sharing a GitRepository cannot
reconcile revision N until the dependency has APPLIED revision N.
Correction 2 (a research agent's): the claim that Helm considered the bad release healthy,
so remediation never fired, is WRONG — measured on the live cluster, the Helm release
secrets show v9 = failed. The machinery did fire. maxUnavailable: 2 of 3 IS confirmed, so
the real harm was blast radius, not missed detection.
Adds a MEASURED substitution-semantics table, including the two anti-patterns nobody had
written down: ${VAR:?msg} renders the MESSAGE as the value, and ${VAR:=default} silently
ships the dev value to prod. Both are exempt from strict mode by design — which is exactly
why strict would never have caught the gitea/nextcloud dead-guard bug.
Also corrects repo doctrine: escaping is TOTAL ($${ARR[0]} is safe); only the unescaped
indexed form is fatal. And qualifies an earlier overstatement in this log: kustomize strips
structural YAML comments before envsubst, so the hazard is block-scalar bodies, not all
comments.
Notes Flux 2.4.0 is out of support and that strict became default-on in kustomize-controller
v1.9.0 — so the audit is a prerequisite for the upgrade regardless.
Ranked plan P0-P5 with what is done, what is ready, and what needs an operator call.
This commit is contained in:
jakman85 committed
1 parent
8a2be824f6
commit
a344b046c9
1 file changed
+112
@@ -1582,3 +1582,115 @@ A Cilium rolling upgrade on this cluster is not risk-free even when the values a
|
||||
`config` init container resolves the API server through the ClusterIP unless `k8sServiceHost` is
|
||||
set, and the agent that translates that ClusterIP is the one being replaced. Keep
|
||||
`k8sServiceHost`/`k8sServicePort` correct at all times, and roll Cilium one node at a time.
|
||||
|
||||
---
|
||||
|
||||
## 2026-09-01 — RESEARCH: Flux substitution + HelmRelease best practice. **Measured semantics + ranked plan.**
|
||||
|
||||
Three parallel research passes (substitution/ordering, HelmRelease/CNI, and a full repo audit),
|
||||
prompted by the incident above. Two of my own earlier conclusions were WRONG and are corrected
|
||||
here. Everything below marked *measured* was run against flux 2.4.0 / the live cluster; everything
|
||||
else is cited inference.
|
||||
|
||||
### THE SUBSTITUTION SEMANTICS TABLE (measured — trust this over prose elsewhere)
|
||||
|
||||
| form | behaviour |
|
||||
|---|---|
|
||||
| `${DEFINED}` | substituted |
|
||||
| `${UNDEFINED}` | **silently empty** (non-strict) · fails loudly (strict) |
|
||||
| `${UNDEFINED:-d}` | **default applied at reconcile time — `--strict` does NOT catch it** |
|
||||
| `${VAR:?msg}` | renders **the message as the value** — a non-empty WRONG value. ANTI-PATTERN. |
|
||||
| `${VAR:=192.168.1.50}` | silently ships the dev value to prod. ANTI-PATTERN. |
|
||||
| `$${VAR}` / `$${VAR:-d}` / `$${ARR[0]}` | literal — **escaping is TOTAL** |
|
||||
| `$VAR` / `$(cmd)` | untouched, no escaping needed |
|
||||
| `${...}` / `${a b}` / `${ARR[0]}` unescaped | **hard parse error — fails the whole Kustomization** |
|
||||
|
||||
Flux exempts default-providing operators (`-`, `:-`, `+`, `:+`, `=`, `:=`, `?`, `:?`) from strict
|
||||
mode by design ("they intentionally handle the unset case"). **That is why strict mode would never
|
||||
have caught the gitea/nextcloud dead-guard bug**, and why a targeted check was required.
|
||||
|
||||
**Doctrine corrected:** `clusters/base/apps-vms.yaml` claimed an indexed expansion is fatal
|
||||
"escaped or not". Measured: escaping is total; only the UNESCAPED form is fatal. Fixed in place.
|
||||
|
||||
**Comment nuance:** `kustomize build` strips *structural* YAML comments before envsubst sees them,
|
||||
so those are usually harmless. But `#` lines INSIDE a block scalar are data and DO reach envsubst,
|
||||
and a `$`-carrying body comment was observed surviving into built `apps/wave-1`. Do not rely on
|
||||
stripping — escape in comments too. (An earlier claim in this log that a YAML comment "nearly broke
|
||||
every merge" was overstated for the structural case; the hazard is real for block-scalar bodies.)
|
||||
|
||||
### CORRECTION 1 (mine) — `StrictPostBuildSubstitutions` alone is INSUFFICIENT
|
||||
|
||||
It fixes nothing about ordering; it converts "render a corrupt value and ship it" into "fail loudly
|
||||
and keep last-good". **The actual ordering fix is one line**, verified against
|
||||
`kustomization_controller.go :: checkDependencies` at our exact version (v1.4.0): `dependsOn` is
|
||||
**revision-gated**, not merely Ready-gated — when the dependency shares a GitRepository, the
|
||||
dependent cannot reconcile revision N until the dependency has APPLIED revision N.
|
||||
|
||||
`platform` is the only Kustomization in the graph with no `dependsOn`; every other consumer chains
|
||||
transitively through it. So:
|
||||
|
||||
```yaml
|
||||
# clusters/base/platform.yaml
|
||||
spec:
|
||||
dependsOn:
|
||||
- name: flux-system # cluster-vars must be applied at THIS revision first
|
||||
```
|
||||
|
||||
**Cost, recorded honestly:** the root becomes a gate — a malformed manifest anywhere under
|
||||
`clusters/dev` now stops the whole graph where today it would not. Fail-closed, consistent with
|
||||
doctrine, but a real blast-radius change. No deadlock risk: the root runs `wait: false`.
|
||||
|
||||
Rejected alternative: moving `cluster-vars` into the consuming Kustomization is **strictly worse** —
|
||||
`SubstituteVariables` reads the ConfigMap from the API at build time, *before* the Kustomization
|
||||
applies its own resources, so it would always read the PREVIOUS value. That converts an occasional
|
||||
race into a permanent one-reconcile lag.
|
||||
|
||||
### CORRECTION 2 (an agent's) — the safety machinery DID fire
|
||||
|
||||
A research pass concluded that Helm considered the bad release healthy (because
|
||||
`updateStrategy.rollingUpdate.maxUnavailable: 2` on 3 nodes means `expectedReady = 1`), and
|
||||
therefore remediation never engaged. **Measured on the live cluster: that is wrong.** The Helm
|
||||
release secrets show **v9 = `failed`**; Helm's wait DID fail and the release was remediated, which
|
||||
is why it self-healed. `maxUnavailable: 2 of desired 3` IS confirmed live — so the real harm was
|
||||
blast radius (2 of 3 agents down at once), not a missed detection.
|
||||
|
||||
### Verified, and worth knowing
|
||||
|
||||
- **Flux 2.4.0 is OUT OF SUPPORT** — the project supports the last three minors; 2.9.x is current.
|
||||
- **Strict substitution became DEFAULT-ON** in kustomize-controller v1.9.0. So upgrading Flux flips
|
||||
this behaviour whether or not we opt in — the variable audit is a prerequisite for the upgrade
|
||||
regardless of today's decision.
|
||||
- **Cilium's `values.schema.json` permits `k8sServiceHost: ""`** — no `minLength`, no `pattern`.
|
||||
Helm was never going to reject the bad render.
|
||||
- **Cilium docs make `k8sServiceHost` REQUIRED under kube-proxy replacement**, precisely because the
|
||||
environment exports the API ClusterIP and there is no kube-proxy to provision it.
|
||||
- **The Cilium chart ships zero test templates**, so `spec.test` is useless for it. A post-upgrade
|
||||
check must live outside Helm.
|
||||
- `valuesFrom` fails HARD on a missing key (verified in `chartutil/values.go`) unlike postBuild's
|
||||
silent empty — but only on a MISSING key, not an empty value, and it cannot cross namespaces
|
||||
(`cluster-vars` is in `flux-system`; the Cilium HelmRelease is in `kube-system`).
|
||||
- **Do NOT enable `driftDetection.mode: enabled` on a CNI.** It compares the cluster against the
|
||||
Helm storage manifest — during the incident that manifest WAS the bad render, so drift correction
|
||||
would have actively re-asserted the broken value.
|
||||
|
||||
### RANKED PLAN
|
||||
|
||||
| P | Action | State |
|
||||
|---|---|---|
|
||||
| **P0** | `platform.dependsOn: [flux-system]` — closes the race completely | **PENDING — operator call** (blast-radius change) |
|
||||
| **P0** | Escape the dead OIDC guards; renderer now refuses the class | ✅ DONE |
|
||||
| **P1** | Substitution-safety L0 gate, unconditional in `ci/l0.sh` | ✅ DONE — 102 assertions |
|
||||
| **P1** | Enable `StrictPostBuildSubstitutions=true` | **READY — proven safe**, all 11 paths exit 0 under `flux envsubst --strict`. Sequence AFTER P0. |
|
||||
| **P2** | `updateStrategy.rollingUpdate.maxUnavailable: 1` on Cilium, as a new **[parity]** field in BOTH the HelmRelease and the Ansible j2 | PENDING — bounds blast radius to one node |
|
||||
| **P3** | Split Cilium into its own Kustomization (`wait: true`, `interval: 1h`, `timeout: 10m`); `platform` dependsOn it | PENDING — gives `flux suspend kustomization cilium` as an emergency valve |
|
||||
| **P4** | Upgrade Flux 2.4.0 → 2.9.x | PENDING — out of support; brings strict-by-default |
|
||||
| **P5** | Cilium MINOR bumps become manual (Renovate PR → human runs the documented pre-flight → merge). Never automerge a CNI minor. | PENDING — process |
|
||||
| — | Record `${VAR:?}` / `${VAR:=}` as anti-patterns | ✅ DONE (table above) |
|
||||
| — | *Not recommended*: drift detection on Cilium; `spec.test` on Cilium; `k8sServiceHost: auto` | — |
|
||||
|
||||
### Follow-ups surfaced, not yet actioned
|
||||
- 6 dead `cluster-vars` knobs; `BASE_DOMAIN` and `CLUSTER_ENV` look like genuine rot.
|
||||
- `platform/{cilium,longhorn,velero,oauth2-proxy}/values.yaml` reference mirrors carry decorative
|
||||
`${...}` that can silently drift from the real HelmRelease.
|
||||
- `platform` Kustomization `timeout: 5m` vs Longhorn HelmRelease `timeout: 15m` — the tier can time
|
||||
out while a legitimate Longhorn upgrade is still running.
|
||||
Reference in new issue
Block a user