-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: get_original_terragrunt_dir during stack generate
#5176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix: get_original_terragrunt_dir during stack generate
#5176
Conversation
… generating stack
|
@philipmckenna-symphony is attempting to deploy a commit to the Gruntwork Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughPropagates the original Terragrunt stack config path into the stack parsing context by setting Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related issues
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
config/stack.go (1)
414-436: Optional: mirror original-path behavior in ReadStackConfigStringIf
ReadStackConfigStringis ever used in contexts that rely onget_original_terragrunt_dirfor stacks, consider cloningoptshere as well and settingOriginalTerragruntConfigPath = configPathbefore building the parsing context, for symmetry withReadStackConfigFile. Not urgent given current usage.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
config/stack.go(1 hunks)test/fixtures/stacks/get-original-terragrunt-dir/live/account1/no-locals-nested/no-locals/terragrunt.stack.hcl(1 hunks)test/fixtures/stacks/get-original-terragrunt-dir/live/account1/no-locals-nested/read-config/terragrunt.stack.hcl(1 hunks)test/fixtures/stacks/get-original-terragrunt-dir/live/account1/no-locals-nested/with-locals/terragrunt.stack.hcl(1 hunks)test/fixtures/stacks/get-original-terragrunt-dir/live/account1/non-nested/no-locals/terragrunt.stack.hcl(1 hunks)test/fixtures/stacks/get-original-terragrunt-dir/live/account1/non-nested/read-config/terragrunt.stack.hcl(1 hunks)test/fixtures/stacks/get-original-terragrunt-dir/live/account1/non-nested/with-locals/terragrunt.stack.hcl(1 hunks)test/fixtures/stacks/get-original-terragrunt-dir/live/account1/read-config-nested/no-locals/terragrunt.stack.hcl(1 hunks)test/fixtures/stacks/get-original-terragrunt-dir/live/account1/read-config-nested/read-config/terragrunt.stack.hcl(1 hunks)test/fixtures/stacks/get-original-terragrunt-dir/live/account1/read-config-nested/with-locals/terragrunt.stack.hcl(1 hunks)test/fixtures/stacks/get-original-terragrunt-dir/live/account1/with-locals-nested/no-locals/terragrunt.stack.hcl(1 hunks)test/fixtures/stacks/get-original-terragrunt-dir/live/account1/with-locals-nested/read-config/terragrunt.stack.hcl(1 hunks)test/fixtures/stacks/get-original-terragrunt-dir/live/account1/with-locals-nested/with-locals/terragrunt.stack.hcl(1 hunks)test/fixtures/stacks/get-original-terragrunt-dir/live/common/stack_config.hcl(1 hunks)test/fixtures/stacks/get-original-terragrunt-dir/stacks/no-locals/terragrunt.stack.hcl(1 hunks)test/fixtures/stacks/get-original-terragrunt-dir/stacks/read-config/terragrunt.stack.hcl(1 hunks)test/fixtures/stacks/get-original-terragrunt-dir/stacks/with-locals/terragrunt.stack.hcl(1 hunks)test/fixtures/stacks/get-original-terragrunt-dir/units/terragrunt.hcl(1 hunks)test/integration_stacks_test.go(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
⚙️ CodeRabbit configuration file
Review the Go code for quality and correctness. Make sure that the Go code follows best practices, is performant, and is easy to understand and maintain.
Files:
config/stack.gotest/integration_stacks_test.go
🧠 Learnings (5)
📓 Common learnings
Learnt from: jkarkoszka
Repo: gruntwork-io/terragrunt PR: 5116
File: test/integration_test.go:2785-2795
Timestamp: 2025-11-18T09:54:06.008Z
Learning: In Terragrunt, `read_terragrunt_config_with_cache` intentionally has different semantics than `read_terragrunt_config`: it caches the first parse result (including context like `original_terragrunt_dir`) and returns that cached value on subsequent calls to the same file, even if the calling context has changed. This is a performance optimization that trades context accuracy for speed. Tests showing different assertions between cached and non-cached variants for context-dependent values like `original_terragrunt_dir` are expected and correct.
📚 Learning: 2025-11-18T09:54:06.008Z
Learnt from: jkarkoszka
Repo: gruntwork-io/terragrunt PR: 5116
File: test/integration_test.go:2785-2795
Timestamp: 2025-11-18T09:54:06.008Z
Learning: In Terragrunt, `read_terragrunt_config_with_cache` intentionally has different semantics than `read_terragrunt_config`: it caches the first parse result (including context like `original_terragrunt_dir`) and returns that cached value on subsequent calls to the same file, even if the calling context has changed. This is a performance optimization that trades context accuracy for speed. Tests showing different assertions between cached and non-cached variants for context-dependent values like `original_terragrunt_dir` are expected and correct.
Applied to files:
test/fixtures/stacks/get-original-terragrunt-dir/live/account1/non-nested/with-locals/terragrunt.stack.hcltest/fixtures/stacks/get-original-terragrunt-dir/live/account1/with-locals-nested/with-locals/terragrunt.stack.hcltest/fixtures/stacks/get-original-terragrunt-dir/live/account1/no-locals-nested/read-config/terragrunt.stack.hcltest/fixtures/stacks/get-original-terragrunt-dir/live/account1/non-nested/read-config/terragrunt.stack.hcltest/fixtures/stacks/get-original-terragrunt-dir/stacks/with-locals/terragrunt.stack.hcltest/fixtures/stacks/get-original-terragrunt-dir/live/account1/read-config-nested/with-locals/terragrunt.stack.hcltest/fixtures/stacks/get-original-terragrunt-dir/live/account1/read-config-nested/read-config/terragrunt.stack.hcltest/fixtures/stacks/get-original-terragrunt-dir/live/account1/with-locals-nested/read-config/terragrunt.stack.hcltest/fixtures/stacks/get-original-terragrunt-dir/stacks/no-locals/terragrunt.stack.hcltest/fixtures/stacks/get-original-terragrunt-dir/stacks/read-config/terragrunt.stack.hcltest/integration_stacks_test.go
📚 Learning: 2025-02-10T13:36:19.542Z
Learnt from: levkohimins
Repo: gruntwork-io/terragrunt PR: 3723
File: cli/commands/stack/action.go:160-160
Timestamp: 2025-02-10T13:36:19.542Z
Learning: The project uses a custom error package `github.com/gruntwork-io/terragrunt/internal/errors` which provides similar functionality to `fmt.Errorf` but includes stack traces. Prefer using this package's error functions (e.g., `errors.Errorf`, `errors.New`) over the standard library's error handling.
Applied to files:
config/stack.go
📚 Learning: 2025-11-03T04:40:01.000Z
Learnt from: ThisGuyCodes
Repo: gruntwork-io/terragrunt PR: 5041
File: test/fixtures/hclvalidate/valid/duplicate-attributes-in-required-providers/main.tf:2-7
Timestamp: 2025-11-03T04:40:01.000Z
Learning: In the terragrunt repository, test fixtures under test/fixtures/hclvalidate/valid/ are intentionally testing that INPUT validation succeeds even when Terraform code contains syntax errors or other issues unrelated to input validation (e.g., duplicate attributes, circular references, invalid locals). The "valid" designation means "valid for input validation purposes" not "syntactically valid Terraform/OpenTofu code".
Applied to files:
test/fixtures/stacks/get-original-terragrunt-dir/live/account1/non-nested/read-config/terragrunt.stack.hcltest/fixtures/stacks/get-original-terragrunt-dir/stacks/no-locals/terragrunt.stack.hcltest/fixtures/stacks/get-original-terragrunt-dir/units/terragrunt.hcltest/fixtures/stacks/get-original-terragrunt-dir/stacks/read-config/terragrunt.stack.hcltest/integration_stacks_test.go
📚 Learning: 2025-02-10T23:20:04.295Z
Learnt from: yhakbar
Repo: gruntwork-io/terragrunt PR: 3868
File: docs-starlight/patches/@astrojs%[email protected]:33-33
Timestamp: 2025-02-10T23:20:04.295Z
Learning: In Terragrunt projects, all `.hcl` files can be assumed to be Terragrunt configurations by default, with specific exceptions like `.terraform.lock.hcl` that need explicit handling.
Applied to files:
test/fixtures/stacks/get-original-terragrunt-dir/live/account1/non-nested/read-config/terragrunt.stack.hcltest/fixtures/stacks/get-original-terragrunt-dir/stacks/no-locals/terragrunt.stack.hcltest/fixtures/stacks/get-original-terragrunt-dir/stacks/read-config/terragrunt.stack.hcl
🔇 Additional comments (20)
test/fixtures/stacks/get-original-terragrunt-dir/live/account1/with-locals-nested/no-locals/terragrunt.stack.hcl (1)
1-4: Test fixture correctly supports nested stack scenario forget_original_terragrunt_dirvalidation.This fixture appropriately tests the case where a nested stack (sourcing parent configuration via
find_in_parent_folders) is evaluated without locals, ensuringget_original_terragrunt_dircorrectly resolves the stack directory in this context.test/fixtures/stacks/get-original-terragrunt-dir/live/account1/read-config-nested/no-locals/terragrunt.stack.hcl (1)
1-4: Nested read-config stack fixture looks correctThis minimally wraps the parent
stacks/read-configstack atunit_dirs, which matches the intended nested/no-locals scenario for the new tests.test/fixtures/stacks/get-original-terragrunt-dir/live/account1/no-locals-nested/no-locals/terragrunt.stack.hcl (1)
1-8: Good coverage of nested no-locals + direct original dir usagePassing
stack_dir = get_original_terragrunt_dir()viavaluesis a clear way to assert the fixed behavior for nested/no-locals stacks.test/fixtures/stacks/get-original-terragrunt-dir/live/account1/non-nested/with-locals/terragrunt.stack.hcl (1)
1-12: Non-nested with-locals fixture is well-structuredCapturing
stack_dironce inlocalsand passing it tounit_1viavaluesis a clean pattern and gives a straightforward assertion point for the fixedget_original_terragrunt_dir.test/fixtures/stacks/get-original-terragrunt-dir/live/common/stack_config.hcl (1)
1-3: Common stack_config.hcl correctly centralizes original dirDefining
locals.stack_dir = get_original_terragrunt_dir()here is a good anchor for scenarios thatread_terragrunt_configinto this file and then propagatestack_diroutward, matching the intended context-dependent behavior while leavingread_terragrunt_config_with_cachesemantics untouched. Based on learnings, this aligns with the expected difference between cached and non-cached reads.test/fixtures/stacks/get-original-terragrunt-dir/units/terragrunt.hcl (1)
1-3: Unit fixture cleanly consumes propagated stack_dirReading
stack_direxclusively fromvalues.stack_dirkeeps the “where did this stack come from?” concern in the stack layer, which is exactly what these tests need.test/fixtures/stacks/get-original-terragrunt-dir/live/account1/non-nested/no-locals/terragrunt.stack.hcl (1)
1-8: Non-nested no-locals fixture nicely complements with-locals testsUsing
values.stack_dir = get_original_terragrunt_dir()here exercises the “simple, single stackfile” case without locals, rounding out the matrix of scenarios.test/fixtures/stacks/get-original-terragrunt-dir/live/account1/no-locals-nested/with-locals/terragrunt.stack.hcl (1)
1-12: Nested with-locals fixture covers the remaining matrix cornerCapturing
stack_dirin locals and passing it into a nestedstacks/no-localssource viavaluesgives good coverage of the “nested + locals” case, complementary to the no-locals variant.config/stack.go (1)
399-412: Correctly scoping OriginalTerragruntConfigPath to each stackfileCloning
optsand setting bothTerragruntConfigPathandOriginalTerragruntConfigPathtofilePathforstackOptsmeans that stack locals and anyread_terragrunt_configcalls from stackfiles will now treat the stackfile’s directory as the “original” dir, which matches the bug report and the new fixtures, while leaving the outeroptsunchanged for downstream logic. This looks like the right minimal fix.test/fixtures/stacks/get-original-terragrunt-dir/stacks/with-locals/terragrunt.stack.hcl (1)
1-21: LGTM! Test fixture correctly validatesget_original_terragrunt_dir()in locals.The fixture appropriately captures the original Terragrunt directory in locals and propagates it to units via the values map. The defensive
try(values, {})pattern ensures graceful handling of undefined values.test/fixtures/stacks/get-original-terragrunt-dir/live/account1/with-locals-nested/with-locals/terragrunt.stack.hcl (1)
1-12: LGTM! Nested stack fixture is correctly structured.This fixture appropriately tests the behavior of
get_original_terragrunt_dir()in nested stack scenarios by capturing the directory in locals and passing it to the child stack.test/fixtures/stacks/get-original-terragrunt-dir/live/account1/with-locals-nested/read-config/terragrunt.stack.hcl (1)
1-12: LGTM! Read-config fixture correctly validates nested behavior.This fixture appropriately tests the behavior when
get_original_terragrunt_dir()is used within a config loaded viaread_terragrunt_config(), ensuring the original directory context is preserved.test/fixtures/stacks/get-original-terragrunt-dir/live/account1/non-nested/read-config/terragrunt.stack.hcl (1)
1-12: LGTM! Non-nested read-config fixture is correct.This fixture appropriately tests the non-nested scenario with
read_terragrunt_config(), ensuring proper directory resolution in simpler stack configurations.test/fixtures/stacks/get-original-terragrunt-dir/stacks/no-locals/terragrunt.stack.hcl (1)
1-17: LGTM! No-locals fixture correctly tests passthrough behavior.This fixture appropriately tests the scenario where
get_original_terragrunt_dir()is not called in the stack's locals block, validating the expected behavior when the function is invoked later in the evaluation chain.test/fixtures/stacks/get-original-terragrunt-dir/live/account1/read-config-nested/read-config/terragrunt.stack.hcl (1)
1-12: LGTM! Nested read-config fixture validates combined scenario.This fixture correctly tests the combination of nested stacks with
read_terragrunt_config(), ensuring proper directory context propagation through multiple levels.test/fixtures/stacks/get-original-terragrunt-dir/live/account1/read-config-nested/with-locals/terragrunt.stack.hcl (1)
1-12: LGTM! Final nested fixture completes test coverage.This fixture appropriately rounds out the test coverage by combining nested stacks with local directory capture, ensuring comprehensive validation of the fix.
test/integration_stacks_test.go (2)
57-57: LGTM! Test fixture constant follows naming convention.The constant name is consistent with other test fixture constants in the file.
1443-1476: LGTM! Test setup and file discovery logic is correct.The test appropriately sets up the environment, generates the stack, and walks the directory tree to collect all
terragrunt.values.hclfiles for validation.test/fixtures/stacks/get-original-terragrunt-dir/live/account1/no-locals-nested/read-config/terragrunt.stack.hcl (1)
1-12: Verify referenced shared config file defines expectedstack_dirlocal.This fixture exercises the PR fix by reading a shared config and extracting
stack_dirfrom its locals (line 10). For this pattern to work,common/stack_config.hclmust definelocals.stack_dirusingget_original_terragrunt_dir(), so that when read from this stack file context, it returns the correct stack directory.test/fixtures/stacks/get-original-terragrunt-dir/stacks/read-config/terragrunt.stack.hcl (1)
1-21: Verify that shared config and unit directory dependencies are present in the fixture tree.This fixture reads shared config from
live/common/stack_config.hcland propagatesstack_dirto nested units viamerge()withtry(values, {})(lines 9, 18). Ensure the following fixture dependencies exist:
live/common/stack_config.hcldefininglocals.stack_dirviaget_original_terragrunt_dir()unitsdirectory containingunit_1andunit_2subdirectories at the relative path specified in thesourceattributes
… reflect behaviour during stacks generate

Description
Fixes #5175.
While each stackfile is being evaluated, set the
OriginalTerragruntConfigPathto that filepath, allowingget_original_terragrunt_dirto function similarly forterragrunt.stack.hclfiles as it does forterragrunt.hclfiles.Each scenario below
terragrunt stack generateis run in the live directory.Scenario 1
Before:
stack_dir=liveAfter:
stack_dir=parentScenario 2
Before:
stack_dir=liveAfter:
stack_dir=parentScenario 3
Before:
stack_dir=liveAfter:
stack_dir=parentTODOs
Read the Gruntwork contribution guidelines.
Release Notes (draft)
During multi-directory
terragrunt stack generate,get_original_terragrunt_dirreturns the expected path when called from stackfile locals or from within aread_terragrunt_configtarget.Migration Guide
Summary by CodeRabbit
New Features
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.