feat(api): add insecureSkipVerify flag to Backend TLS settigns#6222
Merged
arkodg merged 1 commit intoenvoyproxy:mainfrom Jun 3, 2025
Merged
feat(api): add insecureSkipVerify flag to Backend TLS settigns#6222arkodg merged 1 commit intoenvoyproxy:mainfrom
arkodg merged 1 commit intoenvoyproxy:mainfrom
Conversation
Signed-off-by: a.krudu <a.krudu@tbank.ru>
AlexKrudu
force-pushed
the
api-tls-disable
branch
from
4f1a7f0 to
6cfdc23
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6222 +/- ##
==========================================
- Coverage 70.46% 70.46% -0.01%
==========================================
Files 220 220
Lines 36593 36607 +14
==========================================
+ Hits 25787 25794 +7
- Misses 9281 9287 +6
- Partials 1525 1526 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
Author
|
/retest |
1 similar comment
Contributor
Author
|
/retest |
arkodg
reviewed
May 29, 2025
| // +kubebuilder:default=false | ||
| // +optional | ||
| // +notImplementedHide | ||
| InsecureSkipVerify *bool `json:"insecureSkipVerify,omitempty"` |
Contributor
There was a problem hiding this comment.
im not a fan of this term, but its an industry wide term
Contributor
There was a problem hiding this comment.
@envoyproxy/gateway-maintainers thoughts on insecureSkipVerify vs skipValidation ?
AlexKrudu
force-pushed
the
api-tls-disable
branch
from
7a1a1a4 to
6cfdc23
Compare
Contributor
Author
|
I messed up - pushed implementation in the same branch and rolled it back |
Contributor
Author
|
/retest |
arkodg
reviewed
Jun 3, 2025
| // must be specified with at least one entry for a valid configuration. Only one of | ||
| // CACertificateRefs or WellKnownCACertificates may be specified, not both. | ||
| // | ||
| // Only used for DynamicResolver backends. |
Contributor
There was a problem hiding this comment.
I dont think will work, we'll need to enable this TLS section for all backend, lets raise a GH issue to track it
arkodg
approved these changes
Jun 3, 2025
zhaohuabing
approved these changes
Jun 3, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
feat(api): add insecureSkipVerify flag to Backend TLS settigns
What this PR does / why we need it:
provides an opportunity for skipping upstream certificate validation
Fixes #4595
Decided to add the flag to
Backend.BackendSpec.TLSinstead ofEnvoyProxySpec.BackendTLSConfigbecause chosen option provides better granularity – there may be cases where the user wants to disable validation selectively (skip validation only for some of Backends, not for all of them). Also, certificate validation settings within Envoy are already at a level equivalent to the Backend (the setting in Envoy is at the Cluster level, and in EG Backend, after transformations, it maps to the Cluster).Release Notes: Yes/No