From 48f026eb0f363afcad29444d96e330366ddbb9ff Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Thu, 17 Sep 2026 16:35:45 +0200 Subject: [PATCH] fix(datatables): refuse rolling back while external data tables are under roles, and type external_instance in the CLI Co-Authored-By: Claude Opus 5 (1M context) --- .../20260917133824_datatable_role_cluster.down.sql | 11 +++++++++++ cli/src/commands/datatable/datatable.ts | 6 ++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/backend/migrations/20260917133824_datatable_role_cluster.down.sql b/backend/migrations/20260917133824_datatable_role_cluster.down.sql index 2fb7bcc8a8..379f1e3d34 100644 --- a/backend/migrations/20260917133824_datatable_role_cluster.down.sql +++ b/backend/migrations/20260917133824_datatable_role_cluster.down.sql @@ -5,6 +5,17 @@ BEGIN IF EXISTS (SELECT 1 FROM datatable_role WHERE cluster <> 'instance') THEN RAISE EXCEPTION 'datatable_role holds roles on the external instance cluster. Delete them in instance settings first.'; END IF; + -- Before this, only data tables on Windmill's own cluster could be under roles, and a role + -- block left with just `admin` survives deleting every external role. + IF EXISTS ( + SELECT 1 FROM workspace_settings ws, + jsonb_each(CASE WHEN jsonb_typeof(ws.datatable->'datatables') = 'object' + THEN ws.datatable->'datatables' ELSE '{}'::jsonb END) dt + WHERE dt.value->'database'->>'resource_type' = 'external_instance' + AND dt.value ? 'permissions' + ) THEN + RAISE EXCEPTION 'external instance data tables are still under roles. Turn their roles off first.'; + END IF; END $$; ALTER TABLE datatable_role DROP CONSTRAINT datatable_role_cluster_name_key; ALTER TABLE datatable_role ADD CONSTRAINT datatable_role_name_key UNIQUE (name); diff --git a/cli/src/commands/datatable/datatable.ts b/cli/src/commands/datatable/datatable.ts index 6609d08c50..46ee7b4c39 100644 --- a/cli/src/commands/datatable/datatable.ts +++ b/cli/src/commands/datatable/datatable.ts @@ -111,6 +111,8 @@ const migrateCommand = new Command() ) .action(migrateDown as any); +type DataTableResourceType = "postgresql" | "instance" | "external_instance"; + async function create( opts: GlobalOptions & { resource?: string; force?: boolean }, name?: string, @@ -139,12 +141,12 @@ async function create( const datatables: Record< string, - { database: { resource_type: "postgresql" | "instance"; resource_path?: string } } + { database: { resource_type: DataTableResourceType; resource_path?: string } } > = {}; for (const d of existing) { datatables[d.name] = { database: { - resource_type: d.resource_type as "postgresql" | "instance", + resource_type: d.resource_type as DataTableResourceType, resource_path: d.resource_path ?? undefined, }, };