Skip to content

Keep the path prefix of Alertmanager and Prometheus URLs - #799

Merged
vincent-olivert-riera merged 1 commit into
line:masterfrom
MankhongGarden:fix/urljoin-base-path
Oct 19, 2026
Merged

vincent-olivert-riera merged 1 commit into
line:masterfrom
MankhongGarden:fix/urljoin-base-path

Conversation

@MankhongGarden

Copy link
Copy Markdown
Contributor

Fixes #225

What

Promgen builds Alertmanager and Prometheus URLs with urllib.parse.urljoin(base, "/api/..."). When the second argument starts with /, urljoin replaces the whole path of the base URL, so a server running under a path prefix loses it:

>>> urljoin("http://example.com/alertmanager/", "/api/v2/silences")
'http://example.com/api/v2/silences'

So with alertmanager.url: http://example.com/alertmanager/ (a common setup behind a reverse proxy, or Alertmanager started with --web.route-prefix), creating, listing and deleting silences and fetching alerts all call the wrong URL. The same happens to prometheus.url (/-/reload, clean_tombstones) and to the shard URLs used by the query proxy (/api/v1/query, /api/v1/labels, ...).

rest.py already builds the shard query URL as f"{shard.url}/api/v1/query", which keeps the prefix, so the behaviour was also inconsistent between endpoints.

Change (6 files, +68/-10)

  • promgen/util.py: add join_url(base, path), which appends the path to the base URL and keeps its prefix (base.rstrip("/") + "/" + path.lstrip("/")).
  • promgen/prometheus.py, promgen/proxy.py, promgen/tasks.py: use util.join_url at the 7 places that used urljoin with an absolute path (Alertmanager silences/alerts, Prometheus reload and clean_tombstones, proxy shard requests). The unused urljoin imports are removed.
  • Tests: test_util.test_join_url covers base URLs with and without a prefix and trailing slash, and a path with a query string. test_silence.test_alertmanager_url_with_path_prefix posts a silence with alertmanager.url set to http://example.com/alertmanager/ and checks the request goes to .../alertmanager/api/v2/silences.

Why this approach

  • A small helper keeps every call site on one line and makes the intent clear. Alternatives:
    • Make the paths relative (urljoin(base, "api/v2/silences")): only works when the configured URL ends with /. http://example.com/alertmanager would still drop alertmanager.
    • Use f-strings at each call site, as rest.py does: works, but duplicates the trailing-slash handling 7 times.
  • Behaviour for URLs without a path prefix (for example the default http://alertmanager:9093) is unchanged, so existing setups are not affected.

Testing

  • The new silence test fails on current master (request goes to http://example.com/api/v2/silences) and passes with this change.
  • promgen test: no new failures compared with master.
  • ruff check promgen and ruff format --diff promgen (ruff 0.14.14, as in the Makefile): clean.
  • promgen check and promgen makemigrations --check: no changes.

Scope

Only URL building is changed. It does not depend on any other open issue or PR.

@vincent-olivert-riera vincent-olivert-riera left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for your contribution.
Please check the review comments, thanks!

Comment thread promgen/util.py Outdated
Comment thread promgen/util.py Outdated
Comment thread promgen/tests/test_silence.py Outdated
@MankhongGarden

Copy link
Copy Markdown
Contributor Author

Thanks for the review! I've applied all three suggestions in d16438a: switched join_url to the f-string with removesuffix/removeprefix, trimmed the docstring, and removed the silence test.

@vincent-olivert-riera

Copy link
Copy Markdown
Contributor

Thanks for the review! I've applied all three suggestions in d16438a: switched join_url to the f-string with removesuffix/removeprefix, trimmed the docstring, and removed the silence test.

@MankhongGarden, thanks. Please squash the fixup commit and rebase your branch on master before we can merge it.

urllib.parse.urljoin drops the path of the base URL when the second
argument starts with "/", so a Prometheus or Alertmanager served under a
path prefix (e.g. http://example.com/prometheus/) was called without the
prefix. Add util.join_url and use it for the Alertmanager, Prometheus
and proxy shard URLs.

Fixes line#225
@MankhongGarden

Copy link
Copy Markdown
Contributor Author

Done, squashed into a single commit and rebased onto the latest master (7b8dad7). Thanks!

@vincent-olivert-riera
vincent-olivert-riera merged commit 999cd7c into line:master Oct 19, 2026
5 checks passed
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.

urljoin can't return the proper full url

2 participants