Implement immutable manifest reference support

This changeset implements immutable manifest references via the HTTP API. Most
of the changes follow from modifications to ManifestService. Once updates were
made across the repo to implement these changes, the http handlers were change
accordingly. The new methods on ManifestService will be broken out into a
tagging service in a later PR.

Unfortunately, due to complexities around managing the manifest tag index in an
eventually consistent manner, direct deletes of manifests have been disabled.

Signed-off-by: Stephen J Day <stephen.day@docker.com>

notifications/listener.go

@@ -67,8 +67,8 @@ type manifestServiceListener struct {
 	parent *repositoryListener
 }
 
-func (msl *manifestServiceListener) Get(tag string) (*manifest.SignedManifest, error) {
+func (msl *manifestServiceListener) Get(dgst digest.Digest) (*manifest.SignedManifest, error) {
-	sm, err := msl.ManifestService.Get(tag)
+	sm, err := msl.ManifestService.Get(dgst)
 	if err == nil {
 		if err := msl.parent.listener.ManifestPulled(msl.parent.Repository, sm); err != nil {
 			logrus.Errorf("error dispatching manifest pull to listener: %v", err)
@@ -78,8 +78,8 @@ func (msl *manifestServiceListener) Get(tag string) (*manifest.SignedManifest, e
 	return sm, err
 }
 
-func (msl *manifestServiceListener) Put(tag string, sm *manifest.SignedManifest) error {
+func (msl *manifestServiceListener) Put(sm *manifest.SignedManifest) error {
-	err := msl.ManifestService.Put(tag, sm)
+	err := msl.ManifestService.Put(sm)
 
 	if err == nil {
 		if err := msl.parent.listener.ManifestPushed(msl.parent.Repository, sm); err != nil {
@@ -90,6 +90,17 @@ func (msl *manifestServiceListener) Put(tag string, sm *manifest.SignedManifest)
 	return err
 }
 
+func (msl *manifestServiceListener) GetByTag(tag string) (*manifest.SignedManifest, error) {
+	sm, err := msl.ManifestService.GetByTag(tag)
+	if err == nil {
+		if err := msl.parent.listener.ManifestPulled(msl.parent.Repository, sm); err != nil {
+			logrus.Errorf("error dispatching manifest pull to listener: %v", err)
+		}
+	}
+
+	return sm, err
+}
+
 type layerServiceListener struct {
 	distribution.LayerService
 	parent *repositoryListener

notifications/listener_test.go

@@ -32,7 +32,7 @@ func TestListener(t *testing.T) {
 
 	expectedOps := map[string]int{
 		"manifest:push": 1,
-		"manifest:pull": 1,
+		"manifest:pull": 2,
 		// "manifest:delete": 0, // deletes not supported for now
 		"layer:push": 2,
 		"layer:pull": 2,
@@ -143,16 +143,35 @@ func checkExerciseRepository(t *testing.T, repository distribution.Repository) {
 
 	manifests := repository.Manifests()
 
-	if err := manifests.Put(tag, sm); err != nil {
+	if err := manifests.Put(sm); err != nil {
 		t.Fatalf("unexpected error putting the manifest: %v", err)
 	}
 
-	fetched, err := manifests.Get(tag)
+	p, err := sm.Payload()
+	if err != nil {
+		t.Fatalf("unexpected error getting manifest payload: %v", err)
+	}
+
+	dgst, err := digest.FromBytes(p)
+	if err != nil {
+		t.Fatalf("unexpected error digesting manifest payload: %v", err)
+	}
+
+	fetchedByManifest, err := manifests.Get(dgst)
 	if err != nil {
 		t.Fatalf("unexpected error fetching manifest: %v", err)
 	}
 
-	if fetched.Tag != fetched.Tag {
+	if fetchedByManifest.Tag != sm.Tag {
+		t.Fatalf("retrieved unexpected manifest: %v", err)
+	}
+
+	fetched, err := manifests.GetByTag(tag)
+	if err != nil {
+		t.Fatalf("unexpected error fetching manifest: %v", err)
+	}
+
+	if fetched.Tag != fetchedByManifest.Tag {
 		t.Fatalf("retrieved unexpected manifest: %v", err)
 	}
 }

registry.go

@@ -34,38 +34,41 @@ type Repository interface {
 
 // ManifestService provides operations on image manifests.
 type ManifestService interface {
+	// Exists returns true if the manifest exists.
+	Exists(dgst digest.Digest) (bool, error)
+
+	// Get retrieves the identified by the digest, if it exists.
+	Get(dgst digest.Digest) (*manifest.SignedManifest, error)
+
+	// Delete removes the manifest, if it exists.
+	Delete(dgst digest.Digest) error
+
+	// Put creates or updates the manifest.
+	Put(manifest *manifest.SignedManifest) error
+
+	// TODO(stevvooe): The methods after this message should be moved to a
+	// discrete TagService, per active proposals.
+
 	// Tags lists the tags under the named repository.
 	Tags() ([]string, error)
 
-	// Exists returns true if the manifest exists.
+	// ExistsByTag returns true if the manifest exists.
-	Exists(tag string) (bool, error)
+	ExistsByTag(tag string) (bool, error)
 
-	// Get retrieves the named manifest, if it exists.
+	// GetByTag retrieves the named manifest, if it exists.
-	Get(tag string) (*manifest.SignedManifest, error)
+	GetByTag(tag string) (*manifest.SignedManifest, error)
-
-	// Put creates or updates the named manifest.
-	// Put(tag string, manifest *manifest.SignedManifest) (digest.Digest, error)
-	Put(tag string, manifest *manifest.SignedManifest) error
-
-	// Delete removes the named manifest, if it exists.
-	Delete(tag string) error
 
 	// TODO(stevvooe): There are several changes that need to be done to this
 	// interface:
 	//
-	//	1. Get(tag string) should be GetByTag(tag string)
+	//	1. Allow explicit tagging with Tag(digest digest.Digest, tag string)
-	//	2. Put(tag string, manifest *manifest.SignedManifest) should be
+	//	2. Support reading tags with a re-entrant reader to avoid large
-	//       Put(manifest *manifest.SignedManifest). The method can read the
-	//       tag on manifest to automatically tag it in the repository.
-	//	3. Need a GetByDigest(dgst digest.Digest) method.
-	//	4. Allow explicit tagging with Tag(digest digest.Digest, tag string)
-	//	5. Support reading tags with a re-entrant reader to avoid large
 	//       allocations in the registry.
-	//	6. Long-term: Provide All() method that lets one scroll through all of
+	//	3. Long-term: Provide All() method that lets one scroll through all of
 	//       the manifest entries.
-	//	7. Long-term: break out concept of signing from manifests. This is
+	//	4. Long-term: break out concept of signing from manifests. This is
 	//       really a part of the distribution sprint.
-	//	8. Long-term: Manifest should be an interface. This code shouldn't
+	//	5. Long-term: Manifest should be an interface. This code shouldn't
 	//       really be concerned with the storage format.
 }
 

registry/handlers/api_test.go

@@ -218,7 +218,8 @@ func TestLayerAPI(t *testing.T) {
 
 	checkResponse(t, "checking head on existing layer", resp, http.StatusOK)
 	checkHeaders(t, resp, http.Header{
-		"Content-Length": []string{fmt.Sprint(layerLength)},
+		"Content-Length":        []string{fmt.Sprint(layerLength)},
+		"Docker-Content-Digest": []string{layerDigest.String()},
 	})
 
 	// ----------------
@@ -230,7 +231,8 @@ func TestLayerAPI(t *testing.T) {
 
 	checkResponse(t, "fetching layer", resp, http.StatusOK)
 	checkHeaders(t, resp, http.Header{
-		"Content-Length": []string{fmt.Sprint(layerLength)},
+		"Content-Length":        []string{fmt.Sprint(layerLength)},
+		"Docker-Content-Digest": []string{layerDigest.String()},
 	})
 
 	// Verify the body
@@ -286,6 +288,9 @@ func TestManifestAPI(t *testing.T) {
 	// --------------------------------
 	// Attempt to push unsigned manifest with missing layers
 	unsignedManifest := &manifest.Manifest{
+		Versioned: manifest.Versioned{
+			SchemaVersion: 1,
+		},
 		Name: imageName,
 		Tag:  tag,
 		FSLayers: []manifest.FSLayer{
@@ -343,9 +348,33 @@ func TestManifestAPI(t *testing.T) {
 		t.Fatalf("unexpected error signing manifest: %v", err)
 	}
 
+	payload, err := signedManifest.Payload()
+	checkErr(t, err, "getting manifest payload")
+
+	dgst, err := digest.FromBytes(payload)
+	checkErr(t, err, "digesting manifest")
+
+	manifestDigestURL, err := env.builder.BuildManifestURL(imageName, dgst.String())
+	checkErr(t, err, "building manifest url")
+
 	resp = putManifest(t, "putting signed manifest", manifestURL, signedManifest)
 	checkResponse(t, "putting signed manifest", resp, http.StatusAccepted)
+	checkHeaders(t, resp, http.Header{
+		"Location":              []string{manifestDigestURL},
+		"Docker-Content-Digest": []string{dgst.String()},
+	})
 
+	// --------------------
+	// Push by digest -- should get same result
+	resp = putManifest(t, "putting signed manifest", manifestDigestURL, signedManifest)
+	checkResponse(t, "putting signed manifest", resp, http.StatusAccepted)
+	checkHeaders(t, resp, http.Header{
+		"Location":              []string{manifestDigestURL},
+		"Docker-Content-Digest": []string{dgst.String()},
+	})
+
+	// ------------------
+	// Fetch by tag name
 	resp, err = http.Get(manifestURL)
 	if err != nil {
 		t.Fatalf("unexpected error fetching manifest: %v", err)
@@ -353,6 +382,9 @@ func TestManifestAPI(t *testing.T) {
 	defer resp.Body.Close()
 
 	checkResponse(t, "fetching uploaded manifest", resp, http.StatusOK)
+	checkHeaders(t, resp, http.Header{
+		"Docker-Content-Digest": []string{dgst.String()},
+	})
 
 	var fetchedManifest manifest.SignedManifest
 	dec := json.NewDecoder(resp.Body)
@@ -364,6 +396,27 @@ func TestManifestAPI(t *testing.T) {
 		t.Fatalf("manifests do not match")
 	}
 
+	// ---------------
+	// Fetch by digest
+	resp, err = http.Get(manifestDigestURL)
+	checkErr(t, err, "fetching manifest by digest")
+	defer resp.Body.Close()
+
+	checkResponse(t, "fetching uploaded manifest", resp, http.StatusOK)
+	checkHeaders(t, resp, http.Header{
+		"Docker-Content-Digest": []string{dgst.String()},
+	})
+
+	var fetchedManifestByDigest manifest.SignedManifest
+	dec = json.NewDecoder(resp.Body)
+	if err := dec.Decode(&fetchedManifestByDigest); err != nil {
+		t.Fatalf("error decoding fetched manifest: %v", err)
+	}
+
+	if !bytes.Equal(fetchedManifestByDigest.Raw, signedManifest.Raw) {
+		t.Fatalf("manifests do not match")
+	}
+
 	// Ensure that the tag is listed.
 	resp, err = http.Get(tagsURL)
 	if err != nil {
@@ -534,8 +587,9 @@ func pushLayer(t *testing.T, ub *v2.URLBuilder, name string, dgst digest.Digest,
 	}
 
 	checkHeaders(t, resp, http.Header{
-		"Location":       []string{expectedLayerURL},
+		"Location":              []string{expectedLayerURL},
-		"Content-Length": []string{"0"},
+		"Content-Length":        []string{"0"},
+		"Docker-Content-Digest": []string{dgst.String()},
 	})
 
 	return resp.Header.Get("Location")
@@ -634,3 +688,9 @@ func checkHeaders(t *testing.T, resp *http.Response, headers http.Header) {
 		}
 	}
 }
+
+func checkErr(t *testing.T, err error, msg string) {
+	if err != nil {
+		t.Fatalf("unexpected error %s: %v", msg, err)
+	}
+}

registry/handlers/app.go

@@ -277,9 +277,8 @@ func (app *App) context(w http.ResponseWriter, r *http.Request) *Context {
 	ctx = ctxu.WithLogger(ctx, ctxu.GetRequestLogger(ctx))
 	ctx = ctxu.WithLogger(ctx, ctxu.GetLogger(ctx,
 		"vars.name",
-		"vars.tag",
+		"vars.reference",
 		"vars.digest",
-		"vars.tag",
 		"vars.uuid"))
 
 	context := &Context{

registry/handlers/app_test.go

@@ -84,7 +84,7 @@ func TestAppDispatcher(t *testing.T) {
 			endpoint: v2.RouteNameManifest,
 			vars: []string{
 				"name", "foo/bar",
-				"tag", "sometag",
+				"reference", "sometag",
 			},
 		},
 		{

registry/handlers/context.go

@@ -45,8 +45,8 @@ func getName(ctx context.Context) (name string) {
 	return ctxu.GetStringValue(ctx, "vars.name")
 }
 
-func getTag(ctx context.Context) (tag string) {
+func getReference(ctx context.Context) (reference string) {
-	return ctxu.GetStringValue(ctx, "vars.tag")
+	return ctxu.GetStringValue(ctx, "vars.reference")
 }
 
 var errDigestNotAvailable = fmt.Errorf("digest not available in context")

registry/handlers/images.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"strings"
 
 	"github.com/docker/distribution"
 	ctxu "github.com/docker/distribution/context"
@@ -11,6 +12,7 @@ import (
 	"github.com/docker/distribution/manifest"
 	"github.com/docker/distribution/registry/api/v2"
 	"github.com/gorilla/handlers"
+	"golang.org/x/net/context"
 )
 
 // imageManifestDispatcher takes the request context and builds the
@@ -18,7 +20,14 @@ import (
 func imageManifestDispatcher(ctx *Context, r *http.Request) http.Handler {
 	imageManifestHandler := &imageManifestHandler{
 		Context: ctx,
-		Tag:     getTag(ctx),
+	}
+	reference := getReference(ctx)
+	dgst, err := digest.ParseDigest(reference)
+	if err != nil {
+		// We just have a tag
+		imageManifestHandler.Tag = reference
+	} else {
+		imageManifestHandler.Digest = dgst
 	}
 
 	return handlers.MethodHandler{
@@ -32,14 +41,26 @@ func imageManifestDispatcher(ctx *Context, r *http.Request) http.Handler {
 type imageManifestHandler struct {
 	*Context
 
-	Tag string
+	// One of tag or digest gets set, depending on what is present in context.
+	Tag    string
+	Digest digest.Digest
 }
 
 // GetImageManifest fetches the image manifest from the storage backend, if it exists.
 func (imh *imageManifestHandler) GetImageManifest(w http.ResponseWriter, r *http.Request) {
 	ctxu.GetLogger(imh).Debug("GetImageManifest")
 	manifests := imh.Repository.Manifests()
-	manifest, err := manifests.Get(imh.Tag)
+
+	var (
+		sm  *manifest.SignedManifest
+		err error
+	)
+
+	if imh.Tag != "" {
+		sm, err = manifests.GetByTag(imh.Tag)
+	} else {
+		sm, err = manifests.Get(imh.Digest)
+	}
 
 	if err != nil {
 		imh.Errors.Push(v2.ErrorCodeManifestUnknown, err)
@@ -47,9 +68,22 @@ func (imh *imageManifestHandler) GetImageManifest(w http.ResponseWriter, r *http
 		return
 	}
 
+	// Get the digest, if we don't already have it.
+	if imh.Digest == "" {
+		dgst, err := digestManifest(imh, sm)
+		if err != nil {
+			imh.Errors.Push(v2.ErrorCodeDigestInvalid, err)
+			w.WriteHeader(http.StatusBadRequest)
+			return
+		}
+
+		imh.Digest = dgst
+	}
+
 	w.Header().Set("Content-Type", "application/json; charset=utf-8")
-	w.Header().Set("Content-Length", fmt.Sprint(len(manifest.Raw)))
+	w.Header().Set("Content-Length", fmt.Sprint(len(sm.Raw)))
-	w.Write(manifest.Raw)
+	w.Header().Set("Docker-Content-Digest", imh.Digest.String())
+	w.Write(sm.Raw)
 }
 
 // PutImageManifest validates and stores and image in the registry.
@@ -65,7 +99,37 @@ func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http
 		return
 	}
 
-	if err := manifests.Put(imh.Tag, &manifest); err != nil {
+	dgst, err := digestManifest(imh, &manifest)
+	if err != nil {
+		imh.Errors.Push(v2.ErrorCodeDigestInvalid, err)
+		w.WriteHeader(http.StatusBadRequest)
+		return
+	}
+
+	// Validate manifest tag or digest matches payload
+	if imh.Tag != "" {
+		if manifest.Tag != imh.Tag {
+			ctxu.GetLogger(imh).Errorf("invalid tag on manifest payload: %q != %q", manifest.Tag, imh.Tag)
+			imh.Errors.Push(v2.ErrorCodeTagInvalid)
+			w.WriteHeader(http.StatusBadRequest)
+			return
+		}
+
+		imh.Digest = dgst
+	} else if imh.Digest != "" {
+		if dgst != imh.Digest {
+			ctxu.GetLogger(imh).Errorf("payload digest does match: %q != %q", dgst, imh.Digest)
+			imh.Errors.Push(v2.ErrorCodeDigestInvalid)
+			w.WriteHeader(http.StatusBadRequest)
+			return
+		}
+	} else {
+		imh.Errors.Push(v2.ErrorCodeTagInvalid, "no tag or digest specified")
+		w.WriteHeader(http.StatusBadRequest)
+		return
+	}
+
+	if err := manifests.Put(&manifest); err != nil {
 		// TODO(stevvooe): These error handling switches really need to be
 		// handled by an app global mapper.
 		switch err := err.(type) {
@@ -94,25 +158,54 @@ func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http
 		return
 	}
 
+	// Construct a canonical url for the uploaded manifest.
+	location, err := imh.urlBuilder.BuildManifestURL(imh.Repository.Name(), imh.Digest.String())
+	if err != nil {
+		// NOTE(stevvooe): Given the behavior above, this absurdly unlikely to
+		// happen. We'll log the error here but proceed as if it worked. Worst
+		// case, we set an empty location header.
+		ctxu.GetLogger(imh).Errorf("error building manifest url from digest: %v", err)
+	}
+
+	w.Header().Set("Location", location)
+	w.Header().Set("Docker-Content-Digest", imh.Digest.String())
 	w.WriteHeader(http.StatusAccepted)
 }
 
 // DeleteImageManifest removes the image with the given tag from the registry.
 func (imh *imageManifestHandler) DeleteImageManifest(w http.ResponseWriter, r *http.Request) {
 	ctxu.GetLogger(imh).Debug("DeleteImageManifest")
-	manifests := imh.Repository.Manifests()
+
-	if err := manifests.Delete(imh.Tag); err != nil {
+	// TODO(stevvooe): Unfortunately, at this point, manifest deletes are
-		switch err := err.(type) {
+	// unsupported. There are issues with schema version 1 that make removing
-		case distribution.ErrManifestUnknown:
+	// tag index entries a serious problem in eventually consistent storage.
-			imh.Errors.Push(v2.ErrorCodeManifestUnknown, err)
+	// Once we work out schema version 2, the full deletion system will be
-			w.WriteHeader(http.StatusNotFound)
+	// worked out and we can add support back.
-		default:
+	imh.Errors.Push(v2.ErrorCodeUnsupported)
-			imh.Errors.Push(v2.ErrorCodeUnknown, err)
+	w.WriteHeader(http.StatusBadRequest)
-			w.WriteHeader(http.StatusBadRequest)
+}
+
+// digestManifest takes a digest of the given manifest. This belongs somewhere
+// better but we'll wait for a refactoring cycle to find that real somewhere.
+func digestManifest(ctx context.Context, sm *manifest.SignedManifest) (digest.Digest, error) {
+	p, err := sm.Payload()
+	if err != nil {
+		if !strings.Contains(err.Error(), "missing signature key") {
+			ctxu.GetLogger(ctx).Errorf("error getting manifest payload: %v", err)
+			return "", err
 		}
-		return
+
+		// NOTE(stevvooe): There are no signatures but we still have a
+		// payload. The request will fail later but this is not the
+		// responsibility of this part of the code.
+		p = sm.Raw
 	}
 
-	w.Header().Set("Content-Length", "0")
+	dgst, err := digest.FromBytes(p)
-	w.WriteHeader(http.StatusAccepted)
+	if err != nil {
+		ctxu.GetLogger(ctx).Errorf("error digesting manifest: %v", err)
+		return "", err
+	}
+
+	return dgst, err
 }

registry/handlers/layer.go

@@ -64,6 +64,8 @@ func (lh *layerHandler) GetLayer(w http.ResponseWriter, r *http.Request) {
 	}
 	defer layer.Close()
 
+	w.Header().Set("Docker-Content-Digest", lh.Digest.String())
+
 	if lh.layerHandler != nil {
 		handler, _ := lh.layerHandler.Resolve(layer)
 		if handler != nil {

registry/handlers/layerupload.go

@@ -193,6 +193,10 @@ func (luh *layerUploadHandler) PutLayerUploadComplete(w http.ResponseWriter, r *
 	// TODO(stevvooe): Check the incoming range header here, per the
 	// specification. LayerUpload should be seeked (sought?) to that position.
 
+	// TODO(stevvooe): Consider checking the error on this copy.
+	// Theoretically, problems should be detected during verification but we
+	// may miss a root cause.
+
 	// Read in the final chunk, if any.
 	io.Copy(luh.Upload, r.Body)
 
@@ -227,6 +231,7 @@ func (luh *layerUploadHandler) PutLayerUploadComplete(w http.ResponseWriter, r *
 
 	w.Header().Set("Location", layerURL)
 	w.Header().Set("Content-Length", "0")
+	w.Header().Set("Docker-Content-Digest", layer.Digest().String())
 	w.WriteHeader(http.StatusCreated)
 }
 

registry/storage/manifeststore.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/docker/distribution"
 	ctxu "github.com/docker/distribution/context"
+	"github.com/docker/distribution/digest"
 	"github.com/docker/distribution/manifest"
 	"github.com/docker/libtrust"
 )
@@ -18,31 +19,17 @@ type manifestStore struct {
 
 var _ distribution.ManifestService = &manifestStore{}
 
-// func (ms *manifestStore) Repository() Repository {
+func (ms *manifestStore) Exists(dgst digest.Digest) (bool, error) {
-// 	return ms.repository
-// }
-
-func (ms *manifestStore) Tags() ([]string, error) {
-	ctxu.GetLogger(ms.repository.ctx).Debug("(*manifestStore).Tags")
-	return ms.tagStore.tags()
-}
-
-func (ms *manifestStore) Exists(tag string) (bool, error) {
 	ctxu.GetLogger(ms.repository.ctx).Debug("(*manifestStore).Exists")
-	return ms.tagStore.exists(tag)
+	return ms.revisionStore.exists(dgst)
 }
 
-func (ms *manifestStore) Get(tag string) (*manifest.SignedManifest, error) {
+func (ms *manifestStore) Get(dgst digest.Digest) (*manifest.SignedManifest, error) {
 	ctxu.GetLogger(ms.repository.ctx).Debug("(*manifestStore).Get")
-	dgst, err := ms.tagStore.resolve(tag)
-	if err != nil {
-		return nil, err
-	}
-
 	return ms.revisionStore.get(dgst)
 }
 
-func (ms *manifestStore) Put(tag string, manifest *manifest.SignedManifest) error {
+func (ms *manifestStore) Put(manifest *manifest.SignedManifest) error {
 	ctxu.GetLogger(ms.repository.ctx).Debug("(*manifestStore).Put")
 
 	// TODO(stevvooe): Add check here to see if the revision is already
@@ -51,7 +38,7 @@ func (ms *manifestStore) Put(tag string, manifest *manifest.SignedManifest) erro
 	// indicating what happened.
 
 	// Verify the manifest.
-	if err := ms.verifyManifest(tag, manifest); err != nil {
+	if err := ms.verifyManifest(manifest); err != nil {
 		return err
 	}
 
@@ -62,46 +49,46 @@ func (ms *manifestStore) Put(tag string, manifest *manifest.SignedManifest) erro
 	}
 
 	// Now, tag the manifest
-	return ms.tagStore.tag(tag, revision)
+	return ms.tagStore.tag(manifest.Tag, revision)
 }
 
-// Delete removes all revisions of the given tag. We may want to change these
+// Delete removes the revision of the specified manfiest.
-// semantics in the future, but this will maintain consistency. The underlying
+func (ms *manifestStore) Delete(dgst digest.Digest) error {
-// blobs are left alone.
+	ctxu.GetLogger(ms.repository.ctx).Debug("(*manifestStore).Delete - unsupported")
-func (ms *manifestStore) Delete(tag string) error {
+	return fmt.Errorf("deletion of manifests not supported")
-	ctxu.GetLogger(ms.repository.ctx).Debug("(*manifestStore).Delete")
+}
 
-	revisions, err := ms.tagStore.revisions(tag)
+func (ms *manifestStore) Tags() ([]string, error) {
+	ctxu.GetLogger(ms.repository.ctx).Debug("(*manifestStore).Tags")
+	return ms.tagStore.tags()
+}
+
+func (ms *manifestStore) ExistsByTag(tag string) (bool, error) {
+	ctxu.GetLogger(ms.repository.ctx).Debug("(*manifestStore).ExistsByTag")
+	return ms.tagStore.exists(tag)
+}
+
+func (ms *manifestStore) GetByTag(tag string) (*manifest.SignedManifest, error) {
+	ctxu.GetLogger(ms.repository.ctx).Debug("(*manifestStore).GetByTag")
+	dgst, err := ms.tagStore.resolve(tag)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
-	for _, revision := range revisions {
+	return ms.revisionStore.get(dgst)
-		if err := ms.revisionStore.delete(revision); err != nil {
-			return err
-		}
-	}
-
-	return ms.tagStore.delete(tag)
 }
 
 // verifyManifest ensures that the manifest content is valid from the
-// perspective of the registry. It ensures that the name and tag match and
+// perspective of the registry. It ensures that the signature is valid for the
-// that the signature is valid for the enclosed payload. As a policy, the
+// enclosed payload. As a policy, the registry only tries to store valid
-// registry only tries to store valid content, leaving trust policies of that
+// content, leaving trust policies of that content up to consumers.
-// content up to consumers.
+func (ms *manifestStore) verifyManifest(mnfst *manifest.SignedManifest) error {
-func (ms *manifestStore) verifyManifest(tag string, mnfst *manifest.SignedManifest) error {
 	var errs distribution.ErrManifestVerification
 	if mnfst.Name != ms.repository.Name() {
 		// TODO(stevvooe): This needs to be an exported error
 		errs = append(errs, fmt.Errorf("repository name does not match manifest name"))
 	}
 
-	if mnfst.Tag != tag {
-		// TODO(stevvooe): This needs to be an exported error.
-		errs = append(errs, fmt.Errorf("tag does not match manifest tag"))
-	}
-
 	if _, err := manifest.Verify(mnfst); err != nil {
 		switch err {
 		case libtrust.ErrMissingSignatureKey, libtrust.ErrInvalidJSONContent, libtrust.ErrMissingSignatureKey:

registry/storage/manifeststore_test.go

@@ -9,25 +9,47 @@ import (
 	"github.com/docker/distribution"
 	"github.com/docker/distribution/digest"
 	"github.com/docker/distribution/manifest"
+	"github.com/docker/distribution/registry/storage/driver"
 	"github.com/docker/distribution/registry/storage/driver/inmemory"
 	"github.com/docker/distribution/testutil"
 	"github.com/docker/libtrust"
 	"golang.org/x/net/context"
 )
 
-func TestManifestStorage(t *testing.T) {
+type manifestStoreTestEnv struct {
+	ctx        context.Context
+	driver     driver.StorageDriver
+	registry   distribution.Registry
+	repository distribution.Repository
+	name       string
+	tag        string
+}
+
+func newManifestStoreTestEnv(t *testing.T, name, tag string) *manifestStoreTestEnv {
 	ctx := context.Background()
-	name := "foo/bar"
-	tag := "thetag"
 	driver := inmemory.New()
 	registry := NewRegistryWithDriver(driver)
+
 	repo, err := registry.Repository(ctx, name)
 	if err != nil {
 		t.Fatalf("unexpected error getting repo: %v", err)
 	}
-	ms := repo.Manifests()
 
-	exists, err := ms.Exists(tag)
+	return &manifestStoreTestEnv{
+		ctx:        ctx,
+		driver:     driver,
+		registry:   registry,
+		repository: repo,
+		name:       name,
+		tag:        tag,
+	}
+}
+
+func TestManifestStorage(t *testing.T) {
+	env := newManifestStoreTestEnv(t, "foo/bar", "thetag")
+	ms := env.repository.Manifests()
+
+	exists, err := ms.ExistsByTag(env.tag)
 	if err != nil {
 		t.Fatalf("unexpected error checking manifest existence: %v", err)
 	}
@@ -36,7 +58,7 @@ func TestManifestStorage(t *testing.T) {
 		t.Fatalf("manifest should not exist")
 	}
 
-	if _, err := ms.Get(tag); true {
+	if _, err := ms.GetByTag(env.tag); true {
 		switch err.(type) {
 		case distribution.ErrManifestUnknown:
 			break
@@ -49,8 +71,8 @@ func TestManifestStorage(t *testing.T) {
 		Versioned: manifest.Versioned{
 			SchemaVersion: 1,
 		},
-		Name: name,
+		Name: env.name,
-		Tag:  tag,
+		Tag:  env.tag,
 	}
 
 	// Build up some test layers and add them to the manifest, saving the
@@ -79,7 +101,7 @@ func TestManifestStorage(t *testing.T) {
 		t.Fatalf("error signing manifest: %v", err)
 	}
 
-	err = ms.Put(tag, sm)
+	err = ms.Put(sm)
 	if err == nil {
 		t.Fatalf("expected errors putting manifest")
 	}
@@ -88,7 +110,7 @@ func TestManifestStorage(t *testing.T) {
 
 	// Now, upload the layers that were missing!
 	for dgst, rs := range testLayers {
-		upload, err := repo.Layers().Upload()
+		upload, err := env.repository.Layers().Upload()
 		if err != nil {
 			t.Fatalf("unexpected error creating test upload: %v", err)
 		}
@@ -102,11 +124,11 @@ func TestManifestStorage(t *testing.T) {
 		}
 	}
 
-	if err = ms.Put(tag, sm); err != nil {
+	if err = ms.Put(sm); err != nil {
 		t.Fatalf("unexpected error putting manifest: %v", err)
 	}
 
-	exists, err = ms.Exists(tag)
+	exists, err = ms.ExistsByTag(env.tag)
 	if err != nil {
 		t.Fatalf("unexpected error checking manifest existence: %v", err)
 	}
@@ -115,7 +137,7 @@ func TestManifestStorage(t *testing.T) {
 		t.Fatalf("manifest should exist")
 	}
 
-	fetchedManifest, err := ms.Get(tag)
+	fetchedManifest, err := ms.GetByTag(env.tag)
 	if err != nil {
 		t.Fatalf("unexpected error fetching manifest: %v", err)
 	}
@@ -134,6 +156,31 @@ func TestManifestStorage(t *testing.T) {
 		t.Fatalf("unexpected error extracting payload: %v", err)
 	}
 
+	// Now that we have a payload, take a moment to check that the manifest is
+	// return by the payload digest.
+	dgst, err := digest.FromBytes(payload)
+	if err != nil {
+		t.Fatalf("error getting manifest digest: %v", err)
+	}
+
+	exists, err = ms.Exists(dgst)
+	if err != nil {
+		t.Fatalf("error checking manifest existence by digest: %v", err)
+	}
+
+	if !exists {
+		t.Fatalf("manifest %s should exist", dgst)
+	}
+
+	fetchedByDigest, err := ms.Get(dgst)
+	if err != nil {
+		t.Fatalf("unexpected error fetching manifest by digest: %v", err)
+	}
+
+	if !reflect.DeepEqual(fetchedByDigest, fetchedManifest) {
+		t.Fatalf("fetched manifest not equal: %#v != %#v", fetchedByDigest, fetchedManifest)
+	}
+
 	sigs, err := fetchedJWS.Signatures()
 	if err != nil {
 		t.Fatalf("unable to extract signatures: %v", err)
@@ -153,8 +200,8 @@ func TestManifestStorage(t *testing.T) {
 		t.Fatalf("unexpected tags returned: %v", tags)
 	}
 
-	if tags[0] != tag {
+	if tags[0] != env.tag {
-		t.Fatalf("unexpected tag found in tags: %v != %v", tags, []string{tag})
+		t.Fatalf("unexpected tag found in tags: %v != %v", tags, []string{env.tag})
 	}
 
 	// Now, push the same manifest with a different key
@@ -182,11 +229,11 @@ func TestManifestStorage(t *testing.T) {
 		t.Fatalf("unexpected number of signatures: %d != %d", len(sigs2), 1)
 	}
 
-	if err = ms.Put(tag, sm2); err != nil {
+	if err = ms.Put(sm2); err != nil {
 		t.Fatalf("unexpected error putting manifest: %v", err)
 	}
 
-	fetched, err := ms.Get(tag)
+	fetched, err := ms.GetByTag(env.tag)
 	if err != nil {
 		t.Fatalf("unexpected error fetching manifest: %v", err)
 	}
@@ -231,7 +278,11 @@ func TestManifestStorage(t *testing.T) {
 		}
 	}
 
-	if err := ms.Delete(tag); err != nil {
+	// TODO(stevvooe): Currently, deletes are not supported due to some
-		t.Fatalf("unexpected error deleting manifest: %v", err)
+	// complexity around managing tag indexes. We'll add this support back in
+	// when the manifest format has settled. For now, we expect an error for
+	// all deletes.
+	if err := ms.Delete(dgst); err == nil {
+		t.Fatalf("unexpected an error deleting manifest by digest: %v", err)
 	}
 }

registry/storage/paths.go

@@ -72,11 +72,12 @@ const storagePathVersion = "v2"
 //
 //	Tags:
 //
-// 	manifestTagsPathSpec:          <root>/v2/repositories/<name>/_manifests/tags/
+// 	manifestTagsPathSpec:                  <root>/v2/repositories/<name>/_manifests/tags/
-// 	manifestTagPathSpec:           <root>/v2/repositories/<name>/_manifests/tags/<tag>/
+// 	manifestTagPathSpec:                   <root>/v2/repositories/<name>/_manifests/tags/<tag>/
-// 	manifestTagCurrentPathSpec:    <root>/v2/repositories/<name>/_manifests/tags/<tag>/current/link
+// 	manifestTagCurrentPathSpec:            <root>/v2/repositories/<name>/_manifests/tags/<tag>/current/link
-// 	manifestTagIndexPathSpec:      <root>/v2/repositories/<name>/_manifests/tags/<tag>/index/
+// 	manifestTagIndexPathSpec:              <root>/v2/repositories/<name>/_manifests/tags/<tag>/index/
-// 	manifestTagIndexEntryPathSpec: <root>/v2/repositories/<name>/_manifests/tags/<tag>/index/<algorithm>/<hex digest>/link
+// 	manifestTagIndexEntryPathSpec:         <root>/v2/repositories/<name>/_manifests/tags/<tag>/index/<algorithm>/<hex digest>/
+// 	manifestTagIndexEntryLinkPathSpec:     <root>/v2/repositories/<name>/_manifests/tags/<tag>/index/<algorithm>/<hex digest>/link
 //
 // 	Layers:
 //
@@ -199,6 +200,17 @@ func (pm *pathMapper) path(spec pathSpec) (string, error) {
 		}
 
 		return path.Join(root, "index"), nil
+	case manifestTagIndexEntryLinkPathSpec:
+		root, err := pm.path(manifestTagIndexEntryPathSpec{
+			name:     v.name,
+			tag:      v.tag,
+			revision: v.revision,
+		})
+		if err != nil {
+			return "", err
+		}
+
+		return path.Join(root, "link"), nil
 	case manifestTagIndexEntryPathSpec:
 		root, err := pm.path(manifestTagIndexPathSpec{
 			name: v.name,
@@ -213,7 +225,7 @@ func (pm *pathMapper) path(spec pathSpec) (string, error) {
 			return "", err
 		}
 
-		return path.Join(root, path.Join(append(components, "link")...)), nil
+		return path.Join(root, path.Join(components...)), nil
 	case layerLinkPathSpec:
 		components, err := digestPathComponents(v.digest, false)
 		if err != nil {
@@ -332,8 +344,7 @@ type manifestTagIndexPathSpec struct {
 
 func (manifestTagIndexPathSpec) pathSpec() {}
 
-// manifestTagIndexEntryPathSpec describes the link to a revisions of a
+// manifestTagIndexEntryPathSpec contains the entries of the index by revision.
-// manifest with given tag within the index.
 type manifestTagIndexEntryPathSpec struct {
 	name     string
 	tag      string
@@ -342,6 +353,16 @@ type manifestTagIndexEntryPathSpec struct {
 
 func (manifestTagIndexEntryPathSpec) pathSpec() {}
 
+// manifestTagIndexEntryLinkPathSpec describes the link to a revisions of a
+// manifest with given tag within the index.
+type manifestTagIndexEntryLinkPathSpec struct {
+	name     string
+	tag      string
+	revision digest.Digest
+}
+
+func (manifestTagIndexEntryLinkPathSpec) pathSpec() {}
+
 // layerLink specifies a path for a layer link, which is a file with a blob
 // id. The layer link will contain a content addressable blob id reference
 // into the blob store. The format of the contents is as follows:

registry/storage/paths_test.go

@@ -78,6 +78,14 @@ func TestPathMapper(t *testing.T) {
 				tag:      "thetag",
 				revision: "sha256:abcdef0123456789",
 			},
+			expected: "/pathmapper-test/repositories/foo/bar/_manifests/tags/thetag/index/sha256/abcdef0123456789",
+		},
+		{
+			spec: manifestTagIndexEntryLinkPathSpec{
+				name:     "foo/bar",
+				tag:      "thetag",
+				revision: "sha256:abcdef0123456789",
+			},
 			expected: "/pathmapper-test/repositories/foo/bar/_manifests/tags/thetag/index/sha256/abcdef0123456789/link",
 		},
 		{

registry/storage/tagstore.go

@@ -63,7 +63,7 @@ func (ts *tagStore) exists(tag string) (bool, error) {
 // tag tags the digest with the given tag, updating the the store to point at
 // the current tag. The digest must point to a manifest.
 func (ts *tagStore) tag(tag string, revision digest.Digest) error {
-	indexEntryPath, err := ts.pm.path(manifestTagIndexEntryPathSpec{
+	indexEntryPath, err := ts.pm.path(manifestTagIndexEntryLinkPathSpec{
 		name:     ts.Name(),
 		tag:      tag,
 		revision: revision,