-
Notifications
You must be signed in to change notification settings - Fork 621
api: extension post cluster modify hook #5821
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: Guy Daich <guy.daich@sap.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5821 +/- ##
==========================================
- Coverage 65.31% 65.28% -0.03%
==========================================
Files 221 221
Lines 35314 35314
==========================================
- Hits 23064 23056 -8
- Misses 10825 10829 +4
- Partials 1425 1429 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| // PostClusterExtensionContext provides resources introduced by an extension and watched by Envoy Gateway | ||
| // additional context information can be added to this message as more use-cases are discovered | ||
| message PostClusterExtensionContext { | ||
| // Resources introduced by the extension that were used as extensionRefs in an HTTPRoute/GRPCRoute BackendRefs |
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.
Hi! Quinn here, we met last week.
Will one of these ExtensionResources be the gateway CR that is currently used in the xDS translation?
The backrefs by themselves give us the HTTPRoutes but the parentsRefs of those may be all the gateways instead of the current gateway.
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.
To support additional use cases (besides extension server), I was thinking of propagating the GW context in XDS Metadata. That way, the information is also available to dataplane filters and extensions. The extension server can also parse this metadata.
So, first step would be to complete #5523, and then add an opt-in option for including metadata from additional related resources (GW, Route, ... ).
proto/extension/service.proto
Outdated
| // PostClusterModifyResponse is the expected response from an extension and contains a modified version of the Cluster that was sent | ||
| // If an extension returns a nil Cluster then it will not be modified | ||
| message PostClusterModifyResponse { | ||
| envoy.config.route.v3.Route route = 1; |
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.
Should this return a cluster?
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.
yes! my bad.
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, when it's ready. Thank you for your contributions! |
What type of PR is this?
What this PR does / why we need it:
Adds an option for extension server to mutate clusters with context derived from GRPCRoute/HTTPRoute BackendRefs ExtensionRef HTTPRouteFilters.
Since Upstream HTTP Filters are mapped to cluster-level HTTPProtocolOptions in envoy, it makes most sense to aggregate BackendRef ExternalRefs and provide them as context for cluster mutation (as opposed to CLA/Endpoint mutation).
Which issue(s) this PR fixes:
Relates to #5561
Release Notes: Yes/No