atenet: add agentgateway egress support - #909
Conversation
|
looking at this today! |
Lior Lieberman (LiorLieberman)
left a comment
Lior Lieberman (LiorLieberman)
left a comment
There was a problem hiding this comment.
LGTM overall. left a few comments.
| ) | ||
|
|
||
| const ( | ||
| // ClientCertificateAttribute is the PEM peer certificate agentgateway |
There was a problem hiding this comment.
is this only relevant for agentgateway?
If yes, maybe you call that agentgatewayClientCertificateAttribute?
There was a problem hiding this comment.
also is this dynamic metadata? just curios how you compare that vs the forwarded Client Cert Header ?
There was a problem hiding this comment.
Yes, this attribute is agentgateway-specific. I renamed it to agentgatewayClientCertificateAttribute and made it unexported. It is an ext_proc request attribute, not dynamic metadata: agentgateway evaluates source.certificate from the verified downstream TLS connection into ProcessingRequest.attributes. Envoy continues to use the sanitized XFCC header. The handler independently re-verifies the certificate in both cases.
There was a problem hiding this comment.
I cant see the rename, did you commit it?
|
|
||
| const ( | ||
| // DirectionAttribute is set from a dataplane expression, not a client | ||
| // header, by dataplanes without Envoy filter chains. |
There was a problem hiding this comment.
by dataplanes without Envoy filter chains.
who sets this request attribute, agw? Also I think we probably want some more specific namespace like ate.calllout(maybe a better key name?).direction
There was a problem hiding this comment.
Yes, agentgateway sets this through the extProc requestAttributes config. I renamed the key to the more specific ate.extproc.direction.
There was a problem hiding this comment.
I cant see the rename, did you commit it?
| echo " --delete-all Delete core system and all registered demos" | ||
| echo " --ateapi-client-auth=cert|token Select how in-cluster clients authenticate to ateapi for --deploy-ate-system (default: cert; the server always accepts both)" | ||
| echo " --atenet-router=envoy|agentgateway Select the atenet router dataplane (default: envoy)" | ||
| echo " --atenet-router=envoy|agentgateway Select the ingress and egress dataplane (default: envoy)" |
There was a problem hiding this comment.
hmmm dont we want separate flags for that..? atenet-egress|atenet-ingress?
There was a problem hiding this comment.
I would prefer to keep one flag initially. We do not currently have a use case for mixing Envoy ingress with agentgateway egress or vice versa, and separate flags add configuration combinations we would need to support and test. We can split it later if that need appears.
|
ok, lgtm. But lets wait until #715 is merged since it touches a few parts that your are touching here |
|
(will get merged today, pending on a few last touchups from keith) |
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io> # Conflicts: # cmd/atenet/internal/router/egress/egress_test.go # cmd/atenet/internal/router/extproc/metadata.go # cmd/atenet/internal/router/ingress/ingress_test.go # manifests/ate-install/components/agentgateway/configmap.yaml
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io> # Conflicts: # hack/install-ate.sh
2 participants
Summary
Add agentgateway support to the actor egress gateway.
--atenet-router=agentgatewaynow selectsagentgateway for both ingress and egress, while Envoy remains the default.
What changed
ext_prochandlerIP:portsource.certificatetoext_proc.