feat(stalwart): take our own published valkey chart as the subchart
Publish Helm Chart with patched Application Version / publish-helm-chart (push) Successful in 14s
Publish Stalwart Helm Chart / Publish Stalwart Helm Chart (push) Successful in 14s
SonarQube Analysis / SonarQube Analysis (push) Failing after 13s

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
"<release>-valkey-service", not upstream's bare "<release>-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 <pw>"
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
This commit was merged in pull request #77.
This commit is contained in:
jklein authored and Justin Klein committed 2026-10-02 20:59:17 +00:00
1 parent 780cb8eafd
commit 07c75e80c1
6 files changed
+226 -68

No files matched your search

+83 -27
View File
@@ -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 <password>
# 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: []