Conversation
Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>
| // The first rule matching an incoming request is used. | ||
| // An ordered list of route rules for HTTP traffic. HTTP routes will be | ||
| // applied to platform service ports named http-*/http2-*/grpc-*, gateway | ||
| // ports with protocol HTTP/HTTP2/GRPC/HTTPS and service entry ports |
There was a problem hiding this comment.
Note conflict with HTTPS here and mention below.
Suggest differentiating as "TLS terminated HTTPS" vs "unterminated TLS"
| // An ordered list of route rules for TCP traffic. | ||
| // The first rule matching an incoming request is used. | ||
| repeated TCPRoute tcp = 4; | ||
| // An ordered list of route rules for TCP+TLS or HTTPS traffic. Routing |
There was a problem hiding this comment.
Alternate phrasing suggestion
"An ordered list of route rule for non-terminated TLS & HTTPS traffic..."
| repeated TCPRoute tcp = 4; | ||
| // An ordered list of route rules for TCP+TLS or HTTPS traffic. Routing | ||
| // is typically performed using the SNI value presented by the | ||
| // ClientHello message. TLS routes will be applied to platform service |
There was a problem hiding this comment.
I wonder if we should just call the protocol tls instead of TCP_TLS. I think it reads better and doesn't necessarily imply the underlying protocol. Also the tunneling protocol should be listed first so for instance we can have 'tls-mongo' as a port label and have it work with tls-* matching rules
"TLS routes can be applies to service ports labelled tls-* & https-*, ...
| // name: bookinfo-sni | ||
| // spec: | ||
| // hosts: | ||
| // - reviews.prod.svc.cluster.local |
There was a problem hiding this comment.
SNI host should be in agreement with outer host right ??
The more useful example would be outer host is wildcard and SNI host is a sub-domain for routing
| // port: | ||
| // number: 443 | ||
| // ``` | ||
| message TLSRoute { |
There was a problem hiding this comment.
@PiotrSikora should we include ALPN in here at some point ?
| // SNI (server name indicator) to match on. Wildcard prefixes can be used | ||
| // in the SNI value. E.g., *.com will match foo.example.com as well as | ||
| // example.com. | ||
| string sni = 3; |
There was a problem hiding this comment.
repeated ? Also sni_hosts to be consistent with terms.
| // a.b.c.d/xx form or just a.b.c.d. This is only valid when the | ||
| // destination service has several IPs and the application explicitly | ||
| // specifies a particular IP. | ||
| string destination_subnet = 1; |
There was a problem hiding this comment.
Do we think this will be needed ? I guess eventually it might but I suspect we can leave it out for the moment. Same for port as the primary use case is Gateway binding.
There was a problem hiding this comment.
This is very useful and I am waiting for this to land in the filter chain match. I could do things like filter based on the service VIP subnet range if need be.
There was a problem hiding this comment.
I guess it does no harm and we can see if it gains traction.
Aside - We need a refined way to mark individual API features as alpha/beta
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rshriram, vadimeisenbergibm The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| string sni = 3; | ||
| repeated string sni_hosts = 3; | ||
|
|
||
| // ALPN protocols to match on. Multiple ALPN protocols can be specified |
There was a problem hiding this comment.
lets leave this for a follow on PR. Needs some thought
This reverts commit 3f90a76. This PR was merged prematurely. Needs more work.
Treating SNI routing as an explicit act rather than implicitly enabling it
by port names.
Signed-off-by: Shriram Rajagopalan shriramr@vmware.com