Skip to content

Commit a00d856

Browse files
author
Roy Lin
committed
fix(box): retagged/rebuilt image becomes a prunable dangling (no leak)
Re-pointing a tag at a new digest (rebuild -t foo, or tag overwrite) used to silently overwrite the index entry, dropping the displaced image from view AND orphaning its on-disk layout forever (a real disk leak — nothing could reach it to delete it). - store.put: when a reference already maps to a DIFFERENT digest that loses its last reference, re-key the displaced image under its digest so it survives as a dangling entry — visible in `images`, removable by `image prune` (which frees the disk). Same-digest re-puts don't spawn a spurious dangling. - images: render a digest-keyed (dangling) reference as `<none> <none>` like Docker, instead of splitting the digest into repository=sha256 / tag=<hex>. Verified on KVM: build myimg:v1, rebuild with changed content -> old shows as <none> <none>; `image-prune --force` removes exactly it and frees 3.7 MB; the live tag survives. New unit tests cover retag->dangling, same-digest no-op, and the <none> <none> rendering.
1 parent 1cb9bb8 commit a00d856

2 files changed

Lines changed: 99 additions & 8 deletions

File tree

src/cli/src/commands/images.rs

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -179,13 +179,19 @@ struct ImageRow {
179179

180180
impl ImageRow {
181181
fn from_stored(image: &a3s_box_runtime::StoredImage) -> Self {
182-
let (repository, tag) = match a3s_box_runtime::ImageReference::parse(&image.reference) {
183-
Ok(r) => {
184-
let repo = format!("{}/{}", r.registry, r.repository);
185-
let tag = r.tag.unwrap_or_else(|| "<none>".to_string());
186-
(repo, tag)
182+
let (repository, tag) = if crate::image_usage::is_dangling_reference(&image.reference) {
183+
// Dangling image (digest-keyed, no repo:tag) — Docker renders it as
184+
// `<none> <none>` rather than splitting the digest into repo + tag.
185+
("<none>".to_string(), "<none>".to_string())
186+
} else {
187+
match a3s_box_runtime::ImageReference::parse(&image.reference) {
188+
Ok(r) => {
189+
let repo = format!("{}/{}", r.registry, r.repository);
190+
let tag = r.tag.unwrap_or_else(|| "<none>".to_string());
191+
(repo, tag)
192+
}
193+
Err(_) => (image.reference.clone(), "<none>".to_string()),
187194
}
188-
Err(_) => (image.reference.clone(), "<none>".to_string()),
189195
};
190196

191197
// Format digest: "sha256:" prefix + first 12 hex chars
@@ -352,11 +358,27 @@ mod tests {
352358

353359
#[test]
354360
fn test_from_stored_invalid_reference_fallback() {
355-
// Empty reference should fail to parse, falling back to raw reference
361+
// An empty reference is treated as dangling and rendered `<none> <none>`,
362+
// matching Docker's display for untagged images.
356363
let stored = sample_stored("", "sha256:abc", 100);
357364
let row = ImageRow::from_stored(&stored);
358365

359-
assert_eq!(row.repository, "");
366+
assert_eq!(row.repository, "<none>");
367+
assert_eq!(row.tag, "<none>");
368+
}
369+
370+
#[test]
371+
fn test_from_stored_digest_keyed_dangling_renders_none_none() {
372+
// A digest-keyed dangling image (displaced by a re-tag) renders as
373+
// `<none> <none>`, not repository="sha256" / tag=<64-char hex>.
374+
let stored = sample_stored(
375+
"sha256:6c3a24f143efe95ba48765929f1cf75cef0eb96c784f85d588660440796fccbb",
376+
"sha256:6c3a24f143efe95ba48765929f1cf75cef0eb96c784f85d588660440796fccbb",
377+
100,
378+
);
379+
let row = ImageRow::from_stored(&stored);
380+
381+
assert_eq!(row.repository, "<none>");
360382
assert_eq!(row.tag, "<none>");
361383
}
362384

src/runtime/src/oci/store.rs

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,26 @@ impl ImageStore {
150150
};
151151

152152
let mut index = self.index.write().await;
153+
154+
// Docker parity: if this reference already points at a DIFFERENT digest
155+
// and that old digest is about to lose its last reference, keep the
156+
// displaced image as a dangling entry (keyed by its digest) instead of
157+
// dropping it. This makes a rebuilt/re-tagged image show up as
158+
// `<none>` in `images`, be removable by `image prune`, and prevents
159+
// silently orphaning its on-disk layout.
160+
if let Some(old) = index.get(reference).cloned() {
161+
if old.digest != digest {
162+
let still_referenced = index
163+
.iter()
164+
.any(|(key, img)| key.as_str() != reference && img.digest == old.digest);
165+
if !still_referenced && !index.contains_key(&old.digest) {
166+
let mut dangling = old.clone();
167+
dangling.reference = old.digest.clone();
168+
index.insert(old.digest.clone(), dangling);
169+
}
170+
}
171+
}
172+
153173
index.insert(reference.to_string(), stored.clone());
154174
drop(index);
155175

@@ -463,6 +483,55 @@ mod tests {
463483
assert!(store.get("nginx:latest").await.is_none());
464484
}
465485

486+
#[tokio::test]
487+
async fn test_retag_keeps_displaced_image_as_dangling() {
488+
// Docker parity: re-pointing a tag at a new digest leaves the old image
489+
// as a dangling entry (keyed by its digest), not silently dropped.
490+
let tmp = TempDir::new().unwrap();
491+
let store_dir = tmp.path().join("store");
492+
let source_dir = tmp.path().join("source");
493+
create_test_oci_layout(&source_dir);
494+
495+
let store = ImageStore::new(&store_dir, 10 * 1024 * 1024).unwrap();
496+
store
497+
.put("app:latest", "sha256:old", &source_dir)
498+
.await
499+
.unwrap();
500+
store
501+
.put("app:latest", "sha256:new", &source_dir)
502+
.await
503+
.unwrap();
504+
505+
// The tag now resolves to the new digest...
506+
assert_eq!(store.get("app:latest").await.unwrap().digest, "sha256:new");
507+
// ...and the displaced image survives as a digest-keyed dangling entry.
508+
let dangling = store.get("sha256:old").await.unwrap();
509+
assert_eq!(dangling.digest, "sha256:old");
510+
assert_eq!(store.list().await.len(), 2);
511+
}
512+
513+
#[tokio::test]
514+
async fn test_reput_same_digest_does_not_create_dangling() {
515+
// Re-putting the same reference at the SAME digest (e.g. pulling latest
516+
// when content is unchanged) must not spawn a spurious dangling entry.
517+
let tmp = TempDir::new().unwrap();
518+
let store_dir = tmp.path().join("store");
519+
let source_dir = tmp.path().join("source");
520+
create_test_oci_layout(&source_dir);
521+
522+
let store = ImageStore::new(&store_dir, 10 * 1024 * 1024).unwrap();
523+
store
524+
.put("app:latest", "sha256:same", &source_dir)
525+
.await
526+
.unwrap();
527+
store
528+
.put("app:latest", "sha256:same", &source_dir)
529+
.await
530+
.unwrap();
531+
532+
assert_eq!(store.list().await.len(), 1);
533+
}
534+
466535
#[tokio::test]
467536
async fn test_remove_by_digest() {
468537
// CRI RemoveImage identifies the image by its ID (sha256 digest),

0 commit comments

Comments
 (0)