From 1b3bde79914d3411071f6bfdc78b01f77116b62b Mon Sep 17 00:00:00 2001 From: ditadi Date: Mon, 12 Oct 2026 13:41:32 +0100 Subject: [PATCH 1/8] feat(appkit): add appkit db verify `appkit db verify` checks, read-only, that the branch .env points at has every table and column config/database/schema.ts declares and every committed migration. It also reports the migrations the migrator would never run there: merged out of order, edited after they ran, or run from a checkout that no longer commits them. With --deployed it checks a databricks.yml target's app through the Databricks API only: the app exists, its postgres resource names the target's branch and database with CAN_CONNECT_AND_CREATE, its service principal has a role there, it is running, and its active deployment has every committed migration. -t picks the target. Verify refuses when the target's workspace.host is not the workspace the shell is authenticated against. It reads no database, so it needs no role on the deploy branch. databricks.yml is read once for the guard, init and verify. The reader follows include:, resolves ${var.*}, ${bundle.target} and ${bundle.name}, reads the branch on an app's postgres resource, and lists every target. Signed-off-by: ditadi --- .../appkit/src/database/migrations/apply.ts | 3 + .../appkit/src/database/migrations/history.ts | 31 +- .../migrations/tests/apply.pg.test.ts | 2 + .../database/migrations/tests/history.test.ts | 15 + .../appkit/src/database/tooling/context.ts | 2 +- packages/appkit/src/database/tooling/index.ts | 1 + .../appkit/src/database/tooling/migrate.ts | 1 + .../appkit/src/database/tooling/targets.ts | 290 ++++-- .../database/tooling/tests/contract.test.ts | 1 + .../src/database/tooling/tests/fakes.ts | 6 + .../database/tooling/tests/targets.test.ts | 200 ++++- .../src/database/tooling/tests/verify.test.ts | 822 ++++++++++++++++++ .../appkit/src/database/tooling/verify.ts | 519 +++++++++++ .../src/plugins/database/schema-check.ts | 45 +- .../database/tests/schema-check.test.ts | 41 +- packages/shared/src/cli/commands/db/index.ts | 4 +- .../cli/commands/db/init-clean-seed.test.ts | 3 +- packages/shared/src/cli/commands/db/output.ts | 82 ++ .../src/cli/commands/db/tooling-contract.ts | 36 + .../shared/src/cli/commands/db/verify.test.ts | 297 +++++++ packages/shared/src/cli/commands/db/verify.ts | 63 ++ 21 files changed, 2385 insertions(+), 79 deletions(-) create mode 100644 packages/appkit/src/database/tooling/tests/verify.test.ts create mode 100644 packages/appkit/src/database/tooling/verify.ts create mode 100644 packages/shared/src/cli/commands/db/verify.test.ts create mode 100644 packages/shared/src/cli/commands/db/verify.ts diff --git a/packages/appkit/src/database/migrations/apply.ts b/packages/appkit/src/database/migrations/apply.ts index 14d08800c..b87689325 100644 --- a/packages/appkit/src/database/migrations/apply.ts +++ b/packages/appkit/src/database/migrations/apply.ts @@ -582,6 +582,8 @@ interface MigrationDrift { readonly pending: readonly string[]; /** Committed, not recorded, older than the last recorded one. */ readonly stranded: readonly string[]; + /** Recorded, but the committed file changed since. */ + readonly edited: readonly string[]; /** Recorded rows no committed migration matches: the branch is ahead of the checkout. */ readonly unknown: number; } @@ -592,6 +594,7 @@ function driftOf( return { pending: tags(history.pending), stranded: tags(history.stranded), + edited: tags(history.edited), unknown: history.unknown.length, }; } diff --git a/packages/appkit/src/database/migrations/history.ts b/packages/appkit/src/database/migrations/history.ts index c7ca35cda..bf3583c57 100644 --- a/packages/appkit/src/database/migrations/history.ts +++ b/packages/appkit/src/database/migrations/history.ts @@ -32,29 +32,38 @@ interface MigrationHistory { /** * Match each committed migration with a recorded row by `when` = `created_at`, - * the pair drizzle-orm's migrator writes, then compare hashes. + * the pair drizzle-orm's migrator writes, then compare hashes. Two entries can + * share a `when` after a hand edit or a merge, so each `when` keeps every hash + * recorded at it. */ export function reconcileMigrations( files: readonly MigrationFile[], recorded: readonly RecordedMigration[], ): MigrationHistory { - const hashAt = new Map(); + const hashesAt = new Map>(); for (const row of recorded) { - if (row.createdAt !== null) hashAt.set(row.createdAt, row.hash); + if (row.createdAt === null) continue; + const hashes = hashesAt.get(row.createdAt) ?? new Set(); + hashes.add(row.hash); + hashesAt.set(row.createdAt, hashes); } - const lastRecorded = hashAt.size > 0 ? Math.max(...hashAt.keys()) : null; + const lastRecorded = hashesAt.size > 0 ? Math.max(...hashesAt.keys()) : null; const pending: MigrationFile[] = []; const stranded: MigrationFile[] = []; const edited: MigrationFile[] = []; for (const file of files) { - const hash = hashAt.get(file.when); - if (hash === undefined) { - if (lastRecorded === null || file.when > lastRecorded) pending.push(file); - else stranded.push(file); - } else if (hash !== file.hash) { - edited.push(file); - } + const hashes = hashesAt.get(file.when); + if (hashes?.has(file.hash)) continue; + // A row at this `when` that is another entry's: this one never ran. + const sibling = files.some( + (other) => + other !== file && other.when === file.when && hashes?.has(other.hash), + ); + if (hashes && !sibling) edited.push(file); + else if (lastRecorded === null || file.when > lastRecorded) { + pending.push(file); + } else stranded.push(file); } const committed = new Set(files.map((file) => file.when)); const unknown = recorded.filter( diff --git a/packages/appkit/src/database/migrations/tests/apply.pg.test.ts b/packages/appkit/src/database/migrations/tests/apply.pg.test.ts index c3b2d7b99..faa758222 100644 --- a/packages/appkit/src/database/migrations/tests/apply.pg.test.ts +++ b/packages/appkit/src/database/migrations/tests/apply.pg.test.ts @@ -442,6 +442,7 @@ describe.skipIf(!url)("applyPendingMigrations against Postgres", () => { expect(await readMigrationDrift(newPool(), options(folder))).toEqual({ pending: ["0000_notes", "0001_introspect"], stranded: [], + edited: [], unknown: 0, }); @@ -450,6 +451,7 @@ describe.skipIf(!url)("applyPendingMigrations against Postgres", () => { expect(await readMigrationDrift(newPool(), options(folder))).toEqual({ pending: ["0001_introspect"], stranded: [], + edited: [], unknown: 1, }); }); diff --git a/packages/appkit/src/database/migrations/tests/history.test.ts b/packages/appkit/src/database/migrations/tests/history.test.ts index 04da4ac6e..367dd605a 100644 --- a/packages/appkit/src/database/migrations/tests/history.test.ts +++ b/packages/appkit/src/database/migrations/tests/history.test.ts @@ -72,4 +72,19 @@ describe("reconcileMigrations", () => { expect(history.pending).toEqual([]); expect(history.stranded).toEqual([]); }); + + test("two entries with one `when` keep their own rows: both recorded is clean, one recorded strands the other", () => { + const twins = [file("0000_a", 1000), file("0001_twin", 1000)]; + const both = reconcileMigrations(twins, [ + { hash: "hash-0000_a", createdAt: 1000 }, + { hash: "hash-0001_twin", createdAt: 1000 }, + ]); + expect([...both.pending, ...both.stranded, ...both.edited]).toEqual([]); + + const one = reconcileMigrations(twins, [ + { hash: "hash-0000_a", createdAt: 1000 }, + ]); + expect(tags(one.stranded)).toEqual(["0001_twin"]); + expect(one.edited).toEqual([]); + }); }); diff --git a/packages/appkit/src/database/tooling/context.ts b/packages/appkit/src/database/tooling/context.ts index 1f479f942..93daf4b2c 100644 --- a/packages/appkit/src/database/tooling/context.ts +++ b/packages/appkit/src/database/tooling/context.ts @@ -28,7 +28,7 @@ export interface ToolingWorkspaceClient { readonly currentUser: { me(request: { attributes?: string }): Promise<{ userName?: string }>; }; - /** The resolved workspace host, which init writes to a new .env. */ + /** The resolved workspace host, which init writes to a new .env and verify compares with a target's host. */ getHost(): Promise; } diff --git a/packages/appkit/src/database/tooling/index.ts b/packages/appkit/src/database/tooling/index.ts index c355b52c2..d4dd00b32 100644 --- a/packages/appkit/src/database/tooling/index.ts +++ b/packages/appkit/src/database/tooling/index.ts @@ -28,3 +28,4 @@ export { export { type MigrateOptions, type MigrateReport, runMigrate } from "./migrate"; export { runSeed } from "./seed"; export type { BranchMode } from "./targets"; +export { runVerify, type VerifyOptions, type VerifyReport } from "./verify"; diff --git a/packages/appkit/src/database/tooling/migrate.ts b/packages/appkit/src/database/tooling/migrate.ts index 985e1b147..a67c44ca5 100644 --- a/packages/appkit/src/database/tooling/migrate.ts +++ b/packages/appkit/src/database/tooling/migrate.ts @@ -35,6 +35,7 @@ export function hasSchemaFile(root: string): boolean { const WITHOUT_SCHEMA = { migrate: "there is nothing to migrate", clean: "clean cannot tell which schema to drop", + verify: "there is nothing to verify", } as const; /** Without schema.ts, point to introspect instead of a load error. */ diff --git a/packages/appkit/src/database/tooling/targets.ts b/packages/appkit/src/database/tooling/targets.ts index b359e9364..dac93e276 100644 --- a/packages/appkit/src/database/tooling/targets.ts +++ b/packages/appkit/src/database/tooling/targets.ts @@ -79,10 +79,37 @@ export function readLocalTarget(env: NodeJS.ProcessEnv): LocalTarget { }; } -/** The branch the bundle's default target deploys to. */ -export interface DeployTarget { - readonly branch: string; - readonly database: string | null; +/** An app one target of databricks.yml deploys. */ +export interface BundleApp { + /** Its key under resources.apps. */ + readonly key: string; + /** Its name as written; null when it has none. */ + readonly declaredName: string | null; + /** Its name with bundle references resolved; null when one needs a deploy to resolve. */ + readonly name: string | null; + /** Its postgres resource; null when it has none. */ + readonly postgres: { + readonly branch: string | null; + readonly database: string | null; + } | null; +} + +/** One target of databricks.yml, with what the files alone can resolve resolved. */ +export interface BundleTarget { + readonly name: string; + readonly isDefault: boolean; + /** workspace.host of the target, else of the bundle. */ + readonly host: string | null; + readonly apps: readonly BundleApp[]; + /** The target's postgres_branch and postgres_database variables. */ + readonly postgresBranch: string | null; + readonly postgresDatabase: string | null; +} + +interface Bundle { + /** databricks.yml or databricks.yaml. */ + readonly file: string; + readonly targets: readonly BundleTarget[]; } type Yaml = Record | undefined; @@ -93,12 +120,8 @@ function record(value: unknown): Yaml { : undefined; } -/** databricks.yml, parsed; null without one. Invalid YAML throws, so no check that reads it passes silently. */ -function readBundle(root: string): Record | null { - const file = ["databricks.yml", "databricks.yaml"] - .map((name) => path.join(root, name)) - .find((candidate) => fs.existsSync(candidate)); - if (!file) return null; +/** Invalid YAML throws, so no check that reads the bundle passes silently. */ +function parseYaml(file: string): Record { try { return record(yaml.load(fs.readFileSync(file, "utf-8"))) ?? {}; } catch (error) { @@ -110,59 +133,220 @@ function readBundle(root: string): Record | null { } } -/** A target's value for a bundle variable, else the variable's default. */ -function variableOf( +/** Files an `include:` pattern names. `*` and `?` match within one path segment, as in the Databricks CLI. */ +function includedFiles(root: string, pattern: string): string[] { + let paths = [root]; + for (const segment of pattern.split("/")) { + if (!segment || segment === ".") continue; + if (!/[*?]/.test(segment)) { + paths = paths.map((dir) => path.join(dir, segment)); + continue; + } + const matcher = new RegExp( + `^${segment + .replace(/[.+^${}()|[\]\\]/g, "\\$&") + .replace(/\*/g, ".*") + .replace(/\?/g, ".")}$`, + ); + paths = paths.flatMap((dir) => { + try { + return fs + .readdirSync(dir) + .filter((entry) => matcher.test(entry)) + .map((entry) => path.join(dir, entry)); + } catch { + return []; + } + }); + } + return paths.filter((file) => + fs.statSync(file, { throwIfNoEntry: false })?.isFile(), + ); +} + +/** Mappings merge key by key; lists of named entries (an app's resources) merge by name; anything else is replaced. */ +function merge(base: unknown, override: unknown): unknown { + const left = record(base); + const right = record(override); + if (left && right) { + const merged: Record = { ...left }; + for (const [key, value] of Object.entries(right)) { + merged[key] = key in left ? merge(left[key], value) : value; + } + return merged; + } + if (Array.isArray(base) && Array.isArray(override)) { + const named = (items: unknown[]) => + items.every((item) => typeof record(item)?.name === "string"); + if (named(base) && named(override)) { + const byName = new Map( + base.map((item) => [record(item)?.name as string, item]), + ); + for (const item of override) { + const name = record(item)?.name as string; + byName.set(name, merge(byName.get(name), item)); + } + return [...byName.values()]; + } + } + return override; +} + +const REFERENCE = /\$\{([^}]+)\}/g; +const MAX_REFERENCE_DEPTH = 10; + +/** Resolves `${var.*}`, `${bundle.target}` and `${bundle.name}` for one target; null when a reference needs a deploy to resolve. */ +function resolverFor( bundle: Record, target: Yaml, - name: string, -): string | null { - const value = - record(target?.variables)?.[name] ?? - record(record(bundle.variables)?.[name])?.default ?? - null; - return typeof value === "string" && value ? value : null; + targetName: string, +): (value: unknown) => string | null { + const variable = (name: string): unknown => { + const override = record(target?.variables)?.[name]; + if (override !== undefined) return record(override)?.default ?? override; + return record(record(bundle.variables)?.[name])?.default; + }; + const resolve = (value: unknown, depth: number): string | null => { + if (typeof value !== "string" || depth > MAX_REFERENCE_DEPTH) return null; + let unresolved = false; + const text = value.replace(REFERENCE, (_, reference: string) => { + const resolved = reference.startsWith("var.") + ? resolve(variable(reference.slice("var.".length)), depth + 1) + : reference === "bundle.target" + ? targetName + : reference === "bundle.name" + ? resolve(record(bundle.bundle)?.name, depth + 1) + : null; + if (resolved === null) unresolved = true; + return resolved ?? ""; + }); + return unresolved || !text ? null : text; + }; + return (value) => resolve(value, 0); +} + +function bundleApps( + apps: Record, + resolve: (value: unknown) => string | null, +): BundleApp[] { + return Object.entries(apps).map(([key, value]) => { + const app = record(value); + const declared = app?.name; + const resources = Array.isArray(app?.resources) ? app.resources : []; + const postgres = resources + .map((resource) => record(record(resource)?.postgres)) + .find((binding) => binding !== undefined); + return { + key, + declaredName: typeof declared === "string" && declared ? declared : null, + name: resolve(declared), + postgres: postgres + ? { + branch: + parseResourceName(resolve(postgres.branch) ?? undefined) + ?.branch ?? null, + database: + parseResourceName(resolve(postgres.database) ?? undefined) + ?.database ?? null, + } + : null, + }; + }); } -/** Default target's postgres_branch / postgres_database variables; null without databricks.yml or binding. */ +/** + * databricks.yml, with the files its `include:` names merged in, per target; + * null without one. Without `targets:` the bundle has one target, "default". + */ +export function readBundle(root: string): Bundle | null { + const file = ["databricks.yml", "databricks.yaml"] + .map((name) => path.join(root, name)) + .find((candidate) => fs.existsSync(candidate)); + if (!file) return null; + const main = parseYaml(file); + const includes = Array.isArray(main.include) ? main.include : []; + const bundle = record( + includes + .filter((pattern): pattern is string => typeof pattern === "string") + .flatMap((pattern) => includedFiles(root, pattern)) + .reduce( + (merged, included) => merge(merged, parseYaml(included)), + main, + ), + ) as Record; + + const declared = Object.entries(record(bundle.targets) ?? {}); + const targets: Array<[string, Yaml]> = + declared.length > 0 + ? declared.map(([name, target]) => [name, record(target)]) + : [["default", { default: true }]]; + return { + file: path.basename(file), + targets: targets.map(([name, target]) => { + const resolve = resolverFor(bundle, target, name); + const apps = record( + merge( + record(record(bundle.resources)?.apps) ?? {}, + record(record(target?.resources)?.apps) ?? {}, + ), + ); + const variable = (variableName: string) => + resolve(`\${var.${variableName}}`) ?? undefined; + return { + name, + isDefault: target?.default === true, + host: resolve( + record(target?.workspace)?.host ?? record(bundle.workspace)?.host, + ), + apps: bundleApps(apps ?? {}, resolve), + postgresBranch: + parseResourceName(variable("postgres_branch"))?.branch ?? null, + postgresDatabase: + parseResourceName(variable("postgres_database"))?.database ?? null, + }; + }), + }; +} + +/** The target `databricks bundle deploy` picks without -t: the one marked default, else the only one. */ +export function defaultBundleTarget(bundle: Bundle): BundleTarget | null { + return ( + bundle.targets.find((target) => target.isDefault) ?? + (bundle.targets.length === 1 ? (bundle.targets[0] ?? null) : null) + ); +} + +/** Every branch a target deploys to: its apps' postgres resources, and its postgres_branch variable. */ +function targetBranches(target: BundleTarget): string[] { + return [ + ...target.apps.map((app) => app.postgres?.branch), + target.postgresBranch, + ].filter((branch): branch is string => !!branch); +} + +/** The branch and database a target deploys to. */ +export interface DeployTarget { + readonly branch: string; + readonly database: string | null; +} + +/** The default target's branch and database; null without databricks.yml, a default target or a postgres binding. */ export function readDeployTarget(root: string): DeployTarget | null { const bundle = readBundle(root); - if (!bundle) return null; - const targets = record(bundle.targets) ?? {}; - const target = - Object.values(targets) - .map(record) - .find((candidate) => candidate?.default === true) ?? - record(targets.default); - - const branch = parseResourceName( - variableOf(bundle, target, "postgres_branch") ?? undefined, - )?.branch; + const target = bundle ? defaultBundleTarget(bundle) : null; + if (!target) return null; + const bound = target.apps.find((app) => app.postgres?.branch); + const branch = bound?.postgres?.branch ?? target.postgresBranch; if (!branch) return null; - const database = - parseResourceName( - variableOf(bundle, target, "postgres_database") ?? undefined, - )?.database ?? null; - return { branch, database }; + return { + branch, + database: bound?.postgres?.database ?? target.postgresDatabase, + }; } -/** - * Every branch databricks.yml can deploy to: each target's postgres_branch and - * the variable's default. Empty without databricks.yml or a postgres binding. - */ +/** Every branch databricks.yml can deploy to, across targets. Empty without databricks.yml or a postgres binding. */ export function readDeployBranches(root: string): string[] { const bundle = readBundle(root); if (!bundle) return []; - const targets = [ - undefined, - ...Object.values(record(bundle.targets) ?? {}).map(record), - ]; - const branches = targets - .map( - (target) => - parseResourceName( - variableOf(bundle, target, "postgres_branch") ?? undefined, - )?.branch, - ) - .filter((branch): branch is string => !!branch); - return [...new Set(branches)]; + return [...new Set(bundle.targets.flatMap(targetBranches))]; } diff --git a/packages/appkit/src/database/tooling/tests/contract.test.ts b/packages/appkit/src/database/tooling/tests/contract.test.ts index ed47a4f7a..fae9f6560 100644 --- a/packages/appkit/src/database/tooling/tests/contract.test.ts +++ b/packages/appkit/src/database/tooling/tests/contract.test.ts @@ -17,6 +17,7 @@ const contractKeys = { runIntrospect: true, runMigrate: true, runSeed: true, + runVerify: true, } satisfies Record; test("every function the CLI is typed with is exported", () => { diff --git a/packages/appkit/src/database/tooling/tests/fakes.ts b/packages/appkit/src/database/tooling/tests/fakes.ts index d7e829fa4..df31a19fc 100644 --- a/packages/appkit/src/database/tooling/tests/fakes.ts +++ b/packages/appkit/src/database/tooling/tests/fakes.ts @@ -249,6 +249,12 @@ resources: app: name: "my-app" source_code_path: ./ + resources: + - name: postgres + postgres: + branch: \${var.postgres_branch} + database: \${var.postgres_database} + permission: CAN_CONNECT_AND_CREATE targets: default: default: true diff --git a/packages/appkit/src/database/tooling/tests/targets.test.ts b/packages/appkit/src/database/tooling/tests/targets.test.ts index 6be36190e..ad24482f1 100644 --- a/packages/appkit/src/database/tooling/tests/targets.test.ts +++ b/packages/appkit/src/database/tooling/tests/targets.test.ts @@ -3,6 +3,7 @@ import { afterEach, describe, expect, test } from "vitest"; import { parseProject, parseResourceName, + readBundle, readDeployBranches, readDeployTarget, readLocalTarget, @@ -124,6 +125,24 @@ targets: ]); }); + test("a branch set on an app's postgres resource counts, in an included file too", () => { + const root = tempApp({ + "databricks.yml": "include:\n - resources/*.yml\n", + "resources/app.yml": `resources: + apps: + app: + name: shop + resources: + - name: postgres + postgres: + branch: projects/p/branches/production +`, + }); + expect(readDeployBranches(root)).toEqual([ + "projects/p/branches/production", + ]); + }); + test("empty without databricks.yml or without a postgres binding", () => { expect(readDeployBranches(tempApp())).toEqual([]); expect( @@ -140,7 +159,7 @@ targets: }); describe("readDeployTarget", () => { - test("reads the default target's postgres variables", () => { + test("reads the default target's postgres resource", () => { const root = tempApp({ "databricks.yml": databricksYml( "projects/p/branches/production", @@ -169,10 +188,187 @@ targets: }); }); - test("null without databricks.yml or without a postgres binding", () => { + test("null without databricks.yml, a default target or a postgres binding", () => { expect(readDeployTarget(tempApp())).toBeNull(); expect( readDeployTarget(tempApp({ "databricks.yml": "bundle:\n name: x\n" })), ).toBeNull(); + expect( + readDeployTarget( + tempApp({ + "databricks.yml": `variables: + postgres_branch: + default: projects/p/branches/main +targets: + dev: {} + prod: {} +`, + }), + ), + ).toBeNull(); + }); +}); + +describe("readBundle", () => { + test("resolves each target's app, postgres resource and host from its variables", () => { + const root = tempApp({ + "databricks.yml": `bundle: + name: shop +variables: + postgres_branch: + default: projects/p/branches/main + postgres_database: + default: projects/p/branches/main/databases/db + suffix: + default: dev +workspace: + host: https://dev.cloud.databricks.com +resources: + apps: + app: + name: \${bundle.name}-\${var.suffix} + resources: + - name: postgres + postgres: + branch: \${var.postgres_branch} + database: \${var.postgres_database} +targets: + default: + default: true + prod: + workspace: + host: https://prod.cloud.databricks.com + variables: + suffix: \${bundle.target} + postgres_branch: projects/p/branches/production + postgres_database: projects/p/branches/production/databases/db +`, + }); + + expect(readBundle(root)).toEqual({ + file: "databricks.yml", + targets: [ + { + name: "default", + isDefault: true, + host: "https://dev.cloud.databricks.com", + apps: [ + { + key: "app", + declaredName: "${bundle.name}-${var.suffix}", + name: "shop-dev", + postgres: { + branch: "projects/p/branches/main", + database: "projects/p/branches/main/databases/db", + }, + }, + ], + postgresBranch: "projects/p/branches/main", + postgresDatabase: "projects/p/branches/main/databases/db", + }, + { + name: "prod", + isDefault: false, + host: "https://prod.cloud.databricks.com", + apps: [ + { + key: "app", + declaredName: "${bundle.name}-${var.suffix}", + name: "shop-prod", + postgres: { + branch: "projects/p/branches/production", + database: "projects/p/branches/production/databases/db", + }, + }, + ], + postgresBranch: "projects/p/branches/production", + postgresDatabase: "projects/p/branches/production/databases/db", + }, + ], + }); + }); + + test("follows include:, and a target's app override merges resources by name", () => { + const root = tempApp({ + "databricks.yml": `include: + - resources/*.yml +targets: + default: + default: true + resources: + apps: + app: + resources: + - name: postgres + postgres: + branch: projects/p/branches/production +`, + "resources/app.yml": `resources: + apps: + app: + name: shop + resources: + - name: warehouse + sql_warehouse: + id: w + - name: postgres + postgres: + branch: projects/p/branches/main + database: projects/p/branches/main/databases/db +`, + "resources/notes.txt": "not yaml: [", + }); + + const [target] = readBundle(root)?.targets ?? []; + expect(target?.apps).toEqual([ + { + key: "app", + declaredName: "shop", + name: "shop", + postgres: { + branch: "projects/p/branches/production", + database: "projects/p/branches/main/databases/db", + }, + }, + ]); + }); + + test("a reference only a deploy resolves reads as null; no targets is one default target", () => { + const root = tempApp({ + "databricks.yml": `variables: + warehouse: + lookup: + warehouse: Shared +resources: + apps: + app: + name: shop-\${workspace.current_user.short_name} +`, + }); + expect(readBundle(root)?.targets).toEqual([ + { + name: "default", + isDefault: true, + host: null, + apps: [ + { + key: "app", + declaredName: "shop-${workspace.current_user.short_name}", + name: null, + postgres: null, + }, + ], + postgresBranch: null, + postgresDatabase: null, + }, + ]); + }); + + test("an invalid included file throws, naming it", () => { + const root = tempApp({ + "databricks.yml": "include:\n - resources/*.yml\n", + "resources/app.yml": "resources: [\n", + }); + expect(() => readBundle(root)).toThrow(/^Cannot read app\.yml, /); }); }); diff --git a/packages/appkit/src/database/tooling/tests/verify.test.ts b/packages/appkit/src/database/tooling/tests/verify.test.ts new file mode 100644 index 000000000..70d57db23 --- /dev/null +++ b/packages/appkit/src/database/tooling/tests/verify.test.ts @@ -0,0 +1,822 @@ +import { createHash } from "node:crypto"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; + +const { withBranchPool } = vi.hoisted(() => ({ withBranchPool: vi.fn() })); + +vi.mock("../context", async (importOriginal) => ({ + ...(await importOriginal()), + withBranchPool, +})); + +import type { WorkspaceRequest } from "../../../workspace-client"; +import type { DbContext } from "../context"; +import { runVerify, type VerifyOptions } from "../verify"; +import { databricksYml, FakeLakebase } from "./fakes"; + +const builder = path.resolve( + import.meta.dirname, + "../../schema-builder/index.ts", +); +const BRANCH = "projects/p/branches/dev-alice-3f9a2c"; +const ENV = { + PGHOST: "ep-1.example.com", + PGDATABASE: "databricks_postgres", + LAKEBASE_ENDPOINT: `${BRANCH}/endpoints/primary`, +}; +const MIGRATIONS = "config/database/migrations"; +const roots: string[] = []; + +type Migration = { tag: string; when: number }; + +function journal(migrations: Migration[]): string { + return JSON.stringify({ + version: "7", + dialect: "postgresql", + entries: migrations.map((m, idx) => ({ + idx, + version: "7", + when: m.when, + tag: m.tag, + breakpoints: true, + })), + }); +} + +/** An app whose schema.ts declares app.notes(id, body), with the given committed migrations. */ +function app( + migrations: Migration[] = [], + env: NodeJS.ProcessEnv = ENV, +): DbContext { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "appkit-db-verify-")); + roots.push(root); + const file = path.join(root, "config/database/schema.ts"); + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync( + file, + `import { defineSchema, id, text } from ${JSON.stringify(builder)}; + export const schema = defineSchema(({ table }) => ({ + notes: table("notes", { id: id(), body: text() }), + }));`, + ); + if (migrations.length > 0) { + const meta = path.join(root, MIGRATIONS, "meta"); + fs.mkdirSync(meta, { recursive: true }); + fs.writeFileSync(path.join(meta, "_journal.json"), journal(migrations)); + for (const m of migrations) { + fs.writeFileSync( + path.join(root, MIGRATIONS, `${m.tag}.sql`), + `-- ${m.tag}\nselect 1;\n`, + ); + } + } + return { + root, + env, + workspace: () => { + throw new Error("no workspace calls expected"); + }, + api: () => { + throw new Error("no Lakebase API calls expected"); + }, + }; +} + +/** Journal rows as the migrator writes them: the file's sha256 and the journal's `when`. */ +function recordedAs(ctx: DbContext, migrations: Migration[]) { + return migrations.map((m) => ({ + hash: createHash("sha256") + .update( + fs.readFileSync( + path.join(ctx.root, MIGRATIONS, `${m.tag}.sql`), + "utf-8", + ), + ) + .digest("hex"), + created_at: String(m.when), + })); +} + +/** + * A connection answering the catalog and journal queries; it records every + * statement. + */ +function branch(state: { + columns: Record; + /** The journal table's rows; "absent" without the table. */ + recorded: Array<{ hash: string; created_at: string }> | "absent"; + failOn?: RegExp; +}) { + const statements: string[] = []; + const client = { + query: vi.fn(async (text: string, values?: unknown[]) => { + statements.push(text); + if (state.failOn?.test(text)) throw new Error("connection reset"); + if (/pg_catalog\.pg_attribute/.test(text)) { + expect(values?.[0]).toBe("app"); + return { + rows: Object.entries(state.columns).flatMap(([table, columns]) => + columns.map((column) => ({ + table_name: table, + column_name: column, + relation_kind: "r", + })), + ), + }; + } + if (/from pg_catalog\.pg_class c\s+join/.test(text)) { + return { rows: state.recorded === "absent" ? [] : [{ "?column?": 1 }] }; + } + if (/select hash, created_at from/.test(text)) { + return { rows: state.recorded === "absent" ? [] : state.recorded }; + } + return { rows: [] }; + }), + release: vi.fn(), + }; + const pool = { + connect: vi.fn(async () => client), + end: vi.fn(async () => undefined), + }; + withBranchPool.mockImplementation(async (_ctx, fn) => { + try { + return await fn(pool); + } finally { + await pool.end(); + } + }); + return { statements, client, pool }; +} + +const WRITES = /\b(insert|update|delete|create|drop|alter|grant|truncate)\b/i; + +beforeEach(() => { + withBranchPool.mockReset(); +}); + +afterEach(() => { + for (const root of roots.splice(0)) { + fs.rmSync(root, { recursive: true, force: true }); + } +}); + +describe("runVerify", () => { + const two = [ + { tag: "0000_migration", when: 1000 }, + { tag: "0001_migration", when: 2000 }, + ]; + const clean = { + schemaProblems: [], + pendingMigrations: [], + strandedMigrations: [], + editedMigrations: [], + unknownMigrations: 0, + }; + + test("a branch that matches schema.ts and has every migration is ok", async () => { + const ctx = app(two); + const { pool } = branch({ + columns: { notes: ["id", "body"] }, + recorded: recordedAs(ctx, two), + }); + + expect(await runVerify(ctx)).toEqual({ + kind: "local", + branch: BRANCH, + ...clean, + ok: true, + }); + expect(pool.end).toHaveBeenCalledOnce(); + }); + + test("reports schema problems and pending migrations separately", async () => { + const ctx = app(two); + branch({ + columns: { notes: ["id"] }, + recorded: recordedAs(ctx, two.slice(0, 1)), + }); + + expect(await runVerify(ctx)).toMatchObject({ + ...clean, + schemaProblems: ["table app.notes is missing column body"], + pendingMigrations: ["0001_migration"], + ok: false, + }); + }); + + test("without a journal table every committed migration is pending", async () => { + const ctx = app([{ tag: "0000_baseline", when: 1000 }]); + branch({ columns: {}, recorded: "absent" }); + + expect(await runVerify(ctx)).toMatchObject({ + schemaProblems: ["table app.notes does not exist"], + pendingMigrations: ["0000_baseline"], + ok: false, + }); + }); + + test("a migration merged out of order, older than the last one the branch ran, never runs", async () => { + const migrations = [ + { tag: "0000_migration", when: 1000 }, + { tag: "0001_parallel", when: 1500 }, + { tag: "0002_migration", when: 2000 }, + ]; + const ctx = app(migrations); + branch({ + columns: { notes: ["id", "body"] }, + recorded: recordedAs(ctx, [migrations[0], migrations[2]] as Migration[]), + }); + + expect(await runVerify(ctx)).toMatchObject({ + ...clean, + strandedMigrations: ["0001_parallel"], + ok: false, + }); + }); + + test("a migration edited after the branch ran it never runs again", async () => { + const ctx = app(two); + const recorded = recordedAs(ctx, two); + fs.appendFileSync( + path.join(ctx.root, MIGRATIONS, "0000_migration.sql"), + "select 2;\n", + ); + branch({ columns: { notes: ["id", "body"] }, recorded }); + + expect(await runVerify(ctx)).toMatchObject({ + ...clean, + editedMigrations: ["0000_migration"], + ok: false, + }); + }); + + test("a migration the branch ran that this checkout does not commit is reported", async () => { + const ctx = app(two); + branch({ + columns: { notes: ["id", "body"] }, + recorded: [ + ...recordedAs(ctx, two), + { hash: "from-another-git-branch", created_at: "3000" }, + ], + }); + + expect(await runVerify(ctx)).toMatchObject({ + ...clean, + unknownMigrations: 1, + ok: false, + }); + }); + + test("no committed migrations: only the schema is checked", async () => { + const ctx = app(); + branch({ columns: { notes: ["id", "body"] }, recorded: "absent" }); + + expect((await runVerify(ctx)).ok).toBe(true); + }); + + test("never writes: everything runs in a READ ONLY transaction that is rolled back", async () => { + const ctx = app([{ tag: "0000_migration", when: 1000 }]); + const { statements, client } = branch({ + columns: { notes: ["id"] }, + recorded: [], + }); + + await runVerify(ctx); + + expect(statements[0]).toBe("begin transaction read only"); + expect(statements.at(-1)).toBe("rollback"); + expect(statements.filter((s) => WRITES.test(s))).toEqual([]); + expect(client.release).toHaveBeenCalledOnce(); + }); + + test("a failing query rolls back, releases the connection and ends the pool", async () => { + const ctx = app([{ tag: "0000_migration", when: 1000 }]); + const { statements, client, pool } = branch({ + columns: {}, + recorded: [], + failOn: /select hash, created_at/, + }); + + await expect(runVerify(ctx)).rejects.toThrow("connection reset"); + expect(statements.at(-1)).toBe("rollback"); + expect(client.release).toHaveBeenCalledOnce(); + expect(pool.end).toHaveBeenCalledOnce(); + }); + + test("without a configured branch it points to init before connecting", async () => { + await expect(runVerify(app([], {}))).rejects.toThrow( + "Did not verify: no branch is configured in .env (PGHOST, PGDATABASE and LAKEBASE_ENDPOINT); run `appkit db init`.", + ); + expect(withBranchPool).not.toHaveBeenCalled(); + }); + + test("without schema.ts it points to introspect", async () => { + const ctx = app(); + fs.rmSync(path.join(ctx.root, "config/database/schema.ts")); + + await expect(runVerify(ctx)).rejects.toThrow( + "No config/database/schema.ts yet, so there is nothing to verify. Run `appkit db introspect` to write it from the branch's tables, or create it.", + ); + expect(withBranchPool).not.toHaveBeenCalled(); + }); +}); + +const PRODUCTION = "projects/p/branches/production"; +const DATABASE = `${PRODUCTION}/databases/db-1`; +const SP = "11111111-2222-3333-4444-555555555555"; +const HOST = "https://acme.cloud.databricks.com"; +const SNAPSHOT = "/Workspace/Users/sp/src/01f0"; +const DEPLOYED_AT = "2026-10-01T12:00:00Z"; + +/** The app API's response for a deployed, running app bound to the deploy branch. */ +function runningApp(overrides: Record = {}) { + return { + name: "my-app", + service_principal_client_id: SP, + resources: [ + { + name: "postgres", + postgres: { + branch: PRODUCTION, + database: DATABASE, + permission: "CAN_CONNECT_AND_CREATE", + }, + }, + ], + app_status: { state: "RUNNING", message: "App is running." }, + compute_status: { state: "ACTIVE", message: "App compute is running." }, + active_deployment: { + create_time: DEPLOYED_AT, + deployment_artifacts: { source_code_path: SNAPSHOT }, + status: { state: "SUCCEEDED", message: "App started successfully" }, + }, + ...overrides, + }; +} + +/** databricks.yml with a workspace host on its default target. */ +function ymlWithHost(host = HOST): string { + return `${databricksYml(PRODUCTION, DATABASE)} workspace:\n host: ${host}\n`; +} + +/** + * An app that databricks.yml deploys to the deploy branch. The developer has + * no Postgres role there: --deployed must not need one. + */ +function deployedApp( + options: { + app?: Record | "missing" | Error; + yml?: string; + spRole?: boolean; + migrations?: Migration[]; + /** The deployed snapshot's journal; "unreadable" when export fails. */ + deployedJournal?: Migration[] | "unreadable"; + authenticatedHost?: string; + } = {}, +) { + const base = app(options.migrations ?? [], {}); + fs.writeFileSync( + path.join(base.root, "databricks.yml"), + options.yml ?? ymlWithHost(), + ); + const lakebase = new FakeLakebase(); + lakebase.addBranch(PRODUCTION, { default: true }); + if (options.spRole !== false) { + lakebase.addRole(PRODUCTION, `dbrx-apps-${SP}`, SP); + } + lakebase.databases.set(DATABASE, { + name: DATABASE, + status: { postgres_database: "appdb" }, + }); + const requests: string[] = []; + const ctx: DbContext = { + ...base, + workspace: () => ({ + request: async ({ method, path: apiPath, query }: WorkspaceRequest) => { + const appName = /^\/api\/2\.0\/apps\/([^/]+)$/.exec(apiPath)?.[1]; + if (method === "GET" && appName) { + requests.push(`apps.get ${decodeURIComponent(appName)}`); + const response = options.app ?? runningApp(); + if (response === "missing") { + throw Object.assign(new Error("App my-app does not exist."), { + statusCode: 404, + }); + } + if (response instanceof Error) throw response; + return Response.json(response); + } + if (method === "GET" && apiPath === "/api/2.0/workspace/export") { + expect(query?.format).toBe("AUTO"); + requests.push(`workspace.export ${query?.path}`); + const deployed = options.deployedJournal ?? options.migrations ?? []; + if (deployed === "unreadable") { + throw Object.assign(new Error("Permission denied"), { + statusCode: 403, + }); + } + return Response.json({ + content: Buffer.from(journal(deployed)).toString("base64"), + }); + } + throw new Error( + `--deployed only reads the app and its journal, not ${method} ${apiPath}`, + ); + }, + currentUser: { + me: async () => { + throw new Error("--deployed does not need the developer's identity"); + }, + }, + getHost: async () => options.authenticatedHost ?? HOST, + }), + api: () => lakebase.api(), + }; + return { ctx, lakebase, requests }; +} + +const DEPLOYED: VerifyOptions = { deployed: true }; + +/** --deployed's problems; fails when the report is not a deploy report. */ +async function deployProblems( + ctx: DbContext, + options: VerifyOptions = DEPLOYED, +): Promise { + const report = await runVerify(ctx, options); + if (report.kind !== "deploy") throw new Error("expected a deploy report"); + return report.problems; +} + +/** databricks.yml with the given resources block in place of the default app. */ +function ymlWithApps(apps: string): string { + return databricksYml(PRODUCTION, DATABASE).replace( + /resources:\n {2}apps:\n(?: {4}.*\n)+/, + apps, + ); +} + +describe("runVerify --deployed", () => { + test("a running app bound to the deploy branch with every committed migration is ok, with no database connection and no role of yours there", async () => { + const migrations = [{ tag: "0000_migration", when: 1000 }]; + const { ctx, lakebase, requests } = deployedApp({ migrations }); + + expect(await runVerify(ctx, DEPLOYED)).toEqual({ + kind: "deploy", + target: "default", + host: HOST, + branch: PRODUCTION, + app: "my-app", + migrations: { checked: "deployed code", missing: [] }, + problems: [], + ok: true, + }); + expect(requests).toEqual([ + "apps.get my-app", + `workspace.export ${SNAPSHOT}/config/database/migrations/meta/_journal.json`, + ]); + expect(withBranchPool).not.toHaveBeenCalled(); + expect(lakebase.calls.sort()).toEqual([ + `getBranch ${PRODUCTION}`, + `getDatabase ${DATABASE}`, + `listRoles ${PRODUCTION}`, + ]); + }); + + test("a committed migration the active deployment lacks fails it", async () => { + const migrations = [ + { tag: "0000_migration", when: 1000 }, + { tag: "0001_migration", when: 2000 }, + ]; + const { ctx } = deployedApp({ + migrations, + deployedJournal: migrations.slice(0, 1), + }); + + expect(await runVerify(ctx, DEPLOYED)).toMatchObject({ + migrations: { checked: "deployed code", missing: ["0001_migration"] }, + problems: [ + "app my-app's active deployment does not have 0001_migration, which this checkout commits: `databricks apps deploy` deploys it, and the app applies it on start", + ], + ok: false, + }); + }); + + test("when the deployed code cannot be read, migrations generated after the deployment fail it", async () => { + const deployedAt = Date.parse(DEPLOYED_AT); + const { ctx } = deployedApp({ + migrations: [ + { tag: "0000_migration", when: deployedAt - 1000 }, + { tag: "0001_migration", when: deployedAt + 1000 }, + ], + deployedJournal: "unreadable", + }); + + expect(await runVerify(ctx, DEPLOYED)).toMatchObject({ + migrations: { checked: "deploy time", missing: ["0001_migration"] }, + problems: [ + `0001_migration was generated after app my-app's active deployment (${DEPLOYED_AT}), so it does not have it: \`databricks apps deploy\` deploys it, and the app applies it on start`, + ], + }); + }); + + test("without committed migrations nothing is compared, and schema.ts is not needed", async () => { + const { ctx, requests } = deployedApp(); + fs.rmSync(path.join(ctx.root, "config/database/schema.ts")); + + expect(await runVerify(ctx, DEPLOYED)).toMatchObject({ + migrations: null, + ok: true, + }); + expect(requests).toEqual(["apps.get my-app"]); + }); + + test("--target checks that target's app, branch and host", async () => { + const staging = "projects/p/branches/staging"; + const { ctx, lakebase } = deployedApp({ + yml: `${ymlWithHost()} staging: + workspace: + host: ${HOST} + variables: + postgres_branch: ${staging} + postgres_database: ${staging}/databases/db-1 +`, + app: runningApp({ + resources: [ + { + postgres: { + branch: staging, + database: `${staging}/databases/db-1`, + permission: "CAN_CONNECT_AND_CREATE", + }, + }, + ], + }), + }); + lakebase.addBranch(staging); + lakebase.addRole(staging, `dbrx-apps-${SP}`, SP); + lakebase.databases.set(`${staging}/databases/db-1`, { + name: `${staging}/databases/db-1`, + status: { postgres_database: "appdb" }, + }); + + expect( + await runVerify(ctx, { deployed: true, target: "staging" }), + ).toMatchObject({ target: "staging", branch: staging, ok: true }); + }); + + test("an unknown target, or no default target, names the targets there are", async () => { + const { ctx } = deployedApp(); + await expect( + runVerify(ctx, { deployed: true, target: "prod" }), + ).rejects.toThrow( + "databricks.yml has no target prod; its targets are default.", + ); + + const twoTargets = deployedApp({ + yml: ymlWithHost().replace(" default: true\n", "") + " prod: {}\n", + }); + await expect(runVerify(twoTargets.ctx, DEPLOYED)).rejects.toThrow( + "databricks.yml has no default target; pass --target with one of default, prod.", + ); + }); + + test("a shell authenticated against another workspace is refused before any API call", async () => { + const { ctx, lakebase, requests } = deployedApp({ + authenticatedHost: "https://other.cloud.databricks.com", + }); + + await expect(runVerify(ctx, DEPLOYED)).rejects.toThrow( + `Target default deploys to ${HOST}, but this shell is authenticated against https://other.cloud.databricks.com. Set DATABRICKS_CONFIG_PROFILE to a profile for ${HOST}, then rerun.`, + ); + expect(requests).toEqual([]); + expect(lakebase.calls).toEqual([]); + }); + + test("a host written without a scheme or with a trailing slash matches", async () => { + const { ctx } = deployedApp({ + yml: ymlWithHost("acme.cloud.databricks.com/"), + }); + expect((await runVerify(ctx, DEPLOYED)).ok).toBe(true); + }); + + test("an app that does not exist yet: deploy it", async () => { + const { ctx, lakebase } = deployedApp({ app: "missing" }); + + expect(await runVerify(ctx, DEPLOYED)).toMatchObject({ + problems: [ + "app my-app is not deployed yet: `databricks apps deploy` creates it and gives it access to production", + ], + ok: false, + }); + expect(lakebase.calls).not.toContain(`listRoles ${PRODUCTION}`); + }); + + test.each([ + [ + "another branch", + [ + { + postgres: { + branch: "projects/p/branches/staging", + database: DATABASE, + permission: "CAN_CONNECT_AND_CREATE", + }, + }, + ], + `app my-app uses branch projects/p/branches/staging, not ${PRODUCTION} from databricks.yml: \`databricks apps deploy\` applies databricks.yml`, + ], + [ + "no postgres resource", + [{ sql_warehouse: { id: "w", permission: "CAN_USE" } }], + "app my-app has no postgres resource: `databricks apps deploy` adds the one databricks.yml declares", + ], + [ + "another database", + [ + { + postgres: { + branch: PRODUCTION, + database: `${PRODUCTION}/databases/other`, + permission: "CAN_CONNECT_AND_CREATE", + }, + }, + ], + `app my-app uses database ${PRODUCTION}/databases/other, not ${DATABASE} from databricks.yml: \`databricks apps deploy\` applies databricks.yml`, + ], + [ + "a permission that cannot create the schema", + [ + { + postgres: { + branch: PRODUCTION, + database: DATABASE, + permission: "CAN_CONNECT", + }, + }, + ], + "app my-app's postgres resource has permission CAN_CONNECT, but the app needs CAN_CONNECT_AND_CREATE to create its schema: set it in databricks.yml, then run `databricks apps deploy`", + ], + ])("a postgres resource with %s", async (_label, resources, problem) => { + const { ctx, lakebase } = deployedApp({ app: runningApp({ resources }) }); + + expect(await deployProblems(ctx)).toEqual([problem]); + expect(lakebase.calls).not.toContain(`listRoles ${PRODUCTION}`); + }); + + test("a service principal without a role on the branch", async () => { + const { ctx } = deployedApp({ spRole: false }); + + expect(await deployProblems(ctx)).toEqual([ + `the app's service principal ${SP} has no Postgres role on production: \`databricks apps deploy\` creates it from the app's postgres resource`, + ]); + }); + + test.each([ + [ + "crashed", + { + app_status: { state: "CRASHED", message: "App crashed unexpectedly." }, + }, + "app my-app is crashed: App crashed unexpectedly. `databricks apps logs` shows why; a migration that fails stops the app on start", + ], + [ + // As the API returns it: a stopped app has no active_deployment. + "stopped", + { + compute_status: { + state: "STOPPED", + message: "Start the app compute to deploy the app.", + }, + app_status: { + state: "UNAVAILABLE", + message: "App status is unavailable.", + }, + active_deployment: undefined, + }, + "app my-app is stopped: `databricks apps start` starts its last deployment, or `databricks apps deploy` deploys the current code; it migrates on start", + ], + [ + "starting", + { compute_status: { state: "STARTING" } }, + "app my-app's compute is starting", + ], + [ + "a failed deployment", + { + pending_deployment: { + status: { state: "FAILED", message: "Build failed." }, + }, + }, + "the last deployment of app my-app failed: Build failed. `databricks apps logs` shows why", + ], + [ + "a deployment in progress", + { pending_deployment: { status: { state: "IN_PROGRESS" } } }, + "a deployment of app my-app is in progress: run verify again when it finishes", + ], + [ + "no deployment", + { active_deployment: undefined, app_status: { state: "UNAVAILABLE" } }, + "app my-app has no deployment yet: `databricks apps deploy` deploys it", + ], + ])("an app that is %s", async (_label, overrides, problem) => { + const { ctx } = deployedApp({ app: runningApp(overrides) }); + + expect(await deployProblems(ctx)).toEqual([problem]); + }); + + test("a deploy branch or database that does not exist", async () => { + const { ctx, lakebase } = deployedApp(); + lakebase.databases.clear(); + expect(await deployProblems(ctx)).toEqual([ + `databricks.yml binds database ${DATABASE}, which does not exist on production`, + ]); + + lakebase.branches.clear(); + expect(await deployProblems(ctx)).toEqual([ + `databricks.yml deploys to ${PRODUCTION}, which does not exist`, + ]); + }); + + test("an app name with a reference only a deploy resolves is reported before any API call; ${bundle.target} resolves", async () => { + const unresolved = deployedApp({ + yml: ymlWithApps( + `resources:\n apps:\n app:\n name: "my-app-\${workspace.current_user.short_name}"\n resources:\n - name: postgres\n postgres:\n branch: \${var.postgres_branch}\n`, + ), + }); + expect(await deployProblems(unresolved.ctx)).toEqual([ + "databricks.yml names the app my-app-${workspace.current_user.short_name}, with a reference verify cannot resolve", + ]); + expect(unresolved.requests).toEqual([]); + expect(unresolved.lakebase.calls).toEqual([]); + + const perTarget = deployedApp({ + yml: ymlWithApps( + `resources:\n apps:\n app:\n name: "my-\${bundle.target}"\n resources:\n - name: postgres\n postgres:\n branch: \${var.postgres_branch}\n`, + ), + app: "missing", + }); + await runVerify(perTarget.ctx, DEPLOYED); + expect(perTarget.requests).toEqual(["apps.get my-default"]); + }); + + test("a target without an app, or with several apps on postgres, says so", async () => { + const none = deployedApp({ yml: ymlWithApps("") }); + expect(await runVerify(none.ctx, DEPLOYED)).toMatchObject({ + app: null, + problems: ["target default in databricks.yml declares no app"], + ok: false, + }); + expect(none.requests).toEqual([]); + + const postgres = + " resources:\n - name: postgres\n postgres:\n branch: ${var.postgres_branch}\n"; + const several = deployedApp({ + yml: ymlWithApps( + `resources:\n apps:\n web:\n name: web\n${postgres} worker:\n name: worker\n${postgres}`, + ), + }); + await expect(runVerify(several.ctx, DEPLOYED)).rejects.toThrow( + "Target default deploys 2 apps with a postgres resource (web, worker); verify checks one app per target.", + ); + }); + + test("an error other than 404 from the apps API names the app", async () => { + const { ctx } = deployedApp({ + app: Object.assign(new Error("Forbidden"), { statusCode: 403 }), + }); + + await expect(runVerify(ctx, DEPLOYED)).rejects.toThrow( + "Could not read app my-app: Forbidden", + ); + }); + + test("never writes and never connects to the database", async () => { + const { ctx, lakebase } = deployedApp({ spRole: false }); + + await runVerify(ctx, DEPLOYED); + + expect(withBranchPool).not.toHaveBeenCalled(); + expect(lakebase.calls.filter((c) => !/^(get|list)[A-Z]/.test(c))).toEqual( + [], + ); + }); + + test("without databricks.yml, or without a postgres binding in it, it says so", async () => { + const { ctx } = deployedApp(); + fs.writeFileSync( + path.join(ctx.root, "databricks.yml"), + "resources:\n apps:\n app:\n name: my-app\n", + ); + await expect(runVerify(ctx, DEPLOYED)).rejects.toThrow( + "No deploy branch: target default in databricks.yml declares no app with a postgres resource.", + ); + + fs.rmSync(path.join(ctx.root, "databricks.yml")); + await expect(runVerify(ctx, DEPLOYED)).rejects.toThrow( + "No databricks.yml, so there is no deployed app to verify.", + ); + }); +}); diff --git a/packages/appkit/src/database/tooling/verify.ts b/packages/appkit/src/database/tooling/verify.ts new file mode 100644 index 000000000..3c436ff02 --- /dev/null +++ b/packages/appkit/src/database/tooling/verify.ts @@ -0,0 +1,519 @@ +import path from "node:path"; + +import type { PoolClient } from "pg"; + +import { loadDefaultDatabaseSchema } from "../../plugins/database/load-schema"; +import { findSchemaProblems } from "../../plugins/database/schema-check"; +import { readMigrationDrift } from "../migrations/apply"; +import { + journalFile, + type JournalEntry, + migrationsFolder, + readJournal, +} from "../migrations/paths"; +import type { DataPath } from "../runtime"; +import { + type DbContext, + hasEnvBranch, + NO_ENV_BRANCH, + withBranchPool, +} from "./context"; +import { requireSchemaFile } from "./migrate"; +import { + type BundleTarget, + defaultBundleTarget, + parseResourceName, + readBundle, + resourceId, +} from "./targets"; + +export interface VerifyOptions { + /** Check the deployed app of a databricks.yml target instead of the `.env` branch. */ + readonly deployed?: boolean; + /** The target to check; the bundle's default target without one. */ + readonly target?: string; +} + +/** `appkit db verify`: the `.env` branch against schema.ts and the committed migrations. */ +interface LocalVerifyReport { + readonly kind: "local"; + readonly branch: string; + /** Declared tables or columns the branch lacks (the plugin's startup check). */ + readonly schemaProblems: readonly string[]; + /** Committed migrations the branch has not run yet, newer than every one it ran. */ + readonly pendingMigrations: readonly string[]; + /** Committed migrations older than one the branch ran, never run: merged out of order. */ + readonly strandedMigrations: readonly string[]; + /** Migrations the branch ran whose committed file changed since. */ + readonly editedMigrations: readonly string[]; + /** Migrations the branch ran that this checkout does not commit, e.g. from another git branch. */ + readonly unknownMigrations: number; + readonly ok: boolean; +} + +/** How the deployed migrations compare with the committed ones. */ +interface DeployedMigrations { + /** "deployed code": the deployment's journal; "deploy time": committed after the deployment was created. */ + readonly checked: "deployed code" | "deploy time"; + /** Committed migrations the active deployment does not have. */ + readonly missing: readonly string[]; +} + +/** `appkit db verify --deployed`: whether a target's deployed app runs on its branch with the committed migrations. */ +interface DeployVerifyReport { + readonly kind: "deploy"; + readonly target: string; + /** The target's workspace; null when databricks.yml names none. */ + readonly host: string | null; + readonly branch: string; + /** The app the target deploys; null when it declares none. */ + readonly app: string | null; + /** null when there are no committed migrations, or no deployment to compare with. */ + readonly migrations: DeployedMigrations | null; + /** What keeps the deployed app from using the branch, each with its fix. */ + readonly problems: readonly string[]; + readonly ok: boolean; +} + +export type VerifyReport = LocalVerifyReport | DeployVerifyReport; + +const DEPLOY = "`databricks apps deploy`"; +const LOGS = "`databricks apps logs`"; +/** What the app's postgres resource needs to create and own its schema. */ +const POSTGRES_PERMISSION = "CAN_CONNECT_AND_CREATE"; + +/** DataPath.raw over one connection: each interpolation becomes a $n parameter. */ +function rawOn(client: Pick): Pick { + return { + async raw(strings: TemplateStringsArray, ...values: unknown[]) { + const text = strings.reduce( + (sql, part, index) => `${sql}$${index}${part}`, + ); + const result = await client.query(text, values); + return result.rows as T[]; + }, + }; +} + +/** Read-only, as the developer, on the `.env` branch: one READ ONLY transaction, rolled back. */ +async function verifyLocal(ctx: DbContext): Promise { + requireSchemaFile(ctx.root, "verify"); + const branch = parseResourceName(ctx.env.LAKEBASE_ENDPOINT)?.branch; + if (!branch || !hasEnvBranch(ctx.env)) { + throw new Error(`Did not verify: ${NO_ENV_BRANCH}.`); + } + const schema = await loadDefaultDatabaseSchema(ctx.root, { + showImportError: true, + }); + return withBranchPool(ctx, async (pool) => { + const client = await pool.connect(); + try { + await client.query("begin transaction read only"); + const schemaProblems = await findSchemaProblems(rawOn(client), schema); + const drift = await readMigrationDrift(client, { + migrationsFolder: migrationsFolder(ctx.root), + schemaName: schema.$schemaName, + }); + return { + kind: "local", + branch, + schemaProblems, + pendingMigrations: drift.pending, + strandedMigrations: drift.stranded, + editedMigrations: drift.edited, + unknownMigrations: drift.unknown, + ok: + schemaProblems.length === 0 && + drift.pending.length === 0 && + drift.stranded.length === 0 && + drift.edited.length === 0 && + drift.unknown === 0, + }; + } finally { + await client.query("rollback").catch(() => undefined); + client.release(); + } + }); +} + +interface DeploymentStatus { + readonly state?: string; + readonly message?: string; +} + +interface Deployment { + readonly create_time?: string; + /** The snapshot of the source the deployment runs. */ + readonly deployment_artifacts?: { readonly source_code_path?: string }; + readonly status?: DeploymentStatus; +} + +/** The fields of GET /api/2.0/apps/{name} that verify reads, in the API's snake_case. */ +interface DeployedApp { + readonly service_principal_client_id?: string; + readonly resources?: ReadonlyArray<{ + readonly postgres?: { + readonly branch?: string; + readonly database?: string; + readonly permission?: string; + }; + }>; + readonly app_status?: DeploymentStatus; + readonly compute_status?: DeploymentStatus; + readonly active_deployment?: Deployment; + readonly pending_deployment?: Deployment; +} + +/** null when the app does not exist yet. */ +async function readApp( + ctx: DbContext, + name: string, +): Promise { + try { + const response = await ctx.workspace().request({ + method: "GET", + path: `/api/2.0/apps/${encodeURIComponent(name)}`, + }); + return (await response.json()) as DeployedApp; + } catch (error) { + if ((error as { statusCode?: unknown } | null)?.statusCode === 404) { + return null; + } + const message = error instanceof Error ? error.message : String(error); + throw new Error(`Could not read app ${name}: ${message}`, { + cause: error, + }); + } +} + +/** The tags in a deployed journal; null when it cannot be read, which includes a deployment without one. */ +async function readDeployedTags( + ctx: DbContext, + file: string, +): Promise | null> { + try { + const response = await ctx.workspace().request({ + method: "GET", + path: "/api/2.0/workspace/export", + query: { path: file, format: "AUTO" }, + }); + const { content } = (await response.json()) as { content?: string }; + if (!content) return null; + const journal = JSON.parse( + Buffer.from(content, "base64").toString("utf-8"), + ); + const entries: unknown[] = Array.isArray(journal?.entries) + ? journal.entries + : []; + return new Set( + entries + .map((entry) => (entry as { tag?: unknown } | null)?.tag) + .filter((tag): tag is string => typeof tag === "string"), + ); + } catch { + return null; + } +} + +/** + * Committed migrations the active deployment lacks: from the journal in the + * snapshot it runs when that is readable, else the ones generated after the + * deployment was created. + */ +async function deployedMigrations( + ctx: DbContext, + deployment: Deployment | undefined, + committed: readonly JournalEntry[], +): Promise { + if (!deployment || committed.length === 0) return null; + const snapshot = deployment.deployment_artifacts?.source_code_path; + if (snapshot) { + const relative = path + .relative(ctx.root, journalFile(migrationsFolder(ctx.root))) + .split(path.sep) + .join("/"); + const deployed = await readDeployedTags( + ctx, + `${snapshot.replace(/\/+$/, "")}/${relative}`, + ); + if (deployed) { + return { + checked: "deployed code", + missing: committed + .filter((entry) => !deployed.has(entry.tag)) + .map((entry) => entry.tag), + }; + } + } + const created = Date.parse(deployment.create_time ?? ""); + if (Number.isNaN(created)) return null; + return { + checked: "deploy time", + missing: committed + .filter((entry) => entry.when > created) + .map((entry) => entry.tag), + }; +} + +function migrationsProblem( + name: string, + migrations: DeployedMigrations, + deployment: Deployment | undefined, +): string | null { + const { missing } = migrations; + if (missing.length === 0) return null; + const them = missing.length === 1 ? "it" : "them"; + const fix = `${DEPLOY} deploys ${them}, and the app applies ${them} on start`; + return migrations.checked === "deployed code" + ? `app ${name}'s active deployment does not have ${missing.join(", ")}, which this checkout commits: ${fix}` + : `${missing.join(", ")} ${missing.length === 1 ? "was" : "were"} generated after app ${name}'s active deployment (${deployment?.create_time}), so it does not have ${them}: ${fix}`; +} + +/** The app's postgres resource against databricks.yml: same branch and database, and the permission to create its schema. */ +function bindingProblems( + app: DeployedApp, + name: string, + branch: string, + database: string | null, + file: string, +): string[] { + const bound = (app.resources ?? []).flatMap((resource) => + resource.postgres ? [resource.postgres] : [], + ); + const binding = bound.find((postgres) => postgres.branch === branch); + if (!binding) { + return [ + bound.length > 0 + ? `app ${name} uses branch ${bound.map((postgres) => postgres.branch).join(", ")}, not ${branch} from ${file}: ${DEPLOY} applies ${file}` + : `app ${name} has no postgres resource: ${DEPLOY} adds the one ${file} declares`, + ]; + } + const problems: string[] = []; + if (database && binding.database !== database) { + problems.push( + `app ${name} uses database ${binding.database ?? "(none)"}, not ${database} from ${file}: ${DEPLOY} applies ${file}`, + ); + } + if (binding.permission !== POSTGRES_PERMISSION) { + problems.push( + `app ${name}'s postgres resource has permission ${binding.permission ?? "(none)"}, but the app needs ${POSTGRES_PERMISSION} to create its schema: set it in ${file}, then run ${DEPLOY}`, + ); + } + return problems; +} + +/** ": " for an API status message, without its final period; "" without one. */ +function detail(message: string | undefined): string { + const text = message?.trim().replace(/\.+$/, ""); + return text ? `: ${text}` : ""; +} + +/** Whether the app is up: a running app has applied its migrations and checked its schema on start. */ +function runningProblems(app: DeployedApp, name: string): string[] { + const pending = app.pending_deployment?.status; + if (pending?.state === "IN_PROGRESS") { + return [ + `a deployment of app ${name} is in progress: run verify again when it finishes`, + ]; + } + const deployment = app.active_deployment?.status; + if (pending?.state === "FAILED" || deployment?.state === "FAILED") { + const failed = pending?.state === "FAILED" ? pending : deployment; + return [ + `the last deployment of app ${name} failed${detail(failed?.message)}. ${LOGS} shows why`, + ]; + } + const compute = app.compute_status; + // A stopped app's API response has no active_deployment, deployed before or not. + if (compute?.state === "STOPPED") { + return [ + `app ${name} is stopped: \`databricks apps start\` starts its last deployment, or ${DEPLOY} deploys the current code; it migrates on start`, + ]; + } + if (!app.active_deployment) { + return [`app ${name} has no deployment yet: ${DEPLOY} deploys it`]; + } + if (compute?.state && compute.state !== "ACTIVE") { + return [ + `app ${name}'s compute is ${compute.state.toLowerCase()}${detail(compute.message)}`, + ]; + } + const status = app.app_status; + if (status?.state === "RUNNING") return []; + return [ + `app ${name} is ${status?.state?.toLowerCase() ?? "not running"}${detail(status?.message)}. ${LOGS} shows why; a migration that fails stops the app on start`, + ]; +} + +/** "a.cloud.databricks.com" for "https://A.cloud.databricks.com/". */ +function hostOf(url: string): string { + try { + return new URL(/^[a-z]+:\/\//i.test(url) ? url : `https://${url}`).host; + } catch { + return url.toLowerCase(); + } +} + +/** The API calls go to the workspace this shell is authenticated against, which must be the target's. */ +async function assertTargetWorkspace( + ctx: DbContext, + target: BundleTarget, +): Promise { + if (!target.host) return; + const authenticated = await ctx.workspace().getHost(); + if (hostOf(authenticated) === hostOf(target.host)) return; + throw new Error( + `Target ${target.name} deploys to ${target.host}, but this shell is authenticated against ${authenticated}. Set DATABRICKS_CONFIG_PROFILE to a profile for ${target.host}, then rerun.`, + ); +} + +function selectTarget( + file: string, + targets: readonly BundleTarget[], + name: string | undefined, + fallback: BundleTarget | null, +): BundleTarget { + const names = targets.map((target) => target.name).join(", "); + if (name) { + const target = targets.find((candidate) => candidate.name === name); + if (!target) + throw new Error( + `${file} has no target ${name}; its targets are ${names}.`, + ); + return target; + } + if (!fallback) { + throw new Error( + `${file} has no default target; pass --target with one of ${names}.`, + ); + } + return fallback; +} + +/** + * --deployed, through the Databricks API only: no database connection, so no + * Postgres role on the deploy branch is needed. The target's app must exist, + * be bound to the branch and database databricks.yml names with + * CAN_CONNECT_AND_CREATE, have a role on the branch for its service + * principal, be running, and run code with every committed migration. + */ +async function verifyDeploy( + ctx: DbContext, + targetName: string | undefined, +): Promise { + const bundle = readBundle(ctx.root); + if (!bundle) { + throw new Error( + "No databricks.yml, so there is no deployed app to verify.", + ); + } + const { file } = bundle; + const target = selectTarget( + file, + bundle.targets, + targetName, + defaultBundleTarget(bundle), + ); + const bound = target.apps.filter((app) => app.postgres); + if (bound.length > 1) { + throw new Error( + `Target ${target.name} deploys ${bound.length} apps with a postgres resource (${bound.map((app) => app.key).join(", ")}); verify checks one app per target.`, + ); + } + const app = + bound[0] ?? (target.apps.length === 1 ? target.apps[0] : undefined); + const branch = app?.postgres?.branch ?? target.postgresBranch; + if (!branch) { + throw new Error( + app?.postgres + ? `No deploy branch: the postgres resource of app ${app.key} in target ${target.name} names its branch with a reference only a deploy resolves.` + : `No deploy branch: target ${target.name} in ${file} declares no app with a postgres resource.`, + ); + } + const database = app?.postgres?.database ?? target.postgresDatabase; + const report = ( + name: string | null, + problems: string[], + migrations: DeployedMigrations | null = null, + ): DeployVerifyReport => ({ + kind: "deploy", + target: target.name, + host: target.host, + branch, + app: name, + migrations, + problems, + ok: problems.length === 0, + }); + if (!app) { + return report(null, [`target ${target.name} in ${file} declares no app`]); + } + if (!app.name) { + return report(app.declaredName, [ + app.declaredName + ? `${file} names the app ${app.declaredName}, with a reference verify cannot resolve` + : `app ${app.key} in ${file} has no name`, + ]); + } + const name = app.name; + + await assertTargetWorkspace(ctx, target); + const api = ctx.api(); + const [branchFound, databaseFound, deployed] = await Promise.all([ + api.getBranch(branch), + database ? api.getDatabase(database) : null, + readApp(ctx, name), + ]); + const branchId = resourceId(branch); + if (!branchFound) { + return report(name, [`${file} deploys to ${branch}, which does not exist`]); + } + const problems: string[] = []; + if (database && !databaseFound) { + problems.push( + `${file} binds database ${database}, which does not exist on ${branchId}`, + ); + } + if (!deployed) { + problems.push( + `app ${name} is not deployed yet: ${DEPLOY} creates it and gives it access to ${branchId}`, + ); + return report(name, problems); + } + + const binding = bindingProblems(deployed, name, branch, database, file); + problems.push(...binding, ...runningProblems(deployed, name)); + const sp = deployed.service_principal_client_id; + const committed = readJournal(migrationsFolder(ctx.root))?.entries ?? []; + const [roles, migrations] = await Promise.all([ + // Only meaningful once the app is bound to this branch. + sp && binding.length === 0 ? api.listRoles(branch) : null, + deployedMigrations(ctx, deployed.active_deployment, committed), + ]); + if (roles && !roles.some((role) => role.status?.postgres_role === sp)) { + problems.push( + `the app's service principal ${sp} has no Postgres role on ${branchId}: ${DEPLOY} creates it from the app's postgres resource`, + ); + } + const missing = migrations + ? migrationsProblem(name, migrations, deployed.active_deployment) + : null; + if (missing) problems.push(missing); + return report(name, problems, migrations); +} + +/** + * `appkit db verify`: check the `.env` branch against schema.ts (the plugin's + * startup check) and the committed migrations, read-only. With `deployed`, + * check through the Databricks API that a target's deployed app runs on its + * branch with the committed migrations; that reads no database, so it needs + * no role on the deploy branch. Nothing is written or created either way. + */ +export async function runVerify( + ctx: DbContext, + options: VerifyOptions = {}, +): Promise { + return options.deployed + ? verifyDeploy(ctx, options.target) + : verifyLocal(ctx); +} diff --git a/packages/appkit/src/plugins/database/schema-check.ts b/packages/appkit/src/plugins/database/schema-check.ts index 1a9067e78..8e92c12fc 100644 --- a/packages/appkit/src/plugins/database/schema-check.ts +++ b/packages/appkit/src/plugins/database/schema-check.ts @@ -8,19 +8,31 @@ interface CatalogColumn { readonly relation_kind: string; } +interface SchemaCheck { + readonly problems: string[]; + /** Declared tables with no relation of that name in the schema. */ + readonly absent: string[]; +} + /** - * Confirm every declared table and column exists before routes are published. - * Even after a deployed app applies its migrations, drift would surface on each - * request as an undefined-table or undefined-column failure, so check up front. + * Every declared table or column the database lacks, and every declared table + * that is a view or another non-table relation, as human-readable problems. * pg_catalog is read rather than information_schema, which hides objects the * role cannot access and would report a privilege gap as a missing column. */ -export async function assertSchemaMatchesDatabase( - dataPath: DataPath, +export async function findSchemaProblems( + dataPath: Pick, schema: Schema, -): Promise { +): Promise { + return (await checkSchema(dataPath, schema)).problems; +} + +async function checkSchema( + dataPath: Pick, + schema: Schema, +): Promise { const tables = Object.values(schema.$tables); - if (tables.length === 0) return; + if (tables.length === 0) return { problems: [], absent: [] }; const schemaName = schema.$schemaName; const tableNames = tables.map((table) => table.$name); // Include non-tables so setup can report their kind instead of claiming they are absent. @@ -74,10 +86,25 @@ export async function assertSchemaMatchesDatabase( ); } } + return { + problems, + absent: tableNames.filter((name) => !found.has(name)), + }; +} + +/** + * Confirm every declared table and column exists before routes are published. + * Even after a deployed app applies its migrations, drift would surface on each + * request as an undefined-table or undefined-column failure, so check up front. + */ +export async function assertSchemaMatchesDatabase( + dataPath: DataPath, + schema: Schema, +): Promise { + const { problems, absent } = await checkSchema(dataPath, schema); if (problems.length > 0) { - const absent = tableNames.filter((name) => !found.has(name)); const advice = - (await inPublicInstead(dataPath, schemaName, absent)) ?? + (await inPublicInstead(dataPath, schema.$schemaName, absent)) ?? "Run `npx appkit db migrate` locally, or commit migrations under config/database/migrations and set APPKIT_DB_MIGRATE_ON_START=true in app.yaml so the deployed app applies them on start."; throw databaseSetupFailed( `the declared schema does not match the database: ${problems.join("; ")}. ${advice}`, diff --git a/packages/appkit/src/plugins/database/tests/schema-check.test.ts b/packages/appkit/src/plugins/database/tests/schema-check.test.ts index 2abb3d26c..70640c7af 100644 --- a/packages/appkit/src/plugins/database/tests/schema-check.test.ts +++ b/packages/appkit/src/plugins/database/tests/schema-check.test.ts @@ -2,7 +2,10 @@ import { describe, expect, test, vi } from "vitest"; import type { DataPath } from "../../../database/runtime"; import { defineSchema, fk, id, text } from "../../../database/schema-builder"; -import { assertSchemaMatchesDatabase } from "../schema-check"; +import { + assertSchemaMatchesDatabase, + findSchemaProblems, +} from "../schema-check"; // Explicitly public: the assertions name the tables public.boards and public.notes. const schema = defineSchema( @@ -191,3 +194,39 @@ describe("assertSchemaMatchesDatabase", () => { expect(raw).not.toHaveBeenCalled(); }); }); + +describe("findSchemaProblems", () => { + test("returns every problem without throwing; the assert reports the same ones", async () => { + const { path } = catalog( + { boards: ["id", "title"], notes: ["id", "body"] }, + { boards: "v" }, + ); + + const problems = await findSchemaProblems(path, schema); + + expect(problems).toEqual([ + "table public.boards is a view, not a table", + "table public.notes is missing columns board_id, author_email", + ]); + await expect(assertSchemaMatchesDatabase(path, schema)).rejects.toThrow( + `the declared schema does not match the database: ${problems.join("; ")}.`, + ); + }); + + test("returns no problems for a matching database or an empty schema", async () => { + const { path } = catalog({ + boards: ["id", "title"], + notes: ["id", "board_id", "author_email", "body"], + }); + expect(await findSchemaProblems(path, schema)).toEqual([]); + + const empty = catalog({}); + expect( + await findSchemaProblems( + empty.path, + defineSchema(() => ({})), + ), + ).toEqual([]); + expect(empty.raw).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/shared/src/cli/commands/db/index.ts b/packages/shared/src/cli/commands/db/index.ts index ed811c91c..3c5bbcfdf 100644 --- a/packages/shared/src/cli/commands/db/index.ts +++ b/packages/shared/src/cli/commands/db/index.ts @@ -5,6 +5,7 @@ import { dbInitCommand } from "./init"; import { dbIntrospectCommand } from "./introspect"; import { dbMigrateCommand } from "./migrate"; import { dbSeedCommand } from "./seed"; +import { dbVerifyCommand } from "./verify"; /** * Lakebase database lifecycle for the DatabasePlugin. The logic lives in @@ -16,4 +17,5 @@ export const dbCommand = new Command("db") .addCommand(dbMigrateCommand) .addCommand(dbIntrospectCommand) .addCommand(dbCleanCommand) - .addCommand(dbSeedCommand); + .addCommand(dbSeedCommand) + .addCommand(dbVerifyCommand); diff --git a/packages/shared/src/cli/commands/db/init-clean-seed.test.ts b/packages/shared/src/cli/commands/db/init-clean-seed.test.ts index 90dcdf53f..872169aaa 100644 --- a/packages/shared/src/cli/commands/db/init-clean-seed.test.ts +++ b/packages/shared/src/cli/commands/db/init-clean-seed.test.ts @@ -107,13 +107,14 @@ afterEach(() => { }); describe("appkit db init", () => { - test("registers init, migrate, introspect, clean and seed", () => { + test("registers init, migrate, introspect, clean, seed and verify", () => { expect(dbCommand.commands.map((c) => c.name())).toEqual([ "init", "migrate", "introspect", "clean", "seed", + "verify", ]); }); diff --git a/packages/shared/src/cli/commands/db/output.ts b/packages/shared/src/cli/commands/db/output.ts index 72304f19d..326091077 100644 --- a/packages/shared/src/cli/commands/db/output.ts +++ b/packages/shared/src/cli/commands/db/output.ts @@ -5,8 +5,11 @@ import pc from "picocolors"; import type { CleanReport, + DeployVerifyReport, IntrospectReport, + LocalVerifyReport, MigrateReport, + VerifyReport, } from "./tooling-contract"; /** Prompts only when a human can answer them. */ @@ -96,6 +99,85 @@ export function printCleanReport(report: CleanReport): void { } } +const REBUILD = + "`appkit db clean` and then `appkit db migrate` rebuild the branch from this checkout's migrations, in order"; + +/** The one next step for a failed local verify, most fundamental gap first. */ +function localVerifyHint(report: LocalVerifyReport): string { + if (report.editedMigrations.length > 0) { + return "Restore the edited migrations from version control, and put the change in a new migration."; + } + if (report.unknownMigrations > 0) { + return `${REBUILD}. If the branch ran migrations you have not pulled yet, pull them instead.`; + } + if (report.strandedMigrations.length > 0) return `${REBUILD}.`; + // Pending migrations explain the gap; without them, schema.ts changed or the branch was edited. + return report.pendingMigrations.length > 0 + ? "Run `appkit db migrate` to apply the pending migrations." + : "If you changed schema.ts, run `appkit db migrate`; if the branch was changed by hand, `appkit db clean` and then `appkit db migrate` rebuild it from the migrations."; +} + +function printLocalVerifyReport(report: LocalVerifyReport): void { + const branch = shortName(report.branch); + if (report.ok) { + ok( + `${branch} has every table and column config/database/schema.ts declares, and every committed migration`, + ); + return; + } + for (const problem of report.schemaProblems) warn(`Schema: ${problem}`); + if (report.pendingMigrations.length > 0) { + warn(`Pending migrations: ${report.pendingMigrations.join(", ")}`); + } + for (const tag of report.strandedMigrations) { + warn( + `Never runs: ${tag}: it is older than a migration the branch already ran (merged out of order); the deployed app skips it too once it ran a newer one`, + ); + } + for (const tag of report.editedMigrations) { + warn( + `Never runs: ${tag}: it changed after the branch ran it, and a branch that ran it keeps the old version`, + ); + } + if (report.unknownMigrations > 0) { + const one = report.unknownMigrations === 1; + warn( + `Not committed: the branch ran ${report.unknownMigrations} migration${one ? "" : "s"} this checkout does not have, for example from another git branch`, + ); + } + console.error( + `${pc.red("✖")} ${branch} does not match config/database/schema.ts and the committed migrations. ${localVerifyHint(report)}`, + ); +} + +function printDeployVerifyReport(report: DeployVerifyReport): void { + const where = `target ${report.target}${report.host ? ` on ${report.host}` : ""}`; + const app = report.app ? `app ${report.app}` : "The deployed app"; + const branch = shortName(report.branch); + if (report.ok) { + ok( + `${app} (${where}) is running, bound to ${branch} with CAN_CONNECT_AND_CREATE, and its service principal has a role there`, + ); + if (report.migrations?.checked === "deployed code") { + ok("Its active deployment has every committed migration"); + } else if (report.migrations) { + ok( + "Every committed migration was generated before its active deployment; its code could not be read, so verify compared times", + ); + } + return; + } + for (const problem of report.problems) warn(problem); + console.error( + `${pc.red("✖")} ${app} (${where}) cannot use ${branch} yet. Fix the above, then run \`appkit db verify --deployed -t ${report.target}\` again.`, + ); +} + +export function printVerifyReport(report: VerifyReport): void { + if (report.kind === "local") printLocalVerifyReport(report); + else printDeployVerifyReport(report); +} + export function printIntrospectReport(report: IntrospectReport): void { const count = report.tables.length; ok( diff --git a/packages/shared/src/cli/commands/db/tooling-contract.ts b/packages/shared/src/cli/commands/db/tooling-contract.ts index 8298134b0..8bf01a291 100644 --- a/packages/shared/src/cli/commands/db/tooling-contract.ts +++ b/packages/shared/src/cli/commands/db/tooling-contract.ts @@ -67,6 +67,38 @@ export type InitStart = readonly migrate: MigrateReport; }; +export interface VerifyOptions { + readonly deployed?: boolean; + readonly target?: string; +} + +export interface LocalVerifyReport { + readonly kind: "local"; + readonly branch: string; + readonly schemaProblems: readonly string[]; + readonly pendingMigrations: readonly string[]; + readonly strandedMigrations: readonly string[]; + readonly editedMigrations: readonly string[]; + readonly unknownMigrations: number; + readonly ok: boolean; +} + +export interface DeployVerifyReport { + readonly kind: "deploy"; + readonly target: string; + readonly host: string | null; + readonly branch: string; + readonly app: string | null; + readonly migrations: { + readonly checked: "deployed code" | "deploy time"; + readonly missing: readonly string[]; + } | null; + readonly problems: readonly string[]; + readonly ok: boolean; +} + +export type VerifyReport = LocalVerifyReport | DeployVerifyReport; + export interface InitReport { readonly mode: BranchMode; readonly branch: string; @@ -117,4 +149,8 @@ export interface DatabaseTooling { readonly runSeed: ( ctx: Ctx, ) => Promise<{ readonly branch: string; readonly file: string }>; + readonly runVerify: ( + ctx: Ctx, + options?: VerifyOptions, + ) => Promise; } diff --git a/packages/shared/src/cli/commands/db/verify.test.ts b/packages/shared/src/cli/commands/db/verify.test.ts new file mode 100644 index 000000000..2592175e6 --- /dev/null +++ b/packages/shared/src/cli/commands/db/verify.test.ts @@ -0,0 +1,297 @@ +import { + afterEach, + beforeEach, + describe, + expect, + type MockInstance, + test, + vi, +} from "vitest"; + +const { createDbContext, runVerify } = vi.hoisted(() => ({ + createDbContext: vi.fn(() => ({ root: "/app", env: {} })), + runVerify: vi.fn(), +})); + +// The tooling is an ambient module loaded with a dynamic import; mock it so the +// command never touches a database. +vi.mock("@databricks/appkit/database-tooling", () => ({ + createDbContext, + runVerify, +})); + +import { dbCommand } from "./index"; + +let log: MockInstance; +let error: MockInstance; + +function output(spy: MockInstance): string { + return spy.mock.calls.map((call) => call.join(" ")).join("\n"); +} + +const local = { + kind: "local" as const, + branch: "projects/p/branches/dev-alice-3f9a2c", + schemaProblems: [], + pendingMigrations: [], + strandedMigrations: [], + editedMigrations: [], + unknownMigrations: 0, + ok: true, +}; + +/** Commander keeps parsed option values on the command; each test parses afresh. */ +function resetOptionValues() { + for (const command of dbCommand.commands) { + for (const option of command.options) { + command.setOptionValueWithSource( + option.attributeName(), + undefined, + "default", + ); + } + } +} + +beforeEach(() => { + resetOptionValues(); + runVerify.mockReset(); + log = vi.spyOn(console, "log").mockImplementation(() => undefined); + error = vi.spyOn(console, "error").mockImplementation(() => undefined); +}); + +afterEach(() => { + vi.restoreAllMocks(); + process.exitCode = undefined; +}); + +describe("appkit db verify", () => { + test("is registered under the db group", () => { + expect(dbCommand.commands.map((command) => command.name())).toContain( + "verify", + ); + }); + + test("a clean branch prints what was checked and exits 0", async () => { + runVerify.mockResolvedValue(local); + + await dbCommand.parseAsync(["verify"], { from: "user" }); + + expect(runVerify).toHaveBeenCalledWith(expect.anything(), { + deployed: false, + }); + expect(output(log)).toContain( + "dev-alice-3f9a2c has every table and column config/database/schema.ts declares, and every committed migration", + ); + expect(error).not.toHaveBeenCalled(); + expect(process.exitCode).toBeUndefined(); + }); + + test("prints every schema problem and pending migration, and exits 1", async () => { + runVerify.mockResolvedValue({ + ...local, + schemaProblems: [ + "table app.notes is missing column body", + "table app.tags does not exist", + ], + pendingMigrations: ["0001_migration", "0002_migration"], + ok: false, + }); + + await dbCommand.parseAsync(["verify"], { from: "user" }); + + const printed = output(log); + expect(printed).toContain("Schema: table app.notes is missing column body"); + expect(printed).toContain("Schema: table app.tags does not exist"); + expect(printed).toContain( + "Pending migrations: 0001_migration, 0002_migration", + ); + expect(output(error)).toContain( + "dev-alice-3f9a2c does not match config/database/schema.ts and the committed migrations. Run `appkit db migrate` to apply the pending migrations.", + ); + expect(process.exitCode).toBe(1); + }); + + test("schema problems without pending migrations point to migrate or clean", async () => { + runVerify.mockResolvedValue({ + ...local, + schemaProblems: ["table app.notes is missing column body"], + ok: false, + }); + + await dbCommand.parseAsync(["verify"], { from: "user" }); + + expect(output(error)).toContain( + "If you changed schema.ts, run `appkit db migrate`; if the branch was changed by hand, `appkit db clean` and then `appkit db migrate` rebuild it from the migrations.", + ); + expect(process.exitCode).toBe(1); + }); + + test("a migration merged out of order is named, with how to rebuild the branch", async () => { + runVerify.mockResolvedValue({ + ...local, + strandedMigrations: ["0001_parallel"], + ok: false, + }); + + await dbCommand.parseAsync(["verify"], { from: "user" }); + + expect(output(log)).toContain( + "Never runs: 0001_parallel: it is older than a migration the branch already ran (merged out of order); the deployed app skips it too once it ran a newer one", + ); + expect(output(error)).toContain( + "`appkit db clean` and then `appkit db migrate` rebuild the branch from this checkout's migrations, in order.", + ); + expect(process.exitCode).toBe(1); + }); + + test("an edited migration is restored, not rebuilt over", async () => { + runVerify.mockResolvedValue({ + ...local, + strandedMigrations: ["0001_parallel"], + editedMigrations: ["0000_migration"], + ok: false, + }); + + await dbCommand.parseAsync(["verify"], { from: "user" }); + + expect(output(log)).toContain( + "Never runs: 0000_migration: it changed after the branch ran it, and a branch that ran it keeps the old version", + ); + expect(output(error)).toContain( + "Restore the edited migrations from version control, and put the change in a new migration.", + ); + }); + + test("migrations the branch ran that this checkout does not commit", async () => { + runVerify.mockResolvedValue({ ...local, unknownMigrations: 2, ok: false }); + + await dbCommand.parseAsync(["verify"], { from: "user" }); + + expect(output(log)).toContain( + "Not committed: the branch ran 2 migrations this checkout does not have, for example from another git branch", + ); + expect(output(error)).toContain( + "rebuild the branch from this checkout's migrations, in order. If the branch ran migrations you have not pulled yet, pull them instead.", + ); + expect(process.exitCode).toBe(1); + }); + + describe("--deployed", () => { + const deploy = { + kind: "deploy" as const, + target: "prod", + host: "https://acme.cloud.databricks.com", + branch: "projects/p/branches/production", + app: "my-app", + migrations: { checked: "deployed code" as const, missing: [] }, + problems: [], + ok: true, + }; + + test("a ready app prints what was checked, target and host included, and exits 0", async () => { + runVerify.mockResolvedValue(deploy); + + await dbCommand.parseAsync(["verify", "--deployed", "-t", "prod"], { + from: "user", + }); + + expect(runVerify).toHaveBeenCalledWith(expect.anything(), { + deployed: true, + target: "prod", + }); + const printed = output(log); + expect(printed).toContain( + "app my-app (target prod on https://acme.cloud.databricks.com) is running, bound to production with CAN_CONNECT_AND_CREATE, and its service principal has a role there", + ); + expect(printed).toContain( + "Its active deployment has every committed migration", + ); + expect(error).not.toHaveBeenCalled(); + expect(process.exitCode).toBeUndefined(); + }); + + test("says when migrations were compared by deploy time", async () => { + runVerify.mockResolvedValue({ + ...deploy, + migrations: { checked: "deploy time", missing: [] }, + }); + + await dbCommand.parseAsync(["verify", "--deployed"], { from: "user" }); + + expect(runVerify).toHaveBeenCalledWith(expect.anything(), { + deployed: true, + }); + expect(output(log)).toContain( + "Every committed migration was generated before its active deployment; its code could not be read, so verify compared times", + ); + }); + + test("lists each problem with its fix, and exits 1", async () => { + runVerify.mockResolvedValue({ + ...deploy, + problems: [ + "app my-app has no postgres resource: `databricks apps deploy` adds the one databricks.yml declares", + "app my-app's active deployment does not have 0003_migration, which this checkout commits: `databricks apps deploy` deploys it, and the app applies it on start", + ], + ok: false, + }); + + await dbCommand.parseAsync(["verify", "--deployed", "-t", "prod"], { + from: "user", + }); + + const printed = output(log); + expect(printed).toContain( + "app my-app has no postgres resource: `databricks apps deploy` adds the one databricks.yml declares", + ); + expect(printed).toContain( + "app my-app's active deployment does not have 0003_migration", + ); + expect(output(error)).toContain( + "app my-app (target prod on https://acme.cloud.databricks.com) cannot use production yet. Fix the above, then run `appkit db verify --deployed -t prod` again.", + ); + expect(process.exitCode).toBe(1); + }); + + test("without an app in the target it names the deployed app generically", async () => { + runVerify.mockResolvedValue({ + ...deploy, + host: null, + app: null, + migrations: null, + problems: ["target prod in databricks.yml declares no app"], + ok: false, + }); + + await dbCommand.parseAsync(["verify", "--deployed"], { from: "user" }); + + expect(output(error)).toContain( + "The deployed app (target prod) cannot use production yet.", + ); + }); + }); + + test("--target without --deployed is refused before loading the tooling", async () => { + await dbCommand.parseAsync(["verify", "-t", "prod"], { from: "user" }); + + expect(output(error)).toContain( + "--target picks the databricks.yml target --deployed checks; pass --deployed with it.", + ); + expect(runVerify).not.toHaveBeenCalled(); + expect(process.exitCode).toBe(1); + }); + + test("an error is printed and exits 1", async () => { + runVerify.mockRejectedValue( + new Error( + "Did not verify: no branch is configured in .env (PGHOST, PGDATABASE and LAKEBASE_ENDPOINT); run `appkit db init`.", + ), + ); + + await dbCommand.parseAsync(["verify"], { from: "user" }); + + expect(output(error)).toContain("Did not verify: no branch is configured"); + expect(process.exitCode).toBe(1); + }); +}); diff --git a/packages/shared/src/cli/commands/db/verify.ts b/packages/shared/src/cli/commands/db/verify.ts new file mode 100644 index 000000000..3ffa1bece --- /dev/null +++ b/packages/shared/src/cli/commands/db/verify.ts @@ -0,0 +1,63 @@ +import { Command } from "commander"; + +import { loadDatabaseTooling } from "./load-tooling"; +import { fail, printVerifyReport } from "./output"; + +async function runDbVerify(options: { + deployed?: boolean; + target?: string; +}): Promise { + try { + if (options.target && !options.deployed) { + throw new Error( + "--target picks the databricks.yml target --deployed checks; pass --deployed with it.", + ); + } + const tooling = await loadDatabaseTooling(); + const report = await tooling.runVerify(tooling.createDbContext(), { + deployed: options.deployed === true, + ...(options.target ? { target: options.target } : {}), + }); + printVerifyReport(report); + if (!report.ok) process.exitCode = 1; + } catch (error) { + fail(error); + } +} + +export const dbVerifyCommand = new Command("verify") + .description( + "Check the .env branch against config/database/schema.ts and the committed migrations (read-only)", + ) + .option( + "--deployed", + "check instead the app a databricks.yml target deploys, and its branch", + ) + .option( + "-t, --target ", + "the databricks.yml target --deployed checks (default: the bundle's default target)", + ) + .addHelpText( + "after", + ` +Reports declared tables or columns the branch lacks (what the app checks on +start), committed migrations the branch has not applied, migrations it will +never run (merged out of order, or edited after they ran), and migrations it +ran that this checkout does not commit. + +--deployed reads only the Databricks API, so it needs no access to the deploy +branch's database. It checks that the target's app exists, that its postgres +resource names the branch and database in databricks.yml with +CAN_CONNECT_AND_CREATE, that its service principal has a role on the branch, +that the app is running, and that its active deployment has every committed +migration. It refuses when the target's workspace.host is not the workspace +this shell is authenticated against. + +Exits 1 on any finding. Nothing is written or created. + +Examples: + $ appkit db verify + $ appkit db verify --deployed + $ appkit db verify --deployed -t prod`, + ) + .action(runDbVerify); From f1af073839c2fb544758bce0d096b5ec59069f55 Mon Sep 17 00:00:00 2001 From: ditadi Date: Fri, 30 Oct 2026 17:21:35 +0000 Subject: [PATCH 2/8] fix(appkit): verify deployed migrations by their journal entries and SQL verify --deployed treated an unreadable deployed journal as a reason to compare the migrations' timestamps with the deployment's create time, and it matched migrations by tag only, so two checkouts with different SQL under the same 0001_migration tag passed. Now the only evidence is the source snapshot the active deployment runs. Each committed migration needs the same journal entry and byte-identical SQL there. The report says verified, differs (with the missing and changed tags) or unavailable (with the reason). An export, parse or snapshot failure is unavailable, and report.ok is false unless the migrations are verified or none are committed. The deployed journal goes through the same parseJournal check as the committed one. Signed-off-by: ditadi --- .../appkit/src/database/migrations/paths.ts | 19 +- .../src/database/tooling/tests/verify.test.ts | 156 +++++++++-- .../appkit/src/database/tooling/verify.ts | 248 ++++++++++++------ packages/shared/src/cli/commands/db/output.ts | 6 +- .../src/cli/commands/db/tooling-contract.ts | 13 +- .../shared/src/cli/commands/db/verify.test.ts | 16 +- packages/shared/src/cli/commands/db/verify.ts | 7 +- 7 files changed, 334 insertions(+), 131 deletions(-) diff --git a/packages/appkit/src/database/migrations/paths.ts b/packages/appkit/src/database/migrations/paths.ts index 63a8b7263..d4f669aa7 100644 --- a/packages/appkit/src/database/migrations/paths.ts +++ b/packages/appkit/src/database/migrations/paths.ts @@ -81,15 +81,20 @@ export function readJsonFile(file: string): T { } } -/** null when meta/_journal.json is absent (nothing committed yet). */ -export function readJournal(folder: string): Journal | null { - const file = journalFile(folder); - if (!fs.existsSync(file)) return null; - const parsed = readJsonFile>(file); - if (parsed.dialect !== "postgresql" || !Array.isArray(parsed.entries)) { +/** A parsed meta/_journal.json, checked to be drizzle-kit's PostgreSQL journal; label names it in the error. */ +export function parseJournal(json: unknown, label: string): Journal { + const parsed = json as Partial | null; + if (parsed?.dialect !== "postgresql" || !Array.isArray(parsed.entries)) { throw new MigrationFilesError( - `${file} is not a drizzle-kit PostgreSQL migration journal`, + `${label} is not a drizzle-kit PostgreSQL migration journal`, ); } return parsed as Journal; } + +/** null when meta/_journal.json is absent (nothing committed yet). */ +export function readJournal(folder: string): Journal | null { + const file = journalFile(folder); + if (!fs.existsSync(file)) return null; + return parseJournal(readJsonFile(file), file); +} diff --git a/packages/appkit/src/database/tooling/tests/verify.test.ts b/packages/appkit/src/database/tooling/tests/verify.test.ts index 63e332449..be79e597f 100644 --- a/packages/appkit/src/database/tooling/tests/verify.test.ts +++ b/packages/appkit/src/database/tooling/tests/verify.test.ts @@ -329,7 +329,6 @@ const DATABASE = `${PRODUCTION}/databases/db-1`; const SP = "11111111-2222-3333-4444-555555555555"; const HOST = "https://acme.cloud.databricks.com"; const SNAPSHOT = "/Workspace/Users/sp/src/01f0"; -const DEPLOYED_AT = "2026-10-01T12:00:00Z"; /** The app API's response for a deployed, running app bound to the deploy branch. */ function runningApp(overrides: Record = {}) { @@ -349,7 +348,6 @@ function runningApp(overrides: Record = {}) { app_status: { state: "RUNNING", message: "App is running." }, compute_status: { state: "ACTIVE", message: "App compute is running." }, active_deployment: { - create_time: DEPLOYED_AT, deployment_artifacts: { source_code_path: SNAPSHOT }, status: { state: "SUCCEEDED", message: "App started successfully" }, }, @@ -372,8 +370,10 @@ function deployedApp( yml?: string; spRole?: boolean; migrations?: Migration[]; - /** The deployed snapshot's journal; "unreadable" when export fails. */ - deployedJournal?: Migration[] | "unreadable"; + /** The deployed snapshot's journal: its entries, its raw text, or "unreadable" when its export fails. */ + deployedJournal?: Migration[] | string; + /** The deployed SQL by tag, else the committed SQL; "unreadable" when its export fails. */ + deployedSql?: Record; authenticatedHost?: string; } = {}, ) { @@ -410,15 +410,24 @@ function deployedApp( } if (method === "GET" && apiPath === "/api/2.0/workspace/export") { expect(query?.format).toBe("AUTO"); - requests.push(`workspace.export ${query?.path}`); + const file = String(query?.path); + requests.push(`workspace.export ${file}`); + const relative = file.slice(`${SNAPSHOT}/${MIGRATIONS}/`.length); const deployed = options.deployedJournal ?? options.migrations ?? []; - if (deployed === "unreadable") { + const content = + relative === "meta/_journal.json" + ? typeof deployed === "string" + ? deployed + : journal(deployed) + : (options.deployedSql?.[relative.replace(/\.sql$/, "")] ?? + `-- ${relative.replace(/\.sql$/, "")}\nselect 1;\n`); + if (content === "unreadable") { throw Object.assign(new Error("Permission denied"), { statusCode: 403, }); } return Response.json({ - content: Buffer.from(journal(deployed)).toString("base64"), + content: Buffer.from(content).toString("base64"), }); } throw new Error( @@ -458,7 +467,7 @@ function ymlWithApps(apps: string): string { } describe("runVerify --deployed", () => { - test("a running app bound to the deploy branch with every committed migration is ok, with no database connection and no role of yours there", async () => { + test("a running app whose snapshot has every committed migration, with the same journal entry and SQL, is ok, with no database connection and no role of yours there", async () => { const migrations = [{ tag: "0000_migration", when: 1000 }]; const { ctx, lakebase, requests } = deployedApp({ migrations }); @@ -468,13 +477,14 @@ describe("runVerify --deployed", () => { host: HOST, branch: PRODUCTION, app: "my-app", - migrations: { checked: "deployed code", missing: [] }, + migrations: { evidence: "verified" }, problems: [], ok: true, }); expect(requests).toEqual([ "apps.get my-app", - `workspace.export ${SNAPSHOT}/config/database/migrations/meta/_journal.json`, + `workspace.export ${SNAPSHOT}/${MIGRATIONS}/meta/_journal.json`, + `workspace.export ${SNAPSHOT}/${MIGRATIONS}/0000_migration.sql`, ]); expect(withBranchPool).not.toHaveBeenCalled(); expect(lakebase.calls.sort()).toEqual([ @@ -495,7 +505,11 @@ describe("runVerify --deployed", () => { }); expect(await runVerify(ctx, DEPLOYED)).toMatchObject({ - migrations: { checked: "deployed code", missing: ["0001_migration"] }, + migrations: { + evidence: "differs", + missing: ["0001_migration"], + changed: [], + }, problems: [ "app my-app's active deployment does not have 0001_migration, which this checkout commits: `databricks apps deploy` deploys it, and the app applies it on start", ], @@ -503,24 +517,126 @@ describe("runVerify --deployed", () => { }); }); - test("when the deployed code cannot be read, migrations generated after the deployment fail it", async () => { - const deployedAt = Date.parse(DEPLOYED_AT); - const { ctx } = deployedApp({ - migrations: [ - { tag: "0000_migration", when: deployedAt - 1000 }, - { tag: "0001_migration", when: deployedAt + 1000 }, + test("the same tag with other SQL, or with another journal entry, in the deployment fails it", async () => { + const migrations = [ + { tag: "0000_migration", when: 1000 }, + { tag: "0001_migration", when: 2000 }, + ]; + const { ctx, requests } = deployedApp({ + migrations, + deployedJournal: [ + { tag: "0000_migration", when: 1000 }, + { tag: "0001_migration", when: 3000 }, ], - deployedJournal: "unreadable", + deployedSql: { "0000_migration": "-- 0000_migration\nselect 2;\n" }, }); expect(await runVerify(ctx, DEPLOYED)).toMatchObject({ - migrations: { checked: "deploy time", missing: ["0001_migration"] }, + migrations: { + evidence: "differs", + missing: [], + changed: ["0000_migration", "0001_migration"], + }, problems: [ - `0001_migration was generated after app my-app's active deployment (${DEPLOYED_AT}), so it does not have it: \`databricks apps deploy\` deploys it, and the app applies it on start`, + "app my-app's active deployment has 0000_migration, 0001_migration with another journal entry or other SQL than this checkout commits. A migration that ran never runs again: restore them to what was deployed, and put the change in a new migration", ], + ok: false, + }); + // Its journal entry already differs, so its SQL is not read. + expect(requests).not.toContain( + `workspace.export ${SNAPSHOT}/${MIGRATIONS}/0001_migration.sql`, + ); + }); + + const JOURNAL = `${SNAPSHOT}/${MIGRATIONS}/meta/_journal.json`; + test.each([ + [ + "its journal cannot be exported", + { deployedJournal: "unreadable" }, + `exporting ${JOURNAL} failed: Permission denied`, + ], + [ + "its journal is empty", + { deployedJournal: "" }, + `${JOURNAL} exported no content`, + ], + [ + "its journal is not JSON", + { deployedJournal: "{" }, + `${JOURNAL} is not JSON: `, + ], + [ + "its journal is not drizzle-kit's", + { + deployedJournal: JSON.stringify({ dialect: "mysql", entries: [] }), + }, + `${JOURNAL} is not a drizzle-kit PostgreSQL migration journal`, + ], + [ + "a migration's SQL cannot be exported", + { deployedSql: { "0000_migration": "unreadable" } }, + `exporting ${SNAPSHOT}/${MIGRATIONS}/0000_migration.sql failed: Permission denied`, + ], + [ + "the deployment names no source snapshot", + { + app: runningApp({ + active_deployment: { status: { state: "SUCCEEDED" } }, + }), + }, + "its active deployment names no source snapshot", + ], + ] as const)( + "when %s, the migrations are unavailable and verify fails", + async (_label, options, reason) => { + const { ctx } = deployedApp({ + migrations: [{ tag: "0000_migration", when: 1000 }], + ...options, + }); + + expect(await runVerify(ctx, DEPLOYED)).toMatchObject({ + migrations: { + evidence: "unavailable", + reason: expect.stringContaining(reason), + }, + problems: [ + expect.stringContaining( + `verify cannot tell whether app my-app's active deployment has the committed migrations: ${reason}`, + ), + ], + ok: false, + }); + }, + ); + + test("with committed migrations, a report that stops before reading the deployment leaves them unverified", async () => { + const { ctx } = deployedApp({ + migrations: [{ tag: "0000_migration", when: 1000 }], + app: "missing", + }); + + expect(await runVerify(ctx, DEPLOYED)).toMatchObject({ + migrations: { + evidence: "unavailable", + reason: "verify stopped before reading the app's deployment", + }, + ok: false, }); }); + test("a committed migration whose SQL file is missing fails before any API call", async () => { + const { ctx, lakebase, requests } = deployedApp({ + migrations: [{ tag: "0000_migration", when: 1000 }], + }); + fs.rmSync(path.join(ctx.root, MIGRATIONS, "0000_migration.sql")); + + await expect(runVerify(ctx, DEPLOYED)).rejects.toThrow( + /^Missing .*0000_migration\.sql, listed in meta\/_journal\.json\. Restore it from version control\.$/, + ); + expect(requests).toEqual([]); + expect(lakebase.calls).toEqual([]); + }); + test("without committed migrations nothing is compared, and schema.ts is not needed", async () => { const { ctx, requests } = deployedApp(); fs.rmSync(path.join(ctx.root, "config/database/schema.ts")); diff --git a/packages/appkit/src/database/tooling/verify.ts b/packages/appkit/src/database/tooling/verify.ts index 84105ef0a..7cf005924 100644 --- a/packages/appkit/src/database/tooling/verify.ts +++ b/packages/appkit/src/database/tooling/verify.ts @@ -1,3 +1,4 @@ +import fs from "node:fs"; import path from "node:path"; import type { PoolClient } from "pg"; @@ -8,7 +9,10 @@ import { readMigrationDrift } from "../migrations/apply"; import { journalFile, type JournalEntry, + MigrationFilesError, + migrationFile, migrationsFolder, + parseJournal, readJournal, } from "../migrations/paths"; import type { DataPath } from "../runtime"; @@ -51,13 +55,19 @@ interface LocalVerifyReport { readonly ok: boolean; } -/** How the deployed migrations compare with the committed ones. */ -interface DeployedMigrations { - /** "deployed code": the deployment's journal; "deploy time": committed after the deployment was created. */ - readonly checked: "deployed code" | "deploy time"; - /** Committed migrations the active deployment does not have. */ - readonly missing: readonly string[]; -} +/** The committed migrations against the snapshot of the source the active deployment runs. */ +type DeployedMigrations = + /** The snapshot has every committed migration, with the same journal entry and SQL. */ + | { readonly evidence: "verified" } + | { + readonly evidence: "differs"; + /** Committed migrations the snapshot does not have. */ + readonly missing: readonly string[]; + /** Committed migrations the snapshot has with another journal entry or other SQL. */ + readonly changed: readonly string[]; + } + /** The snapshot could not be read, so whether it has them is unknown. */ + | { readonly evidence: "unavailable"; readonly reason: string }; /** `appkit db verify --deployed`: whether a target's deployed app runs on its branch with the committed migrations. */ interface DeployVerifyReport { @@ -68,10 +78,11 @@ interface DeployVerifyReport { readonly branch: string; /** The app the target deploys; null when it declares none. */ readonly app: string | null; - /** null when there are no committed migrations, or no deployment to compare with. */ + /** null only when this checkout commits no migration. */ readonly migrations: DeployedMigrations | null; /** What keeps the deployed app from using the branch, each with its fix. */ readonly problems: readonly string[]; + /** No problem, and the deployed migrations verified or none committed. */ readonly ok: boolean; } @@ -142,7 +153,6 @@ interface DeploymentStatus { } interface Deployment { - readonly create_time?: string; /** The snapshot of the source the deployment runs. */ readonly deployment_artifacts?: { readonly source_code_path?: string }; readonly status?: DeploymentStatus; @@ -186,87 +196,145 @@ async function readApp( } } -/** The tags in a deployed journal; null when it cannot be read, which includes a deployment without one. */ -async function readDeployedTags( - ctx: DbContext, - file: string, -): Promise | null> { +/** A workspace file's bytes; throws with the reason when the export fails or has no content. */ +async function exportFile(ctx: DbContext, file: string): Promise { + let content: string | undefined; try { const response = await ctx.workspace().request({ method: "GET", path: "/api/2.0/workspace/export", query: { path: file, format: "AUTO" }, }); - const { content } = (await response.json()) as { content?: string }; - if (!content) return null; - const journal = JSON.parse( - Buffer.from(content, "base64").toString("utf-8"), - ); - const entries: unknown[] = Array.isArray(journal?.entries) - ? journal.entries - : []; - return new Set( - entries - .map((entry) => (entry as { tag?: unknown } | null)?.tag) - .filter((tag): tag is string => typeof tag === "string"), - ); - } catch { - return null; + ({ content } = (await response.json()) as { content?: string }); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + throw new Error(`exporting ${file} failed: ${message}`, { cause: error }); } + if (!content) throw new Error(`${file} exported no content`); + return Buffer.from(content, "base64"); +} + +/** A committed migration's journal entry and the bytes of its SQL file. */ +interface CommittedMigration { + readonly entry: JournalEntry; + readonly sql: Buffer; +} + +function readCommitted(folder: string): CommittedMigration[] { + return (readJournal(folder)?.entries ?? []).map((entry) => { + const file = migrationFile(folder, entry.tag); + if (!fs.existsSync(file)) { + throw new MigrationFilesError( + `Missing ${file}, listed in meta/_journal.json. Restore it from version control.`, + ); + } + return { entry, sql: fs.readFileSync(file) }; + }); } /** - * Committed migrations the active deployment lacks: from the journal in the - * snapshot it runs when that is readable, else the ones generated after the - * deployment was created. + * The committed migrations against the snapshot of the source the active + * deployment runs: each needs the same journal entry (idx, when, + * breakpoints) and byte-identical SQL there. Only that snapshot is evidence. + * Anything verify cannot read in it makes the result unavailable, never a + * match. */ -async function deployedMigrations( +async function readDeployedMigrations( ctx: DbContext, deployment: Deployment | undefined, - committed: readonly JournalEntry[], + committed: readonly CommittedMigration[], ): Promise { - if (!deployment || committed.length === 0) return null; - const snapshot = deployment.deployment_artifacts?.source_code_path; - if (snapshot) { - const relative = path - .relative(ctx.root, journalFile(migrationsFolder(ctx.root))) - .split(path.sep) - .join("/"); - const deployed = await readDeployedTags( - ctx, - `${snapshot.replace(/\/+$/, "")}/${relative}`, - ); - if (deployed) { - return { - checked: "deployed code", - missing: committed - .filter((entry) => !deployed.has(entry.tag)) - .map((entry) => entry.tag), - }; + if (committed.length === 0) return null; + if (!deployment) { + return { + evidence: "unavailable", + reason: "the app has no active deployment", + }; + } + const snapshot = deployment.deployment_artifacts?.source_code_path?.replace( + /\/+$/, + "", + ); + if (!snapshot) { + return { + evidence: "unavailable", + reason: "its active deployment names no source snapshot", + }; + } + const folder = migrationsFolder(ctx.root); + const deployedPath = (file: string) => + `${snapshot}/${path.relative(ctx.root, file).split(path.sep).join("/")}`; + try { + const journalPath = deployedPath(journalFile(folder)); + const text = (await exportFile(ctx, journalPath)).toString("utf-8"); + let json: unknown; + try { + json = JSON.parse(text); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + throw new Error(`${journalPath} is not JSON: ${message}`, { + cause: error, + }); } + const deployed = parseJournal(json, journalPath).entries; + const compared = await Promise.all( + committed.map(async ({ entry, sql }) => { + const found = deployed.find((other) => other?.tag === entry.tag); + if (!found) return "missing"; + if ( + found.idx !== entry.idx || + found.when !== entry.when || + found.breakpoints !== entry.breakpoints + ) { + return "changed"; + } + const file = deployedPath(migrationFile(folder, entry.tag)); + return (await exportFile(ctx, file)).equals(sql) ? "same" : "changed"; + }), + ); + const tags = (status: "missing" | "changed") => + committed + .filter((_, index) => compared[index] === status) + .map(({ entry }) => entry.tag); + const missing = tags("missing"); + const changed = tags("changed"); + return missing.length === 0 && changed.length === 0 + ? { evidence: "verified" } + : { evidence: "differs", missing, changed }; + } catch (error) { + return { + evidence: "unavailable", + reason: error instanceof Error ? error.message : String(error), + }; } - const created = Date.parse(deployment.create_time ?? ""); - if (Number.isNaN(created)) return null; - return { - checked: "deploy time", - missing: committed - .filter((entry) => entry.when > created) - .map((entry) => entry.tag), - }; } -function migrationsProblem( +/** Why the deployed migrations do not match the committed ones, each with its fix; empty when verified. */ +function migrationsProblems( name: string, migrations: DeployedMigrations, - deployment: Deployment | undefined, -): string | null { - const { missing } = migrations; - if (missing.length === 0) return null; - const them = missing.length === 1 ? "it" : "them"; - const fix = `${DEPLOY} deploys ${them}, and the app applies ${them} on start`; - return migrations.checked === "deployed code" - ? `app ${name}'s active deployment does not have ${missing.join(", ")}, which this checkout commits: ${fix}` - : `${missing.join(", ")} ${missing.length === 1 ? "was" : "were"} generated after app ${name}'s active deployment (${deployment?.create_time}), so it does not have ${them}: ${fix}`; +): string[] { + if (migrations.evidence === "verified") return []; + if (migrations.evidence === "unavailable") { + return [ + `verify cannot tell whether app ${name}'s active deployment has the committed migrations: ${migrations.reason}`, + ]; + } + const problems: string[] = []; + const { missing, changed } = migrations; + if (missing.length > 0) { + const them = missing.length === 1 ? "it" : "them"; + problems.push( + `app ${name}'s active deployment does not have ${missing.join(", ")}, which this checkout commits: ${DEPLOY} deploys ${them}, and the app applies ${them} on start`, + ); + } + if (changed.length > 0) { + const them = changed.length === 1 ? "it" : "them"; + problems.push( + `app ${name}'s active deployment has ${changed.join(", ")} with another journal entry or other SQL than this checkout commits. A migration that ran never runs again: restore ${them} to what was deployed, and put the change in a new migration`, + ); + } + return problems; } /** The app's postgres resource against databricks.yml: same branch and database, and the permission to create its schema. */ @@ -395,7 +463,8 @@ function selectTarget( * Postgres role on the deploy branch is needed. The target's app must exist, * be bound to the branch and database databricks.yml names with * CAN_CONNECT_AND_CREATE, have a role on the branch for its service - * principal, be running, and run code with every committed migration. + * principal, be running, and run a snapshot with the journal entry and SQL of + * every committed migration. */ async function verifyDeploy( ctx: DbContext, @@ -447,10 +516,11 @@ async function verifyDeploy( } const branch = branchValue.name; const database = databaseValue?.name ?? null; + const committed = readCommitted(migrationsFolder(ctx.root)); const report = ( name: string | null, problems: string[], - migrations: DeployedMigrations | null = null, + migrations: DeployedMigrations | null, ): DeployVerifyReport => ({ kind: "deploy", target: target.name, @@ -459,13 +529,27 @@ async function verifyDeploy( app: name, migrations, problems, - ok: problems.length === 0, + ok: + problems.length === 0 && + (migrations === null || migrations.evidence === "verified"), }); + /** A report from before verify reads the deployment; its problems say why. */ + const stopped = (name: string | null, problems: string[]) => + report( + name, + problems, + committed.length === 0 + ? null + : { + evidence: "unavailable", + reason: "verify stopped before reading the app's deployment", + }, + ); if (!app) { - return report(null, [`target ${target.name} in ${file} declares no app`]); + return stopped(null, [`target ${target.name} in ${file} declares no app`]); } if (!app.name) { - return report(app.declaredName, [ + return stopped(app.declaredName, [ app.declaredName ? `${file} names the app ${app.declaredName}, with a reference verify cannot resolve` : `app ${app.key} in ${file} has no name`, @@ -482,7 +566,9 @@ async function verifyDeploy( ]); const branchId = resourceId(branch); if (!branchFound) { - return report(name, [`${file} deploys to ${branch}, which does not exist`]); + return stopped(name, [ + `${file} deploys to ${branch}, which does not exist`, + ]); } const problems: string[] = []; if (database && !databaseFound) { @@ -494,27 +580,23 @@ async function verifyDeploy( problems.push( `app ${name} is not deployed yet: ${DEPLOY} creates it and gives it access to ${branchId}`, ); - return report(name, problems); + return stopped(name, problems); } const binding = bindingProblems(deployed, name, branch, database, file); problems.push(...binding, ...runningProblems(deployed, name)); const sp = deployed.service_principal_client_id; - const committed = readJournal(migrationsFolder(ctx.root))?.entries ?? []; const [roles, migrations] = await Promise.all([ // Only meaningful once the app is bound to this branch. sp && binding.length === 0 ? api.listRoles(branch) : null, - deployedMigrations(ctx, deployed.active_deployment, committed), + readDeployedMigrations(ctx, deployed.active_deployment, committed), ]); if (roles && !roles.some((role) => role.status?.postgres_role === sp)) { problems.push( `the app's service principal ${sp} has no Postgres role on ${branchId}: ${DEPLOY} creates it from the app's postgres resource`, ); } - const missing = migrations - ? migrationsProblem(name, migrations, deployed.active_deployment) - : null; - if (missing) problems.push(missing); + if (migrations) problems.push(...migrationsProblems(name, migrations)); return report(name, problems, migrations); } diff --git a/packages/shared/src/cli/commands/db/output.ts b/packages/shared/src/cli/commands/db/output.ts index 326091077..ca814d8ad 100644 --- a/packages/shared/src/cli/commands/db/output.ts +++ b/packages/shared/src/cli/commands/db/output.ts @@ -158,11 +158,9 @@ function printDeployVerifyReport(report: DeployVerifyReport): void { ok( `${app} (${where}) is running, bound to ${branch} with CAN_CONNECT_AND_CREATE, and its service principal has a role there`, ); - if (report.migrations?.checked === "deployed code") { - ok("Its active deployment has every committed migration"); - } else if (report.migrations) { + if (report.migrations?.evidence === "verified") { ok( - "Every committed migration was generated before its active deployment; its code could not be read, so verify compared times", + "Its active deployment has every committed migration, with the same SQL", ); } return; diff --git a/packages/shared/src/cli/commands/db/tooling-contract.ts b/packages/shared/src/cli/commands/db/tooling-contract.ts index 4813122b0..119809064 100644 --- a/packages/shared/src/cli/commands/db/tooling-contract.ts +++ b/packages/shared/src/cli/commands/db/tooling-contract.ts @@ -89,10 +89,15 @@ export interface DeployVerifyReport { readonly host: string | null; readonly branch: string; readonly app: string | null; - readonly migrations: { - readonly checked: "deployed code" | "deploy time"; - readonly missing: readonly string[]; - } | null; + readonly migrations: + | { readonly evidence: "verified" } + | { + readonly evidence: "differs"; + readonly missing: readonly string[]; + readonly changed: readonly string[]; + } + | { readonly evidence: "unavailable"; readonly reason: string } + | null; readonly problems: readonly string[]; readonly ok: boolean; } diff --git a/packages/shared/src/cli/commands/db/verify.test.ts b/packages/shared/src/cli/commands/db/verify.test.ts index cddcc11ab..e5436e0a0 100644 --- a/packages/shared/src/cli/commands/db/verify.test.ts +++ b/packages/shared/src/cli/commands/db/verify.test.ts @@ -184,7 +184,7 @@ describe("appkit db verify", () => { host: "https://acme.cloud.databricks.com", branch: "projects/p/branches/production", app: "my-app", - migrations: { checked: "deployed code" as const, missing: [] }, + migrations: { evidence: "verified" as const }, problems: [], ok: true, }; @@ -205,26 +205,22 @@ describe("appkit db verify", () => { "app my-app (target prod on https://acme.cloud.databricks.com) is running, bound to production with CAN_CONNECT_AND_CREATE, and its service principal has a role there", ); expect(printed).toContain( - "Its active deployment has every committed migration", + "Its active deployment has every committed migration, with the same SQL", ); expect(error).not.toHaveBeenCalled(); expect(process.exitCode).toBeUndefined(); }); - test("says when migrations were compared by deploy time", async () => { - runVerify.mockResolvedValue({ - ...deploy, - migrations: { checked: "deploy time", missing: [] }, - }); + test("claims nothing about migrations when the checkout commits none", async () => { + runVerify.mockResolvedValue({ ...deploy, migrations: null }); await dbCommand.parseAsync(["verify", "--deployed"], { from: "user" }); expect(runVerify).toHaveBeenCalledWith(expect.anything(), { deployed: true, }); - expect(output(log)).toContain( - "Every committed migration was generated before its active deployment; its code could not be read, so verify compared times", - ); + expect(output(log)).not.toContain("committed migration"); + expect(process.exitCode).toBeUndefined(); }); test("lists each problem with its fix, and exits 1", async () => { diff --git a/packages/shared/src/cli/commands/db/verify.ts b/packages/shared/src/cli/commands/db/verify.ts index 3ffa1bece..944a093cd 100644 --- a/packages/shared/src/cli/commands/db/verify.ts +++ b/packages/shared/src/cli/commands/db/verify.ts @@ -49,9 +49,10 @@ ran that this checkout does not commit. branch's database. It checks that the target's app exists, that its postgres resource names the branch and database in databricks.yml with CAN_CONNECT_AND_CREATE, that its service principal has a role on the branch, -that the app is running, and that its active deployment has every committed -migration. It refuses when the target's workspace.host is not the workspace -this shell is authenticated against. +that the app is running, and that the source its active deployment runs has +every committed migration, with the same journal entry and SQL. When verify +cannot read that source, it fails. It refuses when the target's +workspace.host is not the workspace this shell is authenticated against. Exits 1 on any finding. Nothing is written or created. From 4a480305e17188006660cb9eb6cc4512c421f56d Mon Sep 17 00:00:00 2001 From: ditadi Date: Fri, 30 Oct 2026 17:43:31 +0000 Subject: [PATCH 3/8] fix(appkit): read the deployed journal even when the checkout commits no migration verify --deployed returned no migration result before reading the snapshot when the checkout committed none, so running it from a bundle root whose app lives in a subfolder passed without looking at the deployment. It now always reads the snapshot's journal. With nothing committed, a snapshot without migrations passes and one with migrations is unavailable, naming them. With migrations committed, ones the snapshot has and the checkout does not commit fail too, as in local verify. A snapshot without a journal (a 404) has no migrations, so every committed one is missing and the fix is to deploy. Signed-off-by: ditadi --- .../src/database/tooling/tests/verify.test.ts | 94 ++++++++++++++-- .../appkit/src/database/tooling/verify.ts | 101 ++++++++++++------ .../src/cli/commands/db/tooling-contract.ts | 1 + 3 files changed, 155 insertions(+), 41 deletions(-) diff --git a/packages/appkit/src/database/tooling/tests/verify.test.ts b/packages/appkit/src/database/tooling/tests/verify.test.ts index be79e597f..d83420d82 100644 --- a/packages/appkit/src/database/tooling/tests/verify.test.ts +++ b/packages/appkit/src/database/tooling/tests/verify.test.ts @@ -370,9 +370,9 @@ function deployedApp( yml?: string; spRole?: boolean; migrations?: Migration[]; - /** The deployed snapshot's journal: its entries, its raw text, or "unreadable" when its export fails. */ + /** The deployed snapshot's journal: its entries, its raw text, "unreadable" when its export fails, or "missing" when it does not exist. */ deployedJournal?: Migration[] | string; - /** The deployed SQL by tag, else the committed SQL; "unreadable" when its export fails. */ + /** The deployed SQL by tag, else the committed SQL; "unreadable" or "missing" as for the journal. */ deployedSql?: Record; authenticatedHost?: string; } = {}, @@ -426,6 +426,11 @@ function deployedApp( statusCode: 403, }); } + if (content === "missing") { + throw Object.assign(new Error("Path does not exist."), { + statusCode: 404, + }); + } return Response.json({ content: Buffer.from(content).toString("base64"), }); @@ -637,15 +642,88 @@ describe("runVerify --deployed", () => { expect(lakebase.calls).toEqual([]); }); - test("without committed migrations nothing is compared, and schema.ts is not needed", async () => { - const { ctx, requests } = deployedApp(); - fs.rmSync(path.join(ctx.root, "config/database/schema.ts")); + test.each([ + ["has no journal", "missing"], + ["has a journal without entries", []], + ] as const)( + "without committed migrations, a snapshot that %s passes, and schema.ts is not needed", + async (_label, deployedJournal) => { + const { ctx, requests } = deployedApp({ + deployedJournal: deployedJournal as Migration[] | string, + }); + fs.rmSync(path.join(ctx.root, "config/database/schema.ts")); + + expect(await runVerify(ctx, DEPLOYED)).toMatchObject({ + migrations: null, + ok: true, + }); + expect(requests).toEqual([ + "apps.get my-app", + `workspace.export ${JOURNAL}`, + ]); + }, + ); + + test("without committed migrations, a snapshot with migrations fails, naming them: verify ran outside the app's root, or the checkout is behind", async () => { + const { ctx } = deployedApp({ + deployedJournal: [{ tag: "0000_migration", when: 1000 }], + }); expect(await runVerify(ctx, DEPLOYED)).toMatchObject({ - migrations: null, - ok: true, + migrations: { + evidence: "unavailable", + reason: + "the deployment has 0000_migration, and this checkout commits no migration: run verify from the app's root, or pull them", + }, + problems: [ + "verify cannot tell whether app my-app's active deployment has the committed migrations: the deployment has 0000_migration, and this checkout commits no migration: run verify from the app's root, or pull them", + ], + ok: false, + }); + }); + + test("a migration the deployment has and this checkout does not commit fails it", async () => { + const migrations = [{ tag: "0000_migration", when: 1000 }]; + const { ctx } = deployedApp({ + migrations, + deployedJournal: [...migrations, { tag: "0001_migration", when: 2000 }], + }); + + expect(await runVerify(ctx, DEPLOYED)).toMatchObject({ + migrations: { + evidence: "differs", + missing: [], + changed: [], + unknown: ["0001_migration"], + }, + problems: [ + "app my-app's active deployment has 0001_migration, which this checkout does not commit: pull it, then run verify again", + ], + ok: false, + }); + }); + + test("a snapshot without a journal lacks every committed migration: deploy them", async () => { + const { ctx } = deployedApp({ + migrations: [ + { tag: "0000_migration", when: 1000 }, + { tag: "0001_migration", when: 2000 }, + ], + deployedJournal: "missing", + }); + + expect(await runVerify(ctx, DEPLOYED)).toMatchObject({ + migrations: { + evidence: "differs", + missing: ["0000_migration", "0001_migration"], + changed: [], + unknown: [], + }, + problems: [ + "app my-app's active deployment does not have 0000_migration, 0001_migration, which this checkout commits: `databricks apps deploy` deploys them, and the app applies them on start", + ], + ok: false, }); - expect(requests).toEqual(["apps.get my-app"]); }); test("--target checks that target's app, branch and host", async () => { diff --git a/packages/appkit/src/database/tooling/verify.ts b/packages/appkit/src/database/tooling/verify.ts index 7cf005924..9c24e3798 100644 --- a/packages/appkit/src/database/tooling/verify.ts +++ b/packages/appkit/src/database/tooling/verify.ts @@ -65,6 +65,8 @@ type DeployedMigrations = readonly missing: readonly string[]; /** Committed migrations the snapshot has with another journal entry or other SQL. */ readonly changed: readonly string[]; + /** Migrations the snapshot has that this checkout does not commit. */ + readonly unknown: readonly string[]; } /** The snapshot could not be read, so whether it has them is unknown. */ | { readonly evidence: "unavailable"; readonly reason: string }; @@ -78,11 +80,11 @@ interface DeployVerifyReport { readonly branch: string; /** The app the target deploys; null when it declares none. */ readonly app: string | null; - /** null only when this checkout commits no migration. */ + /** null only when neither this checkout nor the deployed snapshot has a migration. */ readonly migrations: DeployedMigrations | null; /** What keeps the deployed app from using the branch, each with its fix. */ readonly problems: readonly string[]; - /** No problem, and the deployed migrations verified or none committed. */ + /** No problem, and the deployed migrations verified or none on either side. */ readonly ok: boolean; } @@ -196,8 +198,11 @@ async function readApp( } } -/** A workspace file's bytes; throws with the reason when the export fails or has no content. */ -async function exportFile(ctx: DbContext, file: string): Promise { +/** A workspace file's bytes; null when it does not exist. Throws with the reason when the export fails otherwise or has no content. */ +async function exportFile( + ctx: DbContext, + file: string, +): Promise { let content: string | undefined; try { const response = await ctx.workspace().request({ @@ -207,6 +212,9 @@ async function exportFile(ctx: DbContext, file: string): Promise { }); ({ content } = (await response.json()) as { content?: string }); } catch (error) { + if ((error as { statusCode?: unknown } | null)?.statusCode === 404) { + return null; + } const message = error instanceof Error ? error.message : String(error); throw new Error(`exporting ${file} failed: ${message}`, { cause: error }); } @@ -235,16 +243,16 @@ function readCommitted(folder: string): CommittedMigration[] { /** * The committed migrations against the snapshot of the source the active * deployment runs: each needs the same journal entry (idx, when, - * breakpoints) and byte-identical SQL there. Only that snapshot is evidence. - * Anything verify cannot read in it makes the result unavailable, never a - * match. + * breakpoints) and byte-identical SQL there, and the snapshot may have no + * migration the checkout does not commit. A snapshot without a journal has no + * migration. Only that snapshot is evidence. Anything verify cannot read in it + * makes the result unavailable, never a match. */ async function readDeployedMigrations( ctx: DbContext, deployment: Deployment | undefined, committed: readonly CommittedMigration[], ): Promise { - if (committed.length === 0) return null; if (!deployment) { return { evidence: "unavailable", @@ -266,17 +274,34 @@ async function readDeployedMigrations( `${snapshot}/${path.relative(ctx.root, file).split(path.sep).join("/")}`; try { const journalPath = deployedPath(journalFile(folder)); - const text = (await exportFile(ctx, journalPath)).toString("utf-8"); - let json: unknown; - try { - json = JSON.parse(text); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - throw new Error(`${journalPath} is not JSON: ${message}`, { - cause: error, - }); + const journal = await exportFile(ctx, journalPath); + let deployed: JournalEntry[] = []; + // A snapshot without a journal has no migration. + if (journal) { + let json: unknown; + try { + json = JSON.parse(journal.toString("utf-8")); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + throw new Error(`${journalPath} is not JSON: ${message}`, { + cause: error, + }); + } + deployed = parseJournal(json, journalPath).entries; + } + const committedTags = new Set(committed.map(({ entry }) => entry.tag)); + const unknown = deployed.flatMap((entry) => + typeof entry?.tag === "string" && !committedTags.has(entry.tag) + ? [entry.tag] + : [], + ); + if (committed.length === 0) { + if (deployed.length === 0) return null; + return { + evidence: "unavailable", + reason: `the deployment has ${unknown.join(", ")}, and this checkout commits no migration: run verify from the app's root, or pull them`, + }; } - const deployed = parseJournal(json, journalPath).entries; const compared = await Promise.all( committed.map(async ({ entry, sql }) => { const found = deployed.find((other) => other?.tag === entry.tag); @@ -289,7 +314,13 @@ async function readDeployedMigrations( return "changed"; } const file = deployedPath(migrationFile(folder, entry.tag)); - return (await exportFile(ctx, file)).equals(sql) ? "same" : "changed"; + const bytes = await exportFile(ctx, file); + if (!bytes) { + throw new Error( + `${file} is listed in the deployed journal but missing from the snapshot`, + ); + } + return bytes.equals(sql) ? "same" : "changed"; }), ); const tags = (status: "missing" | "changed") => @@ -298,9 +329,9 @@ async function readDeployedMigrations( .map(({ entry }) => entry.tag); const missing = tags("missing"); const changed = tags("changed"); - return missing.length === 0 && changed.length === 0 + return missing.length === 0 && changed.length === 0 && unknown.length === 0 ? { evidence: "verified" } - : { evidence: "differs", missing, changed }; + : { evidence: "differs", missing, changed, unknown }; } catch (error) { return { evidence: "unavailable", @@ -321,7 +352,7 @@ function migrationsProblems( ]; } const problems: string[] = []; - const { missing, changed } = migrations; + const { missing, changed, unknown } = migrations; if (missing.length > 0) { const them = missing.length === 1 ? "it" : "them"; problems.push( @@ -334,6 +365,12 @@ function migrationsProblems( `app ${name}'s active deployment has ${changed.join(", ")} with another journal entry or other SQL than this checkout commits. A migration that ran never runs again: restore ${them} to what was deployed, and put the change in a new migration`, ); } + if (unknown.length > 0) { + const them = unknown.length === 1 ? "it" : "them"; + problems.push( + `app ${name}'s active deployment has ${unknown.join(", ")}, which this checkout does not commit: pull ${them}, then run verify again`, + ); + } return problems; } @@ -535,16 +572,10 @@ async function verifyDeploy( }); /** A report from before verify reads the deployment; its problems say why. */ const stopped = (name: string | null, problems: string[]) => - report( - name, - problems, - committed.length === 0 - ? null - : { - evidence: "unavailable", - reason: "verify stopped before reading the app's deployment", - }, - ); + report(name, problems, { + evidence: "unavailable", + reason: "verify stopped before reading the app's deployment", + }); if (!app) { return stopped(null, [`target ${target.name} in ${file} declares no app`]); } @@ -596,7 +627,11 @@ async function verifyDeploy( `the app's service principal ${sp} has no Postgres role on ${branchId}: ${DEPLOY} creates it from the app's postgres resource`, ); } - if (migrations) problems.push(...migrationsProblems(name, migrations)); + // Without an active deployment, runningProblems already says why; the + // migrations stay unavailable, so the report is not ok either way. + if (migrations && deployed.active_deployment) { + problems.push(...migrationsProblems(name, migrations)); + } return report(name, problems, migrations); } diff --git a/packages/shared/src/cli/commands/db/tooling-contract.ts b/packages/shared/src/cli/commands/db/tooling-contract.ts index 119809064..3c120d125 100644 --- a/packages/shared/src/cli/commands/db/tooling-contract.ts +++ b/packages/shared/src/cli/commands/db/tooling-contract.ts @@ -95,6 +95,7 @@ export interface DeployVerifyReport { readonly evidence: "differs"; readonly missing: readonly string[]; readonly changed: readonly string[]; + readonly unknown: readonly string[]; } | { readonly evidence: "unavailable"; readonly reason: string } | null; From 9466b830442689717aca1fadfbd42f00534ff060 Mon Sep 17 00:00:00 2001 From: ditadi Date: Fri, 30 Oct 2026 17:44:03 +0000 Subject: [PATCH 4/8] test(appkit): prove verify never reads a deployment's live source folder The no-snapshot case gave the deployment no source_code_path at all, so a fallback to that live, mutable folder would have passed. The deployment now has one, and the test asserts the result is unavailable with no export. Signed-off-by: ditadi --- .../src/database/tooling/tests/verify.test.ts | 33 ++++++++++++++----- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/packages/appkit/src/database/tooling/tests/verify.test.ts b/packages/appkit/src/database/tooling/tests/verify.test.ts index d83420d82..485af5d50 100644 --- a/packages/appkit/src/database/tooling/tests/verify.test.ts +++ b/packages/appkit/src/database/tooling/tests/verify.test.ts @@ -582,15 +582,6 @@ describe("runVerify --deployed", () => { { deployedSql: { "0000_migration": "unreadable" } }, `exporting ${SNAPSHOT}/${MIGRATIONS}/0000_migration.sql failed: Permission denied`, ], - [ - "the deployment names no source snapshot", - { - app: runningApp({ - active_deployment: { status: { state: "SUCCEEDED" } }, - }), - }, - "its active deployment names no source snapshot", - ], ] as const)( "when %s, the migrations are unavailable and verify fails", async (_label, options, reason) => { @@ -614,6 +605,30 @@ describe("runVerify --deployed", () => { }, ); + test("a deployment that names no snapshot is unavailable, and its live source folder is never read", async () => { + const { ctx, requests } = deployedApp({ + migrations: [{ tag: "0000_migration", when: 1000 }], + app: runningApp({ + active_deployment: { + // The live folder the deployment came from, which can change after it. + source_code_path: "/Workspace/Users/alice@example.com/my-app", + status: { state: "SUCCEEDED" }, + }, + }), + }); + + expect(await runVerify(ctx, DEPLOYED)).toMatchObject({ + migrations: { + evidence: "unavailable", + reason: "its active deployment names no source snapshot", + }, + ok: false, + }); + expect( + requests.filter((request) => request.startsWith("workspace.export")), + ).toEqual([]); + }); + test("with committed migrations, a report that stops before reading the deployment leaves them unverified", async () => { const { ctx } = deployedApp({ migrations: [{ tag: "0000_migration", when: 1000 }], From 176d7a11c14d80f558c44e113d63af54d5ca6b6b Mon Sep 17 00:00:00 2001 From: ditadi Date: Fri, 30 Oct 2026 17:46:32 +0000 Subject: [PATCH 5/8] fix(appkit): resolve verify's target with variable-overrides.json verify --deployed read the target with databricks.yml's own values only, so a branch set only in .databricks/bundle//variable-overrides.json was refused as unresolved. It now reads the target as a deploy from this checkout does, and as the deploy guard already reads it: the overrides file's values when the file exists. The workspace host is resolved with the same values, and a refusal names the file. Signed-off-by: ditadi --- .../appkit/src/database/tooling/targets.ts | 13 +++----- .../src/database/tooling/tests/verify.test.ts | 31 +++++++++++++++++++ .../appkit/src/database/tooling/verify.ts | 29 ++++++++++------- 3 files changed, 53 insertions(+), 20 deletions(-) diff --git a/packages/appkit/src/database/tooling/targets.ts b/packages/appkit/src/database/tooling/targets.ts index 5026a9522..9b555afeb 100644 --- a/packages/appkit/src/database/tooling/targets.ts +++ b/packages/appkit/src/database/tooling/targets.ts @@ -127,6 +127,8 @@ interface BundleApp { /** What one target binds with one set of variable values. */ interface TargetBindings { + /** workspace.host of the target, else of the bundle; null when neither names one the files resolve. */ + readonly host: string | null; readonly apps: readonly BundleApp[]; /** The target's postgres_branch variable; null when the bundle declares none. */ readonly postgresBranch: BundleValue | null; @@ -139,8 +141,6 @@ export interface BundleTarget extends TargetBindings { readonly name: string; /** `default: true`, or the one target of a bundle that declares none. */ readonly isDefault: boolean; - /** workspace.host of the target, else of the bundle; null when neither names one the files resolve. */ - readonly host: string | null; /** * The same bindings with this checkout's variable-overrides.json applied, * which a deploy from here uses ahead of the target's own values; null @@ -454,6 +454,9 @@ export function readBundle(root: string): Bundle | null { ) : null; return { + host: resolve( + record(target?.workspace)?.host ?? record(bundle.workspace)?.host, + ), apps: bundleApps(apps ?? {}, resolve), postgresBranch: variable("postgres_branch", "branch"), postgresDatabase: variable("postgres_database", "database"), @@ -463,12 +466,6 @@ export function readBundle(root: string): Bundle | null { return { name, isDefault: target?.default === true, - host: resolverFor( - bundle, - target, - name, - null, - )(record(target?.workspace)?.host ?? record(bundle.workspace)?.host), ...bindings(null), overridden: overrides ? { file: overrides.file, ...bindings(overrides.values) } diff --git a/packages/appkit/src/database/tooling/tests/verify.test.ts b/packages/appkit/src/database/tooling/tests/verify.test.ts index 485af5d50..85a2aee09 100644 --- a/packages/appkit/src/database/tooling/tests/verify.test.ts +++ b/packages/appkit/src/database/tooling/tests/verify.test.ts @@ -803,6 +803,37 @@ describe("runVerify --deployed", () => { expect(lakebase.calls).toEqual([]); }); + test("a branch that only variable-overrides.json sets is the one verify checks, as a deploy from this checkout uses it", async () => { + // The target gives the variables no value; only the overrides file does. + const yml = `${databricksYml(PRODUCTION, DATABASE).replace( + / {4}variables:\n(?: {6}.*\n)+/, + "", + )} workspace:\n host: ${HOST}\n`; + const { ctx } = deployedApp({ yml }); + await expect(runVerify(ctx, DEPLOYED)).rejects.toThrow( + 'No deploy branch: resources.apps.app.resources[0].postgres.branch in target default is "${var.postgres_branch}", which this checkout\'s files do not resolve to a branch.', + ); + + const overrides = path.join( + ctx.root, + ".databricks/bundle/default/variable-overrides.json", + ); + fs.mkdirSync(path.dirname(overrides), { recursive: true }); + fs.writeFileSync( + overrides, + JSON.stringify({ + postgres_branch: PRODUCTION, + postgres_database: DATABASE, + }), + ); + + expect(await runVerify(ctx, DEPLOYED)).toMatchObject({ + branch: PRODUCTION, + problems: [], + ok: true, + }); + }); + test("a host written without a scheme or with a trailing slash matches", async () => { const { ctx } = deployedApp({ yml: ymlWithHost("acme.cloud.databricks.com/"), diff --git a/packages/appkit/src/database/tooling/verify.ts b/packages/appkit/src/database/tooling/verify.ts index 9c24e3798..1221ad020 100644 --- a/packages/appkit/src/database/tooling/verify.ts +++ b/packages/appkit/src/database/tooling/verify.ts @@ -462,13 +462,14 @@ function hostOf(url: string): string { /** The API calls go to the workspace this shell is authenticated against, which must be the target's. */ async function assertTargetWorkspace( ctx: DbContext, - target: BundleTarget, + target: string, + host: string | null, ): Promise { - if (!target.host) return; + if (!host) return; const authenticated = await ctx.workspace().getHost(); - if (hostOf(authenticated) === hostOf(target.host)) return; + if (hostOf(authenticated) === hostOf(host)) return; throw new Error( - `Target ${target.name} deploys to ${target.host}, but this shell is authenticated against ${authenticated}. Set DATABRICKS_CONFIG_PROFILE to a profile for ${target.host}, then rerun.`, + `Target ${target} deploys to ${host}, but this shell is authenticated against ${authenticated}. Set DATABRICKS_CONFIG_PROFILE to a profile for ${host}, then rerun.`, ); } @@ -520,21 +521,25 @@ async function verifyDeploy( targetName, defaultBundleTarget(bundle), ); - const bound = target.apps.filter((app) => app.postgres.length > 0); + // What a deploy from this checkout uses: variable-overrides.json's values + // when it has the file, as the deploy guard reads them. + const deploy = target.overridden ?? target; + const withFile = target.overridden ? ` with ${target.overridden.file}` : ""; + const bound = deploy.apps.filter((app) => app.postgres.length > 0); if (bound.length > 1) { throw new Error( `Target ${target.name} deploys ${bound.length} apps with a postgres resource (${bound.map((app) => app.key).join(", ")}); verify checks one app per target.`, ); } const app = - bound[0] ?? (target.apps.length === 1 ? target.apps[0] : undefined); + bound[0] ?? (deploy.apps.length === 1 ? deploy.apps[0] : undefined); if (app && app.postgres.length > 1) { throw new Error( `App ${app.key} in target ${target.name} has ${app.postgres.length} postgres resources (${app.postgres.map((binding) => binding.at).join(", ")}); verify checks one per app.`, ); } const declared = app?.postgres[0]; - const branchValue = declared ? declared.branch : target.postgresBranch; + const branchValue = declared ? declared.branch : deploy.postgresBranch; if (!branchValue) { throw new Error( `No deploy branch: target ${target.name} in ${file} declares no app with a postgres resource.`, @@ -542,13 +547,13 @@ async function verifyDeploy( } if (branchValue.kind === "unresolved") { throw new Error( - `No deploy branch: ${declared ? `${declared.at}.branch` : "variables.postgres_branch"} in target ${target.name} is "${branchValue.written}", which this checkout's files do not resolve to a branch.`, + `No deploy branch: ${declared ? `${declared.at}.branch` : "variables.postgres_branch"} in target ${target.name}${withFile} is "${branchValue.written}", which this checkout's files do not resolve to a branch.`, ); } - const databaseValue = declared ? declared.database : target.postgresDatabase; + const databaseValue = declared ? declared.database : deploy.postgresDatabase; if (databaseValue?.kind === "unresolved") { throw new Error( - `No deploy database: ${declared ? `${declared.at}.database` : "variables.postgres_database"} in target ${target.name} is "${databaseValue.written}", which this checkout's files do not resolve to a database.`, + `No deploy database: ${declared ? `${declared.at}.database` : "variables.postgres_database"} in target ${target.name}${withFile} is "${databaseValue.written}", which this checkout's files do not resolve to a database.`, ); } const branch = branchValue.name; @@ -561,7 +566,7 @@ async function verifyDeploy( ): DeployVerifyReport => ({ kind: "deploy", target: target.name, - host: target.host, + host: deploy.host, branch, app: name, migrations, @@ -588,7 +593,7 @@ async function verifyDeploy( } const name = app.name; - await assertTargetWorkspace(ctx, target); + await assertTargetWorkspace(ctx, target.name, deploy.host); const api = ctx.api(); const [branchFound, databaseFound, deployed] = await Promise.all([ api.getBranch(branch), From 3876ae5cb27bc69bbaf6b7dbe2232903a2560583 Mon Sep 17 00:00:00 2001 From: ditadi Date: Fri, 30 Oct 2026 17:47:11 +0000 Subject: [PATCH 6/8] docs(appkit): say which db commands refuse and which take the history lock With an unresolved reference in databricks.yml, init, clean and seed refuse every branch, not only migrate. Only migrate, introspect, clean and init take the history lock; seed, verify and clean --delete-branch take none. Signed-off-by: ditadi --- docs/docs/plugins/database.md | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/docs/docs/plugins/database.md b/docs/docs/plugins/database.md index c1e588dd9..c88947fd3 100644 --- a/docs/docs/plugins/database.md +++ b/docs/docs/plugins/database.md @@ -144,10 +144,11 @@ checkout uses the file's value. If a branch or database reference has no value in those files, `migrate` cannot tell which branch the reference names. Examples are a target that leaves `postgres_branch` unset and `${workspace.current_user.short_name}`. While any such -reference exists, `migrate` applies migrations to no branch. The message names -the target and where the reference is. To continue, give the reference a value -in `databricks.yml`, or in the `variable-overrides.json` the message names. A -`variable-overrides.json` that is not a JSON object stops `migrate` too. +reference exists, `migrate` applies migrations to no branch, and `init`, `clean` +and `seed` refuse every branch. The message names the target and where the +reference is. To continue, give the reference a value in `databricks.yml`, or in +the `variable-overrides.json` the message names. A `variable-overrides.json` +that is not a JSON object stops these commands too. The check reads only this checkout's files. It does not see values passed at deploy time with `--var` or `BUNDLE_VAR_*`, or resources defined in Python. It @@ -157,11 +158,12 @@ protected branches. ### One command at a time While `migrate`, `introspect`, `clean` or `init` runs, it holds -`config/database/.appkit-db.lock`. A second `appkit db` command in the same -checkout fails and names the process that holds the lock. The lock goes when -the command finishes or exits. `init` holds it for its whole run, including the -clean and migrate it starts. `init --schema none` changes no migration history, -so it takes no lock. +`config/database/.appkit-db.lock`. Another of these commands in the same +checkout fails and names the process that holds the lock. `seed`, `verify` and +`clean --delete-branch` change no migration history and take no lock. The lock +goes when the command finishes or exits. `init` holds it for its whole run, +including the clean and migrate it starts. `init --schema none` changes no +migration history, so it takes no lock. A command stopped by a signal, such as SIGKILL, can leave the file behind. The next command then names the holder. When that process no longer runs, the From bfde0741656f9ab74d9689279600e1c6eea98438 Mon Sep 17 00:00:00 2001 From: ditadi Date: Fri, 30 Oct 2026 18:25:22 +0000 Subject: [PATCH 7/8] fix(appkit): give no init advice while every branch is refused While databricks.yml has a reference the checkout's files cannot resolve, migrate applies to no branch and init, introspect, clean and seed refuse every branch, yet introspect and migrate still pointed at init. One guard function now builds every refusal: it names the references and where to give them values, and gives the caller's advice only when some branch can take writes. The .env branch carries the boundary that classified it. Signed-off-by: ditadi --- docs/docs/plugins/database.md | 10 +-- packages/appkit/src/database/tooling/guard.ts | 70 ++++++++++++++----- packages/appkit/src/database/tooling/init.ts | 15 +++- .../src/database/tooling/introspect/index.ts | 7 +- .../introspect/tests/introspect.test.ts | 35 +++++++++- .../appkit/src/database/tooling/migrate.ts | 5 +- .../database/tooling/tests/clean-seed.test.ts | 1 + .../src/database/tooling/tests/guard.test.ts | 56 ++++++++++++++- .../src/database/tooling/tests/init.test.ts | 6 +- .../database/tooling/tests/migrate.test.ts | 1 + 10 files changed, 174 insertions(+), 32 deletions(-) diff --git a/docs/docs/plugins/database.md b/docs/docs/plugins/database.md index c88947fd3..a7efdeede 100644 --- a/docs/docs/plugins/database.md +++ b/docs/docs/plugins/database.md @@ -144,11 +144,11 @@ checkout uses the file's value. If a branch or database reference has no value in those files, `migrate` cannot tell which branch the reference names. Examples are a target that leaves `postgres_branch` unset and `${workspace.current_user.short_name}`. While any such -reference exists, `migrate` applies migrations to no branch, and `init`, `clean` -and `seed` refuse every branch. The message names the target and where the -reference is. To continue, give the reference a value in `databricks.yml`, or in -the `variable-overrides.json` the message names. A `variable-overrides.json` -that is not a JSON object stops these commands too. +reference exists, `migrate` applies migrations to no branch, and `init`, +`introspect`, `clean` and `seed` refuse every branch. The message names the +target and where the reference is. To continue, give the reference a value in +`databricks.yml`, or in the `variable-overrides.json` the message names. A +`variable-overrides.json` that is not a JSON object stops these commands too. The check reads only this checkout's files. It does not see values passed at deploy time with `--var` or `BUNDLE_VAR_*`, or resources defined in Python. It diff --git a/packages/appkit/src/database/tooling/guard.ts b/packages/appkit/src/database/tooling/guard.ts index d6a444b10..35d76e995 100644 --- a/packages/appkit/src/database/tooling/guard.ts +++ b/packages/appkit/src/database/tooling/guard.ts @@ -27,29 +27,57 @@ interface Classification { /** * Why databricks.yml makes a branch one it deploys to; null when it does not. - * A reference the files cannot resolve may name any branch, so it refuses all. + * A reference the files cannot resolve may name any branch, so it refuses all; + * everyBranchRefusal names the references. */ export function deployRefusal( boundary: DeployBoundary, branch: string, ): string | null { if (boundary.unresolved.length > 0) { - const references = boundary.unresolved - .map( - ({ target, at, written, file }) => - `${at} in target ${target}${file ? ` with ${file}` : ""} (${written ? JSON.stringify(written) : "no value"})`, - ) - .join(", "); - const where = boundary.unresolved.some(({ file }) => file) - ? "databricks.yml or the variable-overrides.json named" - : "databricks.yml"; - return `databricks.yml may deploy to it: the files do not resolve ${references}; give ${boundary.unresolved.length > 1 ? "them values" : "it a value"} in ${where}`; + return "databricks.yml may deploy to it through a reference this checkout's files do not resolve"; } return boundary.branches.has(branch) ? "it is a branch databricks.yml deploys to" : null; } +/** + * While databricks.yml has a reference this checkout's files cannot resolve, + * any branch may be one it deploys to: `migrate` applies migrations to no + * branch, and `init`, `introspect`, `clean` and `seed` refuse every branch. + * This says so, names the references and where to give them values; null when + * every reference resolves. + */ +export function everyBranchRefusal(boundary: DeployBoundary): string | null { + if (boundary.unresolved.length === 0) return null; + const references = boundary.unresolved + .map( + ({ target, at, written, file }) => + `${at} in target ${target}${file ? ` with ${file}` : ""} (${written ? JSON.stringify(written) : "no value"})`, + ) + .join(", "); + const where = boundary.unresolved.some(({ file }) => file) + ? "databricks.yml or the variable-overrides.json named" + : "databricks.yml"; + return `The files do not resolve ${references}, so \`appkit db migrate\` applies migrations to no branch, and \`init\`, \`introspect\`, \`clean\` and \`seed\` refuse every branch. Give ${boundary.unresolved.length > 1 ? "them values" : "it a value"} in ${where}`; +} + +/** + * Why a command writes nothing to a branch: `why`, then `advice` when it + * applies. While every branch is refused (see everyBranchRefusal), advice that + * leads to `appkit db init` or another branch cannot help, so the text names + * the references to resolve instead. + */ +export function writeRefusal( + why: string, + boundary: DeployBoundary, + advice?: string, +): string { + const next = everyBranchRefusal(boundary) ?? advice; + return next ? `${why}. ${next}` : why; +} + export function classifyBranch(input: { branch: LakebaseBranch; userName: string; @@ -86,14 +114,19 @@ export function takesWrites(kind: BranchKind): boolean { return kind === "dev" || kind === "session"; } -/** The .env branch and what it is to this developer. */ +/** The .env branch, what it is to this developer, and the boundary that decided it. */ interface EnvBranch extends Classification { readonly branch: LakebaseBranch; + readonly boundary: DeployBoundary; } -/** Why a command does not write to the branch. */ -export function notYourBranch(env: EnvBranch): string { - return `${resourceId(env.branch.name)} is not your dev or session branch (${env.reason})`; +/** Why a command does not write to the .env branch, then `advice` when it applies (see writeRefusal). */ +export function notYourBranch(env: EnvBranch, advice?: string): string { + return writeRefusal( + `${resourceId(env.branch.name)} is not your dev or session branch (${env.reason})`, + env.boundary, + advice, + ); } /** @@ -113,11 +146,16 @@ export async function resolveEnvBranch( ]); if (!branch) { throw new Error( - `The .env branch ${name} does not exist (deleted or expired). Run \`appkit db init\` to create or pick one.`, + `${writeRefusal( + `The .env branch ${name} does not exist (deleted or expired)`, + boundary, + "Run `appkit db init` to create or pick one", + )}.`, ); } return { branch, + boundary, ...classifyBranch({ branch, userName, diff --git a/packages/appkit/src/database/tooling/init.ts b/packages/appkit/src/database/tooling/init.ts index fa75f2eac..90a08e4fe 100644 --- a/packages/appkit/src/database/tooling/init.ts +++ b/packages/appkit/src/database/tooling/init.ts @@ -13,7 +13,12 @@ import { runCommand, } from "./drizzle-kit-install"; import { refuseSymlink, updateEnvFile } from "./env-file"; -import { classifyBranch, deployRefusal, takesWrites } from "./guard"; +import { + classifyBranch, + deployRefusal, + takesWrites, + writeRefusal, +} from "./guard"; import { handOffAppSchema } from "./hand-off"; import { type HistoryLock, withHistoryLock } from "./history-lock"; import { currentIdentity, type Identity } from "./identity"; @@ -205,6 +210,10 @@ function branchLimitError( return new Error(message, { cause }); } +/** What init says after refusing a branch, unless every branch is refused. */ +const INIT_SETS_UP = + "`appkit db init` only sets up your dev branch or this checkout's session branch"; + /** * Your dev branch or this checkout's session branch: found with one GET, or * forked from the project's default branch. The branch id is resolved and @@ -235,7 +244,7 @@ async function findOrCreateBranch( }); if (!takesWrites(verdict.kind)) { throw new Error( - `Refusing to set up ${resourceId(branch.name)}: ${verdict.reason}. \`appkit db init\` only sets up your dev branch or this checkout's session branch.`, + `Refusing to set up ${writeRefusal(`${resourceId(branch.name)}: ${verdict.reason}`, boundary, INIT_SETS_UP)}.`, ); } onStep(`Using ${resourceId(branch.name)}`); @@ -261,7 +270,7 @@ async function findOrCreateBranch( const refusal = deployRefusal(boundary, name); if (refusal) { throw new Error( - `Refusing to set up ${resourceId(name)}: ${refusal}. \`appkit db init\` only sets up your dev branch or this checkout's session branch.`, + `Refusing to set up ${writeRefusal(`${resourceId(name)}: ${refusal}`, boundary, INIT_SETS_UP)}.`, ); } const existing = await api.getBranch(name); diff --git a/packages/appkit/src/database/tooling/introspect/index.ts b/packages/appkit/src/database/tooling/introspect/index.ts index 91efadae6..66e83086f 100644 --- a/packages/appkit/src/database/tooling/introspect/index.ts +++ b/packages/appkit/src/database/tooling/introspect/index.ts @@ -189,8 +189,11 @@ export async function introspectHolding( if (!env) throw new Error(`Did not introspect: ${NO_ENV_BRANCH}.`); if (!takesWrites(env.kind)) { throw new IntrospectRefusedError( - `Refusing to introspect: ${notYourBranch(env)}. Introspect records the migration it writes as applied, so it runs only on branches \`appkit db init\` created for you. ` + - `Run \`appkit db init\`: it creates your dev branch from the project's default branch, and introspects it when there is no ${DEFAULT_SCHEMA_FILE}.`, + `Refusing to introspect: ${notYourBranch( + env, + "Introspect records the migration it writes as applied, so it runs only on branches `appkit db init` created for you. " + + `Run \`appkit db init\`: it creates your dev branch from the project's default branch, and introspects it when there is no ${DEFAULT_SCHEMA_FILE}`, + )}.`, ); } const branchId = resourceId(env.branch.name); diff --git a/packages/appkit/src/database/tooling/introspect/tests/introspect.test.ts b/packages/appkit/src/database/tooling/introspect/tests/introspect.test.ts index 9bfc2b4cb..7562f4a07 100644 --- a/packages/appkit/src/database/tooling/introspect/tests/introspect.test.ts +++ b/packages/appkit/src/database/tooling/introspect/tests/introspect.test.ts @@ -67,9 +67,11 @@ export const schema = defineSchema(({ table }) => { }); `; -const onDevBranch = { branch: { name: DEV }, kind: "dev" }; +const RESOLVED = { branches: new Set(), unresolved: [] }; +const onDevBranch = { branch: { name: DEV }, boundary: RESOLVED, kind: "dev" }; const onProduction = { branch: { name: PRODUCTION, status: { default: true } }, + boundary: RESOLVED, kind: "default", reason: "it is the project's default branch", }; @@ -284,6 +286,37 @@ describe("runIntrospect", () => { }, ); + test("while databricks.yml leaves a reference unresolved, the refusal names it instead of pointing to appkit db init", async () => { + resolveEnvBranch.mockResolvedValue({ + branch: { name: DEV }, + boundary: { + branches: new Set(), + unresolved: [ + { + target: "default", + at: "variables.postgres_branch", + written: "", + }, + ], + }, + kind: "deploy-target", + reason: + "databricks.yml may deploy to it through a reference this checkout's files do not resolve", + }); + const root = tempApp({ [SCHEMA]: previousSchema }); + const before = tree(root); + + const error = await refusal(root); + + expect(error).toBeInstanceOf(IntrospectRefusedError); + expect(error.message).toBe( + "Refusing to introspect: dev-alice-3f9a2c is not your dev or session branch (databricks.yml may deploy to it through a reference this checkout's files do not resolve). " + + "The files do not resolve variables.postgres_branch in target default (no value), so `appkit db migrate` applies migrations to no branch, and `init`, `introspect`, `clean` and `seed` refuse every branch. Give it a value in databricks.yml.", + ); + expect(withBranchPool).not.toHaveBeenCalled(); + expect(tree(root)).toEqual(before); + }); + test("with history: writes and records the next NNNN_introspect", async () => { const root = tempApp({ [SCHEMA]: declaredSchema }); await commitMigration(root); diff --git a/packages/appkit/src/database/tooling/migrate.ts b/packages/appkit/src/database/tooling/migrate.ts index 4ad773d87..80af77786 100644 --- a/packages/appkit/src/database/tooling/migrate.ts +++ b/packages/appkit/src/database/tooling/migrate.ts @@ -79,7 +79,10 @@ async function applyBlocker(ctx: DbContext): Promise { const env = await resolveEnvBranch(ctx); if (!env) return NO_ENV_BRANCH; if (takesWrites(env.kind)) return undefined; - return `${notYourBranch(env)}. Migrations are applied only to branches \`appkit db init\` created for you; the deployed app applies committed migrations to its own branch`; + return notYourBranch( + env, + "Migrations are applied only to branches `appkit db init` created for you; the deployed app applies committed migrations to its own branch", + ); } /** Apply pending migrations to the `.env` branch, or say why they were not. */ diff --git a/packages/appkit/src/database/tooling/tests/clean-seed.test.ts b/packages/appkit/src/database/tooling/tests/clean-seed.test.ts index a90605eee..71a260bae 100644 --- a/packages/appkit/src/database/tooling/tests/clean-seed.test.ts +++ b/packages/appkit/src/database/tooling/tests/clean-seed.test.ts @@ -202,6 +202,7 @@ describe("deleteSessionBranch", () => { test("refuses any other branch, and a .env without one", async () => { resolveEnvBranch.mockResolvedValueOnce({ branch: { name: "projects/p/branches/production" }, + boundary: { branches: new Set(), unresolved: [] }, kind: "default", reason: "it is the project's default branch", }); diff --git a/packages/appkit/src/database/tooling/tests/guard.test.ts b/packages/appkit/src/database/tooling/tests/guard.test.ts index 15280dc4c..56bf00551 100644 --- a/packages/appkit/src/database/tooling/tests/guard.test.ts +++ b/packages/appkit/src/database/tooling/tests/guard.test.ts @@ -2,9 +2,11 @@ import { afterEach, describe, expect, test } from "vitest"; import { classifyBranch, + everyBranchRefusal, resolveEnvBranch, takesWrites, writableEnvBranch, + writeRefusal, } from "../guard"; import { devBranchId, sessionBranchId } from "../naming"; import type { DeployBoundary } from "../targets"; @@ -98,9 +100,34 @@ describe("classifyBranch", () => { expect(verdict(branch, {}, "k2x9q0ab", unresolved)).toEqual({ kind: "deploy-target", reason: - 'databricks.yml may deploy to it: the files do not resolve resources.apps.app.resources[0].postgres.branch in target prod ("${var.postgres_branch}"); give it a value in databricks.yml', + "databricks.yml may deploy to it through a reference this checkout's files do not resolve", }); } + expect(everyBranchRefusal(unresolved)).toBe( + 'The files do not resolve resources.apps.app.resources[0].postgres.branch in target prod ("${var.postgres_branch}"), so `appkit db migrate` applies migrations to no branch, and `init`, `introspect`, `clean` and `seed` refuse every branch. Give it a value in databricks.yml', + ); + expect(everyBranchRefusal(NO_DEPLOYS)).toBeNull(); + }); + + test("a refusal gives its advice only while some branch can take writes", () => { + const unresolved: DeployBoundary = { + branches: new Set(), + unresolved: [ + { + target: "dev", + at: "variables.postgres_branch", + written: "", + file: ".databricks/bundle/dev/variable-overrides.json", + }, + ], + }; + expect(writeRefusal("x is refused", NO_DEPLOYS, "Run init")).toBe( + "x is refused. Run init", + ); + expect(writeRefusal("x is refused", NO_DEPLOYS)).toBe("x is refused"); + expect(writeRefusal("x is refused", unresolved, "Run init")).toBe( + "x is refused. The files do not resolve variables.postgres_branch in target dev with .databricks/bundle/dev/variable-overrides.json (no value), so `appkit db migrate` applies migrations to no branch, and `init`, `introspect`, `clean` and `seed` refuse every branch. Give it a value in databricks.yml or the variable-overrides.json named", + ); }); }); @@ -207,7 +234,7 @@ describe("resolveEnvBranch", () => { expect(env?.kind).toBe("deploy-target"); expect(env && takesWrites(env.kind)).toBe(false); - expect(env?.reason).toContain( + expect(env && everyBranchRefusal(env.boundary)).toContain( 'resources.apps.app.resources[0].postgres.branch in target staging ("${var.postgres_branch}")', ); }); @@ -285,4 +312,29 @@ describe("writableEnvBranch", () => { "Did not seed: no branch is configured in .env (PGHOST, PGDATABASE and LAKEBASE_ENDPOINT); run `appkit db init`.", ); }); + + test("while a reference is unresolved, the refusal names it and gives no init advice, on the default branch too", async () => { + const lakebase = new FakeLakebase(); + lakebase.addBranch(`${P}/branches/production`, { default: true }); + const root = tempApp({ + "databricks.yml": `${databricksYml(`${P}/branches/production`, `${P}/branches/production/databases/db`)} staging: + variables: + postgres_branch: \${var.staging_branch} + postgres_database: ${P}/branches/staging/databases/db +`, + }); + + const refusal = writableEnvBranch( + fakeContext({ + root, + env: envFor(`${P}/branches/production`), + api: lakebase.api(), + }), + "seed", + ); + + await expect(refusal).rejects.toThrow( + `Refusing to seed: production is not your dev or session branch (it is the project's default branch). The files do not resolve resources.apps.app.resources[0].postgres.branch in target staging ("\${var.postgres_branch}"), variables.postgres_branch in target staging ("\${var.staging_branch}"), so \`appkit db migrate\` applies migrations to no branch, and \`init\`, \`introspect\`, \`clean\` and \`seed\` refuse every branch. Give them values in databricks.yml.`, + ); + }); }); diff --git a/packages/appkit/src/database/tooling/tests/init.test.ts b/packages/appkit/src/database/tooling/tests/init.test.ts index 83dc5fae7..7e0cbc4ff 100644 --- a/packages/appkit/src/database/tooling/tests/init.test.ts +++ b/packages/appkit/src/database/tooling/tests/init.test.ts @@ -584,9 +584,11 @@ describe("runInit — branches it never sets up", () => { ); const before = fs.readFileSync(path.join(ctx.root, ".env"), "utf-8"); - await expect(init(ctx)).rejects.toThrow( - `Refusing to set up ${devBranchId(USER)}: databricks.yml may deploy to it: the files do not resolve resources.apps.app.resources[0].postgres.branch in target default`, + const error: Error = await init(ctx).catch((caught) => caught); + expect(error.message).toContain( + `Refusing to set up ${devBranchId(USER)}: databricks.yml may deploy to it through a reference this checkout's files do not resolve. The files do not resolve resources.apps.app.resources[0].postgres.branch in target default`, ); + expect(error.message).not.toContain("only sets up"); expect(lakebase.calls).toEqual([]); expect(fs.readFileSync(path.join(ctx.root, ".env"), "utf-8")).toBe(before); }); diff --git a/packages/appkit/src/database/tooling/tests/migrate.test.ts b/packages/appkit/src/database/tooling/tests/migrate.test.ts index a99bc3025..1ffe5c2cb 100644 --- a/packages/appkit/src/database/tooling/tests/migrate.test.ts +++ b/packages/appkit/src/database/tooling/tests/migrate.test.ts @@ -167,6 +167,7 @@ describe("runMigrate", () => { const ctx = app(`notes: table("notes", { id: id() })`); resolveEnvBranch.mockResolvedValue({ branch: { name: "projects/p/branches/production" }, + boundary: { branches: new Set(), unresolved: [] }, kind: "default", reason: "it is the project's default branch", }); From bcce3aabac9026258cb34cfc7c5534232fffdc72 Mon Sep 17 00:00:00 2001 From: ditadi Date: Fri, 30 Oct 2026 18:25:36 +0000 Subject: [PATCH 8/8] fix(appkit): fail verify --deployed closed on a missing snapshot A journal 404 counted as "no migrations" even when the snapshot itself was gone, so a checkout without migrations passed on no evidence. A 404 now counts only after get-status shows the snapshot exists; otherwise the result is unavailable. Messages that credit databricks.yml with a branch or database now name variable-overrides.json when the value came from it, the no-commit advice no longer sends you to a directory verify cannot run from, and --help says verify fails on deployed migrations the checkout does not commit and reads the overrides file. Signed-off-by: ditadi --- .../src/database/tooling/tests/verify.test.ts | 84 +++++++++++++++++-- .../appkit/src/database/tooling/verify.ts | 66 ++++++++++++--- packages/shared/src/cli/commands/db/verify.ts | 9 +- 3 files changed, 139 insertions(+), 20 deletions(-) diff --git a/packages/appkit/src/database/tooling/tests/verify.test.ts b/packages/appkit/src/database/tooling/tests/verify.test.ts index 85a2aee09..f79a12b9a 100644 --- a/packages/appkit/src/database/tooling/tests/verify.test.ts +++ b/packages/appkit/src/database/tooling/tests/verify.test.ts @@ -374,6 +374,8 @@ function deployedApp( deployedJournal?: Migration[] | string; /** The deployed SQL by tag, else the committed SQL; "unreadable" or "missing" as for the journal. */ deployedSql?: Record; + /** The deployed snapshot folder's status: "unreadable" or "missing" as for the journal; it exists otherwise. */ + snapshot?: "unreadable" | "missing"; authenticatedHost?: string; } = {}, ) { @@ -435,6 +437,21 @@ function deployedApp( content: Buffer.from(content).toString("base64"), }); } + if (method === "GET" && apiPath === "/api/2.0/workspace/get-status") { + const file = String(query?.path); + requests.push(`workspace.get-status ${file}`); + if (options.snapshot === "unreadable") { + throw Object.assign(new Error("Permission denied"), { + statusCode: 403, + }); + } + if (options.snapshot === "missing" || file !== SNAPSHOT) { + throw Object.assign(new Error("Path does not exist."), { + statusCode: 404, + }); + } + return Response.json({ object_type: "DIRECTORY", path: file }); + } throw new Error( `--deployed only reads the app and its journal, not ${method} ${apiPath}`, ); @@ -658,11 +675,11 @@ describe("runVerify --deployed", () => { }); test.each([ - ["has no journal", "missing"], - ["has a journal without entries", []], + ["has no journal", "missing", [`workspace.get-status ${SNAPSHOT}`]], + ["has a journal without entries", [], []], ] as const)( "without committed migrations, a snapshot that %s passes, and schema.ts is not needed", - async (_label, deployedJournal) => { + async (_label, deployedJournal, statusRequests) => { const { ctx, requests } = deployedApp({ deployedJournal: deployedJournal as Migration[] | string, }); @@ -675,11 +692,38 @@ describe("runVerify --deployed", () => { expect(requests).toEqual([ "apps.get my-app", `workspace.export ${JOURNAL}`, + ...statusRequests, ]); }, ); - test("without committed migrations, a snapshot with migrations fails, naming them: verify ran outside the app's root, or the checkout is behind", async () => { + test.each([ + [ + "does not exist", + "missing", + `the source snapshot ${SNAPSHOT} does not exist`, + ], + [ + "has a status verify cannot read", + "unreadable", + `reading the status of ${SNAPSHOT} failed: Permission denied`, + ], + ] as const)( + "without committed migrations, a journal 404 in a snapshot that %s is unavailable, not a pass", + async (_label, snapshot, reason) => { + const { ctx } = deployedApp({ deployedJournal: "missing", snapshot }); + + expect(await runVerify(ctx, DEPLOYED)).toMatchObject({ + migrations: { evidence: "unavailable", reason }, + problems: [ + `verify cannot tell whether app my-app's active deployment has the committed migrations: ${reason}`, + ], + ok: false, + }); + }, + ); + + test("without committed migrations, a snapshot with migrations fails, naming them: the checkout is behind", async () => { const { ctx } = deployedApp({ deployedJournal: [{ tag: "0000_migration", when: 1000 }], }); @@ -688,10 +732,10 @@ describe("runVerify --deployed", () => { migrations: { evidence: "unavailable", reason: - "the deployment has 0000_migration, and this checkout commits no migration: run verify from the app's root, or pull them", + "the deployment has 0000_migration, and this checkout commits none in config/database/migrations next to databricks.yml: pull it, then run verify again", }, problems: [ - "verify cannot tell whether app my-app's active deployment has the committed migrations: the deployment has 0000_migration, and this checkout commits no migration: run verify from the app's root, or pull them", + "verify cannot tell whether app my-app's active deployment has the committed migrations: the deployment has 0000_migration, and this checkout commits none in config/database/migrations next to databricks.yml: pull it, then run verify again", ], ok: false, }); @@ -834,6 +878,34 @@ describe("runVerify --deployed", () => { }); }); + test("a branch variable-overrides.json sets is credited to that file when it is missing or the app is bound elsewhere", async () => { + const staging = "projects/p/branches/staging"; + const { ctx, lakebase } = deployedApp(); + const overrides = path.join( + ctx.root, + ".databricks/bundle/default/variable-overrides.json", + ); + fs.mkdirSync(path.dirname(overrides), { recursive: true }); + fs.writeFileSync(overrides, JSON.stringify({ postgres_branch: staging })); + const source = + "databricks.yml with .databricks/bundle/default/variable-overrides.json"; + + expect(await runVerify(ctx, DEPLOYED)).toMatchObject({ + branch: staging, + problems: [`${source} deploys to ${staging}, which does not exist`], + ok: false, + }); + + lakebase.addBranch(staging); + expect(await runVerify(ctx, DEPLOYED)).toMatchObject({ + branch: staging, + problems: expect.arrayContaining([ + `app my-app uses branch ${PRODUCTION}, not ${staging} from ${source}: \`databricks apps deploy\` applies ${source}`, + ]), + ok: false, + }); + }); + test("a host written without a scheme or with a trailing slash matches", async () => { const { ctx } = deployedApp({ yml: ymlWithHost("acme.cloud.databricks.com/"), diff --git a/packages/appkit/src/database/tooling/verify.ts b/packages/appkit/src/database/tooling/verify.ts index 1221ad020..4fb817927 100644 --- a/packages/appkit/src/database/tooling/verify.ts +++ b/packages/appkit/src/database/tooling/verify.ts @@ -222,6 +222,29 @@ async function exportFile( return Buffer.from(content, "base64"); } +/** Whether a workspace path exists. Throws with the reason when its status cannot be read. */ +async function workspacePathExists( + ctx: DbContext, + file: string, +): Promise { + try { + await ctx.workspace().request({ + method: "GET", + path: "/api/2.0/workspace/get-status", + query: { path: file }, + }); + return true; + } catch (error) { + if ((error as { statusCode?: unknown } | null)?.statusCode === 404) { + return false; + } + const message = error instanceof Error ? error.message : String(error); + throw new Error(`reading the status of ${file} failed: ${message}`, { + cause: error, + }); + } +} + /** A committed migration's journal entry and the bytes of its SQL file. */ interface CommittedMigration { readonly entry: JournalEntry; @@ -244,9 +267,10 @@ function readCommitted(folder: string): CommittedMigration[] { * The committed migrations against the snapshot of the source the active * deployment runs: each needs the same journal entry (idx, when, * breakpoints) and byte-identical SQL there, and the snapshot may have no - * migration the checkout does not commit. A snapshot without a journal has no - * migration. Only that snapshot is evidence. Anything verify cannot read in it - * makes the result unavailable, never a match. + * migration the checkout does not commit. A snapshot that exists without a + * journal has no migration. Only that snapshot is evidence. Anything verify + * cannot read in it, or a snapshot that does not exist, makes the result + * unavailable, never a match. */ async function readDeployedMigrations( ctx: DbContext, @@ -276,7 +300,11 @@ async function readDeployedMigrations( const journalPath = deployedPath(journalFile(folder)); const journal = await exportFile(ctx, journalPath); let deployed: JournalEntry[] = []; - // A snapshot without a journal has no migration. + // A snapshot without a journal has no migration, but a 404 proves that + // only for a snapshot that exists. + if (!journal && !(await workspacePathExists(ctx, snapshot))) { + throw new Error(`the source snapshot ${snapshot} does not exist`); + } if (journal) { let json: unknown; try { @@ -297,9 +325,10 @@ async function readDeployedMigrations( ); if (committed.length === 0) { if (deployed.length === 0) return null; + const them = unknown.length === 1 ? "it" : "them"; return { evidence: "unavailable", - reason: `the deployment has ${unknown.join(", ")}, and this checkout commits no migration: run verify from the app's root, or pull them`, + reason: `the deployment has ${unknown.join(", ")}, and this checkout commits none in config/database/migrations next to databricks.yml: pull ${them}, then run verify again`, }; } const compared = await Promise.all( @@ -374,13 +403,19 @@ function migrationsProblems( return problems; } -/** The app's postgres resource against databricks.yml: same branch and database, and the permission to create its schema. */ +/** + * The app's postgres resource against databricks.yml: same branch and + * database, and the permission to create its schema. `source` is where the + * branch and database come from: `file`, with variable-overrides.json when + * the checkout has one. + */ function bindingProblems( app: DeployedApp, name: string, branch: string, database: string | null, file: string, + source: string, ): string[] { const bound = (app.resources ?? []).flatMap((resource) => resource.postgres ? [resource.postgres] : [], @@ -389,14 +424,14 @@ function bindingProblems( if (!binding) { return [ bound.length > 0 - ? `app ${name} uses branch ${bound.map((postgres) => postgres.branch).join(", ")}, not ${branch} from ${file}: ${DEPLOY} applies ${file}` + ? `app ${name} uses branch ${bound.map((postgres) => postgres.branch).join(", ")}, not ${branch} from ${source}: ${DEPLOY} applies ${source}` : `app ${name} has no postgres resource: ${DEPLOY} adds the one ${file} declares`, ]; } const problems: string[] = []; if (database && binding.database !== database) { problems.push( - `app ${name} uses database ${binding.database ?? "(none)"}, not ${database} from ${file}: ${DEPLOY} applies ${file}`, + `app ${name} uses database ${binding.database ?? "(none)"}, not ${database} from ${source}: ${DEPLOY} applies ${source}`, ); } if (binding.permission !== POSTGRES_PERMISSION) { @@ -525,6 +560,8 @@ async function verifyDeploy( // when it has the file, as the deploy guard reads them. const deploy = target.overridden ?? target; const withFile = target.overridden ? ` with ${target.overridden.file}` : ""; + // Where the branch and database come from. + const source = `${file}${withFile}`; const bound = deploy.apps.filter((app) => app.postgres.length > 0); if (bound.length > 1) { throw new Error( @@ -603,13 +640,13 @@ async function verifyDeploy( const branchId = resourceId(branch); if (!branchFound) { return stopped(name, [ - `${file} deploys to ${branch}, which does not exist`, + `${source} deploys to ${branch}, which does not exist`, ]); } const problems: string[] = []; if (database && !databaseFound) { problems.push( - `${file} binds database ${database}, which does not exist on ${branchId}`, + `${source} binds database ${database}, which does not exist on ${branchId}`, ); } if (!deployed) { @@ -619,7 +656,14 @@ async function verifyDeploy( return stopped(name, problems); } - const binding = bindingProblems(deployed, name, branch, database, file); + const binding = bindingProblems( + deployed, + name, + branch, + database, + file, + source, + ); problems.push(...binding, ...runningProblems(deployed, name)); const sp = deployed.service_principal_client_id; const [roles, migrations] = await Promise.all([ diff --git a/packages/shared/src/cli/commands/db/verify.ts b/packages/shared/src/cli/commands/db/verify.ts index 944a093cd..1d9f0ab90 100644 --- a/packages/shared/src/cli/commands/db/verify.ts +++ b/packages/shared/src/cli/commands/db/verify.ts @@ -50,9 +50,12 @@ branch's database. It checks that the target's app exists, that its postgres resource names the branch and database in databricks.yml with CAN_CONNECT_AND_CREATE, that its service principal has a role on the branch, that the app is running, and that the source its active deployment runs has -every committed migration, with the same journal entry and SQL. When verify -cannot read that source, it fails. It refuses when the target's -workspace.host is not the workspace this shell is authenticated against. +every committed migration, with the same journal entry and SQL, and no +migration this checkout does not commit. When the checkout has +.databricks/bundle//variable-overrides.json, the branch and database +come from it, as a deploy from this checkout uses them. When verify cannot +read that source, it fails. It refuses when the target's workspace.host is +not the workspace this shell is authenticated against. Exits 1 on any finding. Nothing is written or created.