Fix YAML mapper for helm2dda for Helm's configmap based configurations (#3373) (#3409)

Fix YAML mapper for helm2dda for Helm's configmap based configurations (#3373)

Fix YAML mapper for helm2dda for datadog.otelCollector.config and clusterAgent.confd

Add fixes for agents.customAgentConfig and clusterAgent.cluster_yaml

Restrict multi-key table fallback to mapFunc destinations

Allowing any multi-key table through the fallback (added to support
agents.customAgentConfig/clusterAgent.datadog_cluster_yaml) let it also
apply to plain string/list destinations like
agents.podSecurity.seLinuxContext, which have separate leaf mappings for
their sub-fields. That produced invalid duplicate/extra fields in the
output. Now the multi-key fallback only fires for mapFunc-based
destinations (e.g. mapCustomConfigFile), matching Codex review feedback
on PR #3373.

Co-authored-by: Cursor <cursoragent@cursor.com>

add test coverage for mapCustomConfigFile

Co-authored-by: Cursor <cursoragent@cursor.com>

fix multi-key table handling for helm2dda confd/customAgentConfig mappings

extend helm2dda test fixtures to cover multi-key confd and customAgentConfig mapping

Co-authored-by: patrick.liang <patrick.liang@datadoghq.com>
(cherry picked from commit 96de46d)

Merge branch 'v1.30' into backport-3373-to-v1.30

Co-authored-by: tbavelier <97530782+tbavelier@users.noreply.github.com>
Co-authored-by: mrdoggopat <109171317+mrdoggopat@users.noreply.github.com>
Co-authored-by: timothee.bavelier <timothee.bavelier@datadoghq.com>

Author: 3 people

cmd/yaml-mapper/mapper/map_processors.go

@@ -34,6 +34,7 @@ func mapFuncRegistry() map[string]MappingRunFunc {
 		mapAppendEnvVar,
 		mapMergeEnvs,
 		mapOverrideType,
+		mapCustomConfigFile,
 	} {
 		registry[p.name] = p.runFunc
 	}
@@ -296,6 +297,44 @@ var mapOverrideType = MappingProcessor{
 	},
 }
 
+// mapCustomConfigFile serializes an object-shaped Helm value (e.g. custom datadog.yaml settings)
+// into a YAML string and stores it under CustomConfig's `configData` field, keyed by the given
+// filename. The filename is passed via `args` instead of `newPath` since it can contain dots
+// (e.g. "datadog.yaml") that shouldn't be split into nested path segments.
+// args:
+//   - fileName: datadog.yaml
+var mapCustomConfigFile = MappingProcessor{
+	name: "mapCustomConfigFile",
+	runFunc: func(interim map[string]any, newPath string, pathVal any, args []any) {
+		if len(args) != 1 {
+			return
+		}
+		fileName, ok := utils.GetPathString(args[0], "fileName")
+		if !ok || fileName == "" {
+			return
+		}
+
+		var configData string
+		switch v := pathVal.(type) {
+		case string:
+			configData = v
+		default:
+			out, err := yaml.Marshal(pathVal)
+			if err != nil {
+				slog.Error("failed to marshal custom config content", "path", newPath, "fileName", fileName, "error", err)
+				return
+			}
+			configData = string(out)
+		}
+
+		utils.MergeOrSet(interim, newPath, map[string]any{
+			fileName: map[string]any{
+				"configData": configData,
+			},
+		})
+	},
+}
+
 // hasDuplicateEnv checks if a given env var name is already present in the given list of env vars.
 func hasDuplicateEnv(existingEnvs []any, newEnvName string) bool {
 	for _, existingEnv := range existingEnvs {

cmd/yaml-mapper/mapper/mapper.go

@@ -197,20 +197,27 @@ func (m *Mapper) mapValues(sourceValues chartutil.Values, mappingValues chartuti
 
 	// Map values.yaml => DDA
 	for _, sourceKey := range mappingKeys {
+		destKey, _ := mappingValues[sourceKey]
+		// Only copy a whole table if none of its sub-keys are mapped separately
+		// (e.g. agents.podSecurity.seLinuxContext.rule has its own mapping entry,
+		// so seLinuxContext itself shouldn't be copied as one big table).
+		hasMappedDescendant := hasDescendantMappingKey(mappingKeys, sourceKey)
+
 		pathVal, _ := sourceValues.PathValue(sourceKey)
 		if pathVal == nil {
 			if mapVal, ok := utils.GetPathMap(sourceValues[sourceKey]); ok && mapVal != nil {
 				pathVal = mapVal
-			} else if tableVal, err := sourceValues.Table(sourceKey); err == nil && len(tableVal) == 1 {
+			} else if tableVal, err := sourceValues.Table(sourceKey); err == nil && len(tableVal) > 0 && !hasMappedDescendant {
 				pathVal = tableVal
 			} else {
 				continue
 			}
 		}
 
-		utils.MergeOrSet(sourceKeysRef, sourceKey, map[string]any{"visited": true})
+		// Mark this key and everything nested under it as visited, since copying the
+		// whole table also covers all of its nested values.
+		markVisited(sourceKeysRef, sourceKey)
 
-		destKey, _ := mappingValues[sourceKey]
 		if (destKey == "" || destKey == nil) && !shouldSkipMappingKey(sourceKey) {
 			slog.Error("DDA destination key not found", "sourceKey", sourceKey)
 			errorCount++
@@ -383,6 +390,33 @@ func (m *Mapper) updateMapping(sourceValues chartutil.Values, mappingValues char
 	return nil
 }
 
+// hasDescendantMappingKey returns true if mappingKeys contains an entry that is a strict
+// descendant of sourceKey (i.e. prefixed with "sourceKey."). Such entries indicate that
+// sub-fields of the table at sourceKey are mapped individually, so the table itself must
+// not be copied wholesale.
+func hasDescendantMappingKey(mappingKeys []string, sourceKey string) bool {
+	prefix := sourceKey + "."
+	for _, k := range mappingKeys {
+		if strings.HasPrefix(k, prefix) {
+			return true
+		}
+	}
+	return false
+}
+
+// markVisited marks sourceKey, and every flattened key nested beneath it, as visited in
+// sourceKeysRef. This is used when a table value is consumed as a whole (either copied
+// directly or passed to a mapFunc), which implicitly consumes all of its nested values too.
+func markVisited(sourceKeysRef map[string]any, sourceKey string) {
+	utils.MergeOrSet(sourceKeysRef, sourceKey, map[string]any{"visited": true})
+	prefix := sourceKey + "."
+	for k := range sourceKeysRef {
+		if strings.HasPrefix(k, prefix) {
+			utils.MergeOrSet(sourceKeysRef, k, map[string]any{"visited": true})
+		}
+	}
+}
+
 // flattenValues builds a mapping of dotted-key paths from a provided Values source.
 func flattenValues(sourceValues chartutil.Values, valuesMap map[string]any, prefix string) map[string]any {
 	for key, value := range sourceValues {