Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support ElasticSearch client certificate authentication #21

Merged
merged 5 commits into from
May 15, 2017

Conversation

jrummler
Copy link
Contributor

If ElasticSearch is protected by Shield/SearchGuard the exporter must do the client certificate authentication. This enables configuring this via PEM files containing the certificates / client cert / client private key.

Example call would be:

elasticsearch_exporter --es.uri=elastic:9200 --es.ca=/path/to/pem/with/ca/cert.pem --es.client-private-key=/path/to/pem/with/client/private/key.pem --es.client-cert=/path/to/pem/with/client/cert.pem

@deepthawtz
Copy link

👍 looks good. don't see much momentum on this project anymore which is unfortunate

@metalmatze
Copy link
Collaborator

Hey. We've taken over the project and I would like to ask you to take a look at this PR once again.
If you're still happy with the code we can merge this after another review! 😊

@jrummler
Copy link
Contributor Author

Great news! I am still fine with it! Please get in touch if you need further support! ;-)

@dominikschulz
Copy link
Contributor

@jrummler Could you please rebase your PR? The changes itself look good, I'd like to merge them.

@@ -429,3 +435,47 @@ func main() {
})
log.Fatal(http.ListenAndServe(*listenAddress, nil))
}

func createElasticSearchTlsConfig(pemFile, pemCertFile, pemPrivateKeyFile string) (*tls.Config) {
if (len(pemFile) > 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one can be inverted to if (len(pemFile) <= ) { return nil } so that we can get rid of the extra else at the bottom of this func. I think that applies for a few other conditions in this func too. It might be nitpicking, but definitely something I'd do different.

@jrummler
Copy link
Contributor Author

Guys, I have merged the changes. Since I do not use this tool anymore, I had no possibility to test the functionality.

Copy link
Contributor

@dominikschulz dominikschulz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dominikschulz dominikschulz merged commit 47349ce into prometheus-community:master May 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants