Merge pull request #32627 from smarterclayton/relax_content_id

Automatic merge from submit-queue

Docker digest validation is too strict

Docker 1.10 does not guarantee that the pulled digest matches the digest
on disk when dealing with v1 schemas stored in a Docker registry. This
is the case for images like
centos/ruby-23-centos7@sha256:940584acbbfb0347272112d2eb95574625c0c60b4e2fdadb139de5859cf754bf
which as a result of #30366 cannot be pulled by Kube from a Docker 1.10
system.

This partially reverts commit 875fd16e1e.

pkg/kubelet/dockertools/docker.go

@@ -25,6 +25,7 @@ import (
 	"strings"
 	"time"
 
+	dockerdigest "github.com/docker/distribution/digest"
 	dockerref "github.com/docker/distribution/reference"
 	"github.com/docker/docker/pkg/jsonmessage"
 	dockerapi "github.com/docker/engine-api/client"
@@ -152,7 +153,6 @@ func filterHTTPError(err error, image string) error {
 
 // Check if the inspected image matches what we are looking for
 func matchImageTagOrSHA(inspected dockertypes.ImageInspect, image string) bool {
-
 	// The image string follows the grammar specified here
 	// https://github.com/docker/distribution/blob/master/reference/reference.go#L4
 	named, err := dockerref.ParseNamed(image)
@@ -180,11 +180,27 @@ func matchImageTagOrSHA(inspected dockertypes.ImageInspect, image string) bool {
 	}
 
 	if isDigested {
-		algo := digest.Digest().Algorithm().String()
+		for _, repoDigest := range inspected.RepoDigests {
-		sha := digest.Digest().Hex()
+			named, err := dockerref.ParseNamed(repoDigest)
-		// Look specifically for short and long sha(s)
+			if err != nil {
-		if strings.Contains(inspected.ID, algo+":"+sha) {
+				glog.V(4).Infof("couldn't parse image RepoDigest reference %q: %v", repoDigest, err)
-			// We found the short or long SHA requested
+				continue
+			}
+			if d, isDigested := named.(dockerref.Digested); isDigested {
+				if digest.Digest().Algorithm().String() == d.Digest().Algorithm().String() &&
+					digest.Digest().Hex() == d.Digest().Hex() {
+					return true
+				}
+			}
+		}
+
+		// process the ID as a digest
+		id, err := dockerdigest.ParseDigest(inspected.ID)
+		if err != nil {
+			glog.V(4).Infof("couldn't parse image ID reference %q: %v", id, err)
+			return false
+		}
+		if digest.Digest().Algorithm().String() == id.Algorithm().String() && digest.Digest().Hex() == id.Hex() {
 			return true
 		}
 	}

pkg/kubelet/dockertools/docker_test.go

@@ -158,7 +158,7 @@ func TestContainerNaming(t *testing.T) {
 }
 
 func TestMatchImageTagOrSHA(t *testing.T) {
-	for _, testCase := range []struct {
+	for i, testCase := range []struct {
 		Inspected dockertypes.ImageInspect
 		Image     string
 		Output    bool
@@ -209,9 +209,109 @@ func TestMatchImageTagOrSHA(t *testing.T) {
 			Image:  "myimage@sha256:2208",
 			Output: false,
 		},
+		{
+			// mismatched ID is ignored
+			Inspected: dockertypes.ImageInspect{
+				ID: "sha256:2208f7a29005d226d1ee33a63e33af1f47af6156c740d7d23c7948e8d282d53d",
+			},
+			Image:  "myimage@sha256:0000f7a29005d226d1ee33a63e33af1f47af6156c740d7d23c7948e8d282d53d",
+			Output: false,
+		},
+		{
+			// invalid digest is ignored
+			Inspected: dockertypes.ImageInspect{
+				ID: "sha256:unparseable",
+			},
+			Image:  "myimage@sha256:unparseable",
+			Output: false,
+		},
+		{
+			// v1 schema images can be pulled in one format and returned in another
+			Inspected: dockertypes.ImageInspect{
+				ID:          "sha256:9bbdf247c91345f0789c10f50a57e36a667af1189687ad1de88a6243d05a2227",
+				RepoDigests: []string{"centos/ruby-23-centos7@sha256:940584acbbfb0347272112d2eb95574625c0c60b4e2fdadb139de5859cf754bf"},
+			},
+			Image:  "centos/ruby-23-centos7@sha256:940584acbbfb0347272112d2eb95574625c0c60b4e2fdadb139de5859cf754bf",
+			Output: true,
+		},
+		{
+			// RepoDigest match is is required
+			Inspected: dockertypes.ImageInspect{
+				ID:          "",
+				RepoDigests: []string{"docker.io/centos/ruby-23-centos7@sha256:000084acbbfb0347272112d2eb95574625c0c60b4e2fdadb139de5859cf754bf"},
+			},
+			Image:  "centos/ruby-23-centos7@sha256:940584acbbfb0347272112d2eb95574625c0c60b4e2fdadb139de5859cf754bf",
+			Output: false,
+		},
+		{
+			// RepoDigest match is allowed
+			Inspected: dockertypes.ImageInspect{
+				ID:          "sha256:9bbdf247c91345f0789c10f50a57e36a667af1189687ad1de88a6243d05a2227",
+				RepoDigests: []string{"docker.io/centos/ruby-23-centos7@sha256:940584acbbfb0347272112d2eb95574625c0c60b4e2fdadb139de5859cf754bf"},
+			},
+			Image:  "centos/ruby-23-centos7@sha256:940584acbbfb0347272112d2eb95574625c0c60b4e2fdadb139de5859cf754bf",
+			Output: true,
+		},
+		{
+			// RepoDigest and ID are checked
+			Inspected: dockertypes.ImageInspect{
+				ID:          "sha256:940584acbbfb0347272112d2eb95574625c0c60b4e2fdadb139de5859cf754bf",
+				RepoDigests: []string{"docker.io/centos/ruby-23-centos7@sha256:9bbdf247c91345f0789c10f50a57e36a667af1189687ad1de88a6243d05a2227"},
+			},
+			Image:  "centos/ruby-23-centos7@sha256:940584acbbfb0347272112d2eb95574625c0c60b4e2fdadb139de5859cf754bf",
+			Output: true,
+		},
+		{
+			// unparseable RepoDigests are skipped
+			Inspected: dockertypes.ImageInspect{
+				ID: "sha256:9bbdf247c91345f0789c10f50a57e36a667af1189687ad1de88a6243d05a2227",
+				RepoDigests: []string{
+					"centos/ruby-23-centos7@sha256:unparseable",
+					"docker.io/centos/ruby-23-centos7@sha256:940584acbbfb0347272112d2eb95574625c0c60b4e2fdadb139de5859cf754bf",
+				},
+			},
+			Image:  "centos/ruby-23-centos7@sha256:940584acbbfb0347272112d2eb95574625c0c60b4e2fdadb139de5859cf754bf",
+			Output: true,
+		},
+		{
+			// unparseable RepoDigest is ignored
+			Inspected: dockertypes.ImageInspect{
+				ID:          "sha256:9bbdf247c91345f0789c10f50a57e36a667af1189687ad1de88a6243d05a2227",
+				RepoDigests: []string{"docker.io/centos/ruby-23-centos7@sha256:unparseable"},
+			},
+			Image:  "centos/ruby-23-centos7@sha256:940584acbbfb0347272112d2eb95574625c0c60b4e2fdadb139de5859cf754bf",
+			Output: false,
+		},
+		{
+			// unparseable image digest is ignored
+			Inspected: dockertypes.ImageInspect{
+				ID:          "sha256:9bbdf247c91345f0789c10f50a57e36a667af1189687ad1de88a6243d05a2227",
+				RepoDigests: []string{"docker.io/centos/ruby-23-centos7@sha256:unparseable"},
+			},
+			Image:  "centos/ruby-23-centos7@sha256:unparseable",
+			Output: false,
+		},
+		{
+			// prefix match is rejected for ID and RepoDigest
+			Inspected: dockertypes.ImageInspect{
+				ID:          "sha256:unparseable",
+				RepoDigests: []string{"docker.io/centos/ruby-23-centos7@sha256:unparseable"},
+			},
+			Image:  "sha256:unparseable",
+			Output: false,
+		},
+		{
+			// possible SHA prefix match is rejected for ID and RepoDigest because it is not in the named format
+			Inspected: dockertypes.ImageInspect{
+				ID:          "sha256:0000f247c91345f0789c10f50a57e36a667af1189687ad1de88a6243d05a2227",
+				RepoDigests: []string{"docker.io/centos/ruby-23-centos7@sha256:0000f247c91345f0789c10f50a57e36a667af1189687ad1de88a6243d05a2227"},
+			},
+			Image:  "sha256:0000",
+			Output: false,
+		},
 	} {
 		match := matchImageTagOrSHA(testCase.Inspected, testCase.Image)
-		assert.Equal(t, testCase.Output, match, testCase.Image+" is not a match")
+		assert.Equal(t, testCase.Output, match, testCase.Image+fmt.Sprintf(" is not a match (%d)", i))
 	}
 }