Fix: manifest push --rm removes a correct manifest list

This bug is reproduced when we execute the following command:

1. podman manifest add <manifest list> <images exist on local storage>
2. podman manifest push --rm <manifest list> dir:<directory>

If pushing succeeds, it is expected to remove only a manifest list.
However, manifest list remains on local storage and images are removed.

This commit fixes `podman manifest push --rm` to remove only a manifest list.

And, supports `manifest push --rm option` in remote environment,
like host environment.

Fixes: https://github.com/containers/podman/issues/15033

Signed-off-by: Toshiki Sonoda <sonoda.toshiki@fujitsu.com>

3 files changed, 50 insertions, 7 deletions

diff --git a/pkg/domain/infra/abi/manifest.go b/pkg/domain/infra/abi/manifest.go
index 60f727234..6606b2cd0 100644
--- a/pkg/domain/infra/abi/manifest.go
+++ b/pkg/domain/infra/abi/manifest.go
@@ -334,7 +334,8 @@ func (ir *ImageEngine) ManifestPush(ctx context.Context, name, destination strin
 	}
 
 	if opts.Rm {
-		if _, rmErrors := ir.Libpod.LibimageRuntime().RemoveImages(ctx, []string{manifestList.ID()}, nil); len(rmErrors) > 0 {
+		rmOpts := &libimage.RemoveImagesOptions{LookupManifest: true}
+		if _, rmErrors := ir.Libpod.LibimageRuntime().RemoveImages(ctx, []string{manifestList.ID()}, rmOpts); len(rmErrors) > 0 {
 			return "", fmt.Errorf("error removing manifest after push: %w", rmErrors[0])
 		}
 	}
diff --git a/pkg/domain/infra/tunnel/manifest.go b/pkg/domain/infra/tunnel/manifest.go
index d2554f198..4a3148fac 100644
--- a/pkg/domain/infra/tunnel/manifest.go
+++ b/pkg/domain/infra/tunnel/manifest.go
@@ -110,5 +110,15 @@ func (ir *ImageEngine) ManifestPush(ctx context.Context, name, destination strin
 		}
 	}
 	digest, err := manifests.Push(ir.ClientCtx, name, destination, options)
+	if err != nil {
+		return "", fmt.Errorf("error adding to manifest list %s: %w", name, err)
+	}
+
+	if opts.Rm {
+		if _, rmErrors := ir.Remove(ctx, []string{name}, entities.ImageRemoveOptions{LookupManifest: true}); len(rmErrors) > 0 {
+			return "", fmt.Errorf("error removing manifest after push: %w", rmErrors[0])
+		}
+	}
+
 	return digest, err
 }
diff --git a/test/e2e/manifest_test.go b/test/e2e/manifest_test.go
index 1f58419a1..a7fcd1559 100644
--- a/test/e2e/manifest_test.go
+++ b/test/e2e/manifest_test.go
@@ -339,6 +339,16 @@ var _ = Describe("Podman manifest", func() {
 		push = podmanTest.Podman([]string{"manifest", "push", "--tls-verify=false", "--creds=podmantest:wrongpasswd", "foo", "localhost:" + registry.Port + "/credstest"})
 		push.WaitWithDefaultTimeout()
 		Expect(push).To(ExitWithError())
+
+		// push --rm after pull image (#15033)
+		push = podmanTest.Podman([]string{"manifest", "push", "--rm", "--tls-verify=false", "--creds=" + registry.User + ":" + registry.Password, "foo", "localhost:" + registry.Port + "/rmtest"})
+		push.WaitWithDefaultTimeout()
+		Expect(push).Should(Exit(0))
+
+		session = podmanTest.Podman([]string{"images", "-q", "foo"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0))
 	})
 
 	It("push with error", func() {
@@ -348,8 +358,8 @@ var _ = Describe("Podman manifest", func() {
 		Expect(session.ErrorToString()).NotTo(BeEmpty())
 	})
 
-	It("push --rm", func() {
-		SkipIfRemote("remote does not support --rm")
+	It("push --rm to local directory", func() {
+		SkipIfRemote("manifest push to dir not supported in remote mode")
 		session := podmanTest.Podman([]string{"manifest", "create", "foo"})
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(Exit(0))
@@ -365,13 +375,35 @@ var _ = Describe("Podman manifest", func() {
 		session = podmanTest.Podman([]string{"manifest", "push", "--purge", "foo", "dir:" + dest})
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(Exit(0))
-		session = podmanTest.Podman([]string{"manifest", "inspect", "foo"})
+		session = podmanTest.Podman([]string{"images", "-q", "foo"})
 		session.WaitWithDefaultTimeout()
-		Expect(session).To(ExitWithError())
+		Expect(session).Should(Exit(0))
+		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0))
 
-		session = podmanTest.Podman([]string{"manifest", "rm", "foo1", "foo2"})
+		// push --rm after pull image (#15033)
+		session = podmanTest.Podman([]string{"pull", "quay.io/libpod/testdigest_v2s2"})
 		session.WaitWithDefaultTimeout()
-		Expect(session).To(ExitWithError())
+		Expect(session).Should(Exit(0))
+
+		session = podmanTest.Podman([]string{"manifest", "create", "bar"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		session = podmanTest.Podman([]string{"manifest", "add", "bar", "quay.io/libpod/testdigest_v2s2"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		session = podmanTest.Podman([]string{"manifest", "push", "--rm", "bar", "dir:" + dest})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		session = podmanTest.Podman([]string{"images", "-q", "bar"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0))
+
+		session = podmanTest.Podman([]string{"manifest", "rm", "foo", "bar"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(ExitWithError())
+		Expect(session.ErrorToString()).To(ContainSubstring("foo: image not known"))
+		Expect(session.ErrorToString()).To(ContainSubstring("bar: image not known"))
 	})
 
 	It("exists", func() {