- Split the implementation from the exported function (exported
function is still used by the CLI for Docker Content Trust).
- Pass through context to allow handling context-cancellation
once wired up in callers.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
inline a simplified version of "newIndexInfo" without handling of
insecure registries and mirrors, as we don't need that information
to resolve the auth-config.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Simplify how we lookup auth-config, as we don't need the
additional information provided by RepositoryInfo. There's
still more layers to peel off, which will be done in follow-ups.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This field was introduced in 19515a7ad8 when
the registry code was replaced for code vendored from docker/distribution.
It was used for v1 registries to update the "TrustStore" for signed manifests;
19515a7ad8/graph/pull.go (L89-L93)
Before that, it used the IndexInfo.Official field;
276c640be4/graph/pull.go (L94-L97)
And related to "v2" registries;
88fdcfef02
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function was only used internally, but it still has at least one
external consumer, so adding a "deprecated" comment.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
[homedir.GetConfigHome] only returns an error if the value is empty,
so we can check for a non-empty value instead of an error, which also
means that this value would never be empty.
[homedir.GetConfigHome]: b4bdf12dae/pkg/homedir/homedir_linux.go (L86-L95)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function had to be called both in the daemon startup, as well as
the CLI startup. Which, in case of the cli, meant that the registry
package became a required dependency for all CLI-plugins.
Make the package itself aware of situations where it's running with
rootlessKit enabled. Altogether we should get rid of this package-level
variable, and instead store this in our configuration, and pass through
where it's used.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function was implemented to use various parts of the registry package
that were designed for the daemon code, which was written with the assumption
that it had registry-config available from the daemon's configuration.
However, `ParseSearchIndexInfo` is used by the client / CLI, which does
not have this information.
To work around this problem, the code used a dummy "emptyServiceConfig"
to allow the `Insecure` and `Mirrors` fields to be propagated based on
the same defaults as used by the daemon. The `Mirrors` field would always
be empty, as there are no default mirrors, and (lacking access to the
daemon's config) the `Insecure` field would always default to registries
running on a loopback address (`::1/128`, `127.0.0.1/8`). It's worth noting
that neither the `Mirrors`, nor the `Insecure` field is used by the CLI.
This patch rewrites `ParseSearchIndexInfo` to be self-contained, and not
depend on these constructs (and the `emptyServiceConfig`). For now, the
existing logic for `Insecure` is kept, but replaced by a simplified function
(`isInsecure`) with some optimizations for well-known loopback addresses
(`localhost`, `::1`, `127.0.0.1`) to prevent redundant DNS lookups or
parsing.
Note that similar changes should be made for [ParseRepositoryInfo], which
has a similar fate and is also only used by the client / CLI.
[ResolveRepositoryName]: 11e47996dc/registry/registry.go (L199-L222)
[ParseRepositoryInfo]: d86dd75948/registry/config.go (L375-L381)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function was introduced in 568f86eb18
to replace [ResolveRepositoryName]. The function was implemented to use
various parts of the registry package that were designed for the daemon
code, which was written with the assumption that it had registry-config
available from the daemon's configuration. However, `ParseRepositoryInfo`
was used in the client / CLI, which does not have this information.
To work around this problem, the code used a dummy "emptyServiceConfig"
to allow the `Insecure` and `Mirrors` fields to be propagated based on
the same defaults as used by the daemon. The `Mirrors` field would always
be empty, as there are no default mirrors, and (lacking access to the
daemon's config) the `Insecure` field would always default to registries
running on a loopback address (`::1/128`, `127.0.0.1/8`). It's worth noting
that neither the `Mirrors`, nor the `Insecure` field is used by the CLI.
This patch rewrites `ParseRepositoryInfo` to be self-contained, and not
depend on these constructs (and the `emptyServiceConfig`). For now, the
existing logic for `Insecure` is kept, but replaced by a simplified function
(`isInsecure`) with some optimizations for well-known loopback addresses
(`localhost`, `::1`, `127.0.0.1`) to prevent redundant DNS lookups or
parsing.
Note that similar changes should be made for [ParseSearchIndexInfo], which
has a similar fate and is also only used by the client / CLI.
[ResolveRepositoryName]: 11e47996dc/registry/registry.go (L199-L222)
[ParseSearchIndexInfo]: d86dd75948/registry/search.go (L153-L162)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
registry/service.go:83:4: naked return in func `Auth` with 38 lines of code (nakedret)
return
^
registry/search_session.go:91:2: naked return in func `Read` with 6 lines of code (nakedret)
return
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- Add test-cases for IPv6 refs
- Add test-cases for validating the insecure-registries passed in the test
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Using DeepEquals showed that the test was missing differences between
nil-mirrors and empty-slice, in addition to mirrors being normalized
(the test only checked for the length).
While we should consider if we need an explicit empty slice (or if a
nil value would be appropriate), at least we now have a test to verify
the behavior.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
It was a very thin wrapper around newServiceConfig, and didn't save
any code needed; possibly even the reverse, as it was abstracting
what it did under the hood.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This was not revealed in our tests, which only checked for the length
of the Mirror-slice, but when testing with DeepEquals, tests were failing
when all tests were run (but succeeded on individual tests). The problem
here is that some code can mutate the list of Mirrors and set it to `nil`
or an empty slice, resulting in other tests to fail.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
ValidateIndexName is used by the docker daemon CLI to validate options
passed through CLI flags and daemon.json. However, it also handled
normalizing the registry name ("index.docker.io" -> "docker.io").
This patch splits the normalization code to a separate function. It
is currently not exported, but could be considered in the future;
if we do so, we may want to look for a better place for that function
to not have it in the same package as the registry code.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
emptyServiceConfig is a default service-config for situations where
no config-file is available (e.g. when used in the CLI). If won't
have mirrors configured, but does have the default insecure registry
CIDRs for loopback interfaces configured.
Before this patch, this config was constructeed using the same code
that handled constructing the config with a config present, but this
involved parsing CIDR masks, and much more.
With this patch, the service config is constructed as a literal, making
it more transparent that it does not depend on any config or state.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
RepositoryInfo.Official indicates whether the image repository
is an official (docker library official images) repository.
We only need to check this if the image-repository is on Docker Hub.
This patch renames the variable to make it more transparent that this
boolean is for the repository, and not to be confused for IndexInfo.Official,
which indicates if the _registry_ is the "Official" (Docker Hub) registry.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This field indicates whether a repository is an official image (Docker
Library / Docker Official Images). This information is deducted from the
image reference, and not used anywhere, other than in tests.
The `RepositoryInfo` and `IndexInfo` types were originally introduced in
568f86eb18, with their fields documented in
4fcb9ac40c.
At the time, the `Official` field was only used for `docker push` to produce
a custom error message if someone would attempt to push an official image
to docker hub (assuming nobody would be able to do so);
6870bde584/api/client/commands.go (L1184-L1194)
Before that commit, the condition for this error message was based on the
given image reference directly; b370acd679b370acd679/commands.go (L421-L428)
This patch deprecates the field, because it's not used, and removes
tests related to it.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This type was originally in pkg/transport, but got moved to pkg/ioutils
in 276c640be4.
This type is only used in a single location, and has no external consumers,
so we can move it where it's used and un-export it.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Wrap `http.RoundTripper` used by distribution code (push/pull) with the
`otelhttp.Transport`.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Non-distributable artifacts (foreign layers) were introduced in commit
05bd04350b to accommodate Windows images,
for which the EULA did not allow layers to be distributed through registries
other than those hosted by Microsoft. The concept of foreign / non-distributable
layers was adopted by the OCI distribution spec in [oci#233].
These restrictions were relaxed later to allow distributing these images
through non-public registries, for which a configuration was added in the
daemon in 67fdf574d5. In 2022, Microsoft updated
the EULA and [removed these restrictions altogether][1], and the OCI distribution
spec deprecated the functionality in [oci#965].
In 2023, Microsoft [removed the use of foreign data layers][2] for their images,
making this functionality obsolete.
This patch:
- Deprecates the `--allow-nondistributable-artifacts` daemon flag and corresponding
`allow-nondistributable-artifacts` field in `daemon.json`. Setting either
option will no longer take an effect, but a deprecation warning log is added
to raise awareness about the deprecation. This warning is planned to become
an error in the next release.
- Deprecates the `RegistryConfig.AllowNondistributableArtifactsCIDRs` and
`RegistryConfig.AllowNondistributableArtifactsHostnames` fields in the
`GET /info` API response. For API version v1.48 and lower, the fields are
still included in the response, but always `null`. In API version v1.49 and
higher, the field will be omitted entirely.
- Deprecates the `api/types/registry/ServiceConfig.AllowNondistributableArtifactsCIDRs`
field.
- Deprecates the `api/types/registry/ServiceConfig.AllowNondistributableArtifactsHostnames`
field.
- Deprecates the `registry.ServiceOptions.AllowNondistributableArtifacts` field.
[oci#233]: https://github.com/opencontainers/image-spec/pull/233
[oci#965]: https://github.com/opencontainers/image-spec/pull/965
[1]: https://techcommunity.microsoft.com/blog/containers/announcing-windows-container-base-image-redistribution-rights-change/3645201
[2]: https://techcommunity.microsoft.com/blog/containers/announcing-removal-of-foreign-layers-from-windows-container-images/3846833
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function unconditionally constructed endpoints for mirrors when
requesting endpoints for the default (Docker Hub) registry. Doing so
involves validating the config, which involves;
- parsing the hostname
- constructing TLS config
- performing a DNS lookup to resolve the host's IP address and matching
it against CIDR masks for insecure registries.
When looking up push endpoints or endpoints to consider for authentication,
mirror endpoints were discarded to prevent sending credentials of the upstream
registry to a mirror.
This patch adds a "includeMirrors" argument to skip constructing endpoints
for mirrors when not needed. While at it, also removing named output variables,
as they didn't add much.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
