From 96bcb285d50666f13210e8e04d2966184295e175 Mon Sep 17 00:00:00 2001 From: tanjie Date: Wed, 8 Oct 2025 20:25:07 +0800 Subject: [PATCH 1/5] Add TCPRoute and UDPRoute Support for L4 Load Balancing --- internal/controller/manager.go | 14 ++ .../controller/nginx/config/stream/config.go | 6 + .../controller/nginx/config/stream_servers.go | 52 ++++- .../nginx/config/stream_servers_template.go | 4 + internal/controller/nginx/config/upstreams.go | 8 +- internal/controller/provisioner/objects.go | 42 ++-- internal/controller/state/change_processor.go | 12 + .../controller/state/change_processor_test.go | 6 +- .../state/dataplane/configuration.go | 211 ++++++++++++++++++ internal/controller/state/dataplane/types.go | 8 + .../state/graph/gateway_listener.go | 84 ++++++- .../state/graph/gateway_listener_test.go | 20 +- internal/controller/state/graph/graph.go | 4 + .../controller/state/graph/reference_grant.go | 16 ++ .../controller/state/graph/route_common.go | 48 +++- .../state/graph/route_common_test.go | 4 +- internal/controller/state/graph/tcproute.go | 125 +++++++++++ internal/controller/state/graph/udproute.go | 125 +++++++++++ .../controller/status/prepare_requests.go | 45 +++- internal/controller/status/status_setters.go | 42 ++++ internal/framework/kinds/kinds.go | 4 + 21 files changed, 846 insertions(+), 34 deletions(-) create mode 100644 internal/controller/state/graph/tcproute.go create mode 100644 internal/controller/state/graph/udproute.go diff --git a/internal/controller/manager.go b/internal/controller/manager.go index a4e9fd9cf0..9f05c46cf6 100644 --- a/internal/controller/manager.go +++ b/internal/controller/manager.go @@ -532,6 +532,18 @@ func registerControllers( controller.WithK8sPredicate(k8spredicate.GenerationChangedPredicate{}), }, }, + { + objectType: &gatewayv1alpha2.TCPRoute{}, + options: []controller.Option{ + controller.WithK8sPredicate(k8spredicate.GenerationChangedPredicate{}), + }, + }, + { + objectType: &gatewayv1alpha2.UDPRoute{}, + options: []controller.Option{ + controller.WithK8sPredicate(k8spredicate.GenerationChangedPredicate{}), + }, + }, } controllerRegCfgs = append(controllerRegCfgs, gwExpFeatures...) } @@ -758,6 +770,8 @@ func prepareFirstEventBatchPreparerArgs(cfg config.Config) ([]client.Object, []c &gatewayv1alpha3.BackendTLSPolicyList{}, &apiv1.ConfigMapList{}, &gatewayv1alpha2.TLSRouteList{}, + &gatewayv1alpha2.TCPRouteList{}, + &gatewayv1alpha2.UDPRouteList{}, ) } diff --git a/internal/controller/nginx/config/stream/config.go b/internal/controller/nginx/config/stream/config.go index ca554e18f4..0cfbc546c6 100644 --- a/internal/controller/nginx/config/stream/config.go +++ b/internal/controller/nginx/config/stream/config.go @@ -14,6 +14,12 @@ type Server struct { RewriteClientIP shared.RewriteClientIPSettings SSLPreread bool IsSocket bool + Protocol string + UDPConfig *UDPConfig +} + +type UDPConfig struct { + ProxyTimeout string } // Upstream holds all configuration for a stream upstream. diff --git a/internal/controller/nginx/config/stream_servers.go b/internal/controller/nginx/config/stream_servers.go index cbac92cc7a..1f87f4eac8 100644 --- a/internal/controller/nginx/config/stream_servers.go +++ b/internal/controller/nginx/config/stream_servers.go @@ -33,17 +33,24 @@ func (g GeneratorImpl) executeStreamServers(conf dataplane.Configuration) []exec } func createStreamServers(conf dataplane.Configuration) []stream.Server { - if len(conf.TLSPassthroughServers) == 0 { + totalServers := len(conf.TLSPassthroughServers) + len(conf.TCPServers) + len(conf.UDPServers) + if totalServers == 0 { return nil } - streamServers := make([]stream.Server, 0, len(conf.TLSPassthroughServers)*2) + streamServers := make([]stream.Server, 0, totalServers*2) portSet := make(map[int32]struct{}) upstreams := make(map[string]dataplane.Upstream) for _, u := range conf.StreamUpstreams { upstreams[u.Name] = u } + for _, u := range conf.TCPUpstreams { + upstreams[u.Name] = u + } + for _, u := range conf.UDPUpstreams { + upstreams[u.Name] = u + } for _, server := range conf.TLSPassthroughServers { if u, ok := upstreams[server.UpstreamName]; ok && server.UpstreamName != "" { @@ -77,6 +84,47 @@ func createStreamServers(conf dataplane.Configuration) []stream.Server { } streamServers = append(streamServers, streamServer) } + + // Process TCP servers + for i, server := range conf.TCPServers { + if _, inPortSet := portSet[server.Port]; inPortSet { + continue // Skip if port already in use + } + + if u, ok := upstreams[server.UpstreamName]; ok && server.UpstreamName != "" && len(u.Endpoints) > 0 { + streamServer := stream.Server{ + Listen: fmt.Sprint(server.Port), + StatusZone: fmt.Sprintf("tcp_%d", server.Port), + ProxyPass: server.UpstreamName, + } + streamServers = append(streamServers, streamServer) + portSet[server.Port] = struct{}{} + } else { + fmt.Printf("DEBUG: createStreamServers - TCP Server %d: Skipped - upstream not found or no endpoints\n", i) + } + } + + // Process UDP servers + for _, server := range conf.UDPServers { + if _, inPortSet := portSet[server.Port]; inPortSet { + continue // Skip if port already in use + } + + if u, ok := upstreams[server.UpstreamName]; ok && server.UpstreamName != "" && len(u.Endpoints) > 0 { + streamServer := stream.Server{ + Listen: fmt.Sprintf("%d udp", server.Port), + StatusZone: fmt.Sprintf("udp_%d", server.Port), + ProxyPass: server.UpstreamName, + Protocol: "udp", + UDPConfig: &stream.UDPConfig{ + ProxyTimeout: "1s", + }, + } + streamServers = append(streamServers, streamServer) + portSet[server.Port] = struct{}{} + } + } + return streamServers } diff --git a/internal/controller/nginx/config/stream_servers_template.go b/internal/controller/nginx/config/stream_servers_template.go index 9ad96e6300..9295b50610 100644 --- a/internal/controller/nginx/config/stream_servers_template.go +++ b/internal/controller/nginx/config/stream_servers_template.go @@ -35,6 +35,10 @@ server { {{- if $s.SSLPreread }} ssl_preread on; {{- end }} + + {{- if and (eq $s.Protocol "udp") $s.UDPConfig }} + proxy_timeout {{ $s.UDPConfig.ProxyTimeout }}; + {{- end }} } {{- end }} diff --git a/internal/controller/nginx/config/upstreams.go b/internal/controller/nginx/config/upstreams.go index 4b7e6bd16f..2c994110d2 100644 --- a/internal/controller/nginx/config/upstreams.go +++ b/internal/controller/nginx/config/upstreams.go @@ -69,7 +69,13 @@ func executeUpstreams(upstreams []http.Upstream) []executeResult { } func (g GeneratorImpl) executeStreamUpstreams(conf dataplane.Configuration) []executeResult { - upstreams := g.createStreamUpstreams(conf.StreamUpstreams) + // Combine all stream upstreams: TLS, TCP, and UDP + allUpstreams := make([]dataplane.Upstream, 0, len(conf.StreamUpstreams)+len(conf.TCPUpstreams)+len(conf.UDPUpstreams)) + allUpstreams = append(allUpstreams, conf.StreamUpstreams...) + allUpstreams = append(allUpstreams, conf.TCPUpstreams...) + allUpstreams = append(allUpstreams, conf.UDPUpstreams...) + + upstreams := g.createStreamUpstreams(allUpstreams) result := executeResult{ dest: streamConfigFile, diff --git a/internal/controller/provisioner/objects.go b/internal/controller/provisioner/objects.go index c9fb112e36..aea0f680ab 100644 --- a/internal/controller/provisioner/objects.go +++ b/internal/controller/provisioner/objects.go @@ -44,6 +44,11 @@ const ( defaultInitialDelaySeconds = int32(3) ) +type PortInfo struct { + Port int32 + Protocol corev1.Protocol +} + var emptyDirVolumeSource = corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}} //nolint:gocyclo // will refactor at some point @@ -147,9 +152,18 @@ func (p *NginxProvisioner) buildNginxResourceObjects( openshiftObjs = p.buildOpenshiftObjects(objectMeta) } - ports := make(map[int32]struct{}) + ports := make(map[int32]PortInfo) for _, listener := range gateway.Spec.Listeners { - ports[int32(listener.Port)] = struct{}{} + var protocol corev1.Protocol + switch listener.Protocol { + case gatewayv1.TCPProtocolType: + protocol = corev1.ProtocolTCP + case gatewayv1.UDPProtocolType: + protocol = corev1.ProtocolUDP + default: + protocol = corev1.ProtocolTCP + } + ports[int32(listener.Port)] = PortInfo{Port: int32(listener.Port), Protocol: protocol} } // Create separate copies of objectMeta for service and deployment to avoid shared map references @@ -515,7 +529,7 @@ func (p *NginxProvisioner) buildOpenshiftObjects(objectMeta metav1.ObjectMeta) [ func buildNginxService( objectMeta metav1.ObjectMeta, nProxyCfg *graph.EffectiveNginxProxy, - ports map[int32]struct{}, + ports map[int32]PortInfo, selectorLabels map[string]string, addresses []gatewayv1.GatewaySpecAddress, ) (*corev1.Service, error) { @@ -538,16 +552,17 @@ func buildNginxService( } servicePorts := make([]corev1.ServicePort, 0, len(ports)) - for port := range ports { + for _, portInfo := range ports { servicePort := corev1.ServicePort{ - Name: fmt.Sprintf("port-%d", port), - Port: port, - TargetPort: intstr.FromInt32(port), + Name: fmt.Sprintf("port-%d", portInfo.Port), + Port: portInfo.Port, + TargetPort: intstr.FromInt32(portInfo.Port), + Protocol: portInfo.Protocol, } if serviceType != corev1.ServiceTypeClusterIP { for _, nodePort := range serviceCfg.NodePorts { - if nodePort.ListenerPort == port { + if nodePort.ListenerPort == portInfo.Port { servicePort.NodePort = nodePort.Port } } @@ -625,7 +640,7 @@ func (p *NginxProvisioner) buildNginxDeployment( nProxyCfg *graph.EffectiveNginxProxy, ngxIncludesConfigMapName string, ngxAgentConfigMapName string, - ports map[int32]struct{}, + ports map[int32]PortInfo, selectorLabels map[string]string, agentTLSSecretName string, dockerSecretNames map[string]string, @@ -779,7 +794,7 @@ func (p *NginxProvisioner) buildNginxPodTemplateSpec( nProxyCfg *graph.EffectiveNginxProxy, ngxIncludesConfigMapName string, ngxAgentConfigMapName string, - ports map[int32]struct{}, + ports map[int32]PortInfo, agentTLSSecretName string, dockerSecretNames map[string]string, jwtSecretName string, @@ -788,10 +803,11 @@ func (p *NginxProvisioner) buildNginxPodTemplateSpec( dataplaneKeySecretName string, ) corev1.PodTemplateSpec { containerPorts := make([]corev1.ContainerPort, 0, len(ports)) - for port := range ports { + for _, portInfo := range ports { containerPort := corev1.ContainerPort{ - Name: fmt.Sprintf("port-%d", port), - ContainerPort: port, + Name: fmt.Sprintf("port-%d", portInfo.Port), + ContainerPort: portInfo.Port, + Protocol: portInfo.Protocol, } containerPorts = append(containerPorts, containerPort) } diff --git a/internal/controller/state/change_processor.go b/internal/controller/state/change_processor.go index f3184adde8..a16feb59b3 100644 --- a/internal/controller/state/change_processor.go +++ b/internal/controller/state/change_processor.go @@ -96,6 +96,8 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl { NginxProxies: make(map[types.NamespacedName]*ngfAPIv1alpha2.NginxProxy), GRPCRoutes: make(map[types.NamespacedName]*v1.GRPCRoute), TLSRoutes: make(map[types.NamespacedName]*v1alpha2.TLSRoute), + TCPRoutes: make(map[types.NamespacedName]*v1alpha2.TCPRoute), + UDPRoutes: make(map[types.NamespacedName]*v1alpha2.UDPRoute), NGFPolicies: make(map[graph.PolicyKey]policies.Policy), SnippetsFilters: make(map[types.NamespacedName]*ngfAPIv1alpha1.SnippetsFilter), } @@ -211,6 +213,16 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl { store: newObjectStoreMapAdapter(clusterStore.TLSRoutes), predicate: nil, }, + { + gvk: cfg.MustExtractGVK(&v1alpha2.TCPRoute{}), + store: newObjectStoreMapAdapter(clusterStore.TCPRoutes), + predicate: nil, + }, + { + gvk: cfg.MustExtractGVK(&v1alpha2.UDPRoute{}), + store: newObjectStoreMapAdapter(clusterStore.UDPRoutes), + predicate: nil, + }, { gvk: cfg.MustExtractGVK(&ngfAPIv1alpha1.SnippetsFilter{}), store: newObjectStoreMapAdapter(clusterStore.SnippetsFilters), diff --git a/internal/controller/state/change_processor_test.go b/internal/controller/state/change_processor_test.go index 2d17e6f6e9..ef90638cca 100644 --- a/internal/controller/state/change_processor_test.go +++ b/internal/controller/state/change_processor_test.go @@ -3776,7 +3776,7 @@ var _ = Describe("ChangeProcessor", func() { }, Entry( "an unsupported resource", - &v1alpha2.TCPRoute{ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "tcp"}}, + &apiv1.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "pod"}}, ), Entry( "nil resource", @@ -3794,8 +3794,8 @@ var _ = Describe("ChangeProcessor", func() { }, Entry( "an unsupported resource", - &v1alpha2.TCPRoute{}, - types.NamespacedName{Namespace: "test", Name: "tcp"}, + &apiv1.Pod{}, + types.NamespacedName{Namespace: "test", Name: "pod"}, ), Entry( "nil resource type", diff --git a/internal/controller/state/dataplane/configuration.go b/internal/controller/state/dataplane/configuration.go index 52306f4e0b..dba26dd2fd 100644 --- a/internal/controller/state/dataplane/configuration.go +++ b/internal/controller/state/dataplane/configuration.go @@ -74,6 +74,8 @@ func BuildConfiguration( HTTPServers: httpServers, SSLServers: sslServers, TLSPassthroughServers: buildPassthroughServers(gateway), + TCPServers: buildTCPServers(gateway), + UDPServers: buildUDPServers(gateway), Upstreams: upstreams, StreamUpstreams: buildStreamUpstreams( ctx, @@ -84,6 +86,8 @@ func BuildConfiguration( baseHTTPConfig.IPFamily), BackendGroups: backendGroups, SSLKeyPairs: buildSSLKeyPairs(g.ReferencedSecrets, gateway.Listeners), + TCPUpstreams: buildTCPUpstreams(ctx, gateway, serviceResolver, baseHTTPConfig.IPFamily), + UDPUpstreams: buildUDPUpstreams(ctx, gateway, serviceResolver, baseHTTPConfig.IPFamily), CertBundles: buildCertBundles( buildRefCertificateBundles(g.ReferencedSecrets, g.ReferencedCaCertConfigMaps), backendGroups, @@ -171,6 +175,79 @@ func buildPassthroughServers(gateway *graph.Gateway) []Layer4VirtualServer { return passthroughServers } +// buildTCPServers builds TCPServers from TCPRoutes attached to listeners. +func buildTCPServers(gateway *graph.Gateway) []Layer4VirtualServer { + var tcpServers []Layer4VirtualServer + + for _, l := range gateway.Listeners { + if !l.Valid || l.Source.Protocol != v1.TCPProtocolType { + continue + } + + if len(l.L4Routes) > 1 { + fmt.Printf( + "WARN: Listener %s has %d TCPRoutes, which is not supported. Skipping.", + l.Name, + len(l.L4Routes), + ) + continue + } + + for _, r := range l.L4Routes { + if !r.Valid { + continue + } + + upstreamName := r.Spec.BackendRef.ServicePortReference() + tcpServer := Layer4VirtualServer{ + Hostname: "", // TCP doesn't use hostnames + UpstreamName: upstreamName, + Port: int32(l.Source.Port), + } + + tcpServers = append(tcpServers, tcpServer) + } + } + + return tcpServers +} + +// buildUDPServers builds UDPServers from UDPRoutes attached to listeners. +func buildUDPServers(gateway *graph.Gateway) []Layer4VirtualServer { + var udpServers []Layer4VirtualServer + + for _, l := range gateway.Listeners { + if !l.Valid || l.Source.Protocol != v1.UDPProtocolType { + continue + } + + if len(l.L4Routes) > 1 { + fmt.Printf( + "WARN: Listener %s has %d UDPRoutes, which is not supported. Skipping.", + l.Name, + len(l.L4Routes), + ) + continue + } + + for _, r := range l.L4Routes { + if !r.Valid { + continue + } + + udpServer := Layer4VirtualServer{ + Hostname: "", // UDP doesn't use hostnames + UpstreamName: r.Spec.BackendRef.ServicePortReference(), + Port: int32(l.Source.Port), + } + + udpServers = append(udpServers, udpServer) + } + } + + return udpServers +} + // buildStreamUpstreams builds all stream upstreams. func buildStreamUpstreams( ctx context.Context, @@ -247,6 +324,137 @@ func buildStreamUpstreams( return upstreams } +// buildTCPUpstreams builds all TCP upstreams. +func buildTCPUpstreams( + ctx context.Context, + gateway *graph.Gateway, + serviceResolver resolver.ServiceResolver, + ipFamily IPFamilyType, +) []Upstream { + uniqueUpstreams := make(map[string]Upstream) + + for _, l := range gateway.Listeners { + if !l.Valid || l.Source.Protocol != v1.TCPProtocolType { + continue + } + + for _, route := range l.L4Routes { + if !route.Valid { + continue + } + + br := route.Spec.BackendRef + + if !br.Valid { + continue + } + + gatewayNSName := client.ObjectKeyFromObject(gateway.Source) + if _, ok := br.InvalidForGateways[gatewayNSName]; ok { + continue + } + + upstreamName := br.ServicePortReference() + + if _, exist := uniqueUpstreams[upstreamName]; exist { + continue + } + + var errMsg string + + allowedAddressType := getAllowedAddressType(ipFamily) + + eps, err := serviceResolver.Resolve(ctx, br.SvcNsName, br.ServicePort, allowedAddressType) + if err != nil { + errMsg = err.Error() + } + + uniqueUpstreams[upstreamName] = Upstream{ + Name: upstreamName, + Endpoints: eps, + ErrorMsg: errMsg, + } + } + } + + if len(uniqueUpstreams) == 0 { + return nil + } + + upstreams := make([]Upstream, 0, len(uniqueUpstreams)) + + for _, up := range uniqueUpstreams { + upstreams = append(upstreams, up) + } + + return upstreams +} + +// buildUDPUpstreams builds all UDP upstreams. +func buildUDPUpstreams( + ctx context.Context, + gateway *graph.Gateway, + serviceResolver resolver.ServiceResolver, + ipFamily IPFamilyType, +) []Upstream { + uniqueUpstreams := make(map[string]Upstream) + + for _, l := range gateway.Listeners { + if !l.Valid || l.Source.Protocol != v1.UDPProtocolType { + continue + } + + for _, route := range l.L4Routes { + if !route.Valid { + continue + } + + br := route.Spec.BackendRef + + if !br.Valid { + continue + } + + gatewayNSName := client.ObjectKeyFromObject(gateway.Source) + if _, ok := br.InvalidForGateways[gatewayNSName]; ok { + continue + } + + upstreamName := br.ServicePortReference() + + if _, exist := uniqueUpstreams[upstreamName]; exist { + continue + } + + var errMsg string + + allowedAddressType := getAllowedAddressType(ipFamily) + + eps, err := serviceResolver.Resolve(ctx, br.SvcNsName, br.ServicePort, allowedAddressType) + if err != nil { + errMsg = err.Error() + } + + uniqueUpstreams[upstreamName] = Upstream{ + Name: upstreamName, + Endpoints: eps, + ErrorMsg: errMsg, + } + } + } + + if len(uniqueUpstreams) == 0 { + return nil + } + + upstreams := make([]Upstream, 0, len(uniqueUpstreams)) + + for _, up := range uniqueUpstreams { + upstreams = append(upstreams, up) + } + return upstreams +} + // buildSSLKeyPairs builds the SSLKeyPairs from the Secrets. It will only include Secrets that are referenced by // valid listeners, so that we don't include unused Secrets in the configuration of the data plane. func buildSSLKeyPairs( @@ -435,6 +643,9 @@ func buildServers(gateway *graph.Gateway) (http, ssl []VirtualServer) { if l.Source.Protocol == v1.TLSProtocolType { continue } + if l.Source.Protocol == v1.TCPProtocolType || l.Source.Protocol == v1.UDPProtocolType { + continue + } if l.Valid { rules := rulesForProtocol[l.Source.Protocol][l.Source.Port] if rules == nil { diff --git a/internal/controller/state/dataplane/types.go b/internal/controller/state/dataplane/types.go index e593dfd6e0..a083d9c697 100644 --- a/internal/controller/state/dataplane/types.go +++ b/internal/controller/state/dataplane/types.go @@ -42,6 +42,10 @@ type Configuration struct { StreamUpstreams []Upstream // TLSPassthroughServers hold all TLSPassthroughServers TLSPassthroughServers []Layer4VirtualServer + // TCPUpstreams holds all unique TCP Upstreams + TCPUpstreams []Upstream + // UDPUpstreams holds all unique UDP Upstreams + UDPUpstreams []Upstream // BackendGroups holds all unique BackendGroups. BackendGroups []BackendGroup // MainSnippets holds all the snippets that apply to the main context. @@ -54,6 +58,10 @@ type Configuration struct { SSLServers []VirtualServer // HTTPServers holds all HTTPServers. HTTPServers []VirtualServer + // TCPServers holds all TCPServers + TCPServers []Layer4VirtualServer + // UDPServers holds all UDPServers + UDPServers []Layer4VirtualServer // Telemetry holds the Otel configuration. Telemetry Telemetry // BaseHTTPConfig holds the configuration options at the http context. diff --git a/internal/controller/state/graph/gateway_listener.go b/internal/controller/state/graph/gateway_listener.go index 3f6e69587e..97f7f003f0 100644 --- a/internal/controller/state/graph/gateway_listener.go +++ b/internal/controller/state/graph/gateway_listener.go @@ -66,7 +66,7 @@ func buildListeners( } type listenerConfiguratorFactory struct { - http, https, tls, unsupportedProtocol *listenerConfigurator + http, https, tls, tcp, udp, unsupportedProtocol *listenerConfigurator } func (f *listenerConfiguratorFactory) getConfiguratorForListener(l v1.Listener) *listenerConfigurator { @@ -77,6 +77,10 @@ func (f *listenerConfiguratorFactory) getConfiguratorForListener(l v1.Listener) return f.https case v1.TLSProtocolType: return f.tls + case v1.TCPProtocolType: + return f.tcp + case v1.UDPProtocolType: + return f.udp default: return f.unsupportedProtocol } @@ -98,7 +102,7 @@ func newListenerConfiguratorFactory( valErr := field.NotSupported( field.NewPath("protocol"), listener.Protocol, - []string{string(v1.HTTPProtocolType), string(v1.HTTPSProtocolType), string(v1.TLSProtocolType)}, + []string{string(v1.HTTPProtocolType), string(v1.HTTPSProtocolType), string(v1.TLSProtocolType), string(v1.TCPProtocolType), string(v1.UDPProtocolType)}, ) return conditions.NewListenerUnsupportedProtocol(valErr.Error()), false /* not attachable */ }, @@ -143,6 +147,26 @@ func newListenerConfiguratorFactory( }, externalReferenceResolvers: []listenerExternalReferenceResolver{}, }, + tcp: &listenerConfigurator{ + validators: []listenerValidator{ + validateListenerAllowedRouteKind, + validateListenerLabelSelector, + createTCPListenerValidator(protectedPorts), + }, + conflictResolvers: []listenerConflictResolver{ + sharedPortConflictResolver, + }, + }, + udp: &listenerConfigurator{ + validators: []listenerValidator{ + validateListenerAllowedRouteKind, + validateListenerLabelSelector, + createUDPListenerValidator(protectedPorts), + }, + conflictResolvers: []listenerConflictResolver{ + sharedPortConflictResolver, + }, + }, } } @@ -271,6 +295,14 @@ func getAndValidateListenerSupportedKinds(listener v1.Listener) ( validKinds = []v1.RouteGroupKind{ {Kind: v1.Kind(kinds.TLSRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)}, } + case v1.TCPProtocolType: + validKinds = []v1.RouteGroupKind{ + {Kind: v1.Kind(kinds.TCPRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)}, + } + case v1.UDPProtocolType: + validKinds = []v1.RouteGroupKind{ + {Kind: v1.Kind(kinds.UDPRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)}, + } } validProtocolRouteKind := func(kind v1.RouteGroupKind) bool { @@ -604,6 +636,54 @@ func haveOverlap(hostname1, hostname2 *v1.Hostname) bool { return matchesWildcard(h1, h2) } +func createTCPListenerValidator(protectedPorts ProtectedPorts) listenerValidator { + return func(listener v1.Listener) (conds []conditions.Condition, attachable bool) { + if err := validateListenerPort(listener.Port, protectedPorts); err != nil { + path := field.NewPath("port") + valErr := field.Invalid(path, listener.Port, err.Error()) + conds = append(conds, conditions.NewListenerUnsupportedValue(valErr.Error())...) + } + + if listener.TLS != nil { + path := field.NewPath("tls") + valErr := field.Forbidden(path, "tls is not supported for TCP listener") + conds = append(conds, conditions.NewListenerUnsupportedValue(valErr.Error())...) + } + + if listener.Hostname != nil { + path := field.NewPath("hostname") + valErr := field.Forbidden(path, "hostname is not supported for TCP listener") + conds = append(conds, conditions.NewListenerUnsupportedValue(valErr.Error())...) + } + + return conds, true + } +} + +func createUDPListenerValidator(protectedPorts ProtectedPorts) listenerValidator { + return func(listener v1.Listener) (conds []conditions.Condition, attachable bool) { + if err := validateListenerPort(listener.Port, protectedPorts); err != nil { + path := field.NewPath("port") + valErr := field.Invalid(path, listener.Port, err.Error()) + conds = append(conds, conditions.NewListenerUnsupportedValue(valErr.Error())...) + } + + if listener.TLS != nil { + path := field.NewPath("tls") + valErr := field.Forbidden(path, "tls is not supported for UDP listener") + conds = append(conds, conditions.NewListenerUnsupportedValue(valErr.Error())...) + } + + if listener.Hostname != nil { + path := field.NewPath("hostname") + valErr := field.Forbidden(path, "hostname is not supported for UDP listener") + conds = append(conds, conditions.NewListenerUnsupportedValue(valErr.Error())...) + } + + return conds, true + } +} + func createOverlappingTLSConfigResolver() listenerConflictResolver { listenersByPort := make(map[v1.PortNumber][]*Listener) diff --git a/internal/controller/state/graph/gateway_listener_test.go b/internal/controller/state/graph/gateway_listener_test.go index 804d4638b3..de2a28f988 100644 --- a/internal/controller/state/graph/gateway_listener_test.go +++ b/internal/controller/state/graph/gateway_listener_test.go @@ -323,8 +323,24 @@ func TestGetAndValidateListenerSupportedKinds(t *testing.T) { { protocol: v1.TCPProtocolType, expectErr: false, - name: "unsupported protocol is ignored", - expected: nil, + name: "valid TCP protocol", + expected: []v1.RouteGroupKind{ + { + Kind: kinds.TCPRoute, + Group: helpers.GetPointer[v1.Group](v1.GroupName), + }, + }, + }, + { + protocol: v1.UDPProtocolType, + expectErr: false, + name: "valid UDP protocol", + expected: []v1.RouteGroupKind{ + { + Kind: kinds.UDPRoute, + Group: helpers.GetPointer[v1.Group](v1.GroupName), + }, + }, }, { protocol: v1.HTTPProtocolType, diff --git a/internal/controller/state/graph/graph.go b/internal/controller/state/graph/graph.go index e556c798ba..692b491799 100644 --- a/internal/controller/state/graph/graph.go +++ b/internal/controller/state/graph/graph.go @@ -29,6 +29,8 @@ type ClusterState struct { Gateways map[types.NamespacedName]*gatewayv1.Gateway HTTPRoutes map[types.NamespacedName]*gatewayv1.HTTPRoute TLSRoutes map[types.NamespacedName]*v1alpha2.TLSRoute + TCPRoutes map[types.NamespacedName]*v1alpha2.TCPRoute + UDPRoutes map[types.NamespacedName]*v1alpha2.UDPRoute Services map[types.NamespacedName]*v1.Service Namespaces map[types.NamespacedName]*v1.Namespace ReferenceGrants map[types.NamespacedName]*v1beta1.ReferenceGrant @@ -253,6 +255,8 @@ func BuildGraph( l4routes := buildL4RoutesForGateways( state.TLSRoutes, + state.TCPRoutes, + state.UDPRoutes, state.Services, gws, refGrantResolver, diff --git a/internal/controller/state/graph/reference_grant.go b/internal/controller/state/graph/reference_grant.go index b827d47024..f56949a4be 100644 --- a/internal/controller/state/graph/reference_grant.go +++ b/internal/controller/state/graph/reference_grant.go @@ -89,6 +89,22 @@ func fromTLSRoute(namespace string) fromResource { } } +func fromTCPRoute(namespace string) fromResource { + return fromResource{ + group: v1.GroupName, + kind: kinds.TCPRoute, + namespace: namespace, + } +} + +func fromUDPRoute(namespace string) fromResource { + return fromResource{ + group: v1.GroupName, + kind: kinds.UDPRoute, + namespace: namespace, + } +} + // newReferenceGrantResolver creates a new referenceGrantResolver. func newReferenceGrantResolver(refGrants map[types.NamespacedName]*v1beta1.ReferenceGrant) *referenceGrantResolver { allowed := make(map[allowedReference]struct{}) diff --git a/internal/controller/state/graph/route_common.go b/internal/controller/state/graph/route_common.go index ba7e117c46..534db6c3cc 100644 --- a/internal/controller/state/graph/route_common.go +++ b/internal/controller/state/graph/route_common.go @@ -76,6 +76,10 @@ const ( RouteTypeGRPC RouteType = "grpc" // RouteTypeTLS indicates that the RouteType of the L4Route is TLS. RouteTypeTLS RouteType = "tls" + // RouteTypeTCP indicates that the RouteType of the L4Route is TCP. + RouteTypeTCP RouteType = "tcp" + // RouteTypeUDP indicates that the RouteType of the L4Route is UDP. + RouteTypeUDP RouteType = "udp" ) // L4RouteKey is the unique identifier for a L4Route. @@ -213,6 +217,8 @@ func (e routeRuleErrors) append(newErrors routeRuleErrors) routeRuleErrors { func buildL4RoutesForGateways( tlsRoutes map[types.NamespacedName]*v1alpha.TLSRoute, + tcpRoutes map[types.NamespacedName]*v1alpha.TCPRoute, + udpRoutes map[types.NamespacedName]*v1alpha.UDPRoute, services map[types.NamespacedName]*apiv1.Service, gws map[types.NamespacedName]*Gateway, resolver *referenceGrantResolver, @@ -234,6 +240,32 @@ func buildL4RoutesForGateways( } } + // Process TCP routes + for _, route := range tcpRoutes { + r := buildTCPRoute( + route, + gws, + services, + resolver.refAllowedFrom(fromTCPRoute(route.Namespace)), + ) + if r != nil { + routes[CreateRouteKeyL4(route)] = r + } + } + + // Process UDP routes + for _, route := range udpRoutes { + r := buildUDPRoute( + route, + gws, + services, + resolver.refAllowedFrom(fromUDPRoute(route.Namespace)), + ) + if r != nil { + routes[CreateRouteKeyL4(route)] = r + } + } + return routes } @@ -692,6 +724,19 @@ func tryToAttachL4RouteToListeners( return conditions.Condition{}, true } +func getL4RouteKind(route *L4Route) v1.Kind { + switch route.Source.(type) { + case *v1alpha.TLSRoute: + return v1.Kind(kinds.TLSRoute) + case *v1alpha.TCPRoute: + return v1.Kind(kinds.TCPRoute) + case *v1alpha.UDPRoute: + return v1.Kind(kinds.UDPRoute) + default: + return v1.Kind(kinds.TLSRoute) + } +} + func bindToListenerL4( l *Listener, route *L4Route, @@ -704,7 +749,8 @@ func bindToListenerL4( return false, false, false } - if !isRouteTypeAllowedByListener(l, kinds.TLSRoute) { + routeKind := getL4RouteKind(route) + if !isRouteTypeAllowedByListener(l, routeKind) { return false, false, false } diff --git a/internal/controller/state/graph/route_common_test.go b/internal/controller/state/graph/route_common_test.go index 6ded299d73..9188469724 100644 --- a/internal/controller/state/graph/route_common_test.go +++ b/internal/controller/state/graph/route_common_test.go @@ -2374,8 +2374,10 @@ func TestBuildL4RoutesForGateways_NoGateways(t *testing.T) { g.Expect(buildL4RoutesForGateways( tlsRoutes, + nil, // tcpRoutes + nil, // udpRoutes services, - nil, + nil, // gateways refGrantResolver, )).To(BeNil()) } diff --git a/internal/controller/state/graph/tcproute.go b/internal/controller/state/graph/tcproute.go new file mode 100644 index 0000000000..9cddfe86e6 --- /dev/null +++ b/internal/controller/state/graph/tcproute.go @@ -0,0 +1,125 @@ +package graph + +import ( + apiv1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/gateway-api/apis/v1alpha2" + + "github.com/nginx/nginx-gateway-fabric/internal/controller/state/conditions" +) + +func buildTCPRoute( + tcpRoute *v1alpha2.TCPRoute, + gws map[types.NamespacedName]*Gateway, + services map[types.NamespacedName]*apiv1.Service, + refGrantResolver func(resource toResource) bool, +) *L4Route { + r := &L4Route{ + Source: tcpRoute, + } + + sectionNameRefs, err := buildSectionNameRefs(tcpRoute.Spec.ParentRefs, tcpRoute.Namespace, gws) + if err != nil { + r.Valid = false + return r + } + + // route doesn't belong to any of the Gateways + if len(sectionNameRefs) == 0 { + return nil + } + r.ParentRefs = sectionNameRefs + + // TCPRoute doesn't have hostnames like TLSRoute, so we skip hostname validation + + if len(tcpRoute.Spec.Rules) != 1 || len(tcpRoute.Spec.Rules[0].BackendRefs) != 1 { + r.Valid = false + cond := conditions.NewRouteBackendRefUnsupportedValue( + "Must have exactly one Rule and BackendRef", + ) + r.Conditions = append(r.Conditions, cond) + return r + } + + br, conds := validateBackendRefTCPRoute(tcpRoute, services, r.ParentRefs, refGrantResolver) + + r.Spec.BackendRef = br + r.Valid = true + r.Attachable = true + + if len(conds) > 0 { + r.Conditions = append(r.Conditions, conds...) + } + + return r +} + +func validateBackendRefTCPRoute( + tcpRoute *v1alpha2.TCPRoute, + services map[types.NamespacedName]*apiv1.Service, + parentRefs []ParentRef, + refGrantResolver func(resource toResource) bool, +) (BackendRef, []conditions.Condition) { + // Length of BackendRefs and Rules is guaranteed to be one due to earlier check in buildTCPRoute + refPath := field.NewPath("spec").Child("rules").Index(0).Child("backendRefs").Index(0) + + ref := tcpRoute.Spec.Rules[0].BackendRefs[0] + + if valid, cond := validateBackendRef( + ref, + tcpRoute.Namespace, + refGrantResolver, + refPath, + ); !valid { + backendRef := BackendRef{ + Valid: false, + InvalidForGateways: make(map[types.NamespacedName]conditions.Condition), + } + + return backendRef, []conditions.Condition{cond} + } + + ns := tcpRoute.Namespace + if ref.Namespace != nil { + ns = string(*ref.Namespace) + } + + svcNsName := types.NamespacedName{ + Namespace: ns, + Name: string(tcpRoute.Spec.Rules[0].BackendRefs[0].Name), + } + + svcIPFamily, svcPort, err := getIPFamilyAndPortFromRef( + ref, + svcNsName, + services, + refPath, + ) + + backendRef := BackendRef{ + SvcNsName: svcNsName, + ServicePort: svcPort, + Valid: true, + InvalidForGateways: make(map[types.NamespacedName]conditions.Condition), + } + + if err != nil { + backendRef.Valid = false + + return backendRef, []conditions.Condition{conditions.NewRouteBackendRefRefBackendNotFound(err.Error())} + } + + // For TCPRoute, we don't need to validate app protocol compatibility + // as TCP is protocol-agnostic at the application layer + + var conds []conditions.Condition + for _, parentRef := range parentRefs { + if err := verifyIPFamily(parentRef.Gateway.EffectiveNginxProxy, svcIPFamily); err != nil { + backendRef.Valid = backendRef.Valid || false + backendRef.InvalidForGateways[parentRef.Gateway.NamespacedName] = conditions.NewRouteInvalidIPFamily(err.Error()) + } + } + + return backendRef, conds +} diff --git a/internal/controller/state/graph/udproute.go b/internal/controller/state/graph/udproute.go new file mode 100644 index 0000000000..c54ad538e4 --- /dev/null +++ b/internal/controller/state/graph/udproute.go @@ -0,0 +1,125 @@ +package graph + +import ( + apiv1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/gateway-api/apis/v1alpha2" + + "github.com/nginx/nginx-gateway-fabric/internal/controller/state/conditions" +) + +func buildUDPRoute( + udpRoute *v1alpha2.UDPRoute, + gws map[types.NamespacedName]*Gateway, + services map[types.NamespacedName]*apiv1.Service, + refGrantResolver func(resource toResource) bool, +) *L4Route { + r := &L4Route{ + Source: udpRoute, + } + + sectionNameRefs, err := buildSectionNameRefs(udpRoute.Spec.ParentRefs, udpRoute.Namespace, gws) + if err != nil { + r.Valid = false + return r + } + + // route doesn't belong to any of the Gateways + if len(sectionNameRefs) == 0 { + return nil + } + r.ParentRefs = sectionNameRefs + + // UDPRoute doesn't have hostnames like TLSRoute, so we skip hostname validation + + if len(udpRoute.Spec.Rules) != 1 || len(udpRoute.Spec.Rules[0].BackendRefs) != 1 { + r.Valid = false + cond := conditions.NewRouteBackendRefUnsupportedValue( + "Must have exactly one Rule and BackendRef", + ) + r.Conditions = append(r.Conditions, cond) + return r + } + + br, conds := validateBackendRefUDPRoute(udpRoute, services, r.ParentRefs, refGrantResolver) + + r.Spec.BackendRef = br + r.Valid = true + r.Attachable = true + + if len(conds) > 0 { + r.Conditions = append(r.Conditions, conds...) + } + + return r +} + +func validateBackendRefUDPRoute( + udpRoute *v1alpha2.UDPRoute, + services map[types.NamespacedName]*apiv1.Service, + parentRefs []ParentRef, + refGrantResolver func(resource toResource) bool, +) (BackendRef, []conditions.Condition) { + // Length of BackendRefs and Rules is guaranteed to be one due to earlier check in buildUDPRoute + refPath := field.NewPath("spec").Child("rules").Index(0).Child("backendRefs").Index(0) + + ref := udpRoute.Spec.Rules[0].BackendRefs[0] + + if valid, cond := validateBackendRef( + ref, + udpRoute.Namespace, + refGrantResolver, + refPath, + ); !valid { + backendRef := BackendRef{ + Valid: false, + InvalidForGateways: make(map[types.NamespacedName]conditions.Condition), + } + + return backendRef, []conditions.Condition{cond} + } + + ns := udpRoute.Namespace + if ref.Namespace != nil { + ns = string(*ref.Namespace) + } + + svcNsName := types.NamespacedName{ + Namespace: ns, + Name: string(udpRoute.Spec.Rules[0].BackendRefs[0].Name), + } + + svcIPFamily, svcPort, err := getIPFamilyAndPortFromRef( + ref, + svcNsName, + services, + refPath, + ) + + backendRef := BackendRef{ + SvcNsName: svcNsName, + ServicePort: svcPort, + Valid: true, + InvalidForGateways: make(map[types.NamespacedName]conditions.Condition), + } + + if err != nil { + backendRef.Valid = false + + return backendRef, []conditions.Condition{conditions.NewRouteBackendRefRefBackendNotFound(err.Error())} + } + + // For UDPRoute, we don't need to validate app protocol compatibility + // as UDP is protocol-agnostic at the application layer + + var conds []conditions.Condition + for _, parentRef := range parentRefs { + if err := verifyIPFamily(parentRef.Gateway.EffectiveNginxProxy, svcIPFamily); err != nil { + backendRef.Valid = backendRef.Valid || false + backendRef.InvalidForGateways[parentRef.Gateway.NamespacedName] = conditions.NewRouteInvalidIPFamily(err.Error()) + } + } + + return backendRef, conds +} diff --git a/internal/controller/status/prepare_requests.go b/internal/controller/status/prepare_requests.go index 87e3b441cc..83a7a9604f 100644 --- a/internal/controller/status/prepare_requests.go +++ b/internal/controller/status/prepare_requests.go @@ -41,17 +41,44 @@ func PrepareRouteRequests( r.Source.GetGeneration(), ) - status := v1alpha2.TLSRouteStatus{ - RouteStatus: routeStatus, - } + switch r.Source.(type) { + case *v1alpha2.TLSRoute: + status := v1alpha2.TLSRouteStatus{ + RouteStatus: routeStatus, + } - req := UpdateRequest{ - NsName: routeKey.NamespacedName, - ResourceType: &v1alpha2.TLSRoute{}, - Setter: newTLSRouteStatusSetter(status, gatewayCtlrName), - } + req := UpdateRequest{ + NsName: routeKey.NamespacedName, + ResourceType: &v1alpha2.TLSRoute{}, + Setter: newTLSRouteStatusSetter(status, gatewayCtlrName), + } + reqs = append(reqs, req) - reqs = append(reqs, req) + case *v1alpha2.TCPRoute: + status := v1alpha2.TCPRouteStatus{ + RouteStatus: routeStatus, + } + req := UpdateRequest{ + NsName: routeKey.NamespacedName, + ResourceType: &v1alpha2.TCPRoute{}, + Setter: newTCPRouteStatusSetter(status, gatewayCtlrName), + } + reqs = append(reqs, req) + + case *v1alpha2.UDPRoute: + status := v1alpha2.UDPRouteStatus{ + RouteStatus: routeStatus, + } + req := UpdateRequest{ + NsName: routeKey.NamespacedName, + ResourceType: &v1alpha2.UDPRoute{}, + Setter: newUDPRouteStatusSetter(status, gatewayCtlrName), + } + reqs = append(reqs, req) + + default: + continue + } } for routeKey, r := range routes { diff --git a/internal/controller/status/status_setters.go b/internal/controller/status/status_setters.go index c4fcc7c128..fb96fa97b0 100644 --- a/internal/controller/status/status_setters.go +++ b/internal/controller/status/status_setters.go @@ -142,6 +142,48 @@ func newGRPCRouteStatusSetter(status gatewayv1.GRPCRouteStatus, gatewayCtlrName } } +func newTCPRouteStatusSetter(status v1alpha2.TCPRouteStatus, gatewayCtlrName string) Setter { + return func(object client.Object) (wasSet bool) { + tr := helpers.MustCastObject[*v1alpha2.TCPRoute](object) + + // keep all the parent statuses that belong to other controllers + for _, os := range tr.Status.Parents { + if string(os.ControllerName) != gatewayCtlrName { + status.Parents = append(status.Parents, os) + } + } + + if routeStatusEqual(gatewayCtlrName, tr.Status.Parents, status.Parents) { + return false + } + + tr.Status = status + + return true + } +} + +func newUDPRouteStatusSetter(status v1alpha2.UDPRouteStatus, gatewayCtlrName string) Setter { + return func(object client.Object) (wasSet bool) { + ur := helpers.MustCastObject[*v1alpha2.UDPRoute](object) + + // keep all the parent statuses that belong to other controllers + for _, os := range ur.Status.Parents { + if string(os.ControllerName) != gatewayCtlrName { + status.Parents = append(status.Parents, os) + } + } + + if routeStatusEqual(gatewayCtlrName, ur.Status.Parents, status.Parents) { + return false + } + + ur.Status = status + + return true + } +} + func routeStatusEqual(gatewayCtlrName string, prevParents, curParents []gatewayv1.RouteParentStatus) bool { // Since other controllers may update HTTPRoute status we can't assume anything about the order of the statuses, // and we have to ignore statuses written by other controllers when checking for equality. diff --git a/internal/framework/kinds/kinds.go b/internal/framework/kinds/kinds.go index 35ca8e2b00..eda1320a5f 100644 --- a/internal/framework/kinds/kinds.go +++ b/internal/framework/kinds/kinds.go @@ -21,6 +21,10 @@ const ( GRPCRoute = "GRPCRoute" // TLSRoute is the TLSRoute kind. TLSRoute = "TLSRoute" + // TCPRoute is the TCPRoute kind. + TCPRoute = "TCPRoute" + // UDPRoute is the UDPRoute kind. + UDPRoute = "UDPRoute" // BackendTLSPolicy is the BackendTLSPolicy kind. BackendTLSPolicy = "BackendTLSPolicy" ) From f25a0dd67fe215be3c5e145a4f2d51b266529592 Mon Sep 17 00:00:00 2001 From: tanjie Date: Mon, 13 Oct 2025 14:03:43 +0800 Subject: [PATCH 2/5] fix: add tcproute and udproute to clusterrole --- charts/nginx-gateway-fabric/templates/clusterrole.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/charts/nginx-gateway-fabric/templates/clusterrole.yaml b/charts/nginx-gateway-fabric/templates/clusterrole.yaml index 57c92e4692..59a186b081 100644 --- a/charts/nginx-gateway-fabric/templates/clusterrole.yaml +++ b/charts/nginx-gateway-fabric/templates/clusterrole.yaml @@ -97,6 +97,8 @@ rules: {{- if .Values.nginxGateway.gwAPIExperimentalFeatures.enable }} - backendtlspolicies - tlsroutes + - tcproutes + - udproutes {{- end }} verbs: - list @@ -111,6 +113,8 @@ rules: {{- if .Values.nginxGateway.gwAPIExperimentalFeatures.enable }} - backendtlspolicies/status - tlsroutes/status + - tcproutes/status + - udproutes/status {{- end }} verbs: - update From 617f0384159fda7177a615bfc782aaf3d09a7486 Mon Sep 17 00:00:00 2001 From: tanjie Date: Mon, 13 Oct 2025 14:54:09 +0800 Subject: [PATCH 3/5] Feat: add weight to the backendRef of tcproute and udproute, and optimize the code --- .../controller/nginx/config/stream/config.go | 1 + .../controller/nginx/config/stream_servers.go | 36 ++- internal/controller/nginx/config/upstreams.go | 6 + .../nginx/config/upstreams_template.go | 2 +- .../nginx/modules/package-lock.json | 2 - .../state/dataplane/configuration.go | 277 ++++++++++-------- .../state/graph/gateway_listener.go | 40 +-- .../controller/state/graph/route_common.go | 215 +++++++++++++- internal/controller/state/graph/service.go | 20 +- internal/controller/state/graph/tcproute.go | 119 +------- internal/controller/state/graph/udproute.go | 119 +------- .../controller/state/resolver/resolver.go | 2 + 12 files changed, 451 insertions(+), 388 deletions(-) diff --git a/internal/controller/nginx/config/stream/config.go b/internal/controller/nginx/config/stream/config.go index 0cfbc546c6..c73934948c 100644 --- a/internal/controller/nginx/config/stream/config.go +++ b/internal/controller/nginx/config/stream/config.go @@ -34,6 +34,7 @@ type Upstream struct { type UpstreamServer struct { Address string Resolve bool + Weight int32 // Weight for load balancing, default 1 } // ServerConfig holds configuration for a stream server and IP family to be used by NGINX. diff --git a/internal/controller/nginx/config/stream_servers.go b/internal/controller/nginx/config/stream_servers.go index 1f87f4eac8..1900f1a6e8 100644 --- a/internal/controller/nginx/config/stream_servers.go +++ b/internal/controller/nginx/config/stream_servers.go @@ -4,6 +4,7 @@ import ( "fmt" gotemplate "text/template" + "github.com/go-logr/logr" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/shared" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/stream" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/dataplane" @@ -13,7 +14,7 @@ import ( var streamServersTemplate = gotemplate.Must(gotemplate.New("streamServers").Parse(streamServersTemplateText)) func (g GeneratorImpl) executeStreamServers(conf dataplane.Configuration) []executeResult { - streamServers := createStreamServers(conf) + streamServers := createStreamServers(g.logger, conf) streamServerConfig := stream.ServerConfig{ Servers: streamServers, @@ -32,7 +33,7 @@ func (g GeneratorImpl) executeStreamServers(conf dataplane.Configuration) []exec } } -func createStreamServers(conf dataplane.Configuration) []stream.Server { +func createStreamServers(logger logr.Logger, conf dataplane.Configuration) []stream.Server { totalServers := len(conf.TLSPassthroughServers) + len(conf.TCPServers) + len(conf.UDPServers) if totalServers == 0 { return nil @@ -85,8 +86,23 @@ func createStreamServers(conf dataplane.Configuration) []stream.Server { streamServers = append(streamServers, streamServer) } + // Process Layer4 servers (TCP and UDP) + processLayer4Servers(logger, conf.TCPServers, conf.UDPServers, upstreams, portSet, &streamServers) + + return streamServers +} + +// processLayer4Servers processes TCP and UDP servers to create stream servers. +func processLayer4Servers( + logger logr.Logger, + tcpServers []dataplane.Layer4VirtualServer, + udpServers []dataplane.Layer4VirtualServer, + upstreams map[string]dataplane.Upstream, + portSet map[int32]struct{}, + streamServers *[]stream.Server, +) { // Process TCP servers - for i, server := range conf.TCPServers { + for i, server := range tcpServers { if _, inPortSet := portSet[server.Port]; inPortSet { continue // Skip if port already in use } @@ -97,15 +113,19 @@ func createStreamServers(conf dataplane.Configuration) []stream.Server { StatusZone: fmt.Sprintf("tcp_%d", server.Port), ProxyPass: server.UpstreamName, } - streamServers = append(streamServers, streamServer) + *streamServers = append(*streamServers, streamServer) portSet[server.Port] = struct{}{} } else { - fmt.Printf("DEBUG: createStreamServers - TCP Server %d: Skipped - upstream not found or no endpoints\n", i) + logger.V(1).Info("TCP Server skipped - upstream not found or no endpoints", + "serverIndex", i, + "port", server.Port, + "upstreamName", server.UpstreamName, + ) } } // Process UDP servers - for _, server := range conf.UDPServers { + for _, server := range udpServers { if _, inPortSet := portSet[server.Port]; inPortSet { continue // Skip if port already in use } @@ -120,12 +140,10 @@ func createStreamServers(conf dataplane.Configuration) []stream.Server { ProxyTimeout: "1s", }, } - streamServers = append(streamServers, streamServer) + *streamServers = append(*streamServers, streamServer) portSet[server.Port] = struct{}{} } } - - return streamServers } func getRewriteClientIPSettingsForStream( diff --git a/internal/controller/nginx/config/upstreams.go b/internal/controller/nginx/config/upstreams.go index 2c994110d2..f1132ace06 100644 --- a/internal/controller/nginx/config/upstreams.go +++ b/internal/controller/nginx/config/upstreams.go @@ -115,9 +115,15 @@ func (g GeneratorImpl) createStreamUpstream(up dataplane.Upstream) stream.Upstre if ep.IPv6 { format = "[%s]:%d" } + // Default weight to 1 if not specified + weight := ep.Weight + if weight == 0 { + weight = 1 + } upstreamServers[idx] = stream.UpstreamServer{ Address: fmt.Sprintf(format, ep.Address, ep.Port), Resolve: ep.Resolve, + Weight: weight, } } diff --git a/internal/controller/nginx/config/upstreams_template.go b/internal/controller/nginx/config/upstreams_template.go index 15e9b0c1fc..4a2ce5f04a 100644 --- a/internal/controller/nginx/config/upstreams_template.go +++ b/internal/controller/nginx/config/upstreams_template.go @@ -49,7 +49,7 @@ upstream {{ $u.Name }} { state {{ $u.StateFile }}; {{- else }} {{ range $server := $u.Servers }} - server {{ $server.Address }}{{ if $server.Resolve }} resolve{{ end }}; + server {{ $server.Address }}{{ if ne $server.Weight 0 }}{{ if ne $server.Weight 1 }} weight={{ $server.Weight }}{{ end }}{{ end }}{{ if $server.Resolve }} resolve{{ end }}; {{- end }} {{- end }} } diff --git a/internal/controller/nginx/modules/package-lock.json b/internal/controller/nginx/modules/package-lock.json index 27f383234e..5fb7a865c9 100644 --- a/internal/controller/nginx/modules/package-lock.json +++ b/internal/controller/nginx/modules/package-lock.json @@ -1696,7 +1696,6 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -2181,7 +2180,6 @@ "integrity": "sha512-LUCP5ev3GURDysTWiP47wRRUpLKMOfPh+yKTx3kVIEiu5KOMeqzpnYNsKyOoVrULivR8tLcks4+lga33Whn90A==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@types/chai": "^5.2.2", "@vitest/expect": "3.2.4", diff --git a/internal/controller/state/dataplane/configuration.go b/internal/controller/state/dataplane/configuration.go index dba26dd2fd..4ac04b34aa 100644 --- a/internal/controller/state/dataplane/configuration.go +++ b/internal/controller/state/dataplane/configuration.go @@ -74,8 +74,8 @@ func BuildConfiguration( HTTPServers: httpServers, SSLServers: sslServers, TLSPassthroughServers: buildPassthroughServers(gateway), - TCPServers: buildTCPServers(gateway), - UDPServers: buildUDPServers(gateway), + TCPServers: buildTCPServers(logger, gateway), + UDPServers: buildUDPServers(logger, gateway), Upstreams: upstreams, StreamUpstreams: buildStreamUpstreams( ctx, @@ -84,10 +84,10 @@ func BuildConfiguration( serviceResolver, g.ReferencedServices, baseHTTPConfig.IPFamily), + TCPUpstreams: buildTCPUpstreams(ctx, logger, gateway, serviceResolver, baseHTTPConfig.IPFamily), + UDPUpstreams: buildUDPUpstreams(ctx, logger, gateway, serviceResolver, baseHTTPConfig.IPFamily), BackendGroups: backendGroups, SSLKeyPairs: buildSSLKeyPairs(g.ReferencedSecrets, gateway.Listeners), - TCPUpstreams: buildTCPUpstreams(ctx, gateway, serviceResolver, baseHTTPConfig.IPFamily), - UDPUpstreams: buildUDPUpstreams(ctx, gateway, serviceResolver, baseHTTPConfig.IPFamily), CertBundles: buildCertBundles( buildRefCertificateBundles(g.ReferencedSecrets, g.ReferencedCaCertConfigMaps), backendGroups, @@ -175,20 +175,21 @@ func buildPassthroughServers(gateway *graph.Gateway) []Layer4VirtualServer { return passthroughServers } -// buildTCPServers builds TCPServers from TCPRoutes attached to listeners. -func buildTCPServers(gateway *graph.Gateway) []Layer4VirtualServer { - var tcpServers []Layer4VirtualServer +// buildL4Servers builds Layer4 servers (TCP or UDP) from routes attached to listeners. +func buildL4Servers(logger logr.Logger, gateway *graph.Gateway, protocol v1.ProtocolType) []Layer4VirtualServer { + var servers []Layer4VirtualServer + protocolName := string(protocol) for _, l := range gateway.Listeners { - if !l.Valid || l.Source.Protocol != v1.TCPProtocolType { + if !l.Valid || l.Source.Protocol != protocol { continue } if len(l.L4Routes) > 1 { - fmt.Printf( - "WARN: Listener %s has %d TCPRoutes, which is not supported. Skipping.", - l.Name, - len(l.L4Routes), + logger.V(1).Info("Listener has multiple routes, which is not supported, skipping", + "listener", l.Name, + "protocol", protocolName, + "routeCount", len(l.L4Routes), ) continue } @@ -198,54 +199,53 @@ func buildTCPServers(gateway *graph.Gateway) []Layer4VirtualServer { continue } - upstreamName := r.Spec.BackendRef.ServicePortReference() - tcpServer := Layer4VirtualServer{ - Hostname: "", // TCP doesn't use hostnames - UpstreamName: upstreamName, - Port: int32(l.Source.Port), - } - - tcpServers = append(tcpServers, tcpServer) - } - } - - return tcpServers -} - -// buildUDPServers builds UDPServers from UDPRoutes attached to listeners. -func buildUDPServers(gateway *graph.Gateway) []Layer4VirtualServer { - var udpServers []Layer4VirtualServer - - for _, l := range gateway.Listeners { - if !l.Valid || l.Source.Protocol != v1.UDPProtocolType { - continue - } - - if len(l.L4Routes) > 1 { - fmt.Printf( - "WARN: Listener %s has %d UDPRoutes, which is not supported. Skipping.", - l.Name, - len(l.L4Routes), - ) - continue - } + // Use helper method to get all backend references + backendRefs := r.Spec.GetBackendRefs() - for _, r := range l.L4Routes { - if !r.Valid { + if len(backendRefs) == 0 { + logger.V(1).Info("Route has no valid backend references, skipping", + "route", r.Source.GetName(), + "protocol", protocolName, + ) continue } - udpServer := Layer4VirtualServer{ - Hostname: "", // UDP doesn't use hostnames - UpstreamName: r.Spec.BackendRef.ServicePortReference(), + // For single backend, use direct upstream name + // For multiple backends, we'll create a combined upstream name based on the route + var upstreamName string + if len(backendRefs) == 1 { + upstreamName = backendRefs[0].ServicePortReference() + } else { + // For multiple backends, create a group upstream name + // Format: protocol_namespace_routename + upstreamName = fmt.Sprintf("%s_%s_%s", + protocolName, + r.Source.GetNamespace(), + r.Source.GetName(), + ) + } + + server := Layer4VirtualServer{ + Hostname: "", // Layer4 doesn't use hostnames + UpstreamName: upstreamName, Port: int32(l.Source.Port), } - udpServers = append(udpServers, udpServer) + servers = append(servers, server) } } - return udpServers + return servers +} + +// buildTCPServers builds TCPServers from TCPRoutes attached to listeners. +func buildTCPServers(logger logr.Logger, gateway *graph.Gateway) []Layer4VirtualServer { + return buildL4Servers(logger, gateway, v1.TCPProtocolType) +} + +// buildUDPServers builds UDPServers from UDPRoutes attached to listeners. +func buildUDPServers(logger logr.Logger, gateway *graph.Gateway) []Layer4VirtualServer { + return buildL4Servers(logger, gateway, v1.UDPProtocolType) } // buildStreamUpstreams builds all stream upstreams. @@ -324,17 +324,22 @@ func buildStreamUpstreams( return upstreams } -// buildTCPUpstreams builds all TCP upstreams. -func buildTCPUpstreams( +// buildL4Upstreams builds Layer4 upstreams (TCP or UDP) from routes attached to listeners. +func buildL4Upstreams( ctx context.Context, + logger logr.Logger, gateway *graph.Gateway, serviceResolver resolver.ServiceResolver, ipFamily IPFamilyType, + protocol v1.ProtocolType, ) []Upstream { uniqueUpstreams := make(map[string]Upstream) + protocolName := string(protocol) + gatewayNSName := client.ObjectKeyFromObject(gateway.Source) + for _, l := range gateway.Listeners { - if !l.Valid || l.Source.Protocol != v1.TCPProtocolType { + if !l.Valid || l.Source.Protocol != protocol { continue } @@ -343,36 +348,93 @@ func buildTCPUpstreams( continue } - br := route.Spec.BackendRef + // Use helper method to get all backend references + backendRefs := route.Spec.GetBackendRefs() - if !br.Valid { + if len(backendRefs) == 0 { continue } - gatewayNSName := client.ObjectKeyFromObject(gateway.Source) - if _, ok := br.InvalidForGateways[gatewayNSName]; ok { - continue - } + // For single backend: create one upstream with service name + // For multiple backends: create individual upstreams + one combined upstream with weighted endpoints + if len(backendRefs) == 1 { + br := backendRefs[0] + if !br.Valid { + continue + } - upstreamName := br.ServicePortReference() + if _, ok := br.InvalidForGateways[gatewayNSName]; ok { + continue + } - if _, exist := uniqueUpstreams[upstreamName]; exist { - continue - } + upstreamName := br.ServicePortReference() - var errMsg string + if _, exist := uniqueUpstreams[upstreamName]; exist { + continue + } - allowedAddressType := getAllowedAddressType(ipFamily) + var errMsg string + allowedAddressType := getAllowedAddressType(ipFamily) - eps, err := serviceResolver.Resolve(ctx, br.SvcNsName, br.ServicePort, allowedAddressType) - if err != nil { - errMsg = err.Error() - } + eps, err := serviceResolver.Resolve(ctx, logger, br.SvcNsName, br.ServicePort, allowedAddressType) + if err != nil { + errMsg = err.Error() + } - uniqueUpstreams[upstreamName] = Upstream{ - Name: upstreamName, - Endpoints: eps, - ErrorMsg: errMsg, + uniqueUpstreams[upstreamName] = Upstream{ + Name: upstreamName, + Endpoints: eps, + ErrorMsg: errMsg, + } + } else { + // Multiple backends: create a combined upstream with weighted endpoints + combinedUpstreamName := fmt.Sprintf("%s_%s_%s", + protocolName, + route.Source.GetNamespace(), + route.Source.GetName(), + ) + + if _, exist := uniqueUpstreams[combinedUpstreamName]; exist { + continue + } + + var combinedEndpoints []resolver.Endpoint + var errMsgs []string + allowedAddressType := getAllowedAddressType(ipFamily) + + // Collect endpoints from all backends with their weights + for _, br := range backendRefs { + if !br.Valid { + continue + } + + if _, ok := br.InvalidForGateways[gatewayNSName]; ok { + continue + } + + eps, err := serviceResolver.Resolve(ctx, logger, br.SvcNsName, br.ServicePort, allowedAddressType) + if err != nil { + errMsgs = append(errMsgs, err.Error()) + continue + } + + // Add weight to each endpoint + for _, ep := range eps { + ep.Weight = br.Weight + combinedEndpoints = append(combinedEndpoints, ep) + } + } + + var errMsg string + if len(errMsgs) > 0 { + errMsg = fmt.Sprintf("some backends failed: %v", errMsgs) + } + + uniqueUpstreams[combinedUpstreamName] = Upstream{ + Name: combinedUpstreamName, + Endpoints: combinedEndpoints, + ErrorMsg: errMsg, + } } } } @@ -390,69 +452,26 @@ func buildTCPUpstreams( return upstreams } +// buildTCPUpstreams builds all TCP upstreams. +func buildTCPUpstreams( + ctx context.Context, + logger logr.Logger, + gateway *graph.Gateway, + serviceResolver resolver.ServiceResolver, + ipFamily IPFamilyType, +) []Upstream { + return buildL4Upstreams(ctx, logger, gateway, serviceResolver, ipFamily, v1.TCPProtocolType) +} + // buildUDPUpstreams builds all UDP upstreams. func buildUDPUpstreams( ctx context.Context, + logger logr.Logger, gateway *graph.Gateway, serviceResolver resolver.ServiceResolver, ipFamily IPFamilyType, ) []Upstream { - uniqueUpstreams := make(map[string]Upstream) - - for _, l := range gateway.Listeners { - if !l.Valid || l.Source.Protocol != v1.UDPProtocolType { - continue - } - - for _, route := range l.L4Routes { - if !route.Valid { - continue - } - - br := route.Spec.BackendRef - - if !br.Valid { - continue - } - - gatewayNSName := client.ObjectKeyFromObject(gateway.Source) - if _, ok := br.InvalidForGateways[gatewayNSName]; ok { - continue - } - - upstreamName := br.ServicePortReference() - - if _, exist := uniqueUpstreams[upstreamName]; exist { - continue - } - - var errMsg string - - allowedAddressType := getAllowedAddressType(ipFamily) - - eps, err := serviceResolver.Resolve(ctx, br.SvcNsName, br.ServicePort, allowedAddressType) - if err != nil { - errMsg = err.Error() - } - - uniqueUpstreams[upstreamName] = Upstream{ - Name: upstreamName, - Endpoints: eps, - ErrorMsg: errMsg, - } - } - } - - if len(uniqueUpstreams) == 0 { - return nil - } - - upstreams := make([]Upstream, 0, len(uniqueUpstreams)) - - for _, up := range uniqueUpstreams { - upstreams = append(upstreams, up) - } - return upstreams + return buildL4Upstreams(ctx, logger, gateway, serviceResolver, ipFamily, v1.UDPProtocolType) } // buildSSLKeyPairs builds the SSLKeyPairs from the Secrets. It will only include Secrets that are referenced by diff --git a/internal/controller/state/graph/gateway_listener.go b/internal/controller/state/graph/gateway_listener.go index 97f7f003f0..083e95fcfc 100644 --- a/internal/controller/state/graph/gateway_listener.go +++ b/internal/controller/state/graph/gateway_listener.go @@ -478,6 +478,11 @@ func createHTTPSListenerValidator(protectedPorts ProtectedPorts) listenerValidat } } +// isL4Protocol checks if the protocol is a Layer 4 protocol (TCP or UDP) +func isL4Protocol(protocol v1.ProtocolType) bool { + return protocol == v1.TCPProtocolType || protocol == v1.UDPProtocolType +} + func createPortConflictResolver() listenerConflictResolver { const ( secureProtocolGroup int = 0 @@ -487,6 +492,8 @@ func createPortConflictResolver() listenerConflictResolver { v1.TLSProtocolType: secureProtocolGroup, v1.HTTPProtocolType: insecureProtocolGroup, v1.HTTPSProtocolType: secureProtocolGroup, + v1.TCPProtocolType: insecureProtocolGroup, + v1.UDPProtocolType: insecureProtocolGroup, } conflictedPorts := make(map[v1.PortNumber]bool) portProtocolOwner := make(map[v1.PortNumber]int) @@ -536,6 +543,7 @@ func createPortConflictResolver() listenerConflictResolver { foundConflict := false for _, listener := range listenersByPort[port] { if listener.Source.Protocol != l.Source.Protocol && + !isL4Protocol(listener.Source.Protocol) && !isL4Protocol(l.Source.Protocol) && haveOverlap(l.Source.Hostname, listener.Source.Hostname) { listener.Valid = false conflictedConds := conditions.NewListenerHostnameConflict(fmt.Sprintf(formatHostname, port)) @@ -636,7 +644,7 @@ func haveOverlap(hostname1, hostname2 *v1.Hostname) bool { return matchesWildcard(h1, h2) } -func createTCPListenerValidator(protectedPorts ProtectedPorts) listenerValidator { +func createL4ListenerValidator(protocol v1.ProtocolType, protectedPorts ProtectedPorts) listenerValidator { return func(listener v1.Listener) (conds []conditions.Condition, attachable bool) { if err := validateListenerPort(listener.Port, protectedPorts); err != nil { path := field.NewPath("port") @@ -646,13 +654,13 @@ func createTCPListenerValidator(protectedPorts ProtectedPorts) listenerValidator if listener.TLS != nil { path := field.NewPath("tls") - valErr := field.Forbidden(path, "tls is not supported for TCP listener") + valErr := field.Forbidden(path, fmt.Sprintf("tls is not supported for %s listener", protocol)) conds = append(conds, conditions.NewListenerUnsupportedValue(valErr.Error())...) } if listener.Hostname != nil { path := field.NewPath("hostname") - valErr := field.Forbidden(path, "hostname is not supported for TCP listener") + valErr := field.Forbidden(path, fmt.Sprintf("hostname is not supported for %s listener", protocol)) conds = append(conds, conditions.NewListenerUnsupportedValue(valErr.Error())...) } @@ -660,28 +668,12 @@ func createTCPListenerValidator(protectedPorts ProtectedPorts) listenerValidator } } -func createUDPListenerValidator(protectedPorts ProtectedPorts) listenerValidator { - return func(listener v1.Listener) (conds []conditions.Condition, attachable bool) { - if err := validateListenerPort(listener.Port, protectedPorts); err != nil { - path := field.NewPath("port") - valErr := field.Invalid(path, listener.Port, err.Error()) - conds = append(conds, conditions.NewListenerUnsupportedValue(valErr.Error())...) - } - - if listener.TLS != nil { - path := field.NewPath("tls") - valErr := field.Forbidden(path, "tls is not supported for UDP listener") - conds = append(conds, conditions.NewListenerUnsupportedValue(valErr.Error())...) - } - - if listener.Hostname != nil { - path := field.NewPath("hostname") - valErr := field.Forbidden(path, "hostname is not supported for UDP listener") - conds = append(conds, conditions.NewListenerUnsupportedValue(valErr.Error())...) - } +func createTCPListenerValidator(protectedPorts ProtectedPorts) listenerValidator { + return createL4ListenerValidator(v1.TCPProtocolType, protectedPorts) +} - return conds, true - } +func createUDPListenerValidator(protectedPorts ProtectedPorts) listenerValidator { + return createL4ListenerValidator(v1.UDPProtocolType, protectedPorts) } func createOverlappingTLSConfigResolver() listenerConflictResolver { diff --git a/internal/controller/state/graph/route_common.go b/internal/controller/state/graph/route_common.go index 534db6c3cc..e9621b5445 100644 --- a/internal/controller/state/graph/route_common.go +++ b/internal/controller/state/graph/route_common.go @@ -114,9 +114,38 @@ type L4Route struct { type L4RouteSpec struct { // Hostnames defines a set of hostnames used to select a Route used to process the request. Hostnames []v1.Hostname - // FIXME (sarthyparty): change to slice of BackendRef, as for now we are only supporting one BackendRef. - // We will eventually support multiple BackendRef https://github.com/nginx/nginx-gateway-fabric/issues/2184 + // BackendRef is the single backend reference (deprecated, kept for TLSRoute compatibility). + // For TCPRoute/UDPRoute, use BackendRefs instead. BackendRef BackendRef + // BackendRefs is a list of backend references for TCPRoute/UDPRoute multi-backend support. + // Each BackendRef can have a weight for load balancing. + BackendRefs []BackendRef +} + +// GetBackendRefs returns all backend references for this L4Route. +// For TCPRoute/UDPRoute with multiple backends, it returns BackendRefs. +// For TLSRoute or single backend routes, it returns a slice containing BackendRef. +// Note: Returns BackendRef even if not Valid, as we need to check InvalidForGateways. +func (spec *L4RouteSpec) GetBackendRefs() []BackendRef { + if len(spec.BackendRefs) > 0 { + return spec.BackendRefs + } + // Check if BackendRef has been set (even if invalid) + // An unset BackendRef will have empty SvcNsName and InvalidForGateways + if spec.BackendRef.SvcNsName.Name != "" || len(spec.BackendRef.InvalidForGateways) > 0 { + return []BackendRef{spec.BackendRef} + } + return []BackendRef{} +} + +// GetPrimaryBackendRef returns the primary backend reference. +// For multi-backend routes, it returns the first BackendRef. +// For single backend routes, it returns BackendRef. +func (spec *L4RouteSpec) GetPrimaryBackendRef() BackendRef { + if len(spec.BackendRefs) > 0 { + return spec.BackendRefs[0] + } + return spec.BackendRef } // L7Route is the generic type for the layer 7 routes, HTTPRoute and GRPCRoute. @@ -627,14 +656,12 @@ func bindL4RouteToListeners( attachment, attachableListeners := validateParentRef(ref, gw) + // If there are validation failures (e.g., invalid gateway, wrong namespace), + // we should not try to attach if len(attachment.FailedConditions) > 0 { continue } - if cond, ok := route.Spec.BackendRef.InvalidForGateways[gwNsName]; ok { - attachment.FailedConditions = append(attachment.FailedConditions, cond) - } - // Try to attach Route to all matching listeners cond, attached := tryToAttachL4RouteToListeners( @@ -654,6 +681,16 @@ func bindL4RouteToListeners( } attachment.Attached = true + + // Check if any BackendRef is invalid for this Gateway and add conditions + // This is done after attachment so the route can still be attached even with invalid backends + backendRefs := route.Spec.GetBackendRefs() + for _, br := range backendRefs { + if cond, ok := br.InvalidForGateways[gwNsName]; ok { + attachment.FailedConditions = append(attachment.FailedConditions, cond) + break // Only need to add the condition once + } + } } } @@ -1184,3 +1221,169 @@ func routeKeyForKind(kind v1.Kind, nsname types.NamespacedName) RouteKey { return key } + +// l4RouteConfig holds the configuration needed to build an L4Route generically. +type l4RouteConfig struct { + source client.Object + namespace string + parentRefs []v1.ParentReference + rules []l4RouteRule + routeType string // "TCP" or "UDP" + refGrantResolver func(resource toResource) bool +} + +// l4RouteRule represents a rule in TCPRoute or UDPRoute. +type l4RouteRule struct { + backendRefs []v1alpha.BackendRef +} + +// buildGenericL4Route is a generic function to build L4Route for both TCP and UDP routes. +// This eliminates code duplication between buildTCPRoute and buildUDPRoute. +func buildGenericL4Route( + config l4RouteConfig, + gws map[types.NamespacedName]*Gateway, + services map[types.NamespacedName]*apiv1.Service, +) *L4Route { + r := &L4Route{ + Source: config.source, + } + + sectionNameRefs, err := buildSectionNameRefs(config.parentRefs, config.namespace, gws) + if err != nil { + r.Valid = false + return r + } + + // route doesn't belong to any of the Gateways + if len(sectionNameRefs) == 0 { + return nil + } + r.ParentRefs = sectionNameRefs + + // TCPRoute/UDPRoute don't have hostnames like TLSRoute, so we skip hostname validation + + // Validate that we have at least one rule + if len(config.rules) == 0 { + r.Valid = false + cond := conditions.NewRouteBackendRefUnsupportedValue( + "Must have at least one Rule", + ) + r.Conditions = append(r.Conditions, cond) + return r + } + + // Process all BackendRefs from all rules + var allBackendRefs []BackendRef + var allConditions []conditions.Condition + + for ruleIdx, rule := range config.rules { + if len(rule.backendRefs) == 0 { + continue + } + + for refIdx, ref := range rule.backendRefs { + br, conds := validateBackendRefL4RouteMulti( + config.namespace, config.routeType, ref, services, r.ParentRefs, + config.refGrantResolver, ruleIdx, refIdx, + ) + allBackendRefs = append(allBackendRefs, br) + allConditions = append(allConditions, conds...) + } + } + + // Set BackendRefs for multi-backend support + r.Spec.BackendRefs = allBackendRefs + r.Valid = true + r.Attachable = true + + if len(allConditions) > 0 { + r.Conditions = append(r.Conditions, allConditions...) + } + + return r +} + +// validateBackendRefL4RouteMulti is a generic function to validate BackendRef for both TCP and UDP routes. +// This eliminates code duplication between validateBackendRefTCPRouteMulti and validateBackendRefUDPRouteMulti. +func validateBackendRefL4RouteMulti( + namespace string, + routeType string, // "TCP" or "UDP" + ref v1alpha.BackendRef, + services map[types.NamespacedName]*apiv1.Service, + parentRefs []ParentRef, + refGrantResolver func(resource toResource) bool, + ruleIdx int, + refIdx int, +) (BackendRef, []conditions.Condition) { + refPath := field.NewPath("spec").Child("rules").Index(ruleIdx).Child("backendRefs").Index(refIdx) + + if valid, cond := validateBackendRef( + ref, + namespace, + refGrantResolver, + refPath, + ); !valid { + backendRef := BackendRef{ + Valid: false, + } + + return backendRef, []conditions.Condition{cond} + } + + ns := namespace + if ref.Namespace != nil { + ns = string(*ref.Namespace) + } + + svcNsName := types.NamespacedName{ + Namespace: ns, + Name: string(ref.Name), + } + + svcIPFamily, svcPort, err := getIPFamilyAndPortFromRef( + ref, + svcNsName, + services, + refPath, + ) + + // Handle weight - default to 1 if not specified + weight := int32(1) + if ref.Weight != nil { + if validateWeight(*ref.Weight) != nil { + weight = 0 // Invalid weight set to 0 (no traffic) + } else { + weight = *ref.Weight + } + } + + backendRef := BackendRef{ + SvcNsName: svcNsName, + ServicePort: svcPort, + Weight: weight, + Valid: true, + } + + if err != nil { + backendRef.Valid = false + + return backendRef, []conditions.Condition{conditions.NewRouteBackendRefRefBackendNotFound(err.Error())} + } + + // For TCP/UDPRoute, we don't need to validate app protocol compatibility + // as TCP/UDP are protocol-agnostic at the application layer + + var conds []conditions.Condition + for _, parentRef := range parentRefs { + if err := verifyIPFamily(parentRef.Gateway.EffectiveNginxProxy, svcIPFamily); err != nil { + backendRef.Valid = backendRef.Valid || false + // Initialize the map only when needed + if backendRef.InvalidForGateways == nil { + backendRef.InvalidForGateways = make(map[types.NamespacedName]conditions.Condition) + } + backendRef.InvalidForGateways[parentRef.Gateway.NamespacedName] = conditions.NewRouteInvalidIPFamily(err.Error()) + } + } + + return backendRef, conds +} diff --git a/internal/controller/state/graph/service.go b/internal/controller/state/graph/service.go index d43ecacfd8..31ca8ff9c4 100644 --- a/internal/controller/state/graph/service.go +++ b/internal/controller/state/graph/service.go @@ -95,20 +95,26 @@ func routeBelongsToGateway(refs []ParentRef, gwKey types.NamespacedName) bool { return false } -// addServiceFromL4Route adds a service from an L4 route to the referenced services map. +// addServiceFromL4Route adds services from an L4 route to the referenced services map. +// Supports multiple BackendRefs for TCPRoute/UDPRoute. func addServiceFromL4Route( route *L4Route, gwNsName types.NamespacedName, referencedServices map[types.NamespacedName]*ReferencedService, services map[types.NamespacedName]*v1.Service, ) { - svcNsName := route.Spec.BackendRef.SvcNsName - if svcNsName == (types.NamespacedName{}) { - return - } + // Use helper method to get all backend references + backendRefs := route.Spec.GetBackendRefs() + + for _, br := range backendRefs { + svcNsName := br.SvcNsName + if svcNsName == (types.NamespacedName{}) { + continue + } - ensureReferencedService(svcNsName, referencedServices, services) - referencedServices[svcNsName].GatewayNsNames[gwNsName] = struct{}{} + ensureReferencedService(svcNsName, referencedServices, services) + referencedServices[svcNsName].GatewayNsNames[gwNsName] = struct{}{} + } } // addServicesFromL7RouteRules adds services from L7 route rules to the referenced services map. diff --git a/internal/controller/state/graph/tcproute.go b/internal/controller/state/graph/tcproute.go index 9cddfe86e6..95c4784be2 100644 --- a/internal/controller/state/graph/tcproute.go +++ b/internal/controller/state/graph/tcproute.go @@ -3,10 +3,7 @@ package graph import ( apiv1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/gateway-api/apis/v1alpha2" - - "github.com/nginx/nginx-gateway-fabric/internal/controller/state/conditions" ) func buildTCPRoute( @@ -15,111 +12,23 @@ func buildTCPRoute( services map[types.NamespacedName]*apiv1.Service, refGrantResolver func(resource toResource) bool, ) *L4Route { - r := &L4Route{ - Source: tcpRoute, - } - - sectionNameRefs, err := buildSectionNameRefs(tcpRoute.Spec.ParentRefs, tcpRoute.Namespace, gws) - if err != nil { - r.Valid = false - return r - } - - // route doesn't belong to any of the Gateways - if len(sectionNameRefs) == 0 { - return nil - } - r.ParentRefs = sectionNameRefs - - // TCPRoute doesn't have hostnames like TLSRoute, so we skip hostname validation - - if len(tcpRoute.Spec.Rules) != 1 || len(tcpRoute.Spec.Rules[0].BackendRefs) != 1 { - r.Valid = false - cond := conditions.NewRouteBackendRefUnsupportedValue( - "Must have exactly one Rule and BackendRef", - ) - r.Conditions = append(r.Conditions, cond) - return r - } - - br, conds := validateBackendRefTCPRoute(tcpRoute, services, r.ParentRefs, refGrantResolver) - - r.Spec.BackendRef = br - r.Valid = true - r.Attachable = true - - if len(conds) > 0 { - r.Conditions = append(r.Conditions, conds...) - } - - return r -} - -func validateBackendRefTCPRoute( - tcpRoute *v1alpha2.TCPRoute, - services map[types.NamespacedName]*apiv1.Service, - parentRefs []ParentRef, - refGrantResolver func(resource toResource) bool, -) (BackendRef, []conditions.Condition) { - // Length of BackendRefs and Rules is guaranteed to be one due to earlier check in buildTCPRoute - refPath := field.NewPath("spec").Child("rules").Index(0).Child("backendRefs").Index(0) - - ref := tcpRoute.Spec.Rules[0].BackendRefs[0] - - if valid, cond := validateBackendRef( - ref, - tcpRoute.Namespace, - refGrantResolver, - refPath, - ); !valid { - backendRef := BackendRef{ - Valid: false, - InvalidForGateways: make(map[types.NamespacedName]conditions.Condition), + // Convert TCPRoute rules to generic l4RouteRule format + rules := make([]l4RouteRule, len(tcpRoute.Spec.Rules)) + for i, rule := range tcpRoute.Spec.Rules { + rules[i] = l4RouteRule{ + backendRefs: rule.BackendRefs, } - - return backendRef, []conditions.Condition{cond} - } - - ns := tcpRoute.Namespace - if ref.Namespace != nil { - ns = string(*ref.Namespace) - } - - svcNsName := types.NamespacedName{ - Namespace: ns, - Name: string(tcpRoute.Spec.Rules[0].BackendRefs[0].Name), } - svcIPFamily, svcPort, err := getIPFamilyAndPortFromRef( - ref, - svcNsName, - services, - refPath, - ) - - backendRef := BackendRef{ - SvcNsName: svcNsName, - ServicePort: svcPort, - Valid: true, - InvalidForGateways: make(map[types.NamespacedName]conditions.Condition), - } - - if err != nil { - backendRef.Valid = false - - return backendRef, []conditions.Condition{conditions.NewRouteBackendRefRefBackendNotFound(err.Error())} - } - - // For TCPRoute, we don't need to validate app protocol compatibility - // as TCP is protocol-agnostic at the application layer - - var conds []conditions.Condition - for _, parentRef := range parentRefs { - if err := verifyIPFamily(parentRef.Gateway.EffectiveNginxProxy, svcIPFamily); err != nil { - backendRef.Valid = backendRef.Valid || false - backendRef.InvalidForGateways[parentRef.Gateway.NamespacedName] = conditions.NewRouteInvalidIPFamily(err.Error()) - } + // Use the generic L4 route builder + config := l4RouteConfig{ + source: tcpRoute, + namespace: tcpRoute.Namespace, + parentRefs: tcpRoute.Spec.ParentRefs, + rules: rules, + routeType: "TCP", + refGrantResolver: refGrantResolver, } - return backendRef, conds + return buildGenericL4Route(config, gws, services) } diff --git a/internal/controller/state/graph/udproute.go b/internal/controller/state/graph/udproute.go index c54ad538e4..58a4de27a4 100644 --- a/internal/controller/state/graph/udproute.go +++ b/internal/controller/state/graph/udproute.go @@ -3,10 +3,7 @@ package graph import ( apiv1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/gateway-api/apis/v1alpha2" - - "github.com/nginx/nginx-gateway-fabric/internal/controller/state/conditions" ) func buildUDPRoute( @@ -15,111 +12,23 @@ func buildUDPRoute( services map[types.NamespacedName]*apiv1.Service, refGrantResolver func(resource toResource) bool, ) *L4Route { - r := &L4Route{ - Source: udpRoute, - } - - sectionNameRefs, err := buildSectionNameRefs(udpRoute.Spec.ParentRefs, udpRoute.Namespace, gws) - if err != nil { - r.Valid = false - return r - } - - // route doesn't belong to any of the Gateways - if len(sectionNameRefs) == 0 { - return nil - } - r.ParentRefs = sectionNameRefs - - // UDPRoute doesn't have hostnames like TLSRoute, so we skip hostname validation - - if len(udpRoute.Spec.Rules) != 1 || len(udpRoute.Spec.Rules[0].BackendRefs) != 1 { - r.Valid = false - cond := conditions.NewRouteBackendRefUnsupportedValue( - "Must have exactly one Rule and BackendRef", - ) - r.Conditions = append(r.Conditions, cond) - return r - } - - br, conds := validateBackendRefUDPRoute(udpRoute, services, r.ParentRefs, refGrantResolver) - - r.Spec.BackendRef = br - r.Valid = true - r.Attachable = true - - if len(conds) > 0 { - r.Conditions = append(r.Conditions, conds...) - } - - return r -} - -func validateBackendRefUDPRoute( - udpRoute *v1alpha2.UDPRoute, - services map[types.NamespacedName]*apiv1.Service, - parentRefs []ParentRef, - refGrantResolver func(resource toResource) bool, -) (BackendRef, []conditions.Condition) { - // Length of BackendRefs and Rules is guaranteed to be one due to earlier check in buildUDPRoute - refPath := field.NewPath("spec").Child("rules").Index(0).Child("backendRefs").Index(0) - - ref := udpRoute.Spec.Rules[0].BackendRefs[0] - - if valid, cond := validateBackendRef( - ref, - udpRoute.Namespace, - refGrantResolver, - refPath, - ); !valid { - backendRef := BackendRef{ - Valid: false, - InvalidForGateways: make(map[types.NamespacedName]conditions.Condition), + // Convert UDPRoute rules to generic l4RouteRule format + rules := make([]l4RouteRule, len(udpRoute.Spec.Rules)) + for i, rule := range udpRoute.Spec.Rules { + rules[i] = l4RouteRule{ + backendRefs: rule.BackendRefs, } - - return backendRef, []conditions.Condition{cond} - } - - ns := udpRoute.Namespace - if ref.Namespace != nil { - ns = string(*ref.Namespace) - } - - svcNsName := types.NamespacedName{ - Namespace: ns, - Name: string(udpRoute.Spec.Rules[0].BackendRefs[0].Name), } - svcIPFamily, svcPort, err := getIPFamilyAndPortFromRef( - ref, - svcNsName, - services, - refPath, - ) - - backendRef := BackendRef{ - SvcNsName: svcNsName, - ServicePort: svcPort, - Valid: true, - InvalidForGateways: make(map[types.NamespacedName]conditions.Condition), - } - - if err != nil { - backendRef.Valid = false - - return backendRef, []conditions.Condition{conditions.NewRouteBackendRefRefBackendNotFound(err.Error())} - } - - // For UDPRoute, we don't need to validate app protocol compatibility - // as UDP is protocol-agnostic at the application layer - - var conds []conditions.Condition - for _, parentRef := range parentRefs { - if err := verifyIPFamily(parentRef.Gateway.EffectiveNginxProxy, svcIPFamily); err != nil { - backendRef.Valid = backendRef.Valid || false - backendRef.InvalidForGateways[parentRef.Gateway.NamespacedName] = conditions.NewRouteInvalidIPFamily(err.Error()) - } + // Use the generic L4 route builder + config := l4RouteConfig{ + source: udpRoute, + namespace: udpRoute.Namespace, + parentRefs: udpRoute.Spec.ParentRefs, + rules: rules, + routeType: "UDP", + refGrantResolver: refGrantResolver, } - return backendRef, conds + return buildGenericL4Route(config, gws, services) } diff --git a/internal/controller/state/resolver/resolver.go b/internal/controller/state/resolver/resolver.go index 253b254db0..5e4e66e483 100644 --- a/internal/controller/state/resolver/resolver.go +++ b/internal/controller/state/resolver/resolver.go @@ -40,6 +40,8 @@ type Endpoint struct { IPv6 bool // Resolve is true if the address is a DNS name that needs to be resolved (e.g., for ExternalName services). Resolve bool + // Weight is the weight for load balancing, used for TCPRoute/UDPRoute multi-backend support. + Weight int32 } // ServiceResolverImpl implements ServiceResolver. From 772b4f46334e91e2ed3b5e9d96989d585899bbbd Mon Sep 17 00:00:00 2001 From: tanjie Date: Mon, 13 Oct 2025 14:54:36 +0800 Subject: [PATCH 4/5] add test code for tcproute and udproute --- .../nginx/config/stream_servers_test.go | 7 +- .../controller/state/graph/gateway_test.go | 12 +- .../controller/state/graph/tcproute_test.go | 650 +++++++++++++++++ .../controller/state/graph/udproute_test.go | 654 ++++++++++++++++++ 4 files changed, 1317 insertions(+), 6 deletions(-) create mode 100644 internal/controller/state/graph/tcproute_test.go create mode 100644 internal/controller/state/graph/udproute_test.go diff --git a/internal/controller/nginx/config/stream_servers_test.go b/internal/controller/nginx/config/stream_servers_test.go index e603fe1fed..827ada8c4e 100644 --- a/internal/controller/nginx/config/stream_servers_test.go +++ b/internal/controller/nginx/config/stream_servers_test.go @@ -5,6 +5,7 @@ import ( "strings" "testing" + "github.com/go-logr/logr" . "github.com/onsi/gomega" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/stream" @@ -174,7 +175,8 @@ func TestCreateStreamServers(t *testing.T) { }, } - streamServers := createStreamServers(conf) + logger := logr.Discard() + streamServers := createStreamServers(logger, conf) g := NewWithT(t) @@ -405,7 +407,8 @@ func TestCreateStreamServersWithNone(t *testing.T) { TLSPassthroughServers: nil, } - streamServers := createStreamServers(conf) + logger := logr.Discard() + streamServers := createStreamServers(logger, conf) g := NewWithT(t) diff --git a/internal/controller/state/graph/gateway_test.go b/internal/controller/state/graph/gateway_test.go index e3978df1a9..314c92c999 100644 --- a/internal/controller/state/graph/gateway_test.go +++ b/internal/controller/state/graph/gateway_test.go @@ -246,6 +246,7 @@ func TestBuildGateway(t *testing.T) { foo443TLSListener := createTLSListener("foo-443-tls", "foo.example.com", 443) // invalid listeners + // TCP listener with hostname is invalid because TCP doesn't support hostname invalidProtocolListener := createTCPListener("invalid-protocol", "bar.example.com", 80) invalidPortListener := createHTTPListener("invalid-port", "invalid-port", 0) invalidProtectedPortListener := createHTTPListener("invalid-protected-port", "invalid-protected-port", 9113) @@ -760,10 +761,13 @@ func TestBuildGateway(t *testing.T) { GatewayName: client.ObjectKeyFromObject(getLastCreatedGateway()), Source: invalidProtocolListener, Valid: false, - Attachable: false, - Conditions: conditions.NewListenerUnsupportedProtocol( - `protocol: Unsupported value: "TCP": supported values: "HTTP", "HTTPS", "TLS"`, + Attachable: true, + Conditions: conditions.NewListenerUnsupportedValue( + `hostname: Forbidden: hostname is not supported for TCP listener`, ), + SupportedKinds: []v1.RouteGroupKind{ + {Kind: kinds.TCPRoute, Group: helpers.GetPointer[v1.Group](v1.GroupName)}, + }, Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, }, @@ -775,7 +779,7 @@ func TestBuildGateway(t *testing.T) { Valid: true, }, }, - name: "invalid listener protocol", + name: "invalid listener protocol", // Actually tests TCP listener with invalid hostname }, { gateway: createGateway( diff --git a/internal/controller/state/graph/tcproute_test.go b/internal/controller/state/graph/tcproute_test.go new file mode 100644 index 0000000000..f52516c290 --- /dev/null +++ b/internal/controller/state/graph/tcproute_test.go @@ -0,0 +1,650 @@ +package graph + +import ( + "testing" + + . "github.com/onsi/gomega" + apiv1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + "sigs.k8s.io/gateway-api/apis/v1alpha2" + + ngfAPI "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha2" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/conditions" + "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers" +) + +func createTCPRoute( + rules []v1alpha2.TCPRouteRule, + parentRefs []gatewayv1.ParentReference, +) *v1alpha2.TCPRoute { + return &v1alpha2.TCPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "tcpr", + }, + Spec: v1alpha2.TCPRouteSpec{ + CommonRouteSpec: gatewayv1.CommonRouteSpec{ + ParentRefs: parentRefs, + }, + Rules: rules, + }, + } +} + +func TestBuildTCPRoute(t *testing.T) { + t.Parallel() + + parentRef := gatewayv1.ParentReference{ + Namespace: helpers.GetPointer[gatewayv1.Namespace]("test"), + Name: "gateway", + SectionName: helpers.GetPointer[gatewayv1.SectionName]("l1"), + } + + createGateway := func() *Gateway { + return &Gateway{ + Source: &gatewayv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "gateway", + }, + }, + Valid: true, + } + } + + modGateway := func(gw *Gateway, mod func(*Gateway) *Gateway) *Gateway { + return mod(gw) + } + + parentRefGraph := ParentRef{ + SectionName: helpers.GetPointer[gatewayv1.SectionName]("l1"), + Gateway: &ParentRefGateway{ + NamespacedName: types.NamespacedName{ + Namespace: "test", + Name: "gateway", + }, + }, + } + + // Test cases for invalid TCPRoutes + duplicateParentRefsTCPR := createTCPRoute( + nil, + []gatewayv1.ParentReference{ + parentRef, + parentRef, + }, + ) + + noParentRefsTCPR := createTCPRoute( + nil, + []gatewayv1.ParentReference{}, + ) + + noRulesTCPR := createTCPRoute( + nil, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + backendRefDNETCPR := createTCPRoute( + []v1alpha2.TCPRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "svc-does-not-exist", + Port: helpers.GetPointer[gatewayv1.PortNumber](80), + }, + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + wrongBackendRefGroupTCPR := createTCPRoute( + []v1alpha2.TCPRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "svc1", + Port: helpers.GetPointer[gatewayv1.PortNumber](80), + Group: helpers.GetPointer[gatewayv1.Group]("wrong"), + }, + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + wrongBackendRefKindTCPR := createTCPRoute( + []v1alpha2.TCPRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "svc1", + Port: helpers.GetPointer[gatewayv1.PortNumber](80), + Kind: helpers.GetPointer[gatewayv1.Kind]("not-service"), + }, + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + diffNsBackendRefTCPR := createTCPRoute( + []v1alpha2.TCPRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "svc1", + Port: helpers.GetPointer[gatewayv1.PortNumber](80), + Namespace: helpers.GetPointer[gatewayv1.Namespace]("diff"), + }, + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + portNilBackendRefTCPR := createTCPRoute( + []v1alpha2.TCPRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "svc1", + }, + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + ipFamilyMismatchTCPR := createTCPRoute( + []v1alpha2.TCPRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "svc1", + Port: helpers.GetPointer[gatewayv1.PortNumber](80), + }, + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + // Valid TCPRoute with single backend + validSingleBackendTCPR := createTCPRoute( + []v1alpha2.TCPRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "svc1", + Port: helpers.GetPointer[gatewayv1.PortNumber](80), + }, + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + // Valid TCPRoute with multiple backends (weighted) + validMultiBackendTCPR := createTCPRoute( + []v1alpha2.TCPRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "svc1", + Port: helpers.GetPointer[gatewayv1.PortNumber](80), + }, + Weight: helpers.GetPointer[int32](80), + }, + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "svc2", + Port: helpers.GetPointer[gatewayv1.PortNumber](80), + }, + Weight: helpers.GetPointer[int32](20), + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + // TCPRoute with multiple rules + multiRuleTCPR := createTCPRoute( + []v1alpha2.TCPRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "svc1", + Port: helpers.GetPointer[gatewayv1.PortNumber](80), + }, + }, + }, + }, + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "svc2", + Port: helpers.GetPointer[gatewayv1.PortNumber](80), + }, + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + createSvc := func(name string) *apiv1.Service { + return &apiv1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: name, + }, + Spec: apiv1.ServiceSpec{ + ClusterIP: "10.0.0.1", + Ports: []apiv1.ServicePort{ + { + Port: 80, + }, + }, + }, + } + } + + createModSvc := func(mod func(*apiv1.Service) *apiv1.Service) *apiv1.Service { + return mod(createSvc("svc1")) + } + + tests := []struct { + name string + route *v1alpha2.TCPRoute + gateways map[types.NamespacedName]*Gateway + services map[types.NamespacedName]*apiv1.Service + expected *L4Route + }{ + { + name: "duplicate parent refs", + route: duplicateParentRefsTCPR, + gateways: map[types.NamespacedName]*Gateway{ + {Namespace: "test", Name: "gateway"}: createGateway(), + }, + expected: &L4Route{ + Source: duplicateParentRefsTCPR, + Valid: false, + }, + }, + { + name: "no parent refs", + route: noParentRefsTCPR, + gateways: map[types.NamespacedName]*Gateway{ + {Namespace: "test", Name: "gateway"}: createGateway(), + }, + expected: nil, + }, + { + name: "no rules", + route: noRulesTCPR, + gateways: map[types.NamespacedName]*Gateway{ + {Namespace: "test", Name: "gateway"}: createGateway(), + }, + expected: &L4Route{ + Source: noRulesTCPR, + Valid: false, + Attachable: false, + ParentRefs: []ParentRef{parentRefGraph}, + Conditions: []conditions.Condition{ + conditions.NewRouteBackendRefUnsupportedValue("Must have at least one Rule"), + }, + }, + }, + { + name: "backend ref does not exist", + route: backendRefDNETCPR, + gateways: map[types.NamespacedName]*Gateway{ + {Namespace: "test", Name: "gateway"}: createGateway(), + }, + services: map[types.NamespacedName]*apiv1.Service{}, + expected: &L4Route{ + Source: backendRefDNETCPR, + Valid: true, + Attachable: true, + ParentRefs: []ParentRef{parentRefGraph}, + Spec: L4RouteSpec{ + BackendRefs: []BackendRef{ + { + SvcNsName: types.NamespacedName{Namespace: "test", Name: "svc-does-not-exist"}, + Weight: 1, + Valid: false, + }, + }, + }, + Conditions: []conditions.Condition{ + conditions.NewRouteBackendRefRefBackendNotFound( + "spec.rules[0].backendRefs[0].name: Not found: \"svc-does-not-exist\"", + ), + }, + }, + }, + { + name: "wrong backend ref group", + route: wrongBackendRefGroupTCPR, + gateways: map[types.NamespacedName]*Gateway{ + {Namespace: "test", Name: "gateway"}: createGateway(), + }, + services: map[types.NamespacedName]*apiv1.Service{ + {Namespace: "test", Name: "svc1"}: createSvc("svc1"), + }, + expected: &L4Route{ + Source: wrongBackendRefGroupTCPR, + Valid: true, + Attachable: true, + ParentRefs: []ParentRef{parentRefGraph}, + Spec: L4RouteSpec{ + BackendRefs: []BackendRef{ + { + Valid: false, + }, + }, + }, + Conditions: []conditions.Condition{ + conditions.NewRouteBackendRefInvalidKind( + `spec.rules[0].backendRefs[0].group: Unsupported value: "wrong": supported values: "core", ""`, + ), + }, + }, + }, + { + name: "wrong backend ref kind", + route: wrongBackendRefKindTCPR, + gateways: map[types.NamespacedName]*Gateway{ + {Namespace: "test", Name: "gateway"}: createGateway(), + }, + services: map[types.NamespacedName]*apiv1.Service{ + {Namespace: "test", Name: "svc1"}: createSvc("svc1"), + }, + expected: &L4Route{ + Source: wrongBackendRefKindTCPR, + Valid: true, + Attachable: true, + ParentRefs: []ParentRef{parentRefGraph}, + Spec: L4RouteSpec{ + BackendRefs: []BackendRef{ + { + Valid: false, + }, + }, + }, + Conditions: []conditions.Condition{ + conditions.NewRouteBackendRefInvalidKind( + `spec.rules[0].backendRefs[0].kind: Unsupported value: "not-service": supported values: "Service"`, + ), + }, + }, + }, + { + name: "different namespace backend ref without reference grant", + route: diffNsBackendRefTCPR, + gateways: map[types.NamespacedName]*Gateway{ + {Namespace: "test", Name: "gateway"}: createGateway(), + }, + services: map[types.NamespacedName]*apiv1.Service{ + {Namespace: "diff", Name: "svc1"}: createModSvc(func(svc *apiv1.Service) *apiv1.Service { + svc.Namespace = "diff" + return svc + }), + }, + expected: &L4Route{ + Source: diffNsBackendRefTCPR, + Valid: true, + Attachable: true, + ParentRefs: []ParentRef{parentRefGraph}, + Spec: L4RouteSpec{ + BackendRefs: []BackendRef{ + { + Valid: false, + }, + }, + }, + Conditions: []conditions.Condition{ + conditions.NewRouteBackendRefRefNotPermitted( + `spec.rules[0].backendRefs[0].namespace: Forbidden: Backend ref to Service diff/svc1 not permitted by any ReferenceGrant`, + ), + }, + }, + }, + { + name: "port nil backend ref", + route: portNilBackendRefTCPR, + gateways: map[types.NamespacedName]*Gateway{ + {Namespace: "test", Name: "gateway"}: createGateway(), + }, + services: map[types.NamespacedName]*apiv1.Service{ + {Namespace: "test", Name: "svc1"}: createSvc("svc1"), + }, + expected: &L4Route{ + Source: portNilBackendRefTCPR, + Valid: true, + Attachable: true, + ParentRefs: []ParentRef{parentRefGraph}, + Spec: L4RouteSpec{ + BackendRefs: []BackendRef{ + { + Valid: false, + }, + }, + }, + Conditions: []conditions.Condition{ + conditions.NewRouteBackendRefUnsupportedValue( + "spec.rules[0].backendRefs[0].port: Required value: port cannot be nil", + ), + }, + }, + }, + { + name: "IP family mismatch", + route: ipFamilyMismatchTCPR, + gateways: map[types.NamespacedName]*Gateway{ + {Namespace: "test", Name: "gateway"}: modGateway(createGateway(), func(gw *Gateway) *Gateway { + gw.EffectiveNginxProxy = &EffectiveNginxProxy{IPFamily: helpers.GetPointer(ngfAPI.IPv6)} + return gw + }), + }, + services: map[types.NamespacedName]*apiv1.Service{ + {Namespace: "test", Name: "svc1"}: createModSvc(func(svc *apiv1.Service) *apiv1.Service { + svc.Spec.IPFamilies = []apiv1.IPFamily{apiv1.IPv4Protocol} + return svc + }), + }, + expected: &L4Route{ + Source: ipFamilyMismatchTCPR, + Valid: true, + Attachable: true, + ParentRefs: []ParentRef{ + { + SectionName: helpers.GetPointer[gatewayv1.SectionName]("l1"), + Gateway: &ParentRefGateway{ + NamespacedName: types.NamespacedName{ + Namespace: "test", + Name: "gateway", + }, + EffectiveNginxProxy: &EffectiveNginxProxy{IPFamily: helpers.GetPointer(ngfAPI.IPv6)}, + }, + }, + }, + Spec: L4RouteSpec{ + BackendRefs: []BackendRef{ + { + SvcNsName: types.NamespacedName{Namespace: "test", Name: "svc1"}, + ServicePort: apiv1.ServicePort{ + Port: 80, + }, + Weight: 1, + Valid: true, + InvalidForGateways: map[types.NamespacedName]conditions.Condition{ + {Namespace: "test", Name: "gateway"}: conditions.NewRouteInvalidIPFamily( + "service configured with IPv4 family but NginxProxy is configured with IPv6", + ), + }, + }, + }, + }, + }, + }, + { + name: "valid single backend", + route: validSingleBackendTCPR, + gateways: map[types.NamespacedName]*Gateway{ + {Namespace: "test", Name: "gateway"}: createGateway(), + }, + services: map[types.NamespacedName]*apiv1.Service{ + {Namespace: "test", Name: "svc1"}: createSvc("svc1"), + }, + expected: &L4Route{ + Source: validSingleBackendTCPR, + Valid: true, + Attachable: true, + ParentRefs: []ParentRef{parentRefGraph}, + Spec: L4RouteSpec{ + BackendRefs: []BackendRef{ + { + SvcNsName: types.NamespacedName{Namespace: "test", Name: "svc1"}, + ServicePort: apiv1.ServicePort{ + Port: 80, + }, + Weight: 1, + Valid: true, + }, + }, + }, + }, + }, + { + name: "valid multi-backend with weights", + route: validMultiBackendTCPR, + gateways: map[types.NamespacedName]*Gateway{ + {Namespace: "test", Name: "gateway"}: createGateway(), + }, + services: map[types.NamespacedName]*apiv1.Service{ + {Namespace: "test", Name: "svc1"}: createSvc("svc1"), + {Namespace: "test", Name: "svc2"}: createSvc("svc2"), + }, + expected: &L4Route{ + Source: validMultiBackendTCPR, + Valid: true, + Attachable: true, + ParentRefs: []ParentRef{parentRefGraph}, + Spec: L4RouteSpec{ + BackendRefs: []BackendRef{ + { + SvcNsName: types.NamespacedName{Namespace: "test", Name: "svc1"}, + ServicePort: apiv1.ServicePort{ + Port: 80, + }, + Weight: 80, + Valid: true, + }, + { + SvcNsName: types.NamespacedName{Namespace: "test", Name: "svc2"}, + ServicePort: apiv1.ServicePort{ + Port: 80, + }, + Weight: 20, + Valid: true, + }, + }, + }, + }, + }, + { + name: "multi-rule TCP route", + route: multiRuleTCPR, + gateways: map[types.NamespacedName]*Gateway{ + {Namespace: "test", Name: "gateway"}: createGateway(), + }, + services: map[types.NamespacedName]*apiv1.Service{ + {Namespace: "test", Name: "svc1"}: createSvc("svc1"), + {Namespace: "test", Name: "svc2"}: createSvc("svc2"), + }, + expected: &L4Route{ + Source: multiRuleTCPR, + Valid: true, + Attachable: true, + ParentRefs: []ParentRef{parentRefGraph}, + Spec: L4RouteSpec{ + BackendRefs: []BackendRef{ + { + SvcNsName: types.NamespacedName{Namespace: "test", Name: "svc1"}, + ServicePort: apiv1.ServicePort{ + Port: 80, + }, + Weight: 1, + Valid: true, + }, + { + SvcNsName: types.NamespacedName{Namespace: "test", Name: "svc2"}, + ServicePort: apiv1.ServicePort{ + Port: 80, + }, + Weight: 1, + Valid: true, + }, + }, + }, + }, + }, + } + + refGrantResolver := func(resource toResource) bool { + return false + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + result := buildTCPRoute(test.route, test.gateways, test.services, refGrantResolver) + g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty()) + }) + } +} diff --git a/internal/controller/state/graph/udproute_test.go b/internal/controller/state/graph/udproute_test.go new file mode 100644 index 0000000000..b2ded498ba --- /dev/null +++ b/internal/controller/state/graph/udproute_test.go @@ -0,0 +1,654 @@ +package graph + +import ( + "testing" + + . "github.com/onsi/gomega" + apiv1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + "sigs.k8s.io/gateway-api/apis/v1alpha2" + + ngfAPI "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha2" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/conditions" + "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers" +) + +func createUDPRoute( + rules []v1alpha2.UDPRouteRule, + parentRefs []gatewayv1.ParentReference, +) *v1alpha2.UDPRoute { + return &v1alpha2.UDPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "udpr", + }, + Spec: v1alpha2.UDPRouteSpec{ + CommonRouteSpec: gatewayv1.CommonRouteSpec{ + ParentRefs: parentRefs, + }, + Rules: rules, + }, + } +} + +func TestBuildUDPRoute(t *testing.T) { + t.Parallel() + + parentRef := gatewayv1.ParentReference{ + Namespace: helpers.GetPointer[gatewayv1.Namespace]("test"), + Name: "gateway", + SectionName: helpers.GetPointer[gatewayv1.SectionName]("l1"), + } + + createGateway := func() *Gateway { + return &Gateway{ + Source: &gatewayv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "gateway", + }, + }, + Valid: true, + } + } + + modGateway := func(gw *Gateway, mod func(*Gateway) *Gateway) *Gateway { + return mod(gw) + } + + parentRefGraph := ParentRef{ + SectionName: helpers.GetPointer[gatewayv1.SectionName]("l1"), + Gateway: &ParentRefGateway{ + NamespacedName: types.NamespacedName{ + Namespace: "test", + Name: "gateway", + }, + }, + } + + // Test cases for invalid UDPRoutes + duplicateParentRefsUDPR := createUDPRoute( + nil, + []gatewayv1.ParentReference{ + parentRef, + parentRef, + }, + ) + + noParentRefsUDPR := createUDPRoute( + nil, + []gatewayv1.ParentReference{}, + ) + + noRulesUDPR := createUDPRoute( + nil, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + backendRefDNEUDPR := createUDPRoute( + []v1alpha2.UDPRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "svc-does-not-exist", + Port: helpers.GetPointer[gatewayv1.PortNumber](53), + }, + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + wrongBackendRefGroupUDPR := createUDPRoute( + []v1alpha2.UDPRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "svc1", + Port: helpers.GetPointer[gatewayv1.PortNumber](53), + Group: helpers.GetPointer[gatewayv1.Group]("wrong"), + }, + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + wrongBackendRefKindUDPR := createUDPRoute( + []v1alpha2.UDPRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "svc1", + Port: helpers.GetPointer[gatewayv1.PortNumber](53), + Kind: helpers.GetPointer[gatewayv1.Kind]("not-service"), + }, + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + diffNsBackendRefUDPR := createUDPRoute( + []v1alpha2.UDPRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "svc1", + Port: helpers.GetPointer[gatewayv1.PortNumber](53), + Namespace: helpers.GetPointer[gatewayv1.Namespace]("diff"), + }, + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + portNilBackendRefUDPR := createUDPRoute( + []v1alpha2.UDPRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "svc1", + }, + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + ipFamilyMismatchUDPR := createUDPRoute( + []v1alpha2.UDPRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "svc1", + Port: helpers.GetPointer[gatewayv1.PortNumber](53), + }, + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + // Valid UDPRoute with single backend + validSingleBackendUDPR := createUDPRoute( + []v1alpha2.UDPRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "svc1", + Port: helpers.GetPointer[gatewayv1.PortNumber](53), + }, + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + // Valid UDPRoute with multiple backends (weighted) + validMultiBackendUDPR := createUDPRoute( + []v1alpha2.UDPRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "svc1", + Port: helpers.GetPointer[gatewayv1.PortNumber](53), + }, + Weight: helpers.GetPointer[int32](70), + }, + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "svc2", + Port: helpers.GetPointer[gatewayv1.PortNumber](53), + }, + Weight: helpers.GetPointer[int32](30), + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + // UDPRoute with multiple rules + multiRuleUDPR := createUDPRoute( + []v1alpha2.UDPRouteRule{ + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "svc1", + Port: helpers.GetPointer[gatewayv1.PortNumber](53), + }, + }, + }, + }, + { + BackendRefs: []gatewayv1.BackendRef{ + { + BackendObjectReference: gatewayv1.BackendObjectReference{ + Name: "svc2", + Port: helpers.GetPointer[gatewayv1.PortNumber](53), + }, + }, + }, + }, + }, + []gatewayv1.ParentReference{ + parentRef, + }, + ) + + createSvc := func(name string) *apiv1.Service { + return &apiv1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: name, + }, + Spec: apiv1.ServiceSpec{ + ClusterIP: "10.0.0.1", + Ports: []apiv1.ServicePort{ + { + Port: 53, + }, + }, + }, + } + } + + createModSvc := func(mod func(*apiv1.Service) *apiv1.Service) *apiv1.Service { + return mod(createSvc("svc1")) + } + + tests := []struct { + name string + route *v1alpha2.UDPRoute + gateways map[types.NamespacedName]*Gateway + services map[types.NamespacedName]*apiv1.Service + expected *L4Route + }{ + { + name: "duplicate parent refs", + route: duplicateParentRefsUDPR, + gateways: map[types.NamespacedName]*Gateway{ + {Namespace: "test", Name: "gateway"}: createGateway(), + }, + expected: &L4Route{ + Source: duplicateParentRefsUDPR, + Valid: false, + }, + }, + { + name: "no parent refs", + route: noParentRefsUDPR, + gateways: map[types.NamespacedName]*Gateway{ + {Namespace: "test", Name: "gateway"}: createGateway(), + }, + expected: nil, + }, + { + name: "no rules", + route: noRulesUDPR, + gateways: map[types.NamespacedName]*Gateway{ + {Namespace: "test", Name: "gateway"}: createGateway(), + }, + expected: &L4Route{ + Source: noRulesUDPR, + Valid: false, + Attachable: false, + ParentRefs: []ParentRef{parentRefGraph}, + Conditions: []conditions.Condition{ + conditions.NewRouteBackendRefUnsupportedValue("Must have at least one Rule"), + }, + }, + }, + { + name: "backend ref does not exist", + route: backendRefDNEUDPR, + gateways: map[types.NamespacedName]*Gateway{ + {Namespace: "test", Name: "gateway"}: createGateway(), + }, + services: map[types.NamespacedName]*apiv1.Service{}, + expected: &L4Route{ + Source: backendRefDNEUDPR, + Valid: true, + Attachable: true, + ParentRefs: []ParentRef{parentRefGraph}, + Spec: L4RouteSpec{ + BackendRefs: []BackendRef{ + { + SvcNsName: types.NamespacedName{Namespace: "test", Name: "svc-does-not-exist"}, + Weight: 1, + Valid: false, + }, + }, + }, + Conditions: []conditions.Condition{ + conditions.NewRouteBackendRefRefBackendNotFound( + "spec.rules[0].backendRefs[0].name: Not found: \"svc-does-not-exist\"", + ), + }, + }, + }, + { + name: "wrong backend ref group", + route: wrongBackendRefGroupUDPR, + gateways: map[types.NamespacedName]*Gateway{ + {Namespace: "test", Name: "gateway"}: createGateway(), + }, + services: map[types.NamespacedName]*apiv1.Service{ + {Namespace: "test", Name: "svc1"}: createSvc("svc1"), + }, + expected: &L4Route{ + Source: wrongBackendRefGroupUDPR, + Valid: true, + Attachable: true, + ParentRefs: []ParentRef{parentRefGraph}, + Spec: L4RouteSpec{ + BackendRefs: []BackendRef{ + { + + Valid: false, + }, + }, + }, + Conditions: []conditions.Condition{ + conditions.NewRouteBackendRefInvalidKind( + `spec.rules[0].backendRefs[0].group: Unsupported value: "wrong": supported values: "core", ""`, + ), + }, + }, + }, + { + name: "wrong backend ref kind", + route: wrongBackendRefKindUDPR, + gateways: map[types.NamespacedName]*Gateway{ + {Namespace: "test", Name: "gateway"}: createGateway(), + }, + services: map[types.NamespacedName]*apiv1.Service{ + {Namespace: "test", Name: "svc1"}: createSvc("svc1"), + }, + expected: &L4Route{ + Source: wrongBackendRefKindUDPR, + Valid: true, + Attachable: true, + ParentRefs: []ParentRef{parentRefGraph}, + Spec: L4RouteSpec{ + BackendRefs: []BackendRef{ + { + + Valid: false, + }, + }, + }, + Conditions: []conditions.Condition{ + conditions.NewRouteBackendRefInvalidKind( + `spec.rules[0].backendRefs[0].kind: Unsupported value: "not-service": supported values: "Service"`, + ), + }, + }, + }, + { + name: "different namespace backend ref without reference grant", + route: diffNsBackendRefUDPR, + gateways: map[types.NamespacedName]*Gateway{ + {Namespace: "test", Name: "gateway"}: createGateway(), + }, + services: map[types.NamespacedName]*apiv1.Service{ + {Namespace: "diff", Name: "svc1"}: createModSvc(func(svc *apiv1.Service) *apiv1.Service { + svc.Namespace = "diff" + return svc + }), + }, + expected: &L4Route{ + Source: diffNsBackendRefUDPR, + Valid: true, + Attachable: true, + ParentRefs: []ParentRef{parentRefGraph}, + Spec: L4RouteSpec{ + BackendRefs: []BackendRef{ + { + + Valid: false, + }, + }, + }, + Conditions: []conditions.Condition{ + conditions.NewRouteBackendRefRefNotPermitted( + `spec.rules[0].backendRefs[0].namespace: Forbidden: Backend ref to Service diff/svc1 not permitted by any ReferenceGrant`, + ), + }, + }, + }, + { + name: "port nil backend ref", + route: portNilBackendRefUDPR, + gateways: map[types.NamespacedName]*Gateway{ + {Namespace: "test", Name: "gateway"}: createGateway(), + }, + services: map[types.NamespacedName]*apiv1.Service{ + {Namespace: "test", Name: "svc1"}: createSvc("svc1"), + }, + expected: &L4Route{ + Source: portNilBackendRefUDPR, + Valid: true, + Attachable: true, + ParentRefs: []ParentRef{parentRefGraph}, + Spec: L4RouteSpec{ + BackendRefs: []BackendRef{ + { + + Valid: false, + }, + }, + }, + Conditions: []conditions.Condition{ + conditions.NewRouteBackendRefUnsupportedValue( + "spec.rules[0].backendRefs[0].port: Required value: port cannot be nil", + ), + }, + }, + }, + { + name: "IP family mismatch", + route: ipFamilyMismatchUDPR, + gateways: map[types.NamespacedName]*Gateway{ + {Namespace: "test", Name: "gateway"}: modGateway(createGateway(), func(gw *Gateway) *Gateway { + gw.EffectiveNginxProxy = &EffectiveNginxProxy{IPFamily: helpers.GetPointer(ngfAPI.IPv6)} + return gw + }), + }, + services: map[types.NamespacedName]*apiv1.Service{ + {Namespace: "test", Name: "svc1"}: createModSvc(func(svc *apiv1.Service) *apiv1.Service { + svc.Spec.IPFamilies = []apiv1.IPFamily{apiv1.IPv4Protocol} + return svc + }), + }, + expected: &L4Route{ + Source: ipFamilyMismatchUDPR, + Valid: true, + Attachable: true, + ParentRefs: []ParentRef{ + { + SectionName: helpers.GetPointer[gatewayv1.SectionName]("l1"), + Gateway: &ParentRefGateway{ + NamespacedName: types.NamespacedName{ + Namespace: "test", + Name: "gateway", + }, + EffectiveNginxProxy: &EffectiveNginxProxy{IPFamily: helpers.GetPointer(ngfAPI.IPv6)}, + }, + }, + }, + Spec: L4RouteSpec{ + BackendRefs: []BackendRef{ + { + SvcNsName: types.NamespacedName{Namespace: "test", Name: "svc1"}, + ServicePort: apiv1.ServicePort{ + Port: 53, + }, + Weight: 1, + Valid: true, + InvalidForGateways: map[types.NamespacedName]conditions.Condition{ + {Namespace: "test", Name: "gateway"}: conditions.NewRouteInvalidIPFamily( + "service configured with IPv4 family but NginxProxy is configured with IPv6", + ), + }, + }, + }, + }, + }, + }, + { + name: "valid single backend", + route: validSingleBackendUDPR, + gateways: map[types.NamespacedName]*Gateway{ + {Namespace: "test", Name: "gateway"}: createGateway(), + }, + services: map[types.NamespacedName]*apiv1.Service{ + {Namespace: "test", Name: "svc1"}: createSvc("svc1"), + }, + expected: &L4Route{ + Source: validSingleBackendUDPR, + Valid: true, + Attachable: true, + ParentRefs: []ParentRef{parentRefGraph}, + Spec: L4RouteSpec{ + BackendRefs: []BackendRef{ + { + SvcNsName: types.NamespacedName{Namespace: "test", Name: "svc1"}, + ServicePort: apiv1.ServicePort{ + Port: 53, + }, + Weight: 1, + Valid: true, + }, + }, + }, + }, + }, + { + name: "valid multi-backend with weights", + route: validMultiBackendUDPR, + gateways: map[types.NamespacedName]*Gateway{ + {Namespace: "test", Name: "gateway"}: createGateway(), + }, + services: map[types.NamespacedName]*apiv1.Service{ + {Namespace: "test", Name: "svc1"}: createSvc("svc1"), + {Namespace: "test", Name: "svc2"}: createSvc("svc2"), + }, + expected: &L4Route{ + Source: validMultiBackendUDPR, + Valid: true, + Attachable: true, + ParentRefs: []ParentRef{parentRefGraph}, + Spec: L4RouteSpec{ + BackendRefs: []BackendRef{ + { + SvcNsName: types.NamespacedName{Namespace: "test", Name: "svc1"}, + ServicePort: apiv1.ServicePort{ + Port: 53, + }, + Weight: 70, + Valid: true, + }, + { + SvcNsName: types.NamespacedName{Namespace: "test", Name: "svc2"}, + ServicePort: apiv1.ServicePort{ + Port: 53, + }, + Weight: 30, + Valid: true, + }, + }, + }, + }, + }, + { + name: "multi-rule UDP route", + route: multiRuleUDPR, + gateways: map[types.NamespacedName]*Gateway{ + {Namespace: "test", Name: "gateway"}: createGateway(), + }, + services: map[types.NamespacedName]*apiv1.Service{ + {Namespace: "test", Name: "svc1"}: createSvc("svc1"), + {Namespace: "test", Name: "svc2"}: createSvc("svc2"), + }, + expected: &L4Route{ + Source: multiRuleUDPR, + Valid: true, + Attachable: true, + ParentRefs: []ParentRef{parentRefGraph}, + Spec: L4RouteSpec{ + BackendRefs: []BackendRef{ + { + SvcNsName: types.NamespacedName{Namespace: "test", Name: "svc1"}, + ServicePort: apiv1.ServicePort{ + Port: 53, + }, + Weight: 1, + Valid: true, + }, + { + SvcNsName: types.NamespacedName{Namespace: "test", Name: "svc2"}, + ServicePort: apiv1.ServicePort{ + Port: 53, + }, + Weight: 1, + Valid: true, + }, + }, + }, + }, + }, + } + + refGrantResolver := func(resource toResource) bool { + return false + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + result := buildUDPRoute(test.route, test.gateways, test.services, refGrantResolver) + g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty()) + }) + } +} From b148524089ab6b8783dc475453655b0ac94b7145 Mon Sep 17 00:00:00 2001 From: tanjie Date: Mon, 13 Oct 2025 17:04:19 +0800 Subject: [PATCH 5/5] fix: use corev1.Protocol directly in port maps instead of wrapping in PortInfo --- internal/controller/provisioner/objects.go | 35 ++++++++----------- .../controller/provisioner/objects_test.go | 7 ++++ 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/internal/controller/provisioner/objects.go b/internal/controller/provisioner/objects.go index aea0f680ab..35e6ebb1de 100644 --- a/internal/controller/provisioner/objects.go +++ b/internal/controller/provisioner/objects.go @@ -44,11 +44,6 @@ const ( defaultInitialDelaySeconds = int32(3) ) -type PortInfo struct { - Port int32 - Protocol corev1.Protocol -} - var emptyDirVolumeSource = corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}} //nolint:gocyclo // will refactor at some point @@ -152,7 +147,7 @@ func (p *NginxProvisioner) buildNginxResourceObjects( openshiftObjs = p.buildOpenshiftObjects(objectMeta) } - ports := make(map[int32]PortInfo) + ports := make(map[int32]corev1.Protocol) for _, listener := range gateway.Spec.Listeners { var protocol corev1.Protocol switch listener.Protocol { @@ -163,7 +158,7 @@ func (p *NginxProvisioner) buildNginxResourceObjects( default: protocol = corev1.ProtocolTCP } - ports[int32(listener.Port)] = PortInfo{Port: int32(listener.Port), Protocol: protocol} + ports[int32(listener.Port)] = protocol } // Create separate copies of objectMeta for service and deployment to avoid shared map references @@ -529,7 +524,7 @@ func (p *NginxProvisioner) buildOpenshiftObjects(objectMeta metav1.ObjectMeta) [ func buildNginxService( objectMeta metav1.ObjectMeta, nProxyCfg *graph.EffectiveNginxProxy, - ports map[int32]PortInfo, + ports map[int32]corev1.Protocol, selectorLabels map[string]string, addresses []gatewayv1.GatewaySpecAddress, ) (*corev1.Service, error) { @@ -552,17 +547,17 @@ func buildNginxService( } servicePorts := make([]corev1.ServicePort, 0, len(ports)) - for _, portInfo := range ports { + for port, protocol := range ports { servicePort := corev1.ServicePort{ - Name: fmt.Sprintf("port-%d", portInfo.Port), - Port: portInfo.Port, - TargetPort: intstr.FromInt32(portInfo.Port), - Protocol: portInfo.Protocol, + Name: fmt.Sprintf("port-%d", port), + Port: port, + TargetPort: intstr.FromInt32(port), + Protocol: protocol, } if serviceType != corev1.ServiceTypeClusterIP { for _, nodePort := range serviceCfg.NodePorts { - if nodePort.ListenerPort == portInfo.Port { + if nodePort.ListenerPort == port { servicePort.NodePort = nodePort.Port } } @@ -640,7 +635,7 @@ func (p *NginxProvisioner) buildNginxDeployment( nProxyCfg *graph.EffectiveNginxProxy, ngxIncludesConfigMapName string, ngxAgentConfigMapName string, - ports map[int32]PortInfo, + ports map[int32]corev1.Protocol, selectorLabels map[string]string, agentTLSSecretName string, dockerSecretNames map[string]string, @@ -794,7 +789,7 @@ func (p *NginxProvisioner) buildNginxPodTemplateSpec( nProxyCfg *graph.EffectiveNginxProxy, ngxIncludesConfigMapName string, ngxAgentConfigMapName string, - ports map[int32]PortInfo, + ports map[int32]corev1.Protocol, agentTLSSecretName string, dockerSecretNames map[string]string, jwtSecretName string, @@ -803,11 +798,11 @@ func (p *NginxProvisioner) buildNginxPodTemplateSpec( dataplaneKeySecretName string, ) corev1.PodTemplateSpec { containerPorts := make([]corev1.ContainerPort, 0, len(ports)) - for _, portInfo := range ports { + for port, protocol := range ports { containerPort := corev1.ContainerPort{ - Name: fmt.Sprintf("port-%d", portInfo.Port), - ContainerPort: portInfo.Port, - Protocol: portInfo.Protocol, + Name: fmt.Sprintf("port-%d", port), + ContainerPort: port, + Protocol: protocol, } containerPorts = append(containerPorts, containerPort) } diff --git a/internal/controller/provisioner/objects_test.go b/internal/controller/provisioner/objects_test.go index 2327db259d..b6b0508a78 100644 --- a/internal/controller/provisioner/objects_test.go +++ b/internal/controller/provisioner/objects_test.go @@ -177,17 +177,20 @@ func TestBuildNginxResourceObjects(t *testing.T) { { Port: 80, Name: "port-80", + Protocol: corev1.ProtocolTCP, TargetPort: intstr.FromInt(80), NodePort: 30000, }, { Port: 8888, Name: "port-8888", + Protocol: corev1.ProtocolTCP, TargetPort: intstr.FromInt(8888), }, { Port: 9999, Name: "port-9999", + Protocol: corev1.ProtocolTCP, TargetPort: intstr.FromInt(9999), }, })) @@ -208,10 +211,12 @@ func TestBuildNginxResourceObjects(t *testing.T) { { ContainerPort: 80, Name: "port-80", + Protocol: corev1.ProtocolTCP, }, { ContainerPort: 8888, Name: "port-8888", + Protocol: corev1.ProtocolTCP, }, { ContainerPort: config.DefaultNginxMetricsPort, @@ -220,6 +225,7 @@ func TestBuildNginxResourceObjects(t *testing.T) { { ContainerPort: 9999, Name: "port-9999", + Protocol: corev1.ProtocolTCP, }, })) @@ -374,6 +380,7 @@ func TestBuildNginxResourceObjects_NginxProxyConfig(t *testing.T) { g.Expect(container.Ports).To(ContainElement(corev1.ContainerPort{ ContainerPort: 8443, Name: "port-8443", + Protocol: corev1.ProtocolTCP, HostPort: 8443, }))