Skip to content

fix(terraform): support Terraform 1.15+ module source interpolation - #2915

Open
Erik Osterman (Cloud Posse) (osterman) wants to merge 6 commits into
mainfrom
osterman/issue-2913
Open

fix(terraform): support Terraform 1.15+ module source interpolation#2915
Erik Osterman (Cloud Posse) (osterman) wants to merge 6 commits into
mainfrom
osterman/issue-2913

Conversation

@osterman

@osterman Erik Osterman (Cloud Posse) (osterman) commented Aug 10, 2026

Copy link
Copy Markdown
Member

what

  • Atmos no longer fails to parse a Terraform component whose module block uses variable interpolation in source (e.g. source = "./mods/${var.org}") when the variable is declared const = true — valid syntax under Terraform 1.15+, not just OpenTofu 1.8+.
  • Generalized the existing "Variables not allowed" diagnostic skip in internal/exec/utils.go / internal/exec/terraform_detection.go so it no longer depends on detecting OpenTofu — renamed isKnownOpenTofuFeatureisKnownModuleSourceInterpolationDiagnostic, and the component_info flag validation_skipped_opentofuvalidation_skipped_module_source_interpolation.
  • Hardened that skip so it can never silently swallow a genuine, unrelated HCL error that happens to co-occur in the same module: diagnostics are now inspected individually and grouped by source position (allDiagnosticsAreModuleSourceInterpolation), instead of pattern-matching the collapsed Diagnostics.Error() string, which only renders the first diagnostic's text.
  • Added a new regression fixture/test reproducing the exact issue on plain terraform (no command: override), plus a fixture/test proving a real unrelated error is still surfaced when it co-occurs with the known-safe diagnostic.
  • Investigated whether Atmos's SBOM generation is affected by dynamic module sources; confirmed it isn't (it reads already-resolved sources from terraform modules -json, never the static parser), and added a permanent guard test (pkg/sbom/terraform_test.go) for that invariant.
  • Bumped the nanoid pnpm override in website/package.json to resolve the transitive website/pnpm-lock.yaml dependency to nanoid@3.3.18, fixing an open Dependabot alert (infinite loop on zero-size input). The two open image-size alerts have no upstream patch yet and are not auto-fixable.
  • Fixed pre-existing EditorConfig violations (tabs instead of the required 2-space indent) in docs/prd/opentofu-module-source-interpolation.md, surfaced once that file entered the branch's diff.

why

  • Atmos pre-parses every Terraform component with terraform-config-inspect before running any Terraform/OpenTofu command. That library decodes a module's source attribute with a nil hcl.EvalContext, so any variable reference there always produces the "Variables not allowed" diagnostic — regardless of whether the configured tool/version actually supports it.
  • Atmos already tolerated this diagnostic for OpenTofu 1.8+ (PR Add OpenTofu 1.8+ module source interpolation support #1756), but Terraform 1.15 (April 2026) added the equivalent capability via const = true variables, so plain-Terraform users hit the same diagnostic as a hard failure even though their syntax is valid.
  • The diagnostic text can't distinguish "valid under a modern tool" from "genuinely invalid" — Atmos already accepted that ambiguity unconditionally for OpenTofu, so extending the same leniency to Terraform is consistent, provided a real unrelated error can never be silently discarded alongside it (the second commit's fix).

references

Summary by CodeRabbit

  • Bug Fixes

    • Added support for Terraform 1.15+ module sources using constant-variable interpolation.
    • Prevented known parser diagnostics from blocking component descriptions while preserving unrelated errors.
    • Preserved resolved dynamic module sources in software bill of materials (SBOM) metadata.
  • Documentation

    • Added fix reports, usage guidance, and regression coverage details for Terraform module-source interpolation.

…enTofu

Atmos's terraform-config-inspect-based component validation already
tolerated the "Variables not allowed" HCL diagnostic produced by a
module's `source` attribute referencing a variable, but only when the
configured command was detected as OpenTofu. Since Terraform 1.15
added the equivalent capability via `const = true` variables, plain
Terraform users hit the same diagnostic as a hard failure. Decouple
the skip from tool detection so it applies regardless of terraform vs
tofu, since the diagnostic is a static-parser limitation rather than
an OpenTofu-specific feature gate.

Closes #2913

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Bump the pnpm override for the nanoid@3.3.3 resolution range from
^3.3.15 to ^3.3.17, resolving the transitive website/pnpm-lock.yaml
dependency to nanoid@3.3.18 and picking up the fix for
GHSA-2v37-7h3g-55p8 (CVE-2026-67213), an infinite loop in
customAlphabet/customRandom when size is 0.

The other two open Dependabot alerts on image-size (#275, #276) have
no first_patched_version published upstream yet, so they are not
auto-fixable at this time.

Also fixes pre-existing EditorConfig violations (tabs instead of the
required 2-space indent, and a mis-sized list-continuation indent) in
docs/prd/opentofu-module-source-interpolation.md, surfaced by the
atmos-validate-editorconfig pre-commit hook once that file entered
this branch's diff against origin/main. Whitespace-only, no content
altered.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…nown module-source diagnostic

terraform-config-inspect's Diagnostics.Error() only renders the first
diagnostic's text, collapsing any others to "(and N other messages)".
The previous fix pattern-matched that collapsed string, so a genuine,
unrelated HCL error co-occurring in the same module -- sorting after
the known-safe "Variables not allowed" diagnostic -- was silently
discarded instead of failing.

Inspect every error-severity diagnostic individually and group them by
source position (allDiagnosticsAreModuleSourceInterpolation), since
terraform-config-inspect emits a companion "Unsuitable value" diagnostic
at the same position as the module-source one as a side effect of the
same nil-hcl.EvalContext decode failure. A position's group is
known-safe if any diagnostic in it matches; diagnostics at other
positions must independently match, so a real error is never absorbed
into the known-safe group just by sharing the diagnostics list.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@atmos-pro

atmos-pro Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Tip

Atmos Pro  

No affected stacks workflow was detected for this pull request.
If this is expected, no action is needed.
Learn More. Ask AI.

@osterman Erik Osterman (Cloud Posse) (osterman) added the patch A minor, backward compatible change label Aug 10, 2026
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c38ba5a2-61c5-464c-83bf-6131fe09617d

📥 Commits

Reviewing files that changed from the base of the PR and between 8e2c04d and 261eba8.

📒 Files selected for processing (2)
  • internal/exec/opentofu_module_source_interpolation_test.go
  • pkg/sbom/terraform_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • internal/exec/opentofu_module_source_interpolation_test.go
  • pkg/sbom/terraform_test.go

📝 Walkthrough

Walkthrough

The change generalizes Terraform and OpenTofu module-source interpolation handling. It evaluates diagnostics by source position, preserves unrelated errors, adds Terraform and SBOM regression coverage, updates documentation, and changes the website nanoid override.

Changes

Module source diagnostic handling

Layer / File(s) Summary
Diagnostic matching and validation control
internal/exec/terraform_detection.go, internal/exec/utils.go, internal/exec/*_test.go
Validation recognizes known module-source interpolation diagnostics for Terraform and OpenTofu. Diagnostics are grouped by source position, and unrelated errors remain fatal.
Terraform integration and regression coverage
internal/exec/terraform_module_source_interpolation_test.go, internal/exec/opentofu_module_source_interpolation_test.go, pkg/sbom/terraform_test.go, tests/fixtures/scenarios/terraform-module-source-interpolation*
Fixtures and tests cover const-variable module sources, mixed diagnostics, skipped-validation metadata, and resolved SBOM sources.
Diagnostic behavior documentation
docs/fixes/*, docs/prd/opentofu-module-source-interpolation.md
Documentation describes Terraform 1.15 support, generalized handling, renamed metadata, regression coverage, and validation results.

Website dependency update

Layer / File(s) Summary
Nanoid override
website/package.json
The nanoid@3.3.3 override changes from ^3.3.15 to ^3.3.17.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related issues

Possibly related PRs

  • cloudposse/atmos#2914 — This change extends the const-variable module-source interpolation fix with tool-independent diagnostic handling and mixed-diagnostic tests.

Sequence Diagram(s)

sequenceDiagram
  participant TerraformConfigInspect
  participant TerraformDetection
  participant ProcessStacks
  TerraformConfigInspect->>TerraformDetection: Return configuration diagnostics
  TerraformDetection->>TerraformDetection: Group errors by source position
  TerraformDetection->>ProcessStacks: Allow skip only for known interpolation groups
  ProcessStacks->>ProcessStacks: Record skipped-validation metadata
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: support for Terraform 1.15+ module source interpolation.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch osterman/issue-2913

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues found.

Scanned Files

None

Durable fix-log entry for b44359d per this repo's bug-fixing
convention, including validation performed and the known,
intentionally-untracked follow-up (module.source/version scoping).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/exec/opentofu_module_source_interpolation_test.go`:
- Around line 115-118: Update the validation marker assertion in the
module-source interpolation test to require that componentInfo contains
validation_skipped_module_source_interpolation, then verify its value is true.
Remove the conditional existence guard so the test fails when the marker is
absent.

In `@internal/exec/terraform_detection.go`:
- Around line 142-144: Update matchesModuleSourceInterpolationPattern to
re-parse the affected Terraform file and suppress only diagnostics whose ranges
fall within a module block’s source or version attribute; do not treat generic
“Variables not allowed” text as sufficient. Add a regression case proving an
invalid non-module attribute such as output.description remains rejected.

In `@pkg/sbom/terraform_test.go`:
- Around line 191-206: Remove t.Parallel() from this test because it replaces
the package-global runTerraformModules hook. Keep the test serialized with
TestAppendTerraformMarksModulesIncompleteWhenLocalModuleUnresolvable and
preserve the existing setup and cleanup.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 70c50828-57fe-4185-983f-8137445679e6

📥 Commits

Reviewing files that changed from the base of the PR and between 4aec494 and 8e2c04d.

⛔ Files ignored due to path filters (1)
  • website/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (19)
  • docs/fixes/2026-08-10-module-source-interpolation-diagnostic-swallow.md
  • docs/fixes/2026-08-10-terraform-module-source-interpolation.md
  • docs/prd/opentofu-module-source-interpolation.md
  • internal/exec/opentofu_module_source_interpolation_test.go
  • internal/exec/terraform_detection.go
  • internal/exec/terraform_detection_test.go
  • internal/exec/terraform_module_source_interpolation_test.go
  • internal/exec/utils.go
  • pkg/sbom/terraform_test.go
  • tests/fixtures/scenarios/terraform-module-source-interpolation-mixed-diagnostics/atmos.yaml
  • tests/fixtures/scenarios/terraform-module-source-interpolation-mixed-diagnostics/components/terraform/test-component/main.tf
  • tests/fixtures/scenarios/terraform-module-source-interpolation-mixed-diagnostics/components/terraform/test-component/mods/acme/main.tf
  • tests/fixtures/scenarios/terraform-module-source-interpolation-mixed-diagnostics/stacks/test-stack.yaml
  • tests/fixtures/scenarios/terraform-module-source-interpolation/README.md
  • tests/fixtures/scenarios/terraform-module-source-interpolation/atmos.yaml
  • tests/fixtures/scenarios/terraform-module-source-interpolation/components/terraform/test-component/main.tf
  • tests/fixtures/scenarios/terraform-module-source-interpolation/components/terraform/test-component/mods/acme/main.tf
  • tests/fixtures/scenarios/terraform-module-source-interpolation/stacks/test-stack.yaml
  • website/package.json

Comment thread internal/exec/opentofu_module_source_interpolation_test.go Outdated
Comment thread internal/exec/terraform_detection.go
Comment thread pkg/sbom/terraform_test.go Outdated
- opentofu_module_source_interpolation_test.go: require the
  validation_skipped_module_source_interpolation key to be present
  instead of silently passing when it's absent.
- pkg/sbom/terraform_test.go: drop t.Parallel() from
  TestAppendModulesForDirectoryRecordsResolvedDynamicModuleSource --
  it mutates the package-global runTerraformModules hook, same as
  TestAppendTerraformMarksModulesIncompleteWhenLocalModuleUnresolvable,
  risking a race between the two.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.77%. Comparing base (f31c1ec) to head (911a185).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2915      +/-   ##
==========================================
- Coverage   82.79%   82.77%   -0.02%     
==========================================
  Files        1866     1866              
  Lines      181025   181045      +20     
==========================================
- Hits       149882   149864      -18     
- Misses      23331    23362      +31     
- Partials     7812     7819       +7     
Flag Coverage Δ
unittests 82.77% <100.00%> (-0.02%) ⬇️

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

Files with missing lines Coverage Δ
internal/exec/terraform_detection.go 87.83% <100.00%> (+5.48%) ⬆️
internal/exec/utils.go 89.21% <100.00%> (-0.05%) ⬇️

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

patch A minor, backward compatible change size/m Medium size PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Atmos fails to load any Terraform component that uses variable interpolation

1 participant