quine-global-labs/mirror-coreos-chunkah
1
0
Fork 0
mirror of https://github.com/coreos/chunkah.git synced 2026-11-03 21:49:44 -09:00
Code Issues Projects Releases Packages Wiki Activity

feat: clamp xattr and bigfiles mtimes to most recent file mtime

Browse source 
This solves a reproducibility issue with `bigfiles` and `xattr`
components, which don't automatically have reproducible timestamps: Even
if all the files in the component are given fixed modification
timestamps, the timestamps of ancestor directories aren't clamped to
match the files, which makes the layers non-reproducible unless
`SOURCE_DATE_EPOCH` is set.

To fix this, we clamp `bigfiles` components to the modification time of
the single file, and clamp `xattr` components to the most recent
modification time of a file with that xattr label.
This commit is contained in:
Daniel Hast 2026-08-02 23:17:47 -04:00 committed by Jonathan Lebon
parent 25d8d07790
commit 2869ce754b
5 changed files with 96 additions and 52 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

36
src/components/bigfiles.rs
View file

@ -3,7 +3,7 @@ use std::collections::HashMap;
use anyhow::Result;
use camino::{Utf8Path, Utf8PathBuf};
use cap_std_ext::cap_std::fs::Dir;
use indexmap::IndexSet;
use indexmap::IndexMap;
use super::{ComponentId, ComponentInfo, ComponentsRepo, FileMap, FileType};
@ -25,12 +25,10 @@ const REPO_NAME: &str = "bigfiles";
/// Some special handling for hardlinked files (same inode); we still want
/// unclaimed files that are hardlinked to end up in the same component.
pub struct BigfilesRepo {
/// Component names, indexed by ComponentId.
components: IndexSet<String>,
/// Map from component names to modification times, indexed by ComponentId.
components: IndexMap<String, u64>,
/// Mapping from path to ComponentId.
path_to_component: HashMap<Utf8PathBuf, ComponentId>,
/// Default mtime clamp for components.
default_mtime_clamp: u64,
}
impl BigfilesRepo {
@ -41,8 +39,8 @@ impl BigfilesRepo {
// TODO: the upfront scan logic here (inode table, path_to_component map)
// could be deferred to weak_claims_for_path time since it receives
// &FileInfo with size/inode/nlink.
pub fn load(files: &FileMap, default_mtime_clamp: u64) -> Option<Self> {
let mut components: IndexSet<String> = IndexSet::new();
pub fn load(files: &FileMap) -> Option<Self> {
let mut components: IndexMap<String, u64> = IndexMap::new();
let mut path_to_component: HashMap<Utf8PathBuf, ComponentId> = HashMap::new();
// build inode table for hardlink handling
@ -72,7 +70,7 @@ impl BigfilesRepo {
.expect("filename has no basename");
// derive component name from filename
let component_name = if components.contains(&filename) {
let component_name = if components.contains_key(&filename) {
// filename already used, use full path without leading '/'
path.strip_prefix("/")
.expect("non-absolute file path in FileMap")
@ -83,7 +81,7 @@ impl BigfilesRepo {
};
// create a component for this path
let (idx, _) = components.insert_full(component_name);
let (idx, _) = components.insert_full(component_name, file_info.mtime);
tracing::trace!(path = %path, component = %components[idx], id = idx, "bigfiles component created");
let component_id = ComponentId(idx);
path_to_component.insert(path.clone(), component_id);
@ -109,7 +107,6 @@ impl BigfilesRepo {
Some(Self {
components,
path_to_component,
default_mtime_clamp,
})
}
}
@ -137,14 +134,15 @@ impl ComponentsRepo for BigfilesRepo {
}
fn component_info(&self, id: ComponentId) -> ComponentInfo<'_> {
let (name, &mtime_clamp) = self
.components
.get_index(id.0)
// SAFETY: the ids we're given come from the IndexMap itself
// when we inserted the element, so it must be valid.
.expect("invalid ComponentId");
ComponentInfo {
name: self
.components
.get_index(id.0)
// SAFETY: the ids we're given come from the IndexSet itself
// when we inserted the element, so it must be valid.
.expect("invalid ComponentId"),
mtime_clamp: self.default_mtime_clamp,
name,
mtime_clamp,
stability: 0.0,
}
}
@ -223,7 +221,7 @@ mod tests {
let rootfs = Dir::open_ambient_dir(tmp.path(), ambient_authority()).unwrap();
let files = crate::scan::Scanner::new(&rootfs).scan().unwrap();
let repo = BigfilesRepo::load(&files, 12345).unwrap();
let repo = BigfilesRepo::load(&files).unwrap();
// small file should not be claimed
let claims = repo
@ -271,7 +269,7 @@ mod tests {
create_sparse_file(rootfs, "b/foobar", 4 * 1024 * 1024);
});
let repo = BigfilesRepo::load(&files, 0).unwrap();
let repo = BigfilesRepo::load(&files).unwrap();
// First one uses filename, second uses full path
assert_component(&repo, &rootfs, "/a/foobar", "foobar");

10
src/components/mod.rs
View file

@ -116,9 +116,7 @@ impl ComponentsRepos {
pub fn load(rootfs: &Dir, files: &FileMap, default_mtime_clamp: u64) -> Result<Self> {
let mut repos: Vec<Box<dyn ComponentsRepo>> = Vec::new();
if let Some(repo) =
xattr::XattrRepo::load(files, default_mtime_clamp).context("loading xattrs")?
{
if let Some(repo) = xattr::XattrRepo::load(files).context("loading xattrs")? {
tracing::info!(repo = "xattr", "loaded repo");
repos.push(Box::new(repo));
}
@ -137,7 +135,7 @@ impl ComponentsRepos {
repos.push(Box::new(repo));
}
if let Some(repo) = bigfiles::BigfilesRepo::load(files, default_mtime_clamp) {
if let Some(repo) = bigfiles::BigfilesRepo::load(files) {
tracing::info!(repo = "bigfiles", "loaded repo");
repos.push(Box::new(repo));
}
@ -412,7 +410,7 @@ mod tests {
let files = crate::scan::Scanner::new(&rootfs).scan().unwrap();
let xattr_repo = xattr::XattrRepo::load(&files, 0).unwrap().unwrap();
let xattr_repo = xattr::XattrRepo::load(&files).unwrap().unwrap();
let packages = rpm_qa::load_from_str(RPM_FIXTURE).unwrap();
let now = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
@ -480,7 +478,7 @@ mod tests {
let files = crate::scan::Scanner::new(&rootfs).scan().unwrap();
let xattr_repo = xattr::XattrRepo::load(&files, 0).unwrap().unwrap();
let xattr_repo = xattr::XattrRepo::load(&files).unwrap().unwrap();
let repos: Vec<Box<dyn ComponentsRepo>> = vec![Box::new(xattr_repo)];
let loaded = ComponentsRepos {
repos,

50
src/components/xattr.rs
View file

@ -2,7 +2,7 @@ use std::collections::HashMap;
use anyhow::{Context, Result};
use camino::{Utf8Path, Utf8PathBuf};
use indexmap::IndexSet;
use indexmap::IndexMap;
use super::{
ComponentId, ComponentInfo, ComponentsRepo, FileInfo, FileMap, FileType, STABILITY_PERIOD_DAYS,
@ -19,24 +19,20 @@ const REPO_NAME: &str = "xattr";
/// Directories with this xattr apply to all files underneath unless overridden.
/// Directory inheritance is pre-computed during load.
pub struct XattrRepo {
/// Component names, indexed by ComponentId.
components: IndexSet<String>,
/// Map from component names to modification times, indexed by ComponentId.
components: IndexMap<String, u64>,
/// Mapping from path to ComponentId (pre-computed with inheritance).
path_to_component: HashMap<Utf8PathBuf, ComponentId>,
/// Per-component stability, indexed by ComponentId.
component_stability: Vec<f64>,
/// Currently, the on-disk mtime is canonical and we clamp it, but it would
/// make sense in the future to support another user xattr to specify a
/// canonical mtime for easier layer reproducibility.
default_mtime_clamp: u64,
}
impl XattrRepo {
/// Load xattr repo by scanning rootfs for user.component xattrs.
/// Pre-computes directory inheritance for all paths in `files`.
/// Uses cached xattrs from FileInfo rather than reading from disk.
pub fn load(files: &FileMap, default_mtime_clamp: u64) -> Result<Option<Self>> {
let mut components: IndexSet<String> = IndexSet::new();
pub fn load(files: &FileMap) -> Result<Option<Self>> {
let mut components: IndexMap<String, u64> = IndexMap::new();
let mut path_to_component: HashMap<Utf8PathBuf, ComponentId> = HashMap::new();
// Track raw intervals during scanning to detect conflicts
let mut update_intervals: Vec<Option<u64>> = Vec::new();
@ -63,7 +59,7 @@ impl XattrRepo {
// https://github.com/indexmap-rs/indexmap/issues/355 or
// https://github.com/indexmap-rs/indexmap/issues/388
let idx = components.get_index_of(name).unwrap_or_else(|| {
let idx = components.insert_full(name.clone()).0;
let idx = components.insert_full(name.clone(), file_info.mtime).0;
update_intervals.push(None);
tracing::trace!(path = %path, name = %name, id = idx, "xattr component created");
idx
@ -84,6 +80,12 @@ impl XattrRepo {
if let Some(id) = effective_id {
tracing::trace!(path = %path, component_id = id.0, "xattr assignment");
path_to_component.insert(path.clone(), id);
let (_, component_mtime) = components
.get_index_mut(id.0)
.expect("xattr component should have already been created");
// This ultimately makes the component mtime equal to the most
// recent modification time of a file in the component.
*component_mtime = file_info.mtime.max(*component_mtime);
// Check for user.update-interval xattr (not inherited from directories)
if let Some(interval) = get_update_interval_xattr(file_info)
@ -153,7 +155,6 @@ impl XattrRepo {
components,
path_to_component,
component_stability,
default_mtime_clamp,
}))
}
}
@ -233,14 +234,15 @@ impl ComponentsRepo for XattrRepo {
}
fn component_info(&self, id: ComponentId) -> ComponentInfo<'_> {
let (name, &mtime_clamp) = self
.components
.get_index(id.0)
// SAFETY: the ids we're given come from the IndexMap itself
// when we inserted the element, so it must be valid.
.expect("invalid ComponentId");
ComponentInfo {
name: self
.components
.get_index(id.0)
// SAFETY: the ids we're given come from the IndexSet itself
// when we inserted the element, so it must be valid.
.expect("invalid ComponentId"),
mtime_clamp: self.default_mtime_clamp,
name,
mtime_clamp,
stability: self.component_stability[id.0],
}
}
@ -313,7 +315,7 @@ mod tests {
// File without xattr outside of directory - should not be claimed
rootfs.write("noattr", "content").unwrap();
});
let repo = XattrRepo::load(&files, 0).unwrap().unwrap();
let repo = XattrRepo::load(&files).unwrap().unwrap();
// /mydir and /mydir/normal should be dircomponent
assert_component(&repo, "/mydir", FileType::Directory, "dircomponent");
@ -343,7 +345,7 @@ mod tests {
set_component(rootfs, "a/b/c/d", "compD");
set_component(rootfs, "x", "compX");
});
let repo = XattrRepo::load(&files, 0).unwrap().unwrap();
let repo = XattrRepo::load(&files).unwrap().unwrap();
assert_component(&repo, "/a", FileType::Directory, "compA");
assert_component(&repo, "/a/other", FileType::File, "compA"); // inherits from /a
@ -364,7 +366,7 @@ mod tests {
// Create a symlink inside the directory - it should inherit from parent
rootfs.symlink("../somewhere", "mydir/link").unwrap();
});
let repo = XattrRepo::load(&files, 0).unwrap().unwrap();
let repo = XattrRepo::load(&files).unwrap().unwrap();
// Both should be claimed by mycomp
assert_component(&repo, "/mydir", FileType::Directory, "mycomp");
@ -400,7 +402,7 @@ mod tests {
rootfs.write("mydir/file", "content").unwrap();
set_update_interval(rootfs, "mydir/file", "60");
});
let result = XattrRepo::load(&files, 0);
let result = XattrRepo::load(&files);
assert!(result.is_err());
let msg = format!("{:#}", result.err().unwrap());
assert!(msg.contains("conflicting"), "unexpected error: {msg}");
@ -415,7 +417,7 @@ mod tests {
rootfs.write("mydir/file", "content").unwrap();
set_update_interval(rootfs, "mydir/file", "30");
});
let repo = XattrRepo::load(&files, 0).unwrap().unwrap();
let repo = XattrRepo::load(&files).unwrap().unwrap();
let claims = repo.strong_claims_for_path(Utf8Path::new("/mydir"), &fi(FileType::Directory));
let expected = interval_to_stability(30);
@ -435,7 +437,7 @@ mod tests {
set_component(rootfs, "app2", "comp2");
rootfs.write("app2/file", "content").unwrap();
});
let repo = XattrRepo::load(&files, 0).unwrap().unwrap();
let repo = XattrRepo::load(&files).unwrap().unwrap();
// comp1 should have yearly stability
let claims = repo.strong_claims_for_path(Utf8Path::new("/app1"), &fi(FileType::Directory));

33
tests/e2e/lib.sh
View file

@ -48,6 +48,39 @@ assert_layer_count() {
fi
}
# Get digest of layer with specific component name in image
get_image_component_digest() {
local component="$1"
local image="$2"
local digest
digest="$(skopeo inspect -n "containers-storage:${image}" \
| jq -r --arg component "${component}" '
.LayersData[]
| select(.Annotations."org.chunkah.component" == $component)
| .Digest
')"
if [[ -z "${digest}" ]]; then
echo "ERROR: Layer with component '${component}' not found in image ${image}" >&2
exit 1
fi
printf '%s' "${digest}"
}
# Assert layer with given component name is unmodified between images
assert_layer_unmodified() {
local component="$1"
local image1="$2"
local image2="$3"
local digest1 digest2
digest1=$(get_image_component_digest "${component}" "${image1}") || exit 1
digest2=$(get_image_component_digest "${component}" "${image2}") || exit 1
if [[ "${digest1}" != "${digest2}" ]]; then
echo "ERROR: Digests of layer with component '${component}' differ between ${image1} and ${image2}:" >&2
printf '%s != %s\n' "${digest1}" "${digest2}" >&2
exit 1
fi
}
# Assert that a path exists in an image.
assert_path_exists() {
local image="${1}"; shift

19
tests/e2e/test-buildtime.sh
View file

@ -11,6 +11,7 @@ BASE_IMAGE="quay.io/fedora/fedora-minimal:latest"
ROOTFS_IMAGE="localhost/test-buildtime-rootfs:latest"
CHUNKED_IMAGE="localhost/fedora-minimal-chunked:test"
CHUNKED_IMAGE2="localhost/fedora-minimal-chunked:test2"
CHUNKED_IMAGE3="localhost/fedora-minimal-chunked:test3"
cleanup() {
cleanup_images "${CHUNKED_IMAGE}" "${CHUNKED_IMAGE2}" "${ROOTFS_IMAGE}"
@ -22,9 +23,14 @@ podman pull "${BASE_IMAGE}"
# Build rootfs as a separate image so that the reproducibility check below
# operates on the same rootfs both times. Without this, each buildah_build
# would re-run dnf install, producing different file timestamps.
#
# Also create a large random file used to test reproducibility of bigfiles
# components.
cat > Containerfile.rootfs <<EOF
FROM ${BASE_IMAGE}
RUN dnf install -y jq acl attr && dnf clean all
RUN dnf install -y jq acl attr && \
dnf clean all && \
dd if=/dev/urandom of=/usr/bin/chungus bs=1M count=20
EOF
buildah build -t "${ROOTFS_IMAGE}" -f Containerfile.rootfs .
@ -38,7 +44,8 @@ RUN cp /usr/bin/true /usr/bin/test-xattrs && \
setcap cap_net_raw+ep /usr/bin/test-xattrs && \
setfacl -m u:nobody:r /usr/bin/test-xattrs && \
setfattr -n user.testkey1 -v testvalue1 /usr/bin/test-xattrs && \
setfattr -n user.testkey2 -v testvalue2 /usr/bin/test-xattrs
setfattr -n user.testkey2 -v testvalue2 /usr/bin/test-xattrs && \
touch -d @0 /usr/bin/chungus
FROM ${CHUNKAH_IMG:?} AS chunkah
RUN --mount=from=builder,src=/,target=/chunkah,ro \\
@ -55,7 +62,7 @@ buildah_build -t "${CHUNKED_IMAGE}" -f Containerfile .
podman run --rm "${CHUNKED_IMAGE}" jq --version
# check for expected components
assert_has_components "${CHUNKED_IMAGE}" "rpm/filesystem" "rpm/setup" "rpm/glibc" "rpm/jq"
assert_has_components "${CHUNKED_IMAGE}" "rpm/filesystem" "rpm/setup" "rpm/glibc" "rpm/jq" "bigfiles/chungus"
# verify we got exactly 64 layers (the default)
assert_layer_count "${CHUNKED_IMAGE}" 64
@ -89,3 +96,9 @@ if [[ "${id1}" != "${id2}" ]]; then
assert_no_diff "${CHUNKED_IMAGE}" "${CHUNKED_IMAGE2}"
exit 1 # but just in case
fi
# Verify that bigfiles component with reproducible modification time is
# reproducible even if SOURCE_DATE_EPOCH changes
sed -i -e 's/\<SOURCE_DATE_EPOCH=17/SOURCE_DATE_EPOCH=16/' Containerfile
buildah_build --no-cache -t "${CHUNKED_IMAGE3}" -f Containerfile .
assert_layer_unmodified "bigfiles/chungus" "${CHUNKED_IMAGE}" "${CHUNKED_IMAGE3}"