From c6f935eba503c9ad7b5c5b42078b910a08ce72d3 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 26 Jul 2025 15:58:06 +0200 Subject: [PATCH] cli/command/plugin: fix linting issues, and assorted cleanups - fix various unhandled errors - remove some locally defined option-types in favor of option-types defined by the client / api - don't use unkeyed structs in tests, and add docs for some subtests - fix some values in tests that triggered "spellcheck" warnings - inline vars / functions that only had a single use. Signed-off-by: Sebastiaan van Stijn --- cli/command/plugin/create.go | 2 +- cli/command/plugin/disable.go | 22 ++-- cli/command/plugin/enable.go | 31 ++--- cli/command/plugin/enable_test.go | 2 +- cli/command/plugin/formatter.go | 8 +- cli/command/plugin/formatter_test.go | 116 +++++++++++------- cli/command/plugin/inspect.go | 12 +- cli/command/plugin/inspect_test.go | 8 +- cli/command/plugin/install.go | 4 +- cli/command/plugin/install_test.go | 2 +- cli/command/plugin/list_test.go | 4 +- cli/command/plugin/push.go | 4 +- cli/command/plugin/remove_test.go | 2 +- cli/command/plugin/set.go | 4 +- ...lugin-inspect-single-without-format.golden | 4 +- cli/command/plugin/upgrade.go | 4 +- 16 files changed, 124 insertions(+), 105 deletions(-) diff --git a/cli/command/plugin/create.go b/cli/command/plugin/create.go index 182bb5d71e78..55f5cf270b4a 100644 --- a/cli/command/plugin/create.go +++ b/cli/command/plugin/create.go @@ -40,7 +40,7 @@ func validateConfig(path string) error { return err } -// validateContextDir validates the given dir and returns abs path on success. +// validateContextDir validates the given dir and returns its absolute path on success. func validateContextDir(contextDir string) (string, error) { absContextDir, err := filepath.Abs(contextDir) if err != nil { diff --git a/cli/command/plugin/disable.go b/cli/command/plugin/disable.go index a696717de9b7..22713c57c26a 100644 --- a/cli/command/plugin/disable.go +++ b/cli/command/plugin/disable.go @@ -1,7 +1,6 @@ package plugin import ( - "context" "fmt" "github.com/docker/cli/cli" @@ -10,27 +9,24 @@ import ( "github.com/spf13/cobra" ) -func newDisableCommand(dockerCli command.Cli) *cobra.Command { - var force bool +func newDisableCommand(dockerCLI command.Cli) *cobra.Command { + var opts types.PluginDisableOptions cmd := &cobra.Command{ Use: "disable [OPTIONS] PLUGIN", Short: "Disable a plugin", Args: cli.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - return runDisable(cmd.Context(), dockerCli, args[0], force) + name := args[0] + if err := dockerCLI.Client().PluginDisable(cmd.Context(), name, opts); err != nil { + return err + } + _, _ = fmt.Fprintln(dockerCLI.Out(), name) + return nil }, } flags := cmd.Flags() - flags.BoolVarP(&force, "force", "f", false, "Force the disable of an active plugin") + flags.BoolVarP(&opts.Force, "force", "f", false, "Force the disable of an active plugin") return cmd } - -func runDisable(ctx context.Context, dockerCli command.Cli, name string, force bool) error { - if err := dockerCli.Client().PluginDisable(ctx, name, types.PluginDisableOptions{Force: force}); err != nil { - return err - } - fmt.Fprintln(dockerCli.Out(), name) - return nil -} diff --git a/cli/command/plugin/enable.go b/cli/command/plugin/enable.go index 288c7bcce4ff..216e07275c09 100644 --- a/cli/command/plugin/enable.go +++ b/cli/command/plugin/enable.go @@ -11,38 +11,31 @@ import ( "github.com/spf13/cobra" ) -type enableOpts struct { - timeout int - name string -} - func newEnableCommand(dockerCli command.Cli) *cobra.Command { - var opts enableOpts + var opts types.PluginEnableOptions cmd := &cobra.Command{ Use: "enable [OPTIONS] PLUGIN", Short: "Enable a plugin", Args: cli.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - opts.name = args[0] - return runEnable(cmd.Context(), dockerCli, &opts) + name := args[0] + if err := runEnable(cmd.Context(), dockerCli, name, opts); err != nil { + return err + } + _, _ = fmt.Fprintln(dockerCli.Out(), name) + return nil }, } flags := cmd.Flags() - flags.IntVar(&opts.timeout, "timeout", 30, "HTTP client timeout (in seconds)") + flags.IntVar(&opts.Timeout, "timeout", 30, "HTTP client timeout (in seconds)") return cmd } -func runEnable(ctx context.Context, dockerCli command.Cli, opts *enableOpts) error { - name := opts.name - if opts.timeout < 0 { - return errors.Errorf("negative timeout %d is invalid", opts.timeout) - } - - if err := dockerCli.Client().PluginEnable(ctx, name, types.PluginEnableOptions{Timeout: opts.timeout}); err != nil { - return err +func runEnable(ctx context.Context, dockerCli command.Cli, name string, opts types.PluginEnableOptions) error { + if opts.Timeout < 0 { + return errors.Errorf("negative timeout %d is invalid", opts.Timeout) } - fmt.Fprintln(dockerCli.Out(), name) - return nil + return dockerCli.Client().PluginEnable(ctx, name, opts) } diff --git a/cli/command/plugin/enable_test.go b/cli/command/plugin/enable_test.go index ebfad6d76f01..4f1399bcdb04 100644 --- a/cli/command/plugin/enable_test.go +++ b/cli/command/plugin/enable_test.go @@ -48,7 +48,7 @@ func TestPluginEnableErrors(t *testing.T) { })) cmd.SetArgs(tc.args) for key, value := range tc.flags { - cmd.Flags().Set(key, value) + assert.NilError(t, cmd.Flags().Set(key, value)) } cmd.SetOut(io.Discard) cmd.SetErr(io.Discard) diff --git a/cli/command/plugin/formatter.go b/cli/command/plugin/formatter.go index 310382bbc08f..9b2b5b1a3d7a 100644 --- a/cli/command/plugin/formatter.go +++ b/cli/command/plugin/formatter.go @@ -12,6 +12,12 @@ const ( enabledHeader = "ENABLED" pluginIDHeader = "ID" + + rawFormat = `plugin_id: {{.ID}} +name: {{.Name}} +description: {{.Description}} +enabled: {{.Enabled}} +` ) // NewFormat returns a Format for rendering using a plugin Context @@ -26,7 +32,7 @@ func NewFormat(source string, quiet bool) formatter.Format { if quiet { return `plugin_id: {{.ID}}` } - return `plugin_id: {{.ID}}\nname: {{.Name}}\ndescription: {{.Description}}\nenabled: {{.Enabled}}\n` + return rawFormat } return formatter.Format(source) } diff --git a/cli/command/plugin/formatter_test.go b/cli/command/plugin/formatter_test.go index 1d675155d678..ca321d90ef0c 100644 --- a/cli/command/plugin/formatter_test.go +++ b/cli/command/plugin/formatter_test.go @@ -19,85 +19,106 @@ import ( func TestPluginContext(t *testing.T) { pluginID := test.RandomID() - var ctx pluginContext - cases := []struct { + var pCtx pluginContext + tests := []struct { pluginCtx pluginContext expValue string call func() string }{ - {pluginContext{ - p: types.Plugin{ID: pluginID}, - trunc: false, - }, pluginID, ctx.ID}, - {pluginContext{ - p: types.Plugin{ID: pluginID}, - trunc: true, - }, formatter.TruncateID(pluginID), ctx.ID}, - {pluginContext{ - p: types.Plugin{Name: "plugin_name"}, - }, "plugin_name", ctx.Name}, - {pluginContext{ - p: types.Plugin{Config: types.PluginConfig{Description: "plugin_description"}}, - }, "plugin_description", ctx.Description}, + { + pluginCtx: pluginContext{ + p: types.Plugin{ID: pluginID}, + trunc: false, + }, + expValue: pluginID, + call: pCtx.ID, + }, + { + pluginCtx: pluginContext{ + p: types.Plugin{ID: pluginID}, + trunc: true, + }, + expValue: formatter.TruncateID(pluginID), + call: pCtx.ID, + }, + { + pluginCtx: pluginContext{ + p: types.Plugin{Name: "plugin_name"}, + }, + expValue: "plugin_name", + call: pCtx.Name, + }, + { + pluginCtx: pluginContext{ + p: types.Plugin{Config: types.PluginConfig{Description: "plugin_description"}}, + }, + expValue: "plugin_description", + call: pCtx.Description, + }, } - for _, c := range cases { - ctx = c.pluginCtx - v := c.call() + for _, tc := range tests { + pCtx = tc.pluginCtx + v := tc.call() if strings.Contains(v, ",") { - test.CompareMultipleValues(t, v, c.expValue) - } else if v != c.expValue { - t.Fatalf("Expected %s, was %s\n", c.expValue, v) + test.CompareMultipleValues(t, v, tc.expValue) + } else if v != tc.expValue { + t.Fatalf("Expected %s, was %s\n", tc.expValue, v) } } } func TestPluginContextWrite(t *testing.T) { - cases := []struct { + tests := []struct { + doc string context formatter.Context expected string }{ - // Errors { - formatter.Context{Format: "{{InvalidFunction}}"}, - `template parsing error: template: :1: function "InvalidFunction" not defined`, + doc: "invalid function", + context: formatter.Context{Format: "{{InvalidFunction}}"}, + expected: `template parsing error: template: :1: function "InvalidFunction" not defined`, }, { - formatter.Context{Format: "{{nil}}"}, - `template parsing error: template: :1:2: executing "" at : nil is not a command`, + doc: "nil template", + context: formatter.Context{Format: "{{nil}}"}, + expected: `template parsing error: template: :1:2: executing "" at : nil is not a command`, }, - // Table format { - formatter.Context{Format: NewFormat("table", false)}, - `ID NAME DESCRIPTION ENABLED + doc: "table format", + context: formatter.Context{Format: NewFormat("table", false)}, + expected: `ID NAME DESCRIPTION ENABLED pluginID1 foobar_baz description 1 true pluginID2 foobar_bar description 2 false `, }, { - formatter.Context{Format: NewFormat("table", true)}, - `pluginID1 + doc: "table format, quiet", + context: formatter.Context{Format: NewFormat("table", true)}, + expected: `pluginID1 pluginID2 `, }, { - formatter.Context{Format: NewFormat("table {{.Name}}", false)}, - `NAME + doc: "table format name col", + context: formatter.Context{Format: NewFormat("table {{.Name}}", false)}, + expected: `NAME foobar_baz foobar_bar `, }, { - formatter.Context{Format: NewFormat("table {{.Name}}", true)}, - `NAME + doc: "table format name col, quiet", + context: formatter.Context{Format: NewFormat("table {{.Name}}", true)}, + expected: `NAME foobar_baz foobar_bar `, }, - // Raw Format { - formatter.Context{Format: NewFormat("raw", false)}, - `plugin_id: pluginID1 + doc: "raw format", + context: formatter.Context{Format: NewFormat("raw", false)}, + expected: `plugin_id: pluginID1 name: foobar_baz description: description 1 enabled: true @@ -110,15 +131,16 @@ enabled: false `, }, { - formatter.Context{Format: NewFormat("raw", true)}, - `plugin_id: pluginID1 + doc: "raw format, quiet", + context: formatter.Context{Format: NewFormat("raw", true)}, + expected: `plugin_id: pluginID1 plugin_id: pluginID2 `, }, - // Custom Format { - formatter.Context{Format: NewFormat("{{.Name}}", false)}, - `foobar_baz + doc: "custom format", + context: formatter.Context{Format: NewFormat("{{.Name}}", false)}, + expected: `foobar_baz foobar_bar `, }, @@ -129,8 +151,8 @@ foobar_bar {ID: "pluginID2", Name: "foobar_bar", Config: types.PluginConfig{Description: "description 2"}, Enabled: false}, } - for _, tc := range cases { - t.Run(string(tc.context.Format), func(t *testing.T) { + for _, tc := range tests { + t.Run(tc.doc, func(t *testing.T) { var out bytes.Buffer tc.context.Output = &out diff --git a/cli/command/plugin/inspect.go b/cli/command/plugin/inspect.go index 9b6a453c9336..289fe94f5e24 100644 --- a/cli/command/plugin/inspect.go +++ b/cli/command/plugin/inspect.go @@ -36,11 +36,9 @@ func newInspectCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runInspect(ctx context.Context, dockerCli command.Cli, opts inspectOptions) error { - client := dockerCli.Client() - getRef := func(ref string) (any, []byte, error) { - return client.PluginInspectWithRaw(ctx, ref) - } - - return inspect.Inspect(dockerCli.Out(), opts.pluginNames, opts.format, getRef) +func runInspect(ctx context.Context, dockerCLI command.Cli, opts inspectOptions) error { + apiClient := dockerCLI.Client() + return inspect.Inspect(dockerCLI.Out(), opts.pluginNames, opts.format, func(ref string) (any, []byte, error) { + return apiClient.PluginInspectWithRaw(ctx, ref) + }) } diff --git a/cli/command/plugin/inspect_test.go b/cli/command/plugin/inspect_test.go index 0df119a9ea65..93868ecd27c2 100644 --- a/cli/command/plugin/inspect_test.go +++ b/cli/command/plugin/inspect_test.go @@ -21,14 +21,14 @@ var pluginFoo = &types.Plugin{ Documentation: "plugin foo documentation", Entrypoint: []string{"/foo"}, Interface: types.PluginConfigInterface{ - Socket: "pluginfoo.sock", + Socket: "plugin-foo.sock", }, Linux: types.PluginConfigLinux{ Capabilities: []string{"CAP_SYS_ADMIN"}, }, WorkDir: "workdir-foo", Rootfs: &types.PluginConfigRootfs{ - DiffIds: []string{"sha256:8603eedd4ea52cebb2f22b45405a3dc8f78ba3e31bf18f27b4547a9ff930e0bd"}, + DiffIds: []string{"sha256:deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef"}, Type: "layers", }, }, @@ -71,7 +71,7 @@ func TestInspectErrors(t *testing.T) { cmd := newInspectCommand(cli) cmd.SetArgs(tc.args) for key, value := range tc.flags { - cmd.Flags().Set(key, value) + assert.NilError(t, cmd.Flags().Set(key, value)) } cmd.SetOut(io.Discard) cmd.SetErr(io.Discard) @@ -142,7 +142,7 @@ func TestInspect(t *testing.T) { cmd := newInspectCommand(cli) cmd.SetArgs(tc.args) for key, value := range tc.flags { - cmd.Flags().Set(key, value) + assert.NilError(t, cmd.Flags().Set(key, value)) } assert.NilError(t, cmd.Execute()) golden.Assert(t, cli.OutBuffer().String(), tc.golden) diff --git a/cli/command/plugin/install.go b/cli/command/plugin/install.go index b17c23ba5b5a..2d725154eeed 100644 --- a/cli/command/plugin/install.go +++ b/cli/command/plugin/install.go @@ -126,7 +126,9 @@ func runInstall(ctx context.Context, dockerCLI command.Cli, opts pluginOptions) } return err } - defer responseBody.Close() + defer func() { + _ = responseBody.Close() + }() if err := jsonstream.Display(ctx, responseBody, dockerCLI.Out()); err != nil { return err } diff --git a/cli/command/plugin/install_test.go b/cli/command/plugin/install_test.go index 09dced86bbf8..31f024ec19bb 100644 --- a/cli/command/plugin/install_test.go +++ b/cli/command/plugin/install_test.go @@ -32,7 +32,7 @@ func TestInstallErrors(t *testing.T) { }, { description: "invalid plugin name", - args: []string{"UPPERCASE_REPONAME"}, + args: []string{"UPPERCASE_REPO_NAME"}, expectedError: "invalid", }, { diff --git a/cli/command/plugin/list_test.go b/cli/command/plugin/list_test.go index 65debd5cc340..43a7398b290d 100644 --- a/cli/command/plugin/list_test.go +++ b/cli/command/plugin/list_test.go @@ -51,7 +51,7 @@ func TestListErrors(t *testing.T) { cmd := newListCommand(cli) cmd.SetArgs(tc.args) for key, value := range tc.flags { - cmd.Flags().Set(key, value) + assert.NilError(t, cmd.Flags().Set(key, value)) } cmd.SetOut(io.Discard) cmd.SetErr(io.Discard) @@ -170,7 +170,7 @@ func TestList(t *testing.T) { cmd := newListCommand(cli) cmd.SetArgs(tc.args) for key, value := range tc.flags { - cmd.Flags().Set(key, value) + assert.NilError(t, cmd.Flags().Set(key, value)) } assert.NilError(t, cmd.Execute()) golden.Assert(t, cli.OutBuffer().String(), tc.golden) diff --git a/cli/command/plugin/push.go b/cli/command/plugin/push.go index f7fe2cdee9dc..6101f5183722 100644 --- a/cli/command/plugin/push.go +++ b/cli/command/plugin/push.go @@ -60,7 +60,9 @@ func runPush(ctx context.Context, dockerCli command.Cli, opts pushOptions) error if err != nil { return err } - defer responseBody.Close() + defer func() { + _ = responseBody.Close() + }() if !opts.untrusted { return trust.PushTrustedReference(ctx, dockerCli, repoInfo, named, authConfig, responseBody, command.UserAgent()) diff --git a/cli/command/plugin/remove_test.go b/cli/command/plugin/remove_test.go index 24c99dcc1edb..1fefe1017d28 100644 --- a/cli/command/plugin/remove_test.go +++ b/cli/command/plugin/remove_test.go @@ -64,7 +64,7 @@ func TestRemoveWithForceOption(t *testing.T) { }) cmd := newRemoveCommand(cli) cmd.SetArgs([]string{"plugin-foo"}) - cmd.Flags().Set("force", "true") + assert.NilError(t, cmd.Flags().Set("force", "true")) assert.NilError(t, cmd.Execute()) assert.Check(t, force) assert.Check(t, is.Equal("plugin-foo\n", cli.OutBuffer().String())) diff --git a/cli/command/plugin/set.go b/cli/command/plugin/set.go index 64442097b3aa..2d1eecc0ad0b 100644 --- a/cli/command/plugin/set.go +++ b/cli/command/plugin/set.go @@ -7,7 +7,7 @@ import ( ) func newSetCommand(dockerCli command.Cli) *cobra.Command { - cmd := &cobra.Command{ + return &cobra.Command{ Use: "set PLUGIN KEY=VALUE [KEY=VALUE...]", Short: "Change settings for a plugin", Args: cli.RequiresMinArgs(2), @@ -15,6 +15,4 @@ func newSetCommand(dockerCli command.Cli) *cobra.Command { return dockerCli.Client().PluginSet(cmd.Context(), args[0], args[1:]) }, } - - return cmd } diff --git a/cli/command/plugin/testdata/plugin-inspect-single-without-format.golden b/cli/command/plugin/testdata/plugin-inspect-single-without-format.golden index 65c8d39ce995..be510f40349b 100644 --- a/cli/command/plugin/testdata/plugin-inspect-single-without-format.golden +++ b/cli/command/plugin/testdata/plugin-inspect-single-without-format.golden @@ -15,7 +15,7 @@ ], "Env": null, "Interface": { - "Socket": "pluginfoo.sock", + "Socket": "plugin-foo.sock", "Types": null }, "IpcHost": false, @@ -36,7 +36,7 @@ "WorkDir": "workdir-foo", "rootfs": { "diff_ids": [ - "sha256:8603eedd4ea52cebb2f22b45405a3dc8f78ba3e31bf18f27b4547a9ff930e0bd" + "sha256:deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef" ], "type": "layers" } diff --git a/cli/command/plugin/upgrade.go b/cli/command/plugin/upgrade.go index 5489232d9a2a..7d4548d428f1 100644 --- a/cli/command/plugin/upgrade.go +++ b/cli/command/plugin/upgrade.go @@ -85,7 +85,9 @@ func runUpgrade(ctx context.Context, dockerCLI command.Cli, opts pluginOptions) } return err } - defer responseBody.Close() + defer func() { + _ = responseBody.Close() + }() if err := jsonstream.Display(ctx, responseBody, dockerCLI.Out()); err != nil { return err }