store: Support importing images without /ostree

A sticking point keeping ostree in the picture here for
containers was SELinux handling. When we started this
effort I'd feared rewriting.

But recently we changed things such that we label derived images
using the policy from the final root.

This is a relatively small change in code size and complexity,
that allows us to import images that don't have "ostree stuff"
in them at all, i.e. there's no `/ostree/repo/objects`.

The advantage here is that this significantly simplifies
constructing base images.

The main disadvantage today for people who build images
this way is that we end up re-labeling and re-checksumming all objects.

But, the real fix for that in the future will be for us to
rework things such that we support `security.selinux` for example
as native xattrs in the tar stream.

Author: cgwalters

lib/src/container/store.rs

@@ -227,15 +227,16 @@ pub struct PreparedImport {
     /// The layers containing split objects
     pub ostree_layers: Vec<ManifestLayerState>,
     /// The layer for the ostree commit.
-    pub ostree_commit_layer: ManifestLayerState,
+    pub ostree_commit_layer: Option<ManifestLayerState>,
     /// Any further non-ostree (derived) layers.
     pub layers: Vec<ManifestLayerState>,
 }
 
 impl PreparedImport {
     /// Iterate over all layers; the commit layer, the ostree split object layers, and any non-ostree layers.
     pub fn all_layers(&self) -> impl Iterator<Item = &ManifestLayerState> {
-        std::iter::once(&self.ostree_commit_layer)
+        self.ostree_commit_layer
+            .iter()
             .chain(self.ostree_layers.iter())
             .chain(self.layers.iter())
     }
@@ -376,21 +377,20 @@ fn layer_from_diffid<'a>(
 pub(crate) fn parse_manifest_layout<'a>(
     manifest: &'a ImageManifest,
     config: &ImageConfiguration,
-) -> Result<(&'a Descriptor, Vec<&'a Descriptor>, Vec<&'a Descriptor>)> {
+) -> Result<(
+    Option<&'a Descriptor>,
+    Vec<&'a Descriptor>,
+    Vec<&'a Descriptor>,
+)> {
     let config_labels = super::labels_of(config);
 
     let first_layer = manifest
         .layers()
         .first()
         .ok_or_else(|| anyhow!("No layers in manifest"))?;
-    let target_diffid = config_labels
-        .and_then(|labels| labels.get(DIFFID_LABEL))
-        .ok_or_else(|| {
-            anyhow!(
-                "No {} label found, not an ostree encapsulated container",
-                DIFFID_LABEL
-            )
-        })?;
+    let Some(target_diffid) = config_labels.and_then(|labels| labels.get(DIFFID_LABEL)) else {
+        return Ok((None, Vec::new(), manifest.layers().iter().collect()));
+    };
 
     let target_layer = layer_from_diffid(manifest, config, target_diffid.as_str())?;
     let mut chunk_layers = Vec::new();
@@ -416,7 +416,20 @@ pub(crate) fn parse_manifest_layout<'a>(
         }
     }
 
-    Ok((ostree_layer, chunk_layers, derived_layers))
+    Ok((Some(ostree_layer), chunk_layers, derived_layers))
+}
+
+/// Like [`parse_manifest_layout`] but requires the image has an ostree base.
+#[context("Parsing manifest layout")]
+pub(crate) fn parse_ostree_manifest_layout<'a>(
+    manifest: &'a ImageManifest,
+    config: &ImageConfiguration,
+) -> Result<(&'a Descriptor, Vec<&'a Descriptor>, Vec<&'a Descriptor>)> {
+    let (ostree_layer, component_layers, derived_layers) = parse_manifest_layout(manifest, config)?;
+    let ostree_layer = ostree_layer.ok_or_else(|| {
+        anyhow!("No {DIFFID_LABEL} label found, not an ostree encapsulated container")
+    })?;
+    Ok((ostree_layer, component_layers, derived_layers))
 }
 
 /// Find the timestamp of the manifest (or config), ignoring errors.
@@ -601,10 +614,10 @@ impl ImageImporter {
             parse_manifest_layout(&manifest, &config)?;
 
         let query = |l: &Descriptor| query_layer(&self.repo, l.clone());
-        let commit_layer = query(commit_layer)?;
+        let commit_layer = commit_layer.map(query).transpose()?;
         let component_layers = component_layers
             .into_iter()
-            .map(query)
+            .map(|l| query(l))
             .collect::<Result<Vec<_>>>()?;
         let remaining_layers = remaining_layers
             .into_iter()
@@ -693,6 +706,7 @@ impl ImageImporter {
     pub(crate) async fn unencapsulate_base(
         &mut self,
         import: &mut store::PreparedImport,
+        require_ostree: bool,
         write_refs: bool,
     ) -> Result<()> {
         tracing::debug!("Fetching base");
@@ -707,6 +721,14 @@ impl ImageImporter {
                 None
             }
         };
+        let Some(commit_layer) = import.ostree_commit_layer.as_mut() else {
+            if require_ostree {
+                anyhow::bail!(
+                    "No {DIFFID_LABEL} label found, not an ostree encapsulated container"
+                );
+            }
+            return Ok(());
+        };
         let des_layers = self.proxy.get_layer_info(&self.proxy_img).await?;
         for layer in import.ostree_layers.iter_mut() {
             if layer.commit.is_some() {
@@ -754,31 +776,31 @@ impl ImageImporter {
                     .await?;
             }
         }
-        if import.ostree_commit_layer.commit.is_none() {
+        if commit_layer.commit.is_none() {
             if let Some(p) = self.layer_progress.as_ref() {
                 p.send(ImportProgress::OstreeChunkStarted(
-                    import.ostree_commit_layer.layer.clone(),
+                    commit_layer.layer.clone(),
                 ))
                 .await?;
             }
             let (blob, driver) = fetch_layer_decompress(
                 &self.proxy,
                 &self.proxy_img,
                 &import.manifest,
-                &import.ostree_commit_layer.layer,
+                &commit_layer.layer,
                 self.layer_byte_progress.as_ref(),
                 des_layers.as_ref(),
                 self.imgref.imgref.transport,
             )
             .await?;
             let repo = self.repo.clone();
-            let target_ref = import.ostree_commit_layer.ostree_ref.clone();
+            let target_ref = commit_layer.ostree_ref.clone();
             let import_task =
                 crate::tokio_util::spawn_blocking_cancellable_flatten(move |cancellable| {
                     let txn = repo.auto_transaction(Some(cancellable))?;
                     let mut importer = crate::tar::Importer::new_for_commit(&repo, remote);
-                    let blob = tokio_util::io::SyncIoBridge::new(blob);
-                    let mut archive = tar::Archive::new(blob);
+                    let mut blob = tokio_util::io::SyncIoBridge::new(blob);
+                    let mut archive = tar::Archive::new(&mut blob);
                     importer.import_commit(&mut archive, Some(cancellable))?;
                     let commit = importer.finish_import_commit();
                     if write_refs {
@@ -790,10 +812,10 @@ impl ImageImporter {
                     Ok::<_, anyhow::Error>(commit)
                 });
             let commit = super::unencapsulate::join_fetch(import_task, driver).await?;
-            import.ostree_commit_layer.commit = Some(commit);
+            commit_layer.commit = Some(commit);
             if let Some(p) = self.layer_progress.as_ref() {
                 p.send(ImportProgress::OstreeChunkCompleted(
-                    import.ostree_commit_layer.layer.clone(),
+                    commit_layer.layer.clone(),
                 ))
                 .await?;
             }
@@ -816,11 +838,12 @@ impl ImageImporter {
             anyhow::bail!("Image has {} non-ostree layers", prep.layers.len());
         }
         let deprecated_warning = prep.deprecated_warning().map(ToOwned::to_owned);
-        self.unencapsulate_base(&mut prep, false).await?;
+        self.unencapsulate_base(&mut prep, true, false).await?;
         // TODO change the imageproxy API to ensure this happens automatically when
         // the image reference is dropped
         self.proxy.close_image(&self.proxy_img).await?;
-        let ostree_commit = prep.ostree_commit_layer.commit.unwrap();
+        // SAFETY: We know we have a commit
+        let ostree_commit = prep.ostree_commit_layer.unwrap().commit.unwrap();
         let image_digest = prep.manifest_digest;
         Ok(Import {
             ostree_commit,
@@ -842,20 +865,23 @@ impl ImageImporter {
         }
         // First download all layers for the base image (if necessary) - we need the SELinux policy
         // there to label all following layers.
-        self.unencapsulate_base(&mut import, true).await?;
+        self.unencapsulate_base(&mut import, false, true).await?;
         let des_layers = self.proxy.get_layer_info(&self.proxy_img).await?;
         let proxy = self.proxy;
         let proxy_img = self.proxy_img;
         let target_imgref = self.target_imgref.as_ref().unwrap_or(&self.imgref);
-        let base_commit = import.ostree_commit_layer.commit.clone().unwrap();
+        let base_commit = import
+            .ostree_commit_layer
+            .as_ref()
+            .map(|c| c.commit.clone().unwrap());
 
-        let root_is_transient = {
-            let rootf = self
-                .repo
-                .read_commit(&base_commit, gio::Cancellable::NONE)?
-                .0;
+        let root_is_transient = if let Some(base) = base_commit.as_ref() {
+            let rootf = self.repo.read_commit(&base, gio::Cancellable::NONE)?.0;
             let rootf = rootf.downcast_ref::<ostree::RepoFile>().unwrap();
             crate::ostree_prepareroot::overlayfs_root_enabled(rootf)?
+        } else {
+            // For generic images we assume they're using composefs
+            true
         };
         tracing::debug!("Base rootfs is transient: {root_is_transient}");
 
@@ -886,7 +912,7 @@ impl ImageImporter {
                 // An important aspect of this is that we SELinux label the derived layers using
                 // the base policy.
                 let opts = crate::tar::WriteTarOptions {
-                    base: Some(base_commit.clone()),
+                    base: base_commit.clone(),
                     selinux: true,
                     allow_nonusr: root_is_transient,
                     retain_var: self.ostree_v2024_3,
@@ -968,14 +994,16 @@ impl ImageImporter {
                     process_whiteouts: false,
                     ..Default::default()
                 };
-                repo.checkout_at(
-                    Some(&checkout_opts),
-                    (*td).as_raw_fd(),
-                    rootpath,
-                    &base_commit,
-                    cancellable,
-                )
-                .context("Checking out base commit")?;
+                if let Some(base) = base_commit.as_ref() {
+                    repo.checkout_at(
+                        Some(&checkout_opts),
+                        (*td).as_raw_fd(),
+                        rootpath,
+                        &base,
+                        cancellable,
+                    )
+                    .context("Checking out base commit")?;
+                }
 
                 // Layer all subsequent commits
                 checkout_opts.process_whiteouts = true;
@@ -1001,10 +1029,12 @@ impl ImageImporter {
                     let sepolicy = ostree::SePolicy::new_at(rootpath.as_raw_fd(), cancellable)?;
                     tracing::debug!("labeling from merged tree");
                     modifier.set_sepolicy(Some(&sepolicy));
-                } else {
+                } else if let Some(base) = base_commit.as_ref() {
                     tracing::debug!("labeling from base tree");
                     // TODO: We can likely drop this; we know all labels should be pre-computed.
-                    modifier.set_sepolicy_from_commit(repo, &base_commit, cancellable)?;
+                    modifier.set_sepolicy_from_commit(repo, &base, cancellable)?;
+                } else {
+                    unreachable!()
                 }
 
                 let mt = ostree::MutableTree::new();
@@ -1296,6 +1326,7 @@ pub(crate) fn export_to_oci(
     let srcinfo = query_image(repo, imgref)?.ok_or_else(|| anyhow!("No such image"))?;
     let (commit_layer, component_layers, remaining_layers) =
         parse_manifest_layout(&srcinfo.manifest, &srcinfo.configuration)?;
+    let commit_layer = commit_layer.ok_or_else(|| anyhow!("Missing {DIFFID_LABEL}"))?;
     let commit_chunk_ref = ref_for_layer(commit_layer)?;
     let commit_chunk_rev = repo.require_rev(&commit_chunk_ref)?;
     let mut chunking = chunking::Chunking::new(repo, &commit_chunk_rev)?;
@@ -1761,10 +1792,12 @@ pub(crate) fn verify_container_image(
         .0
         .downcast::<ostree::RepoFile>()
         .expect("downcast");
-    println!(
-        "Verifying with base ostree layer {}",
-        ref_for_layer(commit_layer)?
-    );
+    if let Some(commit_layer) = commit_layer {
+        println!(
+            "Verifying with base ostree layer {}",
+            ref_for_layer(commit_layer)?
+        );
+    }
     compare_commit_trees(
         repo,
         "/".into(),

lib/src/container/update_detachedmeta.rs

@@ -70,7 +70,8 @@ pub async fn update_detached_metadata(
             .ok_or_else(|| anyhow!("Image is missing container configuration"))?;
 
         // Find the OSTree commit layer we want to replace
-        let (commit_layer, _, _) = container_store::parse_manifest_layout(&manifest, &config)?;
+        let (commit_layer, _, _) =
+            container_store::parse_ostree_manifest_layout(&manifest, &config)?;
         let commit_layer_idx = manifest
             .layers()
             .iter()

lib/tests/it/main.rs

@@ -859,7 +859,7 @@ async fn test_container_chunked() -> Result<()> {
     assert!(prep.deprecated_warning().is_none());
     assert_eq!(prep.version(), Some("42.0"));
     let digest = prep.manifest_digest.clone();
-    assert!(prep.ostree_commit_layer.commit.is_none());
+    assert!(prep.ostree_commit_layer.as_ref().unwrap().commit.is_none());
     assert_eq!(prep.ostree_layers.len(), nlayers);
     assert_eq!(prep.layers.len(), 0);
     for layer in prep.layers.iter() {
@@ -936,7 +936,7 @@ r usr/bin/bash bash-v0
     let to_fetch = prep.layers_to_fetch().collect::<Result<Vec<_>>>()?;
     assert_eq!(to_fetch.len(), 2);
     assert_eq!(expected_digest, prep.manifest_digest);
-    assert!(prep.ostree_commit_layer.commit.is_none());
+    assert!(prep.ostree_commit_layer.as_ref().unwrap().commit.is_none());
     assert_eq!(prep.ostree_layers.len(), nlayers);
     let (first, second) = (to_fetch[0], to_fetch[1]);
     assert!(first.0.commit.is_none());
@@ -979,7 +979,7 @@ r usr/bin/bash bash-v0
     };
     let to_fetch = prep.layers_to_fetch().collect::<Result<Vec<_>>>()?;
     assert_eq!(to_fetch.len(), 1);
-    assert!(prep.ostree_commit_layer.commit.is_some());
+    assert!(prep.ostree_commit_layer.as_ref().unwrap().commit.is_some());
     assert_eq!(prep.ostree_layers.len(), nlayers);
 
     // We want to test explicit layer pruning
@@ -1314,7 +1314,7 @@ async fn test_container_write_derive() -> Result<()> {
         store::PrepareResult::Ready(r) => r,
     };
     let expected_digest = prep.manifest_digest.clone();
-    assert!(prep.ostree_commit_layer.commit.is_none());
+    assert!(prep.ostree_commit_layer.as_ref().unwrap().commit.is_none());
     assert_eq!(prep.layers.len(), 1);
     for layer in prep.layers.iter() {
         assert!(layer.commit.is_none());
@@ -1387,7 +1387,7 @@ async fn test_container_write_derive() -> Result<()> {
         store::PrepareResult::Ready(r) => r,
     };
     // We *should* already have the base layer.
-    assert!(prep.ostree_commit_layer.commit.is_some());
+    assert!(prep.ostree_commit_layer.as_ref().unwrap().commit.is_some());
     // One new layer
     assert_eq!(prep.layers.len(), 1);
     for layer in prep.layers.iter() {