From f8dd034c4c386ee55b0347a61773f5d975ebb30e Mon Sep 17 00:00:00 2001 From: y-rabie Date: Wed, 19 Nov 2025 11:40:42 +0200 Subject: [PATCH 1/3] fix: aggregate xRoute/xPolicy statuses across GWCs in gateway-api runner Signed-off-by: y-rabie --- internal/gatewayapi/extensionserverpolicy.go | 41 ++- .../gatewayapi/extensionserverpolicy_test.go | 112 ++++++++ internal/gatewayapi/runner/runner.go | 239 +++++++++++------- 3 files changed, 293 insertions(+), 99 deletions(-) diff --git a/internal/gatewayapi/extensionserverpolicy.go b/internal/gatewayapi/extensionserverpolicy.go index e378c270ba1..8ecb208b967 100644 --- a/internal/gatewayapi/extensionserverpolicy.go +++ b/internal/gatewayapi/extensionserverpolicy.go @@ -81,7 +81,7 @@ func (t *Translator) ProcessExtensionServerPolicies(policies []unstructured.Unst } if accepted { res = append(res, *policy) - policy.Object["status"] = policyStatusToUnstructured(policyStatus) + policy.Object["status"] = PolicyStatusToUnstructured(policyStatus) } } @@ -108,14 +108,6 @@ func extractTargetRefs(policy *unstructured.Unstructured, gateways []*GatewayCon return ret, nil } -func policyStatusToUnstructured(policyStatus gwapiv1.PolicyStatus) map[string]any { - ret := map[string]any{} - // No need to check the marshal/unmarshal error here - d, _ := json.Marshal(policyStatus) - _ = json.Unmarshal(d, &ret) - return ret -} - func resolveExtServerPolicyGatewayTargetRef(policy *unstructured.Unstructured, target gwapiv1.LocalPolicyTargetReferenceWithSectionName, gateways map[types.NamespacedName]*policyGatewayTargetContext) *GatewayContext { // Check if the gateway exists key := types.NamespacedName{ @@ -132,6 +124,29 @@ func resolveExtServerPolicyGatewayTargetRef(policy *unstructured.Unstructured, t return gateway.GatewayContext } +func PolicyStatusToUnstructured(policyStatus gwapiv1.PolicyStatus) map[string]any { + ret := map[string]any{} + // No need to check the marshal/unmarshal error here + d, _ := json.Marshal(policyStatus) + _ = json.Unmarshal(d, &ret) + return ret +} + +func ExtServerPolicyStatusAsPolicyStatus(policy *unstructured.Unstructured) gwapiv1.PolicyStatus { + statusObj := policy.Object["status"] + status := gwapiv1.PolicyStatus{} + if _, ok := statusObj.(map[string]any); ok { + // No need to check the json marshal/unmarshal error, the policyStatus was + // created via a typed object so the marshalling/unmarshalling will always + // work + d, _ := json.Marshal(statusObj) + _ = json.Unmarshal(d, &status) + } else if _, ok := statusObj.(gwapiv1.PolicyStatus); ok { + status = statusObj.(gwapiv1.PolicyStatus) + } + return status +} + func (t *Translator) translateExtServerPolicyForGateway( policy *unstructured.Unstructured, gateway *GatewayContext, @@ -173,3 +188,11 @@ func (t *Translator) translateExtServerPolicyForGateway( } return found } + +// Appends status ancestors from newPolicy into aggregatedPolicy's list of ancestors. +func MergeAncestorsForExtensionServerPolicies(aggregatedPolicy, newPolicy *unstructured.Unstructured) { + aggStatus := ExtServerPolicyStatusAsPolicyStatus(aggregatedPolicy) + newStatus := ExtServerPolicyStatusAsPolicyStatus(newPolicy) + aggStatus.Ancestors = append(aggStatus.Ancestors, newStatus.Ancestors...) + aggregatedPolicy.Object["status"] = PolicyStatusToUnstructured(aggStatus) +} diff --git a/internal/gatewayapi/extensionserverpolicy_test.go b/internal/gatewayapi/extensionserverpolicy_test.go index fbfc8418d6f..73a9477e6e5 100644 --- a/internal/gatewayapi/extensionserverpolicy_test.go +++ b/internal/gatewayapi/extensionserverpolicy_test.go @@ -123,3 +123,115 @@ func TestExtractTargetRefs(t *testing.T) { }) } } + +func TestMergeAncestorsForExtensionServerPolicies(t *testing.T) { + tests := []struct { + aggStatus *gwapiv1.PolicyStatus + newStatus *gwapiv1.PolicyStatus + noStatus bool + }{ + { + aggStatus: &gwapiv1.PolicyStatus{ + Ancestors: []gwapiv1.PolicyAncestorStatus{ + { + AncestorRef: gwapiv1.ParentReference{ + Name: "gateway-1", + }, + }, + }, + }, + newStatus: &gwapiv1.PolicyStatus{ + Ancestors: []gwapiv1.PolicyAncestorStatus{ + { + AncestorRef: gwapiv1.ParentReference{ + Name: "gateway-2", + }, + }, + }, + }, + }, + { + aggStatus: &gwapiv1.PolicyStatus{}, + newStatus: &gwapiv1.PolicyStatus{ + Ancestors: []gwapiv1.PolicyAncestorStatus{ + { + AncestorRef: gwapiv1.ParentReference{ + Name: "gateway-2", + }, + }, + }, + }, + }, + { + aggStatus: &gwapiv1.PolicyStatus{ + Ancestors: []gwapiv1.PolicyAncestorStatus{ + { + AncestorRef: gwapiv1.ParentReference{ + Name: "gateway-1", + }, + }, + }, + }, + newStatus: &gwapiv1.PolicyStatus{}, + }, + { + aggStatus: &gwapiv1.PolicyStatus{}, + newStatus: &gwapiv1.PolicyStatus{}, + }, + { + aggStatus: nil, + newStatus: &gwapiv1.PolicyStatus{ + Ancestors: []gwapiv1.PolicyAncestorStatus{ + { + AncestorRef: gwapiv1.ParentReference{ + Name: "gateway-1", + }, + }, + }, + }, + }, + { + aggStatus: &gwapiv1.PolicyStatus{ + Ancestors: []gwapiv1.PolicyAncestorStatus{ + { + AncestorRef: gwapiv1.ParentReference{ + Name: "gateway-1", + }, + }, + }, + }, + newStatus: nil, + }, + { + aggStatus: nil, + newStatus: nil, + }, + } + + for _, test := range tests { + aggPolicy := unstructured.Unstructured{Object: make(map[string]interface{})} + newPolicy := unstructured.Unstructured{Object: make(map[string]interface{})} + desiredMergedStatus := gwapiv1.PolicyStatus{} + + // aggStatus == nil, means simulate not setting status at all within the policy. + if test.aggStatus != nil { + aggPolicy.Object["status"] = PolicyStatusToUnstructured(*test.aggStatus) + desiredMergedStatus.Ancestors = append(desiredMergedStatus.Ancestors, test.aggStatus.Ancestors...) + } + + // newStatus == nil, means simulate not setting status at all within the policy. + if test.newStatus != nil { + newPolicy.Object["status"] = PolicyStatusToUnstructured(*test.newStatus) + desiredMergedStatus.Ancestors = append(desiredMergedStatus.Ancestors, test.newStatus.Ancestors...) + } + + MergeAncestorsForExtensionServerPolicies(&aggPolicy, &newPolicy) + + // The product object will always have an existing `status`, even if with 0 ancestors. + newAggPolicy := ExtServerPolicyStatusAsPolicyStatus(&aggPolicy) + require.Len(t, newAggPolicy.Ancestors, len(desiredMergedStatus.Ancestors)) + for i := range newAggPolicy.Ancestors { + require.Equal(t, desiredMergedStatus.Ancestors[i].AncestorRef.Name, newAggPolicy.Ancestors[i].AncestorRef.Name) + } + } +} diff --git a/internal/gatewayapi/runner/runner.go b/internal/gatewayapi/runner/runner.go index f3852f773d6..e032d87cd75 100644 --- a/internal/gatewayapi/runner/runner.go +++ b/internal/gatewayapi/runner/runner.go @@ -8,11 +8,11 @@ package runner import ( "context" "crypto/tls" - "encoding/json" "fmt" "os" "path" "path/filepath" + "reflect" "github.com/docker/docker/pkg/fileutils" "github.com/telepresenceio/watchable" @@ -23,6 +23,7 @@ import ( "k8s.io/client-go/kubernetes" ctrl "sigs.k8s.io/controller-runtime" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" + gwapiv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" "github.com/envoyproxy/gateway/internal/crypto" @@ -143,6 +144,29 @@ func (r *Runner) subscribeAndTranslate(sub <-chan watchable.Snapshot[string, *re var backendTLSPolicyStatusCount, clientTrafficPolicyStatusCount, backendTrafficPolicyStatusCount int var securityPolicyStatusCount, envoyExtensionPolicyStatusCount, backendStatusCount, extensionServerPolicyStatusCount int + // `aggregatedStatuses` aggregates status result of resources from all + // parents/ancestors, and then stores the status once for every resource. + aggregatedStatuses := struct { + Backends map[types.NamespacedName]*egv1a1.BackendStatus + Gateways map[types.NamespacedName]*gwapiv1.GatewayStatus + HTTPRoutes map[types.NamespacedName][]*gwapiv1.RouteStatus + GRPCRoutes map[types.NamespacedName][]*gwapiv1.RouteStatus + TLSRoutes map[types.NamespacedName][]*gwapiv1a2.RouteStatus + TCPRoutes map[types.NamespacedName][]*gwapiv1a2.RouteStatus + UDPRoutes map[types.NamespacedName][]*gwapiv1a2.RouteStatus + BackendTLSPolicies map[types.NamespacedName][]*gwapiv1.PolicyStatus + ClientTrafficPolicies map[types.NamespacedName][]*gwapiv1.PolicyStatus + BackendTrafficPolicies map[types.NamespacedName][]*gwapiv1.PolicyStatus + SecurityPolicies map[types.NamespacedName][]*gwapiv1.PolicyStatus + EnvoyExtensionPolicies map[types.NamespacedName][]*gwapiv1.PolicyStatus + ExtensionServerPolicies map[message.NamespacedNameAndGVK][]*gwapiv1.PolicyStatus + }{} + v := reflect.ValueOf(&aggregatedStatuses).Elem() + for i := 0; i < v.NumField(); i++ { + field := v.Field(i) + field.Set(reflect.MakeMap(field.Type())) + } + for _, resources := range *val { // Translate and publish IRs. t := &gatewayapi.Translator{ @@ -218,125 +242,170 @@ func (r *Runner) subscribeAndTranslate(sub <-chan watchable.Snapshot[string, *re r.ProviderResources.GatewayClassStatuses.Store(key, &result.GatewayClass.Status) } + // Aggregate status (parents/ancestors) from all GatewayClasses. for _, gateway := range result.Gateways { - key := utils.NamespacedName(gateway) - r.ProviderResources.GatewayStatuses.Store(key, &gateway.Status) - gatewayStatusCount++ - delete(keysToDelete.GatewayStatus, key) - r.keyCache.GatewayStatus[key] = true + aggregatedStatuses.Gateways[utils.NamespacedName(gateway)] = &gateway.Status } for _, httpRoute := range result.HTTPRoutes { key := utils.NamespacedName(httpRoute) - r.ProviderResources.HTTPRouteStatuses.Store(key, &httpRoute.Status) - httpRouteStatusCount++ - delete(keysToDelete.HTTPRouteStatus, key) - r.keyCache.HTTPRouteStatus[key] = true + aggregatedStatuses.HTTPRoutes[key] = append(aggregatedStatuses.HTTPRoutes[key], &httpRoute.Status.RouteStatus) } for _, grpcRoute := range result.GRPCRoutes { key := utils.NamespacedName(grpcRoute) - r.ProviderResources.GRPCRouteStatuses.Store(key, &grpcRoute.Status) - grpcRouteStatusCount++ - delete(keysToDelete.GRPCRouteStatus, key) - r.keyCache.GRPCRouteStatus[key] = true + aggregatedStatuses.GRPCRoutes[key] = append(aggregatedStatuses.GRPCRoutes[key], &grpcRoute.Status.RouteStatus) } for _, tlsRoute := range result.TLSRoutes { key := utils.NamespacedName(tlsRoute) - r.ProviderResources.TLSRouteStatuses.Store(key, &tlsRoute.Status) - tlsRouteStatusCount++ - delete(keysToDelete.TLSRouteStatus, key) - r.keyCache.TLSRouteStatus[key] = true + aggregatedStatuses.TLSRoutes[key] = append(aggregatedStatuses.TLSRoutes[key], &tlsRoute.Status.RouteStatus) } for _, tcpRoute := range result.TCPRoutes { key := utils.NamespacedName(tcpRoute) - r.ProviderResources.TCPRouteStatuses.Store(key, &tcpRoute.Status) - tcpRouteStatusCount++ - delete(keysToDelete.TCPRouteStatus, key) - r.keyCache.TCPRouteStatus[key] = true + aggregatedStatuses.TCPRoutes[key] = append(aggregatedStatuses.TCPRoutes[key], &tcpRoute.Status.RouteStatus) } for _, udpRoute := range result.UDPRoutes { key := utils.NamespacedName(udpRoute) - r.ProviderResources.UDPRouteStatuses.Store(key, &udpRoute.Status) - udpRouteStatusCount++ - delete(keysToDelete.UDPRouteStatus, key) - r.keyCache.UDPRouteStatus[key] = true + aggregatedStatuses.UDPRoutes[key] = append(aggregatedStatuses.UDPRoutes[key], &udpRoute.Status.RouteStatus) } - - // Skip updating status for policies with empty status - // They may have been skipped in this translation because - // their target is not found (not relevant) - for _, backendTLSPolicy := range result.BackendTLSPolicies { key := utils.NamespacedName(backendTLSPolicy) - if len(backendTLSPolicy.Status.Ancestors) > 0 { - r.ProviderResources.BackendTLSPolicyStatuses.Store(key, &backendTLSPolicy.Status) - backendTLSPolicyStatusCount++ - } - delete(keysToDelete.BackendTLSPolicyStatus, key) - r.keyCache.BackendTLSPolicyStatus[key] = true + aggregatedStatuses.BackendTLSPolicies[key] = append(aggregatedStatuses.BackendTLSPolicies[key], &backendTLSPolicy.Status) } - for _, clientTrafficPolicy := range result.ClientTrafficPolicies { key := utils.NamespacedName(clientTrafficPolicy) - if len(clientTrafficPolicy.Status.Ancestors) > 0 { - r.ProviderResources.ClientTrafficPolicyStatuses.Store(key, &clientTrafficPolicy.Status) - clientTrafficPolicyStatusCount++ - } - delete(keysToDelete.ClientTrafficPolicyStatus, key) - r.keyCache.ClientTrafficPolicyStatus[key] = true + aggregatedStatuses.ClientTrafficPolicies[key] = append(aggregatedStatuses.ClientTrafficPolicies[key], &clientTrafficPolicy.Status) } for _, backendTrafficPolicy := range result.BackendTrafficPolicies { key := utils.NamespacedName(backendTrafficPolicy) - if len(backendTrafficPolicy.Status.Ancestors) > 0 { - r.ProviderResources.BackendTrafficPolicyStatuses.Store(key, &backendTrafficPolicy.Status) - backendTrafficPolicyStatusCount++ - } - delete(keysToDelete.BackendTrafficPolicyStatus, key) - r.keyCache.BackendTrafficPolicyStatus[key] = true + aggregatedStatuses.BackendTrafficPolicies[key] = append(aggregatedStatuses.BackendTrafficPolicies[key], &backendTrafficPolicy.Status) } for _, securityPolicy := range result.SecurityPolicies { key := utils.NamespacedName(securityPolicy) - if len(securityPolicy.Status.Ancestors) > 0 { - r.ProviderResources.SecurityPolicyStatuses.Store(key, &securityPolicy.Status) - securityPolicyStatusCount++ - } - delete(keysToDelete.SecurityPolicyStatus, key) - r.keyCache.SecurityPolicyStatus[key] = true + aggregatedStatuses.SecurityPolicies[key] = append(aggregatedStatuses.SecurityPolicies[key], &securityPolicy.Status) } for _, envoyExtensionPolicy := range result.EnvoyExtensionPolicies { key := utils.NamespacedName(envoyExtensionPolicy) - if len(envoyExtensionPolicy.Status.Ancestors) > 0 { - r.ProviderResources.EnvoyExtensionPolicyStatuses.Store(key, &envoyExtensionPolicy.Status) - envoyExtensionPolicyStatusCount++ - } - delete(keysToDelete.EnvoyExtensionPolicyStatus, key) - r.keyCache.EnvoyExtensionPolicyStatus[key] = true - } - for _, backend := range result.Backends { - key := utils.NamespacedName(backend) - if len(backend.Status.Conditions) > 0 { - r.ProviderResources.BackendStatuses.Store(key, &backend.Status) - backendStatusCount++ - } - delete(keysToDelete.BackendStatus, key) - r.keyCache.BackendStatus[key] = true + aggregatedStatuses.EnvoyExtensionPolicies[key] = append(aggregatedStatuses.EnvoyExtensionPolicies[key], &envoyExtensionPolicy.Status) } for _, extServerPolicy := range result.ExtensionServerPolicies { key := message.NamespacedNameAndGVK{ NamespacedName: utils.NamespacedName(&extServerPolicy), GroupVersionKind: extServerPolicy.GroupVersionKind(), } - if statusObj, hasStatus := extServerPolicy.Object["status"]; hasStatus && statusObj != nil { - if statusMap, ok := statusObj.(map[string]any); ok && len(statusMap) > 0 { - policyStatus := unstructuredToPolicyStatus(statusMap) - r.ProviderResources.ExtensionPolicyStatuses.Store(key, &policyStatus) - extensionServerPolicyStatusCount++ - } - } - delete(keysToDelete.ExtensionServerPolicyStatus, key) - r.keyCache.ExtensionServerPolicyStatus[key] = true + status := gatewayapi.ExtServerPolicyStatusAsPolicyStatus(&extServerPolicy) + aggregatedStatuses.ExtensionServerPolicies[key] = append(aggregatedStatuses.ExtensionServerPolicies[key], &status) + } + for _, backend := range result.Backends { + aggregatedStatuses.Backends[utils.NamespacedName(backend)] = &backend.Status } } + aggregateRouteStatus := func(routeStatuses []*gwapiv1.RouteStatus) gwapiv1.RouteStatus { + var status gwapiv1.RouteStatus + for _, routeStatus := range routeStatuses { + status.Parents = append(status.Parents, routeStatus.Parents...) + } + return status + } + aggregatePolicyStatus := func(policyStatuses []*gwapiv1.PolicyStatus) gwapiv1.PolicyStatus { + var status gwapiv1.PolicyStatus + for _, policyStatus := range policyStatuses { + status.Ancestors = append(status.Ancestors, policyStatus.Ancestors...) + } + return status + } + // Store the stauses of all objects atomically with the aggregated status. + for key, status := range aggregatedStatuses.Gateways { + r.ProviderResources.GatewayStatuses.Store(key, status) + gatewayStatusCount++ + delete(keysToDelete.GatewayStatus, key) + r.keyCache.GatewayStatus[key] = true + } + for key, statuses := range aggregatedStatuses.HTTPRoutes { + status := gwapiv1.HTTPRouteStatus{RouteStatus: aggregateRouteStatus(statuses)} + r.ProviderResources.HTTPRouteStatuses.Store(key, &status) + httpRouteStatusCount++ + delete(keysToDelete.HTTPRouteStatus, key) + r.keyCache.HTTPRouteStatus[key] = true + } + for key, statuses := range aggregatedStatuses.GRPCRoutes { + status := gwapiv1.GRPCRouteStatus{RouteStatus: aggregateRouteStatus(statuses)} + r.ProviderResources.GRPCRouteStatuses.Store(key, &status) + grpcRouteStatusCount++ + delete(keysToDelete.GRPCRouteStatus, key) + r.keyCache.GRPCRouteStatus[key] = true + + } + for key, statuses := range aggregatedStatuses.TLSRoutes { + status := gwapiv1a2.TLSRouteStatus{RouteStatus: aggregateRouteStatus(statuses)} + r.ProviderResources.TLSRouteStatuses.Store(key, &status) + tlsRouteStatusCount++ + delete(keysToDelete.TLSRouteStatus, key) + r.keyCache.TLSRouteStatus[key] = true + } + for key, statuses := range aggregatedStatuses.TCPRoutes { + status := gwapiv1a2.TCPRouteStatus{RouteStatus: aggregateRouteStatus(statuses)} + r.ProviderResources.TCPRouteStatuses.Store(key, &status) + tcpRouteStatusCount++ + delete(keysToDelete.TCPRouteStatus, key) + r.keyCache.TCPRouteStatus[key] = true + } + for key, statuses := range aggregatedStatuses.UDPRoutes { + status := gwapiv1a2.UDPRouteStatus{RouteStatus: aggregateRouteStatus(statuses)} + r.ProviderResources.UDPRouteStatuses.Store(key, &status) + udpRouteStatusCount++ + delete(keysToDelete.UDPRouteStatus, key) + r.keyCache.UDPRouteStatus[key] = true + } + for key, statuses := range aggregatedStatuses.BackendTLSPolicies { + status := aggregatePolicyStatus(statuses) + r.ProviderResources.BackendTLSPolicyStatuses.Store(key, &status) + backendTLSPolicyStatusCount++ + delete(keysToDelete.BackendTLSPolicyStatus, key) + r.keyCache.BackendTLSPolicyStatus[key] = true + } + for key, statuses := range aggregatedStatuses.ClientTrafficPolicies { + status := aggregatePolicyStatus(statuses) + r.ProviderResources.ClientTrafficPolicyStatuses.Store(key, &status) + clientTrafficPolicyStatusCount++ + delete(keysToDelete.ClientTrafficPolicyStatus, key) + r.keyCache.ClientTrafficPolicyStatus[key] = true + } + for key, statuses := range aggregatedStatuses.BackendTrafficPolicies { + status := aggregatePolicyStatus(statuses) + r.ProviderResources.BackendTrafficPolicyStatuses.Store(key, &status) + backendTrafficPolicyStatusCount++ + delete(keysToDelete.BackendTrafficPolicyStatus, key) + r.keyCache.BackendTrafficPolicyStatus[key] = true + } + for key, statuses := range aggregatedStatuses.SecurityPolicies { + status := aggregatePolicyStatus(statuses) + r.ProviderResources.SecurityPolicyStatuses.Store(key, &status) + securityPolicyStatusCount++ + delete(keysToDelete.SecurityPolicyStatus, key) + r.keyCache.SecurityPolicyStatus[key] = true + } + for key, statuses := range aggregatedStatuses.EnvoyExtensionPolicies { + status := aggregatePolicyStatus(statuses) + r.ProviderResources.EnvoyExtensionPolicyStatuses.Store(key, &status) + envoyExtensionPolicyStatusCount++ + delete(keysToDelete.EnvoyExtensionPolicyStatus, key) + r.keyCache.EnvoyExtensionPolicyStatus[key] = true + } + for key, statuses := range aggregatedStatuses.ExtensionServerPolicies { + status := aggregatePolicyStatus(statuses) + r.ProviderResources.ExtensionPolicyStatuses.Store(key, &status) + extensionServerPolicyStatusCount++ + delete(keysToDelete.ExtensionServerPolicyStatus, key) + r.keyCache.ExtensionServerPolicyStatus[key] = true + } + for key, status := range aggregatedStatuses.Backends { + if len(status.Conditions) > 0 { + r.ProviderResources.BackendStatuses.Store(key, status) + backendStatusCount++ + } + delete(keysToDelete.BackendStatus, key) + r.keyCache.BackendStatus[key] = true + } // Publish aggregated metrics message.PublishMetric(message.Metadata{Runner: r.Name(), Message: message.InfraIRMessageName}, infraIRCount) message.PublishMetric(message.Metadata{Runner: r.Name(), Message: message.XDSIRMessageName}, xdsIRCount) @@ -412,16 +481,6 @@ func (r *Runner) loadTLSConfig(ctx context.Context) (*tls.Config, []byte, error) } } -func unstructuredToPolicyStatus(policyStatus map[string]any) gwapiv1.PolicyStatus { - var ret gwapiv1.PolicyStatus - // No need to check the json marshal/unmarshal error, the policyStatus was - // created via a typed object so the marshalling/unmarshalling will always - // work - d, _ := json.Marshal(policyStatus) - _ = json.Unmarshal(d, &ret) - return ret -} - // deleteAllIRKeys deletes all XdsIR and InfraIR using tracked keys func (r *Runner) deleteAllKeys() { // Delete IR keys From bbb99f478069db338ac7654768c9490bbe644c63 Mon Sep 17 00:00:00 2001 From: y-rabie Date: Fri, 5 Dec 2025 03:34:34 +0200 Subject: [PATCH 2/3] fix: merge xRoute/xPolicy statuses across controllers in provider runner Signed-off-by: y-rabie --- internal/provider/kubernetes/status.go | 86 +++++++---- internal/provider/kubernetes/status_test.go | 155 ++++++-------------- 2 files changed, 108 insertions(+), 133 deletions(-) diff --git a/internal/provider/kubernetes/status.go b/internal/provider/kubernetes/status.go index b3aec665f7c..7117c0d7420 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 }), @@ -579,10 +592,11 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context errChan <- err panic(err) } - tCopy := t.DeepCopy() valCopy := val.DeepCopy() setLastTransitionTimeInConditionsForPolicyStatus(valCopy, metav1.Now()) - tCopy.Object["status"] = *valCopy + tCopy := t.DeepCopy() + oldStatus := gatewayapi.ExtServerPolicyStatusAsPolicyStatus(tCopy) + tCopy.Object["status"] = mergeStatus(t.GetNamespace(), r.envoyGateway.Gateway.ControllerName, oldStatus.Ancestors, valCopy.Ancestors) return tCopy }), }) @@ -593,31 +607,29 @@ 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 { - // Allocating with worst-case capacity to avoid reallocation. - merged := make([]gwapiv1.RouteParentStatus, 0, len(old)+len(new)) +// 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 the length of old ancestors. This will only cause + // reallocation on the very first status update when we add our ancestors. + merged := make([]K, 0, len(old)) // 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 +638,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 +651,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) } }) } From ae16e7d764d30d18cbbcf99a08486fee01cae32e Mon Sep 17 00:00:00 2001 From: y-rabie Date: Fri, 5 Dec 2025 03:34:54 +0200 Subject: [PATCH 3/3] Add e2e tests for xRoute/xPolicy status cleanup Signed-off-by: y-rabie --- ...anup-multiple-ancestors-multiple-gwcs.yaml | 14 ++ ...s-cleanup-multiple-ancestors-same-gwc.yaml | 13 ++ .../policy-status-cleanup-no-ancestor.yaml | 10 ++ ...policy-status-cleanup-single-ancestor.yaml | 10 ++ ...leanup-multiple-parents-multiple-gwcs.yaml | 31 ++++ ...tus-cleanup-multiple-parents-same-gwc.yaml | 45 +++++ .../route-status-cleanup-single-parent.yaml | 13 ++ .../status-cleanup-gateway-different-gwc.yaml | 34 ++++ test/e2e/tests/policy_status_cleanup.go | 169 ++++++++++++++++++ test/e2e/tests/route_status_cleanup.go | 105 +++++++++++ test/e2e/tests/stat_name.go | 4 +- test/e2e/tests/tcproute.go | 72 ++++---- .../tests/tcproute_authorization_client_ip.go | 6 +- test/e2e/tests/tcproute_with_backend.go | 6 +- test/e2e/tests/udproute.go | 8 +- test/e2e/tests/utils.go | 55 ++++++ 16 files changed, 551 insertions(+), 44 deletions(-) create mode 100644 test/e2e/testdata/policy-status-cleanup-multiple-ancestors-multiple-gwcs.yaml create mode 100644 test/e2e/testdata/policy-status-cleanup-multiple-ancestors-same-gwc.yaml create mode 100644 test/e2e/testdata/policy-status-cleanup-no-ancestor.yaml create mode 100644 test/e2e/testdata/policy-status-cleanup-single-ancestor.yaml create mode 100644 test/e2e/testdata/route-status-cleanup-multiple-parents-multiple-gwcs.yaml create mode 100644 test/e2e/testdata/route-status-cleanup-multiple-parents-same-gwc.yaml create mode 100644 test/e2e/testdata/route-status-cleanup-single-parent.yaml create mode 100644 test/e2e/testdata/status-cleanup-gateway-different-gwc.yaml create mode 100644 test/e2e/tests/policy_status_cleanup.go create mode 100644 test/e2e/tests/route_status_cleanup.go 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,