From f39ccc85c778aa5486c10f2c083f8feb0acdf9d7 Mon Sep 17 00:00:00 2001 From: Alex Lowe Date: Tue, 21 Apr 2026 15:20:33 -0400 Subject: [PATCH 01/10] feat(Project): Add the ability to accept MBR volumes This adds the ability to accept MBR schemas to the Volume model. It's not fully tested yet but that's the next piece. --- imagecraft/models/__init__.py | 11 +- imagecraft/models/volume.py | 226 +++++++++++++----- imagecraft/pack/gptutil.py | 8 +- tests/integration/conftest.py | 8 + .../invalid-projects/volume-content/error.txt | 5 + .../volume-content/imagecraft.yaml | 41 ++++ tests/integration/services/test_project.py | 83 +++++++ .../valid-projects/simple-mbr/imagecraft.yaml | 48 ++++ .../valid-projects/simple/imagecraft.yaml | 31 +++ tests/unit/models/test_volume.py | 52 ++-- tests/unit/pack/test_gptutil.py | 4 +- tests/unit/pack/test_grubutil.py | 15 +- tests/unit/pack/test_image.py | 8 +- tests/unit/services/test_image.py | 6 +- 14 files changed, 436 insertions(+), 110 deletions(-) create mode 100644 tests/integration/services/invalid-projects/volume-content/error.txt create mode 100644 tests/integration/services/invalid-projects/volume-content/imagecraft.yaml create mode 100644 tests/integration/services/test_project.py create mode 100644 tests/integration/services/valid-projects/simple-mbr/imagecraft.yaml create mode 100644 tests/integration/services/valid-projects/simple/imagecraft.yaml diff --git a/imagecraft/models/__init__.py b/imagecraft/models/__init__.py index cdda7ee3..7b3ffb4e 100644 --- a/imagecraft/models/__init__.py +++ b/imagecraft/models/__init__.py @@ -22,17 +22,24 @@ get_partition_name, ) -from imagecraft.models.volume import FileSystem, Volume, Role, StructureItem +from imagecraft.models.volume import ( + FileSystem, + GPTVolume, + Volume, + Role, + GPTStructureItem, +) from imagecraft.models.grammar import get_grammar_aware_volume_keywords __all__ = [ "FileSystem", "Project", "Platform", + "GPTVolume", "Volume", "VolumeFilesystemsModel", "Role", - "StructureItem", + "GPTStructureItem", "get_partition_name", "get_grammar_aware_volume_keywords", ] diff --git a/imagecraft/models/volume.py b/imagecraft/models/volume.py index 54ab97bf..36a97a56 100644 --- a/imagecraft/models/volume.py +++ b/imagecraft/models/volume.py @@ -22,7 +22,7 @@ import typing import uuid from collections.abc import Collection -from typing import Annotated, Literal, Self, cast +from typing import Annotated, Literal, Self from craft_application.models import ( CraftBaseModel, @@ -38,9 +38,11 @@ ByteSize, Field, StringConstraints, + ValidationInfo, field_validator, model_validator, ) +from pydantic_core import PydanticCustomError MIB = 1 << 20 # 1 MiB (2^20) GIB = 1 << 30 # 1 GiB (2^30) @@ -114,6 +116,13 @@ def _validate_structure_size(value: str) -> str: ] +class MBRPartitionType(str, enum.Enum): + """Supported MBR volume types.""" + + FAT32 = "0C" + LINUX = "83" + + class GptType(str, enum.Enum): """Supported GUID Partition types.""" @@ -161,9 +170,15 @@ class Role(str, enum.Enum): SYSTEM_BOOT = "system-boot" """The partition stores the image's boot assets.""" + SYSTEM_SEED = "system-seed" + """The partition stores the seed used to provision the device.""" + + SYSTEM_SAVE = "system-save" + """The partition stores data preserved across factory resets.""" + class StructureItem(CraftBaseModel): - """Structure item of the image.""" + """A single structure inside a volume.""" name: StructureName = Field( description="The name of the partition.", @@ -182,41 +197,11 @@ class StructureItem(CraftBaseModel): The name is interpreted as a UTF-16 encoded string. """ - id: uuid.UUID | None = Field( - default=None, - description="The partition's unique identifier.", - examples=[ - "6F8C47A6-1C2D-4B35-8B1E-9DE3C4E9E3FF", - "E3B0C442-98FC-1FC0-9B42-9AC7E5BD4B35", - ], - ) - """The partition's unique identifier. - - The identifier must be a unique 32-digit hexadecimal number in the GPT UUID format. - """ - role: Role = Field( description="The partition's purpose in the image.", examples=["system-data", "system-boot"], ) - structure_type: GptType = Field( - alias="type", - description="The type of the partition.", - examples=[ - "0FC63DAF-8483-4772-8E79-3D69D8477DE4", - "EBD0A0A2-B9E5-4433-87C0-68B6B72699C7", - ], - ) - """The type of the partition. - - For GPT partitions, the value must be the standard 32-digit hexadecimal number - associated with the type. - - This is distinct from the ``structure..id`` key, which is unique among - all partitions, regardless of type. - """ - size: StructureSize = Field( description="The size of the partition, in bytes.", examples=["256M", "6G"], @@ -244,24 +229,36 @@ class StructureItem(CraftBaseModel): Labels must be unique to their volume. """ - partition_number: int | None = Field( + content: None = Field( default=None, - description="(Optional) The partition number for this partition.", - ge=1, # GPT partitions are numbered 1-128 - le=128, + deprecated="Imagecraft does not support the content field.", ) - """The partition number for this partition. - If unset, partitions will start at 1 and be read in list order. If set, all - other partitions must also explicitly set their partition number as unique integers. - """ + min_size: None = Field( + default=None, deprecated="Imagecraft does not support the min-size field." + ) + + @field_validator("content", "min_size", mode="before") + @classmethod + def _field_not_supported(cls, value: object, info: ValidationInfo) -> None: + if value is not None: + field_alias = ( + info.field_name.replace("_", "-") + if info.field_name is not None + else "" + ) + raise PydanticCustomError( + "field_not_supported", + "Imagecraft does not support the '{field_alias}' key in volume structures.", + {"field_alias": field_alias}, + ) def __hash__(self) -> int: return hash(self.name) def __eq__(self, other: object) -> bool: if type(other) is type(self): - return self.name == cast(StructureItem, other).name + return self.name == other.name return False @@ -272,16 +269,86 @@ def _set_default_filesystem_label(self) -> Self: return self +class MBRStructureItem(StructureItem): + """An item on an MBR-schema volume.""" + + structure_type: MBRPartitionType = Field( + alias="type", + description="The type of the partition.", + examples=[ + "0C", + "83", + ], + ) + """The type of the partition. + + For GPT partitions, the value must be the standard 32-digit hexadecimal number + associated with the type. + + This is distinct from the ``structure..id`` key, which is unique among + all partitions, regardless of type. + """ + + +class GPTStructureItem(StructureItem): + """An item on a GPT-schema volume.""" + + id: uuid.UUID | None = Field( + default=None, + description="The partition's unique identifier.", + examples=[ + "6F8C47A6-1C2D-4B35-8B1E-9DE3C4E9E3FF", + "E3B0C442-98FC-1FC0-9B42-9AC7E5BD4B35", + ], + ) + """The partition's unique identifier. + + The identifier must be a unique 32-digit hexadecimal number in the GPT UUID format. + """ + + structure_type: GptType = Field( + alias="type", + description="The type of the partition.", + examples=[ + "0FC63DAF-8483-4772-8E79-3D69D8477DE4", + "EBD0A0A2-B9E5-4433-87C0-68B6B72699C7", + ], + ) + """The type of the partition. + + For GPT partitions, the value must be the standard 32-digit hexadecimal number + associated with the type. + + This is distinct from the ``structure..id`` key, which is unique among + all partitions, regardless of type. + """ + + partition_number: int | None = Field( + default=None, + description="(Optional) The partition number for this partition.", + ge=1, # GPT partitions are numbered 1-128 + le=128, + ) + """The partition number for this partition. + + If unset, partitions will start at 1 and be read in list order. If set, all + other partitions must also explicitly set their partition number as unique integers. + """ + + class PartitionSchema(str, enum.Enum): """Supported partition schemas.""" GPT = "gpt" """The GUID partition table (GPT) schema.""" + MBR = "mbr" + """The Master Boot Record (MBR) schema.""" + def _validate_structure_items_partition_numbers( - structures: Collection[StructureItem], -) -> Collection[StructureItem]: + structures: Collection[GPTStructureItem], +) -> Collection[GPTStructureItem]: partition_numbers = {structure.partition_number for structure in structures} # This could be loosened, but it would require us to generate these partition @@ -325,23 +392,46 @@ def _validate_structure_items_partition_numbers( StructureList = Annotated[ - list[StructureItem], + list[GPTStructureItem], AfterValidator(_validate_structure_items_partition_numbers), ] -class Volume(CraftBaseModel): - """Volume defining properties of the image.""" +MBRStructureList = Annotated[list[MBRStructureItem], Field(min_length=1)] + + +class BaseVolume(CraftBaseModel): + """Base class for volume definitions.""" + + @field_validator("structure", mode="after", check_fields=False) + @classmethod + def _validate_no_duplicate_filesystem_labels( + cls, value: list[StructureItem] + ) -> list[StructureItem]: + """Raise ValueError if any two structures share a filesystem label.""" + fs_labels: list[str] = [str(v.filesystem_label) for v in value] + fs_labels_set = set(fs_labels) + + if len(fs_labels_set) == len(fs_labels): + return value + + dupes = [ + item + for item, count in collections.Counter(fs_labels).items() + if count > 1 and item != "" + ] + raise ValueError(f"Duplicate filesystem labels: {dupes}") + + +class GPTVolume(BaseVolume): + """Volume with a GUID Partition Table (GPT) schema.""" volume_schema: Literal[PartitionSchema.GPT] = Field( alias="schema", description="The partitioning schema of the image.", examples=["gpt"], ) - """The partitioning schema of the image. - - Imagecraft currently supports GUID partition tables (GPT). - """ + """The partitioning schema of the image.""" structure: StructureList = Field( min_length=1, @@ -356,18 +446,28 @@ class Volume(CraftBaseModel): image. """ - @field_validator("structure", mode="after") - @classmethod - def _validate_structure(cls, value: StructureList) -> StructureList: - fs_labels: list[str] = [str(v.filesystem_label) for v in value] - fs_labels_set = set(fs_labels) - if len(fs_labels_set) == len(fs_labels): - return value +class MBRVolume(BaseVolume): + """Volume with a Master Boot Record (MBR) schema.""" - dupes = [ - item - for item, count in collections.Counter(fs_labels).items() - if count > 1 and item != "" - ] - raise ValueError(f"Duplicate filesystem labels: {dupes}") + volume_schema: Literal[PartitionSchema.MBR] = Field( + alias="schema", + description="The partitioning schema of the image.", + examples=["mbr"], + ) + """The partitioning schema of the image.""" + + structure: MBRStructureList = Field( + description="The partitions that comprise the image.", + examples=[ + "[{name: ubuntu-seed, type: 0C, filesystem: vfat, role: system-boot, size: 1200M}]" + ], + ) + """The partitions that comprise the image. + + Each entry in the ``structure`` list represents a disk partition in the final + image. + """ + + +Volume = Annotated[GPTVolume | MBRVolume, Field(discriminator="volume_schema")] diff --git a/imagecraft/pack/gptutil.py b/imagecraft/pack/gptutil.py index f358905a..a2af7169 100644 --- a/imagecraft/pack/gptutil.py +++ b/imagecraft/pack/gptutil.py @@ -21,7 +21,7 @@ from craft_cli import CraftError, emit -from imagecraft.models import Role, Volume +from imagecraft.models import GPTVolume, Role from imagecraft.pack import diskutil from imagecraft.subprocesses import run @@ -69,7 +69,7 @@ def _create_gpt_layout( *, imagepath: Path, sector_size: int, - layout: Volume, + layout: GPTVolume, ) -> None: """Partition image. @@ -145,7 +145,7 @@ def secondary_partition_table_size(sector_size: int) -> int: PARTITION_RESERVED_SIZE: int = NON_MBR_START_OFFSET * SECTOR_SIZE_512 -def image_size(sector_size: int, layout: Volume) -> int: +def image_size(sector_size: int, layout: GPTVolume) -> int: """Determine necessary image size in bytes.""" # For now be conservative and replicate safe behavior of reserving the # first 1MiB of the image for partition table. This must be adapted when @@ -161,7 +161,7 @@ def image_size(sector_size: int, layout: Volume) -> int: def create_empty_gpt_image( imagepath: Path, sector_size: int, - layout: Volume, + layout: GPTVolume, ) -> None: """Create a zeroed image file with a GPT partition table, but no filesystems or data. diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index d0fba098..1e539119 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -23,6 +23,14 @@ from imagecraft.cli import register_services +@pytest.fixture(autouse=True) +def enable_features(reset_features): + """Enable both features.""" + from craft_parts import Features # noqa: PLC0415 + + Features(enable_overlay=True, enable_partitions=True) + + @pytest.fixture def project_path( tmp_path: Path, diff --git a/tests/integration/services/invalid-projects/volume-content/error.txt b/tests/integration/services/invalid-projects/volume-content/error.txt new file mode 100644 index 00000000..cbf56ce9 --- /dev/null +++ b/tests/integration/services/invalid-projects/volume-content/error.txt @@ -0,0 +1,5 @@ +2 validation errors for VolumeFilesystemsModel +volumes.disk.mbr.structure.0.content + Imagecraft does not support the 'content' key in volume structures. [type=field_not_supported, input_value=[{'source': '$kernel:dtbs...ssets/', 'target': '/'}], input_type=list] +volumes.disk.mbr.structure.0.min-size + Imagecraft does not support the 'min-size' key in volume structures. [type=field_not_supported, input_value='1M', input_type=str] diff --git a/tests/integration/services/invalid-projects/volume-content/imagecraft.yaml b/tests/integration/services/invalid-projects/volume-content/imagecraft.yaml new file mode 100644 index 00000000..18b28368 --- /dev/null +++ b/tests/integration/services/invalid-projects/volume-content/imagecraft.yaml @@ -0,0 +1,41 @@ +name: simple-project +version: "0.1" +summary: An image that has a `content` key in a volume. +description: | + The `content` key in the volume schema is not supported by Imagecraft. Because it is + supported by other tools like snapd, we explicitly error out when this field is + included in the file. +base: bare +build-base: devel + +platforms: + amd64: + arm64: + riscv64: + +filesystems: + default: + - mount: / + device: (volume/disk/rootfs) + +parts: + my-part: + plugin: nil + +volumes: + disk: + schema: mbr + structure: + - name: ubuntu-seed + role: system-seed + filesystem: vfat + type: 0C + min-size: 1M # Not supported! + size: 1200M + content: # This field is not supported! + # Copied from the pi gadget snap: + # https://github.com/canonical/pi-gadget/blob/24/gadget.yaml + - source: $kernel:dtbs/dtbs/overlays/ + target: /overlays + - source: boot-assets/ + target: / diff --git a/tests/integration/services/test_project.py b/tests/integration/services/test_project.py new file mode 100644 index 00000000..9894cf67 --- /dev/null +++ b/tests/integration/services/test_project.py @@ -0,0 +1,83 @@ +# This file is part of imagecraft. +# +# Copyright 2026 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify it +# under the terms of the GNU General Public License version 3, as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, +# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with this program. If not, see . +"""Integration tests for the project service.""" + +import pathlib + +import pydantic +import pytest +from imagecraft.application import APP_METADATA +from imagecraft.services.project import ImagecraftProjectService + + +@pytest.mark.parametrize( + "project_dir", + [ + pytest.param(path, id=path.name) + for path in sorted((pathlib.Path(__file__).parent / "valid-projects").iterdir()) + ], +) +def test_load_valid_project( + in_project_path: pathlib.Path, + project_dir: pathlib.Path, +): + project_file = in_project_path / "imagecraft.yaml" + project_file.write_text( + (project_dir / "imagecraft.yaml").read_text(), + encoding="utf-8", + ) + + project_service = ImagecraftProjectService( + app=APP_METADATA, + services=None, + project_dir=in_project_path, + ) + project_service.configure(platform=None, build_for=None) + + project_service.get() + + +@pytest.mark.parametrize( + "project_dir", + [ + pytest.param(path, id=path.name) + for path in sorted( + (pathlib.Path(__file__).parent / "invalid-projects").iterdir() + ) + ], +) +def test_load_invalid_project( + in_project_path: pathlib.Path, + project_dir: pathlib.Path, +): + project_file = in_project_path / "imagecraft.yaml" + project_file.write_text( + (project_dir / "imagecraft.yaml").read_text(), + encoding="utf-8", + ) + expected_error = (project_dir / "error.txt").read_text(encoding="utf-8").strip() + + project_service = ImagecraftProjectService( + app=APP_METADATA, + services=None, + project_dir=in_project_path, + ) + project_service.configure(platform=None, build_for=None) + + with pytest.raises(pydantic.ValidationError) as exc_info: + project_service.get() + + assert str(exc_info.value) == expected_error diff --git a/tests/integration/services/valid-projects/simple-mbr/imagecraft.yaml b/tests/integration/services/valid-projects/simple-mbr/imagecraft.yaml new file mode 100644 index 00000000..9348f569 --- /dev/null +++ b/tests/integration/services/valid-projects/simple-mbr/imagecraft.yaml @@ -0,0 +1,48 @@ +name: simple-project +version: "0.1" +summary: An empty image with MBR partitions +description: This image has MBR partitions! Fancy? +base: bare +build-base: devel + +platforms: + amd64: + arm64: + riscv64: + +filesystems: + default: + - mount: / + device: (volume/disk/ubuntu-seed) + +parts: + my-part: + plugin: nil + +volumes: + disk: + # Copied and modified from the Raspberry Pi gadget: + # https://github.com/canonical/pi-gadget/blob/24/gadget.yaml + schema: mbr + structure: + - name: ubuntu-seed + role: system-seed + filesystem: vfat + type: 0C + size: 1200M + - name: ubuntu-boot + role: system-boot + filesystem: vfat + type: 0C + size: 750M + - name: ubuntu-save + role: system-save + filesystem: ext4 + type: "83" + size: 32M + - name: ubuntu-data + role: system-data + filesystem: ext4 + type: "83" + # XXX: make auto-grow to partition + size: 1500M diff --git a/tests/integration/services/valid-projects/simple/imagecraft.yaml b/tests/integration/services/valid-projects/simple/imagecraft.yaml new file mode 100644 index 00000000..b33660e4 --- /dev/null +++ b/tests/integration/services/valid-projects/simple/imagecraft.yaml @@ -0,0 +1,31 @@ +name: simple-project +version: "0.1" +summary: A test image +description: This image exists purely for testing purposes, yo! +base: bare +build-base: devel + +platforms: + amd64: + arm64: + riscv64: + +filesystems: + default: + - mount: / + device: (volume/disk/rootfs) + +parts: + my-part: + plugin: nil + +volumes: + disk: + schema: gpt + structure: + - name: rootfs + role: system-data + type: 0FC63DAF-8483-4772-8E79-3D69D8477DE4 + filesystem: ext4 + filesystem-label: writable + size: 200 M diff --git a/tests/unit/models/test_volume.py b/tests/unit/models/test_volume.py index cc2eee3f..4fab939e 100644 --- a/tests/unit/models/test_volume.py +++ b/tests/unit/models/test_volume.py @@ -23,7 +23,9 @@ def test_volume_valid(): - volume = Volume.model_validate( + volume_adapter = TypeAdapter(Volume) + + volume = volume_adapter.validate_python( { "schema": "gpt", "structure": [ @@ -69,7 +71,7 @@ def test_volume_valid(): ("error_value", "error_class", "volume"), [ ( - "1 validation error for Volume\nschema", + "1 validation error for Volume\n Unable to extract tag", ValidationError, { "structure": [ @@ -83,8 +85,8 @@ def test_volume_valid(): ], }, ), - ( - "1 validation error for Volume\nschema", + pytest.param( + "1 validation error for Volume\n Input tag", ValidationError, { "schema": "", @@ -98,9 +100,10 @@ def test_volume_valid(): } ], }, + id="missing-schema", ), - ( - "1 validation error for Volume\nstructure.0.name\n String should match pattern", + pytest.param( + "1 validation error for Volume\ngpt.structure.0.name\n String should match pattern", ValidationError, { "schema": "gpt", @@ -114,9 +117,10 @@ def test_volume_valid(): } ], }, + id="empty-name", ), ( - "1 validation error for Volume\nstructure.0.name\n String should match pattern", + "1 validation error for Volume\ngpt.structure.0.name\n String should match pattern", ValidationError, { "schema": "gpt", @@ -132,7 +136,7 @@ def test_volume_valid(): }, ), ( - "1 validation error for Volume\nstructure\n List should have at least 1 item after validation, not 0", + "1 validation error for Volume\ngpt.structure\n List should have at least 1 item after validation, not 0", ValidationError, { "schema": "gpt", @@ -140,7 +144,7 @@ def test_volume_valid(): }, ), ( - "1 validation error for Volume\nstructure.0.role\n Input should be 'system-data' or 'system-boot'", + "1 validation error for Volume\ngpt.structure.0.role\n Input should be '", ValidationError, { "schema": "gpt", @@ -156,7 +160,7 @@ def test_volume_valid(): }, ), pytest.param( - "1 validation error for Volume\nstructure\n Value error, Duplicate filesystem labels: ['test']", + "1 validation error for Volume\ngpt.structure\n Value error, Duplicate filesystem labels: ['test']", ValidationError, { "schema": "gpt", @@ -180,7 +184,7 @@ def test_volume_valid(): id="duplicate-values", ), ( - "1 validation error for Volume\nstructure.0.type", + "1 validation error for Volume\ngpt.structure.0.type", ValidationError, { "schema": "gpt", @@ -196,7 +200,7 @@ def test_volume_valid(): }, ), ( - "1 validation error for Volume\nstructure.0.filesystem", + "1 validation error for Volume\ngpt.structure.0.filesystem", ValidationError, { "schema": "gpt", @@ -212,7 +216,7 @@ def test_volume_valid(): }, ), ( - "1 validation error for Volume\nstructure.0.id\n Input should be a valid UUID", + "1 validation error for Volume\ngpt.structure.0.id\n Input should be a valid UUID", ValidationError, { "schema": "gpt", @@ -229,7 +233,7 @@ def test_volume_valid(): }, ), ( - "1 validation error for Volume\nstructure.0.name\n String should have at most 36 characters", + "1 validation error for Volume\ngpt.structure.0.name\n String should have at most 36 characters", ValidationError, { "schema": "gpt", @@ -245,7 +249,7 @@ def test_volume_valid(): }, ), ( - "1 validation error for Volume\nstructure.0.size\n Field required", + "1 validation error for Volume\ngpt.structure.0.size\n Field required", ValidationError, { "schema": "gpt", @@ -260,7 +264,7 @@ def test_volume_valid(): }, ), ( - "1 validation error for Volume\nstructure.0.size\n Value error, size must be expressed in bytes, optionally with M or G unit.", + "1 validation error for Volume\ngpt.structure.0.size\n Value error, size must be expressed in bytes, optionally with M or G unit.", ValidationError, { "schema": "gpt", @@ -276,7 +280,7 @@ def test_volume_valid(): }, ), ( - "1 validation error for Volume\nstructure.0.size\n Value error, size must be expressed in bytes, optionally with M or G unit.", + "1 validation error for Volume\ngpt.structure.0.size\n Value error, size must be expressed in bytes, optionally with M or G unit.", ValidationError, { "schema": "gpt", @@ -292,7 +296,7 @@ def test_volume_valid(): }, ), ( - "1 validation error for Volume\nstructure\n Value error, Duplicate filesystem labels: ['label1']", + "1 validation error for Volume\ngpt.structure\n Value error, Duplicate filesystem labels: ['label1']", ValidationError, { "schema": "gpt", @@ -317,7 +321,7 @@ def test_volume_valid(): }, ), ( - "1 validation error for Volume\nstructure\n Value error, Duplicate filesystem labels: ['test2']", + "1 validation error for Volume\ngpt.structure\n Value error, Duplicate filesystem labels: ['test2']", ValidationError, { "schema": "gpt", @@ -347,13 +351,11 @@ def test_volume_invalid( error_class, volume, ): - def load_volume(volume, raises): - with pytest.raises(raises) as err: - Volume(**volume) - - return str(err.value) + volume_adapter = TypeAdapter(Volume, config={"title": "Volume"}) + with pytest.raises(error_class) as exc_info: + volume_adapter.validate_python(volume) - assert error_value in load_volume(volume, error_class) + assert error_value in str(exc_info.value) @pytest.mark.parametrize( diff --git a/tests/unit/pack/test_gptutil.py b/tests/unit/pack/test_gptutil.py index 1d966937..7640877a 100644 --- a/tests/unit/pack/test_gptutil.py +++ b/tests/unit/pack/test_gptutil.py @@ -16,13 +16,13 @@ import pytest from craft_cli.errors import CraftError -from imagecraft.models import Volume +from imagecraft.models import GPTVolume from imagecraft.pack import diskutil, gptutil @pytest.fixture def volume(): - return Volume.unmarshal( + return GPTVolume.unmarshal( { "schema": "gpt", "structure": [ diff --git a/tests/unit/pack/test_grubutil.py b/tests/unit/pack/test_grubutil.py index d0cf1637..54cc06c7 100644 --- a/tests/unit/pack/test_grubutil.py +++ b/tests/unit/pack/test_grubutil.py @@ -20,6 +20,7 @@ from craft_platforms import DebianArchitecture from imagecraft.errors import ImageError from imagecraft.models import Volume +from imagecraft.models.volume import GPTVolume from imagecraft.pack.chroot import Mount from imagecraft.pack.grubutil import _image_mounts, setup_grub from imagecraft.pack.image import Image @@ -27,7 +28,7 @@ @pytest.fixture def volume(): - return Volume.unmarshal( + return GPTVolume.unmarshal( { "schema": "gpt", "structure": [ @@ -109,7 +110,7 @@ def test_setup_grub(mocker, new_dir, volume, filesystem_mount): ("volume", "arch", "message"), [ ( - Volume.unmarshal( + GPTVolume.unmarshal( { "schema": "gpt", "structure": [ @@ -128,7 +129,7 @@ def test_setup_grub(mocker, new_dir, volume, filesystem_mount): "Skipping GRUB installation because no boot partition was found", ), ( - Volume.unmarshal( + GPTVolume.unmarshal( { "schema": "gpt", "structure": [ @@ -147,7 +148,7 @@ def test_setup_grub(mocker, new_dir, volume, filesystem_mount): "Skipping GRUB installation because no data partition was found", ), ( - Volume.unmarshal( + GPTVolume.unmarshal( { "schema": "gpt", "structure": [ @@ -207,7 +208,7 @@ def test_setup_grub_partitions(mocker, new_dir, volume, arch, emitter, message): [ ( "/dev/loop99", - Volume.unmarshal( + GPTVolume.unmarshal( { "schema": "gpt", "structure": [ @@ -251,7 +252,7 @@ def test_setup_grub_partitions(mocker, new_dir, volume, arch, emitter, message): ), ( "/dev/loop99", - Volume.unmarshal( + GPTVolume.unmarshal( { "schema": "gpt", "structure": [ @@ -303,7 +304,7 @@ def test_image_mounts( [ ( "/dev/loop99", - Volume.unmarshal( + GPTVolume.unmarshal( { "schema": "gpt", "structure": [ diff --git a/tests/unit/pack/test_image.py b/tests/unit/pack/test_image.py index ba441213..c418cc96 100644 --- a/tests/unit/pack/test_image.py +++ b/tests/unit/pack/test_image.py @@ -18,7 +18,7 @@ from unittest.mock import call import pytest -from imagecraft.models import Volume +from imagecraft.models import GPTVolume from imagecraft.pack.image import Image @@ -49,7 +49,7 @@ class TestImage: def test_loopdev(self, mocker, new_dir: Path): mock_run = mocker.patch("imagecraft.pack.image.run", side_effect=run) - volume = Volume.unmarshal( + volume = GPTVolume.unmarshal( { "schema": "gpt", "structure": [ @@ -154,7 +154,7 @@ def test_has_data_partition( new_dir: Path, has_data_partition: bool, # noqa: FBT001 ): - volume = Volume.unmarshal(volume_data) + volume = GPTVolume.unmarshal(volume_data) disk_path = Path(new_dir, "pc.img") disk_path.touch(exist_ok=True) image = Image( @@ -239,7 +239,7 @@ def test_has_boot_partition( new_dir: Path, has_boot_partition: bool, # noqa: FBT001 ): - volume = Volume.unmarshal(volume_data) + volume = GPTVolume.unmarshal(volume_data) disk_path = Path(new_dir, "pc.img") disk_path.touch(exist_ok=True) image = Image( diff --git a/tests/unit/services/test_image.py b/tests/unit/services/test_image.py index 221d84af..4a31b38f 100644 --- a/tests/unit/services/test_image.py +++ b/tests/unit/services/test_image.py @@ -18,7 +18,7 @@ import pytest from craft_application import AppMetadata, ServiceFactory from imagecraft.models import Project, Volume -from imagecraft.models.volume import PartitionSchema, StructureItem +from imagecraft.models.volume import GPTStructureItem, PartitionSchema from imagecraft.services.image import ImageService @@ -51,8 +51,8 @@ def mock_project(): vol = MagicMock(spec=Volume) vol.volume_schema = PartitionSchema.GPT vol.structure = [ - MagicMock(spec=StructureItem, name="efi", partition_number=None), - MagicMock(spec=StructureItem, name="rootfs", partition_number=2), + MagicMock(spec=GPTStructureItem, name="efi", partition_number=None), + MagicMock(spec=GPTStructureItem, name="rootfs", partition_number=2), ] vol.structure[0].name = "efi" vol.structure[1].name = "rootfs" From a9b902358b12148662ce94751fd01e5b483c0037 Mon Sep 17 00:00:00 2001 From: Alex Lowe Date: Tue, 21 Apr 2026 16:27:30 -0400 Subject: [PATCH 02/10] test: add volume tests --- tests/unit/models/test_volume.py | 187 ++++++++++++++++++++++++++++++- 1 file changed, 186 insertions(+), 1 deletion(-) diff --git a/tests/unit/models/test_volume.py b/tests/unit/models/test_volume.py index 4fab939e..1c7a22c3 100644 --- a/tests/unit/models/test_volume.py +++ b/tests/unit/models/test_volume.py @@ -18,7 +18,11 @@ import pytest from imagecraft.models import Role, Volume -from imagecraft.models.volume import StructureList +from imagecraft.models.volume import ( + GPTVolume, + MBRVolume, + StructureList, +) from pydantic import TypeAdapter, ValidationError @@ -462,3 +466,184 @@ def test_structure_list_success(structures: list[dict]): def test_structure_list_errors(structures: list[dict], error_message): with pytest.raises(ValidationError, match=error_message): TypeAdapter(StructureList).validate_python(structures) + + +# --------------------------------------------------------------------------- +# MBRVolume +# --------------------------------------------------------------------------- + +_MBR_SEED = { + "name": "ubuntu-seed", + "role": "system-seed", + "type": "0C", + "filesystem": "vfat", + "size": "1200M", +} +_MBR_DATA = { + "name": "ubuntu-data", + "role": "system-data", + "type": "83", + "filesystem": "ext4", + "size": "1500M", +} + + +@pytest.mark.parametrize( + ("structure_type", "structure"), + [ + ("0C", _MBR_SEED), + ("83", _MBR_DATA), + ], + ids=["fat32", "linux"], +) +def test_mbr_volume_valid(structure_type, structure): + volume_adapter = TypeAdapter(Volume) + volume = volume_adapter.validate_python({"schema": "mbr", "structure": [structure]}) + assert isinstance(volume, MBRVolume) + assert volume.volume_schema == "mbr" + assert volume.structure[0].structure_type == structure_type + + +def test_mbr_volume_invalid_type(): + volume_adapter = TypeAdapter(Volume) + with pytest.raises(ValidationError, match=r"mbr\.structure\.0\.type"): + volume_adapter.validate_python( + { + "schema": "mbr", + "structure": [ + { + "name": "rootfs", + "role": "system-data", + "type": "0FC63DAF-8483-4772-8E79-3D69D8477DE4", + "filesystem": "ext4", + "size": "6G", + } + ], + } + ) + + +def test_mbr_volume_duplicate_filesystem_labels(): + volume_adapter = TypeAdapter(Volume) + with pytest.raises( + ValidationError, + match=re.escape("Value error, Duplicate filesystem labels: ['ubuntu-data']"), + ): + volume_adapter.validate_python( + { + "schema": "mbr", + "structure": [ + {**_MBR_SEED, "filesystem-label": "ubuntu-data"}, + _MBR_DATA, + ], + } + ) + + +# --------------------------------------------------------------------------- +# GPTVolume vs MBRVolume discriminated union +# --------------------------------------------------------------------------- + + +def test_volume_gpt_schema_produces_gpt_volume(): + volume = TypeAdapter(Volume).validate_python( + { + "schema": "gpt", + "structure": [ + { + "name": "rootfs", + "role": "system-data", + "type": "0FC63DAF-8483-4772-8E79-3D69D8477DE4", + "filesystem": "ext4", + "size": "6G", + } + ], + } + ) + assert isinstance(volume, GPTVolume) + + +def test_volume_mbr_schema_produces_mbr_volume(): + volume = TypeAdapter(Volume).validate_python( + {"schema": "mbr", "structure": [_MBR_DATA]} + ) + assert isinstance(volume, MBRVolume) + + +# --------------------------------------------------------------------------- +# content and min-size fields +# --------------------------------------------------------------------------- + +_VALID_GPT_STRUCTURE = { + "name": "rootfs", + "role": "system-data", + "type": "0FC63DAF-8483-4772-8E79-3D69D8477DE4", + "filesystem": "ext4", + "size": "6G", +} +_VALID_MBR_STRUCTURE = _MBR_DATA + + +@pytest.mark.parametrize( + ("schema", "base_structure"), + [ + ("gpt", _VALID_GPT_STRUCTURE), + ("mbr", _VALID_MBR_STRUCTURE), + ], +) +def test_content_null_is_accepted(schema, base_structure): + TypeAdapter(Volume).validate_python( + {"schema": schema, "structure": [{**base_structure, "content": None}]} + ) + + +@pytest.mark.parametrize( + ("schema", "base_structure"), + [ + ("gpt", _VALID_GPT_STRUCTURE), + ("mbr", _VALID_MBR_STRUCTURE), + ], +) +def test_content_non_null_is_rejected(schema, base_structure): + with pytest.raises( + ValidationError, + match="Imagecraft does not support the 'content' key in volume structures.", + ): + TypeAdapter(Volume).validate_python( + { + "schema": schema, + "structure": [ + {**base_structure, "content": [{"source": "boot/", "target": "/"}]} + ], + } + ) + + +@pytest.mark.parametrize( + ("schema", "base_structure"), + [ + ("gpt", _VALID_GPT_STRUCTURE), + ("mbr", _VALID_MBR_STRUCTURE), + ], +) +def test_min_size_null_is_accepted(schema, base_structure): + TypeAdapter(Volume).validate_python( + {"schema": schema, "structure": [{**base_structure, "min-size": None}]} + ) + + +@pytest.mark.parametrize( + ("schema", "base_structure"), + [ + ("gpt", _VALID_GPT_STRUCTURE), + ("mbr", _VALID_MBR_STRUCTURE), + ], +) +def test_min_size_non_null_is_rejected(schema, base_structure): + with pytest.raises( + ValidationError, + match="Imagecraft does not support the 'min-size' key in volume structures.", + ): + TypeAdapter(Volume).validate_python( + {"schema": schema, "structure": [{**base_structure, "min-size": "16M"}]} + ) From c7b12a6befb1e044cff89775bd92743f94c7322d Mon Sep 17 00:00:00 2001 From: Alex Lowe Date: Tue, 21 Apr 2026 17:22:43 -0400 Subject: [PATCH 03/10] feat(volumes): add a HybridVolume schema ("mbr,gpt") --- imagecraft/models/volume.py | 71 +++++++++++++++- .../hybrid-unsupported-fields/error.txt | 5 ++ .../hybrid-unsupported-fields/imagecraft.yaml | 37 +++++++++ .../error.txt | 0 .../imagecraft.yaml | 0 .../simple-hybrid/imagecraft.yaml | 47 +++++++++++ tests/unit/models/test_volume.py | 82 +++++++++++++++++++ 7 files changed, 241 insertions(+), 1 deletion(-) create mode 100644 tests/integration/services/invalid-projects/hybrid-unsupported-fields/error.txt create mode 100644 tests/integration/services/invalid-projects/hybrid-unsupported-fields/imagecraft.yaml rename tests/integration/services/invalid-projects/{volume-content => mbr-unsupported-fields}/error.txt (100%) rename tests/integration/services/invalid-projects/{volume-content => mbr-unsupported-fields}/imagecraft.yaml (100%) create mode 100644 tests/integration/services/valid-projects/simple-hybrid/imagecraft.yaml diff --git a/imagecraft/models/volume.py b/imagecraft/models/volume.py index 36a97a56..bd98362f 100644 --- a/imagecraft/models/volume.py +++ b/imagecraft/models/volume.py @@ -56,6 +56,12 @@ STRUCTURE_NAME_COMPILED_REGEX = PARTITION_COMPILED_STRICT_REGEX STRUCTURE_SIZE_COMPILED_REGEX = re.compile(r"^(?P\d+)\s*(?P[M,G]{0,1})$") +_UUID_PATTERN = ( + r"[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}" +) +_MBR_TYPE_PATTERN = r"(?:0[Cc]|83)" +HYBRID_PARTITION_TYPE_REGEX = re.compile(rf"^{_MBR_TYPE_PATTERN},{_UUID_PATTERN}$") + GPT_NAME_MAX_LENGTH = 36 VOLUME_INVALID_MSG = ( @@ -345,6 +351,9 @@ class PartitionSchema(str, enum.Enum): MBR = "mbr" """The Master Boot Record (MBR) schema.""" + HYBRID = "mbr,gpt" + """A hybrid MBR/GPT schema, providing both partition tables simultaneously.""" + def _validate_structure_items_partition_numbers( structures: Collection[GPTStructureItem], @@ -400,6 +409,41 @@ def _validate_structure_items_partition_numbers( MBRStructureList = Annotated[list[MBRStructureItem], Field(min_length=1)] +HybridPartitionType = Annotated[ + str, StringConstraints(pattern=HYBRID_PARTITION_TYPE_REGEX) +] + + +class HybridStructureItem(StructureItem): + """An item on a hybrid MBR/GPT-schema volume.""" + + id: uuid.UUID | None = Field( + default=None, + description="The partition's unique identifier.", + examples=[ + "6F8C47A6-1C2D-4B35-8B1E-9DE3C4E9E3FF", + ], + ) + """The partition's unique identifier in the GPT table. + + The identifier must be a unique 32-digit hexadecimal number in the GPT UUID format. + """ + + structure_type: HybridPartitionType = Field( + alias="type", + description="The hybrid MBR/GPT type of the partition.", + examples=["0C,C12A7328-F81F-11D2-BA4B-00A0C93EC93B"], + ) + """The hybrid partition type, expressed as ','. + + The MBR component must be a two-digit hex code and the GPT component must be a + standard GUID. + """ + + +HybridStructureList = Annotated[list[HybridStructureItem], Field(min_length=1)] + + class BaseVolume(CraftBaseModel): """Base class for volume definitions.""" @@ -470,4 +514,29 @@ class MBRVolume(BaseVolume): """ -Volume = Annotated[GPTVolume | MBRVolume, Field(discriminator="volume_schema")] +class HybridVolume(BaseVolume): + """Volume with a hybrid MBR/GPT schema.""" + + volume_schema: Literal[PartitionSchema.HYBRID] = Field( + alias="schema", + description="The partitioning schema of the image.", + examples=["mbr,gpt"], + ) + """The partitioning schema of the image.""" + + structure: HybridStructureList = Field( + description="The partitions that comprise the image.", + examples=[ + "[{name: ubuntu-seed, type: 0C,C12A7328-F81F-11D2-BA4B-00A0C93EC93B, filesystem: vfat, role: system-seed, size: 1200M}]" + ], + ) + """The partitions that comprise the image. + + Each entry in the ``structure`` list represents a disk partition in the final + image. + """ + + +Volume = Annotated[ + GPTVolume | MBRVolume | HybridVolume, Field(discriminator="volume_schema") +] diff --git a/tests/integration/services/invalid-projects/hybrid-unsupported-fields/error.txt b/tests/integration/services/invalid-projects/hybrid-unsupported-fields/error.txt new file mode 100644 index 00000000..5e8e9bc0 --- /dev/null +++ b/tests/integration/services/invalid-projects/hybrid-unsupported-fields/error.txt @@ -0,0 +1,5 @@ +2 validation errors for VolumeFilesystemsModel +volumes.disk.mbr,gpt.structure.0.content + Imagecraft does not support the 'content' key in volume structures. [type=field_not_supported, input_value=[{'source': 'boot-assets/', 'target': '/'}], input_type=list] +volumes.disk.mbr,gpt.structure.0.min-size + Imagecraft does not support the 'min-size' key in volume structures. [type=field_not_supported, input_value='1G', input_type=str] diff --git a/tests/integration/services/invalid-projects/hybrid-unsupported-fields/imagecraft.yaml b/tests/integration/services/invalid-projects/hybrid-unsupported-fields/imagecraft.yaml new file mode 100644 index 00000000..9b9e78da --- /dev/null +++ b/tests/integration/services/invalid-projects/hybrid-unsupported-fields/imagecraft.yaml @@ -0,0 +1,37 @@ +name: simple-project +version: "0.1" +summary: An image that uses unsupported volume structure fields in a hybrid MBR/GPT schema. +description: | + The `content` and `min-size` keys in the volume schema are not supported by Imagecraft. + Because they are supported by other tools like snapd, we explicitly error out when these + fields are included in the file. +base: bare +build-base: devel + +platforms: + amd64: + arm64: + riscv64: + +filesystems: + default: + - mount: / + device: (volume/disk/ubuntu-seed) + +parts: + my-part: + plugin: nil + +volumes: + disk: + schema: mbr,gpt + structure: + - name: ubuntu-seed + role: system-seed + filesystem: vfat + type: 0C,C12A7328-F81F-11D2-BA4B-00A0C93EC93B + min-size: 1G # Not supported! + size: 1200M + content: # This field is not supported! + - source: boot-assets/ + target: / diff --git a/tests/integration/services/invalid-projects/volume-content/error.txt b/tests/integration/services/invalid-projects/mbr-unsupported-fields/error.txt similarity index 100% rename from tests/integration/services/invalid-projects/volume-content/error.txt rename to tests/integration/services/invalid-projects/mbr-unsupported-fields/error.txt diff --git a/tests/integration/services/invalid-projects/volume-content/imagecraft.yaml b/tests/integration/services/invalid-projects/mbr-unsupported-fields/imagecraft.yaml similarity index 100% rename from tests/integration/services/invalid-projects/volume-content/imagecraft.yaml rename to tests/integration/services/invalid-projects/mbr-unsupported-fields/imagecraft.yaml diff --git a/tests/integration/services/valid-projects/simple-hybrid/imagecraft.yaml b/tests/integration/services/valid-projects/simple-hybrid/imagecraft.yaml new file mode 100644 index 00000000..b69725dc --- /dev/null +++ b/tests/integration/services/valid-projects/simple-hybrid/imagecraft.yaml @@ -0,0 +1,47 @@ +name: simple-project +version: "0.1" +summary: An empty image with a hybrid MBR/GPT partition schema +description: This image has both MBR and GPT partition tables. +base: bare +build-base: devel + +platforms: + amd64: + arm64: + riscv64: + +filesystems: + default: + - mount: / + device: (volume/disk/ubuntu-seed) + +parts: + my-part: + plugin: nil + +volumes: + disk: + # Copied and modified from the Raspberry Pi gadget. + # https://github.com/canonical/pi-gadget/blob/24/gadget.yaml + schema: mbr,gpt + structure: + - name: ubuntu-seed + role: system-seed + filesystem: vfat + type: 0C,C12A7328-F81F-11D2-BA4B-00A0C93EC93B + size: 1200M + - name: ubuntu-boot + role: system-boot + filesystem: vfat + type: 0C,C12A7328-F81F-11D2-BA4B-00A0C93EC93B + size: 750M + - name: ubuntu-save + role: system-save + filesystem: ext4 + type: 83,0FC63DAF-8483-4772-8E79-3D69D8477DE4 + size: 32M + - name: ubuntu-data + role: system-data + filesystem: ext4 + type: 83,0FC63DAF-8483-4772-8E79-3D69D8477DE4 + size: 1500M diff --git a/tests/unit/models/test_volume.py b/tests/unit/models/test_volume.py index 1c7a22c3..0a0f4262 100644 --- a/tests/unit/models/test_volume.py +++ b/tests/unit/models/test_volume.py @@ -20,6 +20,7 @@ from imagecraft.models import Role, Volume from imagecraft.models.volume import ( GPTVolume, + HybridVolume, MBRVolume, StructureList, ) @@ -647,3 +648,84 @@ def test_min_size_non_null_is_rejected(schema, base_structure): TypeAdapter(Volume).validate_python( {"schema": schema, "structure": [{**base_structure, "min-size": "16M"}]} ) + + +# --------------------------------------------------------------------------- +# HybridVolume +# --------------------------------------------------------------------------- + +_HYBRID_SEED = { + "name": "ubuntu-seed", + "role": "system-seed", + "type": "0C,C12A7328-F81F-11D2-BA4B-00A0C93EC93B", + "filesystem": "vfat", + "size": "1200M", +} +_HYBRID_DATA = { + "name": "ubuntu-data", + "role": "system-data", + "type": "83,0FC63DAF-8483-4772-8E79-3D69D8477DE4", + "filesystem": "ext4", + "size": "1500M", +} + + +@pytest.mark.parametrize( + ("structure_type", "structure"), + [ + ("0C,C12A7328-F81F-11D2-BA4B-00A0C93EC93B", _HYBRID_SEED), + ("83,0FC63DAF-8483-4772-8E79-3D69D8477DE4", _HYBRID_DATA), + ], + ids=["fat32-efi", "linux-linux-data"], +) +def test_hybrid_volume_valid(structure_type, structure): + volume = TypeAdapter(Volume).validate_python( + {"schema": "mbr,gpt", "structure": [structure]} + ) + assert isinstance(volume, HybridVolume) + assert volume.volume_schema == "mbr,gpt" + assert volume.structure[0].structure_type == structure_type + + +def test_hybrid_volume_schema_produces_hybrid_volume(): + volume = TypeAdapter(Volume).validate_python( + {"schema": "mbr,gpt", "structure": [_HYBRID_DATA]} + ) + assert isinstance(volume, HybridVolume) + + +@pytest.mark.parametrize( + "bad_type", + [ + pytest.param("0C", id="mbr-only"), + pytest.param("0FC63DAF-8483-4772-8E79-3D69D8477DE4", id="gpt-only"), + pytest.param( + "FF,0FC63DAF-8483-4772-8E79-3D69D8477DE4", id="invalid-mbr-component" + ), + pytest.param("0C,NOTAGUUID", id="invalid-gpt-component"), + ], +) +def test_hybrid_volume_invalid_type(bad_type): + with pytest.raises(ValidationError, match="String should match pattern"): + TypeAdapter(Volume).validate_python( + { + "schema": "mbr,gpt", + "structure": [{**_HYBRID_DATA, "type": bad_type}], + } + ) + + +def test_hybrid_volume_duplicate_filesystem_labels(): + with pytest.raises( + ValidationError, + match=re.escape("Value error, Duplicate filesystem labels: ['ubuntu-data']"), + ): + TypeAdapter(Volume).validate_python( + { + "schema": "mbr,gpt", + "structure": [ + {**_HYBRID_SEED, "filesystem-label": "ubuntu-data"}, + _HYBRID_DATA, + ], + } + ) From 009248e6b714fc26b12d78a38ecb20e893534c55 Mon Sep 17 00:00:00 2001 From: Alex Lowe Date: Tue, 21 Apr 2026 17:23:19 -0400 Subject: [PATCH 04/10] test: add a gpt-unsupported-fields test case for invalid projects --- .../gpt-unsupported-fields/error.txt | 5 +++ .../gpt-unsupported-fields/imagecraft.yaml | 37 +++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 tests/integration/services/invalid-projects/gpt-unsupported-fields/error.txt create mode 100644 tests/integration/services/invalid-projects/gpt-unsupported-fields/imagecraft.yaml diff --git a/tests/integration/services/invalid-projects/gpt-unsupported-fields/error.txt b/tests/integration/services/invalid-projects/gpt-unsupported-fields/error.txt new file mode 100644 index 00000000..7c70f125 --- /dev/null +++ b/tests/integration/services/invalid-projects/gpt-unsupported-fields/error.txt @@ -0,0 +1,5 @@ +2 validation errors for VolumeFilesystemsModel +volumes.disk.gpt.structure.0.content + Imagecraft does not support the 'content' key in volume structures. [type=field_not_supported, input_value=[{'source': 'boot-assets/', 'target': '/'}], input_type=list] +volumes.disk.gpt.structure.0.min-size + Imagecraft does not support the 'min-size' key in volume structures. [type=field_not_supported, input_value='1G', input_type=str] diff --git a/tests/integration/services/invalid-projects/gpt-unsupported-fields/imagecraft.yaml b/tests/integration/services/invalid-projects/gpt-unsupported-fields/imagecraft.yaml new file mode 100644 index 00000000..90cac0f0 --- /dev/null +++ b/tests/integration/services/invalid-projects/gpt-unsupported-fields/imagecraft.yaml @@ -0,0 +1,37 @@ +name: simple-project +version: "0.1" +summary: An image that uses unsupported volume structure fields in a GPT schema. +description: | + The `content` and `min-size` keys in the volume schema are not supported by Imagecraft. + Because they are supported by other tools like snapd, we explicitly error out when these + fields are included in the file. +base: bare +build-base: devel + +platforms: + amd64: + arm64: + riscv64: + +filesystems: + default: + - mount: / + device: (volume/disk/rootfs) + +parts: + my-part: + plugin: nil + +volumes: + disk: + schema: gpt + structure: + - name: rootfs + role: system-data + type: 0FC63DAF-8483-4772-8E79-3D69D8477DE4 + filesystem: ext4 + min-size: 1G # Not supported! + size: 6G + content: # This field is not supported! + - source: boot-assets/ + target: / From 7b6e51c73d36a36210628ec1b1827c314b4a53e8 Mon Sep 17 00:00:00 2001 From: Alex Lowe Date: Tue, 21 Apr 2026 17:28:58 -0400 Subject: [PATCH 05/10] test: add tests for get_partition_name --- tests/unit/models/test_project.py | 52 +++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tests/unit/models/test_project.py b/tests/unit/models/test_project.py index 7cd71d73..5d426772 100644 --- a/tests/unit/models/test_project.py +++ b/tests/unit/models/test_project.py @@ -23,6 +23,12 @@ from craft_application import util from craft_application.errors import CraftValidationError from imagecraft.models import Platform, Project, VolumeFilesystemsModel +from imagecraft.models.project import get_partition_name +from imagecraft.models.volume import ( + GPTStructureItem, + HybridStructureItem, + MBRStructureItem, +) from pydantic import ValidationError IMAGECRAFT_YAML_GENERIC = """ @@ -596,3 +602,49 @@ def test_volumes_fs_partitions_error(yaml_data, error): with pytest.raises(ValueError, match=error): volume_filesystems.get_partitions() + + +_GPT_STRUCTURE = GPTStructureItem.model_validate( + { + "name": "rootfs", + "role": "system-data", + "type": "0FC63DAF-8483-4772-8E79-3D69D8477DE4", + "filesystem": "ext4", + "size": "6G", + } +) + +_MBR_STRUCTURE = MBRStructureItem.model_validate( + { + "name": "ubuntu-seed", + "role": "system-seed", + "type": "0C", + "filesystem": "vfat", + "size": "1200M", + } +) + +_HYBRID_STRUCTURE = HybridStructureItem.model_validate( + { + "name": "ubuntu-boot", + "role": "system-boot", + "type": "0C,C12A7328-F81F-11D2-BA4B-00A0C93EC93B", + "filesystem": "vfat", + "size": "750M", + } +) + + +@pytest.mark.parametrize( + ("volume_name", "structure", "expected"), + [ + pytest.param("disk", _GPT_STRUCTURE, "volume/disk/rootfs", id="gpt"), + pytest.param("disk", _MBR_STRUCTURE, "volume/disk/ubuntu-seed", id="mbr"), + pytest.param("disk", _HYBRID_STRUCTURE, "volume/disk/ubuntu-boot", id="hybrid"), + pytest.param( + "my-vol", _GPT_STRUCTURE, "volume/my-vol/rootfs", id="volume-name" + ), + ], +) +def test_get_partition_name(volume_name, structure, expected): + assert get_partition_name(volume_name, structure) == expected From b7371ed565605c197860f8a7473711aeee25b0c9 Mon Sep 17 00:00:00 2001 From: Alex Lowe Date: Thu, 23 Apr 2026 10:56:14 -0400 Subject: [PATCH 06/10] fix: tests --- imagecraft/models/volume.py | 9 ++++++--- tests/integration/conftest.py | 8 -------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/imagecraft/models/volume.py b/imagecraft/models/volume.py index bd98362f..db9e3557 100644 --- a/imagecraft/models/volume.py +++ b/imagecraft/models/volume.py @@ -263,8 +263,8 @@ def __hash__(self) -> int: return hash(self.name) def __eq__(self, other: object) -> bool: - if type(other) is type(self): - return self.name == other.name + if type(other) is type(self) and hasattr(other, "name"): + return bool(self.name == other.name) return False @@ -400,7 +400,7 @@ def _validate_structure_items_partition_numbers( return structures -StructureList = Annotated[ +GPTStructureList = Annotated[ list[GPTStructureItem], AfterValidator(_validate_structure_items_partition_numbers), ] @@ -444,6 +444,9 @@ class HybridStructureItem(StructureItem): HybridStructureList = Annotated[list[HybridStructureItem], Field(min_length=1)] +StructureList = GPTStructureList | MBRStructureList | HybridStructureList + + class BaseVolume(CraftBaseModel): """Base class for volume definitions.""" diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 1e539119..d0fba098 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -23,14 +23,6 @@ from imagecraft.cli import register_services -@pytest.fixture(autouse=True) -def enable_features(reset_features): - """Enable both features.""" - from craft_parts import Features # noqa: PLC0415 - - Features(enable_overlay=True, enable_partitions=True) - - @pytest.fixture def project_path( tmp_path: Path, From 458745cbeac3e6846dcf173543bb9b348528d505 Mon Sep 17 00:00:00 2001 From: Alex Lowe Date: Thu, 23 Apr 2026 11:11:14 -0400 Subject: [PATCH 07/10] fix: unit tests --- imagecraft/models/volume.py | 2 +- imagecraft/pack/gptutil.py | 7 +++++-- imagecraft/services/image.py | 4 ++-- imagecraft/subprocesses.py | 1 + tests/integration/conftest.py | 8 ++++++++ tests/integration/services/test_project.py | 4 ++-- 6 files changed, 19 insertions(+), 7 deletions(-) diff --git a/imagecraft/models/volume.py b/imagecraft/models/volume.py index db9e3557..8d9a46cf 100644 --- a/imagecraft/models/volume.py +++ b/imagecraft/models/volume.py @@ -480,7 +480,7 @@ class GPTVolume(BaseVolume): ) """The partitioning schema of the image.""" - structure: StructureList = Field( + structure: GPTStructureList = Field( min_length=1, description="The partitions that comprise the image.", examples=[ diff --git a/imagecraft/pack/gptutil.py b/imagecraft/pack/gptutil.py index a2af7169..2f721ee7 100644 --- a/imagecraft/pack/gptutil.py +++ b/imagecraft/pack/gptutil.py @@ -102,9 +102,12 @@ def _create_gpt_layout( } if structure_item.role == Role.SYSTEM_BOOT.value: partition["bootable"] = None - if structure_item.id: + if hasattr(structure_item, "id") and structure_item.id is not None: partition["uuid"] = str(structure_item.id) - if structure_item.partition_number is not None: + if ( + hasattr(structure_item, "partition_number") + and structure_item.partition_number is not None + ): partition["partition-number"] = str(structure_item.partition_number) partitions.append(partition) start += sectors diff --git a/imagecraft/services/image.py b/imagecraft/services/image.py index 18bf48f7..1c62abc9 100644 --- a/imagecraft/services/image.py +++ b/imagecraft/services/image.py @@ -28,7 +28,7 @@ from craft_cli import CraftError, emit from imagecraft.models import Project -from imagecraft.models.volume import PartitionSchema +from imagecraft.models.volume import GPTVolume, PartitionSchema from imagecraft.pack import gptutil from imagecraft.subprocesses import run @@ -83,7 +83,7 @@ def create_images(self) -> Mapping[str, pathlib.Path]: gptutil.create_empty_gpt_image( imagepath=image_path, sector_size=self._sector_size, - layout=volume, + layout=cast(GPTVolume, volume), ) case _: # Reaching this case is a bug. diff --git a/imagecraft/subprocesses.py b/imagecraft/subprocesses.py index 24eda664..53c9bfee 100644 --- a/imagecraft/subprocesses.py +++ b/imagecraft/subprocesses.py @@ -33,6 +33,7 @@ def run(cmd: str, *args: Any, **kwargs: Any) -> CompletedProcess[str]: "text": True, "check": True, "stdout": PIPE, + "stderr": PIPE, } for key, value in defaults.items(): if key not in kwargs: diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index d0fba098..9cf5f5d3 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -58,3 +58,11 @@ def imagecraft_app( ) register_services() return imagecraft.Imagecraft(app_metadata, service_factory) + + +@pytest.fixture(autouse=True) +def enable_features(reset_features): + """Enable both features.""" + from craft_parts import Features # noqa: PLC0415 + + Features(enable_overlay=True, enable_partitions=True) diff --git a/tests/integration/services/test_project.py b/tests/integration/services/test_project.py index 9894cf67..32676bfa 100644 --- a/tests/integration/services/test_project.py +++ b/tests/integration/services/test_project.py @@ -42,7 +42,7 @@ def test_load_valid_project( project_service = ImagecraftProjectService( app=APP_METADATA, - services=None, + services=None, # ty: ignore[invalid-argument-type] project_dir=in_project_path, ) project_service.configure(platform=None, build_for=None) @@ -72,7 +72,7 @@ def test_load_invalid_project( project_service = ImagecraftProjectService( app=APP_METADATA, - services=None, + services=None, # ty: ignore[invalid-argument-type] project_dir=in_project_path, ) project_service.configure(platform=None, build_for=None) From 9688690eadc3f1e40225a31e2299681c85ae7ccd Mon Sep 17 00:00:00 2001 From: Alex Lowe Date: Thu, 23 Apr 2026 11:22:54 -0400 Subject: [PATCH 08/10] fix: docs --- docs/reference/imagecraft-yaml.rst | 20 ++++++++++---------- imagecraft/models/__init__.py | 1 + 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/docs/reference/imagecraft-yaml.rst b/docs/reference/imagecraft-yaml.rst index 057e8852..6bd8a78f 100644 --- a/docs/reference/imagecraft-yaml.rst +++ b/docs/reference/imagecraft-yaml.rst @@ -74,10 +74,10 @@ partitions. .. kitbash-field:: Project volumes :override-type: dict[str, Volume] -.. kitbash-field:: Volume volume_schema +.. kitbash-field:: GPTVolume volume_schema :prepend-name: volumes. -.. kitbash-field:: Volume structure +.. kitbash-field:: GPTVolume structure :prepend-name: volumes. :override-type: list[Partition] @@ -88,28 +88,28 @@ Partition keys The following keys can be declared for each partition listed in the volume's ``structure`` key. -.. kitbash-field:: StructureItem name +.. kitbash-field:: GPTStructureItem name :prepend-name: volumes..structure. -.. kitbash-field:: StructureItem id +.. kitbash-field:: GPTStructureItem id :prepend-name: volumes..structure. -.. kitbash-field:: StructureItem role +.. kitbash-field:: GPTStructureItem role :prepend-name: volumes..structure. -.. kitbash-field:: StructureItem structure_type +.. kitbash-field:: GPTStructureItem structure_type :prepend-name: volumes..structure. -.. kitbash-field:: StructureItem size +.. kitbash-field:: GPTStructureItem size :prepend-name: volumes..structure. -.. kitbash-field:: StructureItem filesystem +.. kitbash-field:: GPTStructureItem filesystem :prepend-name: volumes..structure. -.. kitbash-field:: StructureItem filesystem_label +.. kitbash-field:: GPTStructureItem filesystem_label :prepend-name: volumes..structure. -.. kitbash-field:: StructureItem partition_number +.. kitbash-field:: GPTStructureItem partition_number :prepend-name: volumes..structure. diff --git a/imagecraft/models/__init__.py b/imagecraft/models/__init__.py index 7b3ffb4e..0ea32e0b 100644 --- a/imagecraft/models/__init__.py +++ b/imagecraft/models/__init__.py @@ -24,6 +24,7 @@ from imagecraft.models.volume import ( FileSystem, + BaseVolume, GPTVolume, Volume, Role, From 63c0336f862d32c3ee267762c726183590443ad1 Mon Sep 17 00:00:00 2001 From: Alex Lowe Date: Thu, 23 Apr 2026 11:30:04 -0400 Subject: [PATCH 09/10] fix: requires-root integration tests --- tests/integration/conftest.py | 8 -------- tests/integration/services/test_image.py | 2 ++ tests/integration/services/test_project.py | 2 ++ 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 9cf5f5d3..d0fba098 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -58,11 +58,3 @@ def imagecraft_app( ) register_services() return imagecraft.Imagecraft(app_metadata, service_factory) - - -@pytest.fixture(autouse=True) -def enable_features(reset_features): - """Enable both features.""" - from craft_parts import Features # noqa: PLC0415 - - Features(enable_overlay=True, enable_partitions=True) diff --git a/tests/integration/services/test_image.py b/tests/integration/services/test_image.py index 63d0718a..2b3a8d23 100644 --- a/tests/integration/services/test_image.py +++ b/tests/integration/services/test_image.py @@ -17,6 +17,8 @@ from craft_application import ServiceFactory from imagecraft.services.image import ImageService +pytestmark = [pytest.mark.usefixtures("enable_features")] + @pytest.fixture def image_service(default_factory: ServiceFactory, enable_features): diff --git a/tests/integration/services/test_project.py b/tests/integration/services/test_project.py index 32676bfa..8d228092 100644 --- a/tests/integration/services/test_project.py +++ b/tests/integration/services/test_project.py @@ -22,6 +22,8 @@ from imagecraft.application import APP_METADATA from imagecraft.services.project import ImagecraftProjectService +pytestmark = [pytest.mark.usefixtures("enable_features")] + @pytest.mark.parametrize( "project_dir", From 9afbe44d3242b6fa4b45ef36242c5042b80b6bd9 Mon Sep 17 00:00:00 2001 From: Alex Lowe Date: Thu, 23 Apr 2026 14:03:18 -0400 Subject: [PATCH 10/10] Update imagecraft/models/volume.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Alex Lowe --- imagecraft/models/volume.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/imagecraft/models/volume.py b/imagecraft/models/volume.py index 8d9a46cf..997f60cb 100644 --- a/imagecraft/models/volume.py +++ b/imagecraft/models/volume.py @@ -286,13 +286,10 @@ class MBRStructureItem(StructureItem): "83", ], ) - """The type of the partition. - - For GPT partitions, the value must be the standard 32-digit hexadecimal number - associated with the type. + """The MBR partition type code. - This is distinct from the ``structure..id`` key, which is unique among - all partitions, regardless of type. + For MBR partitions, the value is the partition type byte written as a + hexadecimal code, such as ``0C`` for FAT32 or ``83`` for Linux. """