From 66d4394ba9ccc347606b3fcbc57a68a27d178833 Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Wed, 1 Oct 2025 10:08:27 -0700 Subject: [PATCH 1/3] fix: use maps for backendRefMappings instead of Sets * Sets compare by value and BackendRef contain multiple ptrs to Kind, Group and Namespace, so this Set didnt really serve us any good purpose of deduping same backendRefs * Instead use maps keyed by a util string - NamespaceNameWithGroupKind similar to what we use for ExtentionRefFilters Signed-off-by: Arko Dasgupta --- internal/provider/kubernetes/controller.go | 34 +++++++++++---- .../provider/kubernetes/controller_test.go | 41 +++++++++++++++---- internal/provider/kubernetes/resource.go | 6 +-- internal/provider/kubernetes/routes_test.go | 2 +- 4 files changed, 63 insertions(+), 20 deletions(-) diff --git a/internal/provider/kubernetes/controller.go b/internal/provider/kubernetes/controller.go index 269bf25f60..8626d2fb98 100644 --- a/internal/provider/kubernetes/controller.go +++ b/internal/provider/kubernetes/controller.go @@ -603,7 +603,7 @@ func (r *gatewayAPIReconciler) processBackendRefs(ctx context.Context, gwcResour // All other errors are just logged and ignored - these errors result in missing referenced backend resources // in the resource tree, which is acceptable as the Gateway API translation layer will handle them. // The Gateway API translation layer will surface these errors in the status of the resources referencing them. - for backendRef := range resourceMappings.allAssociatedBackendRefs { + for _, backendRef := range resourceMappings.allAssociatedBackendRefs { backendRefKind := gatewayapi.KindDerefOr(backendRef.Kind, resource.KindService) backendNs, backendName := string(*backendRef.Namespace), string(backendRef.Name) logger := r.log.WithValues("kind", backendRefKind, "namespace", backendNs, @@ -964,6 +964,24 @@ func getBackendRefs(backendCluster egv1a1.BackendCluster) []gwapiv1.BackendObjec return backendRefs } +func backendRefKey(ref gwapiv1.BackendObjectReference) utils.NamespacedNameWithGroupKind { + namespace := gatewayapi.NamespaceDerefOr(ref.Namespace, "") + group := gatewayapi.GroupDerefOr(ref.Group, "") + kind := gatewayapi.KindDerefOr(ref.Kind, resource.KindService) + return utils.NamespacedNameWithGroupKind{ + NamespacedName: types.NamespacedName{Namespace: namespace, Name: string(ref.Name)}, + GroupKind: schema.GroupKind{Group: group, Kind: kind}, + } +} + +func (rm *resourceMappings) insertBackendRef(ref gwapiv1.BackendObjectReference) { + key := backendRefKey(ref) + if _, exists := rm.allAssociatedBackendRefs[key]; exists { + return + } + rm.allAssociatedBackendRefs[key] = ref +} + // processBackendRef adds the referenced BackendRef to the resourceMap for later processBackendRefs. // If BackendRef exists in a different namespace and there is a ReferenceGrant, adds ReferenceGrant to the resourceTree. func (r *gatewayAPIReconciler) processBackendRef( @@ -976,12 +994,13 @@ func (r *gatewayAPIReconciler) processBackendRef( backendRef gwapiv1.BackendObjectReference, ) error { backendNamespace := gatewayapi.NamespaceDerefOr(backendRef.Namespace, ownerNS) - resourceMap.allAssociatedBackendRefs.Insert(gwapiv1.BackendObjectReference{ + normalizedRef := gwapiv1.BackendObjectReference{ Group: backendRef.Group, Kind: backendRef.Kind, Namespace: gatewayapi.NamespacePtr(backendNamespace), Name: backendRef.Name, - }) + } + resourceMap.insertBackendRef(normalizedRef) if backendNamespace != ownerNS { from := ObjectKindNamespacedName{ @@ -1516,7 +1535,7 @@ func (r *gatewayAPIReconciler) processServiceClusterForGatewayClass(ep *egv1a1.E } } - resourceMap.allAssociatedBackendRefs.Insert(gwapiv1.BackendObjectReference{ + resourceMap.insertBackendRef(gwapiv1.BackendObjectReference{ Kind: ptr.To(gwapiv1.Kind("Service")), Namespace: gatewayapi.NamespacePtr(proxySvcNamespace), Name: gwapiv1.ObjectName(proxySvcName), @@ -1545,7 +1564,7 @@ func (r *gatewayAPIReconciler) processServiceClusterForGateway(ep *egv1a1.EnvoyP } } - resourceMap.allAssociatedBackendRefs.Insert(gwapiv1.BackendObjectReference{ + resourceMap.insertBackendRef(gwapiv1.BackendObjectReference{ Kind: ptr.To(gwapiv1.Kind("Service")), Namespace: gatewayapi.NamespacePtr(proxySvcNamespace), Name: gwapiv1.ObjectName(proxySvcName), @@ -2456,12 +2475,13 @@ func (r *gatewayAPIReconciler) processEnvoyProxy(ep *egv1a1.EnvoyProxy, resource for _, backendRef := range backendRefs { backendNamespace := gatewayapi.NamespaceDerefOr(backendRef.Namespace, ep.Namespace) - resourceMap.allAssociatedBackendRefs.Insert(gwapiv1.BackendObjectReference{ + normalizedRef := gwapiv1.BackendObjectReference{ Group: backendRef.Group, Kind: backendRef.Kind, Namespace: gatewayapi.NamespacePtr(backendNamespace), Name: backendRef.Name, - }) + } + resourceMap.insertBackendRef(normalizedRef) } } diff --git a/internal/provider/kubernetes/controller_test.go b/internal/provider/kubernetes/controller_test.go index d8799ffde2..fd97be1093 100644 --- a/internal/provider/kubernetes/controller_test.go +++ b/internal/provider/kubernetes/controller_test.go @@ -303,7 +303,7 @@ func TestProcessBackendRefsWithCustomBackends(t *testing.T) { // Create resource mappings resourceMappings := &resourceMappings{ - allAssociatedBackendRefs: sets.New[gwapiv1.BackendObjectReference](), + allAssociatedBackendRefs: make(map[utils.NamespacedNameWithGroupKind]gwapiv1.BackendObjectReference), allAssociatedNamespaces: sets.New[string](), allAssociatedBackendRefExtensionFilters: sets.New[utils.NamespacedNameWithGroupKind](), extensionRefFilters: tc.existingExtFilters, @@ -311,7 +311,7 @@ func TestProcessBackendRefsWithCustomBackends(t *testing.T) { // Add backend refs to the mapping for _, backendRef := range tc.backendRefs { - resourceMappings.allAssociatedBackendRefs.Insert(backendRef) + resourceMappings.insertBackendRef(backendRef) } // Create empty resource tree @@ -1303,11 +1303,14 @@ func TestProcessServiceClusterForGatewayClass(t *testing.T) { r.processServiceClusterForGatewayClass(tc.envoyProxy, tc.gatewayClass, resourceMap) - require.Contains(t, resourceMap.allAssociatedBackendRefs, gwapiv1.BackendObjectReference{ - Kind: ptr.To(gwapiv1.Kind("Service")), + expectedRef := gwapiv1.BackendObjectReference{ + Kind: ptr.To(gwapiv1.Kind(resource.KindService)), Namespace: gatewayapi.NamespacePtr(r.namespace), Name: gwapiv1.ObjectName(tc.expectedSvcName), - }) + } + key := backendRefKey(expectedRef) + require.Contains(t, resourceMap.allAssociatedBackendRefs, key) + require.Equal(t, expectedRef, resourceMap.allAssociatedBackendRefs[key]) }) } } @@ -1457,11 +1460,14 @@ func TestProcessServiceClusterForGateway(t *testing.T) { r.processServiceClusterForGateway(tc.envoyProxy, tc.gateway, resourceMap) - require.Contains(t, resourceMap.allAssociatedBackendRefs, gwapiv1.BackendObjectReference{ - Kind: ptr.To(gwapiv1.Kind("Service")), + expectedRef := gwapiv1.BackendObjectReference{ + Kind: ptr.To(gwapiv1.Kind(resource.KindService)), Namespace: gatewayapi.NamespacePtr(tc.expectedSvcNamespace), Name: gwapiv1.ObjectName(tc.expectedSvcName), - }) + } + key := backendRefKey(expectedRef) + require.Contains(t, resourceMap.allAssociatedBackendRefs, key) + require.Equal(t, expectedRef, resourceMap.allAssociatedBackendRefs[key]) }) } } @@ -1481,6 +1487,23 @@ func newGatewayAPIReconciler(logger logging.Logger) *gatewayAPIReconciler { } } +func TestProcessBackendRefDeduplicatesLogicalBackend(t *testing.T) { + logger := logging.DefaultLogger(os.Stdout, egv1a1.LogLevelInfo) + r := newGatewayAPIReconciler(logger) + resourceTree := resource.NewResources() + resourceMap := newResourceMapping() + + backendRef := gwapiv1.BackendObjectReference{ + Namespace: gatewayapi.NamespacePtr("default"), + Name: "svc", + } + + require.NoError(t, r.processBackendRef(t.Context(), resourceMap, resourceTree, resource.KindHTTPRoute, "default", "route-a", backendRef)) + require.NoError(t, r.processBackendRef(t.Context(), resourceMap, resourceTree, resource.KindHTTPRoute, "default", "route-b", backendRef)) + + require.Equal(t, 1, len(resourceMap.allAssociatedBackendRefs)) +} + func TestProcessBackendRefs(t *testing.T) { ns := "default" ctb := test.GetClusterTrustBundle("fake-ctb") @@ -1584,7 +1607,7 @@ func TestProcessBackendRefs(t *testing.T) { resourceTree := resource.NewResources() resourceMap := newResourceMapping() backend := tc.backend - resourceMap.allAssociatedBackendRefs.Insert(gwapiv1.BackendObjectReference{ + resourceMap.insertBackendRef(gwapiv1.BackendObjectReference{ Kind: gatewayapi.KindPtr(resource.KindBackend), Namespace: gatewayapi.NamespacePtr(backend.Namespace), Name: gwapiv1.ObjectName(backend.Name), diff --git a/internal/provider/kubernetes/resource.go b/internal/provider/kubernetes/resource.go index 3c521de950..1a1c03c9e8 100644 --- a/internal/provider/kubernetes/resource.go +++ b/internal/provider/kubernetes/resource.go @@ -46,8 +46,8 @@ type resourceMappings struct { allAssociatedTCPRoutes sets.Set[string] // Set for storing UDPRoutes' NamespacedNames attaching to various Gateway objects. allAssociatedUDPRoutes sets.Set[string] - // Set for storing backendRefs' BackendObjectReference referred by various Route objects. - allAssociatedBackendRefs sets.Set[gwapiv1.BackendObjectReference] + // Map caching BackendObjectReferences keyed by their normalized identifier. + allAssociatedBackendRefs map[utils.NamespacedNameWithGroupKind]gwapiv1.BackendObjectReference // Set for storing ClientTrafficPolicies' NamespacedNames referred by various Route objects. allAssociatedClientTrafficPolicies sets.Set[string] // Set for storing BackendTrafficPolicies' NamespacedNames referred by various Route objects. @@ -90,7 +90,7 @@ func newResourceMapping() *resourceMappings { allAssociatedGRPCRoutes: sets.New[string](), allAssociatedTCPRoutes: sets.New[string](), allAssociatedUDPRoutes: sets.New[string](), - allAssociatedBackendRefs: sets.New[gwapiv1.BackendObjectReference](), + allAssociatedBackendRefs: make(map[utils.NamespacedNameWithGroupKind]gwapiv1.BackendObjectReference), allAssociatedClientTrafficPolicies: sets.New[string](), allAssociatedBackendTrafficPolicies: sets.New[string](), allAssociatedSecurityPolicies: sets.New[string](), diff --git a/internal/provider/kubernetes/routes_test.go b/internal/provider/kubernetes/routes_test.go index 8d69fd9c4a..b81c21da9d 100644 --- a/internal/provider/kubernetes/routes_test.go +++ b/internal/provider/kubernetes/routes_test.go @@ -1302,7 +1302,7 @@ func TestProcessHTTPRoutesWithCustomBackends(t *testing.T) { // Verify results require.NoError(t, err) require.Len(t, resourceMap.extensionRefFilters, tc.expectedExtFiltersCount) - require.Equal(t, tc.expectedBackendRefsCount, resourceMap.allAssociatedBackendRefs.Len()) + require.Equal(t, tc.expectedBackendRefsCount, len(resourceMap.allAssociatedBackendRefs)) // Verify that HTTPRoutes were processed require.Len(t, resourceTree.HTTPRoutes, 1) From 086662b5d195f2062efddafbdb345007b59053eb Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Wed, 1 Oct 2025 10:23:49 -0700 Subject: [PATCH 2/3] make key a ptr Signed-off-by: Arko Dasgupta --- internal/provider/kubernetes/controller.go | 9 +++++++-- internal/provider/kubernetes/controller_test.go | 8 ++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/internal/provider/kubernetes/controller.go b/internal/provider/kubernetes/controller.go index 8626d2fb98..252f33f3b9 100644 --- a/internal/provider/kubernetes/controller.go +++ b/internal/provider/kubernetes/controller.go @@ -964,7 +964,10 @@ func getBackendRefs(backendCluster egv1a1.BackendCluster) []gwapiv1.BackendObjec return backendRefs } -func backendRefKey(ref gwapiv1.BackendObjectReference) utils.NamespacedNameWithGroupKind { +func backendRefKey(ref *gwapiv1.BackendObjectReference) utils.NamespacedNameWithGroupKind { + if ref == nil { + return utils.NamespacedNameWithGroupKind{} + } namespace := gatewayapi.NamespaceDerefOr(ref.Namespace, "") group := gatewayapi.GroupDerefOr(ref.Group, "") kind := gatewayapi.KindDerefOr(ref.Kind, resource.KindService) @@ -975,7 +978,7 @@ func backendRefKey(ref gwapiv1.BackendObjectReference) utils.NamespacedNameWithG } func (rm *resourceMappings) insertBackendRef(ref gwapiv1.BackendObjectReference) { - key := backendRefKey(ref) + key := backendRefKey(&ref) if _, exists := rm.allAssociatedBackendRefs[key]; exists { return } @@ -999,6 +1002,7 @@ func (r *gatewayAPIReconciler) processBackendRef( Kind: backendRef.Kind, Namespace: gatewayapi.NamespacePtr(backendNamespace), Name: backendRef.Name, + Port: backendRef.Port, } resourceMap.insertBackendRef(normalizedRef) @@ -2480,6 +2484,7 @@ func (r *gatewayAPIReconciler) processEnvoyProxy(ep *egv1a1.EnvoyProxy, resource Kind: backendRef.Kind, Namespace: gatewayapi.NamespacePtr(backendNamespace), Name: backendRef.Name, + Port: backendRef.Port, } resourceMap.insertBackendRef(normalizedRef) } diff --git a/internal/provider/kubernetes/controller_test.go b/internal/provider/kubernetes/controller_test.go index fd97be1093..df66559eae 100644 --- a/internal/provider/kubernetes/controller_test.go +++ b/internal/provider/kubernetes/controller_test.go @@ -302,8 +302,8 @@ func TestProcessBackendRefsWithCustomBackends(t *testing.T) { } // Create resource mappings - resourceMappings := &resourceMappings{ - allAssociatedBackendRefs: make(map[utils.NamespacedNameWithGroupKind]gwapiv1.BackendObjectReference), + resourceMappings := &resourceMappings{ + allAssociatedBackendRefs: make(map[utils.NamespacedNameWithGroupKind]gwapiv1.BackendObjectReference), allAssociatedNamespaces: sets.New[string](), allAssociatedBackendRefExtensionFilters: sets.New[utils.NamespacedNameWithGroupKind](), extensionRefFilters: tc.existingExtFilters, @@ -1308,7 +1308,7 @@ func TestProcessServiceClusterForGatewayClass(t *testing.T) { Namespace: gatewayapi.NamespacePtr(r.namespace), Name: gwapiv1.ObjectName(tc.expectedSvcName), } - key := backendRefKey(expectedRef) + key := backendRefKey(&expectedRef) require.Contains(t, resourceMap.allAssociatedBackendRefs, key) require.Equal(t, expectedRef, resourceMap.allAssociatedBackendRefs[key]) }) @@ -1465,7 +1465,7 @@ func TestProcessServiceClusterForGateway(t *testing.T) { Namespace: gatewayapi.NamespacePtr(tc.expectedSvcNamespace), Name: gwapiv1.ObjectName(tc.expectedSvcName), } - key := backendRefKey(expectedRef) + key := backendRefKey(&expectedRef) require.Contains(t, resourceMap.allAssociatedBackendRefs, key) require.Equal(t, expectedRef, resourceMap.allAssociatedBackendRefs[key]) }) From 7f0ee9d3f1522c68cf419bc5c0649e580a83c857 Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Wed, 1 Oct 2025 11:16:40 -0700 Subject: [PATCH 3/3] fix test Signed-off-by: Arko Dasgupta --- internal/provider/kubernetes/controller_test.go | 6 +++--- internal/provider/kubernetes/routes_test.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/provider/kubernetes/controller_test.go b/internal/provider/kubernetes/controller_test.go index df66559eae..9a978495af 100644 --- a/internal/provider/kubernetes/controller_test.go +++ b/internal/provider/kubernetes/controller_test.go @@ -302,8 +302,8 @@ func TestProcessBackendRefsWithCustomBackends(t *testing.T) { } // Create resource mappings - resourceMappings := &resourceMappings{ - allAssociatedBackendRefs: make(map[utils.NamespacedNameWithGroupKind]gwapiv1.BackendObjectReference), + resourceMappings := &resourceMappings{ + allAssociatedBackendRefs: make(map[utils.NamespacedNameWithGroupKind]gwapiv1.BackendObjectReference), allAssociatedNamespaces: sets.New[string](), allAssociatedBackendRefExtensionFilters: sets.New[utils.NamespacedNameWithGroupKind](), extensionRefFilters: tc.existingExtFilters, @@ -1501,7 +1501,7 @@ func TestProcessBackendRefDeduplicatesLogicalBackend(t *testing.T) { require.NoError(t, r.processBackendRef(t.Context(), resourceMap, resourceTree, resource.KindHTTPRoute, "default", "route-a", backendRef)) require.NoError(t, r.processBackendRef(t.Context(), resourceMap, resourceTree, resource.KindHTTPRoute, "default", "route-b", backendRef)) - require.Equal(t, 1, len(resourceMap.allAssociatedBackendRefs)) + require.Len(t, resourceMap.allAssociatedBackendRefs, 1) } func TestProcessBackendRefs(t *testing.T) { diff --git a/internal/provider/kubernetes/routes_test.go b/internal/provider/kubernetes/routes_test.go index b81c21da9d..f3b6261c66 100644 --- a/internal/provider/kubernetes/routes_test.go +++ b/internal/provider/kubernetes/routes_test.go @@ -1302,7 +1302,7 @@ func TestProcessHTTPRoutesWithCustomBackends(t *testing.T) { // Verify results require.NoError(t, err) require.Len(t, resourceMap.extensionRefFilters, tc.expectedExtFiltersCount) - require.Equal(t, tc.expectedBackendRefsCount, len(resourceMap.allAssociatedBackendRefs)) + require.Len(t, resourceMap.allAssociatedBackendRefs, tc.expectedBackendRefsCount) // Verify that HTTPRoutes were processed require.Len(t, resourceTree.HTTPRoutes, 1)