fix(rds): stop CreateDBInstance corrupting state when runtime absent (#2107) (#2109)

Author: vieiralucas

crates/fakecloud-rds/src/service/instances.rs

@@ -25,8 +25,8 @@ impl RdsService {
         let master_user_password = optional_query_param(request, "MasterUserPassword")
             .unwrap_or_else(|| "Password1!".to_string());
         let db_name = optional_query_param(request, "DBName");
-        let engine_version =
-            optional_query_param(request, "EngineVersion").unwrap_or_else(|| "16.3".to_string());
+        let engine_version = optional_query_param(request, "EngineVersion")
+            .unwrap_or_else(|| default_engine_version(&engine).to_string());
         let publicly_accessible =
             parse_optional_bool(optional_query_param(request, "PubliclyAccessible").as_deref())?
                 .unwrap_or(true);
@@ -94,6 +94,14 @@ impl RdsService {
             port,
         )?;
 
+        // Resolve the container runtime BEFORE reserving the identifier.
+        // If it is unavailable this returns early, and reserving first
+        // would leak the reservation into `in_progress_instance_ids`
+        // (nothing ever cancels it), corrupting the service: every later
+        // CreateDBInstance then fails with DBInstanceAlreadyExists while
+        // Describe/Delete see an empty instance map. See issue #2107.
+        let runtime = self.require_runtime()?.clone();
+
         {
             let mut accounts = self.state.write();
             let state = accounts.get_or_create(&request.account_id);
@@ -117,8 +125,6 @@ impl RdsService {
             }
         }
 
-        let runtime = self.require_runtime()?.clone();
-
         let logical_db_name = db_name
             .clone()
             .unwrap_or_else(|| default_db_name(&engine).to_string());

crates/fakecloud-rds/src/service_helpers.rs

@@ -2012,6 +2012,23 @@ pub fn default_port_for_engine(engine: &str) -> i32 {
     }
 }
 
+/// Pick the default `EngineVersion` for `engine` when the caller omits
+/// it. Must land on a version in that engine's supported list
+/// (`validate_create_request`) -- a fixed postgres default like `16.3`
+/// would make every version-less mysql/mariadb/oracle/... create fail
+/// with "EngineVersion '16.3' is not available". See issue #2107.
+pub(crate) fn default_engine_version(engine: &str) -> &'static str {
+    match engine {
+        "postgres" => "16.3",
+        "mysql" => "8.0",
+        "mariadb" => "10.11",
+        "oracle-ee" | "oracle-se2" | "oracle-ee-cdb" | "oracle-se2-cdb" => "19.0.0",
+        "sqlserver-ee" | "sqlserver-se" | "sqlserver-ex" | "sqlserver-web" => "15.00.4322.2.v1",
+        "db2-se" | "db2-ae" => "11.5.9.0.sb00000000.r1",
+        _ => "16.3",
+    }
+}
+
 /// Pick the built-in parameter group name AWS assigns to a new
 /// instance when the caller doesn't override it. The name encodes the
 /// engine family plus its major version (e.g. `default.postgres16`,

crates/fakecloud-rds/src/service_tests.rs

@@ -2832,3 +2832,79 @@ async fn snapshot_hook_fires_with_store() {
         .expect("hook present when a store is set");
     hook().await;
 }
+
+#[tokio::test]
+async fn repro_issue_2107_corrupt_state_when_runtime_absent() {
+    // No runtime -> require_runtime() fails. Reproduce issue #2107:
+    // a failed create must NOT leak the in-progress reservation.
+    let svc = make_service();
+    let create = request(
+        "CreateDBInstance",
+        &[
+            ("DBInstanceIdentifier", "corrupt-db"),
+            ("Engine", "mysql"),
+            ("EngineVersion", "8.0"),
+            ("DBInstanceClass", "db.t3.micro"),
+            ("MasterUsername", "admin"),
+            ("MasterUserPassword", "secretpass"),
+        ],
+    );
+
+    // First create fails because there is no container runtime.
+    let e1 = match svc.create_db_instance(&create).await {
+        Ok(_) => panic!("expected failure with no runtime"),
+        Err(e) => e,
+    };
+    assert_eq!(e1.code(), "InsufficientDBInstanceCapacity");
+
+    // Second create must ALSO be InsufficientDBInstanceCapacity, NOT
+    // DBInstanceAlreadyExists. A leaked reservation shows up here.
+    let e2 = match svc.create_db_instance(&create).await {
+        Ok(_) => panic!("expected failure with no runtime"),
+        Err(e) => e,
+    };
+    assert_ne!(
+        e2.code(),
+        "DBInstanceAlreadyExists",
+        "leaked in-progress reservation corrupts state"
+    );
+
+    let state = svc.state.read();
+    assert!(
+        state.default_ref().in_progress_instance_ids.is_empty(),
+        "failed create left a leaked reservation"
+    );
+}
+
+#[tokio::test]
+async fn create_without_engine_version_uses_engine_default() {
+    // Issue #2107: a version-less create must default EngineVersion to a
+    // version in the requested engine's supported list -- not a fixed
+    // postgres value like "16.3", which would make every version-less
+    // mysql/mariadb/oracle/... create fail validation with
+    // "EngineVersion '16.3' is not available" before it ever reaches the
+    // runtime. With no runtime the create still fails, but it must fail
+    // at require_runtime (InsufficientDBInstanceCapacity), proving
+    // validation passed with the engine-appropriate default.
+    let svc = make_service();
+    let create = request(
+        "CreateDBInstance",
+        &[
+            ("DBInstanceIdentifier", "mysql-nover"),
+            ("Engine", "mysql"),
+            ("DBInstanceClass", "db.t3.micro"),
+            ("MasterUsername", "admin"),
+            ("MasterUserPassword", "secretpass"),
+        ],
+    );
+    let err = match svc.create_db_instance(&create).await {
+        Ok(_) => panic!("expected failure with no runtime"),
+        Err(e) => e,
+    };
+    assert_eq!(
+        err.code(),
+        "InsufficientDBInstanceCapacity",
+        "version-less mysql create must pass validation via the engine default, \
+         not fail on a hardcoded postgres version"
+    );
+}