Repository: gruntwork-io/terragrunt (language: go)

Issue:
Multiple includes circumvent duplicate generate detection
In #1795 / https://github.com/gruntwork-io/terragrunt/releases/tag/v0.31.9 a feature was released that should alert users to the existence of multiple generate blocks with the same name.
I encountered the situation that this check does not detect multiple generate blocks if they come from different includes.

I created a minimal example to check this: https://github.com/erpel/terragrunt_duplicate_generate_reproducer

Both `/terragrunt.hcl` and `/other_module/generate_a.hcl` define a generate block named "a". In module/terragrunt.hcl both files are included. No error is given and the last block wins silently.

To me this looks like a bug since I see no reason to want the duplicate detection be limited to within the same file.


Discussion / hints:
Hi,
indeed, `terragrunt render-json` shows that generate block contains only one entry:
```
{
  "dependencies": null,
  "download_dir": "",
  "generate": {
    "a": {
      "comment_prefix": "# ",
      "contents": "other_module",
      "disable_signature": false,
      "if_exists": "overwrite_terragrunt",
      "path": "a.txt"
    }
  },
  "iam_assume_role_duration": null,
  "iam_assume_role_session_name": "",
  "iam_role": "",
  "inputs": null,
  "locals": null,
  "retry_max_attempts": null,
  "retry_sleep_interval_sec": null,
  "retryable_errors": null,
  "skip": false,
  "terraform_binary": "",
  "terraform_version_constraint": "",
  "terragrunt_version_constraint": ""
}

```

Fix patch:
diff --git a/config/include.go b/config/include.go
index 8154732ca4..90a168591c 100644
--- a/config/include.go
+++ b/config/include.go
@@ -6,6 +6,8 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/gruntwork-io/terragrunt/codegen"
+
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/hcl/v2/hclwrite"
 	"github.com/imdario/mergo"
@@ -126,7 +128,9 @@ func handleInclude(
 			terragruntOptions.Logger.Debugf("Included config %s has strategy no merge: not merging config in.", includeConfig.Path)
 		case ShallowMerge:
 			terragruntOptions.Logger.Debugf("Included config %s has strategy shallow merge: merging config in (shallow).", includeConfig.Path)
-			parsedIncludeConfig.Merge(baseConfig, terragruntOptions)
+			if err := parsedIncludeConfig.Merge(baseConfig, terragruntOptions); err != nil {
+				return nil, err
+			}
 			baseConfig = parsedIncludeConfig
 		case DeepMerge:
 			terragruntOptions.Logger.Debugf("Included config %s has strategy deep merge: merging config in (deep).", includeConfig.Path)
@@ -174,7 +178,9 @@ func handleIncludePartial(
 			terragruntOptions.Logger.Debugf("[Partial] Included config %s has strategy no merge: not merging config in.", includeConfig.Path)
 		case ShallowMerge:
 			terragruntOptions.Logger.Debugf("[Partial] Included config %s has strategy shallow merge: merging config in (shallow).", includeConfig.Path)
-			parsedIncludeConfig.Merge(baseConfig, terragruntOptions)
+			if err := parsedIncludeConfig.Merge(baseConfig, terragruntOptions); err != nil {
+				return nil, err
+			}
 			baseConfig = parsedIncludeConfig
 		case DeepMerge:
 			terragruntOptions.Logger.Debugf("[Partial] Included config %s has strategy deep merge: merging config in (deep).", includeConfig.Path)
@@ -247,7 +253,7 @@ func handleIncludeForDependency(
 // NOTE: dependencies block is a special case and is merged deeply. This is necessary to ensure the configstack system
 // works correctly, as it uses the `Dependencies` list to track the dependencies of modules for graph building purposes.
 // This list includes the dependencies added from dependency blocks, which is handled in a different stage.
-func (targetConfig *TerragruntConfig) Merge(sourceConfig *TerragruntConfig, terragruntOptions *options.TerragruntOptions) {
+func (targetConfig *TerragruntConfig) Merge(sourceConfig *TerragruntConfig, terragruntOptions *options.TerragruntOptions) error {
 	// Merge simple attributes first
 	if sourceConfig.DownloadDir != "" {
 		targetConfig.DownloadDir = sourceConfig.DownloadDir
@@ -326,6 +332,12 @@ func (targetConfig *TerragruntConfig) Merge(sourceConfig *TerragruntConfig, terr
 
 	// Merge the generate configs. This is a shallow merge. Meaning, if the child has the same name generate block, then the
 	// child's generate block will override the parent's block.
+
+	err := validateGenerateConfigs(&sourceConfig.GenerateConfigs, &targetConfig.GenerateConfigs)
+	if err != nil {
+		return err
+	}
+
 	for key, val := range sourceConfig.GenerateConfigs {
 		targetConfig.GenerateConfigs[key] = val
 	}
@@ -335,6 +347,8 @@ func (targetConfig *TerragruntConfig) Merge(sourceConfig *TerragruntConfig, terr
 	}
 
 	copyFieldsMetadata(sourceConfig, targetConfig)
+
+	return nil
 }
 
 // DeepMerge performs a deep merge of the given sourceConfig into the targetConfig. Deep merge is defined as follows:
@@ -861,6 +875,22 @@ func copyFieldsMetadata(sourceConfig *TerragruntConfig, targetConfig *Terragrunt
 	}
 }
 
+// validateGenerateConfigs Validate if exists duplicate generate configs.
+func validateGenerateConfigs(sourceConfig *map[string]codegen.GenerateConfig, targetConfig *map[string]codegen.GenerateConfig) error {
+	var duplicatedNames []string
+	for key := range *targetConfig {
+		if _, found := (*sourceConfig)[key]; found {
+			duplicatedNames = append(duplicatedNames, key)
+		}
+	}
+
+	if len(duplicatedNames) != 0 {
+		return DuplicatedGenerateBlocks{duplicatedNames}
+	}
+
+	return nil
+}
+
 // Custom error types
 
 type MultipleBareIncludeBlocksErr struct{}


Test patch:
diff --git a/config/include_test.go b/config/include_test.go
index 55fc284be4..f46c10c032 100644
--- a/config/include_test.go
+++ b/config/include_test.go
@@ -134,7 +134,8 @@ func TestMergeConfigIntoIncludedConfig(t *testing.T) {
 			testCase.expected.TerragruntDependencies = []Dependency{}
 		}
 
-		testCase.includedConfig.Merge(testCase.config, mockOptionsForTest(t))
+		err := testCase.includedConfig.Merge(testCase.config, mockOptionsForTest(t))
+		assert.NoError(t, err)
 		assert.Equal(t, testCase.expected, testCase.includedConfig)
 	}
 }
diff --git a/test/fixture-codegen/generate-block/nested/child_overwrite/terragrunt.hcl b/test/fixture-codegen/generate-block/nested/child_overwrite/terragrunt.hcl
index 6b4659d4d2..5e34b073a8 100644
--- a/test/fixture-codegen/generate-block/nested/child_overwrite/terragrunt.hcl
+++ b/test/fixture-codegen/generate-block/nested/child_overwrite/terragrunt.hcl
@@ -1,5 +1,6 @@
 include {
   path = "${get_terragrunt_dir()}/../root.hcl"
+  merge_strategy = "deep"
 }
 
 generate "backend" {
diff --git a/test/fixture-codegen/generate-block/same_name_includes_error/app1.hcl b/test/fixture-codegen/generate-block/same_name_includes_error/app1.hcl
new file mode 100644
index 0000000000..da64203381
--- /dev/null
+++ b/test/fixture-codegen/generate-block/same_name_includes_error/app1.hcl
@@ -0,0 +1,6 @@
+generate "backend" {
+  path = "backend.txt"
+  contents = "backend 2"
+  if_exists = "overwrite_terragrunt"
+}
+
diff --git a/test/fixture-codegen/generate-block/same_name_includes_error/app2.hcl b/test/fixture-codegen/generate-block/same_name_includes_error/app2.hcl
new file mode 100644
index 0000000000..d2293792fa
--- /dev/null
+++ b/test/fixture-codegen/generate-block/same_name_includes_error/app2.hcl
@@ -0,0 +1,6 @@
+generate "backend" {
+  path = "backend.txt"
+  contents = "other_module"
+  if_exists = "overwrite_terragrunt"
+}
+
diff --git a/test/fixture-codegen/generate-block/same_name_includes_error/terragrunt.hcl b/test/fixture-codegen/generate-block/same_name_includes_error/terragrunt.hcl
new file mode 100644
index 0000000000..3508cfbec3
--- /dev/null
+++ b/test/fixture-codegen/generate-block/same_name_includes_error/terragrunt.hcl
@@ -0,0 +1,7 @@
+include "app1" {
+  path = "app1.hcl"
+}
+
+include "app2" {
+  path = "app2.hcl"
+}
diff --git a/test/integration_test.go b/test/integration_test.go
index ea12a5a946..3f4ca7d9d1 100644
--- a/test/integration_test.go
+++ b/test/integration_test.go
@@ -3640,6 +3640,23 @@ func TestTerragruntGenerateBlockSameNameFail(t *testing.T) {
 	assert.Contains(t, parsedError.BlockName, "backend")
 }
 
+func TestTerragruntGenerateBlockSameNameIncludeFail(t *testing.T) {
+	t.Parallel()
+
+	generateTestCase := filepath.Join(TEST_FIXTURE_CODEGEN_PATH, "generate-block", "same_name_includes_error")
+	cleanupTerraformFolder(t, generateTestCase)
+	cleanupTerragruntFolder(t, generateTestCase)
+
+	stdout := bytes.Buffer{}
+	stderr := bytes.Buffer{}
+	err := runTerragruntCommand(t, fmt.Sprintf("terragrunt init --terragrunt-working-dir %s", generateTestCase), &stdout, &stderr)
+	require.Error(t, err)
+	parsedError, ok := errors.Unwrap(err).(config.DuplicatedGenerateBlocks)
+	assert.True(t, ok)
+	assert.True(t, len(parsedError.BlockName) == 1)
+	assert.Contains(t, parsedError.BlockName, "backend")
+}
+
 func TestTerragruntGenerateBlockMultipleSameNameFail(t *testing.T) {
 	t.Parallel()