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

bugfix: Remove whitespace and escape characters from URLs that are taken from secrets #4068

Merged
merged 1 commit into from
Jun 9, 2021

Conversation

Jammicus
Copy link
Contributor

@Jammicus Jammicus commented Jun 4, 2021

When decoding from a kubernetes secret the slack url may have whitespace or escape characters appended to its suffix or prefix, this removes these so that it can correctly be parsed into alertmanager config

fixes #4037

Release Note Template (will be copied)

Fixed bug in Alertmanager config where URLS that are taken from Kubernetes secrets might contain whitespace or newline characters

@Jammicus Jammicus requested a review from a team as a code owner June 4, 2021 12:24
@simonpasquier
Copy link
Contributor

it would make sense for all URLs configured via secrets

@Jammicus
Copy link
Contributor Author

Jammicus commented Jun 4, 2021

Failing test for TestAllNS/y/PromPreserveUserAddedMetadata, will fix when doing the Pr comments

@simonpasquier
Copy link
Contributor

PromPreserveUserAddedMetadata is flaky (I thought we had an issue about it but I can't find it right now). Feel free to investigate why the test fails sometimes but it's a bit of a rabbit hole :)

@Jammicus
Copy link
Contributor Author

Jammicus commented Jun 4, 2021

Haha I see what you mean about a rabbit hole....

Looks like its a timing issue when updating during reconciling the secrets annotations and labels, from what I can see happening is:

  1. A series of kubernetes objects have custom annotations and labels added to it, with a secret being the last one
  2. Reconciliation happens via scaling up replicas of the prometheus CRD
  3. Sometimes the secret loses the custom annotations and labels, causing the test to fail

Adding a wait between adding the labels + annotations and reconciliation does not seem to help. Interestingly, moving the ordering of which resources have their labels/annotations changed first does affect it, so putting moving the secret from being the last to be modified to being second from last completely solves the issue. The object that is now last (an endpoint) always seems to keep its annotations and labels

@Jammicus
Copy link
Contributor Author

Jammicus commented Jun 5, 2021

Raised #4070 for the sporadic test failure

When decoding from a kubernetes secret the slackurl may have whitespace or escape characters appended to its suffix and prefix, this removes these so that it can correctly be parsed into alertmanager config

fixes prometheus-operator#4037
@Jammicus Jammicus changed the title bugfix: Remove whitespace and escape characters from slack URLs bugfix: Remove whitespace and escape characters from URLs that are taken from secrets Jun 6, 2021
@Jammicus
Copy link
Contributor Author

Jammicus commented Jun 6, 2021

Updated to include other resources that support taking URLs from secrets

Copy link
Member

@paulfantom paulfantom left a comment

Choose a reason for hiding this comment

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

lgtm

@simonpasquier simonpasquier merged commit dab2194 into prometheus-operator:master Jun 9, 2021
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.

Alertmanager fails with slack api key from secrets
3 participants