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

prometheus input: Remove credentials from URL before setting url tag #3743

Merged
merged 5 commits into from
Feb 5, 2018
Merged

prometheus input: Remove credentials from URL before setting url tag #3743

merged 5 commits into from
Feb 5, 2018

Conversation

phlipse
Copy link
Contributor

@phlipse phlipse commented Feb 2, 2018

Remove user data from URL before setting URL tag to prevent information disclosure.
See #3739.

Also run gofmt and turn variables with name url to uppercase to prevent overwrite of net/url package. Now it is more clear wheter the variable or package is used.

Required for all PRs:

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

@phlipse
Copy link
Contributor Author

phlipse commented Feb 2, 2018

circleci test fail of TestTailBadLine has nothing to do with this pull request. On my local system the failed test runs through. Any ideas?

Copy link
Contributor

@danielnelson danielnelson left a comment

Choose a reason for hiding this comment

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

Sorry about the test, I'll take a look at it.

@@ -67,7 +67,7 @@ Measurement names are based on the Metric Family and tags are created for each
label. The value is added to a field named based on the metric type.

All metrics receive the `url` tag indicating the related URL specified in the
Telegraf configuration. If using Kubernetes service discovery the `address`
Telegraf configuration. Existing username and password for basic authentication will be removed before. If using Kubernetes service discovery the `address`
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove this, we shouldn't have to say this because doing it the other way would be bad. BTW, using the userinfo section of the url is not basic auth.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh sorry, you are right, this is not basic auth! I red it in the referenced issue and wasn't think about it when writing the commit message and Readme.

I'll remove the sentence in Readme.md with next commit.

@phlipse phlipse changed the title prometheus input: Remove basic auth credentials before setting url tag prometheus input: Remove credentials from URL before setting url tag Feb 3, 2018
@phlipse
Copy link
Contributor Author

phlipse commented Feb 3, 2018

I don't know what you have done but it fixed the failing test TestTailBadLine. :)

@danielnelson danielnelson added this to the 1.5.3 milestone Feb 5, 2018
@danielnelson danielnelson added the fix pr to fix corresponding bug label Feb 5, 2018
@danielnelson danielnelson merged commit b7a68ee into influxdata:master Feb 5, 2018
danielnelson pushed a commit that referenced this pull request Feb 5, 2018
@phlipse phlipse deleted the prometheus branch February 6, 2018 08:12
mkboudreau added a commit to mkboudreau/telegraf that referenced this pull request Feb 16, 2018
* master: (59 commits)
  Added additional SQL Server performance counters (influxdata#3770)
  Update changelog
  Fix ping plugin not reporting zero durations (influxdata#3778)
  Adjust time of nightly build
  Update changelog
  Add TLS support to the mesos input plugin (influxdata#3769)
  Install new requirements for fpm gem install
  Update changelog
  Add additional metrics and reverse metric names option to openldap (influxdata#3722)
  Update paho mqtt to latest release
  Update changelog
  Remove userinfo from url tag in prometheus input (influxdata#3743)
  Update sample config in contributing docs
  Run nightly build sequentially
  Fix Makefile on Windows and use in AppVeyor build (influxdata#3748)
  Fix example source_override values in wavefront output (influxdata#3744)
  Update gitignore
  Update changelog
  Improve procstat readme
  Add native Go method for finding pids to procstat (influxdata#3559)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix pr to fix corresponding bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants