diff --git a/registry.go b/registry.go index 2a6637a3..1a3de01d 100644 --- a/registry.go +++ b/registry.go @@ -36,8 +36,11 @@ type Namespace interface { // reference. Repository(ctx context.Context, name string) (Repository, error) - // Catalog returns a reference which can be used for listing repositories - Catalog(ctx context.Context) CatalogService + // Repositories fills 'repos' with a lexigraphically sorted catalog of repositories + // up to the size of 'repos' and returns the value 'n' for the number of entries + // which were filled. 'last' contains an offset in the catalog, and 'err' will be + // set to io.EOF if there are no more entries to obtain. + Repositories(ctx context.Context, repos []string, last string) (n int, err error) } // ManifestServiceOption is a function argument for Manifest Service methods @@ -115,9 +118,3 @@ type SignatureService interface { // Put stores the signature for the provided digest. Put(dgst digest.Digest, signatures ...[]byte) error } - -// CatalogService provides a way of retrieving the names of each of the repositories -type CatalogService interface { - // Get retrieves repository names from the registry. - Get(n int, q string) (p []string, moreEntries bool, err error) -} diff --git a/registry/client/repository.go b/registry/client/repository.go index 6d2fd6e7..6979cc4d 100644 --- a/registry/client/repository.go +++ b/registry/client/repository.go @@ -445,34 +445,7 @@ func (bs *blobStatter) Stat(ctx context.Context, dgst digest.Digest) (distributi } } -// NewCatalog can be used to get a list of repositories -func NewCatalog(ctx context.Context, baseURL string, transport http.RoundTripper) (distribution.CatalogService, error) { - ub, err := v2.NewURLBuilderFromString(baseURL) - if err != nil { - return nil, err - } - - client := &http.Client{ - Transport: transport, - Timeout: 1 * time.Minute, - } - - return &catalog{ - client: client, - ub: ub, - context: ctx, - }, nil -} - -type catalog struct { - client *http.Client - ub *v2.URLBuilder - context context.Context -} - -func (c *catalog) Get(maxEntries int, last string) ([]string, bool, error) { - var repos []string - +func buildCatalogValues(maxEntries int, last string) url.Values { values := url.Values{} if maxEntries > 0 { @@ -483,14 +456,35 @@ func (c *catalog) Get(maxEntries int, last string) ([]string, bool, error) { values.Add("last", last) } - u, err := c.ub.BuildCatalogURL(values) + return values +} + +// Repositories returns a lexigraphically sorted catalog given a base URL. The 'entries' slice will be filled up to the size +// of the slice, starting at the value provided in 'last'. The number of entries will be returned along with io.EOF if there +// are no more entries +func Repositories(ctx context.Context, baseURL string, entries []string, last string, transport http.RoundTripper) (int, error) { + var numFilled int + var returnErr error + + ub, err := v2.NewURLBuilderFromString(baseURL) if err != nil { - return nil, false, err + return 0, err } - resp, err := c.client.Get(u) + client := &http.Client{ + Transport: transport, + Timeout: 1 * time.Minute, + } + + values := buildCatalogValues(len(entries), last) + u, err := ub.BuildCatalogURL(values) if err != nil { - return nil, false, err + return 0, err + } + + resp, err := client.Get(u) + if err != nil { + return 0, err } defer resp.Body.Close() @@ -502,13 +496,22 @@ func (c *catalog) Get(maxEntries int, last string) ([]string, bool, error) { decoder := json.NewDecoder(resp.Body) if err := decoder.Decode(&ctlg); err != nil { - return nil, false, err + return 0, err + } + + for cnt := range ctlg.Repositories { + entries[cnt] = ctlg.Repositories[cnt] + } + numFilled = len(ctlg.Repositories) + + link := resp.Header.Get("Link") + if link == "" { + returnErr = io.EOF } - repos = ctlg.Repositories default: - return nil, false, handleErrorResponse(resp) + return 0, handleErrorResponse(resp) } - return repos, false, nil + return numFilled, returnErr } diff --git a/registry/client/repository_test.go b/registry/client/repository_test.go index e9735cd4..b803d754 100644 --- a/registry/client/repository_test.go +++ b/registry/client/repository_test.go @@ -5,6 +5,7 @@ import ( "crypto/rand" "encoding/json" "fmt" + "io" "log" "net/http" "net/http/httptest" @@ -78,19 +79,24 @@ func addTestFetch(repo string, dgst digest.Digest, content []byte, m *testutil.R }) } -func addTestCatalog(content []byte, m *testutil.RequestResponseMap) { +func addTestCatalog(route string, content []byte, link string, m *testutil.RequestResponseMap) { + headers := map[string][]string{ + "Content-Length": {strconv.Itoa(len(content))}, + "Content-Type": {"application/json; charset=utf-8"}, + } + if link != "" { + headers["Link"] = append(headers["Link"], link) + } + *m = append(*m, testutil.RequestResponseMapping{ Request: testutil.Request{ Method: "GET", - Route: "/v2/_catalog", + Route: route, }, Response: testutil.Response{ StatusCode: http.StatusOK, Body: content, - Headers: http.Header(map[string][]string{ - "Content-Length": {strconv.Itoa(len(content))}, - "Content-Type": {"application/json; charset=utf-8"}, - }), + Headers: http.Header(headers), }, }) } @@ -753,23 +759,58 @@ func TestManifestUnauthorized(t *testing.T) { func TestCatalog(t *testing.T) { var m testutil.RequestResponseMap - addTestCatalog([]byte("{\"repositories\":[\"foo\", \"bar\", \"baz\"]}"), &m) + addTestCatalog( + "/v2/_catalog?n=5", + []byte("{\"repositories\":[\"foo\", \"bar\", \"baz\"]}"), "", &m) e, c := testServer(m) defer c() + entries := make([]string, 5) + ctx := context.Background() - ctlg, err := NewCatalog(ctx, e, nil) - if err != nil { + numFilled, err := Repositories(ctx, e, entries, "", nil) + if err != io.EOF { t.Fatal(err) } - repos, _, err := ctlg.Get(0, "") - if err != nil { - t.Fatal(err) - } - - if len(repos) != 3 { + if numFilled != 3 { + t.Fatalf("Got wrong number of repos") + } +} + +func TestCatalogInParts(t *testing.T) { + var m testutil.RequestResponseMap + addTestCatalog( + "/v2/_catalog?n=2", + []byte("{\"repositories\":[\"bar\", \"baz\"]}"), + "", &m) + addTestCatalog( + "/v2/_catalog?last=baz&n=2", + []byte("{\"repositories\":[\"foo\"]}"), + "", &m) + + e, c := testServer(m) + defer c() + + entries := make([]string, 2) + + ctx := context.Background() + numFilled, err := Repositories(ctx, e, entries, "", nil) + if err != nil { + t.Fatal(err) + } + + if numFilled != 2 { + t.Fatalf("Got wrong number of repos") + } + + numFilled, err = Repositories(ctx, e, entries, "baz", nil) + if err != io.EOF { + t.Fatal(err) + } + + if numFilled != 1 { t.Fatalf("Got wrong number of repos") } } diff --git a/registry/handlers/api_test.go b/registry/handlers/api_test.go index d768a116..4473eb99 100644 --- a/registry/handlers/api_test.go +++ b/registry/handlers/api_test.go @@ -13,6 +13,8 @@ import ( "os" "path" "reflect" + "regexp" + "strconv" "strings" "testing" @@ -60,10 +62,14 @@ func TestCheckAPI(t *testing.T) { } } +// TestCatalogAPI tests the /v2/_catalog endpoint func TestCatalogAPI(t *testing.T) { + chunkLen := 2 env := newTestEnv(t) - values := url.Values{"last": []string{""}, "n": []string{"100"}} + values := url.Values{ + "last": []string{""}, + "n": []string{strconv.Itoa(chunkLen)}} catalogURL, err := env.builder.BuildCatalogURL(values) if err != nil { @@ -90,7 +96,7 @@ func TestCatalogAPI(t *testing.T) { } // we haven't pushed anything to the registry yet - if ctlg.Repositories != nil { + if len(ctlg.Repositories) != 0 { t.Fatalf("repositories has unexpected values") } @@ -100,8 +106,49 @@ func TestCatalogAPI(t *testing.T) { // ----------------------------------- // push something to the registry and try again - imageName := "foo/bar" - createRepository(env, t, imageName, "sometag") + images := []string{"foo/aaaa", "foo/bbbb", "foo/cccc"} + + for _, image := range images { + createRepository(env, t, image, "sometag") + } + + resp, err = http.Get(catalogURL) + if err != nil { + t.Fatalf("unexpected error issuing request: %v", err) + } + defer resp.Body.Close() + + checkResponse(t, "issuing catalog api check", resp, http.StatusOK) + + dec = json.NewDecoder(resp.Body) + if err = dec.Decode(&ctlg); err != nil { + t.Fatalf("error decoding fetched manifest: %v", err) + } + + if len(ctlg.Repositories) != chunkLen { + t.Fatalf("repositories has unexpected values") + } + + for _, image := range images[:chunkLen] { + if !contains(ctlg.Repositories, image) { + t.Fatalf("didn't find our repository '%s' in the catalog", image) + } + } + + link := resp.Header.Get("Link") + if link == "" { + t.Fatalf("repositories has less data than expected") + } + + newValues := checkLink(t, link, chunkLen, ctlg.Repositories[len(ctlg.Repositories)-1]) + + // ----------------------------------- + // get the last chunk of data + + catalogURL, err = env.builder.BuildCatalogURL(newValues) + if err != nil { + t.Fatalf("unexpected error building catalog url: %v", err) + } resp, err = http.Get(catalogURL) if err != nil { @@ -120,14 +167,36 @@ func TestCatalogAPI(t *testing.T) { t.Fatalf("repositories has unexpected values") } - if !contains(ctlg.Repositories, imageName) { - t.Fatalf("didn't find our repository '%s' in the catalog", imageName) + lastImage := images[len(images)-1] + if !contains(ctlg.Repositories, lastImage) { + t.Fatalf("didn't find our repository '%s' in the catalog", lastImage) } - if resp.Header.Get("Link") != "" { - t.Fatalf("repositories has more data when none expected") + link = resp.Header.Get("Link") + if link != "" { + t.Fatalf("catalog has unexpected data") + } +} + +func checkLink(t *testing.T, urlStr string, numEntries int, last string) url.Values { + re := regexp.MustCompile("<(/v2/_catalog.*)>; rel=\"next\"") + matches := re.FindStringSubmatch(urlStr) + + if len(matches) != 2 { + t.Fatalf("Catalog link address response was incorrect") + } + linkURL, _ := url.Parse(matches[1]) + urlValues := linkURL.Query() + + if urlValues.Get("n") != strconv.Itoa(numEntries) { + t.Fatalf("Catalog link entry size is incorrect") } + if urlValues.Get("last") != last { + t.Fatal("Catalog link last entry is incorrect") + } + + return urlValues } func contains(elems []string, e string) bool { diff --git a/registry/handlers/app.go b/registry/handlers/app.go index 45f97966..f61b2c1e 100644 --- a/registry/handlers/app.go +++ b/registry/handlers/app.go @@ -367,9 +367,6 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler { // Add username to request logging context.Context = ctxu.WithLogger(context.Context, ctxu.GetLogger(context.Context, "auth.user.name")) - catalog := app.registry.Catalog(context) - context.Catalog = catalog - if app.nameRequired(r) { repository, err := app.registry.Repository(context, getName(context)) diff --git a/registry/handlers/catalog.go b/registry/handlers/catalog.go index fd2af76e..6ec1fe55 100644 --- a/registry/handlers/catalog.go +++ b/registry/handlers/catalog.go @@ -3,6 +3,7 @@ package handlers import ( "encoding/json" "fmt" + "io" "net/http" "net/url" "strconv" @@ -32,6 +33,8 @@ type catalogAPIResponse struct { } func (ch *catalogHandler) GetCatalog(w http.ResponseWriter, r *http.Request) { + var moreEntries = true + q := r.URL.Query() lastEntry := q.Get("last") maxEntries, err := strconv.Atoi(q.Get("n")) @@ -39,8 +42,12 @@ func (ch *catalogHandler) GetCatalog(w http.ResponseWriter, r *http.Request) { maxEntries = maximumReturnedEntries } - repos, moreEntries, err := ch.Catalog.Get(maxEntries, lastEntry) - if err != nil { + repos := make([]string, maxEntries) + + filled, err := ch.App.registry.Repositories(ch.Context, repos, lastEntry) + if err == io.EOF { + moreEntries = false + } else if err != nil { ch.Errors = append(ch.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) return } @@ -49,7 +56,8 @@ func (ch *catalogHandler) GetCatalog(w http.ResponseWriter, r *http.Request) { // Add a link header if there are more entries to retrieve if moreEntries { - urlStr, err := createLinkEntry(r.URL.String(), maxEntries, repos) + lastEntry = repos[len(repos)-1] + urlStr, err := createLinkEntry(r.URL.String(), maxEntries, lastEntry) if err != nil { ch.Errors = append(ch.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) return @@ -59,7 +67,7 @@ func (ch *catalogHandler) GetCatalog(w http.ResponseWriter, r *http.Request) { enc := json.NewEncoder(w) if err := enc.Encode(catalogAPIResponse{ - Repositories: repos, + Repositories: repos[0:filled], }); err != nil { ch.Errors = append(ch.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) return @@ -68,13 +76,18 @@ func (ch *catalogHandler) GetCatalog(w http.ResponseWriter, r *http.Request) { // Use the original URL from the request to create a new URL for // the link header -func createLinkEntry(origURL string, maxEntries int, repos []string) (string, error) { +func createLinkEntry(origURL string, maxEntries int, lastEntry string) (string, error) { calledURL, err := url.Parse(origURL) if err != nil { return "", err } - calledURL.RawQuery = fmt.Sprintf("n=%d&last=%s", maxEntries, repos[len(repos)-1]) + v := url.Values{} + v.Add("n", strconv.Itoa(maxEntries)) + v.Add("last", lastEntry) + + calledURL.RawQuery = v.Encode() + calledURL.Fragment = "" urlStr := fmt.Sprintf("<%s>; rel=\"next\"", calledURL.String()) diff --git a/registry/handlers/context.go b/registry/handlers/context.go index 6625551d..85a17123 100644 --- a/registry/handlers/context.go +++ b/registry/handlers/context.go @@ -32,9 +32,6 @@ type Context struct { urlBuilder *v2.URLBuilder - // Catalog allows getting a complete list of the contents of the registry. - Catalog distribution.CatalogService - // TODO(stevvooe): The goal is too completely factor this context and // dispatching out of the web application. Ideally, we should lean on // context.Context for injection of these resources. diff --git a/registry/storage/catalog.go b/registry/storage/catalog.go index ce184dba..470894b7 100644 --- a/registry/storage/catalog.go +++ b/registry/storage/catalog.go @@ -1,36 +1,38 @@ package storage import ( + "errors" + "io" "path" "sort" "strings" - log "github.com/Sirupsen/logrus" - "github.com/docker/distribution" "github.com/docker/distribution/context" - storageDriver "github.com/docker/distribution/registry/storage/driver" + "github.com/docker/distribution/registry/storage/driver" ) -type catalogSvc struct { - ctx context.Context - driver storageDriver.StorageDriver -} - -var _ distribution.CatalogService = &catalogSvc{} - -// Get returns a list, or partial list, of repositories in the registry. +// 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 (c *catalogSvc) Get(maxEntries int, lastEntry string) ([]string, bool, error) { - log.Infof("Retrieving up to %d entries of the catalog starting with '%s'", maxEntries, lastEntry) - var repos []string +func (reg *registry) Repositories(ctx context.Context, repos []string, last string) (n int, err error) { + var foundRepos []string + var errVal error + + if len(repos) == 0 { + return 0, errors.New("no space in slice") + } root, err := defaultPathMapper.path(repositoriesRootPathSpec{}) if err != nil { - return repos, false, err + return 0, err } - Walk(c.ctx, c.driver, root, func(fileInfo storageDriver.FileInfo) error { + // Walk each of the directories in our storage. Unfortunately since there's no + // guarantee that storage will return files in lexigraphical order, we have + // to store everything another slice, sort it and then copy it back to our + // passed in slice. + + Walk(ctx, reg.blobStore.driver, root, func(fileInfo driver.FileInfo) error { filePath := fileInfo.Path() // lop the base path off @@ -39,8 +41,8 @@ func (c *catalogSvc) Get(maxEntries int, lastEntry string) ([]string, bool, erro _, file := path.Split(repoPath) if file == "_layers" { repoPath = strings.TrimSuffix(repoPath, "/_layers") - if repoPath > lastEntry { - repos = append(repos, repoPath) + if repoPath > last { + foundRepos = append(foundRepos, repoPath) } return ErrSkipDir } else if strings.HasPrefix(file, "_") { @@ -50,13 +52,14 @@ func (c *catalogSvc) Get(maxEntries int, lastEntry string) ([]string, bool, erro return nil }) - sort.Strings(repos) + sort.Strings(foundRepos) + n = copy(repos, foundRepos) - moreEntries := false - if len(repos) > maxEntries { - moreEntries = true - repos = repos[0:maxEntries] + // Signal that we have no more entries by setting EOF + if len(foundRepos) <= len(repos) { + errVal = io.EOF } - return repos, moreEntries, nil + return n, errVal + } diff --git a/registry/storage/catalog_test.go b/registry/storage/catalog_test.go index 8d9f3854..a9a046a7 100644 --- a/registry/storage/catalog_test.go +++ b/registry/storage/catalog_test.go @@ -1,6 +1,7 @@ package storage import ( + "io" "testing" "github.com/docker/distribution" @@ -15,7 +16,6 @@ type setupEnv struct { driver driver.StorageDriver expected []string registry distribution.Namespace - catalog distribution.CatalogService } func setupFS(t *testing.T) *setupEnv { @@ -41,8 +41,6 @@ func setupFS(t *testing.T) *setupEnv { } } - catalog := registry.Catalog(ctx) - expected := []string{ "bar/c", "bar/d", @@ -56,20 +54,21 @@ func setupFS(t *testing.T) *setupEnv { driver: d, expected: expected, registry: registry, - catalog: catalog, } } func TestCatalog(t *testing.T) { env := setupFS(t) - repos, more, _ := env.catalog.Get(100, "") + p := make([]string, 50) - if !testEq(repos, env.expected) { + numFilled, err := env.registry.Repositories(env.ctx, p, "") + + if !testEq(p, env.expected, numFilled) { t.Errorf("Expected catalog repos err") } - if more { + if err != io.EOF { t.Errorf("Catalog has more values which we aren't expecting") } } @@ -78,50 +77,46 @@ func TestCatalogInParts(t *testing.T) { env := setupFS(t) chunkLen := 2 + p := make([]string, chunkLen) - repos, more, _ := env.catalog.Get(chunkLen, "") - if !testEq(repos, env.expected[0:chunkLen]) { + numFilled, err := env.registry.Repositories(env.ctx, p, "") + if err == io.EOF || numFilled != len(p) { + t.Errorf("Expected more values in catalog") + } + + if !testEq(p, env.expected[0:chunkLen], numFilled) { t.Errorf("Expected catalog first chunk err") } - if !more { + lastRepo := p[len(p)-1] + numFilled, err = env.registry.Repositories(env.ctx, p, lastRepo) + + if err == io.EOF || numFilled != len(p) { t.Errorf("Expected more values in catalog") } - lastRepo := repos[len(repos)-1] - repos, more, _ = env.catalog.Get(chunkLen, lastRepo) - - if !testEq(repos, env.expected[chunkLen:chunkLen*2]) { + if !testEq(p, env.expected[chunkLen:chunkLen*2], numFilled) { t.Errorf("Expected catalog second chunk err") } - if !more { - t.Errorf("Expected more values in catalog") - } + lastRepo = p[len(p)-1] + numFilled, err = env.registry.Repositories(env.ctx, p, lastRepo) - lastRepo = repos[len(repos)-1] - repos, more, _ = env.catalog.Get(chunkLen, lastRepo) - - if !testEq(repos, env.expected[chunkLen*2:chunkLen*3-1]) { - t.Errorf("Expected catalog third chunk err") - } - - if more { + if err != io.EOF { t.Errorf("Catalog has more values which we aren't expecting") } -} - -func testEq(a, b []string) bool { - if len(a) != len(b) { - return false + if !testEq(p, env.expected[chunkLen*2:chunkLen*3-1], numFilled) { + t.Errorf("Expected catalog third chunk err") } - for count := range a { - if a[count] != b[count] { +} + +func testEq(a, b []string, size int) bool { + for cnt := 0; cnt < size-1; cnt++ { + if a[cnt] != b[cnt] { return false } } - return true } diff --git a/registry/storage/registry.go b/registry/storage/registry.go index 17035555..cf0fe3e7 100644 --- a/registry/storage/registry.go +++ b/registry/storage/registry.go @@ -55,15 +55,6 @@ func (reg *registry) Scope() distribution.Scope { return distribution.GlobalScope } -// Catalog returns an instance of the catalog service which can be -// used to dump all of the repositories in a registry -func (reg *registry) Catalog(ctx context.Context) distribution.CatalogService { - return &catalogSvc{ - ctx: ctx, - driver: reg.blobStore.driver, - } -} - // 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.