Process Go templates and Atmos YAML functions only when executing atmos terraform commands that require it
#1023
Conversation
|
Important Cloud Posse Engineering Team Review RequiredThis pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes. To expedite this process, reach out to us on Slack in the |
📝 WalkthroughWalkthroughThis pull request introduces changes to various command and internal files. A new Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant TR as terraformRun
participant AC as actualCmd.Usage
participant Log as Logger
User->>TR: Invoke Terraform command
TR->>AC: Call actualCmd.Usage()
AC-->>TR: Return usage result or error
alt Error Occurs
TR->>Log: Log the error
else No Error
TR->>User: Proceed with command execution
end
sequenceDiagram
participant Exec as ExecuteTerraform
participant Utils as Terraform Utils
participant FS as Filesystem/Logger
Exec->>Utils: needProcessTemplatesAndYamlFunctions(command)
alt Processing required
Utils-->>Exec: true
Exec->>Utils: shouldProcessStacks(info)
Utils-->>Exec: Return processing flags
Exec->>Utils: generateBackendConfig(atmosConfig, info, workingDir)
Exec->>Utils: generateProviderOverrides(atmosConfig, info, workingDir)
Utils->>FS: Write config files (unless dry run)
else Not required
Utils-->>Exec: false
end
Possibly related PRs
Suggested reviewers
Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cmd/cmd_utils.go (1)
609-617: Consider usingslices.Containsfor better maintainability.The implementation is correct, but since Go 1.21+, you can use the standard library's
slices.Containsfunction for a more idiomatic solution.-// Contains checks if a slice of strings contains an exact match for the target string. -func Contains(slice []string, target string) bool { - for _, item := range slice { - if item == target { - return true - } - } - return false -} +import "slices" + +// Contains checks if a slice of strings contains an exact match for the target string. +func Contains(slice []string, target string) bool { + return slices.Contains(slice, target) +}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (8)
cmd/cmd_utils.go(1 hunks)cmd/terraform.go(3 hunks)cmd/terraform_commands.go(1 hunks)examples/quick-start-advanced/Dockerfile(1 hunks)go.mod(6 hunks)internal/exec/terraform.go(1 hunks)internal/exec/terraform_utils.go(1 hunks)website/docs/integrations/atlantis.mdx(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- cmd/terraform_commands.go
- website/docs/integrations/atlantis.mdx
🧰 Additional context used
📓 Learnings (1)
examples/quick-start-advanced/Dockerfile (2)
Learnt from: aknysh
PR: cloudposse/atmos#775
File: examples/quick-start-advanced/Dockerfile:9-9
Timestamp: 2024-11-12T05:52:05.088Z
Learning: It is acceptable to set `ARG ATMOS_VERSION` to a future version like `1.105.0` in `examples/quick-start-advanced/Dockerfile` if that will be the next release.
Learnt from: osterman
PR: cloudposse/atmos#801
File: examples/quick-start-advanced/Dockerfile:9-9
Timestamp: 2024-11-23T00:13:22.004Z
Learning: When updating the `ATMOS_VERSION` in Dockerfiles, the team prefers to pin to the next future version when the PR merges, even if the version is not yet released.
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build (macos-latest, macos)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Summary
🔇 Additional comments (11)
internal/exec/terraform.go (3)
56-57: Looks solid.
This newly added comment and function call help clarify why stack processing can be skipped when cleaning.
60-61: Good approach to selectively process templates.
This aligns with the PR’s goal of processing templates and YAML functions only on certain Terraform subcommands.
67-67: Error message update is reasonable.
It’s now more general and consistent with the new usage flow.cmd/terraform.go (2)
6-6: Neat reorganization of imports and flags.
UsingDisableFlagParsingand persistent flags allows for custom handling of Terraform arguments.Also applies to: 24-29
54-66: Improved help and error handling.
Capturing and loggingUsage()errors is a nice defensive measure. Clean integration withExecuteTerraform.internal/exec/terraform_utils.go (4)
33-46: Simple, readable logic for stack processing.
This function clarifies the conditions to skip or check stack usage, aiding maintainability.
48-70: Pragmatic backend config generation.
Auto-generating thebackend.tf.jsonis concise. Consider unit testing for edge cases like invalid workspace.
72-87: Provider overrides handled well.
Keeping it JSON-based ensures consistency. Suggest verifying no sensitive data is unintentionally logged.
89-100: Command filtering improves performance.
Only processing templates for the listed commands helps meet the PR’s main objective efficiently.examples/quick-start-advanced/Dockerfile (1)
9-9: LGTM! Version update aligns with team preferences.The update to
ATMOS_VERSION=1.160.0is consistent with the team's practice of pinning to future versions when the PR merges.go.mod (1)
12-14: LGTM! Routine dependency updates.The dependency version updates are minor version bumps that keep the project up to date with the latest stable releases.
Also applies to: 16-16, 59-60, 95-95, 109-109, 122-122, 295-295
|
These changes were released in v1.160.0. |
…atmos terraform` commands that require it (#1023) * updates * updates * updates * updates
what
Gotemplates and Atmos YAML functions only when executingatmos terraformcommands that require itterraform_utils.gowhy
Only the following
atmos terraformcommands require processing ofGotemplates and Atmos YAML functions in Atmos manifests:planapplydeploydestroygenerateAll other
atmos terraformcommand don't need the outputs of the templates and functions (e.g.atmos terraform initoratmos terraform workspace)Speed up the commands execution
Some YAML functions like
!terraform.outputrequire assuming roles, and not executing it simplifies CI/CD workflowsSummary by CodeRabbit
New Features
Bug Fixes
Chores