Merge pull request #1836 from hinshun/catalog-walk-return-error

Fix storage drivers dropping non EOF errors when listing repositories

registry/handlers/catalog.go

@@ -9,6 +9,8 @@ import (
 	"strconv"
 
 	"github.com/docker/distribution/registry/api/errcode"
+	"github.com/docker/distribution/registry/storage"
+	"github.com/docker/distribution/registry/storage/driver"
 	"github.com/gorilla/handlers"
 )
 
@@ -45,9 +47,11 @@ func (ch *catalogHandler) GetCatalog(w http.ResponseWriter, r *http.Request) {
 	repos := make([]string, maxEntries)
 
 	filled, err := ch.App.registry.Repositories(ch.Context, repos, lastEntry)
-	if err == io.EOF {
+	_, pathNotFound := err.(driver.PathNotFoundError)
+
+	if err == io.EOF || pathNotFound {
 		moreEntries = false
-	} else if err != nil {
+	} else if err != nil && err != storage.ErrFinishedWalk {
 		ch.Errors = append(ch.Errors, errcode.ErrorCodeUnknown.WithDetail(err))
 		return
 	}

registry/storage/catalog.go

@@ -18,7 +18,7 @@ var ErrFinishedWalk = errors.New("finished walk")
 // Returns a list, or partial list, of repositories in the registry.
 // Because it's a quite expensive operation, it should only be used when building up
 // an initial set of repositories.
-func (reg *registry) Repositories(ctx context.Context, repos []string, last string) (n int, errVal error) {
+func (reg *registry) Repositories(ctx context.Context, repos []string, last string) (n int, err error) {
 	var foundRepos []string
 
 	if len(repos) == 0 {
@@ -58,11 +58,11 @@ func (reg *registry) Repositories(ctx context.Context, repos []string, last stri
 	n = copy(repos, foundRepos)
 
 	// Signal that we have no more entries by setting EOF
-	if len(foundRepos) <= len(repos) && err != ErrFinishedWalk {
-		errVal = io.EOF
+	if len(foundRepos) <= len(repos) && (err == nil || err == ErrSkipDir) {
+		err = io.EOF
 	}
 
-	return n, errVal
+	return n, err
 }
 
 // Enumerate applies ingester to each repository

registry/storage/catalog_test.go

@@ -1,6 +1,7 @@
 package storage
 
 import (
+	"fmt"
 	"io"
 	"testing"
 
@@ -161,3 +162,42 @@ func testEq(a, b []string, size int) bool {
 	}
 	return true
 }
+
+func setupBadWalkEnv(t *testing.T) *setupEnv {
+	d := newBadListDriver()
+	ctx := context.Background()
+	registry, err := NewRegistry(ctx, d, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), EnableRedirect)
+	if err != nil {
+		t.Fatalf("error creating registry: %v", err)
+	}
+
+	return &setupEnv{
+		ctx:      ctx,
+		driver:   d,
+		registry: registry,
+	}
+}
+
+type badListDriver struct {
+	driver.StorageDriver
+}
+
+var _ driver.StorageDriver = &badListDriver{}
+
+func newBadListDriver() *badListDriver {
+	return &badListDriver{StorageDriver: inmemory.New()}
+}
+
+func (d *badListDriver) List(ctx context.Context, path string) ([]string, error) {
+	return nil, fmt.Errorf("List error")
+}
+
+func TestCatalogWalkError(t *testing.T) {
+	env := setupBadWalkEnv(t)
+	p := make([]string, 1)
+
+	_, err := env.registry.Repositories(env.ctx, p, "")
+	if err == io.EOF {
+		t.Errorf("Expected catalog driver list error")
+	}
+}