Merge pull request #484 from carvel-dev/change-registry-creation

Add new constructor to the registry

pkg/imgpkg/registry/registry.go

@@ -131,6 +131,15 @@ type SimpleRegistry struct {
 	transportAccess *sync.Mutex
 }
 
+// NewBasicRegistry does not provide any special behavior and all the options as passed as is to the underlying library
+func NewBasicRegistry(regOpts ...regremote.Option) (*SimpleRegistry, error) {
+	return &SimpleRegistry{
+		remoteOpts:      regOpts,
+		roundTrippers:   NewNoopRoundTripperStorage(),
+		transportAccess: &sync.Mutex{},
+	}, nil
+}
+
 // NewSimpleRegistry Builder for a Simple Registry
 func NewSimpleRegistry(opts Opts) (*SimpleRegistry, error) {
 	httpTran, err := newHTTPTransport(opts)
@@ -141,7 +150,7 @@ func NewSimpleRegistry(opts Opts) (*SimpleRegistry, error) {
 }
 
 // NewSimpleRegistryWithTransport Creates a new Simple Registry using the provided transport
-func NewSimpleRegistryWithTransport(opts Opts, rTripper http.RoundTripper, regOpts ...regremote.Option) (*SimpleRegistry, error) {
+func NewSimpleRegistryWithTransport(opts Opts, rTripper http.RoundTripper) (*SimpleRegistry, error) {
 	var refOpts []regname.Option
 	if opts.Insecure {
 		refOpts = append(refOpts, regname.Insecure)
@@ -166,9 +175,6 @@ func NewSimpleRegistryWithTransport(opts Opts, rTripper http.RoundTripper, regOp
 	if opts.IncludeNonDistributableLayers {
 		regRemoteOptions = append(regRemoteOptions, regremote.WithNondistributable)
 	}
-	if regOpts != nil {
-		regRemoteOptions = append(regRemoteOptions, regOpts...)
-	}
 	tries := opts.RetryCount
 	if tries == 0 {
 		tries = 1
@@ -201,10 +207,14 @@ func NewSimpleRegistryWithTransport(opts Opts, rTripper http.RoundTripper, regOp
 	}, nil
 }
 
-// CloneWithSingleAuth Clones the provided registry replacing the Keychain with a Keychain that can only authenticate
-// the image provided
+// CloneWithSingleAuth produces a copy of this Registry whose keychain has exactly one auth — the one that can be used
+// to access imageRef. If no keychain is explicitly configured on this Registry, the copy is a BasicRegistry.
 // A Registry need to be provided as the first parameter or the function will panic
 func (r SimpleRegistry) CloneWithSingleAuth(imageRef regname.Tag) (Registry, error) {
+	if r.keychain == nil { // If no keychain is present it assumes NewBasicRegistry was used to create the Registry. So we short circuit this execution
+		return NewBasicRegistry(r.remoteOpts...)
+	}
+
 	imgAuth, err := r.keychain.Resolve(imageRef)
 	if err != nil {
 		return nil, err
@@ -216,11 +226,16 @@ func (r SimpleRegistry) CloneWithSingleAuth(imageRef regname.Tag) (Registry, err
 		rt = r.roundTrippers.BaseRoundTripper()
 	}
 
+	var singleRt RoundTripperStorage = NewNoopRoundTripperStorage()
+	if rt != nil {
+		singleRt = NewSingleTripperStorage(rt)
+	}
+
 	return &SimpleRegistry{
 		remoteOpts:      r.remoteOpts,
 		refOpts:         r.refOpts,
 		keychain:        keychain,
-		roundTrippers:   NewSingleTripperStorage(rt),
+		roundTrippers:   singleRt,
 		authn:           map[string]regauthn.Authenticator{},
 		transportAccess: &sync.Mutex{},
 	}, nil
@@ -273,6 +288,10 @@ func (r *SimpleRegistry) transport(ref regname.Reference, scope string) (http.Ro
 
 	rt := r.roundTrippers.RoundTripper(registry, scope)
 	if rt == nil {
+		if r.keychain == nil {
+			return nil, nil, nil
+		}
+
 		resolvedAuth, err := r.keychain.Resolve(registry)
 		if err != nil {
 			return nil, nil, fmt.Errorf("Unable retrieve credentials for registry: %s", err)

pkg/imgpkg/registry/registry_test.go

@@ -244,10 +244,72 @@ func TestInsecureRegistryFlag(t *testing.T) {
 	}
 }
 
+func TestBasicRegistry(t *testing.T) {
+	t.Run("When transport is provided it uses it", func(t *testing.T) {
+		rt := &notFoundRoundTripper{do: func(request *http.Request) (*http.Response, error) {
+			return &http.Response{
+				Status:     "Not Found",
+				StatusCode: http.StatusNotFound,
+			}, errors.New("not found")
+		}}
+		reg, err := registry.NewBasicRegistry(regremote.WithTransport(rt))
+		require.NoError(t, err)
+		ref, err := name.ParseReference("localhost:1111/does/not/exist")
+		require.NoError(t, err)
+		_, err = reg.Get(ref)
+		require.Error(t, err)
+		require.Equal(t, 2, rt.RoundTripNumCalls)
+	})
+
+	t.Run("When cloned with CloneWithLogger it still uses initial transport", func(t *testing.T) {
+		rt := &notFoundRoundTripper{do: func(request *http.Request) (*http.Response, error) {
+			return &http.Response{
+				Status:     "Not Found",
+				StatusCode: http.StatusNotFound,
+			}, errors.New("not found")
+		}}
+		reg, err := registry.NewBasicRegistry(regremote.WithTransport(rt))
+		require.NoError(t, err)
+
+		cReg := reg.CloneWithLogger(nil)
+		ref, err := name.ParseReference("localhost:1111/does/not/exist")
+		require.NoError(t, err)
+		_, err = cReg.Get(ref)
+		require.Error(t, err)
+		require.Equal(t, 2, rt.RoundTripNumCalls)
+	})
+
+	t.Run("When cloned with CloneWithAuth it still uses initial transport", func(t *testing.T) {
+		rt := &notFoundRoundTripper{do: func(request *http.Request) (*http.Response, error) {
+			return &http.Response{
+				Status:     "Not Found",
+				StatusCode: http.StatusNotFound,
+			}, errors.New("not found")
+		}}
+		reg, err := registry.NewBasicRegistry(regremote.WithTransport(rt))
+		require.NoError(t, err)
+
+		tag, err := name.NewTag("localhost:1111/some:tag")
+		require.NoError(t, err)
+
+		cReg, err := reg.CloneWithSingleAuth(tag)
+		require.NoError(t, err)
+
+		ref, err := name.ParseReference("localhost:1111/does/not/exist")
+		require.NoError(t, err)
+
+		_, err = cReg.Get(ref)
+		require.Error(t, err)
+		require.Equal(t, 2, rt.RoundTripNumCalls)
+	})
+}
+
 type notFoundRoundTripper struct {
-	do func(request *http.Request) (*http.Response, error)
+	RoundTripNumCalls int
+	do                func(request *http.Request) (*http.Response, error)
 }
 
 func (n *notFoundRoundTripper) RoundTrip(request *http.Request) (*http.Response, error) {
+	n.RoundTripNumCalls++
 	return n.do(request)
 }

pkg/imgpkg/registry/transport.go

@@ -15,13 +15,6 @@ import (
 	"github.com/google/go-containerregistry/pkg/v1/remote/transport"
 )
 
-// MultiRoundTripperStorage Maintains a storage of all the available RoundTripper for different registries and repositories
-type MultiRoundTripperStorage struct {
-	baseRoundTripper http.RoundTripper
-	transports       map[string]map[string]map[string]http.RoundTripper
-	readWriteAccess  *sync.Mutex
-}
-
 // NewMultiRoundTripperStorage Creates a struct that holds RoundTripper
 func NewMultiRoundTripperStorage(baseRoundTripper http.RoundTripper) *MultiRoundTripperStorage {
 	return &MultiRoundTripperStorage{
@@ -39,6 +32,18 @@ func NewSingleTripperStorage(baseRoundTripper http.RoundTripper) *SingleTripperS
 	}
 }
 
+// NewNoopRoundTripperStorage Creates a struct that does not save any RoundTripper
+func NewNoopRoundTripperStorage() *NoopRoundTripperStorage {
+	return &NoopRoundTripperStorage{}
+}
+
+// MultiRoundTripperStorage Maintains a storage of all the available RoundTripper for different registries and repositories
+type MultiRoundTripperStorage struct {
+	baseRoundTripper http.RoundTripper
+	transports       map[string]map[string]map[string]http.RoundTripper
+	readWriteAccess  *sync.Mutex
+}
+
 // BaseRoundTripper retrieves the base RoundTripper used by the store
 func (r MultiRoundTripperStorage) BaseRoundTripper() http.RoundTripper {
 	return r.baseRoundTripper
@@ -146,3 +151,21 @@ func (r *SingleTripperStorage) CreateRoundTripper(reg regname.Registry, auth aut
 
 	return rt, nil
 }
+
+// NoopRoundTripperStorage does not store any http.RoundTripper
+type NoopRoundTripperStorage struct{}
+
+// RoundTripper returns nil to all invocations
+func (n NoopRoundTripperStorage) RoundTripper(regname.Repository, string) http.RoundTripper {
+	return nil
+}
+
+// CreateRoundTripper does nothing
+func (n NoopRoundTripperStorage) CreateRoundTripper(reg regname.Registry, auth authn.Authenticator, scope string) (http.RoundTripper, error) {
+	return nil, nil
+}
+
+// BaseRoundTripper returns nil to all invocations
+func (n NoopRoundTripperStorage) BaseRoundTripper() http.RoundTripper {
+	return nil
+}

pkg/imgpkg/v1/pull.go

@@ -89,7 +89,11 @@ func Pull(imageRef string, outputPath string, pullOptions PullOpts, registryOpts
 	if err != nil {
 		return PullStatus{}, err
 	}
+	return PullWithRegistry(imageRef, outputPath, pullOptions, reg)
+}
 
+// PullWithRegistry Download the contents of the image referenced by imageRef to the folder outputPath
+func PullWithRegistry(imageRef string, outputPath string, pullOptions PullOpts, reg registry.Registry) (PullStatus, error) {
 	bundleToPull := bundle.NewBundle(imageRef, reg)
 	isBundle, err := bundleToPull.IsBundle()
 	if err != nil {
@@ -129,6 +133,12 @@ func PullRecursive(imageRef string, outputPath string, pullOptions PullOpts, reg
 		return PullStatus{}, err
 	}
 
+	return PullRecursiveWithRegistry(imageRef, outputPath, pullOptions, reg)
+}
+
+// PullRecursiveWithRegistry Downloads the contents of the Bundle and Nested Bundles referenced by imageRef to the folder outputPath.
+// This functions should error out when imageRef does not point to a Bundle
+func PullRecursiveWithRegistry(imageRef string, outputPath string, pullOptions PullOpts, reg registry.Registry) (PullStatus, error) {
 	bundleToPull := bundle.NewBundle(imageRef, reg)
 	isBundle, err := bundleToPull.IsBundle()
 	if err != nil {