chore(config): add validation for ConfigField values

newrelic · Jan 14, 2021 · a20ab91 · a20ab91
1 parent cd5a19f
commit a20ab91
Show file tree

Hide file tree

Showing 3 changed files with 114 additions and 9 deletions.
diff --git a/internal/configuration/config.go b/internal/configuration/config.go
@@ -39,24 +39,36 @@ const (
 var (
 	ConfigFields = []ConfigField{
 		{
-			Name:    "LogLevel",
-			Key:     LogLevel,
-			Default: "info",
+			Name:        "LogLevel",
+			Key:         LogLevel,
+			Default:     "info",
+			ValidValues: LogLevels(),
 		},
 		{
 			Name:    "SendUsageData",
 			Key:     SendUsageData,
 			Default: string(TernaryValues.Unknown),
+			ValidValues: []string{
+				TernaryValues.Unknown.String(),
+				TernaryValues.Allow.String(),
+				TernaryValues.Disallow.String(),
+			},
 		},
 		{
-			Name:    "PluginDir",
-			Key:     PluginDir,
-			Default: "",
+			Name:        "PluginDir",
+			Key:         PluginDir,
+			Default:     "",
+			ValidValues: []string{},
 		},
 		{
 			Name:    "PrereleaseFeatures",
 			Key:     PrereleaseFeatures,
 			Default: string(TernaryValues.Unknown),
+			ValidValues: []string{
+				TernaryValues.Unknown.String(),
+				TernaryValues.Allow.String(),
+				TernaryValues.Disallow.String(),
+			},
 		},
 	}
 	ProfileFields = []ProfileField{
@@ -95,9 +107,10 @@ var (
 )
 
 type ConfigField struct {
-	Name    string
-	Key     ConfigFieldKey
-	Default string
+	Name        string
+	Key         ConfigFieldKey
+	Default     string
+	ValidValues []string
 }
 
 type ProfileField struct {
@@ -166,6 +179,14 @@ func SetDefaultProfileName(profileName string) error {
 }
 
 func SetConfigValue(key ConfigFieldKey, value string) error {
+	if ok := isValidConfigKey(key); !ok {
+		return fmt.Errorf("config key %s is not valid.  valid keys are %s", key, validConfigFieldKeys())
+	}
+
+	if ok := isValidConfigValue(key, value); !ok {
+		return fmt.Errorf("config value '%s' is invalid. Valid values for key %s are [%s]", value, key, strings.Join([]string{"todo", "implement", "this"}, " "))
+	}
+
 	c := config()
 	c.Set(keyGlobalScope(string(key)), value)
 
@@ -388,6 +409,32 @@ func isValidConfigKey(key ConfigFieldKey) bool {
 	return false
 }
 
+func isValidConfigValue(key ConfigFieldKey, value string) bool {
+	configKey := string(key)
+
+	for _, c := range ConfigFields {
+		if !strings.EqualFold(configKey, string(c.Key)) {
+			continue
+		}
+
+		// If the ConfigField.ValidValues slice is empty,
+		// any value can be considered valid.
+		if len(c.ValidValues) == 0 {
+			return true
+		}
+
+		for _, validValue := range c.ValidValues {
+			if strings.EqualFold(value, validValue) {
+				return true
+			}
+		}
+	}
+
+	// Should we return true here? Consider the case
+	// of the PluginsDir field.
+	return false
+}
+
 func isValidCredentialKey(key ProfileFieldKey) bool {
 	for _, v := range ProfileFields {
 		if strings.EqualFold(string(v.Key), string(key)) {

diff --git a/internal/configuration/config_test.go b/internal/configuration/config_test.go
@@ -106,6 +106,25 @@ func TestSetConfigValue_FileNotExists(t *testing.T) {
 	os.Remove(configFilePath)
 }
 
+func TestSetConfigValue_InvalidValue(t *testing.T) {
+	mockConfigFiles := createMockConfigFiles(t)
+	defer mockConfigFiles.teardown()
+
+	var err error
+
+	err = SetConfigValue(LogLevel, "invalid")
+	require.Error(t, err)
+
+	err = SetConfigValue(PrereleaseFeatures, "invalid")
+	require.Error(t, err)
+
+	err = SetConfigValue(SendUsageData, "invalid")
+	require.Error(t, err)
+
+	err = SetConfigValue(PluginDir, "/any/path/is/valid")
+	require.NoError(t, err)
+}
+
 func TestGetActiveProfileValue_Basic(t *testing.T) {
 	EnvVarResolver = &MockEnvResolver{}
 	mockConfigFiles := createMockConfigFiles(t)

diff --git a/internal/configuration/logging.go b/internal/configuration/logging.go
@@ -16,6 +16,45 @@ const (
 	DefaultLogFile = "newrelic-cli.log"
 )
 
+// LogLevelValue represents a single log level value
+type LogLevelValue string
+
+func (l LogLevelValue) String() string {
+	return string(l)
+}
+
+// LogLevelValues provides the set of log level values
+var LogLevelValues = struct {
+	Panic LogLevelValue
+	Fatal LogLevelValue
+	Error LogLevelValue
+	Warn  LogLevelValue
+	Info  LogLevelValue
+	Debug LogLevelValue
+	Trace LogLevelValue
+}{
+	Panic: "panic",
+	Fatal: "fatal",
+	Error: "error",
+	Warn:  "warn",
+	Info:  "info",
+	Debug: "debug",
+	Trace: "trace",
+}
+
+// LogLevels returns a slice of valid log values
+func LogLevels() []string {
+	return []string{
+		LogLevelValues.Panic.String(),
+		LogLevelValues.Fatal.String(),
+		LogLevelValues.Error.String(),
+		LogLevelValues.Warn.String(),
+		LogLevelValues.Info.String(),
+		LogLevelValues.Debug.String(),
+		LogLevelValues.Trace.String(),
+	}
+}
+
 var (
 	fileHookConfigured = false
 )