Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions config/config_partial.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,11 @@ func DecodeBaseBlocks(ctx *ParsingContext, l log.Logger, file *hclparse.File, in
errs = errs.Append(err)
}

// Validate that all include paths are unique
if err := ValidateUniqueIncludePaths(terragruntIncludeList); err != nil {
errs = errs.Append(err)
}

trackInclude, err := getTrackInclude(ctx, terragruntIncludeList, includeFromChild)
if err != nil {
errs = errs.Append(err)
Expand Down Expand Up @@ -458,6 +463,11 @@ func PartialParseConfig(ctx *ParsingContext, l log.Logger, file *hclparse.File,
// In normal operation, if a dependency block does not have a `config_path` attribute, decoding returns an error since this attribute is required, but the `hclvalidate` command suppresses decoding errors and this causes a cycle between modules, so we need to filter out dependencies without a defined `config_path`.
decoded.Dependencies = decoded.Dependencies.FilteredWithoutConfigPath()

// Validate that dependency config paths are unique
if err := ValidateUniqueConfigPaths(decoded.Dependencies); err != nil {
return nil, err
}

output.TerragruntDependencies = decoded.Dependencies
// Convert dependency blocks into module dependency lists. If we already decoded some dependencies,
// merge them in. Otherwise, set as the new list.
Expand Down Expand Up @@ -746,3 +756,26 @@ type InvalidPartialBlockName struct {
func (err InvalidPartialBlockName) Error() string {
return fmt.Sprintf("Unrecognized partial block code %d. This is most likely an error in terragrunt. Please file a bug report on the project repository.", err.sectionCode)
}

// ValidateUniqueIncludePaths ensures that all include paths are unique within a configuration.
func ValidateUniqueIncludePaths(includes IncludeConfigs) error {
includePaths := make(map[string]string) // map[path]include_name

for _, inc := range includes {
if inc.Path == "" {
// Skip includes without path (shouldn't happen but be defensive)
continue
}

if existingIncludeName, exists := includePaths[inc.Path]; exists {
return fmt.Errorf("duplicate include path '%s' found in include blocks. "+
"Include '%s' and include '%s' both point to the same path. "+
"Each include must have a unique path",
inc.Path, existingIncludeName, inc.Name)
}

includePaths[inc.Path] = inc.Name
}

return nil
}
56 changes: 56 additions & 0 deletions config/config_partial_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,3 +388,59 @@ dependency "ec2" {
require.NoError(t, err)
assert.Len(t, terragruntConfig.Dependencies.Paths, 1)
}

func TestValidateUniqueIncludePaths(t *testing.T) {
t.Parallel()

testCases := []struct {
name string
expectedErr string
includes config.IncludeConfigs
expectError bool
}{
{
name: "unique include paths",
includes: config.IncludeConfigs{
{Name: "root", Path: "../../terragrunt.hcl"},
{Name: "common", Path: "../common.hcl"},
},
expectError: false,
},
{
name: "single include",
includes: config.IncludeConfigs{
{Name: "root", Path: "../../terragrunt.hcl"},
},
expectError: false,
},
{
name: "no includes",
includes: config.IncludeConfigs{},
expectError: false,
},
{
name: "duplicate include paths",
includes: config.IncludeConfigs{
{Name: "root", Path: "../../terragrunt.hcl"},
{Name: "backup", Path: "../../terragrunt.hcl"}, // Same path as root
},
expectError: true,
expectedErr: "duplicate include path '../../terragrunt.hcl' found in include blocks. Include 'root' and include 'backup' both point to the same path",
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

err := config.ValidateUniqueIncludePaths(tc.includes)

if tc.expectError {
require.Error(t, err)
assert.Contains(t, err.Error(), tc.expectedErr)
} else {
assert.NoError(t, err)
}
})
}
}
29 changes: 29 additions & 0 deletions config/dependency.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,30 @@ func (dep *Dependency) DeepMerge(sourceDepConfig Dependency) error {
return nil
}

// ValidateUniqueConfigPaths ensures that all dependency config_path values are unique within a configuration.
func ValidateUniqueConfigPaths(dependencies Dependencies) error {
configPaths := make(map[string]string) // map[config_path]dependency_name

for _, dep := range dependencies {
configPath := dep.ConfigPath.AsString()
if configPath == "" {
// Skip dependencies without config_path (filtered out elsewhere)
continue
}

if existingDepName, exists := configPaths[configPath]; exists {
return fmt.Errorf("duplicate config_path '%s' found in dependency blocks. "+
"Dependency '%s' and dependency '%s' both point to the same config path. "+
"Each dependency must have a unique config_path",
configPath, existingDepName, dep.Name)
}

configPaths[configPath] = dep.Name
}

return nil
}

// getMockOutputsMergeStrategy returns the MergeStrategyType following the deprecation of mock_outputs_merge_with_state
// - If mock_outputs_merge_strategy_with_state is not null. The value of mock_outputs_merge_strategy_with_state will be returned
// - If mock_outputs_merge_strategy_with_state is null and mock_outputs_merge_with_state is not null:
Expand Down Expand Up @@ -210,6 +234,11 @@ func decodeAndRetrieveOutputs(ctx *ParsingContext, l log.Logger, file *hclparse.
// In normal operation, if a dependency block does not have a `config_path` attribute, decoding returns an error since this attribute is required, but the `hclvalidate` command suppresses decoding errors and this causes a cycle between modules, so we need to filter out dependencies without a defined `config_path`.
decodedDependency.Dependencies = decodedDependency.Dependencies.FilteredWithoutConfigPath()

// Validate that dependency config paths are unique
if err := ValidateUniqueConfigPaths(decodedDependency.Dependencies); err != nil {
return nil, err
}

Comment on lines +237 to +241
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Validate duplicates after include-merging as well (verify wiring)

Great to validate before cycle detection. Ensure that duplicates introduced by include merging are also validated post-merge (the summary notes include.go does this). Let’s verify.


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Searching for post-merge validations in config/include.go..."
rg -nP -C3 'MergeDependencyBlocks|ValidateUniqueConfigPaths\s*\(' config/include.go || true

echo
echo "Other uses of ValidateUniqueConfigPaths in the codebase:"
rg -nP -C2 '\bValidateUniqueConfigPaths\s*\(' config | sed -n '1,200p'

Length of output: 5704


Ensure duplicate-path validation runs after every include-merge strategy

While we already call ValidateUniqueConfigPaths for shallow merges in config/include.go, the DeepMerge branches never re-validate post-merge. Please add duplicate-path checks in both DeepMerge sites:

• In the per-include DeepMerge case (around line 222 in config/include.go):

  mergedDependencyBlock, err := deepMergeDependencyBlocks(...)
  if err != nil {
      return nil, err
  }
+ // Validate that merged dependencies have unique config paths
+ if err := ValidateUniqueConfigPaths(mergedDependencyBlock); err != nil {
+     return nil, err
+ }

• In the final deep-merge at the bottom of config/include.go (around line 486):

  mergedDeps, err := deepMergeDependencyBlocks(...)
  if err != nil {
      return err
  }
+ // Validate that merged dependencies have unique config paths
+ if err := ValidateUniqueConfigPaths(mergedDeps); err != nil {
+     return err
+ }

Also extend config/include_test.go with a DeepMerge test to verify that duplicates are caught.

🤖 Prompt for AI Agents
In config/dependency.go around lines 237 to 241, add a call to
ValidateUniqueConfigPaths after any deep-merge of included configs so
duplicate-path validation runs post-merge (mirror the existing shallow-merge
check); also update config/include.go: insert ValidateUniqueConfigPaths call in
the per-include DeepMerge branch near line 222 and again after the final
deep-merge near line 486 to re-validate the merged result; finally add a unit
test in config/include_test.go that performs a DeepMerge scenario with duplicate
paths and asserts that ValidateUniqueConfigPaths returns an error.

if err := checkForDependencyBlockCycles(ctx, l, file.ConfigPath, decodedDependency); err != nil {
return nil, err
}
Expand Down
116 changes: 116 additions & 0 deletions config/dependency_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,3 +150,119 @@ dependency "vpc" {
require.NoError(t, file.Decode(&decoded, &hcl.EvalContext{}))
assert.Len(t, decoded.Dependencies, 2)
}

func TestValidateUniqueConfigPaths_Success(t *testing.T) {
t.Parallel()

tests := []struct {
name string
hclCode string
}{
{
name: "unique config paths",
hclCode: `
dependency "vpc" {
config_path = "../vpc"
}

dependency "database" {
config_path = "../database"
}
`,
},
{
name: "single dependency",
hclCode: `
dependency "vpc" {
config_path = "../vpc"
}
`,
},
{
name: "no dependencies",
hclCode: `
locals {
vpc_id = "vpc-123"
}
`,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
t.Parallel()

filename := config.DefaultTerragruntConfigPath
file, err := hclparse.NewParser().ParseFromString(test.hclCode, filename)
require.NoError(t, err)

decoded := config.TerragruntDependency{}
err = file.Decode(&decoded, &hcl.EvalContext{})
require.NoError(t, err)

// Also test the validation function directly
err = config.ValidateUniqueConfigPaths(decoded.Dependencies)
require.NoError(t, err)
})
}
Comment on lines +154 to +207
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Fix parallel subtests capturing loop variable to avoid flakiness

t.Parallel is used inside subtests that close over the loop variable test. This can cause data races/flaky tests. Copy the loop variable before invoking t.Run.

Apply this diff:

-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
+	for _, test := range tests {
+		tc := test
+		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
 			filename := config.DefaultTerragruntConfigPath
-			file, err := hclparse.NewParser().ParseFromString(test.hclCode, filename)
+			file, err := hclparse.NewParser().ParseFromString(tc.hclCode, filename)
 			require.NoError(t, err)
 
 			decoded := config.TerragruntDependency{}
-			err = file.Decode(&decoded, &hcl.EvalContext{})
+			err = file.Decode(&decoded, &hcl.EvalContext{})
 			require.NoError(t, err)
 
 			// Also test the validation function directly
-			err = config.ValidateUniqueConfigPaths(decoded.Dependencies)
+			err = config.ValidateUniqueConfigPaths(decoded.Dependencies)
 			require.NoError(t, err)
 		})
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func TestValidateUniqueConfigPaths_Success(t *testing.T) {
t.Parallel()
tests := []struct {
name string
hclCode string
}{
{
name: "unique config paths",
hclCode: `
dependency "vpc" {
config_path = "../vpc"
}
dependency "database" {
config_path = "../database"
}
`,
},
{
name: "single dependency",
hclCode: `
dependency "vpc" {
config_path = "../vpc"
}
`,
},
{
name: "no dependencies",
hclCode: `
locals {
vpc_id = "vpc-123"
}
`,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
t.Parallel()
filename := config.DefaultTerragruntConfigPath
file, err := hclparse.NewParser().ParseFromString(test.hclCode, filename)
require.NoError(t, err)
decoded := config.TerragruntDependency{}
err = file.Decode(&decoded, &hcl.EvalContext{})
require.NoError(t, err)
// Also test the validation function directly
err = config.ValidateUniqueConfigPaths(decoded.Dependencies)
require.NoError(t, err)
})
}
func TestValidateUniqueConfigPaths_Success(t *testing.T) {
t.Parallel()
tests := []struct {
name string
hclCode string
}{
{
name: "unique config paths",
hclCode: `
dependency "vpc" {
config_path = "../vpc"
}
dependency "database" {
config_path = "../database"
}
`,
},
{
name: "single dependency",
hclCode: `
dependency "vpc" {
config_path = "../vpc"
}
`,
},
{
name: "no dependencies",
hclCode: `
locals {
vpc_id = "vpc-123"
}
`,
},
}
for _, test := range tests {
tc := test
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
filename := config.DefaultTerragruntConfigPath
file, err := hclparse.NewParser().ParseFromString(tc.hclCode, filename)
require.NoError(t, err)
decoded := config.TerragruntDependency{}
err = file.Decode(&decoded, &hcl.EvalContext{})
require.NoError(t, err)
// Also test the validation function directly
err = config.ValidateUniqueConfigPaths(decoded.Dependencies)
require.NoError(t, err)
})
}
}
🤖 Prompt for AI Agents
In config/dependency_test.go around lines 154 to 207, the subtests use t.Run
with a closure that captures the loop variable `test`, causing potential
flakiness when running tests in parallel; to fix it, create a new local variable
inside the loop (e.g., `tc := test`) and use `tc` inside the t.Run closure (and
its inner t.Parallel call) so each goroutine gets its own copy of the test case,
preventing data races and nondeterministic behavior.

}

func TestValidateUniqueConfigPaths_Failure(t *testing.T) {
t.Parallel()

tests := []struct {
name string
hclCode string
expectedErr string
}{
{
name: "duplicate config paths",
hclCode: `
dependency "vpc" {
config_path = "../vpc"
}

dependency "network" {
config_path = "../vpc"
}
`,
expectedErr: "duplicate config_path '../vpc' found in dependency blocks. Dependency 'vpc' and dependency 'network' both point to the same config path",
},
{
name: "multiple duplicates",
hclCode: `
dependency "app1" {
config_path = "../app"
}

dependency "app2" {
config_path = "../app"
}

dependency "app3" {
config_path = "../app"
}
`,
expectedErr: "duplicate config_path '../app' found in dependency blocks. Dependency 'app1' and dependency 'app2' both point to the same config path",
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
t.Parallel()

filename := config.DefaultTerragruntConfigPath
file, err := hclparse.NewParser().ParseFromString(test.hclCode, filename)
require.NoError(t, err)

decoded := config.TerragruntDependency{}
err = file.Decode(&decoded, &hcl.EvalContext{})
require.NoError(t, err)

// Test the validation function directly
err = config.ValidateUniqueConfigPaths(decoded.Dependencies)
require.Error(t, err)
assert.Contains(t, err.Error(), test.expectedErr)
})
}
}
Comment on lines +210 to +268
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Same loop-variable capture issue in failure tests

Mirror the fix here to prevent subtests from racing on test.

Apply this diff:

-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
+	for _, test := range tests {
+		tc := test
+		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
 			filename := config.DefaultTerragruntConfigPath
-			file, err := hclparse.NewParser().ParseFromString(test.hclCode, filename)
+			file, err := hclparse.NewParser().ParseFromString(tc.hclCode, filename)
 			require.NoError(t, err)
 
 			decoded := config.TerragruntDependency{}
 			err = file.Decode(&decoded, &hcl.EvalContext{})
 			require.NoError(t, err)
 
 			// Test the validation function directly
 			err = config.ValidateUniqueConfigPaths(decoded.Dependencies)
 			require.Error(t, err)
-			assert.Contains(t, err.Error(), test.expectedErr)
+			assert.Contains(t, err.Error(), tc.expectedErr)
 		})
 	}
🤖 Prompt for AI Agents
In config/dependency_test.go around lines 210 to 268, the failure tests use the
loop variable 'test' directly inside t.Run with t.Parallel, which causes a
loop-variable capture race between subtests; fix by copying the loop item to a
new local variable before calling t.Run (e.g., for i := range tests { tc :=
tests[i]; t.Run(tc.name, func(t *testing.T) { t.Parallel(); ... use tc ... }) })
so each subtest closes over its own copy.

41 changes: 41 additions & 0 deletions config/hclparse/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/gohcl"
"github.com/hashicorp/hcl/v2/hclparse"
"github.com/hashicorp/hcl/v2/hclsyntax"
)

const (
Expand Down Expand Up @@ -65,6 +66,11 @@ func (file *File) Decode(out any, evalContext *hcl.EvalContext) (err error) {
}
}

// Validate for duplicate labeled blocks before decoding
if err := file.validateNoDuplicateBlocks(); err != nil {
return err
}

diags := gohcl.DecodeBody(file.Body, evalContext, out)
if err := file.HandleDiagnostics(diags); err != nil {
return errors.New(err)
Expand Down Expand Up @@ -129,3 +135,38 @@ func (file *File) JustAttributes() (Attributes, error) {
func (file *File) HandleDiagnostics(diags hcl.Diagnostics) error {
return file.Parser.handleDiagnostics(file, diags)
}

// validateNoDuplicateBlocks checks for duplicate labeled blocks in the HCL file.
func (file *File) validateNoDuplicateBlocks() error {
// Only validate syntax bodies, skip others
body, ok := file.Body.(*hclsyntax.Body)
if !ok {
return nil
}

// Track all block labels to prevent duplicates
allBlockLabels := make(map[string]*hcl.Block)

for _, syntaxBlock := range body.Blocks {
hclBlock := syntaxBlock.AsHCLBlock()

// Check for duplicate block labels (only for blocks with labels)
if len(hclBlock.Labels) > 0 {
blockKey := hclBlock.Type + "." + hclBlock.Labels[0]

if existingBlock, exists := allBlockLabels[blockKey]; exists {
return errors.New(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: fmt.Sprintf("Duplicate %s block", hclBlock.Type),
Detail: fmt.Sprintf("Duplicate %s block with label '%s' found. Previous block was defined at %s",
hclBlock.Type, hclBlock.Labels[0], existingBlock.DefRange.String()),
Subject: &hclBlock.DefRange,
})
}

allBlockLabels[blockKey] = hclBlock
}
}

return nil
}
Loading