-
Notifications
You must be signed in to change notification settings - Fork 807
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
Update prometheus/alertmanager to version v0.25.1-0.20230505130626-263ca5c9438e #5276
Update prometheus/alertmanager to version v0.25.1-0.20230505130626-263ca5c9438e #5276
Conversation
60aa916
to
fe55d4e
Compare
416d73f
to
fce4af6
Compare
3640598
to
11b7ed0
Compare
I think it would be useful if we can pull in the prometheus/alertmanager commit that includes prometheus/alertmanager#3352 as part of this PR too. |
Sure, I can try including that as well @alvinlin123 |
20e48ad
to
9349480
Compare
9349480
to
f7e033e
Compare
f7e033e
to
a485436
Compare
a485436
to
d90e1ac
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.
Thanks. I think it is really a great job! I have only small nit about the changelog.
CHANGELOG.md
Outdated
@@ -1,6 +1,7 @@ | |||
# Changelog | |||
|
|||
## master / unreleased | |||
* [CHANGE] Alertmanager: Validating new fields on the Webhook AM config. #5276 |
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.
Since this update includes multiple changes from AM and the validation change, shall we mention more update/changes we get?
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.
sure will include more changes in the change log.
d90e1ac
to
5dcc5a7
Compare
@@ -1,6 +1,11 @@ | |||
# Changelog | |||
|
|||
## master / unreleased | |||
* [CHANGE] Updating prometheus/alertmanager from v0.25.0 to v0.25.1-0.20230505130626-263ca5c9438e. This includes the below changes. #5276 |
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.
Included user facing changes in the changelog.
5dcc5a7
to
9165bbe
Compare
@@ -686,11 +696,17 @@ func (c *PushoverConfig) UnmarshalYAML(unmarshal func(interface{}) error) error | |||
if err := unmarshal((*plain)(c)); err != nil { | |||
return err | |||
} | |||
if c.UserKey == "" { | |||
return fmt.Errorf("missing user key in Pushover config") | |||
if c.UserKey == "" && c.UserKeyFile == "" { |
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.
I think we need to disallow UserKeyFile
for PushOver config too.
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.
Added validation to disallow UserKeyFile
in PushOver Config.
if c.UserKey != "" && c.UserKeyFile != "" { | ||
return fmt.Errorf("at most one of user_key & user_key_file must be configured") | ||
} | ||
if c.Token == "" && c.TokenFile == "" { |
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.
We need to disallow TokenFIle
for PushOver also.
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.
Added validation to disallow TokenFile
in PushOver Config.
…23bc5f8ec6 Signed-off-by: Krishna Teja Puttagunta <krishtez@amazon.com> Signed-off-by: Krishna Teja Puttagunta <krishnateja325@gmail.com>
9165bbe
to
3e36cca
Compare
What this PR does:
Update Prometheus to prometheus/alertmanager@263ca5c
This includes
url_file
that got introduced as part of prometheus/alertmanager@41eb121Latest released version only contains changes till 2022-12-22. Hence updating the prometheus/alertmanger to include this change.
Which issue(s) this PR fixes:
Fixes #
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]