diff --git a/registry/handlers/catalog.go b/registry/handlers/catalog.go index 6ec1fe55..eca98468 100644 --- a/registry/handlers/catalog.go +++ b/registry/handlers/catalog.go @@ -9,6 +9,7 @@ import ( "strconv" "github.com/docker/distribution/registry/api/errcode" + "github.com/docker/distribution/registry/storage/driver" "github.com/gorilla/handlers" ) @@ -45,7 +46,9 @@ 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 { ch.Errors = append(ch.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) diff --git a/registry/storage/catalog.go b/registry/storage/catalog.go index 3b13b7ad..eb669b77 100644 --- a/registry/storage/catalog.go +++ b/registry/storage/catalog.go @@ -10,15 +10,14 @@ import ( "github.com/docker/distribution/registry/storage/driver" ) -// ErrFinishedWalk is used when the called walk function no longer wants -// to accept any more values. This is used for pagination when the -// required number of repos have been found. -var ErrFinishedWalk = errors.New("finished walk") +// errFinishedWalk signals an early exit to the walk when the current query +// is satisfied. +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 { @@ -30,26 +29,22 @@ func (reg *registry) Repositories(ctx context.Context, repos []string, last stri return 0, err } + // errFinishedWalk signals an early exit to the walk when the current query + // is satisfied. + errFinishedWalk := errors.New("finished walk") + err = Walk(ctx, reg.blobStore.driver, root, func(fileInfo driver.FileInfo) error { - filePath := fileInfo.Path() - - // lop the base path off - repoPath := filePath[len(root)+1:] - - _, file := path.Split(repoPath) - if file == "_layers" { - repoPath = strings.TrimSuffix(repoPath, "/_layers") - if repoPath > last { - foundRepos = append(foundRepos, repoPath) - } - return ErrSkipDir - } else if strings.HasPrefix(file, "_") { - return ErrSkipDir + err := handleRepository(fileInfo, root, last, func(repoPath string) error { + foundRepos = append(foundRepos, repoPath) + return nil + }) + if err != nil { + return err } // if we've filled our array, no need to walk any further if len(foundRepos) == len(repos) { - return ErrFinishedWalk + return errFinishedWalk } return nil @@ -57,41 +52,102 @@ 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 + switch err { + case nil: + // nil means that we completed walk and didn't fill buffer. No more + // records are available. + err = io.EOF + case errFinishedWalk: + // more records are available. + err = nil } - return n, errVal + return n, err } // Enumerate applies ingester to each repository func (reg *registry) Enumerate(ctx context.Context, ingester func(string) error) error { - repoNameBuffer := make([]string, 100) - var last string - for { - n, err := reg.Repositories(ctx, repoNameBuffer, last) - if err != nil && err != io.EOF { - return err + root, err := pathFor(repositoriesRootPathSpec{}) + if err != nil { + return err + } + + err = Walk(ctx, reg.blobStore.driver, root, func(fileInfo driver.FileInfo) error { + return handleRepository(fileInfo, root, "", ingester) + }) + + return err +} + +// lessPath returns true if one path a is less than path b. +// +// A component-wise comparison is done, rather than the lexical comparison of +// strings. +func lessPath(a, b string) bool { + // we provide this behavior by making separator always sort first. + return compareReplaceInline(a, b, '/', '\x00') < 0 +} + +// compareReplaceInline modifies runtime.cmpstring to replace old with new +// during a byte-wise comparison. +func compareReplaceInline(s1, s2 string, old, new byte) int { + l := len(s1) + if len(s2) < l { + l = len(s2) + } + + for i := 0; i < l; i++ { + c1, c2 := s1[i], s2[i] + if c1 == old { + c1 = new } - if n == 0 { - break + if c2 == old { + c2 = new } - last = repoNameBuffer[n-1] - for i := 0; i < n; i++ { - repoName := repoNameBuffer[i] - err = ingester(repoName) - if err != nil { + if c1 < c2 { + return -1 + } + + if c1 > c2 { + return +1 + } + } + + if len(s1) < len(s2) { + return -1 + } + + if len(s1) > len(s2) { + return +1 + } + + return 0 +} + +// handleRepository calls function fn with a repository path if fileInfo +// has a path of a repository under root and that it is lexographically +// after last. Otherwise, it will return ErrSkipDir. This should be used +// with Walk to do handling with repositories in a storage. +func handleRepository(fileInfo driver.FileInfo, root, last string, fn func(repoPath string) error) error { + filePath := fileInfo.Path() + + // lop the base path off + repo := filePath[len(root)+1:] + + _, file := path.Split(repo) + if file == "_layers" { + repo = strings.TrimSuffix(repo, "/_layers") + if lessPath(last, repo) { + if err := fn(repo); err != nil { return err } } - - if err == io.EOF { - break - } + return ErrSkipDir + } else if strings.HasPrefix(file, "_") { + return ErrSkipDir } - return nil + return nil } diff --git a/registry/storage/catalog_test.go b/registry/storage/catalog_test.go index eb062c5b..7fe133f6 100644 --- a/registry/storage/catalog_test.go +++ b/registry/storage/catalog_test.go @@ -1,6 +1,7 @@ package storage import ( + "fmt" "io" "testing" @@ -112,7 +113,28 @@ func TestCatalogInParts(t *testing.T) { if !testEq(p, env.expected[chunkLen*2:chunkLen*3-1], numFilled) { t.Errorf("Expected catalog third chunk err") } +} +func TestCatalogEnumerate(t *testing.T) { + env := setupFS(t) + + var repos []string + repositoryEnumerator := env.registry.(distribution.RepositoryEnumerator) + err := repositoryEnumerator.Enumerate(env.ctx, func(repoName string) error { + repos = append(repos, repoName) + return nil + }) + if err != nil { + t.Errorf("Expected catalog enumerate err") + } + + if len(repos) != len(env.expected) { + t.Errorf("Expected catalog enumerate doesn't have correct number of values") + } + + if !testEq(repos, env.expected, len(env.expected)) { + t.Errorf("Expected catalog enumerate not over all values") + } } func testEq(a, b []string, size int) bool { @@ -123,3 +145,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") + } +}