Update JSON schema for Atmos manifests validation. Update !terraform.output YAML function
#1053
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 makes several updates across Atmos JSON schemas to add new properties for enhanced extensibility. The changes include adding a top-level Changes
Sequence Diagram(s)sequenceDiagram
participant E as execTerraformOutput
participant F as environToMap
participant T as Terraform Execution
E->>F: Call environToMap()
F-->>E: Return filtered environment map
E->>T: Execute Terraform with filtered env vars
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 (2)
internal/exec/terraform_outputs.go (1)
383-397: Consider removing the TODO comment or creating an issue.The TODO comment suggests finding an alternative to using
terraform-exec/tfexec. This should either be addressed now or tracked in an issue.Would you like me to help create an issue to track this improvement?
tests/fixtures/schemas/atmos/atmos-manifest/1.0/atmos-manifest.json (1)
742-747: Consider enhancing hooks schema with common patterns.While the current implementation with
additionalProperties: trueprovides maximum flexibility, consider documenting common hook patterns or providing an enum of known hook types to guide users.Apply this diff to add common hook patterns:
"hooks": { "type": "object", "description": "Hooks section", "additionalProperties": true, + "properties": { + "before": { + "type": "object", + "description": "Hooks to run before component operations", + "additionalProperties": true + }, + "after": { + "type": "object", + "description": "Hooks to run after component operations", + "additionalProperties": true + } + }, "title": "hooks" }
📜 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/demo-context/schemas/atmos-manifest.json(3 hunks)examples/demo-helmfile/schemas/atmos-manifest.json(3 hunks)examples/demo-localstack/schemas/atmos-manifest.json(3 hunks)examples/quick-start-advanced/Dockerfile(1 hunks)go.mod(1 hunks)internal/exec/schemas/atmos/atmos-manifest/1.0/atmos-manifest.json(2 hunks)internal/exec/terraform_outputs.go(5 hunks)tests/fixtures/schemas/atmos/atmos-manifest/1.0/atmos-manifest.json(2 hunks)tests/test-cases/atmos-functions.yaml(1 hunks)website/docs/integrations/atlantis.mdx(1 hunks)website/static/schemas/atmos/atmos-manifest/1.0/atmos-manifest.json(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- website/docs/integrations/atlantis.mdx
- 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 (3)
- GitHub Check: Build (macos-latest, macos)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Summary
🔇 Additional comments (13)
internal/exec/terraform_outputs.go (3)
23-39: LGTM! Well-structured constants for Terraform environment variables.The constants are well-organized and clearly named, making the code more maintainable.
41-54: LGTM! Comprehensive lists of prohibited environment variables.The lists effectively capture both direct variables and prefixes that should be filtered out during Terraform execution.
Also applies to: 56-59
173-174: LGTM! Improved environment variable handling.The code now properly filters out prohibited environment variables before passing them to the Terraform execution context.
Also applies to: 180-184
examples/quick-start-advanced/Dockerfile (1)
9-9: LGTM! Version update aligns with team preferences.The update to
ATMOS_VERSION=1.160.5follows the team's practice of pinning to the next future version when the PR merges.tests/test-cases/atmos-functions.yaml (1)
80-86: LGTM! Test case validates environment variable filtering.The test case effectively demonstrates that prohibited environment variables will not be passed to the child process.
website/static/schemas/atmos/atmos-manifest/1.0/atmos-manifest.json (2)
235-237: LGTM! Added hooks support to terraform component manifest.The schema now properly supports the hooks feature for Terraform components.
742-747: LGTM! Well-defined hooks schema.The hooks schema is properly defined with appropriate type and description, allowing for flexible additional properties.
tests/fixtures/schemas/atmos/atmos-manifest/1.0/atmos-manifest.json (2)
234-237: LGTM! Well-structured hooks integration.The addition of the hooks property to the terraform_component_manifest is clean and follows JSON Schema best practices by referencing the hooks definition.
356-359: LGTM! Clear metadata locking mechanism.The locked property in metadata is well-documented and provides a clear mechanism for controlling component modifications.
internal/exec/schemas/atmos/atmos-manifest/1.0/atmos-manifest.json (1)
234-237: LGTM! Schema changes are consistent.The changes in this file mirror those in the test fixtures, maintaining consistency across the codebase.
Also applies to: 742-747, 356-359
examples/demo-context/schemas/atmos-manifest.json (1)
234-237: LGTM! Example schema maintains consistency.The demo context schema correctly implements the new hooks and locked properties, providing a good reference for users.
Also applies to: 742-747, 356-359
examples/demo-localstack/schemas/atmos-manifest.json (1)
234-237: LGTM! LocalStack demo schema is aligned.The schema changes are consistently implemented in the LocalStack demo, maintaining parity with other examples.
Also applies to: 742-747, 356-359
examples/demo-helmfile/schemas/atmos-manifest.json (1)
234-237: LGTM! Helmfile demo schema is synchronized.The schema changes are properly reflected in the Helmfile demo, ensuring consistency across all example implementations.
Also applies to: 742-747, 356-359
|
These changes were released in v1.160.5. |
what
Update JSON schema for Atmos manifests validation. Add
hooksto the JSON schema for Atmos manifests validationUpdate
!terraform.outputYAML functionwhy
hooksis a recent addition to Atmos and need to be added to the JSON schemaWhen executing
!terraform.output, Atmos sets all the ENV variables from the parent process (that executes the CLI command) in the child process (that executesterraform outputusing the terraform-exec library. The library does not allow certain environment variables to be set in the process, so we don't include thoserelated
Summary by CodeRabbit