Merge pull request #6449 from thaJeztah/context_name_positional

cli/command/context: split name from options struct

cli/command/context/create.go

@@ -19,7 +19,6 @@ import (
 
 // createOptions are the options used for creating a context
 type createOptions struct {
-	name        string
 	description string
 	endpoint    map[string]string
 	from        string
@@ -49,8 +48,7 @@ func newCreateCommand(dockerCLI command.Cli) *cobra.Command {
 		Short: "Create a context",
 		Args:  cli.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			opts.name = args[0]
-			return runCreate(dockerCLI, &opts)
+			return runCreate(dockerCLI, args[0], opts)
 		},
 		Long:                  longCreateDescription(),
 		ValidArgsFunction:     cobra.NoFileCompletions,
@@ -64,28 +62,28 @@ func newCreateCommand(dockerCLI command.Cli) *cobra.Command {
 }
 
 // runCreate creates a Docker context
-func runCreate(dockerCLI command.Cli, opts *createOptions) error {
+func runCreate(dockerCLI command.Cli, name string, opts createOptions) error {
 	s := dockerCLI.ContextStore()
-	err := checkContextNameForCreation(s, opts.name)
+	err := checkContextNameForCreation(s, name)
 	if err != nil {
 		return err
 	}
 	switch {
 	case opts.from == "" && opts.endpoint == nil:
-		err = createFromExistingContext(s, dockerCLI.CurrentContext(), opts)
+		err = createFromExistingContext(s, name, dockerCLI.CurrentContext(), opts)
 	case opts.from != "":
-		err = createFromExistingContext(s, opts.from, opts)
+		err = createFromExistingContext(s, name, opts.from, opts)
 	default:
-		err = createNewContext(s, opts)
+		err = createNewContext(s, name, opts)
 	}
 	if err == nil {
-		_, _ = fmt.Fprintln(dockerCLI.Out(), opts.name)
-		_, _ = fmt.Fprintf(dockerCLI.Err(), "Successfully created context %q\n", opts.name)
+		_, _ = fmt.Fprintln(dockerCLI.Out(), name)
+		_, _ = fmt.Fprintf(dockerCLI.Err(), "Successfully created context %q\n", name)
 	}
 	return err
 }
 
-func createNewContext(contextStore store.ReaderWriter, opts *createOptions) error {
+func createNewContext(contextStore store.ReaderWriter, name string, opts createOptions) error {
 	if opts.endpoint == nil {
 		return errors.New("docker endpoint configuration is required")
 	}
@@ -101,7 +99,7 @@ func createNewContext(contextStore store.ReaderWriter, opts *createOptions) erro
 			Description:      opts.description,
 			AdditionalFields: opts.metaData,
 		},
-		Name: opts.name,
+		Name: name,
 	}
 	contextTLSData := store.ContextTLSData{}
 	if dockerTLS != nil {
@@ -115,7 +113,7 @@ func createNewContext(contextStore store.ReaderWriter, opts *createOptions) erro
 	if err := contextStore.CreateOrUpdate(contextMetadata); err != nil {
 		return err
 	}
-	return contextStore.ResetTLSMaterial(opts.name, &contextTLSData)
+	return contextStore.ResetTLSMaterial(name, &contextTLSData)
 }
 
 func checkContextNameForCreation(s store.Reader, name string) error {
@@ -131,7 +129,7 @@ func checkContextNameForCreation(s store.Reader, name string) error {
 	return nil
 }
 
-func createFromExistingContext(s store.ReaderWriter, fromContextName string, opts *createOptions) error {
+func createFromExistingContext(s store.ReaderWriter, name string, fromContextName string, opts createOptions) error {
 	if len(opts.endpoint) != 0 {
 		return errors.New("cannot use --docker flag when --from is set")
 	}
@@ -140,7 +138,7 @@ func createFromExistingContext(s store.ReaderWriter, fromContextName string, opt
 		description: opts.description,
 	})
 	defer reader.Close()
-	return store.Import(opts.name, s, reader)
+	return store.Import(name, s, reader)
 }
 
 type descriptionDecorator struct {

cli/command/context/create_test.go

@@ -61,6 +61,7 @@ func TestCreate(t *testing.T) {
 	tests := []struct {
 		doc         string
 		options     createOptions
+		name        string
 		expecterErr string
 	}{
 		{
@@ -68,30 +69,24 @@ func TestCreate(t *testing.T) {
 			expecterErr: `context name cannot be empty`,
 		},
 		{
-			doc: "reserved name",
-			options: createOptions{
-				name: "default",
-			},
+			doc:         "reserved name",
+			name:        "default",
 			expecterErr: `"default" is a reserved context name`,
 		},
 		{
-			doc: "whitespace-only name",
-			options: createOptions{
-				name: " ",
-			},
+			doc:         "whitespace-only name",
+			name:        " ",
 			expecterErr: `context name " " is invalid`,
 		},
 		{
-			doc: "existing context",
-			options: createOptions{
-				name: "existing-context",
-			},
+			doc:         "existing context",
+			name:        "existing-context",
 			expecterErr: `context "existing-context" already exists`,
 		},
 		{
-			doc: "invalid docker host",
+			doc:  "invalid docker host",
+			name: "invalid-docker-host",
 			options: createOptions{
-				name: "invalid-docker-host",
 				endpoint: map[string]string{
 					"host": "some///invalid/host",
 				},
@@ -99,27 +94,27 @@ func TestCreate(t *testing.T) {
 			expecterErr: `unable to parse docker host`,
 		},
 		{
-			doc: "ssh host with skip-tls-verify=false",
+			doc:  "ssh host with skip-tls-verify=false",
+			name: "skip-tls-verify-false",
 			options: createOptions{
-				name: "skip-tls-verify-false",
 				endpoint: map[string]string{
 					"host": "ssh://example.com,skip-tls-verify=false",
 				},
 			},
 		},
 		{
-			doc: "ssh host with skip-tls-verify=true",
+			doc:  "ssh host with skip-tls-verify=true",
+			name: "skip-tls-verify-true",
 			options: createOptions{
-				name: "skip-tls-verify-true",
 				endpoint: map[string]string{
 					"host": "ssh://example.com,skip-tls-verify=true",
 				},
 			},
 		},
 		{
-			doc: "ssh host with skip-tls-verify=INVALID",
+			doc:  "ssh host with skip-tls-verify=INVALID",
+			name: "skip-tls-verify-invalid",
 			options: createOptions{
-				name: "skip-tls-verify-invalid",
 				endpoint: map[string]string{
 					"host":            "ssh://example.com",
 					"skip-tls-verify": "INVALID",
@@ -128,9 +123,9 @@ func TestCreate(t *testing.T) {
 			expecterErr: `unable to create docker endpoint config: skip-tls-verify: parsing "INVALID": invalid syntax`,
 		},
 		{
-			doc: "unknown option",
+			doc:  "unknown option",
+			name: "unknown-option",
 			options: createOptions{
-				name: "unknown-option",
 				endpoint: map[string]string{
 					"UNKNOWN": "value",
 				},
@@ -140,7 +135,7 @@ func TestCreate(t *testing.T) {
 	}
 	for _, tc := range tests {
 		t.Run(tc.doc, func(t *testing.T) {
-			err := runCreate(cli, &tc.options)
+			err := runCreate(cli, tc.name, tc.options)
 			if tc.expecterErr == "" {
 				assert.NilError(t, err)
 			} else {
@@ -159,8 +154,7 @@ func assertContextCreateLogging(t *testing.T, cli *test.FakeCli, n string) {
 func TestCreateOrchestratorEmpty(t *testing.T) {
 	cli := makeFakeCli(t)
 
-	err := runCreate(cli, &createOptions{
-		name:     "test",
+	err := runCreate(cli, "test", createOptions{
 		endpoint: map[string]string{},
 	})
 	assert.NilError(t, err)
@@ -187,8 +181,7 @@ func TestCreateFromContext(t *testing.T) {
 
 	cli := makeFakeCli(t)
 	cli.ResetOutputBuffers()
-	assert.NilError(t, runCreate(cli, &createOptions{
-		name:        "original",
+	assert.NilError(t, runCreate(cli, "original", createOptions{
 		description: "original description",
 		endpoint: map[string]string{
 			keyHost: "tcp://42.42.42.42:2375",
@@ -197,8 +190,7 @@ func TestCreateFromContext(t *testing.T) {
 	assertContextCreateLogging(t, cli, "original")
 
 	cli.ResetOutputBuffers()
-	assert.NilError(t, runCreate(cli, &createOptions{
-		name:        "dummy",
+	assert.NilError(t, runCreate(cli, "dummy", createOptions{
 		description: "dummy description",
 		endpoint: map[string]string{
 			keyHost: "tcp://24.24.24.24:2375",
@@ -211,9 +203,8 @@ func TestCreateFromContext(t *testing.T) {
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
 			cli.ResetOutputBuffers()
-			err := runCreate(cli, &createOptions{
+			err := runCreate(cli, tc.name, createOptions{
 				from:        "original",
-				name:        tc.name,
 				description: tc.description,
 				endpoint:    tc.docker,
 			})
@@ -251,8 +242,7 @@ func TestCreateFromCurrent(t *testing.T) {
 
 	cli := makeFakeCli(t)
 	cli.ResetOutputBuffers()
-	assert.NilError(t, runCreate(cli, &createOptions{
-		name:        "original",
+	assert.NilError(t, runCreate(cli, "original", createOptions{
 		description: "original description",
 		endpoint: map[string]string{
 			keyHost: "tcp://42.42.42.42:2375",
@@ -265,8 +255,7 @@ func TestCreateFromCurrent(t *testing.T) {
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
 			cli.ResetOutputBuffers()
-			err := runCreate(cli, &createOptions{
-				name:        tc.name,
+			err := runCreate(cli, tc.name, createOptions{
 				description: tc.description,
 			})
 			assert.NilError(t, err)

cli/command/context/list_test.go

@@ -19,8 +19,7 @@ func createTestContexts(t *testing.T, cli command.Cli, name ...string) {
 func createTestContext(t *testing.T, cli command.Cli, name string, metaData map[string]any) {
 	t.Helper()
 
-	err := runCreate(cli, &createOptions{
-		name:        name,
+	err := runCreate(cli, name, createOptions{
 		description: "description of " + name,
 		endpoint:    map[string]string{keyHost: "https://someswarmserver.example.com"},
 

cli/command/context/update.go

@@ -14,7 +14,6 @@ import (
 
 // updateOptions are the options used to update a context.
 type updateOptions struct {
-	name        string
 	description string
 	endpoint    map[string]string
 }
@@ -39,8 +38,7 @@ func newUpdateCommand(dockerCLI command.Cli) *cobra.Command {
 		Short: "Update a context",
 		Args:  cli.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			opts.name = args[0]
-			return runUpdate(dockerCLI, &opts)
+			return runUpdate(dockerCLI, args[0], opts)
 		},
 		Long:                  longUpdateDescription(),
 		ValidArgsFunction:     completeContextNames(dockerCLI, 1, false),
@@ -53,12 +51,12 @@ func newUpdateCommand(dockerCLI command.Cli) *cobra.Command {
 }
 
 // runUpdate updates a Docker context.
-func runUpdate(dockerCLI command.Cli, opts *updateOptions) error {
-	if err := store.ValidateContextName(opts.name); err != nil {
+func runUpdate(dockerCLI command.Cli, name string, opts updateOptions) error {
+	if err := store.ValidateContextName(name); err != nil {
 		return err
 	}
 	s := dockerCLI.ContextStore()
-	c, err := s.GetMetadata(opts.name)
+	c, err := s.GetMetadata(name)
 	if err != nil {
 		return err
 	}
@@ -89,13 +87,13 @@ func runUpdate(dockerCLI command.Cli, opts *updateOptions) error {
 		return err
 	}
 	for ep, tlsData := range tlsDataToReset {
-		if err := s.ResetEndpointTLSMaterial(opts.name, ep, tlsData); err != nil {
+		if err := s.ResetEndpointTLSMaterial(name, ep, tlsData); err != nil {
 			return err
 		}
 	}
 
-	_, _ = fmt.Fprintln(dockerCLI.Out(), opts.name)
-	_, _ = fmt.Fprintf(dockerCLI.Err(), "Successfully updated context %q\n", opts.name)
+	_, _ = fmt.Fprintln(dockerCLI.Out(), name)
+	_, _ = fmt.Fprintf(dockerCLI.Err(), "Successfully updated context %q\n", name)
 	return nil
 }
 

cli/command/context/update_test.go

@@ -11,15 +11,13 @@ import (
 
 func TestUpdateDescriptionOnly(t *testing.T) {
 	cli := makeFakeCli(t)
-	err := runCreate(cli, &createOptions{
-		name:     "test",
+	err := runCreate(cli, "test", createOptions{
 		endpoint: map[string]string{},
 	})
 	assert.NilError(t, err)
 	cli.OutBuffer().Reset()
 	cli.ErrBuffer().Reset()
-	assert.NilError(t, runUpdate(cli, &updateOptions{
-		name:        "test",
+	assert.NilError(t, runUpdate(cli, "test", updateOptions{
 		description: "description",
 	}))
 	c, err := cli.ContextStore().GetMetadata("test")
@@ -35,8 +33,7 @@ func TestUpdateDescriptionOnly(t *testing.T) {
 func TestUpdateDockerOnly(t *testing.T) {
 	cli := makeFakeCli(t)
 	createTestContext(t, cli, "test", nil)
-	assert.NilError(t, runUpdate(cli, &updateOptions{
-		name: "test",
+	assert.NilError(t, runUpdate(cli, "test", updateOptions{
 		endpoint: map[string]string{
 			keyHost: "tcp://some-host",
 		},
@@ -52,13 +49,11 @@ func TestUpdateDockerOnly(t *testing.T) {
 
 func TestUpdateInvalidDockerHost(t *testing.T) {
 	cli := makeFakeCli(t)
-	err := runCreate(cli, &createOptions{
-		name:     "test",
+	err := runCreate(cli, "test", createOptions{
 		endpoint: map[string]string{},
 	})
 	assert.NilError(t, err)
-	err = runUpdate(cli, &updateOptions{
-		name: "test",
+	err = runUpdate(cli, "test", updateOptions{
 		endpoint: map[string]string{
 			keyHost: "some///invalid/host",
 		},

cli/command/context/use_test.go

@@ -23,8 +23,7 @@ func TestUse(t *testing.T) {
 	configFilePath := filepath.Join(configDir, "config.json")
 	testCfg := configfile.New(configFilePath)
 	cli := makeFakeCli(t, withCliConfig(testCfg))
-	err := runCreate(cli, &createOptions{
-		name:     "test",
+	err := runCreate(cli, "test", createOptions{
 		endpoint: map[string]string{},
 	})
 	assert.NilError(t, err)
@@ -89,8 +88,7 @@ func TestUseHostOverride(t *testing.T) {
 	configFilePath := filepath.Join(configDir, "config.json")
 	testCfg := configfile.New(configFilePath)
 	cli := makeFakeCli(t, withCliConfig(testCfg))
-	err := runCreate(cli, &createOptions{
-		name:     "test",
+	err := runCreate(cli, "test", createOptions{
 		endpoint: map[string]string{},
 	})
 	assert.NilError(t, err)
@@ -136,8 +134,7 @@ func TestUseHostOverrideEmpty(t *testing.T) {
 		assert.NilError(t, cli.Initialize(flags.NewClientOptions()))
 	}
 	loadCli()
-	err := runCreate(cli, &createOptions{
-		name:     "test",
+	err := runCreate(cli, "test", createOptions{
 		endpoint: map[string]string{"host": socketPath},
 	})
 	assert.NilError(t, err)