diff --git a/AGENTS.md b/AGENTS.md index 858e9329ff..850538f080 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -29,6 +29,10 @@ Open-source platform for internal tools, workflows, API integrations, background - **Agent workers**: `docs/agent-worker-e2e.md` — building and running one locally. An agent reaches the DB only through the API, so `Connection::Http` paths are never taken by a plain `cargo run`; a normal build cannot start one at all. +- **External instance data tables**: `docs/external-instance-datatables.md` — the cluster Windmill + administers behind `external_instance` data tables and Ducklake catalogs: its invariants (one + lifecycle lock, managed-object markers, the setup gate, per-cluster roles, fork copy ownership) + and how to run one locally - **Enterprise**: `docs/enterprise.md` — EE file conventions and PR workflow - **Operator write rights**: `docs/operator-write-rights.md` — which `operator_settings` flags are enforced rather than cosmetic, and why a right that is granted-unless-withdrawn needs `Option` diff --git a/backend/.sqlx/query-71ee2cb6661cca1fa4d8874a7f6d368347c59f36fd87df6dc7996152ccb84af0.json b/backend/.sqlx/query-71ee2cb6661cca1fa4d8874a7f6d368347c59f36fd87df6dc7996152ccb84af0.json deleted file mode 100644 index a03fab7081..0000000000 --- a/backend/.sqlx/query-71ee2cb6661cca1fa4d8874a7f6d368347c59f36fd87df6dc7996152ccb84af0.json +++ /dev/null @@ -1,38 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT id, name, enabled, pwd FROM datatable_role", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "id", - "type_info": "Varchar" - }, - { - "ordinal": 1, - "name": "name", - "type_info": "Varchar" - }, - { - "ordinal": 2, - "name": "enabled", - "type_info": "Bool" - }, - { - "ordinal": 3, - "name": "pwd", - "type_info": "Text" - } - ], - "parameters": { - "Left": [] - }, - "nullable": [ - false, - false, - false, - true - ] - }, - "hash": "71ee2cb6661cca1fa4d8874a7f6d368347c59f36fd87df6dc7996152ccb84af0" -} diff --git a/backend/.sqlx/query-79799b5a2e499df6c28e286c42b9ad2db940c2455ab19cc95e5198baf96d5629.json b/backend/.sqlx/query-79799b5a2e499df6c28e286c42b9ad2db940c2455ab19cc95e5198baf96d5629.json deleted file mode 100644 index c36b6fb361..0000000000 --- a/backend/.sqlx/query-79799b5a2e499df6c28e286c42b9ad2db940c2455ab19cc95e5198baf96d5629.json +++ /dev/null @@ -1,29 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "\n SELECT ws.workspace_id AS \"workspace_id!\", dt.key AS \"datatable!\"\n FROM workspace_settings ws\n JOIN workspace w ON w.id = ws.workspace_id AND w.deleted = false\n CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt\n WHERE ws.workspace_id <> $1\n AND dt.value->'database'->>'resource_type' = 'instance'\n AND dt.value->'database'->>'resource_path' = $2\n ORDER BY ws.workspace_id, dt.key\n ", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "workspace_id!", - "type_info": "Varchar" - }, - { - "ordinal": 1, - "name": "datatable!", - "type_info": "Text" - } - ], - "parameters": { - "Left": [ - "Text", - "Text" - ] - }, - "nullable": [ - false, - null - ] - }, - "hash": "79799b5a2e499df6c28e286c42b9ad2db940c2455ab19cc95e5198baf96d5629" -} diff --git a/backend/.sqlx/query-86af9d51a158ea5cb6161461ecddf2a63695f8cbf8af648da5a0a77a5b9d02ba.json b/backend/.sqlx/query-86af9d51a158ea5cb6161461ecddf2a63695f8cbf8af648da5a0a77a5b9d02ba.json deleted file mode 100644 index 7627d9828d..0000000000 --- a/backend/.sqlx/query-86af9d51a158ea5cb6161461ecddf2a63695f8cbf8af648da5a0a77a5b9d02ba.json +++ /dev/null @@ -1,17 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "INSERT INTO datatable_role (id, name, enabled, pwd) VALUES ($1, $2, $3, $4)", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Varchar", - "Varchar", - "Bool", - "Text" - ] - }, - "nullable": [] - }, - "hash": "86af9d51a158ea5cb6161461ecddf2a63695f8cbf8af648da5a0a77a5b9d02ba" -} diff --git a/backend/.sqlx/query-b9842d2d8abf382bd82d8fa1de012373638be391f884f81dc387ffc465badac6.json b/backend/.sqlx/query-b9842d2d8abf382bd82d8fa1de012373638be391f884f81dc387ffc465badac6.json deleted file mode 100644 index 4b3a2f33e9..0000000000 --- a/backend/.sqlx/query-b9842d2d8abf382bd82d8fa1de012373638be391f884f81dc387ffc465badac6.json +++ /dev/null @@ -1,20 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT jsonb_object_keys(value->'databases') FROM global_settings\n WHERE name = 'custom_instance_pg_databases'", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "jsonb_object_keys", - "type_info": "Text" - } - ], - "parameters": { - "Left": [] - }, - "nullable": [ - null - ] - }, - "hash": "b9842d2d8abf382bd82d8fa1de012373638be391f884f81dc387ffc465badac6" -} diff --git a/backend/.sqlx/query-d48ca62c86b1af7a9dd2450c1c28dc45020a2a553d8874c49f9eafedea5a9d40.json b/backend/.sqlx/query-d48ca62c86b1af7a9dd2450c1c28dc45020a2a553d8874c49f9eafedea5a9d40.json deleted file mode 100644 index 49875acd78..0000000000 --- a/backend/.sqlx/query-d48ca62c86b1af7a9dd2450c1c28dc45020a2a553d8874c49f9eafedea5a9d40.json +++ /dev/null @@ -1,24 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT dt.key AS \"datatable!\"\n FROM workspace_settings ws\n CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt\n WHERE ws.workspace_id = $1\n AND dt.key <> $2\n AND NOT dt.value ? 'permissions'\n AND dt.value->'database'->>'resource_type' = 'instance'\n AND dt.value->'database'->>'resource_path' = $3\n ORDER BY dt.key", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "datatable!", - "type_info": "Text" - } - ], - "parameters": { - "Left": [ - "Text", - "Text", - "Text" - ] - }, - "nullable": [ - null - ] - }, - "hash": "d48ca62c86b1af7a9dd2450c1c28dc45020a2a553d8874c49f9eafedea5a9d40" -} diff --git a/backend/Cargo.lock b/backend/Cargo.lock index 9182843789..e80d0edf08 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -15660,6 +15660,7 @@ dependencies = [ "pin-project-lite", "pkcs1", "postgres-native-tls 0.5.3", + "postgres-protocol", "prometheus", "quick_cache", "rand 0.9.5", diff --git a/backend/Cargo.toml b/backend/Cargo.toml index 77f512a3dd..8571b0b99c 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -626,6 +626,7 @@ wasm-bindgen-test = "^0" convert_case = "0.6.0" getrandom = "0.2" tokio-postgres = {version = "^0.7", features = ["array-impls", "with-serde_json-1", "with-chrono-0_4", "with-uuid-1", "with-bit-vec-0_6"]} +postgres-protocol = "0.6" rust-postgres = { package = "tokio-postgres", git = "https://github.com/MaterializeInc/rust-postgres", rev = "78c1222577bb091d69bc22b1bc7ad01c14675abe"} rust-postgres-native-tls = { package = "postgres-native-tls", git = "https://github.com/MaterializeInc/rust-postgres", features = ["runtime"], rev = "78c1222577bb091d69bc22b1bc7ad01c14675abe" } bit-vec = "=0.6.3" diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 58f9aaf07d..21e445394e 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -9855e1b7a43a0a33e04f8accf1c497af3fd9b139 +dfe7b8b9204d21e0264fbea1c6f6eedf9e738d56 diff --git a/backend/migrations/20260917133824_datatable_role_cluster.down.sql b/backend/migrations/20260917133824_datatable_role_cluster.down.sql new file mode 100644 index 0000000000..379f1e3d34 --- /dev/null +++ b/backend/migrations/20260917133824_datatable_role_cluster.down.sql @@ -0,0 +1,22 @@ +-- Roles on the external cluster are live logins there; dropping the column would forget them. +LOCK TABLE datatable_role; +DO $$ +BEGIN + IF EXISTS (SELECT 1 FROM datatable_role WHERE cluster <> 'instance') THEN + RAISE EXCEPTION 'datatable_role holds roles on the external instance cluster. Delete them in instance settings first.'; + END IF; + -- Before this, only data tables on Windmill's own cluster could be under roles, and a role + -- block left with just `admin` survives deleting every external role. + IF EXISTS ( + SELECT 1 FROM workspace_settings ws, + jsonb_each(CASE WHEN jsonb_typeof(ws.datatable->'datatables') = 'object' + THEN ws.datatable->'datatables' ELSE '{}'::jsonb END) dt + WHERE dt.value->'database'->>'resource_type' = 'external_instance' + AND dt.value ? 'permissions' + ) THEN + RAISE EXCEPTION 'external instance data tables are still under roles. Turn their roles off first.'; + END IF; +END $$; +ALTER TABLE datatable_role DROP CONSTRAINT datatable_role_cluster_name_key; +ALTER TABLE datatable_role ADD CONSTRAINT datatable_role_name_key UNIQUE (name); +ALTER TABLE datatable_role DROP COLUMN cluster; diff --git a/backend/migrations/20260917133824_datatable_role_cluster.up.sql b/backend/migrations/20260917133824_datatable_role_cluster.up.sql new file mode 100644 index 0000000000..d63f06f1f4 --- /dev/null +++ b/backend/migrations/20260917133824_datatable_role_cluster.up.sql @@ -0,0 +1,8 @@ +-- A data table role is a Postgres login on one cluster: Windmill's own ('instance'), or the external +-- instance cluster ('external_instance'). Role names are the cluster's own key, so they are unique +-- per cluster rather than across the instance. +ALTER TABLE datatable_role + ADD COLUMN cluster VARCHAR(20) NOT NULL DEFAULT 'instance' + CHECK (cluster IN ('instance', 'external_instance')); +ALTER TABLE datatable_role DROP CONSTRAINT datatable_role_name_key; +ALTER TABLE datatable_role ADD CONSTRAINT datatable_role_cluster_name_key UNIQUE (cluster, name); diff --git a/backend/tests/instance_config.rs b/backend/tests/instance_config.rs index 26a46d272e..52ebbee7e7 100644 --- a/backend/tests/instance_config.rs +++ b/backend/tests/instance_config.rs @@ -1609,6 +1609,52 @@ async fn declarative_sync_rejects_an_unusable_default_allowed_origins(db: Pool

) { + clear_settings_and_configs(&db).await; + let cluster = |host: &str, password: &str| serde_json::json!({ "host": host, "port": 5432, "user": "wm_admin", "password": password }); + sqlx::query( + "INSERT INTO global_settings (name, value) VALUES + ('external_instance_pg', $1), + ('external_instance_pg_state', '{\"databases\": {\"dt_a\": {\"success\": true}}}')", + ) + .bind(cluster("pg-a.internal", "one")) + .execute(&db) + .await + .unwrap(); + let current = BTreeMap::from([( + "external_instance_pg".to_string(), + cluster("pg-a.internal", "one"), + )]); + let sync = |value: serde_json::Value| { + let desired = BTreeMap::from([("external_instance_pg".to_string(), value)]); + let (db, current) = (db.clone(), current.clone()); + async move { + windmill_common::instance_config::sync_global_settings_declarative( + &db, ¤t, &desired, + ) + .await + } + }; + + let err = sync(cluster("pg-b.internal", "one")) + .await + .expect_err("another host must be refused while dt_a is registered"); + assert!(err.to_string().contains("dt_a"), "got: {err}"); + assert_eq!( + get_global_setting(&db, "external_instance_pg").await, + Some(cluster("pg-a.internal", "one")) + ); + + sync(cluster("PG-A.internal ", "two")) + .await + .expect("a new login on the same cluster must sync"); +} + /// The accent color is interpolated into a stylesheet every user loads, so the operator /// path must refuse anything but `#rrggbb` just like the settings API does. #[sqlx::test(fixtures("base"))] diff --git a/backend/windmill-api-integration-tests/tests/datatable_roles.rs b/backend/windmill-api-integration-tests/tests/datatable_roles.rs index 4948a0098a..ddb94f8c2b 100644 --- a/backend/windmill-api-integration-tests/tests/datatable_roles.rs +++ b/backend/windmill-api-integration-tests/tests/datatable_roles.rs @@ -391,7 +391,11 @@ async fn concurrent_role_creations_both_survive(db: Pool) -> anyhow::R assert_eq!(a.0, 200, "{}", a.1); assert_eq!(b.0, 200, "{}", b.1); - let catalog = windmill_common::datatable_roles::read_role_catalog(&db).await?; + let catalog = windmill_common::datatable_roles::read_role_catalog( + &db, + windmill_common::datatable_roles::DatatableRoleCluster::Instance, + ) + .await?; let recorded: Vec<&str> = catalog.values().map(|r| r.name.as_str()).collect(); for name in &names { assert!( @@ -460,7 +464,11 @@ async fn a_role_delete_that_fails_part_way_leaves_the_role_disabled( let body = resp.text().await?; assert_eq!(status, 400, "{body}"); - let catalog = windmill_common::datatable_roles::read_role_catalog(&db).await?; + let catalog = windmill_common::datatable_roles::read_role_catalog( + &db, + windmill_common::datatable_roles::DatatableRoleCluster::Instance, + ) + .await?; let role = catalog .get(&id) .expect("a failed delete keeps the entry to retry"); diff --git a/backend/windmill-api-settings/src/lib.rs b/backend/windmill-api-settings/src/lib.rs index ca93d3c682..71cbcc8460 100644 --- a/backend/windmill-api-settings/src/lib.rs +++ b/backend/windmill-api-settings/src/lib.rs @@ -42,7 +42,6 @@ use axum::{ routing::{get, post}, Json, Router, }; -use serde_json::json; use serde::{Deserialize, Serialize}; use windmill_ai::ai_cache::bump_instance_ai_config_revision; @@ -61,6 +60,7 @@ use windmill_common::{ ACCENT_COLOR_SETTING, AI_CONFIG_SETTING, APP_WORKSPACED_ROUTE_SETTING, AUTOMATE_USERNAME_CREATION_SETTING, CRITICAL_ALERT_MUTE_UI_SETTING, CUSTOM_TAGS_SETTING, DEFAULT_TAGS_WORKSPACES_SETTING, DISABLE_HUB_SETTING, EMAIL_DOMAIN_SETTING, ENV_SETTINGS, + EXTERNAL_INSTANCE_PG_SETTING, GITHUB_APP_WEBHOOK_BASE_URL_SETTING, HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING, HTTP_ROUTE_WORKSPACED_ROUTE_SETTING, HUB_ACCESSIBLE_URL_SETTING, HUB_BASE_URL_SETTING, INSTANCE_BANNER_SETTING, MAX_RETENTION_OVERRIDE_WORKSPACES, @@ -181,6 +181,22 @@ pub fn global_service() -> Router { "/refresh_custom_instance_user_pwd", post(refresh_custom_instance_user_pwd), ) + .route( + "/external_instance_pg/status", + get(get_external_instance_pg_status), + ) + .route( + "/external_instance_pg/setup", + post(setup_external_instance_pg), + ) + .route( + "/external_instance_pg/databases", + get(list_external_instance_pg_databases), + ) + .route( + "/external_instance_pg/databases/{name}", + post(create_external_instance_pg_database).delete(drop_external_instance_pg_database), + ) .route( "/setup_custom_instance_pg_database/{name}", post(setup_custom_instance_pg_database), @@ -853,6 +869,14 @@ pub async fn set_global_setting_internal( ))); } + if key == EXTERNAL_INSTANCE_PG_SETTING { + return windmill_common::external_instance_pg::write_external_instance_pg_setting( + db, + Some(&value), + ) + .await; + } + run_setting_pre_write_hook(db, &key, &value).await?; match value { @@ -1248,7 +1272,7 @@ async fn set_instance_config( let desired_map = desired.global_settings.to_settings_map(); if !desired_map.is_empty() { let current_map = current.global_settings.to_settings_map(); - let settings_diff = + let mut settings_diff = instance_config::diff_global_settings(¤t_map, &desired_map, ApplyMode::Merge); let ai_config_changed = settings_diff .upserts @@ -1277,8 +1301,15 @@ async fn set_instance_config( } for (key, value) in &settings_diff.upserts { - run_setting_pre_write_hook(&db, key, value).await?; + if key != EXTERNAL_INSTANCE_PG_SETTING { + run_setting_pre_write_hook(&db, key, value).await?; + } } + windmill_common::external_instance_pg::write_external_instance_pg_from_diff( + &db, + &mut settings_diff, + ) + .await?; instance_config::apply_settings_diff(&db, &settings_diff) .await @@ -1643,6 +1674,8 @@ struct CustomInstanceDb { tag: Option, #[serde(default, skip_serializing_if = "Vec::is_empty")] used_by_workspaces: Vec, + #[serde(default, skip_serializing_if = "Option::is_none")] + workspace_id: Option, } #[derive(Deserialize, Debug, Serialize, Default)] @@ -1685,7 +1718,40 @@ async fn list_custom_instance_pg_databases( )) })?; - if windmill_api_auth::is_super_admin_authed(&db, &authed).await? { + if !windmill_api_auth::is_super_admin_authed(&db, &authed).await? { + // A fork copy's name gives away the workspace it was reserved for, so every pending fork on + // the instance would be listed. Kept for members of that workspace, and wherever the + // caller's workspaces use it, e.g. the fork it was finalized into. + let reserved_visible: BTreeSet = sqlx::query_scalar( + r#"SELECT e.k FROM global_settings gs + CROSS JOIN LATERAL jsonb_each(gs.value->'databases') AS e(k, v) + WHERE gs.name = 'custom_instance_pg_databases' AND e.v->>'workspace_id' IS NOT NULL + AND (EXISTS (SELECT 1 FROM usr WHERE usr.email = $1 + AND usr.workspace_id = e.v->>'workspace_id') + OR EXISTS (SELECT 1 FROM usr JOIN workspace_settings ws + ON ws.workspace_id = usr.workspace_id + CROSS JOIN LATERAL jsonb_each( + CASE WHEN jsonb_typeof(ws.datatable->'datatables') = 'object' + THEN ws.datatable->'datatables' ELSE '{}'::jsonb END) dt + WHERE usr.email = $1 + AND dt.value->'database'->>'resource_type' = 'instance' + AND dt.value->'database'->>'resource_path' = e.k))"#, + ) + .bind(&authed.email) + .fetch_all(&db) + .await? + .into_iter() + .collect(); + result.retain(|dbname, entry| { + entry.workspace_id.is_none() || reserved_visible.contains(dbname) + }); + // Which workspace reserved a copy is still only for superadmins. + for entry in result.values_mut() { + entry.workspace_id = None; + } + return Ok(Json(result)); + } + { // Enrich each database with the list of workspaces referencing it through // either a ducklake catalog or a datatable database whose resource_type is // 'instance'. Not stored in DB to avoid drift. @@ -1741,6 +1807,139 @@ async fn refresh_custom_instance_user_pwd( Ok(Json(())) } +async fn get_external_instance_pg_status( + authed: ApiAuthed, + Extension(db): Extension, +) -> JsonResult { + require_super_admin(&db, &authed).await?; + Ok(Json( + windmill_common::external_instance_pg::external_instance_pg_status(&db).await?, + )) +} + +#[derive(Deserialize)] +struct SetupExternalInstancePgBody { + #[serde(default)] + rotate_passwords: bool, +} + +async fn setup_external_instance_pg( + authed: ApiAuthed, + Extension(db): Extension, + Json(body): Json, +) -> JsonResult { + require_super_admin(&db, &authed).await?; + let report = windmill_common::external_instance_pg::setup_external_instance_pg_unchecked( + &db, + body.rotate_passwords, + ) + .await?; + let rotated = body.rotate_passwords.to_string(); + let success = report.success.to_string(); + windmill_audit::audit_oss::audit_log( + &db, + &authed, + "settings.setup_external_instance_pg", + windmill_audit::ActionKind::Update, + "global", + Some(&authed.email), + Some( + [ + ("rotate_passwords", rotated.as_str()), + ("success", success.as_str()), + ] + .into(), + ), + ) + .await?; + Ok(Json(report)) +} + +#[derive(Serialize)] +struct ExternalInstancePgDatabase { + #[serde(flatten)] + status: windmill_common::instance_config::CustomInstanceDb, + used_by_workspaces: Vec, +} + +async fn list_external_instance_pg_databases( + authed: ApiAuthed, + Extension(db): Extension, +) -> JsonResult> { + require_super_admin(&db, &authed).await?; + let databases = windmill_common::external_instance_pg::external_instance_databases(&db).await?; + let mut usages = + windmill_common::external_instance_pg::external_instance_database_usages(&db).await?; + Ok(Json( + databases + .into_iter() + .map(|(name, status)| { + let used_by_workspaces = usages.remove(&name).unwrap_or_default(); + ( + name, + ExternalInstancePgDatabase { + status, + used_by_workspaces: used_by_workspaces.into_iter().collect(), + }, + ) + }) + .collect(), + )) +} + +async fn create_external_instance_pg_database( + authed: ApiAuthed, + Extension(db): Extension, + Path(dbname): Path, + Json(body): Json, +) -> JsonResult<()> { + require_super_admin(&db, &authed).await?; + let tag = body.tag.as_deref().unwrap_or("datatable"); + let mut tx = db.begin().await?; + windmill_common::external_instance_pg::create_external_instance_database_unchecked( + &db, &mut tx, &dbname, tag, None, + ) + .await?; + tx.commit().await?; + windmill_audit::audit_oss::audit_log( + &db, + &authed, + "settings.create_external_instance_pg_database", + windmill_audit::ActionKind::Create, + "global", + Some(&authed.email), + Some([("dbname", dbname.as_str()), ("tag", tag)].into()), + ) + .await?; + Ok(Json(())) +} + +async fn drop_external_instance_pg_database( + authed: ApiAuthed, + Extension(db): Extension, + Path(dbname): Path, +) -> JsonResult<()> { + require_super_admin(&db, &authed).await?; + // A data table naming a dropped database fails on every job, far from the drop that caused it. + let mut tx = db.begin().await?; + windmill_common::external_instance_pg::drop_external_instance_database_unchecked( + &mut tx, &dbname, None, + ) + .await?; + tx.commit().await?; + windmill_audit::audit_oss::audit_log( + &db, + &authed, + "settings.drop_external_instance_pg_database", + windmill_audit::ActionKind::Delete, + "global", + Some(&authed.email), + Some([("dbname", dbname.as_str())].into()), + ) + .await?; + Ok(Json(())) +} + #[derive(Deserialize)] struct SetupCustomInstanceDbBody { tag: Option, @@ -1752,18 +1951,46 @@ async fn setup_custom_instance_pg_database( Path(dbname): Path, Json(body): Json, ) -> JsonResult { + // Before anything is recorded: the status written below replaces the registry entry, and with it + // the workspace a fork copy is reserved for. + require_super_admin(&db, &authed).await?; + windmill_common::workspaces::ensure_instance_pg_available(&db).await?; + // Fork cleanup checks and drops the database and its entry under this lock. Held from before + // the setup creates the database to after its entry is written, neither lands on the other's + // half-done state: a dropped database with its entry written back, or the reverse. + let mut tx = db.begin().await?; + windmill_common::datatable_roles::lock_instance_databases_governance(&mut tx, [dbname.trim()]) + .await?; let mut logs = CustomInstanceDbLogs::default(); let result = setup_custom_instance_pg_database_inner(authed, &db, &dbname, &mut logs).await; let success = result.is_ok(); let error = result.err().map(|e| e.to_string()); - let status = - CustomInstanceDb { logs, success, error, tag: body.tag, used_by_workspaces: vec![] }; + let status = CustomInstanceDb { + logs, + success, + error, + tag: body.tag, + used_by_workspaces: vec![], + workspace_id: None, + }; let status_json = serde_json::to_value(&status).map_err(to_anyhow)?; - // Save that the database was setup successfully - sqlx::query!( - r#"UPDATE global_settings SET value = jsonb_set(value, '{databases}', (COALESCE(value->'databases', '{}'::jsonb) || to_jsonb($1::json))) WHERE name = 'custom_instance_pg_databases'"#, - json!({ dbname: status_json }) - ).execute(&db).await?; + // The fork reservation is carried over inside the write, from whatever the row holds then: a + // rename migrating it while the setup above ran would otherwise be overwritten with the value + // this request started from, stranding the copy under the archived workspace. + let saved = sqlx::query_scalar::<_, serde_json::Value>( + r#"UPDATE global_settings SET value = jsonb_set(value, '{databases}', + COALESCE(value->'databases', '{}'::jsonb) + || jsonb_build_object($1::text, $2::jsonb || jsonb_build_object( + 'workspace_id', value->'databases'->$1::text->'workspace_id'))) + WHERE name = 'custom_instance_pg_databases' + RETURNING value->'databases'->$1::text"#, + ) + .bind(&dbname) + .bind(&status_json) + .fetch_one(&mut *tx) + .await?; + tx.commit().await?; + let status: CustomInstanceDb = serde_json::from_value(saved).map_err(to_anyhow)?; Ok(Json(status)) } diff --git a/backend/windmill-api-workspaces/src/datatable_acl.rs b/backend/windmill-api-workspaces/src/datatable_acl.rs index 04dc3d4e80..ebab6214ce 100644 --- a/backend/windmill-api-workspaces/src/datatable_acl.rs +++ b/backend/windmill-api-workspaces/src/datatable_acl.rs @@ -6,7 +6,7 @@ * LICENSE-AGPL for a copy of the license. */ -//! Ownership and grants on the objects of an instance data table. +//! Ownership and grants on the objects of a data table on a cluster Windmill manages. //! //! [`datatable_permissions`](crate::datatable_permissions) decides who may connect as which role; //! this decides what each role may then touch. Every change is a real `GRANT`, `REVOKE`, @@ -33,7 +33,7 @@ use windmill_audit::audit_oss::audit_log; use windmill_audit::ActionKind; use windmill_common::datatable_roles::{ lock_role_catalog, quote_ident, read_role_catalog, read_role_catalog_tx, DatatableRoleCatalog, - ADMIN_DATATABLE_ROLE, CUSTOM_INSTANCE_USER, + DatatableRoleCluster, ADMIN_DATATABLE_ROLE, CUSTOM_INSTANCE_USER, }; use windmill_common::error::{pg_error_message, Error, JsonResult, Result}; use windmill_common::workspaces::{resolve_governing_datatable, DataTable, GoverningDatatable}; @@ -297,22 +297,22 @@ fn role_names(catalog: &DatatableRoleCatalog) -> Vec { names } -fn ensure_instance(governing: &GoverningDatatable) -> Result<()> { - if governing.is_instance() { - return Ok(()); - } - Err(Error::BadRequest(format!( - "Data table '{}' is backed by a Postgres resource, so its access is managed on that \ - server directly. Only a data table on the Windmill instance's own database has data \ - table roles to grant to.", - governing.name - ))) +/// The cluster whose roles the data table's grants name. +fn ensure_managed(governing: &GoverningDatatable) -> Result { + governing.role_cluster().ok_or_else(|| { + Error::BadRequest(format!( + "Data table '{}' is backed by a Postgres resource, so its access is managed on that \ + server directly. Only a data table on a database Windmill manages has data table \ + roles to grant to.", + governing.name + )) + }) } /// The data table's `admin` connection, and the notices Postgres sends on it. /// -/// Authorization: connects as `custom_instance_user` with the instance's own credentials and checks -/// nothing. Callers MUST have authorized the request first — a request about to be refused must +/// Authorization: connects as `custom_instance_user` with the cluster's stored credentials and +/// checks nothing. Callers MUST have authorized the request first — a request about to be refused must /// not get as far as this connection. async fn connect_as_admin_unchecked( db: &DB, @@ -322,21 +322,33 @@ async fn connect_as_admin_unchecked( mpsc::UnboundedReceiver, String, )> { - ensure_instance(governing)?; + let cluster = ensure_managed(governing)?; // Built from the authorized entry, never by resolving the settings again: a save in between // could point the entry at a resource on another server and back, and this connection would // then alter a database the later checks of the entry never see. - let mut pg = PgDatabase::parse_uri(&windmill_common::get_database_url().await?.as_str().await)?; - pg.dbname = governing + let dbname = governing .datatable .database .as_ref() .expect("a governing entry owns a database") .resource_path .clone(); - pg.user = Some(CUSTOM_INSTANCE_USER.to_string()); - pg.password = Some(windmill_common::utils::get_custom_pg_instance_password(db).await?); - let dbname = pg.dbname.clone(); + let pg = match cluster { + DatatableRoleCluster::Instance => { + let mut pg = + PgDatabase::parse_uri(&windmill_common::get_database_url().await?.as_str().await)?; + pg.dbname = dbname.clone(); + pg.user = Some(CUSTOM_INSTANCE_USER.to_string()); + pg.password = Some(windmill_common::utils::get_custom_pg_instance_password(db).await?); + pg + } + DatatableRoleCluster::ExternalInstance => { + windmill_common::external_instance_pg::external_instance_connection_unchecked( + db, &dbname, false, + ) + .await? + } + }; let (client, notices) = connect_with_notices(db, &pg).await?; Ok((client, notices, dbname)) } @@ -1085,12 +1097,12 @@ async fn get_datatable_acl( let target: AclTarget = query.try_into()?; let governing = resolve_governing_datatable(&db, &w_id, &datatable_name).await?; ensure_reaches_governing_datatable(&db, &w_id, &datatable_name, &governing, &authed).await?; - ensure_instance(&governing)?; + let cluster = ensure_managed(&governing)?; let editable = ensure_governs_datatable(&db, &authed, &w_id, &governing) .await .is_ok(); let roles = if editable { - role_names(&read_role_catalog(&db).await?) + role_names(&read_role_catalog(&db, cluster).await?) } else { vec![] }; @@ -1376,7 +1388,7 @@ async fn authorize_acl_change( ) -> Result { let governing = resolve_governing_datatable(db, w_id, datatable_name).await?; ensure_governs_datatable(db, authed, w_id, &governing).await?; - ensure_instance(&governing)?; + ensure_managed(&governing)?; Ok(governing) } @@ -1386,8 +1398,14 @@ static APPLY_SLOT: tokio::sync::Semaphore = tokio::sync::Semaphore::const_new(1) /// provisioned before data table roles gave `custom_instance_user` none. Adds that option to its /// database and `public` privileges, and nothing else: default privileges are left alone, since a /// schema's change of owner is planned against them. Best-effort, as a grant it fails to enable is -/// refused when it runs. -async fn ensure_grant_options(client: &tokio_postgres::Client, db: &DB, dbname: &str) { +/// refused when it runs. An external instance database was created with the options, so one +/// missing there is someone's deliberate revoke and is left alone. +async fn ensure_grant_options( + client: &tokio_postgres::Client, + db: &DB, + cluster: DatatableRoleCluster, + dbname: &str, +) { let held = client .query_one( "SELECT has_database_privilege(current_database(), 'CONNECT WITH GRANT OPTION') @@ -1399,7 +1417,7 @@ async fn ensure_grant_options(client: &tokio_postgres::Client, db: &DB, dbname: ) .await .is_ok_and(|row| row.get::<_, bool>(0)); - if held { + if held || cluster != DatatableRoleCluster::Instance { return; } if let Err(e) = grant_options_as_server(db, dbname).await { @@ -1625,7 +1643,8 @@ async fn plan_datatable_acl( ) -> JsonResult { crate::datatable_acl_oss::ensure_datatable_acl_available()?; let governing = authorize_acl_change(&db, &authed, &w_id, &datatable_name).await?; - let catalog = read_role_catalog(&db).await?; + let cluster = ensure_managed(&governing)?; + let catalog = read_role_catalog(&db, cluster).await?; let (client, _notices, dbname) = connect_as_admin_unchecked(&db, &governing).await?; Ok(Json( build_plan(&client, &dbname, &catalog, &req.target, &req.change).await?, @@ -1649,6 +1668,7 @@ async fn apply_datatable_acl( // connection could wait forever on a pool that concurrent applies, queued on the same locks, // have exhausted. let governing = authorize_acl_change(&db, &authed, &w_id, &datatable_name).await?; + let cluster = ensure_managed(&governing)?; // Applies queue on an instance-wide lock while each holds a direct connection to the instance's // Postgres; unbounded, the queue alone could exhaust its connection limit. One at a time per // server, and the ones waiting hold no connection at all. @@ -1657,7 +1677,7 @@ async fn apply_datatable_acl( .await .map_err(|e| Error::internal_err(format!("ACL apply slot closed: {e}")))?; let (mut client, mut notices, dbname) = connect_as_admin_unchecked(&db, &governing).await?; - ensure_grant_options(&client, &db, &dbname).await; + ensure_grant_options(&client, &db, cluster, &dbname).await; // Held until the change is committed: a role renamed or dropped meanwhile would change what // the plan names, and a settings save could move the entry onto another database. Taken in the @@ -1672,7 +1692,7 @@ async fn apply_datatable_acl( .fetch_optional(&mut *tx) .await? .flatten(); - let catalog = read_role_catalog_tx(&mut tx).await?; + let catalog = read_role_catalog_tx(&mut tx, cluster).await?; let plan = build_plan(&client, &dbname, &catalog, &req.target, &req.change).await?; if !entry_unchanged(&governing, entry_now) || &plan.statements != confirmed { diff --git a/backend/windmill-api-workspaces/src/datatable_clone.rs b/backend/windmill-api-workspaces/src/datatable_clone.rs index a57a2216e5..6083b7b619 100644 --- a/backend/windmill-api-workspaces/src/datatable_clone.rs +++ b/backend/windmill-api-workspaces/src/datatable_clone.rs @@ -11,8 +11,8 @@ //! The fork request makes its copies before it writes the fork: each is created, filled and — for a //! data table under roles — given the source's owners and grants. What each copy was made from //! stays in the request ([`MadeCopy`]), so the fork's transaction checks it against the source as it -//! is then, and a fork that fails drops the instance copies it made ([`drop_copies_after`]) and names -//! the others, which live on servers of the workspace's own. +//! is then, and a fork that fails drops the copies it made on a cluster Windmill manages +//! ([`drop_copies_after`]) and names the others, which live on servers of the workspace's own. use std::collections::BTreeSet; @@ -22,8 +22,8 @@ use windmill_common::error::{pg_error_message, Error, Result}; use windmill_common::utils::require_admin; use windmill_common::worker::CLOUD_HOSTED; use windmill_common::workspaces::{ - get_datatable_resource_from_db_unchecked, DataTableDatabase, DataTableForkBehavior, - GoverningDatatable, + get_datatable_resource_from_db_unchecked, DataTableCatalogResourceType, DataTableDatabase, + DataTableForkBehavior, GoverningDatatable, }; use windmill_common::{PgDatabase, DB}; @@ -312,27 +312,38 @@ pub(crate) async fn drop_copies_after(db: &DB, copies: Vec, error: Err } async fn drop_copy(db: &DB, source_database: &DataTableDatabase, dbname: &str) -> Result<()> { - if source_database.resource_type - == windmill_common::workspaces::DataTableCatalogResourceType::Instance - { - match windmill_common::drop_unused_instance_database(db, dbname).await? { - windmill_common::Cleanup::Dropped => Ok(()), - windmill_common::Cleanup::InUse(users) => Err(Error::BadRequest(format!( - "kept, since workspaces {} now use it", - users.join(", ") - ))), - windmill_common::Cleanup::Waiter(pid) => Err(Error::BadRequest(format!( - "kept, since a request (pid {pid}) is still waiting to name it" - ))), + match source_database.resource_type { + DataTableCatalogResourceType::Instance => { + match windmill_common::drop_unused_instance_database(db, dbname).await? { + windmill_common::Cleanup::Dropped => Ok(()), + windmill_common::Cleanup::InUse(users) => Err(Error::BadRequest(format!( + "kept, since workspaces {} now use it", + users.join(", ") + ))), + windmill_common::Cleanup::Waiter(pid) => Err(Error::BadRequest(format!( + "kept, since a request (pid {pid}) is still waiting to name it" + ))), + } } - } else { + // Refused, and the copy kept, when anything names it by then. + DataTableCatalogResourceType::ExternalInstance => { + let mut tx = db.begin().await?; + windmill_common::external_instance_pg::drop_external_instance_database_unchecked( + &mut tx, dbname, None, + ) + .await?; + tx.commit().await?; + Ok(()) + } + DataTableCatalogResourceType::Postgresql => { // On a server of the workspace's own, where a resource edited meanwhile can already name it // and nothing locks such an edit: dropping it could take someone's data. - Err(Error::BadRequest( - "kept on its PostgreSQL server, where it may already be in use; drop it there once it \ - is not, before forking the same data table under this id again" - .to_string(), - )) + Err(Error::BadRequest( + "kept on its PostgreSQL server, where it may already be in use; drop it there once \ + it is not, before forking the same data table under this id again" + .to_string(), + )) + } } } @@ -349,10 +360,10 @@ async fn make_copy( request.name )) })?; - let is_instance = governing.is_instance(); + let managed = source_database.resource_type.is_windmill_managed(); // Resolved from the snapshot, not read again: the connection the copy is made on is then // exactly what these rows describe, which the fork's own clone of them is checked against. - let (server, connection): (PgDatabase, Option) = if is_instance { + let (server, connection): (PgDatabase, Option) = if managed { let server = serde_json::from_value( get_datatable_resource_from_db_unchecked(db, parent_w_id, request.name).await?, ) @@ -383,22 +394,44 @@ async fn make_copy( // Dumped before the database exists, so a source that cannot be read leaves nothing behind. // Ownership never carries over: the restore runs as the target's connection user. Grants do, - // except on the instance, where the replay below is what reproduces them. + // except on a cluster Windmill manages, which plants its own and where the replay below is what + // reproduces the roles'. let dump = pg_dump_database( &server, PgDumpOptions { schema_only: request.behavior == DataTableForkBehavior::SchemaOnly, no_owner: true, - no_acl: is_instance, + no_acl: managed, ..Default::default() }, ) .await?; - if is_instance { - windmill_common::create_custom_instance_database(db, request.dbname, "datatable").await?; - } else { - create_database_on_server(db, &server, request.dbname).await?; + match source_database.resource_type { + DataTableCatalogResourceType::Instance => { + windmill_common::create_custom_instance_database( + db, + request.dbname, + "datatable", + Some(parent_w_id), + ) + .await? + } + DataTableCatalogResourceType::ExternalInstance => { + let mut tx = db.begin().await?; + windmill_common::external_instance_pg::create_external_instance_database_unchecked( + db, + &mut tx, + request.dbname, + "datatable", + Some(parent_w_id), + ) + .await?; + tx.commit().await?; + } + DataTableCatalogResourceType::Postgresql => { + create_database_on_server(db, &server, request.dbname).await? + } } let target = PgDatabase { dbname: request.dbname.to_string(), ..server.clone() }; @@ -481,11 +514,16 @@ async fn fill( let replayed = now.datatable.permissions.is_some(); if replayed { crate::datatable_replay_oss::ensure_replay()?; - let catalog = read_role_catalog_tx(&mut tx).await?; + // The copy is on the source's cluster, so its roles come from that cluster's catalog. + let cluster = governing + .role_cluster() + .ok_or_else(|| Error::internal_err("a replayed clone is on a managed database"))?; + let catalog = read_role_catalog_tx(&mut tx, cluster).await?; // The replay leaves `CONNECT` to the catalog, and creating the database only tried to set // it: a copy `PUBLIC` could still connect to would admit logins the source turns away. windmill_common::datatable_roles::converge_connect_grants_with( db, + cluster, &target.dbname, &catalog, ) diff --git a/backend/windmill-api-workspaces/src/datatable_migrations.rs b/backend/windmill-api-workspaces/src/datatable_migrations.rs index df1fac203b..f762fda175 100644 --- a/backend/windmill-api-workspaces/src/datatable_migrations.rs +++ b/backend/windmill-api-workspaces/src/datatable_migrations.rs @@ -11,7 +11,7 @@ //! to keep that file focused on core workspace configuration. use crate::workspaces::{ - is_instance_datatable, pg_dump_database, strip_unreplayable_dump_lines, ItemComparison, + managed_datatable_kind, pg_dump_database, strip_unreplayable_dump_lines, ItemComparison, PgDumpOptions, }; @@ -1669,7 +1669,9 @@ async fn generate_initial_datatable_migration( // without what a replay elsewhere cannot run: the replaying user owns none of this // database's objects, and the grants Windmill plants in an instance database (`ALTER // DEFAULT PRIVILEGES FOR ROLE ...`) fail even replaying onto the same server. - let no_acl = is_instance_datatable(&db, &w_id, &datatable_name).await?; + let no_acl = managed_datatable_kind(&db, &w_id, &datatable_name) + .await? + .is_some(); let dump_file = pg_dump_database( &pg_db, PgDumpOptions { diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 185fe17b25..458e63a973 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -2300,7 +2300,8 @@ struct DataTableTables { schemas: TableListMap, #[serde(skip_serializing_if = "Option::is_none")] error: Option, - /// On the instance database: the only kind that can be under roles or have its access edited. + /// On a database Windmill manages, on its own cluster or the external one: the only kinds that + /// can be under roles or have their access edited. instance: bool, permissioned: bool, /// The roles this caller may connect as, by name; empty when not under roles. @@ -2557,7 +2558,7 @@ async fn list_one_datatable_tables( }; let result: Result<()> = async { let governing = resolve_governing_datatable(db, w_id, &entry.datatable_name).await?; - entry.instance = governing.is_instance(); + entry.instance = governing.role_cluster().is_some(); let usable = crate::datatable_permissions_oss::usable_datatable_roles(db, authed, w_id, &governing) .await?; @@ -3082,6 +3083,25 @@ pub(crate) async fn resolve_pg_source_checked( w_id: &str, source: &str, ) -> Result { + Ok( + resolve_pg_source_checked_with_kind(db, user_db, authed, w_id, source) + .await? + .0, + ) +} + +/// [`resolve_pg_source_checked`], also reporting the kind of Windmill-managed database behind the +/// source, `None` for a user resource. Callers that guard a managed connection MUST take the kind +/// from here rather than ask separately: between two reads a save can flip the entry, leaving the guards of one kind +/// applied to the connection of the other. +pub(crate) async fn resolve_pg_source_checked_with_kind( + db: &DB, + user_db: &UserDB, + authed: &ApiAuthed, + w_id: &str, + source: &str, +) -> Result<(PgDatabase, Option)> { + let mut managed_kind = None; let db_resource = if let Some(name) = source.strip_prefix("datatable://") { windmill_common::workspaces::ensure_datatable_admin_access( db, @@ -3090,7 +3110,13 @@ pub(crate) async fn resolve_pg_source_checked( &DatatableAccess::Authed(authed.to_authed_ref()), ) .await?; - get_datatable_resource_from_db_unchecked(db, w_id, name).await? + let (connection, kind) = + windmill_common::workspaces::get_datatable_connection_and_kind_unchecked( + db, w_id, name, + ) + .await?; + managed_kind = kind; + connection } else if let Some(path) = source.strip_prefix("$res:") { let db_with_authed = windmill_common::db::DbWithOptAuthed::from_authed( authed, @@ -3123,27 +3149,37 @@ pub(crate) async fn resolve_pg_source_checked( ))); }; - serde_json::from_value(db_resource) - .map_err(|e| Error::internal_err(format!("Failed to parse database credentials: {}", e))) + let pg: PgDatabase = serde_json::from_value(db_resource) + .map_err(|e| Error::internal_err(format!("Failed to parse database credentials: {}", e)))?; + Ok((pg, managed_kind)) } -/// Whether the data table `name` is backed by the Windmill instance's own PostgreSQL -/// rather than a user resource. -pub(crate) async fn is_instance_datatable(db: &DB, w_id: &str, name: &str) -> Result { +/// The kind of the database backing the data table `name` when Windmill manages it (on its own +/// cluster or the external one), `None` when it is a user resource. +pub(crate) async fn managed_datatable_kind( + db: &DB, + w_id: &str, + name: &str, +) -> Result> { // Resolved rather than read: a pointer entry owns no database of its own, so only the entry it - // lands on can answer. A name that resolves to nothing keeps the historical `false`. + // lands on can answer. A name that resolves to nothing keeps the historical `None`. Ok(resolve_governing_datatable(db, w_id, name) .await .ok() .and_then(|g| g.datatable.database) - .is_some_and(|d| d.resource_type == DataTableCatalogResourceType::Instance)) + .map(|d| d.resource_type) + .filter(|kind| kind.is_windmill_managed())) } /// Same, for the `datatable://` / `$res:` form the import endpoints take. -async fn is_instance_datatable_source(db: &DB, w_id: &str, source: &str) -> Result { +async fn managed_datatable_source_kind( + db: &DB, + w_id: &str, + source: &str, +) -> Result> { match source.strip_prefix("datatable://") { - Some(name) => is_instance_datatable(db, w_id, name).await, - None => Ok(false), + Some(name) => managed_datatable_kind(db, w_id, name).await, + None => Ok(None), } } @@ -3242,13 +3278,7 @@ pub(crate) async fn pg_dump_database( if let Some(ref password) = pg_db.password { cmd.env("PGPASSWORD", password); } - - if let Some(ref sslmode) = pg_db.sslmode { - cmd.env("PGSSLMODE", sslmode); - } - if let Some(options) = pg_db.non_empty_options() { - cmd.env("PGOPTIONS", options); - } + let _root_cert = apply_pg_tls_env(&mut cmd, pg_db)?; let output = cmd .output() @@ -3366,7 +3396,7 @@ async fn comment_out_unsupported_settings( /// A psql invocation against `pg_db`, carrying the connection settings the CLI reads /// from the environment. -fn psql_command(pg_db: &PgDatabase) -> tokio::process::Command { +fn psql_command(pg_db: &PgDatabase) -> Result<(tokio::process::Command, Option)> { let mut cmd = tokio::process::Command::new("psql"); cmd.arg("--host") .arg(&pg_db.host) @@ -3383,13 +3413,92 @@ fn psql_command(pg_db: &PgDatabase) -> tokio::process::Command { if let Some(ref password) = pg_db.password { cmd.env("PGPASSWORD", password); } + let root_cert = apply_pg_tls_env(&mut cmd, pg_db)?; + Ok((cmd, root_cert)) +} + +/// Give libpq the TLS settings `PgDatabase::connect` applies. The returned file holds the root +/// certificate `PGSSLROOTCERT` names, so it must outlive the command. +fn apply_pg_tls_env( + cmd: &mut tokio::process::Command, + pg_db: &PgDatabase, +) -> Result> { if let Some(ref sslmode) = pg_db.sslmode { cmd.env("PGSSLMODE", sslmode); } if let Some(options) = pg_db.non_empty_options() { cmd.env("PGOPTIONS", options); } - cmd + if let Some(pem) = pg_db + .root_certificate_pem + .as_deref() + .filter(|p| !p.is_empty()) + { + let file = DumpFile::new()?; + std::fs::write(&file.path, pem) + .map_err(|e| Error::internal_err(format!("Failed to write root certificate: {e}")))?; + cmd.env("PGSSLROOTCERT", &file.path); + return Ok(Some(file)); + } + // Only a connection that asked to be verified against the system trust store. Without a file, + // libpq's own default would look for `~/.postgresql/root.crt` and refuse a verify-* mode. libpq + // takes the special `system` value with verify-full only, so verify-ca needs the bundle itself. + if pg_db.accept_invalid_certs == Some(false) { + match pg_db.sslmode.as_deref() { + Some("verify-full") => { + cmd.env("PGSSLROOTCERT", "system"); + } + Some("verify-ca") => { + if let Some(bundle) = windmill_common::system_ca_bundle() { + cmd.env("PGSSLROOTCERT", bundle); + } + } + _ => {} + } + } + Ok(None) +} + + +#[cfg(test)] +mod pg_tls_env_tests { + use super::apply_pg_tls_env; + use windmill_common::PgDatabase; + + fn root_cert_env(sslmode: &str) -> Option { + let pg_db = PgDatabase { + host: "db".to_string(), + user: None, + password: None, + port: None, + sslmode: Some(sslmode.to_string()), + dbname: "d".to_string(), + root_certificate_pem: None, + accept_invalid_certs: Some(false), + use_iam_auth: None, + region: None, + options: None, + }; + let mut cmd = tokio::process::Command::new("psql"); + apply_pg_tls_env(&mut cmd, &pg_db).unwrap(); + cmd.as_std() + .get_envs() + .find(|(k, _)| *k == "PGSSLROOTCERT") + .and_then(|(_, v)| v.map(|v| v.to_os_string())) + } + + #[test] + fn system_roots_only_through_verify_full() { + assert_eq!( + root_cert_env("verify-full").as_deref(), + Some("system".as_ref()) + ); + // libpq refuses `sslrootcert=system` with verify-ca, which would fail every dump and restore. + assert_ne!( + root_cert_env("verify-ca").as_deref(), + Some("system".as_ref()) + ); + } } /// GUC names the server backing `pg_db` knows about. @@ -3399,7 +3508,8 @@ fn psql_command(pg_db: &PgDatabase) -> tokio::process::Command { /// and an unset mode, where `PgDatabase::connect` would hand a TLS-only server a /// plaintext socket and fail before the import ever starts. async fn server_setting_names(pg_db: &PgDatabase) -> Result> { - let output = psql_command(pg_db) + let (mut cmd, _root_cert) = psql_command(pg_db)?; + let output = cmd .arg("--tuples-only") .arg("--no-align") .arg("--command") @@ -3436,7 +3546,8 @@ pub(crate) async fn pg_import_dump(target_db: &PgDatabase, dump_file: &DumpFile) let supported_settings = server_setting_names(target_db).await?; comment_out_unsupported_settings(dump_file, &supported_settings).await?; - let output = psql_command(target_db) + let (mut cmd, _root_cert) = psql_command(target_db)?; + let output = cmd .arg("--set") .arg("ON_ERROR_STOP=1") .arg("--single-transaction") @@ -3494,9 +3605,39 @@ async fn create_pg_database( } } - if is_instance_datatable_source(&db, &w_id, &req.source).await? { - windmill_common::create_custom_instance_database(&db, &req.target_dbname, "datatable") + if let Some(source_kind) = managed_datatable_source_kind(&db, &w_id, &req.source).await? { + // Held until the copy is registered, as a rename migrates reservations to the new id under + // it once the old one is archived: a copy registered after that would be reserved for an + // id nothing answers on. + let mut tx = db.begin().await?; + windmill_common::workspaces::lock_fork_datatables(&mut tx, &w_id).await?; + let live = sqlx::query_scalar::<_, bool>("SELECT NOT deleted FROM workspace WHERE id = $1") + .bind(&w_id) + .fetch_optional(&mut *tx) + .await? + .unwrap_or(false); + if !live { + return Err(Error::BadRequest(format!("Workspace '{w_id}' is archived"))); + } + if source_kind == DataTableCatalogResourceType::ExternalInstance { + windmill_common::external_instance_pg::create_external_instance_database_unchecked( + &db, + &mut tx, + &req.target_dbname, + "datatable", + Some(&w_id), + ) .await?; + } else { + windmill_common::create_custom_instance_database( + &db, + &req.target_dbname, + "datatable", + Some(&w_id), + ) + .await?; + } + tx.commit().await?; } else { let source_pg = resolve_pg_source_checked(&db, &user_db, &authed, &w_id, &req.source).await?; @@ -3583,12 +3724,12 @@ pub(crate) async fn ensure_datatable_is_clonable( let governing = resolve_governing_datatable(db, w_id, name).await?; // The copy has to name a database of its own. A resource-backed entry reached through a // pointer names one this workspace does not own, so there is nothing here to repoint. - let is_instance = governing + let is_managed = governing .datatable .database .as_ref() - .is_some_and(|d| d.resource_type == DataTableCatalogResourceType::Instance); - if governing.workspace_id != w_id && !is_instance { + .is_some_and(|d| d.resource_type.is_windmill_managed()); + if governing.workspace_id != w_id && !is_managed { return Err(Error::BadRequest(format!( "Data table '{name}' points at a resource-backed data table in another workspace \ and cannot be copied; fork it from the workspace that owns it." @@ -3642,9 +3783,11 @@ async fn import_pg_database( } let schema_only = req.fork_behavior == DataTableForkBehavior::SchemaOnly; - let source_pg = resolve_pg_source_checked(&db, &user_db, &authed, &w_id, &req.source).await?; - let mut target_pg = - resolve_pg_source_checked(&db, &user_db, &authed, &w_id, &req.target).await?; + let mut fork_lock: Option> = None; + let (source_pg, source_kind) = + resolve_pg_source_checked_with_kind(&db, &user_db, &authed, &w_id, &req.source).await?; + let (mut target_pg, target_kind) = + resolve_pg_source_checked_with_kind(&db, &user_db, &authed, &w_id, &req.target).await?; if let Some(ref override_dbname) = req.target_dbname_override { if !windmill_api_auth::is_super_admin_authed(&db, &authed).await? { @@ -3654,6 +3797,29 @@ async fn import_pg_database( .to_string(), )); } + // The kind the connection above was built from, never a second read: an entry flipped + // to a resource between the two would keep the managed connection here and lose the + // check that decides which copy it may fill. + if let Some(kind) = target_kind { + // Held until the restore is done: fork finalization takes the first, and every + // save newly naming a database, in any workspace, the second. Nothing may start + // using this database while `psql` is still filling it. + let mut tx = db.begin().await?; + windmill_common::workspaces::lock_fork_datatables(&mut tx, &w_id).await?; + windmill_common::datatable_roles::lock_instance_databases_governance( + &mut tx, + [override_dbname.as_str()], + ) + .await?; + windmill_common::ensure_fork_database_available_to( + &mut tx, + kind, + override_dbname, + &w_id, + ) + .await?; + fork_lock = Some(tx); + } } target_pg.dbname = override_dbname.clone(); } @@ -3663,8 +3829,7 @@ async fn import_pg_database( // what it creates it owns. Grants do, except around an instance data table — Windmill // plants `custom_instance_user` grants in one, which nothing else can replay. Elsewhere // the ACLs are user intent (`REVOKE ... FROM PUBLIC`) and dropping them widens access. - let no_acl = is_instance_datatable_source(&db, &w_id, &req.target).await? - || is_instance_datatable_source(&db, &w_id, &req.source).await?; + let no_acl = target_kind.is_some() || source_kind.is_some(); let dump_file = pg_dump_database( &source_pg, @@ -3672,6 +3837,9 @@ async fn import_pg_database( ) .await?; pg_import_dump(&target_pg, &dump_file).await?; + if let Some(tx) = fork_lock { + tx.commit().await?; + } Ok(format!( "Imported from '{}' into '{}'", @@ -3772,39 +3940,76 @@ async fn edit_ducklake_config( ) .await?; - let old_ducklakes = sqlx::query_scalar!( - r#" - SELECT ws.ducklake->'ducklakes' AS ducklake_name - FROM workspace_settings ws - WHERE ws.workspace_id = $1 - "#, - &w_id + // Under the row lock the save writes with, taken before the database locks below as fork + // cleanup takes the two. + let old_ducklakes = sqlx::query_scalar::<_, Option>( + "SELECT ws.ducklake->'ducklakes' FROM workspace_settings ws + WHERE ws.workspace_id = $1 FOR UPDATE", ) + .bind(&w_id) .fetch_one(&mut *tx) .await? .unwrap_or(serde_json::Value::Null); let old_ducklakes: HashMap = serde_json::from_value(old_ducklakes).unwrap_or_default(); - // Check that non-superadmins are not abusing Instance databases - if !is_superadmin { - for (name, dl) in new_config.settings.ducklakes.iter() { - if dl.catalog.resource_type == DucklakeCatalogResourceType::Instance { - let old_dl = old_ducklakes.get(name); - if old_dl.is_none() - || old_dl.unwrap().catalog.resource_type - != DucklakeCatalogResourceType::Instance - || old_dl.unwrap().catalog.resource_path != dl.catalog.resource_path - { - return Err(Error::BadRequest( - "Only superadmins can create or modify ducklakes with Instance databases" - .to_string(), - )); - } - } + // Check that non-superadmins are not abusing Instance databases. An unchanged catalog is left + // alone either way, so a downgraded instance can still save lakes that already name an + // external instance database. + for (name, dl) in new_config.settings.ducklakes.iter() { + let kind = &dl.catalog.resource_type; + if !matches!( + kind, + DucklakeCatalogResourceType::Instance | DucklakeCatalogResourceType::ExternalInstance + ) { + continue; + } + let unchanged = old_ducklakes.get(name).is_some_and(|old| { + &old.catalog.resource_type == kind + && old.catalog.resource_path == dl.catalog.resource_path + }); + if unchanged { + continue; + } + // Before the registration check, whose refusal would otherwise tell a workspace admin + // which databases exist on the cluster. + if !is_superadmin { + return Err(Error::BadRequest( + "Only superadmins can create or modify ducklakes with Instance databases" + .to_string(), + )); + } + if *kind == DucklakeCatalogResourceType::ExternalInstance { + windmill_common::external_instance_pg::ensure_external_instance_available()?; + windmill_common::external_instance_pg::ensure_external_instance_database_registered( + &mut tx, + &dl.catalog.resource_path, + ) + .await?; + } else { + windmill_common::workspaces::ensure_instance_pg_available(&mut *tx).await?; } } + // Fork cleanup decides nothing uses an instance database under this lock, so a catalog newly + // put on one must not commit between its check and its drop. + windmill_common::datatable_roles::lock_instance_databases_governance( + &mut *tx, + new_config + .settings + .ducklakes + .iter() + .filter(|(name, dl)| { + dl.catalog.resource_type == DucklakeCatalogResourceType::Instance + && old_ducklakes.get(name.as_str()).is_none_or(|old| { + old.catalog.resource_type != DucklakeCatalogResourceType::Instance + || old.catalog.resource_path != dl.catalog.resource_path + }) + }) + .map(|(_, dl)| dl.catalog.resource_path.as_str()), + ) + .await?; + let config: serde_json::Value = serde_json::to_value(&new_config.settings) .map_err(|err| Error::internal_err(err.to_string()))?; @@ -3860,6 +4065,8 @@ async fn edit_datatable_config( let is_superadmin = require_super_admin(&db, &authed).await.is_ok(); let mut tx = db.begin().await?; + // Ahead of the settings row, as fork cleanup of this workspace takes the two. + windmill_common::workspaces::lock_fork_datatables(&mut tx, &w_id).await?; windmill_common::lock_instance_databases( &mut tx, new_config.settings.datatables.values().filter_map(|dt| { @@ -3983,6 +4190,7 @@ async fn edit_datatable_config( // so these line up with the `datatable_configured` adoption counts. created_substrates.push(match dt.database.as_ref().map(|d| d.resource_type) { Some(DataTableCatalogResourceType::Instance) => "instance", + Some(DataTableCatalogResourceType::ExternalInstance) => "external_instance", Some(DataTableCatalogResourceType::Postgresql) => "postgresql", None => "reference", }); @@ -4018,18 +4226,20 @@ async fn edit_datatable_config( ))); } // Carrying the block onto a resource-backed entry would produce a data table the chokepoint - // refuses on every job — a save that succeeds and breaks everything afterwards. Refuse it - // instead: turning roles off first is one step, and it keeps discarding an access decision - // something somebody chose rather than a side effect of moving a database. + // refuses on every job — a save that succeeds and breaks everything afterwards — and onto + // the other managed cluster, one whose role ids name nothing in that cluster's catalog. + // Refuse it instead: turning roles off first is one step, and it keeps discarding an access + // decision something somebody chose rather than a side effect of moving a database. + let old_kind = old.and_then(|old| old.database.as_ref()).map(|d| d.resource_type); if dt.permissions.is_some() && dt .database .as_ref() - .is_some_and(|d| d.resource_type != DataTableCatalogResourceType::Instance) + .is_some_and(|d| Some(d.resource_type) != old_kind) { return Err(Error::BadRequest(format!( - "Data table '{name}' is under roles, which only a data table on the instance \ - database can be. Turn its roles off before moving it to a PostgreSQL resource." + "Data table '{name}' is under roles, which belong to the cluster its database is \ + on. Turn its roles off before moving it to another kind of database." ))); } // A pointer names no database of its own, so the form's empty `database` is correct there. @@ -4054,35 +4264,52 @@ async fn edit_datatable_config( // Check that non-superadmins are not abusing Instance databases, which reach a database this // workspace does not own. Pointing an entry at another workspace's data table is not checked // here because it cannot be requested at all: `reference` is overwritten from the stored entry - // above, for every caller. + // above, for every caller. An unchanged entry is left alone either way, so a downgraded + // instance can still save settings that already name an external instance database. // // Compared against the entry the carried fields came from, not the one stored under the same // name: otherwise swapping two names keeps each database in place while moving a clone's // `governed_by` off the copy it governs. - if !is_superadmin { - for (name, dt) in new_config.settings.datatables.iter() { - let old_dt = old_datatables.get( + for (name, dt) in new_config.settings.datatables.iter() { + let Some(database) = dt + .database + .as_ref() + .filter(|d| d.resource_type.is_windmill_managed()) + else { + continue; + }; + let unchanged = old_datatables + .get( rename_src .get(name.as_str()) .copied() .unwrap_or(name.as_str()), - ); - if dt - .database - .as_ref() - .is_some_and(|d| d.resource_type == DataTableCatalogResourceType::Instance) - { - let unchanged = old_dt.and_then(|o| o.database.as_ref()).is_some_and(|o| { - o.resource_type == DataTableCatalogResourceType::Instance - && Some(&o.resource_path) == dt.database.as_ref().map(|d| &d.resource_path) - }); - if !unchanged { - return Err(Error::BadRequest( - "Only superadmins can create or modify data tables with Instance databases" - .to_string(), - )); - } - } + ) + .and_then(|o| o.database.as_ref()) + .is_some_and(|o| { + o.resource_type == database.resource_type + && o.resource_path == database.resource_path + }); + if unchanged { + continue; + } + // Before the registration check, whose refusal would otherwise tell a workspace admin + // which databases exist on the cluster. + if !is_superadmin { + return Err(Error::BadRequest( + "Only superadmins can create or modify data tables with Instance databases" + .to_string(), + )); + } + if database.resource_type == DataTableCatalogResourceType::ExternalInstance { + windmill_common::external_instance_pg::ensure_external_instance_available()?; + windmill_common::external_instance_pg::ensure_external_instance_database_registered( + &mut tx, + &database.resource_path, + ) + .await?; + } else { + windmill_common::workspaces::ensure_instance_pg_available(&mut *tx).await?; } } @@ -4102,7 +4329,7 @@ async fn edit_datatable_config( // entry through a declared rename alone, and a settings sync never declares one, so an entry // without roles that newly points at such a database — a name added, or an existing one // repointed — would answer everyone there as `admin`. That holds whichever workspace governs it. - let newly_pointed: Vec<(&String, &str)> = new_config + let newly_pointed: Vec<(&String, DataTableCatalogResourceType, &str)> = new_config .settings .datatables .iter() @@ -4112,7 +4339,7 @@ async fn edit_datatable_config( let db = dt .database .as_ref() - .filter(|d| d.resource_type == DataTableCatalogResourceType::Instance)?; + .filter(|d| d.resource_type.is_windmill_managed())?; let lookup = rename_src .get(name.as_str()) .copied() @@ -4124,38 +4351,72 @@ async fn edit_datatable_config( old_db.resource_type != db.resource_type || old_db.resource_path != db.resource_path }); - repointed.then_some((name, db.resource_path.as_str())) + repointed.then_some((name, db.resource_type, db.resource_path.as_str())) }) .collect(); // Another workspace turning roles on for the same database holds only its own settings row, so - // without this the scan below could read past its uncommitted write. + // without this the scan below could read past its uncommitted write. Every managed database + // this save newly names is locked, not just the ones the scan is about: fork cleanup takes the + // same lock to decide nothing uses the database it is dropping. + let newly_named: std::collections::BTreeSet<&str> = new_config + .settings + .datatables + .iter() + .filter_map(|(name, dt)| { + let db = dt + .database + .as_ref() + .filter(|d| d.resource_type == DataTableCatalogResourceType::Instance)?; + let lookup = rename_src + .get(name.as_str()) + .copied() + .unwrap_or(name.as_str()); + old_datatables + .get(lookup) + .and_then(|old| old.database.as_ref()) + .is_none_or(|old_db| { + old_db.resource_type != db.resource_type + || old_db.resource_path != db.resource_path + }) + .then_some(db.resource_path.as_str()) + }) + .collect(); windmill_common::datatable_roles::lock_instance_databases_governance( &mut *tx, - newly_pointed.iter().map(|(_, dbname)| *dbname), + newly_pointed + .iter() + .map(|(_, _, dbname)| *dbname) + .chain(newly_named.iter().copied()), ) .await?; - let governed_elsewhere: Vec = if newly_pointed.is_empty() { + let governed_elsewhere: Vec<(String, String)> = if newly_pointed.is_empty() { vec![] } else { - sqlx::query_scalar( - "SELECT DISTINCT dt.value->'database'->>'resource_path' FROM workspace_settings ws + sqlx::query_as( + "SELECT DISTINCT dt.value->'database'->>'resource_type', + dt.value->'database'->>'resource_path' + FROM workspace_settings ws CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt WHERE ws.workspace_id <> $1 AND (dt.value ? 'permissions' OR dt.value ? 'governed_by') - AND dt.value->'database'->>'resource_type' = 'instance'", + AND dt.value->'database'->>'resource_type' IN ('instance', 'external_instance')", ) .bind(&w_id) .fetch_all(&mut *tx) .await? }; - for (name, dbname) in newly_pointed { + for (name, kind, dbname) in newly_pointed { let governed_here = old_datatables.values().any(|old| { (old.permissions.is_some() || old.governed_by.is_some()) - && old.database.as_ref().is_some_and(|d| { - d.resource_type == DataTableCatalogResourceType::Instance - && d.resource_path == dbname - }) + && old + .database + .as_ref() + .is_some_and(|d| d.resource_type == kind && d.resource_path == dbname) }); - if governed_here || governed_elsewhere.iter().any(|g| g == dbname) { + if governed_here + || governed_elsewhere + .iter() + .any(|(k, p)| k == kind.as_ref() && p == dbname) + { return Err(Error::BadRequest(format!( "Data table '{name}' would point at database '{dbname}', which a data table under \ roles uses, without carrying those roles: everyone reaching '{name}' would connect \ @@ -8429,13 +8690,14 @@ async fn point_kept_datatables_at_parent( if dt.reference.is_some() { continue; } - // Only instance databases. A resource-backed data table names a resource, and the settings - // clone gave the fork its own copy of that resource in its own workspace — pointing at the - // parent's entry would silently move the fork onto the parent's resource instead. + // Only instance databases, on either cluster. A resource-backed data table names a + // resource, and the settings clone gave the fork its own copy of that resource in its own + // workspace — pointing at the parent's entry would silently move the fork onto the + // parent's resource instead. if dt .database .as_ref() - .is_none_or(|d| d.resource_type != DataTableCatalogResourceType::Instance) + .is_none_or(|d| !d.resource_type.is_windmill_managed()) { continue; } @@ -8609,11 +8871,39 @@ async fn apply_forked_datatable( })?, }; - if database.resource_type == DataTableCatalogResourceType::Instance { + if database.resource_type == DataTableCatalogResourceType::ExternalInstance { + windmill_common::external_instance_pg::ensure_external_instance_database_registered( + tx, + &fdt.new_dbname, + ) + .await?; + } + if database.resource_type.is_windmill_managed() { + // Held until the fork commits, as every save newly naming a database takes it: none may + // claim the copy between the check below and this fork's entry landing on it. + windmill_common::datatable_roles::lock_instance_databases_governance( + &mut **tx, + [fdt.new_dbname.as_str()], + ) + .await?; + } + if database.resource_type.is_windmill_managed() + && !windmill_api_auth::is_super_admin_authed(db, authed).await? + { + windmill_common::ensure_fork_database_available_to( + &mut **tx, + database.resource_type, + &fdt.new_dbname, + parent_w_id, + ) + .await?; + } + if database.resource_type.is_windmill_managed() { // The whole `database` object, not just its `resource_path`: a pointer entry has none to - // patch. `reference` goes with it — exactly one of the two may be set. + // patch. `reference` goes with it — exactly one of the two may be set. The copy was created + // on the same cluster as its source, so it keeps the source's kind. let new_database = serde_json::json!({ - "resource_type": "instance", + "resource_type": database.resource_type, "resource_path": &fdt.new_dbname, }); sqlx::query( @@ -9184,6 +9474,11 @@ async fn write_workspace_fork( .bind(&nw.id) .execute(&mut *tx) .await?; + // Before the settings clone reads the parent's data tables: a pointer this fork ends up with + // must not be written after cleanup of the parent decided that nothing points at its copies. + // Also before the external cluster's lifecycle lock, which finalizing an external copy takes: + // fork cleanup takes the two in this order. + windmill_common::workspaces::lock_fork_datatables(&mut tx, &parent_workspace_id).await?; if nw.is_dev_workspace { // The checks above ran outside a transaction, so the parent's eligibility and the chain's @@ -9328,6 +9623,32 @@ async fn write_workspace_fork( ))); } } + // Saves name an external database under the external cluster's lifecycle lock instead. + let external_copies: Vec<&str> = copies + .iter() + .filter(|c| { + c.source_database.resource_type == DataTableCatalogResourceType::ExternalInstance + }) + .map(|c| c.dbname.as_str()) + .collect(); + if !external_copies.is_empty() { + windmill_common::external_instance_pg::lock_external_instance_pg_state(&mut tx).await?; + } + for dbname in &external_copies { + let uses = windmill_common::workspaces::managed_database_uses( + &mut tx, + DataTableCatalogResourceType::ExternalInstance, + dbname, + None, + ) + .await?; + if !uses.is_empty() { + return Err(Error::BadRequest(format!( + "Database '{dbname}' copied for this fork is already used by {}; fork again", + uses.join(", ") + ))); + } + } // Update forked datatable settings to point to new databases for fdt in &nw.forked_datatables { diff --git a/backend/windmill-api-workspaces/src/workspaces_extra.rs b/backend/windmill-api-workspaces/src/workspaces_extra.rs index 9039bda2ef..8ca98f66d1 100644 --- a/backend/windmill-api-workspaces/src/workspaces_extra.rs +++ b/backend/windmill-api-workspaces/src/workspaces_extra.rs @@ -56,6 +56,11 @@ pub(crate) async fn change_workspace_id( let mut tx = db.begin().await?; + // The settings copy below carries every data table entry to the new id, which fork cleanup of + // the old id cannot see until this commits: without the lock it could drop a copy the renamed + // workspace goes on using. Before the pairing lock, as forking takes the two in that order. + windmill_common::workspaces::lock_fork_datatables(&mut tx, &old_id).await?; + // A rename rewrites the workspace's dev flag and reparents its children, so it decides on the // same state the pairing handlers do: without this lock a concurrent create/attach could commit // an active dev workspace under the shell this rename is about to archive. Both ids, since the @@ -869,6 +874,10 @@ pub(crate) async fn change_workspace_id( } } + // After every workspace_settings write above: fork cleanup locks a settings row before the + // registry, so taking the registry first here would deadlock with it. + migrate_fork_reservations(&mut tx, &old_id, &rw.new_id).await?; + // Audit log in the same transaction as the workspace changes audit_log( &mut *tx, @@ -937,6 +946,14 @@ pub(crate) async fn change_workspace_id( let (_schedules_count, canceled_count, _deleted_tokens_count) = archive_workspace_impl(&db, &old_id, &authed.username, None).await?; + // The old id stays live between the commit above and the archive, and a fork copy created for + // it in that window registers under it. Creation checks the workspace is live under the fork + // lock, so once this has run under it, no copy can be reserved for the old id any more. + let mut tx = db.begin().await?; + windmill_common::workspaces::lock_fork_datatables(&mut tx, &old_id).await?; + migrate_fork_reservations(&mut tx, &old_id, &rw.new_id).await?; + tx.commit().await?; + info!( "Workspace id change completed: moved {} to {}, archived old workspace", old_id, rw.new_id @@ -948,6 +965,50 @@ pub(crate) async fn change_workspace_id( )) } +/// A fork copy reserved for the old id would otherwise be unreachable: its creator cannot import +/// into it or finish its fork under the new id, and nothing else would ever drop it. +async fn migrate_fork_reservations( + tx: &mut Transaction<'_, Postgres>, + old_id: &str, + new_id: &str, +) -> Result<()> { + const MIGRATE: &str = r#"UPDATE global_settings SET value = jsonb_set(value, '{databases}', ( + SELECT COALESCE(jsonb_object_agg(k, CASE WHEN v->>'workspace_id' = $1 + THEN jsonb_set(v, '{workspace_id}', to_jsonb($2::text)) ELSE v END), '{}'::jsonb) + FROM jsonb_each(COALESCE(value->'databases', '{}'::jsonb)) AS e(k, v) + )) + WHERE name = $3"#; + sqlx::query(MIGRATE) + .bind(old_id) + .bind(new_id) + .bind("custom_instance_pg_databases") + .execute(&mut **tx) + .await?; + // External copies are registered in the cluster state, which its writers rewrite whole under + // the lifecycle lock. Only taken when there is something to move, as setup can hold it for as + // long as the cluster takes to answer. + let state = windmill_common::global_settings::EXTERNAL_INSTANCE_PG_STATE_SETTING; + let reserved = sqlx::query_scalar::<_, bool>( + "SELECT EXISTS (SELECT 1 FROM global_settings, jsonb_each(value->'databases') AS e(k, v) + WHERE name = $1 AND jsonb_typeof(value->'databases') = 'object' + AND v->>'workspace_id' = $2)", + ) + .bind(state) + .bind(old_id) + .fetch_one(&mut **tx) + .await?; + if reserved { + windmill_common::external_instance_pg::lock_external_instance_pg_state(tx).await?; + sqlx::query(MIGRATE) + .bind(old_id) + .bind(new_id) + .bind(state) + .execute(&mut **tx) + .await?; + } + Ok(()) +} + #[derive(Deserialize)] pub(crate) struct DeleteWorkspaceQuery { pub(crate) only_delete_forks: Option, @@ -1430,9 +1491,7 @@ pub async fn drop_forked_datatable_databases( _ => continue, }; - if database.resource_type - == windmill_common::workspaces::DataTableCatalogResourceType::Instance - { + if database.resource_type.is_windmill_managed() { let db_to_drop = &database.resource_path; if !db_to_drop.starts_with("wm_fork_") { errors.push(format!( @@ -1441,7 +1500,112 @@ pub async fn drop_forked_datatable_databases( )); continue; } - if let Err(e) = windmill_common::drop_custom_instance_database(&db, db_to_drop).await { + // The fork's own entry is what is going away; anything else still reaching the copy, + // a child fork's pointer at this entry included, keeps it. The lock keeps a child fork + // from gaining such a pointer before the drop. + // A task of its own, so a client going away cannot stop it between dropping the + // database and committing the entry's removal. + let dropped = tokio::spawn({ + let (db, w_id, dt_name, db_to_drop) = ( + db.clone(), + w_id.clone(), + dt_name.clone(), + db_to_drop.clone(), + ); + let resource_type = database.resource_type; + async move { + let mut tx = db.begin().await?; + // The three locks a settings save takes, in its order: this workspace's data + // tables, its settings row, and the database itself. Without them a save could + // rename this entry, or point another one here, either side of the check below. + windmill_common::workspaces::lock_fork_datatables(&mut tx, &w_id).await?; + // The snapshot above was read unlocked: a save committing since could have + // repointed this entry, and the entry is removed below whatever it names by then. + let current = sqlx::query_scalar::<_, Option>( + "SELECT datatable->'datatables'->$2 FROM workspace_settings + WHERE workspace_id = $1 FOR UPDATE", + ) + .bind(&w_id) + .bind(&dt_name) + .fetch_optional(&mut *tx) + .await? + .flatten() + .and_then(|v| serde_json::from_value::(v).ok()); + if !current.is_some_and(|dt| { + dt.forked_from.is_some() + && dt.database.is_some_and(|d| { + d.resource_type == resource_type && d.resource_path == db_to_drop + }) + }) { + return Err(Error::BadRequest( + "the data table changed while it was being cleaned up".to_string(), + )); + } + let external = resource_type + == windmill_common::workspaces::DataTableCatalogResourceType::ExternalInstance; + if external { + // Before the governance lock, in the order settings saves and fork + // finalization take the two. + windmill_common::external_instance_pg::lock_external_instance_pg_state( + &mut tx, + ) + .await?; + } + windmill_common::datatable_roles::lock_instance_databases_governance( + &mut tx, + [db_to_drop.as_str()], + ) + .await?; + // Before the entry is removed: child fork pointers are found through it. + let uses = windmill_common::workspaces::managed_database_uses( + &mut tx, + resource_type, + &db_to_drop, + Some((w_id.as_str(), dt_name.as_str())), + ) + .await?; + if !uses.is_empty() { + return Err(Error::BadRequest(format!( + "it is still used by {}", + uses.join(", ") + ))); + } + // The entry goes with the database: a fork this one is cloned into afterwards + // must not inherit a pointer at a data table whose database is gone. + sqlx::query( + "UPDATE workspace_settings SET datatable = datatable #- ARRAY['datatables', $2] + WHERE workspace_id = $1", + ) + .bind(&w_id) + .bind(&dt_name) + .execute(&mut *tx) + .await?; + if external { + // Checks the uses of the database itself, and unregisters it. + windmill_common::external_instance_pg::drop_external_instance_database_unchecked( + &mut tx, + &db_to_drop, + Some((w_id.as_str(), dt_name.as_str())), + ) + .await?; + } else { + windmill_common::drop_custom_instance_database_keep_entry(&db, &db_to_drop) + .await?; + sqlx::query( + "UPDATE global_settings SET value = value #- ARRAY['databases', $1] + WHERE name = 'custom_instance_pg_databases'", + ) + .bind(&db_to_drop) + .execute(&mut *tx) + .await?; + } + tx.commit().await?; + Ok::<_, Error>(()) + } + }) + .await + .unwrap_or_else(|e| Err(Error::internal_err(format!("cleanup task failed: {e}")))); + if let Err(e) = dropped { errors.push(format!( "Could not drop instance database '{}' for datatable://{}: {}", db_to_drop, dt_name, e @@ -1808,7 +1972,17 @@ async fn resolve_fork_catalog_pg( "ducklake://{ducklake_name}: malformed registry catalog identity `{catalog}`" )) })?; - let catalog_resource = if resource_type == "instance" { + let catalog_resource = if resource_type == "external_instance" { + serde_json::to_value( + windmill_common::external_instance_pg::external_instance_connection_unchecked( + db, + resource_path, + false, + ) + .await?, + ) + .map_err(|e| Error::internal_err(format!("serializing pg creds: {e}")))? + } else if resource_type == "instance" { let mut pg_creds = windmill_common::PgDatabase::parse_uri( &windmill_common::get_database_url().await?.as_str().await, )?; diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 4a1ce62bad..cf373241d4 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -1572,6 +1572,104 @@ paths: schema: type: object + /settings/external_instance_pg/status: + get: + summary: Returns whether the external instance cluster is configured and how its last setup went + operationId: getExternalInstancePgStatus + tags: + - setting + responses: + "200": + description: external instance cluster status + content: + application/json: + schema: + $ref: "#/components/schemas/ExternalInstancePgStatus" + + /settings/external_instance_pg/setup: + post: + summary: Sets up the external instance cluster with its saved admin login, optionally rotating the passwords Windmill manages on it (enterprise edition only) + operationId: setupExternalInstancePg + tags: + - setting + requestBody: + required: true + content: + application/json: + schema: + type: object + properties: + rotate_passwords: + type: boolean + responses: + "200": + description: the setup report, also stored as the last setup + content: + application/json: + schema: + $ref: "#/components/schemas/ExternalInstancePgSetupReport" + + /settings/external_instance_pg/databases: + get: + summary: Lists the databases Windmill created on the external instance cluster, with the workspaces whose data tables, Ducklake catalogs or pending fork cleanups use each + operationId: listExternalInstancePgDatabases + tags: + - setting + responses: + "200": + description: databases by name + content: + application/json: + schema: + type: object + additionalProperties: + $ref: "#/components/schemas/CustomInstanceDb" + + /settings/external_instance_pg/databases/{name}: + post: + summary: Creates a database on the external instance cluster (enterprise edition only) + operationId: createExternalInstancePgDatabase + tags: + - setting + parameters: + - name: name + in: path + required: true + schema: + type: string + requestBody: + required: true + content: + application/json: + schema: + type: object + properties: + tag: + $ref: "#/components/schemas/CustomInstanceDbTag" + responses: + "200": + description: database created + content: + application/json: + schema: {} + delete: + summary: Drops a database Windmill created on the external instance cluster, refused while a data table, Ducklake catalog or pending fork cleanup uses it + operationId: dropExternalInstancePgDatabase + tags: + - setting + parameters: + - name: name + in: path + required: true + schema: + type: string + responses: + "200": + description: database dropped + content: + application/json: + schema: {} + /settings/list_custom_instance_pg_databases: post: summary: Returns the set-up statuses of custom instance pg databases @@ -1590,13 +1688,19 @@ paths: /settings/datatable_roles: get: - summary: list the instance's data table roles + summary: list the data table roles of one Windmill-managed Postgres cluster operationId: listInstanceDatatableRoles tags: - setting + parameters: + - in: query + name: cluster + required: false + schema: + $ref: "#/components/schemas/DatatableRoleCluster" responses: "200": - description: the instance role catalog + description: the cluster's role catalog content: application/json: schema: @@ -1604,7 +1708,7 @@ paths: items: $ref: "#/components/schemas/InstanceDatatableRole" post: - summary: create a data table role on the instance's Postgres cluster + summary: create a data table role on a Windmill-managed Postgres cluster operationId: createInstanceDatatableRole tags: - setting @@ -1618,6 +1722,8 @@ paths: properties: name: type: string + cluster: + $ref: "#/components/schemas/DatatableRoleCluster" responses: "200": description: the created role @@ -5351,7 +5457,7 @@ paths: type: string resource_type: type: string - enum: [postgres, instance] + enum: [postgres, instance, external_instance] resource_path: type: string governing_workspace_id: @@ -34048,9 +34154,55 @@ components: - ducklake - datatable + ExternalInstancePgSetupStep: + type: object + required: [name, status, message] + properties: + name: + type: string + status: + type: string + enum: [ok, warning, error] + message: + type: string + + ExternalInstancePgSetupReport: + type: object + required: [success, finished_at, steps] + properties: + success: + type: boolean + description: no step failed; warnings leave it true + finished_at: + type: string + format: date-time + steps: + type: array + items: + $ref: "#/components/schemas/ExternalInstancePgSetupStep" + + ExternalInstancePgStatus: + type: object + required: [configured, database_count] + properties: + configured: + type: boolean + database_count: + type: integer + last_setup: + $ref: "#/components/schemas/ExternalInstancePgSetupReport" + + DatatableRoleCluster: + type: string + description: >- + The Windmill-managed Postgres cluster a data table role is a login on: Windmill's own + (behind `instance` data tables) or the external instance cluster (behind + `external_instance` ones). Defaults to `instance`. + enum: [instance, external_instance] + InstanceDatatableRole: type: object - required: [id, name, enabled] + required: [id, name, enabled, cluster] properties: id: type: string @@ -34058,6 +34210,8 @@ components: type: string enabled: type: boolean + cluster: + $ref: "#/components/schemas/DatatableRoleCluster" DatatableRoleTenants: type: object @@ -34079,8 +34233,10 @@ components: supported: type: boolean description: >- - Whether this data table can be put under roles at all. Only one backed by the - instance database can: a role is a login on that cluster. + Whether this data table can be put under roles at all. Only one on a database Windmill + manages can: a role is a login on that database's cluster. + cluster: + $ref: "#/components/schemas/DatatableRoleCluster" permissioned: type: boolean default_role: @@ -34392,7 +34548,10 @@ components: type: array items: type: string - description: Workspaces that reference this database via a ducklake catalog or datatable database with resource_type 'instance'. Computed at request time, not persisted. + description: Workspaces that reference this database through a ducklake catalog or a datatable database of the kind being listed — 'instance' for the instance databases endpoint, 'external_instance' for the external cluster one. Computed at request time, not persisted, and only returned to superadmins. + workspace_id: + type: string + description: The workspace a member created this database for as a fork copy. Only that workspace can import into it or point a fork at it. NewSqsTrigger: type: object @@ -36400,6 +36559,7 @@ components: - postgresql - mysql - instance + - external_instance resource_path: type: string required: @@ -36463,6 +36623,7 @@ components: enum: - postgresql - instance + - external_instance resource_path: type: string required: diff --git a/backend/windmill-common/Cargo.toml b/backend/windmill-common/Cargo.toml index e95fadfe66..96fa7ceb63 100644 --- a/backend/windmill-common/Cargo.toml +++ b/backend/windmill-common/Cargo.toml @@ -76,6 +76,7 @@ bitflags.workspace = true once_cell.workspace = true phf.workspace = true tokio-postgres.workspace = true +postgres-protocol.workspace = true postgres-native-tls.workspace = true native-tls.workspace = true diff --git a/backend/windmill-common/src/datatable_roles.rs b/backend/windmill-common/src/datatable_roles.rs index a9195055ac..f355e733cb 100644 --- a/backend/windmill-common/src/datatable_roles.rs +++ b/backend/windmill-common/src/datatable_roles.rs @@ -6,22 +6,67 @@ * LICENSE-AGPL for a copy of the license. */ -//! The instance's data table role catalog. +//! The instance's data table role catalogs. //! -//! A data table role is a real Postgres login role on the Windmill cluster, named exactly as the -//! user named it, shared by every instance database. Windmill decides who may ask for a role (the -//! per-data-table tenant lists in [`crate::workspaces`]); Postgres decides what the role may then -//! touch. The catalog here is only the first half's vocabulary plus the cluster provisioning. +//! A data table role is a real Postgres login role on one cluster — Windmill's own, or the external +//! instance cluster — named exactly as the user named it, shared by every database Windmill manages +//! on that cluster. Each cluster has its own catalog: a role exists where it was created and nowhere +//! else. Windmill decides who may ask for a role (the per-data-table tenant lists in +//! [`crate::workspaces`]); Postgres decides what the role may then touch. The catalog here is only +//! the first half's vocabulary plus the cluster provisioning. //! //! Entries are keyed by a generated id so a rename moves nothing else: tenants name the id. use std::collections::BTreeMap; +use serde::{Deserialize, Serialize}; + use crate::{ error::{Error, Result}, + workspaces::DataTableCatalogResourceType, DB, }; +/// The cluster a role catalog belongs to. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum DatatableRoleCluster { + /// Windmill's own Postgres, behind `instance` data tables. + #[default] + Instance, + /// The external instance cluster, behind `external_instance` data tables. + ExternalInstance, +} + +impl DatatableRoleCluster { + pub fn as_str(self) -> &'static str { + match self { + Self::Instance => "instance", + Self::ExternalInstance => "external_instance", + } + } + + pub fn parse(value: &str) -> Result { + match value { + "instance" => Ok(Self::Instance), + "external_instance" => Ok(Self::ExternalInstance), + other => Err(Error::BadRequest(format!( + "Unknown data table role cluster '{other}': expected instance or external_instance" + ))), + } + } + + /// The cluster whose roles a data table on `kind` can use. `None` for a resource-backed one, + /// which is never under roles. + pub fn of(kind: DataTableCatalogResourceType) -> Option { + match kind { + DataTableCatalogResourceType::Instance => Some(Self::Instance), + DataTableCatalogResourceType::ExternalInstance => Some(Self::ExternalInstance), + DataTableCatalogResourceType::Postgresql => None, + } + } +} + /// The connection every data table resolved to before roles existed (`custom_instance_user`). It /// owns every pre-existing object, so it is a reserved name rather than a catalog entry: never /// created, renamed or dropped. @@ -164,28 +209,42 @@ pub async fn lock_instance_databases_governance<'a>( /// need the names — but callers MUST NOT let `pwd` reach a response, a log line, an audit record /// or an export. Nothing about who may call it: the credential is the whole risk, and `Debug` is /// hand-written to redact it for the same reason. -pub async fn read_role_catalog(db: &DB) -> Result { - crate::datatable_roles_oss::read_role_catalog(db).await +pub async fn read_role_catalog( + db: &DB, + cluster: DatatableRoleCluster, +) -> Result { + crate::datatable_roles_oss::read_role_catalog(db, cluster).await } /// As [`read_role_catalog`], reading inside the caller's transaction so the value is the one /// [`lock_role_catalog`] is protecting. Same disclosure contract. pub async fn read_role_catalog_tx( tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + cluster: DatatableRoleCluster, ) -> Result { - crate::datatable_roles_oss::read_role_catalog_tx(tx).await + crate::datatable_roles_oss::read_role_catalog_tx(tx, cluster).await } -/// Record a role, in the caller's transaction so it commits with the `CREATE ROLE` it describes. +/// The cluster a role belongs to, or `None` if no role has this id. +pub async fn role_cluster( + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + id: &str, +) -> Result> { + crate::datatable_roles_oss::role_cluster(tx, id).await +} + +/// Record a role, in the caller's transaction. On Windmill's own cluster that commits it with the +/// `CREATE ROLE` it describes; on the external cluster the role already exists by then. /// /// Authorization: writes a generated Postgres credential. Callers MUST restrict this to superadmin /// paths and MUST hold [`lock_role_catalog`] on `tx`. pub async fn insert_role_catalog_entry( tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, id: &str, + cluster: DatatableRoleCluster, role: &InstanceDatatableRole, ) -> Result<()> { - crate::datatable_roles_oss::insert_role_catalog_entry(tx, id, role).await + crate::datatable_roles_oss::insert_role_catalog_entry(tx, id, cluster, role).await } /// Update a role's recorded name, login flag and password. Same contract as @@ -215,7 +274,7 @@ pub fn role_id_by_name<'a>(catalog: &'a DatatableRoleCatalog, name: &str) -> Res .find(|(_, role)| role.name == name) .ok_or_else(|| { Error::NotFound(format!( - "'{name}' is not a data table role of this instance. Defined roles: {}.", + "'{name}' is not a data table role of this database's cluster. Defined roles: {}.", catalog .values() .map(|r| r.name.as_str()) @@ -231,70 +290,90 @@ pub fn role_id_by_name<'a>(catalog: &'a DatatableRoleCatalog, name: &str) -> Res Ok(entry.0.as_str()) } -/// Every instance database the registry knows about. Role provisioning has to reach all of them: -/// a role that cannot `CONNECT` to a database is refused by Postgres before any grant matters. +/// Every database Windmill manages on `cluster`. Role provisioning has to reach all of them: a role +/// that cannot `CONNECT` to a database is refused by Postgres before any grant matters. /// -/// Authorization: checks nothing, and names every instance database across all workspaces. Callers +/// Authorization: checks nothing, and names every managed database across all workspaces. Callers /// MUST be superadmin-gated or keep the names server-side; never return them to a workspace caller. -pub async fn registered_instance_databases(db: &DB) -> Result> { - crate::datatable_roles_oss::registered_instance_databases(db).await +pub async fn registered_instance_databases( + db: &DB, + cluster: DatatableRoleCluster, +) -> Result> { + crate::datatable_roles_oss::registered_instance_databases(db, cluster).await } -/// `CONNECT` on `dbname` for every enabled role, and none for `PUBLIC`. Run at role creation, at -/// database creation, and lazily whenever an instance data table is administered, so a database -/// provisioned before a role existed is repaired rather than left silently unreachable. +/// `CONNECT` on `dbname` for every enabled role of `cluster`, and none for `PUBLIC`. Run at role +/// creation, at database creation, and lazily whenever a managed data table is administered, so a +/// database provisioned before a role existed is repaired rather than left silently unreachable. /// /// Authorization: rewrites a database's ACL with the server's own credentials and checks nothing. /// Callers MUST have authorized administration of `dbname` — superadmin, or an admin of the /// workspace governing a data table on it. -pub async fn converge_connect_grants(db: &DB, dbname: &str) -> Result<()> { - crate::datatable_roles_oss::converge_connect_grants(db, dbname).await +pub async fn converge_connect_grants( + db: &DB, + cluster: DatatableRoleCluster, + dbname: &str, +) -> Result<()> { + crate::datatable_roles_oss::converge_connect_grants(db, cluster, dbname).await } -/// As [`converge_connect_grants`], with a catalog the caller already read. Same contract. +/// As [`converge_connect_grants`], with the catalog of `cluster` the caller already read. Same +/// contract. pub async fn converge_connect_grants_with( db: &DB, + cluster: DatatableRoleCluster, dbname: &str, catalog: &DatatableRoleCatalog, ) -> Result<()> { - crate::datatable_roles_oss::converge_connect_grants_with(db, dbname, catalog).await + crate::datatable_roles_oss::converge_connect_grants_with(db, cluster, dbname, catalog).await } -/// `CREATE ROLE LOGIN PASSWORD ...; GRANT TO custom_instance_user`, and `CONNECT` on -/// every registered database. No privileges beyond that — an admin grants them through SQL or the -/// ACL editor. +/// `CREATE ROLE LOGIN PASSWORD ...; GRANT TO custom_instance_user` on `cluster`. No +/// privileges beyond that — an admin grants them through SQL or the ACL editor. +/// +/// On Windmill's own cluster the DDL runs on `tx`, so it commits with the catalog row. The external +/// cluster is another server: the role is created there before `tx` commits, and callers MUST drop +/// it again ([`drop_datatable_role`]) if `tx` then fails to commit. /// /// Authorization: creates a cluster-wide Postgres login. Callers MUST restrict this to superadmin -/// paths, and MUST hold [`lock_role_catalog`] on the same transaction. -pub async fn create_instance_role( +/// paths, and MUST hold [`lock_role_catalog`] on `tx`. +pub async fn create_datatable_role( + db: &DB, tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + cluster: DatatableRoleCluster, name: &str, password: &str, ) -> Result<()> { - crate::datatable_roles_oss::create_instance_role(tx, name, password).await + crate::datatable_roles_oss::create_datatable_role(db, tx, cluster, name, password).await } /// Authorization: alters a cluster-wide Postgres login. Callers MUST restrict this to superadmin -/// paths, and MUST hold [`lock_role_catalog`] on the same transaction. -pub async fn set_instance_role_login( +/// paths, and MUST hold [`lock_role_catalog`] on `tx`. +pub async fn set_datatable_role_login( + db: &DB, tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + cluster: DatatableRoleCluster, name: &str, enabled: bool, ) -> Result<()> { - crate::datatable_roles_oss::set_instance_role_login(tx, name, enabled).await + crate::datatable_roles_oss::set_datatable_role_login(db, tx, cluster, name, enabled).await } -/// A rename discards an md5-hashed password, so the caller has to hand over a fresh one. +/// A rename discards an md5-hashed password, so the caller has to hand over a fresh one. On the +/// external cluster the rename lands before `tx` commits, and callers MUST rename it back if `tx` +/// then fails to commit. /// /// Authorization: renames a cluster-wide Postgres login. Callers MUST restrict this to superadmin -/// paths, and MUST hold [`lock_role_catalog`] on the same transaction. -pub async fn rename_instance_role( +/// paths, and MUST hold [`lock_role_catalog`] on `tx`. +pub async fn rename_datatable_role( + db: &DB, tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + cluster: DatatableRoleCluster, from: &str, to: &str, password: &str, ) -> Result<()> { - crate::datatable_roles_oss::rename_instance_role(tx, from, to, password).await + crate::datatable_roles_oss::rename_datatable_role(db, tx, cluster, from, to, password).await } /// A role owning anything in any database blocks its own `DROP ROLE`, and both its objects and the @@ -302,8 +381,9 @@ pub async fn rename_instance_role( /// registry. An unreachable database aborts the whole delete: dropping the role while one database /// still holds objects owned by it leaves those objects owned by a numeric OID nobody can name. /// -/// Each pass runs as the instance's own Postgres user rather than `custom_instance_user`, which -/// owns the databases and can therefore revoke a grant whoever made it. `custom_instance_user` +/// Each pass runs as the cluster's administrator rather than `custom_instance_user`: on Windmill's +/// own cluster the instance's Postgres user, on the external one its configured admin login. Both +/// own the databases and can therefore revoke a grant whoever made it. `custom_instance_user` /// could only undo what it granted itself, so a privilege planted by an operator in psql — the /// ordinary way privileges reach a role — would survive and block the drop. /// @@ -311,16 +391,19 @@ pub async fn rename_instance_role( /// MUST restrict this to superadmin paths, and MUST hold [`lock_role_catalog`] on `tx`. /// /// The per-database passes open their own connections and cannot join `tx`; the lock is what keeps -/// a concurrent mutation out while they run. Only the final `DROP ROLE` is on `tx`, so it commits -/// or rolls back with the catalog write that forgets the role. Those passes commit as they go, so -/// callers MUST have disabled the role in an earlier committed transaction: a failure part-way -/// then leaves a disabled role to retry, not an enabled one already stripped in some databases. -pub async fn drop_instance_role( +/// a concurrent mutation out while they run. On Windmill's own cluster only the final `DROP ROLE` +/// is on `tx`, so it commits or rolls back with the catalog write that forgets the role; on the +/// external cluster it runs there, and tolerates a role already gone so a retry after a failed +/// commit can finish. The passes commit as they go, so callers MUST have disabled the role in an +/// earlier committed transaction: a failure part-way then leaves a disabled role to retry, not an +/// enabled one already stripped in some databases. +pub async fn drop_datatable_role( db: &DB, tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + cluster: DatatableRoleCluster, name: &str, ) -> Result<()> { - crate::datatable_roles_oss::drop_instance_role(db, tx, name).await + crate::datatable_roles_oss::drop_datatable_role(db, tx, cluster, name).await } #[cfg(test)] diff --git a/backend/windmill-common/src/datatable_roles_oss.rs b/backend/windmill-common/src/datatable_roles_oss.rs index 2a3f9dda48..a97a936827 100644 --- a/backend/windmill-common/src/datatable_roles_oss.rs +++ b/backend/windmill-common/src/datatable_roles_oss.rs @@ -24,12 +24,12 @@ pub fn datatable_roles_unavailable() -> Error { #[cfg(all(feature = "private", feature = "enterprise"))] pub(crate) use crate::datatable_roles_ee::{ can_use_datatable_role, can_use_datatable_role_in_governing_workspace, converge_connect_grants, - converge_connect_grants_with, create_instance_role, delete_role_catalog_entry, - drop_instance_role, ensure_can_use_datatable_role, ensure_datatable_admin_access, + converge_connect_grants_with, create_datatable_role, delete_role_catalog_entry, + drop_datatable_role, ensure_can_use_datatable_role, ensure_datatable_admin_access, ensure_instance_db_grant_options_unchecked, forget_datatable_role_everywhere, insert_role_catalog_entry, read_role_catalog, read_role_catalog_tx, - registered_instance_databases, rename_instance_role, resolve_datatable_role_connection, - set_instance_role_login, update_role_catalog_entry, + registered_instance_databases, rename_datatable_role, resolve_datatable_role_connection, + role_cluster, set_datatable_role_login, update_role_catalog_entry, }; #[cfg(not(all(feature = "private", feature = "enterprise")))] @@ -39,7 +39,7 @@ pub(crate) use ce::*; mod ce { use super::datatable_roles_unavailable as unavailable; use crate::{ - datatable_roles::{DatatableRoleCatalog, InstanceDatatableRole}, + datatable_roles::{DatatableRoleCatalog, DatatableRoleCluster, InstanceDatatableRole}, db::AuthedRef, error::Result, workspaces::{ @@ -50,17 +50,31 @@ mod ce { type Tx<'a> = sqlx::Transaction<'a, sqlx::Postgres>; - pub(crate) async fn read_role_catalog(_db: &DB) -> Result { + pub(crate) async fn read_role_catalog( + _db: &DB, + _cluster: DatatableRoleCluster, + ) -> Result { Err(unavailable()) } - pub(crate) async fn read_role_catalog_tx(_tx: &mut Tx<'_>) -> Result { + pub(crate) async fn read_role_catalog_tx( + _tx: &mut Tx<'_>, + _cluster: DatatableRoleCluster, + ) -> Result { + Err(unavailable()) + } + + pub(crate) async fn role_cluster( + _tx: &mut Tx<'_>, + _id: &str, + ) -> Result> { Err(unavailable()) } pub(crate) async fn insert_role_catalog_entry( _tx: &mut Tx<'_>, _id: &str, + _cluster: DatatableRoleCluster, _role: &InstanceDatatableRole, ) -> Result<()> { Err(unavailable()) @@ -78,43 +92,57 @@ mod ce { Err(unavailable()) } - pub(crate) async fn registered_instance_databases(_db: &DB) -> Result> { + pub(crate) async fn registered_instance_databases( + _db: &DB, + _cluster: DatatableRoleCluster, + ) -> Result> { Err(unavailable()) } - /// Nothing to converge: with no roles to admit, an instance database keeps the `CONNECT` - /// grants it was created with, `PUBLIC`'s included, as it did before roles existed. - pub(crate) async fn converge_connect_grants(_db: &DB, _dbname: &str) -> Result<()> { + /// Nothing to converge: with no roles to admit, a managed database keeps the `CONNECT` grants + /// it was created with, as it did before roles existed. + pub(crate) async fn converge_connect_grants( + _db: &DB, + _cluster: DatatableRoleCluster, + _dbname: &str, + ) -> Result<()> { Ok(()) } /// As [`converge_connect_grants`]. pub(crate) async fn converge_connect_grants_with( _db: &DB, + _cluster: DatatableRoleCluster, _dbname: &str, _catalog: &DatatableRoleCatalog, ) -> Result<()> { Ok(()) } - pub(crate) async fn create_instance_role( + pub(crate) async fn create_datatable_role( + _db: &DB, _tx: &mut Tx<'_>, + _cluster: DatatableRoleCluster, _name: &str, _password: &str, ) -> Result<()> { Err(unavailable()) } - pub(crate) async fn set_instance_role_login( + pub(crate) async fn set_datatable_role_login( + _db: &DB, _tx: &mut Tx<'_>, + _cluster: DatatableRoleCluster, _name: &str, _enabled: bool, ) -> Result<()> { Err(unavailable()) } - pub(crate) async fn rename_instance_role( + pub(crate) async fn rename_datatable_role( + _db: &DB, _tx: &mut Tx<'_>, + _cluster: DatatableRoleCluster, _from: &str, _to: &str, _password: &str, @@ -122,12 +150,18 @@ mod ce { Err(unavailable()) } - pub(crate) async fn drop_instance_role(_db: &DB, _tx: &mut Tx<'_>, _name: &str) -> Result<()> { + pub(crate) async fn drop_datatable_role( + _db: &DB, + _tx: &mut Tx<'_>, + _cluster: DatatableRoleCluster, + _name: &str, + ) -> Result<()> { Err(unavailable()) } pub(crate) async fn ensure_instance_db_grant_options_unchecked( _db: &DB, + _cluster: DatatableRoleCluster, _dbname: &str, ) -> Result<()> { Err(unavailable()) diff --git a/backend/windmill-common/src/external_instance_pg.rs b/backend/windmill-common/src/external_instance_pg.rs new file mode 100644 index 0000000000..c0b6203e40 --- /dev/null +++ b/backend/windmill-common/src/external_instance_pg.rs @@ -0,0 +1,456 @@ +/* + * Author: Ruben Fiszel + * Copyright: Windmill Labs, Inc 2022 + * This file and its contents are licensed under the AGPLv3 License. + * Please see the included NOTICE for copyright information and + * LICENSE-AGPL for a copy of the license. + */ + +//! The external Postgres cluster behind `external_instance` data tables and Ducklake catalogs. +//! +//! Windmill administers that cluster itself, logged in as the user in +//! [`EXTERNAL_INSTANCE_PG_SETTING`]. It creates `custom_instance_user` and +//! `custom_instance_replication_user` there, with passwords it generates and keeps in +//! [`EXTERNAL_INSTANCE_PG_STATE_SETTING`]. They share their names with the roles on Windmill's own +//! cluster, but they are different roles with different passwords. +//! +//! The cluster may hold data Windmill did not create. Two Windmill instances sharing one is not +//! supported: each would keep resetting the passwords the other depends on. + +use std::collections::{BTreeMap, BTreeSet}; + +use serde::{Deserialize, Serialize}; + +use crate::{ + error::{Error, Result}, + global_settings::{EXTERNAL_INSTANCE_PG_SETTING, EXTERNAL_INSTANCE_PG_STATE_SETTING}, + instance_config::{CustomInstanceDb, ExternalInstancePg}, + DB, +}; + +/// What Windmill keeps about the external cluster. Server-managed and hidden: never part of the +/// instance config, never readable by an agent worker. No `Debug`: it carries live passwords. +#[derive(Serialize, Deserialize, Clone, Default)] +pub struct ExternalInstancePgState { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub user_pwd: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub replication_pwd: Option, + /// The databases Windmill created on the cluster. It only ever drops one of these. + #[serde(default)] + pub databases: BTreeMap, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub last_setup: Option, + /// The cluster ([`external_instance_pg_address`]) the last successful setup converged. Databases + /// are only created on a cluster setup succeeded on: the passwords above exist as soon as setup + /// first runs, whether or not the cluster accepted them. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub set_up_for: Option, +} + +/// What identifies the cluster a configuration points at. Other fields (admin login, sslmode) can +/// change without it becoming another cluster. +pub fn external_instance_pg_address(config: &ExternalInstancePg) -> String { + format!( + "{}:{}", + config.host.trim().to_lowercase(), + config.port.unwrap_or(5432) + ) +} + +#[derive(Serialize, Deserialize, Clone, Debug)] +pub struct ExternalInstancePgSetupReport { + /// No step failed. Warnings leave it true. + pub success: bool, + pub finished_at: chrono::DateTime, + pub steps: Vec, +} + +#[derive(Serialize, Deserialize, Clone, Debug)] +pub struct ExternalInstancePgSetupStep { + pub name: String, + pub status: SetupStepStatus, + pub message: String, +} + +#[derive(Serialize, Deserialize, Clone, Copy, Debug, PartialEq, Eq)] +#[serde(rename_all = "lowercase")] +pub enum SetupStepStatus { + Ok, + Warning, + Error, +} + +/// The status the settings page shows without running anything. +#[derive(Serialize, Debug)] +pub struct ExternalInstancePgStatus { + pub configured: bool, + pub database_count: usize, + #[serde(skip_serializing_if = "Option::is_none")] + pub last_setup: Option, +} + +/// Authorization: returns the cluster's admin password and checks nothing. Callers MUST be +/// superadmin or an internal server path. +pub(crate) async fn read_external_instance_pg_config<'c>( + executor: impl sqlx::PgExecutor<'c>, +) -> Result> { + let value = sqlx::query_scalar!( + "SELECT value FROM global_settings WHERE name = $1", + EXTERNAL_INSTANCE_PG_SETTING + ) + .fetch_optional(executor) + .await?; + value + .map(|v| { + serde_json::from_value(v).map_err(|e| { + Error::internal_err(format!("reading {EXTERNAL_INSTANCE_PG_SETTING}: {e}")) + }) + }) + .transpose() +} + +/// Authorization: returns the passwords Windmill generated on the cluster and checks nothing. +/// Callers MUST be superadmin or an internal server path. +pub(crate) async fn read_external_instance_pg_state<'c>( + executor: impl sqlx::PgExecutor<'c>, +) -> Result { + let value = sqlx::query_scalar!( + "SELECT value FROM global_settings WHERE name = $1", + EXTERNAL_INSTANCE_PG_STATE_SETTING + ) + .fetch_optional(executor) + .await?; + match value { + None => Ok(ExternalInstancePgState::default()), + Some(v) => serde_json::from_value(v).map_err(|e| { + Error::internal_err(format!("reading {EXTERNAL_INSTANCE_PG_STATE_SETTING}: {e}")) + }), + } +} + +/// Authorization: reads the hidden cluster state and checks nothing. Callers MUST be superadmin. +pub async fn external_instance_pg_status(db: &DB) -> Result { + let configured = read_external_instance_pg_config(db).await?.is_some(); + let state = read_external_instance_pg_state(db).await?; + Ok(ExternalInstancePgStatus { + configured, + database_count: state.databases.len(), + last_setup: state.last_setup, + }) +} + +/// The databases Windmill created on the external cluster, without the passwords kept beside them. +/// +/// Authorization: names every database across all workspaces, and the workspace each fork copy is +/// reserved for, and checks nothing. Callers MUST be superadmin or an internal authorization or +/// lifecycle path that does not return the names to a workspace caller. +pub async fn external_instance_databases(db: &DB) -> Result> { + Ok(read_external_instance_pg_state(db).await?.databases) +} + +/// The workspaces whose data tables or Ducklake catalogs name each database on the external cluster, +/// and the forks whose Ducklake metadata schemas there are still waiting to be dropped: those rows +/// outlive a settings change, and cleanup cannot drop a schema in a database that is gone. A row +/// whose schema is already dropped only waits on object storage, which needs no database. +/// +/// Authorization: reads every workspace's settings and checks nothing. Callers MUST be superadmin +/// or an internal lifecycle path. +pub async fn external_instance_database_usages<'c>( + db: impl sqlx::PgExecutor<'c>, +) -> Result>> { + let rows = sqlx::query_as::<_, (String, String)>( + "SELECT ws.workspace_id, entry->'database'->>'resource_path' + FROM workspace_settings ws + CROSS JOIN LATERAL jsonb_each( + CASE WHEN jsonb_typeof(ws.datatable->'datatables') = 'object' + THEN ws.datatable->'datatables' + ELSE '{}'::jsonb END + ) AS dt(k, entry) + WHERE entry->'database'->>'resource_type' = 'external_instance' + AND entry->'database'->>'resource_path' IS NOT NULL + UNION ALL + SELECT ws.workspace_id, entry->'catalog'->>'resource_path' + FROM workspace_settings ws + CROSS JOIN LATERAL jsonb_each( + CASE WHEN jsonb_typeof(ws.ducklake->'ducklakes') = 'object' + THEN ws.ducklake->'ducklakes' + ELSE '{}'::jsonb END + ) AS dl(k, entry) + WHERE entry->'catalog'->>'resource_type' = 'external_instance' + AND entry->'catalog'->>'resource_path' IS NOT NULL + UNION ALL + SELECT workspace_id, substring(catalog FROM length('external_instance:') + 1) + FROM fork_ducklake_namespace + WHERE catalog LIKE 'external\\_instance:%' AND NOT schema_dropped", + ) + .fetch_all(db) + .await?; + let mut usages: BTreeMap> = BTreeMap::new(); + for (workspace_id, dbname) in rows { + usages.entry(dbname).or_default().insert(workspace_id); + } + Ok(usages) +} + +/// Refuse to unset the cluster while Windmill still has databases or data table roles on it, or a +/// workspace still points at one: every data table there would stop resolving, and every role +/// would be a login nothing can drop any more. Allowed on every edition, so a +/// downgraded instance can still clear a setting it no longer uses. +async fn ensure_external_instance_pg_removable(conn: &mut sqlx::PgConnection) -> Result<()> { + ensure_external_instance_pg_unused(conn, &format!("removing {EXTERNAL_INSTANCE_PG_SETTING}")).await +} + +/// Refuse while Windmill has databases or data table roles on the cluster, or a workspace points +/// at one of its databases. `before` finishes the sentence saying what to do first. +async fn ensure_external_instance_pg_unused( + conn: &mut sqlx::PgConnection, + before: &str, +) -> Result<()> { + let state = read_external_instance_pg_state(&mut *conn).await?; + let usages = external_instance_database_usages(&mut *conn).await?; + let roles = sqlx::query_scalar::<_, String>( + "SELECT name FROM datatable_role WHERE cluster = 'external_instance' ORDER BY name", + ) + .fetch_all(&mut *conn) + .await?; + if state.databases.is_empty() && usages.is_empty() && roles.is_empty() { + return Ok(()); + } + let mut held = vec![]; + if !(state.databases.is_empty() && usages.is_empty()) { + let names = state + .databases + .keys() + .chain(usages.keys()) + .collect::>() + .into_iter() + .cloned() + .collect::>() + .join(", "); + held.push(format!("databases in use ({names})")); + } + if !roles.is_empty() { + held.push(format!("data table roles ({})", roles.join(", "))); + } + Err(Error::BadRequest(format!( + "The external instance cluster still holds {}. Drop them and repoint the data tables and \ + Ducklake catalogs using them before {before}.", + held.join(" and ") + ))) +} + +/// Refuse a workspace setting that newly names an `external_instance` database on an edition +/// without them. +pub fn ensure_external_instance_available() -> Result<()> { + crate::external_instance_pg_oss::ensure_external_instance_available() +} + +/// The connection an `external_instance` database resolves to: `custom_instance_user`, or the +/// replication user, on the external cluster. +/// +/// Authorization: returns live credentials and checks nothing. Callers MUST have authorized access +/// to the data table that names `dbname`. +pub async fn external_instance_connection_unchecked( + db: &DB, + dbname: &str, + replication: bool, +) -> Result { + crate::external_instance_pg_oss::external_instance_connection_unchecked(db, dbname, replication) + .await +} + +/// Create `dbname` on the external cluster and register it. Refuses a name already taken there, +/// whoever took it. +/// +/// Runs on `tx`, which it takes [`lock_external_instance_pg_state`] on: the registration lands when +/// the caller commits. Take that lock before any database governance lock, as saves do. +/// +/// Authorization: checks nothing. Callers MUST be superadmin, or be cloning a data table they may +/// fork into a `wm_fork_` database. +pub async fn create_external_instance_database_unchecked( + db: &DB, + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + dbname: &str, + tag: &str, + for_workspace: Option<&str>, +) -> Result<()> { + crate::external_instance_pg_oss::create_external_instance_database_unchecked( + db, + tx, + dbname, + tag, + for_workspace, + ) + .await +} + +/// Drop `dbname` from the external cluster: only a database Windmill registered creating, and still +/// carries the mark it set there. Refused while anything uses it +/// ([`crate::workspaces::managed_database_uses`]), except the `exempt` data table entry: the fork +/// copy being cleaned up. +/// +/// Runs on `tx`, like [`create_external_instance_database_unchecked`]: the database is gone at +/// once, its registry entry when the caller commits. +/// +/// Authorization: checks nothing. Callers MUST be superadmin, or be deleting the fork that owns +/// this `wm_fork_` database. +pub async fn drop_external_instance_database_unchecked( + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + dbname: &str, + exempt: Option<(&str, &str)>, +) -> Result<()> { + crate::external_instance_pg_oss::drop_external_instance_database_unchecked(tx, dbname, exempt) + .await +} + +/// Serializes everything that changes which databases exist on the external cluster, or which data +/// tables name them: setup, creates, drops, and data table saves. Held until `tx` ends. +pub async fn lock_external_instance_pg_state( + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, +) -> Result<()> { + sqlx::query("SELECT pg_advisory_xact_lock(hashtext($1))") + .bind(EXTERNAL_INSTANCE_PG_STATE_SETTING) + .execute(&mut **tx) + .await?; + Ok(()) +} + +/// Refuse a data table naming `dbname` unless Windmill created it on the external cluster. Takes +/// the lock drops take, so none can remove the database before `tx`, which saves the data table, +/// commits. +/// +/// Authorization: its refusal says whether Windmill created a database of that name, which is +/// instance-wide knowledge. Callers MUST have authorized the caller as superadmin first. +pub async fn ensure_external_instance_database_registered( + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + dbname: &str, +) -> Result<()> { + lock_external_instance_pg_state(tx).await?; + if read_external_instance_pg_state(&mut **tx) + .await? + .databases + .contains_key(dbname) + { + return Ok(()); + } + Err(Error::BadRequest(format!( + "Windmill did not create a database named '{dbname}' on the external instance cluster. \ + Create it from the instance settings first." + ))) +} + +/// Write [`EXTERNAL_INSTANCE_PG_SETTING`]: `None`, null or an empty string unsets it. Every writer +/// of global settings goes through this for that key — the per-key and bulk endpoints as well as +/// the declarative sync — instead of writing the row itself. +/// +/// The checks and the write share one transaction holding [`lock_external_instance_pg_state`]. A +/// check taken outside it could pass while a database create still reads the old cluster, which +/// would then register a database there after the setting names another one. +/// +/// Authorization: checks nothing. Callers MUST be superadmin, or the declarative instance config +/// sync, which applies what the operator deployed. +pub async fn write_external_instance_pg_setting( + db: &DB, + value: Option<&serde_json::Value>, +) -> Result<()> { + let value = match value { + None | Some(serde_json::Value::Null) => None, + Some(serde_json::Value::String(s)) if s.trim().is_empty() => None, + Some(value) => Some(value), + }; + let mut tx = db.begin().await?; + lock_external_instance_pg_state(&mut tx).await?; + // Every check runs on this transaction's own connection: it holds the advisory lock, and + // taking a second connection from the pool while other writers queue on that lock is how a + // small pool deadlocks. + match value { + None => { + ensure_external_instance_pg_removable(&mut tx).await?; + sqlx::query("DELETE FROM global_settings WHERE name = $1") + .bind(EXTERNAL_INSTANCE_PG_SETTING) + .execute(&mut *tx) + .await?; + } + Some(value) => { + crate::external_instance_pg_oss::validate_external_instance_pg_setting(value)?; + ensure_external_instance_pg_not_repointed(&mut tx, value).await?; + sqlx::query( + "INSERT INTO global_settings (name, value) VALUES ($1, $2) + ON CONFLICT (name) DO UPDATE SET value = EXCLUDED.value, updated_at = now()", + ) + .bind(EXTERNAL_INSTANCE_PG_SETTING) + .bind(value) + .execute(&mut *tx) + .await?; + } + } + tx.commit().await?; + tracing::info!( + "{} global setting {EXTERNAL_INSTANCE_PG_SETTING}", + if value.is_some() { "Set" } else { "Unset" } + ); + Ok(()) +} + +/// [`write_external_instance_pg_setting`] for a settings diff: writes the key if the diff touches +/// it, and takes it out of the diff so the generic apply does not write it again. +/// +/// Authorization: checks nothing. Callers MUST be superadmin, or the declarative instance config +/// sync, which applies what the operator deployed. +pub async fn write_external_instance_pg_from_diff( + db: &DB, + diff: &mut crate::instance_config::SettingsDiff, +) -> Result<()> { + if let Some(value) = diff.upserts.remove(EXTERNAL_INSTANCE_PG_SETTING) { + write_external_instance_pg_setting(db, Some(&value)).await?; + } + if let Some(i) = diff + .deletes + .iter() + .position(|k| k == EXTERNAL_INSTANCE_PG_SETTING) + { + diff.deletes.remove(i); + write_external_instance_pg_setting(db, None).await?; + } + Ok(()) +} + +/// Refuse pointing the setting at another host or port while databases or data table roles live on +/// the current one. Data tables name databases, and the role catalog names logins, not clusters, so +/// both would silently resolve to whatever the new cluster holds under the same names. Other fields +/// (admin login, sslmode) may change freely. +async fn ensure_external_instance_pg_not_repointed( + conn: &mut sqlx::PgConnection, + value: &serde_json::Value, +) -> Result<()> { + let Some(current) = read_external_instance_pg_config(&mut *conn).await? else { + return Ok(()); + }; + let Ok(desired) = serde_json::from_value::(value.clone()) else { + return Ok(()); + }; + if external_instance_pg_address(¤t) == external_instance_pg_address(&desired) { + return Ok(()); + } + ensure_external_instance_pg_unused( + conn, + &format!("pointing {EXTERNAL_INSTANCE_PG_SETTING} at another cluster"), + ) + .await +} + +/// Converge the external cluster on the configured login: check what it can do, create or update +/// Windmill's two roles with the stored passwords, and report anything that would get in the way. +/// With `rotate_passwords`, generate new passwords first. Safe to run again; running it again is +/// how a failed rotation is repaired. +/// +/// Authorization: administers the external cluster with its admin credentials and checks nothing. +/// Callers MUST be superadmin. +pub async fn setup_external_instance_pg_unchecked( + db: &DB, + rotate_passwords: bool, +) -> Result { + crate::external_instance_pg_oss::setup_external_instance_pg_unchecked(db, rotate_passwords) + .await +} diff --git a/backend/windmill-common/src/external_instance_pg_oss.rs b/backend/windmill-common/src/external_instance_pg_oss.rs new file mode 100644 index 0000000000..93b6fc63dd --- /dev/null +++ b/backend/windmill-common/src/external_instance_pg_oss.rs @@ -0,0 +1,82 @@ +/* + * Author: Ruben Fiszel + * Copyright: Windmill Labs, Inc 2022 + * This file and its contents are licensed under the AGPLv3 License. + * Please see the included NOTICE for copyright information and + * LICENSE-AGPL for a copy of the license. + */ + +//! Where the external instance cluster comes from: the enterprise implementation, or a refusal. +//! `private` alone is not that edition: community builds carry it. + +use crate::error::Error; + +pub fn external_instance_pg_unavailable() -> Error { + Error::BadRequest( + "External instance databases are a Windmill Enterprise Edition feature".to_string(), + ) +} + +#[cfg(all(feature = "private", feature = "enterprise"))] +pub(crate) use crate::external_instance_pg_ee::{ + create_external_instance_database_unchecked, drop_external_instance_database_unchecked, + external_instance_connection_unchecked, setup_external_instance_pg_unchecked, + validate_external_instance_pg_setting, +}; + +#[cfg(all(feature = "private", feature = "enterprise"))] +pub(crate) fn ensure_external_instance_available() -> crate::error::Result<()> { + Ok(()) +} + +#[cfg(not(all(feature = "private", feature = "enterprise")))] +pub(crate) use ce::*; + +#[cfg(not(all(feature = "private", feature = "enterprise")))] +mod ce { + use super::external_instance_pg_unavailable as unavailable; + use crate::{ + error::Result, external_instance_pg::ExternalInstancePgSetupReport, PgDatabase, DB, + }; + + pub(crate) fn validate_external_instance_pg_setting(_value: &serde_json::Value) -> Result<()> { + Err(unavailable()) + } + + pub(crate) fn ensure_external_instance_available() -> Result<()> { + Err(unavailable()) + } + + pub(crate) async fn setup_external_instance_pg_unchecked( + _db: &DB, + _rotate_passwords: bool, + ) -> Result { + Err(unavailable()) + } + + pub(crate) async fn external_instance_connection_unchecked( + _db: &DB, + _dbname: &str, + _replication: bool, + ) -> Result { + Err(unavailable()) + } + + pub(crate) async fn create_external_instance_database_unchecked( + _db: &DB, + _tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + _dbname: &str, + _tag: &str, + _for_workspace: Option<&str>, + ) -> Result<()> { + Err(unavailable()) + } + + pub(crate) async fn drop_external_instance_database_unchecked( + _tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + _dbname: &str, + _exempt: Option<(&str, &str)>, + ) -> Result<()> { + Err(unavailable()) + } +} diff --git a/backend/windmill-common/src/global_settings.rs b/backend/windmill-common/src/global_settings.rs index 7306c5c59a..9137dc364e 100644 --- a/backend/windmill-common/src/global_settings.rs +++ b/backend/windmill-common/src/global_settings.rs @@ -57,6 +57,11 @@ pub const SAML_METADATA_SETTING: &str = "saml_metadata"; pub const SMTP_SETTING: &str = "smtp_settings"; pub const TEAMS_SETTING: &str = "teams"; pub const INDEXER_SETTING: &str = "indexer_settings"; +pub const EXTERNAL_INSTANCE_PG_SETTING: &str = "external_instance_pg"; +/// Turns off Windmill's own Postgres as a data table and Ducklake substrate. Absent means on, +/// which is what every instance that predates the setting expects. +pub const INSTANCE_PG_DISABLED_SETTING: &str = "instance_pg_disabled"; +pub const EXTERNAL_INSTANCE_PG_STATE_SETTING: &str = "external_instance_pg_state"; pub const TIMEOUT_WAIT_RESULT_SETTING: &str = "timeout_wait_result"; pub const UNIQUE_ID_SETTING: &str = "uid"; @@ -448,6 +453,9 @@ pub const AGENT_WORKER_BLOCKED_SETTINGS: &[&str] = &[ // resolve datatable connections through the dedicated datatable endpoints, never these. "custom_instance_pg_databases", "custom_instance_replication_pwd", + // The external cluster's admin login, and the passwords Windmill generated on it. + EXTERNAL_INSTANCE_PG_SETTING, + EXTERNAL_INSTANCE_PG_STATE_SETTING, ]; /// Whether an agent worker may read the given global setting over HTTP. diff --git a/backend/windmill-common/src/instance_config.rs b/backend/windmill-common/src/instance_config.rs index 264916d522..fd790547b6 100644 --- a/backend/windmill-common/src/instance_config.rs +++ b/backend/windmill-common/src/instance_config.rs @@ -354,6 +354,8 @@ pub struct GlobalSettings { pub ducklake_settings: Option, #[serde(skip_serializing_if = "Option::is_none")] pub custom_instance_pg_databases: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub external_instance_pg: Option, // Opaque settings (EE-private structs or no clear schema) #[serde(skip_serializing_if = "Option::is_none")] @@ -815,6 +817,9 @@ pub struct CustomInstanceDb { pub error: Option, #[serde(skip_serializing_if = "Option::is_none")] pub tag: Option, + /// The workspace a member created this fork copy for. Absent when a superadmin created it. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub workspace_id: Option, } /// Setup log entries for a custom instance database. @@ -841,6 +846,36 @@ pub struct CustomInstanceDbLogs { pub user_connect: String, } +// --------------------------------------------------------------------------- +// External instance PG cluster +// --------------------------------------------------------------------------- + +/// The external Postgres cluster Windmill manages for `external_instance` data tables and Ducklake +/// catalogs. `user` logs in as the cluster's administrator: it needs `CREATEDB` and `CREATEROLE`. +/// `dbname` is only where that login connects to run cluster-wide statements. +/// +/// Every field defaults rather than being required: this deserializes as part of the whole +/// instance config, and one malformed row must not make every other setting unreadable. The +/// write path and every use reject an incomplete value instead. +#[derive(Deserialize, Serialize, Clone, Debug, Default)] +#[cfg_attr(feature = "instance_config_schema", derive(schemars::JsonSchema))] +pub struct ExternalInstancePg { + #[serde(default)] + pub host: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub port: Option, + #[serde(default)] + pub user: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub password: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub dbname: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub sslmode: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub root_certificate_pem: Option, +} + // --------------------------------------------------------------------------- // Autoscaling (worker config) // --------------------------------------------------------------------------- @@ -977,6 +1012,7 @@ pub const PROTECTED_SETTINGS: &[&str] = &[ "ducklake_settings", "custom_instance_pg_databases", "custom_instance_replication_pwd", + "external_instance_pg_state", "uid", "rsa_keys", "jwt_secret", @@ -1002,6 +1038,8 @@ pub const HIDDEN_SETTINGS: &[&str] = &[ // Server-only (written by setup/refresh via direct SQL), never operator-authored — // hidden so the config machinery can't read, rewrite, or drop it. "custom_instance_replication_pwd", + // Same for the passwords and database registry Windmill keeps for the external cluster. + "external_instance_pg_state", ]; /// Top-level settings whose entire value is sensitive and must be fully redacted in logs. @@ -1013,6 +1051,7 @@ const SENSITIVE_SETTINGS: &[&str] = &[ "license_key", "ducklake_user_pg_pwd", "custom_instance_replication_pwd", + "external_instance_pg_state", "pip_index_url", "pip_extra_index_url", "npm_config_registry", @@ -1038,6 +1077,7 @@ const NESTED_SENSITIVE_FIELDS: &[(&str, &[&str])] = &[ &["secret_key", "serviceAccountKey", "accessKey"], ), ("custom_instance_pg_databases", &["user_pwd"]), + ("external_instance_pg", &["password"]), ("github_enterprise_app", &["private_key"]), ]; @@ -1379,7 +1419,8 @@ pub async fn sync_global_settings_declarative( crate::global_settings::parse_max_token_expiration_days(desired.get(max_expiration_key)) .map_err(|e| anyhow::anyhow!("{max_expiration_key}: {e}"))?; - let diff = diff_global_settings(current, desired, ApplyMode::Replace); + let mut diff = diff_global_settings(current, desired, ApplyMode::Replace); + crate::external_instance_pg::write_external_instance_pg_from_diff(db, &mut diff).await?; apply_settings_diff(db, &diff).await?; Ok(()) @@ -1512,6 +1553,10 @@ pub fn resolve_env_refs(settings: &mut GlobalSettings) -> Result<(), String> { resolve_env_option(&mut pg.user_pwd)?; } + if let Some(pg) = &mut settings.external_instance_pg { + resolve_env_option(&mut pg.password)?; + } + Ok(()) } @@ -2495,39 +2540,33 @@ mod tests { } #[test] - fn custom_instance_replication_pwd_is_isolated_from_config() { - // The replication-role password is server-only: written by setup/refresh via direct - // SQL, never operator-authored. It must stay out of the declarative config surface - // (hidden on read) and be undeletable, so config sync can't read, rewrite, or drop it. - assert!(HIDDEN_SETTINGS.contains(&"custom_instance_replication_pwd")); - assert!(PROTECTED_SETTINGS.contains(&"custom_instance_replication_pwd")); - assert!(SENSITIVE_SETTINGS.contains(&"custom_instance_replication_pwd")); + fn server_generated_db_passwords_are_isolated_from_config() { + // These hold passwords the server generates: written by setup/refresh via direct SQL, + // never operator-authored. They must stay out of the declarative config surface + // (hidden on read) and be undeletable, so config sync can't read, rewrite, or drop them. + for key in [ + "custom_instance_replication_pwd", + "external_instance_pg_state", + ] { + assert!(HIDDEN_SETTINGS.contains(&key), "{key}"); + assert!(PROTECTED_SETTINGS.contains(&key), "{key}"); + assert!(SENSITIVE_SETTINGS.contains(&key), "{key}"); - // A stray desired value (e.g. flattened into `extra`) is ignored, not upserted. - let mut desired = BTreeMap::new(); - desired.insert( - "custom_instance_replication_pwd".to_string(), - serde_json::json!("attacker-set"), - ); - let diff = diff_global_settings(&BTreeMap::new(), &desired, ApplyMode::Merge); - assert!( - diff.upserts.is_empty(), - "hidden setting must not be upserted" - ); + // A stray desired value (e.g. flattened into `extra`) is ignored, not upserted. + let mut desired = BTreeMap::new(); + desired.insert(key.to_string(), serde_json::json!("attacker-set")); + let diff = diff_global_settings(&BTreeMap::new(), &desired, ApplyMode::Merge); + assert!(diff.upserts.is_empty(), "{key} must not be upserted"); - // A current value is never deleted by a Replace that omits it. - let mut current = BTreeMap::new(); - current.insert( - "custom_instance_replication_pwd".to_string(), - serde_json::json!("live"), - ); - let diff = diff_global_settings(¤t, &BTreeMap::new(), ApplyMode::Replace); - assert!( - !diff - .deletes - .contains(&"custom_instance_replication_pwd".to_string()), - "hidden setting must not be deleted" - ); + // A current value is never deleted by a Replace that omits it. + let mut current = BTreeMap::new(); + current.insert(key.to_string(), serde_json::json!("live")); + let diff = diff_global_settings(¤t, &BTreeMap::new(), ApplyMode::Replace); + assert!( + !diff.deletes.contains(&key.to_string()), + "{key} must not be deleted" + ); + } } #[test] diff --git a/backend/windmill-common/src/lib.rs b/backend/windmill-common/src/lib.rs index 47a7ea5b2a..0a1b0974d5 100644 --- a/backend/windmill-common/src/lib.rs +++ b/backend/windmill-common/src/lib.rs @@ -58,6 +58,10 @@ pub mod ee_oss; pub mod email_ee; pub mod email_oss; pub mod error; +pub mod external_instance_pg; +#[cfg(all(feature = "private", feature = "enterprise"))] +mod external_instance_pg_ee; +pub mod external_instance_pg_oss; pub mod external_ip; #[cfg(feature = "private")] pub mod feature_usage_ee; @@ -1115,7 +1119,13 @@ impl PgDatabase { if err_str.contains("password authentication failed for user") && err_str.contains("custom_instance_user") { - if let Some(db) = main_db { + // The external instance cluster has a `custom_instance_user` of its own, whose + // password setup manages. Rotating the local one would break every instance + // data table and fix nothing. + let local = PgDatabase::parse_uri(&get_database_url().await?.as_str().await)?; + let on_local_cluster = local.host == self.host + && local.port.unwrap_or(5432) == self.port.unwrap_or(5432); + if let Some(db) = main_db.filter(|_| on_local_cluster) { tracing::warn!( "custom_instance_user password auth failed, refreshing and retrying..." ); @@ -1665,13 +1675,41 @@ pub async fn instance_database_users( } /// Drop a custom instance database: validate, terminate connections, DROP DATABASE, remove from global_settings. +/// +/// Authorization: drops any instance database but Windmill's own and checks nothing. Callers MUST +/// be superadmin, or have established the caller may drop this one — a fork's owner cleaning up +/// its own copy that nothing else uses. pub async fn drop_custom_instance_database(db: &DB, dbname: &str) -> error::Result<()> { drop_custom_instance_database_on(&mut *db.acquire().await?, dbname).await } +/// [`drop_custom_instance_database`] leaving its registry entry, for a caller holding row locks in +/// a transaction: the registry write has to go through that transaction, as waiting on another +/// connection for a lock the transaction's own peers hold is a deadlock Postgres cannot see. Same +/// authorization contract. +pub async fn drop_custom_instance_database_keep_entry(db: &DB, dbname: &str) -> error::Result<()> { + drop_instance_database_keep_entry_on(&mut *db.acquire().await?, dbname).await +} + async fn drop_custom_instance_database_on( conn: &mut sqlx::PgConnection, dbname: &str, +) -> error::Result<()> { + let dbname = dbname.trim(); + drop_instance_database_keep_entry_on(&mut *conn, dbname).await?; + // Always remove from global_settings + sqlx::query!( + r#"UPDATE global_settings SET value = value #- ARRAY['databases', $1] WHERE name = 'custom_instance_pg_databases'"#, + dbname + ) + .execute(&mut *conn) + .await?; + Ok(()) +} + +async fn drop_instance_database_keep_entry_on( + conn: &mut sqlx::PgConnection, + dbname: &str, ) -> error::Result<()> { let dbname = dbname.trim(); validate_dbname(dbname)?; @@ -1718,14 +1756,6 @@ async fn drop_custom_instance_database_on( tracing::info!("Database '{}' does not exist, skipping drop", dbname); } - // Always remove from global_settings - sqlx::query!( - r#"UPDATE global_settings SET value = value #- ARRAY['databases', $1] WHERE name = 'custom_instance_pg_databases'"#, - dbname - ) - .execute(&mut *conn) - .await?; - Ok(()) } @@ -1750,26 +1780,30 @@ pub(crate) fn instance_db_grants(dbname: &str) -> String { ) } -/// Re-apply [`instance_db_grants`] to an instance database provisioned before data table roles -/// existed, whose grants carry no grant option. Connects as the instance's own Postgres user — -/// the database and `public` schema owner — since only it can hand out an option it holds. +/// Re-apply [`instance_db_grants`] to a managed database provisioned before data table roles +/// existed, whose grants carry no grant option. Connects as the cluster's administrator — the +/// database and `public` schema owner — since only it can hand out an option it holds. /// -/// Authorization: reaches an instance database with the server's own credentials and checks +/// Authorization: reaches a managed database with the server's own credentials and checks /// nothing. Callers MUST have authorized administration of `dbname` — superadmin, or an admin of /// the workspace governing a data table on it. pub async fn ensure_instance_db_grant_options_unchecked( db: &DB, + cluster: crate::datatable_roles::DatatableRoleCluster, dbname: &str, ) -> error::Result<()> { - crate::datatable_roles_oss::ensure_instance_db_grant_options_unchecked(db, dbname).await + crate::datatable_roles_oss::ensure_instance_db_grant_options_unchecked(db, cluster, dbname) + .await } /// Create a custom instance database: CREATE DATABASE, grant permissions, register in global_settings. -/// The `tag` is stored in global_settings metadata (e.g. "datatable" or "ducklake"). +/// The `tag` is stored in global_settings metadata (e.g. "datatable" or "ducklake"). `for_workspace` +/// is the workspace a member creates a fork copy for; see [`ensure_fork_database_available_to`]. pub async fn create_custom_instance_database( db: &DB, dbname: &str, tag: &str, + for_workspace: Option<&str>, ) -> error::Result<()> { let dbname = dbname.trim(); validate_dbname(dbname)?; @@ -1799,7 +1833,7 @@ pub async fn create_custom_instance_database( // Nothing names a database that failed past this point, and its name blocks the retry: drop it // rather than leave it behind. - if let Err(e) = finish_custom_instance_database(db, dbname, tag).await { + if let Err(e) = finish_custom_instance_database(db, dbname, tag, for_workspace).await { match drop_unused_instance_database(db, dbname).await { Ok(Cleanup::InUse(users)) => tracing::warn!( "Kept '{dbname}' after failing to set it up: workspaces {} use it", @@ -1820,7 +1854,13 @@ pub async fn create_custom_instance_database( // A data table role can only reach a database it may CONNECT to, and PUBLIC's default CONNECT // would otherwise let every role in regardless of what this instance defines. Best-effort: a // failure here leaves the database usable as `admin`, and the next role change repairs it. - if let Err(e) = crate::datatable_roles::converge_connect_grants(db, dbname).await { + if let Err(e) = crate::datatable_roles::converge_connect_grants( + db, + crate::datatable_roles::DatatableRoleCluster::Instance, + dbname, + ) + .await + { tracing::warn!("Could not set CONNECT grants on instance database '{dbname}': {e}"); } @@ -1829,7 +1869,12 @@ pub async fn create_custom_instance_database( } /// Grant `custom_instance_user` its privileges on a database just created, and register it. -async fn finish_custom_instance_database(db: &DB, dbname: &str, tag: &str) -> error::Result<()> { +async fn finish_custom_instance_database( + db: &DB, + dbname: &str, + tag: &str, + for_workspace: Option<&str>, +) -> error::Result<()> { // Grant permissions to custom_instance_user let wmill_pg_creds = PgDatabase::parse_uri(&get_database_url().await?.as_str().await)?; let new_pg_creds = PgDatabase { dbname: dbname.to_string(), ..wmill_pg_creds }; @@ -1856,7 +1901,8 @@ async fn finish_custom_instance_database(db: &DB, dbname: &str, tag: &str) -> er }, "success": true, "error": null, - "tag": tag + "tag": tag, + "workspace_id": for_workspace, }); sqlx::query!( r#"UPDATE global_settings SET value = jsonb_set(value, '{databases}', (COALESCE(value->'databases', '{}'::jsonb) || to_jsonb($1::json))) WHERE name = 'custom_instance_pg_databases'"#, @@ -1864,6 +1910,75 @@ async fn finish_custom_instance_database(db: &DB, dbname: &str, tag: &str) -> er ) .execute(db) .await?; + + Ok(()) +} + +/// The system's CA bundle file, for libpq clients that cannot take `sslrootcert=system`: that value +/// needs libpq 16, and verify-full only. +pub fn system_ca_bundle() -> Option { + std::env::var_os("SSL_CERT_FILE") + .map(std::path::PathBuf::from) + .into_iter() + .chain( + [ + "/etc/ssl/certs/ca-certificates.crt", + "/etc/pki/tls/certs/ca-bundle.crt", + "/etc/ssl/cert.pem", + "/etc/ssl/ca-bundle.pem", + ] + .map(std::path::PathBuf::from), + ) + .find(|path| path.is_file()) +} + +/// Refuse a workspace member writing a fork copy into, or pointing a fork at, the managed database +/// `dbname` of `kind`, unless `w_id` created it for that ([`create_custom_instance_database`], or +/// its external instance counterpart) and nothing uses it yet. The `wm_fork_` prefix is no +/// authorization: every database of a cluster answers to the same `custom_instance_user`, so a name +/// is all it takes to reach another workspace's copy. +/// +/// Runs on `conn`: its callers hold a transaction with the fork lock while they check, and a +/// second connection taken from the pool under it is how a small pool deadlocks. +/// +/// Authorization: reads the global registries and every workspace's settings, and names other +/// workspaces in its refusal. Callers MUST have authorized `w_id` for the caller first — a member +/// of it forking or importing there — and MUST NOT call it on a workspace the caller is not in. +pub async fn ensure_fork_database_available_to( + conn: &mut sqlx::PgConnection, + kind: workspaces::DataTableCatalogResourceType, + dbname: &str, + w_id: &str, +) -> error::Result<()> { + let created_for = match kind { + workspaces::DataTableCatalogResourceType::ExternalInstance => { + external_instance_pg::read_external_instance_pg_state(&mut *conn) + .await? + .databases + .remove(dbname) + .and_then(|entry| entry.workspace_id) + } + _ => sqlx::query_scalar::<_, Option>( + "SELECT value->'databases'->$1->>'workspace_id' FROM global_settings + WHERE name = 'custom_instance_pg_databases'", + ) + .bind(dbname) + .fetch_optional(&mut *conn) + .await? + .flatten(), + }; + if created_for.as_deref() != Some(w_id) { + return Err(Error::BadRequest(format!( + "Database '{dbname}' was not created for a fork of workspace '{w_id}'" + ))); + } + let uses = workspaces::managed_database_uses(conn, kind, dbname, None).await?; + if !uses.is_empty() { + return Err(Error::BadRequest(format!( + "Database '{dbname}' is already in use: {}", + uses.join(", ") + ))); + } Ok(()) } diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index b285332b7d..724743dce9 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -1542,6 +1542,47 @@ pub enum DataTableCatalogResourceType { #[strum(serialize = "postgres")] Postgresql, Instance, + /// On the external instance cluster ([`crate::external_instance_pg`]). Enterprise Edition. + #[serde(rename = "external_instance")] + #[strum(serialize = "external_instance")] + ExternalInstance, +} + +impl DataTableCatalogResourceType { + /// A database Windmill created and administers, on its own cluster or the external one, as + /// opposed to one a user brought as a resource. + pub fn is_windmill_managed(self) -> bool { + matches!(self, Self::Instance | Self::ExternalInstance) + } +} + +/// Refuse a new use of Windmill's own Postgres as a data table or Ducklake substrate where an +/// operator turned it off, and on the managed cloud, which never had it. Entries already on it +/// keep resolving: this gates what a save may newly name, not what runs. +pub async fn ensure_instance_pg_available<'c>(executor: impl sqlx::PgExecutor<'c>) -> Result<()> { + if *crate::worker::CLOUD_HOSTED { + return Err(Error::BadRequest( + "Windmill's own database cannot back a data table or Ducklake catalog on Windmill Cloud" + .to_string(), + )); + } + let disabled = sqlx::query_scalar::<_, Option>( + "SELECT value FROM global_settings WHERE name = $1", + ) + .bind(crate::global_settings::INSTANCE_PG_DISABLED_SETTING) + .fetch_optional(executor) + .await? + .flatten() + .is_some_and(|v| v.as_bool().unwrap_or(false)); + if disabled { + return Err(Error::BadRequest( + "Windmill's own database is disabled as a data table and Ducklake substrate on this \ + instance. Use the external instance cluster, or turn it back on in the instance \ + settings." + .to_string(), + )); + } + Ok(()) } /// Build a self-teaching error for an unresolved `datatable://` reference. @@ -1621,14 +1662,82 @@ pub struct GoverningDatatable { pub governor: Option, } +/// Everything still using the Windmill-managed database `dbname`, one description per use: data +/// table entries naming it, fork entries pointing at those, Ducklake catalogs on it, and fork +/// Ducklake metadata schemas there that cleanup has not dropped yet. `exempt` is the one data table +/// entry, `(workspace_id, name)`, the caller is about to stop using it through; pointers at that +/// entry still count, since dropping the database would leave them resolving to nothing. +/// +/// Authorization: reads every workspace's settings and checks nothing. Callers MUST only turn the +/// answer into a refusal for someone allowed to administer `dbname`. +pub async fn managed_database_uses( + conn: &mut sqlx::PgConnection, + kind: DataTableCatalogResourceType, + dbname: &str, + exempt: Option<(&str, &str)>, +) -> Result> { + let (exempt_workspace, exempt_name) = exempt.unzip(); + Ok(sqlx::query_scalar::<_, String>( + "WITH entries AS ( + SELECT ws.workspace_id::text AS workspace_id, dt.key AS name, dt.value + FROM workspace_settings ws + CROSS JOIN LATERAL jsonb_each( + CASE WHEN jsonb_typeof(ws.datatable->'datatables') = 'object' + THEN ws.datatable->'datatables' ELSE '{}'::jsonb END) dt + ), naming AS ( + SELECT workspace_id, name FROM entries + WHERE value->'database'->>'resource_type' = $1 + AND value->'database'->>'resource_path' = $2 + ) + SELECT format('data table ''%s'' in workspace ''%s''', name, workspace_id) FROM naming + WHERE $3::text IS NULL OR NOT (workspace_id = $3 AND name = $4) + UNION ALL + SELECT format('data table ''%s'' in workspace ''%s'', which points at the one in ''%s''', + e.name, e.workspace_id, n.workspace_id) + FROM entries e JOIN naming n + ON e.value->'reference'->>'workspace_id' = n.workspace_id + AND e.value->'reference'->>'datatable' = n.name + UNION ALL + SELECT format('Ducklake ''%s'' in workspace ''%s''', dl.key, ws.workspace_id) + FROM workspace_settings ws + CROSS JOIN LATERAL jsonb_each( + CASE WHEN jsonb_typeof(ws.ducklake->'ducklakes') = 'object' + THEN ws.ducklake->'ducklakes' ELSE '{}'::jsonb END) dl + WHERE dl.value->'catalog'->>'resource_type' = $1 + AND dl.value->'catalog'->>'resource_path' = $2 + UNION ALL + SELECT format('the Ducklake namespace of fork ''%s'', not cleaned up yet', workspace_id) + FROM fork_ducklake_namespace + WHERE catalog = $1 || ':' || $2 AND NOT schema_dropped + ORDER BY 1", + ) + .bind(kind.as_ref()) + .bind(dbname) + .bind(exempt_workspace) + .bind(exempt_name) + .fetch_all(&mut *conn) + .await?) +} + +/// Held by fork cleanup of `w_id`'s data tables and by forking `w_id`, which can hand the new fork +/// pointers at them, so a pointer cannot appear between cleanup's check and its drop. +pub async fn lock_fork_datatables(conn: &mut sqlx::PgConnection, w_id: &str) -> Result<()> { + sqlx::query("SELECT pg_advisory_xact_lock(hashtext('fork_datatables:' || $1))") + .bind(w_id) + .execute(&mut *conn) + .await?; + Ok(()) +} + impl GoverningDatatable { - /// Backed by the Windmill instance's own Postgres, which is the only substrate data table - /// roles apply to. - pub fn is_instance(&self) -> bool { + /// The Windmill-managed cluster whose data table roles this entry can use. `None` for a + /// resource-backed one: roles are logins Windmill creates, and it creates none on a host a + /// workspace admin chose. + pub fn role_cluster(&self) -> Option { self.datatable .database .as_ref() - .is_some_and(|d| d.resource_type == DataTableCatalogResourceType::Instance) + .and_then(|d| crate::datatable_roles::DatatableRoleCluster::of(d.resource_type)) } /// The workspace whose admins administer the data table and whose members its tenants are. @@ -1776,20 +1885,19 @@ pub async fn resolve_workspace_governing_datatables( clone, )), (None, Some(governed_by)) => { - let (governor_ws, governor_name) = - (governed_by.workspace_id.clone(), governed_by.datatable.clone()); + let (governor_ws, governor_name) = ( + governed_by.workspace_id.clone(), + governed_by.datatable.clone(), + ); let clone = clone.or(Some((ws, name, datatable))); next.push((i, governor_ws, governor_name, clone)); } (None, None) => resolved.push(( i, match clone { - None => GoverningDatatable { - workspace_id: ws, - name, - datatable, - governor: None, - }, + None => { + GoverningDatatable { workspace_id: ws, name, datatable, governor: None } + } Some((clone_ws, clone_name, mut clone_datatable)) => { clone_datatable.permissions = datatable.permissions; GoverningDatatable { @@ -1830,7 +1938,8 @@ pub async fn resolve_workspace_governing_datatables( } /// Build the `admin` connection for a governing entry: `custom_instance_user` for an instance -/// database, the user's own resource for a BYO-postgres one. +/// database, on Windmill's cluster or the external one; the user's own resource for a BYO-postgres +/// one. async fn resolve_datatable_connection_unchecked( db: &DB, governing: &GoverningDatatable, @@ -1841,7 +1950,16 @@ async fn resolve_datatable_connection_unchecked( .database .as_ref() .expect("a governing entry owns a database"); - if database.resource_type == DataTableCatalogResourceType::Instance { + if database.resource_type == DataTableCatalogResourceType::ExternalInstance { + let pg_creds = crate::external_instance_pg::external_instance_connection_unchecked( + db, + &database.resource_path, + replication, + ) + .await?; + serde_json::to_value(&pg_creds) + .map_err(|e| Error::internal_err(format!("Error serializing pg creds: {}", e))) + } else if database.resource_type == DataTableCatalogResourceType::Instance { let mut pg_creds = PgDatabase::parse_uri(&get_database_url().await?.as_str().await)?; pg_creds.dbname = database.resource_path.clone(); if replication { @@ -1879,8 +1997,31 @@ pub async fn get_datatable_resource_from_db_unchecked( w_id: &str, name: &str, ) -> Result { + Ok(get_datatable_connection_and_kind_unchecked(db, w_id, name) + .await? + .0) +} + +/// As [`get_datatable_resource_from_db_unchecked`], also reporting the kind of database Windmill +/// manages behind it, `None` for a user resource. One resolution answers both: a caller reading the +/// kind separately can be handed one kind's connection and the other kind's checks by a save +/// landing between the two, and the entry a pointer lands on is another workspace's to change. +/// +/// Same authorization contract: the connection reaches every role. +pub async fn get_datatable_connection_and_kind_unchecked( + db: &DB, + w_id: &str, + name: &str, +) -> Result<(serde_json::Value, Option)> { let governing = resolve_governing_datatable(db, w_id, name).await?; - resolve_datatable_connection_unchecked(db, &governing, false).await + let kind = governing + .datatable + .database + .as_ref() + .map(|d| d.resource_type) + .filter(|kind| kind.is_windmill_managed()); + let connection = resolve_datatable_connection_unchecked(db, &governing, false).await?; + Ok((connection, kind)) } /// Same as [`get_datatable_resource_from_db_unchecked`] but for postgres trigger @@ -2385,6 +2526,10 @@ pub enum DucklakeCatalogResourceType { Postgresql, Mysql, Instance, + /// On the external instance cluster ([`crate::external_instance_pg`]). Enterprise Edition. + #[serde(rename = "external_instance")] + #[strum(serialize = "external_instance")] + ExternalInstance, } #[derive(Deserialize, Serialize)] @@ -2912,7 +3057,16 @@ async fn ducklake_conn_data( let ducklake = serde_json::from_value::(ducklake)?; let catalog_resource = - if ducklake.catalog.resource_type == DucklakeCatalogResourceType::Instance { + if ducklake.catalog.resource_type == DucklakeCatalogResourceType::ExternalInstance { + let pg_creds = crate::external_instance_pg::external_instance_connection_unchecked( + db, + &ducklake.catalog.resource_path, + false, + ) + .await?; + serde_json::to_value(&pg_creds) + .map_err(|e| Error::internal_err(format!("Error serializing pg creds: {}", e)))? + } else if ducklake.catalog.resource_type == DucklakeCatalogResourceType::Instance { let mut pg_creds = PgDatabase::parse_uri(&get_database_url().await?.as_str().await)?; pg_creds.dbname = ducklake.catalog.resource_path.clone(); pg_creds.user = Some("custom_instance_user".to_string()); @@ -3293,6 +3447,14 @@ async fn register_fork_ducklake_namespace( { return Ok(()); } + let mut tx = db.begin().await?; + // A row naming an external database counts as a use of it. Written under the lock a drop takes, + // and only while the database is still registered, so a drop cannot slip in between the + // settings this attach resolved and the row that protects the database. + if let Some(dbname) = catalog.strip_prefix("external_instance:") { + crate::external_instance_pg::ensure_external_instance_database_registered(&mut tx, dbname) + .await?; + } sqlx::query!( "INSERT INTO fork_ducklake_namespace (workspace_id, ducklake_name, metadata_schema, catalog, storage, storage_ref, data_path) @@ -3307,9 +3469,10 @@ async fn register_fork_ducklake_namespace( &storage_ref, data_path, ) - .execute(db) + .execute(&mut *tx) .await .map_err(|e| Error::internal_err(format!("registering fork ducklake namespace: {e:#}")))?; + tx.commit().await?; let mut locations = FORK_DUCKLAKE_REGISTERED .get(w_id) .filter(|(_, exp)| *exp > now) diff --git a/backend/windmill-worker/src/duckdb_executor.rs b/backend/windmill-worker/src/duckdb_executor.rs index 943703472f..d3bd976f21 100644 --- a/backend/windmill-worker/src/duckdb_executor.rs +++ b/backend/windmill-worker/src/duckdb_executor.rs @@ -1475,6 +1475,7 @@ pub async fn do_duckdb( &job.id, client, &mut hidden_passwords, + job_dir, ) .await?, ); @@ -1490,13 +1491,19 @@ pub async fn do_duckdb( &mut hidden_passwords, &job.workspace_id, materialize.as_ref().map(|(_, m)| m.asset_path.as_str()), + job_dir, ) .await? { probe_blocks.extend(q); - } else if let Some(q) = - transform_attach_datatable(&query_block, conn, &mut hidden_passwords, job) - .await? + } else if let Some(q) = transform_attach_datatable( + &query_block, + conn, + &mut hidden_passwords, + job, + job_dir, + ) + .await? { probe_blocks.extend(q); } else { @@ -1552,6 +1559,7 @@ pub async fn do_duckdb( &job.id, client, &mut hidden_passwords, + job_dir, ) .await?, ); @@ -1567,13 +1575,19 @@ pub async fn do_duckdb( &mut hidden_passwords, &job.workspace_id, materialize.as_ref().map(|(_, m)| m.asset_path.as_str()), + job_dir, ) .await? { v.extend(ducklake_query); - } else if let Some(datatable_query) = - transform_attach_datatable(&query_block, conn, &mut hidden_passwords, job) - .await? + } else if let Some(datatable_query) = transform_attach_datatable( + &query_block, + conn, + &mut hidden_passwords, + job, + job_dir, + ) + .await? { v.extend(datatable_query); } else { @@ -2240,11 +2254,80 @@ fn parse_attach_db_resource<'a>(query: &'a str) -> Option Result { +/// The verification a DuckDB postgres attach keeps, as its libpq `sslmode` and `sslrootcert`. +/// +/// Attaches have always turned verify-ca and verify-full into `require`, which resources rely on. +/// A connection that explicitly refuses invalid certificates — the external instance cluster's — +/// keeps its mode instead: under `require` its shared password would go to whichever server +/// answers. DuckDB's libpq takes one root file, so it gets the system bundle plus the configured +/// certificate, written in the job directory: a resource's certificate is workspace-controlled, so +/// a file per distinct one has to go with the job rather than pile up on the worker. +fn pg_attach_verification<'a>( + res: &'a PgDatabase, + job_dir: &str, +) -> Result> { + let mode = match res.sslmode.as_deref() { + // The rule every other Postgres connection uses, so a resource that verifies elsewhere is + // not quietly downgraded here. + Some(mode @ ("verify-ca" | "verify-full")) if !res.verify_mode_skips_verification() => mode, + _ => return Ok(None), + }; + let bundle = windmill_common::system_ca_bundle() + .map(std::fs::read_to_string) + .transpose() + .map_err(|e| Error::ExecutionErr(format!("Failed to read the system CA bundle: {e}")))? + .unwrap_or_default(); + let pem = res.root_certificate_pem.as_deref().unwrap_or_default(); + if bundle.is_empty() && pem.is_empty() { + return Err(Error::ExecutionErr(format!( + "sslmode {mode} needs a root certificate, and this worker has no system CA bundle" + ))); + } + let roots = format!("{bundle}\n{pem}\n"); + use sha2::Digest; + let path = std::path::Path::new(job_dir).join(format!( + "pg_roots_{}.pem", + hex::encode(&sha2::Sha256::digest(roots.as_bytes())[..8]) + )); + // The job's modules are written in this directory first, so whatever already sits at this path + // may be caller-supplied: trusting it would let the caller pick the CA. Replace it, and + // `create_new` refuses to write through anything recreated there. + let write_roots = || -> std::io::Result<()> { + match std::fs::remove_file(&path) { + Err(e) if e.kind() != std::io::ErrorKind::NotFound => return Err(e), + _ => {} + } + use std::io::Write; + std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&path)? + .write_all(roots.as_bytes()) + }; + write_roots() + .map_err(|e| Error::ExecutionErr(format!("Failed to write root certificates: {e}")))?; + Ok(Some((mode, path))) +} + +fn pg_attach_uri(res: &PgDatabase, job_dir: &str) -> Result { + let uri = res.to_uri(); + let Some((mode, roots)) = pg_attach_verification(res, job_dir)? else { + return Ok(uri); + }; + let base = uri.strip_suffix("?sslmode=require").ok_or_else(|| { + Error::internal_err("unexpected sslmode in a postgres connection URI".to_string()) + })?; + Ok(format!( + "{base}?sslmode={mode}&sslrootcert={}", + urlencoding::encode(&roots.to_string_lossy()) + )) +} + +fn format_attach_db_conn_str(db_resource: Value, db_type: &str, job_dir: &str) -> Result { let s = match db_type.to_lowercase().as_str() { "postgres" | "postgresql" => { let res: PgDatabase = serde_json::from_value(db_resource)?; - res.to_uri() + pg_attach_uri(&res, job_dir)? } #[cfg(feature = "mysql")] "mysql" => { @@ -2316,6 +2399,7 @@ async fn transform_attach_db_resource_query( job_id: &Uuid, client: &AuthedClient, hidden_passwords: &mut Arc>>, + job_dir: &str, ) -> Result> { let db_resource: Value = client .get_resource_value_interpolated(parsed.resource_path, Some(job_id.to_string())) @@ -2323,8 +2407,14 @@ async fn transform_attach_db_resource_query( if let Some(pwd) = db_resource.get("password").and_then(|p| p.as_str()) { hidden_passwords.lock().unwrap().push(pwd.to_string()); } - db_resource_to_attach_statements(db_resource, parsed.name, parsed.db_type, parsed.extra_args) - .await + db_resource_to_attach_statements( + db_resource, + parsed.name, + parsed.db_type, + parsed.extra_args, + job_dir, + ) + .await } async fn db_resource_to_attach_statements( @@ -2332,11 +2422,12 @@ async fn db_resource_to_attach_statements( ident_name: &str, db_type: &str, extra_args: Option<&str>, + job_dir: &str, ) -> Result> { // Escape single quotes: the connection string is built from resource fields // (host/db/user/password) and embedded in a single-quoted DuckDB literal, so an // unescaped quote in any field would otherwise break out of the ATTACH statement. - let conn_str = format_attach_db_conn_str(db_resource, db_type)?.replace('\'', "''"); + let conn_str = format_attach_db_conn_str(db_resource, db_type, job_dir)?.replace('\'', "''"); let attach_str = format!( "ATTACH '{}' as {} (TYPE {}{});", conn_str, @@ -2359,6 +2450,7 @@ async fn transform_attach_ducklake( hidden_passwords: &mut Arc>>, w_id: &str, materialize_target: Option<&str>, + job_dir: &str, ) -> Result>> { lazy_static::lazy_static! { static ref RE: regex::Regex = regex::Regex::new(r"(?i)ATTACH\s*'ducklake(://[^':]+)?'\s*AS\s+([^ ;]+)\s*(\([^)]*\))?").unwrap(); @@ -2391,7 +2483,9 @@ async fn transform_attach_ducklake( format!(", {}", user_extra_args) }; let db_type = match ducklake.catalog.resource_type { - DucklakeCatalogResourceType::Instance => "postgres", + DucklakeCatalogResourceType::Instance | DucklakeCatalogResourceType::ExternalInstance => { + "postgres" + } _ => ducklake.catalog.resource_type.as_ref(), }; @@ -2407,7 +2501,7 @@ async fn transform_attach_ducklake( // single-quoted DuckDB literals below, so an unescaped quote in a resource // field would break out of the ATTACH statement. let db_conn_str = - format_attach_db_conn_str(ducklake.catalog_resource, db_type)?.replace('\'', "''"); + format_attach_db_conn_str(ducklake.catalog_resource, db_type, job_dir)?.replace('\'', "''"); let storage = ducklake .storage .storage @@ -2462,6 +2556,7 @@ async fn transform_attach_ducklake( defer, materialize_target, hidden_passwords, + job_dir, )?); } Ok(Some(statements)) @@ -2495,6 +2590,7 @@ fn fork_defer_statements( defer: &windmill_common::workspaces::DucklakeForkDefer, materialize_target: Option<&str>, hidden_passwords: &mut Arc>>, + job_dir: &str, ) -> Result> { let mut stmts = vec![]; if defer.ancestors.is_empty() { @@ -2507,12 +2603,13 @@ fn fork_defer_statements( hidden_passwords.lock().unwrap().push(pwd.to_string()); } let db_type = match a.catalog.resource_type { - DucklakeCatalogResourceType::Instance => "postgres", + DucklakeCatalogResourceType::Instance + | DucklakeCatalogResourceType::ExternalInstance => "postgres", _ => a.catalog.resource_type.as_ref(), }; stmts.push(get_attach_db_install_str(db_type)?.to_string()); - let conn_str = - format_attach_db_conn_str(a.catalog_resource.clone(), db_type)?.replace('\'', "''"); + let conn_str = format_attach_db_conn_str(a.catalog_resource.clone(), db_type, job_dir)? + .replace('\'', "''"); let storage = a .storage .storage @@ -2631,6 +2728,7 @@ async fn transform_attach_datatable( conn: &Connection, hidden_passwords: &mut Arc>>, job: &MiniPulledJob, + job_dir: &str, ) -> Result>> { let Some(attached) = parse_attach_datatable(query) else { return Ok(None); @@ -2673,6 +2771,7 @@ async fn transform_attach_datatable( Ok(Some(pg_secret_attach_statements( db_resource, attached.alias, + job_dir, )?)) } @@ -2693,17 +2792,32 @@ fn datatable_secret_name(alias: &str) -> String { /// ATTACH a datatable's postgres database through a DuckDB TEMPORARY SECRET holding /// the connection parameters; only sslmode and options ride in the ATTACH string. -fn pg_secret_attach_statements(db_resource: Value, alias_name: &str) -> Result> { +fn pg_secret_attach_statements( + db_resource: Value, + alias_name: &str, + job_dir: &str, +) -> Result> { let res: PgDatabase = serde_json::from_value(db_resource)?; // Escape single quotes: each field is embedded in a single-quoted DuckDB literal, // so an unescaped quote would break out of the CREATE SECRET statement. let esc = |s: &str| s.replace('\'', "''"); // The postgres secret type has no sslmode parameter, so it goes in the ATTACH // string; only the libpq values PgDatabase::to_uri collapses to are forwarded. - let sslmode = match res.sslmode.as_deref() { - Some("disable") => "disable", - Some("require") | Some("verify-ca") | Some("verify-full") => "require", - _ => "prefer", + let sslmode = match pg_attach_verification(&res, job_dir)? { + // A libpq keyword/value string: the path is quoted for libpq, then for the DuckDB literal. + Some((mode, roots)) => format!( + "{mode} sslrootcert=''{}''", + roots + .to_string_lossy() + .replace('\\', "\\\\") + .replace('\'', "\\''") + ), + None => match res.sslmode.as_deref() { + Some("disable") => "disable", + Some("require") | Some("verify-ca") | Some("verify-full") => "require", + _ => "prefer", + } + .to_string(), }; // Nor an options parameter. The value is quoted for libpq's keyword/value syntax first, // then escaped for the DuckDB literal around it. @@ -2805,6 +2919,76 @@ pub struct Arg { mod tests { use super::*; + #[test] + fn pg_attach_keeps_verification_only_when_required() { + let job_dir = std::env::temp_dir().join(format!("wm-test-{}", uuid::Uuid::new_v4())); + std::fs::create_dir_all(&job_dir).unwrap(); + let job_dir = job_dir.to_string_lossy().to_string(); + let pg = |sslmode: &str, accept_invalid_certs: Option| PgDatabase { + host: "db.internal".to_string(), + user: Some("custom_instance_user".to_string()), + password: Some("pw".to_string()), + port: None, + sslmode: Some(sslmode.to_string()), + dbname: "dt".to_string(), + root_certificate_pem: Some("-----BEGIN CERTIFICATE-----test".to_string()), + accept_invalid_certs, + use_iam_auth: None, + region: None, + options: None, + }; + let uri = pg_attach_uri(&pg("verify-full", Some(false)), &job_dir).unwrap(); + assert!(uri.contains("?sslmode=verify-full&sslrootcert="), "{uri}"); + let root = urlencoding::decode(uri.split("sslrootcert=").nth(1).unwrap()).unwrap(); + assert!(std::fs::read_to_string(root.as_ref()) + .unwrap() + .contains("-----BEGIN CERTIFICATE-----test")); + // A file a job module planted under the same name is not trusted. + std::fs::write(root.as_ref(), "-----BEGIN CERTIFICATE-----planted").unwrap(); + assert_eq!( + pg_attach_uri(&pg("verify-full", Some(false)), &job_dir).unwrap(), + uri + ); + assert!(!std::fs::read_to_string(root.as_ref()) + .unwrap() + .contains("planted")); + // Every certificate a job attaches keeps its own file: one attach must not evict another's. + for i in 0..40 { + let mut other = pg("verify-full", Some(false)); + other.root_certificate_pem = Some(format!("-----BEGIN CERTIFICATE-----{i}")); + let other = pg_attach_uri(&other, &job_dir).unwrap(); + let path = urlencoding::decode(other.split("sslrootcert=").nth(1).unwrap()).unwrap(); + assert!(std::path::Path::new(path.as_ref()).is_file(), "{path}"); + } + assert!( + std::path::Path::new(root.as_ref()).is_file(), + "the first file is still there" + ); + let external = serde_json::to_value(pg("verify-full", Some(false))).unwrap(); + let attach = &pg_secret_attach_statements(external, "dt", &job_dir).unwrap()[3]; + assert!( + attach.starts_with(&format!( + "ATTACH 'sslmode=verify-full sslrootcert=''{}''", + root + )), + "{attach}" + ); + // A resource carrying a root certificate verifies without opting in, as it does on every + // other Postgres path; one carrying neither keeps the historical downgrade. + assert!(pg_attach_uri(&pg("verify-full", None), &job_dir) + .unwrap() + .contains("?sslmode=verify-full&sslrootcert=")); + let mut bare = pg("verify-full", None); + bare.root_certificate_pem = None; + assert!(pg_attach_uri(&bare, &job_dir) + .unwrap() + .ends_with("?sslmode=require")); + assert!(pg_attach_uri(&pg("require", Some(false)), &job_dir) + .unwrap() + .ends_with("?sslmode=require")); + std::fs::remove_dir_all(&job_dir).unwrap(); + } + #[test] fn attach_datatable_parses_name_and_role() { let reference_of = |q: &str| parse_attach_datatable(q).unwrap().reference; @@ -2956,7 +3140,7 @@ mod tests { let mut defer = test_fork_defer(vec![("orders", false)], vec![]); defer.ancestors[0].extra_args = Some("ENCRYPTED true".to_string()); let mut hp = Arc::new(Mutex::new(vec![])); - let stmts = fork_defer_statements("lake", "dl", &defer, None, &mut hp).unwrap(); + let stmts = fork_defer_statements("lake", "dl", &defer, None, &mut hp, "/tmp").unwrap(); let attach = stmts .iter() .find(|s| s.starts_with("ATTACH IF NOT EXISTS")) @@ -2980,7 +3164,7 @@ mod tests { vec![], ); let mut hp = Arc::new(Mutex::new(vec![])); - let stmts = fork_defer_statements("lake", "dl", &defer, None, &mut hp).unwrap(); + let stmts = fork_defer_statements("lake", "dl", &defer, None, &mut hp, "/tmp").unwrap(); let joined = stmts.join("\n"); assert!( joined.contains( @@ -3012,7 +3196,7 @@ mod tests { fn test_fork_defer_statements_shape() { let defer = test_fork_defer(vec![("orders", false), ("dim", true)], vec![]); let mut hp = Arc::new(Mutex::new(vec![])); - let stmts = fork_defer_statements("lake", "dl", &defer, None, &mut hp).unwrap(); + let stmts = fork_defer_statements("lake", "dl", &defer, None, &mut hp, "/tmp").unwrap(); let joined = stmts.join("\n"); // Ancestor attach: read-only, idempotent, never auto-migrating or auto-creating. assert!(joined.contains("ATTACH IF NOT EXISTS"), "{joined}"); @@ -3038,9 +3222,15 @@ mod tests { // Target currently a defer view → skip its CREATE, drop the view (+ companion). let defer = test_fork_defer(vec![("orders", false)], vec!["orders", "orders_current"]); let mut hp = Arc::new(Mutex::new(vec![])); - let stmts = - fork_defer_statements("lake", "_wm_target", &defer, Some("lake/orders"), &mut hp) - .unwrap(); + let stmts = fork_defer_statements( + "lake", + "_wm_target", + &defer, + Some("lake/orders"), + &mut hp, + "/tmp", + ) + .unwrap(); let joined = stmts.join("\n"); assert!(!joined.contains("CREATE VIEW"), "{joined}"); assert!( @@ -3055,15 +3245,22 @@ mod tests { // Target already a real table (NOT in fork_views, e.g. after a failed re-run whose // status can't be trusted) → no DROP VIEW, or the job would wedge on a type mismatch. let defer = test_fork_defer(vec![("orders", false)], vec![]); - let stmts = - fork_defer_statements("lake", "_wm_target", &defer, Some("lake/orders"), &mut hp) - .unwrap(); + let stmts = fork_defer_statements( + "lake", + "_wm_target", + &defer, + Some("lake/orders"), + &mut hp, + "/tmp", + ) + .unwrap(); assert!(!stmts.join("\n").contains("DROP VIEW"), "{stmts:?}"); // Target in a different lake → this lake's defer views are untouched. let defer = test_fork_defer(vec![("orders", false)], vec!["orders"]); let stmts = - fork_defer_statements("lake", "dl", &defer, Some("other/orders"), &mut hp).unwrap(); + fork_defer_statements("lake", "dl", &defer, Some("other/orders"), &mut hp, "/tmp") + .unwrap(); let joined = stmts.join("\n"); assert!( joined.contains("CREATE VIEW IF NOT EXISTS dl.\"orders\""), @@ -3076,7 +3273,7 @@ mod tests { fn test_fork_defer_statements_schema_qualified() { let defer = test_fork_defer(vec![("staging.raw", false)], vec![]); let mut hp = Arc::new(Mutex::new(vec![])); - let stmts = fork_defer_statements("lake", "dl", &defer, None, &mut hp).unwrap(); + let stmts = fork_defer_statements("lake", "dl", &defer, None, &mut hp, "/tmp").unwrap(); let joined = stmts.join("\n"); assert!( joined.contains("CREATE SCHEMA IF NOT EXISTS dl.\"staging\";"), @@ -3902,7 +4099,7 @@ mod tests { "dbname": "mydb", "sslmode": "require" }); - let result = format_attach_db_conn_str(db_resource, "postgres").unwrap(); + let result = format_attach_db_conn_str(db_resource, "postgres", "/tmp").unwrap(); // Should be in URI format: postgres://user:password@host:port/dbname?sslmode=require assert!(result.starts_with("postgres://")); assert!(result.contains("admin:secret123@localhost:5432/mydb")); @@ -3915,7 +4112,7 @@ mod tests { "host": "db.example.com", "dbname": "production" }); - let result = format_attach_db_conn_str(db_resource, "postgres").unwrap(); + let result = format_attach_db_conn_str(db_resource, "postgres", "/tmp").unwrap(); // Should be in URI format with defaults: postgres://postgres:@host:5432/dbname?sslmode=prefer assert!(result.starts_with("postgres://")); assert!(result.contains("@db.example.com:5432/production")); @@ -3928,7 +4125,7 @@ mod tests { "host": "localhost", "dbname": "test" }); - let result = format_attach_db_conn_str(db_resource, "postgresql").unwrap(); + let result = format_attach_db_conn_str(db_resource, "postgresql", "/tmp").unwrap(); // Should be in URI format (postgresql is treated the same as postgres) assert!(result.starts_with("postgres://")); assert!(result.contains("@localhost:5432/test")); @@ -3945,7 +4142,7 @@ mod tests { "dbname": "wm_datatables", "sslmode": "require" }); - let stmts = pg_secret_attach_statements(db_resource, "dt").unwrap(); + let stmts = pg_secret_attach_statements(db_resource, "dt", "/tmp").unwrap(); assert_eq!(stmts[0], "INSTALL postgres;"); assert_eq!(stmts[1], "LOAD postgres;"); let secret_name = datatable_secret_name("dt"); @@ -3976,7 +4173,7 @@ mod tests { if let Some(s) = input { db_resource["sslmode"] = json!(s); } - let stmts = pg_secret_attach_statements(db_resource, "dt").unwrap(); + let stmts = pg_secret_attach_statements(db_resource, "dt", "/tmp").unwrap(); assert!( stmts[3].starts_with(&format!("ATTACH 'sslmode={expected}'")), "sslmode {input:?} → {}", @@ -3988,7 +4185,7 @@ mod tests { #[test] fn test_pg_secret_attach_statements_options() { let db_resource = json!({ "host": "h", "dbname": "d", "options": r"-c search_path='a\b'" }); - let stmts = pg_secret_attach_statements(db_resource, "dt").unwrap(); + let stmts = pg_secret_attach_statements(db_resource, "dt", "/tmp").unwrap(); let secret_name = datatable_secret_name("dt"); assert_eq!( stmts[3], @@ -4012,7 +4209,7 @@ mod tests { let db_resource = json!({ "project_id": "my-gcp-project" }); - let result = format_attach_db_conn_str(db_resource, "bigquery").unwrap(); + let result = format_attach_db_conn_str(db_resource, "bigquery", "/tmp").unwrap(); assert_eq!(result, "project=my-gcp-project"); } @@ -4021,7 +4218,7 @@ mod tests { let db_resource = json!({ "other_field": "value" }); - let result = format_attach_db_conn_str(db_resource, "bigquery"); + let result = format_attach_db_conn_str(db_resource, "bigquery", "/tmp"); assert!(result.is_err()); assert!(result.unwrap_err().to_string().contains("project_id")); } @@ -4029,7 +4226,7 @@ mod tests { #[test] fn test_format_attach_db_conn_str_unsupported_type() { let db_resource = json!({}); - let result = format_attach_db_conn_str(db_resource, "oracle"); + let result = format_attach_db_conn_str(db_resource, "oracle", "/tmp"); assert!(result.is_err()); assert!(result .unwrap_err() @@ -4043,7 +4240,7 @@ mod tests { "host": "localhost", "dbname": "test" }); - let result = format_attach_db_conn_str(db_resource, "POSTGRES").unwrap(); + let result = format_attach_db_conn_str(db_resource, "POSTGRES", "/tmp").unwrap(); // Should be in URI format assert!(result.starts_with("postgres://")); assert!(result.contains("@localhost:5432/test")); @@ -4060,7 +4257,7 @@ mod tests { "database": "app_db", "ssl": true }); - let result = format_attach_db_conn_str(db_resource, "mysql").unwrap(); + let result = format_attach_db_conn_str(db_resource, "mysql", "/tmp").unwrap(); assert!(result.contains("database=app_db")); assert!(result.contains("host=mysql.example.com")); assert!(result.contains("ssl_mode=required")); @@ -4077,7 +4274,7 @@ mod tests { "database": "test", "ssl": false }); - let result = format_attach_db_conn_str(db_resource, "mysql").unwrap(); + let result = format_attach_db_conn_str(db_resource, "mysql", "/tmp").unwrap(); assert!(result.contains("ssl_mode=disabled")); } diff --git a/cli/src/commands/datatable/datatable.ts b/cli/src/commands/datatable/datatable.ts index 6609d08c50..46ee7b4c39 100644 --- a/cli/src/commands/datatable/datatable.ts +++ b/cli/src/commands/datatable/datatable.ts @@ -111,6 +111,8 @@ const migrateCommand = new Command() ) .action(migrateDown as any); +type DataTableResourceType = "postgresql" | "instance" | "external_instance"; + async function create( opts: GlobalOptions & { resource?: string; force?: boolean }, name?: string, @@ -139,12 +141,12 @@ async function create( const datatables: Record< string, - { database: { resource_type: "postgresql" | "instance"; resource_path?: string } } + { database: { resource_type: DataTableResourceType; resource_path?: string } } > = {}; for (const d of existing) { datatables[d.name] = { database: { - resource_type: d.resource_type as "postgresql" | "instance", + resource_type: d.resource_type as DataTableResourceType, resource_path: d.resource_path ?? undefined, }, }; diff --git a/docs/external-instance-datatables.md b/docs/external-instance-datatables.md new file mode 100644 index 0000000000..acfb93b90a --- /dev/null +++ b/docs/external-instance-datatables.md @@ -0,0 +1,88 @@ +# External instance data tables + +A data table is backed by one of three things: a Postgres resource a workspace brings +(`postgresql`), a database on Windmill's own cluster (`instance`), or a database on a separate +cluster Windmill administers (`external_instance`, Enterprise Edition). The third is what this +document covers; Ducklake catalogs take the same three shapes. + +Windmill administers the external cluster the way it administers its own: it creates and drops +databases there, owns `custom_instance_user` and `custom_instance_replication_user`, and creates +the data table roles of that cluster. It logs in as the admin in the `external_instance_pg` +instance setting, and keeps what it generates in the hidden `external_instance_pg_state` setting. + +## Code + +| Where | What | +|---|---| +| `windmill-common/src/external_instance_pg.rs` | Setting, state, usage accounting, the lifecycle lock, the OSS forwarders | +| `windmill-common/src/external_instance_pg_ee.rs` | Setup, database create and drop, the admin connection | +| `windmill-common/src/datatable_roles.rs` | Per-cluster role catalogs (`DatatableRoleCluster`) | +| `windmill-common/src/workspaces.rs` | Resolution (`resolve_datatable_connection_unchecked`), `managed_database_uses` | +| `windmill-api-settings/src/lib.rs` | `/settings/external_instance_pg/*`, `/settings/datatable_roles` | + +## What holds it together + +- **One lifecycle lock.** `lock_external_instance_pg_state` serializes everything that changes + which databases exist on the cluster or which entries name them: setup, create, drop, data table + and Ducklake saves, external role DDL, and writes to the setting itself. Anything reading the + configuration to reach the cluster reads it under that lock, so a database is never created on + one cluster and registered while the setting names another. +- **Windmill only touches what it made.** Databases it creates carry a comment, and a drop + requires it. The two managed roles and every data table role carry their own comment, and setup + refuses a `custom_instance_user` without it rather than resetting the password of someone else's + role. +- **Creation needs a successful setup.** `set_up_for` records the `host:port` the last successful + setup converged. Creating a database on a cluster that setup has not succeeded on is refused. +- **Nothing is dropped from under a user.** `managed_database_uses` lists every data table naming + a database, every fork pointing at those, every Ducklake catalog on it, and every fork Ducklake + metadata schema still to be dropped. Fork cleanup exempts exactly the entry it is cleaning up. +- **Fork copies belong to a workspace.** `wm_fork_*` is a name, not an authorization: every + database of a cluster answers to the same `custom_instance_user`. The registry records the + workspace a copy was created for, and a member can only import into or fork onto a copy of their + own workspace. +- **Roles are per cluster.** `datatable_role.cluster` splits the catalog, so the same role name can + exist on both clusters. Role names are unique per cluster, as they are in Postgres. + +## Running one locally + +```bash +docker run -d --name wm-external-pg -e POSTGRES_PASSWORD=external -p 5497:5432 postgres:18 \ + -c wal_level=logical +psql "postgresql://postgres:external@127.0.0.1:5497/postgres" \ + -c "CREATE ROLE wm_admin LOGIN PASSWORD 'adminpw' CREATEDB CREATEROLE REPLICATION" +``` + +A non-superuser admin with `CREATEDB` and `CREATEROLE` is the realistic case: managed Postgres +gives nothing more. `REPLICATION` is only needed for Postgres triggers on external data tables. + +Then, as superadmin (`$T` is a token): + +```bash +api=http://localhost:8000/api +curl -s -X POST $api/settings/global/external_instance_pg -H "Authorization: Bearer $T" \ + -H 'Content-Type: application/json' \ + --data '{"value":{"host":"127.0.0.1","port":5497,"user":"wm_admin","password":"adminpw","sslmode":"disable"}}' +curl -s -X POST $api/settings/external_instance_pg/setup -H "Authorization: Bearer $T" \ + -H 'Content-Type: application/json' --data '{}' # report per step +curl -s -X POST $api/settings/external_instance_pg/databases/dt_demo -H "Authorization: Bearer $T" \ + -H 'Content-Type: application/json' --data '{}' +curl -s -X POST $api/w/admins/workspaces/edit_datatable_config -H "Authorization: Bearer $T" \ + -H 'Content-Type: application/json' \ + --data '{"settings":{"datatables":{"demo":{"database":{"resource_type":"external_instance","resource_path":"dt_demo"}}}}}' +``` + +`sslmode` defaults to `verify-full`; `disable` is for a local container only. With `verify-full` +against a server with a private CA, put the CA in `root_certificate_pem` — `pg_dump`, `psql` and +DuckDB attaches all verify against the system trust store plus that certificate. + +Jobs then reach it as any data table: `ATTACH 'datatable://demo' AS d` from DuckDB, or +`datatable://demo` as the database of a PostgreSQL script, with `-- role ` to connect as a +data table role of that cluster. + +Worth knowing while testing: + +- A worker needs the `postgresql` and `duckdb` tags for those jobs + (`update config set config = jsonb_set(config, '{worker_tags}', …) where name = 'worker__default'`). +- DuckDB jobs load `libwindmill_duckdb_ffi_internal.so` by name, so a binary built into its own + `CARGO_TARGET_DIR` needs that library on `LD_LIBRARY_PATH`. +- Setup holds the lifecycle lock for its whole run, so a settings save during it waits. diff --git a/frontend/src/lib/components/InstanceSetting.svelte b/frontend/src/lib/components/InstanceSetting.svelte index 0da63ffbb0..d6740c6c9b 100644 --- a/frontend/src/lib/components/InstanceSetting.svelte +++ b/frontend/src/lib/components/InstanceSetting.svelte @@ -23,6 +23,8 @@ import RetentionPeriodOverrides from './instanceSettings/RetentionPeriodOverrides.svelte' import SmtpSettings from './instanceSettings/SmtpSettings.svelte' import SecretBackendConfig from './instanceSettings/SecretBackendConfig.svelte' + import ExternalInstancePgSettings from './instanceSettings/ExternalInstancePgSettings.svelte' + import InstancePgSettings from './instanceSettings/InstancePgSettings.svelte' import GhesAppSettings from './instanceSettings/GhesAppSettings.svelte' import WebhookBaseUrlSetting from './instanceSettings/WebhookBaseUrlSetting.svelte' import WsConnectivityTest from './instanceSettings/WsConnectivityTest.svelte' @@ -43,6 +45,7 @@ openSmtpSettings?: () => void oauths?: Record warning?: string + markSettingSaved?: (key: string) => void } let { @@ -52,7 +55,8 @@ loading = true, openSmtpSettings, oauths, - warning + warning, + markSettingSaved }: Props = $props() const dispatch = createEventDispatcher() @@ -782,10 +786,10 @@ />

Comma-separated host/IP patterns the proxy still traces but for which it skips - upstream TLS certificate verification. Use for internal endpoints with - self-signed or otherwise untrusted certificates — unlike NO_PROXY above, these - requests stay traced. Same matching as NO_PROXY (example.com matches - subdomains; .example.com matches subdomains only). + upstream TLS certificate verification. Use for internal endpoints with self-signed + or otherwise untrusted certificates — unlike NO_PROXY above, these requests stay + traced. Same matching as NO_PROXY (example.com matches subdomains; + .example.com matches subdomains only).

@@ -865,6 +869,14 @@ {:else if setting.fieldType == 'secret_backend'} + {:else if setting.fieldType == 'external_instance_pg'} + + {:else if setting.fieldType == 'instance_pg'} + {:else if setting.fieldType == 'github_enterprise_app'} {:else if setting.fieldType == 'webhook_base_url'} diff --git a/frontend/src/lib/components/InstanceSettings.svelte b/frontend/src/lib/components/InstanceSettings.svelte index 0c27a3c7c4..1476b9ad87 100644 --- a/frontend/src/lib/components/InstanceSettings.svelte +++ b/frontend/src/lib/components/InstanceSettings.svelte @@ -60,6 +60,14 @@ let requirePreexistingUserForOauth: boolean = $state(false) let initialValues: Record = $state({}) + + /// A setting its own component writes (it has to persist before acting on the cluster) is + /// already saved, so the baseline must move with it: otherwise Discard restores the value it + /// replaced, and a later bulk save sends that stale one back. + function markSettingSaved(key: string) { + initialValues[key] = + $values[key] === undefined ? undefined : JSON.parse(JSON.stringify($values[key])) + } let baseUrlIsFallback = $state(false) // Per-instance OAuth providers (Snowflake, ServiceNow, …): instance name // keyed by provider, used to build their per-instance connect_config URLs. @@ -730,6 +738,7 @@ secret_backend: ['token', 'client_secret', 'secret_access_key'], object_store_cache_config: ['secret_key', 'serviceAccountKey', 'accessKey'], custom_instance_pg_databases: ['user_pwd'], + external_instance_pg: ['password'], rsa_keys: ['private_key'], github_enterprise_app: ['private_key'] } @@ -1194,6 +1203,7 @@ {loading} {setting} {values} + {markSettingSaved} {version} {oauths} /> @@ -1213,6 +1223,7 @@ {loading} {setting} {values} + {markSettingSaved} {version} {oauths} warning={setting.key === 'base_url' && baseUrlIsFallback @@ -1228,6 +1239,7 @@ {@const licenseKeySetting = settings['Core'].find((s) => s.key === 'license_key')} {#if licenseKeySetting} closeDrawer?.()} {loading} @@ -1253,6 +1265,7 @@ {loading} {setting} {values} + {markSettingSaved} {version} {oauths} /> diff --git a/frontend/src/lib/components/instanceSettings.ts b/frontend/src/lib/components/instanceSettings.ts index 17d5ec9f6e..9701b74e30 100644 --- a/frontend/src/lib/components/instanceSettings.ts +++ b/frontend/src/lib/components/instanceSettings.ts @@ -68,6 +68,8 @@ export interface Setting { | 'otel' | 'otel_tracing_proxy' | 'secret_backend' + | 'external_instance_pg' + | 'instance_pg' | 'github_enterprise_app' | 'webhook_base_url' | 'ws_connectivity' @@ -599,6 +601,25 @@ export const settings: Record = { (Number.isInteger(Number(v)) && Number(v) >= 0 && Number(v) <= 3650) } ], + 'Managed Postgres': [ + { + label: 'External instance', + description: + 'A Postgres cluster Windmill administers for data tables and Ducklake catalogs, instead of its own database. It creates the databases there and manages the roles jobs connect as.', + key: 'external_instance_pg', + fieldType: 'external_instance_pg', + storage: 'setting', + ee_only: '' + }, + { + label: 'Windmill instance', + description: + "Windmill's own database as a data table and Ducklake substrate. On unless turned off here, and never available on cloud.", + key: 'instance_pg_disabled', + fieldType: 'instance_pg', + storage: 'setting' + } + ], 'Object Storage': [ { label: 'Instance object storage', @@ -1309,6 +1330,14 @@ export const instanceSettingsNavigationGroups = [ aiId: 'instance-settings-object-storage', aiDescription: 'Instance object storage settings', isEE: true + }, + { + id: 'managed_postgres', + label: 'Managed Postgres', + aiId: 'instance-settings-managed-postgres', + aiDescription: + 'Postgres substrates Windmill administers for data tables and Ducklake catalogs: its own database and an external cluster', + isEE: true } ] }, @@ -1428,6 +1457,7 @@ export const tabToCategoryMap: Record = { telemetry: 'Telemetry', secret_storage: 'Secret Storage', object_storage: 'Object Storage', + managed_postgres: 'Managed Postgres', jobs: 'Jobs', private_hub: 'Private Hub', github_enterprise_app: 'GitHub App', @@ -1464,6 +1494,7 @@ export const categoryToTabMap: Record = { Telemetry: 'telemetry', 'Secret Storage': 'secret_storage', 'Object Storage': 'object_storage', + 'Managed Postgres': 'managed_postgres', Jobs: 'jobs', 'Private Hub': 'private_hub', 'GitHub App': 'github_enterprise_app', diff --git a/frontend/src/lib/components/instanceSettings/ExternalInstancePgSettings.svelte b/frontend/src/lib/components/instanceSettings/ExternalInstancePgSettings.svelte new file mode 100644 index 0000000000..97b5494f44 --- /dev/null +++ b/frontend/src/lib/components/instanceSettings/ExternalInstancePgSettings.svelte @@ -0,0 +1,471 @@ + + +
+ {#if !$enterpriseLicense} + + {/if} + + + Windmill creates the databases for data tables and Ducklake catalogs on this cluster, and + manages the roles they connect as. The admin login below needs CREATEDB + and CREATEROLE, and is never handed to a job. + + +
+
+ + + +
+ +
+
+ Admin password + +
+ +
+ + SSL mode + + {#snippet text()} + Anything below verify-ca sends the managed roles' passwords to whichever server + answers. Windmill trusts the system roots plus the certificate below. + {/snippet} + + + + +
+ +
+ + + + {#if status} +
+ {#if status.configured} + + Configured · {status.database_count} database{status.database_count === 1 ? '' : 's'} + {:else} + + Not configured yet + {/if} +
+ {/if} +
+ + {#if loadError} + {loadError} + {/if} + + {#if status?.last_setup} + {@const report = status.last_setup} +
+
+ Last setup + {new Date(report.finished_at).toLocaleString()} +
+
+ {#each report.steps as step} +
+
+ {#if step.status === 'ok'} + + {:else if step.status === 'warning'} + + {:else} + + {/if} +
+
+ {step.name} + {step.message} +
+
+ {/each} +
+
+ {/if} + + {#if status?.configured} +
+ Databases on the cluster + + Only databases created here can back a data table or a Ducklake catalog, and one in use + cannot be dropped. + + + + + Name + Tag + Used by + + + + + {#each databaseEntries as [name, db]} + + {name} + {db.tag ?? '-'} + + {db.used_by_workspaces?.length ? db.used_by_workspaces.join(', ') : '-'} + + +
+
+ + + {:else} + + + No database yet. Create one below, then pick it in a workspace's data table or + Ducklake settings. + + + {/each} + + +
+ +
+ dataTable.database.resource_type, (resource_type) => { @@ -451,12 +537,20 @@ } } } + transformInputSelectedText={shortManagedInstanceLabel} id="database-type-select" - class="w-28" + class="w-36" />
- {#if dataTable.database.resource_type !== 'instance'} + {#if dataTable.database.resource_type === 'external_instance'} + + {:else if dataTable.database.resource_type !== 'instance'} diff --git a/frontend/src/lib/components/workspaceSettings/DucklakeSettings.svelte b/frontend/src/lib/components/workspaceSettings/DucklakeSettings.svelte index a0d29af434..f6929e0779 100644 --- a/frontend/src/lib/components/workspaceSettings/DucklakeSettings.svelte +++ b/frontend/src/lib/components/workspaceSettings/DucklakeSettings.svelte @@ -13,7 +13,7 @@ ducklakes: { name: string catalog: { - resource_type: 'postgresql' | 'mysql' | 'instance' + resource_type: 'postgresql' | 'mysql' | 'instance' | 'external_instance' resource_path?: string // Name of the database when resource_type is instance } storage: { @@ -74,7 +74,6 @@ GitFork, Plus, SettingsIcon, - Wrench } from 'lucide-svelte' import Button from '../common/button/Button.svelte' @@ -91,7 +90,7 @@ import { ScheduleService, SettingService, WorkspaceService } from '$lib/gen' import type { GetSettingsResponse } from '$lib/gen' - import { enterpriseLicense, userWorkspaces, workspaceStore } from '$lib/stores' + import { enterpriseLicense, superadmin, userWorkspaces, workspaceStore } from '$lib/stores' import { base } from '$app/paths' import Toggle from '../Toggle.svelte' import { sendUserToast } from '$lib/toast' @@ -105,9 +104,16 @@ import Popover from '../meltComponents/Popover.svelte' import TextInput from '../text_input/TextInput.svelte' import { slide } from 'svelte/transition' - import { isCustomInstanceDbEnabled, getUnusedInstanceDbName } from './utils.svelte' + import { + isCustomInstanceDbEnabled, + managedInstanceLabels, + shortManagedInstanceLabel, + getUnusedInstanceDbName + } from './utils.svelte' import { resource } from 'runed' + import { isCloudHosted } from '$lib/cloud' import CustomInstanceDbSelect from './CustomInstanceDbSelect.svelte' + import ExternalInstanceDbSelect from './ExternalInstanceDbSelect.svelte' import Label from '../Label.svelte' type Props = { @@ -140,8 +146,14 @@ ducklakeSettings.ducklakes.push({ name, catalog: { - resource_type: $isCustomInstanceDbEnabled ? 'instance' : 'postgresql', - resource_path: $isCustomInstanceDbEnabled ? defaultInstanceDbName() : undefined + // A new entry starts on a kind this instance actually offers, so it is never born + // on one the save would refuse. + resource_type: instanceAvailable + ? 'instance' + : externalInstanceConfigured && $superadmin + ? 'external_instance' + : 'postgresql', + resource_path: instanceAvailable ? defaultInstanceDbName() : undefined }, storage: { storage: undefined, @@ -170,6 +182,77 @@ const customInstanceDbs = resource([() => $workspaceStore], SettingService.listCustomInstanceDbs) + // Superadmin-only endpoints, and the kind is theirs to pick: a workspace admin never loads + // them and sees the option disabled instead of an empty picker. + const externalInstanceStatus = resource([() => $superadmin], ([isSuperadmin]) => + isSuperadmin ? SettingService.getExternalInstancePgStatus() : Promise.resolve(undefined) + ) + const externalInstanceDbs = resource([() => $superadmin], ([isSuperadmin]) => + isSuperadmin ? SettingService.listExternalInstancePgDatabases() : Promise.resolve({}) + ) + let externalInstanceConfigured = $derived(externalInstanceStatus.current?.configured === true) + // Superadmin-only like the ones above, and absent means on. + const instancePgDisabled = resource([() => $superadmin], ([isSuperadmin]) => + isSuperadmin + ? SettingService.getGlobal({ key: 'instance_pg_disabled' }).catch(() => undefined) + : Promise.resolve(undefined) + ) + // Both substrates answer only to a superadmin, so nobody else can be told whether one is on + // offer: they see a managed kind only where an entry already sits on it. + let instancePossible = $derived( + !!$superadmin && !isCloudHosted() && !instancePgDisabled.current + ) + let instanceAvailable = $derived(instancePossible) + + // A kind already saved stays listed whatever the instance offers now. + // Qualified per form, not per row: see DataTableSettings. + let anyInstanceLake = $derived( + ducklakeSettings.ducklakes.some((d) => d.catalog.resource_type === 'instance') + ) + let anyExternalLake = $derived( + ducklakeSettings.ducklakes.some((d) => d.catalog.resource_type === 'external_instance') + ) + function catalogItems(current: string | undefined) { + const showInstance = instancePossible || current === 'instance' + const showExternal = + (externalInstanceConfigured && !!$superadmin) || current === 'external_instance' + const labels = managedInstanceLabels( + instancePossible || anyInstanceLake, + (externalInstanceConfigured && !!$superadmin) || anyExternalLake + ) + const items: { value: string; label: string; disabled?: boolean; subtitle?: string }[] = [ + { value: 'postgresql', label: 'Postgres Resource' }, + { value: 'mysql', label: 'MySQL Resource' } + ] + if (showInstance) { + items.push({ + value: 'instance', + label: labels.instance, + disabled: !instanceAvailable, + subtitle: instanceAvailable + ? undefined + : !$superadmin + ? 'Superadmin only' + : isCloudHosted() + ? 'Not available on cloud' + : "Windmill's database is disabled" + }) + } + if (showExternal) { + items.push({ + value: 'external_instance', + label: labels.external, + disabled: !externalInstanceConfigured || !$superadmin, + subtitle: !$superadmin + ? 'Superadmin only' + : externalInstanceConfigured + ? undefined + : 'No external cluster configured' + }) + } + return items + } + async function onSave() { try { if ( @@ -245,15 +328,13 @@ secondaryStorageNames.refresh() }) - let tableHeadNames = ['Name', 'Catalog', 'Workspace storage', 'Maintenance', '', ''] as const + let tableHeadNames = ['Name', 'Catalog', 'Workspace storage', '', ''] as const let tableHeadTooltips: Partial> = { Name: "Ducklakes are referenced in DuckDB scripts with the ATTACH 'ducklake://name' AS dl; syntax", Catalog: 'Ducklake needs an SQL database to store metadata about the data', 'Workspace storage': - 'Where the data is actually stored, in parquet format. You need to configure a workspace storage first', - Maintenance: - 'Scheduled snapshot expiry, small-file compaction and orphaned-file cleanup, run as jobs on a managed per-lake schedule (EE)' + 'Where the data is actually stored, in parquet format. You need to configure a workspace storage first' } let confirmationModal = createAsyncConfirmationModal() @@ -283,10 +364,9 @@ This workspace is a fork, and these settings are its own copy. Lakes marked isolated read the parent's tables through defer views and write to a fork-scoped namespace that is cleaned up when the fork is deleted. Lakes marked - shared with parent read and write the parent's physical - lake directly — editing their catalog or storage here repoints the shared lake for this - fork's jobs. The choice is made per lake when the fork is created and cannot be changed - here. + shared with parent read and write the parent's physical lake + directly — editing their catalog or storage here repoints the shared lake for this fork's jobs. + The choice is made per lake when the fork is created and cannot be changed here.
{/if} @@ -359,8 +439,8 @@ isolated - Writes go to a fork-scoped namespace; reads of tables not yet materialized in - this fork defer to the parent. Deleting the fork cleans the namespace up. + Writes go to a fork-scoped namespace; reads of tables not yet materialized in this + fork defer to the parent. Deleting the fork cleans the namespace up. {/if}
@@ -373,17 +453,13 @@ Use Windmill's PostgreSQL instance as a catalog + {:else if ducklake.catalog.resource_type === 'external_instance'} + + Use a database Windmill manages on the external PostgreSQL cluster as a catalog + {/if} (value = i)} + placeholder="Search or create..." + showPlaceholderOnOpen + {items} + id="external-instance-db-select" + disabled={!$superadmin} + noItemsMsg="Start typing to create a new database" + > + {#snippet endSnippet({ item })} + {@render sharedWorkspacesWarning(item.value)} + {/snippet} + + {#if value} +
+ {#if unknownName} + + {:else if $superadmin} + {@render sharedWorkspacesWarning(value)} +
+ {/if} +
+ {/if} +
+ +{#snippet sharedWorkspacesWarning(dbname: string)} + {@const others = otherWorkspaces(dbname)} + {#if others.length > 0} + + + {#snippet text()} + This database is also used by workspace{others.length > 1 ? 's' : ''} + {others.join(', ')}. Any data written here will be shared + with {others.length > 1 ? 'them' : 'it'}. + {/snippet} + + {/if} +{/snippet} diff --git a/frontend/src/lib/components/workspaceSettings/InstanceRolesButton.svelte b/frontend/src/lib/components/workspaceSettings/InstanceRolesButton.svelte index 9932495fce..6072cdad3f 100644 --- a/frontend/src/lib/components/workspaceSettings/InstanceRolesButton.svelte +++ b/frontend/src/lib/components/workspaceSettings/InstanceRolesButton.svelte @@ -2,14 +2,18 @@ import { Badge, Button, Drawer, DrawerContent } from '../common' import { Users } from 'lucide-svelte' import DataTableRolesSection from './DataTableRolesSection.svelte' + import type { DatatableRoleCluster } from '$lib/gen' let { hideTrigger = false, + cluster, onChanged, unavailable }: { /** Mount the drawer without its button, for a caller that opens it with `open()`. */ hideTrigger?: boolean + /** Whose catalog to manage. A role is a login on one cluster. */ + cluster?: DatatableRoleCluster /** Called after every change to the instance roles. */ onChanged?: () => void /** Why the roles cannot be managed here: the button stays, disabled with this reason, so the @@ -49,13 +53,13 @@ drawer?.closeDrawer()} - tooltip="A data table role is a real Postgres login on this instance, shared by every instance database. A job that names one connects as it, and Postgres decides what it may touch. Which people may use a role on a given data table, and what it may do there, is set per data table, in its roles drawer." + tooltip="A data table role is a real Postgres login on the cluster it belongs to, shared by every database Windmill manages there. A job that names one connects as it, and Postgres decides what it may touch. Which people may use a role on a given data table, and what it may do there, is set per data table, in its roles drawer." > {#snippet titleExtra()} Beta {/snippet} {#key openCount} - + {/key} diff --git a/frontend/src/lib/components/workspaceSettings/addDataTableModel.test.ts b/frontend/src/lib/components/workspaceSettings/addDataTableModel.test.ts index cb42d73075..9461a7b04b 100644 --- a/frontend/src/lib/components/workspaceSettings/addDataTableModel.test.ts +++ b/frontend/src/lib/components/workspaceSettings/addDataTableModel.test.ts @@ -23,6 +23,7 @@ const getSettingsMock = vi.fn() const editDataTableConfigMock = vi.fn() const testDataTableConnectionMock = vi.fn() const setupCustomInstanceDbMock = vi.fn() +const createExternalInstanceDbMock = vi.fn() vi.mock('$lib/gen', () => ({ VariableService: { existsVariable: (...a: any[]) => existsVariableMock(...a), @@ -36,7 +37,10 @@ vi.mock('$lib/gen', () => ({ createResource: vi.fn(), updateResource: vi.fn() }, - SettingService: { setupCustomInstanceDb: (...a: any[]) => setupCustomInstanceDbMock(...a) }, + SettingService: { + setupCustomInstanceDb: (...a: any[]) => setupCustomInstanceDbMock(...a), + createExternalInstancePgDatabase: (...a: any[]) => createExternalInstanceDbMock(...a) + }, WorkspaceService: { getSettings: (...a: any[]) => getSettingsMock(...a), editDataTableConfig: (...a: any[]) => editDataTableConfigMock(...a), @@ -239,6 +243,48 @@ describe('runSetup rolling the instance row back', () => { }) }) +// A database on the external cluster belongs to whoever created it, and the endpoint refuses a +// name it already holds. Skipping the create on any registered name is how a run ends up +// attached to somebody else's data without the sharing warning the existing-database branch +// shows -- so only the databases this run made may make a retry skip it. +describe('runSetup creating a database on the external cluster', () => { + function creatingExternal(): WizardState { + const state = newWizardState({ name: 'main', projectName: 'x', folder: 'f/team' }) + state.provider = 'external_instance' + state.external = { mode: 'create', dbName: 'dt_new' } + return state + } + const externalDeps = (createdExternalDbs: string[] = []) => + ({ + workspace: 'w', + onProgress: () => {}, + claims: noClaims, + username: 'alice', + createdProjects: [], + createdExternalDbs + }) as any + + beforeEach(() => { + vi.clearAllMocks() + getSettingsMock.mockResolvedValue({ datatable: { datatables: {} } }) + editDataTableConfigMock.mockResolvedValue(undefined) + testDataTableConnectionMock.mockResolvedValue({ can_create_table: true }) + }) + + it('creates the database on a first attempt', async () => { + const result = await runSetup(creatingExternal(), externalDeps()) + expect(result.ok).toBe(true) + expect(createExternalInstanceDbMock).toHaveBeenCalledTimes(1) + expect(result.createdExternalDbs).toContain('dt_new') + }) + + it('skips the create only for a database it made itself', async () => { + const result = await runSetup(creatingExternal(), externalDeps(['dt_new'])) + expect(result.ok).toBe(true) + expect(createExternalInstanceDbMock).not.toHaveBeenCalled() + }) +}) + // The fields are the connection; a connection string is a way of writing one down. Reading the // resource back out of the string is what let a URI grammar gap change what got saved. describe('newResourceParts', () => { diff --git a/frontend/src/lib/components/workspaceSettings/addDataTableModel.ts b/frontend/src/lib/components/workspaceSettings/addDataTableModel.ts index 2625786991..073d496d0c 100644 --- a/frontend/src/lib/components/workspaceSettings/addDataTableModel.ts +++ b/frontend/src/lib/components/workspaceSettings/addDataTableModel.ts @@ -42,7 +42,7 @@ import { type SupabaseProject } from './supabaseProvisioning' -export type Provider = 'supabase' | 'instance' | 'resource' +export type Provider = 'supabase' | 'instance' | 'external_instance' | 'resource' export type WizardState = { step: 1 | 2 | 3 @@ -61,6 +61,8 @@ export type WizardState = { connectionMode: SupabaseConnectionMode } instance: { mode: 'existing' | 'create'; dbName: string | undefined } + /** Same shape as `instance`, on the Postgres cluster Windmill administers elsewhere. */ + external: { mode: 'existing' | 'create'; dbName: string | undefined } /** * One list: the workspace's Postgres resources, plus the one about to exist. A * connection string is not an alternative to a resource, it is how one is written -- @@ -107,6 +109,7 @@ export function newWizardState(defaults: { connectionMode: 'session' }, instance: { mode: 'create', dbName: undefined }, + external: { mode: 'create', dbName: undefined }, own: { resourcePath: undefined, creating: false, @@ -137,6 +140,7 @@ export function intentComplete(state: WizardState): boolean { : !!state.supabase.project && !!state.supabase.password } if (state.provider === 'instance') return !!state.instance.dbName?.trim() + if (state.provider === 'external_instance') return !!state.external.dbName?.trim() if (!state.own.creating) return !!state.own.resourcePath // Text that will not parse leaves the fields on their last good values, which is what makes // it correctable -- but the connection on screen is then not the one they describe, and @@ -307,6 +311,7 @@ export type RunStepKey = | 'create_project' | 'wait_healthy' | 'save_credentials' + | 'create_external' | 'setup_instance' | 'check' @@ -331,6 +336,13 @@ export function plan(state: WizardState): { key: RunStepKey; title: string }[] { key: 'setup_instance', title: `Setting up ${state.instance.dbName} in the Windmill database` }) + } else if (state.provider === 'external_instance') { + if (state.external.mode === 'create') { + steps.push({ + key: 'create_external', + title: `Creating ${state.external.dbName} on the external cluster` + }) + } } else if (state.own.creating) { steps.push({ key: 'save_credentials', title: `Saving the connection to ${path}` }) } @@ -362,6 +374,16 @@ export type RunDeps = { * is really there, since the name is also recorded when a create could not be confirmed. */ createdProjects: CreatedProject[] + /** + * The databases earlier attempts in this session created on the external cluster. Only these + * make a retry skip the create: any other registered name is somebody else's database, and + * attaching to it silently would share their data without the warning the existing-database + * branch shows. + */ + createdExternalDbs?: string[] + /** Called once a database lands on the external cluster, so the caller's registry — which + * names the next run's default and validates it — is not a page-load-old view. */ + onExternalDbsChanged?: () => Promise /** * What earlier attempts in this session wrote, and this one may therefore write over again. * The pre-flight checks the names are free, but the Supabase branch then spends minutes @@ -391,6 +413,8 @@ export type RunResult = { * later attempt may write over it. */ createdProjects: CreatedProject[] + /** The external-cluster databases this session created, kept for the same reason. */ + createdExternalDbs: string[] /** What this run holds now, for the next attempt to be given back. */ claims: Claims } @@ -410,6 +434,17 @@ async function exists(kind: 'variable' | 'resource', workspace: string, path: st : ResourceService.existsResource({ workspace, path }) } +/** + * What identifies the database a row points at. The kind belongs in it: `instance` and + * `external_instance` are different databases that may carry the same name, so a row repointed + * from one to the other while this run probes must not read as the row this run wrote. + */ +function rowMark(database: { resource_type?: string; resource_path?: string } | undefined) { + return database?.resource_path === undefined + ? undefined + : `${database.resource_type ?? ''}:${database.resource_path}` +} + /** * Adds the data table to the workspace config, once everything it points at exists. * `edit_datatable_config` replaces the whole map, so the rest is read back and sent with @@ -419,16 +454,16 @@ async function writeRow( deps: RunDeps, claims: Claims, name: string, - database: { resource_type: 'postgresql' | 'instance'; resource_path: string } + database: { + resource_type: 'postgresql' | 'instance' | 'external_instance' + resource_path: string + } ): Promise { const settings = await WorkspaceService.getSettings({ workspace: deps.workspace }) const datatables: Record = { ...(settings.datatable?.datatables ?? {}) } // Free when the pre-flight looked, taken by the time we write: repointing it here would // silently hand another admin's data table a database they never chose. - if ( - datatables[name] && - !stillOurs(claims, 'row', name, datatables[name]?.database?.resource_path) - ) { + if (datatables[name] && !stillOurs(claims, 'row', name, rowMark(datatables[name]?.database))) { throw new Error( `A data table called ${name} was created while this setup was running. Choose another name and try again.` ) @@ -438,7 +473,7 @@ async function writeRow( workspace: deps.workspace, requestBody: { settings: { datatables }, renames: [], deleted_datatables: [] } }) - return claim(claims, 'row', name, database.resource_path) + return claim(claims, 'row', name, rowMark(database)!) } /** @@ -455,7 +490,7 @@ async function removeRow(deps: RunDeps, claims: Claims, name: string): Promise { if (!createdProjects.some((p) => p.path === at)) @@ -595,7 +631,8 @@ export async function runSetup(state: WizardState, deps: RunDeps): Promise p.path === path) const instanceName = state.instance.dbName?.trim() ?? '' + const externalName = state.external.dbName?.trim() ?? '' let project = state.supabase.project let resourcePath = @@ -740,6 +778,18 @@ export async function runSetup(state: WizardState, deps: RunDeps): Promise