satisfactory: server.storage stays the documented exception (#39)
Publish Helm Chart with patched Application Version / publish-helm-chart (push) Successful in 6s
Publish Satisfactory Helm Chart / Publish Satisfactory Helm Chart (push) Successful in 7s

Walks back the one part of the previous commit that changed rendered output:
storage was pulled into the server fallback, so an erased size fell back to
20Gi instead of rendering a PVC without one.

values.schema.json had documented the opposite decision - "an erased value
yields an invalid PVC rather than a silently degraded workload" - and that is
someone's reasoned trade, not collateral of the nil-pointer bug next to it.
Overturning it inside a patch would have made this the only release where
valid values render differently, and it would have settled an open design
question in a PR nobody opened for that. It goes to #39 instead.

So storage is out of the defaults again and the schema note is restored
verbatim, extended only by where the exception is argued. values.yaml marks
it as the one key of the block that is deliberately not null-safe. The
README rule that had leaned on this example now says the thing that actually
holds: an exception is fine, an unmarked one is not.

Everything else stands - dev and server null-safe, the single-key findings on
the ports, formatNumber against exponential notation, 7.0.1.

Re-measured after the walk-back: rendered output against the chart defaults
and against a full override set is byte-identical to 7.0.0, NOTES.txt
included, and "--set server=null" now renders exactly the sizeless PVC that
"--set server.storage=null" rendered on 7.0.0.
This commit was merged in pull request #37.
This commit is contained in:
jklein authored and Justin Klein committed 2026-09-23 20:45:24 +00:00
1 parent 5ff6e401c7
commit 4715d85b99
4 files changed
+34 -19

No files matched your search

+3 -1
View File
@@ -344,7 +344,9 @@ Reference implementation: `template-chart/values.schema.json` and `template-char
**2. Probes, security contexts and resources go through the null-safe helper, never behind an `if`.** An override written without children is YAML null and erases the block: the container renders `securityContext: null` (unhardened, healthy, silent), a probe disappears, or `resources` loses its requests and computed limits. The `defaultedDict` helper defines the opposite semantics — an empty block means "chart defaults apply" — while an explicitly set value still wins, **including `false` and `0`** (hence the `kindIs "invalid"` check; `default` and `mergeOverwrite` both swallow zero values). Do **not** wrap these blocks in `{{- if }}`: a guard does not render a visible `null`, it removes the probe entirely, which is the same bug with no evidence left.
The rule is about the **shape, not about those three block kinds**. `satisfactory` shipped 7.0.0 with `game` null-safe and its two neighbours not (Gitea #16): `--set server=null` aborted the render, and a single erased key was the worse half — a nulled `server.gamePort` rendered `port:` and `nodePort:` empty, a nulled `server.storage` a PVC asking for no size, and both are rejected by nothing until the API server sees them. Every chart-owned block a template dereferences belongs behind the helper, and a chart where only *some* of them do is worse than one where none do: whoever sees `game=null` fall back cleanly assumes `server=null` does too. A block that is deliberately left un-defaulted has to say so in the schema *and* survive the check that the erased form still renders something an operator can read — `satisfactory.storage` carried exactly such a note ("an erased value yields an invalid PVC rather than a silently degraded workload") and it did not hold, because it made the *whole block* being erased produce a sizeless PVC.
The rule is about the **shape, not about those three block kinds**. `satisfactory` shipped 7.0.0 with `game` null-safe and its two neighbours not (Gitea #16): `--set server=null` aborted the render, and a single erased key was the quieter half — a nulled `server.gamePort` rendered `port:` and `nodePort:` empty into the Service and an empty `containerPort` into the StatefulSet, rejected by nothing until the API server saw it. Every chart-owned block a template dereferences belongs behind the helper, and a chart where only *some* of them do is worse than one where none do: whoever sees `game=null` fall back cleanly assumes `server=null` does too.
**A deliberate exception is fine; an undocumented one is the bug.** `satisfactory.storage` is left un-defaulted on purpose — an erased size yields an invalid PVC that fails at apply time rather than a volume quietly sized by the chart — so `server: null` still renders a PVC without a size. That is a defensible trade and it is *not* the null-safety bug next to it; what makes it safe to leave standing is that the schema, `values.yaml` and the chart page all name it as an exception and point at the issue where it is argued (Gitea #39). Note also where the trade actually bites: only on a **new** install. For an existing release a fallback to the default size would fail too, because a PVC cannot shrink. Do not overturn a reasoned exception inside a bugfix patch — the patch then stops being one.
**And `helm template` does not render `NOTES.txt`.** A value read there — `server` is read in four files in that chart — stays broken after a fix that looks green in every render, until someone runs a real `helm install`. `helm install --dry-run=client` is not the way around it either: it still wants a reachable API server (measured on helm v3.22.0). What works without a cluster: in a throwaway copy of the chart, wrap the verbatim `NOTES.txt` body in a `{{- define }}` and `include` it into a scratch ConfigMap, which `helm template` *does* render — same engine, same root context, only the sink differs.