Fix connection to servers with self-signed certificates or unknown certificate authority #414
Conversation
This file contains 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
…rtificate authority The metrics module fails to connect to the Elasticsearch metrics store, if secure transport is used and the server has a self signed certificate or a certificate signed by a certificate authority that's not in the certificate store provided by `certifi`. + Introduce a non-mandatory setting that allows turning off certificate verification. + Introduce a non-mandatroy setting to override the ca_certs certificate store. Fixes elastic#413
danielmitterdorfer
requested changes
Feb 13, 2018
There was a problem hiding this comment.
Many thanks for the PR @kesslerm! I left two suggestions for the property names. I think would also be good to document these two properties towards the end of the section https://github.com/elastic/rally/blob/master/docs/configuration.rst#advanced-configuration.
esrally/metrics.py
Outdated
| @@ -130,6 +130,11 @@ def __init__(self, cfg): | |||
| secure = self._config.opts("reporting", "datastore.secure") == "True" | |||
| user = self._config.opts("reporting", "datastore.user") | |||
| password = self._config.opts("reporting", "datastore.password") | |||
| # poor man's boolean conversion | |||
| verify_certs = self._config.opts("reporting", "datastore.verify_certs", default_value="True", mandatory=False) == "True" | |||
There was a problem hiding this comment.
I'd rather use datastore.certs.verify as name (in the config file we usually use dots as a separator).
esrally/metrics.py
Outdated
| @@ -130,6 +130,11 @@ def __init__(self, cfg): | |||
| secure = self._config.opts("reporting", "datastore.secure") == "True" | |||
| user = self._config.opts("reporting", "datastore.user") | |||
| password = self._config.opts("reporting", "datastore.password") | |||
| # poor man's boolean conversion | |||
| verify_certs = self._config.opts("reporting", "datastore.verify_certs", default_value="True", mandatory=False) == "True" | |||
| ca_certs = self._config.opts("reporting", "datastore.ca_certs", default_value=None, mandatory=False) | |||
There was a problem hiding this comment.
I'd rather use datastore.certs.path as a name.
danielmitterdorfer
added
enhancement
docs/configuration.rst
Outdated
| @@ -124,7 +124,7 @@ Rally will ask you a few more things in the advanced setup: | |||
| * **Elasticsearch project directory**: This is the directory where the Elasticsearch sources are located. If you don't actively develop on Elasticsearch you can just leave the default but if you want to benchmark local changes you should point Rally to your project directory. Note that Rally will run builds with Gradle in this directory (it runs ``gradle clean`` and ``gradle :distribution:tar:assemble``). | |||
| * **JDK root directory**: Rally will only ask this if it could not autodetect the JDK home by itself. Just enter the root directory of the JDK you want to use. By default, Rally will choose Java 8 if available and fallback to Java 9. | |||
| * **Metrics store type**: You can choose between ``in-memory`` which requires no additional setup or ``elasticsearch`` which requires that you start a dedicated Elasticsearch instance to store metrics but gives you much more flexibility to analyse results. | |||
| * **Metrics store settings** (only for metrics store type ``elasticsearch``): Provide the connection details to the Elasticsearch metrics store. This should be an instance that you use just for Rally but it can be a rather small one. A single node cluster with default setting should do it. | |||
| * **Metrics store settings** (only for metrics store type ``elasticsearch``): Provide the connection details to the Elasticsearch metrics store. This should be an instance that you use just for Rally but it can be a rather small one. A single node cluster with default setting should do it. When using self-signed certificates on the Elasticsearch metrics store, certificate verification can be turned off by setting the `datastore.ssl.verification_mode` setting to `none`. Alternatively you can enter the path to the certificate authority's signing certificate in `datastore.ssl.certificate_authorities`. Both settings are optional. | |||
There was a problem hiding this comment.
One minor thing: Contrary to Markdown, reStructuredText requires two backticks for literal text. So e.g. instead of:
`datastore.ssl.verification_mode`
you need to write
``datastore.ssl.verification_mode``
danielmitterdorfer
approved these changes
Feb 19, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The metrics module fails to connect to the Elasticsearch metrics store,
if secure transport is used and the server has a self signed certificate
or a certificate signed by a certificate authority that's not in the
certificate store provided by
certifi.Fixes #413