Merge pull request #372 from vmware-tanzu/correct-transport-for-mount…

…-use-case

Correct transport for mount use case

pkg/imgpkg/cmd/copy_repo_src_test.go

@@ -964,8 +964,8 @@ bundle:
 func TestToRepoImage(t *testing.T) {
 	imageName := "library/image"
 	fakeRegistry := helpers.NewFakeRegistry(t, &helpers.Logger{LogLevel: helpers.LogDebug})
-	image1 := fakeRegistry.WithImageFromPath(imageName, "test_assets/image_with_config", map[string]string{})
 	defer fakeRegistry.CleanUp()
+	image1 := fakeRegistry.WithImageFromPath(imageName, "test_assets/image_with_config", map[string]string{})
 	subject := subject
 	subject.ImageFlags = ImageFlags{
 		fakeRegistry.ReferenceOnTestServer(imageName),
@@ -1054,12 +1054,12 @@ images:
 	})
 
 	t.Run("When copying to same registry but have no permission to mount layer", func(t *testing.T) {
-		t.Skip("Skipping this test because the registry in ggcr does not validate that a blob is part of repository or not, so we will get false positives")
-		// we will need https://github.com/google/go-containerregistry/pull/1158 or something similar to ensure no false positives happen for this test
-		// skipping it for now, but we should review in the future, To run this test use the code in the commit e4c2b6acd5adc569c155a2353bb12b06f46f3e78
 		assets := &helpers.Assets{T: t}
 		defer assets.CleanCreatedFolders()
 
+		fakeRegistry := helpers.NewFakeRegistryWithRepoSeparation(t, &helpers.Logger{LogLevel: helpers.LogDebug})
+		defer fakeRegistry.CleanUp()
+
 		destinationImageName := fakeRegistry.ReferenceOnTestServer("some/other/copied-img")
 		originImageName := "repo/image"
 
@@ -1091,15 +1091,56 @@ images:
 		assert.Equal(t, image2RefDigest, processedImages.All()[0].UnprocessedImageRef.DigestRef)
 	})
 
-	t.Run("When a temporary error happens it retries the configured number of times", func(t *testing.T) {
+	t.Run("When copying to same registry have permission to mount layer", func(t *testing.T) {
 		assets := &helpers.Assets{T: t}
 		defer assets.CleanCreatedFolders()
 
+		fakeRegistry := helpers.NewFakeRegistryWithRepoSeparation(t, &helpers.Logger{LogLevel: helpers.LogDebug})
+		defer fakeRegistry.CleanUp()
+
 		destinationImageName := fakeRegistry.ReferenceOnTestServer("some/other/copied-img")
 		originImageName := "repo/image"
 
 		image2RefDigest := fakeRegistry.WithRandomImage(originImageName).RefDigest
 
+		subject := subject
+		subject.ImageFlags.Image = image2RefDigest
+		subject.registry = fakeRegistry.BuildWithRegistryOpts(registry.Opts{
+			EnvironFunc: func() []string {
+				return []string{
+					"IMGPKG_REGISTRY_HOSTNAME_0=" + fakeRegistry.ReferenceOnTestServer("repo"),
+					"IMGPKG_REGISTRY_USERNAME_0=some-user",
+					"IMGPKG_REGISTRY_PASSWORD_0=some-password",
+					"IMGPKG_REGISTRY_HOSTNAME_1=" + fakeRegistry.ReferenceOnTestServer("some/other"),
+					"IMGPKG_REGISTRY_USERNAME_1=some-user",
+					"IMGPKG_REGISTRY_PASSWORD_1=some-password",
+				}
+			},
+		})
+
+		// Authentication added in this step to ensure the images are created beforehand
+		// because we are not testing here the authentication of image pushing
+		fakeRegistry.WithBasicAuthPerRepository("repo", "some-user", "some-password")
+		fakeRegistry.WithBasicAuthPerRepository("some/other", "some-user", "some-password")
+
+		processedImages, err := subject.CopyToRepo(destinationImageName)
+		require.NoError(t, err)
+		require.Len(t, processedImages.All(), 1)
+		assert.Equal(t, image2RefDigest, processedImages.All()[0].UnprocessedImageRef.DigestRef)
+	})
+
+	t.Run("When a temporary error happens it retries the configured number of times", func(t *testing.T) {
+		assets := &helpers.Assets{T: t}
+		defer assets.CleanCreatedFolders()
+
+		fakeRegistry := helpers.NewFakeRegistry(t, &helpers.Logger{LogLevel: helpers.LogDebug})
+		defer fakeRegistry.CleanUp()
+
+		destinationImageName := fakeRegistry.ReferenceOnTestServer("some/other/copied-img")
+		originImageName := "temp-repo/image"
+
+		image2RefDigest := fakeRegistry.WithRandomImage(originImageName).RefDigest
+
 		subject := subject
 		subject.ImageFlags.Image = image2RefDigest
 		subject.registry = fakeRegistry.BuildWithRegistryOpts(registry.Opts{

pkg/imgpkg/imagedesc/described_compressed_layer.go

@@ -44,7 +44,7 @@ func (l DescribedCompressedLayer) Compressed() (io.ReadCloser, error) {
 		return nil, fmt.Errorf("Computing digest: %v", err)
 	}
 
-	rc, err = verify.ReadCloser(rc, h)
+	rc, err = verify.ReadCloser(rc, verify.SizeUnknown, h)
 	if err != nil {
 		return nil, fmt.Errorf("Creating verified reader: %v", err)
 	}

pkg/imgpkg/imageutils/verify/verify.go

@@ -1,8 +1,3 @@
-// Copyright 2020 VMware, Inc.
-// SPDX-License-Identifier: Apache-2.0
-
-// Using this code as is from: https://github.com/google/go-containerregistry/tree/master/pkg/v1/internal
-
 // Copyright 2020 Google LLC All Rights Reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
@@ -17,10 +12,14 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// Package verify provides a ReadCloser that verifies content matches the
+// expected hash values.
 package verify
 
 import (
+	"bytes"
 	"encoding/hex"
+	"errors"
 	"fmt"
 	"hash"
 	"io"
@@ -29,39 +28,95 @@ import (
 	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/imageutils/and"
 )
 
+// SizeUnknown is a sentinel value to indicate that the expected size is not known.
+const SizeUnknown = -1
+
 type verifyReader struct {
-	inner    io.Reader
-	hasher   hash.Hash
-	expected v1.Hash
+	inner             io.Reader
+	hasher            hash.Hash
+	expected          v1.Hash
+	gotSize, wantSize int64
+}
+
+// Error provides information about the failed hash verification.
+type Error struct {
+	got     string
+	want    v1.Hash
+	gotSize int64
+}
+
+func (v Error) Error() string {
+	return fmt.Sprintf("error verifying %s checksum after reading %d bytes; got %q, want %q",
+		v.want.Algorithm, v.gotSize, v.got, v.want)
 }
 
 // Read implements io.Reader
 func (vc *verifyReader) Read(b []byte) (int, error) {
 	n, err := vc.inner.Read(b)
+	vc.gotSize += int64(n)
 	if err == io.EOF {
-		got := hex.EncodeToString(vc.hasher.Sum(make([]byte, 0, vc.hasher.Size())))
+		if vc.wantSize != SizeUnknown && vc.gotSize != vc.wantSize {
+			return n, fmt.Errorf("error verifying size; got %d, want %d", vc.gotSize, vc.wantSize)
+		}
+		got := hex.EncodeToString(vc.hasher.Sum(nil))
 		if want := vc.expected.Hex; got != want {
-			return n, fmt.Errorf("error verifying %s checksum; got %q, want %q",
-				vc.expected.Algorithm, got, want)
+			return n, Error{
+				got:     vc.expected.Algorithm + ":" + got,
+				want:    vc.expected,
+				gotSize: vc.gotSize,
+			}
 		}
 	}
 	return n, err
 }
 
 // ReadCloser wraps the given io.ReadCloser to verify that its contents match
 // the provided v1.Hash before io.EOF is returned.
-func ReadCloser(r io.ReadCloser, h v1.Hash) (io.ReadCloser, error) {
+//
+// The reader will only be read up to size bytes, to prevent resource
+// exhaustion. If EOF is returned before size bytes are read, an error is
+// returned.
+//
+// A size of SizeUnknown (-1) indicates disables size verification when the size
+// is unknown ahead of time.
+func ReadCloser(r io.ReadCloser, size int64, h v1.Hash) (io.ReadCloser, error) {
 	w, err := v1.Hasher(h.Algorithm)
 	if err != nil {
 		return nil, err
 	}
-	r2 := io.TeeReader(r, w)
+	r2 := io.TeeReader(r, w) // pass all writes to the hasher.
+	if size != SizeUnknown {
+		r2 = io.LimitReader(r2, size) // if we know the size, limit to that size.
+	}
 	return &and.ReadCloser{
 		Reader: &verifyReader{
 			inner:    r2,
 			hasher:   w,
 			expected: h,
+			wantSize: size,
 		},
 		CloseFunc: r.Close,
 	}, nil
 }
+
+// Descriptor verifies that the embedded Data field matches the Size and Digest
+// fields of the given v1.Descriptor, returning an error if the Data field is
+// missing or if it contains incorrect data.
+func Descriptor(d v1.Descriptor) error {
+	if d.Data == nil {
+		return errors.New("error verifying descriptor; Data == nil")
+	}
+
+	h, sz, err := v1.SHA256(bytes.NewReader(d.Data))
+	if err != nil {
+		return err
+	}
+	if h != d.Digest {
+		return fmt.Errorf("error verifying Digest; got %q, want %q", h, d.Digest)
+	}
+	if sz != d.Size {
+		return fmt.Errorf("error verifying Size; got %d, want %d", sz, d.Size)
+	}
+
+	return nil
+}

pkg/imgpkg/imageutils/verify/verify_test.go

@@ -1,7 +1,7 @@
 // Copyright 2020 VMware, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
-// Using this code as is from: https://github.com/google/go-containerregistry/tree/master/pkg/v1/internal
+// Using this code as is from: https://github.com/google/go-containerregistry/tree/main/internal
 
 // Copyright 2020 Google LLC All Rights Reserved.
 //
@@ -21,6 +21,8 @@ package verify
 
 import (
 	"bytes"
+	"errors"
+	"fmt"
 	"io/ioutil"
 	"strings"
 	"testing"
@@ -33,14 +35,15 @@ func mustHash(s string, t *testing.T) v1.Hash {
 	if err != nil {
 		t.Fatalf("v1.SHA256(%s) = %v", s, err)
 	}
+	t.Logf("Hashed: %q -> %q", s, h)
 	return h
 }
 
 func TestVerificationFailure(t *testing.T) {
 	want := "This is the input string."
 	buf := bytes.NewBufferString(want)
 
-	verified, err := ReadCloser(ioutil.NopCloser(buf), mustHash("not the same", t))
+	verified, err := ReadCloser(ioutil.NopCloser(buf), int64(len(want)), mustHash("not the same", t))
 	if err != nil {
 		t.Fatal("ReadCloser() =", err)
 	}
@@ -53,7 +56,20 @@ func TestVerification(t *testing.T) {
 	want := "This is the input string."
 	buf := bytes.NewBufferString(want)
 
-	verified, err := ReadCloser(ioutil.NopCloser(buf), mustHash(want, t))
+	verified, err := ReadCloser(ioutil.NopCloser(buf), int64(len(want)), mustHash(want, t))
+	if err != nil {
+		t.Fatal("ReadCloser() =", err)
+	}
+	if _, err := ioutil.ReadAll(verified); err != nil {
+		t.Error("ReadAll() =", err)
+	}
+}
+
+func TestVerificationSizeUnknown(t *testing.T) {
+	want := "This is the input string."
+	buf := bytes.NewBufferString(want)
+
+	verified, err := ReadCloser(ioutil.NopCloser(buf), SizeUnknown, mustHash(want, t))
 	if err != nil {
 		t.Fatal("ReadCloser() =", err)
 	}
@@ -67,8 +83,70 @@ func TestBadHash(t *testing.T) {
 		Algorithm: "fake256",
 		Hex:       "whatever",
 	}
-	_, err := ReadCloser(ioutil.NopCloser(strings.NewReader("hi")), h)
+	_, err := ReadCloser(ioutil.NopCloser(strings.NewReader("hi")), 0, h)
 	if err == nil {
 		t.Errorf("ReadCloser() = %v, wanted err", err)
 	}
 }
+
+func TestBadSize(t *testing.T) {
+	want := "This is the input string."
+
+	// having too much content or expecting too much content returns an error.
+	for _, size := range []int64{3, 100} {
+		t.Run(fmt.Sprintf("expecting size %d", size), func(t *testing.T) {
+			buf := bytes.NewBufferString(want)
+			rc, err := ReadCloser(ioutil.NopCloser(buf), size, mustHash(want, t))
+			if err != nil {
+				t.Fatal("ReadCloser() =", err)
+			}
+			if b, err := ioutil.ReadAll(rc); err == nil {
+				t.Errorf("ReadAll() = %q; want verification error", string(b))
+			}
+		})
+	}
+}
+
+func TestDescriptor(t *testing.T) {
+	for _, tc := range []struct {
+		err  error
+		desc v1.Descriptor
+	}{{
+		err: errors.New("error verifying descriptor; Data == nil"),
+	}, {
+		err: errors.New(`error verifying Digest; got "sha256:ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad", want ":"`),
+		desc: v1.Descriptor{
+			Data: []byte("abc"),
+		},
+	}, {
+		err: errors.New("error verifying Size; got 3, want 0"),
+		desc: v1.Descriptor{
+			Data: []byte("abc"),
+			Digest: v1.Hash{
+				Algorithm: "sha256",
+				Hex:       "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad",
+			},
+		},
+	}, {
+		desc: v1.Descriptor{
+			Data: []byte("abc"),
+			Size: 3,
+			Digest: v1.Hash{
+				Algorithm: "sha256",
+				Hex:       "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad",
+			},
+		},
+	}} {
+		got, want := Descriptor(tc.desc), tc.err
+
+		if got == nil {
+			if want != nil {
+				t.Errorf("Descriptor(): got nil, want %v", want)
+			}
+		} else if want == nil {
+			t.Errorf("Descriptor(): got %v, want nil", got)
+		} else if got, want := got.Error(), want.Error(); got != want {
+			t.Errorf("Descriptor(): got %q, want %q", got, want)
+		}
+	}
+}

pkg/imgpkg/registry/registry.go

@@ -83,12 +83,19 @@ type ImagesReaderWriter interface {
 
 var _ Registry = &SimpleRegistry{}
 
+// RoundTripperStorage Storage of RoundTripper that will be used to talk to the registry
+type RoundTripperStorage interface {
+	RoundTripper(repo regname.Repository, scope string) http.RoundTripper
+	CreateRoundTripper(reg regname.Registry, auth regauthn.Authenticator, scope string) (http.RoundTripper, error)
+	BaseRoundTripper() http.RoundTripper
+}
+
 // SimpleRegistry Implements Registry interface
 type SimpleRegistry struct {
 	remoteOpts      []regremote.Option
 	refOpts         []regname.Option
 	keychain        regauthn.Keychain
-	roundTrippers   *RoundTripperStorage
+	roundTrippers   RoundTripperStorage
 	transportAccess *sync.Mutex
 }
 
@@ -150,7 +157,7 @@ func NewSimpleRegistry(opts Opts, regOpts ...regremote.Option) (*SimpleRegistry,
 		remoteOpts:      regRemoteOptions,
 		refOpts:         refOpts,
 		keychain:        keychain,
-		roundTrippers:   NewRoundTripperStorage(baseRoundTripper),
+		roundTrippers:   NewMultiRoundTripperStorage(baseRoundTripper),
 		transportAccess: &sync.Mutex{},
 	}, nil
 }
@@ -165,12 +172,16 @@ func (r SimpleRegistry) CloneWithSingleAuth(imageRef regname.Tag) (Registry, err
 	}
 
 	keychain := auth.NewSingleAuthKeychain(imgAuth)
+	rt := r.roundTrippers.RoundTripper(imageRef.Repository, imageRef.Scope(transport.PullScope))
+	if rt == nil {
+		rt = r.roundTrippers.BaseRoundTripper()
+	}
 
 	return &SimpleRegistry{
 		remoteOpts:      r.remoteOpts,
 		refOpts:         r.refOpts,
 		keychain:        keychain,
-		roundTrippers:   r.roundTrippers,
+		roundTrippers:   NewSingleTripperStorage(rt),
 		transportAccess: &sync.Mutex{},
 	}, nil
 }