diff --git a/internal/partition/partition.go b/internal/partition/partition.go index c7880f45..7fcb9316 100644 --- a/internal/partition/partition.go +++ b/internal/partition/partition.go @@ -60,20 +60,30 @@ func newPartitionTable(volume *gadget.Volume, sectorSize uint64, imgSize uint64) // GeneratePartitionTable prepares the partition table for structures in a volume and // returns it with the partition number of the root partition. -func GeneratePartitionTable(volume *gadget.Volume, sectorSize uint64, imgSize uint64, isSeeded bool) (partition.Table, int, error) { - partitionNumber, rootfsPartitionNumber := 1, -1 +func GeneratePartitionTable(volume *gadget.Volume, sectorSize uint64, imgSize uint64, isSeeded bool) (partition.Table, int, int, error) { + partitionNumber, rootfsPartitionNumber, bootPartitionNumber := 1, -1, -1 partitionTable := newPartitionTable(volume, sectorSize, imgSize) onDisk := gadget.OnDiskStructsFromGadget(volume) for i := range volume.Structure { structure := &volume.Structure[i] - if !structure.IsPartition() || helper.ShouldSkipStructure(structure, isSeeded) { + if !structure.IsPartition() { + continue + } + + // Record the actual partition number of the boot partition, as it + // might be useful for certain operations (like updating the bootloader) + if helper.IsSystemBootStructure(structure) { + bootPartitionNumber = partitionNumber + } + + if helper.ShouldSkipStructure(structure, isSeeded) { continue } // Record the actual partition number of the root partition, as it // might be useful for certain operations (like updating the bootloader) - if helper.IsRootfsStructure(structure) { //nolint:gosec,G301 + if helper.IsRootfsStructure(structure) { rootfsPartitionNumber = partitionNumber } @@ -85,13 +95,13 @@ func GeneratePartitionTable(volume *gadget.Volume, sectorSize uint64, imgSize ui structureType := getStructureType(structure, volume.Schema) err := partitionTable.AddPartition(structurePair, structureType) if err != nil { - return nil, rootfsPartitionNumber, err + return nil, rootfsPartitionNumber, bootPartitionNumber, err } partitionNumber++ } - return partitionTable.GetConcreteTable(), rootfsPartitionNumber, nil + return partitionTable.GetConcreteTable(), rootfsPartitionNumber, bootPartitionNumber, nil } // getStructureType extracts the structure type from the structure.Type considering diff --git a/internal/partition/partition_test.go b/internal/partition/partition_test.go index e6da73fe..4eca3011 100644 --- a/internal/partition/partition_test.go +++ b/internal/partition/partition_test.go @@ -168,6 +168,7 @@ func TestGeneratePartitionTable(t *testing.T) { args args wantPartitionTable partition.Table wantRootfsPartNumber int + wantBootPartNumber int expectedError string }{ { @@ -178,6 +179,7 @@ func TestGeneratePartitionTable(t *testing.T) { imgSize: uint64(4 * quantity.SizeKiB), }, wantRootfsPartNumber: 2, + wantBootPartNumber: -1, wantPartitionTable: &gpt.Table{ LogicalSectorSize: int(sectorSize512), PhysicalSectorSize: int(sectorSize512), @@ -206,6 +208,7 @@ func TestGeneratePartitionTable(t *testing.T) { imgSize: uint64(4 * quantity.SizeKiB), }, wantRootfsPartNumber: 2, + wantBootPartNumber: -1, wantPartitionTable: &gpt.Table{ LogicalSectorSize: int(sectorSize4k), PhysicalSectorSize: int(sectorSize4k), @@ -234,6 +237,7 @@ func TestGeneratePartitionTable(t *testing.T) { imgSize: uint64(4 * quantity.SizeKiB), }, wantRootfsPartNumber: 2, + wantBootPartNumber: -1, wantPartitionTable: &gpt.Table{ LogicalSectorSize: int(sectorSize512), PhysicalSectorSize: int(sectorSize512), @@ -262,6 +266,7 @@ func TestGeneratePartitionTable(t *testing.T) { imgSize: uint64(4 * quantity.SizeKiB), }, wantRootfsPartNumber: 2, + wantBootPartNumber: -1, expectedError: `The structure "writable" overlaps GPT header or GPT partition table`, }, { @@ -272,6 +277,7 @@ func TestGeneratePartitionTable(t *testing.T) { imgSize: uint64(4 * quantity.SizeKiB), }, wantRootfsPartNumber: -1, + wantBootPartNumber: -1, wantPartitionTable: &mbr.Table{ Partitions: []*mbr.Partition{ { @@ -288,11 +294,12 @@ func TestGeneratePartitionTable(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { asserter := &helper.Asserter{T: t} - gotPartitionTable, gotRootfsPartNumber, gotErr := GeneratePartitionTable(tt.args.volume, tt.args.sectorSize, tt.args.imgSize, tt.args.isSeeded) + gotPartitionTable, gotRootfsPartNumber, gotBootPartNumber, gotErr := GeneratePartitionTable(tt.args.volume, tt.args.sectorSize, tt.args.imgSize, tt.args.isSeeded) if len(tt.expectedError) == 0 { asserter.AssertErrNil(gotErr, true) asserter.AssertEqual(tt.wantRootfsPartNumber, gotRootfsPartNumber) + asserter.AssertEqual(tt.wantBootPartNumber, gotBootPartNumber) asserter.AssertEqual(tt.wantPartitionTable, gotPartitionTable) } else { asserter.AssertErrContains(gotErr, tt.expectedError) diff --git a/internal/statemachine/classic.go b/internal/statemachine/classic.go index 528aba42..e09493b8 100644 --- a/internal/statemachine/classic.go +++ b/internal/statemachine/classic.go @@ -507,7 +507,7 @@ func (s *StateMachine) addImgStates(states *[]stateFunc) { *states = append(*states, makeDiskState, - updateBootloaderState, + setupBootloaderState, ) } @@ -522,7 +522,7 @@ func (s *StateMachine) addQcow2States(states *[]stateFunc) { if !found { *states = append(*states, makeDiskState, - updateBootloaderState, + setupBootloaderState, ) } *states = append(*states, makeQcow2ImgState) diff --git a/internal/statemachine/classic_states.go b/internal/statemachine/classic_states.go index adc6294b..2f768888 100644 --- a/internal/statemachine/classic_states.go +++ b/internal/statemachine/classic_states.go @@ -1309,18 +1309,21 @@ func (stateMachine *StateMachine) makeQcow2Img() error { return nil } -var updateBootloaderState = stateFunc{"update_bootloader", (*StateMachine).updateBootloader} +var setupBootloaderState = stateFunc{"setup_bootloader", (*StateMachine).setupBootloader} -// updateBootloader determines the bootloader for each volume -// and runs the correct helper function to update the bootloader -func (stateMachine *StateMachine) updateBootloader() error { +// setupBootloader determines the bootloader for each volume +// and runs the correct helper function to install/update the bootloader +func (stateMachine *StateMachine) setupBootloader() error { if stateMachine.RootfsPartNum == -1 || stateMachine.RootfsVolName == "" { return fmt.Errorf("Error: could not determine partition number of the root filesystem") } + if stateMachine.BootPartNum == -1 { + return fmt.Errorf("Error: could not determine partition number of the boot filesystem") + } volume := stateMachine.GadgetInfo.Volumes[stateMachine.RootfsVolName] switch volume.Bootloader { case "grub": - err := stateMachine.updateGrub(stateMachine.RootfsVolName, stateMachine.RootfsPartNum) + err := stateMachine.setupGrub(stateMachine.RootfsVolName, stateMachine.RootfsPartNum, stateMachine.BootPartNum) if err != nil { return err } diff --git a/internal/statemachine/classic_test.go b/internal/statemachine/classic_test.go index 499525db..942995a0 100644 --- a/internal/statemachine/classic_test.go +++ b/internal/statemachine/classic_test.go @@ -195,7 +195,7 @@ func TestClassicStateMachine_calculateStates(t *testing.T) { "populate_bootfs_contents", "populate_prepare_partitions", "make_disk", - "update_bootloader", + "setup_bootloader", "generate_package_manifest", }, }, @@ -221,7 +221,7 @@ func TestClassicStateMachine_calculateStates(t *testing.T) { "populate_bootfs_contents", "populate_prepare_partitions", "make_disk", - "update_bootloader", + "setup_bootloader", "generate_package_manifest", }, }, @@ -248,7 +248,7 @@ func TestClassicStateMachine_calculateStates(t *testing.T) { "populate_bootfs_contents", "populate_prepare_partitions", "make_disk", - "update_bootloader", + "setup_bootloader", "generate_package_manifest", }, }, @@ -277,7 +277,7 @@ func TestClassicStateMachine_calculateStates(t *testing.T) { "populate_bootfs_contents", "populate_prepare_partitions", "make_disk", - "update_bootloader", + "setup_bootloader", "make_qcow2_image", "generate_package_manifest", "generate_filelist", @@ -302,7 +302,7 @@ func TestClassicStateMachine_calculateStates(t *testing.T) { "populate_bootfs_contents", "populate_prepare_partitions", "make_disk", - "update_bootloader", + "setup_bootloader", "generate_package_manifest", }, }, @@ -323,7 +323,7 @@ func TestClassicStateMachine_calculateStates(t *testing.T) { "populate_bootfs_contents", "populate_prepare_partitions", "make_disk", - "update_bootloader", + "setup_bootloader", "generate_package_manifest", }, }, @@ -349,7 +349,7 @@ func TestClassicStateMachine_calculateStates(t *testing.T) { "populate_bootfs_contents", "populate_prepare_partitions", "make_disk", - "update_bootloader", + "setup_bootloader", "generate_package_manifest", }, }, @@ -371,7 +371,7 @@ func TestClassicStateMachine_calculateStates(t *testing.T) { "populate_bootfs_contents", "populate_prepare_partitions", "make_disk", - "update_bootloader", + "setup_bootloader", "generate_package_manifest", }, }, @@ -400,7 +400,7 @@ func TestClassicStateMachine_calculateStates(t *testing.T) { "populate_bootfs_contents", "populate_prepare_partitions", "make_disk", - "update_bootloader", + "setup_bootloader", "generate_package_manifest", }, }, @@ -428,7 +428,7 @@ func TestClassicStateMachine_calculateStates(t *testing.T) { "populate_bootfs_contents", "populate_prepare_partitions", "make_disk", - "update_bootloader", + "setup_bootloader", "make_qcow2_image", }, }, @@ -564,7 +564,7 @@ func TestDisplayStates(t *testing.T) { [17] populate_bootfs_contents [18] populate_prepare_partitions [19] make_disk -[20] update_bootloader +[20] setup_bootloader [21] generate_package_manifest ` if !strings.Contains(string(readStdout), expectedStates) { @@ -4968,7 +4968,7 @@ func TestFailedUpdateBootloader(t *testing.T) { // has not been found in earlier steps stateMachine.RootfsPartNum = -1 stateMachine.RootfsVolName = "" - err = stateMachine.updateBootloader() + err = stateMachine.setupBootloader() asserter.AssertErrContains(err, "Error: could not determine partition number of the root filesystem") // place a test gadget tree in the scratch directory so we don't @@ -4999,13 +4999,13 @@ func TestFailedUpdateBootloader(t *testing.T) { asserter.AssertErrNil(err, true) err = stateMachine.loadGadgetYaml() asserter.AssertErrNil(err, true) - osMkdir = mockMkdir + osMkdirAll = mockMkdirAll t.Cleanup(func() { - osMkdir = os.Mkdir + osMkdirAll = os.MkdirAll }) - err = stateMachine.updateBootloader() - asserter.AssertErrContains(err, "Error creating scratch/loopback directory") + err = stateMachine.setupBootloader() + asserter.AssertErrContains(err, "Error creating scratch/loopback/boot/efi") } // TestUnsupportedBootloader tests that a warning is thrown if the @@ -5065,7 +5065,7 @@ func TestUnsupportedBootloader(t *testing.T) { defer restoreStdout() asserter.AssertErrNil(err, true) - err = stateMachine.updateBootloader() + err = stateMachine.setupBootloader() asserter.AssertErrNil(err, true) // restore stdout and examine what was printed diff --git a/internal/statemachine/common_states.go b/internal/statemachine/common_states.go index 1bd74aa4..694437fa 100644 --- a/internal/statemachine/common_states.go +++ b/internal/statemachine/common_states.go @@ -478,14 +478,15 @@ func (stateMachine *StateMachine) createDiskImage(volumeName string, volume *gad // partitionDisk generates a partition table and applies it to the disk func (stateMachine *StateMachine) partitionDisk(diskImg *diskutils.Disk, volume *gadget.Volume, volumeName string) error { - partitionTable, rootfsPartitionNumber, err := partition.GeneratePartitionTable(volume, uint64(stateMachine.SectorSize), uint64(diskImg.Size), stateMachine.IsSeeded) + partitionTable, rootfsPartitionNumber, bootPartitionNumber, err := partition.GeneratePartitionTable(volume, uint64(stateMachine.SectorSize), uint64(diskImg.Size), stateMachine.IsSeeded) if err != nil { return err } - // Save the rootfs partition number, for later use + // Save the rootfs/boot partition numbers, for later use // Store in any case, even if value is -1 to make it clear later it was not found stateMachine.RootfsPartNum = rootfsPartitionNumber + stateMachine.BootPartNum = bootPartitionNumber if rootfsPartitionNumber != -1 { stateMachine.RootfsVolName = volumeName } diff --git a/internal/statemachine/helper.go b/internal/statemachine/helper.go index 6efb8a97..df422aa7 100644 --- a/internal/statemachine/helper.go +++ b/internal/statemachine/helper.go @@ -932,45 +932,59 @@ func divertOSProber(mountDir string) (*exec.Cmd, *exec.Cmd) { return dpkgDivert(mountDir, "/etc/grub.d/30_os-prober") } -// updateGrub mounts the resulting image and runs update-grub -func (stateMachine *StateMachine) updateGrub(rootfsVolName string, rootfsPartNum int) (err error) { - // create a directory in which to mount the rootfs - mountDir := filepath.Join(stateMachine.tempDirs.scratch, "loopback") - err = osMkdir(mountDir, 0755) +// setupGrub mounts the resulting image and runs update-grub +func (stateMachine *StateMachine) setupGrub(rootfsVolName string, rootfsPartNum int, bootPartNum int) (err error) { + c := stateMachine.parent.(*ClassicStateMachine) + + // create directories in which to mount the rootfs and the boot partition + mountDir := filepath.Join(c.tempDirs.scratch, "loopback") + bootDir := filepath.Join(mountDir, "boot", "efi") + err = osMkdirAll(bootDir, 0755) if err != nil && !os.IsExist(err) { - return fmt.Errorf("Error creating scratch/loopback directory: %s", err.Error()) + return fmt.Errorf("Error creating scratch/loopback/boot/efi directory: %s", err.Error()) } // Slice used to store all the commands that need to be run - // to properly update grub.cfg in the chroot - // updateGrubCmds should be filled as a FIFO list - var updateGrubCmds []*exec.Cmd + // to properly setup grub in the chroot + // setupGrubCmds should be filled as a FIFO list + var setupGrubCmds []*exec.Cmd // Slice used to store all the commands that need to be run - // to properly cleanup everything after the update of grub.cfg - // updateGrubCmds should be filled as a LIFO list (so new entries should added at the start of the slice) + // to properly cleanup everything after the setup of grub + // teardownCmds should be filled as a LIFO list (so new entries should added at the start of the slice) var teardownCmds []*exec.Cmd defer func() { - err = execTeardownCmds(teardownCmds, stateMachine.commonFlags.Debug, err) + err = execTeardownCmds(teardownCmds, c.commonFlags.Debug, err) }() - imgPath := filepath.Join(stateMachine.commonFlags.OutputDir, stateMachine.VolumeNames[rootfsVolName]) + imgPath := filepath.Join(c.commonFlags.OutputDir, c.VolumeNames[rootfsVolName]) - loopUsed, losetupDetachCmd, err := stateMachine.associateLoopDevice(imgPath) + loopUsed, losetupDetachCmd, err := c.associateLoopDevice(imgPath) if err != nil { return err } + target := c.efiTarget(c.ImageDef.Architecture) + if len(target) == 0 { + return fmt.Errorf("no valid efi target for the provided architecture") + } + // detach the loopback device teardownCmds = append(teardownCmds, losetupDetachCmd) - updateGrubCmds = append(updateGrubCmds, + setupGrubCmds = append(setupGrubCmds, // mount the rootfs partition in which to run update-grub //nolint:gosec,G204 execCommand("mount", fmt.Sprintf("%sp%d", loopUsed, rootfsPartNum), mountDir, ), + // mount the boot partition + //nolint:gosec,G204 + execCommand("mount", + fmt.Sprintf("%sp%d", loopUsed, bootPartNum), + bootDir, + ), ) teardownCmds = append([]*exec.Cmd{execCommand("umount", mountDir)}, teardownCmds...) @@ -1012,7 +1026,7 @@ func (stateMachine *StateMachine) updateGrub(rootfsVolName string, rootfsPartNum err.Error(), ) } - updateGrubCmds = append(updateGrubCmds, mountCmds...) + setupGrubCmds = append(setupGrubCmds, mountCmds...) teardownCmds = append(umountCmds, teardownCmds...) } @@ -1020,13 +1034,36 @@ func (stateMachine *StateMachine) updateGrub(rootfsVolName string, rootfsPartNum execCommand("udevadm", "settle"), }, teardownCmds...) + setupGrubCmds = append(setupGrubCmds, + execCommand("chroot", + mountDir, + "grub-install", + loopUsed, + "--boot-directory=/boot", + "--efi-directory=/boot/efi", + fmt.Sprintf("--target=%s", target), + "--uefi-secure-boot", + "--no-nvram", + ), + ) + + if c.ImageDef.Architecture == "amd64" { + setupGrubCmds = append(setupGrubCmds, + execCommand("chroot", + mountDir, + "grub-install", + loopUsed, + "--target=i386-pc", + ), + ) + } + divert, undivert := divertOSProber(mountDir) - updateGrubCmds = append(updateGrubCmds, divert) + setupGrubCmds = append(setupGrubCmds, divert) teardownCmds = append([]*exec.Cmd{undivert}, teardownCmds...) - // actually run update-grub - updateGrubCmds = append(updateGrubCmds, + setupGrubCmds = append(setupGrubCmds, execCommand("chroot", mountDir, "update-grub", @@ -1034,10 +1071,23 @@ func (stateMachine *StateMachine) updateGrub(rootfsVolName string, rootfsPartNum ) // now run all the commands - err = helper.RunCmds(updateGrubCmds, stateMachine.commonFlags.Debug) + err = helper.RunCmds(setupGrubCmds, c.commonFlags.Debug) if err != nil { return err } return nil } + +func (stateMachine *StateMachine) efiTarget(arch string) string { + switch arch { + case "amd64": + return "x86_64-efi" + case "arm64": + return "arm64-efi" + case "armhf": + return "arm-efi" + default: + return "" + } +} diff --git a/internal/statemachine/helper_test.go b/internal/statemachine/helper_test.go index a698de0e..63ab6479 100644 --- a/internal/statemachine/helper_test.go +++ b/internal/statemachine/helper_test.go @@ -1089,13 +1089,21 @@ func TestFailedGetPreseededSnaps(t *testing.T) { asserter.AssertErrNil(err, true) } -// TestStateMachine_updateGrub_checkcmds checks commands to update grub order is ok -func TestStateMachine_updateGrub_checkcmds(t *testing.T) { +// TestStateMachine_setupGrub_checkcmds checks commands to update grub order is ok +func TestStateMachine_setupGrub_checkcmds(t *testing.T) { asserter := helper.Asserter{T: t} - var stateMachine StateMachine + var stateMachine ClassicStateMachine stateMachine.commonFlags, stateMachine.stateMachineFlags = helper.InitCommonOpts() stateMachine.commonFlags.Debug = true stateMachine.commonFlags.OutputDir = "/tmp" + stateMachine.parent = &stateMachine + stateMachine.ImageDef = imagedefinition.ImageDefinition{ + Architecture: getHostArch(), + Series: getHostSuite(), + Rootfs: &imagedefinition.Rootfs{ + Archive: "ubuntu", + }, + } err := stateMachine.makeTemporaryDirectories() asserter.AssertErrNil(err, true) @@ -1111,7 +1119,7 @@ func TestStateMachine_updateGrub_checkcmds(t *testing.T) { asserter.AssertErrNil(err, true) t.Cleanup(func() { restoreStdout() }) - err = stateMachine.updateGrub("", 2) + err = stateMachine.setupGrub("", 2, 1) asserter.AssertErrNil(err, true) restoreStdout() @@ -1120,10 +1128,13 @@ func TestStateMachine_updateGrub_checkcmds(t *testing.T) { expectedCmds := []*regexp.Regexp{ regexp.MustCompile("^mount .*p2 .*/scratch/loopback$"), + regexp.MustCompile("^mount .*p1 .*/scratch/loopback/boot/efi$"), regexp.MustCompile("^mount -t devtmpfs devtmpfs-build .*/scratch/loopback/dev$"), regexp.MustCompile("^mount -t devpts devpts-build -o nodev,nosuid .*/scratch/loopback/dev/pts$"), regexp.MustCompile("^mount -t proc proc-build .*/scratch/loopback/proc$"), regexp.MustCompile("^mount -t sysfs sysfs-build .*/scratch/loopback/sys$"), + regexp.MustCompile("^chroot .*/scratch/loopback grub-install .* --boot-directory=/boot --efi-directory=/boot/efi --target=x86_64-efi --uefi-secure-boot --no-nvram$"), + regexp.MustCompile("^chroot .*/scratch/loopback grub-install .* --target=i386-pc$"), regexp.MustCompile("^chroot .*/scratch/loopback dpkg-divert"), regexp.MustCompile("^chroot .*/scratch/loopback update-grub$"), regexp.MustCompile("^chroot .*/scratch/loopback dpkg-divert --remove"), @@ -1154,8 +1165,8 @@ func TestStateMachine_updateGrub_checkcmds(t *testing.T) { } } -// TestFailedUpdateGrub tests failures in the updateGrub function -func TestFailedUpdateGrub(t *testing.T) { +// TestFailedSetupGrub tests failures in the updateGrub function +func TestFailedSetupGrub(t *testing.T) { asserter := helper.Asserter{T: t} var stateMachine StateMachine stateMachine.commonFlags, stateMachine.stateMachineFlags = helper.InitCommonOpts() @@ -1170,7 +1181,7 @@ func TestFailedUpdateGrub(t *testing.T) { t.Cleanup(func() { osMkdir = os.Mkdir }) - err = stateMachine.updateGrub("", 0) + err = stateMachine.setupGrub("", 0, 0) asserter.AssertErrContains(err, "Error creating scratch/loopback directory") osMkdir = os.Mkdir @@ -1180,12 +1191,12 @@ func TestFailedUpdateGrub(t *testing.T) { t.Cleanup(func() { execCommand = exec.Command }) - err = stateMachine.updateGrub("", 0) + err = stateMachine.setupGrub("", 0, 0) asserter.AssertErrContains(err, "Error running losetup command") // now test a command failure that isn't losetup testCaseName = "TestFailedUpdateGrubOther" - err = stateMachine.updateGrub("", 0) + err = stateMachine.setupGrub("", 0, 0) asserter.AssertErrContains(err, "Error running command") execCommand = exec.Command } diff --git a/internal/statemachine/pack.go b/internal/statemachine/pack.go index 0b507b00..fc2690a9 100644 --- a/internal/statemachine/pack.go +++ b/internal/statemachine/pack.go @@ -15,7 +15,7 @@ var packStates = []stateFunc{ populateBootfsContentsState, populatePreparePartitionsState, makeDiskState, - updateBootloaderState, + setupBootloaderState, } // PackStateMachine embeds StateMachine and adds the command line flags specific to pack images diff --git a/internal/statemachine/state_machine.go b/internal/statemachine/state_machine.go index 636bff85..244cd889 100644 --- a/internal/statemachine/state_machine.go +++ b/internal/statemachine/state_machine.go @@ -104,6 +104,7 @@ type StateMachine struct { IsSeeded bool // core 20 images are seeded RootfsVolName string // volume on which the rootfs is located RootfsPartNum int // rootfs partition number + BootPartNum int // boot partition number SectorSize quantity.Size // parsed (converted) sector size RootfsSize quantity.Size tempDirs temporaryDirectories