Commit 5011759056 implemented a fix that
caused the current environment to be discarded, using `os.Environ()`.
On Windows, `os.Environ()` may produce an incorrect value for `PWD`,
for which a new function was added in go1.19;
- https://tip.golang.org/doc/go1.19#osexecpkgosexec
- https://go-review.googlesource.com/c/go/+/401340
Replace the use of `os.Environ()` with `cmd.Environ()` to address that.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Explicitly create the context and set it on the CLI, instead of depending on
NewDockerCli() to instance a default context.
Co-authored-by: Sebastiaan van Stijn <github@gone.nl>
Co-authored-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>
Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>
Before, for plugin commands, only the plugin name (such as `buildx`)
would be both included as `RootCmd` when passed to the hook plugin,
which isn't enough information for a plugin to decide whether to execute
a hook or not since plugins implement multiple varied commands (`buildx
build`, `buildx prune`, etc.).
This commit changes the hook logic to account for this situation, so
that the the entire configured hook is passed, i.e., if a user has a
hook configured for `buildx imagetools inspect` and the command
`docker buildx imagetools inspect alpine` is called, then the plugin
hooks will be passed `buildx imagetools inspect`.
This logic works for aliased commands too, so whether `docker build ...`
or `docker buildx build` is executed (unless Buildx is disabled) the
hook will be invoked with `buildx build`.
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
hooks: include full match when invoking plugins
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
During normal plugin execution (from the CLI), the CLI configures the
plugin command it's about to execute in order to pass all environment
variables on, as well as to set the ReExec env var that informs the
plugin about how it was executed, and which plugins rely on to check
whether they are being run standalone or not.
This commit adds the same behavior to hook invocations, which is
necessary for some plugins to know that they are not running standalone
so that they expose their root command at the correct level.
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
This adds a default otel error handler for the cli in the debug package.
It uses logrus to log the error on the debug level and should work out
of the box with the `--debug` flag and `DEBUG` environment variable.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Go's `net` package [will unlink][1] for us, as long as we used Listen &
friends to create the Unix socket.
Go will even skip the unlink when the socket appears to be abstract
(starts with a NUL, represented by an @), though we must be cautious to
only create sockets with an abstract address on platforms that actually
support it -- this caused [several][2] [bugs][3] before.
[1]: https://pkg.go.dev/net#UnixListener.SetUnlinkOnClose
[2]: https://github.com/docker/cli/pull/4783
[3]: https://github.com/docker/cli/pull/4863
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
This changes things to rely on a plugin server that manages all
connections made to the server.
An optional handler can be passed into the server when the caller wants
to do extra things with the connection.
It is the caller's responsibility to close the server.
When the server is closed, first all existing connections are closed
(and new connections are prevented).
Now the signal loop only needs to close the server and not deal with
`net.Conn`'s directly (or double-indirects as the case was before this
change).
The socket, when present in the filesystem, is no longer unlinked
eagerly, as reconnections require it to be present for the lifecycle of
the plugin server.
Co-authored-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
When a plugin is invoked, the docker cli will now set
`OTEL_RESOURCE_ATTRIBUTES` to pass OTEL resource attribute names to the
plugin as additional resource attributes. At the moment, the only
resource attribute passed is `cobra.command_path`.
All resource attributes passed by the CLI are prepended with the
namespace `docker.cli` to avoid clashing with existing ones the plugin
uses or ones defined by the user.
For aliased commands like the various builder commands, the command path
is overwritten to match with the original name (such as `docker
builder`) instead of the forwarded name (such as `docker buildx build`).
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
The "github.com/docker/distribution" module moved to the distribution
org ("github.com/docker/distribution/v3"), and the new module deprecated
and removed the uuid package in favor of Google's UUID package.
While we still depend on the old module through packages and as an indirect
dependency, we may want to try avoid using it.
This patch replaces the use for the socket package, and replaces it for a
local utility, taking the same approach as `stringid.GenerateRandomID()`,
which should be random enough for this purpose.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Seems that OpenBSD behaves like darwin and requires to unlink all
socket, after it was used.
Tested on OpenBSD 7.4
Signed-off-by: Kirill A. Korinsky <kirill@korins.ky>
Adds a new plugin to the e2e plugins that simulates an older
plugin binary and a test suite to ensure older plugin binaries
keep behaving the same with newer CLI versions.
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
This reverts commit ef5e5fa03f.
Running new plugins under a new pgid isn't a viable solution due to
it causing issues with plugin processes attempting to read from the
TTY (see: https://github.com/moby/moby/issues/47073).
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
Changes were made in 1554ac3b5f to provide
a mechanism for the CLI to notify running plugin processes that they
should exit, in order to improve the general CLI/plugin UX. The current
implementation boils down to:
1. The CLI creates a socket
2. The CLI executes the plugin
3. The plugin connects to the socket
4. (When) the CLI receives a termination signal, it uses the socket to
notify the plugin that it should exit
5. The plugin's gets notified via the socket, and cancels it's `cmd.Context`,
which then gets handled appropriately
This change works in most cases and fixes the issue it sets out to solve
(see: https://github.com/docker/compose/pull/11292) however, in the case
where the user has a TTY attached and the plugin is not already handling
received signals, steps 4+ changes:
4. (When) the CLI receives a termination signal, before it can use the
socket to notify the plugin that it should exit, the plugin process
also receives a signal due to sharing the pgid with the CLI
Since we now have a proper "job control" mechanism, we can simplify the
scenarios by executing the plugins with their own process group id,
thereby removing the "double notification" issue and making it so that
plugins can handle the same whether attached to a TTY or not.
In order to make this change "plugin-binary" backwards-compatible, in
the case that a plugin does not connect to the socket, the CLI passes
the signal to the plugin process.
Co-authored-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
As macOS does not support the abstract socket namespace, use a temporary
socket in $TMPDIR to connect with the plugin. Ensure this socket is
cleaned up even in the case of crash/ungraceful termination by removing
it after the first connection is accepted.
Co-authored-by: Laura Brehm <laurabrehm@hey.com>
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
This is a follow-up to 0e73168b7e
This repository is not yet a module (i.e., does not have a `go.mod`). This
is not problematic when building the code in GOPATH or "vendor" mode, but
when using the code as a module-dependency (in module-mode), different semantics
are applied since Go1.21, which switches Go _language versions_ on a per-module,
per-package, or even per-file base.
A condensed summary of that logic [is as follows][1]:
- For modules that have a go.mod containing a go version directive; that
version is considered a minimum _required_ version (starting with the
go1.19.13 and go1.20.8 patch releases: before those, it was only a
recommendation).
- For dependencies that don't have a go.mod (not a module), go language
version go1.16 is assumed.
- Likewise, for modules that have a go.mod, but the file does not have a
go version directive, go language version go1.16 is assumed.
- If a go.work file is present, but does not have a go version directive,
language version go1.17 is assumed.
When switching language versions, Go _downgrades_ the language version,
which means that language features (such as generics, and `any`) are not
available, and compilation fails. For example:
# github.com/docker/cli/cli/context/store
/go/pkg/mod/github.com/docker/cli@v25.0.0-beta.2+incompatible/cli/context/store/storeconfig.go:6:24: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
/go/pkg/mod/github.com/docker/cli@v25.0.0-beta.2+incompatible/cli/context/store/store.go:74:12: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
Note that these fallbacks are per-module, per-package, and can even be
per-file, so _(indirect) dependencies_ can still use modern language
features, as long as their respective go.mod has a version specified.
Unfortunately, these failures do not occur when building locally (using
vendor / GOPATH mode), but will affect consumers of the module.
Obviously, this situation is not ideal, and the ultimate solution is to
move to go modules (add a go.mod), but this comes with a non-insignificant
risk in other areas (due to our complex dependency tree).
We can revert to using go1.16 language features only, but this may be
limiting, and may still be problematic when (e.g.) matching signatures
of dependencies.
There is an escape hatch: adding a `//go:build` directive to files that
make use of go language features. From the [go toolchain docs][2]:
> The go line for each module sets the language version the compiler enforces
> when compiling packages in that module. The language version can be changed
> on a per-file basis by using a build constraint.
>
> For example, a module containing code that uses the Go 1.21 language version
> should have a `go.mod` file with a go line such as `go 1.21` or `go 1.21.3`.
> If a specific source file should be compiled only when using a newer Go
> toolchain, adding `//go:build go1.22` to that source file both ensures that
> only Go 1.22 and newer toolchains will compile the file and also changes
> the language version in that file to Go 1.22.
This patch adds `//go:build` directives to those files using recent additions
to the language. It's currently using go1.19 as version to match the version
in our "vendor.mod", but we can consider being more permissive ("any" requires
go1.18 or up), or more "optimistic" (force go1.21, which is the version we
currently use to build).
For completeness sake, note that any file _without_ a `//go:build` directive
will continue to use go1.16 language version when used as a module.
[1]: 58c28ba286/src/cmd/go/internal/gover/version.go (L9-L56)
[2]; https://go.dev/doc/toolchain#:~:text=The%20go%20line%20for,file%20to%20Go%201.22
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The cli/command package defined two option-types with the same signature.
This patch creates a new type instead (CLIOption), and makes the existing
types an alias for this (deprecating their old names).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Previously, long lived CLI plugin processes weren't
properly handled
(see: https://github.com/docker/cli/issues/4402)
resulting in plugin processes being left behind
running, after the CLI process exits.
This commit changes the plugin handling code to open
an abstract unix socket before running the plugin and
passing it to the plugin process, and changes the
signal handling on the CLI side to close this socket
which tells the plugin that it should exit.
This implementation makes use of sockets instead of
simply setting PDEATHSIG on the plugin process
so that it will work on both BSDs, assorted UNIXes
and Windows.
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
The flag-set that was returned is a pointer to the command's Flags(), which
is in itself passed by reference (as it is modified / set up).
This patch removes the flags return, to prevent assuming it's different than
the command's flags.
While SetupRootCommand is exported, a search showed that it's only used internally,
so changing the signature should not be a problem.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This field was marked deprecated in 977d3ae046,
which is part of Docker 20.10 and up.
This patch removes the field.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
the "golang.org/x/sys/execabs" package was introduced to address a security
issue on Windows, and changing the default behavior of os/exec was considered
a breaking change. go1.19 applied the behavior that was previously implemented
in the execabs package;
from the release notes: https://go.dev/doc/go1.19#os-exec-path
> Command and LookPath no longer allow results from a PATH search to be found
> relative to the current directory. This removes a common source of security
> problems but may also break existing programs that depend on using, say,
> exec.Command("prog") to run a binary named prog (or, on Windows, prog.exe)
> in the current directory. See the os/exec package documentation for information
> about how best to update such programs.
>
> On Windows, Command and LookPath now respect the NoDefaultCurrentDirectoryInExePath
> environment variable, making it possible to disable the default implicit search
> of “.” in PATH lookups on Windows systems.
With those changes, we no longer need to use the execabs package, and we can
switch back to os/exec.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This field was marked deprecated in 977d3ae046,
which is part of v20.10 and up, but the comment was missing a newline before
the deprecation message, which may be picked up by IDEs, but is not matching
the correct format, so may not be picked up by linters.
This patch fixes the format, to make sure linters pick up that the field is
deprecated.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This reverts commit 62f2358b99.
Spawning a goroutine for each iteration in the loop when listing
plugins is racy unfortunately. `plugins` slice is protected with
a mutex so not sure why it fails.
I tried using a channel to collect the plugins instead of a slice
to guarantee that they will be appended to the list in the order
they are processed but no dice.
I also tried without errgroup package and simply use sync.WaitGroup
but same. I have also created an extra channel to receive errors
from the goroutines but racy too.
I think the change in this function is not related to the race
condition but newPlugin is. So revert in the meantime :(
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
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>
We are currently loading plugin commands stubs for every
command invocation to add support for Cobra v2 completion.
This cause a significant performance hit if there is a
lot of plugins in the user space (7 atm in Docker Desktop):
`docker --version` takes in current 23.0.1 ~93ms
Instead of removing completion for plugins to fix the
regression, we can slightly improve plugins discovery by
spawning a goroutine for each iteration in the loop when
listing plugins:
`docker --version` now takes ~38ms
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
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>
The test used `gopkg.in/yaml.v2` to verify the TextMarshaller implementation,
which was implemented to allow printing the errors in JSON formatted output;
> This exists primarily to implement encoding.TextMarshaller such that
> rendering a plugin as JSON (e.g. for `docker info -f '{{json .CLIPlugins}}'`)
> renders the Err field as a useful string and not just `{}`.
Given that both yaml.Marshal and json.Marshal use this, we may as well use
Go's stdlib.
While updating, also changed some of the assertions to checks, so that we don't
fail the test early.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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>
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>
On Windows, the os/exec.{Command,CommandContext,LookPath} functions
resolve command names that have neither path separators nor file extension
(e.g., "git") by first looking in the current working directory before
looking in the PATH environment variable.
Go maintainers intended to match cmd.exe's historical behavior.
However, this is pretty much never the intended behavior and as an abundance of precaution
this patch prevents that when executing commands.
Example of commands that docker.exe may execute: `git`, `docker-buildx` (or other cli plugin), `docker-credential-wincred`, `docker`.
Note that this was prompted by the [Go 1.15.7 security fixes](https://blog.golang.org/path-security), but unlike in `go.exe`,
the windows path lookups in docker are not in a code path allowing remote code execution, thus there is no security impact on docker.
Signed-off-by: Tibor Vass <tibor@docker.com>
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>
The vanity domain is down, and the project has moved
to a new location.
vendor check started failing because of this:
Collecting initial packages
Download dependencies
unrecognized import path "vbom.ml/util" (https fetch: Get https://vbom.ml/util?go-get=1: dial tcp: lookup vbom.ml on 169.254.169.254:53: no such host)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Before this change, plugins were listed in a random order:
Client:
Debug Mode: false
Plugins:
doodle: Docker Doodles all around! 🐳🎃 (thaJeztah, v0.0.1)
shell: Open a browser shell on the Docker Host. (thaJeztah, v0.0.1)
app: Docker Application (Docker Inc., v0.8.0)
buildx: Build with BuildKit (Docker Inc., v0.3.1-tp-docker)
With this change, plugins are listed alphabetically:
Client:
Debug Mode: false
Plugins:
app: Docker Application (Docker Inc., v0.8.0)
buildx: Build with BuildKit (Docker Inc., v0.3.1-tp-docker)
doodle: Docker Doodles all around! 🐳🎃 (thaJeztah, v0.0.1)
shell: Open a browser shell on the Docker Host. (thaJeztah, v0.0.1)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
To test, add $(pwd)/build/plugins-linux-amd64 to "cliPluginsExtraDirs" config and run:
make plugins
make binary
HELLO_EXPERIMENTAL=1 docker helloworld
To show it enabled:
HELLO_EXPERIMENTAL=1 DOCKER_CLI_EXPERIMENTAL=enabled docker helloworld
Signed-off-by: Tibor Vass <tibor@docker.com>
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>
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>
Since #1654 so far we've had problems with it not working on Windows (npipe
lacked the `CloseRead` method) and problems with using tcp with tls (the tls
connection also lacks `CloseRead`). Both of these were workedaround in #1718
which added a nop `CloseRead` method.
However I am now seeing hangs (on Windows) where the `system dial-stdio`
subprocess is not exiting (I'm unsure why so far).
I think the 3rd problem found with this is an indication that `dial-stdio` is
not quite ready for wider use outside of its initial usecase (support for
`ssh://` URLs to connect to remote daemons).
This change simply disables the `dial-stdio` path for all plugins. However
rather than completely reverting 891b3d953e ("cli-plugins: use `docker system
dial-stdio` to call the daemon") I've just disabled the functionality at the
point of use and left in a trap door environment variable so that those who
want to experiment with this mode (and perhaps fully debug it) have an easier
path do doing so.
The e2e test for this case is disabled unless the trap door envvar is set. I
also renamed the test to clarify that it is about cli plugins.
Signed-off-by: Ian Campbell <ijc@docker.com>
I got a bit carried away in d4ced2ef77 ("allow plugins to have argument
which match a top-level flag.") and broke the ability of a plugin to use the
`PersistentPreRun(E)` hook on its top-level command (by unconditionally
overwriting it) and also broke the plugin framework if a plugin's subcommand
used those hooks (because they would shadow the root one). This could result in
either `dockerCli.Client()` returning `nil` or whatever initialisation the
plugin hoped to do not occuring.
This change revert the relevant bits and reinstates the requirement that a
plugin calls `plugin.PersistentPreRunE` if it uses that hook itself.
It is at least a bit nicer now since we avoid the need for the global struct
since the interesting state is now encapsulated in `tcmd` (and the closure).
In principal this could be done even more simply (by calling `tcmd.Initialize`
statically between `tcmd.HandleGlobalFlags` and `cmd.Execute`) however this has
the downside of _always_ initialising the cli (and therefore dialing the
daemon) even for the `docker-cli-plugin-metadata` command but also for the
`help foo` and `foo --help` commands (Cobra short-circuits the hooks in this
case).
Signed-off-by: Ian Campbell <ijc@docker.com>
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>
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>
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>
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>
- The placement of the vendor is now in the end of the line.
- A '*' is now added as suffix of plugins' top level commands.
Signed-off-by: Ulysses Souza <ulysses.souza@docker.com>
This means that plugins can use whatever methods the monolithic CLI supports,
which is good for consistency.
This relies on `os.Args[0]` being something which can be executed again to
reach the same binary, since it is propagated (via an envvar) to the plugin for
this purpose. This essentially requires that the current working directory and
path are not modified by the monolithic CLI before it launches the plugin nor
by the plugin before it initializes the client. This should be the case.
Previously the fake apiclient used by `TestExperimentalCLI` was not being used,
since `cli.Initialize` was unconditionally overwriting it with a real one
(talking to a real daemon during unit testing, it seems). This wasn't expected
nor desirable and no longer happens with the new arrangements, exposing the
fact that no `pingFunc` is provided, leading to a panic. Add a `pingFunc` to
the fake client to avoid this.
Signed-off-by: Ian Campbell <ijc@docker.com>
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>
A static global initialiser happens before the arguments are parsed, so we need
to calculate the path later.
Signed-off-by: Ian Campbell <ijc@docker.com>
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>
Fairly straight forward. It became necessary to wrap `Plugin.Err` with a type
which implements `encoding.MarshalText` in order to have that field rendered
properly in the `docker info -f '{{json}}'` output.
Since I changed the type somewhat I also added a unit test for `formatInfo`.
Signed-off-by: Ian Campbell <ijc@docker.com>
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>
To do this we add a stub `cobra.Command` for each installed plugin (only when
invoking `help`, not for normal running).
This requires a function to list all available plugins so that is added here.
Signed-off-by: Ian Campbell <ijc@docker.com>
Also includes the scaffolding for finding a validating plugin candidates.
Argument validation is moved to RunE to support this, so `noArgs` is removed.
Signed-off-by: Ian Campbell <ijc@docker.com>
Alano Terblanche
Sebastiaan van Stijn
Laura Brehm
Jonathan A. Sternberg
Bjorn Neergaard
Brian Goff
Tonis Tiigi
Kirill A. Korinsky
Sebastiaan van Stijn
Kevin Alvarez
Nicolas De Loof
Tibor Vass
Silvin Lubecki
Ian Campbell
Stefan Scherer
Silvin Lubecki
Nick Adcock
Corey Quon
Ulysses Souza