Update !store YAML function
#1185
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 tool version and several dependency versions across multiple files. The update changes the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Process as processTagStore
participant Logger as log.Fatal
Caller->>Process: Invoke processTagStore with input
alt Successful Processing
Process-->>Caller: Return processed parameters
else Error Occurs
Process->>Logger: log.Fatal(error)
Logger-->>Caller: Terminate execution
end
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
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:
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: 3
🧹 Nitpick comments (1)
website/docs/core-concepts/workflows/workflows.mdx (1)
116-121: Consistent Empty List Formatting
The modifications in the code snippets (usingsteps: []instead of a spaced version) improve consistency in YAML formatting. Although functionally equivalent, this tiny nitpick makes the documentation cleaner.
📜 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 (7)
examples/quick-start-advanced/Dockerfile(1 hunks)go.mod(4 hunks)internal/exec/yaml_func_store.go(5 hunks)internal/exec/yaml_func_store_test.go(2 hunks)website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx(1 hunks)website/docs/core-concepts/workflows/workflows.mdx(2 hunks)website/docs/integrations/atlantis.mdx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
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: 2025-04-03T12:14:28.671Z
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: website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx:104-110
Timestamp: 2025-04-03T12:14:28.671Z
Learning: In the documentation for `!terraform.output`, warnings about template variable availability are already covered in other sections, so no need to suggest adding them here.
examples/quick-start-advanced/Dockerfile (2)
Learnt from: aknysh
PR: cloudposse/atmos#775
File: examples/quick-start-advanced/Dockerfile:9-9
Timestamp: 2025-04-03T12:14:28.671Z
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: 2025-04-03T12:14:28.671Z
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.
🪛 GitHub Check: golangci-lint
internal/exec/yaml_func_store.go
[failure] 26-26:
deep-exit: calls to log.Fatal only in main() or init() functions
[failure] 71-71:
deep-exit: calls to log.Fatal only in main() or init() functions
[failure] 80-80:
deep-exit: calls to log.Fatal only in main() or init() functions
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Summary
🔇 Additional comments (16)
website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx (1)
98-100: Clarify Parameter Ordering in Documentation
The added line at 98 clearly explains that when calling!terraform.outputwith three parameters, the stack must be specified as the second argument. This clarification improves the overall usability of the documentation.website/docs/integrations/atlantis.mdx (1)
675-677: Update Atmos Version in GitHub Actions Workflow
The update of theATMOS_VERSIONenvironment variable from1.169.0to1.170.0ensures consistency with other parts of the repository. This change is straightforward and aligns with our versioning strategy.examples/quick-start-advanced/Dockerfile (1)
9-9: Align Dockerfile with Updated Atmos Version
Updating theARG ATMOS_VERSIONto1.170.0maintains consistency with the GitHub Actions workflow configuration and other parts of the codebase. Good job keeping everything in sync!website/docs/core-concepts/workflows/workflows.mdx (1)
374-378: Refine Workflow Failure Handling Section Title
The updated section title from “Workflow Failure Handling and Resume” to “Workflow Failure Handling and Resuming” is a nice grammatical improvement that enhances clarity.go.mod (2)
14-17: Bump AWS SDK and Other Dependency Versions in go.mod
The updates for dependencies such asgithub.com/aws/aws-sdk-go-v2/config,credentials,ssm, andstsas well as minor version bumps for other libraries (including the update forterraform-config-inspectandgoogle.golang.org/grpc) are clear maintenance updates. They appear to align with the latest available versions and ensure compatibility with the current Atmos CLI tool release.
121-123: Update SSO Dependencies
The updates togithub.com/aws/aws-sdk-go-v2/service/ssoandssooidcreflect the latest minor improvements. It’s a good practice to keep these dependencies current.internal/exec/yaml_func_store_test.go (8)
11-13: Imports section is properly organized.Clean import organization with standard libraries first, followed by third-party packages.
58-58: Parameter order correctly updated to fix the issue.The input parameter order has been fixed from
redis dev vpc cidrtoredis vpc dev cidr, which properly aligns with the PR objective to fix the swapped component and stack parameters.
64-65: Fixed parameter order in test case with defaults.Correctly updated to use the new parameter order (
vpc staging cidrinstead ofstaging vpc cidr).
70-71: Consistent parameter ordering in test with single quotes.Parameter ordering properly maintained across all test cases.
76-77: Parameter order fixed in test with double quotes.Consistent update to the parameter ordering pattern.
82-85: Error case test inputs and expected values properly updated.Both the input string and expected error message now correctly reflect the new parameter ordering.
88-91: Fixed parameter order in extra parameters test case.Input and expected error message have been updated to match the new parameter order.
97-97: Added useful logging to improve test diagnostics.The additional logging statement is helpful for debugging and understanding test results.
internal/exec/yaml_func_store.go (2)
7-7: Import alias for log package improves readability.Good practice to use an alias for the charmbracelet log package to avoid confusion with the standard log package.
58-60: Fixed parameter order to correctly assign component and stack.This is the key fix for the reported issue - now
componentis correctly assigned beforestackwhen four parameters are provided. The previous implementation had these swapped, causing retrieval failures.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1185 +/- ##
=======================================
Coverage 29.50% 29.50%
=======================================
Files 196 196
Lines 22549 22549
=======================================
Hits 6652 6652
Misses 14880 14880
Partials 1017 1017
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:
|
|
I have chosen to update the docs in #1186 instead of changing code. I think it makes more sense to order the parameters the way the code currently does it from least specific -> more specific (store -> stack -> component -> key). |
what
!storeYAML functionwhy
!storeYAML function, when the user configured the function with 4 parameters (<store>,<component>,<stack>,<key>), thecomponentandstackparameters were swapped, causing the function to fail to retrieve a value from the store. (!storewith 3 parameters,<store>,<component>,<key>, worked without issues)Summary by CodeRabbit