Skip to content

Preserve Terraform data source policies with unknown values#181

Merged
whitemerch merged 4 commits into
mainfrom
chakib.hamie/unknwon_value_tf
Jun 12, 2026
Merged

Preserve Terraform data source policies with unknown values#181
whitemerch merged 4 commits into
mainfrom
chakib.hamie/unknwon_value_tf

Conversation

@whitemerch

@whitemerch whitemerch commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Motivation

Terraform aws_iam_policy_document data sources can contain values that are unknown during static analysis. Previously, a single unknown value caused JSON serialization to fail, which dropped the whole policy document from analysis and emitted noisy error logs.

Changes

Treat unknown cty values as null before serializing Terraform data source policies, preserving the statically-known portions of the policy instead of dropping the whole document. The remaining serialization failure log is downgraded because routine unknown Terraform values are now handled explicitly.

A regression test covers a policy with an unknown scalar field while preserving known statement fields.

Author Checklist

  • I have reviewed my own PR.
  • I have added or updated relevant unit tests where necessary. If no tests are added, I've explained why.
  • All new and existing tests pass.
  • I have tested my changes on staging (if applicable).
  • I have updated any relevant documentation (if applicable).

QA Instruction

  1. go test ./pkg/parser/terraform/... -run Test_getDataSourcePolicy -v
  2. Confirm the unknown-value fixture still emits the known IAM policy statement instead of an empty policy.

Blast Radius

This PR affects Terraform data source policy parsing in the IaC scanner. It only changes handling of unknown values while decoding aws_iam_policy_document data sources.

Additional Notes

I submit this contribution under the Apache-2.0 license.

@datadog-prod-us1-6

datadog-prod-us1-6 Bot commented Jun 11, 2026

Copy link
Copy Markdown

🎯 Code Coverage (details)
Patch Coverage: 71.43%
Overall Coverage: 47.98% (+0.02%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 38b5f0e | Docs | Datadog PR Page | Give us feedback!

@whitemerch
whitemerch marked this pull request as ready for review June 12, 2026 07:57
@whitemerch
whitemerch requested a review from a team as a code owner June 12, 2026 07:57
chatgpt-codex-connector[bot]

This comment was marked as resolved.

@ChouraquiBen ChouraquiBen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The code seems to be doing what it means to do to me, but I would like a little bit more context if possible 🙏
Do you know which data resources could not be resolved? Within the test, I am not sure which part is supposed to make the scan fail if the feature is inactive

@whitemerch

whitemerch commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

The code seems to be doing what it means to do to me, but I would like a little bit more context if possible 🙏 Do you know which data resources could not be resolved? Within the test, I am not sure which part is supposed to make the scan fail if the feature is inactive

@ChouraquiBen The affected data source is aws_iam_policy_document. When any scalar field in a statement (effect, sid, id, version, condition.test, condition.variable, principals.type) references a Terraform variable that has no default value, the parser was failing to serialise the entire policy block and silently dropping it from analysis, so IAM policy rules would never evaluate it.

If you want to try it out: run the new test added in this PR, but without what is added in the scanner in this PR. It will fail because getDataSourcePolicy returns an empty result for partial_unknowns, whereas the expected output is a valid policy statement with effect, actions, and resources all present

@whitemerch
whitemerch force-pushed the chakib.hamie/unknwon_value_tf branch from 43bde21 to abb0a6f Compare June 12, 2026 09:32
Comment thread pkg/parser/terraform/data_source.go Outdated

@ChouraquiBen ChouraquiBen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@whitemerch
whitemerch merged commit b81498a into main Jun 12, 2026
18 checks passed
@whitemerch
whitemerch deleted the chakib.hamie/unknwon_value_tf branch June 12, 2026 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants