Commit Graph

132 Commits

Author SHA1 Message Date
Sebastiaan van Stijn 98654202c2
linting: G112: Potential Slowloris Attack
Picking 2 seconds, although that's just a randomly picked timeout;
given that this is only for testing, it's not too important.

    e2e/plugin/basic/basic.go:25:12: G112: Potential Slowloris Attack because ReadHeaderTimeout is not configured in the http.Server (gosec)
        server := http.Server{
            Addr:    l.Addr().String(),
            Handler: http.NewServeMux(),
        }

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-09-03 21:25:43 +02:00
Sebastiaan van Stijn 82427d1a07
format (GoDoc) comments with Go 1.19 to prepare for go updates
Older versions of Go do not format these comments, so we can already
reformat them ahead of time to prevent gofmt linting failing once
we update to Go 1.19 or up.

Result of:

    gofmt -s -w $(find . -type f -name '*.go' | grep -v "/vendor/")

With some manual adjusting.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-07-19 19:10:16 +02:00
Sebastiaan van Stijn 2d88c896bc
cli: print full command as aliases in usage output
The default output for Cobra aliases only shows the subcommand as alias, which
is not very intuitive. This patch changes the output to print the full command
as it would be called by the user.

Note that there's still some improvements to be made; due to how aliases must be
set-up in Cobra, aliases at different "levels" are still not shown. So for example,
`docker ps --help` will not show `docker container ps` as alias, and vice-versa.
This will require additional changes, and can possibly be resolved using custom
metadata/annotations.

Before this patch:

    docker container ls --help

    Usage:  docker container ls [OPTIONS]

    List containers

    Aliases:
      ls, ps, list

After this patch:

    docker container ls --help

    Usage:  docker container ls [OPTIONS]

    List containers

    Aliases:
      docker container ls, docker container ps, docker container list

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-06-28 11:03:30 +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
Stoica-Marcu Floris-Andrei dfc214115b
Add stack config command
Make use of existing modules and functions in order to output the merged configs.
Added skip interpolation flag of variables, so that you can pipe the output back to stack deploy without much hassle.

Signed-off-by: Stoica-Marcu Floris-Andrei <floris.sm@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-04-08 14:56:01 +02:00
Sebastiaan van Stijn bc2b48aaf2
e2e: cleanup TestGlobalHelp() to be less brittle
- remove check for "A self-sufficient runtime for containers"; really
  not important to check for.
- don't make the checks positional (just match that we find them, and
  that we don't find them multiple times)
- account for leading whitespace to change instead of hard-coding the
  number of spaces before output.
- change the badopt check; I think it should be sufficient to check
  that the bad option was printed and that "run --help" output is
  printed.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-03-27 10:42:48 +02:00
Sebastiaan van Stijn e89af84ffc
e2e: remove deprecated io/ioutil and use t.TempDir()
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-02-25 15:42:21 +01:00
Sebastiaan van Stijn 242857dd81
update/remove various tests and options related to kubernetes support
Remove various tests and utilities related to testing kubernetes support

Also removing the Kubernetes and DefaultStackOrchestrator from CreateOptions
and UpdateOptions, instead updating the flags to not be bound to a variable.

This might break some consumers of those options, but given that they've become
non-functional, that's probably ok (otherwise they may ignore the deprecation
warning and end up with non-functional code).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-02-24 17:53:18 +01:00
Nicolas De Loof 193ede9b12
remove obsolete mutli-orchestrator support
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
2022-02-22 15:28:12 +01:00
Nicolas De Loof 7b9580df51 Drop support for (archived) Compose-on-Kubernetes
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
2022-02-22 13:47:34 +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
Sebastiaan van Stijn 40c6b117e7
change TestNewAPIClientFromFlagsWithHttpProxyEnv to an e2e test
Golang uses a `sync.Once` when determining the proxy to use. This means
that it's not possible to test the proxy configuration in unit tests,
because the proxy configuration will be "fixated" the first time Golang
detects the proxy configuration.

This patch changes TestNewAPIClientFromFlagsWithHttpProxyEnv to an e2e
test so that we can verify the CLI picks up the proxy configuration.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2021-07-28 22:33:12 +02:00
Sebastiaan van Stijn f3b6ed744f
e2e: make sure that os.environ is preserved
We updated some of these functions to make sure os.environ was
preserved, but some where not.

This adds a utility to help with this, which also prevents the
os.environ to be added multiple times.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2021-06-11 17:46:30 +02:00
Chris Crone b43b852031
context: Add tarball e2e tests
Signed-off-by: Chris Crone <christopher.crone@docker.com>
(cherry picked from commit 18f33b337d)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2021-02-02 13:51:17 +01:00
Sebastiaan van Stijn 7f3717bd2a
Replace tab with spaces in usage output
All output of the usage / --help output uses spaces, and having a tab
in the output can be somewhat cumbersome (e.g. our YAML docs generator
doesn't like them, and copy/pasing the output in iTerm produces a warning).

This patch changes the output to use two spaces instead.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-10-02 15:41:17 +02:00
Sebastiaan van Stijn 0a0037c6fd
builder: rephrase ENV section, remove examples for ENV key value without '='
The `ENV key value` form can be ambiguous, for example, the following defines
a single env-variable (`ONE`) with value `"TWO= THREE=world"`:

    ENV ONE TWO= THREE=world

While we cannot deprecate/remove that syntax (as it would break existing
Dockerfiles), we should reduce exposure of the format in our examples.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-09-23 13:21:20 +02:00
Sebastiaan van Stijn dace8fdc75
formatter: reduce minimum width for columns in table-view
The tabwriter was configured to have a min-width for columns of 20 positions.
This seemed quite wide, and caused smaller columns to be printed with a large
gap between.

Before:

    docker container stats

    CONTAINER ID        NAME                CPU %               MEM USAGE / LIMIT     MEM %               NET I/O             BLOCK I/O           PIDS
    29184b3ae391        amazing_shirley     0.00%               800KiB / 1.944GiB     0.04%               1.44kB / 0B         0B / 0B             1
    403c101bad56        agitated_swartz     0.15%               34.31MiB / 1.944GiB   1.72%               10.2MB / 206kB      0B / 0B             51
    0dc4b7f6c6be        container2          0.00%               1.012MiB / 1.944GiB   0.05%               12.9kB / 0B         0B / 0B             5
    2d99abcc6f62        container99         0.00%               972KiB / 1.944GiB     0.05%               13kB / 0B           0B / 0B             5
    9f9aa90173ac        foo                 0.00%               820KiB / 1.944GiB     0.04%               13kB / 0B           0B / 0B             5

    docker container ls

    CONTAINER ID        IMAGE               COMMAND                  CREATED             STATUS              PORTS               NAMES
    29184b3ae391        docker-cli-dev      "ash"                    4 hours ago         Up 4 hours                              amazing_shirley
    403c101bad56        docker-dev:master   "hack/dind bash"         3 days ago          Up 3 days                               agitated_swartz
    0dc4b7f6c6be        nginx:alpine        "/docker-entrypoint.…"   4 days ago          Up 4 days           80/tcp              container2
    2d99abcc6f62        nginx:alpine        "/docker-entrypoint.…"   4 days ago          Up 4 days           80/tcp              container99
    9f9aa90173ac        nginx:alpine        "/docker-entrypoint.…"   4 days ago          Up 4 days           80/tcp              foo

    docker image ls

    REPOSITORY          TAG                    IMAGE ID            CREATED             SIZE
    docker-cli-dev      latest                 5f603caa04aa        4 hours ago         610MB
    docker-cli-native   latest                 9dd29f8d387b        4 hours ago         519MB
    docker-dev          master                 8132bf7a199e        3 days ago          2.02GB
    docker-dev          improve-build-errors   69e208994b3f        11 days ago         2.01GB
    docker-dev          refactor-idtools       69e208994b3f        11 days ago         2.01GB

After:

    docker container stats

    CONTAINER ID   NAME              CPU %     MEM USAGE / LIMIT     MEM %     NET I/O          BLOCK I/O   PIDS
    29184b3ae391   amazing_shirley   0.14%     5.703MiB / 1.944GiB   0.29%     1.44kB / 0B      0B / 0B     10
    403c101bad56   agitated_swartz   0.15%     56.97MiB / 1.944GiB   2.86%     10.2MB / 206kB   0B / 0B     51
    0dc4b7f6c6be   container2        0.00%     1016KiB / 1.944GiB    0.05%     12.9kB / 0B      0B / 0B     5
    2d99abcc6f62   container99       0.00%     956KiB / 1.944GiB     0.05%     13kB / 0B        0B / 0B     5
    9f9aa90173ac   foo               0.00%     980KiB / 1.944GiB     0.05%     13kB / 0B        0B / 0B     5

    docker container ls

    CONTAINER ID   IMAGE               COMMAND                  CREATED          STATUS          PORTS     NAMES
    29184b3ae391   docker-cli-dev      "ash"                    12 minutes ago   Up 12 minutes             amazing_shirley
    403c101bad56   docker-dev:master   "hack/dind bash"         3 days ago       Up 3 days                 agitated_swartz
    0dc4b7f6c6be   nginx:alpine        "/docker-entrypoint.…"   4 days ago       Up 4 days       80/tcp    container2
    2d99abcc6f62   nginx:alpine        "/docker-entrypoint.…"   4 days ago       Up 4 days       80/tcp    container99
    9f9aa90173ac   nginx:alpine        "/docker-entrypoint.…"   4 days ago       Up 4 days       80/tcp    foo

    docker image ls

    REPOSITORY          TAG                    IMAGE ID       CREATED         SIZE
    docker-cli-dev      latest                 5f603caa04aa   4 hours ago     610MB
    docker-cli-native   latest                 9dd29f8d387b   4 hours ago     519MB
    docker-dev          master                 8132bf7a199e   3 days ago      2.02GB
    docker-dev          improve-build-errors   69e208994b3f   11 days ago     2.01GB
    docker-dev          refactor-idtools       69e208994b3f   11 days ago     2.01GB

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-08-31 16:31:15 +02:00
Sebastiaan van Stijn 3b256db4a3
Add script to regenerate test-certificate fixtures
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-04-10 16:43:06 +02:00
Sebastiaan van Stijn 6bd09229a5
CI: update notary test certificates
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-04-09 17:09:12 +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
Sebastiaan van Stijn 8ef8547eb6
Merge pull request #2024 from rgulewich/1988-run-cgroupns-mode
docker run: specify cgroup namespace mode with --cgroupns
2020-02-11 11:16:05 +01:00
Rob Gulewich 5ad1d4d4c8 docker run: specify cgroup namespace mode with --cgroupns
Signed-off-by: Rob Gulewich <rgulewich@netflix.com>
2020-01-29 22:50:37 +00:00
Sebastiaan van Stijn 9e620e990f
implement docker push -a/--all-tags
The `docker push` command up until [v0.9.1](https://github.com/moby/moby/blob/v0.9.1/api/client.go#L998)
always pushed all tags of a given image, so `docker push foo/bar` would push (e.g.)
all of  `foo/bar:latest`, `foo:/bar:v1`, `foo/bar:v1.0.0`.

Pushing all tags of an image was not desirable in many case, so docker v0.10.0
enhanced `docker push` to optionally specify a tag to push (`docker push foo/bar:v1`)
(see https://github.com/moby/moby/issues/3411 and the pull request that implemented
this: https://github.com/moby/moby/pull/4948).

This behavior exists up until today, and is confusing, because unlike other commands,
`docker push` does not default to use the `:latest` tag when omitted, but instead
makes it push "all tags of the image"

For example, in the following situation;

```
docker images

REPOSITORY          TAG                        IMAGE ID            CREATED             SIZE
thajeztah/myimage   latest                     b534869c81f0        41 hours ago        1.22MB
```

Running `docker push thajeztah/myimage` seemingly does the expected behavior (it
pushes `thajeztah/myimage:latest` to Docker Hub), however, it does not so for the
reason expected (`:latest` being the default tag), but because `:latest` happens
to be the only tag present for the `thajeztah/myimage` image.

If another tag exists for the image:

```
docker images

REPOSITORY          TAG                        IMAGE ID            CREATED             SIZE
thajeztah/myimage   latest                     b534869c81f0        41 hours ago        1.22MB
thajeztah/myimage   v1.0.0                     b534869c81f0        41 hours ago        1.22MB
```

Running the same command (`docker push thajeztah/myimage`) will push _both_ images
to Docker Hub.

> Note that the behavior described above is currently not (clearly) documented;
> the `docker push` reference documentation (https://docs.docker.com/engine/reference/commandline/push/)
does not mention that omitting the tag will push all tags

This patch changes the default behavior, and if no tag is specified, `:latest` is
assumed. To push _all_ tags, a new flag (`-a` / `--all-tags`) is added, similar
to the flag that's present on `docker pull`.

With this change:

- `docker push myname/myimage` will be the equivalent of `docker push myname/myimage:latest`
- to push all images, the user needs to set a flag (`--all-tags`), so `docker push --all-tags myname/myimage:latest`

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-01-28 16:21:06 +01:00
Sebastiaan van Stijn 94443920b1
Fix: docker push --quiet suppressing errors and exit code
Before this patch:

    docker push --quiet nosuchimage
    docker.io/library/nosuchimage

    echo $?
    0

With this patch applied:

    docker push --quiet nosuchimage:latest
    An image does not exist locally with the tag: nosuchimage

    echo $?
    1

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-01-24 13:53:24 +01:00
glefloch 348f24cae6
Add content trust tests for run command
Signed-off-by: Guillaume Le Floch <glfloch@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-01-20 14:18:52 +01:00
Sebastiaan van Stijn 6e5528b650
e2e: fix formatting of comments
Comments should have a leading space unless the comment is
for special purposes (go:generate, nolint:)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-01-16 12:46:07 +01:00
Sebastiaan van Stijn 9ef0c7a9dd
Merge pull request #2196 from tiborvass/test-tlsverify
e2e: add new test package "global" with TestTLSVerify
2019-12-12 15:15:15 +01:00
Sebastiaan van Stijn 99ad13e374
Remove experimental "deploy" from "dab" files
The top-level `docker deploy` command (using the "Docker Application Bundle"
(`.dab`) file format was introduced as an experimental feature in Docker 1.13 /
17.03, but superseded by support for Docker Compose files.

With no development being done on this feature, and no active use of the file
format, support for the DAB file format and the top-level `docker deploy` command
(hidden by default in 19.03), is removed in this patch, in favour of `docker stack deploy`
using compose files.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2019-12-09 10:34:14 +01:00
Tibor Vass e692381d18 e2e: add new test package "global" with TestTLSVerify
Signed-off-by: Tibor Vass <tibor@docker.com>
2019-11-12 18:07:09 +00:00
Sebastiaan van Stijn dd4d216afd
e2e: remove unnecessary trailing newline (whitespace)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2019-10-31 19:22:37 +01:00
Sebastiaan van Stijn 1736662bea
e2e/cli-plugins: Using the variable on range scope `args` in function literal (scopelint)
```
e2e/cli-plugins/flags_test.go:135:27: Using the variable on range scope `args` in function literal (scopelint)
			res := icmd.RunCmd(run(args...))
			                       ^
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2019-10-31 19:22:34 +01:00
Sebastiaan van Stijn 6205ef33be
e2e/container: containerExistsWithStatus - t is unused (unparam)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2019-10-31 19:22:26 +01:00
Silvin Lubecki f123e43c1f
Disable unparam linter: e2e/image/push_test.go:299:27: `withNotaryPassphrase` - `pwd` always receives `"foo"` (unparam)
Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
2019-10-31 19:22:23 +01:00
Kir Kolyshkin bc4ed69a23 TestSigProxyWithTTY: fix
exec.CombinedOutput should not be used here because:
 - it redirects cmd Stdout and Stderr and we want it to be the tty
 - it calls cmd.Run which we already did

While at it
 - use pty.Start() as it is cleaner
 - make sure we don't leave a zombie running, by calling Wait() in defer
 - use test.Name() for containerName

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2019-07-30 17:40:31 -07:00
Sebastiaan van Stijn f290a80846
switch kr/pty to creack/pty v1.1.7
kr/pty was moved to creak/pty and the old location was
archived.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2019-07-29 16:45:41 -07:00
Sebastiaan van Stijn b1a3c1aad1
Disable TLS for e2e docker-in-docker daemon
The docker-in-docker image now enables TLS by default (added in
docker-library/docker#166), which complicates testing in our
environment, and isn't needed for the tests we're running.

This patch sets the `DOCKER_TLS_CERTDIR` to an empty value to
disable TLS.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2019-07-29 14:50:23 -07:00
Sebastiaan van Stijn 08fd6dd63c
e2e: use stable-dind image for testing
The edge channel is deprecated and no longer updated

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2019-07-10 16:41:56 +02:00
Sebastiaan van Stijn 7cf1a8d4c9
Add test for --sig-proxy with a TTY
Add a test to verify that killing the docker CLI forwards
the signal to the container. Test-case for moby/moby 28872

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2019-06-25 16:29:24 +02:00
Ian Campbell d5d693aa6e e2e: add a test for `context ls`
I'm about to refactor the code which includes the Kubernetes support in a way
which relies on something vendoring `./cli/context/kubernetes/` in order to
trigger the inclusion of support for the Kubernetes endpoint in the final
binary.

In practice anything which is interested in Kubernetes must import that package
(e.g. `./cli/command/context.list` does for the `EndpointFromContext`
function). However if it was somehow possible to build without that import then
the `KUBERNETES ENDPOINT` column would be mysteriously empty.

Out of an abundance of caution add a specific check on the final binary.

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-05-20 13:28:11 +01:00
Sebastiaan van Stijn 11d2e404c6
Merge pull request #1853 from ijc/cli-plugin-persistentprerun-hook
cli-plugins: fix when plugin does not use PersistentPreRun* hooks
2019-05-13 07:06:42 -07:00
Ian Campbell af200f14ed cli-plugins: fix when plugin does not use PersistentPreRun* hooks
This regressed in 3af168c7df ("Ensure plugins can use `PersistentPreRunE`
again.") but this wasn't noticed because the helloworld test plugin has it's
own `PersistentPreRunE` which has the effect of deferring the resolution of the
global variable. In the case where the hook isn't used the variable is resolved
during `newPluginCommand` which is before the global variable was set.

Initialize the plugin command with a stub function wrapping the call to the
(global) hook, this defers resolving the variable until after it has been set,
otherwise the initial value (`nil`) is used in the struct.

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-04-30 10:45:29 +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 d57175aa2e Move subtests of TestGlobalHelp into actual subtests
Signed-off-by: Ian Campbell <ijc@docker.com>
2019-04-26 10:43:03 +01:00
Ian Campbell d32f3647b1 e2e: Add tests for #1801 (global args parsed multiple times)
These currently fail.

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-04-03 15:07:38 +01:00
Ian Campbell ef40743669 e2e: Add a TestMain for cli plugins tests
This was omitted when these tests were added.

Adding this means that the tests now see the `$DOCKER_HOST` configured (via
`$TEST_DOCKER_HOST`) where they didn't before. In some cases (specifically the
`test-e2e-connhelper-ssh` case) this results in additional output on stderr
when `-D` (debug) is used:

    time="2019-04-03T11:10:27Z" level=debug msg="commandconn: starting ssh with [-l penguin 172.20.0.2 -- docker system dial-stdio]"

Address this by switching the affected test cases to use `-l info` instead of
`-D`, they all just require some option not specifically `-D`. Note that `info`
is the default log level so this is effectively a nop (which is good).

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-04-03 15:06:34 +01:00
Ian Campbell add2da66c5 e2e: Use icmd.None in a couple more places
This checks for actual emptiness, while `""` means "don't care".

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-04-03 11:07:52 +01:00
Ian Campbell 8f3798cf04 cli-plugins: Reinstate deprecated `-h` short form of `--help`.
In the initial implementation I thought it would be good to not pass on the
deprecation to plugins (since they are new). However it turns out this causes
`docker helloworld -h` to print a spurious "pflag: help requested" line:

    $ docker helloworld -h
    pflag: help requested
    See 'docker helloworld --help'.

    Usage:	docker helloworld [OPTIONS] COMMAND

    A basic Hello World plugin for tests
    ...

Compared with:

    $ docker ps -h
    Flag shorthand -h has been deprecated, please use --help

    Usage:	docker ps [OPTIONS]

This is in essence because having the flag undefined hits a different path
within cobra, causing `c.execute()` to return early due to getting an error
(`flag.ErrHelp`) from `c.ParseFlags`, which launders the error through our
`FlagErrorFunc` which wraps it in a `StatusError` which in turn defeats an `if
err == flag.ErrHelp` check further up the call chain. If the flag is defined we
instead hit a path which returns a bare `flag.ErrHelp` without wrapping it.

I considered updating our `FlagErrorFunc` to not wrap `flag.ErrHelp` (and then
following the chain to the next thing) however while doing that I realised that
the code for `-h` (and `--help`) is deeply embedded into cobra (and its flags
library) such that actually using `-h` as a plugin argument meaning something
other than `help` is basically impossible/impractical. Therefore we may as well
have plugins behave identically to the monolithic CLI and support (deprecated)
the `-h` argument.

With this changed the help related blocks of `SetupRootCommand` and
`SetupPluginRootCommand` are now identical, so consolidate into
`setupCommonRootCommand`.

Tests are updated to check `-h` in a variety of scenarios, including the happy
case here.

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-03-28 17:18:20 +00:00
Ian Campbell 0b30592ee0 e2e/cli-plugins: improve checks for empty output by using `icmd.None`
In some cases this means switching to `icmd.Expected` rather than
`icmd.Success`, but this improves readability overall.

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-03-28 14:30:51 +00:00