fix: Adjusting awshelper
#4502
fix: Adjusting awshelper
#4502
Conversation
📝 WalkthroughWalkthroughThis update migrates all AWS SDK usage from v1 to v2 throughout the codebase, refactors AWS session and credential management into a new internal package, and updates all related client creation, error handling, and test code accordingly. Numerous function signatures change to accept context parameters, and all S3 and DynamoDB operations are adapted for the v2 SDK. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TerragruntCLI
participant AWSHelper (internal)
participant AWS SDK v2
participant AWS Services
User->>TerragruntCLI: Run command requiring AWS
TerragruntCLI->>AWSHelper (internal): CreateAwsConfig(ctx, config, opts)
AWSHelper (internal)->>AWS SDK v2: Load config, credentials, assume role if needed
AWS SDK v2->>AWS Services: Validate credentials (STS GetCallerIdentity)
AWS Services-->>AWS SDK v2: Caller identity/validation response
AWS SDK v2-->>AWSHelper (internal): Config/credentials or error
AWSHelper (internal)-->>TerragruntCLI: Return AWS config or error
TerragruntCLI->>AWSHelper (internal): CreateS3Client(ctx, config, opts)
AWSHelper (internal)->>AWS SDK v2: Initialize S3 client
AWS SDK v2-->>AWSHelper (internal): S3 client instance
AWSHelper (internal)-->>TerragruntCLI: S3 client ready for use
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes
Possibly related PRs
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.go⚙️ CodeRabbit Configuration File
Files:
🧬 Code Graph Analysis (1)internal/awshelper/config.go (3)
⏰ 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). (6)
✨ 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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
yhakbar
force-pushed
the
fix/adjusting-awshelper
branch
from
3f10642 to
a1aeb82
Compare
yhakbar
force-pushed
the
fix/adjusting-awshelper
branch
from
12a797d to
5346066
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🔭 Outside diff range comments (13)
telemetry/tracer.go (1)
157-157: Wrap external package errors for better context.The linter is flagging unwrapped errors from external packages. Consider wrapping these errors with additional context for better debugging.
Apply these fixes to address the pipeline failures:
- return otlptracehttp.New(ctx, config...) + exporter, err := otlptracehttp.New(ctx, config...) + if err != nil { + return nil, errors.Errorf("failed to create OTLP HTTP trace exporter: %w", err) + } + return exporter, nil- return otlptracehttp.New(ctx, config...) + exporter, err := otlptracehttp.New(ctx, config...) + if err != nil { + return nil, errors.Errorf("failed to create OTLP HTTP trace exporter: %w", err) + } + return exporter, nil- return otlptracegrpc.New(ctx, config...) + exporter, err := otlptracegrpc.New(ctx, config...) + if err != nil { + return nil, errors.Errorf("failed to create OTLP gRPC trace exporter: %w", err) + } + return exporter, nil- return stdouttrace.New(stdouttrace.WithWriter(writer)) + exporter, err := stdouttrace.New(stdouttrace.WithWriter(writer)) + if err != nil { + return nil, errors.Errorf("failed to create stdout trace exporter: %w", err) + } + return exporter, nilAlso applies to: 164-164, 171-171, 173-173
internal/cli/map_flag.go (2)
202-203: Fix error wrapping to address linting failureThe pipeline failure indicates that errors from interface methods should be wrapped for better error handling.
Apply this diff to wrap the error:
- if err := key.Set(strings.TrimSpace(parts[0])); err != nil { - return err - } + if err := key.Set(strings.TrimSpace(parts[0])); err != nil { + return errors.Errorf("failed to set key: %w", err) + }
207-208: Fix error wrapping to address linting failureSimilar to the key setting, this error also needs to be wrapped for better error handling.
Apply this diff to wrap the error:
- if err := val.Set(strings.TrimSpace(parts[1])); err != nil { - return err - } + if err := val.Set(strings.TrimSpace(parts[1])); err != nil { + return errors.Errorf("failed to set value: %w", err) + }cli/commands/render/render_test.go (1)
113-113: Fix comment formatting to satisfy linter.Comments should end with periods according to the project's linting rules.
Apply this diff to fix the linting issues:
-// setupTest creates a temporary directory with a terragrunt config file and returns the necessary test setup +// setupTest creates a temporary directory with a terragrunt config file and returns the necessary test setup.-// validateRenderedJSON validates the common JSON structure and values +// validateRenderedJSON validates the common JSON structure and values.Also applies to: 128-128
internal/cli/slice_flag.go (1)
173-173: Wrap the returned error for better error handling.The linter indicates that errors returned from interface methods should be wrapped to provide better context and stack traces.
Apply this diff to wrap the error:
- if err := value.Set(str); err != nil { - return err - } + if err := value.Set(str); err != nil { + return fmt.Errorf("failed to set slice flag value: %w", err) + }You'll also need to add the
fmtimport if it's not already present:import ( libflag "flag" + "fmt" "os" "strings"internal/cli/generic_flag.go (1)
142-142: Fix linting error: Wrap returned error for better error handling.The pipeline failure indicates that the error returned from the interface method should be wrapped for better error handling context.
Apply this diff to wrap the error:
- if err := flag.value.Set(str); err != nil { - return err - } + if err := flag.value.Set(str); err != nil { + return fmt.Errorf("failed to set generic flag value: %w", err) + }You'll also need to add
"fmt"to the imports if it's not already there.internal/runner/configstack/runner.go (1)
611-611: Wrap error for better context.The pipeline failure indicates that the error returned from an external package should be wrapped for better context.
Apply this diff to wrap the error:
- return aws.Config{}, errors.New(err) + return aws.Config{}, errors.Errorf("failed to get path relative to root: %w", err)test/integration_aws_test.go (1)
1900-1914: Consider improving error handling instead of suppressing errors.Returning
nil, nilon error hides potentially important error information. Consider either:
- Properly propagating the error
- Checking for specific expected errors (like "ServerSideEncryptionConfigurationNotFoundError")
- Adding a comment explaining why errors are intentionally ignored
func bucketEncryption(t *testing.T, awsRegion string, bucketName string) (*s3.GetBucketEncryptionOutput, error) { t.Helper() client := helpers.CreateS3ClientForTest(t, awsRegion) ctx := t.Context() input := &s3.GetBucketEncryptionInput{Bucket: aws.String(bucketName)} output, err := client.GetBucketEncryption(ctx, input) if err != nil { - // TODO: Remove this lint suppression - return nil, nil //nolint:nilerr + var apiErr smithy.APIError + if errors.As(err, &apiErr) && apiErr.ErrorCode() == "ServerSideEncryptionConfigurationNotFoundError" { + // No encryption configuration is expected in some test cases + return nil, nil + } + return nil, err } return output, nil }internal/remotestate/backend/s3/client.go (5)
174-174: Fix typo in user prompt messageThere's a typo in the prompt message.
- prompt := fmt.Sprintf("Remote state S3 bucket %s is res of date. Would you like Terragrunt to update it?", bucketName) + prompt := fmt.Sprintf("Remote state S3 bucket %s is out of date. Would you like Terragrunt to update it?", bucketName)
1084-1084: Wrap error from AWS SDK for better contextThe error from the AWS SDK should be wrapped to provide better context.
- return ctx.Err() + return errors.Errorf("context cancelled while deleting S3 bucket objects: %w", ctx.Err())
1116-1116: Wrap error from AWS SDK for better contextThe error from the AWS SDK should be wrapped to provide better context.
- return ctx.Err() + return errors.Errorf("context cancelled while deleting S3 bucket version objects: %w", ctx.Err())
1195-1195: Wrap error from AWS SDK for better contextThe error from the AWS SDK should be wrapped to provide better context.
- return ctx.Err() + return errors.Errorf("context cancelled while waiting for S3 bucket deletion: %w", ctx.Err())
1253-1253: Wrap error for better contextThe error should be wrapped to provide better context about the operation.
- return false, err + return false, errors.Errorf("failed to check if S3 object exists: %w", err)
🧹 Nitpick comments (5)
test/helpers/aws.go (1)
16-24: Make the retry loop explicit to aid readability & Go version compatibilityThe new
for range 3syntax relies on Go 1.22+. Although the project already uses this idiom (per prior PRs), an explicit counter is still clearer to many readers and works on earlier toolchains that may linger in CI images.- for range 3 { + const maxRetries = 3 // keep near the logic for easy tuning + for attempt := 0; attempt < maxRetries; attempt++ { err := DeleteS3Bucket(t, awsRegion, bucketName) if err == nil { return } - t.Logf("Error deleting s3 bucket %s. Sleeping for 10 seconds before retrying.", bucketName) + t.Logf("Error deleting S3 bucket %s (attempt %d/%d). Sleeping 10 s before retrying.", + bucketName, attempt+1, maxRetries) time.Sleep(10 * time.Second) //nolint:mnd }Benefits
• Easier to scan (the loop count is named)
• Logs show attempt numbers, simplifying troubleshooting
• Gracefully degrades if someone builds with < 1.22If the team is fully standardized on Go 1.22+, feel free to keep the original, but consider the logging tweak at least.
test/integration_test.go (1)
3928-3930: Consider parameterising the expected Terraform source version instead of hard-codingv0.83.2The test will fail again the next time the fixture is bumped.
Extract the version into a constant shared with the fixture generator (or match with\bv\d+\.\d+\.\d+\b) to make the assertion future-proof.cli/commands/run/download_source_test.go (1)
484-484: Fix linting issues: Add periods to comment endings.The pipeline failures indicate that comments should end with periods according to the
godotlinter.Apply this fix to add periods to comment endings:
-// TestUpdateGettersExcludeFromCopy verifies the correct behavior of updateGetters with ExcludeFromCopy configuration +// TestUpdateGettersExcludeFromCopy verifies the correct behavior of updateGetters with ExcludeFromCopy configuration. -// TestDownloadSourceWithCASExperimentDisabled tests that CAS is not used when the experiment is disabled +// TestDownloadSourceWithCASExperimentDisabled tests that CAS is not used when the experiment is disabled. -// TestDownloadSourceWithCASExperimentEnabled tests that CAS is attempted when the experiment is enabled +// TestDownloadSourceWithCASExperimentEnabled tests that CAS is attempted when the experiment is enabled. -// TestDownloadSourceWithCASGitSource tests CAS functionality with a Git source +// TestDownloadSourceWithCASGitSource tests CAS functionality with a Git source. -// TestDownloadSourceCASInitializationFailure tests the fallback behavior when CAS initialization fails +// TestDownloadSourceCASInitializationFailure tests the fallback behavior when CAS initialization fails. -// TestDownloadSourceWithCASMultipleSources tests that CAS works with multiple different sources +// TestDownloadSourceWithCASMultipleSources tests that CAS works with multiple different sources.Also applies to: 538-538, 580-580, 622-622, 663-663, 704-704
internal/awshelper/config.go (1)
228-280: Consider consolidating CreateAwsConfig functionsThere are two similar functions
CreateAwsConfigandCreateAwsConfigFromConfig. The mainCreateAwsConfigfunction essentially delegates toCreateAwsConfigFromConfigwhenawsCfgis provided. Consider whether these could be consolidated or if the separation provides value.The current implementation has some duplication in credential handling logic between the two branches. Consider extracting the common credential setup logic into a helper function to reduce duplication and improve maintainability.
internal/remotestate/backend/s3/client.go (1)
707-715: Consider the retry logic for unknown errorsThe error handling is correctly migrated to use
smithy.APIError. However, returningtruefor unknown errors could lead to unnecessary retries for permanent failures.Consider being more conservative with unknown errors:
func isBucketErrorRetriable(err error) bool { var apiErr smithy.APIError if errors.As(err, &apiErr) { return apiErr.ErrorCode() == "InternalError" || apiErr.ErrorCode() == "OperationAborted" || apiErr.ErrorCode() == "InvalidParameter" } - // If it's not a known AWS error, assume it's retriable - return true + // If it's not a known AWS error, don't retry by default + return false }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (71)
awshelper/config.go(0 hunks)cli/commands/commands.go(1 hunks)cli/commands/help/cli.go(1 hunks)cli/commands/info/strict/command.go(1 hunks)cli/commands/render/render_test.go(3 hunks)cli/commands/run/creds/providers/amazonsts/provider.go(3 hunks)cli/commands/run/download_source_test.go(5 hunks)cli/flags/flag.go(1 hunks)cli/flags/flag_opts.go(1 hunks)config/config.go(2 hunks)config/config_helpers.go(6 hunks)config/config_test.go(1 hunks)config/dependency.go(5 hunks)config/hclparse/file.go(2 hunks)go.mod(2 hunks)internal/awshelper/config.go(1 hunks)internal/awshelper/config_test.go(3 hunks)internal/awshelper/policy_test.go(2 hunks)internal/cli/autocomplete.go(1 hunks)internal/cli/bool_flag.go(2 hunks)internal/cli/command.go(3 hunks)internal/cli/flag.go(3 hunks)internal/cli/generic_flag.go(1 hunks)internal/cli/help.go(4 hunks)internal/cli/map_flag.go(1 hunks)internal/cli/slice_flag.go(1 hunks)internal/providercache/provider_cache.go(1 hunks)internal/remotestate/backend/s3/backend.go(6 hunks)internal/remotestate/backend/s3/client.go(51 hunks)internal/remotestate/backend/s3/client_test.go(5 hunks)internal/remotestate/backend/s3/remote_state_config.go(4 hunks)internal/remotestate/backend/s3/remote_state_config_test.go(5 hunks)internal/remotestate/backend/s3/retryer.go(1 hunks)internal/runner/configstack/runner.go(1 hunks)internal/runner/runner.go(2 hunks)pkg/log/logger.go(1 hunks)telemetry/telemeter.go(1 hunks)telemetry/tracer.go(1 hunks)test/fixtures/auth-provider-cmd/sops/secrets.json(1 hunks)test/fixtures/download/hello-world-with-backend/main.tf(1 hunks)test/fixtures/download/invalid-path/terragrunt.hcl(1 hunks)test/fixtures/download/local-windows/JZwoL6Viko8bzuRvTOQFx3Jh8vs/3mU4huxMLOXOW5ZgJOFXGUFDKc8/main.tf(1 hunks)test/fixtures/download/remote-with-backend/terragrunt.hcl(1 hunks)test/fixtures/get-output/regression-906/a/terragrunt.hcl(1 hunks)test/fixtures/get-output/regression-906/b/terragrunt.hcl(1 hunks)test/fixtures/get-output/regression-906/c/terragrunt.hcl(1 hunks)test/fixtures/get-output/regression-906/d/terragrunt.hcl(1 hunks)test/fixtures/get-output/regression-906/e/terragrunt.hcl(1 hunks)test/fixtures/get-output/regression-906/f/terragrunt.hcl(1 hunks)test/fixtures/get-output/regression-906/g/terragrunt.hcl(1 hunks)test/fixtures/hooks/before-after-and-error-merge/qa/my-app/main.tf(1 hunks)test/fixtures/hooks/init-once/base-module/main.tf(1 hunks)test/fixtures/hooks/init-once/no-source-with-backend/main.tf(1 hunks)test/fixtures/sops-kms/secrets.env(1 hunks)test/fixtures/sops-kms/secrets.ini(1 hunks)test/fixtures/sops-kms/secrets.json(1 hunks)test/fixtures/sops-kms/secrets.txt(1 hunks)test/fixtures/sops-kms/secrets.yaml(1 hunks)test/fixtures/validate-inputs/fail-remote-module/terragrunt.hcl(1 hunks)test/fixtures/validate-inputs/fail-unused-inputs/terragrunt.hcl(1 hunks)test/helpers/aws.go(1 hunks)test/helpers/package.go(8 hunks)test/integration_aws_test.go(25 hunks)test/integration_find_test.go(1 hunks)test/integration_runner_pool_test.go(1 hunks)test/integration_s3_encryption_test.go(3 hunks)test/integration_serial_aws_test.go(2 hunks)test/integration_sops_kms_test.go(1 hunks)test/integration_test.go(1 hunks)test/integration_tofu_state_encryption_test.go(1 hunks)tf/cache/handlers/proxy_provider.go(2 hunks)
💤 Files with no reviewable changes (1)
- awshelper/config.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/info/strict/command.gointernal/cli/autocomplete.gotest/integration_sops_kms_test.gocli/commands/help/cli.gocli/commands/render/render_test.gotest/helpers/aws.gocli/commands/commands.gotelemetry/tracer.gotest/integration_runner_pool_test.gointernal/cli/command.gocli/flags/flag_opts.gopkg/log/logger.gointernal/runner/runner.gointernal/cli/bool_flag.gotest/integration_serial_aws_test.goconfig/hclparse/file.gocli/flags/flag.gointernal/cli/slice_flag.gointernal/cli/map_flag.gointernal/providercache/provider_cache.gointernal/cli/generic_flag.gotf/cache/handlers/proxy_provider.gotest/integration_s3_encryption_test.gointernal/awshelper/policy_test.gointernal/remotestate/backend/s3/backend.gointernal/runner/configstack/runner.gointernal/cli/help.gotest/integration_tofu_state_encryption_test.gointernal/remotestate/backend/s3/remote_state_config.gotest/integration_find_test.gotelemetry/telemeter.goconfig/config_test.goconfig/config.gocli/commands/run/creds/providers/amazonsts/provider.gotest/integration_test.gocli/commands/run/download_source_test.gointernal/remotestate/backend/s3/remote_state_config_test.gointernal/remotestate/backend/s3/retryer.gointernal/cli/flag.gotest/helpers/package.goconfig/config_helpers.gointernal/awshelper/config_test.goconfig/dependency.gointernal/awshelper/config.gointernal/remotestate/backend/s3/client_test.gotest/integration_aws_test.gointernal/remotestate/backend/s3/client.go
🧠 Learnings (41)
test/fixtures/get-output/regression-906/e/terragrunt.hcl (2)
Learnt from: yhakbar
PR: #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.
Learnt from: levkohimins
PR: #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.
internal/cli/autocomplete.go (1)
Learnt from: yhakbar
PR: #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.
cli/commands/help/cli.go (2)
Learnt from: levkohimins
PR: #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.
Learnt from: yhakbar
PR: #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.
cli/commands/render/render_test.go (2)
Learnt from: levkohimins
PR: #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.
Learnt from: partcyborg
PR: #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.
test/helpers/aws.go (7)
Learnt from: yhakbar
PR: #4070
File: internal/remotestate/backend/s3/client.go:609-620
Timestamp: 2025-03-24T14:48:52.988Z
Learning: The syntax for retries := range maxRetriesWaitingForS3Bucket is valid in the Terragrunt codebase. While not standard Go, the pattern for x := range someInt is an accepted idiom in this project, used in both GCS and S3 client implementations.
Learnt from: yhakbar
PR: #4070
File: internal/remotestate/backend/gcs/client.go:274-284
Timestamp: 2025-03-24T14:42:18.517Z
Learning: The syntax for retries := range maxRetriesWaitingForGcsBucket is valid in the Terragrunt codebase. While not standard Go, the pattern for x := range someInt is an accepted idiom in this project.
Learnt from: yhakbar
PR: #3723
File: internal/cloner/cloner.go:163-0
Timestamp: 2025-02-06T21:13:52.071Z
Learning: Go 1.22 and later versions support iterating over integers using range syntax (e.g. for i := range n). This feature was introduced in February 2024 and provides a cleaner alternative to traditional for i := 0; i < n; i++ loops.
Learnt from: yhakbar
PR: #4034
File: cli/commands/list/action.go:636-642
Timestamp: 2025-03-19T11:38:58.959Z
Learning: Go 1.22 (released February 2024) introduced the ability to use for range with integers. The syntax for range N { ... } iterates N times (from 0 to N-1), providing a cleaner way to repeat operations a specific number of times without an explicit counter variable.
Learnt from: yhakbar
PR: #4034
File: cli/commands/list/action.go:636-642
Timestamp: 2025-03-19T11:38:58.959Z
Learning: Go 1.22 (released February 2024) introduced the ability to use for range with integers. The syntax for range N { ... } iterates from 0 to N-1, providing a cleaner way to repeat operations a specific number of times without an explicit counter variable.
Learnt from: yhakbar
PR: #3723
File: internal/cloner/cloner.go:176-0
Timestamp: 2025-02-06T21:12:49.153Z
Learning: Using for i := range n is valid Go syntax for iterating over integers, where n is the upper bound (exclusive) of the range. This is equivalent to for i := 0; i < n; i++ but uses newer Go syntax.
Learnt from: yhakbar
PR: #3723
File: internal/cloner/cloner.go:163-0
Timestamp: 2025-02-06T21:13:52.071Z
Learning: Go supports iterating over integers using range syntax (e.g. for i := range n). This is valid syntax and should not be flagged as an error.
test/fixtures/get-output/regression-906/b/terragrunt.hcl (2)
Learnt from: yhakbar
PR: #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.
Learnt from: levkohimins
PR: #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.
test/fixtures/download/remote-with-backend/terragrunt.hcl (3)
Learnt from: yhakbar
PR: #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.
Learnt from: levkohimins
PR: #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.
Learnt from: partcyborg
PR: #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.
cli/commands/commands.go (2)
Learnt from: levkohimins
PR: #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.
Learnt from: yhakbar
PR: #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/fixtures/get-output/regression-906/a/terragrunt.hcl (2)
Learnt from: yhakbar
PR: #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.
Learnt from: levkohimins
PR: #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.
test/fixtures/get-output/regression-906/g/terragrunt.hcl (2)
Learnt from: yhakbar
PR: #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.
Learnt from: levkohimins
PR: #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.
internal/cli/command.go (1)
Learnt from: yhakbar
PR: #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.
cli/flags/flag_opts.go (1)
Learnt from: yhakbar
PR: #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.
internal/runner/runner.go (1)
Learnt from: wakeful
PR: #4491
File: test/integration_test.go:4265-4265
Timestamp: 2025-07-03T22:05:07.356Z
Learning: Constants defined in one Go file are accessible from other files in the same package without explicit imports. When reviewing Go code, consider the package-level scope rather than just the individual file scope.
internal/cli/bool_flag.go (1)
Learnt from: yhakbar
PR: #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/fixtures/get-output/regression-906/c/terragrunt.hcl (3)
Learnt from: yhakbar
PR: #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.
Learnt from: levkohimins
PR: #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.
Learnt from: yhakbar
PR: #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_serial_aws_test.go (1)
Learnt from: yhakbar
PR: #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.
cli/flags/flag.go (1)
Learnt from: yhakbar
PR: #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/fixtures/download/invalid-path/terragrunt.hcl (2)
Learnt from: yhakbar
PR: #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.
Learnt from: levkohimins
PR: #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.
internal/cli/map_flag.go (1)
Learnt from: yhakbar
PR: #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.
internal/awshelper/policy_test.go (2)
Learnt from: levkohimins
PR: #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.
Learnt from: yhakbar
PR: #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.
internal/runner/configstack/runner.go (1)
Learnt from: partcyborg
PR: #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.
internal/cli/help.go (1)
Learnt from: levkohimins
PR: #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.
internal/remotestate/backend/s3/remote_state_config.go (4)
Learnt from: levkohimins
PR: #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.
Learnt from: yhakbar
PR: #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.
Learnt from: yhakbar
PR: #4070
File: internal/remotestate/backend/s3/client.go:609-620
Timestamp: 2025-03-24T14:48:52.988Z
Learning: The syntax for retries := range maxRetriesWaitingForS3Bucket is valid in the Terragrunt codebase. While not standard Go, the pattern for x := range someInt is an accepted idiom in this project, used in both GCS and S3 client implementations.
Learnt from: wakeful
PR: #4491
File: test/integration_test.go:4265-4265
Timestamp: 2025-07-03T22:05:07.356Z
Learning: Constants defined in one Go file are accessible from other files in the same package without explicit imports. When reviewing Go code, consider the package-level scope rather than just the individual file scope.
config/config_test.go (1)
Learnt from: partcyborg
PR: #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.
config/config.go (2)
Learnt from: partcyborg
PR: #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.
Learnt from: yhakbar
PR: #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.
cli/commands/run/creds/providers/amazonsts/provider.go (3)
Learnt from: levkohimins
PR: #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.
Learnt from: yhakbar
PR: #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.
Learnt from: wakeful
PR: #4491
File: test/integration_test.go:4265-4265
Timestamp: 2025-07-03T22:05:07.356Z
Learning: Constants defined in one Go file are accessible from other files in the same package without explicit imports. When reviewing Go code, consider the package-level scope rather than just the individual file scope.
test/fixtures/validate-inputs/fail-remote-module/terragrunt.hcl (2)
Learnt from: yhakbar
PR: #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.
Learnt from: levkohimins
PR: #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.
test/integration_test.go (2)
Learnt from: levkohimins
PR: #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.
Learnt from: yhakbar
PR: #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/fixtures/get-output/regression-906/f/terragrunt.hcl (2)
Learnt from: yhakbar
PR: #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.
Learnt from: levkohimins
PR: #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.
internal/remotestate/backend/s3/remote_state_config_test.go (4)
Learnt from: levkohimins
PR: #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.
Learnt from: yhakbar
PR: #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.
Learnt from: wakeful
PR: #4491
File: test/integration_test.go:4265-4265
Timestamp: 2025-07-03T22:05:07.356Z
Learning: Constants defined in one Go file are accessible from other files in the same package without explicit imports. When reviewing Go code, consider the package-level scope rather than just the individual file scope.
Learnt from: yhakbar
PR: #4070
File: internal/remotestate/backend/s3/client.go:609-620
Timestamp: 2025-03-24T14:48:52.988Z
Learning: The syntax for retries := range maxRetriesWaitingForS3Bucket is valid in the Terragrunt codebase. While not standard Go, the pattern for x := range someInt is an accepted idiom in this project, used in both GCS and S3 client implementations.
test/fixtures/validate-inputs/fail-unused-inputs/terragrunt.hcl (2)
Learnt from: yhakbar
PR: #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.
Learnt from: levkohimins
PR: #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.
internal/cli/flag.go (1)
Learnt from: yhakbar
PR: #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/fixtures/get-output/regression-906/d/terragrunt.hcl (2)
Learnt from: yhakbar
PR: #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.
Learnt from: levkohimins
PR: #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.
test/helpers/package.go (3)
Learnt from: levkohimins
PR: #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.
Learnt from: yhakbar
PR: #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.
Learnt from: wakeful
PR: #4491
File: test/integration_test.go:4265-4265
Timestamp: 2025-07-03T22:05:07.356Z
Learning: Constants defined in one Go file are accessible from other files in the same package without explicit imports. When reviewing Go code, consider the package-level scope rather than just the individual file scope.
config/config_helpers.go (4)
Learnt from: levkohimins
PR: #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.
Learnt from: yhakbar
PR: #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.
Learnt from: wakeful
PR: #4491
File: test/integration_test.go:4265-4265
Timestamp: 2025-07-03T22:05:07.356Z
Learning: Constants defined in one Go file are accessible from other files in the same package without explicit imports. When reviewing Go code, consider the package-level scope rather than just the individual file scope.
Learnt from: partcyborg
PR: #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.
internal/awshelper/config_test.go (1)
Learnt from: levkohimins
PR: #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.
config/dependency.go (4)
Learnt from: levkohimins
PR: #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.
Learnt from: yhakbar
PR: #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.
Learnt from: yhakbar
PR: #4070
File: internal/remotestate/backend/s3/client.go:609-620
Timestamp: 2025-03-24T14:48:52.988Z
Learning: The syntax for retries := range maxRetriesWaitingForS3Bucket is valid in the Terragrunt codebase. While not standard Go, the pattern for x := range someInt is an accepted idiom in this project, used in both GCS and S3 client implementations.
Learnt from: wakeful
PR: #4491
File: test/integration_test.go:4265-4265
Timestamp: 2025-07-03T22:05:07.356Z
Learning: Constants defined in one Go file are accessible from other files in the same package without explicit imports. When reviewing Go code, consider the package-level scope rather than just the individual file scope.
internal/awshelper/config.go (1)
Learnt from: wakeful
PR: #4491
File: test/integration_test.go:4265-4265
Timestamp: 2025-07-03T22:05:07.356Z
Learning: Constants defined in one Go file are accessible from other files in the same package without explicit imports. When reviewing Go code, consider the package-level scope rather than just the individual file scope.
internal/remotestate/backend/s3/client_test.go (4)
Learnt from: levkohimins
PR: #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.
Learnt from: yhakbar
PR: #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.
Learnt from: wakeful
PR: #4491
File: test/integration_test.go:4265-4265
Timestamp: 2025-07-03T22:05:07.356Z
Learning: Constants defined in one Go file are accessible from other files in the same package without explicit imports. When reviewing Go code, consider the package-level scope rather than just the individual file scope.
Learnt from: yhakbar
PR: #4070
File: internal/remotestate/backend/s3/client.go:609-620
Timestamp: 2025-03-24T14:48:52.988Z
Learning: The syntax for retries := range maxRetriesWaitingForS3Bucket is valid in the Terragrunt codebase. While not standard Go, the pattern for x := range someInt is an accepted idiom in this project, used in both GCS and S3 client implementations.
test/integration_aws_test.go (7)
Learnt from: wakeful
PR: #4491
File: test/integration_test.go:4265-4265
Timestamp: 2025-07-03T22:05:07.356Z
Learning: Constants defined in one Go file are accessible from other files in the same package without explicit imports. When reviewing Go code, consider the package-level scope rather than just the individual file scope.
Learnt from: levkohimins
PR: #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.
Learnt from: yhakbar
PR: #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.
Learnt from: yhakbar
PR: #4070
File: internal/remotestate/backend/s3/client.go:609-620
Timestamp: 2025-03-24T14:48:52.988Z
Learning: The syntax for retries := range maxRetriesWaitingForS3Bucket is valid in the Terragrunt codebase. While not standard Go, the pattern for x := range someInt is an accepted idiom in this project, used in both GCS and S3 client implementations.
Learnt from: yhakbar
PR: #3723
File: internal/cloner/cloner.go:176-0
Timestamp: 2025-02-06T21:12:49.153Z
Learning: Using for i := range n is valid Go syntax for iterating over integers, where n is the upper bound (exclusive) of the range. This is equivalent to for i := 0; i < n; i++ but uses newer Go syntax.
Learnt from: yhakbar
PR: #4070
File: internal/remotestate/backend/gcs/client.go:274-284
Timestamp: 2025-03-24T14:42:18.517Z
Learning: The syntax for retries := range maxRetriesWaitingForGcsBucket is valid in the Terragrunt codebase. While not standard Go, the pattern for x := range someInt is an accepted idiom in this project.
Learnt from: yhakbar
PR: #3723
File: internal/cloner/cloner.go:163-0
Timestamp: 2025-02-06T21:13:52.071Z
Learning: Go supports iterating over integers using range syntax (e.g. for i := range n). This is valid syntax and should not be flagged as an error.
internal/remotestate/backend/s3/client.go (3)
Learnt from: levkohimins
PR: #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.
Learnt from: wakeful
PR: #4491
File: test/integration_test.go:4265-4265
Timestamp: 2025-07-03T22:05:07.356Z
Learning: Constants defined in one Go file are accessible from other files in the same package without explicit imports. When reviewing Go code, consider the package-level scope rather than just the individual file scope.
Learnt from: yhakbar
PR: #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.
🧬 Code Graph Analysis (12)
cli/commands/info/strict/command.go (1)
internal/view/writer.go (1)
NewWriter(26-31)
cli/commands/help/cli.go (1)
internal/cli/commands.go (1)
Commands(10-10)
internal/providercache/provider_cache.go (1)
tf/cache/handlers/registry_urls.go (1)
DiscoveryURL(49-82)
internal/awshelper/policy_test.go (1)
internal/awshelper/policy.go (1)
Statement(15-23)
internal/runner/configstack/runner.go (2)
config/parsing_context.go (1)
ParsingContext(17-53)config/config.go (2)
IncludeConfig(748-753)TerragruntConfig(143-173)
config/config_test.go (1)
internal/remotestate/remote_state.go (1)
RemoteState(24-27)
config/config.go (1)
internal/remotestate/remote_state.go (1)
RemoteState(24-27)
cli/commands/run/creds/providers/amazonsts/provider.go (1)
internal/awshelper/config.go (1)
AssumeIamRole(283-375)
internal/awshelper/config_test.go (2)
test/helpers/logger/logger.go (1)
CreateLogger(9-14)internal/awshelper/config.go (2)
CreateAwsConfig(230-280)AwsSessionConfig(29-41)
config/dependency.go (3)
config/parsing_context.go (1)
ParsingContext(17-53)options/options.go (1)
TerragruntOptions(100-330)internal/awshelper/config.go (1)
CreateAwsConfig(230-280)
internal/awshelper/config.go (2)
options/options.go (5)
TerragruntOptions(100-330)IAMRoleOptions(350-355)MergeIAMRoleOptions(357-377)DefaultIAMAssumeRoleDuration(55-55)GetDefaultIAMAssumeRoleSessionName(471-473)pkg/log/log.go (1)
Debugf(72-74)
internal/remotestate/backend/s3/client.go (4)
internal/remotestate/backend/s3/remote_state_config.go (2)
ExtendedRemoteStateConfigS3(48-72)RemoteStateConfigS3(176-194)internal/awshelper/config.go (4)
CreateAwsConfig(230-280)ValidateAwsConfig(384-387)GetAWSAccountID(428-435)GetAWSPartition(390-409)pkg/log/log.go (2)
Debugf(72-74)Warnf(92-94)internal/awshelper/policy.go (4)
Policy(6-9)Statement(15-23)UnmarshalPolicy(25-34)MarshalPolicy(36-43)
🪛 GitHub Actions: Strict Lint
cli/commands/render/render_test.go
[warning] 113-113: Comment should end in a period (godot)
[warning] 128-128: Comment should end in a period (godot)
telemetry/tracer.go
[error] 157-157: Error returned from external package is unwrapped; consider wrapping error for better context (wrapcheck).
[error] 164-164: Error returned from external package is unwrapped; consider wrapping error for better context (wrapcheck).
[error] 171-171: Error returned from external package is unwrapped; consider wrapping error for better context (wrapcheck).
[error] 173-173: Error returned from external package is unwrapped; consider wrapping error for better context (wrapcheck).
internal/cli/slice_flag.go
[error] 173-173: Error returned from interface method should be wrapped for better error handling (wrapcheck).
internal/cli/map_flag.go
[error] 203-203: Error returned from interface method should be wrapped for better error handling (wrapcheck).
[error] 208-208: Error returned from interface method should be wrapped for better error handling (wrapcheck).
internal/cli/generic_flag.go
[error] 142-142: Error returned from interface method should be wrapped for better error handling (wrapcheck).
tf/cache/handlers/proxy_provider.go
[error] 113-113: Error returned from external package is unwrapped; consider wrapping error for better context (wrapcheck).
[error] 134-134: Error returned from external package is unwrapped; consider wrapping error for better context (wrapcheck).
[error] 139-139: Error returned from external package is unwrapped; consider wrapping error for better context (wrapcheck).
internal/runner/configstack/runner.go
[error] 611-611: Error returned from external package is unwrapped; consider wrapping error for better context (wrapcheck).
[error] 666-666: Error returned from external package is unwrapped; consider wrapping error for better context (wrapcheck).
cli/commands/run/download_source_test.go
[warning] 484-484: Comment should end in a period (godot)
[warning] 538-538: Comment should end in a period (godot)
[warning] 580-580: Comment should end in a period (godot)
[warning] 622-622: Comment should end in a period (godot)
[warning] 663-663: Comment should end in a period (godot)
[warning] 704-704: Comment should end in a period (godot)
[warning] 724-724: Comment should end in a period (godot)
[warning] 727-727: Comment should end in a period (godot)
internal/cli/flag.go
[error] 127-127: Error returned from interface method should be wrapped for better error handling (wrapcheck).
[error] 147-147: Error returned from interface method should be wrapped for better error handling (wrapcheck).
test/helpers/package.go
[error] 251-251: Error returned from external package is unwrapped; consider wrapping error for better context (wrapcheck).
[error] 254-254: Error returned from external package is unwrapped; consider wrapping error for better context (wrapcheck).
internal/awshelper/config.go
[error] 147-147: Error returned from external package is unwrapped; consider wrapping error for better context (wrapcheck).
[error] 186-186: Error returned from external package is unwrapped; consider wrapping error for better context (wrapcheck).
[error] 417-417: Error returned from external package is unwrapped; consider wrapping error for better context (wrapcheck).
internal/remotestate/backend/s3/client.go
[error] 1084-1084: Error returned from interface method should be wrapped for better error handling (wrapcheck).
[error] 1116-1116: Error returned from interface method should be wrapped for better error handling (wrapcheck).
[error] 1195-1195: Error returned from interface method should be wrapped for better error handling (wrapcheck).
[error] 1253-1253: Error returned from external package is unwrapped; consider wrapping error for better context (wrapcheck).
[error] 1312-1312: Error returned from external package is unwrapped; consider wrapping error for better context (wrapcheck).
[error] 1427-1427: Error returned from external package is unwrapped; consider wrapping error for better context (wrapcheck).
[error] 1463-1463: Error returned from external package is unwrapped; consider wrapping error for better context (wrapcheck).
⏰ 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 (117)
internal/providercache/provider_cache.go (1)
314-314: LGTM: Method call refactoring improves encapsulation.The change from
cache.Server.DiscoveryURL(ctx, registryName)tocache.DiscoveryURL(ctx, registryName)indicates that theDiscoveryURLmethod has been moved from the embeddedServerfield to theProviderCachereceiver itself. This refactoring improves encapsulation and aligns with the broader internal package reorganization described in the PR objectives.telemetry/tracer.go (1)
220-220: LGTM! Method call refactoring improves encapsulation.The change from
tracer.Tracer.Start(ctx, name)totracer.Start(ctx, name)is correct and improves code readability by leveraging Go's embedded field promotion. SinceTracerembedstrace.Tracer, theStartmethod is promoted and can be called directly.telemetry/telemeter.go (1)
58-60: LGTM! Consistent refactoring improves method call clarity.The changes from
tlm.Tracer.Tracetotlm.Traceandtlm.Meter.Timetotlm.Timeare correct and consistent with the refactoring pattern. SinceTelemeterembeds both*Tracerand*Meter, their methods are promoted and can be called directly on theTelemeterinstance. This improves encapsulation and maintains consistent method invocation patterns throughout the telemetry package.pkg/log/logger.go (1)
350-350: LGTM: Consistent method call patternThe change from
newLogger.Entry.Dup()tonewLogger.Dup()leverages Go's embedding semantics and aligns with the broader refactoring effort to improve consistency across the codebase.test/fixtures/sops-kms/secrets.env (1)
1-8: LGTM: Test fixture re-encryption for AWS SDK migrationThe re-encryption of secrets with updated KMS key ARN and fresh cryptographic material is appropriate for the AWS SDK v2 migration and test infrastructure updates.
test/fixtures/sops-kms/secrets.txt (1)
2-17: LGTM: Consistent test fixture updatesThe re-encryption of this test fixture with updated KMS metadata is consistent with the AWS SDK v2 migration and aligns with similar changes across other SOPS-encrypted files.
test/integration_find_test.go (1)
169-169: LGTM: Modern Go type usageThe change from
...interface{}to...anymodernizes the code to use the more idiomaticanytype alias introduced in Go 1.18, improving readability without changing functionality.internal/cli/map_flag.go (1)
138-138: LGTM: Consistent method call patternThe change to call
GetInitialTextValue()directly on the flag instance aligns with the broader refactoring effort for consistency across flag types.test/fixtures/validate-inputs/fail-remote-module/terragrunt.hcl (1)
2-2: Source path & version correctly bumped
fixture-download➜fixtures/downloadandv0.9.9➜v0.83.2keeps the fixture paths consistent with the rest of the repo.
No further issues spotted.test/fixtures/get-output/regression-906/e/terragrunt.hcl (1)
10-10: Consistent fixture referenceChange aligns this regression test with the new module location and version used elsewhere in the suite.
test/fixtures/get-output/regression-906/b/terragrunt.hcl (1)
10-10: LGTM – reference updatedThe updated git source mirrors the standardized fixture path/version. Looks good.
test/fixtures/get-output/regression-906/a/terragrunt.hcl (1)
10-10: Fixture URL/version syncedMatches the new canonical location; no further feedback.
test/fixtures/get-output/regression-906/d/terragrunt.hcl (1)
10-10: Uniform update appliedConsistent with other 906 fixtures; change is straightforward and correct.
test/integration_sops_kms_test.go (1)
39-45: 👍 Switched to theanyalias – looks good
[]anyis functionally identical to[]interface{}and keeps the code concise without changing behaviour. Tests remain clear and type-safe.test/fixtures/hooks/init-once/base-module/main.tf (1)
7-7: Confirm provider drift risk when loosening the version constraintChanging
nullprovider from a fixed3.2.3to"~> 3.2"permits any future3.2.xpatch. That is usually safe, but CI should runterraform init -upgradeto surface surprises from upstream patch releases.No action needed if CI already covers this; otherwise consider pinning or adding a dependabot-style workflow to spot breaking patches early.
test/fixtures/hooks/init-once/no-source-with-backend/main.tf (1)
9-9: LGTM! Good practice using flexible version constraints.The change from a fixed version
"3.2.3"to a flexible constraint"~> 3.2"allows patch-level updates while maintaining compatibility. This is a best practice for test fixtures as it provides flexibility for patch updates without breaking changes.test/fixtures/hooks/before-after-and-error-merge/qa/my-app/main.tf (1)
9-9: LGTM! Consistent with version constraint improvements.Same beneficial change as other test fixtures - using flexible version constraints for better maintenance.
cli/commands/info/strict/command.go (1)
67-67: LGTM! Consistent with CLI output standardization.The change from
ctx.App.Writertoctx.Writeraligns with the broader CLI refactoring effort to use context-level writers consistently across commands. This improves encapsulation and maintains compatibility with theview.NewWriterinterface.internal/runner/runner.go (2)
6-6: LGTM! Good addition of maps package import.Adding the
mapsimport to support the improved map copying implementation below.
45-45: LGTM! Excellent refactoring using stdlib maps.Copy.Replacing the manual map copying loop with
maps.Copyis a great improvement. This is more efficient, cleaner, and leverages the standard library functionality. The behavior is identical but the code is more maintainable.internal/cli/autocomplete.go (1)
73-73: LGTM! Consistent CLI writer standardization.All three changes from
ctx.App.Writertoctx.Writeralign with the broader CLI refactoring effort to standardize output handling. This improves consistency across the CLI codebase while maintaining full compatibility with theio.Writerinterface expected by the print functions.Also applies to: 76-76, 79-79
test/fixtures/download/hello-world-with-backend/main.tf (1)
10-10: LGTM! Good use of flexible version constraints.The change from a fixed version to
"~> 3.2"follows Terraform best practices by allowing patch version updates within the 3.2 series while maintaining compatibility.cli/commands/render/render_test.go (1)
22-22: LGTM! Consistent writer access pattern.The changes consistently update writer assignment from the nested
opts.TerragruntOptions.Writerto directopts.Writeraccess, which simplifies the API and aligns with the broader CLI output standardization.Also applies to: 44-44, 105-105
test/fixtures/download/invalid-path/terragrunt.hcl (1)
2-2: LGTM! Good standardization and corrections.The changes improve consistency by:
- Standardizing the directory path from
fixture-downloadtofixtures/download- Using more grammatically correct
non-existent-pathinstead ofnot-existing-path- Updating to a more recent version tag
v0.83.2These changes maintain the test's intent (pointing to a non-existent path) while improving overall consistency across test fixtures.
cli/flags/flag_opts.go (1)
101-102: LGTM! Improved API encapsulation.The change from
newFlag.Flag.Names()tonewFlag.Names()improves encapsulation by using the wrapper's method interface instead of directly accessing the embedded field. This provides better abstraction and is applied consistently to both flag names and environment variable generation.internal/cli/slice_flag.go (1)
118-118: LGTM! Consistent API encapsulation.The change from
flag.FlagValue.GetInitialTextValue()toflag.GetInitialTextValue()follows the same encapsulation pattern applied across flag implementations, using the wrapper's method instead of directly accessing the embedded field.cli/commands/help/cli.go (1)
32-32: LGTM! Consistent with CLI refactoring.The change from
ctx.App.Commandstoctx.Commandsaligns with the broader CLI refactoring pattern seen across the codebase, simplifying command access by moving it directly to the context level.test/fixtures/validate-inputs/fail-unused-inputs/terragrunt.hcl (1)
8-8: LGTM! Test fixture maintenance update.The source URL correction from
fixture-downloadtofixtures/downloadand version update fromv0.9.9tov0.83.2aligns with the broader test fixture standardization effort across the codebase.internal/awshelper/policy_test.go (2)
8-8: LGTM! Package relocation to internal.The import path update correctly reflects the
awshelperpackage being moved to theinternaldirectory as part of the broader refactoring effort.
95-95: LGTM! Modern Go type alias usage.The change from
[]interface{}to[]anyis a good modernization -anyis the preferred alias forinterface{}introduced in Go 1.18 and aligns with the field types in theStatementstruct.Also applies to: 103-103
test/fixtures/get-output/regression-906/c/terragrunt.hcl (1)
10-10: LGTM! Consistent test fixture update.The source URL correction and version update matches the standardization effort across test fixtures, ensuring consistent references to the correct module paths and current versions.
test/fixtures/download/local-windows/JZwoL6Viko8bzuRvTOQFx3Jh8vs/3mU4huxMLOXOW5ZgJOFXGUFDKc8/main.tf (1)
32-32: LGTM! Test fixture source update.The module source correction from
fixture-downloadtofixtures/downloadand version update tov0.83.2maintains consistency with the broader test fixture standardization across the codebase.cli/commands/commands.go (1)
405-405: LGTM! Clean refactoring to simplify version access.The change from
cliCtx.App.VersiontocliCtx.Versionsimplifies the code and aligns with the broader CLI refactoring to standardize context usage patterns.test/integration_tofu_state_encryption_test.go (1)
26-26: LGTM! Test fixture update for AWS KMS key.The AWS KMS key ID update is consistent with the broader AWS SDK v2 migration and test infrastructure refresh mentioned in the PR objectives.
test/integration_runner_pool_test.go (1)
69-69: Go version compatibility for strings.SplitSeq is satisfiedThe project’s
go.moddeclaresgo 1.24.4, which includes thestrings.SplitSeqAPI introduced in Go 1.23. This change is safe to merge.test/fixtures/auth-provider-cmd/sops/secrets.json (1)
2-2: LGTM! Consistent SOPS encryption refresh with new AWS KMS key.The encrypted data, KMS key metadata, and MAC values have been consistently updated, indicating a proper re-encryption of the test fixture with the new KMS key. This aligns with the AWS SDK v2 migration requirements.
Also applies to: 6-8, 16-17
test/fixtures/get-output/regression-906/f/terragrunt.hcl (1)
10-10: LGTM! Test fixture maintenance update.The source URL correction fixes the directory path (
fixture-download→fixtures/download) and updates to a more recent version (v0.9.9→v0.83.2). This ensures test consistency across the codebase.test/integration_s3_encryption_test.go (2)
62-65: Excellent documentation addition for test prerequisites.These comments clearly explain the KMS key requirements and provide actionable CLI commands to resolve test failures. This significantly improves the maintainability and usability of these integration tests.
Also applies to: 125-128
160-160: Good formatting improvement.The blank line addition improves code readability before the final assertion.
internal/cli/command.go (2)
149-149: LGTM: Consistent context usage simplification.The changes from
ctx.App.handleExitCodertoctx.handleExitCoderalign with the broader CLI refactoring to standardize context usage patterns. The functionality remains unchanged.Also applies to: 155-155, 161-161, 171-171
190-190: LGTM: Direct context field access.The change from
ctx.App.FlagErrHandlertoctx.FlagErrHandleris consistent with the pattern of accessing properties directly on the context rather than through theAppfield.internal/cli/generic_flag.go (1)
89-89: LGTM: Consistent method call simplification.The change from
flag.FlagValue.GetInitialTextValue()toflag.GetInitialTextValue()is consistent with the broader CLI refactoring to call methods directly on flag instances rather than embedded fields.test/integration_serial_aws_test.go (2)
66-67: LGTM: Environment variable handling for test constraints.The removal of the
tenvlinter directive while retainingusetestingis documented with a comment explaining the test-specific constraint. This is acceptable given the existing explanation about test compatibility issues.Also applies to: 70-71
165-165: Good improvement: More idiomatic Go loop syntax.The change from an indexed for loop to
for i := range numberOfModulesis more idiomatic modern Go when you only need the index value. This improves code readability.cli/flags/flag.go (1)
120-120: LGTM: Consistent method dispatch simplification.The change from
deprecated.Flag.Apply(set)todeprecated.Apply(set)is consistent with the broader CLI refactoring pattern of calling methods directly on wrapper types rather than embedded fields. This simplifies the method dispatch while maintaining the same functionality.internal/cli/bool_flag.go (1)
97-97: LGTM! Refactoring simplifies field access correctly.The changes appropriately remove unnecessary indirection by calling methods directly on the struct rather than through embedded fields, which aligns with the broader CLI package refactoring effort.
Also applies to: 159-159
tf/cache/handlers/proxy_provider.go (2)
70-70: LGTM! Method calls updated correctly.The changes from
handler.ReverseProxy.NewRequesttohandler.NewRequestappear to correctly reflect the refactored structure.Also applies to: 108-108, 116-116
112-114: Fix error wrapping issues flagged by linter.The pipeline failures indicate that errors from external packages should be wrapped for better context. Consider wrapping these errors:
- downloadURL, err := url.Parse(provider.DownloadURL) - if err != nil { - return err - } + downloadURL, err := url.Parse(provider.DownloadURL) + if err != nil { + return fmt.Errorf("failed to parse download URL: %w", err) + }Apply similar wrapping for the
strconv.Unquoteandurl.Parseerrors in themodifyDownloadURLsInJSONBodyfunction.Also applies to: 132-135, 137-140
⛔ Skipped due to learnings
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.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/fixtures/sops-kms/secrets.yaml (1)
1-26: LGTM! Test fixture properly re-encrypted.The SOPS-encrypted test fixture has been appropriately updated with new encrypted values, KMS key reference, and metadata. This aligns with the AWS SDK migration and maintains the same structure while refreshing the encrypted content.
internal/remotestate/backend/s3/backend.go (1)
43-43: LGTM! Context parameters added correctly for AWS SDK v2 migration.The systematic addition of
ctxas the first parameter to allNewClientcalls is the correct approach for AWS SDK v2 migration. This enables proper context-aware operations, cancellation, and timeout handling while maintaining existing functionality.Also applies to: 90-90, 137-137, 170-170, 205-205, 240-240
config/config_test.go (1)
1860-1862: LGTM! Test assertions updated to match structural changes.The test assertions correctly reflect the updated
RemoteStatestructure where*Configis embedded with a squash tag, making fields likeDisableInit,DisableDependencyOptimization, andBackendConfigdirectly accessible on theRemoteStatestruct rather than through a nestedConfigfield.go.mod (1)
84-89: LGTM! AWS SDK v2 dependencies properly configured.The promotion of AWS SDK v2 modules (config, dynamodb, iam, sts, smithy-go) from indirect to direct dependencies correctly supports the codebase migration from AWS SDK v1 to v2. The downgrade of the classic AWS SDK v1 to indirect dependency is appropriate.
cli/commands/run/download_source_test.go (1)
214-214: Test fixture URLs updated correctly.The updates to use
v0.83.2instead of older versions and the path changes fromfixture-download-sourcetofixtures/download-sourcealign with the repository's fixture reorganization and version updates.Also applies to: 226-226, 238-238, 270-270, 290-290
test/fixtures/sops-kms/secrets.json (1)
2-27: Encrypted test fixture properly updated.The re-encryption of all values with new AWS KMS key ARN, updated timestamps, and fresh ciphertext is consistent with supporting the AWS SDK v2 migration in test infrastructure. The encryption scheme and data structure remain properly preserved.
config/hclparse/file.go (1)
63-63: Method call simplification improves code clarity.Removing the intermediate
Parserreceiver when callingfileUpdateHandlerFuncandhandleDiagnosticsmethods eliminates unnecessary indirection and improves code readability. The functionality remains unchanged.Also applies to: 130-130
internal/cli/flag.go (1)
124-124: Method and field access simplification approved.The direct access to fields and methods on the receiver instead of through embedded fields improves code clarity and eliminates unnecessary indirection.
Also applies to: 126-126, 144-144, 146-146, 263-263
internal/cli/help.go (1)
26-26: LGTM: Clean CLI context refactoring.The changes consistently remove the
.Appaccessor and access context fields directly. This refactoring simplifies the API and the implementation looks correct throughout all the affected functions.Also applies to: 35-36, 39-39, 85-85, 89-89, 98-98
test/fixtures/sops-kms/secrets.ini (1)
2-3: LGTM: Test fixture re-encryption.The SOPS-encrypted test fixture has been properly re-encrypted with updated KMS key metadata and timestamps. This is expected as part of the AWS SDK migration and test infrastructure updates.
Also applies to: 6-13
cli/commands/run/creds/providers/amazonsts/provider.go (3)
8-8: LGTM: Correct AWS SDK v2 import migration.The imports have been properly updated from AWS SDK v1 to v2, and the
awshelperpackage correctly moved to the internal package structure.Also applies to: 10-10
46-46: LGTM: Correct function signature update.The call to
awshelper.AssumeIamRolehas been properly updated to match the new AWS SDK v2 signature, including the context parameter and additional arguments as shown in the internal/awshelper implementation.
55-58: LGTM: Correct AWS SDK v2 pointer handling.The usage of
aws.ToStringinstead ofaws.StringValueis the correct approach for AWS SDK v2 pointer dereferencing.config/config.go (2)
363-363: LGTM: Correct remote state config access pattern.The changes properly access remote state configuration fields directly on the
RemoteStatestruct rather than through a nestedConfigfield. This aligns with the struct definition whereConfigis embedded with thesquashmapstructure tag.Also applies to: 367-367, 371-371
1438-1438: LGTM: Bug fix for JSON unmarshaling.The fix correctly uses
file.Bytesinstead offile.File.Bytesto access the raw file content for JSON unmarshaling in the bare include detection logic.internal/remotestate/backend/s3/remote_state_config_test.go (2)
8-11: LGTM: Correct AWS SDK v2 import migration.The imports have been properly updated to AWS SDK v2 with appropriate aliasing for S3 types. The internal
awshelperpackage import is also correct.
37-42: LGTM: Proper AWS SDK v2 type migration.All S3 type references have been correctly updated to use the
s3typesalias, includingBucketLoggingStatus,LoggingEnabled,TargetObjectKeyFormat, andPartitionedPrefix. The change toPartitionDateSourcefrom a pointer to a direct typed value correctly reflects the AWS SDK v2 type system.Also applies to: 55-60, 72-78, 91-102
internal/runner/configstack/runner.go (1)
667-667: LGTM! Context parameter explicitly marked as unused.The change from
ctx context.Contextto_ context.Contextcorrectly indicates that this parameter is intentionally unused, which aligns with the existing//nolint:unparamannotation.internal/awshelper/config_test.go (2)
23-30: LGTM! Test correctly migrated to AWS SDK v2.The test has been properly updated to use the new
CreateAwsConfigfunction with context support, maintaining the same validation behavior.
49-49: LGTM! Types correctly migrated to AWS SDK v2.The
PublicAccessBlockConfigurationtype references have been properly updated to use the v2 SDK'ss3typespackage.Also applies to: 60-60
internal/remotestate/backend/s3/retryer.go (1)
7-17: No custom HTTP 400 retry logic found—default retryer is in use
A search for “400” and all references toRetryershows no custom HTTP 400 handling wired into the S3 client. TheRetryertype defined in retryer.go isn’t passed intos3.NewFromConfig(cfg), so the default AWS SDK v2 retryer remains in effect. This confirms that removing the v1 custom retry logic has no impact on S3 operations.internal/remotestate/backend/s3/remote_state_config.go (2)
76-76: LGTM! Encryption constant correctly migrated to AWS SDK v2.The type conversion from
s3types.ServerSideEncryptionAwsKmsto string is the correct approach for AWS SDK v2.
116-132: LGTM! S3 logging types correctly migrated to AWS SDK v2.All S3 logging-related types have been properly updated to use the v2 SDK's
s3typespackage, including the correct type conversion forPartitionDateSource.test/helpers/package.go (5)
174-186: LGTM! S3 client creation correctly migrated to AWS SDK v2.The function properly uses
CreateAwsConfigands3.NewFromConfigto create a v2 SDK client.
189-205: LGTM! DynamoDB client creation correctly migrated to AWS SDK v2.The function properly uses
CreateAwsConfiganddynamodb.NewFromConfigto create a v2 SDK client.
215-225: Good addition of bucket existence check!The
HeadBucketcall before attempting deletion is a good practice to avoid unnecessary operations and provide clearer logging.
260-345: LGTM! Comprehensive refactoring for AWS SDK v2.The
cleanS3Bucketfunction has been thoroughly updated to:
- Use v2 SDK types and methods correctly
- Handle both object versions and delete markers separately as required by v2
- Include proper error handling for non-existent buckets
- Maintain the same cleanup functionality with improved structure
1208-1215: LGTM! Useful helper for AWS SDK v2 error handling.The
isAWSResourceNotFoundErrorfunction properly uses the v2 SDK's error handling patterns withsmithy.APIErrorand covers the relevant error codes for missing resources.config/config_helpers.go (3)
27-27: LGTM!The import path update to use the internal
awshelperpackage aligns with the PR objective of moving the component into an internal package.
275-275: Good fix!The context value extraction is correctly simplified. Since
ctxis a*ParsingContextthat embedscontext.Context, callingctx.Value()directly is the proper approach.
623-629: Consistent AWS SDK v2 migrationAll four AWS helper functions have been properly updated to use the new SDK v2 pattern:
CreateAwsSession→CreateAwsConfig- Added context as the first parameter to AWS helper calls
- Return values now use the new config type
The changes are consistent and follow AWS SDK v2 best practices.
Also applies to: 638-644, 653-659, 668-674
config/dependency.go (3)
14-16: Correct AWS SDK v2 importsThe import statements have been properly updated to use AWS SDK v2 packages.
590-593: Simplified error handling for AWS SDK v2The error checking has been simplified to use string matching, which is appropriate for AWS SDK v2 since it doesn't expose typed errors the same way v1 did. This is a common pattern for checking S3 errors in SDK v2.
1006-1026: Well-implemented AWS SDK v2 migrationThe function has been properly updated to:
- Accept a
ctx *ParsingContextparameter for context propagation- Use
awshelper.CreateAwsConfigwith the new signature- Create S3 client using
s3.NewFromConfig- Pass context as the first argument to
GetObject- Use pointer types for bucket and key parameters as required by SDK v2
All changes follow AWS SDK v2 best practices.
internal/remotestate/backend/s3/client_test.go (4)
12-14: Correct test imports for AWS SDK v2The imports have been properly updated to use AWS SDK v2 packages, with DynamoDB types aliased for clarity.
42-42: Proper context usage in test functionsAll AWS API calls have been correctly updated to:
- Use
t.Context()for test context propagation- Pass context as the first parameter to all AWS operations
- Access DynamoDB client via
client.GetDynamoDBClient()for proper encapsulationThis follows Go testing best practices for context usage.
Also applies to: 78-78, 113-113, 144-144, 173-173, 210-211
225-225: Improved cleanup patternGood improvement moving the deferred cleanup immediately after table creation. This ensures the table is cleaned up even if the test fails, preventing resource leaks in the test environment.
235-239: Correct AWS SDK v2 type usageThe helper function has been properly updated to return SDK v2 types using the new
AttributeValueMemberSstruct for string values.test/integration_aws_test.go (18)
16-29: LGTM! Import updates are correct for AWS SDK v2 migration.The imports have been properly updated to use AWS SDK v2 packages, with appropriate aliasing for
s3typesto avoid naming conflicts.
637-638: Correct migration of S3 encryption constants.The
ServerSideEncryptionAes256constant has been properly updated to use thes3typespackage reference, which is the correct location in AWS SDK v2.Also applies to: 692-693
648-653: Good simplification of policy statement iteration.Removing the nil check before iterating is correct since Go safely handles iteration over nil slices (zero iterations). The code is now cleaner.
910-925: Correct AWS SDK v2 migration pattern for IAM operations.The changes properly implement the AWS SDK v2 patterns:
- Config creation with context support
- Client instantiation using
NewFromConfig- API calls with context parameter
- Direct value access for AccountAliases (SDK v2 change)
950-959: Consistent AWS SDK v2 migration for STS operations.The STS client creation and API calls follow the same correct pattern as the IAM operations.
974-1027: Good addition oft.Helper()and extensive test refactoring.The addition of
t.Helper()is a good practice for test functions. However, this appears to be a complete rewrite of the test logic beyond just the AWS SDK migration.Please verify that the test semantics and coverage remain equivalent to the previous implementation, as this appears to be more than just an SDK migration.
1314-1315: Modern loop syntax correctly applied.The
for range 3syntax is the modern Go idiom for iterating a specific number of times, which is accepted in the Terragrunt codebase.
1415-1417: Consistent modernization of loop and logger creation.Good use of the modern
range 20syntax and updated logger creation pattern.
1460-1464: Consistent context propagation in AWS config creation.All AWS config creation and identity ARN retrieval calls have been properly updated to accept context parameters, following AWS SDK v2 requirements.
Also applies to: 1498-1502, 1594-1598
1739-1747: Well-structured migration of S3 tags assertion helper.The helper function has been properly updated for AWS SDK v2:
- Correct client type signature
- Proper context usage from
t.Context()- Appropriate pointer conversions with
aws.String()
1761-1774: Correct adaptation to AWS SDK v2 versioning status handling.The migration properly handles the SDK v2 change where disabled versioning returns an empty string instead of nil. The use of
s3types.BucketVersioningStatusEnabledis correct.
1784-1796: Clean DynamoDB helper function migration.The DynamoDB table validation helper has been properly updated with SDK v2 patterns, including correct type references from the
typespackage.
1816-1830: Proper implementation of SDK v2 AttributeValue pattern.The DynamoDB AttributeValue structure correctly uses the new SDK v2 type-specific members pattern (
AttributeValueMemberSfor string values).
1863-1882: Excellent AWS SDK v2 error handling implementation.The migration from
awserr.Errortosmithy.APIErrorwitherrors.Asis the correct pattern for AWS SDK v2 error handling. The check for "NotFound" error code is properly implemented.
1884-1898: Proper use of AWS SDK v2 pointer conversion utilities.Good use of
aws.ToBool()for safe pointer-to-bool conversion, which handles nil pointers gracefully.
1917-1935: Improved S3 bucket creation with proper region configuration.Good addition of
CreateBucketConfigurationwithLocationConstraint. This is required for creating buckets in regions other than us-east-1 and follows AWS SDK v2 best practices.
1950-1966: Robust error handling for DynamoDB table cleanup.The error handling correctly checks for
ResourceNotFoundExceptionusing the smithy error pattern, avoiding unnecessary error logs for non-existent tables.
1984-2016: Excellent implementation of AWS SDK v2 waiter pattern.The migration properly uses the new waiter pattern with
NewTableExistsWaiter, which is more efficient and cleaner than the old SDK v1 approach. The 5-minute timeout is appropriate for DynamoDB table creation.internal/remotestate/backend/s3/client.go (11)
10-16: Import migration looks good!The AWS SDK imports have been correctly updated to v2, and the awshelper import now properly references the internal package.
75-77: Client struct fields properly updated for SDK v2The client fields have been correctly migrated to use AWS SDK v2 types.
82-105: Constructor properly migrated to AWS SDK v2 patternsThe NewClient function correctly:
- Accepts context as the first parameter
- Uses AWS SDK v2 config instead of session
- Creates clients using NewFromConfig
- Validates credentials with context support
110-113: Good defensive nil checkAdding this nil check prevents potential panics from nil pointer dereferences. This is a good practice for robust error handling.
665-668: Excellent improvements to bucket creation logicThe defensive nil check and region-specific configuration are great additions:
- Prevents nil pointer dereferences
- Correctly handles location constraints for non-us-east-1 regions
- Follows AWS SDK v2 best practices
This will prevent IllegalLocationConstraintException errors.
Also applies to: 676-685
719-758: Excellent defensive programming practicesThe comprehensive nil checks and validations are exemplary:
- Prevents nil pointer dereferences at multiple levels
- Provides clear, specific error messages
- Safely initializes the Statement slice to avoid panics
- Validates all required fields before proceeding
This significantly improves the robustness of the code.
Also applies to: 780-783
1978-1988: Good error handling pattern for S3 taggingThe
handleS3TaggingMethodNotAllowedhelper function properly handles the case where S3 bucket tagging is not supported in certain AWS configurations. This prevents failures in environments where tagging is restricted.
1437-1468: Well-implemented retry logic for DynamoDB table deletionThe manual retry loop properly handles the race condition where table deletion can fail due to concurrent tag updates. The implementation includes:
- Clear comments explaining the race condition
- Appropriate retry count and delays
- Progress logging for debugging
- Proper error handling for non-retriable errors
1673-1691: New S3 versioning method properly implementedThe
EnableVersioningForS3Bucketmethod correctly uses AWS SDK v2 types and follows the established patterns in the codebase.
1736-1781: Comprehensive SSE configuration checkingThe
checkIfSSEForS3MatchesConfigmethod thoroughly checks all aspects of the SSE configuration including:
- Algorithm matching
- KMS key ID verification when using KMS encryption
- Proper error handling for missing configurations
This ensures the bucket encryption matches the expected configuration.
1972-1976: Good helper function for AWS error checkingThe
isAWSResourceNotFoundErrorhelper provides a consistent way to check for resource not found errors across the codebase.test/fixtures/download/remote-with-backend/terragrunt.hcl (1)
6-6: Verified: v0.83.2 tag and sub-directory exist
The automated check confirms that thev0.83.2tag andtest/fixtures/download/hello-world-with-backendpath are present in thegruntwork-io/terragruntrepo. No further changes needed here.test/fixtures/get-output/regression-906/g/terragrunt.hcl (1)
10-10: Validate updated module referenceThe source URL now points to
test/fixtures/download/hello-world?ref=v0.83.2.
As with the previous fixture, please ensure the tag and path exist so the regression test remains green.
| if _, err := os.Stat(string(f)); err != nil { | ||
| // TODO: See if this lint error should be ignored | ||
| return []byte(f), nil //nolint: nilerr | ||
| } |
There was a problem hiding this comment.
Address the TODO and potential error suppression
The code has a TODO comment about a lint error and is suppressing a potential file stat error. This could hide important errors like permission issues.
Consider handling the error properly:
- // Check if token is a raw value
- if _, err := os.Stat(string(f)); err != nil {
- // TODO: See if this lint error should be ignored
- return []byte(f), nil //nolint: nilerr
- }
+ // Check if token is a file path
+ if _, err := os.Stat(string(f)); err != nil {
+ if os.IsNotExist(err) {
+ // Token is a raw value, not a file path
+ return []byte(f), nil
+ }
+ // Return other errors (e.g., permission issues)
+ return nil, errors.Errorf("failed to stat token file: %w", err)
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In internal/awshelper/config.go around lines 105 to 108, the code suppresses the
error from os.Stat by returning a byte slice and nil error, which can hide
important issues like permission errors. Remove the nolint directive and handle
the error properly by returning the error instead of nil when os.Stat fails,
ensuring that any file access problems are correctly propagated and not ignored.
| result, err := iamClient.ListAccountAliases(ctx, &iam.ListAccountAliasesInput{}) | ||
| if err != nil { | ||
| return "", err |
There was a problem hiding this comment.
Wrap the error for better context
This error should be wrapped to indicate the failure occurred while listing account aliases.
- result, err := iamClient.ListAccountAliases(ctx, &iam.ListAccountAliasesInput{})
- if err != nil {
- return "", err
- }
+ result, err := iamClient.ListAccountAliases(ctx, &iam.ListAccountAliasesInput{})
+ if err != nil {
+ return "", errors.Errorf("failed to list account aliases: %w", err)
+ }📝 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.
| result, err := iamClient.ListAccountAliases(ctx, &iam.ListAccountAliasesInput{}) | |
| if err != nil { | |
| return "", err | |
| result, err := iamClient.ListAccountAliases(ctx, &iam.ListAccountAliasesInput{}) | |
| if err != nil { | |
| return "", errors.Errorf("failed to list account aliases: %w", err) | |
| } |
🧰 Tools
🪛 GitHub Actions: Strict Lint
[error] 417-417: Error returned from external package is unwrapped; consider wrapping error for better context (wrapcheck).
🤖 Prompt for AI Agents
In internal/awshelper/config.go around lines 415 to 417, the error returned from
iamClient.ListAccountAliases is not wrapped with additional context. Modify the
error return to wrap the original error with a message indicating the failure
occurred while listing account aliases, using fmt.Errorf or a similar error
wrapping method to provide clearer context for debugging.
| if err := flag.value.Set(val); err != nil { | ||
| return err |
There was a problem hiding this comment.
Fix error wrapping for better error handling.
The pipeline failures indicate that errors returned from interface methods should be wrapped according to the wrapcheck linter.
Apply this fix to properly wrap the errors:
- if err := flag.value.Set(val); err != nil {
- return err
+ if err := flag.value.Set(val); err != nil {
+ return errors.Errorf("failed to set flag value: %w", err)Apply the same pattern to both occurrences in the EnvSet and Set methods.
Also applies to: 146-147
🧰 Tools
🪛 GitHub Actions: Strict Lint
[error] 127-127: Error returned from interface method should be wrapped for better error handling (wrapcheck).
🤖 Prompt for AI Agents
In internal/cli/flag.go at lines 126-127 and 146-147, the errors returned from
the flag.value.Set method are not wrapped, which is required by the wrapcheck
linter for better error handling. Modify the error returns to wrap the original
error using fmt.Errorf or errors.Wrap with a descriptive message, ensuring both
the EnvSet and Set methods follow this pattern consistently.
| } | ||
|
|
||
| return false, err |
There was a problem hiding this comment.
Wrap error for better context
The error should be wrapped to provide better context about the DynamoDB operation.
- return false, err
+ return false, errors.Errorf("failed to describe DynamoDB table %s: %w", tableName, err)📝 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.
| return false, err | |
| return false, errors.Errorf("failed to describe DynamoDB table %s: %w", tableName, err) |
🧰 Tools
🪛 GitHub Actions: Strict Lint
[error] 1312-1312: Error returned from external package is unwrapped; consider wrapping error for better context (wrapcheck).
🤖 Prompt for AI Agents
In internal/remotestate/backend/s3/client.go at line 1312, the returned error
should be wrapped to add context about the DynamoDB operation failure. Use an
error wrapping method to include a descriptive message along with the original
error before returning it, so that the caller can understand the source and
nature of the error more clearly.
| } | ||
|
|
||
| var input = dynamodb.TagResourceInput{ | ||
| ResourceArn: tableArn, | ||
| Tags: tagsConverted} | ||
|
|
||
| _, err := client.TagResourceWithContext(ctx, &input) | ||
| _, err := client.dynamoClient.TagResource(ctx, &input) | ||
|
|
||
| return err |
There was a problem hiding this comment.
Wrap error for better context
The error should be wrapped to provide better context.
- return err
+ return errors.Errorf("failed to tag DynamoDB table: %w", err)📝 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.
| return err | |
| return errors.Errorf("failed to tag DynamoDB table: %w", err) |
🧰 Tools
🪛 GitHub Actions: Strict Lint
[error] 1427-1427: Error returned from external package is unwrapped; consider wrapping error for better context (wrapcheck).
🤖 Prompt for AI Agents
In internal/remotestate/backend/s3/client.go at line 1427, the returned error
should be wrapped to add more context about where or why the error occurred. Use
an error wrapping method such as fmt.Errorf with %w or errors.Wrap to include a
descriptive message along with the original error before returning it.
| } | ||
| } | ||
|
|
||
| return err |
There was a problem hiding this comment.
Wrap error for better context
The error should be wrapped to provide better context about the delete operation.
- return err
+ return errors.Errorf("failed to delete DynamoDB table %s: %w", tableName, err)📝 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.
| return err | |
| return errors.Errorf("failed to delete DynamoDB table %s: %w", tableName, err) |
🧰 Tools
🪛 GitHub Actions: Strict Lint
[error] 1463-1463: Error returned from external package is unwrapped; consider wrapping error for better context (wrapcheck).
🤖 Prompt for AI Agents
In internal/remotestate/backend/s3/client.go at line 1463, the returned error
should be wrapped to add context about the delete operation. Modify the return
statement to wrap the error with a descriptive message indicating the failure
occurred during the delete process, using an error wrapping method such as
fmt.Errorf or errors.Wrap.
| @@ -664,7 +664,7 @@ func (runner *Runner) acquireCredentials(ctx context.Context, l log.Logger, opts | |||
| } | |||
|
|
|||
| // nolint:unparam | |||
There was a problem hiding this comment.
Wrap error for better context.
The pipeline failure indicates that the error returned from an external package should be wrapped for better context.
Apply this diff to wrap the error:
- return aws.Config{}, errors.New(err)
+ return aws.Config{}, errors.Errorf("failed to acquire credentials: %w", err)Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: Strict Lint
[error] 666-666: Error returned from external package is unwrapped; consider wrapping error for better context (wrapcheck).
🤖 Prompt for AI Agents
In internal/runner/configstack/runner.go at line 666, the error returned from an
external package is not wrapped, which reduces context for debugging. Modify the
code to wrap the error using fmt.Errorf or errors.Wrap (depending on the project
conventions) to add descriptive context before returning it. This will provide
clearer information about where and why the error occurred.
| t.Logf("Failed to delete S3 bucket %s: %v", bucketName, err) | ||
|
|
||
| return err |
There was a problem hiding this comment.
Wrap errors for better context.
The pipeline failures indicate that errors should be wrapped for better context.
Apply this diff to wrap the errors:
- return err
+ return errors.Errorf("failed to delete S3 bucket %s: %w", bucketName, err)Also applies to: 254-254
🧰 Tools
🪛 GitHub Actions: Strict Lint
[error] 251-251: Error returned from external package is unwrapped; consider wrapping error for better context (wrapcheck).
🤖 Prompt for AI Agents
In test/helpers/package.go at lines 251 and 254, the errors returned are not
wrapped, which reduces context for debugging. Update the return statements to
wrap the errors using fmt.Errorf or errors.Wrap with a descriptive message that
adds context about where or why the error occurred before returning it.
| if awsCfg.Region != "" { | ||
| configOptions = append(configOptions, config.WithRegion(awsCfg.Region)) | ||
| } else { | ||
| configOptions = append(configOptions, config.WithRegion("us-east-1")) |
There was a problem hiding this comment.
The default region is already being set in the CreateAwsConfig function
// Ensure a region is set - fallback to us-east-1 if none is configured
if cfg.Region == "" {
l.Debugf("No region configured, using default region us-east-1")
cfg.Region = "us-east-1"
}comment for this function suggests that instead of performing a fallback, we should return an error
There was a problem hiding this comment.
I added this because my changes introduced a regression in our test suite without it. I do think we need it, or another approach that's better to handle the fallback when no region is defined to avoid regression.
There was a problem hiding this comment.
Can confirm it's necessary:
https://github.com/gruntwork-io/terragrunt/actions/runs/16523674724/job/46731375349#step:10:493
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
internal/awshelper/config.go (4)
105-108: Address the TODO and potential error suppressionThe code suppresses potential file stat errors which could hide important errors like permission issues.
- // Check if token is a raw value - if _, err := os.Stat(string(f)); err != nil { - // TODO: See if this lint error should be ignored - return []byte(f), nil //nolint: nilerr - } + // Check if token is a file path + if _, err := os.Stat(string(f)); err != nil { + if os.IsNotExist(err) { + // Token is a raw value, not a file path + return []byte(f), nil + } + // Return other errors (e.g., permission issues) + return nil, errors.Errorf("failed to stat token file: %w", err) + }
145-147: Wrap the error for better contextThe error from
AssumeRoleWithWebIdentityshould be wrapped to provide context about where the error occurred.result, err := stsClient.AssumeRoleWithWebIdentity(ctx, input) if err != nil { - return aws.Credentials{}, err + return aws.Credentials{}, errors.Errorf("failed to assume role with web identity: %w", err) }
184-186: Wrap the error for better contextThe error from
AssumeRoleshould be wrapped to provide context about the failure.result, err := stsClient.AssumeRole(ctx, input) if err != nil { - return aws.Credentials{}, err + return aws.Credentials{}, errors.Errorf("failed to assume role: %w", err) }
377-379: Wrap the error for better contextThe error from
ListAccountAliasesshould be wrapped to indicate where the failure occurred.result, err := iamClient.ListAccountAliases(ctx, &iam.ListAccountAliasesInput{}) if err != nil { - return "", err + return "", errors.Errorf("failed to list account aliases: %w", err) }
🧹 Nitpick comments (1)
internal/awshelper/config.go (1)
31-31: UnusedTagsfield in AwsSessionConfigThe
Tagsfield in theAwsSessionConfigstruct appears to be unused throughout the codebase. Consider removing it if it's not needed.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
internal/awshelper/config.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/awshelper/config.go
⏰ 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). (23)
- GitHub Check: Pull Request has non-contributor approval
- GitHub Check: Run pre-commit hooks
- GitHub Check: Pull Request has non-contributor approval
- GitHub Check: Test (AWS with Latest Terraform)
- GitHub Check: Test (GCP)
- GitHub Check: Test (Fixtures with Latest OSS Terraform)
- GitHub Check: Test (SSH)
- GitHub Check: Test (Fixtures with OpenTofu)
- GitHub Check: Build (linux/386)
- GitHub Check: Build (windows/amd64)
- GitHub Check: Build (darwin/amd64)
- GitHub Check: Build (windows/386)
- GitHub Check: License Check
- GitHub Check: Build (linux/arm64)
- GitHub Check: Build (darwin/arm64)
- GitHub Check: Build (linux/amd64)
- GitHub Check: Build (linux/amd64)
- GitHub Check: lint
- GitHub Check: Test (macos)
- GitHub Check: Test (ubuntu)
- GitHub Check: Test OIDC (GHA AWS)
- GitHub Check: lint
- GitHub Check: build-and-test
| const ( | ||
| // Minimum ARN parts required for a valid ARN | ||
| minARNParts = 2 | ||
| ) |
There was a problem hiding this comment.
Incorrect ARN parts constant
The minARNParts constant is set to 2, but valid ARNs have at least 6 parts in the format arn:partition:service:region:account:resource. This will cause incorrect validation in the GetAWSPartition function.
const (
// Minimum ARN parts required for a valid ARN
- minARNParts = 2
+ minARNParts = 6
)📝 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.
| const ( | |
| // Minimum ARN parts required for a valid ARN | |
| minARNParts = 2 | |
| ) | |
| const ( | |
| // Minimum ARN parts required for a valid ARN | |
| minARNParts = 6 | |
| ) |
🤖 Prompt for AI Agents
In internal/awshelper/config.go around lines 24 to 27, the constant minARNParts
is incorrectly set to 2, but valid ARNs require at least 6 parts. Update the
minARNParts constant to 6 to correctly reflect the minimum number of ARN parts
needed for validation in the GetAWSPartition function.
internal/awshelper/config.go
Outdated
| // AssumeIamRole assumes an IAM role and returns the credentials | ||
| func AssumeIamRole( | ||
| ctx context.Context, | ||
| iamRoleOpts options.IAMRoleOptions, | ||
| externalID string, | ||
| opts *options.TerragruntOptions, | ||
| ) (*types.Credentials, error) { | ||
| // Set user agent to include terragrunt version | ||
| cfg, err := config.LoadDefaultConfig( | ||
| ctx, | ||
| config.WithAppID("terragrunt/"+version.GetVersion()), | ||
| ) | ||
| if err != nil { | ||
| return nil, errors.Errorf("Error loading AWS config: %w", err) | ||
| } | ||
|
|
||
| stsClient := sts.NewFromConfig(cfg) | ||
|
|
||
| roleSessionName := iamRoleOpts.AssumeRoleSessionName | ||
| if roleSessionName == "" { | ||
| roleSessionName = options.GetDefaultIAMAssumeRoleSessionName() | ||
| } | ||
|
|
||
| duration := time.Duration(options.DefaultIAMAssumeRoleDuration) * time.Second | ||
| if iamRoleOpts.AssumeRoleDuration > 0 { | ||
| duration = time.Duration(iamRoleOpts.AssumeRoleDuration) * time.Second | ||
| } | ||
|
|
||
| if iamRoleOpts.WebIdentityToken == "" { | ||
| // Use regular sts AssumeRole | ||
| input := &sts.AssumeRoleInput{ | ||
| RoleArn: aws.String(iamRoleOpts.RoleARN), | ||
| RoleSessionName: aws.String(roleSessionName), | ||
| DurationSeconds: aws.Int32(int32(duration.Seconds())), | ||
| } | ||
|
|
||
| if externalID != "" { | ||
| input.ExternalId = aws.String(externalID) | ||
| } | ||
|
|
||
| result, err := stsClient.AssumeRole(ctx, input) | ||
| if err != nil { | ||
| return nil, errors.Errorf("Error assuming role: %w", err) | ||
| } | ||
|
|
||
| return result.Credentials, nil | ||
| } | ||
|
|
||
| // Use sts AssumeRoleWithWebIdentity | ||
| var token string | ||
| // Check if value is a raw token or a path to a file with a token | ||
| if _, err := os.Stat(iamRoleOpts.WebIdentityToken); err != nil { | ||
| token = iamRoleOpts.WebIdentityToken | ||
| } else { | ||
| tb, err := os.ReadFile(iamRoleOpts.WebIdentityToken) | ||
| if err != nil { | ||
| return nil, errors.Errorf("Error reading web identity token file: %w", err) | ||
| } | ||
|
|
||
| token = string(tb) | ||
| } | ||
|
|
||
| input := &sts.AssumeRoleWithWebIdentityInput{ | ||
| RoleArn: aws.String(iamRoleOpts.RoleARN), | ||
| RoleSessionName: aws.String(roleSessionName), | ||
| WebIdentityToken: aws.String(token), | ||
| DurationSeconds: aws.Int32(int32(duration.Seconds())), | ||
| } | ||
|
|
||
| result, err := stsClient.AssumeRoleWithWebIdentity(ctx, input) | ||
| if err != nil { | ||
| return nil, errors.Errorf("Error assuming role with web identity: %w", err) | ||
| } | ||
|
|
||
| return result.Credentials, nil | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor to eliminate code duplication
This function duplicates logic from getWebIdentityCredentialsFromIAMRoleOptions, getSTSCredentialsFromIAMRoleOptions, and tokenFetcher. Consider refactoring to reuse these existing functions.
The function could be simplified by:
- Creating a temporary
AwsSessionConfigwith the role ARN - Using
CreateAwsConfigto get a configured client with assumed role credentials - Extracting credentials from the configured client
This would eliminate the duplicate role assumption logic and token handling.
🤖 Prompt for AI Agents
In internal/awshelper/config.go from lines 262 to 337, the AssumeIamRole
function duplicates logic found in getWebIdentityCredentialsFromIAMRoleOptions,
getSTSCredentialsFromIAMRoleOptions, and tokenFetcher. To fix this, refactor
AssumeIamRole to create a temporary AwsSessionConfig with the role ARN and use
CreateAwsConfig to obtain a configured AWS client that handles role assumption
and token fetching internally. Then extract and return the credentials from this
configured client, removing the manual role assumption and token reading code to
eliminate duplication.
Description
Moves
awshelperinto aninternalpackage, and upgrading all usage of the AWS SDK to v2.TODOs
Read the Gruntwork contribution guidelines.
Release Notes (draft)
Updated AWS SDK to v2 and moved
awshelperinto aninternalpackage.Migration Guide
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
Chores