From a780521e24892ec357ce26847b5370b6a50cf985 Mon Sep 17 00:00:00 2001
From: "W. Trevor King" <wking@tremily.us>
Date: Tue, 8 Jan 2019 21:46:20 -0800
Subject: [PATCH] libpod/image: Use ParseNormalizedNamed in RepoDigests

Avoid generating
quay.io/openshift-release-dev/ocp-release@sha256@sha256:239... and
similar when the image name is already digest-based [1].  It's not
clear exactly how we get into this state, but as shown by the unit
tests, the new code handles this case correctly (while the previous
code does not).

[1]: https://github.com/containers/libpod/issues/2086

Signed-off-by: W. Trevor King <wking@tremily.us>
---
 cmd/podman/sign.go         |  5 ++++-
 libpod/image/image.go      | 18 ++++++++++++---
 libpod/image/image_test.go | 46 ++++++++++++++++++++++++++++++++++++++
 pkg/varlinkapi/images.go   | 13 +++++++++--
 4 files changed, 76 insertions(+), 6 deletions(-)

diff --git a/cmd/podman/sign.go b/cmd/podman/sign.go
index 790b6031de..8a31ddb98c 100644
--- a/cmd/podman/sign.go
+++ b/cmd/podman/sign.go
@@ -126,7 +126,10 @@ func signCmd(c *cli.Context) error {
 			sigStoreDir = SignatureStoreDir
 		}
 
-		repos := newImage.RepoDigests()
+		repos, err := newImage.RepoDigests()
+		if err != nil {
+			return errors.Wrapf(err, "error calculating repo digests for %s", signimage)
+		}
 		if len(repos) == 0 {
 			logrus.Errorf("no repodigests associated with the image %s", signimage)
 			continue
diff --git a/libpod/image/image.go b/libpod/image/image.go
index 3a6d0e3052..2e12adb706 100644
--- a/libpod/image/image.go
+++ b/libpod/image/image.go
@@ -305,12 +305,24 @@ func (i *Image) Names() []string {
 }
 
 // RepoDigests returns a string array of repodigests associated with the image
-func (i *Image) RepoDigests() []string {
+func (i *Image) RepoDigests() ([]string, error) {
 	var repoDigests []string
+	digest := i.Digest()
+
 	for _, name := range i.Names() {
-		repoDigests = append(repoDigests, strings.SplitN(name, ":", 2)[0]+"@"+i.Digest().String())
+		named, err := reference.ParseNormalizedNamed(name)
+		if err != nil {
+			return nil, err
+		}
+
+		canonical, err := reference.WithDigest(reference.TrimNamed(named), digest)
+		if err != nil {
+			return nil, err
+		}
+
+		repoDigests = append(repoDigests, canonical.String())
 	}
-	return repoDigests
+	return repoDigests, nil
 }
 
 // Created returns the time the image was created
diff --git a/libpod/image/image_test.go b/libpod/image/image_test.go
index 91bb2411b2..2a68fd273b 100644
--- a/libpod/image/image_test.go
+++ b/libpod/image/image_test.go
@@ -9,6 +9,7 @@ import (
 	"testing"
 
 	"github.com/containers/storage"
+	"github.com/opencontainers/go-digest"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -192,6 +193,51 @@ func TestImage_MatchRepoTag(t *testing.T) {
 	cleanup(workdir, ir)
 }
 
+// TestImage_RepoDigests tests RepoDigest generation.
+func TestImage_RepoDigests(t *testing.T) {
+	dgst, err := digest.Parse("sha256:7173b809ca12ec5dee4506cd86be934c4596dd234ee82c0662eac04a8c2c71dc")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	for _, test := range []struct {
+		name     string
+		names    []string
+		expected []string
+	}{
+		{
+			name:     "empty",
+			names:    []string{},
+			expected: nil,
+		},
+		{
+			name:     "tagged",
+			names:    []string{"docker.io/library/busybox:latest"},
+			expected: []string{"docker.io/library/busybox@sha256:7173b809ca12ec5dee4506cd86be934c4596dd234ee82c0662eac04a8c2c71dc"},
+		},
+		{
+			name:     "digest",
+			names:    []string{"docker.io/library/busybox@sha256:7173b809ca12ec5dee4506cd86be934c4596dd234ee82c0662eac04a8c2c71dc"},
+			expected: []string{"docker.io/library/busybox@sha256:7173b809ca12ec5dee4506cd86be934c4596dd234ee82c0662eac04a8c2c71dc"},
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			image := &Image{
+				image: &storage.Image{
+					Names:  test.names,
+					Digest: dgst,
+				},
+			}
+			actual, err := image.RepoDigests()
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			assert.Equal(t, test.expected, actual)
+		})
+	}
+}
+
 // Test_splitString tests the splitString function in image that
 // takes input and splits on / and returns the last array item
 func Test_splitString(t *testing.T) {
diff --git a/pkg/varlinkapi/images.go b/pkg/varlinkapi/images.go
index 5e4cb4ccb0..8f8934025d 100644
--- a/pkg/varlinkapi/images.go
+++ b/pkg/varlinkapi/images.go
@@ -41,13 +41,18 @@ func (i *LibpodAPI) ListImages(call iopodman.VarlinkCall) error {
 	for _, image := range images {
 		labels, _ := image.Labels(getContext())
 		containers, _ := image.Containers()
+		repoDigests, err := image.RepoDigests()
+		if err != nil {
+			return err
+		}
+
 		size, _ := image.Size(getContext())
 
 		i := iopodman.ImageInList{
 			Id:          image.ID(),
 			ParentId:    image.Parent,
 			RepoTags:    image.Names(),
-			RepoDigests: image.RepoDigests(),
+			RepoDigests: repoDigests,
 			Created:     image.Created().String(),
 			Size:        int64(*size),
 			VirtualSize: image.VirtualSize,
@@ -73,6 +78,10 @@ func (i *LibpodAPI) GetImage(call iopodman.VarlinkCall, name string) error {
 	if err != nil {
 		return err
 	}
+	repoDigests, err := newImage.RepoDigests()
+	if err != nil {
+		return err
+	}
 	size, err := newImage.Size(getContext())
 	if err != nil {
 		return err
@@ -82,7 +91,7 @@ func (i *LibpodAPI) GetImage(call iopodman.VarlinkCall, name string) error {
 		Id:          newImage.ID(),
 		ParentId:    newImage.Parent,
 		RepoTags:    newImage.Names(),
-		RepoDigests: newImage.RepoDigests(),
+		RepoDigests: repoDigests,
 		Created:     newImage.Created().String(),
 		Size:        int64(*size),
 		VirtualSize: newImage.VirtualSize,