Correct parent chain in v2 push when v1Compatibility files on the disk are inconsistent

This fixes an issue where two images with the same filesystem contents
and configuration but different remote IDs could share a v1Compatibility
file, resulting in corrupted manifests.

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

graph/push_v2.go

@@ -3,6 +3,8 @@ package graph
 import (
 	"bufio"
 	"compress/gzip"
+	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -205,6 +207,11 @@ func (p *v2Pusher) pushV2Tag(tag string) error {
 		p.layersPushed[dgst] = true
 	}
 
+	// Fix parent chain if necessary
+	if err = fixHistory(m); err != nil {
+		return err
+	}
+
 	logrus.Infof("Signed manifest for %s:%s using daemon's key: %s", p.repo.Name(), tag, p.trustKey.KeyID())
 	signed, err := schema1.Sign(m, p.trustKey)
 	if err != nil {
@@ -226,6 +233,90 @@ func (p *v2Pusher) pushV2Tag(tag string) error {
 	return manSvc.Put(signed)
 }
 
+// fixHistory makes sure that the manifest has parent IDs that are consistent
+// with its image IDs. Because local image IDs are generated from the
+// configuration and filesystem contents, but IDs in the manifest are preserved
+// from the original pull, it's possible to have inconsistencies where parent
+// IDs don't match up with the other IDs in the manifest. This happens in the
+// case where an engine pulls images where are identical except the IDs from the
+// manifest - the local ID will be the same, and one of the v1Compatibility
+// files gets discarded.
+func fixHistory(m *schema1.Manifest) error {
+	var lastID string
+
+	for i := len(m.History) - 1; i >= 0; i-- {
+		var historyEntry map[string]*json.RawMessage
+		if err := json.Unmarshal([]byte(m.History[i].V1Compatibility), &historyEntry); err != nil {
+			return err
+		}
+
+		idJSON, present := historyEntry["id"]
+		if !present || idJSON == nil {
+			return errors.New("missing id key in v1compatibility file")
+		}
+		var id string
+		if err := json.Unmarshal(*idJSON, &id); err != nil {
+			return err
+		}
+
+		parentJSON, present := historyEntry["parent"]
+
+		if i == len(m.History)-1 {
+			// The base layer must not reference a parent layer,
+			// otherwise the manifest is incomplete. There is an
+			// exception for Windows to handle base layers.
+			if !allowBaseParentImage && present && parentJSON != nil {
+				var parent string
+				if err := json.Unmarshal(*parentJSON, &parent); err != nil {
+					return err
+				}
+				if parent != "" {
+					logrus.Debugf("parent id mismatch detected; fixing. parent reference: %s", parent)
+					delete(historyEntry, "parent")
+					fixedHistory, err := json.Marshal(historyEntry)
+					if err != nil {
+						return err
+					}
+					m.History[i].V1Compatibility = string(fixedHistory)
+				}
+			}
+		} else {
+			// For all other layers, the parent ID should equal the
+			// ID of the next item in the history list. If it
+			// doesn't, fix it up (but preserve all other fields,
+			// possibly including fields that aren't known to this
+			// engine version).
+			if !present || parentJSON == nil {
+				return errors.New("missing parent key in v1compatibility file")
+			}
+			var parent string
+			if err := json.Unmarshal(*parentJSON, &parent); err != nil {
+				return err
+			}
+			if parent != lastID {
+				logrus.Debugf("parent id mismatch detected; fixing. parent reference: %s actual id: %s", parent, id)
+				historyEntry["parent"] = rawJSON(lastID)
+				fixedHistory, err := json.Marshal(historyEntry)
+				if err != nil {
+					return err
+				}
+				m.History[i].V1Compatibility = string(fixedHistory)
+			}
+		}
+		lastID = id
+	}
+
+	return nil
+}
+
+func rawJSON(value interface{}) *json.RawMessage {
+	jsonval, err := json.Marshal(value)
+	if err != nil {
+		return nil
+	}
+	return (*json.RawMessage)(&jsonval)
+}
+
 func (p *v2Pusher) pushV2Image(bs distribution.BlobService, img *image.Image) (digest.Digest, error) {
 	out := p.config.OutStream
 

integration-cli/docker_cli_push_test.go

@@ -2,13 +2,16 @@ package main
 
 import (
 	"archive/tar"
+	"encoding/json"
 	"fmt"
 	"io/ioutil"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"strings"
 	"time"
 
+	"github.com/docker/docker/image"
 	"github.com/docker/docker/pkg/integration/checker"
 	"github.com/go-check/check"
 )
@@ -83,6 +86,46 @@ func (s *DockerRegistrySuite) TestPushMultipleTags(c *check.C) {
 	}
 }
 
+// TestPushBadParentChain tries to push an image with a corrupted parent chain
+// in the v1compatibility files, and makes sure the push process fixes it.
+func (s *DockerRegistrySuite) TestPushBadParentChain(c *check.C) {
+	repoName := fmt.Sprintf("%v/dockercli/badparent", privateRegistryURL)
+
+	id, err := buildImage(repoName, `
+	    FROM busybox
+	    CMD echo "adding another layer"
+	    `, true)
+	if err != nil {
+		c.Fatal(err)
+	}
+
+	// Push to create v1compatibility file
+	dockerCmd(c, "push", repoName)
+
+	// Corrupt the parent in the v1compatibility file from the top layer
+	filename := filepath.Join(dockerBasePath, "graph", id, "v1Compatibility")
+
+	jsonBytes, err := ioutil.ReadFile(filename)
+	c.Assert(err, check.IsNil, check.Commentf("Could not read v1Compatibility file: %s", err))
+
+	var img image.Image
+	err = json.Unmarshal(jsonBytes, &img)
+	c.Assert(err, check.IsNil, check.Commentf("Could not unmarshal json: %s", err))
+
+	img.Parent = "1234123412341234123412341234123412341234123412341234123412341234"
+
+	jsonBytes, err = json.Marshal(&img)
+	c.Assert(err, check.IsNil, check.Commentf("Could not marshal json: %s", err))
+
+	err = ioutil.WriteFile(filename, jsonBytes, 0600)
+	c.Assert(err, check.IsNil, check.Commentf("Could not write v1Compatibility file: %s", err))
+
+	dockerCmd(c, "push", repoName)
+
+	// pull should succeed
+	dockerCmd(c, "pull", repoName)
+}
+
 func (s *DockerRegistrySuite) TestPushEmptyLayer(c *check.C) {
 	repoName := fmt.Sprintf("%v/dockercli/emptylayer", privateRegistryURL)
 	emptyTarball, err := ioutil.TempFile("", "empty_tarball")