Merge pull request #350 from jonseymour/jss-348-apply-env-to-array-el…

…ements apply env to array elements
influxdata · Mar 21, 2016 · e1485a2 · e1485a2
2 parents 541ac20 + 7bae535
commit e1485a2
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 63 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -104,6 +104,7 @@ For example:
 - [#266](https://github.com/influxdata/kapacitor/issues/266): Fixes error log for HipChat that is not an error.
 - [#333](https://github.com/influxdata/kapacitor/issues/333): Fixes hang when replaying with .stats node. Fixes issues with batch and stats.
 - [#340](https://github.com/influxdata/kapacitor/issues/340): BREAKING: Decouples global setting for alert handlers from the state changes only setting.
+- [#348](https://github.com/influxdata/kapacitor/issues/348): config.go: refactor to simplify structure and fix support for array elements
 
 ## v0.10.1 [2016-02-08]
 

diff --git a/cmd/kapacitord/run/config.go b/cmd/kapacitord/run/config.go
@@ -205,21 +205,74 @@ func (c *Config) Validate() error {
 }
 
 func (c *Config) ApplyEnvOverrides() error {
-	return c.applyEnvOverrides("KAPACITOR", reflect.ValueOf(c))
+	return c.applyEnvOverrides("KAPACITOR", "", reflect.ValueOf(c))
 }
 
-func (c *Config) applyEnvOverrides(prefix string, spec reflect.Value) error {
+func (c *Config) applyEnvOverrides(prefix string, fieldDesc string, spec reflect.Value) error {
 	// If we have a pointer, dereference it
 	s := spec
 	if spec.Kind() == reflect.Ptr {
 		s = spec.Elem()
 	}
 
-	// Make sure we have struct
+	var value string
+
 	if s.Kind() != reflect.Struct {
-		return nil
+		value = os.Getenv(prefix)
+		// Skip any fields we don't have a value to set
+		if value == "" {
+			return nil
+		}
+
+		if fieldDesc != "" {
+			fieldDesc = " to " + fieldDesc
+		}
 	}
 
+	switch s.Kind() {
+	case reflect.String:
+		s.SetString(value)
+	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
+
+		var intValue int64
+
+		// Handle toml.Duration
+		if s.Type().Name() == "Duration" {
+			dur, err := time.ParseDuration(value)
+			if err != nil {
+				return fmt.Errorf("failed to apply %v%v using type %v and value '%v'", prefix, fieldDesc, s.Type().String(), value)
+			}
+			intValue = dur.Nanoseconds()
+		} else {
+			var err error
+			intValue, err = strconv.ParseInt(value, 0, s.Type().Bits())
+			if err != nil {
+				return fmt.Errorf("failed to apply %v%v using type %v and value '%v'", prefix, fieldDesc, s.Type().String(), value)
+			}
+		}
+
+		s.SetInt(intValue)
+	case reflect.Bool:
+		boolValue, err := strconv.ParseBool(value)
+		if err != nil {
+			return fmt.Errorf("failed to apply %v%v using type %v and value '%v'", prefix, fieldDesc, s.Type().String(), value)
+
+		}
+		s.SetBool(boolValue)
+	case reflect.Float32, reflect.Float64:
+		floatValue, err := strconv.ParseFloat(value, s.Type().Bits())
+		if err != nil {
+			return fmt.Errorf("failed to apply %v%v using type %v and value '%v'", prefix, fieldDesc, s.Type().String(), value)
+
+		}
+		s.SetFloat(floatValue)
+	case reflect.Struct:
+		c.applyEnvOverridesToStruct(prefix, s)
+	}
+	return nil
+}
+
+func (c *Config) applyEnvOverridesToStruct(prefix string, s reflect.Value) error {
 	typeOfSpec := s.Type()
 	for i := 0; i < s.NumField(); i++ {
 		f := s.Field(i)
@@ -237,73 +290,17 @@ func (c *Config) applyEnvOverrides(prefix string, spec reflect.Value) error {
 			if prefix != "" {
 				key = strings.ToUpper(fmt.Sprintf("%s_%s", prefix, configName))
 			}
-			value := os.Getenv(key)
 
 			// If the type is s slice, apply to each using the index as a suffix
 			// e.g. GRAPHITE_0
 			if f.Kind() == reflect.Slice || f.Kind() == reflect.Array {
 				for i := 0; i < f.Len(); i++ {
-					if err := c.applyEnvOverrides(fmt.Sprintf("%s_%d", key, i), f.Index(i)); err != nil {
+					if err := c.applyEnvOverrides(fmt.Sprintf("%s_%d", key, i), fieldName, f.Index(i)); err != nil {
 						return err
 					}
 				}
-				continue
-			}
-
-			// If it's a sub-config, recursively apply
-			if f.Kind() == reflect.Struct || f.Kind() == reflect.Ptr {
-				if err := c.applyEnvOverrides(key, f); err != nil {
-					return err
-				}
-				continue
-			}
-
-			// Skip any fields we don't have a value to set
-			if value == "" {
-				continue
-			}
-
-			switch f.Kind() {
-			case reflect.String:
-				f.SetString(value)
-			case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
-
-				var intValue int64
-
-				// Handle toml.Duration
-				if f.Type().Name() == "Duration" {
-					dur, err := time.ParseDuration(value)
-					if err != nil {
-						return fmt.Errorf("failed to apply %v to %v using type %v and value '%v'", key, fieldName, f.Type().String(), value)
-					}
-					intValue = dur.Nanoseconds()
-				} else {
-					var err error
-					intValue, err = strconv.ParseInt(value, 0, f.Type().Bits())
-					if err != nil {
-						return fmt.Errorf("failed to apply %v to %v using type %v and value '%v'", key, fieldName, f.Type().String(), value)
-					}
-				}
-
-				f.SetInt(intValue)
-			case reflect.Bool:
-				boolValue, err := strconv.ParseBool(value)
-				if err != nil {
-					return fmt.Errorf("failed to apply %v to %v using type %v and value '%v'", key, fieldName, f.Type().String(), value)
-
-				}
-				f.SetBool(boolValue)
-			case reflect.Float32, reflect.Float64:
-				floatValue, err := strconv.ParseFloat(value, f.Type().Bits())
-				if err != nil {
-					return fmt.Errorf("failed to apply %v to %v using type %v and value '%v'", key, fieldName, f.Type().String(), value)
-
-				}
-				f.SetFloat(floatValue)
-			default:
-				if err := c.applyEnvOverrides(key, f); err != nil {
-					return err
-				}
+			} else if err := c.applyEnvOverrides(key, fieldName, f); err != nil {
+				return err
 			}
 		}
 	}

diff --git a/cmd/kapacitord/run/config_test.go b/cmd/kapacitord/run/config_test.go
@@ -35,6 +35,9 @@ func TestConfig_Parse_EnvOverride(t *testing.T) {
 	// Parse configuration.
 	var c run.Config
 	if _, err := toml.Decode(`
+[[influxdb]]
+urls=["http://localhost:8086"]
+
 [replay]
 dir = "/tmp/replay"
 
@@ -52,6 +55,10 @@ dir = "/tmp/task"
 		t.Fatalf("failed to set env var: %v", err)
 	}
 
+	if err := os.Setenv("KAPACITOR_INFLUXDB_0_URLS_0", "http://localhost:18086"); err != nil {
+		t.Fatalf("failed to set env var: %v", err)
+	}
+
 	if err := c.ApplyEnvOverrides(); err != nil {
 		t.Fatalf("failed to apply env overrides: %v", err)
 	}
@@ -61,5 +68,7 @@ dir = "/tmp/task"
 		t.Fatalf("unexpected replay dir: %s", c.Replay.Dir)
 	} else if c.Task.Dir != "/var/lib/kapacitor/task" {
 		t.Fatalf("unexpected task dir: %s", c.Task.Dir)
+	} else if c.InfluxDB[0].URLs[0] != "http://localhost:18086" {
+		t.Fatalf("unexpected url 0: %s", c.InfluxDB[0].URLs[0])
 	}
 }