From f46a1b73e8d7b26716a5164afd7f9fc756e7fca7 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Wed, 25 Feb 2015 18:04:28 -0800 Subject: [PATCH 1/2] spec: fetch manifests by tag or digest Manifests are now fetched by a field called "reference", which may be a tag or a digest. When using digests to reference a manifest, the data is immutable. The routes and specification have been updated to allow this. There are a few caveats to this approach: 1. It may be problematic to rely on data format to differentiate between a tag and a digest. Currently, they are disjoint but there may modifications on either side that break this guarantee. 2. The caching characteristics of returned content are very different for digest versus tag-based references. Digest urls can be cached forever while tag urls cannot. Both of these are minimal caveats that we can live with in the future. Signed-off-by: Stephen J Day --- docs/api/v2/descriptors.go | 33 +++++++++++++++++++++++++++------ docs/api/v2/errors.go | 3 +++ docs/api/v2/routes_test.go | 20 ++++++++++++++------ docs/api/v2/urls.go | 7 ++++--- 4 files changed, 48 insertions(+), 15 deletions(-) diff --git a/docs/api/v2/descriptors.go b/docs/api/v2/descriptors.go index 301fd596e..5f091bbc9 100644 --- a/docs/api/v2/descriptors.go +++ b/docs/api/v2/descriptors.go @@ -79,6 +79,13 @@ var ( Format: "", } + digestHeader = ParameterDescriptor{ + Name: "Docker-Content-Digest", + Description: "Digest of the targeted content for the request.", + Type: "digest", + Format: "", + } + 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: "", }, 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: "", }, + digestHeader, }, Body: BodyDescriptor{ ContentType: "application/octet-stream", @@ -745,6 +757,7 @@ var routeDescriptors = []RouteDescriptor{ Description: "The location where the layer should be accessible.", Format: "", }, + digestHeader, }, }, }, @@ -1193,6 +1206,7 @@ var routeDescriptors = []RouteDescriptor{ Format: "", 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", diff --git a/docs/api/v2/errors.go b/docs/api/v2/errors.go index 4d5d55c7a..cbae020ef 100644 --- a/docs/api/v2/errors.go +++ b/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 diff --git a/docs/api/v2/routes_test.go b/docs/api/v2/routes_test.go index 9157e21e5..afab71fce 100644 --- a/docs/api/v2/routes_test.go +++ b/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", - "tag": "bar", + "name": "foo", + "reference": "bar", }, }, { RouteName: RouteNameManifest, RequestURI: "/v2/foo/bar/manifests/tag", Vars: map[string]string{ - "name": "foo/bar", - "tag": "tag", + "name": "foo/bar", + "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", - "tag": "tags", + "name": "foo/bar/manifests", + "reference": "tags", }, }, { diff --git a/docs/api/v2/urls.go b/docs/api/v2/urls.go index e36afdabf..4b42dd162 100644 --- a/docs/api/v2/urls.go +++ b/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. -func (ub *URLBuilder) BuildManifestURL(name, tag string) (string, error) { +// BuildManifestURL constructs a url for the manifest identified by name and +// 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 } From 008236cfef2e9eaada4eaf2a99dfd11f902122e9 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Thu, 26 Feb 2015 15:47:04 -0800 Subject: [PATCH 2/2] 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 --- docs/handlers/api_test.go | 68 ++++++++++++++- docs/handlers/app.go | 3 +- docs/handlers/app_test.go | 2 +- docs/handlers/context.go | 4 +- docs/handlers/images.go | 129 +++++++++++++++++++++++++---- docs/handlers/layer.go | 2 + docs/handlers/layerupload.go | 5 ++ docs/storage/manifeststore.go | 75 +++++++---------- docs/storage/manifeststore_test.go | 89 +++++++++++++++----- docs/storage/paths.go | 37 +++++++-- docs/storage/paths_test.go | 8 ++ docs/storage/tagstore.go | 2 +- 12 files changed, 325 insertions(+), 99 deletions(-) diff --git a/docs/handlers/api_test.go b/docs/handlers/api_test.go index 902cb9a66..4a273b288 100644 --- a/docs/handlers/api_test.go +++ b/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}, - "Content-Length": []string{"0"}, + "Location": []string{expectedLayerURL}, + "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) + } +} diff --git a/docs/handlers/app.go b/docs/handlers/app.go index 199ca180f..12837cc88 100644 --- a/docs/handlers/app.go +++ b/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{ diff --git a/docs/handlers/app_test.go b/docs/handlers/app_test.go index 158f5fc18..ba580b118 100644 --- a/docs/handlers/app_test.go +++ b/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", }, }, { diff --git a/docs/handlers/context.go b/docs/handlers/context.go index ee02a53af..5496a7941 100644 --- a/docs/handlers/context.go +++ b/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) { - return ctxu.GetStringValue(ctx, "vars.tag") +func getReference(ctx context.Context) (reference string) { + return ctxu.GetStringValue(ctx, "vars.reference") } var errDigestNotAvailable = fmt.Errorf("digest not available in context") diff --git a/docs/handlers/images.go b/docs/handlers/images.go index de7b6dd6c..174bd3d94 100644 --- a/docs/handlers/images.go +++ b/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.Write(manifest.Raw) + w.Header().Set("Content-Length", fmt.Sprint(len(sm.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 { - switch err := err.(type) { - case distribution.ErrManifestUnknown: - imh.Errors.Push(v2.ErrorCodeManifestUnknown, err) - w.WriteHeader(http.StatusNotFound) - default: - imh.Errors.Push(v2.ErrorCodeUnknown, err) - w.WriteHeader(http.StatusBadRequest) + + // TODO(stevvooe): Unfortunately, at this point, manifest deletes are + // unsupported. There are issues with schema version 1 that make removing + // tag index entries a serious problem in eventually consistent storage. + // Once we work out schema version 2, the full deletion system will be + // worked out and we can add support back. + imh.Errors.Push(v2.ErrorCodeUnsupported) + 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") - w.WriteHeader(http.StatusAccepted) + dgst, err := digest.FromBytes(p) + if err != nil { + ctxu.GetLogger(ctx).Errorf("error digesting manifest: %v", err) + return "", err + } + + return dgst, err } diff --git a/docs/handlers/layer.go b/docs/handlers/layer.go index 69c3df7cd..913002e0e 100644 --- a/docs/handlers/layer.go +++ b/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 { diff --git a/docs/handlers/layerupload.go b/docs/handlers/layerupload.go index 0f0be27f0..b728d0e1a 100644 --- a/docs/handlers/layerupload.go +++ b/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) } diff --git a/docs/storage/manifeststore.go b/docs/storage/manifeststore.go index 765b5d056..4946785d3 100644 --- a/docs/storage/manifeststore.go +++ b/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 { -// 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) { +func (ms *manifestStore) Exists(dgst digest.Digest) (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 -// semantics in the future, but this will maintain consistency. The underlying -// blobs are left alone. -func (ms *manifestStore) Delete(tag string) error { - ctxu.GetLogger(ms.repository.ctx).Debug("(*manifestStore).Delete") +// Delete removes the revision of the specified manfiest. +func (ms *manifestStore) Delete(dgst digest.Digest) error { + ctxu.GetLogger(ms.repository.ctx).Debug("(*manifestStore).Delete - unsupported") + return fmt.Errorf("deletion of manifests not supported") +} - 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 { - if err := ms.revisionStore.delete(revision); err != nil { - return err - } - } - - return ms.tagStore.delete(tag) + return ms.revisionStore.get(dgst) } // verifyManifest ensures that the manifest content is valid from the -// perspective of the registry. It ensures that the name and tag match and -// that the signature is valid for the enclosed payload. As a policy, the -// registry only tries to store valid content, leaving trust policies of that -// content up to consumers. -func (ms *manifestStore) verifyManifest(tag string, mnfst *manifest.SignedManifest) error { +// perspective of the registry. It ensures that the signature is valid for the +// enclosed payload. As a policy, the registry only tries to store valid +// content, leaving trust policies of that content up to consumers. +func (ms *manifestStore) verifyManifest(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: diff --git a/docs/storage/manifeststore_test.go b/docs/storage/manifeststore_test.go index d3a55ce55..dc03dcedd 100644 --- a/docs/storage/manifeststore_test.go +++ b/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, - Tag: tag, + Name: env.name, + 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 { - t.Fatalf("unexpected tag found in tags: %v != %v", tags, []string{tag}) + if tags[0] != env.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 { - t.Fatalf("unexpected error deleting manifest: %v", err) + // TODO(stevvooe): Currently, deletes are not supported due to some + // 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) } } diff --git a/docs/storage/paths.go b/docs/storage/paths.go index 9380dc651..173e98a80 100644 --- a/docs/storage/paths.go +++ b/docs/storage/paths.go @@ -72,11 +72,12 @@ const storagePathVersion = "v2" // // Tags: // -// manifestTagsPathSpec: /v2/repositories//_manifests/tags/ -// manifestTagPathSpec: /v2/repositories//_manifests/tags// -// manifestTagCurrentPathSpec: /v2/repositories//_manifests/tags//current/link -// manifestTagIndexPathSpec: /v2/repositories//_manifests/tags//index/ -// manifestTagIndexEntryPathSpec: /v2/repositories//_manifests/tags//index///link +// manifestTagsPathSpec: /v2/repositories//_manifests/tags/ +// manifestTagPathSpec: /v2/repositories//_manifests/tags// +// manifestTagCurrentPathSpec: /v2/repositories//_manifests/tags//current/link +// manifestTagIndexPathSpec: /v2/repositories//_manifests/tags//index/ +// manifestTagIndexEntryPathSpec: /v2/repositories//_manifests/tags//index/// +// manifestTagIndexEntryLinkPathSpec: /v2/repositories//_manifests/tags//index///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 -// manifest with given tag within the index. +// manifestTagIndexEntryPathSpec contains the entries of the index by revision. 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: diff --git a/docs/storage/paths_test.go b/docs/storage/paths_test.go index 79410e75f..7dff6e093 100644 --- a/docs/storage/paths_test.go +++ b/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", }, { diff --git a/docs/storage/tagstore.go b/docs/storage/tagstore.go index 147623a29..616df9526 100644 --- a/docs/storage/tagstore.go +++ b/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,