From 620ce415d5c2ff251a1ce8997d1bbf5771ee1ba2 Mon Sep 17 00:00:00 2001 From: Maysun J Faisal Date: Tue, 10 Mar 2026 17:17:44 -0400 Subject: [PATCH 1/2] First draft of MCP API Assisted-by: Claude Opus 4.6 Generated-by: Cursor Signed-off-by: Maysun J Faisal --- src/app/endpoints/mcp_servers.py | 247 +++++++++++ src/app/routers.py | 2 + src/configuration.py | 62 +++ src/models/config.py | 5 + src/models/requests.py | 163 ++++++- src/models/responses.py | 136 ++++++ tests/unit/app/endpoints/test_mcp_servers.py | 437 +++++++++++++++++++ 7 files changed, 1051 insertions(+), 1 deletion(-) create mode 100644 src/app/endpoints/mcp_servers.py create mode 100644 tests/unit/app/endpoints/test_mcp_servers.py diff --git a/src/app/endpoints/mcp_servers.py b/src/app/endpoints/mcp_servers.py new file mode 100644 index 000000000..aa9772c77 --- /dev/null +++ b/src/app/endpoints/mcp_servers.py @@ -0,0 +1,247 @@ +"""Handler for REST API calls to dynamically manage MCP servers.""" + +from typing import Annotated, Any + +from fastapi import APIRouter, Depends, HTTPException, Request, status +from llama_stack_client import APIConnectionError + +from authentication import get_auth_dependency +from authentication.interface import AuthTuple +from authorization.middleware import authorize +from client import AsyncLlamaStackClientHolder +from configuration import configuration +from models.config import Action, ModelContextProtocolServer +from models.requests import MCPServerRegistrationRequest +from models.responses import ( + ConflictResponse, + ForbiddenResponse, + InternalServerErrorResponse, + MCPServerDeleteResponse, + MCPServerInfo, + MCPServerListResponse, + MCPServerRegistrationResponse, + NotFoundResponse, + ServiceUnavailableResponse, + UnauthorizedResponse, +) +from utils.endpoints import check_configuration_loaded +from log import get_logger + +logger = get_logger(__name__) +router = APIRouter(tags=["mcp-servers"]) + + +register_responses: dict[int | str, dict[str, Any]] = { + 201: MCPServerRegistrationResponse.openapi_response(), + 401: UnauthorizedResponse.openapi_response( + examples=["missing header", "missing token"] + ), + 403: ForbiddenResponse.openapi_response(examples=["endpoint"]), + 409: ConflictResponse.openapi_response(examples=["mcp server"]), + 500: InternalServerErrorResponse.openapi_response(examples=["configuration"]), + 503: ServiceUnavailableResponse.openapi_response(), +} + + +@router.post( + "/mcp-servers", + responses=register_responses, + status_code=status.HTTP_201_CREATED, +) +@authorize(Action.REGISTER_MCP_SERVER) +async def register_mcp_server_handler( + request: Request, + body: MCPServerRegistrationRequest, + auth: Annotated[AuthTuple, Depends(get_auth_dependency())], +) -> MCPServerRegistrationResponse: + """Register an MCP server dynamically at runtime. + + Adds the MCP server to the runtime configuration and registers it + as a toolgroup with Llama Stack so it becomes available for queries. + + Raises: + HTTPException: On duplicate name, Llama Stack connection error, + or registration failure. + + Returns: + MCPServerRegistrationResponse: Details of the newly registered server. + """ + _ = auth + _ = request + + check_configuration_loaded(configuration) + + mcp_server = ModelContextProtocolServer( + name=body.name, + url=body.url, + provider_id=body.provider_id, + authorization_headers=body.authorization_headers or {}, + headers=body.headers or [], + timeout=body.timeout, + ) + + try: + configuration.add_mcp_server(mcp_server) + except ValueError as e: + response = ConflictResponse(resource="MCP server", resource_id=body.name) + raise HTTPException(**response.model_dump()) from e + + try: + client = AsyncLlamaStackClientHolder().get_client() + await client.toolgroups.register( # pyright: ignore[reportDeprecated] + toolgroup_id=mcp_server.name, + provider_id=mcp_server.provider_id, + mcp_endpoint={"uri": mcp_server.url}, + ) + except APIConnectionError as e: + configuration.remove_mcp_server(body.name) + logger.error("Failed to register MCP server with Llama Stack: %s", e) + response = ServiceUnavailableResponse(backend_name="Llama Stack", cause=str(e)) + raise HTTPException(**response.model_dump()) from e + except Exception as e: # pylint: disable=broad-exception-caught + configuration.remove_mcp_server(body.name) + logger.error("Failed to register MCP toolgroup: %s", e) + error_response = InternalServerErrorResponse( + response="Failed to register MCP server", + cause=str(e), + ) + raise HTTPException(**error_response.model_dump()) from e + + logger.info("Dynamically registered MCP server: %s at %s", body.name, body.url) + + return MCPServerRegistrationResponse( + name=mcp_server.name, + url=mcp_server.url, + provider_id=mcp_server.provider_id, + message=f"MCP server '{mcp_server.name}' registered successfully", + ) + + +list_responses: dict[int | str, dict[str, Any]] = { + 200: MCPServerListResponse.openapi_response(), + 401: UnauthorizedResponse.openapi_response( + examples=["missing header", "missing token"] + ), + 403: ForbiddenResponse.openapi_response(examples=["endpoint"]), + 500: InternalServerErrorResponse.openapi_response(examples=["configuration"]), +} + + +@router.get("/mcp-servers", responses=list_responses) +@authorize(Action.LIST_MCP_SERVERS) +async def list_mcp_servers_handler( + request: Request, + auth: Annotated[AuthTuple, Depends(get_auth_dependency())], +) -> MCPServerListResponse: + """List all registered MCP servers. + + Returns both statically configured (from YAML) and dynamically + registered (via API) MCP servers. + + Raises: + HTTPException: If configuration is not loaded. + + Returns: + MCPServerListResponse: List of all registered MCP servers with source info. + """ + _ = auth + _ = request + + check_configuration_loaded(configuration) + + servers = [] + for mcp in configuration.mcp_servers: + source = "api" if configuration.is_dynamic_mcp_server(mcp.name) else "config" + servers.append( + MCPServerInfo( + name=mcp.name, + url=mcp.url, + provider_id=mcp.provider_id, + source=source, + ) + ) + + return MCPServerListResponse(servers=servers) + + +delete_responses: dict[int | str, dict[str, Any]] = { + 200: MCPServerDeleteResponse.openapi_response(), + 401: UnauthorizedResponse.openapi_response( + examples=["missing header", "missing token"] + ), + 403: ForbiddenResponse.openapi_response(examples=["endpoint"]), + 404: NotFoundResponse.openapi_response(examples=["mcp server"]), + 500: InternalServerErrorResponse.openapi_response(examples=["configuration"]), + 503: ServiceUnavailableResponse.openapi_response(), +} + + +@router.delete("/mcp-servers/{name}", responses=delete_responses) +@authorize(Action.DELETE_MCP_SERVER) +async def delete_mcp_server_handler( + request: Request, + name: str, + auth: Annotated[AuthTuple, Depends(get_auth_dependency())], +) -> MCPServerDeleteResponse: + """Unregister a dynamically registered MCP server. + + Removes the MCP server from the runtime configuration and unregisters + its toolgroup from Llama Stack. Only servers registered via the API + can be deleted; statically configured servers cannot be removed. + + Raises: + HTTPException: If the server is not found, is statically configured, + or Llama Stack unregistration fails. + + Returns: + MCPServerDeleteResponse: Confirmation of the deletion. + """ + _ = auth + _ = request + + check_configuration_loaded(configuration) + + if not configuration.is_dynamic_mcp_server(name): + found = any(s.name == name for s in configuration.mcp_servers) + if found: + response = ForbiddenResponse( + response="Cannot delete statically configured MCP server", + cause=f"MCP server '{name}' was configured in lightspeed-stack.yaml " + "and cannot be removed via the API.", + ) + else: + response = NotFoundResponse(resource="MCP server", resource_id=name) + raise HTTPException(**response.model_dump()) + + try: + client = AsyncLlamaStackClientHolder().get_client() + await client.toolgroups.unregister( # pyright: ignore[reportDeprecated] + toolgroup_id=name + ) + except APIConnectionError as e: + logger.error("Failed to unregister MCP toolgroup from Llama Stack: %s", e) + svc_response = ServiceUnavailableResponse( + backend_name="Llama Stack", cause=str(e) + ) + raise HTTPException(**svc_response.model_dump()) from e + except Exception as e: # pylint: disable=broad-exception-caught + logger.warning( + "Llama Stack toolgroup unregister failed for '%s', " + "proceeding with local removal: %s", + name, + e, + ) + + try: + configuration.remove_mcp_server(name) + except ValueError as e: + logger.error("Failed to remove MCP server from configuration: %s", e) + response = NotFoundResponse(resource="MCP server", resource_id=name) + raise HTTPException(**response.model_dump()) from e + + logger.info("Dynamically unregistered MCP server: %s", name) + + return MCPServerDeleteResponse( + name=name, + message=f"MCP server '{name}' unregistered successfully", + ) diff --git a/src/app/routers.py b/src/app/routers.py index 97e3522f6..d5a06189d 100644 --- a/src/app/routers.py +++ b/src/app/routers.py @@ -20,6 +20,7 @@ metrics, tools, mcp_auth, + mcp_servers, # Query endpoints for Response API support query, # RHEL Lightspeed rlsapi v1 compatibility @@ -48,6 +49,7 @@ def include_routers(app: FastAPI) -> None: app.include_router(models.router, prefix="/v1") app.include_router(tools.router, prefix="/v1") app.include_router(mcp_auth.router, prefix="/v1") + app.include_router(mcp_servers.router, prefix="/v1") app.include_router(shields.router, prefix="/v1") app.include_router(providers.router, prefix="/v1") app.include_router(rags.router, prefix="/v1") diff --git a/src/configuration.py b/src/configuration.py index 41cd3deb1..08d77d22a 100644 --- a/src/configuration.py +++ b/src/configuration.py @@ -64,6 +64,7 @@ def __init__(self) -> None: self._conversation_cache: Optional[Cache] = None self._quota_limiters: list[QuotaLimiter] = [] self._token_usage_history: Optional[TokenUsageHistory] = None + self._dynamic_mcp_server_names: set[str] = set() def load_configuration(self, filename: str) -> None: """Load configuration from YAML file. @@ -165,6 +166,67 @@ def mcp_servers(self) -> list[ModelContextProtocolServer]: raise LogicError("logic error: configuration is not loaded") return self._configuration.mcp_servers + @property + def dynamic_mcp_server_names(self) -> set[str]: + """Return the set of dynamically registered MCP server names. + + Returns: + set[str]: Names of MCP servers added via the API (not from config file). + """ + return self._dynamic_mcp_server_names + + def add_mcp_server(self, mcp_server: ModelContextProtocolServer) -> None: + """Add an MCP server to the runtime configuration. + + Parameters: + mcp_server: The MCP server configuration to add. + + Raises: + LogicError: If the configuration has not been loaded. + ValueError: If an MCP server with the same name already exists. + """ + if self._configuration is None: + raise LogicError("logic error: configuration is not loaded") + for existing in self._configuration.mcp_servers: + if existing.name == mcp_server.name: + raise ValueError( + f"MCP server with name '{mcp_server.name}' already exists" + ) + self._configuration.mcp_servers.append(mcp_server) + self._dynamic_mcp_server_names.add(mcp_server.name) + + def remove_mcp_server(self, name: str) -> None: + """Remove a dynamically registered MCP server from the runtime configuration. + + Parameters: + name: The name of the MCP server to remove. + + Raises: + LogicError: If the configuration has not been loaded. + ValueError: If the server was not found or was statically configured. + """ + if self._configuration is None: + raise LogicError("logic error: configuration is not loaded") + if name not in self._dynamic_mcp_server_names: + raise ValueError( + f"MCP server '{name}' was not dynamically registered or does not exist" + ) + self._configuration.mcp_servers = [ + s for s in self._configuration.mcp_servers if s.name != name + ] + self._dynamic_mcp_server_names.discard(name) + + def is_dynamic_mcp_server(self, name: str) -> bool: + """Check if an MCP server was dynamically registered. + + Parameters: + name: The name of the MCP server. + + Returns: + bool: True if the server was registered via the API. + """ + return name in self._dynamic_mcp_server_names + @property def authentication_configuration(self) -> AuthenticationConfiguration: """Return authentication configuration. diff --git a/src/models/config.py b/src/models/config.py index d7a15a334..90f572f00 100644 --- a/src/models/config.py +++ b/src/models/config.py @@ -1019,6 +1019,11 @@ class Action(str, Enum): # RHEL Lightspeed rlsapi v1 compatibility - stateless inference (no history/RAG) RLSAPI_V1_INFER = "rlsapi_v1_infer" + # Dynamic MCP server management + REGISTER_MCP_SERVER = "register_mcp_server" + LIST_MCP_SERVERS = "list_mcp_servers" + DELETE_MCP_SERVER = "delete_mcp_server" + # A2A (Agent-to-Agent) protocol actions A2A_AGENT_CARD = "a2a_agent_card" A2A_TASK_EXECUTION = "a2a_task_execution" diff --git a/src/models/requests.py b/src/models/requests.py index 0e05e61d5..43c8fda4f 100644 --- a/src/models/requests.py +++ b/src/models/requests.py @@ -14,7 +14,13 @@ ) from pydantic import BaseModel, Field, field_validator, model_validator -from constants import MEDIA_TYPE_JSON, MEDIA_TYPE_TEXT +from constants import ( + MEDIA_TYPE_JSON, + MEDIA_TYPE_TEXT, + MCP_AUTH_CLIENT, + MCP_AUTH_KUBERNETES, + MCP_AUTH_OAUTH, +) from log import get_logger from utils import suid from utils.types import IncludeParameter, ResponseInput @@ -762,3 +768,158 @@ def echoed_params(self) -> dict[str, Any]: ] return data + + +class MCPServerRegistrationRequest(BaseModel): + """Request model for dynamically registering an MCP server. + + Attributes: + name: Unique name for the MCP server. + url: URL of the MCP server endpoint. + provider_id: MCP provider identification (defaults to "model-context-protocol"). + authorization_headers: Optional headers to send to the MCP server. + headers: Optional list of HTTP header names to forward from incoming requests. + timeout: Optional request timeout in seconds. + + Example: + ```python + request = MCPServerRegistrationRequest( + name="my-tools", + url="http://localhost:8888/mcp", + ) + ``` + """ + + name: str = Field( + ..., + description="Unique name for the MCP server", + examples=["my-mcp-tools"], + min_length=1, + max_length=256, + ) + + url: str = Field( + ..., + description="URL of the MCP server endpoint", + examples=["http://host.docker.internal:7008/api/mcp-actions/v1"], + ) + + provider_id: str = Field( + "model-context-protocol", + description="MCP provider identification", + examples=["model-context-protocol"], + ) + + authorization_headers: Optional[dict[str, str]] = Field( + default=None, + description=( + "Headers to send to the MCP server. Values must be one of the " + "supported token resolution keywords: " + "'client' - forward the caller's token provided via MCP-HEADERS, " + "'kubernetes' - use the authenticated user's Kubernetes token, " + "'oauth' - use an OAuth token provided via MCP-HEADERS. " + "File-path based secrets (used in static YAML config) are not " + "supported for dynamically registered servers." + ), + examples=[ + {"Authorization": "client"}, + {"Authorization": "kubernetes"}, + {"Authorization": "oauth"}, + ], + ) + + headers: Optional[list[str]] = Field( + default=None, + description="List of HTTP header names to forward from incoming requests", + examples=[["x-rh-identity"]], + ) + + timeout: Optional[int] = Field( + default=None, + description="Request timeout in seconds for the MCP server", + gt=0, + examples=[30], + ) + + model_config = { + "extra": "forbid", + "json_schema_extra": { + "examples": [ + { + "name": "mcp-integration-tools", + "url": "http://host.docker.internal:7008/api/mcp-actions/v1", + "authorization_headers": {"Authorization": "client"}, + }, + { + "name": "k8s-internal-service", + "url": "http://internal-mcp.default.svc.cluster.local:8080", + "authorization_headers": {"Authorization": "kubernetes"}, + }, + { + "name": "oauth-mcp-server", + "url": "https://mcp.example.com/api", + "authorization_headers": {"Authorization": "oauth"}, + }, + { + "name": "test-mcp-server", + "url": "http://host.docker.internal:8888/mcp", + "provider_id": "model-context-protocol", + "headers": ["x-rh-identity"], + "timeout": 30, + }, + ] + }, + } + + @field_validator("url") + @classmethod + def validate_url(cls, value: str) -> str: + """Validate that URL uses http or https scheme. + + Parameters: + value: The URL string to validate. + + Returns: + The validated URL string. + + Raises: + ValueError: If URL does not start with http:// or https://. + """ + if not value.startswith(("http://", "https://")): + raise ValueError("URL must use http:// or https:// scheme") + return value + + @field_validator("authorization_headers") + @classmethod + def validate_authorization_header_values( + cls, value: Optional[dict[str, str]] + ) -> Optional[dict[str, str]]: + """Validate that authorization header values use supported keywords. + + Dynamic registration only supports the token resolution keywords + ('client', 'kubernetes', 'oauth'). File-path based secrets are + rejected since the API client cannot guarantee files exist on the + server filesystem. + + Parameters: + value: The authorization headers dict to validate. + + Returns: + The validated authorization headers dict. + + Raises: + ValueError: If any header value is not a supported keyword. + """ + if value is None: + return value + allowed = {MCP_AUTH_CLIENT, MCP_AUTH_KUBERNETES, MCP_AUTH_OAUTH} + for header_name, header_value in value.items(): + stripped = header_value.strip() + if stripped not in allowed: + raise ValueError( + f"Authorization header '{header_name}' has unsupported value " + f"'{stripped}'. Dynamic registration only supports: " + f"{', '.join(sorted(allowed))}. " + "File-path based secrets are only supported in static YAML config." + ) + return value diff --git a/src/models/responses.py b/src/models/responses.py index fd5ef955a..4d7791da0 100644 --- a/src/models/responses.py +++ b/src/models/responses.py @@ -161,6 +161,98 @@ class MCPClientAuthOptionsResponse(AbstractSuccessfulResponse): } +class MCPServerInfo(BaseModel): + """Information about a registered MCP server. + + Attributes: + name: Unique name of the MCP server. + url: URL of the MCP server endpoint. + provider_id: MCP provider identification. + source: Whether the server was registered statically (config) or dynamically (api). + """ + + name: str = Field(..., description="MCP server name") + url: str = Field(..., description="MCP server URL") + provider_id: str = Field(..., description="MCP provider identification") + source: str = Field( + ..., + description="How the server was registered: 'config' (static) or 'api' (dynamic)", + examples=["config", "api"], + ) + + +class MCPServerRegistrationResponse(AbstractSuccessfulResponse): + """Response for a successful MCP server registration.""" + + name: str = Field(..., description="Registered MCP server name") + url: str = Field(..., description="Registered MCP server URL") + provider_id: str = Field(..., description="MCP provider identification") + message: str = Field(..., description="Status message") + + model_config = { + "json_schema_extra": { + "examples": [ + { + "name": "mcp-integration-tools", + "url": "http://host.docker.internal:7008/api/mcp-actions/v1", + "provider_id": "model-context-protocol", + "message": "MCP server 'mcp-integration-tools' registered successfully", + } + ] + } + } + + +class MCPServerListResponse(AbstractSuccessfulResponse): + """Response listing all registered MCP servers.""" + + servers: list[MCPServerInfo] = Field( + default_factory=list, + description="List of all registered MCP servers (static and dynamic)", + ) + + model_config = { + "json_schema_extra": { + "examples": [ + { + "servers": [ + { + "name": "mcp-integration-tools", + "url": "http://host.docker.internal:7008/api/mcp-actions/v1", + "provider_id": "model-context-protocol", + "source": "config", + }, + { + "name": "test-mcp-server", + "url": "http://host.docker.internal:8888/mcp", + "provider_id": "model-context-protocol", + "source": "api", + }, + ] + } + ] + } + } + + +class MCPServerDeleteResponse(AbstractSuccessfulResponse): + """Response for a successful MCP server deletion.""" + + name: str = Field(..., description="Deleted MCP server name") + message: str = Field(..., description="Status message") + + model_config = { + "json_schema_extra": { + "examples": [ + { + "name": "test-mcp-server", + "message": "MCP server 'test-mcp-server' unregistered successfully", + } + ] + } + } + + class ShieldsResponse(AbstractSuccessfulResponse): """Model representing a response to shields request.""" @@ -1999,6 +2091,13 @@ class NotFoundResponse(AbstractErrorResponse): ), }, }, + { + "label": "mcp server", + "detail": { + "response": "Mcp Server not found", + "cause": "Mcp Server with ID test-mcp-server does not exist", + }, + }, ] } } @@ -2022,6 +2121,43 @@ def __init__(self, *, resource: str, resource_id: str | None = None): ) +CONFLICT_DESCRIPTION = "Resource already exists" + + +class ConflictResponse(AbstractErrorResponse): + """409 Conflict - Resource already exists.""" + + description: ClassVar[str] = CONFLICT_DESCRIPTION + model_config = { + "json_schema_extra": { + "examples": [ + { + "label": "mcp server", + "detail": { + "response": "Mcp Server already exists", + "cause": ( + "Mcp Server with name 'test-mcp-server' is already registered" + ), + }, + }, + ] + } + } + + def __init__(self, *, resource: str, resource_id: str): + """Create a 409 Conflict response for a duplicate resource. + + Parameters: + resource: Type of the resource (e.g., "MCP server"). + resource_id: The identifier of the conflicting resource. + """ + response = f"{resource.title()} already exists" + cause = f"{resource.title()} with name '{resource_id}' is already registered" + super().__init__( + response=response, cause=cause, status_code=status.HTTP_409_CONFLICT + ) + + class PromptTooLongResponse(AbstractErrorResponse): """413 Payload Too Large - Prompt is too long.""" diff --git a/tests/unit/app/endpoints/test_mcp_servers.py b/tests/unit/app/endpoints/test_mcp_servers.py new file mode 100644 index 000000000..a4f586298 --- /dev/null +++ b/tests/unit/app/endpoints/test_mcp_servers.py @@ -0,0 +1,437 @@ +# pylint: disable=protected-access,redefined-outer-name + +"""Unit tests for the MCP servers dynamic registration endpoint.""" + +from pathlib import Path +from typing import Any + +import pytest +from pydantic import SecretStr, AnyHttpUrl +from fastapi import HTTPException +from llama_stack_client import APIConnectionError +from pytest_mock import MockerFixture + +from app.endpoints import mcp_servers +from authentication.interface import AuthTuple +from configuration import AppConfig +from models.config import ( + Configuration, + LlamaStackConfiguration, + ModelContextProtocolServer, + ServiceConfiguration, + UserDataCollection, + TLSConfiguration, + CORSConfiguration, +) +from models.requests import MCPServerRegistrationRequest +from models.responses import ( + MCPServerRegistrationResponse, + MCPServerListResponse, + MCPServerDeleteResponse, +) + +MOCK_AUTH: AuthTuple = ("mock_user_id", "mock_username", False, "mock_token") + + +@pytest.fixture +def mock_configuration() -> Configuration: + """Create a mock configuration with MCP servers.""" + return Configuration( + name="test", + service=ServiceConfiguration( + tls_config=TLSConfiguration( + tls_certificate_path=Path("tests/configuration/server.crt"), + tls_key_path=Path("tests/configuration/server.key"), + tls_key_password=Path("tests/configuration/password"), + ), + cors=CORSConfiguration( + allow_origins=["*"], + allow_credentials=False, + allow_methods=["*"], + allow_headers=["*"], + ), + host="localhost", + port=1234, + base_url=".", + auth_enabled=False, + workers=1, + color_log=True, + access_log=True, + root_path="/.", + ), + llama_stack=LlamaStackConfiguration( + url=AnyHttpUrl("http://localhost:8321"), + api_key=SecretStr("xyzzy"), + use_as_library_client=False, + library_client_config_path=".", + timeout=10, + ), + user_data_collection=UserDataCollection( + transcripts_enabled=False, + feedback_enabled=False, + transcripts_storage=".", + feedback_storage=".", + ), + mcp_servers=[ + ModelContextProtocolServer( + name="static-mcp", + provider_id="model-context-protocol", + url="http://localhost:3000", + ), + ], + customization=None, + authorization=None, + deployment_environment=".", + ) + + +def _make_app_config(mocker: MockerFixture, config: Configuration) -> AppConfig: + """Create an AppConfig with the given configuration and patch it.""" + app_config = AppConfig() + app_config._configuration = config + app_config._dynamic_mcp_server_names = set() + mocker.patch("app.endpoints.mcp_servers.configuration", app_config) + mocker.patch("app.endpoints.mcp_servers.authorize", lambda _: lambda func: func) + return app_config + + +def _mock_client(mocker: MockerFixture) -> Any: + """Create and patch a mock Llama Stack client.""" + mock_holder = mocker.patch("app.endpoints.mcp_servers.AsyncLlamaStackClientHolder") + mock_client = mocker.AsyncMock() + mock_holder.return_value.get_client.return_value = mock_client + return mock_client + + +@pytest.mark.asyncio +async def test_register_mcp_server_success( + mocker: MockerFixture, + mock_configuration: Configuration, +) -> None: + """Test successful MCP server registration.""" + app_config = _make_app_config(mocker, mock_configuration) + client = _mock_client(mocker) + client.toolgroups.register.return_value = None + + body = MCPServerRegistrationRequest( + name="new-mcp-server", + url="http://localhost:8888/mcp", + ) + + result = await mcp_servers.register_mcp_server_handler( + request=mocker.Mock(), body=body, auth=MOCK_AUTH + ) + + assert isinstance(result, MCPServerRegistrationResponse) + assert result.name == "new-mcp-server" + assert result.url == "http://localhost:8888/mcp" + assert result.provider_id == "model-context-protocol" + assert "registered successfully" in result.message + + client.toolgroups.register.assert_called_once_with( + toolgroup_id="new-mcp-server", + provider_id="model-context-protocol", + mcp_endpoint={"uri": "http://localhost:8888/mcp"}, + ) + + assert app_config.is_dynamic_mcp_server("new-mcp-server") + assert any(s.name == "new-mcp-server" for s in app_config.mcp_servers) + + +@pytest.mark.asyncio +async def test_register_mcp_server_duplicate_name( + mocker: MockerFixture, + mock_configuration: Configuration, +) -> None: + """Test registration fails when name already exists.""" + _make_app_config(mocker, mock_configuration) + _mock_client(mocker) + + body = MCPServerRegistrationRequest( + name="static-mcp", + url="http://localhost:9999/mcp", + ) + + with pytest.raises(HTTPException) as exc_info: + await mcp_servers.register_mcp_server_handler( + request=mocker.Mock(), body=body, auth=MOCK_AUTH + ) + assert exc_info.value.status_code == 409 + + +@pytest.mark.asyncio +async def test_register_mcp_server_llama_stack_failure( + mocker: MockerFixture, + mock_configuration: Configuration, +) -> None: + """Test registration rolls back on Llama Stack connection failure.""" + app_config = _make_app_config(mocker, mock_configuration) + client = _mock_client(mocker) + client.toolgroups.register.side_effect = APIConnectionError(request=mocker.Mock()) + + body = MCPServerRegistrationRequest( + name="failing-server", + url="http://localhost:8888/mcp", + ) + + with pytest.raises(HTTPException) as exc_info: + await mcp_servers.register_mcp_server_handler( + request=mocker.Mock(), body=body, auth=MOCK_AUTH + ) + assert exc_info.value.status_code == 503 + + assert not app_config.is_dynamic_mcp_server("failing-server") + assert not any(s.name == "failing-server" for s in app_config.mcp_servers) + + +@pytest.mark.asyncio +async def test_register_mcp_server_with_all_fields( + mocker: MockerFixture, + mock_configuration: Configuration, +) -> None: + """Test registration with all optional fields provided.""" + _make_app_config(mocker, mock_configuration) + client = _mock_client(mocker) + client.toolgroups.register.return_value = None + + body = MCPServerRegistrationRequest( + name="full-mcp-server", + url="https://mcp.example.com/api", + provider_id="custom-provider", + authorization_headers={"Authorization": "client"}, + headers=["x-rh-identity"], + timeout=30, + ) + + result = await mcp_servers.register_mcp_server_handler( + request=mocker.Mock(), body=body, auth=MOCK_AUTH + ) + + assert result.name == "full-mcp-server" + assert result.provider_id == "custom-provider" + + +@pytest.mark.asyncio +async def test_list_mcp_servers_empty( + mocker: MockerFixture, + mock_configuration: Configuration, +) -> None: + """Test listing servers returns static servers.""" + _make_app_config(mocker, mock_configuration) + + result = await mcp_servers.list_mcp_servers_handler( + request=mocker.Mock(), auth=MOCK_AUTH + ) + + assert isinstance(result, MCPServerListResponse) + assert len(result.servers) == 1 + assert result.servers[0].name == "static-mcp" + assert result.servers[0].source == "config" + + +@pytest.mark.asyncio +async def test_list_mcp_servers_with_dynamic( + mocker: MockerFixture, + mock_configuration: Configuration, +) -> None: + """Test listing shows both static and dynamic servers.""" + _make_app_config(mocker, mock_configuration) + client = _mock_client(mocker) + client.toolgroups.register.return_value = None + + body = MCPServerRegistrationRequest( + name="dynamic-server", + url="http://localhost:9999/mcp", + ) + await mcp_servers.register_mcp_server_handler( + request=mocker.Mock(), body=body, auth=MOCK_AUTH + ) + + result = await mcp_servers.list_mcp_servers_handler( + request=mocker.Mock(), auth=MOCK_AUTH + ) + + assert len(result.servers) == 2 + sources = {s.name: s.source for s in result.servers} + assert sources["static-mcp"] == "config" + assert sources["dynamic-server"] == "api" + + +@pytest.mark.asyncio +async def test_delete_dynamic_mcp_server_success( + mocker: MockerFixture, + mock_configuration: Configuration, +) -> None: + """Test successful deletion of a dynamically registered server.""" + app_config = _make_app_config(mocker, mock_configuration) + client = _mock_client(mocker) + client.toolgroups.register.return_value = None + client.toolgroups.unregister.return_value = None + + body = MCPServerRegistrationRequest( + name="to-delete", + url="http://localhost:7777/mcp", + ) + await mcp_servers.register_mcp_server_handler( + request=mocker.Mock(), body=body, auth=MOCK_AUTH + ) + assert app_config.is_dynamic_mcp_server("to-delete") + + result = await mcp_servers.delete_mcp_server_handler( + request=mocker.Mock(), name="to-delete", auth=MOCK_AUTH + ) + + assert isinstance(result, MCPServerDeleteResponse) + assert result.name == "to-delete" + assert "unregistered successfully" in result.message + + assert not app_config.is_dynamic_mcp_server("to-delete") + assert not any(s.name == "to-delete" for s in app_config.mcp_servers) + + client.toolgroups.unregister.assert_called_once_with(toolgroup_id="to-delete") + + +@pytest.mark.asyncio +async def test_delete_static_mcp_server_forbidden( + mocker: MockerFixture, + mock_configuration: Configuration, +) -> None: + """Test that deleting a statically configured server is forbidden.""" + _make_app_config(mocker, mock_configuration) + _mock_client(mocker) + + with pytest.raises(HTTPException) as exc_info: + await mcp_servers.delete_mcp_server_handler( + request=mocker.Mock(), name="static-mcp", auth=MOCK_AUTH + ) + assert exc_info.value.status_code == 403 + + +@pytest.mark.asyncio +async def test_delete_nonexistent_mcp_server( + mocker: MockerFixture, + mock_configuration: Configuration, +) -> None: + """Test that deleting a non-existent server returns 404.""" + _make_app_config(mocker, mock_configuration) + _mock_client(mocker) + + with pytest.raises(HTTPException) as exc_info: + await mcp_servers.delete_mcp_server_handler( + request=mocker.Mock(), name="no-such-server", auth=MOCK_AUTH + ) + assert exc_info.value.status_code == 404 + + +@pytest.mark.asyncio +async def test_delete_mcp_server_llama_stack_failure( + mocker: MockerFixture, + mock_configuration: Configuration, +) -> None: + """Test deletion handles Llama Stack connection failure gracefully.""" + _make_app_config(mocker, mock_configuration) + client = _mock_client(mocker) + client.toolgroups.register.return_value = None + client.toolgroups.unregister.side_effect = APIConnectionError(request=mocker.Mock()) + + body = MCPServerRegistrationRequest( + name="to-delete-fail", + url="http://localhost:7777/mcp", + ) + await mcp_servers.register_mcp_server_handler( + request=mocker.Mock(), body=body, auth=MOCK_AUTH + ) + + with pytest.raises(HTTPException) as exc_info: + await mcp_servers.delete_mcp_server_handler( + request=mocker.Mock(), name="to-delete-fail", auth=MOCK_AUTH + ) + assert exc_info.value.status_code == 503 + + +def test_mcp_server_registration_request_validation() -> None: + """Test request model validation.""" + with pytest.raises(Exception): + MCPServerRegistrationRequest( + name="test", + url="ftp://invalid-scheme", + ) + + with pytest.raises(Exception): + MCPServerRegistrationRequest( + name="", + url="http://valid.url", + ) + + req = MCPServerRegistrationRequest( + name="valid-server", + url="http://localhost:8080/mcp", + ) + assert req.provider_id == "model-context-protocol" + + +def test_mcp_server_registration_auth_keywords() -> None: + """Test that all three supported auth keywords are accepted.""" + for keyword in ("client", "kubernetes", "oauth"): + req = MCPServerRegistrationRequest( + name=f"server-{keyword}", + url="http://localhost:8080/mcp", + authorization_headers={"Authorization": keyword}, + ) + assert req.authorization_headers is not None + assert req.authorization_headers["Authorization"] == keyword + + +def test_mcp_server_registration_rejects_file_path() -> None: + """Test that file-path based auth headers are rejected for dynamic registration.""" + with pytest.raises(Exception, match="unsupported value"): + MCPServerRegistrationRequest( + name="bad-server", + url="http://localhost:8080/mcp", + authorization_headers={"Authorization": "/var/secrets/token"}, + ) + + +def test_mcp_server_registration_rejects_arbitrary_value() -> None: + """Test that arbitrary auth header values are rejected.""" + with pytest.raises(Exception, match="unsupported value"): + MCPServerRegistrationRequest( + name="bad-server", + url="http://localhost:8080/mcp", + authorization_headers={"Authorization": "Bearer my-static-token"}, + ) + + +@pytest.mark.asyncio +async def test_register_and_delete_roundtrip( + mocker: MockerFixture, + mock_configuration: Configuration, +) -> None: + """Test full register -> list -> delete -> list cycle.""" + _make_app_config(mocker, mock_configuration) + client = _mock_client(mocker) + client.toolgroups.register.return_value = None + client.toolgroups.unregister.return_value = None + + body = MCPServerRegistrationRequest( + name="roundtrip-server", + url="http://localhost:5555/mcp", + ) + await mcp_servers.register_mcp_server_handler( + request=mocker.Mock(), body=body, auth=MOCK_AUTH + ) + + list_result = await mcp_servers.list_mcp_servers_handler( + request=mocker.Mock(), auth=MOCK_AUTH + ) + assert len(list_result.servers) == 2 + + await mcp_servers.delete_mcp_server_handler( + request=mocker.Mock(), name="roundtrip-server", auth=MOCK_AUTH + ) + + list_result = await mcp_servers.list_mcp_servers_handler( + request=mocker.Mock(), auth=MOCK_AUTH + ) + assert len(list_result.servers) == 1 + assert list_result.servers[0].name == "static-mcp" From 8f2a6ecf7962e08c889957d59b8c79c9c463e7e4 Mon Sep 17 00:00:00 2001 From: Maysun J Faisal Date: Mon, 16 Mar 2026 17:57:08 -0400 Subject: [PATCH 2/2] Update tests for MCP Server API Assisted-by: Claude Opus 4.6 Generated-by: Cursor Signed-off-by: Maysun J Faisal --- tests/unit/app/test_routers.py | 7 +++++-- tests/unit/models/responses/test_error_responses.py | 3 ++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/unit/app/test_routers.py b/tests/unit/app/test_routers.py index 668943a50..218d9a6de 100644 --- a/tests/unit/app/test_routers.py +++ b/tests/unit/app/test_routers.py @@ -25,6 +25,7 @@ metrics, tools, mcp_auth, + mcp_servers, rlsapi_v1, a2a, query, @@ -109,12 +110,13 @@ def test_include_routers() -> None: include_routers(app) # are all routers added? - assert len(app.routers) == 21 + assert len(app.routers) == 22 assert root.router in app.get_routers() assert info.router in app.get_routers() assert models.router in app.get_routers() assert tools.router in app.get_routers() assert mcp_auth.router in app.get_routers() + assert mcp_servers.router in app.get_routers() assert shields.router in app.get_routers() assert providers.router in app.get_routers() assert query.router in app.get_routers() @@ -147,12 +149,13 @@ def test_check_prefixes() -> None: include_routers(app) # are all routers added? - assert len(app.routers) == 21 + assert len(app.routers) == 22 assert app.get_router_prefix(root.router) == "" assert app.get_router_prefix(info.router) == "/v1" assert app.get_router_prefix(models.router) == "/v1" assert app.get_router_prefix(tools.router) == "/v1" assert app.get_router_prefix(mcp_auth.router) == "/v1" + assert app.get_router_prefix(mcp_servers.router) == "/v1" assert app.get_router_prefix(shields.router) == "/v1" assert app.get_router_prefix(providers.router) == "/v1" assert app.get_router_prefix(rags.router) == "/v1" diff --git a/tests/unit/models/responses/test_error_responses.py b/tests/unit/models/responses/test_error_responses.py index 3ecf441b8..f1a257e93 100644 --- a/tests/unit/models/responses/test_error_responses.py +++ b/tests/unit/models/responses/test_error_responses.py @@ -472,7 +472,7 @@ def test_openapi_response(self) -> None: # Verify example count matches schema examples count assert len(examples) == expected_count - assert expected_count == 5 + assert expected_count == 6 # Verify all labeled examples are present assert "conversation" in examples @@ -480,6 +480,7 @@ def test_openapi_response(self) -> None: assert "model" in examples assert "rag" in examples assert "streaming request" in examples + assert "mcp server" in examples # Verify example structure for one example conversation_example = examples["conversation"]