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

Make the alerta auth token prefix configurable and default it to Bearer. #1169

Merged
merged 1 commit into from
Feb 7, 2017

Conversation

allen13
Copy link

@allen13 allen13 commented Feb 3, 2017

Required for all non-trivial PRs
  • Rebased/mergable
  • Tests pass
  • CHANGELOG.md updated
  • Sign CLA (if not already signed)

Addresses this issue: #1165

@allen13 allen13 force-pushed the master branch 4 times, most recently from 74cb7dc to ceb2651 Compare February 3, 2017 22:18
Copy link
Contributor

@nathanielc nathanielc left a comment

Choose a reason for hiding this comment

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

@allen13 Thanks for tracking this down and providing a flexible solution.

I have one suggested change below.

@@ -15,6 +15,8 @@ type Config struct {
InsecureSkipVerify bool `toml:"insecure-skip-verify" override:"insecure-skip-verify"`
// The authentication token for this notification, can be overridden per alert.
Token string `toml:"token" override:"token,redact"`
// The prefix for the Authentication field where the token is stored
TokenPrefix string `toml:"token-prefix" override:"token-prefix"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a comment here that if you are on an older version of Alerta you may want to set this to Key? Along those lines can you update the example config in etc/kapacitor/kapacitor.conf with this new option and comments as well.

Copy link
Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

@nathanielc nathanielc left a comment

Choose a reason for hiding this comment

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

Thanks

@nathanielc nathanielc merged commit c427e11 into influxdata:master Feb 7, 2017
nathanielc added a commit that referenced this pull request Feb 7, 2017
Make the alerta auth token prefix configurable and default it to Bearer.
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.

2 participants