Skip to content

Commit 75ae919

Browse files
dspodspo
authored
fix: Set parent namespace in HTTPRoutePolicy and refactor related tests (#152)
1 parent 4ee0e26 commit 75ae919

File tree

7 files changed

+222
-120
lines changed

7 files changed

+222
-120
lines changed

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ KIND_NAME ?= apisix-ingress-cluster
1313
GATEAY_API_VERSION ?= v1.2.0
1414

1515
DASHBOARD_VERSION ?= dev
16-
TEST_TIMEOUT ?= 45m
16+
TEST_TIMEOUT ?= 60m
1717

1818
# CRD Reference Documentation
1919
CRD_REF_DOCS_VERSION ?= v0.1.0

‎internal/controller/httproutepolicy.go‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,16 @@ func (r *HTTPRouteReconciler) processHTTPRoutePolicies(tctx *provider.TranslateC
4646
return nil
4747
}
4848

49+
// set namespace in prentRef if it is not set
50+
var parentRefs = make([]gatewayv1.ParentReference, len(httpRoute.Spec.ParentRefs))
51+
for i := range httpRoute.Spec.ParentRefs {
52+
ref := httpRoute.Spec.ParentRefs[i]
53+
if ref.Namespace == nil || *ref.Namespace == "" {
54+
ref.Namespace = (*gatewayv1.Namespace)(&httpRoute.Namespace)
55+
}
56+
parentRefs[i] = ref
57+
}
58+
4959
var conflicts = make(map[types.NamespacedName]v1alpha1.HTTPRoutePolicy)
5060
for _, rule := range httpRoute.Spec.Rules {
5161
var policies = findPoliciesWhichTargetRefTheRule(rule.Name, "HTTPRoute", list)
@@ -71,7 +81,7 @@ func (r *HTTPRouteReconciler) processHTTPRoutePolicies(tctx *provider.TranslateC
7181
tctx.HTTPRoutePolicies = append(tctx.HTTPRoutePolicies, policy)
7282
}
7383

74-
if updated := setAncestorsForHTTPRoutePolicyStatus(httpRoute.Spec.ParentRefs, &policy, condition); updated {
84+
if updated := setAncestorsForHTTPRoutePolicyStatus(parentRefs, &policy, condition); updated {
7585
tctx.StatusUpdaters = append(tctx.StatusUpdaters, status.Update{
7686
NamespacedName: NamespacedName(&policy),
7787
Resource: policy.DeepCopy(),

‎test/e2e/framework/assertion.go‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
// Licensed under the Apache License, Version 2.0 (the "License");
2+
// you may not use this file except in compliance with the License.
3+
// You may obtain a copy of the License at
4+
//
5+
// http://www.apache.org/licenses/LICENSE-2.0
6+
//
7+
// Unless required by applicable law or agreed to in writing, software
8+
// distributed under the License is distributed on an "AS IS" BASIS,
9+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
10+
// See the License for the specific language governing permissions and
11+
// limitations under the License.
12+
13+
package framework
14+
15+
import (
16+
"context"
17+
"fmt"
18+
"log"
19+
"slices"
20+
"strings"
21+
"time"
22+
23+
"github.com/gruntwork-io/terratest/modules/testing"
24+
"github.com/pkg/errors"
25+
"github.com/stretchr/testify/require"
26+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
27+
"k8s.io/apimachinery/pkg/types"
28+
"k8s.io/apimachinery/pkg/util/wait"
29+
"sigs.k8s.io/controller-runtime/pkg/client"
30+
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
31+
"sigs.k8s.io/gateway-api/conformance/utils/kubernetes"
32+
33+
"github.com/apache/apisix-ingress-controller/api/v1alpha1"
34+
)
35+
36+
func HTTPRouteMustHaveCondition(t testing.TestingT, cli client.Client, timeout time.Duration, refNN, hrNN types.NamespacedName, condition metav1.Condition) {
37+
err := PollUntilHTTPRouteHaveStatus(cli, timeout, hrNN, func(hr *gatewayv1.HTTPRoute) bool {
38+
for _, parent := range hr.Status.Parents {
39+
if err := kubernetes.ConditionsHaveLatestObservedGeneration(hr, parent.Conditions); err != nil {
40+
log.Printf("HTTPRoute %s (parentRef=%v) %v", hrNN, parentRefToString(parent.ParentRef), err)
41+
return false
42+
}
43+
if (refNN.Name == "" || parent.ParentRef.Name == gatewayv1.ObjectName(refNN.Name)) &&
44+
(refNN.Namespace == "" || (parent.ParentRef.Namespace != nil && string(*parent.ParentRef.Namespace) == refNN.Namespace)) {
45+
if findConditionInList(parent.Conditions, condition) {
46+
log.Printf("found condition %v in %v for %s reference %s", condition, parent.Conditions, hrNN, refNN)
47+
return true
48+
} else {
49+
log.Printf("NOT FOUND condition %v in %v for %s reference %s", condition, parent.Conditions, hrNN, refNN)
50+
}
51+
}
52+
}
53+
return false
54+
})
55+
require.NoError(t, err, "error waiting for HTTPRoute status to have a Condition matching %+v", condition)
56+
}
57+
58+
func PollUntilHTTPRouteHaveStatus(cli client.Client, timeout time.Duration, hrNN types.NamespacedName, f func(route *gatewayv1.HTTPRoute) bool) error {
59+
if err := gatewayv1.Install(cli.Scheme()); err != nil {
60+
return err
61+
}
62+
return genericPollResource(new(gatewayv1.HTTPRoute), cli, timeout, hrNN, f)
63+
}
64+
65+
func HTTPRoutePolicyMustHaveCondition(t testing.TestingT, client client.Client, timeout time.Duration, refNN, hrpNN types.NamespacedName,
66+
condition metav1.Condition) {
67+
err := PollUntilHTTPRoutePolicyHaveStatus(client, timeout, hrpNN, func(httpRoutePolicy *v1alpha1.HTTPRoutePolicy) bool {
68+
for _, ancestor := range httpRoutePolicy.Status.Ancestors {
69+
if err := kubernetes.ConditionsHaveLatestObservedGeneration(httpRoutePolicy, ancestor.Conditions); err != nil {
70+
log.Printf("HTTPRoutePolicy %s (parentRef=%v) %v", hrpNN, parentRefToString(ancestor.AncestorRef), err)
71+
return false
72+
}
73+
74+
if ancestor.AncestorRef.Name == gatewayv1.ObjectName(refNN.Name) &&
75+
(refNN.Namespace == "" || (ancestor.AncestorRef.Namespace != nil && string(*ancestor.AncestorRef.Namespace) == refNN.Namespace)) {
76+
if findConditionInList(ancestor.Conditions, condition) {
77+
log.Printf("found condition %v in list %v for %s reference %s", condition, ancestor.Conditions, hrpNN, refNN)
78+
return true
79+
} else {
80+
log.Printf("NOT FOUND condition %v in %v for %s reference %s", condition, ancestor.Conditions, hrpNN, refNN)
81+
}
82+
}
83+
}
84+
return false
85+
})
86+
87+
require.NoError(t, err, "error waiting for HTTPRoutePolicy %s status to have a Condition matching %+v", hrpNN, condition)
88+
}
89+
90+
func PollUntilHTTPRoutePolicyHaveStatus(cli client.Client, timeout time.Duration, hrpNN types.NamespacedName,
91+
f func(httpRoutePolicy *v1alpha1.HTTPRoutePolicy) bool) error {
92+
if err := v1alpha1.AddToScheme(cli.Scheme()); err != nil {
93+
return err
94+
}
95+
return genericPollResource(new(v1alpha1.HTTPRoutePolicy), cli, timeout, hrpNN, f)
96+
}
97+
98+
func parentRefToString(p gatewayv1.ParentReference) string {
99+
if p.Namespace != nil && *p.Namespace != "" {
100+
return fmt.Sprintf("%v/%v", p.Namespace, p.Name)
101+
}
102+
return string(p.Name)
103+
}
104+
105+
func findConditionInList(conditions []metav1.Condition, expected metav1.Condition) bool {
106+
return slices.ContainsFunc(conditions, func(item metav1.Condition) bool {
107+
// an empty Status string means "Match any status".
108+
// an empty Reason string means "Match any reason".
109+
return expected.Type == item.Type &&
110+
(expected.Status == "" || expected.Status == item.Status) &&
111+
(expected.Reason == "" || expected.Reason == item.Reason) &&
112+
(expected.Message == "" || strings.Contains(item.Message, expected.Message))
113+
})
114+
}
115+
116+
func genericPollResource[Obj client.Object](obj Obj, cli client.Client, timeout time.Duration, nn types.NamespacedName, predicate func(Obj) bool) error {
117+
return wait.PollUntilContextTimeout(context.Background(), time.Second, timeout, true, func(ctx context.Context) (done bool, err error) {
118+
if err := cli.Get(ctx, nn, obj); err != nil {
119+
return false, errors.Wrapf(err, "error fetching Object %s", nn)
120+
}
121+
return predicate(obj), nil
122+
})
123+
}

‎test/e2e/framework/utils.go‎

Lines changed: 0 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -20,32 +20,20 @@ import (
2020
"encoding/json"
2121
"fmt"
2222
"io"
23-
"log"
2423
"net/http"
2524
"net/url"
26-
"slices"
2725
"strings"
2826
"sync"
2927
"time"
3028

3129
"github.com/gavv/httpexpect/v2"
32-
"github.com/gruntwork-io/terratest/modules/testing"
3330
"github.com/onsi/gomega"
34-
"github.com/pkg/errors"
35-
"github.com/stretchr/testify/require"
3631
"golang.org/x/net/html"
3732
corev1 "k8s.io/api/core/v1"
3833
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
39-
"k8s.io/apimachinery/pkg/types"
40-
"k8s.io/apimachinery/pkg/util/wait"
4134
"k8s.io/client-go/kubernetes/scheme"
4235
"k8s.io/client-go/tools/remotecommand"
4336
"k8s.io/utils/ptr"
44-
"sigs.k8s.io/controller-runtime/pkg/client"
45-
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
46-
"sigs.k8s.io/gateway-api/conformance/utils/kubernetes"
47-
48-
"github.com/apache/apisix-ingress-controller/api/v1alpha1"
4937
)
5038

5139
func (f *Framework) NewExpectResponse(httpBody any) *httpexpect.Response {
@@ -381,43 +369,6 @@ func CreateTestZipFile(sourceCode, metadata string) ([]byte, error) {
381369
return zipBuffer.Bytes(), nil
382370
}
383371

384-
func HTTPRoutePolicyMustHaveCondition(t testing.TestingT, client client.Client, timeout time.Duration, refNN, hrpNN types.NamespacedName,
385-
condition metav1.Condition) {
386-
err := EventuallyHTTPRoutePolicyHaveStatus(client, timeout, hrpNN, func(httpRoutePolicy v1alpha1.HTTPRoutePolicy, status v1alpha1.PolicyStatus) bool {
387-
for _, ancestor := range status.Ancestors {
388-
if err := kubernetes.ConditionsHaveLatestObservedGeneration(&httpRoutePolicy, ancestor.Conditions); err != nil {
389-
log.Printf("HTTPRoutePolicy %s (parentRef=%v) %v", hrpNN, parentRefToString(ancestor.AncestorRef), err)
390-
return false
391-
}
392-
393-
if ancestor.AncestorRef.Name == gatewayv1.ObjectName(refNN.Name) &&
394-
(ancestor.AncestorRef.Namespace == nil || refNN.Namespace == "" || string(*ancestor.AncestorRef.Namespace) == refNN.Namespace) {
395-
if findConditionInList(ancestor.Conditions, condition) {
396-
log.Printf("found condition %v in list [%v] for %s reference %s", condition, ancestor.Conditions, hrpNN, refNN)
397-
return true
398-
} else {
399-
log.Printf("not found condition %v in list [%v] for %s reference %s", condition, ancestor.Conditions, hrpNN, refNN)
400-
}
401-
}
402-
}
403-
return false
404-
})
405-
406-
require.NoError(t, err, "error waiting for HTTPRoutePolicy status to have a Condition matching expectations")
407-
}
408-
409-
func EventuallyHTTPRoutePolicyHaveStatus(client client.Client, timeout time.Duration, hrpNN types.NamespacedName,
410-
f func(httpRoutePolicy v1alpha1.HTTPRoutePolicy, status v1alpha1.PolicyStatus) bool) error {
411-
_ = v1alpha1.AddToScheme(client.Scheme())
412-
return wait.PollUntilContextTimeout(context.Background(), time.Second, timeout, true, func(ctx context.Context) (done bool, err error) {
413-
var httpRoutePolicy v1alpha1.HTTPRoutePolicy
414-
if err = client.Get(ctx, hrpNN, &httpRoutePolicy); err != nil {
415-
return false, errors.Errorf("error fetching HTTPRoutePolicy %v: %v", hrpNN, err)
416-
}
417-
return f(httpRoutePolicy, httpRoutePolicy.Status), nil
418-
})
419-
}
420-
421372
func addFileToZip(zipWriter *zip.Writer, fileName, fileContent string) error {
422373
file, err := zipWriter.Create(fileName)
423374
if err != nil {
@@ -427,18 +378,3 @@ func addFileToZip(zipWriter *zip.Writer, fileName, fileContent string) error {
427378
_, err = file.Write([]byte(fileContent))
428379
return err
429380
}
430-
431-
func parentRefToString(p gatewayv1.ParentReference) string {
432-
if p.Namespace != nil && *p.Namespace != "" {
433-
return fmt.Sprintf("%v/%v", p.Namespace, p.Name)
434-
}
435-
return string(p.Name)
436-
}
437-
438-
func findConditionInList(conditions []metav1.Condition, expected metav1.Condition) bool {
439-
return slices.ContainsFunc(conditions, func(item metav1.Condition) bool {
440-
// an empty Status string means "Match any status".
441-
// an empty Reason string means "Match any reason".
442-
return expected.Type == item.Type && (expected.Status == "" || expected.Status == item.Status) && (expected.Reason == "" || expected.Reason == item.Reason)
443-
})
444-
}

‎test/e2e/gatewayapi/httproute.go‎

Lines changed: 42 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"k8s.io/apimachinery/pkg/types"
2828
"sigs.k8s.io/gateway-api/apis/v1alpha2"
2929

30+
"github.com/apache/apisix-ingress-controller/api/v1alpha1"
3031
"github.com/apache/apisix-ingress-controller/test/e2e/framework"
3132
"github.com/apache/apisix-ingress-controller/test/e2e/scaffold"
3233
)
@@ -805,25 +806,23 @@ spec:
805806

806807
It("HTTPRoutePolicy in effect", func() {
807808
By("create HTTPRoute")
808-
ResourceApplied("HTTPRoute", "httpbin", varsRoute, 1)
809-
810-
s.NewAPISIXClient().
811-
GET("/get").
812-
WithHost("httpbin.example").
813-
WithHeader("X-Route-Name", "httpbin").
814-
Expect().
815-
Status(http.StatusOK)
809+
s.ApplyHTTPRoute(types.NamespacedName{Namespace: s.Namespace(), Name: "httpbin"}, varsRoute)
810+
request := func() int {
811+
return s.NewAPISIXClient().GET("/get").
812+
WithHost("httpbin.example").WithHeader("X-Route-Name", "httpbin").
813+
Expect().Raw().StatusCode
814+
}
815+
Eventually(request).WithTimeout(5 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusOK))
816816

817817
By("create HTTPRoutePolicy")
818-
ResourceApplied("HTTPRoutePolicy", "http-route-policy-0", httpRoutePolicy, 1)
818+
s.ApplyHTTPRoutePolicy(
819+
types.NamespacedName{Name: "apisix"},
820+
types.NamespacedName{Namespace: s.Namespace(), Name: "http-route-policy-0"},
821+
httpRoutePolicy,
822+
)
819823

820824
By("access dataplane to check the HTTPRoutePolicy")
821-
s.NewAPISIXClient().
822-
GET("/get").
823-
WithHost("httpbin.example").
824-
WithHeader("X-Route-Name", "httpbin").
825-
Expect().
826-
Status(http.StatusNotFound)
825+
Eventually(request).WithTimeout(5 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusNotFound))
827826

828827
s.NewAPISIXClient().
829828
GET("/get").
@@ -852,7 +851,12 @@ spec:
852851
- ==
853852
- new-hrp-name
854853
`
855-
ResourceApplied("HTTPRoutePolicy", "http-route-policy-0", changedHTTPRoutePolicy, 1)
854+
s.ApplyHTTPRoutePolicy(
855+
types.NamespacedName{Name: "apisix"},
856+
types.NamespacedName{Namespace: s.Namespace(), Name: "http-route-policy-0"},
857+
changedHTTPRoutePolicy,
858+
)
859+
856860
// use the old vars cannot match any route
857861
Eventually(func() int {
858862
return s.NewAPISIXClient().
@@ -964,20 +968,18 @@ spec:
964968
- http-route-policy-0
965969
`
966970
By("create HTTPRoute")
967-
ResourceApplied("HTTPRoute", "httpbin", varsRoute, 1)
971+
s.ApplyHTTPRoute(types.NamespacedName{Namespace: s.Namespace(), Name: "httpbin"}, varsRoute)
968972

969973
By("create HTTPRoutePolices")
970974
for name, spec := range map[string]string{
971975
"http-route-policy-0": httpRoutePolicy0,
972976
"http-route-policy-1": httpRoutePolicy1,
973977
"http-route-policy-2": httpRoutePolicy2,
974978
} {
975-
err := s.CreateResourceFromString(spec)
976-
Expect(err).NotTo(HaveOccurred(), "creating HTTPRoutePolicy")
977-
// wait for HTTPRoutePolicy is Accepted
978-
framework.HTTPRoutePolicyMustHaveCondition(s.GinkgoT, s.K8sClient, 10*time.Second,
979+
s.ApplyHTTPRoutePolicy(
979980
types.NamespacedName{Namespace: s.Namespace(), Name: "apisix"},
980981
types.NamespacedName{Namespace: s.Namespace(), Name: name},
982+
spec,
981983
metav1.Condition{
982984
Type: string(v1alpha2.PolicyConditionAccepted),
983985
},
@@ -1058,24 +1060,23 @@ spec:
10581060

10591061
It("HTTPRoutePolicy status changes on HTTPRoute deleting", func() {
10601062
By("create HTTPRoute")
1061-
ResourceApplied("HTTPRoute", "httpbin", varsRoute, 1)
1063+
s.ApplyHTTPRoute(types.NamespacedName{Namespace: s.Namespace(), Name: "httpbin"}, varsRoute)
10621064

10631065
By("create HTTPRoutePolicy")
1064-
ResourceApplied("HTTPRoutePolicy", "http-route-policy-0", httpRoutePolicy, 1)
1065-
1066-
Eventually(func() string {
1067-
spec, err := s.GetResourceYaml("HTTPRoutePolicy", "http-route-policy-0")
1068-
Expect(err).NotTo(HaveOccurred(), "getting HTTPRoutePolicy")
1069-
return spec
1070-
}).WithTimeout(8 * time.Second).ProbeEvery(time.Second).Should(ContainSubstring("type: Accepted"))
1066+
s.ApplyHTTPRoutePolicy(
1067+
types.NamespacedName{Name: "apisix"},
1068+
types.NamespacedName{Namespace: s.Namespace(), Name: "http-route-policy-0"},
1069+
httpRoutePolicy,
1070+
)
10711071

10721072
By("access dataplane to check the HTTPRoutePolicy")
1073-
s.NewAPISIXClient().
1074-
GET("/get").
1075-
WithHost("httpbin.example").
1076-
WithHeader("X-Route-Name", "httpbin").
1077-
Expect().
1078-
Status(http.StatusNotFound)
1073+
Eventually(func() int {
1074+
return s.NewAPISIXClient().
1075+
GET("/get").
1076+
WithHost("httpbin.example").
1077+
WithHeader("X-Route-Name", "httpbin").
1078+
Expect().Raw().StatusCode
1079+
}).WithTimeout(8 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusNotFound))
10791080

10801081
s.NewAPISIXClient().
10811082
GET("/get").
@@ -1104,11 +1105,12 @@ spec:
11041105
})
11051106
s.Logf(message)
11061107

1107-
Eventually(func() string {
1108-
spec, err := s.GetResourceYaml("HTTPRoutePolicy", "http-route-policy-0")
1109-
Expect(err).NotTo(HaveOccurred(), "getting HTTPRoutePolicy")
1110-
return spec
1111-
}).WithTimeout(8 * time.Second).ProbeEvery(time.Second).ShouldNot(ContainSubstring("ancestorRef:"))
1108+
err = framework.PollUntilHTTPRoutePolicyHaveStatus(s.K8sClient, 8*time.Second, types.NamespacedName{Namespace: s.Namespace(), Name: "http-route-policy-0"},
1109+
func(hrp *v1alpha1.HTTPRoutePolicy) bool {
1110+
return len(hrp.Status.Ancestors) == 0
1111+
},
1112+
)
1113+
Expect(err).NotTo(HaveOccurred(), "HTPRoutePolicy.Status should has no ancestor")
11121114
})
11131115
})
11141116

0 commit comments

Comments
 (0)