feat(rds): add multi-engine support (PostgreSQL, MySQL, MariaDB)#220
feat(rds): add multi-engine support (PostgreSQL, MySQL, MariaDB)#220vieiralucas merged 5 commits intomainfrom
Conversation
- Add PostgreSQL versions: 16.3, 15.5, 14.10, 13.13 - Add MySQL versions: 8.0.35, 8.0.28, 5.7.44 - Add MariaDB versions: 10.11.6, 10.6.16 - Update runtime to select Docker images based on engine/version - Handle engine-specific environment variables (POSTGRES_*, MYSQL_*, MARIADB_*) - Default ports: 5432 (postgres), 3306 (mysql/mariadb) - Default parameter groups for all engine families - Update validation to support all engines and versions - Update all tests to account for multiple engines
There was a problem hiding this comment.
5 issues found across 4 files
You’re at about 87% of the monthly review limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/fakecloud-rds/src/service.rs">
<violation number="1" location="crates/fakecloud-rds/src/service.rs:1010">
P1: Snapshot restore is still PostgreSQL-only, so MySQL/MariaDB restores will fail after starting the new engine-specific container.</violation>
<violation number="2" location="crates/fakecloud-rds/src/service.rs:1153">
P1: Read-replica creation is still wired to `pg_dump`/`psql`, so the new MySQL/MariaDB paths cannot replicate data.</violation>
</file>
<file name="crates/fakecloud-conformance/tests/rds.rs">
<violation number="1" location="crates/fakecloud-conformance/tests/rds.rs:21">
P2: These assertions are too permissive for `DescribeDBEngineVersions`: they don't verify that the response is fully filtered to PostgreSQL or that all expected PostgreSQL versions are returned.</violation>
</file>
<file name="crates/fakecloud-rds/src/runtime.rs">
<violation number="1" location="crates/fakecloud-rds/src/runtime.rs:77">
P1: MySQL and MariaDB containers are still looked up on port 5432, so the new engine paths cannot start successfully.</violation>
<violation number="2" location="crates/fakecloud-rds/src/runtime.rs:179">
P2: The new MySQL/MariaDB path returns success after a fixed 5-second sleep without checking that the database is actually ready.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| let running = match runtime | ||
| .ensure_postgres( | ||
| &db_instance_identifier, | ||
| &source_instance.engine, |
There was a problem hiding this comment.
P1: Read-replica creation is still wired to pg_dump/psql, so the new MySQL/MariaDB paths cannot replicate data.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/fakecloud-rds/src/service.rs, line 1153:
<comment>Read-replica creation is still wired to `pg_dump`/`psql`, so the new MySQL/MariaDB paths cannot replicate data.</comment>
<file context>
@@ -1110,6 +1150,8 @@ impl RdsService {
let running = match runtime
.ensure_postgres(
&db_instance_identifier,
+ &source_instance.engine,
+ &source_instance.engine_version,
&source_instance.master_username,
</file context>
| let running = match runtime | ||
| .ensure_postgres( | ||
| &db_instance_identifier, | ||
| &snapshot.engine, |
There was a problem hiding this comment.
P1: Snapshot restore is still PostgreSQL-only, so MySQL/MariaDB restores will fail after starting the new engine-specific container.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/fakecloud-rds/src/service.rs, line 1010:
<comment>Snapshot restore is still PostgreSQL-only, so MySQL/MariaDB restores will fail after starting the new engine-specific container.</comment>
<file context>
@@ -969,6 +1007,8 @@ impl RdsService {
let running = match runtime
.ensure_postgres(
&db_instance_identifier,
+ &snapshot.engine,
+ &snapshot.engine_version,
&snapshot.master_username,
</file context>
| @@ -54,29 +54,84 @@ impl RdsRuntime { | |||
| pub async fn ensure_postgres( | |||
There was a problem hiding this comment.
P1: MySQL and MariaDB containers are still looked up on port 5432, so the new engine paths cannot start successfully.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/fakecloud-rds/src/runtime.rs, line 77:
<comment>MySQL and MariaDB containers are still looked up on port 5432, so the new engine paths cannot start successfully.</comment>
<file context>
@@ -54,29 +54,84 @@ impl RdsRuntime {
+ ];
+ (image, "5432", env_vars)
+ }
+ "mysql" => {
+ let major_version = if engine_version.starts_with("5.7") {
+ "5.7"
</file context>
| assert!(versions.len() >= 4); | ||
| assert!(versions.iter().any(|v| v.engine_version() == Some("16.3"))); |
There was a problem hiding this comment.
P2: These assertions are too permissive for DescribeDBEngineVersions: they don't verify that the response is fully filtered to PostgreSQL or that all expected PostgreSQL versions are returned.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/fakecloud-conformance/tests/rds.rs, line 21:
<comment>These assertions are too permissive for `DescribeDBEngineVersions`: they don't verify that the response is fully filtered to PostgreSQL or that all expected PostgreSQL versions are returned.</comment>
<file context>
@@ -17,10 +17,9 @@ async fn rds_describe_db_engine_versions() {
- assert_eq!(versions[0].engine_version(), Some("16.3"));
- assert_eq!(versions[0].db_parameter_group_family(), Some("postgres16"));
+ // Returns all postgres versions
+ assert!(versions.len() >= 4);
+ assert!(versions.iter().any(|v| v.engine_version() == Some("16.3")));
}
</file context>
| assert!(versions.len() >= 4); | |
| assert!(versions.iter().any(|v| v.engine_version() == Some("16.3"))); | |
| let engine_versions: std::collections::BTreeSet<_> = versions | |
| .iter() | |
| .map(|v| (v.engine(), v.engine_version(), v.db_parameter_group_family())) | |
| .collect(); | |
| assert_eq!( | |
| engine_versions, | |
| std::collections::BTreeSet::from([ | |
| (Some("postgres"), Some("13.13"), Some("postgres13")), | |
| (Some("postgres"), Some("14.10"), Some("postgres14")), | |
| (Some("postgres"), Some("15.5"), Some("postgres15")), | |
| (Some("postgres"), Some("16.3"), Some("postgres16")), | |
| ]) | |
| ); |
| } | ||
| } else { | ||
| // For MySQL/MariaDB, give it a moment to start | ||
| tokio::time::sleep(Duration::from_secs(5)).await; |
There was a problem hiding this comment.
P2: The new MySQL/MariaDB path returns success after a fixed 5-second sleep without checking that the database is actually ready.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/fakecloud-rds/src/runtime.rs, line 179:
<comment>The new MySQL/MariaDB path returns success after a fixed 5-second sleep without checking that the database is actually ready.</comment>
<file context>
@@ -109,12 +164,19 @@ impl RdsRuntime {
+ }
+ } else {
+ // For MySQL/MariaDB, give it a moment to start
+ tokio::time::sleep(Duration::from_secs(5)).await;
}
</file context>
- Add db.t3.small, db.t3.medium, db.t3.large - Add db.t4g.micro, db.t4g.small - Add db.m5.large - All instance classes available for all engines - 63 total orderable options (9 versions × 7 classes)
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
You’re at about 87% of the monthly review limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/fakecloud-rds/src/state.rs">
<violation number="1" location="crates/fakecloud-rds/src/state.rs:362">
P2: This adds a second source of truth for supported DB instance classes. Please share one constant/helper between `default_orderable_options()` and `validate_db_instance_class()` so the describe and create/modify APIs cannot drift apart.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| ("mariadb", "10.6.16", "general-public-license"), | ||
| ]; | ||
|
|
||
| let instance_classes = vec![ |
There was a problem hiding this comment.
P2: This adds a second source of truth for supported DB instance classes. Please share one constant/helper between default_orderable_options() and validate_db_instance_class() so the describe and create/modify APIs cannot drift apart.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/fakecloud-rds/src/state.rs, line 362:
<comment>This adds a second source of truth for supported DB instance classes. Please share one constant/helper between `default_orderable_options()` and `validate_db_instance_class()` so the describe and create/modify APIs cannot drift apart.</comment>
<file context>
@@ -359,16 +359,28 @@ pub fn default_orderable_options() -> Vec<OrderableDbInstanceOption> {
("mariadb", "10.6.16", "general-public-license"),
];
+ let instance_classes = vec![
+ "db.t3.micro",
+ "db.t3.small",
</file context>
Add metadata-only fields for Batches 7-10: - BackupRetentionPeriod (default 1 day) - PreferredBackupWindow (default 03:00-04:00) - LatestRestorableTime (set to created_at) - OptionGroupName (optional) - MultiAZ (boolean flag) Changes: - Parse new parameters in CreateDBInstance - Store fields in DbInstance struct - Render in XML responses - Copy from source for read replicas - Update all test fixtures These are metadata-only - no actual backup automation or multi-AZ standby provisioning implemented.
There was a problem hiding this comment.
3 issues found across 3 files (changes from recent commits).
You’re at about 87% of the monthly review limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/fakecloud-rds/src/state.rs">
<violation number="1" location="crates/fakecloud-rds/src/state.rs:39">
P2: Make `latest_restorable_time` optional. A mandatory timestamp forces backup-disabled instances (`BackupRetentionPeriod=0`) to look point-in-time restorable.</violation>
</file>
<file name="crates/fakecloud-rds/src/service.rs">
<violation number="1" location="crates/fakecloud-rds/src/service.rs:1082">
P1: Snapshot/replica paths are still PostgreSQL-only for MySQL/MariaDB instances.</violation>
<violation number="2" location="crates/fakecloud-rds/src/service.rs:2174">
P2: MySQL/MariaDB instance XML still reports a PostgreSQL license model.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @@ -128,10 +128,49 @@ impl RdsService { | |||
| let deletion_protection = | |||
There was a problem hiding this comment.
P1: Snapshot/replica paths are still PostgreSQL-only for MySQL/MariaDB instances.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/fakecloud-rds/src/service.rs, line 1082:
<comment>Snapshot/replica paths are still PostgreSQL-only for MySQL/MariaDB instances.</comment>
<file context>
@@ -1065,6 +1079,11 @@ impl RdsService {
read_replica_db_instance_identifiers: Vec::new(),
vpc_security_group_ids,
db_parameter_group_name: None,
+ backup_retention_period: 1,
+ preferred_backup_window: "03:00-04:00".to_string(),
+ latest_restorable_time: created_at,
</file context>
| pub db_parameter_group_name: Option<String>, | ||
| pub backup_retention_period: i32, | ||
| pub preferred_backup_window: String, | ||
| pub latest_restorable_time: DateTime<Utc>, |
There was a problem hiding this comment.
P2: Make latest_restorable_time optional. A mandatory timestamp forces backup-disabled instances (BackupRetentionPeriod=0) to look point-in-time restorable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/fakecloud-rds/src/state.rs, line 39:
<comment>Make `latest_restorable_time` optional. A mandatory timestamp forces backup-disabled instances (`BackupRetentionPeriod=0`) to look point-in-time restorable.</comment>
<file context>
@@ -34,6 +34,11 @@ pub struct DbInstance {
pub db_parameter_group_name: Option<String>,
+ pub backup_retention_period: i32,
+ pub preferred_backup_window: String,
+ pub latest_restorable_time: DateTime<Utc>,
+ pub option_group_name: Option<String>,
+ pub multi_az: bool,
</file context>
| @@ -128,10 +128,49 @@ impl RdsService { | |||
| let deletion_protection = | |||
There was a problem hiding this comment.
P2: MySQL/MariaDB instance XML still reports a PostgreSQL license model.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/fakecloud-rds/src/service.rs, line 2174:
<comment>MySQL/MariaDB instance XML still reports a PostgreSQL license model.</comment>
<file context>
@@ -2147,6 +2171,19 @@ fn db_instance_xml(instance: &DbInstance, status_override: Option<&str>) -> Stri
None => "<DBParameterGroups/>".to_string(),
};
+ let option_group_memberships_xml = match &instance.option_group_name {
+ Some(og_name) => format!(
+ "<OptionGroupMemberships>\
</file context>
Add full MySQL/MariaDB support to RDS implementation: - Add mysql_async dependency for MySQL/MariaDB connection testing - Implement wait_for_mysql() with proper connection validation - Update lookup_port() to accept port parameter (5432 for postgres, 3306 for mysql/mariadb) - Update restart_container() to support all engine types - Replace sleep-based MySQL/MariaDB readiness with actual connection tests Addresses Cubic P1 issue from PR #220: MySQL/MariaDB features were incomplete (snapshot restore, read replicas, port lookup all PostgreSQL-only).
Summary
Adds support for multiple database engines and versions in RDS:
PostgreSQL (4 versions):
MySQL (3 versions):
MariaDB (2 versions):
Implementation:
Test plan
Summary by cubic
Adds multi-engine RDS support (PostgreSQL, MySQL, MariaDB) across 9 versions and 7 instance classes, plus backup and Multi-AZ metadata on instances. The runtime picks engine-aware Docker images, ports, defaults, and validation, with 63 orderable options exposed by the API.
New Features
db.t3.micro,db.t3.small,db.t3.medium,db.t3.large,db.t4g.micro,db.t4g.small,db.m5.large.POSTGRES_*,MYSQL_*,MARIADB_*env vars; postgres readiness check; short startup wait for MySQL/MariaDB.default.postgres16,default.mysql8.0,default.mariadb10.11), engine-based default DB name (postgresormysql), and port.BackupRetentionPeriod(default 1),PreferredBackupWindow(default 03:00-04:00),LatestRestorableTime(set to created_at),OptionGroupName(optional),MultiAZ(boolean). Parsed on create, stored, rendered in responses, and copied to read replicas.Tests
Written for commit c506290. Summary will update on new commits.