From 0b4f084329eb08eb097b53b6e71d0cb463c9fbf8 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 3 Jan 2024 09:18:29 -0500 Subject: [PATCH] lib: Make image configuration always present We should really always have it nowadays. In the degenerate case where it somehow still isn't present, just return an empty config. This improves ergonomics because it avoids the need to deal with the `Option` in many consumers. --- lib/src/cli.rs | 63 +++++++++++++++++--------------------- lib/src/container/store.rs | 30 +++++++++--------- 2 files changed, 42 insertions(+), 51 deletions(-) diff --git a/lib/src/cli.rs b/lib/src/cli.rs index 6d9b260fc..724229812 100644 --- a/lib/src/cli.rs +++ b/lib/src/cli.rs @@ -778,40 +778,36 @@ async fn container_history(repo: &ostree::Repo, imgref: &ImageReference) -> Resu let width = terminal_size::terminal_size() .map(|x| x.0) .unwrap_or(terminal_size::Width(80)); - if let Some(config) = img.configuration.as_ref() { - { - let mut remaining = width; - for (name, width) in columns.iter() { - print_column(name, *width, &mut remaining); - } - println!(); + { + let mut remaining = width; + for (name, width) in columns.iter() { + print_column(name, *width, &mut remaining); } + println!(); + } - let mut history = config.history().iter(); - let layers = img.manifest.layers().iter(); - for layer in layers { - let histent = history.next(); - let created_by = histent - .and_then(|s| s.created_by().as_deref()) - .unwrap_or(""); - - let mut remaining = width; - - let digest = layer.digest().as_str(); - // Verify it's OK to slice, this should all be ASCII - assert!(digest.chars().all(|c| c.is_ascii())); - let digest_max = columns[0].1; - let digest = &digest[0..digest_max as usize]; - print_column(digest, digest_max, &mut remaining); - let size = glib::format_size(layer.size() as u64); - print_column(size.as_str(), columns[1].1, &mut remaining); - print_column(created_by, columns[2].1, &mut remaining); - println!(); - } - Ok(()) - } else { - anyhow::bail!("v0 image does not have fetched configuration"); + let mut history = img.configuration.history().iter(); + let layers = img.manifest.layers().iter(); + for layer in layers { + let histent = history.next(); + let created_by = histent + .and_then(|s| s.created_by().as_deref()) + .unwrap_or(""); + + let mut remaining = width; + + let digest = layer.digest().as_str(); + // Verify it's OK to slice, this should all be ASCII + assert!(digest.chars().all(|c| c.is_ascii())); + let digest_max = columns[0].1; + let digest = &digest[0..digest_max as usize]; + print_column(digest, digest_max, &mut remaining); + let size = glib::format_size(layer.size() as u64); + print_column(size.as_str(), columns[1].1, &mut remaining); + print_column(created_by, columns[2].1, &mut remaining); + println!(); } + Ok(()) } /// Add IMA signatures to an ostree commit, generating a new commit. @@ -974,10 +970,7 @@ async fn run_from_opt(opt: Opt) -> Result<()> { let stdout = std::io::stdout().lock(); let mut stdout = std::io::BufWriter::new(stdout); if config { - let config = image - .configuration - .ok_or_else(|| anyhow::anyhow!("Missing configuration"))?; - serde_json::to_writer(&mut stdout, &config)?; + serde_json::to_writer(&mut stdout, &image.configuration)?; } else { serde_json::to_writer(&mut stdout, &image.manifest)?; } diff --git a/lib/src/container/store.rs b/lib/src/container/store.rs index 794496534..88b35fdc0 100644 --- a/lib/src/container/store.rs +++ b/lib/src/container/store.rs @@ -120,8 +120,8 @@ pub struct LayeredImageState { pub manifest_digest: String, /// The image manfiest pub manifest: ImageManifest, - /// The image configuration; for v0 images, may not be available. - pub configuration: Option, + /// The image configuration + pub configuration: ImageConfiguration, /// Metadata for (cached, previously fetched) updates to the image, if any. pub cached_update: Option, } @@ -143,9 +143,7 @@ impl LayeredImageState { /// Retrieve the container image version. pub fn version(&self) -> Option<&str> { - self.configuration - .as_ref() - .and_then(super::version_for_config) + super::version_for_config(&self.configuration) } } @@ -328,14 +326,19 @@ fn manifest_data_from_commitmeta( Ok((r, digest)) } -fn image_config_from_commitmeta( - commit_meta: &glib::VariantDict, -) -> Result> { - commit_meta +fn image_config_from_commitmeta(commit_meta: &glib::VariantDict) -> Result { + let config = if let Some(config) = commit_meta .lookup::(META_CONFIG)? .filter(|v| v != "null") // Format v0 apparently old versions injected `null` here sadly... .map(|v| serde_json::from_str(&v).map_err(anyhow::Error::msg)) - .transpose() + .transpose()? + { + config + } else { + tracing::debug!("No image configuration found"); + Default::default() + }; + Ok(config) } /// Return the original digest of the manifest stored in the commit metadata. @@ -1579,13 +1582,8 @@ pub(crate) fn verify_container_image( .expect("downcast"); merge_commit_root.ensure_resolved()?; - // This shouldn't happen anymore - let config = state - .configuration - .as_ref() - .ok_or_else(|| anyhow!("Missing configuration for image"))?; let (commit_layer, _component_layers, remaining_layers) = - parse_manifest_layout(&state.manifest, config)?; + parse_manifest_layout(&state.manifest, &state.configuration)?; let mut comparison_state = CompareState::default();