From 57e54f93fcb9273d783723f07abd37bd643f2d99 Mon Sep 17 00:00:00 2001 From: Justin Klein Date: Tue, 22 Sep 2026 19:59:35 +0000 Subject: [PATCH 1/2] satisfactory: dev and server fall back instead of aborting (7.0.1) --set dev=null aborted the render at .Values.dev.debug and --set server=null at .Values.server.replicas, while the third block, game, has fallen back to the chart defaults since 7.0.0. Half-safe is the worse state of the two: whoever sees game=null work assumes server=null does too. Both blocks now run through satisfactory.defaultedDict like game, via two new helpers next to it. server.resources is deliberately NOT re-defaulted there - it already runs through satisfactory.effectiveResources, which carries those defaults and is null-safe on its own, and a second copy of limitFactor: 2 / 4Gi / 1 / 5Gi would be a third place for them to drift. server is read in four files, not one, and the fourth is NOTES.txt, which helm template never renders - a fix confined to the manifests would have looked green and still aborted on a real helm install. All four go through the helpers. The single erased key was the quieter half of the bug and is fixed with it: a nulled server.gamePort rendered "port:" and "nodePort:" empty into the Service and an empty containerPort into the StatefulSet, and a nulled server.storage a PVC asking for no size - manifests rejected by nothing until the API server saw them. server.storage was documented in values.schema.json as deliberately not null-safe; that reasoning does not survive the block-level case, because it made "server: null" render a sizeless PVC instead of falling back like everything else. The schema note is replaced rather than left lying. Rendered output for valid values is unchanged: helm template against the chart defaults and against a full override set is byte-identical to 7.0.0, and so is NOTES.txt. Hence a patch bump, appVersion untouched. Fixes #16 --- README.md | 4 ++ satisfactory/Chart.yaml | 2 +- satisfactory/templates/NOTES.txt | 16 ++++--- satisfactory/templates/_helpers.tpl | 48 +++++++++++++++++++ satisfactory/templates/satisfactory.pvc.yaml | 3 +- .../templates/satisfactory.service.yaml | 21 ++++---- satisfactory/templates/satisfactory.sfs.yaml | 18 +++---- satisfactory/values.schema.json | 21 ++++---- satisfactory/values.yaml | 10 ++++ 9 files changed, 109 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index c8d5337..bd44c01 100644 --- a/README.md +++ b/README.md @@ -344,6 +344,10 @@ 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. + + **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. + **3. Do not hang liveness on an external dependency where a dependency-free path exists.** `zipline` deliberately probes `/` for liveness and `/api/healthcheck` only for readiness and startup, so a database outage cannot restart the pod. Where no such path exists — `outline` returns 500 on `/` when the database is down — all three probes may share the endpoint, but the reason belongs in a comment on the helper, or the next reader will "fix" the inconsistency. The other half of the same rule: readiness **must** hang on the dependency, and a separate path is not proof that it does. `urlaubsverwaltung` probed `/actuator/health/liveness` and `/actuator/health/readiness` for two years and still stayed 1/1 Ready through a total database outage (#218), because Spring Boot's `readiness` group contains `readinessState` **alone** — the split paths were right, the group content was empty of anything external. Whatever aggregates behind a probe path (Spring health groups, a `/healthz` a framework assembles for you) has to be read off the **running** endpoint, not off the path name; the chart names the members explicitly (`MANAGEMENT_ENDPOINT_HEALTH_GROUP_READINESS_INCLUDE=readinessState,db`) so the composition is in the template rather than in an upstream default. diff --git a/satisfactory/Chart.yaml b/satisfactory/Chart.yaml index 6c7f7bf..2fdfad3 100644 --- a/satisfactory/Chart.yaml +++ b/satisfactory/Chart.yaml @@ -3,7 +3,7 @@ name: satisfactory description: A Helm chart to deploy a Satisfactory Gameserver on Kubernetes icon: https://raw.githubusercontent.com/wolveix/satisfactory-server/main/.github/logo.png type: application -version: 7.0.0+upv1.9.10 +version: 7.0.1+upv1.9.10 appVersion: "v1.9.10" maintainers: - name: jklein \ No newline at end of file diff --git a/satisfactory/templates/NOTES.txt b/satisfactory/templates/NOTES.txt index dd18b72..f00465d 100644 --- a/satisfactory/templates/NOTES.txt +++ b/satisfactory/templates/NOTES.txt @@ -1,17 +1,19 @@ -{{- $replicas := include "satisfactory.replicas" (dict "scaleDown" .Values.scaleDown "replicas" .Values.server.replicas) -}} -{{- $gamePort := int .Values.server.gamePort -}} -{{- $msgPort := int .Values.server.messagingPort -}} +{{- $server := fromYaml (include "satisfactory.server" .Values.server) -}} +{{- $dev := fromYaml (include "satisfactory.dev" .Values.dev) -}} +{{- $replicas := include "satisfactory.replicas" (dict "scaleDown" .Values.scaleDown "replicas" (include "satisfactory.formatNumber" $server.replicas)) -}} +{{- $gamePort := int $server.gamePort -}} +{{- $msgPort := int $server.messagingPort -}} {{- $svc := printf "%s-%s-service" .Release.Name .Chart.Name -}} {{- $sfs := printf "%s-%s-sfs" .Release.Name .Chart.Name -}} {{- $pod := printf "%s-%s-sfs-0" .Release.Name .Chart.Name -}} {{- $pvc := printf "%s-%s-vc" .Release.Name .Chart.Name -}} -{{- $storage := .Values.server.storage | default "unset" -}} -{{- $res := fromYaml (include "satisfactory.effectiveResources" .Values.server.resources) -}} +{{- $storage := $server.storage | default "unset" -}} +{{- $res := fromYaml (include "satisfactory.effectiveResources" $server.resources) -}} {{- $lim := $res.limits | default dict -}} {{- $req := $res.requests | default dict -}} {{- $eph := index $lim "ephemeral-storage" | default "the computed ephemeral-storage limit" -}} {{- $mem := index $lim "memory" | default "the computed memory limit" -}} -{{- $factor := (.Values.server.resources | default dict).limitFactor | default 2 -}} +{{- $factor := ($server.resources | default dict).limitFactor | default 2 -}} {{- $startup := fromYaml (include "satisfactory.startupProbe" .Values.startupProbe) -}} {{- $budget := "" -}} {{- if and (kindIs "float64" $startup.periodSeconds) (kindIs "float64" $startup.failureThreshold) -}} @@ -21,7 +23,7 @@ {{- $seasonalEnv := include "satisfactory.disableSeasonalEvents" $game.enableSeasonalEvents -}} {{- $seasonalOff := eq $seasonalEnv "true" -}} {{- $beta := eq (lower (printf "%v" $game.experimental)) "true" -}} -{{- $debug := eq (lower (printf "%v" .Values.dev.debug)) "true" -}} +{{- $debug := eq (lower (printf "%v" $dev.debug)) "true" -}} {{- $outOfRange := or (lt $gamePort 30000) (gt $gamePort 32767) (lt $msgPort 30000) (gt $msgPort 32767) -}} {{- $skipRaw := "" -}} {{- range $entry := .Values.env -}} diff --git a/satisfactory/templates/_helpers.tpl b/satisfactory/templates/_helpers.tpl index 2430341..6d6a3df 100644 --- a/satisfactory/templates/_helpers.tpl +++ b/satisfactory/templates/_helpers.tpl @@ -308,6 +308,54 @@ DISABLESEASONALEVENTS at "true", the value this chart has always rendered. {{- include "satisfactory.defaultedDict" (dict "defaults" $defaults "given" . "path" "game") -}} {{- end -}} +{{/* +The effective dev block (Gitea issue #16, the rest of #99). + +"--set dev=null" — or a leftover "dev:" whose last sub-key was deleted — used +to abort the render with a nil-pointer error at .Values.dev.debug, in the +StatefulSet and in NOTES.txt. It now falls back to the default values.yaml +documents, the same way the game block has since 7.0.0. + +The defaults below MUST stay in sync with the dev block in values.yaml, which +stays the documented, user-facing place for them; this copy is only the +fallback for the case where an override has erased the block. +*/}} +{{- define "satisfactory.dev" -}} +{{- $defaults := dict + "debug" false -}} +{{- include "satisfactory.defaultedDict" (dict "defaults" $defaults "given" . "path" "dev") -}} +{{- end -}} + +{{/* +The effective server block (Gitea issue #16, the rest of #99). + +"--set server=null" used to abort the render at .Values.server.replicas, and a +single erased sub-key was worse than that: it rendered silently. A nulled +server.gamePort produced a Service with "port:" and "nodePort:" empty and a +StatefulSet with an empty containerPort — a manifest the API server rejects +only at apply time — and a nulled server.storage produced a PVC asking for no +size at all. Both now fall back to the documented defaults instead. + +Note what is NOT defaulted here: server.resources. That block already runs +through satisfactory.effectiveResources, which carries its own defaults and is +null-safe on its own; repeating limitFactor/4Gi/1/5Gi here would create a +third place for them to drift apart. Keys the defaults do not mention are +passed through by satisfactory.defaultedDict unchanged, so an erased +"server:" simply leaves resources unset and effectiveResources supplies them. + +The defaults below MUST stay in sync with the server block in values.yaml, +which stays the documented, user-facing place for them; this copy is only the +fallback for the case where an override has erased the block. +*/}} +{{- define "satisfactory.server" -}} +{{- $defaults := dict + "replicas" 1 + "gamePort" 30777 + "messagingPort" 30888 + "storage" "20Gi" -}} +{{- include "satisfactory.defaultedDict" (dict "defaults" $defaults "given" . "path" "server") -}} +{{- end -}} + {{/* Render the DISABLESEASONALEVENTS env value from game.enableSeasonalEvents (issue #215). diff --git a/satisfactory/templates/satisfactory.pvc.yaml b/satisfactory/templates/satisfactory.pvc.yaml index af309a7..4b5c2c7 100644 --- a/satisfactory/templates/satisfactory.pvc.yaml +++ b/satisfactory/templates/satisfactory.pvc.yaml @@ -1,3 +1,4 @@ +{{- $server := fromYaml (include "satisfactory.server" .Values.server) -}} apiVersion: v1 kind: PersistentVolumeClaim metadata: @@ -7,5 +8,5 @@ spec: - ReadWriteOnce resources: requests: - storage: {{ .Values.server.storage }} + storage: {{ $server.storage }} storageClassName: longhorn diff --git a/satisfactory/templates/satisfactory.service.yaml b/satisfactory/templates/satisfactory.service.yaml index e9d3b2f..8d146a5 100644 --- a/satisfactory/templates/satisfactory.service.yaml +++ b/satisfactory/templates/satisfactory.service.yaml @@ -1,3 +1,6 @@ +{{- $server := fromYaml (include "satisfactory.server" .Values.server) -}} +{{- $gamePort := include "satisfactory.formatNumber" $server.gamePort -}} +{{- $msgPort := include "satisfactory.formatNumber" $server.messagingPort -}} apiVersion: v1 kind: Service metadata: @@ -8,19 +11,19 @@ spec: type: NodePort ports: - name: "api" - port: {{ .Values.server.gamePort }} + port: {{ $gamePort }} protocol: TCP - nodePort: {{ .Values.server.gamePort }} - targetPort: {{ .Values.server.gamePort }} + nodePort: {{ $gamePort }} + targetPort: {{ $gamePort }} - name: "game" - port: {{ .Values.server.gamePort }} + port: {{ $gamePort }} protocol: UDP - nodePort: {{ .Values.server.gamePort }} - targetPort: {{ .Values.server.gamePort }} + nodePort: {{ $gamePort }} + targetPort: {{ $gamePort }} - name: "messaging" - port: {{ .Values.server.messagingPort }} + port: {{ $msgPort }} protocol: TCP - nodePort: {{ .Values.server.messagingPort }} - targetPort: {{ .Values.server.messagingPort }} + nodePort: {{ $msgPort }} + targetPort: {{ $msgPort }} selector: app: {{ .Release.Name }}-{{ .Chart.Name }}-sfs diff --git a/satisfactory/templates/satisfactory.sfs.yaml b/satisfactory/templates/satisfactory.sfs.yaml index 5826383..9b43889 100644 --- a/satisfactory/templates/satisfactory.sfs.yaml +++ b/satisfactory/templates/satisfactory.sfs.yaml @@ -1,4 +1,6 @@ {{- $game := fromYaml (include "satisfactory.game" .Values.game) -}} +{{- $dev := fromYaml (include "satisfactory.dev" .Values.dev) -}} +{{- $server := fromYaml (include "satisfactory.server" .Values.server) -}} apiVersion: apps/v1 kind: StatefulSet metadata: @@ -10,7 +12,7 @@ spec: matchLabels: app: {{ .Release.Name }}-{{ .Chart.Name }}-sfs serviceName: {{ .Release.Name }}-{{ .Chart.Name }}-service - replicas: {{ include "satisfactory.replicas" (dict "scaleDown" .Values.scaleDown "replicas" .Values.server.replicas) }} + replicas: {{ include "satisfactory.replicas" (dict "scaleDown" .Values.scaleDown "replicas" (include "satisfactory.formatNumber" $server.replicas)) }} template: metadata: labels: @@ -24,7 +26,7 @@ spec: image: "ghcr.io/wolveix/satisfactory-server:{{ .Chart.AppVersion }}" env: - name: DEBUG - value: "{{ .Values.dev.debug }}" + value: "{{ $dev.debug }}" - name: MAXPLAYERS value: "{{ include "satisfactory.formatNumber" $game.maxPlayers }}" # PUID/PGID are only used by the image's root entrypoint path; @@ -42,9 +44,9 @@ spec: - name: DISABLESEASONALEVENTS value: "{{ include "satisfactory.disableSeasonalEvents" $game.enableSeasonalEvents }}" - name: SERVERGAMEPORT - value: "{{ .Values.server.gamePort }}" + value: "{{ include "satisfactory.formatNumber" $server.gamePort }}" - name: SERVERMESSAGINGPORT - value: "{{ .Values.server.messagingPort }}" + value: "{{ include "satisfactory.formatNumber" $server.messagingPort }}" {{- range $value := .Values.env }} {{- if $value.value }} - name: {{ $value.name }} @@ -67,13 +69,13 @@ spec: {{- end }} ports: - name: "api" - containerPort: {{ .Values.server.gamePort }} + containerPort: {{ include "satisfactory.formatNumber" $server.gamePort }} protocol: TCP - name: "game" - containerPort: {{ .Values.server.gamePort }} + containerPort: {{ include "satisfactory.formatNumber" $server.gamePort }} protocol: UDP - name: "messaging" - containerPort: {{ .Values.server.messagingPort }} + containerPort: {{ include "satisfactory.formatNumber" $server.messagingPort }} protocol: TCP livenessProbe: {{- include "satisfactory.livenessProbe" .Values.livenessProbe | nindent 12 }} @@ -82,7 +84,7 @@ spec: startupProbe: {{- include "satisfactory.startupProbe" .Values.startupProbe | nindent 12 }} resources: - {{- include "satisfactory.effectiveResources" .Values.server.resources | nindent 12 }} + {{- include "satisfactory.effectiveResources" $server.resources | nindent 12 }} volumeMounts: - name: satisfactory-volume mountPath: /config diff --git a/satisfactory/values.schema.json b/satisfactory/values.schema.json index e9c9f9e..c49e3eb 100644 --- a/satisfactory/values.schema.json +++ b/satisfactory/values.schema.json @@ -1,7 +1,7 @@ { "$schema": "https://json-schema.org/draft-07/schema#", "title": "satisfactory values", - "description": "Strict values schema (issue #68). additionalProperties is false on every chart-owned object level, so a key that the chart does not read is rejected at render time instead of being silently ignored. The only objects that stay open are the ones passed verbatim into the Kubernetes pod spec (probes, security contexts, global) - the chart does not interpret their contents, so it cannot judge which keys are meaningful. The hardening, probe and resources blocks additionally accept null, because an erased block falls back to the chart defaults (issue #99). Note that this chart keeps env, probes and securityContext at the TOP level rather than under a chart-named key.", + "description": "Strict values schema (issue #68). additionalProperties is false on every chart-owned object level, so a key that the chart does not read is rejected at render time instead of being silently ignored. The only objects that stay open are the ones passed verbatim into the Kubernetes pod spec (probes, security contexts, global) - the chart does not interpret their contents, so it cannot judge which keys are meaningful. The hardening, probe and resources blocks additionally accept null, because an erased block falls back to the chart defaults (issue #99); since 7.0.1 that holds for every chart-owned block, dev, game and server included (Gitea issue #16), so no block is half-safe any more. Note that this chart keeps env, probes and securityContext at the TOP level rather than under a chart-named key.", "type": "object", "additionalProperties": false, "properties": { @@ -13,11 +13,12 @@ "type": "boolean" }, "dev": { - "type": "object", + "description": "May be null / empty: the chart then falls back to its documented dev defaults (Gitea issue #16, the rest of issue #99) instead of aborting the render with a nil-pointer error, which is what an erased block used to do.", + "type": ["object", "null"], "additionalProperties": false, "properties": { "debug": { - "description": "Passed to the image as DEBUG.", + "description": "Passed to the image as DEBUG. Null means the chart default (false).", "type": ["boolean", "string", "null"] } } @@ -41,25 +42,29 @@ } }, "server": { - "type": "object", + "description": "May be null / empty: the chart then falls back to its documented server defaults (Gitea issue #16, the rest of issue #99) instead of aborting the render with a nil-pointer error, which is what an erased block used to do. server.resources is the exception - it is not defaulted here but passed through to the resources fallback, which carries the defaults for it.", + "type": ["object", "null"], "additionalProperties": false, "properties": { "replicas": { - "type": "integer", + "description": "Null means the chart default (1). An explicit 0 is kept and scales the StatefulSet down.", + "type": ["integer", "null"], "minimum": 0 }, "gamePort": { - "type": "integer", + "description": "Null means the chart default (30777). Up to 7.0.0 an erased port rendered an empty port number into the Service and the StatefulSet, which only the API server rejected (Gitea issue #16).", + "type": ["integer", "null"], "minimum": 1, "maximum": 65535 }, "messagingPort": { - "type": "integer", + "description": "Null means the chart default (30888). Same erased-port trap as gamePort above.", + "type": ["integer", "null"], "minimum": 1, "maximum": 65535 }, "storage": { - "description": "Size of the data PVC. Not null-safe on purpose: an erased value yields an invalid PVC rather than a silently degraded workload.", + "description": "Size of the data PVC. Null means the chart default (20Gi). Up to 7.0.0 this was deliberately not null-safe, on the reasoning that an erased value should yield an invalid PVC rather than a silently degraded workload; that reasoning did not survive Gitea issue #16, because it made \"server: null\" render a sizeless PVC instead of falling back like every other block. The chart default is not a degradation - it is the documented size.", "type": ["string", "null"] }, "resources": { diff --git a/satisfactory/values.yaml b/satisfactory/values.yaml index cac24bd..dc7278f 100644 --- a/satisfactory/values.yaml +++ b/satisfactory/values.yaml @@ -4,6 +4,9 @@ # Services stay in place; scaleDown: false brings the workloads back up. scaleDown: false +# Erasing this block (dev: with no sub-keys, or --set dev=null) falls back to +# the defaults below instead of aborting the render — same for game, server and +# the hardening/probe/resources blocks (Gitea issue #16, issue #99). dev: debug: false @@ -28,12 +31,19 @@ game: # again. Set enableSeasonalEvents: true to actually switch the events on. enableSeasonalEvents: false +# Erasing this block, or any single key in it, falls back to the defaults below +# (Gitea issue #16). Before 7.0.1 "server: null" aborted the render and a single +# erased key was worse: a nulled gamePort rendered an empty port number into the +# Service and the StatefulSet, and a nulled storage a PVC without a size — both +# rejected by the API server at apply time, not by the render. server: # Replica count for this workload; 0 is allowed and scales it down. # The chart-global scaleDown flag takes precedence. replicas: 1 gamePort: 30777 messagingPort: 30888 + # The size of the data PVC. An erased value falls back to this default; up to + # 7.0.0 it rendered a PVC asking for no size at all. storage: 20Gi resources: # Limits are optional: memory and ephemeral-storage limits default to -- 2.54.0 From 839b8825ede48b697e0f0c3ea22552b874c5f06a Mon Sep 17 00:00:00 2001 From: Justin Klein Date: Tue, 22 Sep 2026 20:09:29 +0000 Subject: [PATCH 2/2] satisfactory: server.storage stays the documented exception (#39) 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. --- README.md | 4 +++- satisfactory/templates/_helpers.tpl | 27 ++++++++++++++++++--------- satisfactory/values.schema.json | 6 +++--- satisfactory/values.yaml | 16 ++++++++++------ 4 files changed, 34 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index bd44c01..52ee938 100644 --- a/README.md +++ b/README.md @@ -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. diff --git a/satisfactory/templates/_helpers.tpl b/satisfactory/templates/_helpers.tpl index 6d6a3df..1ce5ba6 100644 --- a/satisfactory/templates/_helpers.tpl +++ b/satisfactory/templates/_helpers.tpl @@ -333,15 +333,25 @@ The effective server block (Gitea issue #16, the rest of #99). single erased sub-key was worse than that: it rendered silently. A nulled server.gamePort produced a Service with "port:" and "nodePort:" empty and a StatefulSet with an empty containerPort — a manifest the API server rejects -only at apply time — and a nulled server.storage produced a PVC asking for no -size at all. Both now fall back to the documented defaults instead. +only at apply time. Those now fall back to the documented defaults instead. -Note what is NOT defaulted here: server.resources. That block already runs -through satisfactory.effectiveResources, which carries its own defaults and is +TWO keys are deliberately NOT defaulted here. + +server.resources, because it already runs through +satisfactory.effectiveResources, which carries its own defaults and is null-safe on its own; repeating limitFactor/4Gi/1/5Gi here would create a -third place for them to drift apart. Keys the defaults do not mention are -passed through by satisfactory.defaultedDict unchanged, so an erased -"server:" simply leaves resources unset and effectiveResources supplies them. +third place for them to drift apart. + +server.storage, because values.schema.json documents the opposite decision +for it on purpose: an erased size must yield an invalid PVC that fails at +apply time rather than a workload quietly sized by the chart. An erased +"server:" therefore still renders a PVC without a size — deliberately, not by +oversight. That exception is worth re-examining, but it is someone's reasoned +decision and not a bugfix's to overturn; it is negotiated in Gitea issue #39. + +Both rely on the same mechanic: keys the defaults do not mention are passed +through by satisfactory.defaultedDict unchanged, so an erased "server:" leaves +them unset and whatever consumes them downstream decides what that means. The defaults below MUST stay in sync with the server block in values.yaml, which stays the documented, user-facing place for them; this copy is only the @@ -351,8 +361,7 @@ fallback for the case where an override has erased the block. {{- $defaults := dict "replicas" 1 "gamePort" 30777 - "messagingPort" 30888 - "storage" "20Gi" -}} + "messagingPort" 30888 -}} {{- include "satisfactory.defaultedDict" (dict "defaults" $defaults "given" . "path" "server") -}} {{- end -}} diff --git a/satisfactory/values.schema.json b/satisfactory/values.schema.json index c49e3eb..25a9d92 100644 --- a/satisfactory/values.schema.json +++ b/satisfactory/values.schema.json @@ -1,7 +1,7 @@ { "$schema": "https://json-schema.org/draft-07/schema#", "title": "satisfactory values", - "description": "Strict values schema (issue #68). additionalProperties is false on every chart-owned object level, so a key that the chart does not read is rejected at render time instead of being silently ignored. The only objects that stay open are the ones passed verbatim into the Kubernetes pod spec (probes, security contexts, global) - the chart does not interpret their contents, so it cannot judge which keys are meaningful. The hardening, probe and resources blocks additionally accept null, because an erased block falls back to the chart defaults (issue #99); since 7.0.1 that holds for every chart-owned block, dev, game and server included (Gitea issue #16), so no block is half-safe any more. Note that this chart keeps env, probes and securityContext at the TOP level rather than under a chart-named key.", + "description": "Strict values schema (issue #68). additionalProperties is false on every chart-owned object level, so a key that the chart does not read is rejected at render time instead of being silently ignored. The only objects that stay open are the ones passed verbatim into the Kubernetes pod spec (probes, security contexts, global) - the chart does not interpret their contents, so it cannot judge which keys are meaningful. The hardening, probe and resources blocks additionally accept null, because an erased block falls back to the chart defaults (issue #99); since 7.0.1 that holds for every chart-owned block, dev, game and server included (Gitea issue #16), so no block aborts the render any more. The one key that stays un-defaulted is server.storage, and it says so itself (Gitea issue #39). Note that this chart keeps env, probes and securityContext at the TOP level rather than under a chart-named key.", "type": "object", "additionalProperties": false, "properties": { @@ -42,7 +42,7 @@ } }, "server": { - "description": "May be null / empty: the chart then falls back to its documented server defaults (Gitea issue #16, the rest of issue #99) instead of aborting the render with a nil-pointer error, which is what an erased block used to do. server.resources is the exception - it is not defaulted here but passed through to the resources fallback, which carries the defaults for it.", + "description": "May be null / empty: the chart then falls back to its documented server defaults (Gitea issue #16, the rest of issue #99) instead of aborting the render with a nil-pointer error, which is what an erased block used to do. Two keys are exceptions. server.resources is not defaulted here but passed through to the resources fallback, which carries the defaults for it. server.storage is not defaulted at all, deliberately - see its own description.", "type": ["object", "null"], "additionalProperties": false, "properties": { @@ -64,7 +64,7 @@ "maximum": 65535 }, "storage": { - "description": "Size of the data PVC. Null means the chart default (20Gi). Up to 7.0.0 this was deliberately not null-safe, on the reasoning that an erased value should yield an invalid PVC rather than a silently degraded workload; that reasoning did not survive Gitea issue #16, because it made \"server: null\" render a sizeless PVC instead of falling back like every other block. The chart default is not a degradation - it is the documented size.", + "description": "Size of the data PVC. Not null-safe on purpose: an erased value yields an invalid PVC rather than a silently degraded workload. This stays the documented exception after Gitea issue #16 made every other block in this chart null-safe, so \"server: null\" still renders a PVC without a size; whether failing loudly at apply time beats falling back to 20Gi is negotiated in Gitea issue #39, not inside a bugfix patch.", "type": ["string", "null"] }, "resources": { diff --git a/satisfactory/values.yaml b/satisfactory/values.yaml index dc7278f..e990d0d 100644 --- a/satisfactory/values.yaml +++ b/satisfactory/values.yaml @@ -32,18 +32,22 @@ game: enableSeasonalEvents: false # Erasing this block, or any single key in it, falls back to the defaults below -# (Gitea issue #16). Before 7.0.1 "server: null" aborted the render and a single -# erased key was worse: a nulled gamePort rendered an empty port number into the -# Service and the StatefulSet, and a nulled storage a PVC without a size — both -# rejected by the API server at apply time, not by the render. +# (Gitea issue #16) — with the documented exception of storage, see there. +# Before 7.0.1 "server: null" aborted the render, and a single erased key was +# the quieter half: a nulled gamePort rendered an empty port number into the +# Service and the StatefulSet, rejected by the API server at apply time and by +# nothing before it. server: # Replica count for this workload; 0 is allowed and scales it down. # The chart-global scaleDown flag takes precedence. replicas: 1 gamePort: 30777 messagingPort: 30888 - # The size of the data PVC. An erased value falls back to this default; up to - # 7.0.0 it rendered a PVC asking for no size at all. + # The size of the data PVC, and the ONE key of this block that is NOT + # null-safe — deliberately, not by oversight. An erased storage (including + # "server: null") renders a PVC without a size, which fails at apply time + # instead of quietly sizing the volume from the chart. Whether that is still + # the right trade is negotiated in Gitea issue #39, not here. storage: 20Gi resources: # Limits are optional: memory and ephemeral-storage limits default to -- 2.54.0