containers/bootc
1
0
Fork 0
mirror of https://github.com/containers/bootc.git synced 2026-02-05 06:45:13 +01:00
Code Issues Activity

etc-merge: Refactor and fix dir perms while merging

Browse Source 
While merging, existing directory in new_etc was being recursively
deleted which is not correct as any new files might also be deleted.

Instead, we simply create a directory if it doesn't exists, or if it
does exists, we update its metadata accordingly.

Add some test cases for the above.

Signed-off-by: Johan-Liebert1 <pragyanpoudyal41999@gmail.com>

cli: Add internal opt for printing etc-diff

Signed-off-by: Johan-Liebert1 <pragyanpoudyal41999@gmail.com>

etc-merge: Add license to Cargo.toml

Signed-off-by: Johan-Liebert1 <pragyanpoudyal41999@gmail.com>

etc-merge: More refactoring

Signed-off-by: Johan-Liebert1 <pragyanpoudyal41999@gmail.com>
This commit is contained in:
Johan-Liebert1
2025-08-25 12:11:32 +05:30
parent dff69a996b
commit ccc558c776
5 changed files with 230 additions and 149 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
Cargo.lock generated
View File

@@ -268,6 +268,7 @@ dependencies = [
"composefs",
"composefs-boot",
"composefs-oci",
"etc-merge",
"fn-error-context",
"hex",
"indicatif 0.18.0",
@@ -971,7 +972,7 @@ dependencies = [
"hex",
"openssl",
"owo-colors",
"rustix 1.0.3",
"rustix 1.0.8",
"tracing",
]

2
crates/etc-merge/Cargo.toml
View File

@@ -2,6 +2,8 @@
name = "etc-merge"
version = "0.1.0"
edition = "2024"
license = "MIT OR Apache-2.0"
publish = false
[dependencies]
anyhow = { workspace = true }

338
crates/etc-merge/src/lib.rs
View File

@@ -5,7 +5,7 @@
use fn_error_context::context;
use std::cell::RefCell;
use std::collections::BTreeMap;
use std::ffi::{OsStr, OsString};
use std::ffi::OsStr;
use std::io::BufReader;
use std::io::Write;
use std::os::fd::{AsFd, AsRawFd};
@@ -20,7 +20,7 @@ use cap_std_ext::dirext::CapStdExtDirExt;
use composefs::fsverity::{FsVerityHashValue, Sha256HashValue, Sha512HashValue};
use composefs::generic_tree::{Directory, Inode, Leaf, LeafContent, Stat};
use composefs::tree::ImageError;
use rustix::fs::{lgetxattr, llistxattr, lsetxattr, readlinkat, AtFlags, Gid, Uid, XattrFlags};
use rustix::fs::{AtFlags, Gid, Uid, XattrFlags, lgetxattr, llistxattr, lsetxattr, readlinkat};
/// Metadata associated with a file, directory, or symlink entry.
#[derive(Debug)]
@@ -88,7 +88,11 @@ pub struct Diff {
removed: Vec<PathBuf>,
}
fn collect_all_files(root: &Directory<CustomMetadata>, current_path: PathBuf) -> Vec<PathBuf> {
fn collect_all_files(
root: &Directory<CustomMetadata>,
current_path: PathBuf,
files: &mut Vec<PathBuf>,
) {
fn collect(
root: &Directory<CustomMetadata>,
mut current_path: PathBuf,
@@ -107,12 +111,10 @@ fn collect_all_files(root: &Directory<CustomMetadata>, current_path: PathBuf) ->
}
}
let mut files = vec![];
collect(root, current_path, &mut files);
return files;
collect(root, current_path, files);
}
#[context("Getting deletions")]
fn get_deletions(
pristine: &Directory<CustomMetadata>,
current: &Directory<CustomMetadata>,
@@ -134,6 +136,10 @@ fn get_deletions(
diff.removed.push(current_path.clone());
}
Err(ImageError::NotADirectory(..)) => {
// Already tracked in modifications
}
Err(e) => Err(e)?,
}
}
@@ -148,7 +154,11 @@ fn get_deletions(
diff.removed.push(current_path.clone());
}
Err(e) => Err(e)?,
Err(ImageError::IsADirectory(..)) => {
// Already tracked in modifications
}
Err(e) => Err(e).context(format!("{file_name:?}"))?,
},
}
@@ -172,6 +182,7 @@ fn get_deletions(
// b. Permissions/ownership changed
// c. Was a file but changed to directory/symlink etc or vice versa
// d. xattrs changed - we don't include this right now
#[context("Getting modifications")]
fn get_modifications(
pristine: &Directory<CustomMetadata>,
current: &Directory<CustomMetadata>,
@@ -200,8 +211,13 @@ fn get_modifications(
diff.added.push(current_path.clone());
// Also add every file inside that dir
diff.added
.extend(collect_all_files(&curr_dir, current_path.clone()));
collect_all_files(&curr_dir, current_path.clone(), &mut diff.added);
}
Err(ImageError::NotADirectory(..)) => {
// 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());
}
Err(e) => Err(e)?,
@@ -210,20 +226,22 @@ fn get_modifications(
Inode::Leaf(leaf) => match pristine.ref_leaf(path) {
Ok(old_leaf) => {
if !stat_eq_ignore_mtime(&old_leaf.stat, &leaf.stat) {
diff.modified.push(current_path.clone());
current_path.pop();
continue;
}
match (&old_leaf.content, &leaf.content) {
(Regular(old_meta), Regular(current_meta)) => {
if old_meta.content_hash != current_meta.content_hash
|| !stat_eq_ignore_mtime(&old_leaf.stat, &leaf.stat)
{
if old_meta.content_hash != current_meta.content_hash {
// File modified in some way
diff.modified.push(current_path.clone());
}
}
(Symlink(old_link), Symlink(current_link)) => {
if old_link != current_link
|| !stat_eq_ignore_mtime(&old_leaf.stat, &leaf.stat)
{
if old_link != current_link {
// Symlink modified in some way
diff.modified.push(current_path.clone());
}
@@ -250,7 +268,7 @@ fn get_modifications(
diff.added.push(current_path.clone());
}
Err(e) => Err(e)?,
Err(e) => Err(e).context(format!("{path:?}"))?,
},
}
@@ -311,6 +329,7 @@ pub fn traverse_etc(
}
/// Computes the differences between two directory snapshots.
#[context("Computing diff")]
pub fn compute_diff(
pristine_etc_files: &Directory<CustomMetadata>,
current_etc_files: &Directory<CustomMetadata>,
@@ -339,56 +358,58 @@ pub fn compute_diff(
}
/// Prints a colorized summary of differences to standard output.
pub fn print_diff(diff: &Diff) {
pub fn print_diff(diff: &Diff, writer: &mut impl Write) {
use owo_colors::OwoColorize;
let mut stdout = anstream::stdout();
for added in &diff.added {
let _ = writeln!(stdout, "{} {added:?}", ModificationType::Added.green());
let _ = writeln!(writer, "{} {added:?}", ModificationType::Added.green());
}
for modified in &diff.modified {
let _ = writeln!(stdout, "{} {modified:?}", ModificationType::Modified.cyan());
let _ = writeln!(writer, "{} {modified:?}", ModificationType::Modified.cyan());
}
for removed in &diff.removed {
let _ = writeln!(stdout, "{} {removed:?}", ModificationType::Removed.red());
let _ = writeln!(writer, "{} {removed:?}", ModificationType::Removed.red());
}
}
#[context("Collecting xattrs")]
fn collect_xattrs(etc_fd: &CapStdDir, rel_path: &OsString) -> anyhow::Result<Xattrs> {
fn collect_xattrs(etc_fd: &CapStdDir, rel_path: impl AsRef<Path>) -> anyhow::Result<Xattrs> {
let link = format!("/proc/self/fd/{}", etc_fd.as_fd().as_raw_fd());
let path = Path::new(&link).join(rel_path);
const DEFAULT_SIZE: usize = 128;
// Start with a guess for size
let mut buf: Vec<u8> = vec![0; DEFAULT_SIZE];
let size = llistxattr(&path, &mut buf).context("llistxattr")?;
let mut xattrs_name_buf: Vec<u8> = vec![0; DEFAULT_SIZE];
let mut size = llistxattr(&path, &mut xattrs_name_buf).context("llistxattr")?;
if size > DEFAULT_SIZE {
buf = vec![0; size];
llistxattr(&path, &mut buf).context("llistxattr")?;
if size > xattrs_name_buf.capacity() {
xattrs_name_buf.resize(size, 0);
size = llistxattr(&path, &mut xattrs_name_buf).context("llistxattr")?;
}
let xattrs: Xattrs = RefCell::new(BTreeMap::new());
for name_buf in buf[..size].split(|&b| b == 0).filter(|x| !x.is_empty()) {
for name_buf in xattrs_name_buf[..size]
.split(|&b| b == 0)
.filter(|x| !x.is_empty())
{
let name = OsStr::from_bytes(name_buf);
let mut buf = vec![0; DEFAULT_SIZE];
let size = lgetxattr(&path, name_buf, &mut buf).context("lgetxattr")?;
let mut xattrs_value_buf = vec![0; DEFAULT_SIZE];
let mut size = lgetxattr(&path, name_buf, &mut xattrs_value_buf).context("lgetxattr")?;
if size > DEFAULT_SIZE {
buf = vec![0; size];
lgetxattr(&path, name_buf, &mut buf).context("lgetxattr")?;
if size > xattrs_value_buf.capacity() {
xattrs_value_buf.resize(size, 0);
size = lgetxattr(&path, name_buf, &mut xattrs_value_buf).context("lgetxattr")?;
}
xattrs
.borrow_mut()
.insert(Box::<OsStr>::from(name), Box::<[u8]>::from(&buf[..size]));
xattrs.borrow_mut().insert(
Box::<OsStr>::from(name),
Box::<[u8]>::from(&xattrs_value_buf[..size]),
);
}
Ok(xattrs)
@@ -540,22 +561,27 @@ fn create_dir_with_perms(
new_etc_fd: &CapStdDir,
dir_name: &PathBuf,
stat: &Stat,
remove_existing: bool,
new_inode: Option<&Inode<CustomMetadata>>,
) -> anyhow::Result<()> {
if remove_existing {
let res = new_etc_fd.remove_all_optional(&dir_name);
match res {
Ok(_) => {}
Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}
Err(e) => Err(e).context(format!("Removing {dir_name:?}"))?,
};
// 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 aways 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 premissions 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 premissions 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
.create_dir_all(&dir_name)
.context(format!("Failed to create dir {dir_name:?}"))?;
new_etc_fd
.set_permissions(&dir_name, Permissions::from_mode(stat.st_mode))
.context(format!("Changing permissions for dir {dir_name:?}"))?;
@@ -574,83 +600,62 @@ fn create_dir_with_perms(
Ok(())
}
fn handle_leaf(
fn merge_leaf(
current_etc_fd: &CapStdDir,
new_etc_fd: &CapStdDir,
leaf: &Rc<Leaf<CustomMetadata>>,
new_inode: Option<&Inode<CustomMetadata>>,
file: &PathBuf,
) -> anyhow::Result<&'static str> {
let ty = match &leaf.content {
LeafContent::Regular(..) => {
if matches!(new_inode, Some(Inode::Directory(..))) {
anyhow::bail!("File {file:?} converted to dir");
};
) -> anyhow::Result<()> {
let symlink = match &leaf.content {
LeafContent::Regular(..) => None,
LeafContent::Symlink(target) => Some(target),
// If a new file with the same path exists, we delete it
new_etc_fd
.remove_all_optional(&file)
.context(format!("Deleting {file:?}"))?;
current_etc_fd
.copy(&file, new_etc_fd, &file)
.context(format!("Copying file {file:?}"))?;
rustix::fs::chownat(
&new_etc_fd,
file,
Some(Uid::from_raw(leaf.stat.st_uid)),
Some(Gid::from_raw(leaf.stat.st_gid)),
AtFlags::SYMLINK_NOFOLLOW,
)
.context(format!("chown {file:?}"))?;
copy_xattrs(&leaf.stat.xattrs, new_etc_fd, file)?;
"file"
_ => {
tracing::debug!("Found non file/symlink while merging. Ignoring");
return Ok(());
}
LeafContent::Symlink(os_str) => {
if matches!(new_inode, Some(Inode::Directory(..))) {
anyhow::bail!("Symlink {file:?} coverted to dir");
};
// A new file with the same path exists, we delete this
new_etc_fd
.remove_all_optional(&file)
.context(format!("Deleting {file:?}"))?;
new_etc_fd
.symlink(PathBuf::from(os_str), &file)
.context(format!("Creating symlink {file:?}"))?;
rustix::fs::chownat(
&new_etc_fd,
file,
Some(Uid::from_raw(leaf.stat.st_uid)),
Some(Gid::from_raw(leaf.stat.st_gid)),
AtFlags::SYMLINK_NOFOLLOW,
)
.context(format!("chown {file:?}"))?;
copy_xattrs(&leaf.stat.xattrs, new_etc_fd, file)?;
"symlink"
}
_ => unreachable!(),
};
Ok(ty)
if matches!(new_inode, Some(Inode::Directory(..))) {
anyhow::bail!("Modified config file {file:?} newly defaults to directory. Cannot merge")
};
// If a new file with the same path exists, we delete it
new_etc_fd
.remove_all_optional(&file)
.context(format!("Deleting {file:?}"))?;
if let Some(target) = symlink {
new_etc_fd
.symlink(target.as_ref(), &file)
.context(format!("Creating symlink {file:?}"))?;
} else {
current_etc_fd
.copy(&file, new_etc_fd, &file)
.context(format!("Copying file {file:?}"))?;
};
rustix::fs::chownat(
&new_etc_fd,
file,
Some(Uid::from_raw(leaf.stat.st_uid)),
Some(Gid::from_raw(leaf.stat.st_gid)),
AtFlags::SYMLINK_NOFOLLOW,
)
.context(format!("chown {file:?}"))?;
copy_xattrs(&leaf.stat.xattrs, new_etc_fd, file)?;
Ok(())
}
fn handle_modified_files(
fn merge_modified_files(
files: &Vec<PathBuf>,
current_etc_fd: &CapStdDir,
current_etc_dirtree: &Directory<CustomMetadata>,
new_etc_fd: &CapStdDir,
new_etc_dirtree: &Directory<CustomMetadata>,
m_type: ModificationType,
) -> anyhow::Result<()> {
for file in files {
let (dir, filename) = current_etc_dirtree
@@ -665,32 +670,28 @@ fn handle_modified_files(
let res = new_etc_dirtree.split(OsStr::new(&file));
match res {
// Directory exists in the new /etc, but was modified in some way
Ok((dir, filename)) => {
let new_inode = dir.lookup(filename);
let ty = match current_inode {
Inode::Directory(..) => {
create_dir_with_perms(new_etc_fd, file, current_inode.stat(), true)?;
Ok((new_dir, filename)) => {
let new_inode = new_dir.lookup(filename);
"dir"
match current_inode {
Inode::Directory(..) => {
create_dir_with_perms(new_etc_fd, file, current_inode.stat(), new_inode)?;
}
Inode::Leaf(leaf) => {
handle_leaf(current_etc_fd, new_etc_fd, leaf, new_inode, file)?
merge_leaf(current_etc_fd, new_etc_fd, leaf, new_inode, file)?
}
};
println!("{} {m_type} {ty} {file:?}", m_type.symbol());
}
// Directory/File does not exist in the new /etc
Err(ImageError::NotFound(..)) => match current_inode {
Inode::Directory(..) => {
create_dir_with_perms(new_etc_fd, file, current_inode.stat(), false)?
create_dir_with_perms(new_etc_fd, file, current_inode.stat(), None)?
}
Inode::Leaf(leaf) => {
handle_leaf(current_etc_fd, new_etc_fd, leaf, None, file)?;
merge_leaf(current_etc_fd, new_etc_fd, leaf, None, file)?;
}
},
@@ -701,36 +702,34 @@ fn handle_modified_files(
Ok(())
}
// Goes through the added, modified, removed files and apply those changes to the new_etc
// This will overwrite, remove, modify files in new_etc
// Paths in `diff` are relative to `etc`
/// Goes through the added, modified, removed files and apply those changes to the new_etc
/// This will overwrite, remove, modify files in new_etc
/// Paths in `diff` are relative to `etc`
#[context("Merging")]
fn merge(
pub fn merge(
current_etc_fd: &CapStdDir,
current_etc_dirtree: &Directory<CustomMetadata>,
new_etc_fd: &CapStdDir,
new_etc_dirtree: &Directory<CustomMetadata>,
diff: Diff,
) -> anyhow::Result<()> {
handle_modified_files(
merge_modified_files(
&diff.added,
current_etc_fd,
current_etc_dirtree,
new_etc_fd,
new_etc_dirtree,
ModificationType::Added,
)
.context("Handling added files")?;
.context("Merging added files")?;
handle_modified_files(
merge_modified_files(
&diff.modified,
current_etc_fd,
current_etc_dirtree,
new_etc_fd,
new_etc_dirtree,
ModificationType::Modified,
)
.context("Handling modified files")?;
.context("Merging modified files")?;
for removed in diff.removed {
match new_etc_fd.remove_file(&removed) {
@@ -974,21 +973,23 @@ mod tests {
// Directory permissions
p.create_dir_all("dir/perms")?;
p.create_dir_all("dir/perms/wo")?;
p.create_dir_all("dir/perms/wo/ro")?;
c.create_dir_all("dir/perms")?;
c.set_permissions("dir/perms", Permissions::from_mode(0o777))?;
// Directory ownership
// p.create_dir_all("dir/owner")?;
c.create_dir_all("dir/perms/rwx")?;
c.set_permissions("dir/perms/rwx", Permissions::from_mode(0o777))?;
// c.create_dir_all("dir/owner")?;
// rustix::fs::chownat(
// &c,
// "dir/owner",
// Some(Uid::from_raw(u16::MAX as u32)),
// Some(Gid::from_raw(u16::MAX as u32)),
// AtFlags::SYMLINK_NOFOLLOW,
// )?;
c.create_dir_all("dir/perms/wo")?;
c.set_permissions("dir/perms/wo", Permissions::from_mode(0o733))?;
c.create_dir_all("dir/perms/wo/ro")?;
c.set_permissions("dir/perms/wo/ro", Permissions::from_mode(0o775))?;
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 diff = compute_diff(&pristine_etc_files, &current_etc_files)?;
@@ -1027,10 +1028,51 @@ mod tests {
n.metadata("dir/perms")?
));
// assert!(compare_meta(
// c.metadata("dir/owner")?,
// n.metadata("dir/owner")?
// ));
// Make sure nothing is deleted from a directory
assert!(n.exists("dir/perms/some-file"));
const DIR_BITS: u32 = 0o040000;
assert_eq!(
n.metadata("dir/perms/rwx").unwrap().mode(),
DIR_BITS | 0o777
);
assert_eq!(n.metadata("dir/perms/wo").unwrap().mode(), DIR_BITS | 0o733);
assert_eq!(
n.metadata("dir/perms/wo/ro").unwrap().mode(),
DIR_BITS | 0o775
);
Ok(())
}
#[test]
fn file_to_dir() -> 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", "some text 1")?;
n.create_dir_all("file-to-dir")?;
let (pristine_etc_files, current_etc_files, new_etc_files) = traverse_etc(&p, &c, &n)?;
let diff = compute_diff(&pristine_etc_files, &current_etc_files)?;
let merge_res = merge(&c, &current_etc_files, &n, &new_etc_files, diff);
assert!(merge_res.is_err());
assert_eq!(
merge_res.unwrap_err().root_cause().to_string(),
"Modified config file \"file-to-dir\" newly defaults to directory. Cannot merge"
);
Ok(())
}

1
crates/lib/Cargo.toml
View File

@@ -22,6 +22,7 @@ bootc-sysusers = { path = "../sysusers" }
bootc-tmpfiles = { path = "../tmpfiles" }
bootc-utils = { package = "bootc-internal-utils", path = "../utils", version = "0.0.0" }
ostree-ext = { path = "../ostree-ext", features = ["bootc"] }
etc-merge = { path = "../etc-merge" }
# Workspace dependencies
anstream = { workspace = true }

35
crates/lib/src/cli.rs
View File

@@ -13,6 +13,7 @@ use cap_std_ext::cap_std;
use cap_std_ext::cap_std::fs::Dir;
use clap::Parser;
use clap::ValueEnum;
use etc_merge::{compute_diff, print_diff};
use fn_error_context::context;
use indoc::indoc;
use ostree::gio;
@@ -533,6 +534,18 @@ pub(crate) enum InternalsOpts {
#[cfg(feature = "rhsm")]
/// Publish subscription-manager facts to /etc/rhsm/facts/bootc.facts
PublishRhsmFacts,
/// Internal command for testing etc-diff/etc-merge
DirDiff {
/// Directory path to the pristine_etc
pristine_etc: Utf8PathBuf,
/// Directory path to the current_etc
current_etc: Utf8PathBuf,
/// Directory path to the new_etc
new_etc: Utf8PathBuf,
/// Whether to perform the three way merge or not
#[clap(long)]
merge: bool,
},
}
#[derive(Debug, clap::Subcommand, PartialEq, Eq)]
@@ -1476,6 +1489,28 @@ async fn run_from_opt(opt: Opt) -> Result<()> {
}
#[cfg(feature = "rhsm")]
InternalsOpts::PublishRhsmFacts => crate::rhsm::publish_facts(&root).await,
InternalsOpts::DirDiff {
pristine_etc,
current_etc,
new_etc,
merge,
} => {
let pristine_etc =
Dir::open_ambient_dir(pristine_etc, cap_std::ambient_authority())?;
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 diff = compute_diff(&p, &c)?;
print_diff(&diff, &mut std::io::stdout());
if merge {
etc_merge::merge(&current_etc, &c, &new_etc, &n, diff)?;
}
Ok(())
}
},
#[cfg(feature = "docgen")]
Opt::Man(manopts) => crate::docgen::generate_manpages(&manopts.directory),