From f2ab07cf856141ae5bbd4ea9bfffe48be8033c4b Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Wed, 5 Mar 2025 17:36:34 +0100 Subject: [PATCH] stages/grub2.inst: grub2-mkimage in tmpdir Use a temporary directory for the output of grub2-mkimage. This makes the stage clean up the grub2-core.img from the build root after its done. It also has the nice side-effect that unit tests that call the stage are independent. Previously, a bug in the stage *might* have been missed if a certain configuration of the stage was not creating the grub2-core.img. One unit test could create an image at the fixed path (/var/tmp/grub2-core.img) and then another one could call the stage with the buggy configuration but the `shutil.copyfile()` call at the end of the stage would succeed because it would find the image from the previous stage run. To accommodate for this change, the unit test with the mocked run call is adjusted to intercept the random tmp output path and use it to create a fake file for the stage to succeed. --- stages/org.osbuild.grub2.inst | 22 ++++++++++++---------- stages/test/test_grub2_inst.py | 18 +++++++++++++++++- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/stages/org.osbuild.grub2.inst b/stages/org.osbuild.grub2.inst index f6578aec..4f4bf6ef 100755 --- a/stages/org.osbuild.grub2.inst +++ b/stages/org.osbuild.grub2.inst @@ -4,6 +4,7 @@ import shutil import struct import subprocess import sys +import tempfile from typing import BinaryIO, Dict import osbuild.api @@ -70,12 +71,11 @@ def write_core_image(core_f, image_f, location, sector_size): shutil.copyfileobj(core_f, image_f) -def core_mkimage(platform: str, prefix: str, options: Dict): +def core_mkimage(platform: str, prefix: str, options: Dict, core_dir: str): pt_label = options["partlabel"] fs_type = options["filesystem"] - os.makedirs("/var/tmp/", exist_ok=True) - core_path = "/var/tmp/grub2-core.img" + core_path = os.path.join(core_dir, "grub2-core.img") # Create the level-2 & 3 stages of the bootloader, aka the core # it consists of the kernel plus the core modules required to @@ -174,14 +174,16 @@ def main(tree, options): else: prefix = options["prefix"]["path"] print(f"prefix: {prefix}") - core_path = core_mkimage(platform, prefix, options["core"]) - location = options.get("location") - if location: - patch_core(location, core_path, image, sector_size, platform) - else: - # If location isn't set, use the image file as-is instead of with the MBR - shutil.copyfile(core_path, image) + with tempfile.TemporaryDirectory() as core_tmpdir: + core_path = core_mkimage(platform, prefix, options["core"], core_tmpdir) + + location = options.get("location") + if location: + patch_core(location, core_path, image, sector_size, platform) + else: + # If location isn't set, use the image file as-is instead of with the MBR + shutil.copyfile(core_path, image) return 0 diff --git a/stages/test/test_grub2_inst.py b/stages/test/test_grub2_inst.py index f64e2f53..a49229e1 100644 --- a/stages/test/test_grub2_inst.py +++ b/stages/test/test_grub2_inst.py @@ -89,14 +89,30 @@ def test_grub2_partition_mocked(mocked_run, tmp_path, stage_module): "path": "/boot/", }, } + + output_path = "" + + def mock_side_effect(args, **_): + """ + Intercept the args to subprocess.run() to leak the random output path and create the file that's expected by the + shutil.copy() in the stage's main(). + """ + nonlocal output_path + output_path = args[args.index("--output") + 1] + with open(output_path, "wb") as fake_image_file: + fake_image_file.write(b"I am a grub core image") + + mocked_run.side_effect = mock_side_effect + stage_module.main(tmp_path, options) + assert os.path.basename(output_path) == "grub2-core.img" assert mocked_run.call_args_list == [ call(["grub2-mkimage", "--verbose", "--directory", "/usr/lib/grub/some-platform", "--prefix", "(,gpt1)/boot/", "--format", "some-platform", "--compression", "auto", - "--output", "/var/tmp/grub2-core.img", + "--output", output_path, "part_gpt", "ext2"], check=True), ]