Commit Graph

97 Commits

Author SHA1 Message Date
Sebastiaan van Stijn 35b5ac3b88
cmd/docker: areFlagsSupported: don't Ping if not needed
This is a similar fix as 006c946389, which
fixed this for detection of commands that were executed. Make sure we don't
call the "/_ping" endpoint if we don't need to.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit bb57783ab8)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-22 10:47:14 +02:00
Paweł Gronowski c648e0b065
Handle empty DOCKER_BUILDKIT like unset
This fixes the cli erroring out if the variable is set to an empty
value.

```
$ export DOCKER_BUILDKIT=
$ docker version
DOCKER_BUILDKIT environment variable expects boolean value: strconv.ParseBool: parsing "": invalid syntax
```

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
(cherry picked from commit ff7f76af7a)
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2023-04-19 15:53:40 +02:00
Kevin Alvarez d93d78588d
load plugin command stubs when required
We are currently loading plugin command stubs for every
invocation which still has a significant performance hit.
With this change we are doing this operation only if cobra
completion arg request is found.

- 20.10.23: `docker --version` takes ~15ms
- 23.0.1: `docker --version` takes ~93ms

With this change `docker --version` takes ~9ms

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
(cherry picked from commit c39c711a18)
2023-03-31 16:38:01 +02:00
Sebastiaan van Stijn fc6be6ad30
cli: pass dockerCLI's in/out/err to cobra cmds
Both the DockerCLI and Cobra Commands provide accessors for Input, Output,
and Error streams (usually STDIN, STDOUT, STDERR). While we were already
passing DockerCLI's Output to Cobra, we were not doing so for the other
streams (and were passing none for plugin commands), potentially resulting
in DockerCLI output/input to mean something else than a Cobra Command's
intput/output/error.

This patch sets them to the same streams when constructing the Cobra
command.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-01-15 13:44:33 +01:00
Sebastiaan van Stijn 06eba426d7
cmd/docker: fix typo in deprecation warning
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-12-19 13:03:28 +01:00
Sebastiaan van Stijn 60d62fb729
cmd/docker: improve error message if BUILDKIT_ENABLED=0
Before this change, the error would suggest installing buildx:

    echo "FROM scratch" | DOCKER_BUILDKIT=0  docker build -
    DEPRECATED: The legacy builder is deprecated and will be removed in a future release.
                Install the buildx component to build images with BuildKit:
                https://docs.docker.com/go/buildx/

    ...

However, this error would also be shown if buildx is actually installed,
but disabled through "DOCKER_BUILDKIT=0";

    docker buildx version
    github.com/docker/buildx v0.9.1 ed00243

With this patch, it reports that it's disabled, and how to fix:

    echo "FROM scratch" | DOCKER_BUILDKIT=0  docker build -
    DEPRECATED: The legacy builder is deprecated and will be removed in a future release.
                BuildKit is currently disabled; enabled it by removing the DOCKER_BUILDKIT=0
                environment-variable.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-12-09 13:08:07 +01:00
Sebastiaan van Stijn 006c946389
cmd/docker: make feature detection lazy again
Commit 20ba591b7f fixed incorrect feature
detection in the CLI, but introduced a regression; previously the "ping"
would only be executed if needed (see b39739123b),
but by not inlining the call to `ServerInfo()` would now always be called.

This patch inlines the code again to only execute the "ping" conditionally,
which allows it to be executed lazily (and omitted for commands that don't
require a daemon connection).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-12-06 10:17:50 +01:00
Adyanth Hosavalike 20ba591b7f
Fix bug where incorrect response is returned
When server is unreachable and docker checkpoint (or any command that
needs to check the server type) is run, incorrect error was returned.

When checking if the daemon had the right OS, we compared the OSType
from the clients ServerInfo(). In situations where the client cannot
connect to the daemon, a "stub" Info is used for this, in which we
assume the daemon has experimental enabled, and is running the latest
API version.

However, we cannot fill in the correct OSType, so this field is empty
in this situation.

This patch only compares the OSType if the field is non-empty, otherwise
assumes the platform matches.

before this:

    docker -H unix:///no/such/socket.sock checkpoint create test test
    docker checkpoint create is only supported on a Docker daemon running on linux, but the Docker daemon is running on

with this patch:

    docker -H unix:///no/such/socket.sock checkpoint create test test
    Cannot connect to the Docker daemon at unix:///no/such/socket.sock. Is the docker daemon running?

Co-authored-by: Adyanth Hosavalike <ahosavalike@ucsd.edu>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-12-06 08:55:47 +01:00
Sebastiaan van Stijn 121c613877
cil/command: use dummy client for build-tests
These tests were using the default client, which would try to make a connection
with the daemon (which isn't running). Some of these test subsequently had
tests that depended on the result of that connection (i.e., "ping" result).

This patch updates the test to use a dummy client, so that the ping result is
predictable.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-12-05 22:37:40 +01:00
Sebastiaan van Stijn a7e2c3ea1e
cli/command: add Cli.CurrentVersion() function
This internalizes constructing the Client(), which allows us to provide
fallbacks when trying to determin the current API version.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-11-28 10:49:01 +01:00
Sebastiaan van Stijn 3499669e18
cli/flags: merge CommonOptions into ClientOptions
CommonOptions was inherited from when the cli and daemon were in the same
repository, and some options would be shared between them. That's no longer
the case, and some options are even "incorrect" (for example, while the
daemon can be configured to run on multiple hosts, the CLI can only connect
with a single host / connection). This patch does not (yet) address that,
but merges the CommonOptions into the ClientOptions.

An alias is created for the old type, although it doesn't appear there's
any external consumers using the CommonOptions type (or its constructor).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-11-22 12:32:18 +01:00
Brian Goff 79dca7a38e
Merge pull request #3676 from crazy-max/build-default-builder
build: set default context builder if not specified
2022-11-09 12:37:36 -08:00
CrazyMax 997846918e
build: keep "buildx install" behavior
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
2022-11-04 08:42:34 +01:00
CrazyMax d1cabdff99
build: set default context builder if not specified
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
2022-11-04 08:42:34 +01:00
Sebastiaan van Stijn 616124525e
format go with gofumpt (with -lang=1.19)
Looks like the linter uses an explicit -lang, which (for go1.19)
results in some additional formatting for octal values.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-09-30 19:14:36 +02:00
Sebastiaan van Stijn 1da95ff6aa
format code with gofumpt
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-09-30 11:59:11 +02:00
Sebastiaan van Stijn 90f1238fb2
cli-plugins/manager: add IsPluginCommand(() utility
This makes it more convenient to check if a command is a plugin-stub

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-09-30 02:24:23 +02:00
Sebastiaan van Stijn 7af8aac169
fix broken alias check is buildx is installed as alias for builder
Commit cbec75e2f3 updated `runDocker()` to load
plugin-stubs before `processAliases()` was executed. As a result, plugin
stubs were considered as "builtin commands", causing the alias verification
to fail;

Without alias installed:

```bash
docker version
Client:
 Version:           22.06.0-beta.0-140-g3dad26ca2.m
 API version:       1.42
 Go version:        go1.19.1
 Git commit:        3dad26ca2
 Built:             Wed Sep 28 22:36:09 2022
 OS/Arch:           darwin/arm64
 Context:           default
...
```

After running `docker buildx install`;

```bash
./build/docker buildx install

cat ~/.docker/config.json
{
    "aliases": {
        "builder": "buildx"
    }
}

./build/docker version
not allowed to alias with builtin "buildx" as target
```

This patch moves loading the stubs _after_ the call to `processAliases()`, so
that verification passes. As an extra precaution, the `processAliases()` function
is also updated to exclude plugin-stub commands.

Note that cbec75e2f3 also introduced a performance
regression, which may be related to the early loading of plugins (and creating
stubs); it looks like various other code locations may also be loading plugins,
for example `tryPluginRun()` calls `pluginmanager.PluginRunCommand()`, which
also traverses plugin directories.

We should look under what circumstances the plugin stub-commands are actually
needed, and make sure that they're only created in those situations.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-09-29 22:40:51 +02:00
Sebastiaan van Stijn 28b0aa9f1a
replace uses of deprecated env.Patch()
Also removing redundant defer for env.PatchAll(), which is now automatically
handled in t.Cleanup()

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-09-22 17:28:07 +02:00
Nicolas De Loof cbec75e2f3
Adopt Cobra completion v2 to support completion by CLI plugins
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
2022-05-12 12:59:10 +02:00
Sebastiaan van Stijn db141c21e9
hide swarm-related commands based on the current swarm status and role
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-05-02 14:57:59 +02:00
Sebastiaan van Stijn 257f6149ba
Remove ClientInfo as it is not practically used.
The information in this struct was basically fixed (there's
some discrepancy around the "DefaultVersion" which, probably,
should never vary, and always be set to the Default (maximum)
API version supported by the client.

Experimental is now always enabled, so this information did
not require any dynamic info as well.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-03-04 15:46:50 +01:00
Sebastiaan van Stijn 0e3197ebd4
cmd/docker: remove deprecated io/ioutil
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-02-25 15:42:19 +01:00
CrazyMax e7a8748b93
build: use legacy builder for wcow if not opt-in with a builder component
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
2022-02-24 17:57:56 +01:00
CrazyMax 16edf8bffb
builder: conditional warning for wcow
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
2022-02-03 19:15:58 +01:00
Sebastiaan van Stijn bce65f0edc
builder: simplify error generation, and rephrase error/warning
With this change:

    echo 'FROM busybox' | DOCKER_BUILDKIT=1 docker build -
    ERROR: BuildKit is enabled but the buildx component is missing or broken.
           Install the buildx component to build images with BuildKit:
           https://docs.docker.com/go/buildx/

    echo 'FROM busybox' | docker build -
    DEPRECATED: The legacy builder is deprecated and will be removed in a future release.
                Install the buildx component to build images with BuildKit:
                https://docs.docker.com/go/buildx/

    Sending build context to Docker daemon  2.048kB
    ...

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-02-03 10:38:05 +01:00
CrazyMax 4d8e45782b
builder: fallback to legacy
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
2022-02-03 10:38:05 +01:00
CrazyMax 6fef143dbc
Set buildx as default builder
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
2022-02-03 10:38:05 +01:00
CrazyMax 75284bd1d1
Use goversioninfo to create Windows Version Info
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
2021-10-11 16:54:22 +02:00
Sebastiaan van Stijn b83bc67136
config: print deprecation warning when falling back to ~/.dockercfg
Relates to the deprecation, added in 3c0a167ed5

The docker CLI up until v1.7.0 used the `~/.dockercfg` file to store credentials
after authenticating to a registry (`docker login`). Docker v1.7.0 replaced this
file with a new CLI configuration file, located in `~/.docker/config.json`. When
implementing the new configuration file, the old file (and file-format) was kept
as a fall-back, to assist existing users with migrating to the new file.

Given that the old file format encourages insecure storage of credentials
(credentials are stored unencrypted), and that no version of the CLI since
Docker v1.7.0 has created this file, the file is marked deprecated, and support
for this file will be removed in a future release.

This patch adds a deprecation warning, which is printed if the CLI falls back
to using the deprecated ~/.dockercfg file.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2021-03-08 16:13:02 +01:00
Silvin Lubecki 5695d699ae
Merge pull request #2735 from thaJeztah/fix_flag_hiding
Fix buildkit flags not being hidden without buildkit enabled
2020-10-29 15:59:02 +01:00
Tonis Tiigi 857f5856f8 handle sigterm on running a plugin
While running a plugin and canceling with SIGTERM, main process will
close right away without letting the plugin close itself down and handle
the exit code properly. Add appcontext that is useful for handling
sigterm, as well as supporting sigkill when things go wrong.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
2020-10-21 22:32:49 -07:00
Sebastiaan van Stijn 977d3ae046
Always enable experimental features
The CLI disabled experimental features by default, requiring users
to set a configuration option to enable them.

Disabling experimental features was a request from Enterprise users
that did not want experimental features to be accessible.

We are changing this policy, and now enable experimental features
by default. Experimental features may still change and/or removed,
and will be highlighted in the documentation and "usage" output.

For example, the `docker manifest inspect --help` output now shows:

    EXPERIMENTAL:
      docker manifest inspect is an experimental feature.

      Experimental features provide early access to product functionality. These features
      may change between releases without warning or can be removed entirely from a future
      release. Learn more about experimental features: https://docs.docker.com/go/experimental/

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-10-02 15:59:42 +02:00
Sebastiaan van Stijn b4eb079045
Fix buildkit flags not being hidden without buildkit enabled
These annotations use `nil` as value, which caused the flag-hiding functions
to ignore the annotations, and therefore not hiding the flags.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-09-22 14:47:55 +02:00
Sebastiaan van Stijn 719169db63
Replace deprecated Cobra command.SetOutput() with command.SetOut()
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-05-07 14:25:59 +02:00
Sebastiaan van Stijn b39739123b
cli: perform feature detection lazily
- Perform feature detection when actually needed, instead of during
  initializing
- Version negotiation is performed either when making an API request,
  or when (e.g.) running `docker help` (to hide unsupported features)
- Use a 2 second timeout when 'pinging' the daemon; this should be
  sufficient for most cases, and when feature detection failed, the
  daemon will still perform validation (and produce an error if needed)
- context.WithTimeout doesn't currently work with ssh connections (connhelper),
  so we're only applying this timeout for tcp:// connections, otherwise
  keep the old behavior.

Before this change:

    time sh -c 'DOCKER_HOST=tcp://42.42.42.41:4242 docker help &> /dev/null'
    real   0m32.919s
    user   0m0.370s
    sys    0m0.227s

    time sh -c 'DOCKER_HOST=tcp://42.42.42.41:4242 docker context ls &> /dev/null'
    real   0m32.072s
    user   0m0.029s
    sys    0m0.023s

After this change:

    time sh -c 'DOCKER_HOST=tcp://42.42.42.41:4242 docker help &> /dev/null'
    real   0m 2.28s
    user   0m 0.03s
    sys    0m 0.03s

    time sh -c 'DOCKER_HOST=tcp://42.42.42.41:4242 docker context ls &> /dev/null'
    real   0m 0.13s
    user   0m 0.02s
    sys    0m 0.02s

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-04-10 16:33:21 +02:00
Sebastiaan van Stijn 2c0e93063b
bump gotest.tools v3.0.1 for compatibility with Go 1.14
full diff: https://github.com/gotestyourself/gotest.tools/compare/v2.3.0...v3.0.1

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-02-23 00:28:55 +01:00
Ian Campbell 7d0645c5fe Use command.Cli instead of command.DockerCli
The linter is complaining:

    cmd/docker/docker.go:72:23⚠️ dockerCli can be github.com/docker/cli/cli/command.Cli (interfacer)

Unclear precisely which change in the preceeding commits caused it to notice
this possibility.

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-04-26 15:43:03 +01:00
Ian Campbell 40a6cf7c47 Include CLI plugins in help output on unknown flag.
Previously `docker --badopt` output would not include CLI plugins.

Fixes #1813

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-04-26 15:21:20 +01:00
Ian Campbell 79a75da0fd Hide experimental builtin commands in help output on unknown flag.
Previously `docker --badopt` would always include experimental commands even if
experimental was not enabled.

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-04-26 15:09:09 +01:00
Tibor Vass 1ed02c40fe cli-plugins: alias an existing allowed command (only builder for now)
With this patch it is possible to alias an existing allowed command.
At the moment only builder allows an alias.

This also properly puts the build command under builder, instead of image
where it was for historical reasons.

Signed-off-by: Tibor Vass <tibor@docker.com>
2019-04-19 01:26:45 +00:00
Ian Campbell 8c087b6a1e cmd/docker: only parse global arguments once.
Before calling `cmd.Execute` we need to reset the arguments to be used to not
include the global arguments which we have already parsed. This is precisely
the `args` which we have in our hand at this point.

This fixes `TestGlobalArgsOnlyParsedOnce/builtin` in the cli-plugins e2e tests.

`TestGlobalArgsOnlyParsedOnce/plugin` is still broken will be fixed next.

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-04-03 15:07:38 +01:00
Simon Ferquel 3beb60a96e Fix error message on experimental flags
Signed-off-by: Simon Ferquel <simon.ferquel@docker.com>
2019-03-21 16:21:03 +01:00
Ian Campbell d4ced2ef77 allow plugins to have argument which match a top-level flag.
The issue with plugin options clashing with globals is that when cobra is
parsing the command line and it comes across an argument which doesn't start
with a `-` it (in the absence of plugins) distinguishes between "argument to
current command" and "new subcommand" based on the list of registered sub
commands.

Plugins breaks that model. When presented with `docker -D plugin -c foo` cobra
parses up to the `plugin`, sees it isn't a registered sub-command of the
top-level docker (because it isn't, it's a plugin) so it accumulates it as an
argument to the top-level `docker` command. Then it sees the `-c`, and thinks
it is the global `-c` (for AKA `--context`) option and tries to treat it as
that, which fails.

In the specific case of the top-level `docker` subcommand we know that it has
no arguments which aren't `--flags` (or `-f` short flags) and so anything which
doesn't start with a `-` must either be a (known) subcommand or an attempt to
execute a plugin.

We could simply scan for and register all installed plugins at start of day, so
that cobra can do the right thing, but we want to avoid that since it would
involve executing each plugin to fetch the metadata, even if the command wasn't
going to end up hitting a plugin.

Instead we can parse the initial set of global arguments separately before
hitting the main cobra `Execute` path, which works here exactly because we know
that the top-level has no non-flag arguments.

One slight wrinkle is that the top-level `PersistentPreRunE` is no longer
called on the plugins path (since it no longer goes via `Execute`), so we
arrange for the initialisation done there (which has to be done after global
flags are parsed to handle e.g. `--config`) to happen explictly after the
global flags are parsed. Rather than make `newDockerCommand` return the
complicated set of results needed to make this happen, instead return a closure
which achieves this.

The new functionality is introduced via a common `TopLevelCommand` abstraction
which lets us adjust the plugin entrypoint to use the same strategy for parsing
the global arguments. This isn't strictly required (in this case the stuff in
cobra's `Execute` works fine) but doing it this way avoids the possibility of
subtle differences in behaviour.

Fixes #1699, and also, as a side-effect, the first item in #1661.

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-03-13 11:28:17 +00:00
Corey Quon d871451049
Fix issue where plugin command error exit code is printed out
Signed-off-by: Corey Quon <corey.quon@docker.com>
2019-02-26 09:39:07 -08:00
Silvin Lubecki 2e5639da02
Merge pull request #1564 from ijc/plugins
Basic framework for writing and running CLI plugins
2019-01-31 17:44:46 +01:00
Ian Campbell baabf6e8ad Ensure that plugins are only listed once in help outputs.
They were listed twice in `docker --help` (but not `docker help`), since the
stubs were added in both `tryRunPluginHelp` and the `setHelpFunc` closure.

Calling `AddPluginStubCommands` earlier in `setHelpFunc` before the call to
`tryRunPluginHelp` is sufficient. Also it is no longer necessary to add just
valid plugins (`tryRunPluginHelp` handles invalid plugins correctly) so remove
that logic (which was in any case broken for e.g. `docker --help`).

Update the e2e test to check for duplicate entries and also to test `docker
--help` which was previously missed.

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-01-30 13:55:42 +00:00
Ian Campbell 935d47bbe9 Ignore unknown arguments on the top-level command.
This allows passing argument to plugins, otherwise they are caught by the parse
loop, since cobra does not know about each plugin at this stage (to avoid
having to always scan for all plugins) this means that e.g. `docker plugin
--foo` would accumulate `plugin` as an arg to the `docker` command, then choke
on the unknown `--foo`.

This allows unknown global args only, unknown arguments on subcommands (e.g.
`docker ps --foo`) are still correctly caught.

Add an e2e test covering this case.

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-01-30 13:45:26 +00:00
Ian Campbell 53f018120a Integrate CLI plugins with `docker «plugin» --help`.
To achieve this we hook in at the beginning of our custom `HelpFunc` and detect
the plugin case by adding stub commands.

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-01-30 13:45:25 +00:00
Ian Campbell 20a284721c Integrate CLI plugins with `docker help «foo»`
Signed-off-by: Ian Campbell <ijc@docker.com>
2019-01-30 13:45:18 +00:00