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 custom CA truststore and client SSL keypair. #255

Merged
merged 1 commit into from
Jun 28, 2018
Merged

Support custom CA truststore and client SSL keypair. #255

merged 1 commit into from
Jun 28, 2018

Conversation

jaloren
Copy link
Contributor

@jaloren jaloren commented Jan 7, 2018

Per discussion in #226, there is no way to specify a custom CA that the
mysqld exporter will trust when establishing a SSL connection to a mysql
server. So if a mysql server has a custom truststore, an operator would
need to set DSN, where tls=skip-verify.

With this change, a user can define the ssl options in the mysql cnf and
then the mysqld exporter will construct a DSN and a custom TLS config
based on those options.

Per discussion in #226, there is no way to specify a custom CA that the
mysqld exporter will trust when establishing a SSL connection to a mysql
server. So if a mysql server has a custom truststore, an operator would
need to set DSN, where tls=skip-verify.

With this change, a user can define the ssl options in the mysql cnf and
then the mysqld exporter will construct a DSN and a custom TLS config
based on those options.
@SuperQ
Copy link
Member

SuperQ commented Jan 13, 2018

I would prefer these to be flags, rather than in the config file. By utilizing flags, it's still possible to control normal connection settings via the DATA_SOURCE_NAME env var.

@brian-brazil
Copy link
Contributor

I believe these are standard my.cnf settings.

@SuperQ
Copy link
Member

SuperQ commented Jan 13, 2018

I would like to make it possible to keep the ENV + flags option, rather than force users to use a config file. (It makes it easier for docker users)

@jaloren
Copy link
Contributor Author

jaloren commented Jan 16, 2018

@SuperQ I debated whether to add that in but was on the fence. The thing that makes this a little tricky is that DSN will need to have the name of the custom TLS config struct and it was not entirely clear to me how that should be specified. For example, we could hardcode it to "custom" and then document that the DSN needs to be set to that accordingly.

Anyway, I'll update the PR to support both use cases in the next day or two.

@SuperQ
Copy link
Member

SuperQ commented Jun 28, 2018

@jaloren Any chance you still want to work on the ENV option for TLS configs?

I guess we could merge this and add the ENV stuff later.

@SuperQ SuperQ merged commit ce43513 into prometheus:master Jun 28, 2018
@jaloren
Copy link
Contributor Author

jaloren commented Jun 29, 2018

@SuperQ sorry I just saw this email pop up. I have been swamped at work and I am not sure when I'll have a chance to look at this again. Thanks for the consideration and merge!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants