[Backport v1.30] Fix YAML mapper for helm2dda for Helm's configmap based configurations - #3409
[Backport v1.30] Fix YAML mapper for helm2dda for Helm's configmap based configurations#3409dd-octo-sts[bot] wants to merge 1 commit into
Conversation
#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)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a221ed9294
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| 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 { |
There was a problem hiding this comment.
Preserve parent mappings when descendants are intentionally empty
When a table has any descendant mapping entry, this condition now prevents its parent mapping from running even if those descendants are intentionally unmapped. For example, agents.updateStrategy: {type: OnDelete} should be copied through the parent mapping at mapping_datadog_helm_to_datadogagent_crd.yaml:150, while its type descendant at line 152 has an empty destination; hasMappedDescendant therefore skips the parent and the child emits an error without adding anything, so the generated DatadogAgent loses the requested update strategy. Distinguish descendants that replace the parent mapping from empty bookkeeping entries.
Useful? React with 👍 / 👎.
|
Backport 96de46d from #3373.
What does this PR do?
Fixes the following issues when the
helm2ddaflag is run for the Datadog Plugin forkubectl.When using
datadog.otelCollector.configin Helm like:As a result, the mappings come out with three duplicate entries which the latter two are invalid:
In addition, using
clusterAgent.confdlike:As a result, the mappings for
extraConfdare missing theconfigDataMaplike:datadog.confdis unaffected.In addition,
agents.customAgentConfigandclusterAgent.datadog_cluster_yamldoes not render in the generated DatadogAgent at all and errors out with:Motivation
CONS-8504
Additional Notes
Some things to flag which was not covered in this PR:
clusterAgent.advancedConfdis also broken as it would render something like the following which isn't valid:clusterAgent.advancedConfdwould belong here as it does not have a corresponding configuration in theDatadogAgentCR so not sure if we should also make this an empty string: https://github.com/DataDog/datadog-operator/blob/main/cmd/yaml-mapper/mapper/mapping_datadog_helm_to_datadogagent_crd.yaml#L180Despite my changes rendering the DatadogAgent CR properly, it still produced these phantom errors (first error might be expected as there is no equivalent in the DatadogAgent):
Describe your test plan
Tested changes locally with the following values.yaml:
Then run the
helm2ddaflag with:Desired Output:
Checklist
bug,enhancement,refactoring,documentation,tooling, and/ordependenciesqa/skip-qalabel