fix(datatables): refuse to roll back the catalog while roles exist

The down migration dropped the table and left every role behind: live Postgres
logins whose passwords only that table carried, so after a revert Windmill could
neither use, disable nor delete them, and re-applying could not recreate them
because the names were taken. Cleaning up here is not possible either — dropping
a role means reassigning what it owns in every instance database, and a
migration runs in one — so it now refuses while the catalog is non-empty and
says to delete the roles through instance settings, which does the cluster work.

Also enforces the instance-only invariant the resolved-pointer clone relies on
rather than only asserting it in a comment.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb
This commit is contained in:
Diego Imbert
2026-09-10 08:16:58 +02:00
co-authored by Claude Opus 5
parent 4f4ecba9cc
commit f53652d46f
2 changed files with 38 additions and 9 deletions
@@ -1 +1,16 @@
-- Refuse while the catalog holds anything. Each row is a live Postgres login with a password
-- only this table carries, so dropping it would leave credentials on the cluster that Windmill can
-- no longer disable, delete or even name — and re-applying could not recreate them, because the
-- role names would already be taken. Cleaning them up here is not an option either: dropping a
-- role means reassigning what it owns in *every* instance database, and a migration runs in one.
--
-- Delete the roles through instance settings first; that path does the cluster work.
DO $$
BEGIN
IF EXISTS (SELECT 1 FROM datatable_role) THEN
RAISE EXCEPTION 'Cannot roll back: % data table role(s) still exist as Postgres logins. Delete them in instance settings first, which drops them from the cluster.',
(SELECT count(*) FROM datatable_role);
END IF;
END $$;
DROP TABLE IF EXISTS datatable_role;
@@ -7922,16 +7922,30 @@ async fn apply_forked_datatable(
// (`point_kept_datatables_at_parent`), so the resolved entry is one.
let database = match dt.database.clone() {
Some(database) => database,
None => resolve_governing_datatable(db, parent_w_id, &fdt.name)
.await?
.datatable
.database
.ok_or_else(|| {
Error::internal_err(format!(
"Data table '{}' resolves to an entry that owns no database",
None => {
let resolved = resolve_governing_datatable(db, parent_w_id, &fdt.name)
.await?
.datatable
.database
.ok_or_else(|| {
Error::internal_err(format!(
"Data table '{}' resolves to an entry that owns no database",
fdt.name
))
})?;
// The resource branch below rewrites a resource this workspace owns; a pointer names
// one it does not, so following it there would move the fork onto someone else's
// database. Checked rather than assumed: only `point_kept_datatables_at_parent`
// writes pointers and only for instance entries, but nothing here enforces that.
if resolved.resource_type != DataTableCatalogResourceType::Instance {
return Err(Error::BadRequest(format!(
"Data table '{}' points at a resource-backed data table in another \
workspace and cannot be cloned; fork it from the workspace that owns it.",
fdt.name
))
})?,
)));
}
resolved
}
};
if database.resource_type == DataTableCatalogResourceType::Instance {