fix: Set parent namespace in HTTPRoutePolicy and refactor related tests

Makefile

@@ -13,7 +13,7 @@ KIND_NAME ?= apisix-ingress-cluster
 GATEAY_API_VERSION ?= v1.2.0
 
 DASHBOARD_VERSION ?= dev
-TEST_TIMEOUT ?= 45m
+TEST_TIMEOUT ?= 60m
 
 # CRD Reference Documentation
 CRD_REF_DOCS_VERSION ?= v0.1.0

internal/controller/httproutepolicy.go

@@ -46,6 +46,16 @@ func (r *HTTPRouteReconciler) processHTTPRoutePolicies(tctx *provider.TranslateC
 		return nil
 	}
 
+	// set namespace in prentRef if it is not set
+	var parentRefs = make([]gatewayv1.ParentReference, len(httpRoute.Spec.ParentRefs))
+	for i := range httpRoute.Spec.ParentRefs {
+		ref := httpRoute.Spec.ParentRefs[i]
+		if ref.Namespace == nil || *ref.Namespace == "" {
+			ref.Namespace = (*gatewayv1.Namespace)(&httpRoute.Namespace)
+		}
+		parentRefs[i] = ref
+	}
+
 	var conflicts = make(map[types.NamespacedName]v1alpha1.HTTPRoutePolicy)
 	for _, rule := range httpRoute.Spec.Rules {
 		var policies = findPoliciesWhichTargetRefTheRule(rule.Name, "HTTPRoute", list)
@@ -71,7 +81,7 @@ func (r *HTTPRouteReconciler) processHTTPRoutePolicies(tctx *provider.TranslateC
 			tctx.HTTPRoutePolicies = append(tctx.HTTPRoutePolicies, policy)
 		}
 
-		if updated := setAncestorsForHTTPRoutePolicyStatus(httpRoute.Spec.ParentRefs, &policy, condition); updated {
+		if updated := setAncestorsForHTTPRoutePolicyStatus(parentRefs, &policy, condition); updated {
 			tctx.StatusUpdaters = append(tctx.StatusUpdaters, status.Update{
 				NamespacedName: NamespacedName(&policy),
 				Resource:       policy.DeepCopy(),

test/e2e/framework/assertion.go

@@ -0,0 +1,123 @@
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package framework
+
+import (
+	"context"
+	"fmt"
+	"log"
+	"slices"
+	"strings"
+	"time"
+
+	"github.com/gruntwork-io/terratest/modules/testing"
+	"github.com/pkg/errors"
+	"github.com/stretchr/testify/require"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/wait"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
+	"sigs.k8s.io/gateway-api/conformance/utils/kubernetes"
+
+	"github.com/apache/apisix-ingress-controller/api/v1alpha1"
+)
+
+func HTTPRouteMustHaveCondition(t testing.TestingT, cli client.Client, timeout time.Duration, refNN, hrNN types.NamespacedName, condition metav1.Condition) {
+	err := PollUntilHTTPRouteHaveStatus(cli, timeout, hrNN, func(hr *gatewayv1.HTTPRoute) bool {
+		for _, parent := range hr.Status.Parents {
+			if err := kubernetes.ConditionsHaveLatestObservedGeneration(hr, parent.Conditions); err != nil {
+				log.Printf("HTTPRoute %s (parentRef=%v) %v", hrNN, parentRefToString(parent.ParentRef), err)
+				return false
+			}
+			if (refNN.Name == "" || parent.ParentRef.Name == gatewayv1.ObjectName(refNN.Name)) &&
+				(refNN.Namespace == "" || (parent.ParentRef.Namespace != nil && string(*parent.ParentRef.Namespace) == refNN.Namespace)) {
+				if findConditionInList(parent.Conditions, condition) {
+					log.Printf("found condition %v in %v for %s reference %s", condition, parent.Conditions, hrNN, refNN)
+					return true
+				} else {
+					log.Printf("NOT FOUND condition %v in %v for %s reference %s", condition, parent.Conditions, hrNN, refNN)
+				}
+			}
+		}
+		return false
+	})
+	require.NoError(t, err, "error waiting for HTTPRoute status to have a Condition matching %+v", condition)
+}
+
+func PollUntilHTTPRouteHaveStatus(cli client.Client, timeout time.Duration, hrNN types.NamespacedName, f func(route *gatewayv1.HTTPRoute) bool) error {
+	if err := gatewayv1.Install(cli.Scheme()); err != nil {
+		return err
+	}
+	return genericPollResource(new(gatewayv1.HTTPRoute), cli, timeout, hrNN, f)
+}
+
+func HTTPRoutePolicyMustHaveCondition(t testing.TestingT, client client.Client, timeout time.Duration, refNN, hrpNN types.NamespacedName,
+	condition metav1.Condition) {
+	err := PollUntilHTTPRoutePolicyHaveStatus(client, timeout, hrpNN, func(httpRoutePolicy *v1alpha1.HTTPRoutePolicy) bool {
+		for _, ancestor := range httpRoutePolicy.Status.Ancestors {
+			if err := kubernetes.ConditionsHaveLatestObservedGeneration(httpRoutePolicy, ancestor.Conditions); err != nil {
+				log.Printf("HTTPRoutePolicy %s (parentRef=%v) %v", hrpNN, parentRefToString(ancestor.AncestorRef), err)
+				return false
+			}
+
+			if ancestor.AncestorRef.Name == gatewayv1.ObjectName(refNN.Name) &&
+				(refNN.Namespace == "" || (ancestor.AncestorRef.Namespace != nil && string(*ancestor.AncestorRef.Namespace) == refNN.Namespace)) {
+				if findConditionInList(ancestor.Conditions, condition) {
+					log.Printf("found condition %v in list %v for %s reference %s", condition, ancestor.Conditions, hrpNN, refNN)
+					return true
+				} else {
+					log.Printf("NOT FOUND condition %v in %v for %s reference %s", condition, ancestor.Conditions, hrpNN, refNN)
+				}
+			}
+		}
+		return false
+	})
+
+	require.NoError(t, err, "error waiting for HTTPRoutePolicy %s status to have a Condition matching %+v", hrpNN, condition)
+}
+
+func PollUntilHTTPRoutePolicyHaveStatus(cli client.Client, timeout time.Duration, hrpNN types.NamespacedName,
+	f func(httpRoutePolicy *v1alpha1.HTTPRoutePolicy) bool) error {
+	if err := v1alpha1.AddToScheme(cli.Scheme()); err != nil {
+		return err
+	}
+	return genericPollResource(new(v1alpha1.HTTPRoutePolicy), cli, timeout, hrpNN, f)
+}
+
+func parentRefToString(p gatewayv1.ParentReference) string {
+	if p.Namespace != nil && *p.Namespace != "" {
+		return fmt.Sprintf("%v/%v", p.Namespace, p.Name)
+	}
+	return string(p.Name)
+}
+
+func findConditionInList(conditions []metav1.Condition, expected metav1.Condition) bool {
+	return slices.ContainsFunc(conditions, func(item metav1.Condition) bool {
+		// an empty Status string means "Match any status".
+		// an empty Reason string means "Match any reason".
+		return expected.Type == item.Type &&
+			(expected.Status == "" || expected.Status == item.Status) &&
+			(expected.Reason == "" || expected.Reason == item.Reason) &&
+			(expected.Message == "" || strings.Contains(item.Message, expected.Message))
+	})
+}
+
+func genericPollResource[Obj client.Object](obj Obj, cli client.Client, timeout time.Duration, nn types.NamespacedName, predicate func(Obj) bool) error {
+	return wait.PollUntilContextTimeout(context.Background(), time.Second, timeout, true, func(ctx context.Context) (done bool, err error) {
+		if err := cli.Get(ctx, nn, obj); err != nil {
+			return false, errors.Wrapf(err, "error fetching Object %s", nn)
+		}
+		return predicate(obj), nil
+	})
+}

test/e2e/framework/utils.go

@@ -20,32 +20,20 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
-	"log"
 	"net/http"
 	"net/url"
-	"slices"
 	"strings"
 	"sync"
 	"time"
 
 	"github.com/gavv/httpexpect/v2"
-	"github.com/gruntwork-io/terratest/modules/testing"
 	"github.com/onsi/gomega"
-	"github.com/pkg/errors"
-	"github.com/stretchr/testify/require"
 	"golang.org/x/net/html"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/types"
-	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/tools/remotecommand"
 	"k8s.io/utils/ptr"
-	"sigs.k8s.io/controller-runtime/pkg/client"
-	gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
-	"sigs.k8s.io/gateway-api/conformance/utils/kubernetes"
-
-	"github.com/apache/apisix-ingress-controller/api/v1alpha1"
 )
 
 func (f *Framework) NewExpectResponse(httpBody any) *httpexpect.Response {
@@ -381,43 +369,6 @@ func CreateTestZipFile(sourceCode, metadata string) ([]byte, error) {
 	return zipBuffer.Bytes(), nil
 }
 
-func HTTPRoutePolicyMustHaveCondition(t testing.TestingT, client client.Client, timeout time.Duration, refNN, hrpNN types.NamespacedName,
-	condition metav1.Condition) {
-	err := EventuallyHTTPRoutePolicyHaveStatus(client, timeout, hrpNN, func(httpRoutePolicy v1alpha1.HTTPRoutePolicy, status v1alpha1.PolicyStatus) bool {
-		for _, ancestor := range status.Ancestors {
-			if err := kubernetes.ConditionsHaveLatestObservedGeneration(&httpRoutePolicy, ancestor.Conditions); err != nil {
-				log.Printf("HTTPRoutePolicy %s (parentRef=%v) %v", hrpNN, parentRefToString(ancestor.AncestorRef), err)
-				return false
-			}
-
-			if ancestor.AncestorRef.Name == gatewayv1.ObjectName(refNN.Name) &&
-				(ancestor.AncestorRef.Namespace == nil || refNN.Namespace == "" || string(*ancestor.AncestorRef.Namespace) == refNN.Namespace) {
-				if findConditionInList(ancestor.Conditions, condition) {
-					log.Printf("found condition %v in list [%v] for %s reference %s", condition, ancestor.Conditions, hrpNN, refNN)
-					return true
-				} else {
-					log.Printf("not found condition %v in list [%v] for %s reference %s", condition, ancestor.Conditions, hrpNN, refNN)
-				}
-			}
-		}
-		return false
-	})
-
-	require.NoError(t, err, "error waiting for HTTPRoutePolicy status to have a Condition matching expectations")
-}
-
-func EventuallyHTTPRoutePolicyHaveStatus(client client.Client, timeout time.Duration, hrpNN types.NamespacedName,
-	f func(httpRoutePolicy v1alpha1.HTTPRoutePolicy, status v1alpha1.PolicyStatus) bool) error {
-	_ = v1alpha1.AddToScheme(client.Scheme())
-	return wait.PollUntilContextTimeout(context.Background(), time.Second, timeout, true, func(ctx context.Context) (done bool, err error) {
-		var httpRoutePolicy v1alpha1.HTTPRoutePolicy
-		if err = client.Get(ctx, hrpNN, &httpRoutePolicy); err != nil {
-			return false, errors.Errorf("error fetching HTTPRoutePolicy %v: %v", hrpNN, err)
-		}
-		return f(httpRoutePolicy, httpRoutePolicy.Status), nil
-	})
-}
-
 func addFileToZip(zipWriter *zip.Writer, fileName, fileContent string) error {
 	file, err := zipWriter.Create(fileName)
 	if err != nil {
@@ -427,18 +378,3 @@ func addFileToZip(zipWriter *zip.Writer, fileName, fileContent string) error {
 	_, err = file.Write([]byte(fileContent))
 	return err
 }
-
-func parentRefToString(p gatewayv1.ParentReference) string {
-	if p.Namespace != nil && *p.Namespace != "" {
-		return fmt.Sprintf("%v/%v", p.Namespace, p.Name)
-	}
-	return string(p.Name)
-}
-
-func findConditionInList(conditions []metav1.Condition, expected metav1.Condition) bool {
-	return slices.ContainsFunc(conditions, func(item metav1.Condition) bool {
-		// an empty Status string means "Match any status".
-		// an empty Reason string means "Match any reason".
-		return expected.Type == item.Type && (expected.Status == "" || expected.Status == item.Status) && (expected.Reason == "" || expected.Reason == item.Reason)
-	})
-}

test/e2e/gatewayapi/httproute.go

@@ -27,6 +27,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/gateway-api/apis/v1alpha2"
 
+	"github.com/apache/apisix-ingress-controller/api/v1alpha1"
 	"github.com/apache/apisix-ingress-controller/test/e2e/framework"
 	"github.com/apache/apisix-ingress-controller/test/e2e/scaffold"
 )
@@ -805,25 +806,23 @@ spec:
 
 		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)
+			s.ApplyHTTPRoute(types.NamespacedName{Namespace: s.Namespace(), Name: "httpbin"}, varsRoute)
+			request := func() int {
+				return s.NewAPISIXClient().GET("/get").
+					WithHost("httpbin.example").WithHeader("X-Route-Name", "httpbin").
+					Expect().Raw().StatusCode
+			}
+			Eventually(request).WithTimeout(5 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusOK))
 
 			By("create HTTPRoutePolicy")
-			ResourceApplied("HTTPRoutePolicy", "http-route-policy-0", httpRoutePolicy, 1)
+			s.ApplyHTTPRoutePolicy(
+				types.NamespacedName{Name: "apisix"},
+				types.NamespacedName{Namespace: s.Namespace(), Name: "http-route-policy-0"},
+				httpRoutePolicy,
+			)
 
 			By("access dataplane to check the HTTPRoutePolicy")
-			s.NewAPISIXClient().
-				GET("/get").
-				WithHost("httpbin.example").
-				WithHeader("X-Route-Name", "httpbin").
-				Expect().
-				Status(http.StatusNotFound)
+			Eventually(request).WithTimeout(5 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusNotFound))
 
 			s.NewAPISIXClient().
 				GET("/get").
@@ -852,7 +851,12 @@ spec:
     - ==
     - new-hrp-name
 `
-			ResourceApplied("HTTPRoutePolicy", "http-route-policy-0", changedHTTPRoutePolicy, 1)
+			s.ApplyHTTPRoutePolicy(
+				types.NamespacedName{Name: "apisix"},
+				types.NamespacedName{Namespace: s.Namespace(), Name: "http-route-policy-0"},
+				changedHTTPRoutePolicy,
+			)
+
 			// use the old vars cannot match any route
 			Eventually(func() int {
 				return s.NewAPISIXClient().
@@ -964,20 +968,18 @@ spec:
     - http-route-policy-0
 `
 			By("create HTTPRoute")
-			ResourceApplied("HTTPRoute", "httpbin", varsRoute, 1)
+			s.ApplyHTTPRoute(types.NamespacedName{Namespace: s.Namespace(), Name: "httpbin"}, varsRoute)
 
 			By("create HTTPRoutePolices")
 			for name, spec := range map[string]string{
 				"http-route-policy-0": httpRoutePolicy0,
 				"http-route-policy-1": httpRoutePolicy1,
 				"http-route-policy-2": httpRoutePolicy2,
 			} {
-				err := s.CreateResourceFromString(spec)
-				Expect(err).NotTo(HaveOccurred(), "creating HTTPRoutePolicy")
-				// wait for HTTPRoutePolicy is Accepted
-				framework.HTTPRoutePolicyMustHaveCondition(s.GinkgoT, s.K8sClient, 10*time.Second,
+				s.ApplyHTTPRoutePolicy(
 					types.NamespacedName{Namespace: s.Namespace(), Name: "apisix"},
 					types.NamespacedName{Namespace: s.Namespace(), Name: name},
+					spec,
 					metav1.Condition{
 						Type: string(v1alpha2.PolicyConditionAccepted),
 					},
@@ -1058,24 +1060,23 @@ spec:
 
 		It("HTTPRoutePolicy status changes on HTTPRoute deleting", func() {
 			By("create HTTPRoute")
-			ResourceApplied("HTTPRoute", "httpbin", varsRoute, 1)
+			s.ApplyHTTPRoute(types.NamespacedName{Namespace: s.Namespace(), Name: "httpbin"}, varsRoute)
 
 			By("create HTTPRoutePolicy")
-			ResourceApplied("HTTPRoutePolicy", "http-route-policy-0", httpRoutePolicy, 1)
-
-			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).Should(ContainSubstring("type: Accepted"))
+			s.ApplyHTTPRoutePolicy(
+				types.NamespacedName{Name: "apisix"},
+				types.NamespacedName{Namespace: s.Namespace(), Name: "http-route-policy-0"},
+				httpRoutePolicy,
+			)
 
 			By("access dataplane to check the HTTPRoutePolicy")
-			s.NewAPISIXClient().
-				GET("/get").
-				WithHost("httpbin.example").
-				WithHeader("X-Route-Name", "httpbin").
-				Expect().
-				Status(http.StatusNotFound)
+			Eventually(func() int {
+				return s.NewAPISIXClient().
+					GET("/get").
+					WithHost("httpbin.example").
+					WithHeader("X-Route-Name", "httpbin").
+					Expect().Raw().StatusCode
+			}).WithTimeout(8 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusNotFound))
 
 			s.NewAPISIXClient().
 				GET("/get").
@@ -1104,11 +1105,12 @@ spec:
 			})
 			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:"))
+			err = framework.PollUntilHTTPRoutePolicyHaveStatus(s.K8sClient, 8*time.Second, types.NamespacedName{Namespace: s.Namespace(), Name: "http-route-policy-0"},
+				func(hrp *v1alpha1.HTTPRoutePolicy) bool {
+					return len(hrp.Status.Ancestors) == 0
+				},
+			)
+			Expect(err).NotTo(HaveOccurred(), "HTPRoutePolicy.Status should has no ancestor")
 		})
 	})