fix(gateway)!: require tenant routes to declare their hostnames - #3475

LGTM with non-blocking notes.

This is a real defense-in-depth fix. The old CEL short-circuited on !has(object.spec.hostnames) ||, so a route with no hostnames and a route with hostnames: [] (vacuous all() over an empty list) both passed without an apex check, and a hostless route inherits the listener hostnames while its parentRefs may point at a Gateway in another namespace. The new predicate has(...) && size(object.spec.hostnames) > 0 && all(...) closes both forms. Verified the break does not hit in-tree consumers (the sole Go producer renderHTTPRedirect now declares apex + *.apex, harbor's httproute is unconditional), already-admitted routes keep serving (VAP on CREATE/UPDATE only), and the controller upgrade skew is safe via the DeepEqual no-op. Validation sits in three layers (VAP, CRD MinLength on apex, renderer guard) with named error messages. Tests are non-vacuous: three independent mutations all go red.

[MINOR] hack/e2e-chainsaw/gateway/chainsaw-test.yaml — the TLSRoute deny probe covers only the absent case, not the empty-list case (both policies share one CEL byte and empty-list is proven end-to-end via the HTTPRoute pair, so this is coverage-only).

Note (not code): the do-not-merge/hold label is set.

Verdict

LGTM

The tightened predicate denies both hostname-less shapes and still admits every valid one, no in-tree route producer emits a hostname-less route into a tenant namespace, and the controller half is pinned per conjunct. Three MINOR items and one inaccurate particular in the PR body are left.

Findings

[MINOR] internal/controller/tenantgateway/reconciler.go:332, hostnames on the redirect route are forward-only state that an older controller build mis-reads

After this lands, <tgw>-http-redirect carries <apex> and *.<apex> in every tenant namespace running a TenantGateway. The exclusion keeping those two out of the claim set arrives with this PR at line 332, so any controller build without it reads them as an app publishing hostnames. perListenerName("*.<apex>") (renderers.go:210, via hostnameFirstLabel at renderers.go:184) yields https-*-<8hex>, and the Gateway CRD's listener-name schema is ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ (packages/system/gateway-api-crds/templates/crds-experimental.yaml:2668-2676), which rejects *. runReconcileSteps calls reconcileGateway at line 161 and reconcileHTTPToHTTPSRedirect only at line 177, so the Gateway write is refused first and the reconcile returns before the route could be rewritten back. Every subsequent pass repeats it.

Concretely: an operator rolls the platform back a version after some unrelated problem, the older controller comes up, and every HTTP-01 TenantGateway sits at Ready=False carrying the apiserver's listener-name error until somebody hand-edits or deletes the redirect route.

MINOR rather than higher because a platform downgrade isn't a path cozystack supports today. run-migrations.sh:26 returns early when CURRENT_VERSION -ge TARGET_VERSION, so migration state never unwinds, and nothing under docs/ describes a downgrade procedure. The failure is legible too: markFailed (reconciler.go:186-206) puts the apiserver error on the TenantGateway. One line in the release note, naming the redirect route as state to clear on rollback, closes it.

What would change my mind: evidence that a platform version downgrade is a supported customer operation. That makes this an upgrade-path regression rather than a note.

[MINOR] internal/controller/tenantgateway/reconciler.go:540, the namespace term of isHTTPRedirectRoute has no isolating test

The predicate is a three-term conjunction, and the doc comment above it says name and ownership are "each pinned by its own test". That part holds. TestReconcile_OtherControllerOwnedRouteStillClaimsHostnames isolates the name term, TestReconcile_ForgedRedirectNameStillClaimsHostnames isolates ownership, and deleting either one reddens the package. Deleting route.Namespace == tgw.Namespace doesn't:

$ review-helper mutate ... --mutations M8 --test 'go test ./internal/controller/tenantgateway/'
M8 per-conjunct: drop the NAMESPACE term from isHTTPRedirectRoute GAP: the suite stayed green without the fix
    | ok  	github.com/cozystack/cozystack/internal/controller/tenantgateway	2.530s

The fixture that's missing is a route in a second allowed namespace, either an attachedNamespaces entry or a child tenant carrying namespace.cozystack.io/gateway, sharing the redirect's name and carrying a controller ownerRef with this TenantGateway's UID. Drop the namespace term and that route leaves the claim set. Leaving the claim set is also how a route escapes HostnameConflict marking, which is the reason the comment itself gives for keeping the predicate narrow.

[MINOR] packages/extra/gateway/README.md:87, the allowedRoutes.kinds clause contradicts the code in a paragraph rewritten to match it

Layer 1 still ends "HTTPS listeners additionally restrict allowedRoutes.kinds to HTTPRoute (and TLS-passthrough listeners to TLSRoute)". Both listener classes get the same two-kind set. port443Kinds is HTTPRoute plus TLSRoute (internal/controller/tenantgateway/reconciler.go:801-804), assigned to the HTTPS listeners at line 806 and to each passthrough listener at line 916, deliberately, so Cilium doesn't merge port-443 listeners (cilium#45559). The consequential half of the sentence, that GRPCRoute / TCPRoute / UDPRoute can't attach there, is right. The parenthetical isn't. This PR rewrote the surrounding sentences of that same paragraph under a commit titled "align gateway prose with the code", so the stale clause may as well travel with them.

Claim mismatches

[PARTIAL] "the vendored openbao, harbor, opencost and flux-operator route templates can render one, but each is behind a flag that defaults off with no values path to turn it on"

The conclusion holds and I re-derived it independently: every route template under packages/ carries a hostnames key, and renderHTTPRedirect is the only Go producer. Two particulars in the list don't.

flux-operator isn't a package in this tree. packages/ holds core/flux-aio, system/flux-plunger and system/flux-shard-operator, none of which renders a route.

harbor can't render a hostname-less route here at all, so it isn't an example of the shape being described. packages/apps/harbor/templates/harbor.yaml:163 pins expose.type: clusterIP, and the vendored template is gated on eq .Values.expose.type "route" (packages/system/harbor/charts/harbor/templates/gateway-apis/route.yaml:1).

Both errors point away from risk rather than towards it, so no verdict moves. But the PR body cites this audit as the basis for its blast-radius claim, and a reader who goes checking won't find what it names.

Caveats

What this pass actually covered, and where it stopped.

I compiled and evaluated both revisions of the rendered validator string, taken from helm template output at merge base and at head, under cel-go v0.26.0 with ext.Strings(), the version pinned in go.mod:

== BASE ==                                          == HEAD ==
  hostnames ABSENT                     => true        => false
  hostnames EMPTY list                 => true        => false
  hostnames = apex                     => true        => true
  hostnames = apex + *.apex (redirect) => true        => true
  hostnames = a.b.foo.example.com      => true        => true
  hostnames = evil.example.net         => false       => false
  hostnames = xfoo.example.com         => false       => false
  ns label uppercase                   => true        => true
  ns label missing (fail-closed)       => false       => false
  namespaceObject null (fail-closed)   => false       => false

Both compile clean, the two empty shapes flip to denied, and nothing previously admitted stops being admitted. The apex comparison and the fail-closed halves are untouched, and xfoo.example.com is still not treated as a subdomain of foo.example.com.

On the existing-customer upgrade: the rendered delta between merge base and head for packages/system/cozystack-basics is exactly the validator string plus the two messageExpression strings. apiVersions: ["v1", "v1beta1"] on the HTTPRoute rule was already there at merge base; this PR documents and asserts it. A VAP evaluates at CREATE and UPDATE, so a live hostname-less route keeps serving until something writes to it, and the one writer that could is a no-op: an older controller renders the same hostname-less spec and returns at the equality.Semantic.DeepEqual check (reconciler.go:249) without issuing an Update. Policy landing before the controller wedges nothing. The reverse order is fine too, since the new controller's Update carries hostnames the old policy accepts.

controller-gen v0.16.4 against ./api/gateway/... reproduces packages/system/cozystack-controller/definitions/gateway.cozystack.io_tenantgateways.yaml byte for byte, so minLength: 1 matches its marker and the Codegen Drift Check won't fire. spec.apex is written only from _namespace.host (packages/extra/gateway/templates/tenantgateway.yaml:48), the same value that becomes the namespace.cozystack.io/host label (packages/apps/tenant/templates/namespace.yaml:91, and packages/system/cozystack-basics/templates/cozystack-values-secret.yaml:16 for tenant-root). That equality is what lets the redirect route's own hostnames pass the tightened policy, and it means an empty apex is already unreachable through the chart, which fails the render at tenantgateway.yaml:3.

The template header's account of the CRD bundle checks out. packages/system/gateway-api-crds/templates/crds-experimental.yaml serves httproutes at v1 (storage) and v1beta1, neither requiring hostnames nor setting minItems; tlsroutes at v1 (storage, required plus minItems: 1), v1alpha2 (deprecated, neither) and v1alpha3 (deprecated, required plus minItems: 1). v1alpha2 really is the only TLSRoute version where the shape reaches admission.

Ten mutations, nine covered, the one gap reported above. helm unittest on cozystack-basics is 59 green at baseline and reddens on the CEL revert, on dropping the size() conjunct alone, and on reverting the message. go test ./internal/controller/tenantgateway/ reddens on dropping the redirect hostnames, on dropping the exclusion, on dropping the name term, on dropping the ownership term, and on dropping the empty-apex guard. hack/select-e2e.sh escalates this diff to the full suite, since internal/ matches full_suite_pattern, so the gateway chainsaw suite is in the selected set and the new hostname-less-route-denied step will run. Its deny() helper fails loudly when admission accepts, so it isn't vacuous.

Reasoned rather than run: the redirect route claims only the port-80 listener (SectionName: "http" at renderers.go:530-536), so widening its hostnames can't reach a 443 listener. I didn't exercise Gateway API route merging on a cluster to confirm the ACME solver route still wins precedence for its own exact hostname, though the CRD's own precedence rules put a matching non-wildcard hostname ahead of a wildcard one.

One thing I couldn't settle. cert-manager's Gateway HTTP-01 solver looks inactive on the management cluster: packages/system/cert-manager/values.yaml sets no config.enableGatewayAPI, and the only place that flag appears is the tenant-side chart (packages/apps/kubernetes/templates/helmreleases/cert-manager.yaml:7). If that reading is right, the solver route the PR body reasons about isn't currently created at all, which makes the reasoning moot rather than wrong. Pre-existing either way, and outside this diff.

Recommended follow-ups

The port-80 listener sets no allowedRoutes.kinds (buildHTTPListenerAllowedRoutes, internal/controller/tenantgateway/renderers.go:149-156), so the Gateway API default for an HTTP listener applies and a GRPCRoute from the tenant namespace can attach there. This VAP covers httproutes and tlsroutes only, so such a route is ungated. No GRPCRoute producer exists in the tree and tenants hold no Gateway API RBAC, so it's a gap in the model rather than a live exposure, but it belongs with the same layer.

The TLSRoute rule still names v1alpha2 alone (packages/system/cozystack-basics/templates/route-hostname-policy.yaml:126). The deferral is documented in the header and tracked in #3789, and it's safe while matchPolicy keeps its Equivalent default and v1alpha2 stays served. Worth pairing the version widening with whichever bundle bump drops v1alpha2, because the failure mode there is silence, not an error.

A pre-upgrade check in the release note would let an operator find the affected population before the policy lands: listing HTTPRoutes and TLSRoutes in tenant-* namespaces whose spec.hostnames is absent or empty. Nothing in the tree produces one, so the residual set is hand-written routes, and that's exactly the set an operator can't enumerate from a changelog.

Verdict

LGTM with non-blocking notes

Reviewed at e14554f4f against merge-base 3a70f9142. The two halves do belong together, and the in-tree route inventory checks out. Coverage is unusually well pinned. Four MINOR notes below, none of them blocking.

Findings

[MINOR] api/gateway/v1alpha1/tenantgateway_types.go:152, MinLength=1 does not deliver what the PR body asks of it

The bound is there so a bad apex is "refused at admission naming the field than at reconcile time in controller logs". For the empty string it does that, and only for the empty string. spec.apex now feeds HTTPRoute.spec.hostnames, whose vendored CRD pattern is lowercase-DNS-only, and minLength: 1 admits every non-empty value that is not a hostname: mixed case, a trailing dot, an underscore. Each of those renders a redirect route the apiserver rejects, which is the reconcile-time failure the bound exists to remove. Mixed case is not hypothetical here: nothing in the chain lowercases it, and this repo's own suite provisions a mixed-case label on purpose (hack/e2e-chainsaw/gateway/chainsaw-test.yaml:648, "Kubernetes permits uppercase in a label value").

$ python3  # spec.hostnames.items.pattern out of packages/system/gateway-api-crds/templates/crds-experimental.yaml
### httproutes.gateway.networking.k8s.io
  version v1 served True storage True
    items pattern ^(\*\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$

$ helm template cozystack packages/extra/gateway --namespace tenant-foo \
    --set-json '_namespace={"host":"Foo.Example.COM"}' ...
  apex: "Foo.Example.COM"          # chart passes tenant.spec.host through verbatim

$ go test ./internal/controller/tenantgateway/ -run TestProbe_UppercaseApex -v
  OBSERVED case=uppercase-label apex="Foo.Example.COM" -> hostnames=[Foo.Example.COM *.Foo.Example.COM]
           rejected-by-CRD-pattern=[Foo.Example.COM *.Foo.Example.COM]
  OBSERVED case=trailing-dot     apex="foo.example.com." -> rejected-by-CRD-pattern=[foo.example.com. *.foo.example.com.]
  CONTROL:  apex="foo.example.com" -> hostnames=[foo.example.com *.foo.example.com], all match

Deliberately not filed higher, and this is what caps it. tlsPassthroughServices defaults to three entries and the gateway HelmRelease takes valuesFrom the cozystack-values Secret only, with no spec.values, so there is no supported path to empty it. That means renderGateway already builds api.<apex> listeners, so a mixed-case apex is already rejected at merge base, before this branch adds a second rejected object on the same broken path:

$ go test .../tenantgateway -run TestProbe_ApexCaseAcrossDefaults -v   # merge-base 3a70f9142
  case=chart-default-tlsPassthroughServices unadmittable=[gateway.listener[tls-api]=api.Foo.Example.COM ...]
  case=tlsPassthroughServices-empty         unadmittable=[]
$ same probe at head e14554f4f
  case=chart-default-tlsPassthroughServices unadmittable=[... redirect.hostname=Foo.Example.COM *.Foo.Example.COM]
  case=tlsPassthroughServices-empty         unadmittable=[redirect.hostname=Foo.Example.COM *.Foo.Example.COM]

Not a regression, then. It is an under-specified bound on a field the branch just made load-bearing for admission. Since you are tightening the field anyway, a +kubebuilder:validation:Pattern carrying the Gateway API hostname regex costs one line and closes the whole class, or normalise at render with strings.ToLower the way the CEL already normalises the other operand. Test: extend TestRenderHTTPRedirect_RejectsEmptyApex into a table with Foo.Example.COM and foo.example.com. and assert the named error.

[MINOR] packages/system/cozystack-basics/templates/route-hostname-policy.yaml:102, the denial message is wrong on the label-absent branch

One messageExpression serves the only validation, and the CEL's first branch returns false whenever namespaceObject is null or the host label is missing. A route that already declares in-apex hostnames, in a tenant-* namespace whose label has not landed yet, now reads "HTTPRoute must declare spec.hostnames" and is told to do the thing it did. The old wording was imprecise on that branch too. The new clause is actively misleading rather than merely incomplete, and the template's own comment names that in-flight namespace as the one legitimate state reaching it. Either name the case in the message, or split the label check into its own validation with its own message so the two denials stop sharing wording.

[MINOR] packages/system/cozystack-basics/templates/route-hostname-policy.yaml:77, breaking change ships without a way to find what it breaks

The policy matches CREATE and UPDATE only, so an existing hostname-less route in a tenant-* namespace keeps serving and then fails at whatever writes it next: a Helm re-apply of a tenant-installed third-party chart, a kubectl label, a controller adding a finalizer. That write can land days after the upgrade and far from it. The release note states the behaviour change but ships no discovery step and no migration, so an operator has no way to answer "which of my routes will stop being updatable" before it happens. The in-tree inventory is clean (see Checked and correct), so the exposure is entirely tenant-authored and third-party routes, which is exactly the population an operator cannot enumerate from the diff. A one-liner in the release note would cover it, along the lines of kubectl get httproute,tlsroute -A -o json | jq -r '.items[] | select(.metadata.namespace|startswith("tenant-")) | select((.spec.hostnames // []) | length == 0) | "\(.metadata.namespace)/\(.metadata.name)"'. Recovery is cheap once found: an UPDATE that adds hostnames satisfies the policy, so no delete-and-recreate is needed.

[MINOR] packages/system/cozystack-basics/templates/route-hostname-policy.yaml:77, the policy admits a hostname shape the controller cannot provision

The validator accepts any hostname that equals the apex or ends with "." + apex, and *.<apex> satisfies the second clause. Nothing downstream filters it out: collectHostnameClaims appends every entry of route.Spec.Hostnames verbatim, runReconcileSteps copies every winner key into dynHostnames, and the HTTP-01 branch of renderGateway calls perListenerName(h) on each. hostnameFirstLabel("*.foo.example.com") returns *, so the listener name is https-*-<8hex>, which fails the SectionName pattern ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ the vendored bundle enforces. Because the Gateway is written as one object, a single such route makes every listener update fail, so no other tenant hostname can be provisioned until the route is removed.

This is pre-existing rather than introduced here: perListenerName is byte-identical at the merge base and carries no wildcard filter there either. It belongs in this PR because this is the change that enumerates which hostname shapes a tenant route may declare, and the enumeration admits one the renderer cannot serve. Either reject *. prefixes in the CEL alongside the new size() > 0 term, or skip them in collectHostnameClaims and surface the skip on the route's parent status.

Still open from my earlier rounds

All four items I raised on 2026-08-21 are still open. Re-checked against the code as it stands, not against the old text.

• internal/controller/tenantgateway/reconciler.go:540, the namespace term of isHTTPRedirectRoute is unpinned. Mutating route.Namespace == tgw.Namespace to true still leaves go test ./internal/controller/tenantgateway/... green. The three forgery tests isolate the name term, the ownership term and the labels non-factor; none builds a route that shares name and ownership in a different namespace.
• internal/controller/tenantgateway/reconciler.go:332, the old-controller window is unmitigated. --leader-elect defaults to false and the chart does not set it, and the Deployment is replicas: 1 with no explicit strategy, so the default RollingUpdate can run the pre-guard and post-guard binaries together across this change. A binary without isHTTPRedirectRoute reads the redirect route's spec.hostnames as an ordinary claim.
• packages/extra/gateway/README.md:87, the allowedRoutes.kinds sentence still contradicts the code. reconciler.go:793-806 and :916 give HTTPS-terminate and TLS-passthrough listeners the same two-kind set, as a workaround for cilium#45559; the README describes them as carrying different kinds. The doc commit in this PR rewrote the surrounding paragraph and left this sentence byte-identical.
• hack/e2e-chainsaw/gateway/chainsaw-test.yaml:567, the TLSRoute deny probe still covers only the absent-hostnames case while HTTPRoute gets both absent and empty-list. The file's own comment at 512-519 justifies the asymmetry (one shared CEL byte, empty-list proven end to end through the HTTPRoute pair), so this is coverage bookkeeping rather than a hole.

Caveats

• CEL was not evaluated offline. No container runtime was reachable for a cel-go oracle, so the expression's runtime behaviour rests on the chainsaw suite rather than on anything I ran. That suite does run for this PR: hack/select-e2e.sh escalates to the full set because api/ and internal/ match full_suite_pattern, which I confirmed by running it on the changed-file list.
• CRD validation ratcheting was not exercised. bin/k8s/ is absent, so there are no committed envtest assets and no apiserver to replay a stored TenantGateway carrying an empty apex against the new minLength. Reasoned only: the chart cannot render one (the render errors on an empty _namespace.host), so such an object requires a hand-written CR, and on the 1.33+ floor ratcheting should carry an unchanged spec through a status write. Not executed.
• One cert-manager corner is not covered by the policy's new term. generateHTTPRouteSpec omits Hostnames when ch.Spec.DNSName parses as an IP, and that solver route lands in the tenant namespace, so it would now be denied. I could not reach it on a supported path: per-listener Certificates derive their DNSNames from route hostnames already constrained to the tenant apex, and nothing here wires ACME IP identifiers. Recording it rather than filing it.
• Live-admission surfaces stay out of scope for a static review: the actual apiserver rejection of the redirect route, real VAP evaluation against a served v1alpha2 TLSRoute, and SSA behaviour on the existing redirect route during the upgrade.

Checked and correct

• cert-manager HTTP-01 solver, the highest-stakes item in the change. The body claims the solver route declares its hostname, and I checked that against the pinned tag rather than taking it. packages/system/cert-manager/charts/cert-manager/Chart.yaml pins v1.20.2, and pkg/issuer/acme/http/httproute.go:144-156 at that tag sets hostnames = []gwapi.Hostname{ch.Spec.DNSName} for any non-IP DNS name, in ch.Namespace, which for these per-listener certs is the tenant namespace. HTTP-01 issuance keeps working.
• Rollout ordering: packages/core/platform/sources/cozystack-basics.yaml carries dependsOn: cozystack.cozystack-engine, and that source ships cozystack-controller, so the binary and the CRD land before the policy. The window where the tightened VAP meets the old hostname-less renderer does not exist on the Flux path.
• The redirect route is already owned on existing clusters. renderHTTPRedirect calls SetControllerReference at merge base too, and reconcileHTTPToHTTPSRedirect refuses to adopt an unowned route, so the ownership conjunct in isHTTPRedirectRoute matches on upgrade rather than silently failing open.
• The exclusion restores the prior claim set exactly. collectHostnameClaims only ever writes out[hostname], so a hostname-less route contributed nothing at base; the comment saying so is accurate.
• Route inventory. Every in-tree producer that can land in a tenant-* namespace sets exactly one hostname unconditionally: packages/apps/harbor, packages/system/bucket, packages/system/monitoring/{grafana,alerta}. The vendored openbao, opencost and harbor templates that can emit an empty list install into cozy-* namespaces, where the startsWith("tenant-") matchCondition never fires. packages/apps/openbao renders no route at all.
• TLSRoute version claims, extracted from the vendored bundle: v1 served and storage with required: [hostnames] and minItems: 1, v1alpha3 the same, v1alpha2 served, deprecated, with neither. The header's account is accurate and the new term is redundant everywhere except the one version the rule names.
• Coverage is non-vacuous per conjunct. Reverting the CEL to !has(object.spec.hostnames) || reddens 2 tests; deleting has(...) alone reddens 2; deleting size(...) > 0 alone reddens 2; deleting the .all() comparison reddens 4. On the Go side, dropping the collectHostnameClaims exclusion reddens TestReconcile_RedirectRouteIsNotAHostnameClaim, dropping the ownership conjunct reddens TestReconcile_ForgedRedirectNameStillClaimsHostnames, dropping the name conjunct reddens TestReconcile_OtherControllerOwnedRouteStillClaimsHostnames, reverting the hostnames reddens TestReconcile_RedirectRouteDeclaresApexHostnames, and removing the apex guard reddens two more. Every conjunct has a fixture where it is the sole discriminator.
• documentSelector is not vacuous: pointing it at a value no document carries fails with document not found rather than passing.
• Mechanical sweep run and clean on the diff: no || true / || : / 2>/dev/null on a decision command, no RBAC change, no new fixture fed to a shell parser. The chainsaw deny() helper is fail-closed under set -eu, since a failing grep -qi propagates.
• Both capability-gate corners render (4 documents with the VAP API served, 0 without), kubeconform -strict reports 4 of 4 valid, all three certMode corners render a non-empty apex, go build ./... and go vet clean, 59 of 59 helm unittests and the full Go package green.
• Conventions: release-note block present, ! breaking marker present, Conventional Commits clean, the single generated CRD copy matches the kubebuilder marker, and the README edit sits in ## Security model (line 77) below the generated ## Parameters block (line 67), so make generate will not clobber it.

LGTM, and lifting my hold.

I held this in July on "want to be sure this won't break anything", so here is what I checked rather than a verdict on its own.

The tightening is the right one. !has(object.spec.hostnames) || short-circuited on exactly the shape that needed checking: a route with no hostnames is not inert, it inherits every listener it attaches to, and parentRefs can name a Gateway in another namespace. Requiring the field present and non-empty is what makes the apex comparison reachable at all, since a VAP has no cross-object lookup to resolve what would otherwise be inherited.

Both halves do have to land together, and that answers the hold. renderHTTPRedirect built the redirect with no hostnames deliberately; it lives in a tenant-* namespace, so the stricter policy denies the controller's own route, and reconcileHTTPToHTTPSRedirect runs before the status update, so the TenantGateway never reaches Ready. Splitting the policy from the controller change would have broken every tenant gateway. Naming the apex and its wildcard is also correct rather than belt-and-braces: a wildcard is a suffix match over labels and does not cover the bare apex.

The blast radius on existing objects is what the description says it is. Both rules match CREATE and UPDATE in tenant-* namespaces, so a hand-written hostname-less route that exists today keeps serving but can no longer be updated. That is the correct trade for a bypass of the apex check, and it is stated plainly in the description rather than left to be discovered — which is what I wanted before releasing the hold. Worth carrying into the release notes in those words.

E2E is green on e14554f4f, which none of the other lanes I looked at today can say.

One thing I could not find addressed anywhere, non-blocking and narrow.

cert-manager's HTTP-01 solver route can be hostname-less. renderers.go selects HTTP-01 with a gatewayHTTPRoute solver against the tenant's own Gateway, so cert-manager creates the solver HTTPRoute in the tenant namespace — inside this policy's scope. In pkg/issuer/acme/http/httproute.go upstream sets the field conditionally:

if net.ParseIP(ch.Spec.DNSName) == nil {
    hostnames = []gwapi.Hostname{gwapi.Hostname(ch.Spec.DNSName)}
}

For a DNS name the solver names its hostname and passes. For an IP literal hostnames stays nil, the solver route is denied, and the challenge fails — where before this change it was admitted. TenantGateway.spec.apex and the namespace.cozystack.io/host label carry no format constraint that rules an IP out. I expect an IP apex is unusable end to end for other reasons and this is unreachable in practice; I mention it because the template header documents every other interaction it has, and this is the one it does not name.

Superseded by #3475 (review), which carries the same findings as inline comments on the lines they are about. The original text is removed rather than left as a second copy of the same review.

Verdict

LGTM with non-blocking notes

Reviewed at 4b2507b7 against merge-base b167fd11. The two halves are correctly coupled: a VAP cannot resolve the parent Gateway, so requiring the route to name its own hostnames is the only shape that keeps every admitted hostname comparable against the apex. One unguarded conjunct and three statements in the description that no longer match the branch.

Findings

• [MINOR] internal/controller/tenantgateway/reconciler.go:596, the namespace term of isHTTPRedirectRoute is the one conjunct nothing pins

Claim mismatches

• "Coverage narrows in one direction" understates it. The redirect now scores len(apex) on an exact-apex request instead of zero, so on the bare apex the contest runs down to path length and then creation order, and an app route with a PathPrefix: / rule created after the redirect now gets a 301 where it used to be served. That reads as intended, but the release note should say it.
• The branch vendors Gateway API 1.6.1, not 1.5.1: crds-experimental.yaml:7 carries bundle-version: v1.6.1 after the rebase. Every conclusion still holds there, checked against the bundle.
• The described correction to the template header is not in the diff. Either it landed on main separately or it went away in the force-push.

Caveats

• A partial rollback wedges the redirect: an N-1 controller renders it without hostnames, the Update is denied, and the TenantGateway stops reaching Ready. A full rollback is fine. Worth a release-note line.
• The e2e had not reported when I reviewed, so the new deny step is unproven end to end. hack/select-e2e.sh does emit gateway for this file list.