fix: bound policy ancestors on the fly to avoid O(N^2) translation latency (#9558)

* fix: bound policy ancestors on the fly to avoid O(N^2) translation latency

When many resources reference the same target, a policy's status.Ancestors list
could grow without bound during translation. SetConditionForPolicyAncestor scans
the existing ancestors on every call, so building a status with N ancestors was
O(N^2); for a BackendTLSPolicy shared by e.g. 10k EnvoyExtensionPolicies this
pushed translation time to ~1h.

Cap status.Ancestors at maxPolicyAncestors+1 (17) as entries are added:
SetConditionForPolicyAncestor evicts the lowest-priority ancestor once the list
exceeds the soft cap, keeping every insert O(1) and the total O(N). The single
slot above the CRD limit lets the post-processing TruncatePolicyAncestors still
detect overflow, cut to 16, and stamp the Aggregated condition.

Note: because eviction happens as ancestors are added, before an ancestor's rank
is necessarily final, a policy with more than 16 ancestors that later marks a
truncated ancestor Overridden shows only the Overridden condition on it - the
redundant Accepted condition is dropped. This affects only truncated statuses.

Fixes #9539

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>

* update release note

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>

---------

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>

Author: zhaohuabing

internal/gatewayapi/status/policy.go

@@ -27,6 +27,21 @@ const (
 	PolicyReasonMultipleWarnings gwapiv1.PolicyConditionReason = "Warnings"
 )
 
+const (
+	// maxPolicyAncestors is the Gateway API CRD limit on the number of entries
+	// allowed in status.ancestors.
+	maxPolicyAncestors = 16
+
+	// policyAncestorsSoftCap bounds policyStatus.Ancestors as entries are added, so
+	// SetConditionForPolicyAncestor stays O(1) instead of O(N) at scale (see #9539). It is
+	// intentionally one slot above the CRD limit: keeping a single extra ancestor lets the
+	// post-processing TruncatePolicyAncestors still observe len > maxPolicyAncestors, sort, cut to
+	// maxPolicyAncestors, and stamp the Aggregated condition. Since the best maxPolicyAncestors
+	// ancestors are a subset of the best policyAncestorsSoftCap, the final survivors match the
+	// unbounded behavior for policies whose conditions do not change rank across passes.
+	policyAncestorsSoftCap = maxPolicyAncestors + 1
+)
+
 type PolicyResolveError struct {
 	Reason  gwapiv1.PolicyConditionReason
 	Message string
@@ -166,6 +181,32 @@ func SetConditionForPolicyAncestor(policyStatus *gwapiv1.PolicyStatus, ancestorR
 		ControllerName: gwapiv1a2.GatewayController(controllerName),
 		Conditions:     []metav1.Condition{cond},
 	})
+
+	// Keep the ancestor list bounded as entries are added so this stays O(1) even when a policy is
+	// referenced by a very large number of ancestors (see #9539).
+	boundPolicyAncestors(policyStatus)
+}
+
+// boundPolicyAncestors keeps policyStatus.Ancestors within policyAncestorsSoftCap entries by
+// evicting the single lowest-priority ancestor, using the same ordering as TruncatePolicyAncestors.
+// It is called as each ancestor is added so status building stays O(1) per insert. Because eviction
+// happens before an ancestor's rank is necessarily final, a truncated ancestor that later becomes
+// higher-priority (e.g. Overridden) is re-added carrying only that later condition; the redundant
+// Accepted condition is lost for policies with more than maxPolicyAncestors ancestors. The one slot
+// above the CRD limit lets the post-processing TruncatePolicyAncestors still stamp Aggregated.
+func boundPolicyAncestors(policyStatus *gwapiv1.PolicyStatus) {
+	if len(policyStatus.Ancestors) <= policyAncestorsSoftCap {
+		return
+	}
+
+	// Find the lowest-priority ancestor: the one no other ancestor ranks below.
+	worst := 0
+	for i := 1; i < len(policyStatus.Ancestors); i++ {
+		if lessPolicyAncestor(&policyStatus.Ancestors[worst], &policyStatus.Ancestors[i]) {
+			worst = i
+		}
+	}
+	policyStatus.Ancestors = append(policyStatus.Ancestors[:worst], policyStatus.Ancestors[worst+1:]...)
 }
 
 // buildDeprecationWarningMessage builds a warning message from a map of deprecated fields to their alternatives.
@@ -243,42 +284,22 @@ func ancestorRefsEqual(a, b *gwapiv1.ParentReference) bool {
 // The first 15 ancestors are shown as is.
 // The last 16th ancestor is shown as is and add Aggregated condition.
 func TruncatePolicyAncestors(policyStatus *gwapiv1.PolicyStatus, controllerName string, generation int64) {
-	if len(policyStatus.Ancestors) <= 16 {
+	if len(policyStatus.Ancestors) <= maxPolicyAncestors {
 		return
 	}
 
-	// we need to truncate policy ancestor status due to the item limit (max 16).
-	// so we are choosing to preserve the 16 most important ancestors.
+	// we need to truncate policy ancestor status due to the item limit (max maxPolicyAncestors).
+	// so we are choosing to preserve the maxPolicyAncestors most important ancestors.
 	// negative polarity (Conflicted, Overridden...) should be clearly indicated to the user.
 	sort.Slice(policyStatus.Ancestors, func(i, j int) bool {
-		a, b := policyStatus.Ancestors[i], policyStatus.Ancestors[j]
-		aRank := sortRankForPolicyAncestor(&a)
-		bRank := sortRankForPolicyAncestor(&b)
-
-		if aRank != bRank {
-			return aRank < bRank
-		}
-		// First compare by namespace, then by name
-		aNamespace := ""
-		if a.AncestorRef.Namespace != nil {
-			aNamespace = string(*a.AncestorRef.Namespace)
-		}
-		bNamespace := ""
-		if b.AncestorRef.Namespace != nil {
-			bNamespace = string(*b.AncestorRef.Namespace)
-		}
-
-		if aNamespace != bNamespace {
-			return aNamespace < bNamespace
-		}
-		return string(a.AncestorRef.Name) < string(b.AncestorRef.Name)
+		return lessPolicyAncestor(&policyStatus.Ancestors[i], &policyStatus.Ancestors[j])
 	})
 
 	aggregatedMessage := "Ancestors have been truncated because the number of policy ancestors exceeds 16."
 
-	policyStatus.Ancestors = policyStatus.Ancestors[:16]
+	policyStatus.Ancestors = policyStatus.Ancestors[:maxPolicyAncestors]
 	SetConditionForPolicyAncestor(policyStatus,
-		&policyStatus.Ancestors[15].AncestorRef,
+		&policyStatus.Ancestors[maxPolicyAncestors-1].AncestorRef,
 		controllerName,
 		egv1a1.PolicyConditionAggregated,
 		metav1.ConditionTrue,
@@ -288,6 +309,31 @@ func TruncatePolicyAncestors(policyStatus *gwapiv1.PolicyStatus, controllerName
 	)
 }
 
+// lessPolicyAncestor reports whether ancestor a should be ordered before ancestor b when deciding
+// which ancestors to preserve under the CRD limit: by sort rank (negative polarity first), then
+// namespace, then name.
+func lessPolicyAncestor(a, b *gwapiv1.PolicyAncestorStatus) bool {
+	aRank := sortRankForPolicyAncestor(a)
+	bRank := sortRankForPolicyAncestor(b)
+	if aRank != bRank {
+		return aRank < bRank
+	}
+	// First compare by namespace, then by name
+	aNamespace := ""
+	if a.AncestorRef.Namespace != nil {
+		aNamespace = string(*a.AncestorRef.Namespace)
+	}
+	bNamespace := ""
+	if b.AncestorRef.Namespace != nil {
+		bNamespace = string(*b.AncestorRef.Namespace)
+	}
+
+	if aNamespace != bNamespace {
+		return aNamespace < bNamespace
+	}
+	return string(a.AncestorRef.Name) < string(b.AncestorRef.Name)
+}
+
 // sortRankForPolicyAncestor returns an integer sort key.
 // ranking rules (smaller value -> higher priority):
 //

internal/gatewayapi/status/policy_test.go

@@ -6,6 +6,7 @@
 package status
 
 import (
+	"fmt"
 	"strings"
 	"testing"
 
@@ -16,6 +17,55 @@ import (
 	egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
 )
 
+// TestSetConditionForPolicyAncestorBounded verifies the ancestor list is capped on the fly as
+// entries are added (see #9539) and that the highest-priority (not-accepted) ancestors are kept.
+func TestSetConditionForPolicyAncestorBounded(t *testing.T) {
+	const controller = "example.com/controller"
+	policyStatus := &gwapiv1.PolicyStatus{}
+
+	// A few not-accepted (rank 0) ancestors, then many accepted (rank 2) ones.
+	badNames := []string{"bad-0", "bad-1", "bad-2"}
+	for _, name := range badNames {
+		ref := &gwapiv1.ParentReference{Name: gwapiv1.ObjectName(name)}
+		SetConditionForPolicyAncestor(policyStatus, ref, controller,
+			gwapiv1.PolicyConditionAccepted, metav1.ConditionFalse, gwapiv1.PolicyReasonConflicted, "conflicted", 1)
+	}
+	for i := range 100 {
+		ref := &gwapiv1.ParentReference{Name: gwapiv1.ObjectName(fmt.Sprintf("gw-%03d", i))}
+		SetConditionForPolicyAncestor(policyStatus, ref, controller,
+			gwapiv1.PolicyConditionAccepted, metav1.ConditionTrue, gwapiv1.PolicyReasonAccepted, "accepted", 1)
+	}
+
+	// Bounded on the fly at the soft cap, never growing to 103.
+	assert.Len(t, policyStatus.Ancestors, policyAncestorsSoftCap)
+	// The not-accepted (rank 0) ancestors are preserved; only rank-2 entries are evicted.
+	for _, name := range badNames {
+		found := false
+		for _, a := range policyStatus.Ancestors {
+			if string(a.AncestorRef.Name) == name {
+				found = true
+			}
+		}
+		assert.True(t, found, "not-accepted ancestor %q must be preserved", name)
+	}
+
+	// Final truncation still cuts to the CRD limit.
+	TruncatePolicyAncestors(policyStatus, controller, 1)
+	assert.Len(t, policyStatus.Ancestors, maxPolicyAncestors)
+}
+
+func BenchmarkSetConditionForPolicyAncestorsAtScale(b *testing.B) {
+	const controller = "example.com/controller"
+	for b.Loop() {
+		policyStatus := &gwapiv1.PolicyStatus{}
+		for i := range 10000 {
+			ref := &gwapiv1.ParentReference{Name: gwapiv1.ObjectName(fmt.Sprintf("gw-%05d", i))}
+			SetConditionForPolicyAncestor(policyStatus, ref, controller,
+				gwapiv1.PolicyConditionAccepted, metav1.ConditionTrue, gwapiv1.PolicyReasonAccepted, "accepted", 1)
+		}
+	}
+}
+
 func TestSetConditionForPolicyAncestorsTruncatesMessages(t *testing.T) {
 	longMsg := strings.Repeat("x", conditionMessageMaxLength+5)
 	policyStatus := &gwapiv1.PolicyStatus{}

internal/gatewayapi/testdata/backendtrafficpolicy-status-conditions-truncated.out.yaml

@@ -20,11 +20,6 @@ backendTrafficPolicies:
         name: gateway-9
         namespace: envoy-gateway
       conditions:
-      - lastTransitionTime: null
-        message: Policy has been accepted.
-        reason: Accepted
-        status: "True"
-        type: Accepted
       - lastTransitionTime: null
         message: 'This policy is being overridden by other backendTrafficPolicies
           for these routes: [envoy-gateway/httproute-1 envoy-gateway/httproute-2]'

internal/gatewayapi/testdata/envoyextensionpolicy-status-conditions-truncated.out.yaml

@@ -20,11 +20,6 @@ envoyExtensionPolicies:
         name: gateway-9
         namespace: envoy-gateway
       conditions:
-      - lastTransitionTime: null
-        message: Policy has been accepted.
-        reason: Accepted
-        status: "True"
-        type: Accepted
       - lastTransitionTime: null
         message: 'This policy is being overridden by other envoyExtensionPolicies
           for these routes: [envoy-gateway/httproute-1 envoy-gateway/httproute-2]'

internal/gatewayapi/testdata/securitypolicy-status-conditions-truncated.out.yaml

@@ -1729,11 +1729,6 @@ securityPolicies:
         name: gateway-9
         namespace: envoy-gateway
       conditions:
-      - lastTransitionTime: null
-        message: Policy has been accepted.
-        reason: Accepted
-        status: "True"
-        type: Accepted
       - lastTransitionTime: null
         message: 'This policy is being overridden by other securityPolicies for these
           routes: [envoy-gateway/httproute-1 envoy-gateway/httproute-2]'

release-notes/current/performance_improvements/9558-bound-policy-ancestors.md

@@ -0,0 +1 @@
+Fixed translation latency growing quadratically when many policies share the same target by capping each policy's `status.ancestors` at the CRD limit as ancestors are added, instead of only truncating during post-processing.