mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 16:02:36 +00:00
fix: stop publishing a duplicate resource stub per resource-typed input
A project's resource list answered two questions at once. A `$res:` token (or a trigger's resource field) names a path something in the project reads, so that credential has to be filled in. An item's `resource-<type>` input names no path — it says the project needs a Postgres — and publish minted a stub at `f/<slug>/<type>` to carry it. Flattening the two meant an app pinning `$res:.../prod_db` for a script arg declared as `resource-postgresql` published both `prod_db` and `postgresql`, and the importer filled two credentials to satisfy one dependency. Input-derived stubs now ship unrequired: still created, so a standalone run of the item that declared the type has something to pick, but kept off the import wizard's credential checklist. A path claimed by both stays required. The flag defaults to true on the Hub, so projects published before the distinction keep asking for all of their resources; republishing demotes them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MtNNB49VfeJgxAn3YNS2Cq
This commit is contained in:
co-authored by
Claude Opus 5
parent
aa4a6ffd66
commit
c5a8b5cf1b
@@ -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-<type>` 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)]
|
||||
|
||||
@@ -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}<br /><br />{/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}
|
||||
</Alert>
|
||||
{:else}
|
||||
|
||||
@@ -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 }])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -55,6 +55,44 @@ export function inputResourceTypes(schema: unknown, known: Set<string> | 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-<type>` format names no path at all — it says the project
|
||||
* needs a type, and a stub gets minted at the conventional `f/<slug>/<type>` 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<string, ProjectResourceExport>()
|
||||
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
|
||||
|
||||
@@ -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/<slug>/<type> stub.
|
||||
const stubsByPath = new Map<string, { path: string; resource_type: string }>()
|
||||
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 })
|
||||
|
||||
@@ -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-<type>` 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/<fromSlug>/...` -> `f/<folder>/...`. Only enumerated
|
||||
// paths go in, so rewriters touch real refs, never incidental text.
|
||||
export function buildRetargetMap(
|
||||
|
||||
@@ -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(', ')}.`
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user