diff --git a/config/config_partial.go b/config/config_partial.go index c237c102f5..17d7ebb3bd 100644 --- a/config/config_partial.go +++ b/config/config_partial.go @@ -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) @@ -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. @@ -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 +} diff --git a/config/config_partial_test.go b/config/config_partial_test.go index b5d18cf363..af601f409e 100644 --- a/config/config_partial_test.go +++ b/config/config_partial_test.go @@ -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) + } + }) + } +} diff --git a/config/dependency.go b/config/dependency.go index 5387d92c45..8625d40ced 100644 --- a/config/dependency.go +++ b/config/dependency.go @@ -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: @@ -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 + } + if err := checkForDependencyBlockCycles(ctx, l, file.ConfigPath, decodedDependency); err != nil { return nil, err } diff --git a/config/dependency_test.go b/config/dependency_test.go index d77e153dd6..294379a63f 100644 --- a/config/dependency_test.go +++ b/config/dependency_test.go @@ -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) + }) + } +} + +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) + }) + } +} diff --git a/config/hclparse/file.go b/config/hclparse/file.go index fed5b700f5..ef91e54543 100644 --- a/config/hclparse/file.go +++ b/config/hclparse/file.go @@ -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 ( @@ -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) @@ -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 +} diff --git a/config/hclparse/file_test.go b/config/hclparse/file_test.go new file mode 100644 index 0000000000..c1ce78a109 --- /dev/null +++ b/config/hclparse/file_test.go @@ -0,0 +1,103 @@ +package hclparse_test + +import ( + "testing" + + "github.com/gruntwork-io/terragrunt/config" + "github.com/gruntwork-io/terragrunt/config/hclparse" + "github.com/gruntwork-io/terragrunt/test/helpers/logger" + "github.com/hashicorp/hcl/v2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestValidateNoDuplicateBlocks(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + hclCode string + expectedErr string + expectError bool + }{ + { + name: "unique dependency blocks", + hclCode: ` +dependency "vpc" { + config_path = "../vpc" +} + +dependency "database" { + config_path = "../database" +} +`, + expectError: false, + }, + { + name: "single dependency block", + hclCode: ` +dependency "vpc" { + config_path = "../vpc" +} +`, + expectError: false, + }, + { + name: "empty file", + hclCode: ``, + expectError: false, + }, + { + name: "duplicate dependency blocks", + hclCode: ` +dependency "vpc" { + config_path = "../vpc" +} + +dependency "vpc" { + config_path = "../vpc2" +} +`, + expectError: true, + expectedErr: "Duplicate dependency block with label 'vpc' found", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + file, err := hclparse.NewParser().ParseFromString(test.hclCode, "test.hcl") + require.NoError(t, err) + + // Determine which struct to decode into based on content + if test.name == "empty file" { + var decoded struct{} + err = file.Decode(&decoded, &hcl.EvalContext{}) + } else { + var decoded config.TerragruntDependency + err = file.Decode(&decoded, &hcl.EvalContext{}) + } + + if test.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), test.expectedErr) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestValidateNoDuplicateBlocks_NonSyntaxBody(t *testing.T) { + t.Parallel() + + // Test that non-syntax bodies are handled gracefully + parser := hclparse.NewParser(hclparse.WithLogger(logger.CreateLogger())) + file, err := parser.ParseFromString("", "empty.hcl") + require.NoError(t, err) + + var decoded struct{} + err = file.Decode(&decoded, &hcl.EvalContext{}) + assert.NoError(t, err) +} diff --git a/config/include.go b/config/include.go index 4c7cbad106..a2d0bbd6e7 100644 --- a/config/include.go +++ b/config/include.go @@ -209,7 +209,12 @@ func handleIncludeForDependency(ctx *ParsingContext, l log.Logger, childDecodedD case ShallowMerge: l.Debugf("Included config %s has strategy shallow merge: merging config in (shallow) for dependency.", includeConfig.Path) - mergedDependencyBlock := mergeDependencyBlocks(includedPartialParse.TerragruntDependencies, baseDependencyBlock) + mergedDependencyBlock := MergeDependencyBlocks(includedPartialParse.TerragruntDependencies, baseDependencyBlock) + // Validate that merged dependencies have unique config paths + if err := ValidateUniqueConfigPaths(mergedDependencyBlock); err != nil { + return nil, err + } + baseDependencyBlock = mergedDependencyBlock case DeepMerge: l.Debugf("Included config %s has strategy deep merge: merging config in (deep) for dependency.", includeConfig.Path) @@ -316,7 +321,13 @@ func (cfg *TerragruntConfig) Merge(l log.Logger, sourceConfig *TerragruntConfig, } // Dependency blocks are shallow merged by name - cfg.TerragruntDependencies = mergeDependencyBlocks(cfg.TerragruntDependencies, sourceConfig.TerragruntDependencies) + mergedDeps := MergeDependencyBlocks(cfg.TerragruntDependencies, sourceConfig.TerragruntDependencies) + // Validate that merged dependencies have unique config paths + if err := ValidateUniqueConfigPaths(mergedDeps); err != nil { + return err + } + + cfg.TerragruntDependencies = mergedDeps cfg.FeatureFlags = mergeFeatureFlags(cfg.FeatureFlags, sourceConfig.FeatureFlags) @@ -606,9 +617,9 @@ func mergeFeatureFlags(targetFlags []*FeatureFlag, sourceFlags []*FeatureFlag) [ return combinedFlags } -// Merge dependency blocks shallowly. If the source list has the same name as the target, it will override the +// MergeDependencyBlocks merges dependency blocks shallowly. If the source list has the same name as the target, it will override the // dependency block in the target. Otherwise, the blocks are appended. -func mergeDependencyBlocks(targetDependencies []Dependency, sourceDependencies []Dependency) []Dependency { +func MergeDependencyBlocks(targetDependencies []Dependency, sourceDependencies []Dependency) []Dependency { // We track the keys so that the dependencies are added in order, with those in target prepending those in // source. This is not strictly necessary, but it makes testing easier by making the output list more // predictable. diff --git a/config/include_test.go b/config/include_test.go index eb40191a71..4332fb8015 100644 --- a/config/include_test.go +++ b/config/include_test.go @@ -5,8 +5,10 @@ import ( "testing" "github.com/gruntwork-io/terragrunt/config" + "github.com/gruntwork-io/terragrunt/config/hclparse" "github.com/gruntwork-io/terragrunt/internal/remotestate" "github.com/gruntwork-io/terragrunt/test/helpers/logger" + "github.com/hashicorp/hcl/v2" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/zclconf/go-cty/cty" @@ -418,3 +420,77 @@ func TestIncludeConfigNotFoundError(t *testing.T) { assert.Equal(t, "/different/source.hcl", err2.SourcePath) assert.Contains(t, err2.Error(), "Include configuration not found: /another/path/config.hcl (referenced from: /different/source.hcl)") } + +func TestValidateUniqueConfigPaths_IncludeMerging(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + baseHcl string + includeHcl string + expectedErr string + }{ + { + name: "include merging with duplicate config paths", + baseHcl: ` +dependency "vpc" { + config_path = "../vpc" +} +`, + includeHcl: ` +dependency "network" { + config_path = "../vpc" # Same path as base dependency +} +`, + expectedErr: "duplicate config_path '../vpc' found in dependency blocks. Dependency 'vpc' and dependency 'network' both point to the same config path", + }, + { + name: "include merging with unique config paths", + baseHcl: ` +dependency "vpc" { + config_path = "../vpc" +} +`, + includeHcl: ` +dependency "database" { + config_path = "../database" # Different path - should work +} +`, + expectedErr: "", // No error expected + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + // Parse base dependencies + filename := config.DefaultTerragruntConfigPath + file, err := hclparse.NewParser().ParseFromString(tc.baseHcl, filename) + require.NoError(t, err) + + baseDependency := config.TerragruntDependency{} + err = file.Decode(&baseDependency, &hcl.EvalContext{}) + require.NoError(t, err) + + // Parse include dependencies + includeFile, err := hclparse.NewParser().ParseFromString(tc.includeHcl, "include.hcl") + require.NoError(t, err) + + includeDependency := config.TerragruntDependency{} + err = includeFile.Decode(&includeDependency, &hcl.EvalContext{}) + require.NoError(t, err) + + // Test shallow merge validation + mergedDeps := config.MergeDependencyBlocks(baseDependency.Dependencies, includeDependency.Dependencies) + err = config.ValidateUniqueConfigPaths(mergedDeps) + + if tc.expectedErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErr) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/docs-starlight/src/content/docs/04-reference/01-hcl/02-blocks.mdx b/docs-starlight/src/content/docs/04-reference/01-hcl/02-blocks.mdx index e00229f2d6..a85a8e2933 100644 --- a/docs-starlight/src/content/docs/04-reference/01-hcl/02-blocks.mdx +++ b/docs-starlight/src/content/docs/04-reference/01-hcl/02-blocks.mdx @@ -656,9 +656,9 @@ more about the inheritance properties of Terragrunt in the [Filling in remote st section](/docs/features/state-backend/#generating-remote-state-settings-with-terragrunt) of the "Keep your remote state configuration DRY" use case overview. -You can have more than one `include` block, but each one must have a unique label. It is recommended to always label -your `include` blocks. Bare includes (`include` block with no label - e.g., `include {}`) are currently supported for -backward compatibility, but is deprecated usage and support may be removed in the future. +You can have more than one `include` block, but each one must have a unique label and a unique path. It is recommended to +always label your `include` blocks. Bare includes (`include` block with no label - e.g., `include {}`) are currently +supported for backward compatibility, but is deprecated usage and support may be removed in the future. `include` blocks support the following arguments: @@ -666,7 +666,7 @@ backward compatibility, but is deprecated usage and support may be removed in th must be labeled with a unique name to differentiate it from the other includes. e.g., if you had a block `include "remote" {}`, you can reference the relevant exposed data with the expression `include.remote`. - `path` (attribute): Specifies the path to a Terragrunt configuration file (the `parent` config) that should be merged - with this configuration (the `child` config). + with this configuration (the `child` config). Each include block must have a unique path value. - `expose` (attribute, optional): Specifies whether or not the included config should be parsed and exposed as a variable. When `true`, you can reference the data of the included config under the variable `include`. Defaults to `false`. Note that the `include` variable is a map of `include` labels to the parsed configuration value. @@ -1202,7 +1202,8 @@ in the [Dependencies between modules section](/docs/features/stacks#dependencies-between-units) of the "Execute Opentofu/Terraform commands on multiple modules at once" use case overview. -You can define more than one `dependency` block. Each label you provide to the block identifies another `dependency` +You can define more than one `dependency` block, but each one must have a unique label and a unique `config_path`. +Each label you provide to the block identifies another `dependency` that you can reference in your config. The `dependency` block supports the following arguments: @@ -1212,7 +1213,8 @@ The `dependency` block supports the following arguments: reference the specific dependency output by the name. E.g if you had a block `dependency "vpc"`, you can reference the outputs and inputs of this dependency with the expressions `dependency.vpc.outputs` and `dependency.vpc.inputs`. - `config_path` (attribute): Path to a Terragrunt module (folder with a `terragrunt.hcl` file) that should be included - as a dependency in this configuration. + as a dependency in this configuration. Each dependency block must have a unique config_path value. When using `include` + configurations, merged dependencies are also validated for unique `config_path` values. - `enabled` (attribute): When `false`, excludes the dependency from execution. Defaults to `true`. - `skip_outputs` (attribute): When `true`, skip calling `terragrunt output` when processing this dependency. If `mock_outputs` is configured, set `outputs` to the value of `mock_outputs`. Otherwise, `outputs` will be set to an