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

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

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)

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

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