Merge pull request #211 from stevvooe/immutable-manifest-references

doc/spec, registry: immutable manifest reference support

docs/api/v2/descriptors.go

@@ -79,6 +79,13 @@ var (
 		Format:      "<uuid>",
 	}
 
+	digestHeader = ParameterDescriptor{
+		Name:        "Docker-Content-Digest",
+		Description: "Digest of the targeted content for the request.",
+		Type:        "digest",
+		Format:      "<digest>",
+	}
+
 	unauthorizedResponse = ResponseDescriptor{
 		Description: "The client does not have access to the repository.",
 		StatusCode:  http.StatusUnauthorized,
@@ -454,13 +461,13 @@ var routeDescriptors = []RouteDescriptor{
 	},
 	{
 		Name:        RouteNameManifest,
-		Path:        "/v2/{name:" + RepositoryNameRegexp.String() + "}/manifests/{tag:" + TagNameRegexp.String() + "}",
+		Path:        "/v2/{name:" + RepositoryNameRegexp.String() + "}/manifests/{reference:" + TagNameRegexp.String() + "|" + digest.DigestRegexp.String() + "}",
 		Entity:      "Manifest",
 		Description: "Create, update and retrieve manifests.",
 		Methods: []MethodDescriptor{
 			{
 				Method:      "GET",
-				Description: "Fetch the manifest identified by `name` and `tag`.",
+				Description: "Fetch the manifest identified by `name` and `reference` where `reference` can be a tag or digest.",
 				Requests: []RequestDescriptor{
 					{
 						Headers: []ParameterDescriptor{
@@ -473,8 +480,11 @@ var routeDescriptors = []RouteDescriptor{
 						},
 						Successes: []ResponseDescriptor{
 							{
-								Description: "The manifest idenfied by `name` and `tag`. The contents can be used to identify and resolve resources required to run the specified image.",
+								Description: "The manifest idenfied by `name` and `reference`. The contents can be used to identify and resolve resources required to run the specified image.",
 								StatusCode:  http.StatusOK,
+								Headers: []ParameterDescriptor{
+									digestHeader,
+								},
 								Body: BodyDescriptor{
 									ContentType: "application/json; charset=utf-8",
 									Format:      manifestBody,
@@ -483,7 +493,7 @@ var routeDescriptors = []RouteDescriptor{
 						},
 						Failures: []ResponseDescriptor{
 							{
-								Description: "The name or tag was invalid.",
+								Description: "The name or reference was invalid.",
 								StatusCode:  http.StatusBadRequest,
 								ErrorCodes: []ErrorCode{
 									ErrorCodeNameInvalid,
@@ -523,7 +533,7 @@ var routeDescriptors = []RouteDescriptor{
 			},
 			{
 				Method:      "PUT",
-				Description: "Put the manifest identified by `name` and `tag`.",
+				Description: "Put the manifest identified by `name` and `reference` where `reference` can be a tag or digest.",
 				Requests: []RequestDescriptor{
 					{
 						Headers: []ParameterDescriptor{
@@ -550,6 +560,7 @@ var routeDescriptors = []RouteDescriptor{
 										Format:      "<url>",
 									},
 									contentLengthZeroHeader,
+									digestHeader,
 								},
 							},
 						},
@@ -628,7 +639,7 @@ var routeDescriptors = []RouteDescriptor{
 			},
 			{
 				Method:      "DELETE",
-				Description: "Delete the manifest identified by `name` and `tag`.",
+				Description: "Delete the manifest identified by `name` and `reference` where `reference` can be a tag or digest.",
 				Requests: []RequestDescriptor{
 					{
 						Headers: []ParameterDescriptor{
@@ -729,6 +740,7 @@ var routeDescriptors = []RouteDescriptor{
 										Description: "The length of the requested blob content.",
 										Format:      "<length>",
 									},
+									digestHeader,
 								},
 								Body: BodyDescriptor{
 									ContentType: "application/octet-stream",
@@ -745,6 +757,7 @@ var routeDescriptors = []RouteDescriptor{
 										Description: "The location where the layer should be accessible.",
 										Format:      "<blob location>",
 									},
+									digestHeader,
 								},
 							},
 						},
@@ -1193,6 +1206,7 @@ var routeDescriptors = []RouteDescriptor{
 										Format:      "<length of chunk>",
 										Description: "Length of the chunk being uploaded, corresponding the length of the request body.",
 									},
+									digestHeader,
 								},
 							},
 						},
@@ -1312,6 +1326,13 @@ var errorDescriptors = []ErrorDescriptor{
 		Description: `Generic error returned when the error does not have an
 		API classification.`,
 	},
+	{
+		Code:    ErrorCodeUnsupported,
+		Value:   "UNSUPPORTED",
+		Message: "The operation is unsupported.",
+		Description: `The operation was unsupported due to a missing
+		implementation or invalid set of parameters.`,
+	},
 	{
 		Code:    ErrorCodeUnauthorized,
 		Value:   "UNAUTHORIZED",

docs/api/v2/errors.go

@@ -13,6 +13,9 @@ const (
 	// ErrorCodeUnknown is a catch-all for errors not defined below.
 	ErrorCodeUnknown ErrorCode = iota
 
+	// ErrorCodeUnsupported is returned when an operation is not supported.
+	ErrorCodeUnsupported
+
 	// ErrorCodeUnauthorized is returned if a request is not authorized.
 	ErrorCodeUnauthorized
 

docs/api/v2/routes_test.go

@@ -39,16 +39,24 @@ func TestRouter(t *testing.T) {
 			RouteName:  RouteNameManifest,
 			RequestURI: "/v2/foo/manifests/bar",
 			Vars: map[string]string{
-				"name": "foo",
+				"name":      "foo",
-				"tag":  "bar",
+				"reference": "bar",
 			},
 		},
 		{
 			RouteName:  RouteNameManifest,
 			RequestURI: "/v2/foo/bar/manifests/tag",
 			Vars: map[string]string{
-				"name": "foo/bar",
+				"name":      "foo/bar",
-				"tag":  "tag",
+				"reference": "tag",
+			},
+		},
+		{
+			RouteName:  RouteNameManifest,
+			RequestURI: "/v2/foo/bar/manifests/sha256:abcdef01234567890",
+			Vars: map[string]string{
+				"name":      "foo/bar",
+				"reference": "sha256:abcdef01234567890",
 			},
 		},
 		{
@@ -112,8 +120,8 @@ func TestRouter(t *testing.T) {
 			RouteName:  RouteNameManifest,
 			RequestURI: "/v2/foo/bar/manifests/manifests/tags",
 			Vars: map[string]string{
-				"name": "foo/bar/manifests",
+				"name":      "foo/bar/manifests",
-				"tag":  "tags",
+				"reference": "tags",
 			},
 		},
 		{

docs/api/v2/urls.go

@@ -107,11 +107,12 @@ func (ub *URLBuilder) BuildTagsURL(name string) (string, error) {
 	return tagsURL.String(), nil
 }
 
-// BuildManifestURL constructs a url for the manifest identified by name and tag.
+// BuildManifestURL constructs a url for the manifest identified by name and
-func (ub *URLBuilder) BuildManifestURL(name, tag string) (string, error) {
+// reference. The argument reference may be either a tag or digest.
+func (ub *URLBuilder) BuildManifestURL(name, reference string) (string, error) {
 	route := ub.cloneRoute(RouteNameManifest)
 
-	manifestURL, err := route.URL("name", name, "tag", tag)
+	manifestURL, err := route.URL("name", name, "reference", reference)
 	if err != nil {
 		return "", err
 	}

docs/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)
+	}
+}

docs/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{

docs/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",
 			},
 		},
 		{

docs/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")

docs/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
 }

docs/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 {

docs/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)
 }
 

docs/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:

docs/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)
 	}
 }

docs/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:

docs/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",
 		},
 		{

docs/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,