Merge pull request #175 from vmware-tanzu/multiple-entries-same-image…

…-on-lock Generate pulled ImagesLock based on the file in the Bundle
carvel-dev · Jun 24, 2021 · c1fd21a · c1fd21a
2 parents e22a6ba + 03256cf
commit c1fd21a
Show file tree

Hide file tree

Showing 2 changed files with 101 additions and 12 deletions.
diff --git a/pkg/imgpkg/bundle/images_lock.go b/pkg/imgpkg/bundle/images_lock.go
@@ -96,7 +96,7 @@ func NewImagesLock(imagesLock lockconfig.ImagesLock, imgRetriever ctlimg.ImagesM
 		imageRefs.AddImagesRef(ImageRef{ImageRef: image, IsBundle: nil})
 	}
 
-	imgsLock := &ImagesLock{imageRefs: imageRefs, imgRetriever: imgRetriever, imagesLockLocationsConfig: imagesLockLocationConfig}
+	imgsLock := &ImagesLock{imageRefs: imageRefs, imgRetriever: imgRetriever, imagesLockLocationsConfig: imagesLockLocationConfig, imagesLock: imagesLock}
 	imgsLock.generateImagesLocations(relativeToRepo)
 
 	return imgsLock
@@ -106,6 +106,7 @@ type ImagesLock struct {
 	imageRefs                 ImageRefs
 	imgRetriever              ctlimg.ImagesMetadata
 	imagesLockLocationsConfig ImagesLockLocationsConfig
+	imagesLock                lockconfig.ImagesLock
 }
 
 func (o *ImagesLock) generateImagesLocations(relativeToRepo string) {
@@ -158,9 +159,9 @@ func (o *ImagesLock) LocalizeImagesLock() (ImageRefs, lockconfig.ImagesLock, boo
 	bundleImageRefs := NewImageRefs()
 	imagesLock := lockconfig.NewEmptyImagesLock()
 
-	refs, err := o.ImageRefs()
+	err := o.syncImageRefs()
 	if err != nil {
-		return bundleImageRefs, lockconfig.ImagesLock{}, false, err
+		return ImageRefs{}, lockconfig.ImagesLock{}, false, err
 	}
 
 	_, err = o.imagesLockLocationsConfig.Config()
@@ -169,7 +170,12 @@ func (o *ImagesLock) LocalizeImagesLock() (ImageRefs, lockconfig.ImagesLock, boo
 	if err != nil {
 		skippedLocalization := false
 
-		for _, imgRef := range refs.ImageRefs() {
+		for _, img := range o.imagesLock.Images {
+			imgRef, ok := o.imageRefs.Find(img.Image)
+			if !ok {
+				panic(fmt.Errorf("Internal inconsistency: '%s' could not be found", img.Image))
+			}
+
 			foundImg, err := o.imgRetriever.FirstImageExists(imgRef.Locations())
 			if err != nil {
 				return bundleImageRefs, lockconfig.ImagesLock{}, false, err
@@ -185,7 +191,7 @@ func (o *ImagesLock) LocalizeImagesLock() (ImageRefs, lockconfig.ImagesLock, boo
 
 			lockImgRef := lockconfig.ImageRef{
 				Image:       foundImg,
-				Annotations: imgRef.Annotations,
+				Annotations: img.Annotations,
 			}
 
 			imageRefs = append(imageRefs, lockImgRef)
@@ -200,13 +206,16 @@ func (o *ImagesLock) LocalizeImagesLock() (ImageRefs, lockconfig.ImagesLock, boo
 			bundleImageRefs = NewImageRefs()
 			// Remove the bundle location on all the Images, which is present due to the constructor call to
 			// ImagesLock.generateImagesLocations
-			for _, image := range o.imageRefs.ImageRefs() {
-				lockImgRef := image.DiscardLocationsExcept(image.Image)
+			for _, image := range o.imagesLock.Images {
+				imgRef, ok := o.imageRefs.Find(image.Image)
+				if !ok {
+					panic(fmt.Errorf("Internal inconsistency: '%s' could not be found", image.Image))
+				}
 
-				imageRefs = append(imageRefs, lockImgRef)
+				imageRefs = append(imageRefs, image)
 				bundleImageRefs.AddImagesRef(ImageRef{
-					ImageRef: lockImgRef,
-					IsBundle: image.IsBundle,
+					ImageRef: image,
+					IsBundle: imgRef.IsBundle,
 				})
 			}
 		}
@@ -216,10 +225,15 @@ func (o *ImagesLock) LocalizeImagesLock() (ImageRefs, lockconfig.ImagesLock, boo
 	}
 
 	// Location OCI for bundle was found. Assume that images in images.yml have been relocated to the dst repo.
-	for _, imgRef := range refs.ImageRefs() {
+	for _, img := range o.imagesLock.Images {
+		imgRef, ok := o.imageRefs.Find(img.Image)
+		if !ok {
+			panic(fmt.Errorf("Internal inconsistency: '%s' could not be found", img.Image))
+		}
+
 		lockImgRef := lockconfig.ImageRef{
 			Image:       imgRef.PrimaryLocation(),
-			Annotations: imgRef.Annotations,
+			Annotations: img.Annotations,
 		}
 
 		imageRefs = append(imageRefs, lockImgRef)

diff --git a/pkg/imgpkg/bundle/images_lock_test.go b/pkg/imgpkg/bundle/images_lock_test.go
@@ -94,6 +94,81 @@ func TestImagesLock_LocalizeImagesLock(t *testing.T) {
 		assert.Equal(t, "some.repo.io/img1@sha256:27fde5fa39e3c97cb1e5dabfb664784b605a592d5d2df5482d744742efebba80", bundleImgRefs.ImageRefs()[0].PrimaryLocation())
 		assert.Equal(t, "some.repo.io/img2@sha256:45f3926bca9fc42adb650fef2a41250d77841dde49afc8adc7c0c633b3d5f27a", bundleImgRefs.ImageRefs()[1].PrimaryLocation())
 	})
+
+	t.Run("When one images is present twice without locations OCI Image, it returns the ImagesLock with both images", func(t *testing.T) {
+		imagesLock := lockconfig.ImagesLock{
+			Images: []lockconfig.ImageRef{
+				{
+					Image: "some.repo.io/img1@sha256:27fde5fa39e3c97cb1e5dabfb664784b605a592d5d2df5482d744742efebba80",
+					Annotations: map[string]string{
+						"annot": "1",
+					},
+				},
+				{
+					Image: "some.repo.io/img1@sha256:27fde5fa39e3c97cb1e5dabfb664784b605a592d5d2df5482d744742efebba80",
+					Annotations: map[string]string{
+						"annot": "2",
+					},
+				},
+			},
+		}
+		fakeImagesMetadata := &imagefakes.FakeImagesMetadata{}
+		fakeImagesMetadata.ImageReturns(nil, &transport.Error{
+			StatusCode: http.StatusNotFound,
+		})
+
+		subject := ctlbundle.NewImagesLock(imagesLock, fakeImagesMetadata, "some.repo.io/bundle", config)
+
+		// Other calls will return the default empty Hash and nil error
+		fakeImagesMetadata.DigestReturnsOnCall(1, regv1.Hash{}, errors.New("not found"))
+
+		_, newImagesLock, _, err := subject.LocalizeImagesLock()
+		require.NoError(t, err)
+
+		require.Len(t, newImagesLock.Images, 2)
+		assert.Equal(t, "some.repo.io/img1@sha256:27fde5fa39e3c97cb1e5dabfb664784b605a592d5d2df5482d744742efebba80", newImagesLock.Images[0].PrimaryLocation())
+		assert.Equal(t, "1", newImagesLock.Images[0].Annotations["annot"])
+		assert.Equal(t, "some.repo.io/img1@sha256:27fde5fa39e3c97cb1e5dabfb664784b605a592d5d2df5482d744742efebba80", newImagesLock.Images[1].PrimaryLocation())
+		assert.Equal(t, "2", newImagesLock.Images[1].Annotations["annot"])
+	})
+
+	t.Run("When one images is present twice with locations OCI Image, it returns the ImagesLock with both images", func(t *testing.T) {
+		imagesLock := lockconfig.ImagesLock{
+			Images: []lockconfig.ImageRef{
+				{
+					Image: "some.repo.io/img1@sha256:27fde5fa39e3c97cb1e5dabfb664784b605a592d5d2df5482d744742efebba80",
+					Annotations: map[string]string{
+						"annot": "1",
+					},
+				},
+				{
+					Image: "some.repo.io/img1@sha256:27fde5fa39e3c97cb1e5dabfb664784b605a592d5d2df5482d744742efebba80",
+					Annotations: map[string]string{
+						"annot": "2",
+					},
+				},
+			},
+		}
+		fakeImagesMetadata := &imagefakes.FakeImagesMetadata{}
+		fakeImagesMetadata.ImageReturns(nil, &transport.Error{
+			StatusCode: http.StatusOK,
+		})
+		config.FetchReturns(ctlbundle.ImageLocationsConfig{}, nil)
+
+		subject := ctlbundle.NewImagesLock(imagesLock, fakeImagesMetadata, "some.repo.io/bundle", config)
+
+		// Other calls will return the default empty Hash and nil error
+		fakeImagesMetadata.DigestReturnsOnCall(1, regv1.Hash{}, errors.New("not found"))
+
+		_, newImagesLock, _, err := subject.LocalizeImagesLock()
+		require.NoError(t, err)
+
+		require.Len(t, newImagesLock.Images, 2)
+		assert.Equal(t, "some.repo.io/bundle@sha256:27fde5fa39e3c97cb1e5dabfb664784b605a592d5d2df5482d744742efebba80", newImagesLock.Images[0].PrimaryLocation())
+		assert.Equal(t, "1", newImagesLock.Images[0].Annotations["annot"])
+		assert.Equal(t, "some.repo.io/bundle@sha256:27fde5fa39e3c97cb1e5dabfb664784b605a592d5d2df5482d744742efebba80", newImagesLock.Images[1].PrimaryLocation())
+		assert.Equal(t, "2", newImagesLock.Images[1].Annotations["annot"])
+	})
 }
 
 func TestImagesLock_Merge(t *testing.T) {