From 4a769c954eab6896186a9de602e7df0dbaf0edf9 Mon Sep 17 00:00:00 2001 From: Yassin Kortam Date: Tue, 7 Jul 2026 18:36:38 +0300 Subject: [PATCH] fix(gateway): keep the Prometheus /metrics Mount in the gateway route trim (#32317) --- gateway/main.py | 16 +++- gateway/routes/allowlist.py | 8 +- .../proxy/test_component_allowlists.py | 95 ++++++++++++++++++- 3 files changed, 110 insertions(+), 9 deletions(-) diff --git a/gateway/main.py b/gateway/main.py index 09d30f5da3f..61b885b27e4 100644 --- a/gateway/main.py +++ b/gateway/main.py @@ -25,17 +25,25 @@ DatabaseURLSettings.from_env().apply_to_env() from litellm.proxy.proxy_server import app -from gateway.routes.allowlist import GATEWAY_EXACT_PATHS, GATEWAY_PATH_PREFIXES +from gateway.routes.allowlist import ( + GATEWAY_EXACT_PATHS, + GATEWAY_MOUNT_PATHS, + GATEWAY_PATH_PREFIXES, +) def _is_gateway_route(route) -> bool: - """Keep the route on the gateway if its path is in the LLM data-plane surface.""" + """Keep the route on the gateway if its path is in the LLM data-plane surface. + + Prometheus registers /metrics as a Mount (``app.mount("/metrics", make_asgi_app())``), + so Mounts are matched against GATEWAY_MOUNT_PATHS instead of being dropped with + the UI static mounts. + """ path = getattr(route, "path", None) if path is None: return False if isinstance(route, Mount): - # Gateway never serves the static UI or its asset bundles. - return False + return path in GATEWAY_MOUNT_PATHS if path in GATEWAY_EXACT_PATHS: return True return any(path.startswith(prefix) for prefix in GATEWAY_PATH_PREFIXES) diff --git a/gateway/routes/allowlist.py b/gateway/routes/allowlist.py index 144bb4c473f..792a56a2cd8 100644 --- a/gateway/routes/allowlist.py +++ b/gateway/routes/allowlist.py @@ -106,7 +106,7 @@ GATEWAY_PATH_PREFIXES: tuple[str, ...] = ( # Health & ops "/health", "/metrics", - "/watsonx" + "/watsonx", ) GATEWAY_EXACT_PATHS: frozenset[str] = frozenset( @@ -120,3 +120,9 @@ GATEWAY_EXACT_PATHS: frozenset[str] = frozenset( "/test", } ) + +GATEWAY_MOUNT_PATHS: frozenset[str] = frozenset( + { + "/metrics", + } +) diff --git a/tests/test_litellm/proxy/test_component_allowlists.py b/tests/test_litellm/proxy/test_component_allowlists.py index 926ce3bee66..3dd8d8b28cd 100644 --- a/tests/test_litellm/proxy/test_component_allowlists.py +++ b/tests/test_litellm/proxy/test_component_allowlists.py @@ -11,10 +11,16 @@ clients hitting that path on the corresponding pod get a 404. This test guarantees that the union of the two trimmed route sets equals the full set of routes on the proxy app — i.e. no endpoint is dropped on the floor. -The test reproduces the same predicate that ``gateway/main.py`` and -``backend/main.py`` use, without importing them. The component modules wrap +The union-coverage test reproduces the same predicate that ``gateway/main.py`` +and ``backend/main.py`` use, without importing them. The component modules wrap the shared ``app.router.lifespan_context``; importing them in the test process -would chain wrappers and corrupt the snapshot. +would chain wrappers and corrupt the snapshot. The gateway Mount tests below +import the real ``gateway.main._is_gateway_route`` instead, undoing both of the +module's import-time side effects: the lifespan wrapper is restored right after +the import, and the DATABASE_* env vars are popped for its duration because +``gateway.main`` runs ``DatabaseURLSettings.from_env().apply_to_env()`` at +import (which raises on a non-postgres ``DATABASE_URL`` scheme and can mint an +RDS IAM token when ``IAM_TOKEN_DB_AUTH`` is set). """ import os @@ -36,6 +42,7 @@ for _key, _value in _THROWAWAY_ENV.items(): os.environ.setdefault(_key, _value) from fastapi.routing import Mount +from prometheus_client import make_asgi_app # gateway/ and backend/ live at the repo root, not inside litellm/. _REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..", "..")) @@ -47,7 +54,11 @@ from backend.routes.allowlist import ( BACKEND_MOUNT_PATHS, BACKEND_PATH_PREFIXES, ) -from gateway.routes.allowlist import GATEWAY_EXACT_PATHS, GATEWAY_PATH_PREFIXES +from gateway.routes.allowlist import ( + GATEWAY_EXACT_PATHS, + GATEWAY_MOUNT_PATHS, + GATEWAY_PATH_PREFIXES, +) from litellm.proxy.proxy_server import app for _key, _previous in _PRE_EXISTING_ENV.items(): @@ -56,6 +67,24 @@ for _key, _previous in _PRE_EXISTING_ENV.items(): else: os.environ[_key] = _previous +_DB_ENV_KEYS = ( + "DATABASE_URL", + "DIRECT_URL", + "DATABASE_URL_READ_REPLICA", + "DATABASE_HOST", + "DATABASE_HOST_READ_REPLICA", + "DATABASE_PASSWORD", + "IAM_TOKEN_DB_AUTH", +) +_PRE_DB_ENV = {_key: os.environ.pop(_key, None) for _key in _DB_ENV_KEYS} +_PRE_COMPONENT_LIFESPAN = app.router.lifespan_context +from gateway.main import _is_gateway_route + +app.router.lifespan_context = _PRE_COMPONENT_LIFESPAN +for _key, _previous in _PRE_DB_ENV.items(): + if _previous is not None: + os.environ[_key] = _previous + def _component_paths(routes, exact_paths, path_prefixes) -> set[str]: """Reproduce ``gateway.main._is_gateway_route`` / ``backend.main._is_backend_route``.""" @@ -133,3 +162,61 @@ def test_backend_drops_non_allowlisted_mounts(): for mount_path in non_backend_mounts: assert mount_path not in BACKEND_MOUNT_PATHS, \ f"Mount {mount_path} should not be in BACKEND_MOUNT_PATHS" + + +def test_gateway_mount_paths_defined(): + """GATEWAY_MOUNT_PATHS constant must exist and expose /metrics.""" + assert isinstance(GATEWAY_MOUNT_PATHS, frozenset), \ + f"GATEWAY_MOUNT_PATHS must be a frozenset, got {type(GATEWAY_MOUNT_PATHS)}" + assert "/metrics" in GATEWAY_MOUNT_PATHS, \ + "/metrics Mount path must be in GATEWAY_MOUNT_PATHS" + + +def test_gateway_trim_keeps_metrics_mount(): + """The Prometheus /metrics Mount must survive the gateway route trim. + + Regression test for https://github.com/BerriAI/litellm/issues/30291: + ``_is_gateway_route`` used to reject every Mount before the allowlist + check, so the /metrics Mount registered by + ``PrometheusLogger._mount_metrics_endpoint()`` was dropped at startup and + the gateway returned 404 on /metrics. + """ + metrics_mount = Mount("/metrics", app=make_asgi_app()) + routes = [*app.router.routes, metrics_mount] + trimmed = [r for r in routes if _is_gateway_route(r)] + assert metrics_mount in trimmed, \ + "/metrics Mount must survive the gateway route trim" + + +def test_gateway_drops_ui_and_swagger_mounts(): + """UI static and swagger Mounts must still be trimmed from the gateway.""" + for path in ("/ui", "/_next", "/litellm-asset-prefix/_next", "/swagger"): + assert not _is_gateway_route(Mount(path, app=make_asgi_app())), \ + f"Mount {path} must not be served by the gateway" + + +def test_every_app_mount_is_assigned_to_a_component(): + """Every Mount on the proxy app must be consciously assigned to a component. + + A Mount must be kept by the gateway (GATEWAY_MOUNT_PATHS), kept by the + backend (BACKEND_MOUNT_PATHS), or be a static mount served by the + dedicated UI container. A Mount matching none of these is unreachable in + a componentized deployment, which is exactly how the /metrics Mount was + silently dropped. + """ + ui_served_prefixes = ("/ui", "/_next", "/litellm-asset-prefix") + mounts = [*app.router.routes, Mount("/metrics", app=make_asgi_app())] + unassigned = { + path + for r in mounts + if isinstance(r, Mount) + and (path := getattr(r, "path", None)) is not None + and path not in GATEWAY_MOUNT_PATHS + and path not in BACKEND_MOUNT_PATHS + and not path.startswith(ui_served_prefixes) + } + assert not unassigned, ( + f"{len(unassigned)} Mount(s) are not exposed on any component. " + f"Add them to GATEWAY_MOUNT_PATHS, BACKEND_MOUNT_PATHS, or serve them " + f"from the UI container:\n " + "\n ".join(sorted(unassigned)) + )