From 3cd21cfe760c9e7633ee8c13232af33d9b43a0a9 Mon Sep 17 00:00:00 2001 From: HuijingHei Date: Mon, 6 May 2024 15:55:59 +0800 Subject: [PATCH 1/2] Add `get_efi_vendor` method to both the bios and efi backends See Colin's comment https://github.com/coreos/bootupd/pull/646#issuecomment-2089226909 --- Cargo.lock | 20 ++++++++++ Cargo.toml | 1 + src/bios.rs | 4 ++ src/bootupd.rs | 9 +++-- src/component.rs | 42 ++++++++++++++++++++ src/efi.rs | 53 +++++++++++++++++++++---- src/grubconfigs.rs | 98 ++++++++++++---------------------------------- 7 files changed, 143 insertions(+), 84 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index df0b26f1c..fe1758caf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -90,6 +90,7 @@ dependencies = [ "serde", "serde_json", "tempfile", + "walkdir", "widestring", ] @@ -699,6 +700,15 @@ version = "1.0.16" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f98d2aa92eebf49b69786be48e4477826b256916e84a57ff2a4f21923b48eb4c" +[[package]] +name = "same-file" +version = "1.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93fc1dc3aaa9bfed95e02e6eadabb4baf7e3078b0bd1b4d7b6b0b68378900502" +dependencies = [ + "winapi-util", +] + [[package]] name = "serde" version = "1.0.200" @@ -855,6 +865,16 @@ version = "0.9.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f" +[[package]] +name = "walkdir" +version = "2.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "29790946404f91d9c5d06f9874efddea1dc06c5efe94541a7d6863108e3a5e4b" +dependencies = [ + "same-file", + "winapi-util", +] + [[package]] name = "wasi" version = "0.11.0+wasi-snapshot-preview1" diff --git a/Cargo.toml b/Cargo.toml index 9651cbf77..d92820f31 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,6 +38,7 @@ serde = { version = "^1.0", features = ["derive"] } serde_json = "^1.0" tempfile = "^3.10" widestring = "1.1.0" +walkdir = "2.3.2" [profile.release] # We assume we're being delivered via e.g. RPM which supports split debuginfo diff --git a/src/bios.rs b/src/bios.rs index 9f2546e90..a3fb23fc1 100644 --- a/src/bios.rs +++ b/src/bios.rs @@ -155,4 +155,8 @@ impl Component for Bios { fn validate(&self, _: &InstalledContent) -> Result { Ok(ValidationResult::Skip) } + + fn get_efi_vendor(&self, _: &openat::Dir) -> Result> { + Ok(None) + } } diff --git a/src/bootupd.rs b/src/bootupd.rs index be218b79d..d0eef93c3 100644 --- a/src/bootupd.rs +++ b/src/bootupd.rs @@ -83,7 +83,7 @@ pub(crate) fn install( } let mut state = SavedState::default(); - let mut installed_efi = false; + let mut installed_efi_vendor = None; for &component in target_components.iter() { // skip for BIOS if device is empty if component.name() == "BIOS" && device.is_empty() { @@ -100,8 +100,9 @@ pub(crate) fn install( log::info!("Installed {} {}", component.name(), meta.meta.version); state.installed.insert(component.name().into(), meta); // Yes this is a hack...the Component thing just turns out to be too generic. - if component.name() == "EFI" { - installed_efi = true; + if let Some(vendor) = component.get_efi_vendor(&source_root)? { + assert!(installed_efi_vendor.is_none()); + installed_efi_vendor = Some(vendor); } } let sysroot = &openat::Dir::open(dest_root)?; @@ -115,7 +116,7 @@ pub(crate) fn install( target_arch = "aarch64", target_arch = "powerpc64" ))] - crate::grubconfigs::install(sysroot, installed_efi, uuid)?; + crate::grubconfigs::install(sysroot, installed_efi_vendor.as_deref(), uuid)?; // On other architectures, assume that there's nothing to do. } None => {} diff --git a/src/component.rs b/src/component.rs index dc1c6068a..65be68d15 100644 --- a/src/component.rs +++ b/src/component.rs @@ -72,6 +72,9 @@ pub(crate) trait Component { /// Used on the client to validate an installed version. fn validate(&self, current: &InstalledContent) -> Result; + + /// Locating efi vendor dir + fn get_efi_vendor(&self, sysroot: &openat::Dir) -> Result>; } /// Given a component name, create an implementation. @@ -140,3 +143,42 @@ pub(crate) fn get_component_update( Ok(None) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_get_efi_vendor() -> Result<()> { + let td = tempfile::tempdir()?; + let tdp = td.path(); + let tdp_updates = tdp.join("usr/lib/bootupd/updates"); + let td = openat::Dir::open(tdp)?; + std::fs::create_dir_all(tdp_updates.join("EFI/BOOT"))?; + std::fs::create_dir_all(tdp_updates.join("EFI/fedora"))?; + std::fs::create_dir_all(tdp_updates.join("EFI/centos"))?; + std::fs::write( + tdp_updates.join("EFI/fedora").join(crate::efi::SHIM), + "shim data", + )?; + std::fs::write( + tdp_updates.join("EFI/centos").join(crate::efi::SHIM), + "shim data", + )?; + + let all_components = crate::bootupd::get_components(); + let target_components: Vec<_> = all_components.values().collect(); + for &component in target_components.iter() { + if component.name() == "BIOS" { + assert_eq!(component.get_efi_vendor(&td)?, None); + } + if component.name() == "EFI" { + let x = component.get_efi_vendor(&td); + assert_eq!(x.is_err(), true); + std::fs::remove_dir_all(tdp_updates.join("EFI/centos"))?; + assert_eq!(component.get_efi_vendor(&td)?, Some("fedora".to_string())); + } + } + Ok(()) + } +} diff --git a/src/efi.rs b/src/efi.rs index a8b7d5286..e621726a0 100644 --- a/src/efi.rs +++ b/src/efi.rs @@ -12,6 +12,7 @@ use std::process::Command; use anyhow::{bail, Context, Result}; use fn_error_context::context; use openat_ext::OpenatDirExt; +use walkdir::WalkDir; use widestring::U16CString; use crate::filetree; @@ -131,16 +132,11 @@ impl Efi { } #[context("Updating EFI firmware variables")] - fn update_firmware(&self, device: &str, espdir: &openat::Dir) -> Result<()> { + fn update_firmware(&self, device: &str, espdir: &openat::Dir, vendordir: &str) -> Result<()> { if !is_efi_booted()? { log::debug!("Not booted via EFI, skipping firmware update"); return Ok(()); } - let efidir = &espdir.sub_dir("EFI").context("Opening EFI")?; - let vendordir = super::grubconfigs::find_efi_vendordir(efidir)?; - let vendordir = vendordir - .to_str() - .ok_or_else(|| anyhow::anyhow!("Invalid non-UTF-8 vendordir"))?; clear_efi_current()?; set_efi_current(device, espdir, vendordir) } @@ -319,7 +315,9 @@ impl Component for Efi { anyhow::bail!("Failed to copy"); } if update_firmware { - self.update_firmware(device, destd)? + if let Some(vendordir) = self.get_efi_vendor(&src_root)? { + self.update_firmware(device, destd, &vendordir)? + } } Ok(InstalledContent { meta, @@ -417,6 +415,28 @@ impl Component for Efi { Ok(ValidationResult::Valid) } } + + fn get_efi_vendor(&self, sysroot: &openat::Dir) -> Result> { + let updated = sysroot + .sub_dir(&component_updatedirname(self)) + .context("opening update dir")?; + let shim_files = find_file_recursive(updated.recover_path()?, SHIM)?; + + // Does not support multiple shim for efi + if shim_files.len() > 1 { + anyhow::bail!("Found multiple {SHIM} in the image"); + } + if let Some(p) = shim_files.first() { + let p = p + .parent() + .unwrap() + .file_name() + .ok_or_else(|| anyhow::anyhow!("No file name found"))?; + Ok(Some(p.to_string_lossy().into_owned())) + } else { + anyhow::bail!("Failed to find {SHIM} in the image") + } + } } impl Drop for Efi { @@ -507,3 +527,22 @@ pub(crate) fn set_efi_current(device: &str, espdir: &openat::Dir, vendordir: &st } anyhow::Ok(()) } + +#[context("Find target file recursively")] +fn find_file_recursive>(dir: P, target_file: &str) -> Result> { + let mut result = Vec::new(); + + for entry in WalkDir::new(dir).into_iter().filter_map(|e| e.ok()) { + if entry.file_type().is_file() { + if let Some(file_name) = entry.file_name().to_str() { + if file_name == target_file { + if let Some(path) = entry.path().to_str() { + result.push(path.into()); + } + } + } + } + } + + Ok(result) +} diff --git a/src/grubconfigs.rs b/src/grubconfigs.rs index 1ba9832be..09aeebfb3 100644 --- a/src/grubconfigs.rs +++ b/src/grubconfigs.rs @@ -5,37 +5,18 @@ use anyhow::{anyhow, Context, Result}; use fn_error_context::context; use openat_ext::OpenatDirExt; -use crate::efi::SHIM; - /// The subdirectory of /boot we use const GRUB2DIR: &str = "grub2"; const CONFIGDIR: &str = "/usr/lib/bootupd/grub2-static"; const DROPINDIR: &str = "configs.d"; -#[context("Locating EFI vendordir")] -pub(crate) fn find_efi_vendordir(efidir: &openat::Dir) -> Result { - for d in efidir.list_dir(".")? { - let d = d?; - let meta = efidir.metadata(d.file_name())?; - if !meta.is_dir() { - continue; - } - // skip if not find shim under dir - let dir = efidir.sub_dir(d.file_name())?; - for entry in dir.list_dir(".")? { - let entry = entry?; - if entry.file_name() != SHIM { - continue; - } - return Ok(d.file_name().into()); - } - } - anyhow::bail!("Failed to find EFI vendor dir that contains {SHIM}") -} - /// Install the static GRUB config files. #[context("Installing static GRUB configs")] -pub(crate) fn install(target_root: &openat::Dir, efi: bool, write_uuid: bool) -> Result<()> { +pub(crate) fn install( + target_root: &openat::Dir, + installed_efi_vendor: Option<&str>, + write_uuid: bool, +) -> Result<()> { let bootdir = &target_root.sub_dir("boot").context("Opening /boot")?; let boot_is_mount = { let root_dev = target_root.self_metadata()?.stat().st_dev; @@ -99,29 +80,26 @@ pub(crate) fn install(target_root: &openat::Dir, efi: bool, write_uuid: bool) -> None }; - let efidir = efi - .then(|| { - target_root - .sub_dir_optional("boot/efi/EFI") - .context("Opening /boot/efi/EFI") - }) - .transpose()? - .flatten(); - if let Some(efidir) = efidir.as_ref() { - let vendordir = find_efi_vendordir(efidir)?; + if let Some(vendordir) = installed_efi_vendor { log::debug!("vendordir={:?}", &vendordir); - let target = &vendordir.join("grub.cfg"); - efidir - .copy_file(&Path::new(CONFIGDIR).join("grub-static-efi.cfg"), target) - .context("Copying static EFI")?; - println!("Installed: {target:?}"); - if let Some(uuid_path) = uuid_path { - // SAFETY: we always have a filename - let filename = Path::new(&uuid_path).file_name().unwrap(); - let target = &vendordir.join(filename); - bootdir - .copy_file_at(uuid_path, efidir, target) - .context("Writing bootuuid.cfg to efi dir")?; + let vendor = PathBuf::from(vendordir); + let target = &vendor.join("grub.cfg"); + let dest_efidir = target_root + .sub_dir_optional("boot/efi/EFI") + .context("Opening /boot/efi/EFI")?; + if let Some(efidir) = dest_efidir { + efidir + .copy_file(&Path::new(CONFIGDIR).join("grub-static-efi.cfg"), target) + .context("Copying static EFI")?; + println!("Installed: {target:?}"); + if let Some(uuid_path) = uuid_path { + // SAFETY: we always have a filename + let filename = Path::new(&uuid_path).file_name().unwrap(); + let target = &vendor.join(filename); + bootdir + .copy_file_at(uuid_path, &efidir, target) + .context("Writing bootuuid.cfg to efi dir")?; + } } } @@ -142,36 +120,10 @@ mod tests { std::fs::create_dir_all(tdp.join("boot/grub2"))?; std::fs::create_dir_all(tdp.join("boot/efi/EFI/BOOT"))?; std::fs::create_dir_all(tdp.join("boot/efi/EFI/fedora"))?; - install(&td, true, false).unwrap(); + install(&td, Some("fedora"), false).unwrap(); assert!(td.exists("boot/grub2/grub.cfg")?); assert!(td.exists("boot/efi/EFI/fedora/grub.cfg")?); Ok(()) } - - #[test] - fn test_find_efi_vendordir() -> Result<()> { - let td = tempfile::tempdir()?; - let tdp = td.path(); - let efidir = tdp.join("EFI"); - std::fs::create_dir_all(efidir.join("BOOT"))?; - std::fs::create_dir_all(efidir.join("dell"))?; - std::fs::create_dir_all(efidir.join("fedora"))?; - let td = openat::Dir::open(&efidir)?; - - std::fs::write(efidir.join("dell").join("foo"), "foo data")?; - std::fs::write(efidir.join("fedora").join("grub.cfg"), "grub config")?; - std::fs::write(efidir.join("fedora").join(SHIM), "shim data")?; - - assert!(td.exists("BOOT")?); - assert!(td.exists("dell/foo")?); - assert!(td.exists("fedora/grub.cfg")?); - assert!(td.exists(format!("fedora/{SHIM}"))?); - assert_eq!(find_efi_vendordir(&td)?.to_str(), Some("fedora")); - - std::fs::remove_file(efidir.join("fedora").join(SHIM))?; - let x = find_efi_vendordir(&td); - assert_eq!(x.is_err(), true); - Ok(()) - } } From f7c70f77d0d7fdc973e6bd88caf644aa35b6f46f Mon Sep 17 00:00:00 2001 From: HuijingHei Date: Wed, 8 May 2024 19:36:31 +0800 Subject: [PATCH 2/2] ci: add `bootc install to-disk` test in c9s-bootc-e2e --- .github/workflows/ci.yml | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dc6eb6b24..0e6111914 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,7 +39,23 @@ jobs: tags: localhost/bootupd:latest - name: Copy to podman run: sudo skopeo copy docker-daemon:localhost/bootupd:latest containers-storage:localhost/bootupd:latest - - name: bootc install + - name: bootc install to disk + run: | + set -xeuo pipefail + sudo truncate -s 10G myimage.raw + sudo podman run --rm -ti --privileged -v .:/target --pid=host --security-opt label=disable \ + -v /var/lib/containers:/var/lib/containers \ + -v /dev:/dev \ + localhost/bootupd:latest bootc install to-disk --skip-fetch-check \ + --disable-selinux --generic-image --via-loopback /target/myimage.raw + # Verify we installed grub.cfg and shim on the disk + sudo losetup -P -f myimage.raw + device=$(losetup --list --noheadings --output NAME,BACK-FILE | grep myimage.raw | awk '{print $1}') + sudo mount "${device}p2" /mnt/ + sudo ls /mnt/EFI/centos/{grub.cfg,shimx64.efi} + sudo losetup -D "${device}" + sudo rm -f myimage.raw + - name: bootc install to filesystem run: | set -xeuo pipefail sudo podman run --rm -ti --privileged -v /:/target --pid=host --security-opt label=disable \