From 3c9712fa09ed3980c6ab6ec904cb45882e84ea9b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 14 Aug 2023 13:00:16 -0400 Subject: [PATCH] container: Make requiring bootable field be opt-in This fixes a regression introduced by https://github.com/ostreedev/ostree-rs-ext/commit/9c4a75b3778a3f2fdece095f8f5f7a6289ab512dLooks Brought up in https://github.com/ostreedev/ostree/discussions/2978 Basically we need to make this opt-in at higher levels because encapsulating a non-bootable commit (as well as a commit that historically doesn't have that label) must be supported. --- lib/src/container/deploy.rs | 1 + lib/src/container/store.rs | 22 ++++++++++++---- lib/src/fixture.rs | 8 ++++-- lib/tests/it/main.rs | 51 +++++++++++++++++++++++++++++++++++++ 4 files changed, 75 insertions(+), 7 deletions(-) diff --git a/lib/src/container/deploy.rs b/lib/src/container/deploy.rs index 24651cf0..49a82965 100644 --- a/lib/src/container/deploy.rs +++ b/lib/src/container/deploy.rs @@ -60,6 +60,7 @@ pub async fn deploy( let mut imp = super::store::ImageImporter::new(repo, imgref, options.proxy_cfg.unwrap_or_default()) .await?; + imp.require_bootable(); if let Some(target) = options.target_imgref { imp.set_target(target); } diff --git a/lib/src/container/store.rs b/lib/src/container/store.rs index 9515eb62..e6cb56ab 100644 --- a/lib/src/container/store.rs +++ b/lib/src/container/store.rs @@ -148,6 +148,8 @@ pub struct ImageImporter { target_imgref: Option, no_imgref: bool, // If true, do not write final image ref disable_gc: bool, // If true, don't prune unused image layers + /// If true, require the image has the bootable flag + require_bootable: bool, pub(crate) proxy_img: OpenedImage, layer_progress: Option>, @@ -349,11 +351,6 @@ pub(crate) fn parse_manifest_layout<'a>( config: &ImageConfiguration, ) -> Result<(&'a Descriptor, Vec<&'a Descriptor>, Vec<&'a Descriptor>)> { let config_labels = super::labels_of(config); - let bootable_key = *ostree::METADATA_KEY_BOOTABLE; - let bootable = config_labels.map_or(false, |l| l.contains_key(bootable_key)); - if !bootable { - anyhow::bail!("Target image does not have {bootable_key} label"); - } let first_layer = manifest .layers() @@ -470,6 +467,7 @@ impl ImageImporter { target_imgref: None, no_imgref: false, disable_gc: false, + require_bootable: false, imgref: imgref.clone(), layer_progress: None, layer_byte_progress: None, @@ -488,6 +486,11 @@ impl ImageImporter { self.no_imgref = true; } + /// Require that the image has the bootable metadata field + pub fn require_bootable(&mut self) { + self.require_bootable = true; + } + /// Do not prune image layers. pub fn disable_gc(&mut self) { self.disable_gc = true; @@ -555,6 +558,15 @@ impl ImageImporter { }; let config = self.proxy.fetch_config(&self.proxy_img).await?; + let config_labels = super::labels_of(&config); + + if self.require_bootable { + let bootable_key = *ostree::METADATA_KEY_BOOTABLE; + let bootable = config_labels.map_or(false, |l| l.contains_key(bootable_key)); + if !bootable { + anyhow::bail!("Target image does not have {bootable_key} label"); + } + } let (commit_layer, component_layers, remaining_layers) = parse_manifest_layout(&manifest, &config)?; diff --git a/lib/src/fixture.rs b/lib/src/fixture.rs index fbf649e1..5d66efde 100644 --- a/lib/src/fixture.rs +++ b/lib/src/fixture.rs @@ -140,7 +140,7 @@ static OWNERS: Lazy> = Lazy::new(|| { .collect() }); -static CONTENTS_V0: &str = indoc::indoc! { r##" +pub static CONTENTS_V0: &str = indoc::indoc! { r##" r usr/lib/modules/5.10.18-200.x86_64/vmlinuz this-is-a-kernel r usr/lib/modules/5.10.18-200.x86_64/initramfs this-is-an-initramfs m 0 0 755 @@ -361,6 +361,7 @@ pub struct Fixture { destrepo: ostree::Repo, pub selinux: bool, + pub bootable: bool, } impl Fixture { @@ -407,6 +408,7 @@ impl Fixture { srcrepo, destrepo, selinux: true, + bootable: true, }) } @@ -500,7 +502,9 @@ impl Fixture { metadata.insert("ostree.container-cmd", &vec!["/usr/bin/bash"]); metadata.insert("version", &"42.0"); #[allow(clippy::explicit_auto_deref)] - metadata.insert(*ostree::METADATA_KEY_BOOTABLE, &true); + if self.bootable { + metadata.insert(*ostree::METADATA_KEY_BOOTABLE, &true); + } let metadata = metadata.to_variant(); let commit = self.srcrepo.write_commit_with_time( None, diff --git a/lib/tests/it/main.rs b/lib/tests/it/main.rs index 5926b678..1bbbbea1 100644 --- a/lib/tests/it/main.rs +++ b/lib/tests/it/main.rs @@ -625,6 +625,57 @@ async fn impl_test_container_import_export(chunked: bool) -> Result<()> { Ok(()) } +#[tokio::test] +async fn test_unencapsulate_unbootable() -> Result<()> { + let fixture = { + let mut fixture = Fixture::new_base()?; + fixture.bootable = false; + fixture.commit_filedefs(FileDef::iter_from(ostree_ext::fixture::CONTENTS_V0))?; + fixture + }; + let testrev = fixture + .srcrepo() + .require_rev(fixture.testref()) + .context("Failed to resolve ref")?; + let srcoci_path = &fixture.path.join("oci"); + let srcoci_imgref = ImageReference { + transport: Transport::OciDir, + name: srcoci_path.as_str().to_string(), + }; + let srcoci_unverified = OstreeImageReference { + sigverify: SignatureSource::ContainerPolicyAllowInsecure, + imgref: srcoci_imgref.clone(), + }; + + let config = Config::default(); + let _digest = ostree_ext::container::encapsulate( + fixture.srcrepo(), + fixture.testref(), + &config, + None, + None, + None, + &srcoci_imgref, + ) + .await + .context("exporting")?; + assert!(srcoci_path.exists()); + + assert!(fixture + .destrepo() + .resolve_rev(fixture.testref(), true) + .unwrap() + .is_none()); + + let target = ostree_ext::container::unencapsulate(fixture.destrepo(), &srcoci_unverified) + .await + .unwrap(); + + assert_eq!(target.ostree_commit.as_str(), testrev.as_str()); + + Ok(()) +} + /// Parse a chunked container image and validate its structure; particularly fn validate_chunked_structure(oci_path: &Utf8Path) -> Result<()> { use tar::EntryType::Link;