From 485f4fa52bccdfede11ad09c91f62d3523253cf4 Mon Sep 17 00:00:00 2001 From: MeepMr <67143880+MeepMr@users.noreply.github.com> Date: Wed, 2 Sep 2026 13:42:39 +0200 Subject: [PATCH] satisfactory: game.enableSeasonalEvents says what it does (7.0.0+upv1.9.10) game.seasonalEvents was handed to the container unchanged, into the image's DISABLESEASONALEVENTS. Its default true therefore wrote DISABLESEASONALEVENTS=true: the seasonal events were OFF in every install this chart has ever produced, while values.schema.json described the very same key as "true means events are enabled" (#215). Of the two ways out, this is the loud one. Inverting the template under the old key would have flipped the behaviour of every existing release in silence - the same class of mistake that created the issue. Renaming the key cannot be missed: game.seasonalEvents no longer exists, additionalProperties is false on the game block, and a values file that still carries it fails schema validation on upgrade instead of quietly meaning the opposite. The new key is positive (enableSeasonalEvents) to avoid a double negative, so the chart negates it on the way into the env var. Go's "not" cannot do that: it reads every non-empty string as true, and the schema deliberately accepts the string form of booleans, so "false" would have flipped the wrong way. The helper compares explicitly and fails on anything that is neither true nor false rather than filing it under "off". The default is enableSeasonalEvents: false, which renders the DISABLESEASONALEVENTS this chart has always rendered. At chart defaults the 6.1.0 and 7.0.0 manifests are identical, so upgrading alone moves no server; switching the events on is now a deliberate act with a key that means it. Separate finding from the same issue, fixed here because it lives in the block being touched: "--set game=null" - or a leftover "game:" whose sub-keys were deleted - aborted the render with a nil pointer at .Values.game.maxPlayers. The game block never got the null-safe treatment of #99 that probes, security contexts and resources have. It now falls back to the documented defaults, and the schema accepts the null accordingly. The same gap remains in the dev and server blocks; it is reported, not fixed here. Major bump: a values key is removed, and the schema rejects it. --- CLAUDE.md | 2 +- README.md | 6 ++- satisfactory/Chart.yaml | 2 +- satisfactory/templates/NOTES.txt | 46 +++++++++++++++----- satisfactory/templates/_helpers.tpl | 46 ++++++++++++++++++++ satisfactory/templates/satisfactory.sfs.yaml | 9 ++-- satisfactory/values.schema.json | 7 +-- satisfactory/values.yaml | 18 +++++++- 8 files changed, 114 insertions(+), 22 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 451f3ab..3b33edf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -35,7 +35,7 @@ helm template --set ingress.domain=example.com --set ingress.cluste ## Creating a new chart -Copy `template-chart/` and replace every `xxx` placeholder — in file names (`templates/xxx.deployment.yaml` → `templates/.deployment.yaml`), in `Chart.yaml`, in the `values.yaml` keys and their `.Values.xxx.*` references inside templates, and in `values.schema.json` (both the values paths and the `xxx.` helper prefixes). The schema and the null-safe fallback helpers come with the template but their content is chart-specific — adapt them along the five points of the README section "Values that must not lie", and validate the result against the chart's real bundle values, not only against its defaults. Then wire up CI: +Copy `template-chart/` and replace every `xxx` placeholder — in file names (`templates/xxx.deployment.yaml` → `templates/.deployment.yaml`), in `Chart.yaml`, in the `values.yaml` keys and their `.Values.xxx.*` references inside templates, and in `values.schema.json` (both the values paths and the `xxx.` helper prefixes). The schema and the null-safe fallback helpers come with the template but their content is chart-specific — adapt them along the six points of the README section "Values that must not lie", and validate the result against the chart's real bundle values, not only against its defaults. Then wire up CI: 1. Add `.github/workflows/publish-.yml` (copy an existing one, e.g. `publish-outline.yml`) — it triggers on pushes to `main` touching `/**` and calls the reusable `publish-helm-chart.yml`. 2. Add the chart name to the `chart-name` choice list in `.github/workflows/change-app-version.yml`. diff --git a/README.md b/README.md index 7f8e4f3..8be1591 100644 --- a/README.md +++ b/README.md @@ -141,7 +141,7 @@ If the dependency dashboard reports `Failed to look up github-releases package ## Creating a new chart -1. Copy `template-chart/` to `/` and replace every `xxx` placeholder: template file names (`xxx.deployment.yaml` → `.deployment.yaml`), `Chart.yaml`, the `values.yaml` keys, the `.Values.xxx.*` references inside the templates, **and both the `xxx.` helper prefixes and the values paths in `values.schema.json`**. Then walk the five points in "Values that must not lie" below — the schema and the null-safe fallbacks are copied with the template, but their content is chart-specific and wrong until you adapt it. The same goes for `templates/NOTES.txt`: it comes with the template as a worked example and is placeholder text until you replace it with what an operator of *your* app needs after the install (see "Everything else" below); its leading `{{/* … */}}` block explains the bar and is meant to be deleted. +1. Copy `template-chart/` to `/` and replace every `xxx` placeholder: template file names (`xxx.deployment.yaml` → `.deployment.yaml`), `Chart.yaml`, the `values.yaml` keys, the `.Values.xxx.*` references inside the templates, **and both the `xxx.` helper prefixes and the values paths in `values.schema.json`**. Then walk the six points in "Values that must not lie" below — the schema and the null-safe fallbacks are copied with the template, but their content is chart-specific and wrong until you adapt it. The same goes for `templates/NOTES.txt`: it comes with the template as a worked example and is placeholder text until you replace it with what an operator of *your* app needs after the install (see "Everything else" below); its leading `{{/* … */}}` block explains the bar and is meant to be deleted. 2. Add `.github/workflows/publish-.yml` (copy an existing one, e.g. `publish-outline.yml`). 3. Add the chart name to the choice list in `.github/workflows/change-app-version.yml`. 4. Add a `customManagers` block for the chart's main image in `renovate.json5` (copy a neighbouring one, swap `managerFilePatterns` and `depNameTemplate`) — without it the chart's app version is never updated. Check the image's tag list first: if it carries a flavour, date or prerelease suffix, add a `versioningTemplate` with a comment saying why. **Floating tags belong outside the pattern**: `ytdlp-web-player` anchors its versioning regex on the `v` prefix precisely so `latest` and `stable` — both of which move — can never be selected. @@ -253,7 +253,7 @@ These guidelines are **binding for every current and future chart** in this repo - Every chart has a global `scaleDown` flag (default `false`): `scaleDown: true` takes precedence over all `replicas` values, renders every Deployment/StatefulSet with `replicas: 0` and suspends CronJobs. It **stops** the app but does not uninstall it — PVCs, Secrets, Services and Ingress stay in place; setting it back to `false` brings the workloads back up. - Implemented via a per-chart `.replicas` helper in `_helpers.tpl` (reference in `template-chart`); the helper uses an explicit nil check (`kindIs "invalid"`) because Helm's `default` would swallow an explicit `0`. -### 12. Values that must not lie (#68, #93, #99) +### 12. Values that must not lie (#68, #93, #99, #215) Every chart ships a **`values.schema.json`** with `additionalProperties: false` on **every chart-owned object level** — not just the top one. The gap this closes is a key that looks like configuration and does nothing: the pilot (#68) found three of them in one chart, one of them nested three levels deep and quietly leaving a game world unpinned. Objects that are passed **verbatim** into the pod spec stay open (probes, `securityContext.pod`/`.container`, `global`, free-form annotation maps): Kubernetes owns their schema, and restricting them would only reject fields that do not exist yet. The schema **adds to** `required()`, it does not replace it. @@ -277,6 +277,8 @@ Reference implementation: `template-chart/values.schema.json` and `template-char **5. A probe default carries the probe's context, not just its timings.** Two different symptoms, both from real charts: `teamspeak-server` addresses its TCP port by the name `file-transfer` because the voice service is UDP and cannot be probed — a default without that name leaves the workload **with no working probe at all**. `patchmon` sends a `Host` header because the server answers **403** to any request whose Host is neither loopback nor a CORS_ORIGIN host — a default without it means the pod never becomes Ready. Port names, `Host` headers and `exec` commands belong in the fallback; where the context is dynamic, pass the root context in and rebuild it (patchmon derives the header from `ingress.domain`). +**6. A value that reads as the negation of the variable it feeds must actually be negated — and `not` cannot do it.** `satisfactory` shipped `game.seasonalEvents` straight into the image's `DISABLESEASONALEVENTS` (#215): the default `true` switched the events **off** in every install for the chart's whole life, while the schema described the same key as "true means events are enabled". The fix is the loud one — rename the key so it says what it does (`game.enableSeasonalEvents`) rather than silently inverting the template under existing releases, because the old key then fails schema validation instead of quietly meaning the opposite. Two mechanics come with it: Go's `not` treats **every non-empty string as true**, so `not "false"` is `false` and the string form these schemas deliberately accept flips the wrong way — compare explicitly (`eq (lower (printf "%v" $v)) "true"`) instead. And a value that cannot be read as either boolean belongs in a `fail`, not in an implicit branch: `enableSeasonalEvents: "yes"` must abort the render, not be quietly filed under "off". When such a rename lands, the new default is the one that **reproduces what the chart already rendered**; changing the meaning and the behaviour in the same release turns the loud fix back into a silent one. + **Two things about the mechanics that are easy to lose:** - **`null` reaches the schema in one case and not the other.** For a key **with** a chart default, Helm's coalescing treats the `null` as a deletion and removes it *before* schema validation — the schema never sees it, and only the helper can catch it. For a key **without** a default (e.g. `resources.limits`), there is nothing to delete, the `null` survives into validation, and a strict `"type": "object"` aborts the render. That is why the helper **and** the widened `["object", "null"]` types are both needed: each covers a case the other cannot. Everything else stays strictly typed. diff --git a/satisfactory/Chart.yaml b/satisfactory/Chart.yaml index 79890b6..6c7f7bf 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: 6.1.0+upv1.9.10 +version: 7.0.0+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 8bced7b..dd18b72 100644 --- a/satisfactory/templates/NOTES.txt +++ b/satisfactory/templates/NOTES.txt @@ -17,9 +17,10 @@ {{- if and (kindIs "float64" $startup.periodSeconds) (kindIs "float64" $startup.failureThreshold) -}} {{- $budget = printf "%.0fs x %.0f = %.0f minutes" $startup.periodSeconds $startup.failureThreshold (divf (mulf $startup.periodSeconds $startup.failureThreshold) 60.0) -}} {{- end -}} -{{- $seasonalRaw := printf "%v" .Values.game.seasonalEvents -}} -{{- $seasonalOff := eq (lower $seasonalRaw) "true" -}} -{{- $beta := eq (lower (printf "%v" .Values.game.experimental)) "true" -}} +{{- $game := fromYaml (include "satisfactory.game" .Values.game) -}} +{{- $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" -}} {{- $outOfRange := or (lt $gamePort 30000) (gt $gamePort 32767) (lt $msgPort 30000) (gt $msgPort 32767) -}} {{- $skipRaw := "" -}} @@ -213,14 +214,37 @@ RESOURCE LIMITS ARE COMPUTED FROM limitFactor ({{ $factor }}) unlimited container gets throttled rather than killed — a memory limit can end a session, a missing CPU limit only slows it down. -game.seasonalEvents IS PASSED THROUGH, NOT INVERTED - The chart hands game.seasonalEvents to the container as - DISABLESEASONALEVENTS unchanged. It is currently {{ $seasonalRaw }}, so the container - runs with DISABLESEASONALEVENTS={{ $seasonalRaw }} — seasonal events are {{ if $seasonalOff }}OFF{{ else }}ON{{ end }}. - {{ if $seasonalOff }}To turn them ON, set game.seasonalEvents: false.{{ else }}To turn them OFF, set game.seasonalEvents: true.{{ end }} - Read that twice: the value behaves as the opposite of what its name suggests - and of what values.schema.json claims. Believe this paragraph and the - rendered env var, not the key name. +SEASONAL EVENTS — THE KEY WAS RENAMED IN 7.0.0, AND IT IS BREAKING + Seasonal (holiday) events are {{ if $seasonalOff }}OFF{{ else }}ON{{ end }} in this release: game.enableSeasonalEvents is + {{ $game.enableSeasonalEvents }}, which the chart negates into DISABLESEASONALEVENTS={{ $seasonalEnv }}. + {{ if $seasonalOff }}To turn them ON, set game.enableSeasonalEvents: true.{{ else }}To turn them OFF, set game.enableSeasonalEvents: false.{{ end }} + + UPGRADING FROM 6.x — WHAT YOU HAVE TO DO + The old key game.seasonalEvents IS GONE. It is not deprecated and not + quietly accepted: values.schema.json rejects it, so a values file that still + carries it fails the upgrade with a validation error instead of installing + something you did not ask for. That noise is deliberate — see below. + + Remove the old key and, if you want the events, add the new one: + + game: + enableSeasonalEvents: true # false, the default, keeps them off + + AND THE MEANING IS INVERTED FROM WHAT THE OLD KEY'S NAME SUGGESTED + Up to 6.1.0 the value was handed to the container UNCHANGED, into a variable + whose name says the opposite: game.seasonalEvents: true wrote + DISABLESEASONALEVENTS=true. So the shipped default had the events switched + OFF, in every install, for as long as this chart has existed — while + values.schema.json described the same key as "true means events are + enabled". If you left the default alone believing that description, you have + never had seasonal events, and the new default (enableSeasonalEvents: false) + changes nothing about that: it renders exactly the DISABLESEASONALEVENTS + this chart has always rendered, so upgrading alone does not move your + server. You have to ask for the events, and now the key name means what it + says when you do. + + Only true and false are accepted, boolean or quoted; anything else aborts + the render rather than being guessed at. THE USER ID IS PINNED, AND PUID/PGID DO NOTHING The pod runs as 1000:1000 because the image supports rootless operation only diff --git a/satisfactory/templates/_helpers.tpl b/satisfactory/templates/_helpers.tpl index 370570c..2430341 100644 --- a/satisfactory/templates/_helpers.tpl +++ b/satisfactory/templates/_helpers.tpl @@ -288,6 +288,52 @@ Probe specifics that a rebuilt default must not lose: {{- include "satisfactory.defaultedDict" (dict "defaults" $defaults "given" . "path" "startupProbe") -}} {{- end -}} +{{/* +The effective game block (issue #99, found alongside #215). + +"--set game=null" — or a leftover "game:" with all sub-keys deleted — used to +abort the render with a nil-pointer error at .Values.game.maxPlayers, because +this block never got the null-safe treatment the probes, security contexts and +resources have. It now falls back to the same defaults values.yaml documents. + +The defaults below MUST stay in sync with the game block in values.yaml. +Note that enableSeasonalEvents defaults to FALSE: that is what keeps +DISABLESEASONALEVENTS at "true", the value this chart has always rendered. +*/}} +{{- define "satisfactory.game" -}} +{{- $defaults := dict + "experimental" false + "maxPlayers" 4 + "enableSeasonalEvents" false -}} +{{- include "satisfactory.defaultedDict" (dict "defaults" $defaults "given" . "path" "game") -}} +{{- end -}} + +{{/* +Render the DISABLESEASONALEVENTS env value from game.enableSeasonalEvents +(issue #215). + +The image only offers the negative switch, so the chart negates the positive +value here — that is the whole point of the rename: the key says what it does +and the env var keeps the name the image expects. + +Go's "not" cannot do this: it treats every non-empty string as true, so the +string form "false" (which fleet bundles habitually write for booleans, and +which the schema therefore accepts) would flip the wrong way. Hence the +explicit comparison — and an explicit fail for anything that is neither, so a +typo like "yes" cannot be silently read as "events off". Null never reaches +here; satisfactory.game has already replaced it with the default. +*/}} +{{- define "satisfactory.disableSeasonalEvents" -}} +{{- $raw := printf "%v" . -}} +{{- if eq (lower $raw) "true" -}} +false +{{- else if eq (lower $raw) "false" -}} +true +{{- else -}} +{{- fail (printf "game.enableSeasonalEvents must be true or false (boolean or string), got %q" $raw) -}} +{{- end -}} +{{- end -}} + {{/* The effective resources block, fed into the resources helper above so that an erased "resources:" still yields the chart's requests and computed limits. diff --git a/satisfactory/templates/satisfactory.sfs.yaml b/satisfactory/templates/satisfactory.sfs.yaml index 7f9e1d8..5826383 100644 --- a/satisfactory/templates/satisfactory.sfs.yaml +++ b/satisfactory/templates/satisfactory.sfs.yaml @@ -1,3 +1,4 @@ +{{- $game := fromYaml (include "satisfactory.game" .Values.game) -}} apiVersion: apps/v1 kind: StatefulSet metadata: @@ -25,7 +26,7 @@ spec: - name: DEBUG value: "{{ .Values.dev.debug }}" - name: MAXPLAYERS - value: "{{ .Values.game.maxPlayers }}" + value: "{{ include "satisfactory.formatNumber" $game.maxPlayers }}" # PUID/PGID are only used by the image's root entrypoint path; # with the default non-root security context they are ignored. - name: PGID @@ -35,9 +36,11 @@ spec: - name: SKIPUPDATE value: "false" - name: STEAMBETA - value: "{{ .Values.game.experimental }}" + value: "{{ $game.experimental }}" + # The image only knows the negative switch, so game.enableSeasonalEvents + # is negated here (#215): true -> "false" (events run). - name: DISABLESEASONALEVENTS - value: "{{ .Values.game.seasonalEvents }}" + value: "{{ include "satisfactory.disableSeasonalEvents" $game.enableSeasonalEvents }}" - name: SERVERGAMEPORT value: "{{ .Values.server.gamePort }}" - name: SERVERMESSAGINGPORT diff --git a/satisfactory/values.schema.json b/satisfactory/values.schema.json index 8f4f274..e9c9f9e 100644 --- a/satisfactory/values.schema.json +++ b/satisfactory/values.schema.json @@ -23,7 +23,8 @@ } }, "game": { - "type": "object", + "description": "May be null / empty: the chart then falls back to its documented game defaults (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": { "experimental": { @@ -33,8 +34,8 @@ "maxPlayers": { "type": ["integer", "string", "null"] }, - "seasonalEvents": { - "description": "Inverted by the chart into DISABLESEASONALEVENTS, so true means events are enabled.", + "enableSeasonalEvents": { + "description": "Seasonal (holiday) events. NEGATED by the chart into the image's DISABLESEASONALEVENTS: true renders DISABLESEASONALEVENTS=\"false\" and the events run, false renders \"true\" and they do not. Only true/false are accepted (boolean or string); anything else aborts the render. Default false, which reproduces what the chart has always rendered - up to 6.1.0 the key was game.seasonalEvents and was passed through unchanged, so its default true wrote DISABLESEASONALEVENTS=true and the events were off despite this description claiming the opposite (issue #215). The old key game.seasonalEvents no longer exists and is rejected here rather than silently ignored.", "type": ["boolean", "string", "null"] } } diff --git a/satisfactory/values.yaml b/satisfactory/values.yaml index 13ef3c4..cac24bd 100644 --- a/satisfactory/values.yaml +++ b/satisfactory/values.yaml @@ -10,7 +10,23 @@ dev: game: experimental: false maxPlayers: 4 - seasonalEvents: true + # Seasonal (holiday) events. The image only knows the negative switch + # DISABLESEASONALEVENTS, so the chart NEGATES this value on the way in: + # enableSeasonalEvents: true renders DISABLESEASONALEVENTS="false" (events + # run), false renders "true" (events off). Only true/false are accepted, in + # boolean or string form; anything else aborts the render instead of being + # read as one of the two. + # + # The default is false on purpose, and it is not a preference: it reproduces + # what this chart has ALWAYS rendered. Up to 6.1.0 the key was called + # game.seasonalEvents and was passed through UNCHANGED, so its default true + # wrote DISABLESEASONALEVENTS=true — the events were off in every install, + # while values.schema.json claimed "true means events are enabled" (#215). + # Keeping the rendered result identical is what makes this rename loud + # instead of silent: nobody's server changes behaviour by upgrading, and the + # old key is now rejected by the schema, so anyone who set it has to decide + # again. Set enableSeasonalEvents: true to actually switch the events on. + enableSeasonalEvents: false server: # Replica count for this workload; 0 is allowed and scales it down.