Merge pull request #871 from omertuc/imagelist

List logically bound images

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 }

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,
@@ -886,7 +928,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
             }

lib/src/image.rs

@@ -2,33 +2,122 @@
 //!
 //! APIs for operating on container images in the bootc storage.
 
-use anyhow::{Context, Result};
+use anyhow::{bail, Context, Result};
 use bootc_utils::CommandRunExt;
+use cap_std_ext::cap_std::{self, fs::Dir};
+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::{
+    boundimage::query_bound_images,
+    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<Vec<ImageOutput>> {
+    let repo = sysroot.repo();
+    let images = ostree_ext::container::store::list_images(&repo).context("Querying images")?;
+
+    Ok(images
+        .into_iter()
+        .map(|image| ImageOutput {
+            image,
+            image_type: ImageListTypeColumn::Host,
+        })
+        .collect())
+}
+
+#[context("Listing logical images")]
+fn list_logical_images(root: &Dir) -> Result<Vec<ImageOutput>> {
+    let bound = query_bound_images(root)?;
+
+    Ok(bound
+        .into_iter()
+        .map(|image| ImageOutput {
+            image: image.image,
+            image_type: ImageListTypeColumn::Logical,
+        })
+        .collect())
+}
+
+async fn list_images(list_type: ImageListType) -> Result<Vec<ImageOutput>> {
+    let rootfs = cap_std::fs::Dir::open_ambient_dir("/", cap_std::ambient_authority())
+        .context("Opening /")?;
+
+    let sysroot: Option<crate::store::Storage> =
+        if ostree_ext::container_utils::running_in_container() {
+            None
+        } else {
+            Some(crate::cli::get_storage().await?)
+        };
+
+    Ok(match (list_type, sysroot) {
+        // TODO: Should we list just logical images silently here, or error?
+        (ImageListType::All, None) => list_logical_images(&rootfs)?,
+        (ImageListType::All, Some(sysroot)) => list_host_images(&sysroot)?
+            .into_iter()
+            .chain(list_logical_images(&rootfs)?)
+            .collect(),
+        (ImageListType::Logical, _) => list_logical_images(&rootfs)?,
+        (ImageListType::Host, None) => {
+            bail!("Listing host images requires a booted bootc system")
+        }
+        (ImageListType::Host, Some(sysroot)) => list_host_images(&sysroot)?,
+    })
+}
+
 #[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<()> {
+    let images = list_images(list_type).await?;
 
-    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 +168,7 @@ pub(crate) async fn push_entrypoint(source: Option<&str>, target: Option<&str>)
 /// Thin wrapper for invoking `podman image <X>` 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> {

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