-
Notifications
You must be signed in to change notification settings - Fork 635
use Gateway in ancestor ref on AgentgatewayPolicy #13281
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: npolshakova <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request fixes the ancestor reference reporting in AgentgatewayPolicy status to properly resolve to Gateway resources instead of the direct target (e.g., HTTPRoute). When a policy is attached to an HTTPRoute, the status now reports the Gateway(s) that the route references as ancestors, following Gateway API best practices for lower cardinality status reporting.
Changes:
- Refactored
ResolvedTargetstruct to useAncestorRefs(slice) instead ofGatewayTarget(single reference) - Added new
resolvePolicyAncestorRefsfunction to resolve policy targets to their parent Gateway references - Updated status reporting loop to handle multiple ancestor references per policy target
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/agentgateway/plugins/traffic_plugin.go | Implemented ancestor resolution logic and refactored status reporting to properly reference Gateway resources |
| pkg/agentgateway/plugins/testdata/trafficpolicy/http-route-ancestor-gateway.yaml | Added test case validating that HTTPRoute-targeted policies report Gateway as ancestor |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if len(refs) == 0 { | ||
| return fallback | ||
| } | ||
| slices.SortStableFunc(refs, func(a, b gwv1.ParentReference) int { | ||
| return strings.Compare(reports.ParentString(a), reports.ParentString(b)) | ||
| }) | ||
| return refs | ||
| } |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When filtering parent references to only include Gateway parents, the code continues iterating if a non-Gateway parent is found. However, if none of the parent references are Gateways (e.g., all are custom resources), the function returns the fallback which reports the HTTPRoute as the ancestor. This could lead to inconsistent reporting. Consider documenting this behavior or adding a comment explaining why falling back to the route itself is the intended behavior when no Gateway parents are found.
Signed-off-by: npolshakova <[email protected]>
Signed-off-by: npolshakova <[email protected]> Revert Signed-off-by: npolshakova <[email protected]>
4ac3b32 to
e6257a7
Compare
| if target.SectionName != nil { | ||
| parentRef.SectionName = target.SectionName | ||
| } | ||
| // TODO: add support for XListenerSet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this todo is still valid I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened #13296 to track
Signed-off-by: npolshakova <[email protected]>
Signed-off-by: npolshakova <[email protected]>
Signed-off-by: npolshakova <[email protected]>
Description
Fixes #12696
Change Type
Changelog
Additional Notes
Example from extauthz tests: