Embed JSON Schema for validation of Atmos manifests inside Atmos binary #925
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 introduces modifications to the Atmos tool's schema management and configuration flexibility. Key updates include enhancements to the Changes
Suggested reviewersPossibly related PRs
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (4)
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/validate_stacks.go (1)
403-423: Consider adding cleanup for temporary files.The implementation is solid, but consider adding cleanup logic for temporary files in long-running processes.
func getEmbeddedSchemaPath() (string, error) { embedded, err := fs.ReadFile(atmosManifestDefault, atmosManifestDefaultFileName) if err != nil { return "", err } tempDir := os.TempDir() atmosManifestJsonSchemaFilePath := filepath.Join(tempDir, atmosManifestDefaultFileName) err = u.EnsureDir(atmosManifestJsonSchemaFilePath) if err != nil { return "", err } err = os.WriteFile(atmosManifestJsonSchemaFilePath, embedded, 0644) if err != nil { + // Cleanup on error + _ = os.Remove(atmosManifestJsonSchemaFilePath) return "", err } return atmosManifestJsonSchemaFilePath, nil }website/docs/core-concepts/validate/validate.mdx (1)
30-32: Consider adding a comma after "e.g."The sentence reads well, but for consistency with standard style guides, add a comma after "e.g.".
-The OPA language (Rego) is a high-level declarative language for specifying policy as code. Atmos has native support for the OPA decision-making engine to enforce policies across all the components in your stacks (e.g. for microservice configurations). +The OPA language (Rego) is a high-level declarative language for specifying policy as code. Atmos has native support for the OPA decision-making engine to enforce policies across all the components in your stacks (e.g., for microservice configurations).🧰 Tools
🪛 LanguageTool
[style] ~32-~32: A comma is missing here.
Context: ...ross all the components in your stacks (e.g. for microservice configurations). This...(EG_NO_COMMA)
📜 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)
atmos.yaml(2 hunks)examples/quick-start-advanced/Dockerfile(1 hunks)go.mod(4 hunks)internal/exec/validate_stacks.go(6 hunks)pkg/validate/atmos.yaml(1 hunks)pkg/validate/validate_stacks_test.go(1 hunks)website/docs/cli/commands/validate/validate-stacks.mdx(3 hunks)website/docs/core-concepts/validate/json-schema.mdx(1 hunks)website/docs/core-concepts/validate/opa.mdx(1 hunks)website/docs/core-concepts/validate/validate.mdx(3 hunks)website/docs/integrations/atlantis.mdx(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- website/docs/integrations/atlantis.mdx
- website/docs/core-concepts/validate/opa.mdx
- pkg/validate/atmos.yaml
🧰 Additional context used
📓 Learnings (2)
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.
internal/exec/validate_stacks.go (1)
Learnt from: haitham911
PR: cloudposse/atmos#731
File: internal/exec/validate_stacks.go:0-0
Timestamp: 2024-11-12T05:52:05.088Z
Learning: In `internal/exec/validate_stacks.go`, when downloading the Atmos JSON Schema file to the temp directory, the temporary file is overwritten each time, so explicit removal is not necessary.
🪛 LanguageTool
website/docs/core-concepts/validate/validate.mdx
[style] ~32-~32: A comma is missing here.
Context: ...ross all the components in your stacks (e.g. for microservice configurations). This...
(EG_NO_COMMA)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: [mock-windows] tests/fixtures/scenarios/complete
- GitHub Check: [k3s] demo-helmfile
- GitHub Check: Acceptance Tests (macos-latest, macos)
- GitHub Check: Acceptance Tests (windows-latest, windows)
- GitHub Check: [localstack] demo-localstack
- GitHub Check: Acceptance Tests (ubuntu-latest, linux)
- GitHub Check: Summary
🔇 Additional comments (14)
pkg/validate/validate_stacks_test.go (2)
21-24: LGTM! Path updated to use embedded schema.The schema path has been correctly updated to point to the new embedded schema location.
26-30: LGTM! Good addition of remote schema test.The test ensures remote schema validation works correctly. Consider verifying the URL availability.
✅ Verification successful
Remote schema URL verified and accessible! Test implementation is solid.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the remote schema URL is accessible curl -I https://atmos.tools/schemas/atmos/atmos-manifest/1.0/atmos-manifest.jsonLength of output: 837
internal/exec/validate_stacks.go (2)
24-27: LGTM! Clean implementation of schema embedding.Good use of Go's embed feature to bundle the schema with the binary.
Line range hint
95-119: LGTM! Robust schema validation logic.The implementation gracefully handles:
- Fallback to embedded schema
- Clear error messages with configuration options
- Backward compatibility with existing schema paths
examples/quick-start-advanced/Dockerfile (1)
9-9: LGTM! Version update follows team conventions.Version update to 1.144.0 aligns with the team's practice of pinning to future versions.
website/docs/core-concepts/validate/json-schema.mdx (1)
8-16: LGTM! Documentation improvements enhance readability.Good restructuring of the JSON Schema description and proper component imports.
website/docs/core-concepts/validate/validate.mdx (3)
14-15: Well-crafted enhancement to the introduction!The added text effectively emphasizes the importance of validation in multi-team environments.
107-111: Clear explanation of the new embedded schema feature!The documentation effectively communicates the default behavior and configuration options.
119-129: Well-structured configuration example!The YAML example clearly shows both local and remote schema configuration options.
website/docs/cli/commands/validate/validate-stacks.mdx (2)
80-87: Comprehensive examples covering all use cases!The examples effectively demonstrate the three ways to specify the schema:
- Using the default embedded schema
- Using a local file
- Using a remote URL
92-94: Clear flag documentation!The flag description effectively communicates all supported schema sources.
go.mod (1)
10-12: Verify the impact of dependency updatesThe updates include security-related packages (
crypto,net) and AWS SDK components. While these appear to be routine updates, please ensure they don't introduce any breaking changes.Also applies to: 263-263, 266-266
✅ Verification successful
Dependencies are battle-ready! 💪
The updates are safe and maintain backward compatibility:
- golang.org/x/crypto v0.32.0 is patched against all known vulnerabilities
- AWS SDK updates are minor version bumps (v1.x.x), maintaining backward compatibility
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any known vulnerabilities in the updated dependencies gh api graphql -f query=' { securityVulnerabilities(first: 5, ecosystem: GO, package: "golang.org/x/crypto") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }'Length of output: 1705
Script:
#!/bin/bash # Check the exact versions of dependencies echo "=== Crypto Package Version ===" rg "golang.org/x/crypto" go.mod echo -e "\n=== AWS SDK Version Requirements ===" # Look for any version constraints or requirements rg "aws-sdk-go-v2" go.modLength of output: 1502
atmos.yaml (2)
275-276: Clear documentation of schema configuration options!The comments effectively communicate both URL support and the embedded schema fallback.
286-290: Well-structured examples for schema configuration!The commented examples clearly show both local and remote schema options.
Co-authored-by: Erik Osterman (CEO @ Cloud Posse) <[email protected]>
|
@coderabbitai full review please |
|
@aknysh i've completed the review of your pr. the changes look well-structured and properly implemented. here are my observations: strengths:
suggestions for consideration:
the changes look good to proceed. would you like me to open issues for any of these suggestions for future improvements? 🔗 analysis chain 🏁 scripts executed the following scripts were executed for the analysis: script: length of output: 15117 ✏️ learnings added
✅ Actions performedReview triggered.
|
|
These changes were released in v1.144.0. |
what
why
atmos.yamland monitor it when it needs to be updateddescription
Atmos uses the Atmos Manifest JSON Schema to validate Atmos manifests, and has a default (embedded) JSON Schema.
If you don't configure the path to a JSON Schema in
atmos.yamland don't provide it on the command line using the--schemas-atmos-manifestflag, the default (embedded) JSON Schema will be used when executing the commandatmos validate stacks.To override the default behavior, configure JSON Schema in
atmos.yaml:Add the Atmos Manifest JSON Schema to your repository, for example in
stacks/schemas/atmos/atmos-manifest/1.0/atmos-manifest.jsonConfigure the following section in the
atmos.yamlschemas.atmos.manifestsection inatmos.yaml, you can provide the path tothe Atmos Manifest JSON Schema file by using the ENV variable
ATMOS_SCHEMAS_ATMOS_MANIFESTor the--schemas-atmos-manifestcommand line flag:Summary by CodeRabbit
Configuration Updates
atmos.yamlDependency Updates
Documentation Improvements
Testing