diff --git a/notifications/listener.go b/notifications/listener.go index b55fe326..48c42af2 100644 --- a/notifications/listener.go +++ b/notifications/listener.go @@ -67,8 +67,8 @@ type manifestServiceListener struct { parent *repositoryListener } -func (msl *manifestServiceListener) Get(tag string) (*manifest.SignedManifest, error) { - sm, err := msl.ManifestService.Get(tag) +func (msl *manifestServiceListener) Get(dgst digest.Digest) (*manifest.SignedManifest, error) { + 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 { - err := msl.ManifestService.Put(tag, sm) +func (msl *manifestServiceListener) Put(sm *manifest.SignedManifest) error { + 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 diff --git a/notifications/listener_test.go b/notifications/listener_test.go index c1cb1f70..18633bc6 100644 --- a/notifications/listener_test.go +++ b/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) } } diff --git a/registry.go b/registry.go index ef63a24a..6bc8e2aa 100644 --- a/registry.go +++ b/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. - Exists(tag string) (bool, error) + // ExistsByTag returns true if the manifest exists. + ExistsByTag(tag string) (bool, error) - // Get retrieves the named manifest, if it exists. - Get(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 + // GetByTag retrieves the named manifest, if it exists. + GetByTag(tag string) (*manifest.SignedManifest, error) // TODO(stevvooe): There are several changes that need to be done to this // interface: // - // 1. Get(tag string) should be GetByTag(tag string) - // 2. Put(tag string, manifest *manifest.SignedManifest) should be - // 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 + // 1. Allow explicit tagging with Tag(digest digest.Digest, tag string) + // 2. 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. } diff --git a/registry/handlers/api_test.go b/registry/handlers/api_test.go index 902cb9a6..4a273b28 100644 --- a/registry/handlers/api_test.go +++ b/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}, - "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/registry/handlers/app.go b/registry/handlers/app.go index 199ca180..12837cc8 100644 --- a/registry/handlers/app.go +++ b/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{ diff --git a/registry/handlers/app_test.go b/registry/handlers/app_test.go index 158f5fc1..ba580b11 100644 --- a/registry/handlers/app_test.go +++ b/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", }, }, { diff --git a/registry/handlers/context.go b/registry/handlers/context.go index ee02a53a..5496a794 100644 --- a/registry/handlers/context.go +++ b/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) { - 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/registry/handlers/images.go b/registry/handlers/images.go index de7b6dd6..174bd3d9 100644 --- a/registry/handlers/images.go +++ b/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.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/registry/handlers/layer.go b/registry/handlers/layer.go index 69c3df7c..913002e0 100644 --- a/registry/handlers/layer.go +++ b/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 { diff --git a/registry/handlers/layerupload.go b/registry/handlers/layerupload.go index 0f0be27f..b728d0e1 100644 --- a/registry/handlers/layerupload.go +++ b/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) } diff --git a/registry/storage/manifeststore.go b/registry/storage/manifeststore.go index 765b5d05..4946785d 100644 --- a/registry/storage/manifeststore.go +++ b/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 { -// 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/registry/storage/manifeststore_test.go b/registry/storage/manifeststore_test.go index d3a55ce5..dc03dced 100644 --- a/registry/storage/manifeststore_test.go +++ b/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, - 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/registry/storage/paths.go b/registry/storage/paths.go index 9380dc65..173e98a8 100644 --- a/registry/storage/paths.go +++ b/registry/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/registry/storage/paths_test.go b/registry/storage/paths_test.go index 79410e75..7dff6e09 100644 --- a/registry/storage/paths_test.go +++ b/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", }, { diff --git a/registry/storage/tagstore.go b/registry/storage/tagstore.go index 147623a2..616df952 100644 --- a/registry/storage/tagstore.go +++ b/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,