[feat: gw api] Fix edge cases for lb config finalizers

config/rbac/role.yaml

@@ -135,12 +135,14 @@ rules:
   verbs:
   - get
   - list
+  - patch
   - watch
 - apiGroups:
   - gateway.k8s.aws
   resources:
   - loadbalancerconfigurations/finalizers
   verbs:
+  - patch
   - update
 - apiGroups:
   - gateway.k8s.aws
@@ -157,12 +159,14 @@ rules:
   verbs:
   - get
   - list
+  - patch
   - watch
 - apiGroups:
   - gateway.k8s.aws
   resources:
   - targetgroupconfigurations/finalizers
   verbs:
+  - patch
   - update
 - apiGroups:
   - gateway.k8s.aws
@@ -186,6 +190,7 @@ rules:
   resources:
   - gatewayclasses/finalizers
   verbs:
+  - patch
   - update
 - apiGroups:
   - gateway.networking.k8s.io
@@ -209,6 +214,7 @@ rules:
   resources:
   - gateways/finalizers
   verbs:
+  - patch
   - update
 - apiGroups:
   - gateway.networking.k8s.io

controllers/gateway/config_resolver.go

@@ -6,12 +6,15 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	elbv2gw "sigs.k8s.io/aws-load-balancer-controller/apis/gateway/v1beta1"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/gateway"
+	"sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/gatewayutils"
+	"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
+	"sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	gwv1 "sigs.k8s.io/gateway-api/apis/v1"
 )
 
 type gatewayConfigResolver interface {
-	getLoadBalancerConfigForGateway(ctx context.Context, k8sClient client.Client, gw *gwv1.Gateway, gwClass *gwv1.GatewayClass) (elbv2gw.LoadBalancerConfiguration, error)
+	getLoadBalancerConfigForGateway(ctx context.Context, k8sClient client.Client, finalizerManager k8s.FinalizerManager, gw *gwv1.Gateway, gwClass *gwv1.GatewayClass) (elbv2gw.LoadBalancerConfiguration, error)
 }
 
 type gatewayConfigResolverImpl struct {
@@ -22,11 +25,11 @@ type gatewayConfigResolverImpl struct {
 func newGatewayConfigResolver() gatewayConfigResolver {
 	return &gatewayConfigResolverImpl{
 		configMergeFn:    gateway.NewLoadBalancerConfigMerger().Merge,
-		configResolverFn: resolveLoadBalancerConfig,
+		configResolverFn: gatewayutils.ResolveLoadBalancerConfig,
 	}
 }
 
-func (resolver *gatewayConfigResolverImpl) getLoadBalancerConfigForGateway(ctx context.Context, k8sClient client.Client, gw *gwv1.Gateway, gwClass *gwv1.GatewayClass) (elbv2gw.LoadBalancerConfiguration, error) {
+func (resolver *gatewayConfigResolverImpl) getLoadBalancerConfigForGateway(ctx context.Context, k8sClient client.Client, finalizerManager k8s.FinalizerManager, gw *gwv1.Gateway, gwClass *gwv1.GatewayClass) (elbv2gw.LoadBalancerConfiguration, error) {
 
 	// If the Gateway Class isn't accepted, we shouldn't try to reconcile this Gateway.
 	derivedStatusIndx, ok := deriveAcceptedConditionIndex(gwClass)
@@ -42,6 +45,12 @@ func (resolver *gatewayConfigResolverImpl) getLoadBalancerConfigForGateway(ctx c
 	}
 
 	if gatewayClassLBConfig != nil {
+		// Add finalizers on lb config only when they are referred by gateway indirectly through the gateway class. We call the lb config is in use in such cases.
+		if !k8s.HasFinalizer(gatewayClassLBConfig, shared_constants.LoadBalancerConfigurationFinalizer) {
+			if err := finalizerManager.AddFinalizers(ctx, gatewayClassLBConfig, shared_constants.LoadBalancerConfigurationFinalizer); err != nil {
+				return elbv2gw.LoadBalancerConfiguration{}, errors.Errorf("failed to add finalizers on load balancer configuration %s", k8s.NamespacedName(gatewayClassLBConfig))
+			}
+		}
 		storedVersion := getStoredProcessedConfig(gwClass)
 		if storedVersion == nil || *storedVersion != gatewayClassLBConfig.ResourceVersion {
 			var safeVersion string
@@ -52,24 +61,23 @@ func (resolver *gatewayConfigResolverImpl) getLoadBalancerConfigForGateway(ctx c
 		}
 	}
 
-	var gwParametersRef *gwv1.ParametersReference
-	if gw.Spec.Infrastructure != nil && gw.Spec.Infrastructure.ParametersRef != nil {
-		// Convert local param ref -> namespaced param ref
-		ns := gwv1.Namespace(gw.Namespace)
-		gwParametersRef = &gwv1.ParametersReference{
-			Group:     gw.Spec.Infrastructure.ParametersRef.Group,
-			Kind:      gw.Spec.Infrastructure.ParametersRef.Kind,
-			Name:      gw.Spec.Infrastructure.ParametersRef.Name,
-			Namespace: &ns,
-		}
-	}
+	var gwParametersRef = gatewayutils.GetNamespacedParamRefForGateway(gw)
 
 	gatewayLBConfig, err := resolver.configResolverFn(ctx, k8sClient, gwParametersRef)
 
 	if err != nil {
 		return elbv2gw.LoadBalancerConfiguration{}, err
 	}
 
+	if gatewayLBConfig != nil {
+		// Add finalizers on lb config only when they are referred by gateway directly. We call the lb config is in use in such cases.
+		if !k8s.HasFinalizer(gatewayLBConfig, shared_constants.LoadBalancerConfigurationFinalizer) {
+			if err := finalizerManager.AddFinalizers(ctx, gatewayLBConfig, shared_constants.LoadBalancerConfigurationFinalizer); err != nil {
+				return elbv2gw.LoadBalancerConfiguration{}, errors.Errorf("failed to add finalizers on load balancer configuration %s", k8s.NamespacedName(gatewayLBConfig))
+			}
+		}
+	}
+
 	if gatewayClassLBConfig == nil && gatewayLBConfig == nil {
 		return elbv2gw.LoadBalancerConfiguration{}, nil
 	}

controllers/gateway/config_resolver_test.go

@@ -2,10 +2,14 @@ package gateway
 
 import (
 	"context"
+	"github.com/golang/mock/gomock"
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	elbv2gw "sigs.k8s.io/aws-load-balancer-controller/apis/gateway/v1beta1"
+	mock_client "sigs.k8s.io/aws-load-balancer-controller/mocks/controller-runtime/client"
+	"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
+	"sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	gwv1 "sigs.k8s.io/gateway-api/apis/v1"
 	"testing"
@@ -15,6 +19,11 @@ func Test_getLoadBalancerConfigForGateway(t *testing.T) {
 	mergedConfigName := "mergedConfig"
 	gwClassName := "gwclass"
 	gwName := "gw"
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	k8sClient := mock_client.NewMockClient(ctrl)
+	k8sFinalizerManager := k8s.NewMockFinalizerManager(ctrl)
 
 	testCases := []struct {
 		name             string
@@ -27,12 +36,15 @@ func Test_getLoadBalancerConfigForGateway(t *testing.T) {
 		resolvedGatewayClassConfig *elbv2gw.LoadBalancerConfiguration
 		resolvedGatewayConfig      *elbv2gw.LoadBalancerConfiguration
 
+		setupMocks func()
+
 		expectErr bool
 		expected  elbv2gw.LoadBalancerConfiguration
 	}{
 		{
 			name:              "gw class isnt accepted",
 			inputGatewayClass: &gwv1.GatewayClass{},
+			setupMocks:        func() {},
 			expectErr:         true,
 		},
 		{
@@ -47,7 +59,8 @@ func Test_getLoadBalancerConfigForGateway(t *testing.T) {
 					},
 				},
 			},
-			expectErr: true,
+			setupMocks: func() {},
+			expectErr:  true,
 		},
 		{
 			name: "gw class isnt accepted -- condition is explicitly false",
@@ -61,7 +74,8 @@ func Test_getLoadBalancerConfigForGateway(t *testing.T) {
 					},
 				},
 			},
-			expectErr: true,
+			setupMocks: func() {},
+			expectErr:  true,
 		},
 		{
 			name: "gw class accepted -- fail to get gw class config",
@@ -107,6 +121,11 @@ func Test_getLoadBalancerConfigForGateway(t *testing.T) {
 					ObjectMeta: metav1.ObjectMeta{ResourceVersion: "1"},
 				}, nil
 			},
+			setupMocks: func() {
+				k8sFinalizerManager.EXPECT().
+					AddFinalizers(context.Background(), gomock.Any(), shared_constants.LoadBalancerConfigurationFinalizer).
+					Return(nil)
+			},
 			expectErr: true,
 		},
 		{
@@ -156,6 +175,11 @@ func Test_getLoadBalancerConfigForGateway(t *testing.T) {
 					ObjectMeta: metav1.ObjectMeta{ResourceVersion: "1"},
 				}, nil
 			},
+			setupMocks: func() {
+				k8sFinalizerManager.EXPECT().
+					AddFinalizers(context.Background(), gomock.Any(), shared_constants.LoadBalancerConfigurationFinalizer).
+					Return(nil)
+			},
 			expectErr: true,
 		},
 		{
@@ -188,7 +212,8 @@ func Test_getLoadBalancerConfigForGateway(t *testing.T) {
 				}
 				return nil, errors.New("bad thing")
 			},
-			expected: elbv2gw.LoadBalancerConfiguration{},
+			setupMocks: func() {},
+			expected:   elbv2gw.LoadBalancerConfiguration{},
 		},
 		{
 			name: "gw class accepted -- only gw class configs",
@@ -234,6 +259,11 @@ func Test_getLoadBalancerConfigForGateway(t *testing.T) {
 			expected: elbv2gw.LoadBalancerConfiguration{
 				ObjectMeta: metav1.ObjectMeta{Name: "gwclass", ResourceVersion: "1"},
 			},
+			setupMocks: func() {
+				k8sFinalizerManager.EXPECT().
+					AddFinalizers(context.Background(), gomock.Any(), shared_constants.LoadBalancerConfigurationFinalizer).
+					Return(nil)
+			},
 		},
 		{
 			name: "gw class accepted -- only gw config",
@@ -278,6 +308,11 @@ func Test_getLoadBalancerConfigForGateway(t *testing.T) {
 			expected: elbv2gw.LoadBalancerConfiguration{
 				ObjectMeta: metav1.ObjectMeta{Name: "gw", ResourceVersion: "1"},
 			},
+			setupMocks: func() {
+				k8sFinalizerManager.EXPECT().
+					AddFinalizers(context.Background(), gomock.Any(), shared_constants.LoadBalancerConfigurationFinalizer).
+					Return(nil)
+			},
 		},
 		{
 			name: "gw class accepted -- both gw and gwclass have config - perform merge",
@@ -327,6 +362,11 @@ func Test_getLoadBalancerConfigForGateway(t *testing.T) {
 			expected: elbv2gw.LoadBalancerConfiguration{
 				ObjectMeta: metav1.ObjectMeta{Name: mergedConfigName},
 			},
+			setupMocks: func() {
+				k8sFinalizerManager.EXPECT().
+					AddFinalizers(context.Background(), gomock.Any(), shared_constants.LoadBalancerConfigurationFinalizer).
+					Return(nil).Times(2)
+			},
 		},
 		{
 			name: "gw class accepted -- but processed config version has a mismatch",
@@ -369,6 +409,11 @@ func Test_getLoadBalancerConfigForGateway(t *testing.T) {
 					ObjectMeta: metav1.ObjectMeta{Name: "gwclass", ResourceVersion: "1"},
 				}, nil
 			},
+			setupMocks: func() {
+				k8sFinalizerManager.EXPECT().
+					AddFinalizers(context.Background(), gomock.Any(), shared_constants.LoadBalancerConfigurationFinalizer).
+					Return(nil)
+			},
 			expectErr: true,
 		},
 	}
@@ -384,8 +429,8 @@ func Test_getLoadBalancerConfigForGateway(t *testing.T) {
 				},
 				configResolverFn: tc.configResolverFn,
 			}
-
-			result, err := r.getLoadBalancerConfigForGateway(context.Background(), nil, tc.inputGateway, tc.inputGatewayClass)
+			tc.setupMocks()
+			result, err := r.getLoadBalancerConfigForGateway(context.Background(), k8sClient, k8sFinalizerManager, tc.inputGateway, tc.inputGatewayClass)
 			if tc.expectErr {
 				assert.Error(t, err)
 				return

controllers/gateway/eventhandlers/gateway_class_events.go

@@ -5,6 +5,7 @@ import (
 	"github.com/go-logr/logr"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/workqueue"
+	"sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/gatewayutils"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/event"
@@ -15,23 +16,25 @@ import (
 
 // NewEnqueueRequestsForGatewayClassEvent creates handler for GatewayClass resources
 func NewEnqueueRequestsForGatewayClassEvent(
-	k8sClient client.Client, eventRecorder record.EventRecorder, gwController string, logger logr.Logger) handler.TypedEventHandler[*gatewayv1.GatewayClass, reconcile.Request] {
+	k8sClient client.Client, eventRecorder record.EventRecorder, gwController string, finalizerManager k8s.FinalizerManager, logger logr.Logger) handler.TypedEventHandler[*gatewayv1.GatewayClass, reconcile.Request] {
 	return &enqueueRequestsForGatewayClassEvent{
-		k8sClient:     k8sClient,
-		eventRecorder: eventRecorder,
-		gwController:  gwController,
-		logger:        logger,
+		k8sClient:        k8sClient,
+		finalizerManager: finalizerManager,
+		eventRecorder:    eventRecorder,
+		gwController:     gwController,
+		logger:           logger,
 	}
 }
 
 var _ handler.TypedEventHandler[*gatewayv1.GatewayClass, reconcile.Request] = (*enqueueRequestsForGatewayClassEvent)(nil)
 
 // enqueueRequestsForGatewayClassEvent handles GatewayClass events
 type enqueueRequestsForGatewayClassEvent struct {
-	k8sClient     client.Client
-	eventRecorder record.EventRecorder
-	gwController  string
-	logger        logr.Logger
+	k8sClient        client.Client
+	eventRecorder    record.EventRecorder
+	gwController     string
+	finalizerManager k8s.FinalizerManager
+	logger           logr.Logger
 }
 
 func (h *enqueueRequestsForGatewayClassEvent) Create(ctx context.Context, e event.TypedCreateEvent[*gatewayv1.GatewayClass], queue workqueue.TypedRateLimitingInterface[reconcile.Request]) {
@@ -57,14 +60,13 @@ func (h *enqueueRequestsForGatewayClassEvent) Generic(ctx context.Context, e eve
 }
 
 func (h *enqueueRequestsForGatewayClassEvent) enqueueImpactedGateways(ctx context.Context, gwClass *gatewayv1.GatewayClass, queue workqueue.TypedRateLimitingInterface[reconcile.Request]) {
-	gwList := GetGatewaysManagedByLBController(ctx, h.k8sClient, h.gwController)
+	gwList := gatewayutils.GetGatewaysManagedByGatewayClass(ctx, h.k8sClient, gwClass, h.gwController)
 
 	for _, gw := range gwList {
-		if string(gw.Spec.GatewayClassName) == gwClass.Name {
-			h.logger.V(1).Info("enqueue gateway for gatewayclass event",
-				"gatewayclass", gwClass.GetName(),
-				"gateway", k8s.NamespacedName(gw))
-			queue.Add(reconcile.Request{NamespacedName: k8s.NamespacedName(gw)})
-		}
+		h.logger.V(1).Info("enqueue gateway for gatewayclass event",
+			"gatewayclass", gwClass.GetName(),
+			"gateway", k8s.NamespacedName(gw))
+		queue.Add(reconcile.Request{NamespacedName: k8s.NamespacedName(gw)})
+
 	}
 }

controllers/gateway/eventhandlers/gateway_events.go

@@ -5,6 +5,7 @@ import (
 	"github.com/go-logr/logr"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/workqueue"
+	"sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/gatewayutils"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/event"
@@ -62,7 +63,7 @@ func (h *enqueueRequestsForGatewayEvent) enqueueImpactedGateway(ctx context.Cont
 	if gw == nil {
 		return
 	}
-	if IsGatewayManagedByLBController(ctx, h.k8sClient, gw, h.gwController) {
+	if gatewayutils.IsGatewayManagedByLBController(ctx, h.k8sClient, gw, h.gwController) {
 		h.logger.V(1).Info("enqueue gateway",
 			"gateway", k8s.NamespacedName(gw))
 		queue.Add(reconcile.Request{NamespacedName: k8s.NamespacedName(gw)})