fix(review): #138 review findings, round 1

changelog.d/tenant-extra-hostnames.md

@@ -10,20 +10,27 @@ Een tenant kan nu op meer dan één hostnaam antwoorden. Nieuw optioneel veld
   (`ingress.hosts` uit de ApplicationSet leest die chart niet);
 - geeft elke extra host een eigen TLS-blok en dus een eigen Certificate via
   HTTP-01, zodat één host met nog niet kloppende DNS de andere niet tegenhoudt;
-- neemt annotaties, `ingressClassName` en poort via de `tpl` van de chart over
-  uit dezelfde values als de hoofd-Ingress: geen tweede kopie van de
-  nginx-instellingen.
+- neemt `ingress.enabled`, annotaties, `ingressClassName`, `path`, `pathType`
+  en poort via de `tpl` van de chart over uit dezelfde values als de
+  hoofd-Ingress: geen tweede kopie van de nginx-instellingen.
 
 **Bestaande tenants renderen byte-identiek.** Het blok wordt alleen geëmit als
 de lijst gezet is. Gemeten: alle 89 tenantbestanden op main en op deze branch
 gerenderd (Argo-goTemplate met sprig en `missingkey=default`, daarna
 `helm template` met chart 8.9.0 en dezelfde valueFiles-volgorde), values en
 manifests per tenant byte voor byte gelijk.
 
-`validate-values.sh` controleert het nieuwe veld (lijst, geldige DNS-namen,
-geen dubbele, niet de primaire host, secretnaam ≤ 253), met fixtures in
+`validate-values.sh` controleert het nieuwe veld (lijst, geldige DNS-namen en
+geen IP-adres, geen dubbele, niet de primaire host, onder `commonground.nu`
+alleen de eigen namen, unieke secretnaam ≤ 253, niet samen met
+`ingress.enabled: false`). Ook `tenant.hostname` mag geen IP-adres meer zijn.
+Over alle tenantbestanden heen weigert hij een host die twee tenants claimen
+(primaire host, `extraHostnames`, `frontend.host`, `frontend.extraHosts`) en
+een `tenant.name` die in twee bestanden staat, met fixtures in
 `nextcloud-platform/tests/cases/tenant-extra-hostnames-*`.
 
 Bekende beperkingen: `trusted_domains` wordt alleen bij de eerste installatie
-geschreven; `charts/tenant-httproute` neemt nog één host. Zie
+geschreven, en een verwijderde host blijft erin staan tot een
+`occ config:system:delete trusted_domains <n>`; `charts/tenant-httproute` neemt
+nog één host. Zie
 `docs/ADDING-TENANT.md`.

docs/ADDING-TENANT.md

@@ -202,7 +202,7 @@ What the ApplicationSet does with it (`argo/applicationsets/nextcloud-tenants.ya
 | `trusted_domains` | every extra host is added to `nextcloud.trustedDomains`, after the primary host |
 | routing | a second Ingress `nextcloud-extra-hosts`, because chart 8.9.0 puts only `nextcloud.host` in its own Ingress rules |
 | certificates | one TLS entry per host, secret `nextcloud-<tenant>-<host with dashes>-tls`, so one cert-manager Certificate per host via HTTP-01 (`letsencrypt-prod`). A host whose DNS is not ready does not block the others |
-| annotations | taken from `ingress.annotations`, `ingress.className` and `service.port` through the chart's `tpl`, so they always match the main Ingress |
+| annotations | taken from `ingress.annotations`, `ingress.className`, `ingress.path`, `ingress.pathType` and `service.port` through the chart's `tpl`, so they always match the main Ingress; with `ingress.enabled: false` neither Ingress is rendered |
 
 A tenant without `extraHostnames` renders byte-identical to before; the block is
 only emitted when the list is set.
@@ -220,10 +220,17 @@ Four things to know:
   reads `NEXTCLOUD_TRUSTED_DOMAINS` when it installs; adding a host to a running
   tenant later also needs `occ config:system:set trusted_domains <n> --value=<host>`
   in the pod (same pattern as `cutover-tenant.sh`). Put the full list in the file
-  before the tenant is first created.
+  before the tenant is first created. Removing a host from the list works the
+  same way: Argo prunes the Ingress, but the host stays in `config.php` until
+  you run `occ config:system:delete trusted_domains <n>`. In both cases run
+  `occ config:system:get trusted_domains` first to find the right `<n>`: `set`
+  on an index that is already in use overwrites that entry.
 - **Gateway API routes do not use this list.** `charts/tenant-httproute` takes one
   host; a tenant with `gateway.nextcloud: true` gets a route for the primary host
-  only.
+  only. The extra hosts stay on the nginx Ingress, and that Ingress follows
+  `ingress.enabled` like the chart's own: `ingress.enabled: false` would leave
+  them without any route, so `validate-values.sh` refuses `extraHostnames`
+  together with `ingress.enabled: false`.
 
 Validate before pushing:
 

docs/CHECKS-AND-BALANCES.md

@@ -36,8 +36,13 @@ Before merge, CI runs `scripts/verify.sh`, which wraps:
 - `nextcloud-platform/scripts/smoke-checks.sh` — Helm chart renders
 
 `validate-values.sh` also checks the optional `tenant.extraHostnames` list (a
-list, valid DNS names, no duplicates, not the primary host, a derived TLS secret
-name Kubernetes accepts). A valid list still gives a warning, because DNS and
+list, valid DNS names and no IP address, no duplicates, not the primary host,
+under `commonground.nu` only this tenant's own names, a unique derived TLS
+secret name Kubernetes accepts, and not together with `ingress.enabled: false`).
+`tenant.hostname` is refused as an IP address too. It also checks across all
+tenant files that no host (primary, `extraHostnames`, `frontend.host`,
+`frontend.extraHosts`) is claimed by two tenants, also when it is run on a
+single file, and that no two tenant files share a `tenant.name`. A valid list still gives a warning, because DNS and
 HTTP-01 for those hosts are outside what CI can see. Fixtures for every branch
 are in `nextcloud-platform/tests/cases/tenant-extra-hostnames-*`.
 

docs/GATEWAY-API.md

@@ -50,7 +50,10 @@ blok dat altijd meegaat zou alle 84 Applications tegelijk laten hersyncen.
 `tenant.extraHostnames` (zie `docs/ADDING-TENANT.md`) gaat hier niet mee: de
 route krijgt alleen de primaire host. Een tenant met extra hostnamen blijft voor
 die hosts op de nginx-Ingress tot deze chart een lijst hostnames aanneemt, met
-per host een listener op de Gateway.
+per host een listener op de Gateway. Die Ingress (`nextcloud-extra-hosts`) volgt
+`ingress.enabled`, net als de hoofd-Ingress van de chart. `validate-values.sh`
+weigert daarom `extraHostnames` samen met `ingress.enabled: false`: de extra
+hosts zouden dan geen enkele route meer hebben.
 
 ## Waarom dit nog niet schaalt
 
@@ -94,8 +97,10 @@ De HTTPRoute komt náást de Ingress. Het verkeer verschuift **niet**: het
 DNS-record blijft naar ingress-nginx wijzen, want external-dns laat een bestaand
 record met rust zolang de Ingress bestaat. Gemeten 2026-08-17.
 
-De cutover is dus het weghalen van de Ingress. Doe dat niet lichtvaardig: het
-certificaat van deze host wordt via HTTP-01 over díé Ingress vernieuwd. Verdwijnt
+De cutover is dus het weghalen van de Ingress. Heeft de tenant
+`tenant.extraHostnames`, dan kan dat nog niet: `ingress.enabled: false` haalt ook
+de Ingress van de extra hosts weg, en de validator weigert die combinatie. Doe
+het ook verder niet lichtvaardig: het certificaat van deze host wordt via HTTP-01 over díé Ingress vernieuwd. Verdwijnt
 hij voordat cert-manager's `--enable-gateway-api` aanstaat (die vlag staat nu
 uit), dan breekt de vernieuwing stil en pas bij de eerstvolgende renewal.
 

nextcloud-platform/argo/applicationsets/nextcloud-tenants.yaml

@@ -193,13 +193,15 @@ spec:
               # De chart (8.9.0) zet in zijn eigen Ingress alleen nextcloud.host
               # als rule, dus extra hosts krijgen hier een tweede Ingress. Eén
               # TLS-blok per host, dus één Certificate per host: een host waarvan
-              # DNS nog niet klopt houdt de andere niet tegen. Annotaties,
-              # ingressClassName en poort komen via de tpl van de chart uit
-              # dezelfde values als de hoofd-Ingress, dus geen tweede kopie van
-              # de nginx-instellingen. Alleen geëmit als de lijst gezet is:
-              # bestaande tenants renderen byte-identiek.
+              # DNS nog niet klopt houdt de andere niet tegen. ingress.enabled,
+              # annotaties, ingressClassName, path, pathType en poort komen via
+              # de tpl van de chart uit dezelfde values als de hoofd-Ingress,
+              # op dezelfde manier als templates/ingress.yaml van de chart, dus
+              # geen tweede kopie van de nginx-instellingen. Alleen geëmit als
+              # de lijst gezet is: bestaande tenants renderen byte-identiek.
               extraManifests:
                 - |
+                  {{`{{- if .Values.ingress.enabled }}`}}
                   apiVersion: networking.k8s.io/v1
                   kind: Ingress
                   metadata:
@@ -208,10 +210,14 @@ spec:
                       app.kubernetes.io/name: nextcloud
                       app.kubernetes.io/part-of: nextcloud-platform
                       nextcloud.platform/tenant: "{{ $name }}"
+                    {{`{{- with .Values.ingress.annotations }}`}}
                     annotations:
-                      {{`{{- toYaml .Values.ingress.annotations | nindent 4 }}`}}
+                      {{`{{- toYaml . | nindent 4 }}`}}
+                    {{`{{- end }}`}}
                   spec:
-                    ingressClassName: {{`{{ .Values.ingress.className }}`}}
+                    {{`{{- with .Values.ingress.className }}`}}
+                    ingressClassName: {{`{{ . }}`}}
+                    {{`{{- end }}`}}
                     tls:
                       {{- range $extraHosts }}
                       - secretName: "nextcloud-{{ $name }}-{{ replace "." "-" . }}-tls"
@@ -223,14 +229,15 @@ spec:
                       - host: "{{ . }}"
                         http:
                           paths:
-                            - path: /
-                              pathType: Prefix
+                            - path: {{`{{ .Values.ingress.path }}`}}
+                              pathType: {{`{{ .Values.ingress.pathType }}`}}
                               backend:
                                 service:
                                   name: nextcloud
                                   port:
                                     number: {{`{{ .Values.service.port }}`}}
                       {{- end }}
+                  {{`{{- end }}`}}
               {{- end }}
 
         # Our GitOps repo for values files