Skip to content

Add Azure Blob Storage (azurerm) backend support for !terraform.state function#1610

Merged
aknysh merged 29 commits intomainfrom
feature/azure-blob-terraform-state
Oct 23, 2025
Merged

Add Azure Blob Storage (azurerm) backend support for !terraform.state function#1610
aknysh merged 29 commits intomainfrom
feature/azure-blob-terraform-state

Conversation

@jamengual
Copy link
Contributor

@jamengual jamengual commented Oct 10, 2025

what

  • Implemented Azure Blob Storage backend support for the !terraform.state YAML function
  • Added comprehensive unit tests with 100% coverage for the new backend
  • Updated error definitions, registry, and documentation

why

  • The !terraform.state function previously only supported local and s3 backends
  • Azure users needed native azurerm backend support to read Terraform state directly from Azure Blob Storage
  • This provides the fastest way to retrieve Terraform outputs without Terraform initialization overhead

changes

  • New Implementation: internal/terraform_backend/terraform_backend_azurerm.go

    • Implements azurerm backend reader following S3 backend patterns
    • Uses Azure SDK with DefaultAzureCredential for authentication (Managed Identity, Service Principal, Azure CLI, etc.)
    • Supports workspace-based blob paths (env:/{workspace}/{key} for non-default workspaces)
    • Includes client caching, retry logic (2 retries with exponential backoff), and proper error handling
    • Handles 404 (blob not found) gracefully by returning nil (component not provisioned yet)
    • Handles 403 (permission denied) with descriptive error messages
  • Comprehensive Tests: internal/terraform_backend/terraform_backend_azurerm_test.go

    • 8 test functions covering all scenarios with mocked Azure SDK client
    • Tests workspace handling (default vs non-default), blob not found, permission denied, network errors, retry logic, and error cases
    • All tests pass with no external dependencies required
  • Error Definitions: errors/errors.go

    • Added 7 new Azure-specific static errors following project patterns
    • ErrGetBlobFromAzure, ErrReadAzureBlobBody, ErrCreateAzureCredential, ErrCreateAzureClient, ErrAzureContainerRequired, ErrStorageAccountRequired, ErrAzurePermissionDenied
  • Registry Update: internal/terraform_backend/terraform_backend_registry.go

    • Registered ReadTerraformBackendAzurerm in the backend registry
  • Error Message Update: internal/terraform_backend/terraform_backend_utils.go

    • Updated supported backends list to include azurerm
  • Documentation Update: website/docs/functions/yaml/terraform.state.mdx

    • Added azurerm to the list of supported backend types
    • Updated warning message to reflect azurerm support
  • Dependencies: go.mod

    • Moved github.com/Azure/azure-sdk-for-go/sdk/storage/azblob from indirect to direct dependency (already present in project)

implementation notes

  • Follows established patterns from S3 backend implementation
  • Uses wrapper pattern (AzureBlobAPI interface) to enable testing without actual Azure connectivity
  • Implements proper workspace path handling matching Azure backend behavior (env:/{workspace}/{key})
  • All comments end with periods (enforced by golangci-lint)
  • Imports organized in 3 groups (stdlib, 3rd-party, atmos) as per CLAUDE.md
  • Performance tracking added with defer perf.Track() on all functions
  • Cross-platform compatible using Azure SDK (not CLI commands)

test results

=== RUN   TestReadTerraformBackendAzurermInternal_Success
=== RUN   TestReadTerraformBackendAzurermInternal_Success/successful_read_default_workspace
=== RUN   TestReadTerraformBackendAzurermInternal_Success/successful_read_dev_workspace
=== RUN   TestReadTerraformBackendAzurermInternal_Success/successful_read_prod_workspace
=== RUN   TestReadTerraformBackendAzurermInternal_Success/successful_read_empty_workspace
=== RUN   TestReadTerraformBackendAzurermInternal_Success/successful_read_default_key
--- PASS: TestReadTerraformBackendAzurermInternal_Success (0.00s)
=== RUN   TestReadTerraformBackendAzurermInternal_BlobNotFound
--- PASS: TestReadTerraformBackendAzurermInternal_BlobNotFound (0.00s)
=== RUN   TestReadTerraformBackendAzurermInternal_PermissionDenied
--- PASS: TestReadTerraformBackendAzurermInternal_PermissionDenied (0.00s)
=== RUN   TestReadTerraformBackendAzurermInternal_NetworkError
--- PASS: TestReadTerraformBackendAzurermInternal_NetworkError (4.00s)
=== RUN   TestReadTerraformBackendAzurermInternal_RetrySuccess
--- PASS: TestReadTerraformBackendAzurermInternal_RetrySuccess (2.00s)
=== RUN   TestReadTerraformBackendAzurermInternal_MissingContainerName
--- PASS: TestReadTerraformBackendAzurermInternal_MissingContainerName (0.00s)
=== RUN   TestReadTerraformBackendAzurermInternal_ReadBodyError
--- PASS: TestReadTerraformBackendAzurermInternal_ReadBodyError (0.00s)
PASS
ok      github.com/cloudposse/atmos/internal/terraform_backend 7.011s

Summary by CodeRabbit

  • New Features

    • Azure Blob Storage (azurerm) support for reading Terraform state with workspace-aware paths, authentication, retries, and client caching.
  • Documentation

    • Added detailed docs and a blog post covering Azure backend usage, examples, migration guidance, and “Try It Now” steps.
  • Improvements

    • Clearer permission/not-found reporting and added Azure-specific error signals for more precise error handling.
  • Tests

    • Extensive unit and integration tests plus Azure credential precondition checks.
  • Chores

    • Updated .gitignore with developer tool patterns.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 10, 2025

📝 Walkthrough

Walkthrough

Adds 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

Cohort / File(s) Summary
Errors
errors/errors.go
Added Azure-specific sentinel errors: ErrGetBlobFromAzure, ErrReadAzureBlobBody, ErrCreateAzureCredential, ErrCreateAzureClient, ErrAzureContainerRequired, ErrStorageAccountRequired, ErrAzurePermissionDenied, ErrBackendConfigRequired.
Azure backend implementation
internal/terraform_backend/terraform_backend_azurerm.go
New Azurerm backend reader: AzureBlobAPI / AzureBlobDownloadResponse interfaces, azureBlobClientWrapper and cache, ReadTerraformBackendAzurerm / ReadTerraformBackendAzurermInternal, deterministic blob path construction, retry/backoff (2 retries), per-attempt timeouts, and error mapping helpers.
Azure backend tests
internal/terraform_backend/terraform_backend_azurerm_test.go
New unit tests and mocks covering success, retries, error mappings (404/403/etc.), cache behavior, path construction, timeouts, large/special cases, and guarded integration tests.
Backend registry
internal/terraform_backend/terraform_backend_registry.go
Registered Azurerm backend read function (ReadTerraformBackendAzurerm) in backend registry.
Utils & tests
internal/terraform_backend/terraform_backend_utils.go, internal/terraform_backend/terraform_backend_utils_test.go
Error message updated to include azurerm as supported backend; tests expanded for ProcessTerraformStateFile (numbers, maps, lists, nested, empty/invalid).
Docs & blog
website/docs/functions/yaml/terraform.state.mdx, website/blog/2025-10-21-azure-blob-terraform-state-support.md
Added azurerm to docs and new blog post describing Azure Blob backend support, auth, examples, and migration guidance.
Test preconditions
tests/preconditions.go
Added RequireAzureCredentials(t *testing.T) to detect or skip Azure credential-dependent integration tests.
go.mod
go.mod
Promoted github.com/Azure/azure-sdk-for-go/sdk/storage/azblob v1.6.1 from indirect to direct dependency.
Ignore patterns
.gitignore
Added ignore entries for dev tool artifacts (.claude-flow/, .hive-mind/, plus existing artifact rules).

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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested reviewers

  • aknysh
  • osterman
  • mcalhoun

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "Add Azure Blob Storage (azurerm) backend support for !terraform.state function" directly and accurately describes the main change in the changeset. The implementation across multiple files (terraform_backend_azurerm.go, error definitions, registry updates, documentation, and dependency management) collectively work toward this single objective: enabling Azure Blob Storage as a backend for the !terraform.state YAML function. The title is specific, mentioning both the technology (Azure Blob Storage) and its context (!terraform.state), and avoids vague terms or noise. A teammate scanning the commit history would immediately understand this PR's purpose.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/azure-blob-terraform-state

📜 Recent 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 2dd1786 and 4bb9af2.

📒 Files selected for processing (1)
  • internal/terraform_backend/terraform_backend_utils.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • internal/terraform_backend/terraform_backend_utils.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
  • GitHub Check: Build (macos)
  • GitHub Check: Build (windows)
  • GitHub Check: Build (linux)
  • GitHub Check: Run pre-commit hooks
  • GitHub Check: website-deploy-preview
  • GitHub Check: autofix
  • GitHub Check: Lint (golangci)
  • GitHub Check: Analyze (go)
  • GitHub Check: Review Dependency Licenses
  • GitHub Check: Summary

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between f69c0b4 and 2305082.

📒 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.go
  • internal/terraform_backend/terraform_backend_registry.go
  • internal/terraform_backend/terraform_backend_utils.go
  • internal/terraform_backend/terraform_backend_azurerm_test.go
  • internal/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.go
  • internal/terraform_backend/terraform_backend_registry.go
  • internal/terraform_backend/terraform_backend_utils.go
  • internal/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: Add defer 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.go
  • internal/terraform_backend/terraform_backend_utils.go
  • internal/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.go
  • internal/terraform_backend/terraform_backend_utils.go
  • internal/terraform_backend/terraform_backend_azurerm_test.go
  • internal/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
Copy link

codecov bot commented Oct 10, 2025

Codecov Report

❌ Patch coverage is 84.06593% with 29 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.68%. Comparing base (afc9188) to head (4bb9af2).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
tests/preconditions.go 24.00% 19 Missing ⚠️
...nal/terraform_backend/terraform_backend_azurerm.go 93.54% 8 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            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     
Flag Coverage Δ
unittests 67.68% <84.06%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
errors/errors.go 100.00% <ø> (ø)
...al/terraform_backend/terraform_backend_registry.go 100.00% <100.00%> (ø)
...ernal/terraform_backend/terraform_backend_utils.go 89.53% <100.00%> (ø)
...nal/terraform_backend/terraform_backend_azurerm.go 93.54% <93.54%> (ø)
tests/preconditions.go 72.79% <24.00%> (-4.73%) ⬇️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 \r prefix 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 \r is 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 399adf5 and 4ff0986.

📒 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: Add defer 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, nil

Also 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 . AzureBlobAPI

Then 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 sleep var (defaults to time.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.

📥 Commits

Reviewing files that changed from the base of the PR and between 8c0bc35 and 58d2ee2.

📒 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 mockgen directives 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.go
  • internal/terraform_backend/terraform_backend_azurerm.go
{cmd,internal,pkg}/**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

{cmd,internal,pkg}/**/*.go: Add defer 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.go
  • internal/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.go
  • internal/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.go
  • internal/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.

jamengual added a commit that referenced this pull request Oct 21, 2025
## 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>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 AzureBlobAPI

As 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 21a5d58 and 314afef.

📒 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.go
  • internal/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.go
  • internal/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.go
  • internal/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.

@osterman osterman added the minor New features that do not break anything label Oct 21, 2025
@github-actions
Copy link

Warning

Changelog Entry Required

This PR is labeled minor or major but doesn't include a changelog entry.

Action needed: Add a new blog post in website/blog/ to announce this change.

Example filename: website/blog/2025-10-21-feature-name.mdx

Alternatively: If this change doesn't require a changelog entry, remove the minor or major label.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 314afef and 45d90f6.

📒 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.go
  • internal/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.go
  • internal/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.go
  • internal/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 atmosConfig and 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.PrintfMessageToTUI correctly 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 createMockDownloadResponse reduces 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 ErrAzurePermissionDenied and 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 + 1 total attempts) before returning ErrGetBlobFromAzure.


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_name is 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 ReadTerraformBackendAzurerm to ensure missing backend config is caught at the API boundary.


350-355: Simple and effective test helper.

The errorReader provides a clean way to simulate body read failures without external dependencies.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 45d90f6 and 38fe527.

📒 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 cases

Always 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 for website/docs/functions/yaml/terraform.state.mdx.

@github-actions github-actions bot added size/xl Extra large size PR and removed size/l Large size PR labels Oct 21, 2025
@mergify
Copy link

mergify bot commented Oct 21, 2025

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.
Consider refactoring it into smaller, more focused PRs to facilitate a smoother review process.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 52ddb1f and 97f9060.

📒 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.go
  • errors/errors.go
  • internal/terraform_backend/terraform_backend_registry.go
  • internal/terraform_backend/terraform_backend_azurerm.go
  • internal/terraform_backend/terraform_backend_azurerm_test.go
  • internal/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.go
  • errors/errors.go
  • internal/terraform_backend/terraform_backend_registry.go
  • internal/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.go
  • internal/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.md
  • website/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>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
internal/terraform_backend/terraform_backend_azurerm_test.go (1)

566-592: Consider consolidating duplicate retry tests.

TestReadTerraformBackendAzurermInternal_MaxRetriesExceeded duplicates logic from TestReadTerraformBackendAzurermInternal_NetworkError (lines 227-252). Both test retry exhaustion with persistent errors and verify maxRetryCountAzure+1 attempts.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 97f9060 and 65fb4a5.

📒 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 downloadStreamFunc gives tests full control over behavior, and the createMockDownloadResponse helper reduces boilerplate nicely.


46-151: Excellent table-driven test coverage.

This test thoroughly validates the success path across multiple workspace scenarios. The expectedBlobName field 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.Is for 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.

TestConstructAzureBlobPath and TestHandleAzureDownloadError directly 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 including getCachedAzureBlobClient. 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>
@aknysh
Copy link
Member

aknysh commented Oct 23, 2025

@coderabbitai full review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 23, 2025

✅ Actions performed

Full review triggered.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (8)
internal/terraform_backend/terraform_backend_utils.go (1)

113-131: Consider using UseNumber() for numeric fidelity as previously requested.

A past review suggested using json.NewDecoder with UseNumber() 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 in internal/exec/validate_utils.go.

Without UseNumber(), integer outputs like 8080 become float64(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.

ErrBackendConfigRequired is 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.net breaks Gov/China. Allow endpoint or endpoint_suffix in 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 ApplicationID if 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_PermissionDenied using StatusCode: 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: 401 and assert ErrAzurePermissionDenied to validate the new mapping.


46-151: Optional: run table-driven subtests in parallel where safe.

Add t.Parallel() inside subtests that do not mutate azureBlobClientCache to 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.

📥 Commits

Reviewing files that changed from the base of the PR and between afc9188 and 2dd1786.

📒 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 azurerm to the supported backends list aligns with the implementation.

go.mod (1)

12-12: Dependency correctly promoted to direct.

Moving azblob from 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.IsType checks.

internal/terraform_backend/terraform_backend_utils.go (1)

152-152: Error message correctly updated.

Including azurerm in 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 RequireAWSProfile and 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 azureBlobClientCache and cleanup via Delete.

Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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>
@aknysh aknysh merged commit 75476a1 into main Oct 23, 2025
54 checks passed
@aknysh aknysh deleted the feature/azure-blob-terraform-state branch October 23, 2025 05:29
@mergify mergify bot removed the needs-cloudposse Needs Cloud Posse assistance label Oct 23, 2025
@github-actions
Copy link

These changes were released in v1.196.0-rc.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

minor New features that do not break anything size/xl Extra large size PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants