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

amtool to support http_config to access alertmanager #2764

Merged
merged 10 commits into from
Jan 19, 2022

Conversation

clyang82
Copy link
Contributor

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]

@roidelapluie
Copy link
Member

roidelapluie commented Nov 10, 2021

Thanks! We should error if the user specifies multiple authentication mechanisms.

@clyang82
Copy link
Contributor Author

/retest

cli/root.go Outdated Show resolved Hide resolved
@clyang82 clyang82 force-pushed the amtool_bearer_token branch from 04a8436 to f10f952 Compare November 11, 2021 01:15
@clyang82
Copy link
Contributor Author

Thanks @roidelapluie

@roidelapluie
Copy link
Member

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?

@clyang82
Copy link
Contributor Author

Sorry for late response. You can put the bearer_token in $HOME/.config/amtool/config.yml or /etc/amtool/config.yml. for example:

alertmanager.url: https://xxx
tls.insecure.skip.verify: true
bearer-token: sha256~xxxxxxx

Does it help? @roidelapluie

@roidelapluie
Copy link
Member

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

@clyang82
Copy link
Contributor Author

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).

@clyang82
Copy link
Contributor Author

clyang82 commented Dec 3, 2021

@roidelapluie Can we merge http_config with config.yml? the example of $HOME/.config/amtool/config.yml can be:

alertmanager.url: https://alertmanager
tls.insecure.skip.verify: true
require-comment: false
http_config:
  bearer_token_file: /tmp/bar

@roidelapluie
Copy link
Member

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.

@clyang82 clyang82 force-pushed the amtool_bearer_token branch from f10f952 to 2ff62e4 Compare December 8, 2021 05:56
@clyang82 clyang82 requested a review from roidelapluie December 8, 2021 05:56
@clyang82 clyang82 force-pushed the amtool_bearer_token branch from 2ff62e4 to e94ae73 Compare December 8, 2021 06:02
@clyang82 clyang82 changed the title amtool to support BearerToken to access alertmanager amtool to support http_config to access alertmanager Dec 8, 2021
@clyang82 clyang82 force-pushed the amtool_bearer_token branch from e94ae73 to 5e7eea0 Compare December 8, 2021 06:20
clyang82 and others added 3 commits December 20, 2021 17:47
@clyang82 clyang82 force-pushed the amtool_bearer_token branch from 5e7eea0 to 07714ee Compare December 20, 2021 09:47
@clyang82 clyang82 force-pushed the amtool_bearer_token branch from 07714ee to 35cbac8 Compare December 20, 2021 13:11
cli/root.go Outdated Show resolved Hide resolved
cli/root.go Outdated Show resolved Hide resolved
cli/root.go Outdated Show resolved Hide resolved
cli/root.go Outdated Show resolved Hide resolved
cli/config/http_config.go Outdated Show resolved Hide resolved
cli/root.go Outdated Show resolved Hide resolved
@clyang82 clyang82 force-pushed the amtool_bearer_token branch from df41a14 to dfcbd82 Compare December 21, 2021 06:02
Signed-off-by: clyang82 <[email protected]>
@clyang82 clyang82 force-pushed the amtool_bearer_token branch from 17498b9 to eabd9c4 Compare December 24, 2021 01:25
Copy link
Member

@simonpasquier simonpasquier left a 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!

cli/config/http_config_test.go Outdated Show resolved Hide resolved
cli/config/http_config.go Outdated Show resolved Hide resolved
cli/config/http_config_test.go Outdated Show resolved Hide resolved
cli/config/http_config_test.go Outdated Show resolved Hide resolved
cli/config/http_config_test.go Show resolved Hide resolved
cli/config/http_config.go Outdated Show resolved Hide resolved
cli/root.go Outdated Show resolved Hide resolved
cli/root.go Outdated Show resolved Hide resolved
cli/root.go Show resolved Hide resolved
cli/root.go Outdated Show resolved Hide resolved
@clyang82 clyang82 requested a review from simonpasquier January 6, 2022 03:47
@clyang82 clyang82 force-pushed the amtool_bearer_token branch from d6a5a05 to 5b4796f Compare January 6, 2022 03:52
Signed-off-by: clyang82 <[email protected]>
@clyang82 clyang82 force-pushed the amtool_bearer_token branch from 5b4796f to 9d105f6 Compare January 6, 2022 04:20
cli/root.go Outdated Show resolved Hide resolved
@clyang82 clyang82 force-pushed the amtool_bearer_token branch from 7e26e4c to 1fb8047 Compare January 7, 2022 10:49
Co-authored-by: Simon Pasquier <[email protected]>
Signed-off-by: clyang82 <[email protected]>
@clyang82 clyang82 force-pushed the amtool_bearer_token branch from 1fb8047 to 0625f90 Compare January 7, 2022 10:56
@clyang82 clyang82 requested a review from simonpasquier January 7, 2022 11:40
cli/root.go Outdated Show resolved Hide resolved
Signed-off-by: clyang82 <[email protected]>
@clyang82 clyang82 requested a review from simonpasquier January 8, 2022 02:07
Signed-off-by: clyang82 <[email protected]>
@clyang82
Copy link
Contributor Author

@simonpasquier PTAL if you are OK to merge it in. Thanks.

Copy link
Member

@simonpasquier simonpasquier left a 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!

Copy link
Member

@roidelapluie roidelapluie left a comment

Choose a reason for hiding this comment

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

LGTM

@roidelapluie roidelapluie merged commit ef25f81 into prometheus:main Jan 19, 2022
@clyang82 clyang82 deleted the amtool_bearer_token branch January 19, 2022 12:52
@roidelapluie
Copy link
Member

Thanks!

dubyte pushed a commit to dubyte/alertmanager that referenced this pull request Jan 26, 2022
* 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]>
simonpasquier added a commit that referenced this pull request Mar 4, 2022
* 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]>
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