From 916f2968c75ba38fc01d55ee05812202c343eb52 Mon Sep 17 00:00:00 2001 From: Omer Tuchfeld Date: Tue, 5 Nov 2024 15:57:01 +0100 Subject: [PATCH] DRAFT: List logically bound images For now this is just the implementation, but it does not contain the important bug fix for https://github.com/containers/bootc/issues/846 which is to allow this command to run inside a container and not just on a bootc booted host Signed-off-by: Omer Tuchfeld --- Cargo.lock | 85 +++++++++++++- lib/Cargo.toml | 1 + lib/src/cli.rs | 49 +++++++- lib/src/image.rs | 111 ++++++++++++++++--- tests/booted/test-logically-bound-install.nu | 42 ++++++- 5 files changed, 264 insertions(+), 24 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c4257171b..3c8323941 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -176,6 +176,7 @@ dependencies = [ "chrono", "clap", "clap_mangen", + "comfy-table", "fn-error-context", "hex", "indicatif", @@ -402,6 +403,18 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "acbf1af155f9b9ef647e42cdc158db4b64a1b61f743629225fde6f3e0be2a7c7" +[[package]] +name = "comfy-table" +version = "7.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b34115915337defe99b2aff5c2ce6771e5fbc4079f4b506301f5cf394c8452f7" +dependencies = [ + "crossterm", + "strum", + "strum_macros", + "unicode-width", +] + [[package]] name = "console" version = "0.15.8" @@ -458,6 +471,28 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "crossterm" +version = "0.27.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f476fe445d41c9e991fd07515a6f463074b782242ccf4a5b7b1d1012e70824df" +dependencies = [ + "bitflags 2.4.2", + "crossterm_winapi", + "libc", + "parking_lot", + "winapi", +] + +[[package]] +name = "crossterm_winapi" +version = "0.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "acdd7c62a3665c7f6830a51635d9ac9b23ed385797f70a83bb8bafe9c572ab2b" +dependencies = [ + "winapi", +] + [[package]] name = "crypto-common" version = "0.1.6" @@ -602,7 +637,7 @@ checksum = "1ee447700ac8aa0b2f2bd7bc4462ad686ba06baa6727ac149a2d6277f0d240fd" dependencies = [ "cfg-if", "libc", - "redox_syscall", + "redox_syscall 0.4.1", "windows-sys 0.52.0", ] @@ -1114,6 +1149,16 @@ version = "0.4.14" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "78b3ae25bc7c8c38cec158d1f2757ee79e9b3740fbc7ccf0e59e4b08d793fa89" +[[package]] +name = "lock_api" +version = "0.4.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "07af8b9cdd281b7915f413fa73f29ebd5d55d0d3f0155584dade1ff18cea1b17" +dependencies = [ + "autocfg", + "scopeguard", +] + [[package]] name = "log" version = "0.4.22" @@ -1412,6 +1457,29 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b15813163c1d831bf4a13c3610c05c0d03b39feb07f7e09fa234dac9b15aaf39" +[[package]] +name = "parking_lot" +version = "0.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f1bf18183cf54e8d6059647fc3063646a1801cf30896933ec2311622cc4b9a27" +dependencies = [ + "lock_api", + "parking_lot_core", +] + +[[package]] +name = "parking_lot_core" +version = "0.9.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1e401f977ab385c9e4e3ab30627d6f26d00e2c73eef317493c4ec6d468726cf8" +dependencies = [ + "cfg-if", + "libc", + "redox_syscall 0.5.7", + "smallvec", + "windows-targets 0.52.6", +] + [[package]] name = "pin-project" version = "1.1.4" @@ -1571,6 +1639,15 @@ dependencies = [ "bitflags 1.3.2", ] +[[package]] +name = "redox_syscall" +version = "0.5.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9b6dfecf2c74bce2466cabf93f6664d6998a69eb21e39f4207930065b27b771f" +dependencies = [ + "bitflags 2.4.2", +] + [[package]] name = "ref-cast" version = "1.0.22" @@ -1699,6 +1776,12 @@ dependencies = [ "syn 2.0.82", ] +[[package]] +name = "scopeguard" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" + [[package]] name = "semver" version = "1.0.21" diff --git a/lib/Cargo.toml b/lib/Cargo.toml index 015b489c7..22604f653 100644 --- a/lib/Cargo.toml +++ b/lib/Cargo.toml @@ -46,6 +46,7 @@ toml = "0.8.12" xshell = { version = "0.2.6", optional = true } uuid = { version = "1.8.0", features = ["v4"] } tini = "1.3.0" +comfy-table = "7.1.1" [dev-dependencies] indoc = { workspace = true } diff --git a/lib/src/cli.rs b/lib/src/cli.rs index 5e19d400e..35ad6f3a7 100644 --- a/lib/src/cli.rs +++ b/lib/src/cli.rs @@ -21,6 +21,7 @@ use ostree_ext::container as ostree_container; use ostree_ext::keyfileext::KeyFileExt; use ostree_ext::ostree; use schemars::schema_for; +use serde::{Deserialize, Serialize}; use crate::deploy::RequiredHostSpec; use crate::lints; @@ -235,13 +236,54 @@ pub(crate) enum ImageCmdOpts { }, } +#[derive(ValueEnum, Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize, Default)] +#[serde(rename_all = "kebab-case")] +pub(crate) enum ImageListType { + /// List all images + #[default] + All, + /// List only logically bound images + Logical, + /// List only host images + Host, +} + +impl std::fmt::Display for ImageListType { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.to_possible_value().unwrap().get_name().fmt(f) + } +} + +#[derive(ValueEnum, Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize, Default)] +#[serde(rename_all = "kebab-case")] +pub(crate) enum ImageListFormat { + /// Human readable table format + #[default] + Table, + /// JSON format + Json, +} +impl std::fmt::Display for ImageListFormat { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.to_possible_value().unwrap().get_name().fmt(f) + } +} + /// Subcommands which operate on images. #[derive(Debug, clap::Subcommand, PartialEq, Eq)] pub(crate) enum ImageOpts { /// List fetched images stored in the bootc storage. /// /// Note that these are distinct from images stored via e.g. `podman`. - List, + List { + /// Type of image to list + #[clap(long = "type")] + #[arg(default_value_t)] + list_type: ImageListType, + #[clap(long = "format")] + #[arg(default_value_t)] + list_format: ImageListFormat, + }, /// Copy a container image from the bootc storage to `containers-storage:`. /// /// The source and target are both optional; if both are left unspecified, @@ -876,7 +918,10 @@ async fn run_from_opt(opt: Opt) -> Result<()> { } }, Opt::Image(opts) => match opts { - ImageOpts::List => crate::image::list_entrypoint().await, + ImageOpts::List { + list_type, + list_format, + } => crate::image::list_entrypoint(list_type, list_format).await, ImageOpts::CopyToStorage { source, target } => { crate::image::push_entrypoint(source.as_deref(), target.as_deref()).await } diff --git a/lib/src/image.rs b/lib/src/image.rs index eac891a0d..2a4a302dc 100644 --- a/lib/src/image.rs +++ b/lib/src/image.rs @@ -4,31 +4,112 @@ use anyhow::{Context, Result}; use bootc_utils::CommandRunExt; +use clap::ValueEnum; +use comfy_table::{presets::NOTHING, Table}; use fn_error_context::context; use ostree_ext::container::{ImageReference, Transport}; +use serde::Serialize; -use crate::imgstorage::Storage; +use crate::cli::{ImageListFormat, ImageListType}; /// The name of the image we push to containers-storage if nothing is specified. const IMAGE_DEFAULT: &str = "localhost/bootc"; +#[derive(Clone, Serialize, ValueEnum)] +enum ImageListTypeColumn { + Host, + Logical, +} + +impl std::fmt::Display for ImageListTypeColumn { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.to_possible_value().unwrap().get_name().fmt(f) + } +} + +#[derive(Serialize)] +struct ImageOutput { + image_type: ImageListTypeColumn, + image: String, + // TODO: Add hash, size, etc? Difficult because [`ostree_ext::container::store::list_images`] + // only gives us the pullspec. +} + +#[context("Listing host images")] +fn list_host_images(sysroot: &crate::store::Storage) -> Result> { + let repo = sysroot.repo(); + let images = ostree_ext::container::store::list_images(&repo).context("Querying images")?; + + Ok(images + .iter() + .map(|x| ImageOutput { + image: x.to_string(), + image_type: ImageListTypeColumn::Host, + }) + .collect()) +} + +#[context("Listing logical images")] +fn list_logical_images(sysroot: &crate::store::Storage) -> Result> { + let stdout = { + let mut output_bufread = sysroot + .get_ensure_imgstore()? + .new_image_cmd()? + .arg("list") + .arg("--format={{.Repository}}:{{.Tag}}") + .run_get_output()?; + let mut output_buf = vec![]; + output_bufread.read_to_end(&mut output_buf)?; + String::from_utf8(output_buf)? + }; + + let images = stdout + .lines() + .map(|x| ImageOutput { + image: x.to_string(), + image_type: ImageListTypeColumn::Logical, + }) + .collect(); + + Ok(images) +} + #[context("Listing images")] -pub(crate) async fn list_entrypoint() -> Result<()> { - let sysroot = crate::cli::get_storage().await?; - let repo = &sysroot.repo(); +pub(crate) async fn list_entrypoint( + list_type: ImageListType, + list_format: ImageListFormat, +) -> Result<()> { + // TODO: Get the storage from the container image, not the booted storage + let sysroot: crate::store::Storage = crate::cli::get_storage().await?; + + let images = match list_type { + ImageListType::All => list_host_images(&sysroot)? + .into_iter() + .chain(list_logical_images(&sysroot)?) + .collect(), + ImageListType::Host => list_host_images(&sysroot)?, + ImageListType::Logical => list_logical_images(&sysroot)?, + }; - let images = ostree_ext::container::store::list_images(repo).context("Querying images")?; + match list_format { + ImageListFormat::Table => { + let mut table = Table::new(); - println!("# Host images"); - for image in images { - println!("{image}"); - } - println!(); + table + .load_preset(NOTHING) + .set_header(vec!["REPOSITORY", "TYPE"]); + + for image in images { + table.add_row(vec![image.image, image.image_type.to_string()]); + } - println!("# Logically bound images"); - let mut listcmd = sysroot.get_ensure_imgstore()?.new_image_cmd()?; - listcmd.arg("list"); - listcmd.run()?; + println!("{table}"); + } + ImageListFormat::Json => { + let mut stdout = std::io::stdout(); + serde_json::to_writer_pretty(&mut stdout, &images)?; + } + } Ok(()) } @@ -79,7 +160,7 @@ pub(crate) async fn push_entrypoint(source: Option<&str>, target: Option<&str>) /// Thin wrapper for invoking `podman image ` but set up for our internal /// image store (as distinct from /var/lib/containers default). pub(crate) async fn imgcmd_entrypoint( - storage: &Storage, + storage: &crate::imgstorage::Storage, arg: &str, args: &[std::ffi::OsString], ) -> std::result::Result<(), anyhow::Error> { diff --git a/tests/booted/test-logically-bound-install.nu b/tests/booted/test-logically-bound-install.nu index 6703ab6c1..9756b836a 100644 --- a/tests/booted/test-logically-bound-install.nu +++ b/tests/booted/test-logically-bound-install.nu @@ -1,11 +1,41 @@ use std assert use tap.nu -let images = podman --storage-opt=additionalimagestore=/usr/lib/bootc/storage images --format {{.Repository}} | from csv --noheaders -print "IMAGES:" -podman --storage-opt=additionalimagestore=/usr/lib/bootc/storage images # for debugging -assert ($images | any {|item| $item.column1 == "quay.io/curl/curl"}) -assert ($images | any {|item| $item.column1 == "quay.io/curl/curl-base"}) -assert ($images | any {|item| $item.column1 == "registry.access.redhat.com/ubi9/podman"}) # this image is signed +# This list reflects the LBIs specified in bootc/tests/containerfiles/lbi/usr/share/containers/systemd +let expected_images = [ + "quay.io/curl/curl:latest", + "quay.io/curl/curl-base:latest", + "registry.access.redhat.com/ubi9/podman:latest" # this image is signed +] + +def validate_images [images: table] { + print $"Validating images ($images)" + for expected in $expected_images { + assert ($images | any {|item| $item.image == $expected}) + } +} + +# This test checks that bootc actually populated the bootc storage with the LBI images +def test_logically_bound_images_in_storage [] { + # Use podman to list the images in the bootc storage + let images = podman --storage-opt=additionalimagestore=/usr/lib/bootc/storage images --format {{.Repository}}:{{.Tag}} | from csv --noheaders | rename --column { column1: image } + + # Debug print + print "IMAGES:" + podman --storage-opt=additionalimagestore=/usr/lib/bootc/storage images + + validate_images $images +} + +# This test makes sure that bootc itself knows how to list the LBI images in the bootc storage +def test_bootc_image_list [] { + # Use bootc to list the images in the bootc storage + let images = bootc image list --type logical --format json | from json + + validate_images $images +} + +test_logically_bound_images_in_storage +test_bootc_image_list tap ok