-
-
Notifications
You must be signed in to change notification settings - Fork 102
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve terraform
and helmfile
help. Enable Go
templating in the command
field. Clean Terraform workspace before executing terraform init
#759
Conversation
WalkthroughThis pull request includes updates to multiple files, primarily focusing on version upgrades for dependencies and enhancements to functionality and documentation related to the Atmos tool. Key changes involve updating the Atmos version in the Dockerfile, modifying various Go module dependencies, improving help message formatting, reintroducing Terraform initialization logic, and adding a new function for cleaning the Terraform workspace. Additionally, documentation has been refined to clarify templating configurations and the Atlantis integration process. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
internal/exec/help.go (1)
Line range hint
18-111
: Consider documenting new features in help messages.Given the PR objectives include new features like Go templating in the
command
field and Terraform workspace cleaning, consider adding documentation for these features in the relevant help sections:
- Add information about Go templating capabilities in the command field, explaining available template variables (
vars
,settings
,env
, etc.).- Enhance the Terraform workspace documentation to explain the new workspace cleaning behavior before
terraform init
.Example addition for the Terraform section:
u.PrintMessage(" - before executing other 'terraform' commands, 'atmos' runs 'terraform init'") +u.PrintMessage(" - before running 'terraform init', Atmos cleans the workspace by removing '.terraform/environment' to prevent backend selection prompts") +u.PrintMessage(" - the 'command' field supports Go templating with access to vars, settings, env, providers, and other configuration values")website/docs/core-concepts/stacks/templates/templates.mdx (1)
247-247
: LGTM! Consider adding an example for the newcommand
field.The addition of
command
to the list of sections supporting Go templates aligns with the PR objectives. To make this addition even more helpful, consider adding a practical example demonstrating how to use Go templates in thecommand
field, similar to how other sections are documented with examples.Example addition could look like:
command: terraform: init: description: "Initialize Terraform working directory using the specified version" command: "terraform{{ `${TERRAFORM_BINARY_VERSION:+-${TERRAFORM_BINARY_VERSION}}` }} init"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
go.sum
is excluded by!**/*.sum
website/package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (10)
examples/quick-start-advanced/Dockerfile
(1 hunks)go.mod
(5 hunks)internal/exec/help.go
(2 hunks)internal/exec/terraform.go
(2 hunks)internal/exec/terraform_utils.go
(2 hunks)internal/exec/utils.go
(2 hunks)pkg/utils/file_utils.go
(1 hunks)website/docs/core-concepts/stacks/templates/templates.mdx
(1 hunks)website/docs/integrations/atlantis.mdx
(1 hunks)website/package.json
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- website/package.json
🧰 Additional context used
📓 Learnings (2)
internal/exec/terraform_utils.go (1)
Learnt from: haitham911
PR: cloudposse/atmos#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.
pkg/utils/file_utils.go (1)
Learnt from: osterman
PR: cloudposse/atmos#731
File: pkg/utils/file_utils.go:198-202
Timestamp: 2024-10-23T20:13:23.054Z
Learning: In `pkg/utils/file_utils.go`, the current implementation of the `IsURL` function is considered sufficient; avoid suggesting more complex URL validation in future reviews.
🔇 Additional comments (18)
internal/exec/terraform_utils.go (3)
4-5
: LGTM! Clean import additions.
The new standard library imports are necessary for file operations and path manipulation.
21-26
: Well-documented function with clear purpose.
The function documentation clearly explains its purpose and the problem it solves regarding workspace selection prompts.
27-28
: Consider error handling and adding tests.
-
The current implementation silently ignores errors from
os.Remove
. While this might be intentional since the file may not exist, consider logging the error at debug level for troubleshooting purposes. -
The function would benefit from unit tests to verify its behavior:
- Test successful file deletion
- Test handling of non-existent file
- Test handling of invalid component paths
Let's check if there are any existing test files:
examples/quick-start-advanced/Dockerfile (2)
9-9
: LGTM: Version bump to include new features.
The update to Atmos version 1.100.0 aligns with the PR objectives, incorporating the new features for terraform/helmfile help improvements, Go templating support, and Terraform workspace cleaning.
9-9
: Verify version consistency across the project.
Let's ensure this version bump is consistently applied across all relevant files.
✅ Verification successful
Based on the search results, I can see that the Atmos version 1.100.0 is only referenced in the quick-start-advanced Dockerfile and there are no other version references in the codebase that would need to be synchronized. The version in the Dockerfile is used as a build argument to install a specific version of Atmos in the container image, which is an isolated use case.
No other version references need to be updated
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any inconsistent Atmos versions across the project
# Expected: All version references should be 1.100.0
echo "Checking for Atmos version references..."
rg -g '!*.md' -g '!*.mdx' -i "ATMOS_VERSION.*1\.[0-9]+\.[0-9]+"
echo -e "\nChecking package management files for version constraints..."
fd -e yaml -e yml -e json | xargs rg "atmos.*1\.[0-9]+\.[0-9]+"
Length of output: 426
Script:
#!/bin/bash
# Let's check for any other Dockerfile or configuration files that might reference Atmos
echo "Checking for Dockerfiles..."
fd Dockerfile
echo -e "\nChecking for any version references in other configuration files..."
fd -e yaml -e yml -e json -e tf -e hcl | xargs rg -i "atmos.*version|ATMOS.*VERSION"
echo -e "\nChecking for any version constraints in package files..."
fd "package.json|requirements.txt|go.mod" | xargs cat
Length of output: 17801
Script:
#!/bin/bash
# Let's check the content of the other Dockerfiles to see if they reference Atmos versions
echo "Checking content of all Dockerfiles..."
fd Dockerfile | xargs cat
echo -e "\nChecking for any version references in atmos configuration files..."
fd -e yaml -e yml -e json -g "*atmos*" | xargs cat
Length of output: 343048
pkg/utils/file_utils.go (2)
200-214
: LGTM! Good formatting improvements.
The changes improve readability and follow Go best practices by:
- Using a clearer variable name
u
to avoid shadowing theurl
package - Adding appropriate spacing for better code organization
222-236
: LGTM! Good formatting improvements.
The changes improve readability while maintaining the robust error handling for:
- Empty URLs
- Invalid URL parsing
- Invalid path extraction
internal/exec/help.go (2)
18-20
: LGTM! Consistent branding in help messages.
The capitalization of "Atmos" improves consistency with branding while maintaining clear and informative messaging.
107-111
: LGTM! Clear and consistent help message formatting.
The help message improvements maintain clear command examples while ensuring consistent branding. The structure effectively guides users through command usage.
go.mod (4)
238-238
: Security and stability improvements
The updates to the following packages typically include important security fixes and improvements:
- golang.org/x/crypto v0.28.0
- golang.org/x/net v0.30.0
- golang.org/x/text v0.19.0
- golang.org/x/time v0.7.0
Also applies to: 241-241, 246-247
100-101
: Review containerd updates
The updates to containerd packages:
- containerd v1.7.23
- errdefs v0.3.0
These changes appear to be maintenance updates. The minor version bump in errdefs might introduce new error definitions that could be leveraged for better error handling.
#!/bin/bash
# Search for containerd error handling patterns that might benefit from the update
rg 'containerd/errdefs'
14-14
: Verify compatibility with lipgloss v1.0.0
The major version bump from v0.13.1 to v1.0.0 for github.com/charmbracelet/lipgloss
might introduce breaking changes. Please ensure all existing lipgloss usage in the codebase is compatible with v1.0.0.
✅ Verification successful
Let me analyze the lipgloss usage to verify if there are any breaking changes by checking the changelog.
Based on the analysis of the lipgloss usage in the codebase, I can generate the final response:
Lipgloss v1.0.0 update appears safe to proceed
The codebase uses basic lipgloss functionality that has been stable across versions:
- Simple styling with
NewStyle()
,Padding()
,Margin()
,Height()
, andWidth()
- Basic borders with
RoundedBorder()
andHiddenBorder()
- Common layout functions like
JoinHorizontal()
andJoinVertical()
- Standard color operations using
Color()
These are core features that are unlikely to have breaking changes even in a major version bump.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for lipgloss usage patterns that might be affected by the v1.0.0 update
rg -l 'lipgloss\.' | xargs rg 'lipgloss\.'
Length of output: 2010
Script:
#!/bin/bash
# Check if there's a CHANGELOG.md in the vendor directory for lipgloss
fd -t f "CHANGELOG.md" vendor/github.com/charmbracelet/lipgloss/
# Alternatively, try to find any documentation about breaking changes
fd -t f "README.md" vendor/github.com/charmbracelet/lipgloss/
Length of output: 384
34-34
: Verify OPA compatibility
The update to github.com/open-policy-agent/opa v0.70.0
aligns with the PR objectives for improved templating support. Please verify that existing OPA policies remain compatible.
✅ Verification successful
Let me gather more information about OPA usage in the codebase.
Let me check the actual OPA policy usage patterns and validation logic to ensure compatibility.
OPA v0.70.0 update is compatible with existing policies
The existing OPA policies use standard Rego features that are stable across versions:
- Basic package declarations and imports
- Standard built-ins like
count()
,re_match()
- Common data structures and rule patterns
- No usage of experimental or version-specific features
The policies follow OPA best practices and use well-established syntax that remains compatible with v0.70.0.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for OPA policy files and usage patterns
fd -e rego
rg 'opa\.'
Length of output: 271
Script:
#!/bin/bash
# Search for OPA policy files and related code
fd -e rego
rg -l "opa" --type go
rg "opa\.New|rego\." -A 3
Length of output: 1500
Script:
#!/bin/bash
# Check OPA policy content and validation patterns
cat examples/quick-start-advanced/stacks/schemas/opa/vpc/validate-vpc-component.rego
cat examples/tests/stacks/schemas/opa/test-component/validate-test-component.rego
# Check for any version-specific OPA features or syntax
rg "package\s+|import\s+|default\s+" examples/quick-start-advanced/stacks/schemas/opa/ examples/tests/stacks/schemas/opa/
Length of output: 5316
internal/exec/terraform.go (1)
263-274
: LGTM! The init control flow logic is well-structured.
The conditions for running or skipping terraform init
are comprehensive and properly handle all edge cases.
website/docs/core-concepts/stacks/templates/templates.mdx (1)
Line range hint 1-24
: Documentation is comprehensive and well-structured!
The documentation effectively covers all aspects of stack manifest templating, including:
- Clear configuration instructions
- Comprehensive examples
- Detailed explanations of template processing
- Important warnings and gotchas
website/docs/integrations/atlantis.mdx (1)
676-676
: LGTM! Verify version consistency.
The Atmos version update from 1.99.0 to 1.100.0 looks good.
Let's verify that this version is consistent across the codebase:
✅ Verification successful
Version update is consistent across the codebase
The Atmos version update from 1.99.0 to 1.100.0 has been verified in both locations where it appears:
website/docs/integrations/atlantis.mdx
: ATMOS_VERSION: 1.100.0examples/quick-start-advanced/Dockerfile
: ARG ATMOS_VERSION=1.100.0
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any other occurrences of Atmos version numbers to ensure consistency
rg -g '!website/docs/integrations/atlantis.mdx' "ATMOS_VERSION.*1\.(99|100)\.0"
Length of output: 148
Script:
#!/bin/bash
# Let's search for any other version patterns to ensure we haven't missed anything
rg -g '!{website/docs/integrations/atlantis.mdx,examples/quick-start-advanced/Dockerfile}' "atmos.*1\.(99|100)\.0" -i
Length of output: 119
internal/exec/utils.go (2)
Line range hint 512-548
: LGTM! Clean implementation of manifest section processing.
The code follows a consistent pattern for safely processing each Atmos manifest section after Go template processing, with proper type assertions and default values.
549-551
: LGTM! Command field processing enables Go templating support.
The implementation correctly enables Go templating in the command field, allowing dynamic configurations as specified in the PR objectives.
Let's verify the command field usage in stack configurations:
✅ Verification successful
Command field processing implementation is correctly integrated
Based on the codebase analysis, the command field processing is properly implemented and follows the established patterns:
- The code correctly processes command fields from component sections using the same pattern as other sections
- Command field values are properly extracted and type-checked across different component types (terraform, helmfile)
- The implementation supports both direct command values and inheritance from base components
- While no explicit Go template usage was found in the current configurations, the processing code allows for template interpolation as it handles string values
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find examples of command field usage with Go templates in stack configurations
# Test: Search for command field usage with Go template syntax
rg -A 5 'command:\s*\{\{.*\}\}'
Length of output: 33
Script:
#!/bin/bash
# Let's try a broader search for command field usage in yaml/json files
fd -e yaml -e yml -e json --exec rg -l 'command:' {}
# Then let's look at the actual command field implementations
fd -e yaml -e yml -e json --exec cat {} | grep -A 2 'command:'
# Also search for test files that might contain command field examples
fd -e go | xargs rg -l 'command.*test'
# And check the actual test implementations
ast-grep --pattern 'func Test$_($_) {
$$$
command $$$
$$$
}'
Length of output: 11432
Script:
#!/bin/bash
# Let's check test files for command field templating examples
rg -l "command.*\{\{.*\}\}" internal/exec/
# Look for command field processing tests
ast-grep --pattern 'func Test$_(t *testing.T) {
$$$
command := $$$
$$$
}'
# Check for template processing related to command field
rg -A 3 "CommandSectionName|Command.*template" internal/exec/
Length of output: 4720
These changes were released in v1.100.0. |
what
terraform
andhelmfile
helpGo
templating in thecommand
field of stack configterraform init
why
Improve the help messages. When a user executes
atmos terraform --help
oratmos helmfile --help
(or help for a subcommand), print a message describing the command and how to execute theterraform
andhelmfile
help commandEnable Go templating in the
command
stack config in addition to the already supported sections.You can now use
Go
templates in the following Atmos sections to refer to values in the same or other sections:vars
settings
env
providers
overrides
backend
backend_type
component
metadata.component
command
Enabling
Go
templates in thecommand
section allows specifying different Terraform/OpenTofu/Helmfile versions per component/stack, and get the value from different Atmos sections or from external data sourcesClean Terraform workspace before executing
terraform init
. When using multiple backends for the same component (e.g. separate backends per tenant or account), and if an Atmos command was executed that selected a Terraform workspace, Terraform will prompt the user to select one of the following workspaces:The prompt forces the user to always make a selection (which is error-prone), and also makes it complicated when running on CI/CD.
The PR adds the logic that deletes the
.terraform/environment
file from the component directory before executingterraform init
. The.terraform/environment
file contains the name of the currently selected workspace, helping Terraform identify the active workspace context for managing your infrastructure. We delete the file before executingterraform init
to prevent the Terraform prompt asking to select the default or the previously used workspace.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores