atenet: add option to run agentgateway - #652
Conversation
|
John - this would require #559 to be merged first, wouldnt it? |
No, there is no dependency and it works end to end currently. With 559 we will need a couple line change to make it send to the atunnel instead of directly but either one can merge first |
I was actually wondering if we could do the same for Envoy as well, the dynamic config seems like more complexity than it's worth right now |
Eitan Yarmush (EItanya)
left a comment
There was a problem hiding this comment.
Just a couple of small naming things on my end, overall LGTM
Agentgateway is a popular [AAIF](https://aaif.io/) project designed for agentic systems. It is of particular interest for substrate users as it was designed specifically to serve the role of an egress proxy for agents and other AI-adjacent workloads, with strong support for credential injection, token exchanges, TLS MITM, and other authentication and authorization schemes. Agentgateway supports the ext_proc protocol, like Envoy, so we use that here. There has been some debate in the community around the long term end state of ext_proc vs other options, but for now this maintains the status quo. While Agentgateway can be configured dynamically over XDS, it does not take the same configuration as Envoy. However, none of the config is dynamic anyways, so we use just a static configuration file (configmap) for now to keep things simple. While we don't have specific per-component/file OWNERS in the project at the moment, I can informally commit to myself and Eitan maintaining this integration. Given the goals around velocity in the project we can commit to either rapidly fixing any issues that may arise or removing/disabling the integration if this ends up slowing things down.
e62c12c to
b22e47b
Compare
Rebased on 559 |
Lior Lieberman (LiorLieberman)
left a comment
There was a problem hiding this comment.
overall looks good to me, lots of switch statements/if statements throughout. Is there a way we can make it more self-contained? that most dataplanes logic happens within specific packages?
| if c.atenetRouter() == atenetRouterAgentgateway { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
add a comment why this is not relevant for agw and/or what it does instead?
| name: "agentgateway is valid", | ||
| cfg: routerConfig{AtenetRouter: string(atenetRouterAgentgateway), ParkedRequest: ParkedRequestConfig{Max: defaultParkedRequestMax}}, |
There was a problem hiding this comment.
add a testcase for atenetRouterEnvoy? (both as explicit, and both as a default). Maybe add a atenetRouter field in the test case or this could be better tested on a different func.
nit: change the name of the test to atenet-router set to agentgateway is valid, atenet-router set to envoy is valid.
| mut.SetHeaders = append(mut.SetHeaders, &corev3.HeaderValueOption{ | ||
| AppendAction: corev3.HeaderValueOption_OVERWRITE_IF_EXISTS_OR_ADD, | ||
| Header: &corev3.HeaderValue{ | ||
| Key: ":authority", |
There was a problem hiding this comment.
nit: we have a const for authority likely?
|
|
||
| // Do not expose the router-only routing header to actor code. Restore Host | ||
| // so dataplanes that route dynamically on worker IP still give the actor its | ||
| // stable mesh hostname. |
There was a problem hiding this comment.
mesh?
There was a problem hiding this comment.
why embedding this under a gke specific director?
Agentgateway is a popular AAIF project designed for agentic systems. It is of particular interest for substrate users as it was designed specifically to serve the role of an egress proxy for agents and other AI-adjacent workloads, with strong support for credential injection, token exchanges, TLS MITM, and other authentication and authorization schemes.
Agentgateway supports the ext_proc protocol, like Envoy, so we use that here. There has been some debate in the community around the long term end state of ext_proc vs other options, but for now this maintains the status quo.
While Agentgateway can be configured dynamically over XDS, it does not take the same configuration as Envoy. However, none of the config is dynamic anyways, so we use just a static configuration file (configmap) for now to keep things simple.
While we don't have specific per-component/file OWNERS in the project at the moment, I can informally commit to myself and Eitan maintaining this integration. Given the goals around velocity in the project we can commit to either rapidly fixing any issues that may arise or removing/disabling the integration if this ends up slowing things down.