starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-09-21 00:02:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(datatables): close the last ways a role or a pointer can be left pointing at nothing

Browse Source 
The raw settings readers hand back whatever is in the row, so moving the catalog into its own
`global_settings` key protected the config machinery and left `GET /settings/global/datatable_roles`
and the settings listing returning every live password. Both now filter that one key. The
neighbouring `custom_instance_replication_pwd` has the same shape and is not touched here: it
predates this and widening the fix to it is a decision about an operator workflow, not a
consequence of this change.

Three ways a save could leave something resolving to nothing:

A permissioned data table could be moved to a PostgreSQL resource. The block was carried across
as a server-owned field, the runtime refuses roles on a resource-backed table, so the save
succeeded and every job afterwards failed. Refused instead — turning roles off first is one step,
and it keeps discarding an access decision something somebody chose.

Renaming a governing data table left every fork pointing at the old name: the data table
disappears from their pickers and their jobs stop, with nothing in the renaming workspace to
suggest why. The rename now follows into the pointers in the same transaction.

Deleting one cannot be followed the same way, so it is reported instead — the response names what
it stranded, the way deleting a workspace does, and the fork's own error already says which
workspace is gone.

Also: `ensure_instance_db_grant_options_unchecked` claimed superadmin while the permissions
handler reaches it as a workspace admin (the same class fixed last commit, one instance missed);
the role entry kept an `instance_config_schema` derive it no longer needs; `write_role_catalog`
was the one writer of that table not stamping `updated_at`; and the concurrency test dropped its
roles only on success — a failing run is exactly the one that creates them without recording them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR
This commit is contained in:
Diego Imbert
2026-09-08 15:56:03 +02:00
co-authored by Claude Opus 5
parent 035897b9b9
commit 30803e4e5a
8 changed files with 229 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/windmill-api-workspaces/src/workspaces.rs
+75 -1
View File
@@ -3619,6 +3619,21 @@ async fn edit_datatable_config(
dt.permissions = old.and_then(|old| old.permissions.clone());
dt.reference = old.and_then(|old| old.reference.clone());
dt.forked_from = old.and_then(|old| old.forked_from.clone());
// 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.
if dt.permissions.is_some()
&& dt
.database
.as_ref()
.is_some_and(|d| d.resource_type != DataTableCatalogResourceType::Instance)
{
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."
)));
}
// A pointer names no database of its own, so the form's empty `database` is correct there.
if dt.reference.is_some() {
dt.database = None;
@@ -3685,6 +3700,53 @@ async fn edit_datatable_config(
)
.await?;
// A fork points at a data table by name, so a rename here has to follow or every fork's entry
// resolves to nothing. Inside the transaction: the rename and the pointers that name it are one
// change, and half of it is a fork whose jobs stop.
for r in &new_config.renames {
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' = $1
AND dt.value->'reference'->>'datatable' = $2
THEN jsonb_set(dt.value, '{reference,datatable}', to_jsonb($3::text))
ELSE dt.value END
))
FROM jsonb_each(ws.datatable->'datatables') dt
)
WHERE jsonb_typeof(ws.datatable->'datatables') = 'object'
AND ws.datatable::text LIKE '%"reference"%'"#,
&w_id,
&r.from,
&r.to,
)
.execute(&mut *tx)
.await?;
}
// A deletion cannot be followed the same way — there is nothing to point at any more. Read who
// is left stranded so the caller is told, the way deleting a workspace does.
let mut stranded: Vec<String> = Vec::new();
for name in &new_config.deleted_datatables {
let rows = sqlx::query!(
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"#,
&w_id,
name,
)
.fetch_all(&mut *tx)
.await?;
stranded.extend(
rows.into_iter()
.map(|r| format!("{}/{}", r.workspace_id, r.datatable)),
);
}
tx.commit().await?;
for substrate in created_substrates {
@@ -3699,7 +3761,19 @@ async fn edit_datatable_config(
)
.await?;
Ok(format!("Edit datatable config for workspace {}", &w_id))
if stranded.is_empty() {
Ok(format!("Edit datatable config for workspace {}", &w_id))
} else {
Ok(format!(
concat!(
"Edit datatable config for workspace {}. These data tables were governed by one ",
"you deleted and no longer resolve: {}. Their databases still exist; a superadmin ",
"can point them at another workspace's data table."
),
&w_id,
stranded.join(", ")
))
}
}
#[derive(Deserialize)]