From 888afdccbdac119355a28d0d7de7c41eccfd8f25 Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Tue, 25 Aug 2026 23:39:38 +0800 Subject: [PATCH] fix: return the created template from appliance install POST /appliances/{id}/install replied 204 with an empty body, so the MCP appliance_install tool crashed with 'Expecting value: line 1 column 1' while the template had actually been created. The route now returns the created template (201, response_model=schemas.Template), _create_template propagates it, and the MCP handler parses the body defensively so an empty reply degrades to a plain success message. --- gns3server/agent/mcp/__init__.py | 2 +- gns3server/agent/mcp/appliances.py | 12 +++++++++-- .../api/routes/controller/appliances.py | 9 ++++---- gns3server/controller/appliance_manager.py | 5 +++-- tests/agent/mcp/test_handlers.py | 21 ++++++++++++++++++- .../api/routes/controller/test_appliances.py | 7 +++++-- 6 files changed, 44 insertions(+), 12 deletions(-) diff --git a/gns3server/agent/mcp/__init__.py b/gns3server/agent/mcp/__init__.py index 6d65ff11c..dac6031a4 100644 --- a/gns3server/agent/mcp/__init__.py +++ b/gns3server/agent/mcp/__init__.py @@ -1376,7 +1376,7 @@ async def appliance_install( appliance_id: Annotated[str, Field(description="UUID of the appliance to install")], version: Annotated[str | None, Field(description="Version to install (e.g. '2.7.0.356'). Required if the appliance has multiple versions. Use appliance_get to see available versions.")] = None, ) -> list[dict[str, Any]]: - """Create a template from a GNS3 appliance definition. + """Create a template from a GNS3 appliance definition and return the created template. NOTE: This does NOT download images. Images must be placed in the GNS3 images directory (e.g. ~/GNS3/images/) beforehand. diff --git a/gns3server/agent/mcp/appliances.py b/gns3server/agent/mcp/appliances.py index dd2be1735..ab867fe57 100644 --- a/gns3server/agent/mcp/appliances.py +++ b/gns3server/agent/mcp/appliances.py @@ -85,5 +85,13 @@ def install_appliance_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) version = params.get("version") if version: request_params["version"] = version - result = conn.http_call("post", url, params=request_params).json() - return {"message": f"Appliance {appliance_id} installation requested", "result": result} + response = conn.http_call("post", url, params=request_params) + result = {"message": f"Appliance {appliance_id} installed"} + if response.content: + # the install endpoint returns the created template (201); tolerate an + # empty body in case an older server still replies with 204 + template = response.json() + result["template"] = { + k: template[k] for k in ("template_id", "name", "version", "template_type") if k in template + } + return result diff --git a/gns3server/api/routes/controller/appliances.py b/gns3server/api/routes/controller/appliances.py index 1ca32d8e9..142818f66 100644 --- a/gns3server/api/routes/controller/appliances.py +++ b/gns3server/api/routes/controller/appliances.py @@ -122,7 +122,8 @@ def add_appliance_version(appliance_id: UUID, appliance_version: Union[schemas.A @router.post( "/{appliance_id}/install", - status_code=status.HTTP_204_NO_CONTENT, + response_model=schemas.Template, + status_code=status.HTTP_201_CREATED, dependencies=[Depends(has_privilege("Appliance.Allocate"))] ) async def install_appliance( @@ -132,15 +133,15 @@ async def install_appliance( templates_repo: TemplatesRepository = Depends(get_repository(TemplatesRepository)), current_user: schemas.User = Depends(get_current_active_user), rbac_repo: RbacRepository = Depends(get_repository(RbacRepository)) -) -> None: +) -> schemas.Template: """ - Install an appliance. + Install an appliance and return the created template. Required privilege: Appliance.Allocate """ controller = Controller.instance() - await controller.appliance_manager.install_appliance( + return await controller.appliance_manager.install_appliance( appliance_id, version, images_repo, diff --git a/gns3server/controller/appliance_manager.py b/gns3server/controller/appliance_manager.py index 25b0c366e..7ee36a4d4 100644 --- a/gns3server/controller/appliance_manager.py +++ b/gns3server/controller/appliance_manager.py @@ -204,9 +204,9 @@ class ApplianceManager: else: raise ControllerError(f"Could not find '{appliance_file}'") - async def _create_template(self, template_data, templates_repo, rbac_repo, current_user): + async def _create_template(self, template_data, templates_repo, rbac_repo, current_user) -> dict: """ - Create a new template + Create a new template and return it as a dict. """ try: @@ -217,6 +217,7 @@ class ApplianceManager: #template_id = template.get("template_id") #await rbac_repo.add_permission_to_user_with_path(current_user.user_id, f"/templates/{template_id}/*") log.info(f"Template '{template.get('name')}' has been created") + return template async def _appliance_to_template(self, appliance: Appliance, version: str = None) -> dict: """ diff --git a/tests/agent/mcp/test_handlers.py b/tests/agent/mcp/test_handlers.py index c5cbbbfd5..91f1b9f72 100644 --- a/tests/agent/mcp/test_handlers.py +++ b/tests/agent/mcp/test_handlers.py @@ -3,6 +3,8 @@ MCP handler unit tests with mocked Gns3Connector. Tests that handlers correctly transform tool parameters into HTTP calls. """ +import json + import pytest from unittest.mock import MagicMock, patch @@ -12,6 +14,7 @@ def _mock_conn(json_result=None): conn = MagicMock() conn.base_url = "http://192.168.1.3:3080/v3" conn.http_call.return_value.json.return_value = json_result or {"status": "ok"} + conn.http_call.return_value.content = b"{}" # non-empty body by default return conn @@ -449,7 +452,7 @@ class TestAppliance: def test_install_with_version(self, ctx): from gns3server.agent.mcp.appliances import install_appliance_handler with patch(f"{BASE}.{self.mod}._get_connector") as m: - conn = _mock_conn({"status": "installed"}) + conn = _mock_conn({"template_id": "t1", "name": "FRR", "version": "8.2.2", "template_type": "docker"}) m.return_value = conn result = install_appliance_handler({ "appliance_id": "a1", "version": "2.7.0.356", @@ -458,6 +461,22 @@ class TestAppliance: "post", "http://192.168.1.3:3080/v3/appliances/a1/install", params={"version": "2.7.0.356"}, ) + assert result["template"] == { + "template_id": "t1", "name": "FRR", "version": "8.2.2", "template_type": "docker", + } + + def test_install_empty_body(self, ctx): + # a 204-style empty response must not blow up with a JSON decode error + # (the template is still created server-side) + from gns3server.agent.mcp.appliances import install_appliance_handler + with patch(f"{BASE}.{self.mod}._get_connector") as m: + conn = _mock_conn() + conn.http_call.return_value.content = b"" + conn.http_call.return_value.json.side_effect = json.JSONDecodeError("Expecting value", "", 0) + m.return_value = conn + result = install_appliance_handler({"appliance_id": "a1"}, ctx) + assert "template" not in result + assert result["message"] == "Appliance a1 installed" def test_install_missing_id(self, ctx): from gns3server.agent.mcp.appliances import install_appliance_handler diff --git a/tests/api/routes/controller/test_appliances.py b/tests/api/routes/controller/test_appliances.py index d3c1cc632..356fe2b08 100644 --- a/tests/api/routes/controller/test_appliances.py +++ b/tests/api/routes/controller/test_appliances.py @@ -53,7 +53,8 @@ class TestApplianceRoutes: appliance_id = "fc520ae2-a4e5-48c3-9a13-516bb2e94668" # Alpine Linux appliance response = await client.post(app.url_path_for("install_appliance", appliance_id=appliance_id)) - assert response.status_code == status.HTTP_204_NO_CONTENT + assert response.status_code == status.HTTP_201_CREATED + assert response.json()["name"] == "Alpine Linux" async def test_docker_appliance_install_with_version(self, app: FastAPI, client: AsyncClient) -> None: @@ -68,7 +69,9 @@ class TestApplianceRoutes: appliance_id = "1cfdf900-7c30-4cb7-8f03-3f61d2581633" # Empty VM appliance params = {"version": "8G"} response = await client.post(app.url_path_for("install_appliance", appliance_id=appliance_id), params=params) - assert response.status_code == status.HTTP_204_NO_CONTENT + assert response.status_code == status.HTTP_201_CREATED + assert response.json()["name"] == "Empty VM" + assert response.json()["version"] == "8G" async def test_qemu_appliance_install_without_version(self, app: FastAPI, client: AsyncClient, images_dir: str) -> None: