Update HTTPRoutePolicy and Reconciler for Improved Handling

Remove unnecessary `kind` fields and list map keys in the HTTPRoutePolicy CRD. Add a new generic event channel and update predicates for more efficient event handling in the HTTPRouteReconciler.

Author: dspo

api/v1alpha1/httproutepolicy_types.go

@@ -27,10 +27,6 @@ type HTTPRoutePolicySpec struct {
 	// TargetRef identifies an API object (enum: HTTPRoute, Ingress) to apply HTTPRoutePolicy to.
 	//
 	// target references.
-	// +listType=map
-	// +listMapKey=group
-	// +listMapKey=kind
-	// +listMapKey=name
 	// +kubebuilder:validation:MinItems=1
 	// +kubebuilder:validation:MaxItems=16
 	TargetRefs []gatewayv1alpha2.LocalPolicyTargetReferenceWithSectionName `json:"targetRefs"`

config/crd/bases/gateway.apisix.io_httproutepolicies.yaml

@@ -104,11 +104,6 @@ spec:
                 maxItems: 16
                 minItems: 1
                 type: array
-                x-kubernetes-list-map-keys:
-                - group
-                - kind
-                - name
-                x-kubernetes-list-type: map
               vars:
                 items:
                   x-kubernetes-preserve-unknown-fields: true

internal/controller/httproute_controller.go

@@ -9,6 +9,7 @@ import (
 	"github.com/api7/api7-ingress-controller/internal/controller/indexer"
 	"github.com/api7/api7-ingress-controller/internal/provider"
 	"github.com/go-logr/logr"
+	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	discoveryv1 "k8s.io/api/discovery/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -21,6 +22,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+	"sigs.k8s.io/controller-runtime/pkg/source"
 	gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
 	"sigs.k8s.io/gateway-api/apis/v1alpha2"
 )
@@ -33,10 +35,14 @@ type HTTPRouteReconciler struct { //nolint:revive
 	Log logr.Logger
 
 	Provider provider.Provider
+
+	genericEvent chan event.GenericEvent
 }
 
 // SetupWithManager sets up the controller with the Manager.
 func (r *HTTPRouteReconciler) SetupWithManager(mgr ctrl.Manager) error {
+	r.genericEvent = make(chan event.GenericEvent, 100)
+
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&gatewayv1.HTTPRoute{}).
 		WithEventFilter(predicate.GenerationChangedPredicate{}).
@@ -67,10 +73,56 @@ func (r *HTTPRouteReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		).
 		Watches(&v1alpha1.HTTPRoutePolicy{},
 			handler.EnqueueRequestsFromMapFunc(r.listHTTPRouteByHTTPRoutePolicy),
+			builder.WithPredicates(
+				predicate.Funcs{
+					CreateFunc: func(e event.CreateEvent) bool {
+						return true
+					},
+					DeleteFunc: func(e event.DeleteEvent) bool {
+						return true
+					},
+					UpdateFunc: r.httpRoutePolicyPredicateOnUpdate,
+					GenericFunc: func(e event.GenericEvent) bool {
+						return false
+					},
+				},
+			),
+		).
+		WatchesRawSource(
+			source.Channel(
+				r.genericEvent,
+				handler.EnqueueRequestsFromMapFunc(r.listHTTPRouteForGenericEvent),
+			),
 		).
 		Complete(r)
 }
 
+func (r *HTTPRouteReconciler) httpRoutePolicyPredicateOnUpdate(e event.UpdateEvent) bool {
+	oldPolicy, ok0 := e.ObjectOld.(*v1alpha1.HTTPRoutePolicy)
+	newPolicy, ok1 := e.ObjectNew.(*v1alpha1.HTTPRoutePolicy)
+	if !ok0 || !ok1 {
+		return false
+	}
+	var discardsRefs = make(map[string]v1alpha2.LocalPolicyTargetReferenceWithSectionName)
+	for _, ref := range oldPolicy.Spec.TargetRefs {
+		key := indexer.GenHTTPRoutePolicyIndexKey(string(ref.Group), string(ref.Kind), e.ObjectOld.GetNamespace(), string(ref.Name), "")
+		discardsRefs[key] = ref
+	}
+	for _, ref := range newPolicy.Spec.TargetRefs {
+		key := indexer.GenHTTPRoutePolicyIndexKey(string(ref.Group), string(ref.Kind), e.ObjectOld.GetNamespace(), string(ref.Name), "")
+		delete(discardsRefs, key)
+	}
+	if len(discardsRefs) > 0 {
+		dump := oldPolicy.DeepCopy()
+		dump.Spec.TargetRefs = make([]v1alpha2.LocalPolicyTargetReferenceWithSectionName, 0, len(discardsRefs))
+		for _, ref := range discardsRefs {
+			dump.Spec.TargetRefs = append(dump.Spec.TargetRefs, ref)
+		}
+		r.genericEvent <- event.GenericEvent{Object: dump}
+	}
+	return true
+}
+
 func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	hr := new(gatewayv1.HTTPRoute)
 	if err := r.Get(ctx, req.NamespacedName, hr); err != nil {
@@ -264,7 +316,6 @@ func (r *HTTPRouteReconciler) listHTTPRouteByHTTPRoutePolicy(ctx context.Context
 		if ref.Kind == "HTTPRoute" {
 			key := ancestorRefKey{
 				Group:     gatewayv1.GroupName,
-				Kind:      "HTTPRoute",
 				Namespace: gatewayv1.Namespace(obj.GetNamespace()),
 				Name:      ref.Name,
 			}
@@ -277,11 +328,25 @@ func (r *HTTPRouteReconciler) listHTTPRouteByHTTPRoutePolicy(ctx context.Context
 	for key := range keys {
 		var httpRoute gatewayv1.HTTPRoute
 		if err := r.Get(ctx, client.ObjectKey{Namespace: string(key.Namespace), Name: string(key.Name)}, &httpRoute); err != nil {
-			r.Log.Error(err, "failed to get HTTPRoute by HTTPRoutePolicy targetRef", "namespace", obj.GetNamespace(), "name", obj.GetName())
-			r.modifyHTTPRoutePolicyStatus(key, httpRoutePolicy, false, string(v1alpha2.PolicyReasonTargetNotFound), "not found HTTPRoute")
+			r.Log.Error(err, "failed to get HTTPRoute by HTTPRoutePolicy targetRef", "namespace", key.Namespace, "name", key.Name)
+			r.modifyHTTPRoutePolicyStatus(key, httpRoutePolicy, false, string(v1alpha2.PolicyReasonTargetNotFound), "not found target HTTPRoute")
 			r.Log.Info("status after modified", "key", key, "status", httpRoutePolicy.Status.Ancestors)
 			continue
 		}
+		if key.SectionName != "" {
+			var matchRuleName bool
+			for _, rule := range httpRoute.Spec.Rules {
+				if rule.Name != nil && *rule.Name == key.SectionName {
+					matchRuleName = true
+					break
+				}
+			}
+			if !matchRuleName {
+				r.Log.Error(errors.Errorf("failed to get HTTPRoute rule by HTTPRoutePolicy targetRef"), "namespace", key.Namespace, "name", key.Name, "sectionName", key.SectionName)
+				r.modifyHTTPRoutePolicyStatus(key, httpRoutePolicy, false, string(v1alpha2.PolicyReasonTargetNotFound), "not found target HTTPRoute rule")
+				continue
+			}
+		}
 		requests = append(requests, reconcile.Request{
 			NamespacedName: types.NamespacedName{
 				Namespace: string(key.Namespace),
@@ -293,13 +358,34 @@ func (r *HTTPRouteReconciler) listHTTPRouteByHTTPRoutePolicy(ctx context.Context
 	r.Log.Info("status before clear", "status", httpRoutePolicy.Status.Ancestors)
 	r.clearHTTPRoutePolicyRedundantAncestor(httpRoutePolicy)
 	r.Log.Info("status after clear", "status", httpRoutePolicy.Status.Ancestors)
-	if err := r.Status().Update(ctx, httpRoutePolicy); err != nil {
-		r.Log.Error(err, "failed to update HTTPRoutePolicy status", "namespace", obj.GetNamespace(), "name", obj.GetName())
+	if httpRoutePolicy.GetDeletionTimestamp().IsZero() {
+		if err := r.Status().Update(ctx, httpRoutePolicy); err != nil {
+			r.Log.Error(err, "failed to update HTTPRoutePolicy status", "namespace", obj.GetNamespace(), "name", obj.GetName())
+		}
 	}
 
 	return requests
 }
 
+func (r *HTTPRouteReconciler) listHTTPRouteForGenericEvent(ctx context.Context, obj client.Object) []reconcile.Request {
+	switch v := obj.(type) {
+	case *v1alpha1.HTTPRoutePolicy:
+		var (
+			namespacedNames = make(map[types.NamespacedName]struct{})
+			requests        []reconcile.Request
+		)
+		for _, ref := range v.Spec.TargetRefs {
+			namespacedName := types.NamespacedName{Namespace: v.GetNamespace(), Name: string(ref.Name)}
+			if _, ok := namespacedNames[namespacedName]; !ok {
+				namespacedNames[namespacedName] = struct{}{}
+				requests = append(requests, reconcile.Request{NamespacedName: namespacedName})
+			}
+		}
+		return requests
+	}
+	return nil
+}
+
 func (r *HTTPRouteReconciler) processHTTPRouteBackendRefs(tctx *provider.TranslateContext) error {
 	var terr error
 	for _, backend := range tctx.BackendRefs {

internal/controller/httproutepolicy.go

@@ -84,7 +84,6 @@ func (r *HTTPRouteReconciler) clearHTTPRoutePolicyRedundantAncestor(policy *v1al
 	for _, ref := range policy.Spec.TargetRefs {
 		key := ancestorRefKey{
 			Group:     ref.Group,
-			Kind:      ref.Kind,
 			Namespace: gatewayv1.Namespace(policy.GetNamespace()),
 			Name:      ref.Name,
 		}
@@ -103,9 +102,6 @@ func (r *HTTPRouteReconciler) clearHTTPRoutePolicyRedundantAncestor(policy *v1al
 		if ancestor.AncestorRef.Group != nil {
 			key.Group = *ancestor.AncestorRef.Group
 		}
-		if ancestor.AncestorRef.Kind != nil {
-			key.Kind = *ancestor.AncestorRef.Kind
-		}
 		if ancestor.AncestorRef.SectionName != nil {
 			key.SectionName = *ancestor.AncestorRef.SectionName
 		}
@@ -129,7 +125,7 @@ func (r *HTTPRouteReconciler) modifyHTTPRoutePolicyStatus(key ancestorRefKey, po
 	}
 	var hasAncestor bool
 	for i, ancestor := range policy.Status.Ancestors {
-		if ancestor.AncestorRef.Kind != nil && *ancestor.AncestorRef.Kind == key.Kind && ancestor.AncestorRef.Name == key.Name {
+		if ancestor.AncestorRef.Name == key.Name {
 			ancestor.ControllerName = v1alpha2.GatewayController(config.GetControllerName())
 			ancestor.Conditions = []metav1.Condition{condition}
 			hasAncestor = true
@@ -139,7 +135,6 @@ func (r *HTTPRouteReconciler) modifyHTTPRoutePolicyStatus(key ancestorRefKey, po
 	if !hasAncestor {
 		ref := v1alpha2.ParentReference{
 			Group:     &key.Group,
-			Kind:      &key.Kind,
 			Namespace: &key.Namespace,
 			Name:      key.Name,
 		}
@@ -162,7 +157,6 @@ type conflictChecker struct {
 
 type ancestorRefKey struct {
 	Group       gatewayv1.Group
-	Kind        gatewayv1.Kind
 	Namespace   gatewayv1.Namespace
 	Name        gatewayv1.ObjectName
 	SectionName gatewayv1.SectionName
@@ -171,7 +165,6 @@ type ancestorRefKey struct {
 func (c *conflictChecker) append(sectionName string, policy v1alpha1.HTTPRoutePolicy) {
 	key := ancestorRefKey{
 		Group:       gatewayv1.GroupName,
-		Kind:        "HTTPRoute",
 		Namespace:   gatewayv1.Namespace(c.httpRoute.GetNamespace()),
 		Name:        gatewayv1.ObjectName(c.httpRoute.GetName()),
 		SectionName: gatewayv1.SectionName(sectionName),