Implement missing config methods for nodetreemodel (#31548)

pkg/config/model/types.go

@@ -82,14 +82,16 @@ type Reader interface {
 	// OnUpdate adds a callback to the list receivers to be called each time a value is change in the configuration
 	// by a call to the 'Set' method. The configuration will sequentially call each receiver.
 	OnUpdate(callback NotificationReceiver)
+
+	// Stringify stringifies the config
+	Stringify(source Source) string
 }
 
 // Writer is a subset of Config that only allows writing the configuration
 type Writer interface {
 	Set(key string, value interface{}, source Source)
 	SetWithoutSource(key string, value interface{})
 	UnsetForSource(key string, source Source)
-	CopyConfig(cfg Config)
 }
 
 // ReaderWriter is a subset of Config that allows reading and writing the configuration

pkg/config/model/viper.go

@@ -835,24 +835,9 @@ func NewConfig(name string, envPrefix string, envKeyReplacer *strings.Replacer)
 	return &config
 }
 
-// CopyConfig copies the given config to the receiver config. This should only be used in tests as replacing
-// the global config reference is unsafe.
-func (c *safeConfig) CopyConfig(cfg Config) {
-	c.Lock()
-	defer c.Unlock()
-
-	if cfg, ok := cfg.(*safeConfig); ok {
-		c.Viper = cfg.Viper
-		c.configSources = cfg.configSources
-		c.envPrefix = cfg.envPrefix
-		c.envKeyReplacer = cfg.envKeyReplacer
-		c.proxies = cfg.proxies
-		c.configEnvVars = cfg.configEnvVars
-		c.unknownKeys = cfg.unknownKeys
-		c.notificationReceivers = cfg.notificationReceivers
-		return
-	}
-	panic("Replacement config must be an instance of safeConfig")
+// Stringify stringifies the config, but only for nodetremodel with the test build tag
+func (c *safeConfig) Stringify(_ Source) string {
+	return "safeConfig{...}"
 }
 
 // GetProxies returns the proxy settings from the configuration

pkg/config/model/viper_test.go

@@ -249,26 +249,6 @@ func TestCheckKnownKey(t *testing.T) {
 	assert.Contains(t, config.unknownKeys, "foobar")
 }
 
-func TestCopyConfig(t *testing.T) {
-	config := NewConfig("test", "DD", strings.NewReplacer(".", "_")) // nolint: forbidigo
-	config.SetDefault("baz", "qux")
-	config.Set("foo", "bar", SourceFile)
-	config.BindEnv("xyz", "XXYYZZ")
-	config.SetKnown("tyu")
-	config.OnUpdate(func(_ string, _, _ any) {})
-
-	backup := NewConfig("test", "DD", strings.NewReplacer(".", "_")) // nolint: forbidigo
-	backup.CopyConfig(config)
-
-	assert.Equal(t, "qux", backup.Get("baz"))
-	assert.Equal(t, "bar", backup.Get("foo"))
-	t.Setenv("XXYYZZ", "value")
-	assert.Equal(t, "value", backup.Get("xyz"))
-	assert.True(t, backup.IsKnown("tyu"))
-	// can't compare function pointers directly so just check the number of callbacks
-	assert.Len(t, backup.(*safeConfig).notificationReceivers, 1, "notification receivers should be copied")
-}
-
 func TestExtraConfig(t *testing.T) {
 	config := NewConfig("test", "DD", strings.NewReplacer(".", "_")) // nolint: forbidigo
 

pkg/config/nodetreemodel/config.go

@@ -305,6 +305,18 @@ func (c *ntmConfig) BuildSchema() {
 	}
 }
 
+// Stringify stringifies the config, but only with the test build tag
+func (c *ntmConfig) Stringify(source model.Source) string {
+	c.Lock()
+	defer c.Unlock()
+	// only does anything if the build tag "test" is enabled
+	text, err := c.toDebugString(source)
+	if err != nil {
+		return fmt.Sprintf("Stringify error: %s", err)
+	}
+	return text
+}
+
 func (c *ntmConfig) isReady() bool {
 	return c.ready.Load()
 }
@@ -482,14 +494,25 @@ func (c *ntmConfig) UnmarshalKey(key string, _rawVal interface{}, _opts ...viper
 	c.RLock()
 	defer c.RUnlock()
 	c.checkKnownKey(key)
-	return c.logErrorNotImplemented("UnmarshalKey")
+	return fmt.Errorf("nodetreemodel.UnmarshalKey not available, use pkg/config/structure.UnmarshalKey instead")
 }
 
 // MergeConfig merges in another config
-func (c *ntmConfig) MergeConfig(_in io.Reader) error {
+func (c *ntmConfig) MergeConfig(in io.Reader) error {
 	c.Lock()
 	defer c.Unlock()
-	return c.logErrorNotImplemented("MergeConfig")
+
+	content, err := io.ReadAll(in)
+	if err != nil {
+		return err
+	}
+
+	other := newInnerNode(nil)
+	if err = c.readConfigurationContent(other, content); err != nil {
+		return err
+	}
+
+	return c.root.Merge(other)
 }
 
 // MergeFleetPolicy merges the configuration from the reader given with an existing config
@@ -619,11 +642,9 @@ func (c *ntmConfig) ConfigFileUsed() string {
 	return c.configFile
 }
 
-// SetTypeByDefaultValue enables typing using default values
+// SetTypeByDefaultValue is a no-op
 func (c *ntmConfig) SetTypeByDefaultValue(_in bool) {
-	c.Lock()
-	defer c.Unlock()
-	c.logErrorNotImplemented("SetTypeByDefaultValue")
+	// do nothing: nodetreemodel always does this conversion
 }
 
 // BindEnvAndSetDefault binds an environment variable and sets a default for the given key
@@ -661,34 +682,13 @@ func NewConfig(name string, envPrefix string, envKeyReplacer *strings.Replacer)
 		envTransform:       make(map[string]func(string) interface{}),
 	}
 
-	config.SetTypeByDefaultValue(true)
 	config.SetConfigName(name)
 	config.SetEnvPrefix(envPrefix)
 	config.SetEnvKeyReplacer(envKeyReplacer)
 
 	return &config
 }
 
-// CopyConfig copies the given config to the receiver config. This should only be used in tests as replacing
-// the global config reference is unsafe.
-func (c *ntmConfig) CopyConfig(cfg model.Config) {
-	c.Lock()
-	defer c.Unlock()
-	c.logErrorNotImplemented("CopyConfig")
-	if cfg, ok := cfg.(*ntmConfig); ok {
-		// TODO: Probably a bug, should be a deep copy, add a test and verify
-		c.root = cfg.root
-		c.envPrefix = cfg.envPrefix
-		c.envKeyReplacer = cfg.envKeyReplacer
-		c.proxies = cfg.proxies
-		c.configEnvVars = cfg.configEnvVars
-		c.unknownKeys = cfg.unknownKeys
-		c.notificationReceivers = cfg.notificationReceivers
-		return
-	}
-	panic("Replacement config must be an instance of ntmConfig")
-}
-
 // ExtraConfigFilesUsed returns the additional config files used
 func (c *ntmConfig) ExtraConfigFilesUsed() []string {
 	c.Lock()

pkg/config/nodetreemodel/config_test.go

@@ -347,3 +347,81 @@ func TestAllKeysLowercased(t *testing.T) {
 	sort.Strings(keys)
 	assert.Equal(t, []string{"a", "b"}, keys)
 }
+
+func TestStringify(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)
+
+	txt := cfg.(*ntmConfig).Stringify("none")
+	expect := "Stringify error: invalid source: none"
+	assert.Equal(t, expect, txt)
+
+	txt = cfg.(*ntmConfig).Stringify(model.SourceDefault)
+	expect = `network_path
+  collector
+    input_chan_size
+      val:100000, source:default
+    processing_chan_size
+      val:100000, source:default
+    workers
+      val:4, source:default
+secret_backend_command
+  val:, source:default
+secret_backend_timeout
+  val:0, source:default
+server_timeout
+  val:30, source:default`
+	assert.Equal(t, expect, txt)
+
+	txt = cfg.(*ntmConfig).Stringify(model.SourceFile)
+	expect = `network_path
+  collector
+    workers
+      val:6, source:file
+secret_backend_command
+  val:./my_secret_fetcher.sh, source:file`
+	assert.Equal(t, expect, txt)
+
+	txt = cfg.(*ntmConfig).Stringify(model.SourceEnvVar)
+	expect = `network_path
+  collector
+    input_chan_size
+      val:23456, source:environment-variable
+secret_backend_timeout
+  val:60, source:environment-variable`
+	assert.Equal(t, expect, txt)
+
+	txt = cfg.(*ntmConfig).Stringify("root")
+	expect = `network_path
+  collector
+    input_chan_size
+      val:23456, source:environment-variable
+    processing_chan_size
+      val:100000, source:default
+    workers
+      val:6, source:file
+secret_backend_command
+  val:./my_secret_fetcher.sh, source:file
+secret_backend_timeout
+  val:60, source:environment-variable
+server_timeout
+  val:30, source:default`
+	assert.Equal(t, expect, txt)
+}

pkg/config/nodetreemodel/debug_string.go

@@ -0,0 +1,16 @@
+// 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.
+
+//go:build !test
+// +build !test
+
+package nodetreemodel
+
+import "github.com/DataDog/datadog-agent/pkg/config/model"
+
+func (c *ntmConfig) toDebugString(_ model.Source) (string, error) {
+	// don't show any data outside of tests, that way we don't have to worry about scrubbing
+	return "nodeTreeModelConfig{...}", nil
+}

pkg/config/nodetreemodel/debug_string_testonly.go

@@ -0,0 +1,65 @@
+// 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.
+
+//go:build test
+// +build test
+
+package nodetreemodel
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/DataDog/datadog-agent/pkg/config/model"
+)
+
+func (c *ntmConfig) toDebugString(source model.Source) (string, error) {
+	var node Node
+	switch source {
+	case "root":
+		node = c.root
+	case model.SourceEnvVar:
+		node = c.envs
+	case model.SourceDefault:
+		node = c.defaults
+	case model.SourceFile:
+		node = c.file
+	default:
+		return "", fmt.Errorf("invalid source: %s", source)
+	}
+	lines, err := debugTree(node, 0)
+	if err != nil {
+		return "", err
+	}
+	return strings.Join(lines, "\n"), nil
+}
+
+func debugTree(n Node, depth int) ([]string, error) {
+	padding := strings.Repeat("  ", depth)
+	if n == nil {
+		return []string{fmt.Sprintf("%s%v", padding, "<nil>")}, nil
+	}
+	if leaf, ok := n.(LeafNode); ok {
+		val := leaf.Get()
+		source := leaf.Source()
+		return []string{fmt.Sprintf("%sval:%v, source:%s", padding, val, source)}, nil
+	}
+	inner, ok := n.(InnerNode)
+	if !ok {
+		return nil, fmt.Errorf("unknown node type: %T", n)
+	}
+	keys := inner.ChildrenKeys()
+	result := []string{}
+	for _, key := range keys {
+		msg := fmt.Sprintf("%s%s", padding, key)
+		child, _ := n.GetChild(key)
+		rest, err := debugTree(child, depth+1)
+		if err != nil {
+			return nil, err
+		}
+		result = append(result, append([]string{msg}, rest...)...)
+	}
+	return result, nil
+}

pkg/config/nodetreemodel/read_config_file.go

@@ -60,7 +60,7 @@ func (c *ntmConfig) ReadConfig(in io.Reader) error {
 	if err != nil {
 		return err
 	}
-	if err := c.readConfigurationContent(content); err != nil {
+	if err := c.readConfigurationContent(c.file, content); err != nil {
 		return err
 	}
 	return c.mergeAllLayers()
@@ -71,15 +71,15 @@ func (c *ntmConfig) readInConfig(filePath string) error {
 	if err != nil {
 		return err
 	}
-	return c.readConfigurationContent(content)
+	return c.readConfigurationContent(c.file, content)
 }
 
-func (c *ntmConfig) readConfigurationContent(content []byte) error {
+func (c *ntmConfig) readConfigurationContent(target InnerNode, content []byte) error {
 	var obj map[string]interface{}
 	if err := yaml.Unmarshal(content, &obj); err != nil {
 		return err
 	}
-	c.warnings = append(c.warnings, loadYamlInto(c.defaults, c.file, obj, "")...)
+	c.warnings = append(c.warnings, loadYamlInto(c.defaults, target, obj, "")...)
 	return nil
 }
 

pkg/config/teeconfig/teeconfig.go

@@ -364,11 +364,9 @@ func (t *teeConfig) Object() model.Reader {
 	return t.baseline
 }
 
-// CopyConfig copies the given config to the receiver config. This should only be used in tests as replacing
-// the global config reference is unsafe.
-func (t *teeConfig) CopyConfig(cfg model.Config) {
-	t.baseline.CopyConfig(cfg)
-	t.compare.CopyConfig(cfg)
+// Stringify stringifies the config
+func (t *teeConfig) Stringify(source model.Source) string {
+	return t.baseline.Stringify(source)
 }
 
 func (t *teeConfig) GetProxies() *model.Proxy {