Skip to content

Commit ea8212a

Browse files
thaJeztahthaJeztah
committed
cli/command/context: split name from options struct
Name is a required argument for both "create" and "update", so better to split it from the options struct. Signed-off-by: Sebastiaan van Stijn <[email protected]>
1 parent c5467b5 commit ea8212a

File tree

6 files changed

+53
-77
lines changed

6 files changed

+53
-77
lines changed

‎cli/command/context/create.go‎

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919

2020
// createOptions are the options used for creating a context
2121
type createOptions struct {
22-
name string
2322
description string
2423
endpoint map[string]string
2524
from string
@@ -49,8 +48,7 @@ func newCreateCommand(dockerCLI command.Cli) *cobra.Command {
4948
Short: "Create a context",
5049
Args: cli.ExactArgs(1),
5150
RunE: func(cmd *cobra.Command, args []string) error {
52-
opts.name = args[0]
53-
return runCreate(dockerCLI, &opts)
51+
return runCreate(dockerCLI, args[0], opts)
5452
},
5553
Long: longCreateDescription(),
5654
ValidArgsFunction: cobra.NoFileCompletions,
@@ -64,28 +62,28 @@ func newCreateCommand(dockerCLI command.Cli) *cobra.Command {
6462
}
6563

6664
// runCreate creates a Docker context
67-
func runCreate(dockerCLI command.Cli, opts *createOptions) error {
65+
func runCreate(dockerCLI command.Cli, name string, opts createOptions) error {
6866
s := dockerCLI.ContextStore()
69-
err := checkContextNameForCreation(s, opts.name)
67+
err := checkContextNameForCreation(s, name)
7068
if err != nil {
7169
return err
7270
}
7371
switch {
7472
case opts.from == "" && opts.endpoint == nil:
75-
err = createFromExistingContext(s, dockerCLI.CurrentContext(), opts)
73+
err = createFromExistingContext(s, name, dockerCLI.CurrentContext(), opts)
7674
case opts.from != "":
77-
err = createFromExistingContext(s, opts.from, opts)
75+
err = createFromExistingContext(s, name, opts.from, opts)
7876
default:
79-
err = createNewContext(s, opts)
77+
err = createNewContext(s, name, opts)
8078
}
8179
if err == nil {
82-
_, _ = fmt.Fprintln(dockerCLI.Out(), opts.name)
83-
_, _ = fmt.Fprintf(dockerCLI.Err(), "Successfully created context %q\n", opts.name)
80+
_, _ = fmt.Fprintln(dockerCLI.Out(), name)
81+
_, _ = fmt.Fprintf(dockerCLI.Err(), "Successfully created context %q\n", name)
8482
}
8583
return err
8684
}
8785

88-
func createNewContext(contextStore store.ReaderWriter, opts *createOptions) error {
86+
func createNewContext(contextStore store.ReaderWriter, name string, opts createOptions) error {
8987
if opts.endpoint == nil {
9088
return errors.New("docker endpoint configuration is required")
9189
}
@@ -101,7 +99,7 @@ func createNewContext(contextStore store.ReaderWriter, opts *createOptions) erro
10199
Description: opts.description,
102100
AdditionalFields: opts.metaData,
103101
},
104-
Name: opts.name,
102+
Name: name,
105103
}
106104
contextTLSData := store.ContextTLSData{}
107105
if dockerTLS != nil {
@@ -115,7 +113,7 @@ func createNewContext(contextStore store.ReaderWriter, opts *createOptions) erro
115113
if err := contextStore.CreateOrUpdate(contextMetadata); err != nil {
116114
return err
117115
}
118-
return contextStore.ResetTLSMaterial(opts.name, &contextTLSData)
116+
return contextStore.ResetTLSMaterial(name, &contextTLSData)
119117
}
120118

121119
func checkContextNameForCreation(s store.Reader, name string) error {
@@ -131,7 +129,7 @@ func checkContextNameForCreation(s store.Reader, name string) error {
131129
return nil
132130
}
133131

134-
func createFromExistingContext(s store.ReaderWriter, fromContextName string, opts *createOptions) error {
132+
func createFromExistingContext(s store.ReaderWriter, name string, fromContextName string, opts createOptions) error {
135133
if len(opts.endpoint) != 0 {
136134
return errors.New("cannot use --docker flag when --from is set")
137135
}
@@ -140,7 +138,7 @@ func createFromExistingContext(s store.ReaderWriter, fromContextName string, opt
140138
description: opts.description,
141139
})
142140
defer reader.Close()
143-
return store.Import(opts.name, s, reader)
141+
return store.Import(name, s, reader)
144142
}
145143

146144
type descriptionDecorator struct {

‎cli/command/context/create_test.go‎

Lines changed: 24 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -61,65 +61,60 @@ func TestCreate(t *testing.T) {
6161
tests := []struct {
6262
doc string
6363
options createOptions
64+
name string
6465
expecterErr string
6566
}{
6667
{
6768
doc: "empty name",
6869
expecterErr: `context name cannot be empty`,
6970
},
7071
{
71-
doc: "reserved name",
72-
options: createOptions{
73-
name: "default",
74-
},
72+
doc: "reserved name",
73+
name: "default",
7574
expecterErr: `"default" is a reserved context name`,
7675
},
7776
{
78-
doc: "whitespace-only name",
79-
options: createOptions{
80-
name: " ",
81-
},
77+
doc: "whitespace-only name",
78+
name: " ",
8279
expecterErr: `context name " " is invalid`,
8380
},
8481
{
85-
doc: "existing context",
86-
options: createOptions{
87-
name: "existing-context",
88-
},
82+
doc: "existing context",
83+
name: "existing-context",
8984
expecterErr: `context "existing-context" already exists`,
9085
},
9186
{
92-
doc: "invalid docker host",
87+
doc: "invalid docker host",
88+
name: "invalid-docker-host",
9389
options: createOptions{
94-
name: "invalid-docker-host",
9590
endpoint: map[string]string{
9691
"host": "some///invalid/host",
9792
},
9893
},
9994
expecterErr: `unable to parse docker host`,
10095
},
10196
{
102-
doc: "ssh host with skip-tls-verify=false",
97+
doc: "ssh host with skip-tls-verify=false",
98+
name: "skip-tls-verify-false",
10399
options: createOptions{
104-
name: "skip-tls-verify-false",
105100
endpoint: map[string]string{
106101
"host": "ssh://example.com,skip-tls-verify=false",
107102
},
108103
},
109104
},
110105
{
111-
doc: "ssh host with skip-tls-verify=true",
106+
doc: "ssh host with skip-tls-verify=true",
107+
name: "skip-tls-verify-true",
112108
options: createOptions{
113-
name: "skip-tls-verify-true",
114109
endpoint: map[string]string{
115110
"host": "ssh://example.com,skip-tls-verify=true",
116111
},
117112
},
118113
},
119114
{
120-
doc: "ssh host with skip-tls-verify=INVALID",
115+
doc: "ssh host with skip-tls-verify=INVALID",
116+
name: "skip-tls-verify-invalid",
121117
options: createOptions{
122-
name: "skip-tls-verify-invalid",
123118
endpoint: map[string]string{
124119
"host": "ssh://example.com",
125120
"skip-tls-verify": "INVALID",
@@ -128,9 +123,9 @@ func TestCreate(t *testing.T) {
128123
expecterErr: `unable to create docker endpoint config: skip-tls-verify: parsing "INVALID": invalid syntax`,
129124
},
130125
{
131-
doc: "unknown option",
126+
doc: "unknown option",
127+
name: "unknown-option",
132128
options: createOptions{
133-
name: "unknown-option",
134129
endpoint: map[string]string{
135130
"UNKNOWN": "value",
136131
},
@@ -140,7 +135,7 @@ func TestCreate(t *testing.T) {
140135
}
141136
for _, tc := range tests {
142137
t.Run(tc.doc, func(t *testing.T) {
143-
err := runCreate(cli, &tc.options)
138+
err := runCreate(cli, tc.name, tc.options)
144139
if tc.expecterErr == "" {
145140
assert.NilError(t, err)
146141
} else {
@@ -159,8 +154,7 @@ func assertContextCreateLogging(t *testing.T, cli *test.FakeCli, n string) {
159154
func TestCreateOrchestratorEmpty(t *testing.T) {
160155
cli := makeFakeCli(t)
161156

162-
err := runCreate(cli, &createOptions{
163-
name: "test",
157+
err := runCreate(cli, "test", createOptions{
164158
endpoint: map[string]string{},
165159
})
166160
assert.NilError(t, err)
@@ -187,8 +181,7 @@ func TestCreateFromContext(t *testing.T) {
187181

188182
cli := makeFakeCli(t)
189183
cli.ResetOutputBuffers()
190-
assert.NilError(t, runCreate(cli, &createOptions{
191-
name: "original",
184+
assert.NilError(t, runCreate(cli, "original", createOptions{
192185
description: "original description",
193186
endpoint: map[string]string{
194187
keyHost: "tcp://42.42.42.42:2375",
@@ -197,8 +190,7 @@ func TestCreateFromContext(t *testing.T) {
197190
assertContextCreateLogging(t, cli, "original")
198191

199192
cli.ResetOutputBuffers()
200-
assert.NilError(t, runCreate(cli, &createOptions{
201-
name: "dummy",
193+
assert.NilError(t, runCreate(cli, "dummy", createOptions{
202194
description: "dummy description",
203195
endpoint: map[string]string{
204196
keyHost: "tcp://24.24.24.24:2375",
@@ -211,9 +203,8 @@ func TestCreateFromContext(t *testing.T) {
211203
for _, tc := range cases {
212204
t.Run(tc.name, func(t *testing.T) {
213205
cli.ResetOutputBuffers()
214-
err := runCreate(cli, &createOptions{
206+
err := runCreate(cli, tc.name, createOptions{
215207
from: "original",
216-
name: tc.name,
217208
description: tc.description,
218209
endpoint: tc.docker,
219210
})
@@ -251,8 +242,7 @@ func TestCreateFromCurrent(t *testing.T) {
251242

252243
cli := makeFakeCli(t)
253244
cli.ResetOutputBuffers()
254-
assert.NilError(t, runCreate(cli, &createOptions{
255-
name: "original",
245+
assert.NilError(t, runCreate(cli, "original", createOptions{
256246
description: "original description",
257247
endpoint: map[string]string{
258248
keyHost: "tcp://42.42.42.42:2375",
@@ -265,8 +255,7 @@ func TestCreateFromCurrent(t *testing.T) {
265255
for _, tc := range cases {
266256
t.Run(tc.name, func(t *testing.T) {
267257
cli.ResetOutputBuffers()
268-
err := runCreate(cli, &createOptions{
269-
name: tc.name,
258+
err := runCreate(cli, tc.name, createOptions{
270259
description: tc.description,
271260
})
272261
assert.NilError(t, err)

‎cli/command/context/list_test.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,7 @@ func createTestContexts(t *testing.T, cli command.Cli, name ...string) {
1919
func createTestContext(t *testing.T, cli command.Cli, name string, metaData map[string]any) {
2020
t.Helper()
2121

22-
err := runCreate(cli, &createOptions{
23-
name: name,
22+
err := runCreate(cli, name, createOptions{
2423
description: "description of " + name,
2524
endpoint: map[string]string{keyHost: "https://someswarmserver.example.com"},
2625

‎cli/command/context/update.go‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414

1515
// updateOptions are the options used to update a context.
1616
type updateOptions struct {
17-
name string
1817
description string
1918
endpoint map[string]string
2019
}
@@ -39,8 +38,7 @@ func newUpdateCommand(dockerCLI command.Cli) *cobra.Command {
3938
Short: "Update a context",
4039
Args: cli.ExactArgs(1),
4140
RunE: func(cmd *cobra.Command, args []string) error {
42-
opts.name = args[0]
43-
return runUpdate(dockerCLI, &opts)
41+
return runUpdate(dockerCLI, args[0], opts)
4442
},
4543
Long: longUpdateDescription(),
4644
ValidArgsFunction: completeContextNames(dockerCLI, 1, false),
@@ -53,12 +51,12 @@ func newUpdateCommand(dockerCLI command.Cli) *cobra.Command {
5351
}
5452

5553
// runUpdate updates a Docker context.
56-
func runUpdate(dockerCLI command.Cli, opts *updateOptions) error {
57-
if err := store.ValidateContextName(opts.name); err != nil {
54+
func runUpdate(dockerCLI command.Cli, name string, opts updateOptions) error {
55+
if err := store.ValidateContextName(name); err != nil {
5856
return err
5957
}
6058
s := dockerCLI.ContextStore()
61-
c, err := s.GetMetadata(opts.name)
59+
c, err := s.GetMetadata(name)
6260
if err != nil {
6361
return err
6462
}
@@ -89,13 +87,13 @@ func runUpdate(dockerCLI command.Cli, opts *updateOptions) error {
8987
return err
9088
}
9189
for ep, tlsData := range tlsDataToReset {
92-
if err := s.ResetEndpointTLSMaterial(opts.name, ep, tlsData); err != nil {
90+
if err := s.ResetEndpointTLSMaterial(name, ep, tlsData); err != nil {
9391
return err
9492
}
9593
}
9694

97-
_, _ = fmt.Fprintln(dockerCLI.Out(), opts.name)
98-
_, _ = fmt.Fprintf(dockerCLI.Err(), "Successfully updated context %q\n", opts.name)
95+
_, _ = fmt.Fprintln(dockerCLI.Out(), name)
96+
_, _ = fmt.Fprintf(dockerCLI.Err(), "Successfully updated context %q\n", name)
9997
return nil
10098
}
10199

‎cli/command/context/update_test.go‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,13 @@ import (
1111

1212
func TestUpdateDescriptionOnly(t *testing.T) {
1313
cli := makeFakeCli(t)
14-
err := runCreate(cli, &createOptions{
15-
name: "test",
14+
err := runCreate(cli, "test", createOptions{
1615
endpoint: map[string]string{},
1716
})
1817
assert.NilError(t, err)
1918
cli.OutBuffer().Reset()
2019
cli.ErrBuffer().Reset()
21-
assert.NilError(t, runUpdate(cli, &updateOptions{
22-
name: "test",
20+
assert.NilError(t, runUpdate(cli, "test", updateOptions{
2321
description: "description",
2422
}))
2523
c, err := cli.ContextStore().GetMetadata("test")
@@ -35,8 +33,7 @@ func TestUpdateDescriptionOnly(t *testing.T) {
3533
func TestUpdateDockerOnly(t *testing.T) {
3634
cli := makeFakeCli(t)
3735
createTestContext(t, cli, "test", nil)
38-
assert.NilError(t, runUpdate(cli, &updateOptions{
39-
name: "test",
36+
assert.NilError(t, runUpdate(cli, "test", updateOptions{
4037
endpoint: map[string]string{
4138
keyHost: "tcp://some-host",
4239
},
@@ -52,13 +49,11 @@ func TestUpdateDockerOnly(t *testing.T) {
5249

5350
func TestUpdateInvalidDockerHost(t *testing.T) {
5451
cli := makeFakeCli(t)
55-
err := runCreate(cli, &createOptions{
56-
name: "test",
52+
err := runCreate(cli, "test", createOptions{
5753
endpoint: map[string]string{},
5854
})
5955
assert.NilError(t, err)
60-
err = runUpdate(cli, &updateOptions{
61-
name: "test",
56+
err = runUpdate(cli, "test", updateOptions{
6257
endpoint: map[string]string{
6358
keyHost: "some///invalid/host",
6459
},

‎cli/command/context/use_test.go‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,7 @@ func TestUse(t *testing.T) {
2323
configFilePath := filepath.Join(configDir, "config.json")
2424
testCfg := configfile.New(configFilePath)
2525
cli := makeFakeCli(t, withCliConfig(testCfg))
26-
err := runCreate(cli, &createOptions{
27-
name: "test",
26+
err := runCreate(cli, "test", createOptions{
2827
endpoint: map[string]string{},
2928
})
3029
assert.NilError(t, err)
@@ -89,8 +88,7 @@ func TestUseHostOverride(t *testing.T) {
8988
configFilePath := filepath.Join(configDir, "config.json")
9089
testCfg := configfile.New(configFilePath)
9190
cli := makeFakeCli(t, withCliConfig(testCfg))
92-
err := runCreate(cli, &createOptions{
93-
name: "test",
91+
err := runCreate(cli, "test", createOptions{
9492
endpoint: map[string]string{},
9593
})
9694
assert.NilError(t, err)
@@ -136,8 +134,7 @@ func TestUseHostOverrideEmpty(t *testing.T) {
136134
assert.NilError(t, cli.Initialize(flags.NewClientOptions()))
137135
}
138136
loadCli()
139-
err := runCreate(cli, &createOptions{
140-
name: "test",
137+
err := runCreate(cli, "test", createOptions{
141138
endpoint: map[string]string{"host": socketPath},
142139
})
143140
assert.NilError(t, err)

0 commit comments

Comments
 (0)