diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index a2af4f888a..66137d09db 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -8dad0f8c24d8eeee049cd0623abc130921de2e88 +92a72d522c3934f46f146ac35f565cf4af97928d diff --git a/backend/windmill-api-settings/src/lib.rs b/backend/windmill-api-settings/src/lib.rs index a630742387..9818c3a9c7 100644 --- a/backend/windmill-api-settings/src/lib.rs +++ b/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, diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 88ea96de29..b5ae865df6 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/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), diff --git a/backend/windmill-api-workspaces/src/workspaces_extra.rs b/backend/windmill-api-workspaces/src/workspaces_extra.rs index 96ffd1bd08..e17eeab08f 100644 --- a/backend/windmill-api-workspaces/src/workspaces_extra.rs +++ b/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())), ) diff --git a/backend/windmill-common/src/external_instance_pg.rs b/backend/windmill-common/src/external_instance_pg.rs index 2e66dcd8cf..e965953587 100644 --- a/backend/windmill-common/src/external_instance_pg.rs +++ b/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 } diff --git a/backend/windmill-common/src/external_instance_pg_oss.rs b/backend/windmill-common/src/external_instance_pg_oss.rs index a29c61ebe4..f7104ee6e8 100644 --- a/backend/windmill-common/src/external_instance_pg_oss.rs +++ b/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<()> {