From 762e45871f0138e06dcef35b0deaf7c114bc0b84 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Tue, 25 Jan 2022 11:26:13 +0000 Subject: [PATCH 1/4] tar/export: add additional mandatory ostree repo directories This adds some hierarchies (e.g. 'refs/' and 'tmp/') that are expected by libostree to be present on valid repositories. --- lib/src/tar/export.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/src/tar/export.rs b/lib/src/tar/export.rs index b2af3681..1d623b0d 100644 --- a/lib/src/tar/export.rs +++ b/lib/src/tar/export.rs @@ -111,7 +111,6 @@ impl<'a, W: std::io::Write> OstreeTarWriter<'a, W> { if self.wrote_initdirs { return Ok(()); } - self.wrote_initdirs = true; let objdir: Utf8PathBuf = format!("{}/repo/objects", OSTREEDIR).into(); // Add all parent directories @@ -132,6 +131,16 @@ impl<'a, W: std::io::Write> OstreeTarWriter<'a, W> { let path: Utf8PathBuf = format!("{}/{:02x}", objdir, d).into(); self.append_default_dir(&path)?; } + // Tmp subdirectories + for d in ["tmp", "tmp/cache"] { + let path: Utf8PathBuf = format!("{}/repo/{}", OSTREEDIR, d).into(); + self.append_default_dir(&path)?; + } + // Refs subdirectories + for d in ["refs", "refs/heads", "refs/mirrors", "refs/remotes"] { + let path: Utf8PathBuf = format!("{}/repo/{}", OSTREEDIR, d).into(); + self.append_default_dir(&path)?; + } // The special `repo/xattrs` directory used only in our tar serialization. let path: Utf8PathBuf = format!("{}/repo/xattrs", OSTREEDIR).into(); @@ -150,6 +159,7 @@ impl<'a, W: std::io::Write> OstreeTarWriter<'a, W> { self.out .append_data(&mut h, path, std::io::Cursor::new(REPO_CONFIG))?; + self.wrote_initdirs = true; Ok(()) } From 895c35fef3d0c6efa3fa137417a939730fc499be Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Tue, 25 Jan 2022 14:37:30 +0000 Subject: [PATCH 2/4] tar/export: create 'ff' objects subdirectory This fixes an off-by-one bug, which was causing a missing objects subdirectory. --- lib/src/tar/export.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/tar/export.rs b/lib/src/tar/export.rs index 1d623b0d..2865f4fa 100644 --- a/lib/src/tar/export.rs +++ b/lib/src/tar/export.rs @@ -127,7 +127,7 @@ impl<'a, W: std::io::Write> OstreeTarWriter<'a, W> { self.append_default_dir(path)?; } // Object subdirectories - for d in 0..0xFF { + for d in 0..=0xFF { let path: Utf8PathBuf = format!("{}/{:02x}", objdir, d).into(); self.append_default_dir(&path)?; } From 07e9adcd01a4ca2deb3f5dd2f3afe18813596277 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Tue, 25 Jan 2022 14:55:33 +0000 Subject: [PATCH 3/4] lib/tests: check ostree repo skeleton on tar export --- lib/tests/it/main.rs | 76 ++++++++++++++++++++++++++++++++------------ 1 file changed, 55 insertions(+), 21 deletions(-) diff --git a/lib/tests/it/main.rs b/lib/tests/it/main.rs index 8a21190e..f5dbe250 100644 --- a/lib/tests/it/main.rs +++ b/lib/tests/it/main.rs @@ -10,6 +10,7 @@ use ostree_ext::container::{ use ostree_ext::tar::TarImportOptions; use ostree_ext::{gio, glib}; use sh_inline::bash; +use std::collections::HashMap; use std::convert::TryInto; use std::{io::Write, process::Command}; @@ -237,6 +238,7 @@ async fn test_tar_import_signed() -> Result<()> { Ok(()) } +#[derive(Debug)] struct TarExpected { path: &'static str, etype: tar::EntryType, @@ -257,33 +259,33 @@ fn validate_tar_expected( t: tar::Entries, expected: impl IntoIterator, ) -> Result<()> { - let expected = expected.into_iter(); - let mut entries = t.map(|e| e.unwrap()); + let mut expected: HashMap<&'static str, TarExpected> = + expected.into_iter().map(|exp| (exp.path, exp)).collect(); + let entries = t.map(|e| e.unwrap()); // Verify we're injecting directories, fixes the absence of `/tmp` in our // images for example. - for exp in expected { - let mut found = false; - while let Some(entry) = entries.next() { - let header = entry.header(); - let entry_path = entry.path().unwrap(); - if exp.path == entry_path.as_os_str() { - assert_eq!(header.entry_type(), exp.etype); - assert_eq!(header.mode().unwrap(), exp.mode); - found = true; - break; - } - } - if !found { - anyhow::bail!("Failed to find entry: {}", exp.path); + for entry in entries { + let header = entry.header(); + let entry_path = entry.path().unwrap().to_string_lossy().into_owned(); + if let Some(exp) = expected.remove(entry_path.as_str()) { + assert_eq!(header.entry_type(), exp.etype, "{}", entry_path); + assert_eq!(header.mode().unwrap(), exp.mode, "{}", entry_path); } } + + assert!( + expected.is_empty(), + "Expected but not found:\n{:?}", + expected + ); Ok(()) } /// Validate basic structure of the tar export. -/// Right now just checks the first entry is `sysroot` with mode 0755. #[test] fn test_tar_export_structure() -> Result<()> { + use tar::EntryType::{Directory, Regular}; + let mut fixture = Fixture::new()?; let src_tar = initial_export(&fixture)?; let src_tar = std::io::BufReader::new(std::fs::File::open(&src_tar)?); @@ -299,8 +301,22 @@ fn test_tar_export_structure() -> Result<()> { // Validate format version 0 let expected = [ - ("sysroot/config", tar::EntryType::Regular, 0o644), - ("usr", tar::EntryType::Directory, libc::S_IFDIR | 0o755), + ("sysroot/config", Regular, 0o644), + ("sysroot/ostree/repo", Directory, 0o755), + ("sysroot/ostree/repo/objects/00", Directory, 0o755), + ("sysroot/ostree/repo/objects/23", Directory, 0o755), + ("sysroot/ostree/repo/objects/77", Directory, 0o755), + ("sysroot/ostree/repo/objects/bc", Directory, 0o755), + ("sysroot/ostree/repo/objects/ff", Directory, 0o755), + ("sysroot/ostree/repo/refs", Directory, 0o755), + ("sysroot/ostree/repo/refs", Directory, 0o755), + ("sysroot/ostree/repo/refs/heads", Directory, 0o755), + ("sysroot/ostree/repo/refs/mirrors", Directory, 0o755), + ("sysroot/ostree/repo/refs/remotes", Directory, 0o755), + ("sysroot/ostree/repo/tmp", Directory, 0o755), + ("sysroot/ostree/repo/tmp/cache", Directory, 0o755), + ("sysroot/ostree/repo/xattrs", Directory, 0o755), + ("usr", Directory, libc::S_IFDIR | 0o755), ]; validate_tar_expected(entries, expected.iter().map(Into::into))?; @@ -310,8 +326,26 @@ fn test_tar_export_structure() -> Result<()> { let src_tar = std::io::BufReader::new(std::fs::File::open(&src_tar)?); let mut src_tar = tar::Archive::new(src_tar); let expected = [ - ("sysroot/ostree/repo/config", tar::EntryType::Regular, 0o644), - ("usr", tar::EntryType::Directory, libc::S_IFDIR | 0o755), + ("sysroot/ostree/repo", Directory, 0o755), + ("sysroot/ostree/repo/config", Regular, 0o644), + ("sysroot/ostree/repo/objects/00", Directory, 0o755), + ("sysroot/ostree/repo/objects/23", Directory, 0o755), + ("sysroot/ostree/repo/objects/77", Directory, 0o755), + ("sysroot/ostree/repo/objects/bc", Directory, 0o755), + ("sysroot/ostree/repo/objects/ff", Directory, 0o755), + ("sysroot/ostree/repo/refs", Directory, 0o755), + ("sysroot/ostree/repo/refs", Directory, 0o755), + ("sysroot/ostree/repo/refs/heads", Directory, 0o755), + ("sysroot/ostree/repo/refs/mirrors", Directory, 0o755), + ("sysroot/ostree/repo/refs/remotes", Directory, 0o755), + ("sysroot/ostree/repo/tmp", Directory, 0o755), + ("sysroot/ostree/repo/tmp/cache", Directory, 0o755), + ( + "sysroot/ostree/repo/xattrs", + Directory, + libc::S_IFDIR | 0o755, + ), + ("usr", Directory, libc::S_IFDIR | 0o755), ]; validate_tar_expected(src_tar.entries()?, expected.iter().map(Into::into))?; From 42664283224343c94d6c5af9962c5085973b23ca Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 25 Jan 2022 14:18:08 -0500 Subject: [PATCH 4/4] lib/tests: Mask out `S_IFMT` in tests We should fix the generation code, but for now let's just ignore it because I believe it's harmless. --- lib/tests/it/main.rs | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/lib/tests/it/main.rs b/lib/tests/it/main.rs index f5dbe250..de201c67 100644 --- a/lib/tests/it/main.rs +++ b/lib/tests/it/main.rs @@ -269,7 +269,15 @@ fn validate_tar_expected( let entry_path = entry.path().unwrap().to_string_lossy().into_owned(); if let Some(exp) = expected.remove(entry_path.as_str()) { assert_eq!(header.entry_type(), exp.etype, "{}", entry_path); - assert_eq!(header.mode().unwrap(), exp.mode, "{}", entry_path); + // FIXME: change the generation code to not inject the format bits into the mode, + // because tar doesn't need/use it. + // https://github.com/ostreedev/ostree-rs-ext/pull/217/files#r791942496 + assert_eq!( + header.mode().unwrap() & !libc::S_IFMT, + exp.mode, + "{}", + entry_path + ); } } @@ -316,7 +324,7 @@ fn test_tar_export_structure() -> Result<()> { ("sysroot/ostree/repo/tmp", Directory, 0o755), ("sysroot/ostree/repo/tmp/cache", Directory, 0o755), ("sysroot/ostree/repo/xattrs", Directory, 0o755), - ("usr", Directory, libc::S_IFDIR | 0o755), + ("usr", Directory, 0o755), ]; validate_tar_expected(entries, expected.iter().map(Into::into))?; @@ -340,12 +348,8 @@ fn test_tar_export_structure() -> Result<()> { ("sysroot/ostree/repo/refs/remotes", Directory, 0o755), ("sysroot/ostree/repo/tmp", Directory, 0o755), ("sysroot/ostree/repo/tmp/cache", Directory, 0o755), - ( - "sysroot/ostree/repo/xattrs", - Directory, - libc::S_IFDIR | 0o755, - ), - ("usr", Directory, libc::S_IFDIR | 0o755), + ("sysroot/ostree/repo/xattrs", Directory, 0o755), + ("usr", Directory, 0o755), ]; validate_tar_expected(src_tar.entries()?, expected.iter().map(Into::into))?;