chore: Remove legacy terragrunt flags #4650
Conversation
📝 WalkthroughWalkthroughDeprecation wiring was changed from CLI flag-name based to environment-variable based across many CLI flag definitions; tests and calls updated to use public flag constants and standard flags; a strict control gained a Completed status. No public API signatures were changed. Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
yhakbar
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cli/flags/global/flags.go (1)
222-226: Complex flag deprecation wrapper structureThe deprecation approach for the NonInteractive flag combines both environment variable and nested flag deprecations. The nested flag sets
Negative: trueand uses environment variables rather than a flag name, which could be confusing for maintainers.Consider adding a comment to explain why this flag uses a different deprecation pattern:
}, flags.WithDeprecatedEnvVars(terragruntPrefix.EnvVars(DeprecatedNonInteractiveFlagName), terragruntPrefixControl), + // The tf-input flag is inverted (negative) compared to non-interactive flags.WithDeprecatedFlag(&cli.BoolFlag{ Negative: true, EnvVars: flags.Prefix{}.EnvVars(DeprecatedTFInputFlagName), }, nil, terragruntPrefixControl)),
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
cli/commands/aws-provider-patch/cli.go(1 hunks)cli/commands/hcl/format/cli.go(5 hunks)cli/commands/hcl/validate/cli.go(3 hunks)cli/commands/render/cli.go(3 hunks)cli/commands/run/flags.go(16 hunks)cli/flags/global/flags.go(11 hunks)internal/strict/controls/controls.go(1 hunks)
🧰 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:
internal/strict/controls/controls.gocli/commands/aws-provider-patch/cli.gocli/commands/render/cli.gocli/flags/global/flags.gocli/commands/hcl/format/cli.gocli/commands/hcl/validate/cli.gocli/commands/run/flags.go
🧠 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/commands/aws-provider-patch/cli.gocli/commands/render/cli.gocli/flags/global/flags.gocli/commands/hcl/format/cli.gocli/commands/hcl/validate/cli.gocli/commands/run/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/run/flags.go
📚 Learning: 2025-03-06T23:44:09.413Z
Learnt from: partcyborg
PR: gruntwork-io/terragrunt#3974
File: config/config_partial.go:448-456
Timestamp: 2025-03-06T23:44:09.413Z
Learning: The TerragruntConfig struct in config/config.go does contain an Engine field that's used to store engine configuration data.
Applied to files:
cli/commands/run/flags.go
🧬 Code Graph Analysis (7)
internal/strict/controls/controls.go (1)
internal/strict/status.go (1)
CompletedStatus(9-9)
cli/commands/aws-provider-patch/cli.go (1)
cli/flags/flag_opts.go (1)
WithDeprecatedEnvVars(156-167)
cli/commands/render/cli.go (1)
cli/flags/flag_opts.go (1)
WithDeprecatedEnvVars(156-167)
cli/flags/global/flags.go (4)
cli/flags/flag_opts.go (1)
WithDeprecatedEnvVars(156-167)cli/flags/flag.go (1)
NewFlag(28-41)internal/cli/bool_flag.go (1)
BoolFlag(13-49)internal/cli/generic_flag.go (1)
GenericFlag(19-48)
cli/commands/hcl/format/cli.go (1)
cli/flags/flag_opts.go (1)
WithDeprecatedEnvVars(156-167)
cli/commands/hcl/validate/cli.go (1)
cli/flags/flag_opts.go (1)
WithDeprecatedEnvVars(156-167)
cli/commands/run/flags.go (5)
cli/flags/flag_opts.go (1)
WithDeprecatedEnvVars(156-167)cli/flags/flag.go (1)
NewFlag(28-41)internal/cli/generic_flag.go (1)
GenericFlag(19-48)internal/cli/bool_flag.go (1)
BoolFlag(13-49)internal/cli/slice_flag.go (1)
SliceFlag(23-58)
⏰ 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). (2)
- GitHub Check: Build (darwin/amd64)
- GitHub Check: Build (linux/386)
🔇 Additional comments (20)
internal/strict/controls/controls.go (1)
166-166: LGTM! Status correctly reflects completed migration.The addition of
Status: strict.CompletedStatusto theTerragruntPrefixFlagscontrol aligns with the PR objective of removing legacy--terragrunt-prefixed flags. This status change correctly indicates that the migration away from these deprecated flags has been completed.cli/commands/aws-provider-patch/cli.go (1)
61-61: LGTM! Deprecation migration correctly implemented.The change from
WithDeprecatedNametoWithDeprecatedEnvVarsproperly shifts the deprecation handling from flag-name-based to environment-variable-based, targeting theTERRAGRUNT_OVERRIDE_ATTRenvironment variable. This aligns with the broader PR objective of removing legacy--terragrunt-prefixed flags.cli/commands/hcl/format/cli.go (5)
36-36: LGTM! Environment variable deprecation correctly added.The addition of
WithDeprecatedEnvVars(terragruntPrefix.EnvVars("hclfmt-file"), terragruntPrefixControl)properly deprecates theTERRAGRUNT_HCLFMT_FILEenvironment variable while maintaining the existingTG_HCLFMT_FILEdeprecation.
46-46: LGTM! Environment variable deprecation correctly added.The addition properly deprecates the
TERRAGRUNT_HCLFMT_EXCLUDE_DIRenvironment variable while maintaining backward compatibility.
56-56: LGTM! Environment variable deprecation correctly added.The addition properly deprecates the
TERRAGRUNT_CHECKenvironment variable while maintaining backward compatibility.
66-66: LGTM! Environment variable deprecation correctly added.The addition properly deprecates the
TERRAGRUNT_DIFFenvironment variable while maintaining backward compatibility.
76-76: LGTM! Environment variable deprecation correctly added.The addition properly deprecates the
TERRAGRUNT_HCLFMT_STDINenvironment variable while maintaining backward compatibility.cli/commands/render/cli.go (3)
89-89: LGTM! Environment variable deprecation correctly added.The addition of
WithDeprecatedEnvVars(terragruntPrefix.EnvVars("json-out"), terragruntPrefixControl)properly deprecates theTERRAGRUNT_JSON_OUTenvironment variable while maintaining the existingTG_RENDER_JSON_OUTdeprecation.
99-99: LGTM! Environment variable deprecation correctly added.The addition properly deprecates the
TERRAGRUNT_WITH_METADATAenvironment variable while maintaining backward compatibility.
109-109: LGTM! Environment variable deprecation correctly added.The addition properly deprecates the
TERRAGRUNT_JSON_DISABLE_DEPENDENT_MODULESenvironment variable while maintaining backward compatibility.cli/commands/hcl/validate/cli.go (3)
38-38: LGTM! Environment variable deprecation correctly added.The addition of
WithDeprecatedEnvVars(terragruntPrefix.EnvVars("strict-validate"), terragruntPrefixControl)properly deprecates theTERRAGRUNT_STRICT_VALIDATEenvironment variable while maintaining existingTG_*deprecations.
55-55: LGTM! Environment variable deprecation correctly added.The addition properly deprecates the
TERRAGRUNT_HCLVALIDATE_SHOW_CONFIG_PATHenvironment variable while maintaining backward compatibility.
65-65: LGTM! Environment variable deprecation correctly added.The addition properly deprecates the
TERRAGRUNT_HCLVALIDATE_JSONenvironment variable while maintaining backward compatibility.cli/flags/global/flags.go (2)
307-309: Preserving the typo for backward compatibilityThe environment variable deprecation includes both the correctly spelled ("telemetry-trace-exporter") and misspelled ("telemerty-trace-exporter") variants, which is appropriate for backward compatibility. This pattern is consistently applied across all telemetry-related flags.
143-226: Consistent migration from flag-based to env-var-based deprecationsAll deprecated flags in the global flags file have been successfully migrated from
WithDeprecatedNames/WithDeprecatedNametoWithDeprecatedEnvVars. The changes are consistent and align with the PR's objective to remove legacy--terragrunt-prefixed flags.cli/commands/run/flags.go (5)
205-211: Correctly consolidating multiple deprecated environment variablesThe implementation properly combines both "download" and "download-dir" environment variables into a single deprecation call, ensuring backward compatibility for both legacy names.
167-169: Nested deprecated flags using environment variables onlyThe deprecated flags for
auto-init,auto-retry, andauto-approvenow useEnvVarswithoutNamefields. This is a valid pattern for environment-variable-only deprecations but differs from the typical flag deprecation approach.Also applies to: 178-180, 189-191
379-381: Double deprecation for ForwardTFStdout flagThe flag has two
WithDeprecatedEnvVarscalls - one for "forward-tf-stdout" and another for "include-module-prefix" withlegacyLogsControl. This suggests that "include-module-prefix" was an older name that's being deprecated alongside the general terragrunt prefix migration.
105-638: Comprehensive and consistent migration to env-var-based deprecationsAll flags in the run command have been successfully migrated from flag-name-based deprecations to environment-variable-based deprecations. The changes are systematic and align with the PR objectives to remove legacy
--terragrunt-prefixed flags. The implementation properly handles various deprecation patterns including single flags, multiple legacy names, and nested deprecations.
269-277: Verify deprecated IAM Web Identity Token env var precedenceThe order of the
envVarsslice passed toWithDeprecatedEnvVarsdetermines which deprecated environment variable is recognized first. In cli/commands/run/flags.go (lines 269–277), you’re appending:
iam-web-identity-tokeniam-assume-role-web-identity-tokenIf your deprecation handler picks the first matching var, the more generic
iam-web-identity-tokenwill always take precedence over the more specific one. Please confirm that this matches the intended behavior—otherwise swap the append arguments so thatiam-assume-role-web-identity-tokencomes first.• File: cli/commands/run/flags.go
Lines: 269–277
Snippet:flags.WithDeprecatedEnvVars( append( terragruntPrefix.EnvVars("iam-web-identity-token"), terragruntPrefix.EnvVars("iam-assume-role-web-identity-token")..., ), terragruntPrefixControl, ),
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
test/fixtures/destroy-order/hello/main.tf (1)
7-7: Stabilize provider version to avoid flaky testsUsing a broad lower-bound constraint can introduce nondeterminism as new major releases of hashicorp/null ship. Consider pinning or upper-bounding to keep fixtures stable across time and CI environments.
Apply this change to add an upper bound:
- version = ">= 3.0.0" + version = ">= 3.0.0, < 4.0.0"If you prefer maximal determinism, pin to the exact version used in other fixtures instead.
test/integration_regressions_test.go (1)
96-99: Assert against stderr instead of err.Error for stability.RunTerragruntCommandWithOutput returns stderr; asserting on that is more robust than matching the error string (which can include wrappers and change formatting).
- _, _, err := helpers.RunTerragruntCommandWithOutput(t, "terragrunt plan --non-interactive --working-dir "+rootPath) + _, stderr, err := helpers.RunTerragruntCommandWithOutput(t, "terragrunt plan --non-interactive --working-dir "+rootPath) require.Error(t, err) - assert.Contains(t, err.Error(), "include blocks without label") + assert.Contains(t, stderr, "include blocks without label")
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
cli/app_test.go(4 hunks)cli/commands/render/cli.go(3 hunks)test/fixtures/destroy-order/hello/main.tf(1 hunks)test/integration_debug_test.go(3 hunks)test/integration_deprecated_test.go(4 hunks)test/integration_regressions_test.go(1 hunks)test/integration_runner_pool_test.go(1 hunks)test/integration_test.go(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cli/commands/render/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:
test/integration_regressions_test.gotest/integration_runner_pool_test.gotest/integration_debug_test.gotest/integration_deprecated_test.gocli/app_test.gotest/integration_test.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:
test/integration_deprecated_test.gocli/app_test.go
🧬 Code Graph Analysis (5)
test/integration_regressions_test.go (1)
test/helpers/package.go (1)
RunTerragruntCommandWithOutput(935-939)
test/integration_debug_test.go (1)
test/helpers/package.go (1)
RunTerragruntValidateInputs(968-990)
test/integration_deprecated_test.go (1)
test/helpers/package.go (2)
RunTerragruntCommandWithOutput(935-939)RunTerragrunt(907-911)
cli/app_test.go (3)
cli/commands/run/flags.go (4)
QueueIncludeExternalFlagName(64-64)QueueExcludeExternalFlagName(60-60)IAMAssumeRoleFlagName(51-51)IAMAssumeRoleWebIdentityTokenFlagName(54-54)config/config.go (1)
DefaultTerragruntConfigPath(47-47)cli/commands/aws-provider-patch/cli.go (1)
OverrideAttrFlagName(46-46)
test/integration_test.go (2)
test/helpers/package.go (1)
RunTerragruntCommandWithOutputWithContext(923-933)util/file.go (1)
JoinPath(481-483)
⏰ 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). (1)
- GitHub Check: Pull Request has non-contributor approval
🔇 Additional comments (17)
cli/app_test.go (6)
80-80: Correct migration from legacy flag naming system to modern flag constants.The replacement of
terragruntPrefix.FlagName(run.QueueIncludeExternalFlagName)withrun.QueueIncludeExternalFlagNamecorrectly aligns with the PR objective to remove legacy--terragrunt-prefixed flags.
110-110: Correct migration from legacy flag naming system to modern flag constants.The replacement of
terragruntPrefix.FlagName(run.QueueExcludeExternalFlagName)withrun.QueueExcludeExternalFlagNamecorrectly aligns with the PR objective to remove legacy--terragrunt-prefixed flags.
115-115: Correct migration from legacy flag naming system to modern flag constants.The replacement of
terragruntPrefix.FlagName(run.IAMAssumeRoleFlagName)withrun.IAMAssumeRoleFlagNamecorrectly aligns with the PR objective to remove legacy--terragrunt-prefixed flags.
130-130: Correct migration from legacy flag naming system to modern flag constants.The replacement of
terragruntPrefix.FlagName(run.IAMAssumeRoleWebIdentityTokenFlagName)withrun.IAMAssumeRoleWebIdentityTokenFlagNamecorrectly aligns with the PR objective to remove legacy--terragrunt-prefixed flags.
383-383: Correct migration from legacy flag naming system to modern flag constants.The replacement of
terragruntPrefix.FlagName(awsproviderpatch.OverrideAttrFlagName)withawsproviderpatch.OverrideAttrFlagNamecorrectly aligns with the PR objective to remove legacy--terragrunt-prefixed flags.
469-482: No changes needed: legacy flag removal test is correctThe assertions in
cli/app_test.go(lines 469–482) correctly use
terragruntPrefix.FlagName(awsproviderpatch.OverrideAttrFlagName)
to generate--terragrunt-override-attrand verify it does not appear in the help output for--help,-help, and-h. Verified thatflags.Prefix.FlagNameproduces the expected flag string and there are no stray legacy flag definitions.test/integration_deprecated_test.go (3)
32-32: LGTM: Updated flags on deprecated hclvalidate invocationSwitch to modern flags (--working-dir, --json, --show-config-path) while intentionally exercising the deprecated single-word command looks correct and keeps the test focused on the command alias.
71-71: LGTM: Pass-through subcommand coverage preservedUsing the modern global flags with the passthrough terraform subcommand scenarios maintains intent and test robustness.
114-114: LGTM: render-json test uses modern flagsUsing --with-metadata and --json-out on the deprecated render-json command keeps the test aligned with the new flag semantics.
test/integration_debug_test.go (3)
106-108: Align all tests on --strict (one remaining uses --strict-validate)Good switch to --strict here. There’s still a use of --strict-validate in TestTerragruntValidateInputsWithStrictMode (Lines ~134-137). For consistency with the CLI redesign, update that to --strict as well.
Do you want me to submit a follow-up patch updating that test case too?
151-152: LGTM: Strict mode behavior covered with unused varThe transition to --strict aligns with the new flag and keeps the negative test meaningful.
163-164: LGTM: Strict mode behavior covered with unused inputsModern flag usage looks correct; scenario remains valid.
test/integration_test.go (3)
234-235: LGTM: Switched to run-based CLI for detailed exit code flowThe new invocation style preserves the detailed-exitcode behavior while aligning with the modern run CLI.
255-257: LGTM: Consistent plan invocation post-applyUsing run --all with plan -detailed-exitcode keeps the semantics intact under the new CLI.
276-278: LGTM: No-changes path updated to modern CLIThe updated invocation is correct; assertions remain meaningful.
test/integration_regressions_test.go (2)
96-96: Modern flags usage LGTM.Switching to --non-interactive and --working-dir aligns with the removal of legacy --terragrunt-* flags and the migration guide.
96-96: Remove all remaining legacy--terragrunt-flags
I still see numerous occurrences of deprecated CLI flags and aliases that need to be updated or removed per the PR objective.Please address the following locations (non-exhaustive list):
- Code handling deprecated flags:
- tflint/tflint.go:28
- internal/strict/controls/deprecated_flag_name.go (lines 44, 71)
- config/config_helpers.go (lines 357, 361)
- config/config.go (lines 988, 995)
- cli/app.go (lines 130–131)
- Tests exercising legacy flags:
- config/config_helpers_test.go (params using
--terragrunt-quiet,--terragrunt-global-cache)- cli/app_test.go:144
- Documentation and migration guides (all references to
--terragrunt-*):
- docs-starlight/src/content/docs/**
- docs/_docs/06_migration_guides/**
- docs/_docs/04_reference/02-cli-options.md (many CLI Arg Alias entries)
- Other
.md/.mdxfiles underdocs/_docsanddocs-starlightEnsure each deprecated flag:
- Is removed from usage examples and fixtures
- Updating docs to reference the new flags (e.g.
--non-interactive,run --all)- Cleaning up any parsing logic or aliases that no longer need to exist
⛔ 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.
| stderr := bytes.Buffer{} | ||
|
|
||
| err := helpers.RunTerragruntCommand(t, "terragrunt terragrunt-info --terragrunt-non-interactive --terragrunt-working-dir "+testPath, &stdout, &stderr) | ||
| err := helpers.RunTerragruntCommand(t, "terragrunt terragrunt-info --non-interactive --working-dir "+testPath, &stdout, &stderr) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm ongoing support of 'terragrunt-info' alias, and consider adding a new-CLI companion test
This test intentionally validates the legacy 'terragrunt-info' alias. Given this PR removes legacy terragrunt- prefixed flags (not commands), ensure the alias remains supported; consider adding a companion test that uses the modern 'info print' path to guard both behaviors.
You can verify usages across the repo and confirm both forms are present with:
🏁 Script executed:
#!/bin/bash
rg -n $'^\\s*terragrunt\\s+terragrunt-info\\b' -A2 -S
rg -n $'^\\s*terragrunt\\s+info\\s+print\\b' -A2 -SLength of output: 441
Add companion integration test for the modern info print CLI
The existing test in test/integration_deprecated_test.go at line 97 validates the legacy alias (terragrunt terragrunt-info), which remains supported (confirmed in docs/_docs/04_reference/02-cli-options.md). To ensure we guard the new syntax as well, add a companion test immediately after the current one:
testPath := filepath.Join("fixtures", "deprecated", "simple")
- err := helpers.RunTerragruntCommand(t, "terragrunt terragrunt-info --non-interactive --working-dir "+testPath, &stdout, &stderr)
+ // Legacy alias
+ err := helpers.RunTerragruntCommand(t, "terragrunt terragrunt-info --non-interactive --working-dir "+testPath, &stdout, &stderr)
+ require.NoError(t, err)
+
+ // Modern syntax
+ err = helpers.RunTerragruntCommand(t, "terragrunt info print --non-interactive --working-dir "+testPath, &stdout, &stderr)
+ require.NoError(t, err)This will validate both the old alias and the new info print command path.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| err := helpers.RunTerragruntCommand(t, "terragrunt terragrunt-info --non-interactive --working-dir "+testPath, &stdout, &stderr) | |
| testPath := filepath.Join("fixtures", "deprecated", "simple") | |
| // Legacy alias | |
| err := helpers.RunTerragruntCommand(t, "terragrunt terragrunt-info --non-interactive --working-dir "+testPath, &stdout, &stderr) | |
| require.NoError(t, err) | |
| // Modern syntax | |
| err = helpers.RunTerragruntCommand(t, "terragrunt info print --non-interactive --working-dir "+testPath, &stdout, &stderr) | |
| require.NoError(t, err) |
🤖 Prompt for AI Agents
In test/integration_deprecated_test.go around line 97, add a companion
integration test immediately after the existing legacy check that runs the
modern CLI form; duplicate the existing call to helpers.RunTerragruntCommand but
replace the command string "terragrunt terragrunt-info --non-interactive
--working-dir "+testPath with "terragrunt info print --non-interactive
--working-dir "+testPath, wiring the same stdout/stderr variables and the same
assertions so the new syntax is exercised and validated alongside the legacy
alias.
| var destroyOrder []string | ||
| re := regexp.MustCompile(`Hello, Module ([A-Za-z]+)`) | ||
| for _, line := range strings.Split(stdout, "\n") { | ||
| for line := range strings.SplitSeq(stdout, "\n") { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
strings.SplitSeq requires a recent Go toolchain; verify go.mod/go version in CI
The streaming approach with strings.SplitSeq is a nice memory improvement, but it depends on newer Go (range-over-func additions). Please confirm the repo’s go.mod and CI use a Go version that provides strings.SplitSeq.
Run this to confirm the Go version and surface all usages:
If older Go versions must be supported, a safe fallback is bufio.Scanner:
scanner := bufio.NewScanner(strings.NewReader(stdout))
for scanner.Scan() {
line := scanner.Text()
// existing logic
}
require.NoError(t, scanner.Err())🏁 Script executed:
#!/bin/bash
echo "go.mod version:"
rg -n '^go\\s+\\d+\\.\\d+' go.mod
echo
echo "Occurrences of strings.SplitSeq:"
rg -n 'strings\\.SplitSeq\\(' -A2Length of output: 288
I couldn’t detect the Go version or any occurrences because of a regex escape issue. Let’s re-run with simpler patterns:
🏁 Script executed:
#!/bin/bash
set -e
echo "go.mod version:"
# Match the 'go X.Y' directive
grep -E '^go [0-9]+\.[0-9]+' go.mod || echo "No go version directive found in go.mod"
echo
echo "Occurrences of strings.SplitSeq:"
# Look for any use of strings.SplitSeq(
rg -n 'strings\.SplitSeq\(' --color=never || echo "No occurrences of strings.SplitSeq found"Length of output: 1671
We need to confirm that CI is using Go 1.24+ (which includes strings.SplitSeq). Let’s check your GitHub Actions (or other CI) setup for the Go version:
🏁 Script executed:
#!/bin/bash
set -e
echo "Workflow files:"
find .github/workflows -maxdepth 1 -type f -name '*.yml' -print || echo "No workflows found"
echo
echo "Looking for actions/setup-go usage and go-version in workflows:"
rg -n 'uses:\s*actions/setup-go' -A2 .github/workflows || echo "No setup-go steps detected"
rg -n 'go-version:' -A1 .github/workflows || echo "No go-version fields detected"
echo
echo "Checking other common CI files for Go version:"
for file in .travis.yml azure-pipelines.yml circle.yml; do
if [ -f "$file" ]; then
echo "In $file:"
rg -n 'go_version|go-version|go:' -A1 "$file" || echo " No Go version directive in $file"
fi
doneLength of output: 1415
Pin Go Version in CI Workflows
go.mod already targets Go 1.24.4 (which includes strings.SplitSeq) and you’re using SplitSeq in many places (tests and code). However, I didn’t find any explicit Go setup (actions/setup-go or Docker container) in your .github/workflows/*.yml, so CI may be running on an older Go toolchain that lacks SplitSeq support.
• Add a “Set up Go” step in each workflow, for example:
- name: Set up Go
uses: actions/setup-go@v4
with:
go-version: '1.24'• If you must support Go < 1.21, swap the SplitSeq loops for a bufio.Scanner fallback:
scanner := bufio.NewScanner(strings.NewReader(stdout))
for scanner.Scan() {
line := scanner.Text()
// …
}
require.NoError(t, scanner.Err())🤖 Prompt for AI Agents
In test/integration_runner_pool_test.go around line 70, the test uses
strings.SplitSeq(stdout, "\n") which requires Go 1.24+; update your CI workflows
to explicitly set up Go 1.24 (e.g. add an actions/setup-go@v4 step with
go-version: '1.24') so SplitSeq is available, or if you must support older Go
toolchains change the test to use a bufio.Scanner over strings.NewReader(stdout)
and iterate scanner.Text() with a final check for scanner.Err() instead of
SplitSeq.
|
Noticed references for |
We're deferring updates to comments to a future PR. |
| Negative: true, | ||
| Destination: &opts.AutoInit, | ||
| }, | ||
| flags.WithDeprecatedNames(terragruntPrefix.FlagNames("no-auto-init"), terragruntPrefixControl), |
There was a problem hiding this comment.
should we include no-auto-init, no-auto-retry and no-auto-approve in the #4671 table?
There was a problem hiding this comment.
Will add after we merge that one
Description
Fixes #4597.
TODOs
Read the Gruntwork contribution guidelines.
Release Notes (draft)
Removed support for legacy
--terragrunt-prefixed flags. Only modern flag names are supported going forward.For more information, see the migration guide.
Migration Guide
Summary by CodeRabbit