From fa0930adeb9954bebd6679d004e9082eb585da97 Mon Sep 17 00:00:00 2001 From: Evan Vetere Date: Wed, 24 Jun 2026 14:07:49 -0400 Subject: [PATCH] fix: clear error for hostname-less tenant listeners Tenant Gateways whose listeners use the managed ports (80/443) without a hostname were rejected at admission with the opaque upstream CEL message "Combination of port, protocol and hostname must be unique for each listener", even for a single listener. Root cause: the mutating defaulter injects default-http/default-https listeners with no hostname at creation. A tenant listener on the same port/protocol without a hostname produces a duplicate (port, protocol, hostname) tuple, which the upstream Gateway API CRD CEL rejects. CRD CEL validation runs after mutating webhooks but before validating webhooks, so the equivalent check in validateListeners is preempted and never surfaces at creation. Reject hostname-less tenant (non-default) listeners in the defaulting webhook, before injecting the defaults, so the operator returns an actionable message instead of leaking the CEL error. Closes #219 --- internal/validation/gateway_validation.go | 31 ++++++++++++ .../validation/gateway_validation_test.go | 49 +++++++++++++++++++ internal/webhook/v1/gateway_webhook.go | 9 ++++ 3 files changed, 89 insertions(+) diff --git a/internal/validation/gateway_validation.go b/internal/validation/gateway_validation.go index a59bcdb..dbefeda 100644 --- a/internal/validation/gateway_validation.go +++ b/internal/validation/gateway_validation.go @@ -41,6 +41,37 @@ func ValidateGateway(gateway *gatewayv1.Gateway, opts GatewayValidationOptions) return allErrs } +// ValidateTenantListenerHostnames enforces that every tenant-provided +// (non-default) listener carries a hostname. It is meant to run in the mutating +// defaulting webhook, before SetDefaultListeners injects the hostname-less +// default-http/default-https listeners. +// +// here be dragons +// +// Without this check, a tenant listener on a managed port (80/443) with no +// hostname collides with an injected default on the same port and protocol — +// both have an empty hostname — and the upstream Gateway API CRD CEL rejects the +// whole object with the opaque "Combination of port, protocol and hostname must +// be unique for each listener". CRD CEL validation runs after mutating webhooks +// but before validating webhooks, so the equivalent check in validateListeners +// is preempted and never surfaces for this case at creation time. +func ValidateTenantListenerHostnames(gateway *gatewayv1.Gateway) field.ErrorList { + allErrs := field.ErrorList{} + fldPath := field.NewPath("spec", "listeners") + + for i, l := range gateway.Spec.Listeners { + if gatewayutil.IsDefaultListener(l) { + continue + } + if l.Hostname == nil { + allErrs = append(allErrs, field.Required(fldPath.Index(i).Child("hostname"), + "a hostname must be set; this controller injects default HTTP/HTTPS listeners, so listeners you define must specify a distinguishing hostname")) + } + } + + return allErrs +} + func validateListeners(gateway *gatewayv1.Gateway, fldPath *field.Path, opts GatewayValidationOptions) field.ErrorList { allErrs := field.ErrorList{} diff --git a/internal/validation/gateway_validation_test.go b/internal/validation/gateway_validation_test.go index 5919ed9..743129b 100644 --- a/internal/validation/gateway_validation_test.go +++ b/internal/validation/gateway_validation_test.go @@ -433,3 +433,52 @@ func TestValidateListenersAllowsExistingHostnameInStatus(t *testing.T) { assert.Len(t, errs, 0, "expected validateListeners to permit a hostname on a default listener that matches an existing status address") } + +func TestValidateTenantListenerHostnames(t *testing.T) { + hostnamePath := field.NewPath("spec", "listeners").Index(0).Child("hostname") + hostnameRequired := field.Required(hostnamePath, + "a hostname must be set; this controller injects default HTTP/HTTPS listeners, so listeners you define must specify a distinguishing hostname") + + scenarios := map[string]struct { + listeners []gatewayv1.Listener + expectedErrors field.ErrorList + }{ + "bare gateway, no listeners": { + listeners: nil, + expectedErrors: field.ErrorList{}, + }, + "tenant listener without hostname is rejected": { + listeners: []gatewayv1.Listener{ + {Name: "http", Protocol: gatewayv1.HTTPProtocolType, Port: 80}, + }, + expectedErrors: field.ErrorList{hostnameRequired}, + }, + "tenant listener with hostname is permitted": { + listeners: []gatewayv1.Listener{ + {Name: "http", Protocol: gatewayv1.HTTPProtocolType, Port: 80, Hostname: ptr.To(gatewayv1.Hostname("custom.example.com"))}, + }, + expectedErrors: field.ErrorList{}, + }, + "default-named listeners are exempt": { + listeners: []gatewayv1.Listener{ + {Name: gatewayutil.DefaultHTTPListenerName, Protocol: gatewayv1.HTTPProtocolType, Port: 80}, + {Name: gatewayutil.DefaultHTTPSListenerName, Protocol: gatewayv1.HTTPSProtocolType, Port: 443}, + }, + expectedErrors: field.ErrorList{}, + }, + } + + for name, scenario := range scenarios { + t.Run(name, func(t *testing.T) { + gateway := &gatewayv1.Gateway{ + Spec: gatewayv1.GatewaySpec{Listeners: scenario.listeners}, + } + + errs := ValidateTenantListenerHostnames(gateway) + + if diff := cmp.Diff(scenario.expectedErrors, errs, cmpopts.IgnoreFields(field.Error{}, "BadValue")); diff != "" { + t.Errorf("unexpected errors (-want +got):\n%s", diff) + } + }) + } +} diff --git a/internal/webhook/v1/gateway_webhook.go b/internal/webhook/v1/gateway_webhook.go index 0a34acc..79a2a8e 100644 --- a/internal/webhook/v1/gateway_webhook.go +++ b/internal/webhook/v1/gateway_webhook.go @@ -157,6 +157,15 @@ func (d *GatewayCustomDefaulter) Default(ctx context.Context, gateway *gatewayv1 // of creation. if gateway.CreationTimestamp.IsZero() { + // Reject hostname-less tenant listeners here, before injecting the + // default listeners. The injected defaults share ports 80/443 with no + // hostname, so a colliding tenant listener would otherwise be rejected + // by the upstream CRD CEL (which runs before the validating webhook) + // with an opaque "must be unique" message instead of this one. + if errs := validation.ValidateTenantListenerHostnames(gateway); len(errs) > 0 { + return apierrors.NewInvalid(gateway.GetObjectKind().GroupVersionKind().GroupKind(), gateway.GetName(), errs) + } + gatewayutil.SetDefaultListeners(gateway, d.config.Gateway) }