stalwart: the subchart is our own valkey chart, from the published index

Follows the decision on #65 to depend on our published valkey chart rather
than upstream's. Four pages, and the choice of pages is deliberate.

stalwart.md: the subchart section rewritten - our chart, not upstream's, so
"consumed as it comes" no longer applies; the condition is
valkeySubchart.enabled with the subchart's own error message quoted and the
reason it only appears once the dependency is switched on; the auth-conf field
is a complete "requirepass <pw>" line, not a password; and the cache-vs-queue
choice argued with its cost (allkeys-lru makes the rate limiters and the
auto-ban fail open under memory pressure - size memory, do not switch to
noeviction). Plus a section on the PR-ordering rule.

valkey.md: the same rule, stated FROM THIS SIDE. This is the page someone
editing valkey/ will actually be reading, and this repo's own lesson is to put
the warning where the next change will be made. It also warns against the
tempting repair of adding an "enabled" property here for a key this chart does
not read.

Chart-Style-Guide.md, section 14: two charts carry a dependency now, and point
2 ("we consume their chart as it comes") explicitly does not apply when the
subchart is ours. Adds the PR-ordering rule, the condition-key trap, and the
two measurements that back the published-version form.

Lessons-Learned.md: two entries - the green check that tested the wrong
artifact, and the condition key the subchart rejected (including the part that
makes it dangerous: Helm skips a disabled subchart's validation, so CI stays
green and the error waits for the first operator who enables it). Three lines
in "The recurring shape".

Chart-Style-Guide.md

@@ -192,12 +192,12 @@ Two mechanics come with it: Go's `not` treats **every non-empty string as true**
 
 ## 14. Chart dependencies
 
-The former "no dependencies" rule is gone, replaced by a bar rather than a ban. `ytdlp-web-player` is the only chart using one: it 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.
+The former "no dependencies" rule is gone, replaced by a bar rather than a ban. Two charts use one. `ytdlp-web-player` needed **upstream's** 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. `stalwart` takes **one of our own published charts**, `valkey`, because Stalwart's cluster coordinator rides on the in-memory store connection and a multi-node install does not work without one.
 
 **When a dependency earns its place — all three, not two of three:**
 
 1. The dependent piece would otherwise be a second Helm release that only exists to serve this one, with the wiring between them living in neither chart.
-2. Upstream is the right owner of it. **We consume their chart as it comes: we do not patch it, do not re-harden it, do not re-implement its values.** Where their defaults fall short of these guidelines, that is **documented in our `values.yaml`**, not fixed by us — and a values-level fix stays available to whoever installs it.
+2. Upstream is the right owner of it. **We consume their chart as it comes: we do not patch it, do not re-harden it, do not re-implement its values.** Where their defaults fall short of these guidelines, that is **documented in our `values.yaml`**, not fixed by us — and a values-level fix stays available to whoever installs it. **This point does not apply when the subchart is one of ours** (`stalwart` → `valkey`): there is no third party's defaults to leave alone, because the subchart follows these guidelines by being one of them. What that case adds instead is the PR-ordering rule below.
 3. It is **optional and off by default** (`condition:` in `Chart.yaml`), so the chart is complete without it and a default install renders none of its objects.
 
 **What it costs, all of it visible before the next one is added:**
@@ -207,6 +207,16 @@ The former "no dependencies" rule is gone, replaced by a bar rather than a ban.
 - **`charts/*.tgz` and `Chart.lock` are not committed.** The archive is a third-party binary nobody reviews in a diff, and a committed lock would arrive stale because Renovate rewrites `Chart.yaml` and cannot regenerate it — which is also why the pipelines run `helm dependency update`, not `build`.
 - **The repo's own mechanisms stop at the subchart boundary.** `checksum/config`, the 1Password auto-restart annotation and `scaleDown` reach our templates only. A chart rendering a `OnePasswordItem` for its subchart documents the manual `kubectl rollout restart` that replaces the missing restart-on-change.
 
-**Two implementation notes that cost time otherwise:** a values key containing a dash is unreachable as `.Values.oauth2-proxy` (that parses as a subtraction) and needs `index .Values "oauth2-proxy"`, while `condition:` in `Chart.yaml` splits on dots only and needs no escaping. And inside a subchart `.Chart.Name` is the **subchart's** name — a value templated into it must spell the parent chart out literally; only `.Release.Name` is shared.
+**When the subchart is one of our own charts, one more rule, and it is the sharpest one here:**
+
+> **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. A combined PR therefore runs every `helm` command — lint, package, render **and the install-test** — against the **old** subchart. It goes green on an artifact that is not the one in the diff, the subchart's own change is exercised by nothing, and the first real use of the new subchart is somebody's first install after the merge. Two consecutive PRs: change the subchart, merge, let it publish, then raise the `version:` in the dependent chart.
+
+Two mechanical facts about a self-hosted dependency, measured for `stalwart` rather than assumed, because either could have broken the update path silently: the index is **anonymously readable**, so `ci/helm-dependencies.sh` needs no credential and resolves even a 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 with the dependency pinned low returned `newVersion: "1.0.0+up9.1.2.alpine"`, `versioning: "helm"`. Without the second result the published-version form would have had no update path at all.
+
+**Three implementation notes that cost time otherwise:** a values key containing a dash is unreachable as `.Values.oauth2-proxy` (that parses as a subtraction) and needs `index .Values "oauth2-proxy"`, while `condition:` in `Chart.yaml` splits on dots only and needs no escaping. Inside a subchart `.Chart.Name` is the **subchart's** name — a value templated into it must spell the parent chart out literally; only `.Release.Name` is shared.
+
+And the one that bites hardest: **everything under the subchart's values key is handed to the subchart and validated against *its* schema, so the `condition:` key must not live there.** Our charts ship `additionalProperties: false`, so the conventional `valkey.enabled` is rejected *by the subchart* — `at '': additional properties 'enabled' not allowed`. Helm skips a **disabled** subchart's schema validation, so the default path renders clean and the error appears only for whoever first switches it on. `stalwart` therefore gates on `valkeySubchart.enabled`, a key of the parent's own, which is also the honest shape: `enabled` was never a value of the subchart. It only looked like one because `oauth2-proxy`'s upstream schema is permissive.
 
 `template-chart` remains dependency-free: the scaffold should not carry a `dependencies:` block a new chart has to remember to delete.

Lessons-Learned.md

@@ -292,6 +292,39 @@ An *unparsable* file does abort. It is specifically the missing-file case that f
 
 **Now:** the chart always renders the file and seeds it with an init container, so its absence means something else is broken, and the trap is written on the chart page. Generally: **check what an application does when its configuration is missing, not only when it is wrong** — the two paths are often written by different people, and "absent" is the one that tends to get the lenient fallback.
 
+## The green check that tested the wrong artifact
+
+Caught by reasoning before it cost anything, which is why it reads as a rule rather than an incident — and it is the one hazard that appears only once a repo's own chart becomes somebody's subchart ([stalwart](stalwart) → [valkey](valkey), Gitea #65).
+
+A Helm dependency resolves from the **published index**, not from the working tree, and a chart here is published only by a push to `main` that touches it. Put both changes in one pull request and every `helm` command in that PR — lint, package, render **and the install-test** — pulls the **old** subchart, because the new one does not exist yet. The PR goes green against an artifact that is not the one in the diff, the subchart's change is reviewed while nothing exercised it, and the first real use of the new subchart is somebody's first install after the merge.
+
+**Now:** two consecutive PRs — change the subchart, merge, let it publish, then raise the `version:` in the dependent chart. The warning sits in three places, and the choice of places is half the lesson: `stalwart/Chart.yaml` at the pinned version, the dependent chart's page, and — because that is where the next change will actually be made — the **subchart's own** page, which someone editing `valkey/` is far likelier to be reading than the page of a chart they may not know depends on them.
+
+Generally: **a dependency on your own artifact tests the published copy, not the one you are editing.** Any build that resolves from a registry rather than from the checkout has this property, and the symptom is always a green check on the wrong input.
+
+## The condition key the subchart rejected
+
+Two findings from the same change, both of which could have shipped silently.
+
+**The one that was caught.** The convention for a Helm dependency is `condition: <name>.enabled` — and it is a convention, not a rule: everything under that key is handed to the subchart **as its values** and validated against **its** schema. Our charts ship `additionalProperties: false`, so `valkey.enabled` is rejected by the subchart itself:
+
+```
+Error: values don't meet the specifications of the schema(s) in the following chart(s):
+valkey:
+- at '': additional properties 'enabled' not allowed
+```
+
+What earns this an entry is **when** it appears: Helm skips schema validation for a **disabled** subchart, so the default path renders perfectly clean. Every render, lint and install-test passes, and the error waits for the first operator who switches the dependency on. The repo's previous dependency — upstream's `oauth2-proxy` — never showed it, because that chart's schema is permissive, so the convention looked safe.
+
+**Now:** the condition is a key of the *parent's* own, `valkeySubchart.enabled`, which is also the honest shape, since `enabled` was never a value of the subchart. The tempting repair — adding an `enabled` property to the subchart's schema — was rejected for the reason this page keeps repeating: that chart does not read the key, and a values key that does nothing is worse than its absence. It would also have needed its own PR and a publish first, per the entry above.
+
+**And the two things measured rather than assumed**, because this dependency points at our own self-hosted index and the version carries build metadata (`1.0.0+up9.1.2.alpine`) — either of which 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` — the state a fresh runner is in, so no `helm repo add` and no credential; the index is anonymously readable.
+- 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 returned `newVersion: "1.0.0+up9.1.2.alpine"`, `versioning: "helm"`, with the timestamp read from our index.
+
+The second was load-bearing for the decision, not a nice-to-have: without it the published-version form would have had **no update path at all**, and the comment naming Renovate as the safety net would have been false. "A capability list is about the product; only a run is about the installation" applies to update automation too.
+
 ## The recurring shape
 
 - **Green is a claim about the assertions that ran, not about the system.**
@@ -307,5 +340,8 @@ An *unparsable* file does abort. It is specifically the missing-file case that f
 - **Document a deviation with its mechanism, not its name.** A grant whose comment does not say *why it cannot be removed* reads as dead weight to the next cleanup, and "needed for binding" is worse than nothing when the real reason is `execve`.
 - **Inside a cluster, every per-client identity is a per-node identity.** Masqueraded source addresses and probe traffic both collapse onto node IPs, so any application logic keyed on the client address — bans, limits, geo, audit — is being fed one address, silently and correctly-looking.
 - **Ask what an application does when its configuration is missing, not only when it is wrong.** The lenient fallback is almost always on the "absent" path, and it is the path a broken mount takes.
+- **A dependency on your own artifact tests the published copy, not the one you are editing.** Two changes in one PR means a green check on the wrong input — and nothing in the output says which copy was used.
+- **A convention is not a rule, and the difference shows up in somebody else's validator.** `condition: <name>.enabled` is fine until the subchart has a strict schema; the error then belongs to the subchart and appears only once the feature is switched on.
+- **Validation that is skipped in the default path is not validation.** A check that only runs for the non-default branch passes CI forever and fails for the first person who deviates.
 - **A reference to something that moved is false, not stale.** It still answers the question, which is why nobody re-asks it — so a file leaving the repository is a grep across the whole tree, comments included, not only the lines a tool can resolve.
 - **An answer that arrived by accident is worth making repeatable.** The all-charts install-test exists because one unplanned run produced a result nobody could have asked for; the fix is a way to ask the question on purpose, not a note to remember the accident.

stalwart.md

@@ -18,14 +18,14 @@ So **this chart configures almost nothing, and that is its defining property, no
 | External dependencies | **Postgres** (required), **S3** (blob store), **Valkey** (in-memory store + cluster coordinator) |
 | Secrets | up to 3 × `OnePasswordItem` (data store, recovery admin, S3) |
 | Network | one NodePort Service **per public port** + per-group ClusterIP and headless Services + **NetworkPolicy on by default** |
-| Subchart | optional `valkey` (the Valkey project's own chart), **off by default** |
+| Subchart | optional `valkey` — **this repo's own chart, from the published index**, off by default and gated on `valkeySubchart.enabled` |
 | Install-test | `ci/stalwart/` — and it proves much less than a green run suggests, see [CI](#ci) |
 
 ## Prerequisites
 
 - **Postgres**, reachable from the cluster, with a database and a role for Stalwart. This is the data store and the chart will not render without its host.
 - **S3-compatible object storage** for the blob store. The filesystem blob store is documented as *"available only for single node installations"*, and RocksDB likewise — two nodes force both stores to be external.
-- **A Valkey (or Redis)**, for the in-memory store. Not optional in a cluster: the coordinator's `Default` variant *"reuses the Redis connection already configured on the in-memory store"*, so a multi-node install has no coordination path without one. Either point `externalServices.valkey.host` at an instance (this repo's own [valkey](valkey) chart is the better choice when one application owns it) or set `valkey.enabled: true` for the subchart. **Redis/Valkey is admissible as the in-memory store only — never as the data or blob store.**
+- **A Valkey (or Redis)**, for the in-memory store. Not optional in a cluster: the coordinator's `Default` variant *"reuses the Redis connection already configured on the in-memory store"*, so a multi-node install has no coordination path without one. Either point `externalServices.valkey.host` at an instance or set `valkeySubchart.enabled: true` to deploy this repo's own [valkey](valkey) chart as a subchart. **Redis/Valkey is admissible as the in-memory store only — never as the data or blob store.**
 - **cert-manager**, and for the mail certificate most likely a **DNS-01** issuer — see [TLS](#tls-cert-manager-not-stalwarts-own-acme).
 - **1Password operator**, plus the items in [What the 1Password items must contain](#what-the-1password-items-must-contain).
 - **Inbound reachability on port 25 with matching reverse DNS.** Nothing in the chart can provide this, and without it a mail server is not operable. See [DNS](#dns-a-mail-server-cannot-run-without).
@@ -256,7 +256,7 @@ Egress is restricted **by port, not by destination**: a NetworkPolicy matches CI
 |---|---|---|
 | `externalServices.s3.host` | `""` | the NOTES.txt checklist only |
 | `externalServices.s3.port` | `443` | the egress rule |
-| `externalServices.valkey.host` | `""` | the render-abort check and the checklist; empty + `valkey.enabled` resolves to the subchart's Service |
+| `externalServices.valkey.host` | `""` | the render-abort check and the checklist; empty + `valkeySubchart.enabled` resolves to `<release>-valkey-service` |
 | `externalServices.valkey.port` | `6379` | the egress rule |
 
 Setting a host here does **not** point Stalwart at it. The blob store and in-memory store are database objects created in the WebUI.
@@ -293,13 +293,50 @@ Setting a host here does **not** point Stalwart at it. The blob store and in-mem
 
 The intervals are **calculated, not chosen** — see [The `/healthz` rate limit](#the-healthz-rate-limit-sets-the-probe-intervals).
 
-### `valkey` (subchart)
+### `valkeySubchart` and `valkey` (subchart)
+
+The subchart is **this repo's own [valkey](valkey) chart**, resolved from the published index — not upstream's. So [§14](Chart-Style-Guide#14-chart-dependencies) point 2 ("we consume their chart as it comes") does not apply here: there is no third party's defaults to leave alone, because the subchart follows our conventions by being one of ours.
 
 | Value | Default | Meaning |
 |---|---|---|
-| `valkey.enabled` | `false` | `true` deploys the Valkey project's own chart alongside |
+| `valkeySubchart.enabled` | `false` | `true` deploys our `valkey` chart alongside, reachable at `<release>-valkey-service:6379` |
+| `valkey.*` | see below | Passed **through** to the subchart — these are *its* value names |
 
-Upstream's chart is **consumed as it comes** ([§14](Chart-Style-Guide#14-chart-dependencies)); two of its defaults fall short of our conventions and are documented rather than patched: `auth.enabled` is `false` (unauthenticated access from anything the CNI permits — set `auth.enabled` plus `auth.aclUsers`/`aclConfig`, and prefer `usersExistingSecret` over an inline password), and its NetworkPolicy is its own object with its own semantics that this chart's `networkPolicy` block does not reach. `scaleDown` and `checksum/config` stop at the subchart boundary, and a new Valkey release reaches a cluster only when **this** chart is republished.
+**The switch is `valkeySubchart.enabled`, not `valkey.enabled`, and that is forced.** Everything under `valkey:` is handed to the subchart as its values, and our valkey chart ships `additionalProperties: false` with 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
+```
+
+And because Helm skips a **disabled** subchart's schema validation, that error would surface only for whoever first set it to `true` — the default path renders perfectly clean. The condition therefore lives on a key of this chart's own, which is also the honest shape: `enabled` was never a valkey value.
+
+The values passed through, with the ones that matter:
+
+| Value | Default here | Meaning |
+|---|---|---|
+| `valkey.secrets.authSecretRef` | `null` | **Required** once the subchart is on (the subchart's own `required()` aborts the render). One field, `auth-conf`, holding the **complete line** `requirepass <password>` — **not** a bare password |
+| `valkey.persistence.enabled` | `false` | Cache, not queue — see below |
+| `valkey.valkey.maxmemoryPolicy` | `allkeys-lru` | The other half of the same pair |
+| `valkey.valkey.resources.requests` | `50m` / `128Mi` / `32Mi` | The subchart derives `maxmemory` from the computed limit |
+| `valkey.networkPolicy.allowedNamespaces` | `[]` | The subchart's own policy; needed only when Stalwart runs in a different namespace, which with the subchart it does not |
+
+**It is configured as a CACHE, and that is a decision rather than an inherited default.** 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 coordination pub/sub — every one of those is reconstructible. 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.
+
+**The cost, named rather than discovered later:** `allkeys-lru` really does discard keys under memory pressure, so **the rate limiters and the auto-ban fail open** — someone who can drive memory usage can age out their own ban entry. The answer is to size memory for the working set, *not* to switch to `noeviction`: with `noeviction` Stalwart's writes to the store start erroring instead, which takes out authentication rather than just the limits. Treat the limiters as a nuisance filter, not as a security boundary.
+
+**`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 a cluster only when **stalwart** is republished.
+
+### Never change `valkey/` and `stalwart/` in the same pull request
+
+The dependency resolves from the **published index**, not from the working tree, and `valkey/` is published only by a push to `main` that touches it. In one PR carrying both:
+
+- every `helm` command — lint, package, render **and the install-test** — pulls the **old** valkey `.tgz`, because the new one does not exist yet;
+- the PR goes green against a version of the subchart that is not the one in the diff, so the change to `valkey/` is reviewed while nothing has exercised it;
+- after the merge both publish, and the first install of the new `stalwart` is the first time the new `valkey` runs anywhere.
+
+Two consecutive pull requests: change `valkey/`, merge, let it publish, then raise the `version:` in `stalwart/Chart.yaml`. The green check on a combined PR is a statement about the wrong artifact.
 
 ## Traps
 
@@ -482,5 +519,5 @@ What it does prove is the chart's own responsibility, and the first item is why
 
 - [Chart style guide](Chart-Style-Guide) — the conventions this chart follows and the two it deviates from
 - [Lessons learned](Lessons-Learned) — the source-IP mechanism behind NodePort, which recurs in every chart whose application evaluates the client address
-- [valkey](valkey) — this repo's own single-instance chart, usually the better in-memory store than the subchart
+- [valkey](valkey) — the chart this one takes as a subchart, and the one to install separately when it should outlive this release
 - [Chart index](Charts)

valkey.md

@@ -243,6 +243,19 @@ That must hang or be refused. `NOAUTH Authentication required.` means the policy
 
 The `-alpine` tag was chosen over `-alpine3.24` precisely because both move equally, while only `-alpine` keeps a Renovate rule that cannot freeze: docker versioning splits at the first dash and offers only candidates with the same suffix, and upstream ships exactly one Alpine base per release — so a base change arrives as `9.1.3-alpine3.25`, which a rule bound to `alpine3.24` would never see. The remedy for the moving tag, if it ever matters, is `pinDigests` for this one image; see [Renovate](Renovate).
 
+## This chart is also a subchart — do not change it together with its consumers
+
+Since Gitea #65 **[stalwart](stalwart) takes this chart as a dependency**, resolved from the published index. That makes one rule binding on every change to `valkey/`:
+
+> **Never change `valkey/` 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 this chart is published only by a push to `main` that touches it. So a PR carrying both runs every `helm` command — lint, package, render **and the install-test** — against the **old** `valkey` `.tgz`: it goes green on an artifact that is not the one in the diff, and the change here is reviewed while nothing has exercised it. Change `valkey/`, merge, let it publish, and only then raise the `version:` in the consumer.
+
+Two further consequences of being consumed as a subchart, worth knowing before changing anything here:
+
+- **A change here does not reach a consumer's cluster when this chart publishes.** It arrives when the *consumer* is republished with a raised `version:`. Renovate's `helmv3` manager opens that PR — verified against this self-hosted index — but until it merges, consumers keep shipping the old copy inside their own `.tgz`.
+- **`additionalProperties: false` in this chart's `values.schema.json` is load-bearing for consumers, in a way that is easy to trip over.** Everything a parent puts under its `valkey:` key is handed here and validated against this schema, so a parent that gated the dependency on the conventional `valkey.enabled` is rejected with `at '': additional properties 'enabled' not allowed`. Helm skips a *disabled* subchart's validation, so the error surfaces only for whoever first switches it on. `stalwart` therefore gates on a key of its own, `valkeySubchart.enabled`. **Do not "fix" this by adding an `enabled` property here** — this chart does not read one, and a values key that does nothing is worse than its absence.
+
 ## Upgrades and backup
 
 - **A Valkey upgrade is a restart, and a restart is a data question**, not a version question. With `persistence.enabled: false` the dataset is gone; with AOF it is replayed. Plan the upgrade of a *queue* instance for a moment when losing in-flight work would be tolerable anyway.