diff --git a/crates/lib/src/bootc_composefs/boot.rs b/crates/lib/src/bootc_composefs/boot.rs index 9909aa0c0..232d863d8 100644 --- a/crates/lib/src/bootc_composefs/boot.rs +++ b/crates/lib/src/bootc_composefs/boot.rs @@ -64,7 +64,8 @@ use std::cell::Cell; use std::fs::create_dir_all; use std::io::{Read, Seek, SeekFrom, Write}; -use std::path::{Path, PathBuf}; +use std::os::fd::AsFd; +use std::path::Path; use std::sync::Arc; use anyhow::{Context, Result, anyhow, bail}; @@ -77,7 +78,7 @@ use cap_std_ext::{ use clap::ValueEnum; use composefs::fs::read_file; use composefs::fsverity::{FsVerityHashValue, Sha512HashValue}; -use composefs::tree::RegularFile; +use composefs::tree::{FileSystem, RegularFile}; use composefs_boot::bootloader::{ BootEntry as ComposefsBootEntry, EFI_ADDON_DIR_EXT, EFI_ADDON_FILE_EXT, EFI_EXT, PEType, UsrLibModulesVmlinuz, get_boot_resources, @@ -102,7 +103,10 @@ use crate::composefs_consts::{TYPE1_BOOT_DIR_PREFIX, TYPE1_ENT_PATH, TYPE1_ENT_P use crate::parsers::bls_config::{BLSConfig, BLSConfigType, EFIKey}; use crate::spec::BootloaderKind; use crate::task::Task; -use crate::{bootc_composefs::repo::open_composefs_repo, store::Storage}; +use crate::{ + bootc_composefs::repo::open_composefs_repo, + store::{ComposefsRepository, Storage}, +}; use crate::{bootc_composefs::status::get_sorted_grub_uki_boot_entries, install::PostFetchState}; use crate::{ composefs_consts::{ @@ -144,6 +148,93 @@ pub(crate) struct UKIDigestMismatch { pub uki_name: Option, } +pub(crate) fn print_uki_dumpfile_diff( + mismatch: &UKIDigestMismatch, + repo: &ComposefsRepository, + fs: &FileSystem, +) { + let dumpfile_name = mismatch + .uki_name + .as_ref() + .and_then(|x| x.strip_suffix(EFI_EXT).map(|x| format!("{x}.dump"))); + + let Some(dumpfile_name) = &dumpfile_name else { + return; + }; + + let Some(stored_content) = read_dumpfile_from_fs(fs, dumpfile_name, repo) else { + tracing::debug!("Dumpfile {dumpfile_name} not found in filesystem"); + return; + }; + + let Ok(tempdir) = tempfile::tempdir() else { + return; + }; + let path = tempdir.path(); + let Ok(tempdir_cap) = Dir::open_ambient_dir(path, ambient_authority()) else { + return; + }; + + let Ok(mut stored_file) = tempdir_cap.create("stored") else { + return; + }; + if stored_file.write_all(&stored_content).is_err() { + return; + } + + let Ok(mut current_file) = tempdir_cap.create("current") else { + return; + }; + if let Err(e) = dumpfile::write_dumpfile(&mut current_file, fs) { + tracing::debug!("Writing dumpfile failed: {e}"); + return; + } + + let mut cmd = std::process::Command::new("diff"); + cmd.arg("--color=auto") + .arg(format!("{}/stored", path.display())) + .arg(format!("{}/current", path.display())); + + // Redirect stdout to stderr since this is diagnostic output + if let Ok(fd) = std::io::stderr().as_fd().try_clone_to_owned() { + cmd.stdout(fd); + } + + if let Err(e) = cmd.status() { + tracing::warn!("diffing dumpfiles failed with Err: {e:?}"); + } +} + +fn read_regular_file( + file: &RegularFile, + repo: &ComposefsRepository, +) -> Option> { + match file { + RegularFile::External(object_id, _) | RegularFile::ExternalNoVerity(object_id, _) => { + repo.read_object(object_id).ok() + } + RegularFile::Inline(data) => Some(data.to_vec()), + RegularFile::Sparse(_) => None, + } +} + +fn read_dumpfile_from_fs( + fs: &FileSystem, + dumpfile_name: &str, + repo: &ComposefsRepository, +) -> Option> { + let root = fs.as_dir(); + let dumpfile_os = std::ffi::OsStr::new(dumpfile_name); + + if let Ok(boot_dir) = root.get_directory_ref("boot".as_ref()) { + if let Ok(file) = boot_dir.get_file(dumpfile_os) { + return read_regular_file(file, repo); + } + } + + None +} + pub(crate) enum BootSetupType<'a> { /// For initial setup, i.e. install to-disk Setup((&'a RootSetup, &'a State, &'a PostFetchState)), @@ -1618,64 +1709,7 @@ pub(crate) async fn setup_composefs_boot( Ok(boot_digest) => boot_digest, Err(e) => match e.downcast::() { Ok(mismatch) => { - // We expect the dumpfile to be named the same as the UKI - // Ex. UKI - 6.19.14-108.fc42.x86_64.efi - // Dumpfile - 6.19.14-108.fc42.x86_64.dump - let dumpfile_name = mismatch - .uki_name - .as_ref() - .and_then(|x| x.strip_suffix(EFI_EXT).map(|x| format!("{x}.dump"))); - - let Some(dumpfile_name) = &dumpfile_name else { - return Err(mismatch.into()); - }; - - let dump = composefs_ctl::dump_files( - &repo, - &id.to_hex(), - &vec![PathBuf::from(dumpfile_name)], - true, - ); - - let Ok(dump) = dump else { - tracing::debug!("Dumpfile not found for diff"); - return Err(mismatch.into()); - }; - - // SAFETY: This output is always UTF-8 compatible as it's of the form - // - let text = std::str::from_utf8(&dump)?; - let obj_path = text.split_whitespace().nth(1); - - let Some(obj_path) = obj_path else { - return Err(mismatch.into()); - }; - - let tempdir = tempfile::tempdir()?; - let path = tempdir.path(); - let tempdir = Dir::open_ambient_dir(path, ambient_authority())?; - - let mut tmpfile = tempdir.create("current")?; - dumpfile::write_dumpfile(&mut tmpfile, &fs).context("Writing dumpfile")?; - - let mut cmd = std::process::Command::new("diff"); - let out = cmd - .arg("--color=auto") - .arg( - root_setup - .physical_root_path - .join("sysroot/composefs/objects") - .join(obj_path), - ) - .arg(format!("{}/current", path.display())) - .status(); - - // Intentionally not short-circuiting here as the real error is digest - // mismtach - if let Err(e) = out { - tracing::warn!("diffing dumpfiles failed with Err: {e:?}"); - }; - + print_uki_dumpfile_diff(&mismatch, &repo, &fs); return Err(mismatch.into()); } Err(e) => Err(e)?, diff --git a/crates/lib/src/bootc_composefs/repo.rs b/crates/lib/src/bootc_composefs/repo.rs index 4d9fe068d..9454e61c5 100644 --- a/crates/lib/src/bootc_composefs/repo.rs +++ b/crates/lib/src/bootc_composefs/repo.rs @@ -42,6 +42,7 @@ use anyhow::{Context, Result}; use composefs::fsverity::{FsVerityHashValue, Sha512HashValue}; use composefs::repository::RepositoryConfig; +use composefs::tree::FileSystem; use composefs_boot::bootloader::{BootEntry as ComposefsBootEntry, get_boot_resources}; use composefs_ctl::composefs; use composefs_ctl::composefs_boot; @@ -188,6 +189,8 @@ pub(crate) struct PullRepoResult { pub(crate) id: Sha512HashValue, /// The OCI manifest content digest (e.g. "sha256:abc...") pub(crate) manifest_digest: String, + /// The untransformed OCI filesystem (still has /boot, /sysroot, etc.) + pub(crate) fs: FileSystem, } /// Pull an image directly into the composefs repository via skopeo. @@ -424,6 +427,7 @@ pub(crate) async fn pull_composefs_repo( entries, id, manifest_digest: pull_result.manifest_digest.to_string(), + fs, }) } diff --git a/crates/lib/src/bootc_composefs/update.rs b/crates/lib/src/bootc_composefs/update.rs index 05cb989b5..7c97c353a 100644 --- a/crates/lib/src/bootc_composefs/update.rs +++ b/crates/lib/src/bootc_composefs/update.rs @@ -17,7 +17,10 @@ use crate::bootc_composefs::gc::GCOpts; use crate::spec::BootloaderKind; use crate::{ bootc_composefs::{ - boot::{BootSetupType, BootType, setup_composefs_bls_boot, setup_composefs_uki_boot}, + boot::{ + BootSetupType, BootType, UKIDigestMismatch, print_uki_dumpfile_diff, + setup_composefs_bls_boot, setup_composefs_uki_boot, + }, gc::composefs_gc, repo::pull_composefs_repo, service::start_finalize_stated_svc, @@ -263,6 +266,7 @@ pub(crate) async fn do_upgrade( entries, id, manifest_digest, + fs: oci_fs, } = pull_composefs_repo( imgref, booted_cfs.cmdline.allow_missing_fsverity, @@ -320,12 +324,25 @@ pub(crate) async fn do_upgrade( &mounted_fs, )?, - BootType::Uki => setup_composefs_uki_boot( - BootSetupType::Upgrade((storage, booted_cfs, &host)), - &repo, - &id, - entries, - )?, + BootType::Uki => { + let uki_setup_result = setup_composefs_uki_boot( + BootSetupType::Upgrade((storage, booted_cfs, &host)), + &repo, + &id, + entries, + ); + + match uki_setup_result { + Ok(boot_digest) => boot_digest, + Err(e) => match e.downcast::() { + Ok(mismatch) => { + print_uki_dumpfile_diff(&mismatch, &repo, &oci_fs); + return Err(mismatch.into()); + } + Err(e) => Err(e)?, + }, + } + } }; // `repo` holds its own flock(LOCK_SH) on /sysroot/composefs, taken out by diff --git a/tmt/plans/integration.fmf b/tmt/plans/integration.fmf index 271d862ef..20a1ce693 100644 --- a/tmt/plans/integration.fmf +++ b/tmt/plans/integration.fmf @@ -295,7 +295,7 @@ execute: - /tmt/tests/tests/test-46-etc-merge-conflict /plan-48-composefs-uki-dumpfile: - summary: Test composefs garbage collection for UKI + summary: Test composefs UKI dumpfile diff print discover: how: fmf test: diff --git a/tmt/tests/booted/test-composefs-uki-dumpfile.nu b/tmt/tests/booted/test-composefs-uki-dumpfile.nu index 8ba286364..e238eee7b 100644 --- a/tmt/tests/booted/test-composefs-uki-dumpfile.nu +++ b/tmt/tests/booted/test-composefs-uki-dumpfile.nu @@ -1,13 +1,14 @@ # number: 48 # tmt: -# summary: Test composefs garbage collection for UKI +# summary: Test composefs UKI dumpfile diff print # duration: 30m use std assert use tap.nu -# FIXME(Johan-Liebert1): This job is disabled for now -exit 0 +if not (tap is_composefs) { + exit 0 +} # bootc status let st = bootc status --json | from json @@ -37,7 +38,7 @@ def first_boot [] { let result = do { bootc switch --transport containers-storage localhost/dump-diff } | complete - let actual_digest = bootc internals cfs oci compute-id $"@(podman images --no-trunc | grep dump-diff | awk '{print $3}')" + let actual_digest = bootc internals cfs oci compute-id --bootable $"@(podman images --no-trunc | grep dump-diff | awk '{print $3}')" assert ($result.exit_code != 0) "bootc switch should fail" diff --git a/tmt/tests/tests.fmf b/tmt/tests/tests.fmf index 486e96cfa..2cae2db9e 100644 --- a/tmt/tests/tests.fmf +++ b/tmt/tests/tests.fmf @@ -185,6 +185,6 @@ check: test: nu booted/test-etc-merge-conflict.nu /test-48-composefs-uki-dumpfile: - summary: Test composefs garbage collection for UKI + summary: Test composefs UKI dumpfile diff print duration: 30m test: nu booted/test-composefs-uki-dumpfile.nu