Support insecure TLS and only CA cert for Elasticsearch #1918
Conversation
Signed-off-by: Pavol Loffay <ploffay@redhat.com>
pavolloffay
requested review from
black-adder,
jpkrohling,
objectiser,
tiffon,
vprithvi and
yurishkuro
as code owners
| @@ -301,15 +301,16 @@ func (c *Configuration) getConfigOptions(logger *zap.Logger) ([]elastic.ClientOp | |||
| } | |||
| } else { | |||
| httpTransport := &http.Transport{ | |||
| Proxy: http.ProxyFromEnvironment, | |||
| Proxy: http.ProxyFromEnvironment, | |||
| TLSClientConfig: &tls.Config{InsecureSkipVerify: c.TLS.SkipHostVerify}, | |||
There was a problem hiding this comment.
when no TLS is involved at all, would non-nill TLSClientConfig cause issues?
There was a problem hiding this comment.
It should not godoc from http.Transport.TLSClientConfig
// TLSClientConfig specifies the TLS configuration to use with
// tls.Client.
// If nil, the default configuration is used.
// If non-nil, HTTP/2 support may not be enabled by default.
There was a problem hiding this comment.
this godoc doesn't seem to clarify what happens to tls / no-tls. Perhaps it's just driven off the http scheme
| session = requests.Session() | ||
| if skipHostVerify: | ||
| session.verify = False | ||
| if ca is not None: |
There was a problem hiding this comment.
nit: should this be elif?
There was a problem hiding this comment.
good point I think they are mutually exclusive.
There was a problem hiding this comment.
I have changed it a bit. if CA is provided it will use that in the precedence of skip verify. I want to avoid accidentally using insecure connections.
Signed-off-by: Pavol Loffay <ploffay@redhat.com>
Codecov Report
@@ Coverage Diff @@
## master #1918 +/- ##
==========================================
- Coverage 98.48% 98.45% -0.03%
==========================================
Files 198 198
Lines 9740 9740
==========================================
- Hits 9592 9590 -2
- Misses 113 114 +1
- Partials 35 36 +1
Continue to review full report at Codecov.
|
|
Merging without fuzzing to pass. This PR does not modify anything fuzzer tests. |
pavolloffay
changed the title
Related to jaegertracing/jaeger-operator#471 (comment)