-
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
mark webhook URL as a secret #3228
Conversation
Signed-off-by: Simon Rozet <[email protected]>
774f7cc
to
6a5eed6
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.
LGTM
Any thoughts @simonpasquier? |
I don't feel strongly about it but we need to make it explicit in the CHANGELOG that the webhook's URL is now a secret value and that it won't show up in the Alertmanager's logs in clear. |
Should I update the changelog or will you take care of it? Also I still think #3223 is useful, for deployments where secrets are injected as files (e.g. secret volume in Kubernetes or systemd-creds) I will rebase that PR and add some more tests after this is merged. |
No, whoever is going to shepherd/review the next release should be vigilant. And yes #3223 is useful. |
I'll keep it in mind. |
Signed-off-by: Simon Rozet <[email protected]>
Signed-off-by: Simon Rozet <[email protected]>
As discussed in #3223
/cc @gotjosh @simonpasquier