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 0461e01c8c..d91e1185fa 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 bedc87f929..f6d4b28ef4 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 a9d55e3d87..1cac1dd7b1 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 0be5ec3d4c..87ae6eb488 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 7631bab0de..69fe6d6ad3 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 8c8ad6df66..9a695f1883 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 c3bd279c99..db3628802f 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 3ea41ee85f..8455e979e6 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 d6fe10341a..3e975a81b6 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 be81c21733..30aea832ce 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 d6afb0528c..76f7765cd6 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 a77151c252..a3bbeeedfc 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 c09f8f89f5..ff0179feb3 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 98eba7d1ec..980937e0c1 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 c24a0c6ecb..412df755e9 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 01edd2b042..18d79d08e6 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 93388253f7..bc3ed55b8e 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 d3868352fb..508d30fe50 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 3fb0af5c03..067e0541b6 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 3e06f9a90a..274344452f 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 ac78b14f62..d6ca591ab2 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 29fcffe30f..67c370ec78 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 e43b183190..9331a6d7f1 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 105e629f61..ed1b7f264f 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 ea914bb094..9f6b25ac96 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 c9d45e9cda..2257273972 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 fedf34dec4..809c6b16fa 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 cb366fd406..17d6ae3847 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 69b5c51b5f..d83c75e317 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 3facb75ccc..3fd91d56fd 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 4eb479f3e5..6b5355335a 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 b1bed66195..ec3cf11953 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 efbd5f9ad0..a191aa97bf 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 06be65bb90..f7efa2d498 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 688a3ff2d0..7d34359b6e 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 3aec19002e..cb32534d37 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 811dcf22b3..a9c52f5f54 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 5608a81c8f..8bf04b9aa5 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 2638c985b0..f91ba71edf 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 7fe52f67fd..bfd71f5f65 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 c163d6bf69..cc6a9a0e54 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 5641ea24bd..8c16a7fc04 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 3096079e17..88df4d164e 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 8612a5f8f1..bc3160554e 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 cd8fafac1a..f76ae79c66 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 02ff901ac3..b6d092e8ce 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 de0611700a..08a5d8766c 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 76db830b64..7c819b6fea 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 19dd1dc1f6..a389d90fc7 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 7a47a5f2ba..96e3705c73 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 6e82be010e..b2c727f0a1 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 96190e99b5..b3fe20e2ce 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 7caee23728..af62e0b09a 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 e55567b50e..05bd220d43 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 f0cdfdfa8e..b437cac07f 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 98dd0b7abe..d6246de810 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 9b5f965e5b..d83d36351e 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 0af387946f..1197665558 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 6de7866234..574a1aad7b 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 f0e16006ba..b0b5e8c51c 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 39eb0cb1b9..3a137f1276 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 4a5bb26055..b4ce1d4d51 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 87fce71004..edadeb6388 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 28e63cf1ec..1593a591ad 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 1c85fcd9fd..d2c5e9c711 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 df3dfe79a0..c436182d63 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 46ba8750d6..b46f896df8 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 722901e060..a8cfd58e4d 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 a940d876a1..4537cfcd58 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 5a6df9c889..fc168984b4 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 c436182d63..1eb3887347 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 eaa720d852..32a2c77dd7 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`