Merge pull request #301 from vmware-tanzu/tar-copy-checks-origin-regi…

…stry

Do not access origin registry when copying from tar

pkg/imgpkg/bundle/bundle_images_lock.go

@@ -43,16 +43,18 @@ func (o *Bundle) AllImagesRefs(concurrency int, ui util.UIWithLevels) ([]*Bundle
 	return bundles, allImageRefs, err
 }
 
-// UpdateImageRefs updates the bundle cached images
-func (o *Bundle) UpdateImageRefs(bundles []*Bundle, ui util.UIWithLevels) error {
+// UpdateImageRefs updates the bundle cached images without talking to the registry
+func (o *Bundle) UpdateImageRefs(bundles []*Bundle) error {
 	o.cachedImageRefs = map[string]ImageRef{}
 
 	img, err := o.checkedImage()
 	if err != nil {
 		return err
 	}
 
-	imageRefsToProcess, err := o.fetchImagesRef(img, ui)
+	// Call fetchImagesRef with a NotFoundLocationsConfig because this function should only be used
+	// in the copy from tar to repository
+	imageRefsToProcess, err := o.fetchImagesRef(img, &NotFoundLocationsConfig{})
 	if err != nil {
 		return fmt.Errorf("Fetching images of %s: %s", o.DigestRef(), err)
 	}
@@ -79,7 +81,17 @@ func (o *Bundle) buildAllImagesLock(throttleReq *util.Throttle, processedImgs *p
 		return nil, ImageRefs{}, err
 	}
 
-	imageRefsToProcess, err := o.fetchImagesRef(img, ui)
+	bundleDigestRef, err := regname.NewDigest(o.DigestRef())
+	if err != nil {
+		panic(fmt.Sprintf("Internal inconsistency: The Bundle Reference '%s' does not have a digest", o.DigestRef()))
+	}
+
+	locationsConfig := LocationsConfig{
+		ui:              ui,
+		imgRetriever:    o.imgRetriever,
+		bundleDigestRef: bundleDigestRef,
+	}
+	imageRefsToProcess, err := o.fetchImagesRef(img, &locationsConfig)
 	if err != nil {
 		return nil, ImageRefs{}, err
 	}
@@ -137,12 +149,7 @@ func (o *Bundle) buildAllImagesLock(throttleReq *util.Throttle, processedImgs *p
 	return bundles, processedImageRefs, nil
 }
 
-func (o *Bundle) fetchImagesRef(img regv1.Image, ui util.UIWithLevels) (ImageRefs, error) {
-	bundleDigestRef, err := regname.NewDigest(o.DigestRef())
-	if err != nil {
-		panic(fmt.Sprintf("Internal inconsistency: The Bundle Reference '%s' does not have a digest", o.DigestRef()))
-	}
-
+func (o *Bundle) fetchImagesRef(img regv1.Image, locationsConfig ImageRefLocationsConfig) (ImageRefs, error) {
 	// Reads the ImagesLock of the bundle because this is the source of truth
 	imagesLock, err := o.imagesLockReader.Read(img)
 	if err != nil {
@@ -151,11 +158,7 @@ func (o *Bundle) fetchImagesRef(img regv1.Image, ui util.UIWithLevels) (ImageRef
 
 	// We use ImagesLock struct only to add the bundle repository to the list of locations
 	// maybe we can move this functionality to the bundle in the future
-	refs, err := NewImageRefsFromImagesLock(imagesLock, LocationsConfig{
-		ui:              ui,
-		imgRetriever:    o.imgRetriever,
-		bundleDigestRef: bundleDigestRef,
-	})
+	refs, err := NewImageRefsFromImagesLock(imagesLock, locationsConfig)
 	if err != nil {
 		return ImageRefs{}, err
 	}
@@ -275,3 +278,11 @@ type LocationsConfig struct {
 func (l LocationsConfig) Config() (ImageLocationsConfig, error) {
 	return NewLocations(l.ui).Fetch(l.imgRetriever, l.bundleDigestRef)
 }
+
+// NotFoundLocationsConfig Noop Locations Configuration retrieval
+type NotFoundLocationsConfig struct{}
+
+// Config Returns a LocationsNotFound error
+func (l NotFoundLocationsConfig) Config() (ImageLocationsConfig, error) {
+	return ImageLocationsConfig{}, &LocationsNotFound{}
+}

pkg/imgpkg/bundle/images_refs.go

@@ -50,6 +50,7 @@ func NewImageRefs() ImageRefs {
 	}
 }
 
+// NewImageRefsFromImagesLock Create a new ImageRefs from the provided lockconfig.ImagesLock and ImageLocationsConfig
 func NewImageRefsFromImagesLock(imagesLock lockconfig.ImagesLock, imageRefsLocationConfig ImageRefLocationsConfig) (ImageRefs, error) {
 	imageRefs := ImageRefs{
 		refsLock:           &sync.Mutex{},

pkg/imgpkg/cmd/copy_repo_src.go

@@ -96,7 +96,7 @@ func (c CopyRepoSrc) CopyToRepo(repo string) (*ctlimgset.ProcessedImages, error)
 		}
 
 		for _, bundle := range bundles {
-			if err := bundle.UpdateImageRefs(bundles, c.ui); err != nil {
+			if err := bundle.UpdateImageRefs(bundles); err != nil {
 				return nil, fmt.Errorf("Updating Image Refs %s: %s", bundle.DigestRef(), err)
 			}
 

pkg/imgpkg/cmd/copy_repo_src_test.go

@@ -738,7 +738,55 @@ func TestToRepoBundleCreatesValidLocationOCI(t *testing.T) {
 			require.NoError(t, validateImagesPresenceInRegistry(t, refs))
 		})
 	}
+}
+
+func TestToRepoFromTar(t *testing.T) {
+	logger := &helpers.Logger{LogLevel: helpers.LogDebug}
+	fakeRegistry := helpers.NewFakeRegistry(t, logger)
+	defer fakeRegistry.CleanUp()
 
+	bundleWithOneImages := fakeRegistry.WithBundleFromPath("library/bundle", "test_assets/bundle_with_mult_images").
+		WithImageRefs([]lockconfig.ImageRef{
+			{Image: "hello-world@sha256:ebf526c198a14fa138634b9746c50ec38077ec9b3986227e79eb837d26f59dc6"},
+		})
+
+	bundleWithNestedBundle := fakeRegistry.WithBundleFromPath("library/bundle-with-nested-bundle",
+		"test_assets/bundle_with_mult_images").WithImageRefs([]lockconfig.ImageRef{
+		{Image: bundleWithOneImages.RefDigest},
+	})
+
+	subject := subject
+	subject.BundleFlags.Bundle = bundleWithNestedBundle.RefDigest
+	subject.registry = fakeRegistry.Build()
+
+	t.Run("When copying from tar do not try to reach to the original registry", func(t *testing.T) {
+		assets := &helpers.Assets{T: t}
+		defer assets.CleanCreatedFolders()
+
+		tmpFolder := assets.CreateTempFolder("tar-valid-oci-image")
+		tarFile := filepath.Join(tmpFolder, "bundle.tar")
+
+		subject := subject
+		subject.registry = fakeRegistry.Build()
+
+		logger.Section("create Tar file with bundle", func() {
+			err := subject.CopyToTar(tarFile)
+			require.NoError(t, err)
+		})
+
+		fakeRegistry.CleanUp()
+		destFakeRegistry := helpers.NewFakeRegistry(t, logger)
+		defer destFakeRegistry.CleanUp()
+		subject.registry = destFakeRegistry.Build()
+		destRepo := destFakeRegistry.ReferenceOnTestServer("library/bundle-copy")
+
+		logger.Section("copy bundle from Tar file to Repository", func() {
+			subject.BundleFlags.Bundle = ""
+			subject.TarFlags.TarSrc = tarFile
+			_, err := subject.CopyToRepo(destRepo)
+			require.NoError(t, err)
+		})
+	})
 }
 
 func TestToRepoBundleRunTwiceCreatesValidLocationOCI(t *testing.T) {

test/e2e/copy_from_tar_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/google/go-containerregistry/pkg/name"
 	"github.com/google/go-containerregistry/pkg/v1/remote"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/lockconfig"
 	"github.com/vmware-tanzu/carvel-imgpkg/test/helpers"
 )
@@ -151,4 +152,35 @@ func TestCopyTarSrc(t *testing.T) {
 
 		imgpkg.Run([]string{"copy", "--tar", tempBundleTarFile, "--to-repo", fakeRegistry.ReferenceOnTestServer("copied-bundle")})
 	})
+
+	t.Run("When there is no longer access to the origin registry the copy is successful", func(t *testing.T) {
+		env := helpers.BuildEnv(t)
+		imgpkg := helpers.Imgpkg{T: t, L: helpers.Logger{}, ImgpkgPath: env.ImgpkgPath}
+		defer env.Cleanup()
+
+		destinationFakeRegistry := helpers.NewFakeRegistry(t, &helpers.Logger{LogLevel: helpers.LogDebug})
+		defer destinationFakeRegistry.CleanUp()
+
+		var tempBundleTarFile string
+		originFakeRegistry := helpers.NewFakeRegistry(t, &helpers.Logger{LogLevel: helpers.LogDebug})
+		defer originFakeRegistry.CleanUp()
+		logger.Section("create tar from bundle", func() {
+			randomImage := originFakeRegistry.WithRandomImage("repo/randomimage")
+			bundleInfo := originFakeRegistry.WithBundleFromPath("repo/bundle", "assets/bundle").WithImageRefs([]lockconfig.ImageRef{
+				{Image: randomImage.RefDigest},
+			})
+
+			originFakeRegistry.Build()
+
+			tempBundleTarDir := env.Assets.CreateTempFolder("bundle-tar")
+			tempBundleTarFile = filepath.Join(tempBundleTarDir, "bundle-tar.tgz")
+			imgpkg.Run([]string{"copy", "-b", bundleInfo.RefDigest, "--to-tar", tempBundleTarFile})
+		})
+
+		// Log all the requests done to the origin registry
+		requestLog := originFakeRegistry.WithRequestLogging()
+
+		imgpkg.Run([]string{"copy", "--tar", tempBundleTarFile, "--to-repo", destinationFakeRegistry.ReferenceOnTestServer("copied-bundle")})
+		require.Equal(t, 0, requestLog.Len(), "Requests where sent to the origin registry")
+	})
 }

test/helpers/fake_registry.go

@@ -213,6 +213,14 @@ func (h *HTTPRequestLogs) Last() HTTPRequestLog {
 	return h.requests[len(h.requests)-1]
 }
 
+// Len Length of request logs
+func (h *HTTPRequestLogs) Len() int {
+	h.lock.Lock()
+	defer h.lock.Unlock()
+
+	return len(h.requests)
+}
+
 // WithRequestLogging enables the logging of the HTTP requests sent to the registry
 func (r *FakeTestRegistryBuilder) WithRequestLogging() *HTTPRequestLogs {
 	httpRequestLog := NewHTTPRequestLogs()