feat(policy-engine): reuse unchanged policy chains on xDS update#2741
feat(policy-engine): reuse unchanged policy chains on xDS update#2741renuka-fernando wants to merge 2 commits into
Conversation
xDS is State-of-the-World, so every artifact change pushes the full snapshot and HandlePolicyChainUpdate rebuilt every route's chain, re-running each policy's GetPolicy factory. Redeploying one API thus re-instantiated every other API's policies, churning pools/state. Reconcile each snapshot against the last-applied set: - Hash each route's behavioral config (canonical JSON, order-sensitive, excluding volatile Metadata fields) and reuse the existing chain pointer when unchanged; rebuild only changed/new routes; drop removed. - Fail-safe to rebuild on signature errors; add per-route decision debug logs and per-snapshot reused/rebuilt/new/removed counts. - Add signature + reconciliation tests incl. a completeness guard.
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe xDS client now computes stable route signatures, reuses unchanged policy chains across snapshots, rebuilds changed or new routes, removes absent routes, and applies the reconciled set atomically. Tests cover reuse, rebuild triggers, removals, signature sensitivity, and signature-field completeness. ChangesRoute policy-chain reconciliation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ADS
participant HandlePolicyChainUpdate
participant routeSignature
participant buildPolicyChain
participant Kernel
ADS->>HandlePolicyChainUpdate: provide policy-chain snapshot
HandlePolicyChainUpdate->>routeSignature: compute route signature
alt unchanged route
routeSignature-->>HandlePolicyChainUpdate: reuse applied PolicyChain
else changed or new route
routeSignature-->>HandlePolicyChainUpdate: return changed signature
HandlePolicyChainUpdate->>buildPolicyChain: build policy chain
end
HandlePolicyChainUpdate->>Kernel: ApplyWholeRoutesAndSensitiveValues
Kernel-->>HandlePolicyChainUpdate: apply result
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gateway/gateway-runtime/policy-engine/internal/xdsclient/handler.go`:
- Around line 241-248: Update the reconciliation logic around the removed-route
loop and the related handling near the second reported location to track route
keys present in the snapshot separately from successfully rebuilt entries in
chains. Use the snapshot-key set when determining removals, so routes that fail
validation or rebuilding are not logged or counted as REMOVED, and add a test
covering a failed rebuild.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 83c881c7-01cb-4c1c-86fd-e13143a7c20e
📒 Files selected for processing (3)
gateway/gateway-runtime/policy-engine/internal/xdsclient/handler.gogateway/gateway-runtime/policy-engine/internal/xdsclient/reconcile_test.gogateway/gateway-runtime/policy-engine/internal/xdsclient/signature.go
A route present in the snapshot but dropped due to a validation or rebuild failure was logged and counted as REMOVED, because the check tested membership in the successfully-built chains. Track the route keys seen in the snapshot separately and base removal detection on that set, so present-but-rejected routes (already reported by their own warning/error log) are not double-reported. Add a failed-rebuild test asserting the dropped route is not logged or counted as REMOVED. Addresses CodeRabbit review on wso2#2741.
1 participant
Purpose
The policy engine rebuilt every route's policy chain on every xDS update. Because xDS is State-of-the-World, any artifact change (deploy/redeploy/delete of any REST API, LLM proxy, etc.) pushes the full snapshot, and
HandlePolicyChainUpdateunconditionally callsbuildPolicyChain→registry.GetInstance→ each policy'sGetPolicyfactory for every policy on every route. So redeploying one API re-instantiated the policy instances of all unrelated APIs, discarding any per-instance state (token caches, connection pools, rate-limit windows) and churning resources. This is what forces stateful policies such as backend-jwt to work around the behavior with a process-wide singleton cache.Resolves #2740
Goals
Reconcile each xDS snapshot against what is already applied and rebuild only the routes whose configuration actually changed, so a redeploy of one API no longer re-runs other APIs'
GetPolicyfactories. State-of-the-World remains unchanged on the wire; the reconciliation is entirely policy-engine side.Approach
lastAppliedmap (signature + built chain) on the singletonResourceHandler; no lock needed sinceHandlePolicyChainUpdateruns serially on the single ADS goroutine.RouteKey, the whole orderedPolicieslist, andAPIId/APIName/APIVersion. VolatileMetadatafields (CreatedAt/UpdatedAt/ResourceVersion) andContext(not read bybuildPolicyChain) are deliberately excluded so an unchanged route does not look changed every push.GetPolicy), rebuild only changed/new routes, and drop routes absent from the snapshot. The whole route map is still applied atomically viaApplyWholeRoutesAndSensitiveValues.lastApplied, so an invalid chain can never be reused.REUSED/REBUILT/NEW/REMOVED) and per-snapshotreused/rebuilt/new/removedcounts. Cross-reference comments keepbuildPolicyChainand the signature projection in sync.User stories
N/A
Documentation
N/A — internal policy-engine behavior change with no user-facing configuration or API surface change.
Automation tests
Security checks
Samples
N/A
Related PRs
N/A
Test environment
Go 1.26.5 on macOS (darwin/arm64). Unit tests via
go test ./internal/xdsclient/...;go build ./...andgo vet ./...clean for the policy-engine module.