Add multiple database engine support (MySQL, MariaDB, PostgreSQL, Aurora) - Breaking Change #19
Conversation
Co-authored-by: leek <[email protected]>
Co-authored-by: leek <[email protected]>
Co-authored-by: leek <[email protected]>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThis PR adds multi-engine database support (MySQL, MariaDB, PostgreSQL) and Aurora deployment options to the infrastructure. It introduces engine-agnostic configuration, Aurora Serverless v2 capabilities, and updates modules to dynamically select database ports, connection logic, and provisioning steps based on the chosen database engine. Changes
Sequence Diagram(s)sequenceDiagram
participant root as Root Module<br/>(main.tf)
participant db as Database Module
participant rds as RDS Instance
participant aurora as Aurora Cluster
participant bastion as Bastion Host
participant sg as Security Groups
root->>db: Pass db_engine,<br/>db_engine_version,<br/>aurora_* vars
rect rgba(200, 220, 255, 0.3)
note over db: Engine Config Selection
db->>db: is_aurora =<br/>(db_engine contains 'aurora')
end
alt is_aurora
db->>aurora: Create Aurora cluster<br/>with engine config
aurora-->>db: cluster_endpoint,<br/>cluster_id
db-->>root: is_aurora=true,<br/>endpoint=Aurora endpoint
else RDS path
db->>rds: Create RDS instance<br/>with engine config
rds-->>db: db_instance_endpoint,<br/>db_instance_identifier
db-->>root: is_aurora=false,<br/>endpoint=RDS endpoint
end
root->>root: db_port_map:<br/>select port by engine
root->>sg: Apply db_port
rect rgba(200, 255, 220, 0.3)
note over root,bastion: Bastion Setup
root->>bastion: Pass db_engine,<br/>db_port,<br/>db_endpoint
end
alt db_engine == 'postgres'
bastion->>bastion: Install postgresql-client<br/>wait for port 5432
bastion->>bastion: Create users via psql
else MySQL/MariaDB
bastion->>bastion: MySQL client present<br/>wait for port 3306
bastion->>bastion: Create users via mysql
end
bastion-->>root: Database ready
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes
Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
terraform/environments/example.tfvars (1)
100-156: I'll search for the current default versions for each database engine from AWS documentation.Let me search for MariaDB version information to complete the verification.
Update version numbers and clarify AWS default version behavior.
MySQL 8.0.43 is available on Amazon RDS, which matches the comment. However, the other versions listed are outdated:
- MariaDB's latest supported version is 10.11.15, not 10.11.9
- Aurora PostgreSQL-Compatible Edition supports PostgreSQL 17.5, 16.9, and other versions, making 16.4 outdated
- For RDS PostgreSQL, newer versions like 16.9 are available
- Aurora versions include MySQL 3.08.2 (compatible with MySQL 8.0.39), not 3.07.1
Additionally, AWS defaults to a supported version, typically the most recent version, though defaults vary by region. The comment should reference AWS CLI commands to check current defaults rather than listing static version numbers, since they change frequently and vary by region:
aws rds describe-db-engine-versions --engine <engine> --region <region> --query '*[?Preferred==`true`]'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
README.md(10 hunks)terraform/environments/example.tfvars(2 hunks)terraform/main.tf(6 hunks)terraform/modules/bastion/main.tf(1 hunks)terraform/modules/bastion/user_data.sh(4 hunks)terraform/modules/bastion/variables.tf(1 hunks)terraform/modules/database/main.tf(6 hunks)terraform/modules/database/outputs.tf(1 hunks)terraform/modules/database/variables.tf(1 hunks)terraform/modules/networking/main.tf(1 hunks)terraform/modules/networking/variables.tf(1 hunks)terraform/variables.tf(3 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md
286-286: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
301-301: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
352-352: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
359-359: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
388-388: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
1017-1017: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
1022-1022: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
1034-1034: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
1040-1040: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🔇 Additional comments (23)
terraform/modules/bastion/variables.tf (2)
146-150: LGTM! Engine variable properly defined.The
db_enginevariable is well-structured with a sensible default that maintains backward compatibility. Validation is appropriately handled at the root module level.
152-156: LGTM! Port variable properly defined.The
db_portvariable has an appropriate default that aligns with the MySQL default, maintaining backward compatibility.README.md (4)
26-46: LGTM! Feature documentation accurately reflects multi-engine support.The updated feature list clearly communicates the expanded database capabilities, including both RDS and Aurora options.
472-498: Excellent production configuration examples.Providing three distinct database configuration options (RDS Multi-AZ, Aurora Serverless v2, and Aurora Provisioned) with clear comments helps users choose the right setup for their needs.
1015-1046: LGTM! Database scaling documentation is clear and comprehensive.The scaling section effectively distinguishes between RDS and Aurora capabilities, including Aurora's automatic storage scaling and Serverless v2 options.
343-348: ACU specifications are accurate—no changes required.The ACU-to-RAM mappings in the README are mathematically correct per AWS documentation: 1 ACU ≈ 2 GiB RAM, which validates all listed ratios (0.5 ACU = ~1GB, 2–4 ACU = ~4–8GB, 8+ ACU = ~16GB+). AWS confirms 0.5-ACU increments are the supported minimum. The specifications require no updates.
terraform/variables.tf (1)
174-220: LGTM! Updated variable descriptions accurately distinguish RDS and Aurora.The updated descriptions appropriately clarify which variables apply to RDS vs Aurora, helping users understand the differences.
terraform/modules/database/outputs.tf (2)
2-35: LGTM! Conditional output logic properly handles Aurora vs RDS.The conditional logic correctly routes to Aurora or RDS module outputs based on
local.is_aurora, with appropriate null fallbacks. Note that output names retain the "rds_" prefix for backward compatibility, even though they now support Aurora.
37-50: LGTM! New outputs support engine-agnostic configuration.The new outputs (
rds_port,db_engine,is_aurora) provide necessary information for downstream modules to handle different database engines correctly. All local variables are properly defined with correct engine-to-port mappings: mysql/mariadb use port 3306, postgres/aurora-postgresql use port 5432, andcurrent_enginecorrectly selects the configuration based onvar.db_engine.terraform/modules/networking/variables.tf (1)
32-36: LGTM! Database port variable enables engine-agnostic networking.The
db_portvariable allows security group rules to dynamically configure the correct port based on the database engine.terraform/modules/networking/main.tf (1)
75-103: LGTM! Security group rules properly updated for multi-engine support.The security group now uses
var.db_portfor dynamic port configuration, replacing the hardcoded "mysql-tcp" rule. This enables support for PostgreSQL (port 5432) and other engines while maintaining the same security posture.terraform/environments/example.tfvars (1)
157-182: LGTM! Aurora-specific configuration is well-documented.The Aurora configuration section provides clear guidance on Serverless v2 vs provisioned instances, with practical ACU recommendations for different environments.
terraform/modules/bastion/main.tf (1)
122-136: Verification successful: User data template correctly implements engine-specific provisioning.The user_data.sh template properly handles the
db_engineparameter with engine-specific logic for MySQL/MariaDB and PostgreSQL variants. Database readiness checks and user creation commands are differentiated appropriately for each engine family. The setup_database_user gate correctly controls when user provisioning occurs.terraform/modules/bastion/user_data.sh (2)
53-54: Verify: PostgreSQL client installation should likely be conditional.PostgreSQL client is installed unconditionally (line 53-54), unlike MySQL client which is gated by
install_mysql_client(line 48-51). If the template is engine-aware and only instantiates the database setup when needed, consider making PostgreSQL installation conditional via a template variable (e.g.,%{ if install_postgres_client ~}), to align with the MySQL pattern and avoid unnecessary tooling on MySQL-only deployments.Can you verify whether this unconditional install is intentional (defensive tooling) or should follow the MySQL pattern with a template variable?
109-220: Engine-aware database user setup is well-structured.The conditional branching on
DB_ENGINE(lines 124-140) correctly validates supported engines and gates the entire setup. The MySQL and PostgreSQL user creation paths (lines 160-211) are appropriately implemented:
- MySQL path uses standard
CREATE USER IF NOT EXISTSandGRANTstatements- PostgreSQL path uses
DOblocks to safely handle conditional user creation and properly configures default privilegesThe 30-attempt readiness check (lines 142-157) with 10-second intervals provides adequate time for database startup.
terraform/main.tf (3)
29-37: Database port mapping is correct and properly derived.The
db_port_maplocal correctly maps all supported database engines to their standard ports (3306 for MySQL family, 5432 for PostgreSQL family). The lookup onvar.db_engineis safe because the variable has validation interraform/modules/database/variables.tf.
151-165: Database module variables are properly forwarded.All new Aurora and engine configuration variables (db_engine, db_engine_version, and aurora_*) are correctly passed to the database module. The forwarding pattern is consistent with the PR objectives.
384-385: No issues found. The rds_port output is correctly defined and implemented.The output is properly defined at line 37 to use
local.current_engine.port, which maps to the correct port for each database engine (3306 for MySQL/Aurora MySQL, 5432 for PostgreSQL/Aurora PostgreSQL). The output is correctly referenced in the security group rule at lines 394-395, and the port configuration logic is consistently applied across RDS and Aurora modules.terraform/modules/database/main.tf (5)
6-55: Database engine configuration is well-structured and maintainable.The engine configuration locals approach (lines 6-55) is clear and follows a good pattern:
is_auroradetection is simple and reliableengine_configmap centralizes all per-engine settings (engine name, default version, parameter family, port)current_enginelookup is straightforward- Default versions match PR objectives (MySQL 8.0.43, MariaDB 10.11.9, PostgreSQL 16.4, Aurora variants)
The conditional instance class selection for Aurora Serverless v2 (line 54) is correct.
86-86: Conditional RDS vs Aurora deployment is correct.The count-based gating correctly separates concerns:
- RDS module only creates when
!is_aurora(line 86)- Read replicas only create for RDS (line 157)
- Aurora module only creates when
is_aurora(line 224)This prevents resource conflicts and aligns with the multi-engine design.
Also applies to: 157-157, 224-224
233-245: Verify: Aurora instances map and serverlessv2_scaling_configuration syntax.The instances map (lines 233-239) dynamically creates either a single serverless instance or multiple provisioned instances based on
aurora_enable_serverlessv2. The serverlessv2_scaling_configuration block (lines 242-245) is conditionally set only when serverless is enabled.This pattern appears correct, but should be verified against
terraform-aws-modules/rds-aurorav9.0 module documentation to ensure:
- The instances map structure matches module expectations
- The scaling_configuration is correctly applied
Can you verify this syntax against the rds-aurora module v9.0 documentation, particularly for:
- How the
instancesmap should be structured- Whether
serverlessv2_scaling_configurationis the correct variable name- Whether the on/off pattern with
nullvs configuration object works as expected
297-315: Aurora supporting resources are properly gated.The Aurora subnet group (lines 297-305) and data sources (lines 308-315) are correctly gated by
count = local.is_aurora ? 1 : 0, ensuring they're only created when using Aurora engines. The references in the Aurora module (lines 248, 251) safely use array indexing[0].
289-289: Confirmed: Aurora availability zones logic is correct.When
availability_zonesis set tonull, AWS RDS/Aurora auto-distributes the cluster across available AZs (multi-AZ behavior); when explicitly set to a single AZ, it restricts to that zone. The code correctly implements this:multi_az ? null : [single_az]properly controls multi-AZ vs single-AZ deployments as intended. The logic is not inverted.
| db_subnet_group_name = aws_db_subnet_group.aurora[0].name | ||
| security_group_rules = { | ||
| vpc_ingress = { | ||
| cidr_blocks = [data.aws_vpc.selected[0].cidr_block] | ||
| } | ||
| } | ||
|
|
||
| # Use security group from networking module | ||
| create_security_group = false | ||
| vpc_security_group_ids = [var.rds_security_group_id] |
There was a problem hiding this comment.
🧩 Analysis chain
Verify: Aurora security group configuration pattern.
The Aurora module passes security_group_rules (lines 249-253) while also setting create_security_group = false (line 256) and providing vpc_security_group_ids (line 257). This pattern needs verification with the rds-aurora module to ensure:
security_group_rulesis the correct way to define intra-VPC rules when not creating the security group- The module correctly applies these rules to the externally-provided security group
Alternatively, consider whether the security_group_rules block is even necessary here, since the bastion-to-RDS rule is already defined separately in terraform/main.tf (lines 391-400).
🌐 Web query:
rds-aurora Terraform module v9.0 security group configuration pattern with create_security_group false
💡 Result:
Use an existing SG and pass it into the module instead of letting the module create one.
Recommended pattern
- set create_security_group = false
- provide the existing security-group id(s) via the module input the module expects (vpc_security_group_ids / security_group_id / allowed_security_groups depending on the module version)
- ensure the module call concatenates only when an SG exists (module code v9.x historically had a bug when it always referenced aws_security_group.this[0] — see note below)
Example (minimal):
- if the module expects vpc_security_group_ids:
module "db" {
source = "terraform-aws-modules/rds-aurora/aws"
version = "9.0.0"
create_security_group = false
vpc_security_group_ids = [aws_security_group.rds_existing.id] # <-- supply existing SG(s)
...
}
If the module input name is security_group_id or allowed_security_groups use that input instead.
Note about v9.x bug
- Older v9.x module code referenced aws_security_group.this[0] unconditionally and fails when create_security_group = false. The workaround/fix is to pass only existing SG ids into the module input (so the module doesn’t need to reference an internal SG), or patch the module to use a conditional/ternary (e.g. compact(concat(var.create_security_group ? [aws_security_group.this[0].id] : [], var.vpc_security_group_ids))). See module docs and GH discussion. [1][2]
Sources
- module documentation (conditional creation / create_security_group). [1]
- community/issue note showing the create_security_group=false index error and ternary workaround. [2]
I need to verify a critical detail about the security_group_rules block in this context. Let me search for more specific information about whether this input is applicable when using an external security group.
I need to verify whether the rds-aurora module actually accepts security_group_rules as an input, and inspect the codebase to understand the module configuration.
The search results show that the module uses security_group_ingress_rules and security_group_egress_rules, not security_group_rules. Let me verify the exact module version and confirm the proper input variables.Let me directly inspect the codebase to verify the module version and inputs being used:
Actually, I need to clone the repo. Let me fix that:
🏁 Script executed:
#!/bin/bash
# Find the rds-aurora module source and version
grep -r "source.*rds-aurora" terraform/modules/database/
# Also look for version constraint
grep -r "version" terraform/modules/database/main.tf | head -20Length of output: 1179
🏁 Script executed:
#!/bin/bash
# Inspect the Aurora module configuration
cat terraform/modules/database/main.tf | grep -A 30 "module.*aurora"
# Check if there's a version specification
grep -n "terraform-aws-modules/rds-aurora" terraform/modules/database/main.tfLength of output: 1107
Let me verify what inputs are actually accepted by the rds-aurora module v9.0:
Based on my verification, the security_group_rules variable (lines 249-253) is not a valid input for the rds-aurora module v9.0. The module uses security_group_ingress_rules and security_group_egress_rules, not security_group_rules. This block will be silently ignored by the module and represents dead code that should be removed.
The core configuration pattern is correct: setting create_security_group = false with an externally provided security group via vpc_security_group_ids is the recommended approach. The module's implementation properly handles this using a try() pattern to avoid referencing the non-existent internal security group when create_security_group = false.
Remove the invalid security_group_rules block (lines 249-253). This variable is not recognized by the rds-aurora v9.0 module and serves no purpose. The recommended security group pattern—create_security_group = false with vpc_security_group_ids—is already correctly configured.
🤖 Prompt for AI Agents
In terraform/modules/database/main.tf around lines 248 to 257, remove the
invalid security_group_rules block (lines 249-253) because rds-aurora v9.0
expects security_group_ingress_rules/security_group_egress_rules and will ignore
security_group_rules; keep create_security_group = false and
vpc_security_group_ids = [var.rds_security_group_id] as-is to supply the
external SG.
| variable "aurora_min_capacity" { | ||
| description = "Minimum Aurora Capacity Units (ACUs) for Serverless v2. 0.5 to 128 in 0.5 increments." | ||
| type = number | ||
| default = 0.5 | ||
| } | ||
|
|
||
| variable "aurora_max_capacity" { | ||
| description = "Maximum Aurora Capacity Units (ACUs) for Serverless v2. 0.5 to 128 in 0.5 increments." | ||
| type = number | ||
| default = 1.0 | ||
| } |
There was a problem hiding this comment.
Add validation for Aurora Serverless v2 capacity parameters.
The aurora_min_capacity and aurora_max_capacity variables lack validation. According to AWS Aurora Serverless v2 documentation, valid values are 0.5 to 128 in 0.5 increments, and min must be ≤ max. Without validation, invalid configurations could be provisioned, causing Terraform errors or unexpected behavior.
Add validation blocks to enforce the valid range and logical constraints:
variable "aurora_min_capacity" {
description = "Minimum Aurora Capacity Units (ACUs) for Serverless v2. 0.5 to 128 in 0.5 increments."
type = number
default = 0.5
+ validation {
+ condition = var.aurora_min_capacity >= 0.5 && var.aurora_min_capacity <= 128 && floor(var.aurora_min_capacity * 2) == var.aurora_min_capacity * 2
+ error_message = "aurora_min_capacity must be between 0.5 and 128 in 0.5 increments."
+ }
}
variable "aurora_max_capacity" {
description = "Maximum Aurora Capacity Units (ACUs) for Serverless v2. 0.5 to 128 in 0.5 increments."
type = number
default = 1.0
+ validation {
+ condition = var.aurora_max_capacity >= 0.5 && var.aurora_max_capacity <= 128 && floor(var.aurora_max_capacity * 2) == var.aurora_max_capacity * 2 && var.aurora_max_capacity >= var.aurora_min_capacity
+ error_message = "aurora_max_capacity must be between 0.5 and 128 in 0.5 increments and >= aurora_min_capacity."
+ }
}Note: The validation uses floor(x * 2) == x * 2 to check for 0.5-increment multiples (accounting for floating-point precision).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In terraform/modules/database/variables.tf around lines 111 to 121, the
aurora_min_capacity and aurora_max_capacity variables lack validation; add
Terraform validation blocks for each to enforce numeric range 0.5..128 and 0.5
increments (use the floor(x * 2) == x * 2 check to allow 0.5 steps), and add a
cross-variable validation to ensure aurora_min_capacity <= aurora_max_capacity
(e.g., validate in aurora_min_capacity or a separate local/validation block that
references var.aurora_max_capacity), returning a clear error message when
constraints are violated.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@copilot will this setup "just work" with current configurations using mysql? or do resources need to be imported? |
Good catch! The current implementation has a breaking change due to adding Issue: Module address changes from Migration required: # For existing MySQL deployments
terraform state mv 'module.database.module.rds' 'module.database.module.rds[0]'
terraform state mv 'module.database.module.rds_read_replica' 'module.database.module.rds_read_replica[0]'I'll update the implementation to avoid this breaking change by using a conditional approach that doesn't change the resource address for the default MySQL case. Working on a fix now.
No, existing MySQL configurations require state migration to avoid database recreation. Issue: Adding Required migration: terraform state mv 'module.database.module.rds' 'module.database.module.rds[0]'
terraform state mv 'module.database.module.rds_read_replica' 'module.database.module.rds_read_replica[0]'See MIGRATION.md for complete step-by-step instructions. Fixed in commit 88425f7 with comprehensive migration documentation. |
Co-authored-by: leek <[email protected]>
Description
Extends database support from MySQL-only to five engines: MySQL, MariaDB, PostgreSQL, Aurora MySQL, and Aurora PostgreSQL. Adds Aurora Serverless v2 with automatic scaling (0.5-128 ACUs) and Aurora provisioned clusters with configurable instance counts.
Warning
Breaking Change for Existing Deployments: This PR requires Terraform state migration for existing MySQL deployments to avoid database recreation. See MIGRATION.md for detailed instructions.
Key Changes:
db_enginevariable. Engine-specific parameter families, port mapping (3306/5432), and version defaults.db_engine,db_engine_version,aurora_*configuration options with validation.Configuration Example:
Default Versions:
Type of Change
Testing
Testing Approach:
Screenshots (if applicable)
N/A - Infrastructure as Code changes
Related Issues
Closes #18
Checklist
Additional Notes
Breaking Change - State Migration Required:
Existing MySQL deployments require Terraform state migration to avoid database recreation. The module now uses conditional logic (
count) to support both RDS and Aurora, which changes resource addresses frommodule.database.module.rdstomodule.database.module.rds[0].Migration Commands:
See MIGRATION.md for complete step-by-step instructions including prerequisites, verification steps, troubleshooting, and rollback procedures.
For New Deployments: No migration needed - configure and deploy as normal.
Engine Switching: Users can switch engines by updating
db_engineand redeploying. PostgreSQL requires application code changes (different SQL syntax). Aurora can be created from RDS snapshots.Files Modified:
terraform/modules/database/- Multi-engine support, Aurora cluster configurationterraform/modules/networking/- Dynamic port-based security rulesterraform/modules/bastion/- PostgreSQL client, engine-specific user creationterraform/variables.tf- New database configuration optionsREADME.md- Comprehensive engine comparison, configuration guide, and migration warningsterraform/environments/example.tfvars- Complete configuration examplesMIGRATION.md- Detailed migration guide for existing deployments (new file)Release Notes: This PR will be included in the next release. The PR title and labels will be used to generate release notes automatically.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
Summary by CodeRabbit
New Features
Documentation