From 3b8869c72fab8381887198ce7dcd3d2be4f0130e Mon Sep 17 00:00:00 2001 From: catborise Date: Sat, 3 Oct 2026 10:23:53 +0300 Subject: [PATCH] fix(api): typed URLs and accurate response schemas - Routers use path converters (use_regex_path=False): computes, instances and flavors take ids, so non-numeric ids get 404 and drf-spectacular derives the parameter types itself. - Each method declares what it really returns: detail serializers, a shared StatusSerializer for status-only replies, no request body for actions. Instance properties used by the details serializer get return type hints. - The create-instance API drops the firmware field. Its only accepted value was "" (BIOS), which produced an empty, invalid enum, and the UEFI branch behind it was unreachable. Clients that still send it are unaffected; DRF ignores unknown fields. The schema now passes spectacular --validate --fail-on-warn; a test keeps it that way. --- computes/api/viewsets.py | 7 +++++ instances/api/serializers.py | 6 ----- instances/api/viewsets.py | 32 +++++++++++++---------- instances/models.py | 46 ++++++++++++++++----------------- interfaces/api/viewsets.py | 1 + networks/api/viewsets.py | 1 + storages/api/viewsets.py | 18 +++++++++++-- webvirtcloud/serializers.py | 7 +++++ webvirtcloud/test_api_schema.py | 18 +++++++++++++ webvirtcloud/urls-api.py | 8 +++--- 10 files changed, 95 insertions(+), 49 deletions(-) create mode 100644 webvirtcloud/serializers.py create mode 100644 webvirtcloud/test_api_schema.py diff --git a/computes/api/viewsets.py b/computes/api/viewsets.py index a3d59e92..17bfe165 100644 --- a/computes/api/viewsets.py +++ b/computes/api/viewsets.py @@ -1,5 +1,7 @@ from admin.permissions import IsSuperUser from computes.models import Compute +from drf_spectacular.types import OpenApiTypes +from drf_spectacular.utils import extend_schema from rest_framework import viewsets from rest_framework.response import Response from vrtManager.create import wvmCreate @@ -13,6 +15,7 @@ class ComputeViewSet(viewsets.ModelViewSet): """ queryset = Compute.objects.all().order_by("name") + lookup_value_converter = "int" serializer_class = ComputeSerializer permission_classes = [IsSuperUser] @@ -20,6 +23,9 @@ class ComputeViewSet(viewsets.ModelViewSet): class ComputeArchitecturesView(viewsets.ViewSet): permission_classes = [IsSuperUser] + # The list is a mapping of architecture -> machine types, not an array, + # so drf-spectacular would name it "retrieve" and clash with retrieve(). + @extend_schema(operation_id="api_v1_computes_archs_list", responses=OpenApiTypes.OBJECT) def list(self, request, compute_pk=None): """ Return a list of supported host architectures. @@ -33,6 +39,7 @@ class ComputeArchitecturesView(viewsets.ViewSet): ) return Response(conn.get_hypervisors_machines()) + @extend_schema(responses={200: {"type": "array", "items": {"type": "string"}}}) def retrieve(self, request, compute_pk=None, pk=None): compute = Compute.objects.get(pk=compute_pk) conn = wvmCreate( diff --git a/instances/api/serializers.py b/instances/api/serializers.py index 39d0c0d8..ee4c2272 100644 --- a/instances/api/serializers.py +++ b/instances/api/serializers.py @@ -52,11 +52,6 @@ class FlavorSerializer(serializers.ModelSerializer): class CreateInstanceSerializer(serializers.ModelSerializer): - firmware_choices = ( - ("", "BIOS"), - # ('UEFI', 'UEFI'), - ) - firmware = serializers.ChoiceField(choices=firmware_choices) graphics = serializers.CharField(initial="vnc") video = serializers.CharField(initial="vga") storage = serializers.CharField(initial="default") @@ -68,7 +63,6 @@ class CreateInstanceSerializer(serializers.ModelSerializer): model = CreateInstance fields = [ "name", - "firmware", "vcpu", "vcpu_mode", "memory", diff --git a/instances/api/viewsets.py b/instances/api/viewsets.py index 569ceb67..2c8fd36a 100644 --- a/instances/api/viewsets.py +++ b/instances/api/viewsets.py @@ -16,11 +16,13 @@ from instances.views import ( resume, suspend, ) +from drf_spectacular.utils import extend_schema from rest_framework import permissions, status, viewsets from rest_framework.decorators import action from rest_framework.response import Response from vrtManager import util from vrtManager.create import wvmCreate +from webvirtcloud.serializers import StatusSerializer from .serializers import ( CreateInstanceSerializer, @@ -37,6 +39,8 @@ class InstancesViewSet(viewsets.ViewSet): """ permission_classes = [permissions.IsAuthenticated] + serializer_class = InstanceSerializer + lookup_value_converter = "int" def list(self, request): @@ -66,8 +70,9 @@ class InstanceViewSet(viewsets.ViewSet): A simple ViewSet for listing or retrieving Compute Instances. """ - # serializer_class = CreateInstanceSerializer permission_classes = [permissions.IsAuthenticated] + serializer_class = InstanceSerializer + lookup_value_converter = "int" def list(self, request, compute_pk=None): compute = get_object_or_404(Compute, pk=compute_pk) @@ -98,6 +103,7 @@ class InstanceViewSet(viewsets.ViewSet): return Response(serializer.data) + @extend_schema(responses=InstanceDetailsSerializer) def retrieve(self, request, pk=None, compute_pk=None): queryset = get_instance(request.user, pk) if compute_pk is not None and str(queryset.compute_id) != str(compute_pk): @@ -106,6 +112,7 @@ class InstanceViewSet(viewsets.ViewSet): return Response(serializer.data) + @extend_schema(responses=StatusSerializer) def destroy(self, request, pk=None, compute_pk=None): queryset = get_instance(request.user, pk, perm_type="delete") if compute_pk is not None and str(queryset.compute_id) != str(compute_pk): @@ -113,6 +120,7 @@ class InstanceViewSet(viewsets.ViewSet): instance_destroy(request, pk) return Response({"status": "Instance is destroyed"}) + @extend_schema(request=None, responses=StatusSerializer) @action(detail=True, methods=["post"]) def poweron(self, request, pk=None, compute_pk=None): queryset = get_instance(request.user, pk, perm_type="power") @@ -121,6 +129,7 @@ class InstanceViewSet(viewsets.ViewSet): poweron(request, pk) return Response({"status": "poweron command send"}) + @extend_schema(request=None, responses=StatusSerializer) @action(detail=True, methods=["post"]) def poweroff(self, request, pk=None, compute_pk=None): queryset = get_instance(request.user, pk, perm_type="power") @@ -129,6 +138,7 @@ class InstanceViewSet(viewsets.ViewSet): poweroff(request, pk) return Response({"status": "poweroff command send"}) + @extend_schema(request=None, responses=StatusSerializer) @action(detail=True, methods=["post"]) def powercycle(self, request, pk=None, compute_pk=None): queryset = get_instance(request.user, pk, perm_type="power") @@ -137,6 +147,7 @@ class InstanceViewSet(viewsets.ViewSet): powercycle(request, pk) return Response({"status": "powercycle command send"}) + @extend_schema(request=None, responses=StatusSerializer) @action(detail=True, methods=["post"]) def forceoff(self, request, pk=None, compute_pk=None): queryset = get_instance(request.user, pk, perm_type="power") @@ -145,6 +156,7 @@ class InstanceViewSet(viewsets.ViewSet): force_off(request, pk) return Response({"status": "force off command send"}) + @extend_schema(request=None, responses=StatusSerializer) @action(detail=True, methods=["post"]) def suspend(self, request, pk=None, compute_pk=None): queryset = get_instance(request.user, pk) @@ -153,6 +165,7 @@ class InstanceViewSet(viewsets.ViewSet): suspend(request, pk) return Response({"status": "suspend command send"}) + @extend_schema(request=None, responses=StatusSerializer) @action(detail=True, methods=["post"]) def resume(self, request, pk=None, compute_pk=None): queryset = get_instance(request.user, pk) @@ -171,6 +184,7 @@ class MigrateViewSet(viewsets.ViewSet): serializer_class = MigrateSerializer queryset = "" + @extend_schema(responses=StatusSerializer) def create(self, request): serializer = MigrateSerializer(data=request.data) if serializer.is_valid(): @@ -208,6 +222,7 @@ class FlavorViewSet(viewsets.ModelViewSet): """ queryset = Flavor.objects.all().order_by("id") + lookup_value_converter = "int" serializer_class = FlavorSerializer def get_permissions(self): @@ -225,6 +240,7 @@ class CreateInstanceViewSet(viewsets.ViewSet): serializer_class = CreateInstanceSerializer queryset = "" + @extend_schema(responses=StatusSerializer) def create(self, request, compute_pk=None, arch=None, machine=None): serializer = CreateInstanceSerializer( data=request.data, @@ -260,7 +276,6 @@ class CreateInstanceViewSet(viewsets.ViewSet): default_disk_owner_gid, ) volume = {} - firmware = {} volume["device"] = "disk" volume["path"] = path volume["type"] = conn.get_volume_format_type(path) @@ -274,17 +289,6 @@ class CreateInstanceViewSet(viewsets.ViewSet): volume_list.append(volume) - if "UEFI" in serializer.validated_data["firmware"]: - firmware["loader"] = ( - serializer.validated_data["firmware"].split(":")[1].strip() - ) - firmware["secure"] = "no" - firmware["readonly"] = "yes" - firmware["type"] = "pflash" - if "secboot" in firmware["loader"] and machine != "q35": - machine = "q35" - firmware["secure"] = "yes" - with utils.libvirt_compute_lock(compute): ret = conn.create_instance( name=serializer.validated_data["name"], @@ -294,7 +298,7 @@ class CreateInstanceViewSet(viewsets.ViewSet): uuid=util.randomUUID(), arch=arch, machine=machine, - firmware=firmware, + firmware={}, volumes=volume_list, networks=serializer.validated_data["networks"], nwfilter=serializer.validated_data["nwfilter"], diff --git a/instances/models.py b/instances/models.py index 8d34f88f..7985707d 100644 --- a/instances/models.py +++ b/instances/models.py @@ -51,51 +51,51 @@ class Instance(models.Model): ) @cached_property - def media(self): + def media(self) -> list[dict]: return self.proxy.get_media_devices() @cached_property - def media_iso(self): + def media_iso(self) -> list[str]: return sorted(self.proxy.get_iso_media()) @cached_property - def disks(self): + def disks(self) -> list[dict]: return self.proxy.get_disk_devices() @cached_property - def status(self): + def status(self) -> int: return self.proxy.get_status() @cached_property - def autostart(self): + def autostart(self) -> int: return self.proxy.get_autostart() @cached_property - def bootmenu(self): + def bootmenu(self) -> bool: return self.proxy.get_bootmenu() @cached_property - def boot_order(self): + def boot_order(self) -> dict: return self.proxy.get_bootorder() @cached_property - def arch(self): + def arch(self) -> str: return self.proxy.get_arch() @cached_property - def machine(self): + def machine(self) -> str: return self.proxy.get_machine_type() @cached_property - def firmware(self): + def firmware(self) -> dict: return self.proxy.get_loader() @cached_property - def nvram(self): + def nvram(self) -> str | None: return self.proxy.get_nvram() @cached_property - def vcpu(self): + def vcpu(self) -> int: return self.proxy.get_vcpu() @cached_property @@ -115,7 +115,7 @@ class Instance(models.Model): return self.proxy.get_uuid() @cached_property - def memory(self): + def memory(self) -> int: return self.proxy.get_memory() @cached_property @@ -123,15 +123,15 @@ class Instance(models.Model): return self.proxy.get_cur_memory() @cached_property - def title(self): + def title(self) -> str: return self.proxy.get_title() @cached_property - def description(self): + def description(self) -> str: return self.proxy.get_description() @cached_property - def networks(self): + def networks(self) -> list[dict]: return self.proxy.get_net_devices() @cached_property @@ -143,19 +143,19 @@ class Instance(models.Model): return self.proxy.get_telnet_port() @cached_property - def console_type(self): + def console_type(self) -> str | None: return self.proxy.get_console_type() @cached_property - def console_port(self): + def console_port(self) -> str | None: return self.proxy.get_console_port() @cached_property - def console_keymap(self): + def console_keymap(self) -> str: return self.proxy.get_console_keymap() @cached_property - def console_listener_address(self): + def console_listener_address(self) -> str: return self.proxy.get_console_listener_addr() @cached_property @@ -163,11 +163,11 @@ class Instance(models.Model): return False if self.proxy.get_guest_agent() is None else True @cached_property - def guest_agent_ready(self): + def guest_agent_ready(self) -> bool: return self.proxy.is_agent_ready() @cached_property - def video_model(self): + def video_model(self) -> str | None: return self.proxy.get_video_model() @cached_property @@ -175,7 +175,7 @@ class Instance(models.Model): return self.proxy.get_video_models(self.arch, self.machine) @cached_property - def snapshots(self): + def snapshots(self) -> list[dict]: return sorted(self.proxy.get_snapshot(), reverse=True, key=lambda k: k["date"]) @cached_property diff --git a/interfaces/api/viewsets.py b/interfaces/api/viewsets.py index d7ff73c0..53a67f5e 100644 --- a/interfaces/api/viewsets.py +++ b/interfaces/api/viewsets.py @@ -14,6 +14,7 @@ class InterfaceViewSet(viewsets.ViewSet): """ permission_classes = [IsSuperUser] + serializer_class = InterfacesSerializer def list(self, request, compute_pk=None): queryset = [] diff --git a/networks/api/viewsets.py b/networks/api/viewsets.py index 08372f2d..439f133f 100644 --- a/networks/api/viewsets.py +++ b/networks/api/viewsets.py @@ -14,6 +14,7 @@ class NetworkViewSet(viewsets.ViewSet): """ permission_classes = [IsSuperUser] + serializer_class = NetworksSerializer def list(self, request, compute_pk=None): diff --git a/storages/api/viewsets.py b/storages/api/viewsets.py index 1dee022a..b90e5e78 100644 --- a/storages/api/viewsets.py +++ b/storages/api/viewsets.py @@ -2,10 +2,12 @@ from admin.permissions import IsSuperUser from appsettings.settings import app_settings from computes.models import Compute from django.shortcuts import get_object_or_404 -from rest_framework import status, viewsets +from drf_spectacular.utils import extend_schema, inline_serializer +from rest_framework import serializers, status, viewsets from rest_framework.decorators import action from rest_framework.response import Response from vrtManager.storage import wvmStorage, wvmStorages +from webvirtcloud.serializers import StatusSerializer from .serializers import StorageSerializer, StoragesSerializer, VolumeSerializer @@ -16,6 +18,8 @@ class StorageViewSet(viewsets.ViewSet): """ permission_classes = [IsSuperUser] + serializer_class = StoragesSerializer + lookup_value_converter = "str" def list(self, request, compute_pk=None): @@ -32,6 +36,7 @@ class StorageViewSet(viewsets.ViewSet): return Response(serializer.data) + @extend_schema(responses=StorageSerializer) def retrieve(self, request, pk=None, compute_pk=None): compute = get_object_or_404(Compute, pk=compute_pk) @@ -55,6 +60,7 @@ class StorageViewSet(viewsets.ViewSet): ) return Response(serializer.data) + @extend_schema(request=None, responses=StatusSerializer) @action(detail=True, methods=["post"]) def start(self, request, pk=None, compute_pk=None): compute = get_object_or_404(Compute, pk=compute_pk) @@ -66,6 +72,7 @@ class StorageViewSet(viewsets.ViewSet): conn.close() return Response({"status": "Pool start command send: " + str(ret)}) + @extend_schema(request=None, responses=StatusSerializer) @action(detail=True, methods=["post"]) def stop(self, request, pk=None, compute_pk=None): compute = get_object_or_404(Compute, pk=compute_pk) @@ -77,6 +84,7 @@ class StorageViewSet(viewsets.ViewSet): conn.close() return Response({"status": "Pool stop command send: " + str(ret)}) + @extend_schema(request=None, responses=StatusSerializer) @action(detail=True, methods=["post"]) def refresh(self, request, pk=None, compute_pk=None): compute = get_object_or_404(Compute, pk=compute_pk) @@ -88,6 +96,10 @@ class StorageViewSet(viewsets.ViewSet): conn.close() return Response({"status": "Pool refresh command send: " + str(ret)}) + @extend_schema( + request=None, + responses=inline_serializer("PoolXML", {"return": serializers.CharField()}), + ) @action(detail=True, methods=["post"]) def XML_description(self, request, pk=None, compute_pk=None): compute = get_object_or_404(Compute, pk=compute_pk) @@ -108,7 +120,7 @@ class VolumeViewSet(viewsets.ViewSet): permission_classes = [IsSuperUser] serializer_class = VolumeSerializer - lookup_value_regex = "[^/]+" + lookup_value_converter = "str" def list(self, request, storage_pk=None, compute_pk=None): @@ -147,6 +159,7 @@ class VolumeViewSet(viewsets.ViewSet): return Response(serializer.data) + @extend_schema(responses=StatusSerializer) def create(self, request, storage_pk=None, compute_pk=None): compute = get_object_or_404(Compute, pk=compute_pk) @@ -174,6 +187,7 @@ class VolumeViewSet(viewsets.ViewSet): else: return Response({"status": "Data is not right for create volume"}) + @extend_schema(responses=StatusSerializer) def destroy(self, request, storage_pk=None, compute_pk=None, pk=None): compute = get_object_or_404(Compute, pk=compute_pk) diff --git a/webvirtcloud/serializers.py b/webvirtcloud/serializers.py new file mode 100644 index 00000000..027a2690 --- /dev/null +++ b/webvirtcloud/serializers.py @@ -0,0 +1,7 @@ +from rest_framework import serializers + + +class StatusSerializer(serializers.Serializer): + """Response body of API actions that only report a status message.""" + + status = serializers.CharField() diff --git a/webvirtcloud/test_api_schema.py b/webvirtcloud/test_api_schema.py new file mode 100644 index 00000000..bb600239 --- /dev/null +++ b/webvirtcloud/test_api_schema.py @@ -0,0 +1,18 @@ +import os + +from django.contrib.auth import get_user_model +from django.core.management import call_command +from django.test import TestCase +from rest_framework.test import APIClient + + +class APISchemaTestCase(TestCase): + def test_schema_is_valid_without_warnings(self): + call_command("spectacular", "--validate", "--fail-on-warn", "--file", os.devnull) + + def test_non_numeric_compute_id_is_not_routed(self): + admin = get_user_model().objects.create_superuser("schema_admin", "a@example.com", "pw") + client = APIClient() + client.force_login(admin) + self.assertEqual(client.get("/api/v1/computes/abc/instances/").status_code, 404) + self.assertEqual(client.get("/api/v1/instances/abc/").status_code, 404) diff --git a/webvirtcloud/urls-api.py b/webvirtcloud/urls-api.py index 40dab639..06f610b0 100644 --- a/webvirtcloud/urls-api.py +++ b/webvirtcloud/urls-api.py @@ -12,21 +12,21 @@ from instances.api.viewsets import FlavorViewSet, \ CreateInstanceViewSet -router = routers.SimpleRouter() +router = routers.SimpleRouter(use_regex_path=False) router.register(r'computes', ComputeViewSet) router.register(r'migrate', MigrateViewSet, basename='instance-migrate') router.register(r'flavor', FlavorViewSet, basename='instance-flavor') router.register(r'instances', InstancesViewSet, basename='instance') -compute_router = routers.NestedSimpleRouter(router, r'computes', lookup='compute') +compute_router = routers.NestedSimpleRouter(router, r'computes', lookup='compute', use_regex_path=False) compute_router.register(r'instances', InstanceViewSet, basename='compute-instance') -compute_router.register(r'instances/create/(?P[^/.]+)/(?P[^/.]+)', CreateInstanceViewSet, basename='instance-create') +compute_router.register('instances/create//', CreateInstanceViewSet, basename='instance-create') compute_router.register(r'networks', NetworkViewSet, basename='compute-network') compute_router.register(r'interfaces', InterfaceViewSet, basename='compute-interface') compute_router.register(r'storages', StorageViewSet, basename='compute-storage') compute_router.register(r'archs', ComputeArchitecturesView, basename='compute-archs') -storage_router = routers.NestedSimpleRouter(compute_router, r'storages', lookup='storage') +storage_router = routers.NestedSimpleRouter(compute_router, r'storages', lookup='storage', use_regex_path=False) storage_router.register(r'volumes', VolumeViewSet, basename='compute-storage-volumes')