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

fix(api): an error's status says what it is, and its body is always {"detail"}

- libvirt errors all answered 400. A VM, pool or volume the host does not
  have now answers 404, a volume in use 409 (vrtManager.util.InUse) and a
  host that cannot be reached 503, which a client may retry.
- A template's power on or cycle and a stopped pool answered
  {"status": ...} with 409; they answer {"detail": ...}, as every other
  refusal. The schema documents these 409s (ErrorSerializer).
- The volume list and a volume of a stopped pool answered 200 with an
  empty list or a made-up volume; they answer 409.
- An API URL that names nothing answered the HTML error page; it answers
  a JSON 404 after the API's authentication.
- A session that must change its password was redirected to the HTML
  form; an API request of it gets the JSON 403 a token gets.
This commit is contained in:
catborise 2026-11-07 00:19:13 +03:00
parent 9d22d48a85
commit 0610c23a34
14 changed files with 145 additions and 36 deletions

View file

@ -783,6 +783,7 @@ What an update from an older version changes for operators and users:
- In Docker, the TLS client certificates of TLS computes are kept in a new volume, `webvirtcloud-pki` (`/var/www/.pki`), and backed up; put them there once ([Adding a Compute Node](#adding-a-compute-node), step 2).
- The SQLite database must be on a local file system (WAL). Outside compose, stop the Docker container with `docker stop -t 95` so the worker can finish running jobs.
- Long operations run as jobs in a worker process (`manage.py run_worker`): creating, cloning, migrating and deleting VMs, moving disks, cloning volumes, taking and reverting snapshots and deleting external snapshots. The page returns at once and the job panel and the Jobs page follow them; a migration can be cancelled. The API's VM create, migrate and delete answer 202 with the job and need a new `submission_key` per request. The create form no longer takes a console password; set it on the VM page.
- An API error is `{"detail": ...}` with a status that says what it is: 404 for a VM, pool or volume read or deleted that the host does not have, 409 for a volume in use, a template started or a stopped pool, 503 for a host that cannot be reached (all were 400 or 200 before); an unknown API URL answers JSON.
- An ISO upload is tracked as a job that can be cancelled. The browser sends the file, so stay on the page until it ends; an upload left idle for an hour is ended and its partial file deleted.
- A clone of a running VM (libvirt 11.10 or later, local file disks, no TPM) locks the VM until the copy ends and can be cancelled; cancelling a clone of a shut-off VM has no effect. A job that ends uncertain keeps its VM locked until a superuser resolves it on its job page.
- The job worker runs at most half of its concurrency (Settings, at least one) of one host's jobs at a time, so a host that does not answer cannot take every slot.

View file

@ -1,4 +1,6 @@
from django.http import JsonResponse
from django.shortcuts import redirect
from django.utils.translation import gettext as _
from django.urls import Resolver404, resolve
from accounts.models import UserAttributes
@ -22,5 +24,7 @@ class ForcePasswordChangeMiddleware:
except Resolver404:
view_name = None
if view_name not in ALLOWED_VIEWS:
if request.path_info.startswith("/api/"): # as for a token (accounts.auth)
return JsonResponse({"detail": _("Change your password before using the API.")}, status=403)
return redirect("accounts:change_password")
return self.get_response(request)

View file

@ -38,6 +38,12 @@ class FirstLoginPasswordChangeTestCase(TestCase):
)
self.assertEqual(self.client.get(reverse("accounts:change_password")).status_code, 200)
def test_an_api_request_of_the_session_gets_json_not_the_form(self):
# as a token of that user does (accounts.auth)
self.client.login(request=RequestFactory().get("/"), username="firstadmin", password=GENERATED)
response = self.client.get("/api/v1/instances/")
self.assertEqual((response.status_code, response.json()), (403, {"detail": "Change your password before using the API."}))
def test_changing_the_password_lifts_the_redirect_and_deletes_the_file(self):
self.client.login(request=RequestFactory().get("/"), username="firstadmin", password=GENERATED)
self.client.post(

View file

@ -24,7 +24,7 @@ from operations.api.serializers import OperationSerializer
from rest_framework import permissions, status, viewsets
from rest_framework.decorators import action
from rest_framework.response import Response
from webvirtcloud.serializers import ActionStatusSerializer
from webvirtcloud.serializers import ActionStatusSerializer, ErrorSerializer
from .serializers import (
CreateInstanceSerializer,
@ -124,7 +124,7 @@ class InstanceViewSet(viewsets.ViewSet):
return Response(serializer.data)
@extend_schema(request=DestroySerializer, responses={202: OperationSerializer, 400: dict})
@extend_schema(request=DestroySerializer, responses={202: OperationSerializer, 400: dict, 409: ErrorSerializer})
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):
@ -138,17 +138,17 @@ class InstanceViewSet(viewsets.ViewSet):
return Response(OperationSerializer(op, context={"request": request}).data, status=status.HTTP_202_ACCEPTED,
headers={"Location": reverse("operation-detail", args=[op.pk])})
@extend_schema(request=None, responses=ActionStatusSerializer)
@extend_schema(request=None, responses={200: ActionStatusSerializer, 409: ErrorSerializer})
@action(detail=True, methods=["post"])
def poweron(self, request, pk=None, compute_pk=None):
queryset = get_instance(request.user, pk, perm_type="power")
if compute_pk is not None and str(queryset.compute_id) != str(compute_pk):
raise Http404()
if queryset.is_template:
return Response({"status": _("Templates cannot be started.")}, status=status.HTTP_409_CONFLICT)
return Response({"detail": _("Templates cannot be started.")}, status=status.HTTP_409_CONFLICT)
return instance_action_response(poweron, request, pk, "poweron command send")
@extend_schema(request=None, responses=ActionStatusSerializer)
@extend_schema(request=None, responses={200: ActionStatusSerializer, 409: ErrorSerializer})
@action(detail=True, methods=["post"])
def poweroff(self, request, pk=None, compute_pk=None):
queryset = get_instance(request.user, pk, perm_type="power")
@ -156,17 +156,17 @@ class InstanceViewSet(viewsets.ViewSet):
raise Http404()
return instance_action_response(poweroff, request, pk, "poweroff command send")
@extend_schema(request=None, responses=ActionStatusSerializer)
@extend_schema(request=None, responses={200: ActionStatusSerializer, 409: ErrorSerializer})
@action(detail=True, methods=["post"])
def powercycle(self, request, pk=None, compute_pk=None):
queryset = get_instance(request.user, pk, perm_type="power")
if compute_pk is not None and str(queryset.compute_id) != str(compute_pk):
raise Http404()
if queryset.is_template:
return Response({"status": _("Templates cannot be started.")}, status=status.HTTP_409_CONFLICT)
return Response({"detail": _("Templates cannot be started.")}, status=status.HTTP_409_CONFLICT)
return instance_action_response(powercycle, request, pk, "powercycle command send")
@extend_schema(request=None, responses=ActionStatusSerializer)
@extend_schema(request=None, responses={200: ActionStatusSerializer, 409: ErrorSerializer})
@action(detail=True, methods=["post"])
def forceoff(self, request, pk=None, compute_pk=None):
queryset = get_instance(request.user, pk, perm_type="power")
@ -174,7 +174,7 @@ class InstanceViewSet(viewsets.ViewSet):
raise Http404()
return instance_action_response(force_off, request, pk, "force off command send")
@extend_schema(request=None, responses=ActionStatusSerializer)
@extend_schema(request=None, responses={200: ActionStatusSerializer, 409: ErrorSerializer})
@action(detail=True, methods=["post"])
def suspend(self, request, pk=None, compute_pk=None):
queryset = get_instance(request.user, pk)
@ -182,7 +182,7 @@ class InstanceViewSet(viewsets.ViewSet):
raise Http404()
return instance_action_response(suspend, request, pk, "suspend command send")
@extend_schema(request=None, responses=ActionStatusSerializer)
@extend_schema(request=None, responses={200: ActionStatusSerializer, 409: ErrorSerializer})
@action(detail=True, methods=["post"])
def resume(self, request, pk=None, compute_pk=None):
queryset = get_instance(request.user, pk)
@ -200,7 +200,7 @@ class MigrateViewSet(viewsets.ViewSet):
serializer_class = MigrateSerializer
queryset = ""
@extend_schema(responses={202: OperationSerializer, 400: dict})
@extend_schema(responses={202: OperationSerializer, 400: dict, 409: ErrorSerializer})
def create(self, request):
serializer = MigrateSerializer(data=request.data)
if not serializer.is_valid():
@ -254,7 +254,7 @@ class CreateInstanceViewSet(viewsets.ViewSet):
serializer_class = CreateInstanceSerializer
queryset = ""
@extend_schema(responses={202: OperationSerializer, 400: dict})
@extend_schema(responses={202: OperationSerializer, 400: dict, 409: ErrorSerializer})
def create(self, request, compute_pk=None, arch=None, machine=None):
serializer = CreateInstanceSerializer(data=request.data,
context={"compute_pk": compute_pk, "arch": arch, "machine": machine})

View file

@ -13,7 +13,7 @@ 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 webvirtcloud.serializers import ErrorSerializer, StatusSerializer
from .serializers import StorageSerializer, StoragesSerializer, VolumeSerializer
@ -135,6 +135,7 @@ class VolumeViewSet(viewsets.ViewSet):
serializer_class = VolumeSerializer
lookup_value_converter = "str"
@extend_schema(responses={200: VolumeSerializer(many=True), 409: ErrorSerializer})
def list(self, request, storage_pk=None, compute_pk=None):
compute = get_object_or_404(Compute, pk=compute_pk)
@ -142,12 +143,9 @@ class VolumeViewSet(viewsets.ViewSet):
conn = wvmStorage(
compute.hostname, compute.login, compute.password, compute.type, storage_pk
)
state = conn.is_active()
if state:
volume_queryset = conn.update_volumes()
else:
volume_queryset = None
if not conn.is_active():
return Response({"detail": _("Pool is not active")}, status=status.HTTP_409_CONFLICT)
volume_queryset = conn.update_volumes()
conn.close()
serializer = VolumeSerializer(
volume_queryset, many=True, context={"request": request}
@ -155,15 +153,16 @@ class VolumeViewSet(viewsets.ViewSet):
return Response(serializer.data)
@extend_schema(responses={200: VolumeSerializer, 409: ErrorSerializer})
def retrieve(self, request, storage_pk=None, compute_pk=None, pk=None):
compute = get_object_or_404(Compute, pk=compute_pk)
conn = wvmStorage(
compute.hostname, compute.login, compute.password, compute.type, storage_pk
)
state = conn.is_active()
volume_queryset = conn.get_volume_details(pk) if state else None
if not conn.is_active():
return Response({"detail": _("Pool is not active")}, status=status.HTTP_409_CONFLICT)
volume_queryset = conn.get_volume_details(pk)
conn.close()
serializer = VolumeSerializer(
volume_queryset, many=False, context={"request": request}
@ -171,7 +170,7 @@ class VolumeViewSet(viewsets.ViewSet):
return Response(serializer.data)
@extend_schema(responses={200: StatusSerializer, 400: dict, 409: StatusSerializer})
@extend_schema(responses={200: StatusSerializer, 400: dict, 409: ErrorSerializer})
def create(self, request, storage_pk=None, compute_pk=None):
compute = get_object_or_404(Compute, pk=compute_pk)
@ -200,11 +199,11 @@ class VolumeViewSet(viewsets.ViewSet):
return Response({"status": "Volume: " + ret + " is created"})
else:
failed(request, compute, subject, action, "pool is not active")
return Response({"status": _("Pool is not active")}, status=status.HTTP_409_CONFLICT)
return Response({"detail": _("Pool is not active")}, status=status.HTTP_409_CONFLICT)
else:
return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
@extend_schema(responses={200: StatusSerializer, 409: StatusSerializer})
@extend_schema(responses={200: StatusSerializer, 409: ErrorSerializer})
def destroy(self, request, storage_pk=None, compute_pk=None, pk=None):
compute = get_object_or_404(Compute, pk=compute_pk)
@ -221,4 +220,4 @@ class VolumeViewSet(viewsets.ViewSet):
return Response({"status": "Volume: " + pk + " is deleted"})
else:
failed(request, compute, subject, action, "pool is not active")
return Response({"status": _("Pool is not active")}, status=status.HTTP_409_CONFLICT)
return Response({"detail": _("Pool is not active")}, status=status.HTTP_409_CONFLICT)

View file

@ -70,7 +70,7 @@ class VolumeDeleteTests(TestCase):
conn = mock_wvm.return_value
conn.is_active.return_value = False
conn.get_size.return_value = (1, 1)
conn.del_volume.side_effect = util.OperationError("Volume disk.qcow2 is in use")
conn.del_volume.side_effect = util.InUse("Volume disk.qcow2 is in use")
return conn
@patch("storages.views.wvmStorage")
@ -129,5 +129,15 @@ class VolumeDeleteTests(TestCase):
conn = self.refused(mock_wvm)
conn.is_active.return_value = True
response = self.client.delete(f"/api/v1/computes/{self.compute.id}/storages/default/volumes/disk.qcow2/")
self.assertEqual(response.status_code, 400, response.content)
self.assertEqual(response.status_code, 409, response.content)
self.assertIn("disk.qcow2 is in use", response.json()["detail"])
@patch("storages.api.viewsets.wvmStorage")
def test_the_api_reads_no_volumes_of_a_stopped_pool(self, mock_wvm):
# it answered an empty list, or a made-up volume for any name
mock_wvm.return_value.is_active.return_value = False
base = f"/api/v1/computes/{self.compute.id}/storages/default/volumes/"
for url in (base, base + "anything.qcow2/"):
with self.subTest(url):
response = self.client.get(url)
self.assertEqual((response.status_code, response.json()), (409, {"detail": "Pool is not active"}))

View file

@ -267,7 +267,7 @@ class wvmStorage(wvmConnect):
"""libvirt deletes any volume; one a VM still needs is refused."""
vol = self.pool.storageVolLookupByName(name)
if vol.path() in self.paths_in_use():
raise util.OperationError(gettext_noop("Volume %(name)s is in use: a VM's disk, a backing file or a snapshot's file"), name=name)
raise util.InUse(gettext_noop("Volume %(name)s is in use: a VM's disk, a backing file or a snapshot's file"), name=name)
vol.delete(0)
def get_volume_format_type(self, name):

View file

@ -49,6 +49,10 @@ class OperationError(libvirt.libvirtError):
return render(self.text, self.values, gettext)
class InUse(OperationError):
"""An object something still uses cannot be changed or deleted."""
class ConnectionFailed(OperationError):
"""The host could not be connected to; the message may name it and how it
is reached."""

View file

@ -6,11 +6,13 @@ from django.http import JsonResponse
from django.shortcuts import redirect, render
from django.utils.http import url_has_allowed_host_and_scheme
from django.utils.translation import gettext_lazy as _
import libvirt
from libvirt import libvirtError
from logs.audit import request_ip, write
from operations import claims, jobs
from operations.utils import busy_message, public_error_text, said, summary, words
from vrtManager.util import ConnectionFailed, InUse
from webvirtcloud.log_context import current_request
API_PREFIX = "/api/"
@ -46,6 +48,29 @@ def error_text(request, exception):
return public_error_text(exception)
# libvirt's "no such object" errors
NOT_FOUND_CODES = {
libvirt.VIR_ERR_NO_DOMAIN, libvirt.VIR_ERR_NO_DOMAIN_SNAPSHOT, libvirt.VIR_ERR_NO_STORAGE_POOL,
libvirt.VIR_ERR_NO_STORAGE_VOL, libvirt.VIR_ERR_NO_NETWORK, libvirt.VIR_ERR_NO_INTERFACE,
libvirt.VIR_ERR_NO_NWFILTER, libvirt.VIR_ERR_NO_SECRET, libvirt.VIR_ERR_NO_NODE_DEVICE,
}
def api_status(request, exception):
"""An API client's status for a libvirt error: 404 for an object the host
does not have, 409 for one in use, 503 for a host that cannot be reached
(a client may try again), else 400. Only a read or a delete names the
object not found: an action (POST) may miss another one it needs, as a
VM's removed network, and the VM is there."""
if isinstance(exception, InUse):
return 409
if isinstance(exception, ConnectionFailed):
return 503
if request.method in ("GET", "HEAD", "DELETE") and exception.get_error_code() in NOT_FOUND_CODES:
return 404
return 400
class RequestIdMiddleware:
"""Names the request for its log lines (see log_context) and sends the ID back in X-Request-ID."""
@ -106,7 +131,7 @@ class ExceptionMiddleware:
# API clients get JSON with a failure status, not the HTML error page.
if request.path_info.startswith(API_PREFIX):
if isinstance(exception, libvirtError):
return JsonResponse({"detail": error_text(request, exception)}, status=400)
return JsonResponse({"detail": error_text(request, exception)}, status=api_status(request, exception))
if isinstance(exception, claims.Busy):
return JsonResponse({"detail": busy_message(exception, request.user),
"operation": summary(exception.blocker, request.user) if exception.blocker else None},

View file

@ -7,6 +7,12 @@ class StatusSerializer(serializers.Serializer):
status = serializers.CharField()
class ErrorSerializer(serializers.Serializer):
"""Response body of a refused request: why, as DRF's own errors."""
detail = serializers.CharField()
class ActionStatusSerializer(StatusSerializer):
"""A VM action's status and what the web page would have shown after it,
such as a disk that was kept or a template that was not started."""

View file

@ -3,6 +3,7 @@ import os
from django.contrib.auth import get_user_model
from django.core.management import call_command
from django.test import TestCase
from drf_spectacular.generators import SchemaGenerator
from rest_framework.response import Response
from rest_framework.test import APIClient, APIRequestFactory
from rest_framework.views import APIView
@ -21,6 +22,26 @@ class APISchemaTestCase(TestCase):
self.assertEqual(client.get("/api/v1/computes/abc/instances/").status_code, 404)
self.assertEqual(client.get("/api/v1/instances/abc/").status_code, 404)
def test_an_api_url_that_names_nothing_is_a_json_404_after_authentication(self):
# it was the HTML error page
client = APIClient()
self.assertEqual(client.get("/api/v1/nothere/").status_code, 401)
client.force_login(get_user_model().objects.create_user("schema_user", password="pw"))
for url in ("/api/v1/nothere/", "/api/v1/nothere", "/api/v1/instances/1/x/", "/api/", "/api/v1/", "/api/nope/"):
with self.subTest(url):
response = client.get(url, follow=True)
self.assertEqual((response.status_code, response.json()), (404, {"detail": "Not found."}))
for method in ("head", "options"):
with self.subTest(method):
self.assertEqual(getattr(client, method)("/api/v1/nothere/").status_code, 404)
self.assertEqual(client.get("/api/schema/").status_code, 200)
def test_a_refusal_s_body_is_documented_with_its_409(self):
schema = SchemaGenerator().get_schema(request=None, public=True)
poweron = schema["paths"]["/api/v1/computes/{compute_pk}/instances/{id}/poweron/"]["post"]
self.assertEqual(poweron["responses"]["409"]["content"]["application/json"]["schema"]["$ref"],
"#/components/schemas/Error")
class SchemaPermissionTestCase(TestCase):
def test_schema_requires_login(self):

View file

@ -206,10 +206,14 @@ class AuthorizationMatrixTestCase(TestCase):
data["sshkeyid"] = key.pk
response = self.client.post(url, data, HTTP_REFERER=f"/instances/{self.instance.pk}/")
if response.status_code >= 500:
# Only the fixture's explicit libvirt failure is an acceptable
# authorization outcome. Unexpected failures must fail the test.
self.assertEqual(response.status_code, 500)
self.assertTrue(response.context and response.context.get("libvirt_error"))
# Only the fixture's explicit libvirt failure (its host is not
# reached) is an acceptable authorization outcome: the page's error
# page, the API's 503. Unexpected failures must fail the test.
if url.startswith("/api/"):
self.assertEqual(response.status_code, 503)
else:
self.assertEqual(response.status_code, 500)
self.assertTrue(response.context and response.context.get("libvirt_error"))
return response
def _url(self, name, params):

View file

@ -11,7 +11,7 @@ from django.contrib.messages import get_messages
from django.contrib.messages.storage.fallback import FallbackStorage
from django.contrib.sessions.middleware import SessionMiddleware
from django.test import RequestFactory, TestCase
from vrtManager.util import ConnectionFailed
from vrtManager.util import ConnectionFailed, InUse, OperationError
from webvirtcloud.middleware import ExceptionMiddleware, error_text
HOST_DETAIL = "ssh: connect to host 10.0.0.9 port 22: No route to host"
@ -90,11 +90,25 @@ class ConnectionErrorPageTests(TestCase):
def test_api_answers_the_same_way(self):
response, _ = self.handle(self.owner, ConnectionFailed(f"Connection Failed: {HOST_DETAIL}"), path="/api/v1/instances/1/")
self.assertEqual(response.status_code, 400)
self.assertEqual(response.status_code, 503) # a client may try again
self.assertNotIn(b"10.0.0.9", response.content)
response, _ = self.handle(self.admin, ConnectionFailed(f"Connection Failed: {HOST_DETAIL}"), path="/api/v1/instances/1/")
self.assertIn(b"10.0.0.9", response.content)
def test_the_api_s_status_says_what_kind_of_error_it_is(self):
for error, expected, method in (
(libvirt_error(libvirt.VIR_FROM_STORAGE, "Storage pool not found", libvirt.VIR_ERR_NO_STORAGE_POOL), 404, "get"),
(libvirt_error(libvirt.VIR_FROM_QEMU, "Domain not found", libvirt.VIR_ERR_NO_DOMAIN), 404, "delete"),
# starting a VM whose network was removed: the VM is there
(libvirt_error(libvirt.VIR_FROM_NETWORK, "Network not found", libvirt.VIR_ERR_NO_NETWORK), 400, "post"),
(InUse("Volume %(name)s is in use", name="a.qcow2"), 409, "delete"),
(libvirt_error(libvirt.VIR_FROM_QEMU, "Requested operation is not valid", libvirt.VIR_ERR_OPERATION_INVALID), 400, "post"),
(OperationError("Invalid cache mode"), 400, "post"),
):
with self.subTest(str(error)):
response, _ = self.handle(self.admin, error, path="/api/v1/instances/1/", method=method)
self.assertEqual(response.status_code, expected)
def test_views_that_report_the_error_themselves(self):
request = RequestFactory().post("/")
request.user = self.owner

View file

@ -5,10 +5,24 @@ from drf_spectacular.views import (
SpectacularRedocView,
SpectacularSwaggerSplitView,
)
from drf_spectacular.utils import extend_schema
from rest_framework.decorators import api_view
from rest_framework.exceptions import NotFound
from appsettings.views import appsettings
from console.views import console
from instances.views import index
@extend_schema(exclude=True)
@api_view(["GET", "HEAD", "OPTIONS", "POST", "PUT", "PATCH", "DELETE"])
def api_not_found(request):
"""An API URL that names nothing: JSON, after the API's authentication, as
every other API answer. Only one with a trailing slash: one without is
redirected to it first."""
raise NotFound
urlpatterns = [
path("", index, name="index"),
path("admin/", include(("admin.urls", "admin"), namespace="admin")),
@ -26,6 +40,7 @@ urlpatterns = [
re_path(r"^swagger\.(?P<format>json|yaml)$", SpectacularAPIView.as_view(), name="schema-json"),
path("swagger/", SpectacularSwaggerSplitView.as_view(url_name="schema"), name="schema-swagger-ui"),
path("redoc/", SpectacularRedocView.as_view(url_name="schema"), name="schema-redoc"),
re_path(r"^api/(?:.*/)?$", api_not_found),
]
if settings.DEBUG: