diff --git a/internal/provider/kubernetes/status.go b/internal/provider/kubernetes/status.go index b3aec665f7c..8f075842826 100644 --- a/internal/provider/kubernetes/status.go +++ b/internal/provider/kubernetes/status.go @@ -8,6 +8,7 @@ package kubernetes import ( "context" "fmt" + "reflect" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -111,7 +112,7 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context Spec: h.Spec, Status: gwapiv1.HTTPRouteStatus{ RouteStatus: gwapiv1.RouteStatus{ - Parents: mergeRouteParentStatus(h.Namespace, h.Status.Parents, valCopy.Parents), + Parents: mergeStatus(h.Namespace, r.envoyGateway.Gateway.ControllerName, h.Status.Parents, valCopy.Parents), }, }, } @@ -151,7 +152,7 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context Spec: g.Spec, Status: gwapiv1.GRPCRouteStatus{ RouteStatus: gwapiv1.RouteStatus{ - Parents: mergeRouteParentStatus(g.Namespace, g.Status.Parents, valCopy.Parents), + Parents: mergeStatus(g.Namespace, r.envoyGateway.Gateway.ControllerName, g.Status.Parents, valCopy.Parents), }, }, } @@ -193,7 +194,7 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context Spec: t.Spec, Status: gwapiv1a2.TLSRouteStatus{ RouteStatus: gwapiv1.RouteStatus{ - Parents: mergeRouteParentStatus(t.Namespace, t.Status.Parents, valCopy.Parents), + Parents: mergeStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Parents, valCopy.Parents), }, }, } @@ -235,7 +236,7 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context Spec: t.Spec, Status: gwapiv1a2.TCPRouteStatus{ RouteStatus: gwapiv1.RouteStatus{ - Parents: mergeRouteParentStatus(t.Namespace, t.Status.Parents, valCopy.Parents), + Parents: mergeStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Parents, valCopy.Parents), }, }, } @@ -277,7 +278,7 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context Spec: u.Spec, Status: gwapiv1a2.UDPRouteStatus{ RouteStatus: gwapiv1.RouteStatus{ - Parents: mergeRouteParentStatus(u.Namespace, u.Status.Parents, valCopy.Parents), + Parents: mergeStatus(u.Namespace, r.envoyGateway.Gateway.ControllerName, u.Status.Parents, valCopy.Parents), }, }, } @@ -317,7 +318,9 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context TypeMeta: t.TypeMeta, ObjectMeta: t.ObjectMeta, Spec: t.Spec, - Status: *valCopy, + Status: gwapiv1.PolicyStatus{ + Ancestors: mergeStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Ancestors, valCopy.Ancestors), + }, } return tCopy }), @@ -355,7 +358,9 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context TypeMeta: t.TypeMeta, ObjectMeta: t.ObjectMeta, Spec: t.Spec, - Status: *valCopy, + Status: gwapiv1.PolicyStatus{ + Ancestors: mergeStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Ancestors, valCopy.Ancestors), + }, } return tCopy }), @@ -393,7 +398,9 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context TypeMeta: t.TypeMeta, ObjectMeta: t.ObjectMeta, Spec: t.Spec, - Status: *valCopy, + Status: gwapiv1.PolicyStatus{ + Ancestors: mergeStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Ancestors, valCopy.Ancestors), + }, } return tCopy }), @@ -431,7 +438,9 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context TypeMeta: t.TypeMeta, ObjectMeta: t.ObjectMeta, Spec: t.Spec, - Status: *valCopy, + Status: gwapiv1.PolicyStatus{ + Ancestors: mergeStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Ancestors, valCopy.Ancestors), + }, } return tCopy }), @@ -467,7 +476,9 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context TypeMeta: t.TypeMeta, ObjectMeta: t.ObjectMeta, Spec: t.Spec, - Status: *valCopy, + Status: gwapiv1.PolicyStatus{ + Ancestors: mergeStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Ancestors, valCopy.Ancestors), + }, } return tCopy }), @@ -505,7 +516,9 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context TypeMeta: t.TypeMeta, ObjectMeta: t.ObjectMeta, Spec: t.Spec, - Status: *valCopy, + Status: gwapiv1.PolicyStatus{ + Ancestors: mergeStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Ancestors, valCopy.Ancestors), + }, } return tCopy }), @@ -582,6 +595,15 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context tCopy := t.DeepCopy() valCopy := val.DeepCopy() setLastTransitionTimeInConditionsForPolicyStatus(valCopy, metav1.Now()) + var oldAncestors []gwapiv1.PolicyAncestorStatus + o, found, err := unstructured.NestedFieldCopy(tCopy.Object, "status", "ancestors") + if found && err == nil { + oldAncestors, ok = o.([]gwapiv1.PolicyAncestorStatus) + if ok { + tCopy.Object["status"] = mergeStatus(t.GetNamespace(), r.envoyGateway.Gateway.ControllerName, oldAncestors, valCopy.Ancestors) + return tCopy + } + } tCopy.Object["status"] = *valCopy return tCopy }), @@ -593,31 +615,28 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context } } -// mergeRouteParentStatus merges the old and new RouteParentStatus. -// This is needed because the RouteParentStatus doesn't support strategic merge patch yet. -func mergeRouteParentStatus(ns string, old, new []gwapiv1.RouteParentStatus) []gwapiv1.RouteParentStatus { +// mergeStatus merges the old and new `RouteParentStatus`/`PolicyAncestorStatus`. +// This is needed because the `RouteParentStatus`/`PolicyAncestorStatus` doesn't support strategic merge patch yet. +// This depends on the fact that we get the full updated status of the route/policy (all parents/ancestors), and will break otherwise. +func mergeStatus[K interface{}](ns, controllerName string, old, new []K) []K { // Allocating with worst-case capacity to avoid reallocation. - merged := make([]gwapiv1.RouteParentStatus, 0, len(old)+len(new)) + merged := make([]K, 0, len(old)+len(new)) // Range over old status parentRefs in order: // 1. The parentRef exists in the new status: append the new one to the final status. // 2. The parentRef doesn't exist in the new status and it's not our controller: append it to the final status. - // 3. The parentRef doesn't exist in the new status, and it is our controller: keep it in the final status. - // This is important for routes with multiple parent references - not all parents are updated in each reconciliation. + // 3. The parentRef doesn't exist in the new status, and it is our controller: don't append it to the final status. for _, oldP := range old { found := -1 for newI, newP := range new { - if gatewayapi.IsParentRefEqual(oldP.ParentRef, newP.ParentRef, ns) { + if isParentOrAncestorRefEqual(oldP, newP, ns) { found = newI break } } if found >= 0 { merged = append(merged, new[found]) - } else { - // Keep all old parent statuses, regardless of controller. - // For routes with multiple parents managed by the same controller, - // not all parents are necessarily updated in each reconciliation. + } else if parentOrAncestorControllerName(oldP) != gwapiv1.GatewayController(controllerName) { merged = append(merged, oldP) } } @@ -626,7 +645,7 @@ func mergeRouteParentStatus(ns string, old, new []gwapiv1.RouteParentStatus) []g for _, newP := range new { found := false for _, mergedP := range merged { - if gatewayapi.IsParentRefEqual(newP.ParentRef, mergedP.ParentRef, ns) { + if isParentOrAncestorRefEqual(newP, mergedP, ns) { found = true break } @@ -639,6 +658,28 @@ func mergeRouteParentStatus(ns string, old, new []gwapiv1.RouteParentStatus) []g return merged } +func isParentOrAncestorRefEqual[K any](firstRef, secondRef K, ns string) bool { + switch reflect.TypeOf(firstRef) { + case reflect.TypeOf(gwapiv1.RouteParentStatus{}): + return gatewayapi.IsParentRefEqual(any(firstRef).(gwapiv1.RouteParentStatus).ParentRef, any(secondRef).(gwapiv1.RouteParentStatus).ParentRef, ns) + case reflect.TypeOf(gwapiv1.PolicyAncestorStatus{}): + return gatewayapi.IsParentRefEqual(any(firstRef).(gwapiv1.PolicyAncestorStatus).AncestorRef, any(secondRef).(gwapiv1.PolicyAncestorStatus).AncestorRef, ns) + default: + return false + } +} + +func parentOrAncestorControllerName[K any](ref K) gwapiv1.GatewayController { + switch reflect.TypeOf(ref) { + case reflect.TypeOf(gwapiv1.RouteParentStatus{}): + return any(ref).(gwapiv1.RouteParentStatus).ControllerName + case reflect.TypeOf(gwapiv1.PolicyAncestorStatus{}): + return any(ref).(gwapiv1.PolicyAncestorStatus).ControllerName + default: + return gwapiv1.GatewayController("") + } +} + func (r *gatewayAPIReconciler) updateStatusForGateway(ctx context.Context, gtw *gwapiv1.Gateway) { // nil check for unit tests. if r.statusUpdater == nil { diff --git a/internal/provider/kubernetes/status_test.go b/internal/provider/kubernetes/status_test.go index 1cb4c5730a3..d760ed06c29 100644 --- a/internal/provider/kubernetes/status_test.go +++ b/internal/provider/kubernetes/status_test.go @@ -14,7 +14,7 @@ import ( gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" ) -func Test_mergeRouteParentStatus(t *testing.T) { +func Test_mergeStatusForRoutes(t *testing.T) { type args struct { old []gwapiv1.RouteParentStatus new []gwapiv1.RouteParentStatus @@ -361,24 +361,6 @@ func Test_mergeRouteParentStatus(t *testing.T) { }, }, }, - { - ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", - ParentRef: gwapiv1.ParentReference{ - Name: "gateway2", - }, - Conditions: []metav1.Condition{ - { - Type: string(gwapiv1.RouteConditionAccepted), - Status: metav1.ConditionTrue, - Reason: "Accepted", - }, - { - Type: string(gwapiv1.RouteConditionResolvedRefs), - Status: metav1.ConditionTrue, - Reason: "ResolvedRefs", - }, - }, - }, { ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", ParentRef: gwapiv1.ParentReference{ @@ -474,24 +456,6 @@ func Test_mergeRouteParentStatus(t *testing.T) { }, }, }, - { - ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", - ParentRef: gwapiv1.ParentReference{ - Name: "gateway2", - }, - Conditions: []metav1.Condition{ - { - Type: string(gwapiv1.RouteConditionAccepted), - Status: metav1.ConditionTrue, - Reason: "Accepted", - }, - { - Type: string(gwapiv1.RouteConditionResolvedRefs), - Status: metav1.ConditionTrue, - Reason: "ResolvedRefs", - }, - }, - }, }, }, @@ -706,24 +670,6 @@ func Test_mergeRouteParentStatus(t *testing.T) { }, }, want: []gwapiv1.RouteParentStatus{ - { - ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", - ParentRef: gwapiv1.ParentReference{ - Name: "gateway2", - }, - Conditions: []metav1.Condition{ - { - Type: string(gwapiv1.RouteConditionAccepted), - Status: metav1.ConditionTrue, - Reason: "Accepted", - }, - { - Type: string(gwapiv1.RouteConditionResolvedRefs), - Status: metav1.ConditionTrue, - Reason: "ResolvedRefs", - }, - }, - }, { ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", ParentRef: gwapiv1.ParentReference{ @@ -744,10 +690,9 @@ func Test_mergeRouteParentStatus(t *testing.T) { }, }, }, - // Test that parent refs managed by our controller are preserved even when not in new update. - // This is important for routes with multiple parent references. + // Similar note about practicality of occurrence. { - name: "old contains one parentRef of ours, and it's not in new - should be preserved.", + name: "old contains one parentRef of ours, and it gets dropped in new.", args: args{ old: []gwapiv1.RouteParentStatus{ { @@ -771,36 +716,39 @@ func Test_mergeRouteParentStatus(t *testing.T) { }, new: []gwapiv1.RouteParentStatus{}, }, - want: []gwapiv1.RouteParentStatus{ - { - ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", - ParentRef: gwapiv1.ParentReference{ - Name: "gateway2", - }, - Conditions: []metav1.Condition{ - { - Type: string(gwapiv1.RouteConditionAccepted), - Status: metav1.ConditionTrue, - Reason: "Accepted", - }, - { - Type: string(gwapiv1.RouteConditionResolvedRefs), - Status: metav1.ConditionTrue, - Reason: "ResolvedRefs", - }, - }, - }, - }, + want: []gwapiv1.RouteParentStatus{}, }, - // Test multi-parent scenario where only one parent is updated at a time. + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := mergeStatus("default", "gateway.envoyproxy.io/gatewayclass-controller", tt.args.old, tt.args.new); !reflect.DeepEqual(got, tt.want) { + t.Errorf("mergeStatus() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_mergeStatusForPolicies(t *testing.T) { + type args struct { + old []gwapiv1.PolicyAncestorStatus + new []gwapiv1.PolicyAncestorStatus + } + tests := []struct { + name string + args args + want []gwapiv1.PolicyAncestorStatus + }{ { - name: "multiple parents from same controller - update one, preserve others", + name: "old contains one ancestorRef of ours and one of another controller's, status of ours changed in new.", args: args{ - old: []gwapiv1.RouteParentStatus{ + old: []gwapiv1.PolicyAncestorStatus{ { - ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", - ParentRef: gwapiv1.ParentReference{ - Name: "gateway1", + ControllerName: "istio.io/gateway-controller", + AncestorRef: gwapiv1.ParentReference{ + Name: "gateway1", + Namespace: ptr.To[gwapiv1.Namespace]("default"), + SectionName: ptr.To[gwapiv1.SectionName]("listener1"), + Port: ptr.To[gwapiv1.PortNumber](80), }, Conditions: []metav1.Condition{ { @@ -812,7 +760,7 @@ func Test_mergeRouteParentStatus(t *testing.T) { }, { ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", - ParentRef: gwapiv1.ParentReference{ + AncestorRef: gwapiv1.ParentReference{ Name: "gateway2", }, Conditions: []metav1.Condition{ @@ -824,32 +772,30 @@ func Test_mergeRouteParentStatus(t *testing.T) { }, }, }, - new: []gwapiv1.RouteParentStatus{ + new: []gwapiv1.PolicyAncestorStatus{ { ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", - ParentRef: gwapiv1.ParentReference{ - Name: "gateway1", + AncestorRef: gwapiv1.ParentReference{ + Name: "gateway2", }, Conditions: []metav1.Condition{ { Type: string(gwapiv1.RouteConditionAccepted), - Status: metav1.ConditionTrue, + Status: metav1.ConditionFalse, Reason: "Accepted", }, - { - Type: string(gwapiv1.RouteConditionResolvedRefs), - Status: metav1.ConditionTrue, - Reason: "ResolvedRefs", - }, }, }, }, }, - want: []gwapiv1.RouteParentStatus{ + want: []gwapiv1.PolicyAncestorStatus{ { - ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", - ParentRef: gwapiv1.ParentReference{ - Name: "gateway1", + ControllerName: "istio.io/gateway-controller", + AncestorRef: gwapiv1.ParentReference{ + Name: "gateway1", + Namespace: ptr.To[gwapiv1.Namespace]("default"), + SectionName: ptr.To[gwapiv1.SectionName]("listener1"), + Port: ptr.To[gwapiv1.PortNumber](80), }, Conditions: []metav1.Condition{ { @@ -857,22 +803,17 @@ func Test_mergeRouteParentStatus(t *testing.T) { Status: metav1.ConditionTrue, Reason: "Accepted", }, - { - Type: string(gwapiv1.RouteConditionResolvedRefs), - Status: metav1.ConditionTrue, - Reason: "ResolvedRefs", - }, }, }, { ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", - ParentRef: gwapiv1.ParentReference{ + AncestorRef: gwapiv1.ParentReference{ Name: "gateway2", }, Conditions: []metav1.Condition{ { Type: string(gwapiv1.RouteConditionAccepted), - Status: metav1.ConditionTrue, + Status: metav1.ConditionFalse, Reason: "Accepted", }, }, @@ -882,8 +823,8 @@ func Test_mergeRouteParentStatus(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := mergeRouteParentStatus("default", tt.args.old, tt.args.new); !reflect.DeepEqual(got, tt.want) { - t.Errorf("mergeRouteParentStatus() = %v, want %v", got, tt.want) + if got := mergeStatus("default", "gateway.envoyproxy.io/gatewayclass-controller", tt.args.old, tt.args.new); !reflect.DeepEqual(got, tt.want) { + t.Errorf("mergeStatus() = %v, want %v", got, tt.want) } }) } diff --git a/test/e2e/testdata/policy-status-cleanup-multiple-ancestors-multiple-gwcs.yaml b/test/e2e/testdata/policy-status-cleanup-multiple-ancestors-multiple-gwcs.yaml new file mode 100644 index 00000000000..955393ae0de --- /dev/null +++ b/test/e2e/testdata/policy-status-cleanup-multiple-ancestors-multiple-gwcs.yaml @@ -0,0 +1,14 @@ +apiVersion: gateway.envoyproxy.io/v1alpha1 +kind: BackendTrafficPolicy +metadata: + name: backendtrafficpolicy-multiple-ancestors-same-gwc + namespace: gateway-conformance-infra +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: same-namespace + # This gateway (and its gatewayclass) is created through `status-cleanup-gateway-different-gwc.yaml` manifest. + - group: gateway.networking.k8s.io + kind: Gateway + name: gateway-2 diff --git a/test/e2e/testdata/policy-status-cleanup-multiple-ancestors-same-gwc.yaml b/test/e2e/testdata/policy-status-cleanup-multiple-ancestors-same-gwc.yaml new file mode 100644 index 00000000000..b5f82c16af1 --- /dev/null +++ b/test/e2e/testdata/policy-status-cleanup-multiple-ancestors-same-gwc.yaml @@ -0,0 +1,13 @@ +apiVersion: gateway.envoyproxy.io/v1alpha1 +kind: BackendTrafficPolicy +metadata: + name: backendtrafficpolicy-multiple-ancestors-same-gwc + namespace: gateway-conformance-infra +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: same-namespace + - group: gateway.networking.k8s.io + kind: Gateway + name: all-namespaces diff --git a/test/e2e/testdata/policy-status-cleanup-no-ancestor.yaml b/test/e2e/testdata/policy-status-cleanup-no-ancestor.yaml new file mode 100644 index 00000000000..0ecf36b2e54 --- /dev/null +++ b/test/e2e/testdata/policy-status-cleanup-no-ancestor.yaml @@ -0,0 +1,10 @@ +apiVersion: gateway.envoyproxy.io/v1alpha1 +kind: BackendTrafficPolicy +metadata: + name: backendtrafficpolicy-multiple-ancestors-same-gwc + namespace: gateway-conformance-infra +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: other-controller-ancestor diff --git a/test/e2e/testdata/policy-status-cleanup-single-ancestor.yaml b/test/e2e/testdata/policy-status-cleanup-single-ancestor.yaml new file mode 100644 index 00000000000..d2f8de24433 --- /dev/null +++ b/test/e2e/testdata/policy-status-cleanup-single-ancestor.yaml @@ -0,0 +1,10 @@ +apiVersion: gateway.envoyproxy.io/v1alpha1 +kind: BackendTrafficPolicy +metadata: + name: backendtrafficpolicy-multiple-ancestors-same-gwc + namespace: gateway-conformance-infra +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: same-namespace diff --git a/test/e2e/testdata/route-status-cleanup-multiple-parents-multiple-gwcs.yaml b/test/e2e/testdata/route-status-cleanup-multiple-parents-multiple-gwcs.yaml new file mode 100644 index 00000000000..76e3097ae2f --- /dev/null +++ b/test/e2e/testdata/route-status-cleanup-multiple-parents-multiple-gwcs.yaml @@ -0,0 +1,31 @@ +apiVersion: gateway.networking.k8s.io/v1beta1 +kind: Gateway +metadata: + name: gateway-1 + namespace: gateway-conformance-infra +spec: + gatewayClassName: "{GATEWAY_CLASS_NAME}" + listeners: + - name: foo + protocol: TCP + port: 8080 + allowedRoutes: + kinds: + - kind: TCPRoute +--- +apiVersion: gateway.networking.k8s.io/v1alpha2 +kind: TCPRoute +metadata: + name: tcp-route-status-cleanup + namespace: gateway-conformance-infra +spec: + parentRefs: + - name: gateway-1 + sectionName: foo + # This gateway (and its gatewayclass) is created through `status-cleanup-gateway-different-gwc.yaml` manifest. + - name: gateway-2 + sectionName: foo + rules: + - backendRefs: + - name: infra-backend-v1 + port: 8080 diff --git a/test/e2e/testdata/route-status-cleanup-multiple-parents-same-gwc.yaml b/test/e2e/testdata/route-status-cleanup-multiple-parents-same-gwc.yaml new file mode 100644 index 00000000000..b9585646af9 --- /dev/null +++ b/test/e2e/testdata/route-status-cleanup-multiple-parents-same-gwc.yaml @@ -0,0 +1,45 @@ +apiVersion: gateway.networking.k8s.io/v1beta1 +kind: Gateway +metadata: + name: gateway-1 + namespace: gateway-conformance-infra +spec: + gatewayClassName: "{GATEWAY_CLASS_NAME}" + listeners: + - name: foo + protocol: TCP + port: 8080 + allowedRoutes: + kinds: + - kind: TCPRoute +--- +apiVersion: gateway.networking.k8s.io/v1beta1 +kind: Gateway +metadata: + name: gateway-2 + namespace: gateway-conformance-infra +spec: + gatewayClassName: "{GATEWAY_CLASS_NAME}" + listeners: + - name: foo + protocol: TCP + port: 8080 + allowedRoutes: + kinds: + - kind: TCPRoute +--- +apiVersion: gateway.networking.k8s.io/v1alpha2 +kind: TCPRoute +metadata: + name: tcp-route-status-cleanup + namespace: gateway-conformance-infra +spec: + parentRefs: + - name: gateway-1 + sectionName: foo + - name: gateway-2 + sectionName: foo + rules: + - backendRefs: + - name: infra-backend-v1 + port: 8080 diff --git a/test/e2e/testdata/route-status-cleanup-single-parent.yaml b/test/e2e/testdata/route-status-cleanup-single-parent.yaml new file mode 100644 index 00000000000..e584f591f38 --- /dev/null +++ b/test/e2e/testdata/route-status-cleanup-single-parent.yaml @@ -0,0 +1,13 @@ +apiVersion: gateway.networking.k8s.io/v1alpha2 +kind: TCPRoute +metadata: + name: tcp-route-status-cleanup + namespace: gateway-conformance-infra +spec: + parentRefs: + - name: gateway-1 + sectionName: foo + rules: + - backendRefs: + - name: infra-backend-v1 + port: 8080 diff --git a/test/e2e/testdata/status-cleanup-gateway-different-gwc.yaml b/test/e2e/testdata/status-cleanup-gateway-different-gwc.yaml new file mode 100644 index 00000000000..03659367ffc --- /dev/null +++ b/test/e2e/testdata/status-cleanup-gateway-different-gwc.yaml @@ -0,0 +1,34 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: GatewayClass +metadata: + name: status-cleanup +spec: + controllerName: gateway.envoyproxy.io/gatewayclass-controller +--- +apiVersion: gateway.networking.k8s.io/v1beta1 +kind: Gateway +metadata: + name: gateway-2 + namespace: gateway-conformance-infra +spec: + gatewayClassName: status-cleanup + listeners: + - name: foo + protocol: TCP + port: 8080 + allowedRoutes: + kinds: + - kind: TCPRoute + infrastructure: + parametersRef: + group: gateway.envoyproxy.io + kind: EnvoyProxy + name: status-cleanup +--- +apiVersion: gateway.envoyproxy.io/v1alpha1 +kind: EnvoyProxy +metadata: + name: status-cleanup + namespace: gateway-conformance-infra +spec: + ipFamily: IPv4 diff --git a/test/e2e/tests/policy_status_cleanup.go b/test/e2e/tests/policy_status_cleanup.go new file mode 100644 index 00000000000..8feab945b6e --- /dev/null +++ b/test/e2e/tests/policy_status_cleanup.go @@ -0,0 +1,169 @@ +// Copyright Envoy Gateway Authors +// SPDX-License-Identifier: Apache-2.0 +// The full text of the Apache license is available in the LICENSE file at +// the root of the repo. + +//go:build e2e + +package tests + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" + "sigs.k8s.io/gateway-api/conformance/utils/suite" + + egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" + "github.com/envoyproxy/gateway/internal/gatewayapi" + "github.com/envoyproxy/gateway/internal/gatewayapi/resource" +) + +func init() { + ConformanceTests = append(ConformanceTests, PolicyStatusCleanupSameGatewayClass, PolicyStatusCleanupMultipleGatewayClasses) +} + +var PolicyStatusCleanupSameGatewayClass = suite.ConformanceTest{ + ShortName: "PolicyStatusCleanupSameGatewayClass", + Description: "Testing Policy Status Cleanup With Ancestors of The Same GatewayClass", + Manifests: []string{"testdata/policy-status-cleanup-multiple-ancestors-same-gwc.yaml"}, + Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { + t.Run("PolicyStatusCleanup", func(t *testing.T) { + ns := "gateway-conformance-infra" + gw1NN, gw2NN := types.NamespacedName{Name: "same-namespace", Namespace: ns}, types.NamespacedName{Name: "all-namespaces", Namespace: ns} + + policyNamespacedName := types.NamespacedName{Name: "backendtrafficpolicy-multiple-ancestors-same-gwc", Namespace: ns} + + // Check the policy has two ancestors in its status. + BackendTrafficPolicyMustBeAcceptedByAllAncestors(t, + suite.Client, + policyNamespacedName, + suite.ControllerName, + gwapiv1.ParentReference{ + Group: gatewayapi.GroupPtr(gwapiv1.GroupName), + Kind: gatewayapi.KindPtr(resource.KindGateway), + Namespace: gatewayapi.NamespacePtr(gw1NN.Namespace), + Name: gwapiv1.ObjectName(gw1NN.Name), + }, + gwapiv1.ParentReference{ + Group: gatewayapi.GroupPtr(gwapiv1.GroupName), + Kind: gatewayapi.KindPtr(resource.KindGateway), + Namespace: gatewayapi.NamespacePtr(gw2NN.Namespace), + Name: gwapiv1.ObjectName(gw2NN.Name), + }, + ) + + // Change the policy to have a single ancestor, and check its status. + suite.Applier.MustApplyWithCleanup(t, suite.Client, suite.TimeoutConfig, "testdata/policy-status-cleanup-single-ancestor.yaml", false) + BackendTrafficPolicyMustBeAcceptedByAllAncestors(t, + suite.Client, + policyNamespacedName, + suite.ControllerName, + gwapiv1.ParentReference{ + Group: gatewayapi.GroupPtr(gwapiv1.GroupName), + Kind: gatewayapi.KindPtr(resource.KindGateway), + Namespace: gatewayapi.NamespacePtr(gw1NN.Namespace), + Name: gwapiv1.ObjectName(gw1NN.Name), + }, + ) + + // Update the policy status to include a status ancestor of another controller. + policy := &egv1a1.BackendTrafficPolicy{} + err := suite.Client.Get(context.Background(), policyNamespacedName, policy) + require.NoErrorf(t, err, "error getting BackendTrafficPolicy %s", policyNamespacedName.String()) + otherControllerAncestor := gwapiv1.PolicyAncestorStatus{ + AncestorRef: gwapiv1.ParentReference{ + Group: gatewayapi.GroupPtr(gwapiv1.GroupName), + Kind: gatewayapi.KindPtr(resource.KindGateway), + Name: "other-controller-ancestor", + Namespace: gatewayapi.NamespacePtr(policy.Namespace), + }, + ControllerName: "gateway.envoyproxy.io/other-gatewayclass-controller", + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.PolicyConditionAccepted), + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.NewTime(time.Now()), + Reason: string(gwapiv1.PolicyConditionAccepted), + }, + }, + } + policy.Status.Ancestors = append(policy.Status.Ancestors, otherControllerAncestor) + err = suite.Client.Status().Update(context.Background(), policy) + require.NoErrorf(t, err, "error updating BackendTrafficPolicy status %s", policyNamespacedName.String()) + + // Change the policy spec to have a corresponding ancestor and trigger reconciliation. + suite.Applier.MustApplyWithCleanup(t, suite.Client, suite.TimeoutConfig, "testdata/policy-status-cleanup-no-ancestor.yaml", false) + + // Check its status to only have the ancestor from the other controller. + BackendTrafficPolicyMustBeAcceptedByAllAncestors(t, + suite.Client, + policyNamespacedName, + "gateway.envoyproxy.io/other-gatewayclass-controller", + []gwapiv1.ParentReference{ + { + Group: gatewayapi.GroupPtr(gwapiv1.GroupName), + Kind: gatewayapi.KindPtr(resource.KindGateway), + Name: "other-controller-ancestor", + Namespace: gatewayapi.NamespacePtr(policy.Namespace), + }, + }..., + ) + }) + }, +} + +var PolicyStatusCleanupMultipleGatewayClasses = suite.ConformanceTest{ + ShortName: "PolicyStatusCleanupMultipleGatewayClasses", + Description: "Testing Policy Status Cleanup With Ancestors of Multiple GatewayClasses", + Manifests: []string{"testdata/policy-status-cleanup-multiple-ancestors-multiple-gwcs.yaml"}, + Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { + t.Run("PolicyStatusCleanup", func(t *testing.T) { + // Create the second gateway of a different gatewayclass, which the backendtrafficpolicy is already attached to. + prevGwc := suite.Applier.GatewayClass + suite.Applier.GatewayClass = "status-cleanup" + suite.Applier.MustApplyWithCleanup(t, suite.Client, suite.TimeoutConfig, "testdata/status-cleanup-gateway-different-gwc.yaml", true) + suite.Applier.GatewayClass = prevGwc + + ns := "gateway-conformance-infra" + gw1NN, gw2NN := types.NamespacedName{Name: "same-namespace", Namespace: ns}, types.NamespacedName{Name: "gateway-2", Namespace: ns} + + // Check the policy has two ancestors in its status. + BackendTrafficPolicyMustBeAcceptedByAllAncestors(t, + suite.Client, + types.NamespacedName{Name: "backendtrafficpolicy-multiple-ancestors-same-gwc", Namespace: ns}, + suite.ControllerName, + gwapiv1.ParentReference{ + Group: gatewayapi.GroupPtr(gwapiv1.GroupName), + Kind: gatewayapi.KindPtr(resource.KindGateway), + Namespace: gatewayapi.NamespacePtr(gw1NN.Namespace), + Name: gwapiv1.ObjectName(gw1NN.Name), + }, + gwapiv1.ParentReference{ + Group: gatewayapi.GroupPtr(gwapiv1.GroupName), + Kind: gatewayapi.KindPtr(resource.KindGateway), + Namespace: gatewayapi.NamespacePtr(gw2NN.Namespace), + Name: gwapiv1.ObjectName(gw2NN.Name), + }, + ) + + // Change the policy to have a single ancestor, and check its status. + suite.Applier.MustApplyWithCleanup(t, suite.Client, suite.TimeoutConfig, "testdata/policy-status-cleanup-single-ancestor.yaml", false) + BackendTrafficPolicyMustBeAcceptedByAllAncestors(t, + suite.Client, + types.NamespacedName{Name: "backendtrafficpolicy-multiple-ancestors-same-gwc", Namespace: ns}, + suite.ControllerName, + gwapiv1.ParentReference{ + Group: gatewayapi.GroupPtr(gwapiv1.GroupName), + Kind: gatewayapi.KindPtr(resource.KindGateway), + Namespace: gatewayapi.NamespacePtr(gw1NN.Namespace), + Name: gwapiv1.ObjectName(gw1NN.Name), + }, + ) + }) + }, +} diff --git a/test/e2e/tests/route_status_cleanup.go b/test/e2e/tests/route_status_cleanup.go new file mode 100644 index 00000000000..3e6b10f075b --- /dev/null +++ b/test/e2e/tests/route_status_cleanup.go @@ -0,0 +1,105 @@ +// Copyright Envoy Gateway Authors +// SPDX-License-Identifier: Apache-2.0 +// The full text of the Apache license is available in the LICENSE file at +// the root of the repo. + +// This file contains code derived from upstream gateway-api, it will be moved to upstream. + +//go:build e2e + +package tests + +import ( + "testing" + + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/gateway-api/conformance/utils/http" + "sigs.k8s.io/gateway-api/conformance/utils/suite" +) + +func init() { + ConformanceTests = append(ConformanceTests, RouteStatusCleanupSameGatewayClass, RouteStatusCleanupMultipleGatewayClasses) +} + +var RouteStatusCleanupSameGatewayClass = suite.ConformanceTest{ + ShortName: "RouteStatusCleanupSameGatewayClass", + Description: "Testing Route Status Cleanup With Parents of The Same GatewayClass", + Manifests: []string{"testdata/route-status-cleanup-multiple-parents-same-gwc.yaml"}, + Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { + t.Run("RouteStatusCleanup", func(t *testing.T) { + ns := "gateway-conformance-infra" + routeNN := types.NamespacedName{Name: "tcp-route-status-cleanup", Namespace: ns} + gw1NN, gw2NN := types.NamespacedName{Name: "gateway-1", Namespace: ns}, types.NamespacedName{Name: "gateway-2", Namespace: ns} + gwRefs := []GatewayRef{NewGatewayRef(gw1NN), NewGatewayRef(gw2NN)} + gwAddrs := GatewayAndTCPRoutesMustBeAccepted(t, suite.Client, &suite.TimeoutConfig, suite.ControllerName, gwRefs, routeNN) + OkResp := http.ExpectedResponse{ + Request: http.Request{ + Path: "/", + }, + Response: http.Response{ + StatusCodes: []int{200}, + }, + Namespace: ns, + } + + // Send a request to an valid path and expect a successful response + require.Len(t, gwAddrs, 2) + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddrs[0], OkResp) + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddrs[1], OkResp) + + // Change the route to have a single parent, and check its status + suite.Applier.MustApplyWithCleanup(t, suite.Client, suite.TimeoutConfig, "testdata/route-status-cleanup-single-parent.yaml", false) + gwRefs = []GatewayRef{NewGatewayRef(gw1NN)} + gwAddrs = GatewayAndTCPRoutesMustBeAccepted(t, suite.Client, &suite.TimeoutConfig, suite.ControllerName, gwRefs, routeNN) + + // Send a request to an valid path and expect a successful response + require.Len(t, gwAddrs, 1) + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddrs[0], OkResp) + }) + }, +} + +var RouteStatusCleanupMultipleGatewayClasses = suite.ConformanceTest{ + ShortName: "RouteStatusCleanupMultipleGatewayClasses", + Description: "Testing Route Status Cleanup With Parents of Multiple GatewayClasses", + Manifests: []string{"testdata/route-status-cleanup-multiple-parents-multiple-gwcs.yaml"}, + Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { + t.Run("RouteStatusCleanup", func(t *testing.T) { + // Create the second gateway of a different gatewayclass, which the route is already attached to. + prevGwc := suite.Applier.GatewayClass + suite.Applier.GatewayClass = "status-cleanup" + suite.Applier.MustApplyWithCleanup(t, suite.Client, suite.TimeoutConfig, "testdata/status-cleanup-gateway-different-gwc.yaml", true) + suite.Applier.GatewayClass = prevGwc + + ns := "gateway-conformance-infra" + routeNN := types.NamespacedName{Name: "tcp-route-status-cleanup", Namespace: ns} + gw1NN, gw2NN := types.NamespacedName{Name: "gateway-1", Namespace: ns}, types.NamespacedName{Name: "gateway-2", Namespace: ns} + gwRefs := []GatewayRef{NewGatewayRef(gw1NN), NewGatewayRef(gw2NN)} + gwAddrs := GatewayAndTCPRoutesMustBeAccepted(t, suite.Client, &suite.TimeoutConfig, suite.ControllerName, gwRefs, routeNN) + OkResp := http.ExpectedResponse{ + Request: http.Request{ + Path: "/", + }, + Response: http.Response{ + StatusCodes: []int{200}, + }, + Namespace: ns, + } + + // Send a request to an valid path and expect a successful response + require.Len(t, gwAddrs, 2) + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddrs[0], OkResp) + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddrs[1], OkResp) + + // Change the route to have a single parent, and check its status + suite.Applier.MustApplyWithCleanup(t, suite.Client, suite.TimeoutConfig, "testdata/route-status-cleanup-single-parent.yaml", false) + gwRefs = []GatewayRef{NewGatewayRef(gw1NN)} + gwAddrs = GatewayAndTCPRoutesMustBeAccepted(t, suite.Client, &suite.TimeoutConfig, suite.ControllerName, gwRefs, routeNN) + + // Send a request to an valid path and expect a successful response + require.Len(t, gwAddrs, 1) + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddrs[0], OkResp) + }) + }, +} diff --git a/test/e2e/tests/stat_name.go b/test/e2e/tests/stat_name.go index 754b2476c85..926cc69317f 100644 --- a/test/e2e/tests/stat_name.go +++ b/test/e2e/tests/stat_name.go @@ -121,7 +121,7 @@ var TCPRouteStatNameTest = suite.ConformanceTest{ ns := "gateway-conformance-infra" routeNN := types.NamespacedName{Name: "tcp-route-stat-name", Namespace: ns} gwNN := types.NamespacedName{Name: "tcp-stat-name-backend-gateway", Namespace: ns} - gwAddr := GatewayAndTCPRoutesMustBeAccepted(t, suite.Client, &suite.TimeoutConfig, suite.ControllerName, NewGatewayRef(gwNN), routeNN) + gwAddrs := GatewayAndTCPRoutesMustBeAccepted(t, suite.Client, &suite.TimeoutConfig, suite.ControllerName, []GatewayRef{NewGatewayRef(gwNN)}, routeNN) t.Run("prometheus", func(t *testing.T) { expectedResponse := httputils.ExpectedResponse{ @@ -135,7 +135,7 @@ var TCPRouteStatNameTest = suite.ConformanceTest{ } // make sure listener is ready - httputils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectedResponse) + httputils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddrs[0], expectedResponse) verifyMetrics(t, suite, `envoy_tcp_downstream_cx_total{envoy_tcp_prefix="gateway-conformance-infra/tcp-route-stat-name"}`) }) }, diff --git a/test/e2e/tests/tcproute.go b/test/e2e/tests/tcproute.go index 877663ceac3..fe57fa8dc5d 100644 --- a/test/e2e/tests/tcproute.go +++ b/test/e2e/tests/tcproute.go @@ -44,7 +44,8 @@ var TCPRouteTest = suite.ConformanceTest{ ns := "gateway-conformance-infra" routeNN := types.NamespacedName{Name: "tcp-app-1", Namespace: ns} gwNN := types.NamespacedName{Name: "my-tcp-gateway", Namespace: ns} - gwAddr := GatewayAndTCPRoutesMustBeAccepted(t, suite.Client, &suite.TimeoutConfig, suite.ControllerName, NewGatewayRef(gwNN), routeNN) + gwRefs := []GatewayRef{NewGatewayRef(gwNN)} + gwAddrs := GatewayAndTCPRoutesMustBeAccepted(t, suite.Client, &suite.TimeoutConfig, suite.ControllerName, gwRefs, routeNN) OkResp := http.ExpectedResponse{ Request: http.Request{ Path: "/", @@ -56,13 +57,15 @@ var TCPRouteTest = suite.ConformanceTest{ } // Send a request to an valid path and expect a successful response - http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, OkResp) + require.Len(t, gwAddrs, 1) + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddrs[0], OkResp) }) t.Run("tcp-route-2", func(t *testing.T) { ns := "gateway-conformance-infra" routeNN := types.NamespacedName{Name: "tcp-app-2", Namespace: ns} gwNN := types.NamespacedName{Name: "my-tcp-gateway", Namespace: ns} - gwAddr := GatewayAndTCPRoutesMustBeAccepted(t, suite.Client, &suite.TimeoutConfig, suite.ControllerName, NewGatewayRef(gwNN), routeNN) + gwRefs := []GatewayRef{NewGatewayRef(gwNN)} + gwAddrs := GatewayAndTCPRoutesMustBeAccepted(t, suite.Client, &suite.TimeoutConfig, suite.ControllerName, gwRefs, routeNN) OkResp := http.ExpectedResponse{ Request: http.Request{ Path: "/", @@ -74,12 +77,13 @@ var TCPRouteTest = suite.ConformanceTest{ } // Send a request to an valid path and expect a successful response - http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, OkResp) + require.Len(t, gwAddrs, 1) + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddrs[0], OkResp) }) }, } -func GatewayAndTCPRoutesMustBeAccepted(t *testing.T, c client.Client, timeoutConfig *config.TimeoutConfig, controllerName string, gw GatewayRef, routeNNs ...types.NamespacedName) string { +func GatewayAndTCPRoutesMustBeAccepted(t *testing.T, c client.Client, timeoutConfig *config.TimeoutConfig, controllerName string, gws []GatewayRef, routeNNs ...types.NamespacedName) []string { t.Helper() if timeoutConfig == nil { @@ -92,40 +96,42 @@ func GatewayAndTCPRoutesMustBeAccepted(t *testing.T, c client.Client, timeoutCon tlog.Logf(t, "error fetching TCPRoute: %v", err) } - gwAddr, err := WaitForGatewayAddress(t, c, timeoutConfig, gw.NamespacedName, string(*tcpRoute.Spec.ParentRefs[0].SectionName)) - require.NoErrorf(t, err, "timed out waiting for Gateway address to be assigned") - - ns := gwapiv1.Namespace(gw.Namespace) - kind := gwapiv1.Kind("Gateway") + gwAddrs := make([]string, 0, len(gws)) + for _, gw := range gws { + gwAddr, err := WaitForGatewayAddress(t, c, timeoutConfig, gw.NamespacedName, string(*tcpRoute.Spec.ParentRefs[0].SectionName)) + require.NoErrorf(t, err, "timed out waiting for Gateway address to be assigned") + gwAddrs = append(gwAddrs, gwAddr) + } for _, routeNN := range routeNNs { - namespaceRequired := true - if routeNN.Namespace == gw.Namespace { - namespaceRequired = false - } - var parents []gwapiv1.RouteParentStatus - for _, listener := range gw.listenerNames { - parents = append(parents, gwapiv1.RouteParentStatus{ - ParentRef: gwapiv1.ParentReference{ - Group: (*gwapiv1.Group)(&gwapiv1.GroupVersion.Group), - Kind: &kind, - Name: gwapiv1.ObjectName(gw.Name), - Namespace: &ns, - SectionName: listener, - }, - ControllerName: gwapiv1.GatewayController(controllerName), - Conditions: []metav1.Condition{{ - Type: string(gwapiv1.RouteConditionAccepted), - Status: metav1.ConditionTrue, - Reason: string(gwapiv1.RouteReasonAccepted), - }}, - }) + var namespaceRequired []bool + for _, gw := range gws { + ns := gwapiv1.Namespace(gw.Namespace) + kind := gwapiv1.Kind("Gateway") + namespaceRequired = append(namespaceRequired, routeNN.Namespace != gw.Namespace) + for _, listener := range gw.listenerNames { + parents = append(parents, gwapiv1.RouteParentStatus{ + ParentRef: gwapiv1.ParentReference{ + Group: (*gwapiv1.Group)(&gwapiv1.GroupVersion.Group), + Kind: &kind, + Name: gwapiv1.ObjectName(gw.Name), + Namespace: &ns, + SectionName: listener, + }, + ControllerName: gwapiv1.GatewayController(controllerName), + Conditions: []metav1.Condition{{ + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + Reason: string(gwapiv1.RouteReasonAccepted), + }}, + }) + } } TCPRouteMustHaveParents(t, c, timeoutConfig, routeNN, parents, namespaceRequired) } - return gwAddr + return gwAddrs } // WaitForGatewayAddress waits until at least one IP Address has been set in the @@ -174,7 +180,7 @@ func WaitForGatewayAddress(t *testing.T, client client.Client, timeoutConfig *co return net.JoinHostPort(ipAddr, port), waitErr } -func TCPRouteMustHaveParents(t *testing.T, client client.Client, timeoutConfig *config.TimeoutConfig, routeName types.NamespacedName, parents []gwapiv1.RouteParentStatus, namespaceRequired bool) { +func TCPRouteMustHaveParents(t *testing.T, client client.Client, timeoutConfig *config.TimeoutConfig, routeName types.NamespacedName, parents []gwapiv1.RouteParentStatus, namespaceRequired []bool) { t.Helper() if timeoutConfig == nil { diff --git a/test/e2e/tests/tcproute_authorization_client_ip.go b/test/e2e/tests/tcproute_authorization_client_ip.go index a1a21d5af9a..b25d08a0e3c 100644 --- a/test/e2e/tests/tcproute_authorization_client_ip.go +++ b/test/e2e/tests/tcproute_authorization_client_ip.go @@ -36,7 +36,7 @@ var TCPRouteAuthzWithClientIP = suite.ConformanceTest{ tcpRouteNN := types.NamespacedName{Name: "tcp-backend-authorization-ip", Namespace: ns} tcpRouteFqdnNN := types.NamespacedName{Name: "tcp-backend-authorization-fqdn", Namespace: ns} gwNN := types.NamespacedName{Name: "tcp-authorization-backend", Namespace: ns} - GatewayAndTCPRoutesMustBeAccepted(t, suite.Client, &suite.TimeoutConfig, suite.ControllerName, NewGatewayRef(gwNN), tcpRouteNN, tcpRouteFqdnNN) + GatewayAndTCPRoutesMustBeAccepted(t, suite.Client, &suite.TimeoutConfig, suite.ControllerName, []GatewayRef{NewGatewayRef(gwNN)}, tcpRouteNN, tcpRouteFqdnNN) // Test the blocked route (ip section) ipSection := gwapiv1.SectionName("ip") @@ -74,10 +74,10 @@ func testTCPRouteWithBackendBlocked(t *testing.T, suite *suite.ConformanceTestSu ns := "gateway-conformance-infra" routeNN := types.NamespacedName{Name: routeName, Namespace: ns} gwNN := types.NamespacedName{Name: gwName, Namespace: ns} - gwAddr := GatewayAndTCPRoutesMustBeAccepted(t, suite.Client, &suite.TimeoutConfig, suite.ControllerName, NewGatewayRef(gwNN), routeNN) + gwAddr := GatewayAndTCPRoutesMustBeAccepted(t, suite.Client, &suite.TimeoutConfig, suite.ControllerName, []GatewayRef{NewGatewayRef(gwNN)}, routeNN) BackendMustBeAccepted(t, suite.Client, types.NamespacedName{Name: backendName, Namespace: ns}) - testTCPConnectionBlocked(t, gwAddr) + testTCPConnectionBlocked(t, gwAddr[0]) } func testTCPConnectionBlocked(t *testing.T, gwAddr string) { diff --git a/test/e2e/tests/tcproute_with_backend.go b/test/e2e/tests/tcproute_with_backend.go index f7f68db6b2b..ebd0c759be9 100644 --- a/test/e2e/tests/tcproute_with_backend.go +++ b/test/e2e/tests/tcproute_with_backend.go @@ -12,6 +12,7 @@ package tests import ( "testing" + "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/gateway-api/conformance/utils/http" "sigs.k8s.io/gateway-api/conformance/utils/suite" @@ -61,7 +62,7 @@ func testTCPRouteWithBackend(t *testing.T, suite *suite.ConformanceTestSuite, gw ns := "gateway-conformance-infra" routeNN := types.NamespacedName{Name: routeName, Namespace: ns} gwNN := types.NamespacedName{Name: gwName, Namespace: ns} - gwAddr := GatewayAndTCPRoutesMustBeAccepted(t, suite.Client, &suite.TimeoutConfig, suite.ControllerName, NewGatewayRef(gwNN), routeNN) + gwAddrs := GatewayAndTCPRoutesMustBeAccepted(t, suite.Client, &suite.TimeoutConfig, suite.ControllerName, []GatewayRef{NewGatewayRef(gwNN)}, routeNN) BackendMustBeAccepted(t, suite.Client, types.NamespacedName{Name: backendName, Namespace: ns}) OkResp := http.ExpectedResponse{ Request: http.Request{ @@ -74,5 +75,6 @@ func testTCPRouteWithBackend(t *testing.T, suite *suite.ConformanceTestSuite, gw } // Send a request to a valid path and expect a successful response - http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, OkResp) + require.Len(t, gwAddrs, 1) + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddrs[0], OkResp) } diff --git a/test/e2e/tests/udproute.go b/test/e2e/tests/udproute.go index 6174917d5a2..32bfd802000 100644 --- a/test/e2e/tests/udproute.go +++ b/test/e2e/tests/udproute.go @@ -136,13 +136,13 @@ func GatewayAndUDPRoutesMustBeAccepted(t *testing.T, c client.Client, timeoutCon }, }) } - UDPRouteMustHaveParents(t, c, timeoutConfig, routeNN, parents, namespaceRequired) + UDPRouteMustHaveParents(t, c, timeoutConfig, routeNN, parents, []bool{namespaceRequired}) } return gwAddr } -func UDPRouteMustHaveParents(t *testing.T, client client.Client, timeoutConfig *config.TimeoutConfig, routeName types.NamespacedName, parents []gwapiv1.RouteParentStatus, namespaceRequired bool) { +func UDPRouteMustHaveParents(t *testing.T, client client.Client, timeoutConfig *config.TimeoutConfig, routeName types.NamespacedName, parents []gwapiv1.RouteParentStatus, namespaceRequired []bool) { t.Helper() if timeoutConfig == nil { @@ -163,7 +163,7 @@ func UDPRouteMustHaveParents(t *testing.T, client client.Client, timeoutConfig * require.NoErrorf(t, waitErr, "error waiting for UDPRoute to have parents matching expectations") } -func parentsForRouteMatch(t *testing.T, routeName types.NamespacedName, expected, actual []gwapiv1.RouteParentStatus, namespaceRequired bool) bool { +func parentsForRouteMatch(t *testing.T, routeName types.NamespacedName, expected, actual []gwapiv1.RouteParentStatus, namespaceRequired []bool) bool { t.Helper() if len(expected) != len(actual) { @@ -190,7 +190,7 @@ func parentsForRouteMatch(t *testing.T, routeName types.NamespacedName, expected return false } if !reflect.DeepEqual(actualParent.ParentRef.Namespace, expectedParent.ParentRef.Namespace) { - if namespaceRequired || actualParent.ParentRef.Namespace != nil { + if namespaceRequired[i] || actualParent.ParentRef.Namespace != nil { tlog.Logf(t, "Route %s/%s expected ParentReference.Namespace to be %v, got %v", routeName.Namespace, routeName.Name, expectedParent.ParentRef.Namespace, actualParent.ParentRef.Namespace) return false } diff --git a/test/e2e/tests/utils.go b/test/e2e/tests/utils.go index f0b8dc4fdfc..d160700134e 100644 --- a/test/e2e/tests/utils.go +++ b/test/e2e/tests/utils.go @@ -188,6 +188,31 @@ func BackendTrafficPolicyMustBeAccepted(t *testing.T, client client.Client, poli require.NoErrorf(t, waitErr, "error waiting for BackendTrafficPolicy to be accepted") } +// BackendTrafficPolicyMustBeAccepted waits for the specified BackendTrafficPolicy to be accepted. +func BackendTrafficPolicyMustBeAcceptedByAllAncestors( + t *testing.T, client client.Client, policyName types.NamespacedName, + controllerName string, ancestorRefs ...gwapiv1.ParentReference, +) { + t.Helper() + + waitErr := wait.PollUntilContextTimeout(context.Background(), 1*time.Second, 60*time.Second, true, func(ctx context.Context) (bool, error) { + policy := &egv1a1.BackendTrafficPolicy{} + err := client.Get(ctx, policyName, policy) + if err != nil { + return false, fmt.Errorf("error fetching BackendTrafficPolicy: %w", err) + } + + if ancestorsForPolicyMatch(t, policyName, ancestorRefs, policy.Status.Ancestors, controllerName) { + return true, nil + } + + tlog.Logf(t, "BackendTrafficPolicy not yet accepted: %v", policy) + return false, nil + }) + + require.NoErrorf(t, waitErr, "error waiting for BackendTrafficPolicy to be accepted") +} + // BackendTrafficPolicyMustFail waits for an BackendTrafficPolicy to fail with the specified reason. func BackendTrafficPolicyMustFail( t *testing.T, client client.Client, policyName types.NamespacedName, @@ -314,6 +339,36 @@ func policyAcceptedByAncestor(ancestors []gwapiv1.PolicyAncestorStatus, controll return false } +func ancestorsForPolicyMatch(t *testing.T, policyName types.NamespacedName, expected []gwapiv1.ParentReference, actual []gwapiv1.PolicyAncestorStatus, controllerName string) bool { + t.Helper() + + if len(expected) != len(actual) { + tlog.Logf(t, "Policy %s/%s expected %d ancestors got %d", policyName.Namespace, policyName.Name, len(expected), len(actual)) + return false + } + + for i, expectedAncestor := range expected { + actualAncestor := actual[i] + accepted := false + if string(actualAncestor.ControllerName) == controllerName && cmp.Equal(actualAncestor.AncestorRef, expectedAncestor) { + for _, condition := range actualAncestor.Conditions { + if condition.Type == string(gwapiv1.PolicyConditionAccepted) && condition.Status == metav1.ConditionTrue { + accepted = true + break + } + } + if !accepted { + tlog.Logf(t, "Policy %s/%s expected Accepted condition on ancestor %s", policyName.Namespace, policyName.Name, actualAncestor.AncestorRef.Name) + return false + } + } else { + tlog.Logf(t, "Policy %s/%s expected Ancestor %s", policyName.Namespace, policyName.Name, actualAncestor.AncestorRef.Name) + return false + } + } + return true +} + // EnvoyExtensionPolicyMustFail waits for an EnvoyExtensionPolicy to fail with the specified reason. func EnvoyExtensionPolicyMustFail( t *testing.T, client client.Client, policyName types.NamespacedName,