Commit [cli@27b2797] forked the AuthConfig type from the API, and changed
existing code to do a direct cast / convert of the forked type to the API
type. This can cause issues if the API types diverges, such as the removal
of the Email field.
This patch explicitly maps each field to the corresponding API type, but
adds some TODOs, because various code-paths only included a subset of the
fields, which may be intentional for fields that were meant to be handled
on the daemon / registry-client only.
We should evaluate these conversions to make sure these fields should
be sent from the client or not (and possibly even removed from the API
type).
[cli@27b2797]: https://github.com/docker/cli/commit/27b2797f7deb3ca5b7f80371d825113deb1faca1
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 9f02d9643d)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
All information needed can be deducted from the image reference, which
is used to create a indexInfo, repoInfo, and to resolve auth-config.
In some situations this may result in resolving the auth-config twice
after it already was resolved to an encoded auth-config.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 9a6313ed3b)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Docker Hub's Notary service is being retired, and now produces
failures in most cases. Add a warning when attempting to use
it, pending full removal of trust;
https://www.docker.com/blog/retiring-docker-content-trust/
With this PR:
DOCKER_CONTENT_TRUST=1 docker pull -q hello-world
WARNING: Docker is retiring DCT for Docker Official Images (DOI).
For details, refer to https://docs.docker.com/go/dct-deprecation/
could not validate the path to a trusted root: unable to retrieve valid leaf certificates
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 43b03ef2c5)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- Remove redundant intermediate variables
- Explicitly use an early return on error instead of combining with
other checks.
- Fix unhandled errors and combine defers
- Remove outstanding TODO that unlikely will be addressed
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit c36e67d7b6)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function was used internally, but is no longer used. There are
no known users of this method, so already removing it from the Cli
interface.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 0270b2d6f7)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function was used internally, but is no longer used. Users should check
the value of the `DOCKER_CONTENT_TRUST` environment variable instead.
There are no known external users of this method, so already removing it
from the Cli interface; this method will be removed in the next release.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 11d40488dd)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These options were used internally as defaults for the constructor and
only impact commands implemented in the CLI itself.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 40cdfc0d81)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Add support to the `cli/command` package to accept a custom User
Agent to pass to the underlying client.
This is used as the `UpstreamClient` portion of the `User-Agent`
when the Moby daemon makes requests.
For example, pushing and pulling images with Compose might result
in the registry seeing a `User-Agent` value of:
```
docker/24.0.7 go/go1.20.10 git-commit/311b9ff kernel/6.5.13-linuxkit os/linux arch/arm64 UpstreamClient(docker-cli-plugin-compose/v2.24.0)
```
Signed-off-by: Milas Bowman <milas.bowman@docker.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 048e931b42)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
The CLI currently hard-codes the schema-version for CLI plugins to
"0.1.0", which doesn't allow us to expand the schema for plugins.
As there's many plugins that we shipped already, we can't break
compatibility until we reach 2.0.0, but we can expand the schema
with non-breaking changes.
This patch makes the validation more permissive to allow new schema
versions <= 2.0.0. Note that existing CLIs will still invalidate
such versions, so we cannot update the version until such CLIs are
no longer expected to be used, but this patch lays the ground-work
to open that option.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit ec912e5524)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Currently, the plugin.Run command constructs the DockerCli using
the default options, assuming plugins run with all the same options
as the CLI itself; to customize the CLI there's a "Apply" option,
but this means mutating the CLI after it's already constructed, which
is not ideal.
This patch adds a variadic ops argument to allow CLI plugins to pass
custom options to use for the CLI, so that there's no need to mutate
its config in most cases.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 2711800430)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Slightly more verbose, but makes it easier to see properties
of each test.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 057f3128b6)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Experimental is always enabled (977d3ae046),
and the `Experimental` field in plugin metadata was deprecated in
977d3ae046 and removed in commit
6a50c4f700.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit dfbac70efa)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Before this patch, a broken symlink would print a warning;
docker info > /dev/null
WARNING: Plugin "/Users/thajeztah/.docker/cli-plugins/docker-feedback" is not valid: failed to fetch metadata: fork/exec /Users/thajeztah/.docker/cli-plugins/docker-feedback: no such file or directory
After this patch, such symlinks are ignored:
docker info > /dev/null
With debug enabled, we don't ignore the faulty plugin, which will
make the warning shown on docker info;
mkdir -p ~/.docker/cli-plugins
ln -s nosuchplugin ~/.docker/cli-plugins/docker-brokenplugin
docker --debug info
Client:
Version: 29.0.0-dev
Context: default
Debug Mode: true
Plugins:
buildx: Docker Buildx (Docker Inc.)
Version: v0.25.0
Path: /usr/libexec/docker/cli-plugins/docker-buildx
WARNING: Plugin "/Users/thajeztah/.docker/cli-plugins/docker-brokenplugin" is not valid: failed to fetch metadata: fork/exec /Users/thajeztah/.docker/cli-plugins/docker-brokenplugin: no such file or directory
# ...
We should als consider passing a "seen" map to de-duplicate entries.
Entries can be either a direct symlink or in a symlinked path (for
which we can filepath.EvalSymlinks). We need to benchmark the overhead
of resolving the symlink vs possibly calling the plugin (to get their
metadata) further down the line.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 9b2f831452)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- deprecate sockets.GetProxyEnv, sockets.DialerFromEnvironment
- add support for unix sockets on Windows
- remove legacy CBC cipher suites from client config
- align client and server defaults to be the same.
- remove support for encrypted TLS private keys.
- nat: optimize ParsePortSpec
full diff: https://github.com/docker/go-connections/compare/v0.5.0...v0.6.0
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 3529651fa7)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The Apply method was added when CLI options for constructing the CLI were
rewritten into functional options in [cli@7f207f3]. There was no mention
in the pull request of this method specifically, and this may have been
related to work being done elsewhere on compose-on-kubernetes or the
compose-cli plugin that may have needed options to modify the CLI config
after it was already initialized.
We should try to remove functions that mutate the CLI configuration after
initialization if possible (and likely remove the `Apply` method); currently
this function is used in docker compose, but as part of a hack that can
probably be avoided.
[cli@7f207f3]: https://github.com/docker/cli/commit/7f207f3f957ed3f5129aeb22bef2a429c14caf22
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 133279fb0d)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This change updates the builder prune command to send the `ReservedSpace` parameter in preparation of `KeepStorage` deprecation in API v1.52.
Signed-off-by: Austin Vazquez <austin.vazquez@docker.com>
(cherry picked from commit 7d85d8fbea)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This warning was added in [moby@4a8b3ca] to print a warning when building
Linux images from a Windows client. Window's filesystem does not have an
"executable" bit, which mean that, for example, copying a shell script
to an image during build would lose the executable bit. So for Windows
clients, the executable bit would be set on all files, unconditionally.
Originally this was detected in the client, which had direct access to
the API response headers, but when refactoring the client to use a common
library in [moby@535c4c9], this was refactored into a `ImageBuildResponse`
wrapper, deconstructing the API response into an `io.Reader` and a string
field containing only the `OSType` header.
This was the only use and only purpose of the `OSType` field, and now that
BuildKit is the default builder for Linux images, this warning didn't get
printed unless BuildKit was explicitly disabled.
This patch removes the warning, so that we can potentially remove the
field, or the `ImageBuildResponse` type altogether.
[moby@4a8b3ca]: https://github.com/moby/moby/commit/4a8b3cad6096854027151dfbcfb4b2cd8841ad95
[moby@535c4c9]: https://github.com/moby/moby/commit/535c4c9a59b1e58c897677d6948a595cb3d28639
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit af65ee4584)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Similar to 323fbc485e - this code was added
in [moby@c127d96], but used string-matching to detect cases where a user
tried to install an image as plugin. However, this handling no longer matched
any error-strings, so no longer worked:
docker plugin install busybox
Error response from daemon: did not find plugin config for specified reference docker.io/library/busybox:latest
[moby@c127d96]: https://github.com/moby/moby/commit/c127d9614f5b30bd73861877f8540a63e7d869e9
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit fb3f2da50e)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This special handling was added in [moby@9b6dcc8], and later updated in
[moby@c127d96], but it fully depended on string-matching, which is brittle.
Testing the original ticket that lead to this handling, it looks like the
string matching no longer works, and the daemon error is returned as-is:
With graphdrivers:
docker pull tiborvass/no-remove
Using default tag: latest
Error response from daemon: Encountered remote "application/vnd.docker.plugin.v0+json"(unknown) when fetching
With containerd snapshotters enabled:
docker pull tiborvass/no-remove
Using default tag: latest
latest: Pulling from tiborvass/no-remove
cf635291f7c9: Download complete
failed to unpack image on snapshotter overlayfs: mismatched image rootfs and manifest layers
The error-message for containerd can probably be improved, but as the special
handling in the CLI no longer works, we can remove it.
[moby@9b6dcc8]: https://github.com/moby/moby/commit/9b6dcc8b9d1366d3da3c8f60f89de1a36b087b88
[moby@c127d96]: https://github.com/moby/moby/commit/c127d9614f5b30bd73861877f8540a63e7d869e9
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 323fbc485e)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This code was missing a check for the ID field before truncating it to a
shorter length for presentation. This would result in a panic if an event
would either have an empty ID field or a shorter length ID;
panic: runtime error: slice bounds out of range [:12] with length 0
goroutine 82 [running]:
github.com/docker/cli/cli/command/container.RunStats.func2({{0x0, 0x0}, {0x0, 0x0}, {0x0, 0x0}, {0x40001fcba0, 0x9}, {0x40001fcba9, 0x5}, ...})
/go/src/github.com/docker/cli/cli/command/container/stats.go:146 +0x1d0
created by github.com/docker/cli/cli/command/container.(*eventHandler).watch in goroutine 6
/go/src/github.com/docker/cli/cli/command/container/stats.go:363 +0x1c8
We need to look at this code in general; the truncated ID is passed to
NewStats, which uses the ID to propagate the `Container` field in the
`StatsEntry` struct. which is not used in the default format used by
`docker stats` and, having the same content as the `ID` field on the
same struct, doesn't make it very useful, other than being able to
present it under a `CONTAINER` column (instead of `CONTAINER ID`);
we should consider deprecating it; there may be some subtle things
to look into here; the `Container` field originally held the container
name. This was changed in [moby@ef915fd], which introduced separate
`ID` and `Name` fields, renaming the old `Name` field to container.
Looking at [`Stats.SetStatistics()`] and related code in [stats_helpers.go],
the `Container` field is used as the "canonical" reference for the stats
record; this allows the stats _data_ to be refreshed when a new stats
sample arrives for the same container (also see [moby@929a77b], which
moved locking to the `Stats` wrapper struct). This construct allows to
account for intermediate states, where a stats sample was incomplete
or could produce an error; in that case, the reference to the container
for which the stats were sampled is kept to allow removing a container
from the list once the container was removed. We should consider removing
`Container` as a formatting option, and moving the `Container` field to
the outer struct; this makes the outer struct responsible for keeping a
reference to the container, allowing the `StatsEntry` as a whole to be
replaced atomically.
This patch only addresses the panic;
- It changes the logic to preserve the container ID verbatim instead
of truncating. This allows stats samples to be matched against the
`Actor.ID` as-is.
- Truncating the `Container` is moved to the presentation logic;
currently this does not take `--no-trunc` into account to keep
the existing behavior, but we can (should) consider adding this.
- Logging is improved to use structured logs, and an extra check is
added to prevent empty IDs from being added as watcher.
[`Stats.SetStatistics()`]: https://github.com/docker/cli/blob/82281087e3e186c5a2eafa0d973e849ff84c357d/cli/command/container/formatter_stats.go#L88-L94
[moby@ef915fd]: https://github.com/moby/moby/commit/ef915fd036d9ea5263f9370dce490ef97ea0618d
[moby@929a77b]: https://github.com/moby/moby/commit/929a77b814dfe9ab7a11bffc2d16eebd27bd903a
[stats_helpers.go]: https://github.com/docker/cli/blob/82281087e3e186c5a2eafa0d973e849ff84c357d/cli/command/container/stats_helpers.go#L26-L51
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 9b79e48646)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- Use sub-tests
- Don't use un-named keys
- Add test-cases for 'Name', 'ID' and custom container names
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit b9314938b7)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- Don't use unnamed keys
- Use sub-tests
- Add test-cases for Name and ID fields
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit b8cda96d11)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sebastiaan van Stijn
Austin Vazquez
Sebastiaan van Stijn
Paweł Gronowski
Milas Bowman
Austin Vazquez