feat: Add multiple terraform output formats for CI integration #1885
Conversation
|
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. |
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughRefactors and extracts Terraform output logic into a new pkg/terraform/output package: adds a DI-capable Executor, formatting/export options ( Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as atmos CLI
participant OutputPkg as pkg/terraform/output.Executor
participant Describer as ComponentDescriber
participant Backend as BackendGenerator
participant Env as EnvironmentSetup
participant RunnerFact as RunnerFactory
participant Runner as TerraformRunner
participant Workspace as WorkspaceManager
participant Formatter as Formatter
participant FS as File/Stdout
CLI->>OutputPkg: prepareOutputContext(format,args)
OutputPkg->>Describer: DescribeComponent(params)
Describer-->>OutputPkg: sections (component config, remote-state)
OutputPkg->>Backend: GenerateBackendIfNeeded(config, component, stack)
OutputPkg->>Env: SetupEnvironment(config, authContext)
OutputPkg->>RunnerFact: defaultRunnerFactory(workdir, executable)
RunnerFact-->>Runner: TerraformRunner instance
OutputPkg->>Workspace: EnsureWorkspace(ctx, runner, workspace, backendType,...)
OutputPkg->>Runner: Init(ctx) [optional]
OutputPkg->>Runner: Output(ctx)
Runner-->>OutputPkg: raw tf outputs
OutputPkg->>Formatter: FormatOutputs / FormatSingleValue(outputs, format, opts)
Formatter-->>OutputPkg: formatted string
OutputPkg->>FS: WriteToFile or stdout
FS-->>CLI: done
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📝 WalkthroughWalkthroughThis PR introduces a new Changes
Sequence DiagramsequenceDiagram
participant Client
participant CLI as CLI Output Cmd
participant Executor as pkg/terraform/output
participant Adapter as Internal Adapters
participant Terraform as Terraform/tfexec
participant Cache as Output Cache
participant Formatter as Format Engine
Client->>CLI: atmos terraform output --format json
CLI->>CLI: Validate format flag
CLI->>CLI: Prepare Atmos config context
CLI->>Executor: NewExecutor with adapters
Executor->>Cache: Check cached outputs
alt Cache Hit
Cache-->>Executor: Return cached outputs
else Cache Miss
Executor->>Adapter: DescribeComponent
Adapter-->>Executor: Component config (executable, workspace, etc.)
Executor->>Executor: GenerateBackendIfNeeded
Executor->>Executor: SetupEnvironment (with AWS auth if needed)
Executor->>Executor: EnsureWorkspace
Executor->>Terraform: Init
Terraform-->>Executor: Init complete
Executor->>Terraform: Output
Terraform-->>Executor: Output map[string]OutputMeta
Executor->>Cache: Store outputs
Cache-->>Executor: Cache stored
end
Executor->>Formatter: FormatOutputs(outputs, format)
Formatter-->>CLI: Formatted string (JSON/YAML/HCL/etc.)
alt Output File
CLI->>CLI: Write to --output-file
else Stdout
CLI-->>Client: Print to stdout
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (12)
errors/errors.go (1)
104-115: Approve new error sentinels with minor naming suggestion.The new error sentinels follow the project's error handling pattern correctly and support the Terraform output refactoring well. All messages are clear and descriptive.
Minor suggestion: Line 110 has
ErrInvalidComponentInfoS- consider renaming toErrInvalidComponentInfo(remove the trailing 'S') for consistency with the other error names.Apply this diff to fix the naming:
- ErrInvalidComponentInfoS = errors.New("component has invalid 'component_info' section") + ErrInvalidComponentInfo = errors.New("component has invalid 'component_info' section")pkg/terraform/output/platform_windows.go (1)
14-16: Redundantruntime.GOOScheck.This file has
//go:build windows, soruntime.GOOSwill always be"windows"here. The check is harmless but unnecessary.internal/exec/terraform_output_getter.go (1)
67-79: Parameter order inconsistency with other functions.
GetAllTerraformOutputstakes(component, stack)whileGetTerraformOutputabove takes(stack, component). This asymmetry can cause subtle bugs when developers switch between these functions.Consider documenting this explicitly or aligning parameter order in future refactors.
cmd/terraform/output.go (1)
149-158:extractOutputNamereturns first match without validation.If multiple positional args are passed, only the first is used and others are silently ignored. Consider logging a warning if
len(args) > 1after filtering.pkg/terraform/output/config.go (1)
184-189: Consider removing perf.Track for trivial functions.
GetComponentInfois a simple string formatter. Theperf.Trackoverhead may exceed the function's actual work. Same consideration applies toGetFlattenSeparatorin output.go.pkg/terraform/output/backend.go (1)
85-103: Double validation of backendType.
generateBackendConfigvalidatesbackendType != ""on line 92, but this is already validated byValidateBackendConfigcalled at line 36. The check here is defensive but redundant when called fromGenerateBackendIfNeeded.The redundancy is fine for safety if
generateBackendConfigmight be called from other paths in the future. Consider adding a brief comment noting this is defensive validation.pkg/terraform/output/output.go (2)
58-66: perf.Track on GetFlattenSeparator is excessive overhead.This is a trivial string getter. The tracking overhead likely exceeds the function's work.
func (o FormatOptions) GetFlattenSeparator() string { - defer perf.Track(nil, "output.FormatOptions.GetFlattenSeparator")() - if o.FlattenSeparator == "" { return DefaultFlattenSeparator } return o.FlattenSeparator }
100-114: WriteToFile lacks error wrapping.The raw error from
os.OpenFileorWriteStringis returned without context. Per coding guidelines, errors should be wrapped withfmt.Errorf("context: %w", err)or use the error builder.func WriteToFile(filePath string, content string) error { defer perf.Track(nil, "output.WriteToFile")() f, err := os.OpenFile(filePath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, DefaultFileMode) if err != nil { - return err + return fmt.Errorf("failed to open file %s: %w", filePath, err) } defer f.Close() - _, err = f.WriteString(content) - return err + if _, err = f.WriteString(content); err != nil { + return fmt.Errorf("failed to write to file %s: %w", filePath, err) + } + return nil }You'll need to add
"fmt"to the imports.pkg/terraform/output/format.go (1)
475-487: HCL object formatting uses fixed indentation.The
formatHCLObjectuses hardcoded 4-space indentation which works for top-level objects but may produce inconsistent formatting for deeply nested structures. Consider if this is acceptable for your use case.If deep nesting support is needed later, you could pass an indent level parameter:
-func formatHCLObject(v map[string]any) (string, error) { +func formatHCLObject(v map[string]any, indent int) (string, error) {pkg/terraform/output/executor.go (3)
104-116: Consider removing perf.Track from high-frequency I/O methods.
WriteandStringonquietModeWritermay be called many times during terraform execution. Theperf.Trackoverhead could add up.func (w *quietModeWriter) Write(p []byte) (n int, err error) { - defer perf.Track(nil, "output.quietModeWriter.Write")() - return w.buffer.Write(p) } func (w *quietModeWriter) String() string { - defer perf.Track(nil, "output.quietModeWriter.String")() - return w.buffer.String() }
191-192: ParameterskipInitis accepted but ignored.The comment explains the current behavior, but accepting a parameter that has no effect can confuse callers. Consider removing it or implementing the behavior.
Either implement
skipInitor remove it from the signature to avoid confusion:func (e *Executor) GetAllOutputs( atmosConfig *schema.AtmosConfiguration, component string, stack string, - skipInit bool, ) (map[string]any, error) {
534-544: Unused parameters instartSpinnerOrLog.The
componentandstackparameters (third and fourth) are named with_indicating they're unused. Consider removing them if not needed.-func startSpinnerOrLog(atmosConfig *schema.AtmosConfiguration, message, _, _ string) func() { +func startSpinnerOrLog(atmosConfig *schema.AtmosConfiguration, message string) func() {You'd also need to update call sites.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (13)
CLAUDE.md (1)
22-32: Good addition—practical guidance for parallel sessions. The new "Concurrent Sessions" section is well-placed and addresses a real workflow scenario. Content is clear and follows the existing MANDATORY section style.One small thing: Line 32 uses straight quotes (
"unknown") instead of typographic quotes, per the godot linter. Fix:- **When you see unfamiliar files**, assume another session created them - ask the user what to do + **When you see unfamiliar files**, assume another session created them - ask the user what to do - **If pre-commit hooks fail due to files you didn't touch**, ask the user how to proceed rather than trying to fix or remove themMore specifically, replace
"unknown"on line 32 with curly quotes ("unknown").pkg/terraform/output/platform_windows.go (2)
12-17: Redundantruntime.GOOScheck in Windows-only build.Since this file has
//go:build windows, theruntime.GOOS == "windows"check on line 14 will always be true. The check could be removed, but keeping it may serve as self-documentation for readers who don't notice the build tag.If you prefer to simplify:
func windowsFileDelay() { - if runtime.GOOS == "windows" { - time.Sleep(200 * time.Millisecond) - } + time.Sleep(200 * time.Millisecond) }
19-41: Same note on the GOOS check in retryOnWindows.The early return on line 22-24 will never trigger in this Windows-only file. Consider removing for clarity, or keep it for symmetry with the non-Windows counterpart.
internal/exec/component_describer_adapter.go (1)
16-22: Silent type assertion failure could mask configuration errors.If
params.AuthManageris non-nil but not anauth.AuthManager, the code silently proceeds withauthMgras nil. This could hide integration bugs.Consider logging when the type assertion fails:
var authMgr auth.AuthManager if params.AuthManager != nil { if am, ok := params.AuthManager.(auth.AuthManager); ok { authMgr = am + } else { + // Log unexpected type for debugging. + log.Debug("AuthManager type assertion failed", "type", fmt.Sprintf("%T", params.AuthManager)) } }pkg/terraform/output/workspace.go (1)
46-52: Inconsistent atmosConfig usage in perf.Track.
CleanWorkspacepassesatmosConfigtoperf.Track, butEnsureWorkspacepassesnil. For consistency and better tracing, consider passingatmosConfigif available (or document why it's unavailable here).pkg/terraform/output/config.go (1)
184-189: Consider removing perf.Track from trivial helper.
GetComponentInfois a simplefmt.Sprintfcall. Theperf.Trackoverhead (even as a no-op when disabled) might be unnecessary for such a lightweight function.That said, if the linting policy enforces perf tracking on all public functions, this is fine as-is.
pkg/terraform/output/format.go (1)
184-200: Mixed error handling pattern with fmt.Errorf.Lines 188, 197-198, and others use
fmt.Errorfdirectly instead oferrUtils.Build. Per coding guidelines, errors should be wrapped using static errors fromerrors/errors.go.Consider using the errUtils pattern consistently:
- return "", fmt.Errorf("failed to marshal value to JSON: %w", err) + return "", errUtils.Build(errUtils.ErrJSONMarshal). + WithCause(err). + WithExplanation("Failed to marshal value to JSON."). + Err()Or if these are internal low-level errors that don't need sentinel wrapping, document that choice.
pkg/terraform/output/environment.go (1)
3-11: Import grouping could be improved.The imports are all atmos packages except for
fmt. Per coding guidelines, imports should be organized in three groups: stdlib, 3rd-party, atmos packages. Currently all atmos packages are grouped together correctly, which is fine.import ( "fmt" + awsCloud "github.com/cloudposse/atmos/pkg/auth/cloud/aws"errors/errors.go (1)
106-115: New error sentinels look good overall.Minor nit:
ErrInvalidComponentInfoSappears truncated - was this meant to beErrInvalidComponentInfoSectionfor consistency with similar errors likeErrInvalidSettingsSection?- ErrInvalidComponentInfoS = errors.New("component has invalid 'component_info' section") + ErrInvalidComponentInfoSection = errors.New("component has invalid 'component_info' section")pkg/terraform/output/output.go (1)
102-114: Consider wrapping the file open error with context.Per coding guidelines, wrap errors with context using
fmt.Errorf.func WriteToFile(filePath string, content string) error { defer perf.Track(nil, "output.WriteToFile")() // Open file in append mode, create if doesn't exist. f, err := os.OpenFile(filePath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, DefaultFileMode) if err != nil { - return err + return fmt.Errorf("failed to open file %s: %w", filePath, err) } defer f.Close() - _, err = f.WriteString(content) - return err + if _, err = f.WriteString(content); err != nil { + return fmt.Errorf("failed to write to file %s: %w", filePath, err) + } + return nil }pkg/terraform/output/executor.go (3)
191-193: Document the skipInit TODO or address it.The
skipInitparameter is explicitly ignored. Consider either removing it from the signature or documenting why it's deferred.- // Note: skipInit is currently ignored - terraform init is always run to ensure correct state. - _ = skipInit + // TODO(#XXXX): skipInit is currently ignored. Terraform init is always run to ensure correct state. + // Future optimization: skip init when workspace is already initialized and backend matches. + _ = skipInit
534-544: Unused parameters should be documented or removed.The function signature has
_, _for component and stack parameters. Either use them in logging or document why they're kept for future use.-// startSpinnerOrLog starts a spinner in normal mode or logs in debug mode, returns a stop function. -func startSpinnerOrLog(atmosConfig *schema.AtmosConfiguration, message, _, _ string) func() { +// startSpinnerOrLog starts a spinner in normal mode or logs in debug mode, returns a stop function. +// Component and stack parameters are reserved for future structured logging enhancements. +func startSpinnerOrLog(atmosConfig *schema.AtmosConfiguration, message, _ /* component */, _ /* stack */ string) func() {
502-506: Consider propagating conversion errors instead of silently returning nil.When
ConvertFromJSONfails, the error is logged but the output value becomes nil. This could mask issues. Consider whether this should return an error or at least include the raw string as fallback.d, err := u.ConvertFromJSON(s) if err != nil { log.Error("Failed to convert output", "key", k, "error", err) - return k, nil + // Return raw string value as fallback to preserve data + return k, s }
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/terraform/output/workspace.go (1)
62-90: Consider differentiating workspace errors for clearer diagnostics.The current implementation treats any
WorkspaceNewerror as "workspace already exists" and falls back toWorkspaceSelect. While this fallback strategy will eventually surface real errors (network, permissions) whenWorkspaceSelectalso fails, distinguishing error types up front would improve debugging and avoid unnecessary operations.💡 Optional enhancement for error differentiation
If the Terraform library provides typed errors or distinguishable error messages for "workspace already exists," you could check for that specific condition before falling back. For example:
err := runner.WorkspaceNew(ctx, workspace) if err == nil { log.Debug("Successfully created terraform workspace", "workspace", workspace, "component", component, "stack", stack) windowsFileDelay() return nil } // Log the creation failure before attempting select. log.Debug("Workspace creation failed, attempting select", "workspace", workspace, "error", err) // Check if this is a "workspace already exists" error. // If it's a different error (network, permissions), fail fast. if !isWorkspaceExistsError(err) { return errUtils.Build(errUtils.ErrTerraformWorkspaceOp). WithCause(err). WithExplanationf("Failed to create workspace '%s' for %s.", workspace, GetComponentInfo(component, stack)). Err() } // Workspace already exists, select it. // ... rest of select logicThis would require implementing
isWorkspaceExistsErrorto inspect the error for known patterns or types.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
pkg/terraform/output/spinner.go(1 hunks)pkg/terraform/output/workspace.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/terraform/output/spinner.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
All code must pass golangci-lint checks
Follow Go's error handling idioms: use meaningful error messages, wrap errors with context usingfmt.Errorf("context: %w", err), and consider using custom error types for domain-specific errors
Follow standard Go coding style: usegofmtandgoimportsto format code, prefer short descriptive variable names, use kebab-case for command-line flags, and snake_case for environment variables
Document all exported functions, types, and methods following Go's documentation conventions
Document complex logic with inline comments in Go code
Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults
Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging
**/*.go: NEVER use fmt.Fprintf(os.Stdout/Stderr) or fmt.Println(); use data.* or ui.* functions instead
All comments must end with periods (enforced by godot linter)
Organize imports in three groups separated by blank lines, sorted alphabetically: 1) Go stdlib, 2) 3rd-party (NOT cloudposse/atmos), 3) Atmos packages; maintain aliases: cfg, log, u, errUtils
Adddefer perf.Track(atmosConfig, "pkg.FuncName")()+ blank line to all public functions for performance tracking; use nil if no atmosConfig param
All errors MUST be wrapped using static errors defined in errors/errors.go; use errors.Join for combining multiple errors; use fmt.Errorf with %w for adding string context; use error builder for complex errors; use errors.Is() for error checking; NEVER use dynamic errors directly
Use go.uber.org/mock/mockgen with //go:generate directives for mock generation; never create manual mocks
Keep files small...
Files:
pkg/terraform/output/workspace.go
🧠 Learnings (7)
📓 Common learnings
Learnt from: aknysh
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_model_component.go:3-20
Timestamp: 2024-11-18T13:59:10.824Z
Learning: When replacing significant dependencies like `go-getter` that require extensive changes, prefer to address them in separate PRs.
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:233-235
Timestamp: 2024-10-31T19:25:41.298Z
Learning: When specifying color values in functions like `confirmDeleteTerraformLocal` in `internal/exec/terraform_clean.go`, avoid hardcoding color values. Instead, use predefined color constants or allow customization through configuration settings to improve accessibility and user experience across different terminals and themes.
Learnt from: aknysh
Repo: cloudposse/atmos PR: 759
File: internal/exec/terraform.go:366-368
Timestamp: 2024-11-02T15:35:09.958Z
Learning: In `internal/exec/terraform.go`, the workspace cleaning code under both the general execution path and within the `case "init":` block is intentionally duplicated because the code execution paths are different. The `.terraform/environment` file should be deleted before executing `terraform init` in both scenarios to ensure a clean state.
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/terraform_utils.go:40-213
Timestamp: 2024-12-03T05:18:49.169Z
Learning: In the context of the Atmos project, it's acceptable for functions like `execTerraformOutput` to remain as single functions if they perform a single purpose, such as retrieving Terraform outputs for a component in a stack, even if the function is lengthy.
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml:28-32
Timestamp: 2024-12-01T00:33:20.298Z
Learning: In `examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml`, `!exec atmos terraform output` is used in examples to demonstrate its usage, even though `!terraform.output` is the recommended approach according to the documentation.
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/terraform_utils.go:144-145
Timestamp: 2024-12-03T03:52:02.524Z
Learning: Avoid adding context timeouts to Terraform commands in `execTerraformOutput` because their execution time can vary from seconds to hours, and making it configurable would require redesigning the command interface.
Learnt from: haitham911
Repo: cloudposse/atmos PR: 1195
File: internal/exec/terraform_clean.go:99-99
Timestamp: 2025-04-26T15:54:10.506Z
Learning: The error variable `ErrRelPath` is defined in `internal/exec/terraform_clean_util.go` and is used across files in the `exec` package, including in `terraform_clean.go`. This is part of an approach to standardize error handling in the codebase.
Learnt from: haitham911
Repo: cloudposse/atmos PR: 1195
File: internal/exec/terraform_clean.go:99-99
Timestamp: 2025-04-26T15:54:10.506Z
Learning: The error variable `ErrRelPath` is defined in `internal/exec/terraform_clean_util.go` and is used across files in the `exec` package, including in `terraform_clean.go`. This is part of an approach to standardize error handling in the codebase.
📚 Learning: 2024-11-02T15:35:09.958Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 759
File: internal/exec/terraform.go:366-368
Timestamp: 2024-11-02T15:35:09.958Z
Learning: In `internal/exec/terraform.go`, the workspace cleaning code under both the general execution path and within the `case "init":` block is intentionally duplicated because the code execution paths are different. The `.terraform/environment` file should be deleted before executing `terraform init` in both scenarios to ensure a clean state.
Applied to files:
pkg/terraform/output/workspace.go
📚 Learning: 2024-11-12T03:16:02.910Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 775
File: internal/exec/template_funcs_component.go:157-159
Timestamp: 2024-11-12T03:16:02.910Z
Learning: In the Go code for `componentFunc` in `internal/exec/template_funcs_component.go`, the function `cleanTerraformWorkspace` does not return errors, and it's acceptable if the file does not exist. Therefore, error handling for `cleanTerraformWorkspace` is not needed.
Applied to files:
pkg/terraform/output/workspace.go
📚 Learning: 2025-12-13T06:10:25.156Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: internal/exec/workflow_utils.go:0-0
Timestamp: 2025-12-13T06:10:25.156Z
Learning: Atmos workflows: In internal/exec/workflow_utils.go ExecuteWorkflow, non-identity steps intentionally use baseWorkflowEnv, which is constructed from the parent environment with PATH modifications for the toolchain. Avoid appending os.Environ() again; prefer documenting this behavior and testing that standard environment variables are preserved.
Applied to files:
pkg/terraform/output/workspace.go
📚 Learning: 2025-12-13T06:10:13.688Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: errors/errors.go:184-203
Timestamp: 2025-12-13T06:10:13.688Z
Learning: cloudposse/atmos: For toolchain work, duplicate/unused error sentinels in errors/errors.go should be cleaned up in a separate refactor PR and not block feature PRs; canonical toolchain sentinels live under toolchain/registry with re-exports in toolchain/errors.go.
Applied to files:
pkg/terraform/output/workspace.go
📚 Learning: 2025-06-02T14:12:02.710Z
Learnt from: milldr
Repo: cloudposse/atmos PR: 1229
File: internal/exec/workflow_test.go:0-0
Timestamp: 2025-06-02T14:12:02.710Z
Learning: In the atmos codebase, workflow error handling was refactored to use `PrintErrorMarkdown` followed by returning specific error variables (like `ErrWorkflowNoSteps`, `ErrInvalidFromStep`, `ErrInvalidWorkflowStepType`, `ErrWorkflowStepFailed`) instead of `PrintErrorMarkdownAndExit`. This pattern allows proper error testing without the function terminating the process with `os.Exit`, enabling unit tests to assert on error conditions while maintaining excellent user-facing error formatting.
Applied to files:
pkg/terraform/output/workspace.go
📚 Learning: 2024-11-24T19:13:10.287Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:407-416
Timestamp: 2024-11-24T19:13:10.287Z
Learning: In `internal/exec/terraform_clean.go`, when `getStackTerraformStateFolder` returns an error in the `handleCleanSubCommand` function, the error is logged, and the process continues without returning the error.
Applied to files:
pkg/terraform/output/workspace.go
🧬 Code graph analysis (1)
pkg/terraform/output/workspace.go (7)
pkg/schema/schema.go (1)
AtmosConfiguration(54-99)pkg/terraform/output/executor.go (1)
TerraformRunner(39-54)pkg/perf/perf.go (1)
Track(121-138)pkg/logger/log.go (1)
Debug(24-26)errors/builder.go (1)
Build(24-37)errors/errors.go (1)
ErrTerraformWorkspaceOp(115-115)pkg/terraform/output/config.go (1)
GetComponentInfo(185-189)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
🔇 Additional comments (1)
pkg/terraform/output/workspace.go (1)
71-72: Good addition of debug log before fallback.This debug log addresses the feedback from the previous review. Logging the workspace creation failure before attempting to select the existing workspace helps with troubleshooting.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1885 +/- ##
==========================================
+ Coverage 73.77% 73.94% +0.16%
==========================================
Files 746 756 +10
Lines 67889 68573 +684
==========================================
+ Hits 50085 50703 +618
- Misses 14395 14451 +56
- Partials 3409 3419 +10
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
osterman
force-pushed
the
osterman/tf-output-format
branch
from
69211ce to
8f0c520
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
CLAUDE.md (1)
22-32: Solid addition addressing concurrent session workflow.This new section fills a real gap—developers working in parallel git worktrees need clear guidance on respecting each other's work. The bullet points are actionable and the "why this matters" rationale makes the stakes clear.
One minor note: line 32 uses straight quotes around
"unknown". For consistency with professional technical documentation, consider using typographic quotes instead.pkg/terraform/output/spinner.go (1)
77-97: Add blank line afterdefer perf.Track()for consistency.Per coding guidelines, performance tracking should be followed by a blank line. Lines 79 and 93 are missing this formatting.
🔎 Suggested formatting fix
For
RunSpinner:func RunSpinner(p *tea.Program, spinnerChan chan struct{}, message string) { defer perf.Track(nil, "output.RunSpinner")() + go func() {For
StopSpinner:func StopSpinner(p *tea.Program, spinnerChan chan struct{}) { defer perf.Track(nil, "output.StopSpinner")() + p.Quit()pkg/terraform/output/environment.go (1)
78-80: Consider type handling for env values.Using
fmt.Sprintf("%v", v)works for basic types but may produce unexpected results for complex types (maps, slices). Ifconfig.Envcould contain non-string values, explicit type checking would be safer.Optional: Explicit string conversion
for k, v := range config.Env { - environMap[k] = fmt.Sprintf("%v", v) + switch val := v.(type) { + case string: + environMap[k] = val + default: + environMap[k] = fmt.Sprintf("%v", v) + } }pkg/terraform/output/output.go (1)
100-114: Missing error context in WriteToFile.Both error return paths should wrap errors with context per coding guidelines.
🔎 Proposed fix to add error context
func WriteToFile(filePath string, content string) error { defer perf.Track(nil, "output.WriteToFile")() // Open file in append mode, create if doesn't exist. f, err := os.OpenFile(filePath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, DefaultFileMode) if err != nil { - return err + return fmt.Errorf("failed to open file %s: %w", filePath, err) } defer f.Close() _, err = f.WriteString(content) - return err + if err != nil { + return fmt.Errorf("failed to write to file %s: %w", filePath, err) + } + return nil }
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
osterman
force-pushed
the
osterman/tf-output-format
branch
from
8816fef to
c0cebba
Compare
…utput Add comprehensive unit tests for previously untested code in the terraform output package: - backend_test.go: Tests for generateBackendConfig, generateProviderOverrides, GenerateBackendIfNeeded, and GenerateProvidersIfNeeded functions - environment_test.go: Tests for SetupEnvironment including prohibited variable filtering, AWS auth context merging, and component env overrides These tests increase package coverage from ~70% to ~80%. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
osterman
force-pushed
the
osterman/tf-output-format
branch
from
bd92956 to
07a281c
Compare
|
Warning Release Documentation RequiredThis PR is labeled
|
Add milestone documenting support for exporting terraform outputs in multiple formats (JSON, YAML, HCL, env, dotenv, bash, CSV, TSV) with options for uppercase keys and nested value flattening. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
osterman
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
CLAUDE.md (1)
22-32: Consider using smart quotes for consistency with modern documentation standards.Line 32 uses straight quotes around "unknown", while modern documentation typically uses smart (curly) quotes. This is a minor stylistic improvement for professional polish.
🔎 Proposed fix for typography
- **When you see unfamiliar files**, assume another session created them - ask the user what to do + **When you see unfamiliar files**, assume another session created them - ask the user what to docmd/terraform/output.go (1)
12-12: Consider using a more descriptive alias forinternal/exec.The alias
eis terse. Per coding guidelines, common aliases includecfg,log,u,errUtils. A clearer alias likeexecortfexecwould improve readability.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
CLAUDE.mdcmd/terraform/output.goerrors/errors.gointernal/exec/component_describer_adapter.gointernal/exec/template_funcs_component.gointernal/exec/terraform_output_getter.gointernal/exec/terraform_output_utils.gointernal/exec/terraform_output_utils_integration_test.gointernal/exec/terraform_output_utils_other.gointernal/exec/terraform_output_utils_other_test.gointernal/exec/terraform_output_utils_windows_test.gointernal/exec/terraform_state_utils.gointernal/exec/terraform_utils.gopkg/hooks/store_cmd.go
💤 Files with no reviewable changes (5)
- internal/exec/terraform_output_utils.go
- internal/exec/terraform_output_utils_windows_test.go
- internal/exec/terraform_output_utils_other.go
- internal/exec/terraform_output_utils_integration_test.go
- internal/exec/terraform_output_utils_other_test.go
🚧 Files skipped from review as they are similar to previous changes (5)
- internal/exec/template_funcs_component.go
- internal/exec/terraform_state_utils.go
- pkg/hooks/store_cmd.go
- errors/errors.go
- internal/exec/terraform_utils.go
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
All code must pass golangci-lint checks
Follow Go's error handling idioms: use meaningful error messages, wrap errors with context usingfmt.Errorf("context: %w", err), and consider using custom error types for domain-specific errors
Follow standard Go coding style: usegofmtandgoimportsto format code, prefer short descriptive variable names, use kebab-case for command-line flags, and snake_case for environment variables
Document all exported functions, types, and methods following Go's documentation conventions
Document complex logic with inline comments in Go code
Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults
Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging
**/*.go: All comments must end with periods (enforced bygodotlinter)
Never delete existing comments without a very strong reason; preserve helpful comments explaining why/how/what/where, and update comments to match code when refactoring
Organize imports in three groups separated by blank lines, sorted alphabetically: (1) Go stdlib, (2) 3rd-party (NOT cloudposse/atmos), (3) Atmos packages. Maintain aliases:cfg,log,u,errUtils
Useflags.NewStandardParser()for command-specific flags; NEVER callviper.BindEnv()orviper.BindPFlag()directly (enforced by Forbidigo)
All errors MUST be wrapped using static errors defined inerrors/errors.go; useerrors.Joinfor combining multiple errors,fmt.Errorfwith%wfor adding string context, error builder for complex errors, anderrors.Is()for error checking. Never use dynamic errors directly
Define interfaces for all major funct...
Files:
internal/exec/component_describer_adapter.gointernal/exec/terraform_output_getter.gocmd/terraform/output.go
internal/exec/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
New templates should support Go templating with
FuncMap()frominternal/exec/template_funcs.go
Files:
internal/exec/component_describer_adapter.gointernal/exec/terraform_output_getter.go
cmd/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/**/*.go: Use Cobra's recommended command structure with a root command and subcommands, implementing each command in a separate file undercmd/directory
Provide comprehensive help text for all commands and flags, include examples in command help, and follow Go's documentation conventions in Cobra command definitions
Provide meaningful feedback to users and include progress indicators for long-running operations in CLI commands
cmd/**/*.go: Adddefer perf.Track(atmosConfig, "pkg.FuncName")()+ blank line to all public functions, except trivial getters/setters, command constructors, simple factory functions, and functions that only delegate to another tracked function
Commands MUST use the command registry pattern viaCommandProviderinterface; seedocs/prd/command-registry-pattern.mdandcmd/internal/registry.go
Embed examples fromcmd/markdown/*_usage.mdusing//go:embedand render withutils.PrintfMarkdown()
Files:
cmd/terraform/output.go
🧠 Learnings (31)
📓 Common learnings
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/terraform_utils.go:40-213
Timestamp: 2024-12-03T05:18:49.169Z
Learning: In the context of the Atmos project, it's acceptable for functions like `execTerraformOutput` to remain as single functions if they perform a single purpose, such as retrieving Terraform outputs for a component in a stack, even if the function is lengthy.
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/terraform_utils.go:144-145
Timestamp: 2024-12-03T03:52:02.524Z
Learning: Avoid adding context timeouts to Terraform commands in `execTerraformOutput` because their execution time can vary from seconds to hours, and making it configurable would require redesigning the command interface.
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml:28-32
Timestamp: 2024-12-01T00:33:20.298Z
Learning: In `examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml`, `!exec atmos terraform output` is used in examples to demonstrate its usage, even though `!terraform.output` is the recommended approach according to the documentation.
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/terraform_utils.go:145-146
Timestamp: 2024-12-03T05:29:07.718Z
Learning: In the Atmos project, a 5-minute timeout in the `execTerraformOutput` function is acceptable for retrieving `terraform output` for a component in a stack.
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1327
File: cmd/terraform.go:111-117
Timestamp: 2025-06-23T02:14:30.937Z
Learning: In cmd/terraform.go, flags for the DescribeAffected function are added dynamically at runtime when info.Affected is true. This is intentional to avoid exposing internal flags like "file", "format", "verbose", "include-spacelift-admin-stacks", "include-settings", and "upload" in the terraform command interface, while still providing them for the shared DescribeAffected function used by both `atmos describe affected` and `atmos terraform apply --affected`.
Applied to files:
internal/exec/component_describer_adapter.gocmd/terraform/output.go
📚 Learning: 2026-01-01T18:25:25.942Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T18:25:25.942Z
Learning: Applies to internal/exec/**/*.go : New templates should support Go templating with `FuncMap()` from `internal/exec/template_funcs.go`
Applied to files:
internal/exec/component_describer_adapter.go
📚 Learning: 2024-11-12T03:16:02.910Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 775
File: internal/exec/template_funcs_component.go:157-159
Timestamp: 2024-11-12T03:16:02.910Z
Learning: In the Go code for `componentFunc` in `internal/exec/template_funcs_component.go`, the function `cleanTerraformWorkspace` does not return errors, and it's acceptable if the file does not exist. Therefore, error handling for `cleanTerraformWorkspace` is not needed.
Applied to files:
internal/exec/component_describer_adapter.go
📚 Learning: 2024-11-02T15:35:09.958Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 759
File: internal/exec/terraform.go:366-368
Timestamp: 2024-11-02T15:35:09.958Z
Learning: In `internal/exec/terraform.go`, the workspace cleaning code under both the general execution path and within the `case "init":` block is intentionally duplicated because the code execution paths are different. The `.terraform/environment` file should be deleted before executing `terraform init` in both scenarios to ensure a clean state.
Applied to files:
internal/exec/component_describer_adapter.go
📚 Learning: 2024-11-13T21:37:07.852Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 764
File: internal/exec/describe_stacks.go:289-295
Timestamp: 2024-11-13T21:37:07.852Z
Learning: In the `internal/exec/describe_stacks.go` file of the `atmos` project written in Go, avoid extracting the stack name handling logic into a helper function within the `ExecuteDescribeStacks` method, even if the logic appears duplicated.
Applied to files:
internal/exec/component_describer_adapter.go
📚 Learning: 2025-08-16T23:32:40.412Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1405
File: internal/exec/describe_dependents_test.go:455-456
Timestamp: 2025-08-16T23:32:40.412Z
Learning: In the cloudposse/atmos Go codebase, `InitCliConfig` returns a `schema.AtmosConfiguration` value (not a pointer), while `ExecuteDescribeDependents` expects a `*schema.AtmosConfiguration` pointer parameter. Therefore, when passing the result of `InitCliConfig` to `ExecuteDescribeDependents`, use `&atmosConfig` to pass the address of the value.
Applied to files:
internal/exec/component_describer_adapter.go
📚 Learning: 2025-12-21T04:10:29.030Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1891
File: internal/exec/describe_affected.go:468-468
Timestamp: 2025-12-21T04:10:29.030Z
Learning: In Go, package-level declarations (constants, variables, types, and functions) are visible to all files in the same package without imports. During reviews in cloudposse/atmos (and similar Go codebases), before suggesting to declare a new identifier, first check if it already exists in another file of the same package. If it exists, you can avoid adding a new declaration; if not, proceed with a proper package-level declaration.
Applied to files:
internal/exec/component_describer_adapter.gointernal/exec/terraform_output_getter.gocmd/terraform/output.go
📚 Learning: 2024-12-03T05:18:49.169Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/terraform_utils.go:40-213
Timestamp: 2024-12-03T05:18:49.169Z
Learning: In the context of the Atmos project, it's acceptable for functions like `execTerraformOutput` to remain as single functions if they perform a single purpose, such as retrieving Terraform outputs for a component in a stack, even if the function is lengthy.
Applied to files:
internal/exec/terraform_output_getter.go
📚 Learning: 2024-12-01T00:33:20.298Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml:28-32
Timestamp: 2024-12-01T00:33:20.298Z
Learning: In `examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml`, `!exec atmos terraform output` is used in examples to demonstrate its usage, even though `!terraform.output` is the recommended approach according to the documentation.
Applied to files:
internal/exec/terraform_output_getter.go
📚 Learning: 2025-12-24T04:29:23.938Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1891
File: pkg/ci/terraform/templates/apply.md:17-17
Timestamp: 2025-12-24T04:29:23.938Z
Learning: In the cloudposse/atmos repository, Terraform CI templates (pkg/ci/terraform/templates/*.md) are rendered using TerraformTemplateContext (defined in pkg/ci/terraform/context.go), not the base ci.TemplateContext. TerraformTemplateContext provides top-level fields: .Resources (ci.ResourceCounts), .HasChanges() method, and .HasDestroy field, which are correctly accessed directly in templates without a .Result prefix.
Applied to files:
internal/exec/terraform_output_getter.go
📚 Learning: 2024-12-03T05:29:07.718Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/terraform_utils.go:145-146
Timestamp: 2024-12-03T05:29:07.718Z
Learning: In the Atmos project, a 5-minute timeout in the `execTerraformOutput` function is acceptable for retrieving `terraform output` for a component in a stack.
Applied to files:
internal/exec/terraform_output_getter.go
📚 Learning: 2026-01-01T18:25:25.942Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T18:25:25.942Z
Learning: Stack pipeline: Load atmos.yaml → process imports/inheritance → apply overrides → render templates → generate config. Templates use Go templates + Gomplate with `atmos.Component()`, `!terraform.state`, `!terraform.output`, store integration
Applied to files:
internal/exec/terraform_output_getter.gocmd/terraform/output.go
📚 Learning: 2025-10-10T23:51:36.597Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1599
File: internal/exec/terraform.go:394-402
Timestamp: 2025-10-10T23:51:36.597Z
Learning: In Atmos (internal/exec/terraform.go), when adding OpenTofu-specific flags like `--var-file` for `init`, do not gate them based on command name (e.g., checking if `info.Command == "tofu"` or `info.Command == "opentofu"`) because command names don't reliably indicate the actual binary being executed (symlinks, aliases). Instead, document the OpenTofu requirement in code comments and documentation, trusting users who enable the feature (e.g., `PassVars`) to ensure their terraform command points to an OpenTofu binary.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2026-01-01T18:25:25.942Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T18:25:25.942Z
Learning: Applies to cmd/**/*.go : Add `defer perf.Track(atmosConfig, "pkg.FuncName")()` + blank line to all public functions, except trivial getters/setters, command constructors, simple factory functions, and functions that only delegate to another tracked function
Applied to files:
cmd/terraform/output.go
📚 Learning: 2025-01-09T22:37:01.004Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/terraform_commands.go:260-265
Timestamp: 2025-01-09T22:37:01.004Z
Learning: In the terraform commands implementation (cmd/terraform_commands.go), the direct use of `os.Args[2:]` for argument handling is intentionally preserved to avoid extensive refactoring. While it could be improved to use cobra's argument parsing, such changes should be handled in a dedicated PR to maintain focus and minimize risk.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2025-10-07T00:25:16.333Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1498
File: website/src/components/Screengrabs/atmos-terraform-metadata--help.html:25-55
Timestamp: 2025-10-07T00:25:16.333Z
Learning: In Atmos CLI, subcommands inherit flags from their parent commands via Cobra's command inheritance. For example, `atmos terraform metadata --help` shows `--affected` and related flags inherited from the parent `terraform` command (defined in cmd/terraform.go), even though the metadata subcommand doesn't explicitly define these flags. This is expected Cobra behavior and auto-generated help screengrabs accurately reflect this inheritance.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2024-12-17T07:08:41.288Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 863
File: internal/exec/yaml_func_terraform_output.go:34-38
Timestamp: 2024-12-17T07:08:41.288Z
Learning: In the `processTagTerraformOutput` function within `internal/exec/yaml_func_terraform_output.go`, parameters are separated by spaces and do not contain spaces. Therefore, using `strings.Fields()` for parsing is acceptable, and there's no need to handle parameters with spaces.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2025-09-30T19:03:50.738Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 1560
File: pkg/utils/string_utils.go:43-64
Timestamp: 2025-09-30T19:03:50.738Z
Learning: In the Atmos codebase, YAML tags like !terraform.output rely on positional arguments, so the SplitStringByDelimiter function in pkg/utils/string_utils.go must preserve empty strings (even after trimming quotes) to maintain the correct number of positional arguments. Filtering out empty values after trimming would collapse the array and break these function calls.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2024-10-28T01:51:30.811Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:329-332
Timestamp: 2024-10-28T01:51:30.811Z
Learning: In the Atmos Go code, when deleting directories or handling file paths (e.g., in `terraform_clean.go`), always resolve the absolute path using `filepath.Abs` and use the logger `u.LogWarning` for logging messages instead of using `fmt.Printf`.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2025-12-13T03:21:27.620Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1813
File: cmd/terraform/shell.go:28-73
Timestamp: 2025-12-13T03:21:27.620Z
Learning: In Atmos, when initializing CLI config via cfg.InitCliConfig, always first populate the ConfigAndStacksInfo struct with global flag values by calling flags.ParseGlobalFlags(cmd, v) before LoadConfig. LoadConfig (pkg/config/load.go) reads config selection fields (AtmosConfigFilesFromArg, AtmosConfigDirsFromArg, BasePath, ProfilesFromArg) from the ConfigAndStacksInfo struct, not from Viper. Passing an empty struct causes the --base-path, --config, --config-path, and --profile flags to be ignored. Recommended pattern: parse flags → populate struct → call InitCliConfig. See cmd/terraform/plan_diff.go for reference implementation.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2025-02-18T13:18:53.146Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1068
File: cmd/vendor_pull.go:31-31
Timestamp: 2025-02-18T13:18:53.146Z
Learning: Error checking is not required for cobra.Command.RegisterFlagCompletionFunc calls as these are static configurations done at init time.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2025-11-10T03:03:31.505Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 0
File: :0-0
Timestamp: 2025-11-10T03:03:31.505Z
Learning: In the Atmos codebase, commands using the `StandardParser` flag pattern (from pkg/flags) do NOT need explicit `viper.BindPFlag()` calls in their code. The StandardParser encapsulates flag binding internally: flags are registered via `parser.RegisterFlags(cmd)` in init(), and bound via `parser.BindFlagsToViper(cmd, v)` in RunE, which internally calls viper.BindPFlag for each flag. This pattern is used throughout Atmos (e.g., cmd/toolchain/get.go, cmd/toolchain/info.go, cmd/toolchain/install.go, cmd/toolchain/path.go). Do not flag missing viper.BindPFlag calls when StandardParser is used.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2024-12-05T22:33:40.955Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 820
File: cmd/list_components.go:53-54
Timestamp: 2024-12-05T22:33:40.955Z
Learning: In the Atmos CLI Go codebase, using `u.LogErrorAndExit` within completion functions is acceptable because it logs the error and exits the command execution.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2025-12-13T04:37:25.223Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: cmd/root.go:0-0
Timestamp: 2025-12-13T04:37:25.223Z
Learning: In Atmos cmd/root.go Execute(), after cfg.InitCliConfig, we must call both toolchainCmd.SetAtmosConfig(&atmosConfig) and toolchain.SetAtmosConfig(&atmosConfig) so the CLI wrapper and the toolchain package receive configuration; missing either can cause nil-pointer panics in toolchain path resolution.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2025-09-05T14:57:37.360Z
Learnt from: RoseSecurity
Repo: cloudposse/atmos PR: 1448
File: cmd/ansible.go:26-28
Timestamp: 2025-09-05T14:57:37.360Z
Learning: The Atmos codebase uses a consistent pattern for commands that delegate to external tools: `PersistentFlags().Bool("", false, doubleDashHint)` where doubleDashHint provides help text about using double dashes to separate Atmos options from native command arguments. This pattern is used across terraform, packer, helmfile, atlantis, aws, and ansible commands.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2025-12-13T04:37:40.435Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: cmd/toolchain/get.go:23-40
Timestamp: 2025-12-13T04:37:40.435Z
Learning: In Go CLI command files using Cobra, constrain the subcommand to accept at most one positional argument (MaximumNArgs(1)) so it supports both listing all items (zero args) and fetching a specific item (one arg). Define and parse flags with a standard parser (e.g., flags.NewStandardParser()) and avoid binding flags to Viper (no viper.BindEnv/BindPFlag). This promotes explicit argument handling and predictable flag behavior across command files.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2025-11-07T14:52:55.217Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1761
File: docs/prd/claude-agent-architecture.md:331-439
Timestamp: 2025-11-07T14:52:55.217Z
Learning: In the cloudposse/atmos repository, Claude agents are used as interactive tools, not in automated/headless CI/CD contexts. Agent documentation and patterns should assume synchronous human interaction.
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-01T18:25:25.942Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T18:25:25.942Z
Learning: Never run destructive git commands (`git reset`, `git checkout HEAD --`, `git clean -fd`, `git stash drop`) without explicit user confirmation
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-01T18:25:25.942Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T18:25:25.942Z
Learning: Use git worktrees for parallel development; always stay within the current working directory; use relative paths or paths under the current working directory; never escape to parent directories; never hardcode absolute paths like `/Users/*/atmos/`
Applied to files:
CLAUDE.md
🧬 Code graph analysis (3)
internal/exec/component_describer_adapter.go (4)
pkg/terraform/output/executor.go (3)
DescribeComponentParams(66-73)NewExecutor(160-171)WithStaticRemoteStateGetter(151-157)internal/exec/describe_component.go (1)
ExecuteDescribeComponentParams(203-210)internal/exec/terraform_remote_state_backend_utils.go (1)
GetComponentRemoteStateBackendStaticType(9-26)pkg/terraform/output/get.go (1)
SetDefaultExecutor(14-18)
internal/exec/terraform_output_getter.go (3)
pkg/terraform/output/get.go (2)
GetOutput(105-121)GetComponentOutputs(39-52)pkg/hooks/store_cmd.go (1)
TerraformOutputGetter(21-29)pkg/terraform/output/executor.go (1)
GetStaticRemoteStateOutput(610-621)
cmd/terraform/output.go (4)
pkg/terraform/output/output.go (4)
SupportedFormats(35-35)FormatOptions(44-53)Format(13-13)WriteToFile(102-114)errors/errors.go (3)
ErrInvalidArgumentError(648-648)ErrInitializeCLIConfig(551-551)ErrTerraformOutputFailed(113-113)pkg/terraform/output/get.go (1)
GetComponentOutputs(39-52)pkg/terraform/output/format.go (2)
FormatOutputsWithOptions(32-43)FormatSingleValueWithOptions(140-155)
🪛 LanguageTool
CLAUDE.md
[typographical] ~32-~32: Consider using typographic quotation marks here.
Context: ...ifferent aspects of a feature. Deleting "unknown" files destroys that work. ## Essential...
(EN_QUOTES)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Hadolint
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Summary
🔇 Additional comments (16)
cmd/terraform/output.go (9)
37-50: Clean routing logic.The conditional routing based on
--formatpresence is clear and maintains backward compatibility. Native terraform behavior preserved when format is empty.
53-65: Solid orchestration flow.Good structure: validate → prepare → execute. Error propagation is clean. Using
nilforperf.Trackhere is fine sinceatmosConfigisn't available yet.
67-76: Error handling follows guidelines.Good use of
errUtils.Buildwith static error, explanation, and hint. User gets actionable feedback.
78-107: Context preparation follows the recommended pattern.Per learning, this correctly: parses global flags → populates
ConfigAndStacksInfostruct → callsInitCliConfig. Error wrapping uses static errors.
143-144: Verify append behavior is intentional forWriteToFile.Per the external snippet,
tfoutput.WriteToFileopens withO_APPEND. This will append to existing files rather than overwrite. If the user runs the command multiple times with the same--output-file, content accumulates. Confirm this is the intended behavior.
149-158: Clean extraction logic.Simple, effective. First positional arg (non-flag) is treated as the output name. Empty string fallback is appropriate.
160-170: Helpful error messaging.When output not found, the hint tells the user exactly how to list available outputs. Good UX.
189-191: Silent error handling is acceptable here.Per learnings,
RegisterFlagCompletionFuncerrors don't require checking since these are static init-time configurations. The_ = errpattern is fine.
184-186: No action needed.The
BindToViper()call ininit()andBindFlagsToViper()inRunEare complementary, not redundant. TheFlagParserinterface specifies both methods:BindToViper()sets up environment variable binding after flag registration, whileBindFlagsToViper()handles runtime flag binding from the command context. This two-phase approach is the documented, intentional pattern used consistently across all terraform subcommands (terraform.go,varfile.go,workspace.go, etc.) and is fully encapsulated withinStandardParser—no directviper.BindPFlagcalls bypass the pattern.internal/exec/component_describer_adapter.go (3)
34-42: Clean adapter implementation.The
staticRemoteStateGetterAdaptercorrectly implements the interface with proper delegation and performance tracking.
44-58: Proper initialization using functional options pattern.The initialization function correctly uses the functional options pattern to configure the executor with both adapters, then registers it as the default. Implementation follows Go best practices.
60-63: Auto-initialization via init() is appropriate here.The
init()function ensures the Terraform output executor is initialized when the package is imported. This is a standard Go pattern for package-level setup, and sinceInitTerraformOutputExecutoris exported, tests can re-initialize if needed.internal/exec/terraform_output_getter.go (4)
27-43: Proper delegation to new package.The
defaultOutputGettercorrectly delegates to the newtfoutput.GetOutputfunction while maintaining the interface contract and performance tracking.
48-65: Good backward compatibility wrapper.The wrapper maintains the existing API surface while delegating to the new package. Clear documentation about its purpose and recommendation for new code.
67-79: Consistent backward compatibility pattern.This wrapper follows the same clean pattern as
GetTerraformOutput, maintaining API compatibility while delegating to the new package.
81-94: Final wrapper maintains consistency.The
GetStaticRemoteStateOutputwrapper completes the set of backward-compatible functions, following the same clean delegation pattern with proper documentation.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
pkg/terraform/output/spinner_test.go (5)
18-26: Initialize the spinner field for realistic testing.The
modelSpinneris created without initializing itsspinnerfield. While this test passes, it doesn't reflect the actual runtime behavior wherespinner.New()creates the spinner model. Consider initializing it properly to ensure the test exercises real code paths.🔎 Suggested initialization
+ s := spinner.New() model := modelSpinner{ + spinner: s, message: "Test message", }
58-60: Initialize the spinner field.Same initialization issue as noted in
TestModelSpinner_Init. The spinner field should be properly initialized to match runtime behavior.
85-96: Test logic is sound, but initialize spinner field.The assertions correctly verify that unknown messages don't produce commands. However, apply the same initialization improvement noted in other tests.
98-106: Initialize spinner field for complete view rendering.The View method likely uses the
spinnerfield to render the full output. Initializing it properly would ensure the test validates the complete rendering path.
28-83: Consider adding coverage for spinner tick messages.The tests cover key input handling but don't test
spinner.TickMsg, which drives the spinner animation. Adding a test case for this message type would improve coverage of the Update method's behavior.🔎 Example test structure
func TestModelSpinner_Update_TickMsg(t *testing.T) { s := spinner.New() model := modelSpinner{ spinner: s, message: "Test", } // Simulate a tick message tickMsg := s.Tick() newModel, cmd := model.Update(tickMsg) assert.NotNil(t, newModel) assert.NotNil(t, cmd) // Should return next tick command }pkg/terraform/output/output_test.go (1)
242-247: Consider a more reliable invalid path test.The hardcoded path
/nonexistent/path/that/does/not/exist/file.txtassumes this location is inaccessible, but this could be flaky if the directory exists or in different environments.Consider using a path within
t.TempDir()with invalid permissions, or attempting to write to a directory (not a file):tmpDir := t.TempDir() invalidPath := filepath.Join(tmpDir, "dir_as_file", "file.txt") // Create a file where a directory is expected require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "dir_as_file"), []byte("block"), 0644)) err := WriteToFile(invalidPath, "content") assert.Error(t, err)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
cmd/terraform/output_test.gointernal/exec/terraform_output_utils_test.gopkg/terraform/output/get_test.gopkg/terraform/output/output_test.gopkg/terraform/output/spinner_test.gopkg/terraform/output/workspace_test.go
💤 Files with no reviewable changes (1)
- internal/exec/terraform_output_utils_test.go
🧰 Additional context used
📓 Path-based instructions (4)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
All code must pass golangci-lint checks
Follow Go's error handling idioms: use meaningful error messages, wrap errors with context usingfmt.Errorf("context: %w", err), and consider using custom error types for domain-specific errors
Follow standard Go coding style: usegofmtandgoimportsto format code, prefer short descriptive variable names, use kebab-case for command-line flags, and snake_case for environment variables
Document all exported functions, types, and methods following Go's documentation conventions
Document complex logic with inline comments in Go code
Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults
Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging
**/*.go: All comments must end with periods (enforced bygodotlinter)
Never delete existing comments without a very strong reason; preserve helpful comments explaining why/how/what/where, and update comments to match code when refactoring
Organize imports in three groups separated by blank lines, sorted alphabetically: (1) Go stdlib, (2) 3rd-party (NOT cloudposse/atmos), (3) Atmos packages. Maintain aliases:cfg,log,u,errUtils
Useflags.NewStandardParser()for command-specific flags; NEVER callviper.BindEnv()orviper.BindPFlag()directly (enforced by Forbidigo)
All errors MUST be wrapped using static errors defined inerrors/errors.go; useerrors.Joinfor combining multiple errors,fmt.Errorfwith%wfor adding string context, error builder for complex errors, anderrors.Is()for error checking. Never use dynamic errors directly
Define interfaces for all major funct...
Files:
pkg/terraform/output/get_test.gopkg/terraform/output/spinner_test.gopkg/terraform/output/workspace_test.gopkg/terraform/output/output_test.gocmd/terraform/output_test.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Use table-driven tests for testing multiple scenarios in Go
Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
**/*_test.go: Usecmd.NewTestKit(t)for cmd tests to auto-clean RootCmd state (flags, args)
Test behavior, not implementation; never test stub functions; avoid tautological tests; useerrors.Is()for error checking; remove always-skipped tests
Prefer unit tests with mocks over integration tests; use interfaces and dependency injection for testability; use table-driven tests for comprehensive coverage; target >80% coverage; skip tests gracefully with helpers fromtests/test_preconditions.go
Never manually edit golden snapshot files undertests/test-cases/ortests/testdata/; always use-regenerate-snapshotsflag. Never use pipe redirection when running tests as it breaks TTY detection
Files:
pkg/terraform/output/get_test.gopkg/terraform/output/spinner_test.gopkg/terraform/output/workspace_test.gopkg/terraform/output/output_test.gocmd/terraform/output_test.go
pkg/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
pkg/**/*.go: Adddefer perf.Track(atmosConfig, "pkg.FuncName")()+ blank line to all public functions, except trivial getters/setters, command constructors, simple factory functions, and functions that only delegate to another tracked function. Usenilif no atmosConfig param
Avoid adding new functions topkg/utils/; create purpose-built packages for new functionality (e.g.,pkg/store/,pkg/git/,pkg/pro/,pkg/filesystem/)
Files:
pkg/terraform/output/get_test.gopkg/terraform/output/spinner_test.gopkg/terraform/output/workspace_test.gopkg/terraform/output/output_test.go
cmd/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/**/*.go: Use Cobra's recommended command structure with a root command and subcommands, implementing each command in a separate file undercmd/directory
Provide comprehensive help text for all commands and flags, include examples in command help, and follow Go's documentation conventions in Cobra command definitions
Provide meaningful feedback to users and include progress indicators for long-running operations in CLI commands
cmd/**/*.go: Adddefer perf.Track(atmosConfig, "pkg.FuncName")()+ blank line to all public functions, except trivial getters/setters, command constructors, simple factory functions, and functions that only delegate to another tracked function
Commands MUST use the command registry pattern viaCommandProviderinterface; seedocs/prd/command-registry-pattern.mdandcmd/internal/registry.go
Embed examples fromcmd/markdown/*_usage.mdusing//go:embedand render withutils.PrintfMarkdown()
Files:
cmd/terraform/output_test.go
🧠 Learnings (32)
📓 Common learnings
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/terraform_utils.go:40-213
Timestamp: 2024-12-03T05:18:49.169Z
Learning: In the context of the Atmos project, it's acceptable for functions like `execTerraformOutput` to remain as single functions if they perform a single purpose, such as retrieving Terraform outputs for a component in a stack, even if the function is lengthy.
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml:28-32
Timestamp: 2024-12-01T00:33:20.298Z
Learning: In `examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml`, `!exec atmos terraform output` is used in examples to demonstrate its usage, even though `!terraform.output` is the recommended approach according to the documentation.
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/terraform_utils.go:144-145
Timestamp: 2024-12-03T03:52:02.524Z
Learning: Avoid adding context timeouts to Terraform commands in `execTerraformOutput` because their execution time can vary from seconds to hours, and making it configurable would require redesigning the command interface.
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
Applied to files:
pkg/terraform/output/get_test.gopkg/terraform/output/spinner_test.gopkg/terraform/output/workspace_test.gopkg/terraform/output/output_test.gocmd/terraform/output_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Applied to files:
pkg/terraform/output/get_test.gopkg/terraform/output/spinner_test.gopkg/terraform/output/workspace_test.gopkg/terraform/output/output_test.gocmd/terraform/output_test.go
📚 Learning: 2025-12-10T18:32:51.237Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1808
File: cmd/terraform/backend/backend_delete_test.go:9-23
Timestamp: 2025-12-10T18:32:51.237Z
Learning: In cmd subpackages (e.g., cmd/terraform/backend/), tests cannot use cmd.NewTestKit(t) due to Go's test visibility rules (NewTestKit is in a parent package test file). These tests only need TestKit if they execute commands through RootCmd or modify RootCmd state. Structural tests that only verify command structure/flags without touching RootCmd don't require TestKit cleanup.
Applied to files:
pkg/terraform/output/get_test.gopkg/terraform/output/workspace_test.gopkg/terraform/output/output_test.go
📚 Learning: 2026-01-01T18:25:25.942Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T18:25:25.942Z
Learning: Applies to **/*_test.go : Prefer unit tests with mocks over integration tests; use interfaces and dependency injection for testability; use table-driven tests for comprehensive coverage; target >80% coverage; skip tests gracefully with helpers from `tests/test_preconditions.go`
Applied to files:
pkg/terraform/output/get_test.gopkg/terraform/output/output_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Use table-driven tests for testing multiple scenarios in Go
Applied to files:
pkg/terraform/output/get_test.gopkg/terraform/output/workspace_test.gopkg/terraform/output/output_test.gocmd/terraform/output_test.go
📚 Learning: 2026-01-01T18:25:25.942Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T18:25:25.942Z
Learning: Applies to **/*_test.go : Test behavior, not implementation; never test stub functions; avoid tautological tests; use `errors.Is()` for error checking; remove always-skipped tests
Applied to files:
pkg/terraform/output/get_test.gopkg/terraform/output/workspace_test.gopkg/terraform/output/output_test.gocmd/terraform/output_test.go
📚 Learning: 2025-11-11T03:47:59.576Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/which_test.go:166-223
Timestamp: 2025-11-11T03:47:59.576Z
Learning: In the cloudposse/atmos repo, tests that manipulate environment variables should use testing.T.Setenv for automatic setup/teardown instead of os.Setenv/Unsetenv.
Applied to files:
pkg/terraform/output/get_test.gopkg/terraform/output/workspace_test.go
📚 Learning: 2025-11-11T03:47:45.878Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/add_test.go:67-77
Timestamp: 2025-11-11T03:47:45.878Z
Learning: In the cloudposse/atmos codebase, tests should prefer t.Setenv for environment variable setup/teardown instead of os.Setenv/Unsetenv to ensure test-scoped isolation.
Applied to files:
pkg/terraform/output/get_test.gopkg/terraform/output/workspace_test.gopkg/terraform/output/output_test.go
📚 Learning: 2025-12-21T04:10:29.030Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1891
File: internal/exec/describe_affected.go:468-468
Timestamp: 2025-12-21T04:10:29.030Z
Learning: In Go, package-level declarations (constants, variables, types, and functions) are visible to all files in the same package without imports. During reviews in cloudposse/atmos (and similar Go codebases), before suggesting to declare a new identifier, first check if it already exists in another file of the same package. If it exists, you can avoid adding a new declaration; if not, proceed with a proper package-level declaration.
Applied to files:
pkg/terraform/output/get_test.gopkg/terraform/output/spinner_test.gopkg/terraform/output/workspace_test.gopkg/terraform/output/output_test.gocmd/terraform/output_test.go
📚 Learning: 2024-11-02T15:35:09.958Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 759
File: internal/exec/terraform.go:366-368
Timestamp: 2024-11-02T15:35:09.958Z
Learning: In `internal/exec/terraform.go`, the workspace cleaning code under both the general execution path and within the `case "init":` block is intentionally duplicated because the code execution paths are different. The `.terraform/environment` file should be deleted before executing `terraform init` in both scenarios to ensure a clean state.
Applied to files:
pkg/terraform/output/workspace_test.go
📚 Learning: 2024-11-12T03:16:02.910Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 775
File: internal/exec/template_funcs_component.go:157-159
Timestamp: 2024-11-12T03:16:02.910Z
Learning: In the Go code for `componentFunc` in `internal/exec/template_funcs_component.go`, the function `cleanTerraformWorkspace` does not return errors, and it's acceptable if the file does not exist. Therefore, error handling for `cleanTerraformWorkspace` is not needed.
Applied to files:
pkg/terraform/output/workspace_test.go
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: The atmos codebase has a custom extension to *testing.T that provides a Chdir method, allowing test functions to call t.Chdir() to change working directories during tests. This is used consistently across test files in the codebase.
Applied to files:
pkg/terraform/output/workspace_test.gopkg/terraform/output/output_test.go
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: In the atmos codebase, t.Chdir() is a valid method call on *testing.T objects and works correctly for changing directories in tests. This is implemented through custom testing framework extensions and is used consistently throughout the test suite.
Applied to files:
pkg/terraform/output/workspace_test.gopkg/terraform/output/output_test.go
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: In the atmos codebase, t.Chdir() is a valid method call on *testing.T objects and works correctly for changing directories in tests.
Applied to files:
pkg/terraform/output/workspace_test.go
📚 Learning: 2025-08-15T14:43:41.030Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1352
File: pkg/store/artifactory_store_test.go:108-113
Timestamp: 2025-08-15T14:43:41.030Z
Learning: In test files for the atmos project, it's acceptable to ignore errors from os.Setenv/Unsetenv operations during test environment setup and teardown, as these are controlled test scenarios.
Applied to files:
pkg/terraform/output/workspace_test.gopkg/terraform/output/output_test.go
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: In the atmos codebase, t.Chdir() is a valid method that can be called on *testing.T objects. This functionality is implemented through custom testing framework extensions and is used consistently throughout the test suite for changing working directories during tests.
Applied to files:
pkg/terraform/output/workspace_test.go
📚 Learning: 2025-09-30T19:03:50.738Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 1560
File: pkg/utils/string_utils.go:43-64
Timestamp: 2025-09-30T19:03:50.738Z
Learning: In the Atmos codebase, YAML tags like !terraform.output rely on positional arguments, so the SplitStringByDelimiter function in pkg/utils/string_utils.go must preserve empty strings (even after trimming quotes) to maintain the correct number of positional arguments. Filtering out empty values after trimming would collapse the array and break these function calls.
Applied to files:
pkg/terraform/output/output_test.gocmd/terraform/output_test.go
📚 Learning: 2026-01-01T18:25:25.942Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T18:25:25.942Z
Learning: Applies to **/*_test.go : Never manually edit golden snapshot files under `tests/test-cases/` or `tests/testdata/`; always use `-regenerate-snapshots` flag. Never use pipe redirection when running tests as it breaks TTY detection
Applied to files:
pkg/terraform/output/output_test.gocmd/terraform/output_test.go
📚 Learning: 2026-01-01T18:25:25.942Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T18:25:25.942Z
Learning: Applies to **/*.go : For output, use I/O layer (`pkg/io/`) for streams and UI layer (`pkg/ui/`) for formatting. Use `data.Write/Writef/Writeln/WriteJSON/WriteYAML()` for stdout and `ui.Write/Writef/Writeln/Success/Error/Warning/Info/Markdown()` for stderr. Never use `fmt.Fprintf(os.Stdout/Stderr)` or `fmt.Println()`
Applied to files:
pkg/terraform/output/output_test.go
📚 Learning: 2025-02-19T05:50:35.853Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1068
File: tests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stdout.golden:0-0
Timestamp: 2025-02-19T05:50:35.853Z
Learning: Backtick formatting should only be applied to flag descriptions in Go source files, not in golden test files (test snapshots) as they are meant to capture the raw command output.
Applied to files:
pkg/terraform/output/output_test.gocmd/terraform/output_test.go
📚 Learning: 2025-12-10T18:32:43.260Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1808
File: cmd/terraform/backend/backend_delete_test.go:9-23
Timestamp: 2025-12-10T18:32:43.260Z
Learning: In cmd subpackages, tests should avoid using NewTestKit(t) unless tests actually interact with RootCmd (e.g., execute commands via RootCmd or modify RootCmd state). For structural tests that only verify command structure/flags without touching RootCmd, TestKit cleanup is unnecessary.
Applied to files:
cmd/terraform/output_test.go
📚 Learning: 2025-10-10T23:51:36.597Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1599
File: internal/exec/terraform.go:394-402
Timestamp: 2025-10-10T23:51:36.597Z
Learning: In Atmos (internal/exec/terraform.go), when adding OpenTofu-specific flags like `--var-file` for `init`, do not gate them based on command name (e.g., checking if `info.Command == "tofu"` or `info.Command == "opentofu"`) because command names don't reliably indicate the actual binary being executed (symlinks, aliases). Instead, document the OpenTofu requirement in code comments and documentation, trusting users who enable the feature (e.g., `PassVars`) to ensure their terraform command points to an OpenTofu binary.
Applied to files:
cmd/terraform/output_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to cmd/**/*.go : Provide comprehensive help text for all commands and flags, include examples in command help, and follow Go's documentation conventions in Cobra command definitions
Applied to files:
cmd/terraform/output_test.go
📚 Learning: 2026-01-01T18:25:25.942Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T18:25:25.942Z
Learning: Applies to **/*.go : Keep files focused and small (<600 lines); use one cmd/impl per file; co-locate tests; never use `//revive:disable:file-length-limit`
Applied to files:
cmd/terraform/output_test.go
📚 Learning: 2024-12-01T00:33:20.298Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml:28-32
Timestamp: 2024-12-01T00:33:20.298Z
Learning: In `examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml`, `!exec atmos terraform output` is used in examples to demonstrate its usage, even though `!terraform.output` is the recommended approach according to the documentation.
Applied to files:
cmd/terraform/output_test.go
📚 Learning: 2025-09-05T14:57:37.360Z
Learnt from: RoseSecurity
Repo: cloudposse/atmos PR: 1448
File: cmd/ansible.go:26-28
Timestamp: 2025-09-05T14:57:37.360Z
Learning: The Atmos codebase uses a consistent pattern for commands that delegate to external tools: `PersistentFlags().Bool("", false, doubleDashHint)` where doubleDashHint provides help text about using double dashes to separate Atmos options from native command arguments. This pattern is used across terraform, packer, helmfile, atlantis, aws, and ansible commands.
Applied to files:
cmd/terraform/output_test.go
📚 Learning: 2025-02-18T13:18:53.146Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1068
File: cmd/vendor_pull.go:31-31
Timestamp: 2025-02-18T13:18:53.146Z
Learning: Error checking is not required for cobra.Command.RegisterFlagCompletionFunc calls as these are static configurations done at init time.
Applied to files:
cmd/terraform/output_test.go
📚 Learning: 2025-03-18T12:26:25.329Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1149
File: tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden:7-7
Timestamp: 2025-03-18T12:26:25.329Z
Learning: In the Atmos project, typos or inconsistencies in test snapshot files (such as "terrafrom" instead of "terraform") may be intentional as they capture the exact output of commands and should not be flagged as issues requiring correction.
Applied to files:
cmd/terraform/output_test.go
📚 Learning: 2024-11-16T17:30:52.893Z
Learnt from: pkbhowmick
Repo: cloudposse/atmos PR: 786
File: internal/exec/shell_utils.go:159-162
Timestamp: 2024-11-16T17:30:52.893Z
Learning: For the `atmos terraform shell` command in `internal/exec/shell_utils.go`, input validation for the custom shell prompt is not required, as users will use this as a CLI tool and any issues will impact themselves.
Applied to files:
cmd/terraform/output_test.go
📚 Learning: 2025-12-13T03:21:27.620Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1813
File: cmd/terraform/shell.go:28-73
Timestamp: 2025-12-13T03:21:27.620Z
Learning: In Atmos, when initializing CLI config via cfg.InitCliConfig, always first populate the ConfigAndStacksInfo struct with global flag values by calling flags.ParseGlobalFlags(cmd, v) before LoadConfig. LoadConfig (pkg/config/load.go) reads config selection fields (AtmosConfigFilesFromArg, AtmosConfigDirsFromArg, BasePath, ProfilesFromArg) from the ConfigAndStacksInfo struct, not from Viper. Passing an empty struct causes the --base-path, --config, --config-path, and --profile flags to be ignored. Recommended pattern: parse flags → populate struct → call InitCliConfig. See cmd/terraform/plan_diff.go for reference implementation.
Applied to files:
cmd/terraform/output_test.go
📚 Learning: 2025-12-13T04:37:40.435Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: cmd/toolchain/get.go:23-40
Timestamp: 2025-12-13T04:37:40.435Z
Learning: In Go CLI command files using Cobra, constrain the subcommand to accept at most one positional argument (MaximumNArgs(1)) so it supports both listing all items (zero args) and fetching a specific item (one arg). Define and parse flags with a standard parser (e.g., flags.NewStandardParser()) and avoid binding flags to Viper (no viper.BindEnv/BindPFlag). This promotes explicit argument handling and predictable flag behavior across command files.
Applied to files:
cmd/terraform/output_test.go
🧬 Code graph analysis (2)
pkg/terraform/output/get_test.go (1)
pkg/terraform/output/get.go (6)
SetDefaultExecutor(14-18)GetDefaultExecutor(21-25)GetComponentOutputs(39-52)ExecuteWithSections(71-84)GetOutput(105-121)GetComponentOutputsWithExecutor(56-66)
pkg/terraform/output/spinner_test.go (1)
pkg/terraform/output/spinner.go (1)
NewSpinner(55-75)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
🔇 Additional comments (10)
pkg/terraform/output/workspace_test.go (4)
17-70: Solid workspace cleanup tests.Good use of
t.TempDir()andt.Setenv()for test isolation, and proper coverage of the environment file removal logic including custom TF_DATA_DIR scenarios.
72-125: Well-structured workspace lifecycle tests.Comprehensive coverage of the workspace creation and selection logic with proper gomock expectations. The test cases correctly validate both the happy path and error handling scenarios.
127-171: Good table-driven error detection tests.The test cases properly validate the error pattern matching logic across various scenarios, including edge cases like nil errors and different message formats.
3-15: Add blank lines between import groups.Per coding guidelines, organize imports in three groups separated by blank lines: (1) Go stdlib, (2) 3rd-party, (3) Atmos packages.
🔎 Proposed fix
import ( "context" "errors" "os" "path/filepath" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" "github.com/cloudposse/atmos/pkg/schema" )As per coding guidelines, imports must be organized in three groups with blank line separators.
⛔ Skipped due to learnings
Learnt from: aknysh Repo: cloudposse/atmos PR: 759 File: internal/exec/terraform.go:366-368 Timestamp: 2024-11-02T15:35:09.958Z Learning: In `internal/exec/terraform.go`, the workspace cleaning code under both the general execution path and within the `case "init":` block is intentionally duplicated because the code execution paths are different. The `.terraform/environment` file should be deleted before executing `terraform init` in both scenarios to ensure a clean state.Learnt from: CR Repo: cloudposse/atmos PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-01T18:25:25.942Z Learning: Applies to **/*_test.go : Test behavior, not implementation; never test stub functions; avoid tautological tests; use `errors.Is()` for error checking; remove always-skipped testsLearnt from: CR Repo: cloudposse/atmos PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-01T18:25:25.942Z Learning: Applies to **/*.go : Organize imports in three groups separated by blank lines, sorted alphabetically: (1) Go stdlib, (2) 3rd-party (NOT cloudposse/atmos), (3) Atmos packages. Maintain aliases: `cfg`, `log`, `u`, `errUtils`Learnt from: CR Repo: cloudposse/atmos PR: 0 File: .cursor/rules/atmos-rules.mdc:0-0 Timestamp: 2025-11-24T17:35:37.209Z Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests targeting >80% code coverage for all packagesLearnt from: CR Repo: cloudposse/atmos PR: 0 File: .cursor/rules/atmos-rules.mdc:0-0 Timestamp: 2025-11-24T17:35:37.209Z Learning: Applies to **/*_test.go : Use table-driven tests for testing multiple scenarios in GoLearnt from: aknysh Repo: cloudposse/atmos PR: 775 File: internal/exec/template_funcs_component.go:157-159 Timestamp: 2024-11-12T03:16:02.910Z Learning: In the Go code for `componentFunc` in `internal/exec/template_funcs_component.go`, the function `cleanTerraformWorkspace` does not return errors, and it's acceptable if the file does not exist. Therefore, error handling for `cleanTerraformWorkspace` is not needed.Learnt from: CR Repo: cloudposse/atmos PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-01T18:25:25.942Z Learning: Applies to **/*_test.go : Prefer unit tests with mocks over integration tests; use interfaces and dependency injection for testability; use table-driven tests for comprehensive coverage; target >80% coverage; skip tests gracefully with helpers from `tests/test_preconditions.go`Learnt from: samtholiya Repo: cloudposse/atmos PR: 1255 File: cmd/describe_affected_test.go:15-15 Timestamp: 2025-05-23T19:51:47.091Z Learning: The atmos codebase has a custom extension to *testing.T that provides a Chdir method, allowing test functions to call t.Chdir() to change working directories during tests. This is used consistently across test files in the codebase.Learnt from: osterman Repo: cloudposse/atmos PR: 1686 File: toolchain/add_test.go:67-77 Timestamp: 2025-11-11T03:47:45.878Z Learning: In the cloudposse/atmos codebase, tests should prefer t.Setenv for environment variable setup/teardown instead of os.Setenv/Unsetenv to ensure test-scoped isolation.Learnt from: osterman Repo: cloudposse/atmos PR: 727 File: internal/exec/terraform_clean.go:329-332 Timestamp: 2024-10-28T01:51:30.811Z Learning: In the Atmos Go code, when deleting directories or handling file paths (e.g., in `terraform_clean.go`), always resolve the absolute path using `filepath.Abs` and use the logger `u.LogWarning` for logging messages instead of using `fmt.Printf`.Learnt from: samtholiya Repo: cloudposse/atmos PR: 1147 File: internal/exec/validate_schema.go:42-57 Timestamp: 2025-04-11T22:06:46.999Z Learning: The "ExecuteAtmosValidateSchemaCmd" function in internal/exec/validate_schema.go has been reviewed and confirmed to have acceptable cognitive complexity despite static analysis warnings. The function uses a clean structure with only three if statements for error handling and delegates complex operations to helper methods.Learnt from: CR Repo: cloudposse/atmos PR: 0 File: .cursor/rules/atmos-rules.mdc:0-0 Timestamp: 2025-11-24T17:35:37.209Z Learning: Applies to **/*.go : Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementationsLearnt from: CR Repo: cloudposse/atmos PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-01T18:25:25.942Z Learning: Applies to **/*.go : Define interfaces for all major functionality; use dependency injection for testability; generate mocks with `go.uber.org/mock/mockgen` (with `//go:generate` directives), never manual mocksLearnt from: osterman Repo: cloudposse/atmos PR: 1686 File: toolchain/which_test.go:166-223 Timestamp: 2025-11-11T03:47:59.576Z Learning: In the cloudposse/atmos repo, tests that manipulate environment variables should use testing.T.Setenv for automatic setup/teardown instead of os.Setenv/Unsetenv.Learnt from: CR Repo: cloudposse/atmos PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-01T18:25:25.942Z Learning: Applies to pkg/datafetcher/schema/**/*.go : Update all schemas in `pkg/datafetcher/schema/` when adding config optionsLearnt from: aknysh Repo: cloudposse/atmos PR: 1352 File: pkg/store/artifactory_store_test.go:108-113 Timestamp: 2025-08-15T14:43:41.030Z Learning: In test files for the atmos project, it's acceptable to ignore errors from os.Setenv/Unsetenv operations during test environment setup and teardown, as these are controlled test scenarios.pkg/terraform/output/spinner_test.go (1)
11-16: LGTM - Basic smoke test is appropriate.Simple non-nil check for the constructor is sufficient for this use case.
pkg/terraform/output/get_test.go (1)
1-136: LGTM! Solid test coverage for default executor wiring.The test suite effectively validates:
- Default executor get/set behavior
- Panic scenarios with precise message validation
- Mock-based executor integration
The consistent save/restore pattern for
defaultExecutorensures proper test isolation.pkg/terraform/output/output_test.go (2)
12-195: Excellent use of table-driven tests.The tests for
GetFlattenSeparator,IsComplexValue, andValidateSingleValueFormatprovide comprehensive coverage with clear test case naming and good edge case handling.
250-276: Constant verification tests are acceptable.While these tests verify declared values against expected values, they serve a valid purpose in catching unintended changes to public API constants and ensuring format lists remain consistent.
cmd/terraform/output_test.go (2)
13-102: Well-structured command wiring tests.The tests properly verify:
- Command registration and hierarchy
- Parser and flag setup
- Default values using isolated Viper instance
- Environment variable bindings
Good use of fresh Viper in
TestOutputFlagDefaultsfor test isolation.
104-333: Comprehensive coverage of validation and formatting logic.The table-driven tests for
validateOutputFormat,extractOutputName, andformatSingleOutputprovide thorough coverage including:
- All supported formats (json, yaml, hcl, env, dotenv, bash, csv, tsv)
- Invalid/unsupported format handling
- Various argument parsing scenarios
- Format-specific restrictions (e.g., complex values with env format)
The flag shortcut verification ensures CLI usability.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cmd/terraform/output.go (1)
55-55: Consider the perf.Track with nil atmosConfig.The guidelines recommend passing atmosConfig to
perf.Track, but here it's unavailable until line 60. Passingnilis pragmatic for tracking the full function includingprepareOutputContext, but it deviates from the standard pattern. If the tracking data relies on atmosConfig, this might need restructuring—otherwise, document that nil is intentional for functions that bootstrap the config.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
CLAUDE.mdcmd/terraform/output.gointernal/exec/component_describer_adapter.gopkg/terraform/output/output_test.gopkg/terraform/output/spinner_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- pkg/terraform/output/spinner_test.go
- pkg/terraform/output/output_test.go
- internal/exec/component_describer_adapter.go
🧰 Additional context used
📓 Path-based instructions (2)
cmd/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/**/*.go: Use Cobra's recommended command structure with a root command and subcommands, implementing each command in a separate file undercmd/directory
Provide comprehensive help text for all commands and flags, include examples in command help, and follow Go's documentation conventions in Cobra command definitions
Provide meaningful feedback to users and include progress indicators for long-running operations in CLI commands
cmd/**/*.go: Adddefer perf.Track(atmosConfig, "pkg.FuncName")()+ blank line to all public functions, except trivial getters/setters, command constructors, simple factory functions, and functions that only delegate to another tracked function
Commands MUST use the command registry pattern viaCommandProviderinterface; seedocs/prd/command-registry-pattern.mdandcmd/internal/registry.go
Embed examples fromcmd/markdown/*_usage.mdusing//go:embedand render withutils.PrintfMarkdown()
Files:
cmd/terraform/output.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
All code must pass golangci-lint checks
Follow Go's error handling idioms: use meaningful error messages, wrap errors with context usingfmt.Errorf("context: %w", err), and consider using custom error types for domain-specific errors
Follow standard Go coding style: usegofmtandgoimportsto format code, prefer short descriptive variable names, use kebab-case for command-line flags, and snake_case for environment variables
Document all exported functions, types, and methods following Go's documentation conventions
Document complex logic with inline comments in Go code
Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults
Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging
**/*.go: All comments must end with periods (enforced bygodotlinter)
Never delete existing comments without a very strong reason; preserve helpful comments explaining why/how/what/where, and update comments to match code when refactoring
Organize imports in three groups separated by blank lines, sorted alphabetically: (1) Go stdlib, (2) 3rd-party (NOT cloudposse/atmos), (3) Atmos packages. Maintain aliases:cfg,log,u,errUtils
Useflags.NewStandardParser()for command-specific flags; NEVER callviper.BindEnv()orviper.BindPFlag()directly (enforced by Forbidigo)
All errors MUST be wrapped using static errors defined inerrors/errors.go; useerrors.Joinfor combining multiple errors,fmt.Errorfwith%wfor adding string context, error builder for complex errors, anderrors.Is()for error checking. Never use dynamic errors directly
Define interfaces for all major funct...
Files:
cmd/terraform/output.go
🧠 Learnings (22)
📓 Common learnings
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/terraform_utils.go:40-213
Timestamp: 2024-12-03T05:18:49.169Z
Learning: In the context of the Atmos project, it's acceptable for functions like `execTerraformOutput` to remain as single functions if they perform a single purpose, such as retrieving Terraform outputs for a component in a stack, even if the function is lengthy.
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml:28-32
Timestamp: 2024-12-01T00:33:20.298Z
Learning: In `examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml`, `!exec atmos terraform output` is used in examples to demonstrate its usage, even though `!terraform.output` is the recommended approach according to the documentation.
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/terraform_utils.go:144-145
Timestamp: 2024-12-03T03:52:02.524Z
Learning: Avoid adding context timeouts to Terraform commands in `execTerraformOutput` because their execution time can vary from seconds to hours, and making it configurable would require redesigning the command interface.
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T18:25:25.942Z
Learning: Stack pipeline: Load atmos.yaml → process imports/inheritance → apply overrides → render templates → generate config. Templates use Go templates + Gomplate with `atmos.Component()`, `!terraform.state`, `!terraform.output`, store integration
Learnt from: aknysh
Repo: cloudposse/atmos PR: 759
File: internal/exec/terraform.go:366-368
Timestamp: 2024-11-02T15:35:09.958Z
Learning: In `internal/exec/terraform.go`, the workspace cleaning code under both the general execution path and within the `case "init":` block is intentionally duplicated because the code execution paths are different. The `.terraform/environment` file should be deleted before executing `terraform init` in both scenarios to ensure a clean state.
📚 Learning: 2025-11-07T14:52:55.217Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1761
File: docs/prd/claude-agent-architecture.md:331-439
Timestamp: 2025-11-07T14:52:55.217Z
Learning: In the cloudposse/atmos repository, Claude agents are used as interactive tools, not in automated/headless CI/CD contexts. Agent documentation and patterns should assume synchronous human interaction.
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-01T18:25:25.942Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T18:25:25.942Z
Learning: Never run destructive git commands (`git reset`, `git checkout HEAD --`, `git clean -fd`, `git stash drop`) without explicit user confirmation
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-01T18:25:25.942Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T18:25:25.942Z
Learning: Use git worktrees for parallel development; always stay within the current working directory; use relative paths or paths under the current working directory; never escape to parent directories; never hardcode absolute paths like `/Users/*/atmos/`
Applied to files:
CLAUDE.md
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1327
File: cmd/terraform.go:111-117
Timestamp: 2025-06-23T02:14:30.937Z
Learning: In cmd/terraform.go, flags for the DescribeAffected function are added dynamically at runtime when info.Affected is true. This is intentional to avoid exposing internal flags like "file", "format", "verbose", "include-spacelift-admin-stacks", "include-settings", and "upload" in the terraform command interface, while still providing them for the shared DescribeAffected function used by both `atmos describe affected` and `atmos terraform apply --affected`.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2025-01-09T22:37:01.004Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/terraform_commands.go:260-265
Timestamp: 2025-01-09T22:37:01.004Z
Learning: In the terraform commands implementation (cmd/terraform_commands.go), the direct use of `os.Args[2:]` for argument handling is intentionally preserved to avoid extensive refactoring. While it could be improved to use cobra's argument parsing, such changes should be handled in a dedicated PR to maintain focus and minimize risk.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2024-12-17T07:08:41.288Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 863
File: internal/exec/yaml_func_terraform_output.go:34-38
Timestamp: 2024-12-17T07:08:41.288Z
Learning: In the `processTagTerraformOutput` function within `internal/exec/yaml_func_terraform_output.go`, parameters are separated by spaces and do not contain spaces. Therefore, using `strings.Fields()` for parsing is acceptable, and there's no need to handle parameters with spaces.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2025-10-10T23:51:36.597Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1599
File: internal/exec/terraform.go:394-402
Timestamp: 2025-10-10T23:51:36.597Z
Learning: In Atmos (internal/exec/terraform.go), when adding OpenTofu-specific flags like `--var-file` for `init`, do not gate them based on command name (e.g., checking if `info.Command == "tofu"` or `info.Command == "opentofu"`) because command names don't reliably indicate the actual binary being executed (symlinks, aliases). Instead, document the OpenTofu requirement in code comments and documentation, trusting users who enable the feature (e.g., `PassVars`) to ensure their terraform command points to an OpenTofu binary.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2025-10-07T00:25:16.333Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1498
File: website/src/components/Screengrabs/atmos-terraform-metadata--help.html:25-55
Timestamp: 2025-10-07T00:25:16.333Z
Learning: In Atmos CLI, subcommands inherit flags from their parent commands via Cobra's command inheritance. For example, `atmos terraform metadata --help` shows `--affected` and related flags inherited from the parent `terraform` command (defined in cmd/terraform.go), even though the metadata subcommand doesn't explicitly define these flags. This is expected Cobra behavior and auto-generated help screengrabs accurately reflect this inheritance.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2025-09-30T19:03:50.738Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 1560
File: pkg/utils/string_utils.go:43-64
Timestamp: 2025-09-30T19:03:50.738Z
Learning: In the Atmos codebase, YAML tags like !terraform.output rely on positional arguments, so the SplitStringByDelimiter function in pkg/utils/string_utils.go must preserve empty strings (even after trimming quotes) to maintain the correct number of positional arguments. Filtering out empty values after trimming would collapse the array and break these function calls.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2024-11-30T22:07:08.610Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/yaml_func_terraform_output.go:35-40
Timestamp: 2024-11-30T22:07:08.610Z
Learning: In the Go function `processTagTerraformOutput` in `internal/exec/yaml_func_terraform_output.go`, parameters cannot contain spaces. The code splits the input by spaces, and if the parameters contain spaces, `len(parts) != 3` will fail and show an error to the user.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2026-01-01T18:25:25.942Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T18:25:25.942Z
Learning: Stack pipeline: Load atmos.yaml → process imports/inheritance → apply overrides → render templates → generate config. Templates use Go templates + Gomplate with `atmos.Component()`, `!terraform.state`, `!terraform.output`, store integration
Applied to files:
cmd/terraform/output.go
📚 Learning: 2026-01-01T18:25:25.942Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-01T18:25:25.942Z
Learning: Applies to cmd/**/*.go : Add `defer perf.Track(atmosConfig, "pkg.FuncName")()` + blank line to all public functions, except trivial getters/setters, command constructors, simple factory functions, and functions that only delegate to another tracked function
Applied to files:
cmd/terraform/output.go
📚 Learning: 2025-12-13T03:21:27.620Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1813
File: cmd/terraform/shell.go:28-73
Timestamp: 2025-12-13T03:21:27.620Z
Learning: In Atmos, when initializing CLI config via cfg.InitCliConfig, always first populate the ConfigAndStacksInfo struct with global flag values by calling flags.ParseGlobalFlags(cmd, v) before LoadConfig. LoadConfig (pkg/config/load.go) reads config selection fields (AtmosConfigFilesFromArg, AtmosConfigDirsFromArg, BasePath, ProfilesFromArg) from the ConfigAndStacksInfo struct, not from Viper. Passing an empty struct causes the --base-path, --config, --config-path, and --profile flags to be ignored. Recommended pattern: parse flags → populate struct → call InitCliConfig. See cmd/terraform/plan_diff.go for reference implementation.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2025-02-18T13:18:53.146Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1068
File: cmd/vendor_pull.go:31-31
Timestamp: 2025-02-18T13:18:53.146Z
Learning: Error checking is not required for cobra.Command.RegisterFlagCompletionFunc calls as these are static configurations done at init time.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2025-11-10T03:03:31.505Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 0
File: :0-0
Timestamp: 2025-11-10T03:03:31.505Z
Learning: In the Atmos codebase, commands using the `StandardParser` flag pattern (from pkg/flags) do NOT need explicit `viper.BindPFlag()` calls in their code. The StandardParser encapsulates flag binding internally: flags are registered via `parser.RegisterFlags(cmd)` in init(), and bound via `parser.BindFlagsToViper(cmd, v)` in RunE, which internally calls viper.BindPFlag for each flag. This pattern is used throughout Atmos (e.g., cmd/toolchain/get.go, cmd/toolchain/info.go, cmd/toolchain/install.go, cmd/toolchain/path.go). Do not flag missing viper.BindPFlag calls when StandardParser is used.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2024-12-05T22:33:40.955Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 820
File: cmd/list_components.go:53-54
Timestamp: 2024-12-05T22:33:40.955Z
Learning: In the Atmos CLI Go codebase, using `u.LogErrorAndExit` within completion functions is acceptable because it logs the error and exits the command execution.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2025-12-13T04:37:25.223Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: cmd/root.go:0-0
Timestamp: 2025-12-13T04:37:25.223Z
Learning: In Atmos cmd/root.go Execute(), after cfg.InitCliConfig, we must call both toolchainCmd.SetAtmosConfig(&atmosConfig) and toolchain.SetAtmosConfig(&atmosConfig) so the CLI wrapper and the toolchain package receive configuration; missing either can cause nil-pointer panics in toolchain path resolution.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2025-09-05T14:57:37.360Z
Learnt from: RoseSecurity
Repo: cloudposse/atmos PR: 1448
File: cmd/ansible.go:26-28
Timestamp: 2025-09-05T14:57:37.360Z
Learning: The Atmos codebase uses a consistent pattern for commands that delegate to external tools: `PersistentFlags().Bool("", false, doubleDashHint)` where doubleDashHint provides help text about using double dashes to separate Atmos options from native command arguments. This pattern is used across terraform, packer, helmfile, atlantis, aws, and ansible commands.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2025-12-13T04:37:40.435Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: cmd/toolchain/get.go:23-40
Timestamp: 2025-12-13T04:37:40.435Z
Learning: In Go CLI command files using Cobra, constrain the subcommand to accept at most one positional argument (MaximumNArgs(1)) so it supports both listing all items (zero args) and fetching a specific item (one arg). Define and parse flags with a standard parser (e.g., flags.NewStandardParser()) and avoid binding flags to Viper (no viper.BindEnv/BindPFlag). This promotes explicit argument handling and predictable flag behavior across command files.
Applied to files:
cmd/terraform/output.go
📚 Learning: 2025-12-21T04:10:29.030Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1891
File: internal/exec/describe_affected.go:468-468
Timestamp: 2025-12-21T04:10:29.030Z
Learning: In Go, package-level declarations (constants, variables, types, and functions) are visible to all files in the same package without imports. During reviews in cloudposse/atmos (and similar Go codebases), before suggesting to declare a new identifier, first check if it already exists in another file of the same package. If it exists, you can avoid adding a new declaration; if not, proceed with a proper package-level declaration.
Applied to files:
cmd/terraform/output.go
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
🔇 Additional comments (8)
CLAUDE.md (1)
22-32: Well-structured guidance that aligns with learnings—but appears tangential to the PR objective.The new "Concurrent Sessions (MANDATORY)" section is clear, actionable, and consistent with the repository's existing documentation style. The rationale is solid: preventing accidental deletion of parallel work is a real concern when multiple Claude agents operate on the same branch.
That said, this documentation change seems disconnected from the PR's stated objective (Terraform output format refactoring). Is this an ancillary change from a parallel session, or intentionally bundled with PR #1885?
Can you confirm whether this documentation update belongs in this PR or should be separated into a dedicated documentation PR?
cmd/terraform/output.go (7)
37-50: LGTM on the conditional routing logic.The dual flag binding and conditional execution path are clean. When
--formatis provided, the new formatting flow kicks in; otherwise, it delegates to the existing terraform passthrough logic.
67-76: LGTM on format validation.Clean fail-fast validation with helpful error messages. Using
slices.Containskeeps it simple and the error builder pattern provides clear user guidance.
78-107: LGTM on the context preparation pattern.This correctly implements the recommended flow: parse global flags → populate
ConfigAndStacksInfo→ callInitCliConfig. The struct receivesBasePath,Config,ConfigPath, andProfilefrom parsed flags, ensuring those global flags are respected during config loading.
109-147: LGTM on the execution and formatting logic.The flow is clear: retrieve outputs, apply format options, handle single-output extraction if requested, and write to file or stdout using the correct I/O layer. Error handling provides context at each step.
149-158: LGTM on output name extraction.Extracting the first non-flag argument as the output name is correct for the terraform output use case. Only one output name is valid, so returning the first match is appropriate.
160-170: LGTM on single output formatting.The existence check and error message are user-friendly, guiding users to list all outputs if their requested name isn't found.
172-195: LGTM on flag setup and initialization.The
StandardParserpattern is correctly applied: flags defined with env var bindings, registered to the command, bound to Viper, and completion wired up. The ignored error on line 190 is acceptable for static flag completion registration at init time.
|
These changes were released in v1.204.0-rc.0. |
Summary
internal/exec/to newpkg/terraform/output/packageComponentDescriberinterface to eliminate circular dependencyWhat Changed
Verification
make buildpassesmake lintpasses (0 issues)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.