From 28db5e2ba40c63b13cf99a8751e2504492016369 Mon Sep 17 00:00:00 2001 From: Justin Klein Date: Tue, 13 Oct 2026 10:19:02 +0000 Subject: [PATCH] stalwart: MTA-STS routes as a maintainable list, in both routing modes Gitea #123. The policy route for mta-sts. moves out of jk-cluster-config into the chart as mtaSts.hosts. stalwart: a new mtaSts values section - the entry form, the two object shapes and why they differ, the hardwired RFC 8461 path, the rejected certificate.extraDnsNames alternative, and the migration order. Three new traps: the chart registers a route and does not create a domain (404 at a correctly routed endpoint, silent on the sender's side); a parent wildcard does not cover mta-sts.sub.; and why one Ingress per cluster issuer cannot be one object. CI section: 2 -> 4 assertions, plus the install-test covering the Ingress half the overlay cannot reach. CI-CD: 58 assertions over 21 charts, why the chart count does not move, and the note that an overlay never covers the Ingress half of a both-modes feature. Chart-Style-Guide: section 4 gains the rule for a host list with a per-entry secret AND a per-entry issuer, and the rule that a both-modes feature must render in both - inert and documented where one mode cannot carry a part of it, never required. Lessons-Learned: the two fields cert-manager reads at different scopes. The annotation is object-scoped and the secret is tls-entry-scoped, so a per-entry optional issuer cannot live on one object - and ingress-shim names the Certificate after the secret and owns it from the Ingress, so a shared secret must sit in exactly one object. Generalises past this chart; --- CI-CD.md | 4 +++ Chart-Style-Guide.md | 4 +++ Lessons-Learned.md | 34 ++++++++++++++++++++ stalwart.md | 75 +++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 116 insertions(+), 1 deletion(-) diff --git a/CI-CD.md b/CI-CD.md index 76db66c..762fc12 100644 --- a/CI-CD.md +++ b/CI-CD.md @@ -170,6 +170,10 @@ Group 3 took it to `56` assertions over `21` charts. Its 19 come from six charts The fail path was exercised rather than assumed: a Service renamed in the route, a port the Service does not expose, a port left out entirely, an emptied `parentRefs` with the chart's own guard disabled, an overlay that renders no route, a run with zero assertions, and — for the pairing — both the redirect rule deleted outright and the redirect pointed at a path other than the one stripped; each produced a `FAIL` naming both sides and a non-zero exit. +**Gitea #123 takes it to `58` over `21`.** `stalwart` grows a second route — the MTA-STS policy hosts — so that chart goes from **2** assertions to **4** while the chart count does not move. That is the ordinary way this figure grows now the sweep is done: a **converted chart gaining an object**, not a new chart. And it only counts because the overlay was extended to enter the new shape (`ci/httproute/stalwart.yaml` now carries an entry with a `clusterIssuer`, one without, and one host on a subdomain) — an overlay left alone would have added a route this test never rendered, which is the anti-vacuum rule biting on a *second* object inside a chart that already passes. + +Note which half that leaves uncovered: an overlay switches a chart into Gateway mode, so **the Ingress half of a both-modes feature is never rendered here.** For `stalwart`'s MTA-STS objects that half sits in `ci/stalwart/test-values.yaml` instead, where the install-test has a real API server accept them. + ### 6. `trivy-config` (reporting only) Renders each chart in the scan matrix and lists what `trivy config` finds. **It never fails the job** (`TRIVY_CONFIG_MODE: report`). Deliberately not a gate yet: the repo has several documented deviations, and a check that goes red on those every time is one people learn to scroll past. Switching the mode to `gate` is the intended step once the seccomp gap is closed. diff --git a/Chart-Style-Guide.md b/Chart-Style-Guide.md index 24f675d..3833a36 100644 --- a/Chart-Style-Guide.md +++ b/Chart-Style-Guide.md @@ -48,6 +48,10 @@ Annotations are per entry too (the set is not uniform across the hostnames — t **Documented exception — freesailarr** has no `ingress.domain` at all and offers both shapes — and therefore no `httpRoute.hostnames` either, the only chart without that key; its hostnames stay in `ingress.host` and `.ingress.host`. See [its page](freesailarr). +**A list of hosts each carrying its own TLS secret *and* its own optional issuer splits into several Ingress objects, and the split is on the issuer.** Added with Gitea #123 (`stalwart.mtaSts.hosts`), and it is a rule rather than a preference because cert-manager reads the two fields at different scopes: `cert-manager.io/cluster-issuer` is an **annotation**, so object-scoped and absent-means-nothing-issued-for-this-object, while `secretName` is **`tls`-entry-scoped** and yields one Certificate per entry. One object therefore cannot both request a certificate from issuer X and leave another host's secret alone — which is what an empty per-entry issuer has to mean, for the reason already in the shared-domain rule above (exactly one certificate per secret). So: **group by issuer for the objects, by secret for the `tls` list** (the `rss-bridge.tlsGroups` helper is the shape to reuse), and **abort the render when one secret appears under two different issuers** — ingress-shim names the Certificate after the secret and owns it from the Ingress, so that case is one Certificate name with two owners. Name such objects after the **issuer**, never after the entry's index: an index is only free where the object holds no certificate. Receipt: [Lessons learned](Lessons-Learned#the-two-fields-cert-manager-reads-at-different-scopes). + +**And a feature that spans both routing modes must render in both.** A values block that produced objects only in Gateway mode (or only in Ingress mode) would make a mode switch silently drop a feature — the same class as the annotation losses of the sweep, but self-inflicted. Where one mode genuinely cannot carry part of it, that part is **inert and documented, not required**: `stalwart`'s `mtaSts.hosts[].tlsSecretName` is required in Ingress mode and ignored in Gateway mode, where TLS belongs to the listener, so neither mode demands a value that does nothing ([§12](#12-values-that-must-not-lie)). + ### The Gateway API is the second routing mode Added with the pilot on `cyberchef` (Gitea #114). A chart offers it through one values block, **off by default**, so adding it to an existing chart is additive — a **minor** bump: diff --git a/Lessons-Learned.md b/Lessons-Learned.md index 29ec0fb..de8d3d2 100644 --- a/Lessons-Learned.md +++ b/Lessons-Learned.md @@ -735,3 +735,37 @@ Both statements are true. The second one is useless: the values file's defect is **The transferable rule: a specific validation must not be reachable in a state that a structural validation already rejects.** Fine-grained checks are the ones that fire first by accident, because they sit closer to the data — and the more useful the message, the more convincing the wrong diagnosis. When adding a per-entry check to a chart that already aborts on a mode conflict, gate the new check on the conflict being absent, and verify it by rendering the conflicting combination rather than by reading the templates. It was found that way here: the combination was in the test matrix for the new value, and the output named the wrong thing. A second, smaller piece from the same round, for anyone adding validation to a chart whose schema is strict: with `additionalProperties: false` and `required`/`minLength` declared, the schema rejects a missing `host` or `realm` **before** any template runs, so the template's own `fail` for those is unreachable through an ordinary `helm template`. Keep it — it is the second guard the repo's `required()` convention already asks for — but check it is not dead by rendering once with `--skip-schema-validation`. A guard nobody has ever seen fire can carry a typo in its own condition for years. +## The two fields cert-manager reads at different scopes + +Gitea #123, and the reason it is here rather than only on the [stalwart](stalwart) page: the cause is not specific to that chart, and the next "list of hosts, each with its own TLS secret" will meet it unchanged. Gitea #122 is already one. + +The approved plan asked for three things that read as independent and are not jointly satisfiable: + +1. **one** Ingress object, one rule per host, `tls:` grouped by secret; +2. a **per-entry** `clusterIssuer`, optional, where empty means *no annotation* — so a secret owned by another release is not fought over; +3. a fixture carrying one entry **with** an issuer and one **without**. + +They collide because cert-manager reads the two fields at different scopes, and nothing in either field's name says so: + +- `cert-manager.io/cluster-issuer` is an **annotation** — object-scoped. An Ingress carries exactly one, or none. With it absent, ingress-shim creates **nothing** for that object: for *every* `tls` entry on it, not for some. +- `secretName` is **`tls`-entry-scoped**, and the shim creates one Certificate **per entry**. + +So an object cannot mix "issued by X" with "comes from elsewhere". Point 2 forces a split on the issuer, and point 1 as written cannot hold. + +And a second rule came out of measuring rather than reasoning — ingress-shim **names the Certificate after the secret** and owns it from the Ingress: + +``` +Certificate tls-mta-sts-bekemeyer-klein-de-ingress + secretName: tls-mta-sts-bekemeyer-klein-de-ingress + ownerReferences: Ingress/mta-sts-ingress +``` + +Two Ingress objects whose `tls` entries name the same secret therefore drive the shim at **one** Certificate name from **two** owners. So a shared secret must sit in exactly one object, and a secret must not span two issuer groups. + +**Now:** group by issuer for the objects, by secret for the `tls` list, and **abort** when one secret appears under two issuers. Three consequences worth carrying forward: + +1. **The failure this avoids is not a render error.** Every shape above produces valid YAML that a cluster accepts. What goes wrong is one Certificate with two claimants, or a certificate silently requested for a secret another release owns — visible in cert-manager's log and nowhere in the release. +2. **"One object" is a shape, not a requirement.** Where an approved plan names an object count, check it against the scope at which the controller reads each field before implementing it. Writing the contradiction down beat resolving it quietly in either direction. +3. **An index is not a free object name once a certificate hangs off it.** [homeassistant](homeassistant) names its routes by index and records that reordering recreates them — *"harmless (a route holds no state or certificate)"*. That parenthesis is a precondition, not an aside: an Ingress that owns a cert-manager Certificate fails it, so these objects are named by their issuer instead. + +**And a second occurrence of the entry above it, which is what turns one chart's story into a rule.** `stalwart`'s per-entry `tlsSecretName` check had exactly the defect the realm-list round describes: with **both** routing modes on and a `tlsSecretName` missing, the render reported *"in Ingress mode every entry needs one"* — true, and the wrong diagnosis, because that configuration is not coherent to begin with. Found by rendering the conflicting combination rather than by reading the template, and fixed the same way: the Ingress-mode block is gated on `ingress.enabled` **and not** `httpRoute.enabled`, so in that state it validates nothing and the only `fail` left is the structural one. Two charts met it in the same round, from two agents, neither having seen the other's code — so expect it in the next chart that gains a per-entry check beside a mode switch, and put the conflicting combination in the test matrix for the new value. diff --git a/stalwart.md b/stalwart.md index bdf08f2..f06850e 100644 --- a/stalwart.md +++ b/stalwart.md @@ -312,6 +312,32 @@ The Gateway-API alternative to the Ingress, **off by default**. Added with group | `httpRoute.parentRefs` | `[]` | The Gateway(s) the route attaches to — Gateway API `ParentReference` entries (`name`, and optionally `namespace`, `sectionName`, `port`, `group`, `kind`), passed through unchanged. **Required while `httpRoute.enabled` is true**: the render aborts on an empty list. The schema requires `name` on every entry and is otherwise left open, because Kubernetes owns that schema and adds fields to it across releases | | `httpRoute.hostnames` | `[]` | Empty means `[ingress.domain]`, so the HTTP hostname has a single source of truth. Note that **the server's own hostname lives in the database**, not in any chart value: after bootstrap the WebUI is reached at `https:///admin`, so a second hostname can be reachable without the application agreeing that it is its name | +### `mtaSts` — the MTA-STS policy routes + +Added with Gitea #123. Stalwart serves the MTA-STS policy itself, per domain and out of its data store; this block is only about **routing a sending MTA to it**, and it renders in **both** routing modes. + +| Value | Default | Meaning | +|---|---|---| +| `mtaSts.hosts` | `[]` | One entry per policy hostname. **The list is the switch** — there is deliberately no `mtaSts.enabled`, because "enabled with an empty list" would be a state with no meaning that the chart would then have to catch. The schema rejects `mtaSts.enabled` outright, so the rule is enforced and not merely documented. Empty renders nothing in either mode and makes no value required | +| `mtaSts.hosts[].host` | — | **Required.** The policy hostname, i.e. `mta-sts.` | +| `mtaSts.hosts[].tlsSecretName` | — | **Required in Ingress mode**, inert in Gateway mode (TLS sits on the Gateway's listener there). **No default on purpose:** the plausible one, this release's own `tls---ingress`, is the *wrong* certificate — it covers `ingress.domain`, not this host. Hosts sharing one secret are collected into a single `tls:` entry, so one certificate with several SANs is requested **once** instead of once per host | +| `mtaSts.hosts[].clusterIssuer` | `""` | Optional, and **empty means no annotation** — "the certificate comes from somewhere else, do not ask cert-manager for it". The same deliberate omission as the qBittorrent object in [freesailarr](freesailarr) and two of the four realm ingresses of Gitea #122. Entries sharing a `tlsSecretName` must agree on it; the render aborts naming both | + +**What is rendered is not the same shape on both sides, and that is forced rather than stylistic:** + +| Mode | Objects | +|---|---| +| Ingress | **one Ingress per distinct `clusterIssuer`** — `-stalwart-mta-sts--ingress`, and `-stalwart-mta-sts-ingress` for the issuer-less group — each with one rule per host and its `tls:` list grouped by secret | +| Gateway | **one `HTTPRoute`**, `-stalwart-mta-sts-httproute`, carrying every hostname. Nothing differs per host (same path, same backend, no filter), and it attaches to the same Gateway(s) as the WebUI route | + +**The path is not a value.** RFC 8461 fixes it at `/.well-known/mta-sts.txt`, rendered with `pathType: Exact` (`type: Exact` in the route) because exactly one file lives there. A values key for a protocol-mandated path is one that can only ever be set wrong — and wrong is invisible from this side, see the traps below. `Exact` also matters in its own right: a prefix on these hostnames would publish the mail server's whole management surface under them. + +The chart's own `ingress.annotations` are **not** copied onto these objects. They belong to the WebUI/JMAP host — on this installation they are its CORS configuration — and a policy file served to sending MTAs has no use for them. + +**Why not fold the names into the mail certificate.** The alternative was real: the chart already has `certificate.extraDnsNames`. It was rejected because that would hang the **mail** certificate on this list — a bad entry here would then fail the issuance of the certificate STARTTLS on 25 and implicit TLS on 465/993 depend on. A policy route that is a nice-to-have would be able to take SMTP and IMAP with it. Separate secrets keep the blast radius at the entry. + +**Migration from `jk-cluster-config`.** Where a route for one of these hosts already exists in another repository — and `mta-sts.bekemeyer-klein.de` does, from the Fleet release `jk-cluster-config-fleet-cd-stalwart-mta-sts` — the order is **remove there first, then add the entry here**. The other way round, ingress-nginx's admission webhook rejects the second object on the same host and path. With the default `[]` this block changes nothing about a running installation. + ### `securityContext` and probes | Value | Default | @@ -508,6 +534,51 @@ There *is* a counterpart, then, and **this chart still does not render it**. Thr **So: configure CORS at the Gateway, with that implementation's own support, before you move this host. Then test it from the webmail in a browser — `curl` sends no preflight, so it will pass while the webmail fails.** +### MTA-STS: the chart registers a route, it does not create a domain + +The limit the `mtaSts` block has to name rather than suggest away, and it is the same genus as the `OnePasswordItem` limit this chart already documents: a promise the chart cannot keep. + +Read the first paragraph of `values.yaml` again — since v0.16 the **domains** live in Stalwart's data store, and the chart never sees them. So whether the server answers for `mta-sts.example.com` depends on `example.com` existing in that data store, which no value here can arrange. + +An entry without its domain in the data store gives a **404 at a correctly routed endpoint**: every Kubernetes object green, the Ingress or route resolving perfectly, nothing wrong on this side at all. And the other half is worse — **MTA-STS failures are silent on the sender's side.** A sending MTA that finds no policy reports nothing here; it simply delivers in the clear. So the thing the block exists to achieve can be absent with no symptom in this cluster whatsoever. + +Two more things the chart does not do, both equally silent: the **`_mta-sts.` TXT record** (without it no sending MTA ever looks for the policy — that is DNS, not Kubernetes), and the **policy's content** (`version`/`mode`/`max_age`/`mx` come from the data store, per domain; the running installation serves `mode: testing`, so sending MTAs enforce nothing). + +Verify every entry from **outside** after adding it — `NOTES.txt` prints the command per host: + +```bash +curl https://mta-sts./.well-known/mta-sts.txt +``` + +### MTA-STS on a subdomain: the parent's wildcard does not cover it + +The trap that will actually catch somebody. `mta-sts.sub.example.com` is **one label deeper** than `*.example.com`, and a wildcard certificate matches exactly one label (RFC 6125) — so a wildcard for the parent domain does **not** cover it, and the obvious assumption ("the wildcard already covers that") is false. + +A secret per entry solves it in Ingress mode, which is why `tlsSecretName` is per entry and has no default. The trap is documented anyway because the symptom is a **TLS error at the sending MTA**: nothing in this cluster logs it, the endpoint looks healthy from here, and mail keeps flowing unencrypted. + +**In Gateway mode the trap moves to the Gateway's listener**, where this chart cannot help at all: a wildcard listener for the parent domain will not terminate TLS for a deeper name, and the route carries no certificate to compensate with. + +### One Ingress per cluster issuer, and why it cannot be one object + +Worth knowing before the next "list of hosts with its own TLS secret" lands in some other chart, because the cause is not specific to this one. + +cert-manager reads the two fields at **different scopes**: + +- `cert-manager.io/cluster-issuer` is an **annotation**, i.e. object-scoped. One Ingress carries exactly one, or none — and with it absent, ingress-shim creates **nothing** for that object, for *every* `tls` entry on it rather than for some. +- `secretName` is **`tls`-entry-scoped**, and the shim creates one Certificate **per `tls` entry**. + +So an object cannot mix "issued by X" with "comes from somewhere else", and an entry-level optional issuer forces a split on the issuer. Measured in `jk-cluster` on 2026-10-13, which is where the second half of the rule comes from: + +``` +Certificate tls-mta-sts-bekemeyer-klein-de-ingress + secretName: tls-mta-sts-bekemeyer-klein-de-ingress + ownerReferences: Ingress/mta-sts-ingress +``` + +**ingress-shim names the Certificate after the secret** and owns it from the Ingress. Two Ingress objects whose `tls` entries name the same secret therefore drive the shim at **one** Certificate name from two owners — so a shared secret must sit in exactly one object (hence the grouping), and a secret may **not** span two issuer groups (hence the abort). + +That is also why the object names carry the **issuer** and not the entry's index. [homeassistant](homeassistant) names its routes by index and writes down that reordering the list recreates them — *"harmless (a route holds no state or certificate)"*. These objects **do** hold a certificate, so the exception that precedent names itself applies here. + ### The Ingress ban trap The one the HA proxy does **not** solve. Stalwart sees the ingress controller's **pod IP** for every JMAP and WebUI request, not the client's, and the auto-ban eventually bans that address — at which point WebUI and JMAP are dead for everyone at once. @@ -650,7 +721,7 @@ What it does prove is the chart's own responsibility, and the first item is why **No Valkey in the fixtures, deliberately.** The in-memory store is a database object, so a Valkey in the namespace would be reachable and unused — making the test look as though it covered clustering while covering none of it. The test runs a single node, which needs no in-memory store. The subchart path is covered by a local render only; a known gap. -**The Gateway mode is held at render level**, by `ci/httproute/stalwart.yaml` plus `ci/run-httproute-match-test.sh`, which runs over every chart on every PR ([CI/CD](CI-CD#httproute-render-test)). The overlay switches this chart into Gateway mode; **2 assertions** — `parentRefs` not empty, and the `backendRefs` entry resolving to **that node group's** `http` Service in the same render. The port is held **by number** (`ports.http.containerPort`), and for this chart that is the check worth having: the NodePort Services publish a *different* number (`ports..nodePort`) than the container binds, so naming the wrong one in a route would be a plausible mistake. The one-route-not-one-per-group property is the other thing the overlay pins down, since it is exactly what a copy would get wrong. +**The Gateway mode is held at render level**, by `ci/httproute/stalwart.yaml` plus `ci/run-httproute-match-test.sh`, which runs over every chart on every PR ([CI/CD](CI-CD#httproute-render-test)). The overlay switches this chart into Gateway mode; **4 assertions** since Gitea #123 — `parentRefs` not empty and the `backendRefs` entry resolving to **that node group's** `http` Service in the same render, for the WebUI route **and** for the MTA-STS route. The overlay's `mtaSts.hosts` enters all three shapes of the entry form, so the second route is not green over a single trivial case: an entry with a `clusterIssuer`, one without, and one host on a **subdomain**. The port is held **by number** (`ports.http.containerPort`), and for this chart that is the check worth having: the NodePort Services publish a *different* number (`ports..nodePort`) than the container binds, so naming the wrong one in a route would be a plausible mistake. The one-route-not-one-per-group property is the other thing the overlay pins down, since it is exactly what a copy would get wrong. **What that does not cover**, named so the gaps are not mistaken for coverage: @@ -659,6 +730,8 @@ What it does prove is the chart's own responsibility, and the first item is why - **that SMTP/IMAP still work.** They never went through the Ingress and do not go through the route; the mail Certificate that serves them keeps being rendered in Gateway mode, which is this chart's distinguishing feature in the sweep, and no render test looks at whether it was left alone. - **`NOTES.txt` itself.** `helm template` does not emit it and no script under `ci/` reads it, so its text carries review-only assurance. +**The MTA-STS Ingress half is covered by the install-test, not by the render test.** The overlay's whole job is to switch the mode *off*, so it never renders those Ingress objects. `ci/stalwart/test-values.yaml` therefore carries two `mtaSts.hosts` entries — one with a `clusterIssuer` and one without, the second a subdomain — which is the only stage that puts them in front of a **real API server**: that two objects differing only by the issuer suffix are both accepted, that `pathType: Exact` on `/.well-known/mta-sts.txt` is valid, and that a `tls:` list grouping several hosts under one secret is taken as written. **Nothing issues either certificate** — cert-manager is not installed in the k3d cluster, so the annotation is inert and the named Secrets never exist, which is harmless because nothing mounts them. + ## See also - [Chart style guide](Chart-Style-Guide) — the conventions this chart follows and the two it deviates from