mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-05 00:03:08 +00:00
fix: stop app updates from silently converting an app between raw and low-code
This commit is contained in:
+23
@@ -0,0 +1,23 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "SELECT app_version.raw_app FROM app\n JOIN app_version ON app_version.id = app.versions[array_upper(app.versions, 1)]\n WHERE app.path = $1 AND app.workspace_id = $2",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "raw_app",
|
||||
"type_info": "Bool"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Text",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": [
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "221ebaace16df131a8a75de233869bcce087fc5630dc74c62831847959f2f345"
|
||||
}
|
||||
@@ -335,3 +335,102 @@ async fn test_public_app_by_custom_path(db: Pool<Postgres>) -> anyhow::Result<()
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// A raw app's kind lives on its version row, so a value deployed through the
|
||||
/// low-code endpoint used to convert the app in place and strand its bundle.
|
||||
#[sqlx::test(migrations = "../migrations", fixtures("base"))]
|
||||
async fn test_raw_app_kind_is_not_flipped_by_update(db: Pool<Postgres>) -> anyhow::Result<()> {
|
||||
initialize_tracing().await;
|
||||
let server = ApiServer::start(db.clone()).await?;
|
||||
let port = server.addr.port();
|
||||
let base = format!("http://localhost:{port}/api/w/test-workspace/apps");
|
||||
let raw_path = "u/test-user/raw_app";
|
||||
let low_code_path = "u/test-user/low_code_app";
|
||||
|
||||
let raw_app_form = |path: &str| {
|
||||
reqwest::multipart::Form::new()
|
||||
.text(
|
||||
"app",
|
||||
json!({
|
||||
"path": path,
|
||||
"summary": "Raw app",
|
||||
"value": { "files": { "index.ts": "export {}" }, "runnables": {} },
|
||||
"policy": { "execution_mode": "publisher", "triggerables_v2": {} }
|
||||
})
|
||||
.to_string(),
|
||||
)
|
||||
.part(
|
||||
"js",
|
||||
reqwest::multipart::Part::bytes(b"console.log(1)".to_vec()),
|
||||
)
|
||||
};
|
||||
|
||||
let resp = authed(client().post(format!("{base}/create_raw")))
|
||||
.multipart(raw_app_form(raw_path))
|
||||
.send()
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(resp.status(), 201, "create_raw: {}", resp.text().await?);
|
||||
|
||||
let resp = authed(client().post(format!("{base}/create")))
|
||||
.json(&new_app(low_code_path, "Low code app"))
|
||||
.send()
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(resp.status(), 201, "create: {}", resp.text().await?);
|
||||
|
||||
// Neither endpoint may deploy a value onto an app of the other kind.
|
||||
let resp = authed(client().post(format!("{base}/update/{raw_path}")))
|
||||
.json(&json!({ "value": { "grid": [] } }))
|
||||
.send()
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(resp.status(), 400);
|
||||
assert!(
|
||||
resp.text().await?.contains("is a raw app"),
|
||||
"expected the low-code update of a raw app to be refused"
|
||||
);
|
||||
|
||||
let resp = authed(client().post(format!("{base}/update_raw/{low_code_path}")))
|
||||
.multipart(raw_app_form(low_code_path))
|
||||
.send()
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(resp.status(), 400);
|
||||
assert!(
|
||||
resp.text().await?.contains("is a low-code app"),
|
||||
"expected the raw update of a low-code app to be refused"
|
||||
);
|
||||
|
||||
// Metadata-only updates and same-kind deploys still go through.
|
||||
let resp = authed(client().post(format!("{base}/update/{raw_path}")))
|
||||
.json(&json!({ "summary": "Renamed" }))
|
||||
.send()
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(resp.status(), 200, "update summary: {}", resp.text().await?);
|
||||
|
||||
let resp = authed(client().post(format!("{base}/update_raw/{raw_path}")))
|
||||
.multipart(raw_app_form(raw_path))
|
||||
.send()
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(resp.status(), 200, "update_raw: {}", resp.text().await?);
|
||||
|
||||
let resp = authed_get(port, "get/p", raw_path).await;
|
||||
assert_eq!(resp.json::<serde_json::Value>().await?["raw_app"], true);
|
||||
|
||||
// A caller that means to convert says so, which is how an app converted by
|
||||
// accident gets restored to what it was.
|
||||
let resp = authed(client().post(format!("{base}/update/{raw_path}")))
|
||||
.json(&json!({ "value": { "grid": [] }, "allow_kind_change": true }))
|
||||
.send()
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(resp.status(), 200, "update: {}", resp.text().await?);
|
||||
|
||||
let resp = authed_get(port, "get/p", raw_path).await;
|
||||
assert_eq!(resp.json::<serde_json::Value>().await?["raw_app"], false);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -12302,6 +12302,9 @@ paths:
|
||||
skip_draft_deletion:
|
||||
type: boolean
|
||||
description: "When true (set by the CLI / git sync), deploying this app does not delete an existing user draft at the same path."
|
||||
allow_kind_change:
|
||||
type: boolean
|
||||
description: "When true, this deploy may switch the app between low-code and raw. Without it, deploying a value to an app of the other kind is refused so an app is never converted by accident."
|
||||
responses:
|
||||
"200":
|
||||
description: app updated
|
||||
@@ -12351,6 +12354,9 @@ paths:
|
||||
skip_draft_deletion:
|
||||
type: boolean
|
||||
description: "When true (set by the CLI / git sync), deploying this app does not delete an existing user draft at the same path."
|
||||
allow_kind_change:
|
||||
type: boolean
|
||||
description: "When true, this deploy may switch the app between low-code and raw. Without it, deploying a value to an app of the other kind is refused so an app is never converted by accident."
|
||||
js:
|
||||
type: string
|
||||
css:
|
||||
|
||||
@@ -365,6 +365,12 @@ pub struct EditApp {
|
||||
/// Transient — never persisted.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub skip_draft_deletion: Option<bool>,
|
||||
/// Caller-intent flag: when true this deploy may switch the app between
|
||||
/// low-code and raw. Only a deliberate conversion sets it (restoring a
|
||||
/// version from the app's other-kind history); every other write is
|
||||
/// refused rather than converted. Transient — never persisted.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub allow_kind_change: Option<bool>,
|
||||
}
|
||||
|
||||
#[derive(Serialize, FromRow)]
|
||||
@@ -2419,6 +2425,36 @@ async fn update_app_internal<'a>(
|
||||
|
||||
let mut tx = user_db.clone().begin(&authed).await?;
|
||||
|
||||
// `app_version.raw_app` is whatever the endpoint that wrote the version says
|
||||
// it is, so deploying a value through the wrong one converts the app in
|
||||
// place: a raw app updated via /apps/update becomes a low-code app whose
|
||||
// value no editor can render, and its js/css bundle (keyed on the previous
|
||||
// version id) is orphaned. Refuse instead of letting the write land — the
|
||||
// caller reached for the wrong endpoint, unless it says otherwise.
|
||||
if ns.value.is_some() && !ns.allow_kind_change.unwrap_or(false) {
|
||||
let deployed_raw_app = sqlx::query_scalar!(
|
||||
"SELECT app_version.raw_app FROM app
|
||||
JOIN app_version ON app_version.id = app.versions[array_upper(app.versions, 1)]
|
||||
WHERE app.path = $1 AND app.workspace_id = $2",
|
||||
path,
|
||||
w_id
|
||||
)
|
||||
.fetch_optional(&mut *tx)
|
||||
.await?;
|
||||
if deployed_raw_app.is_some_and(|deployed| deployed != raw_app) {
|
||||
let (kind, endpoint) = if raw_app {
|
||||
("a low-code app", "/apps/update")
|
||||
} else {
|
||||
("a raw app", "/apps/update_raw")
|
||||
};
|
||||
return Err(Error::BadRequest(format!(
|
||||
"App {path} is {kind}: deploying a value to it through the other kind's endpoint \
|
||||
would convert it and strand its bundle. Use {endpoint} instead, or set \
|
||||
allow_kind_change to convert it on purpose."
|
||||
)));
|
||||
}
|
||||
}
|
||||
|
||||
let mut preserved_on_behalf_of: Option<String> = None;
|
||||
let npath = if ns.policy.is_some()
|
||||
|| ns.path.is_some()
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
import { AppService } from '$lib/gen'
|
||||
import { workspaceStore } from '$lib/stores'
|
||||
import { sendUserToast } from '$lib/toast'
|
||||
import { deployRawAppValue } from '$lib/rawAppDeploy'
|
||||
import DeploymentHistory from './DeploymentHistory.svelte'
|
||||
|
||||
interface Props {
|
||||
@@ -16,14 +17,32 @@
|
||||
historyBrowserDrawerOpen = true
|
||||
}
|
||||
|
||||
// Picking a version is an explicit choice of what the app should become, so a
|
||||
// version from before the app changed kind is allowed to convert it back —
|
||||
// that is the only supported way to undo an accidental conversion.
|
||||
async function updateApp(app: any) {
|
||||
await AppService.updateApp({
|
||||
workspace: $workspaceStore!,
|
||||
path: app.path,
|
||||
requestBody: {
|
||||
...app
|
||||
}
|
||||
})
|
||||
if (app.raw_app) {
|
||||
// Restoring a raw app version means re-bundling its sources: `updateApp`
|
||||
// would write a low-code version and leave the bundle behind.
|
||||
await deployRawAppValue({
|
||||
workspace: $workspaceStore!,
|
||||
path: app.path,
|
||||
value: app.value,
|
||||
summary: app.summary,
|
||||
policy: app.policy,
|
||||
customPath: app.custom_path,
|
||||
allowKindChange: true
|
||||
})
|
||||
} else {
|
||||
await AppService.updateApp({
|
||||
workspace: $workspaceStore!,
|
||||
path: app.path,
|
||||
requestBody: {
|
||||
...app,
|
||||
allow_kind_change: true
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
historyBrowserDrawerOpen = false
|
||||
}
|
||||
@@ -32,9 +51,13 @@
|
||||
<Drawer bind:open={historyBrowserDrawerOpen} size="1200px">
|
||||
<DrawerContent title="Deployment History" on:close={() => (historyBrowserDrawerOpen = false)}>
|
||||
<DeploymentHistory
|
||||
on:restore={(e) => {
|
||||
sendUserToast('App restored from previous deployment')
|
||||
updateApp(e.detail)
|
||||
on:restore={async (e) => {
|
||||
try {
|
||||
await updateApp(e.detail)
|
||||
sendUserToast('App restored from previous deployment')
|
||||
} catch (err: any) {
|
||||
sendUserToast(`Could not restore app: ${err.body ?? err.message}`, true)
|
||||
}
|
||||
}}
|
||||
{appPath}
|
||||
on:close={() => {
|
||||
|
||||
@@ -11,6 +11,7 @@
|
||||
import { userStore, workspaceStore } from '$lib/stores'
|
||||
import { createEventDispatcher } from 'svelte'
|
||||
import { Globe, Loader2, Save } from 'lucide-svelte'
|
||||
import { deployRawAppValue } from '$lib/rawAppDeploy'
|
||||
|
||||
let jsonViewerDrawer: Drawer | undefined = $state()
|
||||
|
||||
@@ -65,6 +66,21 @@
|
||||
})
|
||||
dispatch('change')
|
||||
sendUserToast('Draft saved')
|
||||
} else if (isRawApp) {
|
||||
// A raw app's value holds its sources, not a renderable grid: it has to be
|
||||
// re-bundled and written through the raw endpoint. `updateApp` would flag
|
||||
// the new version low-code and leave the bundle on the old one.
|
||||
await deployRawAppValue({
|
||||
workspace: $workspaceStore!,
|
||||
path,
|
||||
value: parsed,
|
||||
summary: app?.summary,
|
||||
policy: app?.policy,
|
||||
customPath: app?.custom_path
|
||||
})
|
||||
dispatch('change')
|
||||
UserDraft.remove('raw_app', path)
|
||||
sendUserToast('App deployed')
|
||||
} else {
|
||||
await AppService.updateApp({
|
||||
workspace: $workspaceStore!,
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
/**
|
||||
* Deploy a raw app (code-based app) from its server-side draft. Raw apps can't
|
||||
* be deployed through the normal AppService.updateApp/createApp path: their
|
||||
* source `files` must be bundled to js/css and saved via the raw-app endpoints.
|
||||
* Deploy a raw app (code-based app), from its server-side draft or from an
|
||||
* explicit value. Raw apps can't be deployed through the normal
|
||||
* AppService.updateApp/createApp path: their source `files` must be bundled to
|
||||
* js/css and saved via the raw-app endpoints.
|
||||
*
|
||||
* This mirrors how the global AI chat deploys raw apps
|
||||
* (`copilot/chat/global/core.ts` → deployDraft, case 'app'): read the item with
|
||||
@@ -18,6 +19,70 @@ import type { AppDraftValue } from '$lib/components/copilot/chat/global/workspac
|
||||
import { updateRawAppPolicy } from '$lib/components/raw_apps/rawAppPolicy'
|
||||
import { DEFAULT_DATA as DEFAULT_RAW_APP_DATA } from '$lib/components/raw_apps/dataTableRefUtils'
|
||||
import { appSourceToDraftValue } from '$lib/components/raw_apps/rawAppDraftValue'
|
||||
import { stateSnapshot } from '$lib/svelte5Utils.svelte'
|
||||
|
||||
/**
|
||||
* Deploy an explicit raw-app value — one the user edited as JSON, or one
|
||||
* restored from a previous version — onto an already deployed raw app. Going
|
||||
* through AppService.updateApp instead would write a version flagged low-code
|
||||
* and leave the bundle behind on the old version, so the app stops rendering;
|
||||
* the backend rejects that mismatch.
|
||||
*/
|
||||
export async function deployRawAppValue({
|
||||
workspace,
|
||||
path,
|
||||
value,
|
||||
summary,
|
||||
policy: currentPolicy,
|
||||
customPath,
|
||||
deploymentMessage,
|
||||
allowKindChange
|
||||
}: {
|
||||
workspace: string
|
||||
path: string
|
||||
value: any
|
||||
summary?: string
|
||||
policy?: Policy
|
||||
customPath?: string | null
|
||||
deploymentMessage?: string
|
||||
/** Let this deploy turn a low-code app into a raw one (restoring a version
|
||||
* from before an accidental conversion). Off for every ordinary deploy. */
|
||||
allowKindChange?: boolean
|
||||
}): Promise<void> {
|
||||
// The value often comes straight out of a `$state` field, and the bundler
|
||||
// runs in an iframe: postMessage refuses to clone a state proxy.
|
||||
const plainValue = stateSnapshot(value) as any
|
||||
const files = (plainValue?.files ?? {}) as Record<string, string>
|
||||
const runnables = plainValue?.runnables ?? {}
|
||||
// The value carries the runnables, so the policy's triggerables have to be
|
||||
// recomputed from it or the deployed app can't call what it now contains.
|
||||
const policy = (await updateRawAppPolicy(runnables, currentPolicy)) as Policy
|
||||
if (!policy.execution_mode) {
|
||||
policy.execution_mode = 'publisher'
|
||||
}
|
||||
|
||||
const bundle = await bundleRawAppDraft({ workspace, files })
|
||||
|
||||
const isAdmin = !!(get(userStore)?.is_admin || get(userStore)?.is_super_admin)
|
||||
await AppService.updateAppRaw({
|
||||
workspace,
|
||||
path,
|
||||
formData: {
|
||||
app: {
|
||||
value: { files, runnables, data: plainValue?.data ?? { ...DEFAULT_RAW_APP_DATA } },
|
||||
summary: summary ?? '',
|
||||
policy,
|
||||
deployment_message: deploymentMessage,
|
||||
// custom_path changes require admin (see deployRawAppDraft).
|
||||
custom_path: isAdmin ? (customPath ?? '') : undefined,
|
||||
preserve_on_behalf_of: policy.on_behalf_of ? true : undefined,
|
||||
allow_kind_change: allowKindChange || undefined
|
||||
},
|
||||
js: bundle.js,
|
||||
css: bundle.css
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
* Promote a raw app's draft to deployed. Throws on failure (caller wraps into a
|
||||
|
||||
Reference in New Issue
Block a user