Skip to content

Commit b52ab17

Browse files
authored
Merge pull request #6340 from thaJeztah/28.x_backport_cleanup_plugins
[28.x backport] cli/command/plugin: fix linting issues, and assorted cleanups
2 parents aa44c43 + ec89d0b commit b52ab17

File tree

16 files changed

+124
-105
lines changed

16 files changed

+124
-105
lines changed

cli/command/plugin/create.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func validateConfig(path string) error {
4040
return err
4141
}
4242

43-
// validateContextDir validates the given dir and returns abs path on success.
43+
// validateContextDir validates the given dir and returns its absolute path on success.
4444
func validateContextDir(contextDir string) (string, error) {
4545
absContextDir, err := filepath.Abs(contextDir)
4646
if err != nil {

cli/command/plugin/disable.go

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package plugin
22

33
import (
4-
"context"
54
"fmt"
65

76
"github.com/docker/cli/cli"
@@ -10,27 +9,24 @@ import (
109
"github.com/spf13/cobra"
1110
)
1211

13-
func newDisableCommand(dockerCli command.Cli) *cobra.Command {
14-
var force bool
12+
func newDisableCommand(dockerCLI command.Cli) *cobra.Command {
13+
var opts types.PluginDisableOptions
1514

1615
cmd := &cobra.Command{
1716
Use: "disable [OPTIONS] PLUGIN",
1817
Short: "Disable a plugin",
1918
Args: cli.ExactArgs(1),
2019
RunE: func(cmd *cobra.Command, args []string) error {
21-
return runDisable(cmd.Context(), dockerCli, args[0], force)
20+
name := args[0]
21+
if err := dockerCLI.Client().PluginDisable(cmd.Context(), name, opts); err != nil {
22+
return err
23+
}
24+
_, _ = fmt.Fprintln(dockerCLI.Out(), name)
25+
return nil
2226
},
2327
}
2428

2529
flags := cmd.Flags()
26-
flags.BoolVarP(&force, "force", "f", false, "Force the disable of an active plugin")
30+
flags.BoolVarP(&opts.Force, "force", "f", false, "Force the disable of an active plugin")
2731
return cmd
2832
}
29-
30-
func runDisable(ctx context.Context, dockerCli command.Cli, name string, force bool) error {
31-
if err := dockerCli.Client().PluginDisable(ctx, name, types.PluginDisableOptions{Force: force}); err != nil {
32-
return err
33-
}
34-
fmt.Fprintln(dockerCli.Out(), name)
35-
return nil
36-
}

cli/command/plugin/enable.go

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,38 +11,31 @@ import (
1111
"github.com/spf13/cobra"
1212
)
1313

14-
type enableOpts struct {
15-
timeout int
16-
name string
17-
}
18-
1914
func newEnableCommand(dockerCli command.Cli) *cobra.Command {
20-
var opts enableOpts
15+
var opts types.PluginEnableOptions
2116

2217
cmd := &cobra.Command{
2318
Use: "enable [OPTIONS] PLUGIN",
2419
Short: "Enable a plugin",
2520
Args: cli.ExactArgs(1),
2621
RunE: func(cmd *cobra.Command, args []string) error {
27-
opts.name = args[0]
28-
return runEnable(cmd.Context(), dockerCli, &opts)
22+
name := args[0]
23+
if err := runEnable(cmd.Context(), dockerCli, name, opts); err != nil {
24+
return err
25+
}
26+
_, _ = fmt.Fprintln(dockerCli.Out(), name)
27+
return nil
2928
},
3029
}
3130

3231
flags := cmd.Flags()
33-
flags.IntVar(&opts.timeout, "timeout", 30, "HTTP client timeout (in seconds)")
32+
flags.IntVar(&opts.Timeout, "timeout", 30, "HTTP client timeout (in seconds)")
3433
return cmd
3534
}
3635

37-
func runEnable(ctx context.Context, dockerCli command.Cli, opts *enableOpts) error {
38-
name := opts.name
39-
if opts.timeout < 0 {
40-
return errors.Errorf("negative timeout %d is invalid", opts.timeout)
41-
}
42-
43-
if err := dockerCli.Client().PluginEnable(ctx, name, types.PluginEnableOptions{Timeout: opts.timeout}); err != nil {
44-
return err
36+
func runEnable(ctx context.Context, dockerCli command.Cli, name string, opts types.PluginEnableOptions) error {
37+
if opts.Timeout < 0 {
38+
return errors.Errorf("negative timeout %d is invalid", opts.Timeout)
4539
}
46-
fmt.Fprintln(dockerCli.Out(), name)
47-
return nil
40+
return dockerCli.Client().PluginEnable(ctx, name, opts)
4841
}

cli/command/plugin/enable_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func TestPluginEnableErrors(t *testing.T) {
4848
}))
4949
cmd.SetArgs(tc.args)
5050
for key, value := range tc.flags {
51-
cmd.Flags().Set(key, value)
51+
assert.NilError(t, cmd.Flags().Set(key, value))
5252
}
5353
cmd.SetOut(io.Discard)
5454
cmd.SetErr(io.Discard)

cli/command/plugin/formatter.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,12 @@ const (
1212

1313
enabledHeader = "ENABLED"
1414
pluginIDHeader = "ID"
15+
16+
rawFormat = `plugin_id: {{.ID}}
17+
name: {{.Name}}
18+
description: {{.Description}}
19+
enabled: {{.Enabled}}
20+
`
1521
)
1622

1723
// NewFormat returns a Format for rendering using a plugin Context
@@ -26,7 +32,7 @@ func NewFormat(source string, quiet bool) formatter.Format {
2632
if quiet {
2733
return `plugin_id: {{.ID}}`
2834
}
29-
return `plugin_id: {{.ID}}\nname: {{.Name}}\ndescription: {{.Description}}\nenabled: {{.Enabled}}\n`
35+
return rawFormat
3036
}
3137
return formatter.Format(source)
3238
}

cli/command/plugin/formatter_test.go

Lines changed: 69 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -19,85 +19,106 @@ import (
1919
func TestPluginContext(t *testing.T) {
2020
pluginID := test.RandomID()
2121

22-
var ctx pluginContext
23-
cases := []struct {
22+
var pCtx pluginContext
23+
tests := []struct {
2424
pluginCtx pluginContext
2525
expValue string
2626
call func() string
2727
}{
28-
{pluginContext{
29-
p: types.Plugin{ID: pluginID},
30-
trunc: false,
31-
}, pluginID, ctx.ID},
32-
{pluginContext{
33-
p: types.Plugin{ID: pluginID},
34-
trunc: true,
35-
}, formatter.TruncateID(pluginID), ctx.ID},
36-
{pluginContext{
37-
p: types.Plugin{Name: "plugin_name"},
38-
}, "plugin_name", ctx.Name},
39-
{pluginContext{
40-
p: types.Plugin{Config: types.PluginConfig{Description: "plugin_description"}},
41-
}, "plugin_description", ctx.Description},
28+
{
29+
pluginCtx: pluginContext{
30+
p: types.Plugin{ID: pluginID},
31+
trunc: false,
32+
},
33+
expValue: pluginID,
34+
call: pCtx.ID,
35+
},
36+
{
37+
pluginCtx: pluginContext{
38+
p: types.Plugin{ID: pluginID},
39+
trunc: true,
40+
},
41+
expValue: formatter.TruncateID(pluginID),
42+
call: pCtx.ID,
43+
},
44+
{
45+
pluginCtx: pluginContext{
46+
p: types.Plugin{Name: "plugin_name"},
47+
},
48+
expValue: "plugin_name",
49+
call: pCtx.Name,
50+
},
51+
{
52+
pluginCtx: pluginContext{
53+
p: types.Plugin{Config: types.PluginConfig{Description: "plugin_description"}},
54+
},
55+
expValue: "plugin_description",
56+
call: pCtx.Description,
57+
},
4258
}
4359

44-
for _, c := range cases {
45-
ctx = c.pluginCtx
46-
v := c.call()
60+
for _, tc := range tests {
61+
pCtx = tc.pluginCtx
62+
v := tc.call()
4763
if strings.Contains(v, ",") {
48-
test.CompareMultipleValues(t, v, c.expValue)
49-
} else if v != c.expValue {
50-
t.Fatalf("Expected %s, was %s\n", c.expValue, v)
64+
test.CompareMultipleValues(t, v, tc.expValue)
65+
} else if v != tc.expValue {
66+
t.Fatalf("Expected %s, was %s\n", tc.expValue, v)
5167
}
5268
}
5369
}
5470

5571
func TestPluginContextWrite(t *testing.T) {
56-
cases := []struct {
72+
tests := []struct {
73+
doc string
5774
context formatter.Context
5875
expected string
5976
}{
60-
// Errors
6177
{
62-
formatter.Context{Format: "{{InvalidFunction}}"},
63-
`template parsing error: template: :1: function "InvalidFunction" not defined`,
78+
doc: "invalid function",
79+
context: formatter.Context{Format: "{{InvalidFunction}}"},
80+
expected: `template parsing error: template: :1: function "InvalidFunction" not defined`,
6481
},
6582
{
66-
formatter.Context{Format: "{{nil}}"},
67-
`template parsing error: template: :1:2: executing "" at <nil>: nil is not a command`,
83+
doc: "nil template",
84+
context: formatter.Context{Format: "{{nil}}"},
85+
expected: `template parsing error: template: :1:2: executing "" at <nil>: nil is not a command`,
6886
},
69-
// Table format
7087
{
71-
formatter.Context{Format: NewFormat("table", false)},
72-
`ID NAME DESCRIPTION ENABLED
88+
doc: "table format",
89+
context: formatter.Context{Format: NewFormat("table", false)},
90+
expected: `ID NAME DESCRIPTION ENABLED
7391
pluginID1 foobar_baz description 1 true
7492
pluginID2 foobar_bar description 2 false
7593
`,
7694
},
7795
{
78-
formatter.Context{Format: NewFormat("table", true)},
79-
`pluginID1
96+
doc: "table format, quiet",
97+
context: formatter.Context{Format: NewFormat("table", true)},
98+
expected: `pluginID1
8099
pluginID2
81100
`,
82101
},
83102
{
84-
formatter.Context{Format: NewFormat("table {{.Name}}", false)},
85-
`NAME
103+
doc: "table format name col",
104+
context: formatter.Context{Format: NewFormat("table {{.Name}}", false)},
105+
expected: `NAME
86106
foobar_baz
87107
foobar_bar
88108
`,
89109
},
90110
{
91-
formatter.Context{Format: NewFormat("table {{.Name}}", true)},
92-
`NAME
111+
doc: "table format name col, quiet",
112+
context: formatter.Context{Format: NewFormat("table {{.Name}}", true)},
113+
expected: `NAME
93114
foobar_baz
94115
foobar_bar
95116
`,
96117
},
97-
// Raw Format
98118
{
99-
formatter.Context{Format: NewFormat("raw", false)},
100-
`plugin_id: pluginID1
119+
doc: "raw format",
120+
context: formatter.Context{Format: NewFormat("raw", false)},
121+
expected: `plugin_id: pluginID1
101122
name: foobar_baz
102123
description: description 1
103124
enabled: true
@@ -110,15 +131,16 @@ enabled: false
110131
`,
111132
},
112133
{
113-
formatter.Context{Format: NewFormat("raw", true)},
114-
`plugin_id: pluginID1
134+
doc: "raw format, quiet",
135+
context: formatter.Context{Format: NewFormat("raw", true)},
136+
expected: `plugin_id: pluginID1
115137
plugin_id: pluginID2
116138
`,
117139
},
118-
// Custom Format
119140
{
120-
formatter.Context{Format: NewFormat("{{.Name}}", false)},
121-
`foobar_baz
141+
doc: "custom format",
142+
context: formatter.Context{Format: NewFormat("{{.Name}}", false)},
143+
expected: `foobar_baz
122144
foobar_bar
123145
`,
124146
},
@@ -129,8 +151,8 @@ foobar_bar
129151
{ID: "pluginID2", Name: "foobar_bar", Config: types.PluginConfig{Description: "description 2"}, Enabled: false},
130152
}
131153

132-
for _, tc := range cases {
133-
t.Run(string(tc.context.Format), func(t *testing.T) {
154+
for _, tc := range tests {
155+
t.Run(tc.doc, func(t *testing.T) {
134156
var out bytes.Buffer
135157
tc.context.Output = &out
136158

cli/command/plugin/inspect.go

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,9 @@ func newInspectCommand(dockerCli command.Cli) *cobra.Command {
3636
return cmd
3737
}
3838

39-
func runInspect(ctx context.Context, dockerCli command.Cli, opts inspectOptions) error {
40-
client := dockerCli.Client()
41-
getRef := func(ref string) (any, []byte, error) {
42-
return client.PluginInspectWithRaw(ctx, ref)
43-
}
44-
45-
return inspect.Inspect(dockerCli.Out(), opts.pluginNames, opts.format, getRef)
39+
func runInspect(ctx context.Context, dockerCLI command.Cli, opts inspectOptions) error {
40+
apiClient := dockerCLI.Client()
41+
return inspect.Inspect(dockerCLI.Out(), opts.pluginNames, opts.format, func(ref string) (any, []byte, error) {
42+
return apiClient.PluginInspectWithRaw(ctx, ref)
43+
})
4644
}

cli/command/plugin/inspect_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,14 @@ var pluginFoo = &types.Plugin{
2121
Documentation: "plugin foo documentation",
2222
Entrypoint: []string{"/foo"},
2323
Interface: types.PluginConfigInterface{
24-
Socket: "pluginfoo.sock",
24+
Socket: "plugin-foo.sock",
2525
},
2626
Linux: types.PluginConfigLinux{
2727
Capabilities: []string{"CAP_SYS_ADMIN"},
2828
},
2929
WorkDir: "workdir-foo",
3030
Rootfs: &types.PluginConfigRootfs{
31-
DiffIds: []string{"sha256:8603eedd4ea52cebb2f22b45405a3dc8f78ba3e31bf18f27b4547a9ff930e0bd"},
31+
DiffIds: []string{"sha256:deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef"},
3232
Type: "layers",
3333
},
3434
},
@@ -71,7 +71,7 @@ func TestInspectErrors(t *testing.T) {
7171
cmd := newInspectCommand(cli)
7272
cmd.SetArgs(tc.args)
7373
for key, value := range tc.flags {
74-
cmd.Flags().Set(key, value)
74+
assert.NilError(t, cmd.Flags().Set(key, value))
7575
}
7676
cmd.SetOut(io.Discard)
7777
cmd.SetErr(io.Discard)
@@ -142,7 +142,7 @@ func TestInspect(t *testing.T) {
142142
cmd := newInspectCommand(cli)
143143
cmd.SetArgs(tc.args)
144144
for key, value := range tc.flags {
145-
cmd.Flags().Set(key, value)
145+
assert.NilError(t, cmd.Flags().Set(key, value))
146146
}
147147
assert.NilError(t, cmd.Execute())
148148
golden.Assert(t, cli.OutBuffer().String(), tc.golden)

cli/command/plugin/install.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,9 @@ func runInstall(ctx context.Context, dockerCLI command.Cli, opts pluginOptions)
125125
}
126126
return err
127127
}
128-
defer responseBody.Close()
128+
defer func() {
129+
_ = responseBody.Close()
130+
}()
129131
if err := jsonstream.Display(ctx, responseBody, dockerCLI.Out()); err != nil {
130132
return err
131133
}

cli/command/plugin/install_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ func TestInstallErrors(t *testing.T) {
3232
},
3333
{
3434
description: "invalid plugin name",
35-
args: []string{"UPPERCASE_REPONAME"},
35+
args: []string{"UPPERCASE_REPO_NAME"},
3636
expectedError: "invalid",
3737
},
3838
{

0 commit comments

Comments
 (0)