-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat!(schema): adds initial JSON schema + Stack Config changes #42
Conversation
WalkthroughThe pull request introduces a standardized configuration structure for stack files across multiple modules. A new Changes
Sequence DiagramsequenceDiagram
participant User
participant StackConfig
participant Automation
User->>StackConfig: Define stack configuration
StackConfig->>StackConfig: Validate against schema
StackConfig->>Automation: Pass configuration
Automation->>Automation: Process automation settings
Automation-->>User: Execute stack operations
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
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
examples/complete/root-modules/random-pet/stacks/dev.yaml (1)
Line range hint
1-5
: Fix environment mismatch in descriptionThe description mentions "Prod" but this is a dev environment configuration file.
kind: StackConfigV1 stack_settings: - description: This stack is used to create a random pet name for Prod! + description: This stack is used to create a random pet name for Dev! labels: - dev_stack_specific_label
🧹 Nitpick comments (6)
tests/fixtures/multi-instance/root-module-a/stacks/test.yaml (1)
1-4
: LGTM! Consider adding a description.The structure follows the new schema correctly. Consider adding a
description
field understack_settings
to document the purpose of this test configuration.examples/single-instance/root-modules/rds-cluster-prod/stack.yaml (1)
1-3
: Consider adding production-specific safeguards.While the configuration follows the new schema correctly, consider adding production-specific settings such as:
administrative: false
to prevent accidental changes- Specific labels for production environment tracking
examples/complete/root-modules/network/stacks/prod.yaml (1)
Line range hint
1-7
: Consider documenting safety settingsThe production-specific safety settings (
protect_from_deletion
andautoretry
) are good additions. Consider adding comments to document:
- The purpose of these settings
- When they should be enabled/disabled
- The implications of enabling them
stack-config.schema.json (2)
213-213
: Consider strengthening the cron schedule patternThe current pattern
^([0-9,\\-\\*]+\\s+){4}[0-9,\\-\\*]+$
allows some invalid cron expressions. Consider using a more restrictive pattern that validates ranges:- "pattern": "^([0-9,\\-\\*]+\\s+){4}[0-9,\\-\\*]+$" + "pattern": "^(((\\d+,)+\\d+|\\d+(\\/|-)\\d+|\\d+|\\*)(\\s+((\\d+,)+\\d+|\\d+(\\/|-)\\d+|\\d+|\\*))){4}$"
181-184
: Consider adding enum for terraform_versionFor better validation, consider adding an enum of supported Terraform versions:
"terraform_version": { "type": "string", + "enum": ["0.12.31", "0.13.7", "0.14.11", "0.15.5", "1.0.11", "1.1.9", "1.2.9", "1.3.9", "1.4.6", "1.5.7", "1.6.3"], "description": "Terraform version to use" },
README.md (1)
194-220
: Comprehensive example configuration!The example effectively demonstrates the new configuration structure. Consider adding a few comments to explain:
- The purpose of
kind: StackConfigV1
- When to use each automation setting
Add explanatory comments to the example:
# Specifies the configuration format version kind: StackConfigV1 # ... rest of the example ... # Settings that control the automation behavior automation_settings: # Use the default workspace instead of creating a new one default_tf_workspace_enabled: true # Control whether tfvars files should be loaded tfvars_enabled: false
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
README.md
(1 hunks)examples/complete/root-modules/network/stacks/dev.yaml
(1 hunks)examples/complete/root-modules/network/stacks/prod.yaml
(1 hunks)examples/complete/root-modules/random-pet/stacks/common.yaml
(1 hunks)examples/complete/root-modules/random-pet/stacks/dev.yaml
(1 hunks)examples/complete/root-modules/random-pet/stacks/prod.yaml
(1 hunks)examples/complete/root-modules/spacelift-automation/stacks/common.yaml
(1 hunks)examples/single-instance/root-modules/random-pet/stack.yaml
(1 hunks)examples/single-instance/root-modules/rds-cluster-dev/stack.yaml
(1 hunks)examples/single-instance/root-modules/rds-cluster-prod/stack.yaml
(1 hunks)examples/single-instance/root-modules/spacelift-automation-example2/stack.yaml
(1 hunks)main.tf
(2 hunks)stack-config.schema.json
(1 hunks)tests/fixtures/multi-instance/root-module-a/stacks/common.yaml
(1 hunks)tests/fixtures/multi-instance/root-module-a/stacks/default-example.yaml
(1 hunks)tests/fixtures/multi-instance/root-module-a/stacks/test.yaml
(1 hunks)tests/fixtures/single-instance/root-module-a/stack.yaml
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
main.tf (1)
Pattern **/*.tf
: You're a Terraform expert who has thoroughly studied all the documentation from Hashicorp https://developer.hashicorp.com/terraform/docs and OpenTofu https://opentofu.org/docs/.
You have a strong grasp of Terraform syntax and prioritize providing accurate and insightful code suggestions.
As a fan of the Cloud Posse / SweetOps ecosystem, you incorporate many of their best practices https://docs.cloudposse.com/best-practices/terraform/ while balancing them with general Terraform guidelines.
🔇 Additional comments (13)
examples/complete/root-modules/random-pet/stacks/common.yaml (1)
Line range hint
1-5
: Verify manage_state setting impact.The configuration follows the new schema correctly. Since
manage_state: true
is a critical setting, ensure all dependent stacks are aware of this configuration.Let's verify other stacks that might be affected:
✅ Verification successful
Manage state setting is properly configured
The
manage_state: true
setting is only defined in the common stack configuration, which is the appropriate location for shared settings. No other stack configurations override this setting, ensuring consistent state management across the module.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for manage_state references across stack configurations rg -l 'manage_state:' 'examples/complete/root-modules/**/stacks/*.{yaml,yml}'Length of output: 171
Script:
#!/bin/bash # First find all yaml/yml files in stacks directories fd -e yaml -e yml . examples/complete/root-modules | grep '/stacks/' | xargs rg -l 'manage_state:'Length of output: 157
examples/single-instance/root-modules/rds-cluster-dev/stack.yaml (1)
1-3
: LGTM! Well-documented configuration.The configuration follows the new schema and includes a clear description of its purpose.
tests/fixtures/multi-instance/root-module-a/stacks/common.yaml (1)
Line range hint
1-5
: LGTM! Verify label consistency across stacks.The configuration follows the new schema and includes good security practices with
administrative: false
.Let's verify label consistency across related stacks:
✅ Verification successful
Label usage follows stack-specific pattern - no issues found
Each stack file has its own unique identifying label, which appears to be an intentional design pattern:
- common.yaml: common_label
- test.yaml: test_label
- default-example.yaml: default_example_label
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for common_label usage across stack configurations rg -l 'common_label' 'tests/fixtures/multi-instance/root-module-a/stacks/*.{yaml,yml}'Length of output: 190
Script:
#!/bin/bash # Find all yaml files in stacks directories and search for common_label fd -e yaml -e yml . tests/fixtures/multi-instance | rg "stacks" | xargs rg -l "common_label" # Also search for other label patterns to understand the labeling scheme fd -e yaml -e yml . tests/fixtures/multi-instance | rg "stacks" | xargs rg "labels:"Length of output: 462
Script:
#!/bin/bash # Show the labels section with context from all stack yaml files fd -e yaml -e yml . tests/fixtures/multi-instance | rg "stacks" | xargs rg -A 3 "labels:"Length of output: 781
tests/fixtures/single-instance/root-module-a/stack.yaml (1)
Line range hint
1-7
: Addautomation_settings
section to align with new schemaThe new schema requires automation-related settings to be scoped under
automation_settings
. Consider adding this section if needed.Also, the
before_init
commandecho 'World'
appears to be a test placeholder. Consider using a more meaningful command or removing it if not needed for testing.examples/complete/root-modules/network/stacks/dev.yaml (1)
Line range hint
1-5
: Consider addingautomation_settings
section if neededWhile the stack settings look good, consider adding the
automation_settings
section if this stack requires any automation-related configurations (e.g.,default_tf_workspace_enabled
ortfvars_enabled
).examples/complete/root-modules/random-pet/stacks/prod.yaml (1)
Line range hint
1-5
: LGTM! Configuration is well-structuredThe stack configuration is properly formatted with appropriate description and prod-specific label.
tests/fixtures/multi-instance/root-module-a/stacks/default-example.yaml (1)
1-10
: Well-structured configuration changes!The reorganization of settings under
automation_settings
and the addition of thekind
field improve the configuration structure and versioning support.examples/single-instance/root-modules/random-pet/stack.yaml (1)
1-9
: Consistent implementation of new configuration structure!The changes maintain consistency with other stack configurations while preserving the existing functionality.
examples/single-instance/root-modules/spacelift-automation-example2/stack.yaml (1)
1-10
: Clean migration to new configuration structure!The changes successfully migrate the settings while maintaining the existing functionality.
examples/complete/root-modules/spacelift-automation/stacks/common.yaml (1)
1-14
: Excellent documentation with clear explanations!The added comments provide valuable context for configuration choices, particularly explaining the rationale behind
default_tf_workspace_enabled
andtfvars_enabled
settings.stack-config.schema.json (1)
1-8
: Well-structured schema using latest JSON Schema draft!The schema follows JSON Schema Draft 2020-12 specifications with appropriate vocabulary definitions.
README.md (2)
188-191
: Documentation updates look good!The section title and description clearly explain the new automation settings structure.
192-193
: Good addition of JSON schema reference!This helps users validate their configurations and improves the developer experience.
@@ -142,7 +142,7 @@ locals { | |||
"root_module" = module, | |||
|
|||
# If default_tf_workspace_enabled is true, use "default" workspace, otherwise our file name is the workspace name | |||
"terraform_workspace" = try(content.default_tf_workspace_enabled, local._default_tf_workspace_enabled) ? "default" : trimsuffix(file, ".yaml"), | |||
"terraform_workspace" = try(content.automation_settings.default_tf_workspace_enabled, local._default_tf_workspace_enabled) ? "default" : trimsuffix(file, ".yaml"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Breaking Change: Update stack configurations for default_tf_workspace_enabled.
The setting has been moved under the automation_settings
section. Existing stack configurations need to be updated.
Update your stack configuration files from:
default_tf_workspace_enabled: true
to:
automation_settings:
default_tf_workspace_enabled: true
@@ -253,7 +253,7 @@ | |||
for stack in local.stacks : stack => | |||
# tfvars are implicitly enabled in MultiInstance, which means we include the tfvars copy command in before_init | |||
# In SingleInstance, we expect the consumer to use an auto.tfvars file, so we don't include the tfvars copy command in before_init | |||
try(local.configs[stack].tfvars.enabled, local._multi_instance_structure) ? | |||
try(local.configs[stack].automation_settings.tfvars_enabled, local._multi_instance_structure) ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Breaking Change: Update stack configurations for tfvars_enabled.
The setting has been renamed and moved under the automation_settings
section. Existing stack configurations need to be updated.
Update your stack configuration files from:
tfvars:
enabled: true
to:
automation_settings:
tfvars_enabled: true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this idea! Glad to see this is solidifying more and more 💪
Also, I think this should be a major bump. We can keep schema versions corresponding to release major versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚢
* feat!(schema): adds initial JSON schema + StackConfig changes (#42) * chore(deps): update dependency opentofu/opentofu to v1.9.0 --------- Co-authored-by: Matt Gowie <[email protected]> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
BREAKING CHANGE
tfvars.enabled
in the Stack Config files totfvars_enabled
default_tf_workspace_enabled
+tfvars_enabled
under theautomation_settings
key in our Stack Config schema.what
default_tf_workspace_enabled
underautomation_settings
tfvars.enabled
totfvars_enabled
and scopes underautomation_settings
kind
so we can manage version / have a formal name for these configswhy
tfvars
settings... and if we do they can be separate?references
Summary by CodeRabbit
Documentation
Configuration Updates
kind: StackConfigV1
across multiple stack configuration filesautomation_settings
section in various stack configurationsSchema