osbuild: extract genMountsDevicesFromPt helper

internal/testdisk/partition.go

@@ -0,0 +1,34 @@
+package testdisk
+
+import (
+	"github.com/osbuild/images/pkg/disk"
+)
+
+// MakeFakePartitionTable is a helper to create partition table structs
+// for tests. It uses sensible defaults for common scenarios.
+func MakeFakePartitionTable(mntPoints ...string) *disk.PartitionTable {
+	var partitions []disk.Partition
+	for _, mntPoint := range mntPoints {
+		payload := &disk.Filesystem{
+			Type:       "ext4",
+			Mountpoint: mntPoint,
+		}
+		switch mntPoint {
+		case "/":
+			payload.UUID = disk.RootPartitionUUID
+		case "/boot/efi":
+			payload.UUID = disk.EFIFilesystemUUID
+			payload.Type = "vfat"
+		default:
+			payload.UUID = disk.FilesystemDataUUID
+		}
+		partitions = append(partitions, disk.Partition{
+			Payload: payload,
+		})
+
+	}
+	return &disk.PartitionTable{
+		Type:       "gpt",
+		Partitions: partitions,
+	}
+}

pkg/image/bootc_disk_test.go

@@ -9,8 +9,8 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/osbuild/images/internal/testdisk"
 	"github.com/osbuild/images/pkg/container"
-	"github.com/osbuild/images/pkg/disk"
 	"github.com/osbuild/images/pkg/image"
 	"github.com/osbuild/images/pkg/manifest"
 	"github.com/osbuild/images/pkg/platform"
@@ -43,21 +43,6 @@ func makeFakePlatform() platform.Platform {
 	}
 }
 
-func makeFakePartitionTable() *disk.PartitionTable {
-	return &disk.PartitionTable{
-		Type: "gpt",
-		Partitions: []disk.Partition{
-			{
-				Payload: &disk.Filesystem{
-					Type:       "ext4",
-					UUID:       disk.RootPartitionUUID,
-					Mountpoint: "/",
-				},
-			},
-		},
-	}
-}
-
 func TestBootcDiskImageInstantiateNoBuildpipelineForQcow2(t *testing.T) {
 	containerSource := container.SourceSpec{
 		Source: "some-src",
@@ -68,7 +53,7 @@ func TestBootcDiskImageInstantiateNoBuildpipelineForQcow2(t *testing.T) {
 	img := image.NewBootcDiskImage(containerSource)
 	require.NotNil(t, img)
 	img.Platform = makeFakePlatform()
-	img.PartitionTable = makeFakePartitionTable()
+	img.PartitionTable = testdisk.MakeFakePartitionTable("/")
 
 	m := &manifest.Manifest{}
 	runi := &runner.Fedora{}

pkg/osbuild/copy_stage.go

@@ -2,8 +2,6 @@ package osbuild
 
 import (
 	"fmt"
-	"reflect"
-	"sort"
 
 	"github.com/osbuild/images/pkg/disk"
 )
@@ -63,59 +61,9 @@ func GenCopyFSTreeOptions(inputName, inputPipeline, filename string, pt *disk.Pa
 	[]Mount,
 ) {
 
-	devices := make(map[string]Device, len(pt.Partitions))
-	mounts := make([]Mount, 0, len(pt.Partitions))
-	var fsRootMntName string
-	genMounts := func(mnt disk.Mountable, path []disk.Entity) error {
-		stageDevices, name := getDevices(path, filename, false)
-		mountpoint := mnt.GetMountpoint()
-
-		if mountpoint == "/" {
-			fsRootMntName = name
-		}
-
-		var mount *Mount
-		t := mnt.GetFSType()
-		switch t {
-		case "xfs":
-			mount = NewXfsMount(name, name, mountpoint)
-		case "vfat":
-			mount = NewFATMount(name, name, mountpoint)
-		case "ext4":
-			mount = NewExt4Mount(name, name, mountpoint)
-		case "btrfs":
-			mount = NewBtrfsMount(name, name, mountpoint)
-		default:
-			panic("unknown fs type " + t)
-		}
-		mounts = append(mounts, *mount)
-
-		// update devices map with new elements from stageDevices
-		for devName := range stageDevices {
-			if existingDevice, exists := devices[devName]; exists {
-				// It is usual that the a device is generated twice for the same Entity e.g. LVM VG, which is OK.
-				// Therefore fail only if a device with the same name is generated for two different Entities.
-				if !reflect.DeepEqual(existingDevice, stageDevices[devName]) {
-					panic(fmt.Sprintf("the device name %q has been generated for two different devices", devName))
-				}
-			}
-			devices[devName] = stageDevices[devName]
-		}
-		return nil
-	}
-
-	_ = pt.ForEachMountable(genMounts)
-
-	// sort the mounts, using < should just work because:
-	// - a parent directory should be always before its children:
-	//   / < /boot
-	// - the order of siblings doesn't matter
-	sort.Slice(mounts, func(i, j int) bool {
-		return mounts[i].Target < mounts[j].Target
-	})
-
-	if fsRootMntName == "" {
-		panic("no mount found for the filesystem root")
+	fsRootMntName, mounts, devices, err := genMountsDevicesFromPt(filename, pt)
+	if err != nil {
+		panic(err)
 	}
 
 	options := CopyStageOptions{

pkg/osbuild/device.go

@@ -2,6 +2,8 @@ package osbuild
 
 import (
 	"fmt"
+	"reflect"
+	"sort"
 	"strings"
 
 	"github.com/osbuild/images/pkg/disk"
@@ -222,3 +224,64 @@ func pathEscape(path string) string {
 
 	return strings.ReplaceAll(path, "/", "-")
 }
+
+func genMountsDevicesFromPt(filename string, pt *disk.PartitionTable) (string, []Mount, map[string]Device, error) {
+	devices := make(map[string]Device, len(pt.Partitions))
+	mounts := make([]Mount, 0, len(pt.Partitions))
+	var fsRootMntName string
+	genMounts := func(mnt disk.Mountable, path []disk.Entity) error {
+		stageDevices, name := getDevices(path, filename, false)
+		mountpoint := mnt.GetMountpoint()
+
+		if mountpoint == "/" {
+			fsRootMntName = name
+		}
+
+		var mount *Mount
+		t := mnt.GetFSType()
+		switch t {
+		case "xfs":
+			mount = NewXfsMount(name, name, mountpoint)
+		case "vfat":
+			mount = NewFATMount(name, name, mountpoint)
+		case "ext4":
+			mount = NewExt4Mount(name, name, mountpoint)
+		case "btrfs":
+			mount = NewBtrfsMount(name, name, mountpoint)
+		default:
+			return fmt.Errorf("unknown fs type " + t)
+		}
+		mounts = append(mounts, *mount)
+
+		// update devices map with new elements from stageDevices
+		for devName := range stageDevices {
+			if existingDevice, exists := devices[devName]; exists {
+				// It is usual that the a device is generated twice for the same Entity e.g. LVM VG, which is OK.
+				// Therefore fail only if a device with the same name is generated for two different Entities.
+				if !reflect.DeepEqual(existingDevice, stageDevices[devName]) {
+					return fmt.Errorf("the device name %q has been generated for two different devices", devName)
+				}
+			}
+			devices[devName] = stageDevices[devName]
+		}
+		return nil
+	}
+
+	if err := pt.ForEachMountable(genMounts); err != nil {
+		return "", nil, nil, err
+	}
+
+	// sort the mounts, using < should just work because:
+	// - a parent directory should be always before its children:
+	//   / < /boot
+	// - the order of siblings doesn't matter
+	sort.Slice(mounts, func(i, j int) bool {
+		return mounts[i].Target < mounts[j].Target
+	})
+
+	if fsRootMntName == "" {
+		return "", nil, nil, fmt.Errorf("no mount found for the filesystem root")
+	}
+
+	return fsRootMntName, mounts, devices, nil
+}

pkg/osbuild/device_test.go

@@ -5,7 +5,9 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
+	"github.com/osbuild/images/internal/testdisk"
 	"github.com/osbuild/images/pkg/blueprint"
 	"github.com/osbuild/images/pkg/disk"
 )
@@ -166,3 +168,39 @@ func TestPathEscape(t *testing.T) {
 		})
 	}
 }
+
+func TestMountsDeviceFromPtEmptyErrors(t *testing.T) {
+	filename := "fake-disk.img"
+	fakePt := testdisk.MakeFakePartitionTable()
+	fsRootMntName, mounts, devices, err := genMountsDevicesFromPt(filename, fakePt)
+	assert.ErrorContains(t, err, "no mount found for the filesystem root")
+	assert.Equal(t, fsRootMntName, "")
+	require.Nil(t, mounts)
+	require.Nil(t, devices)
+}
+
+func TestMountsDeviceFromPtNoRootErrors(t *testing.T) {
+	filename := "fake-disk.img"
+	fakePt := testdisk.MakeFakePartitionTable("/not-root")
+	_, _, _, err := genMountsDevicesFromPt(filename, fakePt)
+	assert.ErrorContains(t, err, "no mount found for the filesystem root")
+}
+
+func TestMountsDeviceFromPtHappy(t *testing.T) {
+	filename := "fake-disk.img"
+	fakePt := testdisk.MakeFakePartitionTable("/")
+	fsRootMntName, mounts, devices, err := genMountsDevicesFromPt(filename, fakePt)
+	require.Nil(t, err)
+	assert.Equal(t, fsRootMntName, "-")
+	assert.Equal(t, mounts, []Mount{
+		{Name: "-", Type: "org.osbuild.ext4", Source: "-", Target: "/"},
+	})
+	assert.Equal(t, devices, map[string]Device{
+		"-": {
+			Type: "org.osbuild.loopback",
+			Options: &LoopbackDeviceOptions{
+				Filename: "fake-disk.img",
+			},
+		},
+	})
+}