From d3bc4c4b388d12302d2520f158f2116c688ef83d Mon Sep 17 00:00:00 2001 From: Josh Hawn Date: Wed, 4 Mar 2015 16:31:31 -0800 Subject: [PATCH 1/2] Switch to SHA256 as canonical digest Also support client digests linking to canonical digest. --- registry/storage/layerupload.go | 54 ++++++++++++++++----------------- registry/storage/paths.go | 6 ---- 2 files changed, 26 insertions(+), 34 deletions(-) diff --git a/registry/storage/layerupload.go b/registry/storage/layerupload.go index 14e42338..940f2938 100644 --- a/registry/storage/layerupload.go +++ b/registry/storage/layerupload.go @@ -11,7 +11,6 @@ import ( ctxu "github.com/docker/distribution/context" "github.com/docker/distribution/digest" storagedriver "github.com/docker/distribution/registry/storage/driver" - "github.com/docker/docker/pkg/tarsum" ) // layerUploadController is used to control the various aspects of resumable @@ -61,7 +60,7 @@ func (luc *layerUploadController) Finish(digest digest.Digest) (distribution.Lay } // Link the layer blob into the repository. - if err := luc.linkLayer(canonical); err != nil { + if err := luc.linkLayer(canonical, digest); err != nil { return nil, err } @@ -86,23 +85,6 @@ func (luc *layerUploadController) Cancel() error { // validateLayer checks the layer data against the digest, returning an error // if it does not match. The canonical digest is returned. func (luc *layerUploadController) validateLayer(dgst digest.Digest) (digest.Digest, error) { - // First, check the incoming tarsum version of the digest. - version, err := tarsum.GetVersionFromTarsum(dgst.String()) - if err != nil { - return "", err - } - - // TODO(stevvooe): Should we push this down into the digest type? - switch version { - case tarsum.Version1: - default: - // version 0 and dev, for now. - return "", distribution.ErrLayerInvalidDigest{ - Digest: dgst, - Reason: distribution.ErrLayerTarSumVersionUnsupported, - } - } - digestVerifier := digest.NewDigestVerifier(dgst) // TODO(stevvooe): Store resumable hash calculations in upload directory @@ -122,7 +104,7 @@ func (luc *layerUploadController) validateLayer(dgst digest.Digest) (digest.Dige // sink. Instead, its read driven. This might be okay. // Calculate an updated digest with the latest version. - canonical, err := digest.FromTarArchive(tr) + canonical, err := digest.FromReader(tr) if err != nil { return "", err } @@ -195,17 +177,33 @@ func (luc *layerUploadController) moveLayer(dgst digest.Digest) error { // linkLayer links a valid, written layer blob into the registry under the // named repository for the upload controller. -func (luc *layerUploadController) linkLayer(digest digest.Digest) error { - layerLinkPath, err := luc.layerStore.repository.registry.pm.path(layerLinkPathSpec{ - name: luc.layerStore.repository.Name(), - digest: digest, - }) +func (luc *layerUploadController) linkLayer(canonical digest.Digest, aliases ...digest.Digest) error { + dgsts := append([]digest.Digest{canonical}, aliases...) - if err != nil { - return err + // Don't make duplicate links. + seenDigests := make(map[digest.Digest]struct{}, len(dgsts)) + + for _, dgst := range dgsts { + if _, seen := seenDigests[dgst]; seen { + continue + } + seenDigests[dgst] = struct{}{} + + layerLinkPath, err := luc.layerStore.repository.registry.pm.path(layerLinkPathSpec{ + name: luc.layerStore.repository.Name(), + digest: dgst, + }) + + if err != nil { + return err + } + + if err := luc.layerStore.repository.registry.driver.PutContent(layerLinkPath, []byte(canonical)); err != nil { + return err + } } - return luc.layerStore.repository.registry.driver.PutContent(layerLinkPath, []byte(digest)) + return nil } // removeResources should clean up all resources associated with the upload diff --git a/registry/storage/paths.go b/registry/storage/paths.go index 173e98a8..179e7b78 100644 --- a/registry/storage/paths.go +++ b/registry/storage/paths.go @@ -232,12 +232,6 @@ func (pm *pathMapper) path(spec pathSpec) (string, error) { return "", err } - // For now, only map tarsum paths. - if components[0] != "tarsum" { - // Only tarsum is supported, for now - return "", fmt.Errorf("unsupported content digest: %v", v.digest) - } - layerLinkPathComponents := append(repoPrefix, v.name, "_layers") return path.Join(path.Join(append(layerLinkPathComponents, components...)...), "link"), nil From b777e389b937399a5deb3adf409152554e16d2a3 Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Wed, 4 Mar 2015 20:26:56 -0800 Subject: [PATCH 2/2] fixing up tests to work with for non-tarsum future Signed-off-by: David Lawrence (github: endophage) --- digest/digest.go | 2 ++ digest/digester.go | 44 +++++++++++++++++++++++++++++++++ registry/handlers/api_test.go | 16 +++++++++--- registry/storage/layer_test.go | 4 +-- registry/storage/layerupload.go | 2 +- 5 files changed, 61 insertions(+), 7 deletions(-) create mode 100644 digest/digester.go diff --git a/digest/digest.go b/digest/digest.go index 06e9fad8..da9a6276 100644 --- a/digest/digest.go +++ b/digest/digest.go @@ -16,6 +16,8 @@ import ( const ( // DigestTarSumV1EmptyTar is the digest for the empty tar file. DigestTarSumV1EmptyTar = "tarsum.v1+sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + // DigestSha256EmptyTar is the canonical sha256 digest of empty data + DigestSha256EmptyTar = "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" ) // Digest allows simple protection of hex formatted digest strings, prefixed diff --git a/digest/digester.go b/digest/digester.go new file mode 100644 index 00000000..9094d662 --- /dev/null +++ b/digest/digester.go @@ -0,0 +1,44 @@ +package digest + +import ( + "crypto/sha256" + "hash" +) + +// Digester calculates the digest of written data. It is functionally +// equivalent to hash.Hash but provides methods for returning the Digest type +// rather than raw bytes. +type Digester struct { + alg string + hash hash.Hash +} + +// NewDigester create a new Digester with the given hashing algorithm and instance +// of that algo's hasher. +func NewDigester(alg string, h hash.Hash) Digester { + return Digester{ + alg: alg, + hash: h, + } +} + +// NewCanonicalDigester is a convenience function to create a new Digester with +// out default settings. +func NewCanonicalDigester() Digester { + return NewDigester("sha256", sha256.New()) +} + +// Write data to the digester. These writes cannot fail. +func (d *Digester) Write(p []byte) (n int, err error) { + return d.hash.Write(p) +} + +// Digest returns the current digest for this digester. +func (d *Digester) Digest() Digest { + return NewDigest(d.alg, d.hash) +} + +// Reset the state of the digester. +func (d *Digester) Reset() { + d.hash.Reset() +} diff --git a/registry/handlers/api_test.go b/registry/handlers/api_test.go index 4a273b28..22f2d9ca 100644 --- a/registry/handlers/api_test.go +++ b/registry/handlers/api_test.go @@ -573,7 +573,9 @@ func doPushLayer(t *testing.T, ub *v2.URLBuilder, name string, dgst digest.Diges // pushLayer pushes the layer content returning the url on success. func pushLayer(t *testing.T, ub *v2.URLBuilder, name string, dgst digest.Digest, uploadURLBase string, body io.Reader) string { - resp, err := doPushLayer(t, ub, name, dgst, uploadURLBase, body) + digester := digest.NewCanonicalDigester() + + resp, err := doPushLayer(t, ub, name, dgst, uploadURLBase, io.TeeReader(body, &digester)) if err != nil { t.Fatalf("unexpected error doing push layer request: %v", err) } @@ -581,7 +583,13 @@ func pushLayer(t *testing.T, ub *v2.URLBuilder, name string, dgst digest.Digest, checkResponse(t, "putting monolithic chunk", resp, http.StatusCreated) - expectedLayerURL, err := ub.BuildBlobURL(name, dgst) + if err != nil { + t.Fatalf("error generating sha256 digest of body") + } + + sha256Dgst := digester.Digest() + + expectedLayerURL, err := ub.BuildBlobURL(name, sha256Dgst) if err != nil { t.Fatalf("error building expected layer url: %v", err) } @@ -589,7 +597,7 @@ func pushLayer(t *testing.T, ub *v2.URLBuilder, name string, dgst digest.Digest, checkHeaders(t, resp, http.Header{ "Location": []string{expectedLayerURL}, "Content-Length": []string{"0"}, - "Docker-Content-Digest": []string{dgst.String()}, + "Docker-Content-Digest": []string{sha256Dgst.String()}, }) return resp.Header.Get("Location") @@ -682,7 +690,7 @@ func checkHeaders(t *testing.T, resp *http.Response, headers http.Header) { for _, hv := range resp.Header[k] { if hv != v { - t.Fatalf("header value not matched in response: %q != %q", hv, v) + t.Fatalf("%v header value not matched in response: %q != %q", k, hv, v) } } } diff --git a/registry/storage/layer_test.go b/registry/storage/layer_test.go index ea101b53..43e028d5 100644 --- a/registry/storage/layer_test.go +++ b/registry/storage/layer_test.go @@ -266,12 +266,12 @@ func TestLayerUploadZeroLength(t *testing.T) { io.Copy(upload, bytes.NewReader([]byte{})) - dgst, err := digest.FromTarArchive(bytes.NewReader([]byte{})) + dgst, err := digest.FromReader(bytes.NewReader([]byte{})) if err != nil { t.Fatalf("error getting zero digest: %v", err) } - if dgst != digest.DigestTarSumV1EmptyTar { + if dgst != digest.DigestSha256EmptyTar { // sanity check on zero digest t.Fatalf("digest not as expected: %v != %v", dgst, digest.DigestTarSumV1EmptyTar) } diff --git a/registry/storage/layerupload.go b/registry/storage/layerupload.go index 940f2938..69b547f5 100644 --- a/registry/storage/layerupload.go +++ b/registry/storage/layerupload.go @@ -159,7 +159,7 @@ func (luc *layerUploadController) moveLayer(dgst digest.Digest) error { // a zero-length blob into a nonzero-length blob location. To // prevent this horrid thing, we employ the hack of only allowing // to this happen for the zero tarsum. - if dgst == digest.DigestTarSumV1EmptyTar { + if dgst == digest.DigestSha256EmptyTar { return luc.driver.PutContent(blobPath, []byte{}) }