-
Notifications
You must be signed in to change notification settings - Fork 4
docs: Begin laying the groundwork for a 'how' section regarding egress gateways. #16
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
…s gateways. Signed-off-by: usize <[email protected]>
…rics tags. Signed-off-by: usize <[email protected]>
|
Welcome @usize! |
proposals/10-egress-gateways.md
Outdated
| # name: egress-client-cert | ||
| extensions: | ||
| - name: inject-credentials | ||
| type: CredentialInjector |
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.
what is this type referring to?
do you assume a predefined catalog of extensions?
can I “bring my own” payload processor?
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.
Good question. We probably want to have a set of built in/standard processors with the ability to have (prefixed) implementation specific ones
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.
type is the processor kind here. I think supporting a small catalog of processors would be great, and correct. Users can define their own BYO Processor -- e.g. foo.bar.com/EntityRedactor:v1 -- by conforming to an extension interface.
The interface for extensions will need something like:
- Phase(s) it operates on e.g.
request-headers - failOpen/Closed (default to closed?)
- priority
- type specific config schema
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.
The idea I was going with here, was to reuse the approach that filters use on HTTPRoute, but without the rule match (since the Backend policy should impact all traffic going to the backend and not depend on routing decisions).
proposals/10-egress-gateways.md
Outdated
| ### Conflict Resolution | ||
| When multiple policies influence the same request: | ||
| - **Specificity precedence**: Route > Backend > Gateway. | ||
| - **Same-level ties**: Implementations MUST use a deterministic tie-break (e.g., lexical name order) and surface status indicating the conflict. |
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 this should be rephrased.
current phrasing leaves the tie breaking question to the specific gateway implementation.
we need to define deterministic order which will apply to all implementations.
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 know in Istio, we pick the oldest policy first to bias towards stability. Is it really a requirement to force all implementations to use the same mechanism? Is that feasible?
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 deterministic order should be defined one way or another, otherwise switching between different Gateways might break expected order.
not talking about the mechanism, only about what is the expected result.
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.
otherwise switching between different Gateways might break expected order.
IMO, switching between Gateway implementations should not be a core design goal of our APIs. In my experience, this is a far more complicated process than most users ever want to take on. I don't feel super strongly, but I am wary about imposing these requirements on implementations w.r.t the tie-breaker
proposals/10-egress-gateways.md
Outdated
|
|
||
| ## Observability Considerations | ||
|
|
||
| - Implementations SHOULD expose metrics tagged by `{gateway, route, backend, namespace, serviceAccount}` and surface conditions (e.g., `Accepted`, `Programmed`, `Degraded`). |
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.
learning from past mistakes - I think we should phrase things we believe in as MUST.
phrasing as SHOULD may or may not be implemented, so it doesn’t say a lot.
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 seems appropriate for metrics IMO, but +1 on a MUST for status
proposals/10-egress-gateways.md
Outdated
|
|
||
| ## Next Steps | ||
|
|
||
| 1. Decide on Gateway resource approach (reuse vs. new EgressGateway type) |
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.
we should reuse as much as we can when possible.
this should probably be stated clearly.
proposals/10-egress-gateways.md
Outdated
| ## Next Steps | ||
|
|
||
| 1. Decide on Gateway resource approach (reuse vs. new EgressGateway type) | ||
| 2. Define Backend resource schema with embedded policy rules |
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 assume embedded policy rules - you mean to state the policy (like rate limit), in the CR?
what about use cases where the payload processing requires custom logic that cannot be expressed declaratively?
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'd felt a bit fuzzy about having a catalog of policies versus embedding some policy directly into the CR. As per our discussion above, I've changed the proposal to suggest maintaining a catalog of policies that can be extended via extensionRef (like filters on HTTPRoute).
proposals/10-egress-gateways.md
Outdated
| 1. Decide on Gateway resource approach (reuse vs. new EgressGateway type) | ||
| 2. Define Backend resource schema with embedded policy rules | ||
| 3. Specify filter extension points for payload processing | ||
| 4. Align with multi-cluster and agentic networking proposals |
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.
completely agree that SIG MC should be involved. having said that, I think it belongs to phase 2.
phase 1 is being able to call an external service while staying within the scope of a single cluster.
proposals/10-egress-gateways.md
Outdated
| extensions: | ||
| - name: inject-credentials | ||
| type: CredentialInjector | ||
| phase: request-headers # request-headers|request-body|connect|backend-request|backend-response|response-body|response-headers |
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 need to flesh out phases a bit more, especially in the security context. For example, some payload processing (most? all?) should probably not be done after the auth step and the request is coming from a trusted peer or rate-limiting is applied (to prevent malicious or untrusted requests). How does an implementation think about that?
proposals/10-egress-gateways.md
Outdated
|
|
||
| Option B implies defining equivalents of parentRefs, listeners, and route attachment; this is a significant fork from Gateway API and should be justified by clear need for an egress specific spec. | ||
|
|
||
| **Recommendation needed**: Feedback requested on whether the semantics justify a new resource or if Gateway reuse is sufficient. |
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'm a +1 on continuing to use Gateway. It's a standard resource that's familiar to users and understood by implementations
proposals/10-egress-gateways.md
Outdated
| hostname: api.openai.com | ||
| port: 443 | ||
| tls: | ||
| mode: SIMPLE # SIMPLE | MUTUAL |
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 borrow from Gateway here for implementation specific TLS modes
proposals/10-egress-gateways.md
Outdated
| port: 443 | ||
| tls: | ||
| mode: SIMPLE # SIMPLE | MUTUAL | ||
| serverName: api.openai.com |
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.
nit: SNI may be more ubiquitous but don't feel strongly
proposals/10-egress-gateways.md
Outdated
| config: | ||
| requestsPerMinute: 1000 | ||
| ``` | ||
| Alternatively, policies MAY be separate CRDs (e.g., `BackendTLSPolicy`, `EgressPolicy`) with `spec.targetRef: Backend`, avoiding schema growth on `Backend`. |
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 definitely see the schema growth issue...does something like Envoy's typed config model help out a bit? Give implementations a way to dynamically instantiate a policy based on well-known type (urls?)?
proposals/10-egress-gateways.md
Outdated
| Client traffic flows through the egress gateway directly to an external endpoint (FQDN or IP). The gateway applies policies and routing logic before forwarding to the destination. | ||
|
|
||
| ### Parent Mode | ||
| Client traffic flows through a local egress gateway to an upstream gateway before reaching the final endpoint. This enables gateway chaining for multi-cluster or multi-zone topologies. The local egress gateway treats the parent as a single upstream. Local retries are limited to establishing the parent connection. Request-level retries are performed by the parent. Implementations MUST prevent retry loops across gateways. |
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.
More details are going to be needed here I think, especially when it comes to communication between the gateway and the client, best practices for ensuring traffic goes through the gateway etc
proposals/10-egress-gateways.md
Outdated
| When multiple policies influence the same request: | ||
| - **Specificity precedence**: Route > Backend > Gateway. | ||
| - **Same-level ties**: Implementations MUST use a deterministic tie-break (e.g., lexical name order) and surface status indicating the conflict. | ||
| - **Same-level ties**: Implementations MUST use a deterministic lexical name order tie-break and surface status indicating the conflict. |
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.
If we're going to be prescriptive on tie-breaker, I think taking the oldest resource is an approach that biases more towards cluster stability. You'll never have a stable system become disrupted by a new, conflicting resource. Granted, I'm biased because Istio does this (less work for me 😄 ), but I figured I'd mention it
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.
Sorry about that. I re-read your original comment thread and saw that I'd missed that. I'm fine with this. Lexical name order was just the first thing I glommed onto.
After thinking about it, I'm leaning toward specifying a tie-break because now is our best opportunity to do it IMO. Trying to do it later, after a few implementations spring up, would be more painful.
Signed-off-by: usize <[email protected]>
Signed-off-by: usize <[email protected]>
…tations. Signed-off-by: usize <[email protected]>
Signed-off-by: usize <[email protected]>
Signed-off-by: usize <[email protected]>
Signed-off-by: usize <[email protected]>
Signed-off-by: usize <[email protected]>
…s a proven record of stability. Signed-off-by: usize <[email protected]>
Signed-off-by: usize <[email protected]>
Signed-off-by: usize <[email protected]>
Signed-off-by: usize <[email protected]>
0646815 to
d99dab1
Compare
shaneutt
left a comment
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.
Sorry for the late review, Kubecon week was last week.
This is a very thoughtful first pass at how we might implement egress. I have a few comments asking for some additional provisions regarding other paths we might take, other considerations we need to work through as we move forward, but once those are accounted for this seems like a great next step.
/approve
proposals/10-egress-gateways.md
Outdated
| > highly focused sections as much as possible to help make things easier to | ||
| > read and review. Long, unbroken walls of code and YAML in this document are | ||
| > not advisable as that may increase the time it takes to review. | ||
| 1. Resource model using Gateway + HTTPRoute with a Backend for destinations (Service or FQDN). |
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 are other ways to implement this (for instance, Linkerd allows for using Gateway, but also attaching to the mesh network as a parentRef to configure different kinds of egress) that we're aware of. I think starting with this is good, but we'll need to leave an open door for other resource models.
| - Introduce dedicated `EgressGateway` resource type | ||
| - Enables egress-specific fields (e.g., global CIDR allow-lists) without policy attachment overhead | ||
| - Clearer separation of ingress vs egress concerns | ||
|
|
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 would add another Alternatives Considered here for the Mesh resource (experimental, currently) as we know that Linkerd at least already does something like this, allowing egress from the sidecars, as opposed to the GW.
| failOpen: true | ||
| config: | ||
| requestsPerMinute: 1000 | ||
| ``` |
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'm inclined to not fuss over this API right now, rather: it would be nice if we could put together a working prototype to test with and inform our decisions about how this API should work.
proposals/10-egress-gateways.md
Outdated
|
|
||
| Additional processors may be defined. They MUST declare the following fields: | ||
|
|
||
| - phase: one of {request-headers, request-body, connect, backend-request, backend-response, response-headers, response-body} |
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.
👍
| - phase: one of {request-headers, request-body, connect, backend-request, backend-response, response-headers, response-body} | ||
| - priority: integer. (Lower runs first within the same phase). | ||
| - failOpen: boolean. Default false (closed). | ||
| - preAuth: boolean. Default false. (trusted-peer context unavailable before authorization) |
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.
Good callout. More to do to figure out how this is going to work ofc, but very important callout.
| extensions: | ||
| - name: pii-detector | ||
| type: acme.example.com/PIIDetector:v1 | ||
| phase: request-body | ||
| priority: 20 | ||
| failOpen: false | ||
| preAuth: true | ||
| config: | ||
| modelRef: pii-detect-small | ||
| redactionStyle: delete | ||
| confidenceThreshold: 0.7 | ||
| maxBodyBytes: 2097152 |
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'm not certain yet whether we want to add these extensions/filters/policies at the Backend level, as it seems that a pii-detector could be applied in an egress, ingress, or mesh context (in theory), so it seems more natural that these would apply to the HTTPRoute? 🤔
I think we can play with this, but before we merge let's add a comment pointing out the alternatives that we need to consider still.
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 didn't put a great deal of effort into the example use case. More-so focusing on the way it would look to add a filter. I'd like to brainstorm some good Backend specific cases.
proposals/10-egress-gateways.md
Outdated
| 1. Define Backend resource schema. | ||
| 2. Specify default Backend policies e.g. CredentialInjector and QoSController. | ||
| 3. Specify filter extension points for payload processing | ||
| 4. Align with multi-cluster and agentic networking proposals |
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.
Depending on who's feeling froggy, I could see us saying we want to prototype this a bit as a next step.
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've proposed setting aside time for myself to do just that as a part of planning for ${MY_JOB}. 😁
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: shaneutt, usize The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
'Filter' is a more standard term, which matches the proposed approach. Co-authored-by: Shane Utt <[email protected]> Signed-off-by: usize <[email protected]>
…arifications Signed-off-by: usize <[email protected]>
18c0800 to
2a8cebc
Compare
| Controllers MUST publish the set of supported processor kinds and versions for a GatewayClass via `GatewayClass.status.parametersRef` or an implementation-specific status e.g. `GatewayClass.status.supportedExtensionKinds`. | ||
|
|
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.
what if the list is VERY long?
e.g., do you still expect to see it in status if there are 1k supported processors?
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.
Based on wg meeting:
-
Maybe we should reference an external source of truth instead of trying to keep the whole catalog in a status.
-
Users can also configure a pointer to their own external catalogs in this way.
-
but -
-
External link can also add complexity for users via indirection.
-
Maybe status could link to a ConfigMap (probably easy to prototype and likely durable)
|
|
||
| Additional processors may be defined. They MUST declare the following fields: | ||
|
|
||
| - phase: one of {request-headers, request-body, connect, backend-request, backend-response, response-headers, response-body} |
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.
does it make sense to add more details on these phases?
what does each phase means? when does it happen?
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.
wg meeting notes:
- for early stage design / proof of concept, maybe we should be less strict about phases.
- we should gather feedback from Gateway API and potential users before defining it.
Co-authored-by: Nir Rozenbaum <[email protected]>
Co-authored-by: Nir Rozenbaum <[email protected]>
| ### Parent Mode | ||
| Client traffic flows through a local egress gateway to an upstream gateway before reaching the final endpoint. This enables gateway chaining for multi-cluster or multi-zone topologies. The local egress gateway treats the parent as a single upstream. Local retries are limited to establishing the parent connection. Request-level retries are performed by the parent. | ||
|
|
||
| Operators MUST use network policy or sidecar/egress proxy configuration to deny direct egress from workloads and force all outbound traffic to the 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.
Does this mean application pods need to be egress gateway-aware? (in lieu of a service mesh/sidecar approach?)
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.
It does not. The expectation is that without a service mesh operators will configure NetworkPolicy or CNI routing to route traffic through the gateway without workloads needing to be aware of it.
| - phase: one of {request-headers, request-body, connect, backend-request, backend-response, response-headers, response-body} | ||
| - priority: integer. (Lower runs first within the same phase). | ||
| - failOpen: boolean. Default false (closed). | ||
| - preAuth: boolean. Default false. (trusted-peer context unavailable before authorization) |
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.
| name: acme-piidetector-v1-schema | ||
| namespace: gateway-system | ||
| data: | ||
| schema.json: | |
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.
We need to make it clear that we can support complicated dynamic use cases: e.g. pattern matching beyond what can be describe by a regular expression.
DamianSawicki
left a comment
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 PR describes egress gateway as a reverse proxy with static Backends. Another approach is a dynamic forward proxy, and we see a huge demand for it. Since the filename and the Why? section are general, I'd suggest splitting How? into subsections corresponding to different models, with the proposals from the present PR fitting into something like Reverse Proxy.
| fqdn: | ||
| hostname: api.openai.com | ||
| port: 443 | ||
| tls: |
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 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'm open to using BackendTLSPolicy here.
I avoided it in the initial proposal because I’m not yet sure what it looks like to apply BackendTLSPolicy where backends are primarily intended to be external FQDNs rather than in-cluster Services.
That is, as of today, to use BackendTLSPolicy for an external FQDN we'd need to represent the FQDN via the creation of a synthetic Service. Since we're proposing the addition of a new Backend type, it seemed useful to try to avoid re-introducing that synthetic Service pattern.
The semantics may line up with minimal changes, but we need to investigate further.
I’ll add a note that we may want to reuse or align with BackendTLSPolicy once the shape of the egress Backend resource stabilizes.
nirrozenbaum
left a comment
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.
@usize @shaneutt
as I was telling in the last call, I have mixed feelings around this PR.
TLDR - This PR is mixing both egress gateway and payload processing.
more details:
In our WG we currently identified two main use cases that are of interest.
- egress gateway - ideally we would like to be able to use HttpRoute (or alike) to route traffic to a service outside of the cluster.
- Payload Processors as part of our request flow, ideally also using some Gateway level objects.
This PR is mixing both, which makes the proposal much more complex and also not very easy to implement.
The ideal is to have both to work using one intuitive solution, but for initial PoC and start to collect feedback I think we should separate concerns.
That is - we should try to implement (or in this PR, to specify the How) only for egress use case, without getting into Payload Processing. then we should try to implement a PoC (quick a dirty) and go with that to Gateway API meetings to collect feedback.
each of the use cases is big by itself and deserves a separate discussion.
@nirrozenbaum looking at the comments here, yes, I agree. I've tried to put a bit too much into this PR. Payload processing already has its own proposal. Some of this might fit better there, or in some other proposal that bridges payload processing and egress as a singular focus. How about this. I'm going to trim it down and simplify in the direction you're suggesting (removing details around payload processing). I'll keep the feedback I've gotten so far, and start toward building a simple PoC. That way we can generate a little forward momentum here without risking needing to backtrack all of it later. |
SGTM! |
This PR introduces the initial “How” section for the Egress Gateways proposal.
It defines a clear model for routing outbound traffic through Kubernetes Gateways and describes how policy and extension points can be applied for both general egress and AI-specific use cases.
Importantly, it opens up discussion around important questions regarding: