Just return a regular error, because the API converts the error to
the expected ErrorResponse. Before/After produce the same API response:
curl -v --unix-socket /var/run/docker.sock 'http://localhost/v1.43/images/search?term=hello'
* Trying /var/run/docker.sock:0...
* Connected to localhost (/var/run/docker.sock) port 80 (#0)
> GET /v1.43/images/search?term=hello HTTP/1.1
> Host: localhost
> User-Agent: curl/7.74.0
> Accept: */*
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 500 Internal Server Error
< Api-Version: 1.44
< Content-Type: application/json
< Docker-Experimental: false
< Ostype: linux
< Server: Docker/dev (linux)
< Traceparent: 00-c38c2da5cf30305fcb66836a28e227bf-d16f4f7d2c7002a1-01
< Date: Mon, 18 Sep 2023 14:30:18 GMT
< Content-Length: 41
<
{"message":"Unexpected status code 409"}
* Connection #0 to host localhost left intact
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
validateEndpoint was doing more than just validating; it was also implicitly
mutating the endpoint that was passed to it (by reference).
Given that validation only happend when constructing a new v1Endpoint, let's
merge these functions.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
validateEndpoint uses `v1Endpoint.ping` to verify if the search API can
use a secure connection, and to fall back to basic auth. For Docker Hub,
we don't allow insecure connections, and `v1Endpoint.ping` will not connect
to Docker Hub (Docker Hub also does not implement the `_ping` endpoint,
so doing so would always fail).
Let's make it more clear that we don't do any validation, and return
early.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
We have the response available, which is an io.Reader, so we don't have
to read the entire response into memory before decoding.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function was making a request to the `_ping` endpoint, which (if
implemented) would return a JSON response, which we unmarshal (the only
field we use from the response is the `Standalone` field).
However, if the response had a `X-Docker-Registry-Standalone`, that header
took precedence, and would overwrite the earlier `Standalone` value we
obtained from the JSON response.
This patch adds a fast-path for situations where the header is present,
in which case we can skip handling the JSON response altogether.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- The `Version` field was not used for any purpose, other than a debug log
- The `X-Docker-Registry-Version` header was part of the registry v1 spec,
however, as we're not using the `Version` field, we don't need the
header for anything.
- The `X-Docker-Registry-Config` header was only set by the mock registry;
there's no code consuming it, so we don't need to mock it (even if an
actual v1 registry / search API would return it).
It's also worth noting that we never call the `_ping` endpoint when using
Docker Hub's search API, and Docker Hub does not even implement the `_ping`
endpoint;
curl -fsSL https://index.docker.io/_ping | head -n 4
<!DOCTYPE html>
<html lang="en">
<head>
<title>Docker</title>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This field was used when the code supported both "v1" and "v2" registries.
We no longer support v1 registries, and the only v1 endpoint that's still
used is for the legacy "search" endpoint, which does not use the APIEndpoint
type.
As no code is using this field, and the value will always be set to "v2",
we can deprecated the Version field.
I'm keeping this field for 1 release, to give notice to any potential
external consumer, after which we can delete it.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Explain that search is not supported on v2 endpoints, and include the
offending endpoint in the error-message.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
First, remove the loop over `apiVersions`. The `apiVersions` map has two
entries (`APIVersion1 => "v1"`, and `APIVersion2 => "v2"`), and `APIVersion1`
is skipped, which means that the loop effectively translates to;
if apiVersionStr == "v2" {
return "", invalidParamf("unsupported V1 version path %s", apiVersionStr)
}
Which leaves us with "anything else" being returned as-is.
This patch removes the loop, and replaces the remaining handling to check
for the "v2" suffix to produce an error, or to strip the "v1" suffix.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Combine the two tests into a TestV1EndpointParse function, and rewrite
them to use gotest.tools for asserting.
Also changing the test-cases to use "https://", as the scheme doesn't
matter for this test, but using "http://" may trip-up some linters,
so let's avoid that.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The is-automated field is being deprecated by Docker Hub's search API,
and will always be set to "false" in future.
This patch deprecates the field and related filter for the Engine's API.
In future, the `is-automated` filter will no longer yield any results
when searching for `is-automated=true`, and will be ignored when
searching for `is-automated=false`.
Given that this field is deprecated by an external API, the deprecation
will not be versioned, and will apply to any API version.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- return a errdefs.System if we fail to decode the registry's response
- use strconv.Itoa instead of fmt.Sprintf
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Config reloading has interleaved validations and other fallible
operations with mutating the live daemon configuration. The daemon
configuration could be left in a partially-reloaded state if any of the
operations returns an error. Mutating a copy of the configuration and
atomically swapping the config struct on success is not currently an
option as config values are not copyable due to the presence of
sync.Mutex fields. Introduce a two-phase commit protocol to defer any
mutations of the daemon state until after all fallible operations have
succeeded.
Reload transactions are not yet entirely hermetic. The platform
reloading logic for custom runtimes on *nix could still leave the
directory of generated runtime wrapper scripts in an indeterminate state
if an error is encountered.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Commit 3991faf464 moved search into the registry
package, which also made the `dockerversion` package a dependency for registry,
which brings additional (indirect) dependencies, such as `pkg/parsers/kernel`,
and `golang.org/x/sys/windows/registry`.
Client code, such as used in docker/cli may depend on the `registry` package,
but should not depend on those additional dependencies.
This patch moves setting the userAgent to the API router, and instead of
passing it as a separate argument, includes it into the "headers".
As these headers now not only contain the `X-Meta-...` headers, the variables
were renamed accordingly.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
SearchRegistryForImages does not make sense as part of the image
service interface. The implementation just wraps the search API of the
registry service to filter the results client-side. It has nothing to do
with local image storage, and the implementation of search does not need
to change when changing which backend (graph driver vs. containerd
snapshotter) is used for local image storage.
Filtering of the search results is an implementation detail: the
consumer of the results does not care which actor does the filtering so
long as the results are filtered as requested. Move filtering into the
exported API of the registry service to hide the implementation details.
Only one thing---the registry service implementation---would need to
change in order to support server-side filtering of search results if
Docker Hub or other registry servers were to add support for it to their
APIs.
Use a fake registry server in the search unit tests to avoid having to
mock out the registry API client.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Previously, Docker Hub was excluded when configuring "allow-nondistributable-artifacts".
With the updated policy announced by Microsoft, we can remove this restriction;
https://techcommunity.microsoft.com/t5/containers/announcing-windows-container-base-image-redistribution-rights/ba-p/3645201
There are plans to deprecated support for foreign layers altogether in the OCI,
and we should consider to make this option the default, but as that requires
deprecating the option (and possibly keeping an "opt-out" option), we can look
at that separately.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
runconfig/config_test.go:23:46: empty-lines: extra empty line at the start of a block (revive)
runconfig/config_test.go:75:55: empty-lines: extra empty line at the start of a block (revive)
oci/devices_linux.go:57:34: empty-lines: extra empty line at the start of a block (revive)
oci/devices_linux.go:60:69: empty-lines: extra empty line at the start of a block (revive)
image/fs_test.go:53:38: empty-lines: extra empty line at the end of a block (revive)
image/tarexport/save.go:88:29: empty-lines: extra empty line at the end of a block (revive)
layer/layer_unix_test.go:21:34: empty-lines: extra empty line at the end of a block (revive)
distribution/xfer/download.go:302:9: empty-lines: extra empty line at the end of a block (revive)
distribution/manifest_test.go:154:99: empty-lines: extra empty line at the end of a block (revive)
distribution/manifest_test.go:329:52: empty-lines: extra empty line at the end of a block (revive)
distribution/manifest_test.go:354:59: empty-lines: extra empty line at the end of a block (revive)
registry/config_test.go:323:42: empty-lines: extra empty line at the end of a block (revive)
registry/config_test.go:350:33: empty-lines: extra empty line at the end of a block (revive)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
strings.ReplaceAll(s, old, new) is a wrapper function for
strings.Replace(s, old, new, -1). But strings.ReplaceAll is more
readable and removes the hardcoded -1.
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
The registry package contained code to automatically set the CertsDir() path,
based on wether or not the daemon was running in rootlessmode. In doing so,
it made use of the `pkg/rootless.RunningWithRootlessKit()` utility.
A recent change in de6732a403 added additional
functionality in the `pkg/rootless` package, introducing a dependency on
`github.com/rootless-containers/rootlesskit`. Unfortunately, the extra
dependency also made its way into the docker cli, which also uses the
registry package.
This patch introduces a new `SetCertsDir()` function, which allows
the default certs-directory to be overridden, and updates the daemon
to configure this location during startup.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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>
