Factor out schema-specific portions of manifestStore

Create signedManifestHandler and schema2ManifestHandler. Use these to
unmarshal and put the respective types of manifests from manifestStore.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>

docs/storage/manifeststore.go

@@ -1,24 +1,51 @@
 package storage
 
 import (
-	"encoding/json"
 	"fmt"
 
+	"encoding/json"
 	"github.com/docker/distribution"
 	"github.com/docker/distribution/context"
 	"github.com/docker/distribution/digest"
+	"github.com/docker/distribution/manifest"
 	"github.com/docker/distribution/manifest/schema1"
-	"github.com/docker/distribution/reference"
+	"github.com/docker/distribution/manifest/schema2"
-	"github.com/docker/libtrust"
 )
 
-// manifestStore is a storage driver based store for storing schema1 manifests.
+// A ManifestHandler gets and puts manifests of a particular type.
+type ManifestHandler interface {
+	// Unmarshal unmarshals the manifest from a byte slice.
+	Unmarshal(ctx context.Context, dgst digest.Digest, content []byte) (distribution.Manifest, error)
+
+	// Put creates or updates the given manifest returning the manifest digest.
+	Put(ctx context.Context, manifest distribution.Manifest, skipDependencyVerification bool) (digest.Digest, error)
+}
+
+// SkipLayerVerification allows a manifest to be Put before its
+// layers are on the filesystem
+func SkipLayerVerification() distribution.ManifestServiceOption {
+	return skipLayerOption{}
+}
+
+type skipLayerOption struct{}
+
+func (o skipLayerOption) Apply(m distribution.ManifestService) error {
+	if ms, ok := m.(*manifestStore); ok {
+		ms.skipDependencyVerification = true
+		return nil
+	}
+	return fmt.Errorf("skip layer verification only valid for manifestStore")
+}
+
 type manifestStore struct {
-	repository                 *repository
+	repository *repository
-	blobStore                  *linkedBlobStore
+	blobStore  *linkedBlobStore
-	ctx                        context.Context
+	ctx        context.Context
-	signatures                 *signatureStore
+
 	skipDependencyVerification bool
+
+	schema1Handler ManifestHandler
+	schema2Handler ManifestHandler
 }
 
 var _ distribution.ManifestService = &manifestStore{}
@@ -40,18 +67,6 @@ func (ms *manifestStore) Exists(ctx context.Context, dgst digest.Digest) (bool,
 
 func (ms *manifestStore) Get(ctx context.Context, dgst digest.Digest, options ...distribution.ManifestServiceOption) (distribution.Manifest, error) {
 	context.GetLogger(ms.ctx).Debug("(*manifestStore).Get")
-	// Ensure that this revision is available in this repository.
-	_, err := ms.blobStore.Stat(ctx, dgst)
-	if err != nil {
-		if err == distribution.ErrBlobUnknown {
-			return nil, distribution.ErrManifestUnknownRevision{
-				Name:     ms.repository.Name(),
-				Revision: dgst,
-			}
-		}
-
-		return nil, err
-	}
 
 	// TODO(stevvooe): Need to check descriptor from above to ensure that the
 	// mediatype is as we expect for the manifest store.
@@ -68,84 +83,32 @@ func (ms *manifestStore) Get(ctx context.Context, dgst digest.Digest, options ..
 		return nil, err
 	}
 
-	// Fetch the signatures for the manifest
+	var versioned manifest.Versioned
-	signatures, err := ms.signatures.Get(dgst)
+	if err = json.Unmarshal(content, &versioned); err != nil {
-	if err != nil {
 		return nil, err
 	}
 
-	jsig, err := libtrust.NewJSONSignature(content, signatures...)
+	switch versioned.SchemaVersion {
-	if err != nil {
+	case 1:
-		return nil, err
+		return ms.schema1Handler.Unmarshal(ctx, dgst, content)
+	case 2:
+		return ms.schema2Handler.Unmarshal(ctx, dgst, content)
 	}
 
-	// Extract the pretty JWS
+	return nil, fmt.Errorf("unrecognized manifest schema version %d", versioned.SchemaVersion)
-	raw, err := jsig.PrettySignature("signatures")
-	if err != nil {
-		return nil, err
-	}
-
-	var sm schema1.SignedManifest
-	if err := json.Unmarshal(raw, &sm); err != nil {
-		return nil, err
-	}
-
-	return &sm, nil
-}
-
-// SkipLayerVerification allows a manifest to be Put before its
-// layers are on the filesystem
-func SkipLayerVerification() distribution.ManifestServiceOption {
-	return skipLayerOption{}
-}
-
-type skipLayerOption struct{}
-
-func (o skipLayerOption) Apply(m distribution.ManifestService) error {
-	if ms, ok := m.(*manifestStore); ok {
-		ms.skipDependencyVerification = true
-		return nil
-	}
-	return fmt.Errorf("skip layer verification only valid for manifestStore")
 }
 
 func (ms *manifestStore) Put(ctx context.Context, manifest distribution.Manifest, options ...distribution.ManifestServiceOption) (digest.Digest, error) {
 	context.GetLogger(ms.ctx).Debug("(*manifestStore).Put")
 
-	sm, ok := manifest.(*schema1.SignedManifest)
+	switch manifest.(type) {
-	if !ok {
+	case *schema1.SignedManifest:
-		return "", fmt.Errorf("non-v1 manifest put to signed manifestStore: %T", manifest)
+		return ms.schema1Handler.Put(ctx, manifest, ms.skipDependencyVerification)
+	case *schema2.DeserializedManifest:
+		return ms.schema2Handler.Put(ctx, manifest, ms.skipDependencyVerification)
 	}
 
-	if err := ms.verifyManifest(ms.ctx, *sm); err != nil {
+	return "", fmt.Errorf("unrecognized manifest type %T", manifest)
-		return "", err
-	}
-
-	mt := schema1.MediaTypeManifest
-	payload := sm.Canonical
-
-	revision, err := ms.blobStore.Put(ctx, mt, payload)
-	if err != nil {
-		context.GetLogger(ctx).Errorf("error putting payload into blobstore: %v", err)
-		return "", err
-	}
-
-	// Link the revision into the repository.
-	if err := ms.blobStore.linkBlob(ctx, revision); err != nil {
-		return "", err
-	}
-
-	// Grab each json signature and store them.
-	signatures, err := sm.Signatures()
-	if err != nil {
-		return "", err
-	}
-
-	if err := ms.signatures.Put(revision.Digest, signatures...); err != nil {
-		return "", err
-	}
-
-	return revision.Digest, nil
 }
 
 // Delete removes the revision of the specified manfiest.
@@ -157,64 +120,3 @@ func (ms *manifestStore) Delete(ctx context.Context, dgst digest.Digest) error {
 func (ms *manifestStore) Enumerate(ctx context.Context, manifests []distribution.Manifest, last distribution.Manifest) (n int, err error) {
 	return 0, distribution.ErrUnsupported
 }
-
-// verifyManifest ensures that the manifest content is valid from the
-// 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 consumems.
-func (ms *manifestStore) verifyManifest(ctx context.Context, mnfst schema1.SignedManifest) error {
-	var errs distribution.ErrManifestVerification
-
-	if len(mnfst.Name) > reference.NameTotalLengthMax {
-		errs = append(errs,
-			distribution.ErrManifestNameInvalid{
-				Name:   mnfst.Name,
-				Reason: fmt.Errorf("manifest name must not be more than %v characters", reference.NameTotalLengthMax),
-			})
-	}
-
-	if !reference.NameRegexp.MatchString(mnfst.Name) {
-		errs = append(errs,
-			distribution.ErrManifestNameInvalid{
-				Name:   mnfst.Name,
-				Reason: fmt.Errorf("invalid manifest name format"),
-			})
-	}
-
-	if len(mnfst.History) != len(mnfst.FSLayers) {
-		errs = append(errs, fmt.Errorf("mismatched history and fslayer cardinality %d != %d",
-			len(mnfst.History), len(mnfst.FSLayers)))
-	}
-
-	if _, err := schema1.Verify(&mnfst); err != nil {
-		switch err {
-		case libtrust.ErrMissingSignatureKey, libtrust.ErrInvalidJSONContent, libtrust.ErrMissingSignatureKey:
-			errs = append(errs, distribution.ErrManifestUnverified{})
-		default:
-			if err.Error() == "invalid signature" { // TODO(stevvooe): This should be exported by libtrust
-				errs = append(errs, distribution.ErrManifestUnverified{})
-			} else {
-				errs = append(errs, err)
-			}
-		}
-	}
-
-	if !ms.skipDependencyVerification {
-		for _, fsLayer := range mnfst.References() {
-			_, err := ms.repository.Blobs(ctx).Stat(ctx, fsLayer.Digest)
-			if err != nil {
-				if err != distribution.ErrBlobUnknown {
-					errs = append(errs, err)
-				}
-
-				// On error here, we always append unknown blob erroms.
-				errs = append(errs, distribution.ErrManifestBlobUnknown{Digest: fsLayer.Digest})
-			}
-		}
-	}
-	if len(errs) != 0 {
-		return errs
-	}
-
-	return nil
-}

docs/storage/registry.go

@@ -165,28 +165,40 @@ func (repo *repository) Manifests(ctx context.Context, options ...distribution.M
 		blobLinkPath,
 	}
 
+	blobStore := &linkedBlobStore{
+		ctx:           ctx,
+		blobStore:     repo.blobStore,
+		repository:    repo,
+		deleteEnabled: repo.registry.deleteEnabled,
+		blobAccessController: &linkedBlobStatter{
+			blobStore:   repo.blobStore,
+			repository:  repo,
+			linkPathFns: manifestLinkPathFns,
+		},
+
+		// TODO(stevvooe): linkPath limits this blob store to only
+		// manifests. This instance cannot be used for blob checks.
+		linkPathFns: manifestLinkPathFns,
+	}
+
 	ms := &manifestStore{
 		ctx:        ctx,
 		repository: repo,
-		blobStore: &linkedBlobStore{
+		blobStore:  blobStore,
-			ctx:           ctx,
+		schema1Handler: &signedManifestHandler{
-			blobStore:     repo.blobStore,
-			repository:    repo,
-			deleteEnabled: repo.registry.deleteEnabled,
-			blobAccessController: &linkedBlobStatter{
-				blobStore:   repo.blobStore,
-				repository:  repo,
-				linkPathFns: manifestLinkPathFns,
-			},
-
-			// TODO(stevvooe): linkPath limits this blob store to only
-			// manifests. This instance cannot be used for blob checks.
-			linkPathFns: manifestLinkPathFns,
-		},
-		signatures: &signatureStore{
 			ctx:        ctx,
 			repository: repo,
-			blobStore:  repo.blobStore,
+			blobStore:  blobStore,
+			signatures: &signatureStore{
+				ctx:        ctx,
+				repository: repo,
+				blobStore:  repo.blobStore,
+			},
+		},
+		schema2Handler: &schema2ManifestHandler{
+			ctx:        ctx,
+			repository: repo,
+			blobStore:  blobStore,
 		},
 	}
 

docs/storage/schema2manifesthandler.go

@@ -0,0 +1,100 @@
+package storage
+
+import (
+	"fmt"
+
+	"encoding/json"
+	"github.com/docker/distribution"
+	"github.com/docker/distribution/context"
+	"github.com/docker/distribution/digest"
+	"github.com/docker/distribution/manifest/schema2"
+)
+
+//schema2ManifestHandler is a ManifestHandler that covers schema2 manifests.
+type schema2ManifestHandler struct {
+	repository *repository
+	blobStore  *linkedBlobStore
+	ctx        context.Context
+}
+
+var _ ManifestHandler = &schema2ManifestHandler{}
+
+func (ms *schema2ManifestHandler) Unmarshal(ctx context.Context, dgst digest.Digest, content []byte) (distribution.Manifest, error) {
+	context.GetLogger(ms.ctx).Debug("(*schema2ManifestHandler).Unmarshal")
+
+	var m schema2.DeserializedManifest
+	if err := json.Unmarshal(content, &m); err != nil {
+		return nil, err
+	}
+
+	return &m, nil
+}
+
+func (ms *schema2ManifestHandler) Put(ctx context.Context, manifest distribution.Manifest, skipDependencyVerification bool) (digest.Digest, error) {
+	context.GetLogger(ms.ctx).Debug("(*schema2ManifestHandler).Put")
+
+	m, ok := manifest.(*schema2.DeserializedManifest)
+	if !ok {
+		return "", fmt.Errorf("non-schema2 manifest put to schema2ManifestHandler: %T", manifest)
+	}
+
+	if err := ms.verifyManifest(ms.ctx, *m, skipDependencyVerification); err != nil {
+		return "", err
+	}
+
+	mt, payload, err := m.Payload()
+	if err != nil {
+		return "", err
+	}
+
+	revision, err := ms.blobStore.Put(ctx, mt, payload)
+	if err != nil {
+		context.GetLogger(ctx).Errorf("error putting payload into blobstore: %v", err)
+		return "", err
+	}
+
+	// Link the revision into the repository.
+	if err := ms.blobStore.linkBlob(ctx, revision); err != nil {
+		return "", err
+	}
+
+	return revision.Digest, nil
+}
+
+// verifyManifest ensures that the manifest content is valid from the
+// 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 consumems.
+func (ms *schema2ManifestHandler) verifyManifest(ctx context.Context, mnfst schema2.DeserializedManifest, skipDependencyVerification bool) error {
+	var errs distribution.ErrManifestVerification
+
+	if !skipDependencyVerification {
+		target := mnfst.Target()
+		_, err := ms.repository.Blobs(ctx).Stat(ctx, target.Digest)
+		if err != nil {
+			if err != distribution.ErrBlobUnknown {
+				errs = append(errs, err)
+			}
+
+			// On error here, we always append unknown blob errors.
+			errs = append(errs, distribution.ErrManifestBlobUnknown{Digest: target.Digest})
+		}
+
+		for _, fsLayer := range mnfst.References() {
+			_, err := ms.repository.Blobs(ctx).Stat(ctx, fsLayer.Digest)
+			if err != nil {
+				if err != distribution.ErrBlobUnknown {
+					errs = append(errs, err)
+				}
+
+				// On error here, we always append unknown blob errors.
+				errs = append(errs, distribution.ErrManifestBlobUnknown{Digest: fsLayer.Digest})
+			}
+		}
+	}
+	if len(errs) != 0 {
+		return errs
+	}
+
+	return nil
+}

docs/storage/signedmanifesthandler.go

@@ -0,0 +1,150 @@
+package storage
+
+import (
+	"encoding/json"
+	"fmt"
+
+	"github.com/docker/distribution"
+	"github.com/docker/distribution/context"
+	"github.com/docker/distribution/digest"
+	"github.com/docker/distribution/manifest/schema1"
+	"github.com/docker/distribution/reference"
+	"github.com/docker/libtrust"
+)
+
+// signedManifestHandler is a ManifestHandler that covers schema1 manifests. It
+// can unmarshal and put schema1 manifests that have been signed by libtrust.
+type signedManifestHandler struct {
+	repository *repository
+	blobStore  *linkedBlobStore
+	ctx        context.Context
+	signatures *signatureStore
+}
+
+var _ ManifestHandler = &signedManifestHandler{}
+
+func (ms *signedManifestHandler) Unmarshal(ctx context.Context, dgst digest.Digest, content []byte) (distribution.Manifest, error) {
+	context.GetLogger(ms.ctx).Debug("(*signedManifestHandler).Unmarshal")
+	// Fetch the signatures for the manifest
+	signatures, err := ms.signatures.Get(dgst)
+	if err != nil {
+		return nil, err
+	}
+
+	jsig, err := libtrust.NewJSONSignature(content, signatures...)
+	if err != nil {
+		return nil, err
+	}
+
+	// Extract the pretty JWS
+	raw, err := jsig.PrettySignature("signatures")
+	if err != nil {
+		return nil, err
+	}
+
+	var sm schema1.SignedManifest
+	if err := json.Unmarshal(raw, &sm); err != nil {
+		return nil, err
+	}
+	return &sm, nil
+}
+
+func (ms *signedManifestHandler) Put(ctx context.Context, manifest distribution.Manifest, skipDependencyVerification bool) (digest.Digest, error) {
+	context.GetLogger(ms.ctx).Debug("(*signedManifestHandler).Put")
+
+	sm, ok := manifest.(*schema1.SignedManifest)
+	if !ok {
+		return "", fmt.Errorf("non-schema1 manifest put to signedManifestHandler: %T", manifest)
+	}
+
+	if err := ms.verifyManifest(ms.ctx, *sm, skipDependencyVerification); err != nil {
+		return "", err
+	}
+
+	mt := schema1.MediaTypeManifest
+	payload := sm.Canonical
+
+	revision, err := ms.blobStore.Put(ctx, mt, payload)
+	if err != nil {
+		context.GetLogger(ctx).Errorf("error putting payload into blobstore: %v", err)
+		return "", err
+	}
+
+	// Link the revision into the repository.
+	if err := ms.blobStore.linkBlob(ctx, revision); err != nil {
+		return "", err
+	}
+
+	// Grab each json signature and store them.
+	signatures, err := sm.Signatures()
+	if err != nil {
+		return "", err
+	}
+
+	if err := ms.signatures.Put(revision.Digest, signatures...); err != nil {
+		return "", err
+	}
+
+	return revision.Digest, nil
+}
+
+// verifyManifest ensures that the manifest content is valid from the
+// 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 consumems.
+func (ms *signedManifestHandler) verifyManifest(ctx context.Context, mnfst schema1.SignedManifest, skipDependencyVerification bool) error {
+	var errs distribution.ErrManifestVerification
+
+	if len(mnfst.Name) > reference.NameTotalLengthMax {
+		errs = append(errs,
+			distribution.ErrManifestNameInvalid{
+				Name:   mnfst.Name,
+				Reason: fmt.Errorf("manifest name must not be more than %v characters", reference.NameTotalLengthMax),
+			})
+	}
+
+	if !reference.NameRegexp.MatchString(mnfst.Name) {
+		errs = append(errs,
+			distribution.ErrManifestNameInvalid{
+				Name:   mnfst.Name,
+				Reason: fmt.Errorf("invalid manifest name format"),
+			})
+	}
+
+	if len(mnfst.History) != len(mnfst.FSLayers) {
+		errs = append(errs, fmt.Errorf("mismatched history and fslayer cardinality %d != %d",
+			len(mnfst.History), len(mnfst.FSLayers)))
+	}
+
+	if _, err := schema1.Verify(&mnfst); err != nil {
+		switch err {
+		case libtrust.ErrMissingSignatureKey, libtrust.ErrInvalidJSONContent, libtrust.ErrMissingSignatureKey:
+			errs = append(errs, distribution.ErrManifestUnverified{})
+		default:
+			if err.Error() == "invalid signature" { // TODO(stevvooe): This should be exported by libtrust
+				errs = append(errs, distribution.ErrManifestUnverified{})
+			} else {
+				errs = append(errs, err)
+			}
+		}
+	}
+
+	if !skipDependencyVerification {
+		for _, fsLayer := range mnfst.References() {
+			_, err := ms.repository.Blobs(ctx).Stat(ctx, fsLayer.Digest)
+			if err != nil {
+				if err != distribution.ErrBlobUnknown {
+					errs = append(errs, err)
+				}
+
+				// On error here, we always append unknown blob errors.
+				errs = append(errs, distribution.ErrManifestBlobUnknown{Digest: fsLayer.Digest})
+			}
+		}
+	}
+	if len(errs) != 0 {
+		return errs
+	}
+
+	return nil
+}