Merge ~cgrabowski/maas:bootresource_form_validation_fix into maas:master

Proposed by Christian Grabowski
Status: Merged
Approved by: Christian Grabowski
Approved revision: 3512836c56ca532bdb6e73fc157b33f8fbadc232
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~cgrabowski/maas:bootresource_form_validation_fix
Merge into: maas:master
Diff against target: 178 lines (+80/-9)
4 files modified
src/maasserver/forms/__init__.py (+17/-4)
src/maasserver/forms/tests/test_bootresource.py (+40/-5)
src/maasserver/models/bootresource.py (+11/-0)
src/maasserver/models/tests/test_bootresource.py (+12/-0)
Reviewer Review Type Date Requested Status
MAAS Lander Approve
Alexsander de Souza Approve
Review via email: mp+417702@code.launchpad.net

Commit message

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

return commissioning OS as base image for older images without one

To post a comment you must log in.
Revision history for this message
MAAS Lander (maas-lander) wrote :

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
Revision history for this message
MAAS Lander (maas-lander) wrote :

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
Revision history for this message
Alexsander de Souza (alexsander-souza) wrote :

+1

review: Approve
Revision history for this message
Christian Grabowski (cgrabowski) wrote :

jenkins: !tests

Revision history for this message
MAAS Lander (maas-lander) wrote :

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
Revision history for this message
MAAS Lander (maas-lander) wrote :

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
Revision history for this message
MAAS Lander (maas-lander) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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):
2511 return name2511 return name
25122512
2513 def _get_base_image_info(self):2513 def _get_base_image_info(self):
2514 base_image = self.cleaned_data["base_image"]2514 base_image = self.cleaned_data.get("base_image")
2515 if not base_image:2515 if not base_image:
2516 existing_resource = super().save(commit=False)2516 existing_resource = super().save(commit=False)
2517 existing_resource.name = self.cleaned_data["name"]2517 existing_resource.name = self.cleaned_data["name"]
@@ -2540,9 +2540,22 @@ class BootResourceForm(MAASModelForm):
2540 try:2540 try:
2541 base_osystem, base_version = self._get_base_image_info()2541 base_osystem, base_version = self._get_base_image_info()
2542 except ValueError:2542 except ValueError:
2543 raise ValidationError(2543 if not self.data.get("base_image"):
2544 "custom images require a valid base image name"2544 return "/".join(
2545 )2545 [
2546 val
2547 for val in Config.objects.get_configs(
2548 [
2549 "commissioning_osystem",
2550 "commissioning_distro_series",
2551 ]
2552 ).values()
2553 ]
2554 )
2555 else:
2556 raise ValidationError(
2557 "a base image must follow the format: <osystem>/<series>"
2558 )
2546 else:2559 else:
2547 if base_osystem not in LINUX_OSYSTEMS:2560 if base_osystem not in LINUX_OSYSTEMS:
2548 raise ValidationError(2561 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
1010
11from maasserver.enum import BOOT_RESOURCE_FILE_TYPE, BOOT_RESOURCE_TYPE11from maasserver.enum import BOOT_RESOURCE_FILE_TYPE, BOOT_RESOURCE_TYPE
12from maasserver.forms import BootResourceForm12from maasserver.forms import BootResourceForm
13from maasserver.models import BootResource, BootResourceFile13from maasserver.models import BootResource, BootResourceFile, Config
14from maasserver.models.signals import bootresourcefiles, bootsources14from maasserver.models.signals import bootresourcefiles, bootsources
15from maasserver.testing.architecture import make_usable_architecture15from maasserver.testing.architecture import make_usable_architecture
16from maasserver.testing.factory import factory16from maasserver.testing.factory import factory
@@ -303,7 +303,7 @@ class TestBootResourceForm(MAASServerTestCase):
303 image = form.save()303 image = form.save()
304 self.assertEqual(image.base_image, data["base_image"])304 self.assertEqual(image.base_image, data["base_image"])
305305
306 def test_invalidates_nonexistent_custom_image_base_os(self):306 def test_uses_commissioning_os_for_nonexistent_custom_image_base_os(self):
307 name = "custom/%s" % factory.make_name("name")307 name = "custom/%s" % factory.make_name("name")
308 upload_type, filetype = self.pick_filetype()308 upload_type, filetype = self.pick_filetype()
309 size = random.randint(1024, 2048)309 size = random.randint(1024, 2048)
@@ -315,10 +315,17 @@ class TestBootResourceForm(MAASServerTestCase):
315 "title": factory.make_name("title"),315 "title": factory.make_name("title"),
316 "architecture": make_usable_architecture(self),316 "architecture": make_usable_architecture(self),
317 "filetype": upload_type,317 "filetype": upload_type,
318 "base_image": factory.make_name("invalid"),318 "base_image": "",
319 }319 }
320 form = BootResourceForm(data=data, files={"content": uploaded_file})320 form = BootResourceForm(data=data, files={"content": uploaded_file})
321 self.assertFalse(form.is_valid())321 form.save()
322 cfg = Config.objects.get_configs(
323 ["commissioning_osystem", "commissioning_distro_series"]
324 )
325 self.assertEqual(
326 f"{cfg['commissioning_osystem']}/{cfg['commissioning_distro_series']}",
327 form.instance.base_image,
328 )
322329
323 def test_invalidates_nonexistent_custom_image_base_os_no_prefix(self):330 def test_invalidates_nonexistent_custom_image_base_os_no_prefix(self):
324 name = factory.make_name("name")331 name = factory.make_name("name")
@@ -485,7 +492,7 @@ class TestBootResourceForm(MAASServerTestCase):
485 form = BootResourceForm(data={})492 form = BootResourceForm(data={})
486 self.assertFalse(form.is_valid(), form.errors)493 self.assertFalse(form.is_valid(), form.errors)
487 self.assertEqual(494 self.assertEqual(
488 {"name", "architecture", "filetype", "content", "base_image"},495 {"name", "architecture", "filetype", "content"},
489 form.errors.keys(),496 form.errors.keys(),
490 )497 )
491498
@@ -522,3 +529,31 @@ class TestBootResourceForm(MAASServerTestCase):
522 resource_set__resource=resource529 resource_set__resource=resource
523 ).count(),530 ).count(),
524 )531 )
532
533 def test_clean_base_image_sets_commissioning_osystem_and_distro_series_where_none_is_given(
534 self,
535 ):
536 architecture = make_usable_architecture(self)
537 upload_type, filetype = self.pick_filetype()
538 content = factory.make_string(1024).encode("utf-8")
539 upload_name = factory.make_name("filename")
540 uploaded_file = SimpleUploadedFile(content=content, name=upload_name)
541 data = {
542 "name": f"custom/{factory.make_name()}",
543 "architecture": architecture,
544 "filetype": upload_type,
545 }
546 form = BootResourceForm(data=data, files={"content": uploaded_file})
547 form.save()
548 cfg = Config.objects.get_configs(
549 ["commissioning_osystem", "commissioning_distro_series"]
550 )
551 self.assertEqual(
552 "/".join(
553 [
554 cfg["commissioning_osystem"],
555 cfg["commissioning_distro_series"],
556 ]
557 ),
558 form.instance.base_image,
559 )
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
27from maasserver.models.bootresourceset import BootResourceSet27from maasserver.models.bootresourceset import BootResourceSet
28from maasserver.models.bootsourcecache import BootSourceCache28from maasserver.models.bootsourcecache import BootSourceCache
29from maasserver.models.cleansave import CleanSave29from maasserver.models.cleansave import CleanSave
30from maasserver.models.config import Config
30from maasserver.models.timestampedmodel import now, TimestampedModel31from maasserver.models.timestampedmodel import now, TimestampedModel
31from maasserver.utils.orm import get_first, get_one32from maasserver.utils.orm import get_first, get_one
32from provisioningserver.drivers.osystem import OperatingSystemRegistry33from provisioningserver.drivers.osystem import OperatingSystemRegistry
@@ -583,6 +584,16 @@ class BootResource(CleanSave, TimestampedModel):
583 return self.architecture.split("/")584 return self.architecture.split("/")
584585
585 def split_base_image(self):586 def split_base_image(self):
587 # handle older custom images that may not have a base image
588 if not self.base_image:
589 cfg = Config.objects.get_configs(
590 ["commissioning_osystem", "commissioning_distro_series"]
591 )
592 return (
593 cfg["commissioning_osystem"],
594 cfg["commissioning_distro_series"],
595 )
596
586 return self.base_image.split("/")597 return self.base_image.split("/")
587598
588 def get_next_version_name(self):599 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 (
21 BootResource,21 BootResource,
22 RTYPE_REQUIRING_OS_SERIES_NAME,22 RTYPE_REQUIRING_OS_SERIES_NAME,
23)23)
24from maasserver.models.config import Config
24from maasserver.models.signals import bootsources25from maasserver.models.signals import bootsources
25from maasserver.testing.factory import factory26from maasserver.testing.factory import factory
26from maasserver.testing.testcase import MAASServerTestCase27from maasserver.testing.testcase import MAASServerTestCase
@@ -1161,3 +1162,14 @@ class TestBootResource(MAASServerTestCase):
1161 self.assertFalse(1162 self.assertFalse(
1162 resource.supports_subarch(factory.make_name("subarch"))1163 resource.supports_subarch(factory.make_name("subarch"))
1163 )1164 )
1165
1166 def test_split_base_image_handles_no_base_image(self):
1167 resource = factory.make_BootResource()
1168 resource.base_image = None
1169 cfg = Config.objects.get_configs(
1170 ["commissioning_osystem", "commissioning_distro_series"]
1171 )
1172 self.assertCountEqual(
1173 (cfg["commissioning_osystem"], cfg["commissioning_distro_series"]),
1174 resource.split_base_image(),
1175 )

Subscribers

People subscribed via source and target branches