feat(stalwart): MTA-STS routes as a maintainable list, in both routing modes #126
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: jlab-cluster/helm-charts#126
Reference in new issue
Block a user
Gitea #123:
stalwart's MTA-STS policy route moves out ofjk-cluster-configand into the chart as a maintainable list, rendering in both routing modes.mtaSts.hosts: []is the default, so this changes nothing about the running installation until it is filled.Minor, additive, empty default —
1.2.0+upv0.16.25→1.3.0+upv0.16.25.helm templateagainstorigin/mainis byte-identical for the defaults and forci/stalwart/test-values.yamlas it stands onorigin/main.Where I could not take the approved plan as written, and this is the part to review
Points 2, 4 and 9 of the issue are not jointly satisfiable, and the cause is cert-manager. The plan asks for one Ingress with one rule per host (4), a per-entry optional
clusterIssuerwhere empty means no annotation (2), and an overlay carrying one entry with an issuer and one without (9). Those three cannot all hold:cert-manager.io/cluster-issueris an annotation — object-scoped. One Ingress carries exactly one, or none, and with it absent the shim creates nothing for that object — for everytlsentry on it, not for some.secretNameistls-entry-scoped, and the shim creates one Certificate per entry.So one object cannot both request a certificate from issuer X and leave another host's secret alone, which is exactly what point 2's empty issuer has to mean. Requirement 9 therefore needs at least two objects.
And a second rule came out of measuring rather than reasoning.
kubectl -n stalwart get certificates, 2026-10-13:ingress-shim names the Certificate after the secret and owns it from the Ingress. Two objects whose
tlsentries name one secret therefore drive the shim at one Certificate name from two owners.What I implemented instead: one Ingress per distinct
clusterIssuer, one rule per host inside it,tls:grouped by secret inside it, and a render abort when one secret appears under two issuers. Point 4's "one rule per host" and "tls grouped by secret" hold literally; only the object count splits, and only on the field that must. The grouping isrss-bridge.tlsGroupsadopted rather than reinvented, as point 4 asks — what is new is the issuer dimension, which that chart does not have (it sets one chart-wide issuer on its single object). Say so if you would rather have the restriction "all entries must agree on one issuer" and a single object; that is the only other coherent shape and it would make requirement 9 impossible.Object names: by index —
<release>-stalwart-mta-sts-ingress,-1, … in this repo's idiom ({{ if $i }}-{{ $i }}{{ end }}), ashomeassistant's routes andkeycloak's realm Ingresses do it. This reverses an earlier draft of this PR, and the reversal is the interesting part.I had named them by issuer, reading the style guide's index rule as resting on "acceptable only because a route holds no state and no certificate" and observing that these objects hold one. That reading was right — and the rule's stated reason was wrong. The parallel #122 round named by index, cited the same rule as binding, and entered
keycloakinto it, whose realm Ingresses own certificates too. So two charts did opposite things for one shape, and the rule had been extended onto exactly the cases its own precondition excluded. #125 is merged and published, so the reason was repaired and the naming unified on index;keycloakwas not touched.What actually carries the index rule, in three steps with deliberately different standing:
jk-cluster): cert-manager runs without--enable-certificate-owner-ref— absent from the Deployment's args and from theControllerConfigurationits--confignames (which carries onlykind:andenableGatewayAPI: true). The controller default applies.And the promise hangs on a cluster-level flag this repository does not control. With it switched on, a reorder would cost a fresh issuance per renumbered object — worth checking before anyone enables it, and not something a chart can defend against.
Unchanged by the rename: the structure. One Ingress per distinct
clusterIssuer, one rule per host,tlsgrouped by secret, and the abort when a secret spans two issuers. Grouping is a cert-manager question, naming is a style question, and they are independent.Two obligations the rename creates, both discharged:
NOTES.txtprints the mapping — index, object name, issuer, secrets and hosts per object, the waykeycloak's does for its realms — because the name no longer carries the issuer and nothing else in the cluster says which object holds which. Visible in the install-test log on this head:mta-sts-ingresswithcluster issuer: "ci-selfsigned",mta-sts-ingress-1withcluster issuer: NONE.homeassistant's. There an index names a host. Here it names an issuer group derived from the list, so moving one entry can change the grouping and renumber objects that were not themselves moved.An asymmetry with
keycloak, named in the template header: there, both modes render one object per entry, so the index question arises twice. Here it arises on the Ingress side only — Gateway mode renders a singleHTTPRoutecarrying every hostname, TLS being none of a route's business, so there is no index and no name to go stale.Version stays
1.3.0+upv0.16.25: nothing is published yet, so nothing in a release is being renamed.Where I reproduced the briefing exactly
Every live measurement in the task held, with nothing to correct:
stalwart, one from releasejk-cluster-config-fleet-cd-stalwart-mta-stsmta-sts.bekemeyer-klein.de, one path/.well-known/mta-sts.txt,pathType: Exactstalwart-stalwart-full-http-service, port namehttpports.http.containerPort, so the route'sbackendRefsnumber is read from the template and not guessedtls-mta-sts-bekemeyer-klein-de-ingress,cluster-issuer: letsencrypt-issuer, no nginx annotationsfield.cattle.ioandobjectset.rio.cattle.io, controller-added)git grep -i 'mta-sts|well-known'overstalwart/onorigin/mainversion: STSv1,mode: testing,max_age: 604800,mx: mail.bekemeyer-klein.decurlagainst the live hostChart.yamlat1.2.0+upv0.16.25/appVersion: "v0.16.25"The two things the issue body did not say explicitly
1. The new objects hang on the mode switch correctly, and no required value does nothing. Executed rather than assumed:
ingress.enabled: true, list setingress.enabled: false,httpRoute.enabled: true, list setCertificatestill renderedfalse, list setCertificatestill rendered, andNOTES.txtsays the list renders nothing and whytlsSecretNameand noclusterIssuertlsSecretNameingress.domainis untouched by all of this: it stays "REQUIRED either way" for the mail certificate'sdnsNames, and the MTA-STS list is independent of it — the hosts are their own values, so nothing new hangs off it.2. Nothing outside the routing templates reads
ingress.enabledin this chart. Screened overstalwart/templates/andstalwart/values.schema.json: the only readers arestalwart.ingress.yaml,stalwart.httproute.yaml(its mutual-exclusion guard) andNOTES.txt, which already goes through$published = ingress.enabled OR httpRoute.enabledfrom group 3. No defect of thefreesailarr/ytdlp-web-playerclass here — measured, not assumed.stalwart.certificate.yamlreadsingress.domainandingress.clusterIssuer, both with their ownrequired(), which is correct and unchanged.A defect found after opening this PR, and found because the parallel round wrote its lesson down
Worth reading as a result about the process as much as about the chart. While pushing the wiki I had to rebase onto the keycloak round's (#122)
Lessons-Learnedentry, read it, and recognised my own code in it: a specific validation must not be reachable in a state that a structural validation already rejects.Rendered the combination it names, against my own branch — and it reproduced:
True, and the wrong diagnosis: the values file's defect is that
ingress.enabledandhttpRoute.enabledare both true, and "in Ingress mode" is a claim about a configuration that is not coherent at all. Whoever met it would add atlsSecretName, re-run, and reach the real message one round later — or conclude this feature was the problem.Fixed in
9f96204by scope, not by ordering (Helm renders templates in an order a chart cannot pin, so this template may not assume the httproute template's abort runs first): the Ingress-mode MTA-STS block is gated oningress.enabledand nothttpRoute.enabled, so in the incoherent combination it validates nothing and the onlyfailleft is the structural one. Re-measured after the fix:tlsSecretNamemissingtlsSecretNamemissingAll three empty render diffs and the assertion figure were re-measured after the fix and were unchanged by it (it was
58/21at that point; see the rebase history below for why that number moved twice afterwards for reasons that have nothing to do with this fix). Two independent charts met this in the same round, from two agents, neither having seen the other's code — so I added that second occurrence to theirLessons-Learnedentry rather than writing a competing one, since one chart's story and two charts' is the difference between an anecdote and a rule.And the
#-comment trap was avoided. Every explanatory comment in the new template blocks is inside{{/* */}}or{{- /* */}}. Confirmed twice: by the empty render diffs, and by grepping the rendered MTA-STS objects in both modes for stray#lines — 0 in each.Verification
ci/helm-dependencies.sh stalwartfirst (valkey), or every helm command exits 1.helm lint --strictthree ways — defaults, CI values, CI values + Gateway overlay:1 chart(s) linted, 0 chart(s) failedeach.helm package→stalwart-1.3.0+upv0.16.25.tgz.helm templatediff againstorigin/main, twice: the defaults (14 objects, identical) andci/stalwart/test-values.yamlas it stands onorigin/main, plus a third for the Gateway overlay as it stands there. With the new fixture the diff is non-empty by construction — that is the feature, and it adds 2 Ingress objects.letsencrypt-issuerone carrying 3 rules and 2tlsentries, with entries 1 and 3 collapsed under one secret as 2 SANs, and the issuer-less one carrying 1 rule and no annotation. Gateway mode gives 1 route, 4 hostnames,type: Exact,port: 8080.ci/run-httproute-match-test.sh: 64 assertions held over 22 charts, 0 failed, read from CI on the final head (runs/814/jobs/2740) — unchanged by the rename, which was expected but measured rather than assumed: object names are not assertions.stalwart2 → 4, both routes'parentRefsand bothbackendRefsresolving tohttproute-stalwart-full-http-service:8080in the same render.ci/run-quantity-render-test.sh(28 exercised, 0 failed) andci/run-selector-match-test.sh(5 selectors, 0 failed) both clean.mtaSts.hosts[0].bogusrejected (additional properties 'bogus' not allowed); an entry withouthostrejected (missing property 'host') — caught by the schema before the template; andmtaSts.enabledrejected, so "the list is the switch" is enforced rather than merely documented.host), missingtlsSecretNamein Ingress mode, both modes at once with the list set (the existing abort fires, naming both value paths), a duplicate host — which aborts in both modes, from each template, since the guard sits in the shared helper — and one secret under two issuers. Each printed its full message and exited non-zero.NOTES.txtreviewed by reading, in all three branches (Ingress / Gateway / neither), rendered through a scratch chart copy becausehelm templatedoes not emit notes — andhelm install --dry-runcannot be used for the Gateway branch here, since this workspace's identity is forbiddenhttproutesand helm looks the object up. One layout defect found that way and fixed: the→ node groupline sat after a two-lines-per-entry host list and read as part of the last entry; it now precedes the list.What stays unproven, by construction
HTTPRouteever reachesAccepted: True. That this repository's Gateway mode routes at all is measured onfreesailarrin production (#114) — that is evidence for the form, not for this chart and not for these routes.values.yaml, the schema,NOTES.txt(with acurlper host) and the wiki; documentation is not something a test can assert.NOTES.txtis read by no pipeline stage.helm templatedoes not emit it and nothing underci/reads it, so its text carries review-only assurance. A rendering error in it does failhelm template, which is all CI can see. (Unchanged from group 3.)http-less node-group guard in both new blocks is unreachable, and I am reporting that rather than claiming it exercised:stalwart.sfs.yamlaborts first (nodes.full.exposedPorts does not include "http"). The identical guards instalwart.ingress.yamlandstalwart.httproute.yamlare equally unreachable onmain; the new ones are there for symmetry, not because they can fire.kubectl apply. Thekubectluse was read-only (get ingress,get svc,get certificates) assystem:serviceaccount:coder-workspaces:claude-readonly.Not in this PR
_mta-sts.<domain>TXT; DNS, not Kubernetes.mode: testing. Stalwart data-store configuration, per domain.jk-cluster-config. A migration, not a chart change: remove there first, then fill the list — the other way round ingress-nginx's admission webhook rejects the second object on the same host and path. With the default[]this PR touches the running installation not at all.jk-cluster-configis untouched.keycloak,rss-bridge,certificate.extraDnsNames(the rejected alternative — folding the names into the mail certificate would let a bad policy entry fail the certificate STARTTLS and IMAPS depend on).README line 184 — the three-round collision, as it actually played out
I was the third of three parallel rounds, and all three change the HTTPRoute test's figure in that one sentence. It took two rebases, and the history is worth reading because the convention that resolved it was not mine:
56/21and I appended58/21.rss-bridge) merged, rewriting the paragraph and adding8896fc8"the 58/22 figure is a delta, not a running total". It had measured58/22from a different delta — both sentences said58and disagreed, the most confusing shape this could have taken. Rebase 1: kept its paragraph word for word, appended my delta, re-measured →60/22.56/21 → 58/22 → 62/22and turning this PRmergeable: false. Rebase 2 onto649417b: kept that whole chain verbatim and appended this round's delta.The number is read, not computed, and that is this paragraph's own rule.
62 + 2 = 64would be arithmetic over a branch whose substructure changed after its2were measured. The committed figure comes fromci/run-httproute-match-test.shon the rebased branch and is confirmed by CI on the final head —runs/814/jobs/2740:So this run is the one the convention was waiting for: being last of the three to be ready, its own run is "the first that holds all of them", and
64/22is the first figure in that sentence that is a genuine running total rather than a delta. The arithmetic agreeing is a check, not the source — it only agrees because nothing else moved in between, and a round that changed an existing chart's routes instead of adding a chart would not add up that way at all.Nothing of anyone else's text was reverted or reworded. Verified mechanically rather than asserted:
README.mddiffers fromorigin/mainin exactly one line, and that line starts withmain's version character for character — the change is a pure append. The commit that set the obsolete60/22was dropped in the second rebase rather than carried, since its only content was a number that no longer holds.Checked for substantive overlap, not just textual. The five commits
maingained touchREADME.md,keycloak/**,rss-bridge/**,ci/keycloak/test-values.yaml,ci/httproute/keycloak.yamlandci/httproute/rss-bridge.yaml— nostalwart/, noci/stalwart/, noci/httproute/stalwart.yaml, andci/run-httproute-match-test.shwas not touched at all (it computes its figure, so it carries no number to go stale). The only overlap wasREADME.md, exactly as expected. Re-verified after the rebase: both empty render diffs against649417bstill hold,helm lint --strictclean,stalwartstill contributes4.The force-pushes were rebases of my own branch only — never
main, never the wiki.Wiki
4 pages, pushed after a fetch and rebase:
stalwart— a new### mtaStsvalues section (entry form, the two object shapes and why they differ, the hardwired path, the rejectedextraDnsNamesalternative, the migration order), three new trap sections (route-not-domain, the subdomain wildcard, one Ingress per cluster issuer and why it cannot be one object), and the CI section updated to 4 assertions plus the install-test's coverage of the Ingress half.CI-CD— the delta of+2, and after both other rounds merged the paragraph now cashes in the delta rule:64over22, named as the first run holding all three. The intermediate58/22,60/21and60/22are kept legible rather than tidied away, each correct for what it was read on and none of them a total, with62 + 2 = 64flagged as a check and not the source. Also why the chart count does not move — the first entry in that history where a chart already in the list gained an object — and that an overlay never covers the Ingress half of a both-modes feature. Corrected twice, since my first wiki commit said58/21and my second60/22.Chart-Style-Guide— the index rule's broken reason repaired (the three-step version above, replacing "acceptable only because a route holds no state and no certificate"), withstalwartandkeycloakadded as the two certificate-bearing cases andhomeassistantnamed as the certificate-free original; plus §4's rule for a host list with per-entry secret and per-entry issuer — corrected from my own earlier "name after the issuer" to "name by index", with theNOTES.txtmapping obligation and the sharper reorder hazard that follow from it.Lessons-Learned— "The two fields cert-manager reads at different scopes", the generalisation for the next list-of-hosts-with-its-own-TLS-secret; the second occurrence of #122's validation-precedence entry, appended to theirs rather than written as a competing one; and my own third point replaced, since it argued the position that was overruled. What replaces it is what the round actually taught: a rule can be right while its written reason is wrong; a justification is load-bearing because it is what someone reaches for when the rule meets a case its author did not foresee; and parallel rounds are a conformance test nobody wrote — run sequentially, the second would have followed the first and the broken reason would have survived unexamined.No new page, so
_Sidebar.mdis unchanged.The wiki push collided twice and was merged, never forced. Both other rounds had already pushed their wiki edits (their PRs are still open), so my first rebase hit a conflict in
Lessons-Learned.mdwhere their new section and mine were appended at the same place — resolved by keeping both, theirs first. A second commit landed while I was amending, so I fetched and rebased again.git rev-list --left-right --count origin/main...HEAD→0 1before each push; no force-push, nothing of theirs reverted. Wiki commit28db5e2, signed.Commits are signed (
%G?→G). Opened under the bot account viatea.Fixes #123
9f9620431ftobf7640104fThe policy route for mta-sts.<domain> belonged to another repository (jk-cluster-config) while pointing at this chart's own Service. It moves here as mtaSts.hosts, a list, so several domains - subdomains included - are maintainable in one place and in whichever routing mode is active. THE LIST IS THE SWITCH. No mtaSts.enabled: "enabled with an empty list" would be a state with no meaning that the chart would then have to catch. The schema rejects the key outright, so the rule is enforced rather than documented. Default [] renders nothing in either mode - the defaults and the unchanged CI fixture both produce a byte-identical helm template against origin/main. Entry form {host, tlsSecretName, clusterIssuer?}. The two TLS fields are read ONLY in Ingress mode; in Gateway mode TLS sits on the listener, so requiring them there would demand a value that does nothing. THE PATH IS HARDWIRED. RFC 8461 fixes /.well-known/mta-sts.txt, rendered with pathType Exact (type: Exact in the route). A values key for a protocol-mandated path can only ever be set wrong, and wrong is invisible from this side. Exact also matters in itself: a prefix on these hostnames would publish the mail server's whole management surface under them. ONE INGRESS PER CLUSTER ISSUER, NOT ONE OBJECT - and the plan's own three requirements forced that. cert-manager reads the two fields at different scopes: cluster-issuer is an ANNOTATION (object-scoped, and absent means nothing is issued for that object at all), while secretName is tls-entry-scoped and yields one Certificate per entry. So an object cannot both request a certificate and leave another host's secret alone, which is what an empty per-entry issuer has to mean. Measured in jk-cluster: ingress-shim names the Certificate AFTER THE SECRET and owns it from the Ingress, so a shared secret must sit in exactly one object (hence grouping the tls list by secret, reusing rss-bridge.tlsGroups' shape) and a secret may not span two issuer groups (hence the abort). Objects are named by issuer, not by index: an index is only free where the object holds no certificate, which is the precondition homeassistant's route naming states for itself. Gateway mode renders ONE HTTPRoute with every hostname - nothing differs per host there - attached to the same Gateway(s) as the WebUI route. WHAT THE CHART CANNOT PROMISE, AND THEREFORE NAMES: it registers a route, it does not create a domain. The domains live in Stalwart's data store, so an entry whose domain is missing there answers 404 at a correctly routed endpoint with every object green - and MTA-STS failures are silent on the SENDER's side, which delivers in the clear and logs nothing here. Same for the _mta-sts.<domain> TXT record and the policy's content. The subdomain trap is documented too: mta-sts.sub.<domain> is one label below *.<domain>, so a parent wildcard certificate does not cover it. Not folded into certificate.extraDnsNames deliberately - that would hang the MAIL certificate on this list, letting a bad policy entry take STARTTLS and IMAPS with it. Minor, additive, empty default: 1.2.0+upv0.16.25 -> 1.3.0+upv0.16.25. CI: ci/httproute/stalwart.yaml enters all three shapes of the entry form (with issuer, without, and a subdomain), taking the HTTPRoute render test from 56 assertions over 21 charts to 58 over 21 - stalwart from 2 to 4. The Ingress half cannot be covered there, since an overlay's whole job is to switch the mode off, so ci/stalwart/test-values.yaml carries two entries and the install-test has a real API server accept them. Fixes #123bf7640104fto71b757bf9fMaintainer's decision, and it reverses this chart's naming rather than the style guide's rule. The objects become <release>-stalwart-mta-sts-ingress, -1, -2, ... in this repo's idiom ({{ if $i }}-{{ $i }}{{ end }}), the same as homeassistant's routes and keycloak's realm Ingresses, inlined in the template instead of behind a helper of its own. WHY IT GOES THIS WAY. This chart named by issuer, reading the style guide's index rule as resting on "a route holds no state and no certificate" and observing that these objects hold one. That reading was right and the rule's STATED REASON was wrong: the parallel #122 round named by index, cited the same rule as binding, and entered keycloak into it - whose realm Ingresses hold certificates too. Two charts did opposite things for one shape, and the rule had been extended onto cases its own precondition excluded. #125 is merged and published, so the reason is repaired and the naming unified on index; keycloak is not touched. What actually carries the index rule, in three steps that have different standing and are written as such in the template header and the style guide: cert-manager here runs WITHOUT --enable-certificate-owner-ref (MEASURED by the maintainer - absent from the Deployment's args and from the ControllerConfiguration its --config names, which carries only kind: ControllerConfiguration and enableGatewayAPI: true); by that default the Certificate is collected with its Ingress but the SECRET IS NOT (DOCUMENTED behaviour, not measured here), so a rename recreates the Certificate object against a surviving secret and issues nothing; and NO RENAME WAS APPLIED against a cluster. The promise therefore hangs on a cluster-level flag this repository does not control - with it on, a reorder would cost a fresh ACME request per renumbered object. UNCHANGED: the structure. One Ingress per distinct clusterIssuer, one rule per host, tls grouped by secret, and the abort when one secret spans two issuers - grouping is a cert-manager question and naming is a style question, and they are independent. TWO THINGS THE RENAME OBLIGES. 1. NOTES.txt now prints the mapping - index, object name, issuer, secrets and hosts per object, the way keycloak's does for its realms - because the name no longer carries the issuer and nothing else in the cluster says which object holds which. 2. The reordering hazard is documented as SHARPER than homeassistant's: there an index names a host, here it names an issuer GROUP derived from the list, so moving one entry can change the grouping and renumber objects that were not themselves moved. AN ASYMMETRY WITH keycloak, named in the template header: there both modes render one object per entry, so the index question arises twice. Here it arises on the Ingress side ONLY - Gateway mode renders one HTTPRoute carrying every hostname, because TLS is none of the route's business, so there is no index and no name to go stale. Version stays 1.3.0+upv0.16.25: nothing is published yet, so nothing is being renamed in a release. Re-measured after the rename, not assumed: three empty helm template diffs against origin/main (defaults, CI fixture, overlay), helm lint --strict clean three ways, every abort path still firing with the structural one still winning over the per-entry one, the schema still rejecting mtaSts.enabled, and the HTTPRoute render test still at 64 assertions over 22 charts - object names are not assertions.