From 411dec03579e918ab6475060990e6b8dbf0dc043 Mon Sep 17 00:00:00 2001 From: Joshua Reese Date: Wed, 17 Sep 2025 18:51:48 -0500 Subject: [PATCH 1/2] Default Gateway hostname changes to avoid issues outlined in GEP-3567. Before these changes, downstream gateways were being provisioned with listeners which are considered to have overlapping hostnames. This is a result of configuring the `.` hostname on a listener, along with the `[v4|v6]..` hostnames on listeners. When this occurs, Envoy Gateway will disable HTTP2 on the listeners, requiring explicit ALPN configuration via a ClientTrafficPolicy. To make sure we provide users with gateways which support HTTP2 out of the box, the decision was made to no longer configure the gateway with the v4 or v6 prefixed hostnames. These addresses will remain in DNS, responding only with A or AAAA records. In an effort to make it more clear to users which hostnames a gateway will accept, the system now injects default HTTP and HTTPS listeners into gateways created by the user which have the `.` hostname. The user may remove these listeners if they wish. All three DNS addresses will remain in the `Status.Addresses` field of the Gateway, which now makes this field more aligned with the intent defined in the spec. The default listener names are now `default-http` and `default-https`, which will result in the reconfiguration of some gateway resources upon deployment - particularly those maintained via HTTPProxy resources. See: https://gateway-api.sigs.k8s.io/geps/gep-3567/ --- PROJECT | 1 + cmd/main.go | 10 +- config/dev/config.yaml | 5 +- config/dev/kustomization.yaml | 3 +- config/dev/webhook_patch.yaml | 12 -- config/webhook/manifests.yaml | 26 ++++ go.mod | 4 +- internal/config/config.go | 14 +- internal/config/zz_generated.deepcopy.go | 16 +- internal/config/zz_generated.defaults.go | 10 +- internal/controller/gateway_controller.go | 146 +++++++----------- .../controller/gateway_controller_test.go | 138 +++++++---------- internal/controller/httpproxy_controller.go | 48 +++--- .../controller/httpproxy_controller_test.go | 17 +- internal/util/gateway/listeners.go | 70 +++++++++ internal/validation/gateway_validation.go | 34 ++-- .../validation/gateway_validation_test.go | 28 +++- internal/webhook/v1/gateway_webhook.go | 74 ++++++++- test/e2e/gateway/chainsaw-test.yaml | 107 ++----------- 19 files changed, 400 insertions(+), 363 deletions(-) create mode 100644 internal/util/gateway/listeners.go diff --git a/PROJECT b/PROJECT index bbb48fe7..73f45bea 100644 --- a/PROJECT +++ b/PROJECT @@ -78,6 +78,7 @@ resources: version: v1 webhooks: validation: true + defaulting: true webhookVersion: v1 - domain: k8s.io external: true diff --git a/cmd/main.go b/cmd/main.go index d9f1eff2..88db2416 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -35,7 +35,6 @@ import ( networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" "go.datum.net/network-services-operator/internal/config" "go.datum.net/network-services-operator/internal/controller" - "go.datum.net/network-services-operator/internal/validation" networkingwebhook "go.datum.net/network-services-operator/internal/webhook" networkinggatewayv1webhooks "go.datum.net/network-services-operator/internal/webhook/v1" networkingv1alphawebhooks "go.datum.net/network-services-operator/internal/webhook/v1alpha" @@ -239,14 +238,7 @@ func main() { os.Exit(1) } - validationOpts := validation.GatewayValidationOptions{ - ControllerName: serverConfig.Gateway.ControllerName, - PermittedTLSOptions: serverConfig.Gateway.PermittedTLSOptions, - ValidPortNumbers: serverConfig.Gateway.ValidPortNumbers, - ValidProtocolTypes: serverConfig.Gateway.ValidProtocolTypes, - } - - if err := networkinggatewayv1webhooks.SetupGatewayWebhookWithManager(mgr, validationOpts); err != nil { + if err := networkinggatewayv1webhooks.SetupGatewayWebhookWithManager(mgr, serverConfig); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "Gateway") os.Exit(1) } diff --git a/config/dev/config.yaml b/config/dev/config.yaml index 2a9f868f..1e130cb4 100644 --- a/config/dev/config.yaml +++ b/config/dev/config.yaml @@ -15,11 +15,10 @@ gateway: downstreamGatewayClassName: datum-downstream-gateway-e2e permittedTLSOptions: gateway.networking.datumapis.com/certificate-issuer: [] - -httpProxy: - tlsOptions: + listenerTLSOptions: gateway.networking.datumapis.com/certificate-issuer: gateway-clusterissuer-selfsigned-ca + downstreamResourceManagement: # TODO(jreese) remove this when we make the downstream resource strategy # configurable diff --git a/config/dev/kustomization.yaml b/config/dev/kustomization.yaml index adfe85cb..e85f8f4f 100644 --- a/config/dev/kustomization.yaml +++ b/config/dev/kustomization.yaml @@ -2,8 +2,7 @@ namespace: kube-system namePrefix: network-services-operator- resources: - - ../crd - - ../crd/gateway + - ../upstream_resources - ../webhook - ../certmanager diff --git a/config/dev/webhook_patch.yaml b/config/dev/webhook_patch.yaml index 6cf10092..4abd41a8 100644 --- a/config/dev/webhook_patch.yaml +++ b/config/dev/webhook_patch.yaml @@ -27,18 +27,6 @@ patch: |- - op: remove path: /webhooks/0/clientConfig/service - - op: move - from: /webhooks/1/clientConfig/service/path - path: /webhooks/1/clientConfig/url - - op: remove - path: /webhooks/1/clientConfig/service - - - op: move - from: /webhooks/2/clientConfig/service/path - path: /webhooks/2/clientConfig/url - - op: remove - path: /webhooks/2/clientConfig/service - target: kind: MutatingWebhookConfiguration --- diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 01016cb4..6007c1f6 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -1,5 +1,31 @@ --- apiVersion: admissionregistration.k8s.io/v1 +kind: MutatingWebhookConfiguration +metadata: + name: mutating-webhook-configuration +webhooks: +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /mutate-gateway-networking-k8s-io-v1-gateway + failurePolicy: Fail + name: mgateway-v1.kb.io + rules: + - apiGroups: + - gateway.networking.k8s.io + apiVersions: + - v1 + operations: + - CREATE + - UPDATE + resources: + - gateways + sideEffects: None +--- +apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration metadata: name: validating-webhook-configuration diff --git a/go.mod b/go.mod index 4e2f7df7..1556ea73 100644 --- a/go.mod +++ b/go.mod @@ -5,10 +5,10 @@ go 1.24.0 toolchain go1.24.2 require ( - github.com/go-logr/logr v1.4.3 github.com/google/go-cmp v0.7.0 github.com/google/uuid v1.6.0 github.com/onsi/ginkgo/v2 v2.23.4 + github.com/onsi/gomega v1.37.0 github.com/stretchr/testify v1.10.0 go.miloapis.com/milo v0.1.0 golang.org/x/sync v0.15.0 @@ -36,6 +36,7 @@ require ( github.com/felixge/httpsnoop v1.0.4 // indirect github.com/fsnotify/fsnotify v1.9.0 // indirect github.com/fxamacker/cbor/v2 v2.8.0 // indirect + github.com/go-logr/logr v1.4.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/go-logr/zapr v1.3.0 // indirect github.com/go-openapi/jsonpointer v0.21.1 // indirect @@ -55,7 +56,6 @@ require ( github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect - github.com/onsi/gomega v1.37.0 // indirect github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/prometheus/client_golang v1.22.0 // indirect diff --git a/internal/config/config.go b/internal/config/config.go index c319f52d..c7a3d71a 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -339,6 +339,11 @@ type GatewayConfig struct { // needed for, or implement our own ACME integration. PerGatewayCertificateIssuer bool `json:"perGatewayCertificateIssuer,omitempty"` + // ListenerTLSOptions specifies the TLS options to program on generated + // TLS listeners. + // +default={"gateway.networking.datumapis.com/certificate-issuer": "auto"} + ListenerTLSOptions map[gatewayv1.AnnotationKey]gatewayv1.AnnotationValue `json:"listenerTLSOptions"` + // ValidPortNumbers is a list of port numbers that are permitted on gateway // listeners. // @@ -352,6 +357,10 @@ type GatewayConfig struct { ValidProtocolTypes map[int][]gatewayv1.ProtocolType `json:"validProtocolTypes,omitempty"` } +func (c *GatewayConfig) GatewayDNSAddress(gateway *gatewayv1.Gateway) string { + return fmt.Sprintf("%s.%s", gateway.UID, c.TargetDomain) +} + // +k8s:deepcopy-gen=true type HTTPProxyConfig struct { @@ -359,11 +368,6 @@ type HTTPProxyConfig struct { // underlying Gateway for an HTTPProxy. // +default="datum-external-global-proxy" GatewayClassName gatewayv1.ObjectName `json:"gatewayClassName"` - - // GatewayTLSOptions specifies the TLS options to program on the underlying - // Gateway for an HTTPProxy. - // +default={"gateway.networking.datumapis.com/certificate-issuer": "auto"} - GatewayTLSOptions map[gatewayv1.AnnotationKey]gatewayv1.AnnotationValue `json:"tlsOptions"` } // +k8s:deepcopy-gen=true diff --git a/internal/config/zz_generated.deepcopy.go b/internal/config/zz_generated.deepcopy.go index 8e6d4c06..c8f2050e 100644 --- a/internal/config/zz_generated.deepcopy.go +++ b/internal/config/zz_generated.deepcopy.go @@ -117,6 +117,13 @@ func (in *GatewayConfig) DeepCopyInto(out *GatewayConfig) { (*out)[key] = val } } + if in.ListenerTLSOptions != nil { + in, out := &in.ListenerTLSOptions, &out.ListenerTLSOptions + *out = make(map[apisv1.AnnotationKey]apisv1.AnnotationValue, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.ValidPortNumbers != nil { in, out := &in.ValidPortNumbers, &out.ValidPortNumbers *out = make([]int, len(*in)) @@ -153,13 +160,6 @@ func (in *GatewayConfig) DeepCopy() *GatewayConfig { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HTTPProxyConfig) DeepCopyInto(out *HTTPProxyConfig) { *out = *in - if in.GatewayTLSOptions != nil { - in, out := &in.GatewayTLSOptions, &out.GatewayTLSOptions - *out = make(map[apisv1.AnnotationKey]apisv1.AnnotationValue, len(*in)) - for key, val := range *in { - (*out)[key] = val - } - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HTTPProxyConfig. @@ -200,7 +200,7 @@ func (in *NetworkServicesOperator) DeepCopyInto(out *NetworkServicesOperator) { in.MetricsServer.DeepCopyInto(&out.MetricsServer) in.WebhookServer.DeepCopyInto(&out.WebhookServer) in.Gateway.DeepCopyInto(&out.Gateway) - in.HTTPProxy.DeepCopyInto(&out.HTTPProxy) + out.HTTPProxy = in.HTTPProxy out.Discovery = in.Discovery out.DownstreamResourceManagement = in.DownstreamResourceManagement in.DomainVerification.DeepCopyInto(&out.DomainVerification) diff --git a/internal/config/zz_generated.defaults.go b/internal/config/zz_generated.defaults.go index bf3ae243..9ac8830d 100644 --- a/internal/config/zz_generated.defaults.go +++ b/internal/config/zz_generated.defaults.go @@ -27,6 +27,11 @@ func SetObjectDefaults_NetworkServicesOperator(in *NetworkServicesOperator) { if in.Gateway.DownstreamHostnameAccountingNamespace == "" { in.Gateway.DownstreamHostnameAccountingNamespace = "datum-downstream-gateway-hostnames" } + if in.Gateway.ListenerTLSOptions == nil { + if err := json.Unmarshal([]byte(`{"gateway.networking.datumapis.com/certificate-issuer": "auto"}`), &in.Gateway.ListenerTLSOptions); err != nil { + panic(err) + } + } if in.Gateway.ValidPortNumbers == nil { if err := json.Unmarshal([]byte(`[80,443]`), &in.Gateway.ValidPortNumbers); err != nil { panic(err) @@ -40,11 +45,6 @@ func SetObjectDefaults_NetworkServicesOperator(in *NetworkServicesOperator) { if in.HTTPProxy.GatewayClassName == "" { in.HTTPProxy.GatewayClassName = "datum-external-global-proxy" } - if in.HTTPProxy.GatewayTLSOptions == nil { - if err := json.Unmarshal([]byte(`{"gateway.networking.datumapis.com/certificate-issuer": "auto"}`), &in.HTTPProxy.GatewayTLSOptions); err != nil { - panic(err) - } - } SetDefaults_DiscoveryConfig(&in.Discovery) if in.DomainVerification.RetryIntervals == nil { if err := json.Unmarshal([]byte(`[{"interval": "5s", "maxElapsed": "5m"}, {"interval": "1m", "maxElapsed": "15m"}, {"interval": "5m"}]`), &in.DomainVerification.RetryIntervals); err != nil { diff --git a/internal/controller/gateway_controller.go b/internal/controller/gateway_controller.go index 8b48fc35..c3649ebc 100644 --- a/internal/controller/gateway_controller.go +++ b/internal/controller/gateway_controller.go @@ -40,6 +40,7 @@ import ( networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" "go.datum.net/network-services-operator/internal/config" downstreamclient "go.datum.net/network-services-operator/internal/downstreamclient" + gatewayutil "go.datum.net/network-services-operator/internal/util/gateway" "go.datum.net/network-services-operator/internal/util/resourcename" ) @@ -126,10 +127,9 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req mcreconcile.Reque return ctrl.Result{}, nil } - if !controllerutil.ContainsFinalizer(&gateway, gatewayControllerFinalizer) { - controllerutil.AddFinalizer(&gateway, gatewayControllerFinalizer) + if r.prepareUpstreamGateway(&gateway) { if err := cl.GetClient().Update(ctx, &gateway); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to add finalizer to gateway: %w", err) + return ctrl.Result{}, fmt.Errorf("failed preparing upstream gateway: %w", err) } return ctrl.Result{}, nil @@ -146,6 +146,35 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req mcreconcile.Reque return result.Complete(ctx) } +// prepareUpstreamGateway adds a finalizer to the upstream gateway and ensures +// that default listeners have their hostname fields set. +func (r *GatewayReconciler) prepareUpstreamGateway(gateway *gatewayv1.Gateway) (needsUpdate bool) { + if !controllerutil.ContainsFinalizer(gateway, gatewayControllerFinalizer) { + controllerutil.AddFinalizer(gateway, gatewayControllerFinalizer) + needsUpdate = true + } + + // Ensure that the default listeners have hostnames set. This is not done in + // the defaulting webhook as the UID is not available, but we still want to + // let users create gateways without listeners and receive sane defaults. + + gatewayDefaultHostname := ptr.To(gatewayv1.Hostname(r.Config.Gateway.GatewayDNSAddress(gateway))) + + defaultHTTPListener := gatewayutil.GetListenerByName(gateway.Spec.Listeners, gatewayutil.DefaultHTTPListenerName) + if defaultHTTPListener != nil && defaultHTTPListener.Hostname == nil { + needsUpdate = true + defaultHTTPListener.Hostname = gatewayDefaultHostname + } + + defaultHTTPSListener := gatewayutil.GetListenerByName(gateway.Spec.Listeners, gatewayutil.DefaultHTTPSListenerName) + if defaultHTTPSListener != nil && defaultHTTPSListener.Hostname == nil { + needsUpdate = true + defaultHTTPSListener.Hostname = gatewayDefaultHostname + } + + return needsUpdate +} + func (r *GatewayReconciler) ensureDownstreamGateway( ctx context.Context, upstreamClusterName string, @@ -163,19 +192,21 @@ func (r *GatewayReconciler) ensureDownstreamGateway( } upstreamGatewayClassControllerName := string(upstreamGatewayClass.Spec.ControllerName) + gatewayDNSAddress := r.Config.Gateway.GatewayDNSAddress(upstreamGateway) + // targetDomainHostnames are default hostnames that are unique to each gateway, and // will have DNS records created for them. Any custom hostnames provided in // listeners WILL NOT be added to the addresses list in the gateway status. targetDomainHostnames := []string{ - fmt.Sprintf("%s.%s", upstreamGateway.UID, r.Config.Gateway.TargetDomain), + gatewayDNSAddress, } if r.Config.Gateway.IPv4Enabled() { - targetDomainHostnames = append(targetDomainHostnames, fmt.Sprintf("v4.%s.%s", upstreamGateway.UID, r.Config.Gateway.TargetDomain)) + targetDomainHostnames = append(targetDomainHostnames, fmt.Sprintf("v4.%s", gatewayDNSAddress)) } if r.Config.Gateway.IPv6Enabled() { - targetDomainHostnames = append(targetDomainHostnames, fmt.Sprintf("v6.%s.%s", upstreamGateway.UID, r.Config.Gateway.TargetDomain)) + targetDomainHostnames = append(targetDomainHostnames, fmt.Sprintf("v6.%s", gatewayDNSAddress)) } downstreamClient := downstreamStrategy.GetClient() @@ -208,7 +239,6 @@ func (r *GatewayReconciler) ensureDownstreamGateway( desiredDownstreamGateway := r.getDesiredDownstreamGateway( ctx, upstreamGateway, - targetDomainHostnames, claimedHostnames, ) @@ -287,32 +317,27 @@ func (r *GatewayReconciler) ensureDownstreamGateway( func (r *GatewayReconciler) getDesiredDownstreamGateway( ctx context.Context, upstreamGateway *gatewayv1.Gateway, - targetDomainHostnames []string, claimedHostnames []string, ) *gatewayv1.Gateway { logger := log.FromContext(ctx) var downstreamGateway gatewayv1.Gateway var listeners []gatewayv1.Listener - foundHTTPListener := false - foundHTTPSListener := false - for listenerIndex, l := range upstreamGateway.Spec.Listeners { - switch l.Protocol { - case gatewayv1.HTTPProtocolType: - foundHTTPListener = true - case gatewayv1.HTTPSProtocolType: - foundHTTPSListener = true - } + for listenerIndex, l := range upstreamGateway.Spec.Listeners { if l.TLS != nil && l.TLS.Options[certificateIssuerTLSOption] != "" { if r.Config.Gateway.PerGatewayCertificateIssuer { - metav1.SetMetaDataAnnotation(&downstreamGateway.ObjectMeta, "cert-manager.io/issuer", upstreamGateway.Name) + if !metav1.HasAnnotation(downstreamGateway.ObjectMeta, "cert-manager.io/issuer") { + metav1.SetMetaDataAnnotation(&downstreamGateway.ObjectMeta, "cert-manager.io/issuer", upstreamGateway.Name) + } } else { clusterIssuerName := string(l.TLS.Options[certificateIssuerTLSOption]) if r.Config.Gateway.ClusterIssuerMap[clusterIssuerName] != "" { clusterIssuerName = r.Config.Gateway.ClusterIssuerMap[clusterIssuerName] } - metav1.SetMetaDataAnnotation(&downstreamGateway.ObjectMeta, "cert-manager.io/cluster-issuer", clusterIssuerName) + if !metav1.HasAnnotation(downstreamGateway.ObjectMeta, "cert-manager.io/cluster-issuer") { + metav1.SetMetaDataAnnotation(&downstreamGateway.ObjectMeta, "cert-manager.io/cluster-issuer", clusterIssuerName) + } } } @@ -349,33 +374,6 @@ func (r *GatewayReconciler) getDesiredDownstreamGateway( // TODO(jreese) get from "scheduler" downstreamGateway.Spec.GatewayClassName = gatewayv1.ObjectName(r.Config.Gateway.DownstreamGatewayClassName) - for i, hostname := range targetDomainHostnames { - if foundHTTPListener { - listenerName := fmt.Sprintf("http-%d", i) - listeners = append(listeners, - listenerFactory( - listenerName, - hostname, - gatewayv1.HTTPProtocolType, - gatewayv1.PortNumber(DefaultHTTPPort), - "", - ), - ) - } - if foundHTTPSListener { - listenerName := fmt.Sprintf("https-%d", i) - listeners = append(listeners, - listenerFactory( - listenerName, - hostname, - gatewayv1.HTTPSProtocolType, - gatewayv1.PortNumber(DefaultHTTPSPort), - fmt.Sprintf("%s-%s", upstreamGateway.Name, listenerName), - ), - ) - } - } - downstreamGateway.Spec.Listeners = listeners return &downstreamGateway @@ -445,6 +443,12 @@ func (r *GatewayReconciler) ensureHostnamesClaimed( // but is considered sufficient for now. for _, hostname := range verifiedHostnames { + // No accounting for the gateway DNS address hostname + if hostname == r.Config.Gateway.GatewayDNSAddress(upstreamGateway) { + claimedHostnames = append(claimedHostnames, hostname) + continue + } + objectKey := client.ObjectKey{ Namespace: r.Config.Gateway.DownstreamHostnameAccountingNamespace, Name: hostname, @@ -515,6 +519,8 @@ func (r *GatewayReconciler) ensureHostnamesClaimed( } } + slices.Sort(claimedHostnames) + return verifiedHostnames, claimedHostnames, notClaimedHostnames, nil } @@ -531,6 +537,8 @@ func (r *GatewayReconciler) ensureHostnameVerification( ) ([]string, error) { logger := log.FromContext(ctx) + gatewayDefaultHostname := r.Config.Gateway.GatewayDNSAddress(upstreamGateway) + // Allow hostnames which have been successfully configured on the downstream // gateway stay on the gateway, regardless of whether or not there is a // matching Domain that's verified. This is done in case the user deletes the @@ -543,6 +551,7 @@ func (r *GatewayReconciler) ensureHostnameVerification( // For more thoughts on liens, the following issue provides good context: // https://github.com/kubernetes/kubernetes/issues/10179#issuecomment-2889238042 verifiedHostnames := sets.New[string]() + verifiedHostnames.Insert(gatewayDefaultHostname) if dt := downstreamGateway.DeletionTimestamp; dt.IsZero() { for _, listener := range downstreamGateway.Spec.Listeners { if listener.Hostname != nil && !strings.HasSuffix(string(*listener.Hostname), r.Config.Gateway.TargetDomain) { @@ -574,6 +583,10 @@ func (r *GatewayReconciler) ensureHostnameVerification( domainsToCreate := sets.New[string]() for _, hostname := range hostnames.UnsortedList() { + // Gateway DNS address hostname is exempt from verification + if hostname == gatewayDefaultHostname { + continue + } foundMatchingDomain := false for _, domain := range domainList.Items { if hostname == domain.Spec.DomainName || strings.HasSuffix(hostname, "."+domain.Spec.DomainName) { @@ -1579,44 +1592,3 @@ func (r *GatewayReconciler) listGatewaysForDomainFunc(clusterName string, cl clu return requests }) } - -func listenerFactory( - name, - hostname string, - protocol gatewayv1.ProtocolType, - port gatewayv1.PortNumber, - tlsSecretName string, -) gatewayv1.Listener { - h := gatewayv1.Hostname(hostname) - - fromSelector := gatewayv1.NamespacesFromSame - listener := gatewayv1.Listener{ - Protocol: protocol, - Port: port, - Name: gatewayv1.SectionName(name), - Hostname: &h, - AllowedRoutes: &gatewayv1.AllowedRoutes{ - Namespaces: &gatewayv1.RouteNamespaces{ - From: &fromSelector, - }, - }, - } - - if protocol == gatewayv1.HTTPSProtocolType { - tlsMode := gatewayv1.TLSModeTerminate - listener.TLS = &gatewayv1.GatewayTLSConfig{ - Mode: &tlsMode, - // TODO(jreese) investigate secret deletion when Cert (gateway) is deleted - // See: https://cert-manager.io/docs/usage/certificate/#cleaning-up-secrets-when-certificates-are-deleted - CertificateRefs: []gatewayv1.SecretObjectReference{ - { - Group: ptr.To(gatewayv1.Group("")), - Kind: ptr.To(gatewayv1.Kind("Secret")), - Name: gatewayv1.ObjectName(resourcename.GetValidDNS1123Name(tlsSecretName)), - }, - }, - } - } - - return listener -} diff --git a/internal/controller/gateway_controller_test.go b/internal/controller/gateway_controller_test.go index 00268ee4..3e4f017d 100644 --- a/internal/controller/gateway_controller_test.go +++ b/internal/controller/gateway_controller_test.go @@ -3,8 +3,10 @@ package controller import ( "context" "fmt" + "slices" "testing" + "github.com/davecgh/go-spew/spew" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" discoveryv1 "k8s.io/api/discovery/v1" @@ -16,11 +18,14 @@ import ( "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/log/zap" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" "go.datum.net/network-services-operator/internal/config" downstreamclient "go.datum.net/network-services-operator/internal/downstreamclient" + gatewayutil "go.datum.net/network-services-operator/internal/util/gateway" ) func TestEnsureDownstreamGateway(t *testing.T) { @@ -52,78 +57,35 @@ func TestEnsureDownstreamGateway(t *testing.T) { existingDownstreamObjects []client.Object assert func(t *testing.T, upstreamGateway, downstreamGateway *gatewayv1.Gateway) }{ - { - name: "http listener only", - upstreamGateway: newGateway(upstreamNamespace.Name, "test", func(g *gatewayv1.Gateway) { - g.Spec.Listeners = []gatewayv1.Listener{ - { - Name: gatewayv1.SectionName(SchemeHTTP), - Port: DefaultHTTPPort, - Protocol: gatewayv1.HTTPProtocolType, - }, - } - }), - assert: func(t *testing.T, upstreamGateway, downstreamGateway *gatewayv1.Gateway) { - assert.Len(t, downstreamGateway.Spec.Listeners, 1) - assert.Equal(t, gatewayv1.PortNumber(DefaultHTTPPort), downstreamGateway.Spec.Listeners[0].Port) - assert.Equal(t, gatewayv1.HTTPProtocolType, downstreamGateway.Spec.Listeners[0].Protocol) - }, - }, - { - name: "https listener only", - upstreamGateway: newGateway(upstreamNamespace.Name, "test", func(g *gatewayv1.Gateway) { - g.Spec.Listeners = []gatewayv1.Listener{ - { - Name: gatewayv1.SectionName(SchemeHTTPS), - Port: DefaultHTTPSPort, - Protocol: gatewayv1.HTTPSProtocolType, - }, - } - }), - assert: func(t *testing.T, upstreamGateway, downstreamGateway *gatewayv1.Gateway) { - assert.Len(t, downstreamGateway.Spec.Listeners, 1) - assert.Equal(t, gatewayv1.PortNumber(DefaultHTTPSPort), downstreamGateway.Spec.Listeners[0].Port) - assert.Equal(t, gatewayv1.HTTPSProtocolType, downstreamGateway.Spec.Listeners[0].Protocol) - }, - }, { name: "http and https listeners", - upstreamGateway: newGateway(upstreamNamespace.Name, "test", func(g *gatewayv1.Gateway) { - g.Spec.Listeners = []gatewayv1.Listener{ - { - Name: gatewayv1.SectionName(SchemeHTTP), - Port: DefaultHTTPPort, - Protocol: gatewayv1.HTTPProtocolType, - }, - { - Name: gatewayv1.SectionName(SchemeHTTPS), - Port: DefaultHTTPSPort, - Protocol: gatewayv1.HTTPSProtocolType, - }, - } + upstreamGateway: newGateway(testConfig, upstreamNamespace.Name, "test", func(g *gatewayv1.Gateway) { + }), assert: func(t *testing.T, upstreamGateway, downstreamGateway *gatewayv1.Gateway) { - assert.Len(t, downstreamGateway.Spec.Listeners, 2) + spew.Dump(downstreamGateway) + if assert.Len(t, downstreamGateway.Spec.Listeners, 2) { - assert.Equal(t, gatewayv1.PortNumber(DefaultHTTPPort), downstreamGateway.Spec.Listeners[0].Port) - assert.Equal(t, gatewayv1.HTTPProtocolType, downstreamGateway.Spec.Listeners[0].Protocol) + assert.Equal(t, gatewayv1.PortNumber(DefaultHTTPPort), downstreamGateway.Spec.Listeners[0].Port) + assert.Equal(t, gatewayv1.HTTPProtocolType, downstreamGateway.Spec.Listeners[0].Protocol) - assert.Equal(t, gatewayv1.PortNumber(DefaultHTTPSPort), downstreamGateway.Spec.Listeners[1].Port) - assert.Equal(t, gatewayv1.HTTPSProtocolType, downstreamGateway.Spec.Listeners[1].Protocol) + assert.Equal(t, gatewayv1.PortNumber(DefaultHTTPSPort), downstreamGateway.Spec.Listeners[1].Port) + assert.Equal(t, gatewayv1.HTTPSProtocolType, downstreamGateway.Spec.Listeners[1].Protocol) + } }, }, { name: "hostname claimed by different gateway", - upstreamGateway: newGateway(upstreamNamespace.Name, "test", func(g *gatewayv1.Gateway) { + upstreamGateway: newGateway(testConfig, upstreamNamespace.Name, "test", func(g *gatewayv1.Gateway) { g.Spec.Listeners = []gatewayv1.Listener{ { - Name: gatewayv1.SectionName(SchemeHTTP), + Name: "custom-hostname-0", Port: DefaultHTTPPort, Protocol: gatewayv1.HTTPProtocolType, Hostname: ptr.To(gatewayv1.Hostname("example.com")), }, { - Name: gatewayv1.SectionName(SchemeHTTPS), + Name: "custom-hostname-1", Port: DefaultHTTPPort, Protocol: gatewayv1.HTTPProtocolType, Hostname: ptr.To(gatewayv1.Hostname("test.example.com")), @@ -180,6 +142,8 @@ func TestEnsureDownstreamGateway(t *testing.T) { }, } + logger := zap.New(zap.UseFlagOptions(&zap.Options{Development: true})) + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -213,6 +177,7 @@ func TestEnsureDownstreamGateway(t *testing.T) { Build() ctx := context.Background() + ctx = log.IntoContext(ctx, logger) mgr := &fakeMockManager{cl: fakeUpstreamClient} @@ -224,6 +189,7 @@ func TestEnsureDownstreamGateway(t *testing.T) { downstreamStrategy := downstreamclient.NewMappedNamespaceResourceStrategy("test", fakeUpstreamClient, fakeDownstreamClient) + reconciler.prepareUpstreamGateway(tt.upstreamGateway) result, downstreamGateway := reconciler.ensureDownstreamGateway( ctx, "test-suite", @@ -253,6 +219,14 @@ func TestEnsureDownstreamGatewayHTTPRoutes(t *testing.T) { assert.NoError(t, gatewayv1.Install(testScheme)) assert.NoError(t, discoveryv1.AddToScheme(testScheme)) + testConfig := config.NetworkServicesOperator{ + Gateway: config.GatewayConfig{ + DownstreamGatewayClassName: "test-suite", + DownstreamHostnameAccountingNamespace: "default", + TargetDomain: "test-suite.com", + }, + } + upstreamNamespace := &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: "test", @@ -269,7 +243,7 @@ func TestEnsureDownstreamGatewayHTTPRoutes(t *testing.T) { }{ { name: "no routes", - upstreamGateway: newGateway(upstreamNamespace.Name, "test"), + upstreamGateway: newGateway(testConfig, upstreamNamespace.Name, "test"), assert: func(t *testing.T, gateway *gatewayv1.Gateway) { assert.Len(t, gateway.Status.Listeners, len(gateway.Spec.Listeners), "number of listeners in status does not match spec") @@ -280,7 +254,7 @@ func TestEnsureDownstreamGatewayHTTPRoutes(t *testing.T) { }, { name: "single route, all listeners", - upstreamGateway: newGateway(upstreamNamespace.Name, "test"), + upstreamGateway: newGateway(testConfig, upstreamNamespace.Name, "test"), existingUpstreamObjects: []client.Object{ newHTTPRoute(upstreamNamespace.Name, "route-1", func(route *gatewayv1.HTTPRoute) { route.Spec.ParentRefs = []gatewayv1.ParentReference{ @@ -300,7 +274,7 @@ func TestEnsureDownstreamGatewayHTTPRoutes(t *testing.T) { }, { name: "multiple routes, varied listener attachments", - upstreamGateway: newGateway( + upstreamGateway: newGateway(testConfig, upstreamNamespace.Name, "test", func(g *gatewayv1.Gateway) { @@ -408,7 +382,7 @@ func TestEnsureDownstreamGatewayHTTPRoutes(t *testing.T) { WithStatusSubresource(tt.existingUpstreamObjects...). Build() - downstreamGateway := newGateway(downstreamNamespaceName, tt.upstreamGateway.Name) + downstreamGateway := newGateway(testConfig, downstreamNamespaceName, tt.upstreamGateway.Name) fakeDownstreamClient := fake.NewClientBuilder(). WithScheme(testScheme). @@ -490,7 +464,7 @@ func TestEnsureHostnamesClaimed(t *testing.T) { }{ { name: "domains created for custom hostnames on HTTP listeners", - upstreamGateway: newGateway(upstreamNamespace.Name, "test", func(g *gatewayv1.Gateway) { + upstreamGateway: newGateway(testConfig, upstreamNamespace.Name, "test", func(g *gatewayv1.Gateway) { g.Spec.Listeners = []gatewayv1.Listener{ { Name: "listener-1", @@ -532,7 +506,7 @@ func TestEnsureHostnamesClaimed(t *testing.T) { }, { name: "domain exists but is not verified", - upstreamGateway: newGateway(upstreamNamespace.Name, "test", func(g *gatewayv1.Gateway) { + upstreamGateway: newGateway(testConfig, upstreamNamespace.Name, "test", func(g *gatewayv1.Gateway) { g.Spec.Listeners = []gatewayv1.Listener{ { Name: gatewayv1.SectionName(SchemeHTTP), @@ -549,7 +523,7 @@ func TestEnsureHostnamesClaimed(t *testing.T) { }, { name: "verified domain exists", - upstreamGateway: newGateway(upstreamNamespace.Name, "test", func(g *gatewayv1.Gateway) { + upstreamGateway: newGateway(testConfig, upstreamNamespace.Name, "test", func(g *gatewayv1.Gateway) { g.Spec.Listeners = []gatewayv1.Listener{ { Name: "listener-1", @@ -578,7 +552,7 @@ func TestEnsureHostnamesClaimed(t *testing.T) { }, { name: "verified domain exists but hostname is claimed by different gateway", - upstreamGateway: newGateway(upstreamNamespace.Name, "test", func(g *gatewayv1.Gateway) { + upstreamGateway: newGateway(testConfig, upstreamNamespace.Name, "test", func(g *gatewayv1.Gateway) { g.Spec.Listeners = []gatewayv1.Listener{ { Name: "listener-1", @@ -619,7 +593,7 @@ func TestEnsureHostnamesClaimed(t *testing.T) { }, { name: "hostname verified by being programmed on downstream gateway", - upstreamGateway: newGateway(upstreamNamespace.Name, "test", func(g *gatewayv1.Gateway) { + upstreamGateway: newGateway(testConfig, upstreamNamespace.Name, "test", func(g *gatewayv1.Gateway) { g.Spec.Listeners = []gatewayv1.Listener{ { Name: gatewayv1.SectionName(SchemeHTTP), @@ -629,7 +603,7 @@ func TestEnsureHostnamesClaimed(t *testing.T) { }, } }), - downstreamGateway: newGateway(downstreamNamespaceName, "test", func(g *gatewayv1.Gateway) { + downstreamGateway: newGateway(testConfig, downstreamNamespaceName, "test", func(g *gatewayv1.Gateway) { g.Spec.Listeners = []gatewayv1.Listener{ { Name: gatewayv1.SectionName(SchemeHTTP), @@ -644,7 +618,7 @@ func TestEnsureHostnamesClaimed(t *testing.T) { }, { name: "exact or subdomain match only", - upstreamGateway: newGateway(upstreamNamespace.Name, "test", func(g *gatewayv1.Gateway) { + upstreamGateway: newGateway(testConfig, upstreamNamespace.Name, "test", func(g *gatewayv1.Gateway) { g.Spec.Listeners = []gatewayv1.Listener{ { Name: "listener-1", @@ -735,8 +709,18 @@ func TestEnsureHostnamesClaimed(t *testing.T) { ) if assert.NoError(t, err, "unexpected error calling ensureHostnameVerification") { - assert.EqualValues(t, tt.expectedVerifiedHostnames, verifiedHostnames, "expected verified hostnames mismatch") - assert.EqualValues(t, tt.expectedClaimedHostnames, claimedHostnames, "expected claimed hostnames mistmatch") + expectedVerifiedHostnames := append( + tt.expectedVerifiedHostnames, + testConfig.Gateway.GatewayDNSAddress(tt.upstreamGateway), + ) + expectedClaimedHostnames := append( + tt.expectedClaimedHostnames, + testConfig.Gateway.GatewayDNSAddress(tt.upstreamGateway), + ) + slices.Sort(expectedVerifiedHostnames) + slices.Sort(expectedClaimedHostnames) + assert.EqualValues(t, expectedVerifiedHostnames, verifiedHostnames, "expected verified hostnames mismatch") + assert.EqualValues(t, expectedClaimedHostnames, claimedHostnames, "expected claimed hostnames mistmatch") assert.EqualValues(t, tt.expectedNotClaimedHostnames, notClaimedHostnames, "expected not claimed hostnames mismatch") } @@ -751,29 +735,20 @@ func TestEnsureHostnamesClaimed(t *testing.T) { } } -func newGateway(namespace, name string, opts ...func(*gatewayv1.Gateway)) *gatewayv1.Gateway { +func newGateway(testConfig config.NetworkServicesOperator, namespace, name string, opts ...func(*gatewayv1.Gateway)) *gatewayv1.Gateway { gw := &gatewayv1.Gateway{ ObjectMeta: metav1.ObjectMeta{ Namespace: namespace, Name: name, + UID: uuid.NewUUID(), }, Spec: gatewayv1.GatewaySpec{ GatewayClassName: "test", - Listeners: []gatewayv1.Listener{ - { - Name: gatewayv1.SectionName(SchemeHTTP), - Port: DefaultHTTPPort, - Protocol: gatewayv1.HTTPProtocolType, - }, - { - Name: gatewayv1.SectionName(SchemeHTTPS), - Port: DefaultHTTPSPort, - Protocol: gatewayv1.HTTPSProtocolType, - }, - }, }, } + gatewayutil.SetDefaultListeners(gw, testConfig.Gateway) + for _, opt := range opts { opt(gw) } @@ -786,6 +761,7 @@ func newHTTPRoute(namespace, name string, opts ...func(*gatewayv1.HTTPRoute)) *g ObjectMeta: metav1.ObjectMeta{ Namespace: namespace, Name: name, + UID: uuid.NewUUID(), }, Spec: gatewayv1.HTTPRouteSpec{}, } diff --git a/internal/controller/httpproxy_controller.go b/internal/controller/httpproxy_controller.go index 1ff24de1..7b502ee0 100644 --- a/internal/controller/httpproxy_controller.go +++ b/internal/controller/httpproxy_controller.go @@ -31,6 +31,7 @@ import ( networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" "go.datum.net/network-services-operator/internal/config" conditionutil "go.datum.net/network-services-operator/internal/util/condition" + gatewayutil "go.datum.net/network-services-operator/internal/util/gateway" ) // HTTPProxyReconciler reconciles a HTTPProxy object @@ -129,6 +130,20 @@ func (r *HTTPProxyReconciler) Reconcile(ctx context.Context, req mcreconcile.Req return fmt.Errorf("failed to set controller on gateway: %w", err) } + // Special handling for default gateway listeners, as the hostnames will be + // updated by the controller. Only required on updates. + if !gateway.CreationTimestamp.IsZero() { + defaultHTTPListener := gatewayutil.GetListenerByName(gateway.Spec.Listeners, gatewayutil.DefaultHTTPListenerName) + if defaultHTTPListener != nil { + desiredResources.gateway.Spec.Listeners = append(desiredResources.gateway.Spec.Listeners, *defaultHTTPListener) + } + + defaultHTTPSListener := gatewayutil.GetListenerByName(gateway.Spec.Listeners, gatewayutil.DefaultHTTPSListenerName) + if defaultHTTPSListener != nil { + desiredResources.gateway.Spec.Listeners = append(desiredResources.gateway.Spec.Listeners, *defaultHTTPSListener) + } + } + gateway.Spec = desiredResources.gateway.Spec return nil @@ -380,35 +395,14 @@ func (r *HTTPProxyReconciler) collectDesiredResources( }, Spec: gatewayv1.GatewaySpec{ GatewayClassName: r.Config.HTTPProxy.GatewayClassName, - Listeners: []gatewayv1.Listener{ - { - Name: SchemeHTTP, - Protocol: gatewayv1.HTTPProtocolType, - Port: DefaultHTTPPort, - AllowedRoutes: &gatewayv1.AllowedRoutes{ - Namespaces: &gatewayv1.RouteNamespaces{ - From: ptr.To(gatewayv1.NamespacesFromSame), - }, - }, - }, - { - Name: SchemeHTTPS, - Protocol: gatewayv1.HTTPSProtocolType, - Port: DefaultHTTPSPort, - AllowedRoutes: &gatewayv1.AllowedRoutes{ - Namespaces: &gatewayv1.RouteNamespaces{ - From: ptr.To(gatewayv1.NamespacesFromSame), - }, - }, - TLS: &gatewayv1.GatewayTLSConfig{ - Mode: ptr.To(gatewayv1.TLSModeTerminate), - Options: r.Config.HTTPProxy.GatewayTLSOptions, - }, - }, - }, }, } + // Hostname fields will be nil on the default listeners until the gateway + // controller updates them. There's special handling for this in the + // CreateOrUpdate logic for maintaining the gateway. + gatewayutil.SetDefaultListeners(gateway, r.Config.Gateway) + // Add listeners for each hostname for i, hostname := range httpProxy.Spec.Hostnames { gateway.Spec.Listeners = append(gateway.Spec.Listeners, gatewayv1.Listener{ @@ -435,7 +429,7 @@ func (r *HTTPProxyReconciler) collectDesiredResources( }, TLS: &gatewayv1.GatewayTLSConfig{ Mode: ptr.To(gatewayv1.TLSModeTerminate), - Options: r.Config.HTTPProxy.GatewayTLSOptions, + Options: r.Config.Gateway.ListenerTLSOptions, }, }) } diff --git a/internal/controller/httpproxy_controller_test.go b/internal/controller/httpproxy_controller_test.go index 30ff1d2d..12e6db3a 100644 --- a/internal/controller/httpproxy_controller_test.go +++ b/internal/controller/httpproxy_controller_test.go @@ -31,12 +31,15 @@ import ( func TestHTTPProxyCollectDesiredResources(t *testing.T) { operatorConfig := config.NetworkServicesOperator{ - HTTPProxy: config.HTTPProxyConfig{ - GatewayClassName: "test", - GatewayTLSOptions: map[gatewayv1.AnnotationKey]gatewayv1.AnnotationValue{ + Gateway: config.GatewayConfig{ + TargetDomain: "example.com", + ListenerTLSOptions: map[gatewayv1.AnnotationKey]gatewayv1.AnnotationValue{ gatewayv1.AnnotationKey("gateway.networking.datumapis.com/certificate-issuer"): gatewayv1.AnnotationValue("test"), }, }, + HTTPProxy: config.HTTPProxyConfig{ + GatewayClassName: "test", + }, } tests := []struct { @@ -252,7 +255,7 @@ func TestHTTPProxyCollectDesiredResources(t *testing.T) { assert.Equal(t, operatorConfig.HTTPProxy.GatewayClassName, gateway.Spec.GatewayClassName) assert.Len(t, gateway.Spec.Listeners, 2+(len(tt.httpProxy.Spec.Hostnames)*2)) - assert.Equal(t, operatorConfig.HTTPProxy.GatewayTLSOptions, gateway.Spec.Listeners[1].TLS.Options) + assert.Equal(t, operatorConfig.Gateway.ListenerTLSOptions, gateway.Spec.Listeners[1].TLS.Options) // HTTPRoute assertions on items that are not hard coded assert.Equal(t, tt.httpProxy.Namespace, httpRoute.Namespace) @@ -297,12 +300,12 @@ func TestHTTPProxyReconcile(t *testing.T) { testConfig := config.NetworkServicesOperator{ HTTPProxy: config.HTTPProxyConfig{ GatewayClassName: "test-gateway-class", - GatewayTLSOptions: map[gatewayv1.AnnotationKey]gatewayv1.AnnotationValue{ - gatewayv1.AnnotationKey("gateway.networking.datumapis.com/certificate-issuer"): gatewayv1.AnnotationValue("test-issuer"), - }, }, Gateway: config.GatewayConfig{ TargetDomain: "example.com", + ListenerTLSOptions: map[gatewayv1.AnnotationKey]gatewayv1.AnnotationValue{ + gatewayv1.AnnotationKey("gateway.networking.datumapis.com/certificate-issuer"): gatewayv1.AnnotationValue("test-issuer"), + }, }, } diff --git a/internal/util/gateway/listeners.go b/internal/util/gateway/listeners.go new file mode 100644 index 00000000..47b6c441 --- /dev/null +++ b/internal/util/gateway/listeners.go @@ -0,0 +1,70 @@ +package gateway + +import ( + "k8s.io/utils/ptr" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + + "go.datum.net/network-services-operator/internal/config" +) + +const ( + DefaultHTTPPort = 80 + DefaultHTTPSPort = 443 + + DefaultHTTPListenerName = "default-http" + DefaultHTTPSListenerName = "default-https" +) + +func IsDefaultListener(l gatewayv1.Listener) bool { + return l.Name == DefaultHTTPListenerName || l.Name == DefaultHTTPSListenerName +} + +func GetListenerByName(listeners []gatewayv1.Listener, name gatewayv1.SectionName) *gatewayv1.Listener { + for i, l := range listeners { + if l.Name == name { + return &listeners[i] + } + } + return nil +} + +// SetListener will insert a listener into the gateway, or replace the existing +// listener of the same name with the provided listener. +func SetListener(gateway *gatewayv1.Gateway, listener gatewayv1.Listener) { + for i, l := range gateway.Spec.Listeners { + if l.Name == listener.Name { + gateway.Spec.Listeners[i] = listener + return + } + } + + gateway.Spec.Listeners = append(gateway.Spec.Listeners, listener) +} + +func SetDefaultListeners(gateway *gatewayv1.Gateway, gatewayConfig config.GatewayConfig) { + SetListener(gateway, gatewayv1.Listener{ + Name: DefaultHTTPListenerName, + Protocol: gatewayv1.HTTPProtocolType, + Port: DefaultHTTPPort, + AllowedRoutes: &gatewayv1.AllowedRoutes{ + Namespaces: &gatewayv1.RouteNamespaces{ + From: ptr.To(gatewayv1.NamespacesFromSame), + }, + }, + }) + + SetListener(gateway, gatewayv1.Listener{ + Name: DefaultHTTPSListenerName, + Protocol: gatewayv1.HTTPSProtocolType, + Port: DefaultHTTPSPort, + AllowedRoutes: &gatewayv1.AllowedRoutes{ + Namespaces: &gatewayv1.RouteNamespaces{ + From: ptr.To(gatewayv1.NamespacesFromSame), + }, + }, + TLS: &gatewayv1.GatewayTLSConfig{ + Mode: ptr.To(gatewayv1.TLSModeTerminate), + Options: gatewayConfig.ListenerTLSOptions, + }, + }) +} diff --git a/internal/validation/gateway_validation.go b/internal/validation/gateway_validation.go index 573841b6..f9b12295 100644 --- a/internal/validation/gateway_validation.go +++ b/internal/validation/gateway_validation.go @@ -8,11 +8,8 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/utils/ptr" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" -) -const ( - HTTPPort = 80 - HTTPSPort = 443 + gatewayutil "go.datum.net/network-services-operator/internal/util/gateway" ) func ValidateGateway(gateway *gatewayv1.Gateway, opts GatewayValidationOptions) field.ErrorList { @@ -22,7 +19,7 @@ func ValidateGateway(gateway *gatewayv1.Gateway, opts GatewayValidationOptions) allErrs = append(allErrs, field.Required(field.NewPath("spec", "gatewayClassName"), "gatewayClassName is required")) } - allErrs = append(allErrs, validateListeners(gateway.Spec.Listeners, field.NewPath("spec", "listeners"), opts)...) + allErrs = append(allErrs, validateListeners(gateway, field.NewPath("spec", "listeners"), opts)...) if len(gateway.Spec.Addresses) > 0 { allErrs = append(allErrs, field.TooMany(field.NewPath("spec", "addresses"), len(gateway.Spec.Addresses), 0)) @@ -39,13 +36,23 @@ func ValidateGateway(gateway *gatewayv1.Gateway, opts GatewayValidationOptions) return allErrs } -func validateListeners(listeners []gatewayv1.Listener, fldPath *field.Path, opts GatewayValidationOptions) field.ErrorList { +func validateListeners(gateway *gatewayv1.Gateway, fldPath *field.Path, opts GatewayValidationOptions) field.ErrorList { allErrs := field.ErrorList{} - for i, l := range listeners { + for i, l := range gateway.Spec.Listeners { listenerPath := fldPath.Index(i) - if l.Hostname != nil { + if gatewayutil.IsDefaultListener(l) { + expectedHostname := opts.GatewayDNSAddressFunc(gateway) + + // Require expected hostname on default listeners if set. Note that these + // listeners will not have the hostname set at time of creation. + if l.Hostname != nil && *l.Hostname != gatewayv1.Hostname(expectedHostname) { + allErrs = append(allErrs, field.NotSupported(listenerPath.Child("hostname"), l.Hostname, []string{expectedHostname})) + } + } else if l.Hostname == nil { + allErrs = append(allErrs, field.Required(listenerPath.Child("hostname"), fmt.Sprintf("must be set to %q or a custom hostname", opts.GatewayDNSAddressFunc(gateway)))) + } else { allErrs = append(allErrs, validation.IsFullyQualifiedDomainName(listenerPath.Child("hostname"), string(*l.Hostname))...) } @@ -127,11 +134,12 @@ func validateGatewayTLSConfig(tls *gatewayv1.GatewayTLSConfig, fldPath *field.Pa } type GatewayValidationOptions struct { - ControllerName gatewayv1.GatewayController - PermittedTLSOptions map[string][]string - ValidPortNumbers validPortNumbers - ValidProtocolTypes map[int][]gatewayv1.ProtocolType - ClusterName string + ControllerName gatewayv1.GatewayController + PermittedTLSOptions map[string][]string + ValidPortNumbers validPortNumbers + ValidProtocolTypes map[int][]gatewayv1.ProtocolType + GatewayDNSAddressFunc func(gateway *gatewayv1.Gateway) string + ClusterName string } type validPortNumbers []int diff --git a/internal/validation/gateway_validation_test.go b/internal/validation/gateway_validation_test.go index 389a011d..b30142d8 100644 --- a/internal/validation/gateway_validation_test.go +++ b/internal/validation/gateway_validation_test.go @@ -5,16 +5,28 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "k8s.io/apimachinery/pkg/util/uuid" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/utils/ptr" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + + "go.datum.net/network-services-operator/internal/config" + gatewayutil "go.datum.net/network-services-operator/internal/util/gateway" ) func TestValidateGateway(t *testing.T) { defaultValidProtocolTypes := map[int][]gatewayv1.ProtocolType{ - HTTPPort: {gatewayv1.HTTPProtocolType}, - HTTPSPort: {gatewayv1.HTTPSProtocolType}, + gatewayutil.DefaultHTTPPort: {gatewayv1.HTTPProtocolType}, + gatewayutil.DefaultHTTPSPort: {gatewayv1.HTTPSProtocolType}, + } + + testConfig := config.NetworkServicesOperator{ + Gateway: config.GatewayConfig{ + DownstreamGatewayClassName: "test-suite", + DownstreamHostnameAccountingNamespace: "default", + TargetDomain: "test-suite.com", + }, } scenarios := map[string]struct { @@ -32,9 +44,6 @@ func TestValidateGateway(t *testing.T) { field.Required(field.NewPath("spec", "gatewayClassName"), "gatewayClassName is required"), }, }, - // "invalid hostname": { - - // }, "invalid port number": { gateway: &gatewayv1.Gateway{ Spec: gatewayv1.GatewaySpec{ @@ -353,6 +362,15 @@ func TestValidateGateway(t *testing.T) { for name, scenario := range scenarios { t.Run(name, func(t *testing.T) { + scenario.opts.GatewayDNSAddressFunc = testConfig.Gateway.GatewayDNSAddress + scenario.gateway.UID = uuid.NewUUID() + + for i, l := range scenario.gateway.Spec.Listeners { + if l.Hostname == nil { + scenario.gateway.Spec.Listeners[i].Hostname = ptr.To(gatewayv1.Hostname(testConfig.Gateway.GatewayDNSAddress(scenario.gateway))) + } + } + errs := ValidateGateway(scenario.gateway, scenario.opts) delta := cmp.Diff(scenario.expectedErrors, errs, cmpopts.IgnoreFields(field.Error{}, "BadValue", "Detail")) if delta != "" { diff --git a/internal/webhook/v1/gateway_webhook.go b/internal/webhook/v1/gateway_webhook.go index 47f31178..d324bdb3 100644 --- a/internal/webhook/v1/gateway_webhook.go +++ b/internal/webhook/v1/gateway_webhook.go @@ -18,15 +18,26 @@ import ( mccontext "sigs.k8s.io/multicluster-runtime/pkg/context" mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager" + "go.datum.net/network-services-operator/internal/config" + gatewayutil "go.datum.net/network-services-operator/internal/util/gateway" "go.datum.net/network-services-operator/internal/validation" ) // nolint:unused // SetupGatewayWebhookWithManager registers the webhook for Gateway in the manager. -func SetupGatewayWebhookWithManager(mgr mcmanager.Manager, validationOpts validation.GatewayValidationOptions) error { +func SetupGatewayWebhookWithManager(mgr mcmanager.Manager, config config.NetworkServicesOperator) error { + validationOpts := validation.GatewayValidationOptions{ + ControllerName: config.Gateway.ControllerName, + PermittedTLSOptions: config.Gateway.PermittedTLSOptions, + ValidPortNumbers: config.Gateway.ValidPortNumbers, + ValidProtocolTypes: config.Gateway.ValidProtocolTypes, + GatewayDNSAddressFunc: config.Gateway.GatewayDNSAddress, + } + return ctrl.NewWebhookManagedBy(mgr.GetLocalManager()).For(&gatewayv1.Gateway{}). WithValidator(&GatewayCustomValidator{mgr: mgr, validationOpts: validationOpts}). + WithDefaulter(&GatewayCustomDefaulter{mgr: mgr, config: config}). Complete() } @@ -57,7 +68,7 @@ func (v *GatewayCustomValidator) ValidateCreate(ctx context.Context, obj runtime } clusterClient := cluster.GetClient() - if shouldProcess, err := v.shouldProcess(ctx, clusterClient, gateway); !shouldProcess || err != nil { + if shouldProcess, err := shouldProcess(ctx, clusterClient, v.validationOpts.ControllerName, gateway); !shouldProcess || err != nil { return nil, err } @@ -92,7 +103,7 @@ func (v *GatewayCustomValidator) ValidateUpdate(ctx context.Context, oldObj, new } clusterClient := cluster.GetClient() - if shouldProcess, err := v.shouldProcess(ctx, clusterClient, gateway); !shouldProcess || err != nil { + if shouldProcess, err := shouldProcess(ctx, clusterClient, v.validationOpts.ControllerName, gateway); !shouldProcess || err != nil { return nil, err } @@ -118,12 +129,61 @@ func (v *GatewayCustomValidator) ValidateDelete(ctx context.Context, obj runtime gatewaylog := logf.FromContext(ctx) gatewaylog.Info("Validation for Gateway upon deletion", "name", gateway.GetName()) - // TODO(user): fill in your validation logic upon object deletion. - return nil, nil } -func (v *GatewayCustomValidator) shouldProcess(ctx context.Context, clusterClient client.Client, gateway *gatewayv1.Gateway) (bool, error) { +// +kubebuilder:webhook:path=/mutate-gateway-networking-k8s-io-v1-gateway,mutating=true,failurePolicy=fail,sideEffects=None,groups=gateway.networking.k8s.io,resources=gateways,verbs=create;update,versions=v1,name=mgateway-v1.kb.io,admissionReviewVersions=v1 + +type GatewayCustomDefaulter struct { + mgr mcmanager.Manager + config config.NetworkServicesOperator +} + +var _ webhook.CustomDefaulter = &GatewayCustomDefaulter{} + +// Default implements webhook.CustomDefaulter so a webhook will be registered for the Kind Gateway. +func (d *GatewayCustomDefaulter) Default(ctx context.Context, obj runtime.Object) error { + gateway, ok := obj.(*gatewayv1.Gateway) + if !ok { + return fmt.Errorf("expected a Gateway object but got %T", obj) + } + + clusterName, ok := mccontext.ClusterFrom(ctx) + if !ok { + return fmt.Errorf("expected a cluster name in the context") + } + + cluster, err := d.mgr.GetCluster(ctx, clusterName) + if err != nil { + return err + } + clusterClient := cluster.GetClient() + + if shouldProcess, err := shouldProcess(ctx, clusterClient, d.config.Gateway.ControllerName, gateway); !shouldProcess || err != nil { + return err + } + + gatewaylog := logf.FromContext(ctx).WithValues("cluster", clusterName) + gatewaylog.Info("Defaulting for Gateway", "name", gateway.GetName()) + + // Inject default listeners at time of creation. These will be updated by + // the controller to have the hostname fields set to a value that includes the + // UID of the resource, which is not available in a mutating webhook at time + // of creation. + + if gateway.CreationTimestamp.IsZero() { + gatewayutil.SetDefaultListeners(gateway, d.config.Gateway) + } + + return nil +} + +func shouldProcess( + ctx context.Context, + clusterClient client.Client, + controllerName gatewayv1.GatewayController, + gateway *gatewayv1.Gateway, +) (bool, error) { var gatewayClass gatewayv1.GatewayClass if err := clusterClient.Get(ctx, types.NamespacedName{Name: string(gateway.Spec.GatewayClassName)}, &gatewayClass); err != nil { if apierrors.IsNotFound(err) { @@ -134,7 +194,7 @@ func (v *GatewayCustomValidator) shouldProcess(ctx context.Context, clusterClien return false, err } - if gatewayClass.Spec.ControllerName != v.validationOpts.ControllerName { + if gatewayClass.Spec.ControllerName != controllerName { // No error if the GatewayClass is not managed by the operator logf.FromContext(ctx).Info("GatewayClass is not managed by the operator, skipping validation", "name", gatewayClass.GetName()) return false, nil diff --git a/test/e2e/gateway/chainsaw-test.yaml b/test/e2e/gateway/chainsaw-test.yaml index ec334d09..503ed196 100644 --- a/test/e2e/gateway/chainsaw-test.yaml +++ b/test/e2e/gateway/chainsaw-test.yaml @@ -286,12 +286,6 @@ spec: spec: gatewayClassName: ($gatewayClassName) listeners: - - protocol: HTTP - port: 80 - name: http - allowedRoutes: - namespaces: - from: Same - protocol: HTTP port: 80 name: http-test-e2e @@ -299,16 +293,6 @@ spec: namespaces: from: Same hostname: test.e2e.env.datum.net - - protocol: HTTPS - port: 443 - name: https - allowedRoutes: - namespaces: - from: Same - tls: - mode: Terminate - options: - gateway.networking.datumapis.com/certificate-issuer: ($clusterIssuerName) - protocol: HTTPS port: 443 name: https-test-e2e @@ -365,61 +349,21 @@ spec: namespaces: from: Same hostname: (join('.', [$upstreamGateway.metadata.uid, 'prism.e2e.env.datum.net'])) - name: http-0 + name: default-http port: 80 protocol: HTTP - allowedRoutes: namespaces: from: Same hostname: (join('.', [$upstreamGateway.metadata.uid, 'prism.e2e.env.datum.net'])) - name: https-0 - port: 443 - protocol: HTTPS - tls: - certificateRefs: - - group: "" - kind: Secret - name: test-gateway-https-0 - mode: Terminate - - allowedRoutes: - namespaces: - from: Same - hostname: (join('.', ['v4', $upstreamGateway.metadata.uid, 'prism.e2e.env.datum.net'])) - name: http-1 - port: 80 - protocol: HTTP - - allowedRoutes: - namespaces: - from: Same - hostname: (join('.', ['v4', $upstreamGateway.metadata.uid, 'prism.e2e.env.datum.net'])) - name: https-1 - port: 443 - protocol: HTTPS - tls: - certificateRefs: - - group: "" - kind: Secret - name: test-gateway-https-1 - mode: Terminate - - allowedRoutes: - namespaces: - from: Same - hostname: (join('.', ['v6', $upstreamGateway.metadata.uid, 'prism.e2e.env.datum.net'])) - name: http-2 - port: 80 - protocol: HTTP - - allowedRoutes: - namespaces: - from: Same - hostname: (join('.', ['v6', $upstreamGateway.metadata.uid, 'prism.e2e.env.datum.net'])) - name: https-2 + name: default-https port: 443 protocol: HTTPS tls: certificateRefs: - group: "" kind: Secret - name: test-gateway-https-2 + name: test-gateway-default-https mode: Terminate status: conditions: @@ -444,32 +388,12 @@ spec: - status: "True" - status: "True" - status: "True" - name: http-0 - - (conditions[?contains(['Programmed', 'Accepted', 'ResolvedRefs'], type)]): - - status: "True" - - status: "True" - - status: "True" - name: https-0 - - (conditions[?contains(['Programmed', 'Accepted', 'ResolvedRefs'], type)]): - - status: "True" - - status: "True" - - status: "True" - name: http-1 - - (conditions[?contains(['Programmed', 'Accepted', 'ResolvedRefs'], type)]): - - status: "True" - - status: "True" - - status: "True" - name: https-1 + name: default-http - (conditions[?contains(['Programmed', 'Accepted', 'ResolvedRefs'], type)]): - status: "True" - status: "True" - status: "True" - name: http-2 - - (conditions[?contains(['Programmed', 'Accepted', 'ResolvedRefs'], type)]): - - status: "True" - - status: "True" - - status: "True" - name: https-2 + name: default-https # Load the downstream gateway so we can get the IP address info from its # status. @@ -555,7 +479,7 @@ spec: - status: "True" (conditions[?type == 'ResolvedRefs']): - status: "True" - name: http + name: http-test-e2e - attachedRoutes: 0 (conditions[?type == 'Programmed']): - status: "True" @@ -563,7 +487,7 @@ spec: - status: "True" (conditions[?type == 'ResolvedRefs']): - status: "True" - name: http-test-e2e + name: https-test-e2e - attachedRoutes: 0 (conditions[?type == 'Programmed']): - status: "True" @@ -571,7 +495,7 @@ spec: - status: "True" (conditions[?type == 'ResolvedRefs']): - status: "True" - name: https + name: default-http - attachedRoutes: 0 (conditions[?type == 'Programmed']): - status: "True" @@ -579,7 +503,7 @@ spec: - status: "True" (conditions[?type == 'ResolvedRefs']): - status: "True" - name: https-test-e2e + name: default-https (conditions[?type == 'Accepted']): - status: "True" (conditions[?type == 'Programmed']): @@ -594,12 +518,15 @@ spec: kubectl get endpointslices -n $NAMESPACE -o yaml - script: cluster: nso-infra + env: + - name: DOWNSTREAM_NAMESPACE + value: ($downstreamNamespaceName) content: | - kubectl get gateways -n $NAMESPACE -o yaml - kubectl get httproutes -n $NAMESPACE -o yaml - kubectl get endpointslices -n $NAMESPACE -o yaml - kubectl get dnsendpoints -n $NAMESPACE -o yaml - kubectl get svc -n $NAMESPACE -o yaml + kubectl get gateways -n $DOWNSTREAM_NAMESPACE -o yaml + kubectl get httproutes -n $DOWNSTREAM_NAMESPACE -o yaml + kubectl get endpointslices -n $DOWNSTREAM_NAMESPACE -o yaml + kubectl get dnsendpoints -n $DOWNSTREAM_NAMESPACE -o yaml + kubectl get svc -n $DOWNSTREAM_NAMESPACE -o yaml - name: Provision HTTPRoute try: From 62137b4396c235a0af9bebc3cb691eebdd72358f Mon Sep 17 00:00:00 2001 From: Joshua Reese Date: Thu, 18 Sep 2025 11:15:45 -0500 Subject: [PATCH 2/2] Correctly pull default listener settings from the underlying gateway in the HTTPProxy reconciler, move the HTTPProxy reconciler to accumulate hostnames based on listeners and their status, instead of addresses. --- .../controller/gateway_controller_test.go | 2 - internal/controller/httpproxy_controller.go | 60 ++++++------- .../controller/httpproxy_controller_test.go | 90 +++++++++++++++---- 3 files changed, 98 insertions(+), 54 deletions(-) diff --git a/internal/controller/gateway_controller_test.go b/internal/controller/gateway_controller_test.go index 3e4f017d..aef3f403 100644 --- a/internal/controller/gateway_controller_test.go +++ b/internal/controller/gateway_controller_test.go @@ -6,7 +6,6 @@ import ( "slices" "testing" - "github.com/davecgh/go-spew/spew" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" discoveryv1 "k8s.io/api/discovery/v1" @@ -63,7 +62,6 @@ func TestEnsureDownstreamGateway(t *testing.T) { }), assert: func(t *testing.T, upstreamGateway, downstreamGateway *gatewayv1.Gateway) { - spew.Dump(downstreamGateway) if assert.Len(t, downstreamGateway.Spec.Listeners, 2) { assert.Equal(t, gatewayv1.PortNumber(DefaultHTTPPort), downstreamGateway.Spec.Listeners[0].Port) diff --git a/internal/controller/httpproxy_controller.go b/internal/controller/httpproxy_controller.go index 7b502ee0..386fd478 100644 --- a/internal/controller/httpproxy_controller.go +++ b/internal/controller/httpproxy_controller.go @@ -135,12 +135,12 @@ func (r *HTTPProxyReconciler) Reconcile(ctx context.Context, req mcreconcile.Req if !gateway.CreationTimestamp.IsZero() { defaultHTTPListener := gatewayutil.GetListenerByName(gateway.Spec.Listeners, gatewayutil.DefaultHTTPListenerName) if defaultHTTPListener != nil { - desiredResources.gateway.Spec.Listeners = append(desiredResources.gateway.Spec.Listeners, *defaultHTTPListener) + gatewayutil.SetListener(desiredResources.gateway, *defaultHTTPListener) } defaultHTTPSListener := gatewayutil.GetListenerByName(gateway.Spec.Listeners, gatewayutil.DefaultHTTPSListenerName) if defaultHTTPSListener != nil { - desiredResources.gateway.Spec.Listeners = append(desiredResources.gateway.Spec.Listeners, *defaultHTTPSListener) + gatewayutil.SetListener(desiredResources.gateway, *defaultHTTPSListener) } } @@ -276,17 +276,6 @@ func (r *HTTPProxyReconciler) reconcileHTTPProxyHostnameStatus( logger.Info("updating hostname status") var hostnames []gatewayv1.Hostname - // Copy over addresses for the TargetDomain, as they are also configured - // as hostnames. Eventually we will also copy over hostnames which have been - // successfully programmed on the HTTPProxy (custom domains need to be added, - // along with validation for them) - for _, address := range gateway.Status.Addresses { - if ptr.Deref(address.Type, gatewayv1.IPAddressType) == gatewayv1.HostnameAddressType && - strings.HasSuffix(address.Value, r.Config.Gateway.TargetDomain) { - hostnames = append(hostnames, gatewayv1.Hostname(address.Value)) - } - } - currentListenerStatus := map[gatewayv1.SectionName]gatewayv1.ListenerStatus{} for _, listener := range gateway.Status.Listeners { currentListenerStatus[listener.Name] = *listener.DeepCopy() @@ -295,31 +284,30 @@ func (r *HTTPProxyReconciler) reconcileHTTPProxyHostnameStatus( acceptedHostnames := sets.New[gatewayv1.Hostname]() nonAcceptedHostnames := sets.New[string]() inUseHostnames := sets.New[string]() - for _, hostname := range httpProxyCopy.Spec.Hostnames { - for _, listener := range gateway.Spec.Listeners { - if listener.Hostname == nil || *listener.Hostname != hostname { - continue - } - - listenerStatus, ok := currentListenerStatus[listener.Name] - if !ok { - logger.Info("listener status not found", "listener_name", listener.Name) - continue - } + for _, listener := range gateway.Spec.Listeners { + if listener.Hostname == nil { + // Should only happen shortly after creation, before the default hostnames + // are assigned + continue + } - listenerAcceptedCondition := apimeta.FindStatusCondition(listenerStatus.Conditions, string(gatewayv1.ListenerConditionAccepted)) - if listenerAcceptedCondition != nil { + listenerStatus, ok := currentListenerStatus[listener.Name] + if !ok { + logger.Info("listener status not found", "listener_name", listener.Name) + continue + } - if listenerAcceptedCondition.Status == metav1.ConditionTrue { - acceptedHostnames.Insert(hostname) - } else if listenerAcceptedCondition.Reason == networkingv1alpha.HostnameInUseReason { - inUseHostnames.Insert(string(hostname)) - } else { - nonAcceptedHostnames.Insert(string(hostname)) - } + listenerAcceptedCondition := apimeta.FindStatusCondition(listenerStatus.Conditions, string(gatewayv1.ListenerConditionAccepted)) + if listenerAcceptedCondition != nil { + if listenerAcceptedCondition.Status == metav1.ConditionTrue { + acceptedHostnames.Insert(*listener.Hostname) + } else if listenerAcceptedCondition.Reason == networkingv1alpha.HostnameInUseReason { + inUseHostnames.Insert(string(*listener.Hostname)) } else { - nonAcceptedHostnames.Insert(string(hostname)) + nonAcceptedHostnames.Insert(string(*listener.Hostname)) } + } else { + nonAcceptedHostnames.Insert(string(*listener.Hostname)) } } @@ -340,7 +328,9 @@ func (r *HTTPProxyReconciler) reconcileHTTPProxyHostnameStatus( hostnamesVerifiedCondition.Status = metav1.ConditionFalse hostnamesVerifiedCondition.Reason = networkingv1alpha.UnverifiedHostnamesPresent hostnamesVerifiedCondition.Message = fmt.Sprintf("unverified hostnames present, check status of Domains in the same namespace: %s", strings.Join(nonAcceptedHostnamesSlice, ",")) - } else if acceptedHostnames.Len() == len(httpProxyCopy.Spec.Hostnames) { + } else if acceptedHostnames.Len() == len(httpProxyCopy.Spec.Hostnames) || acceptedHostnames.Len() == len(httpProxyCopy.Spec.Hostnames)+1 { + // acceptedHostnames may contain the default listener hostname if it has + // not been removed by the user. hostnamesVerifiedCondition.Status = metav1.ConditionTrue hostnamesVerifiedCondition.Reason = networkingv1alpha.HTTPProxyReasonHostnamesVerified hostnamesVerifiedCondition.Message = "All hostnames have been accepted and programmed" diff --git a/internal/controller/httpproxy_controller_test.go b/internal/controller/httpproxy_controller_test.go index 12e6db3a..e78891c8 100644 --- a/internal/controller/httpproxy_controller_test.go +++ b/internal/controller/httpproxy_controller_test.go @@ -15,6 +15,7 @@ import ( "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" "sigs.k8s.io/controller-runtime/pkg/cluster" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" @@ -25,6 +26,7 @@ import ( networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" "go.datum.net/network-services-operator/internal/config" + gatewayutil "go.datum.net/network-services-operator/internal/util/gateway" ) //nolint:gocyclo @@ -302,7 +304,8 @@ func TestHTTPProxyReconcile(t *testing.T) { GatewayClassName: "test-gateway-class", }, Gateway: config.GatewayConfig{ - TargetDomain: "example.com", + ControllerName: gatewayv1.GatewayController("test-gateway-class"), + TargetDomain: "example.com", ListenerTLSOptions: map[gatewayv1.AnnotationKey]gatewayv1.AnnotationValue{ gatewayv1.AnnotationKey("gateway.networking.datumapis.com/certificate-issuer"): gatewayv1.AnnotationValue("test-issuer"), }, @@ -312,13 +315,14 @@ func TestHTTPProxyReconcile(t *testing.T) { type testContext struct { *testing.T reconciler *HTTPProxyReconciler + gateway *gatewayv1.Gateway } tests := []struct { name string httpProxy *networkingv1alpha.HTTPProxy existingObjects []client.Object - postCreateGatewayStatus *gatewayv1.Gateway + postCreateGatewayStatus func(*gatewayv1.Gateway) expectedError bool expectedConditions []metav1.Condition assert func(t *testContext, cl client.Client, httpProxy *networkingv1alpha.HTTPProxy) @@ -471,12 +475,12 @@ func TestHTTPProxyReconcile(t *testing.T) { { name: "address and hostname propagation", httpProxy: newHTTPProxy(), - postCreateGatewayStatus: &gatewayv1.Gateway{ - Status: gatewayv1.GatewayStatus{ + postCreateGatewayStatus: func(g *gatewayv1.Gateway) { + g.Status = gatewayv1.GatewayStatus{ Addresses: []gatewayv1.GatewayStatusAddress{ { Type: ptr.To(gatewayv1.HostnameAddressType), - Value: "test.example.com", + Value: testConfig.Gateway.GatewayDNSAddress(g), }, }, Conditions: []metav1.Condition{ @@ -491,7 +495,29 @@ func TestHTTPProxyReconcile(t *testing.T) { Reason: string(gatewayv1.GatewayReasonProgrammed), }, }, - }, + Listeners: []gatewayv1.ListenerStatus{ + { + Name: gatewayutil.DefaultHTTPListenerName, + Conditions: []metav1.Condition{ + { + Type: string(gatewayv1.GatewayConditionAccepted), + Status: metav1.ConditionTrue, + Reason: string(gatewayv1.GatewayReasonAccepted), + }, + }, + }, + { + Name: gatewayutil.DefaultHTTPSListenerName, + Conditions: []metav1.Condition{ + { + Type: string(gatewayv1.GatewayConditionAccepted), + Status: metav1.ConditionTrue, + Reason: string(gatewayv1.GatewayReasonAccepted), + }, + }, + }, + }, + } }, expectedError: false, expectedConditions: []metav1.Condition{ @@ -508,11 +534,11 @@ func TestHTTPProxyReconcile(t *testing.T) { }, assert: func(t *testContext, cl client.Client, httpProxy *networkingv1alpha.HTTPProxy) { if assert.Len(t, httpProxy.Status.Addresses, 1) { - assert.Equal(t, "test.example.com", httpProxy.Status.Addresses[0].Value) + assert.Equal(t, t.gateway.Status.Addresses[0].Value, httpProxy.Status.Addresses[0].Value) } if assert.Len(t, httpProxy.Status.Hostnames, 1) { - assert.Equal(t, gatewayv1.Hostname("test.example.com"), httpProxy.Status.Hostnames[0]) + assert.Equal(t, ptr.Deref(t.gateway.Spec.Listeners[0].Hostname, ""), httpProxy.Status.Hostnames[0]) } }, }, @@ -665,6 +691,15 @@ func TestHTTPProxyReconcile(t *testing.T) { t.Run(tt.name, func(t *testing.T) { var initialObjects []client.Object + tt.existingObjects = append(tt.existingObjects, &gatewayv1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-gateway-class", + }, + Spec: gatewayv1.GatewayClassSpec{ + ControllerName: testConfig.Gateway.ControllerName, + }, + }) + for _, obj := range tt.existingObjects { obj.SetCreationTimestamp(metav1.Now()) } @@ -676,13 +711,22 @@ func TestHTTPProxyReconcile(t *testing.T) { WithScheme(testScheme). WithObjects(initialObjects...). WithStatusSubresource(initialObjects...). - WithStatusSubresource(&gatewayv1.Gateway{}) - - if tt.postCreateGatewayStatus != nil { - fakeClientBuilder.WithStatusSubresource(tt.postCreateGatewayStatus) - } + WithStatusSubresource(&gatewayv1.Gateway{}). + WithInterceptorFuncs(interceptor.Funcs{ + Create: func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.CreateOption) error { + obj.SetUID(uuid.NewUUID()) + obj.SetCreationTimestamp(metav1.Now()) + return client.Create(ctx, obj, opts...) + }, + }) fakeClient := fakeClientBuilder.Build() + + fakeDownstreamClient := fake.NewClientBuilder(). + WithScheme(testScheme). + WithStatusSubresource(&gatewayv1.Gateway{}). + Build() + mgr := &fakeMockManager{cl: fakeClient} reconciler := &HTTPProxyReconciler{ @@ -690,6 +734,12 @@ func TestHTTPProxyReconcile(t *testing.T) { Config: testConfig, } + gatewayReconciler := &GatewayReconciler{ + mgr: mgr, + Config: testConfig, + DownstreamCluster: &fakeCluster{cl: fakeDownstreamClient}, + } + req := mcreconcile.Request{ Request: reconcile.Request{ NamespacedName: client.ObjectKeyFromObject(tt.httpProxy), @@ -708,6 +758,9 @@ func TestHTTPProxyReconcile(t *testing.T) { assert.NoError(t, err) } + _, err = gatewayReconciler.Reconcile(ctx, req) + assert.NoError(t, err, "unexpected error reconciling gateway") + var updatedProxy networkingv1alpha.HTTPProxy err = fakeClient.Get(ctx, client.ObjectKeyFromObject(tt.httpProxy), &updatedProxy) assert.NoError(t, err) @@ -715,7 +768,6 @@ func TestHTTPProxyReconcile(t *testing.T) { var gateway gatewayv1.Gateway err = fakeClient.Get(ctx, client.ObjectKeyFromObject(tt.httpProxy), &gateway) if assert.NoError(t, err) { - apimeta.SetStatusCondition(&gateway.Status.Conditions, metav1.Condition{ Type: string(gatewayv1.GatewayConditionAccepted), Status: metav1.ConditionTrue, @@ -727,10 +779,9 @@ func TestHTTPProxyReconcile(t *testing.T) { if assert.NoError(t, fakeClient.Status().Update(ctx, &gateway), "unexpected error while updating gateway status") { if tt.postCreateGatewayStatus != nil { - gateway.Status = tt.postCreateGatewayStatus.Status + tt.postCreateGatewayStatus(&gateway) err = fakeClient.Status().Update(ctx, &gateway) assert.NoError(t, err) - } _, err = reconciler.Reconcile(ctx, req) @@ -751,7 +802,12 @@ func TestHTTPProxyReconcile(t *testing.T) { } if tt.assert != nil { - tt.assert(&testContext{T: t, reconciler: reconciler}, fakeClient, &updatedProxy) + testCtx := &testContext{ + T: t, + reconciler: reconciler, + gateway: &gateway, + } + tt.assert(testCtx, fakeClient, &updatedProxy) } })