mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix: declare the default role in migrations written for a data table whose name contains '?'
Such a data table connects as its default role without naming it, so the migrations the manager wrote for it declared no role and ran as admin. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
10861bb529
commit
dcc37bf6f2
@@ -17,6 +17,7 @@
|
||||
import {
|
||||
ADMIN_DATATABLE_ROLE,
|
||||
datatableNameTakesRole,
|
||||
defaultMigrationRole,
|
||||
type DatatableRowAction
|
||||
} from './dbTypes'
|
||||
import ResourcePicker from './ResourcePicker.svelte'
|
||||
@@ -103,6 +104,17 @@
|
||||
if (effective) untrack(() => (uriState.selectedRole = effective))
|
||||
})
|
||||
|
||||
const contentInput = $derived.by(() => {
|
||||
const input = uriState.effectiveInput
|
||||
if (input?.type !== 'database' || selectedDatatable === undefined) return input
|
||||
const migrationRole = defaultMigrationRole(
|
||||
selectedDatatable,
|
||||
rolesOfCurrent?.permissioned,
|
||||
rolesOfCurrent?.default_role
|
||||
)
|
||||
return migrationRole === undefined ? input : { ...input, migrationRole }
|
||||
})
|
||||
|
||||
// Every data table with its schemas and tables, in one call: this is what the
|
||||
// left pane's tree navigates, so it has to cover the data tables the user is
|
||||
// not currently on, not just the selected one. The privileges it reports are
|
||||
@@ -295,11 +307,11 @@
|
||||
noPadding
|
||||
id="db-manager-drawer"
|
||||
>
|
||||
{#if uriState.effectiveInput && ws && roleSettled}
|
||||
{#if contentInput && ws && roleSettled}
|
||||
{#key `${selectedDatatable}~${selectedRole ?? ''}`}
|
||||
<DBManagerContent
|
||||
bind:this={dbManagerContent}
|
||||
input={uriState.effectiveInput}
|
||||
input={contentInput}
|
||||
workspace={uriState.workspace}
|
||||
datatableTree={uriState.isDatatableInput ? datatables.current : undefined}
|
||||
datatableTreeLoading={datatables.loading}
|
||||
|
||||
@@ -227,6 +227,6 @@
|
||||
bind:this={ddlGuard}
|
||||
workspace={ws}
|
||||
datatable={datatableName}
|
||||
role={input.type === 'database' ? input.role : undefined}
|
||||
role={input.type === 'database' ? (input.role ?? input.migrationRole) : undefined}
|
||||
/>
|
||||
{/if}
|
||||
|
||||
@@ -299,7 +299,7 @@ export function dbSchemaOpsWithPreviewScripts({
|
||||
? input.resourcePath.slice('datatable://'.length)
|
||||
: undefined
|
||||
// A migration declaring no role runs as admin, whatever role the manager connects as.
|
||||
const migrationRole = input.type === 'database' ? input.role : undefined
|
||||
const migrationRole = input.type === 'database' ? (input.role ?? input.migrationRole) : undefined
|
||||
|
||||
function makeMarker(op: string, payload: Record<string, unknown>): string {
|
||||
if (ducklake) payload.ducklake = ducklake
|
||||
|
||||
@@ -6,6 +6,9 @@ export type DbInput =
|
||||
/** The data table role to connect as; the data table's default when unset. Only
|
||||
* meaningful for a `datatable://` path. */
|
||||
role?: string
|
||||
/** The role migrations written through this input declare when `role` is unset. A
|
||||
* migration declaring none runs as admin, not as the role the manager connects as. */
|
||||
migrationRole?: string
|
||||
specificSchema?: string
|
||||
specificTable?: string
|
||||
}
|
||||
@@ -42,6 +45,16 @@ export function datatableNameTakesRole(name: string): boolean {
|
||||
return !name.includes('?')
|
||||
}
|
||||
|
||||
/** The `migrationRole` of a data table that cannot name a role in its reference: it connects as
|
||||
* its default role, which its migrations must then declare. */
|
||||
export function defaultMigrationRole(
|
||||
name: string,
|
||||
permissioned: boolean | undefined,
|
||||
defaultRole: string | undefined
|
||||
): string | undefined {
|
||||
return permissioned && !datatableNameTakesRole(name) ? defaultRole : undefined
|
||||
}
|
||||
|
||||
/** `datatable://<name>`, with `?role=<role>` when a role is named. Throws rather than build a
|
||||
* reference the executor would refuse, or one that would silently mean another role. */
|
||||
export function datatableReference(name: string, role: string | undefined): string {
|
||||
|
||||
@@ -11,7 +11,12 @@
|
||||
import { resource } from 'runed'
|
||||
import { ArrowLeft, Expand, Minimize, Plus, RefreshCcw } from 'lucide-svelte'
|
||||
import DBManagerContent from '../DBManagerContent.svelte'
|
||||
import { ADMIN_DATATABLE_ROLE, datatableNameTakesRole, type DbInput } from '../dbTypes'
|
||||
import {
|
||||
ADMIN_DATATABLE_ROLE,
|
||||
datatableNameTakesRole,
|
||||
defaultMigrationRole,
|
||||
type DbInput
|
||||
} from '../dbTypes'
|
||||
import type { PendingRowAction, SelectedTable } from '../DBManager.svelte'
|
||||
import { getRawAppOperatingWorkspace } from './rawAppWorkspace'
|
||||
import { useDbManagerTag } from '../dbManagerTag.svelte'
|
||||
@@ -107,12 +112,12 @@
|
||||
// without one runs, and caches, as whatever the server defaults to.
|
||||
const roleSettled = $derived(
|
||||
selectedDatatable === undefined ||
|
||||
// Its reference cannot name a role, so it connects as the default one.
|
||||
!datatableNameTakesRole(selectedDatatable) ||
|
||||
(rolesOfCurrent !== undefined &&
|
||||
(!rolesOfCurrent.permissioned ||
|
||||
rolesOfCurrent.roles.length === 0 ||
|
||||
selectedRole !== undefined))
|
||||
selectedRole !== undefined ||
|
||||
// Its reference cannot name a role, so it connects as the default one.
|
||||
!datatableNameTakesRole(selectedDatatable)))
|
||||
)
|
||||
|
||||
$effect(() => {
|
||||
@@ -307,6 +312,11 @@
|
||||
resourceType: 'postgresql' as const,
|
||||
resourcePath: `datatable://${selectedDatatable}`,
|
||||
role: selectedRole,
|
||||
migrationRole: defaultMigrationRole(
|
||||
selectedDatatable,
|
||||
rolesOfCurrent?.permissioned,
|
||||
rolesOfCurrent?.default_role
|
||||
),
|
||||
specificSchema: openSchemaKey,
|
||||
specificTable: openTableKey
|
||||
}
|
||||
|
||||
@@ -34,7 +34,7 @@
|
||||
toDatatableItems,
|
||||
toSchemaItems
|
||||
} from './datatableUtils.svelte'
|
||||
import { datatableNameTakesRole } from '../dbTypes'
|
||||
import { datatableNameTakesRole, defaultMigrationRole } from '../dbTypes'
|
||||
import RawAppDataTableList from './RawAppDataTableList.svelte'
|
||||
import RawAppDataTableDrawer from './RawAppDataTableDrawer.svelte'
|
||||
import FileEditorIcon from './FileEditorIcon.svelte'
|
||||
@@ -290,7 +290,12 @@
|
||||
type: 'database',
|
||||
resourceType: 'postgresql',
|
||||
resourcePath: `datatable://${selectedDatatable}`,
|
||||
role: effectiveRole
|
||||
role: effectiveRole,
|
||||
migrationRole: defaultMigrationRole(
|
||||
selectedDatatable,
|
||||
roles.current.permissioned,
|
||||
roles.current.defaultRole
|
||||
)
|
||||
}
|
||||
})
|
||||
await dbOps.onCreateSchema({ schema: newSchemaName })
|
||||
|
||||
Reference in New Issue
Block a user