Conversation
📝 WalkthroughWalkthroughThe pull request modifies the handling of Terraform command-line arguments and environment variables within the Atmos shell utility functions. It introduces a structured approach to manage Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (4)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/exec/terraform.go (1)
262-264: Catching var-file collisions in component environment ensures thorough coverage.
These lines do a great job of warning users when environment-level settings might be overridden.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/exec/shell_utils.go(1 hunks)internal/exec/terraform.go(1 hunks)
🔇 Additional comments (4)
internal/exec/shell_utils.go (3)
188-190: Neat definition of terraform commands.
This approach centralizes the commands that may require var-file handling, simplifying future maintenance and extension.
191-197: Clear warning message for potential collisions.
It is helpful to log a warning whenever these TF_CLI_ARGS variables contain a “-var-file” argument, reducing user confusion.
198-200: Appending the var file reference to each command is straightforward.
Ensure users understand that this logic overrides any existing “-var-file” specification at runtime.
internal/exec/terraform.go (1)
248-257: Collisions detection is consistent with shell_utils.
Capturing conflicts across both the shell and terraform commands maintains consistent user visibility.
|
Here's the misunderstanding.
Here are the new requirements
|
TF_CLI_ARGS_* Handling for -var-fileTF_CLI_ARGS_* Handling
Co-authored-by: Erik Osterman (CEO @ Cloud Posse) <erik@cloudposse.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/exec/shell_utils.go (1)
356-385: Well-structured environment variable merging function!The implementation is clean and handles the merging of environment variables effectively. However, consider adding error handling for malformed environment variables.
func mergeEnvVars(baseEnv, overrideEnv []string) []string { envMap := make(map[string]string) // Parse base environment variables for _, env := range baseEnv { if parts := strings.SplitN(env, "=", 2); len(parts) == 2 { envMap[parts[0]] = parts[1] + } else { + // Log malformed environment variables + u.LogWarning(atmosConfig, fmt.Sprintf("Skipping malformed environment variable: %s", env)) } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
internal/exec/shell_utils.go(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Summary
🔇 Additional comments (3)
internal/exec/shell_utils.go (3)
281-283: Excellent use of environment variable merging!The approach ensures that component-specific variables take precedence while preserving system environment variables.
290-298: Good debugging support!The debug logging of merged environment variables helps with troubleshooting.
230-251: Strong approach to handling TF_CLI_ARGS!The implementation correctly preserves existing TF_CLI_ARGS values while appending Atmos-specific var-file arguments. The warning message helps users understand potential conflicts.
However, consider adding validation for the existing arguments to prevent invalid combinations:
|
These changes were released in v1.139.0. |
what
TF_CLI_*environment variable, since this may conflict with Atmos behavior.terraform shelland to anyterraformcommand, since these are handled separatelyterraform shellwhy
references
atmos terraform shell
atmos terraform plan
Summary by CodeRabbit
Summary by CodeRabbit
New Features
-var-filearguments to respect existing configurations.Bug Fixes