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..4656ab5b0 --- /dev/null +++ b/packages/shared/src/cli/commands/plugin/add/add.test.ts @@ -0,0 +1,134 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { expect, test } from "vitest"; + +import { envConflicts, 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(); +}); + +test("envConflicts flags env names bound to another resource or value", () => { + const cwd = fs.mkdtempSync(path.join(os.tmpdir(), "plugin-add-")); + fs.writeFileSync(path.join(cwd, ".env"), "A=old\nB=\n"); + fs.writeFileSync( + path.join(cwd, "app.yaml"), + "env:\n - name: C\n valueFrom: serving-endpoint\n - name: D\n valueFrom: same\n", + ); + const plan = { + dotEnv: [ + { env: "A", value: "new" }, + { env: "B", value: "x" }, + ], + appYamlEnv: [ + { name: "C", valueFrom: "agents-serving-endpoint" }, + { name: "D", valueFrom: "same" }, + ], + bundleVariables: [], + resourceBindings: [], + unverifiedTypes: [], + }; + expect(envConflicts(cwd, plan)).toEqual(["A", "C"]); + fs.rmSync(cwd, { recursive: true, force: true }); +}); + +test("files with mixed modes sets auth on the OBO volumes only", () => { + const vol = (key: string, env: string): ManifestResource => ({ + type: "volume", + alias: key, + resourceKey: key, + scope: "files", + fields: { path: { env } }, + }); + const priv = vol("files", "DATABRICKS_VOLUME_FILES"); + const exp = vol("exports", "DATABRICKS_VOLUME_EXPORTS"); + expect( + pluginConfig(plugin("files"), [priv, exp], { "volume:files": "obo" }), + ).toEqual({ volumes: { files: { auth: "on-behalf-of-user" } } }); +}); 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..846007b68 --- /dev/null +++ b/packages/shared/src/cli/commands/plugin/add/add.ts @@ -0,0 +1,829 @@ +import fs from "node:fs"; +import path from "node:path"; +import process from "node:process"; + +import { Command } from "commander"; +import pc from "picocolors"; +import { parseDocument } from "yaml"; +import { z } from "zod"; + +import { APP_YAML_FILE } from "../../../deploy-config"; +import { profileFromEnv } from "../../registry/add"; +import type { ConfigPlan } from "../../registry/config-plan"; +import { + assertBundleApp, + planHasContent, + reportConfigWrite, + validateBundle, + writeConfig, +} from "../../registry/config-writer"; +import { parseEnv, serializeEnvAppend } from "../../registry/env-reconcile"; +import { + AUTH_MODE_BOTH, + AUTH_MODE_FIELD, + AUTH_MODE_OBO, + AUTH_MODE_SP, + type AddPlan, + 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[]; + fromPlan?: 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); +} + +/** The env var agents falls back to for its model; serving binds it too. */ +const AGENTS_MODEL_ENV = "DATABRICKS_SERVING_ENDPOINT_NAME"; + +/** + * Env names in the plan already bound to something else: a different + * non-empty value in .env, or an app.yaml entry with another valueFrom/value. + */ +export function envConflicts( + cwd: string, + plan: AddPlan, + /** Plans of earlier plugins in the same run, not yet written. */ + pending: AddPlan[] = [], +): string[] { + const dotEnv = fs.existsSync(path.join(cwd, ".env")) + ? parseEnv(fs.readFileSync(path.join(cwd, ".env"), "utf-8")) + : {}; + const appYamlFile = path.join(cwd, APP_YAML_FILE); + const appEnv = new Map(); + if (fs.existsSync(appYamlFile)) { + const env = parseDocument(fs.readFileSync(appYamlFile, "utf-8")).toJSON() + ?.env as + | { name?: string; valueFrom?: string; value?: string }[] + | undefined; + for (const e of env ?? []) { + if (e?.name) appEnv.set(e.name, JSON.stringify([e.valueFrom, e.value])); + } + } + for (const p of pending) { + for (const e of p.dotEnv) if (!dotEnv[e.env]) dotEnv[e.env] = e.value; + for (const e of p.appYamlEnv) { + if (!appEnv.has(e.name)) { + appEnv.set( + e.name, + JSON.stringify([ + "valueFrom" in e ? e.valueFrom : undefined, + "value" in e ? e.value : undefined, + ]), + ); + } + } + } + const out = new Set(); + for (const e of plan.dotEnv) { + if (dotEnv[e.env] && e.value && dotEnv[e.env] !== e.value) out.add(e.env); + } + for (const e of plan.appYamlEnv) { + const want = JSON.stringify([ + "valueFrom" in e ? e.valueFrom : undefined, + "value" in e ? e.value : undefined, + ]); + const have = appEnv.get(e.name); + if (have !== undefined && have !== want) out.add(e.name); + } + return [...out]; +} + +/** One plugin of a run: its name, its --set pairs and its default auth mode. */ +export interface PluginRequest { + name: string; + set: string[]; + authMode?: string; +} + +interface PreparedPlugin { + plugin: ManifestPlugin; + plan: AddPlan; + config?: Record; +} + +/** + * Everything for one plugin short of writing: prompts, derived values, the + * file plan and its registration config. Throws on conflicts with the app's + * files or with earlier plugins of the run (`pending`). + */ +async function preparePlugin( + cwd: string, + req: PluginRequest, + opts: AddOptions, + pending: AddPlan[], +): Promise { + if ( + req.authMode !== undefined && + req.authMode !== AUTH_MODE_SP && + req.authMode !== AUTH_MODE_OBO + ) { + throw new Error( + `invalid --auth-mode value: "${req.authMode}" (must be obo or sp; use --set ..${AUTH_MODE_FIELD}=both for a single resource)`, + ); + } + const plugin = loadPlugin(cwd, req.name); + const values = parseSetValues(req.set, plugin); + const setModes = parseSetAuthModes(req.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 && req.authMode === undefined + ? await promptAuthModes( + resources.filter((r) => !(authKey(r) in setModes)), + ) + : {}; + const { modes, keptSP } = resolveAuthModes( + resources, + setModes, + promptedModes, + req.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); + + let config = pluginConfig( + plugin, + resources, + modes, + plugin.name === "aiSearch" && opts.register !== false + ? await aiSearchColumns(values, profile, interactive) + : undefined, + ); + + const plan = buildAddPlan(plugin, resources, values, modes); + + // An env var holds one value: if another resource already binds one of + // these names to something else, the new value would never reach the app. + const conflicts = envConflicts(cwd, plan, pending); + const model = values["agents-serving-endpoint.name"]; + if ( + plugin.name === "agents" && + model && + conflicts.includes(AGENTS_MODEL_ENV) + ) { + // agents takes its model from defaultModel before that env var. + config = { ...config, defaultModel: model }; + plan.dotEnv = plan.dotEnv.filter((e) => e.env !== AGENTS_MODEL_ENV); + plan.appYamlEnv = plan.appYamlEnv.filter( + (e) => e.name !== AGENTS_MODEL_ENV, + ); + conflicts.splice(conflicts.indexOf(AGENTS_MODEL_ENV), 1); + console.log( + pc.dim( + `${AGENTS_MODEL_ENV} is already used by another resource; agents gets the endpoint as defaultModel in its registration instead.`, + ), + ); + } + if (conflicts.length > 0) { + throw new Error( + `${conflicts.join(", ")} already set to a different resource or value in .env or app.yaml; nothing was written. Edit those entries by hand, then rerun.`, + ); + } + + return { plugin, plan, config }; +} + +/** Requests from `plugin add a b --set ...`: each plugin gets its own --set pairs. */ +export function requestsFromArgs( + names: string[], + set: string[], + authMode: string | undefined, +): PluginRequest[] { + for (const sv of set) { + const prefix = sv.split(".")[0]; + if (!names.includes(prefix)) { + throw new Error( + `--set "${sv}" is for "${prefix}", which this run doesn't add (${names.join(", ")})`, + ); + } + } + return names.map((name) => ({ + name, + set: set.filter((sv) => sv.split(".")[0] === name), + authMode, + })); +} + +const dataPlanSchema = z.object({ + version: z.literal(1), + plugins: z + .array( + z.object({ + name: z.string().min(1), + authMode: z.enum([AUTH_MODE_OBO, AUTH_MODE_SP]).optional(), + /** + * `resourceKey.field` (or `resourceKey.authMode`) → value; the full + * --set key with the plugin prefix is accepted too. + */ + set: z.record(z.string(), z.string()).optional(), + }), + ) + .min(1), +}); + +/** Requests from a data plan file (`.appkit/data-plan.json`, version 1). */ +export function requestsFromPlan(file: string): PluginRequest[] { + const parsed = dataPlanSchema.safeParse( + JSON.parse(fs.readFileSync(file, "utf-8")), + ); + if (!parsed.success) { + throw new Error( + `invalid data plan ${file}: ${parsed.error.issues.map((i) => `${i.path.join(".")}: ${i.message}`).join("; ")}`, + ); + } + return parsed.data.plugins.map((p) => ({ + name: p.name, + authMode: p.authMode, + set: Object.entries(p.set ?? {}).map(([k, v]) => { + // Prefixed = 3+ parts led by the plugin name; counting parts keeps + // "files.path" (resourceKey "files") unprefixed. + const parts = k.split("."); + const prefixed = parts.length >= 3 && parts[0] === p.name; + return `${prefixed ? k : `${p.name}.${k}`}=${v}`; + }), + })); +} + +/** + * Adds every requested plugin in one pass: all are planned first (nothing is + * written if any fails), then each file is written at most once, so a dev + * server watching them restarts once. + */ +async function runPluginAdd(names: string[], opts: AddOptions): Promise { + const cwd = opts.cwd ? path.resolve(opts.cwd) : process.cwd(); + if ( + opts.fromPlan && + (names.length > 0 || opts.set.length > 0 || opts.authMode) + ) { + throw new Error( + "--from-plan can't be combined with plugin names, --set or --auth-mode", + ); + } + if (!opts.fromPlan && names.length === 0) { + throw new Error("name at least one plugin, or pass --from-plan "); + } + const requests = opts.fromPlan + ? requestsFromPlan(path.resolve(cwd, opts.fromPlan)) + : requestsFromArgs(names, opts.set, opts.authMode); + const seen = new Set(); + for (const r of requests) { + if (seen.has(r.name)) throw new Error(`plugin "${r.name}" is listed twice`); + seen.add(r.name); + } + + // agents is planned last: it is the one plugin that adapts to an env clash + // (see preparePlugin), so it must see the others' entries in any order. + const ordered = [ + ...requests.filter((r) => r.name !== "agents"), + ...requests.filter((r) => r.name === "agents"), + ]; + const prepared: PreparedPlugin[] = []; + for (const req of ordered) { + prepared.push( + await preparePlugin( + cwd, + req, + opts, + prepared.map((p) => p.plan), + ), + ); + } + + // Fail before any write rather than leave a partial install. + assertBundleApp(cwd); + + if (opts.register === false) { + for (const { plugin, config } of prepared) { + if (!config) continue; + // The config carries required settings (auth mode, columns, model). + console.log( + `${pc.bold("Register it with this config in your createApp call:")}\n` + + pc.dim(` ${plugin.name}(${toSource(config)})`), + ); + } + } else { + await registerInServer(cwd, prepared); + } + + const plans = prepared.map((p) => p.plan); + await writeDotEnv( + cwd, + plans.flatMap((p) => p.dotEnv), + ); + + // Plugins can share a resource (same env var, variable and binding); + // envConflicts already rejected any that differ, so keep the first. + const byName = (xs: T[]) => + xs.filter((x, i) => xs.findIndex((y) => y.name === x.name) === i); + const merged: ConfigPlan = { + appYamlEnv: byName(plans.flatMap((p) => p.appYamlEnv)), + bundleVariables: byName(plans.flatMap((p) => p.bundleVariables)), + resourceBindings: byName(plans.flatMap((p) => p.resourceBindings)), + userApiScopes: [...new Set(plans.flatMap((p) => p.userApiScopes ?? []))], + unverifiedTypes: [...new Set(plans.flatMap((p) => p.unverifiedTypes))], + }; + if (planHasContent(merged)) { + const result = writeConfig(cwd, merged); + reportConfigWrite(result); + if (result.databricksYmlChanged) { + validateBundle(cwd, opts.profile ?? profileFromEnv(cwd)); + } + } +} + +/** + * Adds the entries to .env in one write: empty placeholders (`KEY=`) are + * filled in place, new names appended, set values kept. + */ +async function writeDotEnv( + cwd: string, + entries: { env: string; value: string }[], +): Promise { + const { readEnvFile } = await import("../../registry/env-writer.js"); + const envPath = path.join(cwd, ".env"); + const existing = readEnvFile(envPath); + const dotEnv = entries.filter( + (e, i) => entries.findIndex((x) => x.env === e.env) === i, + ); + const filled = dotEnv.filter((e) => existing[e.env] === "" && e.value); + const newEnv = dotEnv.filter((e) => !(e.env in existing)); + if (filled.length === 0 && newEnv.length === 0) return; + let content = fs.existsSync(envPath) ? 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, + ); + } + const append = serializeEnvAppend(newEnv); + if (append) { + if (content.length > 0 && !content.endsWith("\n")) content += "\n"; + content += append; + } + fs.writeFileSync(envPath, content); + console.log( + `${pc.green("Updated .env:")} ${[...filled, ...newEnv].map((e) => e.env).join(", ")}`, + ); +} + +/** 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).`, + ), + ); + } + } + } + // files takes auth per volume too: a volume's key is the lowercased + // suffix of its DATABRICKS_VOLUME_ env var (FilesPlugin.discoverVolumes). + if (plugin.name === "files" && oboRes.length > 0 && !obo) { + const keys = oboRes.map((r) => filesVolumeKey(r)); + if (keys.every(Boolean)) { + return { + volumes: Object.fromEntries(keys.map((k) => [k, { auth: OBO }])), + }; + } + } + 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; +} + +/** The files plugin's volume key for a volume resource, from its env var. */ +function filesVolumeKey(r: ManifestResource): string | undefined { + const prefix = "DATABRICKS_VOLUME_"; + const env = Object.values(r.fields ?? {}) + .map((f) => f.env) + .find((e) => e?.startsWith(prefix) && e.length > prefix.length); + return env?.slice(prefix.length).toLowerCase(); +} + +/** 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, + prepared: PreparedPlugin[], +): 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, registerPluginsInServer } = + await import("../../registry/server-register.js"); + const serverFile = findServerFile(serverRoot); + const entries = prepared.map(({ plugin, config }) => { + // 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(serverFile ?? serverRoot); + const rel = path + .relative(from, path.resolve(cwd, plugin.package)) + .split(path.sep) + .join("/"); + importPath = rel.startsWith(".") ? rel : `./${rel}`; + } + return { + importPath, + exportName: plugin.name, + callArgs: config ? toSource(config) : "", + }; + }); + const results = registerPluginsInServer(serverRoot, entries); + results.forEach((result, i) => { + const { importPath, exportName, callArgs } = entries[i]; + const call = `${exportName}(${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(`${exportName} 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 ${exportName}: ${result.reason}.`) + + "\nAdd it to your server's createApp call:\n" + + pc.dim( + ` import { ${exportName} } from "${importPath}";\n` + + ` createApp({ plugins: [${call}, /* ... */] });`, + ), + ); + } + }); +} + +/** A fresh `plugin add` command (commander keeps parsed values per instance). */ +export const createPluginAddCommand = () => + new Command("add") + .description( + "Add plugins from appkit.plugins.json to the app: server registration, .env, app.yaml and databricks.yml, each written once", + ) + .argument("[plugins...]", "Plugin names, as in appkit.plugins.json") + .option( + "--from-plan ", + "Add the plugins listed in a data plan file (e.g. .appkit/data-plan.json, version 1)", + ) + .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. + +Several plugins can be added in one run; each file is written once, so a dev +server watching them restarts once. Rerunning with the same input changes nothing. +Existing entries in .env, app.yaml and databricks.yml are never overwritten. + +Data plan file (--from-plan), version 1: + { "version": 1, + "plugins": [ + { "name": "analytics", "authMode": "obo", + "set": { "sql-warehouse.id": "abc123" } }, + { "name": "genie", "set": { "genie-space.id": "s1", "genie-space.name": "Sales" } } ] } + "set" keys are the --set keys, with or without the plugin prefix ("authMode" works per + resource, e.g. "sql-warehouse.authMode": "both"); "authMode" is the plugin's default. + +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 + $ appkit plugin add analytics genie --set analytics.sql-warehouse.id=abc123 \\ + --set genie.genie-space.id=s1 --set genie.genie-space.name=Sales + $ appkit plugin add --from-plan .appkit/data-plan.json --yes`, + ) + .action((names: string[], opts: AddOptions) => + runPluginAdd(names, opts).catch((err: Error) => { + console.error(pc.red(err.message)); + process.exit(1); + }), + ); + +export const pluginAddCommand = createPluginAddCommand(); 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/add/run.test.ts b/packages/shared/src/cli/commands/plugin/add/run.test.ts new file mode 100644 index 000000000..db102843c --- /dev/null +++ b/packages/shared/src/cli/commands/plugin/add/run.test.ts @@ -0,0 +1,270 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { afterEach, describe, expect, test, vi } from "vitest"; + +import { + createPluginAddCommand, + requestsFromArgs, + requestsFromPlan, +} from "./add"; + +// bundle validate would shell out to the Databricks CLI. +vi.mock("../../registry/config-writer", async (importOriginal) => ({ + ...(await importOriginal()), + validateBundle: () => true, +})); + +const MANIFEST = path.resolve( + __dirname, + "../../../../../../../template/appkit.plugins.json", +); +const dirs: string[] = []; + +function makeApp(): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "plugin-add-run-")); + dirs.push(dir); + fs.copyFileSync(MANIFEST, path.join(dir, "appkit.plugins.json")); + fs.mkdirSync(path.join(dir, "server")); + fs.writeFileSync( + path.join(dir, "server", "server.ts"), + `import { createApp, server } from "@databricks/appkit";\n\ncreateApp({ plugins: [server()] });\n`, + ); + return dir; +} + +afterEach(() => { + for (const d of dirs) fs.rmSync(d, { recursive: true, force: true }); + dirs.length = 0; + vi.restoreAllMocks(); +}); + +describe("requestsFromArgs", () => { + test("gives each plugin its own --set pairs", () => { + expect( + requestsFromArgs( + ["analytics", "genie"], + ["analytics.sql-warehouse.id=wh", "genie.genie-space.id=s"], + "obo", + ), + ).toEqual([ + { + name: "analytics", + set: ["analytics.sql-warehouse.id=wh"], + authMode: "obo", + }, + { name: "genie", set: ["genie.genie-space.id=s"], authMode: "obo" }, + ]); + }); + + test("rejects --set for a plugin the run doesn't add", () => { + expect(() => + requestsFromArgs(["analytics"], ["genie.genie-space.id=s"], undefined), + ).toThrow("doesn't add"); + }); +}); + +describe("requestsFromPlan", () => { + test("maps plan entries to prefixed --set pairs", () => { + const dir = makeApp(); + const file = path.join(dir, "plan.json"); + fs.writeFileSync( + file, + JSON.stringify({ + version: 1, + plugins: [ + { + name: "analytics", + authMode: "sp", + set: { "sql-warehouse.id": "wh" }, + }, + ], + }), + ); + expect(requestsFromPlan(file)).toEqual([ + { + name: "analytics", + authMode: "sp", + set: ["analytics.sql-warehouse.id=wh"], + }, + ]); + }); + + test("accepts set keys that already carry the plugin prefix", () => { + const dir = makeApp(); + const file = path.join(dir, "plan.json"); + fs.writeFileSync( + file, + JSON.stringify({ + version: 1, + plugins: [ + { name: "analytics", set: { "analytics.sql-warehouse.id": "wh" } }, + { name: "files", set: { "files.path": "/Volumes/c/s/v" } }, + ], + }), + ); + expect(requestsFromPlan(file).map((r) => r.set)).toEqual([ + ["analytics.sql-warehouse.id=wh"], + ["files.files.path=/Volumes/c/s/v"], + ]); + }); + + test("rejects an unknown version or auth mode", () => { + const dir = makeApp(); + const file = path.join(dir, "plan.json"); + fs.writeFileSync( + file, + JSON.stringify({ + version: 2, + plugins: [{ name: "a", authMode: "both" }], + }), + ); + expect(() => requestsFromPlan(file)).toThrow( + /invalid data plan.*version.*authMode/s, + ); + }); +}); + +test("adds several plugins writing each file once, and nothing on a rerun", async () => { + const dir = makeApp(); + fs.mkdirSync(path.join(dir, ".appkit")); + fs.writeFileSync( + path.join(dir, ".appkit", "data-plan.json"), + JSON.stringify({ + version: 1, + plugins: [ + { + name: "analytics", + authMode: "obo", + set: { "sql-warehouse.id": "wh-1" }, + }, + { + name: "genie", + set: { "genie-space.id": "space-1", "genie-space.name": "Sales" }, + }, + ], + }), + ); + vi.spyOn(console, "log").mockImplementation(() => {}); + vi.spyOn(console, "warn").mockImplementation(() => {}); + const run = () => + createPluginAddCommand().parseAsync( + ["--from-plan", ".appkit/data-plan.json", "--yes", "-C", dir], + { from: "user" }, + ); + + const writes = vi.spyOn(fs, "writeFileSync"); + await run(); + const written = writes.mock.calls.map((c) => + path.relative(dir, String(c[0])), + ); + expect(written.sort()).toEqual([ + ".env", + "app.yaml", + "databricks.yml", + "server/server.ts", + ]); + + const server = fs.readFileSync( + path.join(dir, "server", "server.ts"), + "utf-8", + ); + expect(server).toContain( + `import { createApp, server, analytics, genie } from "@databricks/appkit";`, + ); + expect(fs.readFileSync(path.join(dir, ".env"), "utf-8")).toContain( + "DATABRICKS_WAREHOUSE_ID=wh-1", + ); + const yml = fs.readFileSync(path.join(dir, "databricks.yml"), "utf-8"); + expect(yml).toContain("- sql"); + expect(yml).toContain("- genie"); + + writes.mockClear(); + await run(); + expect(writes).not.toHaveBeenCalled(); +}); + +test.each([[["agents", "serving"]], [["serving", "agents"]]])( + "agents and serving work in either order (%j)", + async (names) => { + const dir = makeApp(); + vi.spyOn(console, "log").mockImplementation(() => {}); + vi.spyOn(console, "warn").mockImplementation(() => {}); + await createPluginAddCommand().parseAsync( + [ + ...names, + "--set", + "agents.agents-serving-endpoint.name=agent-model", + "--set", + "serving.serving-endpoint.name=serving-model", + "--yes", + "-C", + dir, + ], + { from: "user" }, + ); + const server = fs.readFileSync( + path.join(dir, "server", "server.ts"), + "utf-8", + ); + expect(server).toContain(`agents({ defaultModel: "agent-model" })`); + expect(fs.readFileSync(path.join(dir, ".env"), "utf-8")).toContain( + "DATABRICKS_SERVING_ENDPOINT_NAME=serving-model", + ); + }, +); + +test("plugins sharing a resource in one run write its entries once", async () => { + const dir = makeApp(); + const file = path.join(dir, "appkit.plugins.json"); + const manifest = JSON.parse(fs.readFileSync(file, "utf-8")); + manifest.plugins.reporting = { + ...manifest.plugins.analytics, + name: "reporting", + }; + fs.writeFileSync(file, JSON.stringify(manifest)); + vi.spyOn(console, "log").mockImplementation(() => {}); + vi.spyOn(console, "warn").mockImplementation(() => {}); + await createPluginAddCommand().parseAsync( + [ + "analytics", + "reporting", + "--set", + "analytics.sql-warehouse.id=wh", + "--set", + "reporting.sql-warehouse.id=wh", + "--yes", + "-C", + dir, + ], + { from: "user" }, + ); + const appYaml = fs.readFileSync(path.join(dir, "app.yaml"), "utf-8"); + expect(appYaml.match(/DATABRICKS_WAREHOUSE_ID/g)).toHaveLength(1); + const yml = fs.readFileSync(path.join(dir, "databricks.yml"), "utf-8"); + expect(yml.match(/- name: sql-warehouse/g)).toHaveLength(1); +}); + +test("--from-plan with prefixed set keys adds the plugin", async () => { + const dir = makeApp(); + fs.mkdirSync(path.join(dir, ".appkit")); + fs.writeFileSync( + path.join(dir, ".appkit", "data-plan.json"), + JSON.stringify({ + version: 1, + plugins: [ + { name: "analytics", set: { "analytics.sql-warehouse.id": "wh-9" } }, + ], + }), + ); + vi.spyOn(console, "log").mockImplementation(() => {}); + vi.spyOn(console, "warn").mockImplementation(() => {}); + await createPluginAddCommand().parseAsync( + ["--from-plan", ".appkit/data-plan.json", "--yes", "-C", dir], + { from: "user" }, + ); + expect(fs.readFileSync(path.join(dir, ".env"), "utf-8")).toContain( + "DATABRICKS_WAREHOUSE_ID=wh-9", + ); +}); 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/add-package-manager.test.ts b/packages/shared/src/cli/commands/registry/add-package-manager.test.ts index 065e73b3c..4e7ef4cbb 100644 --- a/packages/shared/src/cli/commands/registry/add-package-manager.test.ts +++ b/packages/shared/src/cli/commands/registry/add-package-manager.test.ts @@ -83,4 +83,30 @@ describe("registry add package manager", () => { ); }, ); + + it("adds a UI-only item to a bundle with several apps", async () => { + fs.writeFileSync( + path.join(cwd, "databricks.yml"), + "resources:\n apps:\n frontend: {}\n backend: {}\n", + ); + vi.mocked(fetchRegistryItem).mockResolvedValue({ + name: "pm-fixture", + files: [ + { + type: "registry:component", + path: "banner.tsx", + target: "components/banner.tsx", + content: "export {};\n", + }, + ], + }); + + await addCommand.parseAsync(["pm-fixture", "--cwd", cwd, "--yes"], { + from: "user", + }); + + expect(fs.existsSync(path.join(cwd, "components", "banner.tsx"))).toBe( + true, + ); + }); }); diff --git a/packages/shared/src/cli/commands/registry/add.ts b/packages/shared/src/cli/commands/registry/add.ts index 086239108..bb492e8c6 100644 --- a/packages/shared/src/cli/commands/registry/add.ts +++ b/packages/shared/src/cli/commands/registry/add.ts @@ -22,6 +22,7 @@ import { planHasContent, } from "./config-plan"; import { + assertBundleApp, reportConfigWrite, validateBundle, writeConfig, @@ -486,6 +487,15 @@ async function runAdd(refs: string[], opts: AddOptions): Promise { } } + // The deploy-config write refuses an ambiguous bundle: check before any + // file, and only when an item has resources to write (UI items don't). + if ( + opts.resources !== false && + items.some((i) => extractRequirements(i).length > 0) + ) { + assertBundleApp(cwd); + } + const hasUi = items.some((i) => !isPluginItem(i)); const hasPlugin = items.some(isPluginItem); const frontendRoot = hasUi ? findFrontendRoot(cwd) : cwd; 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..82cb37f7e 100644 --- a/packages/shared/src/cli/commands/registry/config-writer.test.ts +++ b/packages/shared/src/cli/commands/registry/config-writer.test.ts @@ -6,7 +6,7 @@ import { afterEach, describe, expect, it } from "vitest"; import { parseDocument } from "yaml"; import { buildConfigPlan } from "./config-plan"; -import { writeConfig } from "./config-writer"; +import { assertBundleApp, writeConfig } from "./config-writer"; import type { ResourceRequirementRow } from "./requirements"; const FIXTURES = path.join(__dirname, "__fixtures__"); @@ -195,3 +195,78 @@ 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"); + }); +}); + +describe("assertBundleApp", () => { + it("throws for several apps without one named app, before any write", () => { + const cwd = makeTempDir(); + fs.writeFileSync( + path.join(cwd, "databricks.yml"), + "resources:\n apps:\n frontend: {}\n backend: {}\n", + ); + expect(() => assertBundleApp(cwd)).toThrow("several apps"); + fs.writeFileSync( + path.join(cwd, "databricks.yml"), + "resources:\n apps:\n dash: {}\n", + ); + expect(() => assertBundleApp(cwd)).not.toThrow(); + }); +}); diff --git a/packages/shared/src/cli/commands/registry/config-writer.ts b/packages/shared/src/cli/commands/registry/config-writer.ts index 479d30ed7..78e94ce27 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,25 @@ 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`, + ); +} + +/** Throws (see {@link appKey}) when databricks.yml has no single target app. */ +export function assertBundleApp(cwd: string): void { + appKey(loadDoc(path.join(cwd, DATABRICKS_YML_FILE))); +} + /** * Additively patches `databricks.yml`: adds bundle `variables`, the app * `resources` bindings, and the target-level variable values — each only if @@ -89,7 +108,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 +132,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 +150,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..13094a703 100644 --- a/packages/shared/src/cli/commands/registry/env-writer.ts +++ b/packages/shared/src/cli/commands/registry/env-writer.ts @@ -46,7 +46,7 @@ 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")); } 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..4809d31c2 100644 --- a/packages/shared/src/cli/commands/registry/server-register.test.ts +++ b/packages/shared/src/cli/commands/registry/server-register.test.ts @@ -2,9 +2,12 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; -import { registerPluginInServer } from "./server-register"; +import { + registerPluginInServer, + registerPluginsInServer, +} from "./server-register"; const tempDirs: string[] = []; @@ -37,7 +40,142 @@ afterEach(() => { tempDirs.length = 0; }); +describe("registerPluginsInServer", () => { + it("registers several plugins with one write, and none on a rerun", () => { + const dir = makeTempDir(); + const file = writeServer(dir, "server.ts", "server()"); + const entries = [ + { importPath: "@databricks/appkit", exportName: "analytics" }, + { importPath: "@databricks/appkit", exportName: "genie" }, + { + importPath: "@databricks/appkit/beta", + exportName: "agents", + callArgs: `{ auth: "on-behalf-of-user" }`, + }, + ]; + const writes = vi.spyOn(fs, "writeFileSync"); + const first = registerPluginsInServer(dir, entries); + expect(first.map((r) => r.status)).toEqual(["wired", "wired", "wired"]); + expect(writes).toHaveBeenCalledTimes(1); + const out = fs.readFileSync(file, "utf-8"); + expect(out).toContain( + `import { createApp, analytics, genie } from "@databricks/appkit";`, + ); + expect(out).toContain(`import { agents } from "@databricks/appkit/beta";`); + expect(out).toContain( + `agents({ auth: "on-behalf-of-user" }), genie(), analytics(), server()`, + ); + + writes.mockClear(); + const second = registerPluginsInServer(dir, entries); + expect(second.map((r) => r.status)).toEqual([ + "already", + "already", + "already", + ]); + expect(writes).not.toHaveBeenCalled(); + expect(fs.readFileSync(file, "utf-8")).toBe(out); + writes.mockRestore(); + }); +}); + describe("registerPluginInServer", () => { + it("extends an existing import from the same module, idempotently", () => { + const dir = makeTempDir(); + const file = writeServer(dir, "server.ts", "server()"); + registerPluginInServer(dir, "@databricks/appkit", "analytics"); + registerPluginInServer(dir, "@databricks/appkit", "analytics"); + const out = fs.readFileSync(file, "utf-8"); + expect(out).toContain( + `import { createApp, analytics } from "@databricks/appkit";`, + ); + expect(out.match(/@databricks\/appkit"/g)).toHaveLength(1); + expect(out.match(/analytics\(\)/g)).toHaveLength(1); + }); + + it("extends the /beta import and keeps a multi-line list's layout", () => { + const dir = makeTempDir(); + const file = path.join(dir, "server.ts"); + fs.writeFileSync( + file, + `import { createApp, server } from "@databricks/appkit";\n` + + `import {\n agents,\n} from "@databricks/appkit/beta";\n\n` + + `createApp({ plugins: [server(), agents()] });\n`, + ); + registerPluginInServer(dir, "@databricks/appkit/beta", "aiSearch"); + const out = fs.readFileSync(file, "utf-8"); + expect(out).toContain( + `import {\n agents,\n aiSearch,\n} from "@databricks/appkit/beta";`, + ); + expect(out).toContain( + `import { createApp, server } from "@databricks/appkit";`, + ); + }); + + it("treats a commented `import /* x */ type` as type-only", () => { + const dir = makeTempDir(); + const file = path.join(dir, "server.ts"); + fs.writeFileSync( + file, + `import /* types used below */ type { Plugin } from "@databricks/appkit";\n` + + `import { createApp } from "@databricks/appkit";\n\n` + + `createApp({ plugins: [] });\n`, + ); + registerPluginInServer(dir, "@databricks/appkit", "analytics"); + const out = fs.readFileSync(file, "utf-8"); + expect(out).toContain( + `import /* types used below */ type { Plugin } from "@databricks/appkit";`, + ); + expect(out).toContain( + `import { createApp, analytics } from "@databricks/appkit";`, + ); + }); + + it("adds a new line rather than extending a type-only import", () => { + const dir = makeTempDir(); + const file = path.join(dir, "server.ts"); + fs.writeFileSync( + file, + `import type { PluginData } from "@databricks/appkit/beta";\n` + + `import { createApp } from "@databricks/appkit";\n\n` + + `createApp({ plugins: [] });\n`, + ); + registerPluginInServer(dir, "@databricks/appkit/beta", "agents"); + const out = fs.readFileSync(file, "utf-8"); + expect(out).toContain( + `import type { PluginData } from "@databricks/appkit/beta";`, + ); + expect(out).toContain(`import { agents } from "@databricks/appkit/beta";`); + }); + + 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 { createApp, 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. @@ -97,10 +235,11 @@ describe("registerPluginInServer", () => { expect(result.status).toBe("wired"); const written = fs.readFileSync(file, "utf-8"); - expect(written).toContain('import { hello } from "./plugins/hello";'); + // `hello` is bound by extending the same-path import; HelloPlugin stays. + expect(written).toContain( + 'import { HelloPlugin, hello } from "./plugins/hello";', + ); expect(written).toContain("hello()"); - // The pre-existing, differently-named import is left intact. - expect(written).toContain('import { HelloPlugin } from "./plugins/hello";'); }); it("does not add a conflicting import when the binding already exists", () => { diff --git a/packages/shared/src/cli/commands/registry/server-register.ts b/packages/shared/src/cli/commands/registry/server-register.ts index 327b0f4ee..0b21a51c3 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; @@ -28,6 +28,25 @@ function findServerFile(serverRoot: string): string | null { return null; } +/** + * The named-imports node (`{ a, b }`) of a value import from `module`, to + * extend with a new name instead of adding a second import line. Type-only + * imports are skipped: a value added there would be erased at compile time. + */ +function namedImportsFrom(stmts: SgNode[], module: string): SgNode | null { + for (const stmt of stmts) { + const source = stmt.field("source")?.text().slice(1, -1); + // `import type` shows as an unnamed `type` token, wherever comments sit. + const typeOnly = stmt + .children() + .some((c) => c.kind() === "type" && !c.isNamed()); + if (source !== module || typeOnly) continue; + const named = stmt.find({ rule: { kind: "named_imports" } }); + if (named) return named; + } + return null; +} + /** The `plugins: [...]` array node inside a createApp call, if present. */ function findPluginsArray(root: SgNode): SgNode | null { for (const pair of root.findAll({ rule: { kind: "pair" } })) { @@ -83,29 +102,75 @@ function importBindingNames(stmt: SgNode): Set { * standard shape (a `plugins: [...]` array literal); returns `skipped` otherwise * so the caller can fall back to printing manual instructions. Idempotent. */ +/** A plugin to register: `import { exportName } from importPath` + `exportName(callArgs)`. */ +export interface RegisterEntry { + importPath: string; + exportName: string; + /** Call arguments, e.g. `{ auth: "on-behalf-of-user" }` (CLI-built, never registry data). */ + callArgs?: string; +} + export function registerPluginInServer( serverRoot: string, importPath: string, exportName: string, + callArgs = "", ): RegisterResult { + return registerPluginsInServer(serverRoot, [ + { importPath, exportName, callArgs }, + ])[0]; +} + +/** + * Registers several plugins in the server entry, writing the file once (a + * dev server watching it restarts once). Entries apply in order; each + * result is per entry. + */ +export function registerPluginsInServer( + serverRoot: string, + entries: RegisterEntry[], +): RegisterResult[] { + const serverFile = findServerFile(serverRoot); + if (!serverFile) { + return entries.map(() => ({ + status: "skipped", + reason: "no server entry file found", + })); + } + const file = path.relative(serverRoot, serverFile); + const lang = serverFile.endsWith(".tsx") ? Lang.Tsx : Lang.TypeScript; + const original = fs.readFileSync(serverFile, "utf-8"); + let content = original; + const results = entries.map((e): RegisterResult => { + const { output, ...r } = registerInSource(content, lang, e); + if (output !== undefined) content = output; + return r.status === "skipped" ? r : { ...r, file }; + }); + if (content !== original) fs.writeFileSync(serverFile, content); + return results; +} + +/** Applies one entry to the server source; `output` is set when it changed. */ +function registerInSource( + content: string, + lang: Lang, + { importPath, exportName, callArgs = "" }: RegisterEntry, +): RegisterResult & { output?: string } { // 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" }; } - const serverFile = findServerFile(serverRoot); - if (!serverFile) { - return { status: "skipped", reason: "no server entry file found" }; - } - - const content = fs.readFileSync(serverFile, "utf-8"); - const lang = serverFile.endsWith(".tsx") ? Lang.Tsx : Lang.TypeScript; const root = parse(lang, content).root(); const arr = findPluginsArray(root); @@ -116,15 +181,14 @@ export function registerPluginInServer( }; } - const file = path.relative(serverRoot, serverFile); if (arrayElementNames(arr).has(exportName)) { - return { status: "already", file }; + return { status: "already" }; } 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). @@ -153,16 +217,29 @@ export function registerPluginInServer( importBindingNames(s).has(exportName), ); const importLine = `import { ${exportName} } from "${importPath}";`; - if (!hasBinding && importStmts.length > 0) { + const named = hasBinding ? null : namedImportsFrom(importStmts, importPath); + if (named) { + // Append after the last specifier, matching a multi-line list's layout. + const specs = named.findAll({ rule: { kind: "import_specifier" } }); + const lastSpec = specs[specs.length - 1]; + if (!lastSpec) { + edits.push(named.replace(`{ ${exportName} }`)); + } else { + const startIdx = lastSpec.range().start.index; + const lineStart = content.lastIndexOf("\n", startIdx - 1); + const indent = content.slice(lineStart + 1, startIdx); + const multiline = named.text().includes("\n") && /^[ \t]*$/.test(indent); + const sep = multiline ? `,\n${indent}` : ", "; + edits.push(lastSpec.replace(`${lastSpec.text()}${sep}${exportName}`)); + } + } else if (!hasBinding && importStmts.length > 0) { const last = importStmts[importStmts.length - 1]; edits.push(last.replace(`${last.text()}\n${importLine}`)); } let output = root.commitEdits(edits); - if (!hasBinding && importStmts.length === 0) { + if (!hasBinding && !named && importStmts.length === 0) { output = `${importLine}\n${output}`; } - fs.writeFileSync(serverFile, output); - - return { status: "wired", file }; + return { status: "wired", output }; } 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[]`, diff --git a/tools/template-smoke-runner.ts b/tools/template-smoke-runner.ts index 18541702c..8dc8add04 100644 --- a/tools/template-smoke-runner.ts +++ b/tools/template-smoke-runner.ts @@ -138,7 +138,17 @@ export async function runTemplateSmoke( writeFileSync(lockPath, committedLock); run("npm", ["ci", "--no-audit", "--no-fund"]); } else { - run("pnpm", ["install", "--frozen-lockfile"]); + // A fresh store: pnpm 11 drops a package's bundledDependencies when it + // reuses that package from a store, and appkit ships patched SDKs that + // way. CI restores a cached store, so a shared one breaks every run + // after the first. Users with a warm store are affected too; that needs + // a packaging fix, not this test. + run("pnpm", [ + "install", + "--frozen-lockfile", + "--store-dir", + join(guardDir, "pnpm-store"), + ]); const host: Record = { os: process.platform, cpu: process.arch,