Etc merge fix

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<CustomMetadata>,
     Directory<CustomMetadata>,
-    Directory<CustomMetadata>,
+    Option<Directory<CustomMetadata>>,
 )> {
     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));
@@ -732,14 +740,21 @@ 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)?;
 
-        println!("- Removed file {removed:?}");
+        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() {
+            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
+            new_etc_fd.remove_dir_all(&removed)?;
+        }
     }
 
     Ok(())
@@ -807,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, &current_etc_files)?;
 
         // Test added files
@@ -991,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, &current_etc_files)?;
-        merge(&c, &current_etc_files, &n, &new_etc_files, diff)?;
+        merge(&c, &current_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")?);
@@ -1063,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, &current_etc_files)?;
 
-        let merge_res = merge(&c, &current_etc_files, &n, &new_etc_files, diff);
+        let merge_res = merge(&c, &current_etc_files, &n, &new_etc_files.unwrap(), diff);
 
         assert!(merge_res.is_err());
         assert_eq!(

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, &current_etc, None)?;
+    let diff = compute_diff(&pristine_files, &current_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, &current_etc, &new_etc)?;
+        traverse_etc(&pristine_etc, &current_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, &current_files)?;
     merge(&current_etc, &current_files, &new_etc, &new_files, diff)?;

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, &current_etc, &new_etc)?;
+                let (p, c, n) =
+                    etc_merge::traverse_etc(&pristine_etc, &current_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(&current_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,
     }
 }