Cleanup v2 push logic

Manifest is now generated during a v2 push, not relying on previously generated hashes. When pushing a layer, the hash is directly calculated from the tar contents which will be pushed. Computing the hash on push ensures that the hash contents always match what is seen by the registry. This also mitigates issues with tarsum differences and permits using pure SHA digests. Additionally the new manifest function is moved to the unit tests since it is no longer called outside the tests. Signed-off-by: Derek McGowan <[email protected]> (github: dmcgowan)
delftswa2014 · Mar 10, 2015 · d172f12 · d172f12
1 parent c5af44e
commit d172f12
Show file tree

Hide file tree

Showing 4 changed files with 227 additions and 178 deletions.
diff --git a/graph/manifest.go b/graph/manifest.go
@@ -4,113 +4,13 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
-	"io"
-	"io/ioutil"
 
 	log "github.com/Sirupsen/logrus"
 	"github.com/docker/docker/engine"
-	"github.com/docker/docker/pkg/tarsum"
 	"github.com/docker/docker/registry"
-	"github.com/docker/docker/runconfig"
 	"github.com/docker/libtrust"
 )
 
-func (s *TagStore) newManifest(localName, remoteName, tag string) ([]byte, error) {
-	manifest := &registry.ManifestData{
-		Name:          remoteName,
-		Tag:           tag,
-		SchemaVersion: 1,
-	}
-	localRepo, err := s.Get(localName)
-	if err != nil {
-		return nil, err
-	}
-	if localRepo == nil {
-		return nil, fmt.Errorf("Repo does not exist: %s", localName)
-	}
-
-	// Get the top-most layer id which the tag points to
-	layerId, exists := localRepo[tag]
-	if !exists {
-		return nil, fmt.Errorf("Tag does not exist for %s: %s", localName, tag)
-	}
-	layersSeen := make(map[string]bool)
-
-	layer, err := s.graph.Get(layerId)
-	if err != nil {
-		return nil, err
-	}
-	manifest.Architecture = layer.Architecture
-	manifest.FSLayers = make([]*registry.FSLayer, 0, 4)
-	manifest.History = make([]*registry.ManifestHistory, 0, 4)
-	var metadata runconfig.Config
-	if layer.Config != nil {
-		metadata = *layer.Config
-	}
-
-	for ; layer != nil; layer, err = layer.GetParent() {
-		if err != nil {
-			return nil, err
-		}
-
-		if layersSeen[layer.ID] {
-			break
-		}
-		if layer.Config != nil && metadata.Image != layer.ID {
-			err = runconfig.Merge(&metadata, layer.Config)
-			if err != nil {
-				return nil, err
-			}
-		}
-
-		checksum, err := layer.GetCheckSum(s.graph.ImageRoot(layer.ID))
-		if err != nil {
-			return nil, fmt.Errorf("Error getting image checksum: %s", err)
-		}
-		if tarsum.VersionLabelForChecksum(checksum) != tarsum.Version1.String() {
-			archive, err := layer.TarLayer()
-			if err != nil {
-				return nil, err
-			}
-
-			defer archive.Close()
-
-			tarSum, err := tarsum.NewTarSum(archive, true, tarsum.Version1)
-			if err != nil {
-				return nil, err
-			}
-			if _, err := io.Copy(ioutil.Discard, tarSum); err != nil {
-				return nil, err
-			}
-
-			checksum = tarSum.Sum(nil)
-
-			// Save checksum value
-			if err := layer.SaveCheckSum(s.graph.ImageRoot(layer.ID), checksum); err != nil {
-				return nil, err
-			}
-		}
-
-		jsonData, err := layer.RawJson()
-		if err != nil {
-			return nil, fmt.Errorf("Cannot retrieve the path for {%s}: %s", layer.ID, err)
-		}
-
-		manifest.FSLayers = append(manifest.FSLayers, &registry.FSLayer{BlobSum: checksum})
-
-		layersSeen[layer.ID] = true
-
-		manifest.History = append(manifest.History, &registry.ManifestHistory{V1Compatibility: string(jsonData)})
-	}
-
-	manifestBytes, err := json.MarshalIndent(manifest, "", "   ")
-	if err != nil {
-		return nil, err
-	}
-
-	return manifestBytes, nil
-}
-
 // loadManifest loads a manifest from a byte array and verifies its content.
 // The signature must be verified or an error is returned. If the manifest
 // contains no signatures by a trusted key for the name in the manifest, the

diff --git a/graph/manifest_test.go b/graph/manifest_test.go
@@ -2,11 +2,16 @@ package graph
 
 import (
 	"encoding/json"
+	"fmt"
+	"io"
+	"io/ioutil"
 	"os"
 	"testing"
 
 	"github.com/docker/docker/image"
+	"github.com/docker/docker/pkg/tarsum"
 	"github.com/docker/docker/registry"
+	"github.com/docker/docker/runconfig"
 	"github.com/docker/docker/utils"
 )
 
@@ -17,6 +22,102 @@ const (
 	testManifestTag          = "manifesttest"
 )
 
+func (s *TagStore) newManifest(localName, remoteName, tag string) ([]byte, error) {
+	manifest := &registry.ManifestData{
+		Name:          remoteName,
+		Tag:           tag,
+		SchemaVersion: 1,
+	}
+	localRepo, err := s.Get(localName)
+	if err != nil {
+		return nil, err
+	}
+	if localRepo == nil {
+		return nil, fmt.Errorf("Repo does not exist: %s", localName)
+	}
+
+	// Get the top-most layer id which the tag points to
+	layerId, exists := localRepo[tag]
+	if !exists {
+		return nil, fmt.Errorf("Tag does not exist for %s: %s", localName, tag)
+	}
+	layersSeen := make(map[string]bool)
+
+	layer, err := s.graph.Get(layerId)
+	if err != nil {
+		return nil, err
+	}
+	manifest.Architecture = layer.Architecture
+	manifest.FSLayers = make([]*registry.FSLayer, 0, 4)
+	manifest.History = make([]*registry.ManifestHistory, 0, 4)
+	var metadata runconfig.Config
+	if layer.Config != nil {
+		metadata = *layer.Config
+	}
+
+	for ; layer != nil; layer, err = layer.GetParent() {
+		if err != nil {
+			return nil, err
+		}
+
+		if layersSeen[layer.ID] {
+			break
+		}
+		if layer.Config != nil && metadata.Image != layer.ID {
+			err = runconfig.Merge(&metadata, layer.Config)
+			if err != nil {
+				return nil, err
+			}
+		}
+
+		checksum, err := layer.GetCheckSum(s.graph.ImageRoot(layer.ID))
+		if err != nil {
+			return nil, fmt.Errorf("Error getting image checksum: %s", err)
+		}
+		if tarsum.VersionLabelForChecksum(checksum) != tarsum.Version1.String() {
+			archive, err := layer.TarLayer()
+			if err != nil {
+				return nil, err
+			}
+
+			defer archive.Close()
+
+			tarSum, err := tarsum.NewTarSum(archive, true, tarsum.Version1)
+			if err != nil {
+				return nil, err
+			}
+			if _, err := io.Copy(ioutil.Discard, tarSum); err != nil {
+				return nil, err
+			}
+
+			checksum = tarSum.Sum(nil)
+
+			// Save checksum value
+			if err := layer.SaveCheckSum(s.graph.ImageRoot(layer.ID), checksum); err != nil {
+				return nil, err
+			}
+		}
+
+		jsonData, err := layer.RawJson()
+		if err != nil {
+			return nil, fmt.Errorf("Cannot retrieve the path for {%s}: %s", layer.ID, err)
+		}
+
+		manifest.FSLayers = append(manifest.FSLayers, &registry.FSLayer{BlobSum: checksum})
+
+		layersSeen[layer.ID] = true
+
+		manifest.History = append(manifest.History, &registry.ManifestHistory{V1Compatibility: string(jsonData)})
+	}
+
+	manifestBytes, err := json.MarshalIndent(manifest, "", "   ")
+	if err != nil {
+		return nil, err
+	}
+
+	return manifestBytes, nil
+}
+
 func TestManifestTarsumCache(t *testing.T) {
 	tmp, err := utils.TestDirectory("")
 	if err != nil {