From e60f6e52e00e322c425352b30558ef6222963756 Mon Sep 17 00:00:00 2001 From: Chris Trombley Date: Wed, 20 Jan 2021 21:43:26 -0800 Subject: [PATCH] chore: incorporate review feedback --- cmd/newrelic/command.go | 10 +++--- cmd/newrelic/command_integration_test.go | 2 +- internal/apiaccess/command.go | 14 +++----- internal/config/command.go | 5 +-- internal/config/config.go | 33 ++++++++----------- ...fig_test.go => config_integration_test.go} | 4 +-- internal/edge/command_trace_observer.go | 2 +- 7 files changed, 27 insertions(+), 43 deletions(-) rename internal/config/{config_test.go => config_integration_test.go} (98%) diff --git a/cmd/newrelic/command.go b/cmd/newrelic/command.go index d08344b9e..bba94f00c 100644 --- 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 index b2c7ac9b5..566e6ee5a 100644 --- 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 index 1b5626f6b..7cae98a7c 100644 --- 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 index 66ec64ebf..ea6bb1a11 100644 --- 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 index 8a59649cd..68724bdd3 100644 --- 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 b/internal/config/config_integration_test.go similarity index 98% rename from internal/config/config_test.go rename to internal/config/config_integration_test.go index 763158c28..30f335e0e 100644 --- a/internal/config/config_test.go +++ b/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 index d70cedbf8..d024703be 100644 --- 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 ", - 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)