cli-plugins/manager: various fixes and deprecations

[thaJeztah]

cli-plugins/manager: un-export "Candidate" interface

It is for internal use for mocking purposes, and is not part
of any public interface / signature.

cli-plugins/manager: fix Plugin marshaling with regular errors

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.

cli-plugins/manager: un-export "NewPluginError"

It is for internal use, and no longer needed for testing, now that
the Plugin type handles marshalling errors.

cli-plugins/manager: deprecate "IsNotFound"

These errors satisfy errdefs.IsNotFound, so make it a wrapper, and
deprecate it.

cli-plugins/manager: pluginError: remove Causer interface

We no longer depend on this interface and it implements Unwrap for
native handling by go stdlib.

cli-plugins/manager: wrapAsPluginError: don't special-case nil

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.

cli-plugins/manager: deprecate metadata aliases

These aliases were added in 4321293
(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

cli-plugins/manager: remove deprecated ResourceAttributesEnvvar

This const was deprecated in 9dc175d,
which is part of v28.0, so let's remove it.

- Human readable description for the release notes

Go SDK: `cli-plugins/manager`: remove `Candidate` interface, which was only for internal use.
Go SDK: `cli-plugins/manager`: remove `NewPluginError` function, which was only for internal use.
Go SDK: `cli-plugins/manager`: remove deprecated `ResourceAttributesEnvvar` const.
Go SDK: `cli-plugins/manager`: deprecate metadata aliases in favor of their equivalent in `cli-plugins/manager/metadata`:

- `NamePrefix`
- `MetadataSubcommandName`
- `HookSubcommandName`
- `Metadata`
- `ReexecEnvvar`

- A picture of a cute animal (not mandatory but encouraged)

[codecov-commenter]

Codecov Report

❌ Patch coverage is 79.16667% with 5 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
cmd/docker/docker.go	0.00%	2 Missing and 1 partial ⚠️
cli-plugins/manager/manager.go	66.66%	1 Missing ⚠️
cmd/docker/builder.go	0.00%	0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[thaJeztah]

After this, I want to look at removing the use of pluginError altogether; it can probably just embed the error, but it has some surprising behavior; its Unwrap doesn't really "unwrap", or at least, wrapping an error using wrapAsPluginError wraps it as pluginError

cli/cli-plugins/manager/error.go

Lines 41 to 48 in f86ad2e

	// wrapAsPluginError wraps an error in a pluginError with an
	// additional message.
	func wrapAsPluginError(err error, msg string) error {
	if err == nil {
	return nil
	}
	return &pluginError{cause: fmt.Errorf("%s: %w", msg, err)}
	}

But the wrapped error doesn't unwrap; it's effectively "masked", because "unwrap" would return the wrapped error (need to have a closer look at that);

cli/cli-plugins/manager/error.go

Lines 31 to 34 in f86ad2e

	// Unwrap provides compatibility for Go 1.13 error chains.
	func (e *pluginError) Unwrap() error {
	return e.cause
	}

[Copilot]

Pull Request Overview

This PR makes various fixes and deprecations to the cli-plugins/manager package to improve error handling, clean up internal APIs, and deprecate unused functionality. The key changes focus on better JSON marshaling of plugin errors, removing internal-only exports, and preparing for future API cleanup.

• Fix Plugin marshaling to properly handle regular errors in JSON output (e.g., docker info)
• Remove internal-only exports (Candidate interface, NewPluginError function) and deprecated constants
• Deprecate metadata aliases and IsNotFound function in favor of standard alternatives

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
cmd/docker/docker.go	Replace deprecated pluginmanager.IsNotFound with errdefs.IsNotFound
cmd/docker/builder.go	Replace deprecated pluginmanager.IsNotFound with errdefs.IsNotFound
cli/command/system/info_test.go	Update test to use regular errors.New instead of removed NewPluginError
cli-plugins/manager/plugin_test.go	Add comprehensive tests for Plugin JSON marshaling
cli-plugins/manager/plugin.go	Add custom MarshalJSON method and remove exported Candidate interface
cli-plugins/manager/metadata.go	Add deprecation warnings to metadata aliases
cli-plugins/manager/manager_test.go	Replace deprecated IsNotFound with errdefs.IsNotFound
cli-plugins/manager/manager.go	Deprecate IsNotFound and remove deprecated constant
cli-plugins/manager/error_test.go	Update tests for internal error handling changes
cli-plugins/manager/error.go	Remove Causer interface and make NewPluginError internal
cli-plugins/manager/candidate.go	Remove exported Candidate interface

[Copilot]

The function wrapAsPluginError no longer handles nil errors, but the implementation will create a pluginError with a formatted error containing %!w(<nil>) when err is nil. This could lead to confusing error messages. Consider adding a nil check or document this behavior clearly.

Suggested change

	func wrapAsPluginError(err error, msg string) error {
	func wrapAsPluginError(err error, msg string) error {
	if err == nil {
	return nil
	}

[thaJeztah]

Nope; that's exactly the reason I don't want it; the caller should check if there's an error to wrap. Silently discarding such mistakes leads to subtle bugs.