diff --git a/packages/shared/src/cli/commands/plugin/add/add.test.ts b/packages/shared/src/cli/commands/plugin/add/add.test.ts new file mode 100644 index 000000000..59d27286f --- /dev/null +++ b/packages/shared/src/cli/commands/plugin/add/add.test.ts @@ -0,0 +1,91 @@ +import { expect, test } from "vitest"; + +import { pluginConfig, toSource } from "./add"; +import type { ManifestPlugin, ManifestResource } from "./plan"; + +const index: ManifestResource = { + type: "vector_search_index", + alias: "Index", + resourceKey: "vector-search-index", + scope: "vector-search", +}; +const plugin = (name: string): ManifestPlugin => ({ + name, + package: "@databricks/appkit", + resources: { required: [index], optional: [] }, +}); +const obo = { "vector_search_index:vector-search-index": "obo" } as const; + +test("aiSearch sets columns and OBO auth on the default index", () => { + const config = pluginConfig(plugin("aiSearch"), [index], obo, ["id", "t"]); + expect(toSource(config)).toBe( + `{ indexes: { default: { columns: ["id", "t"], auth: "on-behalf-of-user" } } }`, + ); + expect(pluginConfig(plugin("aiSearch"), [index], {}, ["id"])).toEqual({ + indexes: { default: { columns: ["id"] } }, + }); +}); + +test("files OBO sets the plugin-level auth", () => { + expect(pluginConfig(plugin("files"), [index], obo)).toEqual({ + auth: "on-behalf-of-user", + }); +}); + +test("no config for sp or plugins without an auth option", () => { + expect(pluginConfig(plugin("files"), [index], {})).toBeUndefined(); + expect(pluginConfig(plugin("analytics"), [index], obo)).toBeUndefined(); +}); + +test("toSource escapes strings and quotes non-identifier keys", () => { + expect(toSource({ "a-b": ['x"); evil()'] })).toBe( + `{ "a-b": ["x\\"); evil()"] }`, + ); +}); + +test("agents OBO sets plugin auth; resources that can't be OBO don't block it", () => { + const experiment: ManifestResource = { + type: "experiment", + alias: "Experiment", + resourceKey: "agents-mlflow-experiment", + }; + const endpoint: ManifestResource = { + type: "serving_endpoint", + alias: "LLM", + resourceKey: "agents-serving-endpoint", + scope: "model-serving", + }; + expect( + pluginConfig(plugin("agents"), [endpoint, experiment], { + "serving_endpoint:agents-serving-endpoint": "obo", + }), + ).toEqual({ + auth: "on-behalf-of-user", + }); +}); + +test("agents auth follows the model endpoint, not the skills volume", () => { + const endpoint: ManifestResource = { + type: "serving_endpoint", + alias: "LLM", + resourceKey: "agents-serving-endpoint", + scope: "model-serving", + }; + const skills: ManifestResource = { + type: "volume", + alias: "Skills", + resourceKey: "agents-skills", + scope: "files", + }; + const modelObo = { + "serving_endpoint:agents-serving-endpoint": "obo", + } as const; + expect(pluginConfig(plugin("agents"), [endpoint, skills], modelObo)).toEqual({ + auth: "on-behalf-of-user", + }); + expect( + pluginConfig(plugin("agents"), [endpoint, skills], { + "volume:agents-skills": "obo", + }), + ).toBeUndefined(); +}); diff --git a/packages/shared/src/cli/commands/plugin/add/add.ts b/packages/shared/src/cli/commands/plugin/add/add.ts new file mode 100644 index 000000000..a7b44b302 --- /dev/null +++ b/packages/shared/src/cli/commands/plugin/add/add.ts @@ -0,0 +1,529 @@ +import fs from "node:fs"; +import path from "node:path"; +import process from "node:process"; + +import { Command } from "commander"; +import pc from "picocolors"; + +import { profileFromEnv } from "../../registry/add"; +import { + planHasContent, + reportConfigWrite, + validateBundle, + writeConfig, +} from "../../registry/config-writer"; +import { serializeEnvAppend } from "../../registry/env-reconcile"; +import { + AUTH_MODE_BOTH, + AUTH_MODE_FIELD, + AUTH_MODE_OBO, + AUTH_MODE_SP, + type AuthMode, + authKey, + buildAddPlan, + findMissingValues, + type ManifestPlugin, + type ManifestResource, + type MissingValue, + oboCapable, + parseSetAuthModes, + parseSetValues, + resolveAuthModes, + resourceConfigured, + selectResources, +} from "./plan"; +import { discoverIndexColumns, resolveDerivedValues } from "./resolve"; + +const MANIFEST_FILE = "appkit.plugins.json"; +const SERVER_SUBDIRS = ["server", "api", "backend"]; + +interface AddOptions { + set: string[]; + authMode?: string; + cwd?: string; + yes?: boolean; + register?: boolean; + profile?: string; +} + +function loadPlugin(cwd: string, name: string): ManifestPlugin { + const file = path.join(cwd, MANIFEST_FILE); + if (!fs.existsSync(file)) { + throw new Error( + `${MANIFEST_FILE} not found in ${cwd}; run \`appkit plugin sync --write\` first`, + ); + } + const manifest = JSON.parse(fs.readFileSync(file, "utf-8")) as { + plugins?: Record; + }; + const plugins = manifest.plugins ?? {}; + const plugin = plugins[name]; + if (!plugin) { + throw new Error( + `unknown plugin "${name}"; available: ${Object.keys(plugins).sort().join(", ")}`, + ); + } + return { + ...plugin, + name, + resources: { + required: plugin.resources?.required ?? [], + optional: plugin.resources?.optional ?? [], + }, + }; +} + +function authModeLabel(mode: AuthMode): string { + if (mode === AUTH_MODE_OBO) return "on behalf of user"; + if (mode === AUTH_MODE_BOTH) return "service principal and on behalf of user"; + return "service principal"; +} + +/** Bails out of the command on Ctrl+C in a prompt. */ +async function answered(value: T | symbol): Promise { + const { cancel, isCancel } = await import("@clack/prompts"); + if (isCancel(value)) { + cancel("Cancelled."); + process.exit(1); + } + return value as T; +} + +/** Asks which optional resources to configure (they're only added with values). */ +async function promptOptional( + plugin: ManifestPlugin, + values: Record, +): Promise { + const { confirm } = await import("@clack/prompts"); + const picked: ManifestResource[] = []; + for (const r of plugin.resources.optional) { + if (resourceConfigured(r, values)) continue; + const yes = await answered( + await confirm({ + message: `Configure ${r.alias}? ${pc.dim(`${r.description ?? ""} (optional)`)}`, + initialValue: false, + }), + ); + if (yes) picked.push(r); + } + return picked; +} + +/** + * As `apps init`: one question for the app, and for "Mixed" one per + * OBO-capable resource. Resources without --set authMode only. + */ +async function promptAuthModes( + resources: ManifestResource[], +): Promise> { + const capable = resources.filter(oboCapable); + if (capable.length === 0) return {}; + const { log, select } = await import("@clack/prompts"); + const appMode = await answered( + await select({ + message: "How should the app access resources?", + options: [ + { value: AUTH_MODE_SP, label: "Service principal" }, + { value: AUTH_MODE_OBO, label: "On behalf of user" }, + { value: "mixed", label: "Mixed (choose per resource)" }, + ], + }), + ); + const modes: Record = {}; + for (const r of capable) { + modes[authKey(r)] = + appMode === "mixed" + ? await answered( + await select({ + message: `How should the app access ${r.alias}?`, + options: [ + { value: AUTH_MODE_OBO, label: "On behalf of user" }, + { value: AUTH_MODE_SP, label: "Service principal" }, + { value: AUTH_MODE_BOTH, label: "Both" }, + ], + }), + ) + : (appMode as AuthMode); + log.info(`${r.alias}: ${authModeLabel(modes[authKey(r)])}`); + } + for (const r of resources) { + if (!oboCapable(r)) { + log.info(`${r.alias}: ${authModeLabel(AUTH_MODE_SP)} (only option)`); + } + } + return modes; +} + +/** Prompts for each missing value, or fails on the first one when not interactive. */ +async function fillMissing( + missing: MissingValue[], + values: Record, + interactive: boolean, +): Promise { + if (missing.length > 0 && !interactive) { + const m = missing[0]; + throw new Error( + `missing value for ${m.setKey}; use --set ${m.setKey}=value`, + ); + } + if (missing.length === 0) return; + const { text } = await import("@clack/prompts"); + for (const m of missing) { + const field = m.resource.fields?.[m.field]; + const value = await answered( + await text({ + message: `${m.resource.alias}: ${m.field}`, + placeholder: field?.description, + validate: (v) => + !v?.trim() || /[\r\n]/.test(v) ? "Enter a value" : undefined, + }), + ); + values[`${m.resource.resourceKey}.${m.field}`] = value.trim(); + } +} + +/** + * Columns for the aiSearch default index: discovered from the workspace, else + * prompted. Without a terminal it fails before anything is written, since the + * app would not start in production without them. + */ +async function aiSearchColumns( + values: Record, + profile: string | undefined, + interactive: boolean, +): Promise { + const indexName = values["vector-search-index.id"]; + try { + const columns = await discoverIndexColumns(indexName, profile); + if (columns.length > 0) { + console.log(pc.dim(`Index columns: ${columns.join(", ")}`)); + return columns; + } + } catch (err) { + console.warn( + pc.yellow( + `Could not discover the columns of ${indexName}: ${(err as Error).message}`, + ), + ); + } + if (!interactive) { + throw new Error( + `aiSearch needs the columns of ${indexName} to start in production; run with workspace access (--profile) or interactively to enter them`, + ); + } + const { text } = await import("@clack/prompts"); + const answer = await answered( + await text({ + message: `Columns to return from ${indexName} (comma-separated)`, + validate: (v) => + v?.split(",").some((c) => c.trim()) + ? undefined + : "Enter at least one column", + }), + ); + return answer + .split(",") + .map((c) => c.trim()) + .filter(Boolean); +} + +async function runPluginAdd(name: string, opts: AddOptions): Promise { + const cwd = opts.cwd ? path.resolve(opts.cwd) : process.cwd(); + if ( + opts.authMode !== undefined && + opts.authMode !== AUTH_MODE_SP && + opts.authMode !== AUTH_MODE_OBO + ) { + throw new Error( + `invalid --auth-mode value: "${opts.authMode}" (must be obo or sp; use --set ..${AUTH_MODE_FIELD}=both for a single resource)`, + ); + } + const plugin = loadPlugin(cwd, name); + const values = parseSetValues(opts.set, plugin); + const setModes = parseSetAuthModes(opts.set, plugin); + const interactive = !opts.yes && Boolean(process.stdin.isTTY); + + let resources = selectResources(plugin, values); + if (interactive) resources.push(...(await promptOptional(plugin, values))); + + const promptedModes = + interactive && opts.authMode === undefined + ? await promptAuthModes( + resources.filter((r) => !(authKey(r) in setModes)), + ) + : {}; + const { modes, keptSP } = resolveAuthModes( + resources, + setModes, + promptedModes, + opts.authMode, + ); + if (keptSP.length > 0) { + console.log( + `Note: these resources cannot be accessed on behalf of the user and use the service principal: ${keptSP.join(", ")}`, + ); + } + + const missingNow = () => + findMissingValues(plugin.name, resources, values, modes); + // Values the user supplies come first: the derived ones need them. + await fillMissing( + missingNow().filter((m) => !m.resource.fields?.[m.field]?.resolve), + values, + interactive, + ); + const profile = opts.profile ?? profileFromEnv(cwd); + for (const r of resources) { + try { + for (const [k, v] of Object.entries( + await resolveDerivedValues(r, values, profile), + )) { + values[k] ||= v; + } + } catch (err) { + console.warn( + pc.yellow( + `Could not resolve derived values for ${r.alias}: ${(err as Error).message}`, + ), + ); + } + } + await fillMissing(missingNow(), values, interactive); + resources = selectResources(plugin, values); + + const config = pluginConfig( + plugin, + resources, + modes, + plugin.name === "aiSearch" && opts.register !== false + ? await aiSearchColumns(values, profile, interactive) + : undefined, + ); + + const plan = buildAddPlan(plugin, resources, values, modes); + + if (opts.register !== false) { + await registerInServer(cwd, plugin, config ? toSource(config) : ""); + } + + const { appendToFile, readEnvFile } = + await import("../../registry/env-writer.js"); + const envPath = path.join(cwd, ".env"); + const existing = readEnvFile(envPath); + const dotEnv = plan.dotEnv.filter( + (e, i) => plan.dotEnv.findIndex((x) => x.env === e.env) === i, + ); + // Empty placeholders (`KEY=`) are filled in place; set values are kept. + const filled = dotEnv.filter((e) => existing[e.env] === "" && e.value); + if (filled.length > 0) { + let content = fs.readFileSync(envPath, "utf-8"); + for (const e of filled) { + // e.env is a validated identifier, so it is safe in the pattern. + content = content.replace( + new RegExp(`^(\\s*(?:export\\s+)?${e.env}\\s*=).*$`, "m"), + (_, key: string) => key + e.value, + ); + } + fs.writeFileSync(envPath, content); + } + const newEnv = dotEnv.filter((e) => !(e.env in existing)); + appendToFile(envPath, serializeEnvAppend(newEnv)); + const updated = [...filled, ...newEnv].map((e) => e.env); + if (updated.length > 0) { + console.log(`${pc.green("Updated .env:")} ${updated.join(", ")}`); + } + for (const e of dotEnv) { + if (existing[e.env] && e.value && existing[e.env] !== e.value) { + console.warn( + pc.yellow( + `.env already sets ${e.env}; kept it (the app.yaml and databricks.yml entries use the new value).`, + ), + ); + } + } + + if (planHasContent(plan)) { + const result = writeConfig(cwd, plan); + reportConfigWrite(result); + if (result.databricksYmlChanged) { + validateBundle(cwd, profile); + } + } +} + +/** Plugins that default to the service principal unless told otherwise. */ +const AUTH_OPTION_PLUGINS = new Set(["files", "aiSearch", "agents"]); +const OBO = "on-behalf-of-user"; + +/** + * The `createApp` config for the plugin, or undefined for none. An OBO plan + * leaves the service principal unbound, so plugins that default to it must be + * told: files and agents read a plugin-level `auth`, aiSearch reads it per index. The + * aiSearch `default` index takes DATABRICKS_VS_INDEX_NAME, and production + * refuses to start without its `columns`. + */ +export function pluginConfig( + plugin: ManifestPlugin, + resources: ManifestResource[], + modes: Record, + columns?: string[], +): Record | undefined { + let obo = false; + if (AUTH_OPTION_PLUGINS.has(plugin.name)) { + // The resources whose access the option governs. For agents that is the + // model endpoint: skills load from their volume as the service principal + // at boot whatever `auth` is, and the tracing experiment can't be OBO. + // Elsewhere, every resource that can be accessed on behalf of the user. + const governed = resources.filter( + (r) => + oboCapable(r) && + (plugin.name !== "agents" || r.type === "serving_endpoint"), + ); + const oboRes = governed.filter((r) => modes[authKey(r)] === AUTH_MODE_OBO); + obo = oboRes.length > 0 && oboRes.length === governed.length; + if (plugin.name === "agents") { + for (const r of resources) { + if (r.type === "volume" && modes[authKey(r)] === AUTH_MODE_OBO) { + console.warn( + pc.yellow( + `agents loads skills from ${r.resourceKey} as the service principal at boot, which gets no access in obo mode; use --set agents.${r.resourceKey}.authMode=sp (or both).`, + ), + ); + } + } + } + if (oboRes.length > 0 && !obo) { + console.warn( + pc.yellow( + `${plugin.name} is registered with one auth setting for all its resources; set auth per volume or index in its config to mix on-behalf-of-user (${oboRes.map((r) => r.resourceKey).join(", ")}) with service principal access.`, + ), + ); + } + } + if (plugin.name === "aiSearch") { + const index = { ...(columns && { columns }), ...(obo && { auth: OBO }) }; + return Object.keys(index).length > 0 + ? { indexes: { default: index } } + : undefined; + } + return obo ? { auth: OBO } : undefined; +} + +/** Source text for a config literal: `{ auth: "x", columns: ["a"] }`. */ +export function toSource(v: unknown): string { + if (Array.isArray(v)) return `[${v.map(toSource).join(", ")}]`; + if (v && typeof v === "object") { + const props = Object.entries(v).map( + ([k, x]) => + `${/^[A-Za-z_$][\w$]*$/.test(k) ? k : JSON.stringify(k)}: ${toSource(x)}`, + ); + return `{ ${props.join(", ")} }`; + } + return JSON.stringify(v); +} + +async function registerInServer( + cwd: string, + plugin: ManifestPlugin, + callArgs: string, +): Promise { + const serverRoot = + SERVER_SUBDIRS.map((s) => path.join(cwd, s)).find((p) => + fs.existsSync(p), + ) ?? cwd; + // Lazy: server-register pulls in the @ast-grep/napi native addon. + const { findServerFile, registerPluginInServer } = + await import("../../registry/server-register.js"); + // The /beta subpath is AppKit's; other packages export their plugins as is. + let importPath = + plugin.stability === "beta" && plugin.package === "@databricks/appkit" + ? `${plugin.package}/beta` + : plugin.package; + // Local plugins are recorded relative to the app root; imports resolve + // relative to the server file. + if (plugin.package.startsWith(".")) { + const from = path.dirname(findServerFile(serverRoot) ?? serverRoot); + const rel = path + .relative(from, path.resolve(cwd, plugin.package)) + .split(path.sep) + .join("/"); + importPath = rel.startsWith(".") ? rel : `./${rel}`; + } + const call = `${plugin.name}(${callArgs})`; + const result = registerPluginInServer( + serverRoot, + importPath, + plugin.name, + callArgs, + ); + const shown = + result.file && path.relative(cwd, path.join(serverRoot, result.file)); + if (result.status === "wired") { + console.log(`${pc.green("Registered")} ${call} in ${shown}`); + } else if (result.status === "already") { + console.log(pc.dim(`${plugin.name} is already registered in ${shown}`)); + if (callArgs) { + console.warn( + pc.yellow( + ` Make sure it is configured as ${call} for the chosen auth mode.`, + ), + ); + } + } else { + console.log( + pc.yellow(`Couldn't auto-register ${plugin.name}: ${result.reason}.`) + + "\nAdd it to your server's createApp call:\n" + + pc.dim( + ` import { ${plugin.name} } from "${importPath}";\n` + + ` createApp({ plugins: [${call}, /* ... */] });`, + ), + ); + } +} + +export const pluginAddCommand = new Command("add") + .description( + "Add a plugin from appkit.plugins.json to the app: server registration, .env, app.yaml and databricks.yml", + ) + .argument("", "Plugin name, as in appkit.plugins.json") + .option( + "--set ", + "Set resource values (format: plugin.resourceKey.field=value, can specify multiple)", + (v: string, acc: string[]) => [...acc, v], + [] as string[], + ) + .option( + "--auth-mode ", + "Default resource auth mode: obo (on behalf of the user) or sp (service principal). With obo, resources that cannot be accessed on behalf of the user use sp. Prompts if not provided in interactive mode, otherwise sp.", + ) + .option("-C, --cwd ", "Run as if started in ") + .option("-y, --yes", "Don't prompt; fail on missing required values") + .option("--no-register", "Don't edit the server entry to register the plugin") + .option( + "-p, --profile ", + "Databricks profile for bundle validate (defaults to the app's DATABRICKS_CONFIG_PROFILE)", + ) + .addHelpText( + "after", + ` +Resource configuration (--set), as in \`databricks apps init\`: + Set resource values using --set plugin.resourceKey.field=value + Keys are defined in the app's appkit.plugins.json manifest. + Multi-field resources (e.g., database, secret) require all fields to be set together. + Use --set plugin.resourceKey.authMode=obo|sp|both to choose how the app accesses a resource: + on behalf of the user (obo), as the app's service principal (sp), or both. + obo/both add the resource's user_api_scopes; sp/both bind it with its permission. + +Existing entries in .env, app.yaml and databricks.yml are never overwritten. + +Examples: + $ appkit plugin add analytics --set analytics.sql-warehouse.id=abc123 + $ appkit plugin add analytics --auth-mode obo --set analytics.sql-warehouse.id=abc123 + $ appkit plugin add jobs --auth-mode obo --set jobs.job.authMode=sp --set jobs.job.id=42`, + ) + .action((name: string, opts: AddOptions) => + runPluginAdd(name, opts).catch((err: Error) => { + console.error(pc.red(err.message)); + process.exit(1); + }), + ); diff --git a/packages/shared/src/cli/commands/plugin/add/plan.test.ts b/packages/shared/src/cli/commands/plugin/add/plan.test.ts new file mode 100644 index 000000000..e7be3da51 --- /dev/null +++ b/packages/shared/src/cli/commands/plugin/add/plan.test.ts @@ -0,0 +1,251 @@ +import { describe, expect, test } from "vitest"; + +import { + buildAddPlan, + findMissingValues, + type ManifestPlugin, + parseSetAuthModes, + parseSetValues, + resolveAuthModes, + selectResources, + validSetKeys, +} from "./plan"; + +const warehouse = { + type: "sql_warehouse", + alias: "SQL Warehouse", + resourceKey: "sql-warehouse", + description: "Warehouse", + permission: "CAN_USE", + fields: { id: { env: "DATABRICKS_WAREHOUSE_ID", description: "id" } }, + scope: "sql", + binding: { yamlKey: "sql_warehouse", varFields: [["id", "id"]] }, +} satisfies ManifestPlugin["resources"]["required"][number]; + +const secret = { + type: "secret", + alias: "Secret", + resourceKey: "secret", + permission: "READ", + fields: { key: { env: "SECRET_KEY" }, scope: { env: "SECRET_SCOPE" } }, + appOnly: true, + binding: { + yamlKey: "secret", + varFields: [ + ["scope", "scope"], + ["key", "key"], + ], + }, +} satisfies ManifestPlugin["resources"]["optional"][number]; + +const job = { + type: "job", + alias: "Job", + resourceKey: "job", + permission: "CAN_MANAGE_RUN", + fields: { id: { env: "DATABRICKS_JOB_ID" } }, + binding: { yamlKey: "job", varFields: [["id", "id"]] }, +} satisfies ManifestPlugin["resources"]["optional"][number]; + +const plugin: ManifestPlugin = { + name: "analytics", + package: "@databricks/appkit", + scopes: ["ai-gateway"], + resources: { required: [warehouse], optional: [secret, job] }, +}; + +describe("parseSetValues", () => { + test("keys values by resourceKey.field and skips authMode", () => { + expect( + parseSetValues( + [ + "analytics.sql-warehouse.id=wh", + "analytics.sql-warehouse.authMode=obo", + ], + plugin, + ), + ).toEqual({ "sql-warehouse.id": "wh" }); + }); + + test.each([ + ["other.sql-warehouse.id=x", "unknown plugin"], + ["analytics.nope.id=x", "does not match a resource declared"], + ["analytics.sql-warehouse=x", "invalid --set key"], + ["analytics.secret.scope=s", "incomplete resource"], + ])("%s → %s", (sv, err) => { + expect(() => parseSetValues([sv], plugin)).toThrow(err); + }); +}); + +test("validSetKeys derives keys and values from the manifest", () => { + const keys = validSetKeys(plugin); + expect([...keys.keys()]).toEqual([ + "analytics.sql-warehouse.id", + "analytics.sql-warehouse.authMode", + "analytics.secret.key", + "analytics.secret.scope", + "analytics.job.id", + "analytics.job.authMode", + ]); + expect(keys.get("analytics.job.authMode")).toEqual(["sp"]); + expect(() => + parseSetValues(["analytics.sql-warehouse.name=x"], plugin), + ).toThrow("valid keys: analytics.sql-warehouse.id"); + expect(() => + parseSetAuthModes(["analytics.nope.authMode=sp"], plugin), + ).toThrow("does not match a resource declared"); +}); + +describe("parseSetAuthModes", () => { + test.each([ + ["analytics.sql-warehouse.authMode=user", "invalid auth mode"], + ["analytics.secret.authMode=sp", "always accessed by the app's service"], + ["analytics.job.authMode=obo", "cannot be accessed on behalf of the user"], + ])("%s → %s", (sv, err) => { + expect(() => parseSetAuthModes([sv], plugin)).toThrow(err); + }); + + test("accepts both and sp without scope", () => { + expect( + parseSetAuthModes( + ["analytics.sql-warehouse.authMode=both", "analytics.job.authMode=sp"], + plugin, + ), + ).toEqual({ "sql_warehouse:sql-warehouse": "both", "job:job": "sp" }); + }); +}); + +test("resolveAuthModes: --set wins, obo default skips non-capable", () => { + const { modes, keptSP } = resolveAuthModes( + [warehouse, secret, job], + {}, + {}, + "obo", + ); + expect(modes).toEqual({ "sql_warehouse:sql-warehouse": "obo" }); + expect(keptSP).toEqual(["secret", "job"]); + expect( + resolveAuthModes( + [warehouse], + { "sql_warehouse:sql-warehouse": "sp" }, + {}, + "obo", + ).modes, + ).toEqual({}); +}); + +describe("buildAddPlan", () => { + const values = { "sql-warehouse.id": "wh", "job.id": "42" }; + const resources = selectResources(plugin, values); + + test("sp binds the resource with its permission; only plugin scopes", () => { + const plan = buildAddPlan(plugin, resources, values, {}); + expect(resources.map((r) => r.resourceKey)).toEqual([ + "sql-warehouse", + "job", + ]); + // Plugin scopes (genie-style always-OBO routes) apply in sp mode too. + expect(plan.userApiScopes).toEqual(["ai-gateway"]); + expect(plan.appYamlEnv).toEqual([ + { name: "DATABRICKS_WAREHOUSE_ID", valueFrom: "sql-warehouse" }, + { name: "DATABRICKS_JOB_ID", valueFrom: "job" }, + ]); + expect(plan.resourceBindings[0]).toEqual({ + name: "sql-warehouse", + type: "sql_warehouse", + permission: "CAN_USE", + fields: { id: "${var.sql_warehouse_id}" }, + }); + expect(plan.bundleVariables[0]).toMatchObject({ + name: "sql_warehouse_id", + value: "wh", + }); + expect(plan.dotEnv).toContainEqual({ + env: "DATABRICKS_WAREHOUSE_ID", + value: "wh", + }); + }); + + test("obo uses a literal value, no binding, and adds scopes", () => { + const modes = { "sql_warehouse:sql-warehouse": "obo" } as const; + const plan = buildAddPlan(plugin, resources, values, modes); + expect(plan.appYamlEnv[0]).toEqual({ + name: "DATABRICKS_WAREHOUSE_ID", + value: "wh", + }); + expect(plan.resourceBindings.map((b) => b.name)).toEqual(["job"]); + expect(plan.userApiScopes).toEqual(["sql", "ai-gateway"]); + }); + + test("both binds and adds the scope", () => { + const modes = { "sql_warehouse:sql-warehouse": "both" } as const; + const plan = buildAddPlan(plugin, resources, values, modes); + expect(plan.resourceBindings.map((b) => b.name)).toEqual([ + "sql-warehouse", + "job", + ]); + expect(plan.userApiScopes).toEqual(["sql", "ai-gateway"]); + }); +}); + +test("findMissingValues names the --set key", () => { + expect( + findMissingValues("analytics", [warehouse], {}, {}).map((m) => m.setKey), + ).toEqual(["analytics.sql-warehouse.id"]); +}); + +test("fields with a resolver are required even when localOnly", () => { + const pg = { + type: "postgres", + alias: "Postgres", + resourceKey: "postgres", + appOnly: true, + fields: { + branch: {}, + host: { env: "PGHOST", localOnly: true, resolve: "postgres:host" }, + port: { env: "PGPORT", localOnly: true, value: "5432" }, + }, + binding: { yamlKey: "postgres", varFields: [["branch", "branch"]] }, + } satisfies ManifestPlugin["resources"]["required"][number]; + expect( + findMissingValues("lakebase", [pg], { "postgres.branch": "b" }, {}).map( + (m) => m.setKey, + ), + ).toEqual(["lakebase.postgres.host"]); +}); + +test("a manifest default fills the bundle variable", () => { + const p: ManifestPlugin = { + ...plugin, + resources: { + required: [ + { + ...warehouse, + fields: { id: { env: "DATABRICKS_WAREHOUSE_ID", value: "dflt" } }, + }, + ], + optional: [], + }, + }; + const plan = buildAddPlan(p, p.resources.required, {}, {}); + expect(plan.bundleVariables[0]).toMatchObject({ + name: "sql_warehouse_id", + value: "dflt", + }); +}); + +test("a resource without binding metadata uses the built-in binding", () => { + const { binding: _, ...noBinding } = warehouse; + const plan = buildAddPlan( + plugin, + [noBinding], + { "sql-warehouse.id": "wh" }, + {}, + ); + expect(plan.unverifiedTypes).toEqual([]); + expect(plan.resourceBindings[0]).toMatchObject({ + name: "sql-warehouse", + type: "sql_warehouse", + fields: { id: "${var.sql_warehouse_id}" }, + }); +}); diff --git a/packages/shared/src/cli/commands/plugin/add/plan.ts b/packages/shared/src/cli/commands/plugin/add/plan.ts new file mode 100644 index 000000000..8a5a7bf19 --- /dev/null +++ b/packages/shared/src/cli/commands/plugin/add/plan.ts @@ -0,0 +1,440 @@ +import { DABS_BINDING_BY_TYPE } from "../../../../schemas/manifest"; +import type { AppYamlEnvEntry, ResourceBinding } from "../../../deploy-config"; +import type { BundleVariable, ConfigPlan } from "../../registry/config-plan"; +import { isValidEnvName } from "../../registry/requirements"; + +/** + * Pure planning for `appkit plugin add`: a port of the Databricks CLI's + * `apps init` resource handling (cmd/apps/init.go + libs/apps/generator), so a + * plugin added later gets the same .env, app.yaml and databricks.yml entries + * (and the same --set / --auth-mode semantics) it would have had at init. + */ + +export const AUTH_MODE_SP = "sp"; +export const AUTH_MODE_OBO = "obo"; +export const AUTH_MODE_BOTH = "both"; +export type AuthMode = + | typeof AUTH_MODE_SP + | typeof AUTH_MODE_OBO + | typeof AUTH_MODE_BOTH; + +/** Reserved --set field that selects a resource's auth mode. */ +export const AUTH_MODE_FIELD = "authMode"; + +export interface ManifestField { + env?: string; + description?: string; + value?: string; + localOnly?: boolean; + bundleIgnore?: boolean; + /** Derived-value resolver key, e.g. "postgres:host" (see resolve.ts). */ + resolve?: string; +} + +export interface ManifestResource { + type: string; + alias: string; + resourceKey: string; + description?: string; + permission?: string; + fields?: Record; + scope?: string; + appOnly?: boolean; + binding?: Binding; +} + +type FieldPair = readonly [string, string]; +interface Binding { + yamlKey: string; + varFields: readonly FieldPair[]; + staticFields?: readonly FieldPair[]; +} + +/** + * The resource's databricks.yml binding: the manifest's, else the built-in + * one for its type (`binding` is optional in the manifest schema; the + * Databricks CLI generator falls back the same way). + */ +function bindingOf(r: ManifestResource): Binding | undefined { + return ( + r.binding ?? + (DABS_BINDING_BY_TYPE as Record)[r.type] + ); +} + +/** A plugin entry of the app's appkit.plugins.json (the fields used here). */ +export interface ManifestPlugin { + name: string; + package: string; + stability?: string; + scopes?: string[]; + resources: { required: ManifestResource[]; optional: ManifestResource[] }; +} + +/** Same identity as resource de-duplication: type + key. */ +export function authKey(r: ManifestResource): string { + return `${r.type}:${r.resourceKey}`; +} + +/** Keys `resourceKey.field`, as the CLI's resourceValues map. */ +function valueKey(r: ManifestResource, field: string): string { + return `${r.resourceKey}.${field}`; +} + +function fieldNames(r: ManifestResource): string[] { + return Object.keys(r.fields ?? {}).sort(); +} + +function allResources(p: ManifestPlugin): ManifestResource[] { + return [...p.resources.required, ...p.resources.optional]; +} + +/** Manifest fields the resource's databricks.yml binding references. */ +function bindingVarFields(r: ManifestResource): string[] { + return (bindingOf(r)?.varFields ?? []).map(([f]) => f); +} + +/** + * The --set keys the plugin manifest declares, each with its valid values: + * `plugin.resourceKey.field` for every declared field and binding var field + * (any value), plus `plugin.resourceKey.authMode` for resources that are not + * app-only (obo/both only when the resource declares a user scope). + */ +export function validSetKeys( + plugin: ManifestPlugin, +): Map { + const keys = new Map(); + for (const r of allResources(plugin)) { + const prefix = `${plugin.name}.${r.resourceKey}.`; + for (const f of new Set([...fieldNames(r), ...bindingVarFields(r)])) { + keys.set(prefix + f, null); + } + if (!r.appOnly) { + keys.set( + prefix + AUTH_MODE_FIELD, + r.scope + ? [AUTH_MODE_OBO, AUTH_MODE_SP, AUTH_MODE_BOTH] + : [AUTH_MODE_SP], + ); + } + } + return keys; +} + +/** Throws unless the key is one the plugin manifest declares. */ +function assertDeclared(plugin: ManifestPlugin, sv: string, key: string): void { + const keys = validSetKeys(plugin); + if (keys.has(key)) return; + throw new Error( + `--set "${sv}" does not match a resource declared by plugin "${plugin.name}"; valid keys: ${[...keys.keys()].join(", ") || "(none, the plugin declares no resources)"}`, + ); +} + +function splitSetKey(sv: string): { + pluginName: string; + resourceKey: string; + field: string; + value: string; +} { + const eq = sv.indexOf("="); + const key = eq === -1 ? "" : sv.slice(0, eq); + if (!key) { + throw new Error( + `invalid --set format "${sv}", expected plugin.resourceKey.field=value`, + ); + } + const parts = key.split("."); + if (parts.length < 3) { + throw new Error( + `invalid --set key "${key}", expected plugin.resourceKey.field`, + ); + } + return { + pluginName: parts[0], + resourceKey: parts[1], + field: parts.slice(2).join("."), + value: sv.slice(eq + 1), + }; +} + +/** + * Parses `--set plugin.resourceKey.field=value` into `resourceKey.field` → value, + * validating plugin, resource and field against the manifest and that + * multi-field resources are set together. `authMode` pairs are skipped (see + * {@link parseSetAuthModes}). + */ +export function parseSetValues( + setValues: string[], + plugin: ManifestPlugin, +): Record { + const rv: Record = {}; + for (const sv of setValues) { + const { pluginName, resourceKey, field, value } = splitSetKey(sv); + if (field === AUTH_MODE_FIELD) continue; + if (pluginName !== plugin.name) { + throw new Error( + `unknown plugin "${pluginName}" in --set "${sv}"; this command adds "${plugin.name}"`, + ); + } + assertDeclared(plugin, sv, `${pluginName}.${resourceKey}.${field}`); + rv[`${resourceKey}.${field}`] = value; + } + + for (const r of allResources(plugin)) { + const checkable = fieldNames(r).filter( + (f) => !r.fields?.[f].bundleIgnore && !r.fields?.[f].localOnly, + ); + if (checkable.length <= 1) continue; + const missing = checkable.filter((f) => !rv[valueKey(r, f)]); + if (missing.length > 0 && missing.length < checkable.length) { + throw new Error( + `incomplete resource "${r.resourceKey}": missing fields [${missing.map((f) => valueKey(r, f)).join(" ")}] (all fields must be set together)`, + ); + } + } + return rv; +} + +/** Parses `--set plugin.resourceKey.authMode=obo|sp|both` into authKey → mode. */ +export function parseSetAuthModes( + setValues: string[], + plugin: ManifestPlugin, +): Record { + const modes: Record = {}; + for (const sv of setValues) { + const { pluginName, resourceKey, field, value } = splitSetKey(sv); + if (field !== AUTH_MODE_FIELD) continue; + if (pluginName !== plugin.name) { + throw new Error( + `unknown plugin "${pluginName}" in --set "${sv}"; this command adds "${plugin.name}"`, + ); + } + const r = allResources(plugin).find((x) => x.resourceKey === resourceKey); + if (!r) + throw assertDeclared(plugin, sv, `${pluginName}.${resourceKey}.${field}`); + if ( + value !== AUTH_MODE_SP && + value !== AUTH_MODE_OBO && + value !== AUTH_MODE_BOTH + ) { + throw new Error( + `invalid auth mode "${value}" in --set "${sv}" (must be obo, sp, or both)`, + ); + } + if (r.appOnly) { + throw new Error( + `resource "${resourceKey}" of plugin "${plugin.name}" is always accessed by the app's service principal; remove --set ${sv}`, + ); + } + if (value !== AUTH_MODE_SP && !r.scope) { + throw new Error( + `resource "${resourceKey}" of plugin "${plugin.name}" cannot be accessed on behalf of the user; use ${plugin.name}.${resourceKey}.${AUTH_MODE_FIELD}=sp`, + ); + } + modes[authKey(r)] = value; + } + return modes; +} + +/** True when the resource can be accessed on behalf of the user. */ +export function oboCapable(r: ManifestResource): boolean { + return Boolean(r.scope) && !r.appOnly; +} + +/** + * Auth mode per resource. Precedence: --set authMode, the prompted choice, the + * --auth-mode default, then sp. The obo default only applies to OBO-capable + * resources; the others are returned in `keptSP`. Only obo/both are stored. + */ +export function resolveAuthModes( + resources: ManifestResource[], + setModes: Record, + promptedModes: Record, + defaultMode: string | undefined, +): { modes: Record; keptSP: string[] } { + const modes: Record = {}; + const keptSP: string[] = []; + for (const r of resources) { + let mode: AuthMode | undefined = + setModes[authKey(r)] ?? promptedModes[authKey(r)]; + if (mode === undefined && defaultMode === AUTH_MODE_OBO) { + if (!oboCapable(r)) { + keptSP.push(r.resourceKey); + continue; + } + mode = AUTH_MODE_OBO; + } + if (mode === AUTH_MODE_OBO || mode === AUTH_MODE_BOTH) { + modes[authKey(r)] = mode; + } + } + return { modes, keptSP }; +} + +/** True when any field value of the resource is set. */ +export function resourceConfigured( + r: ManifestResource, + values: Record, +): boolean { + return [...fieldNames(r), ...bindingVarFields(r)].some( + (f) => values[valueKey(r, f)], + ); +} + +/** + * The resources that end up in the app: every required one, plus optional + * ones that have a value. + */ +export function selectResources( + plugin: ManifestPlugin, + values: Record, +): ManifestResource[] { + return [ + ...plugin.resources.required, + ...plugin.resources.optional.filter((r) => resourceConfigured(r, values)), + ]; +} + +export interface MissingValue { + resource: ManifestResource; + field: string; + /** The --set key that supplies it. */ + setKey: string; +} + +/** + * Values still needed, in prompt order: for each resource bound to the service + * principal (sp/both), every field its binding references; for resources + * accessed only on behalf of the user, every field with an env var (its + * app.yaml entry carries the literal value). Fields with a resolver (e.g. + * Lakebase PGHOST) are always needed: local .env must carry them to connect. + */ +export function findMissingValues( + pluginName: string, + resources: ManifestResource[], + values: Record, + modes: Record, +): MissingValue[] { + const missing: MissingValue[] = []; + for (const r of resources) { + const fields = + modes[authKey(r)] === AUTH_MODE_OBO + ? fieldNames(r).filter( + (f) => r.fields?.[f].env && !r.fields[f].localOnly, + ) + : variableFields(r); + const resolved = fieldNames(r).filter((f) => r.fields?.[f].resolve); + for (const field of new Set([...fields, ...resolved])) { + if (values[valueKey(r, field)] || r.fields?.[field]?.value) continue; + missing.push({ + resource: r, + field, + setKey: `${pluginName}.${r.resourceKey}.${field}`, + }); + } + } + return missing; +} + +/** Fields that become bundle variables: manifest fields plus binding var fields. */ +function variableFields(r: ManifestResource): string[] { + const out = fieldNames(r).filter( + (f) => !r.fields?.[f].bundleIgnore && !r.fields?.[f].localOnly, + ); + const covered = new Set(fieldNames(r)); + for (const f of bindingVarFields(r)) { + if (!covered.has(f)) out.push(f); + } + return out; +} + +function varName(r: ManifestResource, field: string): string { + return `${r.resourceKey.replaceAll("-", "_")}_${field}`; +} + +export interface AddPlan extends ConfigPlan { + /** .env lines to add (skipped later when the name is already present). */ + dotEnv: { env: string; value: string }[]; +} + +/** + * Builds the file changes for a plugin's resources, mirroring the CLI + * generator: OBO-only resources get a literal app.yaml `value` and no bundle + * binding; sp/both resources get variables, a binding and `valueFrom`; + * obo/both resources add their scope, and the plugin always adds its own, to + * user_api_scopes. + */ +export function buildAddPlan( + plugin: ManifestPlugin, + resources: ManifestResource[], + values: Record, + modes: Record, +): AddPlan { + const dotEnv: AddPlan["dotEnv"] = []; + const appYamlEnv: AppYamlEnvEntry[] = []; + const bundleVariables: BundleVariable[] = []; + const resourceBindings: ResourceBinding[] = []; + const unverifiedTypes: string[] = []; + const scopes: string[] = []; + + for (const r of resources) { + const mode = modes[authKey(r)]; + const oboOnly = mode === AUTH_MODE_OBO; + if ((mode === AUTH_MODE_OBO || mode === AUTH_MODE_BOTH) && r.scope) { + scopes.push(r.scope); + } + + for (const f of fieldNames(r)) { + const field = r.fields?.[f] ?? {}; + if (!field.env || !isValidEnvName(field.env)) continue; + const value = (values[valueKey(r, f)] || field.value || "").replace( + /[\r\n]/g, + "", + ); + dotEnv.push({ env: field.env, value }); + if (field.localOnly) continue; + appYamlEnv.push( + oboOnly + ? { name: field.env, value } + : { name: field.env, valueFrom: r.resourceKey }, + ); + } + + if (oboOnly) continue; + const binding = bindingOf(r); + if (!binding) { + unverifiedTypes.push(r.type); + continue; + } + for (const f of variableFields(r)) { + bundleVariables.push({ + name: varName(r, f), + description: r.fields?.[f]?.description || r.description, + value: values[valueKey(r, f)] || r.fields?.[f]?.value || undefined, + }); + } + const fields: Record = {}; + for (const [mf, yf] of binding.varFields) { + fields[yf] = `\${var.${varName(r, mf)}}`; + } + for (const [yf, v] of binding.staticFields ?? []) fields[yf] = v; + resourceBindings.push({ + name: r.resourceKey, + type: binding.yamlKey, + permission: r.permission, + fields, + }); + } + + // Plugin scopes (e.g. genie) cover routes that always run on behalf of the + // user, so they apply whatever the resources' auth mode is. + scopes.push(...(plugin.scopes ?? [])); + return { + dotEnv, + appYamlEnv, + bundleVariables, + resourceBindings, + unverifiedTypes, + userApiScopes: [...new Set(scopes)], + }; +} diff --git a/packages/shared/src/cli/commands/plugin/add/resolve.test.ts b/packages/shared/src/cli/commands/plugin/add/resolve.test.ts new file mode 100644 index 000000000..0e756aadc --- /dev/null +++ b/packages/shared/src/cli/commands/plugin/add/resolve.test.ts @@ -0,0 +1,115 @@ +import { expect, test, vi } from "vitest"; + +import type { ManifestResource } from "./plan"; +import { discoverIndexColumns, resolveDerivedValues } from "./resolve"; + +let columnsToSync: string[] | undefined; +let columnsToIndex: string[] | undefined; +const listEndpointsIter = vi.fn(async function* () { + yield { name: "ro", status: { endpointType: "ENDPOINT_TYPE_READ_ONLY" } }; + yield { + name: "b/endpoints/rw", + status: { endpointType: "ENDPOINT_TYPE_READ_WRITE", hosts: { host: "h" } }, + }; +}); +const listDatabasesIter = vi.fn(async function* () { + yield { name: "b/databases/db", status: { postgresDatabase: "appdb" } }; +}); +vi.mock("../../registry/workspace-picker.js", () => ({ + makeWorkspaceClient: () => ({ + postgres: { listEndpointsIter, listDatabasesIter }, + vectorSearch: { + getVectorIndex: async () => ({ + indexSpec: { + $case: "deltaSyncIndexSpec", + deltaSyncIndexSpec: { + sourceTable: "c.s.t", + embeddingVectorColumns: [{ name: "vec" }], + columnsToSync, + columnsToIndex, + }, + }, + }), + }, + tables: { + getTable: async () => ({ + columns: [ + { name: "id" }, + { name: "text" }, + { name: "vec" }, + { name: "notes" }, + ], + }), + }, + }), +})); + +const pg: ManifestResource = { + type: "postgres", + alias: "Postgres", + resourceKey: "postgres", + fields: { + branch: {}, + database: {}, + host: { env: "PGHOST", resolve: "postgres:host" }, + databaseName: { env: "PGDATABASE", resolve: "postgres:databaseName" }, + endpointPath: { + env: "LAKEBASE_ENDPOINT", + resolve: "postgres:endpointPath", + }, + }, +}; + +test("maps the read-write endpoint and database to resolver fields", async () => { + const values = { + "postgres.branch": "b", + "postgres.database": "b/databases/db", + }; + expect(await resolveDerivedValues(pg, values, undefined)).toEqual({ + "postgres.host": "h", + "postgres.databaseName": "appdb", + "postgres.endpointPath": "b/endpoints/rw", + }); +}); + +test("skips the API when every derived field is set", async () => { + listEndpointsIter.mockClear(); + await resolveDerivedValues( + pg, + { + "postgres.branch": "b", + "postgres.host": "x", + "postgres.databaseName": "x", + "postgres.endpointPath": "x", + }, + undefined, + ); + expect(listEndpointsIter).not.toHaveBeenCalled(); +}); + +test("discoverIndexColumns drops embedding columns", async () => { + columnsToSync = undefined; + expect(await discoverIndexColumns("c.s.i", undefined)).toEqual([ + "id", + "text", + "notes", + ]); +}); + +test("discoverIndexColumns keeps only synced columns", async () => { + columnsToSync = ["id", "text", "vec"]; + expect(await discoverIndexColumns("c.s.i", undefined)).toEqual([ + "id", + "text", + ]); +}); + +test("discoverIndexColumns honors the columnsToIndex alias", async () => { + columnsToSync = undefined; + columnsToIndex = ["id", "text", "vec"]; + expect(await discoverIndexColumns("c.s.i", undefined)).toEqual([ + "id", + "text", + ]); + columnsToIndex = undefined; +}); diff --git a/packages/shared/src/cli/commands/plugin/add/resolve.ts b/packages/shared/src/cli/commands/plugin/add/resolve.ts new file mode 100644 index 000000000..58d47da7b --- /dev/null +++ b/packages/shared/src/cli/commands/plugin/add/resolve.ts @@ -0,0 +1,85 @@ +import type { ManifestResource } from "./plan"; + +/** + * Derived values for fields with a `resolve` key, a port of the Databricks + * CLI's ResolvePostgresValues (libs/apps/prompt): the branch's read-write + * endpoint gives `postgres:host` and `postgres:endpointPath`, the database + * gives `postgres:databaseName`. Returns `resourceKey.field` → value for the + * fields it could resolve; throws on API errors. + */ +export async function resolveDerivedValues( + r: ManifestResource, + values: Record, + profile: string | undefined, +): Promise> { + const branch = values[`${r.resourceKey}.branch`]; + const database = values[`${r.resourceKey}.database`]; + const needs = Object.entries(r.fields ?? {}).some( + ([name, f]) => + f.resolve?.startsWith("postgres:") && !values[`${r.resourceKey}.${name}`], + ); + if (r.type !== "postgres" || !needs || !branch) return {}; + + const { makeWorkspaceClient } = + await import("../../registry/workspace-picker.js"); + const pg = makeWorkspaceClient(profile).postgres; + const resolved: Record = {}; + for await (const ep of pg.listEndpointsIter({ parent: branch })) { + if (ep.status?.endpointType !== "ENDPOINT_TYPE_READ_WRITE") continue; + if (ep.name) resolved["postgres:endpointPath"] = ep.name; + if (ep.status.hosts?.host) resolved["postgres:host"] = ep.status.hosts.host; + break; + } + if (database) { + for await (const db of pg.listDatabasesIter({ parent: branch })) { + if (db.name !== database) continue; + if (db.status?.postgresDatabase) { + resolved["postgres:databaseName"] = db.status.postgresDatabase; + } + break; + } + } + + const out: Record = {}; + for (const [name, field] of Object.entries(r.fields ?? {})) { + const v = field.resolve && resolved[field.resolve]; + if (v) out[`${r.resourceKey}.${name}`] = v; + } + return out; +} + +/** + * Columns for an AI Search index: the Delta Sync source table's columns minus + * embedding vectors, limited to `columnsToSync` (or its alias `columnsToIndex`) when the index syncs a subset + * (other source columns aren't in the index, and querying them fails). + * Production refuses to start without `columns`, so they are written into the + * plugin config at add time. Throws when the index has no source table. + */ +export async function discoverIndexColumns( + indexName: string, + profile: string | undefined, +): Promise { + const { makeWorkspaceClient } = + await import("../../registry/workspace-picker.js"); + const client = makeWorkspaceClient(profile); + const index = await client.vectorSearch.getVectorIndex({ name: indexName }); + const spec = + index.indexSpec?.$case === "deltaSyncIndexSpec" + ? index.indexSpec.deltaSyncIndexSpec + : undefined; + if (!spec?.sourceTable) { + throw new Error(`index ${indexName} has no Delta Sync source table`); + } + const excluded = new Set(spec.embeddingVectorColumns?.map((c) => c.name)); + const table = await client.tables.getTable({ fullNameArg: spec.sourceTable }); + // columnsToIndex is the response's alias of columnsToSync. + const subset = spec.columnsToSync?.length + ? spec.columnsToSync + : spec.columnsToIndex; + const synced = subset?.length ? new Set(subset) : undefined; + return (table.columns ?? []).flatMap((c) => + c.name && !excluded.has(c.name) && (!synced || synced.has(c.name)) + ? [c.name] + : [], + ); +} diff --git a/packages/shared/src/cli/commands/plugin/index.ts b/packages/shared/src/cli/commands/plugin/index.ts index 6612d205a..810ae673b 100644 --- a/packages/shared/src/cli/commands/plugin/index.ts +++ b/packages/shared/src/cli/commands/plugin/index.ts @@ -1,6 +1,7 @@ import { Command } from "commander"; import { pluginAddResourceCommand } from "./add-resource/add-resource"; +import { pluginAddCommand } from "./add/add"; import { pluginCreateCommand } from "./create/create"; import { pluginListCommand } from "./list/list"; import { pluginPromoteCommand } from "./promote/promote"; @@ -14,6 +15,7 @@ import { pluginValidateCommand } from "./validate/validate"; * - create: Scaffold a new plugin (interactive) * - validate: Validate manifest(s) against the JSON schema * - list: List plugins from appkit.plugins.json or a directory + * - add: Add a plugin to the app (server, .env, app.yaml, databricks.yml) * - add-resource: Add a resource requirement to a plugin manifest (interactive) * - promote: Promote a plugin to a higher stability tier */ @@ -23,6 +25,7 @@ export const pluginCommand = new Command("plugin") .addCommand(pluginCreateCommand) .addCommand(pluginValidateCommand) .addCommand(pluginListCommand) + .addCommand(pluginAddCommand) .addCommand(pluginAddResourceCommand) .addCommand(pluginPromoteCommand) .addHelpText( @@ -33,6 +36,7 @@ Examples: $ appkit plugin create --placement in-repo --path plugins/my-plugin --name my-plugin --description "Does X" $ appkit plugin validate . $ appkit plugin list --json + $ appkit plugin add analytics --auth-mode obo --set analytics.sql-warehouse.id=abc123 $ appkit plugin add-resource --path plugins/my-plugin --type sql_warehouse $ appkit plugin promote my-plugin --to ga`, ); diff --git a/packages/shared/src/cli/commands/registry/config-plan.ts b/packages/shared/src/cli/commands/registry/config-plan.ts index 01260cf8a..168bd19f1 100644 --- a/packages/shared/src/cli/commands/registry/config-plan.ts +++ b/packages/shared/src/cli/commands/registry/config-plan.ts @@ -26,6 +26,8 @@ export interface ConfigPlan { appYamlEnv: AppYamlEnvEntry[]; bundleVariables: BundleVariable[]; resourceBindings: ResourceBinding[]; + /** Entries for resources.apps.app.user_api_scopes (added if absent). */ + userApiScopes?: string[]; /** Resource types encountered that have no verified binding spec. */ unverifiedTypes: string[]; } @@ -178,6 +180,7 @@ export function planHasContent(plan: ConfigPlan): boolean { return ( plan.appYamlEnv.length > 0 || plan.bundleVariables.length > 0 || - plan.resourceBindings.length > 0 + plan.resourceBindings.length > 0 || + (plan.userApiScopes?.length ?? 0) > 0 ); } diff --git a/packages/shared/src/cli/commands/registry/config-writer.test.ts b/packages/shared/src/cli/commands/registry/config-writer.test.ts index 9bcf7f04e..51a13cb7c 100644 --- a/packages/shared/src/cli/commands/registry/config-writer.test.ts +++ b/packages/shared/src/cli/commands/registry/config-writer.test.ts @@ -195,3 +195,62 @@ describe("writeConfig — additive patching", () => { expect(sameYaml(db, db)).toBe(true); }); }); + +describe("writeConfig — user_api_scopes and literal env values", () => { + it("adds missing scopes once and writes literal app.yaml values", () => { + const cwd = makeTempDir(); + fs.writeFileSync( + path.join(cwd, "databricks.yml"), + "resources:\n apps:\n app:\n user_api_scopes:\n - sql\n", + ); + const result = writeConfig(cwd, { + appYamlEnv: [{ name: "DATABRICKS_WAREHOUSE_ID", value: "wh" }], + bundleVariables: [], + resourceBindings: [], + userApiScopes: ["sql", "files.files"], + unverifiedTypes: [], + }); + const db = parseDocument( + fs.readFileSync(path.join(cwd, "databricks.yml"), "utf-8"), + ).toJSON(); + expect(db.resources.apps.app.user_api_scopes).toEqual([ + "sql", + "files.files", + ]); + expect(result.databricksYmlChanged).toBe(true); + const app = parseDocument( + fs.readFileSync(path.join(cwd, "app.yaml"), "utf-8"), + ).toJSON(); + expect(app.env).toEqual([{ name: "DATABRICKS_WAREHOUSE_ID", value: "wh" }]); + }); +}); + +describe("writeConfig — app key", () => { + it("writes scopes and bindings to the bundle's only app, whatever its key", () => { + const cwd = makeTempDir(); + fs.writeFileSync( + path.join(cwd, "databricks.yml"), + "resources:\n apps:\n dashboard:\n name: dash\n", + ); + writeConfig(cwd, { + appYamlEnv: [], + bundleVariables: [], + resourceBindings: [ + { + name: "wh", + type: "sql_warehouse", + permission: "CAN_USE", + fields: {}, + }, + ], + userApiScopes: ["sql"], + unverifiedTypes: [], + }); + const apps = parseDocument( + fs.readFileSync(path.join(cwd, "databricks.yml"), "utf-8"), + ).toJSON().resources.apps; + expect(Object.keys(apps)).toEqual(["dashboard"]); + expect(apps.dashboard.user_api_scopes).toEqual(["sql"]); + expect(apps.dashboard.resources[0].name).toBe("wh"); + }); +}); diff --git a/packages/shared/src/cli/commands/registry/config-writer.ts b/packages/shared/src/cli/commands/registry/config-writer.ts index 479d30ed7..d2cd1e9da 100644 --- a/packages/shared/src/cli/commands/registry/config-writer.ts +++ b/packages/shared/src/cli/commands/registry/config-writer.ts @@ -52,7 +52,7 @@ function patchAppYaml(file: string, entries: AppYamlEnvEntry[]): string[] { const added: string[] = []; for (const entry of entries) { if (existingNames.has(entry.name)) continue; - seq.add(doc.createNode({ name: entry.name, valueFrom: entry.valueFrom })); + seq.add(doc.createNode(entry)); added.push(entry.name); } @@ -79,6 +79,20 @@ function ensureMap( return node; } +/** + * Key of the bundle's app under `resources.apps`: the only one there (apps + * are often named after the project, not `app`), else `app`. Throws when + * several apps exist and none is `app`, rather than guess or add a new one. + */ +function appKey(doc: ReturnType): string { + const keys = Object.keys(doc.toJSON()?.resources?.apps ?? {}); + if (keys.length === 1) return keys[0]; + if (keys.length === 0 || keys.includes("app")) return "app"; + throw new Error( + `databricks.yml defines several apps (${keys.join(", ")}); add the resources to the right one by hand`, + ); +} + /** * Additively patches `databricks.yml`: adds bundle `variables`, the app * `resources` bindings, and the target-level variable values — each only if @@ -89,7 +103,11 @@ function patchDatabricksYml( file: string, plan: ConfigPlan, ): { added: string[]; changed: boolean } { - if (plan.bundleVariables.length === 0 && plan.resourceBindings.length === 0) { + if ( + plan.bundleVariables.length === 0 && + plan.resourceBindings.length === 0 && + !plan.userApiScopes?.length + ) { return { added: [], changed: false }; } const doc = loadDoc(file); @@ -109,7 +127,7 @@ function patchDatabricksYml( // App resource bindings. if (plan.resourceBindings.length > 0) { - const app = ensureMap(doc, ["resources", "apps", "app"]); + const app = ensureMap(doc, ["resources", "apps", appKey(doc)]); let bindings = app.get("resources") as YAMLSeq | undefined; if (!bindings || typeof (bindings as YAMLSeq).add !== "function") { app.set("resources", doc.createNode([])); @@ -127,6 +145,21 @@ function patchDatabricksYml( } } + if (plan.userApiScopes?.length) { + const app = ensureMap(doc, ["resources", "apps", appKey(doc)]); + let scopes = app.get("user_api_scopes") as YAMLSeq | undefined; + if (!scopes || typeof (scopes as YAMLSeq).add !== "function") { + app.set("user_api_scopes", doc.createNode([])); + scopes = app.get("user_api_scopes") as YAMLSeq; + } + const existing = new Set(scopes.toJSON() as unknown[]); + for (const scope of plan.userApiScopes) { + if (existing.has(scope)) continue; + scopes.add(scope); + added.push(scope); + } + } + // Target-level variable values. Tracked separately from `added` because the // binding/top-level var may already exist (e.g. from scaffold) while the // target VALUE is still missing — in that case nothing is in `added` yet the diff --git a/packages/shared/src/cli/commands/registry/env-writer.ts b/packages/shared/src/cli/commands/registry/env-writer.ts index 4f06b66e8..7f136a578 100644 --- a/packages/shared/src/cli/commands/registry/env-writer.ts +++ b/packages/shared/src/cli/commands/registry/env-writer.ts @@ -46,13 +46,13 @@ const MANUAL = "__manual__"; const PICKER_LIMIT = 25; /** Reads a `.env`-style file into a map; empty when the file is absent. */ -function readEnvFile(file: string): Record { +export function readEnvFile(file: string): Record { if (!fs.existsSync(file)) return {}; return parseEnv(fs.readFileSync(file, "utf-8")); } /** Appends text to a file, creating it (with a trailing newline) if needed. */ -function appendToFile(file: string, text: string): void { +export function appendToFile(file: string, text: string): void { if (text === "") return; if (fs.existsSync(file)) { const existing = fs.readFileSync(file, "utf-8"); diff --git a/packages/shared/src/cli/commands/registry/server-register.test.ts b/packages/shared/src/cli/commands/registry/server-register.test.ts index 0298545c7..34d089a22 100644 --- a/packages/shared/src/cli/commands/registry/server-register.test.ts +++ b/packages/shared/src/cli/commands/registry/server-register.test.ts @@ -38,6 +38,32 @@ afterEach(() => { }); describe("registerPluginInServer", () => { + it("accepts an external npm package and rejects code in the path", () => { + const dir = makeTempDir(); + writeServer(dir, "server.ts", "server()"); + expect( + registerPluginInServer(dir, "@example/analytics", "analytics").status, + ).toBe("wired"); + expect(registerPluginInServer(dir, `x"; evil(); "`, "other").status).toBe( + "skipped", + ); + }); + + it("passes call args and accepts the AppKit package import path", () => { + const dir = makeTempDir(); + const file = writeServer(dir, "server.ts", "server()"); + const result = registerPluginInServer( + dir, + "@databricks/appkit", + "files", + `{ auth: "on-behalf-of-user" }`, + ); + expect(result.status).toBe("wired"); + const out = fs.readFileSync(file, "utf-8"); + expect(out).toContain(`files({ auth: "on-behalf-of-user" }), server()`); + expect(out).toContain(`import { files } from "@databricks/appkit";`); + }); + it("wires a plugin into a server root that is a subdir (e.g. api/)", () => { // The server root is the resolved subdir, not the repo root — entry files // are looked up within it, so an app laid out under api/ still gets wired. diff --git a/packages/shared/src/cli/commands/registry/server-register.ts b/packages/shared/src/cli/commands/registry/server-register.ts index 327b0f4ee..26b10eb33 100644 --- a/packages/shared/src/cli/commands/registry/server-register.ts +++ b/packages/shared/src/cli/commands/registry/server-register.ts @@ -20,7 +20,7 @@ export interface RegisterResult { reason?: string; } -function findServerFile(serverRoot: string): string | null { +export function findServerFile(serverRoot: string): string | null { for (const candidate of SERVER_FILE_CANDIDATES) { const p = path.join(serverRoot, candidate); if (fs.existsSync(p)) return p; @@ -87,15 +87,21 @@ export function registerPluginInServer( serverRoot: string, importPath: string, exportName: string, + /** Call arguments, e.g. `{ auth: "on-behalf-of-user" }` (CLI-built, never registry data). */ + callArgs = "", ): RegisterResult { // exportName and importPath are interpolated into the user's server source. // Registry items are untrusted, so refuse anything that isn't a plain JS // identifier / clean relative module path — prevents code injection via a - // crafted export name or import path. + // crafted export name or import path. Plugins declared by an npm package + // (AppKit's or another) import from a plain package specifier. if (!JS_IDENTIFIER.test(exportName)) { return { status: "skipped", reason: "invalid plugin export name" }; } - if (!/^[.][./A-Za-z0-9_-]*$/.test(importPath)) { + if ( + !/^[.][./A-Za-z0-9_-]*$/.test(importPath) && + !/^(@[a-z0-9][\w.-]*\/)?[a-z0-9][\w.-]*(\/[\w.-]+)*$/i.test(importPath) + ) { return { status: "skipped", reason: "invalid plugin import path" }; } @@ -124,7 +130,7 @@ export function registerPluginInServer( const edits = []; // toPlugin exports are factories, registered as a call: `hello()`. - const newElem = `${exportName}()`; + const newElem = `${exportName}(${callArgs})`; // Insert before the first element, matching its indentation so the array // formatting is preserved (or inline for a single-line array). diff --git a/packages/shared/src/cli/deploy-config.ts b/packages/shared/src/cli/deploy-config.ts index 9da8c0df4..b0bdec65d 100644 --- a/packages/shared/src/cli/deploy-config.ts +++ b/packages/shared/src/cli/deploy-config.ts @@ -10,11 +10,14 @@ export const DATABRICKS_YML_FILE = "databricks.yml"; /** Canonical Databricks Apps runtime config file name. */ export const APP_YAML_FILE = "app.yaml"; -/** An `app.yaml` env entry: `- name: `, `valueFrom: `. */ -export interface AppYamlEnvEntry { - name: string; - valueFrom: string; -} +/** + * An `app.yaml` env entry: `- name: ` with `valueFrom: `, + * or a literal `value` (resources accessed only on behalf of the user have no + * binding to reference). + */ +export type AppYamlEnvEntry = + | { name: string; valueFrom: string } + | { name: string; value: string }; /** * A `databricks.yml` app-resource binding under `resources.apps..resources[]`,