Close prepared-lower identity and reclaim races
This commit is contained in:
parent
5458460717
commit
0dfd7767b5
4 changed files with 142 additions and 30 deletions
|
|
@ -95,6 +95,7 @@ struct Descriptor {
|
|||
}
|
||||
|
||||
pub(crate) struct VerifiedLayout {
|
||||
pub(crate) manifest_digest: [u8; 32],
|
||||
layers: Vec<Descriptor>,
|
||||
diff_ids: Vec<String>,
|
||||
}
|
||||
|
|
@ -130,6 +131,7 @@ pub(crate) fn verify_layout(layout: &Path, limits: PreparationLimits) -> Result<
|
|||
verify_blob_digest(layout, layer, limits.max_layer_blob_bytes)?;
|
||||
}
|
||||
Ok(VerifiedLayout {
|
||||
manifest_digest: parse_sha256(&descriptor.digest)?,
|
||||
layers: manifest.layers,
|
||||
diff_ids: config.rootfs.diff_ids,
|
||||
})
|
||||
|
|
@ -147,41 +149,24 @@ pub(crate) fn apply_layout(
|
|||
let mut usage = Usage::default();
|
||||
for (index, (descriptor, diff_id)) in verified.layers.iter().zip(&verified.diff_ids).enumerate()
|
||||
{
|
||||
let spool_name = format!(".layer-{index}.tar");
|
||||
let spool_output = File::from(openat2(
|
||||
let spool = File::from(openat2(
|
||||
rootfs,
|
||||
&spool_name,
|
||||
OFlags::WRONLY | OFlags::CREATE | OFlags::EXCL | OFlags::CLOEXEC | OFlags::NOFOLLOW,
|
||||
".",
|
||||
OFlags::RDWR | OFlags::TMPFILE | OFlags::CLOEXEC,
|
||||
Mode::from_raw_mode(0o600),
|
||||
secure_resolve(),
|
||||
)?);
|
||||
let observed = decompress_layer(
|
||||
&blob_path(layout, &descriptor.digest)?,
|
||||
&descriptor.media_type,
|
||||
spool_output,
|
||||
spool.try_clone()?,
|
||||
limits.max_uncompressed_layer_bytes,
|
||||
)?;
|
||||
if &observed != diff_id {
|
||||
let _ = unlinkat(rootfs, &spool_name, AtFlags::empty());
|
||||
bail!("layer {index} diff-ID mismatch");
|
||||
}
|
||||
let spool = File::from(openat2(
|
||||
rootfs,
|
||||
&spool_name,
|
||||
OFlags::RDONLY | OFlags::CLOEXEC | OFlags::NOFOLLOW,
|
||||
Mode::empty(),
|
||||
secure_resolve(),
|
||||
)?);
|
||||
let result = validate_tar(&spool, id_map, limits, &mut usage)
|
||||
.and_then(|()| state.apply(&spool, rootfs));
|
||||
if let Err(error) = result {
|
||||
let _ = unlinkat(rootfs, &spool_name, AtFlags::empty());
|
||||
return Err(error);
|
||||
}
|
||||
match unlinkat(rootfs, &spool_name, AtFlags::empty()) {
|
||||
Ok(()) | Err(rustix::io::Errno::NOENT) => {}
|
||||
Err(error) => return Err(error.into()),
|
||||
}
|
||||
validate_tar(&spool, id_map, limits, &mut usage)?;
|
||||
state.apply(&spool, rootfs)?;
|
||||
}
|
||||
state.finish(rootfs)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -14,7 +14,10 @@ use lazaret_core::identity::{
|
|||
use lazaret_core::records::{ArtifactState, PrepareOperation, PrepareStageState, PreparedArtifact};
|
||||
use lazaret_linux::filesystem::{ExclusiveFileLock, try_exclusive_flock};
|
||||
use lazaret_storage_catalog::{Catalog, CatalogError, CatalogRow};
|
||||
use rustix::fs::{AtFlags, Mode, OFlags, ResolveFlags, mkdirat, openat2, renameat, unlinkat};
|
||||
use rustix::fs::{
|
||||
AtFlags, Mode, OFlags, RenameFlags, ResolveFlags, mkdirat, openat2, renameat, renameat_with,
|
||||
unlinkat,
|
||||
};
|
||||
use sha2::{Digest, Sha256};
|
||||
|
||||
use crate::layer::{
|
||||
|
|
@ -94,6 +97,9 @@ impl PreparedLowerStore {
|
|||
bail!("source identity is empty or exceeds 4096 bytes");
|
||||
}
|
||||
let verified = verify_layout(&request.layout_path, self.limits)?;
|
||||
if request.selected_manifest_digest != verified.manifest_digest {
|
||||
bail!("selected manifest identity does not match verified OCI layout");
|
||||
}
|
||||
let identities = Identities::new(request)?;
|
||||
if let Some(existing) = self.available_artifact(identities.artifact_id)? {
|
||||
identities.require_equivalent(&existing)?;
|
||||
|
|
@ -218,10 +224,20 @@ impl PreparedLowerStore {
|
|||
{
|
||||
let _lock = self.physical_lock()?;
|
||||
if let Some(canonical) = self.open_relative(&expected_artifact)? {
|
||||
require_identity(&canonical, reclaiming.device, reclaiming.inode)?;
|
||||
self.rename_relative(&expected_artifact, &quarantine)?;
|
||||
if file_identity(&canonical)? == (reclaiming.device, reclaiming.inode) {
|
||||
self.rename_relative_noreplace(&expected_artifact, &quarantine)?;
|
||||
self.sync_fixed_directory("artifacts")?;
|
||||
self.sync_fixed_directory("quarantine")?;
|
||||
} else {
|
||||
let quarantined = self
|
||||
.open_relative(&quarantine)?
|
||||
.context("canonical replacement exists without quarantined old artifact")?;
|
||||
require_identity(&quarantined, reclaiming.device, reclaiming.inode)?;
|
||||
let replacement = self
|
||||
.available_artifact(reclaiming.prepared_artifact_id)?
|
||||
.context("canonical replacement exists without available ownership")?;
|
||||
require_identity(&canonical, replacement.device, replacement.inode)?;
|
||||
}
|
||||
} else {
|
||||
let existing = self
|
||||
.open_relative(&quarantine)?
|
||||
|
|
@ -275,7 +291,7 @@ impl PreparedLowerStore {
|
|||
.context("canonical artifact path exists without matching catalog ownership")?;
|
||||
require_identity(&existing, reclaiming.device, reclaiming.inode)?;
|
||||
let quarantine = quarantine_path(&reclaiming);
|
||||
self.rename_relative(&artifact_path, &quarantine)?;
|
||||
self.rename_relative_noreplace(&artifact_path, &quarantine)?;
|
||||
self.sync_fixed_directory("quarantine")?;
|
||||
}
|
||||
self.rename_relative(&stage, &artifact_path)?;
|
||||
|
|
@ -360,6 +376,12 @@ impl PreparedLowerStore {
|
|||
Ok(())
|
||||
}
|
||||
|
||||
fn rename_relative_noreplace(&self, from: &Path, to: &Path) -> Result<()> {
|
||||
let root = File::open(&self.root)?;
|
||||
renameat_with(&root, from, &root, to, RenameFlags::NOREPLACE)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn remove_exact_path(&self, relative: &Path, device: DeviceId, inode: InodeId) -> Result<()> {
|
||||
let root = File::open(&self.root)?;
|
||||
let file = open_directory_at(&root, relative)?;
|
||||
|
|
|
|||
|
|
@ -164,7 +164,7 @@ impl OciRegistryTransport {
|
|||
let local_digest = format!("sha256:{}", hex(&Sha256::digest(&local_manifest)));
|
||||
write_verified_blob(&blobs, &local_digest, &local_manifest)?;
|
||||
write_layout_index(layout_path, &local_digest, local_manifest.len())?;
|
||||
let selected_manifest_digest = parse_sha256(&manifest_digest)?;
|
||||
let selected_manifest_digest = parse_sha256(&local_digest)?;
|
||||
let source_identity = format!(
|
||||
"oci-v1\0{}\0{}\0{manifest_digest}",
|
||||
reference.registry(),
|
||||
|
|
|
|||
|
|
@ -400,6 +400,44 @@ fn semantic_corpus_preserves_supported_changesets_and_read_only_lower_use() {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn request_manifest_identity_must_match_verified_layout_manifest() {
|
||||
let fixture = Fixture::new();
|
||||
let bytes = layer(&[Entry {
|
||||
path: "payload",
|
||||
kind: EntryType::Regular,
|
||||
data: b"content",
|
||||
target: None,
|
||||
mode: 0o644,
|
||||
}]);
|
||||
let mut request = fixture.request(3, &[(bytes, OCI_TAR)]);
|
||||
request.selected_manifest_digest = [0xff; 32];
|
||||
|
||||
assert!(fixture.store.prepare(&request).is_err());
|
||||
let snapshot = fixture.catalog.snapshot().unwrap();
|
||||
assert!(snapshot.prepared_artifacts.is_empty());
|
||||
assert!(snapshot.prepare_operations.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn layer_entry_matching_internal_spool_name_is_preserved() {
|
||||
let fixture = Fixture::new();
|
||||
let bytes = layer(&[Entry {
|
||||
path: ".layer-0.tar",
|
||||
kind: EntryType::Regular,
|
||||
data: b"image-owned",
|
||||
target: None,
|
||||
mode: 0o640,
|
||||
}]);
|
||||
let request = fixture.request(4, &[(bytes, OCI_TAR)]);
|
||||
|
||||
let artifact = fixture.store.prepare(&request).unwrap();
|
||||
assert_eq!(
|
||||
std::fs::read(fixture.artifact_root(&artifact).join(".layer-0.tar")).unwrap(),
|
||||
b"image-owned"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn root_opaque_whiteout_removes_only_lower_layer_entries() {
|
||||
let fixture = Fixture::new();
|
||||
|
|
@ -641,6 +679,73 @@ fn publication_during_reclaim_preserves_replacement_and_deletes_old_inode() {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn replacement_winning_before_reclaimer_resumes_still_cleans_old_quarantine() {
|
||||
let fixture = Fixture::new();
|
||||
let bytes = layer(&[Entry {
|
||||
path: "payload",
|
||||
kind: EntryType::Regular,
|
||||
data: b"same",
|
||||
target: None,
|
||||
mode: 0o644,
|
||||
}]);
|
||||
let first_request = fixture.request(52, &[(bytes, OCI_TAR)]);
|
||||
let first = fixture.store.prepare(&first_request).unwrap();
|
||||
let reclaiming = fixture.catalog.begin_reclaim(&first).unwrap();
|
||||
let replacement_request = PrepareRequest {
|
||||
operation_id: PrepareOperationId::new([53; 32]),
|
||||
..first_request
|
||||
};
|
||||
|
||||
let replacement = fixture.store.prepare(&replacement_request).unwrap();
|
||||
assert_ne!(replacement.inode, first.inode);
|
||||
assert_eq!(
|
||||
fixture.store.resume_reclaim(&reclaiming).unwrap(),
|
||||
ReclaimOutcome::ReplacementPreserved
|
||||
);
|
||||
assert_eq!(
|
||||
fixture.catalog.snapshot().unwrap().prepared_artifacts,
|
||||
vec![replacement.clone()]
|
||||
);
|
||||
assert!(fixture.artifact_root(&replacement).exists());
|
||||
assert!(
|
||||
fixture
|
||||
.root
|
||||
.join("quarantine")
|
||||
.read_dir()
|
||||
.unwrap()
|
||||
.next()
|
||||
.is_none()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn reclaim_refuses_preexisting_foreign_quarantine_path() {
|
||||
let fixture = Fixture::new();
|
||||
let bytes = layer(&[Entry {
|
||||
path: "payload",
|
||||
kind: EntryType::Regular,
|
||||
data: b"same",
|
||||
target: None,
|
||||
mode: 0o644,
|
||||
}]);
|
||||
let artifact = fixture
|
||||
.store
|
||||
.prepare(&fixture.request(54, &[(bytes, OCI_TAR)]))
|
||||
.unwrap();
|
||||
let foreign = fixture.root.join("quarantine").join(format!(
|
||||
"{}-{}-{}",
|
||||
hex(artifact.prepared_artifact_id.as_bytes()),
|
||||
artifact.device.get(),
|
||||
artifact.inode.get()
|
||||
));
|
||||
std::fs::create_dir(&foreign).unwrap();
|
||||
|
||||
assert!(fixture.store.reclaim(&artifact).is_err());
|
||||
assert!(foreign.is_dir());
|
||||
assert!(fixture.artifact_root(&artifact).is_dir());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn recovery_deletes_only_exact_owned_stage_and_preserves_foreign_stage() {
|
||||
let fixture = Fixture::new();
|
||||
|
|
|
|||
Loading…
Reference in a new issue