chore: incorporate review feedback

newrelic · Feb 2, 2021 · e60f6e5 · e60f6e5
1 parent 3071d45
commit e60f6e5
Show file tree

Hide file tree

Showing 7 changed files with 27 additions and 43 deletions.
diff --git a/cmd/newrelic/command.go b/cmd/newrelic/command.go
@@ -24,8 +24,6 @@ var (
 	trace        bool
 )
 
-const defaultProfileName string = "default"
-
 // Command represents the base command when called without any subcommands
 var Command = &cobra.Command{
 	PersistentPreRun:  initializeCLI,
@@ -101,13 +99,13 @@ func initializeDefaultProfile() {
 
 	log.Infof("default profile does not exist and API key detected. attempting to initialize")
 
-	if config.ProfileExists(defaultProfileName) {
-		log.Warnf("a profile named %s already exists, cannot initialize default profile", defaultProfileName)
+	if config.ProfileExists(config.DefaultDefaultProfileName) {
+		log.Warnf("a profile named %s already exists, cannot initialize default profile", config.DefaultDefaultProfileName)
 		return
 	}
 
 	// Saving an initial value will also set the default profile.
-	if err = config.SaveValueToProfile(defaultProfileName, config.UserKey, userKey); err != nil {
+	if err = config.SaveValueToProfile(config.DefaultDefaultProfileName, config.UserKey, userKey); err != nil {
 		log.Warnf("error saving API key to profile, cannot initialize default profile: %s", err)
 		return
 	}
@@ -161,7 +159,7 @@ func initializeDefaultProfile() {
 		}
 	}
 
-	log.Infof("profile %s added", text.FgCyan.Sprint(defaultProfileName))
+	log.Infof("profile %s added", text.FgCyan.Sprint(config.DefaultDefaultProfileName))
 }
 
 func fetchLicenseKey(accountID int) (string, error) {

diff --git a/cmd/newrelic/command_integration_test.go b/cmd/newrelic/command_integration_test.go
@@ -55,7 +55,7 @@ func TestInitializeProfile(t *testing.T) {
 	require.NoError(t, err)
 
 	require.Equal(t, 1, len(config.GetProfileNames()))
-	require.Equal(t, defaultProfileName, config.GetDefaultProfileName())
+	require.Equal(t, config.DefaultDefaultProfileName, config.GetDefaultProfileName())
 	require.Equal(t, envUserKey, actualUserKey)
 	require.NotEmpty(t, actualRegion)
 	require.NotEmpty(t, actualLicenseKey)

diff --git a/internal/apiaccess/command.go b/internal/apiaccess/command.go
@@ -18,6 +18,11 @@ var Command = &cobra.Command{
 	Long:    "",
 	Example: "newrelic apiaccess apiAccess --help",
 	Hidden:  true, // Mark as pre-release
+	PersistentPreRun: func(cmd *cobra.Command, args []string) {
+		if _, err := config.RequireUserKey(); err != nil {
+			log.Fatal(err)
+		}
+	},
 }
 
 var apiAccessGetKeyid string
@@ -28,11 +33,6 @@ var cmdKey = &cobra.Command{
 	Short:   "Fetch a single key by ID and type.\n\n---\n**NR Internal** | [#help-unified-api](https://newrelic.slack.com/archives/CBHJRSPSA) | visibility(customer)\n\n",
 	Long:    "",
 	Example: "newrelic apiAccess apiAccessGetKey --id --keyType",
-	PreRun: func(cmd *cobra.Command, args []string) {
-		if _, err := config.RequireUserKey(); err != nil {
-			log.Fatal(err)
-		}
-	},
 	Run: func(cmd *cobra.Command, args []string) {
 		resp, err := client.Client.APIAccess.GetAPIAccessKey(apiAccessGetKeyid, apiaccess.APIAccessKeyType(apiAccessGetKeykeyType))
 		if err != nil {
@@ -129,15 +129,11 @@ func init() {
 	}
 
 	Command.AddCommand(cmdAPIAccessCreateKeys)
-
 	cmdAPIAccessCreateKeys.Flags().StringVar(&apiAccessCreateKeysInput, "keys", "", "A list of the configurations for each key you want to create.")
 
 	Command.AddCommand(cmdAPIAccessUpdateKeys)
-
 	cmdAPIAccessUpdateKeys.Flags().StringVar(&apiAccessUpdateKeysInput, "keys", "", "The configurations of each key you want to update.")
 
 	Command.AddCommand(cmdAPIAccessDeleteKeys)
-
 	cmdAPIAccessDeleteKeys.Flags().StringVar(&apiAccessDeleteKeysInput, "keys", "", "A list of each key `id` that you want to delete. You can read more about managing keys on [this documentation page](https://docs.newrelic.com/docs/apis/nerdgraph/examples/use-nerdgraph-manage-license-keys-personal-api-keys).")
-
 }
diff --git a/internal/config/command.go b/internal/config/command.go
@@ -1,8 +1,6 @@
 package config
 
 import (
-	"os"
-
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 
@@ -89,8 +87,7 @@ This will have the effect of resetting the value to its default.
 	Run: func(cmd *cobra.Command, args []string) {
 		err := SaveConfigValue(CfgFieldKey(key), "")
 		if err != nil {
-			log.Error(err)
-			os.Exit(1)
+			log.Fatal(err)
 		}
 	},
 	Aliases: []string{

diff --git a/internal/config/config.go b/internal/config/config.go
@@ -19,7 +19,7 @@ import (
 const (
 	configType                = "json"
 	globalScopeIdentifier     = "*"
-	defaultDefaultProfileName = "default"
+	DefaultDefaultProfileName = "default"
 )
 
 type CfgFieldKey string
@@ -124,15 +124,6 @@ type CfgValue struct {
 	Default interface{}
 }
 
-// IsDefault returns true if the field's value is the default value.
-func (c *CfgValue) IsDefault() bool {
-	if v, ok := c.Value.(string); ok {
-		return strings.EqualFold(v, c.Default.(string))
-	}
-
-	return c.Value == c.Default
-}
-
 func init() {
 	var err error
 	ConfigDir, err = getDefaultConfigDirectory()
@@ -268,17 +259,17 @@ func GetProfileValueString(profileName string, key ProfileFieldKey) string {
 
 func GetActiveProfileName() string {
 	defaultProfile := defaultProfileName()
-	if ProfileOverride != "" {
-		if !ProfileExists(ProfileOverride) {
-			log.Warnf("profile %s requested but not found.  using default profile: %s", ProfileOverride, defaultProfile)
-			return defaultProfile
-		}
+	if ProfileOverride == "" {
+		return defaultProfile
+	}
 
-		log.Infof("using requested profile %s", ProfileOverride)
-		return ProfileOverride
+	if !ProfileExists(ProfileOverride) {
+		log.Warnf("profile %s requested but not found.  using default profile: %s", ProfileOverride, defaultProfile)
+		return defaultProfile
 	}
 
-	return defaultProfile
+	log.Infof("using requested profile %s", ProfileOverride)
+	return ProfileOverride
 }
 
 func GetDefaultProfileName() string {
@@ -367,7 +358,9 @@ func RemoveProfile(profileName string) error {
 
 	if defaultProfileName() == profileName {
 		log.Infof("unsetting %s as default profile.", profileName)
-		if err := SaveDefaultProfileName(""); err != nil {
+
+		defaultProfileFilePath := filepath.Join(ConfigDir, defaultProfileFilename)
+		if err := os.Remove(defaultProfileFilePath); err != nil {
 			log.Warnf("could not unset default profile %s", profileName)
 		}
 	}
@@ -512,7 +505,7 @@ func loadFile(v *viper.Viper) error {
 func saveDefaultProfileName(profileName string) error {
 	defaultProfileFilePath := filepath.Join(ConfigDir, defaultProfileFilename)
 
-	if err := ioutil.WriteFile(defaultProfileFilePath, []byte("\""+profileName+"\""), 0644); err != nil {
+	if err := ioutil.WriteFile(defaultProfileFilePath, []byte("\""+profileName+"\""), 0640); err != nil {
 		return err
 	}
 

diff --git a/internal/config/config_test.go → internal/config/config_integration_test.go b/internal/config/config_test.go → internal/config/config_integration_test.go
@@ -171,10 +171,10 @@ func TestSetProfileValue_Basic(t *testing.T) {
 	mockConfigFiles := createMockConfigFiles(t)
 	defer mockConfigFiles.teardown()
 
-	err := SaveValueToProfile(defaultDefaultProfileName, UserKey, "NRAK-abc123")
+	err := SaveValueToProfile(DefaultDefaultProfileName, UserKey, "NRAK-abc123")
 	require.NoError(t, err)
 
-	credsValue, err := GetProfileValue(defaultDefaultProfileName, UserKey)
+	credsValue, err := GetProfileValue(DefaultDefaultProfileName, UserKey)
 	require.NoError(t, err)
 	require.Equal(t, "NRAK-abc123", credsValue)
 }

diff --git a/internal/edge/command_trace_observer.go b/internal/edge/command_trace_observer.go
@@ -31,7 +31,7 @@ var cmdTraceObserver = &cobra.Command{
 	provides visualization for the most actionable data so you can investigate and
 	solve issues faster.`,
 	Example: "newrelic edge trace-observer list --accountId <accountID>",
-	PreRun: func(cmd *cobra.Command, args []string) {
+	PersistentPreRun: func(cmd *cobra.Command, args []string) {
 		var err error
 		if accountID, err = config.RequireAccountID(); err != nil {
 			log.Fatal(err)