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) }