diff --git a/Cargo.lock b/Cargo.lock index 1741e7d90..cda51d834 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1422,12 +1422,14 @@ dependencies = [ "anstream", "anyhow", "cap-std-ext 5.1.2", + "clap", "composefs-ctl", "fn-error-context", "hex", "openssl", "owo-colors", "rustix", + "serde", "tracing", ] diff --git a/crates/etc-merge/Cargo.toml b/crates/etc-merge/Cargo.toml index ce698250e..f66090362 100644 --- a/crates/etc-merge/Cargo.toml +++ b/crates/etc-merge/Cargo.toml @@ -16,6 +16,8 @@ composefs-ctl = { workspace = true } fn-error-context = { workspace = true } owo-colors = { workspace = true } anstream = { workspace = true } +clap = { workspace = true } +serde = { workspace = true } [lints] workspace = true diff --git a/crates/etc-merge/src/lib.rs b/crates/etc-merge/src/lib.rs index 0b78a0af5..4ab1cce78 100644 --- a/crates/etc-merge/src/lib.rs +++ b/crates/etc-merge/src/lib.rs @@ -3,8 +3,10 @@ #![allow(dead_code)] use fn_error_context::context; +use serde::{Deserialize, Serialize}; use std::collections::BTreeMap; use std::ffi::OsStr; +use std::fmt::Display; use std::io::BufReader; use std::io::Write; use std::os::fd::{AsFd, AsRawFd}; @@ -15,6 +17,7 @@ use anyhow::Context; use cap_std_ext::cap_std; use cap_std_ext::cap_std::fs::{Dir as CapStdDir, MetadataExt, Permissions, PermissionsExt}; use cap_std_ext::dirext::CapStdExtDirExt; +use clap::ValueEnum; use composefs::fsverity::{FsVerityHashValue, Sha256HashValue, Sha512HashValue}; use composefs::generic_tree::{Directory, FileSystem, Inode, Leaf, LeafContent, LeafId, Stat}; use composefs::tree::ImageError; @@ -23,6 +26,42 @@ use rustix::fs::{ AtFlags, Gid, Uid, XattrFlags, lgetxattr, llistxattr, lsetxattr, readlinkat, symlinkat, }; +#[derive(Debug, Clone, Copy, ValueEnum, PartialEq, Eq, Default, Serialize, Deserialize)] +#[clap(rename_all = "lowercase")] +#[serde(rename_all = "lowercase")] +pub enum MergeStrategy { + /// Fail if there is a merge conflict + #[default] + Fail, + /// Keep the new deployment's default, drop the user's conflicting modification + Skip, + /// Replace the new deployment's entry with the user's version + Replace, +} + +impl Display for MergeStrategy { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + MergeStrategy::Fail => f.write_str("fail"), + MergeStrategy::Skip => f.write_str("skip"), + MergeStrategy::Replace => f.write_str("replace"), + } + } +} + +impl TryFrom<&str> for MergeStrategy { + type Error = anyhow::Error; + + fn try_from(value: &str) -> Result { + match value { + "fail" => Ok(Self::Fail), + "skip" => Ok(Self::Skip), + "replace" => Ok(Self::Replace), + s => anyhow::bail!("Unknown strategy {s}"), + } + } +} + /// Metadata associated with a file, directory, or symlink entry. #[derive(Debug)] pub struct CustomMetadata { @@ -78,6 +117,12 @@ fn stat_eq_ignore_mtime(this: &Stat, other: &Stat) -> bool { return true; } +#[derive(Debug)] +pub struct UnmergablePaths { + path: PathBuf, + reason: String, +} + /// Represents the differences between two directory trees. #[derive(Debug)] pub struct Diff { @@ -88,6 +133,8 @@ pub struct Diff { modified: Vec, /// Paths that exist in the pristine /etc but not in the current one removed: Vec, + /// Paths that are unmergable + pub unmergable_paths: Vec, } fn collect_all_files( @@ -170,6 +217,47 @@ fn get_deletions( Ok(()) } +fn check_if_mergable( + new: &Directory, + current_inode: &Inode, + current_path: &PathBuf, + diff: &mut Diff, +) { + match current_inode { + // If currently 'file' is a directory, make sure it's not a regular file + // new_etc as well, else we can't merge + Inode::Directory(..) => { + let new_dir = new.get_directory(¤t_path.as_os_str()); + + if matches!(new_dir, Err(ImageError::NotADirectory(..))) { + diff.unmergable_paths.push(UnmergablePaths { + path: current_path.clone(), + reason: format!( + "Directory '{}' now defaults to a file in new etc", + current_path.display() + ), + }); + } + } + + // If currently 'file' is not a directory, make sure it's not a directory in the + // new_etc either, else we can't merge + Inode::Leaf(..) => { + let new_dir = new.get_directory(¤t_path.as_os_str()); + + if matches!(new_dir, Ok(..)) { + diff.unmergable_paths.push(UnmergablePaths { + path: current_path.clone(), + reason: format!( + "File '{}' now defaults to a directory in new etc", + current_path.display() + ), + }); + } + } + } +} + // 1. Files in the currently booted deployment’s /etc which were modified from the default /usr/etc (of the same deployment) are retained. // // 2. Files in the currently booted deployment’s /etc which were not modified from the default /usr/etc (of the same deployment) @@ -190,6 +278,7 @@ fn get_modifications( current: &Directory, pristine_leaves: &[Leaf], current_leaves: &[Leaf], + new_leaves: &[Leaf], new: &Directory, mut current_path: PathBuf, diff: &mut Diff, @@ -216,26 +305,45 @@ fn get_modifications( &curr_dir, pristine_leaves, current_leaves, + new_leaves, new, current_path.clone(), diff, )?; - // This directory or its contents were modified/added - // Check if the new directory was deleted from new_etc - // If it was, we want to add the directory back - if new.get_directory_opt(¤t_path.as_os_str())?.is_none() { - if diff.added.len() != total_added { - diff.added.insert(total_added, current_path.clone()); - } else if diff.modified.len() != total_modified { - diff.modified.insert(total_modified, current_path.clone()); + match new.get_directory(¤t_path.as_os_str()) { + Ok(..) => { + // Directory exists in both current and new etc. + // Modifications/additions within this directory are handled recursively. + // No additional action needed here. } + + Err(ImageError::NotFound(..)) | Err(ImageError::NotADirectory(..)) => { + // This directory was deleted in new_etc + // If it was modified in the current etc, we want this back + // + // Was a directory in current etc, but is now + // a file/symlink in the new etc + if diff.added.len() != total_added { + diff.added.insert(total_added, current_path.clone()); + } else if diff.modified.len() != total_modified { + diff.modified.insert(total_added, current_path.clone()); + } + } + + Err(e) => Err(e)?, + } + + if diff.modified.len() != total_modified || diff.added.len() != total_added + { + check_if_mergable(new, inode, ¤t_path, diff); } } Err(ImageError::NotFound(..)) => { // Dir not found in original /etc, dir was added diff.added.push(current_path.clone()); + check_if_mergable(new, inode, ¤t_path, diff); // Also add every file inside that dir collect_all_files(&curr_dir, current_path.clone(), &mut diff.added); @@ -245,9 +353,10 @@ fn get_modifications( // Some directory was changed to a file/symlink // This should be counted in the diff, but we don't really merge this diff.modified.push(current_path.clone()); + check_if_mergable(new, inode, ¤t_path, diff); } - Err(e) => Err(e)?, + Err(e) => Err(e).with_context(|| format!("Opening pristine {path:?}"))?, } } @@ -255,8 +364,10 @@ fn get_modifications( Ok(old_leaf_id) => { let leaf = ¤t_leaves[leaf_id.0]; let old_leaf = &pristine_leaves[old_leaf_id.0]; + if !stat_eq_ignore_mtime(&old_leaf.stat, &leaf.stat) { diff.modified.push(current_path.clone()); + check_if_mergable(new, inode, ¤t_path, diff); current_path.pop(); continue; } @@ -266,6 +377,7 @@ fn get_modifications( if old_meta.content_hash != current_meta.content_hash { // File modified in some way diff.modified.push(current_path.clone()); + check_if_mergable(new, inode, ¤t_path, diff); } } @@ -273,12 +385,14 @@ fn get_modifications( if old_link != current_link { // Symlink modified in some way diff.modified.push(current_path.clone()); + check_if_mergable(new, inode, ¤t_path, diff); } } (Symlink(..), Regular(..)) | (Regular(..), Symlink(..)) => { // File changed to symlink or vice-versa diff.modified.push(current_path.clone()); + check_if_mergable(new, inode, ¤t_path, diff); } (a, b) => { @@ -290,14 +404,16 @@ fn get_modifications( Err(ImageError::IsADirectory(..)) => { // A directory was changed to a file diff.modified.push(current_path.clone()); + check_if_mergable(new, inode, ¤t_path, diff); } Err(ImageError::NotFound(..)) => { // File not found in original /etc, file was added diff.added.push(current_path.clone()); + check_if_mergable(new, inode, ¤t_path, diff); } - Err(e) => Err(e).context(format!("{path:?}"))?, + Err(e) => Err(e).with_context(|| format!("Opening pristine {path:?}"))?, }, } @@ -385,6 +501,7 @@ pub fn compute_diff( added: vec![], modified: vec![], removed: vec![], + unmergable_paths: vec![], }; get_modifications( @@ -392,6 +509,7 @@ pub fn compute_diff( ¤t_etc_files.root, &pristine_etc_files.leaves, ¤t_etc_files.leaves, + &new_etc_files.leaves, &new_etc_files.root, PathBuf::new(), &mut diff, @@ -422,6 +540,15 @@ pub fn print_diff(diff: &Diff, writer: &mut impl Write) { for removed in &diff.removed { let _ = writeln!(writer, "{} {removed:?}", ModificationType::Removed.red()); } + + for unmergable in &diff.unmergable_paths { + let _ = writeln!( + writer, + "{} {}", + ModificationType::Unmergable.magenta(), + unmergable.reason + ); + } } #[context("Collecting xattrs")] @@ -589,6 +716,7 @@ enum ModificationType { Added, Modified, Removed, + Unmergable, } impl std::fmt::Display for ModificationType { @@ -603,6 +731,7 @@ impl ModificationType { ModificationType::Added => "+", ModificationType::Modified => "~", ModificationType::Removed => "-", + ModificationType::Unmergable => "*", } } } @@ -612,24 +741,54 @@ fn create_dir_with_perms( dir_name: &PathBuf, stat: &Stat, new_inode: Option<&Inode>, + merge_strategy: MergeStrategy, ) -> anyhow::Result<()> { - // The new directory is not present in the new_etc, so we create it, else we only copy the - // metadata - if new_inode.is_none() { - // Here we use `create_dir_all` to create every parent as we will set the permissions later - // on. Due to the fact that we have an ordered (sorted) list of directories and directory - // entries and we have a DFS traversal, we will always have directory creation starting from - // the parent anyway. - // - // The exception being, if a directory is modified in the current_etc, and a new directory - // is added inside the modified directory, say `dir/prems` has its permissions modified and - // `dir/prems/new` is the new directory created. Since we handle added files/directories first, - // we will create the directories `perms/new` with directory `new` also getting its - // permissions set, but `perms` will not. `perms` will have its permissions set up when we - // handle the modified directories. - new_etc_fd - .create_dir_all(&dir_name) - .context(format!("Failed to create dir {dir_name:?}"))?; + match new_inode { + Some(inode) => match inode { + Inode::Directory(..) => { /* no-op */ } + + Inode::Leaf(..) => match merge_strategy { + MergeStrategy::Fail => { + anyhow::bail!( + "Modified config directory {dir_name:?} newly defaults to file. Cannot merge" + ) + } + MergeStrategy::Skip => { + tracing::info!( + "Modified config directory {dir_name:?} newly defaults to file, skipping" + ); + return Ok(()); + } + MergeStrategy::Replace => { + new_etc_fd + .remove_all_optional(&dir_name) + .with_context(|| format!("Removing {dir_name:?} for replace"))?; + + new_etc_fd + .create_dir_all(&dir_name) + .context(format!("Failed to create dir {dir_name:?}"))?; + } + }, + }, + + // The new directory is not present in the new_etc, so we create it, else we only copy the + // metadata + None => { + // Here we use `create_dir_all` to create every parent as we will set the permissions later + // on. Due to the fact that we have an ordered (sorted) list of directories and directory + // entries and we have a DFS traversal, we will always have directory creation starting from + // the parent anyway. + // + // The exception being, if a directory is modified in the current_etc, and a new directory + // is added inside the modified directory, say `dir/prems` has its permissions modified and + // `dir/prems/new` is the new directory created. Since we handle added files/directories first, + // we will create the directories `perms/new` with directory `new` also getting its + // permissions set, but `perms` will not. `perms` will have its permissions set up when we + // handle the modified directories. + new_etc_fd + .create_dir_all(&dir_name) + .context(format!("Failed to create dir {dir_name:?}"))?; + } } new_etc_fd @@ -656,6 +815,7 @@ fn merge_leaf( leaf: &Leaf, new_inode: Option<&Inode>, file: &PathBuf, + merge_strategy: MergeStrategy, ) -> anyhow::Result<()> { let symlink = match &leaf.content { LeafContent::Regular(..) => None, @@ -668,7 +828,22 @@ fn merge_leaf( }; if matches!(new_inode, Some(Inode::Directory(..))) { - anyhow::bail!("Modified config file {file:?} newly defaults to directory. Cannot merge") + match merge_strategy { + MergeStrategy::Fail => { + anyhow::bail!( + "Modified config file {file:?} newly defaults to directory. Cannot merge" + ) + } + MergeStrategy::Skip => { + tracing::info!( + "Modified config file {file:?} newly defaults to directory, skipping" + ); + return Ok(()); + } + MergeStrategy::Replace => { + // Fall through remove_all_optional below handles directory removal + } + } }; // If a new file with the same path exists, we delete it @@ -705,9 +880,18 @@ fn merge_modified_files( current_etc_dirtree: &Directory, current_leaves: &[Leaf], new_etc_fd: &CapStdDir, - new_etc_dirtree: &Directory, + new_etc_dirtree: &mut Directory, + merge_strategy: MergeStrategy, ) -> anyhow::Result<()> { + let mut skipped_dirs: Vec = Vec::new(); + for file in files { + if merge_strategy == MergeStrategy::Skip && skipped_dirs.iter().any(|d| file.starts_with(d)) + { + tracing::info!("Skipping {file:?}: parent directory was skipped"); + continue; + } + let (dir, filename) = current_etc_dirtree .split(OsStr::new(&file)) .context("Getting directory and file")?; @@ -717,25 +901,64 @@ fn merge_modified_files( .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)); + let new_etc_res = new_etc_dirtree.split(OsStr::new(&file)); - match res { + let mut update_new_etc_tree_with: Option> = None; + + match new_etc_res { Ok((new_dir, filename)) => { let new_inode = new_dir.lookup(filename); match current_inode { Inode::Directory(..) => { + let is_type_conflict = matches!(new_inode, Some(Inode::Leaf(..))); + + if is_type_conflict && merge_strategy == MergeStrategy::Skip { + tracing::info!( + "Modified config directory {file:?} newly defaults to file, skipping" + ); + skipped_dirs.push(file.clone()); + continue; + } + create_dir_with_perms( new_etc_fd, file, current_inode.stat(current_leaves), new_inode, - )?; + merge_strategy, + ) + .context("Merging directory")?; + + if is_type_conflict && merge_strategy == MergeStrategy::Replace { + // If in new etc this was a leaf, we'd have replaced it with a + // directory + update_new_etc_tree_with = + Some(Inode::Directory(Box::new(Directory::new(Stat::default())))); + } } Inode::Leaf(leaf_id, _) => { + let is_type_conflict = matches!(new_inode, Some(Inode::Directory(..))); + let leaf = ¤t_leaves[leaf_id.0]; - merge_leaf(current_etc_fd, new_etc_fd, leaf, new_inode, file)? + merge_leaf( + current_etc_fd, + new_etc_fd, + leaf, + new_inode, + file, + merge_strategy, + ) + .context("Merging leaf")?; + + // Not strictly necessary but here anyway for consistency + // a leaf has no children in the diff, so no subsequent split() + // will try to touch it + if is_type_conflict && merge_strategy == MergeStrategy::Replace { + update_new_etc_tree_with = + Some(Inode::Leaf(LeafId(leaf_id.0), std::marker::PhantomData)); + } } }; } @@ -747,16 +970,39 @@ fn merge_modified_files( file, current_inode.stat(current_leaves), None, + merge_strategy, )?, Inode::Leaf(leaf_id, _) => { let leaf = ¤t_leaves[leaf_id.0]; - merge_leaf(current_etc_fd, new_etc_fd, leaf, None, file)?; + merge_leaf(current_etc_fd, new_etc_fd, leaf, None, file, merge_strategy)?; } }, - Err(e) => Err(e)?, + Err(e) => Err(e).with_context(|| format!("Opening {file:?} in new etc"))?, }; + + // After Replace, update the in-memory tree to reflect the type change. + // Required for the directory case so children can find the new directory via split(). + if let Some(new_inode) = update_new_etc_tree_with { + let file_name = file + .file_name() + .ok_or_else(|| anyhow::anyhow!("No filename for {file:?}"))?; + + let parent_path = file + .parent() + .ok_or_else(|| anyhow::anyhow!("No parent for {file:?}"))?; + + let parent_dir = if parent_path.as_os_str().is_empty() { + &mut *new_etc_dirtree + } else { + new_etc_dirtree + .get_directory_mut(parent_path.as_os_str()) + .with_context(|| format!("Getting parent dir for {file:?}"))? + }; + + parent_dir.insert(file_name, new_inode); + } } Ok(()) @@ -770,16 +1016,30 @@ pub fn merge( current_etc_fd: &CapStdDir, current_etc_dirtree: &FileSystem, new_etc_fd: &CapStdDir, - new_etc_dirtree: &FileSystem, + new_etc_dirtree: &mut FileSystem, diff: &Diff, + merge_strategy: MergeStrategy, ) -> anyhow::Result<()> { + if merge_strategy == MergeStrategy::Fail && !diff.unmergable_paths.is_empty() { + anyhow::bail!( + "Cannot merge:\n{}", + diff.unmergable_paths + .iter() + .enumerate() + .map(|(i, r)| format!("{}. {}", i + 1, r.reason)) + .collect::>() + .join("\n") + ); + } + merge_modified_files( &diff.added, current_etc_fd, ¤t_etc_dirtree.root, ¤t_etc_dirtree.leaves, new_etc_fd, - &new_etc_dirtree.root, + &mut new_etc_dirtree.root, + merge_strategy, ) .context("Merging added files")?; @@ -789,7 +1049,8 @@ pub fn merge( ¤t_etc_dirtree.root, ¤t_etc_dirtree.leaves, new_etc_fd, - &new_etc_dirtree.root, + &mut new_etc_dirtree.root, + merge_strategy, ) .context("Merging modified files")?; @@ -876,7 +1137,7 @@ mod tests { c.remove_file(deleted_files[0])?; c.remove_file(deleted_files[1])?; - let (pristine_etc_files, current_etc_files, new_etc_files) = + let (pristine_etc_files, current_etc_files, mut new_etc_files) = traverse_etc(&p, &c, Some(&n))?; let res = compute_diff( @@ -889,8 +1150,9 @@ mod tests { &c, ¤t_etc_files, &n, - new_etc_files.as_ref().unwrap(), + new_etc_files.as_mut().unwrap(), &res, + MergeStrategy::Fail, ) .expect("Merge failed"); @@ -1071,14 +1333,21 @@ 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) = + let (pristine_etc_files, current_etc_files, mut new_etc_files) = traverse_etc(&p, &c, Some(&n))?; let diff = compute_diff( &pristine_etc_files, ¤t_etc_files, - &new_etc_files.as_ref().unwrap(), + new_etc_files.as_ref().unwrap(), + )?; + merge( + &c, + ¤t_etc_files, + &n, + new_etc_files.as_mut().unwrap(), + &diff, + MergeStrategy::Fail, )?; - 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")?); @@ -1132,7 +1401,7 @@ mod tests { } #[test] - fn file_to_dir() -> anyhow::Result<()> { + fn file_to_dir_skip() -> anyhow::Result<()> { let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; tempdir.create_dir("pristine_etc")?; @@ -1144,24 +1413,166 @@ mod tests { let n = tempdir.open_dir("new_etc")?; p.write("file-to-dir", "some text")?; - c.write("file-to-dir", "some text 1")?; - + c.write("file-to-dir", "modified text")?; n.create_dir_all("file-to-dir")?; - let (pristine_etc_files, current_etc_files, new_etc_files) = + let (pristine_etc_files, current_etc_files, mut new_etc_files) = traverse_etc(&p, &c, Some(&n))?; let diff = compute_diff( &pristine_etc_files, ¤t_etc_files, - &new_etc_files.as_ref().unwrap(), + new_etc_files.as_ref().unwrap(), )?; - let merge_res = merge(&c, ¤t_etc_files, &n, &new_etc_files.unwrap(), &diff); + assert!(!diff.unmergable_paths.is_empty()); + // Fail strategy should error + let merge_res = merge( + &c, + ¤t_etc_files, + &n, + new_etc_files.as_mut().unwrap(), + &diff, + MergeStrategy::Fail, + ); assert!(merge_res.is_err()); + + // Skip strategy should succeed, keeping the new default (directory) + merge( + &c, + ¤t_etc_files, + &n, + new_etc_files.as_mut().unwrap(), + &diff, + MergeStrategy::Skip, + )?; + assert!(n.metadata("file-to-dir")?.is_dir()); + + Ok(()) + } + + #[test] + fn file_to_dir_replace() -> anyhow::Result<()> { + let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; + + tempdir.create_dir("pristine_etc")?; + tempdir.create_dir("current_etc")?; + tempdir.create_dir("new_etc")?; + + let p = tempdir.open_dir("pristine_etc")?; + let c = tempdir.open_dir("current_etc")?; + let n = tempdir.open_dir("new_etc")?; + + p.write("file-to-dir", "some text")?; + c.write("file-to-dir", "modified text")?; + n.create_dir_all("file-to-dir")?; + + let (pristine_etc_files, current_etc_files, mut new_etc_files) = + traverse_etc(&p, &c, Some(&n))?; + let diff = compute_diff( + &pristine_etc_files, + ¤t_etc_files, + new_etc_files.as_ref().unwrap(), + )?; + + // Replace strategy should succeed, user's file wins + merge( + &c, + ¤t_etc_files, + &n, + new_etc_files.as_mut().unwrap(), + &diff, + MergeStrategy::Replace, + )?; + assert!(n.metadata("file-to-dir")?.is_file()); + assert_eq!(n.read_to_string("file-to-dir")?, "modified text"); + + Ok(()) + } + + #[test] + fn dir_to_file_skip() -> anyhow::Result<()> { + let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; + + tempdir.create_dir("pristine_etc")?; + tempdir.create_dir("current_etc")?; + tempdir.create_dir("new_etc")?; + + let p = tempdir.open_dir("pristine_etc")?; + let c = tempdir.open_dir("current_etc")?; + let n = tempdir.open_dir("new_etc")?; + + // Directory in pristine and current (with modified child), file in new + p.create_dir("dir-to-file")?; + p.write("dir-to-file/child.txt", "pristine content")?; + c.create_dir("dir-to-file")?; + c.write("dir-to-file/child.txt", "modified content")?; + n.write("dir-to-file", "new file content")?; + + let (pristine_etc_files, current_etc_files, mut new_etc_files) = + traverse_etc(&p, &c, Some(&n))?; + let diff = compute_diff( + &pristine_etc_files, + ¤t_etc_files, + new_etc_files.as_ref().unwrap(), + )?; + + assert!(!diff.unmergable_paths.is_empty()); + + // Skip: new default (file) preserved, user's directory dropped + merge( + &c, + ¤t_etc_files, + &n, + new_etc_files.as_mut().unwrap(), + &diff, + MergeStrategy::Skip, + )?; + assert!(n.metadata("dir-to-file")?.is_file()); + assert_eq!(n.read_to_string("dir-to-file")?, "new file content"); + + Ok(()) + } + + #[test] + fn dir_to_file_replace() -> anyhow::Result<()> { + let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; + + tempdir.create_dir("pristine_etc")?; + tempdir.create_dir("current_etc")?; + tempdir.create_dir("new_etc")?; + + let p = tempdir.open_dir("pristine_etc")?; + let c = tempdir.open_dir("current_etc")?; + let n = tempdir.open_dir("new_etc")?; + + p.create_dir("dir-to-file")?; + p.write("dir-to-file/child.txt", "pristine content")?; + c.create_dir("dir-to-file")?; + c.write("dir-to-file/child.txt", "modified content")?; + n.write("dir-to-file", "new file content")?; + + let (pristine_etc_files, current_etc_files, mut new_etc_files) = + traverse_etc(&p, &c, Some(&n))?; + let diff = compute_diff( + &pristine_etc_files, + ¤t_etc_files, + new_etc_files.as_ref().unwrap(), + )?; + + // Replace: user's directory wins + merge( + &c, + ¤t_etc_files, + &n, + new_etc_files.as_mut().unwrap(), + &diff, + MergeStrategy::Replace, + )?; + assert!(n.metadata("dir-to-file")?.is_dir()); assert_eq!( - merge_res.unwrap_err().root_cause().to_string(), - "Modified config file \"file-to-dir\" newly defaults to directory. Cannot merge" + n.read_to_string("dir-to-file/child.txt")?, + "modified content" ); Ok(()) diff --git a/crates/lib/src/bootc_composefs/finalize.rs b/crates/lib/src/bootc_composefs/finalize.rs index 9e0f5e3d4..1527f77d8 100644 --- a/crates/lib/src/bootc_composefs/finalize.rs +++ b/crates/lib/src/bootc_composefs/finalize.rs @@ -3,8 +3,10 @@ use std::path::Path; use crate::bootc_composefs::boot::BootType; use crate::bootc_composefs::gc::{GCOpts, composefs_gc}; use crate::bootc_composefs::rollback::{rename_exchange_bls_entries, rename_exchange_user_cfg}; -use crate::bootc_composefs::status::get_composefs_status; -use crate::composefs_consts::STATE_DIR_ABS; +use crate::bootc_composefs::status::{StagedDeployment, get_composefs_status}; +use crate::composefs_consts::{ + COMPOSEFS_STAGED_DEPLOYMENT_FNAME, COMPOSEFS_TRANSIENT_STATE_DIR, STATE_DIR_ABS, +}; use crate::spec::BootloaderKind; use crate::store::{BootedComposefs, Storage}; use anyhow::{Context, Result}; @@ -14,12 +16,17 @@ use cap_std_ext::cap_std::{ambient_authority, fs::Dir}; use cap_std_ext::dirext::CapStdExtDirExt; use composefs::generic_tree::{FileSystem, Stat}; use composefs_ctl::composefs; -use etc_merge::{compute_diff, merge, print_diff, traverse_etc}; +use etc_merge::{Diff, compute_diff, merge, print_diff, traverse_etc}; use rustix::fs::fsync; use fn_error_context::context; -pub(crate) async fn get_etc_diff(storage: &Storage, booted_cfs: &BootedComposefs) -> Result<()> { +pub(crate) async fn get_etc_diff( + storage: &Storage, + booted_cfs: &BootedComposefs, + against_deployment: Option<&str>, + print: bool, +) -> Result { let host = get_composefs_status(storage, booted_cfs).await?; let booted_composefs = host.require_composefs_booted()?; @@ -37,16 +44,30 @@ pub(crate) async fn get_etc_diff(storage: &Storage, booted_cfs: &BootedComposefs 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 new_etc = match against_deployment { + Some(verity) => { + let new_etc_path = Path::new(STATE_DIR_ABS).join(&verity).join("etc"); + Some(Dir::open_ambient_dir(&new_etc_path, ambient_authority())?) + } + None => None, + }; + + let (pristine_files, current_files, new_etc_files) = + traverse_etc(&pristine_etc, ¤t_etc, new_etc.as_ref())?; + let diff = compute_diff( &pristine_files, ¤t_files, - &FileSystem::new(Stat::uninitialized()), + &new_etc_files + .as_ref() + .unwrap_or(&FileSystem::new(Stat::uninitialized())), )?; - print_diff(&diff, &mut std::io::stdout()); + if print { + print_diff(&diff, &mut std::io::stdout()); + } - Ok(()) + Ok(diff) } pub(crate) async fn composefs_backend_finalize( @@ -99,6 +120,16 @@ pub(crate) async fn composefs_backend_finalize( let erofs_tmp_mnt = TempMount::mount_fd(&composefs_fd)?; + let staged_depl_dir = Dir::open_ambient_dir(COMPOSEFS_TRANSIENT_STATE_DIR, ambient_authority()) + .context("Opening transient state directory")?; + + let current = staged_depl_dir + .read_to_string(COMPOSEFS_STAGED_DEPLOYMENT_FNAME) + .context("Reading staged file")?; + + let staged_depl_file: StagedDeployment = + serde_json::from_str(¤t).context("Deserialzing staged file")?; + // Perform the /etc merge let pristine_etc = Dir::open_ambient_dir(erofs_tmp_mnt.dir.path().join("etc"), ambient_authority())?; @@ -113,11 +144,18 @@ pub(crate) async fn composefs_backend_finalize( let (pristine_files, current_files, new_files) = traverse_etc(&pristine_etc, ¤t_etc, Some(&new_etc))?; - let new_files = + let mut new_files = new_files.ok_or_else(|| anyhow::anyhow!("Failed to get dirtree for new etc"))?; let diff = compute_diff(&pristine_files, ¤t_files, &new_files)?; - merge(¤t_etc, ¤t_files, &new_etc, &new_files, &diff)?; + merge( + ¤t_etc, + ¤t_files, + &new_etc, + &mut new_files, + &diff, + staged_depl_file.merge_strategy, + )?; // Remove /etc/.updated from the new deployment so that ConditionNeedsUpdate=|/etc // services (systemd-sysusers, systemd-tmpfiles) run on the first boot, mirroring diff --git a/crates/lib/src/bootc_composefs/state.rs b/crates/lib/src/bootc_composefs/state.rs index 019662559..b36424ee8 100644 --- a/crates/lib/src/bootc_composefs/state.rs +++ b/crates/lib/src/bootc_composefs/state.rs @@ -1,4 +1,4 @@ -use std::io::Write; +use std::io::{Read, Write}; use std::os::unix::fs::symlink; use std::path::Path; use std::{fs::create_dir_all, process::Command}; @@ -15,6 +15,7 @@ use cap_std_ext::cap_std::fs::{Dir, Permissions, PermissionsExt}; use cap_std_ext::dirext::CapStdExtDirExt; use composefs::fsverity::{FsVerityHashValue, Sha512HashValue}; use composefs_ctl::composefs; +use etc_merge::MergeStrategy; use fn_error_context::context; use ostree_ext::container::deploy::ORIGIN_CONTAINER; @@ -24,11 +25,14 @@ use rustix::{ mount::MountAttrFlags, path::Arg, }; +use serde::{Deserialize, Serialize}; use crate::bootc_composefs::boot::BootType; +use crate::bootc_composefs::finalize::get_etc_diff; use crate::bootc_composefs::status::{ - ComposefsCmdline, StagedDeployment, get_sorted_type1_boot_entries, + ComposefsCmdline, StagedDeploymentState, get_sorted_type1_boot_entries, }; +use crate::composefs_consts::FINALIZE_CONF_FILE; use crate::parsers::bls_config::{BLSConfigType, EFIKey}; use crate::store::{BootedComposefs, Storage}; use crate::{ @@ -43,6 +47,12 @@ use crate::{ utils::path_relative_to, }; +#[derive(Debug, Serialize, Deserialize)] +struct FinalizeConfFile { + /// Merge strategy for etc + merge_strategy: Option, +} + /// Read and parse the `.origin` INI file for a deployment. /// /// Returns `None` if the state directory or origin file doesn't exist @@ -245,7 +255,7 @@ pub(crate) async fn write_composefs_state( root_path: &Utf8PathBuf, deployment_id: &Sha512HashValue, target_imgref: &ImageReference, - staged: Option, + staged: Option>, boot_type: BootType, boot_digest: String, manifest_digest: &str, @@ -307,7 +317,50 @@ pub(crate) async fn write_composefs_state( ) .context("Failed to write to .origin file")?; - if let Some(staged) = staged { + if let Some(mut staged_opts) = staged { + let diff = get_etc_diff( + staged_opts.storage, + staged_opts.booted_cfs, + Some(&deployment_id.to_hex()), + false, + ) + .await?; + + if !diff.unmergable_paths.is_empty() + && staged_opts.staged_depl.merge_strategy == MergeStrategy::Fail + { + let error = anyhow::anyhow!( + "Merge conflicts found in etc and current strategy is either to fail or is not configured. {}", + "Hint: use --merge-strategy or config file /usr/lib/bootc/finalize.toml to specify the strategy" + ); + + // The CLI default is Fail; check if the image ships a different default + let file = staged_opts + .mounted_staged_depl + .open_optional(FINALIZE_CONF_FILE) + .context("Opening finalize.toml")?; + + let Some(mut file) = file else { + return Err(error); + }; + + let mut s = String::new(); + file.read_to_string(&mut s)?; + let conf: FinalizeConfFile = + toml::from_str(&s).with_context(|| format!("Parsing {FINALIZE_CONF_FILE}"))?; + + let Some(strategy) = conf.merge_strategy else { + return Err(error); + }; + + if strategy == MergeStrategy::Fail { + return Err(error); + } + + // Store this in finalized file + staged_opts.staged_depl.merge_strategy = strategy; + } + std::fs::create_dir_all(COMPOSEFS_TRANSIENT_STATE_DIR) .with_context(|| format!("Creating {COMPOSEFS_TRANSIENT_STATE_DIR}"))?; @@ -318,7 +371,8 @@ pub(crate) async fn write_composefs_state( staged_depl_dir .atomic_write( COMPOSEFS_STAGED_DEPLOYMENT_FNAME, - staged + staged_opts + .staged_depl .to_canon_json_vec() .context("Failed to serialize staged deployment JSON")?, ) diff --git a/crates/lib/src/bootc_composefs/status.rs b/crates/lib/src/bootc_composefs/status.rs index c8e7851c3..b83cc91e3 100644 --- a/crates/lib/src/bootc_composefs/status.rs +++ b/crates/lib/src/bootc_composefs/status.rs @@ -6,6 +6,7 @@ use bootc_mount::inspect_filesystem; use composefs_ctl::composefs::fsverity::Sha512HashValue; use composefs_ctl::composefs_oci; use composefs_oci::OciImage; +use etc_merge::MergeStrategy; use fn_error_context::context; use serde::{Deserialize, Serialize}; @@ -26,7 +27,7 @@ use crate::{ grub_menuconfig::{MenuEntry, parse_grub_menuentry_file}, }, spec::{BootEntry, BootOrder, BootloaderKind, Host, HostSpec, ImageStatus}, - store::Storage, + store::{BootedComposefs, Storage}, utils::{EfiError, read_uefi_var}, }; @@ -127,6 +128,20 @@ pub(crate) struct StagedDeployment { /// Whether to finalize this staged deployment on reboot or not /// This also maps to `download_only` field in `BootEntry` pub(crate) finalization_locked: bool, + /// The merge strategy configured for three way etc merge + pub(crate) merge_strategy: MergeStrategy, +} + +/// The state needed to write `/run/composefs/staged-deployment` +pub(crate) struct StagedDeploymentState<'a> { + /// The JSON that's serialized into `/run/composefs/staged-deployment` + pub(crate) staged_depl: StagedDeployment, + /// Storage object + pub(crate) storage: &'a Storage, + /// The current booted composefs info + pub(crate) booted_cfs: &'a BootedComposefs, + /// Mounted staged EROFS fd + pub(crate) mounted_staged_depl: &'a Dir, } #[derive(Debug, PartialEq)] diff --git a/crates/lib/src/bootc_composefs/switch.rs b/crates/lib/src/bootc_composefs/switch.rs index 3f8b5af8d..bf9af5d1c 100644 --- a/crates/lib/src/bootc_composefs/switch.rs +++ b/crates/lib/src/bootc_composefs/switch.rs @@ -77,6 +77,7 @@ pub(crate) async fn switch_composefs( apply: opts.apply, download_only: false, use_unified, + merge_strategy: opts.merge_strategy.merge_strategy, }; if let Some(cfg_verity) = image { diff --git a/crates/lib/src/bootc_composefs/update.rs b/crates/lib/src/bootc_composefs/update.rs index e0e677958..3c2c9fe6e 100644 --- a/crates/lib/src/bootc_composefs/update.rs +++ b/crates/lib/src/bootc_composefs/update.rs @@ -7,11 +7,13 @@ use composefs_ctl::composefs; use composefs_ctl::composefs_boot; use composefs_ctl::composefs_oci; use composefs_oci::image::create_filesystem; +use etc_merge::MergeStrategy; use fn_error_context::context; use ocidir::cap_std::ambient_authority; use ostree_ext::container::ManifestDiff; use crate::bootc_composefs::gc::GCOpts; +use crate::bootc_composefs::status::StagedDeploymentState; use crate::spec::BootloaderKind; use crate::{ bootc_composefs::{ @@ -210,6 +212,8 @@ pub(crate) struct DoUpgradeOpts { pub(crate) download_only: bool, /// Whether to use unified storage (containers-storage + composefs). pub(crate) use_unified: bool, + /// Merge strategy to use when we have a conflict when merging etc + pub(crate) merge_strategy: MergeStrategy, } async fn apply_upgrade( @@ -307,14 +311,22 @@ pub(crate) async fn do_upgrade( )?, }; + let staged_state = StagedDeploymentState { + storage, + booted_cfs, + mounted_staged_depl: &mounted_fs, + staged_depl: StagedDeployment { + depl_id: id.to_hex(), + finalization_locked: opts.download_only, + merge_strategy: opts.merge_strategy, + }, + }; + write_composefs_state( &Utf8PathBuf::from("/sysroot"), &id, imgref, - Some(StagedDeployment { - depl_id: id.to_hex(), - finalization_locked: opts.download_only, - }), + Some(staged_state), boot_type, boot_digest, &manifest_digest, @@ -375,6 +387,7 @@ pub(crate) async fn upgrade_composefs( apply: opts.apply, download_only: opts.download_only, use_unified: false, + merge_strategy: opts.merge_strategy.merge_strategy, }; if opts.from_downloaded { @@ -393,16 +406,20 @@ pub(crate) async fn upgrade_composefs( start_finalize_stated_svc()?; - // Make the staged deployment not download_only - let new_staged = StagedDeployment { - depl_id: staged.require_composefs()?.verity.clone(), - finalization_locked: false, - }; - let staged_depl_dir = Dir::open_ambient_dir(COMPOSEFS_TRANSIENT_STATE_DIR, ambient_authority()) .context("Opening transient state directory")?; + let current = staged_depl_dir + .read_to_string(COMPOSEFS_STAGED_DEPLOYMENT_FNAME) + .context("Reading staged file")?; + + let mut new_staged: StagedDeployment = + serde_json::from_str(¤t).context("Deserialzing staged file")?; + + // Make the staged deployment not download_only + new_staged.finalization_locked = false; + staged_depl_dir .atomic_replace_with( COMPOSEFS_STAGED_DEPLOYMENT_FNAME, diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index 26a4ba8f3..e43357f72 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -24,7 +24,7 @@ use composefs_ctl::composefs_boot; use composefs_ctl::composefs_oci; use composefs_boot::BootOps as _; -use etc_merge::{compute_diff, print_diff}; +use etc_merge::{MergeStrategy, compute_diff, print_diff}; use fn_error_context::context; use indoc::indoc; use ostree::gio; @@ -83,6 +83,13 @@ impl TryFrom for ProgressWriter { } } +#[derive(Debug, Parser, PartialEq, Eq)] +pub(crate) struct EtcMergeStrategyOpts { + /// The merge strategy to use when conflicts are found while performing three way etc merge + #[clap(long, default_value_t = MergeStrategy::Fail)] + pub(crate) merge_strategy: MergeStrategy, +} + /// Perform an upgrade operation #[derive(Debug, Parser, PartialEq, Eq)] pub(crate) struct UpgradeOpts { @@ -133,6 +140,9 @@ pub(crate) struct UpgradeOpts { #[clap(flatten)] pub(crate) progress: ProgressOptions, + + #[clap(flatten)] + pub(crate) merge_strategy: EtcMergeStrategyOpts, } /// Perform an switch operation @@ -194,6 +204,9 @@ pub(crate) struct SwitchOpts { #[clap(flatten)] pub(crate) progress: ProgressOptions, + + #[clap(flatten)] + pub(crate) merge_strategy: EtcMergeStrategyOpts, } /// Options controlling rollback @@ -717,6 +730,8 @@ pub(crate) enum InternalsOpts { /// Whether to perform the three way merge or not #[clap(long)] merge: bool, + #[clap(long, requires = "merge", default_value_t = MergeStrategy::Fail)] + strategy: MergeStrategy, }, #[cfg(feature = "docgen")] /// Dump CLI structure as JSON for documentation generation @@ -2202,6 +2217,7 @@ async fn run_from_opt(opt: Opt) -> Result<()> { current_etc, new_etc, merge, + strategy: merge_strategy, } => { let pristine_etc = Dir::open_ambient_dir(pristine_etc, cap_std::ambient_authority())?; @@ -2211,15 +2227,13 @@ async fn run_from_opt(opt: Opt) -> Result<()> { let (p, c, n) = etc_merge::traverse_etc(&pristine_etc, ¤t_etc, Some(&new_etc))?; - let n = n - .as_ref() - .ok_or_else(|| anyhow::anyhow!("Failed to get new directory tree"))?; + let mut n = n.ok_or_else(|| anyhow::anyhow!("Failed to get new directory tree"))?; let diff = compute_diff(&p, &c, &n)?; print_diff(&diff, &mut std::io::stdout()); if merge { - etc_merge::merge(¤t_etc, &c, &new_etc, &n, &diff)?; + etc_merge::merge(¤t_etc, &c, &new_etc, &mut n, &diff, merge_strategy)?; } Ok(()) @@ -2352,7 +2366,8 @@ async fn run_from_opt(opt: Opt) -> Result<()> { anyhow::bail!("ConfigDiff is only supported for composefs backend") } BootedStorageKind::Composefs(booted_cfs) => { - get_etc_diff(storage, &booted_cfs).await + get_etc_diff(storage, &booted_cfs, None, true).await?; + Ok(()) } } } diff --git a/crates/lib/src/composefs_consts.rs b/crates/lib/src/composefs_consts.rs index 8617f1005..c72eeb3f2 100644 --- a/crates/lib/src/composefs_consts.rs +++ b/crates/lib/src/composefs_consts.rs @@ -54,3 +54,10 @@ pub(crate) const UKI_NAME_PREFIX: &str = TYPE1_BOOT_DIR_PREFIX; /// that keep the manifest, config, and layer splitstreams alive. This is /// analogous to how ostree uses `ostree/` refs. pub(crate) const BOOTC_TAG_PREFIX: &str = "localhost/bootc-"; + +/// This is the file that stores configuration for deployment finalization +/// ```toml +/// # /etc merge strategy +/// merge_strategy = "replace" +/// ``` +pub(crate) const FINALIZE_CONF_FILE: &str = "usr/lib/bootc/finalize.toml"; diff --git a/crates/lib/src/store/mod.rs b/crates/lib/src/store/mod.rs index 78926d173..d7274d868 100644 --- a/crates/lib/src/store/mod.rs +++ b/crates/lib/src/store/mod.rs @@ -249,7 +249,6 @@ impl<'a> BootedOstree<'a> { } /// Represents a composefs-based boot environment -#[allow(dead_code)] pub struct BootedComposefs { pub repo: Arc, pub cmdline: &'static ComposefsCmdline, diff --git a/docs/src/man/bootc-switch.8.md b/docs/src/man/bootc-switch.8.md index 116f98553..a8990d7bc 100644 --- a/docs/src/man/bootc-switch.8.md +++ b/docs/src/man/bootc-switch.8.md @@ -73,6 +73,17 @@ Soft reboot allows faster system restart by avoiding full hardware reboot when p Retain reference to currently booted image +**--merge-strategy**=*MERGE_STRATEGY* + + The merge strategy to use when conflicts are found while performing three way etc merge + + Possible values: + - fail + - skip + - replace + + Default: fail + # EXAMPLES diff --git a/docs/src/man/bootc-upgrade.8.md b/docs/src/man/bootc-upgrade.8.md index b1b3f3b69..49e7d5df3 100644 --- a/docs/src/man/bootc-upgrade.8.md +++ b/docs/src/man/bootc-upgrade.8.md @@ -73,6 +73,17 @@ Soft reboot allows faster system restart by avoiding full hardware reboot when p Upgrade to a different tag of the currently booted image +**--merge-strategy**=*MERGE_STRATEGY* + + The merge strategy to use when conflicts are found while performing three way etc merge + + Possible values: + - fail + - skip + - replace + + Default: fail + # EXAMPLES diff --git a/tmt/plans/integration.fmf b/tmt/plans/integration.fmf index 43e3f2cbc..1ec117f48 100644 --- a/tmt/plans/integration.fmf +++ b/tmt/plans/integration.fmf @@ -285,4 +285,11 @@ execute: how: fmf test: - /tmt/tests/tests/test-45-composefs-corruped-state-resilience + +/plan-46-etc-merge-strategy: + summary: Test etc merge strategy (skip/replace/fail) + discover: + how: fmf + test: + - /tmt/tests/tests/test-46-etc-merge-strategy # END GENERATED PLANS diff --git a/tmt/tests/booted/test-etc-merge-strategy.nu b/tmt/tests/booted/test-etc-merge-strategy.nu new file mode 100644 index 000000000..fb5157de5 --- /dev/null +++ b/tmt/tests/booted/test-etc-merge-strategy.nu @@ -0,0 +1,106 @@ +# number: 46 +# tmt: +# summary: Test etc merge strategy (skip/replace/fail) +# duration: 30m + +use std assert +use tap.nu +use bootc_testlib.nu + +if not (tap is_composefs) { + tap ok + exit 0 +} + +# Image that changes a file to a directory and a directory to a file +const DOCKERFILE_CONFLICT = ' +FROM localhost/bootc as base + +# file-to-dir: /etc/test-file-to-dir was a file in base, now becomes a directory +RUN rm -f /etc/test-file-to-dir && mkdir -p /etc/test-file-to-dir && echo "new-default" > /etc/test-file-to-dir/config + +# dir-to-file: /etc/test-dir-to-file/ was a directory in base, now becomes a file +RUN rm -rf /etc/test-dir-to-file && echo "new-default-file" > /etc/test-dir-to-file +' + +def first_boot [] { + tap begin "etc merge strategy test" + + bootc image copy-to-storage + + # Create the paths in the running pristine image so they exist in the base + # file-to-dir: starts as a file + echo "pristine-content" | save --force /etc/test-file-to-dir + + # dir-to-file: starts as a directory with a child + mkdir /etc/test-dir-to-file + echo "pristine-child" | save --force /etc/test-dir-to-file/child.conf + + # Now modify them (simulating user customization on the live system) + echo "user-modified-content" | save --force /etc/test-file-to-dir + echo "user-modified-child" | save --force /etc/test-dir-to-file/child.conf + + # Build the conflicting image + (tap make_uki_containerfile $DOCKERFILE_CONFLICT) | save --force Dockerfile + podman build -t localhost/bootc-etc-conflict . + + # Switch with skip strategy: new defaults should win + bootc switch --transport containers-storage --merge-strategy skip localhost/bootc-etc-conflict + tmt-reboot +} + +def second_boot [] { + # Verify skip: new defaults won + # file-to-dir: should now be a directory (new default won) + assert (/etc/test-file-to-dir | path type) == "dir" "skip: file-to-dir should be a directory" + assert (open /etc/test-file-to-dir/config | str trim) == "new-default" "skip: file-to-dir/config should have new default content" + + # dir-to-file: should now be a file (new default won) + assert (/etc/test-dir-to-file | path type) == "file" "skip: dir-to-file should be a file" + assert (open /etc/test-dir-to-file | str trim) == "new-default-file" "skip: dir-to-file should have new default content" + + # Re-create the original paths as files/dirs so they exist in this image's pristine etc + echo "pristine-content-2" | save --force /etc/test-file-to-dir-2 + + mkdir /etc/test-dir-to-file-2 + echo "pristine-child-2" | save --force /etc/test-dir-to-file-2/child.conf + + # User modifications + echo "user-keeps-this" | save --force /etc/test-file-to-dir-2 + echo "user-keeps-this-child" | save --force /etc/test-dir-to-file-2/child.conf + + # Build image with type conflicts on the -2 paths + let dockerfile_replace = ' +FROM localhost/bootc as base +RUN rm -f /etc/test-file-to-dir-2 && mkdir -p /etc/test-file-to-dir-2 && echo "should-be-replaced" > /etc/test-file-to-dir-2/config +RUN rm -rf /etc/test-dir-to-file-2 && echo "should-be-replaced" > /etc/test-dir-to-file-2 +' + (tap make_uki_containerfile $dockerfile_replace) | save --force Dockerfile + podman build -t localhost/bootc-etc-replace . + + # Switch with replace strategy: user modifications should win + bootc switch --transport containers-storage --merge-strategy replace localhost/bootc-etc-replace + tmt-reboot +} + +def third_boot [] { + # Verify replace: user modifications won + # file-to-dir: user had a file, should still be a file with user content + assert (/etc/test-file-to-dir-2 | path type) == "file" "replace: file-to-dir-2 should be a file (user wins)" + assert (open /etc/test-file-to-dir-2 | str trim) == "user-keeps-this" "replace: file-to-dir-2 should have user content" + + # dir-to-file: user had a directory, should still be a directory with user content + assert (/etc/test-dir-to-file-2 | path type) == "dir" "replace: dir-to-file-2 should be a directory (user wins)" + assert (open /etc/test-dir-to-file-2/child.conf | str trim) == "user-keeps-this-child" "replace: child.conf should have user content" + + tap ok +} + +def main [] { + match $env.TMT_REBOOT_COUNT? { + null | "0" => first_boot, + "1" => second_boot, + "2" => third_boot, + $o => { error make { msg: $"Invalid TMT_REBOOT_COUNT ($o)" } }, + } +} diff --git a/tmt/tests/tests.fmf b/tmt/tests/tests.fmf index a8b5b91f8..af03dfb40 100644 --- a/tmt/tests/tests.fmf +++ b/tmt/tests/tests.fmf @@ -178,3 +178,8 @@ check: summary: Test composefs backend resilience to state corruption duration: 30m test: nu booted/test-composefs-corruped-state-resilience.nu + +/test-46-etc-merge-strategy: + summary: Test etc merge strategy (skip/replace/fail) + duration: 30m + test: nu booted/test-etc-merge-strategy.nu