This alias was added in 4321293972, which is
part of v28.0, but did not deprecate them. They are no longer used in the
CLI itself, but may be used by cli-plugin implementations.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These aliases were added in 292713c887
(part of v28.0), but did not deprecate them. They are no longer used
in the CLI itself, but may be used by cli-plugin implementations.
This deprecates the aliases in `cli-plugins/manager` in favor of
their equivalent in `cli-plugins/manager/metadata`:
- `CommandAnnotationPlugin`
- `CommandAnnotationPluginVendor`
- `CommandAnnotationPluginVersion`
- `CommandAnnotationPluginInvalid`
- `CommandAnnotationPluginCommandPath`
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These utilities were only used internally; create a local copy
where used, and deprecate the ones in cli.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These aliases were added in 4321293972
(part of v28.0), but did not deprecate them. They are no longer used
in the CLI itself, but may be used by cli-plugin implementations.
This deprecates the aliases in `cli-plugins/manager` in favor of
their equivalent in `cli-plugins/manager/metadata`:
- `NamePrefix`
- `MetadataSubcommandName`
- `HookSubcommandName`
- `Metadata`
- `ReexecEnvvar`
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This was a pattern inheritted from pkg/errors.Wrapf, which ignored
nil errors for convenience. However, it is error-prone, as it is
not obvious when returning a nil-error.
All call-sites using `wrapAsPluginError` already do a check for
nil errors, so remove this code to prevent hard to find bugs.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
It is for internal use, and no longer needed for testing, now that
the `Plugin` type handles marshalling errors.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Go does not by default marshal `error` type fields to JSON. The manager
package therefore implemented a `pluginError` type that implements
[encoding.TextMarshaler]. However, the field was marked as a regular
`error`, which made it brittle; assining any other type of error would
result in the error being discarded in the marshaled JSON (as used in
`docker info` output), resulting in the error being marshaled as `{}`.
This patch adds a custom `MarshalJSON()` on the `Plugin` type itself
so that any error is rendered. It checks if the error used already
implements [encoding.TextMarshaler], otherwise wraps the error in
a `pluginError`.
[encoding.TextMarshaler]: https://pkg.go.dev/encoding#TextMarshaler
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Experimental is always enabled (977d3ae046),
and the `Experimental` field in plugin metadata was deprecated in
977d3ae046 and removed in commit
6a50c4f700.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The WithInitializeClient looks redundant altogether, so let's
rewrite this function to not depend on it.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Go maintainers started to unconditionally update the minimum go version
for golang.org/x/ dependencies to go1.23, which means that we'll no longer
be able to support any version below that when updating those dependencies;
> all: upgrade go directive to at least 1.23.0 [generated]
>
> By now Go 1.24.0 has been released, and Go 1.22 is no longer supported
> per the Go Release Policy (https://go.dev/doc/devel/release#policy).
>
> For golang/go#69095.
This updates our minimum version to go1.23, as we won't be able to maintain
compatibility with older versions because of the above.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- Add runtime.Gosched() calls to encourage goroutine scheduling
- Increase the timeout from 10ms to 500ms
- Use poll.WaitOn with appropriate delays to ensure the goroutine has
spawned before checking
- Lock the test goroutines to its own thread
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
This function returned an error (if any) from [config.Path]. However, the
only situation in which an error could be returned was if the given path
to append to `config.Dir` was outside of the config directory. This can
only happen if the path to append would try to traverse directories (e.g.,
passing `../../cli-plugins`).
Given that we're passing a hard-coded value, that would not be the case,
so we can simplify the code to join the path directly, and don't have to
handle errors.
[config.Path]: 2d74733942/cli/config/config.go (L100-L107)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Skip the other logic, which includes listing all commands provided; if
there's no plugin-candidates, those steps won't be needed.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This error-group was added in 89583b92b7, but
passed a context.TODO because the function didn't have a context as argument.
However, it does get the root-command passed, which holds the context, so
we can pass that.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The manager only requires the CLI's configuration; define a shallow interface
for this so that we don't have to import cli/command.
In addition to the CLI's configuration, `runHooks` also used the CLI's configured
StdErr output. We set the Cobra input and output streams to be the same as the
DockerCLI outputs in [newDockerCommand] and [newPluginCommand], so we can
get this from the Cobra command.
[newDockerCommand]: ea1f10b440/cmd/docker/docker.go (L148-L150)
[newPluginCommand]: ea1f10b440/cli-plugins/plugin/plugin.go (L166-L168)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These utility functions were added in 8890a1c929,
and are all related to OTEL. The ResourceAttributesEnvvar const defines
the "OTEL_RESOURCE_ATTRIBUTES" environment-variable to use, which is part
of the [OpenTelemetry specification], so should be considered a well-known
env-var, and not up to us to define a const for. These code-changes were not
yet included in a release, so we don't have to deprecate.
This patch:
- Moves the utility functions to the telemetry files, so that all code related
to OpenTelemetry is together.
- Un-exports the ResourceAttributesEnvvar to reduce our public API.
- Un-exports the DockerCliAttributePrefix to reduce depdency on cli/command
in CLI-plugins, but adds a TODO to move telemetry-related code to a common
(internal) package.
- Deprecates the cli-plugins/manager.ResourceAttributesEnvvar const. This
const has no known consumers, so we could skip deprecation, but just in
case some codebase uses this.
[OpenTelemetry specification]: https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/#general-sdk-configuration
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Remove the `docker.cli` prefixed attributes from
`OTEL_RESOURCE_ATTRIBUTES` after the telemetry provider has been created
within a plugin. This prevents accidentally sending the attributes to
something downstream for the user.
This also fixes an issue with compose where the self-injected `OTEL_RESOURCE_ATTRIBUTES`
would override an existing attribute in the environment file because the
"user environment" overrode the environment file, but the "user
environment" was created by the `docker` tool rather than by the user's
environment.
When `OTEL_RESOURCE_ATTRIBUTES` is empty after pruning, the environment
variable is unset.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Merge `OTEL_RESOURCE_ATTRIBUTES` when there is one already in the
environment. This allows user-specified resource attributes to be passed
on to CLI plugins while still allowing the extra attributes added for
telemetry information.
This was the original intended use-case but it seems to have never made
it in. The reason `OTEL_RESOURCE_ATTRIBUTES` was used is because we
could combine it with user-centric ones.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
cli-plugins/manager/manager.go:35:7: unused-receiver: method receiver 'e' is not referenced in method's body, consider removing or renaming it as _ (revive)
func (e errPluginNotFound) NotFound() {}
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- use Println to print newline instead of custom format
- suppress some errors to make my IDE and linters happier
- use res.Assert() with icmd.Expected{} where possible to make
assertions not depend on newline / whitespace randomness
- use apiClient instead of client for the API client to
prevent shadowing imports.
- use dockerCLI with Go's standard camelCase casing.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This patch enables descriptions on the CLI completion script.
It used to be disabled due to the CLI historically not supporting
cobra v2 completions, as seen by this patch
cbec75e2f3.
As an escape hatch, the user can set the `DOCKER_CLI_DISABLE_COMPLETION_DESCRIPTION`
environment variable to disable the completion description when
generating the completion file with `docker completion <fish|bash|zsh>`.
Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>
Fix a case where one inaccessible plugin search path stops the whole
search and prevents latter paths from being scanned.
Remove a preliminary `Stat` call that verifies whether path is an actual
directory and is accessible.
It's unneeded and doesn't actually check whether the directory can be
listed or not.
`os.ReadDir` will fail in such case anyway, so just attempt to do that
and ignore any encountered error, instead of erroring out the whole
plugin candidate listing.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
commit 4a7b04d412 configured golangci-lint
to use go1.23 semantics, which enabled the copyloopvar linter.
go1.22 now creates a copy of variables when assigned in a loop; make sure we
don't have files that may downgrade semantics to go1.21 in case that also means
disabling that feature; https://go.dev/ref/spec#Go_1.22
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
go1.22 and up now produce a unique variable in loops, tehrefore no longer
requiring to capture the variable manually;
cli-plugins/manager/cobra.go:55:4: The copy of the 'for' variable "p" can be deleted (Go 1.22+) (copyloopvar)
p := p
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The logic in this function is confusing; let's start make it obvious where
the error that is returned is produced,
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Before this patch, output for invalid top-level and sub-commands differed.
For top-level commands, the CLI would print an error-message and a suggestion
to use `--help`. For missing *subcommands*, we would hit a different code-path,
and different output, which includes full "usage" / "help" output.
While it is a common convention to show usage output, and may have been
a nice gesture when docker was still young and only had a few commands
and options ("you did something wrong; here's an overview of what you
can use"), that's no longer the case, and many commands have a _very_
long output.
The result of this is that the error message, which is the relevant
information in this case - "You mis-typed something" - is lost in the
output, and hard to find (sometimes even requiring scrolling back).
The output is also confusing, because it _looks_ like something ran
successfully (most of the output is not about the error!).
Even further; the suggested resolution (try `--help` to see the correct
options) is rather redundant, because running teh command with `--help`
produces _exactly_ the same output as was just showh, baring the error
message. As a fun fact, due to the usage output being printed, the
output even contains not one, but _two_ "call to actions";
- `See 'docker volume --help'.` (under the erro message)
- `Run 'docker volume COMMAND --help' for more information on a command.`
(under the usage output)
In short; the output is too verbose, confusing, and doesn't provide
a good UX. Let's reduce the output produced so that the focus is on the
important information.
This patch:
- Changes the usage to the short-usage.
- Changes the error-message to mention the _full_ command instead of only
the command after `docker` (so `docker no-such-command` instead of
`no-such-command`).
- Prefixes the error message with the binary / root-command name
(usually `docker:`); this is something we can still decide on, but
it's a pattern we already use in some places. The motivation for this
is that `docker` commands can often produce output that's a combination
of output from the CLI itself, output from the daemon, and even output
from the container. The `docker:` prefix helps to distinguish where
the message originated from (the `docker` CLI in this case).
- Adds an empty line between the error-message and the "call to action"
(`Run 'docker volume --help'...` in the example below). This helps
separating the error message ("unkown flag") from the call-to-action.
Before this patch:
Unknown top-level command:
docker nosuchcommand foo
docker: 'nosuchcommand' is not a docker command.
See 'docker --help'
Unknown sub-command:
docker volume nosuchcommand foo
Usage: docker volume COMMAND
Manage volumes
Commands:
create Create a volume
inspect Display detailed information on one or more volumes
ls List volumes
prune Remove unused local volumes
rm Remove one or more volumes
update Update a volume (cluster volumes only)
Run 'docker volume COMMAND --help' for more information on a command.
After this patch:
Unknown top-level command:
docker nosuchcommand foo
docker: unknown command: docker nosuchcommand
Run 'docker --help' for more information
Unknown sub-command:
docker volume nosuchcommand foo
docker: unknown command: 'docker volume nosuchcommand'
Usage: docker volume COMMAND
Run 'docker volume --help' for more information
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Add some documentation about their purpose, and document order of preference
when resolving plugins.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
