From d4e92ccc37e67048a957aa8e08dc5a2bc665f422 Mon Sep 17 00:00:00 2001 From: shaohuaxi Date: Mon, 12 Oct 2026 11:58:15 +0800 Subject: [PATCH] [Bugfix][Frontend] Honor endpoint plugin route overrides (#58839) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: 子华 Co-authored-by: Codex Co-authored-by: Cyrus Leung --- docs/design/endpoint_plugins.md | 4 +- tests/plugins_tests/test_endpoint_plugins.py | 160 ++++++++++++++++++- vllm/entrypoints/launchers/app.py | 4 +- vllm/plugins/endpoint_plugins/interface.py | 51 +++++- 4 files changed, 210 insertions(+), 9 deletions(-) diff --git a/docs/design/endpoint_plugins.md b/docs/design/endpoint_plugins.md index 98b755788e..361bd7099b 100644 --- a/docs/design/endpoint_plugins.md +++ b/docs/design/endpoint_plugins.md @@ -126,7 +126,9 @@ Do not expect a single endpoint plugin to also mutate engine/worker state. If yo ## Path-prefix convention -There is currently no route conflict enforcement (tracked as a follow-up to RFC [#46565](https://github.com/vllm-project/vllm/issues/46565)). A plugin's `attach_router` can register a path that collides with a core route and routes attached later win. To avoid surprising operators: +Plugin hooks run after the core routers have been registered. HTTP routes added by plugins replace earlier routes with the same path template and HTTP method, including their OpenAPI description. Methods not overridden by the plugin remain available. When multiple plugins register the same operation, the last registered route takes precedence. Different path templates are not treated as equivalent. + +There is currently no rejection of route conflicts (tracked as a follow-up to RFC [#46565](https://github.com/vllm-project/vllm/issues/46565)). To avoid surprising operators: - Namespace your routes under a distinct prefix, e.g. `/plugins//...`, rather than reusing `/v1/...` or other core prefixes - Only register routes under a core prefix (like the worked example's `/v1/admin/scheduler_config`) if you specifically intend to override or extend existing behavior and document that clearly for operators allowlisting your plugin diff --git a/tests/plugins_tests/test_endpoint_plugins.py b/tests/plugins_tests/test_endpoint_plugins.py index 2d92153900..864c915fe0 100644 --- a/tests/plugins_tests/test_endpoint_plugins.py +++ b/tests/plugins_tests/test_endpoint_plugins.py @@ -14,7 +14,7 @@ from typing import Any import httpx import pytest -from fastapi import FastAPI +from fastapi import APIRouter, FastAPI from vllm_add_dummy_endpoint_plugin import DummyAdminEndpointPlugin from vllm.entrypoints.launchers.app import build_app @@ -38,6 +38,34 @@ class _RaisingEndpointPlugin: raise RuntimeError("boom") +class _ShadowEndpointPlugin: + name = "shadow_endpoint_plugin" + required_tasks = None + + def __init__(self, path: str, include_in_schema: bool): + self.path = path + self.include_in_schema = include_in_schema + + def attach_router(self, app: FastAPI) -> None: + assert str(app.url_path_for("show_version")) == "/version" + app.openapi() + + @app.get(self.path, status_code=209, include_in_schema=self.include_in_schema) + async def plugin_version(plugin_arg: str) -> dict[str, str]: + return {"served_by": plugin_arg} + + async def init_state(self, engine_client, state, args) -> None: + pass + + +class _RouterEndpointPlugin(DummyAdminEndpointPlugin): + def __init__(self, router: APIRouter): + self.router = router + + def attach_router(self, app: FastAPI) -> None: + app.include_router(self.router) + + class _FakeEngineClient: """Minimal stand in exercising `collective_rpc`. Not a real engine.""" @@ -162,6 +190,136 @@ def test_attach_is_noop_when_nothing_discovered(monkeypatch: pytest.MonkeyPatch) assert app.state.endpoint_plugins == [] +@pytest.mark.asyncio +@pytest.mark.parametrize("path", ["/version", "/metrics"]) +@pytest.mark.parametrize("include_in_schema", [True, False]) +async def test_endpoint_plugin_override_preserves_hook_and_schema( + monkeypatch: pytest.MonkeyPatch, path: str, include_in_schema: bool +): + """Hooks see core routes; overrides win over both HTTP routes and mounts.""" + args = _build_args() + monkeypatch.setenv("VLLM_PLUGINS", "shadow_endpoint_plugin") + monkeypatch.setattr( + "vllm.plugins.load_plugins_by_group", + _fake_loader( + { + "shadow_endpoint_plugin": lambda: _ShadowEndpointPlugin( + path, include_in_schema + ) + } + ), + ) + app = build_app(args, supported_tasks=()) + + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: + response = await client.get(path, params={"plugin_arg": "plugin"}) + schema = (await client.get("/openapi.json")).json() + + assert response.status_code == 209 + assert response.json() == {"served_by": "plugin"} + if include_in_schema: + operation = schema["paths"][path]["get"] + assert "209" in operation["responses"] + assert "200" not in operation["responses"] + assert operation["parameters"][0]["name"] == "plugin_arg" + assert operation["parameters"][0]["required"] is True + response_schema = operation["responses"]["209"]["content"]["application/json"][ + "schema" + ] + assert response_schema["additionalProperties"] == {"type": "string"} + else: + assert path not in schema["paths"] + + +@pytest.mark.asyncio +async def test_endpoint_plugin_override_preserves_other_methods( + monkeypatch: pytest.MonkeyPatch, +): + """Overriding GET preserves POST's validation and the shared core route.""" + core = APIRouter() + + @core.api_route("/shared", methods=["GET", "POST"], status_code=201) + async def original(value: int) -> dict[str, int]: + return {"value": value} + + plugin = APIRouter() + + @plugin.get("/shared", status_code=202) + async def replacement(): + return {"served_by": "plugin"} + + app = FastAPI() + app.include_router(core) + original_route = app.routes[-1] + monkeypatch.setattr( + "vllm.plugins.load_endpoint_plugins", + lambda supported_tasks: [_RouterEndpointPlugin(plugin)], + ) + attach_endpoint_plugins(app, ()) + + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: + response = await client.get("/shared") + post_response = await client.post("/shared", params={"value": 7}) + invalid_response = await client.post("/shared", params={"value": "invalid"}) + + assert response.status_code == 202 + assert response.json() == {"served_by": "plugin"} + assert post_response.status_code == 201 + assert post_response.json() == {"value": 7} + assert invalid_response.status_code == 422 + assert original_route.methods == {"GET", "POST"} + assert core.routes[0].methods == {"GET", "POST"} + operations = app.openapi()["paths"]["/shared"] + assert "202" in operations["get"]["responses"] + assert "201" in operations["post"]["responses"] + + +@pytest.mark.asyncio +async def test_later_endpoint_plugin_overrides_same_operation( + monkeypatch: pytest.MonkeyPatch, +): + """The last registered override wins while unrelated plugin routes survive.""" + first = APIRouter() + + @first.get("/shared", status_code=201) + @first.get("/plugins/first") + async def first_handler(): + return {"served_by": "first"} + + second = APIRouter() + + @second.get("/shared", status_code=202) + async def second_handler(): + return {"served_by": "second"} + + monkeypatch.setattr( + "vllm.plugins.load_endpoint_plugins", + lambda supported_tasks: [ + _RouterEndpointPlugin(first), + _RouterEndpointPlugin(second), + ], + ) + app = FastAPI() + attach_endpoint_plugins(app, ()) + + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: + response = await client.get("/shared") + unrelated = await client.get("/plugins/first") + unsupported = await client.delete("/shared") + + assert response.status_code == 202 + assert response.json() == {"served_by": "second"} + assert unrelated.status_code == 200 + assert unrelated.json() == {"served_by": "first"} + assert unsupported.status_code == 405 + responses = app.openapi()["paths"]["/shared"]["get"]["responses"] + assert "202" in responses + assert "201" not in responses + + @pytest.mark.asyncio async def test_init_state_is_noop_without_phase_a(monkeypatch: pytest.MonkeyPatch): """`init_app_state` callers that never ran `build_app` (e.g. diff --git a/vllm/entrypoints/launchers/app.py b/vllm/entrypoints/launchers/app.py index ddea707810..59c4fc37a0 100644 --- a/vllm/entrypoints/launchers/app.py +++ b/vllm/entrypoints/launchers/app.py @@ -44,8 +44,8 @@ def build_app( register_api_routers(args, app, supported_tasks, model_config) - # Endpoint plugins are attached last so their routes are registered after all core - # routers. This runs even for the CPU only render server. A plugin eligible for + # Endpoint plugin hooks run after core routers so plugins can inspect them. + # This runs even for the CPU only render server. A plugin eligible for # the `render` task still gets its routes registered. It receives # `engine_client=None` at Phase B (see `_init_endpoint_plugins_state`). attach_endpoint_plugins(app, supported_tasks) diff --git a/vllm/plugins/endpoint_plugins/interface.py b/vllm/plugins/endpoint_plugins/interface.py index 00451217d2..0d847895cf 100644 --- a/vllm/plugins/endpoint_plugins/interface.py +++ b/vllm/plugins/endpoint_plugins/interface.py @@ -29,16 +29,22 @@ either exclude `"render"` from `required_tasks` or check for `None` in """ from argparse import Namespace +from copy import copy from typing import TYPE_CHECKING, Protocol, runtime_checkable from fastapi import FastAPI +from fastapi.routing import APIRoute from starlette.datastructures import State +from starlette.routing import BaseRoute from vllm.engine.protocol import EngineClient +from vllm.logger import init_logger if TYPE_CHECKING: from vllm.tasks import SupportedTask +logger = init_logger(__name__) + @runtime_checkable class EndpointPlugin(Protocol): @@ -64,8 +70,8 @@ class EndpointPlugin(Protocol): """Register this plugin's routes on `app`. Called once during `build_app()` after all core routers have been - attached. Routes attached here can shadow core routes with the same - path. There is currently no conflict enforcement (see RFC #46565 follow ups). + attached. HTTP routes registered here replace earlier routes with the + same path template and method. Other methods remain available. """ ... @@ -93,15 +99,50 @@ def attach_endpoint_plugins( ) -> None: """Phase A of endpoint plugin wiring: discover, gate and attach routes. - Attached last after all core routers. This is so endpoint plugin routes can - shadow core routes with the same path (see `EndpointPlugin.attach_router` - docstring). No-ops when no plugins are discovered/allowlisted. + Hooks run after all core routers have been attached. Plugin HTTP routes + replace earlier operations with the same path template and method in both + routing and OpenAPI. No-ops when no plugins are discovered/allowlisted. """ from vllm.plugins import load_endpoint_plugins endpoint_plugins = load_endpoint_plugins(supported_tasks) + existing_routes = {id(route): route for route in app.routes} for plugin in endpoint_plugins: plugin.attach_router(app) + + routes: list[BaseRoute] = [] + for route in app.routes: + if id(route) in existing_routes or not isinstance(route, APIRoute): + routes.append(route) + continue + + insert_at = len(routes) + retained: list[BaseRoute] = [] + for previous in routes: + if ( + isinstance(previous, APIRoute) + and previous.path == route.path + and previous.methods & route.methods + ): + logger.info( + "Endpoint plugin overrides %s %s", + ", ".join(sorted(previous.methods & route.methods)), + route.path, + ) + # Keep replacements ahead of mounts, preserving other methods. + insert_at = min(insert_at, len(retained)) + methods = previous.methods - route.methods + if not methods: + continue + previous = copy(previous) + previous.methods = methods + retained.append(previous) + retained.insert(insert_at, route) + routes = retained + + app.router.routes[:] = routes + if endpoint_plugins: + app.openapi_schema = None app.state.endpoint_plugins = endpoint_plugins