-
Notifications
You must be signed in to change notification settings - Fork 565
api: Backend TLS SNI #7014
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
api: Backend TLS SNI #7014
Conversation
Signed-off-by: Guy Daich <[email protected]>
Signed-off-by: Guy Daich <[email protected]>
Signed-off-by: Guy Daich <[email protected]>
Signed-off-by: Guy Daich <[email protected]>
Signed-off-by: Guy Daich <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7014 +/- ##
==========================================
- Coverage 71.10% 71.08% -0.02%
==========================================
Files 228 228
Lines 40640 40676 +36
==========================================
+ Hits 28897 28916 +19
- Misses 10052 10065 +13
- Partials 1691 1695 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Guy Daich <[email protected]>
Signed-off-by: Guy Daich <[email protected]>
Signed-off-by: Guy Daich <[email protected]>
Signed-off-by: Guy Daich <[email protected]>
Signed-off-by: Guy Daich <[email protected]>
Signed-off-by: Guy Daich <[email protected]>
api/v1alpha1/backend_types.go
Outdated
// SANValidation specifies how the server certificate SANs are validated. | ||
// | ||
// +optional | ||
SANValidation *SANValidation `json:"sanValidation,omitempty"` |
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 we reuse upstream definition here ?
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.
Sorry, I actually removed this option for now. Auto SAN SNI validation is inferred for situations where auto SNI is used.
In the future, we can expand the backend API to also include static SAN validation, achieving full parity with BTLSP, but it's out of scope for this PR.
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.
sgtm
Signed-off-by: Guy Daich <[email protected]>
Signed-off-by: Guy Daich <[email protected]>
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 move the as part of make docs
to reduce the conflict when there's multiple PR change API?
Signed-off-by: Guy Daich <[email protected]>
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.
LGTM thanks
What type of PR is this?
What this PR does / why we need it:
Currently, BackendTLSPolicy requires SNI and SAN validations to be configured explicitly using static values. In some cases, users may need to use the (dynamic) downstream host header value as upstream SNI and validate upstream certificate SANs accordingly.
Additionally, some users today avoid using BTLSP and only use Backend TLS settings, since it offers more flexibility wrt. skipping TLS validation, etc. At this time, there is no option to specify the SNI value in Backend TLS, which might be needed for some of these users.
To support these use cases, while still adhering to GW-API definitions, the Backend TLS settings are now extended with an explicit SNI attribute, with the following rules:
Summary of BTLSP/Backend combinations below:
Which issue(s) this PR fixes:
Related #4610, #6901
Release Notes: Yes/No