Skip to content

Conversation

@fanny-jiang
Copy link
Contributor

@fanny-jiang fanny-jiang commented Oct 27, 2025

What does this PR do?

Add a helm values.yaml->DDA mapper binary.

Motivation

What inspired you to submit this pull request?

Additional Notes

The mapper currently generates a large DDA manifest yaml file. Future work to improve readability of the DDA includes omitting default fields in the final mapped DDA.

Minimum Agent Versions

Are there minimum versions of the Datadog Agent and/or Cluster Agent required?

  • Agent: vX.Y.Z
  • Cluster Agent: vX.Y.Z

Describe your test plan

  • Run unit tests: make yaml-mapper-unit-tests
  • Build yaml-mapper: make yaml-mapper
  • Show yaml-mapper usage:
bin/yaml-mapper
bin/yaml-mapper map --help 
  • Test mapper with examples/example_source.yaml
bin/yaml-mapper map --sourcePath=cmd/yaml-mapper/examples/example_source.yaml --namespace=<namespace> 
  • Should create a mapped DDA at filename dda.yaml.<timestamp>
  • Run the command again a few more times and the output content should remain the same across all runs.
  • In a k8s cluster with the latest datadog-operator running and DDA CRD installed, apply the dda.yaml file. There should be no errors.
  • Modify the example_source.yaml to add a key that doesn't have a mapped destination key, e.g. agents.podSecurity.allowedUnsafeSysctls1.
    • This key should get logged in the output with a warning, indicating that the key could not be mapped.
    • The DDA output should remain unchanged

Test flags:

  -d, --destPath string      Path to destination YAML file.
  -m, --mappingPath string   Path to mapping YAML file.
  -n, --namespace string     If present, the namespace scope for this CLI request
  -o, --printOutput          print mapped DDA output to stdout (default true)
  -f, --sourcePath string    Path to source YAML file. Required. Example: source.yaml
  • destPath:

    • if the path exists, mapped DDA is written to this path
    • if the path does not exist, a new file is created at this path and mapped DDA is written to it
    • if the path is not provided, a new file at dda.yaml.<timestamp> is created
  • mappingPath:

    • if mappingPath is provided, it'll be used
    • if mappingPath is not provided, it'll use the default mapping. The default mapping file is determined in this order:
      1. The latest mapping file in the helm-charts repo
      2. If the mapping file cannot be retrieved, it'll fall back on the embedded mapping file at cmd/yaml-mapper/mapper/mapping_datadog_helm_to_datadogagent_crd.yaml
  • sourcePath:

    • required: binary should error if it's not provided
  • namespace:

    • if provided, will be set in the DDA metadata.namespace field
    • if not provided, the DDA metadata.namespace field is omitted
  • printOutput

    • default true: prints the DDA output to the stdout
    • if set to false, DDA output is not sent to stdout. Logging will continue to be printed

(Let's ignore the updateMap and headerPath for now since they're not critical for functionality)

Checklist

  • PR has at least one valid label: bug, enhancement, refactoring, documentation, tooling, and/or dependencies
  • PR has a milestone or the qa/skip-qa label

@fanny-jiang fanny-jiang added this to the v1.21.0 milestone Oct 27, 2025
@fanny-jiang fanny-jiang added the enhancement New feature or request label Oct 27, 2025
@fanny-jiang fanny-jiang mentioned this pull request Oct 27, 2025
2 tasks
@fanny-jiang fanny-jiang force-pushed the fanny/AGENTONB-2618/yaml-mapper branch from 31d84ed to 9b2c720 Compare October 27, 2025 17:56
@fanny-jiang fanny-jiang changed the title Add helm-dda mapper binary Add helm-dda yaml mapper Oct 27, 2025
@codecov-commenter
Copy link

codecov-commenter commented Oct 27, 2025

Codecov Report

❌ Patch coverage is 32.29167% with 455 lines in your changes missing coverage. Please review.
✅ Project coverage is 36.83%. Comparing base (c9626d1) to head (91a11cf).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
cmd/yaml-mapper/mapper/mapper.go 50.50% 73 Missing and 26 partials ⚠️
cmd/yaml-mapper/mapper/cmd.go 0.00% 96 Missing ⚠️
cmd/yaml-mapper/utils/maps.go 0.00% 60 Missing ⚠️
cmd/yaml-mapper/utils/deprecations.go 0.00% 49 Missing ⚠️
cmd/yaml-mapper/utils/utils.go 0.00% 47 Missing ⚠️
cmd/yaml-mapper/utils/charts_remote.go 0.00% 37 Missing ⚠️
cmd/yaml-mapper/mapper/map_processors.go 76.82% 20 Missing and 15 partials ⚠️
cmd/yaml-mapper/main.go 0.00% 29 Missing ⚠️
hack/generate-docs/annotation-parser.go 0.00% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2268      +/-   ##
==========================================
- Coverage   36.92%   36.83%   -0.09%     
==========================================
  Files         273      281       +8     
  Lines       22921    23622     +701     
==========================================
+ Hits         8463     8701     +238     
- Misses      13819    14240     +421     
- Partials      639      681      +42     
Flag Coverage Δ
unittests 36.83% <32.29%> (-0.09%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
hack/generate-docs/annotation-parser.go 0.00% <0.00%> (ø)
cmd/yaml-mapper/main.go 0.00% <0.00%> (ø)
cmd/yaml-mapper/mapper/map_processors.go 76.82% <76.82%> (ø)
cmd/yaml-mapper/utils/charts_remote.go 0.00% <0.00%> (ø)
cmd/yaml-mapper/utils/utils.go 0.00% <0.00%> (ø)
cmd/yaml-mapper/utils/deprecations.go 0.00% <0.00%> (ø)
cmd/yaml-mapper/utils/maps.go 0.00% <0.00%> (ø)
cmd/yaml-mapper/mapper/cmd.go 0.00% <0.00%> (ø)
cmd/yaml-mapper/mapper/mapper.go 50.50% <50.50%> (ø)

... and 2 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c9626d1...91a11cf. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fanny-jiang fanny-jiang marked this pull request as ready for review October 27, 2025 21:33
@fanny-jiang fanny-jiang requested review from a team as code owners October 27, 2025 21:33
@fanny-jiang fanny-jiang force-pushed the fanny/AGENTONB-2618/yaml-mapper branch from c74a02e to e2d359a Compare October 27, 2025 22:26
@fanny-jiang fanny-jiang force-pushed the fanny/AGENTONB-2618/yaml-mapper branch from e2d359a to 174967e Compare October 28, 2025 18:36
@fanny-jiang fanny-jiang requested a review from a team as a code owner October 28, 2025 18:36
Copy link
Contributor

@levan-m levan-m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High level structure and cli experience is great! didn't go through the mapping logic line by line but tests seem to cover a lot and did my testing too.
I left some comments/suggestions most of those can be addressed in follow-ups except the deadline/timeout.


destKey, _ := mappingValues[sourceKey]
if (destKey == "" || destKey == nil) && !apiutils.IsEqualStruct(pathVal, defaultVal) {
log.Printf("Warning: DDA destination key not found. Could not map: %s\n", sourceKey)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't catch case when source contains keys not mapped to "" in the mapper file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed in 5d36072

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix added by go linter

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixes added by go linter

@fanny-jiang fanny-jiang requested a review from levan-m November 19, 2025 22:33
@fanny-jiang fanny-jiang requested a review from a team as a code owner November 24, 2025 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants