feat: support merging the "skip" attribute from included files. (#3225)

* feat: support merging the "skip" attribute from included files. Signed-off-by: Rodrigo Fior Kuntzer <rodrigo@miro.com> * test: fix failing tests and prepare the TestApplyAllSkipTrue test to handle the case where skip is inherited from the included terragrunt file Signed-off-by: Rodrigo Fior Kuntzer <rodrigo@miro.com> * fix: post merge fixes Signed-off-by: Rodrigo Fior Kuntzer <rodrigo@miro.com> * test: fix failing tests Signed-off-by: Rodrigo Fior Kuntzer <rodrigo@miro.com> --------- Signed-off-by: Rodrigo Fior Kuntzer <rodrigo@miro.com>
gruntwork-io · Oct 7, 2024 · 38c8f29 · 38c8f29
1 parent 6916aeb
commit 38c8f29
Show file tree

Hide file tree

Showing 14 changed files with 68 additions and 32 deletions.
diff --git a/cli/commands/terraform/action.go b/cli/commands/terraform/action.go
@@ -126,7 +126,7 @@ func runTerraform(ctx context.Context, terragruntOptions *options.TerragruntOpti
 		return target.runErrorCallback(terragruntOptions, terragruntConfig, err)
 	}
 
-	if terragruntConfig.Skip {
+	if terragruntConfig.Skip != nil && *terragruntConfig.Skip {
 		terragruntOptions.Logger.Infof(
 			"Skipping terragrunt module %s due to skip = true.",
 			terragruntOptions.TerragruntConfigPath,

diff --git a/config/config.go b/config/config.go
@@ -102,7 +102,7 @@ type TerragruntConfig struct {
 	Dependencies                *ModuleDependencies
 	DownloadDir                 string
 	PreventDestroy              *bool
-	Skip                        bool
+	Skip                        *bool
 	IamRole                     string
 	IamAssumeRoleDuration       *int64
 	IamAssumeRoleSessionName    string
@@ -1114,7 +1114,7 @@ func convertToTerragruntConfig(ctx *ParsingContext, configPath string, terragrun
 	}
 
 	if terragruntConfigFromFile.Skip != nil {
-		terragruntConfig.Skip = *terragruntConfigFromFile.Skip
+		terragruntConfig.Skip = terragruntConfigFromFile.Skip
 		terragruntConfig.SetFieldMetadata(MetadataSkip, defaultMetadata)
 	}
 

diff --git a/config/config_as_cty.go b/config/config_as_cty.go
@@ -25,10 +25,13 @@ func TerragruntConfigAsCty(config *TerragruntConfig) (cty.Value, error) {
 	output[MetadataTerragruntVersionConstraint] = gostringToCty(config.TerragruntVersionConstraint)
 	output[MetadataDownloadDir] = gostringToCty(config.DownloadDir)
 	output[MetadataIamRole] = gostringToCty(config.IamRole)
-	output[MetadataSkip] = goboolToCty(config.Skip)
 	output[MetadataIamAssumeRoleSessionName] = gostringToCty(config.IamAssumeRoleSessionName)
 	output[MetadataIamWebIdentityToken] = gostringToCty(config.IamWebIdentityToken)
 
+	if config.Skip != nil {
+		output[MetadataSkip] = goboolToCty(*config.Skip)
+	}
+
 	catalogConfigCty, err := catalogConfigAsCty(config.Catalog)
 	if err != nil {
 		return cty.NilVal, err

diff --git a/config/config_as_cty_test.go b/config/config_as_cty_test.go
@@ -82,7 +82,7 @@ func TestTerragruntConfigAsCtyDrift(t *testing.T) {
 		},
 		DownloadDir:    ".terragrunt-cache",
 		PreventDestroy: &testTrue,
-		Skip:           true,
+		Skip:           &testTrue,
 		IamRole:        "terragruntRole",
 		Inputs: map[string]interface{}{
 			"aws_region": "us-east-1",

diff --git a/config/config_partial.go b/config/config_partial.go
@@ -323,7 +323,7 @@ func PartialParseConfig(ctx *ParsingContext, file *hclparse.File, includeFromChi
 			}
 
 			if decoded.Skip != nil {
-				output.Skip = *decoded.Skip
+				output.Skip = decoded.Skip
 			}
 
 			if decoded.IamRole != nil {

diff --git a/config/config_partial_test.go b/config/config_partial_test.go
@@ -33,7 +33,7 @@ dependencies {
 	assert.Equal(t, "../app1", terragruntConfig.Dependencies.Paths[0])
 	assert.Equal(t, map[string]interface{}{"app1": "../app1"}, terragruntConfig.Locals)
 
-	assert.False(t, terragruntConfig.Skip)
+	assert.Nil(t, terragruntConfig.Skip)
 	assert.Nil(t, terragruntConfig.PreventDestroy)
 	assert.Nil(t, terragruntConfig.Terraform)
 	assert.Nil(t, terragruntConfig.RemoteState)
@@ -81,7 +81,8 @@ skip = true
 	assert.Len(t, terragruntConfig.Dependencies.Paths, 1)
 	assert.Equal(t, "../app1", terragruntConfig.Dependencies.Paths[0])
 
-	assert.True(t, terragruntConfig.Skip)
+	assert.NotNil(t, terragruntConfig.Skip)
+	assert.True(t, *terragruntConfig.Skip)
 	assert.True(t, *terragruntConfig.PreventDestroy)
 
 	assert.Nil(t, terragruntConfig.Terraform)
@@ -99,7 +100,7 @@ func TestPartialParseOmittedItems(t *testing.T) {
 	require.NoError(t, err)
 	assert.True(t, terragruntConfig.IsPartial)
 	assert.Nil(t, terragruntConfig.Dependencies)
-	assert.False(t, terragruntConfig.Skip)
+	assert.Nil(t, terragruntConfig.Skip)
 	assert.Nil(t, terragruntConfig.PreventDestroy)
 	assert.Nil(t, terragruntConfig.Terraform)
 	assert.Nil(t, terragruntConfig.RemoteState)
@@ -131,7 +132,7 @@ func TestPartialParseOnlyInheritsSelectedBlocksFlags(t *testing.T) {
 
 	assert.True(t, terragruntConfig.IsPartial)
 	assert.Nil(t, terragruntConfig.Dependencies)
-	assert.False(t, terragruntConfig.Skip)
+	assert.Nil(t, terragruntConfig.Skip)
 	assert.True(t, *terragruntConfig.PreventDestroy)
 	assert.Nil(t, terragruntConfig.Terraform)
 	assert.Nil(t, terragruntConfig.RemoteState)
@@ -154,7 +155,7 @@ func TestPartialParseOnlyInheritsSelectedBlocksDependencies(t *testing.T) {
 	assert.Len(t, terragruntConfig.Dependencies.Paths, 1)
 	assert.Equal(t, "../app1", terragruntConfig.Dependencies.Paths[0])
 
-	assert.False(t, terragruntConfig.Skip)
+	assert.Nil(t, terragruntConfig.Skip)
 	assert.Nil(t, terragruntConfig.PreventDestroy)
 	assert.Nil(t, terragruntConfig.Terraform)
 	assert.Nil(t, terragruntConfig.RemoteState)

diff --git a/config/config_test.go b/config/config_test.go
@@ -720,7 +720,7 @@ func TestParseTerragruntConfigEmptyConfig(t *testing.T) {
 	assert.Nil(t, terragruntConfig.RemoteState)
 	assert.Nil(t, terragruntConfig.Dependencies)
 	assert.Nil(t, terragruntConfig.PreventDestroy)
-	assert.False(t, terragruntConfig.Skip)
+	assert.Nil(t, terragruntConfig.Skip)
 	assert.Empty(t, terragruntConfig.IamRole)
 	assert.Empty(t, terragruntConfig.IamWebIdentityToken)
 	assert.Nil(t, terragruntConfig.RetryMaxAttempts)
@@ -1213,7 +1213,8 @@ skip = true
 	assert.Nil(t, terragruntConfig.Terraform)
 	assert.Nil(t, terragruntConfig.RemoteState)
 	assert.Nil(t, terragruntConfig.Dependencies)
-	assert.True(t, terragruntConfig.Skip)
+	assert.NotNil(t, terragruntConfig.Skip)
+	assert.True(t, *terragruntConfig.Skip)
 }
 
 func TestParseTerragruntConfigSkipFalse(t *testing.T) {
@@ -1232,7 +1233,8 @@ skip = false
 	assert.Nil(t, terragruntConfig.Terraform)
 	assert.Nil(t, terragruntConfig.RemoteState)
 	assert.Nil(t, terragruntConfig.Dependencies)
-	assert.False(t, terragruntConfig.Skip)
+	assert.NotNil(t, terragruntConfig.Skip)
+	assert.False(t, *terragruntConfig.Skip)
 }
 
 func TestIncludeFunctionsWorkInChildConfig(t *testing.T) {

diff --git a/config/dependency.go b/config/dependency.go
@@ -244,7 +244,7 @@ func decodeDependencies(ctx *ParsingContext, decodedDependency TerragruntDepende
 				depCtx := ctx.WithDecodeList(TerragruntFlags, TerragruntInputs).WithTerragruntOptions(depOpts)
 
 				if depConfig, err := PartialParseConfigFile(depCtx, depPath, nil); err == nil {
-					if depConfig.Skip {
+					if depConfig.Skip != nil && *depConfig.Skip {
 						ctx.TerragruntOptions.Logger.Debugf("Skipping outputs reading for disabled dependency %s", dep.Name)
 						dep.Enabled = new(bool)
 					}

diff --git a/config/include.go b/config/include.go
@@ -263,8 +263,9 @@ func (cfg *TerragruntConfig) Merge(sourceConfig *TerragruntConfig, terragruntOpt
 		cfg.Engine = sourceConfig.Engine.Clone()
 	}
 
-	// Skip has to be set specifically in each file that should be skipped
-	cfg.Skip = sourceConfig.Skip
+	if sourceConfig.Skip != nil {
+		cfg.Skip = sourceConfig.Skip
+	}
 
 	if sourceConfig.RemoteState != nil {
 		cfg.RemoteState = sourceConfig.RemoteState
@@ -388,8 +389,9 @@ func (cfg *TerragruntConfig) DeepMerge(sourceConfig *TerragruntConfig, terragrun
 		cfg.Engine.Merge(sourceConfig.Engine)
 	}
 
-	// Skip has to be set specifically in each file that should be skipped
-	cfg.Skip = sourceConfig.Skip
+	if sourceConfig.Skip != nil {
+		cfg.Skip = sourceConfig.Skip
+	}
 
 	// Copy only dependencies which doesn't exist in source
 	if sourceConfig.Dependencies != nil {

diff --git a/config/include_test.go b/config/include_test.go
@@ -14,6 +14,9 @@ import (
 func TestMergeConfigIntoIncludedConfig(t *testing.T) {
 	t.Parallel()
 
+	testTrue := true
+	testFalse := false
+
 	testCases := []struct {
 		config         *config.TerragruntConfig
 		includedConfig *config.TerragruntConfig
@@ -111,18 +114,18 @@ func TestMergeConfigIntoIncludedConfig(t *testing.T) {
 		},
 		{
 			&config.TerragruntConfig{},
-			&config.TerragruntConfig{Skip: true},
-			&config.TerragruntConfig{Skip: false},
+			&config.TerragruntConfig{Skip: &testTrue},
+			&config.TerragruntConfig{Skip: &testTrue},
 		},
 		{
-			&config.TerragruntConfig{Skip: false},
-			&config.TerragruntConfig{Skip: true},
-			&config.TerragruntConfig{Skip: false},
+			&config.TerragruntConfig{Skip: &testFalse},
+			&config.TerragruntConfig{Skip: &testTrue},
+			&config.TerragruntConfig{Skip: &testFalse},
 		},
 		{
-			&config.TerragruntConfig{Skip: true},
-			&config.TerragruntConfig{Skip: true},
-			&config.TerragruntConfig{Skip: true},
+			&config.TerragruntConfig{Skip: &testTrue},
+			&config.TerragruntConfig{Skip: &testTrue},
+			&config.TerragruntConfig{Skip: &testTrue},
 		},
 		{
 			&config.TerragruntConfig{IamRole: "role2"},
@@ -166,6 +169,9 @@ func TestMergeConfigIntoIncludedConfig(t *testing.T) {
 func TestDeepMergeConfigIntoIncludedConfig(t *testing.T) {
 	t.Parallel()
 
+	testTrue := true
+	testFalse := false
+
 	// The following maps are convenience vars for setting up deep merge map tests
 	overrideMap := map[string]interface{}{
 		"simple_string_override": "hello, mock",
@@ -250,6 +256,25 @@ func TestDeepMergeConfigIntoIncludedConfig(t *testing.T) {
 			&config.TerragruntConfig{IamRole: "bar"},
 			&config.TerragruntConfig{IamRole: "foo"},
 		},
+		// skip related tests
+		{
+			"skip - preserve target",
+			&config.TerragruntConfig{},
+			&config.TerragruntConfig{Skip: &testTrue},
+			&config.TerragruntConfig{Skip: &testTrue},
+		},
+		{
+			"skip - copy source",
+			&config.TerragruntConfig{Skip: &testFalse},
+			&config.TerragruntConfig{Skip: &testTrue},
+			&config.TerragruntConfig{Skip: &testFalse},
+		},
+		{
+			"skip - still copy source",
+			&config.TerragruntConfig{Skip: &testTrue},
+			&config.TerragruntConfig{Skip: &testTrue},
+			&config.TerragruntConfig{Skip: &testTrue},
+		},
 		// Deep merge dependencies
 		{
 			"dependencies",

diff --git a/configstack/stack_test.go b/configstack/stack_test.go
@@ -318,7 +318,6 @@ func TestResolveTerraformModulesReadConfigFromParentConfig(t *testing.T) {
 			"retry_max_attempts":            interface{}(nil),
 			"retry_sleep_interval_sec":      interface{}(nil),
 			"retryable_errors":              interface{}(nil),
-			"skip":                          false,
 			"terraform_binary":              "",
 			"terraform_version_constraint":  "",
 			"terragrunt_version_constraint": "",

diff --git a/docs/_docs/04_reference/config-blocks-and-attributes.md b/docs/_docs/04_reference/config-blocks-and-attributes.md
@@ -1301,9 +1301,8 @@ root level `terragrunt.hcl` file, you can set `skip = true`:
 skip = true
 ```
 
-The `skip` flag must be set explicitly in terragrunt modules that should be skipped. If you set `skip = true` in a
-`terragrunt.hcl` file that is included by another `terragrunt.hcl` file, only the `terragrunt.hcl` file that explicitly
-set `skip = true` will be skipped.
+The `skip` flag can be inherited from an included `terragrunt.hcl` file if `skip` is defined there, unless it is
+explicitly redefined in the current's module `terragrunt.hcl` file.
 
 ### iam_role
 

diff --git a/test/fixtures/skip/skip-true/resource1/terragrunt.hcl b/test/fixtures/skip/skip-true/resource1/terragrunt.hcl
@@ -2,6 +2,8 @@ include {
   path = find_in_parent_folders()
 }
 
+skip = false
+
 inputs = {
   person = "Ernie"
 }

diff --git a/test/integration_test.go b/test/integration_test.go
@@ -927,10 +927,13 @@ func TestApplyAllSkipTrue(t *testing.T) {
 	stdout := showStdout.String()
 	stderr := showStderr.String()
 
+	// this test is now prepared to handle the case where skip is inherited from the included terragrunt file
+	// meaning the skip-true/resource2 module will be skipped as well and only the skip-true/resource1 module will be applied
+
 	require.NoError(t, err)
 	assert.Contains(t, stderr, "Skipping terragrunt module ./terragrunt.hcl due to skip = true.")
 	assert.Contains(t, stdout, "hello, Ernie")
-	assert.Contains(t, stdout, "hello, Bert")
+	assert.NotContains(t, stdout, "hello, Bert")
 }
 
 func TestApplyAllSkipFalse(t *testing.T) {