From a0eb84541aa13bb4a833c1e938304caf67c3c750 Mon Sep 17 00:00:00 2001
From: Paul Holzinger <pholzing@redhat.com>
Date: Fri, 20 Oct 2023 12:07:11 +0200
Subject: [PATCH] libimage: Untag should error for non existent name

podman untag should error out of a name is given which does not exists
for the given image. This regression was added in commit 465dd3e8be.

There was even a test which meant to check for it but unfortunately it
did not actually check for what it should. The doNotExist check failed
early to the upper case in the repo name.
The tests have been updated to check for actual error messages to show
ensure it is failing for the right reason.
This also showed that `normalizing name` message was included twice so
I removed one case to not stutter.

Fixes 465dd3e8be ("libimage: support parallel tag/untag")

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
---
 libimage/image.go      | 18 +++++++++++++++++-
 libimage/image_test.go | 23 ++++++++++++-----------
 2 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/libimage/image.go b/libimage/image.go
index f5b7fb918..cc77fc34e 100644
--- a/libimage/image.go
+++ b/libimage/image.go
@@ -587,6 +587,9 @@ func (i *Image) Tag(name string) error {
 	return i.reload()
 }
 
+// to have some symmetry with the errors from containers/storage.
+var errTagUnknown = errors.New("tag not known")
+
 // TODO (@vrothberg) - `docker rmi sha256:` will remove the digest from the
 // image.  However, that's something containers storage does not support.
 var errUntagDigest = errors.New("untag by digest not supported")
@@ -601,7 +604,7 @@ func (i *Image) Untag(name string) error {
 
 	ref, err := NormalizeName(name)
 	if err != nil {
-		return fmt.Errorf("normalizing name %q: %w", name, err)
+		return err
 	}
 
 	// FIXME: this is breaking Podman CI but must be re-enabled once
@@ -616,6 +619,19 @@ func (i *Image) Untag(name string) error {
 
 	name = ref.String()
 
+	foundName := false
+	for _, n := range i.Names() {
+		if n == name {
+			foundName = true
+			break
+		}
+	}
+	// Return an error if the name is not found, the c/storage
+	// RemoveNames() API does not create one if no match is found.
+	if !foundName {
+		return fmt.Errorf("%s: %w", name, errTagUnknown)
+	}
+
 	logrus.Debugf("Untagging %q from image %s", ref.String(), i.ID())
 	if i.runtime.eventChannel != nil {
 		defer i.runtime.writeEvent(&Event{ID: i.ID(), Name: name, Time: time.Now(), Type: EventTypeImageUntag})
diff --git a/libimage/image_test.go b/libimage/image_test.go
index 6edc59ecf..f12a546ad 100644
--- a/libimage/image_test.go
+++ b/libimage/image_test.go
@@ -361,15 +361,16 @@ func TestUntag(t *testing.T) {
 	for _, test := range []struct {
 		tag         string
 		untag       string
-		expectError bool
+		expectError string
 	}{
-		{"foo", "foo", false},
-		{"foo", "foo:latest", false},
-		{"foo", "localhost/foo", false},
-		{"foo", "localhost/foo:latest", false},
-		{"quay.io/image/foo", "quay.io/image/foo", false},
-		{"foo", "doNotExist", true},
-		{"foo", digest, true},
+		{"foo", "foo", ""},
+		{"foo", "foo:latest", ""},
+		{"foo", "localhost/foo", ""},
+		{"foo", "localhost/foo:latest", ""},
+		{"quay.io/image/foo", "quay.io/image/foo", ""},
+		{"foo", "upperCase", "normalizing name \"upperCase\": repository name must be lowercase"},
+		{"foo", "donotexist", "localhost/donotexist:latest: tag not known"},
+		{"foo", digest, digest + ": untag by digest not supported"},
 		//		{"foo", "foo@" + digest, false},
 		//		{"foo", "localhost/foo@" + digest, false},
 	} {
@@ -377,8 +378,8 @@ func TestUntag(t *testing.T) {
 		require.NoError(t, err, "tag should have succeeded: %v", test)
 
 		err = image.Untag(test.untag)
-		if test.expectError {
-			require.Error(t, err, "untag should have failed: %v", test)
+		if test.expectError != "" {
+			require.EqualError(t, err, test.expectError, "untag should have failed: %v", test)
 			continue
 		}
 		require.NoError(t, err, "untag should have succeedded: %v", test)
@@ -388,7 +389,7 @@ func TestUntag(t *testing.T) {
 
 	// Check for specific error.
 	err := image.Untag(digest)
-	require.True(t, errors.Is(err, errUntagDigest), "check for specific digest error")
+	require.ErrorIs(t, err, errUntagDigest, "check for specific digest error")
 }
 
 func getImageAndRuntime(t *testing.T) (*Runtime, *Image, func()) {