store: Remove dynamic abstraction

A while ago we added a trait in preparation for multiple backends.
The current composefs branch ignores it and has a bunch of
`if {} else {}`.

Looking at this, what I think will work better in the end is
actually a more refined version of the `if {} else {}` model
instead of trying to really flesh out this trait. It's
hard to say of course until we get there, but the trait
approach forces a high level of abstraction vs just trying
to factor out common code between two backends.

Signed-off-by: Colin Walters <[email protected]>

Author: cgwalters

crates/lib/src/status.rs

@@ -13,12 +13,16 @@ use ostree_ext::container as ostree_container;
 use ostree_ext::container_utils::ostree_booted;
 use ostree_ext::keyfileext::KeyFileExt;
 use ostree_ext::oci_spec;
+use ostree_ext::oci_spec::image::Digest;
+use ostree_ext::oci_spec::image::ImageConfiguration;
 use ostree_ext::ostree;
+use ostree_ext::sysroot::SysrootLock;
 
 use crate::cli::OutputFormat;
+use crate::spec::ImageStatus;
 use crate::spec::{BootEntry, BootOrder, Host, HostSpec, HostStatus, HostType};
 use crate::spec::{ImageReference, ImageSignature};
-use crate::store::{CachedImageStatus, ContainerImageStore, Storage};
+use crate::store::{CachedImageStatus, Storage};
 
 impl From<ostree_container::SignatureSource> for ImageSignature {
     fn from(sig: ostree_container::SignatureSource) -> Self {
@@ -109,41 +113,81 @@ pub(crate) fn labels_of_config(
     config.config().as_ref().and_then(|c| c.labels().as_ref())
 }
 
+/// Convert between a subset of ostree-ext metadata and the exposed spec API.
+fn create_imagestatus(
+    image: ImageReference,
+    manifest_digest: &Digest,
+    config: &ImageConfiguration,
+) -> ImageStatus {
+    let labels = labels_of_config(config);
+    let timestamp = labels
+        .and_then(|l| {
+            l.get(oci_spec::image::ANNOTATION_CREATED)
+                .map(|s| s.as_str())
+        })
+        .or_else(|| config.created().as_deref())
+        .and_then(bootc_utils::try_deserialize_timestamp);
+
+    let version = ostree_container::version_for_config(config).map(ToOwned::to_owned);
+    let architecture = config.architecture().to_string();
+    ImageStatus {
+        image,
+        version,
+        timestamp,
+        image_digest: manifest_digest.to_string(),
+        architecture,
+    }
+}
+
+fn imagestatus(
+    sysroot: &SysrootLock,
+    deployment: &ostree::Deployment,
+    image: ostree_container::OstreeImageReference,
+) -> Result<CachedImageStatus> {
+    let repo = &sysroot.repo();
+    let imgstate = ostree_container::store::query_image_commit(repo, &deployment.csum())?;
+    let image = ImageReference::from(image);
+    let cached = imgstate
+        .cached_update
+        .map(|cached| create_imagestatus(image.clone(), &cached.manifest_digest, &cached.config));
+    let imagestatus = create_imagestatus(image, &imgstate.manifest_digest, &imgstate.configuration);
+
+    Ok(CachedImageStatus {
+        image: Some(imagestatus),
+        cached_update: cached,
+    })
+}
+
 /// Given an OSTree deployment, parse out metadata into our spec.
 #[context("Reading deployment metadata")]
 fn boot_entry_from_deployment(
     sysroot: &Storage,
     deployment: &ostree::Deployment,
 ) -> Result<BootEntry> {
     let (
-        store,
         CachedImageStatus {
             image,
             cached_update,
         },
         incompatible,
     ) = if let Some(origin) = deployment.origin().as_ref() {
         let incompatible = crate::utils::origin_has_rpmostree_stuff(origin);
-        let (store, cached_imagestatus) = if incompatible {
+        let cached_imagestatus = if incompatible {
             // If there are local changes, we can't represent it as a bootc compatible image.
-            (None, CachedImageStatus::default())
+            CachedImageStatus::default()
         } else if let Some(image) = get_image_origin(origin)? {
-            let store = deployment.store()?;
-            let store = store.as_ref().unwrap_or(&sysroot.store);
-            let spec = Some(store.spec());
-            let status = store.imagestatus(sysroot, deployment, image)?;
-
-            (spec, status)
+            imagestatus(sysroot, deployment, image)?
         } else {
             // The deployment isn't using a container image
-            (None, CachedImageStatus::default())
+            CachedImageStatus::default()
         };
-        (store, cached_imagestatus, incompatible)
+        (cached_imagestatus, incompatible)
     } else {
         // The deployment has no origin at all (this generally shouldn't happen)
-        (None, CachedImageStatus::default(), false)
+        (CachedImageStatus::default(), false)
     };
 
+    let store = Some(crate::spec::Store::OstreeContainer);
     let r = BootEntry {
         image,
         cached_update,

crates/lib/src/store/mod.rs

@@ -1,27 +1,21 @@
 use std::cell::OnceCell;
-use std::env;
 use std::ops::Deref;
 use std::sync::Arc;
 
 use anyhow::{Context, Result};
 use cap_std_ext::cap_std;
 use cap_std_ext::cap_std::fs::{Dir, DirBuilder, DirBuilderExt as _};
 use cap_std_ext::dirext::CapStdExtDirExt;
-use clap::ValueEnum;
 use fn_error_context::context;
 
-use ostree_ext::container::OstreeImageReference;
-use ostree_ext::keyfileext::KeyFileExt;
+use composefs;
 use ostree_ext::sysroot::SysrootLock;
-use ostree_ext::{composefs, ostree};
 use rustix::fs::Mode;
 
 use crate::lsm;
 use crate::spec::ImageStatus;
 use crate::utils::deployment_fd;
 
-mod ostree_container;
-
 /// See https://github.com/containers/composefs-rs/issues/159
 pub type ComposefsRepository =
     composefs::repository::Repository<composefs::fsverity::Sha512HashValue>;
@@ -50,10 +44,6 @@ pub(crate) struct Storage {
 
     /// Our runtime state
     run: Dir,
-
-    /// This is a stub abstraction that tries to hide ostree
-    /// that we aren't really using right now
-    pub store: Box<dyn ContainerImageStoreImpl>,
 }
 
 #[derive(Default)]
@@ -62,21 +52,6 @@ pub(crate) struct CachedImageStatus {
     pub cached_update: Option<ImageStatus>,
 }
 
-pub(crate) trait ContainerImageStore {
-    fn store(&self) -> Result<Option<Box<dyn ContainerImageStoreImpl>>>;
-}
-
-pub(crate) trait ContainerImageStoreImpl {
-    fn spec(&self) -> crate::spec::Store;
-
-    fn imagestatus(
-        &self,
-        sysroot: &SysrootLock,
-        deployment: &ostree::Deployment,
-        image: OstreeImageReference,
-    ) -> Result<CachedImageStatus>;
-}
-
 impl Deref for Storage {
     type Target = SysrootLock;
 
@@ -88,15 +63,6 @@ impl Deref for Storage {
 impl Storage {
     pub fn new(sysroot: SysrootLock, run: &Dir) -> Result<Self> {
         let run = run.try_clone()?;
-        let store = match env::var("BOOTC_STORAGE") {
-            Ok(val) => crate::spec::Store::from_str(&val, true).unwrap_or_else(|_| {
-                let default = crate::spec::Store::default();
-                tracing::warn!("Unknown BOOTC_STORAGE option {val}, falling back to {default:?}");
-                default
-            }),
-            Err(_) => crate::spec::Store::default(),
-        };
-        let store = load(store);
 
         // ostree has historically always relied on
         // having ostree -> sysroot/ostree as a symlink in the image to
@@ -119,7 +85,6 @@ impl Storage {
             sysroot,
             run,
             composefs: Default::default(),
-            store,
             imgstore: Default::default(),
         })
     }
@@ -190,25 +155,3 @@ impl Storage {
             .map_err(Into::into)
     }
 }
-
-impl ContainerImageStore for ostree::Deployment {
-    fn store<'a>(&self) -> Result<Option<Box<dyn ContainerImageStoreImpl>>> {
-        if let Some(origin) = self.origin().as_ref() {
-            if let Some(store) = origin.optional_string("bootc", "backend")? {
-                let store =
-                    crate::spec::Store::from_str(&store, true).map_err(anyhow::Error::msg)?;
-                Ok(Some(load(store)))
-            } else {
-                Ok(None)
-            }
-        } else {
-            Ok(None)
-        }
-    }
-}
-
-pub(crate) fn load(ty: crate::spec::Store) -> Box<dyn ContainerImageStoreImpl> {
-    match ty {
-        crate::spec::Store::OstreeContainer => Box::new(ostree_container::OstreeContainerStore),
-    }
-}