diff --git a/Cargo.lock b/Cargo.lock index e3f395a41..bb9b0f128 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -193,6 +193,7 @@ dependencies = [ "serde_ignored", "serde_json", "serde_yaml", + "similar-asserts", "tempfile", "tokio", "tokio-util", @@ -202,6 +203,17 @@ dependencies = [ "xshell", ] +[[package]] +name = "bstr" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba3569f383e8f1598449f1a423e72e99569137b47740b1da11ef19af3d5c3223" +dependencies = [ + "lazy_static", + "memchr", + "regex-automata 0.1.10", +] + [[package]] name = "bumpalo" version = "3.14.0" @@ -1793,6 +1805,26 @@ dependencies = [ "libc", ] +[[package]] +name = "similar" +version = "2.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fa42c91313f1d05da9b26f267f931cf178d4aba455b4c4622dd7355eb80c6640" +dependencies = [ + "bstr", + "unicode-segmentation", +] + +[[package]] +name = "similar-asserts" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e041bb827d1bfca18f213411d51b665309f1afb37a04a5d1464530e13779fc0f" +dependencies = [ + "console", + "similar", +] + [[package]] name = "slab" version = "0.4.9" @@ -2181,6 +2213,12 @@ dependencies = [ "tinyvec", ] +[[package]] +name = "unicode-segmentation" +version = "1.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d4c87d22b6e3f4a18d4d40ef354e97c90fcb14dd91d7dc0aa9d8a1172ebf7202" + [[package]] name = "unicode-width" version = "0.1.11" diff --git a/hack/Containerfile b/hack/Containerfile index 3c4141265..bc45d202c 100644 --- a/hack/Containerfile +++ b/hack/Containerfile @@ -25,7 +25,7 @@ RUN /tmp/provision-derived.sh "$variant" && rm -f /tmp/*.sh COPY hack/install-test-configs/* /usr/lib/bootc/install/ # Inject our built code COPY --from=build /out/bootc.tar.zst /tmp -RUN tar -C / --zstd -xvf /tmp/bootc.tar.zst && rm -vf /tmp/* +RUN tar -C / --zstd -xvf /tmp/bootc.tar.zst && rm -vrf /tmp/* # Also copy over arbitrary bits from the target root COPY --from=build /build/target/dev-rootfs/ / # Test our own linting diff --git a/lib/Cargo.toml b/lib/Cargo.toml index d184271f4..dd5ed1969 100644 --- a/lib/Cargo.toml +++ b/lib/Cargo.toml @@ -46,6 +46,9 @@ toml = "0.8.12" xshell = { version = "0.2.6", optional = true } uuid = { version = "1.8.0", features = ["v4"] } +[dev-dependencies] +similar-asserts = { version = "1.5.0" } + [features] default = ["install"] # This feature enables `bootc install`. Disable if you always want to use an external installer. diff --git a/lib/src/kargs.rs b/lib/src/kargs.rs index 27c87cbe0..d4815cbef 100644 --- a/lib/src/kargs.rs +++ b/lib/src/kargs.rs @@ -1,5 +1,8 @@ -use anyhow::Ok; -use anyhow::Result; +use anyhow::{Context, Result}; +use camino::Utf8Path; +use cap_std_ext::cap_std; +use cap_std_ext::cap_std::fs::Dir; +use cap_std_ext::dirext::CapStdExtDirExt; use ostree::gio; use ostree_ext::ostree; use ostree_ext::ostree::Deployment; @@ -17,6 +20,35 @@ struct Config { match_architectures: Option>, } +/// Load and parse all bootc kargs.d files in the specified root, returning +/// a combined list. +fn get_kargs_in_root(d: &Dir, sys_arch: &str) -> Result> { + // If the directory doesn't exist, that's OK. + let d = if let Some(d) = d.open_dir_optional("usr/lib/bootc/kargs.d")? { + d + } else { + return Ok(Default::default()); + }; + let mut ret = Vec::new(); + // Read all the entries + let mut entries = d.entries()?.collect::>>()?; + // cc https://github.com/rust-lang/rust/issues/85573 re the allocation-per-comparison here + entries.sort_by(|a, b| a.file_name().cmp(&b.file_name())); + for ent in entries { + let name = ent.file_name(); + let name = name + .to_str() + .ok_or_else(|| anyhow::anyhow!("Invalid non-UTF8 filename: {name:?}"))?; + if !matches!(Utf8Path::new(name).extension(), Some("toml")) { + continue; + } + let buf = d.read_to_string(name)?; + let kargs = parse_kargs_toml(&buf, sys_arch).with_context(|| format!("Parsing {name}"))?; + ret.extend(kargs) + } + Ok(ret) +} + /// Compute the kernel arguments for the new deployment. This starts from the booted /// karg, but applies the diff between the bootc karg files in /usr/lib/bootc/kargs.d /// between the booted deployment and the new one. @@ -27,38 +59,34 @@ pub(crate) fn get_kargs( ) -> Result> { let cancellable = gio::Cancellable::NONE; let mut kargs: Vec = vec![]; - let sys_arch = std::env::consts::ARCH.to_string(); + let sys_arch = std::env::consts::ARCH; // Get the running kargs of the booted system if let Some(bootconfig) = ostree::Deployment::bootconfig(booted_deployment) { if let Some(options) = ostree::BootconfigParser::get(&bootconfig, "options") { - let options: Vec<&str> = options.split_whitespace().collect(); - let mut options: Vec = options.into_iter().map(|s| s.to_string()).collect(); - kargs.append(&mut options); + let options = options.split_whitespace().map(|s| s.to_owned()); + kargs.extend(options); } }; // Get the kargs in kargs.d of the booted system - let mut existing_kargs: Vec = vec![]; - let fragments = liboverdrop::scan(&["/usr/lib"], "bootc/kargs.d", &["toml"], true); - for (_name, path) in fragments { - let s = std::fs::read_to_string(&path)?; - let mut parsed_kargs = parse_file(s.clone(), sys_arch.clone())?; - existing_kargs.append(&mut parsed_kargs); - } + let root = &cap_std::fs::Dir::open_ambient_dir("/", cap_std::ambient_authority())?; + let existing_kargs: Vec = get_kargs_in_root(root, sys_arch)?; // Get the kargs in kargs.d of the pending image - let mut remote_kargs: Vec = vec![]; let (fetched_tree, _) = repo.read_commit(fetched.ostree_commit.as_str(), cancellable)?; let fetched_tree = fetched_tree.resolve_relative_path("/usr/lib/bootc/kargs.d"); let fetched_tree = fetched_tree .downcast::() .expect("downcast"); + // A special case: if there's no kargs.d directory in the pending (fetched) image, + // then we can just use the combined current kargs + kargs from booted if !fetched_tree.query_exists(cancellable) { - // if the kargs.d directory does not exist in the fetched image, return the existing kargs - kargs.append(&mut existing_kargs); + kargs.extend(existing_kargs); return Ok(kargs); } + + let mut remote_kargs: Vec = vec![]; let queryattrs = "standard::name,standard::type"; let queryflags = gio::FileQueryInfoFlags::NOFOLLOW_SYMLINKS; let fetched_iter = fetched_tree.enumerate_children(queryattrs, queryflags, cancellable)?; @@ -79,7 +107,8 @@ pub(crate) fn get_kargs( let mut reader = ostree_ext::prelude::InputStreamExtManual::into_read(file_content.unwrap()); let s = std::io::read_to_string(&mut reader)?; - let mut parsed_kargs = parse_file(s.clone(), sys_arch.clone())?; + let mut parsed_kargs = + parse_kargs_toml(&s, sys_arch).with_context(|| format!("Parsing {name}"))?; remote_kargs.append(&mut parsed_kargs); } } @@ -110,58 +139,99 @@ pub(crate) fn get_kargs( Ok(kargs) } -pub fn parse_file(file_content: String, sys_arch: String) -> Result> { - let mut de: Config = toml::from_str(&file_content)?; - let mut parsed_kargs: Vec = vec![]; +/// This parses a bootc kargs.d toml file, returning the resulting +/// vector of kernel arguments. Architecture matching is performed using +/// `sys_arch`. +fn parse_kargs_toml(contents: &str, sys_arch: &str) -> Result> { + let de: Config = toml::from_str(contents)?; // if arch specified, apply kargs only if the arch matches // if arch not specified, apply kargs unconditionally - match de.match_architectures { - None => parsed_kargs = de.kargs, - Some(match_architectures) => { - if match_architectures.contains(&sys_arch) { - parsed_kargs.append(&mut de.kargs); - } - } - } - Ok(parsed_kargs) + let matched = de + .match_architectures + .map(|arches| arches.iter().any(|s| s == sys_arch)) + .unwrap_or(true); + let r = if matched { de.kargs } else { Vec::new() }; + Ok(r) } -#[test] -/// Verify that kargs are only applied to supported architectures -fn test_arch() { - // no arch specified, kargs ensure that kargs are applied unconditionally - let sys_arch = "x86_64".to_string(); - let file_content = r##"kargs = ["console=tty0", "nosmt"]"##.to_string(); - let parsed_kargs = parse_file(file_content.clone(), sys_arch.clone()).unwrap(); - assert_eq!(parsed_kargs, ["console=tty0", "nosmt"]); - let sys_arch = "aarch64".to_string(); - let parsed_kargs = parse_file(file_content.clone(), sys_arch.clone()).unwrap(); - assert_eq!(parsed_kargs, ["console=tty0", "nosmt"]); - - // one arch matches and one doesn't, ensure that kargs are only applied for the matching arch - let sys_arch = "aarch64".to_string(); - let file_content = r##"kargs = ["console=tty0", "nosmt"] +#[cfg(test)] +mod tests { + use super::*; + + #[test] + /// Verify that kargs are only applied to supported architectures + fn test_arch() { + // no arch specified, kargs ensure that kargs are applied unconditionally + let sys_arch = "x86_64"; + let file_content = r##"kargs = ["console=tty0", "nosmt"]"##.to_string(); + let parsed_kargs = parse_kargs_toml(&file_content, sys_arch).unwrap(); + assert_eq!(parsed_kargs, ["console=tty0", "nosmt"]); + let sys_arch = "aarch64"; + let parsed_kargs = parse_kargs_toml(&file_content, sys_arch).unwrap(); + assert_eq!(parsed_kargs, ["console=tty0", "nosmt"]); + + // one arch matches and one doesn't, ensure that kargs are only applied for the matching arch + let sys_arch = "aarch64"; + let file_content = r##"kargs = ["console=tty0", "nosmt"] match-architectures = ["x86_64"] "## - .to_string(); - let parsed_kargs = parse_file(file_content.clone(), sys_arch.clone()).unwrap(); - assert_eq!(parsed_kargs, [] as [String; 0]); - let file_content = r##"kargs = ["console=tty0", "nosmt"] + .to_string(); + let parsed_kargs = parse_kargs_toml(&file_content, sys_arch).unwrap(); + assert_eq!(parsed_kargs, [] as [String; 0]); + let file_content = r##"kargs = ["console=tty0", "nosmt"] match-architectures = ["aarch64"] "## - .to_string(); - let parsed_kargs = parse_file(file_content.clone(), sys_arch.clone()).unwrap(); - assert_eq!(parsed_kargs, ["console=tty0", "nosmt"]); + .to_string(); + let parsed_kargs = parse_kargs_toml(&file_content, sys_arch).unwrap(); + assert_eq!(parsed_kargs, ["console=tty0", "nosmt"]); - // multiple arch specified, ensure that kargs are applied to both archs - let sys_arch = "x86_64".to_string(); - let file_content = r##"kargs = ["console=tty0", "nosmt"] + // multiple arch specified, ensure that kargs are applied to both archs + let sys_arch = "x86_64"; + let file_content = r##"kargs = ["console=tty0", "nosmt"] match-architectures = ["x86_64", "aarch64"] "## - .to_string(); - let parsed_kargs = parse_file(file_content.clone(), sys_arch.clone()).unwrap(); - assert_eq!(parsed_kargs, ["console=tty0", "nosmt"]); - std::env::set_var("ARCH", "aarch64"); - let parsed_kargs = parse_file(file_content.clone(), sys_arch.clone()).unwrap(); - assert_eq!(parsed_kargs, ["console=tty0", "nosmt"]); + .to_string(); + let parsed_kargs = parse_kargs_toml(&file_content, sys_arch).unwrap(); + assert_eq!(parsed_kargs, ["console=tty0", "nosmt"]); + std::env::set_var("ARCH", "aarch64"); + let parsed_kargs = parse_kargs_toml(&file_content, sys_arch).unwrap(); + assert_eq!(parsed_kargs, ["console=tty0", "nosmt"]); + } + + #[test] + /// Verify some error cases + fn test_invalid() { + let test_invalid_extra = r#"kargs = ["console=tty0", "nosmt"]\nfoo=bar"#; + assert!(parse_kargs_toml(test_invalid_extra, "x86_64").is_err()); + + let test_missing = r#"foo=bar"#; + assert!(parse_kargs_toml(test_missing, "x86_64").is_err()); + } + + #[test] + fn test_get_kargs_in_root() -> Result<()> { + let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?; + + // No directory + assert_eq!(get_kargs_in_root(&td, "x86_64").unwrap().len(), 0); + // Empty directory + td.create_dir_all("usr/lib/bootc/kargs.d")?; + assert_eq!(get_kargs_in_root(&td, "x86_64").unwrap().len(), 0); + // Non-toml file + td.write("usr/lib/bootc/kargs.d/somegarbage", "garbage")?; + assert_eq!(get_kargs_in_root(&td, "x86_64").unwrap().len(), 0); + td.write( + "usr/lib/bootc/kargs.d/01-foo.toml", + r##"kargs = ["console=tty0", "nosmt"]"##, + )?; + td.write( + "usr/lib/bootc/kargs.d/02-bar.toml", + r##"kargs = ["console=ttyS1"]"##, + )?; + + let args = get_kargs_in_root(&td, "x86_64").unwrap(); + similar_asserts::assert_eq!(args, ["console=tty0", "nosmt", "console=ttyS1"]); + + Ok(()) + } }