CLOUDP-137380: [CLI] profile names with dots(.) break the cli (#2566)

jeroenvervaeke · andreaangiolillo · Gustavo Bazan · web-flow · commit af1c7c52ee35 · 2024-01-25T09:46:52.000Z
Co-authored-by: Andrea Angiolillo &lt;andrea.angiolillo@mongodb.com&gt;
Co-authored-by: Gustavo Bazan &lt;gustavo.bazan@mongodb.com&gt;
diff --git a/cmd/mongocli/mongocli.go b/cmd/mongocli/mongocli.go
@@ -24,9 +24,9 @@ import (
 	"strings"
 
 	survey "github.com/AlecAivazis/survey/v2/core"
+	"github.com/mongodb/mongodb-atlas-cli/internal/cli"
 	"github.com/mongodb/mongodb-atlas-cli/internal/cli/root/mongocli"
 	"github.com/mongodb/mongodb-atlas-cli/internal/config"
-	"github.com/mongodb/mongodb-atlas-cli/internal/flag"
 	"github.com/spf13/cobra"
 )
 
@@ -106,12 +106,8 @@ func initConfig() {
 		log.Fatalf("Error loading config: %v", err)
 	}
 
-	if profile != "" {
-		config.SetName(profile)
-	} else if profile = config.GetString(flag.Profile); profile != "" {
-		config.SetName(profile)
-	} else if availableProfiles := config.List(); len(availableProfiles) == 1 {
-		config.SetName(availableProfiles[0])
+	if err := cli.InitProfile(profile); err != nil {
+		log.Fatalf("Error loading profile: %v", err)
 	}
 }
 
diff --git a/internal/cli/atlas/clusters/region_tier_autocomplete.go b/internal/cli/atlas/clusters/region_tier_autocomplete.go
@@ -16,6 +16,7 @@ package clusters
 
 import (
 	"context"
+	"fmt"
 	"sort"
 	"strings"
 
@@ -38,7 +39,11 @@ type autoCompleteOpts struct {
 
 func (opts *autoCompleteOpts) autocompleteTier() cli.AutoFunc {
 	return func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
-		opts.parseFlags(cmd)
+		if err := opts.parseFlags(cmd); err != nil {
+			cobra.CompErrorln(fmt.Sprintf("failed to parse flags: %v", err))
+			return nil, cobra.ShellCompDirectiveError
+		}
+
 		if err := validate.Credentials(); err != nil {
 			cobra.CompErrorln("no credentials")
 			return nil, cobra.ShellCompDirectiveError
@@ -86,7 +91,11 @@ func (opts *autoCompleteOpts) tierSuggestions(toComplete string) ([]string, erro
 
 func (opts *autoCompleteOpts) autocompleteRegion() cli.AutoFunc {
 	return func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
-		opts.parseFlags(cmd)
+		if err := opts.parseFlags(cmd); err != nil {
+			cobra.CompErrorln(fmt.Sprintf("failed to parse flags: %v", err))
+			return nil, cobra.ShellCompDirectiveError
+		}
+
 		if err := validate.Credentials(); err != nil {
 			cobra.CompErrorln("no credentials")
 			return nil, cobra.ShellCompDirectiveError
@@ -139,15 +148,13 @@ func (opts *autoCompleteOpts) initStore(ctx context.Context) error {
 	return err
 }
 
-func (opts *autoCompleteOpts) parseFlags(cmd *cobra.Command) {
+func (opts *autoCompleteOpts) parseFlags(cmd *cobra.Command) error {
 	profile := cmd.Flag(flag.Profile).Value.String()
-	if profile != "" {
-		config.SetName(profile)
-	} else if profile = config.GetString(flag.Profile); profile != "" {
-		config.SetName(profile)
-	} else if availableProfiles := config.List(); len(availableProfiles) == 1 {
-		config.SetName(availableProfiles[0])
+
+	if err := cli.InitProfile(profile); err != nil {
+		return err
 	}
+
 	if project := cmd.Flag(flag.ProjectID).Value.String(); project != "" {
 		opts.ProjectID = project
 	}
@@ -159,4 +166,6 @@ func (opts *autoCompleteOpts) parseFlags(cmd *cobra.Command) {
 	if tier := cmd.Flag(flag.Tier).Value.String(); tier != "" {
 		opts.tier = tier
 	}
+
+	return nil
 }
diff --git a/internal/cli/atlas/processes/process_autocomplete.go b/internal/cli/atlas/processes/process_autocomplete.go
@@ -16,6 +16,7 @@ package processes
 
 import (
 	"context"
+	"fmt"
 	"sort"
 	"strings"
 
@@ -35,7 +36,11 @@ type AutoCompleteOpts struct {
 
 func (opts *AutoCompleteOpts) AutocompleteProcesses() cli.AutoFunc {
 	return func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
-		opts.parseFlags(cmd)
+		if err := opts.parseFlags(cmd); err != nil {
+			cobra.CompErrorln(fmt.Sprintf("failed to parse flags: %v", err))
+			return nil, cobra.ShellCompDirectiveError
+		}
+
 		if err := validate.Credentials(); err != nil {
 			cobra.CompErrorln("no credentials")
 			return nil, cobra.ShellCompDirectiveError
@@ -82,16 +87,16 @@ func (opts *AutoCompleteOpts) initStore(ctx context.Context) error {
 	return err
 }
 
-func (opts *AutoCompleteOpts) parseFlags(cmd *cobra.Command) {
+func (opts *AutoCompleteOpts) parseFlags(cmd *cobra.Command) error {
 	profile := cmd.Flag(flag.Profile).Value.String()
-	if profile != "" {
-		config.SetName(profile)
-	} else if profile = config.GetString(flag.Profile); profile != "" {
-		config.SetName(profile)
-	} else if availableProfiles := config.List(); len(availableProfiles) == 1 {
-		config.SetName(availableProfiles[0])
+
+	if err := cli.InitProfile(profile); err != nil {
+		return err
 	}
+
 	if project := cmd.Flag(flag.ProjectID).Value.String(); project != "" {
 		opts.ProjectID = project
 	}
+
+	return nil
 }
diff --git a/internal/cli/config/delete.go b/internal/cli/config/delete.go
@@ -34,7 +34,10 @@ func (opts *DeleteOpts) Run() error {
 		return nil
 	}
 
-	config.SetName(opts.Entry)
+	if err := config.SetName(opts.Entry); err != nil {
+		return err
+	}
+
 	if err := config.Delete(); err != nil {
 		return err
 	}
diff --git a/internal/cli/config/describe.go b/internal/cli/config/describe.go
@@ -38,7 +38,11 @@ func (opts *describeOpts) Run() error {
 	if !config.Exists(opts.name) {
 		return fmt.Errorf("you don't have a profile named '%s'", opts.name)
 	}
-	config.SetName(opts.name)
+
+	if err := config.SetName(opts.name); err != nil {
+		return err
+	}
+
 	return opts.Print(config.Map())
 }
 
diff --git a/internal/cli/config/rename.go b/internal/cli/config/rename.go
@@ -47,7 +47,10 @@ func (opts *RenameOpts) Run() error {
 		}
 	}
 
-	config.SetName(opts.oldName)
+	if err := config.SetName(opts.oldName); err != nil {
+		return err
+	}
+
 	if err := config.Rename(opts.newName); err != nil {
 		return err
 	}
diff --git a/internal/cli/profile.go b/internal/cli/profile.go
@@ -0,0 +1,32 @@
+// Copyright 2024 MongoDB Inc
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package cli
+
+import (
+	"github.com/mongodb/mongodb-atlas-cli/internal/config"
+	"github.com/mongodb/mongodb-atlas-cli/internal/flag"
+)
+
+func InitProfile(profile string) error {
+	if profile != "" {
+		return config.SetName(profile)
+	} else if profile = config.GetString(flag.Profile); profile != "" {
+		return config.SetName(profile)
+	} else if availableProfiles := config.List(); len(availableProfiles) == 1 {
+		return config.SetName(availableProfiles[0])
+	}
+
+	return nil
+}
diff --git a/internal/cli/root/atlas/builder.go b/internal/cli/root/atlas/builder.go
@@ -107,16 +107,6 @@ func handleSignal() {
 	}, os.Interrupt, syscall.SIGTERM)
 }
 
-func initProfile(profile string) {
-	if profile != "" {
-		config.SetName(profile)
-	} else if profile = config.GetString(flag.Profile); profile != "" {
-		config.SetName(profile)
-	} else if availableProfiles := config.List(); len(availableProfiles) == 1 {
-		config.SetName(availableProfiles[0])
-	}
-}
-
 // Builder conditionally adds children commands as needed.
 func Builder() *cobra.Command {
 	var (
@@ -144,7 +134,9 @@ Use the --help flag with any command for more info on that command.`,
 				log.SetLevel(log.DebugLevel)
 			}
 
-			initProfile(profile)
+			if err := cli.InitProfile(profile); err != nil {
+				return err
+			}
 
 			telemetry.StartTrackingCommand(cmd, args)
 
diff --git a/internal/config/profile.go b/internal/config/profile.go
@@ -241,9 +241,25 @@ func (p *Profile) Name() string {
 	return p.name
 }
 
-func SetName(name string) { Default().SetName(name) }
-func (p *Profile) SetName(name string) {
+var ErrProfileNameHasDots = errors.New("profile should not contain '.'")
+
+func validateName(name string) error {
+	if strings.Contains(name, ".") {
+		return fmt.Errorf("%w: %q", ErrProfileNameHasDots, name)
+	}
+
+	return nil
+}
+
+func SetName(name string) error { return Default().SetName(name) }
+func (p *Profile) SetName(name string) error {
+	if err := validateName(name); err != nil {
+		return err
+	}
+
 	p.name = strings.ToLower(name)
+
+	return nil
 }
 
 func Set(name string, value interface{}) { Default().Set(name, value) }
@@ -623,6 +639,10 @@ func Filename() string {
 // Rename replaces the Profile to a new Profile name, overwriting any Profile that existed before.
 func Rename(newProfileName string) error { return Default().Rename(newProfileName) }
 func (p *Profile) Rename(newProfileName string) error {
+	if err := validateName(newProfileName); err != nil {
+		return err
+	}
+
 	// Configuration needs to be deleted from toml, as viper doesn't support this yet.
 	// FIXME :: change when https://github.com/spf13/viper/pull/519 is merged.
 	configurationAfterDelete := viper.AllSettings()
diff --git a/internal/config/profile_integration_test.go b/internal/config/profile_integration_test.go
@@ -117,7 +117,7 @@ func TestProfile_Get_Default(t *testing.T) {
 
 func TestProfile_Get_NonDefault(t *testing.T) {
 	profile := profileWithOneNonDefaultUser()
-	profile.SetName(atlas)
+	require.NoError(t, profile.SetName(atlas))
 
 	require.NoError(t, profile.LoadMongoCLIConfig(false))
 
@@ -134,7 +134,7 @@ func TestProfile_Delete_NonDefault(t *testing.T) {
 	profile := profileWithOneDefaultUserOneNonDefault()
 	require.NoError(t, profile.LoadMongoCLIConfig(false))
 
-	profile.SetName(atlas)
+	require.NoError(t, profile.SetName(atlas))
 
 	require.NoError(t, profile.Delete())
 	require.NoError(t, profile.LoadMongoCLIConfig(false))
@@ -144,31 +144,31 @@ func TestProfile_Delete_NonDefault(t *testing.T) {
 
 func TestProfile_Rename(t *testing.T) {
 	profile := profileWithOneDefaultUserOneNonDefault()
-	profile.SetName(DefaultProfile)
+	require.NoError(t, profile.SetName(DefaultProfile))
 
 	require.NoError(t, profile.LoadMongoCLIConfig(false))
 	defaultDescription := profile.Map()
 	require.NoError(t, profile.Rename(newProfileName))
 	require.NoError(t, profile.LoadMongoCLIConfig(false))
-	profile.SetName(DefaultProfile)
+	require.NoError(t, profile.SetName(DefaultProfile))
 	a := assert.New(t)
 	a.Empty(profile.Map())
-	profile.SetName(newProfileName)
+	require.NoError(t, profile.SetName(newProfileName))
 	descriptionAfterRename := profile.Map()
 	// after renaming, one Profile should exist
 	a.Equal(defaultDescription, descriptionAfterRename, "descriptions should be equal after renaming")
 }
 
 func TestProfile_Rename_OverwriteExisting(t *testing.T) {
 	profile := profileWithOneDefaultUserOneNonDefault()
-	profile.SetName(DefaultProfile)
+	require.NoError(t, profile.SetName(DefaultProfile))
 
 	require.NoError(t, profile.LoadMongoCLIConfig(false))
 	defaultDescription := profile.Map()
 
 	require.NoError(t, profile.Rename(atlas))
 	require.NoError(t, profile.LoadMongoCLIConfig(false))
-	profile.SetName(atlas)
+	require.NoError(t, profile.SetName(atlas))
 	descriptionAfterRename := profile.Map()
 	// after renaming, one Profile should exist
 	assert.Equal(t, defaultDescription, descriptionAfterRename, "descriptions should be equal after renaming")
@@ -178,7 +178,7 @@ func TestProfile_Set(t *testing.T) {
 	profile := profileWithOneDefaultUserOneNonDefault()
 	require.NoError(t, profile.LoadMongoCLIConfig(false))
 
-	profile.SetName(DefaultProfile)
+	require.NoError(t, profile.SetName(DefaultProfile))
 
 	profile.Set(projectID, "newProjectId")
 
diff --git a/internal/config/profile_test.go b/internal/config/profile_test.go
@@ -22,6 +22,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestConfig_MongoCLIConfigHome(t *testing.T) {
@@ -232,3 +233,31 @@ func Test_getConfigHostname(t *testing.T) {
 		})
 	}
 }
+
+func TestConfig_SetName(t *testing.T) {
+	t.Run("valid", func(t *testing.T) {
+		require.NoError(t, SetName("default"))
+		require.NoError(t, SetName("default-123"))
+		require.NoError(t, SetName("default-test"))
+	})
+
+	t.Run("invalid", func(t *testing.T) {
+		require.Error(t, SetName("d.efault"))
+		require.Error(t, SetName("default.123"))
+		require.Error(t, SetName("default.test"))
+	})
+}
+
+func TestConfig_Rename(t *testing.T) {
+	t.Run("valid", func(t *testing.T) {
+		require.NoError(t, Rename("default"))
+		require.NoError(t, Rename("default-123"))
+		require.NoError(t, Rename("default-test"))
+	})
+
+	t.Run("invalid", func(t *testing.T) {
+		require.Error(t, Rename("d.efault"))
+		require.Error(t, Rename("default.123"))
+		require.Error(t, Rename("default.test"))
+	})
+}
diff --git a/test/e2e/atlas/helper_test.go b/test/e2e/atlas/helper_test.go
@@ -98,6 +98,7 @@ const (
 	setupEntity                  = "setup"
 	deploymentEntity             = "deployments"
 	deletingState                = "DELETING"
+	authEntity                   = "auth"
 )
 
 // AlertConfig constants.
@@ -108,6 +109,11 @@ const (
 	delayMin      = 0
 )
 
+// Auth constants.
+const (
+	whoami = "whoami"
+)
+
 // Integration constants.
 const (
 	datadogEntity   = "DATADOG"
diff --git a/test/e2e/atlas/profile_test.go b/test/e2e/atlas/profile_test.go

Original file line number	Diff line number	Diff line change
`@@ -24,9 +24,9 @@ import (`
`24`	`24`	`"strings"`
`25`	`25`
`26`	`26`	`survey "github.com/AlecAivazis/survey/v2/core"`
	`27`	`+ "github.com/mongodb/mongodb-atlas-cli/internal/cli"`
`27`	`28`	`"github.com/mongodb/mongodb-atlas-cli/internal/cli/root/mongocli"`
`28`	`29`	`"github.com/mongodb/mongodb-atlas-cli/internal/config"`
`29`		`- "github.com/mongodb/mongodb-atlas-cli/internal/flag"`
`30`	`30`	`"github.com/spf13/cobra"`
`31`	`31`	`)`
`32`	`32`
`@@ -106,12 +106,8 @@ func initConfig() {`
`106`	`106`	`log.Fatalf("Error loading config: %v", err)`
`107`	`107`	`}`
`108`	`108`
`109`		`- if profile != "" {`
`110`		`- config.SetName(profile)`
`111`		`- } else if profile = config.GetString(flag.Profile); profile != "" {`
`112`		`- config.SetName(profile)`
`113`		`- } else if availableProfiles := config.List(); len(availableProfiles) == 1 {`
`114`		`- config.SetName(availableProfiles[0])`
	`109`	`+ if err := cli.InitProfile(profile); err != nil {`
	`110`	`+ log.Fatalf("Error loading profile: %v", err)`
`115`	`111`	`}`
`116`	`112`	`}`
`117`	`113`
Original file line number	Diff line number	Diff line change
`@@ -34,7 +34,10 @@ func (opts *DeleteOpts) Run() error {`
`34`	`34`	`return nil`
`35`	`35`	`}`
`36`	`36`
`37`		`- config.SetName(opts.Entry)`
	`37`	`+ if err := config.SetName(opts.Entry); err != nil {`
	`38`	`+ return err`
	`39`	`+ }`
	`40`	`+`
`38`	`41`	`if err := config.Delete(); err != nil {`
`39`	`42`	`return err`
`40`	`43`	`}`
Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,11 @@ func (opts *describeOpts) Run() error {`
`38`	`38`	`if !config.Exists(opts.name) {`
`39`	`39`	`return fmt.Errorf("you don't have a profile named '%s'", opts.name)`
`40`	`40`	`}`
`41`		`- config.SetName(opts.name)`
	`41`	`+`
	`42`	`+ if err := config.SetName(opts.name); err != nil {`
	`43`	`+ return err`
	`44`	`+ }`
	`45`	`+`
`42`	`46`	`return opts.Print(config.Map())`
`43`	`47`	`}`
`44`	`48`
Original file line number	Diff line number	Diff line change
`@@ -47,7 +47,10 @@ func (opts *RenameOpts) Run() error {`
`47`	`47`	`}`
`48`	`48`	`}`
`49`	`49`
`50`		`- config.SetName(opts.oldName)`
	`50`	`+ if err := config.SetName(opts.oldName); err != nil {`
	`51`	`+ return err`
	`52`	`+ }`
	`53`	`+`
`51`	`54`	`if err := config.Rename(opts.newName); err != nil {`
`52`	`55`	`return err`
`53`	`56`	`}`