diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 00e192fc36..e326eaa88f 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -63cf1cf4bf643209eeea8540fc1542734b2474da +a03c4cc3da17d828e492d6b3d77eee3d2fee21c5 \ No newline at end of file diff --git a/backend/windmill-api-settings/src/lib.rs b/backend/windmill-api-settings/src/lib.rs index fc627f115f..3bf04d1ef7 100644 --- a/backend/windmill-api-settings/src/lib.rs +++ b/backend/windmill-api-settings/src/lib.rs @@ -893,6 +893,14 @@ pub async fn set_global_setting_internal( ))); } + if key == EXTERNAL_INSTANCE_PG_SETTING { + return windmill_common::external_instance_pg::write_external_instance_pg_setting( + db, + Some(&value), + ) + .await; + } + run_setting_pre_write_hook(db, &key, &value).await?; match value { @@ -954,13 +962,6 @@ async fn run_setting_pre_write_hook( value: &serde_json::Value, ) -> error::Result<()> { match key { - EXTERNAL_INSTANCE_PG_SETTING => { - windmill_common::external_instance_pg::check_external_instance_pg_write( - db, - Some(value), - ) - .await?; - } // The instance AI config is written as an untyped blob through this generic // endpoint, so it never passes the typed check the workspace handler applies. // Rates that reach a cost total unbounded would make it negative or infinite. @@ -1281,7 +1282,7 @@ async fn set_instance_config( let desired_map = desired.global_settings.to_settings_map(); if !desired_map.is_empty() { let current_map = current.global_settings.to_settings_map(); - let settings_diff = + let mut settings_diff = instance_config::diff_global_settings(¤t_map, &desired_map, ApplyMode::Merge); let ai_config_changed = settings_diff .upserts @@ -1310,16 +1311,15 @@ async fn set_instance_config( } for (key, value) in &settings_diff.upserts { - run_setting_pre_write_hook(&db, key, value).await?; - } - if settings_diff - .deletes - .iter() - .any(|k| k == EXTERNAL_INSTANCE_PG_SETTING) - { - windmill_common::external_instance_pg::check_external_instance_pg_write(&db, None) - .await?; + if key != EXTERNAL_INSTANCE_PG_SETTING { + run_setting_pre_write_hook(&db, key, value).await?; + } } + windmill_common::external_instance_pg::write_external_instance_pg_from_diff( + &db, + &mut settings_diff, + ) + .await?; instance_config::apply_settings_diff(&db, &settings_diff) .await diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index a9c3775222..869ea81a6b 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -8343,6 +8343,13 @@ async fn apply_forked_datatable( })?, }; + if database.resource_type == DataTableCatalogResourceType::ExternalInstance { + windmill_common::external_instance_pg::ensure_external_instance_database_registered( + tx, + &fdt.new_dbname, + ) + .await?; + } if database.resource_type.is_windmill_managed() { // The whole `database` object, not just its `resource_path`: a pointer entry has none to // patch. `reference` goes with it — exactly one of the two may be set. The copy was created diff --git a/backend/windmill-common/src/external_instance_pg.rs b/backend/windmill-common/src/external_instance_pg.rs index 66317ef05a..72960d89ae 100644 --- a/backend/windmill-common/src/external_instance_pg.rs +++ b/backend/windmill-common/src/external_instance_pg.rs @@ -281,23 +281,73 @@ pub async fn ensure_external_instance_database_registered( ))) } -/// Check a write to [`EXTERNAL_INSTANCE_PG_SETTING`] before it happens: `None`, null or an empty -/// string unsets it. Every writer of global settings calls this, the per-key and bulk endpoints -/// as well as the declarative sync. -pub async fn check_external_instance_pg_write( +/// Write [`EXTERNAL_INSTANCE_PG_SETTING`]: `None`, null or an empty string unsets it. Every writer +/// of global settings goes through this for that key — the per-key and bulk endpoints as well as +/// the declarative sync — instead of writing the row itself. +/// +/// The checks and the write share one transaction holding [`lock_external_instance_pg_state`]. A +/// check taken outside it could pass while a database create still reads the old cluster, which +/// would then register a database there after the setting names another one. +/// +/// Authorization: checks nothing. Callers MUST be superadmin. +pub async fn write_external_instance_pg_setting( db: &DB, value: Option<&serde_json::Value>, ) -> Result<()> { + let value = match value { + None | Some(serde_json::Value::Null) => None, + Some(serde_json::Value::String(s)) if s.trim().is_empty() => None, + Some(value) => Some(value), + }; + let mut tx = db.begin().await?; + lock_external_instance_pg_state(&mut tx).await?; match value { - None | Some(serde_json::Value::Null) => ensure_external_instance_pg_removable(db).await, - Some(serde_json::Value::String(s)) if s.trim().is_empty() => { - ensure_external_instance_pg_removable(db).await + None => { + ensure_external_instance_pg_removable(db).await?; + sqlx::query("DELETE FROM global_settings WHERE name = $1") + .bind(EXTERNAL_INSTANCE_PG_SETTING) + .execute(&mut *tx) + .await?; } Some(value) => { crate::external_instance_pg_oss::validate_external_instance_pg_setting(value)?; - ensure_external_instance_pg_not_repointed(db, value).await + ensure_external_instance_pg_not_repointed(db, value).await?; + sqlx::query( + "INSERT INTO global_settings (name, value) VALUES ($1, $2) + ON CONFLICT (name) DO UPDATE SET value = EXCLUDED.value, updated_at = now()", + ) + .bind(EXTERNAL_INSTANCE_PG_SETTING) + .bind(value) + .execute(&mut *tx) + .await?; } } + tx.commit().await?; + tracing::info!( + "{} global setting {EXTERNAL_INSTANCE_PG_SETTING}", + if value.is_some() { "Set" } else { "Unset" } + ); + Ok(()) +} + +/// [`write_external_instance_pg_setting`] for a settings diff: writes the key if the diff touches +/// it, and takes it out of the diff so the generic apply does not write it again. +pub async fn write_external_instance_pg_from_diff( + db: &DB, + diff: &mut crate::instance_config::SettingsDiff, +) -> Result<()> { + if let Some(value) = diff.upserts.remove(EXTERNAL_INSTANCE_PG_SETTING) { + write_external_instance_pg_setting(db, Some(&value)).await?; + } + if let Some(i) = diff + .deletes + .iter() + .position(|k| k == EXTERNAL_INSTANCE_PG_SETTING) + { + diff.deletes.remove(i); + write_external_instance_pg_setting(db, None).await?; + } + Ok(()) } /// Refuse pointing the setting at another host or port while databases live on the current one. diff --git a/backend/windmill-common/src/instance_config.rs b/backend/windmill-common/src/instance_config.rs index d26b974717..f5683db687 100644 --- a/backend/windmill-common/src/instance_config.rs +++ b/backend/windmill-common/src/instance_config.rs @@ -1395,14 +1395,8 @@ pub async fn sync_global_settings_declarative( crate::global_settings::parse_allowed_origins_setting(desired.get(origins_key)) .map_err(|e| anyhow::anyhow!("{origins_key}: {e}"))?; - let diff = diff_global_settings(current, desired, ApplyMode::Replace); - let external_pg_key = crate::global_settings::EXTERNAL_INSTANCE_PG_SETTING; - if diff.deletes.iter().any(|k| k == external_pg_key) { - crate::external_instance_pg::check_external_instance_pg_write(db, None).await?; - } - if let Some(value) = diff.upserts.get(external_pg_key) { - crate::external_instance_pg::check_external_instance_pg_write(db, Some(value)).await?; - } + let mut diff = diff_global_settings(current, desired, ApplyMode::Replace); + crate::external_instance_pg::write_external_instance_pg_from_diff(db, &mut diff).await?; apply_settings_diff(db, &diff).await?; Ok(())