From 34b5a02b7fab833c5f736fb7b9b0e971a2ad6840 Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Fri, 18 Sep 2026 14:29:25 +0200 Subject: [PATCH] feat(datatables): clone a data table under roles with its owners and grants Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UbrtwiYNfayrmqouBJHwGV --- ...522398d2a3ae99d0568b785e63749d6f3225a.json | 15 + ...df6bc09f19cf53b141fac5aefa981463d39fa.json | 29 + ...052b42aaf5c87e5be6333b2df67a81990c8a6.json | 16 + backend/ee-repo-ref.txt | 2 +- .../tests/datatable_roles.rs | 229 ++++++- .../src/datatable_acl.rs | 96 ++- .../src/datatable_clone.rs | 560 ++++++++++++++++ .../src/datatable_replay_oss.rs | 48 ++ backend/windmill-api-workspaces/src/lib.rs | 4 + .../windmill-api-workspaces/src/workspaces.rs | 619 ++++++++++++++---- .../src/workspaces_extra.rs | 30 +- backend/windmill-api/openapi.yaml | 35 +- backend/windmill-common/src/lib.rs | 136 +++- backend/windmill-common/src/workspaces.rs | 142 +++- cli/src/commands/workspace/fork.ts | 24 +- .../lib/components/InstanceSettings.svelte | 14 +- .../datatableAcl/PgAclEditor.svelte | 7 +- .../CreateWorkspaceInner.svelte | 22 +- .../DataTablePermissionsButton.svelte | 9 +- .../ForkDatatableSection.svelte | 152 +---- 20 files changed, 1817 insertions(+), 372 deletions(-) create mode 100644 backend/.sqlx/query-d0548225d92e6e7d0eb9a0227a6522398d2a3ae99d0568b785e63749d6f3225a.json create mode 100644 backend/.sqlx/query-e524e89440004953cc0b1cb57b8df6bc09f19cf53b141fac5aefa981463d39fa.json create mode 100644 backend/.sqlx/query-f7ece5036ad92e485b5e15a70e5052b42aaf5c87e5be6333b2df67a81990c8a6.json create mode 100644 backend/windmill-api-workspaces/src/datatable_clone.rs create mode 100644 backend/windmill-api-workspaces/src/datatable_replay_oss.rs diff --git a/backend/.sqlx/query-d0548225d92e6e7d0eb9a0227a6522398d2a3ae99d0568b785e63749d6f3225a.json b/backend/.sqlx/query-d0548225d92e6e7d0eb9a0227a6522398d2a3ae99d0568b785e63749d6f3225a.json new file mode 100644 index 0000000000..2ff2722f77 --- /dev/null +++ b/backend/.sqlx/query-d0548225d92e6e7d0eb9a0227a6522398d2a3ae99d0568b785e63749d6f3225a.json @@ -0,0 +1,15 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE workspace_settings ws\n SET datatable = (\n SELECT jsonb_set(ws.datatable, '{datatables}', jsonb_object_agg(\n dt.key,\n (SELECT CASE WHEN r.v->'governed_by'->>'workspace_id' = $2\n THEN jsonb_set(r.v, '{governed_by,workspace_id}', to_jsonb($1::text))\n ELSE r.v END\n FROM (SELECT CASE WHEN dt.value->'reference'->>'workspace_id' = $2\n THEN jsonb_set(dt.value, '{reference,workspace_id}', to_jsonb($1::text))\n ELSE dt.value END AS v) r)\n ))\n FROM jsonb_each(ws.datatable->'datatables') dt\n )\n WHERE jsonb_typeof(ws.datatable->'datatables') = 'object'\n AND (ws.datatable::text LIKE '%\"reference\"%'\n OR ws.datatable::text LIKE '%\"governed_by\"%')", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "d0548225d92e6e7d0eb9a0227a6522398d2a3ae99d0568b785e63749d6f3225a" +} diff --git a/backend/.sqlx/query-e524e89440004953cc0b1cb57b8df6bc09f19cf53b141fac5aefa981463d39fa.json b/backend/.sqlx/query-e524e89440004953cc0b1cb57b8df6bc09f19cf53b141fac5aefa981463d39fa.json new file mode 100644 index 0000000000..af8d0eb1ab --- /dev/null +++ b/backend/.sqlx/query-e524e89440004953cc0b1cb57b8df6bc09f19cf53b141fac5aefa981463d39fa.json @@ -0,0 +1,29 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT ws.workspace_id AS \"workspace_id!\", dt.key AS \"datatable!\"\n FROM workspace_settings ws\n CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt\n WHERE EXISTS (\n SELECT 1 FROM (VALUES ('reference'), ('governed_by')) k(link)\n WHERE dt.value->k.link->>'workspace_id' = $1\n AND dt.value->k.link->>'datatable' = $2\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": "e524e89440004953cc0b1cb57b8df6bc09f19cf53b141fac5aefa981463d39fa" +} diff --git a/backend/.sqlx/query-f7ece5036ad92e485b5e15a70e5052b42aaf5c87e5be6333b2df67a81990c8a6.json b/backend/.sqlx/query-f7ece5036ad92e485b5e15a70e5052b42aaf5c87e5be6333b2df67a81990c8a6.json new file mode 100644 index 0000000000..1be51feb85 --- /dev/null +++ b/backend/.sqlx/query-f7ece5036ad92e485b5e15a70e5052b42aaf5c87e5be6333b2df67a81990c8a6.json @@ -0,0 +1,16 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE workspace_settings ws\n SET datatable = (\n SELECT jsonb_set(ws.datatable, '{datatables}', jsonb_object_agg(\n dt.key,\n (SELECT CASE WHEN r.v->'governed_by'->>'workspace_id' = $1\n AND r.v->'governed_by'->>'datatable' = $2\n THEN jsonb_set(r.v, '{governed_by,datatable}', to_jsonb($3::text))\n ELSE r.v END\n FROM (SELECT CASE WHEN dt.value->'reference'->>'workspace_id' = $1\n AND dt.value->'reference'->>'datatable' = $2\n THEN jsonb_set(dt.value, '{reference,datatable}', to_jsonb($3::text))\n ELSE dt.value END AS v) r)\n ))\n FROM jsonb_each(ws.datatable->'datatables') dt\n )\n WHERE EXISTS (\n SELECT 1 FROM jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) d,\n LATERAL (VALUES ('reference'), ('governed_by')) k(link)\n WHERE d.value->k.link->>'workspace_id' = $1\n AND d.value->k.link->>'datatable' = $2\n )", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "f7ece5036ad92e485b5e15a70e5052b42aaf5c87e5be6333b2df67a81990c8a6" +} diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index cc50737a1d..7419e4e5b6 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -7e338e4dabf91689bfd7fb0333c6534040b17b59 +6992e02506c163117f28bdd67aec9579e1353665 diff --git a/backend/windmill-api-integration-tests/tests/datatable_roles.rs b/backend/windmill-api-integration-tests/tests/datatable_roles.rs index 57b45088bd..224e5100f3 100644 --- a/backend/windmill-api-integration-tests/tests/datatable_roles.rs +++ b/backend/windmill-api-integration-tests/tests/datatable_roles.rs @@ -627,21 +627,234 @@ async fn a_rename_has_to_match_the_save_it_claims_to_describe( Ok(()) } +async fn database_exists(db: &Pool, name: &str) -> anyhow::Result { + Ok( + sqlx::query_scalar("SELECT EXISTS (SELECT 1 FROM pg_database WHERE datname = $1)") + .bind(name) + .fetch_one(db) + .await?, + ) +} + +async fn workspace_exists(db: &Pool, id: &str) -> anyhow::Result { + Ok( + sqlx::query_scalar("SELECT EXISTS (SELECT 1 FROM workspace WHERE id = $1)") + .bind(id) + .fetch_one(db) + .await?, + ) +} + #[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))] -async fn a_data_table_under_roles_is_not_copied_into_a_fork( +async fn a_data_table_under_roles_is_cloned_only_with_its_grants( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + // The database a fork's copy of `main` goes into is named after the fork. Every request below + // is refused before it is created, so the cluster-wide name never collides with a sibling run. + let target = "wm_fork_copy__main"; + let fork = |token: &str, forked: Value| { + authed( + client().post(format!( + "http://localhost:{port}/api/w/test-workspace/workspaces/create_fork" + )), + token, + ) + .json(&json!({"id": "wm-fork-copy", "name": "copy", "forked_datatables": [forked]})) + }; + + // Rows are a workspace admin's to copy, as they were before roles. + let resp = fork( + "SECRET_TOKEN_2", + json!({"name": "main", "new_dbname": target, "fork_behavior": "schema_and_data"}), + ) + .send() + .await?; + assert_eq!(resp.status(), 403, "{}", resp.text().await?); + assert!(!database_exists(&db, target).await?); + + // Even the schema alone lists every table, which a member covered by no role cannot read in + // the parent. + let resp = fork( + "SECRET_TOKEN_3", + json!({"name": "main", "new_dbname": target, "fork_behavior": "schema_only"}), + ) + .send() + .await?; + assert_eq!(resp.status(), 401, "{}", resp.text().await?); + assert!(!database_exists(&db, target).await?); + + // Refused in the first phase too, before a git branch is created for a fork that cannot be. + let resp = authed( + client().post(format!( + "http://localhost:{port}/api/w/test-workspace/workspaces/create_workspace_fork_branch" + )), + "SECRET_TOKEN_3", + ) + .json( + &json!({"id": "wm-fork-copy", "name": "copy", "forked_datatables": [ + {"name": "main", "new_dbname": target, "fork_behavior": "schema_only"} + ]}), + ) + .send() + .await?; + assert_eq!(resp.status(), 401, "{}", resp.text().await?); + + // A database the request did not create — whatever the data table, under roles or not — may be + // a copy left behind by a deleted fork, which the entry would reach without its governance. + let resp = fork( + "SECRET_TOKEN", + json!({"name": "other", "new_dbname": "wm_fork_copy__other"}), + ) + .send() + .await?; + assert_eq!(resp.status(), 400); + assert!( + resp.text().await?.contains("fork_behavior"), + "a database the fork did not create was taken" + ); + assert!(!workspace_exists(&db, "wm-fork-copy").await?); + + // The copy an admin asks for goes ahead in an edition that replays grants, and is refused + // before any database exists in one that does not. The fixture's database does not exist, so + // the dump fails either way — and leaves neither a database nor a fork behind. + let resp = fork( + "SECRET_TOKEN", + json!({"name": "main", "new_dbname": target, "fork_behavior": "schema_only"}), + ) + .send() + .await?; + assert_ne!(resp.status(), 200); + let body = resp.text().await?; + #[cfg(all(feature = "private", feature = "enterprise"))] + assert!(body.contains("pg_dump"), "{body}"); + #[cfg(not(all(feature = "private", feature = "enterprise")))] + assert!(body.contains("Enterprise Edition"), "{body}"); + assert!(!database_exists(&db, target).await?); + assert!(!workspace_exists(&db, "wm-fork-copy").await?); + Ok(()) +} + +#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))] +async fn a_clone_takes_its_roles_from_the_data_table_it_was_cloned_from( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + sqlx::query( + r#"UPDATE workspace_settings SET datatable = jsonb_set(datatable, '{datatables,copy}', '{ + "database": {"resource_type": "instance", "resource_path": "wm_fork_dt__copy"}, + "governed_by": {"workspace_id": "test-workspace", "datatable": "main"}, + "forked_from": {} + }'::jsonb) WHERE workspace_id = 'wm-fork-dt'"#, + ) + .execute(&db) + .await?; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let fork = format!("http://localhost:{port}/api/w/wm-fork-dt/workspaces"); + + // `test-user-2` administers the fork, and is only a tenant of `analytics` in the parent. + let resp = authed( + client().get(format!("{fork}/datatable_usable_roles/copy")), + "SECRET_TOKEN_2", + ) + .send() + .await?; + let body: Value = resp.json().await?; + assert_eq!(body["roles"], json!(["analytics"]), "{body}"); + + let resp = authed( + client().get(format!("{fork}/datatable_permissions/copy")), + "SECRET_TOKEN_2", + ) + .send() + .await?; + let body: Value = resp.json().await?; + assert_eq!(body["editable"], false, "{body}"); + assert_eq!(body["clone_of"]["workspace_id"], "test-workspace", "{body}"); + + // Nobody changes a clone's roles, a superadmin included: they are the source's. + for token in ["SECRET_TOKEN_2", "SECRET_TOKEN"] { + let resp = authed( + client().post(format!("{fork}/datatable_permissions/copy")), + token, + ) + .json(&json!({"permissioned": false})) + .send() + .await?; + assert_eq!(resp.status(), 400, "{token} changed a clone's roles"); + } + + // A settings save cannot clear the link. + let resp = authed( + client().post(format!("{fork}/edit_datatable_config")), + "SECRET_TOKEN_2", + ) + .json(&json!({ + "settings": {"datatables": {"main": {}, "copy": { + "database": {"resource_type": "instance", "resource_path": "wm_fork_dt__copy"} + }}}, + "renames": [], "deleted_datatables": [] + })) + .send() + .await?; + assert_eq!(resp.status(), 200, "{}", resp.text().await?); + let governed_by: Option = sqlx::query_scalar( + "SELECT datatable->'datatables'->'copy'->'governed_by' FROM workspace_settings + WHERE workspace_id = 'wm-fork-dt'", + ) + .fetch_one(&db) + .await?; + assert_eq!(governed_by.unwrap()["datatable"], "main"); + + // Nor move it, a superadmin included: its grants were replayed into that database alone. + let resp = authed( + client().post(format!("{fork}/edit_datatable_config")), + "SECRET_TOKEN", + ) + .json(&json!({ + "settings": {"datatables": {"main": {}, "copy": { + "database": {"resource_type": "instance", "resource_path": "wm_fork_dt__elsewhere"} + }}}, + "renames": [], "deleted_datatables": [] + })) + .send() + .await?; + assert_eq!(resp.status(), 400, "{}", resp.text().await?); + + // Nor reach its database through a second entry without roles, which would connect as `admin`. + let resp = authed( + client().post(format!( + "http://localhost:{port}/api/w/test-workspace/workspaces/edit_datatable_config" + )), + "SECRET_TOKEN", + ) + .json(&json!({ + "settings": {"datatables": { + "main": {"database": {"resource_type": "instance", "resource_path": "dt_main"}}, + "alias": {"database": {"resource_type": "instance", "resource_path": "wm_fork_dt__copy"}} + }}, + "renames": [], "deleted_datatables": [] + })) + .send() + .await?; + let status = resp.status(); + let body = resp.text().await?; + assert_eq!(status, 400, "{body}"); + assert!(body.contains("without carrying those roles"), "{body}"); + Ok(()) +} + +#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))] +async fn a_data_table_under_roles_is_not_copied_without_its_grants( db: Pool, ) -> anyhow::Result<()> { initialize_tracing().await; - // `pg_dump` carries no roles and the restore drops ACLs, so a copy would arrive with the - // parent's tenants and none of the grants behind them: every role but admin denied by - // Postgres in a data table that reads as configured. Refuse the copy rather than ship that, - // and refuse it before any data moves. let server = ApiServer::start(db.clone()).await?; let port = server.addr.port(); - // Both halves of the clone: the database the copy would land in, then the copy itself. The - // first has to refuse too, or a permissioned fork leaves an empty registered database that - // no data table entry names and nothing collects. let resp = authed( client().post(format!( "http://localhost:{port}/api/w/test-workspace/workspaces/create_pg_database" diff --git a/backend/windmill-api-workspaces/src/datatable_acl.rs b/backend/windmill-api-workspaces/src/datatable_acl.rs index 6bd14316fc..04dc3d4e80 100644 --- a/backend/windmill-api-workspaces/src/datatable_acl.rs +++ b/backend/windmill-api-workspaces/src/datatable_acl.rs @@ -245,6 +245,8 @@ pub struct DatatableAclInfo { /// Whether this caller may plan and apply changes: they administer the data table, on an /// edition that has the planner. pub editable: bool, + /// Whether the data table is a clone, whose grants stay as they were copied from its source. + pub clone: bool, /// Whether the server is Postgres 17 or later, which added the `MAINTAIN` table privilege. pub supports_maintain: bool, /// The database the target lives in, which no target carries itself. @@ -335,13 +337,40 @@ async fn connect_as_admin_unchecked( 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 (client, mut connection) = pg.connect(Some(db)).await?; + let (client, notices) = connect_with_notices(db, &pg).await?; + Ok((client, notices, dbname)) +} + +/// A connection to `pg`, and the notices Postgres sends on it — which is where a grant or revoke +/// that changed nothing is reported ([`execute_acl_statements`]). +pub(crate) async fn connect_with_notices( + db: &DB, + pg: &PgDatabase, +) -> Result<(tokio_postgres::Client, mpsc::UnboundedReceiver)> { + let (client, connection) = pg.connect(Some(db)).await?; + Ok(( + client, + drive_with_notices(connection, windmill_common::TokioPgConnection::poll_message), + )) +} + +type PollMessage = + fn( + &mut C, + &mut std::task::Context<'_>, + ) -> std::task::Poll>>; + +/// Drive `connection` in the background with `poll`, forwarding its notices. +pub(crate) fn drive_with_notices( + mut connection: C, + poll: PollMessage, +) -> mpsc::UnboundedReceiver { // Unbounded: the driver must never wait on the receiver, which only drains once the statement // the driver is carrying has completed. let (notices_tx, notices) = mpsc::unbounded_channel(); tokio::spawn(async move { loop { - match std::future::poll_fn(|cx| connection.poll_message(cx)).await { + match std::future::poll_fn(|cx| poll(&mut connection, cx)).await { Some(Ok(AsyncMessage::Notice(notice))) => { let _ = notices_tx.send(notice); } @@ -354,7 +383,41 @@ async fn connect_as_admin_unchecked( } } }); - Ok((client, notices, dbname)) + notices +} + +/// Run `statements` in order on `tx`, failing on the first that errors or that Postgres only warns +/// about. A privilege the connection cannot pass on is a warning to Postgres (`01007` / `01006`), +/// which then carries on having changed nothing; returning drops the transaction, rolling back +/// everything before it. +/// +/// Authorization: none. Runs `statements` as the connection `tx` is on; callers MUST have authorized +/// changing that database's access, and built the statements themselves. +pub(crate) async fn execute_acl_statements( + tx: &tokio_postgres::Transaction<'_>, + notices: &mut mpsc::UnboundedReceiver, + statements: &[String], +) -> Result<()> { + while notices.try_recv().is_ok() {} + for statement in statements { + tx.batch_execute(statement).await.map_err(|e| { + Error::ExecutionErr(format!( + "Failed to run `{statement}`: {}", + pg_error_message(&e) + )) + })?; + while let Ok(notice) = notices.try_recv() { + if *notice.code() == SqlState::WARNING_PRIVILEGE_NOT_GRANTED + || *notice.code() == SqlState::WARNING_PRIVILEGE_NOT_REVOKED + { + return Err(Error::ExecutionErr(format!( + "`{statement}` did not take effect ({}), so nothing was applied", + notice.message() + ))); + } + } + } + Ok(()) } /// An object whose ownership follows the schema's. @@ -399,10 +462,12 @@ macro_rules! schema_owned_objects { AND x.refobjsubid <> 0)))" }; } +#[allow(unused_imports)] +pub(crate) use schema_owned_objects; /// The keyword `ALTER ... OWNER TO` takes for a kind of object, as `pg_identify_object` names the /// kind. A kind missing here is refused rather than skipped, which would leave it behind. -fn owned_keyword(kind: &str) -> Option<&'static str> { +pub(crate) fn owned_keyword(kind: &str) -> Option<&'static str> { Some(match kind { "table" => "TABLE", "view" => "VIEW", @@ -1054,6 +1119,7 @@ async fn get_datatable_acl( owner: role_name_of(&owner), roles, editable, + clone: governing.governor.is_some(), supports_maintain, dbname, grants, @@ -1625,27 +1691,7 @@ async fn apply_datatable_acl( pg_error_message(&e) )) })?; - for statement in &plan.statements { - pg_tx.batch_execute(statement).await.map_err(|e| { - Error::ExecutionErr(format!( - "Failed to run `{statement}`: {}", - pg_error_message(&e) - )) - })?; - // A privilege the connection cannot pass on is only a warning to Postgres, which then - // carries on having changed nothing. Returning drops the transaction, rolling back - // everything before it. - while let Ok(notice) = notices.try_recv() { - if *notice.code() == SqlState::WARNING_PRIVILEGE_NOT_GRANTED - || *notice.code() == SqlState::WARNING_PRIVILEGE_NOT_REVOKED - { - return Err(Error::ExecutionErr(format!( - "`{statement}` did not take effect ({}), so nothing was applied", - notice.message() - ))); - } - } - } + execute_acl_statements(&pg_tx, &mut notices, &plan.statements).await?; // A schema's objects were listed before the transaction opened; one committed since would stay // with its old owner. One created while this transaction is still open can still slip past, as diff --git a/backend/windmill-api-workspaces/src/datatable_clone.rs b/backend/windmill-api-workspaces/src/datatable_clone.rs new file mode 100644 index 0000000000..be728782af --- /dev/null +++ b/backend/windmill-api-workspaces/src/datatable_clone.rs @@ -0,0 +1,560 @@ +/* + * 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. + */ + +//! Copying a data table's database for the fork being created. +//! +//! 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. + +use std::collections::BTreeSet; + +use windmill_api_auth::ApiAuthed; +use windmill_common::datatable_roles::{lock_role_catalog, read_role_catalog_tx}; +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, +}; +use windmill_common::{PgDatabase, DB}; + +use crate::datatable_acl::connect_with_notices; +use crate::datatable_permissions::ensure_reaches_governing_datatable; +use crate::workspaces::{ + create_database_on_server, ensure_datatable_is_clonable, pg_dump_database, pg_import_dump, + DumpFile, PgDumpOptions, +}; + +/// A database this request created and filled for one data table of the fork. +pub(crate) struct MadeCopy { + /// The data table's name, in the parent and in the fork. + pub(crate) name: String, + pub(crate) dbname: String, + pub(crate) behavior: DataTableForkBehavior, + /// The database the source resolved to when it was copied. + pub(crate) source_database: DataTableDatabase, + /// Whether the source's owners and grants were replayed into the copy: it was under roles. + pub(crate) replayed: bool, + /// For a resource-backed copy, the resource and variables its connection was resolved from. + pub(crate) connection: Option, +} + +/// What a resource-backed data table's connection is resolved from: its resource, and every +/// resource and variable that one references, as stored — and, for a secret kept in an external +/// backend, the value that backend holds. The connection is resolved from the snapshot itself +/// ([`ConnectionSnapshot::resolve`]), so it is exactly the one these rows describe. +#[derive(PartialEq)] +pub(crate) struct ConnectionSnapshot { + root: String, + resources: std::collections::BTreeMap>, + /// Stored value and whether it is secret. + variables: std::collections::BTreeMap>, + external_secrets: std::collections::BTreeMap, +} + +/// Read the [`ConnectionSnapshot`] of resource `resource_path` in workspace `w_id`. +/// +/// Authorization: none, and it holds secret values. Callers MUST have authorized using that +/// resource, and never disclose the snapshot or what it resolves to. +pub(crate) async fn connection_snapshot( + db: &DB, + conn: &mut sqlx::PgConnection, + w_id: &str, + resource_path: &str, +) -> Result { + let root = resource_path.trim_start_matches("$res:").to_string(); + let mut snapshot = ConnectionSnapshot { + root: root.clone(), + resources: Default::default(), + variables: Default::default(), + external_secrets: Default::default(), + }; + let mut pending = vec![root]; + while let Some(path) = pending.pop() { + if snapshot.resources.contains_key(&path) { + continue; + } + let value: Option = sqlx::query_scalar( + "SELECT value FROM resource WHERE workspace_id = $1 AND path = $2", + ) + .bind(w_id) + .bind(&path) + .fetch_optional(&mut *conn) + .await? + .flatten(); + let mut strings = vec![]; + collect_strings(value.as_ref(), &mut strings); + for reference in strings { + if let Some(var) = reference.strip_prefix("$var:") { + if snapshot.variables.contains_key(var) { + continue; + } + let row: Option<(String, bool)> = sqlx::query_as( + "SELECT value, is_secret FROM variable WHERE workspace_id = $1 AND path = $2", + ) + .bind(w_id) + .bind(var) + .fetch_optional(&mut *conn) + .await?; + if let Some((stored, true)) = &row { + if windmill_common::secret_backend::is_external_stored_value(stored) { + let secret = windmill_common::secret_backend::get_secret_value( + db, w_id, var, stored, + ) + .await?; + snapshot.external_secrets.insert(var.to_string(), secret); + } + } + snapshot.variables.insert(var.to_string(), row); + } else if let Some(res) = reference.strip_prefix("$res:") { + pending.push(res.to_string()); + } + } + snapshot.resources.insert(path, value); + } + Ok(snapshot) +} + +impl ConnectionSnapshot { + /// The connection these rows resolve to, as the data table's own resolution would: references + /// substituted, secrets decrypted. + pub(crate) async fn resolve(&self, db: &DB, w_id: &str) -> Result { + let root = self.resource(&self.root)?; + self.substitute(db, w_id, root, 0).await + } + + /// Whether the resource itself holds the connection's fields, which the fork repoints by setting + /// its `dbname`, rather than being a reference to another resource. + pub(crate) fn root_holds_connection(&self) -> bool { + self.resource(&self.root).is_ok_and(|v| v.is_object()) + } + + fn resource(&self, path: &str) -> Result<&serde_json::Value> { + self.resources + .get(path) + .and_then(|v| v.as_ref()) + .ok_or_else(|| Error::NotFound(format!("resource {path} does not exist"))) + } + + fn substitute<'a>( + &'a self, + db: &'a DB, + w_id: &'a str, + value: &'a serde_json::Value, + depth: usize, + ) -> std::pin::Pin> + Send + 'a>> + { + Box::pin(async move { + if depth > 32 { + return Err(Error::BadRequest("resource references nest too deeply".to_string())); + } + Ok(match value { + serde_json::Value::Object(map) => { + let mut out = serde_json::Map::new(); + for (key, val) in map { + out.insert(key.clone(), self.substitute(db, w_id, val, depth).await?); + } + serde_json::Value::Object(out) + } + serde_json::Value::Array(items) => { + let mut out = vec![]; + for val in items { + out.push(self.substitute(db, w_id, val, depth).await?); + } + serde_json::Value::Array(out) + } + serde_json::Value::String(s) if s.starts_with("$res:") => { + let path = &s[5..]; + self.substitute(db, w_id, self.resource(path)?, depth + 1).await? + } + serde_json::Value::String(s) if s.starts_with("$var:") => { + let path = &s[5..]; + let (stored, secret) = self + .variables + .get(path) + .and_then(|v| v.as_ref()) + .ok_or_else(|| Error::NotFound(format!("variable {path} does not exist")))?; + serde_json::Value::String(match (secret, self.external_secrets.get(path)) { + (false, _) => stored.clone(), + (true, Some(external)) => external.clone(), + (true, None) => windmill_common::variables::decrypt( + &windmill_common::variables::build_crypt(db, w_id).await?, + stored.clone(), + ) + .map_err(|e| { + Error::internal_err(format!("Error decrypting variable {s}: {e}")) + })?, + }) + } + other => other.clone(), + }) + }) + } +} + +fn collect_strings<'a>(value: Option<&'a serde_json::Value>, out: &mut Vec<&'a str>) { + match value { + Some(serde_json::Value::String(s)) => out.push(s), + Some(serde_json::Value::Array(items)) => { + items.iter().for_each(|v| collect_strings(Some(v), out)) + } + Some(serde_json::Value::Object(map)) => { + map.values().for_each(|v| collect_strings(Some(v), out)) + } + _ => {} + } +} + +/// What a data table of the fork should be copied as. +pub(crate) struct CopyRequest<'a> { + pub(crate) name: &'a str, + pub(crate) dbname: &'a str, + pub(crate) behavior: DataTableForkBehavior, +} + +/// Check that `authed` may copy each data table of `parent_w_id` as asked, before anything is +/// created. +/// +/// Who may copy is what it was before data table roles — anyone for the schema, an admin of the +/// workspace for the rows — narrowed under roles to whoever may connect as one of them: even the +/// schema alone lists every table, which under roles only role holders can read in the parent. +pub(crate) async fn authorize_copies( + db: &DB, + authed: &ApiAuthed, + parent_w_id: &str, + requests: &[CopyRequest<'_>], +) -> Result<()> { + for request in requests { + match request.behavior { + DataTableForkBehavior::KeepOriginal => { + return Err(Error::BadRequest(format!( + "Data table '{}' is kept, which copies nothing", + request.name + ))) + } + DataTableForkBehavior::SchemaOnly => {} + DataTableForkBehavior::SchemaAndData => { + require_admin(authed.is_admin, &authed.username)?; + if *CLOUD_HOSTED { + return Err(Error::BadRequest( + "Cloning schema and data is not available on cloud".to_string(), + )); + } + } + } + windmill_common::validate_dbname(request.dbname)?; + if !request.dbname.starts_with("wm_fork_") { + return Err(Error::BadRequest(format!( + "Forked datatable database name '{}' must start with 'wm_fork_'", + request.dbname + ))); + } + let governing = ensure_datatable_is_clonable(db, parent_w_id, request.name).await?; + ensure_reaches_governing_datatable(db, parent_w_id, request.name, &governing, authed) + .await?; + if governing.datatable.permissions.is_some() { + crate::datatable_replay_oss::ensure_replay()?; + } + } + Ok(()) +} + +/// Make every copy in `requests`, in order, once [`authorize_copies`] allows them all. When one +/// fails, the copies already made are dropped before the error is returned. +pub(crate) async fn make_copies( + db: &DB, + authed: &ApiAuthed, + parent_w_id: &str, + requests: &[CopyRequest<'_>], +) -> Result> { + authorize_copies(db, authed, parent_w_id, requests).await?; + let mut copies = Vec::with_capacity(requests.len()); + for request in requests { + match make_copy(db, authed, parent_w_id, request).await { + Ok(copy) => copies.push(copy), + Err(e) => return Err(drop_copies_after(db, copies, e).await), + } + } + Ok(copies) +} + +/// Drop every copy, returning `error` — with what could not be dropped appended, since nothing +/// else will name those databases again. +pub(crate) async fn drop_copies_after(db: &DB, copies: Vec, error: Error) -> Error { + let mut stranded = Vec::new(); + for copy in copies { + if let Err(e) = drop_copy(db, ©.source_database, ©.dbname).await { + tracing::error!("Could not drop '{}' after a failed fork: {e}", copy.dbname); + stranded.push(format!("'{}' ({e})", copy.dbname)); + } + } + if stranded.is_empty() { + error + } else { + Error::ExecutionErr(format!( + "{error}. These databases created for the fork could not be dropped: {}", + stranded.join(", ") + )) + } +} + +async fn drop_copy(db: &DB, source_database: &DataTableDatabase, dbname: &str) -> Result<()> { + if source_database.resource_type + == windmill_common::workspaces::DataTableCatalogResourceType::Instance + { + let users = windmill_common::drop_unused_instance_database(db, dbname).await?; + if users.is_empty() { + Ok(()) + } else { + Err(Error::BadRequest(format!( + "kept, since workspaces {} now use it", + users.join(", ") + ))) + } + } else { + // 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(), + )) + } +} + +async fn make_copy( + db: &DB, + authed: &ApiAuthed, + parent_w_id: &str, + request: &CopyRequest<'_>, +) -> Result { + let governing = ensure_datatable_is_clonable(db, parent_w_id, request.name).await?; + let source_database = governing.datatable.database.clone().ok_or_else(|| { + Error::internal_err(format!( + "Data table '{}' resolves to an entry that owns no database", + request.name + )) + })?; + let is_instance = governing.is_instance(); + // 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 = serde_json::from_value( + get_datatable_resource_from_db_unchecked(db, parent_w_id, request.name).await?, + ) + .map_err(|e| Error::internal_err(format!("Failed to parse database credentials: {e}")))?; + (server, None) + } else { + let snapshot = connection_snapshot( + db, + &mut *db.acquire().await?, + &governing.workspace_id, + &source_database.resource_path, + ) + .await?; + if !snapshot.root_holds_connection() { + return Err(Error::BadRequest(format!( + "Data table '{}' uses resource '{}', which only refers to another resource: the \ + fork's copy of it could not be pointed at the copied database. Point the data \ + table at the resource holding the connection, then fork again.", + request.name, source_database.resource_path + ))); + } + let server = serde_json::from_value(snapshot.resolve(db, &governing.workspace_id).await?) + .map_err(|e| { + Error::internal_err(format!("Failed to parse database credentials: {e}")) + })?; + (server, Some(snapshot)) + }; + + // 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. + let dump = pg_dump_database( + &server, + PgDumpOptions { + schema_only: request.behavior == DataTableForkBehavior::SchemaOnly, + no_owner: true, + no_acl: is_instance, + ..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?; + } + + let target = PgDatabase { dbname: request.dbname.to_string(), ..server.clone() }; + let filled = fill( + db, + authed, + parent_w_id, + request.name, + &governing, + &server, + &target, + &dump, + ) + .await; + match filled { + Ok(replayed) => Ok(MadeCopy { + name: request.name.to_string(), + dbname: request.dbname.to_string(), + behavior: request.behavior, + source_database, + replayed, + connection, + }), + Err(e) => { + if let Err(drop_err) = drop_copy(db, &source_database, request.dbname).await { + tracing::error!( + "Could not drop '{}' after a failed copy of data table '{}': {drop_err}", + request.dbname, + request.name + ); + return Err(Error::ExecutionErr(format!( + "{e}. The database '{}' created for the copy could not be dropped: {drop_err}", + request.dbname + ))); + } + Err(e) + } + } +} + +/// Restore the dump into the new database and, for a data table under roles, replay the source's +/// owners and grants into it. Returns whether it replayed. +#[allow(clippy::too_many_arguments)] +async fn fill( + db: &DB, + authed: &ApiAuthed, + parent_w_id: &str, + name: &str, + governing: &GoverningDatatable, + source: &PgDatabase, + target: &PgDatabase, + dump: &DumpFile, +) -> Result { + pg_import_dump(target, dump).await?; + + // Held until the replay commits: a role renamed or dropped meanwhile would change what the + // replay names, and a settings save could move the source onto another database or put it under + // roles. Taken in the same order as the permissions save and the ACL apply. + // On a connection of its own: the checks below take theirs from the pool, and a lock holder + // drawn from the pool too could leave a small one with nothing to give them. + let database_url = windmill_common::get_database_url().await?; + let mut lock_holder = + ::connect_with(&database_url.connect_options().await?) + .await + .map_err(|e| Error::internal_err(format!("Failed to connect to the database: {e}")))?; + let mut tx = sqlx::Connection::begin(&mut lock_holder).await?; + lock_role_catalog(&mut tx).await?; + lock_settings_rows(&mut tx, parent_w_id, name).await?; + + // Everything so far was decided before the locks. + let now = ensure_datatable_is_clonable(db, parent_w_id, name).await?; + ensure_reaches_governing_datatable(db, parent_w_id, name, &now, authed).await?; + if !same_database(&now.datatable.database, &governing.datatable.database) { + return Err(Error::BadRequest(format!( + "Data table '{name}' moved to another database while it was being copied; fork again" + ))); + } + + 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 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, + &target.dbname, + &catalog, + ) + .await?; + let catalog_roles: BTreeSet = catalog.values().map(|r| r.name.clone()).collect(); + let (source, _source_notices) = connect_with_notices(db, source).await?; + let (mut target, mut notices) = connect_with_notices(db, target).await?; + // One transaction on the copy: a replay that stops halfway leaves objects owned by one role + // and granted as another. + let pg_tx = target.transaction().await.map_err(|e| { + Error::internal_err(format!( + "Failed to open a transaction on the copy: {}", + pg_error_message(&e) + )) + })?; + crate::datatable_replay_oss::replay_owners_and_grants( + &source, + &pg_tx, + &mut notices, + &catalog_roles, + ) + .await?; + pg_tx.commit().await.map_err(|e| { + Error::internal_err(format!( + "Failed to commit the replayed grants: {}", + pg_error_message(&e) + )) + })?; + } + tx.commit().await?; + Ok(replayed) +} + +/// Lock every settings row the resolution of data table `name` of `start_w_id` passes through, in +/// the order it passes them — each pointer, the entry that owns the database, and each workspace its +/// roles come from — and return them. Deleting or repointing any of them would leave a copy made +/// for the resolution answering for another one. +pub(crate) async fn lock_settings_rows( + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + start_w_id: &str, + name: &str, +) -> Result> { + let mut path: Vec<(String, String)> = vec![]; + let (mut w_id, mut datatable) = (start_w_id.to_string(), name.to_string()); + loop { + if path.contains(&(w_id.clone(), datatable.clone())) || path.len() > 32 { + return Err(Error::BadRequest(format!( + "Data table '{name}' of workspace '{start_w_id}' resolves through a cycle" + ))); + } + let entry: Option = sqlx::query_scalar( + "SELECT datatable->'datatables'->$2 FROM workspace_settings + WHERE workspace_id = $1 FOR UPDATE", + ) + .bind(&w_id) + .bind(&datatable) + .fetch_optional(&mut **tx) + .await? + .flatten(); + path.push((w_id.clone(), datatable.clone())); + let entry: Option = + entry.and_then(|e| serde_json::from_value(e).ok()); + match entry.and_then(|e| e.reference.or(e.governed_by)) { + Some(next) => (w_id, datatable) = (next.workspace_id, next.datatable), + None => break, + } + } + Ok(path.into_iter().map(|(w_id, _)| w_id).collect()) +} + +pub(crate) fn same_database(a: &Option, b: &Option) -> bool { + match (a, b) { + (Some(a), Some(b)) => { + a.resource_type == b.resource_type && a.resource_path == b.resource_path + } + _ => false, + } +} diff --git a/backend/windmill-api-workspaces/src/datatable_replay_oss.rs b/backend/windmill-api-workspaces/src/datatable_replay_oss.rs new file mode 100644 index 0000000000..edcf8e402b --- /dev/null +++ b/backend/windmill-api-workspaces/src/datatable_replay_oss.rs @@ -0,0 +1,48 @@ +/* + * 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 replay of a data table's owners and grants into its copy comes from: the enterprise +//! one, or a refusal. Without it a data table under roles is not copied at all: its rows would +//! arrive owned by the admin connection with no grant for any role. + +#[cfg(all(feature = "private", feature = "enterprise"))] +pub(crate) use crate::datatable_replay_ee::replay_owners_and_grants; + +#[cfg(all(feature = "private", feature = "enterprise"))] +pub(crate) fn ensure_replay() -> windmill_common::error::Result<()> { + Ok(()) +} + +#[cfg(not(all(feature = "private", feature = "enterprise")))] +use { + std::collections::BTreeSet, + tokio::sync::mpsc, + tokio_postgres::error::DbError, + windmill_common::error::{Error, Result}, +}; + +/// Checked before anything is created. +#[cfg(not(all(feature = "private", feature = "enterprise")))] +pub(crate) fn ensure_replay() -> Result<()> { + Err(Error::BadRequest( + "Cloning a data table under roles is a Windmill Enterprise Edition feature: the copy \ + needs the source's owners and grants replayed. Fork it keeping the original database \ + instead." + .to_string(), + )) +} + +#[cfg(not(all(feature = "private", feature = "enterprise")))] +pub(crate) async fn replay_owners_and_grants( + _source: &tokio_postgres::Client, + _target: &tokio_postgres::Transaction<'_>, + _notices: &mut mpsc::UnboundedReceiver, + _catalog_roles: &BTreeSet, +) -> Result<()> { + ensure_replay() +} diff --git a/backend/windmill-api-workspaces/src/lib.rs b/backend/windmill-api-workspaces/src/lib.rs index 5dfdece644..e7da2b8496 100644 --- a/backend/windmill-api-workspaces/src/lib.rs +++ b/backend/windmill-api-workspaces/src/lib.rs @@ -3,9 +3,11 @@ pub mod ai_session_backups; pub mod data_metrics; pub mod datatable_acl; pub mod datatable_acl_oss; +pub mod datatable_clone; pub mod datatable_migrations; pub mod datatable_permissions; pub mod datatable_permissions_oss; +pub mod datatable_replay_oss; pub mod deployment_requests; pub mod workspaces; pub mod workspaces_extra; @@ -16,6 +18,8 @@ pub mod workspaces_ee; #[cfg(all(feature = "private", feature = "enterprise"))] pub mod datatable_acl_ee; +#[cfg(all(feature = "private", feature = "enterprise"))] +pub mod datatable_replay_ee; #[cfg(all(feature = "private", feature = "enterprise"))] pub mod datatable_permissions_ee; diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index b31b0d0785..b434eb0084 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -494,8 +494,8 @@ struct CreateWorkspaceFork { id: String, name: String, color: Option, - /// Datatable names that were forked. For each, the backend will update the - /// forked workspace's datatable config to point to the new database. + /// Data tables the fork gets a copy of rather than a pointer at the parent's. For each, the + /// fork's entry is pointed at the new database. #[serde(default)] forked_datatables: Vec, /// Lakes the user explicitly chose to SHARE with the parent (the fork then reads and @@ -527,6 +527,11 @@ struct CreateWorkspaceFork { struct ForkedDatatableInfo { name: String, new_dbname: String, + /// What this request copies into `new_dbname`, which it creates. Optional only so that the + /// entry older CLIs send — naming a database they created and filled themselves — is refused + /// with a message rather than a parse error. + #[serde(default)] + fork_behavior: Option, } #[derive(Deserialize)] @@ -2223,8 +2228,8 @@ async fn list_datatables( name, resource_type: database.resource_type.as_ref().to_string(), resource_path: database.resource_path.clone(), - governing_workspace_id: (governing.workspace_id != w_id) - .then(|| governing.workspace_id.clone()), + governing_workspace_id: (governing.governing_workspace_id() != w_id) + .then(|| governing.governing_workspace_id().to_string()), permissioned: governing.datatable.permissions.is_some(), }); } @@ -2796,6 +2801,18 @@ fn truncate_column_default(default: String) -> String { mod tests { use super::*; + #[test] + fn a_dev_workspace_copies_into_the_fork_database_namespace() { + assert_eq!( + forked_datatable_dbname("wm-fork-my-fork", "main"), + "wm_fork_my_fork__main" + ); + assert_eq!( + forked_datatable_dbname("my-dev", "main"), + "wm_fork_my_dev__main" + ); + } + /// The header of a pg_dump, followed by an object whose body also holds a `SET`. const DUMP: &str = "--\n\ -- PostgreSQL database dump\n\ @@ -3264,7 +3281,10 @@ async fn server_setting_names(pg_db: &PgDatabase) -> Result> { /// so a dump that breaks partway through imports partially and reads as a success. /// ON_ERROR_STOP surfaces the failure and --single-transaction makes the restore /// all-or-nothing, leaving the target as it was and the import retryable. -async fn pg_import_dump(target_db: &PgDatabase, dump_file: &DumpFile) -> Result<()> { +/// +/// Authorization: none. Writes into `target_db` with the credentials it carries; callers MUST have +/// authorized writing to that database. +pub(crate) async fn pg_import_dump(target_db: &PgDatabase, dump_file: &DumpFile) -> Result<()> { let supported_settings = server_setting_names(target_db).await?; comment_out_unsupported_settings(dump_file, &supported_settings).await?; @@ -3313,7 +3333,7 @@ async fn create_pg_database( // exists rather than leaving an empty registered `wm_fork_…` behind. if let Some(reference) = req.source.strip_prefix("datatable://") { let (name, _) = parse_datatable_ref_for(&db, &w_id, reference).await?; - ensure_datatable_is_clonable(&db, &w_id, &name).await?; + ensure_copied_without_roles(&ensure_datatable_is_clonable(&db, &w_id, &name).await?)?; } // Non-superadmin: restrict dbname to wm_fork_ prefix @@ -3332,50 +3352,62 @@ async fn create_pg_database( } else { let source_pg = resolve_pg_source_checked(&db, &user_db, &authed, &w_id, &req.source).await?; - let (client, connection) = source_pg.connect(Some(&db)).await?; - let join_handle = tokio::spawn(async move { connection.await }); - - let row = client - .query_one( - "SELECT EXISTS (SELECT 1 FROM pg_catalog.pg_database WHERE datname = $1)", - &[&req.target_dbname], - ) - .await - .map_err(|e| { - Error::internal_err(format!( - "Failed to check database existence: {}", - pg_error_message(&e) - )) - })?; - let db_exists: bool = row.get(0); - - if db_exists { - drop(client); - let _ = windmill_common::shutdown_pg_connection(join_handle).await; - return Err(Error::BadRequest(format!( - "Database '{}' already exists on the resource server", - req.target_dbname - ))); - } - - client - .execute(&format!("CREATE DATABASE \"{}\"", &req.target_dbname), &[]) - .await - .map_err(|e| { - Error::internal_err(format!( - "Failed to create database '{}': {}", - req.target_dbname, - pg_error_message(&e) - )) - })?; - - drop(client); - windmill_common::shutdown_pg_connection(join_handle).await?; + create_database_on_server(&db, &source_pg, &req.target_dbname).await?; } Ok(format!("Created database '{}'", req.target_dbname)) } +/// `CREATE DATABASE` on the server `server` connects to, refusing a name already taken there. +/// +/// Authorization: none. Callers MUST have authorized creating a database on that server — resolved +/// from a data table or resource the caller may administer. +pub(crate) async fn create_database_on_server( + db: &DB, + server: &PgDatabase, + dbname: &str, +) -> Result<()> { + windmill_common::validate_dbname(dbname)?; + let (client, connection) = server.connect(Some(db)).await?; + let join_handle = tokio::spawn(async move { connection.await }); + + let row = client + .query_one( + "SELECT EXISTS (SELECT 1 FROM pg_catalog.pg_database WHERE datname = $1)", + &[&dbname], + ) + .await + .map_err(|e| { + Error::internal_err(format!( + "Failed to check database existence: {}", + pg_error_message(&e) + )) + })?; + let db_exists: bool = row.get(0); + + if db_exists { + drop(client); + let _ = windmill_common::shutdown_pg_connection(join_handle).await; + return Err(Error::BadRequest(format!( + "Database '{dbname}' already exists on the resource server" + ))); + } + + client + .execute(&format!("CREATE DATABASE \"{dbname}\""), &[]) + .await + .map_err(|e| { + Error::internal_err(format!( + "Failed to create database '{dbname}': {}", + pg_error_message(&e) + )) + })?; + + drop(client); + windmill_common::shutdown_pg_connection(join_handle).await?; + Ok(()) +} + #[derive(Deserialize)] struct ImportPgDatabaseRequest { source: String, @@ -3385,48 +3417,22 @@ struct ImportPgDatabaseRequest { fork_behavior: DataTableForkBehavior, } -/// Refuse to copy a data table that is under roles. +/// Whether data table `name` of `w_id` has a shape a copy can be made of, returning what governs +/// it. Checked before anything is created — by the fork request for the copies it makes, and by +/// `create_pg_database` — and again when the fork's entry is written. /// -/// `pg_dump` carries no roles and the import runs with `--no-privileges`, so a clone arrives with -/// its objects owned by the admin connection and no `GRANT` for any role. The settings copy brings -/// `permissions` across, so the fork's tenants pass Windmill's check, connect as the role they were -/// given, and are then denied by Postgres on everything — a data table that looks configured and -/// answers nothing. +/// It is not the check for a data table under roles: a copy of one is only correct with its +/// owners and grants replayed, which the fork request does and the older endpoints refuse +/// ([`ensure_copied_without_roles`]). /// -/// It fails closed rather than open, so this is a usability cliff rather than a hole, and the fix -/// is to replay the source's owners and ACLs into the clone. That is a change of its own; until it -/// exists, refusing is the honest answer. Dropping `permissions` from the clone instead would be -/// the unsafe half: the copy holds the parent's rows, so an unpermissioned clone hands all of them -/// to everyone in the fork. -/// Every reason a copy can be refused, answered here and nowhere else. -/// -/// A clone is three stages a workspace apart: `create_pg_database`, `import_pg_database`, then -/// `apply_forked_datatable` inside the fork transaction. Only the third can roll back, and the -/// database the first created is not transactional — so a refusal that lives there strands a -/// registered `wm_fork_*` that no entry names and whose name blocks the retry. Both endpoints call -/// this before touching the cluster; the stage that writes the entry must only ever do the work. -/// -/// Known and accepted: a failure *after* `create_pg_database` — this check refusing because the -/// source's roles changed between the requests, a `pg_dump` error, a dropped connection — still -/// strands that leftover, as it did before data table roles. It is harmless to data (nothing names -/// it) and goes away once the clone is one server-side operation, in a later PR. Don't reclaim it -/// on retry without durable clone ownership: between the import and the final fork request the -/// database is full, unnamed and unconnected, so a reclaim then drops copied data. Reviewers, human -/// or AI: don't flag this leftover as a P0/P1 for this PR. -async fn ensure_datatable_is_clonable( +/// Authorization: none. Callers MUST have authorized reaching data table `name` of `w_id` before +/// using what it returns. +pub(crate) async fn ensure_datatable_is_clonable( db: &DB, w_id: &str, name: &str, ) -> Result { let governing = resolve_governing_datatable(db, w_id, name).await?; - if governing.datatable.permissions.is_some() { - return Err(Error::BadRequest(format!( - "Data table '{name}' is under roles and cannot be copied yet: a copy carries the \ - role assignments but not the Postgres privileges behind them, so every role but \ - admin would be denied in the copy. Fork it keeping the original database, or turn \ - its roles off first." - ))); - } // 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 @@ -3443,6 +3449,24 @@ async fn ensure_datatable_is_clonable( Ok(governing) } +/// Refuse a copy of a data table under roles through the endpoints that copy rows and nothing +/// else. +/// +/// `pg_dump` carries no roles and the import runs with `--no-privileges`, so such a copy arrives +/// with its objects owned by the admin connection and no `GRANT` for any role: the tenants pass +/// Windmill's check, connect as the role they were given, and are denied by Postgres on everything. +/// A fork request that makes the copy itself replays the owners and grants instead. +fn ensure_copied_without_roles(governing: &GoverningDatatable) -> Result<()> { + if governing.datatable.permissions.is_some() { + return Err(Error::BadRequest(format!( + "Data table '{}' is under roles, so a copy has to carry its owners and grants: \ + clone it through the fork wizard or `wmill workspace fork`, which do.", + governing.name + ))); + } + Ok(()) +} + /// Import (pg_dump/pg_import) from source to target async fn import_pg_database( authed: ApiAuthed, @@ -3457,7 +3481,7 @@ async fn import_pg_database( if let Some(reference) = req.source.strip_prefix("datatable://") { let (name, _) = parse_datatable_ref_for(&db, &w_id, reference).await?; - ensure_datatable_is_clonable(&db, &w_id, &name).await?; + ensure_copied_without_roles(&ensure_datatable_is_clonable(&db, &w_id, &name).await?)?; } if req.fork_behavior == DataTableForkBehavior::SchemaAndData { @@ -3578,6 +3602,15 @@ async fn edit_ducklake_config( } let mut tx = db.begin().await?; + windmill_common::lock_instance_databases( + &mut tx, + new_config.settings.ducklakes.values().filter_map(|lake| { + (lake.catalog.resource_type + == windmill_common::workspaces::DucklakeCatalogResourceType::Instance) + .then_some(lake.catalog.resource_path.as_str()) + }), + ) + .await?; let args_for_audit = format!("{:?}", new_config.settings); audit_log( @@ -3679,6 +3712,16 @@ async fn edit_datatable_config( let is_superadmin = require_super_admin(&db, &authed).await.is_ok(); let mut tx = db.begin().await?; + windmill_common::lock_instance_databases( + &mut tx, + new_config.settings.datatables.values().filter_map(|dt| { + dt.database + .as_ref() + .filter(|d| d.resource_type == DataTableCatalogResourceType::Instance) + .map(|d| d.resource_path.as_str()) + }), + ) + .await?; // Read under the row lock this transaction will write with. `permissions`, `reference` and // `forked_from` are carried across from what this read returns, so a permissions save @@ -3804,12 +3847,28 @@ async fn edit_datatable_config( // hand the fork the database outright. `forked_from` is the clone stamp the fork flow // writes: whether an entry has one is carried the same way, since it is what marks the // database droppable, but the schema baseline inside it is the diff view's to advance. + // `governed_by` is a clone's `reference` for its roles, and clearing it would hand the + // fork the copied rows the same way. dt.permissions = old.and_then(|old| old.permissions.clone()); dt.reference = old.and_then(|old| old.reference.clone()); + dt.governed_by = old.and_then(|old| old.governed_by.clone()); dt.forked_from = match old.and_then(|old| old.forked_from.as_ref()) { Some(stored) => Some(dt.forked_from.take().unwrap_or_else(|| stored.clone())), None => None, }; + // A clone's roles and grants were replayed into the database it was copied into, and hold + // for that database alone: whoever saves, it stays where it is. + if dt.governed_by.is_some() + && !crate::datatable_clone::same_database( + &dt.database, + &old.and_then(|old| old.database.clone()), + ) + { + return Err(Error::BadRequest(format!( + "Data table '{name}' is a clone taking its roles from the data table it was copied \ + from, and its grants hold for its own database only: it cannot be moved." + ))); + } // 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 @@ -3848,9 +3907,18 @@ async fn edit_datatable_config( // 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. + // + // 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(name); + let old_dt = old_datatables.get( + rename_src + .get(name.as_str()) + .copied() + .unwrap_or(name.as_str()), + ); if dt .database .as_ref() @@ -3890,7 +3958,8 @@ async fn edit_datatable_config( .settings .datatables .iter() - .filter(|(_, dt)| dt.permissions.is_none()) + // A clone carries its roles through `governed_by` rather than `permissions`. + .filter(|(_, dt)| dt.permissions.is_none() && dt.governed_by.is_none()) .filter_map(|(name, dt)| { let db = dt .database @@ -3923,7 +3992,7 @@ async fn edit_datatable_config( sqlx::query_scalar( "SELECT DISTINCT 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' + WHERE ws.workspace_id <> $1 AND (dt.value ? 'permissions' OR dt.value ? 'governed_by') AND dt.value->'database'->>'resource_type' = 'instance'", ) .bind(&w_id) @@ -3932,7 +4001,7 @@ async fn edit_datatable_config( }; for (name, dbname) in newly_pointed { let governed_here = old_datatables.values().any(|old| { - old.permissions.is_some() + (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 @@ -3992,8 +4061,11 @@ async fn edit_datatable_config( r#"SELECT ws.workspace_id AS "workspace_id!", dt.key AS "datatable!" FROM workspace_settings ws CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt - WHERE dt.value->'reference'->>'workspace_id' = $1 - AND dt.value->'reference'->>'datatable' = $2"#, + WHERE EXISTS ( + SELECT 1 FROM (VALUES ('reference'), ('governed_by')) k(link) + WHERE dt.value->k.link->>'workspace_id' = $1 + AND dt.value->k.link->>'datatable' = $2 + )"#, &w_id, name, ) @@ -7873,6 +7945,7 @@ async fn create_workspace_fork_branch( // dangling branch on the synced repos. check_fork_w_id_conflict(&db, &nw.id).await?; purge_stale_fork_diff_state(&db, &nw.id).await?; + validate_forked_datatables(&db, &authed, &w_id, &nw).await?; Ok(Json( handle_fork_branch_creation(&authed.email, &authed.username, &db, &w_id, &nw.id).await?, @@ -7912,8 +7985,9 @@ async fn snapshot_datatable_schema( /// a fork admin could then edit to widen their own access to it. A pointer has nothing local to /// edit: the parent's entry stays the only place the decision lives. /// -/// The cloned data tables are skipped: they own a fresh database of their own, and they keep the -/// copied `permissions` as their starting point, which they then govern. +/// The cloned data tables are skipped: they own a fresh database of their own, and +/// `apply_forked_datatable` has already dropped their copied `permissions` — for a clone of a data +/// table under roles, in favor of `governed_by`. async fn point_kept_datatables_at_parent( tx: &mut Transaction<'_, Postgres>, parent_w_id: &str, @@ -7968,6 +8042,7 @@ async fn point_kept_datatables_at_parent( workspace_id: parent_w_id.to_string(), datatable: name.clone(), }), + governed_by: None, forked_from: None, migrations_enabled: dt.migrations_enabled, permissions: None, @@ -7988,7 +8063,8 @@ async fn point_kept_datatables_at_parent( Ok(()) } -/// Move every pointer in any workspace that names `(w_id, from)` to `(w_id, to)`. +/// Move every pointer, and every clone's `governed_by`, in any workspace that names `(w_id, from)` +/// to `(w_id, to)`. /// /// `EXISTS` rather than a `LIKE` over the whole document: the update rewrites the row, so matching /// every workspace that holds any pointer would rewrite rows to a byte-identical value and hold an @@ -8004,17 +8080,22 @@ async fn repoint_datatable_references( SET datatable = ( SELECT jsonb_set(ws.datatable, '{datatables}', jsonb_object_agg( dt.key, - CASE WHEN dt.value->'reference'->>'workspace_id' = $1 - AND dt.value->'reference'->>'datatable' = $2 - THEN jsonb_set(dt.value, '{reference,datatable}', to_jsonb($3::text)) - ELSE dt.value END + (SELECT CASE WHEN r.v->'governed_by'->>'workspace_id' = $1 + AND r.v->'governed_by'->>'datatable' = $2 + THEN jsonb_set(r.v, '{governed_by,datatable}', to_jsonb($3::text)) + ELSE r.v END + FROM (SELECT CASE WHEN dt.value->'reference'->>'workspace_id' = $1 + AND dt.value->'reference'->>'datatable' = $2 + THEN jsonb_set(dt.value, '{reference,datatable}', to_jsonb($3::text)) + ELSE dt.value END AS v) r) )) FROM jsonb_each(ws.datatable->'datatables') dt ) WHERE EXISTS ( - SELECT 1 FROM jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) d - WHERE d.value->'reference'->>'workspace_id' = $1 - AND d.value->'reference'->>'datatable' = $2 + SELECT 1 FROM jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) d, + LATERAL (VALUES ('reference'), ('governed_by')) k(link) + WHERE d.value->k.link->>'workspace_id' = $1 + AND d.value->k.link->>'datatable' = $2 )"#, w_id, from, @@ -8032,17 +8113,8 @@ async fn apply_forked_datatable( parent_w_id: &str, forked_w_id: &str, fdt: &ForkedDatatableInfo, + copy: &crate::datatable_clone::MadeCopy, ) -> Result<()> { - // Cloning reads the parent's whole schema as admin and hands the copy to the fork, so it is - // for the workspace that governs the data table — a fork can use one, never duplicate it. - windmill_common::workspaces::ensure_datatable_admin_access( - db, - parent_w_id, - &fdt.name, - &DatatableAccess::Authed(authed.to_authed_ref()), - ) - .await?; - let governing = ensure_datatable_is_clonable(db, parent_w_id, &fdt.name).await?; windmill_common::validate_dbname(&fdt.new_dbname)?; if !fdt.new_dbname.starts_with("wm_fork_") { return Err(Error::BadRequest(format!( @@ -8050,6 +8122,52 @@ async fn apply_forked_datatable( fdt.new_dbname ))); } + // Held until the fork commits, as the permissions save holds them: the source moved onto + // another database, or put under roles or taken off them, since its copy was made would link + // the copy to roles its grants were not replayed for. + let locked = crate::datatable_clone::lock_settings_rows(tx, parent_w_id, &fdt.name).await?; + let governing = ensure_datatable_is_clonable(db, parent_w_id, &fdt.name).await?; + let under_roles = governing.datatable.permissions.is_some(); + let unchanged = crate::datatable_clone::same_database( + &governing.datatable.database, + &Some(copy.source_database.clone()), + ) && copy.replayed == under_roles + && crate::datatable_clone::lock_settings_rows(tx, parent_w_id, &fdt.name).await? == locked; + if !unchanged { + return Err(Error::BadRequest(format!( + "Data table '{}' changed while it was being copied for this fork — its database, or \ + whether it is under roles. Create the fork again.", + fdt.name + ))); + } + // The fork keeps a snapshot of the source's schema, which under roles is only for those who may + // connect as one of them — as the copy itself was. + crate::datatable_permissions::ensure_reaches_governing_datatable( + db, + parent_w_id, + &fdt.name, + &governing, + authed, + ) + .await?; + // Under roles, the copy holds rows the source's roles decide who reaches, so that entry keeps + // deciding. Settled from the source as it resolves now: the settings clone may have handed the + // fork a pointer, or a clone of its own. A copy of a data table without roles is the fork's, + // as it was before roles existed: everyone reached all of it already. + let governed_by = governing + .datatable + .permissions + .is_some() + .then(|| { + serde_json::to_value(governing.governor.clone().unwrap_or_else(|| { + windmill_common::workspaces::DataTableReference { + workspace_id: governing.workspace_id.clone(), + datatable: governing.name.clone(), + } + })) + }) + .transpose() + .map_err(|e| Error::internal_err(format!("serializing a clone's governor: {e}")))?; // Snapshot the schema from the source (parent) datatable let schema = snapshot_datatable_schema(db, parent_w_id, &fdt.name).await?; @@ -8095,33 +8213,42 @@ async fn apply_forked_datatable( "resource_type": "instance", "resource_path": &fdt.new_dbname, }); - sqlx::query!( + sqlx::query( r#"UPDATE workspace_settings SET datatable = jsonb_set( - jsonb_set( - datatable #- ARRAY['datatables', $2, 'reference'], - ARRAY['datatables', $2, 'database'], $3::jsonb), - ARRAY['datatables', $2, 'forked_from'], $4::jsonb - ) + datatable #- ARRAY['datatables', $2, 'reference'], + ARRAY['datatables', $2, 'database'], $3::jsonb) WHERE workspace_id = $1"#, - forked_w_id, - &fdt.name, - new_database, - forked_from, ) + .bind(forked_w_id) + .bind(&fdt.name) + .bind(new_database) .execute(&mut **tx) .await?; } else { - // Resource: update the resource's dbname and mark as ws_specific + // Resource: point it at the copy and mark it ws_specific. What the settings clone carried is + // the source's resource and variables as they are now, which an edit made while the copy + // was being made can have changed, and changed back: the clone has to be what the copy's + // connection was resolved from. let resource_path = &database.resource_path; - sqlx::query!( + let cloned = + crate::datatable_clone::connection_snapshot(db, &mut **tx, forked_w_id, resource_path) + .await?; + if copy.connection.as_ref() != Some(&cloned) { + return Err(Error::BadRequest(format!( + "The resource of data table '{}' changed while it was being copied for this \ + fork. Create the fork again.", + fdt.name + ))); + } + sqlx::query( r#"UPDATE resource SET value = jsonb_set(value, '{dbname}', to_jsonb($3::text)) WHERE workspace_id = $1 AND path = $2"#, - forked_w_id, - resource_path, - &fdt.new_dbname, ) + .bind(forked_w_id) + .bind(resource_path) + .bind(&fdt.new_dbname) .execute(&mut **tx) .await?; @@ -8132,20 +8259,31 @@ async fn apply_forked_datatable( ) .execute(&mut **tx) .await?; - - // Set forked_from on the datatable config - sqlx::query!( - r#"UPDATE workspace_settings - SET datatable = jsonb_set(datatable, ARRAY['datatables', $2, 'forked_from'], $3::jsonb) - WHERE workspace_id = $1"#, - forked_w_id, - &fdt.name, - forked_from, - ) - .execute(&mut **tx) - .await?; } + // The settings clone copied the source's `permissions` along, and a clone of a clone its + // `governed_by`: a clone keeps no `permissions` of its own, and a link only under roles. + sqlx::query( + r#"UPDATE workspace_settings + SET datatable = jsonb_set( + CASE WHEN $3::jsonb IS NULL + THEN datatable #- ARRAY['datatables', $2, 'permissions'] + #- ARRAY['datatables', $2, 'governed_by'] + ELSE jsonb_set( + datatable #- ARRAY['datatables', $2, 'permissions'], + ARRAY['datatables', $2, 'governed_by'], $3::jsonb) + END, + ARRAY['datatables', $2, 'forked_from'], $4::jsonb + ) + WHERE workspace_id = $1"#, + ) + .bind(forked_w_id) + .bind(&fdt.name) + .bind(governed_by) + .bind(forked_from) + .execute(&mut **tx) + .await?; + Ok(()) } @@ -8480,7 +8618,169 @@ async fn create_workspace_fork( ensure_no_existing_dev_workspace(&db, &parent_workspace_id).await?; } + // Refused here, before any database exists; `make_copies` checks again under the locks. + validate_forked_datatables(&db, &authed, &parent_workspace_id, &nw).await?; + + // Detached from the request: a client that goes away while the copies are made must still end + // with the fork created or no copy left behind, and dropping the handler's future would skip + // that cleanup. + tokio::spawn(async move { + let fork_id = nw.id.clone(); + let copies = crate::datatable_clone::make_copies( + &db, + &authed, + &parent_workspace_id, + ©_requests(&nw.forked_datatables), + ) + .await?; + let replayed: Vec = copies + .iter() + .filter(|c| c.replayed) + .map(|c| c.behavior) + .collect(); + match write_workspace_fork( + db.clone(), + authed, + parent_workspace_id, + nw, + dev_workspace_label, + &copies, + ) + .await + { + Ok(message) => { + for behavior in replayed { + windmill_common::feature_usage::log_feature_usage( + "datatable", + "clone_replayed", + match behavior { + DataTableForkBehavior::SchemaOnly => "schema_only", + _ => "schema_and_data", + }, + ); + } + Ok(message) + } + // A commit whose acknowledgement was lost can still have committed, and a concurrent + // request for the same id can have: once the write has settled, the cleanup keeps + // whichever copies a committed fork names, and drops the rest. + Err(e) => match wait_for_fork_write(&db, &fork_id).await { + Ok(()) => Err(crate::datatable_clone::drop_copies_after(&db, copies, e).await), + Err(settle) => { + tracing::error!( + "Could not tell whether fork '{fork_id}' was created, so its copies were \ + kept: {settle}" + ); + Err(e) + } + }, + } + }) + .await + .map_err(|e| Error::internal_err(format!("Creating the fork stopped unexpectedly: {e}")))? +} + +/// The database a data table of the fork or dev workspace `fork_id` is copied into, as the wizard +/// and the CLI derive it. A dev workspace's id has no `wm-fork-` prefix, and its copies are dropped +/// by the same `wm_fork_` rule as a fork's. Dev workspace `x` and fork `wm-fork-x` share a name, as +/// their branches do: the second to copy a data table of that name is refused at `CREATE`. +fn forked_datatable_dbname(fork_id: &str, datatable: &str) -> String { + let suffix = fork_id + .strip_prefix(windmill_common::workspaces::WM_FORK_PREFIX) + .unwrap_or(fork_id); + format!("wm_fork_{}__{datatable}", suffix.replace('-', "_")) +} + +/// Wait until no transaction writing fork `fork_id` is still running: the lock +/// `write_workspace_fork` holds until its transaction ends is taken and released. +async fn wait_for_fork_write(db: &DB, fork_id: &str) -> Result<()> { + let mut tx = db.begin().await?; + sqlx::query("SELECT pg_advisory_xact_lock(hashtext('fork:' || $1))") + .bind(fork_id) + .execute(&mut *tx) + .await?; + tx.commit().await?; + Ok(()) +} + +/// Everything about a fork's data tables that can be refused before a git branch or a database is +/// created, and that the fork request re-checks. +/// +/// Every data table is copied by the fork request, into a database it creates: an entry naming a +/// database that already exists could reach a copy made for another fork — one left behind by a +/// deleted fork included — without that copy's governance. +async fn validate_forked_datatables( + db: &DB, + authed: &ApiAuthed, + parent_w_id: &str, + nw: &CreateWorkspaceFork, +) -> Result<()> { + let mut names = HashSet::new(); + for fdt in &nw.forked_datatables { + if fdt.fork_behavior.is_none() { + return Err(Error::BadRequest(format!( + "Data table '{}' names no `fork_behavior`: the fork request makes the copy of each \ + data table itself. Update the Windmill CLI.", + fdt.name + ))); + } + let expected = forked_datatable_dbname(&nw.id, &fdt.name); + if fdt.new_dbname != expected { + return Err(Error::BadRequest(format!( + "Data table '{}' of fork '{}' is copied into database '{expected}', not '{}'", + fdt.name, nw.id, fdt.new_dbname + ))); + } + if !names.insert(fdt.name.as_str()) { + return Err(Error::BadRequest(format!( + "Data table '{}' is named more than once in this fork", + fdt.name + ))); + } + } + crate::datatable_clone::authorize_copies( + db, + authed, + parent_w_id, + ©_requests(&nw.forked_datatables), + ) + .await +} + +/// The copies a fork request asks this request to make. [`validate_forked_datatables`] refuses an +/// entry without a `fork_behavior`. +fn copy_requests( + forked_datatables: &[ForkedDatatableInfo], +) -> Vec> { + forked_datatables + .iter() + .filter_map(|fdt| { + fdt.fork_behavior + .map(|behavior| crate::datatable_clone::CopyRequest { + name: &fdt.name, + dbname: &fdt.new_dbname, + behavior, + }) + }) + .collect() +} + +/// Write the fork, with its data tables pointed at `copies`. +async fn write_workspace_fork( + db: DB, + authed: ApiAuthed, + parent_workspace_id: String, + nw: CreateWorkspaceFork, + dev_workspace_label: Option, + copies: &[crate::datatable_clone::MadeCopy], +) -> Result { let mut tx: Transaction<'_, Postgres> = db.begin().await?; + // Held until this transaction ends: after an error, the copies' cleanup waits on it, so it reads + // whether the fork exists only once a commit still being resolved has settled. + sqlx::query("SELECT pg_advisory_xact_lock(hashtext('fork:' || $1))") + .bind(&nw.id) + .execute(&mut *tx) + .await?; if nw.is_dev_workspace { // The checks above ran outside a transaction, so the parent's eligibility and the chain's @@ -8593,10 +8893,41 @@ async fn create_workspace_fork( // re-enables in the fork, with parent-conflict warnings on enable. clone_triggers_and_schedules(&mut tx, &parent_workspace_id, &forked_id).await?; + // Held until the fork commits, as settings saves naming these databases hold it: an entry + // saved elsewhere while a copy was being made would reach it without the governance written + // below, and the alias check on that save saw no governed entry yet. + let instance_copies: Vec<&str> = copies + .iter() + .filter(|c| c.source_database.resource_type == DataTableCatalogResourceType::Instance) + .map(|c| c.dbname.as_str()) + .collect(); + windmill_common::lock_instance_databases(&mut tx, instance_copies.iter().copied()).await?; + for dbname in &instance_copies { + let users = windmill_common::instance_database_users(&mut *tx, dbname).await?; + if !users.is_empty() { + return Err(Error::BadRequest(format!( + "Database '{dbname}' copied for this fork is already used by workspaces {}; \ + fork again", + users.join(", ") + ))); + } + } + // Update forked datatable settings to point to new databases for fdt in &nw.forked_datatables { - apply_forked_datatable(&db, &mut tx, &authed, &parent_workspace_id, &forked_id, fdt) - .await?; + let copy = copies.iter().find(|c| c.name == fdt.name).ok_or_else(|| { + Error::internal_err(format!("No copy was made of data table '{}'", fdt.name)) + })?; + apply_forked_datatable( + &db, + &mut tx, + &authed, + &parent_workspace_id, + &forked_id, + fdt, + copy, + ) + .await?; } point_kept_datatables_at_parent( @@ -8655,6 +8986,20 @@ async fn create_workspace_fork( .await?; } + let copied = copies + .iter() + .map(|c| { + format!( + "{} ({})", + c.name, + match c.behavior { + DataTableForkBehavior::SchemaOnly => "schema_only", + _ => "schema_and_data", + } + ) + }) + .collect::>() + .join(", "); audit_log( &mut *tx, &authed, @@ -8662,7 +9007,7 @@ async fn create_workspace_fork( ActionKind::Create, &forked_id, Some(nw.name.as_str()), - None, + (!copied.is_empty()).then(|| [("copied_datatables", copied.as_str())].into()), ) .await?; tx.commit().await?; diff --git a/backend/windmill-api-workspaces/src/workspaces_extra.rs b/backend/windmill-api-workspaces/src/workspaces_extra.rs index 1cb5188e5b..e64142f05e 100644 --- a/backend/windmill-api-workspaces/src/workspaces_extra.rs +++ b/backend/windmill-api-workspaces/src/workspaces_extra.rs @@ -495,21 +495,25 @@ pub(crate) async fn change_workspace_id( // A fork's data table entry names the workspace that governs it by id, so the rename has to // follow there too — anywhere, not just in the reparented children: a detached workspace can // point at this one without being its fork. Left behind, the pointer resolves to the archived - // shell and every job through it stops. + // shell and every job through it stops. A clone's `governed_by` names it the same way. info!("Re-pointing data table references to the new workspace id"); sqlx::query!( r#"UPDATE workspace_settings ws SET datatable = ( SELECT jsonb_set(ws.datatable, '{datatables}', jsonb_object_agg( dt.key, - CASE WHEN dt.value->'reference'->>'workspace_id' = $2 - THEN jsonb_set(dt.value, '{reference,workspace_id}', to_jsonb($1::text)) - ELSE dt.value END + (SELECT CASE WHEN r.v->'governed_by'->>'workspace_id' = $2 + THEN jsonb_set(r.v, '{governed_by,workspace_id}', to_jsonb($1::text)) + ELSE r.v END + FROM (SELECT CASE WHEN dt.value->'reference'->>'workspace_id' = $2 + THEN jsonb_set(dt.value, '{reference,workspace_id}', to_jsonb($1::text)) + ELSE dt.value END AS v) r) )) FROM jsonb_each(ws.datatable->'datatables') dt ) WHERE jsonb_typeof(ws.datatable->'datatables') = 'object' - AND ws.datatable::text LIKE '%"reference"%'"#, + AND (ws.datatable::text LIKE '%"reference"%' + OR ws.datatable::text LIKE '%"governed_by"%')"#, &rw.new_id, &old_id, ) @@ -996,17 +1000,19 @@ pub(crate) async fn delete_workspace( // fails mid-way must never leave a live workspace with its fork data destroyed and no // registry row to retry from. Read-only: nothing is dropped here. // Read before the delete: another workspace's data table entry can point at one of this - // workspace's, and deleting the workspace it names leaves that pointer resolving to nothing. - // Nothing sweeps them — turning them back into copies would hand each fork the database - // outright — so the deleter is told which data tables they just stranded. - let stranded_pointers = sqlx::query!( - r#"SELECT ws.workspace_id AS "workspace_id!", dt.key AS "datatable!" + // workspace's, or be a clone taking its roles from one, and deleting the workspace it names + // leaves it resolving to nothing. Nothing sweeps them — turning them back into copies would + // hand each fork the database outright — so the deleter is told which data tables they just + // stranded. + let stranded_pointers = sqlx::query_as::<_, (String, String)>( + r#"SELECT ws.workspace_id, dt.key FROM workspace_settings ws CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt WHERE dt.value->'reference'->>'workspace_id' = $1 + OR dt.value->'governed_by'->>'workspace_id' = $1 ORDER BY ws.workspace_id, dt.key"#, - &w_id, ) + .bind(&w_id) .fetch_all(&db) .await .unwrap_or_default(); @@ -1334,7 +1340,7 @@ pub(crate) async fn delete_workspace( } else { let stranded = stranded_pointers .iter() - .map(|r| format!("{}/{}", r.workspace_id, r.datatable)) + .map(|(workspace_id, datatable)| format!("{workspace_id}/{datatable}")) .collect::>() .join(", "); Ok(format!( diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index fc1831b59c..703ceb077c 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -33723,6 +33723,15 @@ components: $ref: "#/components/schemas/DatatableRoleTenants" governing_workspace_id: type: string + clone_of: + type: object + description: for a clone, the data table whose roles it takes + required: [workspace_id, datatable] + properties: + workspace_id: + type: string + datatable: + type: string editable: type: boolean available_roles: @@ -33961,7 +33970,7 @@ components: DatatableAclInfo: type: object - required: [owner, roles, editable, supports_maintain, dbname, grants, children] + required: [owner, roles, editable, clone, supports_maintain, dbname, grants, children] properties: owner: type: string @@ -33973,6 +33982,9 @@ components: editable: type: boolean description: whether the caller may plan and apply changes + clone: + type: boolean + description: whether this is a clone, whose grants stay as they were copied supports_maintain: type: boolean description: whether the server is Postgres 17+, which added the MAINTAIN table privilege @@ -35122,6 +35134,16 @@ components: new_dbname: type: string description: "New database name for the fork" + fork_behavior: + type: string + enum: + - schema_only + - schema_and_data + description: >- + What the fork request copies into `new_dbname`, which it creates — with the + owners and grants of a data table under roles. This server refuses an entry + without it; servers predating it expect `new_dbname` created and filled + beforehand. shared_ducklakes: type: array items: @@ -36064,6 +36086,17 @@ components: type: string datatable: type: string + governed_by: + description: >- + On a clone, the data table it was copied from, whose roles it takes. Server-owned + like `reference`. + type: object + required: [workspace_id, datatable] + properties: + workspace_id: + type: string + datatable: + type: string migrations_enabled: type: boolean description: Whether the SQL migrations feature is opted in for this data table diff --git a/backend/windmill-common/src/lib.rs b/backend/windmill-common/src/lib.rs index a67bb6bfa1..5feaba7800 100644 --- a/backend/windmill-common/src/lib.rs +++ b/backend/windmill-common/src/lib.rs @@ -1474,8 +1474,98 @@ pub fn validate_dbname(dbname: &str) -> error::Result<()> { Ok(()) } +/// Lock the instance databases among `names` until `tx` ends, in a stable order. Taken by every +/// settings save naming an instance database, and by [`drop_unused_instance_database`]: a save +/// cannot start using a database between that drop's check that nothing does and the drop. +pub async fn lock_instance_databases<'a>( + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + names: impl IntoIterator, +) -> error::Result<()> { + let names: std::collections::BTreeSet<&str> = names.into_iter().collect(); + for name in names { + sqlx::query("SELECT pg_advisory_xact_lock(hashtext('instance_database:' || $1))") + .bind(name) + .execute(&mut **tx) + .await?; + } + Ok(()) +} + +/// Drop instance database `dbname`, which a request just created, unless a workspace names it as a +/// data table or a ducklake catalog — the workspaces returned, with nothing dropped. A database is +/// registered on the instance as soon as it is created, so a superadmin can point a workspace at it +/// before the request that created it gives up on it. +/// +/// Authorization: none. Callers MUST pass only a database the same request created and has not +/// handed to anything yet. +/// +/// The lock, the check and the drop share one connection: a second one taken from the pool while +/// the first is held could wait forever on a small pool. +pub async fn drop_unused_instance_database(db: &DB, dbname: &str) -> error::Result> { + let mut conn = db.acquire().await?; + let key = format!("instance_database:{dbname}"); + sqlx::query("SELECT pg_advisory_lock(hashtext($1))") + .bind(&key) + .execute(&mut *conn) + .await?; + let dropped = drop_if_unused_on(&mut conn, dbname).await; + let unlocked = sqlx::query("SELECT pg_advisory_unlock(hashtext($1))") + .bind(&key) + .execute(&mut *conn) + .await; + if unlocked.is_err() { + // Closing the connection is what releases a session lock it could not release itself. + drop(conn.detach()); + } + dropped +} + +async fn drop_if_unused_on( + conn: &mut sqlx::PgConnection, + dbname: &str, +) -> error::Result> { + let users = instance_database_users(conn, dbname).await?; + if users.is_empty() { + drop_custom_instance_database_on(conn, dbname).await?; + } + Ok(users) +} + +/// The workspaces naming instance database `dbname` as a data table or a ducklake catalog. Taken +/// under [`lock_instance_databases`] for `dbname`, the answer holds until that lock is released. +/// +/// Authorization: none, and it reads every workspace's settings. Callers MUST pass only a database +/// their own request created, and may name the workspaces returned only to a caller allowed to +/// create or drop instance databases. +pub async fn instance_database_users( + conn: &mut sqlx::PgConnection, + dbname: &str, +) -> error::Result> { + Ok(sqlx::query_scalar( + "SELECT DISTINCT ws.workspace_id FROM workspace_settings ws + WHERE EXISTS (SELECT 1 FROM jsonb_each(CASE WHEN jsonb_typeof(ws.datatable->'datatables') = 'object' + THEN ws.datatable->'datatables' ELSE '{}'::jsonb END) dt + WHERE dt.value->'database'->>'resource_type' = 'instance' + AND dt.value->'database'->>'resource_path' = $1) + OR EXISTS (SELECT 1 FROM jsonb_each(CASE WHEN jsonb_typeof(ws.ducklake->'ducklakes') = 'object' + THEN ws.ducklake->'ducklakes' ELSE '{}'::jsonb END) dl + WHERE dl.value->'catalog'->>'resource_type' = 'instance' + AND dl.value->'catalog'->>'resource_path' = $1)", + ) + .bind(dbname) + .fetch_all(conn) + .await?) +} + /// Drop a custom instance database: validate, terminate connections, DROP DATABASE, remove from global_settings. pub async fn drop_custom_instance_database(db: &DB, dbname: &str) -> error::Result<()> { + drop_custom_instance_database_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(); validate_dbname(dbname)?; @@ -1490,7 +1580,7 @@ pub async fn drop_custom_instance_database(db: &DB, dbname: &str) -> error::Resu "SELECT EXISTS (SELECT 1 FROM pg_catalog.pg_database WHERE datname = $1)", dbname ) - .fetch_one(db) + .fetch_one(&mut *conn) .await? .unwrap_or(false); @@ -1501,7 +1591,7 @@ pub async fn drop_custom_instance_database(db: &DB, dbname: &str) -> error::Resu "SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = '{}' AND pid <> pg_backend_pid()", dbname.replace('\'', "''") )) - .execute(db) + .execute(&mut *conn) .await { tracing::warn!("Failed to terminate connections to '{}': {}", dbname, e); @@ -1510,7 +1600,7 @@ pub async fn drop_custom_instance_database(db: &DB, dbname: &str) -> error::Resu // Drop the database // SAFETY: `dbname` has been validated via validate_dbname() before reaching this point. sqlx::query(&format!("DROP DATABASE IF EXISTS \"{}\"", dbname)) - .execute(db) + .execute(&mut *conn) .await .map_err(|e| { error::Error::internal_err(format!("Failed to drop database '{}': {}", dbname, e)) @@ -1526,7 +1616,7 @@ pub async fn drop_custom_instance_database(db: &DB, dbname: &str) -> error::Resu r#"UPDATE global_settings SET value = value #- ARRAY['databases', $1] WHERE name = 'custom_instance_pg_databases'"#, dbname ) - .execute(db) + .execute(&mut *conn) .await?; Ok(()) @@ -1600,6 +1690,35 @@ pub async fn create_custom_instance_database( error::Error::internal_err(format!("Failed to create database '{}': {}", dbname, e)) })?; + // 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 { + match drop_unused_instance_database(db, dbname).await { + Ok(users) if !users.is_empty() => tracing::warn!( + "Kept '{dbname}' after failing to set it up: workspaces {} use it", + users.join(", ") + ), + Ok(_) => {} + Err(drop_err) => { + tracing::error!("Could not drop '{dbname}' after failing to set it up: {drop_err}") + } + } + return Err(e); + } + + // 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 { + tracing::warn!("Could not set CONNECT grants on instance database '{dbname}': {e}"); + } + + tracing::info!("Created custom instance database '{}'", dbname); + Ok(()) +} + +/// 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<()> { // 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 }; @@ -1634,15 +1753,6 @@ pub async fn create_custom_instance_database( ) .execute(db) .await?; - - // 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 { - tracing::warn!("Could not set CONNECT grants on instance database '{dbname}': {e}"); - } - - tracing::info!("Created custom instance database '{}'", dbname); Ok(()) } diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index 711b4dac29..1dd91e56bb 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -1304,6 +1304,12 @@ pub struct DataTable { /// nothing local for a fork admin to widen. #[serde(default, skip_serializing_if = "Option::is_none")] pub reference: Option, + /// Set on a *clone* — a terminal entry whose database was copied from the entry this names. The + /// copy holds that entry's rows, so who may connect as which role stays that entry's decision: + /// the clone carries no `permissions` of its own and is governed like a pointer, while + /// connecting to its own database. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub governed_by: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub forked_from: Option, /// Whether the SQL-migrations feature is opted in for this data table. @@ -1362,9 +1368,16 @@ pub const DATATABLE_TENANT_WILDCARD: &str = "*"; /// enough to survive a fork of a fork. const DATATABLE_REFERENCE_MAX_DEPTH: usize = 20; -/// Exactly one of `database` and `reference` must be set. Called wherever an entry is persisted, -/// so nothing downstream has to handle an entry that is both or neither. +/// Exactly one of `database` and `reference` must be set, and a clone's `governed_by` leaves no +/// `permissions` beside it. Called wherever an entry is persisted, so nothing downstream has to +/// handle an entry that is both or neither, or a clone with a decision of its own. pub fn validate_datatable_shape(name: &str, dt: &DataTable) -> Result<()> { + if dt.governed_by.is_some() && (dt.database.is_none() || dt.permissions.is_some()) { + return Err(Error::BadRequest(format!( + "Data table '{name}' is a clone, which owns a database and takes its roles from the \ + data table it was cloned from" + ))); + } match (&dt.database, &dt.reference) { (Some(_), None) | (None, Some(_)) => Ok(()), (Some(_), Some(_)) => Err(Error::BadRequest(format!( @@ -1452,23 +1465,28 @@ pub async fn read_datatable_entry(db: &DB, w_id: &str, name: &str) -> Result(datatable.clone())?) } -/// The terminal entry a reference chain lands on: the workspace that governs the data table, the -/// entry name there, and the entry itself. A terminal entry resolves to itself. +/// The terminal entry a reference chain lands on, and what governs it: the workspace and name of +/// the entry that owns the database, the entry itself, and — for a clone — the entry its +/// `governed_by` chain lands on. A terminal entry that is not a clone resolves to itself and +/// governs itself. /// -/// Every decision downstream — which database to connect to, whose `permissions` apply, whose -/// members tenants are evaluated against, who may administer it — is taken on this, never on the -/// entry the caller named. +/// Every decision downstream is taken on this, never on the entry the caller named. Which database +/// to connect to comes from `workspace_id` / `name` / `datatable.database`; whose `permissions` +/// apply (already in `datatable.permissions`), whose members tenants are evaluated against and who +/// may administer it come from [`GoverningDatatable::governing_workspace_id`]. /// /// Authorization: resolving deliberately crosses into the governing workspace, so it answers for a /// workspace the caller may not belong to and checks nothing itself. It is the input to the /// checks, not one of them: callers MUST pass what it returns to /// [`can_use_datatable_role_in_governing_workspace`] or [`ensure_datatable_admin_access`] before -/// acting on it, and MUST NOT return its `permissions` or `workspace_id` to a caller from +/// acting on it, and MUST NOT return its `permissions` or workspace ids to a caller from /// elsewhere without gating on the answer. pub struct GoverningDatatable { pub workspace_id: String, pub name: String, pub datatable: DataTable, + /// For a clone, the entry whose `permissions` govern it. `None` when the entry governs itself. + pub governor: Option, } impl GoverningDatatable { @@ -1480,6 +1498,13 @@ impl GoverningDatatable { .as_ref() .is_some_and(|d| d.resource_type == DataTableCatalogResourceType::Instance) } + + /// The workspace whose admins administer the data table and whose members its tenants are. + pub fn governing_workspace_id(&self) -> &str { + self.governor + .as_ref() + .map_or(&self.workspace_id, |g| &g.workspace_id) + } } pub async fn resolve_governing_datatable( @@ -1490,6 +1515,9 @@ pub async fn resolve_governing_datatable( let mut workspace_id = w_id.to_string(); let mut name = name.to_string(); let mut hops = 0; + // The clone a `governed_by` chain started from: it keeps its database, and takes the + // `permissions` of wherever the chain lands. + let mut clone: Option<(String, String, DataTable)> = None; for _ in 0..DATATABLE_REFERENCE_MAX_DEPTH { let datatable = read_datatable_entry(db, &workspace_id, &name) .await @@ -1500,21 +1528,48 @@ pub async fn resolve_governing_datatable( // A pointer outlives the workspace it names: deleting one only nulls the fork // lineage, it does not sweep the entries that pointed at it. Say which one is // gone rather than reporting a data table this workspace never had. - Error::NotFound(format!( - "Data table '{name}' of workspace '{workspace_id}' governs this one and no \ - longer exists. A superadmin can point this data table somewhere else." - )) + if clone.is_some() { + Error::NotFound(format!( + "Data table '{name}' of workspace '{workspace_id}', which this clone \ + takes its roles from, no longer exists, so nobody is let into the copy." + )) + } else { + Error::NotFound(format!( + "Data table '{name}' of workspace '{workspace_id}' governs this one and \ + no longer exists. A superadmin can point this data table somewhere \ + else." + )) + } } })?; hops += 1; validate_datatable_shape(&name, &datatable)?; - match &datatable.reference { - None => return Ok(GoverningDatatable { workspace_id, name, datatable }), - Some(reference) => { - workspace_id = reference.workspace_id.clone(); - name = reference.datatable.clone(); + let next = match (&datatable.reference, &datatable.governed_by) { + (Some(reference), _) => reference.clone(), + (None, Some(governed_by)) => { + let governed_by = governed_by.clone(); + if clone.is_none() { + clone = Some((workspace_id.clone(), name.clone(), datatable)); + } + governed_by } - } + (None, None) => { + return Ok(match clone { + None => GoverningDatatable { workspace_id, name, datatable, governor: None }, + Some((clone_w_id, clone_name, mut clone_datatable)) => { + clone_datatable.permissions = datatable.permissions; + GoverningDatatable { + workspace_id: clone_w_id, + name: clone_name, + datatable: clone_datatable, + governor: Some(DataTableReference { workspace_id, datatable: name }), + } + } + }); + } + }; + workspace_id = next.workspace_id; + name = next.datatable; } Err(Error::BadRequest(format!( "Data table '{name}' points at another data table through more than \ @@ -1554,17 +1609,20 @@ pub async fn resolve_workspace_governing_datatables( let mut entries = Entries::new(); let listed = load(db, &[w_id.to_string()], &mut entries).await?; - // (index into `listed`, workspace, entry name) still to be followed. - let mut cursors: Vec<(usize, String, String)> = listed + // (index into `listed`, workspace, entry name, the clone the chain started from) still to be + // followed. A clone keeps its own database and takes the `permissions` of wherever its + // `governed_by` chain lands, as the single resolution does. + type Clone = Option<(String, String, DataTable)>; + let mut cursors: Vec<(usize, String, String, Clone)> = listed .iter() .enumerate() - .map(|(i, name)| (i, w_id.to_string(), name.clone())) + .map(|(i, name)| (i, w_id.to_string(), name.clone(), None)) .collect(); let mut resolved: Vec<(usize, GoverningDatatable)> = vec![]; for _ in 0..DATATABLE_REFERENCE_MAX_DEPTH { let mut next = vec![]; - for (i, ws, name) in cursors.drain(..) { + for (i, ws, name, clone) in cursors.drain(..) { let Some(value) = entries .get(&ws) .and_then(|m| m.get(&name)) @@ -1578,14 +1636,41 @@ pub async fn resolve_workspace_governing_datatables( if validate_datatable_shape(&name, &datatable).is_err() { continue; } - match &datatable.reference { - None => { - resolved.push((i, GoverningDatatable { workspace_id: ws, name, datatable })) - } - Some(reference) => next.push(( + match (&datatable.reference, &datatable.governed_by) { + (Some(reference), _) => next.push(( i, reference.workspace_id.clone(), reference.datatable.clone(), + clone, + )), + (None, Some(governed_by)) => { + 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, + }, + Some((clone_ws, clone_name, mut clone_datatable)) => { + clone_datatable.permissions = datatable.permissions; + GoverningDatatable { + workspace_id: clone_ws, + name: clone_name, + datatable: clone_datatable, + governor: Some(DataTableReference { + workspace_id: ws, + datatable: name, + }), + } + } + }, )), } } @@ -1594,7 +1679,7 @@ pub async fn resolve_workspace_governing_datatables( } let to_load: Vec = next .iter() - .map(|(_, ws, _)| ws.clone()) + .map(|(_, ws, _, _)| ws.clone()) .filter(|ws| !entries.contains_key(ws)) .collect::>() .into_iter() @@ -3344,6 +3429,7 @@ mod tests { resource_path: "dt_main".to_string(), }), reference: None, + governed_by: None, forked_from: None, migrations_enabled: None, permissions: None, diff --git a/cli/src/commands/workspace/fork.ts b/cli/src/commands/workspace/fork.ts index 219252bb7a..0aca0984ff 100644 --- a/cli/src/commands/workspace/fork.ts +++ b/cli/src/commands/workspace/fork.ts @@ -239,6 +239,7 @@ async function createWorkspaceFork( interface ForkedDatatableInfo { name: string; new_dbname: string; + fork_behavior?: "schema_only" | "schema_and_data"; } const forkedDatatables: ForkedDatatableInfo[] = []; @@ -285,6 +286,23 @@ async function createWorkspaceFork( const newDbName = `${trueWorkspaceId.replace(/-/g, "_")}__${dt.name}`; + const forkBehavior = dtBehavior as "schema_only" | "schema_and_data"; + // A server that reports `permissioned` copies each data table in the fork request itself, + // and refuses a database copied beforehand. An older one only takes a database copied here. + if (typeof dt.permissioned === "boolean") { + log.info( + colors.blue( + ` Datatable "${dt.name}" will be cloned (${forkBehavior === "schema_only" ? "schema" : "schema + data"}) into "${newDbName}" when the fork is created.` + ) + ); + forkedDatatables.push({ + name: dt.name, + new_dbname: newDbName, + fork_behavior: forkBehavior, + }); + continue; + } + try { log.info( colors.blue(` Creating database "${newDbName}" for datatable "${dt.name}"...`) @@ -300,7 +318,7 @@ async function createWorkspaceFork( log.info( colors.blue( - ` Importing ${dtBehavior === "schema_only" ? "schema" : "schema + data"}...` + ` Importing ${forkBehavior === "schema_only" ? "schema" : "schema + data"}...` ) ); @@ -310,7 +328,7 @@ async function createWorkspaceFork( source: `datatable://${dt.name}`, target: `datatable://${dt.name}`, target_dbname_override: newDbName, - fork_behavior: dtBehavior as "schema_only" | "schema_and_data", + fork_behavior: forkBehavior, }, }); @@ -336,6 +354,8 @@ async function createWorkspaceFork( id: trueWorkspaceId, name: opts.createWorkspaceName ?? workspaceName ?? trueWorkspaceId, color: forkColor, + // So a clone the fork would refuse is refused before any branch is created. + forked_datatables: forkedDatatables, }, }); if (gitSyncJobIds && gitSyncJobIds.length > 0) { diff --git a/frontend/src/lib/components/InstanceSettings.svelte b/frontend/src/lib/components/InstanceSettings.svelte index abe37c3537..d87de125a9 100644 --- a/frontend/src/lib/components/InstanceSettings.svelte +++ b/frontend/src/lib/components/InstanceSettings.svelte @@ -1094,9 +1094,10 @@ the home page’s create menu and hub-project picker are opened and from which entry point, the name of any public hub project imported from the home page and how far that import got, whether a pre-approved trial offer was opened, whether data tables are put - under roles and whether callers name a role or take the default, and which kinds of - access change (grant, revoke, ownership, default privileges) are applied to data - tables, last 30 days)
  • feature adoption (counts of which flow, script, trigger, worker and data table @@ -1163,9 +1164,10 @@ the home page’s create menu and hub-project picker are opened and from which entry point, the name of any public hub project imported from the home page and how far that import got, whether a pre-approved trial offer was opened, whether data tables are put - under roles and whether callers name a role or take the default, and which kinds of - access change (grant, revoke, ownership, default privileges) are applied to data - tables, last 30 days)
  • feature adoption (counts of which flow, script, trigger, worker and data table diff --git a/frontend/src/lib/components/datatableAcl/PgAclEditor.svelte b/frontend/src/lib/components/datatableAcl/PgAclEditor.svelte index e0ae596a3d..2a951cc260 100644 --- a/frontend/src/lib/components/datatableAcl/PgAclEditor.svelte +++ b/frontend/src/lib/components/datatableAcl/PgAclEditor.svelte @@ -120,7 +120,12 @@ Loading… {:else}
    - {#if !info.editable} + {#if info.clone} + + Read only: this data table is a clone, and its owners and grants stay as they were copied + from the data table it was cloned from. + + {:else if !info.editable} Read only: access is changed by the admins of the workspace that governs this data table, on Windmill Enterprise Edition. diff --git a/frontend/src/lib/components/workspaceSettings/CreateWorkspaceInner.svelte b/frontend/src/lib/components/workspaceSettings/CreateWorkspaceInner.svelte index 22a2fc889b..963eef3699 100644 --- a/frontend/src/lib/components/workspaceSettings/CreateWorkspaceInner.svelte +++ b/frontend/src/lib/components/workspaceSettings/CreateWorkspaceInner.svelte @@ -367,7 +367,7 @@ failedSyncJobs = [] forkCreationError = '' - // Clone datatables BEFORE creating the workspace fork + // Each data table copy is confirmed first; the fork request makes the copies if (forkDatatableSection) { const queue = forkDatatableSection.buildCloneQueue(prefixed_id) if (queue.length > 0) { @@ -383,6 +383,15 @@ } async function completeFork(prefixed_id: string): Promise { + // The fork request makes these copies, and drops them if the fork is not created + const forkedDatatables = forkDatatableSection + ? forkDatatableSection.getConfirmedCloneJobs().map((job) => ({ + name: job.name, + new_dbname: job._newDbName, + fork_behavior: job.behavior + })) + : [] + let gitSyncJobIds: string[] try { gitSyncJobIds = await WorkspaceService.createWorkspaceForkGitBranch({ @@ -391,6 +400,9 @@ id: prefixed_id, name, color: colorEnabled && workspaceColor ? workspaceColor : undefined, + // Sent in this first phase too, so a clone the fork would refuse is refused before + // any branch is created. + forked_datatables: forkedDatatables, is_dev_workspace: createAsDevWorkspace, dev_workspace_label: createAsDevWorkspace ? devWorkspaceLabel : undefined, // Send the lock intent in this first phase too so the backend can reject a non-admin's @@ -444,14 +456,6 @@ return } - // Build forked_datatables info from completed clone jobs - const forkedDatatables = forkDatatableSection - ? forkDatatableSection.getCompletedCloneJobs().map((job) => ({ - name: job.name, - new_dbname: job._newDbName - })) - : [] - try { await WorkspaceService.createWorkspaceFork({ workspace: baseWorkspaceId!, diff --git a/frontend/src/lib/components/workspaceSettings/DataTablePermissionsButton.svelte b/frontend/src/lib/components/workspaceSettings/DataTablePermissionsButton.svelte index c5c4171150..a9c53791d5 100644 --- a/frontend/src/lib/components/workspaceSettings/DataTablePermissionsButton.svelte +++ b/frontend/src/lib/components/workspaceSettings/DataTablePermissionsButton.svelte @@ -196,6 +196,12 @@ data table backed by that database can use one. This one is backed by a PostgreSQL resource — grant access on that server directly. + {:else if info?.clone_of} + + This data table is a clone of {info.clone_of.datatable} + in workspace {info.clone_of.workspace_id}, so it takes + its roles from there. You are evaluated as a member of that workspace. + {:else if governing} This data table points at the one in workspace {governing} {#if permissioned} - {#if availableRoles.length === 0} + + {#if editable && availableRoles.length === 0} Only admin can be used until a superadmin adds a data table role, from Instance roles at the top of the data tables settings page. diff --git a/frontend/src/lib/components/workspaceSettings/ForkDatatableSection.svelte b/frontend/src/lib/components/workspaceSettings/ForkDatatableSection.svelte index c060c1d1ed..8af5c4cb9e 100644 --- a/frontend/src/lib/components/workspaceSettings/ForkDatatableSection.svelte +++ b/frontend/src/lib/components/workspaceSettings/ForkDatatableSection.svelte @@ -1,20 +1,9 @@ @@ -27,7 +16,6 @@ import Label from '../Label.svelte' import Alert from '../common/alert/Alert.svelte' import ConfirmationModal from '../common/confirmationModal/ConfirmationModal.svelte' - import { Check, X, Loader2 } from 'lucide-svelte' interface Props { // Workspace whose datatables are cloned into the fork (the fork's base). Falls back to the @@ -52,111 +40,46 @@ let cloneModalOpen = $state(false) let currentCloneJob: DatatableCloneJob | undefined = $state(undefined) let cloneQueue: DatatableCloneJob[] = $state([]) - let cloneRunning = $state(false) export function hasDatatables(): boolean { return (allDatatables.current?.length ?? 0) > 0 } export function buildCloneQueue(targetWorkspaceId: string): DatatableCloneJob[] { + // A fork attempt starts here: what an earlier attempt confirmed is not this one's to send. + confirmedJobs = [] return (allDatatables.current ?? []) .filter((dt) => { const behavior = datatableBehaviors[dt.name] ?? 'keep_original' return behavior !== 'keep_original' }) - .map((dt) => { - const behavior = datatableBehaviors[dt.name] as 'schema_only' | 'schema_and_data' - const isInstance = dt.resource_type === 'instance' - const newDbName = `${targetWorkspaceId.replace(/-/g, '_')}__${dt.name}` - - const steps: ForkStep[] = [ - { - label: `CREATE DATABASE "${newDbName}"`, - status: 'pending' - }, - { - label: `pg_dump → pg_import (${behavior === 'schema_only' ? 'schema only' : 'schema + data'})`, - status: 'pending' - } - ] - - return { - name: dt.name, - resourceType: dt.resource_type, - behavior, - steps, - _newDbName: newDbName, - _isInstance: isInstance, - _sourceWorkspace: effectiveSource!, - _targetWorkspace: targetWorkspaceId, - _resourcePath: dt.resource_path - } - }) + .map((dt) => ({ + name: dt.name, + resourceType: dt.resource_type, + behavior: datatableBehaviors[dt.name] as 'schema_only' | 'schema_and_data', + // A dev workspace's id has no `wm-fork-` prefix; its copies are named like a fork's + _newDbName: `wm_fork_${targetWorkspaceId.replace(/^wm-fork-/, '').replace(/-/g, '_')}__${dt.name}` + })) } - let completedJobs: DatatableCloneJob[] = $state([]) + let confirmedJobs: DatatableCloneJob[] = $state([]) + // Each clone is only confirmed here: the fork request makes the copies, and drops them if the + // fork is not created. export function startCloning(queue: DatatableCloneJob[]) { - completedJobs = [] + confirmedJobs = [] cloneQueue = queue currentCloneJob = cloneQueue[0] cloneModalOpen = true } - export function getCompletedCloneJobs(): DatatableCloneJob[] { - return completedJobs - } - - async function executeCloneJob(job: DatatableCloneJob) { - cloneRunning = true - let stepIdx = 0 - - // Step 1: Create the database - job.steps[stepIdx].status = 'running' - try { - await WorkspaceService.createPgDatabase({ - workspace: job._sourceWorkspace, - requestBody: { - source: `datatable://${job.name}`, - target_dbname: job._newDbName - } - }) - job.steps[stepIdx].status = 'done' - } catch (e: any) { - job.steps[stepIdx].status = 'error' - job.steps[stepIdx].error = e?.body ?? e?.message ?? String(e) - cloneRunning = false - return - } - stepIdx++ - - // Step 2: Import data - job.steps[stepIdx].status = 'running' - try { - await WorkspaceService.importPgDatabase({ - workspace: job._sourceWorkspace, - requestBody: { - source: `datatable://${job.name}`, - target: `datatable://${job.name}`, - target_dbname_override: job._newDbName, - fork_behavior: job.behavior - } - }) - job.steps[stepIdx].status = 'done' - } catch (e: any) { - job.steps[stepIdx].status = 'error' - job.steps[stepIdx].error = e?.body ?? e?.message ?? String(e) - cloneRunning = false - return - } - stepIdx++ - - cloneRunning = false + export function getConfirmedCloneJobs(): DatatableCloneJob[] { + return confirmedJobs } function advanceCloneQueue() { if (currentCloneJob) { - completedJobs.push(currentCloneJob) + confirmedJobs.push(currentCloneJob) } const idx = cloneQueue.indexOf(currentCloneJob!) if (idx < cloneQueue.length - 1) { @@ -205,13 +128,9 @@ {#if cloneModalOpen && currentCloneJob} { - await executeCloneJob(currentCloneJob!) - advanceCloneQueue() - }} + onConfirmed={advanceCloneQueue} onCanceled={() => { cloneModalOpen = false currentCloneJob = undefined @@ -233,39 +152,16 @@ {#if currentCloneJob.resourceType === 'instance'}

    - This will run CREATE DATABASE {currentCloneJob.steps[0]?.label.match(/"([^"]+)"/)?.[1] ?? ''} on the Windmill PostgreSQL instance. + Creating the fork will run CREATE DATABASE {currentCloneJob._newDbName} on the Windmill + PostgreSQL instance. A data table under roles keeps its owners and grants in the copy, and its + roles stay decided where they are decided today.

    {:else}

    - This will run CREATE DATABASE on the resource's PostgreSQL server. + Creating the fork will run CREATE DATABASE {currentCloneJob._newDbName} on the resource's + PostgreSQL server.

    {/if} - -
    - {#each currentCloneJob.steps as step} -
    - {#if step.status === 'done'} - - {:else if step.status === 'running'} - - {:else if step.status === 'error'} - - {:else} -
    - {/if} - - {step.label} - -
    - {#if step.error} -

    {step.error}

    - {/if} - {/each} -
    +

    If the fork cannot be created, the copy is dropped.

    {/if}