-
Notifications
You must be signed in to change notification settings - Fork 596
agentgateway: add extproc translation in traffic policy plugin #12667
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?
agentgateway: add extproc translation in traffic policy plugin #12667
Conversation
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
Adds support for translating ExtProc (external processing) policies into AgentGateway policies within the traffic policy plugin.
- Introduces extproc policy handling and integrates it into translateTrafficPolicyToAgw.
- Implements processExtProcPolicy to resolve GatewayExtension, build backend reference, and emit an ExtProc policy.
- Minor Makefile improvement for KinD image flag usage.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| pkg/agentgateway/plugins/traffic_plugin.go | Adds extproc policy suffix, integrates extproc processing in translation, and implements processExtProcPolicy. |
| Makefile | Fixes kind create cluster --image usage to use the full image from CLUSTER_NODE_VERSION directly. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if err != nil { | ||
| return nil, err | ||
| } | ||
| extProc := (*gwExt).Spec.ExtProc |
Copilot
AI
Oct 19, 2025
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.
Potential nil dereference: Spec.ExtProc is accessed without a nil check. Add a guard (e.g., if extProc == nil { return nil, fmt.Errorf("gateway extension %s/%s missing extProc spec", …) }) before accessing extProc.GrpcService.
| extProc := (*gwExt).Spec.ExtProc | |
| extProc := (*gwExt).Spec.ExtProc | |
| if extProc == nil { | |
| return nil, fmt.Errorf("gateway extension %s/%s missing extProc spec", trafficPolicy.Namespace, trafficPolicy.Name) | |
| } |
36b6099 to
f96d5f9
Compare
|
The code is ready for review . |
|
|
||
| .PHONY: kind-create | ||
| kind-create: ## Create a KinD cluster | ||
| $(KIND) get clusters | grep $(CLUSTER_NAME) || $(KIND) create cluster --name $(CLUSTER_NAME) --image kindest/node:$(CLUSTER_NODE_VERSION) |
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.
Why this change?
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.
there was a bug introduced where kindset/node was repeated twice.
its solved in the upstream . so i will remove this
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.
removed this
| schema "k8s.io/apimachinery/pkg/runtime/schema" | ||
| v1 "sigs.k8s.io/gateway-api/apis/v1" | ||
|
|
||
| query "github.com/kgateway-dev/kgateway/v2/internal/kgateway/query" |
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.
I think we want to revert this change and keep the kgateway imports in a separate group
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.
done
| } | ||
|
|
||
| func processExtProcPolicy(ctx krt.HandlerContext, gatewayExtensions krt.Collection[*v1alpha1.GatewayExtension], trafficPolicy *v1alpha1.TrafficPolicy, policyName string, policyTarget *api.PolicyTarget) ([]AgwPolicy, error) { | ||
| // TODO: warn if unsupported policy parameters are used? |
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.
Yep, let's make sure we report an error if any unsupported policy fields are set.
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.
added validation for both extproc policy and extproc provider settings
| if backendRef.Namespace != nil { | ||
| namespace = string(*extProc.GrpcService.BackendRef.Namespace) | ||
| } | ||
| port := uint32(80) // default port |
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.
Can we reuse buildServiceHost
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.
used buildAGWServiceRef which in turn calls buildServiceHost
| containers: | ||
| - name: ext-proc-grpc | ||
| # uses https://github.com/TheRealSibasishBehera/ext-proc-examples/tree/ag-support | ||
| image: sibseh/basic-sink:fix-2 |
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.
Is there a reason we can't use the same setup as the envoy test? https://github.com/kgateway-dev/kgateway/blob/main/test/kubernetes/e2e/features/extproc/testdata/setup.yaml#L40
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.
agentgateway only supports streaming body responses
the extproc impl used for envoy tests were not based on that , resulting the connections to hang (the gateway sent the requests but the response body from ext_proc server were considered invalid )
more about agentgateway specific code here
This fork includes
- updated implementation to send streaming responses with the end_of_stream flag set to true.
- updated the Go bindings to align with the latest envoy control plane API, removing the previous solo.io fork (the reason for that fork’s use is unclear to me).
|
https://github.com/agentgateway/agentgateway/releases/tag/v0.10.4 is now out, so we should be able to bump to that and get the ext-proc xds support! 🎉 |
|
Thanks for the update. I will bump the version |
f96d5f9 to
03d65a9
Compare
f2606a5 to
db41934
Compare
| errs = append(errs, fmt.Errorf("statPrefix field in ExtProcProvider is not supported for agentgateway")) | ||
| } | ||
|
|
||
| // TODO: RouteCacheAction has a kubebuilder default so we don't validate it here |
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.
can the default marker be removed and handled in envoy specific code ?
I dont think it would be a breaking change this way and validation can be performed here properly
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.
update:
envoy specific code was already setting the default value for RouteCacheAction to FromResponse
So I had to just remove the kubebuilder marker and add validation here
db41934 to
5cebb15
Compare
|
Hey there are some up coming changes to the API as part of #12723 that will affect this. Let's also move the code used to build the test extproc image into the kgateway repo so we can rebuild it as necessary! This can live under the agentgateway/extproc directory similar to how we have the a2a example. |
5cebb15 to
e405149
Compare
Signed-off-by: Sibasish Behera <[email protected]>
e405149 to
0fd7017
Compare
Signed-off-by: Sibasish Behera <[email protected]>
0fd7017 to
68730ef
Compare
|
Now that #12723 has gone in, this should be unblocked! We'll just want to update the api to be on AgentgatewayPolicy! |
Description
closes #12487
Change Type
/kind new_feature
Changelog
Additional Notes
NONE