From 999480301e377f94e86d42807ca9133a4fe42adb Mon Sep 17 00:00:00 2001 From: catborise Date: Sat, 24 Oct 2026 00:33:10 +0300 Subject: [PATCH] fix(instances): a new VM's vCPUs, memory and disk size are at least 1 The create form and the API took 0 or a negative number for vCPUs, memory and a new disk's size: the API queued the job (202) and it failed later in libvirt; the page did the same, its formnovalidate skipping the browser's min. The create model's fields now refuse values below 1, so the form shows the error beside its field and the API answers 400 naming it. The form also checks the MAC addresses as the API and the job do (one per NIC at most, each a MAC address), so a bad one is an error on the form instead of a job that fails. Closes #175 --- instances/forms.py | 10 ++++++++++ instances/models.py | 7 ++++--- instances/tests/test_create_form_input.py | 16 ++++++++++++++++ instances/tests/test_create_job.py | 10 ++++++++++ 4 files changed, 40 insertions(+), 3 deletions(-) diff --git a/instances/forms.py b/instances/forms.py index 5b0ddd1b..987bd45b 100644 --- a/instances/forms.py +++ b/instances/forms.py @@ -6,6 +6,7 @@ from django.utils.translation import gettext_lazy as _ from webvirtcloud.settings import QEMU_CONSOLE_LISTENER_ADDRESSES, QEMU_KEYMAPS from .models import CreateInstance, Flavor +from .utils import nic_macs class FlavorForm(forms.ModelForm): @@ -82,3 +83,12 @@ class NewVMForm(forms.ModelForm): ) ) return name + + def clean(self): + # one MAC per NIC at most, each a MAC address, as the job checks + cleaned = super().clean() + try: + nic_macs(cleaned.get("mac") or "", cleaned.get("networks") or "") + except ValueError as err: + self.add_error("mac", str(err)) + return cleaned diff --git a/instances/models.py b/instances/models.py index 16d8df13..9248037a 100644 --- a/instances/models.py +++ b/instances/models.py @@ -1,4 +1,5 @@ from computes.models import Compute +from django.core.validators import MinValueValidator from django.db import models from django.utils.functional import cached_property from django.utils.translation import gettext_lazy as _ @@ -349,12 +350,12 @@ class CreateInstance(models.Model): ) firmware = models.CharField(max_length=64) vcpu = models.IntegerField( - error_messages={"required": _("No VCPU has been entered")} + error_messages={"required": _("No VCPU has been entered")}, validators=[MinValueValidator(1)] ) vcpu_mode = models.CharField(max_length=20, blank=True) disk = models.IntegerField(blank=True) memory = models.IntegerField( - error_messages={"required": _("No RAM size has been entered")} + error_messages={"required": _("No RAM size has been entered")}, validators=[MinValueValidator(1)] ) networks = models.CharField( max_length=256, @@ -368,7 +369,7 @@ class CreateInstance(models.Model): cache_mode = models.CharField( max_length=16, error_messages={"required": _("Please select HDD cache mode")} ) - hdd_size = models.IntegerField(blank=True) + hdd_size = models.IntegerField(blank=True, validators=[MinValueValidator(1)]) meta_prealloc = models.BooleanField(default=False, blank=True) virtio = models.BooleanField(default=True) qemu_ga = models.BooleanField(default=False) diff --git a/instances/tests/test_create_form_input.py b/instances/tests/test_create_form_input.py index c965bd53..9e0a5ff2 100644 --- a/instances/tests/test_create_form_input.py +++ b/instances/tests/test_create_form_input.py @@ -81,6 +81,22 @@ class CreateFormInputTests(TestCase): self.assertTrue(any("Choose one disk source" in str(m) for m in response.context["messages"])) self.assertFalse(Operation.objects.exists()) + def test_impossible_sizes_are_refused_beside_their_fields(self): + _, response = render_create_page(self, valid(vcpu="0", memory="-1", hdd_size="0", storage="default-pool")) + self.assertEqual(response.status_code, 200) + page = html.fromstring(response.content) + for field in ("vm_vcpu", "vm_memory", "vm_hdd_size"): + with self.subTest(field=field): + self.assertIn("greater than or equal to 1", page.get_element_by_id(f"{field}_error").text_content()) + self.assertFalse(Operation.objects.exists()) + + def test_a_bad_mac_is_refused_by_the_form(self): + _, response = render_create_page(self, valid(hdd_size="10", storage="default-pool", + networks="default,isolated", mac="52:54:00:00:00:01,aa:")) + self.assertEqual(response.status_code, 200) + self.assertIn("Invalid MAC address", " ".join(response.context["vm_form"].errors.get("mac", []))) + self.assertFalse(Operation.objects.exists()) + def test_only_the_chosen_disk_source_is_enabled(self): for posted, chosen in (("template", "template"), ("existing", "existing"), ("nonsense", "new")): with self.subTest(posted=posted): diff --git a/instances/tests/test_create_job.py b/instances/tests/test_create_job.py index 46dab46f..76313ebe 100644 --- a/instances/tests/test_create_job.py +++ b/instances/tests/test_create_job.py @@ -55,6 +55,16 @@ class CreateJobTestCase(TransactionTestCase): self.assertEqual(response.status_code, 400) self.assertFalse(Operation.objects.exists()) + def test_impossible_sizes_are_refused_by_the_api(self): + url = f"/api/v1/computes/{self.compute.pk}/instances/create/x86_64/q35/" + for field, value in (("vcpu", 0), ("memory", -512), ("hdd_size", 0)): + with self.subTest(field=field): + response = self.client.post(url, {**BODY, field: value, "submission_key": f"key-min-{field}"}, + content_type="application/json") + self.assertEqual(response.status_code, 400) + self.assertIn(field, response.json()) + self.assertFalse(Operation.objects.exists()) + def test_a_resubmitted_key_with_other_fields_is_a_conflict(self): url = f"/api/v1/computes/{self.compute.pk}/instances/create/x86_64/q35/" self.client.post(url, BODY, content_type="application/json")