From 9e23994d95532eca8ca0e03b3867cf25b96156b7 Mon Sep 17 00:00:00 2001 From: shuqz Date: Tue, 11 Nov 2025 13:02:23 -0800 Subject: [PATCH] [feat gw-api]add dedupe in route mapper --- pkg/gateway/routeutils/loader.go | 5 +- pkg/gateway/routeutils/loader_test.go | 4 +- .../routeutils/route_listener_mapper.go | 24 +++-- .../routeutils/route_listener_mapper_test.go | 99 ++++++++++++++++++- 4 files changed, 118 insertions(+), 14 deletions(-) diff --git a/pkg/gateway/routeutils/loader.go b/pkg/gateway/routeutils/loader.go index 561e87c3e..b0536aa0a 100644 --- a/pkg/gateway/routeutils/loader.go +++ b/pkg/gateway/routeutils/loader.go @@ -6,7 +6,6 @@ import ( "github.com/go-logr/logr" "github.com/pkg/errors" - "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/controller-runtime/pkg/client" gwv1 "sigs.k8s.io/gateway-api/apis/v1" @@ -150,7 +149,7 @@ func (l *loaderImpl) LoadRoutesForGateway(ctx context.Context, gw gwv1.Gateway, } // loadChildResources responsible for loading all resources that a route descriptor references. -func (l *loaderImpl) loadChildResources(ctx context.Context, preloadedRoutes map[int][]preLoadRouteDescriptor, compatibleHostnamesByPort map[int32]map[types.NamespacedName][]gwv1.Hostname, gw gwv1.Gateway) (map[int32][]RouteDescriptor, []RouteData, error) { +func (l *loaderImpl) loadChildResources(ctx context.Context, preloadedRoutes map[int][]preLoadRouteDescriptor, compatibleHostnamesByPort map[int32]map[string][]gwv1.Hostname, gw gwv1.Gateway) (map[int32][]RouteDescriptor, []RouteData, error) { // Cache to reduce duplicate route lookups. // Kind -> [NamespacedName:Previously Loaded Descriptor] resourceCache := make(map[string]RouteDescriptor) @@ -190,7 +189,7 @@ func (l *loaderImpl) loadChildResources(ctx context.Context, preloadedRoutes map // Set compatible hostnames by port for all routes for _, route := range resourceCache { hostnamesByPort := make(map[int32][]gwv1.Hostname) - routeKey := route.GetRouteNamespacedName() + routeKey := fmt.Sprintf("%s-%s", route.GetRouteKind(), route.GetRouteNamespacedName()) for port, compatibleHostnames := range compatibleHostnamesByPort { if hostnames, exists := compatibleHostnames[routeKey]; exists { hostnamesByPort[port] = hostnames diff --git a/pkg/gateway/routeutils/loader_test.go b/pkg/gateway/routeutils/loader_test.go index c2be5f4f5..efd935ba9 100644 --- a/pkg/gateway/routeutils/loader_test.go +++ b/pkg/gateway/routeutils/loader_test.go @@ -22,9 +22,9 @@ type mockMapper struct { routeStatusUpdates []RouteData } -func (m *mockMapper) mapGatewayAndRoutes(context context.Context, gw gwv1.Gateway, routes []preLoadRouteDescriptor) (map[int][]preLoadRouteDescriptor, map[int32]map[types.NamespacedName][]gwv1.Hostname, []RouteData, error) { +func (m *mockMapper) mapGatewayAndRoutes(context context.Context, gw gwv1.Gateway, routes []preLoadRouteDescriptor) (map[int][]preLoadRouteDescriptor, map[int32]map[string][]gwv1.Hostname, []RouteData, error) { assert.ElementsMatch(m.t, m.expectedRoutes, routes) - return m.mapToReturn, make(map[int32]map[types.NamespacedName][]gwv1.Hostname), m.routeStatusUpdates, nil + return m.mapToReturn, make(map[int32]map[string][]gwv1.Hostname), m.routeStatusUpdates, nil } var _ RouteDescriptor = &mockRoute{} diff --git a/pkg/gateway/routeutils/route_listener_mapper.go b/pkg/gateway/routeutils/route_listener_mapper.go index 40262d15c..7a086f89c 100644 --- a/pkg/gateway/routeutils/route_listener_mapper.go +++ b/pkg/gateway/routeutils/route_listener_mapper.go @@ -2,6 +2,7 @@ package routeutils import ( "context" + "fmt" "github.com/go-logr/logr" "k8s.io/apimachinery/pkg/types" @@ -12,7 +13,7 @@ import ( // listenerToRouteMapper is an internal utility that will map a list of routes to the listeners of a gateway // if the gateway and/or route are incompatible, then the route is discarded. type listenerToRouteMapper interface { - mapGatewayAndRoutes(context context.Context, gw gwv1.Gateway, routes []preLoadRouteDescriptor) (map[int][]preLoadRouteDescriptor, map[int32]map[types.NamespacedName][]gwv1.Hostname, []RouteData, error) + mapGatewayAndRoutes(context context.Context, gw gwv1.Gateway, routes []preLoadRouteDescriptor) (map[int][]preLoadRouteDescriptor, map[int32]map[string][]gwv1.Hostname, []RouteData, error) } var _ listenerToRouteMapper = &listenerToRouteMapperImpl{} @@ -33,9 +34,9 @@ func newListenerToRouteMapper(k8sClient client.Client, logger logr.Logger) liste // mapGatewayAndRoutes will map route to the corresponding listener ports using the Gateway API spec rules. // Returns: (routesByPort, compatibleHostnamesByPort, failedRoutes, error) -func (ltr *listenerToRouteMapperImpl) mapGatewayAndRoutes(ctx context.Context, gw gwv1.Gateway, routes []preLoadRouteDescriptor) (map[int][]preLoadRouteDescriptor, map[int32]map[types.NamespacedName][]gwv1.Hostname, []RouteData, error) { +func (ltr *listenerToRouteMapperImpl) mapGatewayAndRoutes(ctx context.Context, gw gwv1.Gateway, routes []preLoadRouteDescriptor) (map[int][]preLoadRouteDescriptor, map[int32]map[string][]gwv1.Hostname, []RouteData, error) { result := make(map[int][]preLoadRouteDescriptor) - compatibleHostnamesByPort := make(map[int32]map[types.NamespacedName][]gwv1.Hostname) + compatibleHostnamesByPort := make(map[int32]map[string][]gwv1.Hostname) failedRoutes := make([]RouteData, 0) // First filter out any routes that are not intended for this Gateway. @@ -48,6 +49,8 @@ func (ltr *listenerToRouteMapperImpl) mapGatewayAndRoutes(ctx context.Context, g } } + // Dedupe - Check if route already exists for this port before adding + seenRoutesPerPort := make(map[int]map[string]bool) // Next, greedily looking for the route to attach to. for _, listener := range gw.Spec.Listeners { // used for cross serving check @@ -74,16 +77,23 @@ func (ltr *listenerToRouteMapperImpl) mapGatewayAndRoutes(ctx context.Context, g if allowedAttachment { port := int32(listener.Port) - result[int(port)] = append(result[int(port)], route) + routeKey := fmt.Sprintf("%s-%s", route.GetRouteKind(), route.GetRouteNamespacedName()) + if seenRoutesPerPort[int(port)] == nil { + seenRoutesPerPort[int(port)] = make(map[string]bool) + } + if !seenRoutesPerPort[int(port)][routeKey] { + seenRoutesPerPort[int(port)][routeKey] = true + result[int(port)] = append(result[int(port)], route) + } - // Store compatible hostnames per port per route + // Store compatible hostnames per port per route per kind if compatibleHostnamesByPort[port] == nil { - compatibleHostnamesByPort[port] = make(map[types.NamespacedName][]gwv1.Hostname) + compatibleHostnamesByPort[port] = make(map[string][]gwv1.Hostname) } // Append hostnames for routes that attach to multiple listeners on the same port - routeKey := route.GetRouteNamespacedName() compatibleHostnamesByPort[port][routeKey] = append(compatibleHostnamesByPort[port][routeKey], compatibleHostnames...) } + } } return result, compatibleHostnamesByPort, failedRoutes, nil diff --git a/pkg/gateway/routeutils/route_listener_mapper_test.go b/pkg/gateway/routeutils/route_listener_mapper_test.go index f14f31178..5e8cfa728 100644 --- a/pkg/gateway/routeutils/route_listener_mapper_test.go +++ b/pkg/gateway/routeutils/route_listener_mapper_test.go @@ -3,12 +3,13 @@ package routeutils import ( "context" "fmt" + "testing" + "github.com/go-logr/logr" "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" gwv1 "sigs.k8s.io/gateway-api/apis/v1" - "testing" ) type mockListenerAttachmentHelper struct { @@ -299,6 +300,99 @@ func Test_mapGatewayAndRoutes(t *testing.T) { name: "no output", expected: make(map[int][]preLoadRouteDescriptor), }, + { + name: "route attaches to multiple listeners on same port - verify deduplication", + gw: gwv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gw1", + Namespace: "ns-gw", + }, + Spec: gwv1.GatewaySpec{ + Listeners: []gwv1.Listener{ + { + Name: "listener1-port80", + Port: gwv1.PortNumber(80), + }, + { + Name: "listener2-port80", + Port: gwv1.PortNumber(80), + }, + }, + }, + }, + routes: []preLoadRouteDescriptor{route1}, + listenerAttachmentMap: map[string]bool{ + "listener1-port80-80-route1-ns1": true, + "listener2-port80-80-route1-ns1": true, + }, + routeListenerMap: map[string]bool{ + "listener1-port80-80-route1-ns1": true, + "listener2-port80-80-route1-ns1": true, + }, + routeGatewayMap: map[string]bool{ + makeRouteGatewayMapKey(gateway, route1): true, + }, + expected: map[int][]preLoadRouteDescriptor{ + 80: {route1}, // Only one route1, not duplicated + }, + }, + { + name: "different route kinds with same name attach to same listener", + gw: gwv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gw1", + Namespace: "ns-gw", + }, + Spec: gwv1.GatewaySpec{ + Listeners: []gwv1.Listener{ + { + Name: "https-listener", + Port: gwv1.PortNumber(443), + Protocol: gwv1.HTTPSProtocolType, + }, + }, + }, + }, + routes: []preLoadRouteDescriptor{ + convertHTTPRoute(gwv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-route", + Namespace: "default", + }, + }), + convertGRPCRoute(gwv1.GRPCRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-route", + Namespace: "default", + }, + }), + }, + listenerAttachmentMap: map[string]bool{ + "https-listener-443-my-route-default": true, + }, + routeListenerMap: map[string]bool{ + "https-listener-443-my-route-default": true, + }, + routeGatewayMap: map[string]bool{ + "gw1-ns-gw-my-route-default": true, + }, + expected: map[int][]preLoadRouteDescriptor{ + 443: { + convertHTTPRoute(gwv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-route", + Namespace: "default", + }, + }), + convertGRPCRoute(gwv1.GRPCRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-route", + Namespace: "default", + }, + }), + }, + }, + }, } for _, tc := range testCases { @@ -313,7 +407,7 @@ func Test_mapGatewayAndRoutes(t *testing.T) { }, logger: logr.Discard(), } - result, _, statusUpdates, err := mapper.mapGatewayAndRoutes(context.Background(), tc.gw, tc.routes) + result, compatibleHostnames, statusUpdates, err := mapper.mapGatewayAndRoutes(context.Background(), tc.gw, tc.routes) if tc.expectErr { assert.Error(t, err) @@ -322,6 +416,7 @@ func Test_mapGatewayAndRoutes(t *testing.T) { assert.NoError(t, err) assert.Equal(t, len(tc.expected), len(result)) + assert.NotNil(t, compatibleHostnames) assert.Equal(t, 0, len(statusUpdates))