diff --git a/backend/.sqlx/query-71ee2cb6661cca1fa4d8874a7f6d368347c59f36fd87df6dc7996152ccb84af0.json b/backend/.sqlx/query-71ee2cb6661cca1fa4d8874a7f6d368347c59f36fd87df6dc7996152ccb84af0.json deleted file mode 100644 index a03fab7081..0000000000 --- a/backend/.sqlx/query-71ee2cb6661cca1fa4d8874a7f6d368347c59f36fd87df6dc7996152ccb84af0.json +++ /dev/null @@ -1,38 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT id, name, enabled, pwd FROM datatable_role", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "id", - "type_info": "Varchar" - }, - { - "ordinal": 1, - "name": "name", - "type_info": "Varchar" - }, - { - "ordinal": 2, - "name": "enabled", - "type_info": "Bool" - }, - { - "ordinal": 3, - "name": "pwd", - "type_info": "Text" - } - ], - "parameters": { - "Left": [] - }, - "nullable": [ - false, - false, - false, - true - ] - }, - "hash": "71ee2cb6661cca1fa4d8874a7f6d368347c59f36fd87df6dc7996152ccb84af0" -} diff --git a/backend/.sqlx/query-79799b5a2e499df6c28e286c42b9ad2db940c2455ab19cc95e5198baf96d5629.json b/backend/.sqlx/query-79799b5a2e499df6c28e286c42b9ad2db940c2455ab19cc95e5198baf96d5629.json deleted file mode 100644 index c36b6fb361..0000000000 --- a/backend/.sqlx/query-79799b5a2e499df6c28e286c42b9ad2db940c2455ab19cc95e5198baf96d5629.json +++ /dev/null @@ -1,29 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "\n SELECT ws.workspace_id AS \"workspace_id!\", dt.key AS \"datatable!\"\n FROM workspace_settings ws\n JOIN workspace w ON w.id = ws.workspace_id AND w.deleted = false\n CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt\n WHERE ws.workspace_id <> $1\n AND dt.value->'database'->>'resource_type' = 'instance'\n AND dt.value->'database'->>'resource_path' = $2\n ORDER BY ws.workspace_id, dt.key\n ", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "workspace_id!", - "type_info": "Varchar" - }, - { - "ordinal": 1, - "name": "datatable!", - "type_info": "Text" - } - ], - "parameters": { - "Left": [ - "Text", - "Text" - ] - }, - "nullable": [ - false, - null - ] - }, - "hash": "79799b5a2e499df6c28e286c42b9ad2db940c2455ab19cc95e5198baf96d5629" -} diff --git a/backend/.sqlx/query-86af9d51a158ea5cb6161461ecddf2a63695f8cbf8af648da5a0a77a5b9d02ba.json b/backend/.sqlx/query-86af9d51a158ea5cb6161461ecddf2a63695f8cbf8af648da5a0a77a5b9d02ba.json deleted file mode 100644 index 7627d9828d..0000000000 --- a/backend/.sqlx/query-86af9d51a158ea5cb6161461ecddf2a63695f8cbf8af648da5a0a77a5b9d02ba.json +++ /dev/null @@ -1,17 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "INSERT INTO datatable_role (id, name, enabled, pwd) VALUES ($1, $2, $3, $4)", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Varchar", - "Varchar", - "Bool", - "Text" - ] - }, - "nullable": [] - }, - "hash": "86af9d51a158ea5cb6161461ecddf2a63695f8cbf8af648da5a0a77a5b9d02ba" -} diff --git a/backend/.sqlx/query-b9842d2d8abf382bd82d8fa1de012373638be391f884f81dc387ffc465badac6.json b/backend/.sqlx/query-b9842d2d8abf382bd82d8fa1de012373638be391f884f81dc387ffc465badac6.json deleted file mode 100644 index 4b3a2f33e9..0000000000 --- a/backend/.sqlx/query-b9842d2d8abf382bd82d8fa1de012373638be391f884f81dc387ffc465badac6.json +++ /dev/null @@ -1,20 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT jsonb_object_keys(value->'databases') FROM global_settings\n WHERE name = 'custom_instance_pg_databases'", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "jsonb_object_keys", - "type_info": "Text" - } - ], - "parameters": { - "Left": [] - }, - "nullable": [ - null - ] - }, - "hash": "b9842d2d8abf382bd82d8fa1de012373638be391f884f81dc387ffc465badac6" -} diff --git a/backend/.sqlx/query-d48ca62c86b1af7a9dd2450c1c28dc45020a2a553d8874c49f9eafedea5a9d40.json b/backend/.sqlx/query-d48ca62c86b1af7a9dd2450c1c28dc45020a2a553d8874c49f9eafedea5a9d40.json deleted file mode 100644 index 49875acd78..0000000000 --- a/backend/.sqlx/query-d48ca62c86b1af7a9dd2450c1c28dc45020a2a553d8874c49f9eafedea5a9d40.json +++ /dev/null @@ -1,24 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT dt.key AS \"datatable!\"\n FROM workspace_settings ws\n CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt\n WHERE ws.workspace_id = $1\n AND dt.key <> $2\n AND NOT dt.value ? 'permissions'\n AND dt.value->'database'->>'resource_type' = 'instance'\n AND dt.value->'database'->>'resource_path' = $3\n ORDER BY dt.key", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "datatable!", - "type_info": "Text" - } - ], - "parameters": { - "Left": [ - "Text", - "Text", - "Text" - ] - }, - "nullable": [ - null - ] - }, - "hash": "d48ca62c86b1af7a9dd2450c1c28dc45020a2a553d8874c49f9eafedea5a9d40" -} diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 00e192fc36..5a9ef2b01c 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -63cf1cf4bf643209eeea8540fc1542734b2474da +8efda07e42b4154e50c6340eaf7f1be7d2a0f827 \ No newline at end of file diff --git a/backend/migrations/20260917133824_datatable_role_cluster.down.sql b/backend/migrations/20260917133824_datatable_role_cluster.down.sql new file mode 100644 index 0000000000..2fb7bcc8a8 --- /dev/null +++ b/backend/migrations/20260917133824_datatable_role_cluster.down.sql @@ -0,0 +1,11 @@ +-- Roles on the external cluster are live logins there; dropping the column would forget them. +LOCK TABLE datatable_role; +DO $$ +BEGIN + IF EXISTS (SELECT 1 FROM datatable_role WHERE cluster <> 'instance') THEN + RAISE EXCEPTION 'datatable_role holds roles on the external instance cluster. Delete them in instance settings first.'; + END IF; +END $$; +ALTER TABLE datatable_role DROP CONSTRAINT datatable_role_cluster_name_key; +ALTER TABLE datatable_role ADD CONSTRAINT datatable_role_name_key UNIQUE (name); +ALTER TABLE datatable_role DROP COLUMN cluster; diff --git a/backend/migrations/20260917133824_datatable_role_cluster.up.sql b/backend/migrations/20260917133824_datatable_role_cluster.up.sql new file mode 100644 index 0000000000..d63f06f1f4 --- /dev/null +++ b/backend/migrations/20260917133824_datatable_role_cluster.up.sql @@ -0,0 +1,8 @@ +-- A data table role is a Postgres login on one cluster: Windmill's own ('instance'), or the external +-- instance cluster ('external_instance'). Role names are the cluster's own key, so they are unique +-- per cluster rather than across the instance. +ALTER TABLE datatable_role + ADD COLUMN cluster VARCHAR(20) NOT NULL DEFAULT 'instance' + CHECK (cluster IN ('instance', 'external_instance')); +ALTER TABLE datatable_role DROP CONSTRAINT datatable_role_name_key; +ALTER TABLE datatable_role ADD CONSTRAINT datatable_role_cluster_name_key UNIQUE (cluster, name); diff --git a/backend/windmill-api-integration-tests/tests/datatable_roles.rs b/backend/windmill-api-integration-tests/tests/datatable_roles.rs index 10b273cbc7..e92508d917 100644 --- a/backend/windmill-api-integration-tests/tests/datatable_roles.rs +++ b/backend/windmill-api-integration-tests/tests/datatable_roles.rs @@ -391,7 +391,11 @@ async fn concurrent_role_creations_both_survive(db: Pool) -> anyhow::R assert_eq!(a.0, 200, "{}", a.1); assert_eq!(b.0, 200, "{}", b.1); - let catalog = windmill_common::datatable_roles::read_role_catalog(&db).await?; + let catalog = windmill_common::datatable_roles::read_role_catalog( + &db, + windmill_common::datatable_roles::DatatableRoleCluster::Instance, + ) + .await?; let recorded: Vec<&str> = catalog.values().map(|r| r.name.as_str()).collect(); for name in &names { assert!( @@ -460,7 +464,11 @@ async fn a_role_delete_that_fails_part_way_leaves_the_role_disabled( let body = resp.text().await?; assert_eq!(status, 400, "{body}"); - let catalog = windmill_common::datatable_roles::read_role_catalog(&db).await?; + let catalog = windmill_common::datatable_roles::read_role_catalog( + &db, + windmill_common::datatable_roles::DatatableRoleCluster::Instance, + ) + .await?; let role = catalog .get(&id) .expect("a failed delete keeps the entry to retry"); diff --git a/backend/windmill-api-workspaces/src/datatable_acl.rs b/backend/windmill-api-workspaces/src/datatable_acl.rs index 6bd14316fc..f5e1374689 100644 --- a/backend/windmill-api-workspaces/src/datatable_acl.rs +++ b/backend/windmill-api-workspaces/src/datatable_acl.rs @@ -6,7 +6,7 @@ * LICENSE-AGPL for a copy of the license. */ -//! Ownership and grants on the objects of an instance data table. +//! Ownership and grants on the objects of a data table on a cluster Windmill manages. //! //! [`datatable_permissions`](crate::datatable_permissions) decides who may connect as which role; //! this decides what each role may then touch. Every change is a real `GRANT`, `REVOKE`, @@ -33,7 +33,7 @@ use windmill_audit::audit_oss::audit_log; use windmill_audit::ActionKind; use windmill_common::datatable_roles::{ lock_role_catalog, quote_ident, read_role_catalog, read_role_catalog_tx, DatatableRoleCatalog, - ADMIN_DATATABLE_ROLE, CUSTOM_INSTANCE_USER, + DatatableRoleCluster, ADMIN_DATATABLE_ROLE, CUSTOM_INSTANCE_USER, }; use windmill_common::error::{pg_error_message, Error, JsonResult, Result}; use windmill_common::workspaces::{resolve_governing_datatable, DataTable, GoverningDatatable}; @@ -295,22 +295,22 @@ fn role_names(catalog: &DatatableRoleCatalog) -> Vec { names } -fn ensure_instance(governing: &GoverningDatatable) -> Result<()> { - if governing.is_instance() { - return Ok(()); - } - Err(Error::BadRequest(format!( - "Data table '{}' is backed by a Postgres resource, so its access is managed on that \ - server directly. Only a data table on the Windmill instance's own database has data \ - table roles to grant to.", - governing.name - ))) +/// The cluster whose roles the data table's grants name. +fn ensure_managed(governing: &GoverningDatatable) -> Result { + governing.role_cluster().ok_or_else(|| { + Error::BadRequest(format!( + "Data table '{}' is backed by a Postgres resource, so its access is managed on that \ + server directly. Only a data table on a database Windmill manages has data table \ + roles to grant to.", + governing.name + )) + }) } /// The data table's `admin` connection, and the notices Postgres sends on it. /// -/// Authorization: connects as `custom_instance_user` with the instance's own credentials and checks -/// nothing. Callers MUST have authorized the request first — a request about to be refused must +/// Authorization: connects as `custom_instance_user` with the cluster's stored credentials and +/// checks nothing. Callers MUST have authorized the request first — a request about to be refused must /// not get as far as this connection. async fn connect_as_admin_unchecked( db: &DB, @@ -320,21 +320,33 @@ async fn connect_as_admin_unchecked( mpsc::UnboundedReceiver, String, )> { - ensure_instance(governing)?; + let cluster = ensure_managed(governing)?; // Built from the authorized entry, never by resolving the settings again: a save in between // could point the entry at a resource on another server and back, and this connection would // then alter a database the later checks of the entry never see. - let mut pg = PgDatabase::parse_uri(&windmill_common::get_database_url().await?.as_str().await)?; - pg.dbname = governing + let dbname = governing .datatable .database .as_ref() .expect("a governing entry owns a database") .resource_path .clone(); - pg.user = Some(CUSTOM_INSTANCE_USER.to_string()); - pg.password = Some(windmill_common::utils::get_custom_pg_instance_password(db).await?); - let dbname = pg.dbname.clone(); + let pg = match cluster { + DatatableRoleCluster::Instance => { + let mut pg = + PgDatabase::parse_uri(&windmill_common::get_database_url().await?.as_str().await)?; + pg.dbname = dbname.clone(); + pg.user = Some(CUSTOM_INSTANCE_USER.to_string()); + pg.password = Some(windmill_common::utils::get_custom_pg_instance_password(db).await?); + pg + } + DatatableRoleCluster::ExternalInstance => { + windmill_common::external_instance_pg::external_instance_connection_unchecked( + db, &dbname, false, + ) + .await? + } + }; let (client, mut connection) = pg.connect(Some(db)).await?; // Unbounded: the driver must never wait on the receiver, which only drains once the statement // the driver is carrying has completed. @@ -1020,12 +1032,12 @@ async fn get_datatable_acl( let target: AclTarget = query.try_into()?; let governing = resolve_governing_datatable(&db, &w_id, &datatable_name).await?; ensure_reaches_governing_datatable(&db, &w_id, &datatable_name, &governing, &authed).await?; - ensure_instance(&governing)?; + let cluster = ensure_managed(&governing)?; let editable = ensure_governs_datatable(&db, &authed, &w_id, &governing) .await .is_ok(); let roles = if editable { - role_names(&read_role_catalog(&db).await?) + role_names(&read_role_catalog(&db, cluster).await?) } else { vec![] }; @@ -1310,7 +1322,7 @@ async fn authorize_acl_change( ) -> Result { let governing = resolve_governing_datatable(db, w_id, datatable_name).await?; ensure_governs_datatable(db, authed, w_id, &governing).await?; - ensure_instance(&governing)?; + ensure_managed(&governing)?; Ok(governing) } @@ -1320,8 +1332,14 @@ static APPLY_SLOT: tokio::sync::Semaphore = tokio::sync::Semaphore::const_new(1) /// provisioned before data table roles gave `custom_instance_user` none. Adds that option to its /// database and `public` privileges, and nothing else: default privileges are left alone, since a /// schema's change of owner is planned against them. Best-effort, as a grant it fails to enable is -/// refused when it runs. -async fn ensure_grant_options(client: &tokio_postgres::Client, db: &DB, dbname: &str) { +/// refused when it runs. An external instance database was created with the options, so one +/// missing there is someone's deliberate revoke and is left alone. +async fn ensure_grant_options( + client: &tokio_postgres::Client, + db: &DB, + cluster: DatatableRoleCluster, + dbname: &str, +) { let held = client .query_one( "SELECT has_database_privilege(current_database(), 'CONNECT WITH GRANT OPTION') @@ -1333,7 +1351,7 @@ async fn ensure_grant_options(client: &tokio_postgres::Client, db: &DB, dbname: ) .await .is_ok_and(|row| row.get::<_, bool>(0)); - if held { + if held || cluster != DatatableRoleCluster::Instance { return; } if let Err(e) = grant_options_as_server(db, dbname).await { @@ -1559,7 +1577,8 @@ async fn plan_datatable_acl( ) -> JsonResult { crate::datatable_acl_oss::ensure_datatable_acl_available()?; let governing = authorize_acl_change(&db, &authed, &w_id, &datatable_name).await?; - let catalog = read_role_catalog(&db).await?; + let cluster = ensure_managed(&governing)?; + let catalog = read_role_catalog(&db, cluster).await?; let (client, _notices, dbname) = connect_as_admin_unchecked(&db, &governing).await?; Ok(Json( build_plan(&client, &dbname, &catalog, &req.target, &req.change).await?, @@ -1583,6 +1602,7 @@ async fn apply_datatable_acl( // connection could wait forever on a pool that concurrent applies, queued on the same locks, // have exhausted. let governing = authorize_acl_change(&db, &authed, &w_id, &datatable_name).await?; + let cluster = ensure_managed(&governing)?; // Applies queue on an instance-wide lock while each holds a direct connection to the instance's // Postgres; unbounded, the queue alone could exhaust its connection limit. One at a time per // server, and the ones waiting hold no connection at all. @@ -1591,7 +1611,7 @@ async fn apply_datatable_acl( .await .map_err(|e| Error::internal_err(format!("ACL apply slot closed: {e}")))?; let (mut client, mut notices, dbname) = connect_as_admin_unchecked(&db, &governing).await?; - ensure_grant_options(&client, &db, &dbname).await; + ensure_grant_options(&client, &db, cluster, &dbname).await; // Held until the change is committed: a role renamed or dropped meanwhile would change what // the plan names, and a settings save could move the entry onto another database. Taken in the @@ -1606,7 +1626,7 @@ async fn apply_datatable_acl( .fetch_optional(&mut *tx) .await? .flatten(); - let catalog = read_role_catalog_tx(&mut tx).await?; + let catalog = read_role_catalog_tx(&mut tx, cluster).await?; let plan = build_plan(&client, &dbname, &catalog, &req.target, &req.change).await?; if !entry_unchanged(&governing, entry_now) || &plan.statements != confirmed { diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index a9c3775222..cabd362cac 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -2265,7 +2265,8 @@ struct DataTableTables { schemas: TableListMap, #[serde(skip_serializing_if = "Option::is_none")] error: Option, - /// On the instance database: the only kind that can be under roles or have its access edited. + /// On a database Windmill manages, on its own cluster or the external one: the only kinds that + /// can be under roles or have their access edited. instance: bool, permissioned: bool, /// The roles this caller may connect as, by name; empty when not under roles. @@ -2522,7 +2523,7 @@ async fn list_one_datatable_tables( }; let result: Result<()> = async { let governing = resolve_governing_datatable(db, w_id, &entry.datatable_name).await?; - entry.instance = governing.is_instance(); + entry.instance = governing.role_cluster().is_some(); let usable = crate::datatable_permissions_oss::usable_datatable_roles(db, authed, w_id, &governing) .await?; @@ -4052,18 +4053,20 @@ async fn edit_datatable_config( None => None, }; // Carrying the block onto a resource-backed entry would produce a data table the chokepoint - // refuses on every job — a save that succeeds and breaks everything afterwards. Refuse it - // instead: turning roles off first is one step, and it keeps discarding an access decision - // something somebody chose rather than a side effect of moving a database. + // refuses on every job — a save that succeeds and breaks everything afterwards — and onto + // the other managed cluster, one whose role ids name nothing in that cluster's catalog. + // Refuse it instead: turning roles off first is one step, and it keeps discarding an access + // decision something somebody chose rather than a side effect of moving a database. + let old_kind = old.and_then(|old| old.database.as_ref()).map(|d| d.resource_type); if dt.permissions.is_some() && dt .database .as_ref() - .is_some_and(|d| d.resource_type != DataTableCatalogResourceType::Instance) + .is_some_and(|d| Some(d.resource_type) != old_kind) { return Err(Error::BadRequest(format!( - "Data table '{name}' is under roles, which only a data table on the instance \ - database can be. Turn its roles off before moving it to a PostgreSQL resource." + "Data table '{name}' is under roles, which belong to the cluster its database is \ + on. Turn its roles off before moving it to another kind of database." ))); } // A pointer names no database of its own, so the form's empty `database` is correct there. @@ -4140,7 +4143,7 @@ async fn edit_datatable_config( // entry through a declared rename alone, and a settings sync never declares one, so an entry // without roles that newly points at such a database — a name added, or an existing one // repointed — would answer everyone there as `admin`. That holds whichever workspace governs it. - let newly_pointed: Vec<(&String, &str)> = new_config + let newly_pointed: Vec<(&String, DataTableCatalogResourceType, &str)> = new_config .settings .datatables .iter() @@ -4149,7 +4152,7 @@ async fn edit_datatable_config( let db = dt .database .as_ref() - .filter(|d| d.resource_type == DataTableCatalogResourceType::Instance)?; + .filter(|d| d.resource_type.is_windmill_managed())?; let lookup = rename_src .get(name.as_str()) .copied() @@ -4161,38 +4164,44 @@ async fn edit_datatable_config( old_db.resource_type != db.resource_type || old_db.resource_path != db.resource_path }); - repointed.then_some((name, db.resource_path.as_str())) + repointed.then_some((name, db.resource_type, db.resource_path.as_str())) }) .collect(); // Another workspace turning roles on for the same database holds only its own settings row, so // without this the scan below could read past its uncommitted write. windmill_common::datatable_roles::lock_instance_databases_governance( &mut *tx, - newly_pointed.iter().map(|(_, dbname)| *dbname), + newly_pointed.iter().map(|(_, _, dbname)| *dbname), ) .await?; - let governed_elsewhere: Vec = if newly_pointed.is_empty() { + let governed_elsewhere: Vec<(String, String)> = if newly_pointed.is_empty() { vec![] } else { - sqlx::query_scalar( - "SELECT DISTINCT dt.value->'database'->>'resource_path' FROM workspace_settings ws + sqlx::query_as( + "SELECT DISTINCT dt.value->'database'->>'resource_type', + dt.value->'database'->>'resource_path' + FROM workspace_settings ws CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt WHERE ws.workspace_id <> $1 AND dt.value ? 'permissions' - AND dt.value->'database'->>'resource_type' = 'instance'", + AND dt.value->'database'->>'resource_type' IN ('instance', 'external_instance')", ) .bind(&w_id) .fetch_all(&mut *tx) .await? }; - for (name, dbname) in newly_pointed { + for (name, kind, dbname) in newly_pointed { let governed_here = old_datatables.values().any(|old| { old.permissions.is_some() - && old.database.as_ref().is_some_and(|d| { - d.resource_type == DataTableCatalogResourceType::Instance - && d.resource_path == dbname - }) + && old + .database + .as_ref() + .is_some_and(|d| d.resource_type == kind && d.resource_path == dbname) }); - if governed_here || governed_elsewhere.iter().any(|g| g == dbname) { + if governed_here + || governed_elsewhere + .iter() + .any(|(k, p)| k == kind.as_ref() && p == dbname) + { return Err(Error::BadRequest(format!( "Data table '{name}' would point at database '{dbname}', which a data table under \ roles uses, without carrying those roles: everyone reaching '{name}' would connect \ diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 371785c404..8e27601115 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -1688,13 +1688,19 @@ paths: /settings/datatable_roles: get: - summary: list the instance's data table roles + summary: list the data table roles of one Windmill-managed Postgres cluster operationId: listInstanceDatatableRoles tags: - setting + parameters: + - in: query + name: cluster + required: false + schema: + $ref: "#/components/schemas/DatatableRoleCluster" responses: "200": - description: the instance role catalog + description: the cluster's role catalog content: application/json: schema: @@ -1702,7 +1708,7 @@ paths: items: $ref: "#/components/schemas/InstanceDatatableRole" post: - summary: create a data table role on the instance's Postgres cluster + summary: create a data table role on a Windmill-managed Postgres cluster operationId: createInstanceDatatableRole tags: - setting @@ -1716,6 +1722,8 @@ paths: properties: name: type: string + cluster: + $ref: "#/components/schemas/DatatableRoleCluster" responses: "200": description: the created role @@ -33757,9 +33765,17 @@ components: last_setup: $ref: "#/components/schemas/ExternalInstancePgSetupReport" + DatatableRoleCluster: + type: string + description: >- + The Windmill-managed Postgres cluster a data table role is a login on: Windmill's own + (behind `instance` data tables) or the external instance cluster (behind + `external_instance` ones). Defaults to `instance`. + enum: [instance, external_instance] + InstanceDatatableRole: type: object - required: [id, name, enabled] + required: [id, name, enabled, cluster] properties: id: type: string @@ -33767,6 +33783,8 @@ components: type: string enabled: type: boolean + cluster: + $ref: "#/components/schemas/DatatableRoleCluster" DatatableRoleTenants: type: object @@ -33788,8 +33806,10 @@ components: supported: type: boolean description: >- - Whether this data table can be put under roles at all. Only one backed by the - instance database can: a role is a login on that cluster. + Whether this data table can be put under roles at all. Only one on a database Windmill + manages can: a role is a login on that database's cluster. + cluster: + $ref: "#/components/schemas/DatatableRoleCluster" permissioned: type: boolean default_role: diff --git a/backend/windmill-common/src/datatable_roles.rs b/backend/windmill-common/src/datatable_roles.rs index 4dd9b06fde..dea39ff1b9 100644 --- a/backend/windmill-common/src/datatable_roles.rs +++ b/backend/windmill-common/src/datatable_roles.rs @@ -6,22 +6,67 @@ * LICENSE-AGPL for a copy of the license. */ -//! The instance's data table role catalog. +//! The instance's data table role catalogs. //! -//! A data table role is a real Postgres login role on the Windmill cluster, named exactly as the -//! user named it, shared by every instance database. Windmill decides who may ask for a role (the -//! per-data-table tenant lists in [`crate::workspaces`]); Postgres decides what the role may then -//! touch. The catalog here is only the first half's vocabulary plus the cluster provisioning. +//! A data table role is a real Postgres login role on one cluster — Windmill's own, or the external +//! instance cluster — named exactly as the user named it, shared by every database Windmill manages +//! on that cluster. Each cluster has its own catalog: a role exists where it was created and nowhere +//! else. Windmill decides who may ask for a role (the per-data-table tenant lists in +//! [`crate::workspaces`]); Postgres decides what the role may then touch. The catalog here is only +//! the first half's vocabulary plus the cluster provisioning. //! //! Entries are keyed by a generated id so a rename moves nothing else: tenants name the id. use std::collections::BTreeMap; +use serde::{Deserialize, Serialize}; + use crate::{ error::{Error, Result}, + workspaces::DataTableCatalogResourceType, DB, }; +/// The cluster a role catalog belongs to. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum DatatableRoleCluster { + /// Windmill's own Postgres, behind `instance` data tables. + #[default] + Instance, + /// The external instance cluster, behind `external_instance` data tables. + ExternalInstance, +} + +impl DatatableRoleCluster { + pub fn as_str(self) -> &'static str { + match self { + Self::Instance => "instance", + Self::ExternalInstance => "external_instance", + } + } + + pub fn parse(value: &str) -> Result { + match value { + "instance" => Ok(Self::Instance), + "external_instance" => Ok(Self::ExternalInstance), + other => Err(Error::BadRequest(format!( + "Unknown data table role cluster '{other}': expected instance or external_instance" + ))), + } + } + + /// The cluster whose roles a data table on `kind` can use. `None` for a resource-backed one, + /// which is never under roles. + pub fn of(kind: DataTableCatalogResourceType) -> Option { + match kind { + DataTableCatalogResourceType::Instance => Some(Self::Instance), + DataTableCatalogResourceType::ExternalInstance => Some(Self::ExternalInstance), + DataTableCatalogResourceType::Postgresql => None, + } + } +} + /// The connection every data table resolved to before roles existed (`custom_instance_user`). It /// owns every pre-existing object, so it is a reserved name rather than a catalog entry: never /// created, renamed or dropped. @@ -164,28 +209,42 @@ pub async fn lock_instance_databases_governance<'a>( /// need the names — but callers MUST NOT let `pwd` reach a response, a log line, an audit record /// or an export. Nothing about who may call it: the credential is the whole risk, and `Debug` is /// hand-written to redact it for the same reason. -pub async fn read_role_catalog(db: &DB) -> Result { - crate::datatable_roles_oss::read_role_catalog(db).await +pub async fn read_role_catalog( + db: &DB, + cluster: DatatableRoleCluster, +) -> Result { + crate::datatable_roles_oss::read_role_catalog(db, cluster).await } /// As [`read_role_catalog`], reading inside the caller's transaction so the value is the one /// [`lock_role_catalog`] is protecting. Same disclosure contract. pub async fn read_role_catalog_tx( tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + cluster: DatatableRoleCluster, ) -> Result { - crate::datatable_roles_oss::read_role_catalog_tx(tx).await + crate::datatable_roles_oss::read_role_catalog_tx(tx, cluster).await } -/// Record a role, in the caller's transaction so it commits with the `CREATE ROLE` it describes. +/// The cluster a role belongs to, or `None` if no role has this id. +pub async fn role_cluster( + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + id: &str, +) -> Result> { + crate::datatable_roles_oss::role_cluster(tx, id).await +} + +/// Record a role, in the caller's transaction. On Windmill's own cluster that commits it with the +/// `CREATE ROLE` it describes; on the external cluster the role already exists by then. /// /// Authorization: writes a generated Postgres credential. Callers MUST restrict this to superadmin /// paths and MUST hold [`lock_role_catalog`] on `tx`. pub async fn insert_role_catalog_entry( tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, id: &str, + cluster: DatatableRoleCluster, role: &InstanceDatatableRole, ) -> Result<()> { - crate::datatable_roles_oss::insert_role_catalog_entry(tx, id, role).await + crate::datatable_roles_oss::insert_role_catalog_entry(tx, id, cluster, role).await } /// Update a role's recorded name, login flag and password. Same contract as @@ -215,7 +274,7 @@ pub fn role_id_by_name<'a>(catalog: &'a DatatableRoleCatalog, name: &str) -> Res .find(|(_, role)| role.name == name) .ok_or_else(|| { Error::NotFound(format!( - "'{name}' is not a data table role of this instance. Defined roles: {}.", + "'{name}' is not a data table role of this database's cluster. Defined roles: {}.", catalog .values() .map(|r| r.name.as_str()) @@ -231,67 +290,87 @@ pub fn role_id_by_name<'a>(catalog: &'a DatatableRoleCatalog, name: &str) -> Res Ok(entry.0.as_str()) } -/// Every instance database the registry knows about. Role provisioning has to reach all of them: -/// a role that cannot `CONNECT` to a database is refused by Postgres before any grant matters. -pub async fn registered_instance_databases(db: &DB) -> Result> { - crate::datatable_roles_oss::registered_instance_databases(db).await +/// Every database Windmill manages on `cluster`. Role provisioning has to reach all of them: a role +/// that cannot `CONNECT` to a database is refused by Postgres before any grant matters. +pub async fn registered_instance_databases( + db: &DB, + cluster: DatatableRoleCluster, +) -> Result> { + crate::datatable_roles_oss::registered_instance_databases(db, cluster).await } -/// `CONNECT` on `dbname` for every enabled role, and none for `PUBLIC`. Run at role creation, at -/// database creation, and lazily whenever an instance data table is administered, so a database -/// provisioned before a role existed is repaired rather than left silently unreachable. +/// `CONNECT` on `dbname` for every enabled role of `cluster`, and none for `PUBLIC`. Run at role +/// creation, at database creation, and lazily whenever a managed data table is administered, so a +/// database provisioned before a role existed is repaired rather than left silently unreachable. /// /// Authorization: rewrites a database's ACL with the server's own credentials and checks nothing. /// Callers MUST have authorized administration of `dbname` — superadmin, or an admin of the /// workspace governing a data table on it. -pub async fn converge_connect_grants(db: &DB, dbname: &str) -> Result<()> { - crate::datatable_roles_oss::converge_connect_grants(db, dbname).await +pub async fn converge_connect_grants( + db: &DB, + cluster: DatatableRoleCluster, + dbname: &str, +) -> Result<()> { + crate::datatable_roles_oss::converge_connect_grants(db, cluster, dbname).await } -/// As [`converge_connect_grants`], with a catalog the caller already read. Same contract. +/// As [`converge_connect_grants`], with the catalog of `cluster` the caller already read. Same +/// contract. pub async fn converge_connect_grants_with( db: &DB, + cluster: DatatableRoleCluster, dbname: &str, catalog: &DatatableRoleCatalog, ) -> Result<()> { - crate::datatable_roles_oss::converge_connect_grants_with(db, dbname, catalog).await + crate::datatable_roles_oss::converge_connect_grants_with(db, cluster, dbname, catalog).await } -/// `CREATE ROLE LOGIN PASSWORD ...; GRANT TO custom_instance_user`, and `CONNECT` on -/// every registered database. No privileges beyond that — an admin grants them through SQL or the -/// ACL editor. +/// `CREATE ROLE LOGIN PASSWORD ...; GRANT TO custom_instance_user` on `cluster`. No +/// privileges beyond that — an admin grants them through SQL or the ACL editor. +/// +/// On Windmill's own cluster the DDL runs on `tx`, so it commits with the catalog row. The external +/// cluster is another server: the role is created there before `tx` commits, and callers MUST drop +/// it again ([`drop_datatable_role`]) if `tx` then fails to commit. /// /// Authorization: creates a cluster-wide Postgres login. Callers MUST restrict this to superadmin -/// paths, and MUST hold [`lock_role_catalog`] on the same transaction. -pub async fn create_instance_role( +/// paths, and MUST hold [`lock_role_catalog`] on `tx`. +pub async fn create_datatable_role( + db: &DB, tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + cluster: DatatableRoleCluster, name: &str, password: &str, ) -> Result<()> { - crate::datatable_roles_oss::create_instance_role(tx, name, password).await + crate::datatable_roles_oss::create_datatable_role(db, tx, cluster, name, password).await } /// Authorization: alters a cluster-wide Postgres login. Callers MUST restrict this to superadmin -/// paths, and MUST hold [`lock_role_catalog`] on the same transaction. -pub async fn set_instance_role_login( +/// paths, and MUST hold [`lock_role_catalog`] on `tx`. +pub async fn set_datatable_role_login( + db: &DB, tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + cluster: DatatableRoleCluster, name: &str, enabled: bool, ) -> Result<()> { - crate::datatable_roles_oss::set_instance_role_login(tx, name, enabled).await + crate::datatable_roles_oss::set_datatable_role_login(db, tx, cluster, name, enabled).await } -/// A rename discards an md5-hashed password, so the caller has to hand over a fresh one. +/// A rename discards an md5-hashed password, so the caller has to hand over a fresh one. On the +/// external cluster the rename lands before `tx` commits, and callers MUST rename it back if `tx` +/// then fails to commit. /// /// Authorization: renames a cluster-wide Postgres login. Callers MUST restrict this to superadmin -/// paths, and MUST hold [`lock_role_catalog`] on the same transaction. -pub async fn rename_instance_role( +/// paths, and MUST hold [`lock_role_catalog`] on `tx`. +pub async fn rename_datatable_role( + db: &DB, tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + cluster: DatatableRoleCluster, from: &str, to: &str, password: &str, ) -> Result<()> { - crate::datatable_roles_oss::rename_instance_role(tx, from, to, password).await + crate::datatable_roles_oss::rename_datatable_role(db, tx, cluster, from, to, password).await } /// A role owning anything in any database blocks its own `DROP ROLE`, and both its objects and the @@ -299,8 +378,9 @@ pub async fn rename_instance_role( /// registry. An unreachable database aborts the whole delete: dropping the role while one database /// still holds objects owned by it leaves those objects owned by a numeric OID nobody can name. /// -/// Each pass runs as the instance's own Postgres user rather than `custom_instance_user`, which -/// owns the databases and can therefore revoke a grant whoever made it. `custom_instance_user` +/// Each pass runs as the cluster's administrator rather than `custom_instance_user`: on Windmill's +/// own cluster the instance's Postgres user, on the external one its configured admin login. Both +/// own the databases and can therefore revoke a grant whoever made it. `custom_instance_user` /// could only undo what it granted itself, so a privilege planted by an operator in psql — the /// ordinary way privileges reach a role — would survive and block the drop. /// @@ -308,16 +388,19 @@ pub async fn rename_instance_role( /// MUST restrict this to superadmin paths, and MUST hold [`lock_role_catalog`] on `tx`. /// /// The per-database passes open their own connections and cannot join `tx`; the lock is what keeps -/// a concurrent mutation out while they run. Only the final `DROP ROLE` is on `tx`, so it commits -/// or rolls back with the catalog write that forgets the role. Those passes commit as they go, so -/// callers MUST have disabled the role in an earlier committed transaction: a failure part-way -/// then leaves a disabled role to retry, not an enabled one already stripped in some databases. -pub async fn drop_instance_role( +/// a concurrent mutation out while they run. On Windmill's own cluster only the final `DROP ROLE` +/// is on `tx`, so it commits or rolls back with the catalog write that forgets the role; on the +/// external cluster it runs there, and tolerates a role already gone so a retry after a failed +/// commit can finish. The passes commit as they go, so callers MUST have disabled the role in an +/// earlier committed transaction: a failure part-way then leaves a disabled role to retry, not an +/// enabled one already stripped in some databases. +pub async fn drop_datatable_role( db: &DB, tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + cluster: DatatableRoleCluster, name: &str, ) -> Result<()> { - crate::datatable_roles_oss::drop_instance_role(db, tx, name).await + crate::datatable_roles_oss::drop_datatable_role(db, tx, cluster, name).await } #[cfg(test)] diff --git a/backend/windmill-common/src/datatable_roles_oss.rs b/backend/windmill-common/src/datatable_roles_oss.rs index 2a3f9dda48..a97a936827 100644 --- a/backend/windmill-common/src/datatable_roles_oss.rs +++ b/backend/windmill-common/src/datatable_roles_oss.rs @@ -24,12 +24,12 @@ pub fn datatable_roles_unavailable() -> Error { #[cfg(all(feature = "private", feature = "enterprise"))] pub(crate) use crate::datatable_roles_ee::{ can_use_datatable_role, can_use_datatable_role_in_governing_workspace, converge_connect_grants, - converge_connect_grants_with, create_instance_role, delete_role_catalog_entry, - drop_instance_role, ensure_can_use_datatable_role, ensure_datatable_admin_access, + converge_connect_grants_with, create_datatable_role, delete_role_catalog_entry, + drop_datatable_role, ensure_can_use_datatable_role, ensure_datatable_admin_access, ensure_instance_db_grant_options_unchecked, forget_datatable_role_everywhere, insert_role_catalog_entry, read_role_catalog, read_role_catalog_tx, - registered_instance_databases, rename_instance_role, resolve_datatable_role_connection, - set_instance_role_login, update_role_catalog_entry, + registered_instance_databases, rename_datatable_role, resolve_datatable_role_connection, + role_cluster, set_datatable_role_login, update_role_catalog_entry, }; #[cfg(not(all(feature = "private", feature = "enterprise")))] @@ -39,7 +39,7 @@ pub(crate) use ce::*; mod ce { use super::datatable_roles_unavailable as unavailable; use crate::{ - datatable_roles::{DatatableRoleCatalog, InstanceDatatableRole}, + datatable_roles::{DatatableRoleCatalog, DatatableRoleCluster, InstanceDatatableRole}, db::AuthedRef, error::Result, workspaces::{ @@ -50,17 +50,31 @@ mod ce { type Tx<'a> = sqlx::Transaction<'a, sqlx::Postgres>; - pub(crate) async fn read_role_catalog(_db: &DB) -> Result { + pub(crate) async fn read_role_catalog( + _db: &DB, + _cluster: DatatableRoleCluster, + ) -> Result { Err(unavailable()) } - pub(crate) async fn read_role_catalog_tx(_tx: &mut Tx<'_>) -> Result { + pub(crate) async fn read_role_catalog_tx( + _tx: &mut Tx<'_>, + _cluster: DatatableRoleCluster, + ) -> Result { + Err(unavailable()) + } + + pub(crate) async fn role_cluster( + _tx: &mut Tx<'_>, + _id: &str, + ) -> Result> { Err(unavailable()) } pub(crate) async fn insert_role_catalog_entry( _tx: &mut Tx<'_>, _id: &str, + _cluster: DatatableRoleCluster, _role: &InstanceDatatableRole, ) -> Result<()> { Err(unavailable()) @@ -78,43 +92,57 @@ mod ce { Err(unavailable()) } - pub(crate) async fn registered_instance_databases(_db: &DB) -> Result> { + pub(crate) async fn registered_instance_databases( + _db: &DB, + _cluster: DatatableRoleCluster, + ) -> Result> { Err(unavailable()) } - /// Nothing to converge: with no roles to admit, an instance database keeps the `CONNECT` - /// grants it was created with, `PUBLIC`'s included, as it did before roles existed. - pub(crate) async fn converge_connect_grants(_db: &DB, _dbname: &str) -> Result<()> { + /// Nothing to converge: with no roles to admit, a managed database keeps the `CONNECT` grants + /// it was created with, as it did before roles existed. + pub(crate) async fn converge_connect_grants( + _db: &DB, + _cluster: DatatableRoleCluster, + _dbname: &str, + ) -> Result<()> { Ok(()) } /// As [`converge_connect_grants`]. pub(crate) async fn converge_connect_grants_with( _db: &DB, + _cluster: DatatableRoleCluster, _dbname: &str, _catalog: &DatatableRoleCatalog, ) -> Result<()> { Ok(()) } - pub(crate) async fn create_instance_role( + pub(crate) async fn create_datatable_role( + _db: &DB, _tx: &mut Tx<'_>, + _cluster: DatatableRoleCluster, _name: &str, _password: &str, ) -> Result<()> { Err(unavailable()) } - pub(crate) async fn set_instance_role_login( + pub(crate) async fn set_datatable_role_login( + _db: &DB, _tx: &mut Tx<'_>, + _cluster: DatatableRoleCluster, _name: &str, _enabled: bool, ) -> Result<()> { Err(unavailable()) } - pub(crate) async fn rename_instance_role( + pub(crate) async fn rename_datatable_role( + _db: &DB, _tx: &mut Tx<'_>, + _cluster: DatatableRoleCluster, _from: &str, _to: &str, _password: &str, @@ -122,12 +150,18 @@ mod ce { Err(unavailable()) } - pub(crate) async fn drop_instance_role(_db: &DB, _tx: &mut Tx<'_>, _name: &str) -> Result<()> { + pub(crate) async fn drop_datatable_role( + _db: &DB, + _tx: &mut Tx<'_>, + _cluster: DatatableRoleCluster, + _name: &str, + ) -> Result<()> { Err(unavailable()) } pub(crate) async fn ensure_instance_db_grant_options_unchecked( _db: &DB, + _cluster: DatatableRoleCluster, _dbname: &str, ) -> Result<()> { Err(unavailable()) diff --git a/backend/windmill-common/src/external_instance_pg.rs b/backend/windmill-common/src/external_instance_pg.rs index 66317ef05a..cb5691b2a6 100644 --- a/backend/windmill-common/src/external_instance_pg.rs +++ b/backend/windmill-common/src/external_instance_pg.rs @@ -172,27 +172,48 @@ pub async fn external_instance_database_usages<'c>( Ok(usages) } -/// Refuse to unset the cluster while Windmill still has databases on it, or a workspace still -/// points at one: every data table there would stop resolving. Allowed on every edition, so a +/// Refuse to unset the cluster while Windmill still has databases or data table roles on it, or a +/// workspace still points at one: every data table there would stop resolving, and every role +/// would be a login nothing can drop any more. Allowed on every edition, so a /// downgraded instance can still clear a setting it no longer uses. pub async fn ensure_external_instance_pg_removable(db: &DB) -> Result<()> { + ensure_external_instance_pg_unused(db, &format!("removing {EXTERNAL_INSTANCE_PG_SETTING}")) + .await +} + +/// Refuse while Windmill has databases or data table roles on the cluster, or a workspace points +/// at one of its databases. `before` finishes the sentence saying what to do first. +async fn ensure_external_instance_pg_unused(db: &DB, before: &str) -> Result<()> { let state = read_external_instance_pg_state(db).await?; let usages = external_instance_database_usages(db).await?; - if state.databases.is_empty() && usages.is_empty() { + let roles = sqlx::query_scalar::<_, String>( + "SELECT name FROM datatable_role WHERE cluster = 'external_instance' ORDER BY name", + ) + .fetch_all(db) + .await?; + if state.databases.is_empty() && usages.is_empty() && roles.is_empty() { return Ok(()); } - let names = state - .databases - .keys() - .chain(usages.keys()) - .collect::>() - .into_iter() - .cloned() - .collect::>() - .join(", "); + let mut held = vec![]; + if !(state.databases.is_empty() && usages.is_empty()) { + let names = state + .databases + .keys() + .chain(usages.keys()) + .collect::>() + .into_iter() + .cloned() + .collect::>() + .join(", "); + held.push(format!("databases in use ({names})")); + } + if !roles.is_empty() { + held.push(format!("data table roles ({})", roles.join(", "))); + } Err(Error::BadRequest(format!( - "The external instance cluster still holds databases in use ({names}). Drop them and \ - repoint the data tables and Ducklake catalogs using them before removing {EXTERNAL_INSTANCE_PG_SETTING}." + "The external instance cluster still holds {}. Drop them and repoint the data tables and \ + Ducklake catalogs using them before {before}.", + held.join(" and ") ))) } @@ -300,9 +321,10 @@ pub async fn check_external_instance_pg_write( } } -/// Refuse pointing the setting at another host or port while databases live on the current one. -/// Data tables name databases, not clusters, so they would silently resolve to whatever the new -/// cluster holds under the same names. Other fields (admin login, sslmode) may change freely. +/// Refuse pointing the setting at another host or port while databases or data table roles live on +/// the current one. Data tables name databases, and the role catalog names logins, not clusters, so +/// both would silently resolve to whatever the new cluster holds under the same names. Other fields +/// (admin login, sslmode) may change freely. async fn ensure_external_instance_pg_not_repointed( db: &DB, value: &serde_json::Value, @@ -317,17 +339,11 @@ async fn ensure_external_instance_pg_not_repointed( if address(¤t) == address(&desired) { return Ok(()); } - let state = read_external_instance_pg_state(db).await?; - let usages = external_instance_database_usages(db).await?; - if state.databases.is_empty() && usages.is_empty() { - return Ok(()); - } - Err(Error::BadRequest(format!( - "The external instance cluster at {}:{} still holds databases in use. Drop them and repoint \ - what uses them before pointing {EXTERNAL_INSTANCE_PG_SETTING} at another cluster.", - current.host.trim(), - current.port.unwrap_or(5432) - ))) + ensure_external_instance_pg_unused( + db, + &format!("pointing {EXTERNAL_INSTANCE_PG_SETTING} at another cluster"), + ) + .await } /// Converge the external cluster on the configured login: check what it can do, create or update diff --git a/backend/windmill-common/src/lib.rs b/backend/windmill-common/src/lib.rs index 83295c6c45..1122c46ac8 100644 --- a/backend/windmill-common/src/lib.rs +++ b/backend/windmill-common/src/lib.rs @@ -1563,18 +1563,20 @@ pub(crate) fn instance_db_grants(dbname: &str) -> String { ) } -/// Re-apply [`instance_db_grants`] to an instance database provisioned before data table roles -/// existed, whose grants carry no grant option. Connects as the instance's own Postgres user — -/// the database and `public` schema owner — since only it can hand out an option it holds. +/// Re-apply [`instance_db_grants`] to a managed database provisioned before data table roles +/// existed, whose grants carry no grant option. Connects as the cluster's administrator — the +/// database and `public` schema owner — since only it can hand out an option it holds. /// -/// Authorization: reaches an instance database with the server's own credentials and checks +/// Authorization: reaches a managed database with the server's own credentials and checks /// nothing. Callers MUST have authorized administration of `dbname` — superadmin, or an admin of /// the workspace governing a data table on it. pub async fn ensure_instance_db_grant_options_unchecked( db: &DB, + cluster: crate::datatable_roles::DatatableRoleCluster, dbname: &str, ) -> error::Result<()> { - crate::datatable_roles_oss::ensure_instance_db_grant_options_unchecked(db, dbname).await + crate::datatable_roles_oss::ensure_instance_db_grant_options_unchecked(db, cluster, dbname) + .await } /// Create a custom instance database: CREATE DATABASE, grant permissions, register in global_settings. @@ -1648,7 +1650,13 @@ pub async fn create_custom_instance_database( // A data table role can only reach a database it may CONNECT to, and PUBLIC's default CONNECT // would otherwise let every role in regardless of what this instance defines. Best-effort: a // failure here leaves the database usable as `admin`, and the next role change repairs it. - if let Err(e) = crate::datatable_roles::converge_connect_grants(db, dbname).await { + if let Err(e) = crate::datatable_roles::converge_connect_grants( + db, + crate::datatable_roles::DatatableRoleCluster::Instance, + dbname, + ) + .await + { tracing::warn!("Could not set CONNECT grants on instance database '{dbname}': {e}"); } diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index 931a971ccb..87bbba4761 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -1484,13 +1484,14 @@ pub struct GoverningDatatable { } impl GoverningDatatable { - /// Backed by the Windmill instance's own Postgres, which is the only substrate data table - /// roles apply to. - pub fn is_instance(&self) -> bool { + /// The Windmill-managed cluster whose data table roles this entry can use. `None` for a + /// resource-backed one: roles are logins Windmill creates, and it creates none on a host a + /// workspace admin chose. + pub fn role_cluster(&self) -> Option { self.datatable .database .as_ref() - .is_some_and(|d| d.resource_type == DataTableCatalogResourceType::Instance) + .and_then(|d| crate::datatable_roles::DatatableRoleCluster::of(d.resource_type)) } }