From 2f048f73e122ab90b8f35a088b4be52bd255caad Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Wed, 6 Jan 2016 17:57:21 -0800 Subject: [PATCH] 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 --- daemon/image_delete.go | 30 ++++++++++- docs/reference/commandline/rmi.md | 5 +- integration-cli/docker_cli_by_digest_test.go | 29 ++++++++++ .../docker_cli_pull_trusted_test.go | 53 +++++++++++++++++++ 4 files changed, 113 insertions(+), 4 deletions(-) diff --git a/daemon/image_delete.go b/daemon/image_delete.go index b2aacfb606..9a0df1f2ca 100644 --- a/daemon/image_delete.go +++ b/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 } diff --git a/docs/reference/commandline/rmi.md b/docs/reference/commandline/rmi.md index 69e8ef892f..022a415928 100644 --- a/docs/reference/commandline/rmi.md +++ b/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 diff --git a/integration-cli/docker_cli_by_digest_test.go b/integration-cli/docker_cli_by_digest_test.go index 75c8b4fd6f..1e25052723 100644 --- a/integration-cli/docker_cli_by_digest_test.go +++ b/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 @ 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) { diff --git a/integration-cli/docker_cli_pull_trusted_test.go b/integration-cli/docker_cli_pull_trusted_test.go index 23bc1af755..0da24bc9fb 100644 --- a/integration-cli/docker_cli_pull_trusted_test.go +++ b/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")) +}