From 07c75e80c11a8f7559d0f12c123ca5ed34aabd7d Mon Sep 17 00:00:00 2001 From: Justin Klein Date: Fri, 2 Oct 2026 20:40:24 +0000 Subject: [PATCH] feat(stalwart): take our own published valkey chart as the subchart Replaces the dependency on upstream's valkey chart with this repo's own, resolved from the published index: version: "1.0.0+up9.1.2.alpine" repository: "https://git.jk-dev.de/api/packages/jlab-cluster/helm" The condition is "valkeySubchart.enabled", NOT "valkey.enabled", and that is forced rather than stylistic. Everything under "valkey:" is handed to the subchart as its values and validated against ITS schema, and our valkey chart ships additionalProperties: false with no "enabled" property - so "valkey.enabled" is rejected by the subchart: valkey: - at '': additional properties 'enabled' not allowed Measured. And because Helm skips a DISABLED subchart's schema validation, that error would have surfaced only for whoever first set it to true: every render, lint and install-test in CI would have stayed green. Adding "enabled" to valkey/values.schema.json was rejected as the repair - that chart does not read the key, and it would have needed its own PR and a publish first. The host helper was also wrong for our chart and is fixed: our Service is "-valkey-service", not upstream's bare "-valkey". The values block is rewritten for our chart's schema, with the pair chosen rather than inherited: CACHE (persistence off + allkeys-lru), because everything Stalwart keeps in the in-memory store - rate limiters, auth tokens, sessions, auto-ban entries, coordination pub/sub - is reconstructible, and the node-id lease lives in the data store. The cost is named where it is set: with allkeys-lru the rate limiters and the auto-ban FAIL OPEN under memory pressure, so the answer is to size memory, not to switch to noeviction (which would make writes error and take out authentication instead). secrets.authSecretRef wants an item whose single field "auth-conf" holds the complete "requirepass " line, not a bare password. Chart.yaml's comments are corrected where they became false: nothing is "consumed as it comes" any more and there is no shortfall of someone else's to document. The Renovate reasoning stays and is now backed by measurement. New trap, in Chart.yaml and in three places in the wiki: NEVER change valkey/ and a chart depending on it in the same PR. The dependency resolves from the published index, so a combined PR lints, packages, renders AND install-tests against the OLD subchart - green on an artifact that is not the one in the diff. Two consecutive PRs. Two things measured, not assumed, because either could have broken the update path with no symptom but "no updates ever": - helm dependency update AND build resolve the exact build-metadata version from the URL alone with an EMPTY HELM_REPOSITORY_CONFIG - a fresh runner's state - so the anonymous index needs no credential and ci/helm-dependencies.sh is untouched; - Renovate 44.132.2 dry run with the dependency pinned low: manager helmv3 matched the file, took registryUrls from Chart.yaml and offered newVersion "1.0.0+up9.1.2.alpine", versioning "helm". The self-hosted index and the "+up..." suffix are both handled. README: the claim "no registry address appears in this repository" is narrowed to what still holds - the publish target and its credentials come from 1Password and are not in the repo. The read path is now in Chart.yaml by necessity, carries no credential, and the host was always in CLAUDE.md. Done here rather than later so the paperless PR finds it already corrected. Re-verified after the change: helm lint --strict (0 failed), helm package (the subchart is embedded in the .tgz), helm template with the subchart OFF and ON (ON was previously impossible), against CI test-values, the rendered config.json, ci/run-quantity-render-test.sh (0 failed), NOTES.txt through the scratch-ConfigMap trick, and the fail paths including the reworded abort. The rendered subchart Service name matches what the helper emits, the cache pairing really renders (maxmemory-policy allkeys-lru, appendonly no, save "", emptyDir) and no PVC appears. Part of #65 --- README.md | 18 +++++- stalwart/Chart.yaml | 87 ++++++++++++++++++++----- stalwart/templates/NOTES.txt | 40 +++++++++--- stalwart/templates/_helpers.tpl | 27 ++++---- stalwart/values.schema.json | 12 +++- stalwart/values.yaml | 110 ++++++++++++++++++++++++-------- 6 files changed, 226 insertions(+), 68 deletions(-) diff --git a/README.md b/README.md index 56e39fa..43734b5 100644 --- a/README.md +++ b/README.md @@ -32,7 +32,7 @@ Every chart has a wiki page of its own — `https://git.jk-dev.de/jlab-cluster/h | `samba` | Samba | StatefulSet, users secret + config ConfigMap | | `satisfactory` | Satisfactory game server | StatefulSet, no ingress | | `searxng` | SearXNG | Stateless Deployment + settings ConfigMap (`use_default_settings: true` plus a free-form `settings.extra` passthrough); the signing key is a required `OnePasswordItem`, an external Valkey an optional second one. `ingress.domain` is read twice — Ingress host and `SEARXNG_BASE_URL` — so it stays required with the Ingress switched off | -| `stalwart` | Stalwart mail server (SMTP, IMAP, POP3, JMAP, ManageSieve) | **The chart configures almost nothing, and that is the chart's defining property.** Since v0.16 Stalwart keeps its whole configuration — every listener, the blob store, the in-memory store, TLS objects, domains and the cluster roles — inside the data store and manages it over JMAP; the only file on disk is a `config.json` that names the data store and nothing else. So the chart renders that ConfigMap (secret-free: `authSecret` uses the `EnvironmentVariable` variant, so the password arrives by `secretKeyRef` and `checksum/config` stays meaningful), one StatefulSet per entry of the `nodes` **map** with `STALWART_ROLE` and `STALWART_HOSTNAME`, one NodePort Service **per port** (not per group — a NodePort is cluster-unique, so `full` and `mx` can only take SMTP behind the same Service, selected by a per-port pod label), a per-group ClusterIP `http` Service for the Ingress, per-group headless Services and NetworkPolicies, and a cert-manager `Certificate` for the mail ports. Everything else is operator work in the WebUI, which is why `NOTES.txt` carries an eight-step first install. Four traps worth knowing before reading the chart: **the role lookup fails open** — a node whose `ClusterRole` is missing from the database enables every listener *and* every task while staying Running with a green probe, so `mx` ships at `replicas: 0` and the NetworkPolicy is a ceiling independent of what was bound; **`NET_BIND_SERVICE` is a documented deviation needed to `exec`, not to bind** (the image sets `setcap cap_net_bind_service=+ep`, so `drop: [ALL]` alone makes the kernel fail `execve` — removing it as "unused" stops the chart starting); **`externalTrafficPolicy` must be `Cluster`** (no pod runs on a control-plane node while the HA proxy delivers to them, so `Local` would drop every packet) and the source address it costs is restored only by the HA proxy's PROXY v2 header; and **`/healthz` is rate-limited to 100 req/60s per source IP**, which is a node IP for kubelet probes, so the probe intervals are calculated rather than chosen. A first install needs `bootstrap.enabled: true` because the setup wizard appears only when `config.json` is *absent*. Optional `valkey` subchart (the Valkey project's own chart), off by default | +| `stalwart` | Stalwart mail server (SMTP, IMAP, POP3, JMAP, ManageSieve) | **The chart configures almost nothing, and that is the chart's defining property.** Since v0.16 Stalwart keeps its whole configuration — every listener, the blob store, the in-memory store, TLS objects, domains and the cluster roles — inside the data store and manages it over JMAP; the only file on disk is a `config.json` that names the data store and nothing else. So the chart renders that ConfigMap (secret-free: `authSecret` uses the `EnvironmentVariable` variant, so the password arrives by `secretKeyRef` and `checksum/config` stays meaningful), one StatefulSet per entry of the `nodes` **map** with `STALWART_ROLE` and `STALWART_HOSTNAME`, one NodePort Service **per port** (not per group — a NodePort is cluster-unique, so `full` and `mx` can only take SMTP behind the same Service, selected by a per-port pod label), a per-group ClusterIP `http` Service for the Ingress, per-group headless Services and NetworkPolicies, and a cert-manager `Certificate` for the mail ports. Everything else is operator work in the WebUI, which is why `NOTES.txt` carries an eight-step first install. Four traps worth knowing before reading the chart: **the role lookup fails open** — a node whose `ClusterRole` is missing from the database enables every listener *and* every task while staying Running with a green probe, so `mx` ships at `replicas: 0` and the NetworkPolicy is a ceiling independent of what was bound; **`NET_BIND_SERVICE` is a documented deviation needed to `exec`, not to bind** (the image sets `setcap cap_net_bind_service=+ep`, so `drop: [ALL]` alone makes the kernel fail `execve` — removing it as "unused" stops the chart starting); **`externalTrafficPolicy` must be `Cluster`** (no pod runs on a control-plane node while the HA proxy delivers to them, so `Local` would drop every packet) and the source address it costs is restored only by the HA proxy's PROXY v2 header; and **`/healthz` is rate-limited to 100 req/60s per source IP**, which is a node IP for kubelet probes, so the probe intervals are calculated rather than chosen. A first install needs `bootstrap.enabled: true` because the setup wizard appears only when `config.json` is *absent*. Optional `valkey` subchart — **this repo's own chart, from the published index**, off by default and gated on `valkeySubchart.enabled` (not `valkey.enabled`, which the subchart's strict schema rejects); never change `valkey/` and this chart in the same PR, see "Chart dependencies" | | `sure` | Sure (we-promise, the maintained Maybe-finance fork) | One StatefulSet with **two containers**: the Puma web process and a Sidekiq worker, sharing one ReadWriteOnce PVC at `/rails/storage` — that shared volume is the reason they live in one pod. Only the web container migrates (the image entrypoint runs `db:prepare` solely for the `./bin/rails server` command, so the worker is started through `args:` and skips it). Three 1Password items (app/database/redis); Postgres and Redis stay external. Readiness deliberately probes `/` rather than Rails' `/up`, which is a boot check and touches no database; the worker's three probes ask `sidekiqmon` for this pod's own heartbeat. `ingress.domain` is read for `APP_DOMAIN` and the WebAuthn relying party, so it stays required with the Ingress switched off | | `teamspeak-server` | TeamSpeak server | StatefulSet, no ingress | | `template-chart` | — | Scaffold for new charts (placeholder `xxx`), never published | @@ -57,7 +57,9 @@ Every chart has a wiki page of its own — `https://git.jk-dev.de/jlab-cluster/h - **`secrets: inherit`**, unchanged, for `OP_CONNECT_TOKEN`. - the repository **variable `OP_CONNECT_HOST`**, which replaced the Connect host that used to be hardcoded in our copy. A shared block must not carry one tenant's infrastructure address; the block's first step fails and names the variable when it is unset. - The registry credentials and the target URL still come from 1Password Connect (item `op://GitHub-Actions/jlab-registry-access/` with `username`, `password`, `charts-url`). `charts-url` holds only the **base up to `/api/packages`** and the block composes `//helm/api/charts`, so **no registry address appears in this repository** — not even the owner path segment. `ci/helm-dependencies.sh` is still run before `helm package`, and it is still **our** script: the block's checkout clones the calling repository, so it runs from this tree. + The registry credentials and the target URL still come from 1Password Connect (item `op://GitHub-Actions/jlab-registry-access/` with `username`, `password`, `charts-url`). `charts-url` holds only the **base up to `/api/packages`** and the block composes `//helm/api/charts`, so **the publish target and its credentials appear nowhere in this repository.** `ci/helm-dependencies.sh` is still run before `helm package`, and it is still **our** script: the block's checkout clones the calling repository, so it runs from this tree. + + **What this does *not* claim any more**: that no registry address is in the repo at all. That wording was true until a chart took one of our own published charts as a subchart — `stalwart`'s `Chart.yaml` names `https://git.jk-dev.de/api/packages/jlab-cluster/helm` as a dependency `repository:`, because a Helm dependency is resolved from a URL in the chart and there is nowhere else to put it. The narrower claim above is the one that matters and it is unchanged: that is the **read** path, the index is anonymously readable (measured), so it carries **no credential** and grants nothing a `git clone` of this repo does not already imply. The host itself has always been in `CLAUDE.md`; what is new is only the read-only path segment. The reasons behind the upload step are **not repeated here**, because they are no longer ours to keep current: why both halves of the composed URL are guarded, why the credentials go in through `curl --config -` instead of `--user`, why `--fail-with-body` is what makes a failed upload fail the job, and why the file name is built rather than globbed, are all on the [`publish-helm-chart` wiki page](https://git.jk-dev.de/workflow-util/pipelines/wiki/publish-helm-chart) of that repository. Nothing was dropped in the move — the upload step's shell body went across unchanged except for two error messages that name the owner as an input. @@ -438,7 +440,17 @@ Reference implementation: `template-chart/values.schema.json` and `template-char Until `ytdlp-web-player` (#214) this section read "charts have no dependencies (`charts/` directories are empty)". That rule is gone, replaced by a bar rather than a ban — a dependency is **admissible, not free**, and the reason it stopped being a ban is worth keeping: the chart needed an oauth2-proxy in the data path in front of an application that ships no authentication at all, and proxy and app are only useful wired to each other. -Two charts carry one, and comparing them is the quickest way to see where the bar sits. `ytdlp-web-player` takes `oauth2-proxy` because the app has no authentication and the two are useless apart. `stalwart` (Gitea #65) takes the Valkey project's own chart because Stalwart's **cluster coordinator rides on the in-memory store connection**, so a multi-node install does not work without one — and because this repo also publishes a `valkey` chart of its own, the subchart is the convenience path rather than the expected one: `externalServices.valkey.host` points at an existing instance and then the dependency renders nothing. Both are `condition:`-gated and off by default, and in both cases upstream's shortfalls against the guidelines above (`stalwart`: `auth.enabled: false`, and a NetworkPolicy with upstream's own semantics that our `networkPolicy` block does not reach) are **documented in our `values.yaml` rather than patched**. +Two charts carry one, and comparing them is the quickest way to see where the bar sits. `ytdlp-web-player` takes **upstream's** `oauth2-proxy` because the app has no authentication and the two are useless apart; there, point 2 above applies in full. `stalwart` (Gitea #65) takes **one of our own published charts**, `valkey`, because Stalwart's **cluster coordinator rides on the in-memory store connection**, so a multi-node install does not work without one. Both are `condition:`-gated and off by default — for `stalwart` the subchart is the path for an installation that has no Valkey yet, and one that already has points `externalServices.valkey.host` at it so the dependency renders nothing. + +**A dependency on one of our own charts is cheaper in one way and more dangerous in another.** Point 2 stops applying: there are no third-party defaults to leave unpatched, because the subchart follows these guidelines by being one of ours. In exchange it adds a hazard an external dependency does not have, and it is sharp enough to be a rule: + +> **Never change a chart and a chart that depends on it in the same pull request.** + +The dependency resolves from the **published index**, not from the working tree, and a chart is published only by a push to `main` that touches it. So a combined PR runs every `helm` command — lint, package, render **and the install-test** — against the **old** subchart, goes green on an artifact that is not the one in the diff, and leaves the subchart's own change exercised by nothing. Two consecutive PRs: change the subchart, merge, let it publish, then raise the `version:` in the dependent chart. + +Two mechanical details of a self-referencing dependency, both measured for #65: the index is **anonymously readable**, so `ci/helm-dependencies.sh` needs no credential and resolves the exact build-metadata version (`1.0.0+up9.1.2.alpine`) from the URL alone with an empty `HELM_REPOSITORY_CONFIG` — the state a fresh runner is in. And Renovate's built-in `helmv3` manager does follow a self-hosted Gitea index: a dry run (Renovate 44.132.2) with the dependency pinned low reported `newVersion: "1.0.0+up9.1.2.alpine"`, `versioning: "helm"`, timestamp read from our index. Without that second result the published-version form would have had no update path at all, which is why it was measured rather than assumed. + +One more trap, and it is the reason `stalwart` gates its subchart on `valkeySubchart.enabled` rather than the conventional `valkey.enabled`: **everything under the subchart's key is handed to the subchart as its values**, so a `condition:` key placed there is validated against *its* schema. Our charts ship `additionalProperties: false`, so `valkey.enabled` is rejected by the subchart — and because Helm skips a **disabled** subchart's schema validation, the error surfaces only for whoever first switches it on. Put the condition on a key of the parent's own. **When a dependency earns its place.** All three, not two of three: diff --git a/stalwart/Chart.yaml b/stalwart/Chart.yaml index bd9b987..131812f 100644 --- a/stalwart/Chart.yaml +++ b/stalwart/Chart.yaml @@ -37,15 +37,17 @@ maintainers: # the chart means the wiring (host, port, egress rule) is in one place instead # of split across two releases that have to agree. # -# WHY IT IS OFF BY DEFAULT, and why that is more than convention here: most -# installations already run a Valkey, and this repo publishes a valkey chart of -# its own that is the better choice when one application owns the instance. -# Point externalServices.valkey at it and this dependency renders nothing. +# WHY IT IS OFF BY DEFAULT: an installation that already runs a Valkey points +# externalServices.valkey.host at it and this dependency renders nothing. The +# subchart is the path for an installation that has none. # -# UPSTREAM'S CHART IS CONSUMED AS IT COMES (guideline 14.2). Two of its defaults -# fall short of this repo's conventions and are deliberately NOT patched here; -# they are documented in our values.yaml under "valkey:" instead, where the -# values-level fix stays available to whoever installs the chart. +# THE SUBCHART IS OUR OWN valkey CHART, resolved from the published index - +# NOT upstream's. So guideline 14.2 ("we consume their chart as it comes") does +# not apply here and there is nothing of someone else's to leave unpatched: the +# subchart follows this repo's conventions because it IS this repo's chart. What +# does apply is everything about the subchart being invisible downstream, plus +# one hazard that a self-referencing dependency adds and an external one does +# not - see the PR-ordering warning below. dependencies: - name: valkey # KEEP THIS VERSION MOVING - it is the only path a Valkey update has. @@ -63,14 +65,65 @@ dependencies: # actually ships it. That chain is the whole safety net; switched off, this # line needs a human on a calendar instead. # + # MEASURED, not assumed, because the registry is self-hosted and the version + # carries build metadata - two things that could each have broken the chain + # silently. Renovate 44.132.2, dry run on 2026-10-02 with this dependency + # pinned at a lower version: manager helmv3 matched the file, took + # registryUrls from the line below, and offered + # newVersion: "1.0.0+up9.1.2.alpine", versioning: "helm" + # with the release timestamp read from our index. So both the self-hosted + # index and the "+up..." suffix are handled. + # # Independent of appVersion: raising it is a patch or minor bump of this # chart and is never tied to the Stalwart image. - version: "0.12.0" - # The Valkey project's own chart, so upstream owns the schema - not a - # re-packaging. Note the repository is the plain HTTP index, not OCI. - repository: "https://valkey.io/valkey-helm" - # Off by default, so a plain install renders none of its objects. Helm - # splits a condition on dots only; this name carries no dash, so unlike - # ytdlp-web-player's oauth2-proxy it is also reachable as .Values.valkey.* - # inside templates without index. - condition: valkey.enabled + version: "1.0.0+up9.1.2.alpine" + # Our own published index, and the READ path only. It is anonymously + # readable - measured 2026-10-02: GET .../index.yaml returns 200 without + # credentials - so ci/helm-dependencies.sh needs no secret and was not + # touched. Verified with an empty HELM_REPOSITORY_CONFIG, i.e. the state a + # fresh runner is in: helm reports "Getting updates for unmanaged Helm + # repositories" and resolves the exact build-metadata version from the URL + # alone, with no "helm repo add" anywhere. Both "helm dependency update" and + # "helm dependency build" were tried. + # + # The PUBLISH target and its credentials still live in 1Password and appear + # nowhere in this repository - see README, "CI/CD". + repository: "https://git.jk-dev.de/api/packages/jlab-cluster/helm" + # Off by default, so a plain install renders none of its objects. + # + # NOT "valkey.enabled", AND THAT IS NOT A STYLE CHOICE. Everything under the + # "valkey:" key is passed to the subchart as its values, and our valkey + # chart ships a values.schema.json with additionalProperties: false and no + # "enabled" property - so "valkey.enabled" is rejected by the SUBCHART: + # Error: values don't meet the specifications of the schema(s) in the + # following chart(s): valkey: - at '': additional properties 'enabled' + # not allowed + # Measured. Worse, Helm skips a disabled subchart's schema validation, so + # the error appears only for whoever first sets it to true - the default + # path renders clean. + # + # The condition is therefore a key of OUR OWN, outside the subchart's value + # subtree, which is also the honest shape: "enabled" is not a valkey value. + # It only ever looked like one because upstream's schema was permissive. + # The alternative - adding "enabled" to valkey/values.schema.json - would + # have to be its own PR and be published first (see the warning below), for + # a key that chart does not read. + condition: valkeySubchart.enabled +# +# !!! NEVER CHANGE valkey/ AND THIS CHART IN THE SAME PULL REQUEST !!! +# +# The hazard a self-referencing dependency adds. This dependency resolves out of +# the PUBLISHED index, not out of the working tree, and valkey/ is only +# published by a push to main that touches it. So in one PR carrying both: +# +# - every helm command here - lint, package, render, AND the install-test - +# pulls the OLD valkey .tgz, because the new one does not exist yet; +# - the PR therefore goes green against a version of the subchart that is not +# the one in the diff, and the diff to valkey/ is reviewed while nothing +# actually exercised it; +# - after the merge both charts publish, and the first install of the new +# stalwart is the first time the new valkey is used anywhere. +# +# Two consecutive pull requests instead: change valkey/, merge, let it publish, +# then raise the version here. The ordering is not a nicety - the green check on +# the combined PR is a statement about the wrong artifact. diff --git a/stalwart/templates/NOTES.txt b/stalwart/templates/NOTES.txt index 4b2b63b..fe2bf0d 100644 --- a/stalwart/templates/NOTES.txt +++ b/stalwart/templates/NOTES.txt @@ -236,18 +236,40 @@ NO INGRESS ingress.domain STAYS REQUIRED even so — the mail Certificate reads it for its dnsNames, and without it the certificate would carry no name at all. {{- end }} -{{- if .Values.valkey.enabled }} +{{- if .Values.valkeySubchart.enabled }} THE VALKEY SUBCHART IS ENABLED - Reachable at {{ $valkeyHost }}:{{ .Values.externalServices.valkey.port }}, and it is upstream's chart consumed as it - comes. Two of its defaults fall short of this repo's conventions and are NOT - patched by us: auth.enabled is false (unauthenticated access from anything - the CNI permits), and its NetworkPolicy is its own object with its own - semantics, which this chart's networkPolicy block does not reach. See the - valkey section of values.yaml. + Reachable at {{ $valkeyHost }}:{{ .Values.externalServices.valkey.port }} — this repo's own valkey chart, + pulled from the published index. Enter that address as the in-memory store in + step 3; the coordinator's Default variant reuses the same connection. - scaleDown and checksum/config stop at the subchart boundary, and a new Valkey - release reaches you only when this chart is republished. + ITS 1PASSWORD ITEM IS A SEPARATE ONE, AND ITS FIELD IS NOT A PASSWORD: + valkey.secrets.authSecretRef points at + {{ .Values.valkey.secrets.authSecretRef | default "" }} + and that item carries ONE field, "auth-conf", whose value is the complete + configuration line + + requirepass + + not a bare password. The valkey chart mounts the file and includes it, and + an init container refuses to start the pod unless the line is really there — + because the alternative failure is an open server that looks healthy. + + IT IS CONFIGURED AS A CACHE, NOT A QUEUE + persistence.enabled false + maxmemoryPolicy allkeys-lru. Everything Stalwart + keeps here — rate-limiter counters, auth tokens, sessions, auto-ban entries, + the coordination pub/sub — is reconstructible after a restart; nothing in it + is the system of record. + + The cost of allkeys-lru, so it is not a surprise: under memory pressure keys + are discarded, so the rate limiters and the auto-ban FAIL OPEN. Size memory + for the working set rather than switching to noeviction — with noeviction + Stalwart's writes to the store start erroring, which takes out + authentication instead of just the limits. + + scaleDown and checksum/config stop at the subchart boundary, so scaling this + release to zero does NOT stop the Valkey, and a new valkey chart release + reaches you only when stalwart itself is republished. {{- end }} {{- if not .Values.networkPolicy.enabled }} diff --git a/stalwart/templates/_helpers.tpl b/stalwart/templates/_helpers.tpl index ee5ef55..3790233 100644 --- a/stalwart/templates/_helpers.tpl +++ b/stalwart/templates/_helpers.tpl @@ -375,28 +375,33 @@ is the address to type into the WebUI, and the port for the egress rule. {{- end -}} {{- end -}} {{- $subchartOn := false -}} -{{- if and (not (kindIs "invalid" .Values.valkey)) .Values.valkey.enabled -}} +{{- if and (not (kindIs "invalid" .Values.valkeySubchart)) .Values.valkeySubchart.enabled -}} {{- $subchartOn = true -}} {{- end -}} {{- if $given -}} {{- $given -}} {{- else if $subchartOn -}} -{{/* The subchart's own fullname helper: release name plus chart name, unless - the release name already contains it. Reproduced rather than called, - because a parent cannot include a subchart's define with the subchart's - context. */}} -{{- if contains "valkey" .Release.Name -}} -{{- .Release.Name | trunc 63 | trimSuffix "-" -}} -{{- else -}} -{{- printf "%s-valkey" .Release.Name | trunc 63 | trimSuffix "-" -}} -{{- end -}} +{{/* The Service our own valkey chart renders: + {{ .Release.Name }}-{{ .Chart.Name }}-service + and inside a subchart .Chart.Name is the SUBCHART's name, so this is + "-valkey-service" with no special case. Reproduced rather than + included, because a parent cannot call a subchart's define with the + subchart's context. + + Note the "-service" suffix: this repo's naming scheme ends every Service + that way, which is NOT what upstream's valkey chart does (it would be a + bare "-valkey"). An earlier draft of this chart depended on + upstream's and carried their name; getting this wrong points the operator + at an address that does not resolve, and nothing in the chart would say + so. */}} +{{- printf "%s-valkey-service" .Release.Name | trunc 63 | trimSuffix "-" -}} {{- else -}} {{- $total := 0 -}} {{- range $group, $spec := fromYaml (include "stalwart.nodes" .) -}} {{- $total = add $total (int (include "stalwart.replicas" (dict "scaleDown" $.Values.scaleDown "replicas" $spec.replicas))) -}} {{- end -}} {{- if gt $total 1 -}} -{{- fail (printf "this release runs %d Stalwart pods, which is a cluster, but no in-memory store has an address: externalServices.valkey.host is empty and valkey.enabled is false. Stalwart's cluster coordinator rides on the in-memory store connection, so a multi-node install has no coordination path at all - and that failure does not show up as a failed install, it shows up as nodes that do not agree with each other. Set externalServices.valkey.host to an existing instance, or set valkey.enabled: true to deploy the subchart" $total) -}} +{{- fail (printf "this release runs %d Stalwart pods, which is a cluster, but no in-memory store has an address: externalServices.valkey.host is empty and valkeySubchart.enabled is false. Stalwart's cluster coordinator rides on the in-memory store connection, so a multi-node install has no coordination path at all - and that failure does not show up as a failed install, it shows up as nodes that do not agree with each other. Set externalServices.valkey.host to an existing instance, or set valkeySubchart.enabled: true to deploy the subchart" $total) -}} {{- end -}} {{- end -}} {{- end -}} diff --git a/stalwart/values.schema.json b/stalwart/values.schema.json index be2068b..7398770 100644 --- a/stalwart/values.schema.json +++ b/stalwart/values.schema.json @@ -385,8 +385,18 @@ "startupProbe": { "$ref": "#/definitions/probe" }, + "valkeySubchart": { + "description": "The dependency condition for the valkey subchart. A key of THIS chart, deliberately outside the 'valkey' subtree: everything under 'valkey:' is handed to the subchart as its values, and this repo's valkey chart has additionalProperties: false with no 'enabled' property, so 'valkey.enabled' is rejected by the subchart itself. Helm skips a disabled subchart's schema validation, so that error would only have surfaced for whoever first set it to true.", + "type": "object", + "additionalProperties": false, + "properties": { + "enabled": { + "type": "boolean" + } + } + }, "valkey": { - "description": "Values of the valkey subchart (the Valkey project's own chart), passed straight through. Open on purpose and at every level: this chart owns neither the keys nor their meaning, and a closed copy would reject whatever the subchart adds in its next release - the exact failure a schema exists to prevent, pointed at the wrong chart. This chart reads exactly one of them, 'enabled', which is the dependency condition. Two of upstream's defaults fall short of this repo's conventions and are documented in values.yaml rather than patched here.", + "description": "Values passed through to the valkey subchart - this repo's OWN valkey chart, resolved from the published index. Open on purpose and at every level: the subchart owns this schema (it ships its own, and a stricter one than this), and a closed copy here would reject whatever that chart adds in its next release while also duplicating a validation that already happens. This chart reads NONE of these keys; the dependency condition is valkeySubchart.enabled instead. Note the field shape that is easy to get wrong: secrets.authSecretRef points at an item whose single field 'auth-conf' holds the complete line 'requirepass ', not a bare password. values.yaml argues the cache-vs-queue choice.", "type": "object" } }, diff --git a/stalwart/values.yaml b/stalwart/values.yaml index ae06b03..5798257 100644 --- a/stalwart/values.yaml +++ b/stalwart/values.yaml @@ -643,34 +643,90 @@ startupProbe: failureThreshold: 30 # --------------------------------------------------------------------------- -# Valkey subchart (the Valkey project's own chart) — OFF by default +# The Valkey subchart — THIS REPO'S OWN valkey CHART, off by default # --------------------------------------------------------------------------- # Stalwart needs an in-memory store for a cluster, and the coordinator rides on -# the same connection. This dependency is the convenience path for a -# deployment that has no Valkey yet; point externalServices.valkey.host at an -# existing instance (this repo's own valkey chart, for example) and leave this -# off. +# the same connection. The subchart is the path for a deployment that has no +# Valkey yet; an installation that already runs one points +# externalServices.valkey.host at it and leaves this off. # -# UPSTREAM'S CHART IS CONSUMED AS IT COMES — two of its defaults fall short of -# this repo's conventions and are deliberately NOT patched here (README, -# "Chart dependencies", point 2). Named rather than silently inherited: -# -# 1. auth.enabled defaults to FALSE, so the instance accepts unauthenticated -# connections from anything the CNI lets through to it. Upstream's own -# values file warns about the related footgun (enabling auth without a -# "default" user leaves it open anyway). If this Valkey is reachable -# beyond the pod, set auth.enabled plus auth.aclUsers / auth.aclConfig — -# and prefer usersExistingSecret over an inline password, since inline -# values land in the Helm release. -# 2. networkPolicy is upstream's own object with upstream's own semantics; -# this chart's networkPolicy block does not reach it. Our Stalwart pods -# get an egress rule for externalServices.valkey.port, but nothing here -# restricts who else may reach the Valkey. -# -# Other costs, stated so they are not rediscovered: the subchart is invisible -# to anything watching Helm release versions (a Valkey fix arrives only when -# this chart is republished — Chart.yaml has the detail), scaleDown and -# checksum/config stop at the subchart boundary, and every helm command needs -# ci/helm-dependencies.sh to have run first. -valkey: +# THE SWITCH IS HERE AND NOT UNDER "valkey:", and it has to be. Everything under +# "valkey:" is handed to the subchart as its values, and our valkey chart ships +# a strict schema (additionalProperties: false) with no "enabled" property — so +# "valkey.enabled" is rejected BY THE SUBCHART. Measured; and because Helm skips +# a disabled subchart's schema validation, the error would only ever appear for +# whoever first switched it on. Chart.yaml has the error text. +valkeySubchart: enabled: false + +# --------------------------------------------------------------------------- +# Values passed THROUGH to the valkey subchart +# --------------------------------------------------------------------------- +# Read as the subchart's own values, so the key names are ITS schema, not +# upstream's and not this chart's. `valkey/values.yaml` in the repo is the +# reference; its header comment explains the pairs. Nothing here has any effect +# while valkeySubchart.enabled is false. +# +# WHY THIS IS NOT "upstream's chart as it comes": it is our chart, so there is +# no third party's defaults to leave unpatched and no shortfall against this +# repo's conventions to document — it follows them because it IS one of them. +# What still applies in full is the subchart being invisible downstream (a +# Valkey fix reaches a cluster only when THIS chart is republished — Chart.yaml +# has the detail and the PR-ordering warning), that scaleDown and +# checksum/config stop at the subchart boundary, and that every helm command +# needs ci/helm-dependencies.sh to have run first. +valkey: + secrets: + # REQUIRED once the subchart is on — the subchart's own required() aborts + # the render without it, which is the correct loud failure. + # + # MIND THE FIELD, it is not a password. The 1Password item carries ONE + # field, `auth-conf`, whose value is the COMPLETE configuration line + # requirepass + # The valkey chart mounts that file and `include`s it, and an init container + # refuses to start the pod unless the line is really there — because the + # alternative failure is an open server that looks perfectly healthy. A bare + # password in that field does not work. + authSecretRef: null + + # CACHE, not queue — a decision, with its cost named. + # + # The valkey chart treats persistence and eviction as a PAIR and warns against + # mixing them: QUEUE is `persistence.enabled: true` + `noeviction`, CACHE is + # `persistence.enabled: false` + `allkeys-lru`. + # + # Stalwart's in-memory store holds rate-limiter counters, auth tokens and + # sessions, auto-ban entries, and the cluster coordination pub/sub. Every one + # of those is reconstructible: counters and bans restart from zero, tokens and + # sessions force a re-authentication, and pub/sub messages are not stored keys + # at all. Nothing in it is the system of record — messages and metadata are in + # Postgres, blobs in S3, and the cluster node-id lease is in the data store + # too, not here. Paying for a PVC and for AOF replay to protect data that is + # worthless after a restart is the wrong trade, so: cache. + # + # THE COST, because `allkeys-lru` means keys really are discarded under memory + # pressure: the rate limiters and the auto-ban FAIL OPEN. An attacker who can + # drive memory usage can age out their own ban entry. The answer is to size + # memory for the working set rather than to switch to `noeviction` — with + # `noeviction` Stalwart's writes to the store start erroring instead, which + # takes out authentication rather than just the limits. Size it, and treat the + # limiters as a nuisance filter, not as a security boundary. + persistence: + enabled: false + + valkey: + maxmemoryPolicy: allkeys-lru + # The subchart derives maxmemory from the computed memory limit and aborts + # its render if an explicit maxmemory exceeds it. Defaults left alone. + resources: + requests: + cpu: 50m + memory: 128Mi + ephemeral-storage: 32Mi + + # The subchart's own NetworkPolicy, with its own semantics — this chart's + # networkPolicy block does not reach it. On by default there, allowing the + # release's own namespace. It needs an entry here only when Stalwart runs in a + # DIFFERENT namespace from the Valkey, which with the subchart it does not. + networkPolicy: + allowedNamespaces: []