fix: address appliance v8 install review findings

- install: resolve the image directory from the version's settings type and
  skip image handling for docker appliances; guard appliance.images
- appliance schema: validate template_properties against template_type,
  align cpu_throttling with the qemu template, add kvm and version idlepc
- conversion: map IOU image to path, kvm disable to accel=tcg, inherit only
  same-type default settings, symbol fallback from the effective category,
  template_properties cannot override structural fields
- allow clearing netmiko_device_type with an empty string
- download the template symbol regardless of the level it is defined at and
  give qemu guests a default symbol
This commit is contained in:
YueGuobin committed 2026-08-21 21:41:42 +08:00
1 parent 435aa4c257
commit 1c68a52856
10 files changed
+603 -39

No files matched your search

@@ -166,6 +166,13 @@ class TestTemplateRoutes:
assert response.json()["name"] == "VPCS_TEST_RENAMED"
assert response.json()["netmiko_device_type"] == "cisco_ios"
# the field can also be cleared with an empty string
params = {"netmiko_device_type": ""}
response = await client.put(app.url_path_for("update_template", template_id=template_id), json=params)
assert response.status_code == status.HTTP_200_OK
assert response.json()["netmiko_device_type"] == ""
async def test_template_delete(self, app: FastAPI, client: AsyncClient) -> None:
template_id = str(uuid.uuid4())
+94 -13
View File
@@ -15,6 +15,9 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
import pydantic
import pytest
from gns3server.controller.appliance import Appliance
from gns3server.controller.appliance_to_template import ApplianceToTemplate
from gns3server.schemas.controller.appliances import ApplianceModel
@@ -77,25 +80,24 @@ def test_v8_docker_appliance_type():
def test_v8_type_from_default_settings():
# the default set wins over the other sets
appliance = {
"registry_version": 8,
"name": "mixed",
"status": "stable",
"settings": [
{"name": "a", "template_type": "docker", "template_properties": {"image": "x"}},
appliance = dict(
XRD_V8,
settings=[
{"name": "a", "template_type": "docker", "template_properties": {"image": "test:latest"}},
{"name": "b", "default": True, "template_type": "qemu", "template_properties": {"ram": 512}},
],
}
)
# the fixture must be a loadable appliance, not an unreachable shape
ApplianceModel.model_validate(appliance)
assert _appliance(appliance).type == "qemu"
def test_v8_qemu_appliance_type_without_default():
appliance = {
"registry_version": 8,
"name": "single",
"status": "stable",
"settings": [{"name": "a", "template_type": "qemu", "template_properties": {"ram": 512}}],
}
appliance = dict(
XRD_V8,
settings=[{"name": "a", "template_type": "qemu", "template_properties": {"ram": 512}}],
)
ApplianceModel.model_validate(appliance)
assert _appliance(appliance).type == "qemu"
@@ -127,3 +129,82 @@ def test_v8_docker_install_conversion():
{"adapter_number": 1, "port_name": "Gi0/0/0/0"},
]
assert template["extra_configs"] == [{"target": "/firstboot.cfg", "content": "!\nend\n"}]
def test_v8_docker_settings_require_image():
"""
template_properties must be validated against the model matching
template_type: a docker settings set without the required image is
rejected instead of being silently misrouted to another union member.
"""
appliance = dict(
XRD_V8,
settings=[
{"name": "only", "default": True, "template_type": "docker",
"template_properties": {"adapters": 2}},
],
)
with pytest.raises(pydantic.ValidationError):
ApplianceModel.model_validate(appliance)
def test_v8_template_properties_validated_against_template_type():
"""
Invalid enum values in qemu properties must be rejected at load time,
not silently discarded by a misrouted union member.
"""
appliance = dict(
XRD_V8,
settings=[
{"name": "only", "default": True, "template_type": "qemu",
"template_properties": {"ram": 512, "boot_priority": "zzz"}},
],
)
with pytest.raises(pydantic.ValidationError):
ApplianceModel.model_validate(appliance)
def test_v8_qemu_kvm_property_validates():
appliance = dict(
XRD_V8,
settings=[
{"name": "only", "default": True, "template_type": "qemu",
"template_properties": {"ram": 512, "kvm": "disable"}},
],
)
model = ApplianceModel.model_validate(appliance)
assert model.settings[0].template_properties.kvm == "disable"
def test_v8_qemu_cpu_throttling_range():
"""
cpu_throttling in v8 properties uses the same type and range as the
qemu template (int, 0-800).
"""
settings = {"name": "only", "default": True, "template_type": "qemu",
"template_properties": {"ram": 512, "cpu_throttling": 500}}
model = ApplianceModel.model_validate(dict(XRD_V8, settings=[settings]))
assert model.settings[0].template_properties.cpu_throttling == 500
for bad in (150.5, 900):
bad_settings = dict(settings, template_properties={"ram": 512, "cpu_throttling": bad})
with pytest.raises(pydantic.ValidationError):
ApplianceModel.model_validate(dict(XRD_V8, settings=[bad_settings]))
def test_v8_version_idlepc_validates():
appliance = dict(XRD_V8, versions=[{"name": "1.0", "idlepc": "0x613080c0"}])
ApplianceModel.model_validate(appliance)
bad = dict(XRD_V8, versions=[{"name": "1.0", "idlepc": "not-an-idlepc"}])
with pytest.raises(pydantic.ValidationError):
ApplianceModel.model_validate(bad)
def test_v8_netmiko_device_type_empty_string_clears():
appliance = dict(XRD_V8, netmiko_device_type="")
model = ApplianceModel.model_validate(appliance)
assert model.netmiko_device_type == ""
+150
View File
@@ -0,0 +1,150 @@
#!/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 <http://www.gnu.org/licenses/>.
import uuid
import pytest
from gns3server.controller.appliance import Appliance
from gns3server.controller.appliance_manager import ApplianceManager
def _v8_appliance(settings, versions=None):
"""
A minimal but fully valid registry version 8 appliance (it must pass
ApplianceModel validation, like appliances loaded by load_appliances).
"""
data = {
"registry_version": 8,
"appliance_id": str(uuid.uuid4()),
"name": "Test appliance",
"category": "router",
"description": "Appliance description",
"vendor_name": "Test vendor",
"vendor_url": "https://example.com/",
"product_name": "Test product",
"status": "stable",
"maintainer": "Test maintainer",
"maintainer_email": "maintainer@example.com",
"symbol": ":/symbols/router.svg",
"settings": settings,
}
if versions:
data["versions"] = versions
return data
class _FakeTemplatesService:
"""
Stands in for TemplatesService so install_appliance can be exercised
without a controller instance or database.
"""
created = []
def __init__(self, templates_repo):
self._templates_repo = templates_repo
async def create_template(self, template_create):
_FakeTemplatesService.created.append(template_create)
return {"name": template_create.name}
@pytest.mark.asyncio
async def test_install_docker_version_skips_image_resolution(monkeypatch):
"""
v8 docker appliances have no image files: installing a version must not
resolve an image directory (default_images_directory does not support
docker) nor iterate the appliance images list.
"""
_FakeTemplatesService.created = []
appliance_data = _v8_appliance(
[
{"name": "default", "default": True, "template_type": "docker",
"template_properties": {"image": "xrd:latest"}},
],
versions=[{"name": "1.0", "images": {"image": "xrd:1.0"}}],
)
manager = ApplianceManager()
appliance = Appliance("test.gns3a", appliance_data)
manager._appliances[appliance.id] = appliance
def _boom(image_type):
raise AssertionError(f"default_images_directory must not be called for docker (got '{image_type}')")
monkeypatch.setattr("gns3server.controller.appliance_manager.default_images_directory", _boom)
monkeypatch.setattr("gns3server.controller.appliance_manager.TemplatesService", _FakeTemplatesService)
await manager.install_appliance(uuid.UUID(appliance.id), "1.0", None, None, None, None)
assert len(_FakeTemplatesService.created) == 1
template = _FakeTemplatesService.created[0].model_dump()
assert template["template_type"] == "docker"
# the version image name is injected into the template
assert template["image"] == "xrd:1.0"
@pytest.mark.asyncio
async def test_install_iou_version_maps_image_to_path(monkeypatch, tmp_path):
"""
v8 IOU versions install: the version image is mapped to the template
path (an IOU template has no 'image' field).
"""
_FakeTemplatesService.created = []
appliance_data = _v8_appliance(
[
{"name": "default", "default": True, "template_type": "iou",
"template_properties": {"ethernet_adapters": 4, "ram": 256}},
],
versions=[{"name": "15.9", "images": {"image": "i86bi-linux-l3-15.9.bin"}}],
)
manager = ApplianceManager()
appliance = Appliance("test.gns3a", appliance_data)
manager._appliances[appliance.id] = appliance
monkeypatch.setattr(
"gns3server.controller.appliance_manager.default_images_directory",
lambda image_type: str(tmp_path),
)
monkeypatch.setattr("gns3server.controller.appliance_manager.TemplatesService", _FakeTemplatesService)
await manager.install_appliance(uuid.UUID(appliance.id), "15.9", None, None, None, None)
assert len(_FakeTemplatesService.created) == 1
template = _FakeTemplatesService.created[0].model_dump()
assert template["template_type"] == "iou"
assert template["path"] == "i86bi-linux-l3-15.9.bin"
assert "image" not in template
@pytest.mark.asyncio
async def test_install_version_not_found(monkeypatch):
manager = ApplianceManager()
appliance_data = _v8_appliance(
[{"name": "only", "default": True, "template_type": "docker",
"template_properties": {"image": "xrd:latest"}}],
versions=[{"name": "1.0", "images": {"image": "xrd:1.0"}}],
)
appliance = Appliance("test.gns3a", appliance_data)
manager._appliances[appliance.id] = appliance
from gns3server.controller.controller_error import ControllerNotFoundError
with pytest.raises(ControllerNotFoundError):
await manager.install_appliance(uuid.UUID(appliance.id), "9.9", None, None, None, None)
@@ -314,3 +314,244 @@ def test_v6_netmiko_device_type_copied_to_template():
template = ApplianceToTemplate().new_template(appliance, None, "local")
assert template["netmiko_device_type"] == "nokia_srl"
def test_v8_iou_version_images_mapped_to_path():
"""
An IOU template takes the image as a path: the version image name must be
mapped to 'path', never to an 'image' key.
"""
appliance = {
"registry_version": 8,
"name": "IOU L3",
"category": "router",
"settings": [
{"name": "only", "template_type": "iou",
"template_properties": {"ethernet_adapters": 4, "ram": 256}},
],
}
version = {"name": "15.9", "images": {"image": "i86bi-linux-l3-15.9.bin"}}
template = ApplianceToTemplate().new_template(appliance, version, "local")
assert template["template_type"] == "iou"
assert template["path"] == "i86bi-linux-l3-15.9.bin"
assert "image" not in template
def test_v8_dynamips_idlepc_from_version():
appliance = {
"registry_version": 8,
"name": "Cisco 7200",
"category": "router",
"settings": [
{"name": "only", "template_type": "dynamips",
"template_properties": {"ram": 512, "platform": "c7200"}},
],
}
version = {"name": "12.4", "idlepc": "0x613080c0", "images": {"image": "c7200.bin"}}
template = ApplianceToTemplate().new_template(appliance, version, "local")
assert template["idlepc"] == "0x613080c0"
assert template["image"] == "c7200.bin"
def test_v8_dynamips_settings_idlepc_precedence():
"""
An idlepc defined in template_properties wins over the version level.
"""
appliance = {
"registry_version": 8,
"name": "Cisco 7200",
"category": "router",
"settings": [
{"name": "only", "template_type": "dynamips",
"template_properties": {"ram": 512, "idlepc": "0x6142da40"}},
],
}
version = {"name": "12.4", "idlepc": "0x613080c0", "images": {"image": "c7200.bin"}}
template = ApplianceToTemplate().new_template(appliance, version, "local")
assert template["idlepc"] == "0x6142da40"
def test_v8_qemu_kvm_disable_forces_accel_tcg():
"""
kvm: disable is not a valid template property: it is converted to the
equivalent qemu options like for registry versions 1-6.
"""
appliance = {
"registry_version": 8,
"name": "QEMU VM",
"category": "guest",
"settings": [
{"name": "only", "template_type": "qemu",
"template_properties": {"ram": 512, "options": "-m 512", "kvm": "disable"}},
],
}
template = ApplianceToTemplate().new_template(appliance, None, "local")
assert template["options"] == "-m 512 -machine accel=tcg"
assert "kvm" not in template
def test_v8_qemu_kvm_allow_keeps_options():
appliance = {
"registry_version": 8,
"name": "QEMU VM",
"category": "guest",
"settings": [
{"name": "only", "template_type": "qemu",
"template_properties": {"ram": 512, "options": "-m 512", "kvm": "allow"}},
],
}
template = ApplianceToTemplate().new_template(appliance, None, "local")
assert template["options"] == "-m 512"
assert "kvm" not in template
def test_v8_no_cross_type_inheritance():
"""
A version referencing a docker settings set must not inherit properties
from a default qemu settings set.
"""
appliance = {
"registry_version": 8,
"name": "Mixed",
"category": "router",
"settings": [
{"name": "default qemu", "default": True, "template_type": "qemu",
"template_properties": {"ram": 2048, "adapters": 10}},
{"name": "docker alt", "template_type": "docker",
"template_properties": {"image": "xrd:latest", "adapters": 2}},
],
}
version = {"name": "1.0", "settings": "docker alt", "images": {"image": "xrd:1.0"}}
template = ApplianceToTemplate().new_template(appliance, version, "local")
assert template["template_type"] == "docker"
assert template["adapters"] == 2
assert "ram" not in template
assert template["image"] == "xrd:1.0"
def test_v8_multiple_defaults_inherit_first_same_type():
"""
With several default settings sets of the same type, the first one is
inherited from, matching the selection rule.
"""
appliance = {
"registry_version": 8,
"name": "Multi",
"category": "router",
"settings": [
{"name": "default one", "default": True, "template_type": "qemu",
"template_properties": {"ram": 1024}},
{"name": "default two", "default": True, "template_type": "qemu",
"template_properties": {"ram": 2048}},
{"name": "alt", "template_type": "qemu", "template_properties": {"cpus": 2}},
],
}
version = {"name": "1.0", "settings": "alt"}
template = ApplianceToTemplate().new_template(appliance, version, "local")
assert template["ram"] == 1024
assert template["cpus"] == 2
def test_v8_symbol_fallback_uses_effective_category():
"""
The default symbol must reflect the effective category (template_properties,
then version, then appliance), not the appliance-level one.
"""
appliance = {
"registry_version": 8,
"name": "Test",
"category": "router",
"settings": [
{"name": "only", "template_type": "qemu",
"template_properties": {"ram": 512, "category": "guest"}},
],
}
template = ApplianceToTemplate().new_template(appliance, None, "local")
assert template["category"] == "guest"
assert template["symbol"] == ":/symbols/qemu_guest.svg"
appliance = {
"registry_version": 8,
"name": "Test",
"category": "guest",
"settings": [
{"name": "only", "template_type": "qemu", "template_properties": {"ram": 512}},
],
}
version = {"name": "1.0", "category": "router"}
template = ApplianceToTemplate().new_template(appliance, version, "local")
assert template["category"] == "router"
assert template["symbol"] == ":/symbols/router.svg"
def test_v8_reserved_keys_cannot_be_overridden():
"""
template_properties must not override the structural template fields.
"""
appliance = {
"registry_version": 8,
"name": "Test",
"category": "router",
"settings": [
{"name": "only", "template_type": "docker",
"template_properties": {
"image": "xrd:latest",
"template_type": "qemu",
"compute_id": "evil-compute",
"version": "9.9",
}},
],
}
template = ApplianceToTemplate().new_template(appliance, None, "local")
assert template["template_type"] == "docker"
assert template["compute_id"] == "local"
assert "version" not in template
def test_v8_get_template_type():
"""
get_template_type resolves the emulator type from the settings set
selected for the version, like the install flow does.
"""
converter = ApplianceToTemplate()
appliance = {
"registry_version": 8,
"name": "Mixed",
"category": "router",
"settings": [
{"name": "default", "default": True, "template_type": "qemu",
"template_properties": {"ram": 512}},
{"name": "alt", "template_type": "docker",
"template_properties": {"image": "xrd:latest"}},
],
}
assert converter.get_template_type(appliance, {"name": "1.0", "settings": "alt"}) == "docker"
assert converter.get_template_type(appliance, None) == "qemu"
v6 = {"registry_version": 6, "docker": {"image": "test:latest"}}
assert converter.get_template_type(v6, None) == "docker"
+5
View File
@@ -401,6 +401,11 @@ async def test_update_netmiko_device_type(node, compute):
assert node.netmiko_device_type == "cisco_ios_telnet"
assert node.asdict()["netmiko_device_type"] == "cisco_ios_telnet"
# the field can also be cleared with an empty string
await node.update(netmiko_device_type="")
assert node.netmiko_device_type == ""
assert node.asdict()["netmiko_device_type"] == ""
def test_netmiko_device_type_from_template_kwargs(compute, project):
"""