Various dependencies, including "golang.org/x/.." started to update
the minimum required version,so we should follow suit for the next
release.
Note that the `//go:build` directives not necesserily have to be
updated, but it's good to keep them in sync until we have a go.mod
to control this.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The CLI currently hard-codes the schema-version for CLI plugins to
"0.1.0", which doesn't allow us to expand the schema for plugins.
As there's many plugins that we shipped already, we can't break
compatibility until we reach 2.0.0, but we can expand the schema
with non-breaking changes.
This patch makes the validation more permissive to allow new schema
versions <= 2.0.0. Note that existing CLIs will still invalidate
such versions, so we cannot update the version until such CLIs are
no longer expected to be used, but this patch lays the ground-work
to open that option.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Before this patch, a broken symlink would print a warning;
docker info > /dev/null
WARNING: Plugin "/Users/thajeztah/.docker/cli-plugins/docker-feedback" is not valid: failed to fetch metadata: fork/exec /Users/thajeztah/.docker/cli-plugins/docker-feedback: no such file or directory
After this patch, such symlinks are ignored:
docker info > /dev/null
With debug enabled, we don't ignore the faulty plugin, which will
make the warning shown on docker info;
mkdir -p ~/.docker/cli-plugins
ln -s nosuchplugin ~/.docker/cli-plugins/docker-brokenplugin
docker --debug info
Client:
Version: 29.0.0-dev
Context: default
Debug Mode: true
Plugins:
buildx: Docker Buildx (Docker Inc.)
Version: v0.25.0
Path: /usr/libexec/docker/cli-plugins/docker-buildx
WARNING: Plugin "/Users/thajeztah/.docker/cli-plugins/docker-brokenplugin" is not valid: failed to fetch metadata: fork/exec /Users/thajeztah/.docker/cli-plugins/docker-brokenplugin: no such file or directory
# ...
We should als consider passing a "seen" map to de-duplicate entries.
Entries can be either a direct symlink or in a symlinked path (for
which we can filepath.EvalSymlinks). We need to benchmark the overhead
of resolving the symlink vs possibly calling the plugin (to get their
metadata) further down the line.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The pluginNameRe was a basic regular expression, effectively only checking
if the name consisted of lowercase alphanumeric characters. Replace it
with a minimal utility to do the same, without having to use regular
expressions (or the "lazyregexp" package).
Some quick benchmarking (not committed) show that the non-regex approach
is ~25x faster:
BenchmarkIsValidPluginName_Regex_Valid-10 13956240 81.39 ns/op 0 B/op 0 allocs/op
BenchmarkIsValidPluginName_Manual_Valid-10 360003060 3.318 ns/op 0 B/op 0 allocs/op
BenchmarkIsValidPluginName_Regex_Invalid-10 35281794 33.74 ns/op 0 B/op 0 allocs/op
BenchmarkIsValidPluginName_Manual_Invalid-10 906072663 1.320 ns/op 0 B/op 0 allocs/op
BenchmarkIsValidPluginName_Regex_Parallel-10 96595677 12.04 ns/op 0 B/op 0 allocs/op
BenchmarkIsValidPluginName_Manual_Parallel-10 1000000000 0.4541 ns/op 0 B/op 0 allocs/op
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These consts and types were moved to a separate metadata package in commits
292713c887 and 4321293972,
and deprecated in 72f76f2720, 5876b2941c,
and 6fa7d18320.
This removes the deprecated aliases in `cli-plugins/manager` in favor of
their equivalent in `cli-plugins/manager/metadata`:
- `CommandAnnotationPlugin`
- `CommandAnnotationPluginVendor`
- `CommandAnnotationPluginVersion`
- `CommandAnnotationPluginInvalid`
- `CommandAnnotationPluginCommandPath`
- `NamePrefix`
- `MetadataSubcommandName`
- `HookSubcommandName`
- `Metadata`
- `ReexecEnvvar`
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These errors satisfy errdefs.IsNotFound, which can be used instead. This
function was deprecated in 71460215d3 and
is no longer used.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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 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>
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>
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>
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>
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>
internal/test/cli.go:175:14: fmt.Errorf can be replaced with errors.New (perfsprint)
return nil, fmt.Errorf("no notary client available unless defined")
^
cli/command/cli.go:318:29: fmt.Errorf can be replaced with errors.New (perfsprint)
return docker.Endpoint{}, fmt.Errorf("no context store initialized")
^
cli/command/container/attach.go:161:11: fmt.Errorf can be replaced with errors.New (perfsprint)
return fmt.Errorf(result.Error.Message)
^
cli/command/container/opts.go:577:16: fmt.Errorf can be replaced with errors.New (perfsprint)
return nil, fmt.Errorf("--health-start-period cannot be negative")
^
cli/command/container/opts.go:580:16: fmt.Errorf can be replaced with errors.New (perfsprint)
return nil, fmt.Errorf("--health-start-interval cannot be negative")
^
cli/command/container/stats.go:221:11: fmt.Errorf can be replaced with errors.New (perfsprint)
return fmt.Errorf("filtering is not supported when specifying a list of containers")
^
cli/command/container/attach_test.go:82:17: fmt.Errorf can be replaced with errors.New (perfsprint)
expectedErr = fmt.Errorf("unexpected error")
^
cli/command/container/create_test.go:234:40: fmt.Errorf can be replaced with errors.New (perfsprint)
return container.CreateResponse{}, fmt.Errorf("shouldn't try to pull image")
^
cli/command/container/list_test.go:150:17: fmt.Errorf can be replaced with errors.New (perfsprint)
return nil, fmt.Errorf("error listing containers")
^
cli/command/container/rm_test.go:40:31: fmt.Errorf can be replaced with errors.New (perfsprint)
return errdefs.NotFound(fmt.Errorf("Error: no such container: " + container))
^
cli/command/container/run_test.go:138:40: fmt.Errorf can be replaced with errors.New (perfsprint)
return container.CreateResponse{}, fmt.Errorf("shouldn't try to pull image")
^
cli/command/image/pull_test.go:115:49: fmt.Errorf can be replaced with errors.New (perfsprint)
return io.NopCloser(strings.NewReader("")), fmt.Errorf("shouldn't try to pull image")
^
cli/command/network/connect.go:88:16: fmt.Errorf can be replaced with errors.New (perfsprint)
return nil, fmt.Errorf("invalid key/value pair format in driver options")
^
cli/command/plugin/create_test.go:96:11: fmt.Errorf can be replaced with errors.New (perfsprint)
return fmt.Errorf("Error creating plugin")
^
cli/command/plugin/disable_test.go:32:12: fmt.Errorf can be replaced with errors.New (perfsprint)
return fmt.Errorf("Error disabling plugin")
^
cli/command/plugin/enable_test.go:32:12: fmt.Errorf can be replaced with errors.New (perfsprint)
return fmt.Errorf("failed to enable plugin")
^
cli/command/plugin/inspect_test.go:55:22: fmt.Errorf can be replaced with errors.New (perfsprint)
return nil, nil, fmt.Errorf("error inspecting plugin")
^
cli/command/plugin/install_test.go:43:17: fmt.Errorf can be replaced with errors.New (perfsprint)
return nil, fmt.Errorf("Error installing plugin")
^
cli/command/plugin/install_test.go:51:17: fmt.Errorf can be replaced with errors.New (perfsprint)
return nil, fmt.Errorf("(image) when fetching")
^
cli/command/plugin/install_test.go:95:17: fmt.Errorf can be replaced with errors.New (perfsprint)
return nil, fmt.Errorf("should not try to install plugin")
^
cli/command/plugin/list_test.go:35:41: fmt.Errorf can be replaced with errors.New (perfsprint)
return types.PluginsListResponse{}, fmt.Errorf("error listing plugins")
^
cli/command/plugin/remove_test.go:27:12: fmt.Errorf can be replaced with errors.New (perfsprint)
return fmt.Errorf("Error removing plugin")
^
cli/command/registry/login_test.go:36:46: fmt.Errorf can be replaced with errors.New (perfsprint)
return registrytypes.AuthenticateOKBody{}, fmt.Errorf("Invalid Username or Password")
^
cli/command/registry/login_test.go:44:46: fmt.Errorf can be replaced with errors.New (perfsprint)
return registrytypes.AuthenticateOKBody{}, fmt.Errorf(errUnknownUser)
^
cli/command/system/info.go:190:10: fmt.Errorf can be replaced with errors.New (perfsprint)
return fmt.Errorf("errors pretty printing info")
^
cli/command/system/prune.go:77:10: fmt.Errorf can be replaced with errors.New (perfsprint)
return fmt.Errorf(`ERROR: The "until" filter is not supported with "--volumes"`)
^
cli/command/system/version_test.go:19:28: fmt.Errorf can be replaced with errors.New (perfsprint)
return types.Version{}, fmt.Errorf("no server")
^
cli/command/trust/key_load.go:112:22: fmt.Errorf can be replaced with errors.New (perfsprint)
return []byte{}, fmt.Errorf("could not decrypt key")
^
cli/command/trust/revoke.go:44:10: fmt.Errorf can be replaced with errors.New (perfsprint)
return fmt.Errorf("cannot use a digest reference for IMAGE:TAG")
^
cli/command/trust/revoke.go:105:10: fmt.Errorf can be replaced with errors.New (perfsprint)
return fmt.Errorf("no signed tags to remove")
^
cli/command/trust/signer_add.go:56:10: fmt.Errorf can be replaced with errors.New (perfsprint)
return fmt.Errorf("releases is a reserved keyword, please use a different signer name")
^
cli/command/trust/signer_add.go:60:10: fmt.Errorf can be replaced with errors.New (perfsprint)
return fmt.Errorf("path to a public key must be provided using the `--key` flag")
^
opts/config.go:71:10: fmt.Errorf can be replaced with errors.New (perfsprint)
return fmt.Errorf("source is required")
^
opts/mount.go:168:10: fmt.Errorf can be replaced with errors.New (perfsprint)
return fmt.Errorf("type is required")
^
opts/mount.go:172:10: fmt.Errorf can be replaced with errors.New (perfsprint)
return fmt.Errorf("target is required")
^
opts/network.go:90:11: fmt.Errorf can be replaced with errors.New (perfsprint)
return fmt.Errorf("network name/id is not specified")
^
opts/network.go:129:18: fmt.Errorf can be replaced with errors.New (perfsprint)
return "", "", fmt.Errorf("invalid key value pair format in driver options")
^
opts/opts.go:404:13: fmt.Errorf can be replaced with errors.New (perfsprint)
return 0, fmt.Errorf("value is too precise")
^
opts/opts.go:412:18: fmt.Errorf can be replaced with errors.New (perfsprint)
return "", "", fmt.Errorf("empty string specified for links")
^
opts/parse.go:84:37: fmt.Errorf can be replaced with errors.New (perfsprint)
return container.RestartPolicy{}, fmt.Errorf("invalid restart policy format: no policy provided before colon")
^
opts/parse.go:89:38: fmt.Errorf can be replaced with errors.New (perfsprint)
return container.RestartPolicy{}, fmt.Errorf("invalid restart policy format: maximum retry count must be an integer")
^
opts/port.go:105:13: fmt.Errorf can be replaced with errors.New (perfsprint)
return fmt.Errorf("hostip is not supported")
^
opts/secret.go:70:10: fmt.Errorf can be replaced with errors.New (perfsprint)
return fmt.Errorf("source is required")
^
opts/env_test.go:57:11: fmt.Errorf can be replaced with errors.New (perfsprint)
err: fmt.Errorf("invalid environment variable: =a"),
^
opts/env_test.go:93:11: fmt.Errorf can be replaced with errors.New (perfsprint)
err: fmt.Errorf("invalid environment variable: ="),
^
cli-plugins/manager/error_test.go:16:11: fmt.Errorf can be replaced with errors.New (perfsprint)
inner := fmt.Errorf("testing")
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Don't show `Next steps:` with no messages at all when plugin returns an
unitialized value of `HookMessage` (zero-initialization sets its type to
NextSteps and empty template).
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
