diff --git a/Makefile b/Makefile index 194ae9a0b..3073e3790 100644 --- a/Makefile +++ b/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 diff --git a/internal/controller/httproutepolicy.go b/internal/controller/httproutepolicy.go index 08142664e..15fba525d 100644 --- a/internal/controller/httproutepolicy.go +++ b/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(), diff --git a/test/e2e/framework/assertion.go b/test/e2e/framework/assertion.go new file mode 100644 index 000000000..8f7bfe8a1 --- /dev/null +++ b/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 + }) +} diff --git a/test/e2e/framework/utils.go b/test/e2e/framework/utils.go index 511fd94fc..bf7f77c15 100644 --- a/test/e2e/framework/utils.go +++ b/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) - }) -} diff --git a/test/e2e/gatewayapi/httproute.go b/test/e2e/gatewayapi/httproute.go index d710875a3..8d2183ed5 100644 --- a/test/e2e/gatewayapi/httproute.go +++ b/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,7 +968,7 @@ 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{ @@ -972,12 +976,10 @@ spec: "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") }) }) diff --git a/test/e2e/ingress/ingress.go b/test/e2e/ingress/ingress.go index fb0de348a..7f95534e8 100644 --- a/test/e2e/ingress/ingress.go +++ b/test/e2e/ingress/ingress.go @@ -23,9 +23,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/stretchr/testify/assert" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" "github.com/apache/apisix-ingress-controller/api/v1alpha1" "github.com/apache/apisix-ingress-controller/test/e2e/framework" @@ -732,26 +730,20 @@ spec: Expect(err).NotTo(HaveOccurred(), "creating Ingress") By("create HTTPRoutePolicy") - err = s.CreateResourceFromString(httpRoutePolicySpec0) - Expect(err).NotTo(HaveOccurred(), "creating HTTPRoutePolicy") - framework.HTTPRoutePolicyMustHaveCondition(s.GinkgoT, s.K8sClient, 8*time.Second, - types.NamespacedName{Namespace: s.Namespace(), Name: "apisix"}, + s.ApplyHTTPRoutePolicy( + types.NamespacedName{Name: "apisix"}, types.NamespacedName{Namespace: s.Namespace(), Name: "http-route-policy-0"}, - metav1.Condition{ - Type: string(gatewayv1alpha2.PolicyConditionAccepted), - Status: metav1.ConditionTrue, - Reason: string(gatewayv1alpha2.PolicyReasonAccepted), - }, + httpRoutePolicySpec0, ) By("delete ingress") err = s.DeleteResource("Ingress", "default") Expect(err).NotTo(HaveOccurred(), "delete Ingress") - err = framework.EventuallyHTTPRoutePolicyHaveStatus(s.K8sClient, 8*time.Second, + err = framework.PollUntilHTTPRoutePolicyHaveStatus(s.K8sClient, 8*time.Second, types.NamespacedName{Namespace: s.Namespace(), Name: "http-route-policy-0"}, - func(_ v1alpha1.HTTPRoutePolicy, status v1alpha1.PolicyStatus) bool { - return len(status.Ancestors) == 0 + func(hrp *v1alpha1.HTTPRoutePolicy) bool { + return len(hrp.Status.Ancestors) == 0 }, ) Expect(err).NotTo(HaveOccurred(), "expected HTTPRoutePolicy.Status has no Ancestor") diff --git a/test/e2e/scaffold/k8s.go b/test/e2e/scaffold/k8s.go index eb29bd259..fc14fcf72 100644 --- a/test/e2e/scaffold/k8s.go +++ b/test/e2e/scaffold/k8s.go @@ -13,6 +13,7 @@ package scaffold import ( + "cmp" "context" "fmt" "net/url" @@ -25,9 +26,14 @@ import ( "github.com/gruntwork-io/terratest/modules/testing" . "github.com/onsi/ginkgo/v2" //nolint:staticcheck . "github.com/onsi/gomega" //nolint:staticcheck + "github.com/stretchr/testify/require" 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" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + "sigs.k8s.io/gateway-api/apis/v1alpha2" "github.com/apache/apisix-ingress-controller/pkg/dashboard" "github.com/apache/apisix-ingress-controller/test/e2e/framework" @@ -281,3 +287,36 @@ func (s *Scaffold) ApplyDefaultGatewayResource( s.ResourceApplied("httproute", "httpbin", defaultHTTPRoute, 1) } + +func (s *Scaffold) ApplyHTTPRoute(hrNN types.NamespacedName, spec string, until ...wait.ConditionWithContextFunc) { + err := s.CreateResourceFromString(spec) + Expect(err).NotTo(HaveOccurred(), "creating HTTPRoute %s", hrNN) + framework.HTTPRouteMustHaveCondition(s.GinkgoT, s.K8sClient, 8*time.Second, + types.NamespacedName{}, + types.NamespacedName{Namespace: cmp.Or(hrNN.Namespace, s.Namespace()), Name: hrNN.Name}, + metav1.Condition{ + Type: string(gatewayv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }, + ) + for i, f := range until { + err := wait.PollUntilContextTimeout(context.Background(), time.Second, 10*time.Second, true, f) + require.NoError(s.GinkgoT, err, "wait for ConditionWithContextFunc[%d] OK", i) + } +} + +func (s *Scaffold) ApplyHTTPRoutePolicy(refNN, hrpNN types.NamespacedName, spec string, conditions ...metav1.Condition) { + err := s.CreateResourceFromString(spec) + Expect(err).NotTo(HaveOccurred(), "creating HTTPRoutePolicy %s", hrpNN) + if len(conditions) == 0 { + conditions = []metav1.Condition{ + { + Type: string(v1alpha2.PolicyConditionAccepted), + Status: metav1.ConditionTrue, + }, + } + } + for _, condition := range conditions { + framework.HTTPRoutePolicyMustHaveCondition(s.GinkgoT, s.K8sClient, 8*time.Second, refNN, hrpNN, condition) + } +}