Add method to inspect Docker images by ID

Previously, the `InspectImage` method of the Docker interface expected a
"pullable" image ref (name, tag, or manifest digest).  If you tried to
inspect an image by its ID (config digest), the inspect would fail to
validate the image against the input identifier.  This commit changes
the original method to be named `InspectImageByRef`, and introduces a
new method called `InspectImageByID` which validates that the input
identifier was an image ID.

pkg/kubelet/dockertools/docker.go

@@ -66,7 +66,8 @@ type DockerInterface interface {
 	StartContainer(id string) error
 	StopContainer(id string, timeout int) error
 	RemoveContainer(id string, opts dockertypes.ContainerRemoveOptions) error
-	InspectImage(image string) (*dockertypes.ImageInspect, error)
+	InspectImageByRef(imageRef string) (*dockertypes.ImageInspect, error)
+	InspectImageByID(imageID string) (*dockertypes.ImageInspect, error)
 	ListImages(opts dockertypes.ImageListOptions) ([]dockertypes.Image, error)
 	PullImage(image string, auth dockertypes.AuthConfig, opts dockertypes.ImagePullOptions) error
 	RemoveImage(image string, opts dockertypes.ImageRemoveOptions) ([]dockertypes.ImageDelete, error)
@@ -136,7 +137,11 @@ func filterHTTPError(err error, image string) error {
 	}
 }
 
-// Check if the inspected image matches what we are looking for
+// matchImageTagOrSHA checks if the given image specifier is a valid image ref,
+// and that it matches the given image. It should fail on things like image IDs
+// (config digests) and other digest-only references, but succeed on image names
+// (`foo`), tag references (`foo:bar`), and manifest digest references
+// (`foo@sha256:xyz`).
 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
@@ -193,6 +198,43 @@ func matchImageTagOrSHA(inspected dockertypes.ImageInspect, image string) bool {
 	return false
 }
 
+// matchImageIDOnly checks that the given image specifier is a digest-only
+// reference, and that it matches the given image.
+func matchImageIDOnly(inspected dockertypes.ImageInspect, image string) bool {
+	// If the image ref is literally equal to the inspected image's ID,
+	// just return true here (this might be the case for Docker 1.9,
+	// where we won't have a digest for the ID)
+	if inspected.ID == image {
+		return true
+	}
+
+	// Otherwise, we should try actual parsing to be more correct
+	ref, err := dockerref.Parse(image)
+	if err != nil {
+		glog.V(4).Infof("couldn't parse image reference %q: %v", image, err)
+		return false
+	}
+
+	digest, isDigested := ref.(dockerref.Digested)
+	if !isDigested {
+		glog.V(4).Infof("the image reference %q was not a digest reference")
+		return false
+	}
+
+	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
+	}
+
+	glog.V(4).Infof("The reference %s does not directly refer to the given image's ID (%q)", image, inspected.ID)
+	return false
+}
+
 func (p dockerPuller) Pull(image string, secrets []api.Secret) error {
 	keyring, err := credentialprovider.MakeDockerKeyring(secrets, p.keyring)
 	if err != nil {
@@ -246,7 +288,7 @@ func (p dockerPuller) Pull(image string, secrets []api.Secret) error {
 }
 
 func (p dockerPuller) IsImagePresent(image string) (bool, error) {
-	_, err := p.client.InspectImage(image)
+	_, err := p.client.InspectImageByRef(image)
 	if err == nil {
 		return true, nil
 	}

pkg/kubelet/dockertools/docker_manager.go

@@ -913,7 +913,7 @@ func (dm *DockerManager) IsImagePresent(image kubecontainer.ImageSpec) (bool, er
 // Removes the specified image.
 func (dm *DockerManager) RemoveImage(image kubecontainer.ImageSpec) error {
 	// If the image has multiple tags, we need to remove all the tags
-	if inspectImage, err := dm.client.InspectImage(image.Image); err == nil && len(inspectImage.RepoTags) > 1 {
+	if inspectImage, err := dm.client.InspectImageByID(image.Image); err == nil && len(inspectImage.RepoTags) > 1 {
 		for _, tag := range inspectImage.RepoTags {
 			if _, err := dm.client.RemoveImage(tag, dockertypes.ImageRemoveOptions{PruneChildren: true}); err != nil {
 				return err
@@ -2414,7 +2414,7 @@ func (dm *DockerManager) verifyNonRoot(container *api.Container) error {
 // or the user is set to root.  If there is an error inspecting the image this method will return
 // false and return the error.
 func (dm *DockerManager) isImageRoot(image string) (bool, error) {
-	img, err := dm.client.InspectImage(image)
+	img, err := dm.client.InspectImageByRef(image)
 	if err != nil {
 		return false, err
 	}

pkg/kubelet/dockertools/docker_test.go

@@ -315,6 +315,85 @@ func TestMatchImageTagOrSHA(t *testing.T) {
 	}
 }
 
+func TestMatchImageIDOnly(t *testing.T) {
+	for i, testCase := range []struct {
+		Inspected dockertypes.ImageInspect
+		Image     string
+		Output    bool
+	}{
+		// shouldn't match names or tagged names
+		{
+			Inspected: dockertypes.ImageInspect{RepoTags: []string{"ubuntu:latest"}},
+			Image:     "ubuntu",
+			Output:    false,
+		},
+		{
+			Inspected: dockertypes.ImageInspect{RepoTags: []string{"colemickens/hyperkube-amd64:217.9beff63"}},
+			Image:     "colemickens/hyperkube-amd64:217.9beff63",
+			Output:    false,
+		},
+		// should match name@digest refs if they refer to the image ID (but only the full ID)
+		{
+			Inspected: dockertypes.ImageInspect{
+				ID: "sha256:2208f7a29005d226d1ee33a63e33af1f47af6156c740d7d23c7948e8d282d53d",
+			},
+			Image:  "myimage@sha256:2208f7a29005d226d1ee33a63e33af1f47af6156c740d7d23c7948e8d282d53d",
+			Output: true,
+		},
+		{
+			Inspected: dockertypes.ImageInspect{
+				ID: "sha256:2208f7a29005d226d1ee33a63e33af1f47af6156c740d7d23c7948e8d282d53d",
+			},
+			Image:  "myimage@sha256:2208f7a29005",
+			Output: false,
+		},
+		{
+			Inspected: dockertypes.ImageInspect{
+				ID: "sha256:2208f7a29005d226d1ee33a63e33af1f47af6156c740d7d23c7948e8d282d53d",
+			},
+			Image:  "myimage@sha256:2208",
+			Output: false,
+		},
+		// should match when the IDs are literally the same
+		{
+			Inspected: dockertypes.ImageInspect{
+				ID: "foobar",
+			},
+			Image:  "foobar",
+			Output: true,
+		},
+		// shouldn't match mismatched IDs
+		{
+			Inspected: dockertypes.ImageInspect{
+				ID: "sha256:2208f7a29005d226d1ee33a63e33af1f47af6156c740d7d23c7948e8d282d53d",
+			},
+			Image:  "myimage@sha256:0000f7a29005d226d1ee33a63e33af1f47af6156c740d7d23c7948e8d282d53d",
+			Output: false,
+		},
+		// shouldn't match invalid IDs or refs
+		{
+			Inspected: dockertypes.ImageInspect{
+				ID: "sha256:unparseable",
+			},
+			Image:  "myimage@sha256:unparseable",
+			Output: false,
+		},
+		// shouldn't match against repo digests
+		{
+			Inspected: dockertypes.ImageInspect{
+				ID:          "sha256:9bbdf247c91345f0789c10f50a57e36a667af1189687ad1de88a6243d05a2227",
+				RepoDigests: []string{"centos/ruby-23-centos7@sha256:940584acbbfb0347272112d2eb95574625c0c60b4e2fdadb139de5859cf754bf"},
+			},
+			Image:  "centos/ruby-23-centos7@sha256:940584acbbfb0347272112d2eb95574625c0c60b4e2fdadb139de5859cf754bf",
+			Output: false,
+		},
+	} {
+		match := matchImageIDOnly(testCase.Inspected, testCase.Image)
+		assert.Equal(t, testCase.Output, match, fmt.Sprintf("%s is not a match (%d)", testCase.Image, i))
+	}
+
+}
+
 func TestPullWithNoSecrets(t *testing.T) {
 	tests := []struct {
 		imageName     string
@@ -614,8 +693,14 @@ type imageTrackingDockerClient struct {
 	imageName string
 }
 
-func (f *imageTrackingDockerClient) InspectImage(name string) (image *dockertypes.ImageInspect, err error) {
-	image, err = f.FakeDockerClient.InspectImage(name)
+func (f *imageTrackingDockerClient) InspectImageByID(name string) (image *dockertypes.ImageInspect, err error) {
+	image, err = f.FakeDockerClient.InspectImageByID(name)
+	f.imageName = name
+	return
+}
+
+func (f *imageTrackingDockerClient) InspectImageByRef(name string) (image *dockertypes.ImageInspect, err error) {
+	image, err = f.FakeDockerClient.InspectImageByRef(name)
 	f.imageName = name
 	return
 }

pkg/kubelet/dockertools/fake_docker_client.go

@@ -310,9 +310,19 @@ func (f *FakeDockerClient) InspectContainer(id string) (*dockertypes.ContainerJS
 	return nil, fmt.Errorf("container %q not found", id)
 }
 
-// InspectImage is a test-spy implementation of DockerInterface.InspectImage.
+// InspectImageByRef is a test-spy implementation of DockerInterface.InspectImageByRef.
 // It adds an entry "inspect" to the internal method call record.
-func (f *FakeDockerClient) InspectImage(name string) (*dockertypes.ImageInspect, error) {
+func (f *FakeDockerClient) InspectImageByRef(name string) (*dockertypes.ImageInspect, error) {
+	f.Lock()
+	defer f.Unlock()
+	f.called = append(f.called, calledDetail{name: "inspect_image"})
+	err := f.popError("inspect_image")
+	return f.Image, err
+}
+
+// InspectImageByID is a test-spy implementation of DockerInterface.InspectImageByID.
+// It adds an entry "inspect" to the internal method call record.
+func (f *FakeDockerClient) InspectImageByID(name string) (*dockertypes.ImageInspect, error) {
 	f.Lock()
 	defer f.Unlock()
 	f.called = append(f.called, calledDetail{name: "inspect_image"})

pkg/kubelet/dockertools/instrumented_docker.go

@@ -107,11 +107,20 @@ func (in instrumentedDockerInterface) RemoveContainer(id string, opts dockertype
 	return err
 }
 
-func (in instrumentedDockerInterface) InspectImage(image string) (*dockertypes.ImageInspect, error) {
+func (in instrumentedDockerInterface) InspectImageByRef(image string) (*dockertypes.ImageInspect, error) {
 	const operation = "inspect_image"
 	defer recordOperation(operation, time.Now())
 
-	out, err := in.client.InspectImage(image)
+	out, err := in.client.InspectImageByRef(image)
+	recordError(operation, err)
+	return out, err
+}
+
+func (in instrumentedDockerInterface) InspectImageByID(image string) (*dockertypes.ImageInspect, error) {
+	const operation = "inspect_image"
+	defer recordOperation(operation, time.Now())
+
+	out, err := in.client.InspectImageByID(image)
 	recordError(operation, err)
 	return out, err
 }

pkg/kubelet/dockertools/kube_docker_client.go

@@ -182,25 +182,47 @@ func (d *kubeDockerClient) RemoveContainer(id string, opts dockertypes.Container
 	return err
 }
 
-func (d *kubeDockerClient) InspectImage(image string) (*dockertypes.ImageInspect, error) {
+func (d *kubeDockerClient) inspectImageRaw(ref string) (*dockertypes.ImageInspect, error) {
 	ctx, cancel := d.getTimeoutContext()
 	defer cancel()
-	resp, _, err := d.client.ImageInspectWithRaw(ctx, image, true)
+	resp, _, err := d.client.ImageInspectWithRaw(ctx, ref, true)
 	if ctxErr := contextError(ctx); ctxErr != nil {
 		return nil, ctxErr
 	}
 	if err != nil {
 		if dockerapi.IsErrImageNotFound(err) {
-			err = imageNotFoundError{ID: image}
+			err = imageNotFoundError{ID: ref}
 		}
 		return nil, err
 	}
-	if !matchImageTagOrSHA(resp, image) {
-		return nil, imageNotFoundError{ID: image}
-	}
+
 	return &resp, nil
 }
 
+func (d *kubeDockerClient) InspectImageByID(imageID string) (*dockertypes.ImageInspect, error) {
+	resp, err := d.inspectImageRaw(imageID)
+	if err != nil {
+		return nil, err
+	}
+
+	if !matchImageIDOnly(*resp, imageID) {
+		return nil, imageNotFoundError{ID: imageID}
+	}
+	return resp, nil
+}
+
+func (d *kubeDockerClient) InspectImageByRef(imageRef string) (*dockertypes.ImageInspect, error) {
+	resp, err := d.inspectImageRaw(imageRef)
+	if err != nil {
+		return nil, err
+	}
+
+	if !matchImageTagOrSHA(*resp, imageRef) {
+		return nil, imageNotFoundError{ID: imageRef}
+	}
+	return resp, nil
+}
+
 func (d *kubeDockerClient) ImageHistory(id string) ([]dockertypes.ImageHistory, error) {
 	ctx, cancel := d.getTimeoutContext()
 	defer cancel()