diff --git a/backend/windmill-api/src/hub_publish.rs b/backend/windmill-api/src/hub_publish.rs index ccb530ce20..5185b437c8 100644 --- a/backend/windmill-api/src/hub_publish.rs +++ b/backend/windmill-api/src/hub_publish.rs @@ -462,6 +462,17 @@ async fn publish_resource_type( struct PublishResourceBody { path: String, resource_type: String, + /// Whether an importer has to fill this in for the project to run. False for a + /// stub minted from an item's `resource-` input, which is created so a + /// standalone run has something to pick but which nothing in the project reads. + /// The body is re-serialized into the Hub call, so a field missing here is + /// dropped on the way through; defaulted so an older client still posts. + #[serde(default = "default_true")] + required: bool, +} + +fn default_true() -> bool { + true } #[derive(Deserialize, Serialize)] diff --git a/frontend/src/lib/components/ImportSetupStep.svelte b/frontend/src/lib/components/ImportSetupStep.svelte index 86670b0817..d5ad9db953 100644 --- a/frontend/src/lib/components/ImportSetupStep.svelte +++ b/frontend/src/lib/components/ImportSetupStep.svelte @@ -20,6 +20,7 @@ import { applyOneMigration } from '$lib/components/workspaceSettings/projectInstall' import { probeMigrationsApplied } from '$lib/importWizard/probe' import { + isRequiredResource, retargetProjectExport, type ProjectExport, type ProjectMigration @@ -253,7 +254,12 @@ // Contained for the same reason the import contains: a crafted export can name a // path outside the folder, and offering that for editing would reach a resource // this import was never allowed to create. + // Unrequired stubs are imported but never asked for: nothing in the project + // reads them, they exist so a standalone run of the item that declared the + // type has something to pick. Listing them would ask for a second credential + // to satisfy one dependency, and would block Finish on it. projectResources = (retargeted.resources ?? []) + .filter(isRequiredResource) .map((r) => ({ path: String(r.path), resource_type: String((r as any).resource_type) })) .filter((r) => r.path.startsWith(`f/${target}/`)) await refreshBlanks() @@ -757,15 +763,17 @@ size="xs" > {#if missingTables.length > 0} - The tables {missingTables.length === 1 ? 'this data table holds' : 'these data tables hold'} + The tables {missingTables.length === 1 + ? 'this data table holds' + : 'these data tables hold'} do not exist, and the project's apps and flows read them. Every one of those fails as soon as it opens. {/if} {#if uncheckedTables.length > 0} {#if missingTables.length > 0}

{/if} {uncheckedTables.length === 1 ? 'One data table is' : 'Some data tables are'} set up, but - {uncheckedTables.length === 1 ? 'its' : 'their'} schema could not be read, so whether the - project's tables are there is unknown. Check again once the database is reachable. + {uncheckedTables.length === 1 ? 'its' : 'their'} schema could not be read, so whether the project's + tables are there is unknown. Check again once the database is reachable. {/if} {:else} diff --git a/frontend/src/lib/components/workspaceSettings/deployToHubItems.test.ts b/frontend/src/lib/components/workspaceSettings/deployToHubItems.test.ts index 5ec8eed841..933fa60fa5 100644 --- a/frontend/src/lib/components/workspaceSettings/deployToHubItems.test.ts +++ b/frontend/src/lib/components/workspaceSettings/deployToHubItems.test.ts @@ -3,6 +3,7 @@ import { canRecordSession, inputResourceTypes, mergeAppTableOrigin, + projectResourceExports, type DeployItem } from './deployToHubItems' @@ -62,3 +63,43 @@ describe('inputResourceTypes', () => { expect(inputResourceTypes(schema, new Set())).toEqual(all) }) }) + +describe('projectResourceExports', () => { + // An app that pins `$res:f/mine/prod_db` for a script arg declared as + // `resource-postgresql` used to publish both `prod_db` and `postgresql`, so the + // importer filled two credentials to satisfy one dependency. + it('ships an input-derived stub unrequired when a $res: one covers the type', () => { + expect( + projectResourceExports( + [{ newPath: 'f/proj/prod_db', resource_type: 'postgresql' }], + ['postgresql'], + 'proj' + ) + ).toEqual([ + { path: 'f/proj/prod_db', resource_type: 'postgresql', required: true }, + { path: 'f/proj/postgresql', resource_type: 'postgresql', required: false } + ]) + }) + + // Unrequired even when it is the project's only resource: an input format names + // no path, so there is nothing to say the item will ever be run. A project whose + // resources are all input-derived asks the importer for nothing, and the stubs + // are what a standalone run picks from. + it('ships an input-derived stub unrequired even when nothing else covers the type', () => { + expect(projectResourceExports([], ['postgresql'], 'proj')).toEqual([ + { path: 'f/proj/postgresql', resource_type: 'postgresql', required: false } + ]) + }) + + // A referenced resource named after its own type relocates onto the conventional + // stub path. Something reads it, so the reference has to win the collision. + it('lets a referenced resource win a path claimed by both', () => { + expect( + projectResourceExports( + [{ newPath: 'f/proj/postgresql', resource_type: 'postgresql' }], + ['postgresql'], + 'proj' + ) + ).toEqual([{ path: 'f/proj/postgresql', resource_type: 'postgresql', required: true }]) + }) +}) diff --git a/frontend/src/lib/components/workspaceSettings/deployToHubItems.ts b/frontend/src/lib/components/workspaceSettings/deployToHubItems.ts index 5c018bcc96..7bcde8f0c5 100644 --- a/frontend/src/lib/components/workspaceSettings/deployToHubItems.ts +++ b/frontend/src/lib/components/workspaceSettings/deployToHubItems.ts @@ -55,6 +55,44 @@ export function inputResourceTypes(schema: unknown, known: Set | undefin return [...out] } +export interface ProjectResourceExport { + path: string + resource_type: string + /** Whether the importer has to fill this in for the project to run. */ + required: boolean +} + +/** + * The resources a project publishes, from the two things that produce one. + * + * A `$res:` token (or a trigger's resource field) names a path something in the + * project reads, so the credential is required or that item cannot run. An input + * schema's `resource-` format names no path at all — it says the project + * needs a type, and a stub gets minted at the conventional `f//` so a + * standalone run of that item has something to pick. Flattening the two makes an + * app that pins `$res:.../prod_db` for a script arg publish `prod_db` *and* + * `postgresql`, and the importer fills two credentials to satisfy one dependency. + * + * So the second kind ships unrequired: still created, never asked for. A path + * claimed by both is required — a referenced resource is read whatever else + * happens to mint the same path. + */ +export function projectResourceExports( + resourceStubs: { newPath: string; resource_type: string }[], + inputTypes: string[], + slug: string +): ProjectResourceExport[] { + const byPath = new Map() + for (const s of resourceStubs) { + byPath.set(s.newPath, { path: s.newPath, resource_type: s.resource_type, required: true }) + } + for (const t of inputTypes) { + const path = `f/${slug}/${t}` + if (!byPath.has(path)) byPath.set(path, { path, resource_type: t, required: false }) + } + return [...byPath.values()] +} + // A raw app has no run to capture: its demo is a recorded session of someone // using it, driven in the record drawer and replayed on the Hub page. Legacy raw // apps live only in the `raw_app` table, and the record surface loads the app diff --git a/frontend/src/lib/components/workspaceSettings/deployToHubSession.svelte.ts b/frontend/src/lib/components/workspaceSettings/deployToHubSession.svelte.ts index c421553a7c..1a60069c19 100644 --- a/frontend/src/lib/components/workspaceSettings/deployToHubSession.svelte.ts +++ b/frontend/src/lib/components/workspaceSettings/deployToHubSession.svelte.ts @@ -40,6 +40,7 @@ import { canRecordSession, inputResourceTypes, mergeAppTableOrigin, + projectResourceExports, HIDDEN_RESOURCE_TYPES, type DeployItem } from './deployToHubItems' @@ -1198,15 +1199,7 @@ export class DeployToHubSession { types.filter((t) => exportedTypes.has(t)) ) - // Input-type deps with no path get a conventional f// stub. - const stubsByPath = new Map() - for (const s of bundle.resourceStubs) - stubsByPath.set(s.newPath, { path: s.newPath, resource_type: s.resource_type }) - for (const t of inputTypes) { - const path = `f/${slug}/${t}` - if (!stubsByPath.has(path)) stubsByPath.set(path, { path, resource_type: t }) - } - const stubs = [...stubsByPath.values()] + const stubs = projectResourceExports(bundle.resourceStubs, inputTypes, slug) if (stubs.length > 0) { try { await this.#postHub('/hub/resources', { resources: stubs, project_slug: slug }) diff --git a/frontend/src/lib/components/workspaceSettings/projectBundle.ts b/frontend/src/lib/components/workspaceSettings/projectBundle.ts index 1fd62498d5..05b81d409e 100644 --- a/frontend/src/lib/components/workspaceSettings/projectBundle.ts +++ b/frontend/src/lib/components/workspaceSettings/projectBundle.ts @@ -376,6 +376,19 @@ export interface ProjectExport { migrations?: ProjectMigration[] } +/** + * Whether a shipped resource is one the importer has to fill in, as opposed to a + * stub minted from an item's `resource-` input so a standalone run of it has + * something to pick. Both are created; only these are asked for. + * + * Absent means required. Projects published before the distinction existed carry + * no flag, and reading that as "not required" would silently stop asking for + * credentials they genuinely need. + */ +export function isRequiredResource(r: ExportItem): boolean { + return r?.required !== false +} + // Map bundled paths `f//...` -> `f//...`. Only enumerated // paths go in, so rewriters touch real refs, never incidental text. export function buildRetargetMap( diff --git a/frontend/src/lib/importWizard/execution.svelte.ts b/frontend/src/lib/importWizard/execution.svelte.ts index 06c33f3222..8c29efb30c 100644 --- a/frontend/src/lib/importWizard/execution.svelte.ts +++ b/frontend/src/lib/importWizard/execution.svelte.ts @@ -8,9 +8,10 @@ import { installProject, type InstallResult } from '$lib/components/workspaceSettings/projectInstall' -import type { - ProjectExport, - ProjectMigration +import { + isRequiredResource, + type ProjectExport, + type ProjectMigration } from '$lib/components/workspaceSettings/projectBundle' import { planWorkspaceId, type ImportPlan } from './plan' import { probeImportedPaths, probeWorkspace } from './probe' @@ -110,12 +111,16 @@ export class ImportExecution { } /** - * How many resources the project shipped. Every one arrives as an empty stub — - * the hub never publishes resource values — so a non-zero count means the setup - * step has something to offer. + * How many resources the importer has to fill in. Every shipped resource arrives + * as an empty stub — the hub never publishes resource values — so a non-zero + * count means the setup step has something to offer. + * + * Only the required ones: an unrequired stub exists so a standalone run of the + * item that declared its type has something to pick, and nothing in the project + * reads it. Counting those would open a setup step listing nothing. */ get resourceCount(): number { - return this.#export?.resources?.length ?? 0 + return (this.#export?.resources ?? []).filter(isRequiredResource).length } get extraCounts(): { triggers: number; migrations: number } | undefined { @@ -453,9 +458,7 @@ export class ImportExecution { const problems: string[] = [] if (failed > 0) problems.push(`${failed} item${failed === 1 ? '' : 's'} failed to import`) if (badMigrations > 0) { - problems.push( - `${badMigrations} data table migration${badMigrations === 1 ? '' : 's'} failed` - ) + problems.push(`${badMigrations} data table migration${badMigrations === 1 ? '' : 's'} failed`) } if (problems.length) this.error = `${problems.join(', ')}.` }