diff --git a/gns3server/api/routes/controller/images.py b/gns3server/api/routes/controller/images.py index 9a43152ed..26b9a2ed2 100644 --- a/gns3server/api/routes/controller/images.py +++ b/gns3server/api/routes/controller/images.py @@ -154,6 +154,17 @@ async def upload_image( if os.path.commonprefix([base_images_directory, full_path]) != base_images_directory: raise ControllerForbiddenError(f"Cannot write image, '{image_path}' is forbidden") + # If the client sends X-MD5-Checksum, check for a duplicate before consuming the upload stream + checksum_header = request.headers.get("X-MD5-Checksum") + if checksum_header: + check_dir = os.path.dirname(full_path) if image_dir else None + duplicate = await images_repo.get_image_by_checksum(checksum_header, check_dir) + if duplicate: + location = f" in '{check_dir}'" if check_dir else "" + raise ControllerError( + f"Image '{duplicate.filename}' with the same checksum already exists{location}" + ) + try: allow_raw_image = Config.instance().settings.Server.allow_raw_images image = await write_image(image_path, full_path, request.stream(), images_repo, allow_raw_image=allow_raw_image) diff --git a/gns3server/api/routes/controller/nodes.py b/gns3server/api/routes/controller/nodes.py index d38384470..860e8fe92 100644 --- a/gns3server/api/routes/controller/nodes.py +++ b/gns3server/api/routes/controller/nodes.py @@ -105,6 +105,16 @@ async def dep_node(node_id: UUID, project: Project = Depends(dep_project)) -> No return node +def _check_node_type(node: Node, *required_types: str) -> None: + """ + Raise ControllerBadRequestError if node is not one of the required types. + """ + + if node.node_type not in required_types: + type_str = "/".join(required_types) + raise ControllerBadRequestError(f"This endpoint is only supported on a {type_str} node") + + @router.post( "", status_code=status.HTTP_201_CREATED, @@ -407,8 +417,7 @@ async def auto_idlepc(node: Node = Depends(dep_node)) -> dict: Required privilege: Node.Audit """ - if node.node_type != "dynamips": - raise ControllerBadRequestError("Auto Idle-PC is only supported on a Dynamips node") + _check_node_type(node, "dynamips") return await node.dynamips_auto_idlepc() @@ -420,8 +429,7 @@ async def idlepc_proposals(node: Node = Depends(dep_node)) -> List[str]: Required privilege: Node.Audit """ - if node.node_type != "dynamips": - raise ControllerBadRequestError("Idle-PC proposals is only supported on a Dynamips node") + _check_node_type(node, "dynamips") return await node.dynamips_idlepc_proposals() @@ -441,8 +449,7 @@ async def create_disk_image( Required privilege: Node.Allocate """ - if node.node_type != "qemu": - raise ControllerBadRequestError("Creating a disk image is only supported on a Qemu node") + _check_node_type(node, "qemu") await node.post(f"/disk_image/{disk_name}", data=disk_data.model_dump(exclude_unset=True)) @@ -462,8 +469,7 @@ async def update_disk_image( Required privilege: Node.Allocate """ - if node.node_type != "qemu": - raise ControllerBadRequestError("Updating a disk image is only supported on a Qemu node") + _check_node_type(node, "qemu") await node.put(f"/disk_image/{disk_name}", data=disk_data.model_dump(exclude_unset=True)) @@ -482,8 +488,7 @@ async def delete_disk_image( Required privilege: Node.Allocate """ - if node.node_type != "qemu": - raise ControllerBadRequestError("Deleting a disk image is only supported on a Qemu node") + _check_node_type(node, "qemu") await node.delete(f"/disk_image/{disk_name}") diff --git a/gns3server/api/server.py b/gns3server/api/server.py index 145394681..888822978 100644 --- a/gns3server/api/server.py +++ b/gns3server/api/server.py @@ -207,7 +207,7 @@ async def sqlalchemy_error_handler(request: Request, exc: SQLAlchemyError): async def validation_exception_handler(request: Request, exc: RequestValidationError): log.error(f"Request validation error in {request.url.path} ({request.method}): {exc}") return JSONResponse( - status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + status_code=status.HTTP_422_UNPROCESSABLE_CONTENT, content={"message": str(exc)} ) diff --git a/gns3server/controller/export_project.py b/gns3server/controller/export_project.py index 29226fc7a..11edd3fa2 100644 --- a/gns3server/controller/export_project.py +++ b/gns3server/controller/export_project.py @@ -289,7 +289,7 @@ def _export_local_image(image, zstream): # Some modules don't have images continue - directory = os.path.split(images_directory)[-1:][0] + directory = os.path.basename(images_directory) if os.path.exists(image): path = image else: @@ -309,7 +309,7 @@ async def _export_remote_images(project, compute_id, image_type, image, project_ log.debug(f"Downloading image '{image}' from compute '{compute_id}'") try: - compute = [compute for compute in project.computes if compute.id == compute_id][0] + compute = next(c for c in project.computes if c.id == compute_id) except IndexError: raise ControllerNotFoundError(f"Cannot export image from '{compute_id}' compute. Compute doesn't exist.") diff --git a/gns3server/controller/node.py b/gns3server/controller/node.py index 8622b23b7..37dcff665 100644 --- a/gns3server/controller/node.py +++ b/gns3server/controller/node.py @@ -27,6 +27,7 @@ from .controller_error import ( ComputeError, ComputeConflictError ) +from .node_types import BUILTIN_NODE_TYPES from .ports.port_factory import PortFactory, StandardPortFactory, DynamipsPortFactory from ..utils.images import images_directories from ..utils import macaddress_to_int, int_to_macaddress @@ -452,8 +453,7 @@ class Node: if ( prop == "name" and self.status == "started" - and self._node_type - not in ("cloud", "nat", "ethernet_switch", "ethernet_hub", "frame_relay_switch", "atm_switch") + and self._node_type not in BUILTIN_NODE_TYPES ): raise ControllerError("Sorry, it is not possible to rename a node that is already powered on") setattr(self, prop, kwargs[prop]) @@ -541,35 +541,22 @@ class Node: if self._console: # console is optional for builtin nodes data["console"] = self._console - if self._console_type and self._node_type not in ( - "cloud", - "nat", - "ethernet_hub", - "frame_relay_switch", - "atm_switch", - ): + if self._console_type and self._node_type not in (BUILTIN_NODE_TYPES - {"ethernet_switch"}): # console_type is not supported by all builtin nodes excepting Ethernet switch data["console_type"] = self._console_type if self._aux: # aux is optional for builtin nodes data["aux"] = self._aux - if self._aux_type and self._node_type not in ( - "cloud", - "nat", - "ethernet_switch", - "ethernet_hub", - "frame_relay_switch", - "atm_switch", - ): + if self._aux_type and self._node_type not in BUILTIN_NODE_TYPES: # aux_type is not supported by all builtin nodes data["aux_type"] = self._aux_type if self.custom_adapters: data["custom_adapters"] = self.custom_adapters # None properties are not be sent because it can mean the emulator doesn't support it - for key in list(data.keys()): - if data[key] is None or data[key] == {} or key in self.CONTROLLER_ONLY_PROPERTIES: - del data[key] + for key, value in list(data.items()): + if value is None or value == {} or key in self.CONTROLLER_ONLY_PROPERTIES: + del value return data diff --git a/gns3server/controller/node_types.py b/gns3server/controller/node_types.py new file mode 100644 index 000000000..4c4683c62 --- /dev/null +++ b/gns3server/controller/node_types.py @@ -0,0 +1,31 @@ +#!/usr/bin/env python +# +# Copyright (C) 2026 GNS3 Technologies Inc. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +""" +Shared node type constants used across the controller. +""" + +# Node types that are always running (builtin/virtual switch nodes). +# These nodes do not require explicit start/stop and have limited feature support. +BUILTIN_NODE_TYPES = frozenset({ + "cloud", + "nat", + "ethernet_switch", + "ethernet_hub", + "frame_relay_switch", + "atm_switch", +}) \ No newline at end of file diff --git a/gns3server/controller/topology.py b/gns3server/controller/topology.py index 22dfc56ce..150283c39 100644 --- a/gns3server/controller/topology.py +++ b/gns3server/controller/topology.py @@ -175,7 +175,8 @@ def load_topology(path): # Version GNS3 2.2 dev (for project created with 2.2dev). # Appliance ID has been replaced by Template ID if topo["revision"] == 9: - for node in topo.get("topology", {}).get("nodes", []): + nodes = topo.get("topology", {}).get("nodes", []) + for node in nodes: if "appliance_id" in node: node["template_id"] = node["appliance_id"] del node["appliance_id"] @@ -218,7 +219,8 @@ def _convert_2_2_0(topo, topo_path): topo["revision"] = 10 - for node in topo.get("topology", {}).get("nodes", []): + nodes = topo.get("topology", {}).get("nodes", []) + for node in nodes: if "properties" in node: if node["node_type"] in ("qemu", "docker") and not is_rfc1123_hostname_valid(node["name"]): new_name = to_rfc1123_hostname(node["name"]) @@ -246,7 +248,8 @@ def _convert_2_1_0(topo, topo_path): # to avoid overlapping grids topo["drawing_grid_size"] = topo["grid_size"] - for node in topo.get("topology", {}).get("nodes", []): + nodes = topo.get("topology", {}).get("nodes", []) + for node in nodes: # make sure console_type is not None but "none" string if "console_type" in node and node["console_type"] is None: node["console_type"] = "none" @@ -272,7 +275,8 @@ def _convert_2_0_0(topo, topo_path): """ topo["revision"] = 8 - for node in topo.get("topology", {}).get("nodes", []): + nodes = topo.get("topology", {}).get("nodes", []) + for node in nodes: if "properties" in node: if node["node_type"] == "vpcs": if "startup_script_path" in node["properties"]: @@ -301,7 +305,8 @@ def _convert_2_0_0_beta_2(topo, topo_path): topo_dir = os.path.dirname(topo_path) topo["revision"] = 7 - for node in topo.get("topology", {}).get("nodes", []): + nodes = topo.get("topology", {}).get("nodes", []) + for node in nodes: if node["node_type"] == "dynamips": node_id = node["node_id"] dynamips_id = node["properties"]["dynamips_id"] @@ -328,7 +333,8 @@ def _convert_2_0_0_alpha(topo, topo_path): * No more option for VMware / VirtualBox remote console (always use telnet) """ topo["revision"] = 6 - for node in topo.get("topology", {}).get("nodes", []): + nodes = topo.get("topology", {}).get("nodes", []) + for node in nodes: if node.get("console_type") == "serial": node["console_type"] = "telnet" if node["node_type"] in ("vmware", "virtualbox"): diff --git a/gns3server/controller/udp_link.py b/gns3server/controller/udp_link.py index 4788f61ab..9b9172f38 100644 --- a/gns3server/controller/udp_link.py +++ b/gns3server/controller/udp_link.py @@ -18,6 +18,7 @@ from .controller_error import ControllerError, ControllerNotFoundError from .link import Link +from .node_types import BUILTIN_NODE_TYPES class UDPLink(Link): @@ -32,6 +33,18 @@ class UDPLink(Link): Use for the debug exports """ return self._link_data + + def _get_node_filters(self, node1, node2): + """ + Determine which node gets the active filters applied. + + :returns: Tuple of (node1_filters, node2_filters) + """ + filter_node = self._get_filter_node() + return ( + self.get_active_filters() if filter_node == node1 else {}, + self.get_active_filters() if filter_node == node2 else {}, + ) async def create(self): """ @@ -57,13 +70,7 @@ class UDPLink(Link): response = await node2.compute.post(f"/projects/{self._project.id}/ports/udp") self._node2_port = response.json["udp_port"] - node1_filters = {} - node2_filters = {} - filter_node = self._get_filter_node() - if filter_node == node1: - node1_filters = self.get_active_filters() - elif filter_node == node2: - node2_filters = self.get_active_filters() + node1_filters, node2_filters = self._get_node_filters(node1, node2) # Create the tunnel on both side self._link_data.append( @@ -108,13 +115,7 @@ class UDPLink(Link): node1 = self._nodes[0]["node"] node2 = self._nodes[1]["node"] - node1_filters = {} - node2_filters = {} - filter_node = self._get_filter_node() - if filter_node == node1: - node1_filters = self.get_active_filters() - elif filter_node == node2: - node2_filters = self.get_active_filters() + node1_filters, node2_filters = self._get_node_filters(node1, node2) adapter_number1 = self._nodes[0]["adapter_number"] port_number1 = self._nodes[0]["port_number"] @@ -213,18 +214,16 @@ class UDPLink(Link): :returns: Node where the capture should run """ - ALWAYS_RUNNING_NODES_TYPE = ("cloud", "nat", "ethernet_switch", "ethernet_hub", "frame_relay_switch", "atm_switch") - for node in self._nodes: if ( node["node"].compute.id == "local" - and node["node"].node_type in ALWAYS_RUNNING_NODES_TYPE + and node["node"].node_type in BUILTIN_NODE_TYPES and node["node"].status == "started" ): return node for node in self._nodes: - if node["node"].node_type in ALWAYS_RUNNING_NODES_TYPE and node["node"].status == "started": + if node["node"].node_type in BUILTIN_NODE_TYPES and node["node"].status == "started": return node for node in self._nodes: diff --git a/tests/api/routes/compute/test_ethernet_switch_nodes.py b/tests/api/routes/compute/test_ethernet_switch_nodes.py index f0dc400b6..ab395522c 100644 --- a/tests/api/routes/compute/test_ethernet_switch_nodes.py +++ b/tests/api/routes/compute/test_ethernet_switch_nodes.py @@ -297,7 +297,7 @@ class TestEthernetSwitchNodesRoutes: node_id=ethernet_switch["node_id"]), json=port_params ) - assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY + assert response.status_code == status.HTTP_422_UNPROCESSABLE_CONTENT async def test_ethernet_switch_delete( diff --git a/tests/api/routes/controller/test_templates.py b/tests/api/routes/controller/test_templates.py index ba7055755..3ae7e9383 100644 --- a/tests/api/routes/controller/test_templates.py +++ b/tests/api/routes/controller/test_templates.py @@ -133,7 +133,7 @@ class TestTemplateRoutes: "template_type": "invalid_template_type"} response = await client.post(app.url_path_for("create_template"), json=params) - assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY + assert response.status_code == status.HTTP_422_UNPROCESSABLE_CONTENT async def test_template_update(self, app: FastAPI, client: AsyncClient) -> None: diff --git a/tests/api/routes/controller/test_users.py b/tests/api/routes/controller/test_users.py index e068e3ca2..4041f63bb 100644 --- a/tests/api/routes/controller/test_users.py +++ b/tests/api/routes/controller/test_users.py @@ -75,10 +75,10 @@ class TestUserRoutes: ( ("email", "user2@email.com", status.HTTP_400_BAD_REQUEST), ("username", "user2", status.HTTP_400_BAD_REQUEST), - ("email", "invalid_email@one@two.io", status.HTTP_422_UNPROCESSABLE_ENTITY), - ("password", "short", status.HTTP_422_UNPROCESSABLE_ENTITY), - ("username", "user2@#$%^<>", status.HTTP_422_UNPROCESSABLE_ENTITY), - ("username", "ab", status.HTTP_422_UNPROCESSABLE_ENTITY), + ("email", "invalid_email@one@two.io", status.HTTP_422_UNPROCESSABLE_CONTENT), + ("password", "short", status.HTTP_422_UNPROCESSABLE_CONTENT), + ("username", "user2@#$%^<>", status.HTTP_422_UNPROCESSABLE_CONTENT), + ("username", "ab", status.HTTP_422_UNPROCESSABLE_CONTENT), ) ) async def test_user_registration_fails_when_credentials_are_taken( @@ -101,10 +101,10 @@ class TestUserRoutes: ("email", "user@email.com", status.HTTP_200_OK), ("email", "user@email.com", status.HTTP_400_BAD_REQUEST), ("username", "user2", status.HTTP_400_BAD_REQUEST), - ("email", "invalid_email@one@two.io", status.HTTP_422_UNPROCESSABLE_ENTITY), - ("password", "short", status.HTTP_422_UNPROCESSABLE_ENTITY), - ("username", "user2@#$%^<>", status.HTTP_422_UNPROCESSABLE_ENTITY), - ("username", "ab", status.HTTP_422_UNPROCESSABLE_ENTITY), + ("email", "invalid_email@one@two.io", status.HTTP_422_UNPROCESSABLE_CONTENT), + ("password", "short", status.HTTP_422_UNPROCESSABLE_CONTENT), + ("username", "user2@#$%^<>", status.HTTP_422_UNPROCESSABLE_CONTENT), + ("username", "ab", status.HTTP_422_UNPROCESSABLE_CONTENT), ("full_name", "John Doe", status.HTTP_200_OK), ("password", "password123", status.HTTP_200_OK), ("is_active", True, status.HTTP_200_OK), @@ -259,7 +259,7 @@ class TestUserLogin: ( ("wrong_username", "user1_password", status.HTTP_401_UNAUTHORIZED), ("user1", "wrong_password", status.HTTP_401_UNAUTHORIZED), - ("user1", None, status.HTTP_422_UNPROCESSABLE_ENTITY), + ("user1", None, status.HTTP_422_UNPROCESSABLE_CONTENT), ), ) async def test_user_with_wrong_creds_doesnt_receive_token( diff --git a/tests/api/routes/controller/test_version.py b/tests/api/routes/controller/test_version.py index 1e229dddf..95824067a 100644 --- a/tests/api/routes/controller/test_version.py +++ b/tests/api/routes/controller/test_version.py @@ -62,4 +62,4 @@ class TestVersionRoutes: params = "BOUM" response = await client.post(app.url_path_for("check_version"), json=params) - assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY + assert response.status_code == status.HTTP_422_UNPROCESSABLE_CONTENT