-
Notifications
You must be signed in to change notification settings - Fork 79
Add system-internal-tls-allow-serve-mode configuration option #1093
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
base: main
Are you sure you want to change the base?
Conversation
|
Skipping CI for Draft Pull Request. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1093 +/- ##
==========================================
+ Coverage 93.14% 93.16% +0.02%
==========================================
Files 36 36
Lines 1255 1259 +4
==========================================
+ Hits 1169 1173 +4
Misses 72 72
Partials 14 14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This adds a new configuration field to allow Serve mode when system-internal-tls is enabled. By default, enabling system-internal-tls forces all traffic to use Proxy mode because not all Knative ingress implementations support Serve mode with TLS enabled. The new system-internal-tls-allow-serve-mode option allows users to enable Serve mode when their ingress implementation supports direct TLS connections to application containers. Configuration: - Default: Disabled (forces Proxy mode for compatibility) - Enabled: Allows Serve mode even with TLS enabled
71ee816 to
94883fd
Compare
|
Hey! Thanks for the work you've done. The code is good, so I'm happy to put LGTM when you are ready. Wrt your questions:
I think so, yes. The feature enables a potentially breaking behavior change for users whose ingress implementations don't support Serve mode with TLS, so this should be marked as alpha.
For config options - yes, as release notes are valuable as they help users discover new configuration capabilities. AI suggested this release note:
Imo, it's okay. If we really need something shorter, it can be |
|
Thanks for the review, I updated the PR description and the other related PR. /remove-approve - was added automatically as I'm co-release lead. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/check-cla |
|
Could you please take a look @dprotaso ? |
Hi,
related serving PR: knative/serving#16183
this PR adds a new configuration field to allow Serve mode when system-internal-tls is enabled. By default, enabling system-internal-tls forces all traffic to use Proxy mode because not all Knative ingress implementations support Serve mode with TLS enabled.
The new system-internal-tls-allow-serve-mode option allows users to enable Serve mode when their ingress implementation supports direct TLS connections to application containers.
Configuration:
I added the option to the network config as it makes IMO sense to put it close to the relevant
system-internal-tlsoption, even if the effect is not really related to it.Changes
/kind enhancement
Release Note
Questions:
Docs
Thanks for the feedback!
/cc @Fedosin