List logically bound images

Solves the second part of #846

The (hidden) image list command now has a `--type` flag to allow users
to list only logical images, only host images, or all images.

Also the command has been adjusted so that it can run even when not
booted off of a bootc system. In that case, it will only list logical
images. If a user tries to list host images without a booted system, an
error will be thrown.

The command also has a `--format` flag to allow users to choose between
a human-readable table format and a JSON format.

Signed-off-by: Omer Tuchfeld <[email protected]>

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
+        .iter()
+        .map(|x| ImageOutput {
+            image: x.to_string(),
+            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
+        .iter()
+        .map(|x| ImageOutput {
+            image: x.image.clone(),
+            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-integration/src/container.rs

@@ -23,7 +23,7 @@ pub(crate) fn test_bootc_upgrade() -> Result<()> {
         assert!(!st.success());
         let stderr = String::from_utf8(o.stderr)?;
         assert!(
-            stderr.contains("this command requires a booted host system"),
+            stderr.contains("This command requires a booted host system"),
             "stderr: {stderr}",
         );
     }