fix(datatables): refuse repointing the external cluster while it is in use, and keep verify-ca working for pg_dump

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Diego Imbert
2026-09-17 15:28:31 +02:00
co-authored by Claude Opus 5
parent 9b0046d867
commit 3981b2eb1b
2 changed files with 102 additions and 9 deletions
@@ -3364,18 +3364,80 @@ fn apply_pg_tls_env(
return Ok(Some(file));
}
// Only a connection that asked to be verified against the system trust store. Without a file,
// libpq's own default would look for `~/.postgresql/root.crt` and refuse a verify-* mode.
if pg_db.accept_invalid_certs == Some(false)
&& matches!(
pg_db.sslmode.as_deref(),
Some("verify-full") | Some("verify-ca")
)
{
cmd.env("PGSSLROOTCERT", "system");
// libpq's own default would look for `~/.postgresql/root.crt` and refuse a verify-* mode. libpq
// takes the special `system` value with verify-full only, so verify-ca needs the bundle itself.
if pg_db.accept_invalid_certs == Some(false) {
match pg_db.sslmode.as_deref() {
Some("verify-full") => {
cmd.env("PGSSLROOTCERT", "system");
}
Some("verify-ca") => {
if let Some(bundle) = system_ca_bundle() {
cmd.env("PGSSLROOTCERT", bundle);
}
}
_ => {}
}
}
Ok(None)
}
fn system_ca_bundle() -> Option<std::path::PathBuf> {
std::env::var_os("SSL_CERT_FILE")
.map(std::path::PathBuf::from)
.into_iter()
.chain(
[
"/etc/ssl/certs/ca-certificates.crt",
"/etc/pki/tls/certs/ca-bundle.crt",
"/etc/ssl/cert.pem",
"/etc/ssl/ca-bundle.pem",
]
.map(std::path::PathBuf::from),
)
.find(|path| path.is_file())
}
#[cfg(test)]
mod pg_tls_env_tests {
use super::apply_pg_tls_env;
use windmill_common::PgDatabase;
fn root_cert_env(sslmode: &str) -> Option<std::ffi::OsString> {
let pg_db = PgDatabase {
host: "db".to_string(),
user: None,
password: None,
port: None,
sslmode: Some(sslmode.to_string()),
dbname: "d".to_string(),
root_certificate_pem: None,
accept_invalid_certs: Some(false),
use_iam_auth: None,
region: None,
};
let mut cmd = tokio::process::Command::new("psql");
apply_pg_tls_env(&mut cmd, &pg_db).unwrap();
cmd.as_std()
.get_envs()
.find(|(k, _)| *k == "PGSSLROOTCERT")
.and_then(|(_, v)| v.map(|v| v.to_os_string()))
}
#[test]
fn system_roots_only_through_verify_full() {
assert_eq!(
root_cert_env("verify-full").as_deref(),
Some("system".as_ref())
);
// libpq refuses `sslrootcert=system` with verify-ca, which would fail every dump and restore.
assert_ne!(
root_cert_env("verify-ca").as_deref(),
Some("system".as_ref())
);
}
}
/// GUC names the server backing `pg_db` knows about.
///
/// Asked through psql rather than a tokio-postgres connection so the lookup reaches
@@ -278,11 +278,42 @@ 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_external_instance_pg_not_repointed(db, value).await
}
}
}
/// Refuse pointing the setting at another host or port while databases live on the current one.
/// Data tables name databases, not clusters, so they would silently resolve to whatever the new
/// cluster holds under the same names. Other fields (admin login, sslmode) may change freely.
async fn ensure_external_instance_pg_not_repointed(
db: &DB,
value: &serde_json::Value,
) -> Result<()> {
let Some(current) = read_external_instance_pg_config(db).await? else {
return Ok(());
};
let Ok(desired) = serde_json::from_value::<ExternalInstancePg>(value.clone()) else {
return Ok(());
};
let address = |c: &ExternalInstancePg| (c.host.trim().to_lowercase(), c.port.unwrap_or(5432));
if address(&current) == address(&desired) {
return Ok(());
}
let state = read_external_instance_pg_state(db).await?;
let usages = external_instance_database_usages(db).await?;
if state.databases.is_empty() && usages.is_empty() {
return Ok(());
}
Err(Error::BadRequest(format!(
"The external instance cluster at {}:{} still holds databases in use. Drop them and repoint \
what uses them before pointing {EXTERNAL_INSTANCE_PG_SETTING} at another cluster.",
current.host.trim(),
current.port.unwrap_or(5432)
)))
}
/// 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