Skip to content

Commit d912cfa

Browse files
authored
digest only regression fix (#643)
Signed-off-by: Adam Martin <adam.martin@ranchergovernment.com>
1 parent 7d00a53 commit d912cfa

4 files changed

Lines changed: 110 additions & 2 deletions

File tree

cmd/hauler/cli/store/lifecycle_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,63 @@ func TestLifecycle_Chart_AddSaveLoadExtract(t *testing.T) {
241241
}
242242
}
243243

244+
// TestLifecycle_DigestOnlyImage_AddSaveLoad exercises the full save/load round-trip
245+
// for an image added by digest only (no tag). Before fix #642, the image disappeared
246+
// from index.json after LoadCmd because CopyAll appended a double-@ digest.
247+
func TestLifecycle_DigestOnlyImage_AddSaveLoad(t *testing.T) {
248+
ctx := newTestContext(t)
249+
250+
// Step 1: seed a tagged image so we can get its digest
251+
srcHost, srcOpts := newLocalhostRegistry(t)
252+
srcImg := seedImage(t, srcHost, "lifecycle/digestonly", "v1", srcOpts...)
253+
hash, err := srcImg.Digest()
254+
if err != nil {
255+
t.Fatalf("srcImg.Digest: %v", err)
256+
}
257+
258+
// Step 2: add BY DIGEST (not tag) into store A
259+
storeA := newTestStore(t)
260+
rso := defaultRootOpts(storeA.Root)
261+
ro := defaultCliOpts()
262+
digestRef := srcHost + "/lifecycle/digestonly@" + hash.String()
263+
if err := storeImage(ctx, storeA, v1.Image{Name: digestRef}, "", false, rso, ro, ""); err != nil {
264+
t.Fatalf("storeImage by digest: %v", err)
265+
}
266+
// The image should be findable by its digest hex
267+
assertArtifactInStore(t, storeA, hash.Hex)
268+
269+
// Flush index.json for SaveCmd
270+
if err := storeA.SaveIndex(); err != nil {
271+
t.Fatalf("SaveIndex: %v", err)
272+
}
273+
274+
// Step 3: SaveCmd -> archive
275+
archivePath := filepath.Join(t.TempDir(), "lifecycle-digestonly.tar.zst")
276+
saveOpts := newSaveOpts(storeA.Root, archivePath)
277+
if err := SaveCmd(ctx, saveOpts, defaultRootOpts(storeA.Root), defaultCliOpts()); err != nil {
278+
t.Fatalf("SaveCmd: %v", err)
279+
}
280+
281+
// Step 4: LoadCmd -> fresh store B
282+
storeBDir := t.TempDir()
283+
loadOpts := &flags.LoadOpts{
284+
StoreRootOpts: defaultRootOpts(storeBDir),
285+
FileName: []string{archivePath},
286+
}
287+
if err := LoadCmd(ctx, loadOpts, defaultRootOpts(storeBDir), defaultCliOpts()); err != nil {
288+
t.Fatalf("LoadCmd: %v", err)
289+
}
290+
291+
storeB, err := store.NewLayout(storeBDir)
292+
if err != nil {
293+
t.Fatalf("store.NewLayout(storeB): %v", err)
294+
}
295+
296+
// Regression assertion: the digest-only image must survive the save/load round-trip.
297+
// Before fix 1, the image disappears from the loaded store's index.json.
298+
assertArtifactInStore(t, storeB, hash.Hex)
299+
}
300+
244301
// TestLifecycle_Remove_ThenSave verifies that removing one artifact from a store
245302
// with two file artifacts, then saving/loading, results in only the retained
246303
// artifact being present.

cmd/hauler/cli/store/save.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,20 @@ func (x *exports) record(ctx context.Context, index libv1.ImageIndex, desc libv1
322322
slices.Sort(xd.RepoTags)
323323
xd.RepoTags = slices.Compact(xd.RepoTags)
324324
ref = tag.Digest(digest)
325+
case name.Digest:
326+
// For digest-only refs, derive a deterministic, docker-valid tag from
327+
// the manifest digest so ctr/docker can import the image (#642).
328+
// Convention mirrors copy.go:229: "sha256-<hex>".
329+
named, err := referencev3.ParseNormalizedNamed(tag.Repository.Name())
330+
if err != nil {
331+
return err
332+
}
333+
familiarRepo := referencev3.FamiliarName(named)
334+
digestTag := strings.ReplaceAll(digest, ":", "-") // e.g. "sha256-498a..."
335+
repotag := familiarRepo + ":" + digestTag
336+
xd.RepoTags = append(xd.RepoTags[:], repotag)
337+
slices.Sort(xd.RepoTags)
338+
xd.RepoTags = slices.Compact(xd.RepoTags)
325339
}
326340

327341
l.Debugf("image [%s]: type=%s, size=%d", ref.Name(), desc.MediaType, desc.Size)

cmd/hauler/cli/store/save_test.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,37 @@ func TestWriteExportsManifest(t *testing.T) {
113113
})
114114
}
115115

116+
func TestWriteExportsManifest_DigestOnlyImageHasRepoTag(t *testing.T) {
117+
ctx := newTestContext(t)
118+
119+
// Seed a tagged image so we can reference it by digest
120+
host, srcOpts := newLocalhostRegistry(t)
121+
img := seedImage(t, host, "test/digestonly", "v1", srcOpts...)
122+
hash, err := img.Digest()
123+
if err != nil {
124+
t.Fatalf("img.Digest: %v", err)
125+
}
126+
127+
// Add the image BY DIGEST
128+
s := newTestStore(t)
129+
if err := s.AddImage(ctx, host+"/test/digestonly@"+hash.String(), "", false); err != nil {
130+
t.Fatalf("AddImage by digest: %v", err)
131+
}
132+
133+
if err := writeExportsManifest(ctx, s.Root, ""); err != nil {
134+
t.Fatalf("writeExportsManifest: %v", err)
135+
}
136+
137+
entries := readManifestJSON(t, s.Root)
138+
if len(entries) != 1 {
139+
t.Fatalf("expected 1 manifest entry, got %d", len(entries))
140+
}
141+
// Before fix 2, digest-only refs fall through the switch without setting RepoTags.
142+
if len(entries[0].RepoTags) == 0 {
143+
t.Errorf("expected at least one RepoTag for digest-only image, got none")
144+
}
145+
}
146+
116147
func TestWriteExportsManifest_SkipsNonImages(t *testing.T) {
117148
ctx := newTestContext(t)
118149

pkg/store/store.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -753,9 +753,15 @@ func (l *Layout) CopyAll(ctx context.Context, to content.Target, toMapper func(s
753753
toRef = tr
754754
}
755755

756-
// Append the digest to help the target pusher identify the root descriptor
757-
// Format: "reference@digest" allows the pusher to update its index.json
756+
// Append the digest to help the target pusher identify the root descriptor.
757+
// AnnotationRefName for digest-only images already ends in "@sha256:...".
758+
// Strip any existing digest before appending the authoritative descriptor
759+
// digest so the destination pusher can match the root manifest. A double "@"
760+
// yields a digest the pusher never matches, leaving the image unindexed (#642).
758761
if desc.Digest.Validate() == nil {
762+
if at := strings.Index(toRef, "@"); at != -1 {
763+
toRef = toRef[:at]
764+
}
759765
toRef = fmt.Sprintf("%s@%s", toRef, desc.Digest)
760766
}
761767

0 commit comments

Comments
 (0)