feat: Deprecating `terragrunt.hcl` as root #3588

yhakbar · 2024-11-21T23:16:46Z

Description

Making it so that usage of a root terragrunt.hcl throws a warning for users, and gives them guidance to move to a differently named root configuration file.

TODOs

Read the Gruntwork contribution guidelines.

Update the docs.
Run the relevant tests successfully, including pre-commit checks.
Ensure any 3rd party code adheres with our license policy or delete this line if its not applicable.
Include release notes. If this PR is backward incompatible, include a migration guide.

Release Notes (draft)

Updated strict package.
Updated config package.
Updated scaffold command.
Updated catalog command.

Migration Guide

Current idiomatic usage of Terragrunt involves using a root configuration file named terragrunt.hcl.

This causes a lot of confusion for users, and potential errors. With this change, the new recommendation of naming it anything else will be enforced via the strict package.

It will likely take a very long time before this functionality can be removed, so we should start updating our documentation to reflect this change and throw warnings that users are using less reliable configurations.

…_parent_config("root")`

See this: https://circleci.com/docs/configuration-reference/#using-when-in-workflows

… `find_in_parent_folders("root.hcl")`

This reverts commit c0ecfb9.

test/integration_test.go

denis256 · 2024-12-06T16:56:31Z

test/integration_gcp_test.go

 	project := os.Getenv("GOOGLE_CLOUD_PROJECT")
 	gcsBucketName := "terragrunt-test-bucket-" + strings.ToLower(helpers.UniqueID())
-	tmpTerragruntGCSConfigPath := createTmpTerragruntGCSConfig(t, testFixtureGcsParallelStateInit, project, terraformRemoteStateGcpRegion, gcsBucketName, config.DefaultTerragruntConfigPath)
+	tmpTerragruntGCSConfigPath := createTmpTerragruntGCSConfig(t, testFixtureGcsParallelStateInit, project, terraformRemoteStateGcpRegion, gcsBucketName, "root.hcl")


Can config.RecommendedParentConfigName be used?

Ya, but it's a pain to update them all, and I don't think it's that important. I'll look to use that constant more consistently in the future.

levkohimins

You don't really need the Name field in strict.Control.

--- a/internal/strict/strict.go
+++ b/internal/strict/strict.go
@@ -21,8 +21,6 @@ import (
 // Control represents a control that can be enabled or disabled in strict mode.
 // When the control is enabled, Terragrunt will behave in a way that is not backwards compatible.
 type Control struct {
-	// Name is the name of the strict control.
-	Name string
 	// Error is the error that will be returned when the control is enabled.
 	Error error
 	// Warning is a warning that will be logged when the control is not enabled.
@@ -57,7 +55,7 @@ const (
 )
 
 // GetStrictControl returns the strict control with the given name.
-func GetStrictControl(name string) (Control, bool) {
+func GetStrictControl(name string) (*Control, bool) {
 	control, ok := StrictControls[name]
 
 	return control, ok
@@ -65,8 +63,8 @@ func GetStrictControl(name string) (Control, bool) {
 
 // Evaluate returns a warning if the control is not enabled, an indication of whether the control has already been triggered,
 // and an error if the control is enabled.
-func (control Control) Evaluate(opts *options.TerragruntOptions) (string, bool, error) {
-	_, triggered := TriggeredControls.LoadAndStore(control.Name, true)
+func (control *Control) Evaluate(opts *options.TerragruntOptions) (string, bool, error) {
+	_, triggered := TriggeredControls.LoadAndStore(control, true)
 
 	if opts.StrictMode {
 		return "", triggered, control.Error
@@ -88,73 +86,61 @@ func (control Control) Evaluate(opts *options.TerragruntOptions) (string, bool,
 	return control.Warning, triggered, nil
 }
 
-type Controls map[string]Control
+type Controls map[string]*Control
 
 //nolint:lll,gochecknoglobals,stylecheck,revive
 var StrictControls = Controls{
 	SpinUp: {
-		Name:    SpinUp,
 		Error:   errors.Errorf("The `%s` command is no longer supported. Use `terragrunt run-all apply` instead.", SpinUp),
 		Warning: fmt.Sprintf("The `%s` command is deprecated and will be removed in a future version. Use `terragrunt run-all apply` instead.", SpinUp),
 	},
 	TearDown: {
-		Name:    TearDown,
 		Error:   errors.Errorf("The `%s` command is no longer supported. Use `terragrunt run-all destroy` instead.", TearDown),
 		Warning: fmt.Sprintf("The `%s` command is deprecated and will be removed in a future version. Use `terragrunt run-all destroy` instead.", TearDown),
 	},
 	PlanAll: {
-		Name:    PlanAll,
 		Error:   errors.Errorf("The `%s` command is no longer supported. Use `terragrunt run-all plan` instead.", PlanAll),
 		Warning: fmt.Sprintf("The `%s` command is deprecated and will be removed in a future version. Use `terragrunt run-all plan` instead.", PlanAll),
 	},
 	ApplyAll: {
-		Name:    ApplyAll,
 		Error:   errors.Errorf("The `%s` command is no longer supported. Use `terragrunt run-all apply` instead.", ApplyAll),
 		Warning: fmt.Sprintf("The `%s` command is deprecated and will be removed in a future version. Use `terragrunt run-all apply` instead.", ApplyAll),
 	},
 	DestroyAll: {
-		Name:    DestroyAll,
 		Error:   errors.Errorf("The `%s` command is no longer supported. Use `terragrunt run-all destroy` instead.", DestroyAll),
 		Warning: fmt.Sprintf("The `%s` command is deprecated and will be removed in a future version. Use `terragrunt run-all destroy` instead.", DestroyAll),
 	},
 	OutputAll: {
-		Name:    OutputAll,
 		Error:   errors.Errorf("The `%s` command is no longer supported. Use `terragrunt run-all output` instead.", OutputAll),
 		Warning: fmt.Sprintf("The `%s` command is deprecated and will be removed in a future version. Use `terragrunt run-all output` instead.", OutputAll),
 	},
 	ValidateAll: {
-		Name:    ValidateAll,
 		Error:   errors.Errorf("The `%s` command is no longer supported. Use `terragrunt run-all validate` instead.", ValidateAll),
 		Warning: fmt.Sprintf("The `%s` command is deprecated and will be removed in a future version. Use `terragrunt run-all validate` instead.", ValidateAll),
 	},
 	SkipDependenciesInputs: {
-		Name:    SkipDependenciesInputs,
 		Error:   errors.Errorf("The `%s` option is deprecated. Reading inputs from dependencies has been deprecated and will be removed in a future version of Terragrunt. To continue using inputs from dependencies, forward them as outputs.", SkipDependenciesInputs),
 		Warning: fmt.Sprintf("The `%s` option is deprecated and will be removed in a future version of Terragrunt. Reading inputs from dependencies has been deprecated. To continue using inputs from dependencies, forward them as outputs.", SkipDependenciesInputs),
 	},
 	DisableLogFormatting: {
-		Name:    DisableLogFormatting,
 		Error:   errors.Errorf("The `--%s` flag is no longer supported. Use `--terragrunt-log-format=key-value` instead.", DisableLogFormatting),
 		Warning: fmt.Sprintf("The `--%s` flag is deprecated and will be removed in a future version. Use `--terragrunt-log-format=key-value` instead.", DisableLogFormatting),
 	},
 	JSONLog: {
-		Name:    JSONLog,
 		Error:   errors.Errorf("The `--%s` flag is no longer supported. Use `--terragrunt-log-format=json` instead.", JSONLog),
 		Warning: fmt.Sprintf("The `--%s` flag is deprecated and will be removed in a future version. Use `--terragrunt-log-format=json` instead.", JSONLog),
 	},
 	TfLogJSON: {
-		Name:    TfLogJSON,
 		Error:   errors.Errorf("The `--%s` flag is no longer supported. Use `--terragrunt-log-format=json` instead.", TfLogJSON),
 		Warning: fmt.Sprintf("The `--%s` flag is deprecated and will be removed in a future version. Use `--terragrunt-log-format=json` instead.", TfLogJSON),
 	},
 	RootTerragruntHCL: {
-		Name:    RootTerragruntHCL,
 		Error:   errors.Errorf("Using `terragrunt.hcl` as the root of Terragrunt configurations is an anti-pattern, and no longer supported. Use a differently named file like `root.hcl` instead. For more information, see https://terragrunt.gruntwork.io/docs/migrate/migrating-from-root-terragrunt-hcl"),
 		Warning: "Using `terragrunt.hcl` as the root of Terragrunt configurations is an anti-pattern, and no longer recommended. In a future version of Terragrunt, this will result in an error. You are advised to use a differently named file like `root.hcl` instead. For more information, see https://terragrunt.gruntwork.io/docs/migrate/migrating-from-root-terragrunt-hcl",
 	},
 }
 
-var TriggeredControls = xsync.NewMapOf[string, bool]()
+var TriggeredControls = xsync.NewMapOf[*Control, bool]()
 
 // Names returns the names of all strict controls.
 func (controls Controls) Names() []string {

internal/strict/strict.go

yhakbar · 2024-12-09T21:11:16Z

I'm moving this into draft status, as we are discussing internally on the best way to roll this out and communicate what's going to happen to Terragrunt as a consequence of this.

yhakbar · 2024-12-09T21:15:57Z

You don't really need the Name field in strict.Control.

I considered this, but I wonder if it's actually better. I don't like Golang pointers as it's easy to create nil pointer exceptions, and I wouldn't want to put the whole struct in the triggered map.

Is it that bad to duplicate the name of the control in the value found by the key?

levkohimins · 2024-12-10T00:04:28Z

You don't really need the Name field in strict.Control.

I considered this, but I wonder if it's actually better. I don't like Golang pointers as it's easy to create nil pointer exceptions, and I wouldn't want to put the whole struct in the triggered map.

Is it that bad to duplicate the name of the control in the value found by the key?

Not sure if it's a good idea to duplicate the same string data in the map key and field structure, I see two options:

What I already suggested (I don't see anything wrong with using a pointer).
Remake StrictControls from map to slice.

docs/_docs/04_reference/built-in-functions.md

…runt-hcl-as-root

levkohimins

Left some recommendations for the code.

internal/strict/strict.go

cli/commands/catalog/tui/tui.go

cli/commands/scaffold/action.go

options/options.go

levkohimins

Great job!

yhakbar force-pushed the feat/deprecate-terragrunt-hcl-as-root branch 2 times, most recently from b25c288 to 4d12fbe Compare December 2, 2024 23:16

yhakbar added 23 commits December 3, 2024 17:17

feat: Deprecating terragrunt.hcl as root

49b3a89

fix: Passing in Logger to catalog

3f86768

fix: Updating all references to find_in_parent_config() to `find_in…

4ab7b1b

…_parent_config("root")`

fix: Addressing markdownlint errors

9d48306

fix: Addressing deprecated CircleCI configs.

e75210b

See this: https://circleci.com/docs/configuration-reference/#using-when-in-workflows

fix: Adjusting usage of find_in_parent_folders("terragrunt.hcl") to…

2cc070f

… `find_in_parent_folders("root.hcl")`

Revert "fix: Addressing deprecated CircleCI configs."

a4040b6

This reverts commit c0ecfb9.

fix: Renaming more root terragrunt.hcl files

7722fe6

fix: Test fixes

5e94730

fix: Addressing some hidden dependencies on root terragrunt.hcl

948af67

fix: Addressing more hidden root terragrunt.hcl usage

3264a04

fix: Using non-deprecated resource instead of data

9408b4a

fix: Using non-deprecated run-all instead of apply-all

01711b8

fix: Fixing more tests

4c9ddc2

fix: Setting root as root.hcl in AWS and GCP tests

e3e2112

fix: Removing unneeded config

d57c894

fix: Adjusting which tests use root.hcl and which use terragrunt.hcl

7aaaf21

fix: Hopefully last config adjustment

a7e8d7f

fix: Another unit test fix

532c309

fix: Adding debounce to strict controls

53cb839

wip: Working on getting scaffold to allow for non-terragrunt.hcl roots

e478cb9

fix: Explicitly ignoring triggered status during tests

4191c33

fix: Fixing more tests

4ab39f9

yhakbar force-pushed the feat/deprecate-terragrunt-hcl-as-root branch from e003d54 to 4ab39f9 Compare December 3, 2024 22:19

fix: Fixing TestDefaultTemplateVariables

fd3c255

yhakbar commented Dec 4, 2024

View reviewed changes

test/integration_test.go Show resolved Hide resolved

yhakbar added 2 commits December 4, 2024 11:49

fix: Fixing more tests

1520a06

fix: Remove strict mode integration tests from parallel

09cc43f

yhakbar requested a review from levkohimins as a code owner December 5, 2024 15:10

denis256 previously approved these changes Dec 6, 2024

View reviewed changes

levkohimins requested changes Dec 8, 2024

View reviewed changes

levkohimins reviewed Dec 8, 2024

View reviewed changes

internal/strict/strict.go Outdated Show resolved Hide resolved

yhakbar marked this pull request as draft December 9, 2024 21:10

yhakbar commented Dec 13, 2024

View reviewed changes

docs/_docs/04_reference/built-in-functions.md Show resolved Hide resolved

Merge remote-tracking branch 'origin/main' into feat/deprecate-terrag…

464c69e

…runt-hcl-as-root

yhakbar dismissed denis256’s stale review via 464c69e December 16, 2024 22:11

yhakbar added 2 commits December 16, 2024 17:28

feat: Trying things out the way Levko recommended

7bd9961

fix: More like Levko recommended

7c457ed

yhakbar marked this pull request as ready for review December 16, 2024 23:02

feat: Adding more docs for the deprecation

3dc5e6a

yhakbar requested a review from levkohimins December 16, 2024 23:45

levkohimins requested changes Dec 17, 2024

View reviewed changes

internal/strict/strict.go Outdated Show resolved Hide resolved

cli/commands/catalog/tui/tui.go Outdated Show resolved Hide resolved

cli/commands/scaffold/action.go Outdated Show resolved Hide resolved

cli/commands/scaffold/action.go Outdated Show resolved Hide resolved

yhakbar added 3 commits December 17, 2024 11:14

fix: Using errors instead of fmt

dcb292e

fix: Moving shared logic to cli/commands

a2c2775

fix: Reduce, Reuse, Recycle

216463b

denis256 reviewed Dec 17, 2024

View reviewed changes

options/options.go Show resolved Hide resolved

yhakbar added 2 commits December 17, 2024 11:57

fix: Fixing reference left for config.RecommendedParentConfigName

168d838

fix: It's easier to add it than to argue

d2a961f

yhakbar requested review from levkohimins and denis256 December 17, 2024 18:33

denis256 approved these changes Dec 17, 2024

View reviewed changes

levkohimins approved these changes Dec 18, 2024

View reviewed changes

yhakbar merged commit 5eba8e1 into main Dec 18, 2024
5 of 6 checks passed

yhakbar deleted the feat/deprecate-terragrunt-hcl-as-root branch December 18, 2024 14:25

BrewTestBot mentioned this pull request Dec 18, 2024

terragrunt 0.70.0 Homebrew/homebrew-core#201663

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Deprecating `terragrunt.hcl` as root #3588

feat: Deprecating `terragrunt.hcl` as root #3588

yhakbar commented Nov 21, 2024 •

edited

Loading

denis256 Dec 6, 2024

yhakbar Dec 6, 2024

denis256 Dec 6, 2024

levkohimins left a comment •

edited

Loading

yhakbar commented Dec 9, 2024

yhakbar commented Dec 9, 2024

levkohimins commented Dec 10, 2024

levkohimins left a comment

levkohimins left a comment

feat: Deprecating terragrunt.hcl as root #3588

feat: Deprecating terragrunt.hcl as root #3588

Conversation

yhakbar commented Nov 21, 2024 • edited Loading

Description

TODOs

Release Notes (draft)

Migration Guide

denis256 Dec 6, 2024

Choose a reason for hiding this comment

yhakbar Dec 6, 2024

Choose a reason for hiding this comment

denis256 Dec 6, 2024

Choose a reason for hiding this comment

levkohimins left a comment • edited Loading

Choose a reason for hiding this comment

yhakbar commented Dec 9, 2024

yhakbar commented Dec 9, 2024

levkohimins commented Dec 10, 2024

levkohimins left a comment

Choose a reason for hiding this comment

levkohimins left a comment

Choose a reason for hiding this comment

feat: Deprecating `terragrunt.hcl` as root #3588

feat: Deprecating `terragrunt.hcl` as root #3588

yhakbar commented Nov 21, 2024 •

edited

Loading

levkohimins left a comment •

edited

Loading