feat(tls-route): add TLS route matching crate #3192
Conversation
Signed-off-by: Zahari Dichev <[email protected]>
linkerd/tls/route/src/lib.rs
Outdated
| #[derive(Clone, Debug, Hash, PartialEq, Eq)] | ||
| pub struct SessionInfo { | ||
| pub sni: ServerName, | ||
| } |
There was a problem hiding this comment.
This should be moved down below the mods with the other structs. I would probably move it to directly about the find impl, so that it's defined nearest where it is used.
It would also be good to include a comment a brief comment describing that this is intended to capture the routing metadata of a proxied/unterminated server-side TLS connection.
| /// A list of session matchers, *any* of which may apply. | ||
| /// | ||
| /// The "best" match is used when comparing rules. | ||
| pub matches: Vec<r#match::MatchSession>, |
There was a problem hiding this comment.
So it seems that we have both SNI matching at the Route level and also the Rule level. If we're going to include MatchSni at the Route level, then we probably want MatchSession to be empty for now. Otherwise, we probably want to omit the sni matching at the Route level.
I don't have enough information to suggest one or the other. I don't really see any harm in it being defined at the Route level (since this matches the gateway spec). Given that the spec does not include any rule based matches currently, I'm not sure of a good reason to include SNI matches in rules.
| use linkerd_tls::ServerName; | ||
|
|
||
| #[derive(Clone, Debug, Hash, PartialEq, Eq)] | ||
| pub enum MatchSni { |
There was a problem hiding this comment.
It's probably worth documenting SNI matching semantics with some of the Gateway API's verbiage.
Signed-off-by: Zahari Dichev <[email protected]>
There was a problem hiding this comment.
This is looking pretty good to me!
It would probably be good to have some tests cases for sessions that do not match any routes. For example, it would be good to explore the corner cases like a suffix match like *.test.example.com and a session for test.example.com. What other subtleties should we consider?
Signed-off-by: Zahari Dichev <[email protected]>
olix0r
changed the title
|
When merging, please ensure that the commit message includes an accurate summary of the change like that in the PR description. |
This PR includes a nonfunctional change that adds a TLS route-matching library.
The library works similarly to the http-route one but instead of relying on HTTP concepts
such as paths, headers, etc is specific to TLS ones (i.e. SNI).
At the moment the library is capable of matching routes based on SNIs.