fix: Cleaning up #5232 - #6009
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThe changes fix configuration inheritance behavior for exclude blocks in HCL configuration files. When including parent configurations, exclude rules are now properly preserved instead of being overwritten by nil values when the child config doesn't explicitly define its own exclude block. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms Review rate limit: 4/5 reviews remaining, refill in 12 minutes. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/config/exclude_include_test.go`:
- Around line 59-60: Test fixtures embed parentPath into quoted HCL via
expressions like "path = \"`+parentPath+`\"" which breaks on Windows because
backslashes are treated as escapes; update each test block that interpolates
parentPath (e.g., where parentPath and tt.includeBody are concatenated into HCL
fixtures) to call filepath.ToSlash(parentPath) before embedding so the HCL
strings use forward slashes; ensure all occurrences mentioned in the comment
(the five test blocks) are updated consistently.
🪄 Autofix (Beta)
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
Run ID: 06610abe-58ce-4522-9984-aaade1c36534
📒 Files selected for processing (3)
docs/src/data/changelog/v1.0.4/exclude-include-inheritance.mdxpkg/config/config.gopkg/config/exclude_include_test.go
Comment on lines
+59
to
+60
| path = "`+parentPath+`" | ||
| `+tt.includeBody+` |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'path = "`\+parentPath\+`"' pkg/config/exclude_include_test.go
rg -n 'ToSlash\(parentPath\)' pkg/config/exclude_include_test.goRepository: gruntwork-io/terragrunt
Length of output: 216
🏁 Script executed:
# View the actual test code sections with interpolations
sed -n '55,65p' pkg/config/exclude_include_test.go
sed -n '120,130p' pkg/config/exclude_include_test.go
sed -n '190,200p' pkg/config/exclude_include_test.go
# Check if filepath.ToSlash is used elsewhere in the codebase
rg 'filepath\.ToSlash' --type go
rg 'ToSlash.*path' --type go
# Check for backslash handling patterns in Go files
rg '\\\\' pkg/config/exclude_include_test.goRepository: gruntwork-io/terragrunt
Length of output: 3450
🏁 Script executed:
# Check context around the config_helpers.go comment about filepath.ToSlash
sed -n '1,50p' pkg/config/config_helpers.go | tail -30
# Look for any Windows-specific test handling or bug reports
rg -i 'windows|backslash|escape.*path' pkg/config/exclude_include_test.go
# Check if this test file has any platform-specific build tags
head -20 pkg/config/exclude_include_test.go
# Search for HCL parsing logic to understand escape handling
rg 'hcl.*parse|hclparse' --type go -A 3 | head -40
# Check if there are any existing issues or TODOs about path escaping
rg -i 'TODO|FIXME|BUG.*path|path.*escape' pkg/config/exclude_include_test.goRepository: gruntwork-io/terragrunt
Length of output: 4416
🏁 Script executed:
# Look at the full test context to understand what's being tested
sed -n '38,70p' pkg/config/exclude_include_test.go
# Check the hclparse package to understand how it handles paths
find . -name "hclparse*" -type f | grep -E '\.(go|mod)$'
cat pkg/config/hclparse/parser.go | head -50
# Verify how the include path is actually used in config parsing
rg -A 5 'include.*path.*parentPath' pkg/config/exclude_include_test.go | head -20
# Check if there are any Windows-specific test failures or known issues
rg -i 'windows|os\.sep' pkg/config/exclude_include_test.goRepository: gruntwork-io/terragrunt
Length of output: 2360
🏁 Script executed:
# Check if this test file has been run on Windows or if there are any Windows-specific issues
rg -i 'windows|skip.*windows|build.*windows' pkg/config/exclude_include_test.go
# Look at how HCL2 parser handles escape sequences in strings
# Check if there's any documentation or known issues
rg -i 'escape|backslash' pkg/config/hclparse/
# Check the actual go-cty/hcl/v2 parsing behavior by looking at similar cases in the codebase
rg -B 3 -A 3 'filepath.ToSlash.*hcl|include.*path.*ToSlash' --type go | head -30
# Verify if the include path parsing actually uses the string as-is or processes it
sed -n '500,600p' pkg/config/config.go | grep -A 10 -B 10 'include.*path'Repository: gruntwork-io/terragrunt
Length of output: 456
🏁 Script executed:
# Check if tests have Windows build tags or are skipped on Windows
head -30 pkg/config/exclude_include_test.go | grep -i "build\|skip\|windows"
# Check what the actual parsed result looks like by examining the ParseConfigFile function
rg -A 10 'func ParseConfigFile' pkg/config/ | head -30
# Look for any existing issues or PRs related to Windows path handling
rg -i 'issue.*path|github.*windows.*path' pkg/config/ | head -10
# Check if filepath.ToSlash is conditionally applied or always applied in similar contexts
rg -B 5 'filepath.ToSlash.*path' pkg/config/config_helpers.go | head -20Repository: gruntwork-io/terragrunt
Length of output: 539
🏁 Script executed:
# Check how the include path attribute is processed after HCL parsing
rg -A 20 'include.*config.*path' pkg/config/config.go | head -40
# Look for the actual attribute parsing of include paths
rg -B 5 -A 10 'func.*parseInclude|processInclude' pkg/config/ --type go | head -50
# Check if there's any string unquoting or processing of the path value
rg 'unquote|quote.*path|path.*quote' pkg/config/ --type go
# Verify if the issue is actually triggered in practice - check for any test failures or issues
find . -name "*.md" -type f -exec grep -l "Windows.*path\|path.*Windows" {} \; | head -5Repository: gruntwork-io/terragrunt
Length of output: 1504
🏁 Script executed:
# Check if the tests are platform-specific or have skip conditions
grep -n "skip\|build.*windows\|GOOS" pkg/config/exclude_include_test.go
# Look at how HCL2 library handles string escaping
# Check if there are comments in the code about this
rg -i 'hcl.*escape|escape.*hcl|string.*escape' pkg/config/ -A 2 | head -20
# Check if the issue has been reported in the codebase
rg 'windows.*path|path.*windows|backslash.*hcl' -i . --type md --type go | head -10
# Try to understand the actual behavior by looking at a real example
# Find if there are any error messages or tests that validate path handling
rg 'path.*escape|escape.*path' --type go | head -10
# Check if there's any documentation on how HCL2 handles paths
rg 'hashicorp/hcl' pkg/config/hclparse/ -A 5 | head -20Repository: gruntwork-io/terragrunt
Length of output: 3277
🏁 Script executed:
# Look at the full context of the config_helpers.go comment about filepath.ToSlash and escapes
sed -n '200,250p' pkg/config/config_helpers.go
# Check the specific issue about HCL2 parsing with backslashes
# Look for how include paths are actually resolved
sed -n '1,100p' pkg/config/include.go
# Verify if the parsed include path value is used as a filesystem path or as a string
rg -A 15 'includePath :=' pkg/config/include.go | head -25
# Check if there are any test failures or issues reported about Windows
find . -name "*.go" -type f -exec grep -l 'exclude_include\|exclude.*include' {} \; | xargs grep -l windows
# Check the actual HCL2 specification or how paths are handled
rg 'path.*=.*"' pkg/config/exclude_include_test.go | head -5Repository: gruntwork-io/terragrunt
Length of output: 8131
🏁 Script executed:
# Check if the test has any build constraints that skip it on Windows
head -40 pkg/config/exclude_include_test.go | grep -E "^//go:build|^// +build"
# Check if the tests actually run on Windows in CI
find . -name "*.yml" -o -name "*.yaml" | xargs grep -l "windows" | head -5
# Look at how the include path is actually parsed by the HCL parser
# The key is whether the string value is unquoted and processed
rg -B 10 -A 10 'DecodeAsString.*includePath|getStringAttributeValue.*include' pkg/config/ --type go | head -50
# Check the actual attribute decoding logic
rg 'DecodeAsString|getAttribute.*include' pkg/config/ -B 3 -A 3 | head -40Repository: gruntwork-io/terragrunt
Length of output: 260
🏁 Script executed:
# Check the integration test workflow to see if Windows is tested
grep -A 20 "windows" ./.github/workflows/integration-test.yml | head -30
# Check if the exclude_include_test is included in Windows tests
grep -i "exclude_include" ./.github/workflows/*.yml
# Look at the actual include attribute parsing code
rg -B 5 -A 15 'func.*decodeInclude|func.*parseInclude' pkg/config/ --type go | head -60
# Check how string attributes are decoded from HCL
rg -B 3 -A 10 'getStringAttribute|DecodeAsString' pkg/config/include.go | head -40Repository: gruntwork-io/terragrunt
Length of output: 2656
🏁 Script executed:
# Check if the exclude_include_test matches the Windows test pattern in CI
grep -E "TestParseConfig.*Exclude|TestParseConfig.*Include" pkg/config/exclude_include_test.go | head -5
# Check what the Windows test pattern actually matches
grep -B 5 -A 5 "TestWindows" .github/workflows/integration-test.yml | head -20
# Look for any HCL string escape handling or unquoting logic
rg 'unquote|strconv.*Unquote|cty.*String' pkg/config/hclparse/ --type go -B 2 -A 5 | head -50
# Check if there's any decoding of HCL attributes that handles path values
rg -A 20 'func.*decodeIncludeAttributes|func.*decodeIncludeBlock' pkg/config/ --type go | head -60Repository: gruntwork-io/terragrunt
Length of output: 820
🏁 Script executed:
# The key test functions don't start with "TestWindows", so they don't run on Windows CI
# Let's verify this by checking what pattern Windows tests actually use
grep -i "TestWindows\|windows" pkg/config/exclude_include_test.go
# Check if there's a general parsing test or config test that runs on Windows
find pkg/config -name "*test.go" -type f -exec grep -l "TestWindows" {} \;
# Now let's understand how HCL2 actually handles quoted strings with backslashes
# by looking at real usage patterns in the codebase
rg -B 3 -A 3 'include.*path.*=' pkg/config/config.go | head -30
# Check if the include path string is actually unquoted by the HCL parser
# Look for how cty values are converted to strings
rg 'asString|cty.*String|gocty.*FromCtyValue' pkg/config/ --type go | head -20Repository: gruntwork-io/terragrunt
Length of output: 1920
🏁 Script executed:
# Find where the include block's path attribute is actually decoded
rg -B 5 -A 15 'decodeIncludeBlock\|parseIncludeAttribute' pkg/config/ --type go | head -80
# Look for the actual function that extracts the path value from HCL attributes
rg 'func.*Include.*decode\|func.*decodeInclude' pkg/config/ --type go -A 20 | head -60
# Check how attributes are extracted from HCL in this file
rg -B 2 -A 10 'includeBlock\|include.*block' pkg/config/include.go --type go | head -50Repository: gruntwork-io/terragrunt
Length of output: 49
🏁 Script executed:
# Find the actual include block parsing in config.go
sed -n '1,200p' pkg/config/config.go | grep -A 30 -B 5 'include'
# Look specifically for how include attributes are decoded
rg -B 10 -A 20 'func decodeIncludeAttribute\|decodeIncludeBlock\|parseIncludeBlock' pkg/config/ --type go
# Check the actual include.go file for attribute parsing
head -200 pkg/config/include.go | tail -100
# Search for where the path attribute value is actually extracted and used
rg 'includedConfig.Path\s*=' pkg/config/ --type go -B 3 -A 3 | head -40Repository: gruntwork-io/terragrunt
Length of output: 6388
🏁 Script executed:
# Look for where the include attributes are actually decoded from HCL
rg -B 5 -A 20 'gohcl.*Decode\|DecodeBody.*include' pkg/config/ --type go | head -80
# Check the IncludeConfig struct definition
rg -B 3 -A 10 'type IncludeConfig struct' pkg/config/ --type go
# Look for how the path attribute is decoded
rg -B 5 -A 15 'Path.*string\|ConfigPath.*string' pkg/config/ --type go | grep -A 10 -B 5 "IncludeConfig\|Include struct"
# Check the actual attribute decoding in config.go
sed -n '600,900p' pkg/config/config.go | grep -B 10 -A 10 -i 'include\|path'Repository: gruntwork-io/terragrunt
Length of output: 7710
🏁 Script executed:
# Look for where IncludeConfig is actually decoded from HCL
rg -B 10 -A 10 'gohcl.*Decode.*include\|Decode.*terragruntInclude' pkg/config/ --type go | head -60
# Check if there's any code that handles the include blocks decoding
rg -B 5 -A 15 'decodeLocalsAndInclude\|parseInclude' pkg/config/ --type go | head -100
# Look at the actual HCL2 gohcl package usage
rg 'gohcl.DecodeBody' pkg/config/ --type go -B 3 -A 5Repository: gruntwork-io/terragrunt
Length of output: 479
Escape interpolated include paths for Windows-safe HCL fixtures.
These fixtures embed parentPath directly into quoted HCL strings. On Windows, filepath.Join produces backslashes, which HCL2's string parser interprets as escape sequences, causing path mangling or parse errors. The pattern of using filepath.ToSlash() for similar path-in-HCL scenarios is established throughout the codebase.
💡 Proposed fix (apply to each test block)
parentPath := filepath.Join(tmpDir, "root.hcl")
+parentPathHCL := filepath.ToSlash(parentPath)
require.NoError(t, os.WriteFile(parentPath, []byte(`
exclude {
if = true
@@
include "root" {
- path = "`+parentPath+`"
+ path = "`+parentPathHCL+`"
`+tt.includeBody+`
}
`), 0644))Applies to lines: 59-60, 125-126, 195-196, 250-251, 304-305
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/config/exclude_include_test.go` around lines 59 - 60, Test fixtures embed
parentPath into quoted HCL via expressions like "path = \"`+parentPath+`\""
which breaks on Windows because backslashes are treated as escapes; update each
test block that interpolates parentPath (e.g., where parentPath and
tt.includeBody are concatenated into HCL fixtures) to call
filepath.ToSlash(parentPath) before embedding so the HCL strings use forward
slashes; ensure all occurrences mentioned in the comment (the five test blocks)
are updated consistently.
denis256
approved these changes
May 1, 2026
|
|
||
| // Child-defined exclude blocks must still take precedence over the included | ||
| // parent's exclude block, regardless of merge strategy. | ||
| func TestParseConfig_ChildExcludeOverridesIncludedConfig(t *testing.T) { |
Member
There was a problem hiding this comment.
Makes sense to add a test for child with exclude { if = false }
| @@ -0,0 +1,323 @@ | |||
| package config_test | |||
Member
There was a problem hiding this comment.
Missing test for empty exclude {} block on child, it may trigger different nil/zero errors
Rahul-Kumar-prog
pushed a commit
to Rahul-Kumar-prog/terragrunt
that referenced
this pull request
May 10, 2026
* fix(config): prevent overwriting Exclude when not set in current config (gruntwork-io#5089) * fix: Adding some testing that wasn't addressed in gruntwork-io#5232 * fix: Increasing coverage of other included blocks * docs: Documenting exclude fix in changelog --------- Co-authored-by: HeikoNeblung <Heiko.Neblung@telekom.de>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #5089.
Closes #5232.
The implementation in #5232 was correct, but it didn't include tests for the behavior being fixed, and it also didn't include coverage of other blocks that might also be included from separate config.
This pulls in the fix from #5232 and adds tests for the behavior being fixed.
TODOs
Read the Gruntwork contribution guidelines.
Release Notes (draft)
Added / Removed / Updated [X].
Migration Guide
Summary by CodeRabbit
Bug Fixes
Tests