From 31009a827d20f2dd19d7d2ddc58bab276932c52b Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Fri, 26 Jun 2026 11:40:13 +0200 Subject: [PATCH 01/19] feat: add as_str/from_str to PostgresDumpFormat --- src/domain/postgres/format.rs | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/src/domain/postgres/format.rs b/src/domain/postgres/format.rs index 5415fd7..cff40c4 100644 --- a/src/domain/postgres/format.rs +++ b/src/domain/postgres/format.rs @@ -1,5 +1,38 @@ -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum PostgresDumpFormat { Fc, Fd, } + +impl PostgresDumpFormat { + pub fn as_str(&self) -> &'static str { + match self { + PostgresDumpFormat::Fc => "fc", + PostgresDumpFormat::Fd => "fd", + } + } + + pub fn from_str(s: &str) -> Option { + match s { + "fc" => Some(PostgresDumpFormat::Fc), + "fd" => Some(PostgresDumpFormat::Fd), + _ => None, + } + } +} + +#[cfg(test)] +mod tests { + use super::PostgresDumpFormat; + + #[test] + fn round_trips_through_as_str_and_from_str() { + assert_eq!(PostgresDumpFormat::from_str(PostgresDumpFormat::Fc.as_str()), Some(PostgresDumpFormat::Fc)); + assert_eq!(PostgresDumpFormat::from_str(PostgresDumpFormat::Fd.as_str()), Some(PostgresDumpFormat::Fd)); + } + + #[test] + fn from_str_rejects_unknown_value() { + assert_eq!(PostgresDumpFormat::from_str("plain"), None); + } +} From 7fab28a63d65697beb099488f24f1e48cb43239d Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Fri, 26 Jun 2026 11:42:17 +0200 Subject: [PATCH 02/19] feat: resolve pg_dumpall/psql binary names --- src/domain/postgres/connection.rs | 16 ++++++++++++++++ src/tests/domain/postgres.rs | 23 ++++++++++++++++++++++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/domain/postgres/connection.rs b/src/domain/postgres/connection.rs index a8bb098..6546e1d 100644 --- a/src/domain/postgres/connection.rs +++ b/src/domain/postgres/connection.rs @@ -109,6 +109,22 @@ pub(crate) fn pg_dump_binary_name() -> &'static str { } } +pub(crate) fn pg_dumpall_binary_name() -> &'static str { + if cfg!(target_os = "windows") { + "pg_dumpall.exe" + } else { + "pg_dumpall" + } +} + +pub(crate) fn psql_binary_name() -> &'static str { + if cfg!(target_os = "windows") { + "psql.exe" + } else { + "psql" + } +} + pub(crate) fn pg_dump_exists_in(dir: &std::path::Path) -> bool { dir.join(pg_dump_binary_name()).is_file() } diff --git a/src/tests/domain/postgres.rs b/src/tests/domain/postgres.rs index c03bdcd..4721d95 100644 --- a/src/tests/domain/postgres.rs +++ b/src/tests/domain/postgres.rs @@ -152,7 +152,8 @@ async fn postgres_password_with_slash_test() { mod select_pg_path_tests { use crate::domain::postgres::connection::{ - pg_dump_binary_name, pg_dump_exists_in, select_pg_path_with, + pg_dump_binary_name, pg_dump_exists_in, pg_dumpall_binary_name, psql_binary_name, + select_pg_path_with, }; // `select_pg_path_with` takes the `PG_BIN_DIR` override as a plain @@ -209,4 +210,24 @@ mod select_pg_path_tests { let dir = std::path::Path::new("this/path/almost-certainly/does-not-exist-12345"); assert!(!pg_dump_exists_in(dir)); } + + #[test] + fn pg_dumpall_binary_name_is_platform_specific() { + let name = pg_dumpall_binary_name(); + if cfg!(target_os = "windows") { + assert_eq!(name, "pg_dumpall.exe"); + } else { + assert_eq!(name, "pg_dumpall"); + } + } + + #[test] + fn psql_binary_name_is_platform_specific() { + let name = psql_binary_name(); + if cfg!(target_os = "windows") { + assert_eq!(name, "psql.exe"); + } else { + assert_eq!(name, "psql"); + } + } } From 3ff03d7b2f6666a9c291db9b8e31f2018eba725a Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Fri, 26 Jun 2026 11:45:19 +0200 Subject: [PATCH 03/19] feat: add include_globals field to database config --- src/services/config.rs | 9 ++ src/tests/domain/firebird.rs | 1 + src/tests/domain/mariadb.rs | 1 + src/tests/domain/mongodb.rs | 1 + src/tests/domain/mssql.rs | 1 + src/tests/domain/mysql.rs | 1 + src/tests/domain/postgres.rs | 2 + src/tests/domain/redis.rs | 1 + src/tests/domain/valkey.rs | 1 + src/tests/services/config_tests.rs | 129 +++++++++++++++++++++++++++++ src/tests/services/mod.rs | 1 + 11 files changed, 148 insertions(+) create mode 100644 src/tests/services/config_tests.rs diff --git a/src/services/config.rs b/src/services/config.rs index fe9f3c6..7f4a698 100644 --- a/src/services/config.rs +++ b/src/services/config.rs @@ -55,6 +55,7 @@ pub struct DatabaseConfig { pub generated_id: String, pub path: String, pub max_packet_size: String, + pub include_globals: bool, } #[allow(dead_code)] @@ -77,6 +78,8 @@ pub struct InputDatabaseConfig { pub generated_id: String, pub path: Option, pub max_packet_size: Option, + #[serde(default, alias = "includeGlobals")] + pub include_globals: Option, } #[allow(dead_code)] @@ -223,6 +226,11 @@ impl ConfigService { _ => String::new(), }; + let include_globals = match db.db_type { + DbType::Postgresql => optional(&db.include_globals), + _ => false, + }; + databases.push(DatabaseConfig { name: db.name, database: database_name, @@ -234,6 +242,7 @@ impl ConfigService { generated_id: db.generated_id, path: path_val, max_packet_size, + include_globals, }); } diff --git a/src/tests/domain/firebird.rs b/src/tests/domain/firebird.rs index 2ae4320..b4fa769 100644 --- a/src/tests/domain/firebird.rs +++ b/src/tests/domain/firebird.rs @@ -40,6 +40,7 @@ async fn create_config() -> (ContainerAsync, DatabaseConfig) { generated_id: "3c445eb4-c2c6-4bde-a423-ee1385dcf6d2".to_string(), path: "".to_string(), max_packet_size: "".to_string(), + include_globals: false, }; (container, config) diff --git a/src/tests/domain/mariadb.rs b/src/tests/domain/mariadb.rs index 1989bbd..2105a73 100644 --- a/src/tests/domain/mariadb.rs +++ b/src/tests/domain/mariadb.rs @@ -32,6 +32,7 @@ async fn create_config() -> (ContainerAsync, DatabaseConfig) { generated_id: "3c4b4eb4-c2c6-4bde-a423-ee1385dcf6d2".to_string(), path: "".to_string(), max_packet_size: "512M".to_string(), + include_globals: false, }; (container, config) diff --git a/src/tests/domain/mongodb.rs b/src/tests/domain/mongodb.rs index d2d0688..fb2cc32 100644 --- a/src/tests/domain/mongodb.rs +++ b/src/tests/domain/mongodb.rs @@ -30,6 +30,7 @@ async fn create_config() -> (ContainerAsync, DatabaseConfig) { generated_id: "96d30a9f-ff4b-47c9-aaab-f3147bb34f16".to_string(), path: "".to_string(), max_packet_size: "".to_string(), + include_globals: false, }; (container, config) diff --git a/src/tests/domain/mssql.rs b/src/tests/domain/mssql.rs index d6a4001..2c8303e 100644 --- a/src/tests/domain/mssql.rs +++ b/src/tests/domain/mssql.rs @@ -55,6 +55,7 @@ fn make_config(host: String, port: u16, database: &str, generated_id: &str) -> D generated_id: generated_id.to_string(), path: "".to_string(), max_packet_size: "".to_string(), + include_globals: false, } } diff --git a/src/tests/domain/mysql.rs b/src/tests/domain/mysql.rs index 7a48bea..4b50661 100644 --- a/src/tests/domain/mysql.rs +++ b/src/tests/domain/mysql.rs @@ -32,6 +32,7 @@ async fn create_config() -> (ContainerAsync, DatabaseConfig) { generated_id: "0f1bb8f2-35a0-4c91-8098-e36873d3ce31".to_string(), path: "".to_string(), max_packet_size: "512M".to_string(), + include_globals: false, }; (container, config) diff --git a/src/tests/domain/postgres.rs b/src/tests/domain/postgres.rs index 4721d95..fa541ee 100644 --- a/src/tests/domain/postgres.rs +++ b/src/tests/domain/postgres.rs @@ -39,6 +39,7 @@ async fn create_config() -> (ContainerAsync, DatabaseConfig) { generated_id: "40875631-e3d2-4dfe-a26b-2a347ecc64fd".to_string(), path: "".to_string(), max_packet_size: "".to_string(), + include_globals: false, }; (container, config) @@ -142,6 +143,7 @@ async fn postgres_password_with_slash_test() { generated_id: "5a1f0e3c-9b8a-4a8e-9b1b-0a1c2d3e4f5a".to_string(), path: "".to_string(), max_packet_size: "".to_string(), + include_globals: false, }; let db = DatabaseFactory::create_for_backup(config.clone()).await; diff --git a/src/tests/domain/redis.rs b/src/tests/domain/redis.rs index 9cd63ee..6953fe6 100644 --- a/src/tests/domain/redis.rs +++ b/src/tests/domain/redis.rs @@ -29,6 +29,7 @@ async fn create_config() -> (ContainerAsync, DatabaseConfig) { generated_id: "40875631-e3d2-4dfe-a26b-2a347ecc64fd".to_string(), path: "".to_string(), max_packet_size: "".to_string(), + include_globals: false, }; (container, config) diff --git a/src/tests/domain/valkey.rs b/src/tests/domain/valkey.rs index 4a78369..e6145b3 100644 --- a/src/tests/domain/valkey.rs +++ b/src/tests/domain/valkey.rs @@ -28,6 +28,7 @@ async fn create_config() -> (ContainerAsync, DatabaseConfig) { generated_id: "40875485-e3d2-4dfe-a26b-2a347ecc64fd".to_string(), path: "".to_string(), max_packet_size: "".to_string(), + include_globals: false, }; (container, config) diff --git a/src/tests/services/config_tests.rs b/src/tests/services/config_tests.rs new file mode 100644 index 0000000..796ddab --- /dev/null +++ b/src/tests/services/config_tests.rs @@ -0,0 +1,129 @@ +use crate::core::context::Context; +use crate::services::api::ApiClient; +use crate::services::config::ConfigService; +use crate::utils::edge_key::EdgeKey; +use std::io::Write; +use std::sync::Arc; +use tempfile::NamedTempFile; + +// `ConfigService::load` never touches `self.ctx` on the `Some(file_path)` +// path these tests use, so the values here don't matter — but +// `Context::new()` panics without an `EDGE_KEY` env var, so build the +// struct directly, the same way `backup_uploader_tests.rs`'s +// `ctx_pointing_at` helper does. +fn test_context() -> Arc { + Arc::new(Context { + edge_key: EdgeKey { + server_url: String::new(), + agent_id: "agent-1".to_string(), + master_key_b64: String::new(), + }, + api: ApiClient::new(String::new()), + }) +} + +fn write_json(contents: &str) -> NamedTempFile { + let mut file = NamedTempFile::with_suffix(".json").unwrap(); + file.write_all(contents.as_bytes()).unwrap(); + file +} + +#[test] +fn include_globals_defaults_to_false_when_absent() { + let file = write_json( + r#"{ + "databases": [ + { + "name": "db1", + "database": "app", + "type": "postgresql", + "username": "u", + "password": "p", + "port": 5432, + "host": "localhost", + "generated_id": "16678159-ff7e-4c97-8c83-0adeff214681" + } + ] + }"#, + ); + + let service = ConfigService::new(test_context()); + let cfg = service.load(Some(file.path().to_str().unwrap())).unwrap(); + + assert_eq!(cfg.databases[0].include_globals, false); +} + +#[test] +fn include_globals_true_is_parsed() { + let file = write_json( + r#"{ + "databases": [ + { + "name": "db1", + "database": "app", + "type": "postgresql", + "username": "u", + "password": "p", + "port": 5432, + "host": "localhost", + "generated_id": "16678159-ff7e-4c97-8c83-0adeff214681", + "include_globals": true + } + ] + }"#, + ); + + let service = ConfigService::new(test_context()); + let cfg = service.load(Some(file.path().to_str().unwrap())).unwrap(); + + assert_eq!(cfg.databases[0].include_globals, true); +} + +#[test] +fn include_globals_camel_case_alias_is_accepted() { + let file = write_json( + r#"{ + "databases": [ + { + "name": "db1", + "database": "app", + "type": "postgresql", + "username": "u", + "password": "p", + "port": 5432, + "host": "localhost", + "generated_id": "16678159-ff7e-4c97-8c83-0adeff214681", + "includeGlobals": true + } + ] + }"#, + ); + + let service = ConfigService::new(test_context()); + let cfg = service.load(Some(file.path().to_str().unwrap())).unwrap(); + + assert_eq!(cfg.databases[0].include_globals, true); +} + +#[test] +fn include_globals_ignored_for_non_postgres_types() { + let file = write_json( + r#"{ + "databases": [ + { + "name": "db1", + "type": "redis", + "port": 6379, + "host": "localhost", + "generated_id": "16678159-ff7e-4c97-8c83-0adeff214681", + "include_globals": true + } + ] + }"#, + ); + + let service = ConfigService::new(test_context()); + let cfg = service.load(Some(file.path().to_str().unwrap())).unwrap(); + + assert_eq!(cfg.databases[0].include_globals, false); +} diff --git a/src/tests/services/mod.rs b/src/tests/services/mod.rs index 1780d61..af4f3eb 100644 --- a/src/tests/services/mod.rs +++ b/src/tests/services/mod.rs @@ -1,2 +1,3 @@ mod api_models_tests; mod backup_uploader_tests; +mod config_tests; From 72619012e55638d37531ebd4fea957f1e06cbabc Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Fri, 26 Jun 2026 11:48:22 +0200 Subject: [PATCH 04/19] feat: add pg_dumpall/psql globals dump and apply --- src/domain/postgres/globals.rs | 116 +++++++++++++++++++++++++++++++++ src/domain/postgres/mod.rs | 2 + src/tests/domain/postgres.rs | 35 ++++++++++ 3 files changed, 153 insertions(+) create mode 100644 src/domain/postgres/globals.rs diff --git a/src/domain/postgres/globals.rs b/src/domain/postgres/globals.rs new file mode 100644 index 0000000..184e2df --- /dev/null +++ b/src/domain/postgres/globals.rs @@ -0,0 +1,116 @@ +use anyhow::Result; +use std::collections::HashMap; +use std::path::{Path, PathBuf}; +use std::process::Command; +use std::sync::Arc; +use std::time::Instant; + +use super::connection::{pg_dumpall_binary_name, psql_binary_name, select_pg_path}; +use crate::services::backup::logger::JobLogger; +use crate::services::config::DatabaseConfig; + +pub fn dump( + cfg: &DatabaseConfig, + pg_version: &str, + out_dir: &Path, + env: &HashMap, + logger: &Arc, +) -> Result { + let pg_dumpall = select_pg_path(pg_version).join(pg_dumpall_binary_name()); + let globals_path = out_dir.join("globals.sql"); + + logger.log("info", format!("Dumping cluster globals via {:?}", pg_dumpall)); + + let start = Instant::now(); + let output = Command::new(&pg_dumpall) + .arg("--host").arg(&cfg.host) + .arg("--port").arg(cfg.port.to_string()) + .arg("--username").arg(&cfg.username) + .arg("--globals-only") + .arg("-f").arg(&globals_path) + .envs(env.clone()) + .output(); + let duration_ms = start.elapsed().as_millis() as f64; + + match output { + Ok(o) => { + let stderr = String::from_utf8_lossy(&o.stderr).to_string(); + let exit_code = o.status.code().unwrap_or(-1); + + if o.status.success() { + logger.log_command( + "pg_dumpall --globals-only", + if stderr.is_empty() { None } else { Some(stderr) }, + Some(0), + Some(duration_ms), + ); + logger.log("info", "Globals dump completed".to_string()); + Ok(globals_path) + } else { + logger.log_command("pg_dumpall --globals-only", Some(stderr), Some(exit_code), Some(duration_ms)); + anyhow::bail!("pg_dumpall --globals-only failed for cluster {}:{}", cfg.host, cfg.port); + } + } + Err(e) => { + logger.log_command("pg_dumpall --globals-only", Some(e.to_string()), Some(-1), Some(duration_ms)); + Err(e.into()) + } + } +} + +/// Replays a previously captured `globals.sql` against the cluster's +/// `postgres` maintenance database. Globals are best-effort enrichment +/// (roles/tablespaces commonly already exist on a shared target cluster) — +/// this function logs failures but never returns an error, so it can never +/// block the real database restore that follows it. +pub fn apply( + cfg: &DatabaseConfig, + pg_version: &str, + globals_sql: &Path, + env: &HashMap, + logger: &Arc, +) { + let psql = select_pg_path(pg_version).join(psql_binary_name()); + + logger.log("info", format!("Applying cluster globals from {:?}", globals_sql)); + + let start = Instant::now(); + let output = Command::new(&psql) + .arg("--host").arg(&cfg.host) + .arg("--port").arg(cfg.port.to_string()) + .arg("--username").arg(&cfg.username) + .arg("--dbname").arg("postgres") + .arg("-v").arg("ON_ERROR_STOP=0") + .arg("-f").arg(globals_sql) + .envs(env.clone()) + .output(); + let duration_ms = start.elapsed().as_millis() as f64; + + match output { + Ok(o) => { + let stderr = String::from_utf8_lossy(&o.stderr).to_string(); + let stdout = String::from_utf8_lossy(&o.stdout).to_string(); + let combined = format!("{}{}", stdout, stderr); + let exit_code = o.status.code(); + + logger.log_command( + "psql -f globals.sql", + if combined.is_empty() { None } else { Some(combined) }, + exit_code, + Some(duration_ms), + ); + + if exit_code == Some(0) { + logger.log("info", "Globals applied successfully".to_string()); + } else { + logger.log( + "warn", + "Globals apply reported errors (often pre-existing roles/tablespaces); continuing with database restore".to_string(), + ); + } + } + Err(e) => { + logger.log("warn", format!("Could not run psql for globals apply, continuing without globals: {:?}", e)); + } + } +} diff --git a/src/domain/postgres/mod.rs b/src/domain/postgres/mod.rs index c7cd4cb..c6aa9ab 100644 --- a/src/domain/postgres/mod.rs +++ b/src/domain/postgres/mod.rs @@ -2,7 +2,9 @@ pub mod backup; pub(crate) mod connection; pub mod database; mod format; +pub(crate) mod globals; mod ping; mod restore; pub use connection::{detect_format_from_file, detect_format_from_size}; +pub use format::PostgresDumpFormat; diff --git a/src/tests/domain/postgres.rs b/src/tests/domain/postgres.rs index fa541ee..e0a9920 100644 --- a/src/tests/domain/postgres.rs +++ b/src/tests/domain/postgres.rs @@ -110,6 +110,41 @@ async fn postgres_backup_restore_test() { } } +#[tokio::test] +async fn globals_dump_then_apply_round_trips() { + init_tracing_for_test(); + + let (_container, config) = create_config().await; + let temp_dir = TempDir::new().unwrap(); + + let version = crate::domain::postgres::connection::server_version(&config) + .await + .unwrap(); + + let mut env = std::env::vars().collect::>(); + env.insert("PGPASSWORD".to_string(), config.password.clone()); + + let logger = std::sync::Arc::new(crate::services::backup::logger::JobLogger::new()); + + let globals_path = crate::domain::postgres::globals::dump( + &config, + &version, + temp_dir.path(), + &env, + &logger, + ) + .unwrap(); + + assert!(globals_path.is_file()); + let contents = std::fs::read_to_string(&globals_path).unwrap(); + assert!(contents.contains("ROLE"), "expected role statements in globals.sql, got: {contents}"); + + // Re-applying an already-applied globals.sql must not error out the + // caller: roles/tablespaces already existing on the cluster are expected + // and must be swallowed as warnings, not failures. + crate::domain::postgres::globals::apply(&config, &version, &globals_path, &env, &logger); +} + #[tokio::test] async fn postgres_password_with_slash_test() { init_tracing_for_test(); From 7262a9a7df8b467d3fefeca0fb98aa891d906043 Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Fri, 26 Jun 2026 11:54:02 +0200 Subject: [PATCH 05/19] feat: add postgres backup bundle (manifest + build + resolve) --- src/domain/postgres/bundle.rs | 142 ++++++++++++++++++++++++++++ src/domain/postgres/mod.rs | 1 + src/tests/domain/mod.rs | 1 + src/tests/domain/postgres_bundle.rs | 63 ++++++++++++ 4 files changed, 207 insertions(+) create mode 100644 src/domain/postgres/bundle.rs create mode 100644 src/tests/domain/postgres_bundle.rs diff --git a/src/domain/postgres/bundle.rs b/src/domain/postgres/bundle.rs new file mode 100644 index 0000000..b688211 --- /dev/null +++ b/src/domain/postgres/bundle.rs @@ -0,0 +1,142 @@ +use anyhow::Result; +use serde::{Deserialize, Serialize}; +use std::collections::HashMap; +use std::path::{Path, PathBuf}; +use std::sync::Arc; + +use super::format::PostgresDumpFormat; +use super::globals; +use crate::services::backup::logger::JobLogger; +use crate::services::config::DatabaseConfig; + +const MANIFEST_FILENAME: &str = "manifest.json"; + +#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)] +pub struct BundleManifest { + pub format: String, + pub has_globals: bool, + pub dump_path: String, +} + +impl BundleManifest { + pub fn write(&self, dir: &Path) -> Result<()> { + let path = dir.join(MANIFEST_FILENAME); + std::fs::write(path, serde_json::to_string_pretty(self)?)?; + Ok(()) + } + + pub fn read(dir: &Path) -> Result> { + let path = dir.join(MANIFEST_FILENAME); + if !path.is_file() { + return Ok(None); + } + let contents = std::fs::read_to_string(path)?; + Ok(Some(serde_json::from_str(&contents)?)) + } +} + +/// Packs an already-produced dump artifact (a `.dump` file for `Fc`, or the +/// raw `pg_dump -Fd` output directory for `Fd`) together with a fresh +/// `globals.sql` and a `manifest.json` into one tar.gz. The returned path +/// already ends in `.tar.gz` so `compress_backup`'s "already compressed" +/// check short-circuits and never re-wraps it. +pub fn build( + cfg: &DatabaseConfig, + format: PostgresDumpFormat, + dump_artifact: &Path, + backup_dir: &Path, + pg_version: &str, + env: &HashMap, + logger: Arc, +) -> Result { + let bundle_dir = backup_dir.join(format!("{}_bundle", cfg.generated_id)); + std::fs::create_dir_all(&bundle_dir)?; + + let dump_path_in_bundle = if dump_artifact.is_dir() { + let dest = bundle_dir.join("dump_dir"); + std::fs::rename(dump_artifact, &dest)?; + "dump_dir".to_string() + } else { + let dest = bundle_dir.join("dump.dump"); + std::fs::rename(dump_artifact, &dest)?; + "dump.dump".to_string() + }; + + globals::dump(cfg, pg_version, &bundle_dir, env, &logger)?; + + BundleManifest { + format: format.as_str().to_string(), + has_globals: true, + dump_path: dump_path_in_bundle, + } + .write(&bundle_dir)?; + + let tar_path = backup_dir.join(format!("{}.tar.gz", cfg.generated_id)); + let tar_gz = std::fs::File::create(&tar_path)?; + let enc = flate2::write::GzEncoder::new(tar_gz, flate2::Compression::default()); + let mut tar = tar::Builder::new(enc); + tar.append_dir_all(".", &bundle_dir)?; + tar.finish()?; + + logger.log("info", format!("Globals bundle created at {:?}", tar_path)); + + Ok(tar_path) +} + +pub struct ResolvedRestore { + pub dump_path: PathBuf, + pub globals_path: Option, + pub format_override: Option, + _tmp_dir: Option, +} + +/// Peeks `restore_file` for a bundle produced by `build()`. Any archive +/// without a `manifest.json` at its root — every backup taken before this +/// feature existed, and every backup taken with `include_globals: false` — +/// passes through with `dump_path` unchanged and `format_override: None`, +/// leaving `restore.rs`'s existing Fc/Fd handling exactly as it was. +pub fn resolve(restore_file: &Path) -> Result { + let passthrough = || ResolvedRestore { + dump_path: restore_file.to_path_buf(), + globals_path: None, + format_override: None, + _tmp_dir: None, + }; + + let file = match std::fs::File::open(restore_file) { + Ok(f) => f, + Err(_) => return Ok(passthrough()), + }; + + let dec = flate2::read::GzDecoder::new(file); + let mut archive = tar::Archive::new(dec); + let tmp_dir = tempfile::TempDir::new()?; + + if archive.unpack(tmp_dir.path()).is_err() { + return Ok(passthrough()); + } + + let manifest = match BundleManifest::read(tmp_dir.path())? { + Some(m) => m, + None => return Ok(passthrough()), + }; + + let format_override = PostgresDumpFormat::from_str(&manifest.format).ok_or_else(|| { + anyhow::anyhow!("Unknown format '{}' in bundle manifest", manifest.format) + })?; + + let dump_path = tmp_dir.path().join(&manifest.dump_path); + let globals_path = if manifest.has_globals { + let p = tmp_dir.path().join("globals.sql"); + if p.is_file() { Some(p) } else { None } + } else { + None + }; + + Ok(ResolvedRestore { + dump_path, + globals_path, + format_override: Some(format_override), + _tmp_dir: Some(tmp_dir), + }) +} diff --git a/src/domain/postgres/mod.rs b/src/domain/postgres/mod.rs index c6aa9ab..5545750 100644 --- a/src/domain/postgres/mod.rs +++ b/src/domain/postgres/mod.rs @@ -1,4 +1,5 @@ pub mod backup; +pub(crate) mod bundle; pub(crate) mod connection; pub mod database; mod format; diff --git a/src/tests/domain/mod.rs b/src/tests/domain/mod.rs index 641e097..f7d3586 100644 --- a/src/tests/domain/mod.rs +++ b/src/tests/domain/mod.rs @@ -2,6 +2,7 @@ mod mariadb; mod mongodb; mod mysql; mod postgres; +mod postgres_bundle; mod redis; mod valkey; mod firebird; diff --git a/src/tests/domain/postgres_bundle.rs b/src/tests/domain/postgres_bundle.rs new file mode 100644 index 0000000..c7e0e18 --- /dev/null +++ b/src/tests/domain/postgres_bundle.rs @@ -0,0 +1,63 @@ +use crate::domain::postgres::bundle::{self, BundleManifest}; +use crate::domain::postgres::PostgresDumpFormat; +use std::fs::File; +use std::io::Write; +use tempfile::TempDir; + +#[test] +fn manifest_round_trips_through_write_and_read() { + let dir = TempDir::new().unwrap(); + + let manifest = BundleManifest { + format: PostgresDumpFormat::Fc.as_str().to_string(), + has_globals: true, + dump_path: "dump.dump".to_string(), + }; + manifest.write(dir.path()).unwrap(); + + let read_back = BundleManifest::read(dir.path()).unwrap(); + assert_eq!(read_back, Some(manifest)); +} + +#[test] +fn manifest_read_returns_none_when_absent() { + let dir = TempDir::new().unwrap(); + assert_eq!(BundleManifest::read(dir.path()).unwrap(), None); +} + +#[test] +fn resolve_passes_through_a_plain_dump_file_unchanged() { + let dir = TempDir::new().unwrap(); + let dump_file = dir.path().join("16678159.dump"); + File::create(&dump_file).unwrap().write_all(b"not a tarball").unwrap(); + + let resolved = bundle::resolve(&dump_file).unwrap(); + + assert_eq!(resolved.dump_path, dump_file); + assert!(resolved.globals_path.is_none()); + assert!(resolved.format_override.is_none()); +} + +#[test] +fn resolve_passes_through_a_legacy_multi_file_tar_gz_unchanged() { + let dir = TempDir::new().unwrap(); + let inner_dir = dir.path().join("inner"); + std::fs::create_dir_all(&inner_dir).unwrap(); + File::create(inner_dir.join("toc.dat")).unwrap().write_all(b"toc").unwrap(); + File::create(inner_dir.join("1234.dat.gz")).unwrap().write_all(b"data").unwrap(); + + let tar_path = dir.path().join("legacy.tar.gz"); + let tar_gz = File::create(&tar_path).unwrap(); + let enc = flate2::write::GzEncoder::new(tar_gz, flate2::Compression::default()); + let mut tar = tar::Builder::new(enc); + tar.append_dir_all(".", &inner_dir).unwrap(); + tar.finish().unwrap(); + + let resolved = bundle::resolve(&tar_path).unwrap(); + + // No manifest.json inside this archive: bundle::resolve must defer to + // restore.rs's own Fd unpack logic rather than touching it. + assert_eq!(resolved.dump_path, tar_path); + assert!(resolved.globals_path.is_none()); + assert!(resolved.format_override.is_none()); +} From a336cbbbffb0dadf93ebd3ba9fc1a610d3091e87 Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Fri, 26 Jun 2026 11:59:55 +0200 Subject: [PATCH 06/19] feat: bundle globals into postgres backup when include_globals is set --- src/domain/postgres/backup.rs | 19 +++++++++++-- src/tests/domain/postgres.rs | 53 +++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/src/domain/postgres/backup.rs b/src/domain/postgres/backup.rs index 906cfd9..e5198e2 100644 --- a/src/domain/postgres/backup.rs +++ b/src/domain/postgres/backup.rs @@ -7,6 +7,7 @@ use std::time::Instant; use super::connection::{select_pg_path, server_version}; use super::format::PostgresDumpFormat; +use super::bundle; use crate::services::backup::logger::JobLogger; use crate::services::config::DatabaseConfig; @@ -50,7 +51,7 @@ pub async fn run( .arg("-f").arg(&file_path) .arg("-v") .arg("--compress=3") - .envs(env) + .envs(env.clone()) .output(); let duration_ms = start.elapsed().as_millis() as f64; @@ -74,6 +75,13 @@ pub async fn run( return Err(e.into()); } } + if cfg.include_globals { + logger.log("info", format!("Building globals bundle for {}", cfg.name)); + let bundled = bundle::build(&cfg, format, &file_path, &backup_dir, &version, &env, Arc::clone(&logger))?; + logger.log("info", format!("Backup finished for database {}", cfg.name)); + return Ok(bundled); + } + logger.log("info", format!("Backup finished for database {}", cfg.name)); Ok(file_path) } @@ -101,7 +109,7 @@ pub async fn run( .arg("-j").arg("4") .arg("-f").arg(&dump_dir) .arg("-v") - .envs(env) + .envs(env.clone()) .output(); let duration_ms = start.elapsed().as_millis() as f64; @@ -125,6 +133,13 @@ pub async fn run( } } + if cfg.include_globals { + logger.log("info", format!("Building globals bundle for {}", cfg.name)); + let bundled = bundle::build(&cfg, format, &dump_dir, &backup_dir, &version, &env, Arc::clone(&logger))?; + logger.log("info", format!("Backup finished for database {}", cfg.name)); + return Ok(bundled); + } + match std::fs::File::create(&tar_file) { Ok(tar_gz) => { let enc = flate2::write::GzEncoder::new(tar_gz, flate2::Compression::default()); diff --git a/src/tests/domain/postgres.rs b/src/tests/domain/postgres.rs index e0a9920..eb0431b 100644 --- a/src/tests/domain/postgres.rs +++ b/src/tests/domain/postgres.rs @@ -110,6 +110,59 @@ async fn postgres_backup_restore_test() { } } +#[tokio::test] +async fn backup_with_include_globals_produces_a_manifest_bundle() { + init_tracing_for_test(); + + let (_container, mut config) = create_config().await; + config.include_globals = true; + + let temp_dir = TempDir::new().unwrap(); + let backup_path = temp_dir.path(); + + let db = DatabaseFactory::create_for_backup(config.clone()).await; + let file_path = db + .backup(backup_path, std::sync::Arc::new(crate::services::backup::logger::JobLogger::new())) + .await + .unwrap(); + + assert!(file_path.to_string_lossy().ends_with(".tar.gz")); + + let extract_dir = TempDir::new().unwrap(); + let files = decompress_large_tar_gz(&file_path, extract_dir.path()).await.unwrap(); + assert!(files.len() >= 3, "expected dump + globals.sql + manifest.json, got {:?}", files); + + let manifest = crate::domain::postgres::bundle::BundleManifest::read(extract_dir.path()) + .unwrap() + .expect("manifest.json must be present when include_globals is true"); + assert_eq!(manifest.has_globals, true); + assert_eq!(manifest.format, "fc"); + + assert!(extract_dir.path().join("globals.sql").is_file()); + assert!(extract_dir.path().join(&manifest.dump_path).exists()); +} + +#[tokio::test] +async fn backup_without_include_globals_is_unaffected() { + init_tracing_for_test(); + + let (_container, config) = create_config().await; + assert_eq!(config.include_globals, false); + + let temp_dir = TempDir::new().unwrap(); + let backup_path = temp_dir.path(); + + let db = DatabaseFactory::create_for_backup(config.clone()).await; + let file_path = db + .backup(backup_path, std::sync::Arc::new(crate::services::backup::logger::JobLogger::new())) + .await + .unwrap(); + + // Same artifact shape as before this feature existed: a bare `.dump` + // file, not a bundle. + assert!(file_path.to_string_lossy().ends_with(".dump")); +} + #[tokio::test] async fn globals_dump_then_apply_round_trips() { init_tracing_for_test(); From b7ec9a3b83188b52147d37924eb5f267fdfd4641 Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Fri, 26 Jun 2026 12:03:13 +0200 Subject: [PATCH 07/19] feat: replay globals before pg_restore when backup archive is a bundle Co-Authored-By: Claude Sonnet 4.6 --- src/domain/postgres/restore.rs | 132 ++++++++++++++++++++------------- src/tests/domain/postgres.rs | 37 +++++++++ 2 files changed, 118 insertions(+), 51 deletions(-) diff --git a/src/domain/postgres/restore.rs b/src/domain/postgres/restore.rs index 9a005ec..714f9fa 100644 --- a/src/domain/postgres/restore.rs +++ b/src/domain/postgres/restore.rs @@ -7,6 +7,8 @@ use std::time::Instant; use super::connection::{select_pg_path, server_version, terminate_connections}; use super::format::PostgresDumpFormat; +use super::bundle; +use super::globals; use crate::services::backup::logger::JobLogger; use crate::services::config::DatabaseConfig; @@ -35,6 +37,19 @@ pub async fn run( logger.log("debug", format!("Using pg_restore at {:?}", pg_restore)); + let resolved = match bundle::resolve(&restore_file) { + Ok(r) => r, + Err(e) => { + logger.log("error", format!("Failed to inspect restore archive for {}: {:?}", cfg.name, e)); + return Err(e); + } + }; + let format = resolved.format_override.unwrap_or(format); + + if resolved.globals_path.is_some() { + logger.log("info", format!("Globals found in backup archive for {}", cfg.name)); + } + if let Err(e) = futures::executor::block_on(terminate_connections(&cfg)) { logger.log("error", format!("Failed to terminate connections for {}: {:?}", cfg.name, e)); return Err(e.into()); @@ -44,6 +59,11 @@ pub async fn run( match format { PostgresDumpFormat::Fc => { logger.log("info", format!("Running FC restore for {}", cfg.name)); + + if let Some(globals_sql) = &resolved.globals_path { + globals::apply(&cfg, &version, globals_sql, &env, &logger); + } + let start = Instant::now(); let output = Command::new(&pg_restore) .arg("--no-owner") @@ -56,7 +76,7 @@ pub async fn run( .arg("--username").arg(&cfg.username) .arg("--dbname").arg(&cfg.database) .arg("-v") - .arg(&restore_file) + .arg(&resolved.dump_path) .envs(env) .output(); @@ -89,63 +109,73 @@ pub async fn run( PostgresDumpFormat::Fd => { logger.log("info", format!("Running FD restore for {}", cfg.name)); - let tar_gz = match std::fs::File::open(&restore_file) { - Ok(f) => f, - Err(e) => { - logger.log("error", format!( - "Failed to open restore file {:?} for {}: {:?}", - restore_file, cfg.name, e - )); - return Err(e.into()); + // Must outlive the pg_restore call below: dropping it before + // then would delete the files pg_restore is about to read. + let mut legacy_tmp_dir: Option = None; + + let dump_dir = if resolved.dump_path.is_dir() { + // bundle::resolve already unpacked this for us. + if resolved.dump_path.join("toc.dat").exists() { + resolved.dump_path.clone() + } else { + std::fs::read_dir(&resolved.dump_path)? + .filter_map(|e| e.ok()) + .find(|entry| entry.path().join("toc.dat").exists()) + .map(|e| e.path()) + .ok_or_else(|| anyhow::anyhow!("Invalid bundle: toc.dat not found under {:?}", resolved.dump_path))? } - }; - - logger.log("info", format!("tar_gz {:?}", tar_gz)); - - let dec = flate2::read::GzDecoder::new(tar_gz); - let mut archive = tar::Archive::new(dec); + } else { + // Legacy path: resolved.dump_path is the tar.gz itself + // (no manifest.json was found by bundle::resolve), unpack + // it exactly as before this feature existed. + let tar_gz = match std::fs::File::open(&resolved.dump_path) { + Ok(f) => f, + Err(e) => { + logger.log("error", format!( + "Failed to open restore file {:?} for {}: {:?}", + resolved.dump_path, cfg.name, e + )); + return Err(e.into()); + } + }; + + let dec = flate2::read::GzDecoder::new(tar_gz); + let mut archive = tar::Archive::new(dec); + + let tmp_dir = match tempfile::TempDir::new() { + Ok(d) => d, + Err(e) => { + logger.log("error", format!( + "Failed to create temporary directory for FD restore of {}: {:?}", + cfg.name, e + )); + return Err(e.into()); + } + }; - let tmp_dir = match tempfile::TempDir::new() { - Ok(d) => d, - Err(e) => { - logger.log("error", format!( - "Failed to create temporary directory for FD restore of {}: {:?}", - cfg.name, e - )); + if let Err(e) = archive.unpack(tmp_dir.path()) { + logger.log("error", format!("Failed to unpack FD archive for {}: {:?}", cfg.name, e)); return Err(e.into()); } - }; - if let Err(e) = archive.unpack(tmp_dir.path()) { - logger.log("error", format!("Failed to unpack FD archive for {}: {:?}", cfg.name, e)); - return Err(e.into()); - } + let found = if tmp_dir.path().join("toc.dat").exists() { + tmp_dir.path().to_path_buf() + } else { + std::fs::read_dir(tmp_dir.path())? + .filter_map(|e| e.ok()) + .find(|entry| entry.path().join("toc.dat").exists()) + .map(|e| e.path()) + .ok_or_else(|| anyhow::anyhow!("Invalid FD archive: toc.dat not found"))? + }; + + legacy_tmp_dir = Some(tmp_dir); + found + }; - logger.log("debug", format!("Listing contents of temp dir: {}", tmp_dir.path().display())); - - for entry in std::fs::read_dir(tmp_dir.path())? { - if let Ok(entry) = entry { - let path = entry.path(); - let file_type = entry.file_type()?; - logger.log("debug", format!( - " - {} | is_dir: {} | is_file: {}", - path.display(), - file_type.is_dir(), - file_type.is_file() - )); - } + if let Some(globals_sql) = &resolved.globals_path { + globals::apply(&cfg, &version, globals_sql, &env, &logger); } - let dump_dir = if tmp_dir.path().join("toc.dat").exists() { - tmp_dir.path().to_path_buf() - } else { - std::fs::read_dir(tmp_dir.path())? - .filter_map(|e| e.ok()) - .find(|entry| entry.path().join("toc.dat").exists()) - .map(|e| e.path()) - .ok_or_else(|| anyhow::anyhow!("Invalid FD archive: toc.dat not found"))? - }; - let start = Instant::now(); let output = Command::new(&pg_restore) .arg("--no-owner") @@ -160,7 +190,7 @@ pub async fn run( .arg("-v") .arg("-j") .arg("4") - .arg(dump_dir) + .arg(&dump_dir) .envs(env) .output(); diff --git a/src/tests/domain/postgres.rs b/src/tests/domain/postgres.rs index eb0431b..bffc154 100644 --- a/src/tests/domain/postgres.rs +++ b/src/tests/domain/postgres.rs @@ -110,6 +110,43 @@ async fn postgres_backup_restore_test() { } } +#[tokio::test] +async fn backup_and_restore_round_trip_with_globals() { + init_tracing_for_test(); + + let (_container, mut config) = create_config().await; + config.include_globals = true; + + let temp_dir = TempDir::new().unwrap(); + let backup_path = temp_dir.path(); + + let db = DatabaseFactory::create_for_backup(config.clone()).await; + let file_path = db + .backup(backup_path, std::sync::Arc::new(crate::services::backup::logger::JobLogger::new())) + .await + .unwrap(); + + // backup.rs already hands back a `.tar.gz` when include_globals is set, + // so compress_backup's own "already compressed" short-circuit applies — + // this mirrors what executor.rs does in production. + let compression = compress_to_tar_gz_large(&file_path, std::sync::Arc::new(crate::services::backup::logger::JobLogger::new())) + .await + .unwrap(); + assert_eq!(compression.compressed_path, file_path); + + let db = DatabaseFactory::create_for_restore(config.clone(), &compression.compressed_path).await; + let reachable = db.ping().await.unwrap_or(false); + assert!(reachable); + + match db.restore(&compression.compressed_path, std::sync::Arc::new(crate::services::backup::logger::JobLogger::new())).await { + Ok(_) => info!("Restore with globals succeeded for {}", config.generated_id), + Err(e) => { + error!("Restore with globals failed for {}: {:?}", config.generated_id, e); + assert!(false); + } + } +} + #[tokio::test] async fn backup_with_include_globals_produces_a_manifest_bundle() { init_tracing_for_test(); From 26d4dc054e6c7ec5c36ecf17e9ee6a3cdc2b67e2 Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Fri, 26 Jun 2026 12:07:22 +0200 Subject: [PATCH 08/19] refactor: bind FD restore tempdir guard once to clear unused warnings The legacy_tmp_dir Option was assigned-but-never-read, tripping unused_variables/unused_assignments. Replace the mut + reassignment with a bind-once (PathBuf, Option) tuple so the TempDir is still held for RAII through pg_restore, with no warnings. Co-Authored-By: Claude Opus 4.8 --- src/domain/postgres/restore.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/domain/postgres/restore.rs b/src/domain/postgres/restore.rs index 714f9fa..035a22f 100644 --- a/src/domain/postgres/restore.rs +++ b/src/domain/postgres/restore.rs @@ -109,13 +109,12 @@ pub async fn run( PostgresDumpFormat::Fd => { logger.log("info", format!("Running FD restore for {}", cfg.name)); - // Must outlive the pg_restore call below: dropping it before - // then would delete the files pg_restore is about to read. - let mut legacy_tmp_dir: Option = None; - - let dump_dir = if resolved.dump_path.is_dir() { + // `_tmp_guard` must outlive the pg_restore call below: dropping + // the TempDir before then would delete the files pg_restore is + // about to read. It is bound only for its RAII drop, never read. + let (dump_dir, _tmp_guard): (std::path::PathBuf, Option) = if resolved.dump_path.is_dir() { // bundle::resolve already unpacked this for us. - if resolved.dump_path.join("toc.dat").exists() { + let dir = if resolved.dump_path.join("toc.dat").exists() { resolved.dump_path.clone() } else { std::fs::read_dir(&resolved.dump_path)? @@ -123,7 +122,8 @@ pub async fn run( .find(|entry| entry.path().join("toc.dat").exists()) .map(|e| e.path()) .ok_or_else(|| anyhow::anyhow!("Invalid bundle: toc.dat not found under {:?}", resolved.dump_path))? - } + }; + (dir, None) } else { // Legacy path: resolved.dump_path is the tar.gz itself // (no manifest.json was found by bundle::resolve), unpack @@ -168,8 +168,7 @@ pub async fn run( .ok_or_else(|| anyhow::anyhow!("Invalid FD archive: toc.dat not found"))? }; - legacy_tmp_dir = Some(tmp_dir); - found + (found, Some(tmp_dir)) }; if let Some(globals_sql) = &resolved.globals_path { From 5220c4827d9d99766f2e94e4bb7999dc11a25dd9 Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Fri, 26 Jun 2026 12:08:03 +0200 Subject: [PATCH 09/19] docs: demonstrate include_globals in sample databases.json Co-Authored-By: Claude Opus 4.8 --- databases.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/databases.json b/databases.json index 70eface..720c12e 100644 --- a/databases.json +++ b/databases.json @@ -8,7 +8,8 @@ "password": "changeme", "port": 5432, "host": "db-postgres", - "generated_id": "16678159-ff7e-4c97-8c83-0adeff214681" + "generated_id": "16678159-ff7e-4c97-8c83-0adeff214681", + "include_globals": true }, { "name": "Test database 2 - MariaDB", From ec81a9dd705aa0c8aa04a7c374f4f075819c4b3f Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Fri, 26 Jun 2026 12:15:18 +0200 Subject: [PATCH 10/19] chore: silence test-only re-export warning in non-test builds PostgresDumpFormat re-export is consumed only by the in-crate test suite; guard it with cfg_attr(not(test), allow(unused_imports)) so production cargo build stays warning-free. Co-Authored-By: Claude Opus 4.8 --- src/domain/postgres/mod.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/domain/postgres/mod.rs b/src/domain/postgres/mod.rs index 5545750..ecdf885 100644 --- a/src/domain/postgres/mod.rs +++ b/src/domain/postgres/mod.rs @@ -8,4 +8,8 @@ mod ping; mod restore; pub use connection::{detect_format_from_file, detect_format_from_size}; +// Re-exported for the in-crate test suite (`tests::domain::postgres_bundle`); +// production code reaches the type via `super::format::PostgresDumpFormat`, +// so the re-export is unused in a non-test build. +#[cfg_attr(not(test), allow(unused_imports))] pub use format::PostgresDumpFormat; From 5fa44ee3202963c7493962d012d3cacbaa900224 Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Fri, 26 Jun 2026 20:10:53 +0200 Subject: [PATCH 11/19] revert: remove include_globals feature, restore plain pg_dump/pg_restore Replaced by the upcoming postgresql-cluster (pg_dumpall) mode. Reverts the include_globals bundle/globals system to the pre-feature baseline. --- databases.json | 3 +- src/domain/postgres/backup.rs | 19 +--- src/domain/postgres/bundle.rs | 142 -------------------------- src/domain/postgres/connection.rs | 16 --- src/domain/postgres/format.rs | 35 +------ src/domain/postgres/globals.rs | 116 --------------------- src/domain/postgres/mod.rs | 7 -- src/domain/postgres/restore.rs | 133 ++++++++++-------------- src/services/config.rs | 9 -- src/tests/domain/firebird.rs | 1 - src/tests/domain/mariadb.rs | 1 - src/tests/domain/mod.rs | 1 - src/tests/domain/mongodb.rs | 1 - src/tests/domain/mssql.rs | 1 - src/tests/domain/mysql.rs | 1 - src/tests/domain/postgres.rs | 150 +--------------------------- src/tests/domain/postgres_bundle.rs | 63 ------------ src/tests/domain/redis.rs | 1 - src/tests/domain/valkey.rs | 1 - src/tests/services/config_tests.rs | 129 ------------------------ src/tests/services/mod.rs | 1 - 21 files changed, 57 insertions(+), 774 deletions(-) delete mode 100644 src/domain/postgres/bundle.rs delete mode 100644 src/domain/postgres/globals.rs delete mode 100644 src/tests/domain/postgres_bundle.rs delete mode 100644 src/tests/services/config_tests.rs diff --git a/databases.json b/databases.json index 720c12e..70eface 100644 --- a/databases.json +++ b/databases.json @@ -8,8 +8,7 @@ "password": "changeme", "port": 5432, "host": "db-postgres", - "generated_id": "16678159-ff7e-4c97-8c83-0adeff214681", - "include_globals": true + "generated_id": "16678159-ff7e-4c97-8c83-0adeff214681" }, { "name": "Test database 2 - MariaDB", diff --git a/src/domain/postgres/backup.rs b/src/domain/postgres/backup.rs index e5198e2..906cfd9 100644 --- a/src/domain/postgres/backup.rs +++ b/src/domain/postgres/backup.rs @@ -7,7 +7,6 @@ use std::time::Instant; use super::connection::{select_pg_path, server_version}; use super::format::PostgresDumpFormat; -use super::bundle; use crate::services::backup::logger::JobLogger; use crate::services::config::DatabaseConfig; @@ -51,7 +50,7 @@ pub async fn run( .arg("-f").arg(&file_path) .arg("-v") .arg("--compress=3") - .envs(env.clone()) + .envs(env) .output(); let duration_ms = start.elapsed().as_millis() as f64; @@ -75,13 +74,6 @@ pub async fn run( return Err(e.into()); } } - if cfg.include_globals { - logger.log("info", format!("Building globals bundle for {}", cfg.name)); - let bundled = bundle::build(&cfg, format, &file_path, &backup_dir, &version, &env, Arc::clone(&logger))?; - logger.log("info", format!("Backup finished for database {}", cfg.name)); - return Ok(bundled); - } - logger.log("info", format!("Backup finished for database {}", cfg.name)); Ok(file_path) } @@ -109,7 +101,7 @@ pub async fn run( .arg("-j").arg("4") .arg("-f").arg(&dump_dir) .arg("-v") - .envs(env.clone()) + .envs(env) .output(); let duration_ms = start.elapsed().as_millis() as f64; @@ -133,13 +125,6 @@ pub async fn run( } } - if cfg.include_globals { - logger.log("info", format!("Building globals bundle for {}", cfg.name)); - let bundled = bundle::build(&cfg, format, &dump_dir, &backup_dir, &version, &env, Arc::clone(&logger))?; - logger.log("info", format!("Backup finished for database {}", cfg.name)); - return Ok(bundled); - } - match std::fs::File::create(&tar_file) { Ok(tar_gz) => { let enc = flate2::write::GzEncoder::new(tar_gz, flate2::Compression::default()); diff --git a/src/domain/postgres/bundle.rs b/src/domain/postgres/bundle.rs deleted file mode 100644 index b688211..0000000 --- a/src/domain/postgres/bundle.rs +++ /dev/null @@ -1,142 +0,0 @@ -use anyhow::Result; -use serde::{Deserialize, Serialize}; -use std::collections::HashMap; -use std::path::{Path, PathBuf}; -use std::sync::Arc; - -use super::format::PostgresDumpFormat; -use super::globals; -use crate::services::backup::logger::JobLogger; -use crate::services::config::DatabaseConfig; - -const MANIFEST_FILENAME: &str = "manifest.json"; - -#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)] -pub struct BundleManifest { - pub format: String, - pub has_globals: bool, - pub dump_path: String, -} - -impl BundleManifest { - pub fn write(&self, dir: &Path) -> Result<()> { - let path = dir.join(MANIFEST_FILENAME); - std::fs::write(path, serde_json::to_string_pretty(self)?)?; - Ok(()) - } - - pub fn read(dir: &Path) -> Result> { - let path = dir.join(MANIFEST_FILENAME); - if !path.is_file() { - return Ok(None); - } - let contents = std::fs::read_to_string(path)?; - Ok(Some(serde_json::from_str(&contents)?)) - } -} - -/// Packs an already-produced dump artifact (a `.dump` file for `Fc`, or the -/// raw `pg_dump -Fd` output directory for `Fd`) together with a fresh -/// `globals.sql` and a `manifest.json` into one tar.gz. The returned path -/// already ends in `.tar.gz` so `compress_backup`'s "already compressed" -/// check short-circuits and never re-wraps it. -pub fn build( - cfg: &DatabaseConfig, - format: PostgresDumpFormat, - dump_artifact: &Path, - backup_dir: &Path, - pg_version: &str, - env: &HashMap, - logger: Arc, -) -> Result { - let bundle_dir = backup_dir.join(format!("{}_bundle", cfg.generated_id)); - std::fs::create_dir_all(&bundle_dir)?; - - let dump_path_in_bundle = if dump_artifact.is_dir() { - let dest = bundle_dir.join("dump_dir"); - std::fs::rename(dump_artifact, &dest)?; - "dump_dir".to_string() - } else { - let dest = bundle_dir.join("dump.dump"); - std::fs::rename(dump_artifact, &dest)?; - "dump.dump".to_string() - }; - - globals::dump(cfg, pg_version, &bundle_dir, env, &logger)?; - - BundleManifest { - format: format.as_str().to_string(), - has_globals: true, - dump_path: dump_path_in_bundle, - } - .write(&bundle_dir)?; - - let tar_path = backup_dir.join(format!("{}.tar.gz", cfg.generated_id)); - let tar_gz = std::fs::File::create(&tar_path)?; - let enc = flate2::write::GzEncoder::new(tar_gz, flate2::Compression::default()); - let mut tar = tar::Builder::new(enc); - tar.append_dir_all(".", &bundle_dir)?; - tar.finish()?; - - logger.log("info", format!("Globals bundle created at {:?}", tar_path)); - - Ok(tar_path) -} - -pub struct ResolvedRestore { - pub dump_path: PathBuf, - pub globals_path: Option, - pub format_override: Option, - _tmp_dir: Option, -} - -/// Peeks `restore_file` for a bundle produced by `build()`. Any archive -/// without a `manifest.json` at its root — every backup taken before this -/// feature existed, and every backup taken with `include_globals: false` — -/// passes through with `dump_path` unchanged and `format_override: None`, -/// leaving `restore.rs`'s existing Fc/Fd handling exactly as it was. -pub fn resolve(restore_file: &Path) -> Result { - let passthrough = || ResolvedRestore { - dump_path: restore_file.to_path_buf(), - globals_path: None, - format_override: None, - _tmp_dir: None, - }; - - let file = match std::fs::File::open(restore_file) { - Ok(f) => f, - Err(_) => return Ok(passthrough()), - }; - - let dec = flate2::read::GzDecoder::new(file); - let mut archive = tar::Archive::new(dec); - let tmp_dir = tempfile::TempDir::new()?; - - if archive.unpack(tmp_dir.path()).is_err() { - return Ok(passthrough()); - } - - let manifest = match BundleManifest::read(tmp_dir.path())? { - Some(m) => m, - None => return Ok(passthrough()), - }; - - let format_override = PostgresDumpFormat::from_str(&manifest.format).ok_or_else(|| { - anyhow::anyhow!("Unknown format '{}' in bundle manifest", manifest.format) - })?; - - let dump_path = tmp_dir.path().join(&manifest.dump_path); - let globals_path = if manifest.has_globals { - let p = tmp_dir.path().join("globals.sql"); - if p.is_file() { Some(p) } else { None } - } else { - None - }; - - Ok(ResolvedRestore { - dump_path, - globals_path, - format_override: Some(format_override), - _tmp_dir: Some(tmp_dir), - }) -} diff --git a/src/domain/postgres/connection.rs b/src/domain/postgres/connection.rs index 6546e1d..a8bb098 100644 --- a/src/domain/postgres/connection.rs +++ b/src/domain/postgres/connection.rs @@ -109,22 +109,6 @@ pub(crate) fn pg_dump_binary_name() -> &'static str { } } -pub(crate) fn pg_dumpall_binary_name() -> &'static str { - if cfg!(target_os = "windows") { - "pg_dumpall.exe" - } else { - "pg_dumpall" - } -} - -pub(crate) fn psql_binary_name() -> &'static str { - if cfg!(target_os = "windows") { - "psql.exe" - } else { - "psql" - } -} - pub(crate) fn pg_dump_exists_in(dir: &std::path::Path) -> bool { dir.join(pg_dump_binary_name()).is_file() } diff --git a/src/domain/postgres/format.rs b/src/domain/postgres/format.rs index cff40c4..5415fd7 100644 --- a/src/domain/postgres/format.rs +++ b/src/domain/postgres/format.rs @@ -1,38 +1,5 @@ -#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[derive(Clone, Copy)] pub enum PostgresDumpFormat { Fc, Fd, } - -impl PostgresDumpFormat { - pub fn as_str(&self) -> &'static str { - match self { - PostgresDumpFormat::Fc => "fc", - PostgresDumpFormat::Fd => "fd", - } - } - - pub fn from_str(s: &str) -> Option { - match s { - "fc" => Some(PostgresDumpFormat::Fc), - "fd" => Some(PostgresDumpFormat::Fd), - _ => None, - } - } -} - -#[cfg(test)] -mod tests { - use super::PostgresDumpFormat; - - #[test] - fn round_trips_through_as_str_and_from_str() { - assert_eq!(PostgresDumpFormat::from_str(PostgresDumpFormat::Fc.as_str()), Some(PostgresDumpFormat::Fc)); - assert_eq!(PostgresDumpFormat::from_str(PostgresDumpFormat::Fd.as_str()), Some(PostgresDumpFormat::Fd)); - } - - #[test] - fn from_str_rejects_unknown_value() { - assert_eq!(PostgresDumpFormat::from_str("plain"), None); - } -} diff --git a/src/domain/postgres/globals.rs b/src/domain/postgres/globals.rs deleted file mode 100644 index 184e2df..0000000 --- a/src/domain/postgres/globals.rs +++ /dev/null @@ -1,116 +0,0 @@ -use anyhow::Result; -use std::collections::HashMap; -use std::path::{Path, PathBuf}; -use std::process::Command; -use std::sync::Arc; -use std::time::Instant; - -use super::connection::{pg_dumpall_binary_name, psql_binary_name, select_pg_path}; -use crate::services::backup::logger::JobLogger; -use crate::services::config::DatabaseConfig; - -pub fn dump( - cfg: &DatabaseConfig, - pg_version: &str, - out_dir: &Path, - env: &HashMap, - logger: &Arc, -) -> Result { - let pg_dumpall = select_pg_path(pg_version).join(pg_dumpall_binary_name()); - let globals_path = out_dir.join("globals.sql"); - - logger.log("info", format!("Dumping cluster globals via {:?}", pg_dumpall)); - - let start = Instant::now(); - let output = Command::new(&pg_dumpall) - .arg("--host").arg(&cfg.host) - .arg("--port").arg(cfg.port.to_string()) - .arg("--username").arg(&cfg.username) - .arg("--globals-only") - .arg("-f").arg(&globals_path) - .envs(env.clone()) - .output(); - let duration_ms = start.elapsed().as_millis() as f64; - - match output { - Ok(o) => { - let stderr = String::from_utf8_lossy(&o.stderr).to_string(); - let exit_code = o.status.code().unwrap_or(-1); - - if o.status.success() { - logger.log_command( - "pg_dumpall --globals-only", - if stderr.is_empty() { None } else { Some(stderr) }, - Some(0), - Some(duration_ms), - ); - logger.log("info", "Globals dump completed".to_string()); - Ok(globals_path) - } else { - logger.log_command("pg_dumpall --globals-only", Some(stderr), Some(exit_code), Some(duration_ms)); - anyhow::bail!("pg_dumpall --globals-only failed for cluster {}:{}", cfg.host, cfg.port); - } - } - Err(e) => { - logger.log_command("pg_dumpall --globals-only", Some(e.to_string()), Some(-1), Some(duration_ms)); - Err(e.into()) - } - } -} - -/// Replays a previously captured `globals.sql` against the cluster's -/// `postgres` maintenance database. Globals are best-effort enrichment -/// (roles/tablespaces commonly already exist on a shared target cluster) — -/// this function logs failures but never returns an error, so it can never -/// block the real database restore that follows it. -pub fn apply( - cfg: &DatabaseConfig, - pg_version: &str, - globals_sql: &Path, - env: &HashMap, - logger: &Arc, -) { - let psql = select_pg_path(pg_version).join(psql_binary_name()); - - logger.log("info", format!("Applying cluster globals from {:?}", globals_sql)); - - let start = Instant::now(); - let output = Command::new(&psql) - .arg("--host").arg(&cfg.host) - .arg("--port").arg(cfg.port.to_string()) - .arg("--username").arg(&cfg.username) - .arg("--dbname").arg("postgres") - .arg("-v").arg("ON_ERROR_STOP=0") - .arg("-f").arg(globals_sql) - .envs(env.clone()) - .output(); - let duration_ms = start.elapsed().as_millis() as f64; - - match output { - Ok(o) => { - let stderr = String::from_utf8_lossy(&o.stderr).to_string(); - let stdout = String::from_utf8_lossy(&o.stdout).to_string(); - let combined = format!("{}{}", stdout, stderr); - let exit_code = o.status.code(); - - logger.log_command( - "psql -f globals.sql", - if combined.is_empty() { None } else { Some(combined) }, - exit_code, - Some(duration_ms), - ); - - if exit_code == Some(0) { - logger.log("info", "Globals applied successfully".to_string()); - } else { - logger.log( - "warn", - "Globals apply reported errors (often pre-existing roles/tablespaces); continuing with database restore".to_string(), - ); - } - } - Err(e) => { - logger.log("warn", format!("Could not run psql for globals apply, continuing without globals: {:?}", e)); - } - } -} diff --git a/src/domain/postgres/mod.rs b/src/domain/postgres/mod.rs index ecdf885..c7cd4cb 100644 --- a/src/domain/postgres/mod.rs +++ b/src/domain/postgres/mod.rs @@ -1,15 +1,8 @@ pub mod backup; -pub(crate) mod bundle; pub(crate) mod connection; pub mod database; mod format; -pub(crate) mod globals; mod ping; mod restore; pub use connection::{detect_format_from_file, detect_format_from_size}; -// Re-exported for the in-crate test suite (`tests::domain::postgres_bundle`); -// production code reaches the type via `super::format::PostgresDumpFormat`, -// so the re-export is unused in a non-test build. -#[cfg_attr(not(test), allow(unused_imports))] -pub use format::PostgresDumpFormat; diff --git a/src/domain/postgres/restore.rs b/src/domain/postgres/restore.rs index 035a22f..9a005ec 100644 --- a/src/domain/postgres/restore.rs +++ b/src/domain/postgres/restore.rs @@ -7,8 +7,6 @@ use std::time::Instant; use super::connection::{select_pg_path, server_version, terminate_connections}; use super::format::PostgresDumpFormat; -use super::bundle; -use super::globals; use crate::services::backup::logger::JobLogger; use crate::services::config::DatabaseConfig; @@ -37,19 +35,6 @@ pub async fn run( logger.log("debug", format!("Using pg_restore at {:?}", pg_restore)); - let resolved = match bundle::resolve(&restore_file) { - Ok(r) => r, - Err(e) => { - logger.log("error", format!("Failed to inspect restore archive for {}: {:?}", cfg.name, e)); - return Err(e); - } - }; - let format = resolved.format_override.unwrap_or(format); - - if resolved.globals_path.is_some() { - logger.log("info", format!("Globals found in backup archive for {}", cfg.name)); - } - if let Err(e) = futures::executor::block_on(terminate_connections(&cfg)) { logger.log("error", format!("Failed to terminate connections for {}: {:?}", cfg.name, e)); return Err(e.into()); @@ -59,11 +44,6 @@ pub async fn run( match format { PostgresDumpFormat::Fc => { logger.log("info", format!("Running FC restore for {}", cfg.name)); - - if let Some(globals_sql) = &resolved.globals_path { - globals::apply(&cfg, &version, globals_sql, &env, &logger); - } - let start = Instant::now(); let output = Command::new(&pg_restore) .arg("--no-owner") @@ -76,7 +56,7 @@ pub async fn run( .arg("--username").arg(&cfg.username) .arg("--dbname").arg(&cfg.database) .arg("-v") - .arg(&resolved.dump_path) + .arg(&restore_file) .envs(env) .output(); @@ -109,72 +89,63 @@ pub async fn run( PostgresDumpFormat::Fd => { logger.log("info", format!("Running FD restore for {}", cfg.name)); - // `_tmp_guard` must outlive the pg_restore call below: dropping - // the TempDir before then would delete the files pg_restore is - // about to read. It is bound only for its RAII drop, never read. - let (dump_dir, _tmp_guard): (std::path::PathBuf, Option) = if resolved.dump_path.is_dir() { - // bundle::resolve already unpacked this for us. - let dir = if resolved.dump_path.join("toc.dat").exists() { - resolved.dump_path.clone() - } else { - std::fs::read_dir(&resolved.dump_path)? - .filter_map(|e| e.ok()) - .find(|entry| entry.path().join("toc.dat").exists()) - .map(|e| e.path()) - .ok_or_else(|| anyhow::anyhow!("Invalid bundle: toc.dat not found under {:?}", resolved.dump_path))? - }; - (dir, None) - } else { - // Legacy path: resolved.dump_path is the tar.gz itself - // (no manifest.json was found by bundle::resolve), unpack - // it exactly as before this feature existed. - let tar_gz = match std::fs::File::open(&resolved.dump_path) { - Ok(f) => f, - Err(e) => { - logger.log("error", format!( - "Failed to open restore file {:?} for {}: {:?}", - resolved.dump_path, cfg.name, e - )); - return Err(e.into()); - } - }; - - let dec = flate2::read::GzDecoder::new(tar_gz); - let mut archive = tar::Archive::new(dec); - - let tmp_dir = match tempfile::TempDir::new() { - Ok(d) => d, - Err(e) => { - logger.log("error", format!( - "Failed to create temporary directory for FD restore of {}: {:?}", - cfg.name, e - )); - return Err(e.into()); - } - }; - - if let Err(e) = archive.unpack(tmp_dir.path()) { - logger.log("error", format!("Failed to unpack FD archive for {}: {:?}", cfg.name, e)); + let tar_gz = match std::fs::File::open(&restore_file) { + Ok(f) => f, + Err(e) => { + logger.log("error", format!( + "Failed to open restore file {:?} for {}: {:?}", + restore_file, cfg.name, e + )); return Err(e.into()); } + }; + + logger.log("info", format!("tar_gz {:?}", tar_gz)); + + let dec = flate2::read::GzDecoder::new(tar_gz); + let mut archive = tar::Archive::new(dec); - let found = if tmp_dir.path().join("toc.dat").exists() { - tmp_dir.path().to_path_buf() - } else { - std::fs::read_dir(tmp_dir.path())? - .filter_map(|e| e.ok()) - .find(|entry| entry.path().join("toc.dat").exists()) - .map(|e| e.path()) - .ok_or_else(|| anyhow::anyhow!("Invalid FD archive: toc.dat not found"))? - }; - - (found, Some(tmp_dir)) + let tmp_dir = match tempfile::TempDir::new() { + Ok(d) => d, + Err(e) => { + logger.log("error", format!( + "Failed to create temporary directory for FD restore of {}: {:?}", + cfg.name, e + )); + return Err(e.into()); + } }; - if let Some(globals_sql) = &resolved.globals_path { - globals::apply(&cfg, &version, globals_sql, &env, &logger); + if let Err(e) = archive.unpack(tmp_dir.path()) { + logger.log("error", format!("Failed to unpack FD archive for {}: {:?}", cfg.name, e)); + return Err(e.into()); } + logger.log("debug", format!("Listing contents of temp dir: {}", tmp_dir.path().display())); + + for entry in std::fs::read_dir(tmp_dir.path())? { + if let Ok(entry) = entry { + let path = entry.path(); + let file_type = entry.file_type()?; + logger.log("debug", format!( + " - {} | is_dir: {} | is_file: {}", + path.display(), + file_type.is_dir(), + file_type.is_file() + )); + } + } + + let dump_dir = if tmp_dir.path().join("toc.dat").exists() { + tmp_dir.path().to_path_buf() + } else { + std::fs::read_dir(tmp_dir.path())? + .filter_map(|e| e.ok()) + .find(|entry| entry.path().join("toc.dat").exists()) + .map(|e| e.path()) + .ok_or_else(|| anyhow::anyhow!("Invalid FD archive: toc.dat not found"))? + }; + let start = Instant::now(); let output = Command::new(&pg_restore) .arg("--no-owner") @@ -189,7 +160,7 @@ pub async fn run( .arg("-v") .arg("-j") .arg("4") - .arg(&dump_dir) + .arg(dump_dir) .envs(env) .output(); diff --git a/src/services/config.rs b/src/services/config.rs index 7f4a698..fe9f3c6 100644 --- a/src/services/config.rs +++ b/src/services/config.rs @@ -55,7 +55,6 @@ pub struct DatabaseConfig { pub generated_id: String, pub path: String, pub max_packet_size: String, - pub include_globals: bool, } #[allow(dead_code)] @@ -78,8 +77,6 @@ pub struct InputDatabaseConfig { pub generated_id: String, pub path: Option, pub max_packet_size: Option, - #[serde(default, alias = "includeGlobals")] - pub include_globals: Option, } #[allow(dead_code)] @@ -226,11 +223,6 @@ impl ConfigService { _ => String::new(), }; - let include_globals = match db.db_type { - DbType::Postgresql => optional(&db.include_globals), - _ => false, - }; - databases.push(DatabaseConfig { name: db.name, database: database_name, @@ -242,7 +234,6 @@ impl ConfigService { generated_id: db.generated_id, path: path_val, max_packet_size, - include_globals, }); } diff --git a/src/tests/domain/firebird.rs b/src/tests/domain/firebird.rs index b4fa769..2ae4320 100644 --- a/src/tests/domain/firebird.rs +++ b/src/tests/domain/firebird.rs @@ -40,7 +40,6 @@ async fn create_config() -> (ContainerAsync, DatabaseConfig) { generated_id: "3c445eb4-c2c6-4bde-a423-ee1385dcf6d2".to_string(), path: "".to_string(), max_packet_size: "".to_string(), - include_globals: false, }; (container, config) diff --git a/src/tests/domain/mariadb.rs b/src/tests/domain/mariadb.rs index 2105a73..1989bbd 100644 --- a/src/tests/domain/mariadb.rs +++ b/src/tests/domain/mariadb.rs @@ -32,7 +32,6 @@ async fn create_config() -> (ContainerAsync, DatabaseConfig) { generated_id: "3c4b4eb4-c2c6-4bde-a423-ee1385dcf6d2".to_string(), path: "".to_string(), max_packet_size: "512M".to_string(), - include_globals: false, }; (container, config) diff --git a/src/tests/domain/mod.rs b/src/tests/domain/mod.rs index f7d3586..641e097 100644 --- a/src/tests/domain/mod.rs +++ b/src/tests/domain/mod.rs @@ -2,7 +2,6 @@ mod mariadb; mod mongodb; mod mysql; mod postgres; -mod postgres_bundle; mod redis; mod valkey; mod firebird; diff --git a/src/tests/domain/mongodb.rs b/src/tests/domain/mongodb.rs index fb2cc32..d2d0688 100644 --- a/src/tests/domain/mongodb.rs +++ b/src/tests/domain/mongodb.rs @@ -30,7 +30,6 @@ async fn create_config() -> (ContainerAsync, DatabaseConfig) { generated_id: "96d30a9f-ff4b-47c9-aaab-f3147bb34f16".to_string(), path: "".to_string(), max_packet_size: "".to_string(), - include_globals: false, }; (container, config) diff --git a/src/tests/domain/mssql.rs b/src/tests/domain/mssql.rs index 2c8303e..d6a4001 100644 --- a/src/tests/domain/mssql.rs +++ b/src/tests/domain/mssql.rs @@ -55,7 +55,6 @@ fn make_config(host: String, port: u16, database: &str, generated_id: &str) -> D generated_id: generated_id.to_string(), path: "".to_string(), max_packet_size: "".to_string(), - include_globals: false, } } diff --git a/src/tests/domain/mysql.rs b/src/tests/domain/mysql.rs index 4b50661..7a48bea 100644 --- a/src/tests/domain/mysql.rs +++ b/src/tests/domain/mysql.rs @@ -32,7 +32,6 @@ async fn create_config() -> (ContainerAsync, DatabaseConfig) { generated_id: "0f1bb8f2-35a0-4c91-8098-e36873d3ce31".to_string(), path: "".to_string(), max_packet_size: "512M".to_string(), - include_globals: false, }; (container, config) diff --git a/src/tests/domain/postgres.rs b/src/tests/domain/postgres.rs index bffc154..c03bdcd 100644 --- a/src/tests/domain/postgres.rs +++ b/src/tests/domain/postgres.rs @@ -39,7 +39,6 @@ async fn create_config() -> (ContainerAsync, DatabaseConfig) { generated_id: "40875631-e3d2-4dfe-a26b-2a347ecc64fd".to_string(), path: "".to_string(), max_packet_size: "".to_string(), - include_globals: false, }; (container, config) @@ -110,131 +109,6 @@ async fn postgres_backup_restore_test() { } } -#[tokio::test] -async fn backup_and_restore_round_trip_with_globals() { - init_tracing_for_test(); - - let (_container, mut config) = create_config().await; - config.include_globals = true; - - let temp_dir = TempDir::new().unwrap(); - let backup_path = temp_dir.path(); - - let db = DatabaseFactory::create_for_backup(config.clone()).await; - let file_path = db - .backup(backup_path, std::sync::Arc::new(crate::services::backup::logger::JobLogger::new())) - .await - .unwrap(); - - // backup.rs already hands back a `.tar.gz` when include_globals is set, - // so compress_backup's own "already compressed" short-circuit applies — - // this mirrors what executor.rs does in production. - let compression = compress_to_tar_gz_large(&file_path, std::sync::Arc::new(crate::services::backup::logger::JobLogger::new())) - .await - .unwrap(); - assert_eq!(compression.compressed_path, file_path); - - let db = DatabaseFactory::create_for_restore(config.clone(), &compression.compressed_path).await; - let reachable = db.ping().await.unwrap_or(false); - assert!(reachable); - - match db.restore(&compression.compressed_path, std::sync::Arc::new(crate::services::backup::logger::JobLogger::new())).await { - Ok(_) => info!("Restore with globals succeeded for {}", config.generated_id), - Err(e) => { - error!("Restore with globals failed for {}: {:?}", config.generated_id, e); - assert!(false); - } - } -} - -#[tokio::test] -async fn backup_with_include_globals_produces_a_manifest_bundle() { - init_tracing_for_test(); - - let (_container, mut config) = create_config().await; - config.include_globals = true; - - let temp_dir = TempDir::new().unwrap(); - let backup_path = temp_dir.path(); - - let db = DatabaseFactory::create_for_backup(config.clone()).await; - let file_path = db - .backup(backup_path, std::sync::Arc::new(crate::services::backup::logger::JobLogger::new())) - .await - .unwrap(); - - assert!(file_path.to_string_lossy().ends_with(".tar.gz")); - - let extract_dir = TempDir::new().unwrap(); - let files = decompress_large_tar_gz(&file_path, extract_dir.path()).await.unwrap(); - assert!(files.len() >= 3, "expected dump + globals.sql + manifest.json, got {:?}", files); - - let manifest = crate::domain::postgres::bundle::BundleManifest::read(extract_dir.path()) - .unwrap() - .expect("manifest.json must be present when include_globals is true"); - assert_eq!(manifest.has_globals, true); - assert_eq!(manifest.format, "fc"); - - assert!(extract_dir.path().join("globals.sql").is_file()); - assert!(extract_dir.path().join(&manifest.dump_path).exists()); -} - -#[tokio::test] -async fn backup_without_include_globals_is_unaffected() { - init_tracing_for_test(); - - let (_container, config) = create_config().await; - assert_eq!(config.include_globals, false); - - let temp_dir = TempDir::new().unwrap(); - let backup_path = temp_dir.path(); - - let db = DatabaseFactory::create_for_backup(config.clone()).await; - let file_path = db - .backup(backup_path, std::sync::Arc::new(crate::services::backup::logger::JobLogger::new())) - .await - .unwrap(); - - // Same artifact shape as before this feature existed: a bare `.dump` - // file, not a bundle. - assert!(file_path.to_string_lossy().ends_with(".dump")); -} - -#[tokio::test] -async fn globals_dump_then_apply_round_trips() { - init_tracing_for_test(); - - let (_container, config) = create_config().await; - let temp_dir = TempDir::new().unwrap(); - - let version = crate::domain::postgres::connection::server_version(&config) - .await - .unwrap(); - - let mut env = std::env::vars().collect::>(); - env.insert("PGPASSWORD".to_string(), config.password.clone()); - - let logger = std::sync::Arc::new(crate::services::backup::logger::JobLogger::new()); - - let globals_path = crate::domain::postgres::globals::dump( - &config, - &version, - temp_dir.path(), - &env, - &logger, - ) - .unwrap(); - - assert!(globals_path.is_file()); - let contents = std::fs::read_to_string(&globals_path).unwrap(); - assert!(contents.contains("ROLE"), "expected role statements in globals.sql, got: {contents}"); - - // Re-applying an already-applied globals.sql must not error out the - // caller: roles/tablespaces already existing on the cluster are expected - // and must be swallowed as warnings, not failures. - crate::domain::postgres::globals::apply(&config, &version, &globals_path, &env, &logger); -} - #[tokio::test] async fn postgres_password_with_slash_test() { init_tracing_for_test(); @@ -268,7 +142,6 @@ async fn postgres_password_with_slash_test() { generated_id: "5a1f0e3c-9b8a-4a8e-9b1b-0a1c2d3e4f5a".to_string(), path: "".to_string(), max_packet_size: "".to_string(), - include_globals: false, }; let db = DatabaseFactory::create_for_backup(config.clone()).await; @@ -279,8 +152,7 @@ async fn postgres_password_with_slash_test() { mod select_pg_path_tests { use crate::domain::postgres::connection::{ - pg_dump_binary_name, pg_dump_exists_in, pg_dumpall_binary_name, psql_binary_name, - select_pg_path_with, + pg_dump_binary_name, pg_dump_exists_in, select_pg_path_with, }; // `select_pg_path_with` takes the `PG_BIN_DIR` override as a plain @@ -337,24 +209,4 @@ mod select_pg_path_tests { let dir = std::path::Path::new("this/path/almost-certainly/does-not-exist-12345"); assert!(!pg_dump_exists_in(dir)); } - - #[test] - fn pg_dumpall_binary_name_is_platform_specific() { - let name = pg_dumpall_binary_name(); - if cfg!(target_os = "windows") { - assert_eq!(name, "pg_dumpall.exe"); - } else { - assert_eq!(name, "pg_dumpall"); - } - } - - #[test] - fn psql_binary_name_is_platform_specific() { - let name = psql_binary_name(); - if cfg!(target_os = "windows") { - assert_eq!(name, "psql.exe"); - } else { - assert_eq!(name, "psql"); - } - } } diff --git a/src/tests/domain/postgres_bundle.rs b/src/tests/domain/postgres_bundle.rs deleted file mode 100644 index c7e0e18..0000000 --- a/src/tests/domain/postgres_bundle.rs +++ /dev/null @@ -1,63 +0,0 @@ -use crate::domain::postgres::bundle::{self, BundleManifest}; -use crate::domain::postgres::PostgresDumpFormat; -use std::fs::File; -use std::io::Write; -use tempfile::TempDir; - -#[test] -fn manifest_round_trips_through_write_and_read() { - let dir = TempDir::new().unwrap(); - - let manifest = BundleManifest { - format: PostgresDumpFormat::Fc.as_str().to_string(), - has_globals: true, - dump_path: "dump.dump".to_string(), - }; - manifest.write(dir.path()).unwrap(); - - let read_back = BundleManifest::read(dir.path()).unwrap(); - assert_eq!(read_back, Some(manifest)); -} - -#[test] -fn manifest_read_returns_none_when_absent() { - let dir = TempDir::new().unwrap(); - assert_eq!(BundleManifest::read(dir.path()).unwrap(), None); -} - -#[test] -fn resolve_passes_through_a_plain_dump_file_unchanged() { - let dir = TempDir::new().unwrap(); - let dump_file = dir.path().join("16678159.dump"); - File::create(&dump_file).unwrap().write_all(b"not a tarball").unwrap(); - - let resolved = bundle::resolve(&dump_file).unwrap(); - - assert_eq!(resolved.dump_path, dump_file); - assert!(resolved.globals_path.is_none()); - assert!(resolved.format_override.is_none()); -} - -#[test] -fn resolve_passes_through_a_legacy_multi_file_tar_gz_unchanged() { - let dir = TempDir::new().unwrap(); - let inner_dir = dir.path().join("inner"); - std::fs::create_dir_all(&inner_dir).unwrap(); - File::create(inner_dir.join("toc.dat")).unwrap().write_all(b"toc").unwrap(); - File::create(inner_dir.join("1234.dat.gz")).unwrap().write_all(b"data").unwrap(); - - let tar_path = dir.path().join("legacy.tar.gz"); - let tar_gz = File::create(&tar_path).unwrap(); - let enc = flate2::write::GzEncoder::new(tar_gz, flate2::Compression::default()); - let mut tar = tar::Builder::new(enc); - tar.append_dir_all(".", &inner_dir).unwrap(); - tar.finish().unwrap(); - - let resolved = bundle::resolve(&tar_path).unwrap(); - - // No manifest.json inside this archive: bundle::resolve must defer to - // restore.rs's own Fd unpack logic rather than touching it. - assert_eq!(resolved.dump_path, tar_path); - assert!(resolved.globals_path.is_none()); - assert!(resolved.format_override.is_none()); -} diff --git a/src/tests/domain/redis.rs b/src/tests/domain/redis.rs index 6953fe6..9cd63ee 100644 --- a/src/tests/domain/redis.rs +++ b/src/tests/domain/redis.rs @@ -29,7 +29,6 @@ async fn create_config() -> (ContainerAsync, DatabaseConfig) { generated_id: "40875631-e3d2-4dfe-a26b-2a347ecc64fd".to_string(), path: "".to_string(), max_packet_size: "".to_string(), - include_globals: false, }; (container, config) diff --git a/src/tests/domain/valkey.rs b/src/tests/domain/valkey.rs index e6145b3..4a78369 100644 --- a/src/tests/domain/valkey.rs +++ b/src/tests/domain/valkey.rs @@ -28,7 +28,6 @@ async fn create_config() -> (ContainerAsync, DatabaseConfig) { generated_id: "40875485-e3d2-4dfe-a26b-2a347ecc64fd".to_string(), path: "".to_string(), max_packet_size: "".to_string(), - include_globals: false, }; (container, config) diff --git a/src/tests/services/config_tests.rs b/src/tests/services/config_tests.rs deleted file mode 100644 index 796ddab..0000000 --- a/src/tests/services/config_tests.rs +++ /dev/null @@ -1,129 +0,0 @@ -use crate::core::context::Context; -use crate::services::api::ApiClient; -use crate::services::config::ConfigService; -use crate::utils::edge_key::EdgeKey; -use std::io::Write; -use std::sync::Arc; -use tempfile::NamedTempFile; - -// `ConfigService::load` never touches `self.ctx` on the `Some(file_path)` -// path these tests use, so the values here don't matter — but -// `Context::new()` panics without an `EDGE_KEY` env var, so build the -// struct directly, the same way `backup_uploader_tests.rs`'s -// `ctx_pointing_at` helper does. -fn test_context() -> Arc { - Arc::new(Context { - edge_key: EdgeKey { - server_url: String::new(), - agent_id: "agent-1".to_string(), - master_key_b64: String::new(), - }, - api: ApiClient::new(String::new()), - }) -} - -fn write_json(contents: &str) -> NamedTempFile { - let mut file = NamedTempFile::with_suffix(".json").unwrap(); - file.write_all(contents.as_bytes()).unwrap(); - file -} - -#[test] -fn include_globals_defaults_to_false_when_absent() { - let file = write_json( - r#"{ - "databases": [ - { - "name": "db1", - "database": "app", - "type": "postgresql", - "username": "u", - "password": "p", - "port": 5432, - "host": "localhost", - "generated_id": "16678159-ff7e-4c97-8c83-0adeff214681" - } - ] - }"#, - ); - - let service = ConfigService::new(test_context()); - let cfg = service.load(Some(file.path().to_str().unwrap())).unwrap(); - - assert_eq!(cfg.databases[0].include_globals, false); -} - -#[test] -fn include_globals_true_is_parsed() { - let file = write_json( - r#"{ - "databases": [ - { - "name": "db1", - "database": "app", - "type": "postgresql", - "username": "u", - "password": "p", - "port": 5432, - "host": "localhost", - "generated_id": "16678159-ff7e-4c97-8c83-0adeff214681", - "include_globals": true - } - ] - }"#, - ); - - let service = ConfigService::new(test_context()); - let cfg = service.load(Some(file.path().to_str().unwrap())).unwrap(); - - assert_eq!(cfg.databases[0].include_globals, true); -} - -#[test] -fn include_globals_camel_case_alias_is_accepted() { - let file = write_json( - r#"{ - "databases": [ - { - "name": "db1", - "database": "app", - "type": "postgresql", - "username": "u", - "password": "p", - "port": 5432, - "host": "localhost", - "generated_id": "16678159-ff7e-4c97-8c83-0adeff214681", - "includeGlobals": true - } - ] - }"#, - ); - - let service = ConfigService::new(test_context()); - let cfg = service.load(Some(file.path().to_str().unwrap())).unwrap(); - - assert_eq!(cfg.databases[0].include_globals, true); -} - -#[test] -fn include_globals_ignored_for_non_postgres_types() { - let file = write_json( - r#"{ - "databases": [ - { - "name": "db1", - "type": "redis", - "port": 6379, - "host": "localhost", - "generated_id": "16678159-ff7e-4c97-8c83-0adeff214681", - "include_globals": true - } - ] - }"#, - ); - - let service = ConfigService::new(test_context()); - let cfg = service.load(Some(file.path().to_str().unwrap())).unwrap(); - - assert_eq!(cfg.databases[0].include_globals, false); -} diff --git a/src/tests/services/mod.rs b/src/tests/services/mod.rs index af4f3eb..1780d61 100644 --- a/src/tests/services/mod.rs +++ b/src/tests/services/mod.rs @@ -1,3 +1,2 @@ mod api_models_tests; mod backup_uploader_tests; -mod config_tests; From 5460c82881539445a38f7a40a0ed8351d606fa5d Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Fri, 26 Jun 2026 20:13:19 +0200 Subject: [PATCH 12/19] feat: add pg_dumpall/psql binary names and is_superuser check --- src/domain/postgres/connection.rs | 30 +++++++++++++++++++++++++ src/tests/domain/postgres.rs | 37 ++++++++++++++++++++++++++++++- 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/src/domain/postgres/connection.rs b/src/domain/postgres/connection.rs index a8bb098..6f12035 100644 --- a/src/domain/postgres/connection.rs +++ b/src/domain/postgres/connection.rs @@ -33,6 +33,20 @@ pub async fn server_version(cfg: &DatabaseConfig) -> Result { Ok(version) } +/// Whether the role used by `cfg` is a cluster superuser. Cluster backup +/// (`pg_dumpall`, needs to read role passwords) and cluster restore (`CREATE +/// ROLE`, `ALTER ... OWNER`) both require a superuser; callers pre-check this +/// and fail fast with a clear error. +pub async fn is_superuser(cfg: &DatabaseConfig) -> Result { + let client = connect(cfg).await?; + let is_super: bool = client + .query_one("SELECT current_setting('is_superuser') = 'on';", &[]) + .await? + .get(0); + + Ok(is_super) +} + /// Resolves the `bin` directory of a PostgreSQL installation for the given /// major version, in a cross-platform way. /// @@ -109,6 +123,22 @@ pub(crate) fn pg_dump_binary_name() -> &'static str { } } +pub(crate) fn pg_dumpall_binary_name() -> &'static str { + if cfg!(target_os = "windows") { + "pg_dumpall.exe" + } else { + "pg_dumpall" + } +} + +pub(crate) fn psql_binary_name() -> &'static str { + if cfg!(target_os = "windows") { + "psql.exe" + } else { + "psql" + } +} + pub(crate) fn pg_dump_exists_in(dir: &std::path::Path) -> bool { dir.join(pg_dump_binary_name()).is_file() } diff --git a/src/tests/domain/postgres.rs b/src/tests/domain/postgres.rs index c03bdcd..6e48d05 100644 --- a/src/tests/domain/postgres.rs +++ b/src/tests/domain/postgres.rs @@ -56,6 +56,20 @@ async fn postgres_ping_test() { assert_eq!(reachable, true); } +#[tokio::test] +async fn is_superuser_detects_superuser_role() { + init_tracing_for_test(); + + // The testcontainer's POSTGRES_USER ("testuser") is the bootstrap superuser. + let (_container, config) = create_config().await; + + let is_super = crate::domain::postgres::connection::is_superuser(&config) + .await + .unwrap(); + + assert!(is_super); +} + #[tokio::test] async fn postgres_backup_restore_test() { init_tracing_for_test(); @@ -152,7 +166,8 @@ async fn postgres_password_with_slash_test() { mod select_pg_path_tests { use crate::domain::postgres::connection::{ - pg_dump_binary_name, pg_dump_exists_in, select_pg_path_with, + pg_dump_binary_name, pg_dump_exists_in, pg_dumpall_binary_name, psql_binary_name, + select_pg_path_with, }; // `select_pg_path_with` takes the `PG_BIN_DIR` override as a plain @@ -209,4 +224,24 @@ mod select_pg_path_tests { let dir = std::path::Path::new("this/path/almost-certainly/does-not-exist-12345"); assert!(!pg_dump_exists_in(dir)); } + + #[test] + fn pg_dumpall_binary_name_is_platform_specific() { + let name = pg_dumpall_binary_name(); + if cfg!(target_os = "windows") { + assert_eq!(name, "pg_dumpall.exe"); + } else { + assert_eq!(name, "pg_dumpall"); + } + } + + #[test] + fn psql_binary_name_is_platform_specific() { + let name = psql_binary_name(); + if cfg!(target_os = "windows") { + assert_eq!(name, "psql.exe"); + } else { + assert_eq!(name, "psql"); + } + } } From e5d1df4de51afcbda8883b3ae7dd3034fba207c6 Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Fri, 26 Jun 2026 20:16:53 +0200 Subject: [PATCH 13/19] feat: add postgresql-cluster db type and config parsing --- src/domain/factory.rs | 2 + src/services/config.rs | 25 +++++++--- src/tests/services/config_tests.rs | 79 ++++++++++++++++++++++++++++++ src/tests/services/mod.rs | 1 + 4 files changed, 101 insertions(+), 6 deletions(-) create mode 100644 src/tests/services/config_tests.rs diff --git a/src/domain/factory.rs b/src/domain/factory.rs index df5651d..901f410 100644 --- a/src/domain/factory.rs +++ b/src/domain/factory.rs @@ -31,6 +31,7 @@ impl DatabaseFactory { let format = detect_format_from_size(&cfg).await; Arc::new(PostgresDatabase::new(cfg, format)) } + DbType::PostgresqlCluster => unimplemented!("postgresql-cluster wired in Task 5"), DbType::Mysql => Arc::new(MySQLDatabase::new(cfg)), DbType::Mariadb => Arc::new(MariaDBDatabase::new(cfg)), DbType::MongoDB => Arc::new(MongoDatabase::new(cfg)), @@ -48,6 +49,7 @@ impl DatabaseFactory { let format = detect_format_from_file(restore_file); Arc::new(PostgresDatabase::new(cfg, format)) } + DbType::PostgresqlCluster => unimplemented!("postgresql-cluster wired in Task 5"), DbType::Mysql => Arc::new(MySQLDatabase::new(cfg)), DbType::Mariadb => Arc::new(MariaDBDatabase::new(cfg)), DbType::MongoDB => Arc::new(MongoDatabase::new(cfg)), diff --git a/src/services/config.rs b/src/services/config.rs index fe9f3c6..eea6eb6 100644 --- a/src/services/config.rs +++ b/src/services/config.rs @@ -17,6 +17,8 @@ pub enum DbType { Mysql, Mariadb, Postgresql, + #[serde(rename = "postgresql-cluster")] + PostgresqlCluster, MongoDB, Sqlite, Redis, @@ -31,6 +33,7 @@ impl DbType { DbType::Mysql => "mysql", DbType::Mariadb => "mariadb", DbType::Postgresql => "postgresql", + DbType::PostgresqlCluster => "postgresql-cluster", DbType::MongoDB => "mongodb", DbType::Sqlite => "sqlite", DbType::Redis => "redis", @@ -169,21 +172,26 @@ impl ConfigService { } let username = match db.db_type { - DbType::Postgresql | DbType::Mysql | DbType::Mariadb | DbType::Mssql => { - required(&db.username, &db.name, "username")? - } + DbType::Postgresql + | DbType::PostgresqlCluster + | DbType::Mysql + | DbType::Mariadb + | DbType::Mssql => required(&db.username, &db.name, "username")?, _ => optional(&db.username), }; let password = match db.db_type { - DbType::Postgresql | DbType::Mysql | DbType::Mariadb | DbType::Mssql => { - required(&db.password, &db.name, "password")? - } + DbType::Postgresql + | DbType::PostgresqlCluster + | DbType::Mysql + | DbType::Mariadb + | DbType::Mssql => required(&db.password, &db.name, "password")?, _ => optional(&db.password), }; let host = match db.db_type { DbType::Postgresql + | DbType::PostgresqlCluster | DbType::Mysql | DbType::Mariadb | DbType::MongoDB @@ -196,6 +204,7 @@ impl ConfigService { let port = match db.db_type { DbType::Postgresql + | DbType::PostgresqlCluster | DbType::Mysql | DbType::Mariadb | DbType::MongoDB @@ -208,6 +217,10 @@ impl ConfigService { let database_name = match db.db_type { DbType::Sqlite | DbType::Redis | DbType::Valkey => optional(&db.database), + DbType::PostgresqlCluster => db + .database + .clone() + .unwrap_or_else(|| "postgres".to_string()), _ => required(&db.database, &db.name, "database")?, }; diff --git a/src/tests/services/config_tests.rs b/src/tests/services/config_tests.rs new file mode 100644 index 0000000..6c5f79b --- /dev/null +++ b/src/tests/services/config_tests.rs @@ -0,0 +1,79 @@ +use crate::core::context::Context; +use crate::services::api::ApiClient; +use crate::services::config::ConfigService; +use crate::utils::edge_key::EdgeKey; +use std::io::Write; +use std::sync::Arc; +use tempfile::NamedTempFile; + +// `ConfigService::load` never touches `self.ctx` on the `Some(file_path)` path, +// so the values here don't matter — but `Context::new()` panics without an +// `EDGE_KEY` env var, so build the struct directly (mirrors +// backup_uploader_tests.rs's `ctx_pointing_at`). +fn test_context() -> Arc { + Arc::new(Context { + edge_key: EdgeKey { + server_url: String::new(), + agent_id: "agent-1".to_string(), + master_key_b64: String::new(), + }, + api: ApiClient::new(String::new()), + }) +} + +fn write_json(contents: &str) -> NamedTempFile { + let mut file = NamedTempFile::with_suffix(".json").unwrap(); + file.write_all(contents.as_bytes()).unwrap(); + file +} + +#[test] +fn parses_postgresql_cluster_type() { + let file = write_json( + r#"{ + "databases": [ + { + "name": "cluster1", + "type": "postgresql-cluster", + "username": "postgres", + "password": "p", + "port": 5432, + "host": "localhost", + "generated_id": "16678159-ff7e-4c97-8c83-0adeff214681" + } + ] + }"#, + ); + + let service = ConfigService::new(test_context()); + let cfg = service.load(Some(file.path().to_str().unwrap())).unwrap(); + + assert_eq!(cfg.databases[0].db_type.as_str(), "postgresql-cluster"); + // `database` is optional for cluster entries and defaults to "postgres". + assert_eq!(cfg.databases[0].database, "postgres"); +} + +#[test] +fn postgresql_cluster_respects_explicit_database() { + let file = write_json( + r#"{ + "databases": [ + { + "name": "cluster1", + "type": "postgresql-cluster", + "database": "maintenance", + "username": "postgres", + "password": "p", + "port": 5432, + "host": "localhost", + "generated_id": "16678159-ff7e-4c97-8c83-0adeff214681" + } + ] + }"#, + ); + + let service = ConfigService::new(test_context()); + let cfg = service.load(Some(file.path().to_str().unwrap())).unwrap(); + + assert_eq!(cfg.databases[0].database, "maintenance"); +} diff --git a/src/tests/services/mod.rs b/src/tests/services/mod.rs index 1780d61..af4f3eb 100644 --- a/src/tests/services/mod.rs +++ b/src/tests/services/mod.rs @@ -1,2 +1,3 @@ mod api_models_tests; mod backup_uploader_tests; +mod config_tests; From d5309572ede8ee6a2c8bb7c86c078affee2f50ab Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Fri, 26 Jun 2026 22:06:27 +0200 Subject: [PATCH 14/19] feat: pg_dumpall cluster backup and psql restore --- src/domain/postgres/cluster.rs | 151 +++++++++++++++++++++++++++ src/domain/postgres/mod.rs | 1 + src/tests/domain/mod.rs | 1 + src/tests/domain/postgres_cluster.rs | 142 +++++++++++++++++++++++++ 4 files changed, 295 insertions(+) create mode 100644 src/domain/postgres/cluster.rs create mode 100644 src/tests/domain/postgres_cluster.rs diff --git a/src/domain/postgres/cluster.rs b/src/domain/postgres/cluster.rs new file mode 100644 index 0000000..e9a183f --- /dev/null +++ b/src/domain/postgres/cluster.rs @@ -0,0 +1,151 @@ +use anyhow::Result; +use std::collections::HashMap; +use std::path::PathBuf; +use std::process::Command; +use std::sync::Arc; +use std::time::Instant; + +use super::connection::{ + is_superuser, pg_dumpall_binary_name, psql_binary_name, select_pg_path, server_version, +}; +use crate::services::backup::logger::JobLogger; +use crate::services::config::DatabaseConfig; + +/// Backs up an entire PostgreSQL cluster (roles + all databases + ownership + +/// privileges) with `pg_dumpall` into a single `.sql`. Requires a superuser. +pub async fn backup( + cfg: DatabaseConfig, + backup_dir: PathBuf, + env: HashMap, + logger: Arc, +) -> Result { + tokio::task::spawn_blocking(move || -> Result { + logger.log("info", format!("Starting cluster backup for {}", cfg.name)); + + let version = match futures::executor::block_on(server_version(&cfg)) { + Ok(v) => v, + Err(e) => { + logger.log("error", format!("Failed to get server version for {}: {:?}", cfg.name, e)); + return Err(e.into()); + } + }; + + match futures::executor::block_on(is_superuser(&cfg)) { + Ok(true) => {} + Ok(false) => { + logger.log("error", format!("postgresql-cluster backup requires a superuser role for {}", cfg.name)); + anyhow::bail!("postgresql-cluster backup requires a superuser role for {}", cfg.name); + } + Err(e) => { + logger.log("error", format!("Failed to check superuser status for {}: {:?}", cfg.name, e)); + return Err(e.into()); + } + } + + let pg_dumpall = select_pg_path(&version).join(pg_dumpall_binary_name()); + let file_path = backup_dir.join(format!("{}.sql", cfg.generated_id)); + + logger.log("info", format!("Running pg_dumpall for cluster {} via {:?}", cfg.name, pg_dumpall)); + + let start = Instant::now(); + let output = Command::new(&pg_dumpall) + .arg("--host").arg(&cfg.host) + .arg("--port").arg(cfg.port.to_string()) + .arg("--username").arg(&cfg.username) + .arg("-f").arg(&file_path) + .envs(env) + .output(); + let duration_ms = start.elapsed().as_millis() as f64; + + match output { + Ok(o) => { + let stderr = String::from_utf8_lossy(&o.stderr).to_string(); + let exit_code = o.status.code().unwrap_or(-1); + if o.status.success() { + logger.log_command("pg_dumpall", if stderr.is_empty() { None } else { Some(stderr) }, Some(0), Some(duration_ms)); + logger.log("info", format!("Cluster backup completed for {} at {:?}", cfg.name, file_path)); + Ok(file_path) + } else { + logger.log_command("pg_dumpall", Some(stderr), Some(exit_code), Some(duration_ms)); + anyhow::bail!("Cluster backup (pg_dumpall) failed for {}", cfg.name); + } + } + Err(e) => { + logger.log_command("pg_dumpall", Some(e.to_string()), Some(-1), Some(duration_ms)); + Err(e.into()) + } + } + }) + .await? +} + +/// Restores a cluster `.sql` produced by [`backup`] via `psql` against a fresh +/// target cluster. Requires a superuser. psql runs continue-on-error (its +/// default); a non-zero process exit is treated as failure. +pub async fn restore( + cfg: DatabaseConfig, + restore_file: PathBuf, + env: HashMap, + logger: Arc, +) -> Result<()> { + tokio::task::spawn_blocking(move || -> Result<()> { + logger.log("info", format!("Starting cluster restore for {}", cfg.name)); + + let version = match futures::executor::block_on(server_version(&cfg)) { + Ok(v) => v, + Err(e) => { + logger.log("error", format!("Failed to get server version for {}: {:?}", cfg.name, e)); + return Err(e.into()); + } + }; + + match futures::executor::block_on(is_superuser(&cfg)) { + Ok(true) => {} + Ok(false) => { + logger.log("error", format!("postgresql-cluster restore requires a superuser role for {}", cfg.name)); + anyhow::bail!("postgresql-cluster restore requires a superuser role for {}", cfg.name); + } + Err(e) => { + logger.log("error", format!("Failed to check superuser status for {}: {:?}", cfg.name, e)); + return Err(e.into()); + } + } + + let psql = select_pg_path(&version).join(psql_binary_name()); + + logger.log("info", format!("Replaying cluster dump for {} via {:?}", cfg.name, psql)); + + let start = Instant::now(); + let output = Command::new(&psql) + .arg("--host").arg(&cfg.host) + .arg("--port").arg(cfg.port.to_string()) + .arg("--username").arg(&cfg.username) + .arg("--dbname").arg("postgres") + .arg("-f").arg(&restore_file) + .envs(env) + .output(); + let duration_ms = start.elapsed().as_millis() as f64; + + match output { + Ok(o) => { + let stderr = String::from_utf8_lossy(&o.stderr).to_string(); + let stdout = String::from_utf8_lossy(&o.stdout).to_string(); + let combined = format!("{}{}", stdout, stderr); + let exit_code = o.status.code().unwrap_or(-1); + if o.status.success() { + logger.log_command("psql", if combined.is_empty() { None } else { Some(combined) }, Some(0), Some(duration_ms)); + logger.log("info", format!("Cluster restore completed for {}", cfg.name)); + Ok(()) + } else { + logger.log_command("psql", if combined.is_empty() { None } else { Some(combined) }, Some(exit_code), Some(duration_ms)); + anyhow::bail!("Cluster restore (psql) failed for {}", cfg.name); + } + } + Err(e) => { + logger.log_command("psql", Some(e.to_string()), Some(-1), Some(duration_ms)); + Err(e.into()) + } + } + }) + .await? +} diff --git a/src/domain/postgres/mod.rs b/src/domain/postgres/mod.rs index c7cd4cb..0f8aee3 100644 --- a/src/domain/postgres/mod.rs +++ b/src/domain/postgres/mod.rs @@ -1,4 +1,5 @@ pub mod backup; +pub(crate) mod cluster; pub(crate) mod connection; pub mod database; mod format; diff --git a/src/tests/domain/mod.rs b/src/tests/domain/mod.rs index 641e097..a3d9561 100644 --- a/src/tests/domain/mod.rs +++ b/src/tests/domain/mod.rs @@ -2,6 +2,7 @@ mod mariadb; mod mongodb; mod mysql; mod postgres; +mod postgres_cluster; mod redis; mod valkey; mod firebird; diff --git a/src/tests/domain/postgres_cluster.rs b/src/tests/domain/postgres_cluster.rs new file mode 100644 index 0000000..ed2e155 --- /dev/null +++ b/src/tests/domain/postgres_cluster.rs @@ -0,0 +1,142 @@ +use crate::domain::postgres::{cluster, connection}; +use crate::services::backup::logger::JobLogger; +use crate::services::config::{DatabaseConfig, DbType}; +use crate::tests::init_tracing_for_test; +use std::collections::HashMap; +use std::sync::Arc; +use tempfile::TempDir; +use testcontainers::runners::AsyncRunner; +use testcontainers::{ContainerAsync, ImageExt}; +use testcontainers_modules::postgres::Postgres; +use url::Host; + +async fn start_cluster(user: &str) -> (ContainerAsync, DatabaseConfig) { + let container = Postgres::default() + .with_env_var("POSTGRES_DB", "postgres") + .with_env_var("POSTGRES_USER", user) + .with_env_var("POSTGRES_PASSWORD", "changeme") + .with_tag("17") + .start() + .await + .unwrap(); + + let host = container + .get_host() + .await + .unwrap_or(Host::parse("127.0.0.1").unwrap()); + let port = container.get_host_port_ipv4(5432).await.unwrap_or(5432); + + let config = DatabaseConfig { + name: format!("cluster-{}", user), + database: "postgres".to_string(), + db_type: DbType::PostgresqlCluster, + username: user.to_string(), + password: "changeme".to_string(), + port, + host: host.to_string(), + generated_id: "40875631-e3d2-4dfe-a26b-2a347ecc64fd".to_string(), + path: "".to_string(), + max_packet_size: "".to_string(), + }; + (container, config) +} + +fn env_for(cfg: &DatabaseConfig) -> HashMap { + let mut env = std::env::vars().collect::>(); + env.insert("PGPASSWORD".to_string(), cfg.password.clone()); + env +} + +#[tokio::test] +async fn cluster_backup_produces_sql_with_roles_and_databases() { + init_tracing_for_test(); + let (_c, cfg) = start_cluster("testuser").await; + + let dir = TempDir::new().unwrap(); + let logger = Arc::new(JobLogger::new()); + let sql = cluster::backup(cfg.clone(), dir.path().to_path_buf(), env_for(&cfg), logger) + .await + .unwrap(); + + assert!(sql.is_file()); + let contents = std::fs::read_to_string(&sql).unwrap(); + assert!(contents.contains("CREATE ROLE"), "expected CREATE ROLE in dump"); + assert!(contents.contains("CREATE DATABASE") || contents.contains("\\connect"), + "expected database statements in dump"); +} + +#[tokio::test] +async fn cluster_backup_requires_superuser() { + init_tracing_for_test(); + let (_c, super_cfg) = start_cluster("testuser").await; + + // Create a NON-superuser login role on the cluster. + let client = connection::connect(&super_cfg).await.unwrap(); + client + .batch_execute("CREATE ROLE appuser LOGIN PASSWORD 'changeme' NOSUPERUSER;") + .await + .unwrap(); + + let mut weak = super_cfg.clone(); + weak.username = "appuser".to_string(); + + let dir = TempDir::new().unwrap(); + let logger = Arc::new(JobLogger::new()); + let err = cluster::backup(weak.clone(), dir.path().to_path_buf(), env_for(&weak), logger) + .await + .unwrap_err(); + + assert!( + err.to_string().contains("superuser"), + "expected a superuser error, got: {err}" + ); +} + +#[tokio::test] +async fn cluster_backup_restore_round_trip() { + init_tracing_for_test(); + + // Source cluster A: seed a role + a table owned by that role. + let (_a, src) = start_cluster("testuser").await; + let client = connection::connect(&src).await.unwrap(); + client + .batch_execute( + "CREATE ROLE appowner LOGIN PASSWORD 'changeme' NOSUPERUSER;\n\ + CREATE TABLE owned_tbl (id int);\n\ + ALTER TABLE owned_tbl OWNER TO appowner;", + ) + .await + .unwrap(); + + let dir = TempDir::new().unwrap(); + let sql = cluster::backup(src.clone(), dir.path().to_path_buf(), env_for(&src), Arc::new(JobLogger::new())) + .await + .unwrap(); + + // Target cluster B: fresh, same bootstrap user. + let (_b, mut dst) = start_cluster("testuser").await; + + cluster::restore(dst.clone(), sql.clone(), env_for(&dst), Arc::new(JobLogger::new())) + .await + .unwrap(); + + // Verify the seeded role exists and the table's owner was preserved on B. + dst.database = "postgres".to_string(); + let bclient = connection::connect(&dst).await.unwrap(); + let role_exists: bool = bclient + .query_one("SELECT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'appowner');", &[]) + .await + .unwrap() + .get(0); + assert!(role_exists, "appowner role must be recreated on the target"); + + let owner: String = bclient + .query_one( + "SELECT tableowner FROM pg_tables WHERE tablename = 'owned_tbl';", + &[], + ) + .await + .unwrap() + .get(0); + assert_eq!(owner, "appowner", "table ownership must be preserved"); +} From d488f8fc696e6ff972e2c4903141cab0650c2abb Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Fri, 26 Jun 2026 22:08:46 +0200 Subject: [PATCH 15/19] feat: route postgresql-cluster through PostgresClusterDatabase --- src/domain/factory.rs | 5 ++- src/domain/postgres/cluster_database.rs | 52 +++++++++++++++++++++++++ src/domain/postgres/mod.rs | 1 + 3 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 src/domain/postgres/cluster_database.rs diff --git a/src/domain/factory.rs b/src/domain/factory.rs index 901f410..8427a9f 100644 --- a/src/domain/factory.rs +++ b/src/domain/factory.rs @@ -1,5 +1,6 @@ use crate::domain::mongodb::database::MongoDatabase; use crate::domain::mysql::database::MySQLDatabase; +use crate::domain::postgres::cluster_database::PostgresClusterDatabase; use crate::domain::postgres::database::PostgresDatabase; use crate::domain::postgres::{detect_format_from_file, detect_format_from_size}; use crate::domain::redis::database::RedisDatabase; @@ -31,7 +32,7 @@ impl DatabaseFactory { let format = detect_format_from_size(&cfg).await; Arc::new(PostgresDatabase::new(cfg, format)) } - DbType::PostgresqlCluster => unimplemented!("postgresql-cluster wired in Task 5"), + DbType::PostgresqlCluster => Arc::new(PostgresClusterDatabase::new(cfg)), DbType::Mysql => Arc::new(MySQLDatabase::new(cfg)), DbType::Mariadb => Arc::new(MariaDBDatabase::new(cfg)), DbType::MongoDB => Arc::new(MongoDatabase::new(cfg)), @@ -49,7 +50,7 @@ impl DatabaseFactory { let format = detect_format_from_file(restore_file); Arc::new(PostgresDatabase::new(cfg, format)) } - DbType::PostgresqlCluster => unimplemented!("postgresql-cluster wired in Task 5"), + DbType::PostgresqlCluster => Arc::new(PostgresClusterDatabase::new(cfg)), DbType::Mysql => Arc::new(MySQLDatabase::new(cfg)), DbType::Mariadb => Arc::new(MariaDBDatabase::new(cfg)), DbType::MongoDB => Arc::new(MongoDatabase::new(cfg)), diff --git a/src/domain/postgres/cluster_database.rs b/src/domain/postgres/cluster_database.rs new file mode 100644 index 0000000..f120259 --- /dev/null +++ b/src/domain/postgres/cluster_database.rs @@ -0,0 +1,52 @@ +use anyhow::Result; +use async_trait::async_trait; +use std::collections::HashMap; +use std::path::{Path, PathBuf}; +use std::sync::Arc; + +use super::{cluster, ping}; +use crate::domain::factory::Database; +use crate::services::backup::logger::JobLogger; +use crate::services::config::DatabaseConfig; +use crate::utils::locks::{DbOpLock, FileLock}; + +pub struct PostgresClusterDatabase { + pub cfg: DatabaseConfig, +} + +impl PostgresClusterDatabase { + pub fn new(cfg: DatabaseConfig) -> Self { + Self { cfg } + } + + fn build_env(&self) -> HashMap { + let mut envs = std::env::vars().collect::>(); + envs.insert("PGPASSWORD".to_string(), self.cfg.password.to_string()); + envs + } +} + +#[async_trait] +impl Database for PostgresClusterDatabase { + fn file_extension(&self) -> &'static str { + ".sql" + } + + async fn ping(&self) -> Result { + ping::run(self.cfg.clone()).await + } + + async fn backup(&self, dir: &Path, logger: Arc) -> Result { + FileLock::acquire(&self.cfg.generated_id, DbOpLock::Backup.as_str()).await?; + let res = cluster::backup(self.cfg.clone(), dir.to_path_buf(), self.build_env(), logger).await; + FileLock::release(&self.cfg.generated_id).await?; + res + } + + async fn restore(&self, file: &Path, logger: Arc) -> Result<()> { + FileLock::acquire(&self.cfg.generated_id, DbOpLock::Restore.as_str()).await?; + let res = cluster::restore(self.cfg.clone(), file.to_path_buf(), self.build_env(), logger).await; + FileLock::release(&self.cfg.generated_id).await?; + res + } +} diff --git a/src/domain/postgres/mod.rs b/src/domain/postgres/mod.rs index 0f8aee3..4c7951f 100644 --- a/src/domain/postgres/mod.rs +++ b/src/domain/postgres/mod.rs @@ -2,6 +2,7 @@ pub mod backup; pub(crate) mod cluster; pub(crate) mod connection; pub mod database; +pub mod cluster_database; mod format; mod ping; mod restore; From c7bdfe6bc75c4d5baf2435404d881cb18c63e79e Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Fri, 26 Jun 2026 22:10:12 +0200 Subject: [PATCH 16/19] docs: add postgresql-cluster sample to databases.json Co-Authored-By: Claude Opus 4.8 --- databases.json | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/databases.json b/databases.json index 70eface..4f52f29 100644 --- a/databases.json +++ b/databases.json @@ -111,6 +111,15 @@ "port": 1433, "host": "db-mssql", "generated_id": "16706125-ff7e-4c97-8c83-0adeff214682" + }, + { + "name": "Test database - PostgreSQL cluster", + "type": "postgresql-cluster", + "username": "postgres", + "password": "changeme", + "port": 5432, + "host": "db-postgres", + "generated_id": "16678199-ff7e-4c97-8c83-0adeff214681" } ] } From dae2e45e28c66b32de321acb55d9579460cf73f7 Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Fri, 26 Jun 2026 22:19:31 +0200 Subject: [PATCH 17/19] refactor: split cluster mode into cluster/ module (backup, restore, database) Move the single cluster.rs + cluster_database.rs into a cluster/ folder: - cluster/backup.rs (run = pg_dumpall) - cluster/restore.rs (run = psql replay) - cluster/database.rs (PostgresClusterDatabase trait impl) - cluster/mod.rs Free fns renamed to run() to match postgres/{backup,restore}.rs convention. No behavior change; cluster tests pass. Co-Authored-By: Claude Opus 4.8 --- src/domain/factory.rs | 2 +- src/domain/postgres/cluster.rs | 151 ------------------ src/domain/postgres/cluster/backup.rs | 80 ++++++++++ .../database.rs} | 7 +- src/domain/postgres/cluster/mod.rs | 3 + src/domain/postgres/cluster/restore.rs | 81 ++++++++++ src/domain/postgres/mod.rs | 1 - src/tests/domain/postgres_cluster.rs | 8 +- 8 files changed, 173 insertions(+), 160 deletions(-) delete mode 100644 src/domain/postgres/cluster.rs create mode 100644 src/domain/postgres/cluster/backup.rs rename src/domain/postgres/{cluster_database.rs => cluster/database.rs} (84%) create mode 100644 src/domain/postgres/cluster/mod.rs create mode 100644 src/domain/postgres/cluster/restore.rs diff --git a/src/domain/factory.rs b/src/domain/factory.rs index 8427a9f..a2f107b 100644 --- a/src/domain/factory.rs +++ b/src/domain/factory.rs @@ -1,6 +1,6 @@ use crate::domain::mongodb::database::MongoDatabase; use crate::domain::mysql::database::MySQLDatabase; -use crate::domain::postgres::cluster_database::PostgresClusterDatabase; +use crate::domain::postgres::cluster::database::PostgresClusterDatabase; use crate::domain::postgres::database::PostgresDatabase; use crate::domain::postgres::{detect_format_from_file, detect_format_from_size}; use crate::domain::redis::database::RedisDatabase; diff --git a/src/domain/postgres/cluster.rs b/src/domain/postgres/cluster.rs deleted file mode 100644 index e9a183f..0000000 --- a/src/domain/postgres/cluster.rs +++ /dev/null @@ -1,151 +0,0 @@ -use anyhow::Result; -use std::collections::HashMap; -use std::path::PathBuf; -use std::process::Command; -use std::sync::Arc; -use std::time::Instant; - -use super::connection::{ - is_superuser, pg_dumpall_binary_name, psql_binary_name, select_pg_path, server_version, -}; -use crate::services::backup::logger::JobLogger; -use crate::services::config::DatabaseConfig; - -/// Backs up an entire PostgreSQL cluster (roles + all databases + ownership + -/// privileges) with `pg_dumpall` into a single `.sql`. Requires a superuser. -pub async fn backup( - cfg: DatabaseConfig, - backup_dir: PathBuf, - env: HashMap, - logger: Arc, -) -> Result { - tokio::task::spawn_blocking(move || -> Result { - logger.log("info", format!("Starting cluster backup for {}", cfg.name)); - - let version = match futures::executor::block_on(server_version(&cfg)) { - Ok(v) => v, - Err(e) => { - logger.log("error", format!("Failed to get server version for {}: {:?}", cfg.name, e)); - return Err(e.into()); - } - }; - - match futures::executor::block_on(is_superuser(&cfg)) { - Ok(true) => {} - Ok(false) => { - logger.log("error", format!("postgresql-cluster backup requires a superuser role for {}", cfg.name)); - anyhow::bail!("postgresql-cluster backup requires a superuser role for {}", cfg.name); - } - Err(e) => { - logger.log("error", format!("Failed to check superuser status for {}: {:?}", cfg.name, e)); - return Err(e.into()); - } - } - - let pg_dumpall = select_pg_path(&version).join(pg_dumpall_binary_name()); - let file_path = backup_dir.join(format!("{}.sql", cfg.generated_id)); - - logger.log("info", format!("Running pg_dumpall for cluster {} via {:?}", cfg.name, pg_dumpall)); - - let start = Instant::now(); - let output = Command::new(&pg_dumpall) - .arg("--host").arg(&cfg.host) - .arg("--port").arg(cfg.port.to_string()) - .arg("--username").arg(&cfg.username) - .arg("-f").arg(&file_path) - .envs(env) - .output(); - let duration_ms = start.elapsed().as_millis() as f64; - - match output { - Ok(o) => { - let stderr = String::from_utf8_lossy(&o.stderr).to_string(); - let exit_code = o.status.code().unwrap_or(-1); - if o.status.success() { - logger.log_command("pg_dumpall", if stderr.is_empty() { None } else { Some(stderr) }, Some(0), Some(duration_ms)); - logger.log("info", format!("Cluster backup completed for {} at {:?}", cfg.name, file_path)); - Ok(file_path) - } else { - logger.log_command("pg_dumpall", Some(stderr), Some(exit_code), Some(duration_ms)); - anyhow::bail!("Cluster backup (pg_dumpall) failed for {}", cfg.name); - } - } - Err(e) => { - logger.log_command("pg_dumpall", Some(e.to_string()), Some(-1), Some(duration_ms)); - Err(e.into()) - } - } - }) - .await? -} - -/// Restores a cluster `.sql` produced by [`backup`] via `psql` against a fresh -/// target cluster. Requires a superuser. psql runs continue-on-error (its -/// default); a non-zero process exit is treated as failure. -pub async fn restore( - cfg: DatabaseConfig, - restore_file: PathBuf, - env: HashMap, - logger: Arc, -) -> Result<()> { - tokio::task::spawn_blocking(move || -> Result<()> { - logger.log("info", format!("Starting cluster restore for {}", cfg.name)); - - let version = match futures::executor::block_on(server_version(&cfg)) { - Ok(v) => v, - Err(e) => { - logger.log("error", format!("Failed to get server version for {}: {:?}", cfg.name, e)); - return Err(e.into()); - } - }; - - match futures::executor::block_on(is_superuser(&cfg)) { - Ok(true) => {} - Ok(false) => { - logger.log("error", format!("postgresql-cluster restore requires a superuser role for {}", cfg.name)); - anyhow::bail!("postgresql-cluster restore requires a superuser role for {}", cfg.name); - } - Err(e) => { - logger.log("error", format!("Failed to check superuser status for {}: {:?}", cfg.name, e)); - return Err(e.into()); - } - } - - let psql = select_pg_path(&version).join(psql_binary_name()); - - logger.log("info", format!("Replaying cluster dump for {} via {:?}", cfg.name, psql)); - - let start = Instant::now(); - let output = Command::new(&psql) - .arg("--host").arg(&cfg.host) - .arg("--port").arg(cfg.port.to_string()) - .arg("--username").arg(&cfg.username) - .arg("--dbname").arg("postgres") - .arg("-f").arg(&restore_file) - .envs(env) - .output(); - let duration_ms = start.elapsed().as_millis() as f64; - - match output { - Ok(o) => { - let stderr = String::from_utf8_lossy(&o.stderr).to_string(); - let stdout = String::from_utf8_lossy(&o.stdout).to_string(); - let combined = format!("{}{}", stdout, stderr); - let exit_code = o.status.code().unwrap_or(-1); - if o.status.success() { - logger.log_command("psql", if combined.is_empty() { None } else { Some(combined) }, Some(0), Some(duration_ms)); - logger.log("info", format!("Cluster restore completed for {}", cfg.name)); - Ok(()) - } else { - logger.log_command("psql", if combined.is_empty() { None } else { Some(combined) }, Some(exit_code), Some(duration_ms)); - anyhow::bail!("Cluster restore (psql) failed for {}", cfg.name); - } - } - Err(e) => { - logger.log_command("psql", Some(e.to_string()), Some(-1), Some(duration_ms)); - Err(e.into()) - } - } - }) - .await? -} diff --git a/src/domain/postgres/cluster/backup.rs b/src/domain/postgres/cluster/backup.rs new file mode 100644 index 0000000..dac6fa2 --- /dev/null +++ b/src/domain/postgres/cluster/backup.rs @@ -0,0 +1,80 @@ +use anyhow::Result; +use std::collections::HashMap; +use std::path::PathBuf; +use std::process::Command; +use std::sync::Arc; +use std::time::Instant; + +use super::super::connection::{ + is_superuser, pg_dumpall_binary_name, select_pg_path, server_version, +}; +use crate::services::backup::logger::JobLogger; +use crate::services::config::DatabaseConfig; + +/// Backs up an entire PostgreSQL cluster (roles + all databases + ownership + +/// privileges) with `pg_dumpall` into a single `.sql`. Requires a superuser. +pub async fn run( + cfg: DatabaseConfig, + backup_dir: PathBuf, + env: HashMap, + logger: Arc, +) -> Result { + tokio::task::spawn_blocking(move || -> Result { + logger.log("info", format!("Starting cluster backup for {}", cfg.name)); + + let version = match futures::executor::block_on(server_version(&cfg)) { + Ok(v) => v, + Err(e) => { + logger.log("error", format!("Failed to get server version for {}: {:?}", cfg.name, e)); + return Err(e.into()); + } + }; + + match futures::executor::block_on(is_superuser(&cfg)) { + Ok(true) => {} + Ok(false) => { + logger.log("error", format!("postgresql-cluster backup requires a superuser role for {}", cfg.name)); + anyhow::bail!("postgresql-cluster backup requires a superuser role for {}", cfg.name); + } + Err(e) => { + logger.log("error", format!("Failed to check superuser status for {}: {:?}", cfg.name, e)); + return Err(e.into()); + } + } + + let pg_dumpall = select_pg_path(&version).join(pg_dumpall_binary_name()); + let file_path = backup_dir.join(format!("{}.sql", cfg.generated_id)); + + logger.log("info", format!("Running pg_dumpall for cluster {} via {:?}", cfg.name, pg_dumpall)); + + let start = Instant::now(); + let output = Command::new(&pg_dumpall) + .arg("--host").arg(&cfg.host) + .arg("--port").arg(cfg.port.to_string()) + .arg("--username").arg(&cfg.username) + .arg("-f").arg(&file_path) + .envs(env) + .output(); + let duration_ms = start.elapsed().as_millis() as f64; + + match output { + Ok(o) => { + let stderr = String::from_utf8_lossy(&o.stderr).to_string(); + let exit_code = o.status.code().unwrap_or(-1); + if o.status.success() { + logger.log_command("pg_dumpall", if stderr.is_empty() { None } else { Some(stderr) }, Some(0), Some(duration_ms)); + logger.log("info", format!("Cluster backup completed for {} at {:?}", cfg.name, file_path)); + Ok(file_path) + } else { + logger.log_command("pg_dumpall", Some(stderr), Some(exit_code), Some(duration_ms)); + anyhow::bail!("Cluster backup (pg_dumpall) failed for {}", cfg.name); + } + } + Err(e) => { + logger.log_command("pg_dumpall", Some(e.to_string()), Some(-1), Some(duration_ms)); + Err(e.into()) + } + } + }) + .await? +} diff --git a/src/domain/postgres/cluster_database.rs b/src/domain/postgres/cluster/database.rs similarity index 84% rename from src/domain/postgres/cluster_database.rs rename to src/domain/postgres/cluster/database.rs index f120259..fd677c4 100644 --- a/src/domain/postgres/cluster_database.rs +++ b/src/domain/postgres/cluster/database.rs @@ -4,7 +4,8 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; use std::sync::Arc; -use super::{cluster, ping}; +use super::super::ping; +use super::{backup, restore}; use crate::domain::factory::Database; use crate::services::backup::logger::JobLogger; use crate::services::config::DatabaseConfig; @@ -38,14 +39,14 @@ impl Database for PostgresClusterDatabase { async fn backup(&self, dir: &Path, logger: Arc) -> Result { FileLock::acquire(&self.cfg.generated_id, DbOpLock::Backup.as_str()).await?; - let res = cluster::backup(self.cfg.clone(), dir.to_path_buf(), self.build_env(), logger).await; + let res = backup::run(self.cfg.clone(), dir.to_path_buf(), self.build_env(), logger).await; FileLock::release(&self.cfg.generated_id).await?; res } async fn restore(&self, file: &Path, logger: Arc) -> Result<()> { FileLock::acquire(&self.cfg.generated_id, DbOpLock::Restore.as_str()).await?; - let res = cluster::restore(self.cfg.clone(), file.to_path_buf(), self.build_env(), logger).await; + let res = restore::run(self.cfg.clone(), file.to_path_buf(), self.build_env(), logger).await; FileLock::release(&self.cfg.generated_id).await?; res } diff --git a/src/domain/postgres/cluster/mod.rs b/src/domain/postgres/cluster/mod.rs new file mode 100644 index 0000000..4e99356 --- /dev/null +++ b/src/domain/postgres/cluster/mod.rs @@ -0,0 +1,3 @@ +pub mod backup; +pub mod database; +pub mod restore; diff --git a/src/domain/postgres/cluster/restore.rs b/src/domain/postgres/cluster/restore.rs new file mode 100644 index 0000000..d47fc2f --- /dev/null +++ b/src/domain/postgres/cluster/restore.rs @@ -0,0 +1,81 @@ +use anyhow::Result; +use std::collections::HashMap; +use std::path::PathBuf; +use std::process::Command; +use std::sync::Arc; +use std::time::Instant; + +use super::super::connection::{is_superuser, psql_binary_name, select_pg_path, server_version}; +use crate::services::backup::logger::JobLogger; +use crate::services::config::DatabaseConfig; + +/// Restores a cluster `.sql` produced by the cluster backup via `psql` against +/// a fresh target cluster. Requires a superuser. psql runs continue-on-error +/// (its default); a non-zero process exit is treated as failure. +pub async fn run( + cfg: DatabaseConfig, + restore_file: PathBuf, + env: HashMap, + logger: Arc, +) -> Result<()> { + tokio::task::spawn_blocking(move || -> Result<()> { + logger.log("info", format!("Starting cluster restore for {}", cfg.name)); + + let version = match futures::executor::block_on(server_version(&cfg)) { + Ok(v) => v, + Err(e) => { + logger.log("error", format!("Failed to get server version for {}: {:?}", cfg.name, e)); + return Err(e.into()); + } + }; + + match futures::executor::block_on(is_superuser(&cfg)) { + Ok(true) => {} + Ok(false) => { + logger.log("error", format!("postgresql-cluster restore requires a superuser role for {}", cfg.name)); + anyhow::bail!("postgresql-cluster restore requires a superuser role for {}", cfg.name); + } + Err(e) => { + logger.log("error", format!("Failed to check superuser status for {}: {:?}", cfg.name, e)); + return Err(e.into()); + } + } + + let psql = select_pg_path(&version).join(psql_binary_name()); + + logger.log("info", format!("Replaying cluster dump for {} via {:?}", cfg.name, psql)); + + let start = Instant::now(); + let output = Command::new(&psql) + .arg("--host").arg(&cfg.host) + .arg("--port").arg(cfg.port.to_string()) + .arg("--username").arg(&cfg.username) + .arg("--dbname").arg("postgres") + .arg("-f").arg(&restore_file) + .envs(env) + .output(); + let duration_ms = start.elapsed().as_millis() as f64; + + match output { + Ok(o) => { + let stderr = String::from_utf8_lossy(&o.stderr).to_string(); + let stdout = String::from_utf8_lossy(&o.stdout).to_string(); + let combined = format!("{}{}", stdout, stderr); + let exit_code = o.status.code().unwrap_or(-1); + if o.status.success() { + logger.log_command("psql", if combined.is_empty() { None } else { Some(combined) }, Some(0), Some(duration_ms)); + logger.log("info", format!("Cluster restore completed for {}", cfg.name)); + Ok(()) + } else { + logger.log_command("psql", if combined.is_empty() { None } else { Some(combined) }, Some(exit_code), Some(duration_ms)); + anyhow::bail!("Cluster restore (psql) failed for {}", cfg.name); + } + } + Err(e) => { + logger.log_command("psql", Some(e.to_string()), Some(-1), Some(duration_ms)); + Err(e.into()) + } + } + }) + .await? +} diff --git a/src/domain/postgres/mod.rs b/src/domain/postgres/mod.rs index 4c7951f..0f8aee3 100644 --- a/src/domain/postgres/mod.rs +++ b/src/domain/postgres/mod.rs @@ -2,7 +2,6 @@ pub mod backup; pub(crate) mod cluster; pub(crate) mod connection; pub mod database; -pub mod cluster_database; mod format; mod ping; mod restore; diff --git a/src/tests/domain/postgres_cluster.rs b/src/tests/domain/postgres_cluster.rs index ed2e155..370855a 100644 --- a/src/tests/domain/postgres_cluster.rs +++ b/src/tests/domain/postgres_cluster.rs @@ -54,7 +54,7 @@ async fn cluster_backup_produces_sql_with_roles_and_databases() { let dir = TempDir::new().unwrap(); let logger = Arc::new(JobLogger::new()); - let sql = cluster::backup(cfg.clone(), dir.path().to_path_buf(), env_for(&cfg), logger) + let sql = cluster::backup::run(cfg.clone(), dir.path().to_path_buf(), env_for(&cfg), logger) .await .unwrap(); @@ -82,7 +82,7 @@ async fn cluster_backup_requires_superuser() { let dir = TempDir::new().unwrap(); let logger = Arc::new(JobLogger::new()); - let err = cluster::backup(weak.clone(), dir.path().to_path_buf(), env_for(&weak), logger) + let err = cluster::backup::run(weak.clone(), dir.path().to_path_buf(), env_for(&weak), logger) .await .unwrap_err(); @@ -109,14 +109,14 @@ async fn cluster_backup_restore_round_trip() { .unwrap(); let dir = TempDir::new().unwrap(); - let sql = cluster::backup(src.clone(), dir.path().to_path_buf(), env_for(&src), Arc::new(JobLogger::new())) + let sql = cluster::backup::run(src.clone(), dir.path().to_path_buf(), env_for(&src), Arc::new(JobLogger::new())) .await .unwrap(); // Target cluster B: fresh, same bootstrap user. let (_b, mut dst) = start_cluster("testuser").await; - cluster::restore(dst.clone(), sql.clone(), env_for(&dst), Arc::new(JobLogger::new())) + cluster::restore::run(dst.clone(), sql.clone(), env_for(&dst), Arc::new(JobLogger::new())) .await .unwrap(); From 0c6e3e3dbe103dc661f0b848e883b49f87fd212d Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Fri, 26 Jun 2026 22:48:19 +0200 Subject: [PATCH 18/19] test: mirror cluster tests into src/tests/domain/cluster/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split the flat postgres_cluster.rs into a cluster/ test module matching the src/domain/postgres/cluster/ layout: - cluster/backup.rs (produces_sql, requires_superuser) - cluster/restore.rs (round_trip_preserves_ownership, requires_superuser — new) - cluster/database.rs (factory routing -> .sql, no container) - cluster/mod.rs (shared start_cluster/env_for helpers) Adds restore-side superuser pre-check coverage and fast no-container factory routing tests. Co-Authored-By: Claude Opus 4.8 --- src/tests/domain/cluster/backup.rs | 53 ++++++++++ src/tests/domain/cluster/database.rs | 32 ++++++ src/tests/domain/cluster/mod.rs | 50 ++++++++++ src/tests/domain/cluster/restore.rs | 83 ++++++++++++++++ src/tests/domain/mod.rs | 2 +- src/tests/domain/postgres_cluster.rs | 142 --------------------------- 6 files changed, 219 insertions(+), 143 deletions(-) create mode 100644 src/tests/domain/cluster/backup.rs create mode 100644 src/tests/domain/cluster/database.rs create mode 100644 src/tests/domain/cluster/mod.rs create mode 100644 src/tests/domain/cluster/restore.rs delete mode 100644 src/tests/domain/postgres_cluster.rs diff --git a/src/tests/domain/cluster/backup.rs b/src/tests/domain/cluster/backup.rs new file mode 100644 index 0000000..32adebd --- /dev/null +++ b/src/tests/domain/cluster/backup.rs @@ -0,0 +1,53 @@ +use super::{env_for, start_cluster}; +use crate::domain::postgres::{cluster, connection}; +use crate::services::backup::logger::JobLogger; +use crate::tests::init_tracing_for_test; +use std::sync::Arc; +use tempfile::TempDir; + +#[tokio::test] +async fn produces_sql_with_roles_and_databases() { + init_tracing_for_test(); + let (_c, cfg) = start_cluster("testuser").await; + + let dir = TempDir::new().unwrap(); + let logger = Arc::new(JobLogger::new()); + let sql = cluster::backup::run(cfg.clone(), dir.path().to_path_buf(), env_for(&cfg), logger) + .await + .unwrap(); + + assert!(sql.is_file()); + let contents = std::fs::read_to_string(&sql).unwrap(); + assert!(contents.contains("CREATE ROLE"), "expected CREATE ROLE in dump"); + assert!( + contents.contains("CREATE DATABASE") || contents.contains("\\connect"), + "expected database statements in dump" + ); +} + +#[tokio::test] +async fn requires_superuser() { + init_tracing_for_test(); + let (_c, super_cfg) = start_cluster("testuser").await; + + // Create a NON-superuser login role on the cluster. + let client = connection::connect(&super_cfg).await.unwrap(); + client + .batch_execute("CREATE ROLE appuser LOGIN PASSWORD 'changeme' NOSUPERUSER;") + .await + .unwrap(); + + let mut weak = super_cfg.clone(); + weak.username = "appuser".to_string(); + + let dir = TempDir::new().unwrap(); + let logger = Arc::new(JobLogger::new()); + let err = cluster::backup::run(weak.clone(), dir.path().to_path_buf(), env_for(&weak), logger) + .await + .unwrap_err(); + + assert!( + err.to_string().contains("superuser"), + "expected a superuser error, got: {err}" + ); +} diff --git a/src/tests/domain/cluster/database.rs b/src/tests/domain/cluster/database.rs new file mode 100644 index 0000000..a29c285 --- /dev/null +++ b/src/tests/domain/cluster/database.rs @@ -0,0 +1,32 @@ +use crate::domain::factory::DatabaseFactory; +use crate::services::config::{DatabaseConfig, DbType}; +use std::path::Path; + +// A `postgresql-cluster` config. The factory constructs `PostgresClusterDatabase` +// without opening a connection, so these tests need no container. +fn cluster_config() -> DatabaseConfig { + DatabaseConfig { + name: "cluster".to_string(), + database: "postgres".to_string(), + db_type: DbType::PostgresqlCluster, + username: "postgres".to_string(), + password: "changeme".to_string(), + port: 5432, + host: "localhost".to_string(), + generated_id: "40875631-e3d2-4dfe-a26b-2a347ecc64fd".to_string(), + path: String::new(), + max_packet_size: String::new(), + } +} + +#[tokio::test] +async fn factory_routes_cluster_for_backup_with_sql_extension() { + let db = DatabaseFactory::create_for_backup(cluster_config()).await; + assert_eq!(db.file_extension(), ".sql"); +} + +#[tokio::test] +async fn factory_routes_cluster_for_restore_with_sql_extension() { + let db = DatabaseFactory::create_for_restore(cluster_config(), Path::new("dump.sql")).await; + assert_eq!(db.file_extension(), ".sql"); +} diff --git a/src/tests/domain/cluster/mod.rs b/src/tests/domain/cluster/mod.rs new file mode 100644 index 0000000..3288f87 --- /dev/null +++ b/src/tests/domain/cluster/mod.rs @@ -0,0 +1,50 @@ +mod backup; +mod database; +mod restore; + +use crate::services::config::{DatabaseConfig, DbType}; +use std::collections::HashMap; +use testcontainers::runners::AsyncRunner; +use testcontainers::{ContainerAsync, ImageExt}; +use testcontainers_modules::postgres::Postgres; +use url::Host; + +/// Starts a fresh Postgres 17 cluster whose bootstrap superuser is `user`, and +/// returns the container guard plus a `postgresql-cluster` config pointing at it. +/// Shared by the `backup` and `restore` integration tests. +async fn start_cluster(user: &str) -> (ContainerAsync, DatabaseConfig) { + let container = Postgres::default() + .with_env_var("POSTGRES_DB", "postgres") + .with_env_var("POSTGRES_USER", user) + .with_env_var("POSTGRES_PASSWORD", "changeme") + .with_tag("17") + .start() + .await + .unwrap(); + + let host = container + .get_host() + .await + .unwrap_or(Host::parse("127.0.0.1").unwrap()); + let port = container.get_host_port_ipv4(5432).await.unwrap_or(5432); + + let config = DatabaseConfig { + name: format!("cluster-{}", user), + database: "postgres".to_string(), + db_type: DbType::PostgresqlCluster, + username: user.to_string(), + password: "changeme".to_string(), + port, + host: host.to_string(), + generated_id: "40875631-e3d2-4dfe-a26b-2a347ecc64fd".to_string(), + path: "".to_string(), + max_packet_size: "".to_string(), + }; + (container, config) +} + +fn env_for(cfg: &DatabaseConfig) -> HashMap { + let mut env = std::env::vars().collect::>(); + env.insert("PGPASSWORD".to_string(), cfg.password.clone()); + env +} diff --git a/src/tests/domain/cluster/restore.rs b/src/tests/domain/cluster/restore.rs new file mode 100644 index 0000000..1bcd46c --- /dev/null +++ b/src/tests/domain/cluster/restore.rs @@ -0,0 +1,83 @@ +use super::{env_for, start_cluster}; +use crate::domain::postgres::{cluster, connection}; +use crate::services::backup::logger::JobLogger; +use crate::tests::init_tracing_for_test; +use std::sync::Arc; +use tempfile::TempDir; + +#[tokio::test] +async fn backup_restore_round_trip_preserves_ownership() { + init_tracing_for_test(); + + // Source cluster A: seed a role + a table owned by that role. + let (_a, src) = start_cluster("testuser").await; + let client = connection::connect(&src).await.unwrap(); + client + .batch_execute( + "CREATE ROLE appowner LOGIN PASSWORD 'changeme' NOSUPERUSER;\n\ + CREATE TABLE owned_tbl (id int);\n\ + ALTER TABLE owned_tbl OWNER TO appowner;", + ) + .await + .unwrap(); + + let dir = TempDir::new().unwrap(); + let sql = cluster::backup::run(src.clone(), dir.path().to_path_buf(), env_for(&src), Arc::new(JobLogger::new())) + .await + .unwrap(); + + // Target cluster B: fresh, same bootstrap user. + let (_b, mut dst) = start_cluster("testuser").await; + + cluster::restore::run(dst.clone(), sql.clone(), env_for(&dst), Arc::new(JobLogger::new())) + .await + .unwrap(); + + // Verify the seeded role exists and the table's owner was preserved on B. + dst.database = "postgres".to_string(); + let bclient = connection::connect(&dst).await.unwrap(); + let role_exists: bool = bclient + .query_one("SELECT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'appowner');", &[]) + .await + .unwrap() + .get(0); + assert!(role_exists, "appowner role must be recreated on the target"); + + let owner: String = bclient + .query_one( + "SELECT tableowner FROM pg_tables WHERE tablename = 'owned_tbl';", + &[], + ) + .await + .unwrap() + .get(0); + assert_eq!(owner, "appowner", "table ownership must be preserved"); +} + +#[tokio::test] +async fn requires_superuser() { + init_tracing_for_test(); + let (_c, super_cfg) = start_cluster("testuser").await; + + // A non-superuser login role must be rejected before psql runs. + let client = connection::connect(&super_cfg).await.unwrap(); + client + .batch_execute("CREATE ROLE appuser LOGIN PASSWORD 'changeme' NOSUPERUSER;") + .await + .unwrap(); + + let mut weak = super_cfg.clone(); + weak.username = "appuser".to_string(); + + // The superuser pre-check happens before the dump file is read, so a + // non-existent restore path is fine — it must never be touched. + let missing = std::path::PathBuf::from("/nonexistent/cluster.sql"); + let err = cluster::restore::run(weak.clone(), missing, env_for(&weak), Arc::new(JobLogger::new())) + .await + .unwrap_err(); + + assert!( + err.to_string().contains("superuser"), + "expected a superuser error, got: {err}" + ); +} diff --git a/src/tests/domain/mod.rs b/src/tests/domain/mod.rs index a3d9561..9dc6940 100644 --- a/src/tests/domain/mod.rs +++ b/src/tests/domain/mod.rs @@ -2,7 +2,7 @@ mod mariadb; mod mongodb; mod mysql; mod postgres; -mod postgres_cluster; +mod cluster; mod redis; mod valkey; mod firebird; diff --git a/src/tests/domain/postgres_cluster.rs b/src/tests/domain/postgres_cluster.rs deleted file mode 100644 index 370855a..0000000 --- a/src/tests/domain/postgres_cluster.rs +++ /dev/null @@ -1,142 +0,0 @@ -use crate::domain::postgres::{cluster, connection}; -use crate::services::backup::logger::JobLogger; -use crate::services::config::{DatabaseConfig, DbType}; -use crate::tests::init_tracing_for_test; -use std::collections::HashMap; -use std::sync::Arc; -use tempfile::TempDir; -use testcontainers::runners::AsyncRunner; -use testcontainers::{ContainerAsync, ImageExt}; -use testcontainers_modules::postgres::Postgres; -use url::Host; - -async fn start_cluster(user: &str) -> (ContainerAsync, DatabaseConfig) { - let container = Postgres::default() - .with_env_var("POSTGRES_DB", "postgres") - .with_env_var("POSTGRES_USER", user) - .with_env_var("POSTGRES_PASSWORD", "changeme") - .with_tag("17") - .start() - .await - .unwrap(); - - let host = container - .get_host() - .await - .unwrap_or(Host::parse("127.0.0.1").unwrap()); - let port = container.get_host_port_ipv4(5432).await.unwrap_or(5432); - - let config = DatabaseConfig { - name: format!("cluster-{}", user), - database: "postgres".to_string(), - db_type: DbType::PostgresqlCluster, - username: user.to_string(), - password: "changeme".to_string(), - port, - host: host.to_string(), - generated_id: "40875631-e3d2-4dfe-a26b-2a347ecc64fd".to_string(), - path: "".to_string(), - max_packet_size: "".to_string(), - }; - (container, config) -} - -fn env_for(cfg: &DatabaseConfig) -> HashMap { - let mut env = std::env::vars().collect::>(); - env.insert("PGPASSWORD".to_string(), cfg.password.clone()); - env -} - -#[tokio::test] -async fn cluster_backup_produces_sql_with_roles_and_databases() { - init_tracing_for_test(); - let (_c, cfg) = start_cluster("testuser").await; - - let dir = TempDir::new().unwrap(); - let logger = Arc::new(JobLogger::new()); - let sql = cluster::backup::run(cfg.clone(), dir.path().to_path_buf(), env_for(&cfg), logger) - .await - .unwrap(); - - assert!(sql.is_file()); - let contents = std::fs::read_to_string(&sql).unwrap(); - assert!(contents.contains("CREATE ROLE"), "expected CREATE ROLE in dump"); - assert!(contents.contains("CREATE DATABASE") || contents.contains("\\connect"), - "expected database statements in dump"); -} - -#[tokio::test] -async fn cluster_backup_requires_superuser() { - init_tracing_for_test(); - let (_c, super_cfg) = start_cluster("testuser").await; - - // Create a NON-superuser login role on the cluster. - let client = connection::connect(&super_cfg).await.unwrap(); - client - .batch_execute("CREATE ROLE appuser LOGIN PASSWORD 'changeme' NOSUPERUSER;") - .await - .unwrap(); - - let mut weak = super_cfg.clone(); - weak.username = "appuser".to_string(); - - let dir = TempDir::new().unwrap(); - let logger = Arc::new(JobLogger::new()); - let err = cluster::backup::run(weak.clone(), dir.path().to_path_buf(), env_for(&weak), logger) - .await - .unwrap_err(); - - assert!( - err.to_string().contains("superuser"), - "expected a superuser error, got: {err}" - ); -} - -#[tokio::test] -async fn cluster_backup_restore_round_trip() { - init_tracing_for_test(); - - // Source cluster A: seed a role + a table owned by that role. - let (_a, src) = start_cluster("testuser").await; - let client = connection::connect(&src).await.unwrap(); - client - .batch_execute( - "CREATE ROLE appowner LOGIN PASSWORD 'changeme' NOSUPERUSER;\n\ - CREATE TABLE owned_tbl (id int);\n\ - ALTER TABLE owned_tbl OWNER TO appowner;", - ) - .await - .unwrap(); - - let dir = TempDir::new().unwrap(); - let sql = cluster::backup::run(src.clone(), dir.path().to_path_buf(), env_for(&src), Arc::new(JobLogger::new())) - .await - .unwrap(); - - // Target cluster B: fresh, same bootstrap user. - let (_b, mut dst) = start_cluster("testuser").await; - - cluster::restore::run(dst.clone(), sql.clone(), env_for(&dst), Arc::new(JobLogger::new())) - .await - .unwrap(); - - // Verify the seeded role exists and the table's owner was preserved on B. - dst.database = "postgres".to_string(); - let bclient = connection::connect(&dst).await.unwrap(); - let role_exists: bool = bclient - .query_one("SELECT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'appowner');", &[]) - .await - .unwrap() - .get(0); - assert!(role_exists, "appowner role must be recreated on the target"); - - let owner: String = bclient - .query_one( - "SELECT tableowner FROM pg_tables WHERE tablename = 'owned_tbl';", - &[], - ) - .await - .unwrap() - .get(0); - assert_eq!(owner, "appowner", "table ownership must be preserved"); -} From c3f7e7b10cf14a487b8d4a99e3075b32049556a3 Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Fri, 26 Jun 2026 22:53:25 +0200 Subject: [PATCH 19/19] fix --- databases.json | 6 +++--- src/domain/postgres/cluster/backup.rs | 3 +-- src/domain/postgres/cluster/restore.rs | 3 --- src/domain/postgres/connection.rs | 27 +------------------------- src/tests/domain/cluster/database.rs | 2 -- src/tests/domain/cluster/mod.rs | 3 --- 6 files changed, 5 insertions(+), 39 deletions(-) diff --git a/databases.json b/databases.json index 4f52f29..40c2f90 100644 --- a/databases.json +++ b/databases.json @@ -115,10 +115,10 @@ { "name": "Test database - PostgreSQL cluster", "type": "postgresql-cluster", - "username": "postgres", - "password": "changeme", + "username": "nextclouddbuser", + "password": "50AL2Oh5IXajbOAxfJ", "port": 5432, - "host": "db-postgres", + "host": "nextcloud-db", "generated_id": "16678199-ff7e-4c97-8c83-0adeff214681" } ] diff --git a/src/domain/postgres/cluster/backup.rs b/src/domain/postgres/cluster/backup.rs index dac6fa2..ecaf240 100644 --- a/src/domain/postgres/cluster/backup.rs +++ b/src/domain/postgres/cluster/backup.rs @@ -11,8 +11,6 @@ use super::super::connection::{ use crate::services::backup::logger::JobLogger; use crate::services::config::DatabaseConfig; -/// Backs up an entire PostgreSQL cluster (roles + all databases + ownership + -/// privileges) with `pg_dumpall` into a single `.sql`. Requires a superuser. pub async fn run( cfg: DatabaseConfig, backup_dir: PathBuf, @@ -52,6 +50,7 @@ pub async fn run( .arg("--host").arg(&cfg.host) .arg("--port").arg(cfg.port.to_string()) .arg("--username").arg(&cfg.username) + .arg("-v") .arg("-f").arg(&file_path) .envs(env) .output(); diff --git a/src/domain/postgres/cluster/restore.rs b/src/domain/postgres/cluster/restore.rs index d47fc2f..e24efa8 100644 --- a/src/domain/postgres/cluster/restore.rs +++ b/src/domain/postgres/cluster/restore.rs @@ -9,9 +9,6 @@ use super::super::connection::{is_superuser, psql_binary_name, select_pg_path, s use crate::services::backup::logger::JobLogger; use crate::services::config::DatabaseConfig; -/// Restores a cluster `.sql` produced by the cluster backup via `psql` against -/// a fresh target cluster. Requires a superuser. psql runs continue-on-error -/// (its default); a non-zero process exit is treated as failure. pub async fn run( cfg: DatabaseConfig, restore_file: PathBuf, diff --git a/src/domain/postgres/connection.rs b/src/domain/postgres/connection.rs index 6f12035..0b0aebd 100644 --- a/src/domain/postgres/connection.rs +++ b/src/domain/postgres/connection.rs @@ -33,10 +33,6 @@ pub async fn server_version(cfg: &DatabaseConfig) -> Result { Ok(version) } -/// Whether the role used by `cfg` is a cluster superuser. Cluster backup -/// (`pg_dumpall`, needs to read role passwords) and cluster restore (`CREATE -/// ROLE`, `ALTER ... OWNER`) both require a superuser; callers pre-check this -/// and fail fast with a clear error. pub async fn is_superuser(cfg: &DatabaseConfig) -> Result { let client = connect(cfg).await?; let is_super: bool = client @@ -47,32 +43,11 @@ pub async fn is_superuser(cfg: &DatabaseConfig) -> Result { Ok(is_super) } -/// Resolves the `bin` directory of a PostgreSQL installation for the given -/// major version, in a cross-platform way. -/// -/// Resolution order: -/// 1. The `PG_BIN_DIR` environment variable, if set, is used as-is. This -/// allows users/CI to override detection for non-standard installs -/// (e.g. portable PostgreSQL distributions, custom install locations). -/// 2. Platform-specific default install locations (Debian/Ubuntu packages, -/// the official Windows installer, Homebrew/Postgres.app on macOS, and -/// common RPM-based layouts on other Linux distros). -/// 3. A `PATH` lookup for `pg_dump` (`pg_dump.exe` on Windows), returning -/// its parent directory. -/// 4. The historical Debian/Ubuntu path as a last-resort fallback, so the -/// function keeps returning a `PathBuf` (never panics) even when nothing -/// was found, preserving the previous behavior for callers. -/// -/// The override is sourced from `CONFIG.pg_bin_dir` (the `PG_BIN_DIR` -/// environment variable). An empty value means "unset" and falls through to -/// detection. + pub fn select_pg_path(version: &str) -> std::path::PathBuf { select_pg_path_with(version, &CONFIG.pg_bin_dir) } -/// Inner resolver behind [`select_pg_path`], parameterized over the -/// `PG_BIN_DIR` override. Kept pure (no env / no `CONFIG` access) so it is -/// unit-testable without mutating process-global state. pub(crate) fn select_pg_path_with(version: &str, pg_bin_dir: &str) -> std::path::PathBuf { let major = version.split('.').next().unwrap_or("17"); diff --git a/src/tests/domain/cluster/database.rs b/src/tests/domain/cluster/database.rs index a29c285..2f40cfd 100644 --- a/src/tests/domain/cluster/database.rs +++ b/src/tests/domain/cluster/database.rs @@ -2,8 +2,6 @@ use crate::domain::factory::DatabaseFactory; use crate::services::config::{DatabaseConfig, DbType}; use std::path::Path; -// A `postgresql-cluster` config. The factory constructs `PostgresClusterDatabase` -// without opening a connection, so these tests need no container. fn cluster_config() -> DatabaseConfig { DatabaseConfig { name: "cluster".to_string(), diff --git a/src/tests/domain/cluster/mod.rs b/src/tests/domain/cluster/mod.rs index 3288f87..e26cd04 100644 --- a/src/tests/domain/cluster/mod.rs +++ b/src/tests/domain/cluster/mod.rs @@ -9,9 +9,6 @@ use testcontainers::{ContainerAsync, ImageExt}; use testcontainers_modules::postgres::Postgres; use url::Host; -/// Starts a fresh Postgres 17 cluster whose bootstrap superuser is `user`, and -/// returns the container guard plus a `postgresql-cluster` config pointing at it. -/// Shared by the `backup` and `restore` integration tests. async fn start_cluster(user: &str) -> (ContainerAsync, DatabaseConfig) { let container = Postgres::default() .with_env_var("POSTGRES_DB", "postgres")