Add error return to Repository method on Registry

The method (Registry).Repository may now return an error. This is too allow
certain implementationt to validate the name or opt to not return a repository
under certain conditions.

In conjunction with this change, error declarations have been moved into a
single file in the distribution package. Several error declarations that had
remained in the storage package have been moved into distribution, as well. The
declarations for Layer and LayerUpload have also been moved into the main
registry file, as a result.

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

docs/api/v2/names.go

@@ -6,6 +6,10 @@ import (
 	"strings"
 )
 
+// TODO(stevvooe): Move these definitions back to an exported package. While
+// they are used with v2 definitions, their relevance expands beyond.
+// "distribution/names" is a candidate package.
+
 const (
 	// RepositoryNameComponentMinLength is the minimum number of characters in a
 	// single repository name slash-delimited component
@@ -37,10 +41,6 @@ var RepositoryNameComponentRegexp = regexp.MustCompile(`[a-z0-9]+(?:[._-][a-z0-9
 // RepositoryNameComponentRegexp which must completely match the content
 var RepositoryNameComponentAnchoredRegexp = regexp.MustCompile(`^` + RepositoryNameComponentRegexp.String() + `$`)
 
-// TODO(stevvooe): RepositoryName needs to be limited to some fixed length.
-// Looking path prefixes and s3 limitation of 1024, this should likely be
-// around 512 bytes. 256 bytes might be more manageable.
-
 // RepositoryNameRegexp builds on RepositoryNameComponentRegexp to allow 1 to
 // 5 path components, separated by a forward slash.
 var RepositoryNameRegexp = regexp.MustCompile(`(?:` + RepositoryNameComponentRegexp.String() + `/){0,4}` + RepositoryNameComponentRegexp.String())

docs/handlers/app.go

@@ -227,10 +227,30 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler {
 			return
 		}
 
-		// decorate the authorized repository with an event bridge.
+		if app.nameRequired(r) {
-		context.Repository = notifications.Listen(
+			repository, err := app.registry.Repository(context, getName(context))
-			app.registry.Repository(context, getName(context)),
+
-			app.eventBridge(context, r))
+			if err != nil {
+				ctxu.GetLogger(context).Errorf("error resolving repository: %v", err)
+
+				switch err := err.(type) {
+				case distribution.ErrRepositoryUnknown:
+					context.Errors.Push(v2.ErrorCodeNameUnknown, err)
+				case distribution.ErrRepositoryNameInvalid:
+					context.Errors.Push(v2.ErrorCodeNameInvalid, err)
+				}
+
+				w.WriteHeader(http.StatusBadRequest)
+				serveJSON(w, context.Errors)
+				return
+			}
+
+			// assign and decorate the authorized repository with an event bridge.
+			context.Repository = notifications.Listen(
+				repository,
+				app.eventBridge(context, r))
+		}
+
 		handler := dispatch(context, r)
 
 		ssrw := &singleStatusResponseWriter{ResponseWriter: w}
@@ -318,9 +338,7 @@ func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Cont
 		}
 	} else {
 		// Only allow the name not to be set on the base route.
-		route := mux.CurrentRoute(r)
+		if app.nameRequired(r) {
-
-		if route == nil || route.GetName() != v2.RouteNameBase {
 			// For this to be properly secured, repo must always be set for a
 			// resource that may make a modification. The only condition under
 			// which name is not set and we still allow access is when the
@@ -378,6 +396,12 @@ func (app *App) eventBridge(ctx *Context, r *http.Request) notifications.Listene
 	return notifications.NewBridge(ctx.urlBuilder, app.events.source, actor, request, app.events.sink)
 }
 
+// nameRequired returns true if the route requires a name.
+func (app *App) nameRequired(r *http.Request) bool {
+	route := mux.CurrentRoute(r)
+	return route == nil || route.GetName() != v2.RouteNameBase
+}
+
 // apiBase implements a simple yes-man for doing overall checks against the
 // api. This can support auth roundtrips to support docker login.
 func apiBase(w http.ResponseWriter, r *http.Request) {

docs/handlers/images.go

@@ -10,7 +10,6 @@ import (
 	"github.com/docker/distribution/digest"
 	"github.com/docker/distribution/manifest"
 	"github.com/docker/distribution/registry/api/v2"
-	"github.com/docker/distribution/registry/storage"
 	"github.com/gorilla/handlers"
 )
 
@@ -70,12 +69,12 @@ func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http
 		// TODO(stevvooe): These error handling switches really need to be
 		// handled by an app global mapper.
 		switch err := err.(type) {
-		case storage.ErrManifestVerification:
+		case distribution.ErrManifestVerification:
 			for _, verificationError := range err {
 				switch verificationError := verificationError.(type) {
 				case distribution.ErrUnknownLayer:
 					imh.Errors.Push(v2.ErrorCodeBlobUnknown, verificationError.FSLayer)
-				case storage.ErrManifestUnverified:
+				case distribution.ErrManifestUnverified:
 					imh.Errors.Push(v2.ErrorCodeManifestUnverified)
 				default:
 					if verificationError == digest.ErrDigestInvalidFormat {
@@ -104,7 +103,7 @@ func (imh *imageManifestHandler) DeleteImageManifest(w http.ResponseWriter, r *h
 	manifests := imh.Repository.Manifests()
 	if err := manifests.Delete(imh.Tag); err != nil {
 		switch err := err.(type) {
-		case storage.ErrUnknownManifest:
+		case distribution.ErrManifestUnknown:
 			imh.Errors.Push(v2.ErrorCodeManifestUnknown, err)
 			w.WriteHeader(http.StatusNotFound)
 		default:

docs/handlers/tags.go

@@ -4,8 +4,8 @@ import (
 	"encoding/json"
 	"net/http"
 
+	"github.com/docker/distribution"
 	"github.com/docker/distribution/registry/api/v2"
-	"github.com/docker/distribution/registry/storage"
 	"github.com/gorilla/handlers"
 )
 
@@ -38,7 +38,7 @@ func (th *tagsHandler) GetTags(w http.ResponseWriter, r *http.Request) {
 	tags, err := manifests.Tags()
 	if err != nil {
 		switch err := err.(type) {
-		case storage.ErrUnknownRepository:
+		case distribution.ErrRepositoryUnknown:
 			w.WriteHeader(404)
 			th.Errors.Push(v2.ErrorCodeNameUnknown, map[string]string{"name": th.Repository.Name()})
 		default:

docs/storage/layer_test.go

@@ -36,7 +36,11 @@ func TestSimpleLayerUpload(t *testing.T) {
 	imageName := "foo/bar"
 	driver := inmemory.New()
 	registry := NewRegistryWithDriver(driver)
-	ls := registry.Repository(ctx, imageName).Layers()
+	repository, err := registry.Repository(ctx, imageName)
+	if err != nil {
+		t.Fatalf("unexpected error getting repo: %v", err)
+	}
+	ls := repository.Layers()
 
 	h := sha256.New()
 	rd := io.TeeReader(randomDataReader, h)
@@ -140,7 +144,11 @@ func TestSimpleLayerRead(t *testing.T) {
 	imageName := "foo/bar"
 	driver := inmemory.New()
 	registry := NewRegistryWithDriver(driver)
-	ls := registry.Repository(ctx, imageName).Layers()
+	repository, err := registry.Repository(ctx, imageName)
+	if err != nil {
+		t.Fatalf("unexpected error getting repo: %v", err)
+	}
+	ls := repository.Layers()
 
 	randomLayerReader, tarSumStr, err := testutil.CreateRandomTarFile()
 	if err != nil {
@@ -245,7 +253,11 @@ func TestLayerUploadZeroLength(t *testing.T) {
 	imageName := "foo/bar"
 	driver := inmemory.New()
 	registry := NewRegistryWithDriver(driver)
-	ls := registry.Repository(ctx, imageName).Layers()
+	repository, err := registry.Repository(ctx, imageName)
+	if err != nil {
+		t.Fatalf("unexpected error getting repo: %v", err)
+	}
+	ls := repository.Layers()
 
 	upload, err := ls.Upload()
 	if err != nil {

docs/storage/manifeststore.go

@@ -2,69 +2,13 @@ package storage
 
 import (
 	"fmt"
-	"strings"
 
 	"github.com/docker/distribution"
 	ctxu "github.com/docker/distribution/context"
-	"github.com/docker/distribution/digest"
 	"github.com/docker/distribution/manifest"
 	"github.com/docker/libtrust"
 )
 
-// ErrUnknownRepository is returned if the named repository is not known by
-// the registry.
-type ErrUnknownRepository struct {
-	Name string
-}
-
-func (err ErrUnknownRepository) Error() string {
-	return fmt.Sprintf("unknown respository name=%s", err.Name)
-}
-
-// ErrUnknownManifest is returned if the manifest is not known by the
-// registry.
-type ErrUnknownManifest struct {
-	Name string
-	Tag  string
-}
-
-func (err ErrUnknownManifest) Error() string {
-	return fmt.Sprintf("unknown manifest name=%s tag=%s", err.Name, err.Tag)
-}
-
-// ErrUnknownManifestRevision is returned when a manifest cannot be found by
-// revision within a repository.
-type ErrUnknownManifestRevision struct {
-	Name     string
-	Revision digest.Digest
-}
-
-func (err ErrUnknownManifestRevision) Error() string {
-	return fmt.Sprintf("unknown manifest name=%s revision=%s", err.Name, err.Revision)
-}
-
-// ErrManifestUnverified is returned when the registry is unable to verify
-// the manifest.
-type ErrManifestUnverified struct{}
-
-func (ErrManifestUnverified) Error() string {
-	return fmt.Sprintf("unverified manifest")
-}
-
-// ErrManifestVerification provides a type to collect errors encountered
-// during manifest verification. Currently, it accepts errors of all types,
-// but it may be narrowed to those involving manifest verification.
-type ErrManifestVerification []error
-
-func (errs ErrManifestVerification) Error() string {
-	var parts []string
-	for _, err := range errs {
-		parts = append(parts, err.Error())
-	}
-
-	return fmt.Sprintf("errors verifying manifest: %v", strings.Join(parts, ","))
-}
-
 type manifestStore struct {
 	repository *repository
 
@@ -147,7 +91,7 @@ func (ms *manifestStore) Delete(tag string) error {
 // 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 {
-	var errs ErrManifestVerification
+	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"))
@@ -161,10 +105,10 @@ func (ms *manifestStore) verifyManifest(tag string, mnfst *manifest.SignedManife
 	if _, err := manifest.Verify(mnfst); err != nil {
 		switch err {
 		case libtrust.ErrMissingSignatureKey, libtrust.ErrInvalidJSONContent, libtrust.ErrMissingSignatureKey:
-			errs = append(errs, ErrManifestUnverified{})
+			errs = append(errs, distribution.ErrManifestUnverified{})
 		default:
 			if err.Error() == "invalid signature" { // TODO(stevvooe): This should be exported by libtrust
-				errs = append(errs, ErrManifestUnverified{})
+				errs = append(errs, distribution.ErrManifestUnverified{})
 			} else {
 				errs = append(errs, err)
 			}

docs/storage/manifeststore_test.go

@@ -6,6 +6,7 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/docker/distribution"
 	"github.com/docker/distribution/digest"
 	"github.com/docker/distribution/manifest"
 	"github.com/docker/distribution/registry/storage/driver/inmemory"
@@ -20,7 +21,10 @@ func TestManifestStorage(t *testing.T) {
 	tag := "thetag"
 	driver := inmemory.New()
 	registry := NewRegistryWithDriver(driver)
-	repo := registry.Repository(ctx, name)
+	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)
@@ -34,7 +38,7 @@ func TestManifestStorage(t *testing.T) {
 
 	if _, err := ms.Get(tag); true {
 		switch err.(type) {
-		case ErrUnknownManifest:
+		case distribution.ErrManifestUnknown:
 			break
 		default:
 			t.Fatalf("expected manifest unknown error: %#v", err)

docs/storage/registry.go

@@ -2,6 +2,7 @@ package storage
 
 import (
 	"github.com/docker/distribution"
+	"github.com/docker/distribution/registry/api/v2"
 	storagedriver "github.com/docker/distribution/registry/storage/driver"
 	"golang.org/x/net/context"
 )
@@ -36,12 +37,19 @@ func NewRegistryWithDriver(driver storagedriver.StorageDriver) distribution.Regi
 // Repository returns an instance of the repository tied to the registry.
 // Instances should not be shared between goroutines but are cheap to
 // allocate. In general, they should be request scoped.
-func (reg *registry) Repository(ctx context.Context, name string) distribution.Repository {
+func (reg *registry) Repository(ctx context.Context, name string) (distribution.Repository, error) {
+	if err := v2.ValidateRespositoryName(name); err != nil {
+		return nil, distribution.ErrRepositoryNameInvalid{
+			Name:   name,
+			Reason: err,
+		}
+	}
+
 	return &repository{
 		ctx:      ctx,
 		registry: reg,
 		name:     name,
-	}
+	}, nil
 }
 
 // repository provides name-scoped access to various services.

docs/storage/revisionstore.go

@@ -5,6 +5,7 @@ import (
 	"path"
 
 	"github.com/Sirupsen/logrus"
+	"github.com/docker/distribution"
 	"github.com/docker/distribution/digest"
 	"github.com/docker/distribution/manifest"
 	"github.com/docker/libtrust"
@@ -40,7 +41,7 @@ func (rs *revisionStore) get(revision digest.Digest) (*manifest.SignedManifest,
 	if exists, err := rs.exists(revision); err != nil {
 		return nil, err
 	} else if !exists {
-		return nil, ErrUnknownManifestRevision{
+		return nil, distribution.ErrUnknownManifestRevision{
 			Name:     rs.Name(),
 			Revision: revision,
 		}

docs/storage/tagstore.go

@@ -3,6 +3,7 @@ package storage
 import (
 	"path"
 
+	"github.com/docker/distribution"
 	"github.com/docker/distribution/digest"
 	storagedriver "github.com/docker/distribution/registry/storage/driver"
 )
@@ -26,7 +27,7 @@ func (ts *tagStore) tags() ([]string, error) {
 	if err != nil {
 		switch err := err.(type) {
 		case storagedriver.PathNotFoundError:
-			return nil, ErrUnknownRepository{Name: ts.name}
+			return nil, distribution.ErrRepositoryUnknown{Name: ts.name}
 		default:
 			return nil, err
 		}
@@ -104,7 +105,7 @@ func (ts *tagStore) resolve(tag string) (digest.Digest, error) {
 	if exists, err := exists(ts.driver, currentPath); err != nil {
 		return "", err
 	} else if !exists {
-		return "", ErrUnknownManifest{Name: ts.Name(), Tag: tag}
+		return "", distribution.ErrManifestUnknown{Name: ts.Name(), Tag: tag}
 	}
 
 	revision, err := ts.blobStore.readlink(currentPath)