Commit Graph

2004 Commits

Author SHA1 Message Date
15b95beac7 cli/command: add unit-test for RetrieveAuthTokenFromImage
It's currently slower because it calls registry.ParseRepositoryInfo,
which does a DNS lookup for hostnames to determine if they're a loopback
address (and marked "insecure");

    go test -v -run TestRetrieveAuthTokenFromImage
    === RUN   TestRetrieveAuthTokenFromImage
    === RUN   TestRetrieveAuthTokenFromImage/no-prefix
    === RUN   TestRetrieveAuthTokenFromImage/docker.io
    === RUN   TestRetrieveAuthTokenFromImage/index.docker.io
    === RUN   TestRetrieveAuthTokenFromImage/registry-1.docker.io
    === RUN   TestRetrieveAuthTokenFromImage/registry.hub.docker.com
    === RUN   TestRetrieveAuthTokenFromImage/[::1]
    === RUN   TestRetrieveAuthTokenFromImage/[::1]:5000
    === RUN   TestRetrieveAuthTokenFromImage/127.0.0.1
    === RUN   TestRetrieveAuthTokenFromImage/localhost
    === RUN   TestRetrieveAuthTokenFromImage/localhost:5000
    === RUN   TestRetrieveAuthTokenFromImage/no-auth.example.com
    --- PASS: TestRetrieveAuthTokenFromImage (0.35s)
        --- PASS: TestRetrieveAuthTokenFromImage/no-prefix (0.00s)
        --- PASS: TestRetrieveAuthTokenFromImage/docker.io (0.00s)
        --- PASS: TestRetrieveAuthTokenFromImage/index.docker.io (0.00s)
        --- PASS: TestRetrieveAuthTokenFromImage/registry-1.docker.io (0.08s)
        --- PASS: TestRetrieveAuthTokenFromImage/registry.hub.docker.com (0.12s)
        --- PASS: TestRetrieveAuthTokenFromImage/[::1] (0.13s)
        --- PASS: TestRetrieveAuthTokenFromImage/[::1]:5000 (0.00s)
        --- PASS: TestRetrieveAuthTokenFromImage/127.0.0.1 (0.00s)
        --- PASS: TestRetrieveAuthTokenFromImage/localhost (0.00s)
        --- PASS: TestRetrieveAuthTokenFromImage/localhost:5000 (0.00s)
        --- PASS: TestRetrieveAuthTokenFromImage/no-auth.example.com (0.01s)
    PASS
    ok  	github.com/docker/cli/cli/command	1.367s

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-26 14:25:50 +01:00
f596202125 Revert "cli/command: remove uses of GetAuthConfigKey, ParseRepositoryInfo"
This reverts commit 79141ce5eb.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2025-03-25 15:37:51 +01:00
5a8120c809 container/run: Fix TestRunAttachTermination
Restore part of the code removed by 966b44183f
that closed the stream. It's required now because the Run command won't
finish before the output stream was processed by the caller.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2025-03-24 17:09:35 +01:00
c27751fcfe container/run: Fix stdout/err truncation after container exit
Fix a regression introduced by 30c4637f03
which made the `docker run` command produce potentially truncated
stdout/stderr output.

Previous implementation stopped the content streaming as soon as the
container exited which would potentially truncate a long outputs.

This change fixes the issue by only canceling the IO stream immediately
if neither stdout nor stderr is attached.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2025-03-24 17:09:32 +01:00
9f19820f88 cli/command/completion: deprecate ValidArgsFn
Cobra now defines a CompletionFunc for the same, so we can alias
it to that, and stop using our own definition.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-21 20:30:58 +01:00
4541df21e5 cli/command/image: remove deprecated TagTrusted
This function was only used internally, and has no known external consumers.
It was deprecated in e37d814ce96b01393a400c081666ea1cca2eb8bd; this commit
removes it.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-20 11:11:39 +01:00
eaf98b2202 cli/command/image: remove deprecated PushTrustedReference
This function was only used internally, and has no known external consumers.
It was deprecated in d80436021c21c26b492f0014511f13f41d8b42d9; this commit
removes it.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-20 11:11:39 +01:00
98d0b0cc14 cli/command/image: remove deprecated TrustedPush
This function was only used by "docker trust sign", and has no known external
consumers. It was deprecated in c6f456bc90574f4180f3b990e8a4e216485e35b7;
this commit removes it.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-20 11:11:39 +01:00
5ea072d936 cli/command: remove deprecated RegistryClient from CLI interface
This method was a shallow wrapper around registryclient.NewRegistryClient but
due to its signature resulted in various dependencies becoming a dependency
of the "command" package. Consequence of this was that cli-plugins, which
need the cli/command package, would also get those dependencies. It is no
longer used, and was deprecated in 8ad07217dc.

This patch removes the RegistryClient method from the interface

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-20 11:11:38 +01:00
08f86507b4 cli/command: remove deprecated ManifestStore from CLI interface
This method is a shallow wrapper around manifeststore.NewStore, but
due to its signature resulted in various dependencies becoming a dependency
of the "command" package. Consequence of this was that cli-plugins, which
need the cli/command package, would also get those dependencies. It is no
longer used, and was deprecated in e32d5d56f5.

This patch removes the ManifestStore method from the interface

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-20 11:11:38 +01:00
66eb27a487 cli/command: remove deprecated NotaryClient from CLI interface
This method is a shallow wrapper around trust.GetNotaryRepository, but
due to its signature resulted in the trust package, and notary dependencies
to become a dependency of the CLI. Consequence of this was that cli-plugins,
which need the cli/command package, would also get notary and its
dependencies as a dependency. It is no longer used, and was deprecated
in 9bc16bbde0.

This patch removes the NotaryClient method from the interface

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-20 11:11:35 +01:00
e002576821 Merge pull request #5942 from thaJeztah/simplify_auth_step1
cli/command: remove uses of GetAuthConfigKey, ParseRepositoryInfo
2025-03-20 11:10:47 +01:00
3b45f3c09a Merge pull request #5926 from Benehiko/fix-attach-test-flake
test: fix flaky TestRunAttachTermination
2025-03-19 16:10:49 +01:00
79141ce5eb cli/command: remove uses of GetAuthConfigKey, ParseRepositoryInfo
Re-implement locally, based on the code in github.com/docker/docker/registry,
but leaving out bits that are not used on the client-side, such as
configuration of Mirrors, and configurable insecure-registry, which
are not used on the client side.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-19 15:34:57 +01:00
bb0e9adbc0 remove redundant error-handling for registry.ParseRepositoryInfo
Since [moby@c2c3d59], [registry.ParseRepositoryInfo] now always returns
a nil error, so we can remove the error handling.

[registry.ParseRepositoryInfo]: 5f0d6731eb/registry/config.go (L414-L443)
[moby@c2c3d59]: c2c3d593cf

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2025-03-19 13:55:38 +01:00
e0979b3adf cli/command: remove ValidateMountWithAPIVersion
This validation is now handled by the API-client since [moby@5d6b566],
so no longer needed to be done in the cli. This function was only used
internally and has no external consumers, so removing it without
deprecating first.

[moby@5d6b566]: 5d6b56699d

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2025-03-19 13:53:24 +01:00
966b44183f test: fix flaky TestRunAttachTermination
This patch fixes the `TestRunAttachTermination` flaky runs.
It seems like we weren't halting on the `waitFunc` so if the
process was fast enough to setup the signal handler and execute
`waitExitOrRemoved`. We now instead wait for the `killCh` channel
to close inside the mocked `waitFunc`.

Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>
2025-03-18 16:11:54 +01:00
d6d8ca6ebe config create: refactor, use limit reader, and touch up errors
Swarm has size constraints on the size of configs, but the client-side would
read content into memory, regardless its size. This could lead to either the
client reading too much into memory, or it sending data that's larger than
the size limit of gRPC, which resulted in the error not being handled by
SwarmKit and a generic gRPC error returned.

Reading a config from a file used a system.OpenSequential for reading
([FILE_FLAG_SEQUENTIAL_SCAN]). While there could be a very marginal benefit
to prevent polluting the system's cache (Windows won’t aggressively keep it
in the cache, freeing up system memory for other tasks). These details were
not documented in code, and possibly may be too marginal, but adding a comment
to outline won't hurt so this patch also adds a comment.

This patch:

- Factors out the reading code to a readConfigData, analogous to the
  equivalent in secret create.
- Implements reading the data with a limit-reader to prevent reading
  large files into memory.
- The limit is based on SwarmKits limits ([MaxConfigSize]), but made
  twice that size, just in case larger sizes are supported in future;
  the main goal is to have some constraints, and to prevent hitting
  the gRPC limit.
- Updates some error messages to include STDIN (when used), or the
  filename (when used).

Before this patch:

    ls -lh largefile
    -rw-------  1 thajeztah  staff   8.1M Mar  9 00:19 largefile

    docker config create nosuchfile ./nosuchfile
    Error reading content from "./nosuchfile": open ./nosuchfile: no such file or directory

    docker config create toolarge ./largefile
    Error response from daemon: rpc error: code = ResourceExhausted desc = grpc: received message larger than max (8462870 vs. 4194304)

    docker config create empty ./emptyfile
    Error response from daemon: rpc error: code = InvalidArgument desc = config data must be larger than 0 and less than 1024000 bytes

    cat ./largefile | docker config create toolarge -
    Error response from daemon: rpc error: code = ResourceExhausted desc = grpc: received message larger than max (8462870 vs. 4194304)

    cat ./emptyfile | docker config create empty -
    Error response from daemon: rpc error: code = InvalidArgument desc = config data must be larger than 0 and less than 1024000 bytes

With this patch:

    docker config create nosuchfile ./nosuchfile
    error reading from ./nosuchfile: open ./nosuchfile: no such file or directory

    docker config create empty ./emptyfile
    error reading from ./emptyfile: data is empty

    docker config create toolarge ./largefile
    Error response from daemon: rpc error: code = InvalidArgument desc = config data must be larger than 0 and less than 1024000 bytes

    cat ./largefile | docker config create toolarge -
    Error response from daemon: rpc error: code = InvalidArgument desc = secret data must be larger than 0 and less than 1024000 bytes

    cat ./emptyfile | docker config create empty -
    error reading from STDIN: data is empty

[FILE_FLAG_SEQUENTIAL_SCAN]: https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilea#FILE_FLAG_SEQUENTIAL_SCAN
[MaxConfigSize]: https://pkg.go.dev/github.com/moby/swarmkit/v2@v2.0.0-20250103191802-8c1959736554/manager/controlapi#MaxConfigSize

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-18 14:48:15 +01:00
3a35b16669 secret create: refactor, use limit reader, and touch up errors
Swarm has size constraints on the size of secrets, but the client-side would
read content into memory, regardless its size. This could lead to either the
client reading too much into memory, or it sending data that's larger than
the size limit of gRPC, which resulted in the error not being handled by
SwarmKit and a generic gRPC error returned.

Reading a secret from a file was added in [moby@c6f0b7f], which used a
system.OpenSequential for reading ([FILE_FLAG_SEQUENTIAL_SCAN]). While
there could be a very marginal benefit to prevent polluting the system's
cache (Windows won’t aggressively keep it in the cache, freeing up system
memory for other tasks). These details were not documented in code, and
possibly may be too marginal, but adding a comment to outline won't hurt
so this patch also adds a comment.

This patch:

- Rewrites readSecretData to not return a nil-error if no file was
  set, in stead only calling it when not using a driver.
- Implements reading the data with a limit-reader to prevent reading
  large files into memory.
- The limit is based on SwarmKits limits ([MaxSecretSize]), but made
  twice that size, just in case larger sizes are supported in future;
  the main goal is to have some constraints, and to prevent hitting
  the gRPC limit.
- Updates some error messages to include STDIN (when used), or the
  filename (when used).

Before this patch:

    ls -lh largefile
    -rw-------  1 thajeztah  staff   8.1M Mar  9 00:19 largefile

    docker secret create nosuchfile ./nosuchfile
    Error reading content from "./nosuchfile": open ./nosuchfile: no such file or directory

    docker secret create toolarge ./largefile
    Error response from daemon: rpc error: code = ResourceExhausted desc = grpc: received message larger than max (8462870 vs. 4194304)

    docker secret create empty ./emptyfile
    Error response from daemon: rpc error: code = InvalidArgument desc = secret data must be larger than 0 and less than 512000 bytes

    cat ./largefile | docker secret create toolarge -
    Error response from daemon: rpc error: code = ResourceExhausted desc = grpc: received message larger than max (8462870 vs. 4194304)

    cat ./emptyfile | docker secret create empty -
    Error response from daemon: rpc error: code = InvalidArgument desc = secret data must be larger than 0 and less than 512000 bytes

With this patch:

    docker secret create nosuchfile ./nosuchfile
    error reading from ./nosuchfile: open ./nosuchfile: no such file or directory

    docker secret create empty ./emptyfile
    error reading from ./emptyfile: data is empty

    docker secret create toolarge ./largefile
    Error response from daemon: rpc error: code = InvalidArgument desc = secret data must be larger than 0 and less than 512000 bytes

    cat ./largefile | docker secret create toolarge -
    Error response from daemon: rpc error: code = InvalidArgument desc = secret data must be larger than 0 and less than 512000 bytes

    cat ./emptyfile | docker secret create empty -
    error reading from STDIN: data is empty

[moby@c6f0b7f]: c6f0b7f448
[FILE_FLAG_SEQUENTIAL_SCAN]: https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilea#FILE_FLAG_SEQUENTIAL_SCAN
[MaxSecretSize]: https://pkg.go.dev/github.com/moby/swarmkit/v2@v2.0.0-20250103191802-8c1959736554/api/validation#MaxSecretSize

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-18 14:47:56 +01:00
d421dea843 Merge pull request #5908 from thaJeztah/client_api_version
remove uses of cli.DefaultVersion()
2025-03-11 10:07:31 +01:00
3d3f78028a Merge pull request #5919 from thaJeztah/no_generics
cli/command/formatter: add missing go:build tag
2025-03-10 18:18:06 +01:00
64b56179b5 Merge pull request #5907 from thaJeztah/opts_cleanup
opts: remove uses pkg/errors, and move swarm-specific opts to a separate package
2025-03-10 18:03:49 +01:00
2cd4786630 cli/command/formatter: add missing go:build tag
Seen failing when used elsewhere;

    vendor/github.com/docker/cli/cli/command/formatter/displayutils.go:78:20: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-10 17:39:06 +01:00
60ae1bb1fc Merge pull request #5910 from thaJeztah/move_service_logs
service/logs: move to cli/internal/logdetails
2025-03-10 16:14:22 +01:00
879acd15ff Merge pull request #5911 from thaJeztah/builder_nits
cli/command/image: fix some minor linting issues
2025-03-10 16:12:24 +01:00
a07391c65d Merge pull request #5906 from thaJeztah/remove_client_warnings
fix duplicate warnings on docker run / docker create, and slight refactor
2025-03-10 16:03:38 +01:00
650b45a42a Merge pull request #5915 from thaJeztah/remove_StringSliceReplaceAt
cli/command: remove StringSliceReplaceAt utility
2025-03-10 13:37:31 +01:00
bc57a035c4 Merge pull request #5916 from thaJeztah/move_command_prettyprint
cli/command: move PrettyPrint utility to cli/command/formatter
2025-03-10 13:35:03 +01:00
a390a32da1 Merge pull request #5917 from thaJeztah/prunefilters_shallow_interface
cli/command: PruneFilters: require smaller interface
2025-03-10 13:33:00 +01:00
e9cf371b56 Merge pull request #5913 from thaJeztah/image_load_cleanup
image load: combine checks to a single switch
2025-03-10 11:54:30 +00:00
d2b751ce58 cli/command: PruneFilters: require smaller interface
This function only needs access to the CLI's configfile; use the
config.Prider interface to be more clear on what's expected.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-09 22:30:05 +01:00
a5ec6c2963 cli/command: remove StringSliceReplaceAt utility
It was only used internally in cmd/docker and has no known external
consumers. Move it to cmd/docker and un-export it.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-09 22:22:29 +01:00
ce3090ccc4 cli/command: move PrettyPrint utility to cli/command/formatter
This utility was only used internally, and has no external consumers;
move it to the "formatter" package, which is also imported in all files
using this utility.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-09 22:20:12 +01:00
802d8e801a image load: combine checks to a single switch
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-09 14:43:57 +01:00
6bd6b3e8ac service/logs: move to cli/internal/logdetails
This package is only used by cli/command/service, and has no
external consumers.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-08 22:51:11 +01:00
2c0f9f476d cli/command/image: explicitly ignore some unhandled errs
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-08 22:47:56 +01:00
e73fb7d2f6 cli/command/image: rename var that shadowed type
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-08 22:43:21 +01:00
79c9c7e3e4 cli/command/system: ignore unhandled errors
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-08 19:32:35 +01:00
5f13d0f2b5 remove uses of cli.DefaultVersion()
It's hard-coded to the API defaultversion, so we can use
that const directly. Ultimately, this should be something
returned by the API client configured on the CLI, not the
CLI itself.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-08 19:32:31 +01:00
ad21055bac opts: move swarm-specific options to a separate package
This prevents users of the CLI that don't implement swarm-related
features from depending on the swarm API types.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-08 18:39:07 +01:00
bc90bb6855 container create: combine client-side warning with daemon-side
Use a consistent approach for producing warnings, but add a TODO for moving
this warning to the daemon, which can make a better call if it will work
or not (depending on networking mode).

This warning was originally added in [moby@afa92a9], before integration with
libnetwork, and this warning may be incorrect in many scenarios.

While updating, also removing the custom regular expression used to
detect if the IP is a loopback address, and using go's netip package
instead.

[moby@afa92a9]: afa92a9af0

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-08 16:15:50 +01:00
58a35692d6 remove duplicate --oom-kill-disable warnings on docker run / docker create
This warning was originally added in [moby@3aa70c1], and moved to be printed
on both `run` and `create` in commit 7c514a31c9.

However, [moby@57f1305] (docker 19.03, API 1.40) moved such warnings to
the daemon side. The patch mentioned this issue:

> This patch will have one side-effect; docker cli's that also perform this check
> client-side will print the warning twice; this can be addressed by disabling
> the cli-side check for newer API versions, but will generate a bit of extra
> noise when using an older CLI.

The CLI does not take this into account currently, and still prints warnings
twice; even in cases where the option is not supported by the daemon, and
discarded:

On a host without OomKillDisable support:

    docker create --oom-kill-disable alpine
    WARNING: Disabling the OOM killer on containers without setting a '-m/--memory' limit may be dangerous.
    WARNING: Your kernel does not support OomKillDisable. OomKillDisable discarded.

On a host that supports it:

    docker create --oom-kill-disable alpine
    WARNING: Disabling the OOM killer on containers without setting a '-m/--memory' limit may be dangerous.
    WARNING: OOM killer is disabled for the container, but no memory limit is set, this can result in the system running out of resources.

This patch removes the client-side warning, leaving it to the daemon to
report if any warnings should produced (and the client to print them).

With this patch applied:

On a host without OomKillDisable support:

    docker create --oom-kill-disable alpine
    WARNING: Your kernel does not support OomKillDisable. OomKillDisable discarded.

On a host that supports it:

    docker create --oom-kill-disable alpine
    WARNING: OOM killer is disabled for the container, but no memory limit is set, this can result in the system running out of resources.

[moby@3aa70c1]: 3aa70c1948
[moby@57f1305]: 57f1305e74

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-08 15:39:21 +01:00
46caf5697c remove legacy build-tags
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-08 12:47:03 +01:00
2eec74659e Merge pull request #5901 from NinaLua/master
chore: make function comment match function name
2025-03-07 20:42:38 +01:00
4321293972 move cli-plugins metadata types/consts to a separate package
This prevents cli-plugins having to import the plugin-manager.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-07 12:38:06 +01:00
aa66f07a3e chore: make function comment match function name
Signed-off-by: NinaLua <iturf@sina.cn>
2025-03-07 14:21:27 +08:00
e37d814ce9 cli/command/image: deprecate TagTrusted, move to cli/trust
This function was shared between "image" and "container" packages,
all of which needed the trust package, so move it there instead.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-05 21:36:53 +01:00
d80436021c cli/command/image: deprecate PushTrustedReference, move to trust
This function was shared between "trust" "image" and "plugin" packages,
all of which needed the trust package, so move it there instead.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-05 18:29:12 +01:00
c6f456bc90 cli/command/image: deprecate and internalize TrustedPush
This function was only used by "docker trust sign"; inline the code
and deprecate the function.

This function has no known external consumers, so we should remove
it on the first possible ocassion (which could be a minor release).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-03-05 18:29:08 +01:00
e558b915c2 Merge pull request #5892 from thaJeztah/pluginmanager_smaller_interface
cli-plugins/manager: use shallower interface
2025-03-05 13:18:57 +01:00