fix(datatables): refuse moving the external cluster to another host or port while it holds databases

Every settings writer goes through the same check as removal: the login, TLS and maintenance
database may still change, the cluster may not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Diego Imbert
2026-09-18 16:32:59 +02:00
co-authored by Claude Opus 5
parent 1d76c06426
commit c96ddc0333
2 changed files with 75 additions and 1 deletions
+46
View File
@@ -1583,3 +1583,49 @@ async fn declarative_sync_rejects_an_unusable_default_allowed_origins(db: Pool<P
.await
.expect("a valid origin list must sync");
}
/// While Windmill has databases on the external cluster, a sync may change its login but not
/// point it at another cluster: the databases would be stranded there, and their registry and
/// role passwords applied to a cluster that has neither.
#[cfg(all(feature = "enterprise", feature = "private"))]
#[sqlx::test(fixtures("base"))]
async fn declarative_sync_keeps_the_external_cluster_while_it_holds_databases(db: Pool<Postgres>) {
clear_settings_and_configs(&db).await;
let cluster = |host: &str, password: &str| serde_json::json!({ "host": host, "port": 5432, "user": "wm_admin", "password": password });
sqlx::query(
"INSERT INTO global_settings (name, value) VALUES
('external_instance_pg', $1),
('external_instance_pg_state', '{\"databases\": {\"dt_a\": {\"success\": true}}}')",
)
.bind(cluster("pg-a.internal", "one"))
.execute(&db)
.await
.unwrap();
let current = BTreeMap::from([(
"external_instance_pg".to_string(),
cluster("pg-a.internal", "one"),
)]);
let sync = |value: serde_json::Value| {
let desired = BTreeMap::from([("external_instance_pg".to_string(), value)]);
let (db, current) = (db.clone(), current.clone());
async move {
windmill_common::instance_config::sync_global_settings_declarative(
&db, &current, &desired,
)
.await
}
};
let err = sync(cluster("pg-b.internal", "one"))
.await
.expect_err("another host must be refused while dt_a is registered");
assert!(err.to_string().contains("dt_a"), "got: {err}");
assert_eq!(
get_global_setting(&db, "external_instance_pg").await,
Some(cluster("pg-a.internal", "one"))
);
sync(cluster("PG-A.internal ", "two"))
.await
.expect("a new login on the same cluster must sync");
}
@@ -161,11 +161,39 @@ pub async fn check_external_instance_pg_write(
ensure_external_instance_pg_removable(db).await
}
Some(value) => {
crate::external_instance_pg_oss::validate_external_instance_pg_setting(value)
crate::external_instance_pg_oss::validate_external_instance_pg_setting(value)?;
ensure_same_cluster_while_in_use(db, value).await
}
}
}
/// Pointing the setting at another cluster strands the databases Windmill created on this one as
/// surely as unsetting it, and would hand their registry and role passwords to a cluster that has
/// neither. The login, TLS and maintenance database may change; the host and port may not.
async fn ensure_same_cluster_while_in_use(db: &DB, value: &serde_json::Value) -> Result<()> {
let state = read_external_instance_pg_state(db).await?;
if state.databases.is_empty() {
return Ok(());
}
let identity = |c: &ExternalInstancePg| (c.host.trim().to_lowercase(), c.port.unwrap_or(5432));
let new: ExternalInstancePg = serde_json::from_value(value.clone())
.map_err(|e| Error::BadRequest(format!("{EXTERNAL_INSTANCE_PG_SETTING}: {e}")))?;
let current = read_external_instance_pg_config(db).await?;
if current.is_some_and(|current| identity(&current) == identity(&new)) {
return Ok(());
}
Err(Error::BadRequest(format!(
"The external instance cluster still holds databases Windmill created ({}). Drop them \
before pointing {EXTERNAL_INSTANCE_PG_SETTING} at another host or port.",
state
.databases
.keys()
.cloned()
.collect::<Vec<_>>()
.join(", ")
)))
}
/// Converge the external cluster on the configured login: check what it can do, create or update
/// Windmill's two roles with the stored passwords, and report anything that would get in the way.
/// With `rotate_passwords`, generate new passwords first. Safe to run again; running it again is