From dcc5db5be7a6388d6964e1d3b050248440bb3734 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Thu, 25 Sep 2025 10:53:10 +0530 Subject: [PATCH 1/2] etc-merge: Fix directory removal We weren't checking if the deleted path is a file or a directory and were calling `remove_file` unconditionally. Update to check for file/directory first Signed-off-by: Pragyan Poudyal --- crates/etc-merge/src/lib.rs | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/crates/etc-merge/src/lib.rs b/crates/etc-merge/src/lib.rs index b6388e2a5..1ab647e15 100644 --- a/crates/etc-merge/src/lib.rs +++ b/crates/etc-merge/src/lib.rs @@ -732,14 +732,33 @@ pub fn merge( .context("Merging modified files")?; for removed in diff.removed { - match new_etc_fd.remove_file(&removed) { - Ok(..) => { /* no-op */ } - // Removed file's not present in the new etc dir, nothing to do - Err(e) if e.kind() == std::io::ErrorKind::NotFound => continue, - Err(e) => Err(e)?, + let stat = new_etc_fd.metadata_optional(&removed)?; + + let stat = match stat { + Some(s) => s, + + None => { + // File/dir doesn't exist in new_etc + // Basically a no-op + continue; + } + }; + + if stat.is_file() || stat.is_symlink() { + match new_etc_fd.remove_file(&removed) { + Ok(..) => { /* no-op */ } + Err(e) => Err(e)?, + } } - println!("- Removed file {removed:?}"); + if stat.is_dir() { + // We only add the directory to the removed array, if the entire directory was deleted + // So `remove_dir_all` should be okay here + match new_etc_fd.remove_dir_all(&removed) { + Ok(..) => { /* no-op */ } + Err(e) => Err(e)?, + } + } } Ok(()) From 1b3915326dff8c97a76c679e35e8fba666ea3ef3 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Thu, 25 Sep 2025 11:14:42 +0530 Subject: [PATCH 2/2] cli: Add option to current vs default /etc Signed-off-by: Pragyan Poudyal --- crates/etc-merge/src/lib.rs | 60 +++++++++++----------- crates/lib/src/bootc_composefs/finalize.rs | 28 +++++++++- crates/lib/src/cli.rs | 18 +++++-- 3 files changed, 70 insertions(+), 36 deletions(-) diff --git a/crates/etc-merge/src/lib.rs b/crates/etc-merge/src/lib.rs index 1ab647e15..6f32d4af5 100644 --- a/crates/etc-merge/src/lib.rs +++ b/crates/etc-merge/src/lib.rs @@ -305,16 +305,16 @@ fn get_modifications( /// [`anyhow::Result`] containing a tuple of directory trees in the order: /// /// 1. `pristine_etc_files` – Dirtree of the pristine etc state -/// 2. `current_etc_files` – Dirtree of the current etc state -/// 3. `new_etc_files` – Dirtree of the new etc state +/// 2. `current_etc_files` – Dirtree of the current etc state +/// 3. `new_etc_files` – Dirtree of the new etc state (if new_etc directory is passed) pub fn traverse_etc( pristine_etc: &CapStdDir, current_etc: &CapStdDir, - new_etc: &CapStdDir, + new_etc: Option<&CapStdDir>, ) -> anyhow::Result<( Directory, Directory, - Directory, + Option>, )> { let mut pristine_etc_files = Directory::default(); recurse_dir(pristine_etc, &mut pristine_etc_files) @@ -324,8 +324,16 @@ pub fn traverse_etc( recurse_dir(current_etc, &mut current_etc_files) .context(format!("Recursing {current_etc:?}"))?; - let mut new_etc_files = Directory::default(); - recurse_dir(new_etc, &mut new_etc_files).context(format!("Recursing {new_etc:?}"))?; + let new_etc_files = match new_etc { + Some(new_etc) => { + let mut new_etc_files = Directory::default(); + recurse_dir(new_etc, &mut new_etc_files).context(format!("Recursing {new_etc:?}"))?; + + Some(new_etc_files) + } + + None => None, + }; return Ok((pristine_etc_files, current_etc_files, new_etc_files)); } @@ -664,7 +672,7 @@ fn merge_modified_files( let current_inode = dir .lookup(filename) - .ok_or(anyhow::anyhow!("{filename:?} not found"))?; + .ok_or_else(|| anyhow::anyhow!("{filename:?} not found"))?; // This will error out if some directory in a chain does not exist let res = new_etc_dirtree.split(OsStr::new(&file)); @@ -734,30 +742,18 @@ pub fn merge( for removed in diff.removed { let stat = new_etc_fd.metadata_optional(&removed)?; - let stat = match stat { - Some(s) => s, - - None => { - // File/dir doesn't exist in new_etc - // Basically a no-op - continue; - } + let Some(stat) = stat else { + // File/dir doesn't exist in new_etc + // Basically a no-op + continue; }; if stat.is_file() || stat.is_symlink() { - match new_etc_fd.remove_file(&removed) { - Ok(..) => { /* no-op */ } - Err(e) => Err(e)?, - } - } - - if stat.is_dir() { + new_etc_fd.remove_file(&removed)?; + } else if stat.is_dir() { // We only add the directory to the removed array, if the entire directory was deleted // So `remove_dir_all` should be okay here - match new_etc_fd.remove_dir_all(&removed) { - Ok(..) => { /* no-op */ } - Err(e) => Err(e)?, - } + new_etc_fd.remove_dir_all(&removed)?; } } @@ -826,7 +822,7 @@ mod tests { c.remove_file(deleted_files[0])?; c.remove_file(deleted_files[1])?; - let (pristine_etc_files, current_etc_files, _) = traverse_etc(&p, &c, &n)?; + let (pristine_etc_files, current_etc_files, _) = traverse_etc(&p, &c, Some(&n))?; let res = compute_diff(&pristine_etc_files, ¤t_etc_files)?; // Test added files @@ -1010,9 +1006,10 @@ mod tests { n.create_dir_all("dir/perms")?; n.write("dir/perms/some-file", "Some-file")?; - let (pristine_etc_files, current_etc_files, new_etc_files) = traverse_etc(&p, &c, &n)?; + let (pristine_etc_files, current_etc_files, new_etc_files) = + traverse_etc(&p, &c, Some(&n))?; let diff = compute_diff(&pristine_etc_files, ¤t_etc_files)?; - merge(&c, ¤t_etc_files, &n, &new_etc_files, diff)?; + merge(&c, ¤t_etc_files, &n, &new_etc_files.unwrap(), diff)?; assert!(files_eq(&c, &n, "new_file.txt")?); assert!(files_eq(&c, &n, "a/new_file.txt")?); @@ -1082,10 +1079,11 @@ mod tests { n.create_dir_all("file-to-dir")?; - let (pristine_etc_files, current_etc_files, new_etc_files) = traverse_etc(&p, &c, &n)?; + let (pristine_etc_files, current_etc_files, new_etc_files) = + traverse_etc(&p, &c, Some(&n))?; let diff = compute_diff(&pristine_etc_files, ¤t_etc_files)?; - let merge_res = merge(&c, ¤t_etc_files, &n, &new_etc_files, diff); + let merge_res = merge(&c, ¤t_etc_files, &n, &new_etc_files.unwrap(), diff); assert!(merge_res.is_err()); assert_eq!( diff --git a/crates/lib/src/bootc_composefs/finalize.rs b/crates/lib/src/bootc_composefs/finalize.rs index 22ee0ea96..d8aba078b 100644 --- a/crates/lib/src/bootc_composefs/finalize.rs +++ b/crates/lib/src/bootc_composefs/finalize.rs @@ -11,12 +11,34 @@ use bootc_initramfs_setup::{mount_composefs_image, open_dir}; use bootc_mount::tempmount::TempMount; use cap_std_ext::cap_std::{ambient_authority, fs::Dir}; use cap_std_ext::dirext::CapStdExtDirExt; -use etc_merge::{compute_diff, merge, traverse_etc}; +use etc_merge::{compute_diff, merge, print_diff, traverse_etc}; use rustix::fs::{fsync, renameat, CWD}; use rustix::path::Arg; use fn_error_context::context; +pub(crate) async fn get_etc_diff() -> Result<()> { + let host = composefs_deployment_status().await?; + let booted_composefs = host.require_composefs_booted()?; + + // Mount the booted EROFS image to get pristine etc + let sysroot = open_dir(CWD, "/sysroot").context("Opening /sysroot")?; + let composefs_fd = mount_composefs_image(&sysroot, &booted_composefs.verity, false)?; + + let erofs_tmp_mnt = TempMount::mount_fd(&composefs_fd)?; + + let pristine_etc = + Dir::open_ambient_dir(erofs_tmp_mnt.dir.path().join("etc"), ambient_authority())?; + let current_etc = Dir::open_ambient_dir("/etc", ambient_authority())?; + + let (pristine_files, current_files, _) = traverse_etc(&pristine_etc, ¤t_etc, None)?; + let diff = compute_diff(&pristine_files, ¤t_files)?; + + print_diff(&diff, &mut std::io::stdout()); + + Ok(()) +} + pub(crate) async fn composefs_native_finalize() -> Result<()> { let host = composefs_deployment_status().await?; @@ -49,7 +71,9 @@ pub(crate) async fn composefs_native_finalize() -> Result<()> { let new_etc = Dir::open_ambient_dir(new_etc_path, ambient_authority())?; let (pristine_files, current_files, new_files) = - traverse_etc(&pristine_etc, ¤t_etc, &new_etc)?; + traverse_etc(&pristine_etc, ¤t_etc, Some(&new_etc))?; + + let new_files = new_files.ok_or(anyhow::anyhow!("Failed to get dirtree for new etc"))?; let diff = compute_diff(&pristine_files, ¤t_files)?; merge(¤t_etc, ¤t_files, &new_etc, &new_files, diff)?; diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index e4b2371e7..b6706d36a 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -31,8 +31,11 @@ use serde::{Deserialize, Serialize}; #[cfg(feature = "composefs-backend")] use crate::bootc_composefs::{ - finalize::composefs_native_finalize, rollback::composefs_rollback, status::composefs_booted, - switch::switch_composefs, update::upgrade_composefs, + finalize::{composefs_native_finalize, get_etc_diff}, + rollback::composefs_rollback, + status::composefs_booted, + switch::switch_composefs, + update::upgrade_composefs, }; use crate::deploy::RequiredHostSpec; use crate::lints; @@ -653,6 +656,9 @@ pub(crate) enum Opt { Internals(InternalsOpts), #[cfg(feature = "composefs-backend")] ComposefsFinalizeStaged, + #[cfg(feature = "composefs-backend")] + /// Diff current /etc configuration versus default + ConfigDiff, } /// Ensure we've entered a mount namespace, so that we can remount @@ -1502,12 +1508,15 @@ async fn run_from_opt(opt: Opt) -> Result<()> { let current_etc = Dir::open_ambient_dir(current_etc, cap_std::ambient_authority())?; let new_etc = Dir::open_ambient_dir(new_etc, cap_std::ambient_authority())?; - let (p, c, n) = etc_merge::traverse_etc(&pristine_etc, ¤t_etc, &new_etc)?; + let (p, c, n) = + etc_merge::traverse_etc(&pristine_etc, ¤t_etc, Some(&new_etc))?; let diff = compute_diff(&p, &c)?; print_diff(&diff, &mut std::io::stdout()); if merge { + let n = + n.ok_or_else(|| anyhow::anyhow!("Failed to get dirtree for new etc"))?; etc_merge::merge(¤t_etc, &c, &new_etc, &n, diff)?; } @@ -1525,6 +1534,9 @@ async fn run_from_opt(opt: Opt) -> Result<()> { #[cfg(feature = "composefs-backend")] Opt::ComposefsFinalizeStaged => composefs_native_finalize().await, + + #[cfg(feature = "composefs-backend")] + Opt::ConfigDiff => get_etc_diff().await, } }