chore: removal of command forwarding by default#4871
Conversation
…ag' into 4599-command-forward
📝 WalkthroughWalkthroughReworks CLI flag plumbing by removing "moved flag" APIs, introduces shared backend/feature flag helpers, fast-fails unknown top-level commands instructing users to use Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant TG as Terragrunt CLI
participant Parser as Arg Parser
participant Cmd as Terragrunt Command Handler
participant Run as Run Command
participant TF as OpenTofu/Terraform
User->>TG: terragrunt <first-token> [args...]
TG->>Parser: parse args, find first non-flag token
alt token is known Terragrunt command
Parser-->>TG: route to handler
TG->>Cmd: execute command (flags include NewBackendFlags/NewFeatureFlags)
Cmd-->>User: result
else token unknown
Parser-->>TG: unknown top-level command
TG-->>User: exit error -> "use 'terragrunt run -- <cmd> [args]'"
end
Note right of User: Explicit forwarding via run
User->>TG: terragrunt run -- <tf-cmd> [args]
TG->>Run: validate flags (incl. backend/feature)
Run->>TF: invoke <tf-cmd> with args
TF-->>Run: result
Run-->>User: propagate result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/strict/controls/deprecated_flag_name.go (1)
33-80: Honor the explicitnewValueoverride when composing the replacement flag.
NewDeprecatedFlagNamenow acceptsnewValue, but after construction the evaluation path ignores it and falls back to whatever value happens to be present onnewFlag(often empty) or the deprecated flag’s runtime value. That produces misleading guidance whenever callers pass a fixed override such as--foo=auto. PersistnewValueon the control and prefer it when formatting the replacement suggestion.type DeprecatedFlagName struct { deprecatedFlag cli.Flag newFlag cli.Flag + newValue string *Control ErrorFmt string WarningFmt string } @@ func NewDeprecatedFlagName(deprecatedFlag, newFlag cli.Flag, newValue string) *DeprecatedFlagName { @@ - deprecatedFlag: deprecatedFlag, - newFlag: newFlag, + deprecatedFlag: deprecatedFlag, + newFlag: newFlag, + newValue: newValue, } } @@ - if names := ctrl.newFlag.Names(); len(names) > 0 { + if names := ctrl.newFlag.Names(); len(names) > 0 { flagName = names[0] - if ctrl.newFlag.TakesValue() { - value := ctrl.newFlag.Value().String() + if ctrl.newValue != "" { + flagName += "=" + ctrl.newValue + } else if ctrl.newFlag.TakesValue() { + value := ctrl.newFlag.Value().String() if value == "" { value = ctrl.deprecatedFlag.Value().String() } flagName += "=" + value } }
🧹 Nitpick comments (10)
cli/flags/flag_test.go (1)
83-89: Consider extracting warning format functions to test helpers.These helper functions create warning format strings but are defined inline within the test. Consider moving them to a test helper file or package-level helpers if they're used across multiple test files.
// Move these to a test helper file or as package-level test helpers var ( deprecatedFlagWarning = func() string { return controls.NewDeprecatedFlagName(&cli.BoolFlag{}, &cli.BoolFlag{}, "").WarningFmt } deprecatedEnvVarWarning = func() string { return controls.NewDeprecatedEnvVar(&cli.BoolFlag{}, &cli.BoolFlag{}, "").WarningFmt } )test/integration_test.go (1)
4378-4389: Assert guidance to userun --for unknown commandsStrengthen the test by asserting the error message hints at using
run --to run workspace operations explicitly.func TestNoDefaultForwardingUnknownCommand(t *testing.T) { t.Parallel() helpers.CleanupTerraformFolder(t, testFixturePath) tmpEnvPath := helpers.CopyEnvironment(t, testFixturePath) rootPath := util.JoinPath(tmpEnvPath, testFixturePath) - _, _, err := helpers.RunTerragruntCommandWithOutput(t, "terragrunt workspace list --non-interactive --working-dir "+rootPath) - require.Error(t, err, "expected error when invoking unknown top-level command without 'run'") + _, _, err := helpers.RunTerragruntCommandWithOutput(t, "terragrunt workspace list --non-interactive --working-dir "+rootPath) + require.Error(t, err, "expected error when invoking unknown top-level command without 'run'") + // Ensure the error nudges users towards the explicit run flow. + require.Contains(t, err.Error(), "run --", "error should guide users to use 'terragrunt run -- ...'") }cli/commands/find/cli.go (1)
101-105: Avoid shadowing theflagsimportThe local variable named
flagsshadows the importedgithub.com/gruntwork-io/terragrunt/cli/flagspackage, reducing readability and risking mistakes.- // Base flags for find plus backend/feature flags - flags := NewFlags(cmdOpts, nil) - flags = append(flags, runCmd.NewBackendFlags(l, opts, nil)...) - flags = append(flags, runCmd.NewFeatureFlags(l, opts, nil)...) + // Base flags for find plus backend/feature flags + cmdFlags := NewFlags(cmdOpts, nil) + cmdFlags = append(cmdFlags, runCmd.NewBackendFlags(l, opts, nil)...) + cmdFlags = append(cmdFlags, runCmd.NewFeatureFlags(l, opts, nil)...) ... - Flags: flags, + Flags: cmdFlags,Based on learnings
Also applies to: 110-110
cli/commands/scaffold/cli.go (1)
95-99: Avoid shadowing theflagsimportRename the local
flagsslice to avoid shadowinggithub.com/gruntwork-io/terragrunt/cli/flags.-func NewCommand(l log.Logger, opts *options.TerragruntOptions) *cli.Command { - flags := NewFlags(opts, nil) - // Accept backend and feature flags for scaffold as well - flags = append(flags, runCmd.NewBackendFlags(l, opts, nil)...) - flags = append(flags, runCmd.NewFeatureFlags(l, opts, nil)...) +func NewCommand(l log.Logger, opts *options.TerragruntOptions) *cli.Command { + flagSet := NewFlags(opts, nil) + // Accept backend and feature flags for scaffold as well + flagSet = append(flagSet, runCmd.NewBackendFlags(l, opts, nil)...) + flagSet = append(flagSet, runCmd.NewFeatureFlags(l, opts, nil)...) ... - Flags: flags, + Flags: flagSet,Based on learnings
Also applies to: 103-103
docs-starlight/src/content/docs/07-migrate/03-cli-redesign.md (2)
166-170: Call out bothworkspace listandworkspace lsUsers often use both forms; suggesting both improves clarity. Also consider briefly showing the error users will see when default forwarding is disabled.
Example:
- If you previously ran
terragrunt workspace list(orworkspace ls), use:
terragrunt run -- workspace list
208-213: Clarify Terragrunt vs Terraform “graph”The top-level Terragrunt
graphis deprecated in favor ofdag graph, whileterragrunt run graphinvokes the Terraform/OpenTofugraph. A brief note here will avoid confusion.Proposed addition after the example:
- Note:
terragrunt run graphruns the Terraform/OpenTofu graph command. For Terragrunt’s DAG visualization, useterragrunt dag graph.cli/commands/run/flags.go (4)
96-101: Deduplicate backend/feature flag name constants; use shared globals.These constants duplicate the ones in cli/flags/global/flags.go, risking drift.
Apply this diff to remove local duplicates:
- // Backend and feature flags (shared with backend commands) - BackendBootstrapFlagName = "backend-bootstrap" - BackendRequireBootstrapFlagName = "backend-require-bootstrap" - DisableBucketUpdateFlagName = "disable-bucket-update" - FeatureFlagName = "feature"Add this import (outside this range) to use the shared constants:
import ( "fmt" "path/filepath" "github.com/gruntwork-io/terragrunt/cli/flags" + "github.com/gruntwork-io/terragrunt/cli/flags/global" "github.com/gruntwork-io/terragrunt/internal/cli" "github.com/gruntwork-io/terragrunt/internal/report" "github.com/gruntwork-io/terragrunt/internal/strict/controls" "github.com/gruntwork-io/terragrunt/options" "github.com/gruntwork-io/terragrunt/pkg/log" "github.com/gruntwork-io/terragrunt/util" )
635-647: Switch usages to shared global constants.Update references to point at cli/flags/global to complete the deduplication.
Apply this diff:
flags.NewFlag(&cli.BoolFlag{ - Name: BackendBootstrapFlagName, - EnvVars: tgPrefix.EnvVars(BackendBootstrapFlagName), + Name: global.BackendBootstrapFlagName, + EnvVars: tgPrefix.EnvVars(global.BackendBootstrapFlagName), Destination: &opts.BackendBootstrap, Usage: "Automatically bootstrap backend infrastructure before attempting to use it.", }), flags.NewFlag(&cli.BoolFlag{ - Name: BackendRequireBootstrapFlagName, - EnvVars: tgPrefix.EnvVars(BackendRequireBootstrapFlagName), + Name: global.BackendRequireBootstrapFlagName, + EnvVars: tgPrefix.EnvVars(global.BackendRequireBootstrapFlagName), Destination: &opts.FailIfBucketCreationRequired, Usage: "When this flag is set Terragrunt will fail if the remote state bucket needs to be created.", }, flags.WithDeprecatedEnvVars(terragruntPrefix.EnvVars("fail-on-state-bucket-creation"), terragruntPrefixControl), ), flags.NewFlag(&cli.BoolFlag{ - Name: DisableBucketUpdateFlagName, - EnvVars: tgPrefix.EnvVars(DisableBucketUpdateFlagName), + Name: global.DisableBucketUpdateFlagName, + EnvVars: tgPrefix.EnvVars(global.DisableBucketUpdateFlagName), Destination: &opts.DisableBucketUpdate, Usage: "When this flag is set Terragrunt will not update the remote state bucket.", }, flags.WithDeprecatedEnvVars(terragruntPrefix.EnvVars("disable-bucket-update"), terragruntPrefixControl), ),- flags.NewFlag(&cli.MapFlag[string, string]{ - Name: FeatureFlagName, - EnvVars: tgPrefix.EnvVars(FeatureFlagName), + flags.NewFlag(&cli.MapFlag[string, string]{ + Name: global.FeatureFlagName, + EnvVars: tgPrefix.EnvVars(global.FeatureFlagName), Usage: "Set feature flags for the HCL code.", - Splitter: util.SplitComma, + EnvVarSep: ",", + KeyValSep: "=", },Also applies to: 649-655, 667-669
110-110: Avoid shadowing the imported flags package.The local variable named flags shadows the imported package, which is error-prone. Prefer flagSet.
Apply this diff:
- flags := cli.Flags{ + flagSet := cli.Flags{- // Add shared backend and feature flags - flags = flags.Add(NewBackendFlags(l, opts, prefix)...) - flags = flags.Add(NewFeatureFlags(l, opts, prefix)...) - - return flags.Sort() + // Add shared backend and feature flags + flagSet = flagSet.Add(NewBackendFlags(l, opts, prefix)...) + flagSet = flagSet.Add(NewFeatureFlags(l, opts, prefix)...) + + return flagSet.Sort()Based on learnings
Also applies to: 599-603
129-129: Typo in comment."graph/-grpah" -> "graph".
- // `graph/-grpah` related flags. + // `graph` related flags.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
cli/app.go(2 hunks)cli/commands/backend/bootstrap/cli.go(1 hunks)cli/commands/backend/delete/cli.go(1 hunks)cli/commands/backend/migrate/cli.go(1 hunks)cli/commands/deprecated.go(1 hunks)cli/commands/find/cli.go(2 hunks)cli/commands/list/cli.go(2 hunks)cli/commands/run/flags.go(3 hunks)cli/commands/scaffold/cli.go(2 hunks)cli/flags/deprecated_flag.go(0 hunks)cli/flags/flag.go(0 hunks)cli/flags/flag_opts.go(0 hunks)cli/flags/flag_test.go(1 hunks)cli/flags/global/flags.go(1 hunks)docs-starlight/src/content/docs/01-getting-started/04-terminology.md(1 hunks)docs-starlight/src/content/docs/07-migrate/03-cli-redesign.md(2 hunks)internal/strict/controls/deprecated_flag_name.go(1 hunks)test/integration_aws_test.go(3 hunks)test/integration_gcp_test.go(3 hunks)test/integration_test.go(1 hunks)
💤 Files with no reviewable changes (3)
- cli/flags/flag.go
- cli/flags/flag_opts.go
- cli/flags/deprecated_flag.go
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
⚙️ CodeRabbit configuration file
Review the Go code for quality and correctness. Make sure that the Go code follows best practices, is performant, and is easy to understand and maintain.
Files:
test/integration_test.gotest/integration_aws_test.gointernal/strict/controls/deprecated_flag_name.gocli/commands/deprecated.gocli/commands/backend/delete/cli.gocli/commands/backend/bootstrap/cli.gotest/integration_gcp_test.gocli/app.gocli/commands/scaffold/cli.gocli/flags/flag_test.gocli/commands/find/cli.gocli/commands/run/flags.gocli/commands/backend/migrate/cli.gocli/commands/list/cli.gocli/flags/global/flags.go
docs-starlight/**/*.md*
⚙️ CodeRabbit configuration file
Review the documentation for clarity, grammar, and spelling. Make sure that the documentation is easy to understand and follow. There is currently a migration underway from the Jekyll based documentation in
docsto the Starlight + Astro based documentation indocs-starlight. Make sure that thedocs-starlightdocumentation is accurate and up-to-date with thedocsdocumentation, and that any difference between them results in an improvement in thedocs-starlightdocumentation.
Files:
docs-starlight/src/content/docs/01-getting-started/04-terminology.mddocs-starlight/src/content/docs/07-migrate/03-cli-redesign.md
🧠 Learnings (3)
📚 Learning: 2025-04-17T13:02:28.098Z
Learnt from: yhakbar
PR: gruntwork-io/terragrunt#4169
File: cli/commands/hcl/validate/cli.go:29-60
Timestamp: 2025-04-17T13:02:28.098Z
Learning: Avoid shadowing imported packages with local variables in Go code, such as using a variable named `flags` when the `github.com/gruntwork-io/terragrunt/cli/flags` package is imported. Use more specific variable names like `flagSet` instead.
Applied to files:
cli/app.gocli/commands/scaffold/cli.gocli/commands/find/cli.gocli/commands/run/flags.gocli/commands/list/cli.gocli/flags/global/flags.go
📚 Learning: 2025-02-10T13:36:19.542Z
Learnt from: levkohimins
PR: gruntwork-io/terragrunt#3723
File: cli/commands/stack/action.go:160-160
Timestamp: 2025-02-10T13:36:19.542Z
Learning: The project uses a custom error package `github.com/gruntwork-io/terragrunt/internal/errors` which provides similar functionality to `fmt.Errorf` but includes stack traces. Prefer using this package's error functions (e.g., `errors.Errorf`, `errors.New`) over the standard library's error handling.
Applied to files:
cli/commands/scaffold/cli.gocli/commands/find/cli.gocli/flags/global/flags.go
📚 Learning: 2025-02-10T23:20:04.295Z
Learnt from: yhakbar
PR: gruntwork-io/terragrunt#3868
File: docs-starlight/patches/@astrojs%2Fstarlight@0.31.1.patch:33-33
Timestamp: 2025-02-10T23:20:04.295Z
Learning: In Terragrunt projects, all `.hcl` files can be assumed to be Terragrunt configurations by default, with specific exceptions like `.terraform.lock.hcl` that need explicit handling.
Applied to files:
docs-starlight/src/content/docs/01-getting-started/04-terminology.md
🧬 Code graph analysis (12)
test/integration_test.go (2)
test/helpers/package.go (3)
CleanupTerraformFolder(878-885)CopyEnvironment(88-101)RunTerragruntCommandWithOutput(993-997)util/file.go (1)
JoinPath(528-530)
test/integration_aws_test.go (1)
test/helpers/package.go (2)
RunTerragruntCommandWithOutput(993-997)TerraformRemoteStateS3Region(60-60)
cli/commands/deprecated.go (2)
cli/commands/run/cli.go (1)
Action(78-92)internal/cli/errors.go (1)
NewExitError(53-69)
cli/commands/backend/delete/cli.go (3)
cli/commands/backend/bootstrap/cli.go (1)
NewFlags(14-21)cli/commands/backend/migrate/cli.go (1)
NewFlags(20-37)cli/commands/run/flags.go (5)
NewFlags(104-604)ConfigFlagName(18-18)DownloadDirFlagName(23-23)NewBackendFlags(628-657)NewFeatureFlags(660-681)
cli/commands/backend/bootstrap/cli.go (1)
cli/commands/run/flags.go (5)
NewFlags(104-604)ConfigFlagName(18-18)DownloadDirFlagName(23-23)NewBackendFlags(628-657)NewFeatureFlags(660-681)
test/integration_gcp_test.go (1)
test/helpers/package.go (1)
RunTerragruntCommandWithOutput(993-997)
cli/app.go (3)
cli/flags/global/flags.go (1)
NewFlags(88-244)internal/cli/command.go (1)
Command(9-80)internal/cli/errors.go (1)
NewExitError(53-69)
cli/commands/scaffold/cli.go (1)
cli/commands/run/flags.go (2)
NewBackendFlags(628-657)NewFeatureFlags(660-681)
cli/commands/find/cli.go (1)
cli/commands/run/flags.go (2)
NewBackendFlags(628-657)NewFeatureFlags(660-681)
cli/commands/run/flags.go (6)
options/options.go (1)
TerragruntOptions(100-332)internal/cli/bool_flag.go (1)
BoolFlag(13-49)cli/flags/global/flags.go (4)
BackendBootstrapFlagName(79-79)BackendRequireBootstrapFlagName(80-80)DisableBucketUpdateFlagName(81-81)FeatureFlagName(84-84)cli/flags/flag_opts.go (1)
WithDeprecatedEnvVars(103-114)internal/cli/map_flag.go (1)
MapFlag(33-71)util/collections.go (1)
SplitComma(209-211)
cli/commands/backend/migrate/cli.go (2)
cli/commands/backend/bootstrap/cli.go (1)
NewFlags(14-21)cli/commands/run/flags.go (5)
NewFlags(104-604)ConfigFlagName(18-18)DownloadDirFlagName(23-23)NewBackendFlags(628-657)NewFeatureFlags(660-681)
cli/commands/list/cli.go (2)
cli/commands/run/flags.go (2)
NewBackendFlags(628-657)NewFeatureFlags(660-681)internal/cli/command.go (1)
Command(9-80)
🪛 GitHub Actions: CI
test/integration_test.go
[error] 78-78: Received unexpected error: run not found in report: /tmp/TestIgnoreError654045578/001/fixtures/errors/ignore
[error] 78-78: Received unexpected error: run not found in report: /tmp/TestRetryFailError746391533/001/fixtures/errors/retry
[error] 78-78: Received unexpected error: run not found in report: /tmp/TestError/fixtures/errors/default
🔇 Additional comments (20)
cli/flags/flag_test.go (2)
102-126: LGTM! Test case properly validates deprecation warnings.The test case correctly validates both deprecated flag name and deprecated environment variable scenarios. The logic properly sets up the flags with deprecation options and verifies the expected warning messages are generated.
101-101: Ignore outdated refactoring concern. ThetestCasesslice inTestFlag_TakesValueis populated (including aWithDeprecatedNamescenario) and isn’t empty—there’s no missing “moved flag” test.Likely an incorrect or invalid review comment.
cli/app.go (1)
169-176: RebindcmdNameinside the closure to avoid stale values.Because
cmdNamecomes from the outer scope, any later mutation would leak into the closure. Capture it in a new local before use.- cmdName := ctx.Args().CommandName() - if cmdName != "" { - if ctx.Command == nil || ctx.Command.Subcommand(cmdName) == nil { + cmdName := ctx.Args().CommandName() + if name := cmdName; name != "" { + if ctx.Command == nil || ctx.Command.Subcommand(name) == nil { return cli.NewExitError( - errors.Errorf("unknown command: %q. Terragrunt no longer forwards unknown commands by default. Use 'terragrunt run -- %s ...' or a supported shortcut. Learn more: https://terragrunt.gruntwork.io/docs/migrate/cli-redesign/#use-the-new-run-command", cmdName, cmdName), + errors.Errorf("unknown command: %q. Terragrunt no longer forwards unknown commands by default. Use 'terragrunt run -- %s ...' or a supported shortcut. Learn more: https://terragrunt.gruntwork.io/docs/migrate/cli-redesign/#use-the-new-run-command", name, name), cli.ExitCodeGeneralError, ) } }⛔ Skipped due to learnings
Learnt from: yhakbar PR: gruntwork-io/terragrunt#4169 File: cli/commands/hcl/validate/cli.go:29-60 Timestamp: 2025-04-17T13:02:28.098Z Learning: Avoid shadowing imported packages with local variables in Go code, such as using a variable named `flags` when the `github.com/gruntwork-io/terragrunt/cli/flags` package is imported. Use more specific variable names like `flagSet` instead.test/integration_aws_test.go (7)
247-248: LGTM: flag ordering aligned with new behavior
--feature disable_versioning=truebeforeapplyand explicit--backend-bootstraplook correct.
253-258: LGTM: backend delete flow matches redesigned CLIUsing top-level
backend deletewith--backend-bootstrapand--forceis consistent.
282-284: LGTM: access logging feature flag usage
--feature access_logging_bucket=...combined with--backend-bootstrapis correct.
311-313: LGTM: unit-scoped bootstrap with featuresPattern
run --non-interactive ... --feature disable_versioning=true apply --backend-bootstrapis correct.
317-322: LGTM: backend migrate with explicit bootstrap/feature flagsBoth guarded and forced migrate variants look correct.
424-427: LGTM: apply with backend bootstrap post-migrationRe-running apply after migration with
--backend-bootstrapis coherent.
686-693: LGTM: run output with backend bootstrapStack output invocation and assertions align with new semantics.
test/integration_gcp_test.go (7)
63-66: LGTM: explicit backend bootstrap viabackendcommandUsing
backend bootstrap --backend-bootstrapis consistent with the redesign.
116-117: LGTM: feature flag and bootstrap ordering
--feature disable_versioning=truebeforeapplyand explicit bootstrap is correct.
121-127: LGTM: backend delete sequenceDelete without bootstrap failing then with
--backend-bootstrap --forcesucceeding matches expectations.
147-149: LGTM: unit-scoped bootstrap with featuresCorrect usage for unit path with
--feature disable_versioning=true.
152-157: LGTM: backend migrate with explicit bootstrap/feature flagsThe guarded and forced paths look correct.
240-243: LGTM: apply with bootstrap after migration
run apply --backend-bootstrapon unit2 is consistent.
340-341: LGTM: explicitrun --separator usage
terragrunt run --all ... -- apply -auto-approvedemonstrates clear separation; good example.cli/commands/run/flags.go (1)
671-675: Guard against nil FeatureFlags map.opts.FeatureFlags is a pointer; if nil, Store will panic. Ensure it’s initialized during options construction or guard here.
If needed, initialize before use:
if opts.FeatureFlags == nil { opts.FeatureFlags = xsync.NewMapOf[string, string]() }Would you like me to scan the repo to confirm initialization happens in options constructors?
cli/flags/global/flags.go (1)
78-85: Backend/feature flag constants look solidNew identifiers align with existing naming and expose the required flag names for downstream wiring. No issues spotted.
docs-starlight/src/content/docs/01-getting-started/04-terminology.md (1)
143-143: Confirm the referenced version before mergingPlease double-check that the release carrying this behavioral change will indeed be tagged v0.88.0. If the final version differs, update the note so the docs stay accurate.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
cli/commands/run/flags.go (4)
96-101: Deduplicate shared flag constants; import from cli/flags/global.These constants now exist in cli/flags/global/flags.go. Keep a single source of truth and reference the shared ones instead of redefining here.
Apply this diff to remove the local redefinitions:
- // Backend and feature flags (shared with backend commands) - BackendBootstrapFlagName = "backend-bootstrap" - BackendRequireBootstrapFlagName = "backend-require-bootstrap" - DisableBucketUpdateFlagName = "disable-bucket-update" - FeatureFlagName = "feature" + // Backend and feature flags are defined in cli/flags/global.And add this import (outside this range):
import ( // ... globalflags "github.com/gruntwork-io/terragrunt/cli/flags/global" )
599-602: Avoid shadowing the imported flags package with the local variable.
flags = flags.Add(...)is easy to misread. Prefer a distinct name (e.g.,flagSet).Based on learnings
626-657: Use shared constants from cli/flags/global to prevent drift.Replace local names with the shared ones.
Apply this diff:
return cli.Flags{ - flags.NewFlag(&cli.BoolFlag{ - Name: BackendBootstrapFlagName, - EnvVars: tgPrefix.EnvVars(BackendBootstrapFlagName), + flags.NewFlag(&cli.BoolFlag{ + Name: globalflags.BackendBootstrapFlagName, + EnvVars: tgPrefix.EnvVars(globalflags.BackendBootstrapFlagName), Destination: &opts.BackendBootstrap, Usage: "Automatically bootstrap backend infrastructure before attempting to use it.", }), flags.NewFlag(&cli.BoolFlag{ - Name: BackendRequireBootstrapFlagName, - EnvVars: tgPrefix.EnvVars(BackendRequireBootstrapFlagName), + Name: globalflags.BackendRequireBootstrapFlagName, + EnvVars: tgPrefix.EnvVars(globalflags.BackendRequireBootstrapFlagName), Destination: &opts.FailIfBucketCreationRequired, Usage: "When this flag is set Terragrunt will fail if the remote state bucket needs to be created.", }, flags.WithDeprecatedEnvVars(terragruntPrefix.EnvVars("fail-on-state-bucket-creation"), terragruntPrefixControl), ), flags.NewFlag(&cli.BoolFlag{ - Name: DisableBucketUpdateFlagName, - EnvVars: tgPrefix.EnvVars(DisableBucketUpdateFlagName), + Name: globalflags.DisableBucketUpdateFlagName, + EnvVars: tgPrefix.EnvVars(globalflags.DisableBucketUpdateFlagName), Destination: &opts.DisableBucketUpdate, Usage: "When this flag is set Terragrunt will not update the remote state bucket.", }, flags.WithDeprecatedEnvVars(terragruntPrefix.EnvVars("disable-bucket-update"), terragruntPrefixControl), ),
660-681: Explicitly set separators and use shared global flag constant
In NewFeatureFlags (cli/commands/run/flags.go), importgithub.com/gruntwork-io/terragrunt/cli/flags/globalasglobalflags, then update your MapFlag to:
- Name:
globalflags.FeatureFlagName- EnvVars:
tgPrefix.EnvVars(globalflags.FeatureFlagName)- Usage:
"Set feature flags for the HCL code (key=value[,key=value]...)"- EnvVarSep:
,- KeyValSep:
=
This makes parsing explicit and reuses the single source‐of‐truth for the “feature” flag.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
cli/commands/backend/bootstrap/cli.go(1 hunks)cli/commands/backend/delete/cli.go(1 hunks)cli/commands/backend/migrate/cli.go(1 hunks)cli/commands/deprecated.go(1 hunks)cli/commands/list/cli.go(2 hunks)cli/commands/run/flags.go(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- cli/commands/list/cli.go
- cli/commands/backend/bootstrap/cli.go
- cli/commands/backend/migrate/cli.go
- cli/commands/backend/delete/cli.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
⚙️ CodeRabbit configuration file
Review the Go code for quality and correctness. Make sure that the Go code follows best practices, is performant, and is easy to understand and maintain.
Files:
cli/commands/deprecated.gocli/commands/run/flags.go
🧠 Learnings (1)
📚 Learning: 2025-04-17T13:02:28.098Z
Learnt from: yhakbar
PR: gruntwork-io/terragrunt#4169
File: cli/commands/hcl/validate/cli.go:29-60
Timestamp: 2025-04-17T13:02:28.098Z
Learning: Avoid shadowing imported packages with local variables in Go code, such as using a variable named `flags` when the `github.com/gruntwork-io/terragrunt/cli/flags` package is imported. Use more specific variable names like `flagSet` instead.
Applied to files:
cli/commands/run/flags.go
🧬 Code graph analysis (2)
cli/commands/deprecated.go (3)
internal/cli/command.go (1)
Command(9-80)cli/commands/run/cli.go (1)
Action(78-92)internal/cli/errors.go (1)
NewExitError(53-69)
cli/commands/run/flags.go (5)
options/options.go (1)
TerragruntOptions(100-332)internal/cli/bool_flag.go (1)
BoolFlag(13-49)cli/flags/global/flags.go (4)
BackendBootstrapFlagName(79-79)BackendRequireBootstrapFlagName(80-80)DisableBucketUpdateFlagName(81-81)FeatureFlagName(84-84)cli/flags/flag_opts.go (1)
WithDeprecatedEnvVars(103-114)internal/cli/map_flag.go (1)
MapFlag(33-71)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
- GitHub Check: build / Build (windows/386)
- GitHub Check: build_no_proxy / Build (darwin/arm64)
- GitHub Check: build_no_proxy / Build (windows/amd64)
- GitHub Check: build_no_proxy / Build (linux/amd64)
- GitHub Check: build_no_proxy / Build (linux/386)
- GitHub Check: build_no_proxy / Build (windows/386)
- GitHub Check: build / Build (linux/arm64)
- GitHub Check: build / Build (windows/amd64)
- GitHub Check: build / Build (linux/386)
- GitHub Check: build_no_proxy / Build (linux/arm64)
- GitHub Check: build_no_proxy / Build (darwin/amd64)
- GitHub Check: build / Build (linux/amd64)
- GitHub Check: build / Build (darwin/amd64)
- GitHub Check: build / Build (darwin/arm64)
- GitHub Check: base_tests / Test (ubuntu)
- GitHub Check: base_tests / Test (macos)
- GitHub Check: Pull Request has non-contributor approval
- GitHub Check: build-and-test
🔇 Additional comments (3)
cli/commands/deprecated.go (1)
179-188: Closure rebinding and fast-fail logic look solidRebinding
controltoctlavoids the loop-variable capture pitfall, and returning the pre-baked guidance viaNewExitErrorcleanly enforces the new “userun --” flow. Nicely done.cli/commands/run/flags.go (2)
599-602: Good: shared flags appended via helpers.Clear separation of concerns and reuse across commands looks good.
660-681: Resolved: removed Splitter mismatch; compile now.Dropping the custom Splitter avoids the compile-time mismatch previously flagged.
docs-starlight/src/content/docs/01-getting-started/04-terminology.md
Outdated
Show resolved
Hide resolved
…ogy.md Co-authored-by: Yousif Akbar <11247449+yhakbar@users.noreply.github.com>
2 participants
Description
Fixes #4599.
TODOs
Read the Gruntwork contribution guidelines.
Release Notes (draft)
Added / Removed / Updated [X].
Migration Guide
Breaking change: default forwarding disabled, users must use run -- for unknown commands.
Summary by CodeRabbit
New Features
Documentation
Tests