Skip to content

Commit 78ab9c7

Browse files
committed
kargs: Parse booted kargs via cap-std, not liboverdrop
liboverdrop isn't helpful here because I don't think we want or need right now to actually support things like having a file in `/etc` or `/run` override the kargs. Especially for `/run` there's no dynamic runtime state. It *might* in theory be useful for someone to have `/etc` overrides, but since we weren't actually doing that today, don't pretend we might by using liboverdrop but not passing `/etc` to it. Using cap-std here makes it easier to unit test safely. (Although, liboverdrop really should grow an optional feature to use cap-std) Signed-off-by: Colin Walters <[email protected]>
1 parent ee0b350 commit 78ab9c7

File tree

3 files changed

+103
-8
lines changed

3 files changed

+103
-8
lines changed

Cargo.lock

Lines changed: 38 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ toml = "0.8.12"
4646
xshell = { version = "0.2.6", optional = true }
4747
uuid = { version = "1.8.0", features = ["v4"] }
4848

49+
[dev-dependencies]
50+
similar-asserts = { version = "1.5.0" }
51+
4952
[features]
5053
default = ["install"]
5154
# This feature enables `bootc install`. Disable if you always want to use an external installer.

lib/src/kargs.rs

Lines changed: 62 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
11
use anyhow::{Context, Result};
2+
use camino::Utf8Path;
3+
use cap_std_ext::cap_std;
4+
use cap_std_ext::cap_std::fs::Dir;
5+
use cap_std_ext::dirext::CapStdExtDirExt;
26
use ostree::gio;
37
use ostree_ext::ostree;
48
use ostree_ext::ostree::Deployment;
@@ -16,6 +20,35 @@ struct Config {
1620
match_architectures: Option<Vec<String>>,
1721
}
1822

23+
/// Load and parse all bootc kargs.d files in the specified root, returning
24+
/// a combined list.
25+
fn get_kargs_in_root(d: &Dir, sys_arch: &str) -> Result<Vec<String>> {
26+
// If the directory doesn't exist, that's OK.
27+
let d = if let Some(d) = d.open_dir_optional("usr/lib/bootc/kargs.d")? {
28+
d
29+
} else {
30+
return Ok(Default::default());
31+
};
32+
let mut ret = Vec::new();
33+
// Read all the entries
34+
let mut entries = d.entries()?.collect::<std::io::Result<Vec<_>>>()?;
35+
// cc https://github.com/rust-lang/rust/issues/85573 re the allocation-per-comparison here
36+
entries.sort_by(|a, b| a.file_name().cmp(&b.file_name()));
37+
for ent in entries {
38+
let name = ent.file_name();
39+
let name = name
40+
.to_str()
41+
.ok_or_else(|| anyhow::anyhow!("Invalid non-UTF8 filename: {name:?}"))?;
42+
if !matches!(Utf8Path::new(name).extension(), Some("toml")) {
43+
continue;
44+
}
45+
let buf = d.read_to_string(name)?;
46+
let kargs = parse_kargs_toml(&buf, sys_arch).with_context(|| format!("Parsing {name}"))?;
47+
ret.extend(kargs)
48+
}
49+
Ok(ret)
50+
}
51+
1952
/// Compute the kernel arguments for the new deployment. This starts from the booted
2053
/// karg, but applies the diff between the bootc karg files in /usr/lib/bootc/kargs.d
2154
/// between the booted deployment and the new one.
@@ -38,14 +71,8 @@ pub(crate) fn get_kargs(
3871
};
3972

4073
// Get the kargs in kargs.d of the booted system
41-
let mut existing_kargs: Vec<String> = vec![];
42-
let fragments = liboverdrop::scan(&["/usr/lib"], "bootc/kargs.d", &["toml"], true);
43-
for (name, path) in fragments {
44-
let s = std::fs::read_to_string(&path)?;
45-
let mut parsed_kargs =
46-
parse_kargs_toml(&s, &sys_arch).with_context(|| format!("Parsing {name:?}"))?;
47-
existing_kargs.append(&mut parsed_kargs);
48-
}
74+
let root = &cap_std::fs::Dir::open_ambient_dir("/", cap_std::ambient_authority())?;
75+
let mut existing_kargs: Vec<String> = get_kargs_in_root(root, &sys_arch)?;
4976

5077
// Get the kargs in kargs.d of the pending image
5178
let mut remote_kargs: Vec<String> = vec![];
@@ -179,3 +206,30 @@ fn test_invalid() {
179206
let test_missing = r#"foo=bar"#;
180207
assert!(parse_kargs_toml(test_missing, "x86_64").is_err());
181208
}
209+
210+
#[test]
211+
fn test_get_kargs_in_root() -> Result<()> {
212+
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
213+
214+
// No directory
215+
assert_eq!(get_kargs_in_root(&td, "x86_64").unwrap().len(), 0);
216+
// Empty directory
217+
td.create_dir_all("usr/lib/bootc/kargs.d")?;
218+
assert_eq!(get_kargs_in_root(&td, "x86_64").unwrap().len(), 0);
219+
// Non-toml file
220+
td.write("usr/lib/bootc/kargs.d/somegarbage", "garbage")?;
221+
assert_eq!(get_kargs_in_root(&td, "x86_64").unwrap().len(), 0);
222+
td.write(
223+
"usr/lib/bootc/kargs.d/01-foo.toml",
224+
r##"kargs = ["console=tty0", "nosmt"]"##,
225+
)?;
226+
td.write(
227+
"usr/lib/bootc/kargs.d/02-bar.toml",
228+
r##"kargs = ["console=ttyS1"]"##,
229+
)?;
230+
231+
let args = get_kargs_in_root(&td, "x86_64").unwrap();
232+
similar_asserts::assert_eq!(args, ["console=tty0", "nosmt", "console=ttyS1"]);
233+
234+
Ok(())
235+
}

0 commit comments

Comments
 (0)