mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix(datatables): only drop external databases Windmill marked, and check use under the lock
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
a41644c795
commit
955aadbf10
@@ -1886,18 +1886,10 @@ 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.
|
||||
if let Some(workspaces) =
|
||||
windmill_common::external_instance_pg::external_instance_database_usages(&db)
|
||||
.await?
|
||||
.remove(dbname.trim())
|
||||
{
|
||||
return Err(error::Error::BadRequest(format!(
|
||||
"Database '{dbname}' is still used by data tables in {}",
|
||||
workspaces.into_iter().collect::<Vec<_>>().join(", ")
|
||||
)));
|
||||
}
|
||||
windmill_common::external_instance_pg::drop_external_instance_database_unchecked(&db, &dbname)
|
||||
.await?;
|
||||
windmill_common::external_instance_pg::drop_external_instance_database_unchecked(
|
||||
&db, &dbname, true,
|
||||
)
|
||||
.await?;
|
||||
windmill_audit::audit_oss::audit_log(
|
||||
&db,
|
||||
&authed,
|
||||
|
||||
@@ -4037,6 +4037,11 @@ async fn edit_datatable_config(
|
||||
}
|
||||
if database.resource_type == DataTableCatalogResourceType::ExternalInstance {
|
||||
windmill_common::external_instance_pg::ensure_external_instance_available()?;
|
||||
windmill_common::external_instance_pg::ensure_external_instance_database_registered(
|
||||
&mut tx,
|
||||
&database.resource_path,
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
if !is_superadmin {
|
||||
return Err(Error::BadRequest(
|
||||
|
||||
@@ -1422,7 +1422,7 @@ pub async fn drop_forked_datatable_databases(
|
||||
== windmill_common::workspaces::DataTableCatalogResourceType::ExternalInstance
|
||||
{
|
||||
windmill_common::external_instance_pg::drop_external_instance_database_unchecked(
|
||||
&db, db_to_drop,
|
||||
&db, db_to_drop, false,
|
||||
)
|
||||
.await
|
||||
} else {
|
||||
|
||||
@@ -130,8 +130,8 @@ pub async fn external_instance_databases(db: &DB) -> Result<BTreeMap<String, Cus
|
||||
}
|
||||
|
||||
/// The workspaces whose data tables name each database on the external cluster.
|
||||
pub async fn external_instance_database_usages(
|
||||
db: &DB,
|
||||
pub async fn external_instance_database_usages<'c>(
|
||||
db: impl sqlx::PgExecutor<'c>,
|
||||
) -> Result<BTreeMap<String, BTreeSet<String>>> {
|
||||
let rows = sqlx::query_as::<_, (String, String)>(
|
||||
"SELECT ws.workspace_id, entry->'database'->>'resource_path'
|
||||
@@ -211,12 +211,55 @@ pub async fn create_external_instance_database_unchecked(
|
||||
.await
|
||||
}
|
||||
|
||||
/// Drop `dbname` from the external cluster. Only ever a database Windmill registered creating.
|
||||
/// Drop `dbname` from the external cluster: only a database Windmill registered creating, and still
|
||||
/// carries the mark it set there. With `refuse_if_used`, refuse while a data table names it.
|
||||
///
|
||||
/// 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, dbname: &str) -> Result<()> {
|
||||
crate::external_instance_pg_oss::drop_external_instance_database_unchecked(db, dbname).await
|
||||
pub async fn drop_external_instance_database_unchecked(
|
||||
db: &DB,
|
||||
dbname: &str,
|
||||
refuse_if_used: bool,
|
||||
) -> Result<()> {
|
||||
crate::external_instance_pg_oss::drop_external_instance_database_unchecked(
|
||||
db,
|
||||
dbname,
|
||||
refuse_if_used,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
/// Serializes everything that changes which databases exist on the external cluster, or which data
|
||||
/// tables name them: setup, creates, drops, and data table saves. Held until `tx` ends.
|
||||
pub async fn lock_external_instance_pg_state(
|
||||
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
|
||||
) -> Result<()> {
|
||||
sqlx::query("SELECT pg_advisory_xact_lock(hashtext($1))")
|
||||
.bind(EXTERNAL_INSTANCE_PG_STATE_SETTING)
|
||||
.execute(&mut **tx)
|
||||
.await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Refuse a data table naming `dbname` unless Windmill created it on the external cluster. Takes
|
||||
/// the lock drops take, so none can remove the database before `tx`, which saves the data table,
|
||||
/// commits.
|
||||
pub async fn ensure_external_instance_database_registered(
|
||||
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
|
||||
dbname: &str,
|
||||
) -> Result<()> {
|
||||
lock_external_instance_pg_state(tx).await?;
|
||||
if read_external_instance_pg_state(&mut **tx)
|
||||
.await?
|
||||
.databases
|
||||
.contains_key(dbname)
|
||||
{
|
||||
return Ok(());
|
||||
}
|
||||
Err(Error::BadRequest(format!(
|
||||
"Windmill did not create a database named '{dbname}' on the external instance cluster. \
|
||||
Create it from the instance settings first."
|
||||
)))
|
||||
}
|
||||
|
||||
/// Check a write to [`EXTERNAL_INSTANCE_PG_SETTING`] before it happens: `None`, null or an empty
|
||||
|
||||
@@ -73,6 +73,7 @@ mod ce {
|
||||
pub(crate) async fn drop_external_instance_database_unchecked(
|
||||
_db: &DB,
|
||||
_dbname: &str,
|
||||
_refuse_if_used: bool,
|
||||
) -> Result<()> {
|
||||
Err(unavailable())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user