IngressRoute v1 design document #1195
Conversation
Signed-off-by: Steve Sloka <slokas@vmware.com>
|
|
||
| Here are some changes that are directly different from the v1beta1 design: | ||
|
|
||
| - `services` under the `match` struct are renamed to `backends` |
There was a problem hiding this comment.
Looks like you may need to s/services/backends/g - some of the sample IngressRoutes below are still using services:.
Signed-off-by: Steve Sloka <slokas@vmware.com>
There was a problem hiding this comment.
@stevesloka this is a fantastic design document, thanks!!
Just putting my two cents:
- Route matching is not needed in tcp proxying and the current design seems to require it.
- Is it possible to add a note to explicitly say that all the maching rules in route are additive (AND)?
| @@ -98,7 +98,7 @@ Here are some changes that are directly different from the v1beta1 design: | |||
|
|
|||
| - `services` under the `match` struct are renamed to `backends` | |||
| - `services` struct previously had a `name` parameter, now it has a set of options which define the protocol to implement for that specific backend | |||
| - `match` previously took a string arguement which was the pathPrefix to route on, now this has been moved to a `path` parameter under the `match` struct | |||
| - `match` previously took a string argument which was the pathPrefix to route on, now this has been moved to a `path` parameter under the `match` struct | |||
| - `tcpproxy`: Is removed and replaced by a `supported protoco` defined in the route | |||
There was a problem hiding this comment.
typo in supported protoco and is it possible to put an example to further clarify a little bit?
|
|
||
| - **http**: l7 insecure http proxy | ||
| - **https**: l7 secure (tls) http proxy | ||
| - **http2**: l7 insecure http2 proxy |
There was a problem hiding this comment.
Usually http2 is referred as h2c.
| - **http**: l7 insecure http proxy | ||
| - **https**: l7 secure (tls) http proxy | ||
| - **http2**: l7 insecure http2 proxy | ||
| - **https2**: l7 secure http2 proxy |
There was a problem hiding this comment.
Usually http2s is referred as h2.
There was a problem hiding this comment.
Thank you for working on this. I'm sorry it has taken me several days to reply, there is a lot to digest here.
I would like you to break this design into smaller pieces. As I see it there are 3-4 major pieces of work here
- The renaming of ingressroute fields; service -> backends, and name -> protocol hint. This is independant of any routing or delegation discussion. I think we can do this in a backwards compatible way in beta1 by introducing a new
backendsfield with the protocol named fields and deprecating service. When we move to ingressroute v1, we'll remove service. - The Issues with the current design section is great to have all those things together as we want to make sure they are addressed before v1, but perhaps some of them can be addressed cheaply without being rolled into this design. For example Permit Insecure in multi-team environments #864 could be addressed by a
permitInsecure: falseflag in thecontour serveconfig we're going to do in 0.14 contour serve should take its configuration from a configuration file #1130. Maybe its possible to chip away at those outstanding issues independantly of this design. - Header based routing. I'd like to step back a bit and define, before talking about the implementation, the things Ingressroute v1 will let users route on. Headers is well covered in this document, but what about client ip? What about HTTP method? What about path globbing, suffix routing, substring matching, query string matching, and so on. I'm wary of designing for the things we know how to do well without having a holistic plan for other kinds of routing we might be asked for.
- Healthcheck failure policy. Similar to 1. I think these can be separated out and treated independently. I wasn't aware that these were a problem and hopefully we can address them cheaply in ingressroute beta1 similar to point 1.
Routing design vs ingressroute design
This proposal operates at many levels; talking about the ingressroute limitations and bugs, a fleshed out section on header routing, but is noncommittal on other kinds of path based routing other than our current prefix matching. Routing is going to be a huge topic by itself, I'd really like to see it pulled out into its own proposal -- even if that means that this proposal, or any discussion of ingressroute v1 is put on hold.
Routing is the key deliverable, I see little value in delivering ingressroute v1, even incorporating the other sub areas outlined above, unless we have a better routing story so let's concentrate on that. Key to this is to define a list of the types of things that we want contour 1.0 users to be able to route on. Let's start there, without being drwaing to closely into the how, but stay at the level of saying "we want to let you route like this so you can do X". This is as much a process of saying what we will allow users to route on as it is one of excluding things we will not permit routing on. As an example, I do not think we can support regex path matching, I don't see how that can interoperate with delegation, but I think we can support glob style, /foo/*/bar matching.
My suggestion is to start a new document for just the routing component of ingressroute; just the match key, and working only at the high level, declare the things we will let users route on.
As for the other areas in this design such as backend name protocol hints, and healthcheck policies, we can tear these off and work on them in parallel.
|
|
||
| The scope of this design doc looks to improve on the IngressRoute.v1beta1 design and plan how the current design will change to support these additional routing features, yet still preserve the current multi-team capabilities with delegation. | ||
|
|
||
| ## Header Routing |
There was a problem hiding this comment.
I think this block about header based routing and use cases are good, but should be moved later in the document.
| - **<u>Overall</u>**: | ||
| - Deprecate Contour's support for v1beta1.Ingress. | ||
| - Support Ingress objects outside the current cluster. | ||
| - IP in IP or GRE tunneling between clusters with discontinuous or overlapping IP ranges. |
There was a problem hiding this comment.
This feels like a weirdly specific thing to add to this list to specifically say we're not going to do it. How would this read if we just deleted this line?
| - match: | ||
| path: /foo | ||
| - header: x-header | ||
| value: a |
There was a problem hiding this comment.
Is this an exact match, or a substring match?
|
|
||
| Currently with IngressRoute, path prefixes of a request can be delegated to teams, users, namespaces, within a single Kubernetes cluster. Additionally, this design looks to add headers as a mechanism to pass authority off to teams, users or namespaces. | ||
|
|
||
| The following example shows how requests to a specific path with a specific header can be delegated to a team in another namespace as well as how requests that don't match the headers specified previously will be handled in the current namespace. |
There was a problem hiding this comment.
I'd like to see a discussion on what can be delegated on; will it be only headers and path prefixes?
| IngressRoutev1 will support routing from various headers. It will implement commonly used headers such as user-agent & cookie since those will require a regex style implementation. By implementing them as speific types, we can apply those regex's automatically. | ||
|
|
||
| - **header:** This is a generic header type and allows for matching custom headers | ||
| - **cookie**: This is a built-in type and matches a `cookie` in the header while being implemented as a "contains" regex |
There was a problem hiding this comment.
Cookies are just headers with a fixed value. Can we simplify cookie routing into a header called cookie and substring matching for key=value?
There was a problem hiding this comment.
We could, I was just looking for ways to simplify the IngressRoute document. We're adding more cases and tests in Contour, but the user has a better UX. We could also start here and design the other later.
There was a problem hiding this comment.
Cookie and user-agent feel like helpers. If we give people flexible enough ways to match on headers generically, they can implement Cookie and User-agent themselves. I think this would be enough for IngressRoute 1 and we can add cookie and user-agent, and perhaps others, when needed, post contour 1.0.
👍
| - header: x-header | ||
| value: a | ||
| delegate: | ||
| name: headera-ir |
There was a problem hiding this comment.
What would the child ingressroute look like? Would it have the same match: stanza as the parent following the example we have in it beta1 where every path in the child ingressroute must share the same prefix as the parent?
|
|
||
| - **header:** This is a generic header type and allows for matching custom headers | ||
| - **cookie**: This is a built-in type and matches a `cookie` in the header while being implemented as a "contains" regex | ||
| - **user-agent**: This is a built-in type and matches the `user-agent` in the header while being implemented as a "contains" regex |
There was a problem hiding this comment.
User agent is the same as cookie, its another header, and people will absolutely have to use regex here because of the trash fire that is the user agent header.
Cookie and user-agent feel like helpers. If we give people flexible enough ways to match on headers generically, they can implement Cookie and User-agent themselves. I think this would be enough for IngressRoute 1 and we can add cookie and user-agent, and perhaps others, when needed, post contour 1.0.
|
I'm going to close this PR for now. I was a bit overzealous in what this design could tackle. I won't delete the branch just yet and will 100% take the feedback given into the next rev, but I think to leave this as-is will do more harm and be confusing for folks. |
Fixes #947, updates #1075 by providing an initial design for IngressRoute v1.
Signed-off-by: Steve Sloka slokas@vmware.com