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

error if http method for home or config is not GET #78

Merged

Conversation

erdii
Copy link
Member

@erdii erdii commented Mar 2, 2021

I accidentally used http://jiralert:9097/ as webhook url in alertmanager and searched for a full day, wondering why neither alertmanager nor jiralert logged an error 😅

I propose this fix as a future safeguard for other poor souls like me:
Check request method on the home and config endpoints and error out if it's not a GET request

@erdii erdii force-pushed the guard-home-and-config-routes branch from cf6403d to ec3fd77 Compare March 2, 2021 15:40
Signed-off-by: Josh Gwosdz <jgwosdz@redhat.com>
Signed-off-by: Josh Gwosdz <jgwosdz@redhat.com>
@erdii
Copy link
Member Author

erdii commented Mar 2, 2021

Sadly I cannot see the test results without logging in to CircleCI (which wants access to my private repos) :(

…o anyways here

Signed-off-by: Josh Gwosdz <jgwosdz@redhat.com>
@erdiis-pawn
Copy link

hahaaa it's me! somebody totally different who surely isn't @erdii with an empty profile!!

Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

LGTM, just minor nit 🤗

Thanks!

cmd/jiralert/content.go Outdated Show resolved Hide resolved
cmd/jiralert/content.go Show resolved Hide resolved
Signed-off-by: Josh Gwosdz <jgwosdz@redhat.com>
@erdii
Copy link
Member Author

erdii commented Mar 5, 2021

nit resolved! :) your stage @bwplotka

@bwplotka bwplotka merged commit 98f6a70 into prometheus-community:master Mar 5, 2021
@erdii erdii deleted the guard-home-and-config-routes branch March 29, 2021 11:43
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