diff --git a/stalwart/templates/NOTES.txt b/stalwart/templates/NOTES.txt index 377e691..42c7ae5 100644 --- a/stalwart/templates/NOTES.txt +++ b/stalwart/templates/NOTES.txt @@ -331,20 +331,45 @@ WEB UI AND JMAP — INGRESS MODE (ingress.enabled: true, httpRoute.enabled: fals Ingress resolves to a port that answers nothing. {{- if $mtaStsHosts }} -MTA-STS — {{ len $mtaStsHosts }} POLICY ROUTE(S), INGRESS MODE +{{- $mtaStsGroups := fromYaml (include "stalwart.mtaSts.ingressGroups" .) }} + +MTA-STS — {{ len $mtaStsHosts }} POLICY HOST(S) IN {{ len $mtaStsGroups.order }} INGRESS OBJECT(S), INGRESS MODE All of them → node group "{{ $ingressGroup }}", path {{ $mtaStsPath }} only (pathType Exact) -{{ range $entry := $mtaStsHosts }} - https://{{ $entry.host }}{{ $mtaStsPath }} - secret {{ $entry.tlsSecretName }}{{ if $entry.clusterIssuer }}, requested from the "{{ $entry.clusterIssuer }}" cluster issuer{{ else }}, NO cluster issuer set — this release does not request that certificate, it has to exist already{{ end }} + + ONE OBJECT PER CLUSTER ISSUER, NAMED BY INDEX — so the name does not say + which issuer it carries, and this is the mapping. Read it here rather than + guessing from the object names: +{{- range $i, $issuer := $mtaStsGroups.order }} +{{- $grp := index $mtaStsGroups.groups $issuer }} + + {{ printf "%s-mta-sts-ingress%s" $release (ternary "" (printf "-%d" $i) (eq $i 0)) }} + cluster issuer: {{ if $issuer }}"{{ $issuer }}" — cert-manager requests these certificates{{ else }}NONE — no annotation, so this release requests nothing; every secret below has to exist already{{ end }} + {{- range $secret := $grp.tlsOrder }} + secret {{ $secret }} + {{- range $host := index $grp.tls $secret }} + https://{{ $host }}{{ $mtaStsPath }} + {{- end }} + {{- end }} {{- end }} - One Ingress object PER CLUSTER ISSUER, not one per host: the cert-manager - issuer is an object-level ANNOTATION while the TLS secret is a per-entry - field, so one object cannot both request a certificate and leave another + Why one object per issuer rather than one for everything: the cert-manager + issuer is an object-level ANNOTATION while the TLS secret is a per-tls-entry + field, so a single object cannot both request a certificate and leave another host's secret alone. Hosts sharing a secret share one tls: entry, so one certificate with several names is requested once instead of once per host: kubectl -n {{ .Release.Namespace }} get ingress -l app={{ $release }} + AND MIND THE ORDER OF mtaSts.hosts. The index above runs over the issuer + GROUPS, which are derived from the list — so reordering entries can change + how many objects there are and which index carries which issuer, renumbering + objects that were not themselves moved. Kubernetes then deletes and recreates + them, which takes their Certificate objects with them. No certificate is + RE-ISSUED by that, because the Secrets survive it (cert-manager leaves them + alone unless it runs with --enable-certificate-owner-ref, which this cluster + does not set) — but that is a property of the CLUSTER, not of this chart. If + that flag is ever switched on, a reorder means a fresh ACME request per + renumbered object. + TWO THINGS THIS RELEASE DOES NOT DO, AND BOTH ARE SILENT WHEN MISSING: 1. IT DOES NOT CREATE THE DOMAIN. The domains live in Stalwart's data diff --git a/stalwart/templates/_helpers.tpl b/stalwart/templates/_helpers.tpl index 5da0287..08a314f 100644 --- a/stalwart/templates/_helpers.tpl +++ b/stalwart/templates/_helpers.tpl @@ -695,29 +695,23 @@ logged on this side. {{- end -}} {{/* -The name of the MTA-STS Ingress object for one clusterIssuer group. Input: dict -"root" "issuer" . +NOTE ON THE OBJECT NAMES, because this is where an earlier draft of this chart +disagreed with the rest of the repository and lost. -NOT AN INDEX, and the reason is the one exception the comparable precedent -names itself. homeassistant carries the entry's index in its HTTPRoute names -(Gitea #114, group 3) and writes down that reordering the list therefore -recreates rather than edits the objects - "harmless (a route holds no state or -certificate)". These objects DO hold a certificate: cert-manager owns the -Certificate from the Ingress, so a rename deletes one Certificate and creates -another. The Secret survives that - cert-manager does not delete it with the -Certificate - so no re-issue follows and nothing breaks, but it is churn on an -ACME account in return for a reordered list, and the issuer is a stable key -that costs nothing to use instead. +The MTA-STS Ingress objects are named BY INDEX over the issuer groups - +"-stalwart-mta-sts-ingress", then "-1", "-2" - inlined in +stalwart.ingress.yaml in the same idiom as homeassistant's routes and +keycloak's realm Ingresses ({{ if $i }}-{{ $i }}{{ end }}), rather than by a +helper of their own. -The issuer-less group takes the unsuffixed name, and that cannot collide with -an issuer's: a clusterIssuer name is never the empty string, so no suffix is -ever empty. +An earlier draft named them after the clusterIssuer, arguing that the style +guide's index rule rested on "a route holds no state and no certificate" and +that these objects hold one. The reading of the rule was right and the rule's +STATED REASON was wrong - that sentence has been corrected in the style guide +(section 4) rather than the naming of two charts, because keycloak's realm +Ingresses hold certificates too and are already published. What actually +carries the index rule is that THE SECRET SURVIVES a rename, so no certificate +is re-issued; the full three-step version, including which step is measured and +which is only documented behaviour, is in that section and in the header of +stalwart.ingress.yaml. */}} -{{- define "stalwart.mtaSts.ingressName" -}} -{{- $base := printf "%s-mta-sts" (include "stalwart.fullname" .root) -}} -{{- if .issuer -}} -{{- printf "%s-%s-ingress" $base .issuer -}} -{{- else -}} -{{- printf "%s-ingress" $base -}} -{{- end -}} -{{- end -}} diff --git a/stalwart/templates/stalwart.ingress.yaml b/stalwart/templates/stalwart.ingress.yaml index 56c882e..a740348 100644 --- a/stalwart/templates/stalwart.ingress.yaml +++ b/stalwart/templates/stalwart.ingress.yaml @@ -108,6 +108,56 @@ AND THE LIMIT THAT NO OBJECT HERE CAN FIX: this chart registers a ROUTE, it does not create a DOMAIN. Whether Stalwart answers for mta-sts. depends on existing in its data store, which is not in any chart value (see values.yaml, line 1 onwards, and the mtaSts block). + +THE NAMES ARE INDEXED OVER THE ISSUER GROUPS, AND THE INDEX IS ONLY AN INGRESS +QUESTION + +"-stalwart-mta-sts-ingress", then "-1", "-2" - the repo's idiom +({{ if $i }}-{{ $i }}{{ end }}), as homeassistant's routes and keycloak's realm +Ingresses use it (style guide section 4). The index runs over the ISSUER +GROUPS, not over mtaSts.hosts entries: the grouping is a separate decision, +made for cert-manager's sake and argued in stalwart.mtaSts.ingressGroups, and +it is untouched by how the objects are named. + +AN ASYMMETRY WITH keycloak WORTH KNOWING: there, both modes render one object +per entry, so the index question arises twice. HERE IT ARISES ONLY ON THIS +SIDE. Gateway mode renders exactly ONE HTTPRoute carrying every hostname, +because TLS is not involved there at all - no certificate, no secret, nothing +to split an object over - so there is no index and no name to go stale. The +whole of the naming question below is an Ingress-mode question. + +THE REORDERING HAZARD IS SHARPER HERE THAN AT homeassistant, so read this +before shuffling mtaSts.hosts. There, an index names a host, and reordering +changes which index serves which host. Here an index names an ISSUER GROUP +derived from the list, so reordering can change THE GROUPING ITSELF - how many +objects exist, and which index carries which issuer and which hosts. Moving +one entry can therefore renumber objects that were not themselves moved. What +makes it bearable rather than a reason to avoid indices: + + 1. MEASURED (by the maintainer, in jk-cluster, on this round): cert-manager + runs WITHOUT --enable-certificate-owner-ref - the flag appears neither in + the cert-manager Deployment's args nor in the ControllerConfiguration its + --config points at (which carries only kind: ControllerConfiguration and + enableGatewayAPI: true). So the controller's default applies. + 2. DOCUMENTED BEHAVIOUR of that default, not measured here: the Certificate + is garbage-collected with the Ingress it is owner-referenced from, but the + SECRET is not - cert-manager only adds an owner reference to the Secret + when that flag is on. A rename therefore deletes and recreates the + Certificate object while the Secret stays, so the new Certificate finds a + valid secret already present and ISSUES NOTHING. No ACME request, no rate + limit touched. + 3. NOT APPLIED: no rename was actually performed against a cluster. The two + steps above are the mechanism, not a trial of it. + +And note what the promise hangs on: a CLUSTER-LEVEL FLAG this repository does +not control. With --enable-certificate-owner-ref switched on, the Secret would +be collected with the Certificate and a reorder WOULD mean a re-issue for every +renumbered object. That is worth checking before anyone turns it on, and it is +not something a chart can defend itself against. + +Because the name no longer carries the issuer, NOTES.txt prints the mapping - +index, issuer and hosts per object - the way keycloak's does for its realms. +Without it, nothing in the cluster says which object holds which issuer. */ -}} {{- $httpRoute := .Values.httpRoute | default dict }} {{- if and .Values.ingress.enabled (not $httpRoute.enabled) }} @@ -121,13 +171,13 @@ values.yaml, line 1 onwards, and the mtaSts block). {{- end }} {{- $service := printf "%s-%s-http-service" (include "stalwart.fullname" .) $group }} {{- $path := include "stalwart.mtaSts.path" . }} -{{- range $issuer := $mtaSts.order }} +{{- range $i, $issuer := $mtaSts.order }} {{- $entry := index $mtaSts.groups $issuer }} --- apiVersion: networking.k8s.io/v1 kind: Ingress metadata: - name: {{ include "stalwart.mtaSts.ingressName" (dict "root" $ "issuer" $issuer) }} + name: "{{ include "stalwart.fullname" $ }}-mta-sts-ingress{{ if $i }}-{{ $i }}{{ end }}" labels: app: {{ include "stalwart.fullname" $ }} {{- if $issuer }} diff --git a/stalwart/values.yaml b/stalwart/values.yaml index c208c83..347d104 100644 --- a/stalwart/values.yaml +++ b/stalwart/values.yaml @@ -775,6 +775,25 @@ httpRoute: # argument, with the cert-manager behaviour it was measured # against, is in stalwart.mtaSts.ingressGroups in # _helpers.tpl. +# Named BY INDEX over those groups — +# -stalwart-mta-sts-ingress, then -1, -2 — in this +# repo's idiom (README/style guide section 4, as +# homeassistant's routes and keycloak's realm Ingresses do +# it). So the NAME DOES NOT SAY WHICH ISSUER it carries: +# NOTES.txt prints that mapping, and it is the only place the +# association is visible. +# REORDERING THIS LIST IS NOT FREE. The index runs over the +# issuer groups, which are derived from the list, so moving +# one entry can change how many objects exist and which index +# holds which issuer — renumbering objects that were not +# themselves moved. Kubernetes recreates them, which takes +# their Certificate objects along; nothing is RE-ISSUED, +# because the Secrets survive (cert-manager only +# owner-references a Secret with +# --enable-certificate-owner-ref, which this cluster does not +# set). That last part is a property of the CLUSTER and not +# of this chart: with that flag on, a reorder costs a fresh +# ACME request per renumbered object. # Gateway mode — ONE HTTPRoute with all the hostnames. Nothing differs per # host here (same path, same backend, no filter) and # spec.hostnames is a list, so one object says exactly what