From 259fe3415a204c55d9a6349223b8b64ea3d28f96 Mon Sep 17 00:00:00 2001
From: Michael Vogt <michael.vogt@gmail.com>
Date: Tue, 16 Jan 2024 13:27:22 +0100
Subject: [PATCH] osbuild,manifest: simplify `osbuild.Devices`

The osbuild.Devices type is just an alias to map[string]osbuild.Device
and it feels like it might be easier to expose the map[string]Device
directly. From the type it's not clear if it's a map and when using
the type it needs to be used like map it seems easiest to expose it
directly.

Another benefit of this is that the pointers used are now clearer (IMHO)
i.e. no indirections required for maps.
---
 pkg/manifest/iso_rootfs.go            |  4 ++--
 pkg/osbuild/bootupd_stage.go          |  6 +++---
 pkg/osbuild/bootupd_stage_test.go     | 10 +++++-----
 pkg/osbuild/clevis_luks_bind_stage.go |  2 +-
 pkg/osbuild/copy_stage.go             | 10 ++++------
 pkg/osbuild/copy_stage_test.go        |  3 +--
 pkg/osbuild/device.go                 |  2 --
 pkg/osbuild/luks2_format_stage.go     |  2 +-
 pkg/osbuild/luks2_remove_key_stage.go |  2 +-
 pkg/osbuild/lvm2_create_stage.go      |  2 +-
 pkg/osbuild/lvm2_metadata_stage.go    |  2 +-
 pkg/osbuild/mkfs_stages_test.go       |  8 ++++----
 pkg/osbuild/sfdisk_stage.go           |  4 +++-
 pkg/osbuild/sfdisk_stage_test.go      |  2 +-
 pkg/osbuild/sgdisk_stage.go           |  4 +++-
 pkg/osbuild/sgdisk_stage_test.go      |  2 +-
 pkg/osbuild/stage.go                  |  8 ++++----
 pkg/osbuild/zipl_inst_stage.go        |  6 +++---
 18 files changed, 39 insertions(+), 40 deletions(-)

diff --git a/pkg/manifest/iso_rootfs.go b/pkg/manifest/iso_rootfs.go
index 4599c70e25..f7fb6a73de 100644
--- a/pkg/manifest/iso_rootfs.go
+++ b/pkg/manifest/iso_rootfs.go
@@ -49,7 +49,7 @@ func (p *ISORootfsImg) serialize() osbuild.Pipeline {
 	)
 
 	devName := "device"
-	devices := osbuild.Devices{devName: *lodevice}
+	devices := map[string]osbuild.Device{devName: *lodevice}
 	mkfsStage := osbuild.NewMkfsExt4Stage(mkfsStageOptions, devices)
 	pipeline.AddStage(mkfsStage)
 
@@ -64,7 +64,7 @@ func (p *ISORootfsImg) serialize() osbuild.Pipeline {
 	}
 	copyStageInputs := osbuild.NewPipelineTreeInputs(inputName, p.installerPipeline.Name())
 	copyStageMounts := []osbuild.Mount{*osbuild.NewExt4Mount(devName, devName, "/")}
-	copyStage := osbuild.NewCopyStage(copyStageOptions, copyStageInputs, &devices, copyStageMounts)
+	copyStage := osbuild.NewCopyStage(copyStageOptions, copyStageInputs, devices, copyStageMounts)
 	pipeline.AddStage(copyStage)
 	return pipeline
 }
diff --git a/pkg/osbuild/bootupd_stage.go b/pkg/osbuild/bootupd_stage.go
index decc20440b..1e431a72d5 100644
--- a/pkg/osbuild/bootupd_stage.go
+++ b/pkg/osbuild/bootupd_stage.go
@@ -58,18 +58,18 @@ func validateBootupdMounts(mounts []Mount) error {
 // NewBootupdStage creates a new stage for the org.osbuild.bootupd stage. It
 // requires a mount setup of "/", "/boot" and "/boot/efi" right now so that
 // bootupd can find and install all required bootloader bits.
-func NewBootupdStage(opts *BootupdStageOptions, devices *Devices, mounts []Mount) (*Stage, error) {
+func NewBootupdStage(opts *BootupdStageOptions, devices map[string]Device, mounts []Mount) (*Stage, error) {
 	if err := validateBootupdMounts(mounts); err != nil {
 		return nil, err
 	}
-	if err := opts.validate(*devices); err != nil {
+	if err := opts.validate(devices); err != nil {
 		return nil, err
 	}
 
 	return &Stage{
 		Type:    "org.osbuild.bootupd",
 		Options: opts,
-		Devices: *devices,
+		Devices: devices,
 		Mounts:  mounts,
 	}, nil
 }
diff --git a/pkg/osbuild/bootupd_stage_test.go b/pkg/osbuild/bootupd_stage_test.go
index 12e0827fcc..2c0461f15a 100644
--- a/pkg/osbuild/bootupd_stage_test.go
+++ b/pkg/osbuild/bootupd_stage_test.go
@@ -23,7 +23,7 @@ func makeOsbuildMounts(targets ...string) []osbuild.Mount {
 	return mnts
 }
 
-func makeOsbuildDevices(devnames ...string) osbuild.Devices {
+func makeOsbuildDevices(devnames ...string) map[string]osbuild.Device {
 	devices := make(map[string]osbuild.Device)
 	for _, devname := range devnames {
 		devices[devname] = osbuild.Device{
@@ -46,7 +46,7 @@ func TestBootupdStageNewHappy(t *testing.T) {
 		Devices: devices,
 		Mounts:  mounts,
 	}
-	stage, err := osbuild.NewBootupdStage(opts, &devices, mounts)
+	stage, err := osbuild.NewBootupdStage(opts, devices, mounts)
 	require.Nil(t, err)
 	assert.Equal(t, stage, expectedStage)
 }
@@ -58,7 +58,7 @@ func TestBootupdStageMissingMounts(t *testing.T) {
 	devices := makeOsbuildDevices("dev-/")
 	mounts := makeOsbuildMounts("/")
 
-	stage, err := osbuild.NewBootupdStage(opts, &devices, mounts)
+	stage, err := osbuild.NewBootupdStage(opts, devices, mounts)
 	assert.ErrorContains(t, err, "required mounts for bootupd stage [/boot /boot/efi] missing")
 	require.Nil(t, stage)
 }
@@ -72,7 +72,7 @@ func TestBootupdStageMissingDevice(t *testing.T) {
 	devices := makeOsbuildDevices("dev-/", "dev-/boot", "dev-/boot/efi")
 	mounts := makeOsbuildMounts("/", "/boot", "/boot/efi")
 
-	stage, err := osbuild.NewBootupdStage(opts, &devices, mounts)
+	stage, err := osbuild.NewBootupdStage(opts, devices, mounts)
 	assert.ErrorContains(t, err, `cannot find expected device "disk" for bootupd bios option in [dev-/ dev-/boot dev-/boot/efi]`)
 	require.Nil(t, stage)
 }
@@ -91,7 +91,7 @@ func TestBootupdStageJsonHappy(t *testing.T) {
 	devices := makeOsbuildDevices("disk", "dev-/", "dev-/boot", "dev-/boot/efi")
 	mounts := makeOsbuildMounts("/", "/boot", "/boot/efi")
 
-	stage, err := osbuild.NewBootupdStage(opts, &devices, mounts)
+	stage, err := osbuild.NewBootupdStage(opts, devices, mounts)
 	require.Nil(t, err)
 	stageJson, err := json.MarshalIndent(stage, "", "  ")
 	require.Nil(t, err)
diff --git a/pkg/osbuild/clevis_luks_bind_stage.go b/pkg/osbuild/clevis_luks_bind_stage.go
index fd83fce536..17155adc77 100644
--- a/pkg/osbuild/clevis_luks_bind_stage.go
+++ b/pkg/osbuild/clevis_luks_bind_stage.go
@@ -8,7 +8,7 @@ type ClevisLuksBindStageOptions struct {
 
 func (ClevisLuksBindStageOptions) isStageOptions() {}
 
-func NewClevisLuksBindStage(options *ClevisLuksBindStageOptions, devices Devices) *Stage {
+func NewClevisLuksBindStage(options *ClevisLuksBindStageOptions, devices map[string]Device) *Stage {
 	return &Stage{
 		Type:    "org.osbuild.clevis.luks-bind",
 		Options: options,
diff --git a/pkg/osbuild/copy_stage.go b/pkg/osbuild/copy_stage.go
index 3a20bfd774..2c0b1cfdbf 100644
--- a/pkg/osbuild/copy_stage.go
+++ b/pkg/osbuild/copy_stage.go
@@ -26,12 +26,12 @@ type CopyStagePath struct {
 
 func (CopyStageOptions) isStageOptions() {}
 
-func NewCopyStage(options *CopyStageOptions, inputs Inputs, devices *Devices, mounts []Mount) *Stage {
+func NewCopyStage(options *CopyStageOptions, inputs Inputs, devices map[string]Device, mounts []Mount) *Stage {
 	return &Stage{
 		Type:    "org.osbuild.copy",
 		Options: options,
 		Inputs:  inputs,
-		Devices: *devices,
+		Devices: devices,
 		Mounts:  mounts,
 	}
 }
@@ -59,7 +59,7 @@ func (*CopyStageFilesInputs) isStageInputs() {}
 // function, not just the options, devices, and mounts.
 func GenCopyFSTreeOptions(inputName, inputPipeline, filename string, pt *disk.PartitionTable) (
 	*CopyStageOptions,
-	*Devices,
+	map[string]Device,
 	[]Mount,
 ) {
 
@@ -118,8 +118,6 @@ func GenCopyFSTreeOptions(inputName, inputPipeline, filename string, pt *disk.Pa
 		panic("no mount found for the filesystem root")
 	}
 
-	stageDevices := Devices(devices)
-
 	options := CopyStageOptions{
 		Paths: []CopyStagePath{
 			{
@@ -129,5 +127,5 @@ func GenCopyFSTreeOptions(inputName, inputPipeline, filename string, pt *disk.Pa
 		},
 	}
 
-	return &options, &stageDevices, mounts
+	return &options, devices, mounts
 }
diff --git a/pkg/osbuild/copy_stage_test.go b/pkg/osbuild/copy_stage_test.go
index 110caaee72..2eecc49e8f 100644
--- a/pkg/osbuild/copy_stage_test.go
+++ b/pkg/osbuild/copy_stage_test.go
@@ -39,8 +39,7 @@ func TestNewCopyStage(t *testing.T) {
 		Mounts:  mounts,
 	}
 	// convert to alias types
-	stageDevices := Devices(devices)
-	actualStage := NewCopyStage(&CopyStageOptions{paths}, NewPipelineTreeInputs("tree-input", "input-pipeline"), &stageDevices, mounts)
+	actualStage := NewCopyStage(&CopyStageOptions{paths}, NewPipelineTreeInputs("tree-input", "input-pipeline"), devices, mounts)
 	assert.Equal(t, expectedStage, actualStage)
 }
 
diff --git a/pkg/osbuild/device.go b/pkg/osbuild/device.go
index feba2c8342..1ea4ee7888 100644
--- a/pkg/osbuild/device.go
+++ b/pkg/osbuild/device.go
@@ -7,8 +7,6 @@ import (
 	"github.com/osbuild/images/pkg/disk"
 )
 
-type Devices map[string]Device
-
 type Device struct {
 	Type    string        `json:"type"`
 	Parent  string        `json:"parent,omitempty"`
diff --git a/pkg/osbuild/luks2_format_stage.go b/pkg/osbuild/luks2_format_stage.go
index 5880f7490d..439d720e4f 100644
--- a/pkg/osbuild/luks2_format_stage.go
+++ b/pkg/osbuild/luks2_format_stage.go
@@ -42,7 +42,7 @@ func (o LUKS2CreateStageOptions) validate() error {
 	return nil
 }
 
-func NewLUKS2CreateStage(options *LUKS2CreateStageOptions, devices Devices) *Stage {
+func NewLUKS2CreateStage(options *LUKS2CreateStageOptions, devices map[string]Device) *Stage {
 	if err := options.validate(); err != nil {
 		panic(err)
 	}
diff --git a/pkg/osbuild/luks2_remove_key_stage.go b/pkg/osbuild/luks2_remove_key_stage.go
index 03d4b862b8..e4ce9958aa 100644
--- a/pkg/osbuild/luks2_remove_key_stage.go
+++ b/pkg/osbuild/luks2_remove_key_stage.go
@@ -6,7 +6,7 @@ type LUKS2RemoveKeyStageOptions struct {
 
 func (LUKS2RemoveKeyStageOptions) isStageOptions() {}
 
-func NewLUKS2RemoveKeyStage(options *LUKS2RemoveKeyStageOptions, devices Devices) *Stage {
+func NewLUKS2RemoveKeyStage(options *LUKS2RemoveKeyStageOptions, devices map[string]Device) *Stage {
 	return &Stage{
 		Type:    "org.osbuild.luks2.remove-key",
 		Options: options,
diff --git a/pkg/osbuild/lvm2_create_stage.go b/pkg/osbuild/lvm2_create_stage.go
index a29ddb2a86..a69d765e75 100644
--- a/pkg/osbuild/lvm2_create_stage.go
+++ b/pkg/osbuild/lvm2_create_stage.go
@@ -35,7 +35,7 @@ type LogicalVolume struct {
 	Size string `json:"size"`
 }
 
-func NewLVM2CreateStage(options *LVM2CreateStageOptions, devices Devices) *Stage {
+func NewLVM2CreateStage(options *LVM2CreateStageOptions, devices map[string]Device) *Stage {
 	if err := options.validate(); err != nil {
 		panic(err)
 	}
diff --git a/pkg/osbuild/lvm2_metadata_stage.go b/pkg/osbuild/lvm2_metadata_stage.go
index 4d3ce3bd7e..d158209c2f 100644
--- a/pkg/osbuild/lvm2_metadata_stage.go
+++ b/pkg/osbuild/lvm2_metadata_stage.go
@@ -35,7 +35,7 @@ func (o LVM2MetadataStageOptions) validate() error {
 	return nil
 }
 
-func NewLVM2MetadataStage(options *LVM2MetadataStageOptions, devices Devices) *Stage {
+func NewLVM2MetadataStage(options *LVM2MetadataStageOptions, devices map[string]Device) *Stage {
 	if err := options.validate(); err != nil {
 		panic(err)
 	}
diff --git a/pkg/osbuild/mkfs_stages_test.go b/pkg/osbuild/mkfs_stages_test.go
index bfafda4953..86165d2e50 100644
--- a/pkg/osbuild/mkfs_stages_test.go
+++ b/pkg/osbuild/mkfs_stages_test.go
@@ -30,7 +30,7 @@ func TestNewMkfsStage(t *testing.T) {
 	mkbtrfsExpected := &Stage{
 		Type:    "org.osbuild.mkfs.btrfs",
 		Options: btrfsOptions,
-		Devices: Devices{"device": *device},
+		Devices: map[string]Device{"device": *device},
 	}
 	assert.Equal(t, mkbtrfsExpected, mkbtrfs)
 
@@ -42,7 +42,7 @@ func TestNewMkfsStage(t *testing.T) {
 	mkext4Expected := &Stage{
 		Type:    "org.osbuild.mkfs.ext4",
 		Options: ext4Options,
-		Devices: Devices{"device": *device},
+		Devices: map[string]Device{"device": *device},
 	}
 	assert.Equal(t, mkext4Expected, mkext4)
 
@@ -55,7 +55,7 @@ func TestNewMkfsStage(t *testing.T) {
 	mkfatExpected := &Stage{
 		Type:    "org.osbuild.mkfs.fat",
 		Options: fatOptions,
-		Devices: Devices{"device": *device},
+		Devices: map[string]Device{"device": *device},
 	}
 	assert.Equal(t, mkfatExpected, mkfat)
 
@@ -67,7 +67,7 @@ func TestNewMkfsStage(t *testing.T) {
 	mkxfsExpected := &Stage{
 		Type:    "org.osbuild.mkfs.xfs",
 		Options: xfsOptions,
-		Devices: Devices{"device": *device},
+		Devices: map[string]Device{"device": *device},
 	}
 	assert.Equal(t, mkxfsExpected, mkxfs)
 }
diff --git a/pkg/osbuild/sfdisk_stage.go b/pkg/osbuild/sfdisk_stage.go
index 9e5f441b54..d5a73daedb 100644
--- a/pkg/osbuild/sfdisk_stage.go
+++ b/pkg/osbuild/sfdisk_stage.go
@@ -40,6 +40,8 @@ func NewSfdiskStage(options *SfdiskStageOptions, device *Device) *Stage {
 	return &Stage{
 		Type:    "org.osbuild.sfdisk",
 		Options: options,
-		Devices: Devices{"device": *device},
+		Devices: map[string]Device{
+			"device": *device,
+		},
 	}
 }
diff --git a/pkg/osbuild/sfdisk_stage_test.go b/pkg/osbuild/sfdisk_stage_test.go
index 09da6a8f2d..e28b02b64a 100644
--- a/pkg/osbuild/sfdisk_stage_test.go
+++ b/pkg/osbuild/sfdisk_stage_test.go
@@ -24,7 +24,7 @@ func TestNewSfdiskStage(t *testing.T) {
 	}
 
 	device := NewLoopbackDevice(&LoopbackDeviceOptions{Filename: "disk.raw"})
-	devices := Devices{"device": *device}
+	devices := map[string]Device{"device": *device}
 
 	expectedStage := &Stage{
 		Type:    "org.osbuild.sfdisk",
diff --git a/pkg/osbuild/sgdisk_stage.go b/pkg/osbuild/sgdisk_stage.go
index aa011def88..204eaac577 100644
--- a/pkg/osbuild/sgdisk_stage.go
+++ b/pkg/osbuild/sgdisk_stage.go
@@ -41,6 +41,8 @@ func NewSgdiskStage(options *SgdiskStageOptions, device *Device) *Stage {
 	return &Stage{
 		Type:    "org.osbuild.sgdisk",
 		Options: options,
-		Devices: Devices{"device": *device},
+		Devices: map[string]Device{
+			"device": *device,
+		},
 	}
 }
diff --git a/pkg/osbuild/sgdisk_stage_test.go b/pkg/osbuild/sgdisk_stage_test.go
index b53b9f32c7..15237bd451 100644
--- a/pkg/osbuild/sgdisk_stage_test.go
+++ b/pkg/osbuild/sgdisk_stage_test.go
@@ -25,7 +25,7 @@ func TestNewSgdiskStage(t *testing.T) {
 	}
 
 	device := NewLoopbackDevice(&LoopbackDeviceOptions{Filename: "disk.raw"})
-	devices := Devices{"device": *device}
+	devices := map[string]Device{"device": *device}
 
 	expectedStage := &Stage{
 		Type:    "org.osbuild.sgdisk",
diff --git a/pkg/osbuild/stage.go b/pkg/osbuild/stage.go
index 4c1734acf7..d192e5ff55 100644
--- a/pkg/osbuild/stage.go
+++ b/pkg/osbuild/stage.go
@@ -7,10 +7,10 @@ type Stage struct {
 	Type string `json:"type"`
 	// Stage-type specific options fully determining the operations of the
 
-	Inputs  Inputs       `json:"inputs,omitempty"`
-	Options StageOptions `json:"options,omitempty"`
-	Devices Devices      `json:"devices,omitempty"`
-	Mounts  []Mount      `json:"mounts,omitempty"`
+	Inputs  Inputs            `json:"inputs,omitempty"`
+	Options StageOptions      `json:"options,omitempty"`
+	Devices map[string]Device `json:"devices,omitempty"`
+	Mounts  []Mount           `json:"mounts,omitempty"`
 }
 
 // StageOptions specify the operations of a given stage-type.
diff --git a/pkg/osbuild/zipl_inst_stage.go b/pkg/osbuild/zipl_inst_stage.go
index f89efa270d..dd51fe8d9f 100644
--- a/pkg/osbuild/zipl_inst_stage.go
+++ b/pkg/osbuild/zipl_inst_stage.go
@@ -17,11 +17,11 @@ func (ZiplInstStageOptions) isStageOptions() {}
 
 // Return a new zipl.inst stage. The 'disk' parameter must represent the
 // (entire) device that contains the /boot partition.
-func NewZiplInstStage(options *ZiplInstStageOptions, disk *Device, devices *Devices, mounts []Mount) *Stage {
+func NewZiplInstStage(options *ZiplInstStageOptions, disk *Device, devices map[string]Device, mounts []Mount) *Stage {
 	// create a new devices map and add the disk to it
-	devmap := map[string]Device(*devices)
+	devmap := map[string]Device(devices)
 	devmap["disk"] = *disk
-	ziplDevices := Devices(devmap)
+	ziplDevices := map[string]Device(devmap)
 	return &Stage{
 		Type:    "org.osbuild.zipl.inst",
 		Options: options,