Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add GEP-3155: Complete Backend mTLS Configuration #3180
Add GEP-3155: Complete Backend mTLS Configuration #3180
Changes from all commits
9f99d93
c19be24
cd2fe15
b5f5112
cf4906d
edbf961
9c29265
42498db
f25dbbc
37590b9
3408b5a
15fdfd9
bc3183e
ba3084b
6d1b13c
a2d458b
912f965
b3dae17
0d84adf
ef5b22b
bb48234
c46bb1e
308c7b0
7426dd8
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Hello @mkosieradzki , if we specify client cert on Gateway then can you please share some insights as to how would the scenarios where there are services which do not need MTLS but need only End2End TLS coexist in the same Gateway -> Route configurations.
There could be a route targeting service (music) which needs MTLS and another route targeting service (shopping) which only needs End to End TLS.
These scenarios would not work on same Gateway if we use MTLS to all the services
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.
I think the distinction here is that we're providing a client cert at the Gateway level that should be presented if it's requested by the backend. @mkosieradzki had previously included a per-Service override for this config in BackendTLSPolicy. We ran into issues there where the personas attached to BackendTLSPolicy weren't particularly clear (see #3226 to chime in on that discussion), so this iteration of the GEP will leave per-Service overrides out, but I think that's still very much a longer term goal.
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.
To my best understanding whether the Gateway will send the certificate to the Backend or not depends on the backend server configuration, i.e. whether it will send
CertificateRequest
message during the handshake as per https://datatracker.ietf.org/doc/html/rfc5246#section-7.3 or not.Therefore if backend is not configured to request a client certificate, the Gateway will not send it, even it is configured. My original rationale for adding per-service overrides was about corner cases:
I think we should revisit this scenario as soon as we figure out the #3226.
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.
This is missing an API example - seems to be suggesting the addition of a new field under the Gateway
spec
stanza https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io%2fv1.Gateway?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.
Yes, I think this should have a quick sketch of what we need to add to Gateway.
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.
I just want to be on record as saying that I didn't like this field when we added it to Gateway, I don't like this field here, but I don't have a good enough reason to block any changes. Let's see if it ends up misused in the wild.
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.
I definitely agree with the hesitation here, but ultimately believe this is more helpful than not. There's such a wide range of TLS config possible that it seems unlikely that we'll ever cover all of the potential concepts in the core API. I think it's likely preferable to mirror the options map we have on Gateway Listeners than to leave out a form of extensibility here, though I'll admit that both options have risks.