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

Add mutual TLS support to prometheus_client output plugin #5473

Merged
merged 4 commits into from Feb 27, 2019

Conversation

robertjsullivan
Copy link
Contributor

Signed-off-by: Robert Sullivan rsullivan@pivotal.io

Required for all PRs:

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

Signed-off-by: Robert Sullivan <rsullivan@pivotal.io>
@danielnelson
Copy link
Contributor

This would be a nice feature, we actually have most of this implemented for use in plugins. The best example to follow is in http_listener_v2:

For tests, look in https://github.com/influxdata/telegraf/tree/master/testutil/pki

Signed-off-by: Jesse Weaver <jeweaver@pivotal.io>
"github.com/influxdata/telegraf/plugins/outputs/prometheus_client"
"github.com/influxdata/telegraf/testutil"
"github.com/influxdata/toml"
. "github.com/onsi/gomega"
Copy link
Contributor

Choose a reason for hiding this comment

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

We will need to remove this dependency, I don't want to add it only for testing this one aspect.

Copy link
Contributor

Choose a reason for hiding this comment

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

Replaced with require

@@ -105,6 +106,12 @@ var sampleConfig = `
## If set, enable TLS with the given certificate.
# tls_cert = "/etc/ssl/telegraf.crt"
# tls_key = "/etc/ssl/telegraf.key"

## If set, enable TLS client authentication with the given CA.
# tls_ca = "/etc/ssl/telegraf_ca.crt"
Copy link
Contributor

Choose a reason for hiding this comment

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

Update this option:

  ## Set one or more allowed client CA certificate file names to 
  ## enable mutually authenticated TLS connections
  # tls_allowed_cacerts = ["/etc/telegraf/clientca.pem"]

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

# tls_ca = "/etc/ssl/telegraf_ca.crt"

## Boolean value indicating whether or not to skip SSL verification
# insecure_skip_verify = false
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this option, it's not available for the server configuration. Don't forget to update the README as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

Max Eshleman added 2 commits February 26, 2019 11:34
Signed-off-by: Robert Sullivan <rsullivan@pivotal.io>
Signed-off-by: Robert Sullivan <rsullivan@pivotal.io>
@danielnelson danielnelson added this to the 1.10.0 milestone Feb 27, 2019
@danielnelson danielnelson added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Feb 27, 2019
@danielnelson danielnelson merged commit 29cbb0a into influxdata:master Feb 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants