From ae1296e33a26b564b96f8e7d763733228da026ed Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Tue, 20 Jul 2021 17:34:04 +0000 Subject: [PATCH] formats/v2: mounts are arrays The order of entries in a dictionary is not specified by the JSON standard and hard to control when marshalling dictionaries in Go. Since the order of mounts is important and the wrong order leads to wrong mount trees change the `mounts` field to an array. This breaks existing manifests but after careful deliberation it was concluded that the original schema with mounts as dictionaries is not something we want to support. Apologies to everyone. Adjust the schema of the copy and zipl stage accordingly. --- osbuild/formats/v2.py | 26 +++++++++++++------ schemas/osbuild2.json | 8 +++--- stages/org.osbuild.copy | 3 +-- stages/org.osbuild.zipl.inst | 11 ++------ .../data/manifests/fedora-ostree-bootiso.json | 14 +++++----- .../manifests/fedora-ostree-bootiso.mpp.json | 14 +++++----- test/data/manifests/fedora-ostree-image.json | 13 ++++++---- .../manifests/fedora-ostree-image.mpp.json | 13 ++++++---- 8 files changed, 57 insertions(+), 45 deletions(-) diff --git a/osbuild/formats/v2.py b/osbuild/formats/v2.py index ec660c0b..8d3efd2a 100644 --- a/osbuild/formats/v2.py +++ b/osbuild/formats/v2.py @@ -75,6 +75,7 @@ def describe(manifest: Manifest, *, with_id=False) -> Dict: def describe_mount(mnt): desc = { + "name": mnt.name, "type": mnt.info.name, "device": mnt.device.name, "target": mnt.target @@ -85,10 +86,10 @@ def describe(manifest: Manifest, *, with_id=False) -> Dict: return desc def describe_mounts(mounts: Dict): - desc = { - name: describe_mount(mnt) - for name, mnt in mounts.items() - } + desc = [ + describe_mount(mnt) + for mnt in mounts.values() + ] return desc def describe_stage(s: Stage): @@ -214,10 +215,15 @@ def load_input(name: str, description: Dict, index: Index, stage: Stage, manifes ip.add_reference(r, desc) -def load_mount(name: str, description: Dict, index: Index, stage: Stage): +def load_mount(description: Dict, index: Index, stage: Stage): mount_type = description["type"] info = index.get_module_info("Mount", mount_type) + name = description["name"] + + if name in stage.mounts: + raise ValueError(f"Duplicated mount '{name}'") + source = description["source"] target = description["target"] @@ -245,9 +251,9 @@ def load_stage(description: Dict, index: Index, pipeline: Pipeline, manifest: Ma for name, desc in ips.items(): load_input(name, desc, index, stage, manifest, source_refs) - mounts = description.get("mounts", {}) - for name, desc in mounts.items(): - load_mount(name, desc, index, stage) + mounts = description.get("mounts", []) + for mount in mounts: + load_mount(mount, index, stage) return stage @@ -394,6 +400,10 @@ def validate(manifest: Dict, index: Index) -> ValidationResult: def validate_stage_modules(klass, stage, path): group = ModuleInfo.MODULES[klass] items = stage.get(group, {}) + + if isinstance(items, list): + items = {i["name"]: i for i in items} + for name, mod in items.items(): validate_module(mod, klass, path + [group, name]) diff --git a/schemas/osbuild2.json b/schemas/osbuild2.json index 1f19e8f4..5261b751 100644 --- a/schemas/osbuild2.json +++ b/schemas/osbuild2.json @@ -63,16 +63,16 @@ "mounts": { "title": "Collection of mount points for a stage", - "additionalProperties": { - "$ref": "#/definitions/mount" - } + "type": "array", + "items": { "$ref": "#/definitions/mount"} }, "mount": { "title": "Mount point for a stage", "additionalProperties": false, - "required": ["type", "source", "target"], + "required": ["name", "type", "source", "target"], "properties": { + "name": { "type": "string" }, "type": { "type": "string" }, "source": { "type": "string" diff --git a/stages/org.osbuild.copy b/stages/org.osbuild.copy index 15e3cd7d..19330968 100755 --- a/stages/org.osbuild.copy +++ b/stages/org.osbuild.copy @@ -64,8 +64,7 @@ SCHEMA_2 = r""" "additionalProperties": true }, "mounts": { - "type": "object", - "additionalProperties": true + "type": "array" }, "inputs": { "type": "object", diff --git a/stages/org.osbuild.zipl.inst b/stages/org.osbuild.zipl.inst index d8bacff6..f5c9721d 100755 --- a/stages/org.osbuild.zipl.inst +++ b/stages/org.osbuild.zipl.inst @@ -47,15 +47,8 @@ SCHEMA_2 = r""" } }, "mounts": { - "type": "object", - "additionalProperties": true, - "required": ["root"], - "properties": { - "root": { - "type": "object", - "additionalProperties": true - } - } + "type": "array", + "minItems": 1 } """ diff --git a/test/data/manifests/fedora-ostree-bootiso.json b/test/data/manifests/fedora-ostree-bootiso.json index 3f43bb97..3df7747a 100644 --- a/test/data/manifests/fedora-ostree-bootiso.json +++ b/test/data/manifests/fedora-ostree-bootiso.json @@ -1763,13 +1763,14 @@ } } }, - "mounts": { - "root": { + "mounts": [ + { + "name": "root", "type": "org.osbuild.ext4", "source": "root", "target": "/" } - } + ] } ] }, @@ -1935,13 +1936,14 @@ } } }, - "mounts": { - "efi": { + "mounts": [ + { + "name": "efi", "type": "org.osbuild.fat", "source": "efi", "target": "/" } - } + ] }, { "type": "org.osbuild.copy", diff --git a/test/data/manifests/fedora-ostree-bootiso.mpp.json b/test/data/manifests/fedora-ostree-bootiso.mpp.json index b2c55024..732eefe8 100644 --- a/test/data/manifests/fedora-ostree-bootiso.mpp.json +++ b/test/data/manifests/fedora-ostree-bootiso.mpp.json @@ -534,13 +534,14 @@ } } }, - "mounts": { - "root": { + "mounts": [ + { + "name": "root", "type": "org.osbuild.ext4", "source": "root", "target": "/" } - } + ] } ] }, @@ -720,13 +721,14 @@ } } }, - "mounts": { - "efi": { + "mounts": [ + { + "name": "efi", "type": "org.osbuild.fat", "source": "efi", "target": "/" } - } + ] }, { "type": "org.osbuild.copy", diff --git a/test/data/manifests/fedora-ostree-image.json b/test/data/manifests/fedora-ostree-image.json index c1ecb768..18c9e20b 100644 --- a/test/data/manifests/fedora-ostree-image.json +++ b/test/data/manifests/fedora-ostree-image.json @@ -1080,23 +1080,26 @@ } } }, - "mounts": { - "root": { + "mounts": [ + { + "name": "root", "type": "org.osbuild.xfs", "source": "root", "target": "/" }, - "boot": { + { + "name": "boot", "type": "org.osbuild.ext4", "source": "boot", "target": "/boot" }, - "efi": { + { + "name": "efi", "type": "org.osbuild.fat", "source": "efi", "target": "/boot/efi" } - } + ] }, { "type": "org.osbuild.grub2.inst", diff --git a/test/data/manifests/fedora-ostree-image.mpp.json b/test/data/manifests/fedora-ostree-image.mpp.json index 00ad6a60..4b8309c5 100644 --- a/test/data/manifests/fedora-ostree-image.mpp.json +++ b/test/data/manifests/fedora-ostree-image.mpp.json @@ -534,23 +534,26 @@ } } }, - "mounts": { - "root": { + "mounts": [ + { + "name": "root", "type": "org.osbuild.xfs", "source": "root", "target": "/" }, - "boot": { + { + "name": "boot", "type": "org.osbuild.ext4", "source": "boot", "target": "/boot" }, - "efi": { + { + "name": "efi", "type": "org.osbuild.fat", "source": "efi", "target": "/boot/efi" } - } + ] }, { "type": "org.osbuild.grub2.inst",