diff --git a/gns3server/api/routes/controller/images.py b/gns3server/api/routes/controller/images.py
index 70c49b098..fe27c20d8 100644
--- a/gns3server/api/routes/controller/images.py
+++ b/gns3server/api/routes/controller/images.py
@@ -197,11 +197,16 @@ async def prune_images(
"""
Prune images not attached to any template.
+ Images referenced by a node in any project (opened or closed) are kept.
+
Required privilege: Image.Allocate
"""
skip_images = get_builtin_disks()
- await images_repo.prune_images(skip_images)
+ # a single pass over all projects' node properties protects every
+ # referenced file name at once
+ referenced_filenames = Controller.instance().collect_referenced_image_filenames()
+ await images_repo.prune_images(list(skip_images) + list(referenced_filenames))
@router.post(
@@ -309,6 +314,10 @@ async def delete_image(
template_names = ", ".join([template.name for template in templates])
raise ControllerError(f"Image '{image_path}' is used by one or more templates: {template_names}")
+ project_names = Controller.instance().find_projects_using_image(image.filename)
+ if project_names:
+ raise ControllerError(f"Image '{image_path}' is used by one or more projects: {', '.join(project_names)}")
+
try:
os.remove(image.path)
except OSError:
diff --git a/gns3server/api/routes/controller/templates.py b/gns3server/api/routes/controller/templates.py
index 05fa6482c..6d59cd913 100644
--- a/gns3server/api/routes/controller/templates.py
+++ b/gns3server/api/routes/controller/templates.py
@@ -35,6 +35,7 @@ from gns3server.db.repositories.templates import TemplatesRepository
from gns3server.services.templates import TemplatesService
from gns3server.db.repositories.rbac import RbacRepository
from gns3server.db.repositories.images import ImagesRepository
+from gns3server.controller import Controller
from gns3server.controller.controller_error import ControllerError, ControllerBadRequestError
from gns3server.utils.images import get_builtin_disks
@@ -135,28 +136,49 @@ async def delete_template(
Required privilege: Template.Allocate
"""
+ controller = Controller.instance()
images = await templates_repo.get_template_images(template_id)
- await TemplatesService(templates_repo).delete_template(template_id)
- await rbac_repo.delete_all_ace_starting_with_path(f"/templates/{template_id}")
+
+ # Run every usage check before mutating anything, so a refused deletion
+ # cannot leave the template gone while its images survive.
+ images_to_prune = []
if prune_images and images:
skip_images = get_builtin_disks()
+ # collected lazily: a single pass over all projects' node properties
+ referenced_filenames = None
for image in images:
if image.filename in skip_images:
continue
- templates = await images_repo.get_image_templates(image.image_id)
- if templates:
- template_names = ", ".join([template.name for template in templates])
+ # the template being deleted is still in the database at this
+ # point, exclude it from the other-templates check
+ other_templates = [
+ template for template in await images_repo.get_image_templates(image.image_id)
+ if str(template.template_id) != str(template_id)
+ ]
+ if other_templates:
+ template_names = ", ".join([template.name for template in other_templates])
raise ControllerError(f"Image '{image.path}' is used by one or more templates: {template_names}")
- try:
- os.remove(image.path)
- except OSError:
- log.warning(f"Could not delete image file {image.path}")
+ if referenced_filenames is None:
+ referenced_filenames = controller.collect_referenced_image_filenames()
+ if image.filename in referenced_filenames:
+ project_names = controller.find_projects_using_image(image.filename)
+ raise ControllerError(f"Image '{image.path}' is used by one or more projects: {', '.join(project_names)}")
+ images_to_prune.append(image)
- print(f"Deleting image '{image.path}'")
- success = await images_repo.delete_image(image.path)
- if not success:
- raise ControllerError(f"Image '{image.path}' could not removed from the database")
+ await TemplatesService(templates_repo).delete_template(template_id)
+ await rbac_repo.delete_all_ace_starting_with_path(f"/templates/{template_id}")
+
+ for image in images_to_prune:
+ try:
+ os.remove(image.path)
+ except OSError:
+ log.warning(f"Could not delete image file {image.path}")
+
+ log.info(f"Deleting image '{image.path}'")
+ success = await images_repo.delete_image(image.path)
+ if not success:
+ raise ControllerError(f"Image '{image.path}' could not removed from the database")
@router.get(
diff --git a/gns3server/controller/__init__.py b/gns3server/controller/__init__.py
index 9593ee1cc..99416e202 100644
--- a/gns3server/controller/__init__.py
+++ b/gns3server/controller/__init__.py
@@ -38,6 +38,7 @@ from ..utils.images import default_images_directory
from ..utils.asyncio import wait_run_in_executor
from .project import Project
+from .node import Node
from .appliance import Appliance
from .appliance_manager import ApplianceManager
from .compute import Compute, ComputeError
@@ -91,6 +92,35 @@ class _ProjectsDirectoryEventHandler(FileSystemEventHandler):
return
+def _iter_string_values(value):
+ """
+ Yield every string contained in value, recursing into dicts and lists.
+ """
+
+ if isinstance(value, str):
+ yield value
+ elif isinstance(value, dict):
+ for child in value.values():
+ yield from _iter_string_values(child)
+ elif isinstance(value, (list, tuple, set)):
+ for child in value:
+ yield from _iter_string_values(child)
+
+
+def _image_referenced(node_properties, image_filename: str) -> bool:
+ """
+ Whether a node's properties reference an image by file name.
+
+ Image references live under type-specific property keys
+ (hda_disk_image, hda_disk_image_backing_file, image, initrd, path...)
+ and are stored either as bare file names or as absolute paths
+ depending on topology age, so every string value is matched on its
+ base name instead of walking a fixed key list.
+ """
+
+ return any(os.path.basename(string) == image_filename for string in _iter_string_values(node_properties))
+
+
class Controller:
"""
The controller is responsible to manage one or more computes.
@@ -811,6 +841,84 @@ class Controller:
raise ControllerError("A project name could not be allocated (node limit reached?)")
return new_name
+ def _iter_project_nodes(self):
+ """
+ Iterate over (project, node) for every known project, opened or closed.
+
+ Opened projects yield controller Node objects; closed projects yield
+ the raw node dicts read from their .gns3 file. A project whose
+ topology cannot be read is skipped — it must not block the usage
+ checks that rely on this iterator.
+ """
+
+ for project in self._projects.values():
+ try:
+ nodes = project.nodes.values()
+ except ControllerError:
+ continue
+ for node in nodes:
+ yield project, node
+
+ def find_projects_using_template(self, template_id) -> list:
+ """
+ Return the names of the projects with at least one node created
+ from this template.
+
+ Used to forbid template deletion while any project still
+ references it.
+ """
+
+ template_id = str(template_id)
+ project_names = []
+ for project, node in self._iter_project_nodes():
+ if isinstance(node, Node):
+ node_template_id = node.template_id
+ else:
+ node_template_id = node.get("template_id")
+ if node_template_id and str(node_template_id) == template_id:
+ project_names.append(project.name)
+ break
+ return project_names
+
+ def find_projects_using_image(self, image_filename: str) -> list:
+ """
+ Return the names of the projects with at least one node whose
+ properties reference this image file.
+
+ Used to forbid image deletion while any project still uses it.
+ """
+
+ project_names = []
+ for project, node in self._iter_project_nodes():
+ if isinstance(node, Node):
+ node_properties = node.properties
+ else:
+ node_properties = node.get("properties") or {}
+ if _image_referenced(node_properties, image_filename):
+ project_names.append(project.name)
+ break
+ return project_names
+
+ def collect_referenced_image_filenames(self) -> set:
+ """
+ Return every file name referenced by a node property across all
+ projects (opened or closed).
+
+ The set deliberately over-approximates (any property string counts):
+ it protects images from pruning, so a false positive only keeps a
+ file alive. Used as a single-pass skip list for image pruning.
+ """
+
+ filenames = set()
+ for _project, node in self._iter_project_nodes():
+ if isinstance(node, Node):
+ node_properties = node.properties
+ else:
+ node_properties = node.get("properties") or {}
+ for string in _iter_string_values(node_properties):
+ filenames.add(os.path.basename(string))
+ return filenames
+
@property
def projects(self):
"""
diff --git a/gns3server/controller/node.py b/gns3server/controller/node.py
index cd474912f..8ea491b3c 100644
--- a/gns3server/controller/node.py
+++ b/gns3server/controller/node.py
@@ -162,6 +162,10 @@ class Node:
def status(self):
return self._status
+ @property
+ def template_id(self):
+ return self._template_id
+
@property
def name(self):
return self._name
diff --git a/gns3server/services/templates.py b/gns3server/services/templates.py
index c079608dd..66b793566 100644
--- a/gns3server/services/templates.py
+++ b/gns3server/services/templates.py
@@ -341,6 +341,15 @@ class TemplatesService:
if self.get_builtin_template(template_id):
raise ControllerForbiddenError(f"Template '{template_id}' cannot be deleted because it is built-in")
+
+ template = await self.get_template(template_id)
+ project_names = self._controller.find_projects_using_template(template_id)
+ if project_names:
+ raise ControllerError(
+ f"Template '{template['name']}' cannot be deleted because it is used by "
+ f"one or more projects: {', '.join(project_names)}"
+ )
+
if await self._templates_repo.delete_template(template_id):
self._controller.notification.controller_emit("template.deleted", {"template_id": str(template_id)})
else:
diff --git a/tests/api/routes/controller/test_images.py b/tests/api/routes/controller/test_images.py
index 8a7a4d68c..ea0b2f587 100644
--- a/tests/api/routes/controller/test_images.py
+++ b/tests/api/routes/controller/test_images.py
@@ -16,6 +16,8 @@
# along with this program. If not, see .
import os
+import json
+import uuid
import pytest
import hashlib
@@ -282,6 +284,99 @@ class TestImageRoutes:
images_in_db = await images_repo.get_images()
assert len(images_in_db) == 0
+ @staticmethod
+ async def _add_closed_project_with_node(controller: Controller, name: str, node: dict):
+ """
+ Register a closed project whose .gns3 contains a single node,
+ mirroring how the controller picks up existing projects from disk.
+ """
+
+ project_dir = os.path.join(controller.projects_directory(), name)
+ os.makedirs(project_dir)
+ project_id = str(uuid.uuid4())
+ topology = {
+ "name": name,
+ "project_id": project_id,
+ "topology": {"computes": [], "links": [], "drawings": [], "nodes": [node]},
+ }
+ with open(os.path.join(project_dir, f"{name}.gns3"), "w+") as f:
+ json.dump(topology, f)
+ return await controller.add_project(
+ project_id=project_id, name=name, path=project_dir,
+ filename=f"{name}.gns3", status="closed",
+ )
+
+ async def test_image_delete_used_by_project(
+ self,
+ app: FastAPI,
+ client: AsyncClient,
+ controller: Controller,
+ db_session: AsyncSession,
+ tmpdir: str,
+ ) -> None:
+ """
+ An image referenced by a node in a project must not be deleted,
+ even when the project is closed and no template uses the image.
+ """
+
+ image_path = os.path.join(tmpdir, "used.qcow2")
+ with open(image_path, "wb+") as f:
+ f.write(b'\x42\x42\x42\x42')
+
+ images_repo = ImagesRepository(db_session)
+ await images_repo.add_image("used.qcow2", "qemu", 42, image_path, "e342eb86c1229b6c154367a5476969b5", "md5")
+
+ guarded_project = await self._add_closed_project_with_node(
+ controller, "Guarded",
+ {"node_id": str(uuid.uuid4()), "node_type": "qemu", "compute_id": "local",
+ "name": "n1", "properties": {"hda_disk_image_backing_file": "used.qcow2"}},
+ )
+
+ response = await client.delete(app.url_path_for("delete_image", image_path="used.qcow2"))
+ assert response.status_code == status.HTTP_409_CONFLICT
+ assert "Guarded" in response.json()["message"]
+ assert os.path.exists(image_path)
+
+ # once no project uses it anymore the deletion goes through
+ controller.remove_project(guarded_project)
+ response = await client.delete(app.url_path_for("delete_image", image_path="used.qcow2"))
+ assert response.status_code == status.HTTP_204_NO_CONTENT
+ assert not os.path.exists(image_path)
+
+ async def test_prune_images_keeps_project_referenced(
+ self,
+ app: FastAPI,
+ client: AsyncClient,
+ controller: Controller,
+ db_session: AsyncSession,
+ tmpdir: str,
+ ) -> None:
+ """
+ Pruning must keep images referenced by a project node while
+ removing the unreferenced ones.
+ """
+
+ images_repo = ImagesRepository(db_session)
+ for filename in ("used.qcow2", "unused.qcow2"):
+ image_path = os.path.join(tmpdir, filename)
+ with open(image_path, "wb+") as f:
+ f.write(b'\x42\x42\x42\x42')
+ await images_repo.add_image(filename, "qemu", 42, image_path, "e342eb86c1229b6c154367a5476969b5", "md5")
+
+ await self._add_closed_project_with_node(
+ controller, "Guarded",
+ {"node_id": str(uuid.uuid4()), "node_type": "qemu", "compute_id": "local",
+ "name": "n1", "properties": {"hda_disk_image_backing_file": "used.qcow2"}},
+ )
+
+ response = await client.delete(app.url_path_for("prune_images"))
+ assert response.status_code == status.HTTP_204_NO_CONTENT
+
+ assert await images_repo.get_image(os.path.join(tmpdir, "used.qcow2")) is not None
+ assert os.path.exists(os.path.join(tmpdir, "used.qcow2"))
+ assert await images_repo.get_image(os.path.join(tmpdir, "unused.qcow2")) is None
+ assert not os.path.exists(os.path.join(tmpdir, "unused.qcow2"))
+
async def test_image_upload_create_appliance(
self, app: FastAPI,
client: AsyncClient,
diff --git a/tests/api/routes/controller/test_templates.py b/tests/api/routes/controller/test_templates.py
index 75cae55d5..a1bf507e9 100644
--- a/tests/api/routes/controller/test_templates.py
+++ b/tests/api/routes/controller/test_templates.py
@@ -16,6 +16,7 @@
# along with this program. If not, see .
import os
+import json
import shutil
import pytest
@@ -270,6 +271,128 @@ class TestTemplateRoutes:
images = await images_repo.get_images()
assert len(images) == 0
+ @staticmethod
+ async def _add_closed_project_with_node(controller: Controller, name: str, node: dict):
+ """
+ Register a closed project whose .gns3 contains a single node,
+ mirroring how the controller picks up existing projects from disk.
+ """
+
+ project_dir = os.path.join(controller.projects_directory(), name)
+ os.makedirs(project_dir)
+ project_id = str(uuid.uuid4())
+ topology = {
+ "name": name,
+ "project_id": project_id,
+ "topology": {"computes": [], "links": [], "drawings": [], "nodes": [node]},
+ }
+ with open(os.path.join(project_dir, f"{name}.gns3"), "w+") as f:
+ json.dump(topology, f)
+ # an explicit project_id marks this as an existing on-disk project
+ # (same path load_project takes at startup)
+ return await controller.add_project(
+ project_id=project_id, name=name, path=project_dir,
+ filename=f"{name}.gns3", status="closed",
+ )
+
+ async def test_template_delete_used_by_project(
+ self,
+ app: FastAPI,
+ client: AsyncClient,
+ controller: Controller,
+ ) -> None:
+ """
+ A template referenced by a node in a project must not be deleted,
+ even when the project is closed.
+ """
+
+ template_id = str(uuid.uuid4())
+ params = {"template_id": template_id,
+ "name": "VPCS_GUARDED",
+ "compute_id": "local",
+ "template_type": "vpcs"}
+
+ response = await client.post(app.url_path_for("create_template"), json=params)
+ assert response.status_code == status.HTTP_201_CREATED
+
+ guarded_project = await self._add_closed_project_with_node(
+ controller, "Guarded",
+ {"node_id": str(uuid.uuid4()), "node_type": "vpcs", "compute_id": "local",
+ "name": "n1", "template_id": template_id, "properties": {}},
+ )
+
+ response = await client.delete(app.url_path_for("delete_template", template_id=template_id))
+ assert response.status_code == status.HTTP_409_CONFLICT
+ assert "Guarded" in response.json()["message"]
+
+ # the template survived the refused deletion
+ response = await client.get(app.url_path_for("get_template", template_id=template_id))
+ assert response.status_code == status.HTTP_200_OK
+
+ # once no project uses it anymore the deletion goes through
+ controller.remove_project(guarded_project)
+ response = await client.delete(app.url_path_for("delete_template", template_id=template_id))
+ assert response.status_code == status.HTTP_204_NO_CONTENT
+
+ async def test_template_delete_with_prune_images_used_by_project(
+ self,
+ app: FastAPI,
+ client: AsyncClient,
+ controller: Controller,
+ db_session: AsyncSession,
+ tmpdir: str,
+ ) -> None:
+ """
+ Pruning an image still referenced by a project node must be refused
+ before any mutation: the template and the image file both survive.
+ """
+
+ image_path = os.path.join(tmpdir, "used.qcow2")
+ with open(image_path, "wb+") as f:
+ f.write(b'\x42\x42\x42\x42')
+
+ images_repo = ImagesRepository(db_session)
+ await images_repo.add_image("used.qcow2", "qemu", 42, image_path, "e342eb86c1229b6c154367a5476969b5", "md5")
+
+ template_id = str(uuid.uuid4())
+ params = {"template_id": template_id,
+ "name": "QEMU_GUARDED",
+ "compute_id": "local",
+ "hda_disk_image": "used.qcow2",
+ "template_type": "qemu"}
+
+ response = await client.post(app.url_path_for("create_template"), json=params)
+ assert response.status_code == status.HTTP_201_CREATED
+
+ # the node references the image but not the template: the template
+ # check passes, the image check must still refuse the prune
+ guarded_project = await self._add_closed_project_with_node(
+ controller, "Guarded",
+ {"node_id": str(uuid.uuid4()), "node_type": "qemu", "compute_id": "local",
+ "name": "n1", "properties": {"hda_disk_image_backing_file": "used.qcow2"}},
+ )
+
+ response = await client.delete(
+ app.url_path_for("delete_template", template_id=template_id),
+ params={"prune_images": True}
+ )
+ assert response.status_code == status.HTTP_409_CONFLICT
+ assert "Guarded" in response.json()["message"]
+
+ # neither the template nor the image file was touched
+ response = await client.get(app.url_path_for("get_template", template_id=template_id))
+ assert response.status_code == status.HTTP_200_OK
+ assert os.path.exists(image_path)
+
+ controller.remove_project(guarded_project)
+ response = await client.delete(
+ app.url_path_for("delete_template", template_id=template_id),
+ params={"prune_images": True}
+ )
+ assert response.status_code == status.HTTP_204_NO_CONTENT
+ assert not os.path.exists(image_path)
+ assert await images_repo.get_image(image_path) is None
+
# async def test_create_node_from_template(self, controller_api, controller, project):
#
# id = str(uuid.uuid4())
diff --git a/tests/controller/test_controller.py b/tests/controller/test_controller.py
index 4f158c9b3..ca689edf5 100644
--- a/tests/controller/test_controller.py
+++ b/tests/controller/test_controller.py
@@ -534,3 +534,74 @@ async def test_autoidlepc(controller):
await controller.autoidlepc("local", "c7200", "test.bin", 512)
assert node_mock.dynamips_auto_idlepc.called
assert len(controller.projects) == 0
+
+
+@pytest.mark.asyncio
+async def test_find_projects_using_template_and_images(controller):
+ """
+ The template/image usage checks must see nodes of opened projects
+ (in-memory Node objects) as well as nodes of closed projects (raw
+ node dicts read back from the .gns3 file).
+ """
+
+ compute = MagicMock()
+ response = MagicMock()
+ response.json = {"console": 2048}
+ compute.post = AsyncioMagicMock(return_value=response)
+
+ project1 = await controller.add_project(name="Test1")
+ project2 = await controller.add_project(name="Test2")
+
+ template_id = str(uuid.uuid4())
+ await project1.add_node(
+ compute,
+ "n1",
+ None,
+ node_type="vpcs",
+ template_id=template_id,
+ properties={"hda_disk_image": "/tmp/images/disk.qcow2", "hda_disk_image_backing_file": "base.qcow2"},
+ )
+ await project2.add_node(compute, "n2", None, node_type="vpcs", properties={})
+
+ # opened projects
+ assert controller.find_projects_using_template(template_id) == ["Test1"]
+ assert controller.find_projects_using_template(str(uuid.uuid4())) == []
+ # image references are matched on file name whether stored as a bare
+ # name or as an absolute path
+ assert controller.find_projects_using_image("base.qcow2") == ["Test1"]
+ assert controller.find_projects_using_image("disk.qcow2") == ["Test1"]
+ assert controller.find_projects_using_image("unknown.qcow2") == []
+ referenced = controller.collect_referenced_image_filenames()
+ assert "base.qcow2" in referenced
+ assert "disk.qcow2" in referenced
+
+ # closed projects: same answers from the .gns3 file on disk
+ await project1.close()
+ await project2.close()
+ assert controller.find_projects_using_template(template_id) == ["Test1"]
+ assert controller.find_projects_using_image("base.qcow2") == ["Test1"]
+ assert controller.find_projects_using_image("disk.qcow2") == ["Test1"]
+
+
+@pytest.mark.asyncio
+async def test_find_projects_skips_unreadable_topology(controller):
+ """
+ A project whose topology cannot be read must not break the usage checks.
+ """
+
+ compute = MagicMock()
+ response = MagicMock()
+ response.json = {"console": 2048}
+ compute.post = AsyncioMagicMock(return_value=response)
+
+ project = await controller.add_project(name="Broken")
+ await project.add_node(
+ compute, "n1", None, node_type="vpcs",
+ template_id=str(uuid.uuid4()), properties={"hda_disk_image": "lost.qcow2"},
+ )
+ await project.close()
+ os.remove(project.topology_file)
+
+ assert controller.find_projects_using_template(str(uuid.uuid4())) == []
+ assert controller.find_projects_using_image("lost.qcow2") == []
+ assert controller.collect_referenced_image_filenames() == set()