diff --git a/lib/src/cli.rs b/lib/src/cli.rs index f1e62c2d..42bdb60a 100644 --- a/lib/src/cli.rs +++ b/lib/src/cli.rs @@ -849,7 +849,8 @@ async fn testing(opts: &TestingOpts) -> Result<()> { TestingOpts::Run => crate::integrationtest::run_tests(), TestingOpts::RunIMA => crate::integrationtest::test_ima(), TestingOpts::FilterTar => { - crate::tar::filter_tar(std::io::stdin(), std::io::stdout(), false).map(|_| {}) + crate::tar::filter_tar(std::io::stdin(), std::io::stdout(), &Default::default()) + .map(|_| {}) } } } diff --git a/lib/src/container/store.rs b/lib/src/container/store.rs index a2d35972..ca6d1899 100644 --- a/lib/src/container/store.rs +++ b/lib/src/container/store.rs @@ -166,6 +166,8 @@ pub struct ImageImporter { disable_gc: bool, // If true, don't prune unused image layers /// If true, require the image has the bootable flag require_bootable: bool, + /// If true, we have ostree v2024.3 or newer. + ostree_v2024_3: bool, pub(crate) proxy_img: OpenedImage, layer_progress: Option>, @@ -471,6 +473,7 @@ impl ImageImporter { proxy_img, target_imgref: None, no_imgref: false, + ostree_v2024_3: ostree::check_version(2024, 3), disable_gc: false, require_bootable: false, imgref: imgref.clone(), @@ -496,6 +499,11 @@ impl ImageImporter { self.require_bootable = true; } + /// Override the ostree version being targeted + pub fn set_ostree_version(&mut self, year: u32, v: u32) { + self.ostree_v2024_3 = (year > 2024) || (year == 2024 && v >= 3) + } + /// Do not prune image layers. pub fn disable_gc(&mut self) { self.disable_gc = true; @@ -864,6 +872,7 @@ impl ImageImporter { base: Some(base_commit.clone()), selinux: true, allow_nonusr: root_is_transient, + retain_var: self.ostree_v2024_3, }; let r = crate::tar::write_tar(&self.repo, blob, layer.ostree_ref.as_str(), Some(opts)); diff --git a/lib/src/fixture.rs b/lib/src/fixture.rs index 3322d04b..cc001993 100644 --- a/lib/src/fixture.rs +++ b/lib/src/fixture.rs @@ -439,6 +439,10 @@ impl Fixture { 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(()) diff --git a/lib/src/tar/write.rs b/lib/src/tar/write.rs index 62691697..82c887ec 100644 --- a/lib/src/tar/write.rs +++ b/lib/src/tar/write.rs @@ -68,6 +68,9 @@ pub struct WriteTarOptions { pub selinux: bool, /// Allow content not in /usr; this should be paired with ostree rootfs.transient = true pub allow_nonusr: bool, + /// If true, do not move content in /var to /usr/share/factory/var. This should be used + /// with ostree v2024.3 or newer. + pub retain_var: bool, } /// The result of writing a tar stream. @@ -109,10 +112,16 @@ enum NormalizedPathResult<'a> { Normal(Utf8PathBuf), } -fn normalize_validate_path( - path: &Utf8Path, +#[derive(Debug, Clone, PartialEq, Eq, Default)] +pub(crate) struct TarImportConfig { allow_nonusr: bool, -) -> Result> { + remap_factory_var: bool, +} + +fn normalize_validate_path<'a>( + path: &'a Utf8Path, + config: &'_ TarImportConfig, +) -> Result> { // This converts e.g. `foo//bar/./baz` into `foo/bar/baz`. let mut components = path .components() @@ -145,14 +154,18 @@ fn normalize_validate_path( "etc" => { ret.push("usr/etc"); } - // Content in /var will get copied by a systemd tmpfiles.d unit "var" => { - ret.push("usr/share/factory/var"); + // Content in /var will get copied by a systemd tmpfiles.d unit + if config.remap_factory_var { + ret.push("usr/share/factory/var"); + } else { + ret.push(part) + } } o if EXCLUDED_TOPLEVEL_PATHS.contains(&o) => { return Ok(NormalizedPathResult::Filtered(part)); } - _ if allow_nonusr => ret.push(part), + _ if config.allow_nonusr => ret.push(part), _ => { return Ok(NormalizedPathResult::Filtered(part)); } @@ -180,7 +193,7 @@ fn normalize_validate_path( pub(crate) fn filter_tar( src: impl std::io::Read, dest: impl std::io::Write, - allow_nonusr: bool, + config: &TarImportConfig, ) -> Result> { let src = std::io::BufReader::new(src); let mut src = tar::Archive::new(src); @@ -190,7 +203,7 @@ pub(crate) fn filter_tar( let ents = src.entries()?; - tracing::debug!("Filtering tar; allow_nonusr={allow_nonusr}"); + tracing::debug!("Filtering tar; config={config:?}"); // Lookaside data for dealing with hardlinked files into /sysroot; see below. let mut changed_sysroot_objects = HashMap::new(); @@ -259,7 +272,7 @@ pub(crate) fn filter_tar( } } - let normalized = match normalize_validate_path(path, allow_nonusr)? { + let normalized = match normalize_validate_path(path, config)? { NormalizedPathResult::Filtered(path) => { tracing::trace!("Filtered: {path}"); if let Some(v) = filtered.get_mut(path) { @@ -282,15 +295,16 @@ pub(crate) fn filter_tar( async fn filter_tar_async( src: impl AsyncRead + Send + 'static, mut dest: impl AsyncWrite + Send + Unpin, - allow_nonusr: bool, + config: &TarImportConfig, ) -> Result> { let (tx_buf, mut rx_buf) = tokio::io::duplex(8192); // The source must be moved to the heap so we know it is stable for passing to the worker thread let src = Box::pin(src); + let config = config.clone(); let tar_transformer = tokio::task::spawn_blocking(move || { let mut src = tokio_util::io::SyncIoBridge::new(src); let dest = tokio_util::io::SyncIoBridge::new(tx_buf); - let r = filter_tar(&mut src, dest, allow_nonusr); + let r = filter_tar(&mut src, dest, &config); // Pass ownership of the input stream back to the caller - see below. (r, src) }); @@ -365,7 +379,10 @@ pub async fn write_tar( let mut child_stdout = r.stdout.take().unwrap(); let mut child_stderr = r.stderr.take().unwrap(); // Copy the filtered tar stream to child stdin - let filtered_result = filter_tar_async(src, child_stdin, options.allow_nonusr); + let mut import_config = TarImportConfig::default(); + import_config.allow_nonusr = options.allow_nonusr; + import_config.remap_factory_var = !options.retain_var; + let filtered_result = filter_tar_async(src, child_stdin, &import_config); let output_copier = async move { // Gather stdout/stderr to buffers let mut child_stdout_buf = String::new(); @@ -421,6 +438,18 @@ mod tests { #[test] fn test_normalize_path() { + let imp_default = &TarImportConfig { + allow_nonusr: false, + remap_factory_var: true, + }; + let allow_nonusr = &TarImportConfig { + allow_nonusr: true, + remap_factory_var: true, + }; + let composefs_and_new_ostree = &TarImportConfig { + allow_nonusr: true, + remap_factory_var: false, + }; let valid_all = &[ ("/usr/bin/blah", "./usr/bin/blah"), ("usr/bin/blah", "./usr/bin/blah"), @@ -431,8 +460,8 @@ mod tests { ]; let valid_nonusr = &[("boot", "./boot"), ("opt/puppet/blah", "./opt/puppet/blah")]; for &(k, v) in valid_all { - let r = normalize_validate_path(k.into(), false).unwrap(); - let r2 = normalize_validate_path(k.into(), true).unwrap(); + let r = normalize_validate_path(k.into(), imp_default).unwrap(); + let r2 = normalize_validate_path(k.into(), allow_nonusr).unwrap(); assert_eq!(r, r2); match r { NormalizedPathResult::Normal(r) => assert_eq!(r, v), @@ -440,12 +469,12 @@ mod tests { } } for &(k, v) in valid_nonusr { - let strict = normalize_validate_path(k.into(), false).unwrap(); + let strict = normalize_validate_path(k.into(), imp_default).unwrap(); assert!( matches!(strict, NormalizedPathResult::Filtered(_)), "Incorrect filter for {k}" ); - let nonusr = normalize_validate_path(k.into(), true).unwrap(); + let nonusr = normalize_validate_path(k.into(), allow_nonusr).unwrap(); match nonusr { NormalizedPathResult::Normal(r) => assert_eq!(r, v), NormalizedPathResult::Filtered(o) => panic!("Should not have filtered {o}"), @@ -453,7 +482,7 @@ mod tests { } let filtered = &["/run/blah", "/sys/foo", "/dev/somedev"]; for &k in filtered { - match normalize_validate_path(k.into(), true).unwrap() { + match normalize_validate_path(k.into(), imp_default).unwrap() { NormalizedPathResult::Filtered(_) => {} NormalizedPathResult::Normal(_) => { panic!("{} should be filtered", k) @@ -462,9 +491,13 @@ mod tests { } let errs = &["usr/foo/../../bar"]; for &k in errs { - assert!(normalize_validate_path(k.into(), true).is_err()); - assert!(normalize_validate_path(k.into(), false).is_err()); + assert!(normalize_validate_path(k.into(), allow_nonusr).is_err()); + assert!(normalize_validate_path(k.into(), imp_default).is_err()); } + assert!(matches!( + normalize_validate_path("var/lib/foo".into(), composefs_and_new_ostree).unwrap(), + NormalizedPathResult::Normal(_) + )); } #[tokio::test] @@ -481,7 +514,7 @@ mod tests { let _ = rootfs_tar.into_inner()?; let mut dest = Vec::new(); let src = tokio::io::BufReader::new(tokio::fs::File::open(rootfs_tar_path).await?); - filter_tar_async(src, &mut dest, false).await?; + filter_tar_async(src, &mut dest, &Default::default()).await?; let dest = dest.as_slice(); let mut final_tar = tar::Archive::new(Cursor::new(dest)); let destdir = &tempd.path().join("destdir"); diff --git a/lib/tests/it/main.rs b/lib/tests/it/main.rs index 3a0bfdab..2c4380f9 100644 --- a/lib/tests/it/main.rs +++ b/lib/tests/it/main.rs @@ -935,6 +935,7 @@ async fn test_container_var_content() -> Result<()> { }; let mut imp = store::ImageImporter::new(fixture.destrepo(), &derived_imgref, Default::default()).await?; + imp.set_ostree_version(2023, 11); let prep = match imp.prepare().await.unwrap() { store::PrepareResult::AlreadyPresent(_) => panic!("should not be already imported"), store::PrepareResult::Ready(r) => r, @@ -963,6 +964,29 @@ async fn test_container_var_content() -> Result<()> { .is_none() ); + // Reset things + fixture.clear_destrepo()?; + + let mut imp = + store::ImageImporter::new(fixture.destrepo(), &derived_imgref, Default::default()).await?; + imp.set_ostree_version(2024, 3); + let prep = match imp.prepare().await.unwrap() { + store::PrepareResult::AlreadyPresent(_) => panic!("should not be already imported"), + store::PrepareResult::Ready(r) => r, + }; + let import = imp.import(prep).await.unwrap(); + let ostree_root = fixture + .destrepo() + .read_commit(&import.merge_commit, gio::Cancellable::NONE)? + .0; + let varfile = ostree_root + .child("usr/share/factory/var/lib/foo") + .downcast::() + .unwrap(); + assert!(!varfile.query_exists(gio::Cancellable::NONE)); + assert!(ostree_root + .child("var/lib/foo") + .query_exists(gio::Cancellable::NONE)); Ok(()) }