Skip to content

Commit

Permalink
pkg/osbuild: Skip EFI partition check for non UEFI platforms
Browse files Browse the repository at this point in the history 
Currently, validateBootupdMounts always ensures that /boot/efi exists.
This requirement is not necessary on non-UEFI platforms such as
s390x. This patch skips the check for /boot/efi when UEFIVendor is not set.

Signed-off-by: Yohei Ueda <[email protected]>
  • Loading branch information
@yoheiueda
yoheiueda committed Jun 24, 2024
1 parent 293f1bc commit 5650c07
Show file tree
Hide file tree
Showing 7 changed files with 98 additions and 30 deletions.
6 changes: 3 additions & 3 deletions pkg/manifest/raw_bootc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,11 @@ func (p *RawBootcImage) serialize() osbuild.Pipeline {
inputs := osbuild.ContainerDeployInputs{
Images: osbuild.NewContainersInputForSingleSource(p.containerSpecs[0]),
}
devices, mounts, err := osbuild.GenBootupdDevicesMounts(p.filename, p.PartitionTable)
devices, mounts, err := osbuild.GenBootupdDevicesMounts(p.filename, p.PartitionTable, p.platform)
if err != nil {
panic(err)
}
st, err := osbuild.NewBootcInstallToFilesystemStage(opts, inputs, devices, mounts)
st, err := osbuild.NewBootcInstallToFilesystemStage(opts, inputs, devices, mounts, p.platform)
if err != nil {
panic(err)
}
Expand All @@ -158,7 +158,7 @@ func (p *RawBootcImage) serialize() osbuild.Pipeline {

// all our customizations work directly on the mounted deployment
// root from the image so generate the devices/mounts for all
devices, mounts, err = osbuild.GenBootupdDevicesMounts(p.filename, p.PartitionTable)
devices, mounts, err = osbuild.GenBootupdDevicesMounts(p.filename, p.PartitionTable, p.platform)
if err != nil {
panic(fmt.Sprintf("gen devices stage failed %v", err))
}
Expand Down
19 changes: 16 additions & 3 deletions pkg/manifest/raw_bootc_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/osbuild/images/pkg/customizations/users"
"github.com/osbuild/images/pkg/manifest"
"github.com/osbuild/images/pkg/osbuild"
"github.com/osbuild/images/pkg/platform"
"github.com/osbuild/images/pkg/runner"
)

Expand Down Expand Up @@ -50,8 +51,12 @@ func TestRawBootcImageSerialize(t *testing.T) {
mani := manifest.New()
runner := &runner.Linux{}
build := manifest.NewBuildFromContainer(&mani, runner, nil, nil)
pf := &platform.X86{
BasePlatform: platform.BasePlatform{},
UEFIVendor: "test",
}

rawBootcPipeline := manifest.NewRawBootcImage(build, containers, nil)
rawBootcPipeline := manifest.NewRawBootcImage(build, containers, pf)
rawBootcPipeline.PartitionTable = testdisk.MakeFakePartitionTable("/", "/boot", "/boot/efi")
rawBootcPipeline.Users = []users.User{{Name: "root", Key: common.ToPtr("some-ssh-key")}}
rawBootcPipeline.KernelOptionsAppend = []string{"karg1", "karg2"}
Expand All @@ -74,8 +79,12 @@ func TestRawBootcImageSerializeMountsValidated(t *testing.T) {
mani := manifest.New()
runner := &runner.Linux{}
build := manifest.NewBuildFromContainer(&mani, runner, nil, nil)
pf := &platform.X86{
BasePlatform: platform.BasePlatform{},
UEFIVendor: "test",
}

rawBootcPipeline := manifest.NewRawBootcImage(build, nil, nil)
rawBootcPipeline := manifest.NewRawBootcImage(build, nil, pf)
// note that we create a partition table without /boot here
rawBootcPipeline.PartitionTable = testdisk.MakeFakePartitionTable("/", "/missing-boot")
rawBootcPipeline.SerializeStart(nil, []container.Spec{{Source: "foo"}}, nil, nil)
Expand All @@ -96,8 +105,12 @@ func findMountIdx(mounts []osbuild.Mount, mntType string) int {
func makeFakeRawBootcPipeline() *manifest.RawBootcImage {
mani := manifest.New()
runner := &runner.Linux{}
pf := &platform.X86{
BasePlatform: platform.BasePlatform{},
UEFIVendor: "test",
}
build := manifest.NewBuildFromContainer(&mani, runner, nil, nil)
rawBootcPipeline := manifest.NewRawBootcImage(build, nil, nil)
rawBootcPipeline := manifest.NewRawBootcImage(build, nil, pf)
rawBootcPipeline.PartitionTable = testdisk.MakeFakePartitionTable("/", "/boot", "/boot/efi")
rawBootcPipeline.SerializeStart(nil, []container.Spec{{Source: "foo"}}, nil, nil)

Expand Down
4 changes: 2 additions & 2 deletions pkg/manifest/raw_ostree.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func (p *RawOSTreeImage) serialize() osbuild.Pipeline {
func (p *RawOSTreeImage) addBootupdStage(pipeline *osbuild.Pipeline) {
pt := p.treePipeline.PartitionTable

treeBootupdDevices, treeBootupdMounts, err := osbuild.GenBootupdDevicesMounts(p.Filename(), pt)
treeBootupdDevices, treeBootupdMounts, err := osbuild.GenBootupdDevicesMounts(p.Filename(), pt, p.platform)
if err != nil {
panic(err)
}
Expand All @@ -138,7 +138,7 @@ func (p *RawOSTreeImage) addBootupdStage(pipeline *osbuild.Pipeline) {
Device: "disk",
}
}
bootupd, err := osbuild.NewBootupdStage(opts, treeBootupdDevices, treeBootupdMounts)
bootupd, err := osbuild.NewBootupdStage(opts, treeBootupdDevices, treeBootupdMounts, p.platform)
if err != nil {
panic(err)
}
Expand Down
6 changes: 4 additions & 2 deletions pkg/osbuild/bootc_install_to_filesystem_stage.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package osbuild

import (
"fmt"

"github.com/osbuild/images/pkg/platform"
)

type BootcInstallToFilesystemOptions struct {
Expand All @@ -24,8 +26,8 @@ func (BootcInstallToFilesystemOptions) isStageOptions() {}
// bootc/bootupd find and install all required bootloader bits.
//
// The mounts input should be generated with GenBootupdDevicesMounts.
func NewBootcInstallToFilesystemStage(options *BootcInstallToFilesystemOptions, inputs ContainerDeployInputs, devices map[string]Device, mounts []Mount) (*Stage, error) {
if err := validateBootupdMounts(mounts); err != nil {
func NewBootcInstallToFilesystemStage(options *BootcInstallToFilesystemOptions, inputs ContainerDeployInputs, devices map[string]Device, mounts []Mount, pltf platform.Platform) (*Stage, error) {
if err := validateBootupdMounts(mounts, pltf); err != nil {
return nil, err
}

Expand Down
31 changes: 26 additions & 5 deletions pkg/osbuild/bootc_install_to_filesystem_stage_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/osbuild/images/pkg/container"
"github.com/osbuild/images/pkg/osbuild"
"github.com/osbuild/images/pkg/platform"
)

func makeFakeContainerInputs() osbuild.ContainerDeployInputs {
Expand All @@ -28,6 +29,10 @@ func TestBootcInstallToFilesystemStageNewHappy(t *testing.T) {
devices := makeOsbuildDevices("dev-for-/", "dev-for-/boot", "dev-for-/boot/efi")
mounts := makeOsbuildMounts("/", "/boot", "/boot/efi")
inputs := makeFakeContainerInputs()
pf := &platform.X86{
BasePlatform: platform.BasePlatform{},
UEFIVendor: "test",
}

expectedStage := &osbuild.Stage{
Type: "org.osbuild.bootc.install-to-filesystem",
Expand All @@ -36,7 +41,7 @@ func TestBootcInstallToFilesystemStageNewHappy(t *testing.T) {
Devices: devices,
Mounts: mounts,
}
stage, err := osbuild.NewBootcInstallToFilesystemStage(nil, inputs, devices, mounts)
stage, err := osbuild.NewBootcInstallToFilesystemStage(nil, inputs, devices, mounts, pf)
require.Nil(t, err)
assert.Equal(t, stage, expectedStage)
}
Expand All @@ -45,8 +50,12 @@ func TestBootcInstallToFilesystemStageNewNoContainers(t *testing.T) {
devices := makeOsbuildDevices("dev-for-/", "dev-for-/boot", "dev-for-/boot/efi")
mounts := makeOsbuildMounts("/", "/boot", "/boot/efi")
inputs := osbuild.ContainerDeployInputs{}
pf := &platform.X86{
BasePlatform: platform.BasePlatform{},
UEFIVendor: "test",
}

_, err := osbuild.NewBootcInstallToFilesystemStage(nil, inputs, devices, mounts)
_, err := osbuild.NewBootcInstallToFilesystemStage(nil, inputs, devices, mounts, pf)
assert.EqualError(t, err, "expected exactly one container input but got: 0 (map[])")
}

Expand All @@ -61,17 +70,25 @@ func TestBootcInstallToFilesystemStageNewTwoContainers(t *testing.T) {
},
},
}
pf := &platform.X86{
BasePlatform: platform.BasePlatform{},
UEFIVendor: "test",
}

_, err := osbuild.NewBootcInstallToFilesystemStage(nil, inputs, devices, mounts)
_, err := osbuild.NewBootcInstallToFilesystemStage(nil, inputs, devices, mounts, pf)
assert.EqualError(t, err, "expected exactly one container input but got: 2 (map[1:{} 2:{}])")
}

func TestBootcInstallToFilesystemStageMissingMounts(t *testing.T) {
devices := makeOsbuildDevices("dev-for-/")
mounts := makeOsbuildMounts("/")
inputs := makeFakeContainerInputs()
pf := &platform.X86{
BasePlatform: platform.BasePlatform{},
UEFIVendor: "test",
}

stage, err := osbuild.NewBootcInstallToFilesystemStage(nil, inputs, devices, mounts)
stage, err := osbuild.NewBootcInstallToFilesystemStage(nil, inputs, devices, mounts, pf)
// XXX: rename error
assert.ErrorContains(t, err, "required mounts for bootupd stage [/boot /boot/efi] missing")
require.Nil(t, stage)
Expand All @@ -81,11 +98,15 @@ func TestBootcInstallToFilesystemStageJsonHappy(t *testing.T) {
devices := makeOsbuildDevices("disk", "dev-for-/", "dev-for-/boot", "dev-for-/boot/efi")
mounts := makeOsbuildMounts("/", "/boot", "/boot/efi")
inputs := makeFakeContainerInputs()
pf := &platform.X86{
BasePlatform: platform.BasePlatform{},
UEFIVendor: "test",
}

opts := &osbuild.BootcInstallToFilesystemOptions{
TargetImgref: "quay.io/centos-bootc/centos-bootc-dev:stream9",
}
stage, err := osbuild.NewBootcInstallToFilesystemStage(opts, inputs, devices, mounts)
stage, err := osbuild.NewBootcInstallToFilesystemStage(opts, inputs, devices, mounts, pf)
require.Nil(t, err)
stageJson, err := json.MarshalIndent(stage, "", " ")
require.Nil(t, err)
Expand Down
19 changes: 11 additions & 8 deletions pkg/osbuild/bootupd_stage.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/osbuild/images/internal/common"
"github.com/osbuild/images/pkg/disk"
"github.com/osbuild/images/pkg/platform"
)

type BootupdStageOptionsBios struct {
Expand Down Expand Up @@ -38,11 +39,13 @@ func (opts *BootupdStageOptions) validate(devices map[string]Device) error {
// validateBootupdMounts ensures that all required mounts for the bootup
// stage are generated. Right now the stage requires root, boot and boot/efi
// to find all the bootloader configs
func validateBootupdMounts(mounts []Mount) error {
func validateBootupdMounts(mounts []Mount, pf platform.Platform) error {
requiredMounts := map[string]bool{
"/": true,
"/boot": true,
"/boot/efi": true,
"/": true,
"/boot": true,
}
if pf.GetUEFIVendor() != "" {
requiredMounts["/boot/efi"] = true
}
for _, mnt := range mounts {
delete(requiredMounts, mnt.Target)
Expand All @@ -61,8 +64,8 @@ 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 map[string]Device, mounts []Mount) (*Stage, error) {
if err := validateBootupdMounts(mounts); err != nil {
func NewBootupdStage(opts *BootupdStageOptions, devices map[string]Device, mounts []Mount, pltf platform.Platform) (*Stage, error) {
if err := validateBootupdMounts(mounts, pltf); err != nil {
return nil, err
}
if err := opts.validate(devices); err != nil {
Expand Down Expand Up @@ -108,7 +111,7 @@ func genMountsForBootupd(source string, pt *disk.PartitionTable) ([]Mount, error
return mounts, nil
}

func GenBootupdDevicesMounts(filename string, pt *disk.PartitionTable) (map[string]Device, []Mount, error) {
func GenBootupdDevicesMounts(filename string, pt *disk.PartitionTable, pltf platform.Platform) (map[string]Device, []Mount, error) {
devName := "disk"
devices := map[string]Device{
devName: Device{
Expand All @@ -123,7 +126,7 @@ func GenBootupdDevicesMounts(filename string, pt *disk.PartitionTable) (map[stri
if err != nil {
return nil, nil, err
}
if err := validateBootupdMounts(mounts); err != nil {
if err := validateBootupdMounts(mounts, pltf); err != nil {
return nil, nil, err
}

Expand Down
43 changes: 36 additions & 7 deletions pkg/osbuild/bootupd_stage_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/osbuild/images/internal/common"
"github.com/osbuild/images/pkg/disk"
"github.com/osbuild/images/pkg/osbuild"
"github.com/osbuild/images/pkg/platform"
)

func makeOsbuildMounts(targets ...string) []osbuild.Mount {
Expand Down Expand Up @@ -41,14 +42,18 @@ func TestBootupdStageNewHappy(t *testing.T) {
}
devices := makeOsbuildDevices("dev-for-/", "dev-for-/boot", "dev-for-/boot/efi")
mounts := makeOsbuildMounts("/", "/boot", "/boot/efi")
pf := &platform.X86{
BasePlatform: platform.BasePlatform{},
UEFIVendor: "test",
}

expectedStage := &osbuild.Stage{
Type: "org.osbuild.bootupd",
Options: opts,
Devices: devices,
Mounts: mounts,
}
stage, err := osbuild.NewBootupdStage(opts, devices, mounts)
stage, err := osbuild.NewBootupdStage(opts, devices, mounts, pf)
require.Nil(t, err)
assert.Equal(t, stage, expectedStage)
}
Expand All @@ -59,8 +64,12 @@ func TestBootupdStageMissingMounts(t *testing.T) {
}
devices := makeOsbuildDevices("dev-for-/")
mounts := makeOsbuildMounts("/")
pf := &platform.X86{
BasePlatform: platform.BasePlatform{},
UEFIVendor: "test",
}

stage, err := osbuild.NewBootupdStage(opts, devices, mounts)
stage, err := osbuild.NewBootupdStage(opts, devices, mounts, pf)
assert.ErrorContains(t, err, "required mounts for bootupd stage [/boot /boot/efi] missing")
require.Nil(t, stage)
}
Expand All @@ -73,8 +82,12 @@ func TestBootupdStageMissingDevice(t *testing.T) {
}
devices := makeOsbuildDevices("dev-for-/", "dev-for-/boot", "dev-for-/boot/efi")
mounts := makeOsbuildMounts("/", "/boot", "/boot/efi")
pf := &platform.X86{
BasePlatform: platform.BasePlatform{},
UEFIVendor: "test",
}

stage, err := osbuild.NewBootupdStage(opts, devices, mounts)
stage, err := osbuild.NewBootupdStage(opts, devices, mounts, pf)
assert.ErrorContains(t, err, `cannot find expected device "disk" for bootupd bios option in [dev-for-/ dev-for-/boot dev-for-/boot/efi]`)
require.Nil(t, stage)
}
Expand All @@ -92,8 +105,12 @@ func TestBootupdStageJsonHappy(t *testing.T) {
}
devices := makeOsbuildDevices("disk", "dev-for-/", "dev-for-/boot", "dev-for-/boot/efi")
mounts := makeOsbuildMounts("/", "/boot", "/boot/efi")
pf := &platform.X86{
BasePlatform: platform.BasePlatform{},
UEFIVendor: "test",
}

stage, err := osbuild.NewBootupdStage(opts, devices, mounts)
stage, err := osbuild.NewBootupdStage(opts, devices, mounts, pf)
require.Nil(t, err)
stageJson, err := json.MarshalIndent(stage, "", " ")
require.Nil(t, err)
Expand Down Expand Up @@ -149,7 +166,11 @@ func TestBootupdStageJsonHappy(t *testing.T) {
func TestGenBootupdDevicesMountsMissingRoot(t *testing.T) {
filename := "fake-disk.img"
pt := &disk.PartitionTable{}
_, _, err := osbuild.GenBootupdDevicesMounts(filename, pt)
pf := &platform.X86{
BasePlatform: platform.BasePlatform{},
UEFIVendor: "test",
}
_, _, err := osbuild.GenBootupdDevicesMounts(filename, pt, pf)
assert.EqualError(t, err, "required mounts for bootupd stage [/ /boot /boot/efi] missing")
}

Expand All @@ -162,7 +183,11 @@ func TestGenBootupdDevicesMountsUnexpectedEntity(t *testing.T) {
},
},
}
_, _, err := osbuild.GenBootupdDevicesMounts(filename, pt)
pf := &platform.X86{
BasePlatform: platform.BasePlatform{},
UEFIVendor: "test",
}
_, _, err := osbuild.GenBootupdDevicesMounts(filename, pt, pf)
assert.EqualError(t, err, "type *disk.LVMVolumeGroup not supported by bootupd handling yet")
}

Expand Down Expand Up @@ -221,8 +246,12 @@ var fakePt = &disk.PartitionTable{

func TestGenBootupdDevicesMountsHappy(t *testing.T) {
filename := "fake-disk.img"
pf := &platform.X86{
BasePlatform: platform.BasePlatform{},
UEFIVendor: "test",
}

devices, mounts, err := osbuild.GenBootupdDevicesMounts(filename, fakePt)
devices, mounts, err := osbuild.GenBootupdDevicesMounts(filename, fakePt, pf)
require.Nil(t, err)
assert.Equal(t, devices, map[string]osbuild.Device{
"disk": {
Expand Down

0 comments on commit 5650c07

Please sign in to comment.