-
-
Notifications
You must be signed in to change notification settings - Fork 105
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.output
Atmos YAML function. Implement static
remote state backend for !terraform.output
and atmos.Component
functions
#863
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 |
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces updates to the Atmos tool, primarily focusing on upgrading the Atmos version and enhancing support for static remote state backends. Key changes include modifications to the Dockerfile, the addition of new variables in YAML configuration files, and updates to various functions to incorporate stack context. Documentation improvements are also made to clarify the use of the Changes
Possibly related PRs
Suggested Reviewers
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (3)
internal/exec/yaml_func_utils.go (1)
Line range hint
19-57
: Consider adding error handling for stack contextWhile the implementation looks solid, consider adding validation for the
currentStack
parameter to handle edge cases.func processNodes( cliConfig schema.CliConfiguration, data map[string]any, currentStack string, ) map[string]any { + if currentStack == "" { + log.Debug("Processing nodes with empty stack context") + } newMap := make(map[string]any)website/docs/core-concepts/stacks/yaml-functions/template.mdx (1)
143-185
: Excellent practical example with a minor style issue!The Security Group ingress example effectively demonstrates the DRY principle and practical usage. However, there's a minor style issue to address.
Add a comma after "e.g." in the following line:
-For example, suppose we need to restrict the Security Group ingresses on all components provisioned in the infrastructure (e.g. EKS cluster, RDS Aurora cluster, MemoryDB cluster, Istio Ingress Gateway) to a specific list of IP CIDR blocks. +For example, suppose we need to restrict the Security Group ingresses on all components provisioned in the infrastructure (e.g., EKS cluster, RDS Aurora cluster, MemoryDB cluster, Istio Ingress Gateway) to a specific list of IP CIDR blocks.🧰 Tools
🪛 LanguageTool
[style] ~146-~146: A comma is missing here.
Context: ...ents provisioned in the infrastructure (e.g. EKS cluster, RDS Aurora cluster, Memory...(EG_NO_COMMA)
[style] ~181-~181: Using many exclamation marks might seem excessive (in this case: 10 exclamation marks for a text that’s 4062 characters long)
Context: ...lowed_ingress_cidrs }}' ```
The!template
function and the `'{{ toJson ....(EN_EXCESSIVE_EXCLAMATION)
website/docs/core-concepts/stacks/templates/functions/atmos.Component.mdx (1)
286-286
: Add a period at the end of the section header.The section header should end with proper punctuation.
-## Using `atmos.Component` with `static` remote state backend +## Using `atmos.Component` with `static` remote state backend.🧰 Tools
🪛 LanguageTool
[grammar] ~286-~286: Please add a punctuation mark at the end of paragraph.
Context: ...s.Componentwith
static` remote state backend Atmos supports [brownfield configurati...(PUNCTUATION_PARAGRAPH_END)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
website/package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (19)
examples/quick-start-advanced/Dockerfile
(1 hunks)examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml
(1 hunks)examples/tests/stacks/catalog/terraform/template-functions-test3/defaults.yaml
(1 hunks)examples/tests/stacks/orgs/cp/tenant1/prod/us-east-2.yaml
(1 hunks)internal/exec/describe_stacks.go
(2 hunks)internal/exec/stack_utils.go
(2 hunks)internal/exec/template_funcs_component.go
(2 hunks)internal/exec/terraform_generate_backends.go
(1 hunks)internal/exec/terraform_generate_varfiles.go
(1 hunks)internal/exec/utils.go
(2 hunks)internal/exec/yaml_func_exec.go
(1 hunks)internal/exec/yaml_func_template.go
(1 hunks)internal/exec/yaml_func_terraform_output.go
(3 hunks)internal/exec/yaml_func_utils.go
(2 hunks)website/docs/core-concepts/stacks/templates/functions/atmos.Component.mdx
(2 hunks)website/docs/core-concepts/stacks/yaml-functions/template.mdx
(1 hunks)website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx
(7 hunks)website/docs/integrations/atlantis.mdx
(1 hunks)website/package.json
(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- examples/tests/stacks/orgs/cp/tenant1/prod/us-east-2.yaml
- examples/tests/stacks/catalog/terraform/template-functions-test3/defaults.yaml
🧰 Additional context used
📓 Learnings (2)
internal/exec/yaml_func_terraform_output.go (1)
Learnt from: aknysh
PR: cloudposse/atmos#810
File: internal/exec/yaml_func_terraform_output.go:35-40
Timestamp: 2024-11-30T22:07:08.610Z
Learning: In the Go function `processTagTerraformOutput` in `internal/exec/yaml_func_terraform_output.go`, parameters cannot contain spaces. The code splits the input by spaces, and if the parameters contain spaces, `len(parts) != 3` will fail and show an error to the user.
examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml (1)
Learnt from: aknysh
PR: cloudposse/atmos#810
File: examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml:28-32
Timestamp: 2024-12-01T00:33:20.298Z
Learning: In `examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml`, `!exec atmos terraform output` is used in examples to demonstrate its usage, even though `!terraform.output` is the recommended approach according to the documentation.
🪛 LanguageTool
website/docs/core-concepts/stacks/yaml-functions/template.mdx
[style] ~146-~146: A comma is missing here.
Context: ...ents provisioned in the infrastructure (e.g. EKS cluster, RDS Aurora cluster, Memory...
(EG_NO_COMMA)
[style] ~181-~181: Using many exclamation marks might seem excessive (in this case: 10 exclamation marks for a text that’s 4062 characters long)
Context: ...lowed_ingress_cidrs }}' ```
The !template
function and the `'{{ toJson ....
(EN_EXCESSIVE_EXCLAMATION)
website/docs/core-concepts/stacks/templates/functions/atmos.Component.mdx
[grammar] ~286-~286: Please add a punctuation mark at the end of paragraph.
Context: ...s.Componentwith
static` remote state backend Atmos supports [brownfield configurati...
(PUNCTUATION_PARAGRAPH_END)
🪛 golangci-lint (1.62.2)
internal/exec/yaml_func_terraform_output.go
50-50: S1028: should use fmt.Errorf(...) instead of errors.New(fmt.Sprintf(...))
(gosimple)
🔇 Additional comments (24)
internal/exec/template_funcs_component.go (1)
47-65
: Solid implementation of output handling
The updated logic correctly retrieves outputs based on the remote state backend configuration.
internal/exec/yaml_func_template.go (1)
12-16
: Consistent function signature update
Adding the currentStack
parameter ensures proper stack context handling.
internal/exec/yaml_func_exec.go (1)
12-16
: Consistent function signature inclusion
Including currentStack
aligns this function with others for consistent context management.
internal/exec/yaml_func_utils.go (2)
11-16
: LGTM! Enhanced stack context handling
The addition of currentStack
parameter improves the function's ability to handle stack-specific operations, which is crucial for the new !terraform.output
functionality.
Line range hint 58-73
: Verify tag processor implementations
The changes correctly propagate stack context to all tag processors. Let's verify the implementations of these processors.
✅ Verification successful
Tag processors properly handle stack context
The verification confirms that all three tag processors (Template
, Exec
, and TerraformOutput
) correctly handle the currentStack
parameter:
Template
andExec
processors maintain the stack context for potential downstream operationsTerraformOutput
explicitly usescurrentStack
as a fallback when only two parameters are provided, demonstrating proper stack context propagation- All processors follow consistent error handling patterns and input processing
The implementation is solid and maintains stack context as intended.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check implementation of tag processors
ast-grep --pattern 'func processTag(Template|Exec|TerraformOutput)($_, $_, $_) {
$$$
}'
Length of output: 91
Script:
#!/bin/bash
# Let's try a different approach to find the tag processor implementations
# First, let's search for processTagTemplate
rg "func processTagTemplate" -A 10
# Then search for processTagExec
rg "func processTagExec" -A 10
# Finally, search for processTagTerraformOutput
rg "func processTagTerraformOutput" -A 10
Length of output: 2313
Script:
#!/bin/bash
# Let's get more context about how these processors handle the currentStack parameter
# Search for currentStack usage in all three files
rg "currentStack" internal/exec/yaml_func_template.go -A 5 -B 5
rg "currentStack" internal/exec/yaml_func_exec.go -A 5 -B 5
rg "currentStack" internal/exec/yaml_func_terraform_output.go -A 5 -B 5
Length of output: 1635
Script:
#!/bin/bash
# Let's check the implementation details of each processor to see how they handle the input
rg -A 20 "str, err := getStringAfterTag" internal/exec/yaml_func_template.go
rg -A 20 "str, err := getStringAfterTag" internal/exec/yaml_func_exec.go
rg -A 20 "str, err := getStringAfterTag" internal/exec/yaml_func_terraform_output.go
Length of output: 1519
website/package.json (1)
Line range hint 24-40
: Verify dependency compatibility
The dependency updates appear to be minor version bumps. Let's verify compatibility and check for any security advisories.
✅ Verification successful
Let me analyze the dependency usage and resolved versions to ensure compatibility. The previous scan showed no security issues with the current version of marked (15.0.4), as all vulnerabilities were in much older versions. Let's verify the actual usage and integration of these updated packages.
Dependencies are compatible and secure
The updated dependencies show no compatibility issues:
- All packages have clean resolution in package-lock.json with proper integrity hashes
- Marked v15.0.4 is well above any known vulnerable versions
- @mdx-js/react v3.1.0 maintains proper peer dependencies with React
- All FontAwesome packages are at consistent v6.7.2
- PostHog packages have compatible versions with required dependencies
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for security advisories in updated packages
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "marked") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
}
}
}'
Length of output: 1119
Script:
#!/bin/bash
# Let's check for usage of these dependencies in the codebase
rg -l "@fortawesome|@mdx-js|marked|posthog" --type js --type jsx --type ts --type tsx
# Check package-lock.json for actual resolved versions
fd package-lock.json --exec cat {} | jq '.packages | ."node_modules/@fortawesome/fontawesome-svg-core", ."node_modules/@mdx-js/react", ."node_modules/marked", ."node_modules/posthog-js"'
Length of output: 1860
examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml (3)
34-37
: Well structured demonstration of the two-parameter syntax!
The examples effectively showcase the simplified !terraform.output
syntax, making it clearer and more concise than the three-parameter version.
38-45
: Excellent demonstration of static remote state backend usage!
The examples comprehensively cover various test cases for the static remote state backend configuration, which is crucial for brownfield configurations.
46-47
: Good error case coverage!
The commented test cases effectively demonstrate how the function handles non-existent components and invalid values.
website/docs/core-concepts/stacks/yaml-functions/template.mdx (2)
140-142
: Clear section organization!
The introduction of the Advanced Examples section enhances the documentation structure.
186-193
: Helpful tip with practical example!
The tip about appending additional CIDRs provides valuable information for real-world scenarios.
internal/exec/stack_utils.go (1)
57-58
: Improved comment readability!
The multi-line formatting enhances the comment's readability.
website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx (4)
18-22
: LGTM! Clear and concise function signature documentation.
The documentation clearly explains that the function can be called with either two or three parameters, which aligns with the PR objectives.
58-60
: LGTM! Well-structured examples.
The examples effectively demonstrate both forms of the function call, using both hardcoded values and template variables.
107-112
: LGTM! Important clarification about stack identifiers.
The note effectively explains the relationship between using stack identifiers and the two-parameter function call.
189-250
: LGTM! Comprehensive documentation of static remote state backend.
The new section thoroughly documents:
- The concept of static remote state backend
- Example configuration with various data types
- Expected behavior and output
internal/exec/terraform_generate_varfiles.go (1)
272-272
: LGTM! Stack-aware YAML tag processing.
The addition of stackName
parameter to ProcessCustomYamlTags
aligns with the PR objectives for improved stack context handling.
internal/exec/terraform_generate_backends.go (1)
256-256
: LGTM! Consistent implementation of stack-aware processing.
The addition of stackName
parameter to ProcessCustomYamlTags
maintains consistency with other files and supports the PR objectives.
website/docs/core-concepts/stacks/templates/functions/atmos.Component.mdx (1)
285-346
: LGTM! Well-documented static remote state backend feature.
The new section provides clear examples and explanations for using atmos.Component
with static remote state backend. The documentation is comprehensive and includes practical examples.
🧰 Tools
🪛 LanguageTool
[grammar] ~286-~286: Please add a punctuation mark at the end of paragraph.
Context: ...s.Componentwith
static` remote state backend Atmos supports [brownfield configurati...
(PUNCTUATION_PARAGRAPH_END)
internal/exec/describe_stacks.go (1)
365-367
: LGTM! Consistent implementation of stack name parameter.
The stackName
parameter has been correctly added to both calls of ProcessCustomYamlTags
, maintaining consistency throughout the implementation.
Also applies to: 550-552
website/docs/integrations/atlantis.mdx (1)
676-677
: LGTM! Updated Atmos version.
The Atmos version has been updated from 1.127.0 to 1.129.0, keeping the documentation in sync with the latest release.
internal/exec/utils.go (3)
401-403
: LGTM! Improved error handling for stackless operations.
The change allows the function to proceed without error when checkStack
is false, which is appropriate for operations that don't require a stack context.
Line range hint 406-424
: LGTM! Enhanced error messaging with improved context.
The error messages now provide more detailed context about stack and component relationships, with additional CLI config details in trace mode. This improvement will help users better understand and debug stack-related issues.
522-524
: LGTM! Enhanced YAML tag processing with stack context.
The ProcessCustomYamlTags
function now receives the stack name, enabling stack-aware YAML tag processing. This change supports the new static remote state backend feature.
Let's verify the usage of the new stack parameter:
website/docs/core-concepts/stacks/templates/functions/atmos.Component.mdx
Outdated
Show resolved
Hide resolved
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: 0
🧹 Nitpick comments (2)
website/docs/core-concepts/stacks/templates/functions/atmos.Component.mdx (2)
286-286
: Add missing period at the end of the section header.For consistency with other sections, add a period after "backend".
-## Using `atmos.Component` with `static` remote state backend +## Using `atmos.Component` with `static` remote state backend.🧰 Tools
🪛 LanguageTool
[grammar] ~286-~286: Please add a punctuation mark at the end of paragraph.
Context: ...s.Componentwith
static` remote state backend Atmos supports [brownfield configurati...(PUNCTUATION_PARAGRAPH_END)
288-289
: Consider expanding the brownfield configuration context.While the link to brownfield configuration is helpful, it would be valuable to briefly explain why someone might want to use a static backend in a brownfield scenario. For example, you could add:
"Static backends are particularly useful in brownfield scenarios where you want to reference existing infrastructure without importing it into Terraform's state. This allows you to gradually adopt infrastructure as code while maintaining access to existing resource information."
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
website/docs/core-concepts/stacks/templates/functions/atmos.Component.mdx
(2 hunks)website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx
(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx
🧰 Additional context used
🪛 LanguageTool
website/docs/core-concepts/stacks/templates/functions/atmos.Component.mdx
[grammar] ~286-~286: Please add a punctuation mark at the end of paragraph.
Context: ...s.Componentwith
static` remote state backend Atmos supports [brownfield configurati...
(PUNCTUATION_PARAGRAPH_END)
🔇 Additional comments (2)
website/docs/core-concepts/stacks/templates/functions/atmos.Component.mdx (2)
9-9
: LGTM! Terminal component import added correctly.
The Terminal component import is well-placed and properly utilized in the documentation.
292-348
: Excellent example demonstrating static backend usage!
The example effectively shows:
- Proper configuration of static backend
- Handling of both simple and complex data types
- Clear demonstration of command output
These changes were released in v1.129.0. |
what
Improve
!terraform.output
Atmos YAML functionImplement
static
remote state backend for!terraform.output
andatmos.Component
functionsAdd more advanced usage examples of the
!template
Atmos YAML functionFix the error messages when the user specifies an invalid component names in
atmos terraform plan/apply
Update docs
https://pr-863.atmos-docs.ue2.dev.plat.cloudposse.org/core-concepts/stacks/yaml-functions/terraform.output/
https://pr-863.atmos-docs.ue2.dev.plat.cloudposse.org/core-concepts/stacks/yaml-functions/template/
https://pr-863.atmos-docs.ue2.dev.plat.cloudposse.org/core-concepts/stacks/templates/functions/atmos.Component/
why
Improve
!terraform.output
Atmos YAML functionThe
!template
Atmos YAML function now can be called with either two or three parameters:Examples:
NOTE: Using the
.stack
or.atmos_stack
template identifiers to specify the stack is the same as calling the!template
function with two parameters without specifying the current stack, but without usingGo
templates.If you need to get an output of a component in the current stack, using the
!template
function with two parametersis preferred because it has a simpler syntax and executes faster.
Implement
static
remote state backend for!terraform.output
andatmos.Component
functionsAtmos supports brownfield configuration by using the remote state of type
static
.For example:
When the functions are executed, Atmos detects that the
test
component has thestatic
remote state configured,and instead of executing
terrafrom output
, it just returns the static values from theremote_state_backend.static
section.Executing the command
atmos describe component test2 -s <stack>
produces the following result:Add more advanced usage examples of the
!template
Atmos YAML functionThe
!template
Atmos YAML function can be used to make your stack configuration DRY and reusable.For example, suppose we need to restrict the Security Group ingresses on all components provisioned in the infrastructure
(e.g. EKS cluster, RDS Aurora cluster, MemoryDB cluster, Istio Ingress Gateway) to a specific list of IP CIDR blocks.
We can define the list of allowed CIDR blocks in the global
settings
section (used by all components in all stacks)in the
allowed_ingress_cidrs
variable:We can then use the
!template
function with the following template in all components that need their Security Groupto be restricted:
The
!template
function and the'{{ toJson .settings.allowed_ingress_cidrs }}'
expression allows you touse the global
allowed_ingress_cidrs
variable and the same template even if the components have differentvariable names for the allowed CIDR blocks (which would be difficult to implement using
Atmos inheritance or other Atmos design patterns).
NOTE:
To append additional CIDRs to the template itself, use the
list
and Sprigconcat
functions:Summary by CodeRabbit
Release Notes
New Features
template-functions-test2
and a new configuration fortemplate-functions-test3
.atmos.Component
and!template
functions, including examples for static remote state backend usage.Bug Fixes
Documentation
Chores