Skip to content

Commit 6d583bd

Browse files
arkodgarkodg
authored
fix: use maps for backendRefMappings instead of Sets (#7120)
* 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 <[email protected]> * make key a ptr Signed-off-by: Arko Dasgupta <[email protected]> * fix test Signed-off-by: Arko Dasgupta <[email protected]> --------- Signed-off-by: Arko Dasgupta <[email protected]>
1 parent c15d754 commit 6d583bd

File tree

4 files changed

+68
-20
lines changed

4 files changed

+68
-20
lines changed

‎internal/provider/kubernetes/controller.go‎

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -603,7 +603,7 @@ func (r *gatewayAPIReconciler) processBackendRefs(ctx context.Context, gwcResour
603603
// All other errors are just logged and ignored - these errors result in missing referenced backend resources
604604
// in the resource tree, which is acceptable as the Gateway API translation layer will handle them.
605605
// The Gateway API translation layer will surface these errors in the status of the resources referencing them.
606-
for backendRef := range resourceMappings.allAssociatedBackendRefs {
606+
for _, backendRef := range resourceMappings.allAssociatedBackendRefs {
607607
backendRefKind := gatewayapi.KindDerefOr(backendRef.Kind, resource.KindService)
608608
backendNs, backendName := string(*backendRef.Namespace), string(backendRef.Name)
609609
logger := r.log.WithValues("kind", backendRefKind, "namespace", backendNs,
@@ -964,6 +964,27 @@ func getBackendRefs(backendCluster egv1a1.BackendCluster) []gwapiv1.BackendObjec
964964
return backendRefs
965965
}
966966

967+
func backendRefKey(ref *gwapiv1.BackendObjectReference) utils.NamespacedNameWithGroupKind {
968+
if ref == nil {
969+
return utils.NamespacedNameWithGroupKind{}
970+
}
971+
namespace := gatewayapi.NamespaceDerefOr(ref.Namespace, "")
972+
group := gatewayapi.GroupDerefOr(ref.Group, "")
973+
kind := gatewayapi.KindDerefOr(ref.Kind, resource.KindService)
974+
return utils.NamespacedNameWithGroupKind{
975+
NamespacedName: types.NamespacedName{Namespace: namespace, Name: string(ref.Name)},
976+
GroupKind: schema.GroupKind{Group: group, Kind: kind},
977+
}
978+
}
979+
980+
func (rm *resourceMappings) insertBackendRef(ref gwapiv1.BackendObjectReference) {
981+
key := backendRefKey(&ref)
982+
if _, exists := rm.allAssociatedBackendRefs[key]; exists {
983+
return
984+
}
985+
rm.allAssociatedBackendRefs[key] = ref
986+
}
987+
967988
// processBackendRef adds the referenced BackendRef to the resourceMap for later processBackendRefs.
968989
// If BackendRef exists in a different namespace and there is a ReferenceGrant, adds ReferenceGrant to the resourceTree.
969990
func (r *gatewayAPIReconciler) processBackendRef(
@@ -976,12 +997,14 @@ func (r *gatewayAPIReconciler) processBackendRef(
976997
backendRef gwapiv1.BackendObjectReference,
977998
) error {
978999
backendNamespace := gatewayapi.NamespaceDerefOr(backendRef.Namespace, ownerNS)
979-
resourceMap.allAssociatedBackendRefs.Insert(gwapiv1.BackendObjectReference{
1000+
normalizedRef := gwapiv1.BackendObjectReference{
9801001
Group: backendRef.Group,
9811002
Kind: backendRef.Kind,
9821003
Namespace: gatewayapi.NamespacePtr(backendNamespace),
9831004
Name: backendRef.Name,
984-
})
1005+
Port: backendRef.Port,
1006+
}
1007+
resourceMap.insertBackendRef(normalizedRef)
9851008

9861009
if backendNamespace != ownerNS {
9871010
from := ObjectKindNamespacedName{
@@ -1516,7 +1539,7 @@ func (r *gatewayAPIReconciler) processServiceClusterForGatewayClass(ep *egv1a1.E
15161539
}
15171540
}
15181541

1519-
resourceMap.allAssociatedBackendRefs.Insert(gwapiv1.BackendObjectReference{
1542+
resourceMap.insertBackendRef(gwapiv1.BackendObjectReference{
15201543
Kind: ptr.To(gwapiv1.Kind("Service")),
15211544
Namespace: gatewayapi.NamespacePtr(proxySvcNamespace),
15221545
Name: gwapiv1.ObjectName(proxySvcName),
@@ -1545,7 +1568,7 @@ func (r *gatewayAPIReconciler) processServiceClusterForGateway(ep *egv1a1.EnvoyP
15451568
}
15461569
}
15471570

1548-
resourceMap.allAssociatedBackendRefs.Insert(gwapiv1.BackendObjectReference{
1571+
resourceMap.insertBackendRef(gwapiv1.BackendObjectReference{
15491572
Kind: ptr.To(gwapiv1.Kind("Service")),
15501573
Namespace: gatewayapi.NamespacePtr(proxySvcNamespace),
15511574
Name: gwapiv1.ObjectName(proxySvcName),
@@ -2456,12 +2479,14 @@ func (r *gatewayAPIReconciler) processEnvoyProxy(ep *egv1a1.EnvoyProxy, resource
24562479

24572480
for _, backendRef := range backendRefs {
24582481
backendNamespace := gatewayapi.NamespaceDerefOr(backendRef.Namespace, ep.Namespace)
2459-
resourceMap.allAssociatedBackendRefs.Insert(gwapiv1.BackendObjectReference{
2482+
normalizedRef := gwapiv1.BackendObjectReference{
24602483
Group: backendRef.Group,
24612484
Kind: backendRef.Kind,
24622485
Namespace: gatewayapi.NamespacePtr(backendNamespace),
24632486
Name: backendRef.Name,
2464-
})
2487+
Port: backendRef.Port,
2488+
}
2489+
resourceMap.insertBackendRef(normalizedRef)
24652490
}
24662491
}
24672492

‎internal/provider/kubernetes/controller_test.go‎

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -303,15 +303,15 @@ func TestProcessBackendRefsWithCustomBackends(t *testing.T) {
303303

304304
// Create resource mappings
305305
resourceMappings := &resourceMappings{
306-
allAssociatedBackendRefs: sets.New[gwapiv1.BackendObjectReference](),
306+
allAssociatedBackendRefs: make(map[utils.NamespacedNameWithGroupKind]gwapiv1.BackendObjectReference),
307307
allAssociatedNamespaces: sets.New[string](),
308308
allAssociatedBackendRefExtensionFilters: sets.New[utils.NamespacedNameWithGroupKind](),
309309
extensionRefFilters: tc.existingExtFilters,
310310
}
311311

312312
// Add backend refs to the mapping
313313
for _, backendRef := range tc.backendRefs {
314-
resourceMappings.allAssociatedBackendRefs.Insert(backendRef)
314+
resourceMappings.insertBackendRef(backendRef)
315315
}
316316

317317
// Create empty resource tree
@@ -1303,11 +1303,14 @@ func TestProcessServiceClusterForGatewayClass(t *testing.T) {
13031303

13041304
r.processServiceClusterForGatewayClass(tc.envoyProxy, tc.gatewayClass, resourceMap)
13051305

1306-
require.Contains(t, resourceMap.allAssociatedBackendRefs, gwapiv1.BackendObjectReference{
1307-
Kind: ptr.To(gwapiv1.Kind("Service")),
1306+
expectedRef := gwapiv1.BackendObjectReference{
1307+
Kind: ptr.To(gwapiv1.Kind(resource.KindService)),
13081308
Namespace: gatewayapi.NamespacePtr(r.namespace),
13091309
Name: gwapiv1.ObjectName(tc.expectedSvcName),
1310-
})
1310+
}
1311+
key := backendRefKey(&expectedRef)
1312+
require.Contains(t, resourceMap.allAssociatedBackendRefs, key)
1313+
require.Equal(t, expectedRef, resourceMap.allAssociatedBackendRefs[key])
13111314
})
13121315
}
13131316
}
@@ -1457,11 +1460,14 @@ func TestProcessServiceClusterForGateway(t *testing.T) {
14571460

14581461
r.processServiceClusterForGateway(tc.envoyProxy, tc.gateway, resourceMap)
14591462

1460-
require.Contains(t, resourceMap.allAssociatedBackendRefs, gwapiv1.BackendObjectReference{
1461-
Kind: ptr.To(gwapiv1.Kind("Service")),
1463+
expectedRef := gwapiv1.BackendObjectReference{
1464+
Kind: ptr.To(gwapiv1.Kind(resource.KindService)),
14621465
Namespace: gatewayapi.NamespacePtr(tc.expectedSvcNamespace),
14631466
Name: gwapiv1.ObjectName(tc.expectedSvcName),
1464-
})
1467+
}
1468+
key := backendRefKey(&expectedRef)
1469+
require.Contains(t, resourceMap.allAssociatedBackendRefs, key)
1470+
require.Equal(t, expectedRef, resourceMap.allAssociatedBackendRefs[key])
14651471
})
14661472
}
14671473
}
@@ -1481,6 +1487,23 @@ func newGatewayAPIReconciler(logger logging.Logger) *gatewayAPIReconciler {
14811487
}
14821488
}
14831489

1490+
func TestProcessBackendRefDeduplicatesLogicalBackend(t *testing.T) {
1491+
logger := logging.DefaultLogger(os.Stdout, egv1a1.LogLevelInfo)
1492+
r := newGatewayAPIReconciler(logger)
1493+
resourceTree := resource.NewResources()
1494+
resourceMap := newResourceMapping()
1495+
1496+
backendRef := gwapiv1.BackendObjectReference{
1497+
Namespace: gatewayapi.NamespacePtr("default"),
1498+
Name: "svc",
1499+
}
1500+
1501+
require.NoError(t, r.processBackendRef(t.Context(), resourceMap, resourceTree, resource.KindHTTPRoute, "default", "route-a", backendRef))
1502+
require.NoError(t, r.processBackendRef(t.Context(), resourceMap, resourceTree, resource.KindHTTPRoute, "default", "route-b", backendRef))
1503+
1504+
require.Len(t, resourceMap.allAssociatedBackendRefs, 1)
1505+
}
1506+
14841507
func TestProcessBackendRefs(t *testing.T) {
14851508
ns := "default"
14861509
ctb := test.GetClusterTrustBundle("fake-ctb")
@@ -1584,7 +1607,7 @@ func TestProcessBackendRefs(t *testing.T) {
15841607
resourceTree := resource.NewResources()
15851608
resourceMap := newResourceMapping()
15861609
backend := tc.backend
1587-
resourceMap.allAssociatedBackendRefs.Insert(gwapiv1.BackendObjectReference{
1610+
resourceMap.insertBackendRef(gwapiv1.BackendObjectReference{
15881611
Kind: gatewayapi.KindPtr(resource.KindBackend),
15891612
Namespace: gatewayapi.NamespacePtr(backend.Namespace),
15901613
Name: gwapiv1.ObjectName(backend.Name),

‎internal/provider/kubernetes/resource.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ type resourceMappings struct {
4646
allAssociatedTCPRoutes sets.Set[string]
4747
// Set for storing UDPRoutes' NamespacedNames attaching to various Gateway objects.
4848
allAssociatedUDPRoutes sets.Set[string]
49-
// Set for storing backendRefs' BackendObjectReference referred by various Route objects.
50-
allAssociatedBackendRefs sets.Set[gwapiv1.BackendObjectReference]
49+
// Map caching BackendObjectReferences keyed by their normalized identifier.
50+
allAssociatedBackendRefs map[utils.NamespacedNameWithGroupKind]gwapiv1.BackendObjectReference
5151
// Set for storing ClientTrafficPolicies' NamespacedNames referred by various Route objects.
5252
allAssociatedClientTrafficPolicies sets.Set[string]
5353
// Set for storing BackendTrafficPolicies' NamespacedNames referred by various Route objects.
@@ -90,7 +90,7 @@ func newResourceMapping() *resourceMappings {
9090
allAssociatedGRPCRoutes: sets.New[string](),
9191
allAssociatedTCPRoutes: sets.New[string](),
9292
allAssociatedUDPRoutes: sets.New[string](),
93-
allAssociatedBackendRefs: sets.New[gwapiv1.BackendObjectReference](),
93+
allAssociatedBackendRefs: make(map[utils.NamespacedNameWithGroupKind]gwapiv1.BackendObjectReference),
9494
allAssociatedClientTrafficPolicies: sets.New[string](),
9595
allAssociatedBackendTrafficPolicies: sets.New[string](),
9696
allAssociatedSecurityPolicies: sets.New[string](),

‎internal/provider/kubernetes/routes_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1302,7 +1302,7 @@ func TestProcessHTTPRoutesWithCustomBackends(t *testing.T) {
13021302
// Verify results
13031303
require.NoError(t, err)
13041304
require.Len(t, resourceMap.extensionRefFilters, tc.expectedExtFiltersCount)
1305-
require.Equal(t, tc.expectedBackendRefsCount, resourceMap.allAssociatedBackendRefs.Len())
1305+
require.Len(t, resourceMap.allAssociatedBackendRefs, tc.expectedBackendRefsCount)
13061306

13071307
// Verify that HTTPRoutes were processed
13081308
require.Len(t, resourceTree.HTTPRoutes, 1)

0 commit comments

Comments
 (0)