Add flag to atmos terraform generate planfile#1612
Add flag to atmos terraform generate planfile#1612Cerebrovinny wants to merge 2 commits intomainfrom
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughRefactors planfile path resolution to add an output-path mode and related CLI flag. Introduces an unexported planfilePathOptions struct, changes resolvePlanfilePath to accept it, adds OutputPath to PlanfileOptions across packages, updates logging and tests, and enforces mutual exclusivity with the existing file option. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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: 1
🧹 Nitpick comments (4)
internal/exec/terraform_generate_planfile_test.go (3)
88-121: Good mutual-exclusivity test. Consider also exercising Cobra’s exclusivity.Current test hits exec-level check. Add a variant using the actual command wiring (with MarkFlagsMutuallyExclusive) to ensure Cobra returns the exclusivity error pre-execution.
241-287: Dir-path scenarios look solid. One more case: absolute file path.Add a case where options.File is an absolute path to ensure resolvePlanfilePath honors it without prefixing componentPath.
467-541: Nice table-driven coverage for path resolution.Covers defaults, relative file, relative/absolute dirs. Add one for absolute file path for completeness.
internal/exec/terraform_generate_planfile.go (1)
26-27: Centralize ErrMutuallyExclusiveFlags per repo error policy.Per guidelines, prefer static errors from errors/errors.go. Consider moving ErrMutuallyExclusiveFlags there to keep errors discoverable and consistent.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
cmd/terraform_generate_planfile.go(1 hunks)internal/exec/terraform_generate_planfile.go(7 hunks)internal/exec/terraform_generate_planfile_test.go(6 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
cmd/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/**/*.go: Use Cobra's recommended command structure with a root command and subcommands
Implement each CLI command in a separate file under cmd/
Use Viper for managing configuration, environment variables, and flags in the CLI
Keep separation of concerns between CLI interface (cmd) and business logic
Use kebab-case for command-line flags
Provide comprehensive help text for all commands and flags
Include examples in Cobra command help
Use Viper for configuration management; support files, env vars, and flags with precedence flags > env > config > defaults
Follow single responsibility; separate command interface from business logic
Provide meaningful user feedback and include progress indicators for long-running operations
Provide clear error messages to users and troubleshooting hints where appropriate
cmd/**/*.go: Cobra command files should embed usage markdown via //go:embed and render with utils.PrintfMarkdown().
For non-standard execution paths, capture telemetry with telemetry.CaptureCmd or telemetry.CaptureCmdString.
Files:
cmd/terraform_generate_planfile.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: All comments must end with periods; comments should be complete sentences (golangci-lint godot).
Organize imports into three groups separated by blank lines: standard library, third-party, and github.com/cloudposse/atmos packages; sort alphabetically within each group and preserve existing aliases.
Wrap all errors using static errors from errors/errors.go; prefer errors.Join for multiple errors; use fmt.Errorf with %w for context; check with errors.Is; never use dynamic errors directly.
Distinguish structured logging from UI output: use stderr for prompts/status/errors needing user action, stdout only for data; never use logging for UI elements.
Most text UI must go to stderr; only data/results go to stdout; prefer utils.PrintfMessageToTUI for UI messages.
Ensure cross-platform compatibility: prefer SDKs over external binaries; use filepath.Join, os.PathSeparator, and runtime.GOOS for OS-specific logic; add build constraints if needed.
Files:
cmd/terraform_generate_planfile.gointernal/exec/terraform_generate_planfile.gointernal/exec/terraform_generate_planfile_test.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
cmd/terraform_generate_planfile.gointernal/exec/terraform_generate_planfile.go
@(cmd|internal|pkg)/**/!(*_test).go
📄 CodeRabbit inference engine (CLAUDE.md)
@(cmd|internal|pkg)/**/!(*_test).go: Adddefer perf.Track()to all public and critical private functions with a blank line after the call; use package-qualified names like "packagename.FunctionName"; pass atmosConfig if present, else nil.
Ensure 80% minimum coverage on new/changed lines; exclude mock files from coverage (mock_.go, **/mock_.go, **/mock/*.go); include comprehensive unit and required integration tests.
Files:
cmd/terraform_generate_planfile.gointernal/exec/terraform_generate_planfile.go
cmd/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
One Cobra command per file under cmd/.
Files:
cmd/terraform_generate_planfile.go
@(cmd|internal|pkg)/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Bind environment variables with viper.BindEnv() and provide ATMOS_ alternatives for every env var.
Files:
cmd/terraform_generate_planfile.gointernal/exec/terraform_generate_planfile.gointernal/exec/terraform_generate_planfile_test.go
internal/exec/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
All new configs must support Go templating using FuncMap() from internal/exec/template_funcs.go; test template rendering with various contexts.
Files:
internal/exec/terraform_generate_planfile.gointernal/exec/terraform_generate_planfile_test.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios
**/*_test.go: Use table-driven tests for unit tests; focus on pure functions; generate and use mocks for external dependencies.
Test behavior, not implementation; avoid tautological or stub tests; remove always-skipped tests; ensure tests validate real behavior.
Always use t.Skipf() with a reason for skipped tests; never call t.Skipf without a reason; CLI tests build a temp binary and TestMain must call os.Exit(m.Run()).
Files:
internal/exec/terraform_generate_planfile_test.go
🧠 Learnings (3)
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
PR: cloudposse/atmos#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/terraform_generate_planfile.go
📚 Learning: 2025-09-05T14:57:37.360Z
Learnt from: RoseSecurity
PR: cloudposse/atmos#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:
internal/exec/terraform_generate_planfile_test.go
📚 Learning: 2025-10-07T14:35:22.186Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-07T14:35:22.186Z
Learning: Use Atmos as a Go CLI managing Terraform, Helmfile, and Packer with stack-based YAML configuration and templating.
Applied to files:
internal/exec/terraform_generate_planfile_test.go
🧬 Code graph analysis (2)
internal/exec/terraform_generate_planfile.go (1)
pkg/schema/schema.go (2)
ConfigAndStacksInfo(460-539)AtmosConfiguration(27-65)
internal/exec/terraform_generate_planfile_test.go (2)
internal/exec/terraform_generate_planfile.go (4)
ExecuteTerraformGeneratePlanfileCmd(42-111)ErrMutuallyExclusiveFlags(26-26)ExecuteTerraformGeneratePlanfile(114-193)PlanfileOptions(30-39)pkg/schema/schema.go (2)
AtmosConfiguration(27-65)ConfigAndStacksInfo(460-539)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: Build (macos-latest, macos)
- GitHub Check: Build (ubuntu-latest, linux)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: Run pre-commit hooks
- GitHub Check: Summary
🔇 Additional comments (1)
internal/exec/terraform_generate_planfile.go (1)
216-243: Ignore EnsureDir argument change
EnsureDir is implemented to accept a full file path and create its parent directories (see pkg/utils/file_utils.go and corresponding tests), so passingplanFilePathis correct.Likely an incorrect or invalid review comment.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1612 +/- ##
==========================================
- Coverage 73.67% 73.66% -0.01%
==========================================
Files 656 656
Lines 60813 60831 +18
==========================================
+ Hits 44802 44813 +11
- Misses 12970 12982 +12
+ Partials 3041 3036 -5
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
errors/errors.go(1 hunks)internal/exec/terraform_generate_planfile.go(7 hunks)internal/exec/terraform_generate_planfile_test.go(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/exec/terraform_generate_planfile_test.go
🧰 Additional context used
📓 Path-based instructions (6)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: All comments must end with periods; comments should be complete sentences (golangci-lint godot).
Organize imports into three groups separated by blank lines: standard library, third-party, and github.com/cloudposse/atmos packages; sort alphabetically within each group and preserve existing aliases.
Wrap all errors using static errors from errors/errors.go; prefer errors.Join for multiple errors; use fmt.Errorf with %w for context; check with errors.Is; never use dynamic errors directly.
Distinguish structured logging from UI output: use stderr for prompts/status/errors needing user action, stdout only for data; never use logging for UI elements.
Most text UI must go to stderr; only data/results go to stdout; prefer utils.PrintfMessageToTUI for UI messages.
Ensure cross-platform compatibility: prefer SDKs over external binaries; use filepath.Join, os.PathSeparator, and runtime.GOOS for OS-specific logic; add build constraints if needed.
Files:
errors/errors.gointernal/exec/terraform_generate_planfile.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
errors/errors.gointernal/exec/terraform_generate_planfile.go
errors/errors.go
📄 CodeRabbit inference engine (CLAUDE.md)
Define static errors in errors/errors.go as package-level variables using errors.New().
Files:
errors/errors.go
@(cmd|internal|pkg)/**/!(*_test).go
📄 CodeRabbit inference engine (CLAUDE.md)
@(cmd|internal|pkg)/**/!(*_test).go: Adddefer perf.Track()to all public and critical private functions with a blank line after the call; use package-qualified names like "packagename.FunctionName"; pass atmosConfig if present, else nil.
Ensure 80% minimum coverage on new/changed lines; exclude mock files from coverage (mock_.go, **/mock_.go, **/mock/*.go); include comprehensive unit and required integration tests.
Files:
internal/exec/terraform_generate_planfile.go
@(cmd|internal|pkg)/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Bind environment variables with viper.BindEnv() and provide ATMOS_ alternatives for every env var.
Files:
internal/exec/terraform_generate_planfile.go
internal/exec/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
All new configs must support Go templating using FuncMap() from internal/exec/template_funcs.go; test template rendering with various contexts.
Files:
internal/exec/terraform_generate_planfile.go
🧠 Learnings (2)
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
PR: cloudposse/atmos#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:
errors/errors.gointernal/exec/terraform_generate_planfile.go
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
PR: cloudposse/atmos#1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
internal/exec/terraform_generate_planfile.go
🧬 Code graph analysis (1)
internal/exec/terraform_generate_planfile.go (2)
errors/errors.go (1)
ErrMutuallyExclusiveFlags(52-52)pkg/schema/schema.go (1)
ConfigAndStacksInfo(460-539)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Build (macos-latest, macos)
- GitHub Check: Build (ubuntu-latest, linux)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Run pre-commit hooks
- GitHub Check: Summary
🔇 Additional comments (8)
errors/errors.go (1)
52-52: Clear and actionable error message.The new error sentinel follows the established pattern and provides helpful guidance to users on the mutual exclusivity requirement.
internal/exec/terraform_generate_planfile.go (7)
9-9: Import follows conventions.The error utils import with alias is clear and necessary for accessing the new mutual exclusivity error sentinel.
35-35: Field addition is straightforward.The Dir field complements the existing File field and follows the struct's naming conventions.
61-64: Standard flag handling.Reading the --dir flag follows the established pattern used for other flags in this function.
95-97: Mutual exclusivity enforced at CLI level.This check correctly prevents both flags from being specified. Note that a similar check exists in ExecuteTerraformGeneratePlanfile (lines 128-130). While this provides defense in depth, consider if both checks are necessary or if the execution-level check alone would suffice.
104-104: Options initialization looks good.The Dir field is properly populated in the PlanfileOptions struct.
128-130: Execution-level validation.This mutual exclusivity check mirrors the one in ExecuteTerraformGeneratePlanfileCmd (lines 95-97), providing protection if this function is called directly from other code paths.
218-235: Path resolution logic looks good.constructTerraformComponentPlanfileNameis defined at internal/exec/path_utils.go:37.
|
@coderabbitai can you confirm that the |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 5044 🏁 Script executed: Length of output: 7094 🏁 Script executed: Length of output: 9021 @osterman, based on my analysis of the codebase, Existing output path patterns:
Key findings:
Alternative to consider:
Recommendation: Would you like me to search for how similar tools in the ecosystem handle directory output flags? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 8229 🏁 Script executed: Length of output: 8816 @osterman: After analyzing the codebase, I can confirm that Evidence of the established pattern: Path-related flags throughout the codebase:
Recommendation: The most conventional and consistent name would be The current |
|
@Cerebrovinny see the @coderabbitai comment about flag name.
|
|
Tip For best results, initiate chat on the files or code changes. @osterman: Acknowledged. I'm here to help if @Cerebrovinny has any questions about the flag renaming or needs assistance with the refactoring to use |
|
Tip For best results, initiate chat on the files or code changes. @osterman: Thank you for forwarding the recommendation. I'm standing by to assist @Cerebrovinny with any questions about refactoring from |
|
Warning Changelog Entry RequiredThis PR is labeled Action needed: Add a new blog post in Example filename: Alternatively: If this change doesn't require a changelog entry, remove the |
There was a problem hiding this comment.
Pull request overview
This PR adds support for organizing planfiles in custom directories by introducing a --dir flag to the terraform generate planfile command. The flag allows users to specify an output directory (absolute or relative) while maintaining the default naming convention, and is mutually exclusive with the existing --file flag.
Key Changes:
- Added
--dirflag with mutual exclusivity enforcement against--file - Enhanced path resolution logic to support both absolute and relative directory paths
- Added comprehensive test coverage for the new directory functionality and flag conflict scenarios
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cmd/terraform_generate_planfile.go | Adds the --dir flag definition and marks it mutually exclusive with --file |
| internal/exec/terraform_generate_planfile.go | Implements directory path resolution logic and mutual exclusivity validation |
| internal/exec/terraform_generate_planfile_test.go | Adds tests for directory functionality, path resolution, and mutual exclusivity enforcement |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Add command-specific flags. | ||
| cmd.PersistentFlags().StringP("stack", "s", "", "Atmos stack") | ||
| cmd.PersistentFlags().StringP("file", "f", "", "Planfile name") | ||
| cmd.PersistentFlags().StringP("dir", "d", "", "Directory where the planfile will be generated using the default naming convention ({stack}-{component}.planfile.{format})") |
There was a problem hiding this comment.
The flag description is inconsistent with the production code. In cmd/terraform_generate_planfile.go line 36, the description includes 'Mutually exclusive with --file.' but this test's description omits that detail. Update the description to match the production code for consistency.
| cmd.PersistentFlags().StringP("dir", "d", "", "Directory where the planfile will be generated using the default naming convention ({stack}-{component}.planfile.{format})") | |
| cmd.PersistentFlags().StringP("dir", "d", "", "Directory where the planfile will be generated using the default naming convention ({stack}-{component}.planfile.{format}). Mutually exclusive with --file.") |
| // Add command-specific flags. | ||
| conflictingCmd.PersistentFlags().StringP("stack", "s", "", "Atmos stack") | ||
| conflictingCmd.PersistentFlags().StringP("file", "f", "", "Planfile name") | ||
| conflictingCmd.PersistentFlags().StringP("dir", "d", "", "Directory where the planfile will be generated using the default naming convention ({stack}-{component}.planfile.{format})") |
There was a problem hiding this comment.
The flag description is inconsistent with the production code. This description should match the one in cmd/terraform_generate_planfile.go line 36, which includes 'Mutually exclusive with --file.'
| conflictingCmd.PersistentFlags().StringP("dir", "d", "", "Directory where the planfile will be generated using the default naming convention ({stack}-{component}.planfile.{format})") | |
| conflictingCmd.PersistentFlags().StringP("dir", "d", "", "Directory where the planfile will be generated using the default naming convention ({stack}-{component}.planfile.{format}). Mutually exclusive with --file.") |
|
Warning Changelog Entry RequiredThis PR is labeled Action needed: Add a new blog post in Example filename: Alternatively: If this change doesn't require a changelog entry, remove the |
|
💥 This pull request now has conflicts. Could you fix it @Cerebrovinny? 🙏 |
osterman
force-pushed
the
feat/file-flag
branch
from
eac4850 to
8392b24
Compare
|
Warning Changelog Entry RequiredThis PR is labeled Action needed: Add a new blog post in Example filename: Alternatively: If this change doesn't require a changelog entry, remove the |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cmd/terraform/generate/planfile.go (1)
101-101: Clarify flag description: directory path expected.The description mentions "using default naming ({stack}-{component}.planfile.{format})", which indicates a directory path is expected where a file with default naming will be created. However, "Output path" is ambiguous—it could be interpreted as either a directory or a file path.
Consider updating the description to explicitly state "Output directory path" to match the intended behavior described in the PR objectives.
Apply this diff to clarify the description:
- flags.WithStringFlag("output-path", "o", "", "Output path for planfile using default naming ({stack}-{component}.planfile.{format})"), + flags.WithStringFlag("output-path", "o", "", "Output directory path for planfile using default naming ({stack}-{component}.planfile.{format})"),
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
cmd/terraform/generate/planfile.go(4 hunks)
🧰 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: Commands MUST use flags.NewStandardParser() for command-specific flags; NEVER call viper.BindEnv() or viper.BindPFlag() directly (Forbidigo enforces this); see cmd/version/version.go for reference
Embed CLI examples from cmd/markdown/*_usage.md using //go:embed; render with utils.PrintfMarkdown()
Files:
cmd/terraform/generate/planfile.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: 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:
cmd/terraform/generate/planfile.go
🧠 Learnings (18)
📓 Common learnings
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.
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_utils.go:496-513
Timestamp: 2024-11-22T12:38:33.132Z
Learning: In the Atmos project, continue to flag path traversal issues in code reviews but acknowledge when they are expected and acceptable in specific cases.
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: 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.
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`.
📚 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/generate/planfile.go
📚 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/generate/planfile.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/generate/planfile.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/generate/planfile.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/generate/planfile.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/generate/planfile.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/generate/planfile.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/generate/planfile.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 **/*.go : Use Viper for managing configuration, environment variables, and flags in CLI commands
Applied to files:
cmd/terraform/generate/planfile.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 **/*.go : Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults
Applied to files:
cmd/terraform/generate/planfile.go
📚 Learning: 2025-12-16T18:20:55.614Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.614Z
Learning: Applies to cmd/**/*.go : Commands MUST use flags.NewStandardParser() for command-specific flags; NEVER call viper.BindEnv() or viper.BindPFlag() directly (Forbidigo enforces this); see cmd/version/version.go for reference
Applied to files:
cmd/terraform/generate/planfile.go
📚 Learning: 2025-04-23T15:02:50.246Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1202
File: pkg/utils/yaml_func_exec.go:104-104
Timestamp: 2025-04-23T15:02:50.246Z
Learning: In the Atmos codebase, direct calls to `os.Getenv` should be avoided. Instead, use `viper.BindEnv` for environment variable access. This provides a consistent approach to configuration management across the codebase.
Applied to files:
cmd/terraform/generate/planfile.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/generate/planfile.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/generate/planfile.go
📚 Learning: 2025-06-07T19:28:21.289Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1266
File: cmd/describe_affected.go:0-0
Timestamp: 2025-06-07T19:28:21.289Z
Learning: In the Atmos codebase, using panic for unsupported flag types in flag processing functions like setDescribeAffectedFlagValueInCliArgs is the expected behavior rather than returning errors. This pattern is preferred for developer errors when unsupported types are added to the flagsKeyValue map.
Applied to files:
cmd/terraform/generate/planfile.go
📚 Learning: 2025-05-22T15:42:10.906Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1261
File: internal/exec/utils.go:639-640
Timestamp: 2025-05-22T15:42:10.906Z
Learning: In the Atmos codebase, when appending slices with `args := append(configAndStacksInfo.CliArgs, configAndStacksInfo.AdditionalArgsAndFlags...)`, it's intentional that the result is not stored back in the original slice. This pattern is used when the merged result serves a different purpose than the original slices, such as when creating a filtered version for component section assignments.
Applied to files:
cmd/terraform/generate/planfile.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/generate/planfile.go
🧬 Code graph analysis (1)
cmd/terraform/generate/planfile.go (2)
pkg/flags/parser.go (1)
GetString(117-126)pkg/flags/options.go (4)
WithStringFlag(49-60)WithBoolFlag(63-74)WithStringSliceFlag(101-112)WithEnvVars(221-244)
⏰ 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 (2)
cmd/terraform/generate/planfile.go (2)
118-119: Mutual exclusivity correctly enforced.The implementation correctly prevents users from specifying both
--fileand--output-pathsimultaneously, matching the PR objectives.
56-56: Flag name and description are sufficiently clear; implementation correctly treats--output-pathas a directory.The
--output-pathflag description states "using default naming ({stack}-{component}.planfile.{format})", which clarifies that the flag accepts a directory path where a default-named file will be created. The execution layer confirms this behavior, and mutual exclusivity with--fileis properly enforced. No changes needed.
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
osterman
force-pushed
the
feat/file-flag
branch
from
045c842 to
4b50fc9
Compare
|
Warning Release Documentation RequiredThis PR is labeled
|
|
💥 This pull request now has conflicts. Could you fix it @Cerebrovinny? 🙏 |
Add a new --output-path (-o) flag to the terraform generate planfile command
that specifies an output directory while using the default naming convention
({stack}-{component}.planfile.{format}).
This flag is mutually exclusive with --file:
- --file: specify exact output file path
- --output-path: specify output directory, use default filename
Changes:
- Add --output-path flag with -o alias and ATMOS_OUTPUT_PATH env var
- Add OutputPath field to PlanfileOptions
- Update resolvePlanfilePath to handle three modes (exact file, output dir, default)
- Add unit tests for path resolution modes
- Add integration test for --output-path option
- Update documentation with flag description and examples
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
osterman
force-pushed
the
feat/file-flag
branch
from
4b50fc9 to
d599f73
Compare
|
Warning Release Documentation RequiredThis PR is labeled
|
|
Warning Release Documentation RequiredThis PR is labeled
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/exec/terraform_generate_planfile_test.go (1)
362-423: Excellent table-driven test structure.The test comprehensively covers Mode 1 (exact file path) and Mode 2 (output directory with default naming) for both relative and absolute paths. Well-organized and clear.
Consider adding a test case for Mode 3 (default behavior when neither
customFilenoroutputPathis set) for completeness, though the existingTestExecuteTerraformGeneratePlanfiledoes cover this implicitly.Optional: Add Mode 3 test case
{ name: "Mode 2: Output directory with default filename (absolute)", customFile: "", outputPath: filepath.Join(tmpDir, "output"), format: "yaml", expectedPath: filepath.Join(tmpDir, "output", "dev-us-east-1-vpc.planfile.yaml"), }, + { + name: "Mode 3: Default atmos config path", + customFile: "", + outputPath: "", + format: "json", + expectedPath: filepath.Join(componentPath, "dev-us-east-1-vpc.planfile.json"), + }, }Note: You'll need to update the test setup to provide a proper
atmosConfigmock that returns the expected default path for Mode 3.
📜 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 (4)
cmd/terraform/generate/planfile.gointernal/exec/terraform_generate_planfile.gointernal/exec/terraform_generate_planfile_test.gopkg/terraform/options.go
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/terraform/generate/planfile.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: Use context.Context only for cancellation signals, deadlines/timeouts, and request-scoped values (sparingly). DO NOT use context for configuration, dependencies, or avoiding proper function parameters. Context should be first parameter in functions that accept it
All comments must end with periods (enforced by godot linter)
NEVER delete existing comments without a very strong reason. Preserve helpful comments explaining why/how/what/where. 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
Add defer perf.Track(atmosConfig, "pkg.FuncName")() + blank line to all public functions. Use nil if no atmosConfig param. Exceptions: trivial getters/setters, command constructors, simple factories, functions that onl...
Files:
pkg/terraform/options.gointernal/exec/terraform_generate_planfile_test.gointernal/exec/terraform_generate_planfile.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 fixturesPrefer unit tests with mocks over integration tests. Use interfaces + dependency injection for testability. Generate mocks with go.uber.org/mock/mockgen. Use table-driven tests for comprehensive coverage. Target >80% coverage
Files:
internal/exec/terraform_generate_planfile_test.go
{cmd,internal}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
{cmd,internal}/**/*.go: Use I/O layer (pkg/io/) for stream access and UI layer (pkg/ui/) for formatting. Output data via data.Write/Writef/Writeln/WriteJSON/WriteYAML to stdout. Output UI messages via ui.Write/Writef/Writeln/Success/Error/Warning/Info/Markdown to stderr. DO NOT use fmt.Fprintf, fmt.Println, os.Stdout, or os.Stderr
Assume full TTY when writing output code. System automatically handles color degradation, width adaptation, TTY detection, CI detection, markdown rendering, icon support, secret masking, and format-aware masking
Telemetry is auto-enabled via RootCmd.ExecuteC(). Non-standard paths use telemetry.CaptureCmd(). Never capture user data
Files:
internal/exec/terraform_generate_planfile_test.gointernal/exec/terraform_generate_planfile.go
{cmd,internal,pkg/ui}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Use colors from pkg/ui/theme/colors.go for styling output
Files:
internal/exec/terraform_generate_planfile_test.gointernal/exec/terraform_generate_planfile.go
🧠 Learnings (32)
📓 Common learnings
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.
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_utils.go:496-513
Timestamp: 2024-11-22T12:38:33.132Z
Learning: In the Atmos project, continue to flag path traversal issues in code reviews but acknowledge when they are expected and acceptable in specific cases.
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: 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.
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: osterman
Repo: cloudposse/atmos PR: 887
File: internal/exec/workflow_utils.go:167-169
Timestamp: 2024-12-25T20:28:19.618Z
Learning: The user plans to revert the change from `path.Join` to `filepath.Join` in this PR due to testing gaps and will open a new PR to safely handle the migration without breaking `main`.
📚 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/options.gointernal/exec/terraform_generate_planfile_test.gointernal/exec/terraform_generate_planfile.go
📚 Learning: 2024-10-27T04:41:49.199Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:215-223
Timestamp: 2024-10-27T04:41:49.199Z
Learning: In `internal/exec/terraform_clean.go`, the function `determineCleanPath` is necessary and should not be removed.
Applied to files:
internal/exec/terraform_generate_planfile_test.gointernal/exec/terraform_generate_planfile.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:
internal/exec/terraform_generate_planfile_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:
internal/exec/terraform_generate_planfile_test.gointernal/exec/terraform_generate_planfile.go
📚 Learning: 2024-10-23T21:36:40.262Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.
Applied to files:
internal/exec/terraform_generate_planfile_test.gointernal/exec/terraform_generate_planfile.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/terraform_generate_planfile_test.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:
internal/exec/terraform_generate_planfile_test.gointernal/exec/terraform_generate_planfile.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/terraform_generate_planfile_test.go
📚 Learning: 2024-10-21T17:51:07.087Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform.go:114-118
Timestamp: 2024-10-21T17:51:07.087Z
Learning: Use `bubbletea` for confirmation prompts instead of `fmt.Scanln` in the `atmos terraform clean` command.
Applied to files:
internal/exec/terraform_generate_planfile_test.go
📚 Learning: 2025-04-11T22:06:46.999Z
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.
Applied to files:
internal/exec/terraform_generate_planfile_test.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:
internal/exec/terraform_generate_planfile_test.gointernal/exec/terraform_generate_planfile.go
📚 Learning: 2024-10-31T19:25:41.298Z
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.
Applied to files:
internal/exec/terraform_generate_planfile_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:
internal/exec/terraform_generate_planfile_test.go
📚 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/terraform_generate_planfile_test.gointernal/exec/terraform_generate_planfile.go
📚 Learning: 2025-02-03T06:00:11.419Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: cmd/describe_config.go:20-20
Timestamp: 2025-02-03T06:00:11.419Z
Learning: The `describe config` command should use `PrintErrorMarkdownAndExit` with empty title and suggestion for consistency with other commands.
Applied to files:
internal/exec/terraform_generate_planfile_test.go
📚 Learning: 2024-12-07T16:19:01.683Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 825
File: internal/exec/terraform.go:30-30
Timestamp: 2024-12-07T16:19:01.683Z
Learning: In `internal/exec/terraform.go`, skipping stack validation when help flags are present is not necessary.
Applied to files:
internal/exec/terraform_generate_planfile_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:
internal/exec/terraform_generate_planfile_test.go
📚 Learning: 2025-05-22T19:58:32.988Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected.go:122-123
Timestamp: 2025-05-22T19:58:32.988Z
Learning: The "pager" flag is defined as a PersistentFlag at the describe command level in cmd/describe.go, making it available to all subcommands including describeAffectedCmd without needing to redeclare it.
Applied to files:
internal/exec/terraform_generate_planfile_test.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:
internal/exec/terraform_generate_planfile_test.go
📚 Learning: 2024-10-31T19:23:45.538Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform.go:65-66
Timestamp: 2024-10-31T19:23:45.538Z
Learning: The variable `shouldCheckStack` in `ExecuteTerraform` controls whether validation is performed.
Applied to files:
internal/exec/terraform_generate_planfile_test.go
📚 Learning: 2025-04-26T15:54:10.506Z
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.
Applied to files:
internal/exec/terraform_generate_planfile_test.gointernal/exec/terraform_generate_planfile.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:
internal/exec/terraform_generate_planfile_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:
internal/exec/terraform_generate_planfile_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:
internal/exec/terraform_generate_planfile_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:
internal/exec/terraform_generate_planfile_test.go
📚 Learning: 2025-12-13T03:21:35.786Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1813
File: cmd/terraform/shell.go:28-73
Timestamp: 2025-12-13T03:21:35.786Z
Learning: In Atmos, when calling cfg.InitCliConfig, you must first populate the schema.ConfigAndStacksInfo struct with global flag values using flags.ParseGlobalFlags(cmd, v) rather than passing an empty struct. The LoadConfig function (pkg/config/load.go) reads config selection fields (AtmosConfigFilesFromArg, AtmosConfigDirsFromArg, BasePath, ProfilesFromArg) directly from the ConfigAndStacksInfo struct, NOT from Viper. Passing an empty struct causes config selection flags (--base-path, --config, --config-path, --profile) to be silently ignored. Correct pattern: parse flags → populate struct → call InitCliConfig. See cmd/terraform/plan_diff.go for reference implementation.
Applied to files:
internal/exec/terraform_generate_planfile.go
📚 Learning: 2024-12-12T15:17:45.245Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: examples/demo-atmos.d/atmos.d/tools/helmfile.yml:10-10
Timestamp: 2024-12-12T15:17:45.245Z
Learning: In `examples/demo-atmos.d/atmos.d/tools/helmfile.yml`, when suggesting changes to `kubeconfig_path`, ensure that the values use valid Go template syntax.
Applied to files:
internal/exec/terraform_generate_planfile.go
📚 Learning: 2024-10-20T13:12:46.499Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 736
File: pkg/config/const.go:6-6
Timestamp: 2024-10-20T13:12:46.499Z
Learning: In `cmd/cmd_utils.go`, it's acceptable to have hardcoded references to `atmos.yaml` in logs, and it's not necessary to update them to use the `CliConfigFileName` constant.
Applied to files:
internal/exec/terraform_generate_planfile.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/terraform_generate_planfile.go
📚 Learning: 2025-11-08T19:56:18.660Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1697
File: internal/exec/oci_utils.go:0-0
Timestamp: 2025-11-08T19:56:18.660Z
Learning: In the Atmos codebase, when a function receives an `*schema.AtmosConfiguration` parameter, it should read configuration values from `atmosConfig.Settings` fields rather than using direct `os.Getenv()` or `viper.GetString()` calls. The Atmos pattern is: viper.BindEnv in cmd/root.go binds environment variables → Viper unmarshals into atmosConfig.Settings via mapstructure → business logic reads from the Settings struct. This provides centralized config management, respects precedence, and enables testability. Example: `atmosConfig.Settings.AtmosGithubToken` instead of `os.Getenv("ATMOS_GITHUB_TOKEN")` in functions like `getGHCRAuth` in internal/exec/oci_utils.go.
Applied to files:
internal/exec/terraform_generate_planfile.go
📚 Learning: 2024-12-02T21:26:32.337Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.
Applied to files:
internal/exec/terraform_generate_planfile.go
🧬 Code graph analysis (2)
internal/exec/terraform_generate_planfile_test.go (2)
internal/exec/terraform_generate_planfile.go (2)
PlanfileOptions(30-30)ExecuteTerraformGeneratePlanfile(131-212)pkg/terraform/options.go (1)
PlanfileOptions(48-57)
internal/exec/terraform_generate_planfile.go (1)
pkg/schema/schema.go (2)
ConfigAndStacksInfo(692-788)AtmosConfiguration(54-99)
⏰ 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 (7)
pkg/terraform/options.go (1)
53-53: LGTM! Clean API extension.The
OutputPathfield is well-documented and logically placed. The comment is clear about its purpose for default naming convention.internal/exec/terraform_generate_planfile_test.go (2)
110-114: Proper cleanup order.Cleanup correctly removes the generated file before removing the directory. Good attention to detail.
184-201: Good test coverage for OutputPath feature.The test validates that OutputPath generates files with default naming in the specified directory. Clean assertions and proper error handling.
internal/exec/terraform_generate_planfile.go (4)
40-40: Good logging addition.Including
outputPathin debug logs aids troubleshooting. Consistent with the existing log pattern.
192-198: Clean refactoring.Introducing
planfilePathOptionsimproves readability by grouping related parameters. The struct initialization is clear and explicit.
234-240: Well-scoped internal struct.The
planfilePathOptionsstruct is appropriately unexported and clearly structured. Good encapsulation of path resolution parameters.
242-275: Solid three-mode implementation.The path resolution logic is clean and well-documented. The switch statement handles all three modes correctly:
- Mode 1: Custom file path (absolute or relative)
- Mode 2: Output directory with default naming
- Mode 3: Atmos config default
The implementation properly handles both absolute and relative paths using
filepath.IsAbsandfilepath.Join, ensuring cross-platform compatibility.Mutual exclusivity between
--fileand--output-pathis properly enforced at the CLI layer viaMarkFlagsMutuallyExclusive()incmd/terraform/generate/planfile.go:119, so no validation is needed in this function.
##what
naming
##why
manually specifying full file paths
specified
references
closes #1403
Summary by CodeRabbit
New Features
Behavior Changes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.