From 53c2b7dae453daf6cba238b4817e95e703d6a519 Mon Sep 17 00:00:00 2001 From: stevenhorsman Date: Tue, 2 Jul 2024 11:48:21 +0100 Subject: [PATCH] Revert "Handle gzip whiteouts correctly" Revert whiteout handling whilst we investigate image pull errors with whiteouts. See #604 This reverts commit a54646f4dd544128328bed3be0a6a3dde9fcd1a5. Signed-off-by: stevenhorsman --- Cargo.lock | 1 - image-rs/Cargo.toml | 1 - image-rs/src/unpack.rs | 99 ------------------------------------------ 3 files changed, 101 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index baee3afad..3ab441268 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2803,7 +2803,6 @@ dependencies = [ "ttrpc-codegen", "url", "walkdir", - "xattr", "zstd 0.12.4", ] diff --git a/image-rs/Cargo.toml b/image-rs/Cargo.toml index f690b634c..43b84d33a 100644 --- a/image-rs/Cargo.toml +++ b/image-rs/Cargo.toml @@ -61,7 +61,6 @@ tonic = { workspace = true, optional = true } ttrpc = { workspace = true, features = ["async"], optional = true } url = "2.5.2" walkdir = "2" -xattr = "1.0" zstd = "0.12" nydus-api = { version = "0.3.0", optional = true } diff --git a/image-rs/src/unpack.rs b/image-rs/src/unpack.rs index 7af7f76bc..779b1980c 100644 --- a/image-rs/src/unpack.rs +++ b/image-rs/src/unpack.rs @@ -2,23 +2,16 @@ // // SPDX-License-Identifier: Apache-2.0 -use anyhow::anyhow; use anyhow::Context; use anyhow::{bail, Result}; use libc::timeval; use log::warn; use std::collections::HashMap; -use std::convert::TryInto; use std::ffi::CString; use std::fs; use std::io; use std::path::Path; use tar::Archive; -use tar::Header; - -// See https://github.com/opencontainers/image-spec/blob/main/layer.md#whiteouts -const WHITEOUT_PREFIX: &str = ".wh."; -const WHITEOUT_OPAQUE_DIR: &str = ".wh..wh..opq"; /// Unpack the contents of tarball to the destination path pub fn unpack(input: R, destination: &Path) -> Result<()> { @@ -40,11 +33,6 @@ pub fn unpack(input: R, destination: &Path) -> Result<()> { let mut dirs: HashMap = HashMap::default(); for file in archive.entries()? { let mut file = file?; - - if !convert_whiteout(&file.path()?, file.header(), destination)? { - continue; - } - file.unpack_in(destination)?; // tar-rs crate only preserve timestamps of files, @@ -95,69 +83,11 @@ pub fn unpack(input: R, destination: &Path) -> Result<()> { Ok(()) } -fn convert_whiteout(path: &Path, header: &Header, destination: &Path) -> Result { - // Handle whiteout conversion - let name = path - .file_name() - .unwrap_or_default() - .to_str() - .ok_or(anyhow!("Invalid unicode in whiteout path: {:?}", path))?; - - if name.starts_with(WHITEOUT_PREFIX) { - let parent = path - .parent() - .ok_or(anyhow!("Invalid whiteout parent for path: {:?}", path))?; - - if name == WHITEOUT_OPAQUE_DIR { - let destination_parent = destination.join(parent); - xattr::set(destination_parent, "trusted.overlay.opaque", b"y")?; - return Ok(false); - } - - let original_name = name - .strip_prefix(WHITEOUT_PREFIX) - .ok_or(anyhow!("Failed to strip whiteout prefix for: {}", name))?; - let original_path = parent.join(original_name); - let path = CString::new(format!( - "{}/{}", - destination.display(), - original_path.display() - ))?; - - let ret = unsafe { libc::mknod(path.as_ptr(), libc::S_IFCHR, 0) }; - if ret != 0 { - bail!("mknod: {:?} error: {:?}", path, io::Error::last_os_error()); - } - - let uid: libc::uid_t = header - .uid()? - .try_into() - .map_err(|_| io::Error::new(io::ErrorKind::Other, "UID is too large!"))?; - let gid: libc::gid_t = header - .gid()? - .try_into() - .map_err(|_| io::Error::new(io::ErrorKind::Other, "GID is too large!"))?; - - let ret = unsafe { libc::lchown(path.as_ptr(), uid, gid) }; - if ret != 0 { - bail!( - "change ownership: {:?} error: {:?}", - path, - io::Error::last_os_error() - ); - } - return Ok(false); - } - - Ok(true) -} - #[cfg(test)] mod tests { use super::*; use std::fs::File; use std::io::prelude::*; - use std::os::unix::fs::FileTypeExt; #[test] fn test_unpack() { @@ -175,33 +105,12 @@ mod tests { ar.append_file("file.txt", &mut File::open(&path).unwrap()) .unwrap(); - let path = tempdir - .path() - .join(WHITEOUT_PREFIX.to_owned() + "whiteout_file.txt"); - File::create(&path).unwrap(); - ar.append_file( - WHITEOUT_PREFIX.to_owned() + "whiteout_file.txt", - &mut File::open(&path).unwrap(), - ) - .unwrap(); - let path = tempdir.path().join("dir"); fs::create_dir(&path).unwrap(); filetime::set_file_mtime(&path, mtime).unwrap(); ar.append_path_with_name(&path, "dir").unwrap(); - let path = tempdir.path().join("whiteout_dir"); - fs::create_dir(&path).unwrap(); - ar.append_path_with_name(&path, "whiteout_dir").unwrap(); - - let path = tempdir - .path() - .join("whiteout_dir/".to_owned() + WHITEOUT_OPAQUE_DIR); - fs::create_dir(&path).unwrap(); - ar.append_path_with_name(&path, "whiteout_dir/".to_owned() + WHITEOUT_OPAQUE_DIR) - .unwrap(); - // TODO: Add more file types like symlink, char, block devices. let data = ar.into_inner().unwrap(); tempdir.close().unwrap(); @@ -218,19 +127,11 @@ mod tests { let new_mtime = filetime::FileTime::from_last_modification_time(&metadata); assert_eq!(mtime, new_mtime); - let path = destination.join("whiteout_file.txt"); - let metadata = fs::metadata(path).unwrap(); - assert!(metadata.file_type().is_char_device()); - let path = destination.join("dir"); let metadata = fs::metadata(path).unwrap(); let new_mtime = filetime::FileTime::from_last_modification_time(&metadata); assert_eq!(mtime, new_mtime); - let path = destination.join("whiteout_dir"); - let opaque = xattr::get(path, "trusted.overlay.opaque").unwrap().unwrap(); - assert_eq!(opaque, b"y"); - // though destination already exists, it will be deleted // and rewrite assert!(unpack(data.as_slice(), destination).is_ok());