fix: default backend TLS max version to 1.3#9412
Conversation
Backend (upstream) TLS connections were silently capped at TLS 1.2. Apply the defaults (min 1.2, max 1.3) in applyBackendTLSSetting when not configured. Fixes envoyproxy#9395 Signed-off-by: vishwas-bm <b_m.vishwas@nokia.com>
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@guydc is this a breaking change? should we fix the document instead？ |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e369633d80
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9412 +/- ##
=======================================
Coverage 75.31% 75.32%
=======================================
Files 252 252
Lines 41434 41438 +4
=======================================
+ Hits 31208 31215 +7
Misses 8111 8111
+ Partials 2115 2112 -3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
vishwas-bm
force-pushed
the
fix/backend-tls-default-max-version-9395
branch
from
e369633 to
d7d8a30
Compare
|
I suspect this was a misunderstanding based on the envoy docs: https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/transport_sockets/tls/v3/common.proto
I don't think there's a scenario where a connection that previously succeeded would now fail so I think bugfix is right vs breaking change. |
3 participants
What this PR does / why we need it:
This bug fix applies the documented defaults (min TLS 1.2, max TLS 1.3) in
applyBackendTLSSettingwhen the versions are not configured.Which issue(s) this PR fixes:
Fixes #9395
PR Checklist
git commit -s). See DCO: Sign your work./api), the API was discussed and agreed before the implementation. The API change can be in a separate PR, or in the same PR, but the API must be agreed before implementation. N/A if this PR does not contain API changes.make generate gen-check,make lint, and the unit-test/coverage build pass. (Flaky e2e failures are not considered breakages, butgen-check,lint, and coverage MUST pass.)release-notes/current/<section>/<pr-number>-<slug>.md(seerelease-notes/current/README.mdfor sections and naming). N/A if this PR does not contain non-trivial changes.make gen-checkand committed the result if API/helm charts/modules changed.release-notes/current/breaking_changes/.