Refactor HTTPRoutePolicy handling and update tests

This commit refactors the handling of HTTPRoutePolicy, including moving the `httpRoutePolicyPredicateOnUpdate` function and updating the type definitions. It also updates the e2e test for HTTPRoute to correct a typo in the test description.

Author: dspo

api/v1alpha1/httproutepolicy_types.go

@@ -43,8 +43,8 @@ type HTTPRoutePolicy struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
 
-	Spec   HTTPRoutePolicySpec          `json:"spec,omitempty"`
-	Status gatewayv1alpha2.PolicyStatus `json:"status,omitempty"`
+	Spec   HTTPRoutePolicySpec `json:"spec,omitempty"`
+	Status PolicyStatus        `json:"status,omitempty"`
 }
 
 // +kubebuilder:object:root=true

internal/controller/httproute_controller.go

@@ -97,32 +97,6 @@ func (r *HTTPRouteReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		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 {
@@ -221,6 +195,7 @@ func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 	if err := r.Status().Update(ctx, hr); err != nil {
 		return ctrl.Result{}, err
 	}
+	UpdateStatus(r.Client, r.Log, tctx)
 	return ctrl.Result{}, nil
 }
 
@@ -311,10 +286,10 @@ func (r *HTTPRouteReconciler) listHTTPRouteByHTTPRoutePolicy(ctx context.Context
 		return nil
 	}
 
-	var keys = make(map[ancestorRefKey]struct{})
+	var keys = make(map[targetRefKey]struct{})
 	for _, ref := range httpRoutePolicy.Spec.TargetRefs {
 		if ref.Kind == "HTTPRoute" {
-			key := ancestorRefKey{
+			key := targetRefKey{
 				Group:     gatewayv1.GroupName,
 				Namespace: gatewayv1.Namespace(obj.GetNamespace()),
 				Name:      ref.Name,
@@ -329,8 +304,6 @@ func (r *HTTPRouteReconciler) listHTTPRouteByHTTPRoutePolicy(ctx context.Context
 		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", 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 != "" {
@@ -343,7 +316,6 @@ func (r *HTTPRouteReconciler) listHTTPRouteByHTTPRoutePolicy(ctx context.Context
 			}
 			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
 			}
 		}
@@ -355,15 +327,6 @@ 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 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
 }
 
@@ -502,3 +465,29 @@ func (r *HTTPRouteReconciler) processHTTPRoute(tctx *provider.TranslateContext,
 
 	return terror
 }
+
+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
+}

internal/controller/httproutepolicy.go

@@ -2,11 +2,9 @@ package controller
 
 import (
 	"context"
-	"slices"
 	"time"
 
 	"github.com/api7/api7-ingress-controller/api/v1alpha1"
-	"github.com/api7/api7-ingress-controller/internal/controller/config"
 	"github.com/api7/api7-ingress-controller/internal/controller/indexer"
 	"github.com/api7/api7-ingress-controller/internal/provider"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -20,7 +18,7 @@ func (r *HTTPRouteReconciler) processHTTPRoutePolicies(tctx *provider.TranslateC
 	var (
 		checker = conflictChecker{
 			httpRoute: httpRoute,
-			policies:  make(map[ancestorRefKey][]v1alpha1.HTTPRoutePolicy),
+			policies:  make(map[targetRefKey][]v1alpha1.HTTPRoutePolicy),
 		}
 		listForAllRules v1alpha1.HTTPRoutePolicyList
 		key             = indexer.GenHTTPRoutePolicyIndexKey(v1alpha1.GroupVersion.Group, "HTTPRoute", httpRoute.GetNamespace(), httpRoute.GetName(), "")
@@ -67,51 +65,18 @@ func (r *HTTPRouteReconciler) processHTTPRoutePolicies(tctx *provider.TranslateC
 		tctx.HTTPRoutePolicies = make(map[string][]v1alpha1.HTTPRoutePolicy)
 	}
 
-	for key, policies := range checker.policies {
-		for _, policy := range policies {
-			r.modifyHTTPRoutePolicyStatus(key, &policy, status, reason, message)
-			if err := r.Status().Update(context.Background(), &policy); err != nil {
-				r.Log.Error(err, "failed to update HTTPRoutePolicyStatus")
-			}
+	for _, policies := range checker.policies {
+		for i := range policies {
+			policy := policies[i]
+			r.modifyHTTPRoutePolicyStatus(httpRoute, &policy, status, reason, message)
+			tctx.StatusUpdaters = append(tctx.StatusUpdaters, &policy)
 		}
 	}
 
 	return nil
 }
 
-func (r *HTTPRouteReconciler) clearHTTPRoutePolicyRedundantAncestor(policy *v1alpha1.HTTPRoutePolicy) {
-	var keys = make(map[ancestorRefKey]struct{})
-	for _, ref := range policy.Spec.TargetRefs {
-		key := ancestorRefKey{
-			Group:     ref.Group,
-			Namespace: gatewayv1.Namespace(policy.GetNamespace()),
-			Name:      ref.Name,
-		}
-		if ref.SectionName != nil {
-			key.SectionName = *ref.SectionName
-		}
-		r.Log.Info("clearHTTPRoutePolicyRedundantAncestor", "keys[]", key)
-		keys[key] = struct{}{}
-	}
-
-	policy.Status.Ancestors = slices.DeleteFunc(policy.Status.Ancestors, func(ancestor v1alpha2.PolicyAncestorStatus) bool {
-		key := ancestorRefKey{
-			Namespace: gatewayv1.Namespace(policy.GetNamespace()),
-			Name:      ancestor.AncestorRef.Name,
-		}
-		if ancestor.AncestorRef.Group != nil {
-			key.Group = *ancestor.AncestorRef.Group
-		}
-		if ancestor.AncestorRef.SectionName != nil {
-			key.SectionName = *ancestor.AncestorRef.SectionName
-		}
-		r.Log.Info("clearHTTPRoutePolicyRedundantAncestor", "key", key)
-		_, ok := keys[key]
-		return !ok
-	})
-}
-
-func (r *HTTPRouteReconciler) modifyHTTPRoutePolicyStatus(key ancestorRefKey, policy *v1alpha1.HTTPRoutePolicy, status bool, reason, message string) {
+func (r *HTTPRouteReconciler) modifyHTTPRoutePolicyStatus(httpRoute *gatewayv1.HTTPRoute, policy *v1alpha1.HTTPRoutePolicy, status bool, reason, message string) {
 	condition := metav1.Condition{
 		Type:               string(v1alpha2.PolicyConditionAccepted),
 		Status:             metav1.ConditionTrue,
@@ -123,47 +88,24 @@ func (r *HTTPRouteReconciler) modifyHTTPRoutePolicyStatus(key ancestorRefKey, po
 	if !status {
 		condition.Status = metav1.ConditionFalse
 	}
-	var hasAncestor bool
-	for i, ancestor := range policy.Status.Ancestors {
-		if ancestor.AncestorRef.Name == key.Name {
-			ancestor.ControllerName = v1alpha2.GatewayController(config.GetControllerName())
-			ancestor.Conditions = []metav1.Condition{condition}
-			hasAncestor = true
-		}
-		policy.Status.Ancestors[i] = ancestor
-	}
-	if !hasAncestor {
-		ref := v1alpha2.ParentReference{
-			Group:     &key.Group,
-			Namespace: &key.Namespace,
-			Name:      key.Name,
-		}
-		if key.SectionName != "" {
-			ref.SectionName = &key.SectionName
-		}
-		policy.Status.Ancestors = append(policy.Status.Ancestors, v1alpha2.PolicyAncestorStatus{
-			AncestorRef:    ref,
-			ControllerName: v1alpha2.GatewayController(config.GetControllerName()),
-			Conditions:     []metav1.Condition{condition},
-		})
-	}
+	_ = SetAncestors(&policy.Status, httpRoute.Spec.ParentRefs, condition)
 }
 
 type conflictChecker struct {
 	httpRoute *gatewayv1.HTTPRoute
-	policies  map[ancestorRefKey][]v1alpha1.HTTPRoutePolicy
+	policies  map[targetRefKey][]v1alpha1.HTTPRoutePolicy
 	conflict  bool
 }
 
-type ancestorRefKey struct {
+type targetRefKey struct {
 	Group       gatewayv1.Group
 	Namespace   gatewayv1.Namespace
 	Name        gatewayv1.ObjectName
 	SectionName gatewayv1.SectionName
 }
 
 func (c *conflictChecker) append(sectionName string, policy v1alpha1.HTTPRoutePolicy) {
-	key := ancestorRefKey{
+	key := targetRefKey{
 		Group:       gatewayv1.GroupName,
 		Namespace:   gatewayv1.Namespace(c.httpRoute.GetNamespace()),
 		Name:        gatewayv1.ObjectName(c.httpRoute.GetName()),

test/e2e/gatewayapi/httproute.go

@@ -421,7 +421,7 @@ spec:
 				Status(404)
 		})
 
-		FIt("HTTPRoute Vars Match", func() {
+		It("HTTPRoute Vars Match", func() {
 			By("create HTTPRoute")
 			ResourceApplied("HTTPRoute", "httpbin", varsRoute, 1)