Update YAML function !terraform.output
#1048
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 updates the Atmos version across the Dockerfile and GitHub workflow, updates a dependency in go.mod, and enhances environment variable handling in the Terraform output function. New utility functions are added for environment processing and string splitting. Additionally, test scripts are enhanced, adding a new Terraform clean function, new components, workflow commands, and test cases for env and terraform output functionalities. Minor formatting adjustments are also applied to Terraform configuration files. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Suite
participant CleanFn as runCLITerraformClean
participant Terraform as Terraform CLI
Test->>CleanFn: Invoke runCLITerraformClean
CleanFn->>Terraform: Execute "terraform clean"
Terraform-->>CleanFn: Return output & error
CleanFn-->>Test: Return result
sequenceDiagram
participant Parent as Parent Process
participant TF as execTerraformOutput
participant Env as Component Env Settings
Parent->>TF: Provide environment variables
TF->>Env: Merge with component-specific env
Env-->>TF: Return merged environment
TF->>Log: Log debug message with env info
Possibly related PRs
Suggested labels
Suggested reviewers
✨ 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)
pkg/utils/string_utils.go (1)
36-43: Consider using slice instead of fixed-size array.While the implementation is correct, using
[2]stringas the return type might be limiting. Consider returning[]stringfor better flexibility in future use cases.-func SplitStringAtFirstOccurrence(s string, sep string) [2]string { +func SplitStringAtFirstOccurrence(s string, sep string) []string { parts := strings.SplitN(s, sep, 2) if len(parts) == 1 { - return [2]string{parts[0], ""} + return []string{parts[0], ""} } - return [2]string{parts[0], parts[1]} + return parts }
📜 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 (11)
examples/quick-start-advanced/Dockerfile(1 hunks)go.mod(1 hunks)internal/exec/terraform_outputs.go(1 hunks)pkg/utils/env_utils.go(2 hunks)pkg/utils/string_utils.go(1 hunks)tests/cli_terraform_test.go(1 hunks)tests/fixtures/components/terraform/mock/main.tf(1 hunks)tests/fixtures/scenarios/atmos-functions/stacks/deploy/nonprod.yaml(1 hunks)tests/fixtures/scenarios/atmos-functions/stacks/workflows/terraform-output.yaml(1 hunks)tests/test-cases/atmos-functions.yaml(1 hunks)website/docs/integrations/atlantis.mdx(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- tests/fixtures/components/terraform/mock/main.tf
- go.mod
🧰 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 (1)
- GitHub Check: Summary
🔇 Additional comments (10)
pkg/utils/env_utils.go (2)
8-8: LGTM! Grammar improvement in comment.The verb tense correction from "convert" to "converts" improves the documentation clarity.
20-30: LGTM! Well-structured environment variable conversion.The new
EnvironToMapfunction effectively converts environment variables into a map, which is essential for preserving parent process environment variables. The implementation is clean and uses the newSplitStringAtFirstOccurrencehelper function.tests/cli_terraform_test.go (1)
151-161: LGTM! Well-structured test function.The new
runCLITerraformCleanfunction follows consistent patterns with existing test functions and provides good coverage for non-forced clean operations.🧰 Tools
🪛 golangci-lint (1.62.2)
151-151: func
runCLITerraformCleanis unused(unused)
internal/exec/terraform_outputs.go (1)
133-141: LGTM! Robust environment variable handling.The implementation correctly:
- Preserves parent process environment variables
- Allows component-specific overrides
- Provides clear debug logging
This enhancement ensures proper environment variable inheritance, which is crucial for Terraform binary discovery and execution.
tests/fixtures/scenarios/atmos-functions/stacks/workflows/terraform-output.yaml (1)
9-10: LGTM! Well-structured test workflow additions.The new workflow steps for component-5 and component-6 create a good test chain for validating environment variable handling and terraform output functionality.
tests/fixtures/scenarios/atmos-functions/stacks/deploy/nonprod.yaml (1)
31-40: LGTM! Well-designed test components.The component chain effectively tests the environment variable handling:
- component-5 validates environment variable injection
- component-6 validates that terraform.output can access values from components using environment variables
examples/quick-start-advanced/Dockerfile (1)
9-9: LGTM! Version update follows team conventions.The ATMOS_VERSION update to 1.160.4 is consistent with other files and aligns with team preferences for version handling.
tests/test-cases/atmos-functions.yaml (2)
49-66: LGTM! Well-structured environment variable test.The test case effectively validates environment variable injection with proper assertions and expected outputs.
68-83: LGTM! Comprehensive terraform.output test.The test case thoroughly validates that terraform.output can access values from components using environment variables.
website/docs/integrations/atlantis.mdx (1)
676-676: LGTM! Documentation version update.The ATMOS_VERSION update to 1.160.4 in the GitHub Actions example maintains consistency with other files.
|
These changes were released in v1.160.4. |
what
!terraform.outputwhy
!terraform.outputfor a component that sets environment variables in theenvsection, set the environment variables from theenvsection in the child process, and also copy the parent process environment variables into the child process!terraform.outputexecution, e.g. finding a terraform binary in thePATHSummary by CodeRabbit
Chores
New Features
Tests
Style