refactor: Centralize and streamline ReferenceGrant validation

Reorganized and simplified the predicate logic for ReferenceGrant handling across gateway and HTTPRoute controllers. Consolidated duplicate code into reusable functions, reducing redundancy and improving maintainability. This centralization ensures consistent behavior and clearer code structure.

Author: dspo

internal/controller/gateway_controller.go

@@ -27,7 +27,6 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -87,20 +86,7 @@ func (r *GatewayReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		).
 		Watches(&v1beta1.ReferenceGrant{},
 			handler.EnqueueRequestsFromMapFunc(r.listReferenceGrantsForGateway),
-			builder.WithPredicates(predicate.Funcs{
-				CreateFunc: func(e event.CreateEvent) bool {
-					return referenceGrantHasGatewayFrom(e.Object)
-				},
-				UpdateFunc: func(e event.UpdateEvent) bool {
-					return referenceGrantHasGatewayFrom(e.ObjectOld) || referenceGrantHasGatewayFrom(e.ObjectNew)
-				},
-				DeleteFunc: func(e event.DeleteEvent) bool {
-					return referenceGrantHasGatewayFrom(e.Object)
-				},
-				GenericFunc: func(e event.GenericEvent) bool {
-					return referenceGrantHasGatewayFrom(e.Object)
-				},
-			}),
+			builder.WithPredicates(referenceGrantPredicates(KindGateway)),
 		).
 		Complete(r)
 }
@@ -190,7 +176,7 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 	}
 	listenerStatuses, err := getListenerStatus(ctx, r.Client, gateway, referenceGrantList.Items)
 	if err != nil {
-		r.Log.Error(err, "failed to get listener status", "gateway", types.NamespacedName{Namespace: gateway.GetNamespace(), Name: gateway.GetName()})
+		r.Log.Error(err, "failed to get listener status", "gateway", types.NamespacedName{Namespace: gateway.GetNamespace()})
 		return ctrl.Result{}, err
 	}
 
@@ -390,12 +376,12 @@ func (r *GatewayReconciler) listReferenceGrantsForGateway(ctx context.Context, o
 	}
 
 	for _, gateway := range gatewayList.Items {
+		gw := v1beta1.ReferenceGrantFrom{
+			Group:     gatewayv1.GroupName,
+			Kind:      KindGateway,
+			Namespace: v1beta1.Namespace(gateway.GetNamespace()),
+		}
 		for _, from := range grant.Spec.From {
-			gw := v1beta1.ReferenceGrantFrom{
-				Group:     gatewayv1.GroupName,
-				Kind:      KindGateway,
-				Namespace: v1beta1.Namespace(gateway.GetNamespace()),
-			}
 			if from == gw {
 				requests = append(requests, reconcile.Request{
 					NamespacedName: types.NamespacedName{
@@ -409,19 +395,6 @@ func (r *GatewayReconciler) listReferenceGrantsForGateway(ctx context.Context, o
 	return requests
 }
 
-func referenceGrantHasGatewayFrom(obj client.Object) bool {
-	grant, ok := obj.(*v1beta1.ReferenceGrant)
-	if !ok {
-		return false
-	}
-	for _, from := range grant.Spec.From {
-		if from.Kind == KindGateway && string(from.Group) == gatewayv1.GroupName {
-			return true
-		}
-	}
-	return false
-}
-
 func (r *GatewayReconciler) processInfrastructure(tctx *provider.TranslateContext, gateway *gatewayv1.Gateway) error {
 	rk := provider.ResourceKind{
 		Kind:      gateway.Kind,

internal/controller/httproute_controller.go

@@ -104,7 +104,7 @@ func (r *HTTPRouteReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		).
 		Watches(&v1beta1.ReferenceGrant{},
 			handler.EnqueueRequestsFromMapFunc(r.lisHTTPRoutesForReferenceGrant),
-			builder.WithPredicates(httpRouteReferenceGrantPredicates()),
+			builder.WithPredicates(referenceGrantPredicates(KindHTTPRoute)),
 		).
 		WatchesRawSource(
 			source.Channel(
@@ -464,7 +464,19 @@ func (r *HTTPRouteReconciler) processHTTPRouteBackendRefs(tctx *provider.Transla
 				r.Log.Error(err, "failed to list ReferenceGrants", "namespace", targetNN.Namespace)
 				return err
 			}
-			if !checkReferenceGrantBetweenHTTPRouteAndService(hrNN, targetNN, referenceGrantList.Items) {
+			if !checkReferenceGrant(
+				v1beta1.ReferenceGrantFrom{
+					Group:     gatewayv1.GroupName,
+					Kind:      KindHTTPRoute,
+					Namespace: v1beta1.Namespace(hrNN.Namespace),
+				},
+				v1beta1.ReferenceGrantTo{
+					Group: corev1.GroupName,
+					Kind:  KindService,
+					Name:  (*gatewayv1.ObjectName)(&targetNN.Name),
+				},
+				referenceGrantList.Items,
+			) {
 				terr = ReasonError{
 					Reason:  string(v1beta1.RouteReasonRefNotPermitted),
 					Message: fmt.Sprintf("%s is in a different namespace than the HTTPRoute %s and no ReferenceGrant allowing reference is configured", targetNN, hrNN),
@@ -639,12 +651,12 @@ func (r *HTTPRouteReconciler) lisHTTPRoutesForReferenceGrant(ctx context.Context
 	}
 
 	for _, httpRoute := range httpRouteList.Items {
+		hr := v1beta1.ReferenceGrantFrom{
+			Group:     gatewayv1.GroupName,
+			Kind:      KindHTTPRoute,
+			Namespace: v1beta1.Namespace(httpRoute.GetNamespace()),
+		}
 		for _, from := range grant.Spec.From {
-			hr := v1beta1.ReferenceGrantFrom{
-				Group:     gatewayv1.GroupName,
-				Kind:      KindHTTPRoute,
-				Namespace: v1beta1.Namespace(httpRoute.GetNamespace()),
-			}
 			if from == hr {
 				requests = append(requests, reconcile.Request{
 					NamespacedName: client.ObjectKey{
@@ -657,43 +669,3 @@ func (r *HTTPRouteReconciler) lisHTTPRoutesForReferenceGrant(ctx context.Context
 	}
 	return requests
 }
-
-func httpRouteReferenceGrantPredicates() predicate.Funcs {
-	var filter = func(obj client.Object) bool {
-		grant, ok := obj.(*v1beta1.ReferenceGrant)
-		if !ok {
-			return false
-		}
-		for _, from := range grant.Spec.From {
-			if from.Kind == KindHTTPRoute && string(from.Group) == gatewayv1.GroupName {
-				return true
-			}
-		}
-		return false
-	}
-	predicates := predicate.NewPredicateFuncs(filter)
-	predicates.UpdateFunc = func(e event.UpdateEvent) bool {
-		return filter(e.ObjectOld) || filter(e.ObjectNew)
-	}
-	return predicates
-}
-
-func checkReferenceGrantBetweenHTTPRouteAndService(hrNN, targetNN types.NamespacedName, grants []v1beta1.ReferenceGrant) bool {
-	hr := v1beta1.ReferenceGrantFrom{
-		Group:     gatewayv1.GroupName,
-		Kind:      KindHTTPRoute,
-		Namespace: v1beta1.Namespace(hrNN.Namespace),
-	}
-	for _, grant := range grants {
-		fromHTTPRoute := slices.ContainsFunc(grant.Spec.From, func(from v1beta1.ReferenceGrantFrom) bool {
-			return from == hr
-		})
-		toService := slices.ContainsFunc(grant.Spec.To, func(to v1beta1.ReferenceGrantTo) bool {
-			return to.Group == corev1.GroupName && to.Kind == KindService && (to.Name == nil || string(*to.Name) == targetNN.Name)
-		})
-		if fromHTTPRoute && toService {
-			return true
-		}
-	}
-	return false
-}

internal/controller/utils.go

@@ -13,6 +13,7 @@
 package controller
 
 import (
+	"cmp"
 	"context"
 	"encoding/pem"
 	"errors"
@@ -31,6 +32,8 @@ import (
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/event"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
 	"sigs.k8s.io/gateway-api/apis/v1beta1"
@@ -736,19 +739,32 @@ func getListenerStatus(
 					conditionProgrammed.Reason = string(gatewayv1.ListenerReasonInvalid)
 					break
 				}
-				if ok := checkReferenceGrantBetweenGatewayAndSecret(gateway.Namespace, ref, grants); !ok {
-					conditionResolvedRefs.Status = metav1.ConditionFalse
-					conditionResolvedRefs.Reason = string(gatewayv1.ListenerReasonRefNotPermitted)
-					conditionResolvedRefs.Message = "certificateRefs cross namespaces is not permitted"
-					conditionProgrammed.Status = metav1.ConditionFalse
-					conditionProgrammed.Reason = string(gatewayv1.ListenerReasonInvalid)
-					break
-				}
-				ns := gateway.Namespace
-				if ref.Namespace != nil {
-					ns = string(*ref.Namespace)
+				// if cross namespaces, check if the Gateway has the permission to access the Secret
+				if ref.Namespace != nil && string(*ref.Namespace) != gateway.Namespace {
+					if ok := checkReferenceGrant(
+						v1beta1.ReferenceGrantFrom{
+							Group:     gatewayv1.GroupName,
+							Kind:      KindGateway,
+							Namespace: v1beta1.Namespace(gateway.Namespace),
+						},
+						v1beta1.ReferenceGrantTo{
+							Group: corev1.GroupName,
+							Kind:  KindSecret,
+							Name:  &ref.Name,
+						},
+						grants,
+					); !ok {
+						conditionResolvedRefs.Status = metav1.ConditionFalse
+						conditionResolvedRefs.Reason = string(gatewayv1.ListenerReasonRefNotPermitted)
+						conditionResolvedRefs.Message = "certificateRefs cross namespaces is not permitted"
+						conditionProgrammed.Status = metav1.ConditionFalse
+						conditionProgrammed.Reason = string(gatewayv1.ListenerReasonInvalid)
+						break
+					}
 				}
-				if err := mrgc.Get(ctx, client.ObjectKey{Namespace: ns, Name: string(ref.Name)}, &secret); err != nil {
+
+				ns := cmp.Or(ref.Namespace, (*gatewayv1.Namespace)(&gateway.Namespace))
+				if err := mrgc.Get(ctx, client.ObjectKey{Namespace: string(*ns), Name: string(ref.Name)}, &secret); err != nil {
 					conditionResolvedRefs.Status = metav1.ConditionFalse
 					conditionResolvedRefs.Reason = string(gatewayv1.ListenerReasonInvalidCertificateRef)
 					conditionResolvedRefs.Message = err.Error()
@@ -1070,28 +1086,36 @@ func isTLSSecretValid(secret *corev1.Secret) (string, bool) {
 	return "", true
 }
 
-func checkReferenceGrantBetweenGatewayAndSecret(gwNamespace string, certRef gatewayv1.SecretObjectReference, grants []v1beta1.ReferenceGrant) bool {
-	// if not cross namespaces
-	if certRef.Namespace == nil || string(*certRef.Namespace) == gwNamespace {
-		return true
+func referenceGrantPredicates(kind gatewayv1.Kind) predicate.Funcs {
+	var filter = func(obj client.Object) bool {
+		grant, ok := obj.(*v1beta1.ReferenceGrant)
+		if !ok {
+			return false
+		}
+		for _, from := range grant.Spec.From {
+			if from.Kind == kind && string(from.Group) == gatewayv1.GroupName {
+				return true
+			}
+		}
+		return false
+	}
+	predicates := predicate.NewPredicateFuncs(filter)
+	predicates.UpdateFunc = func(e event.UpdateEvent) bool {
+		return filter(e.ObjectOld) || filter(e.ObjectNew)
 	}
+	return predicates
+}
 
+func checkReferenceGrant(from v1beta1.ReferenceGrantFrom, to v1beta1.ReferenceGrantTo, grants []v1beta1.ReferenceGrant) bool {
 	for _, grant := range grants {
-		if grant.Namespace == string(*certRef.Namespace) {
-			for _, from := range grant.Spec.From {
-				gw := v1beta1.ReferenceGrantFrom{
-					Group:     gatewayv1.GroupName,
-					Kind:      KindGateway,
-					Namespace: v1beta1.Namespace(gwNamespace),
-				}
-				if from == gw {
-					for _, to := range grant.Spec.To {
-						if to.Group == corev1.GroupName && to.Kind == KindSecret && (to.Name == nil || *to.Name == certRef.Name) {
-							return true
-						}
-					}
-				}
-			}
+		grantFrom := slices.ContainsFunc(grant.Spec.From, func(item v1beta1.ReferenceGrantFrom) bool {
+			return item == from
+		})
+		grantTo := slices.ContainsFunc(grant.Spec.To, func(item v1beta1.ReferenceGrantTo) bool {
+			return item.Group == to.Group && item.Kind == to.Kind && to.Name != nil && (item.Name == nil || *item.Name == *to.Name)
+		})
+		if grantFrom && grantTo {
+			return true
 		}
 	}
 	return false