-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
amtool to support http_config to access alertmanager #2764
Conversation
Thanks! We should error if the user specifies multiple authentication mechanisms. |
/retest |
04a8436
to
f10f952
Compare
Thanks @roidelapluie |
Wait, I now realize that this is giving the token on the command line, which is insecure. Can we point to a bearer token file instead? |
Sorry for late response. You can put the bearer_token in $HOME/.config/amtool/config.yml or /etc/amtool/config.yml. for example:
Does it help? @roidelapluie |
I am still against it but I do not want to be "on the way" so I have reached the community for more data/ideas/opinions: https://groups.google.com/g/prometheus-developers/c/-lXLx2nYKlk |
Agree with your point that give the token on the command line is insecure. But it is a little complex for the user who needs to create 2 or more files (config.yml and http_client conf file, etc). |
@roidelapluie Can we merge http_config with config.yml? the example of $HOME/.config/amtool/config.yml can be:
|
This is two different things. I would not mix them: First, the style is different: one is dot.style.keys; the other is snake_case_keys. Then, they map to different things. I would avois users thinking they can pass the parameters to the command line, like the other values. |
f10f952
to
2ff62e4
Compare
2ff62e4
to
e94ae73
Compare
e94ae73
to
5e7eea0
Compare
Signed-off-by: clyang82 <[email protected]>
Signed-off-by: clyang82 <[email protected]>
Co-authored-by: Julien Pivotto <[email protected]> Signed-off-by: clyang82 <[email protected]>
5e7eea0
to
07714ee
Compare
Signed-off-by: clyang82 <[email protected]>
07714ee
to
35cbac8
Compare
df41a14
to
dfcbd82
Compare
Signed-off-by: clyang82 <[email protected]>
dfcbd82
to
7cac620
Compare
Signed-off-by: clyang82 <[email protected]>
17498b9
to
eabd9c4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments, looks good otherwise. Thanks!
d6a5a05
to
5b4796f
Compare
Signed-off-by: clyang82 <[email protected]>
5b4796f
to
9d105f6
Compare
7e26e4c
to
1fb8047
Compare
Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: clyang82 <[email protected]>
1fb8047
to
0625f90
Compare
Signed-off-by: clyang82 <[email protected]>
Signed-off-by: clyang82 <[email protected]>
@simonpasquier PTAL if you are OK to merge it in. Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. @roidelapluie could you give a final look in case I've missed anything? thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks! |
* Support http_config for amtool Co-authored-by: Julien Pivotto <[email protected]> Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: clyang82 <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]>
* add active time interval Signed-off-by: Sinuhe Tellez <[email protected]> * fix active time interval Signed-off-by: Sinuhe Tellez <[email protected]> * fix unittests for active time interval Signed-off-by: Sinuhe Tellez <[email protected]> * Update notify/notify.go Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * Update dispatch/route.go Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * split the stage for active and mute intervals Signed-off-by: Sinuhe Tellez <[email protected]> * Update notify/notify.go Adds doc for a helper function Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * Update notify/notify.go Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * Update notify/notify.go Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * Update notify/notify.go Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * fix code after commit suggestions Signed-off-by: Sinuhe Tellez <[email protected]> * Making mute_time_interval and time_intervals can coexist in the config Signed-off-by: Sinuhe Tellez <[email protected]> * docs: configuration's doc has been updated about time intervals Signed-off-by: Sinuhe Tellez <[email protected]> * Update config/config.go Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * Update docs/configuration.md Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * Update docs/configuration.md Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * Update docs/configuration.md Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * Update docs/configuration.md Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * updates configuration readme to improve active time description Signed-off-by: Sinuhe Tellez <[email protected]> * merge deprecated mute_time_intervals and time_intervals Signed-off-by: Sinuhe Tellez <[email protected]> * Update docs/configuration.md Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * Update docs/configuration.md Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * Update docs/configuration.md Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * Update docs/configuration.md Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * Update docs/configuration.md Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * Update docs/configuration.md Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * Update cmd/alertmanager/main.go Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * Update cmd/alertmanager/main.go Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * fmt main.go Signed-off-by: Sinuhe Tellez <[email protected]> * Update docs/configuration.md Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * Update docs/configuration.md Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * fix lint error Signed-off-by: clyang82 <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * Document that matchers are ANDed together Signed-off-by: Mac Chaffee <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * Remove extra parentheticals Signed-off-by: Mac Chaffee <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * config: root route should have empty matchers Unmarshal should validate that the root route does not contain any matchers. Prior to this change, only the deprecated match structures were checked. Signed-off-by: Philip Gough <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * chore: Let git ignore temporary files for ui/app Signed-off-by: nekketsuuu <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * adding max_alerts parameter to slack webhook config correcting the logic to trucate fields instead of dropping alerts in the slack integration Signed-off-by: Prashant Balachandran <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * *: bump to Go 1.17 (#2792) * *: bump to Go 1.17 Signed-off-by: Simon Pasquier <[email protected]> * *: fix yamllint errors Signed-off-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * Automate CSS-inlining for default HTML email template (#2798) * Automate CSS-inlining for default HTML email template The original HTML email template was added in `template/email.html`. It looks like the CSS was manually inlined. Most likely using the premailer.dialect.ca web form, which is mentioned in the README for the Mailgun transactional-email-templates project. The resulting HTML with inlined CSS was then copied into `template/default.tmpl`. This has resulted in `email.html` and `default.tmpl` diverging at times. This commit adds build automation to inline the CSS automatically using [juice][1]. The Go template containing the resulting HTML has been moved into its own file to avoid the script that performs the CSS inlining having to parse the `default.tmpl` file to insert it there. Fixes #1939. [1]: https://www.npmjs.com/package/juice Signed-off-by: Brad Ison <[email protected]> * Update asset/assets_vfsdata.go Signed-off-by: Brad Ison <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * go.{mod,sum}: update Go dependencies Signed-off-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * amtool to support http_config to access alertmanager (#2764) * Support http_config for amtool Co-authored-by: Julien Pivotto <[email protected]> Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: clyang82 <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * notify/sns: detect FIFO topic based on the rendered value Since the TopicARN field is a template string, it's safer to check for the ".fifo" suffix in the rendered string. Signed-off-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * config: delegate Sigv4 validation to the inner type This change also adds unit tests for SNS configuration. Signed-off-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * fix unittests Signed-off-by: Sinuhe Tellez <[email protected]> * fix comment about active time interval Signed-off-by: Sinuhe Tellez <[email protected]> * fix another comment about active time interval Signed-off-by: Sinuhe Tellez <[email protected]> * Update docs/configuration.md Fix typo in documentation Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> * Update docs/configuration.md Co-authored-by: Simon Pasquier <[email protected]> Signed-off-by: Sinuhe Tellez <[email protected]> Co-authored-by: Simon Pasquier <[email protected]> Co-authored-by: clyang82 <[email protected]> Co-authored-by: Mac Chaffee <[email protected]> Co-authored-by: Philip Gough <[email protected]> Co-authored-by: nekketsuuu <[email protected]> Co-authored-by: Prashant Balachandran <[email protected]> Co-authored-by: Simon Pasquier <[email protected]> Co-authored-by: Brad Ison <[email protected]> Co-authored-by: Julien Pivotto <[email protected]>
This PR is to enable amtool using bearerToken to access alertmanager. the use case is expose the alertmanager api via openshift route or k8s ingress, the external amtool can access the exposed alertmanager by providing bearerToken.
Signed-off-by: clyang82 [email protected]