Merge remote-tracking branch 'origin/datatable-external-instance-4' into datatable-external-instance-ui

backend/ee-repo-ref.txt

@@ -1 +1 @@
-438e5efda9674b37d25e6c02e286adbf2566d25c
+50ef80045ddb208ee1feee2d9670210f703620bf

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);

backend/windmill-api-settings/src/lib.rs

@@ -1887,7 +1887,7 @@ async fn drop_external_instance_pg_database(
     require_super_admin(&db, &authed).await?;
     // A data table naming a dropped database fails on every job, far from the drop that caused it.
     windmill_common::external_instance_pg::drop_external_instance_database_unchecked(
-        &db, &dbname, true,
+        &db, &dbname, None,
     )
     .await?;
     windmill_audit::audit_oss::audit_log(

backend/windmill-api-workspaces/src/workspaces_extra.rs

@@ -1421,8 +1421,11 @@ pub async fn drop_forked_datatable_databases(
             let dropped = if database.resource_type
                 == windmill_common::workspaces::DataTableCatalogResourceType::ExternalInstance
             {
+                // Its own entry still names the copy; another workspace's never should.
                 windmill_common::external_instance_pg::drop_external_instance_database_unchecked(
-                    &db, db_to_drop, false,
+                    &db,
+                    db_to_drop,
+                    Some(&w_id),
                 )
                 .await
             } else {

backend/windmill-common/src/external_instance_pg.rs

@@ -252,19 +252,20 @@ pub async fn create_external_instance_database_unchecked(
 }
 
 /// Drop `dbname` from the external cluster: only a database Windmill registered creating, and still
-/// carries the mark it set there. With `refuse_if_used`, refuse while a data table names it.
+/// carries the mark it set there. Refused while a data table names it, except one in
+/// `usage_allowed_in`: the fork whose own copy is being cleaned up.
 ///
 /// Authorization: checks nothing. Callers MUST be superadmin, or be deleting the fork that owns
 /// this `wm_fork_` database.
 pub async fn drop_external_instance_database_unchecked(
     db: &DB,
     dbname: &str,
-    refuse_if_used: bool,
+    usage_allowed_in: Option<&str>,
 ) -> Result<()> {
     crate::external_instance_pg_oss::drop_external_instance_database_unchecked(
         db,
         dbname,
-        refuse_if_used,
+        usage_allowed_in,
     )
     .await
 }
@@ -310,7 +311,8 @@ pub async fn ensure_external_instance_database_registered(
 /// check taken outside it could pass while a database create still reads the old cluster, which
 /// would then register a database there after the setting names another one.
 ///
-/// Authorization: checks nothing. Callers MUST be superadmin.
+/// Authorization: checks nothing. Callers MUST be superadmin, or the declarative instance config
+/// sync, which applies what the operator deployed.
 pub async fn write_external_instance_pg_setting(
     db: &DB,
     value: Option<&serde_json::Value>,
@@ -353,6 +355,9 @@ pub async fn write_external_instance_pg_setting(
 
 /// [`write_external_instance_pg_setting`] for a settings diff: writes the key if the diff touches
 /// it, and takes it out of the diff so the generic apply does not write it again.
+///
+/// Authorization: checks nothing. Callers MUST be superadmin, or the declarative instance config
+/// sync, which applies what the operator deployed.
 pub async fn write_external_instance_pg_from_diff(
     db: &DB,
     diff: &mut crate::instance_config::SettingsDiff,

backend/windmill-common/src/external_instance_pg_oss.rs

@@ -73,7 +73,7 @@ mod ce {
     pub(crate) async fn drop_external_instance_database_unchecked(
         _db: &DB,
         _dbname: &str,
-        _refuse_if_used: bool,
+        _usage_allowed_in: Option<&str>,
     ) -> Result<()> {
         Err(unavailable())
     }

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,
       },
     };