Skip to content

Update prometheus/alertmanager to version v0.25.1-0.20230505130626-263ca5c9438e#5276

Merged
alvinlin123 merged 1 commit intocortexproject:masterfrom
krishnateja325:alertmangerErrorFiltering
Jun 1, 2023
Merged

Update prometheus/alertmanager to version v0.25.1-0.20230505130626-263ca5c9438e#5276
alvinlin123 merged 1 commit intocortexproject:masterfrom
krishnateja325:alertmangerErrorFiltering

Conversation

@krishnateja325
Copy link
Contributor

@krishnateja325 krishnateja325 commented Apr 19, 2023 •
edited
Loading

What this PR does:
Update Prometheus to prometheus/alertmanager@263ca5c

This includes

Latest 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

  • Tests updated
  • [NA] Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@krishnateja325 krishnateja325 force-pushed the alertmangerErrorFiltering branch 2 times, most recently from 60aa916 to fe55d4e Compare April 19, 2023 20:47
@pull-request-size pull-request-size bot added size/M and removed size/S labels Apr 19, 2023
@krishnateja325 krishnateja325 force-pushed the alertmangerErrorFiltering branch 4 times, most recently from 416d73f to fce4af6 Compare April 21, 2023 02:06
@krishnateja325 krishnateja325 force-pushed the alertmangerErrorFiltering branch 3 times, most recently from 3640598 to 11b7ed0 Compare May 4, 2023 18:07
@alvinlin123
Copy link
Contributor

alvinlin123 commented May 22, 2023

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.

@krishnateja325
Copy link
Contributor Author

krishnateja325 commented May 22, 2023

Sure, I can try including that as well @alvinlin123

@krishnateja325 krishnateja325 force-pushed the alertmangerErrorFiltering branch 3 times, most recently from 20e48ad to 9349480 Compare May 25, 2023 00:50
@pull-request-size pull-request-size bot added size/L and removed size/M labels May 25, 2023
@krishnateja325 krishnateja325 force-pushed the alertmangerErrorFiltering branch from 9349480 to f7e033e Compare May 25, 2023 01:04
@pull-request-size pull-request-size bot added size/M and removed size/L labels May 25, 2023
@krishnateja325 krishnateja325 force-pushed the alertmangerErrorFiltering branch from f7e033e to a485436 Compare May 30, 2023 18:50
@krishnateja325 krishnateja325 changed the title Update prometheus/alertmanager to version v0.25.1-0.20230203120921-7923bc5f8ec6 Update prometheus/alertmanager to version v0.25.1-0.20230505130626-263ca5c9438e May 30, 2023
@krishnateja325 krishnateja325 force-pushed the alertmangerErrorFiltering branch from a485436 to d90e1ac Compare May 30, 2023 22:21
@krishnateja325 krishnateja325 marked this pull request as ready for review May 30, 2023 22:42
yeya24
yeya24 approved these changes May 31, 2023
View reviewed changes
Copy link
Contributor

@yeya24 yeya24 left a 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
# Changelog

## master / unreleased
* [CHANGE] Alertmanager: Validating new fields on the Webhook AM config. #5276
Copy link
Contributor

@yeya24 yeya24 May 31, 2023

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?

Copy link
Contributor Author

@krishnateja325 krishnateja325 May 31, 2023

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.

@krishnateja325 krishnateja325 force-pushed the alertmangerErrorFiltering branch from d90e1ac to 5dcc5a7 Compare May 31, 2023 04:23
krishnateja325
krishnateja325 commented May 31, 2023
View reviewed changes
CHANGELOG.md
# Changelog

## master / unreleased
* [CHANGE] Updating prometheus/alertmanager from v0.25.0 to v0.25.1-0.20230505130626-263ca5c9438e. This includes the below changes. #5276
Copy link
Contributor Author

@krishnateja325 krishnateja325 May 31, 2023

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.

@krishnateja325 krishnateja325 force-pushed the alertmangerErrorFiltering branch from 5dcc5a7 to 9165bbe Compare May 31, 2023 04:59
alvinlin123
alvinlin123 reviewed Jun 1, 2023
View reviewed changes
vendor/github.com/prometheus/alertmanager/config/notifiers.go
}
if c.UserKey == "" {
return fmt.Errorf("missing user key in Pushover config")
if c.UserKey == "" && c.UserKeyFile == "" {
Copy link
Contributor

@alvinlin123 alvinlin123 Jun 1, 2023

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.

Copy link
Contributor Author

@krishnateja325 krishnateja325 Jun 1, 2023

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.

alvinlin123
alvinlin123 reviewed Jun 1, 2023
View reviewed changes
vendor/github.com/prometheus/alertmanager/config/notifiers.go
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 == "" {
Copy link
Contributor

@alvinlin123 alvinlin123 Jun 1, 2023

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.

Copy link
Contributor Author

@krishnateja325 krishnateja325 Jun 1, 2023

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.

@krishnateja325
Update prometheus/alertmanager to version v0.25.1-0.20230203120921-79…
3e36cca 
…23bc5f8ec6

Signed-off-by: Krishna Teja Puttagunta <krishtez@amazon.com>
Signed-off-by: Krishna Teja Puttagunta <krishnateja325@gmail.com>
@krishnateja325 krishnateja325 force-pushed the alertmangerErrorFiltering branch from 9165bbe to 3e36cca Compare June 1, 2023 16:04
@pull-request-size pull-request-size bot added size/L and removed size/M labels Jun 1, 2023
@alvinlin123 alvinlin123 merged commit fba65a1 into cortexproject:master Jun 1, 2023
@nschad nschad mentioned this pull request Jul 30, 2023
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yeya24 yeya24 yeya24 approved these changes

+1 more reviewer

@alvinlin123 alvinlin123 alvinlin123 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

size/L

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@krishnateja325 @alvinlin123 @yeya24