From 4a677b86a6fd0689c9ca65c66c463b92b9535388 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 26 Aug 2025 23:47:17 +0200 Subject: [PATCH 1/3] cli/command/image: remove special handling for plugin errors on pull This special handling was added in [moby@9b6dcc8], and later updated in [moby@c127d96], but it fully depended on string-matching, which is brittle. Testing the original ticket that lead to this handling, it looks like the string matching no longer works, and the daemon error is returned as-is: With graphdrivers: docker pull tiborvass/no-remove Using default tag: latest Error response from daemon: Encountered remote "application/vnd.docker.plugin.v0+json"(unknown) when fetching With containerd snapshotters enabled: docker pull tiborvass/no-remove Using default tag: latest latest: Pulling from tiborvass/no-remove cf635291f7c9: Download complete failed to unpack image on snapshotter overlayfs: mismatched image rootfs and manifest layers The error-message for containerd can probably be improved, but as the special handling in the CLI no longer works, we can remove it. [moby@9b6dcc8]: https://github.com/moby/moby/commit/9b6dcc8b9d1366d3da3c8f60f89de1a36b087b88 [moby@c127d96]: https://github.com/moby/moby/commit/c127d9614f5b30bd73861877f8540a63e7d869e9 Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 323fbc485e5902fbbab97ad86cb5d4f90f956e39) Signed-off-by: Sebastiaan van Stijn --- cli/command/image/pull.go | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/cli/command/image/pull.go b/cli/command/image/pull.go index f1d66e2e8261..2f010fb6f139 100644 --- a/cli/command/image/pull.go +++ b/cli/command/image/pull.go @@ -2,15 +2,14 @@ package image import ( "context" + "errors" "fmt" - "strings" "github.com/distribution/reference" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command/completion" "github.com/docker/cli/cli/trust" - "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -87,15 +86,13 @@ func runPull(ctx context.Context, dockerCLI command.Cli, opts pullOptions) error // Check if reference has a digest _, isCanonical := distributionRef.(reference.Canonical) if !opts.untrusted && !isCanonical { - err = trustedPull(ctx, dockerCLI, imgRefAndAuth, opts) + if err := trustedPull(ctx, dockerCLI, imgRefAndAuth, opts); err != nil { + return err + } } else { - err = imagePullPrivileged(ctx, dockerCLI, imgRefAndAuth, opts) - } - if err != nil { - if strings.Contains(err.Error(), "when fetching 'plugin'") { - return errors.New(err.Error() + " - Use `docker plugin install`") + if err := imagePullPrivileged(ctx, dockerCLI, imgRefAndAuth, opts); err != nil { + return err } - return err } _, _ = fmt.Fprintln(dockerCLI.Out(), imgRefAndAuth.Reference().String()) return nil From f77defa891ce4aacce3dc157da694e481a1213f1 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 8 Sep 2025 20:02:40 +0200 Subject: [PATCH 2/3] cli/command/plugin: remove special error handling on install, upgrade Similar to 323fbc485e5902fbbab97ad86cb5d4f90f956e39 - this code was added in [moby@c127d96], but used string-matching to detect cases where a user tried to install an image as plugin. However, this handling no longer matched any error-strings, so no longer worked: docker plugin install busybox Error response from daemon: did not find plugin config for specified reference docker.io/library/busybox:latest [moby@c127d96]: https://github.com/moby/moby/commit/c127d9614f5b30bd73861877f8540a63e7d869e9 Signed-off-by: Sebastiaan van Stijn (cherry picked from commit fb3f2da50ea63e219080c2e63b64c64ac58c80be) Signed-off-by: Sebastiaan van Stijn --- cli/command/plugin/install.go | 4 ---- cli/command/plugin/install_test.go | 8 -------- cli/command/plugin/upgrade.go | 4 ---- 3 files changed, 16 deletions(-) diff --git a/cli/command/plugin/install.go b/cli/command/plugin/install.go index e232fdf0034a..715ae76ae107 100644 --- a/cli/command/plugin/install.go +++ b/cli/command/plugin/install.go @@ -3,7 +3,6 @@ package plugin import ( "context" "fmt" - "strings" "github.com/distribution/reference" "github.com/docker/cli/cli" @@ -120,9 +119,6 @@ func runInstall(ctx context.Context, dockerCLI command.Cli, opts pluginOptions) } responseBody, err := dockerCLI.Client().PluginInstall(ctx, localName, options) if err != nil { - if strings.Contains(err.Error(), "(image) when fetching") { - return errors.New(err.Error() + " - Use \"docker image pull\"") - } return err } defer func() { diff --git a/cli/command/plugin/install_test.go b/cli/command/plugin/install_test.go index f66082d9302e..ee67e611d7dd 100644 --- a/cli/command/plugin/install_test.go +++ b/cli/command/plugin/install_test.go @@ -43,14 +43,6 @@ func TestInstallErrors(t *testing.T) { return nil, errors.New("error installing plugin") }, }, - { - description: "installation error due to missing image", - args: []string{"foo"}, - expectedError: "docker image pull", - installFunc: func(name string, options types.PluginInstallOptions) (io.ReadCloser, error) { - return nil, errors.New("(image) when fetching") - }, - }, } for _, tc := range testCases { diff --git a/cli/command/plugin/upgrade.go b/cli/command/plugin/upgrade.go index 7d4548d428f1..54a39685b702 100644 --- a/cli/command/plugin/upgrade.go +++ b/cli/command/plugin/upgrade.go @@ -3,7 +3,6 @@ package plugin import ( "context" "fmt" - "strings" "github.com/distribution/reference" "github.com/docker/cli/cli" @@ -80,9 +79,6 @@ func runUpgrade(ctx context.Context, dockerCLI command.Cli, opts pluginOptions) responseBody, err := dockerCLI.Client().PluginUpgrade(ctx, opts.localName, options) if err != nil { - if strings.Contains(err.Error(), "target is image") { - return errors.New(err.Error() + " - Use `docker image pull`") - } return err } defer func() { From 3bacc99580e805a248a8850622ca288aca5ca2cb Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 5 Sep 2025 13:30:47 +0200 Subject: [PATCH 3/3] cli/command/image: build: remove permissions warning on Windows This warning was added in [moby@4a8b3ca] to print a warning when building Linux images from a Windows client. Window's filesystem does not have an "executable" bit, which mean that, for example, copying a shell script to an image during build would lose the executable bit. So for Windows clients, the executable bit would be set on all files, unconditionally. Originally this was detected in the client, which had direct access to the API response headers, but when refactoring the client to use a common library in [moby@535c4c9], this was refactored into a `ImageBuildResponse` wrapper, deconstructing the API response into an `io.Reader` and a string field containing only the `OSType` header. This was the only use and only purpose of the `OSType` field, and now that BuildKit is the default builder for Linux images, this warning didn't get printed unless BuildKit was explicitly disabled. This patch removes the warning, so that we can potentially remove the field, or the `ImageBuildResponse` type altogether. [moby@4a8b3ca]: https://github.com/moby/moby/commit/4a8b3cad6096854027151dfbcfb4b2cd8841ad95 [moby@535c4c9]: https://github.com/moby/moby/commit/535c4c9a59b1e58c897677d6948a595cb3d28639 Signed-off-by: Sebastiaan van Stijn (cherry picked from commit af65ee4584196d543a3d548de4de97b62850505e) Signed-off-by: Sebastiaan van Stijn --- cli/command/image/build.go | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/cli/command/image/build.go b/cli/command/image/build.go index f7f0493d5d6f..4d54cae2f7d0 100644 --- a/cli/command/image/build.go +++ b/cli/command/image/build.go @@ -10,7 +10,6 @@ import ( "io" "os" "path/filepath" - "runtime" "strings" "github.com/distribution/reference" @@ -385,16 +384,6 @@ func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions) return err } - // Windows: show error message about modified file permissions if the - // daemon isn't running Windows. - if response.OSType != "windows" && runtime.GOOS == "windows" && !options.quiet { - _, _ = fmt.Fprintln(dockerCli.Out(), "SECURITY WARNING: You are building a Docker "+ - "image from Windows against a non-Windows Docker host. All files and "+ - "directories added to build context will have '-rwxr-xr-x' permissions. "+ - "It is recommended to double check and reset permissions for sensitive "+ - "files and directories.") - } - // Everything worked so if -q was provided the output from the daemon // should be just the image ID and we'll print that to stdout. if options.quiet {