Add atmos terraform generate planfile command
#1214
Conversation
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
internal/exec/terraform_generate_planfile.go (1)
200-200: Inconsistent error wrapping format usage.This line uses string literal "%w: %w" directly, while elsewhere the code uses the ErrWrappingFormat constant (lines 124, 143, 190).
Update this line to use the constant for consistency:
- return fmt.Errorf("%w: %w", ErrConvertingJsonToGoType, err) + return fmt.Errorf(ErrWrappingFormat, ErrConvertingJsonToGoType, err)
🧹 Nitpick comments (1)
internal/exec/terraform_generate_planfile.go (1)
95-161: Function slightly exceeds maximum line limit - consider further extraction.The ExecuteTerraformGeneratePlanfile function is 61 lines, which exceeds the maximum of 60 lines identified by the static analyzer. This function has already been improved by extracting helper methods, but could be further streamlined.
Consider extracting the temporary directory creation and cleanup into a helper function:
+// createTempDirectory creates a temporary directory and returns a cleanup function. +func createTempDirectory() (string, func(), error) { + tmpDir, err := os.MkdirTemp("", "atmos-terraform-generate-planfile") + if err != nil { + return "", nil, fmt.Errorf(ErrWrappingFormat, ErrCreatingTempDirectory, err) + } + + cleanup := func() { + err = os.RemoveAll(tmpDir) + if err != nil { + log.Warn("Error removing temporary directory", "path", tmpDir, "error", err) + } + } + + return tmpDir, cleanup, nil +} func ExecuteTerraformGeneratePlanfile( options *PlanfileOptions, info *schema.ConfigAndStacksInfo, ) error { // Existing code... - // Create a temporary directory for all temporary files. - tmpDir, err := os.MkdirTemp("", "atmos-terraform-generate-planfile") - if err != nil { - return fmt.Errorf(ErrWrappingFormat, ErrCreatingTempDirectory, err) - } - - defer func(path string) { - err = os.RemoveAll(path) - if err != nil { - log.Warn("Error removing temporary directory", "path", path, "error", err) - } - }(tmpDir) + // Create a temporary directory for all temporary files. + tmpDir, cleanup, err := createTempDirectory() + if err != nil { + return err + } + defer cleanup() // Remaining code...🧰 Tools
🪛 GitHub Check: golangci-lint
[failure] 96-96:
function-length: maximum number of lines per function exceeded; max 60 but got 61
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
internal/exec/terraform_generate_planfile.go(1 hunks)
🧰 Additional context used
🪛 GitHub Check: golangci-lint
internal/exec/terraform_generate_planfile.go
[failure] 96-96:
function-length: maximum number of lines per function exceeded; max 60 but got 61
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Summary
🔇 Additional comments (5)
internal/exec/terraform_generate_planfile.go (5)
17-24: Well-organized error definitions with clear naming.The error variables are defined with descriptive names, making them easily identifiable and useful for error handling throughout the code.
26-35: Good structure design for PlanfileOptions.Using a dedicated struct to encapsulate the various options is a clean approach that addresses the previous issue of having too many function parameters.
39-41: Proper validation for required component argument.This validation correctly prevents potential nil pointer dereference when no component is specified.
163-173: Well-implemented format validation with sensible defaults.The validation function properly sets a default format when none is provided and validates against supported formats, which is good defensive programming.
127-132: Effective cleanup of temporary resources.The deferred function ensures temporary directories are cleaned up after use, with appropriate logging of any cleanup errors.
website/docs/cli/commands/terraform/terraform-generate-planfile.mdx
Outdated
Show resolved
Hide resolved
…e.mdx Co-authored-by: Erik Osterman (CEO @ Cloud Posse) <erik@cloudposse.com>
tests/fixtures/scenarios/terraform-generate-planfile/components/terraform/mock/main.tf
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/exec/terraform_generate_planfile.go (1)
212-212: Use the defined error format constant for consistencyThe error wrapping format string is used directly here, while other locations use the ErrWrappingFormat constant.
- return fmt.Errorf("%w: %w", ErrConvertingJsonToGoType, err) + return fmt.Errorf(ErrWrappingFormat, ErrConvertingJsonToGoType, err)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
internal/exec/terraform_generate_planfile.go(1 hunks)internal/exec/terraform_generate_planfile_test.go(1 hunks)tests/fixtures/scenarios/terraform-generate-planfile/atmos.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/fixtures/scenarios/terraform-generate-planfile/atmos.yaml
- internal/exec/terraform_generate_planfile_test.go
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Summary
🔇 Additional comments (6)
internal/exec/terraform_generate_planfile.go (6)
17-24: Errors are appropriately definedGood job defining comprehensive error variables to clarify different types of errors that might occur during execution. This improves error handling and makes debugging easier.
26-35: Well-designed options structureThe PlanfileOptions struct provides a clean way to group related parameters, solving the "too many arguments" issue identified in previous reviews. This pattern improves maintainability and readability.
37-93: Command execution function looks solidThe function thoroughly validates inputs, properly extracts flags, and safely passes pointers to the execution function. The early check for empty arguments at line 39 prevents panic conditions.
95-165: Core execution function is well-structuredThe function has been nicely refactored into smaller, focused helper functions that follow the single responsibility principle. The temporary directory is properly managed with deferred cleanup.
125-136: Good resource managementProperly creating and cleaning up the temporary directory with a deferred function call. The warning log on cleanup failure is a thoughtful addition.
167-223: Helper functions are well-designedBreaking the functionality into smaller, focused helper functions improves readability and testability. Each function has a clear purpose and appropriate error handling.
|
Hello! I am currently testing this new functionality and finding that it's hanging. Not quite sure how to debug or troubleshoot further. Suggestions? |
|
@cfsb-jrose can you re-run the command with |
what
atmos terraform generate planfilecommandwhy
Use the
atmos terraform generate planfilecommand to generate a planfile for an Atmos Terraform/OpenTofu component in a stack in JSON or YAML formats.atmos terraform generate planshould create JSON file #1172Usage
Execute the
terraform generate planfilecommand like this:This command generates a planfile for an Atmos terraform component in a stack.
Examples
Validate Terraform/OpenTofu planfiles using Checkov
You can generate a planfile for a component in a stack and validate it using Checkov.
Refer to Evaluate Checkov Policies on Terraform Plan for more information.
Summary by CodeRabbit