Update Atmos manifests validation JSON schema. Improve help for Atmos commands. Deep-merge the settings.integrations.github section from stack manifests with the integrations.github section from atmos.yaml
#755
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces several changes across multiple files, primarily focusing on enhancements to the Atmos configuration and schema definitions. A new section for GitHub integration is added to the Changes
Possibly related PRs
Suggested reviewers
Poem
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
CodeRabbit Configuration File (
|
|
@aknysh, With this change, is it possible to set vars:
location: westus3
environment: wus3
namespace: dev
settings:
integrations:
github:
xxxxxx``` |
yes, at any level, any scope (this is the same as any other Atmos section like
And you can override any field per stack or component (if you need them to be different) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Outside diff range comments (1)
go.mod (1)
Line range hint
3-3: Fix invalid Go version.The specified Go version
1.23is invalid and not yet released. Go versions should follow the format1.xor1.x.ywhere x and y are numbers.Update to a valid and released version:
-go 1.23 +go 1.22
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
⛔ Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumwebsite/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (18)
atmos.yaml(1 hunks)examples/demo-context/schemas/atmos-manifest.json(1 hunks)examples/demo-helmfile/schemas/atmos-manifest.json(1 hunks)examples/demo-localstack/schemas/atmos-manifest.json(1 hunks)examples/quick-start-advanced/Dockerfile(1 hunks)examples/quick-start-advanced/stacks/schemas/atmos/atmos-manifest/1.0/atmos-manifest.json(1 hunks)examples/tests/atmos.yaml(1 hunks)examples/tests/stacks/catalog/terraform/vpc.yaml(1 hunks)go.mod(1 hunks)internal/exec/helmfile.go(1 hunks)internal/exec/help.go(3 hunks)internal/exec/stack_processor_utils.go(3 hunks)internal/exec/terraform.go(1 hunks)internal/exec/utils.go(2 hunks)pkg/config/const.go(1 hunks)website/docs/integrations/atlantis.mdx(1 hunks)website/package.json(1 hunks)website/static/schemas/atmos/atmos-manifest/1.0/atmos-manifest.json(1 hunks)
🔇 Additional comments (26)
examples/quick-start-advanced/Dockerfile (2)
9-9: Verify Atmos version aligns with PR objectives
The update to Atmos 1.99.0 should support the new features being introduced in this PR, particularly:
- Updated manifest validation JSON schema
- Improved command help
- Deep-merge functionality for GitHub integration settings
2-2: Verify Geodesic version compatibility
The update to Geodesic 3.4.0 should be verified for compatibility with the other tools in the environment, particularly with Atmos 1.99.0.
✅ Verification successful
Let me try another verification approach to check for any compatibility issues or version-related information.
Let me try one final verification to check the release notes and changes between versions.
Version update appears to be a routine maintenance change
The version update from Geodesic 3.3.0 to 3.4.0 appears to be a minor version bump in the development environment. While we couldn't find explicit compatibility documentation, the change follows semantic versioning principles where minor version updates should maintain backward compatibility. Since this is used in an example Dockerfile and no compatibility issues were found in the codebase, the update should be safe to proceed with.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if there are any known compatibility issues or breaking changes
# between Geodesic 3.4.0 and other components
# Look for compatibility notes in changelog
rg -i "compatibility|breaking|atmos" $(fd -t f "CHANGELOG.md")
# Check if this version is mentioned in any documentation
rg "3\.4\.0" $(fd -t f "README.md" "docs")
Length of output: 296
Script:
#!/bin/bash
# Search for Geodesic version references and any compatibility requirements
rg -i "geodesic.*version|version.*geodesic" --type md
# Look for any documentation about version requirements or compatibility
rg -i "require.*geodesic|geodesic.*require|compatible.*with.*geodesic|geodesic.*compatible" --type md
# Check if there are any specific version constraints mentioned in the codebase
rg "GEODESIC_VERSION" -A 2 -B 2
Length of output: 779
Script:
#!/bin/bash
# Check the repository's documentation for any version-specific information
rg -i "3\.[34]\.0" --type md
# Look for any version constraints or dependencies in configuration files
fd -e yaml -e yml -e json | xargs rg -i "geodesic.*3\.[34]\.0"
# Check for any issues or discussions mentioning these versions
rg -i "geodesic.*(3\.3\.0|3\.4\.0)" --type md
Length of output: 143
website/package.json (2)
30-30: LGTM! Safe dependency updates.
The version updates for @mdx-js/react, marked, prism-react-renderer, and react-social-media-embed are minor or patch updates that include bug fixes and improvements. These changes are safe and maintain backward compatibility.
Also applies to: 36-36, 40-40, 47-47
39-39: Verify PostHog integration after significant version update.
The posthog-js update from 1.157.2 to 1.178.0 spans multiple minor versions. While this update brings new features and improvements, it's important to verify that the analytics integration continues to work as expected.
✅ Verification successful
PostHog integration appears minimal and configuration-based
The PostHog integration is only used in docusaurus.config.js through a configuration object, with no direct API calls in the codebase. The update from 1.157.2 to 1.178.0 should not impact the current implementation since:
- The integration is handled through Docusaurus configuration
- No direct PostHog API methods are being called in the codebase
- The configuration pattern (
appUrlandenableInDevelopment) remains stable across PostHog versions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for PostHog configuration and usage patterns that might need updates
# Look for PostHog configuration and initialization
rg -l "posthog" --type js --type jsx --type ts --type tsx
# Look for PostHog API usage patterns that might need updates
ast-grep --pattern 'posthog.$_($$$)'
Length of output: 124
Script:
#!/bin/bash
# Search for PostHog-related files and usage with correct file extensions
rg -l "posthog" -g '*.{js,ts,tsx}'
# Look for specific PostHog initialization and configuration
rg -A 5 "posthog\." -g '*.{js,ts,tsx}'
# Look for import statements related to PostHog
rg "import.*posthog" -g '*.{js,ts,tsx}'
Length of output: 500
examples/tests/stacks/catalog/terraform/vpc.yaml (1)
36-40: 🧹 Nitpick (assertive)
Consider adding schema validation for the new GitHub integration settings.
Given that this file already implements JSON Schema validation for other sections (e.g., VPC component variables), it would be beneficial to add schema validation for the new GitHub integration settings to ensure configuration consistency.
Let's check if schema validation exists for these settings:
internal/exec/helmfile.go (1)
48-51: LGTM! Verify processHelp function signature.
The change to pass cliConfig instead of the string literal "helmfile" aligns with the PR objective of improving help command output. This modification enables more context-aware help information.
Let's verify that the processHelp function signature has been updated to accept the CLI configuration:
✅ Verification successful
Change verified - processHelp function signature matches
The function signature in internal/exec/help.go correctly accepts cliConfig schema.CliConfiguration as its first parameter, which matches the updated call site. The change is safe and maintains the expected interface.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify processHelp function signature in help.go
# Expect: Function signature should accept cliConfig as first parameter
ast-grep --pattern 'func processHelp($config, $_, $_)'
Length of output: 56
Script:
#!/bin/bash
# Let's try a different approach to find the processHelp function definition
# First, let's use ripgrep to find the function definition with context
rg "func processHelp" -A 5
# Also search for the file where it might be defined
fd "help.go" --type f
Length of output: 332
internal/exec/help.go (1)
12-16: LGTM: Good improvement to function signature.
The addition of cliConfig parameter improves the function's design by making configuration dependencies explicit rather than relying on global state.
go.mod (1)
13-13: LGTM: Glamour dependency maintained for CLI help formatting.
The re-addition of github.com/charmbracelet/glamour v0.8.0 is appropriate as it's needed for formatting the improved CLI help output, which aligns with the PR objectives.
internal/exec/terraform.go (1)
55-58: LGTM! The change aligns with improving help output.
The modification to include cliConfig in the processHelp call enables better context-aware help information, which aligns with the PR objective of improving command help output when executed without specific parameters.
Let's verify the function signature change:
✅ Verification successful
Function signature mismatch detected
The processHelp function in internal/exec/help.go has a different signature than how it's being called:
- Actual:
processHelp(cliConfig schema.CliConfiguration, componentType string, command string) - Usage:
processHelp(cliConfig, "terraform", "")
The function expects three parameters:
cliConfig(✓ provided correctly)componentType(✓ provided as "terraform")command(✓ provided as empty string)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify processHelp function signature matches the new call
# Test: Search for processHelp function definition
ast-grep --pattern 'func processHelp($config $type, $cmd string, $args string)'
Length of output: 81
Script:
#!/bin/bash
# Let's try a different approach to find processHelp function definition
# First, let's find the file containing processHelp
rg -l "func processHelp"
# Then let's see the actual function signature with context
rg "func processHelp" -A 5
Length of output: 337
examples/demo-context/schemas/atmos-manifest.json (2)
480-483: LGTM: New local backend type added correctly.
The addition of the local backend type with additionalProperties: true maintains consistency with other backend types and provides the necessary flexibility for local backend configurations.
485-486: Verify handling of required S3 backend properties.
The change to allow additional properties in the s3 backend configuration aligns with the PR objectives and provides more flexibility. However, please verify that required S3 backend properties (like bucket and key) are still validated elsewhere in the codebase to prevent misconfiguration.
✅ Verification successful
Based on the search results, I can now generate a final response. The code shows that there are specific validations for S3 backend configuration:
S3 backend validation is present and sufficient
The codebase validates critical S3 backend properties:
workspace_key_prefixis explicitly required and validated for S3 backend type ininternal/exec/terraform_generate_backend.go- The backend configuration is processed and validated in
internal/exec/stack_processor_utils.go - The schema change to allow additional properties is safe as the core validation logic remains in the Go code
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if S3 backend validation exists elsewhere in the codebase
# Test 1: Look for S3 backend validation in Go code
echo "Searching for S3 backend validation in Go code..."
rg -t go "s3.*backend.*validation" -A 5
# Test 2: Look for S3 backend tests
echo "Searching for S3 backend tests..."
rg -t go "TestS3Backend" -A 5
Length of output: 257
Script:
#!/bin/bash
# Let's try a broader search for S3 backend related code and validation
# Search for S3 backend related code
echo "Searching for S3 backend patterns..."
rg -t go "s3" -A 5 | grep -i "backend"
# Search for backend validation patterns
echo "Searching for backend validation patterns..."
rg -t go "backend.*validate" -A 5
# Look for schema validation code
echo "Searching for schema validation code..."
rg -t go "schema.*validate" -A 5
# Search for S3 related structs or types
echo "Looking for S3 related structs..."
ast-grep --pattern 'type $_ struct {
$$$
Bucket $_
Key $_
$$$
}'
Length of output: 5127
examples/demo-helmfile/schemas/atmos-manifest.json (2)
480-483: LGTM: Addition of local backend type
The addition of the local backend type with additionalProperties: true aligns with the schema's flexible configuration approach and matches the structure of other backend types.
485-486: LGTM: Updated s3 backend configuration
The modification to allow additional properties in the s3 backend configuration achieves the PR objective of avoiding hardcoded fields. This change:
- Provides more flexibility in defining
s3backend configurations - Aligns with the free-form configuration approach used by other backend types (GCP, Azure)
- Accommodates potential variations in Terraform's
s3backend configuration
examples/demo-localstack/schemas/atmos-manifest.json (2)
480-483: LGTM: New local backend property follows consistent pattern
The addition of the local backend property follows the established pattern used by other backend types in the schema, allowing for flexible configuration through additionalProperties: true.
485-486: LGTM: Improved flexibility for s3 backend configuration
The modification to allow additional properties in the s3 backend configuration aligns well with the PR objective to avoid hardcoding s3 backend fields. This change:
- Provides more flexibility for various s3 backend configurations
- Maintains consistency with other backend types in the schema
- Allows for future Terraform s3 backend additions without schema updates
examples/quick-start-advanced/stacks/schemas/atmos/atmos-manifest/1.0/atmos-manifest.json (2)
480-483: LGTM: Local backend configuration added consistently.
The new local backend property follows the established pattern of other backend configurations, maintaining schema consistency while expanding supported backend types.
485-486: LGTM: S3 backend schema made more flexible.
The change aligns with the PR objective to avoid hardcoding s3 backend fields, allowing for more flexible configuration. This modification maintains consistency with other backend types and accommodates future Terraform s3 backend property additions without requiring schema updates.
Let's verify existing s3 backend configurations in the codebase:
✅ Verification successful
Let me try another search strategy to find s3 backend configurations.
Let's examine the actual content of these YAML files to see the s3 backend configurations.
Schema change for s3 backend is safe and compatible with existing usage
Based on the examination of existing s3 backend configurations in the codebase, the schema change to allow additional properties is safe and appropriate. The current usage patterns show simple configurations with standard properties like encrypt, key, and bucket, and the more flexible schema will maintain backward compatibility while allowing for future Terraform s3 backend property additions.
Examples of current usage found:
examples/tests/stacks/catalog/terraform/spacelift-and-backend-override-1.yaml: Usesencrypt,key, andbucketexamples/tests/stacks/orgs/cp/_defaults.yaml: Usesencryptexamples/quick-start-advanced/stacks/orgs/acme/_defaults.yaml: Showsaclandencryptin comments
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for s3 backend configurations to ensure compatibility
# Expected: Find existing s3 backend configurations to review their properties
# Search for s3 backend configurations in stack manifests
echo "Searching for s3 backend configurations in stack manifests..."
rg -l --type yaml 'backend:.*s3:' || echo "No s3 backend configurations found in YAML files"
# Search for s3 backend configurations in terraform files
echo "Searching for s3 backend configurations in terraform files..."
rg -l --type terraform 'backend\s+"s3"' || echo "No s3 backend configurations found in Terraform files"
Length of output: 601
Script:
#!/bin/bash
# Search for s3 backend configurations with different approaches
# Search for s3 backend in all files (not limiting by type)
echo "Searching for s3 backend configurations..."
rg -l 'backend.*s3' || echo "No s3 backend configurations found"
# Search specifically in .tf files
echo -e "\nSearching in .tf files..."
fd -e tf -x rg -l 'backend.*s3' {} \; || echo "No matches in .tf files"
# Search in example configurations
echo -e "\nSearching in examples..."
fd -t f -p 'examples/**' -x rg -l 's3.*backend|backend.*s3' {} \; || echo "No matches in examples"
# Look for any JSON/YAML files that might contain backend configurations
echo -e "\nSearching in JSON/YAML files..."
fd -e json -e yaml -e yml -x rg -l 'backend.*s3' {} \; || echo "No matches in JSON/YAML files"
Length of output: 2282
Script:
#!/bin/bash
# Examine s3 backend configurations in the identified YAML files
echo "Content of examples/quick-start-advanced/stacks/orgs/acme/_defaults.yaml:"
rg -A 5 -B 2 'backend.*s3' examples/quick-start-advanced/stacks/orgs/acme/_defaults.yaml
echo -e "\nContent of examples/tests/stacks/catalog/terraform/test-component-override.yaml:"
rg -A 5 -B 2 'backend.*s3' examples/tests/stacks/catalog/terraform/test-component-override.yaml
echo -e "\nContent of examples/tests/stacks/catalog/terraform/test-component-override-2.yaml:"
rg -A 5 -B 2 'backend.*s3' examples/tests/stacks/catalog/terraform/test-component-override-2.yaml
echo -e "\nContent of examples/tests/stacks/catalog/terraform/spacelift-and-backend-override-1.yaml:"
rg -A 5 -B 2 'backend.*s3' examples/tests/stacks/catalog/terraform/spacelift-and-backend-override-1.yaml
echo -e "\nContent of examples/tests/stacks/orgs/cp/_defaults.yaml:"
rg -A 5 -B 2 'backend.*s3' examples/tests/stacks/orgs/cp/_defaults.yaml
Length of output: 2588
website/static/schemas/atmos/atmos-manifest/1.0/atmos-manifest.json (2)
480-483: LGTM: Well-structured addition of local backend support.
The new local backend property follows the established pattern and correctly allows for flexible configuration through additionalProperties: true.
485-486: LGTM: Good improvement to S3 backend flexibility.
Removing hardcoded S3 backend fields in favor of additionalProperties: true is a positive change that:
- Allows for all valid Terraform S3 backend configurations
- Makes the schema more maintainable
- Aligns with how other backend types are defined
examples/tests/atmos.yaml (1)
342-348: LGTM! GitHub integration configuration looks well-structured.
The new GitHub integration section is properly placed under integrations and includes appropriate version specifications for both OpenTofu and Terraform, along with Infracost configuration. This aligns with the PR objective of supporting deep-merge functionality for GitHub integration settings.
Let's verify the version specifications are consistent across the codebase:
✅ Verification successful
Version specifications are consistent across configuration files
The verification confirms that both OpenTofu version (1.8.4) and Terraform version (1.9.8) specifications are consistent across the codebase. They appear in both atmos.yaml and examples/tests/atmos.yaml with identical version numbers and structure.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any inconsistent version specifications across the codebase
# Expect: Consistent version numbers for opentofu and terraform across all configuration files
echo "Checking for OpenTofu version specifications..."
rg --type yaml "opentofu-version:\s*1\.8\.4" -C 2
echo "Checking for Terraform version specifications..."
rg --type yaml "terraform-version:\s*1\.9\.8" -C 2
Length of output: 1130
internal/exec/utils.go (2)
222-224: LGTM: Improved help processing with CLI configuration.
The change to use schema.CliConfiguration{} instead of just componentType aligns with the PR objective to enhance help output for Atmos commands.
638-644: LGTM: Enhanced UX for single-argument commands.
The new conditional block improves user experience by showing help information instead of errors when only a command is provided (e.g., atmos terraform plan without component/stack). This aligns with the PR objective to improve help output for Atmos commands.
internal/exec/stack_processor_utils.go (4)
1187-1191: Consistent Processing of GitHub Integration Settings
The introduction of processSettingsIntegrationsGithub ensures that GitHub integration settings are consistently deep-merged with global settings for Terraform components. Error handling is appropriately implemented.
1194-1199: Correct Assignment of Merged Settings to Component Configuration
Assigning finalSettings to comp[cfg.SettingsSectionName] correctly incorporates the merged settings into the component configuration.
1480-1484: Unified GitHub Settings Processing for Helmfile Components
Applying processSettingsIntegrationsGithub to Helmfile components ensures consistent deep-merging of GitHub integration settings across both Terraform and Helmfile components.
1487-1487: Proper Inclusion of Merged Settings in Helmfile Component
The assignment of finalSettings to comp[cfg.SettingsSectionName] correctly updates the Helmfile component configuration with the merged settings.
@aknysh when I build atmos and run it in this branch, I still see a wall of text.
By comparison, if running 
And with the Which is a lot of text, but contextually relevant. We are showing contextually irrelevant help output. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range comments (2)
internal/exec/help.go (2)
Line range hint
12-120: Consider improving help text organization and readability.The current help output presents a "wall of text" that can overwhelm users. Consider these improvements:
- Organize help text into sections with clear visual separation
- Use a table format for command options
- Implement progressive disclosure (show basic help first, detailed help with
--verbose)Example structure:
type HelpSection struct { Title string Basic []string // Shown by default Detailed []string // Shown with --verbose } var terraformHelp = []HelpSection{ { Title: "Usage", Basic: []string{ "atmos terraform <command> <component> -s <stack> [options]", "atmos terraform <command> <component> --stack <stack> [options]", }, }, { Title: "Common Commands", Basic: []string{ "plan Create execution plan", "apply Apply changes", "destroy Destroy resources", }, Detailed: []string{ "deploy Execute 'terraform apply -auto-approve'", "clean Delete temporary files", // ... more commands }, }, }Would you like me to help implement this improved help text structure?
Line range hint
12-120: Consider refactoring to improve code maintainability.The current implementation has several areas for improvement:
- Long if-else chains make the code hard to maintain
- Repeated string formatting patterns
- Help text content is mixed with display logic
Consider these refactoring suggestions:
- Extract help text content to separate configuration files:
// help_text.go var commandHelp = map[string]map[string]struct { Usage string Basic []string Detailed []string }{ "terraform": { "default": { Usage: "atmos terraform <command> <component> -s <stack> [options]", Basic: []string{ "Supports all native terraform commands", "Requires component and stack arguments", }, }, "clean": { Usage: "atmos terraform clean <component> -s <stack> <flags>", Basic: []string{ "Deletes temporary files and folders", }, Detailed: []string{ "Deletes: .terraform folder, TF_DATA_DIR, .terraform.lock.hcl", "Also removes: varfile, planfile, backend.tf.json", }, }, }, }
- Simplify the main function:
func processHelp(cliConfig schema.CliConfiguration, componentType string, command string) error { helpText := getHelpText(componentType, command) return displayHelp(cliConfig, helpText) }
- Add helper functions for formatting:
func displayHelp(config schema.CliConfiguration, help HelpText) error { if config.Verbose { return displayDetailedHelp(help) } return displayBasicHelp(help) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
internal/exec/help.go(1 hunks)
🔇 Additional comments (1)
internal/exec/help.go (1)
12-16: LGTM: Function signature change improves configurability.
The addition of cliConfig parameter allows for better control over help output behavior through configuration.
|
These changes were released in v1.99.0. |
what
settings.integrations.githubsection from Atmos stack manifests with theintegrations.githubsection fromatmos.yamlwhy
In Atmos manifests validation JSON schema, don't "hardcode" the
s3backend section fields, allow it to be a free-form map so the user can define any configuration for it. The Terraforms3backend can change, can be different for Terraform and OpenTofu. Also, the other backends (e.g. GCP, Azure, remote) are already free-form maps in the validation schemaWhen Atmos commands are executed w/o specifying a component and a stack (e.g.
atmos terraform plan,atmos terraform apply,atmos terraform clean), print help for the command w/o throwing errors that a component and stack are missingDeep-merging the
settings.integrations.githubsection from Atmos stack manifests with theintegrations.githubsection fromatmos.yamlallows configuring the global settings forintegrations.githubinatmos.yaml, and then override them in the Atmos stack manifests in thesettings.integrations.githubsection. Every component in every stack will getsettings.integrations.githubfromatmos.yaml. You can override any field in stack manifests. Atmos deep-merges theintegrations.githubvalues from all scopes in the following order (from the lowest to highest priority):integrations.githubsection fromatmos.yamlsettings.integrations.githubconfigured in Atmos stack manifests per Org, per tenant, per region, per accountsettings.integrations.githubsectionsettings.integrations.githubsectionFor example:
atmos.yamlstacks/catalog/vpc.yamlHaving the above config, the command
atmos describe component vpc -s tenant1-ue2-devreturns the following deep-merged configuration for the component'ssettings.integrations.githubsection:Summary by CodeRabbit
Release Notes
New Features
localproperty to backend definitions, increasing flexibility in configurations.Bug Fixes
Chores