enhance policy handling

Author: dspo

charts/values.yaml

@@ -2,7 +2,7 @@ nameOverride: ""
 labelsOverride: {}
 annotations: {}
 podAnnotations: {}
-controllerName: gateway.api7.io/api7-ingress-controller
+controllerName: gateway.apisix.io/api7-ingress-controller
 replicas: 1
 admin:
   key: '' # Pass the admin key generated for the ingress gateway group

config/samples/config.yaml

@@ -1,7 +1,7 @@
 log_level: "info"                               # The log level of the API7 Ingress Controller.
                                                 # the default value is "info".
 
-controller_name: gateway.api7.io/api7-ingress-controller  # The controller name of the API7 Ingress Controller,
+controller_name: gateway.apisix.io/api7-ingress-controller  # The controller name of the API7 Ingress Controller,
                                                           # which is used to identify the controller in the GatewayClass.
                                                           # The default value is "gateway.api7.io/api7-ingress-controller".
 leader_election_id: "api7-ingress-controller-leader" # The leader election ID for the API7 Ingress Controller.

docs/configure.md

@@ -8,7 +8,7 @@ The API7 Ingress Controller is a Kubernetes Ingress Controller that implements t
 log_level: "info"                               # The log level of the API7 Ingress Controller.
                                                 # the default value is "info".
 
-controller_name: gateway.api7.io/api7-ingress-controller  # The controller name of the API7 Ingress Controller,
+controller_name: gateway.apisix.io/api7-ingress-controller  # The controller name of the API7 Ingress Controller,
                                                           # which is used to identify the controller in the GatewayClass.
                                                           # The default value is "gateway.api7.io/api7-ingress-controller".
 

internal/controller/httproutepolicy.go

@@ -52,6 +52,24 @@ func (r *HTTPRouteReconciler) processHTTPRoutePolicies(tctx *provider.TranslateC
 		}
 	}
 
+	// todo: unreachable
+	// if the HTTPRoute is deleted, clear tctx.HTTPRoutePolicies and delete Ancestors from HTTPRoutePolicies status
+	// if !httpRoute.GetDeletionTimestamp().IsZero() {
+	// 	for _, policies := range checker.policies {
+	// 		for i := range policies {
+	// 			policy := policies[i]
+	// 			_ = DeleteAncestors(&policy.Status, httpRoute.Spec.ParentRefs)
+	// 			data, _ := json.Marshal(policy.Status)
+	// 			r.Log.Info("policy status after delete ancestor", "data", string(data))
+	// 			if err := r.Status().Update(context.Background(), &policy); err != nil {
+	// 				r.Log.Error(err, "failed to Update policy status")
+	// 			}
+	// 			// tctx.StatusUpdaters = append(tctx.StatusUpdaters, &policy)
+	// 		}
+	// 	}
+	// 	return nil
+	// }
+
 	var (
 		status  = true
 		reason  = string(v1alpha2.PolicyReasonAccepted)

internal/controller/policies.go

@@ -2,19 +2,23 @@ package controller
 
 import (
 	"fmt"
+	"slices"
 
+	"k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/utils/ptr"
 	gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
 	gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
 
-	"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"
 	"github.com/go-logr/logr"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"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"
 )
 
 type PolicyTargetKey struct {
@@ -115,19 +119,37 @@ func SetAncestors(status *v1alpha1.PolicyStatus, parentRefs []gatewayv1.ParentRe
 }
 
 func SetAncestorStatus(status *v1alpha1.PolicyStatus, ancestorStatus gatewayv1alpha2.PolicyAncestorStatus) bool {
+	if len(ancestorStatus.Conditions) == 0 {
+		return false
+	}
+	condition := ancestorStatus.Conditions[0]
 	for _, c := range status.Ancestors {
-		if c.AncestorRef == ancestorStatus.AncestorRef {
-			if len(c.Conditions) == 0 || len(ancestorStatus.Conditions) == 0 {
-				c.Conditions = ancestorStatus.Conditions
-				return true
+		if parentRefValueEqual(c.AncestorRef, ancestorStatus.AncestorRef) &&
+			c.ControllerName == ancestorStatus.ControllerName {
+			if !VerifyConditions(&c.Conditions, condition) {
+				return false
 			}
-			if c.Conditions[0].ObservedGeneration < ancestorStatus.Conditions[0].ObservedGeneration {
-				c.Conditions = ancestorStatus.Conditions
-				return true
-			}
-			return false
+			meta.SetStatusCondition(&c.Conditions, condition)
+			return true
 		}
 	}
 	status.Ancestors = append(status.Ancestors, ancestorStatus)
 	return true
 }
+
+func DeleteAncestors(status *v1alpha1.PolicyStatus, parentRefs []gatewayv1.ParentReference) bool {
+	var length = len(status.Ancestors)
+	for _, parentRef := range parentRefs {
+		status.Ancestors = slices.DeleteFunc(status.Ancestors, func(status gatewayv1alpha2.PolicyAncestorStatus) bool {
+			return parentRefValueEqual(parentRef, status.AncestorRef)
+		})
+	}
+	return length != len(status.Ancestors)
+}
+
+func parentRefValueEqual(a, b gatewayv1.ParentReference) bool {
+	return ptr.Equal(a.Group, b.Group) &&
+		ptr.Equal(a.Kind, b.Kind) &&
+		ptr.Equal(a.Namespace, b.Namespace) &&
+		a.Name == b.Name
+}

test/e2e/crds/consumer.go

@@ -67,7 +67,7 @@ spec:
     filters:
     - type: ExtensionRef
       extensionRef:
-        group: gateway.api7.io
+        group: gateway.apisix.io
         kind: PluginConfig
         name: auth-plugin-config
     backendRefs:

test/e2e/gatewayapi/httproute.go

@@ -6,8 +6,10 @@ import (
 	"strings"
 	"time"
 
+	"github.com/gruntwork-io/terratest/modules/retry"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"github.com/pkg/errors"
 
 	"github.com/api7/api7-ingress-controller/test/e2e/framework"
 	"github.com/api7/api7-ingress-controller/test/e2e/scaffold"
@@ -290,7 +292,7 @@ spec:
     - name: httpbin-service-e2e-test
       port: 80
 `
-		var httpRoutePolicy = `
+		const httpRoutePolicy = `
 apiVersion: gateway.apisix.io/v1alpha1
 kind: HTTPRoutePolicy
 metadata:
@@ -438,6 +440,18 @@ spec:
 				WithHeader("X-Route-Name", "httpbin").
 				Expect().
 				Status(http.StatusOK)
+		})
+
+		It("HTTPRoutePolicy in effect", func() {
+			By("create HTTPRoute")
+			ResourceApplied("HTTPRoute", "httpbin", varsRoute, 1)
+
+			s.NewAPISIXClient().
+				GET("/get").
+				WithHost("httpbin.example").
+				WithHeader("X-Route-Name", "httpbin").
+				Expect().
+				Status(http.StatusOK)
 
 			By("create HTTPRoutePolicy")
 			ResourceApplied("HTTPRoutePolicy", "http-route-policy-0", httpRoutePolicy, 1)
@@ -458,6 +472,191 @@ spec:
 				WithQuery("hrp_name", "http-route-policy-0").
 				Expect().
 				Status(http.StatusOK)
+
+			By("update HTTPRoutePolicy")
+			const changedHTTPRoutePolicy = `
+apiVersion: gateway.apisix.io/v1alpha1
+kind: HTTPRoutePolicy
+metadata:
+  name: http-route-policy-0
+spec:
+  targetRefs:
+  - group: gateway.networking.k8s.io
+    kind: HTTPRoute
+    name: httpbin
+    # sectionName: get
+  priority: 10
+  vars:
+  - - http_x_hrp_name
+    - ==
+    - new-hrp-name
+`
+			ResourceApplied("HTTPRoutePolicy", "http-route-policy-0", changedHTTPRoutePolicy, 1)
+			// use the old vars cannot match any route
+			s.NewAPISIXClient().
+				GET("/get").
+				WithHost("httpbin.example").
+				WithHeader("X-Route-Name", "httpbin").
+				WithHeader("X-HRP-Name", "http-route-policy-0").
+				WithQuery("hrp_name", "http-route-policy-0").
+				Expect().
+				Status(http.StatusNotFound)
+			// use the new vars can match the route
+			s.NewAPISIXClient().
+				GET("/get").
+				WithHost("httpbin.example").
+				WithHeader("X-Route-Name", "httpbin").
+				WithHeader("X-HRP-Name", "new-hrp-name").
+				Expect().
+				Status(http.StatusOK)
+
+			By("delete the HTTPRoutePolicy")
+			err := s.DeleteResource("HTTPRoutePolicy", "http-route-policy-0")
+			Expect(err).NotTo(HaveOccurred(), "deleting HTTPRoutePolicy")
+			Eventually(func() string {
+				_, err := s.GetResourceYaml("HTTPRoutePolicy", "http-route-policy-0")
+				return err.Error()
+			}).WithTimeout(8 * time.Second).ProbeEvery(time.Second).Should(ContainSubstring(`httproutepolicies.gateway.apisix.io "http-route-policy-0" not found`))
+			// access the route without additional vars should be OK
+			message := retry.DoWithRetry(s.GinkgoT, "", 10, time.Second, func() (string, error) {
+				statusCode := s.NewAPISIXClient().
+					GET("/get").
+					WithHost("httpbin.example").
+					WithHeader("X-Route-Name", "httpbin").
+					Expect().Raw().StatusCode
+				if statusCode != http.StatusOK {
+					return "", errors.Errorf("unexpected status code: %v", statusCode)
+				}
+				return "request OK", nil
+			})
+			s.Logf(message)
+		})
+
+		It("HTTPRoutePolicy conflicts", func() {
+			const httpRoutePolicy0 = `
+apiVersion: gateway.apisix.io/v1alpha1
+kind: HTTPRoutePolicy
+metadata:
+  name: http-route-policy-0
+spec:
+  targetRefs:
+  - group: gateway.networking.k8s.io
+    kind: HTTPRoute
+    name: httpbin
+  priority: 10
+  vars:
+  - - http_x_hrp_name
+    - ==
+    - http-route-policy-0
+`
+			const httpRoutePolicy1 = `
+apiVersion: gateway.apisix.io/v1alpha1
+kind: HTTPRoutePolicy
+metadata:
+  name: http-route-policy-1
+spec:
+  targetRefs:
+  - group: gateway.networking.k8s.io
+    kind: HTTPRoute
+    name: httpbin
+  priority: 20
+  vars:
+  - - http_x_hrp_name
+    - ==
+    - http-route-policy-0
+`
+			const httpRoutePolicy2 = `
+apiVersion: gateway.apisix.io/v1alpha1
+kind: HTTPRoutePolicy
+metadata:
+  name: http-route-policy-2
+spec:
+  targetRefs:
+  - group: gateway.networking.k8s.io
+    kind: HTTPRoute
+    name: httpbin
+  - group: gateway.networking.k8s.io
+    kind: HTTPRoute
+    name: httpbin-1
+  priority: 30
+  vars:
+  - - http_x_hrp_name
+    - ==
+    - http-route-policy-0
+`
+			By("create HTTPRoute")
+			ResourceApplied("HTTPRoute", "httpbin", varsRoute, 1)
+
+			By("create HTTPRoutePolices")
+			for _, spec := range []string{httpRoutePolicy0, httpRoutePolicy1, httpRoutePolicy2} {
+				err := s.CreateResourceFromString(spec)
+				Expect(err).NotTo(HaveOccurred(), "creating HTTPRoutePolicy")
+			}
+			for _, name := range []string{"http-route-policy-0", "http-route-policy-1", "http-route-policy-2"} {
+				Eventually(func() string {
+					spec, err := s.GetResourceYaml("HTTPRoutePolicy", name)
+					Expect(err).NotTo(HaveOccurred(), "getting HTTPRoutePolicy yaml")
+					return spec
+				}).WithTimeout(8 * time.Second).ProbeEvery(time.Second).
+					Should(ContainSubstring("reason: Conflicted"))
+			}
+
+			// assert that conflict policies are not in effect
+			s.NewAPISIXClient().
+				GET("/get").
+				WithHost("httpbin.example").
+				WithHeader("X-Route-Name", "httpbin").
+				Expect().
+				Status(http.StatusOK)
+		})
+
+		PIt("HTTPRoutePolicy status changes on HTTPRoute deleting", func() {
+			By("create HTTPRoute")
+			ResourceApplied("HTTPRoute", "httpbin", varsRoute, 1)
+
+			By("create HTTPRoutePolicy")
+			ResourceApplied("HTTPRoutePolicy", "http-route-policy-0", httpRoutePolicy, 1)
+
+			By("access dataplane to check the HTTPRoutePolicy")
+			s.NewAPISIXClient().
+				GET("/get").
+				WithHost("httpbin.example").
+				WithHeader("X-Route-Name", "httpbin").
+				Expect().
+				Status(http.StatusNotFound)
+
+			s.NewAPISIXClient().
+				GET("/get").
+				WithHost("httpbin.example").
+				WithHeader("X-Route-Name", "httpbin").
+				WithHeader("X-HRP-Name", "http-route-policy-0").
+				WithQuery("hrp_name", "http-route-policy-0").
+				Expect().
+				Status(http.StatusOK)
+
+			By("delete the HTTPRoute, assert the HTTPRoutePolicy's status will be changed")
+			err := s.DeleteResource("HTTPRoute", "httpbin")
+			Expect(err).NotTo(HaveOccurred(), "deleting HTTPRoute")
+			message := retry.DoWithRetry(s.GinkgoT, "request the deleted route", 10, time.Second, func() (string, error) {
+				statusCode := s.NewAPISIXClient().
+					GET("/get").
+					WithHost("httpbin.example").
+					WithHeader("X-Route-Name", "httpbin").
+					WithHeader("X-HRP-Name", "http-route-policy-0").
+					WithQuery("hrp_name", "http-route-policy-0").
+					Expect().Raw().StatusCode
+				if statusCode != http.StatusNotFound {
+					return "", errors.Errorf("unexpected status code: %v", statusCode)
+				}
+				return "the route is deleted", nil
+			})
+			s.Logf(message)
+
+			Eventually(func() string {
+				spec, err := s.GetResourceYaml("HTTPRoutePolicy", "http-route-policy-0")
+				Expect(err).NotTo(HaveOccurred(), "getting HTTPRoutePolicy")
+				return spec
+			}).WithTimeout(8 * time.Second).ProbeEvery(time.Second).ShouldNot(ContainSubstring("ancestorRef:"))
 		})
 	})
 
@@ -661,7 +860,7 @@ spec:
     filters:
     - type: ExtensionRef
       extensionRef:
-        group: gateway.api7.io
+        group: gateway.apisix.io
         kind: PluginConfig
         name: example-plugin-config
     backendRefs: