Commit Graph

6204 Commits

Author SHA1 Message Date
Ian Campbell 2c624e8984 Add e2e test for handling of `version`/`-v`/`--version` with plugins
Previous commits fixed the first issue on #1661, this simply adds a test for
it. Note that this is testing the current behaviour, without regard for the
second issue in #1661 which proposes a different behaviour.

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-03-13 11:28:17 +00: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
Ian Campbell 8289ae03f8 Add e2e tests for plugin vs global argument issues
These won't pass right now due to https://github.com/docker/cli/issues/1699
("Plugins can't re-use the same flags as cli global flags") and the change in
935d47bbe9 ("Ignore unknown arguments on the top-level command."), but the
intention is to fix them now.

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-03-13 10:54:31 +00:00
Ian Campbell 4e5f0af9cd e2e: start a new file for cli plugins arguments tests
`TestRunGoodArgument` fits here.

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-03-13 10:54:31 +00:00
Sebastiaan van Stijn d6a230606c
Merge pull request #1718 from ijc/dial-stdio-npipe-on-windows
dial-stdio: handle connections which lack CloseRead method.
2019-03-13 11:49:25 +01:00
Stephane Jeandeaux 05674a5096 [pretty print] pretty print and healthcheck
fixes #117

Print healthcheck information in pretty mode.

Signed-off-by: Stephane Jeandeaux <stephane.jeandeaux@gmail.com>
2019-03-12 22:00:46 -04:00
Tibor Vass 81ac432cc2
Merge pull request #1700 from thaJeztah/update_engine
Update docker/docker 8aca18d, containerd v1.2.4
2019-03-12 10:41:51 -07:00
Ian Campbell 0449ad8d06 Revert "Disable `docker system dial-stdio` on Windows"
This reverts commit c41c23813c.

This case is now handled due to the previous commit.

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-03-12 14:57:52 +00:00
Ian Campbell 186e7456ac dial-stdio: Close the connection
This was leaking the fd.

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-03-12 14:57:40 +00:00
Ian Campbell 8919bbf04d dial-stdio: handle connections which lack CloseRead method.
This happens on Windows when dialing a named pipe (a path which is used by CLI
plugins), in that case some debugging shows:

    DEBU[0000] conn is a *winio.win32MessageBytePipe
    DEBU[0000] conn is a halfReadCloser: false
    DEBU[0000] conn is a halfWriteCloser: true
    the raw stream connection does not implement halfCloser
In such cases we can simply wrap with a nop function since closing for read
isn't too critical.

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-03-12 14:52:42 +00:00
Sebastiaan van Stijn bf4a96e564
Merge pull request #1688 from luoyunpeng/optimize-blockIOTypecheck
use char to check blockIO type
2019-03-12 10:32:31 +01:00
Ian Campbell e1a7b56308 Allow control over dirs passed to e2e and unit tests
Allows e.g.:

    $ make -f docker.Makefile TESTDIRS=./cli/command/trust/... test-unit

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-03-11 16:02:55 +00:00
Ian Campbell 023559b98c Allow flags to be passed to e2e and unit tests
This allows e.g.

    $ make -f docker.Makefile fmt test-e2e-non-experimental TESTFLAGS="-test.run TestRunGoodArgument|TestRunGood"

As well as adding the var to `$(ENVVARS)` we also need to use that when
invocking the e2e image, the existing `$(DOCKER_RUN)` is not used here because
the bind mounts differ. The other variables included in `$(ENVVARS)` are
harmless when running the e2e tests.

The `${TESTFLAGS}` envvar is already understood by `scripts/test/e2e/run`.

Note that since this modifies `$(ENVVARS)` this is now also available to the unit
test target too, so add the use to the invocation so it takes effect.

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-03-11 15:59:27 +00:00
Silvin Lubecki 6d2d597a6d
Merge pull request #1728 from ijc/e2e-handle-alpine-bump
Fixes for e2e testing after Alpine bump
2019-03-11 15:59:17 +01:00
Ian Campbell 0b0c57871a e2e: avoid `usermod -p` by using `useradd`'s `--password` option
Signed-off-by: Ian Campbell <ijc@docker.com>
2019-03-11 14:25:41 +00:00
Ian Campbell e854a9cf96 e2e: Expand `useradd`'s `-m` otion into `--create-home`
... for improved readability

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-03-11 14:25:41 +00:00
Ian Campbell 5de2d9e8a9 e2e Use `useradd`'s `--shell` option
... in preference to `chsh`, since in recent alpine 3.9.2 images that can fail
with:

    Password: chsh: PAM: Authentication token manipulation error

Which seems to relate to the use of `!` as the password for `root` in `/etc/shadow`gq

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-03-11 14:25:19 +00:00
Silvin Lubecki b86bff84b6
Merge pull request #1710 from ijc/no-dial-stdio-on-windows
Disable `docker system dial-stdio` on Windows
2019-03-11 15:14:23 +01:00
Elliot Luo 0bb397f9ef use char to check blockIO type
Signed-off-by: Elliot Luo <956941328@qq.com>
2019-03-11 10:01:22 +08:00
Silvin Lubecki fdb0ef7be0
Merge pull request #1720 from zappy-shu/harden-config-path
hardening config.Path() to disallow directory traversal
2019-03-10 21:04:08 +01:00
Harald Albers 24ca6cc68c Add bash completion for `builder prune`
Signed-off-by: Harald Albers <github@albersweb.de>
2019-03-09 16:04:08 +01:00
Harald Albers 9c8dec9f0b Add bash completion for `network ls --filter dangling`
Signed-off-by: Harald Albers <github@albersweb.de>
2019-03-09 14:41:41 +01:00
Vincent Demeester 89dd14d665
Merge pull request #1693 from thaJeztah/upgrade_shellcheck_0.6.0
Upgrade shellcheck 0.6.0
2019-03-08 20:58:24 +01:00
Nick Adcock ff51b0d77d harden config.Path() to disallow directory traversal
Signed-off-by: Nick Adcock <nick.adcock@docker.com>
2019-03-07 14:40:53 +00:00
Vincent Demeester 39c327ab93
Merge pull request #1717 from ry4nz/export-config
Export cli/command/config
2019-03-07 08:04:33 +01:00
Akihiro Suda 62a15c16fc commandconn: set SysProcAttr.Setsid
Setting `Setsid` is needed for SSH connection helper with `ProxyCommand`
config, so as to detach TTY.

e.g.

  $ cat ~/.ssh/config
  Host foo
    Hostname foo
    ProxyCommand ssh -W %h:%p bastion
  $ DOCKER_HOST=ssh://foo docker run -it --rm alpine
  / #

Fix #1707

Signed-off-by: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>
2019-03-06 13:22:54 +09:00
Ryan Zhang f60369dfe6 Export cli/command/config
Signed-off-by: Ryan Zhang <ryan.zhang@docker.com>
2019-03-05 14:26:42 -08:00
Sebastiaan van Stijn 79e1cabf17
Merge pull request #1680 from cquon/plugin_error_handling
Fix issue where plugin command error exit code is printed out
2019-03-05 09:22:47 +01:00
Vincent Demeester 8f68971ede
Merge pull request #1701 from ijc/plugin-build-fixes
Fix cli-plugins build.
2019-03-05 09:10:08 +01:00
Sebastiaan van Stijn d4877fb225
Fix test for Go 1.12.x
After switching to Go 1.12, the format-string causes an error;

```
=== Errors
cli/config/config_test.go:154:3: Fatalf format %q has arg config of wrong type *github.com/docker/cli/cli/config/configfile.ConfigFile
cli/config/config_test.go:217:3: Fatalf format %q has arg config of wrong type *github.com/docker/cli/cli/config/configfile.ConfigFile
cli/config/config_test.go:253:3: Fatalf format %q has arg config of wrong type *github.com/docker/cli/cli/config/configfile.ConfigFile
cli/config/config_test.go:288:3: Fatalf format %q has arg config of wrong type *github.com/docker/cli/cli/config/configfile.ConfigFile
cli/config/config_test.go:435:3: Fatalf format %q has arg config of wrong type *github.com/docker/cli/cli/config/configfile.ConfigFile
cli/config/config_test.go:448:3: Fatalf format %q has arg config of wrong type *github.com/docker/cli/cli/config/configfile.ConfigFile

DONE 1115 tests, 2 skipped, 6 errors in 215.984s
make: *** [Makefile:22: test-coverage] Error 2
Exited with code 2
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2019-03-04 20:19:14 +01:00
Sebastiaan van Stijn 6c4fbb7738
Fix: plugin-tests discarding current environment
By default, exec uses the environment of the current process, however,
if `exec.Env` is not `nil`, the environment is discarded:

e73f489494/src/os/exec/exec.go (L57-L60)

> If Env is nil, the new process uses the current process's environment.

When adding a new environment variable, prepend the current environment,
to make sure it is not discarded.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2019-03-04 20:18:24 +01:00
Ian Campbell c41c23813c Disable `docker system dial-stdio` on Windows
The `conn` here is `*winio.win32MessageBytePipe` which does not have a
`CloseRead` method (it does have `CloseWrite`) resulting in:

    docker@WIN-NUC0 C:\Users\docker>.\docker-windows-amd64.exe system dial-stdio
    the raw stream connection does not implement halfCloser

Also disable the path which uses this for cli-plugins on Windows.

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-03-04 17:36:21 +00:00
Ian Campbell 38480d9a96 Use `$(DOCKER_RUN)` for cli plugins build.
Seems I rebased over b039db985a ("Make it possible to override the volume
mounts and shell for the dev container") at some point and failed to notice
that some of the variable names had changed.

In the meantime the underlying issue was fixed in #1698 but here we switch to
using `$(DOCKER_RUN)`. This means that these rules now use
`$(DOCKER_RUN_NAME_OPTION)` and thus obey the `$(DOCKER_CLI_CONTAINER_NAME)`
variable.

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-03-04 10:17:58 +00:00
Sebastiaan van Stijn 8ddde26af6
Merge pull request #1702 from AkihiroSuda/connhelper-export
connhelper: export functions for other projects
2019-03-03 11:40:10 +01:00
Sebastiaan van Stijn 05fd2a87dc
Update containerd 1.2.4 and dependencies
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2019-03-02 13:44:57 +01:00
Sebastiaan van Stijn 087a7ee712
bump github.com/gorilla/mux v1.7.0
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2019-03-02 13:42:07 +01:00
Sebastiaan van Stijn 0fc0015173
bump docker/docker to 8aca18d631f3f72d4c6e3dc01b6e5d468ad941b8
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2019-03-02 13:41:42 +01:00
Akihiro Suda dbe7afbd04 connhelper: export functions for other projects
Exposed functions are planned to be used by `buildctl`:
https://github.com/moby/buildkit/issues/769

Signed-off-by: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>
2019-03-02 21:11:49 +09:00
Sebastiaan van Stijn ee94f72e2c
Merge pull request #1698 from silvin-lubecki/fix-cross-dev-mounts
Fix unknown $(MOUNTS) variable in makefile plugins target
2019-03-01 18:48:25 +01:00
Silvin Lubecki 2c6b2ccbdd Fix unknown $(MOUNTS) variable in makefile plugins target
Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
2019-02-28 11:56:13 +01: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
Sebastiaan van Stijn 2178fea84d
Merge pull request #1665 from jcrben/patch-1
Note caveat with detaching using key sequence
2019-02-26 16:56:31 +01:00
Sebastiaan van Stijn 5aeb7a0f55
Remove some bashisms
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2019-02-26 15:31:12 +01:00
Sebastiaan van Stijn ff107b313a
Update to shellcheck v0.6.0
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2019-02-26 15:12:52 +01:00
Sebastiaan van Stijn 388646eab0
Use official shellcheck image
This patch switches the shellcheck image to use the official image
from Docker Hub.

Note that this does not yet update shellcheck to the latest version (v0.5.x);
Shellcheck v0.4.7 added some new checks, which makes CI currently fail, so will
be done in a follow-up PR. Instead, the v0.4.6 version is used in this PR, which
is closest to the same version as was installed in the image before this change;

```
docker run --rm docker-cli-shell-validate shellcheck --version
ShellCheck - shell script analysis tool
version: 0.4.4
license: GNU General Public License, version 3
website: http://www.shellcheck.net
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2019-02-26 15:10:38 +01:00
Sebastiaan van Stijn 69311b5ad9
Merge pull request #1691 from justincormack/nolibtool
Update PKCS11 library
2019-02-26 14:52:52 +01:00
Silvin Lubecki 9b837be8e2
Merge pull request #1689 from ijc/plugins-docker-system-info-format
Reformat the output of CLI plugins in `docker system info`
2019-02-26 14:51:39 +01:00
Silvin Lubecki 3174ca0e69
Merge pull request #1684 from thaJeztah/plugin_subcommand_detection
Show plugins as Management commands
2019-02-26 14:22:18 +01:00
Justin Cormack cb3e55bf58
Update PKCS11 library
The new version no longer links to libltdl which simplifies build
and dependencies.

See https://github.com/theupdateframework/notary/pull/1434

Signed-off-by: Justin Cormack <justin.cormack@docker.com>
2019-02-26 11:45:19 +00:00
Sebastiaan van Stijn f8c5f5d9b8
Show plugins as Management commands
Plugins are expected to be management commands ("docker <object> <verb>").

This patch modified the usage output to shown plugins in the "Management commands"
section.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2019-02-26 00:28:41 +01:00