Commit Graph

13 Commits

Author SHA1 Message Date
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 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 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
Ian Campbell 0b794e0620 e2e/cli-plugins: explicitly check that PersistentPreRunE works
I regressed this in d4ced2ef77 ("allow plugins to have argument which match a
top-level flag.") by unconditionally overwriting any `PersistentRunE` that the
user may have supplied.

We need to ensure two things:

1. That the user can use `PersistentRunE` (or `PersistentRun`) for their own
   purposes.
2. That our initialisation always runs, even if the user has used
   `PersistentRun*`, since that will shadow the root.

To do this add a `PersistentRunE` to the helloworld plugin which logs (covers 1
above) and then use it when calling the `apiversion` subcommand (which covers 2
since that uses the client)

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-03-14 14:20:19 +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
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
Ian Campbell 20439aa662 Simplify cli plugin config file entry
Make it a simple `map[string]string` for now.

Added a unit test for it.

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-02-25 10:38:48 +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 e5e578abc7 Allow plugins to make use of the cobra `PersistentPreRun` hooks.
Previously a plugin which used these hooks would overwrite the top-level plugin
command's use of this hook, resulting in the dockerCli object not being fully
initialised.

Provide a function which plugins can use to chain to the required behaviour.
This required some fairly ugly arrangements to preserve state (which was
previously in-scope in `newPluginCOmmand`) to be used by the new function.

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
Ian Campbell 5db336798c Add some simple e2e tests for executing CLI plugins
To help with this add a bad plugin which produces invalid metadata and arrange
for it to be built in the e2e container.

Signed-off-by: Ian Campbell <ijc@docker.com>
2019-01-30 13:44:05 +00:00