Browse code
search: un-export registry.DefaultSearchLimit, and fix API status codes
Move the default to the service itself, and produce the correct status code
if an invalid limit was specified. The default is currently set both on the
cli and on the daemon side, and it should be only set on one of them.
There is a slight change in behavior; previously, searching with `--limit=0`
would produce an error, but with this change, it's considered the equivalent
of "no limit set" (and using the default).
We could keep the old behavior by passing a pointer (`nil` means "not set"),
but I left that for a follow-up exercise (we may want to pass an actual
config instead of separate arguments, as well as some other things that need
cleaning up).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sebastiaan van Stijn authored on 2022/03/02 21:29:47Showing 5 changed files
- api/server/router/image/image_routes.go
- daemon/images/image_search_test.go
- integration-cli/docker_cli_search_test.go
- registry/service.go
- registry/session.go
| ... | ... |
@@ -16,7 +16,6 @@ import ( |
| 16 | 16 |
"github.com/docker/docker/errdefs" |
| 17 | 17 |
"github.com/docker/docker/pkg/ioutils" |
| 18 | 18 |
"github.com/docker/docker/pkg/streamformatter" |
| 19 |
- "github.com/docker/docker/registry" |
|
| 20 | 19 |
specs "github.com/opencontainers/image-spec/specs-go/v1" |
| 21 | 20 |
"github.com/pkg/errors" |
| 22 | 21 |
) |
| ... | ... |
@@ -290,13 +289,14 @@ func (s *imageRouter) getImagesSearch(ctx context.Context, w http.ResponseWriter |
| 290 | 290 |
headers[k] = v |
| 291 | 291 |
} |
| 292 | 292 |
} |
| 293 |
- limit := registry.DefaultSearchLimit |
|
| 293 |
+ |
|
| 294 |
+ var limit int |
|
| 294 | 295 |
if r.Form.Get("limit") != "" {
|
| 295 |
- limitValue, err := strconv.Atoi(r.Form.Get("limit"))
|
|
| 296 |
- if err != nil {
|
|
| 297 |
- return err |
|
| 296 |
+ var err error |
|
| 297 |
+ limit, err = strconv.Atoi(r.Form.Get("limit"))
|
|
| 298 |
+ if err != nil || limit < 0 {
|
|
| 299 |
+ return errdefs.InvalidParameter(errors.Wrap(err, "invalid limit specified")) |
|
| 298 | 300 |
} |
| 299 |
- limit = limitValue |
|
| 300 | 301 |
} |
| 301 | 302 |
query, err := s.backend.SearchRegistryForImages(ctx, r.Form.Get("filters"), r.Form.Get("term"), limit, config, headers)
|
| 302 | 303 |
if err != nil {
|
| ... | ... |
@@ -79,7 +79,7 @@ func TestSearchRegistryForImagesErrors(t *testing.T) {
|
| 79 | 79 |
shouldReturnError: e.shouldReturnError, |
| 80 | 80 |
}, |
| 81 | 81 |
} |
| 82 |
- _, err := daemon.SearchRegistryForImages(context.Background(), e.filtersArgs, "term", 25, nil, map[string][]string{})
|
|
| 82 |
+ _, err := daemon.SearchRegistryForImages(context.Background(), e.filtersArgs, "term", 0, nil, map[string][]string{})
|
|
| 83 | 83 |
if err == nil {
|
| 84 | 84 |
t.Errorf("%d: expected an error, got nothing", index)
|
| 85 | 85 |
} |
| ... | ... |
@@ -326,7 +326,7 @@ func TestSearchRegistryForImages(t *testing.T) {
|
| 326 | 326 |
results: s.registryResults, |
| 327 | 327 |
}, |
| 328 | 328 |
} |
| 329 |
- results, err := daemon.SearchRegistryForImages(context.Background(), s.filtersArgs, term, 25, nil, map[string][]string{})
|
|
| 329 |
+ results, err := daemon.SearchRegistryForImages(context.Background(), s.filtersArgs, term, 0, nil, map[string][]string{})
|
|
| 330 | 330 |
if err != nil {
|
| 331 | 331 |
t.Errorf("%d: %v", index, err)
|
| 332 | 332 |
} |
| ... | ... |
@@ -73,7 +73,7 @@ func (s *DockerSuite) TestSearchWithLimit(c *testing.T) {
|
| 73 | 73 |
assert.Equal(c, len(outSlice), limit+2) // 1 header, 1 carriage return |
| 74 | 74 |
} |
| 75 | 75 |
|
| 76 |
- for _, limit := range []int{-1, 0, 101} {
|
|
| 76 |
+ for _, limit := range []int{-1, 101} {
|
|
| 77 | 77 |
_, _, err := dockerCmdWithError("search", fmt.Sprintf("--limit=%d", limit), "docker")
|
| 78 | 78 |
assert.ErrorContains(c, err, "") |
| 79 | 79 |
} |
| ... | ... |
@@ -16,11 +16,6 @@ import ( |
| 16 | 16 |
"github.com/sirupsen/logrus" |
| 17 | 17 |
) |
| 18 | 18 |
|
| 19 |
-const ( |
|
| 20 |
- // DefaultSearchLimit is the default value for maximum number of returned search results. |
|
| 21 |
- DefaultSearchLimit = 25 |
|
| 22 |
-) |
|
| 23 |
- |
|
| 24 | 19 |
// Service is the interface defining what a registry service should implement. |
| 25 | 20 |
type Service interface {
|
| 26 | 21 |
Auth(ctx context.Context, authConfig *types.AuthConfig, userAgent string) (status, token string, err error) |
| ... | ... |
@@ -184,8 +184,14 @@ func newSession(client *http.Client, endpoint *v1Endpoint) *session {
|
| 184 | 184 |
} |
| 185 | 185 |
} |
| 186 | 186 |
|
| 187 |
+// defaultSearchLimit is the default value for maximum number of returned search results. |
|
| 188 |
+const defaultSearchLimit = 25 |
|
| 189 |
+ |
|
| 187 | 190 |
// searchRepositories performs a search against the remote repository |
| 188 | 191 |
func (r *session) searchRepositories(term string, limit int) (*registry.SearchResults, error) {
|
| 192 |
+ if limit == 0 {
|
|
| 193 |
+ limit = defaultSearchLimit |
|
| 194 |
+ } |
|
| 189 | 195 |
if limit < 1 || limit > 100 {
|
| 190 | 196 |
return nil, invalidParamf("limit %d is outside the range of [1, 100]", limit)
|
| 191 | 197 |
} |