Merge ~cgrabowski/maas:bootresource_form_validation_fix into maas:master

[Christian Grabowski]

allow arbitrary custom images default to using the commissioning OS for base_image

return commissioning OS as base image for older images without one

[MAAS Lander (maas-lander)]

UNIT TESTS
-b bootresource_form_validation_fix lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/12203/console
COMMIT: 841263a5e802d4ceb2e64f99c2d1ed33d7d04cae

review: Needs Fixing

[MAAS Lander (maas-lander)]

UNIT TESTS
-b bootresource_form_validation_fix lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/12204/console
COMMIT: c20bdadfdae3d100252f6f8d13a50044a58cc2a4

review: Needs Fixing

[Alexsander de Souza (alexsander-souza)]

+1

review: Approve

[Christian Grabowski (cgrabowski)]

jenkins: !tests

[MAAS Lander (maas-lander)]

UNIT TESTS
-b bootresource_form_validation_fix lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/12207/console
COMMIT: c20bdadfdae3d100252f6f8d13a50044a58cc2a4

review: Needs Fixing

[MAAS Lander (maas-lander)]

UNIT TESTS
-b bootresource_form_validation_fix lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/12209/console
COMMIT: a6f5f73ee484ac88b724068663898ee4890f97ea

review: Needs Fixing

[MAAS Lander (maas-lander)]

UNIT TESTS
-b bootresource_form_validation_fix lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 3512836c56ca532bdb6e73fc157b33f8fbadc232

review: Approve

diff --git a/src/maasserver/forms/__init__.py b/src/maasserver/forms/__init__.py
index 8bb1aa3..fa8983d 100644
--- a/src/maasserver/forms/__init__.py
+++ b/src/maasserver/forms/__init__.py
@@ -2511,7 +2511,7 @@ class BootResourceForm(MAASModelForm):
         return name
 
     def _get_base_image_info(self):
-        base_image = self.cleaned_data["base_image"]
+        base_image = self.cleaned_data.get("base_image")
         if not base_image:
             existing_resource = super().save(commit=False)
             existing_resource.name = self.cleaned_data["name"]
@@ -2540,9 +2540,22 @@ class BootResourceForm(MAASModelForm):
         try:
             base_osystem, base_version = self._get_base_image_info()
         except ValueError:
-            raise ValidationError(
-                "custom images require a valid base image name"
-            )
+            if not self.data.get("base_image"):
+                return "/".join(
+                    [
+                        val
+                        for val in Config.objects.get_configs(
+                            [
+                                "commissioning_osystem",
+                                "commissioning_distro_series",
+                            ]
+                        ).values()
+                    ]
+                )
+            else:
+                raise ValidationError(
+                    "a base image must follow the format: <osystem>/<series>"
+                )
         else:
             if base_osystem not in LINUX_OSYSTEMS:
                 raise ValidationError(
diff --git a/src/maasserver/forms/tests/test_bootresource.py b/src/maasserver/forms/tests/test_bootresource.py
index d43592a..4a3c51e 100644
--- a/src/maasserver/forms/tests/test_bootresource.py
+++ b/src/maasserver/forms/tests/test_bootresource.py
@@ -10,7 +10,7 @@ from django.core.files.uploadedfile import SimpleUploadedFile
 
 from maasserver.enum import BOOT_RESOURCE_FILE_TYPE, BOOT_RESOURCE_TYPE
 from maasserver.forms import BootResourceForm
-from maasserver.models import BootResource, BootResourceFile
+from maasserver.models import BootResource, BootResourceFile, Config
 from maasserver.models.signals import bootresourcefiles, bootsources
 from maasserver.testing.architecture import make_usable_architecture
 from maasserver.testing.factory import factory
@@ -303,7 +303,7 @@ class TestBootResourceForm(MAASServerTestCase):
         image = form.save()
         self.assertEqual(image.base_image, data["base_image"])
 
-    def test_invalidates_nonexistent_custom_image_base_os(self):
+    def test_uses_commissioning_os_for_nonexistent_custom_image_base_os(self):
         name = "custom/%s" % factory.make_name("name")
         upload_type, filetype = self.pick_filetype()
         size = random.randint(1024, 2048)
@@ -315,10 +315,17 @@ class TestBootResourceForm(MAASServerTestCase):
             "title": factory.make_name("title"),
             "architecture": make_usable_architecture(self),
             "filetype": upload_type,
-            "base_image": factory.make_name("invalid"),
+            "base_image": "",
         }
         form = BootResourceForm(data=data, files={"content": uploaded_file})
-        self.assertFalse(form.is_valid())
+        form.save()
+        cfg = Config.objects.get_configs(
+            ["commissioning_osystem", "commissioning_distro_series"]
+        )
+        self.assertEqual(
+            f"{cfg['commissioning_osystem']}/{cfg['commissioning_distro_series']}",
+            form.instance.base_image,
+        )
 
     def test_invalidates_nonexistent_custom_image_base_os_no_prefix(self):
         name = factory.make_name("name")
@@ -485,7 +492,7 @@ class TestBootResourceForm(MAASServerTestCase):
         form = BootResourceForm(data={})
         self.assertFalse(form.is_valid(), form.errors)
         self.assertEqual(
-            {"name", "architecture", "filetype", "content", "base_image"},
+            {"name", "architecture", "filetype", "content"},
             form.errors.keys(),
         )
 
@@ -522,3 +529,31 @@ class TestBootResourceForm(MAASServerTestCase):
                 resource_set__resource=resource
             ).count(),
         )
+
+    def test_clean_base_image_sets_commissioning_osystem_and_distro_series_where_none_is_given(
+        self,
+    ):
+        architecture = make_usable_architecture(self)
+        upload_type, filetype = self.pick_filetype()
+        content = factory.make_string(1024).encode("utf-8")
+        upload_name = factory.make_name("filename")
+        uploaded_file = SimpleUploadedFile(content=content, name=upload_name)
+        data = {
+            "name": f"custom/{factory.make_name()}",
+            "architecture": architecture,
+            "filetype": upload_type,
+        }
+        form = BootResourceForm(data=data, files={"content": uploaded_file})
+        form.save()
+        cfg = Config.objects.get_configs(
+            ["commissioning_osystem", "commissioning_distro_series"]
+        )
+        self.assertEqual(
+            "/".join(
+                [
+                    cfg["commissioning_osystem"],
+                    cfg["commissioning_distro_series"],
+                ]
+            ),
+            form.instance.base_image,
+        )
diff --git a/src/maasserver/models/bootresource.py b/src/maasserver/models/bootresource.py
index ee0479b..9389830 100644
--- a/src/maasserver/models/bootresource.py
+++ b/src/maasserver/models/bootresource.py
@@ -27,6 +27,7 @@ from maasserver.fields import JSONObjectField
 from maasserver.models.bootresourceset import BootResourceSet
 from maasserver.models.bootsourcecache import BootSourceCache
 from maasserver.models.cleansave import CleanSave
+from maasserver.models.config import Config
 from maasserver.models.timestampedmodel import now, TimestampedModel
 from maasserver.utils.orm import get_first, get_one
 from provisioningserver.drivers.osystem import OperatingSystemRegistry
@@ -583,6 +584,16 @@ class BootResource(CleanSave, TimestampedModel):
         return self.architecture.split("/")
 
     def split_base_image(self):
+        # handle older custom images that may not have a base image
+        if not self.base_image:
+            cfg = Config.objects.get_configs(
+                ["commissioning_osystem", "commissioning_distro_series"]
+            )
+            return (
+                cfg["commissioning_osystem"],
+                cfg["commissioning_distro_series"],
+            )
+
         return self.base_image.split("/")
 
     def get_next_version_name(self):
diff --git a/src/maasserver/models/tests/test_bootresource.py b/src/maasserver/models/tests/test_bootresource.py
index 62f0145..d2a0b9e 100644
--- a/src/maasserver/models/tests/test_bootresource.py
+++ b/src/maasserver/models/tests/test_bootresource.py
@@ -21,6 +21,7 @@ from maasserver.models.bootresource import (
     BootResource,
     RTYPE_REQUIRING_OS_SERIES_NAME,
 )
+from maasserver.models.config import Config
 from maasserver.models.signals import bootsources
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
@@ -1161,3 +1162,14 @@ class TestBootResource(MAASServerTestCase):
         self.assertFalse(
             resource.supports_subarch(factory.make_name("subarch"))
         )
+
+    def test_split_base_image_handles_no_base_image(self):
+        resource = factory.make_BootResource()
+        resource.base_image = None
+        cfg = Config.objects.get_configs(
+            ["commissioning_osystem", "commissioning_distro_series"]
+        )
+        self.assertCountEqual(
+            (cfg["commissioning_osystem"], cfg["commissioning_distro_series"]),
+            resource.split_base_image(),
+        )