-
Notifications
You must be signed in to change notification settings - Fork 2
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
Support terraform config generation from auditctx template #242
base: terraform-feature
Are you sure you want to change the base?
Support terraform config generation from auditctx template #242
Conversation
WalkthroughThe changes in this pull request involve updates to the Changes
Possibly related PRs
Suggested labels
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 (
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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: 1
🧹 Outside diff range and nitpick comments (4)
lib/core/terraform_config/audit_context.rb (1)
15-21
: Add documentation for the to_tf method.While the implementation is correct, adding documentation would help users understand the expected format and usage.
Add documentation like this:
+# Generates Terraform configuration for a Control Plane audit context resource. +# @return [String] Terraform configuration block def to_tf block :resource, :cpln_audit_context, name dospec/core/terraform_config/audit_context_spec.rb (1)
8-10
: Consider adding more test cases for edge scenarios.While the basic test setup is good, consider adding test cases for:
- Empty tags
- Missing optional fields
- Special characters in name/description
- Maximum length validation (if applicable)
lib/core/terraform_config/generator.rb (1)
4-4
: Consider refactoring to reduce class complexityThe Generator class is handling multiple template kinds with similar patterns but varying details. With the addition of audit context support, consider refactoring to improve maintainability:
- Create a base template handler class with common logic
- Implement specific handlers for each template kind (GVC, Workload, AuditContext, etc.)
- Use a factory pattern to instantiate the appropriate handler
This would:
- Reduce the class length
- Improve separation of concerns
- Make it easier to add new template kinds
- Make the code more testable
Would you like me to provide a detailed refactoring proposal?
spec/core/terraform_config/generator_spec.rb (1)
256-282
: The test implementation looks good but could benefit from additional coverage.The test context follows the project's patterns and correctly verifies basic functionality. However, consider enhancing the test coverage with:
- Validation of the generated Terraform configuration using
to_tf
method- Negative test cases (e.g., missing required fields)
Add the following test cases:
it "generates correct terraform configuration" do tf_config = generator.tf_configs["audit_contexts.tf"] expect(tf_config.to_tf).to eq( <<~HCL resource "cpln_audit_context" "audit-context-name" { name = "audit-context-name" description = "audit context description" tags = { tag1 = "tag1_value" tag2 = "tag2_value" } } HCL ) end context "with missing required fields" do let(:template) do { "kind" => "auditctx" # name is required but missing } end it "raises an error" do expect { generator.tf_configs }.to raise_error(ArgumentError, /name is required/) end end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
.gitignore
(1 hunks)lib/core/terraform_config/audit_context.rb
(1 hunks)lib/core/terraform_config/generator.rb
(4 hunks)spec/command/terraform/generate_spec.rb
(1 hunks)spec/core/terraform_config/audit_context_spec.rb
(1 hunks)spec/core/terraform_config/generator_spec.rb
(1 hunks)spec/dummy/.controlplane/templates/audit_context.yml
(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- .gitignore
- spec/command/terraform/generate_spec.rb
- spec/dummy/.controlplane/templates/audit_context.yml
🔇 Additional comments (11)
lib/core/terraform_config/audit_context.rb (3)
1-4
: LGTM! Well-structured class definition.
The file follows Ruby best practices with frozen_string_literal pragma and proper module namespacing.
5-5
: LGTM! Appropriate use of attribute readers.
The attributes align with the Terraform audit_context resource specification and are correctly defined as read-only.
15-21
: Verify the resource type and arguments match the provider schema.
The implementation looks correct, but let's verify it matches the Terraform provider's schema.
✅ Verification successful
Let me gather more information about the provider schema by searching for additional references and documentation.
Let me try one more search pattern to find any provider-related files or documentation.
Based on the gathered information, I can now provide the final response:
The resource type and arguments implementation is correct
The implementation matches the expected schema based on:
- Test files showing correct usage with all supported arguments (
name
,description
, andtags
) - Consistent usage across test fixtures and implementation
- Integration with the terraform config generator that properly handles this resource type
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the resource type and arguments against provider documentation
# Expected: Find references to cpln_audit_context resource and its arguments
# Search for provider schema or documentation files
fd -e tf -e md -e hcl . | xargs rg -l "cpln_audit_context"
# Search for example usage
rg -A 5 "resource \"cpln_audit_context\""
Length of output: 629
Script:
#!/bin/bash
# Search for provider documentation or schema files
fd "provider|README" -e md | xargs rg -l "audit.?context" -i
# Search for any other test files or examples using audit context
rg -g "!spec/*" "cpln_audit_context" -A 5
# Look for any schema definitions or validations
ast-grep --pattern 'schema "cpln_audit_context" {
$$$
}'
# Search for any resource definitions in test fixtures
fd "fixtures" | xargs rg "cpln_audit_context" -A 5
Length of output: 675
Script:
#!/bin/bash
# Search for terraform provider files and documentation
fd . -e tf -e hcl -e md | xargs rg "provider|resource" -l | xargs rg -l "control.?plane|cpln" -i
# Search specifically for audit context in any file
rg "audit.?context" -i -A 3 -B 3
# Look for any terraform block definitions
ast-grep --pattern 'terraform {
$$$
required_providers {
$$$
}
$$$
}'
Length of output: 6435
spec/core/terraform_config/audit_context_spec.rb (4)
1-4
: LGTM!
The file setup follows Ruby best practices with frozen_string_literal pragma and proper spec_helper inclusion.
11-18
: LGTM!
The test data setup is clear and provides a good representation of the expected audit context attributes.
19-33
: Verify the Terraform resource structure against documentation.
The expected output follows Terraform HCL syntax. However, let's verify that all required fields for cpln_audit_context
are covered and the structure matches the provider's documentation.
#!/bin/bash
# Description: Search for Terraform provider documentation or examples
# Expected: Find provider documentation or example configurations
# Search for Terraform provider files or documentation
fd -e tf -e md -e hcl | grep -i audit
Additionally, consider adding test cases that verify:
- Resource naming conventions
- Required vs optional fields
- Valid tag value formats
5-7
: Verify the existence of TerraformConfig::AuditContext class.
The test structure follows RSpec conventions. Let's verify that the tested class exists in the codebase.
✅ Verification successful
The TerraformConfig::AuditContext class exists and is properly defined
The class exists in lib/core/terraform_config/audit_context.rb
and is correctly defined within the TerraformConfig
module, matching the test structure in the spec file. The class inherits from Base
and includes the necessary attributes and methods that would be tested.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence of TerraformConfig::AuditContext class
# Expected: Find the class definition in lib/core/terraform_config/audit_context.rb
# Search for the class definition
ast-grep --pattern 'class AuditContext'
# Alternatively, search for the file
fd audit_context.rb
Length of output: 133
Script:
#!/bin/bash
# Let's try a more comprehensive search approach
# Search for any Ruby files containing 'audit_context'
rg -t ruby --ignore-case "audit_context"
# Search for class definition using ripgrep
rg -t ruby "class.*AuditContext"
# Look for Ruby files with audit in the name
fd -e rb audit
Length of output: 677
Script:
#!/bin/bash
# Let's verify the full class definition and namespace
cat lib/core/terraform_config/audit_context.rb
Length of output: 551
lib/core/terraform_config/generator.rb (4)
5-5
: LGTM: Clean addition of auditctx template kind
The addition of 'auditctx' to SUPPORTED_TEMPLATE_KINDS follows the existing pattern and maintains immutability through FREEZE.
59-63
: LGTM: Verify AuditContext class implementation
The config_class mapping for auditctx follows the established pattern. However, let's verify the implementation of the referenced class.
#!/bin/bash
# Verify the existence and implementation of AuditContext class
rg -g '*.rb' 'class\s+AuditContext' ./lib/core/terraform_config/
112-114
: Verify completeness of audit context parameters
The method extracts basic fields (name, description, tags), but might need additional parameters based on Terraform's audit_context resource requirements. Consider whether fields like audit rules, retention policies, or other context-specific configurations should be included.
Let's verify the required fields for audit contexts:
#!/bin/bash
# Search for audit context template examples and specs
rg -g '*.yml' -g '*.yaml' 'kind:\s*auditctx' -A 10
# Search for any existing audit context implementations
rg -g '*.rb' 'def.*audit.*context' -A 5
47-48
: Verify the audit_contexts.tf filename convention
The filename for auditctx differs from other patterns in the method. While most templates either use the name or pluralized kind, this uses a specific hardcoded name.
Let's verify if this naming convention aligns with existing audit context resources:
✅ Verification successful
The audit_contexts.tf filename convention is correct and consistent
Based on the evidence from the codebase:
- The
audit_contexts.tf
filename aligns with the resource typecpln_audit_context
defined in the AuditContext class - The test suite confirms this naming convention is expected and verified
- The implementation follows a consistent pattern where plural form is used for the resource type
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for existing audit context resource definitions
rg -g '*.tf' 'resource\s+"(\w+_audit_context|\w+_auditctx|\w+_audit_contexts)"'
# Search for any existing audit context file names
fd -e tf -x basename {} \; | grep -i 'audit'
Length of output: 204
Script:
#!/bin/bash
# Let's try a broader search to understand the audit context patterns
# Search for audit-related terms in terraform files
rg -g '*.tf' -i 'audit' -A 5 -B 5
# Search for terraform config generation related files
fd -e rb -x grep -l "terraform.*config" {} \;
# Look for similar case statements in generator
rg -g '*.rb' 'when.*".*ctx"' -A 2 -B 2
Length of output: 903
Script:
#!/bin/bash
# Let's examine the TerraformConfig::AuditContext class implementation
ast-grep --pattern 'class AuditContext'
# Look for any existing test files related to audit contexts
fd -e rb -x grep -l "audit.*context" {} \;
# Check the generator_spec for any audit context related tests
cat spec/core/terraform_config/generator_spec.rb
Length of output: 20062
terraform/ | ||
.controlplane/ | ||
/terraform/ | ||
/.controlplane/ |
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.
What does this PR do?
This PR adds support for converting templates with
type: auditctx
to terraform config formatTerraform docs
https://registry.terraform.io/providers/controlplane-com/cpln/latest/docs/resources/audit_context
Examples
Transforms to:
Summary by CodeRabbit
Release Notes
New Features
auditctx
, enhancing configuration generation capabilities.Bug Fixes
.gitignore
file for better organization.Tests
TerraformConfig::AuditContext
class and theGenerator
class to ensure correct functionality for the new template kind.