-
Notifications
You must be signed in to change notification settings - Fork 502
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
Remove RouteTLSConfig requirement for gateway TLS passthrough. #745
Conversation
TLS passthrough never terminates TLS sessions, so it doesn't make sense for TLS certificates to be required in this configuration. Signed-off-by: James Peach <jpeach@apache.org>
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jpeach 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 |
@@ -147,9 +147,6 @@ type RouteTLSConfig struct { | |||
// handshake for requests that match the hostname of the associated HTTPRoute. | |||
// The referenced object MUST reside in the same namespace as HTTPRoute. | |||
// | |||
// This field is required when the TLS configuration mode of the associated | |||
// Gateway listener is set to "Passthrough". |
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.
Would this be accurate if "Passthrough" was replaced with "Terminate"?
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.
So certificateRef
is a required field (syntactically), and IIUC you would need to set the RouteTLSConfig
if the listener protocol is HTTPS
, the mode is Terminate
, you don't want the listener to use the default certificate and there is a TLS route override policy of Allow
.
Phew! 😂
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.
Yeah, that's a good point. This comment never really made sense, and a replacement one is not particularly straightforward or valuable, I'm fine with just removing this.
Thanks! /lgtm |
TLS passthrough never terminates TLS sessions, so it doesn't make
sense for TLS certificates to be required in this configuration.
/kind bug