fix(datatables): run external database lifecycle on the caller's transaction, one lock order for fork cleanup

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

backend/ee-repo-ref.txt

@@ -1 +1 @@
-8dad0f8c24d8eeee049cd0623abc130921de2e88
+92a72d522c3934f46f146ac35f565cf4af97928d

backend/windmill-api-settings/src/lib.rs

@@ -1909,10 +1909,12 @@ async fn create_external_instance_pg_database(
 ) -> JsonResult<()> {
     require_super_admin(&db, &authed).await?;
     let tag = body.tag.as_deref().unwrap_or("datatable");
+    let mut tx = db.begin().await?;
     windmill_common::external_instance_pg::create_external_instance_database_unchecked(
-        &db, &dbname, tag, None,
+        &mut tx, &dbname, tag, None,
     )
     .await?;
+    tx.commit().await?;
     windmill_audit::audit_oss::audit_log(
         &db,
         &authed,
@@ -1933,10 +1935,12 @@ async fn drop_external_instance_pg_database(
 ) -> JsonResult<()> {
     require_super_admin(&db, &authed).await?;
     // A data table naming a dropped database fails on every job, far from the drop that caused it.
+    let mut tx = db.begin().await?;
     windmill_common::external_instance_pg::drop_external_instance_database_unchecked(
-        &db, &dbname, None,
+        &mut tx, &dbname, None,
     )
     .await?;
+    tx.commit().await?;
     windmill_audit::audit_oss::audit_log(
         &db,
         &authed,

backend/windmill-api-workspaces/src/workspaces.rs

@@ -3563,7 +3563,7 @@ async fn create_pg_database(
         }
         if source_kind == DataTableCatalogResourceType::ExternalInstance {
             windmill_common::external_instance_pg::create_external_instance_database_unchecked(
-                &db,
+                &mut tx,
                 &req.target_dbname,
                 "datatable",
                 Some(&w_id),

backend/windmill-api-workspaces/src/workspaces_extra.rs

@@ -1505,14 +1505,22 @@ pub async fn drop_forked_datatable_databases(
                             "the data table changed while it was being cleaned up".to_string(),
                         ));
                     }
+                    let external = resource_type
+                        == windmill_common::workspaces::DataTableCatalogResourceType::ExternalInstance;
+                    if external {
+                        // Before the governance lock, in the order settings saves and fork
+                        // finalization take the two.
+                        windmill_common::external_instance_pg::lock_external_instance_pg_state(
+                            &mut tx,
+                        )
+                        .await?;
+                    }
                     windmill_common::datatable_roles::lock_instance_databases_governance(
                         &mut tx,
                         [db_to_drop.as_str()],
                     )
                     .await?;
-                    if resource_type
-                        != windmill_common::workspaces::DataTableCatalogResourceType::ExternalInstance
-                    {
+                    if !external {
                         let uses = windmill_common::workspaces::managed_database_uses(
                             &mut tx,
                             windmill_common::workspaces::DataTableCatalogResourceType::Instance,
@@ -1537,12 +1545,10 @@ pub async fn drop_forked_datatable_databases(
                     .bind(&dt_name)
                     .execute(&mut *tx)
                     .await?;
-                    if resource_type
-                        == windmill_common::workspaces::DataTableCatalogResourceType::ExternalInstance
-                    {
+                    if external {
                         // Checks the uses of the database itself, and unregisters it.
                         windmill_common::external_instance_pg::drop_external_instance_database_unchecked(
-                            &db,
+                            &mut tx,
                             &db_to_drop,
                             Some((w_id.as_str(), dt_name.as_str())),
                         )

backend/windmill-common/src/external_instance_pg.rs

@@ -179,7 +179,7 @@ pub async fn external_instance_database_usages<'c>(
 /// Refuse to unset the cluster while Windmill still has databases on it, or a workspace still
 /// points at one: every data table there would stop resolving. Allowed on every edition, so a
 /// downgraded instance can still clear a setting it no longer uses.
-pub async fn ensure_external_instance_pg_removable(conn: &mut sqlx::PgConnection) -> Result<()> {
+async fn ensure_external_instance_pg_removable(conn: &mut sqlx::PgConnection) -> Result<()> {
     let state = read_external_instance_pg_state(&mut *conn).await?;
     let usages = external_instance_database_usages(&mut *conn).await?;
     if state.databases.is_empty() && usages.is_empty() {
@@ -231,16 +231,19 @@ pub async fn external_instance_connection_unchecked(
 /// Create `dbname` on the external cluster and register it. Refuses a name already taken there,
 /// whoever took it.
 ///
+/// Runs on `tx`, which it takes [`lock_external_instance_pg_state`] on: the registration lands when
+/// the caller commits. Take that lock before any database governance lock, as saves do.
+///
 /// Authorization: checks nothing. Callers MUST be superadmin, or be cloning a data table they may
 /// fork into a `wm_fork_` database.
 pub async fn create_external_instance_database_unchecked(
-    db: &DB,
+    tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
     dbname: &str,
     tag: &str,
     for_workspace: Option<&str>,
 ) -> Result<()> {
     crate::external_instance_pg_oss::create_external_instance_database_unchecked(
-        db,
+        tx,
         dbname,
         tag,
         for_workspace,
@@ -253,14 +256,17 @@ pub async fn create_external_instance_database_unchecked(
 /// ([`crate::workspaces::managed_database_uses`]), except the `exempt` data table entry: the fork
 /// copy being cleaned up.
 ///
+/// Runs on `tx`, like [`create_external_instance_database_unchecked`]: the database is gone at
+/// once, its registry entry when the caller commits.
+///
 /// Authorization: checks nothing. Callers MUST be superadmin, or be deleting the fork that owns
 /// this `wm_fork_` database.
 pub async fn drop_external_instance_database_unchecked(
-    db: &DB,
+    tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
     dbname: &str,
     exempt: Option<(&str, &str)>,
 ) -> Result<()> {
-    crate::external_instance_pg_oss::drop_external_instance_database_unchecked(db, dbname, exempt)
+    crate::external_instance_pg_oss::drop_external_instance_database_unchecked(tx, dbname, exempt)
         .await
 }
 

backend/windmill-common/src/external_instance_pg_oss.rs

@@ -63,7 +63,7 @@ mod ce {
     }
 
     pub(crate) async fn create_external_instance_database_unchecked(
-        _db: &DB,
+        _tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
         _dbname: &str,
         _tag: &str,
         _for_workspace: Option<&str>,
@@ -72,7 +72,7 @@ mod ce {
     }
 
     pub(crate) async fn drop_external_instance_database_unchecked(
-        _db: &DB,
+        _tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
         _dbname: &str,
         _exempt: Option<(&str, &str)>,
     ) -> Result<()> {