CLI Unification: Config (#914)

* unify config command

* unified config file, IsCloud and IsOnPrem

* consolidate ccloud hostnames var

* finish config unification and test

* add golden files

* check for test url second

* add back missing return

* support all cpdev platforms

* trim trailing slash from platform names, and use contains

* revert trim trailing slash

* fix error

internal/cmd/command.go

@@ -92,8 +92,11 @@ func NewConfluentCommand(cliName string, isTest bool, ver *pversion.Version) *co
 	cfg, configLoadingErr := loadConfig(cliName, logger)
 	if cfg != nil {
 		cfg.Logger = logger
+		cfg.IsTest = isTest
 	}
 
+	isCloud := cfg.IsCloud()
+
 	disableUpdateCheck := cfg != nil && (cfg.DisableUpdates || cfg.DisableUpdateCheck)
 	updateClient := update.NewClient(cliName, disableUpdateCheck, logger)
 
@@ -136,7 +139,7 @@ func NewConfluentCommand(cliName string, isTest bool, ver *pversion.Version) *co
 
 	cli.AddCommand(auditlog.New(cliName, prerunner))
 	cli.AddCommand(completion.New(cli, cliName))
-	cli.AddCommand(config.New(cliName, prerunner, analyticsClient))
+	cli.AddCommand(config.New(isCloud, prerunner, analyticsClient))
 	cli.AddCommand(kafka.New(isAPIKeyLogin, cliName, prerunner, logger.Named("kafka"), ver.ClientID, serverCompleter, analyticsClient))
 	cli.AddCommand(login.New(prerunner, logger, ccloudClientFactory, mdsClientManager, analyticsClient, netrcHandler, loginCredentialsManager, authTokenHandler, isTest).Command)
 	cli.AddCommand(logout.New(cliName, prerunner, analyticsClient, netrcHandler).Command)

internal/cmd/config/command.go

@@ -9,28 +9,25 @@ import (
 
 type command struct {
 	*pcmd.CLICommand
-	cliName   string
 	prerunner pcmd.PreRunner
 	analytics analytics.Client
 }
 
 // New returns the Cobra command for `config`.
-func New(cliName string, prerunner pcmd.PreRunner, analytics analytics.Client) *cobra.Command {
+func New(isCloud bool, prerunner pcmd.PreRunner, analytics analytics.Client) *cobra.Command {
 	cliCmd := pcmd.NewAnonymousCLICommand(
 		&cobra.Command{
 			Use:   "config",
 			Short: "Modify the CLI configuration.",
 		}, prerunner)
+
 	cmd := &command{
-		cliName:    cliName,
 		CLICommand: cliCmd,
 		prerunner:  prerunner,
 		analytics:  analytics,
 	}
-	cmd.init()
-	return cmd.Command
-}
 
-func (c *command) init() {
-	c.AddCommand(NewContext(c.cliName, c.prerunner, c.analytics))
+	cmd.AddCommand(NewContext(isCloud, prerunner, analytics))
+
+	return cmd.Command
 }

internal/cmd/config/context.go

@@ -23,21 +23,21 @@ var (
 
 type contextCommand struct {
 	*pcmd.CLICommand
-	cliName   string
+	isCloud   bool
 	prerunner pcmd.PreRunner
 	analytics analytics.Client
 }
 
 // NewContext returns the Cobra contextCommand for `config context`.
-func NewContext(cliName string, prerunner pcmd.PreRunner, analytics analytics.Client) *cobra.Command {
+func NewContext(isCloud bool, prerunner pcmd.PreRunner, analytics analytics.Client) *cobra.Command {
 	cliCmd := pcmd.NewAnonymousCLICommand(
 		&cobra.Command{
 			Use:   "context",
 			Short: "Manage config contexts.",
 		}, prerunner)
 	cmd := &contextCommand{
-		cliName:    cliName,
 		CLICommand: cliCmd,
+		isCloud:    isCloud,
 		prerunner:  prerunner,
 		analytics:  analytics,
 	}
@@ -71,17 +71,11 @@ func (c *contextCommand) init() {
 		Args:  cobra.NoArgs,
 		RunE:  pcmd.NewCLIRunE(c.current),
 	}
-	var usernameFlagUsage string
-	if c.cliName == "ccloud" {
-		usernameFlagUsage = "Returns email if logged in, and returns API key if API key context."
-	} else {
-		usernameFlagUsage = "Returns username."
-	}
-	currentCmd.Flags().Bool("username", false, usernameFlagUsage)
+	currentCmd.Flags().Bool("username", false, "Return username, email, or API key based on context.")
 	currentCmd.Flags().SortFlags = false
 	c.AddCommand(currentCmd)
 
-	if c.cliName == "ccloud" {
+	if c.isCloud {
 		getCmd := &cobra.Command{
 			Use:   "get <id or no argument for current context>",
 			Short: "Get a config context parameter.",

internal/cmd/login/command.go

@@ -11,6 +11,7 @@ import (
 	"github.com/confluentinc/cli/internal/pkg/analytics"
 	pauth "github.com/confluentinc/cli/internal/pkg/auth"
 	pcmd "github.com/confluentinc/cli/internal/pkg/cmd"
+	v3 "github.com/confluentinc/cli/internal/pkg/config/v3"
 	"github.com/confluentinc/cli/internal/pkg/errors"
 	"github.com/confluentinc/cli/internal/pkg/log"
 	"github.com/confluentinc/cli/internal/pkg/netrc"
@@ -325,16 +326,14 @@ func validateURL(url string, isCCloud bool) (string, bool, string) {
 }
 
 func (c *Command) isCCloudURL(url string) bool {
-	if c.isTest {
-		if strings.Contains(url, testserver.TestCloudURL) {
+	for _, hostname := range v3.CCloudHostnames {
+		if strings.Contains(url, hostname) {
 			return true
 		}
 	}
 
-	for _, hostname := range pauth.CCloudHostnames {
-		if strings.Contains(url, hostname) {
-			return true
-		}
+	if c.isTest {
+		return strings.Contains(url, testserver.TestCloudURL.Host)
 	}
 
 	return false

internal/cmd/login/command_test.go

@@ -22,6 +22,9 @@ import (
 	orgv1 "github.com/confluentinc/cc-structs/kafka/org/v1"
 	"github.com/confluentinc/ccloud-sdk-go-v1"
 	sdkMock "github.com/confluentinc/ccloud-sdk-go-v1/mock"
+	mds "github.com/confluentinc/mds-sdk-go/mdsv1"
+	mdsMock "github.com/confluentinc/mds-sdk-go/mdsv1/mock"
+
 	"github.com/confluentinc/cli/internal/cmd/logout"
 	pauth "github.com/confluentinc/cli/internal/pkg/auth"
 	pcmd "github.com/confluentinc/cli/internal/pkg/cmd"
@@ -35,8 +38,6 @@ import (
 	"github.com/confluentinc/cli/internal/pkg/netrc"
 	"github.com/confluentinc/cli/internal/pkg/utils"
 	cliMock "github.com/confluentinc/cli/mock"
-	mds "github.com/confluentinc/mds-sdk-go/mdsv1"
-	mdsMock "github.com/confluentinc/mds-sdk-go/mdsv1/mock"
 )
 
 const (
@@ -887,13 +888,15 @@ func clearCCloudDeprecatedEnvVar(req *require.Assertions) {
 
 func TestIsCCloudURL_True(t *testing.T) {
 	for _, url := range []string{
-		"devel.cpdev.cloud",
-		"stag.cpdev.cloud",
 		"confluent.cloud",
 		"confluent.cloud:8090",
 		"https://confluent.cloud",
+		"https://confluent.cloud/",
+		"devel.cpdev.cloud",
+		"stag.cpdev.cloud",
+		"premium-oryx.gcp.priv.cpdev.cloud",
 	} {
-		c := &Command{isTest: true}
+		c := new(Command)
 		require.True(t, c.isCCloudURL(url), url+" should return true")
 	}
 }
@@ -904,7 +907,7 @@ func TestIsCCloudURL_False(t *testing.T) {
 		"example.com:8090",
 		"https://example.com",
 	} {
-		c := &Command{isTest: true}
+		c := new(Command)
 		require.False(t, c.isCCloudURL(url), url+" should return false")
 	}
 }

internal/pkg/auth/auth.go

@@ -32,10 +32,6 @@ const (
 	ConfluentCACertPathEnvVar = "CONFLUENT_CA_CERT_PATH"
 )
 
-var (
-	CCloudHostnames = []string{"confluent.cloud", "devel.cpdev.cloud", "stag.cpdev.cloud"}
-)
-
 func PersistLogoutToConfig(config *v3.Config) error {
 	ctx := config.Context()
 	if ctx == nil {

internal/pkg/config/v3/config.go

@@ -6,16 +6,18 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
-
-	orgv1 "github.com/confluentinc/cc-structs/kafka/org/v1"
+	"strings"
 
 	"github.com/blang/semver"
 	"github.com/google/uuid"
 
+	orgv1 "github.com/confluentinc/cc-structs/kafka/org/v1"
+
 	"github.com/confluentinc/cli/internal/pkg/config"
 	v1 "github.com/confluentinc/cli/internal/pkg/config/v1"
 	v2 "github.com/confluentinc/cli/internal/pkg/config/v2"
 	"github.com/confluentinc/cli/internal/pkg/errors"
+	testserver "github.com/confluentinc/cli/test/test-server"
 )
 
 const (
@@ -24,7 +26,8 @@ const (
 )
 
 var (
-	Version = semver.MustParse("3.0.0")
+	Version         = semver.MustParse("3.0.0")
+	CCloudHostnames = []string{"confluent.cloud", "cpdev.cloud"}
 )
 
 // Config represents the CLI configuration.
@@ -39,6 +42,7 @@ type Config struct {
 	ContextStates          map[string]*v2.ContextState `json:"context_states,omitempty"`
 	CurrentContext         string                      `json:"current_context"`
 	AnonymousId            string                      `json:"anonymous_id,omitempty"`
+	IsTest                 bool                        `json:"-"`
 	overwrittenAccount     *orgv1.Account
 	overwrittenCurrContext string
 	overwrittenActiveKafka string
@@ -411,7 +415,34 @@ func (c *Config) ResetAnonymousId() error {
 func (c *Config) getFilename() (string, error) {
 	if c.Filename == "" {
 		homedir, _ := os.UserHomeDir()
-		c.Filename = filepath.FromSlash(fmt.Sprintf(defaultConfigFileFmt, homedir, c.CLIName))
+		c.Filename = filepath.FromSlash(fmt.Sprintf(defaultConfigFileFmt, homedir, "confluent"))
 	}
 	return c.Filename, nil
 }
+
+func (c *Config) IsCloud() bool {
+	ctx := c.Context()
+	if ctx == nil {
+		return false
+	}
+
+	if c.IsTest {
+		return ctx.PlatformName == testserver.TestCloudURL.String()
+	}
+
+	for _, hostname := range CCloudHostnames {
+		if strings.Contains(ctx.PlatformName, hostname) {
+			return true
+		}
+	}
+	return false
+}
+
+func (c *Config) IsOnPrem() bool {
+	ctx := c.Context()
+	if ctx == nil {
+		return false
+	}
+
+	return ctx.PlatformName != "" && !c.IsCloud()
+}

internal/pkg/config/v3/config_test.go

@@ -9,8 +9,6 @@ import (
 	"runtime"
 	"testing"
 
-	"github.com/confluentinc/cli/internal/pkg/utils"
-
 	orgv1 "github.com/confluentinc/cc-structs/kafka/org/v1"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -20,6 +18,7 @@ import (
 	v1 "github.com/confluentinc/cli/internal/pkg/config/v1"
 	v2 "github.com/confluentinc/cli/internal/pkg/config/v2"
 	"github.com/confluentinc/cli/internal/pkg/log"
+	"github.com/confluentinc/cli/internal/pkg/utils"
 	"github.com/confluentinc/cli/internal/pkg/version"
 )
 
@@ -593,24 +592,9 @@ func TestConfig_getFilename(t *testing.T) {
 		wantErr bool
 	}{
 		{
-			name: "config file for ccloud binary",
-			fields: fields{
-				CLIName: "ccloud",
-			},
-			want: filepath.FromSlash(os.Getenv("HOME") + "/.ccloud/config.json"),
-		},
-		{
-			name: "config file for confluent binary",
-			fields: fields{
-				CLIName: "confluent",
-			},
+			name: "config filepath is ~/.confluent/config.json",
 			want: filepath.FromSlash(os.Getenv("HOME") + "/.confluent/config.json"),
 		},
-		{
-			name:   "should default to ~/.confluent if CLIName isn't provided",
-			fields: fields{},
-			want:   filepath.FromSlash(os.Getenv("HOME") + "/.confluent/config.json"),
-		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -1077,3 +1061,60 @@ func TestKafkaClusterContext_RemoveKafkaCluster(t *testing.T) {
 		require.Empty(t, kafkaClusterContext.GetActiveKafkaClusterId())
 	}
 }
+
+func TestConfig_IsCloud_True(t *testing.T) {
+	platforms := []string{
+		"devel.cpdev.cloud",
+		"stag.cpdev.cloud",
+		"confluent.cloud",
+		"premium-oryx.gcp.priv.cpdev.cloud",
+	}
+
+	for _, platform := range platforms {
+		config := &Config{
+			Contexts:       map[string]*Context{"context": {PlatformName: platform}},
+			CurrentContext: "context",
+		}
+		require.True(t, config.IsCloud(), platform+" should be true")
+	}
+}
+
+func TestConfig_IsCloud_False(t *testing.T) {
+	configs := []*Config{
+		nil,
+		{
+			Contexts:       map[string]*Context{"context": {PlatformName: "https://example.com"}},
+			CurrentContext: "context",
+		},
+	}
+
+	for _, config := range configs {
+		require.False(t, config.IsCloud())
+	}
+}
+
+func TestConfig_IsOnPrem_True(t *testing.T) {
+	config := &Config{
+		Contexts:       map[string]*Context{"context": {PlatformName: "https://example.com"}},
+		CurrentContext: "context",
+	}
+	require.True(t, config.IsOnPrem())
+}
+
+func TestConfig_IsOnPrem_False(t *testing.T) {
+	configs := []*Config{
+		nil,
+		{
+			Contexts:       map[string]*Context{"context": new(Context)},
+			CurrentContext: "context",
+		},
+		{
+			Contexts:       map[string]*Context{"context": {PlatformName: "confluent.cloud"}},
+			CurrentContext: "context",
+		},
+	}
+
+	for _, config := range configs {
+		require.False(t, config.IsOnPrem())
+	}
+}