Refactor from feedback

Signed-off-by: Derek McGowan <derek@mcgstyle.net> (github: dmcgowan)

docker/docker.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
-	"path"
 	"strings"
 
 	log "github.com/Sirupsen/logrus"
@@ -16,7 +15,6 @@ import (
 	flag "github.com/docker/docker/pkg/mflag"
 	"github.com/docker/docker/pkg/reexec"
 	"github.com/docker/docker/utils"
-	"github.com/docker/libtrust"
 )
 
 const (
@@ -79,22 +77,10 @@ func main() {
 	}
 	protoAddrParts := strings.SplitN(flHosts[0], "://", 2)
 
-	err := os.MkdirAll(path.Dir(*flTrustKey), 0700)
+	trustKey, err := api.LoadOrCreateTrustKey(*flTrustKey)
 	if err != nil {
 		log.Fatal(err)
 	}
-	trustKey, err := libtrust.LoadKeyFile(*flTrustKey)
-	if err == libtrust.ErrKeyFileDoesNotExist {
-		trustKey, err = libtrust.GenerateECP256PrivateKey()
-		if err != nil {
-			log.Fatalf("Error generating key: %s", err)
-		}
-		if err := libtrust.SaveKey(*flTrustKey, trustKey); err != nil {
-			log.Fatalf("Error saving key file: %s", err)
-		}
-	} else if err != nil {
-		log.Fatalf("Error loading key file: %s", err)
-	}
 
 	var (
 		cli       *client.DockerCli

graph/manifest.go

@@ -1,6 +1,7 @@
 package graph
 
 import (
+	"bytes"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -8,10 +9,12 @@ import (
 	"io/ioutil"
 	"path"
 
+	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) CmdManifest(job *engine.Job) engine.Status {
@@ -49,11 +52,15 @@ func (s *TagStore) newManifest(localName, remoteName, tag string) ([]byte, error
 		Tag:           tag,
 		SchemaVersion: 1,
 	}
-	localRepo, exists := s.Repositories[localName]
-	if !exists {
+	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)
@@ -102,7 +109,6 @@ func (s *TagStore) newManifest(localName, remoteName, tag string) ([]byte, error
 		}
 
 		tarId := tarSum.Sum(nil)
-		// Save tarsum to image json
 
 		manifest.FSLayers = append(manifest.FSLayers, &registry.FSLayer{BlobSum: tarId})
 
@@ -121,3 +127,70 @@ func (s *TagStore) newManifest(localName, remoteName, tag string) ([]byte, error
 
 	return manifestBytes, nil
 }
+
+func (s *TagStore) verifyManifest(eng *engine.Engine, manifestBytes []byte) (*registry.ManifestData, bool, error) {
+	sig, err := libtrust.ParsePrettySignature(manifestBytes, "signatures")
+	if err != nil {
+		return nil, false, fmt.Errorf("error parsing payload: %s", err)
+	}
+
+	keys, err := sig.Verify()
+	if err != nil {
+		return nil, false, fmt.Errorf("error verifying payload: %s", err)
+	}
+
+	payload, err := sig.Payload()
+	if err != nil {
+		return nil, false, fmt.Errorf("error retrieving payload: %s", err)
+	}
+
+	var manifest registry.ManifestData
+	if err := json.Unmarshal(payload, &manifest); err != nil {
+		return nil, false, fmt.Errorf("error unmarshalling manifest: %s", err)
+	}
+	if manifest.SchemaVersion != 1 {
+		return nil, false, fmt.Errorf("unsupported schema version: %d", manifest.SchemaVersion)
+	}
+
+	var verified bool
+	for _, key := range keys {
+		job := eng.Job("trust_key_check")
+		b, err := key.MarshalJSON()
+		if err != nil {
+			return nil, false, fmt.Errorf("error marshalling public key: %s", err)
+		}
+		namespace := manifest.Name
+		if namespace[0] != '/' {
+			namespace = "/" + namespace
+		}
+		stdoutBuffer := bytes.NewBuffer(nil)
+
+		job.Args = append(job.Args, namespace)
+		job.Setenv("PublicKey", string(b))
+		// Check key has read/write permission (0x03)
+		job.SetenvInt("Permission", 0x03)
+		job.Stdout.Add(stdoutBuffer)
+		if err = job.Run(); err != nil {
+			return nil, false, fmt.Errorf("error running key check: %s", err)
+		}
+		result := engine.Tail(stdoutBuffer, 1)
+		log.Debugf("Key check result: %q", result)
+		if result == "verified" {
+			verified = true
+		}
+	}
+
+	return &manifest, verified, nil
+}
+
+func checkValidManifest(manifest *registry.ManifestData) error {
+	if len(manifest.FSLayers) != len(manifest.History) {
+		return fmt.Errorf("length of history not equal to number of layers")
+	}
+
+	if len(manifest.FSLayers) == 0 {
+		return fmt.Errorf("no FSLayers in manifest")
+	}
+
+	return nil
+}

graph/pull.go

@@ -1,8 +1,6 @@
 package graph
 
 import (
-	"bytes"
-	"encoding/json"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -18,63 +16,8 @@ import (
 	"github.com/docker/docker/pkg/tarsum"
 	"github.com/docker/docker/registry"
 	"github.com/docker/docker/utils"
-	"github.com/docker/libtrust"
 )
 
-func (s *TagStore) verifyManifest(eng *engine.Engine, manifestBytes []byte) (*registry.ManifestData, bool, error) {
-	sig, err := libtrust.ParsePrettySignature(manifestBytes, "signatures")
-	if err != nil {
-		return nil, false, fmt.Errorf("error parsing payload: %s", err)
-	}
-	keys, err := sig.Verify()
-	if err != nil {
-		return nil, false, fmt.Errorf("error verifying payload: %s", err)
-	}
-
-	payload, err := sig.Payload()
-	if err != nil {
-		return nil, false, fmt.Errorf("error retrieving payload: %s", err)
-	}
-
-	var manifest registry.ManifestData
-	if err := json.Unmarshal(payload, &manifest); err != nil {
-		return nil, false, fmt.Errorf("error unmarshalling manifest: %s", err)
-	}
-	if manifest.SchemaVersion != 1 {
-		return nil, false, fmt.Errorf("unsupported schema version: %d", manifest.SchemaVersion)
-	}
-
-	var verified bool
-	for _, key := range keys {
-		job := eng.Job("trust_key_check")
-		b, err := key.MarshalJSON()
-		if err != nil {
-			return nil, false, fmt.Errorf("error marshalling public key: %s", err)
-		}
-		namespace := manifest.Name
-		if namespace[0] != '/' {
-			namespace = "/" + namespace
-		}
-		stdoutBuffer := bytes.NewBuffer(nil)
-
-		job.Args = append(job.Args, namespace)
-		job.Setenv("PublicKey", string(b))
-		// Check key has read/write permission (0x03)
-		job.SetenvInt("Permission", 0x03)
-		job.Stdout.Add(stdoutBuffer)
-		if err = job.Run(); err != nil {
-			return nil, false, fmt.Errorf("error running key check: %s", err)
-		}
-		result := engine.Tail(stdoutBuffer, 1)
-		log.Debugf("Key check result: %q", result)
-		if result == "verified" {
-			verified = true
-		}
-	}
-
-	return &manifest, verified, nil
-}
-
 func (s *TagStore) CmdPull(job *engine.Job) engine.Status {
 	if n := len(job.Args); n != 1 && n != 2 {
 		return job.Errorf("Usage: %s IMAGE [TAG]", job.Name)
@@ -113,7 +56,6 @@ func (s *TagStore) CmdPull(job *engine.Job) engine.Status {
 	}
 	defer s.poolRemove("pull", repoInfo.LocalName+":"+tag)
 
-
 	log.Debugf("pulling image from host %q with remote name %q", repoInfo.Index.Name, repoInfo.RemoteName)
 	endpoint, err := repoInfo.GetEndpoint()
 	if err != nil {
@@ -484,8 +426,8 @@ func (s *TagStore) pullV2Tag(eng *engine.Engine, r *registry.Session, out io.Wri
 		return false, fmt.Errorf("error verifying manifest: %s", err)
 	}
 
-	if len(manifest.FSLayers) != len(manifest.History) {
-		return false, fmt.Errorf("length of history not equal to number of layers")
+	if err := checkValidManifest(manifest); err != nil {
+		return false, err
 	}
 
 	if verified {
@@ -493,11 +435,6 @@ func (s *TagStore) pullV2Tag(eng *engine.Engine, r *registry.Session, out io.Wri
 	} else {
 		out.Write(sf.FormatStatus(tag, "Pulling from %s", repoInfo.CanonicalName))
 	}
-
-	if len(manifest.FSLayers) == 0 {
-		return false, fmt.Errorf("no blobSums in manifest")
-	}
-
 	downloads := make([]downloadInfo, len(manifest.FSLayers))
 
 	for i := len(manifest.FSLayers) - 1; i >= 0; i-- {

graph/push.go

@@ -311,14 +311,13 @@ func (s *TagStore) CmdPush(job *engine.Job) engine.Status {
 			// TODO Create manifest and sign
 		}
 
-		// try via manifest
 		manifest, verified, err := s.verifyManifest(job.Eng, []byte(manifestBytes))
 		if err != nil {
 			return job.Errorf("error verifying manifest: %s", err)
 		}
 
-		if len(manifest.FSLayers) != len(manifest.History) {
-			return job.Errorf("length of history not equal to number of layers")
+		if err := checkValidManifest(manifest); err != nil {
+			return job.Errorf("invalid manifest: %s", err)
 		}
 
 		if !verified {
@@ -337,11 +336,6 @@ func (s *TagStore) CmdPush(job *engine.Job) engine.Status {
 			}
 			manifestSum := sumParts[1]
 
-			// for each layer, check if it exists ...
-			// XXX wait this requires having the TarSum of the layer.tar first
-			// skip this step for now. Just push the layer every time for this naive implementation
-			//shouldPush, err := r.PostV2ImageMountBlob(imageName, sumType, sum string, token []string)
-
 			img, err := image.NewImgJSON(imgJSON)
 			if err != nil {
 				return job.Errorf("Failed to parse json: %s", err)

registry/session_v2.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
-	"net/url"
 	"strconv"
 
 	log "github.com/Sirupsen/logrus"
@@ -223,7 +222,7 @@ func (r *Session) PutV2ImageBlob(imageName, sumType, sumStr string, blobRdr io.R
 	if err != nil {
 		return err
 	}
-	queryParams := url.Values{}
+	queryParams := req.URL.Query()
 	queryParams.Add("digest", sumType+":"+sumStr)
 	req.URL.RawQuery = queryParams.Encode()
 	if err := auth.Authorize(req); err != nil {