From a7a12d719c6b562f34d70c7e3495b80df986c75f Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 4 Oct 2024 14:32:03 -0400 Subject: [PATCH] compose: Initial kickstart support The treefile syntax is...ok, but it is bespoke to us and we never really emphasized its use outside of our sphere. As I'm looking at https://gitlab.com/fedora/bootc/tracker/-/issues/32 I'd really like to expose something that has existing widespread use as a default entrypoint. kickstart is really old, long predating even virtualization. It has a decent set of features (includes, comments) as a baseline specifically as a mechanism to define package sets, it has key features we want: - turning off weak dependencies - excludes This is just a stub kickstart parser...the next step is to wire this up into the treefile. Signed-off-by: Colin Walters --- Cargo.lock | 7 ++ Cargo.toml | 1 + docs/treefile.md | 11 ++ rust/src/kickstart.rs | 256 ++++++++++++++++++++++++++++++++++++++++++ rust/src/lib.rs | 1 + rust/src/treefile.rs | 95 ++++++++++++++-- 6 files changed, 364 insertions(+), 7 deletions(-) create mode 100644 rust/src/kickstart.rs diff --git a/Cargo.lock b/Cargo.lock index e46e3a8829..0f4a255f8d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2381,6 +2381,7 @@ dependencies = [ "serde_derive", "serde_json", "serde_yaml", + "shlex", "system-deps 7.0.3", "systemd", "tempfile", @@ -2596,6 +2597,12 @@ dependencies = [ "lazy_static", ] +[[package]] +name = "shlex" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0fda2ff0d084019ba4d7c6f371c95d8fd75ce3524c3cb8fb653a3023f6323e64" + [[package]] name = "signal-hook-registry" version = "1.4.1" diff --git a/Cargo.toml b/Cargo.toml index 367b6462b4..86c41c6d94 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -92,6 +92,7 @@ tracing-subscriber = { version = "0.3", features = ["env-filter"] } tokio = { version = "1.40.0", features = ["time", "process", "rt", "net"] } xmlrpc = "0.15.1" termcolor = "1.4.1" +shlex = "1.3.0" [build-dependencies] anyhow = "1.0" diff --git a/docs/treefile.md b/docs/treefile.md index 7226679cba..ad5cb04b86 100644 --- a/docs/treefile.md +++ b/docs/treefile.md @@ -526,6 +526,17 @@ version of `rpm-ostree`. and are purely machine-local state. - `root`: These are plain directories; only use this with composefs enabled! +### Kickstarts + +A file ending in `.ks` will be parsed as a +[kickstart file](https://pykickstart.readthedocs.io/en/latest/kickstart-docs.html). +Only a small subset of kickstart configuration is honored: + +- `%packages` (both install and excludes via prefixing with `-`) + The only supported argument is `--exclude-weakdeps`, which translates + to `recommends: false`. Note that this has a global effect if set. +- `%include` (Only child paths from the included file, not absolute paths) + ### Associated directories In edition `2024`, "associated directories" have been introduced as an experimental feature. These diff --git a/rust/src/kickstart.rs b/rust/src/kickstart.rs new file mode 100644 index 0000000000..5c9ab7d752 --- /dev/null +++ b/rust/src/kickstart.rs @@ -0,0 +1,256 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT + +//! This is a Rust implementation of a small subset of the kickstart +//! file format. It may grow in the future. + +use std::{ + collections::HashSet, + io::{BufReader, Read}, +}; + +use anyhow::{Context, Result}; +use cap_std::fs::{Dir, MetadataExt}; +use clap::Parser; + +// Cap includes just to avoid stack overflow in pathological cases +const MAX_INCLUDES: usize = 256; +const INCLUDE: &str = "%include"; +const PACKAGES: &str = "%packages"; +const END: &str = "%end"; + +#[derive(clap::Parser, Debug)] +pub(crate) struct PackageArgs { + /// Do not include weak dependencies + #[clap(long)] + pub(crate) exclude_weakdeps: bool, +} + +#[derive(Debug)] +pub(crate) struct Packages { + pub(crate) args: PackageArgs, + pub(crate) install: Vec, + pub(crate) excludes: Vec, +} + +#[derive(Debug)] +pub(crate) struct Kickstart { + pub(crate) includes: Vec, + pub(crate) packages: Vec, +} + +#[derive(Debug)] +pub(crate) struct KickstartParsed { + pub(crate) packages: Vec, +} + +fn filtermap_line(line: &str) -> Option<&str> { + // Ignore comments + if line.starts_with('#') { + return None; + } + let line = line.trim(); + if line.is_empty() { + return None; + } + Some(line) +} + +impl Packages { + fn parse<'a, 'b>( + args: impl Iterator, + mut lines: impl Iterator, + ) -> Result { + // Ensure there's an argv0 + let args = [PACKAGES].into_iter().chain(args); + let args = PackageArgs::try_parse_from(args)?; + let mut install = Vec::new(); + let mut excludes = Vec::new(); + while let Some(line) = lines.next() { + let line = line.trim(); + if line == END { + return Ok(Self { + args, + install, + excludes, + }); + } + if let Some(rest) = line.strip_prefix('-') { + excludes.push(rest.to_owned()); + } else { + install.push(line.to_owned()); + } + } + anyhow::bail!("Missing {END} for {PACKAGES}") + } +} + +impl Kickstart { + pub(crate) fn parse(s: &str) -> Result { + let mut includes = Vec::new(); + let mut packages = Vec::new(); + let mut lines = s.lines().filter_map(filtermap_line); + while let Some(line) = lines.next() { + let line = + shlex::split(line).ok_or_else(|| anyhow::anyhow!("Invalid syntax: {line}"))?; + let mut line = line.iter(); + let Some(verb) = line.next() else { continue }; + let mut line = line.map(|s| s.as_str()); + match verb.as_str() { + PACKAGES => { + packages.push(Packages::parse(line, &mut lines)?); + } + INCLUDE => { + let include = line + .next() + .ok_or_else(|| anyhow::anyhow!("Missing path for {INCLUDE}"))?; + if line.next().is_some() { + anyhow::bail!("Too many arguments for {INCLUDE}"); + } + includes.push(include.to_owned()); + } + o => { + anyhow::bail!("Unhandled verb: {o}") + } + } + } + Ok(Self { includes, packages }) + } +} + +impl KickstartParsed { + pub(crate) fn new(d: &Dir, path: &str) -> Result { + let mut loaded = HashSet::new(); + Self::new_recurse(d, path, &mut loaded) + } + + pub(crate) fn new_recurse( + d: &Dir, + path: &str, + loaded: &mut HashSet<(u64, u64)>, + ) -> Result { + let mut ret = KickstartParsed { + packages: Vec::new(), + }; + let f = d.open(path).with_context(|| format!("Opening {path}"))?; + let devino = f.metadata().map(|m| (m.dev(), m.ino()))?; + if !loaded.insert(devino) { + anyhow::bail!("Recursive include: {path}"); + } + anyhow::ensure!(loaded.len() < MAX_INCLUDES); + let mut f = BufReader::new(f); + let mut s = String::new(); + f.read_to_string(&mut s)?; + let ks = Kickstart::parse(&s)?; + ret.packages.extend(ks.packages); + for include in ks.includes { + let child = KickstartParsed::new_recurse(d, &include, loaded)?; + ret.packages.extend(child.packages); + } + Ok(ret) + } +} + +#[cfg(test)] +mod tests { + use cap_std_ext::cap_tempfile::TempDir; + + use super::*; + + #[test] + fn test_filtermap_line() { + let nones = ["", " ", "# foo"]; + for v in nones.iter() { + assert_eq!(filtermap_line(v), None); + } + let idem = ["foo bar baz"]; + for &v in idem.iter() { + assert_eq!(filtermap_line(v), Some(v)); + } + } + + #[test] + fn test_basic() { + let ks = Kickstart::parse(indoc::indoc! { r#" + # This is a comment + %include foo.ks + # Include this + %include subdir/bar.ks + # Blank line below + + %packages + foo + -bar + baz + %end + "# }) + .unwrap(); + assert_eq!(ks.includes.len(), 2); + assert_eq!(ks.includes[1].as_str(), "subdir/bar.ks"); + assert_eq!(ks.packages.len(), 1); + let pkgs = ks.packages.first().unwrap(); + assert_eq!(pkgs.install.len(), 2); + assert_eq!(pkgs.excludes.len(), 1); + } + + #[test] + fn test_load_from_dir() -> Result<()> { + let td = TempDir::new(cap_std::ambient_authority())?; + td.write("empty.ks", "")?; + let ks = KickstartParsed::new(&td, "empty.ks").unwrap(); + assert_eq!(ks.packages.len(), 0); + + td.create_dir("subdir")?; + td.write( + "subdir/inc.ks", + indoc::indoc! { r#" + %packages --exclude-weakdeps + systemd + # Let's go modern + -bash + nushell + %end + "#}, + )?; + + td.write( + "basic.ks", + indoc::indoc! { r#" + %packages + foo + -bar + baz + %end + # Our includes + %include empty.ks + %include subdir/inc.ks + "#}, + )?; + + let ks = KickstartParsed::new(&td, "basic.ks").unwrap(); + assert_eq!(ks.packages.len(), 2); + let pkgs = &ks.packages[0]; + assert!(!pkgs.args.exclude_weakdeps); + assert_eq!(pkgs.install[0], "foo"); + assert!(ks.packages[1].args.exclude_weakdeps); + assert_eq!(ks.packages[1].excludes[0], "bash"); + + Ok(()) + } + + #[test] + fn test_loop() -> Result<()> { + let td = TempDir::new(cap_std::ambient_authority())?; + td.write("recursive1.ks", "%include recursive2.ks")?; + td.write("recursive2.ks", "%include recursive1.ks")?; + assert!(KickstartParsed::new(&td, "recursive1.ks").is_err()); + Ok(()) + } + + #[test] + fn test_parse_err() { + let errs = ["%packages\n", "%packages --foo\n%end\n"]; + for err in errs { + assert!(Kickstart::parse(err).is_err()); + } + } +} diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 56d8b57fa3..7052304e48 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -990,6 +990,7 @@ pub(crate) use self::initramfs::*; mod isolation; mod journal; pub(crate) use self::journal::*; +mod kickstart; mod lockfile; pub(crate) use self::lockfile::*; mod live; diff --git a/rust/src/treefile.rs b/rust/src/treefile.rs index b43f0fc75c..ced64b4dba 100644 --- a/rust/src/treefile.rs +++ b/rust/src/treefile.rs @@ -35,6 +35,7 @@ use std::collections::btree_map::Entry; use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use std::fs::{read_dir, File}; use std::io::prelude::*; +use std::os::unix::ffi::OsStrExt; use std::os::unix::fs::{MetadataExt, PermissionsExt}; use std::os::unix::io::{AsRawFd, RawFd}; use std::path::{Path, PathBuf}; @@ -289,13 +290,29 @@ fn treefile_parse>( } }; let mut f = io::BufReader::new(f); - let fmt = utils::InputFormat::detect_from_filename(filename)?; - let tf = treefile_parse_stream(fmt, &mut f, basearch).map_err(|e| { - io::Error::new( - io::ErrorKind::InvalidInput, - format!("Parsing {}: {e}", filename.to_string_lossy()), - ) - })?; + let extension = filename.extension().map(|b| b.as_bytes()); + let tf = if matches!(extension, Some(b"ks")) { + let parent = filename + .parent() + .ok_or_else(|| anyhow::anyhow!("Expected parent for filename {filename:?}"))?; + let name = filename + .file_name() + .ok_or_else(|| anyhow::anyhow!("Expected filename in {filename:?}"))?; + let name = name + .to_str() + .ok_or_else(|| anyhow::anyhow!("Invalid non-UTF8 filename: {filename:?}"))?; + let parent = Dir::open_ambient_dir(parent, cap_std::ambient_authority())?; + let ks = crate::kickstart::KickstartParsed::new(&parent, name)?; + TreeComposeConfig::from_kickstart(ks) + } else { + let fmt = utils::InputFormat::detect_from_filename(filename)?; + treefile_parse_stream(fmt, &mut f, basearch).map_err(|e| { + io::Error::new( + io::ErrorKind::InvalidInput, + format!("Parsing {}: {e}", filename.to_string_lossy()), + ) + })? + }; let postprocess_script = if let Some(ref postprocess) = tf.base.postprocess_script.as_ref() { Some(utils::open_file(filename.with_file_name(postprocess))?) } else { @@ -2870,6 +2887,30 @@ impl TreeComposeConfig { Ok(()) } + /// Convert a kickstart into a treefile. + fn from_kickstart(ks: crate::kickstart::KickstartParsed) -> Self { + let mut packages = BTreeSet::new(); + let mut excludes = Vec::new(); + let mut recommends = None; + for pkg in ks.packages { + packages.extend(pkg.install); + excludes.extend(pkg.excludes); + if pkg.args.exclude_weakdeps { + recommends = Some(false) + } + } + let base = BaseComposeConfigFields { + exclude_packages: Some(excludes), + recommends, + ..Default::default() + }; + Self { + packages: Some(packages), + base, + ..Default::default() + } + } + pub(crate) fn get_check_passwd(&self) -> &CheckPasswd { static DEFAULT: CheckPasswd = CheckPasswd::Previous; self.base.check_passwd.as_ref().unwrap_or(&DEFAULT) @@ -3462,6 +3503,46 @@ arch-include: Ok(()) } + const BASIC_KS: &str = indoc::indoc! { r#" + %packages + systemd + -bash + nushell + %end + "# }; + + #[test] + fn test_kickstart() -> Result<()> { + let workdir = tempfile::tempdir()?; + let workdir: &Utf8Path = workdir.path().try_into().unwrap(); + std::fs::write(workdir.join("foo.ks"), BASIC_KS)?; + let ks_path = &workdir.join("test.ks"); + std::fs::write(ks_path, BASIC_KS)?; + let tf = Treefile::new_boxed(ks_path, None).unwrap(); + let pkgs = tf.parsed.packages.as_ref().unwrap(); + assert_eq!(pkgs.len(), 2); + assert!(pkgs.iter().any(|p| p.as_str() == "nushell")); + assert_eq!(tf.parsed.base.exclude_packages.unwrap().len(), 1); + Ok(()) + } + + #[test] + fn test_treefile_with_kickstart() -> Result<()> { + let workdir = tempfile::tempdir()?; + let workdir: &Utf8Path = workdir.path().try_into().unwrap(); + std::fs::write(workdir.join("foo.ks"), BASIC_KS)?; + let mut buf = VALID_PRELUDE.to_string(); + buf.push_str(indoc! {" + include: foo.ks + "}); + let tf = new_test_treefile(workdir, buf.as_str(), None)?; + let pkgs = tf.parsed.packages.as_ref().unwrap(); + assert_eq!(pkgs.len(), 7); + assert!(pkgs.iter().any(|p| p.as_str() == "nushell")); + assert_eq!(tf.parsed.base.exclude_packages.unwrap().len(), 2); + Ok(()) + } + #[test] fn test_treefile_conditional_includes() -> Result<()> { let workdir = tempfile::tempdir()?;