Support !terraform.state on GCS Backends
#1393
Conversation
…Google Cloud auth Performance Improvements: - Add client caching to avoid recreating GCS clients for repeated operations - Implement retry logic with exponential backoff (up to 3 attempts) - Extend timeouts to 30 seconds to match S3 backend performance - Add comprehensive debug logging for better troubleshooting Unified Google Cloud Authentication: - Create internal/gcp package for unified authentication across all GCP services - Support both JSON credentials and file paths consistently - Leverage Google's Application Default Credentials (ADC) automatically - Unify authentication between GCS backend and Secret Manager store - Remove duplicate authentication logic and improve maintainability Enhanced Error Handling: - Improve resource cleanup (explicit close instead of defer in loops) - Add detailed error context and logging - Better handling of transient failures and missing files Test Coverage: - Add comprehensive test suite for unified authentication (4 test functions) - Enhance GCS backend tests with retry and caching scenarios - Maintain 100% backward compatibility with existing configurations This brings GCS backend performance and reliability in line with S3 backend while establishing a foundation for consistent Google Cloud integrations.
📝 WalkthroughWalkthroughAdds first-class GCS Terraform backend support: new GCP auth helpers, GCS client implementation with caching and retry logic, backend registration and validation, new error definitions, GSM store auth refactor to use unified auth, tests, docs, and a dependency change. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as Atmos
participant Auth as internal/gcp
participant Cache as GCS Client Cache
participant GCS as cloud.google.com/go/storage
participant Bucket as BucketHandle
participant Object as ObjectHandle
CLI->>Auth: Get credentials from backend/store
Auth-->>CLI: Return client options (JSON / file / ADC)
CLI->>Cache: Request client for cred+impersonation key
alt cached
Cache-->>CLI: Return cached client
else not cached
Cache->>GCS: Create client with options
GCS-->>Cache: Return client or error
Cache-->>CLI: Return client
end
CLI->>Bucket: Bucket(bucket)
Bucket->>Object: Object(state path)
CLI->>Object: NewReader(ctx) with retry/backoff
alt success
Object-->>CLI: Reader
CLI->>CLI: Read bytes, close reader
else error
Object-->>CLI: Return wrapped error (create/get/read)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
internal/gcp/auth.go (1)
17-18: Track the impersonation TODO.The service account impersonation feature is noted but not implemented. This is fine for the initial implementation.
Would you like me to create an issue to track adding service account impersonation support in a follow-up PR?
internal/terraform_backend/terraform_backend_gcs.go (1)
159-165: Service account impersonation needs implementation.The TODO for impersonation is logged but not implemented. This aligns with the TODO in the auth package.
Would you like me to help implement service account impersonation using
google.golang.org/api/impersonatein a follow-up PR?internal/terraform_backend/terraform_backend_gcs_test.go (3)
48-52: Move mock client creation outside the loop.The mock client doesn't change between test cases.
+ mockClient := &mockGCSClient{} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Create a mock client that should never be called due to validation errors - mockClient := &mockGCSClient{} - result, err := tb.ReadTerraformBackendGCSInternal(mockClient, &tt.componentData, &tt.gcsBackend)
126-126: Inconsistent test function naming.Use consistent naming - either
TestReadTerraformBackendGCSInternalor keep underscores for all similar tests.
347-349: Use consistent assertion style.Consider using assert package for consistency with other tests.
- if got != tt.expected { - t.Errorf("GetGCSBackendCredentials() = %v, want %v", got, tt.expected) - } + assert.Equal(t, tt.expected, got, "GetGCSBackendCredentials() should return expected value")And similarly for line 384-386:
- if got != tt.expected { - t.Errorf("GetGCSBackendImpersonateServiceAccount() = %v, want %v", got, tt.expected) - } + assert.Equal(t, tt.expected, got, "GetGCSBackendImpersonateServiceAccount() should return expected value")Also applies to: 384-386
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
errors/errors.go(1 hunks)go.mod(1 hunks)internal/exec/terraform_generate_backend.go(1 hunks)internal/gcp/auth.go(1 hunks)internal/gcp/auth_test.go(1 hunks)internal/terraform_backend/terraform_backend_gcs.go(1 hunks)internal/terraform_backend/terraform_backend_gcs_test.go(1 hunks)internal/terraform_backend/terraform_backend_registry.go(1 hunks)internal/terraform_backend/terraform_backend_utils.go(1 hunks)pkg/store/google_secret_manager_store.go(2 hunks)website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.go
📄 CodeRabbit Inference Engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags
Use interfaces for external dependencies to facilitate mocking
All code must pass golangci-lint checks
Follow Go's error handling idioms
Use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider using a custom error type for domain-specific errors
Follow standard Go coding style
Use gofmt and goimports to format code
Prefer short, descriptive variable names
Use snake_case for environment variables
Document all exported functions, types, and methods
Document complex logic with inline comments
Follow Go's documentation conventions
Use Viper for configuration management
Support configuration via files, environment variables, and flags
Follow the precedence order: flags > environment variables > config file > defaults
Files:
internal/exec/terraform_generate_backend.gointernal/terraform_backend/terraform_backend_utils.gointernal/terraform_backend/terraform_backend_registry.gopkg/store/google_secret_manager_store.gointernal/gcp/auth.goerrors/errors.gointernal/terraform_backend/terraform_backend_gcs.gointernal/gcp/auth_test.gointernal/terraform_backend/terraform_backend_gcs_test.go
go.mod
📄 CodeRabbit Inference Engine (.cursor/rules/atmos-rules.mdc)
Manage dependencies with Go modules
Files:
go.mod
website/**
📄 CodeRabbit Inference Engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in the website/ directory when adding new features
Follow the website's documentation structure and style
Keep website code in the website/ directory
Follow the existing website architecture and style
Document new features on the website
Include examples and use cases in website documentation
Files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
**/*_test.go
📄 CodeRabbit Inference Engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for testing multiple scenarios
Include integration tests for command flows
Test CLI end-to-end when possible
Use test fixtures for complex inputs
Consider using testify/mock for creating mock implementations
Files:
internal/gcp/auth_test.gointernal/terraform_backend/terraform_backend_gcs_test.go
🧠 Learnings (51)
📓 Common learnings
Learnt from: Listener430
PR: cloudposse/atmos#934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Learnt from: mcalhoun
PR: cloudposse/atmos#963
File: website/docs/core-concepts/projects/configuration/stores.mdx:286-286
Timestamp: 2025-04-25T20:54:19.701Z
Learning: For the AWS SSM Parameter Store implementation in Atmos, support for `read_role_arn` and `write_role_arn` options is essential to enable cross-account access, allowing users to run operations like `terraform plan` in multiple accounts while accessing values across keystores. Azure Key Vault would need similar capabilities for cross-tenant/subscription authentication.
📚 Learning: in `internal/exec/terraform.go`, skipping stack validation when help flags are present is not necess...
Learnt from: aknysh
PR: cloudposse/atmos#825
File: internal/exec/terraform.go:30-30
Timestamp: 2024-12-07T16:19:01.683Z
Learning: In `internal/exec/terraform.go`, skipping stack validation when help flags are present is not necessary.
Applied to files:
internal/exec/terraform_generate_backend.go
📚 Learning: the variable `shouldcheckstack` in `executeterraform` controls whether validation is performed....
Learnt from: osterman
PR: cloudposse/atmos#727
File: internal/exec/terraform.go:65-66
Timestamp: 2024-10-31T19:23:45.538Z
Learning: The variable `shouldCheckStack` in `ExecuteTerraform` controls whether validation is performed.
Applied to files:
internal/exec/terraform_generate_backend.go
📚 Learning: in the go function `processtagterraformoutput` in `internal/exec/yaml_func_terraform_output.go`, par...
Learnt from: aknysh
PR: cloudposse/atmos#810
File: internal/exec/yaml_func_terraform_output.go:35-40
Timestamp: 2024-11-30T22:07:08.610Z
Learning: In the Go function `processTagTerraformOutput` in `internal/exec/yaml_func_terraform_output.go`, parameters cannot contain spaces. The code splits the input by spaces, and if the parameters contain spaces, `len(parts) != 3` will fail and show an error to the user.
Applied to files:
internal/exec/terraform_generate_backend.go
📚 Learning: the "executeatmosvalidateschemacmd" function in internal/exec/validate_schema.go has been reviewed a...
Learnt from: samtholiya
PR: cloudposse/atmos#1147
File: internal/exec/validate_schema.go:42-57
Timestamp: 2025-04-11T22:06:46.999Z
Learning: The "ExecuteAtmosValidateSchemaCmd" function in internal/exec/validate_schema.go has been reviewed and confirmed to have acceptable cognitive complexity despite static analysis warnings. The function uses a clean structure with only three if statements for error handling and delegates complex operations to helper methods.
Applied to files:
internal/exec/terraform_generate_backend.go
📚 Learning: in `internal/exec/terraform_clean.go`, the function `determinecleanpath` is necessary and should not...
Learnt from: haitham911
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:215-223
Timestamp: 2024-10-27T04:41:49.199Z
Learning: In `internal/exec/terraform_clean.go`, the function `determineCleanPath` is necessary and should not be removed.
Applied to files:
internal/exec/terraform_generate_backend.go
📚 Learning: in `cmd/version.go`, when handling the `--check` flag in the `versioncmd`, avoid using `checkforatmo...
Learnt from: Listener430
PR: cloudposse/atmos#844
File: cmd/version.go:34-44
Timestamp: 2024-12-13T15:28:13.630Z
Learning: In `cmd/version.go`, when handling the `--check` flag in the `versionCmd`, avoid using `CheckForAtmosUpdateAndPrintMessage(cliConfig)` as it updates the cache timestamp, which may not be desired in this context.
Applied to files:
internal/exec/terraform_generate_backend.go
📚 Learning: in `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate va...
Learnt from: Listener430
PR: cloudposse/atmos#825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
internal/exec/terraform_generate_backend.go
📚 Learning: in the go code for `componentfunc` in `internal/exec/template_funcs_component.go`, the function `cle...
Learnt from: aknysh
PR: cloudposse/atmos#775
File: internal/exec/template_funcs_component.go:157-159
Timestamp: 2024-11-12T03:16:02.910Z
Learning: In the Go code for `componentFunc` in `internal/exec/template_funcs_component.go`, the function `cleanTerraformWorkspace` does not return errors, and it's acceptable if the file does not exist. Therefore, error handling for `cleanTerraformWorkspace` is not needed.
Applied to files:
internal/exec/terraform_generate_backend.gointernal/terraform_backend/terraform_backend_utils.gointernal/terraform_backend/terraform_backend_gcs.gointernal/terraform_backend/terraform_backend_gcs_test.go
📚 Learning: when specifying color values in functions like `confirmdeleteterraformlocal` in `internal/exec/terra...
Learnt from: osterman
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:233-235
Timestamp: 2024-10-31T19:25:41.298Z
Learning: When specifying color values in functions like `confirmDeleteTerraformLocal` in `internal/exec/terraform_clean.go`, avoid hardcoding color values. Instead, use predefined color constants or allow customization through configuration settings to improve accessibility and user experience across different terminals and themes.
Applied to files:
internal/exec/terraform_generate_backend.gointernal/terraform_backend/terraform_backend_utils.go
📚 Learning: in `internal/exec/terraform.go`, the workspace cleaning code under both the general execution path a...
Learnt from: aknysh
PR: cloudposse/atmos#759
File: internal/exec/terraform.go:366-368
Timestamp: 2024-11-02T15:35:09.958Z
Learning: In `internal/exec/terraform.go`, the workspace cleaning code under both the general execution path and within the `case "init":` block is intentionally duplicated because the code execution paths are different. The `.terraform/environment` file should be deleted before executing `terraform init` in both scenarios to ensure a clean state.
Applied to files:
internal/exec/terraform_generate_backend.gointernal/terraform_backend/terraform_backend_gcs_test.go
📚 Learning: applies to go.mod : manage dependencies with go modules...
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to go.mod : Manage dependencies with Go modules
Applied to files:
go.mod
📚 Learning: go version 1.23.0 was deliberately introduced by the maintainer (aknysh) in january 2025. while this...
Learnt from: aknysh
PR: cloudposse/atmos#944
File: go.mod:3-3
Timestamp: 2025-01-17T00:21:32.987Z
Learning: Go version 1.23.0 was deliberately introduced by the maintainer (aknysh) in January 2025. While this might be a pre-release or development version of Go, it has been approved for use in this project.
Applied to files:
go.mod
📚 Learning: the project uses go version 1.23.0 which has been confirmed by the maintainer to be working in produ...
Learnt from: aknysh
PR: cloudposse/atmos#944
File: go.mod:3-3
Timestamp: 2025-01-17T00:21:32.987Z
Learning: The project uses Go version 1.23.0 which has been confirmed by the maintainer to be working in production for months. Do not flag this as an invalid Go version.
Applied to files:
go.mod
📚 Learning: when replacing significant dependencies like `go-getter` that require extensive changes, prefer to a...
Learnt from: aknysh
PR: cloudposse/atmos#768
File: internal/exec/vendor_model_component.go:3-20
Timestamp: 2024-11-18T13:59:10.824Z
Learning: When replacing significant dependencies like `go-getter` that require extensive changes, prefer to address them in separate PRs.
Applied to files:
go.mod
📚 Learning: in the atmos project, gomplate v4 is imported with a blank import (`_ "github.com/hairyhenderson/gom...
Learnt from: aknysh
PR: cloudposse/atmos#1363
File: internal/exec/template_utils.go:18-18
Timestamp: 2025-07-05T20:59:02.914Z
Learning: In the Atmos project, gomplate v4 is imported with a blank import (`_ "github.com/hairyhenderson/gomplate/v4"`) alongside v3 imports to resolve AWS SDK version conflicts. V3 uses older AWS SDK versions that conflict with newer AWS modules used by Atmos. A full migration to v4 requires extensive refactoring due to API changes and should be handled in a separate PR.
Applied to files:
go.modinternal/terraform_backend/terraform_backend_utils.gopkg/store/google_secret_manager_store.gointernal/gcp/auth.go
📚 Learning: applies to **/*.go : use interfaces for external dependencies to facilitate mocking...
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*.go : Use interfaces for external dependencies to facilitate mocking
Applied to files:
go.modinternal/gcp/auth_test.gointernal/terraform_backend/terraform_backend_gcs_test.go
📚 Learning: all code must pass golangci-lint checks...
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: All code must pass golangci-lint checks
Applied to files:
go.modinternal/gcp/auth_test.gointernal/terraform_backend/terraform_backend_gcs_test.go
📚 Learning: applies to **/*.go : use viper for managing configuration, environment variables, and flags...
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*.go : Use Viper for managing configuration, environment variables, and flags
Applied to files:
go.mod
📚 Learning: go doesn't have a built-in secure string type. for sensitive data like tokens, using environment var...
Learnt from: samtholiya
PR: cloudposse/atmos#1077
File: pkg/downloader/custom_github_detector.go:47-68
Timestamp: 2025-02-20T18:33:16.567Z
Learning: Go doesn't have a built-in secure string type. For sensitive data like tokens, using environment variables (which are protected by the OS) is a common and secure practice.
Applied to files:
go.modpkg/store/google_secret_manager_store.go
📚 Learning: applies to **/*.go : use viper for configuration management...
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*.go : Use Viper for configuration management
Applied to files:
go.mod
📚 Learning: the code in pkg/config/go-homedir is a direct fork of the mitchellh/go-homedir package and was inten...
Learnt from: osterman
PR: cloudposse/atmos#1139
File: pkg/config/go-homedir/homedir.go:183-196
Timestamp: 2025-03-12T21:38:42.699Z
Learning: The code in pkg/config/go-homedir is a direct fork of the mitchellh/go-homedir package and was intentionally imported as-is without modifications to maintain consistency with the original source. Security concerns or other improvements may be addressed in future PRs.
Applied to files:
go.modpkg/store/google_secret_manager_store.go
📚 Learning: applies to **/*.go : support configuration via files, environment variables, and flags...
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*.go : Support configuration via files, environment variables, and flags
Applied to files:
go.modinternal/gcp/auth.go
📚 Learning: the redis dependency (github.com/redis/go-redis/v9) in the atmos project is only used in tests, not ...
Learnt from: aknysh
PR: cloudposse/atmos#1274
File: go.mod:63-63
Timestamp: 2025-05-30T03:21:37.197Z
Learning: The redis dependency (github.com/redis/go-redis/v9) in the atmos project is only used in tests, not in production code.
Applied to files:
go.modpkg/store/google_secret_manager_store.go
📚 Learning: applies to **/*.go : provide clear error messages to users...
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*.go : Provide clear error messages to users
Applied to files:
internal/terraform_backend/terraform_backend_utils.goerrors/errors.go
📚 Learning: applies to **/*.go : use meaningful error messages...
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*.go : Use meaningful error messages
Applied to files:
internal/terraform_backend/terraform_backend_utils.goerrors/errors.go
📚 Learning: applies to **/*.go : consider using a custom error type for domain-specific errors...
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*.go : Consider using a custom error type for domain-specific errors
Applied to files:
internal/terraform_backend/terraform_backend_utils.goerrors/errors.go
📚 Learning: the error variable `errrelpath` is defined in `internal/exec/terraform_clean_util.go` and is used ac...
Learnt from: haitham911
PR: cloudposse/atmos#1195
File: internal/exec/terraform_clean.go:99-99
Timestamp: 2025-04-26T15:54:10.506Z
Learning: The error variable `ErrRelPath` is defined in `internal/exec/terraform_clean_util.go` and is used across files in the `exec` package, including in `terraform_clean.go`. This is part of an approach to standardize error handling in the codebase.
Applied to files:
internal/terraform_backend/terraform_backend_utils.go
📚 Learning: in the go codebase for atmos, when reviewing functions like `checkatmosconfig` in `cmd/cmd_utils.go`...
Learnt from: osterman
PR: cloudposse/atmos#740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.
Applied to files:
internal/terraform_backend/terraform_backend_utils.gopkg/store/google_secret_manager_store.goerrors/errors.gointernal/terraform_backend/terraform_backend_gcs_test.go
📚 Learning: in the 'atmos' project, when reviewing go code like `pkg/config/config.go`, avoid suggesting file si...
Learnt from: osterman
PR: cloudposse/atmos#808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.
Applied to files:
internal/terraform_backend/terraform_backend_utils.go
📚 Learning: in `cmd/cmd_utils.go`, it's acceptable to have hardcoded references to `atmos.yaml` in logs, and it'...
Learnt from: haitham911
PR: cloudposse/atmos#736
File: pkg/config/const.go:6-6
Timestamp: 2024-10-20T13:12:46.499Z
Learning: In `cmd/cmd_utils.go`, it's acceptable to have hardcoded references to `atmos.yaml` in logs, and it's not necessary to update them to use the `CliConfigFileName` constant.
Applied to files:
internal/terraform_backend/terraform_backend_utils.go
📚 Learning: applies to **/*.go : follow go's error handling idioms...
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*.go : Follow Go's error handling idioms
Applied to files:
errors/errors.go
📚 Learning: in the `terraform.output.mdx` documentation file (`website/docs/core-concepts/stacks/yaml-functions/...
Learnt from: aknysh
PR: cloudposse/atmos#810
File: website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx:0-0
Timestamp: 2024-12-03T04:01:16.446Z
Learning: In the `terraform.output.mdx` documentation file (`website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx`), the cache invalidation and cache scope behavior for the `!terraform.output` function are already described.
Applied to files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
📚 Learning: in `examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml`, `!exec atmos te...
Learnt from: aknysh
PR: cloudposse/atmos#810
File: examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml:28-32
Timestamp: 2024-12-01T00:33:20.298Z
Learning: In `examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml`, `!exec atmos terraform output` is used in examples to demonstrate its usage, even though `!terraform.output` is the recommended approach according to the documentation.
Applied to files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
📚 Learning: in the documentation for `!terraform.output`, warnings about template variable availability are alre...
Learnt from: aknysh
PR: cloudposse/atmos#810
File: website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx:104-110
Timestamp: 2024-12-03T03:49:30.395Z
Learning: In the documentation for `!terraform.output`, warnings about template variable availability are already covered in other sections, so no need to suggest adding them here.
Applied to files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
📚 Learning: the atmos yaml function `!env` is used to retrieve environment variables and assign them to sections...
Learnt from: aknysh
PR: cloudposse/atmos#0
File: :0-0
Timestamp: 2025-01-19T22:30:27.600Z
Learning: The Atmos YAML function `!env` is used to retrieve environment variables and assign them to sections in stack manifests. It supports both simple types (string, number, boolean) and complex types (JSON-encoded lists, maps, objects).
Applied to files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
📚 Learning: the atmos yaml function `!include` allows downloading local or remote files from different sources a...
Learnt from: aknysh
PR: cloudposse/atmos#0
File: :0-0
Timestamp: 2025-01-19T22:30:27.600Z
Learning: The Atmos YAML function `!include` allows downloading local or remote files from different sources and assigning their contents to sections in stack manifests. It supports various protocols (file, http, git, s3, etc.) and can filter content using YQ expressions.
Applied to files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
📚 Learning: in the context of the atmos project, it's acceptable for functions like `execterraformoutput` to rem...
Learnt from: aknysh
PR: cloudposse/atmos#810
File: internal/exec/terraform_utils.go:40-213
Timestamp: 2024-12-03T05:18:49.169Z
Learning: In the context of the Atmos project, it's acceptable for functions like `execTerraformOutput` to remain as single functions if they perform a single purpose, such as retrieving Terraform outputs for a component in a stack, even if the function is lengthy.
Applied to files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
📚 Learning: in the `getstackterraformstatefolder` function, it's acceptable and not an error if no terraform sta...
Learnt from: haitham911
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:183-187
Timestamp: 2024-10-27T04:34:08.011Z
Learning: In the `getStackTerraformStateFolder` function, it's acceptable and not an error if no Terraform state folders are found for a stack.
Applied to files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
📚 Learning: for the aws ssm parameter store implementation in atmos, support for `read_role_arn` and `write_role...
Learnt from: mcalhoun
PR: cloudposse/atmos#963
File: website/docs/core-concepts/projects/configuration/stores.mdx:286-286
Timestamp: 2025-04-25T20:54:19.701Z
Learning: For the AWS SSM Parameter Store implementation in Atmos, support for `read_role_arn` and `write_role_arn` options is essential to enable cross-account access, allowing users to run operations like `terraform plan` in multiple accounts while accessing values across keystores. Azure Key Vault would need similar capabilities for cross-tenant/subscription authentication.
Applied to files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
📚 Learning: in the atmos project, a 5-minute timeout in the `execterraformoutput` function is acceptable for ret...
Learnt from: aknysh
PR: cloudposse/atmos#810
File: internal/exec/terraform_utils.go:145-146
Timestamp: 2024-12-03T05:29:07.718Z
Learning: In the Atmos project, a 5-minute timeout in the `execTerraformOutput` function is acceptable for retrieving `terraform output` for a component in a stack.
Applied to files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
📚 Learning: unit tests for remote state backend functionality are located in the `examples/tests` directory....
Learnt from: aknysh
PR: cloudposse/atmos#863
File: internal/exec/stack_utils.go:201-223
Timestamp: 2024-12-17T07:10:26.295Z
Learning: Unit tests for remote state backend functionality are located in the `examples/tests` directory.
Applied to files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdxinternal/terraform_backend/terraform_backend_gcs_test.go
📚 Learning: in `internal/exec/terraform_clean.go`, when `getstackterraformstatefolder` returns an error in the `...
Learnt from: haitham911
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:407-416
Timestamp: 2024-11-24T19:13:10.287Z
Learning: In `internal/exec/terraform_clean.go`, when `getStackTerraformStateFolder` returns an error in the `handleCleanSubCommand` function, the error is logged, and the process continues without returning the error.
Applied to files:
internal/terraform_backend/terraform_backend_gcs.go
📚 Learning: applies to **/*_test.go : every new feature must include comprehensive unit tests...
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests
Applied to files:
internal/gcp/auth_test.gointernal/terraform_backend/terraform_backend_gcs_test.go
📚 Learning: applies to **/*_test.go : test both happy paths and error conditions...
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Test both happy paths and error conditions
Applied to files:
internal/gcp/auth_test.gointernal/terraform_backend/terraform_backend_gcs_test.go
📚 Learning: applies to **/*_test.go : consider using testify/mock for creating mock implementations...
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Consider using testify/mock for creating mock implementations
Applied to files:
internal/gcp/auth_test.gointernal/terraform_backend/terraform_backend_gcs_test.go
📚 Learning: applies to **/*_test.go : use table-driven tests for testing multiple scenarios...
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Use table-driven tests for testing multiple scenarios
Applied to files:
internal/gcp/auth_test.gointernal/terraform_backend/terraform_backend_gcs_test.go
📚 Learning: applies to **/*_test.go : include integration tests for command flows...
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Include integration tests for command flows
Applied to files:
internal/gcp/auth_test.gointernal/terraform_backend/terraform_backend_gcs_test.go
📚 Learning: applies to **/*_test.go : use test fixtures for complex inputs...
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Use test fixtures for complex inputs
Applied to files:
internal/gcp/auth_test.gointernal/terraform_backend/terraform_backend_gcs_test.go
📚 Learning: the atmos codebase has a custom extension to *testing.t that provides a chdir method, allowing test ...
Learnt from: samtholiya
PR: cloudposse/atmos#1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: The atmos codebase has a custom extension to *testing.T that provides a Chdir method, allowing test functions to call t.Chdir() to change working directories during tests. This is used consistently across test files in the codebase.
Applied to files:
internal/gcp/auth_test.go
📚 Learning: ci should run unit tests, integration tests, golangci-lint, and coverage reporting...
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: CI should run unit tests, integration tests, golangci-lint, and coverage reporting
Applied to files:
internal/terraform_backend/terraform_backend_gcs_test.go
🧬 Code Graph Analysis (2)
internal/terraform_backend/terraform_backend_utils.go (2)
errors/errors.go (1)
ErrUnsupportedBackendType(49-49)internal/terraform_backend/terraform_backend_utils_test.go (1)
TestGetTerraformBackend(103-190)
pkg/store/google_secret_manager_store.go (1)
internal/gcp/auth.go (3)
GetClientOptions(32-48)AuthOptions(10-19)GetCredentialsFromStore(61-66)
🪛 LanguageTool
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
[style] ~24-~24: Consider using a more formal/concise alternative here.
Context: ...accordingly. If you are using backends other than the supported ones, consider the [__`!s...
(OTHER_THAN)
⏰ 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: Summary
🔇 Additional comments (16)
go.mod (1)
7-7: Clean dependency declaration for GCS support.Moving
cloud.google.com/go/storagefrom indirect to direct requirements properly reflects its usage in the new GCS backend implementation.errors/errors.go (1)
54-57: Well-defined error variables for GCS backend operations.The new error variables follow established patterns and provide clear, meaningful messages. The generic
ErrInvalidBackendConfigis a nice addition that can be reused across backends.internal/terraform_backend/terraform_backend_utils.go (1)
119-119: Error message updated to reflect GCS support.Good maintenance - the error message now accurately lists all currently supported backend types.
internal/exec/terraform_generate_backend.go (1)
87-92: Proper validation for GCS backend configuration.The validation follows the established pattern for S3 backend validation and ensures required configuration is present. Clear error messaging helps users understand what's missing.
internal/terraform_backend/terraform_backend_registry.go (1)
23-23: GCS backend registration completed.This replaces the nil placeholder with the actual implementation, following the same pattern as other backend registrations. Clean integration.
pkg/store/google_secret_manager_store.go (1)
15-16: Clean integration of unified GCP authentication.The refactoring to use the centralized authentication utilities is well done. It removes duplicate authentication logic and ensures consistent credential handling across GCP services.
Also applies to: 61-64
internal/gcp/auth.go (1)
1-48: Solid foundation for unified GCP authentication.The authentication utility is well-structured with clear separation of concerns. The automatic handling of JSON vs file path credentials and fallback to ADC provides good flexibility.
internal/gcp/auth_test.go (1)
1-178: Comprehensive test coverage for authentication utilities.The tests properly cover all credential scenarios and edge cases. Good use of table-driven tests as per project guidelines.
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx (1)
16-27: Clear documentation for GCS backend support.The documentation effectively explains the new GCS backend functionality with practical examples and comprehensive authentication options. The structure matches existing backend documentation patterns.
Also applies to: 356-398
internal/terraform_backend/terraform_backend_gcs.go (1)
113-136: Well-structured state reading implementation.The separation between public and internal functions, proper backend extraction, and integration with the caching layer is clean and maintainable.
internal/terraform_backend/terraform_backend_gcs_test.go (6)
1-16: LGTM!Package declaration and imports are properly structured.
69-124: Well-structured mock implementations.Clean interface implementations with realistic GCS behavior simulation.
143-168: Good coverage of nested configuration scenario.Tests realistic usage pattern from YAML functions.
170-204: Clean error injection mocks.Effective design for testing various failure scenarios.
205-301: Comprehensive error scenario coverage.Excellent table-driven tests covering all major error paths including gRPC errors, timeouts, and IO failures.
446-486: Excellent caching verification tests.Properly validates cache key generation for different authentication configurations.
Critical fixes: - Fix indentation issue in test assertions - Replace len() with SHA256 hash for cache key security - Fix context leak in retry loop by removing defer from loop Code quality improvements: - Move mock client creation outside test loop for performance - Standardize test function naming (Test_* -> Test*) - Use consistent assert.Equal() style instead of manual error checks - Optimize resource management and prevent cache collisions All tests pass with improved performance and security.
- Remove impersonate_service_account from example configurations - Update authentication methods to remove impersonation reference - Add clear limitation notice that impersonation is not yet implemented - Remove commented impersonation example in configuration This ensures users understand current limitations and prevents confusion about unsupported features.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx (1)
360-377: Minor YAML hygiene for the exampleThe example is great, but two tiny tweaks improve copy-paste safety:
credentialspaths that start with/can break when the file isn’t present on a reader’s machine. Consider replacing with a placeholder environment variable reference to encourage best practice:- credentials: "/path/to/service-account-key.json" + credentials: "${GOOGLE_APPLICATION_CREDENTIALS}"
- Since the third argument is a YQ expression containing brackets, quoting it avoids accidental YAML parsing surprises:
- public_subnet_id: !terraform.state subnets dev .subnet_ids[0] + public_subnet_id: !terraform.state subnets dev ".subnet_ids[0]"Not critical, but they make the snippet safer for newcomers.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
website/**
📄 CodeRabbit Inference Engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in the website/ directory when adding new features
Follow the website's documentation structure and style
Keep website code in the website/ directory
Follow the existing website architecture and style
Document new features on the website
Include examples and use cases in website documentation
Files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
🧠 Learnings (11)
📓 Common learnings
Learnt from: Listener430
PR: cloudposse/atmos#934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Learnt from: mcalhoun
PR: cloudposse/atmos#963
File: website/docs/core-concepts/projects/configuration/stores.mdx:286-286
Timestamp: 2025-04-25T20:54:19.701Z
Learning: For the AWS SSM Parameter Store implementation in Atmos, support for `read_role_arn` and `write_role_arn` options is essential to enable cross-account access, allowing users to run operations like `terraform plan` in multiple accounts while accessing values across keystores. Azure Key Vault would need similar capabilities for cross-tenant/subscription authentication.
📚 Learning: in the `terraform.output.mdx` documentation file (`website/docs/core-concepts/stacks/yaml-functions/...
Learnt from: aknysh
PR: cloudposse/atmos#810
File: website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx:0-0
Timestamp: 2024-12-03T04:01:16.446Z
Learning: In the `terraform.output.mdx` documentation file (`website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx`), the cache invalidation and cache scope behavior for the `!terraform.output` function are already described.
Applied to files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
📚 Learning: in `examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml`, `!exec atmos te...
Learnt from: aknysh
PR: cloudposse/atmos#810
File: examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml:28-32
Timestamp: 2024-12-01T00:33:20.298Z
Learning: In `examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml`, `!exec atmos terraform output` is used in examples to demonstrate its usage, even though `!terraform.output` is the recommended approach according to the documentation.
Applied to files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
📚 Learning: in the documentation for `!terraform.output`, warnings about template variable availability are alre...
Learnt from: aknysh
PR: cloudposse/atmos#810
File: website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx:104-110
Timestamp: 2024-12-03T03:49:30.395Z
Learning: In the documentation for `!terraform.output`, warnings about template variable availability are already covered in other sections, so no need to suggest adding them here.
Applied to files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
📚 Learning: the atmos yaml function `!env` is used to retrieve environment variables and assign them to sections...
Learnt from: aknysh
PR: cloudposse/atmos#0
File: :0-0
Timestamp: 2025-01-19T22:30:27.600Z
Learning: The Atmos YAML function `!env` is used to retrieve environment variables and assign them to sections in stack manifests. It supports both simple types (string, number, boolean) and complex types (JSON-encoded lists, maps, objects).
Applied to files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
📚 Learning: the atmos yaml function `!include` allows downloading local or remote files from different sources a...
Learnt from: aknysh
PR: cloudposse/atmos#0
File: :0-0
Timestamp: 2025-01-19T22:30:27.600Z
Learning: The Atmos YAML function `!include` allows downloading local or remote files from different sources and assigning their contents to sections in stack manifests. It supports various protocols (file, http, git, s3, etc.) and can filter content using YQ expressions.
Applied to files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
📚 Learning: in the context of the atmos project, it's acceptable for functions like `execterraformoutput` to rem...
Learnt from: aknysh
PR: cloudposse/atmos#810
File: internal/exec/terraform_utils.go:40-213
Timestamp: 2024-12-03T05:18:49.169Z
Learning: In the context of the Atmos project, it's acceptable for functions like `execTerraformOutput` to remain as single functions if they perform a single purpose, such as retrieving Terraform outputs for a component in a stack, even if the function is lengthy.
Applied to files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
📚 Learning: in the `getstackterraformstatefolder` function, it's acceptable and not an error if no terraform sta...
Learnt from: haitham911
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:183-187
Timestamp: 2024-10-27T04:34:08.011Z
Learning: In the `getStackTerraformStateFolder` function, it's acceptable and not an error if no Terraform state folders are found for a stack.
Applied to files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
📚 Learning: for the aws ssm parameter store implementation in atmos, support for `read_role_arn` and `write_role...
Learnt from: mcalhoun
PR: cloudposse/atmos#963
File: website/docs/core-concepts/projects/configuration/stores.mdx:286-286
Timestamp: 2025-04-25T20:54:19.701Z
Learning: For the AWS SSM Parameter Store implementation in Atmos, support for `read_role_arn` and `write_role_arn` options is essential to enable cross-account access, allowing users to run operations like `terraform plan` in multiple accounts while accessing values across keystores. Azure Key Vault would need similar capabilities for cross-tenant/subscription authentication.
Applied to files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
📚 Learning: in the atmos project, a 5-minute timeout in the `execterraformoutput` function is acceptable for ret...
Learnt from: aknysh
PR: cloudposse/atmos#810
File: internal/exec/terraform_utils.go:145-146
Timestamp: 2024-12-03T05:29:07.718Z
Learning: In the Atmos project, a 5-minute timeout in the `execTerraformOutput` function is acceptable for retrieving `terraform output` for a component in a stack.
Applied to files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
📚 Learning: unit tests for remote state backend functionality are located in the `examples/tests` directory....
Learnt from: aknysh
PR: cloudposse/atmos#863
File: internal/exec/stack_utils.go:201-223
Timestamp: 2024-12-17T07:10:26.295Z
Learning: Unit tests for remote state backend functionality are located in the `examples/tests` directory.
Applied to files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
🪛 LanguageTool
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
[style] ~24-~24: Consider using a more formal/concise alternative here.
Context: ...accordingly. If you are using backends other than the supported ones, consider the [__`!s...
(OTHER_THAN)
⏰ 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: Summary
🔇 Additional comments (1)
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx (1)
381-397: Impersonation support is documented as “not yet implemented”, but the PR description says it IS implemented – please reconcileThe PR objectives explicitly list “service account impersonation” as a supported authentication method, yet this section:
- omits
impersonate_service_accountfrom the auth-method list (Lines 384-387)- omits the parameter from the config table (Lines 390-394)
- states in the note (Lines 395-397) that it “is parsed but not yet implemented”.
This directly contradicts the PR summary and will confuse users.
Suggested fix:
@@ -1. **Service Account Key File**: Specify the path to a service account JSON key file using the `credentials` parameter. -2. **Default Credentials**: When no credentials are specified, the Google Cloud SDK default credentials are used. -3. **Workload Identity** (GKE): Automatically uses the workload identity when running in GKE with Workload Identity enabled. +1. **Service Account Key File** – specify the path to a service-account JSON key via `credentials`. +2. **Default Credentials** – if `credentials` is omitted, Application-Default Credentials (ADC) are used. +3. **Workload Identity** (GKE) – automatically picked up when running in clusters with WI enabled. +4. **Service Account Impersonation** – set `impersonate_service_account` to the email of the target service account. @@ -- `state_file`: Optional custom name for the state file (defaults to `default.tfstate`) +- `state_file`: Optional custom name for the state file (defaults to `default.tfstate`) +- `impersonate_service_account`: Optional service-account email to impersonate @@ -:::note Current Limitations -The `impersonate_service_account` parameter is parsed but not yet implemented. This feature is planned for a future release. -::: +:::tip +`impersonate_service_account` is fully supported starting with this release. +:::Adjust wording if the implementation status is different, but ensure docs and code agree.
Likely an incorrect or invalid review comment.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // Check if the `backend` section has `bucket` when `backend_type` is `gcs` | ||
| if info.ComponentBackendType == "gcs" { | ||
| if _, ok := info.ComponentBackendSection["bucket"].(string); !ok { | ||
| return fmt.Errorf("backend config for the '%s' component is missing 'bucket'", component) |
Check failure
Code scanning / golangci-lint
do not define dynamic errors, use wrapped static errors instead: "fmt.Errorf("backend config for the '%s' component is missing 'bucket'", component)" Error
| } | ||
| } | ||
|
|
||
| // Helper function to create string pointers for tests |
Check failure
Code scanning / golangci-lint
Comment should end in a period Error test
| NewReader(ctx context.Context) (io.ReadCloser, error) | ||
| } | ||
|
|
||
| // Concrete implementations for production use |
Check failure
Code scanning / golangci-lint
Comment should end in a period Error
| return content, nil | ||
| } | ||
|
|
||
| return nil, fmt.Errorf("%w: %v", errUtils.ErrGetObjectFromGCS, lastErr) |
Check failure
Code scanning / golangci-lint
add-constant: string literal "%w: %v" appears, at least, 4 times, create a named constant for it Error
| } | ||
| } | ||
|
|
||
| // Mock implementations for testing |
Check failure
Code scanning / golangci-lint
Comment should end in a period Error test
| assert.Contains(t, string(content), "mocked-gcs-output") | ||
| } | ||
|
|
||
| // Error testing mock |
Check failure
Code scanning / golangci-lint
Comment should end in a period Error test
| return o.client.body, nil | ||
| } | ||
|
|
||
| // Simulate failure in io.ReadAll |
Check failure
Code scanning / golangci-lint
Comment should end in a period Error test
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1393 +/- ##
==========================================
- Coverage 55.29% 55.25% -0.04%
==========================================
Files 272 274 +2
Lines 28491 28648 +157
==========================================
+ Hits 15754 15830 +76
- Misses 10960 11037 +77
- Partials 1777 1781 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@shirkevich thanks a lot for the PR can you please review these issues (should be easy to fix) https://github.com/cloudposse/atmos/pull/1393/checks?check_run_id=47682846306 Please use these constants for error wrapping formats https://github.com/cloudposse/atmos/blob/main/errors/errors.go#L8 Please use the constant https://github.com/cloudposse/atmos/blob/main/pkg/config/const.go#L86 instead of the strings |
osterman
changed the title
!terraform.state on GCS Backends
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx (3)
22-27: Tighten wording and align tone in the tipMinor copyedits for clarity/conciseness and to address the style lint hint on Line 24.
-As support for new backend types is added (e.g. `azurerm`), this document will be updated accordingly. - -If you are using backends other than the supported ones, consider the [__`!store`__](/core-concepts/stacks/yaml-functions/store) -or [__`!terraform.output`__](/core-concepts/stacks/yaml-functions/terraform.output) YAML function to read remote state -and [share data between components](/core-concepts/share-data). +As support for additional backend types (for example, `azurerm`) is added, this page will be updated. + +If your workspace uses a backend not listed above, use the [__`!store`__](/core-concepts/stacks/yaml-functions/store) +or [__`!terraform.output`__](/core-concepts/stacks/yaml-functions/terraform.output) functions to read remote state and +[share data between components](/core-concepts/share-data).
356-377: Expand the GCS example to cover optional fields and preferred auth pathRecommend demonstrating ADC-first auth and optional fields users commonly need.
File title="stacks/gcp/dev.yaml" @@ my_component: backend_type: gcs backend: gcs: bucket: "my-terraform-state" prefix: "terraform/state" - credentials: "/path/to/service-account-key.json" + # Auth options (choose one; if omitted, Atmos uses Application Default Credentials): + # credentials: "/path/to/service-account-key.json" # Path to SA JSON key file + # credentials: '{"type":"service_account", ...}' # Inline SA JSON (single line) + # impersonate_service_account: "[email protected]" # If supported by implementation + # Optional when you diverge from the default file name: + # state_file: "default.tfstate" vars: # Read state from another component vpc_id: !terraform.state vpc dev vpc_id subnet_ids: !terraform.state subnets dev subnet_ids # Use YQ expression to get specific subnet public_subnet_id: !terraform.state subnets dev .subnet_ids[0]Note: apply the
impersonate_service_accountexample only if the backend implementation supports it (see next comments).
388-394: Parameter list: add periods and include impersonation (if supported)Fix the grammar flagged by the linter and document
impersonate_service_accountfor completeness.-- `bucket`: The GCS bucket name where Terraform state files are stored -- `prefix`: Optional prefix for the state file path within the bucket -- `credentials`: Optional path to a service account JSON key file -- `state_file`: Optional custom name for the state file (defaults to `default.tfstate`) +- `bucket`: The GCS bucket name where Terraform state files are stored. +- `prefix`: Optional prefix for the state file path within the bucket. +- `credentials`: Optional service account JSON; either a file path or an inline JSON string. +- `state_file`: Optional custom name for the state file (defaults to `default.tfstate`). + If omitted, the default is used under the configured `prefix`. +- `impersonate_service_account`: Optional email of the service account to impersonate (if supported by the implementation).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
go.mod(1 hunks)website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- go.mod
🧰 Additional context used
📓 Path-based instructions (1)
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in the website/ directory when adding new features
Follow the website's documentation structure and style
Keep website code in the website/ directory
Follow the existing website architecture and style
Document new features on the website
Include examples and use cases in website documentation
Files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
🧠 Learnings (4)
📚 Learning: 2024-12-03T04:01:16.446Z
Learnt from: aknysh
PR: cloudposse/atmos#810
File: website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx:0-0
Timestamp: 2024-12-03T04:01:16.446Z
Learning: In the `terraform.output.mdx` documentation file (`website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx`), the cache invalidation and cache scope behavior for the `!terraform.output` function are already described.
Applied to files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
📚 Learning: 2024-12-01T00:33:20.298Z
Learnt from: aknysh
PR: cloudposse/atmos#810
File: examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml:28-32
Timestamp: 2024-12-01T00:33:20.298Z
Learning: In `examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml`, `!exec atmos terraform output` is used in examples to demonstrate its usage, even though `!terraform.output` is the recommended approach according to the documentation.
Applied to files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
📚 Learning: 2025-01-19T22:30:27.600Z
Learnt from: aknysh
PR: cloudposse/atmos#0
File: :0-0
Timestamp: 2025-01-19T22:30:27.600Z
Learning: The Atmos YAML function `!env` is used to retrieve environment variables and assign them to sections in stack manifests. It supports both simple types (string, number, boolean) and complex types (JSON-encoded lists, maps, objects).
Applied to files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
📚 Learning: 2025-01-19T22:30:27.600Z
Learnt from: aknysh
PR: cloudposse/atmos#0
File: :0-0
Timestamp: 2025-01-19T22:30:27.600Z
Learning: The Atmos YAML function `!include` allows downloading local or remote files from different sources and assigning their contents to sections in stack manifests. It supports various protocols (file, http, git, s3, etc.) and can filter content using YQ expressions.
Applied to files:
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
🪛 LanguageTool
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx
[grammar] ~16-~16: There might be a mistake here.
Context: ...ts/terraform/backends). :::tip The !terraform.state YAML function sup...
(QB_NEW_EN)
[style] ~24-~24: Consider using a more formal/concise alternative here.
Context: ...accordingly. If you are using backends other than the supported ones, consider the [__`!s...
(OTHER_THAN)
[grammar] ~390-~390: There might be a mistake here.
Context: ...e where Terraform state files are stored - prefix: Optional prefix for the state file pat...
(QB_NEW_EN)
[grammar] ~391-~391: There might be a mistake here.
Context: ...or the state file path within the bucket - credentials: Optional path to a service account JSO...
(QB_NEW_EN)
[grammar] ~392-~392: There might be a mistake here.
Context: ... path to a service account JSON key file - state_file: Optional custom name for the state fil...
(QB_NEW_EN)
⏰ 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: Summary
🔇 Additional comments (2)
website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx (2)
16-21: Good addition of GCS to supported backendsListing
gcsalongsidelocalands3is clear and consistent with the new feature.
395-397: Docs vs implementation: is impersonation supported or not? Pick one message and be consistentThis note conflicts with the PR summary (“impersonation support”). Please verify the code path and update the docs accordingly. Two options below.
Option A — impersonation is supported (recommended):
-:::note Current Limitations -The `impersonate_service_account` parameter is parsed but not yet implemented. This feature is planned for a future release. -::: +:::tip +Service Account impersonation is supported via `impersonate_service_account`. When set, Atmos acquires short‑lived +credentials for the target service account before accessing the state object. +:::Option B — impersonation not yet implemented:
-:::note Current Limitations -The `impersonate_service_account` parameter is parsed but not yet implemented. This feature is planned for a future release. -::: +:::caution Current Limitations +`impersonate_service_account` is currently parsed but ignored. Until implemented, use ADC or direct SA JSON credentials. +:::
| ### GCS Backend Authentication | ||
|
|
||
| The GCS backend supports multiple authentication methods: | ||
|
|
||
| 1. **Service Account Key File**: Specify the path to a service account JSON key file using the `credentials` parameter. | ||
| 2. **Default Credentials**: When no credentials are specified, the Google Cloud SDK default credentials are used. | ||
| 3. **Workload Identity** (GKE): Automatically uses the workload identity when running in GKE with Workload Identity enabled. | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Use “Application Default Credentials (ADC)” and document all supported auth modes
Terminology and coverage: prefer “ADC” and include inline JSON + impersonation if implemented.
-### GCS Backend Authentication
-
-The GCS backend supports multiple authentication methods:
-
-1. **Service Account Key File**: Specify the path to a service account JSON key file using the `credentials` parameter.
-2. **Default Credentials**: When no credentials are specified, the Google Cloud SDK default credentials are used.
-3. **Workload Identity** (GKE): Automatically uses the workload identity when running in GKE with Workload Identity enabled.
+### GCS Backend Authentication
+
+The GCS backend supports multiple authentication methods:
+
+1. **Application Default Credentials (ADC)**: Used when no explicit credentials are provided. ADC can originate from the
+ Compute/GKE metadata server, `gcloud auth application-default login`, or Workload Identity.
+2. **Service Account JSON credentials**: Provide credentials via the `credentials` parameter either as a filesystem path
+ to a JSON key file or as an inline JSON string.
+3. **Service Account impersonation** (if enabled): Set `impersonate_service_account` to obtain short‑lived credentials for
+ the target service account. ADC or provided credentials are used as the source identity.If impersonation is not supported in code, drop bullet 3; otherwise keep and link to any repo-wide “Unified GCP auth” docs if available.
Update GCS Backend Authentication docs to use ADC terminology and accurate auth modes
Below is a proposed replacement for lines 380–387 in website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx. It
- Renames “Default Credentials” to Application Default Credentials (ADC).
- Clarifies that the
credentialsparameter only supports a file path (or env var pointing to a file), not inline JSON. - Adds the supported
impersonate_service_accountandaccess_tokenoptions.
### GCS Backend Authentication
-The GCS backend supports multiple authentication methods:
-
-1. **Service Account Key File**: Specify the path to a service account JSON key file using the `credentials` parameter.
-2. **Default Credentials**: When no credentials are specified, the Google Cloud SDK default credentials are used.
-3. **Workload Identity** (GKE): Automatically uses the workload identity when running in GKE with Workload Identity enabled.
+The GCS backend supports multiple authentication methods:
+
+1. **Application Default Credentials (ADC)**: Used when no explicit credentials are provided.
+ ADC can originate from the Compute/GKE metadata server, via `gcloud auth application-default login`, or through Workload Identity on GKE. ([developer.hashicorp.com](https://developer.hashicorp.com/terraform/language/settings/backends/gcs?utm_source=chatgpt.com), [cloud.google.com](https://cloud.google.com/docs/terraform/authentication?utm_source=chatgpt.com))
+2. **Service Account JSON key**: Supply a JSON key file path with the `credentials` parameter or set `GOOGLE_CREDENTIALS` to that filepath. ([developer.hashicorp.com](https://developer.hashicorp.com/terraform/language/settings/backends/gcs?utm_source=chatgpt.com))
+3. **Service Account impersonation**: Specify `impersonate_service_account` to acquire short-lived credentials for a target service account, using ADC or provided credentials as the source identity. ([developer.hashicorp.com](https://developer.hashicorp.com/terraform/language/settings/backends/gcs?utm_source=chatgpt.com))
+4. **OAuth2 access token**: Provide a temporary token via the `access_token` parameter as an alternative to a JSON key file. ([developer.hashicorp.com](https://developer.hashicorp.com/terraform/language/settings/backends/gcs?utm_source=chatgpt.com))
-If impersonation is not supported in code, drop bullet 3; otherwise keep and link to any repo-wide “Unified GCP auth” docs if available.
+If your YAML-function implementation does not support impersonation, remove bullet 3. Otherwise, you may link to any existing “Unified GCP auth” guidance.📝 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.
| ### GCS Backend Authentication | |
| The GCS backend supports multiple authentication methods: | |
| 1. **Service Account Key File**: Specify the path to a service account JSON key file using the `credentials` parameter. | |
| 2. **Default Credentials**: When no credentials are specified, the Google Cloud SDK default credentials are used. | |
| 3. **Workload Identity** (GKE): Automatically uses the workload identity when running in GKE with Workload Identity enabled. | |
| ### GCS Backend Authentication | |
| The GCS backend supports multiple authentication methods: | |
| 1. **Application Default Credentials (ADC)**: Used when no explicit credentials are provided. | |
| ADC can originate from the Compute/GKE metadata server, via `gcloud auth application-default login`, or through Workload Identity on GKE. ([developer.hashicorp.com](https://developer.hashicorp.com/terraform/language/settings/backends/gcs?utm_source=chatgpt.com), [cloud.google.com](https://cloud.google.com/docs/terraform/authentication?utm_source=chatgpt.com)) | |
| 2. **Service Account JSON key**: Supply a JSON key file path with the `credentials` parameter or set `GOOGLE_CREDENTIALS` to that filepath. ([developer.hashicorp.com](https://developer.hashicorp.com/terraform/language/settings/backends/gcs?utm_source=chatgpt.com)) | |
| 3. **Service Account impersonation**: Specify `impersonate_service_account` to acquire short-lived credentials for a target service account, using ADC or provided credentials as the source identity. ([developer.hashicorp.com](https://developer.hashicorp.com/terraform/language/settings/backends/gcs?utm_source=chatgpt.com)) | |
| 4. **OAuth2 access token**: Provide a temporary token via the `access_token` parameter as an alternative to a JSON key file. ([developer.hashicorp.com](https://developer.hashicorp.com/terraform/language/settings/backends/gcs?utm_source=chatgpt.com)) | |
| If your YAML-function implementation does not support impersonation, remove bullet 3. Otherwise, you may link to any existing “Unified GCP auth” guidance. |
🤖 Prompt for AI Agents
In website/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx around
lines 380–387, update the GCS Backend Authentication section: rename “Default
Credentials” to “Application Default Credentials (ADC)”, state that the
credentials parameter accepts only a path to a service account JSON file (or an
environment variable that points to such a file) and does not accept inline
JSON, and add brief bullet points describing the supported
impersonate_service_account and access_token options (how they are used for
service account impersonation and short-lived token authentication). Ensure
wording is concise and consistent with existing doc style.
|
💥 This pull request now has conflicts. Could you fix it @shirkevich? 🙏 |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/exec/stack_processor_utils.go (1)
1282-1304: Fix AzureRM backend default key logic (missing or empty key).The current condition only fires when the azurerm section is missing, skipping the case where it exists but lacks key. Also, use constants for backend names.
- if finalComponentBackendType == cfg.BackendTypeAzurerm { - if componentAzurerm, componentAzurermExists := componentBackendSection["azurerm"].(map[string]any); !componentAzurermExists { - if _, componentAzurermKeyExists := componentAzurerm["key"].(string); !componentAzurermKeyExists { + if finalComponentBackendType == cfg.BackendTypeAzurerm { + componentAzurerm, componentAzurermExists := componentBackendSection[cfg.BackendTypeAzurerm].(map[string]any) + // Add default key when the azurerm section is missing OR when it exists but lacks `key`. + if !componentAzurermExists || componentAzurerm["key"] == nil || componentAzurerm["key"] == "" { azureKeyPrefixComponent := component var keyName []string if baseComponentName != "" { azureKeyPrefixComponent = baseComponentName } - if globalAzurerm, globalAzurermExists := globalBackendSection["azurerm"].(map[string]any); globalAzurermExists { - if _, globalAzurermKeyExists := globalAzurerm["key"].(string); globalAzurermKeyExists { - keyName = append(keyName, globalAzurerm["key"].(string)) - } - } + if globalAzurerm, globalAzurermExists := globalBackendSection[cfg.BackendTypeAzurerm].(map[string]any); globalAzurermExists { + if v, ok := globalAzurerm["key"].(string); ok && v != "" { + keyName = append(keyName, v) + } + } componentKeyName := strings.Replace(azureKeyPrefixComponent, "/", "-", -1) keyName = append(keyName, fmt.Sprintf("%s.terraform.tfstate", componentKeyName)) finalComponentBackend["key"] = strings.Join(keyName, "/") - } - } + } }
🧹 Nitpick comments (4)
internal/exec/stack_processor_utils.go (2)
1253-1266: Prefer constant for S3 backend type.Use cfg.BackendTypeS3 instead of the string literal to align with the repo constants and recent CI guidance.
- if finalComponentBackendType == "s3" { + if finalComponentBackendType == cfg.BackendTypeS3 {
1267-1281: Fix typo and punctuation in comments; keep linter happy."GSC" → "GCS" and end sentences with periods (godot).
- // Google GSC backend - // Check if `backend` section has `prefix` for `gcs` backend type - // If it does not, use the component name instead - // https://developer.hashicorp.com/terraform/language/settings/backends/gcs - // https://developer.hashicorp.com/terraform/language/settings/backends/gcs#prefix + // Google GCS backend. + // Check if `backend` section has `prefix` for the GCS backend type. + // If it does not, use the component name instead. + // https://developer.hashicorp.com/terraform/language/settings/backends/gcs. + // https://developer.hashicorp.com/terraform/language/settings/backends/gcs#prefix.internal/terraform_backend/terraform_backend_gcs_test.go (2)
198-203: End comment with a period (godot).Keeps golangci-lint happy.
-// Simulate failure in io.ReadAll +// Simulate failure in io.ReadAll.
395-405: Test name does not match behavior.This path never simulates a transient failure; rename for clarity or make the client flaky.
- name: "transient error with eventual success", + name: "success (no error path)",If you prefer a real transient case, I can provide a flaky mock that fails once then succeeds.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
internal/exec/stack_processor_utils.go(2 hunks)internal/exec/terraform_generate_backend.go(2 hunks)internal/terraform_backend/terraform_backend_gcs.go(1 hunks)internal/terraform_backend/terraform_backend_gcs_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/exec/terraform_generate_backend.go
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags
Use interfaces for external dependencies to facilitate mocking
All code must pass golangci-lint checks
Follow Go's error handling idioms
Use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider using a custom error type for domain-specific errors
Follow standard Go coding style
Use gofmt and goimports to format code
Prefer short, descriptive variable names
Use snake_case for environment variables
Document all exported functions, types, and methods
Document complex logic with inline comments
Follow Go's documentation conventions
Use Viper for configuration management
Support configuration via files, environment variables, and flags
Follow the precedence order: flags > environment variables > config file > defaultsFormat Go code with
gofumpt -w .andgoimports -w .
**/*.go: All Go comments must be complete sentences ending with periods (enforced by golangci-lint godot).
Always wrap errors with a static error first using fmt.Errorf("%w: details", errUtils.ErrStaticError, ...); never return dynamic errors directly.
Bind environment variables with viper.BindEnv() and provide an ATMOS_ alternative for every env var.
Distinguish UI output (stderr) from structured logging; never use logging for UI prompts/errors; data/results go to stdout for piping.
Use structured logging without message interpolation and respect logging level hierarchy; logging must not affect execution.
Most text UI must go to stderr (e.g., utils.PrintfMessageToTUI or fmt.Fprintf(os.Stderr,...)); only data/results go to stdout.
For non-standard execution paths, capture telemetry with telemetry.CaptureCmd(cmd, err) or telemetry.CaptureCmdString("name", err); never capture user data.
Files:
internal/exec/stack_processor_utils.gointernal/terraform_backend/terraform_backend_gcs.gointernal/terraform_backend/terraform_backend_gcs_test.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for testing multiple scenarios
Include integration tests for command flows
Test CLI end-to-end when possible
Use test fixtures for complex inputs
Consider using testify/mock for creating mock implementations
**/*_test.go: Use t.Skipf() with a clear reason when skipping tests; never use t.Skip() or t.Skipf() without a reason.
In TestMain, set a package-level skipReason for CLI-dependent tests and call os.Exit(m.Run()); do not use log.Fatal() for missing/stale binaries.
Mirror test files to implementation names (e.g., aws_ssm_store_test.go for aws_ssm_store.go) and co-locate tests with code.
New features must include comprehensive unit tests, and CLI features require integration tests using tests/ fixtures.
Files:
internal/terraform_backend/terraform_backend_gcs_test.go
🧠 Learnings (4)
📚 Learning: 2024-12-05T02:48:53.818Z
Learnt from: aknysh
PR: cloudposse/atmos#809
File: cmd/cmd_utils.go:470-477
Timestamp: 2024-12-05T02:48:53.818Z
Learning: The function `GetLatestGitHubRepoRelease` in the Go codebase already uses a context with a timeout, so wrapping it with an additional context is unnecessary.
Applied to files:
internal/terraform_backend/terraform_backend_gcs.go
📚 Learning: 2025-09-05T14:40:05.107Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T14:40:05.107Z
Learning: Applies to **/*.go : All Go comments must be complete sentences ending with periods (enforced by golangci-lint godot).
Applied to files:
internal/terraform_backend/terraform_backend_gcs.gointernal/terraform_backend/terraform_backend_gcs_test.go
📚 Learning: 2025-09-05T14:40:05.107Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T14:40:05.107Z
Learning: Applies to **/*_test.go : New features must include comprehensive unit tests, and CLI features require integration tests using tests/ fixtures.
Applied to files:
internal/terraform_backend/terraform_backend_gcs_test.go
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests
Applied to files:
internal/terraform_backend/terraform_backend_gcs_test.go
🧬 Code graph analysis (3)
internal/exec/stack_processor_utils.go (1)
pkg/config/const.go (2)
BackendTypeGCS(86-86)BackendTypeAzurerm(85-85)
internal/terraform_backend/terraform_backend_gcs.go (3)
internal/gcp/auth.go (3)
GetCredentialsFromBackend(52-57)GetClientOptions(32-48)AuthOptions(10-19)internal/terraform_backend/terraform_backend_utils.go (3)
GetComponentBackend(31-36)GetBackendAttribute(47-52)GetTerraformWorkspace(15-20)errors/errors.go (6)
ErrWrappingFormat(8-8)ErrCreateGCSClient(54-54)ErrStringWrappingFormat(9-9)ErrInvalidBackendConfig(57-57)ErrGetObjectFromGCS(55-55)ErrReadGCSObjectBody(56-56)
internal/terraform_backend/terraform_backend_gcs_test.go (2)
errors/errors.go (1)
ErrInvalidBackendConfig(57-57)internal/terraform_backend/terraform_backend_gcs.go (5)
ReadTerraformBackendGCSInternal(181-241)GCSBucketHandle(49-51)GCSObjectHandle(54-56)GetGCSBackendCredentials(30-32)GetGCSBackendImpersonateServiceAccount(36-41)
⏰ 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: Summary
🔇 Additional comments (1)
internal/terraform_backend/terraform_backend_gcs.go (1)
163-170: Docs vs code: impersonation is logged as TODO.PR summary says impersonation is supported; code logs it as “not yet implemented.” Please align implementation and docs or scope it for a follow-up.
| import ( | ||
| "context" | ||
| "crypto/sha256" | ||
| "encoding/hex" | ||
| "fmt" | ||
| "io" | ||
| "path" | ||
| "strings" | ||
| "sync" | ||
| "time" | ||
|
|
||
| "cloud.google.com/go/storage" | ||
| log "github.com/charmbracelet/log" | ||
| "google.golang.org/grpc/codes" | ||
| "google.golang.org/grpc/status" | ||
|
|
||
| errUtils "github.com/cloudposse/atmos/errors" | ||
| "github.com/cloudposse/atmos/internal/gcp" | ||
| "github.com/cloudposse/atmos/pkg/schema" | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add missing imports for errors and backend constants.
Needed for improved NotFound detection and using the backend type constant.
import (
"context"
+ "errors"
"crypto/sha256"
"encoding/hex"
"fmt"
"io"
"path"
"strings"
"sync"
"time"
"cloud.google.com/go/storage"
log "github.com/charmbracelet/log"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
errUtils "github.com/cloudposse/atmos/errors"
+ cfg "github.com/cloudposse/atmos/pkg/config"
"github.com/cloudposse/atmos/internal/gcp"
"github.com/cloudposse/atmos/pkg/schema"
)📝 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.
| import ( | |
| "context" | |
| "crypto/sha256" | |
| "encoding/hex" | |
| "fmt" | |
| "io" | |
| "path" | |
| "strings" | |
| "sync" | |
| "time" | |
| "cloud.google.com/go/storage" | |
| log "github.com/charmbracelet/log" | |
| "google.golang.org/grpc/codes" | |
| "google.golang.org/grpc/status" | |
| errUtils "github.com/cloudposse/atmos/errors" | |
| "github.com/cloudposse/atmos/internal/gcp" | |
| "github.com/cloudposse/atmos/pkg/schema" | |
| ) | |
| import ( | |
| "context" | |
| "errors" | |
| "crypto/sha256" | |
| "encoding/hex" | |
| "fmt" | |
| "io" | |
| "path" | |
| "strings" | |
| "sync" | |
| "time" | |
| "cloud.google.com/go/storage" | |
| log "github.com/charmbracelet/log" | |
| "google.golang.org/grpc/codes" | |
| "google.golang.org/grpc/status" | |
| errUtils "github.com/cloudposse/atmos/errors" | |
| cfg "github.com/cloudposse/atmos/pkg/config" | |
| "github.com/cloudposse/atmos/internal/gcp" | |
| "github.com/cloudposse/atmos/pkg/schema" | |
| ) |
🤖 Prompt for AI Agents
In internal/terraform_backend/terraform_backend_gcs.go around lines 3 to 22, add
the missing imports for the standard "errors" package and the package that
exposes the Terraform backend type constant (e.g. the package where your backend
type const is defined, such as github.com/cloudposse/atmos/pkg/terraform); then
update the code to use errors.Is(err, ...) for NotFound detection and reference
the backend type constant from that package rather than a hardcoded string.
| // Extract the GCS-specific configuration section (same pattern as stack processor). | ||
| gcsBackend := map[string]any{} | ||
| if gcsSection, ok := backend["gcs"].(map[string]any); ok { | ||
| gcsBackend = gcsSection | ||
| } else { | ||
| // If no nested gcs section, assume the backend config is already flattened (e.g., from stack processor). | ||
| gcsBackend = backend | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid hardcoded "gcs" key; use the existing constant.
Aligns with the request to avoid string literals for backend names.
- gcsBackend := map[string]any{}
- if gcsSection, ok := backend["gcs"].(map[string]any); ok {
+ gcsBackend := map[string]any{}
+ if gcsSection, ok := backend[cfg.BackendTypeGCS].(map[string]any); ok {
gcsBackend = gcsSection
} else {
// If no nested gcs section, assume the backend config is already flattened (e.g., from stack processor).
gcsBackend = backend
}📝 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.
| // Extract the GCS-specific configuration section (same pattern as stack processor). | |
| gcsBackend := map[string]any{} | |
| if gcsSection, ok := backend["gcs"].(map[string]any); ok { | |
| gcsBackend = gcsSection | |
| } else { | |
| // If no nested gcs section, assume the backend config is already flattened (e.g., from stack processor). | |
| gcsBackend = backend | |
| } | |
| // Extract the GCS-specific configuration section (same pattern as stack processor). | |
| gcsBackend := map[string]any{} | |
| if gcsSection, ok := backend[cfg.BackendTypeGCS].(map[string]any); ok { | |
| gcsBackend = gcsSection | |
| } else { | |
| // If no nested gcs section, assume the backend config is already flattened (e.g., from stack processor). | |
| gcsBackend = backend | |
| } |
🤖 Prompt for AI Agents
internal/terraform_backend/terraform_backend_gcs.go lines 125-133: the code
currently checks for the literal "gcs" key; replace that hardcoded string with
the package constant used for the GCS backend (i.e., use the existing GCS
backend constant instead of "gcs") so both the type assertion and the fallback
logic reference the constant; update both occurrences in this block to use the
constant.
| // Build the path to the tfstate file in the GCS bucket. | ||
| // According to Terraform docs: "Named states for workspaces are stored in an object called `<prefix>/<workspace>.tfstate`". | ||
| prefix := GetBackendAttribute(backend, "prefix") | ||
| workspace := GetTerraformWorkspace(componentSections) | ||
|
|
||
| var tfStateFilePath string | ||
| if prefix == "" { | ||
| // If no prefix is set, store at root level: <workspace>.tfstate. | ||
| tfStateFilePath = workspace + ".tfstate" | ||
| } else { | ||
| // If prefix is set: <prefix>/<workspace>.tfstate. | ||
| tfStateFilePath = path.Join(prefix, workspace+".tfstate") | ||
| } | ||
|
|
There was a problem hiding this comment.
Default empty workspace to "default".
Terraform uses "default" workspace; building ".tfstate" is incorrect and will miss the object.
- workspace := GetTerraformWorkspace(componentSections)
+ workspace := GetTerraformWorkspace(componentSections)
+ if workspace == "" {
+ workspace = "default"
+ }📝 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.
| // Build the path to the tfstate file in the GCS bucket. | |
| // According to Terraform docs: "Named states for workspaces are stored in an object called `<prefix>/<workspace>.tfstate`". | |
| prefix := GetBackendAttribute(backend, "prefix") | |
| workspace := GetTerraformWorkspace(componentSections) | |
| var tfStateFilePath string | |
| if prefix == "" { | |
| // If no prefix is set, store at root level: <workspace>.tfstate. | |
| tfStateFilePath = workspace + ".tfstate" | |
| } else { | |
| // If prefix is set: <prefix>/<workspace>.tfstate. | |
| tfStateFilePath = path.Join(prefix, workspace+".tfstate") | |
| } | |
| // Build the path to the tfstate file in the GCS bucket. | |
| // According to Terraform docs: "Named states for workspaces are stored in an object called `<prefix>/<workspace>.tfstate`". | |
| prefix := GetBackendAttribute(backend, "prefix") | |
| workspace := GetTerraformWorkspace(componentSections) | |
| if workspace == "" { | |
| workspace = "default" | |
| } | |
| var tfStateFilePath string | |
| if prefix == "" { | |
| // If no prefix is set, store at root level: <workspace>.tfstate. | |
| tfStateFilePath = workspace + ".tfstate" | |
| } else { | |
| // If prefix is set: <prefix>/<workspace>.tfstate. | |
| tfStateFilePath = path.Join(prefix, workspace+".tfstate") | |
| } |
🤖 Prompt for AI Agents
In internal/terraform_backend/terraform_backend_gcs.go around lines 186 to 199,
the code builds the tfstate path using the workspace value but doesn't default
an empty workspace to "default", which yields ".tfstate" and misses Terraform's
actual state object; update the code to set workspace = "default" if
GetTerraformWorkspace returns an empty string before composing tfStateFilePath,
then construct tfStateFilePath exactly as before (workspace+".tfstate" for root,
or path.Join(prefix, workspace+".tfstate") when prefix is set).
| // Get the object from GCS. | ||
| objectHandle := gcsClient.Bucket(bucket).Object(tfStateFilePath) | ||
| reader, err := objectHandle.NewReader(ctx) | ||
| if err != nil { | ||
| cancel() // Cancel immediately after use | ||
| // Check if the error is because the object doesn't exist. | ||
| // If the state file does not exist (the component in the stack has not been provisioned yet), return a `nil` result and no error. | ||
| if status.Code(err) == codes.NotFound { | ||
| log.Debug("Terraform state file doesn't exist in the GCS bucket; returning 'null'", "file", tfStateFilePath, "bucket", bucket) | ||
| return nil, nil | ||
| } | ||
|
|
||
| lastErr = err | ||
| if attempt < maxGCSRetryCount { | ||
| log.Debug("Failed to read Terraform state file from GCS bucket", "attempt", attempt+1, "file", tfStateFilePath, "bucket", bucket, "error", err) | ||
| time.Sleep(time.Second * 2) // backoff | ||
| continue | ||
| } | ||
| return nil, fmt.Errorf(errUtils.ErrWrappingFormat, errUtils.ErrGetObjectFromGCS, lastErr) | ||
| } |
There was a problem hiding this comment.
Treat NotFound for both gRPC and HTTP clients.
storage.NewReader may return storage.ErrObjectNotExist (HTTP) or a gRPC NotFound. Handle both.
- if err != nil {
- cancel() // Cancel immediately after use
- // Check if the error is because the object doesn't exist.
- // If the state file does not exist (the component in the stack has not been provisioned yet), return a `nil` result and no error.
- if status.Code(err) == codes.NotFound {
+ if err != nil {
+ cancel() // Cancel immediately after use.
+ // If the state file does not exist (the component in the stack has not been provisioned yet), return a `nil` result and no error.
+ if errors.Is(err, storage.ErrObjectNotExist) || status.Code(err) == codes.NotFound {
log.Debug("Terraform state file doesn't exist in the GCS bucket; returning 'null'", "file", tfStateFilePath, "bucket", bucket)
return nil, nil
}📝 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.
| // Get the object from GCS. | |
| objectHandle := gcsClient.Bucket(bucket).Object(tfStateFilePath) | |
| reader, err := objectHandle.NewReader(ctx) | |
| if err != nil { | |
| cancel() // Cancel immediately after use | |
| // Check if the error is because the object doesn't exist. | |
| // If the state file does not exist (the component in the stack has not been provisioned yet), return a `nil` result and no error. | |
| if status.Code(err) == codes.NotFound { | |
| log.Debug("Terraform state file doesn't exist in the GCS bucket; returning 'null'", "file", tfStateFilePath, "bucket", bucket) | |
| return nil, nil | |
| } | |
| lastErr = err | |
| if attempt < maxGCSRetryCount { | |
| log.Debug("Failed to read Terraform state file from GCS bucket", "attempt", attempt+1, "file", tfStateFilePath, "bucket", bucket, "error", err) | |
| time.Sleep(time.Second * 2) // backoff | |
| continue | |
| } | |
| return nil, fmt.Errorf(errUtils.ErrWrappingFormat, errUtils.ErrGetObjectFromGCS, lastErr) | |
| } | |
| if err != nil { | |
| cancel() // Cancel immediately after use. | |
| // If the state file does not exist (the component in the stack has not been provisioned yet), return a `nil` result and no error. | |
| if errors.Is(err, storage.ErrObjectNotExist) || status.Code(err) == codes.NotFound { | |
| log.Debug("Terraform state file doesn't exist in the GCS bucket; returning 'null'", "file", tfStateFilePath, "bucket", bucket) | |
| return nil, nil | |
| } | |
| lastErr = err | |
| if attempt < maxGCSRetryCount { | |
| log.Debug("Failed to read Terraform state file from GCS bucket", "attempt", attempt+1, "file", tfStateFilePath, "bucket", bucket, "error", err) | |
| time.Sleep(time.Second * 2) // backoff | |
| continue | |
| } | |
| return nil, fmt.Errorf(errUtils.ErrWrappingFormat, errUtils.ErrGetObjectFromGCS, lastErr) | |
| } |
🤖 Prompt for AI Agents
internal/terraform_backend/terraform_backend_gcs.go around lines 210 to 229: the
reader.NewReader error handling only checks gRPC NotFound; update the branch to
treat both storage.ErrObjectNotExist (HTTP client) and gRPC codes.NotFound as
the same "missing state" case, i.e., if errors.Is(err,
storage.ErrObjectNotExist) || status.Code(err) == codes.NotFound then log and
return (nil, nil); ensure cancel() is still invoked appropriately (call cancel()
before returning) and keep the existing retry/lastErr logic for other errors.
aknysh
added
no-release
|
@shirkevich can you please resolve the conflicts, thanks |
|
I like Atmos, but I really need GCS support to be able to use it, otherwise I'll have to go with Terragrunt, so any information when this is released? |
Add GCS backend support to
!terraform.stateYAML functionwhat
!terraform.stateAtmos YAML functionwhy
The
!terraform.stateYAML function allows reading the outputs (remote state) of components in Atmos stack manifests directly from the configured Terraform/OpenTofu backends.Previously, the
!terraform.stateYAML function only supported:local(Terraform and OpenTofu)s3(Terraform and OpenTofu)This PR adds support for:
gcs(Google Cloud Storage - Terraform and OpenTofu)With GCS backend support, users can now leverage the high-performance
!terraform.statefunction instead of the slower!terraform.outputor!storefunctions when using Google Cloud Storage for Terraform state storage.Implementation Details
GCS Backend Features
impersonate_service_accountconfigurationUsage
The GCS backend works seamlessly with existing
!terraform.statesyntax:GCS Backend Configuration
The GCS backend supports all standard Terraform GCS backend configurations:
Performance Benefits
Compared to
!terraform.output, the!terraform.statefunction with GCS backend:Testing
Backward Compatibility
local,s3) remain unchanged!terraform.outputand!storefunctionsFiles Changed
Core Implementation
internal/terraform_backend/terraform_backend_gcs.go- GCS backend implementationinternal/terraform_backend/terraform_backend_gcs_test.go- Comprehensive test suiteinternal/terraform_backend/terraform_backend_registry.go- Register GCS backendinternal/terraform_backend/terraform_backend_utils.go- Updated error messagesUnified Authentication System
internal/gcp/auth.go- New unified Google Cloud authentication (created)internal/gcp/auth_test.go- Authentication tests (created)pkg/store/google_secret_manager_store.go- Updated to use unified authinternal/gcp_utils/gcp_utils.go- Removed (replaced by unified auth)Configuration & Documentation
internal/exec/terraform_generate_backend.go- GCS backend validationwebsite/docs/core-concepts/stacks/yaml-functions/terraform.state.mdx- Updated documentationerrors/errors.go- Added GCS-specific error typesgo.mod- Added GCS storage dependencyMigration Guide
For users currently using
!terraform.outputor!storewith GCS-stored state:Before (slower)
After (faster)
Simply update your backend configuration to use
gcsand replace function calls - no other changes needed!Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores