From 33984f256b1a281b1130ac7e8edb7bc311750ccf Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Mon, 27 Jun 2016 10:09:57 -0700 Subject: [PATCH 1/2] Always store the image digest when pulling and pushing an image. Always attempt to add digest even when tag already exists. Ensure digest does not currently exist. When image id is mismatched, output an error log. Signed-off-by: Daniel Nephin Signed-off-by: Derek McGowan (github: dmcgowan) --- distribution/pull.go | 20 ++++++++++++++++++++ distribution/pull_v2.go | 12 ++++++++---- distribution/push_v2.go | 5 +++++ integration-cli/docker_cli_push_test.go | 8 ++++---- 4 files changed, 37 insertions(+), 8 deletions(-) diff --git a/distribution/pull.go b/distribution/pull.go index d54acff4f3..dad93b656d 100644 --- a/distribution/pull.go +++ b/distribution/pull.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/Sirupsen/logrus" + "github.com/docker/distribution/digest" "github.com/docker/docker/api" "github.com/docker/docker/distribution/metadata" "github.com/docker/docker/distribution/xfer" @@ -203,3 +204,22 @@ func ValidateRepoName(name string) error { } return nil } + +func addDigestReference(store reference.Store, ref reference.Named, dgst digest.Digest, imageID image.ID) error { + dgstRef, err := reference.WithDigest(ref, dgst) + if err != nil { + return err + } + + if oldTagImageID, err := store.Get(dgstRef); err == nil { + if oldTagImageID != imageID { + // Updating digests not supported by reference store + logrus.Errorf("Image ID for digest %s changed from %s to %s, cannot update", dgst.String(), oldTagImageID, imageID) + } + return nil + } else if err != reference.ErrDoesNotExist { + return err + } + + return store.AddDigest(dgstRef, imageID, true) +} diff --git a/distribution/pull_v2.go b/distribution/pull_v2.go index 607300a316..c78e221f04 100644 --- a/distribution/pull_v2.go +++ b/distribution/pull_v2.go @@ -393,7 +393,7 @@ func (p *v2Puller) pullV2Tag(ctx context.Context, ref reference.Named) (tagUpdat oldTagImageID, err := p.config.ReferenceStore.Get(ref) if err == nil { if oldTagImageID == imageID { - return false, nil + return false, addDigestReference(p.config.ReferenceStore, ref, manifestDigest, imageID) } } else if err != reference.ErrDoesNotExist { return false, err @@ -403,10 +403,14 @@ func (p *v2Puller) pullV2Tag(ctx context.Context, ref reference.Named) (tagUpdat if err = p.config.ReferenceStore.AddDigest(canonical, imageID, true); err != nil { return false, err } - } else if err = p.config.ReferenceStore.AddTag(ref, imageID, true); err != nil { - return false, err + } else { + if err = addDigestReference(p.config.ReferenceStore, ref, manifestDigest, imageID); err != nil { + return false, err + } + if err = p.config.ReferenceStore.AddTag(ref, imageID, true); err != nil { + return false, err + } } - return true, nil } diff --git a/distribution/push_v2.go b/distribution/push_v2.go index 0bb0a1de30..7d331f43d2 100644 --- a/distribution/push_v2.go +++ b/distribution/push_v2.go @@ -200,6 +200,11 @@ func (p *v2Pusher) pushV2Tag(ctx context.Context, ref reference.NamedTagged, ima manifestDigest := digest.FromBytes(canonicalManifest) progress.Messagef(p.config.ProgressOutput, "", "%s: digest: %s size: %d", ref.Tag(), manifestDigest, len(canonicalManifest)) + + if err := addDigestReference(p.config.ReferenceStore, ref, manifestDigest, imageID); err != nil { + return err + } + // Signal digest to the trust client so it can sign the // push, if appropriate. progress.Aux(p.config.ProgressOutput, PushResult{Tag: ref.Tag(), Digest: manifestDigest, Size: len(canonicalManifest)}) diff --git a/integration-cli/docker_cli_push_test.go b/integration-cli/docker_cli_push_test.go index dd3a0572a1..d30770b2fe 100644 --- a/integration-cli/docker_cli_push_test.go +++ b/integration-cli/docker_cli_push_test.go @@ -287,7 +287,7 @@ func (s *DockerTrustSuite) TestTrustedPush(c *check.C) { s.trustedCmd(pullCmd) out, _, err = runCommandWithOutput(pullCmd) c.Assert(err, check.IsNil, check.Commentf(out)) - c.Assert(string(out), checker.Contains, "Status: Downloaded", check.Commentf(out)) + c.Assert(string(out), checker.Contains, "Status: Image is up to date", check.Commentf(out)) // Assert that we rotated the snapshot key to the server by checking our local keystore contents, err := ioutil.ReadDir(filepath.Join(cliconfig.ConfigDir(), "trust/private/tuf_keys", privateRegistryURL, "dockerclitrusted/pushtest")) @@ -312,7 +312,7 @@ func (s *DockerTrustSuite) TestTrustedPushWithEnvPasswords(c *check.C) { s.trustedCmd(pullCmd) out, _, err = runCommandWithOutput(pullCmd) c.Assert(err, check.IsNil, check.Commentf(out)) - c.Assert(string(out), checker.Contains, "Status: Downloaded", check.Commentf(out)) + c.Assert(string(out), checker.Contains, "Status: Image is up to date", check.Commentf(out)) } func (s *DockerTrustSuite) TestTrustedPushWithFailingServer(c *check.C) { @@ -358,7 +358,7 @@ func (s *DockerTrustSuite) TestTrustedPushWithExistingTag(c *check.C) { s.trustedCmd(pullCmd) out, _, err = runCommandWithOutput(pullCmd) c.Assert(err, check.IsNil, check.Commentf(out)) - c.Assert(string(out), checker.Contains, "Status: Downloaded", check.Commentf(out)) + c.Assert(string(out), checker.Contains, "Status: Image is up to date", check.Commentf(out)) } func (s *DockerTrustSuite) TestTrustedPushWithExistingSignedTag(c *check.C) { @@ -492,7 +492,7 @@ func (s *DockerTrustSuite) TestTrustedPushWithReleasesDelegationOnly(c *check.C) s.trustedCmd(pullCmd) out, _, err = runCommandWithOutput(pullCmd) c.Assert(err, check.IsNil, check.Commentf(out)) - c.Assert(string(out), checker.Contains, "Status: Downloaded", check.Commentf(out)) + c.Assert(string(out), checker.Contains, "Status: Image is up to date", check.Commentf(out)) } func (s *DockerTrustSuite) TestTrustedPushSignsAllFirstLevelRolesWeHaveKeysFor(c *check.C) { From 8906f8307c86ce59f51d2f1b750f5e698f3cf8a5 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Thu, 30 Jun 2016 09:24:46 -0700 Subject: [PATCH 2/2] Treat digest only images as dangling Add test for dangling digest images Signed-off-by: Derek McGowan (github: dmcgowan) --- daemon/images.go | 2 +- integration-cli/docker_cli_by_digest_test.go | 73 ++++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/daemon/images.go b/daemon/images.go index 52dc67508b..0060cdafb2 100644 --- a/daemon/images.go +++ b/daemon/images.go @@ -167,7 +167,7 @@ func (daemon *Daemon) Images(filterArgs, filter string, all bool) ([]*types.Imag } else { continue } - } else if danglingOnly { + } else if danglingOnly && len(newImage.RepoTags) > 0 { continue } diff --git a/integration-cli/docker_cli_by_digest_test.go b/integration-cli/docker_cli_by_digest_test.go index 3f948a5169..a62b251364 100644 --- a/integration-cli/docker_cli_by_digest_test.go +++ b/integration-cli/docker_cli_by_digest_test.go @@ -314,6 +314,79 @@ func (s *DockerRegistrySuite) TestListImagesWithDigests(c *check.C) { c.Assert(busyboxRe.MatchString(out), checker.True, check.Commentf("expected %q: %s", busyboxRe.String(), out)) } +func (s *DockerRegistrySuite) TestListDanglingImagesWithDigests(c *check.C) { + // setup image1 + digest1, err := setupImageWithTag(c, "dangle1") + c.Assert(err, checker.IsNil, check.Commentf("error setting up image")) + imageReference1 := fmt.Sprintf("%s@%s", repoName, digest1) + c.Logf("imageReference1 = %s", imageReference1) + + // pull image1 by digest + dockerCmd(c, "pull", imageReference1) + + // list images + out, _ := dockerCmd(c, "images", "--digests") + + // make sure repo shown, tag=, digest = $digest1 + re1 := regexp.MustCompile(`\s*` + repoName + `\s*\s*` + digest1.String() + `\s`) + c.Assert(re1.MatchString(out), checker.True, check.Commentf("expected %q: %s", re1.String(), out)) + // setup image2 + digest2, err := setupImageWithTag(c, "dangle2") + //error setting up image + c.Assert(err, checker.IsNil) + imageReference2 := fmt.Sprintf("%s@%s", repoName, digest2) + c.Logf("imageReference2 = %s", imageReference2) + + // pull image1 by digest + dockerCmd(c, "pull", imageReference1) + + // pull image2 by digest + dockerCmd(c, "pull", imageReference2) + + // list images + out, _ = dockerCmd(c, "images", "--digests", "--filter=\"dangling=true\"") + + // make sure repo shown, tag=, digest = $digest1 + c.Assert(re1.MatchString(out), checker.True, check.Commentf("expected %q: %s", re1.String(), out)) + + // make sure repo shown, tag=, digest = $digest2 + re2 := regexp.MustCompile(`\s*` + repoName + `\s*\s*` + digest2.String() + `\s`) + c.Assert(re2.MatchString(out), checker.True, check.Commentf("expected %q: %s", re2.String(), out)) + + // pull dangle1 tag + dockerCmd(c, "pull", repoName+":dangle1") + + // list images + out, _ = dockerCmd(c, "images", "--digests", "--filter=\"dangling=true\"") + + // make sure image 1 has repo, tag, AND repo, , digest + reWithDigest1 := regexp.MustCompile(`\s*` + repoName + `\s*dangle1\s*` + digest1.String() + `\s`) + c.Assert(reWithDigest1.MatchString(out), checker.False, check.Commentf("unexpected %q: %s", reWithDigest1.String(), out)) + // make sure image 2 has repo, , digest + c.Assert(re2.MatchString(out), checker.True, check.Commentf("expected %q: %s", re2.String(), out)) + + // pull dangle2 tag + dockerCmd(c, "pull", repoName+":dangle2") + + // list images, show tagged images + out, _ = dockerCmd(c, "images", "--digests") + + // make sure image 1 has repo, tag, digest + c.Assert(reWithDigest1.MatchString(out), checker.True, check.Commentf("expected %q: %s", reWithDigest1.String(), out)) + + // make sure image 2 has repo, tag, digest + reWithDigest2 := regexp.MustCompile(`\s*` + repoName + `\s*dangle2\s*` + digest2.String() + `\s`) + c.Assert(reWithDigest2.MatchString(out), checker.True, check.Commentf("expected %q: %s", reWithDigest2.String(), out)) + + // list images, no longer dangling, should not match + out, _ = dockerCmd(c, "images", "--digests", "--filter=\"dangling=true\"") + + // make sure image 1 has repo, tag, digest + c.Assert(reWithDigest1.MatchString(out), checker.False, check.Commentf("unexpected %q: %s", reWithDigest1.String(), out)) + // make sure image 2 has repo, tag, digest + c.Assert(reWithDigest2.MatchString(out), checker.False, check.Commentf("unexpected %q: %s", reWithDigest2.String(), out)) +} + func (s *DockerRegistrySuite) TestInspectImageWithDigests(c *check.C) { digest, err := setupImage(c) c.Assert(err, check.IsNil, check.Commentf("error setting up image"))