feat: forbid deleting templates and images still used by projects

Deleting a template with prune_images, deleting an image, or pruning
orphan images only checked template references — a project node still
pointing at the image (e.g. via hda_disk_image_backing_file) was left
with a dangling reference and the project could no longer be opened.

- Add controller helpers scanning every known project (opened projects
  via in-memory nodes, closed projects via their .gns3 file) for
  template and image usage; unreadable topologies are skipped
- Guard DELETE /templates/{id}, DELETE /templates/{id}?prune_images,
  DELETE /images/{path} and /images/prune with a 409 listing the
  project names
- Run all template-delete checks before any mutation so a refused
  deletion cannot leave the template gone while its images survive
This commit is contained in:
YueGuobin 2026-08-31 00:33:29 +08:00
parent 861b7e86ff
commit 13548deae8
No known key found for this signature in database
8 changed files with 455 additions and 14 deletions

View File

@ -197,11 +197,16 @@ async def prune_images(
""" """
Prune images not attached to any template. Prune images not attached to any template.
Images referenced by a node in any project (opened or closed) are kept.
Required privilege: Image.Allocate Required privilege: Image.Allocate
""" """
skip_images = get_builtin_disks() 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( @router.post(
@ -309,6 +314,10 @@ async def delete_image(
template_names = ", ".join([template.name for template in templates]) template_names = ", ".join([template.name for template in templates])
raise ControllerError(f"Image '{image_path}' is used by one or more templates: {template_names}") 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: try:
os.remove(image.path) os.remove(image.path)
except OSError: except OSError:

View File

@ -35,6 +35,7 @@ from gns3server.db.repositories.templates import TemplatesRepository
from gns3server.services.templates import TemplatesService from gns3server.services.templates import TemplatesService
from gns3server.db.repositories.rbac import RbacRepository from gns3server.db.repositories.rbac import RbacRepository
from gns3server.db.repositories.images import ImagesRepository from gns3server.db.repositories.images import ImagesRepository
from gns3server.controller import Controller
from gns3server.controller.controller_error import ControllerError, ControllerBadRequestError from gns3server.controller.controller_error import ControllerError, ControllerBadRequestError
from gns3server.utils.images import get_builtin_disks from gns3server.utils.images import get_builtin_disks
@ -135,28 +136,49 @@ async def delete_template(
Required privilege: Template.Allocate Required privilege: Template.Allocate
""" """
controller = Controller.instance()
images = await templates_repo.get_template_images(template_id) 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: if prune_images and images:
skip_images = get_builtin_disks() skip_images = get_builtin_disks()
# collected lazily: a single pass over all projects' node properties
referenced_filenames = None
for image in images: for image in images:
if image.filename in skip_images: if image.filename in skip_images:
continue continue
templates = await images_repo.get_image_templates(image.image_id) # the template being deleted is still in the database at this
if templates: # point, exclude it from the other-templates check
template_names = ", ".join([template.name for template in templates]) 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}") raise ControllerError(f"Image '{image.path}' is used by one or more templates: {template_names}")
try: if referenced_filenames is None:
os.remove(image.path) referenced_filenames = controller.collect_referenced_image_filenames()
except OSError: if image.filename in referenced_filenames:
log.warning(f"Could not delete image file {image.path}") 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}'") await TemplatesService(templates_repo).delete_template(template_id)
success = await images_repo.delete_image(image.path) await rbac_repo.delete_all_ace_starting_with_path(f"/templates/{template_id}")
if not success:
raise ControllerError(f"Image '{image.path}' could not removed from the database") 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( @router.get(

View File

@ -38,6 +38,7 @@ from ..utils.images import default_images_directory
from ..utils.asyncio import wait_run_in_executor from ..utils.asyncio import wait_run_in_executor
from .project import Project from .project import Project
from .node import Node
from .appliance import Appliance from .appliance import Appliance
from .appliance_manager import ApplianceManager from .appliance_manager import ApplianceManager
from .compute import Compute, ComputeError from .compute import Compute, ComputeError
@ -91,6 +92,35 @@ class _ProjectsDirectoryEventHandler(FileSystemEventHandler):
return 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: class Controller:
""" """
The controller is responsible to manage one or more computes. 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?)") raise ControllerError("A project name could not be allocated (node limit reached?)")
return new_name 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 @property
def projects(self): def projects(self):
""" """

View File

@ -162,6 +162,10 @@ class Node:
def status(self): def status(self):
return self._status return self._status
@property
def template_id(self):
return self._template_id
@property @property
def name(self): def name(self):
return self._name return self._name

View File

@ -341,6 +341,15 @@ class TemplatesService:
if self.get_builtin_template(template_id): if self.get_builtin_template(template_id):
raise ControllerForbiddenError(f"Template '{template_id}' cannot be deleted because it is built-in") 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): if await self._templates_repo.delete_template(template_id):
self._controller.notification.controller_emit("template.deleted", {"template_id": str(template_id)}) self._controller.notification.controller_emit("template.deleted", {"template_id": str(template_id)})
else: else:

View File

@ -16,6 +16,8 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>. # along with this program. If not, see <http://www.gnu.org/licenses/>.
import os import os
import json
import uuid
import pytest import pytest
import hashlib import hashlib
@ -282,6 +284,99 @@ class TestImageRoutes:
images_in_db = await images_repo.get_images() images_in_db = await images_repo.get_images()
assert len(images_in_db) == 0 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( async def test_image_upload_create_appliance(
self, app: FastAPI, self, app: FastAPI,
client: AsyncClient, client: AsyncClient,

View File

@ -16,6 +16,7 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>. # along with this program. If not, see <http://www.gnu.org/licenses/>.
import os import os
import json
import shutil import shutil
import pytest import pytest
@ -270,6 +271,128 @@ class TestTemplateRoutes:
images = await images_repo.get_images() images = await images_repo.get_images()
assert len(images) == 0 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): # async def test_create_node_from_template(self, controller_api, controller, project):
# #
# id = str(uuid.uuid4()) # id = str(uuid.uuid4())

View File

@ -534,3 +534,74 @@ async def test_autoidlepc(controller):
await controller.autoidlepc("local", "c7200", "test.bin", 512) await controller.autoidlepc("local", "c7200", "test.bin", 512)
assert node_mock.dynamips_auto_idlepc.called assert node_mock.dynamips_auto_idlepc.called
assert len(controller.projects) == 0 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()