1
0
Fork 0
mirror of https://github.com/retspen/webvirtcloud synced 2026-11-08 19:26:48 +00:00

fix(api): typed URLs and accurate response schemas

- Routers use path converters (use_regex_path=False): computes, instances
  and flavors take <int:...> 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.
This commit is contained in:
catborise 2026-10-03 10:23:53 +03:00
parent 61208606f7
commit 3b8869c72f
10 changed files with 95 additions and 49 deletions

View file

@ -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(

View file

@ -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",

View file

@ -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"],

View file

@ -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

View file

@ -14,6 +14,7 @@ class InterfaceViewSet(viewsets.ViewSet):
"""
permission_classes = [IsSuperUser]
serializer_class = InterfacesSerializer
def list(self, request, compute_pk=None):
queryset = []

View file

@ -14,6 +14,7 @@ class NetworkViewSet(viewsets.ViewSet):
"""
permission_classes = [IsSuperUser]
serializer_class = NetworksSerializer
def list(self, request, compute_pk=None):

View file

@ -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)

View file

@ -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()

View file

@ -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)

View file

@ -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<arch>[^/.]+)/(?P<machine>[^/.]+)', CreateInstanceViewSet, basename='instance-create')
compute_router.register('instances/create/<str:arch>/<str:machine>', 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')