Skip to content

Commit

Permalink
When 2 images with same SHA are reference by 2 nested bundles (#638)
Browse files Browse the repository at this point in the history
When there are 2 nested bundles that reference the same image (AKA the
same SHA) these images are considered as separated images internally.
This behavior causes problems when imgpkg is trying to create the
Locations image because it searches for the images by SHA, creating a
dupplicated entry.

To fix this we decided to force imgpkg to select only one of the entries
to add to the Locations list.

This could be averted as well if we stored only 1 copy of these images
in the UnprocessedImagesRef struct the main issue is that this is going
to hide from imgpkg that the same image is referenced from 2 different
sources, this might cause problems in the long run, specially if one day
we decide to implement the ability for users to copy bundles to
different locations in the destination repository.

Signed-off-by: Joao Pereira <joaod@vmware.com>
  • Loading branch information
joaopapereira authored Feb 26, 2024
1 parent 7ae3b0c commit 287c735
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 7 deletions.
19 changes: 12 additions & 7 deletions pkg/imgpkg/bundle/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,20 +126,25 @@ func (o *Bundle) NoteCopy(processedImages *imageset.ProcessedImages, reg ImagesM
APIVersion: LocationAPIVersion,
Kind: ImageLocationsKind,
}
foundImages := map[string]bool{}
var bundleProcessedImage imageset.ProcessedImage
for _, image := range processedImages.All() {
ref, found := o.findCachedImageRef(image.UnprocessedImageRef.DigestRef)
if found {
locationsCfg.Images = append(locationsCfg.Images, ImageLocation{
Image: ref.Image,
IsBundle: *ref.IsBundle,
})
}
imgDigest, err := regname.NewDigest(image.UnprocessedImageRef.DigestRef)
if err != nil {
panic(fmt.Sprintf("Internal inconsistency: Image '%s' is not a valid Digest Reference", err))
}

ref, found := o.findCachedImageRef(image.UnprocessedImageRef.DigestRef)
if found {
if _, ok := foundImages[imgDigest.DigestStr()]; !ok {
locationsCfg.Images = append(locationsCfg.Images, ImageLocation{
Image: ref.Image,
IsBundle: *ref.IsBundle,
})
foundImages[imgDigest.DigestStr()] = true
}
}

if imgDigest.DigestStr() == o.Digest() {
bundleProcessedImage = image
}
Expand Down
7 changes: 7 additions & 0 deletions pkg/imgpkg/imageset/unprocessed_image_refs.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,14 @@ type UnprocessedImageRef struct {
OrigRef string
}

// Key that uniquely identify a ImageRef
func (u UnprocessedImageRef) Key() string {
// With this definition of key if one image is shared by 2 bundles
// but it is referred in the ImagesLock using a different Registry/Repository
// while the SHA is the same, we consider them to be different images, which they are not.
// This will lead to duplication of the image in the UnprocessedImageRef that needs to be
// address by whoever is using it. This should impact performance of copy because ggcr
// will dedupe the Image/Layers based on the SHA.
return u.DigestRef + ":" + u.Tag
}

Expand Down

0 comments on commit 287c735

Please sign in to comment.