Add Azure Blob Storage (azurerm) backend support for !terraform.state function#1610
Add Azure Blob Storage (azurerm) backend support for !terraform.state function#1610
Conversation
📝 WalkthroughWalkthroughAdds Azure Blob (azurerm) Terraform backend read support: new sentinel errors, a pluggable cached Azure Blob client with retries/timeouts and path logic, public read functions, comprehensive unit/integration tests, registry registration, docs/blog updates, go.mod dependency change, and an Azure test precondition helper. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller as Caller
participant Reader as ReadTerraformBackendAzurerm
participant Cache as azureBlobClientCache
participant Client as AzureBlobClient
participant Blob as Azure Blob Storage
Caller->>Reader: ReadTerraformBackendAzurerm(ctx, cfg, workspace)
Reader->>Cache: getCachedAzureBlobClient(storageAccount)
Cache-->>Reader: azureBlobClientWrapper
Reader->>Client: DownloadStream(ctx, container, blobPath)
alt 200 OK
Client-->>Reader: AzureBlobDownloadResponse
Reader->>Reader: read body (30s per attempt)\nretry up to 2 times (backoff 1s,2s)
Reader-->>Caller: []byte (state), nil
else 404 Not Found
Client-->>Reader: 404
Reader-->>Caller: nil, nil
else 403 Forbidden
Client-->>Reader: 403
Reader-->>Caller: ErrAzurePermissionDenied
else Other/Error
Client-->>Reader: error
Reader-->>Caller: ErrGetBlobFromAzure / ErrReadAzureBlobBody
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
errors/errors.go(1 hunks)go.mod(1 hunks)internal/terraform_backend/terraform_backend_azurerm.go(1 hunks)internal/terraform_backend/terraform_backend_azurerm_test.go(1 hunks)internal/terraform_backend/terraform_backend_registry.go(1 hunks)internal/terraform_backend/terraform_backend_utils.go(1 hunks)website/docs/functions/yaml/terraform.state.mdx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: All comments must end with periods; comments should be complete sentences (golangci-lint godot).
Organize imports into three groups separated by blank lines: standard library, third-party, and github.com/cloudposse/atmos packages; sort alphabetically within each group and preserve existing aliases.
Wrap all errors using static errors from errors/errors.go; prefer errors.Join for multiple errors; use fmt.Errorf with %w for context; check with errors.Is; never use dynamic errors directly.
Distinguish structured logging from UI output: use stderr for prompts/status/errors needing user action, stdout only for data; never use logging for UI elements.
Most text UI must go to stderr; only data/results go to stdout; prefer utils.PrintfMessageToTUI for UI messages.
Ensure cross-platform compatibility: prefer SDKs over external binaries; use filepath.Join, os.PathSeparator, and runtime.GOOS for OS-specific logic; add build constraints if needed.
Files:
errors/errors.gointernal/terraform_backend/terraform_backend_registry.gointernal/terraform_backend/terraform_backend_utils.gointernal/terraform_backend/terraform_backend_azurerm_test.gointernal/terraform_backend/terraform_backend_azurerm.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
errors/errors.gointernal/terraform_backend/terraform_backend_registry.gointernal/terraform_backend/terraform_backend_utils.gointernal/terraform_backend/terraform_backend_azurerm.go
errors/errors.go
📄 CodeRabbit inference engine (CLAUDE.md)
Define static errors in errors/errors.go as package-level variables using errors.New().
Files:
errors/errors.go
@(cmd|internal|pkg)/**/!(*_test).go
📄 CodeRabbit inference engine (CLAUDE.md)
@(cmd|internal|pkg)/**/!(*_test).go: Adddefer perf.Track()to all public and critical private functions with a blank line after the call; use package-qualified names like "packagename.FunctionName"; pass atmosConfig if present, else nil.
Ensure 80% minimum coverage on new/changed lines; exclude mock files from coverage (mock_.go, **/mock_.go, **/mock/*.go); include comprehensive unit and required integration tests.
Files:
internal/terraform_backend/terraform_backend_registry.gointernal/terraform_backend/terraform_backend_utils.gointernal/terraform_backend/terraform_backend_azurerm.go
@(cmd|internal|pkg)/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Bind environment variables with viper.BindEnv() and provide ATMOS_ alternatives for every env var.
Files:
internal/terraform_backend/terraform_backend_registry.gointernal/terraform_backend/terraform_backend_utils.gointernal/terraform_backend/terraform_backend_azurerm_test.gointernal/terraform_backend/terraform_backend_azurerm.go
go.{mod,sum}
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
go.{mod,sum}: Manage dependencies with Go modules
Keep dependencies up to date
Files:
go.mod
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in website/ when adding features
Ensure consistency between CLI help text and website documentation
Follow the website's documentation structure and style
Keep website code in website/ and follow its architecture/style; test changes locally
Keep CLI and website documentation in sync; document new features with examples and use cases
Files:
website/docs/functions/yaml/terraform.state.mdx
website/docs/**/*.mdx
📄 CodeRabbit inference engine (CLAUDE.md)
Follow Docusaurus conventions for frontmatter, purpose note, screengrab components, and section ordering (Usage → Examples → Arguments → Flags).
Files:
website/docs/functions/yaml/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 multiple scenarios
**/*_test.go: Use table-driven tests for unit tests; focus on pure functions; generate and use mocks for external dependencies.
Test behavior, not implementation; avoid tautological or stub tests; remove always-skipped tests; ensure tests validate real behavior.
Always use t.Skipf() with a reason for skipped tests; never call t.Skipf without a reason; CLI tests build a temp binary and TestMain must call os.Exit(m.Run()).
Files:
internal/terraform_backend/terraform_backend_azurerm_test.go
🧬 Code graph analysis (4)
internal/terraform_backend/terraform_backend_registry.go (2)
pkg/config/const.go (1)
BackendTypeAzurerm(89-89)internal/terraform_backend/terraform_backend_azurerm.go (1)
ReadTerraformBackendAzurerm(121-135)
internal/terraform_backend/terraform_backend_utils.go (1)
errors/errors.go (1)
ErrUnsupportedBackendType(57-57)
internal/terraform_backend/terraform_backend_azurerm_test.go (2)
internal/terraform_backend/terraform_backend_azurerm.go (2)
AzureBlobDownloadResponse(42-44)ReadTerraformBackendAzurermInternal(138-229)errors/errors.go (4)
ErrAzurePermissionDenied(71-71)ErrGetBlobFromAzure(65-65)ErrAzureContainerRequired(69-69)ErrReadAzureBlobBody(66-66)
internal/terraform_backend/terraform_backend_azurerm.go (3)
pkg/perf/perf.go (1)
Track(58-86)internal/terraform_backend/terraform_backend_utils.go (3)
GetBackendAttribute(56-63)GetComponentBackend(36-43)GetTerraformWorkspace(16-23)errors/errors.go (7)
ErrStorageAccountRequired(70-70)ErrCreateAzureCredential(67-67)ErrCreateAzureClient(68-68)ErrAzureContainerRequired(69-69)ErrAzurePermissionDenied(71-71)ErrGetBlobFromAzure(65-65)ErrReadAzureBlobBody(66-66)
⏰ 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). (8)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Build (macos-latest, macos)
- GitHub Check: Build (ubuntu-latest, linux)
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: Run pre-commit hooks
- GitHub Check: website-deploy-preview
- GitHub Check: Summary
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1610 +/- ##
==========================================
+ Coverage 67.61% 67.68% +0.06%
==========================================
Files 369 370 +1
Lines 43109 43289 +180
==========================================
+ Hits 29148 29300 +152
- Misses 11820 11845 +25
- Partials 2141 2144 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/exec/terraform_state_utils.go (1)
3-15: Reorganize imports per coding guidelines.Imports must be grouped: stdlib, third-party, Atmos, with blank-line separators and alphabetical sorting within each group.
Apply this diff:
import ( "fmt" "sync" + log "github.com/cloudposse/atmos/pkg/logger" - "github.com/cloudposse/atmos/pkg/perf" - errUtils "github.com/cloudposse/atmos/errors" tb "github.com/cloudposse/atmos/internal/terraform_backend" cfg "github.com/cloudposse/atmos/pkg/config" + log "github.com/cloudposse/atmos/pkg/logger" + "github.com/cloudposse/atmos/pkg/perf" "github.com/cloudposse/atmos/pkg/schema" u "github.com/cloudposse/atmos/pkg/utils" )As per coding guidelines.
🧹 Nitpick comments (1)
internal/exec/terraform_state_utils.go (1)
75-127: Consider conditionally prefixing TUI messages with\r.The
\rprefix assumes a spinner line exists to overwrite, but the spinner only runs in trace/debug modes (line 63). In production (info/warn/error levels), the\ris unnecessary and slightly awkward. You could track whether the spinner is active and conditionally prepend\r, or accept the current behavior as harmless.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
internal/exec/terraform_state_utils.go(5 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: All comments must end with periods; enforced by golangci-lint godot across all Go comments.
Organize imports into three groups (stdlib, third-party, Atmos) separated by blank lines and sorted alphabetically within each group; keep existing aliases.
All errors must be wrapped using static errors defined in errors/errors.go; prefer errors.Join for multiple, fmt.Errorf with %w for context, and errors.Is for checks; never rely on string comparisons.
Prefer cross-platform implementations: use SDKs over external binaries; use filepath/os facilities; gate OS-specific logic with runtime.GOOS or build tags.
Files:
internal/exec/terraform_state_utils.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
internal/exec/terraform_state_utils.go
{cmd,internal,pkg}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
{cmd,internal,pkg}/**/*.go: Adddefer perf.Track()to all public functions and critical private ones, include a blank line after it, and use package-qualified names (e.g., "exec.ProcessComponent"). Use atmosConfig if available, else nil.
Always bind environment variables with viper.BindEnv; every var must have an ATMOS_ alternative and prefer ATMOS_ over external names.
Distinguish structured logging from UI output: UI prompts/errors/status to stderr; data/results to stdout; logging for system/debug only; no UI via logging.
Most text UI must go to stderr (via utils.PrintfMessageToTUI or fmt.Fprintf(os.Stderr,...)); only data/results to stdout.
Files:
internal/exec/terraform_state_utils.go
{pkg,internal,cmd}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Always use mockgen for interface mocks; never write manual mocks with many stub methods.
Files:
internal/exec/terraform_state_utils.go
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Target minimum 80% coverage on new/changed lines; exclude mock files from coverage: **/mock_.go, mock_.go, **/mock/*.go.
Files:
internal/exec/terraform_state_utils.go
🧠 Learnings (1)
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
PR: cloudposse/atmos#1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
internal/exec/terraform_state_utils.go
🧬 Code graph analysis (1)
internal/exec/terraform_state_utils.go (6)
pkg/logger/log.go (3)
Debug(24-26)Level(171-171)Errorf(59-61)pkg/schema/schema.go (1)
Logs(376-379)internal/exec/spinner_utils.go (3)
NewSpinner(46-64)RunSpinner(67-76)StopSpinner(79-82)internal/exec/describe_component.go (1)
ExecuteDescribeComponent(198-219)pkg/utils/log_utils.go (1)
PrintfMessageToTUI(29-31)internal/terraform_backend/terraform_backend_utils.go (2)
GetTerraformBackend(121-150)GetTerraformBackendVariable(66-84)
⏰ 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). (7)
- GitHub Check: Build (linux)
- GitHub Check: Build (windows)
- GitHub Check: Build (macos)
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: website-deploy-preview
- GitHub Check: Summary
🔇 Additional comments (1)
internal/exec/terraform_state_utils.go (1)
46-51: Nice improvement to debug logging.Adding function, component, stack, and output context to the cache-hit log makes debugging and tracing much easier.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
internal/terraform_backend/terraform_backend_azurerm.go (1)
205-207: Fix context timeout leaks across retries.
defer cancel()inside the retry loop keeps timers alive for the full 30s per attempt. Cancel explicitly on each path.Apply:
- ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) downloadResponse, err := azureClient.DownloadStream(ctx, containerName, tfStateFilePath, nil) if err != nil { + cancel() handledErr := handleAzureDownloadError(err, tfStateFilePath, containerName) if handledErr == nil { // Blob not found - component not provisioned yet. return nil, nil } if errors.Is(handledErr, errUtils.ErrAzurePermissionDenied) { // Permission denied - return immediately. return nil, handledErr } lastErr = err if attempt < maxRetryCountAzure { log.Debug( "Failed to read Terraform state file from Azure Blob Storage", "attempt", attempt+1, "file", tfStateFilePath, "container", containerName, "error", err, ) time.Sleep(time.Second * 2) // backoff continue } return nil, fmt.Errorf(errWrapFormat, errUtils.ErrGetBlobFromAzure, lastErr) } body := downloadResponse.GetBody() content, err := io.ReadAll(body) _ = body.Close() + cancel() if err != nil { return nil, fmt.Errorf(errWrapFormat, errUtils.ErrReadAzureBlobBody, err) } return content, nilAlso applies to: 208-233, 235-241
🧹 Nitpick comments (7)
internal/terraform_backend/terraform_backend_azurerm.go (3)
232-245: Add blob/container context to final error.Surface which blob/container failed for faster triage.
- return nil, fmt.Errorf(errWrapFormat, errUtils.ErrGetBlobFromAzure, lastErr) + return nil, fmt.Errorf("%w: blob %q in container %q: %v", errUtils.ErrGetBlobFromAzure, tfStateFilePath, containerName, lastErr) @@ - return nil, fmt.Errorf(errWrapFormat, errUtils.ErrGetBlobFromAzure, lastErr) + return nil, fmt.Errorf("%w: blob %q in container %q: %v", errUtils.ErrGetBlobFromAzure, tfStateFilePath, containerName, lastErr)
83-89: Client cache key and options.
- Cache by storage account only to avoid redundant clients per container.
- Pass
azblob.ClientOptions(retry/UA) instead of defaults to centralize behavior.- // Build a deterministic cache key. - cacheKey := fmt.Sprintf("account=%s;container=%s", storageAccountName, containerName) + // Build a deterministic cache key (per account). + cacheKey := fmt.Sprintf("account=%s", storageAccountName) @@ - client, err := azblob.NewClient(serviceURL, cred, nil) + opts := &azblob.ClientOptions{ + // Let Azure SDK handle transient retries in addition to our lightweight outer retry. + // Consider tuning to small, bounded attempts. + Retry: azcore.RetryOptions{MaxRetries: 2}, + Telemetry: azcore.TelemetryOptions{ + ApplicationID: "atmos", + }, + } + client, err := azblob.NewClient(serviceURL, cred, opts)Also applies to: 110-117
100-105: Punctuation for lint (godot).List items should end with periods to satisfy godot.
- // 1. Environment variables (AZURE_TENANT_ID, AZURE_CLIENT_ID, AZURE_CLIENT_SECRET) - // 2. Managed Identity (when running in Azure) - // 3. Azure CLI credentials - // 4. Visual Studio Code credentials + // 1. Environment variables (AZURE_TENANT_ID, AZURE_CLIENT_ID, AZURE_CLIENT_SECRET). + // 2. Managed Identity (when running in Azure). + // 3. Azure CLI credentials. + // 4. Visual Studio Code credentials.internal/terraform_backend/terraform_backend_azurerm_test.go (4)
18-25: Prefer generated mocks (mockgen) for interfaces.Use mockgen and a generate directive to keep mocks consistent and lint-friendly.
Add at top of the file:
//go:generate mockgen -destination=mock_terraform_backend_azurerm_test.go -package=terraform_backend . AzureBlobAPIThen replace the hand-written mock with the generated one in follow-ups.
121-123: Fix comment punctuation for godot.End comments with periods to satisfy lint.
- // key not specified, should default to terraform.tfstate + // key not specified, should default to terraform.tfstate. @@ - // container_name is missing + // container_name is missing.Also applies to: 289-293
43-147: Enable t.Parallel in tests.Safe here and speeds up the suite.
func TestReadTerraformBackendAzurermInternal_Success(t *testing.T) { + t.Parallel() @@ func TestReadTerraformBackendAzurermInternal_BlobNotFound(t *testing.T) { + t.Parallel() @@ func TestReadTerraformBackendAzurermInternal_PermissionDenied(t *testing.T) { + t.Parallel() @@ func TestReadTerraformBackendAzurermInternal_NetworkError(t *testing.T) { + t.Parallel() @@ func TestReadTerraformBackendAzurermInternal_RetrySuccess(t *testing.T) { + t.Parallel() @@ func TestReadTerraformBackendAzurermInternal_MissingContainerName(t *testing.T) { + t.Parallel() @@ func TestReadTerraformBackendAzurermInternal_ReadBodyError(t *testing.T) { + t.Parallel()Also applies to: 150-196, 198-223, 224-249, 251-282, 284-307, 309-341
224-249: Speed up retry tests by injecting backoff.Current 2s sleeps make tests slow. Consider a package-level
sleepvar (defaults totime.Sleep) and use it in production; tests can override it to no-op.Also applies to: 251-282
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
internal/terraform_backend/terraform_backend_azurerm.go(1 hunks)internal/terraform_backend/terraform_backend_azurerm_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*_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 multiple scenarios
**/*_test.go: Unit tests should be table-driven where appropriate and focus on pure functions; target >80% coverage with emphasis on pkg/ and internal/exec/.
Test behavior, not implementation; avoid tautological or stub-only tests; use dependency injection to make code testable; remove always-skipped tests; table-driven tests must use realistic scenarios.
Place//go:generate mockgendirectives for mocks at the top of test files; for internal interfaces use-source=$GOFILE -destination=mock_$GOFILE -package=$GOPACKAGE.
Tests must call production code paths (do not duplicate production logic within tests).
Always use t.Skipf with a reason (never t.Skip or Skipf without context).
Test files should mirror implementation structure and be co-located with source files (foo.go ↔ foo_test.go).
Use precondition-based test skipping helpers from tests/test_preconditions.go (e.g., RequireAWSProfile, RequireGitHubAccess).
Files:
internal/terraform_backend/terraform_backend_azurerm_test.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: All comments must end with periods; enforced by golangci-lint godot across all Go comments.
Organize imports into three groups (stdlib, third-party, Atmos) separated by blank lines and sorted alphabetically within each group; keep existing aliases.
All errors must be wrapped using static errors defined in errors/errors.go; prefer errors.Join for multiple, fmt.Errorf with %w for context, and errors.Is for checks; never rely on string comparisons.
Prefer cross-platform implementations: use SDKs over external binaries; use filepath/os facilities; gate OS-specific logic with runtime.GOOS or build tags.
Files:
internal/terraform_backend/terraform_backend_azurerm_test.gointernal/terraform_backend/terraform_backend_azurerm.go
{cmd,internal,pkg}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
{cmd,internal,pkg}/**/*.go: Adddefer perf.Track()to all public functions and critical private ones, include a blank line after it, and use package-qualified names (e.g., "exec.ProcessComponent"). Use atmosConfig if available, else nil.
Always bind environment variables with viper.BindEnv; every var must have an ATMOS_ alternative and prefer ATMOS_ over external names.
Distinguish structured logging from UI output: UI prompts/errors/status to stderr; data/results to stdout; logging for system/debug only; no UI via logging.
Most text UI must go to stderr (via utils.PrintfMessageToTUI or fmt.Fprintf(os.Stderr,...)); only data/results to stdout.
Files:
internal/terraform_backend/terraform_backend_azurerm_test.gointernal/terraform_backend/terraform_backend_azurerm.go
{pkg,internal,cmd}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Always use mockgen for interface mocks; never write manual mocks with many stub methods.
Files:
internal/terraform_backend/terraform_backend_azurerm_test.gointernal/terraform_backend/terraform_backend_azurerm.go
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Target minimum 80% coverage on new/changed lines; exclude mock files from coverage: **/mock_.go, mock_.go, **/mock/*.go.
Files:
internal/terraform_backend/terraform_backend_azurerm_test.gointernal/terraform_backend/terraform_backend_azurerm.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
internal/terraform_backend/terraform_backend_azurerm.go
🧬 Code graph analysis (2)
internal/terraform_backend/terraform_backend_azurerm_test.go (3)
pkg/schema/schema.go (1)
Context(381-396)internal/terraform_backend/terraform_backend_azurerm.go (2)
AzureBlobDownloadResponse(43-45)ReadTerraformBackendAzurermInternal(187-245)errors/errors.go (4)
ErrAzurePermissionDenied(71-71)ErrGetBlobFromAzure(65-65)ErrAzureContainerRequired(69-69)ErrReadAzureBlobBody(66-66)
internal/terraform_backend/terraform_backend_azurerm.go (4)
pkg/perf/perf.go (1)
Track(58-86)internal/terraform_backend/terraform_backend_utils.go (3)
GetBackendAttribute(56-63)GetComponentBackend(36-43)GetTerraformWorkspace(16-23)errors/errors.go (7)
ErrStorageAccountRequired(70-70)ErrCreateAzureCredential(67-67)ErrCreateAzureClient(68-68)ErrAzurePermissionDenied(71-71)ErrAzureContainerRequired(69-69)ErrGetBlobFromAzure(65-65)ErrReadAzureBlobBody(66-66)pkg/logger/log.go (1)
Debug(24-26)
⏰ 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). (7)
- GitHub Check: Build (linux)
- GitHub Check: Build (macos)
- GitHub Check: Build (windows)
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: website-deploy-preview
- GitHub Check: Summary
🔇 Additional comments (1)
internal/terraform_backend/terraform_backend_azurerm.go (1)
139-157: Workspace suffix logic looks correct.Matches Terraform azurerm backend behavior
{key}env:{workspace}; tests validate it.
## what - Add nil check for backend map returned by GetComponentBackend - Add new error constant ErrBackendConfigRequired - Add test for missing backend configuration case ## why - Prevent potential nil pointer dereference if component has no backend configured - Provide clear error message when backend configuration is missing - Addresses CodeRabbit review feedback on PR #1610 ## references - #1610 (review) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
internal/terraform_backend/terraform_backend_azurerm_test.go (2)
1-16: Optional: generate mocks via mockgen.Small hand-written mocks are fine here, but consider adding a //go:generate directive for future growth.
//go:generate mockgen -destination=internal/terraform_backend/mocks/azureblob_mock.go -package terraform_backend github.com/cloudposse/atmos/internal/terraform_backend AzureBlobAPIAs per coding guidelines.
349-355: Add a test for ErrStorageAccountRequired path.Exercise getCachedAzureBlobClient error surfacing when storage_account_name is missing. I can draft it if helpful.
internal/terraform_backend/terraform_backend_azurerm.go (3)
232-233: Use exponential backoff to match PR intent.Sleep grows with attempts; simple and keeps tests stable.
- time.Sleep(time.Second * 2) // backoff + time.Sleep(time.Second * time.Duration(1<<attempt)) // exponential backoff.
110-117: Avoid stacked retries; set ClientOptions and telemetry.Currently Azure SDK default retries + our loop can multiply attempts. Prefer one layer:
- Configure azblob.Client with azcore.ClientOptions Retry (e.g., MaxRetries=0) and Telemetry ApplicationID "atmos".
- Or remove our loop and rely on SDK retries with exponential backoff.
Happy to propose concrete code if you choose a direction.
83-89: Cache clients per account, not container (optional).Client is scoped to the account endpoint; including container in the key reduces reuse.
Also applies to: 115-117
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
errors/errors.go(1 hunks)internal/terraform_backend/terraform_backend_azurerm.go(1 hunks)internal/terraform_backend/terraform_backend_azurerm_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- errors/errors.go
🧰 Additional context used
📓 Path-based instructions (6)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: All comments must end with periods. Applies to single-line, multi-line, inline, and documentation comments.
Organize imports into three groups (stdlib, 3rd-party, Atmos) separated by blank lines and sort alphabetically within each group; preserve existing aliases.
Add defer perf.Track(...) in all public and critical private functions; include a blank line after the perf.Track call; use "packagename.FunctionName" as the name; pass atmosConfig when available, otherwise nil.
Wrap all returned errors using static errors (from errors/errors.go); use errors.Join to combine errors; use fmt.Errorf with %w to add context; use errors.Is for checks; never rely on err.Error() string matching.
Always use viper.BindEnv with ATMOS_ alternatives; prefer ATMOS_* first with fallback to external env vars when needed.
Prefer many small, focused files; never use //revive:disable:file-length-limit to bypass file length limits.
Use filepath.Join, os.PathSeparator, and runtime.GOOS for portable path/OS handling; use build constraints when necessary.
Files:
internal/terraform_backend/terraform_backend_azurerm.gointernal/terraform_backend/terraform_backend_azurerm_test.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
internal/terraform_backend/terraform_backend_azurerm.go
{cmd,internal,pkg}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
{cmd,internal,pkg}/**/*.go: Use Viper for configuration management and bind environment variables consistently.
Distinguish structured logging from UI output: UI prompts/status/errors to stderr; data/results to stdout; do not use logging for UI elements.
Prefer SDKs over invoking external binaries for cross-platform compatibility (e.g., terraform-exec instead of exec.Command("terraform")).
Files:
internal/terraform_backend/terraform_backend_azurerm.gointernal/terraform_backend/terraform_backend_azurerm_test.go
{cmd,internal}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
{cmd,internal}/**/*.go: Most text UI must go to stderr (e.g., utils.PrintfMessageToTUI or fmt.Fprintf(os.Stderr, ...)); only data/results go to stdout.
Telemetry for commands is automatic via RootCmd.ExecuteC(); for non-standard paths, call telemetry.CaptureCmd/CaptureCmdString explicitly.
Files:
internal/terraform_backend/terraform_backend_azurerm.gointernal/terraform_backend/terraform_backend_azurerm_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 multiple scenarios
**/*_test.go: Unit tests should focus on pure functions and favor table-driven tests.
Test behavior (inputs/outputs), not implementation detail; avoid tautologies and stub-only tests; remove always-skipped tests.
Always call production code within tests rather than duplicating logic under test.
Use t.Skipf with a clear reason when skipping tests; never call t.Skipf without a reason.
Use go.uber.org/mock/mockgen for interface mocks; never hand-write large mock implementations.
Add //go:generate mockgen directives for mocks in test files.
Files:
internal/terraform_backend/terraform_backend_azurerm_test.go
{pkg,internal,cmd}/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Test files should mirror implementation naming (e.g., foo.go has foo_test.go) and be co-located with implementations.
Files:
internal/terraform_backend/terraform_backend_azurerm_test.go
🧬 Code graph analysis (2)
internal/terraform_backend/terraform_backend_azurerm.go (4)
pkg/perf/perf.go (1)
Track(121-138)internal/terraform_backend/terraform_backend_utils.go (3)
GetBackendAttribute(56-63)GetComponentBackend(36-43)GetTerraformWorkspace(16-23)errors/errors.go (8)
ErrStorageAccountRequired(78-78)ErrCreateAzureCredential(75-75)ErrCreateAzureClient(76-76)ErrBackendConfigRequired(80-80)ErrAzurePermissionDenied(79-79)ErrAzureContainerRequired(77-77)ErrGetBlobFromAzure(73-73)ErrReadAzureBlobBody(74-74)pkg/logger/log.go (2)
Errorf(59-61)Debug(24-26)
internal/terraform_backend/terraform_backend_azurerm_test.go (2)
internal/terraform_backend/terraform_backend_azurerm.go (3)
AzureBlobDownloadResponse(43-45)ReadTerraformBackendAzurermInternal(190-248)ReadTerraformBackendAzurerm(122-139)errors/errors.go (5)
ErrAzurePermissionDenied(79-79)ErrGetBlobFromAzure(73-73)ErrAzureContainerRequired(77-77)ErrReadAzureBlobBody(74-74)ErrBackendConfigRequired(80-80)
⏰ 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). (7)
- GitHub Check: Build (windows)
- GitHub Check: Build (macos)
- GitHub Check: Run pre-commit hooks
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: website-deploy-preview
- GitHub Check: Summary
🔇 Additional comments (2)
internal/terraform_backend/terraform_backend_azurerm_test.go (1)
43-147: Tests cover the right behaviors.Good coverage of default/non-default workspaces, not-found, permission denied, retries, and read failures. LGTM.
internal/terraform_backend/terraform_backend_azurerm.go (1)
162-187: Nice error mapping.404→nil content, 403→typed sentinel with context. Good use of errors.Is and wrapping.
|
Warning Changelog Entry RequiredThis PR is labeled Action needed: Add a new blog post in Example filename: Alternatively: If this change doesn't require a changelog entry, remove the |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/exec/terraform_state_utils.go (1)
52-57: Add TUI message on cache hit error path for consistency.All other error paths print a TUI message (e.g., line 75, 90, 105, 122) before returning, but this cache-hit error path doesn't. Consider adding
u.PrintfMessageToTUI("\r✗ %s\n", message)before line 54 for consistent user feedback.result, err := tb.GetTerraformBackendVariable(atmosConfig, backend.(map[string]any), output) if err != nil { + u.PrintfMessageToTUI("\r✗ Fetching %s output from %s in %s\n", output, component, stack) er := fmt.Errorf("%w %s for component `%s` in stack `%s`\nin YAML function: `%s`\n%v", errUtils.ErrEvaluateTerraformBackendVariable, output, component, stack, yamlFunc, err) return nil, er }
♻️ Duplicate comments (3)
internal/terraform_backend/terraform_backend_azurerm_test.go (3)
121-121: End comment with period.This was already flagged in a previous review.
Apply the fix:
- // key not specified, should default to terraform.tfstate + // key not specified, should default to terraform.tfstate.
291-291: End comment with period.This was already flagged in a previous review.
Apply the fix:
- // container_name is missing + // container_name is missing.
340-340: End comment with period.This was already flagged in a previous review.
Apply the fix:
- // No backend section + // No backend section.
🧹 Nitpick comments (1)
internal/exec/terraform_state_utils.go (1)
43-59: Consider TUI feedback on cache hit for consistency.The cache hit path is silent (only debug log at line 46), while cache miss paths show spinner and success/error messages. If you want consistent user feedback, you could add a success message after line 56, though the current silent behavior for cache hits is also reasonable since they're fast.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
internal/exec/terraform_state_utils.go(5 hunks)internal/terraform_backend/terraform_backend_azurerm_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: All comments must end with periods. Applies to single-line, multi-line, inline, and documentation comments.
Organize imports into three groups (stdlib, 3rd-party, Atmos) separated by blank lines and sort alphabetically within each group; preserve existing aliases.
Add defer perf.Track(...) in all public and critical private functions; include a blank line after the perf.Track call; use "packagename.FunctionName" as the name; pass atmosConfig when available, otherwise nil.
Wrap all returned errors using static errors (from errors/errors.go); use errors.Join to combine errors; use fmt.Errorf with %w to add context; use errors.Is for checks; never rely on err.Error() string matching.
Always use viper.BindEnv with ATMOS_ alternatives; prefer ATMOS_* first with fallback to external env vars when needed.
Prefer many small, focused files; never use //revive:disable:file-length-limit to bypass file length limits.
Use filepath.Join, os.PathSeparator, and runtime.GOOS for portable path/OS handling; use build constraints when necessary.
Files:
internal/exec/terraform_state_utils.gointernal/terraform_backend/terraform_backend_azurerm_test.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
internal/exec/terraform_state_utils.go
{cmd,internal,pkg}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
{cmd,internal,pkg}/**/*.go: Use Viper for configuration management and bind environment variables consistently.
Distinguish structured logging from UI output: UI prompts/status/errors to stderr; data/results to stdout; do not use logging for UI elements.
Prefer SDKs over invoking external binaries for cross-platform compatibility (e.g., terraform-exec instead of exec.Command("terraform")).
Files:
internal/exec/terraform_state_utils.gointernal/terraform_backend/terraform_backend_azurerm_test.go
{cmd,internal}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
{cmd,internal}/**/*.go: Most text UI must go to stderr (e.g., utils.PrintfMessageToTUI or fmt.Fprintf(os.Stderr, ...)); only data/results go to stdout.
Telemetry for commands is automatic via RootCmd.ExecuteC(); for non-standard paths, call telemetry.CaptureCmd/CaptureCmdString explicitly.
Files:
internal/exec/terraform_state_utils.gointernal/terraform_backend/terraform_backend_azurerm_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 multiple scenarios
**/*_test.go: Unit tests should focus on pure functions and favor table-driven tests.
Test behavior (inputs/outputs), not implementation detail; avoid tautologies and stub-only tests; remove always-skipped tests.
Always call production code within tests rather than duplicating logic under test.
Use t.Skipf with a clear reason when skipping tests; never call t.Skipf without a reason.
Use go.uber.org/mock/mockgen for interface mocks; never hand-write large mock implementations.
Add //go:generate mockgen directives for mocks in test files.
Files:
internal/terraform_backend/terraform_backend_azurerm_test.go
{pkg,internal,cmd}/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Test files should mirror implementation naming (e.g., foo.go has foo_test.go) and be co-located with implementations.
Files:
internal/terraform_backend/terraform_backend_azurerm_test.go
🧠 Learnings (5)
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
PR: cloudposse/atmos#1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
internal/exec/terraform_state_utils.go
📚 Learning: 2024-10-23T21:36:40.262Z
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/exec/terraform_state_utils.go
📚 Learning: 2024-12-02T21:26:32.337Z
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/exec/terraform_state_utils.go
📚 Learning: 2025-10-21T15:50:22.359Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-21T15:50:22.359Z
Learning: Applies to **/*.go : Add defer perf.Track(...) in all public and critical private functions; include a blank line after the perf.Track call; use "packagename.FunctionName" as the name; pass atmosConfig when available, otherwise nil.
Applied to files:
internal/exec/terraform_state_utils.go
📚 Learning: 2025-10-21T15:50:22.359Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-21T15:50:22.359Z
Learning: Applies to **/*.go : All comments must end with periods. Applies to single-line, multi-line, inline, and documentation comments.
Applied to files:
internal/terraform_backend/terraform_backend_azurerm_test.go
🧬 Code graph analysis (2)
internal/exec/terraform_state_utils.go (5)
pkg/schema/schema.go (1)
Logs(404-407)internal/exec/spinner_utils.go (3)
NewSpinner(46-64)RunSpinner(67-76)StopSpinner(79-82)internal/exec/describe_component.go (1)
ExecuteDescribeComponent(198-219)pkg/utils/log_utils.go (1)
PrintfMessageToTUI(33-35)internal/terraform_backend/terraform_backend_utils.go (2)
GetTerraformBackend(137-166)GetTerraformBackendVariable(66-84)
internal/terraform_backend/terraform_backend_azurerm_test.go (2)
internal/terraform_backend/terraform_backend_azurerm.go (3)
AzureBlobDownloadResponse(43-45)ReadTerraformBackendAzurermInternal(190-248)ReadTerraformBackendAzurerm(122-139)errors/errors.go (5)
ErrAzurePermissionDenied(79-79)ErrGetBlobFromAzure(73-73)ErrAzureContainerRequired(77-77)ErrReadAzureBlobBody(74-74)ErrBackendConfigRequired(80-80)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Build (windows)
- GitHub Check: Run pre-commit hooks
- GitHub Check: website-deploy-preview
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: Summary
🔇 Additional comments (14)
internal/exec/terraform_state_utils.go (3)
8-8: LGTM on import additions.The perf and utils imports are properly organized and follow the established patterns.
Also applies to: 14-14
61-71: Spinner logic looks solid.The nil guard on
atmosConfigand log-level checks are appropriate. The defer ensures cleanup even on error paths.
75-75: TUI messaging pattern is well-implemented.The success (✓) and error (✗) indicators provide clear feedback, and using
u.PrintfMessageToTUIcorrectly routes output to stderr. The carriage return (\r) properly overwrites spinner output in TTY environments.Also applies to: 90-90, 94-94, 98-98, 105-105, 115-115, 122-122, 127-127
internal/terraform_backend/terraform_backend_azurerm_test.go (11)
3-16: Imports properly organized.The three-group organization (stdlib, 3rd-party, Atmos) with blank line separators and alphabetical sorting within groups follows the coding guidelines correctly.
18-41: Clean mock implementation.The callback-based mock pattern provides flexibility for different test scenarios without requiring a heavy mocking framework. The helper function
createMockDownloadResponsereduces test boilerplate effectively.
43-148: Comprehensive workspace coverage.The table-driven test covers default, named, empty, and missing-key scenarios effectively. The blob naming assertions validate Terraform's Azure backend convention (
{key}env:{workspace}for non-default workspaces).
150-196: Correct not-found handling.Properly validates that missing blobs return
(nil, nil)rather than an error, aligning with the pattern for components that haven't been provisioned yet.
198-222: Permission denied properly surfaced.The test validates that 403 errors return immediately with
ErrAzurePermissionDeniedand include contextual information (blob path and container) in the error message.
224-249: Retry logic validated.The test confirms that transient errors trigger the expected number of retries (
maxRetryCountAzure + 1total attempts) before returningErrGetBlobFromAzure.
251-282: Retry success path covered.Validates that the implementation successfully recovers from transient failures, confirming that the retry logic doesn't just fail repeatedly but can succeed on subsequent attempts.
284-307: Early validation working correctly.The test confirms that missing
container_nameis caught before attempting any Azure API calls, preventing wasteful network operations.
309-334: Body read failure path tested.Properly validates error handling when the Azure download succeeds but reading the response body fails, ensuring distinct error messages for different failure modes.
336-348: Public API validation covered.Tests the public entry point
ReadTerraformBackendAzurermto ensure missing backend config is caught at the API boundary.
350-355: Simple and effective test helper.The
errorReaderprovides a clean way to simulate body read failures without external dependencies.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
website/blog/2025-10-21-azure-blob-terraform-state-support.md (1)
90-105: Clarify workspace naming convention with a leading sentence.The section explains Azure's workspace suffix convention, but readers may initially find the
env:separator notation unclear. Consider adding a brief intro sentence before the bullet points to set context.## Workspace Handling Azure Blob Storage uses a specific naming convention for workspaces: +When a non-default workspace is used, Atmos appends it as a suffix to the blob key using the `env:` separator: - **Default workspace**: Uses the key as-is (e.g., `terraform.tfstate`) - **Non-default workspaces**: Appends workspace as suffix (e.g., `terraform.tfstateenv:dev`)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
website/blog/2025-10-21-azure-blob-terraform-state-support.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in website/ when adding features
Ensure consistency between CLI help text and website documentation
Follow the website's documentation structure and style
Keep website code in website/ and follow its architecture/style; test changes locally
Keep CLI and website documentation in sync; document new features with examples and use casesAlways build the website (npm run build) after documentation changes to ensure no errors, broken links, or missing assets.
Files:
website/blog/2025-10-21-azure-blob-terraform-state-support.md
website/**/{*.md,*.mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use definition lists
for arguments and flags in docs instead of tables.
Files:
website/blog/2025-10-21-azure-blob-terraform-state-support.md
website/blog/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
website/blog/*.md: PRs labeled minor or major must include a blog post in website/blog/YYYY-MM-DD-feature-name.md with required frontmatter, tags, and truncate marker.
Use appropriate audience tags (feature/enhancement/bugfix or contributors) and relevant secondary tags in blog posts.
Files:
website/blog/2025-10-21-azure-blob-terraform-state-support.md
🪛 LanguageTool
website/blog/2025-10-21-azure-blob-terraform-state-support.md
[grammar] ~302-~302: Please add a punctuation mark at the end of paragraph.
Context: ... add features? Review our [contribution guide](https://atmos.tools/community/contribu...
(PUNCTUATION_PARAGRAPH_END)
[style] ~306-~306: Using many exclamation marks might seem excessive (in this case: 13 exclamation marks for a text that’s 4890 characters long)
Context: ...pport for !terraform.state. Stay tuned!
(EN_EXCESSIVE_EXCLAMATION)
⏰ 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). (7)
- GitHub Check: Build (windows)
- GitHub Check: Build (macos)
- GitHub Check: Run pre-commit hooks
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: website-deploy-preview
- GitHub Check: Summary
🔇 Additional comments (1)
website/blog/2025-10-21-azure-blob-terraform-state-support.md (1)
1-306: Blog post structure and content align well with PR objectives.Frontmatter, truncate marker, tags, and documentation structure follow guidelines. Examples are concrete and internally consistent. Technical details (DefaultAzureCredential, retry logic, workspace handling, error responses) match the implementation. Performance claims are contextualized as examples. Migration guidance is practical. Cross-references to related docs are appropriate. Minor issues flagged separately.
Verify that the referenced documentation at line 293 (
/functions/yaml/terraform.state) has been updated to include azurerm backend examples and matches any adjustments mentioned in the PR objectives forwebsite/docs/functions/yaml/terraform.state.mdx.
|
Warning This PR exceeds the recommended limit of 1,000 lines.Large PRs are difficult to review and may be rejected due to their size. Please verify that this PR does not address multiple issues. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
internal/terraform_backend/terraform_backend_azurerm.go (1)
133-137: Address the atmosConfig parameter as noted in previous review.Past review by aknysh requested this be fixed: the atmosConfig parameter is ignored and perf.Track receives nil instead of the config. This violates coding guidelines.
Apply:
func ReadTerraformBackendAzurerm( - _ *schema.AtmosConfiguration, + atmosConfig *schema.AtmosConfiguration, componentSections *map[string]any, ) ([]byte, error) { - defer perf.Track(nil, "terraform_backend.ReadTerraformBackendAzurerm")() + defer perf.Track(atmosConfig, "terraform_backend.ReadTerraformBackendAzurerm")()Based on learnings
🧹 Nitpick comments (1)
internal/terraform_backend/terraform_backend_azurerm.go (1)
201-266: Consider extracting retry logic to reduce function length.ReadTerraformBackendAzurermInternal is 66 lines, exceeding the 60-line guideline. You could extract the retry loop into a helper function to improve readability.
Example approach:
func downloadBlobWithRetry(azureClient AzureBlobAPI, containerName, blobPath string) ([]byte, error) { // retry loop here } func ReadTerraformBackendAzurermInternal(...) ([]byte, error) { // validation // call downloadBlobWithRetry }This is optional given the "Chill" review mode.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
.gitignore(1 hunks)errors/errors.go(1 hunks)go.mod(1 hunks)internal/terraform_backend/terraform_backend_azurerm.go(1 hunks)internal/terraform_backend/terraform_backend_azurerm_test.go(1 hunks)internal/terraform_backend/terraform_backend_registry.go(1 hunks)internal/terraform_backend/terraform_backend_utils.go(2 hunks)internal/terraform_backend/terraform_backend_utils_test.go(2 hunks)tests/preconditions.go(1 hunks)website/blog/2025-10-21-azure-blob-terraform-state-support.md(1 hunks)website/docs/functions/yaml/terraform.state.mdx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- internal/terraform_backend/terraform_backend_utils.go
- .gitignore
- go.mod
🧰 Additional context used
📓 Path-based instructions (6)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: Use interface-driven design with dependency injection; define interfaces for major functionality and inject dependencies.
Generate mocks with go.uber.org/mock/mockgen using //go:generate directives; never write manual mocks.
Use the functional Options pattern instead of functions with many parameters for configuration.
Use context.Context strictly for cancellation, deadlines/timeouts, and request-scoped values; never for config or dependencies. Context must be the first parameter when present.
All comments must end with periods (godot linter).
Organize imports in three groups separated by blank lines and sorted alphabetically: stdlib, third-party (not cloudposse/atmos), then Atmos packages; maintain aliases cfg, log, u, errUtils.
Add defer perf.Track(atmosConfig, "pkg.FuncName")() (or nil) and a blank line at the start of all public functions to track performance.
Use static error types from errors/errors.go, wrap with fmt.Errorf("%w: msg", err), combine with errors.Join, and check with errors.Is; never use dynamic string comparisons.
Keep files small and focused (<600 lines). One command or implementation per file; co-locate tests; never use //revive:disable:file-length-limit.
Bind environment variables with viper.BindEnv using ATMOS_ prefix (e.g., viper.BindEnv("ATMOS_VAR", "ATMOS_VAR", "FALLBACK")).
UI output (prompts/status) must go to stderr; data output to stdout; logging is for system events only and never for UI.
Ensure cross-platform compatibility: avoid hardcoded path separators; use filepath.Join and prefer SDKs over shelling out to binaries.
Files:
tests/preconditions.goerrors/errors.gointernal/terraform_backend/terraform_backend_registry.gointernal/terraform_backend/terraform_backend_azurerm.gointernal/terraform_backend/terraform_backend_azurerm_test.gointernal/terraform_backend/terraform_backend_utils_test.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
tests/preconditions.goerrors/errors.gointernal/terraform_backend/terraform_backend_registry.gointernal/terraform_backend/terraform_backend_azurerm.go
tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**: Place integration tests only under tests/ with fixtures in tests/test-cases/.
Use test precondition helpers from tests/test_preconditions.go so tests skip gracefully when prerequisites are missing.
Files:
tests/preconditions.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 multiple scenarios
**/*_test.go: Prefer unit tests with mocks over integration tests; use table-driven tests; target >80% coverage.
Write tests that validate behavior rather than implementation; avoid tautological or stub-only tests.
Tests must exercise production code paths rather than duplicating logic in test helpers.
When skipping tests, use t.Skipf("reason") with clear context; CLI tests may auto-build temporary binaries.
Files:
internal/terraform_backend/terraform_backend_azurerm_test.gointernal/terraform_backend/terraform_backend_utils_test.go
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in website/ when adding features
Ensure consistency between CLI help text and website documentation
Follow the website's documentation structure and style
Keep website code in website/ and follow its architecture/style; test changes locally
Keep CLI and website documentation in sync; document new features with examples and use cases
Files:
website/blog/2025-10-21-azure-blob-terraform-state-support.mdwebsite/docs/functions/yaml/terraform.state.mdx
website/blog/[0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9]-*.md
📄 CodeRabbit inference engine (CLAUDE.md)
PRs labeled minor or major must include a blog post with date-prefixed filename and include after the intro; tag appropriately.
Files:
website/blog/2025-10-21-azure-blob-terraform-state-support.md
🧠 Learnings (3)
📚 Learning: 2025-10-22T06:25:54.400Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T06:25:54.400Z
Learning: Applies to **/*.go : Add defer perf.Track(atmosConfig, "pkg.FuncName")() (or nil) and a blank line at the start of all public functions to track performance.
Applied to files:
internal/terraform_backend/terraform_backend_azurerm.go
📚 Learning: 2024-10-23T21:36:40.262Z
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_azurerm.go
📚 Learning: 2024-12-02T21:26:32.337Z
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_azurerm.go
🧬 Code graph analysis (4)
internal/terraform_backend/terraform_backend_registry.go (2)
pkg/config/const.go (1)
BackendTypeAzurerm(89-89)internal/terraform_backend/terraform_backend_azurerm.go (1)
ReadTerraformBackendAzurerm(133-150)
internal/terraform_backend/terraform_backend_azurerm.go (4)
pkg/perf/perf.go (1)
Track(121-138)internal/terraform_backend/terraform_backend_utils.go (3)
GetBackendAttribute(56-63)GetComponentBackend(36-43)GetTerraformWorkspace(16-23)errors/errors.go (8)
ErrStorageAccountRequired(78-78)ErrCreateAzureCredential(75-75)ErrCreateAzureClient(76-76)ErrBackendConfigRequired(80-80)ErrAzurePermissionDenied(79-79)ErrAzureContainerRequired(77-77)ErrGetBlobFromAzure(73-73)ErrReadAzureBlobBody(74-74)pkg/logger/log.go (1)
Debug(24-26)
internal/terraform_backend/terraform_backend_azurerm_test.go (4)
internal/terraform_backend/terraform_backend_azurerm.go (3)
AzureBlobDownloadResponse(46-48)ReadTerraformBackendAzurermInternal(201-267)ReadTerraformBackendAzurerm(133-150)errors/errors.go (6)
ErrAzurePermissionDenied(79-79)ErrGetBlobFromAzure(73-73)ErrAzureContainerRequired(77-77)ErrReadAzureBlobBody(74-74)ErrBackendConfigRequired(80-80)ErrStorageAccountRequired(78-78)tests/preconditions.go (1)
RequireAzureCredentials(92-123)internal/terraform_backend/terraform_backend_utils.go (1)
GetBackendAttribute(56-63)
internal/terraform_backend/terraform_backend_utils_test.go (1)
internal/terraform_backend/terraform_backend_utils.go (1)
ProcessTerraformStateFile(98-134)
🪛 LanguageTool
website/blog/2025-10-21-azure-blob-terraform-state-support.md
[style] ~308-~308: Using many exclamation marks might seem excessive (in this case: 13 exclamation marks for a text that’s 5035 characters long)
Context: ...pport for !terraform.state. Stay tuned!
(EN_EXCESSIVE_EXCLAMATION)
website/docs/functions/yaml/terraform.state.mdx
[style] ~26-~26: Consider using a more formal/concise alternative here.
Context: .... Meanwhile, if you are using backends other than local, s3, and azurerm, consider ...
(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). (9)
- GitHub Check: Build (macos)
- GitHub Check: Build (linux)
- GitHub Check: Build (windows)
- GitHub Check: website-deploy-preview
- GitHub Check: autofix
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Run pre-commit hooks
- GitHub Check: Summary
🔇 Additional comments (14)
internal/terraform_backend/terraform_backend_registry.go (1)
26-26: LGTM - Backend registration complete.The azurerm backend is now properly wired through the registry. This enables functional Terraform state reads from Azure Blob Storage.
tests/preconditions.go (1)
87-123: LGTM - Azure precondition helper implemented correctly.The function follows existing patterns for precondition checks and correctly validates Azure credentials via environment variables or Azure CLI.
website/docs/functions/yaml/terraform.state.mdx (1)
22-29: LGTM - Documentation updated for azurerm support.The docs now correctly list azurerm as a supported backend and provide appropriate fallback guidance for unsupported backends.
website/blog/2025-10-21-azure-blob-terraform-state-support.md (1)
1-308: LGTM - Comprehensive blog post for feature announcement.The blog post effectively communicates the new Azure Blob Storage support with clear examples, migration guidance, and technical details. Well done on the documentation.
internal/terraform_backend/terraform_backend_utils_test.go (1)
66-185: LGTM - Comprehensive test coverage for state processing.The expanded tests properly validate ProcessTerraformStateFile across string, number, map, array, and nested structure types. Good coverage of edge cases including empty state and invalid JSON.
errors/errors.go (1)
72-80: LGTM - Azure error sentinels follow established patterns.The new error sentinels are well-defined and enable proper error handling with errors.Is checks in the Azure backend implementation.
internal/terraform_backend/terraform_backend_azurerm_test.go (1)
1-933: LGTM - Excellent test coverage for Azure backend.Comprehensive test suite covering success paths, error scenarios, retry logic, cache behavior, and integration cases. The mock implementations properly exercise the AzureBlobAPI interface, and tests validate workspace naming, blob path construction, and error mappings.
internal/terraform_backend/terraform_backend_azurerm.go (7)
217-266: Context cancellation and exponential backoff implemented correctly.The explicit cancel() calls on each path and exponential backoff calculation (1s, 2s, 4s) properly address previous review feedback. Good work.
139-142: Nil backend check correctly implemented.The guard against nil backend prevents panics in downstream dereferences. Well done addressing the previous feedback.
22-31: LGTM - Constants properly defined.The constants for retry count, status codes, and error format string are well-defined and improve code maintainability.
35-75: LGTM - Interface design enables testability.The AzureBlobAPI interface and wrapper implementations provide clean abstraction for testing, as demonstrated in the comprehensive test suite.
80-129: LGTM - Client caching implemented correctly.The getCachedAzureBlobClient function properly caches clients by storage account, uses DefaultAzureCredential for authentication, and includes telemetry configuration.
152-171: LGTM - Workspace path construction handles Azure naming convention.The constructAzureBlobPath function correctly implements Azure's workspace naming (env:{workspace} suffix for non-default workspaces), matching the behavior validated in tests.
173-198: LGTM - Error handling maps Azure responses correctly.The handleAzureDownloadError function properly distinguishes between blob not found (404), permission denied (403), and other errors, returning appropriate sentinel errors.
Update TestReadTerraformBackendAzurerm_CachedClient to use the correct cache key format (account name only) instead of the old format (account=name;container=name). This matches the caching implementation change where clients are cached by storage account only. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/terraform_backend/terraform_backend_azurerm_test.go (1)
566-592: Consider consolidating duplicate retry tests.
TestReadTerraformBackendAzurermInternal_MaxRetriesExceededduplicates logic fromTestReadTerraformBackendAzurermInternal_NetworkError(lines 227-252). Both test retry exhaustion with persistent errors and verifymaxRetryCountAzure+1attempts.Consider removing one or combining them into a single table-driven test if you want to reduce duplication.
-func TestReadTerraformBackendAzurermInternal_MaxRetriesExceeded(t *testing.T) { - componentSections := map[string]any{ - "component": "test-component", - "workspace": "default", - } - backend := map[string]any{ - "storage_account_name": "testaccount", - "container_name": "tfstate", - "key": "terraform.tfstate", - } - - attemptCount := 0 - mockClient := &mockAzureBlobClient{ - downloadStreamFunc: func(ctx context.Context, containerName string, blobName string, options *azblob.DownloadStreamOptions) (AzureBlobDownloadResponse, error) { - attemptCount++ - // Always fail to test retry exhaustion. - return nil, errors.New("persistent error") - }, - } - - result, err := ReadTerraformBackendAzurermInternal(mockClient, &componentSections, &backend) - - assert.Error(t, err) - assert.Nil(t, result) - assert.ErrorIs(t, err, errUtils.ErrGetBlobFromAzure) - assert.Equal(t, maxRetryCountAzure+1, attemptCount, "Should exhaust all retries") -}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
internal/terraform_backend/terraform_backend_azurerm_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*_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 multiple scenarios
**/*_test.go: Prefer unit tests with mocks over integration tests; use table-driven tests; target >80% coverage.
Write tests that validate behavior rather than implementation; avoid tautological or stub-only tests.
Tests must exercise production code paths rather than duplicating logic in test helpers.
When skipping tests, use t.Skipf("reason") with clear context; CLI tests may auto-build temporary binaries.
Files:
internal/terraform_backend/terraform_backend_azurerm_test.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: Use interface-driven design with dependency injection; define interfaces for major functionality and inject dependencies.
Generate mocks with go.uber.org/mock/mockgen using //go:generate directives; never write manual mocks.
Use the functional Options pattern instead of functions with many parameters for configuration.
Use context.Context strictly for cancellation, deadlines/timeouts, and request-scoped values; never for config or dependencies. Context must be the first parameter when present.
All comments must end with periods (godot linter).
Organize imports in three groups separated by blank lines and sorted alphabetically: stdlib, third-party (not cloudposse/atmos), then Atmos packages; maintain aliases cfg, log, u, errUtils.
Add defer perf.Track(atmosConfig, "pkg.FuncName")() (or nil) and a blank line at the start of all public functions to track performance.
Use static error types from errors/errors.go, wrap with fmt.Errorf("%w: msg", err), combine with errors.Join, and check with errors.Is; never use dynamic string comparisons.
Keep files small and focused (<600 lines). One command or implementation per file; co-locate tests; never use //revive:disable:file-length-limit.
Bind environment variables with viper.BindEnv using ATMOS_ prefix (e.g., viper.BindEnv("ATMOS_VAR", "ATMOS_VAR", "FALLBACK")).
UI output (prompts/status) must go to stderr; data output to stdout; logging is for system events only and never for UI.
Ensure cross-platform compatibility: avoid hardcoded path separators; use filepath.Join and prefer SDKs over shelling out to binaries.
Files:
internal/terraform_backend/terraform_backend_azurerm_test.go
🧬 Code graph analysis (1)
internal/terraform_backend/terraform_backend_azurerm_test.go (5)
pkg/schema/schema.go (2)
Context(409-424)AtmosConfiguration(27-65)internal/terraform_backend/terraform_backend_azurerm.go (3)
AzureBlobDownloadResponse(46-48)ReadTerraformBackendAzurermInternal(201-267)ReadTerraformBackendAzurerm(133-150)errors/errors.go (6)
ErrAzurePermissionDenied(79-79)ErrGetBlobFromAzure(73-73)ErrAzureContainerRequired(77-77)ErrReadAzureBlobBody(74-74)ErrBackendConfigRequired(80-80)ErrStorageAccountRequired(78-78)tests/preconditions.go (1)
RequireAzureCredentials(92-123)internal/terraform_backend/terraform_backend_utils.go (1)
GetBackendAttribute(56-63)
⏰ 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). (9)
- GitHub Check: Build (macos)
- GitHub Check: Build (linux)
- GitHub Check: Build (windows)
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: Review Dependency Licenses
- GitHub Check: autofix
- GitHub Check: website-deploy-preview
- GitHub Check: Summary
🔇 Additional comments (7)
internal/terraform_backend/terraform_backend_azurerm_test.go (7)
21-44: LGTM! Clean mock design.The mock implementations follow interface-driven patterns effectively. Using function fields for
downloadStreamFuncgives tests full control over behavior, and thecreateMockDownloadResponsehelper reduces boilerplate nicely.
46-151: Excellent table-driven test coverage.This test thoroughly validates the success path across multiple workspace scenarios. The
expectedBlobNamefield and corresponding assertions ensure blob path construction is correct for default, named, empty workspaces, and default key behavior.
153-337: Thorough error scenario coverage.These tests validate critical error paths: blob not found (404), permission denied (403), network errors with retries, missing validation, and body read failures. Good use of
errors.Isfor type checking and proper assertions on nil returns for 404 cases.
339-420: Good validation and caching tests.These tests ensure required fields are validated (backend, storage account, container) and that client caching works correctly. The cache test properly cleans up with
defer azureBlobClientCache.Delete(cacheKey).
422-539: Well-structured unit tests for helpers.
TestConstructAzureBlobPathandTestHandleAzureDownloadErrordirectly test critical helper logic with table-driven approaches. These ensure blob path construction and error mapping work correctly across various inputs.
541-679: Good edge case coverage.These tests validate important edge cases: context timeout, large blobs, empty content, and special characters in workspace names. The large blob test is particularly good for ensuring the implementation handles real-world complexity.
681-933: Well-structured integration tests.These integration tests are properly gated with
tests.RequireAzureCredentials(t)and exercise the full path includinggetCachedAzureBlobClient. The cache determinism test (lines 808-881) is particularly valuable for verifying that cache keys work correctly based on storage account names.
Change ReadTerraformBackendAzurerm to use the atmosConfig parameter instead of ignoring it with `_`. Pass atmosConfig to perf.Track() for proper performance tracking per project guidelines. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
internal/terraform_backend/terraform_backend_utils.go (1)
113-131: Consider usingUseNumber()for numeric fidelity as previously requested.A past review suggested using
json.NewDecoderwithUseNumber()to preserve exact numeric types instead of coercing all numbers to float64. This was acknowledged by aknysh who asked jamengual to address it, but it appears unimplemented. The codebase already uses this pattern ininternal/exec/validate_utils.go.Without
UseNumber(), integer outputs like8080becomefloat64(8080), which could affect downstream type expectations.Apply this diff to use
UseNumber():+import ( + "bytes" "encoding/json" "fmt"- // Unmarshal it back to ensure consistent type representation - var convertedValue any - if err := json.Unmarshal(valueJSON, &convertedValue); err != nil { + // Unmarshal with UseNumber for numeric fidelity + var convertedValue any + dec := json.NewDecoder(bytes.NewReader(valueJSON)) + dec.UseNumber() + if err := dec.Decode(&convertedValue); err != nil { return nil, fmt.Errorf("failed to unmarshal output %s: %w", key, err) }Based on previous review comments.
errors/errors.go (1)
78-87: Place the generic sentinel outside the Azure-only block.
ErrBackendConfigRequiredis not Azure-specific; move it next to other Terraform/backend sentinels for clarity.@@ - // Azure Blob Storage specific errors. + // Azure Blob Storage specific errors. ErrGetBlobFromAzure = errors.New("failed to get blob from Azure Blob Storage") ErrReadAzureBlobBody = errors.New("failed to read Azure blob body") ErrCreateAzureCredential = errors.New("failed to create Azure credential") ErrCreateAzureClient = errors.New("failed to create Azure Blob Storage client") ErrAzureContainerRequired = errors.New("container_name is required for azurerm backend") ErrStorageAccountRequired = errors.New("storage_account_name is required for azurerm backend") ErrAzurePermissionDenied = errors.New("permission denied accessing Azure blob") - ErrBackendConfigRequired = errors.New("backend configuration is required") + @@ ErrYamlFuncInvalidArguments = errors.New("invalid number of arguments in the Atmos YAML function") ErrDescribeComponent = errors.New("failed to describe component") ErrReadTerraformState = errors.New("failed to read Terraform state") ErrEvaluateTerraformBackendVariable = errors.New("failed to evaluate terraform backend variable") ErrUnsupportedBackendType = errors.New("unsupported backend type") ErrProcessTerraformStateFile = errors.New("error processing terraform state file") + ErrBackendConfigRequired = errors.New("backend configuration is required")internal/terraform_backend/terraform_backend_azurerm.go (4)
98-101: Support sovereign clouds (endpoint override).Hardcoding
blob.core.windows.netbreaks Gov/China. Allowendpointorendpoint_suffixin backend to form the service URL.- // Construct the blob service URL. - serviceURL := fmt.Sprintf("https://%s.blob.core.windows.net/", storageAccountName) + // Construct the blob service URL (support sovereign clouds). + endpoint := GetBackendAttribute(backend, "endpoint") // full URL, e.g. https://acct.blob.usgovcloudapi.net/ + endpointSuffix := GetBackendAttribute(backend, "endpoint_suffix") // e.g. blob.usgovcloudapi.net + var serviceURL string + if endpoint != "" { + serviceURL = endpoint + } else { + suffix := "blob.core.windows.net" + if endpointSuffix != "" { + suffix = endpointSuffix + } + serviceURL = fmt.Sprintf("https://%s.%s/", storageAccountName, suffix) + }
112-119: Richer telemetry.Include version in
ApplicationIDif available, e.g.,atmos/<version>. Helps Azure support and tracing.
173-198: Map 401 Unauthorized to the permission-denied sentinel.Treat 401 the same as 403 for clearer caller semantics.
@@ const ( @@ // StatusCodeForbiddenAzure represents the HTTP 403 status code. statusCodeForbiddenAzure = 403 + // StatusCodeUnauthorizedAzure represents the HTTP 401 status code. + statusCodeUnauthorizedAzure = 401 @@ if errors.As(err, &respErr) { switch respErr.StatusCode { case statusCodeNotFoundAzure: @@ - case statusCodeForbiddenAzure: + case statusCodeForbiddenAzure, statusCodeUnauthorizedAzure: return fmt.Errorf( "%w: blob '%s' in container '%s': %v", errUtils.ErrAzurePermissionDenied, tfStateFilePath, containerName, err, )Add a unit test mirroring
TestReadTerraformBackendAzurermInternal_PermissionDeniedusingStatusCode: 401.
Based on learnings.
152-171: Workspace suffix behavior is correct; add a brief code comment.Terraform azurerm backend appends
env:{workspace}to the key for non-default workspaces. A short comment here reduces future churn. (stackoverflow.com)internal/terraform_backend/terraform_backend_azurerm_test.go (2)
201-225: Add a 401 Unauthorized test case.Mirror this test with
StatusCode: 401and assertErrAzurePermissionDeniedto validate the new mapping.
46-151: Optional: run table-driven subtests in parallel where safe.Add
t.Parallel()inside subtests that do not mutateazureBlobClientCacheto speed CI.Also applies to: 153-199, 254-286, 312-337, 429-539
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
.gitignore(1 hunks)errors/errors.go(1 hunks)go.mod(1 hunks)internal/terraform_backend/terraform_backend_azurerm.go(1 hunks)internal/terraform_backend/terraform_backend_azurerm_test.go(1 hunks)internal/terraform_backend/terraform_backend_registry.go(1 hunks)internal/terraform_backend/terraform_backend_utils.go(2 hunks)internal/terraform_backend/terraform_backend_utils_test.go(2 hunks)tests/preconditions.go(1 hunks)website/blog/2025-10-21-azure-blob-terraform-state-support.md(1 hunks)website/docs/functions/yaml/terraform.state.mdx(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-10-23T00:58:28.954Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T00:58:28.954Z
Learning: Applies to {internal,pkg}/**/*.go : Add defer perf.Track(atmosConfig, "pkg.FuncName")() + a blank line at the start of all public functions (use nil if no atmosConfig)
Applied to files:
internal/terraform_backend/terraform_backend_azurerm.go
📚 Learning: 2024-12-02T21:26:32.337Z
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_azurerm.go
📚 Learning: 2024-10-23T21:36:40.262Z
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_azurerm.go
📚 Learning: 2025-10-23T00:58:28.954Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T00:58:28.954Z
Learning: Applies to **/*.go : All comments must end with periods (godot linter)
Applied to files:
internal/terraform_backend/terraform_backend_azurerm_test.go
🪛 LanguageTool
website/docs/functions/yaml/terraform.state.mdx
[style] ~26-~26: Consider using a more formal/concise alternative here.
Context: .... Meanwhile, if you are using backends other than local, s3, and azurerm, consider ...
(OTHER_THAN)
website/blog/2025-10-21-azure-blob-terraform-state-support.md
[style] ~308-~308: Using many exclamation marks might seem excessive (in this case: 13 exclamation marks for a text that’s 5035 characters long)
Context: ...pport for !terraform.state. Stay tuned!
(EN_EXCESSIVE_EXCLAMATION)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build (linux)
- GitHub Check: Summary
🔇 Additional comments (8)
internal/terraform_backend/terraform_backend_registry.go (1)
26-26: Azure backend registration looks correct.The wiring follows the same pattern as S3 and local backends.
website/docs/functions/yaml/terraform.state.mdx (1)
22-22: Documentation correctly reflects Azure backend support.The addition of
azurermto the supported backends list aligns with the implementation.go.mod (1)
12-12: Dependency correctly promoted to direct.Moving
azblobfrom indirect to direct dependency is expected when adding Azure backend implementation.internal/terraform_backend/terraform_backend_utils_test.go (1)
67-185: Comprehensive test coverage for state processing.The expanded test cases cover various value types (numbers, objects, arrays, nested structures) and edge cases (empty state, invalid JSON). Type assertions are now safely guarded with
assert.IsTypechecks.internal/terraform_backend/terraform_backend_utils.go (1)
152-152: Error message correctly updated.Including
azurermin the list of supported backends provides accurate guidance to users.tests/preconditions.go (1)
87-123: Azure credential precondition helper looks solid.The implementation follows the same pattern as
RequireAWSProfileand accurately checks for Azure credentials via environment variables and Azure CLI. The doc comment correctly describes what the function does.internal/terraform_backend/terraform_backend_azurerm.go (1)
216-266: Retries, backoff, and per-attempt cancellation look solid.Exponential backoff and explicit
cancel()per attempt prevent timer leaks and align with PR intent.internal/terraform_backend/terraform_backend_azurerm_test.go (1)
388-420: Nice cache test.Good use of pre-populating
azureBlobClientCacheand cleanup viaDelete.
aknysh
left a comment
There was a problem hiding this comment.
@jamengual thanks for the PR, please see comment
Revert the JSON marshal/unmarshal round-trip in ProcessTerraformStateFile back to the original simple assignment. The round-trip was unnecessary because output.Value is already the correct Go type after json.Unmarshal unmarshals the state file. The original code works correctly and the round-trip adds no value while adding computational overhead. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
These changes were released in v1.196.0-rc.1. |
what
!terraform.stateYAML functionwhy
!terraform.statefunction previously only supportedlocalands3backendschanges
New Implementation:
internal/terraform_backend/terraform_backend_azurerm.goenv:/{workspace}/{key}for non-default workspaces)Comprehensive Tests:
internal/terraform_backend/terraform_backend_azurerm_test.goError Definitions:
errors/errors.goRegistry Update:
internal/terraform_backend/terraform_backend_registry.goError Message Update:
internal/terraform_backend/terraform_backend_utils.goazurermDocumentation Update:
website/docs/functions/yaml/terraform.state.mdxDependencies:
go.modgithub.com/Azure/azure-sdk-for-go/sdk/storage/azblobfrom indirect to direct dependency (already present in project)implementation notes
env:/{workspace}/{key})defer perf.Track()on all functionstest results
Summary by CodeRabbit
New Features
Documentation
Improvements
Tests
Chores