diff --git a/gns3server/api/routes/controller/links.py b/gns3server/api/routes/controller/links.py index 79a471787..83c0899fd 100644 --- a/gns3server/api/routes/controller/links.py +++ b/gns3server/api/routes/controller/links.py @@ -33,8 +33,13 @@ from gns3server import schemas from gns3server.agent.web_wireshark.manager import WebWiresharkManager from gns3server.api.openapi import PCAP_MEDIA_TYPE, binary_response from gns3server.controller import Controller -from gns3server.controller.controller_error import ControllerError, ControllerNotFoundError +from gns3server.controller.controller_error import ( + ControllerError, + ControllerNotFoundError, + controller_error_status_code, +) from gns3server.controller.link import _UNSET, Link +from gns3server.controller.project import Project from gns3server.db.repositories.rbac import RbacRepository from gns3server.utils.http_client import HTTPClient from gns3server.utils.port_allocator import link_id_to_port @@ -123,7 +128,7 @@ async def get_links(project_id: UUID) -> List[schemas.Link]: }, dependencies=[Depends(has_privilege("Link.Allocate"))], ) -async def create_link(project_id: UUID, link_create: schemas.LinkCreate) -> schemas.Link: +async def create_link(project_id: UUID, link_create: schemas.LinkCreate) -> dict: """ Create a new link. @@ -131,17 +136,22 @@ async def create_link(project_id: UUID, link_create: schemas.LinkCreate) -> sche """ project = await Controller.instance().get_loaded_project(str(project_id)) + return await _create_link(project, link_create) + + +async def _create_link(project: Project, link_create: schemas.LinkCreate) -> dict: + link = await project.add_link() link_data = jsonable_encoder(link_create, exclude_unset=True) - if "filters" in link_data: - await link.update_filters(link_data["filters"]) - if "link_style" in link_data: - await link.update_link_style(link_data["link_style"]) - if "suspend" in link_data: - await link.update_suspend(link_data["suspend"]) - if "show_filters_icon" in link_data: - await link.update_show_filters_icon(link_data["show_filters_icon"]) try: + if "filters" in link_data: + await link.update_filters(link_data["filters"]) + if "link_style" in link_data: + await link.update_link_style(link_data["link_style"]) + if "suspend" in link_data: + await link.update_suspend(link_data["suspend"]) + if "show_filters_icon" in link_data: + await link.update_show_filters_icon(link_data["show_filters_icon"]) endpoints = link_data["nodes"] attached_port = None for index, endpoint in enumerate(endpoints): @@ -159,6 +169,40 @@ async def create_link(project_id: UUID, link_create: schemas.LinkCreate) -> sche return link.asdict() +@router.post( + "/batch", + response_model=List[schemas.LinkBatchResult], + responses={ + 404: {"model": schemas.ErrorMessage, "description": "Could not find project"}, + }, + dependencies=[Depends(has_privilege("Link.Allocate"))], +) +async def create_links(project_id: UUID, links_create: List[schemas.LinkCreate]) -> List[schemas.LinkBatchResult]: + """ + Create several links in the order they are listed. + + The response lists one result per link in the same order. A link that fails to be created + does not stop the following ones. + + Required privilege: Link.Allocate + """ + + project = await Controller.instance().get_loaded_project(str(project_id)) + results = [] + for link_create in links_create: + try: + link = await _create_link(project, link_create) + results.append(schemas.LinkBatchResult(status_code=status.HTTP_201_CREATED, link=link)) + except ControllerError as e: + log.error(f"Could not create link in batch: {e}") + results.append( + schemas.LinkBatchResult( + status_code=controller_error_status_code(e), error=schemas.ErrorMessage(message=str(e)) + ) + ) + return results + + @router.get( "/{link_id}/available_filters", response_model=List[schemas.LinkFilterDefinition], diff --git a/gns3server/api/routes/controller/nodes.py b/gns3server/api/routes/controller/nodes.py index 4000f0da6..04dbd332a 100644 --- a/gns3server/api/routes/controller/nodes.py +++ b/gns3server/api/routes/controller/nodes.py @@ -37,8 +37,10 @@ from gns3server.config import Config from gns3server.controller import Controller from gns3server.controller.controller_error import ( ControllerBadRequestError, + ControllerError, ControllerForbiddenError, ControllerTimeoutError, + controller_error_status_code, ) from gns3server.controller.node import Node from gns3server.controller.project import Project @@ -141,13 +143,20 @@ def _check_node_type(node: Node, *required_types: str) -> None: }, dependencies=[Depends(has_privilege("Node.Allocate"))], ) -async def create_node(node_create: schemas.NodeCreate, project: Project = Depends(dep_project)) -> schemas.Node: +async def create_node(node_create: schemas.NodeCreate, project: Project = Depends(dep_project)) -> dict: """ Create a new node. + Set strict_names to get a 409 error instead of an automatic rename when the node name is already used. + Required privilege: Node.Allocate """ + return await _create_node(project, node_create) + + +async def _create_node(project: Project, node_create: schemas.NodeCreate) -> dict: + controller = Controller.instance() compute = controller.get_compute(str(node_create.compute_id)) node_data = jsonable_encoder(node_create, exclude_unset=True) @@ -155,6 +164,41 @@ async def create_node(node_create: schemas.NodeCreate, project: Project = Depend return node.asdict() +@router.post( + "/batch", + response_model=List[schemas.NodeBatchResult], + responses={ + 404: {"model": schemas.ErrorMessage, "description": "Could not find project"}, + }, + dependencies=[Depends(has_privilege("Node.Allocate"))], +) +async def create_nodes( + nodes_create: List[schemas.NodeCreate], project: Project = Depends(dep_project) +) -> List[schemas.NodeBatchResult]: + """ + Create several nodes in the order they are listed. + + The response lists one result per node in the same order. A node that fails to be created + does not stop the following ones. + + Required privilege: Node.Allocate + """ + + results = [] + for node_create in nodes_create: + try: + node = await _create_node(project, node_create) + results.append(schemas.NodeBatchResult(status_code=status.HTTP_201_CREATED, node=node)) + except ControllerError as e: + log.error(f"Could not create node {node_create.name} in batch: {e}") + results.append( + schemas.NodeBatchResult( + status_code=controller_error_status_code(e), error=schemas.ErrorMessage(message=str(e)) + ) + ) + return results + + @router.get( "", response_model=List[schemas.Node], diff --git a/gns3server/controller/controller_error.py b/gns3server/controller/controller_error.py index ac6a747aa..da661eb0d 100644 --- a/gns3server/controller/controller_error.py +++ b/gns3server/controller/controller_error.py @@ -86,3 +86,21 @@ class ComputeConflictError(ComputeError): def response(self): return self._response + + +def controller_error_status_code(error: ControllerError) -> int: + """ + HTTP status code returned by the API for a controller error. + """ + + if isinstance(error, ControllerTimeoutError): + return 408 + if isinstance(error, ControllerUnauthorizedError): + return 401 + if isinstance(error, ControllerForbiddenError): + return 403 + if isinstance(error, ControllerNotFoundError): + return 404 + if isinstance(error, ControllerBadRequestError): + return 400 + return 409 diff --git a/gns3server/controller/node.py b/gns3server/controller/node.py index 2f37d7332..ab20af194 100644 --- a/gns3server/controller/node.py +++ b/gns3server/controller/node.py @@ -101,7 +101,9 @@ class Node: "default_password", ] - def __init__(self, project, compute, name, node_id=None, node_type=None, template_id=None, **kwargs): + def __init__( + self, project, compute, name, node_id=None, node_type=None, template_id=None, strict_names=False, **kwargs + ): """ :param project: Project of the node :param compute: Compute where the server will run @@ -109,6 +111,7 @@ class Node: :param node_id: UUID of the node (integer) :param node_type: Type of emulator :param template_id: Template ID used to create this node + :param strict_names: Fail instead of renaming the node when the name is already used :param kwargs: Node properties """ @@ -126,7 +129,7 @@ class Node: self._label = None self._links = set() self._name = None - self.name = name + self._name = project.update_node_name(self, name, strict=strict_names) self._console = None self._console_type = None self._aux = None diff --git a/gns3server/controller/project.py b/gns3server/controller/project.py index 09e750040..a403f8227 100644 --- a/gns3server/controller/project.py +++ b/gns3server/controller/project.py @@ -557,17 +557,20 @@ class Project: if name in self._allocated_node_names: self._allocated_node_names.remove(name) - def update_allocated_node_name(self, base_name): + def update_allocated_node_name(self, base_name, strict=False): """ Updates a node name or generate a new if no node name is available. :param base_name: new node base name + :param strict: raise an error instead of renaming when the name is already taken """ if base_name is None: return None base_name = re.sub(r"[ ]", "", base_name) # remove spaces in node name + if strict and base_name in self._allocated_node_names: + raise ControllerError(f"Node name {base_name} is already used") if base_name in self._allocated_node_names: base_name = re.sub(r"[0-9]+$", "{0}", base_name) @@ -595,11 +598,11 @@ class Project: return name raise ControllerError("A node name could not be allocated (node limit reached?)") - def update_node_name(self, node, new_name): + def update_node_name(self, node, new_name, strict=False): if new_name and node.name != new_name: self.remove_allocated_node_name(node.name) - return self.update_allocated_node_name(new_name) + return self.update_allocated_node_name(new_name, strict=strict) return new_name @open_required @@ -630,26 +633,32 @@ class Project: node = await self.add_node(compute, name, node_id, node_type=node_type, **template) return node - async def _create_node(self, compute, name, node_id, node_type=None, allow_missing_image=False, **kwargs): + async def _create_node( + self, compute, name, node_id, node_type=None, allow_missing_image=False, strict_names=False, **kwargs + ): - node = Node(self, compute, name, node_id=node_id, node_type=node_type, **kwargs) - # Hold the lock across the check + POST + register so that concurrent - # node creations on the same compute don't all race past the check and - # each POST /projects (the compute-side sync handler then instantiated - # the Project N times). Once one creation registers the compute, the - # rest see it in the set and return immediately. - async with self._create_node_lock: - if compute not in self._project_created_on_compute: - if compute.id == "local": - data = {"name": self._name, "project_id": self._id, "path": self._path} - else: - data = {"name": self._name, "project_id": self._id} - if self._variables: - data["variables"] = self._variables - await compute.post("/projects", data=data) - self._project_created_on_compute.add(compute) + node = Node(self, compute, name, node_id=node_id, node_type=node_type, strict_names=strict_names, **kwargs) + try: + # Hold the lock across the check + POST + register so that concurrent + # node creations on the same compute don't all race past the check and + # each POST /projects (the compute-side sync handler then instantiated + # the Project N times). Once one creation registers the compute, the + # rest see it in the set and return immediately. + async with self._create_node_lock: + if compute not in self._project_created_on_compute: + if compute.id == "local": + data = {"name": self._name, "project_id": self._id, "path": self._path} + else: + data = {"name": self._name, "project_id": self._id} + if self._variables: + data["variables"] = self._variables + await compute.post("/projects", data=data) + self._project_created_on_compute.add(compute) - await node.create(allow_missing_image=allow_missing_image) + await node.create(allow_missing_image=allow_missing_image) + except BaseException: + self.remove_allocated_node_name(node.name) + raise self._nodes[node.id] = node return node diff --git a/gns3server/schemas/__init__.py b/gns3server/schemas/__init__.py index 5338f5b57..64949b0b4 100644 --- a/gns3server/schemas/__init__.py +++ b/gns3server/schemas/__init__.py @@ -35,6 +35,7 @@ from .controller.images import Image, ImageSyncJob, ImageSyncRequest, ImageType, from .controller.links import ( EthernetPortInfo, Link, + LinkBatchResult, LinkCapture, LinkCreate, LinkFilterDefinition, @@ -48,7 +49,7 @@ from .controller.links import ( MarkerUpdate, UDPPortInfo, ) -from .controller.nodes import Node, NodeCapture, NodeCreate, NodeDuplicate, NodeUpdate +from .controller.nodes import Node, NodeBatchResult, NodeCapture, NodeCreate, NodeDuplicate, NodeUpdate from .controller.projects import ( NodeFile, Project, @@ -249,6 +250,7 @@ __all__ = [ "LLMModelConfigUpdate", "LLMModelConfigWithSource", "Link", + "LinkBatchResult", "LinkCapture", "LinkCreate", "LinkFilterDefinition", @@ -268,6 +270,7 @@ __all__ = [ "NetmikoDeviceType", "NetmikoDeviceTypeList", "Node", + "NodeBatchResult", "NodeCapture", "NodeCreate", "NodeDuplicate", diff --git a/gns3server/schemas/controller/links.py b/gns3server/schemas/controller/links.py index f74635b8c..7f4b1fc22 100644 --- a/gns3server/schemas/controller/links.py +++ b/gns3server/schemas/controller/links.py @@ -20,6 +20,7 @@ from uuid import UUID, uuid4 from pydantic import BaseModel, ConfigDict, Discriminator, Field, field_validator, model_validator +from ..common import ErrorMessage from ..update import PartialUpdateModel from .labels import Label @@ -195,6 +196,16 @@ class Link(LinkBase): ) +class LinkBatchResult(BaseModel): + """ + Outcome of one link of a batch creation request. + """ + + status_code: int = Field(..., description="HTTP status code the same request would have returned on its own") + link: Optional[Link] = Field(None, description="The created link, set when status_code is 201") + error: Optional[ErrorMessage] = Field(None, description="The error, set when the link could not be created") + + class UDPPortInfo(BaseModel): """ UDP port information. diff --git a/gns3server/schemas/controller/nodes.py b/gns3server/schemas/controller/nodes.py index 6d731fc91..ff1d90d2c 100644 --- a/gns3server/schemas/controller/nodes.py +++ b/gns3server/schemas/controller/nodes.py @@ -20,7 +20,7 @@ from uuid import UUID, uuid4 from pydantic import BaseModel, Field -from ..common import ConsoleType, CustomAdapter, NodeStatus +from ..common import ConsoleType, CustomAdapter, ErrorMessage, NodeStatus from ..update import PartialUpdateModel from .labels import Label @@ -168,6 +168,10 @@ class NodeCreate(NodeBase): name: str node_type: NodeType node_id: UUID = Field(default_factory=uuid4) + strict_names: bool = Field( + False, + description="Return a 409 error instead of renaming the node when the requested name is already used", + ) class NodeUpdate(PartialUpdateModel, NodeBase): @@ -205,6 +209,16 @@ class Node(NodeBase): ) +class NodeBatchResult(BaseModel): + """ + Outcome of one node of a batch creation request. + """ + + status_code: int = Field(..., description="HTTP status code the same request would have returned on its own") + node: Optional[Node] = Field(None, description="The created node, set when status_code is 201") + error: Optional[ErrorMessage] = Field(None, description="The error, set when the node could not be created") + + class NodeDuplicate(BaseModel): """ Data to duplicate a node. diff --git a/tests/api/routes/controller/test_links.py b/tests/api/routes/controller/test_links.py index d7daded0e..51618715f 100644 --- a/tests/api/routes/controller/test_links.py +++ b/tests/api/routes/controller/test_links.py @@ -81,6 +81,45 @@ class TestLinkRoutes: assert len(project.links) == 1 assert list(project.links.values())[0].filters == filters + async def test_create_links_batch( + self, app: FastAPI, client: AsyncClient, project: Project, nodes: Tuple[Node, Node] + ) -> None: + + node1, node2 = nodes + valid = { + "nodes": [ + {"node_id": node1.id, "adapter_number": 0, "port_number": 3}, + {"node_id": node2.id, "adapter_number": 2, "port_number": 4}, + ] + } + same_node = { + "nodes": [ + {"node_id": node1.id, "adapter_number": 0, "port_number": 3}, + {"node_id": node1.id, "adapter_number": 0, "port_number": 3}, + ] + } + unknown_node = { + "nodes": [ + {"node_id": str(uuid.uuid4()), "adapter_number": 0, "port_number": 3}, + {"node_id": node2.id, "adapter_number": 2, "port_number": 4}, + ] + } + + with asyncio_patch("gns3server.controller.udp_link.UDPLink.create"): + response = await client.post( + app.url_path_for("create_links", project_id=project.id), json=[same_node, unknown_node, valid] + ) + + assert response.status_code == status.HTTP_200_OK + results = response.json() + assert [r["status_code"] for r in results] == [409, 404, 201] + assert results[0]["link"] is None + assert results[0]["error"]["message"] + assert results[1]["error"]["message"] + assert results[2]["error"] is None + assert results[2]["link"]["link_id"] in project.links + assert len(project.links) == 1 + async def test_create_link_failure( self, app: FastAPI, client: AsyncClient, compute: Compute, project: Project ) -> None: diff --git a/tests/api/routes/controller/test_nodes.py b/tests/api/routes/controller/test_nodes.py index 458bfb309..be05b9cf3 100644 --- a/tests/api/routes/controller/test_nodes.py +++ b/tests/api/routes/controller/test_nodes.py @@ -34,7 +34,7 @@ from pydantic import SecretStr from gns3server.api.routes.controller.nodes import vnc_console, ws_console from gns3server.config import Config from gns3server.controller.compute import Compute -from gns3server.controller.controller_error import ComputeError +from gns3server.controller.controller_error import ComputeError, ControllerError from gns3server.controller.node import Node from gns3server.controller.project import Project from gns3server.services import auth_service @@ -73,6 +73,103 @@ class TestNodeRoutes: assert response.json()["name"] == "test" assert "name" not in response.json()["properties"] + async def test_create_node_taken_name_is_renamed( + self, app: FastAPI, client: AsyncClient, project: Project, compute: Compute + ) -> None: + + response = MagicMock() + response.json = {"console": 2048} + compute.post = AsyncioMagicMock(return_value=response) + payload = {"name": "R1", "node_type": "vpcs", "compute_id": "example.com"} + + first = await client.post(app.url_path_for("create_node", project_id=project.id), json=payload) + second = await client.post(app.url_path_for("create_node", project_id=project.id), json=payload) + + assert first.json()["name"] == "R1" + assert second.status_code == status.HTTP_201_CREATED + assert second.json()["name"] == "R2" + + async def test_create_node_strict_names( + self, app: FastAPI, client: AsyncClient, project: Project, compute: Compute + ) -> None: + + response = MagicMock() + response.json = {"console": 2048} + compute.post = AsyncioMagicMock(return_value=response) + payload = {"name": "R1", "node_type": "vpcs", "compute_id": "example.com", "strict_names": True} + + first = await client.post(app.url_path_for("create_node", project_id=project.id), json=payload) + second = await client.post(app.url_path_for("create_node", project_id=project.id), json=payload) + + assert first.status_code == status.HTTP_201_CREATED + assert second.status_code == status.HTTP_409_CONFLICT + assert "R1" in second.json()["message"] + assert len(project.nodes) == 1 + + async def test_create_nodes_batch( + self, app: FastAPI, client: AsyncClient, project: Project, compute: Compute + ) -> None: + + response = MagicMock() + response.json = {"console": 2048} + compute.post = AsyncioMagicMock(return_value=response) + nodes = [{"name": "R{0}", "node_type": "vpcs", "compute_id": "example.com"} for _ in range(3)] + + response = await client.post(app.url_path_for("create_nodes", project_id=project.id), json=nodes) + + assert response.status_code == status.HTTP_200_OK + results = response.json() + assert [r["status_code"] for r in results] == [201, 201, 201] + assert [r["node"]["name"] for r in results] == ["R1", "R2", "R3"] + assert all(r["error"] is None for r in results) + assert len(project.nodes) == 3 + + async def test_create_nodes_batch_item_failures_keep_order( + self, app: FastAPI, client: AsyncClient, project: Project, compute: Compute + ) -> None: + + response = MagicMock() + response.json = {"console": 2048} + compute.post = AsyncioMagicMock(return_value=response) + nodes = [ + {"name": "R1", "node_type": "vpcs", "compute_id": "example.com", "strict_names": True}, + {"name": "R1", "node_type": "vpcs", "compute_id": "example.com", "strict_names": True}, + {"name": "R1", "node_type": "vpcs", "compute_id": "unknown"}, + {"name": "R1", "node_type": "vpcs", "compute_id": "example.com"}, + ] + + response = await client.post(app.url_path_for("create_nodes", project_id=project.id), json=nodes) + + assert response.status_code == status.HTTP_200_OK + results = response.json() + assert [r["status_code"] for r in results] == [201, 409, 404, 201] + assert results[0]["node"]["name"] == "R1" + assert results[1]["node"] is None + assert "R1" in results[1]["error"]["message"] + assert results[2]["error"]["message"] + assert results[3]["node"]["name"] == "R2" + assert len(project.nodes) == 2 + + async def test_create_nodes_batch_failed_node_releases_name( + self, app: FastAPI, client: AsyncClient, project: Project, compute: Compute + ) -> None: + + compute.post = AsyncioMagicMock(side_effect=ControllerError("compute failure")) + nodes = [{"name": "R1", "node_type": "vpcs", "compute_id": "example.com", "strict_names": True}] + + response = await client.post(app.url_path_for("create_nodes", project_id=project.id), json=nodes) + + assert response.json()[0]["status_code"] == status.HTTP_409_CONFLICT + assert len(project.nodes) == 0 + + response_ok = MagicMock() + response_ok.json = {"console": 2048} + compute.post = AsyncioMagicMock(return_value=response_ok) + response = await client.post(app.url_path_for("create_nodes", project_id=project.id), json=nodes) + + assert response.json()[0]["status_code"] == status.HTTP_201_CREATED + assert response.json()[0]["node"]["name"] == "R1" + async def test_list_node(self, app: FastAPI, client: AsyncClient, project: Project, compute: Compute) -> None: response = MagicMock() diff --git a/tests/controller/test_project.py b/tests/controller/test_project.py index 92d51cd82..b41c7cb53 100644 --- a/tests/controller/test_project.py +++ b/tests/controller/test_project.py @@ -1118,6 +1118,46 @@ async def test_node_name(project): assert node.name == "R3" +@pytest.mark.asyncio +async def test_node_name_strict(project): + + compute = MagicMock() + compute.id = "local" + response = MagicMock() + response.json = {"console": 2048} + compute.post = AsyncioMagicMock(return_value=response) + + node = await project.add_node(compute, "R1", None, node_type="vpcs", strict_names=True) + assert node.name == "R1" + with pytest.raises(ControllerError): + await project.add_node(compute, "R1", None, node_type="vpcs", strict_names=True) + assert len(project.nodes) == 1 + + node = await project.add_node(compute, "R1", None, node_type="vpcs", strict_names=False) + assert node.name == "R2" + node = await project.add_node(compute, "T{0}", None, node_type="vpcs", strict_names=True) + assert node.name == "T1" + node = await project.add_node(compute, "T{0}", None, node_type="vpcs", strict_names=True) + assert node.name == "T2" + + +@pytest.mark.asyncio +async def test_node_name_released_when_creation_fails(project): + + compute = MagicMock() + compute.id = "local" + compute.post = AsyncioMagicMock(side_effect=ControllerError("failure")) + + with pytest.raises(ControllerError): + await project.add_node(compute, "R1", None, node_type="vpcs") + + response = MagicMock() + response.json = {"console": 2048} + compute.post = AsyncioMagicMock(return_value=response) + node = await project.add_node(compute, "R1", None, node_type="vpcs", strict_names=True) + assert node.name == "R1" + + @pytest.mark.asyncio async def test_duplicate_node(project):