Skip to content

Commit

Permalink
Merge pull request #645 from cgwalters/kargs-cleanup
Browse files Browse the repository at this point in the history 
kargs: Various cleanups
  • Loading branch information
@cgwalters
cgwalters authored Jun 28, 2024
2 parents 5e9279d + 27d955c commit 0bd1956
Show file tree
Hide file tree
Showing 4 changed files with 172 additions and 61 deletions.
38 changes: 38 additions & 0 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion hack/Containerfile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions lib/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
190 changes: 130 additions & 60 deletions lib/src/kargs.rs
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -17,6 +20,35 @@ struct Config {
match_architectures: Option<Vec<String>>,
}

/// 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<Vec<String>> {
// 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::<std::io::Result<Vec<_>>>()?;
// 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.
Expand All @@ -27,38 +59,34 @@ pub(crate) fn get_kargs(
) -> Result<Vec<String>> {
let cancellable = gio::Cancellable::NONE;
let mut kargs: Vec<String> = 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<String> = 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<String> = 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<String> = get_kargs_in_root(root, sys_arch)?;

// Get the kargs in kargs.d of the pending image
let mut remote_kargs: Vec<String> = 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::<ostree::RepoFile>()
.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<String> = vec![];
let queryattrs = "standard::name,standard::type";
let queryflags = gio::FileQueryInfoFlags::NOFOLLOW_SYMLINKS;
let fetched_iter = fetched_tree.enumerate_children(queryattrs, queryflags, cancellable)?;
Expand All @@ -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);
}
}
Expand Down Expand Up @@ -110,58 +139,99 @@ pub(crate) fn get_kargs(
Ok(kargs)
}

pub fn parse_file(file_content: String, sys_arch: String) -> Result<Vec<String>> {
let mut de: Config = toml::from_str(&file_content)?;
let mut parsed_kargs: Vec<String> = 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<Vec<String>> {
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(())
}
}

0 comments on commit 0bd1956

Please sign in to comment.