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.
osbuild · Jan 17, 2024 · 259fe34 · 259fe34
1 parent b639124
commit 259fe34
Show file tree

Hide file tree

Showing 18 changed files with 39 additions and 40 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,