Repository: vmware-tanzu/carvel-imgpkg (go)

Issue:
multiple active keychain selection does not work
**Describe the problem/challenge you have**

```
$ IMGPKG_ACTIVE_KEYCHAINS=gke,ecr imgpkg pull -i anything -o /tmp/lol
imgpkg: Error: Creating registry keychain:
  Unable to load keychain for , available keychains [aks, ecr, gke, github]
```

it seems that we are picking up empty string instead of env value when comma is present: https://github.com/vmware-tanzu/carvel-imgpkg/blob/d5c9d0c9a8c62293f371fcd7b8d772dab215f795/pkg/imgpkg/cmd/registry_flags.go#L85. this means i believe that we can only support one active keychain at a time.

**Describe the solution you'd like**

abillity to support multiple active keychains

**Anything else you would like to add:**
[Additional information that will assist in solving the issue.]

---
Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible"
👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help work on this issue.


Pull request:
Keychains parsing was parsing the wrong variable
Fixes #439 

Patch:
diff --git a/pkg/imgpkg/cmd/registry_flags.go b/pkg/imgpkg/cmd/registry_flags.go
index b533dc1f..572d1b8d 100644
--- a/pkg/imgpkg/cmd/registry_flags.go
+++ b/pkg/imgpkg/cmd/registry_flags.go
@@ -82,7 +82,7 @@ func (r *RegistryFlags) AsRegistryOpts() registry.Opts {
 	if found {
 		if len(keychains) > 0 {
 			if strings.Contains(keychains, ",") {
-				for _, keychainName := range strings.Split(r.ActiveKeychains, ",") {
+				for _, keychainName := range strings.Split(keychains, ",") {
 					opts.ActiveKeychains = append(opts.ActiveKeychains, auth.IAASKeychain(strings.TrimSpace(keychainName)))
 				}
 			} else {
diff --git a/pkg/imgpkg/cmd/tag_list.go b/pkg/imgpkg/cmd/tag_list.go
index b35ba7c0..cef17e00 100644
--- a/pkg/imgpkg/cmd/tag_list.go
+++ b/pkg/imgpkg/cmd/tag_list.go
@@ -6,9 +6,8 @@ package cmd
 import (
 	"github.com/cppforlife/go-cli-ui/ui"
 	uitable "github.com/cppforlife/go-cli-ui/ui/table"
-	regname "github.com/google/go-containerregistry/pkg/name"
 	"github.com/spf13/cobra"
-	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/registry"
+	v1 "github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/v1"
 )
 
 type TagListOptions struct {
@@ -38,17 +37,7 @@ func NewTagListCmd(o *TagListOptions) *cobra.Command {
 }
 
 func (t *TagListOptions) Run() error {
-	reg, err := registry.NewSimpleRegistry(t.RegistryFlags.AsRegistryOpts())
-	if err != nil {
-		return err
-	}
-
-	ref, err := regname.ParseReference(t.ImageFlags.Image, regname.WeakValidation)
-	if err != nil {
-		return err
-	}
-
-	tags, err := reg.ListTags(ref.Context())
+	tagInfo, err := v1.TagList(t.ImageFlags.Image, t.Digests, t.RegistryFlags.AsRegistryOpts())
 	if err != nil {
 		return err
 	}
@@ -70,26 +59,10 @@ func (t *TagListOptions) Run() error {
 		},
 	}
 
-	for _, tag := range tags {
-		var digest string
-
-		if t.Digests {
-			tagRef, err := regname.NewTag(ref.Context().String()+":"+tag, regname.WeakValidation)
-			if err != nil {
-				return err
-			}
-
-			hash, err := reg.Digest(tagRef)
-			if err != nil {
-				return err
-			}
-
-			digest = hash.String()
-		}
-
+	for _, tag := range tagInfo.Tags {
 		table.Rows = append(table.Rows, []uitable.Value{
-			uitable.NewValueString(tag),
-			uitable.NewValueString(digest),
+			uitable.NewValueString(tag.Tag),
+			uitable.NewValueString(tag.Digest),
 		})
 	}
 
diff --git a/pkg/imgpkg/v1/tag.go b/pkg/imgpkg/v1/tag.go
new file mode 100644
index 00000000..3f35b673
--- /dev/null
+++ b/pkg/imgpkg/v1/tag.go
@@ -0,0 +1,69 @@
+// Copyright 2022 VMware, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package v1
+
+import (
+	regname "github.com/google/go-containerregistry/pkg/name"
+	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/registry"
+)
+
+// TagInfo Contains the tag name and the digest associated with the tag
+// TagInfo.Digest might be empty if caller ask for it not to be retrieved
+type TagInfo struct {
+	Tag    string
+	Digest string
+}
+
+// TagsInfo Contains all the tags associated with the repository on Image
+type TagsInfo struct {
+	Repository string
+	Tags       []TagInfo
+}
+
+// TagList Retrieve all the tags associated with a repository
+// imageRef contains the address for the repository
+// getDigests when set to true, provides
+func TagList(imageRef string, getDigests bool, registryOpts registry.Opts) (TagsInfo, error) {
+	reg, err := registry.NewSimpleRegistry(registryOpts)
+	if err != nil {
+		return TagsInfo{}, err
+	}
+
+	ref, err := regname.ParseReference(imageRef, regname.WeakValidation)
+	if err != nil {
+		return TagsInfo{}, err
+	}
+
+	tags, err := reg.ListTags(ref.Context())
+	if err != nil {
+		return TagsInfo{}, err
+	}
+
+	tagList := TagsInfo{
+		Repository: ref.Context().String(),
+	}
+
+	for _, tag := range tags {
+		tagInfo := TagInfo{
+			Tag: tag,
+		}
+
+		if getDigests {
+			tagRef, err := regname.NewTag(ref.Context().String()+":"+tag, regname.WeakValidation)
+			if err != nil {
+				return TagsInfo{}, err
+			}
+
+			hash, err := reg.Digest(tagRef)
+			if err != nil {
+				return TagsInfo{}, err
+			}
+
+			tagInfo.Digest = hash.String()
+		}
+		tagList.Tags = append(tagList.Tags, tagInfo)
+	}
+
+	return tagList, nil
+}


Test patch:
diff --git a/pkg/imgpkg/v1/tag_test.go b/pkg/imgpkg/v1/tag_test.go
new file mode 100644
index 00000000..2954f471
--- /dev/null
+++ b/pkg/imgpkg/v1/tag_test.go
@@ -0,0 +1,104 @@
+// Copyright 2022 VMware, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package v1_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/registry"
+	v1 "github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/v1"
+	"github.com/vmware-tanzu/carvel-imgpkg/test/helpers"
+)
+
+func TestTagList(t *testing.T) {
+	fakeRegistry := helpers.NewFakeRegistry(t, &helpers.Logger{LogLevel: helpers.LogDebug})
+
+	img1 := fakeRegistry.WithRandomImage("some/image-1")
+	img21 := fakeRegistry.WithRandomImage("some/image-2")
+	img22 := fakeRegistry.WithRandomImage("some/image-2")
+	// This image needs to be last because it will get the latest tag
+	img2 := fakeRegistry.WithRandomImage("some/image-2")
+	fakeRegistry.Tag(img21.RefDigest, "tag-2-1")
+	fakeRegistry.Tag(img22.RefDigest, "tag-2-2")
+
+	defer fakeRegistry.CleanUp()
+	fakeRegistry.Build()
+
+	t.Run("when only latest tag is present, it returns 1 tag", func(t *testing.T) {
+		tagList, err := v1.TagList(img1.RefDigest, false, registry.Opts{})
+		require.NoError(t, err)
+
+		require.Equal(t, v1.TagsInfo{
+			Repository: fakeRegistry.ReferenceOnTestServer("some/image-1"),
+			Tags: []v1.TagInfo{
+				{
+					Tag:    "latest",
+					Digest: "",
+				},
+			},
+		}, tagList)
+	})
+
+	t.Run("when only latest tag is present and getDigests is set to true, it returns 1 tag and the associated digest", func(t *testing.T) {
+		tagList, err := v1.TagList(img1.RefDigest, true, registry.Opts{})
+		require.NoError(t, err)
+
+		require.Equal(t, v1.TagsInfo{
+			Repository: fakeRegistry.ReferenceOnTestServer("some/image-1"),
+			Tags: []v1.TagInfo{
+				{
+					Tag:    "latest",
+					Digest: img1.Digest,
+				},
+			},
+		}, tagList)
+	})
+
+	t.Run("when multiple tags are present, it returns all tags", func(t *testing.T) {
+		tagList, err := v1.TagList(img2.RefDigest, false, registry.Opts{})
+		require.NoError(t, err)
+
+		require.Equal(t, v1.TagsInfo{
+			Repository: fakeRegistry.ReferenceOnTestServer("some/image-2"),
+			Tags: []v1.TagInfo{
+				{
+					Tag:    "latest",
+					Digest: "",
+				},
+				{
+					Tag:    "tag-2-1",
+					Digest: "",
+				},
+				{
+					Tag:    "tag-2-2",
+					Digest: "",
+				},
+			},
+		}, tagList)
+	})
+
+	t.Run("when multiple tags are present and getDigests is set to true, it returns all tags and the associated digests", func(t *testing.T) {
+		tagList, err := v1.TagList(img2.RefDigest, true, registry.Opts{})
+		require.NoError(t, err)
+
+		require.Equal(t, v1.TagsInfo{
+			Repository: fakeRegistry.ReferenceOnTestServer("some/image-2"),
+			Tags: []v1.TagInfo{
+				{
+					Tag:    "latest",
+					Digest: img2.Digest,
+				},
+				{
+					Tag:    "tag-2-1",
+					Digest: img21.Digest,
+				},
+				{
+					Tag:    "tag-2-2",
+					Digest: img22.Digest,
+				},
+			},
+		}, tagList)
+	})
+}
diff --git a/test/e2e/auth_error_test.go b/test/e2e/auth_error_test.go
index 08c4e2f3..b03ac75e 100644
--- a/test/e2e/auth_error_test.go
+++ b/test/e2e/auth_error_test.go
@@ -35,3 +35,29 @@ func TestAuthErr(t *testing.T) {
 	require.Error(t, err)
 	assert.Contains(t, errOut, "incorrect username or password")
 }
+
+func TestKeychainErrors(t *testing.T) {
+	env := helpers.BuildEnv(t)
+	imgpkg := helpers.Imgpkg{T: t, ImgpkgPath: env.ImgpkgPath}
+	t.Run("when providing multiple keychains the failure", func(t *testing.T) {
+
+		var stderrBs bytes.Buffer
+		_, err := imgpkg.RunWithOpts([]string{
+			"pull", "-i", "some-image", "-o", "something",
+		}, helpers.RunOpts{AllowError: true, EnvVars: []string{"IMGPKG_ACTIVE_KEYCHAINS=gke,aks"}, StderrWriter: &stderrBs})
+
+		require.Error(t, err)
+		require.Contains(t, stderrBs.String(), "UNAUTHORIZED: authentication required")
+	})
+
+	t.Run("keychain provider name is unknown", func(t *testing.T) {
+
+		var stderrBs bytes.Buffer
+		_, err := imgpkg.RunWithOpts([]string{
+			"pull", "-i", "some-image", "-o", "something",
+		}, helpers.RunOpts{AllowError: true, EnvVars: []string{"IMGPKG_ACTIVE_KEYCHAINS=gke,random-name"}, StderrWriter: &stderrBs})
+
+		require.Error(t, err)
+		require.Contains(t, stderrBs.String(), "Unable to load keychain for random-name, available keychains [aks, ecr, gke, github]")
+	})
+}