Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pkg/osbuild: Skip EFI partition check for non UEFI platforms #758

Merged
merged 1 commit into from
Jun 25, 2024
Merged

pkg/osbuild: Skip EFI partition check for non UEFI platforms #758

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pkg/manifest/raw_bootc.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading