Prune digest references when deleting by tag

When pulling an image with content trust enabled, two references are
created: a digest reference and a tag reference. Deleting by tag
wouldn't actually remove the image, because the digest reference keeps
it alive.

This change modifies the rmi logic so that digest references don't keep
an image alive. If the last tag referencing a given image is deleted,
any digest references to it will be removed as well, so the image can
actually get deleted. This fixes the usability problem with deletions
when content trust is in use, so something like "docker pull busybox;
docker rmi busybox" will work as expected.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>

daemon/image_delete.go

@@ -90,8 +90,34 @@ func (daemon *Daemon) ImageDelete(imageRef string, force, prune bool) ([]types.I
 		daemon.LogImageEvent(imgID.String(), imgID.String(), "untag")
 		records = append(records, untaggedRecord)
 
-		// If has remaining references then untag finishes the remove
-		if len(repoRefs) > 1 {
+		repoRefs = daemon.referenceStore.References(imgID)
+
+		// If this is a tag reference and all the remaining references
+		// to this image are digest references, delete the remaining
+		// references so that they don't prevent removal of the image.
+		if _, isCanonical := parsedRef.(reference.Canonical); !isCanonical {
+			foundTagRef := false
+			for _, repoRef := range repoRefs {
+				if _, repoRefIsCanonical := repoRef.(reference.Canonical); !repoRefIsCanonical {
+					foundTagRef = true
+					break
+				}
+			}
+			if !foundTagRef {
+				for _, repoRef := range repoRefs {
+					if _, err := daemon.removeImageRef(repoRef); err != nil {
+						return records, err
+					}
+
+					untaggedRecord := types.ImageDelete{Untagged: repoRef.String()}
+					records = append(records, untaggedRecord)
+				}
+				repoRefs = []reference.Named{}
+			}
+		}
+
+		// If it has remaining references then the untag finished the remove
+		if len(repoRefs) > 0 {
 			return records, nil
 		}
 

docs/reference/commandline/rmi.md

@@ -19,8 +19,9 @@ parent = "smn_cli"
       --no-prune           Do not delete untagged parents
 
 You can remove an image using its short or long ID, its tag, or its digest. If
-an image has one or more tag or digest reference, you must remove all of them
-before the image is removed.
+an image has one or more tag referencing it, you must remove all of them before
+the image is removed. Digest references are removed automatically when an image
+is removed by tag.
 
     $ docker images
     REPOSITORY                TAG                 IMAGE ID            CREATED             SIZE

integration-cli/docker_cli_by_digest_test.go

@@ -370,6 +370,35 @@ func (s *DockerRegistrySuite) TestDeleteImageByIDOnlyPulledByDigest(c *check.C)
 	dockerCmd(c, "rmi", imageID)
 }
 
+func (s *DockerRegistrySuite) TestDeleteImageWithDigestAndTag(c *check.C) {
+	pushDigest, err := setupImage(c)
+	c.Assert(err, checker.IsNil, check.Commentf("error setting up image"))
+
+	// pull from the registry using the <name>@<digest> reference
+	imageReference := fmt.Sprintf("%s@%s", repoName, pushDigest)
+	dockerCmd(c, "pull", imageReference)
+
+	imageID, err := inspectField(imageReference, "Id")
+	c.Assert(err, checker.IsNil, check.Commentf("error inspecting image id"))
+
+	repoTag := repoName + ":sometag"
+	repoTag2 := repoName + ":othertag"
+	dockerCmd(c, "tag", imageReference, repoTag)
+	dockerCmd(c, "tag", imageReference, repoTag2)
+
+	dockerCmd(c, "rmi", repoTag2)
+
+	// rmi should have deleted only repoTag2, because there's another tag
+	_, err = inspectField(repoTag, "Id")
+	c.Assert(err, checker.IsNil, check.Commentf("repoTag should not have been removed"))
+
+	dockerCmd(c, "rmi", repoTag)
+
+	// rmi should have deleted the tag, the digest reference, and the image itself
+	_, err = inspectField(imageID, "Id")
+	c.Assert(err, checker.NotNil, check.Commentf("image should have been deleted"))
+}
+
 // TestPullFailsWithAlteredManifest tests that a `docker pull` fails when
 // we have modified a manifest blob and its digest cannot be verified.
 func (s *DockerRegistrySuite) TestPullFailsWithAlteredManifest(c *check.C) {

integration-cli/docker_cli_pull_trusted_test.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os/exec"
+	"strings"
 	"time"
 
 	"github.com/docker/docker/pkg/integration/checker"
@@ -200,3 +201,55 @@ func (s *DockerTrustSuite) TestTrustedOfflinePull(c *check.C) {
 	c.Assert(err, check.IsNil, check.Commentf(out))
 	c.Assert(string(out), checker.Contains, "Tagging", check.Commentf(out))
 }
+
+func (s *DockerTrustSuite) TestTrustedPullDelete(c *check.C) {
+	repoName := fmt.Sprintf("%v/dockercli/%s:latest", privateRegistryURL, "trusted-pull-delete")
+	// tag the image and upload it to the private registry
+	_, err := buildImage(repoName, `
+                    FROM busybox
+                    CMD echo trustedpulldelete
+                `, true)
+
+	pushCmd := exec.Command(dockerBinary, "push", repoName)
+	s.trustedCmd(pushCmd)
+	out, _, err := runCommandWithOutput(pushCmd)
+	if err != nil {
+		c.Fatalf("Error running trusted push: %s\n%s", err, out)
+	}
+	if !strings.Contains(string(out), "Signing and pushing trust metadata") {
+		c.Fatalf("Missing expected output on trusted push:\n%s", out)
+	}
+
+	if out, status := dockerCmd(c, "rmi", repoName); status != 0 {
+		c.Fatalf("Error removing image %q\n%s", repoName, out)
+	}
+
+	// Try pull
+	pullCmd := exec.Command(dockerBinary, "pull", repoName)
+	s.trustedCmd(pullCmd)
+	out, _, err = runCommandWithOutput(pullCmd)
+
+	c.Assert(err, check.IsNil, check.Commentf(out))
+
+	matches := digestRegex.FindStringSubmatch(out)
+	c.Assert(matches, checker.HasLen, 2, check.Commentf("unable to parse digest from pull output: %s", out))
+	pullDigest := matches[1]
+
+	imageID, err := inspectField(repoName, "Id")
+	c.Assert(err, checker.IsNil, check.Commentf("error inspecting image id"))
+
+	imageByDigest := repoName + "@" + pullDigest
+	byDigestID, err := inspectField(imageByDigest, "Id")
+	c.Assert(err, checker.IsNil, check.Commentf("error inspecting image id"))
+
+	c.Assert(byDigestID, checker.Equals, imageID)
+
+	// rmi of tag should also remove the digest reference
+	dockerCmd(c, "rmi", repoName)
+
+	_, err = inspectField(imageByDigest, "Id")
+	c.Assert(err, checker.NotNil, check.Commentf("digest reference should have been removed"))
+
+	_, err = inspectField(imageID, "Id")
+	c.Assert(err, checker.NotNil, check.Commentf("image should have been deleted"))
+}