From 75fb7cd2a463a91bfe059dea462a89c8f7bfab04 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 28 May 2024 09:06:56 +0200 Subject: [PATCH] image: only allow tweaks to /, /boot for now The "images" library does not support custom mount points for bootc based images just yet. The reason is that images will generate an osbuild manifest that contains all the "mounts" for the generated disk. This means that with an extra partition like `/var/log` this is visible for the "bootc install-to-filesystem" stage. And that will trip up bootc because it validates the content of the target directory. Example error with `/var/log` as a custom mount point: ``` ... Installing image: docker://quay.io/centos-bootc/centos-bootc:stream9 ERROR Installing to filesystem: Verifying empty rootfs: Non-empty root filesystem; found "var" Traceback (most recent call last): File "/run/osbuild/bin/org.osbuild.bootc.install-to-filesystem", line 53, in r = main(args["options"], args["inputs"], args["paths"]) File "/run/osbuild/bin/org.osbuild.bootc.install-to-filesystem", line 48, in main subprocess.run(pargs, env=env, check=True) File "/usr/lib64/python3.9/subprocess.py", line 528, in run raise CalledProcessError(retcode, process.args, subprocess.CalledProcessError: Command '['bootc', 'install', 'to-filesystem', '--source-imgref', 'containers-storage:[overlay@/run/osbuild/containers/storage+/run/containers/storage]3b612dd1fae2437c00ae3187d0e63daa7a94711560fb1712389edd4121668c96', '--skip-fetch-check', '--generic-image', '--karg', 'rw', '--karg', 'console=tty0', '--karg', 'console=ttyS0', '--karg', 'systemd.journald.forward_to_console=1', '--target-imgref', 'quay.io/centos-bootc/centos-bootc:stream9', '/run/osbuild/mounts']' returned non-zero exit status 1. ``` So AFAICT "images" need sto be changed so that: 1. The "install-to-filesystem" stage only takes the "essential" mounts (/, /boot/, /boot/efi) 2. After "install-to-filesystem" ran we need a "org.osbuild.mkdir" stage for the extra mount points that also only mounts the "essential" mounts As a first step on the journy this commit limits customizations to "/" and "/boot" which is already very useful as many people have asked for precisely those. --- bib/cmd/bootc-image-builder/image.go | 32 +++++++++++---- bib/cmd/bootc-image-builder/image_test.go | 50 ++++++++++++++++++++++- test/test_build.py | 8 +--- 3 files changed, 73 insertions(+), 17 deletions(-) diff --git a/bib/cmd/bootc-image-builder/image.go b/bib/cmd/bootc-image-builder/image.go index c9d9229a..96244dae 100644 --- a/bib/cmd/bootc-image-builder/image.go +++ b/bib/cmd/bootc-image-builder/image.go @@ -3,6 +3,7 @@ package main import ( cryptorand "crypto/rand" "fmt" + "golang.org/x/exp/slices" "math" "math/big" "math/rand" @@ -73,6 +74,24 @@ func Manifest(c *ManifestConfig) (*manifest.Manifest, error) { } } +func applyFilesystemCustomizations(customizations *blueprint.Customizations, c *ManifestConfig) error { + // Check the filesystem customizations against the policy and set + // if user configured + if err := blueprint.CheckMountpointsPolicy(customizations.GetFilesystems(), policies.OstreeMountpointPolicies); err != nil { + return err + } + if customFS := customizations.GetFilesystems(); len(customFS) != 0 { + allowedMounts := []string{"/", "/boot"} + for _, mp := range customFS { + if !slices.Contains(allowedMounts, mp.Mountpoint) { + return fmt.Errorf("cannot use custom mount points yet, found: %v", mp.Mountpoint) + } + } + c.Filesystems = customFS + } + return nil +} + func manifestForDiskImage(c *ManifestConfig, rng *rand.Rand) (*manifest.Manifest, error) { if c.Imgref == "" { return nil, fmt.Errorf("pipeline: no base image defined") @@ -122,11 +141,6 @@ func manifestForDiskImage(c *ManifestConfig, rng *rand.Rand) (*manifest.Manifest img.KernelOptionsAppend = append(img.KernelOptionsAppend, kopts.Append) } - // Check the filesystem customizations against the policy - if err := blueprint.CheckMountpointsPolicy(customizations.GetFilesystems(), policies.OstreeMountpointPolicies); err != nil { - return nil, err - } - basept, ok := partitionTables[c.Architecture.String()] if !ok { return nil, fmt.Errorf("pipelines: no partition tables defined for %s", c.Architecture) @@ -143,12 +157,12 @@ func manifestForDiskImage(c *ManifestConfig, rng *rand.Rand) (*manifest.Manifest } rootFS.Type = c.RootFSType } - filesystems := customizations.GetFilesystems() - if len(filesystems) == 0 { - filesystems = c.Filesystems + + if err := applyFilesystemCustomizations(customizations, c); err != nil { + return nil, err } - pt, err := disk.NewPartitionTable(&basept, filesystems, DEFAULT_SIZE, disk.RawPartitioningMode, nil, rng) + pt, err := disk.NewPartitionTable(&basept, c.Filesystems, DEFAULT_SIZE, disk.RawPartitioningMode, nil, rng) if err != nil { return nil, err } diff --git a/bib/cmd/bootc-image-builder/image_test.go b/bib/cmd/bootc-image-builder/image_test.go index 943bc4bf..43e6d266 100644 --- a/bib/cmd/bootc-image-builder/image_test.go +++ b/bib/cmd/bootc-image-builder/image_test.go @@ -4,11 +4,13 @@ import ( "fmt" "testing" - "github.com/osbuild/images/pkg/manifest" - "github.com/osbuild/images/pkg/runner" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/osbuild/images/pkg/blueprint" + "github.com/osbuild/images/pkg/manifest" + "github.com/osbuild/images/pkg/runner" + bib "github.com/osbuild/bootc-image-builder/bib/cmd/bootc-image-builder" "github.com/osbuild/bootc-image-builder/bib/internal/source" ) @@ -55,3 +57,47 @@ func TestGetDistroAndRunner(t *testing.T) { }) } } + +func TestApplyFilesystemCustomizationsValidates(t *testing.T) { + for _, tc := range []struct { + fsCust []blueprint.FilesystemCustomization + expectedErr string + }{ + // happy + { + fsCust: []blueprint.FilesystemCustomization{}, + expectedErr: "", + }, + { + fsCust: []blueprint.FilesystemCustomization{ + {Mountpoint: "/"}, {Mountpoint: "/boot"}, + }, + expectedErr: "", + }, + // sad + { + fsCust: []blueprint.FilesystemCustomization{ + {Mountpoint: "/"}, + {Mountpoint: "/ostree"}, + }, + expectedErr: `The following custom mountpoints are not supported ["/ostree"]`, + }, + { + fsCust: []blueprint.FilesystemCustomization{ + {Mountpoint: "/"}, + {Mountpoint: "/var/log"}, + }, + expectedErr: "cannot use custom mount points yet, found: /var/log", + }, + } { + conf := &bib.ManifestConfig{} + cust := &blueprint.Customizations{ + Filesystem: tc.fsCust, + } + if tc.expectedErr == "" { + assert.NoError(t, bib.ApplyFilesystemCustomizations(cust, conf)) + } else { + assert.ErrorContains(t, bib.ApplyFilesystemCustomizations(cust, conf), tc.expectedErr) + } + } +} diff --git a/test/test_build.py b/test/test_build.py index 7e183c85..da27fbc6 100644 --- a/test/test_build.py +++ b/test/test_build.py @@ -222,10 +222,6 @@ def build_images(shared_tmpdir, build_container, request, force_aws_upload): "mountpoint": "/", "minsize": "12GiB" }, - { - "mountpoint": "/var/log", - "minsize": "1GiB" - }, ], "kernel": { "append": kargs, @@ -381,13 +377,13 @@ def test_image_boots(image_type): # XXX: read the fully yaml instead? assert f"image: {image_type.container_ref}" in output - # Figure out how big / is and make sure it is > 10GiB + # Figure out how big / is and make sure it is > 11bGiB # Note that df output is in 1k blocks, not bytes for line in output.splitlines(): fields = line.split() if fields[0] == "/sysroot": size = int(fields[1]) - assert size > 10 * 1024 * 1024 + assert size > 11 * 1024 * 1024 break