From 360b85faf19102ee1a68ffb15502f62d9f9c2c0b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 29 Apr 2022 17:54:35 -0400 Subject: [PATCH] cliwrap: Enable `yum install foo` in a container For the "ostree native container" bits, the goal is to make things *extremely obvious* to someone who is using a `Dockerfile` to build derived images. Another way to say this is that a goal is for ostree to "fade into the background". It's still there - just not something everyone needs to learn or care about. This makes it so that typing `yum install usbguard && yum clean all` Just Works as one would expect. --- ci/test-container.sh | 9 +++ rust/src/cliwrap.rs | 5 +- rust/src/cliwrap/rpm.rs | 53 ++++++++++++---- rust/src/cliwrap/yumdnf.rs | 122 ++++++++++++++++++++++++++++--------- 4 files changed, 147 insertions(+), 42 deletions(-) diff --git a/ci/test-container.sh b/ci/test-container.sh index 8539d2b6a0..cecdf90a13 100755 --- a/ci/test-container.sh +++ b/ci/test-container.sh @@ -28,4 +28,13 @@ fi rm "${origindir}/clienterror.yaml" rpm-ostree ex rebuild +if ! test -x /usr/bin/yum; then + rpm-ostree cliwrap install-to-root / +fi + +# Test a critical path package +yum install cowsay && yum clean all +cowsay "It worked" +test '!' -d /var/cache/rpm-ostree + echo ok diff --git a/rust/src/cliwrap.rs b/rust/src/cliwrap.rs index 0a91d229db..3d35049102 100644 --- a/rust/src/cliwrap.rs +++ b/rust/src/cliwrap.rs @@ -37,6 +37,7 @@ pub(crate) enum RunDisposition { Ok, Warn, Notice(String), + Unsupported, } /// Main entrypoint for cliwrap @@ -68,8 +69,8 @@ pub fn entrypoint(args: &[&str]) -> Result<()> { SystemHostType::OstreeHost | SystemHostType::OstreeContainer ) { match name { - "rpm" => Ok(self::rpm::main(args)?), - "yum" | "dnf" => Ok(self::yumdnf::main(args)?), + "rpm" => Ok(self::rpm::main(host_type, args)?), + "yum" | "dnf" => Ok(self::yumdnf::main(host_type, args)?), "dracut" => Ok(self::dracut::main(args)?), "grubby" => Ok(self::grubby::main(args)?), _ => Err(anyhow!("Unknown wrapped binary: {}", name)), diff --git a/rust/src/cliwrap/rpm.rs b/rust/src/cliwrap/rpm.rs index 7cab3a4413..7f990e3374 100644 --- a/rust/src/cliwrap/rpm.rs +++ b/rust/src/cliwrap/rpm.rs @@ -4,6 +4,7 @@ use clap::{App, Arg}; use crate::cliwrap::cliutil; use crate::cliwrap::RunDisposition; +use crate::ffi::SystemHostType; fn new_rpm_app<'r>() -> App<'r, 'static> { let name = "cli-ostree-wrapper-rpm"; @@ -41,7 +42,14 @@ fn has_query(argv: &[&str]) -> bool { false } -fn disposition(argv: &[&str]) -> Result { +fn disposition(host: SystemHostType, argv: &[&str]) -> Result { + // For now, all rpm invocations are directly passed through + match host { + SystemHostType::OstreeContainer => return Ok(RunDisposition::Ok), + SystemHostType::OstreeHost => {} + _ => return Ok(RunDisposition::Unsupported), + }; + // Today rpm has --query take precendence over --erase and --install // apparently, so let's just accept anything with --query as there // are a lot of sub-options for that. @@ -74,15 +82,18 @@ fn disposition(argv: &[&str]) -> Result { } /// Primary entrypoint to running our wrapped `rpm` handling. -pub(crate) fn main(argv: &[&str]) -> Result<()> { - if cliutil::is_unlocked()? { +pub(crate) fn main(host: SystemHostType, argv: &[&str]) -> Result<()> { + if host == SystemHostType::OstreeHost && cliutil::is_unlocked()? { // For now if we're unlocked, just directly exec rpm. In the future we // may choose to take over installing a package live. cliutil::exec_real_binary("rpm", argv) } else { - match disposition(argv)? { + match disposition(host, argv)? { RunDisposition::Ok => cliutil::run_unprivileged(false, "rpm", argv), RunDisposition::Warn => cliutil::run_unprivileged(true, "rpm", argv), + RunDisposition::Unsupported => Err(anyhow::anyhow!( + "This command is only supported on ostree-based systems." + )), RunDisposition::Notice(ref s) => { println!("{}", s); Ok(()) @@ -97,20 +108,29 @@ mod tests { #[test] fn test_version() -> Result<()> { - assert_eq!(disposition(&["--version"])?, RunDisposition::Ok); + assert_eq!( + disposition(SystemHostType::OstreeHost, &["--version"])?, + RunDisposition::Ok + ); Ok(()) } #[test] fn test_query_all() -> Result<()> { - assert_eq!(disposition(&["-qa"])?, RunDisposition::Ok); + assert_eq!( + disposition(SystemHostType::OstreeHost, &["-qa"])?, + RunDisposition::Ok + ); Ok(()) } #[test] fn test_query_file() -> Result<()> { assert_eq!( - disposition(&["--query", "-f", "/usr/bin/bash"])?, + disposition( + SystemHostType::OstreeHost, + &["--query", "-f", "/usr/bin/bash"] + )?, RunDisposition::Ok ); Ok(()) @@ -119,7 +139,7 @@ mod tests { #[test] fn test_query_requires() -> Result<()> { assert_eq!( - disposition(&["--requires", "-q", "blah"])?, + disposition(SystemHostType::OstreeHost, &["--requires", "-q", "blah"])?, RunDisposition::Ok ); Ok(()) @@ -128,26 +148,35 @@ mod tests { #[test] fn test_query_erase() -> Result<()> { // Note --query overrides --erase today - assert_eq!(disposition(&["-qea", "bash"])?, RunDisposition::Ok); + assert_eq!( + disposition(SystemHostType::OstreeHost, &["-qea", "bash"])?, + RunDisposition::Ok + ); Ok(()) } #[test] fn test_erase() -> Result<()> { - assert_eq!(disposition(&["--erase", "bash"])?, RunDisposition::Warn); + assert_eq!( + disposition(SystemHostType::OstreeHost, &["--erase", "bash"])?, + RunDisposition::Warn + ); Ok(()) } #[test] fn test_shorterase() -> Result<()> { - assert_eq!(disposition(&["-e", "bash"])?, RunDisposition::Warn); + assert_eq!( + disposition(SystemHostType::OstreeHost, &["-e", "bash"])?, + RunDisposition::Warn + ); Ok(()) } #[test] fn test_verify() -> Result<()> { assert!(matches!( - disposition(&["--verify", "bash"])?, + disposition(SystemHostType::OstreeHost, &["--verify", "bash"])?, RunDisposition::Notice(_) )); Ok(()) diff --git a/rust/src/cliwrap/yumdnf.rs b/rust/src/cliwrap/yumdnf.rs index db748f5b46..2a71ecfeaa 100644 --- a/rust/src/cliwrap/yumdnf.rs +++ b/rust/src/cliwrap/yumdnf.rs @@ -7,6 +7,8 @@ use std::path::Path; use std::process::Command; use structopt::StructOpt; +use crate::ffi::SystemHostType; + /// Emitted at the first line. pub(crate) const IMAGEBASED: &str = "Note: This system is image (rpm-ostree) based."; @@ -57,9 +59,11 @@ enum Opt { /// Will return an error suggesting other approaches. Install { /// Set of packages to install - #[allow(dead_code)] packages: Vec, }, + Clean { + subargs: Vec, + }, } #[derive(Debug, PartialEq, Eq)] @@ -68,10 +72,25 @@ enum RunDisposition { ExecRpmOstree(Vec), UseSomethingElse, NotImplementedYet(&'static str), + Unsupported, Unhandled, } -fn disposition(argv: &[&str]) -> Result { +fn run_clean(argv: &Vec) -> Result { + let arg = if let Some(subarg) = argv.get(0) { + subarg + } else { + anyhow::bail!("Missing required argument"); + }; + match arg.as_str() { + "all" | "metadata" | "packages" => Ok(RunDisposition::ExecRpmOstree( + ["cleanup", "-m"].iter().map(|&s| String::from(s)).collect(), + )), + o => anyhow::bail!("Unknown argument: {o}"), + } +} + +fn disposition(hosttype: SystemHostType, argv: &[&str]) -> Result { let opt = match Opt::from_iter_safe(std::iter::once(&"yum").chain(argv.iter())) { Ok(v) => v, Err(e) @@ -85,25 +104,45 @@ fn disposition(argv: &[&str]) -> Result { } }; - let disp = match opt { - Opt::Upgrade | Opt::Update => RunDisposition::ExecRpmOstree(vec!["upgrade".into()]), - Opt::Status => RunDisposition::ExecRpmOstree(vec!["status".into()]), - Opt::Install { packages: _ } => { - // TODO analyze packages to find e.g. `gcc` (not ok, use `toolbox`) versus `libvirt` (ok) - RunDisposition::UseSomethingElse - } - Opt::Search { .. } => RunDisposition::NotImplementedYet(indoc! { r##" + let disp = match hosttype { + SystemHostType::OstreeHost => { + match opt { + Opt::Upgrade | Opt::Update => RunDisposition::ExecRpmOstree(vec!["upgrade".into()]), + Opt::Status => RunDisposition::ExecRpmOstree(vec!["status".into()]), + Opt::Install { packages: _ } => { + // TODO analyze packages to find e.g. `gcc` (not ok, use `toolbox`) versus `libvirt` (ok) + RunDisposition::UseSomethingElse + }, + Opt::Clean { subargs } => { + run_clean(&subargs)? + } + Opt::Search { .. } => RunDisposition::NotImplementedYet(indoc! { r##" Package search is not yet implemented. For now, it's recommended to use e.g. `toolbox` and `dnf search` inside there. "##}), + } + }, + SystemHostType::OstreeContainer => match opt { + Opt::Upgrade | Opt::Update => RunDisposition::NotImplementedYet("At the current time, it is not supported to update packages independently of the base image."), + Opt::Install { mut packages } => { + packages.insert(0, "install".into()); + RunDisposition::ExecRpmOstree(packages) + }, + Opt::Clean { subargs } => run_clean(&subargs)?, + Opt::Status => RunDisposition::ExecRpmOstree(vec!["status".into()]), + Opt::Search { .. } => { + RunDisposition::NotImplementedYet("Package search is not yet implemented.") + } + }, + _ => RunDisposition::Unsupported }; Ok(disp) } /// Primary entrypoint to running our wrapped `yum`/`dnf` handling. -pub(crate) fn main(argv: &[&str]) -> Result<()> { - match disposition(argv)? { +pub(crate) fn main(hosttype: SystemHostType, argv: &[&str]) -> Result<()> { + match disposition(hosttype, argv)? { RunDisposition::HelpOrVersionDisplayed => Ok(()), RunDisposition::ExecRpmOstree(args) => { eprintln!("{}", IMAGEBASED); @@ -132,6 +171,9 @@ pub(crate) fn main(argv: &[&str]) -> Result<()> { Err(anyhow!("not implemented")) } RunDisposition::Unhandled => Err(anyhow!("{}", UNHANDLED)), + RunDisposition::Unsupported => Err(anyhow!( + "This command is only supported on ostree-based systems." + )), RunDisposition::NotImplementedYet(msg) => Err(anyhow!("{}\n{}", IMAGEBASED, msg)), } } @@ -142,32 +184,56 @@ mod tests { #[test] fn test_yumdnf() -> Result<()> { + for common in [SystemHostType::OstreeContainer, SystemHostType::OstreeHost] { + assert!(matches!( + disposition(common, &["--version"])?, + RunDisposition::HelpOrVersionDisplayed + )); + assert!(matches!( + disposition(common, &["--help"])?, + RunDisposition::HelpOrVersionDisplayed + )); + assert!(matches!( + disposition(common, &["unknown", "--other"])?, + RunDisposition::Unhandled + )); + assert!(matches!( + disposition(common, &["search", "foo", "bar"])?, + RunDisposition::NotImplementedYet(_) + )); + } + + // Tests for the ostree host case + let host = SystemHostType::OstreeHost; assert!(matches!( - disposition(&["--version"])?, - RunDisposition::HelpOrVersionDisplayed - )); - assert!(matches!( - disposition(&["--help"])?, - RunDisposition::HelpOrVersionDisplayed - )); - assert!(matches!( - disposition(&["unknown", "--other"])?, - RunDisposition::Unhandled - )); - assert!(matches!( - disposition(&["upgrade"])?, + disposition(host, &["upgrade"])?, RunDisposition::ExecRpmOstree(_) )); assert!(matches!( - disposition(&["install", "foo"])?, + disposition(host, &["install", "foo"])?, RunDisposition::UseSomethingElse )); assert!(matches!( - disposition(&["install", "foo", "bar"])?, + disposition(host, &["install", "foo", "bar"])?, RunDisposition::UseSomethingElse )); + + fn strvec(s: impl IntoIterator) -> Vec { + s.into_iter().map(|s| String::from(s)).collect() + } + + // Tests for the ostree container case + let host = SystemHostType::OstreeContainer; + assert_eq!( + disposition(host, &["install", "foo", "bar"])?, + RunDisposition::ExecRpmOstree(strvec(["install", "foo", "bar"])) + ); + assert_eq!( + disposition(host, &["clean", "all"])?, + RunDisposition::ExecRpmOstree(strvec(["cleanup", "-m"])) + ); assert!(matches!( - disposition(&["search", "foo", "bar"])?, + disposition(host, &["upgrade"])?, RunDisposition::NotImplementedYet(_) )); Ok(())