chore: Adding duplicate-dependency-labels strict control

Author: yhakbar

docs/src/data/changelog/v1.1.4/duplicate-dependency-labels.mdx

@@ -0,0 +1,35 @@
+---
+version: "v1.1.4"
+category: "new-features"
+---
+
+#### `duplicate-dependency-labels` strict control
+
+Declaring two `dependency` blocks with the same label in one `terragrunt.hcl` configuration file parsed without error, and then quietly resolved every reference to that label to whichever block came last. The blocks before it were silently overridden:
+
+```hcl
+dependency "vpc" {
+  config_path = "../vpc-us-east-1"
+}
+
+dependency "vpc" {
+  config_path = "../vpc-us-west-2"
+}
+
+inputs = {
+  # Reads ../vpc-us-west-2.
+  vpc_id = dependency.vpc.outputs.vpc_id
+}
+```
+
+Terragrunt now warns when it finds this. With the new [`duplicate-dependency-labels`](/reference/strict-controls/active#duplicate-dependency-labels) strict control enabled, the warning becomes an error naming the address the blocks share:
+
+```bash
+terragrunt run plan --strict-control duplicate-dependency-labels
+```
+
+```text
+/path/to/terragrunt.hcl: dependency vpc is declared more than once; every dependency needs an address of its own
+```
+
+Give each block a label of its own. A configuration that was relying on the shadowing to pick the last block should keep only that block.

docs/src/data/commands/render.mdx

@@ -138,6 +138,6 @@ dependency "shard" {
 # }
 ```
 
-The expanded blocks are in comments because expanded blocks are not valid HCL configurations (Terragrunt disallows having multiple `dependency` blocks with the same label). They are present as comments to help you predict how Terragrunt will expand the block with an `expansion` block.
+The expanded blocks are in comments because they are not a valid Terragrunt configuration: they all carry one label, and only the last block with a given label can be referenced. Terragrunt warns about that, and rejects it outright under the `duplicate-dependency-labels` [strict control](/reference/strict-controls/active#duplicate-dependency-labels). They are present as comments to help you predict how Terragrunt will expand the block with an `expansion` block.
 
 A `dependency` block with no `expansion` block renders as it always has.

docs/src/data/strict-controls/duplicate-dependency-labels.mdx

@@ -0,0 +1,35 @@
+---
+name: duplicate-dependency-labels
+status: active
+---
+
+Throw an error when two `dependency` blocks in one configuration claim the same address.
+
+### `duplicate-dependency-labels` - Reason
+
+Two `dependency` blocks written with the same label both parse, and Terragrunt then resolves every reference to that label to whichever block came last. The blocks before it are unreachable, with nothing to say so:
+
+```hcl
+dependency "vpc" {
+  config_path = "../vpc-us-east-1"
+}
+
+dependency "vpc" {
+  config_path = "../vpc-us-west-2"
+}
+
+inputs = {
+  # Reads ../vpc-us-west-2. The first block may as well not be there.
+  vpc_id = dependency.vpc.outputs.vpc_id
+}
+```
+
+Terragrunt warns about this by default. Enabling the control turns it into an error naming the address the blocks share:
+
+```text
+/path/to/terragrunt.hcl: dependency vpc is declared more than once; every dependency needs an address of its own
+```
+
+Give each block a label of its own. A configuration that was relying on the shadowing to pick the last block should keep only that block.
+
+Blocks are compared by the address they resolve to rather than by label alone, so the elements of a block expanded with the [`block-iteration`](/reference/experiments/active#block-iteration) experiment, which all carry the label the block was written with, remain valid.

internal/strict/controls/controls.go

@@ -94,6 +94,10 @@ const (
 	LegacyGCSPublicPrefix = "legacy-gcs-public-prefix"
 
 	OptionalHooks = "optional-hooks"
+
+	// DuplicateDependencyLabels is the control that prevents two `dependency` blocks in one
+	// configuration from claiming the same address.
+	DuplicateDependencyLabels = "duplicate-dependency-labels"
 )
 
 // LegacyGCSDeprecationWarning is the warning text emitted when a plain
@@ -234,6 +238,15 @@ func New() strict.Controls {
 			Warning: "Using an `include` block without a label is deprecated. Please use the `include` block with a label instead. For more information, see https://docs.terragrunt.com/migrate/bare-include/",
 		},
 
+		&Control{
+			Name:        DuplicateDependencyLabels,
+			Description: "Prevents two `dependency` blocks in one configuration from claiming the same address.",
+			Error: errors.New( //nolint:staticcheck // user-facing message intentionally written as full sentences
+				"Two `dependency` blocks address the same dependency. Give each block a label of its own.",
+			),
+			Warning: "Two `dependency` blocks address the same dependency, so only the last of them can be referenced and the rest are unreachable. Give each block a label of its own. In a future version of Terragrunt, this will result in an error.",
+		},
+
 		&Control{
 			Name:        DoubleStar,
 			Description: "Use the `**` glob pattern to select all files in a directory and its subdirectories.",

internal/strict/controls/controls_test.go

@@ -248,3 +248,16 @@ func TestControlEvaluate(t *testing.T) {
 		require.ErrorIs(t, err, bootErr)
 	})
 }
+
+// TestDuplicateDependencyLabelsControlIsRegistered pins that the control the dependency
+// decoder looks up by name is one the registry hands back.
+func TestDuplicateDependencyLabelsControlIsRegistered(t *testing.T) {
+	t.Parallel()
+
+	ctrl := controls.New().Find(controls.DuplicateDependencyLabels)
+
+	if assert.NotNil(t, ctrl, "duplicate-dependency-labels must be registered") {
+		assert.Equal(t, strict.ActiveStatus, ctrl.GetStatus())
+		assert.Error(t, ctrl.(*controls.Control).Error)
+	}
+}

pkg/config/config.go

@@ -767,10 +767,9 @@ func groupExpandedDependencies(deps Dependencies) []dependencyGroup {
 // appendExpansionPreview writes an expanded block as it was written, followed by the
 // elements it expanded into, commented out and with every reference resolved.
 //
-// The elements stay comments because they all repeat the block's label. Terragrunt reads
-// repeated labels without complaint, but every reference to one resolves to the last
-// block carrying it, so uncommented elements would render a config that quietly drops all
-// but one of them.
+// The elements stay comments because they all repeat the block's label, which
+// [validateUniqueDependencies] warns about and, under its strict control, rejects.
+// Rendering them as configuration would produce a file that reads back as one dependency.
 func appendExpansionPreview(body *hclwrite.Body, group dependencyGroup) error {
 	source, diags := hclwrite.ParseConfig(
 		[]byte(group.source.Text),
@@ -1731,7 +1730,7 @@ func ParseConfig(
 
 	// Decode the rest of the config, passing in this config's `include` block or the child's `include` block, whichever
 	// is appropriate
-	terragruntConfigFile, err := decodeAsTerragruntConfigFile(pctx, l, file, evalContext)
+	terragruntConfigFile, err := decodeAsTerragruntConfigFile(ctx, pctx, l, file, evalContext)
 	if err != nil {
 		errs = append(errs, err)
 	}
@@ -1978,6 +1977,7 @@ func setIAMRole(
 }
 
 func decodeAsTerragruntConfigFile(
+	ctx context.Context,
 	pctx *ParsingContext,
 	l log.Logger,
 	file *hclparse.File,
@@ -2001,7 +2001,7 @@ func decodeAsTerragruntConfigFile(
 		l.Debugf("Deferred attribute access error to autoinclude merge: %v", diagErr)
 	}
 
-	dependencies, err := decodeDependencyBlocks(file, evalContext, pctx.Experiments)
+	dependencies, err := decodeDependencyBlocks(ctx, pctx, l, file, evalContext)
 	if err != nil {
 		return &terragruntConfig, err
 	}

pkg/config/config_partial.go

@@ -735,9 +735,11 @@ func PartialParseConfig(
 
 		case DependencyBlock:
 			decodedDeps, err := decodeDependencyBlocks(
+				ctx,
+				pctx,
+				l,
 				file,
 				evalParsingContext,
-				pctx.Experiments,
 			)
 			if err != nil {
 				return nil, err

pkg/config/dependency.go

@@ -22,6 +22,7 @@ import (
 	"github.com/gruntwork-io/terragrunt/internal/experiment"
 	"github.com/gruntwork-io/terragrunt/internal/iacargs"
 	"github.com/gruntwork-io/terragrunt/internal/remotestate"
+	"github.com/gruntwork-io/terragrunt/internal/strict/controls"
 	"github.com/gruntwork-io/terragrunt/pkg/log"
 
 	s3backend "github.com/gruntwork-io/terragrunt/internal/remotestate/backend/s3"
@@ -270,10 +271,14 @@ func outputLocksFromContext(ctx context.Context) *util.KeyLocks {
 // decodeDependencyBlocks decodes a config's dependency blocks, returning one Dependency
 // per iteration element. A block that declares no expansion yields a single Dependency.
 func decodeDependencyBlocks(
+	ctx context.Context,
+	pctx *ParsingContext,
+	l log.Logger,
 	file *hclparse.File,
 	evalContext *hcl.EvalContext,
-	experiments experiment.Experiments,
 ) (Dependencies, error) {
+	experiments := pctx.Experiments
+
 	instances, err := file.ExpandBlocks(MetadataDependency, &Dependency{}, evalContext)
 	if err != nil {
 		return nil, err
@@ -303,9 +308,63 @@ func decodeDependencyBlocks(
 		dependencies = append(dependencies, *dep)
 	}
 
+	if err := validateUniqueDependencies(ctx, pctx, l, file.ConfigPath, dependencies); err != nil {
+		return nil, err
+	}
+
 	return dependencies, nil
 }
 
+// validateUniqueDependencies reports two dependency blocks in one config that address the
+// same dependency. HCL allows the repeated label, and nothing downstream reports it: the
+// dependency map is keyed by address, so the last block silently wins and the ones before
+// it become unreachable.
+//
+// Whether that is an error or a warning is left to the duplicate-dependency-labels strict
+// control, since configs carrying a shadowed block have always run.
+func validateUniqueDependencies(
+	ctx context.Context,
+	pctx *ParsingContext,
+	l log.Logger,
+	configPath string,
+	deps Dependencies,
+) error {
+	address, found := duplicateDependencyAddress(deps)
+	if !found {
+		return nil
+	}
+
+	control := pctx.StrictControls.Find(controls.DuplicateDependencyLabels)
+	if control == nil {
+		return errors.New("failed to find control " + controls.DuplicateDependencyLabels)
+	}
+
+	if control.GetEnabled() {
+		return DuplicateDependencyError{ConfigPath: configPath, Address: address}
+	}
+
+	return control.Evaluate(log.ContextWithLogger(ctx, l))
+}
+
+// duplicateDependencyAddress returns the first address that two dependency blocks both
+// claim. Blocks are compared by address rather than by label, so the elements of an
+// expanded block, which all carry the label the block was written with, stay distinct.
+func duplicateDependencyAddress(deps Dependencies) (string, bool) {
+	seen := make(map[string]struct{}, len(deps))
+
+	for i := range deps {
+		address := deps[i].mergeKey()
+
+		if _, duplicate := seen[address]; duplicate {
+			return address, true
+		}
+
+		seen[address] = struct{}{}
+	}
+
+	return "", false
+}
+
 // Decode the dependency blocks from the file, and then retrieve all the outputs from the remote state. Then encode the
 // resulting map as a cty.Value object.
 // TODO: In the future, consider allowing importing dependency blocks from included config
@@ -323,7 +382,7 @@ func decodeAndRetrieveOutputs(
 		return nil, err
 	}
 
-	dependencies, err := decodeDependencyBlocks(file, evalParsingContext, pctx.Experiments)
+	dependencies, err := decodeDependencyBlocks(ctx, pctx, l, file, evalParsingContext)
 	if err != nil {
 		return nil, err
 	}

pkg/config/dependency_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/gruntwork-io/terragrunt/internal/experiment"
+	"github.com/gruntwork-io/terragrunt/internal/strict/controls"
 	"github.com/gruntwork-io/terragrunt/pkg/config"
 	"github.com/gruntwork-io/terragrunt/pkg/config/hclparse"
 	"github.com/gruntwork-io/terragrunt/test/helpers/logger"
@@ -359,3 +360,105 @@ func TestDependencyDeepMergeExpansion(t *testing.T) {
 		})
 	}
 }
+
+// parseDependencyStringStrict parses cfg with the duplicate-dependency-labels strict
+// control enabled.
+func parseDependencyStringStrict(tb testing.TB, cfg string) (*config.TerragruntConfig, error) {
+	tb.Helper()
+
+	ctx, pctx := newExpansionParsingContext(tb, config.DefaultTerragruntConfigPath)
+
+	control := pctx.StrictControls.Find(controls.DuplicateDependencyLabels)
+	require.NotNil(tb, control)
+	control.Enable()
+
+	return config.PartialParseConfigString(
+		ctx,
+		pctx.WithDecodeList(config.DependencyBlock),
+		logger.CreateLogger(),
+		config.DefaultTerragruntConfigPath,
+		cfg,
+		nil,
+	)
+}
+
+const duplicateDependencyLabels = `
+dependency "foo" {
+  config_path = "../a"
+}
+
+dependency "foo" {
+  config_path = "../b"
+}
+`
+
+// TestDuplicateDependencyLabelsWarnByDefault pins that a config whose blocks shadow each
+// other still parses, since such configs have always run.
+func TestDuplicateDependencyLabelsWarnByDefault(t *testing.T) {
+	t.Parallel()
+
+	cfg, err := parseDependencyString(t, duplicateDependencyLabels)
+
+	require.NoError(t, err)
+	assert.Len(t, cfg.TerragruntDependencies, 2)
+}
+
+// TestDuplicateDependencyLabelsRejectedWhenStrict pins that the strict control turns the
+// shadowing into a parse failure naming the address two blocks claim.
+func TestDuplicateDependencyLabelsRejectedWhenStrict(t *testing.T) {
+	t.Parallel()
+
+	_, err := parseDependencyStringStrict(t, duplicateDependencyLabels)
+
+	var typed config.DuplicateDependencyError
+	require.ErrorAs(t, err, &typed)
+	assert.Equal(t, "foo", typed.Address)
+}
+
+// TestExpandedDependencyLabelsAccepted pins that the elements of one expanded block, which
+// all carry the label the block was written with, are not read as duplicates even under
+// the strict control.
+func TestExpandedDependencyLabelsAccepted(t *testing.T) {
+	t.Parallel()
+
+	cfg, err := parseDependencyStringStrict(t, `
+dependency "foo" {
+  expansion {
+    for_each = toset(["a", "b"])
+  }
+
+  config_path = "../${each.key}"
+}
+`)
+
+	require.NoError(t, err)
+	assert.Len(t, cfg.TerragruntDependencies, 2)
+}
+
+// TestExpandedDependencyLabelCollisionRejectedWhenStrict pins that two expanded blocks
+// sharing a label collide on the elements whose keys they both produce.
+func TestExpandedDependencyLabelCollisionRejectedWhenStrict(t *testing.T) {
+	t.Parallel()
+
+	_, err := parseDependencyStringStrict(t, `
+dependency "foo" {
+  expansion {
+    for_each = toset(["a"])
+  }
+
+  config_path = "../first-${each.key}"
+}
+
+dependency "foo" {
+  expansion {
+    for_each = toset(["a"])
+  }
+
+  config_path = "../second-${each.key}"
+}
+`)
+
+	var typed config.DuplicateDependencyError
+	require.ErrorAs(t, err, &typed)
+	assert.Equal(t, "foo[a]", typed.Address)
+}