Build the env var layer for the nodetreemodel config (#30835)

pkg/config/nodetreemodel/config.go

@@ -54,13 +54,16 @@ type ntmConfig struct {
 	ready *atomic.Bool
 	// defaults contains the settings with a default value
 	defaults InnerNode
+	// envs contains config settings created by environment variables
+	envs InnerNode
 	// file contains the settings pulled from YAML files
 	file InnerNode
 	// root contains the final configuration, it's the result of merging all other tree by ordre of priority
 	root InnerNode
 
 	envPrefix      string
 	envKeyReplacer *strings.Replacer
+	envTransform   map[string]func(string) interface{}
 
 	notificationReceivers []model.NotificationReceiver
 
@@ -72,8 +75,8 @@ type ntmConfig struct {
 	configType string
 
 	// configEnvVars is the set of env vars that are consulted for
-	// configuration values.
-	configEnvVars map[string]struct{}
+	// any given configuration key. Multiple env vars can be associated with one key
+	configEnvVars map[string]string
 
 	// known keys are all the keys that meet at least one of these criteria:
 	// 1) have a default, 2) have an environment variable binded, 3) are an alias or 4) have been SetKnown()
@@ -118,12 +121,15 @@ func (c *ntmConfig) setValueSource(key string, newValue interface{}, source mode
 }
 
 func (c *ntmConfig) set(key string, value interface{}, tree InnerNode, source model.Source) (bool, error) {
-	parts := strings.Split(strings.ToLower(key), ",")
+	if tree == nil {
+		return false, fmt.Errorf("cannot assign to nil Node")
+	}
+	parts := splitKey(key)
 	return tree.SetAt(parts, value, source)
 }
 
 func (c *ntmConfig) setDefault(key string, value interface{}) {
-	parts := strings.Split(strings.ToLower(key), ",")
+	parts := splitKey(key)
 	// TODO: Ensure that for default tree, setting nil to a node will not override
 	// an existing value
 	_, _ = c.defaults.SetAt(parts, value, model.SourceDefault)
@@ -141,22 +147,22 @@ func (c *ntmConfig) Set(key string, newValue interface{}, source model.Source) {
 		tree = c.defaults
 	case model.SourceFile:
 		tree = c.file
+	default:
+		log.Errorf("unknown source tree: %s\n", source)
 	}
 
 	c.Lock()
-
 	previousValue, _ := c.getValue(key)
 	_, _ = c.set(key, newValue, tree, source)
 	updated, _ := c.set(key, newValue, c.root, source)
+	receivers := slices.Clone(c.notificationReceivers)
+	c.Unlock()
 
 	// if no value has changed we don't notify
 	if !updated || reflect.DeepEqual(previousValue, newValue) {
 		return
 	}
 
-	receivers := slices.Clone(c.notificationReceivers)
-	c.Unlock()
-
 	// notifying all receiver about the updated setting
 	for _, receiver := range receivers {
 		receiver(key, previousValue, newValue)
@@ -247,42 +253,76 @@ func (c *ntmConfig) GetKnownKeysLowercased() map[string]interface{} {
 func (c *ntmConfig) BuildSchema() {
 	c.Lock()
 	defer c.Unlock()
+	c.buildEnvVars()
 	c.ready.Store(true)
-	// TODO: Build the environment variable tree
-	// TODO: Instead of assigning defaultSource to root, merge the trees
-	c.root = c.defaults
+	if err := c.mergeAllLayers(); err != nil {
+		c.warnings = append(c.warnings, err.Error())
+	}
 }
 
 func (c *ntmConfig) isReady() bool {
 	return c.ready.Load()
 }
 
+func (c *ntmConfig) buildEnvVars() {
+	root := newInnerNode(nil)
+	envWarnings := []string{}
+	for _, e := range os.Environ() {
+		pair := strings.SplitN(e, "=", 2)
+		if len(pair) != 2 {
+			continue
+		}
+		envkey := pair[0]
+		envval := pair[1]
+
+		if configKey, found := c.configEnvVars[envkey]; found {
+			if err := c.insertNodeFromString(root, configKey, envval); err != nil {
+				envWarnings = append(envWarnings, fmt.Sprintf("inserting env var: %s", err))
+			}
+		}
+	}
+	c.envs = root
+	c.warnings = append(c.warnings, envWarnings...)
+}
+
+func (c *ntmConfig) insertNodeFromString(curr InnerNode, key string, envval string) error {
+	var actualValue interface{} = envval
+	// TODO: When the nodetreemodel config is further along, we should get the default[key] node
+	// and use its type to convert the envval into something appropriate.
+	if transformer, found := c.envTransform[key]; found {
+		actualValue = transformer(envval)
+	}
+	parts := splitKey(key)
+	_, err := curr.SetAt(parts, actualValue, model.SourceEnvVar)
+	return err
+}
+
 // ParseEnvAsStringSlice registers a transform function to parse an environment variable as a []string.
 func (c *ntmConfig) ParseEnvAsStringSlice(key string, fn func(string) []string) {
 	c.Lock()
 	defer c.Unlock()
-	c.noimpl.SetEnvKeyTransformer(key, func(data string) interface{} { return fn(data) })
+	c.envTransform[strings.ToLower(key)] = func(k string) interface{} { return fn(k) }
 }
 
 // ParseEnvAsMapStringInterface registers a transform function to parse an environment variable as a map[string]interface{}
 func (c *ntmConfig) ParseEnvAsMapStringInterface(key string, fn func(string) map[string]interface{}) {
 	c.Lock()
 	defer c.Unlock()
-	c.noimpl.SetEnvKeyTransformer(key, func(data string) interface{} { return fn(data) })
+	c.envTransform[strings.ToLower(key)] = func(k string) interface{} { return fn(k) }
 }
 
 // ParseEnvAsSliceMapString registers a transform function to parse an environment variable as a []map[string]string
 func (c *ntmConfig) ParseEnvAsSliceMapString(key string, fn func(string) []map[string]string) {
 	c.Lock()
 	defer c.Unlock()
-	c.noimpl.SetEnvKeyTransformer(key, func(data string) interface{} { return fn(data) })
+	c.envTransform[strings.ToLower(key)] = func(k string) interface{} { return fn(k) }
 }
 
 // ParseEnvAsSlice registers a transform function to parse an environment variable as a []interface
 func (c *ntmConfig) ParseEnvAsSlice(key string, fn func(string) []interface{}) {
 	c.Lock()
 	defer c.Unlock()
-	c.noimpl.SetEnvKeyTransformer(key, func(data string) interface{} { return fn(data) })
+	c.envTransform[strings.ToLower(key)] = func(k string) interface{} { return fn(k) }
 }
 
 // SetFs assigns a filesystem to the config
@@ -312,7 +352,7 @@ func (c *ntmConfig) leafAtPath(key string) LeafNode {
 		return missingLeaf
 	}
 
-	pathParts := strings.Split(strings.ToLower(key), ".")
+	pathParts := splitKey(key)
 	var curr Node = c.root
 	for _, part := range pathParts {
 		next, err := curr.GetChild(part)
@@ -332,7 +372,7 @@ func (c *ntmConfig) GetNode(key string) (Node, error) {
 	if !c.isReady() {
 		return nil, log.Errorf("attempt to read key before config is constructed: %s", key)
 	}
-	pathParts := strings.Split(key, ".")
+	pathParts := splitKey(key)
 	var curr Node = c.root
 	for _, part := range pathParts {
 		next, err := curr.GetChild(part)
@@ -559,8 +599,7 @@ func (c *ntmConfig) GetSource(key string) model.Source {
 	c.RLock()
 	defer c.RUnlock()
 	c.checkKnownKey(key)
-	c.logErrorNotImplemented("GetSource")
-	return model.SourceUnknown
+	return c.leafAtPath(key).Source()
 }
 
 // SetEnvPrefix sets the environment variable prefix to use
@@ -592,23 +631,24 @@ func (c *ntmConfig) BindEnv(key string, envvars ...string) {
 	}
 
 	for _, envvar := range envvars {
-		// apply EnvKeyReplacer to each key
 		if c.envKeyReplacer != nil {
 			envvar = c.envKeyReplacer.Replace(envvar)
 		}
-		// TODO: Use envvar to build the envvar source tree
-		c.configEnvVars[envvar] = struct{}{}
+		c.configEnvVars[envvar] = key
 	}
 
 	c.knownKeys[key] = struct{}{}
 	c.setDefault(key, nil)
 }
 
 // SetEnvKeyReplacer binds a replacer function for keys
-func (c *ntmConfig) SetEnvKeyReplacer(_r *strings.Replacer) {
+func (c *ntmConfig) SetEnvKeyReplacer(r *strings.Replacer) {
 	c.Lock()
 	defer c.Unlock()
-	c.logErrorNotImplemented("SetEnvKeyReplacer")
+	if c.isReady() {
+		panic("cannot SetEnvKeyReplacer() once the config has been marked as ready for use")
+	}
+	c.envKeyReplacer = r
 }
 
 // UnmarshalKey unmarshals the data for the given key
@@ -773,8 +813,8 @@ func (c *ntmConfig) GetEnvVars() []string {
 
 // BindEnvAndSetDefault binds an environment variable and sets a default for the given key
 func (c *ntmConfig) BindEnvAndSetDefault(key string, val interface{}, envvars ...string) {
-	c.SetDefault(key, val)
 	c.BindEnv(key, envvars...) //nolint:errcheck
+	c.SetDefault(key, val)
 }
 
 // Warnings just returns nil
@@ -792,11 +832,12 @@ func NewConfig(name string, envPrefix string, envKeyReplacer *strings.Replacer)
 	config := ntmConfig{
 		ready:         atomic.NewBool(false),
 		noimpl:        &notImplMethodsImpl{},
-		configEnvVars: map[string]struct{}{},
+		configEnvVars: map[string]string{},
 		knownKeys:     map[string]struct{}{},
 		unknownKeys:   map[string]struct{}{},
 		defaults:      newInnerNode(nil),
 		file:          newInnerNode(nil),
+		envTransform:  make(map[string]func(string) interface{}),
 	}
 
 	config.SetTypeByDefaultValue(true)

pkg/config/nodetreemodel/config_test.go

@@ -6,8 +6,12 @@
 package nodetreemodel
 
 import (
+	"fmt"
+	"os"
+	"strings"
 	"testing"
 
+	"github.com/DataDog/datadog-agent/pkg/config/model"
 	"github.com/stretchr/testify/require"
 )
 
@@ -25,3 +29,80 @@ func TestBuildDefaultMakesTooManyNodes(t *testing.T) {
 	_, ok = n.(LeafNode)
 	require.Equal(t, ok, true)
 }
+
+// Test that default, file, and env layers can build, get merged, and retrieve settings
+func TestBuildDefaultFileAndEnv(t *testing.T) {
+	configData := `network_path:
+  collector:
+    workers: 6
+secret_backend_command: ./my_secret_fetcher.sh
+`
+	os.Setenv("TEST_SECRET_BACKEND_TIMEOUT", "60")
+	os.Setenv("TEST_NETWORK_PATH_COLLECTOR_INPUT_CHAN_SIZE", "23456")
+
+	cfg := NewConfig("test", "TEST", strings.NewReplacer(".", "_"))
+	cfg.BindEnvAndSetDefault("network_path.collector.input_chan_size", 100000)
+	cfg.BindEnvAndSetDefault("network_path.collector.processing_chan_size", 100000)
+	cfg.BindEnvAndSetDefault("network_path.collector.workers", 4)
+	cfg.BindEnvAndSetDefault("secret_backend_command", "")
+	cfg.BindEnvAndSetDefault("secret_backend_timeout", 0)
+	cfg.BindEnvAndSetDefault("server_timeout", 30)
+
+	cfg.BuildSchema()
+	err := cfg.ReadConfig(strings.NewReader(configData))
+	require.NoError(t, err)
+
+	testCases := []struct {
+		description  string
+		setting      string
+		expectValue  interface{}
+		expectIntVal int
+		expectSource model.Source
+	}{
+		{
+			description:  "nested setting from env var works",
+			setting:      "network_path.collector.input_chan_size",
+			expectValue:  "23456",
+			expectSource: model.SourceEnvVar,
+		},
+		{
+			description:  "top-level setting from env var works",
+			setting:      "secret_backend_timeout",
+			expectValue:  "60", // TODO: cfg.Get returns string because this is an env var
+			expectSource: model.SourceEnvVar,
+		},
+		{
+			description:  "nested setting from config file works",
+			setting:      "network_path.collector.workers",
+			expectValue:  6,
+			expectSource: model.SourceFile,
+		},
+		{
+			description:  "top-level setting from config file works",
+			setting:      "secret_backend_command",
+			expectValue:  "./my_secret_fetcher.sh",
+			expectSource: model.SourceFile,
+		},
+		{
+			description:  "nested setting from default works",
+			setting:      "network_path.collector.processing_chan_size",
+			expectValue:  100000,
+			expectSource: model.SourceDefault,
+		},
+		{
+			description:  "top-level setting from default works",
+			setting:      "server_timeout",
+			expectValue:  30,
+			expectSource: model.SourceDefault,
+		},
+	}
+
+	for i, tc := range testCases {
+		t.Run(fmt.Sprintf("case %d: setting %s", i, tc.setting), func(t *testing.T) {
+			val := cfg.Get(tc.setting)
+			require.Equal(t, tc.expectValue, val)
+			src := cfg.GetSource(tc.setting)
+			require.Equal(t, tc.expectSource, src)
+		})
+	}
+}

pkg/config/nodetreemodel/helpers.go

@@ -0,0 +1,14 @@
+// Unless explicitly stated otherwise all files in this repository are licensed
+// under the Apache License Version 2.0.
+// This product includes software developed at Datadog (https://www.datadoghq.com/).
+// Copyright 2016-present Datadog, Inc.
+
+package nodetreemodel
+
+import (
+	"strings"
+)
+
+func splitKey(key string) []string {
+	return strings.Split(strings.ToLower(key), ".")
+}

pkg/config/nodetreemodel/noimpl_methods.go

@@ -16,7 +16,6 @@ type notImplementedMethods interface {
 	SetFs(afero.Fs)
 	IsSet(string) bool
 	AllKeys() []string
-	SetEnvKeyTransformer(string, func(data string) interface{})
 	GetStringSliceE(string) ([]string, error)
 	GetStringMapE(string) (map[string]interface{}, error)
 	GetStringMapStringE(string) (map[string]string, error)
@@ -40,10 +39,6 @@ func (n *notImplMethodsImpl) AllKeys() []string {
 	return nil
 }
 
-func (n *notImplMethodsImpl) SetEnvKeyTransformer(string, func(data string) interface{}) {
-	n.logErrorNotImplemented("SetEnvKeyTransformer")
-}
-
 func (n *notImplMethodsImpl) GetStringSliceE(string) ([]string, error) {
 	return nil, n.logErrorNotImplemented("GetStringSliceE")
 }

pkg/config/nodetreemodel/read_config_file.go

@@ -22,10 +22,14 @@ func (c *ntmConfig) mergeAllLayers() error {
 	treeList := []InnerNode{
 		c.defaults,
 		c.file,
+		c.envs,
 	}
 
 	// TODO: handle all configuration sources
 	for _, tree := range treeList {
+		if tree == nil {
+			continue
+		}
 		err := root.Merge(tree)
 		if err != nil {
 			return err