Fix atmos terraform shell command. Improve !terraform.output YAML function#1252
Fix atmos terraform shell command. Improve !terraform.output YAML function#1252
atmos terraform shell command. Improve !terraform.output YAML function#1252Conversation
📝 WalkthroughWalkthroughThis update refreshes Go dependencies and the Go version in Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Config
participant EnvMerger
participant Logger
User->>CLI: Run command
CLI->>Config: Initialize CLI config (once)
CLI->>EnvMerger: Merge system and Atmos env vars
EnvMerger-->>CLI: Merged env vars
CLI->>Logger: Log errors using charmbracelet/log
CLI->>User: Output results
Possibly related PRs
Suggested reviewers
Tip ⚡️ Free AI Code Reviews for VS Code, Cursor, Windsurf
📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 1
🧹 Nitpick comments (1)
website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx (1)
78-124: Excellent documentation of default value support.This new section thoroughly explains the default value feature enhancement. Key strengths:
- Clear explanation of the problem being solved (handling non-provisioned components)
- Detailed notes on the YQ operator syntax and quoting requirements
- Comprehensive examples covering string, list, and map default values with properly escaped quotes
- Links to relevant YQ documentation for further reading
Minor grammar improvements needed:
- Add punctuation at the end of paragraphs on lines 99, 107, and 115
- Consider rephrasing "This will allow you" to "This allows you" on line 83
- If the `config` component has not been provisioned yet, return the default value `default-user` + If the `config` component has not been provisioned yet, return the default value `default-user`. - If the `vpc` component has not been provisioned yet, return the default value `["mock-subnet1", "mock-subnet2"]` + If the `vpc` component has not been provisioned yet, return the default value `["mock-subnet1", "mock-subnet2"]`. - If the `config` component has not been provisioned yet, return the default value `{"api_endpoint": "localhost:3000", "user": "test"}` + If the `config` component has not been provisioned yet, return the default value `{"api_endpoint": "localhost:3000", "user": "test"}`. -This will allow you to mock outputs when executing `atmos terraform plan` +This allows you to mock outputs when executing `atmos terraform plan`🧰 Tools
🪛 LanguageTool
[style] ~83-~83: Future tense may not be necessary in this context.
Context: ...unction will return the default value. This will allow you to mock outputs when executing `atm...(WILL_ALLOW)
[grammar] ~99-~99: Please add a punctuation mark at the end of paragraph.
Context: ...een provisioned yet, return the default valuedefault-user```yaml username: !terr...(PUNCTUATION_PARAGRAPH_END)
[grammar] ~107-~107: Please add a punctuation mark at the end of paragraph.
Context: ...een provisioned yet, return the default value["mock-subnet1", "mock-subnet2"]```...(PUNCTUATION_PARAGRAPH_END)
[grammar] ~115-~115: Please add a punctuation mark at the end of paragraph.
Context: ...een provisioned yet, return the default value `{"api_endpoint": "localhost:3000", "us...(PUNCTUATION_PARAGRAPH_END)
📜 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 (9)
go.mod(11 hunks)internal/exec/describe_component.go(1 hunks)internal/exec/shell_utils.go(4 hunks)internal/exec/yaml_func_terraform_output.go(1 hunks)internal/exec/yaml_func_terraform_output_test.go(1 hunks)pkg/component/component_processor.go(4 hunks)tests/fixtures/scenarios/atmos-terraform-output-yaml-function/stacks/deploy/nonprod.yaml(2 hunks)website/docs/core-concepts/stacks/yaml-functions/store.mdx(1 hunks)website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
internal/exec/yaml_func_terraform_output.go (2)
Learnt from: aknysh
PR: cloudposse/atmos#863
File: internal/exec/yaml_func_terraform_output.go:34-38
Timestamp: 2024-12-17T07:08:41.288Z
Learning: In the `processTagTerraformOutput` function within `internal/exec/yaml_func_terraform_output.go`, parameters are separated by spaces and do not contain spaces. Therefore, using `strings.Fields()` for parsing is acceptable, and there's no need to handle parameters with spaces.
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.
website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx (2)
Learnt from: aknysh
PR: cloudposse/atmos#810
File: website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx:0-0
Timestamp: 2024-12-03T04:01:16.446Z
Learning: In the `terraform.output.mdx` documentation file (`website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx`), the cache invalidation and cache scope behavior for the `!terraform.output` function are already described.
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.
pkg/component/component_processor.go (1)
Learnt from: Listener430
PR: cloudposse/atmos#844
File: cmd/helmfile.go:37-37
Timestamp: 2024-12-11T18:40:12.808Z
Learning: In the atmos project, `cliConfig` is initialized within the `cmd` package in `root.go` and can be used in other command files.
🧬 Code Graph Analysis (2)
internal/exec/yaml_func_terraform_output.go (1)
pkg/utils/string_utils.go (1)
SplitStringByDelimiter(24-43)
pkg/component/component_processor.go (2)
internal/exec/stack_utils.go (1)
GetStackNamePattern(188-190)pkg/config/utils.go (1)
GetStackNameFromContextAndStackNamePattern(702-758)
🪛 GitHub Check: golangci-lint
internal/exec/yaml_func_terraform_output.go
[failure] 34-34:
deep-exit: calls to log.Fatal only in main() or init() functions
🪛 LanguageTool
website/docs/core-concepts/stacks/yaml-functions/store.mdx
[style] ~207-~207: Using many exclamation marks might seem excessive (in this case: 11 exclamation marks for a text that’s 3777 characters long)
Context: ...tore may not yet be available and the !store function call will fail unless yo...
(EN_EXCESSIVE_EXCLAMATION)
website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx
[style] ~83-~83: Future tense may not be necessary in this context.
Context: ...unction will return the default value. This will allow you to mock outputs when executing `atm...
(WILL_ALLOW)
[grammar] ~99-~99: Please add a punctuation mark at the end of paragraph.
Context: ...een provisioned yet, return the default value default-user ```yaml username: !terr...
(PUNCTUATION_PARAGRAPH_END)
[grammar] ~107-~107: Please add a punctuation mark at the end of paragraph.
Context: ...een provisioned yet, return the default value ["mock-subnet1", "mock-subnet2"] ```...
(PUNCTUATION_PARAGRAPH_END)
[grammar] ~115-~115: Please add a punctuation mark at the end of paragraph.
Context: ...een provisioned yet, return the default value `{"api_endpoint": "localhost:3000", "us...
(PUNCTUATION_PARAGRAPH_END)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Summary
🔇 Additional comments (34)
website/docs/core-concepts/stacks/yaml-functions/store.mdx (1)
207-207: Clean formatting improvement.The addition of a space between the pipe and "default" makes the text more readable and consistent with other documentation styles.
🧰 Tools
🪛 LanguageTool
[style] ~207-~207: Using many exclamation marks might seem excessive (in this case: 11 exclamation marks for a text that’s 3777 characters long)
Context: ...tore may not yet be available and the!storefunction call will fail unless yo...(EN_EXCESSIVE_EXCLAMATION)
internal/exec/yaml_func_terraform_output.go (1)
30-35: Improved string parsing with better handling of quoted expressions.The change from simple whitespace splitting to using
SplitStringByDelimiterenables proper parsing of YQ expressions with quotes, supporting the new default value feature outlined in the PR objectives.Consider replacing
log.Fatalwith error return since this is library code:-if err != nil { - log.Fatal(err) -} +if err != nil { + return nil, fmt.Errorf("error parsing terraform output function parameters: %w", err) +}This would require changing the function signature to:
func processTagTerraformOutput( atmosConfig schema.AtmosConfiguration, input string, currentStack string, ) (any, error) { // ... }🧰 Tools
🪛 GitHub Check: golangci-lint
[failure] 34-34:
deep-exit: calls to log.Fatal only in main() or init() functionsinternal/exec/describe_component.go (1)
19-19: Good refactoring to eliminate redundant config initialization.The code now initializes the CLI config once at the start and reuses it later, making the code more efficient and readable.
internal/exec/yaml_func_terraform_output_test.go (1)
139-145: Test updates properly validate the new default value functionality.The test assertions now verify that default values work correctly for different data types (string, list, map) when Terraform outputs are missing or invalid, aligning with the PR objectives.
tests/fixtures/scenarios/atmos-terraform-output-yaml-function/stacks/deploy/nonprod.yaml (3)
28-28: Clear output key modification.Changed from direct key reference to YQ expression syntax with a dot prefix, enabling support for complex YQ operations.
37-37: Consistent output key modification.This follows the same pattern as the earlier change - switching to YQ expression syntax with a dot prefix.
38-40: Excellent examples of default value fallbacks.These new test cases implement the core feature enhancement from this PR - providing fallback values for terraform outputs using YQ's alternative value operator (
//). The examples cover all key data types:
- String default:
"default-value"- List default:
["fallback1", "fallback2"]- Map default:
{"key1": "value1", "key2": "value2"}The proper escaping of double quotes ensures compatibility with YQ syntax requirements.
pkg/component/component_processor.go (3)
4-4: Good standardization of logging imports.Switching to direct import of the
charmbracelet/logpackage aligns with the modernized logging approach.
27-27: Consistent error logging updates.Replaced custom utility logging with direct calls to the
log.Error()function from the updated charmbracelet log package.Also applies to: 37-38, 62-63, 70-71, 76-77
66-68: Improved code readability.Assigning the stack name pattern to a variable before use makes the code more readable and maintainable without changing the behavior.
internal/exec/shell_utils.go (1)
280-281: Clear comment update.Updated the comment to better reflect the variable handling behavior.
website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx (2)
24-31: Enhanced usage documentation.Clear additions to the usage section that demonstrate the new YQ expression capabilities, including both with and without stack parameters.
43-44: Updated argument documentation.Extended the output parameter description to include YQ expressions as an alternative.
go.mod (21)
3-3: Update Go language version to 1.24.3
Bumping to the latest patch release is good. Please ensure that CI pipelines and any Dockerfiles or toolchains are also updated to use Go 1.24.3.
7-7: Upgrade mergo to v1.0.2
Patch bump looks safe and aligns with improved merge behavior.
25-25: Upgrade charmbracelet/log to v0.4.2
This matches the new error‐logging changes in the component processor.
27-27: Upgrade editorconfig-checker to v3.3.0
Keeping the linter up-to-date is helpful for consistent formatting checks.
38-38: Upgrade googleapis/gax-go/v2 to v2.14.2
Minor bump should remain backward compatible; tests should catch any unexpected behavior.
46-46: Upgrade jfrog-client-go to v1.53.0
Patch bump in our Artifactory client—looks safe.
52-52: Upgrade mikefarah/yq/v4 to v4.45.4
This aligns with the enhanced!terraform.outputdefault‐value functionality.
70-70: Upgrade terraform-docs to v0.20.0
Docs generator bump is consistent with updated documentation examples.
76-76: Upgrade google.golang.org/api to v0.233.0
Minor version bump—verify any new APIs don’t introduce breaking changes.
90-90: Upgrade cloud.google.com/go/iam to v1.5.2
Indirect bump—should be backward compatible, but keep an eye on IAM client behavior.
91-91: Upgrade cloud.google.com/go/monitoring to v1.24.2
Patch bump for monitoring client—looks safe.
104-104: Upgrade a8m/envsubst to v1.4.3
Env‐subst library patch bump—no red flags.
178-178: Upgrade gabriel-vasile/mimetype to v1.4.9
MIME type detection patch bump—approved.
232-232: Upgrade jfrog/build-info-go to v1.10.11
Patch bump for build‐info library—no issues expected.
243-243: Upgrade mattn/go-colorable to v0.1.14
Indirect bump for Windows color support—looks good.
262-262: Upgrade pelletier/go-toml/v2 to v2.2.4
TOML parser patch bump—approved.
319-319: Upgrade golang.org/x/crypto to v0.38.0
Security‐related libraries bumped—ensure that no deprecations affect encryption routines.
322-322: Upgrade golang.org/x/net to v0.40.0
Networking utilities patch bump—safe to merge.
328-328: Upgrade google.golang.org/genproto to the latest
Keeping generated protos in sync—approved.
329-329: Upgrade google.golang.org/genproto/googleapis/api to the latest
Proto bindings bump—looks good.
330-330: Upgrade google.golang.org/genproto/googleapis/rpc to the latest
RPC proto bindings patch bump—approved.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1252 +/- ##
==========================================
+ Coverage 34.83% 34.94% +0.10%
==========================================
Files 229 229
Lines 24611 24617 +6
==========================================
+ Hits 8573 8602 +29
+ Misses 14791 14766 -25
- Partials 1247 1249 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/exec/shell_utils_test.go (1)
38-46: Consider reusing this map conversion logic.This conversion from environment variable strings to a map is a common testing pattern. You might want to extract this into a helper function if you plan to add more environment-related tests in the future.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/exec/shell_utils.go(6 hunks)internal/exec/shell_utils_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/exec/shell_utils.go
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Summary
🔇 Additional comments (4)
internal/exec/shell_utils_test.go (4)
9-66: Well-designed test for environment variable merging.The test offers good coverage for the
mergeEnvVarsfunction, properly testing all key scenarios including:
- Preserving existing system variables
- Prepending new values to TF_CLI_ARGS_* variables
- Adding new variables
- Overriding system variables with component values
The approach of saving and restoring the original environment ensures the test doesn't affect other tests running in the same environment. Using table-driven tests is also a good Go testing practice.
12-21: Clean environment restoration implementation.The defer function correctly restores the system's original environment after the test completes. This is important for test isolation.
52-56: Complete test coverage of environment merging behaviors.The test cases thoroughly validate all expected behaviors of the merging function:
- Preservation of system variables (PATH)
- Prepending new values for special Terraform arguments
- Addition of new variables
- Override of existing variables
59-65: Good error messages for test failures.The error messages clearly indicate what failed and why, which helps with debugging should a test fail.
|
These changes were released in v1.175.0. |
what
atmos terraform shellcommand!terraform.outputYAML functionwhy
In
atmos terraform shellcommand, parsing and adding the system (parent process) environment variables to the launched shell was accidentally removed in a previous PR. Added it backAllow specifying a default value in the
!terraform.outputYAML functionUsing YQ Expressions to provide a default value
If the component for which you are reading the output has not been provisioned yet, the
!terraform.outputfunctionwill return the string
<no value>unless you specify a default value in the YQ expression, in which case the function will return the default value.This will allow you to mock outputs when executing
atmos terraform planwhere there are dependencies between components, and the dependent components are not provisioned yet.NOTE: To provide a default value, you use the
//YQ operator. The whole YQ expression contains spaces, and to make it a single parameter, you need to double-quote it. YQ requires the strings in the default values to be double-quoted as well.This means that you have to escape the double-quotes in the default values by using two double-quotes.
For example:
Read the
usernameoutput from theconfigcomponent in the current stack.If the
configcomponent has not been provisioned yet, return the default valuedefault-userRead the
private_subnet_idsoutput from thevpccomponent in the current stack.If the
vpccomponent has not been provisioned yet, return the default value["mock-subnet1", "mock-subnet2"]Read the
config_mapoutput from theconfigcomponent in the current stack.If the
configcomponent has not been provisioned yet, return the default value{"api_endpoint": "localhost:3000", "user": "test"}For more details, review the following docs:
Summary by CodeRabbit
Bug Fixes
Refactor
Tests
Chores
Documentation