Add support for custom CAs for TLS validation in clients #847
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
🌐 Coverage report
|
internal/certs/pool.go
Outdated
| // PoolWithCACertificate returns a new pool that includes the CA certificate | ||
| // in the given path. | ||
| func PoolWithCACertificate(path string) (*x509.CertPool, error) { | ||
| pool := x509.NewCertPool() | ||
| err := addCACertificateToPool(pool, path) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return pool, nil | ||
| } |
There was a problem hiding this comment.
question Currently, it is just being used the function related to the system certs pool. Should this one be kept too? It's true that it could allow us to test just a subset of CAs in case it is needed easily.
| } | ||
|
|
||
| // Address option sets the host to use to connect to Kibana. | ||
| func Address(address string) ClientOption { |
There was a problem hiding this comment.
I suppose that this function will be used in the original PR, right?
There was a problem hiding this comment.
Added for completitude, this is only used on tests by now https://github.com/elastic/elastic-package/pull/847/files/bff27e46db7e789f768740b4a4b18cc7c403793b#diff-9c4e0882a886054c20aa4c09e6c0e667826fd54b97f98e5ec36066eb0798feb8R30
| @@ -0,0 +1,5 @@ | |||
| -----BEGIN EC PRIVATE KEY----- | |||
There was a problem hiding this comment.
Maybe drop a CONTRIBUTING.md with instructions to generate those :)
There was a problem hiding this comment.
Added as comment in the tests.
|
Thanks for the reviews! Comments addressed, ready for another review. |
4 participants
Initially implemented as part of #789, moved to a separate PR for clarity.
Add support for custom CAs for TLS in clients for Elasticsearch and Kibana.
Include helpers to create certificate pools that include a given CA certificate file.
Related to #654.