Skip to content

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Aug 15, 2025


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)

@thaJeztah thaJeztah added this to the 28.4.0 milestone Aug 15, 2025
@thaJeztah thaJeztah added impact/deprecation status/2-code-review area/plugins area/go-sdk Changes affecting the Go SDK impact/go-sdk Noteworthy (compatibility changes) in the Go SDK labels Aug 15, 2025
@thaJeztah
Copy link
Member Author

Ah; we're probably still using cerrdefs as alias in some places;

30.99 cmd/docker/docker.go:204:8: undefined: errdefs
30.99 cmd/docker/docker.go:243:8: undefined: errdefs
30.99 cmd/docker/docker.go:476:8: undefined: errdefs

Let me have a look

@thaJeztah
Copy link
Member Author

OK, that would need #6217, but that one needs #6221

Let me open a draft PR for that, and move this into draft as well

@thaJeztah thaJeztah force-pushed the 28.x_backport_plugin_manager_unexport branch from 4015634 to b29ec52 Compare August 15, 2025 11:38
@thaJeztah thaJeztah requested review from silvin-lubecki and a team as code owners August 15, 2025 11:38
@thaJeztah thaJeztah marked this pull request as draft August 15, 2025 11:39
@codecov-commenter
Copy link

codecov-commenter commented Aug 15, 2025

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 thaJeztah force-pushed the 28.x_backport_plugin_manager_unexport branch 2 times, most recently from ab70659 to 16e059b Compare August 16, 2025 16:30
It is for internal use for mocking purposes, and is not part
of any public interface / signature.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 54367b3)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
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 <[email protected]>
(cherry picked from commit 549d39a)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
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 <[email protected]>
(cherry picked from commit 1cc698c)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
These errors satisfy errdefs.IsNotFound, so make it a wrapper, and
deprecate it.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 7146021)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
We no longer depend on this interface and it implements Unwrap for
native handling by go stdlib.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit d789bac)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
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 <[email protected]>
(cherry picked from commit 50963ac)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
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`

Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 5876b29)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
This const was deprecated in 9dc175d,
which is part of v28.0, so let's remove it.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 513ceee)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah force-pushed the 28.x_backport_plugin_manager_unexport branch from 16e059b to 79e0b1d Compare August 17, 2025 17:45
@thaJeztah thaJeztah marked this pull request as ready for review August 17, 2025 17:55
@thaJeztah thaJeztah merged commit 6a596c0 into docker:28.x Aug 18, 2025
103 of 104 checks passed
@thaJeztah thaJeztah deleted the 28.x_backport_plugin_manager_unexport branch August 18, 2025 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/go-sdk Changes affecting the Go SDK area/plugins impact/deprecation impact/go-sdk Noteworthy (compatibility changes) in the Go SDK status/2-code-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants