-
Notifications
You must be signed in to change notification settings - Fork 473
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
initial TLSRoute support #10648
initial TLSRoute support #10648
Conversation
…rtomontt/tls-route
…ertomontt/tls-route
…ertomontt/tls-route
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.
initial comments. over all looks good. need more tests - would be good to have an e2e test as well to prove the code.
var hostnames []string | ||
if l == nil || hr == nil { | ||
if l == nil { |
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 we check len(hr) == 0
here? (not sure, asking because there was a conditional before)
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.
no, because empty list is treated as "select all" . So I maintained that behavior. Later we check :
if len(routeHostnames) == 0 {
return true, []string{listenerHostname}
}
routeInfos []*query.RouteInfo, | ||
reporter reports.ListenerReporter, | ||
) { | ||
var validRouteInfos []*query.RouteInfo |
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.
can you add a yaml tests for that in https://github.com/kgateway-dev/kgateway/blob/main/internal/kgateway/translator/gateway/gateway_translator_test.go ?
can this be unit-tested as well?
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.
done
…ertomontt/tls-route
…ertomontt/tls-route
…ertomontt/tls-route
…ertomontt/tls-route
…ertomontt/tls-route
…ertomontt/tls-route
Description
Crossport of solo-io#10601
Adds initial support for TLSRoutes. TLSRoutes are similar to TCPRoutes, but the distinction between them is that TLSRoutes allows routing decisions by SNI. See docs.
This PR adds support for TLS Passthrough on a TLS listener using a TLSRoute.
API changes
Code changes
CI changes
Docs changes
Context
Interesting decisions
Testing steps
Notes for reviewers
Checklist: