From b10b79e6fd9c111d76e960326efa4204ccff5b0f Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 22:16:12 +0100 Subject: [PATCH 01/21] cli-plugins: minor cleanups: use Println - 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 --- cli-plugins/examples/helloworld/main.go | 26 +++++++++++------------ cli-plugins/hooks/printer.go | 4 ++-- e2e/cli-plugins/config_test.go | 2 +- e2e/cli-plugins/dial_test.go | 10 ++++----- e2e/cli-plugins/flags_test.go | 12 +++++------ e2e/cli-plugins/plugins/presocket/main.go | 2 +- e2e/cli-plugins/run_test.go | 22 +++++++++++-------- 7 files changed, 41 insertions(+), 37 deletions(-) diff --git a/cli-plugins/examples/helloworld/main.go b/cli-plugins/examples/helloworld/main.go index 0461e01c8c1d..d91e1185fa96 100644 --- a/cli-plugins/examples/helloworld/main.go +++ b/cli-plugins/examples/helloworld/main.go @@ -12,24 +12,24 @@ import ( ) func main() { - plugin.Run(func(dockerCli command.Cli) *cobra.Command { + plugin.Run(func(dockerCLI command.Cli) *cobra.Command { goodbye := &cobra.Command{ Use: "goodbye", Short: "Say Goodbye instead of Hello", Run: func(cmd *cobra.Command, _ []string) { - fmt.Fprintln(dockerCli.Out(), "Goodbye World!") + _, _ = fmt.Fprintln(dockerCLI.Out(), "Goodbye World!") }, } apiversion := &cobra.Command{ Use: "apiversion", Short: "Print the API version of the server", RunE: func(_ *cobra.Command, _ []string) error { - cli := dockerCli.Client() - ping, err := cli.Ping(context.Background()) + apiClient := dockerCLI.Client() + ping, err := apiClient.Ping(context.Background()) if err != nil { return err } - fmt.Println(ping.APIVersion) + _, _ = fmt.Println(ping.APIVersion) return nil }, } @@ -38,7 +38,7 @@ func main() { Use: "exitstatus2", Short: "Exit with status 2", RunE: func(_ *cobra.Command, _ []string) error { - fmt.Fprintln(dockerCli.Err(), "Exiting with error status 2") + _, _ = fmt.Fprintln(dockerCLI.Err(), "Exiting with error status 2") os.Exit(2) return nil }, @@ -56,33 +56,33 @@ func main() { return err } if preRun { - fmt.Fprintf(dockerCli.Err(), "Plugin PersistentPreRunE called") + _, _ = fmt.Fprintln(dockerCLI.Err(), "Plugin PersistentPreRunE called") } return nil }, RunE: func(cmd *cobra.Command, args []string) error { if debug { - fmt.Fprintf(dockerCli.Err(), "Plugin debug mode enabled") + _, _ = fmt.Fprintln(dockerCLI.Err(), "Plugin debug mode enabled") } switch optContext { case "Christmas": - fmt.Fprintf(dockerCli.Out(), "Merry Christmas!\n") + _, _ = fmt.Fprintln(dockerCLI.Out(), "Merry Christmas!") return nil case "": // nothing } if who == "" { - who, _ = dockerCli.ConfigFile().PluginConfig("helloworld", "who") + who, _ = dockerCLI.ConfigFile().PluginConfig("helloworld", "who") } if who == "" { who = "World" } - fmt.Fprintf(dockerCli.Out(), "Hello %s!\n", who) - dockerCli.ConfigFile().SetPluginConfig("helloworld", "lastwho", who) - return dockerCli.ConfigFile().Save() + _, _ = fmt.Fprintln(dockerCLI.Out(), "Hello", who) + dockerCLI.ConfigFile().SetPluginConfig("helloworld", "lastwho", who) + return dockerCLI.ConfigFile().Save() }, } diff --git a/cli-plugins/hooks/printer.go b/cli-plugins/hooks/printer.go index bedc87f929b5..f6d4b28ef488 100644 --- a/cli-plugins/hooks/printer.go +++ b/cli-plugins/hooks/printer.go @@ -11,8 +11,8 @@ func PrintNextSteps(out io.Writer, messages []string) { if len(messages) == 0 { return } - fmt.Fprintln(out, aec.Bold.Apply("\nWhat's next:")) + _, _ = fmt.Fprintln(out, aec.Bold.Apply("\nWhat's next:")) for _, n := range messages { - _, _ = fmt.Fprintf(out, " %s\n", n) + _, _ = fmt.Fprintln(out, " ", n) } } diff --git a/e2e/cli-plugins/config_test.go b/e2e/cli-plugins/config_test.go index a9d55e3d8716..1cac1dd7b1e8 100644 --- a/e2e/cli-plugins/config_test.go +++ b/e2e/cli-plugins/config_test.go @@ -20,7 +20,7 @@ func TestConfig(t *testing.T) { res := icmd.RunCmd(run("helloworld")) res.Assert(t, icmd.Expected{ ExitCode: 0, - Out: "Hello Cambridge!", + Out: "Hello Cambridge", }) cfg2, err := config.Load(filepath.Dir(cfg.GetFilename())) diff --git a/e2e/cli-plugins/dial_test.go b/e2e/cli-plugins/dial_test.go index 0be5ec3d4c95..87ae6eb4884e 100644 --- a/e2e/cli-plugins/dial_test.go +++ b/e2e/cli-plugins/dial_test.go @@ -6,8 +6,6 @@ import ( "testing" "github.com/docker/cli/cli-plugins/manager" - "gotest.tools/v3/assert" - is "gotest.tools/v3/assert/cmp" "gotest.tools/v3/icmd" ) @@ -24,7 +22,9 @@ func TestCLIPluginDialStdio(t *testing.T) { helloworld := filepath.Join(os.Getenv("DOCKER_CLI_E2E_PLUGINS_EXTRA_DIRS"), "docker-helloworld") cmd := icmd.Command(helloworld, "--config=blah", "--log-level", "debug", "helloworld", "--who=foo") res := icmd.RunCmd(cmd, icmd.WithEnv(manager.ReexecEnvvar+"=/bin/true")) - res.Assert(t, icmd.Success) - assert.Assert(t, is.Contains(res.Stderr(), `msg="commandconn: starting /bin/true with [--config=blah --log-level debug system dial-stdio]"`)) - assert.Assert(t, is.Equal(res.Stdout(), "Hello foo!\n")) + res.Assert(t, icmd.Expected{ + ExitCode: 0, + Err: `msg="commandconn: starting /bin/true with [--config=blah --log-level debug system dial-stdio]"`, + Out: `Hello foo`, + }) } diff --git a/e2e/cli-plugins/flags_test.go b/e2e/cli-plugins/flags_test.go index 7631bab0ded4..69fe6d6ad3b4 100644 --- a/e2e/cli-plugins/flags_test.go +++ b/e2e/cli-plugins/flags_test.go @@ -15,7 +15,7 @@ func TestRunGoodArgument(t *testing.T) { res := icmd.RunCmd(run("helloworld", "--who", "Cleveland")) res.Assert(t, icmd.Expected{ ExitCode: 0, - Out: "Hello Cleveland!", + Out: "Hello Cleveland", }) } @@ -33,25 +33,25 @@ func TestClashWithGlobalArgs(t *testing.T) { { name: "short-without-val", args: []string{"-D"}, - expectedOut: "Hello World!", + expectedOut: "Hello World", expectedErr: "Plugin debug mode enabled", }, { name: "long-without-val", args: []string{"--debug"}, - expectedOut: "Hello World!", + expectedOut: "Hello World", expectedErr: "Plugin debug mode enabled", }, { name: "short-with-val", args: []string{"-c", "Christmas"}, - expectedOut: "Merry Christmas!", + expectedOut: "Merry Christmas", expectedErr: icmd.None, }, { name: "short-with-val", args: []string{"--context", "Christmas"}, - expectedOut: "Merry Christmas!", + expectedOut: "Merry Christmas", expectedErr: icmd.None, }, } { @@ -220,7 +220,7 @@ func TestCliPluginsVersion(t *testing.T) { name: "plugin-with-version", args: []string{"helloworld", "version"}, expCode: 0, - expOut: "Hello World!", + expOut: "Hello World", expErr: icmd.None, }, { diff --git a/e2e/cli-plugins/plugins/presocket/main.go b/e2e/cli-plugins/plugins/presocket/main.go index 8c8ad6df6625..9a695f188374 100644 --- a/e2e/cli-plugins/plugins/presocket/main.go +++ b/e2e/cli-plugins/plugins/presocket/main.go @@ -113,7 +113,7 @@ func RootCmd(dockerCli command.Cli) *cobra.Command { select { case <-done: case <-time.After(2 * time.Second): - _, _ = fmt.Fprint(dockerCli.Err(), "timeout after 2 seconds") + _, _ = fmt.Fprintln(dockerCli.Err(), "timeout after 2 seconds") } return nil }, diff --git a/e2e/cli-plugins/run_test.go b/e2e/cli-plugins/run_test.go index c3bd279c996a..db3628802fc3 100644 --- a/e2e/cli-plugins/run_test.go +++ b/e2e/cli-plugins/run_test.go @@ -4,7 +4,6 @@ import ( "testing" "gotest.tools/v3/assert" - is "gotest.tools/v3/assert/cmp" "gotest.tools/v3/golden" "gotest.tools/v3/icmd" ) @@ -123,7 +122,7 @@ func TestRunGood(t *testing.T) { res := icmd.RunCmd(run("helloworld")) res.Assert(t, icmd.Expected{ ExitCode: 0, - Out: "Hello World!", + Out: "Hello World", Err: icmd.None, }) } @@ -218,18 +217,23 @@ func TestCliInitialized(t *testing.T) { run, _, cleanup := prepare(t) defer cleanup() - var apiversion string + var apiVersion string t.Run("withhook", func(t *testing.T) { res := icmd.RunCmd(run("helloworld", "--pre-run", "apiversion")) - res.Assert(t, icmd.Success) - assert.Assert(t, res.Stdout() != "") - apiversion = res.Stdout() - assert.Assert(t, is.Equal(res.Stderr(), "Plugin PersistentPreRunE called")) + res.Assert(t, icmd.Expected{ + ExitCode: 0, + Err: "Plugin PersistentPreRunE called", + }) + apiVersion = res.Stdout() + assert.Assert(t, apiVersion != "") }) t.Run("withouthook", func(t *testing.T) { res := icmd.RunCmd(run("nopersistentprerun")) - res.Assert(t, icmd.Success) - assert.Assert(t, is.Equal(res.Stdout(), apiversion)) + res.Assert(t, icmd.Expected{ + ExitCode: 0, + Err: icmd.None, + Out: apiVersion, + }) }) } From 8650ffef3864d55f56d7f56a500edb1fdd05b1c9 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 22:18:07 +0100 Subject: [PATCH 02/21] cli/command/checkpoint: minor cleanups: use Println, rename vars - use Println to print newline instead of custom format - use dockerCLI with Go's standard camelCase casing. - suppress some errors to make my IDE and linters happier Signed-off-by: Sebastiaan van Stijn --- cli/command/checkpoint/create.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cli/command/checkpoint/create.go b/cli/command/checkpoint/create.go index 3ea41ee85fbe..8455e979e64f 100644 --- a/cli/command/checkpoint/create.go +++ b/cli/command/checkpoint/create.go @@ -40,8 +40,8 @@ func newCreateCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runCreate(ctx context.Context, dockerCli command.Cli, opts createOptions) error { - err := dockerCli.Client().CheckpointCreate(ctx, opts.container, checkpoint.CreateOptions{ +func runCreate(ctx context.Context, dockerCLI command.Cli, opts createOptions) error { + err := dockerCLI.Client().CheckpointCreate(ctx, opts.container, checkpoint.CreateOptions{ CheckpointID: opts.checkpoint, CheckpointDir: opts.checkpointDir, Exit: !opts.leaveRunning, @@ -50,6 +50,6 @@ func runCreate(ctx context.Context, dockerCli command.Cli, opts createOptions) e return err } - fmt.Fprintf(dockerCli.Out(), "%s\n", opts.checkpoint) + _, _ = fmt.Fprintln(dockerCLI.Out(), opts.checkpoint) return nil } From c462eaee115445ba4a6e42506199be011c787217 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 22:30:53 +0100 Subject: [PATCH 03/21] cli/command/container: minor cleanups: use Println - use Println to print newline instead of custom format - suppress some errors to make my IDE and linters happier Signed-off-by: Sebastiaan van Stijn --- cli/command/container/cp.go | 13 ++++++------- cli/command/container/create.go | 8 ++++---- cli/command/container/stats.go | 2 +- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/cli/command/container/cp.go b/cli/command/container/cp.go index d6fe10341a21..3e975a81b62d 100644 --- a/cli/command/container/cp.go +++ b/cli/command/container/cp.go @@ -129,13 +129,12 @@ func NewCopyCommand(dockerCli command.Cli) *cobra.Command { Use: `cp [OPTIONS] CONTAINER:SRC_PATH DEST_PATH|- docker cp [OPTIONS] SRC_PATH|- CONTAINER:DEST_PATH`, Short: "Copy files/folders between a container and the local filesystem", - Long: strings.Join([]string{ - "Copy files/folders between a container and the local filesystem\n", - "\nUse '-' as the source to read a tar archive from stdin\n", - "and extract it to a directory destination in a container.\n", - "Use '-' as the destination to stream a tar archive of a\n", - "container source to stdout.", - }, ""), + Long: `Copy files/folders between a container and the local filesystem + +Use '-' as the source to read a tar archive from stdin +and extract it to a directory destination in a container. +Use '-' as the destination to stream a tar archive of a +container source to stdout.`, Args: cli.ExactArgs(2), RunE: func(cmd *cobra.Command, args []string) error { if args[0] == "" { diff --git a/cli/command/container/create.go b/cli/command/container/create.go index be81c2173311..30aea832ce81 100644 --- a/cli/command/container/create.go +++ b/cli/command/container/create.go @@ -274,7 +274,7 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerCfg *c if errdefs.IsNotFound(err) && namedRef != nil && options.pull == PullImageMissing { if !options.quiet { // we don't want to write to stdout anything apart from container.ID - fmt.Fprintf(dockerCli.Err(), "Unable to find image '%s' locally\n", reference.FamiliarString(namedRef)) + _, _ = fmt.Fprintf(dockerCli.Err(), "Unable to find image '%s' locally\n", reference.FamiliarString(namedRef)) } if err := pullAndTagImage(); err != nil { @@ -292,7 +292,7 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerCfg *c } for _, w := range response.Warnings { - _, _ = fmt.Fprintf(dockerCli.Err(), "WARNING: %s\n", w) + _, _ = fmt.Fprintln(dockerCli.Err(), "WARNING:", w) } err = containerIDFile.Write(response.ID) return response.ID, err @@ -300,7 +300,7 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerCfg *c func warnOnOomKillDisable(hostConfig container.HostConfig, stderr io.Writer) { if hostConfig.OomKillDisable != nil && *hostConfig.OomKillDisable && hostConfig.Memory == 0 { - fmt.Fprintln(stderr, "WARNING: Disabling the OOM killer on containers without setting a '-m/--memory' limit may be dangerous.") + _, _ = fmt.Fprintln(stderr, "WARNING: Disabling the OOM killer on containers without setting a '-m/--memory' limit may be dangerous.") } } @@ -309,7 +309,7 @@ func warnOnOomKillDisable(hostConfig container.HostConfig, stderr io.Writer) { func warnOnLocalhostDNS(hostConfig container.HostConfig, stderr io.Writer) { for _, dnsIP := range hostConfig.DNS { if isLocalhost(dnsIP) { - fmt.Fprintf(stderr, "WARNING: Localhost DNS setting (--dns=%s) may fail in containers.\n", dnsIP) + _, _ = fmt.Fprintf(stderr, "WARNING: Localhost DNS setting (--dns=%s) may fail in containers.\n", dnsIP) return } } diff --git a/cli/command/container/stats.go b/cli/command/container/stats.go index d6afb0528c6f..76f7765cd66e 100644 --- a/cli/command/container/stats.go +++ b/cli/command/container/stats.go @@ -299,7 +299,7 @@ func RunStats(ctx context.Context, dockerCLI command.Cli, options *StatsOptions) for _, line := range strings.Split(statsTextBuffer.String(), "\n") { // In case the new text is shorter than the one we are writing over, // we'll append the "erase line" escape sequence to clear the remaining text. - _, _ = fmt.Fprint(&statsTextBuffer, line, "\033[K\n") + _, _ = fmt.Fprintln(&statsTextBuffer, line, "\033[K") } // We might have fewer containers than before, so let's clear the remaining text From 82e2efbbf72a56d3c2b4520e886be1c9249ad038 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 22:32:30 +0100 Subject: [PATCH 04/21] cli/command/context: minor cleanups - use dockerCLI with Go's standard camelCase casing. - suppress some errors to make my IDE and linters happier Signed-off-by: Sebastiaan van Stijn --- cli/command/context/create.go | 10 +++++----- cli/command/context/import.go | 4 ++-- cli/command/context/update.go | 10 +++++----- cli/command/context/use.go | 12 ++++++------ 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/cli/command/context/create.go b/cli/command/context/create.go index a77151c25208..a3bbeeedfce3 100644 --- a/cli/command/context/create.go +++ b/cli/command/context/create.go @@ -34,11 +34,11 @@ func longCreateDescription() string { buf := bytes.NewBuffer(nil) buf.WriteString("Create a context\n\nDocker endpoint config:\n\n") tw := tabwriter.NewWriter(buf, 20, 1, 3, ' ', 0) - fmt.Fprintln(tw, "NAME\tDESCRIPTION") + _, _ = fmt.Fprintln(tw, "NAME\tDESCRIPTION") for _, d := range dockerConfigKeysDescriptions { - fmt.Fprintf(tw, "%s\t%s\n", d.name, d.description) + _, _ = fmt.Fprintf(tw, "%s\t%s\n", d.name, d.description) } - tw.Flush() + _ = tw.Flush() buf.WriteString("\nExample:\n\n$ docker context create my-context --description \"some description\" --docker \"host=tcp://myserver:2376,ca=~/ca-file,cert=~/cert-file,key=~/key-file\"\n") return buf.String() } @@ -79,8 +79,8 @@ func RunCreate(dockerCLI command.Cli, o *CreateOptions) error { err = createNewContext(s, o) } if err == nil { - fmt.Fprintln(dockerCLI.Out(), o.Name) - fmt.Fprintf(dockerCLI.Err(), "Successfully created context %q\n", o.Name) + _, _ = fmt.Fprintln(dockerCLI.Out(), o.Name) + _, _ = fmt.Fprintf(dockerCLI.Err(), "Successfully created context %q\n", o.Name) } return err } diff --git a/cli/command/context/import.go b/cli/command/context/import.go index c09f8f89f5b1..ff0179feb33f 100644 --- a/cli/command/context/import.go +++ b/cli/command/context/import.go @@ -45,7 +45,7 @@ func RunImport(dockerCli command.Cli, name string, source string) error { return err } - fmt.Fprintln(dockerCli.Out(), name) - fmt.Fprintf(dockerCli.Err(), "Successfully imported context %q\n", name) + _, _ = fmt.Fprintln(dockerCli.Out(), name) + _, _ = fmt.Fprintf(dockerCli.Err(), "Successfully imported context %q\n", name) return nil } diff --git a/cli/command/context/update.go b/cli/command/context/update.go index 98eba7d1ecaa..980937e0c1fe 100644 --- a/cli/command/context/update.go +++ b/cli/command/context/update.go @@ -24,11 +24,11 @@ func longUpdateDescription() string { buf := bytes.NewBuffer(nil) buf.WriteString("Update a context\n\nDocker endpoint config:\n\n") tw := tabwriter.NewWriter(buf, 20, 1, 3, ' ', 0) - fmt.Fprintln(tw, "NAME\tDESCRIPTION") + _, _ = fmt.Fprintln(tw, "NAME\tDESCRIPTION") for _, d := range dockerConfigKeysDescriptions { - fmt.Fprintf(tw, "%s\t%s\n", d.name, d.description) + _, _ = fmt.Fprintf(tw, "%s\t%s\n", d.name, d.description) } - tw.Flush() + _ = tw.Flush() buf.WriteString("\nExample:\n\n$ docker context update my-context --description \"some description\" --docker \"host=tcp://myserver:2376,ca=~/ca-file,cert=~/cert-file,key=~/key-file\"\n") return buf.String() } @@ -93,8 +93,8 @@ func RunUpdate(dockerCLI command.Cli, o *UpdateOptions) error { } } - fmt.Fprintln(dockerCLI.Out(), o.Name) - fmt.Fprintf(dockerCLI.Err(), "Successfully updated context %q\n", o.Name) + _, _ = fmt.Fprintln(dockerCLI.Out(), o.Name) + _, _ = fmt.Fprintf(dockerCLI.Err(), "Successfully updated context %q\n", o.Name) return nil } diff --git a/cli/command/context/use.go b/cli/command/context/use.go index c24a0c6ecb54..412df755e92d 100644 --- a/cli/command/context/use.go +++ b/cli/command/context/use.go @@ -24,19 +24,19 @@ func newUseCommand(dockerCli command.Cli) *cobra.Command { } // RunUse set the current Docker context -func RunUse(dockerCli command.Cli, name string) error { +func RunUse(dockerCLI command.Cli, name string) error { // configValue uses an empty string for "default" var configValue string if name != command.DefaultContextName { if err := store.ValidateContextName(name); err != nil { return err } - if _, err := dockerCli.ContextStore().GetMetadata(name); err != nil { + if _, err := dockerCLI.ContextStore().GetMetadata(name); err != nil { return err } configValue = name } - dockerConfig := dockerCli.ConfigFile() + dockerConfig := dockerCLI.ConfigFile() // Avoid updating the config-file if nothing changed. This also prevents // creating the file and config-directory if the default is used and // no config-file existed yet. @@ -46,10 +46,10 @@ func RunUse(dockerCli command.Cli, name string) error { return err } } - fmt.Fprintln(dockerCli.Out(), name) - fmt.Fprintf(dockerCli.Err(), "Current context is now %q\n", name) + _, _ = fmt.Fprintln(dockerCLI.Out(), name) + _, _ = fmt.Fprintf(dockerCLI.Err(), "Current context is now %q\n", name) if name != command.DefaultContextName && os.Getenv(client.EnvOverrideHost) != "" { - fmt.Fprintf(dockerCli.Err(), "Warning: %[1]s environment variable overrides the active context. "+ + _, _ = fmt.Fprintf(dockerCLI.Err(), "Warning: %[1]s environment variable overrides the active context. "+ "To use %[2]q, either set the global --context flag, or unset %[1]s environment variable.\n", client.EnvOverrideHost, name) } return nil From a0ca41e6f64271651809bea52de4fc4a4dd4780b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 22:33:22 +0100 Subject: [PATCH 05/21] cli/command/formatter: suppress some errors Signed-off-by: Sebastiaan van Stijn --- cli/command/formatter/disk_usage.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/command/formatter/disk_usage.go b/cli/command/formatter/disk_usage.go index 01edd2b042e4..18d79d08e6e8 100644 --- a/cli/command/formatter/disk_usage.go +++ b/cli/command/formatter/disk_usage.go @@ -237,7 +237,7 @@ func (ctx *DiskUsageContext) verboseWriteTable(duc *diskUsageContext) error { if err != nil { return err } - ctx.Output.Write([]byte("\nLocal Volumes space usage:\n\n")) + _, _ = ctx.Output.Write([]byte("\nLocal Volumes space usage:\n\n")) for _, v := range duc.Volumes { if err := ctx.contextFormat(tmpl, v); err != nil { return err @@ -249,7 +249,7 @@ func (ctx *DiskUsageContext) verboseWriteTable(duc *diskUsageContext) error { if err != nil { return err } - fmt.Fprintf(ctx.Output, "\nBuild cache usage: %s\n\n", units.HumanSize(float64(ctx.BuilderSize))) + _, _ = fmt.Fprintf(ctx.Output, "\nBuild cache usage: %s\n\n", units.HumanSize(float64(ctx.BuilderSize))) for _, v := range duc.BuildCache { if err := ctx.contextFormat(tmpl, v); err != nil { return err From c8f27b027f1edb2e26092178b9e9a3fab13b440b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 22:35:20 +0100 Subject: [PATCH 06/21] cli/command/image: minor cleanups: use Println, rename vars - use Println to print newline instead of custom format - use apiClient instead of client for the API client to prevent shadowing imports. - use dockerCLI with Go's standard camelCase casing. - suppress some errors to make my IDE and linters happier Signed-off-by: Sebastiaan van Stijn --- cli/command/image/pull.go | 4 ++-- cli/command/image/push.go | 2 +- cli/command/image/remove.go | 12 ++++++------ cli/command/image/trust.go | 14 +++++++------- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/cli/command/image/pull.go b/cli/command/image/pull.go index 93388253f75a..bc3ed55b8eea 100644 --- a/cli/command/image/pull.go +++ b/cli/command/image/pull.go @@ -66,7 +66,7 @@ func RunPull(ctx context.Context, dockerCLI command.Cli, opts PullOptions) error case !opts.all && reference.IsNameOnly(distributionRef): distributionRef = reference.TagNameOnly(distributionRef) if tagged, ok := distributionRef.(reference.Tagged); ok && !opts.quiet { - fmt.Fprintf(dockerCLI.Out(), "Using default tag: %s\n", tagged.Tag()) + _, _ = fmt.Fprintln(dockerCLI.Out(), "Using default tag:", tagged.Tag()) } } @@ -88,6 +88,6 @@ func RunPull(ctx context.Context, dockerCLI command.Cli, opts PullOptions) error } return err } - fmt.Fprintln(dockerCLI.Out(), imgRefAndAuth.Reference().String()) + _, _ = fmt.Fprintln(dockerCLI.Out(), imgRefAndAuth.Reference().String()) return nil } diff --git a/cli/command/image/push.go b/cli/command/image/push.go index d3868352fb09..508d30fe5065 100644 --- a/cli/command/image/push.go +++ b/cli/command/image/push.go @@ -101,7 +101,7 @@ To push the complete multi-platform image, remove the --platform flag. case !opts.all && reference.IsNameOnly(ref): ref = reference.TagNameOnly(ref) if tagged, ok := ref.(reference.Tagged); ok && !opts.quiet { - _, _ = fmt.Fprintf(dockerCli.Out(), "Using default tag: %s\n", tagged.Tag()) + _, _ = fmt.Fprintln(dockerCli.Out(), "Using default tag:", tagged.Tag()) } } diff --git a/cli/command/image/remove.go b/cli/command/image/remove.go index 3fb0af5c03de..067e0541b629 100644 --- a/cli/command/image/remove.go +++ b/cli/command/image/remove.go @@ -51,8 +51,8 @@ func newRemoveCommand(dockerCli command.Cli) *cobra.Command { return &cmd } -func runRemove(ctx context.Context, dockerCli command.Cli, opts removeOptions, images []string) error { - client := dockerCli.Client() +func runRemove(ctx context.Context, dockerCLI command.Cli, opts removeOptions, images []string) error { + apiClient := dockerCLI.Client() options := image.RemoveOptions{ Force: opts.force, @@ -62,7 +62,7 @@ func runRemove(ctx context.Context, dockerCli command.Cli, opts removeOptions, i var errs []string fatalErr := false for _, img := range images { - dels, err := client.ImageRemove(ctx, img, options) + dels, err := apiClient.ImageRemove(ctx, img, options) if err != nil { if !errdefs.IsNotFound(err) { fatalErr = true @@ -71,9 +71,9 @@ func runRemove(ctx context.Context, dockerCli command.Cli, opts removeOptions, i } else { for _, del := range dels { if del.Deleted != "" { - fmt.Fprintf(dockerCli.Out(), "Deleted: %s\n", del.Deleted) + _, _ = fmt.Fprintln(dockerCLI.Out(), "Deleted:", del.Deleted) } else { - fmt.Fprintf(dockerCli.Out(), "Untagged: %s\n", del.Untagged) + _, _ = fmt.Fprintln(dockerCLI.Out(), "Untagged:", del.Untagged) } } } @@ -84,7 +84,7 @@ func runRemove(ctx context.Context, dockerCli command.Cli, opts removeOptions, i if !opts.force || fatalErr { return errors.New(msg) } - fmt.Fprintln(dockerCli.Err(), msg) + _, _ = fmt.Fprintln(dockerCLI.Err(), msg) } return nil } diff --git a/cli/command/image/trust.go b/cli/command/image/trust.go index 3e06f9a90a7a..274344452f39 100644 --- a/cli/command/image/trust.go +++ b/cli/command/image/trust.go @@ -87,7 +87,7 @@ func PushTrustedReference(ctx context.Context, ioStreams command.Streams, repoIn if err := jsonstream.Display(ctx, in, ioStreams.Out()); err != nil { return err } - fmt.Fprintln(ioStreams.Err(), "No tag specified, skipping trust metadata push") + _, _ = fmt.Fprintln(ioStreams.Err(), "No tag specified, skipping trust metadata push") return nil } @@ -103,7 +103,7 @@ func PushTrustedReference(ctx context.Context, ioStreams command.Streams, repoIn return errors.Errorf("no targets found, provide a specific tag in order to sign it") } - fmt.Fprintln(ioStreams.Out(), "Signing and pushing trust metadata") + _, _ = fmt.Fprintln(ioStreams.Out(), "Signing and pushing trust metadata") repo, err := trust.GetNotaryRepository(ioStreams.In(), ioStreams.Out(), command.UserAgent(), repoInfo, &authConfig, "push", "pull") if err != nil { @@ -133,7 +133,7 @@ func PushTrustedReference(ctx context.Context, ioStreams command.Streams, repoIn if err := repo.Initialize([]string{rootKeyID}, data.CanonicalSnapshotRole); err != nil { return trust.NotaryError(repoInfo.Name.Name(), err) } - fmt.Fprintf(ioStreams.Out(), "Finished initializing %q\n", repoInfo.Name.Name()) + _, _ = fmt.Fprintf(ioStreams.Out(), "Finished initializing %q\n", repoInfo.Name.Name()) err = repo.AddTarget(target, data.CanonicalTargetsRole) case nil: // already initialized and we have successfully downloaded the latest metadata @@ -151,7 +151,7 @@ func PushTrustedReference(ctx context.Context, ioStreams command.Streams, repoIn return trust.NotaryError(repoInfo.Name.Name(), err) } - fmt.Fprintf(ioStreams.Out(), "Successfully signed %s:%s\n", repoInfo.Name.Name(), tag) + _, _ = fmt.Fprintf(ioStreams.Out(), "Successfully signed %s:%s\n", repoInfo.Name.Name(), tag) return nil } @@ -181,7 +181,7 @@ func trustedPull(ctx context.Context, cli command.Cli, imgRefAndAuth trust.Image if displayTag != "" { displayTag = ":" + displayTag } - fmt.Fprintf(cli.Out(), "Pull (%d of %d): %s%s@%s\n", i+1, len(refs), reference.FamiliarName(ref), displayTag, r.digest) + _, _ = fmt.Fprintf(cli.Out(), "Pull (%d of %d): %s%s@%s\n", i+1, len(refs), reference.FamiliarName(ref), displayTag, r.digest) trustedRef, err := reference.WithDigest(reference.TrimNamed(ref), r.digest) if err != nil { @@ -230,7 +230,7 @@ func getTrustedPullTargets(cli command.Cli, imgRefAndAuth trust.ImageRefAndAuth) for _, tgt := range targets { t, err := convertTarget(tgt.Target) if err != nil { - fmt.Fprintf(cli.Err(), "Skipping target for %q\n", reference.FamiliarName(ref)) + _, _ = fmt.Fprintf(cli.Err(), "Skipping target for %q\n", reference.FamiliarName(ref)) continue } // Only list tags in the top level targets role or the releases delegation role - ignore @@ -332,7 +332,7 @@ func TagTrusted(ctx context.Context, cli command.Cli, trustedRef reference.Canon familiarRef := reference.FamiliarString(ref) trustedFamiliarRef := reference.FamiliarString(trustedRef) - fmt.Fprintf(cli.Err(), "Tagging %s as %s\n", trustedFamiliarRef, familiarRef) + _, _ = fmt.Fprintf(cli.Err(), "Tagging %s as %s\n", trustedFamiliarRef, familiarRef) return cli.Client().ImageTag(ctx, trustedFamiliarRef, familiarRef) } From 5d3bdf8ac24814673531be9cd33f554a0cfdbef5 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 22:38:48 +0100 Subject: [PATCH 07/21] cli/command/manifest: minor cleanups: use Println, rename vars - use Println to print newline instead of custom format - use dockerCLI with Go's standard camelCase casing. - suppress some errors to make my IDE and linters happier Signed-off-by: Sebastiaan van Stijn --- cli/command/manifest/create_list.go | 8 ++++---- cli/command/manifest/push.go | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/cli/command/manifest/create_list.go b/cli/command/manifest/create_list.go index ac78b14f62c1..d6ca591ab22b 100644 --- a/cli/command/manifest/create_list.go +++ b/cli/command/manifest/create_list.go @@ -35,7 +35,7 @@ func newCreateListCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func createManifestList(ctx context.Context, dockerCli command.Cli, args []string, opts createOpts) error { +func createManifestList(ctx context.Context, dockerCLI command.Cli, args []string, opts createOpts) error { newRef := args[0] targetRef, err := normalizeReference(newRef) if err != nil { @@ -47,7 +47,7 @@ func createManifestList(ctx context.Context, dockerCli command.Cli, args []strin return errors.Wrapf(err, "error parsing repository name for manifest list %s", newRef) } - manifestStore := dockerCli.ManifestStore() + manifestStore := dockerCLI.ManifestStore() _, err = manifestStore.GetList(targetRef) switch { case store.IsNotFound(err): @@ -68,7 +68,7 @@ func createManifestList(ctx context.Context, dockerCli command.Cli, args []strin return err } - manifest, err := getManifest(ctx, dockerCli, targetRef, namedRef, opts.insecure) + manifest, err := getManifest(ctx, dockerCLI, targetRef, namedRef, opts.insecure) if err != nil { return err } @@ -76,6 +76,6 @@ func createManifestList(ctx context.Context, dockerCli command.Cli, args []strin return err } } - fmt.Fprintf(dockerCli.Out(), "Created manifest list %s\n", targetRef.String()) + _, _ = fmt.Fprintln(dockerCLI.Out(), "Created manifest list", targetRef.String()) return nil } diff --git a/cli/command/manifest/push.go b/cli/command/manifest/push.go index 29fcffe30f99..67c370ec7880 100644 --- a/cli/command/manifest/push.go +++ b/cli/command/manifest/push.go @@ -268,13 +268,13 @@ func buildPutManifestRequest(imageManifest types.ImageManifest, targetRef refere return mountRequest{ref: mountRef, manifest: imageManifest}, err } -func pushList(ctx context.Context, dockerCli command.Cli, req pushRequest) error { - rclient := dockerCli.RegistryClient(req.insecure) +func pushList(ctx context.Context, dockerCLI command.Cli, req pushRequest) error { + rclient := dockerCLI.RegistryClient(req.insecure) if err := mountBlobs(ctx, rclient, req.targetRef, req.manifestBlobs); err != nil { return err } - if err := pushReferences(ctx, dockerCli.Out(), rclient, req.mountRequests); err != nil { + if err := pushReferences(ctx, dockerCLI.Out(), rclient, req.mountRequests); err != nil { return err } dgst, err := rclient.PutManifest(ctx, req.targetRef, req.list) @@ -282,7 +282,7 @@ func pushList(ctx context.Context, dockerCli command.Cli, req pushRequest) error return err } - fmt.Fprintln(dockerCli.Out(), dgst.String()) + _, _ = fmt.Fprintln(dockerCLI.Out(), dgst.String()) return nil } @@ -292,7 +292,7 @@ func pushReferences(ctx context.Context, out io.Writer, client registryclient.Re if err != nil { return err } - fmt.Fprintf(out, "Pushed ref %s with digest: %s\n", mount.ref, newDigest) + _, _ = fmt.Fprintf(out, "Pushed ref %s with digest: %s\n", mount.ref, newDigest) } return nil } From 886f2295cf190d73197f82d8be4ecf953fde3963 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 22:40:07 +0100 Subject: [PATCH 08/21] cli/command/network: minor cleanups: use Println, rename vars - use Println to print newline instead of custom format - use apiClient instead of client for the API client to prevent shadowing imports. - use dockerCLI with Go's standard camelCase casing. - suppress some errors to make my IDE and linters happier Signed-off-by: Sebastiaan van Stijn --- cli/command/network/create.go | 2 +- cli/command/network/remove.go | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/cli/command/network/create.go b/cli/command/network/create.go index e43b183190c4..9331a6d7f147 100644 --- a/cli/command/network/create.go +++ b/cli/command/network/create.go @@ -130,7 +130,7 @@ func runCreate(ctx context.Context, apiClient client.NetworkAPIClient, output io if err != nil { return err } - _, _ = fmt.Fprintf(output, "%s\n", resp.ID) + _, _ = fmt.Fprintln(output, resp.ID) return nil } diff --git a/cli/command/network/remove.go b/cli/command/network/remove.go index 105e629f614c..ed1b7f264f7a 100644 --- a/cli/command/network/remove.go +++ b/cli/command/network/remove.go @@ -40,15 +40,15 @@ const ingressWarning = "WARNING! Before removing the routing-mesh network, " + "Otherwise, removal may not be effective and functionality of newly create " + "ingress networks will be impaired.\nAre you sure you want to continue?" -func runRemove(ctx context.Context, dockerCli command.Cli, networks []string, opts *removeOptions) error { - client := dockerCli.Client() +func runRemove(ctx context.Context, dockerCLI command.Cli, networks []string, opts *removeOptions) error { + apiClient := dockerCLI.Client() status := 0 for _, name := range networks { - nw, _, err := client.NetworkInspectWithRaw(ctx, name, network.InspectOptions{}) + nw, _, err := apiClient.NetworkInspectWithRaw(ctx, name, network.InspectOptions{}) if err == nil && nw.Ingress { - r, err := command.PromptForConfirmation(ctx, dockerCli.In(), dockerCli.Out(), ingressWarning) + r, err := command.PromptForConfirmation(ctx, dockerCLI.In(), dockerCLI.Out(), ingressWarning) if err != nil { return err } @@ -56,15 +56,15 @@ func runRemove(ctx context.Context, dockerCli command.Cli, networks []string, op continue } } - if err := client.NetworkRemove(ctx, name); err != nil { + if err := apiClient.NetworkRemove(ctx, name); err != nil { if opts.force && errdefs.IsNotFound(err) { continue } - _, _ = fmt.Fprintf(dockerCli.Err(), "%s\n", err) + _, _ = fmt.Fprintln(dockerCLI.Err(), err) status = 1 continue } - _, _ = fmt.Fprintf(dockerCli.Out(), "%s\n", name) + _, _ = fmt.Fprintln(dockerCLI.Out(), name) } if status != 0 { From 35e74d58e3457f689ec0c1e9fee1abfb01a55ce6 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 22:41:10 +0100 Subject: [PATCH 09/21] cli/command/node: minor cleanups: use Println, rename vars - use Println to print newline instead of custom format - use apiClient instead of client for the API client to prevent shadowing imports. - use dockerCLI with Go's standard camelCase casing. - suppress some errors to make my IDE and linters happier Signed-off-by: Sebastiaan van Stijn --- cli/command/node/demote.go | 4 ++-- cli/command/node/promote.go | 4 ++-- cli/command/node/remove.go | 10 +++++----- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/cli/command/node/demote.go b/cli/command/node/demote.go index ea914bb0941e..9f6b25ac96e5 100644 --- a/cli/command/node/demote.go +++ b/cli/command/node/demote.go @@ -24,14 +24,14 @@ func newDemoteCommand(dockerCli command.Cli) *cobra.Command { func runDemote(ctx context.Context, dockerCli command.Cli, nodes []string) error { demote := func(node *swarm.Node) error { if node.Spec.Role == swarm.NodeRoleWorker { - fmt.Fprintf(dockerCli.Out(), "Node %s is already a worker.\n", node.ID) + _, _ = fmt.Fprintf(dockerCli.Out(), "Node %s is already a worker.\n", node.ID) return errNoRoleChange } node.Spec.Role = swarm.NodeRoleWorker return nil } success := func(nodeID string) { - fmt.Fprintf(dockerCli.Out(), "Manager %s demoted in the swarm.\n", nodeID) + _, _ = fmt.Fprintf(dockerCli.Out(), "Manager %s demoted in the swarm.\n", nodeID) } return updateNodes(ctx, dockerCli, nodes, demote, success) } diff --git a/cli/command/node/promote.go b/cli/command/node/promote.go index c9d45e9cda52..225727397228 100644 --- a/cli/command/node/promote.go +++ b/cli/command/node/promote.go @@ -24,14 +24,14 @@ func newPromoteCommand(dockerCli command.Cli) *cobra.Command { func runPromote(ctx context.Context, dockerCli command.Cli, nodes []string) error { promote := func(node *swarm.Node) error { if node.Spec.Role == swarm.NodeRoleManager { - fmt.Fprintf(dockerCli.Out(), "Node %s is already a manager.\n", node.ID) + _, _ = fmt.Fprintf(dockerCli.Out(), "Node %s is already a manager.\n", node.ID) return errNoRoleChange } node.Spec.Role = swarm.NodeRoleManager return nil } success := func(nodeID string) { - fmt.Fprintf(dockerCli.Out(), "Node %s promoted to a manager in the swarm.\n", nodeID) + _, _ = fmt.Fprintf(dockerCli.Out(), "Node %s promoted to a manager in the swarm.\n", nodeID) } return updateNodes(ctx, dockerCli, nodes, promote, success) } diff --git a/cli/command/node/remove.go b/cli/command/node/remove.go index fedf34dec4c1..809c6b16facf 100644 --- a/cli/command/node/remove.go +++ b/cli/command/node/remove.go @@ -33,18 +33,18 @@ func newRemoveCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runRemove(ctx context.Context, dockerCli command.Cli, args []string, opts removeOptions) error { - client := dockerCli.Client() +func runRemove(ctx context.Context, dockerCLI command.Cli, nodeIDs []string, opts removeOptions) error { + apiClient := dockerCLI.Client() var errs []string - for _, nodeID := range args { - err := client.NodeRemove(ctx, nodeID, types.NodeRemoveOptions{Force: opts.force}) + for _, id := range nodeIDs { + err := apiClient.NodeRemove(ctx, id, types.NodeRemoveOptions{Force: opts.force}) if err != nil { errs = append(errs, err.Error()) continue } - fmt.Fprintf(dockerCli.Out(), "%s\n", nodeID) + _, _ = fmt.Fprintln(dockerCLI.Out(), id) } if len(errs) > 0 { From 53aed6119b0a3d4c9a36e152945da5c757869189 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 22:48:33 +0100 Subject: [PATCH 10/21] cli/command/plugin: minor cleanups: use Println, rename vars - use Println to print newline instead of custom format - use dockerCLI with Go's standard camelCase casing. - suppress some errors to make my IDE and linters happier Signed-off-by: Sebastiaan van Stijn --- cli/command/plugin/install.go | 18 +++++++++--------- cli/command/plugin/upgrade.go | 16 ++++++++-------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/cli/command/plugin/install.go b/cli/command/plugin/install.go index cb366fd406af..17d6ae384769 100644 --- a/cli/command/plugin/install.go +++ b/cli/command/plugin/install.go @@ -104,7 +104,7 @@ func buildPullConfig(ctx context.Context, dockerCli command.Cli, opts pluginOpti return options, nil } -func runInstall(ctx context.Context, dockerCli command.Cli, opts pluginOptions) error { +func runInstall(ctx context.Context, dockerCLI command.Cli, opts pluginOptions) error { var localName string if opts.localName != "" { aref, err := reference.ParseNormalizedNamed(opts.localName) @@ -117,11 +117,11 @@ func runInstall(ctx context.Context, dockerCli command.Cli, opts pluginOptions) localName = reference.FamiliarString(reference.TagNameOnly(aref)) } - options, err := buildPullConfig(ctx, dockerCli, opts, "plugin install") + options, err := buildPullConfig(ctx, dockerCLI, opts, "plugin install") if err != nil { return err } - responseBody, err := dockerCli.Client().PluginInstall(ctx, localName, options) + 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\"") @@ -129,19 +129,19 @@ func runInstall(ctx context.Context, dockerCli command.Cli, opts pluginOptions) return err } defer responseBody.Close() - if err := jsonstream.Display(ctx, responseBody, dockerCli.Out()); err != nil { + if err := jsonstream.Display(ctx, responseBody, dockerCLI.Out()); err != nil { return err } - fmt.Fprintf(dockerCli.Out(), "Installed plugin %s\n", opts.remote) // todo: return proper values from the API for this result + _, _ = fmt.Fprintln(dockerCLI.Out(), "Installed plugin", opts.remote) // todo: return proper values from the API for this result return nil } -func acceptPrivileges(dockerCli command.Cli, name string) func(ctx context.Context, privileges types.PluginPrivileges) (bool, error) { +func acceptPrivileges(dockerCLI command.Cli, name string) func(ctx context.Context, privileges types.PluginPrivileges) (bool, error) { return func(ctx context.Context, privileges types.PluginPrivileges) (bool, error) { - fmt.Fprintf(dockerCli.Out(), "Plugin %q is requesting the following privileges:\n", name) + _, _ = fmt.Fprintf(dockerCLI.Out(), "Plugin %q is requesting the following privileges:\n", name) for _, privilege := range privileges { - fmt.Fprintf(dockerCli.Out(), " - %s: %v\n", privilege.Name, privilege.Value) + _, _ = fmt.Fprintf(dockerCLI.Out(), " - %s: %v\n", privilege.Name, privilege.Value) } - return command.PromptForConfirmation(ctx, dockerCli.In(), dockerCli.Out(), "Do you grant the above permissions?") + return command.PromptForConfirmation(ctx, dockerCLI.In(), dockerCLI.Out(), "Do you grant the above permissions?") } } diff --git a/cli/command/plugin/upgrade.go b/cli/command/plugin/upgrade.go index 69b5c51b5f14..d83c75e317eb 100644 --- a/cli/command/plugin/upgrade.go +++ b/cli/command/plugin/upgrade.go @@ -36,8 +36,8 @@ func newUpgradeCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runUpgrade(ctx context.Context, dockerCli command.Cli, opts pluginOptions) error { - p, _, err := dockerCli.Client().PluginInspectWithRaw(ctx, opts.localName) +func runUpgrade(ctx context.Context, dockerCLI command.Cli, opts pluginOptions) error { + p, _, err := dockerCLI.Client().PluginInspectWithRaw(ctx, opts.localName) if err != nil { return errors.Errorf("error reading plugin data: %v", err) } @@ -62,9 +62,9 @@ func runUpgrade(ctx context.Context, dockerCli command.Cli, opts pluginOptions) } old = reference.TagNameOnly(old) - fmt.Fprintf(dockerCli.Out(), "Upgrading plugin %s from %s to %s\n", p.Name, reference.FamiliarString(old), reference.FamiliarString(remote)) + _, _ = fmt.Fprintf(dockerCLI.Out(), "Upgrading plugin %s from %s to %s\n", p.Name, reference.FamiliarString(old), reference.FamiliarString(remote)) if !opts.skipRemoteCheck && remote.String() != old.String() { - r, err := command.PromptForConfirmation(ctx, dockerCli.In(), dockerCli.Out(), "Plugin images do not match, are you sure?") + r, err := command.PromptForConfirmation(ctx, dockerCLI.In(), dockerCLI.Out(), "Plugin images do not match, are you sure?") if err != nil { return err } @@ -73,12 +73,12 @@ func runUpgrade(ctx context.Context, dockerCli command.Cli, opts pluginOptions) } } - options, err := buildPullConfig(ctx, dockerCli, opts, "plugin upgrade") + options, err := buildPullConfig(ctx, dockerCLI, opts, "plugin upgrade") if err != nil { return err } - responseBody, err := dockerCli.Client().PluginUpgrade(ctx, opts.localName, options) + 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`") @@ -86,9 +86,9 @@ func runUpgrade(ctx context.Context, dockerCli command.Cli, opts pluginOptions) return err } defer responseBody.Close() - if err := jsonstream.Display(ctx, responseBody, dockerCli.Out()); err != nil { + if err := jsonstream.Display(ctx, responseBody, dockerCLI.Out()); err != nil { return err } - fmt.Fprintf(dockerCli.Out(), "Upgraded plugin %s to %s\n", opts.localName, opts.remote) // todo: return proper values from the API for this result + _, _ = fmt.Fprintf(dockerCLI.Out(), "Upgraded plugin %s to %s\n", opts.localName, opts.remote) // todo: return proper values from the API for this result return nil } From 016dbef4493b081c57e727a3d1064ec613306fef Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 22:50:54 +0100 Subject: [PATCH 11/21] cli/command/registry: minor cleanups: use Println, rename vars - use Println to print newline instead of custom format - use dockerCLI with Go's standard camelCase casing. - suppress some errors to make my IDE and linters happier Signed-off-by: Sebastiaan van Stijn --- cli/command/registry/login.go | 12 ++++++------ cli/command/registry/logout.go | 14 +++++++------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/cli/command/registry/login.go b/cli/command/registry/login.go index 3facb75ccc7e..3fd91d56fd27 100644 --- a/cli/command/registry/login.go +++ b/cli/command/registry/login.go @@ -119,14 +119,14 @@ func runLogin(ctx context.Context, dockerCli command.Cli, opts loginOptions) err return nil } -func loginWithStoredCredentials(ctx context.Context, dockerCli command.Cli, authConfig registrytypes.AuthConfig) (msg string, _ error) { - _, _ = fmt.Fprintf(dockerCli.Out(), "Authenticating with existing credentials...\n") - response, err := dockerCli.Client().RegistryLogin(ctx, authConfig) +func loginWithStoredCredentials(ctx context.Context, dockerCLI command.Cli, authConfig registrytypes.AuthConfig) (msg string, _ error) { + _, _ = fmt.Fprintln(dockerCLI.Out(), "Authenticating with existing credentials...") + response, err := dockerCLI.Client().RegistryLogin(ctx, authConfig) if err != nil { if errdefs.IsUnauthorized(err) { - _, _ = fmt.Fprintf(dockerCli.Err(), "Stored credentials invalid or expired\n") + _, _ = fmt.Fprintln(dockerCLI.Err(), "Stored credentials invalid or expired") } else { - _, _ = fmt.Fprintf(dockerCli.Err(), "Login did not succeed, error: %s\n", err) + _, _ = fmt.Fprintln(dockerCLI.Err(), "Login did not succeed, error:", err) } } @@ -135,7 +135,7 @@ func loginWithStoredCredentials(ctx context.Context, dockerCli command.Cli, auth authConfig.IdentityToken = response.IdentityToken } - if err := storeCredentials(dockerCli.ConfigFile(), authConfig); err != nil { + if err := storeCredentials(dockerCLI.ConfigFile(), authConfig); err != nil { return "", err } diff --git a/cli/command/registry/logout.go b/cli/command/registry/logout.go index 4eb479f3e55b..6b5355335a0f 100644 --- a/cli/command/registry/logout.go +++ b/cli/command/registry/logout.go @@ -35,7 +35,7 @@ func NewLogoutCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runLogout(ctx context.Context, dockerCli command.Cli, serverAddress string) error { +func runLogout(ctx context.Context, dockerCLI command.Cli, serverAddress string) error { var isDefaultRegistry bool if serverAddress == "" { @@ -55,25 +55,25 @@ func runLogout(ctx context.Context, dockerCli command.Cli, serverAddress string) } if isDefaultRegistry { - store := dockerCli.ConfigFile().GetCredentialsStore(registry.IndexServer) + store := dockerCLI.ConfigFile().GetCredentialsStore(registry.IndexServer) if err := manager.NewManager(store).Logout(ctx); err != nil { - fmt.Fprintf(dockerCli.Err(), "WARNING: %v\n", err) + _, _ = fmt.Fprintln(dockerCLI.Err(), "WARNING:", err) } } - fmt.Fprintf(dockerCli.Out(), "Removing login credentials for %s\n", hostnameAddress) + _, _ = fmt.Fprintln(dockerCLI.Out(), "Removing login credentials for", hostnameAddress) errs := make(map[string]error) for _, r := range regsToLogout { - if err := dockerCli.ConfigFile().GetCredentialsStore(r).Erase(r); err != nil { + if err := dockerCLI.ConfigFile().GetCredentialsStore(r).Erase(r); err != nil { errs[r] = err } } // if at least one removal succeeded, report success. Otherwise report errors if len(errs) == len(regsToLogout) { - fmt.Fprintln(dockerCli.Err(), "WARNING: could not erase credentials:") + _, _ = fmt.Fprintln(dockerCLI.Err(), "WARNING: could not erase credentials:") for k, v := range errs { - fmt.Fprintf(dockerCli.Err(), "%s: %s\n", k, v) + _, _ = fmt.Fprintf(dockerCLI.Err(), "%s: %s\n", k, v) } } From aa74f931d3854582673c1b28c7fc8df2ed60742f Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 22:51:46 +0100 Subject: [PATCH 12/21] cli/command/service: minor cleanups: use Println, rename vars - use Println to print newline instead of custom format - use apiClient instead of client for the API client to prevent shadowing imports. - use dockerCLI with Go's standard camelCase casing. - suppress some errors to make my IDE and linters happier Signed-off-by: Sebastiaan van Stijn --- cli/command/service/create.go | 16 ++++++++-------- cli/command/service/remove.go | 10 +++++----- cli/command/service/rollback.go | 10 +++++----- cli/command/service/scale.go | 4 ++-- cli/command/service/trust.go | 2 +- cli/command/service/update.go | 14 +++++++------- 6 files changed, 28 insertions(+), 28 deletions(-) diff --git a/cli/command/service/create.go b/cli/command/service/create.go index b1bed6619594..ec3cf119531a 100644 --- a/cli/command/service/create.go +++ b/cli/command/service/create.go @@ -78,8 +78,8 @@ func newCreateCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runCreate(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet, opts *serviceOptions) error { - apiClient := dockerCli.Client() +func runCreate(ctx context.Context, dockerCLI command.Cli, flags *pflag.FlagSet, opts *serviceOptions) error { + apiClient := dockerCLI.Client() createOpts := types.ServiceCreateOptions{} service, err := opts.ToService(ctx, apiClient, flags) @@ -87,7 +87,7 @@ func runCreate(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet, return err } - if err = validateAPIVersion(service, dockerCli.Client().ClientVersion()); err != nil { + if err = validateAPIVersion(service, dockerCLI.Client().ClientVersion()); err != nil { return err } @@ -105,14 +105,14 @@ func runCreate(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet, return err } - if err := resolveServiceImageDigestContentTrust(dockerCli, &service); err != nil { + if err := resolveServiceImageDigestContentTrust(dockerCLI, &service); err != nil { return err } // only send auth if flag was set if opts.registryAuth { // Retrieve encoded auth token from the image reference - encodedAuth, err := command.RetrieveAuthTokenFromImage(dockerCli.ConfigFile(), opts.image) + encodedAuth, err := command.RetrieveAuthTokenFromImage(dockerCLI.ConfigFile(), opts.image) if err != nil { return err } @@ -130,16 +130,16 @@ func runCreate(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet, } for _, warning := range response.Warnings { - fmt.Fprintln(dockerCli.Err(), warning) + _, _ = fmt.Fprintln(dockerCLI.Err(), warning) } - fmt.Fprintf(dockerCli.Out(), "%s\n", response.ID) + _, _ = fmt.Fprintln(dockerCLI.Out(), response.ID) if opts.detach || versions.LessThan(apiClient.ClientVersion(), "1.29") { return nil } - return WaitOnService(ctx, dockerCli, response.ID, opts.quiet) + return WaitOnService(ctx, dockerCLI, response.ID, opts.quiet) } // setConfigs does double duty: it both sets the ConfigReferences of the diff --git a/cli/command/service/remove.go b/cli/command/service/remove.go index efbd5f9ad063..a191aa97bf35 100644 --- a/cli/command/service/remove.go +++ b/cli/command/service/remove.go @@ -29,17 +29,17 @@ func newRemoveCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runRemove(ctx context.Context, dockerCli command.Cli, sids []string) error { - client := dockerCli.Client() +func runRemove(ctx context.Context, dockerCLI command.Cli, serviceIDs []string) error { + apiClient := dockerCLI.Client() var errs []string - for _, sid := range sids { - err := client.ServiceRemove(ctx, sid) + for _, id := range serviceIDs { + err := apiClient.ServiceRemove(ctx, id) if err != nil { errs = append(errs, err.Error()) continue } - _, _ = fmt.Fprintf(dockerCli.Out(), "%s\n", sid) + _, _ = fmt.Fprintln(dockerCLI.Out(), id) } if len(errs) > 0 { return errors.New(strings.Join(errs, "\n")) diff --git a/cli/command/service/rollback.go b/cli/command/service/rollback.go index 06be65bb9084..f7efa2d4983b 100644 --- a/cli/command/service/rollback.go +++ b/cli/command/service/rollback.go @@ -34,8 +34,8 @@ func newRollbackCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runRollback(ctx context.Context, dockerCli command.Cli, options *serviceOptions, serviceID string) error { - apiClient := dockerCli.Client() +func runRollback(ctx context.Context, dockerCLI command.Cli, options *serviceOptions, serviceID string) error { + apiClient := dockerCLI.Client() service, _, err := apiClient.ServiceInspectWithRaw(ctx, serviceID, types.ServiceInspectOptions{}) if err != nil { @@ -53,14 +53,14 @@ func runRollback(ctx context.Context, dockerCli command.Cli, options *serviceOpt } for _, warning := range response.Warnings { - fmt.Fprintln(dockerCli.Err(), warning) + _, _ = fmt.Fprintln(dockerCLI.Err(), warning) } - fmt.Fprintf(dockerCli.Out(), "%s\n", serviceID) + _, _ = fmt.Fprintln(dockerCLI.Out(), serviceID) if options.detach || versions.LessThan(apiClient.ClientVersion(), "1.29") { return nil } - return WaitOnService(ctx, dockerCli, serviceID, options.quiet) + return WaitOnService(ctx, dockerCLI, serviceID, options.quiet) } diff --git a/cli/command/service/scale.go b/cli/command/service/scale.go index 688a3ff2d098..7d34359b6ed9 100644 --- a/cli/command/service/scale.go +++ b/cli/command/service/scale.go @@ -117,9 +117,9 @@ func runServiceScale(ctx context.Context, dockerCli command.Cli, serviceID strin } for _, warning := range response.Warnings { - fmt.Fprintln(dockerCli.Err(), warning) + _, _ = fmt.Fprintln(dockerCli.Err(), warning) } - fmt.Fprintf(dockerCli.Out(), "%s scaled to %d\n", serviceID, scale) + _, _ = fmt.Fprintf(dockerCli.Out(), "%s scaled to %d\n", serviceID, scale) return nil } diff --git a/cli/command/service/trust.go b/cli/command/service/trust.go index 3aec19002e18..cb32534d3728 100644 --- a/cli/command/service/trust.go +++ b/cli/command/service/trust.go @@ -73,7 +73,7 @@ func trustedResolveDigest(cli command.Cli, ref reference.NamedTagged) (reference return nil, trust.NotaryError(repoInfo.Name.Name(), errors.Errorf("No trust data for %s", reference.FamiliarString(ref))) } - logrus.Debugf("retrieving target for %s role\n", t.Role) + logrus.Debugf("retrieving target for %s role", t.Role) h, ok := t.Hashes["sha256"] if !ok { return nil, errors.New("no valid hash, expecting sha256") diff --git a/cli/command/service/update.go b/cli/command/service/update.go index 811dcf22b339..a9c52f5f54f5 100644 --- a/cli/command/service/update.go +++ b/cli/command/service/update.go @@ -129,8 +129,8 @@ func newListOptsVarWithValidator(validator opts.ValidatorFctType) *opts.ListOpts } //nolint:gocyclo -func runUpdate(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet, options *serviceOptions, serviceID string) error { - apiClient := dockerCli.Client() +func runUpdate(ctx context.Context, dockerCLI command.Cli, flags *pflag.FlagSet, options *serviceOptions, serviceID string) error { + apiClient := dockerCLI.Client() service, _, err := apiClient.ServiceInspectWithRaw(ctx, serviceID, types.ServiceInspectOptions{}) if err != nil { @@ -188,7 +188,7 @@ func runUpdate(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet, } if flags.Changed("image") { - if err := resolveServiceImageDigestContentTrust(dockerCli, spec); err != nil { + if err := resolveServiceImageDigestContentTrust(dockerCLI, spec); err != nil { return err } if !options.noResolveImage && versions.GreaterThanOrEqualTo(apiClient.ClientVersion(), "1.30") { @@ -225,7 +225,7 @@ func runUpdate(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet, // Retrieve encoded auth token from the image reference // This would be the old image if it didn't change in this update image := spec.TaskTemplate.ContainerSpec.Image - encodedAuth, err := command.RetrieveAuthTokenFromImage(dockerCli.ConfigFile(), image) + encodedAuth, err := command.RetrieveAuthTokenFromImage(dockerCLI.ConfigFile(), image) if err != nil { return err } @@ -242,16 +242,16 @@ func runUpdate(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet, } for _, warning := range response.Warnings { - fmt.Fprintln(dockerCli.Err(), warning) + _, _ = fmt.Fprintln(dockerCLI.Err(), warning) } - fmt.Fprintf(dockerCli.Out(), "%s\n", serviceID) + _, _ = fmt.Fprintln(dockerCLI.Out(), serviceID) if options.detach || versions.LessThan(apiClient.ClientVersion(), "1.29") { return nil } - return WaitOnService(ctx, dockerCli, serviceID, options.quiet) + return WaitOnService(ctx, dockerCLI, serviceID, options.quiet) } //nolint:gocyclo From 925b8fe34cf524a5a4187082c8887ce81643e1bc Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 22:56:30 +0100 Subject: [PATCH 13/21] cli/command/stack: minor cleanups: use Println, rename vars - use Println to print newline instead of custom format - use dockerCLI with Go's standard camelCase casing. - suppress some errors to make my IDE and linters happier Signed-off-by: Sebastiaan van Stijn --- cli/command/stack/loader/loader.go | 4 +-- cli/command/stack/services.go | 18 +++++------ cli/command/stack/swarm/deploy.go | 18 +++++------ cli/command/stack/swarm/deploy_composefile.go | 32 +++++++++---------- cli/command/stack/swarm/remove.go | 30 ++++++++--------- 5 files changed, 51 insertions(+), 51 deletions(-) diff --git a/cli/command/stack/loader/loader.go b/cli/command/stack/loader/loader.go index 5608a81c8fa3..8bf04b9aa5fb 100644 --- a/cli/command/stack/loader/loader.go +++ b/cli/command/stack/loader/loader.go @@ -40,13 +40,13 @@ func LoadComposefile(dockerCli command.Cli, opts options.Deploy) (*composetypes. unsupportedProperties := loader.GetUnsupportedProperties(dicts...) if len(unsupportedProperties) > 0 { - fmt.Fprintf(dockerCli.Err(), "Ignoring unsupported options: %s\n\n", + _, _ = fmt.Fprintf(dockerCli.Err(), "Ignoring unsupported options: %s\n\n", strings.Join(unsupportedProperties, ", ")) } deprecatedProperties := loader.GetDeprecatedProperties(dicts...) if len(deprecatedProperties) > 0 { - fmt.Fprintf(dockerCli.Err(), "Ignoring deprecated options:\n\n%s\n\n", + _, _ = fmt.Fprintf(dockerCli.Err(), "Ignoring deprecated options:\n\n%s\n\n", propertyWarnings(deprecatedProperties)) } return config, nil diff --git a/cli/command/stack/services.go b/cli/command/stack/services.go index 2638c985b0cc..f91ba71edfa5 100644 --- a/cli/command/stack/services.go +++ b/cli/command/stack/services.go @@ -52,28 +52,28 @@ func RunServices(ctx context.Context, dockerCli command.Cli, opts options.Servic return formatWrite(dockerCli, services, opts) } -func formatWrite(dockerCli command.Cli, services []swarmtypes.Service, opts options.Services) error { +func formatWrite(dockerCLI command.Cli, services []swarmtypes.Service, opts options.Services) error { // if no services in the stack, print message and exit 0 if len(services) == 0 { - _, _ = fmt.Fprintf(dockerCli.Err(), "Nothing found in stack: %s\n", opts.Namespace) + _, _ = fmt.Fprintln(dockerCLI.Err(), "Nothing found in stack:", opts.Namespace) return nil } sort.Slice(services, func(i, j int) bool { return sortorder.NaturalLess(services[i].Spec.Name, services[j].Spec.Name) }) - format := opts.Format - if len(format) == 0 { - if len(dockerCli.ConfigFile().ServicesFormat) > 0 && !opts.Quiet { - format = dockerCli.ConfigFile().ServicesFormat + f := opts.Format + if len(f) == 0 { + if len(dockerCLI.ConfigFile().ServicesFormat) > 0 && !opts.Quiet { + f = dockerCLI.ConfigFile().ServicesFormat } else { - format = formatter.TableFormatKey + f = formatter.TableFormatKey } } servicesCtx := formatter.Context{ - Output: dockerCli.Out(), - Format: service.NewListFormat(format, opts.Quiet), + Output: dockerCLI.Out(), + Format: service.NewListFormat(f, opts.Quiet), } return service.ListFormatWrite(servicesCtx, services) } diff --git a/cli/command/stack/swarm/deploy.go b/cli/command/stack/swarm/deploy.go index 7fe52f67fd72..bfd71f5f6566 100644 --- a/cli/command/stack/swarm/deploy.go +++ b/cli/command/stack/swarm/deploy.go @@ -23,22 +23,22 @@ const ( ) // RunDeploy is the swarm implementation of docker stack deploy -func RunDeploy(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet, opts *options.Deploy, cfg *composetypes.Config) error { +func RunDeploy(ctx context.Context, dockerCLI command.Cli, flags *pflag.FlagSet, opts *options.Deploy, cfg *composetypes.Config) error { if err := validateResolveImageFlag(opts); err != nil { return err } // client side image resolution should not be done when the supported // server version is older than 1.30 - if versions.LessThan(dockerCli.Client().ClientVersion(), "1.30") { + if versions.LessThan(dockerCLI.Client().ClientVersion(), "1.30") { opts.ResolveImage = ResolveImageNever } if opts.Detach && !flags.Changed("detach") { - fmt.Fprintln(dockerCli.Err(), "Since --detach=false was not specified, tasks will be created in the background.\n"+ + _, _ = fmt.Fprintln(dockerCLI.Err(), "Since --detach=false was not specified, tasks will be created in the background.\n"+ "In a future release, --detach=false will become the default.") } - return deployCompose(ctx, dockerCli, opts, cfg) + return deployCompose(ctx, dockerCLI, opts, cfg) } // validateResolveImageFlag validates the opts.resolveImage command line option @@ -67,12 +67,12 @@ func checkDaemonIsSwarmManager(ctx context.Context, dockerCli command.Cli) error } // pruneServices removes services that are no longer referenced in the source -func pruneServices(ctx context.Context, dockerCli command.Cli, namespace convert.Namespace, services map[string]struct{}) { - client := dockerCli.Client() +func pruneServices(ctx context.Context, dockerCCLI command.Cli, namespace convert.Namespace, services map[string]struct{}) { + apiClient := dockerCCLI.Client() - oldServices, err := getStackServices(ctx, client, namespace.Name()) + oldServices, err := getStackServices(ctx, apiClient, namespace.Name()) if err != nil { - fmt.Fprintf(dockerCli.Err(), "Failed to list services: %s\n", err) + _, _ = fmt.Fprintln(dockerCCLI.Err(), "Failed to list services:", err) } pruneServices := []swarm.Service{} @@ -81,5 +81,5 @@ func pruneServices(ctx context.Context, dockerCli command.Cli, namespace convert pruneServices = append(pruneServices, service) } } - removeServices(ctx, dockerCli, pruneServices) + removeServices(ctx, dockerCCLI, pruneServices) } diff --git a/cli/command/stack/swarm/deploy_composefile.go b/cli/command/stack/swarm/deploy_composefile.go index c163d6bf6914..cc6a9a0e5476 100644 --- a/cli/command/stack/swarm/deploy_composefile.go +++ b/cli/command/stack/swarm/deploy_composefile.go @@ -109,8 +109,8 @@ func validateExternalNetworks(ctx context.Context, apiClient client.NetworkAPICl return nil } -func createSecrets(ctx context.Context, dockerCli command.Cli, secrets []swarm.SecretSpec) error { - apiClient := dockerCli.Client() +func createSecrets(ctx context.Context, dockerCLI command.Cli, secrets []swarm.SecretSpec) error { + apiClient := dockerCLI.Client() for _, secretSpec := range secrets { secret, _, err := apiClient.SecretInspectWithRaw(ctx, secretSpec.Name) @@ -122,7 +122,7 @@ func createSecrets(ctx context.Context, dockerCli command.Cli, secrets []swarm.S } case errdefs.IsNotFound(err): // secret does not exist, then we create a new one. - fmt.Fprintf(dockerCli.Out(), "Creating secret %s\n", secretSpec.Name) + _, _ = fmt.Fprintln(dockerCLI.Out(), "Creating secret", secretSpec.Name) if _, err := apiClient.SecretCreate(ctx, secretSpec); err != nil { return fmt.Errorf("failed to create secret %s: %w", secretSpec.Name, err) } @@ -133,8 +133,8 @@ func createSecrets(ctx context.Context, dockerCli command.Cli, secrets []swarm.S return nil } -func createConfigs(ctx context.Context, dockerCli command.Cli, configs []swarm.ConfigSpec) error { - apiClient := dockerCli.Client() +func createConfigs(ctx context.Context, dockerCLI command.Cli, configs []swarm.ConfigSpec) error { + apiClient := dockerCLI.Client() for _, configSpec := range configs { config, _, err := apiClient.ConfigInspectWithRaw(ctx, configSpec.Name) @@ -146,7 +146,7 @@ func createConfigs(ctx context.Context, dockerCli command.Cli, configs []swarm.C } case errdefs.IsNotFound(err): // config does not exist, then we create a new one. - fmt.Fprintf(dockerCli.Out(), "Creating config %s\n", configSpec.Name) + _, _ = fmt.Fprintln(dockerCLI.Out(), "Creating config", configSpec.Name) if _, err := apiClient.ConfigCreate(ctx, configSpec); err != nil { return fmt.Errorf("failed to create config %s: %w", configSpec.Name, err) } @@ -157,8 +157,8 @@ func createConfigs(ctx context.Context, dockerCli command.Cli, configs []swarm.C return nil } -func createNetworks(ctx context.Context, dockerCli command.Cli, namespace convert.Namespace, networks map[string]network.CreateOptions) error { - apiClient := dockerCli.Client() +func createNetworks(ctx context.Context, dockerCLI command.Cli, namespace convert.Namespace, networks map[string]network.CreateOptions) error { + apiClient := dockerCLI.Client() existingNetworks, err := getStackNetworks(ctx, apiClient, namespace.Name()) if err != nil { @@ -179,7 +179,7 @@ func createNetworks(ctx context.Context, dockerCli command.Cli, namespace conver createOpts.Driver = defaultNetworkDriver } - fmt.Fprintf(dockerCli.Out(), "Creating network %s\n", name) + _, _ = fmt.Fprintln(dockerCLI.Out(), "Creating network", name) if _, err := apiClient.NetworkCreate(ctx, name, createOpts); err != nil { return fmt.Errorf("failed to create network %s: %w", name, err) } @@ -187,9 +187,9 @@ func createNetworks(ctx context.Context, dockerCli command.Cli, namespace conver return nil } -func deployServices(ctx context.Context, dockerCli command.Cli, services map[string]swarm.ServiceSpec, namespace convert.Namespace, sendAuth bool, resolveImage string) ([]string, error) { - apiClient := dockerCli.Client() - out := dockerCli.Out() +func deployServices(ctx context.Context, dockerCLI command.Cli, services map[string]swarm.ServiceSpec, namespace convert.Namespace, sendAuth bool, resolveImage string) ([]string, error) { + apiClient := dockerCLI.Client() + out := dockerCLI.Out() existingServices, err := getStackServices(ctx, apiClient, namespace.Name()) if err != nil { @@ -212,14 +212,14 @@ func deployServices(ctx context.Context, dockerCli command.Cli, services map[str if sendAuth { // Retrieve encoded auth token from the image reference - encodedAuth, err = command.RetrieveAuthTokenFromImage(dockerCli.ConfigFile(), image) + encodedAuth, err = command.RetrieveAuthTokenFromImage(dockerCLI.ConfigFile(), image) if err != nil { return nil, err } } if service, exists := existingServiceMap[name]; exists { - fmt.Fprintf(out, "Updating service %s (id: %s)\n", name, service.ID) + _, _ = fmt.Fprintf(out, "Updating service %s (id: %s)\n", name, service.ID) updateOpts := types.ServiceUpdateOptions{EncodedRegistryAuth: encodedAuth} @@ -259,12 +259,12 @@ func deployServices(ctx context.Context, dockerCli command.Cli, services map[str } for _, warning := range response.Warnings { - fmt.Fprintln(dockerCli.Err(), warning) + _, _ = fmt.Fprintln(dockerCLI.Err(), warning) } serviceIDs = append(serviceIDs, service.ID) } else { - fmt.Fprintf(out, "Creating service %s\n", name) + _, _ = fmt.Fprintln(out, "Creating service", name) createOpts := types.ServiceCreateOptions{EncodedRegistryAuth: encodedAuth} diff --git a/cli/command/stack/swarm/remove.go b/cli/command/stack/swarm/remove.go index 5641ea24bdf6..8c16a7fc042d 100644 --- a/cli/command/stack/swarm/remove.go +++ b/cli/command/stack/swarm/remove.go @@ -48,7 +48,7 @@ func RunRemove(ctx context.Context, dockerCli command.Cli, opts options.Remove) } if len(services)+len(networks)+len(secrets)+len(configs) == 0 { - _, _ = fmt.Fprintf(dockerCli.Err(), "Nothing found in stack: %s\n", namespace) + _, _ = fmt.Fprintln(dockerCli.Err(), "Nothing found in stack:", namespace) continue } @@ -82,26 +82,26 @@ func sortServiceByName(services []swarm.Service) func(i, j int) bool { } } -func removeServices(ctx context.Context, dockerCli command.Cli, services []swarm.Service) bool { +func removeServices(ctx context.Context, dockerCLI command.Cli, services []swarm.Service) bool { var hasError bool sort.Slice(services, sortServiceByName(services)) for _, service := range services { - fmt.Fprintf(dockerCli.Out(), "Removing service %s\n", service.Spec.Name) - if err := dockerCli.Client().ServiceRemove(ctx, service.ID); err != nil { + _, _ = fmt.Fprintln(dockerCLI.Out(), "Removing service", service.Spec.Name) + if err := dockerCLI.Client().ServiceRemove(ctx, service.ID); err != nil { hasError = true - fmt.Fprintf(dockerCli.Err(), "Failed to remove service %s: %s", service.ID, err) + _, _ = fmt.Fprintf(dockerCLI.Err(), "Failed to remove service %s: %s", service.ID, err) } } return hasError } -func removeNetworks(ctx context.Context, dockerCli command.Cli, networks []network.Summary) bool { +func removeNetworks(ctx context.Context, dockerCLI command.Cli, networks []network.Summary) bool { var hasError bool for _, nw := range networks { - fmt.Fprintf(dockerCli.Out(), "Removing network %s\n", nw.Name) - if err := dockerCli.Client().NetworkRemove(ctx, nw.ID); err != nil { + _, _ = fmt.Fprintln(dockerCLI.Out(), "Removing network", nw.Name) + if err := dockerCLI.Client().NetworkRemove(ctx, nw.ID); err != nil { hasError = true - fmt.Fprintf(dockerCli.Err(), "Failed to remove network %s: %s", nw.ID, err) + _, _ = fmt.Fprintf(dockerCLI.Err(), "Failed to remove network %s: %s", nw.ID, err) } } return hasError @@ -110,22 +110,22 @@ func removeNetworks(ctx context.Context, dockerCli command.Cli, networks []netwo func removeSecrets(ctx context.Context, dockerCli command.Cli, secrets []swarm.Secret) bool { var hasError bool for _, secret := range secrets { - fmt.Fprintf(dockerCli.Out(), "Removing secret %s\n", secret.Spec.Name) + _, _ = fmt.Fprintln(dockerCli.Out(), "Removing secret", secret.Spec.Name) if err := dockerCli.Client().SecretRemove(ctx, secret.ID); err != nil { hasError = true - fmt.Fprintf(dockerCli.Err(), "Failed to remove secret %s: %s", secret.ID, err) + _, _ = fmt.Fprintf(dockerCli.Err(), "Failed to remove secret %s: %s", secret.ID, err) } } return hasError } -func removeConfigs(ctx context.Context, dockerCli command.Cli, configs []swarm.Config) bool { +func removeConfigs(ctx context.Context, dockerCLI command.Cli, configs []swarm.Config) bool { var hasError bool for _, config := range configs { - fmt.Fprintf(dockerCli.Out(), "Removing config %s\n", config.Spec.Name) - if err := dockerCli.Client().ConfigRemove(ctx, config.ID); err != nil { + _, _ = fmt.Fprintln(dockerCLI.Out(), "Removing config", config.Spec.Name) + if err := dockerCLI.Client().ConfigRemove(ctx, config.ID); err != nil { hasError = true - fmt.Fprintf(dockerCli.Err(), "Failed to remove config %s: %s", config.ID, err) + _, _ = fmt.Fprintf(dockerCLI.Err(), "Failed to remove config %s: %s", config.ID, err) } } return hasError From 8c5e85d4cf8bb47f5cef07f77ae3320ff563d5b4 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 23:02:47 +0100 Subject: [PATCH 14/21] cli/command/swarm: minor cleanups: use Println, rename vars - use Println to print newline instead of custom format - use apiClient instead of client for the API client to prevent shadowing imports. - use dockerCLI with Go's standard camelCase casing. - suppress some errors to make my IDE and linters happier - fix some tests to work with "go test -update" - rewrite TestSwarmInit to use sub-tests Signed-off-by: Sebastiaan van Stijn --- cli/command/swarm/ca_test.go | 3 +- cli/command/swarm/init.go | 16 ++++---- cli/command/swarm/init_test.go | 34 ++++++++------- cli/command/swarm/join_test.go | 1 + cli/command/swarm/join_token.go | 41 +++++++++---------- cli/command/swarm/join_token_test.go | 1 + cli/command/swarm/leave_test.go | 4 ++ .../swarm/testdata/init-init-autolock.golden | 1 - cli/command/swarm/testdata/init-init.golden | 1 - .../unlockkeys-unlock-key-rotate.golden | 1 - cli/command/swarm/unlock_key.go | 18 ++++---- cli/command/swarm/unlock_key_test.go | 15 ++++--- cli/command/swarm/unlock_test.go | 9 +++- cli/command/swarm/update_test.go | 14 ++++--- 14 files changed, 92 insertions(+), 67 deletions(-) diff --git a/cli/command/swarm/ca_test.go b/cli/command/swarm/ca_test.go index 3096079e1757..88df4d164ecd 100644 --- a/cli/command/swarm/ca_test.go +++ b/cli/command/swarm/ca_test.go @@ -143,9 +143,10 @@ func TestDisplayTrustRootInvalidFlags(t *testing.T) { }, nil }, })) - assert.Check(t, cmd.Flags().Parse(testCase.args)) + cmd.SetArgs([]string{}) cmd.SetOut(io.Discard) cmd.SetErr(io.Discard) + assert.Check(t, cmd.Flags().Parse(testCase.args)) assert.ErrorContains(t, cmd.Execute(), testCase.errorMsg) } } diff --git a/cli/command/swarm/init.go b/cli/command/swarm/init.go index 8612a5f8f1b2..bc3160554e85 100644 --- a/cli/command/swarm/init.go +++ b/cli/command/swarm/init.go @@ -65,8 +65,8 @@ func newInitCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runInit(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet, opts initOptions) error { - client := dockerCli.Client() +func runInit(ctx context.Context, dockerCLI command.Cli, flags *pflag.FlagSet, opts initOptions) error { + apiClient := dockerCLI.Client() defaultAddrPool := make([]string, 0, len(opts.defaultAddrPools)) for _, p := range opts.defaultAddrPools { @@ -93,7 +93,7 @@ func runInit(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet, o } } - nodeID, err := client.SwarmInit(ctx, req) + nodeID, err := apiClient.SwarmInit(ctx, req) if err != nil { if strings.Contains(err.Error(), "could not choose an IP address to advertise") || strings.Contains(err.Error(), "could not find the system's IP address") { return errors.New(err.Error() + " - specify one with --advertise-addr") @@ -101,20 +101,20 @@ func runInit(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet, o return err } - fmt.Fprintf(dockerCli.Out(), "Swarm initialized: current node (%s) is now a manager.\n\n", nodeID) + _, _ = fmt.Fprintf(dockerCLI.Out(), "Swarm initialized: current node (%s) is now a manager.\n\n", nodeID) - if err := printJoinCommand(ctx, dockerCli, nodeID, true, false); err != nil { + if err := printJoinCommand(ctx, dockerCLI, nodeID, true, false); err != nil { return err } - fmt.Fprint(dockerCli.Out(), "To add a manager to this swarm, run 'docker swarm join-token manager' and follow the instructions.\n\n") + _, _ = fmt.Fprintln(dockerCLI.Out(), "To add a manager to this swarm, run 'docker swarm join-token manager' and follow the instructions.") if req.AutoLockManagers { - unlockKeyResp, err := client.SwarmGetUnlockKey(ctx) + unlockKeyResp, err := apiClient.SwarmGetUnlockKey(ctx) if err != nil { return errors.Wrap(err, "could not fetch unlock key") } - printUnlockCommand(dockerCli.Out(), unlockKeyResp.UnlockKey) + printUnlockCommand(dockerCLI.Out(), unlockKeyResp.UnlockKey) } return nil diff --git a/cli/command/swarm/init_test.go b/cli/command/swarm/init_test.go index cd8fafac1a5b..f76ae79c6611 100644 --- a/cli/command/swarm/init_test.go +++ b/cli/command/swarm/init_test.go @@ -71,11 +71,12 @@ func TestSwarmInitErrorOnAPIFailure(t *testing.T) { swarmGetUnlockKeyFunc: tc.swarmGetUnlockKeyFunc, nodeInspectFunc: tc.nodeInspectFunc, })) - for key, value := range tc.flags { - cmd.Flags().Set(key, value) - } + cmd.SetArgs([]string{}) cmd.SetOut(io.Discard) cmd.SetErr(io.Discard) + for k, v := range tc.flags { + assert.Check(t, cmd.Flags().Set(k, v)) + } assert.Error(t, cmd.Execute(), tc.expectedError) }) } @@ -112,17 +113,22 @@ func TestSwarmInit(t *testing.T) { }, } for _, tc := range testCases { - cli := test.NewFakeCli(&fakeClient{ - swarmInitFunc: tc.swarmInitFunc, - swarmInspectFunc: tc.swarmInspectFunc, - swarmGetUnlockKeyFunc: tc.swarmGetUnlockKeyFunc, - nodeInspectFunc: tc.nodeInspectFunc, + t.Run(tc.name, func(t *testing.T) { + cli := test.NewFakeCli(&fakeClient{ + swarmInitFunc: tc.swarmInitFunc, + swarmInspectFunc: tc.swarmInspectFunc, + swarmGetUnlockKeyFunc: tc.swarmGetUnlockKeyFunc, + nodeInspectFunc: tc.nodeInspectFunc, + }) + cmd := newInitCommand(cli) + cmd.SetArgs([]string{}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + for k, v := range tc.flags { + assert.Check(t, cmd.Flags().Set(k, v)) + } + assert.NilError(t, cmd.Execute()) + golden.Assert(t, cli.OutBuffer().String(), fmt.Sprintf("init-%s.golden", tc.name)) }) - cmd := newInitCommand(cli) - for key, value := range tc.flags { - cmd.Flags().Set(key, value) - } - assert.NilError(t, cmd.Execute()) - golden.Assert(t, cli.OutBuffer().String(), fmt.Sprintf("init-%s.golden", tc.name)) } } diff --git a/cli/command/swarm/join_test.go b/cli/command/swarm/join_test.go index 02ff901ac375..b6d092e8ce7a 100644 --- a/cli/command/swarm/join_test.go +++ b/cli/command/swarm/join_test.go @@ -23,6 +23,7 @@ func TestSwarmJoinErrors(t *testing.T) { }{ { name: "not-enough-args", + args: []string{}, expectedError: "requires 1 argument", }, { diff --git a/cli/command/swarm/join_token.go b/cli/command/swarm/join_token.go index de0611700a27..08a5d8766c2b 100644 --- a/cli/command/swarm/join_token.go +++ b/cli/command/swarm/join_token.go @@ -41,7 +41,7 @@ func newJoinTokenCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runJoinToken(ctx context.Context, dockerCli command.Cli, opts joinTokenOptions) error { +func runJoinToken(ctx context.Context, dockerCLI command.Cli, opts joinTokenOptions) error { worker := opts.role == "worker" manager := opts.role == "manager" @@ -49,72 +49,71 @@ func runJoinToken(ctx context.Context, dockerCli command.Cli, opts joinTokenOpti return errors.New("unknown role " + opts.role) } - client := dockerCli.Client() + apiClient := dockerCLI.Client() if opts.rotate { - flags := swarm.UpdateFlags{ - RotateWorkerToken: worker, - RotateManagerToken: manager, - } - - sw, err := client.SwarmInspect(ctx) + sw, err := apiClient.SwarmInspect(ctx) if err != nil { return err } - if err := client.SwarmUpdate(ctx, sw.Version, sw.Spec, flags); err != nil { + err = apiClient.SwarmUpdate(ctx, sw.Version, sw.Spec, swarm.UpdateFlags{ + RotateWorkerToken: worker, + RotateManagerToken: manager, + }) + if err != nil { return err } if !opts.quiet { - fmt.Fprintf(dockerCli.Out(), "Successfully rotated %s join token.\n\n", opts.role) + _, _ = fmt.Fprintf(dockerCLI.Out(), "Successfully rotated %s join token.\n\n", opts.role) } } // second SwarmInspect in this function, // this is necessary since SwarmUpdate after first changes the join tokens - sw, err := client.SwarmInspect(ctx) + sw, err := apiClient.SwarmInspect(ctx) if err != nil { return err } if opts.quiet && worker { - fmt.Fprintln(dockerCli.Out(), sw.JoinTokens.Worker) + _, _ = fmt.Fprintln(dockerCLI.Out(), sw.JoinTokens.Worker) return nil } if opts.quiet && manager { - fmt.Fprintln(dockerCli.Out(), sw.JoinTokens.Manager) + _, _ = fmt.Fprintln(dockerCLI.Out(), sw.JoinTokens.Manager) return nil } - info, err := client.Info(ctx) + info, err := apiClient.Info(ctx) if err != nil { return err } - return printJoinCommand(ctx, dockerCli, info.Swarm.NodeID, worker, manager) + return printJoinCommand(ctx, dockerCLI, info.Swarm.NodeID, worker, manager) } -func printJoinCommand(ctx context.Context, dockerCli command.Cli, nodeID string, worker bool, manager bool) error { - client := dockerCli.Client() +func printJoinCommand(ctx context.Context, dockerCLI command.Cli, nodeID string, worker bool, manager bool) error { + apiClient := dockerCLI.Client() - node, _, err := client.NodeInspectWithRaw(ctx, nodeID) + node, _, err := apiClient.NodeInspectWithRaw(ctx, nodeID) if err != nil { return err } - sw, err := client.SwarmInspect(ctx) + sw, err := apiClient.SwarmInspect(ctx) if err != nil { return err } if node.ManagerStatus != nil { if worker { - fmt.Fprintf(dockerCli.Out(), "To add a worker to this swarm, run the following command:\n\n docker swarm join --token %s %s\n\n", sw.JoinTokens.Worker, node.ManagerStatus.Addr) + _, _ = fmt.Fprintf(dockerCLI.Out(), "To add a worker to this swarm, run the following command:\n\n docker swarm join --token %s %s\n\n", sw.JoinTokens.Worker, node.ManagerStatus.Addr) } if manager { - fmt.Fprintf(dockerCli.Out(), "To add a manager to this swarm, run the following command:\n\n docker swarm join --token %s %s\n\n", sw.JoinTokens.Manager, node.ManagerStatus.Addr) + _, _ = fmt.Fprintf(dockerCLI.Out(), "To add a manager to this swarm, run the following command:\n\n docker swarm join --token %s %s\n\n", sw.JoinTokens.Manager, node.ManagerStatus.Addr) } } diff --git a/cli/command/swarm/join_token_test.go b/cli/command/swarm/join_token_test.go index 76db830b64f8..7c819b6fea71 100644 --- a/cli/command/swarm/join_token_test.go +++ b/cli/command/swarm/join_token_test.go @@ -27,6 +27,7 @@ func TestSwarmJoinTokenErrors(t *testing.T) { }{ { name: "not-enough-args", + args: []string{}, expectedError: "requires 1 argument", }, { diff --git a/cli/command/swarm/leave_test.go b/cli/command/swarm/leave_test.go index 19dd1dc1f6ca..a389d90fc7f7 100644 --- a/cli/command/swarm/leave_test.go +++ b/cli/command/swarm/leave_test.go @@ -25,6 +25,7 @@ func TestSwarmLeaveErrors(t *testing.T) { }, { name: "leave-failed", + args: []string{}, swarmLeaveFunc: func() error { return errors.New("error leaving the swarm") }, @@ -48,6 +49,9 @@ func TestSwarmLeaveErrors(t *testing.T) { func TestSwarmLeave(t *testing.T) { cli := test.NewFakeCli(&fakeClient{}) cmd := newLeaveCommand(cli) + cmd.SetArgs([]string{}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) assert.NilError(t, cmd.Execute()) assert.Check(t, is.Equal("Node left the swarm.", strings.TrimSpace(cli.OutBuffer().String()))) } diff --git a/cli/command/swarm/testdata/init-init-autolock.golden b/cli/command/swarm/testdata/init-init-autolock.golden index 7a47a5f2ba7d..96e3705c7375 100644 --- a/cli/command/swarm/testdata/init-init-autolock.golden +++ b/cli/command/swarm/testdata/init-init-autolock.golden @@ -1,7 +1,6 @@ Swarm initialized: current node (nodeID) is now a manager. To add a manager to this swarm, run 'docker swarm join-token manager' and follow the instructions. - To unlock a swarm manager after it restarts, run the `docker swarm unlock` command and provide the following key: diff --git a/cli/command/swarm/testdata/init-init.golden b/cli/command/swarm/testdata/init-init.golden index 6e82be010ee9..b2c727f0a199 100644 --- a/cli/command/swarm/testdata/init-init.golden +++ b/cli/command/swarm/testdata/init-init.golden @@ -1,4 +1,3 @@ Swarm initialized: current node (nodeID) is now a manager. To add a manager to this swarm, run 'docker swarm join-token manager' and follow the instructions. - diff --git a/cli/command/swarm/testdata/unlockkeys-unlock-key-rotate.golden b/cli/command/swarm/testdata/unlockkeys-unlock-key-rotate.golden index 96190e99b53d..b3fe20e2cec5 100644 --- a/cli/command/swarm/testdata/unlockkeys-unlock-key-rotate.golden +++ b/cli/command/swarm/testdata/unlockkeys-unlock-key-rotate.golden @@ -1,5 +1,4 @@ Successfully rotated manager unlock key. - To unlock a swarm manager after it restarts, run the `docker swarm unlock` command and provide the following key: diff --git a/cli/command/swarm/unlock_key.go b/cli/command/swarm/unlock_key.go index 7caee23728cf..af62e0b09a95 100644 --- a/cli/command/swarm/unlock_key.go +++ b/cli/command/swarm/unlock_key.go @@ -42,13 +42,13 @@ func newUnlockKeyCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runUnlockKey(ctx context.Context, dockerCli command.Cli, opts unlockKeyOptions) error { - client := dockerCli.Client() +func runUnlockKey(ctx context.Context, dockerCLI command.Cli, opts unlockKeyOptions) error { + apiClient := dockerCLI.Client() if opts.rotate { flags := swarm.UpdateFlags{RotateManagerUnlockKey: true} - sw, err := client.SwarmInspect(ctx) + sw, err := apiClient.SwarmInspect(ctx) if err != nil { return err } @@ -57,16 +57,16 @@ func runUnlockKey(ctx context.Context, dockerCli command.Cli, opts unlockKeyOpti return errors.New("cannot rotate because autolock is not turned on") } - if err := client.SwarmUpdate(ctx, sw.Version, sw.Spec, flags); err != nil { + if err := apiClient.SwarmUpdate(ctx, sw.Version, sw.Spec, flags); err != nil { return err } if !opts.quiet { - fmt.Fprintf(dockerCli.Out(), "Successfully rotated manager unlock key.\n\n") + _, _ = fmt.Fprintln(dockerCLI.Out(), "Successfully rotated manager unlock key.") } } - unlockKeyResp, err := client.SwarmGetUnlockKey(ctx) + unlockKeyResp, err := apiClient.SwarmGetUnlockKey(ctx) if err != nil { return errors.Wrap(err, "could not fetch unlock key") } @@ -76,17 +76,17 @@ func runUnlockKey(ctx context.Context, dockerCli command.Cli, opts unlockKeyOpti } if opts.quiet { - fmt.Fprintln(dockerCli.Out(), unlockKeyResp.UnlockKey) + _, _ = fmt.Fprintln(dockerCLI.Out(), unlockKeyResp.UnlockKey) return nil } - printUnlockCommand(dockerCli.Out(), unlockKeyResp.UnlockKey) + printUnlockCommand(dockerCLI.Out(), unlockKeyResp.UnlockKey) return nil } func printUnlockCommand(out io.Writer, unlockKey string) { if len(unlockKey) > 0 { - fmt.Fprintf(out, "To unlock a swarm manager after it restarts, "+ + _, _ = fmt.Fprintf(out, "To unlock a swarm manager after it restarts, "+ "run the `docker swarm unlock`\ncommand and provide the following key:\n\n %s\n\n"+ "Remember to store this key in a password manager, since without it you\n"+ "will not be able to restart the manager.\n", unlockKey) diff --git a/cli/command/swarm/unlock_key_test.go b/cli/command/swarm/unlock_key_test.go index e55567b50ee2..05bd220d43da 100644 --- a/cli/command/swarm/unlock_key_test.go +++ b/cli/command/swarm/unlock_key_test.go @@ -87,12 +87,16 @@ func TestSwarmUnlockKeyErrors(t *testing.T) { swarmUpdateFunc: tc.swarmUpdateFunc, swarmGetUnlockKeyFunc: tc.swarmGetUnlockKeyFunc, })) - cmd.SetArgs(tc.args) - for k, v := range tc.flags { - assert.Check(t, cmd.Flags().Set(k, v)) + if tc.args == nil { + cmd.SetArgs([]string{}) + } else { + cmd.SetArgs(tc.args) } cmd.SetOut(io.Discard) cmd.SetErr(io.Discard) + for k, v := range tc.flags { + assert.Check(t, cmd.Flags().Set(k, v)) + } assert.ErrorContains(t, cmd.Execute(), tc.expectedError) }) } @@ -101,7 +105,6 @@ func TestSwarmUnlockKeyErrors(t *testing.T) { func TestSwarmUnlockKey(t *testing.T) { testCases := []struct { name string - args []string flags map[string]string swarmInspectFunc func() (swarm.Swarm, error) swarmUpdateFunc func(swarm swarm.Spec, flags swarm.UpdateFlags) error @@ -164,7 +167,9 @@ func TestSwarmUnlockKey(t *testing.T) { swarmGetUnlockKeyFunc: tc.swarmGetUnlockKeyFunc, }) cmd := newUnlockKeyCommand(cli) - cmd.SetArgs(tc.args) + cmd.SetArgs([]string{}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) for k, v := range tc.flags { assert.Check(t, cmd.Flags().Set(k, v)) } diff --git a/cli/command/swarm/unlock_test.go b/cli/command/swarm/unlock_test.go index f0cdfdfa8e66..b437cac07fd7 100644 --- a/cli/command/swarm/unlock_test.go +++ b/cli/command/swarm/unlock_test.go @@ -70,7 +70,11 @@ func TestSwarmUnlockErrors(t *testing.T) { infoFunc: tc.infoFunc, swarmUnlockFunc: tc.swarmUnlockFunc, })) - cmd.SetArgs(tc.args) + if tc.args == nil { + cmd.SetArgs([]string{}) + } else { + cmd.SetArgs(tc.args) + } cmd.SetOut(io.Discard) cmd.SetErr(io.Discard) assert.ErrorContains(t, cmd.Execute(), tc.expectedError) @@ -97,5 +101,8 @@ func TestSwarmUnlock(t *testing.T) { }) dockerCli.SetIn(streams.NewIn(io.NopCloser(strings.NewReader(input)))) cmd := newUnlockCommand(dockerCli) + cmd.SetArgs([]string{}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) assert.NilError(t, cmd.Execute()) } diff --git a/cli/command/swarm/update_test.go b/cli/command/swarm/update_test.go index 98dd0b7abe8c..d6246de81093 100644 --- a/cli/command/swarm/update_test.go +++ b/cli/command/swarm/update_test.go @@ -72,12 +72,16 @@ func TestSwarmUpdateErrors(t *testing.T) { swarmUpdateFunc: tc.swarmUpdateFunc, swarmGetUnlockKeyFunc: tc.swarmGetUnlockKeyFunc, })) - cmd.SetArgs(tc.args) - for key, value := range tc.flags { - assert.Check(t, cmd.Flags().Set(key, value)) + if tc.args == nil { + cmd.SetArgs([]string{}) + } else { + cmd.SetArgs(tc.args) } cmd.SetOut(io.Discard) cmd.SetErr(io.Discard) + for k, v := range tc.flags { + assert.Check(t, cmd.Flags().Set(k, v)) + } assert.ErrorContains(t, cmd.Execute(), tc.expectedError) }) } @@ -180,8 +184,8 @@ func TestSwarmUpdate(t *testing.T) { } else { cmd.SetArgs(tc.args) } - for key, value := range tc.flags { - assert.Check(t, cmd.Flags().Set(key, value)) + for k, v := range tc.flags { + assert.Check(t, cmd.Flags().Set(k, v)) } cmd.SetOut(cli.OutBuffer()) assert.NilError(t, cmd.Execute()) From 5cfc89c1c2e6d8221af2e4a0c5ca323b403ac382 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 23:06:56 +0100 Subject: [PATCH 15/21] cli/command/system: minor cleanups: use Println, rename vars - use Println to print newline instead of custom format - use dockerCLI with Go's standard camelCase casing. - suppress some errors to make my IDE and linters happier Signed-off-by: Sebastiaan van Stijn --- cli/command/system/events.go | 10 +++++----- cli/command/system/inspect.go | 28 ++++++++++++++-------------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/cli/command/system/events.go b/cli/command/system/events.go index 9b5f965e5b3c..d83d36351e33 100644 --- a/cli/command/system/events.go +++ b/cli/command/system/events.go @@ -121,12 +121,12 @@ const rfc3339NanoFixed = "2006-01-02T15:04:05.000000000Z07:00" // Actor attributes are printed at the end if the actor has any. func prettyPrintEvent(out io.Writer, event events.Message) error { if event.TimeNano != 0 { - fmt.Fprintf(out, "%s ", time.Unix(0, event.TimeNano).Format(rfc3339NanoFixed)) + _, _ = fmt.Fprintf(out, "%s ", time.Unix(0, event.TimeNano).Format(rfc3339NanoFixed)) } else if event.Time != 0 { - fmt.Fprintf(out, "%s ", time.Unix(event.Time, 0).Format(rfc3339NanoFixed)) + _, _ = fmt.Fprintf(out, "%s ", time.Unix(event.Time, 0).Format(rfc3339NanoFixed)) } - fmt.Fprintf(out, "%s %s %s", event.Type, event.Action, event.Actor.ID) + _, _ = fmt.Fprintf(out, "%s %s %s", event.Type, event.Action, event.Actor.ID) if len(event.Actor.Attributes) > 0 { var attrs []string @@ -139,9 +139,9 @@ func prettyPrintEvent(out io.Writer, event events.Message) error { v := event.Actor.Attributes[k] attrs = append(attrs, k+"="+v) } - fmt.Fprintf(out, " (%s)", strings.Join(attrs, ", ")) + _, _ = fmt.Fprintf(out, " (%s)", strings.Join(attrs, ", ")) } - fmt.Fprint(out, "\n") + _, _ = fmt.Fprint(out, "\n") return nil } diff --git a/cli/command/system/inspect.go b/cli/command/system/inspect.go index 0af387946fd7..11976655585f 100644 --- a/cli/command/system/inspect.go +++ b/cli/command/system/inspect.go @@ -120,7 +120,7 @@ func inspectConfig(ctx context.Context, dockerCLI command.Cli) inspect.GetRefFun } } -func inspectAll(ctx context.Context, dockerCli command.Cli, getSize bool, typeConstraint string) inspect.GetRefFunc { +func inspectAll(ctx context.Context, dockerCLI command.Cli, getSize bool, typeConstraint string) inspect.GetRefFunc { inspectAutodetect := []struct { objectType string isSizeSupported bool @@ -130,57 +130,57 @@ func inspectAll(ctx context.Context, dockerCli command.Cli, getSize bool, typeCo { objectType: "container", isSizeSupported: true, - objectInspector: inspectContainers(ctx, dockerCli, getSize), + objectInspector: inspectContainers(ctx, dockerCLI, getSize), }, { objectType: "image", - objectInspector: inspectImages(ctx, dockerCli), + objectInspector: inspectImages(ctx, dockerCLI), }, { objectType: "network", - objectInspector: inspectNetwork(ctx, dockerCli), + objectInspector: inspectNetwork(ctx, dockerCLI), }, { objectType: "volume", - objectInspector: inspectVolume(ctx, dockerCli), + objectInspector: inspectVolume(ctx, dockerCLI), }, { objectType: "service", isSwarmObject: true, - objectInspector: inspectService(ctx, dockerCli), + objectInspector: inspectService(ctx, dockerCLI), }, { objectType: "task", isSwarmObject: true, - objectInspector: inspectTasks(ctx, dockerCli), + objectInspector: inspectTasks(ctx, dockerCLI), }, { objectType: "node", isSwarmObject: true, - objectInspector: inspectNode(ctx, dockerCli), + objectInspector: inspectNode(ctx, dockerCLI), }, { objectType: "plugin", - objectInspector: inspectPlugin(ctx, dockerCli), + objectInspector: inspectPlugin(ctx, dockerCLI), }, { objectType: "secret", isSwarmObject: true, - objectInspector: inspectSecret(ctx, dockerCli), + objectInspector: inspectSecret(ctx, dockerCLI), }, { objectType: "config", isSwarmObject: true, - objectInspector: inspectConfig(ctx, dockerCli), + objectInspector: inspectConfig(ctx, dockerCLI), }, } // isSwarmManager does an Info API call to verify that the daemon is // a swarm manager. isSwarmManager := func() bool { - info, err := dockerCli.Client().Info(ctx) + info, err := dockerCLI.Client().Info(ctx) if err != nil { - fmt.Fprintln(dockerCli.Err(), err) + _, _ = fmt.Fprintln(dockerCLI.Err(), err) return false } return info.Swarm.ControlAvailable @@ -219,7 +219,7 @@ func inspectAll(ctx context.Context, dockerCli command.Cli, getSize bool, typeCo return v, raw, err } if getSize && !inspectData.isSizeSupported { - fmt.Fprintf(dockerCli.Err(), "WARNING: --size ignored for %s\n", inspectData.objectType) + _, _ = fmt.Fprintln(dockerCLI.Err(), "WARNING: --size ignored for", inspectData.objectType) } return v, raw, err } From 299aae041907b499ea719137c3c66d3a7dd5ec36 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 23:12:56 +0100 Subject: [PATCH 16/21] cli/command/trust: minor cleanups: use Println, rename vars - use Println to print newline instead of custom format - use dockerCLI with Go's standard camelCase casing. - suppress some errors to make my IDE and linters happier Signed-off-by: Sebastiaan van Stijn --- cli/command/trust/inspect.go | 2 +- cli/command/trust/inspect_pretty.go | 20 ++++++++++---------- cli/command/trust/key_generate.go | 6 +++--- cli/command/trust/key_load.go | 4 ++-- cli/command/trust/sign.go | 17 ++++++++--------- cli/command/trust/signer_add.go | 10 +++++----- cli/command/trust/signer_remove.go | 6 +++--- 7 files changed, 32 insertions(+), 33 deletions(-) diff --git a/cli/command/trust/inspect.go b/cli/command/trust/inspect.go index 6de786623427..574a1aad7b2d 100644 --- a/cli/command/trust/inspect.go +++ b/cli/command/trust/inspect.go @@ -53,7 +53,7 @@ func runInspect(ctx context.Context, dockerCLI command.Cli, opts inspectOptions) // Additional separator between the inspection output of each image if index < len(opts.remotes)-1 { - fmt.Fprint(dockerCLI.Out(), "\n\n") + _, _ = fmt.Fprint(dockerCLI.Out(), "\n\n") } } diff --git a/cli/command/trust/inspect_pretty.go b/cli/command/trust/inspect_pretty.go index f0e16006bad6..b0b5e8c51c84 100644 --- a/cli/command/trust/inspect_pretty.go +++ b/cli/command/trust/inspect_pretty.go @@ -12,34 +12,34 @@ import ( "github.com/theupdateframework/notary/client" ) -func prettyPrintTrustInfo(ctx context.Context, cli command.Cli, remote string) error { - signatureRows, adminRolesWithSigs, delegationRoles, err := lookupTrustInfo(ctx, cli, remote) +func prettyPrintTrustInfo(ctx context.Context, dockerCLI command.Cli, remote string) error { + signatureRows, adminRolesWithSigs, delegationRoles, err := lookupTrustInfo(ctx, dockerCLI, remote) if err != nil { return err } if len(signatureRows) > 0 { - fmt.Fprintf(cli.Out(), "\nSignatures for %s\n\n", remote) + _, _ = fmt.Fprintf(dockerCLI.Out(), "\nSignatures for %s\n\n", remote) - if err := printSignatures(cli.Out(), signatureRows); err != nil { + if err := printSignatures(dockerCLI.Out(), signatureRows); err != nil { return err } } else { - fmt.Fprintf(cli.Out(), "\nNo signatures for %s\n\n", remote) + _, _ = fmt.Fprintf(dockerCLI.Out(), "\nNo signatures for %s\n\n", remote) } signerRoleToKeyIDs := getDelegationRoleToKeyMap(delegationRoles) // If we do not have additional signers, do not display if len(signerRoleToKeyIDs) > 0 { - fmt.Fprintf(cli.Out(), "\nList of signers and their keys for %s\n\n", remote) - if err := printSignerInfo(cli.Out(), signerRoleToKeyIDs); err != nil { + _, _ = fmt.Fprintf(dockerCLI.Out(), "\nList of signers and their keys for %s\n\n", remote) + if err := printSignerInfo(dockerCLI.Out(), signerRoleToKeyIDs); err != nil { return err } } // This will always have the root and targets information - fmt.Fprintf(cli.Out(), "\nAdministrative keys for %s\n\n", remote) - printSortedAdminKeys(cli.Out(), adminRolesWithSigs) + _, _ = fmt.Fprintf(dockerCLI.Out(), "\nAdministrative keys for %s\n\n", remote) + printSortedAdminKeys(dockerCLI.Out(), adminRolesWithSigs) return nil } @@ -47,7 +47,7 @@ func printSortedAdminKeys(out io.Writer, adminRoles []client.RoleWithSignatures) sort.Slice(adminRoles, func(i, j int) bool { return adminRoles[i].Name > adminRoles[j].Name }) for _, adminRole := range adminRoles { if formattedAdminRole := formatAdminRole(adminRole); formattedAdminRole != "" { - fmt.Fprintf(out, " %s", formattedAdminRole) + _, _ = fmt.Fprintf(out, " %s", formattedAdminRole) } } } diff --git a/cli/command/trust/key_generate.go b/cli/command/trust/key_generate.go index 39eb0cb1b998..3a137f1276e1 100644 --- a/cli/command/trust/key_generate.go +++ b/cli/command/trust/key_generate.go @@ -78,7 +78,7 @@ func validateAndGenerateKey(streams command.Streams, keyName string, workingDir if err := validateKeyArgs(keyName, workingDir); err != nil { return err } - fmt.Fprintf(streams.Out(), "Generating key for %s...\n", keyName) + _, _ = fmt.Fprintf(streams.Out(), "Generating key for %s...\n", keyName) // Automatically load the private key to local storage for use privKeyFileStore, err := trustmanager.NewKeyFileStore(trust.GetTrustDirectory(), freshPassRetGetter()) if err != nil { @@ -87,7 +87,7 @@ func validateAndGenerateKey(streams command.Streams, keyName string, workingDir pubPEM, err := generateKeyAndOutputPubPEM(keyName, privKeyFileStore) if err != nil { - fmt.Fprint(streams.Out(), err.Error()) + _, _ = fmt.Fprint(streams.Out(), err) return errors.Wrapf(err, "failed to generate key for %s", keyName) } @@ -96,7 +96,7 @@ func validateAndGenerateKey(streams command.Streams, keyName string, workingDir if err != nil { return err } - fmt.Fprintf(streams.Out(), "Successfully generated and loaded private key. Corresponding public key available: %s\n", writtenPubFile) + _, _ = fmt.Fprintln(streams.Out(), "Successfully generated and loaded private key. Corresponding public key available:", writtenPubFile) return nil } diff --git a/cli/command/trust/key_load.go b/cli/command/trust/key_load.go index 4a5bb2605578..b4ce1d4d51f6 100644 --- a/cli/command/trust/key_load.go +++ b/cli/command/trust/key_load.go @@ -54,7 +54,7 @@ func loadPrivKey(streams command.Streams, keyPath string, options keyLoadOptions } privKeyImporters := []trustmanager.Importer{keyFileStore} - fmt.Fprintf(streams.Out(), "Loading key from \"%s\"...\n", keyPath) + _, _ = fmt.Fprintf(streams.Out(), "Loading key from \"%s\"...\n", keyPath) // Always use a fresh passphrase retriever for each import passRet := trust.GetPassphraseRetriever(streams.In(), streams.Out()) @@ -65,7 +65,7 @@ func loadPrivKey(streams command.Streams, keyPath string, options keyLoadOptions if err := loadPrivKeyBytesToStore(keyBytes, privKeyImporters, keyPath, options.keyName, passRet); err != nil { return errors.Wrapf(err, "error importing key from %s", keyPath) } - fmt.Fprintf(streams.Out(), "Successfully imported key from %s\n", keyPath) + _, _ = fmt.Fprintln(streams.Out(), "Successfully imported key from", keyPath) return nil } diff --git a/cli/command/trust/sign.go b/cli/command/trust/sign.go index 87fce71004b7..edadeb63880a 100644 --- a/cli/command/trust/sign.go +++ b/cli/command/trust/sign.go @@ -75,8 +75,8 @@ func runSignImage(ctx context.Context, dockerCLI command.Cli, options signOption return trust.NotaryError(imgRefAndAuth.Reference().Name(), err) } - fmt.Fprintf(dockerCLI.Out(), "Created signer: %s\n", imgRefAndAuth.AuthConfig().Username) - fmt.Fprintf(dockerCLI.Out(), "Finished initializing signed repository for %s\n", imageName) + _, _ = fmt.Fprintln(dockerCLI.Out(), "Created signer:", imgRefAndAuth.AuthConfig().Username) + _, _ = fmt.Fprintln(dockerCLI.Out(), "Finished initializing signed repository for", imageName) default: return trust.NotaryError(imgRefAndAuth.RepoInfo().Name.Name(), err) } @@ -91,18 +91,17 @@ func runSignImage(ctx context.Context, dockerCLI command.Cli, options signOption if err := checkLocalImageExistence(ctx, dockerCLI.Client(), imageName); err != nil { return err } - fmt.Fprintf(dockerCLI.Err(), "Signing and pushing trust data for local image %s, may overwrite remote trust data\n", imageName) + _, _ = fmt.Fprintf(dockerCLI.Err(), "Signing and pushing trust data for local image %s, may overwrite remote trust data\n", imageName) authConfig := command.ResolveAuthConfig(dockerCLI.ConfigFile(), imgRefAndAuth.RepoInfo().Index) encodedAuth, err := registrytypes.EncodeAuthConfig(authConfig) if err != nil { return err } - options := imagetypes.PushOptions{ + return image.TrustedPush(ctx, dockerCLI, imgRefAndAuth.RepoInfo(), imgRefAndAuth.Reference(), *imgRefAndAuth.AuthConfig(), imagetypes.PushOptions{ RegistryAuth: encodedAuth, PrivilegeFunc: requestPrivilege, - } - return image.TrustedPush(ctx, dockerCLI, imgRefAndAuth.RepoInfo(), imgRefAndAuth.Reference(), *imgRefAndAuth.AuthConfig(), options) + }) default: return err } @@ -112,7 +111,7 @@ func runSignImage(ctx context.Context, dockerCLI command.Cli, options signOption func signAndPublishToTarget(out io.Writer, imgRefAndAuth trust.ImageRefAndAuth, notaryRepo notaryclient.Repository, target notaryclient.Target) error { tag := imgRefAndAuth.Tag() - fmt.Fprintf(out, "Signing and pushing trust metadata for %s\n", imgRefAndAuth.Name()) + _, _ = fmt.Fprintln(out, "Signing and pushing trust metadata for", imgRefAndAuth.Name()) existingSigInfo, err := getExistingSignatureInfoForReleasedTag(notaryRepo, tag) if err != nil { return err @@ -125,7 +124,7 @@ func signAndPublishToTarget(out io.Writer, imgRefAndAuth trust.ImageRefAndAuth, if err != nil { return errors.Wrapf(err, "failed to sign %s:%s", imgRefAndAuth.RepoInfo().Name.Name(), tag) } - fmt.Fprintf(out, "Successfully signed %s:%s\n", imgRefAndAuth.RepoInfo().Name.Name(), tag) + _, _ = fmt.Fprintf(out, "Successfully signed %s:%s\n", imgRefAndAuth.RepoInfo().Name.Name(), tag) return nil } @@ -188,7 +187,7 @@ func getExistingSignatureInfoForReleasedTag(notaryRepo notaryclient.Repository, func prettyPrintExistingSignatureInfo(out io.Writer, existingSigInfo trustTagRow) { sort.Strings(existingSigInfo.Signers) joinedSigners := strings.Join(existingSigInfo.Signers, ", ") - fmt.Fprintf(out, "Existing signatures for tag %s digest %s from:\n%s\n", existingSigInfo.SignedTag, existingSigInfo.Digest, joinedSigners) + _, _ = fmt.Fprintf(out, "Existing signatures for tag %s digest %s from:\n%s\n", existingSigInfo.SignedTag, existingSigInfo.Digest, joinedSigners) } func initNotaryRepoWithSigners(notaryRepo notaryclient.Repository, newSigner data.RoleName) error { diff --git a/cli/command/trust/signer_add.go b/cli/command/trust/signer_add.go index 28e63cf1ece4..1593a591ad65 100644 --- a/cli/command/trust/signer_add.go +++ b/cli/command/trust/signer_add.go @@ -65,12 +65,12 @@ func addSigner(ctx context.Context, dockerCLI command.Cli, options signerAddOpti } var errRepos []string for _, repoName := range options.repos { - fmt.Fprintf(dockerCLI.Out(), "Adding signer \"%s\" to %s...\n", signerName, repoName) + _, _ = fmt.Fprintf(dockerCLI.Out(), "Adding signer \"%s\" to %s...\n", signerName, repoName) if err := addSignerToRepo(ctx, dockerCLI, signerName, repoName, signerPubKeys); err != nil { - fmt.Fprintln(dockerCLI.Err(), err.Error()+"\n") + _, _ = fmt.Fprintln(dockerCLI.Err(), err.Error()+"\n") errRepos = append(errRepos, repoName) } else { - fmt.Fprintf(dockerCLI.Out(), "Successfully added signer: %s to %s\n\n", signerName, repoName) + _, _ = fmt.Fprintf(dockerCLI.Out(), "Successfully added signer: %s to %s\n\n", signerName, repoName) } } if len(errRepos) > 0 { @@ -93,11 +93,11 @@ func addSignerToRepo(ctx context.Context, dockerCLI command.Cli, signerName stri if _, err = notaryRepo.ListTargets(); err != nil { switch err.(type) { case client.ErrRepoNotInitialized, client.ErrRepositoryNotExist: - fmt.Fprintf(dockerCLI.Out(), "Initializing signed repository for %s...\n", repoName) + _, _ = fmt.Fprintf(dockerCLI.Out(), "Initializing signed repository for %s...\n", repoName) if err := getOrGenerateRootKeyAndInitRepo(notaryRepo); err != nil { return trust.NotaryError(repoName, err) } - fmt.Fprintf(dockerCLI.Out(), "Successfully initialized %q\n", repoName) + _, _ = fmt.Fprintf(dockerCLI.Out(), "Successfully initialized %q\n", repoName) default: return trust.NotaryError(repoName, err) } diff --git a/cli/command/trust/signer_remove.go b/cli/command/trust/signer_remove.go index 1c85fcd9fd58..d2c5e9c71143 100644 --- a/cli/command/trust/signer_remove.go +++ b/cli/command/trust/signer_remove.go @@ -41,9 +41,9 @@ func newSignerRemoveCommand(dockerCli command.Cli) *cobra.Command { func removeSigner(ctx context.Context, dockerCLI command.Cli, options signerRemoveOptions) error { var errRepos []string for _, repo := range options.repos { - fmt.Fprintf(dockerCLI.Out(), "Removing signer \"%s\" from %s...\n", options.signer, repo) + _, _ = fmt.Fprintf(dockerCLI.Out(), "Removing signer \"%s\" from %s...\n", options.signer, repo) if _, err := removeSingleSigner(ctx, dockerCLI, repo, options.signer, options.forceYes); err != nil { - fmt.Fprintln(dockerCLI.Err(), err.Error()+"\n") + _, _ = fmt.Fprintln(dockerCLI.Err(), err.Error()+"\n") errRepos = append(errRepos, repo) } } @@ -150,7 +150,7 @@ func removeSingleSigner(ctx context.Context, dockerCLI command.Cli, repoName, si return false, err } - fmt.Fprintf(dockerCLI.Out(), "Successfully removed %s from %s\n\n", signerName, repoName) + _, _ = fmt.Fprintf(dockerCLI.Out(), "Successfully removed %s from %s\n\n", signerName, repoName) return true, nil } From 10f5b3f73a7f87beecf47369d99874dfeea1266e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 23:13:59 +0100 Subject: [PATCH 17/21] cli/command/volumes: minor cleanups: use Println, rename vars - use Println to print newline instead of custom format - use apiClient instead of client for the API client to prevent shadowing imports. - use dockerCLI with Go's standard camelCase casing. - suppress some errors to make my IDE and linters happier Signed-off-by: Sebastiaan van Stijn --- cli/command/volume/remove.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cli/command/volume/remove.go b/cli/command/volume/remove.go index df3dfe79a0a2..c436182d6364 100644 --- a/cli/command/volume/remove.go +++ b/cli/command/volume/remove.go @@ -41,17 +41,17 @@ func newRemoveCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runRemove(ctx context.Context, dockerCli command.Cli, opts *removeOptions) error { - client := dockerCli.Client() +func runRemove(ctx context.Context, dockerCLI command.Cli, opts *removeOptions) error { + apiClient := dockerCLI.Client() var errs []string for _, name := range opts.volumes { - if err := client.VolumeRemove(ctx, name, opts.force); err != nil { + if err := apiClient.VolumeRemove(ctx, name, opts.force); err != nil { errs = append(errs, err.Error()) continue } - fmt.Fprintf(dockerCli.Out(), "%s\n", name) + _, _ = fmt.Fprintln(dockerCLI.Out(), name) } if len(errs) > 0 { From cd6d902dff5414aeaa31c7e514d5da1e2625cd24 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 23:14:51 +0100 Subject: [PATCH 18/21] cli/command/inspect: remove additional newline from log Signed-off-by: Sebastiaan van Stijn --- cli/command/inspect/inspector.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/command/inspect/inspector.go b/cli/command/inspect/inspector.go index 46ba8750d6a4..b46f896df8e1 100644 --- a/cli/command/inspect/inspector.go +++ b/cli/command/inspect/inspector.go @@ -84,7 +84,7 @@ func Inspect(out io.Writer, references []string, tmplStr string, getRef GetRefFu } if err := inspector.Flush(); err != nil { - logrus.Errorf("%s\n", err) + logrus.Error(err) } if len(inspectErrs) != 0 { From dc5a4501a40f892220b5beeb3d317c6822c8b2b4 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 23:16:01 +0100 Subject: [PATCH 19/21] cli/command: minor cleanups: use Println, suppress errors - use Println to print newline instead of custom format - suppress some errors to make my IDE and linters happier Signed-off-by: Sebastiaan van Stijn --- cli/command/cli.go | 4 ++-- cli/command/utils_test.go | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cli/command/cli.go b/cli/command/cli.go index 722901e06043..a8cfd58e4df6 100644 --- a/cli/command/cli.go +++ b/cli/command/cli.go @@ -114,7 +114,7 @@ func (cli *DockerCli) CurrentVersion() string { // Client returns the APIClient func (cli *DockerCli) Client() client.APIClient { if err := cli.initialize(); err != nil { - _, _ = fmt.Fprintf(cli.Err(), "Failed to initialize: %s\n", err) + _, _ = fmt.Fprintln(cli.Err(), "Failed to initialize:", err) os.Exit(1) } return cli.client @@ -475,7 +475,7 @@ func (cli *DockerCli) DockerEndpoint() docker.Endpoint { if err := cli.initialize(); err != nil { // Note that we're not terminating here, as this function may be used // in cases where we're able to continue. - _, _ = fmt.Fprintf(cli.Err(), "%v\n", cli.initErr) + _, _ = fmt.Fprintln(cli.Err(), cli.initErr) } return cli.dockerEndpoint } diff --git a/cli/command/utils_test.go b/cli/command/utils_test.go index a940d876a1ec..4537cfcd5870 100644 --- a/cli/command/utils_test.go +++ b/cli/command/utils_test.go @@ -177,19 +177,19 @@ func TestPromptForConfirmation(t *testing.T) { return nil }, promptResult{false, command.ErrPromptTerminated}}, {"no", func() error { - _, err := fmt.Fprint(promptWriter, "n\n") + _, err := fmt.Fprintln(promptWriter, "n") return err }, promptResult{false, nil}}, {"yes", func() error { - _, err := fmt.Fprint(promptWriter, "y\n") + _, err := fmt.Fprintln(promptWriter, "y") return err }, promptResult{true, nil}}, {"any", func() error { - _, err := fmt.Fprint(promptWriter, "a\n") + _, err := fmt.Fprintln(promptWriter, "a") return err }, promptResult{false, nil}}, {"with space", func() error { - _, err := fmt.Fprint(promptWriter, " y\n") + _, err := fmt.Fprintln(promptWriter, " y") return err }, promptResult{true, nil}}, {"reader closed", func() error { From 13ef82974dcbd9e69e4c86b43474202ca0cf1f46 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 23:16:55 +0100 Subject: [PATCH 20/21] cli/flags: suppress some errors Signed-off-by: Sebastiaan van Stijn --- cli/flags/options.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/flags/options.go b/cli/flags/options.go index 5a6df9c8897d..fc168984b44b 100644 --- a/cli/flags/options.go +++ b/cli/flags/options.go @@ -138,7 +138,7 @@ func SetLogLevel(logLevel string) { if logLevel != "" { lvl, err := logrus.ParseLevel(logLevel) if err != nil { - fmt.Fprintf(os.Stderr, "Unable to parse logging level: %s\n", logLevel) + _, _ = fmt.Fprintln(os.Stderr, "Unable to parse logging level:", logLevel) os.Exit(1) } logrus.SetLevel(lvl) From 987da09578134c12c8dc2bd959f83dbcdc416fe4 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Feb 2025 19:31:47 +0100 Subject: [PATCH 21/21] cli/command/volume: remove example and var for long description This was the only command for which we set the "example" field; while we could consider doing this for other commands, we need to look what's best w.r.t. duplicating the information maintained in markdown. Also remove the intermediate variable used for the long description, as this was also the only location where we used one. Signed-off-by: Sebastiaan van Stijn --- cli/command/volume/remove.go | 12 +----------- docs/reference/commandline/volume_rm.md | 2 -- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/cli/command/volume/remove.go b/cli/command/volume/remove.go index c436182d6364..1eb38873473d 100644 --- a/cli/command/volume/remove.go +++ b/cli/command/volume/remove.go @@ -25,8 +25,7 @@ func newRemoveCommand(dockerCli command.Cli) *cobra.Command { Use: "rm [OPTIONS] VOLUME [VOLUME...]", Aliases: []string{"remove"}, Short: "Remove one or more volumes", - Long: removeDescription, - Example: removeExample, + Long: "Remove one or more volumes. You cannot remove a volume that is in use by a container.", Args: cli.RequiresMinArgs(1), RunE: func(cmd *cobra.Command, args []string) error { opts.volumes = args @@ -59,12 +58,3 @@ func runRemove(ctx context.Context, dockerCLI command.Cli, opts *removeOptions) } return nil } - -var removeDescription = ` -Remove one or more volumes. You cannot remove a volume that is in use by a container. -` - -var removeExample = ` -$ docker volume rm hello -hello -` diff --git a/docs/reference/commandline/volume_rm.md b/docs/reference/commandline/volume_rm.md index eaa720d85285..32a2c77dd7b7 100644 --- a/docs/reference/commandline/volume_rm.md +++ b/docs/reference/commandline/volume_rm.md @@ -1,10 +1,8 @@ # volume rm - Remove one or more volumes. You cannot remove a volume that is in use by a container. - ### Aliases `docker volume rm`, `docker volume remove`