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

fix: set correct web.external-url and web.route-prefix #302

Merged
merged 6 commits into from
Dec 3, 2024

Conversation

lucabello
Copy link
Contributor

Issue

Closes #293.

Currently, we were setting web.external-url to the internal, not external url because the external one from Traefik contains the path; this messes things up and makes Alertmanager unreachable.

The route-prefix is deduced from the external-url (by default), meaning that if Traefik is serving Alertmanager at 1.2.3.4/am, Alertmanager will then prefix all of its endpoints with /am, redirecting the user to 1.2.3.4/am/am. This will happen recursively, leading to ERR_TOO_MANY_REDIRECTS.

Solution

There is a separate config option in the alertmanager binary to set the route-prefix; setting that to / allows us to avoid the infinite redirect issue, and to correctly link to Alertmanager when using an external URL.

Here's an example alert using {{ .ExternalURL }} in its message.

Before this PR:
❌ (related** to Traefik, but using internal URL)
Screenshot-2024-11-26_10-37

After this PR:
✅ (not related to Traefik, correctly displaying the internal url)
Screenshot-2024-11-26_11-10

✅ (related to Traefik, correctly displaying the external url)
Screenshot-2024-11-26_11-10_1

Testing Instructions

Follow the reproduction steps in the linked issue, and verify that the alerts are sent with the correct {{ .ExternalUrl }}.

@lucabello lucabello requested a review from a team as a code owner November 26, 2024 10:13
sed-i and others added 2 commits December 2, 2024 11:04
* Stop svc while waiting for cert

* handle_exec

* Lint

* Debug

* Partial revert

* Cleanup

* Update test

* fetch-lib the library after merging that PR

* trigger ci

---------

Co-authored-by: Luca Bello <luca.bello@canonical.com>
@lucabello
Copy link
Contributor Author

Thanks @sed-i for fixing the cert handler bug, that fixed CI!

@lucabello lucabello merged commit 59d1731 into main Dec 3, 2024
13 checks passed
@lucabello lucabello deleted the fix/web-external-url branch December 3, 2024 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect web.external-url set
2 participants