From 1770055f2b724bd8f93f509c3bce2b7579a25206 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 24 Oct 2024 08:03:00 -0400 Subject: [PATCH] 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. Signed-off-by: Colin Walters --- ostree-ext/src/container/store.rs | 121 +++++++++----- .../src/container/update_detachedmeta.rs | 3 +- ostree-ext/src/fixture.rs | 154 ++++++++++++++++-- ostree-ext/tests/it/main.rs | 25 ++- 4 files changed, 242 insertions(+), 61 deletions(-) diff --git a/ostree-ext/src/container/store.rs b/ostree-ext/src/container/store.rs index c715f8ca4..2d9efafd8 100644 --- a/ostree-ext/src/container/store.rs +++ b/ostree-ext/src/container/store.rs @@ -227,7 +227,7 @@ pub struct PreparedImport { /// The layers containing split objects pub ostree_layers: Vec, /// The layer for the ostree commit. - pub ostree_commit_layer: ManifestLayerState, + pub ostree_commit_layer: Option, /// Any further non-ostree (derived) layers. pub layers: Vec, } @@ -235,7 +235,8 @@ pub struct PreparedImport { 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 { - 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::>>()?; 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() { @@ -755,10 +777,10 @@ 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?; } @@ -766,14 +788,14 @@ impl ImageImporter { &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))?; @@ -792,10 +814,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?; } @@ -818,11 +840,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, @@ -844,20 +867,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::().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}"); @@ -888,7 +914,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, @@ -975,14 +1001,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; @@ -1008,10 +1036,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(); @@ -1303,6 +1333,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)?; @@ -1768,10 +1799,12 @@ pub(crate) fn verify_container_image( .0 .downcast::() .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(), diff --git a/ostree-ext/src/container/update_detachedmeta.rs b/ostree-ext/src/container/update_detachedmeta.rs index 2803fab1c..f637d53a0 100644 --- a/ostree-ext/src/container/update_detachedmeta.rs +++ b/ostree-ext/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() diff --git a/ostree-ext/src/fixture.rs b/ostree-ext/src/fixture.rs index 99a2e3841..790b54a53 100644 --- a/ostree-ext/src/fixture.rs +++ b/ostree-ext/src/fixture.rs @@ -21,11 +21,12 @@ use gvariant::aligned_bytes::TryAsAligned; use gvariant::{Marker, Structure}; use io_lifetimes::AsFd; use ocidir::cap_std::fs::{DirBuilder, DirBuilderExt as _}; +use ocidir::oci_spec::image::ImageConfigurationBuilder; use once_cell::sync::Lazy; use regex::Regex; use std::borrow::Cow; use std::fmt::Write as _; -use std::io::Write; +use std::io::{self, Write}; use std::ops::Add; use std::process::{Command, Stdio}; use std::rc::Rc; @@ -129,6 +130,33 @@ impl FileDef { } }) } + + pub fn append_tar(&self, w: &mut tar::Builder) -> Result<()> { + let mut h = tar::Header::new_ustar(); + h.set_mtime(0); + h.set_uid(self.uid.into()); + h.set_gid(self.gid.into()); + h.set_mode(self.mode.into()); + match &self.ty { + FileDefType::Regular(data) => { + let data = data.as_bytes(); + h.set_entry_type(tar::EntryType::Regular); + h.set_size(data.len().try_into().unwrap()); + w.append_data(&mut h, &*self.path, std::io::Cursor::new(data))?; + } + FileDefType::Symlink(target) => { + h.set_entry_type(tar::EntryType::Symlink); + h.set_size(0); + w.append_link(&mut h, &*self.path, target.as_std_path())?; + } + FileDefType::Directory => { + h.set_entry_type(tar::EntryType::Directory); + h.set_size(0); + w.append_data(&mut h, &*self.path, std::io::empty())?; + } + } + Ok(()) + } } /// This is like a package database, mapping our test fixture files to package names @@ -470,6 +498,14 @@ pub fn assert_commits_filenames_equal( similar_asserts::assert_eq!(a_contents_buf, b_contents_buf); } +fn clear_ostree_repo(repo: &ostree::Repo) -> Result<()> { + for (r, _) in repo.list_refs(None, gio::Cancellable::NONE)? { + repo.set_ref_immediate(None, &r, None, gio::Cancellable::NONE)?; + } + repo.prune(ostree::RepoPruneFlags::REFS_ONLY, 0, gio::Cancellable::NONE)?; + Ok(()) +} + #[derive(Debug)] pub struct Fixture { // Just holds a reference @@ -576,15 +612,7 @@ impl Fixture { // Delete all objects in the destrepo pub fn clear_destrepo(&self) -> Result<()> { - self.destrepo() - .set_ref_immediate(None, self.testref(), None, gio::Cancellable::NONE)?; - for (r, _) in self.destrepo().list_refs(None, gio::Cancellable::NONE)? { - self.destrepo() - .set_ref_immediate(None, &r, None, gio::Cancellable::NONE)?; - } - self.destrepo() - .prune(ostree::RepoPruneFlags::REFS_ONLY, 0, gio::Cancellable::NONE)?; - Ok(()) + clear_ostree_repo(self.destrepo()) } #[context("Writing filedef {}", def.path.as_str())] @@ -864,3 +892,109 @@ impl Fixture { Ok(()) } } + +#[derive(Debug)] +pub struct NonOstreeFixture { + // Just holds a reference + _tempdir: tempfile::TempDir, + pub dir: Arc, + pub path: Utf8PathBuf, + src_oci: ocidir::OciDir, + destrepo: ostree::Repo, + + pub bootable: bool, +} + +impl NonOstreeFixture { + const SRCOCI: &str = "src/oci"; + + #[context("Initializing fixture")] + pub fn new_base() -> Result { + // Basic setup, allocate a tempdir + let tempdir = tempfile::tempdir_in("/var/tmp")?; + let dir = Arc::new(cap_std::fs::Dir::open_ambient_dir( + tempdir.path(), + cap_std::ambient_authority(), + )?); + let path: &Utf8Path = tempdir.path().try_into().unwrap(); + let path = path.to_path_buf(); + + // Create the src/ directory + dir.create_dir_all(Self::SRCOCI)?; + let src_oci = dir.open_dir(Self::SRCOCI)?; + let src_oci = ocidir::OciDir::ensure(&src_oci)?; + + dir.create_dir("dest")?; + let destrepo = ostree::Repo::create_at_dir( + dir.as_fd(), + "dest/repo", + ostree::RepoMode::BareUser, + None, + )?; + Ok(Self { + _tempdir: tempdir, + dir, + path, + src_oci, + destrepo, + bootable: true, + }) + } + + pub fn destrepo(&self) -> &ostree::Repo { + &self.destrepo + } + + #[context("Exporting container")] + pub async fn export_container(&self) -> Result<(ImageReference, oci_image::Digest)> { + let imgref = ImageReference { + transport: Transport::OciDir, + name: self.path.join(Self::SRCOCI).to_string(), + }; + + let mut config = ImageConfigurationBuilder::default().build().unwrap(); + let mut manifest = ocidir::new_empty_manifest().build().unwrap(); + + let bw = self.src_oci.create_gzip_layer(None)?; + let mut bw = tar::Builder::new(bw); + for def in FileDef::iter_from(CONTENTS_V0) { + let def = def.unwrap(); + def.append_tar(&mut bw)?; + } + let bw = bw.into_inner()?; + let new_layer = bw.complete()?; + + self.src_oci + .push_layer(&mut manifest, &mut config, new_layer, "root", None); + let config = self.src_oci.write_config(config)?; + + manifest.set_config(config); + self.src_oci + .replace_with_single_manifest(manifest, oci_image::Platform::default())?; + let idx = self.src_oci.read_index()?.unwrap(); + let descriptor = idx.manifests().first().unwrap(); + + Ok((imgref, descriptor.digest().to_owned())) + } + + /// Given the input image reference, import it into destrepo using the default + /// import config. The image must not exist already in the store. + pub async fn must_import(&self, imgref: &ImageReference) -> Result> { + let ostree_imgref = crate::container::OstreeImageReference { + sigverify: crate::container::SignatureSource::ContainerPolicyAllowInsecure, + imgref: imgref.clone(), + }; + let mut imp = + store::ImageImporter::new(self.destrepo(), &ostree_imgref, Default::default()) + .await + .unwrap(); + assert!(store::query_image(self.destrepo(), &imgref) + .unwrap() + .is_none()); + let prep = match imp.prepare().await.context("Init prep derived")? { + store::PrepareResult::AlreadyPresent(_) => panic!("should not be already imported"), + store::PrepareResult::Ready(r) => r, + }; + imp.import(prep).await + } +} diff --git a/ostree-ext/tests/it/main.rs b/ostree-ext/tests/it/main.rs index e7576409d..a1fd8138b 100644 --- a/ostree-ext/tests/it/main.rs +++ b/ostree-ext/tests/it/main.rs @@ -23,7 +23,9 @@ use std::process::Command; use std::time::SystemTime; use xshell::cmd; -use ostree_ext::fixture::{FileDef, Fixture, CONTENTS_CHECKSUM_V0, LAYERS_V0_LEN, PKGS_V0_LEN}; +use ostree_ext::fixture::{ + FileDef, Fixture, NonOstreeFixture, CONTENTS_CHECKSUM_V0, LAYERS_V0_LEN, PKGS_V0_LEN, +}; const EXAMPLE_TAR_LAYER: &[u8] = include_bytes!("fixtures/hlinks.tar.gz"); const TEST_REGISTRY_DEFAULT: &str = "localhost:5000"; @@ -875,7 +877,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() { @@ -952,7 +954,7 @@ r usr/bin/bash bash-v0 let to_fetch = prep.layers_to_fetch().collect::>>()?; 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()); @@ -995,7 +997,7 @@ r usr/bin/bash bash-v0 }; let to_fetch = prep.layers_to_fetch().collect::>>()?; 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 @@ -1251,6 +1253,17 @@ async fn test_container_etc_hardlinked(absolute: bool) -> Result<()> { Ok(()) } +#[tokio::test] +async fn test_non_ostree() -> Result<()> { + let fixture = NonOstreeFixture::new_base()?; + let (src_imgref, src_digest) = fixture.export_container().await?; + + let imgref = fixture.export_container().await.unwrap().0; + let imp = fixture.must_import(&imgref).await?; + assert_eq!(imp.manifest_digest, src_digest); + Ok(()) +} + /// Copy an OCI directory. async fn oci_clone(src: impl AsRef, dest: impl AsRef) -> Result<()> { let src = src.as_ref(); @@ -1357,7 +1370,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()); @@ -1430,7 +1443,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() {