Skip to content

Commit

Permalink
image: only allow tweaks to /, /boot for now
Browse files Browse the repository at this point in the history
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 <module>
    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.
  • Loading branch information
mvo5 committed May 28, 2024
1 parent 191a221 commit 75fb7cd
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 17 deletions.
32 changes: 23 additions & 9 deletions bib/cmd/bootc-image-builder/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main
import (
cryptorand "crypto/rand"
"fmt"
"golang.org/x/exp/slices"
"math"
"math/big"
"math/rand"
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand All @@ -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
}
Expand Down
50 changes: 48 additions & 2 deletions bib/cmd/bootc-image-builder/image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
}
}
}
8 changes: 2 additions & 6 deletions test/test_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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


Expand Down

0 comments on commit 75fb7cd

Please sign in to comment.