From 80ca5a3d7058c25a59b2857c8849fc18772e520a Mon Sep 17 00:00:00 2001 From: Thomas Bouldin Date: Mon, 31 Aug 2026 14:27:10 -0700 Subject: [PATCH 01/15] feat(ext:migrate): export configs into functions env and invoke kit init ### Description Integrates configuration exporting and kit scaffolding into `ext:migrate`: - Exports Extension parameters and system parameters into Functions environment variables (`functionsEnvFromInstance`), mapping `EXT_MIGRATED_SYSTEM_LOCATION` to `DEFAULT_FUNCTION_REGION`. - Adds `migrateSecrets` helper to transfer extension secrets to Functions management (with early return if no secrets exist and IAM administrator guidance on permission denial). - Passes exported environment variables into `installKitOrInstance` to seed `.env.` during kit installation. - Refactors kit install helper APIs to accept intrinsic arrays (`string[]`) instead of Sets per codebase convention (intrinsics only in module APIs). - Updates package specifier and kit name parsing to support versions (e.g. `@1.2.3`) and tags (e.g. `@next`). ### Scenarios Tested - Unit tests in `src/extensions/export.spec.ts`, `src/extensions/migrate.spec.ts`, and `src/functions/kits/install.spec.ts`. - Tested `ext:migrate` command execution flow with exported env seeding and secret migration. - Tested `migrateSecrets` with instances containing no secrets, active secrets, and permission errors. - Tested package specifier parsing with scoped/unscoped packages, versions, and tags. ### Sample Commands - `firebase ext:migrate` - `firebase ext:migrate --extension firestore-send-email` - `firebase ext:migrate --package @firebase-function-kits/firestore-send-email@next` --- CHANGELOG.md | 1 + src/commands/ext-migrate.ts | 48 +++- src/extensions/export.spec.ts | 220 ++++++++++++++- src/extensions/export.ts | 13 +- src/extensions/extensionsHelper.ts | 94 ++++++- src/extensions/migrate.spec.ts | 157 ++++++++++- src/extensions/migrate.ts | 45 +++- src/extensions/secretsUtils.ts | 3 + src/extensions/types.ts | 2 +- src/functions/kits/install.spec.ts | 250 +++++++++++++++--- src/functions/kits/install.ts | 119 ++++++--- .../typescript/index-kit-migration.ts | 2 +- 12 files changed, 845 insertions(+), 109 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fdef2c52643..3650dcf365b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,2 +1,3 @@ +- [Added] Support exporting Extension parameters and initializing Function Kits in `firebase ext:migrate`. - [Added] Add -f, --force option to `firebase ext:migrate`. - [Fixed] Fix parameter type preservation and optional system parameter handling during extension updates in `firebase ext:migrate`. diff --git a/src/commands/ext-migrate.ts b/src/commands/ext-migrate.ts index 63d18493b0d..fc350f85b0d 100644 --- a/src/commands/ext-migrate.ts +++ b/src/commands/ext-migrate.ts @@ -2,13 +2,20 @@ import * as clc from "colorette"; import { checkMinRequiredVersion } from "../checkMinRequiredVersion"; import { Command } from "../command"; import { needProjectId } from "../projectUtils"; -import { ensureExtensionsApiEnabled, logPrefix } from "../extensions/extensionsHelper"; +import { + ensureExtensionsApiEnabled, + ensureInstanceSpec, + logPrefix, +} from "../extensions/extensionsHelper"; import { requirePermissions } from "../requirePermissions"; -import { createMigrationPlan, ensureInstanceUpToDate } from "../extensions/migrate"; +import { createMigrationPlan, ensureInstanceUpToDate, migrateSecrets } from "../extensions/migrate"; +import { functionsEnvFromInstance } from "../extensions/export"; +import { installKitOrInstance } from "../functions/kits/install"; import { validateNpmPackageName } from "../functions/kits"; -import { logger } from "../logger"; import { Options } from "../options"; -import { logLabeledBullet } from "../utils"; +import { logLabeledBullet, logLabeledWarning } from "../utils"; +import { FirebaseError } from "../error"; +import { logger } from "../logger"; export interface ExtMigrateOptions extends Options { package?: string; @@ -26,6 +33,9 @@ export const command = new Command("ext:migrate") .before(ensureExtensionsApiEnabled) .before(checkMinRequiredVersion, "extMinVersion") .action(async (options: ExtMigrateOptions) => { + if (!options.config) { + throw new FirebaseError("Not in a Firebase project directory (firebase.json not found)."); + } const projectId = needProjectId(options); if (options.package) { validateNpmPackageName(options.package); @@ -45,6 +55,36 @@ export const command = new Command("ext:migrate") plan.instance = await ensureInstanceUpToDate(projectId, plan.instance, options); + if (plan.instance.state !== "ACTIVE") { + logLabeledWarning( + logPrefix, + `Extension instance ${clc.bold(plan.instanceId)} is in state ${plan.instance.state}. Migration may not function as expected.`, + ); + } + + plan.instance = await ensureInstanceSpec(plan.instance); + + const exportedEnvs = functionsEnvFromInstance(plan.instance); + + await migrateSecrets(plan.instance); + + logLabeledBullet( + logPrefix, + `Installing kit ${clc.bold(plan.kitPackage)} for instance ${clc.bold(plan.instanceId)}...`, + ); + + await installKitOrInstance({ + ...options, + config: options.config, + package: plan.kitPackage, + template: "migration", + seedEnv: { + projectId, + envs: exportedEnvs, + }, + skipReport: true, + }); + logger.info("TODO: Draw the rest of the owl"); return plan; }); diff --git a/src/extensions/export.spec.ts b/src/extensions/export.spec.ts index bc23bfd5e27..64f1bf2882a 100644 --- a/src/extensions/export.spec.ts +++ b/src/extensions/export.spec.ts @@ -1,9 +1,19 @@ import { expect } from "chai"; +import * as sinon from "sinon"; -import { functionsEnvFromInstance, parameterizeProject, setSecretParamsToLatest } from "./export"; +import { + functionsEnvFromInstance, + parameterizeProject, + setSecretParamsToLatest, + ejectSecretsFromInstance, +} from "./export"; +import { ensureInstanceSpec } from "./extensionsHelper"; import { DeploymentInstanceSpec } from "../deploy/extensions/planner"; import { ParamType } from "./types"; import { ExtensionInstance } from "./types"; +import * as publisherApi from "./publisherApi"; +import * as secretsModule from "../deploy/extensions/secrets"; +import { FirebaseError } from "../error"; describe("ext:export helpers", () => { describe("parameterizeProject", () => { @@ -296,6 +306,41 @@ describe("functionsEnvFromInstance", () => { }); }); + it("system params location should map to DEFAULT_FUNCTION_REGION", () => { + const instance: ExtensionInstance = { + name: "", + createTime: "", + updateTime: "", + state: "ACTIVE", + serviceAccountEmail: "", + config: { + name: "", + createTime: "", + params: {}, + systemParams: { + "firebaseextensions.v1beta.function/location": "us-central1", + }, + source: { + name: "", + state: "ACTIVE", + packageUri: "", + hash: "", + spec: { + name: "", + version: "1", + resources: [], + params: [], + systemParams: [], + }, + }, + }, + }; + const output = functionsEnvFromInstance(instance); + expect(output).to.deep.equal({ + DEFAULT_FUNCTION_REGION: "us-central1", + }); + }); + it("eventarc special cases", () => { const instance: ExtensionInstance = { name: "", @@ -332,3 +377,176 @@ describe("functionsEnvFromInstance", () => { }); }); }); + +describe("ensureInstanceSpec", () => { + let getExtensionVersionStub: sinon.SinonStub; + + beforeEach(() => { + getExtensionVersionStub = sinon.stub(publisherApi, "getExtensionVersion"); + }); + + afterEach(() => { + sinon.restore(); + }); + + it("should return instance as is if spec already exists", async () => { + const instance: ExtensionInstance = { + name: "projects/p/instances/i", + createTime: "", + updateTime: "", + state: "ACTIVE", + serviceAccountEmail: "", + config: { + name: "projects/p/instances/i/configurations/1", + createTime: "", + params: {}, + systemParams: {}, + source: { + name: "sources/1", + state: "ACTIVE", + packageUri: "", + hash: "", + spec: { + name: "my-ext", + version: "0.1.0", + resources: [], + params: [], + systemParams: [], + }, + }, + }, + }; + + const result = await ensureInstanceSpec(instance); + expect(result.config.source?.spec?.name).to.equal("my-ext"); + expect(getExtensionVersionStub).to.not.have.been.called; + }); + + it("should fetch spec on demand if missing", async () => { + const instance = { + name: "projects/p/instances/i", + createTime: "", + updateTime: "", + state: "ACTIVE", + serviceAccountEmail: "", + config: { + name: "projects/p/instances/i/configurations/1", + createTime: "", + extensionRef: "firebase/storage-resize-images", + extensionVersion: "0.1.30", + params: {}, + systemParams: {}, + }, + } satisfies ExtensionInstance; + + getExtensionVersionStub.withArgs("firebase/storage-resize-images@0.1.30").resolves({ + spec: { + name: "storage-resize-images", + version: "0.1.30", + resources: [], + params: [], + systemParams: [], + }, + }); + + const result = await ensureInstanceSpec(instance); + expect(result.config.source?.spec?.name).to.equal("storage-resize-images"); + expect(getExtensionVersionStub).to.have.been.calledOnce; + }); +}); + +describe("ejectSecretsFromInstance", () => { + let transferSecretToKitsStub: sinon.SinonStub; + + beforeEach(() => { + transferSecretToKitsStub = sinon.stub(secretsModule, "transferSecretToKits"); + }); + + afterEach(() => { + sinon.restore(); + }); + + it("should eject secrets successfully", async () => { + const instance: ExtensionInstance = { + name: "projects/my-proj/instances/my-inst", + createTime: "", + updateTime: "", + state: "ACTIVE", + serviceAccountEmail: "", + config: { + name: "projects/my-proj/instances/my-inst/configurations/1", + createTime: "", + params: { + API_KEY: "projects/my-proj/secrets/API_KEY/versions/1", + }, + systemParams: {}, + source: { + name: "sources/1", + state: "ACTIVE", + packageUri: "", + hash: "", + spec: { + name: "my-ext", + version: "1.0.0", + resources: [], + params: [ + { + param: "API_KEY", + label: "API Key", + type: ParamType.SECRET, + }, + ], + systemParams: [], + }, + }, + }, + }; + + transferSecretToKitsStub.resolves(); + const changed = await ejectSecretsFromInstance(instance); + expect(changed).to.deep.equal(["my-proj/API_KEY"]); + expect(transferSecretToKitsStub).to.have.been.calledWith("my-proj", "API_KEY"); + }); + + it("should propagate errors thrown by transferSecretToKits", async () => { + const instance: ExtensionInstance = { + name: "projects/my-proj/instances/my-inst", + createTime: "", + updateTime: "", + state: "ACTIVE", + serviceAccountEmail: "", + config: { + name: "projects/my-proj/instances/my-inst/configurations/1", + createTime: "", + params: { + API_KEY: "projects/my-proj/secrets/API_KEY/versions/1", + }, + systemParams: {}, + source: { + name: "sources/1", + state: "ACTIVE", + packageUri: "", + hash: "", + spec: { + name: "my-ext", + version: "1.0.0", + resources: [], + params: [ + { + param: "API_KEY", + label: "API Key", + type: ParamType.SECRET, + }, + ], + systemParams: [], + }, + }, + }, + }; + + const permError = new FirebaseError("Forbidden", { status: 403 }); + transferSecretToKitsStub.rejects(permError); + + await expect(ejectSecretsFromInstance(instance)).to.be.rejectedWith(permError); + }); +}); diff --git a/src/extensions/export.ts b/src/extensions/export.ts index 6c72c602669..fa9d1cc7f54 100644 --- a/src/extensions/export.ts +++ b/src/extensions/export.ts @@ -105,8 +105,8 @@ function displaySpecs(specs: DeploymentInstanceSpec[]): void { export function functionsEnvFromInstance(instance: ExtensionInstance): Record { const liveParams = instance.config?.params || {}; const liveSystemParams = instance.config?.systemParams || {}; - const specParams = instance.config?.source?.spec?.params || {}; - const specSystemParams = instance.config?.source?.spec?.systemParams || {}; + const specParams = instance.config?.source?.spec?.params || []; + const specSystemParams = instance.config?.source?.spec?.systemParams || []; const envs: Record = {}; @@ -132,15 +132,18 @@ export function functionsEnvFromInstance(instance: ExtensionInstance): Record; * Substitutes any secret parameters with the correct format * @param projectNumber The project number we are installing into * @param params the full list of params to check for substitution. - * @returns The substituted list of params + * @return The substituted list of params */ export async function substituteSecretParams( projectNumber: string, @@ -195,10 +202,9 @@ export async function substituteSecretParams( const newParams: Record = {}; for await (const [key, value] of Object.entries(params)) { if (typeof value !== "string") { - newParams[key] = - `projects/${projectNumber}/secrets/${(value as SecretParam).name}/versions/latest`; + newParams[key] = `projects/${projectNumber}/secrets/${value.name}/versions/latest`; } else { - newParams[key] = value as string; + newParams[key] = value; } } return newParams; @@ -438,7 +444,6 @@ export async function promptForValidRepoURI(): Promise { /** * Prompts for an extension root. - * * @param defaultRoot the default extension root */ export async function promptForExtensionRoot(defaultRoot: string): Promise { @@ -451,7 +456,6 @@ export async function promptForExtensionRoot(defaultRoot: string): Promise { const projectId = getProjectId(options); if (!projectId) { @@ -517,6 +524,9 @@ export async function checkExtensionsApiEnabled(options: any): Promise return await check(projectId, extensionsOrigin(), "extensions", options.markdown); } +/** + * + */ export async function ensureExtensionsApiEnabled(options: any): Promise { const projectId = getProjectId(options); if (!projectId) { @@ -525,6 +535,9 @@ export async function ensureExtensionsApiEnabled(options: any): Promise { return await ensure(projectId, extensionsOrigin(), "extensions", options.markdown); } +/** + * + */ export async function ensureExtensionsPublisherApiEnabled(options: any): Promise { const projectId = getProjectId(options); if (!projectId) { @@ -549,7 +562,6 @@ async function archiveAndUploadSource(extPath: string, bucketName: string): Prom /** * Gets a list of the next version to upload by release stage. - * * @param extensionRef the ref of the extension * @param version the new version of the extension */ @@ -589,7 +601,6 @@ export async function getNextVersionByStage( /** * Validates the extension spec. - * * @param rootDirectory the directory with the extension source * @param extensionRef the ref of the extension */ @@ -617,7 +628,6 @@ async function validateExtensionSpec( /** * Validates the release notes. - * * @param rootDirectory the directory with the extension source * @param newVersion the new extension version */ @@ -646,7 +656,6 @@ function validateReleaseNotes(rootDirectory: string, newVersion: string, extensi /** * Validates the extension version. - * * @param extensionRef the ref of the extension * @param newVersion the new extension version * @param latestVersion the latest extension version @@ -712,7 +721,7 @@ function displayExtensionHeader( if (extension) { let source = "Local source"; if (extension.repoUri) { - const uri = new URL(extension.repoUri!); + const uri = new URL(extension.repoUri); uri.pathname = path.join(uri.pathname, extensionRoot ?? ""); source = `${uri.toString()} (use --repo and --root to modify)`; } @@ -1030,6 +1039,9 @@ export async function uploadExtensionVersionFromLocalSource(args: { return res; } +/** + * + */ export function getMissingPublisherError(publisherId: string): FirebaseError { return new FirebaseError( `Couldn't find publisher ID '${clc.bold( @@ -1187,10 +1199,16 @@ export async function instanceIdExists(projectId: string, instanceId: string): P return true; } +/** + * + */ export function isUrlPath(extInstallPath: string): boolean { return extInstallPath.startsWith("https:"); } +/** + * + */ export function isLocalPath(extInstallPath: string): boolean { const trimmedPath = extInstallPath.trim(); return ( @@ -1208,6 +1226,9 @@ export function isLocalPath(extInstallPath: string): boolean { ); } +/** + * + */ export function isLocalOrURLPath(extInstallPath: string): boolean { return isLocalPath(extInstallPath) || isUrlPath(extInstallPath); } @@ -1245,6 +1266,9 @@ export function getSourceOrigin(sourceOrVersion: string): SourceOrigin { ); } +/** + * + */ export async function diagnoseAndFixProject(options: any): Promise { const projectId = getProjectId(options); if (!projectId) { @@ -1255,3 +1279,49 @@ export async function diagnoseAndFixProject(options: any): Promise { throw new FirebaseError("Unable to proceed until all issues are resolved."); } } + +/** + * Ensures that the extension instance has its spec loaded, fetching it on demand if missing. + */ +export async function ensureInstanceSpec(instance: ExtensionInstance): Promise { + if (instance.config?.source?.spec) { + return instance; + } + const ref = instance.config?.extensionRef; + const version = instance.config?.extensionVersion; + const versionRef = ref + ? ref.includes("@") + ? ref + : version + ? `${ref}@${version}` + : ref + : undefined; + if (versionRef) { + try { + const extVersion = await getExtensionVersion(versionRef); + if (extVersion?.spec) { + instance.config = instance.config || { + name: "", + createTime: "", + params: {}, + systemParams: {}, + }; + instance.config.source = { + ...(instance.config.source || { + name: "", + state: "ACTIVE", + packageUri: "", + hash: "", + }), + spec: extVersion.spec, + }; + } + } catch (err: unknown) { + logger.debug( + `[ensureInstanceSpec] Could not fetch extension version for ${versionRef}:`, + err, + ); + } + } + return instance; +} diff --git a/src/extensions/migrate.spec.ts b/src/extensions/migrate.spec.ts index 0924f64b2f8..04834c06c31 100644 --- a/src/extensions/migrate.spec.ts +++ b/src/extensions/migrate.spec.ts @@ -8,7 +8,12 @@ import * as extensionsApi from "./extensionsApi"; import * as migrateModule from "./migrate"; import * as paramHelper from "./paramHelper"; import * as updateHelper from "./updateHelper"; -import { ExtensionInstance } from "./types"; +import * as exportModule from "./export"; +import * as kitInstallModule from "../functions/kits/install"; +import * as extensionsHelper from "./extensionsHelper"; +import { command as extMigrateCommand, ExtMigrateOptions } from "../commands/ext-migrate"; +import { Config } from "../config"; +import { ExtensionInstance, ParamType } from "./types"; describe("ext:migrate core logic (Unique Veneer)", () => { let sandbox: sinon.SinonSandbox; @@ -453,4 +458,154 @@ describe("ext:migrate core logic (Unique Veneer)", () => { expect(result).to.equal(invalidRefInstance); }); }); + + describe("migrateSecrets", () => { + it("should return empty array without logging if instance has no secrets", async () => { + const res = await migrateModule.migrateSecrets(mockInstance1); + expect(res).to.deep.equal([]); + expect(logger.info).to.not.have.been.called; + }); + + it("should eject secrets and log when instance has secrets", async () => { + const instanceWithSecret: ExtensionInstance = { + ...mockInstance1, + config: { + ...mockInstance1.config, + source: { + name: "sources/1", + state: "ACTIVE", + packageUri: "https://example.com/package.zip", + hash: "hash123", + spec: { + name: "ext", + version: "1.0.0", + resources: [], + params: [ + { + param: "API_KEY", + label: "API Key", + type: ParamType.SECRET, + }, + ], + systemParams: [], + }, + }, + }, + }; + + sandbox.stub(exportModule, "ejectSecretsFromInstance").resolves(["test-project/API_KEY"]); + + const res = await migrateModule.migrateSecrets(instanceWithSecret); + expect(res).to.deep.equal(["test-project/API_KEY"]); + }); + + it("should throw informative error on IAM permission error (403)", async () => { + const instanceWithSecret: ExtensionInstance = { + ...mockInstance1, + config: { + ...mockInstance1.config, + source: { + name: "sources/1", + state: "ACTIVE", + packageUri: "https://example.com/package.zip", + hash: "hash123", + spec: { + name: "ext", + version: "1.0.0", + resources: [], + params: [ + { + param: "API_KEY", + label: "API Key", + type: ParamType.SECRET, + }, + ], + systemParams: [], + }, + }, + }, + }; + + const permError = new FirebaseError("Forbidden", { status: 403 }); + sandbox.stub(exportModule, "ejectSecretsFromInstance").rejects(permError); + + await expect(migrateModule.migrateSecrets(instanceWithSecret)).to.be.rejectedWith( + FirebaseError, + "You do not have permissions to transfer secrets from Extensions to Functions. Please ask an IAM administrator to run the migration", + ); + }); + }); + + describe("ext:migrate command action", () => { + let installKitOrInstanceStub: sinon.SinonStub; + let migrateSecretsStub: sinon.SinonStub; + let functionsEnvStub: sinon.SinonStub; + let ensureSpecStub: sinon.SinonStub; + + beforeEach(() => { + (extMigrateCommand as unknown as { befores: unknown[] }).befores = []; + sandbox.stub(extMigrateCommand, "prepare").resolves(); + sandbox.stub(extensionsApi, "listInstances").resolves([mockInstance1]); + sandbox.stub(migrateModule, "ensureInstanceUpToDate").resolves(mockInstance1); + installKitOrInstanceStub = sandbox.stub(kitInstallModule, "installKitOrInstance").resolves({ + action: "installedKit", + kitId: "firestore-send-email", + instanceId: "email-1", + }); + migrateSecretsStub = sandbox + .stub(migrateModule, "migrateSecrets") + .resolves(["test-project/SECRET1"]); + functionsEnvStub = sandbox.stub(exportModule, "functionsEnvFromInstance").returns({ + PARAM_A: "val_a", + }); + ensureSpecStub = sandbox.stub(extensionsHelper, "ensureInstanceSpec").resolves(mockInstance1); + }); + + it("should throw if options.config is not provided", async () => { + await expect( + extMigrateCommand.runner()({ + project: "test-project", + projectId: "test-project", + } as unknown as ExtMigrateOptions), + ).to.be.rejectedWith( + FirebaseError, + "Not in a Firebase project directory (firebase.json not found).", + ); + }); + + it("should export envs, migrate secrets, call installKitOrInstance, and return plan", async () => { + const mockConfig = { + projectDir: "/mock/project", + src: { functions: [] }, + } as unknown as Config; + + const res = (await extMigrateCommand.runner()({ + project: "test-project", + projectId: "test-project", + extInstance: "email-1", + config: mockConfig, + nonInteractive: true, + } as unknown as ExtMigrateOptions)) as migrateModule.ExtensionMigrationPlan; + + expect(ensureSpecStub).to.have.been.calledOnce; + expect(functionsEnvStub).to.have.been.calledOnce; + expect(migrateSecretsStub).to.have.been.calledOnce; + expect(installKitOrInstanceStub).to.have.been.calledOnceWith( + sinon.match({ + config: mockConfig, + package: "@firebase-function-kits/firestore-send-email", + template: "migration", + seedEnv: { + projectId: "test-project", + envs: { + PARAM_A: "val_a", + }, + }, + skipReport: true, + }), + ); + + expect(res.instanceId).to.equal("email-1"); + }); + }); }); diff --git a/src/extensions/migrate.ts b/src/extensions/migrate.ts index e02981db614..c07a3dfb22a 100644 --- a/src/extensions/migrate.ts +++ b/src/extensions/migrate.ts @@ -1,17 +1,18 @@ import * as clc from "colorette"; import * as Table from "cli-table3"; -import { FirebaseError } from "../error"; +import { FirebaseError, getErrStatus } from "../error"; import { logger } from "../logger"; -import { last, logLabeledBullet, logLabeledWarning } from "../utils"; +import { last, logLabeledBullet, logLabeledSuccess, logLabeledWarning } from "../utils"; import { logPrefix } from "./extensionsHelper"; import { confirm, select } from "../prompt"; import * as extensionsApi from "./extensionsApi"; import * as refs from "./refs"; import * as paramHelper from "./paramHelper"; import * as updateHelper from "./updateHelper"; -import { ExtensionInstance, ExtensionSpec } from "./types"; +import { ExtensionInstance, ExtensionSpec, ParamType } from "./types"; import * as replacements from "./replacements.json"; +import { ejectSecretsFromInstance } from "./export"; export interface MigrateOptions { package?: string; @@ -432,3 +433,41 @@ export async function ensureInstanceUpToDate( const updatedInstance = await extensionsApi.getInstance(projectId, instanceId); return updatedInstance ?? instance; } + +/** + * Migrates secrets for an extension instance to Functions management if secrets are present. + * If no secrets are defined in the extension spec, exits early without logging. + */ +export async function migrateSecrets(instance: ExtensionInstance): Promise { + const hasSecrets = (instance.config?.source?.spec?.params ?? []).some( + (p) => p.type === ParamType.SECRET, + ); + if (!hasSecrets) { + return []; + } + + const instanceId = instance.name.split("/").pop() || ""; + logLabeledBullet( + logPrefix, + `Transferring secrets for instance ${clc.bold(instanceId)} to Functions management...`, + ); + + try { + const secretsChanged = await ejectSecretsFromInstance(instance); + if (secretsChanged.length > 0) { + logLabeledSuccess( + logPrefix, + `Successfully transferred secrets to Functions management: ${secretsChanged.join(", ")}`, + ); + } + return secretsChanged; + } catch (err: unknown) { + if (getErrStatus(err) === 403) { + throw new FirebaseError( + "You do not have permissions to transfer secrets from Extensions to Functions. Please ask an IAM administrator to run the migration", + { original: err instanceof Error ? err : undefined, exit: 1 }, + ); + } + throw err; + } +} diff --git a/src/extensions/secretsUtils.ts b/src/extensions/secretsUtils.ts index 0cbfcc75dc1..feda9fb68e6 100644 --- a/src/extensions/secretsUtils.ts +++ b/src/extensions/secretsUtils.ts @@ -33,6 +33,9 @@ export async function grantFirexServiceAgentSecretAdminRole( } export async function getManagedSecrets(instance: ExtensionInstance): Promise { + if (!instance.config.source?.spec) { + return []; + } return ( await Promise.all( getActiveSecrets(instance.config.source.spec, instance.config.params).map( diff --git a/src/extensions/types.ts b/src/extensions/types.ts index 007d1350f8e..7d79f6e234c 100644 --- a/src/extensions/types.ts +++ b/src/extensions/types.ts @@ -99,7 +99,7 @@ export const isExtensionInstance = (value: unknown): value is ExtensionInstance export interface ExtensionConfig { name: string; createTime: string; - source: ExtensionSource; + source?: ExtensionSource; params: Record; systemParams: Record; populatedPostinstallContent?: string; diff --git a/src/functions/kits/install.spec.ts b/src/functions/kits/install.spec.ts index 71d172645a4..e8cf5852fb1 100644 --- a/src/functions/kits/install.spec.ts +++ b/src/functions/kits/install.spec.ts @@ -84,6 +84,12 @@ describe("functions/kits/install", () => { expect(() => validateNpmPackageName("kit_123.v1")).to.not.throw(); }); + it("should accept valid unscoped package specifiers with version or tag", () => { + expect(() => validateNpmPackageName("my-kit@1.2.3")).to.not.throw(); + expect(() => validateNpmPackageName("my-kit@next")).to.not.throw(); + expect(() => validateNpmPackageName("my-kit@^2.0.0")).to.not.throw(); + }); + it("should accept valid scoped package names with exactly one slash", () => { expect(() => validateNpmPackageName("@firebase-function-kits/firestore-bigquery-export"), @@ -91,6 +97,21 @@ describe("functions/kits/install", () => { expect(() => validateNpmPackageName("@invertase/example-kit")).to.not.throw(); }); + it("should accept valid scoped package specifiers with version or tag", () => { + expect(() => + validateNpmPackageName("@firebase-function-kits/firestore-bigquery-export@1.0.0"), + ).to.not.throw(); + expect(() => + validateNpmPackageName("@firebase-function-kits/firestore-bigquery-export@1.0.0-rc.1"), + ).to.not.throw(); + expect(() => + validateNpmPackageName("@firebase-function-kits/firestore-bigquery-export@latest"), + ).to.not.throw(); + expect(() => + validateNpmPackageName("@firebase-function-kits/firestore-bigquery-export@next"), + ).to.not.throw(); + }); + it("should reject package names with multiple slashes", () => { expect(() => validateNpmPackageName("@scope/pkg/extra")).to.throw( FirebaseError, @@ -115,6 +136,14 @@ describe("functions/kits/install", () => { FirebaseError, /Invalid NPM package name/, ); + expect(() => validateNpmPackageName("my-kit@")).to.throw( + FirebaseError, + /Invalid NPM package name/, + ); + expect(() => validateNpmPackageName("@scope/my-kit@")).to.throw( + FirebaseError, + /Invalid NPM package name/, + ); expect(() => validateNpmPackageName("a".repeat(215))).to.throw( FirebaseError, /Invalid NPM package name/, @@ -124,20 +153,20 @@ describe("functions/kits/install", () => { describe("generateUniqueId", () => { it("should return base ID when it is not in existing IDs", () => { - const existing = new Set(["other-kit"]); + const existing = ["other-kit"]; expect(generateUniqueId("my-kit", existing)).to.equal("my-kit"); }); it("should append random 4-character hex suffix when base ID collides", () => { - const existing = new Set(["my-kit"]); + const existing = ["my-kit"]; const res = generateUniqueId("my-kit", existing); expect(res).to.match(/^my-kit-[a-f0-9]{4}$/); - expect(existing.has(res)).to.be.false; + expect(existing.includes(res)).to.be.false; }); it("should truncate long base IDs to ensure total length <= 40", () => { const longBase = "a".repeat(40); - const existing = new Set([longBase]); + const existing = [longBase]; const res = generateUniqueId(longBase, existing); expect(res.length).to.be.at.most(40); expect(res).to.match(/^a{35}-[a-f0-9]{4}$/); @@ -214,11 +243,30 @@ describe("functions/kits/install", () => { expect(sanitizePackageNameToKitName("@foo/bar")).to.equal("bar"); }); + it("should extract kit name from scoped package specifier with version or tag", () => { + expect( + sanitizePackageNameToKitName("@firebase-function-kits/firestore-bigquery-export@1.0.0"), + ).to.equal("firestore-bigquery-export"); + expect( + sanitizePackageNameToKitName("@firebase-function-kits/firestore-bigquery-export@next"), + ).to.equal("firestore-bigquery-export"); + expect( + sanitizePackageNameToKitName( + "@firebase-function-kits/firestore-bigquery-export@1.0.0-rc.1", + ), + ).to.equal("firestore-bigquery-export"); + }); + it("should sanitize non-scoped package name", () => { expect(sanitizePackageNameToKitName("my-kit")).to.equal("my-kit"); expect(sanitizePackageNameToKitName("My_Kit!")).to.equal("my_kit"); }); + it("should sanitize non-scoped package specifier with version or tag", () => { + expect(sanitizePackageNameToKitName("my-kit@1.2.3")).to.equal("my-kit"); + expect(sanitizePackageNameToKitName("my-kit@next")).to.equal("my-kit"); + }); + it("should truncate long names to 40 characters", () => { const longName = "@scope/" + "a".repeat(50); expect(sanitizePackageNameToKitName(longName)).to.equal("a".repeat(40)); @@ -228,6 +276,10 @@ describe("functions/kits/install", () => { describe("isThirdPartyPackage", () => { it("should return false for packages under @firebase-function-kits scope", () => { expect(isThirdPartyPackage("@firebase-function-kits/firestore-bigquery-export")).to.be.false; + expect(isThirdPartyPackage("@firebase-function-kits/firestore-bigquery-export@1.0.0")).to.be + .false; + expect(isThirdPartyPackage("@firebase-function-kits/firestore-bigquery-export@next")).to.be + .false; }); it("should return true for packages outside @firebase-function-kits scope", () => { @@ -235,6 +287,8 @@ describe("functions/kits/install", () => { expect(isThirdPartyPackage("@firebase-function-kits-fake/foo")).to.be.true; expect(isThirdPartyPackage("@other-scope/my-kit")).to.be.true; expect(isThirdPartyPackage("third-party-kit")).to.be.true; + expect(isThirdPartyPackage("third-party-kit@1.2.3")).to.be.true; + expect(isThirdPartyPackage("third-party-kit@next")).to.be.true; }); }); @@ -310,18 +364,18 @@ describe("functions/kits/install", () => { }); describe("extractExistingFunctionsInfo", () => { - it("should return empty sets when configFunctions is undefined or empty", () => { + it("should return empty arrays when configFunctions is undefined or empty", () => { const resUndefined = extractExistingFunctionsInfo(undefined); expect(resUndefined.existingFunctions).to.deep.equal([]); - expect(resUndefined.existingKitIds.size).to.equal(0); - expect(resUndefined.existingCodebases.size).to.equal(0); - expect(resUndefined.existingInstanceIds.size).to.equal(0); + expect(resUndefined.existingKitIds).to.deep.equal([]); + expect(resUndefined.existingCodebases).to.deep.equal([]); + expect(resUndefined.existingInstanceIds).to.deep.equal([]); const resEmpty = extractExistingFunctionsInfo([]); expect(resEmpty.existingFunctions).to.deep.equal([]); - expect(resEmpty.existingKitIds.size).to.equal(0); - expect(resEmpty.existingCodebases.size).to.equal(0); - expect(resEmpty.existingInstanceIds.size).to.equal(0); + expect(resEmpty.existingKitIds).to.deep.equal([]); + expect(resEmpty.existingCodebases).to.deep.equal([]); + expect(resEmpty.existingInstanceIds).to.deep.equal([]); }); it("should extract kit IDs, instance IDs, and codebases correctly", () => { @@ -341,10 +395,10 @@ describe("functions/kits/install", () => { ]; const res = extractExistingFunctionsInfo(functionsConfig); - expect(res.existingCodebases.has("my-codebase")).to.be.true; - expect(res.existingKitIds.has("my-kit")).to.be.true; - expect(res.existingInstanceIds.has("inst-1")).to.be.true; - expect(res.existingInstanceIds.has("inst-2")).to.be.true; + expect(res.existingCodebases).to.include("my-codebase"); + expect(res.existingKitIds).to.include("my-kit"); + expect(res.existingInstanceIds).to.include("inst-1"); + expect(res.existingInstanceIds).to.include("inst-2"); }); }); @@ -1056,8 +1110,8 @@ describe("functions/kits/install", () => { it("should return custom instance ID directly if provided and valid", async () => { const res = await promptKitInstanceId( "my-kit", - new Set(["other-inst"]), - new Set(["codebase1"]), + ["other-inst"], + ["codebase1"], false, "valid-custom-inst", ); @@ -1066,50 +1120,38 @@ describe("functions/kits/install", () => { it("should throw if custom instance ID collides with existing instances", async () => { await expect( - promptKitInstanceId( - "my-kit", - new Set(["existing-inst"]), - new Set(), - false, - "existing-inst", - ), + promptKitInstanceId("my-kit", ["existing-inst"], [], false, "existing-inst"), ).to.be.rejectedWith(FirebaseError, /must be unique across all kits/); }); it("should throw if custom instance ID collides with codebase name", async () => { await expect( - promptKitInstanceId( - "my-kit", - new Set(), - new Set(["existing-codebase"]), - false, - "existing-codebase", - ), + promptKitInstanceId("my-kit", [], ["existing-codebase"], false, "existing-codebase"), ).to.be.rejectedWith(FirebaseError, /must be mutually exclusive/); }); it("should prompt user when custom instance ID is not provided", async () => { sinon.stub(prompt, "input").resolves("prompted-inst"); - const res = await promptKitInstanceId("my-kit", new Set(), new Set()); + const res = await promptKitInstanceId("my-kit", [], []); expect(res).to.equal("prompted-inst"); }); }); describe("promptKitId", () => { it("should return custom kit ID directly if provided and valid", async () => { - const res = await promptKitId("my-pkg", new Set(["other-kit"]), false, "custom-kit-id"); + const res = await promptKitId("my-pkg", ["other-kit"], false, "custom-kit-id"); expect(res).to.equal("custom-kit-id"); }); it("should throw if custom kit ID collides with existing kit IDs", async () => { await expect( - promptKitId("my-pkg", new Set(["existing-kit"]), false, "existing-kit"), + promptKitId("my-pkg", ["existing-kit"], false, "existing-kit"), ).to.be.rejectedWith(FirebaseError, /functions.kit must be unique/); }); it("should prompt user when custom kit ID is not provided", async () => { sinon.stub(prompt, "input").resolves("prompted-kit"); - const res = await promptKitId("my-pkg", new Set()); + const res = await promptKitId("my-pkg", []); expect(res).to.equal("prompted-kit"); }); }); @@ -1586,9 +1628,9 @@ describe("functions/kits/install", () => { existingKit, { existingFunctions: [existingKit], - existingKitIds: new Set(["firestore-bigquery-export"]), - existingCodebases: new Set(), - existingInstanceIds: new Set(["inst1"]), + existingKitIds: ["firestore-bigquery-export"], + existingCodebases: [], + existingInstanceIds: ["inst1"], }, ); @@ -1629,9 +1671,9 @@ describe("functions/kits/install", () => { existingKit, { existingFunctions: [existingKit], - existingKitIds: new Set(["firestore-bigquery-export"]), - existingCodebases: new Set(), - existingInstanceIds: new Set(["inst1"]), + existingKitIds: ["firestore-bigquery-export"], + existingCodebases: [], + existingInstanceIds: ["inst1"], }, ); @@ -1641,6 +1683,111 @@ describe("functions/kits/install", () => { instanceId: "inst1", }); }); + + it("should seed env for existing instance when seedEnv is provided", async () => { + const existingKit: ValidatedKitSingle = { + kit: "firestore-bigquery-export", + sourcePackage: { name: "@firebase-function-kits/firestore-bigquery-export" }, + source: "function-kits/firestore-bigquery-export/source", + instances: { + inst1: "function-kits/firestore-bigquery-export/config-inst1", + }, + }; + const mockConfig = { + projectDir: "/mock/project", + src: { functions: [existingKit] }, + path: (p: string) => path.join("/mock/project", p), + writeProjectFile: sinon.stub(), + askWriteProjectFile: sinon.stub().resolves(), + } as unknown as Config; + + sinon.stub(prompt, "select").resolves("addEnv"); + + const res = await addKitInstanceOrConfigureProject( + { + config: mockConfig, + project: "my-project", + seedEnv: { + projectId: "my-project", + envs: { + PARAM1: "val1", + }, + }, + }, + existingKit, + { + existingFunctions: [existingKit], + existingKitIds: ["firestore-bigquery-export"], + existingCodebases: [], + existingInstanceIds: ["inst1"], + }, + ); + + expect(res).to.deep.equal({ + action: "configuredEnv", + kitId: "firestore-bigquery-export", + instanceId: "inst1", + }); + + expect(seedKitInstanceEnvStub).to.have.been.calledOnceWith({ + configDir: path.join( + "/mock/project", + "function-kits/firestore-bigquery-export/config-inst1", + ), + functionsSource: path.join( + "/mock/project", + "function-kits/firestore-bigquery-export/source", + ), + projectDir: "/mock/project", + projectId: "my-project", + projectAlias: undefined, + envs: { + PARAM1: "val1", + }, + }); + }); + + it("should automatically select addEnv when instanceId matches an existing instance", async () => { + const existingKit: ValidatedKitSingle = { + kit: "firestore-bigquery-export", + sourcePackage: { name: "@firebase-function-kits/firestore-bigquery-export" }, + source: "function-kits/firestore-bigquery-export/source", + instances: { + inst1: "function-kits/firestore-bigquery-export/config-inst1", + }, + }; + const mockConfig = { + projectDir: "/mock/project", + src: { functions: [existingKit] }, + path: (p: string) => path.join("/mock/project", p), + writeProjectFile: sinon.stub(), + askWriteProjectFile: sinon.stub().resolves(), + } as unknown as Config; + + const selectSpy = sinon.spy(prompt, "select"); + + const res = await addKitInstanceOrConfigureProject( + { + config: mockConfig, + instanceId: "inst1", + project: "my-project", + }, + existingKit, + { + existingFunctions: [existingKit], + existingKitIds: ["firestore-bigquery-export"], + existingCodebases: [], + existingInstanceIds: ["inst1"], + }, + ); + + expect(res).to.deep.equal({ + action: "configuredEnv", + kitId: "firestore-bigquery-export", + instanceId: "inst1", + }); + expect(selectSpy).to.not.have.been.called; + }); }); describe("installKitOrInstance", () => { @@ -1863,6 +2010,27 @@ describe("functions/kits/install", () => { }); }); + it("should suppress first deploy report when skipReport is true", async () => { + const mockConfig = { + projectDir: "/mock/project", + src: { functions: [] }, + path: (p: string) => path.join("/mock/project", p), + writeProjectFile: sinon.stub(), + askWriteProjectFile: sinon.stub().resolves(), + } as unknown as Config; + + const getRuntimeDelegateStub = sinon.stub(runtimes, "getRuntimeDelegate"); + + await installKitOrInstance({ + config: mockConfig, + package: "@firebase-function-kits/firestore-bigquery-export@1.0.0", + nonInteractive: true, + skipReport: true, + }); + + expect(getRuntimeDelegateStub).to.not.have.been.called; + }); + it("should handle existing kit when package is already in firebase.json", async () => { const existingKit: ValidatedKitSingle = { kit: "firestore-bigquery-export", diff --git a/src/functions/kits/install.ts b/src/functions/kits/install.ts index 69339017212..edf4c78bcf1 100644 --- a/src/functions/kits/install.ts +++ b/src/functions/kits/install.ts @@ -41,9 +41,9 @@ export const FUNCTION_KITS_DIR = "function-kits"; export interface ExistingFunctionsInfo { existingFunctions: ValidatedSingle[]; - existingKitIds: Set; - existingCodebases: Set; - existingInstanceIds: Set; + existingKitIds: string[]; + existingCodebases: string[]; + existingInstanceIds: string[]; } export interface ScaffoldedKitPaths { @@ -100,6 +100,7 @@ export interface InstallKitOrInstanceOptions { project?: string; projectId?: string; rc?: RC; + skipReport?: boolean; } export interface InstallKitOrInstanceResult { @@ -124,14 +125,15 @@ export interface ExistingKitInstallOptions { rc?: RC; instanceId?: string; seedEnv?: KitInstanceEnvSeed; + skipReport?: boolean; } /** * Generates a unique identifier by appending a random 4-character hex suffix if a collision exists. * Ensures the candidate is truncated so the total length does not exceed 40 characters. */ -export function generateUniqueId(baseId: string, existingIds: Set): string { - if (!existingIds.has(baseId)) { +export function generateUniqueId(baseId: string, existingIds: string[]): string { + if (!existingIds.includes(baseId)) { return baseId; } const prefix = baseId.slice(0, 35); @@ -139,7 +141,7 @@ export function generateUniqueId(baseId: string, existingIds: Set): stri do { const randomSuffix = crypto.randomBytes(2).toString("hex"); candidate = `${prefix}-${randomSuffix}`; - } while (existingIds.has(candidate)); + } while (existingIds.includes(candidate)); return candidate; } @@ -154,33 +156,43 @@ export function parseNpmPackageSpecifier(rawPkg: string): { } { const lastAt = rawPkg.lastIndexOf("@"); if (lastAt > 0) { + const version = rawPkg.substring(lastAt + 1); return { packageName: rawPkg.substring(0, lastAt), - version: rawPkg.substring(lastAt + 1), + ...(version ? { version } : {}), }; } return { packageName: rawPkg }; } /** - * Validates that an npm package name adheres to npm naming conventions. + * Validates that an npm package name or specifier adheres to npm naming conventions. * - Unscoped: 'name' (no slashes) * - Scoped: '@scope/name' (exactly one slash) + * Supports optional version or tag suffix (e.g. '@1.2.3' or '@next'). */ -export function validateNpmPackageName(packageName: string): void { +export function validateNpmPackageName(packageNameOrSpecifier: string): void { + const { packageName, version } = parseNpmPackageSpecifier(packageNameOrSpecifier); const npmPackageRegex = /^(?:@[a-z0-9_.-]+\/[a-z0-9_.-]+|[a-z0-9_.-]+)$/i; if (!packageName || packageName.length > 214 || !npmPackageRegex.test(packageName)) { throw new FirebaseError( - `Invalid NPM package name '${packageName}'. Package names must be valid npm package specifiers (e.g. 'my-kit' or '@scope/my-kit').`, + `Invalid NPM package name '${packageNameOrSpecifier}'. Package names must be valid npm package specifiers (e.g. 'my-kit' or '@scope/my-kit').`, + ); + } + if (packageNameOrSpecifier.lastIndexOf("@") > 0 && !version) { + throw new FirebaseError( + `Invalid NPM package name '${packageNameOrSpecifier}'. Package names must be valid npm package specifiers (e.g. 'my-kit' or '@scope/my-kit').`, ); } } /** - * Sanitizes an npm package name into a valid kit identifier. - * e.g., "@firebase-function-kits/firestore-bigquery-export" -> "firestore-bigquery-export" + * Sanitizes an npm package name or specifier into a valid kit identifier. + * e.g., "@firebase-function-kits/firestore-bigquery-export@1.0.0" -> "firestore-bigquery-export" + * e.g., "my-kit@next" -> "my-kit" */ -export function sanitizePackageNameToKitName(packageName: string): string { +export function sanitizePackageNameToKitName(packageNameOrSpecifier: string): string { + const { packageName } = parseNpmPackageSpecifier(packageNameOrSpecifier); const parts = packageName.split("/"); const nameWithoutScope = parts[parts.length - 1] || packageName; const sanitized = nameWithoutScope.toLowerCase().replace(/[^a-z0-9_-]/g, ""); @@ -188,9 +200,10 @@ export function sanitizePackageNameToKitName(packageName: string): string { } /** - * Checks if a package name is third-party (outside the @firebase-function-kits scope). + * Checks if a package name or specifier is third-party (outside the @firebase-function-kits scope). */ -export function isThirdPartyPackage(packageName: string): boolean { +export function isThirdPartyPackage(packageNameOrSpecifier: string): boolean { + const { packageName } = parseNpmPackageSpecifier(packageNameOrSpecifier); return !packageName.startsWith("@firebase-function-kits/"); } @@ -246,22 +259,24 @@ export function extractExistingFunctionsInfo( ? normalizeAndValidate(configFunctions) : []; - const existingKitIds = new Set(); - const existingCodebases = new Set(); - const existingInstanceIds = new Set(); + const existingKitIds: string[] = []; + const existingCodebases: string[] = []; + const existingInstanceIds: string[] = []; for (const c of existingFunctions) { if (isKitConfig(c)) { - if (c.kit) { - existingKitIds.add(c.kit); + if (c.kit && !existingKitIds.includes(c.kit)) { + existingKitIds.push(c.kit); } if (c.instances) { for (const instId of Object.keys(c.instances)) { - existingInstanceIds.add(instId); + if (!existingInstanceIds.includes(instId)) { + existingInstanceIds.push(instId); + } } } - } else if (c.codebase) { - existingCodebases.add(c.codebase); + } else if (c.codebase && !existingCodebases.includes(c.codebase)) { + existingCodebases.push(c.codebase); } } @@ -278,22 +293,22 @@ export function extractExistingFunctionsInfo( */ export async function promptKitInstanceId( baseKitId: string, - existingInstanceIds: Set, - existingCodebases: Set, + existingInstanceIds: string[], + existingCodebases: string[], nonInteractive?: boolean, customInstanceId?: string, ): Promise { - const instanceCollisions = new Set([...existingInstanceIds, ...existingCodebases]); + const instanceCollisions = [...existingInstanceIds, ...existingCodebases]; const defaultInstanceId = generateUniqueId(baseKitId, instanceCollisions); if (customInstanceId) { validateKitInstanceId(customInstanceId); - if (existingInstanceIds.has(customInstanceId)) { + if (existingInstanceIds.includes(customInstanceId)) { throw new FirebaseError( `functions kit instance ID must be unique across all kits, but '${customInstanceId}' was used more than once.`, ); } - if (existingCodebases.has(customInstanceId)) { + if (existingCodebases.includes(customInstanceId)) { throw new FirebaseError( `functions codebase name and kit instance ID must be mutually exclusive, but '${customInstanceId}' was used as both a codebase name and a kit instance ID.`, ); @@ -311,10 +326,10 @@ export async function promptKitInstanceId( } catch (err: unknown) { return getErrMsg(err); } - if (existingInstanceIds.has(val)) { + if (existingInstanceIds.includes(val)) { return `functions kit instance ID must be unique across all kits, but '${val}' was used more than once.`; } - if (existingCodebases.has(val)) { + if (existingCodebases.includes(val)) { return `functions codebase name and kit instance ID must be mutually exclusive, but '${val}' was used as both a codebase name and a kit instance ID.`; } return true; @@ -322,12 +337,12 @@ export async function promptKitInstanceId( }); validateKitInstanceId(instanceId); - if (existingInstanceIds.has(instanceId)) { + if (existingInstanceIds.includes(instanceId)) { throw new FirebaseError( `functions kit instance ID must be unique across all kits, but '${instanceId}' was used more than once.`, ); } - if (existingCodebases.has(instanceId)) { + if (existingCodebases.includes(instanceId)) { throw new FirebaseError( `functions codebase name and kit instance ID must be mutually exclusive, but '${instanceId}' was used as both a codebase name and a kit instance ID.`, ); @@ -341,7 +356,7 @@ export async function promptKitInstanceId( */ export async function promptKitId( packageName: string, - existingKitIds: Set, + existingKitIds: string[], nonInteractive?: boolean, customKitId?: string, ): Promise { @@ -350,7 +365,7 @@ export async function promptKitId( if (customKitId) { validateKit(customKitId); - if (existingKitIds.has(customKitId)) { + if (existingKitIds.includes(customKitId)) { throw new FirebaseError( `functions.kit must be unique but '${customKitId}' was used more than once.`, ); @@ -368,7 +383,7 @@ export async function promptKitId( } catch (err: unknown) { return getErrMsg(err); } - if (existingKitIds.has(val)) { + if (existingKitIds.includes(val)) { return `functions.kit must be unique but '${val}' was used more than once.`; } return true; @@ -376,7 +391,7 @@ export async function promptKitId( }); validateKit(kitId); - if (existingKitIds.has(kitId)) { + if (existingKitIds.includes(kitId)) { throw new FirebaseError(`functions.kit must be unique but '${kitId}' was used more than once.`); } @@ -929,7 +944,12 @@ export async function addKitInstanceOrConfigureProject( ); let action: "addInstance" | "addEnv"; - if (!isConfiguredForProject && !options.nonInteractive) { + let preselectedInstanceId: string | undefined; + + if (options.instanceId && existingKit.instances && options.instanceId in existingKit.instances) { + action = "addEnv"; + preselectedInstanceId = options.instanceId; + } else if (!isConfiguredForProject && !options.nonInteractive) { const existingInstances = Object.keys(existingKit.instances || {}).join(", "); action = await select<"addInstance" | "addEnv">({ message: `The following instances already exist, but are not configured for this project: ${existingInstances}. What would you like to do?`, @@ -974,7 +994,9 @@ export async function addKitInstanceOrConfigureProject( "functions", `Function kit instance ${clc.bold(instanceId)} successfully added to kit ${clc.bold(existingKit.kit)}.`, ); - await printKitFirstDeployReport(options, instanceId, options.config.path(existingKit.source)); + if (!options.skipReport) { + await printKitFirstDeployReport(options, instanceId, options.config.path(existingKit.source)); + } return { action: "addedInstance", @@ -985,7 +1007,22 @@ export async function addKitInstanceOrConfigureProject( }; } - const selectedInstanceId = await promptExistingInstanceForProject(options, existingKit); + const selectedInstanceId = + preselectedInstanceId || (await promptExistingInstanceForProject(options, existingKit)); + const configDirPath = existingKit.instances[selectedInstanceId]; + if (configDirPath && options.seedEnv?.envs && Object.keys(options.seedEnv.envs).length > 0) { + const absConfigDirPath = options.config.path(configDirPath); + await fs.ensureDir(absConfigDirPath); + seedKitInstanceEnv({ + configDir: absConfigDirPath, + functionsSource: options.config.path(existingKit.source), + projectDir: options.config.projectDir, + projectId: options.seedEnv.projectId, + projectAlias: options.seedEnv.projectAlias, + envs: options.seedEnv.envs, + }); + } + return { action: "configuredEnv", kitId: existingKit.kit, @@ -1166,7 +1203,9 @@ export async function installKitOrInstance( }); logLabeledSuccess("functions", `Function kit ${clc.bold(kitId)} successfully installed.`); - await printKitFirstDeployReport(options, instanceId, absSourcePath); + if (!options.skipReport) { + await printKitFirstDeployReport(options, instanceId, absSourcePath); + } return { action: "installedKit", diff --git a/templates/init/functions/typescript/index-kit-migration.ts b/templates/init/functions/typescript/index-kit-migration.ts index 8b85b0003b0..053bcce3622 100644 --- a/templates/init/functions/typescript/index-kit-migration.ts +++ b/templates/init/functions/typescript/index-kit-migration.ts @@ -17,7 +17,7 @@ import { defineString } from "firebase-functions/params"; // which is determined at install/deploy time. Use a param whenever you want // the value to differ. Learn more at // https://firebase.google.com/docs/functions/config-env#params -export const regionParam = defineString("FUNCTION_DEFAULT_REGION", { +export const regionParam = defineString("DEFAULT_FUNCTION_REGION", { description: "Global default region where functions should be deployed. Can be overriden per-function.", }); From 6e040eb1b29384c37232323a35b11612d7ed2727 Mon Sep 17 00:00:00 2001 From: Thomas Bouldin Date: Mon, 31 Aug 2026 14:36:07 -0700 Subject: [PATCH 02/15] fix(ext:migrate): address code review feedback - Throw blocking error when extension specification cannot be loaded in ext:migrate. - Reuse getInstanceId helper in migrateSecrets. - Use optional chaining on existingKit.instances. - Remove redundant empty JSDoc blocks in extensionsHelper.ts. --- src/commands/ext-migrate.ts | 5 +++++ src/extensions/extensionsHelper.ts | 24 ------------------------ src/extensions/migrate.spec.ts | 25 +++++++++++++++++++++++++ src/extensions/migrate.ts | 2 +- src/functions/kits/install.ts | 2 +- 5 files changed, 32 insertions(+), 26 deletions(-) diff --git a/src/commands/ext-migrate.ts b/src/commands/ext-migrate.ts index fc350f85b0d..bc79e22a508 100644 --- a/src/commands/ext-migrate.ts +++ b/src/commands/ext-migrate.ts @@ -63,6 +63,11 @@ export const command = new Command("ext:migrate") } plan.instance = await ensureInstanceSpec(plan.instance); + if (!plan.instance.config?.source?.spec) { + throw new FirebaseError( + `Could not load extension specification for ${clc.bold(plan.instanceId)}. Unable to export configuration.`, + ); + } const exportedEnvs = functionsEnvFromInstance(plan.instance); diff --git a/src/extensions/extensionsHelper.ts b/src/extensions/extensionsHelper.ts index 2ca6c4a11ae..8a2311df941 100644 --- a/src/extensions/extensionsHelper.ts +++ b/src/extensions/extensionsHelper.ts @@ -513,9 +513,6 @@ async function promptForReleaseStage(args: { return stage; } -/** - * - */ export async function checkExtensionsApiEnabled(options: any): Promise { const projectId = getProjectId(options); if (!projectId) { @@ -524,9 +521,6 @@ export async function checkExtensionsApiEnabled(options: any): Promise return await check(projectId, extensionsOrigin(), "extensions", options.markdown); } -/** - * - */ export async function ensureExtensionsApiEnabled(options: any): Promise { const projectId = getProjectId(options); if (!projectId) { @@ -535,9 +529,6 @@ export async function ensureExtensionsApiEnabled(options: any): Promise { return await ensure(projectId, extensionsOrigin(), "extensions", options.markdown); } -/** - * - */ export async function ensureExtensionsPublisherApiEnabled(options: any): Promise { const projectId = getProjectId(options); if (!projectId) { @@ -1039,9 +1030,6 @@ export async function uploadExtensionVersionFromLocalSource(args: { return res; } -/** - * - */ export function getMissingPublisherError(publisherId: string): FirebaseError { return new FirebaseError( `Couldn't find publisher ID '${clc.bold( @@ -1199,16 +1187,10 @@ export async function instanceIdExists(projectId: string, instanceId: string): P return true; } -/** - * - */ export function isUrlPath(extInstallPath: string): boolean { return extInstallPath.startsWith("https:"); } -/** - * - */ export function isLocalPath(extInstallPath: string): boolean { const trimmedPath = extInstallPath.trim(); return ( @@ -1226,9 +1208,6 @@ export function isLocalPath(extInstallPath: string): boolean { ); } -/** - * - */ export function isLocalOrURLPath(extInstallPath: string): boolean { return isLocalPath(extInstallPath) || isUrlPath(extInstallPath); } @@ -1266,9 +1245,6 @@ export function getSourceOrigin(sourceOrVersion: string): SourceOrigin { ); } -/** - * - */ export async function diagnoseAndFixProject(options: any): Promise { const projectId = getProjectId(options); if (!projectId) { diff --git a/src/extensions/migrate.spec.ts b/src/extensions/migrate.spec.ts index 04834c06c31..7210560b345 100644 --- a/src/extensions/migrate.spec.ts +++ b/src/extensions/migrate.spec.ts @@ -573,6 +573,31 @@ describe("ext:migrate core logic (Unique Veneer)", () => { ); }); + it("should throw if extension specification cannot be loaded", async () => { + const mockConfig = { + projectDir: "/mock/project", + src: { functions: [] }, + } as unknown as Config; + + ensureSpecStub.resolves({ + ...mockInstance1, + config: { + ...mockInstance1.config, + source: undefined, + }, + }); + + await expect( + extMigrateCommand.runner()({ + project: "test-project", + projectId: "test-project", + extInstance: "email-1", + config: mockConfig, + nonInteractive: true, + } as unknown as ExtMigrateOptions), + ).to.be.rejectedWith(FirebaseError, /Could not load extension specification for/); + }); + it("should export envs, migrate secrets, call installKitOrInstance, and return plan", async () => { const mockConfig = { projectDir: "/mock/project", diff --git a/src/extensions/migrate.ts b/src/extensions/migrate.ts index c07a3dfb22a..ac50ce8414d 100644 --- a/src/extensions/migrate.ts +++ b/src/extensions/migrate.ts @@ -446,7 +446,7 @@ export async function migrateSecrets(instance: ExtensionInstance): Promise 0) { const absConfigDirPath = options.config.path(configDirPath); await fs.ensureDir(absConfigDirPath); From cf9170e189b043032de0a921138c720961f8b7ae Mon Sep 17 00:00:00 2001 From: Thomas Bouldin Date: Mon, 31 Aug 2026 15:26:18 -0700 Subject: [PATCH 03/15] refactor(functions): use string arrays instead of Sets in kit install helpers ### Description Refactors function kit installation helper APIs (`extractExistingFunctionsInfo`, `promptKitInstanceId`, `promptKitId`, `generateUniqueId`, and `ExistingFunctionsInfo`) to use standard `string[]` arrays instead of `Set` per codebase conventions (using intrinsic arrays in module APIs). ### Scenarios Tested - Unit tests in `src/functions/kits/install.spec.ts` (`npm run mocha:fast -- src/functions/kits/install.spec.ts`). - Verified unique ID generation and suffix collision avoidance with string arrays. - Verified existing kit, codebase, and instance ID validation and prompt collisions. ### Sample Commands N/A (Internal refactoring) --- src/functions/kits/install.spec.ts | 88 +++++++++++++----------------- src/functions/kits/install.ts | 52 +++++++++--------- 2 files changed, 63 insertions(+), 77 deletions(-) diff --git a/src/functions/kits/install.spec.ts b/src/functions/kits/install.spec.ts index 487a476bcff..160e6537968 100644 --- a/src/functions/kits/install.spec.ts +++ b/src/functions/kits/install.spec.ts @@ -128,20 +128,20 @@ describe("functions/kits/install", () => { describe("generateUniqueId", () => { it("should return base ID when it is not in existing IDs", () => { - const existing = new Set(["other-kit"]); + const existing = ["other-kit"]; expect(generateUniqueId("my-kit", existing)).to.equal("my-kit"); }); it("should append random 4-character hex suffix when base ID collides", () => { - const existing = new Set(["my-kit"]); + const existing = ["my-kit"]; const res = generateUniqueId("my-kit", existing); expect(res).to.match(/^my-kit-[a-f0-9]{4}$/); - expect(existing.has(res)).to.be.false; + expect(existing.includes(res)).to.be.false; }); it("should truncate long base IDs to ensure total length <= 40", () => { const longBase = "a".repeat(40); - const existing = new Set([longBase]); + const existing = [longBase]; const res = generateUniqueId(longBase, existing); expect(res.length).to.be.at.most(40); expect(res).to.match(/^a{35}-[a-f0-9]{4}$/); @@ -314,18 +314,18 @@ describe("functions/kits/install", () => { }); describe("extractExistingFunctionsInfo", () => { - it("should return empty sets when configFunctions is undefined or empty", () => { + it("should return empty arrays when configFunctions is undefined or empty", () => { const resUndefined = extractExistingFunctionsInfo(undefined); expect(resUndefined.existingFunctions).to.deep.equal([]); - expect(resUndefined.existingKitIds.size).to.equal(0); - expect(resUndefined.existingCodebases.size).to.equal(0); - expect(resUndefined.existingInstanceIds.size).to.equal(0); + expect(resUndefined.existingKitIds).to.deep.equal([]); + expect(resUndefined.existingCodebases).to.deep.equal([]); + expect(resUndefined.existingInstanceIds).to.deep.equal([]); const resEmpty = extractExistingFunctionsInfo([]); expect(resEmpty.existingFunctions).to.deep.equal([]); - expect(resEmpty.existingKitIds.size).to.equal(0); - expect(resEmpty.existingCodebases.size).to.equal(0); - expect(resEmpty.existingInstanceIds.size).to.equal(0); + expect(resEmpty.existingKitIds).to.deep.equal([]); + expect(resEmpty.existingCodebases).to.deep.equal([]); + expect(resEmpty.existingInstanceIds).to.deep.equal([]); }); it("should extract kit IDs, instance IDs, and codebases correctly", () => { @@ -345,10 +345,10 @@ describe("functions/kits/install", () => { ]; const res = extractExistingFunctionsInfo(functionsConfig); - expect(res.existingCodebases.has("my-codebase")).to.be.true; - expect(res.existingKitIds.has("my-kit")).to.be.true; - expect(res.existingInstanceIds.has("inst-1")).to.be.true; - expect(res.existingInstanceIds.has("inst-2")).to.be.true; + expect(res.existingCodebases).to.include("my-codebase"); + expect(res.existingKitIds).to.include("my-kit"); + expect(res.existingInstanceIds).to.include("inst-1"); + expect(res.existingInstanceIds).to.include("inst-2"); }); }); @@ -1060,8 +1060,8 @@ describe("functions/kits/install", () => { it("should return custom instance ID directly if provided and valid", async () => { const res = await promptKitInstanceId( "my-kit", - new Set(["other-inst"]), - new Set(["codebase1"]), + ["other-inst"], + ["codebase1"], false, "valid-custom-inst", ); @@ -1070,50 +1070,38 @@ describe("functions/kits/install", () => { it("should throw if custom instance ID collides with existing instances", async () => { await expect( - promptKitInstanceId( - "my-kit", - new Set(["existing-inst"]), - new Set(), - false, - "existing-inst", - ), + promptKitInstanceId("my-kit", ["existing-inst"], [], false, "existing-inst"), ).to.be.rejectedWith(FirebaseError, /must be unique across all kits/); }); it("should throw if custom instance ID collides with codebase name", async () => { await expect( - promptKitInstanceId( - "my-kit", - new Set(), - new Set(["existing-codebase"]), - false, - "existing-codebase", - ), + promptKitInstanceId("my-kit", [], ["existing-codebase"], false, "existing-codebase"), ).to.be.rejectedWith(FirebaseError, /must be mutually exclusive/); }); it("should prompt user when custom instance ID is not provided", async () => { sinon.stub(prompt, "input").resolves("prompted-inst"); - const res = await promptKitInstanceId("my-kit", new Set(), new Set()); + const res = await promptKitInstanceId("my-kit", [], []); expect(res).to.equal("prompted-inst"); }); }); describe("promptKitId", () => { it("should return custom kit ID directly if provided and valid", async () => { - const res = await promptKitId("my-pkg", new Set(["other-kit"]), false, "custom-kit-id"); + const res = await promptKitId("my-pkg", ["other-kit"], false, "custom-kit-id"); expect(res).to.equal("custom-kit-id"); }); it("should throw if custom kit ID collides with existing kit IDs", async () => { await expect( - promptKitId("my-pkg", new Set(["existing-kit"]), false, "existing-kit"), + promptKitId("my-pkg", ["existing-kit"], false, "existing-kit"), ).to.be.rejectedWith(FirebaseError, /functions.kit must be unique/); }); it("should prompt user when custom kit ID is not provided", async () => { sinon.stub(prompt, "input").resolves("prompted-kit"); - const res = await promptKitId("my-pkg", new Set()); + const res = await promptKitId("my-pkg", []); expect(res).to.equal("prompted-kit"); }); }); @@ -1850,9 +1838,9 @@ describe("functions/kits/install", () => { existingKit, { existingFunctions: [existingKit], - existingKitIds: new Set(["firestore-bigquery-export"]), - existingCodebases: new Set(), - existingInstanceIds: new Set(["inst1"]), + existingKitIds: ["firestore-bigquery-export"], + existingCodebases: [], + existingInstanceIds: ["inst1"], }, ); @@ -1916,9 +1904,9 @@ describe("functions/kits/install", () => { existingKit, { existingFunctions: [existingKit], - existingKitIds: new Set(["firestore-bigquery-export"]), - existingCodebases: new Set(), - existingInstanceIds: new Set(["inst1"]), + existingKitIds: ["firestore-bigquery-export"], + existingCodebases: [], + existingInstanceIds: ["inst1"], }, ); @@ -1952,9 +1940,9 @@ describe("functions/kits/install", () => { existingKit, { existingFunctions: [existingKit], - existingKitIds: new Set(["firestore-bigquery-export"]), - existingCodebases: new Set(), - existingInstanceIds: new Set(["inst1"]), + existingKitIds: ["firestore-bigquery-export"], + existingCodebases: [], + existingInstanceIds: ["inst1"], }, ); @@ -2011,9 +1999,9 @@ describe("functions/kits/install", () => { existingKit, { existingFunctions: [existingKit], - existingKitIds: new Set(["firestore-bigquery-export"]), - existingCodebases: new Set(), - existingInstanceIds: new Set(["inst1"]), + existingKitIds: ["firestore-bigquery-export"], + existingCodebases: [], + existingInstanceIds: ["inst1"], }, ); @@ -2057,9 +2045,9 @@ describe("functions/kits/install", () => { existingKit, { existingFunctions: [existingKit], - existingKitIds: new Set(["firestore-bigquery-export"]), - existingCodebases: new Set(), - existingInstanceIds: new Set(["inst1"]), + existingKitIds: ["firestore-bigquery-export"], + existingCodebases: [], + existingInstanceIds: ["inst1"], }, ); diff --git a/src/functions/kits/install.ts b/src/functions/kits/install.ts index ec3cd4ebbe1..ad9084b8632 100644 --- a/src/functions/kits/install.ts +++ b/src/functions/kits/install.ts @@ -48,9 +48,9 @@ export const FUNCTION_KITS_DIR = "function-kits"; export interface ExistingFunctionsInfo { existingFunctions: ValidatedSingle[]; - existingKitIds: Set; - existingCodebases: Set; - existingInstanceIds: Set; + existingKitIds: string[]; + existingCodebases: string[]; + existingInstanceIds: string[]; } export interface ScaffoldedKitPaths { @@ -163,8 +163,8 @@ export interface PrintKitFirstDeployReportOptions { * Generates a unique identifier by appending a random 4-character hex suffix if a collision exists. * Ensures the candidate is truncated so the total length does not exceed 40 characters. */ -export function generateUniqueId(baseId: string, existingIds: Set): string { - if (!existingIds.has(baseId)) { +export function generateUniqueId(baseId: string, existingIds: string[]): string { + if (!existingIds.includes(baseId)) { return baseId; } const prefix = baseId.slice(0, 35); @@ -172,7 +172,7 @@ export function generateUniqueId(baseId: string, existingIds: Set): stri do { const randomSuffix = crypto.randomBytes(2).toString("hex"); candidate = `${prefix}-${randomSuffix}`; - } while (existingIds.has(candidate)); + } while (existingIds.includes(candidate)); return candidate; } @@ -279,22 +279,20 @@ export function extractExistingFunctionsInfo( ? normalizeAndValidate(configFunctions) : []; - const existingKitIds = new Set(); - const existingCodebases = new Set(); - const existingInstanceIds = new Set(); + const existingKitIds: string[] = []; + const existingCodebases: string[] = []; + const existingInstanceIds: string[] = []; for (const c of existingFunctions) { if (isKitConfig(c)) { if (c.kit) { - existingKitIds.add(c.kit); + existingKitIds.push(c.kit); } if (c.instances) { - for (const instId of Object.keys(c.instances)) { - existingInstanceIds.add(instId); - } + existingInstanceIds.push(...Object.keys(c.instances)); } } else if (c.codebase) { - existingCodebases.add(c.codebase); + existingCodebases.push(c.codebase); } } @@ -311,22 +309,22 @@ export function extractExistingFunctionsInfo( */ export async function promptKitInstanceId( baseKitId: string, - existingInstanceIds: Set, - existingCodebases: Set, + existingInstanceIds: string[], + existingCodebases: string[], nonInteractive?: boolean, customInstanceId?: string, ): Promise { - const instanceCollisions = new Set([...existingInstanceIds, ...existingCodebases]); + const instanceCollisions = [...existingInstanceIds, ...existingCodebases]; const defaultInstanceId = generateUniqueId(baseKitId, instanceCollisions); if (customInstanceId) { validateKitInstanceId(customInstanceId); - if (existingInstanceIds.has(customInstanceId)) { + if (existingInstanceIds.includes(customInstanceId)) { throw new FirebaseError( `functions kit instance ID must be unique across all kits, but '${customInstanceId}' was used more than once.`, ); } - if (existingCodebases.has(customInstanceId)) { + if (existingCodebases.includes(customInstanceId)) { throw new FirebaseError( `functions codebase name and kit instance ID must be mutually exclusive, but '${customInstanceId}' was used as both a codebase name and a kit instance ID.`, ); @@ -344,10 +342,10 @@ export async function promptKitInstanceId( } catch (err: unknown) { return getErrMsg(err); } - if (existingInstanceIds.has(val)) { + if (existingInstanceIds.includes(val)) { return `functions kit instance ID must be unique across all kits, but '${val}' was used more than once.`; } - if (existingCodebases.has(val)) { + if (existingCodebases.includes(val)) { return `functions codebase name and kit instance ID must be mutually exclusive, but '${val}' was used as both a codebase name and a kit instance ID.`; } return true; @@ -355,12 +353,12 @@ export async function promptKitInstanceId( }); validateKitInstanceId(instanceId); - if (existingInstanceIds.has(instanceId)) { + if (existingInstanceIds.includes(instanceId)) { throw new FirebaseError( `functions kit instance ID must be unique across all kits, but '${instanceId}' was used more than once.`, ); } - if (existingCodebases.has(instanceId)) { + if (existingCodebases.includes(instanceId)) { throw new FirebaseError( `functions codebase name and kit instance ID must be mutually exclusive, but '${instanceId}' was used as both a codebase name and a kit instance ID.`, ); @@ -374,7 +372,7 @@ export async function promptKitInstanceId( */ export async function promptKitId( packageName: string, - existingKitIds: Set, + existingKitIds: string[], nonInteractive?: boolean, customKitId?: string, ): Promise { @@ -383,7 +381,7 @@ export async function promptKitId( if (customKitId) { validateKit(customKitId); - if (existingKitIds.has(customKitId)) { + if (existingKitIds.includes(customKitId)) { throw new FirebaseError( `functions.kit must be unique but '${customKitId}' was used more than once.`, ); @@ -401,7 +399,7 @@ export async function promptKitId( } catch (err: unknown) { return getErrMsg(err); } - if (existingKitIds.has(val)) { + if (existingKitIds.includes(val)) { return `functions.kit must be unique but '${val}' was used more than once.`; } return true; @@ -409,7 +407,7 @@ export async function promptKitId( }); validateKit(kitId); - if (existingKitIds.has(kitId)) { + if (existingKitIds.includes(kitId)) { throw new FirebaseError(`functions.kit must be unique but '${kitId}' was used more than once.`); } From 1f2bf6e34625be7cca6016fb7c74ce215526e813 Mon Sep 17 00:00:00 2001 From: Thomas Bouldin Date: Mon, 31 Aug 2026 15:37:17 -0700 Subject: [PATCH 04/15] feat(functions): support npm package versions and tags in kit install ### Description Enhances package specifier and kit name parsing to support npm version numbers (e.g. `@1.2.3`, `@^2.0.0`, `@1.0.0-rc.1`) and distribution tags (e.g. `@next`, `@latest`): - Updates `parseNpmPackageSpecifier` to properly handle empty versions vs defined versions. - Updates `validateNpmPackageName` to allow specifiers with versions/tags while rejecting trailing `@` without a version. - Updates `sanitizePackageNameToKitName` to parse the specifier first and extract the unscoped package name before sanitizing to avoid baking versions/tags into default kit IDs. - Updates `isThirdPartyPackage` to extract the base package name before evaluating scope. ### Scenarios Tested - Added unit tests for scoped and unscoped package specifiers with versions and tags. - Verified validation and error cases for trailing `@` and malformed names. - Ran full test suite in `src/functions/kits/install.spec.ts`. ### Sample Commands - `firebase functions:kits:install --package @firebase-function-kits/firestore-bigquery-export@next` - `firebase functions:kits:install --package my-kit@1.2.3` --- src/functions/kits/install.spec.ts | 54 ++++++++++++++++++++++++++++++ src/functions/kits/install.ts | 22 ++++++++---- 2 files changed, 70 insertions(+), 6 deletions(-) diff --git a/src/functions/kits/install.spec.ts b/src/functions/kits/install.spec.ts index 487a476bcff..9071d8e339f 100644 --- a/src/functions/kits/install.spec.ts +++ b/src/functions/kits/install.spec.ts @@ -88,6 +88,12 @@ describe("functions/kits/install", () => { expect(() => validateNpmPackageName("kit_123.v1")).to.not.throw(); }); + it("should accept valid unscoped package specifiers with version or tag", () => { + expect(() => validateNpmPackageName("my-kit@1.2.3")).to.not.throw(); + expect(() => validateNpmPackageName("my-kit@next")).to.not.throw(); + expect(() => validateNpmPackageName("my-kit@^2.0.0")).to.not.throw(); + }); + it("should accept valid scoped package names with exactly one slash", () => { expect(() => validateNpmPackageName("@firebase-function-kits/firestore-bigquery-export"), @@ -95,6 +101,21 @@ describe("functions/kits/install", () => { expect(() => validateNpmPackageName("@invertase/example-kit")).to.not.throw(); }); + it("should accept valid scoped package specifiers with version or tag", () => { + expect(() => + validateNpmPackageName("@firebase-function-kits/firestore-bigquery-export@1.0.0"), + ).to.not.throw(); + expect(() => + validateNpmPackageName("@firebase-function-kits/firestore-bigquery-export@1.0.0-rc.1"), + ).to.not.throw(); + expect(() => + validateNpmPackageName("@firebase-function-kits/firestore-bigquery-export@latest"), + ).to.not.throw(); + expect(() => + validateNpmPackageName("@firebase-function-kits/firestore-bigquery-export@next"), + ).to.not.throw(); + }); + it("should reject package names with multiple slashes", () => { expect(() => validateNpmPackageName("@scope/pkg/extra")).to.throw( FirebaseError, @@ -119,6 +140,14 @@ describe("functions/kits/install", () => { FirebaseError, /Invalid NPM package name/, ); + expect(() => validateNpmPackageName("my-kit@")).to.throw( + FirebaseError, + /Invalid NPM package name/, + ); + expect(() => validateNpmPackageName("@scope/my-kit@")).to.throw( + FirebaseError, + /Invalid NPM package name/, + ); expect(() => validateNpmPackageName("a".repeat(215))).to.throw( FirebaseError, /Invalid NPM package name/, @@ -218,11 +247,30 @@ describe("functions/kits/install", () => { expect(sanitizePackageNameToKitName("@foo/bar")).to.equal("bar"); }); + it("should extract kit name from scoped package specifier with version or tag", () => { + expect( + sanitizePackageNameToKitName("@firebase-function-kits/firestore-bigquery-export@1.0.0"), + ).to.equal("firestore-bigquery-export"); + expect( + sanitizePackageNameToKitName("@firebase-function-kits/firestore-bigquery-export@next"), + ).to.equal("firestore-bigquery-export"); + expect( + sanitizePackageNameToKitName( + "@firebase-function-kits/firestore-bigquery-export@1.0.0-rc.1", + ), + ).to.equal("firestore-bigquery-export"); + }); + it("should sanitize non-scoped package name", () => { expect(sanitizePackageNameToKitName("my-kit")).to.equal("my-kit"); expect(sanitizePackageNameToKitName("My_Kit!")).to.equal("my_kit"); }); + it("should sanitize non-scoped package specifier with version or tag", () => { + expect(sanitizePackageNameToKitName("my-kit@1.2.3")).to.equal("my-kit"); + expect(sanitizePackageNameToKitName("my-kit@next")).to.equal("my-kit"); + }); + it("should truncate long names to 40 characters", () => { const longName = "@scope/" + "a".repeat(50); expect(sanitizePackageNameToKitName(longName)).to.equal("a".repeat(40)); @@ -232,6 +280,10 @@ describe("functions/kits/install", () => { describe("isThirdPartyPackage", () => { it("should return false for packages under @firebase-function-kits scope", () => { expect(isThirdPartyPackage("@firebase-function-kits/firestore-bigquery-export")).to.be.false; + expect(isThirdPartyPackage("@firebase-function-kits/firestore-bigquery-export@1.0.0")).to.be + .false; + expect(isThirdPartyPackage("@firebase-function-kits/firestore-bigquery-export@next")).to.be + .false; }); it("should return true for packages outside @firebase-function-kits scope", () => { @@ -239,6 +291,8 @@ describe("functions/kits/install", () => { expect(isThirdPartyPackage("@firebase-function-kits-fake/foo")).to.be.true; expect(isThirdPartyPackage("@other-scope/my-kit")).to.be.true; expect(isThirdPartyPackage("third-party-kit")).to.be.true; + expect(isThirdPartyPackage("third-party-kit@1.2.3")).to.be.true; + expect(isThirdPartyPackage("third-party-kit@next")).to.be.true; }); }); diff --git a/src/functions/kits/install.ts b/src/functions/kits/install.ts index ec3cd4ebbe1..cdb95350577 100644 --- a/src/functions/kits/install.ts +++ b/src/functions/kits/install.ts @@ -187,9 +187,10 @@ export function parseNpmPackageSpecifier(rawPkg: string): { } { const lastAt = rawPkg.lastIndexOf("@"); if (lastAt > 0) { + const version = rawPkg.substring(lastAt + 1); return { packageName: rawPkg.substring(0, lastAt), - version: rawPkg.substring(lastAt + 1), + ...(version ? { version } : {}), }; } return { packageName: rawPkg }; @@ -200,11 +201,18 @@ export function parseNpmPackageSpecifier(rawPkg: string): { * - Unscoped: 'name' (no slashes) * - Scoped: '@scope/name' (exactly one slash) */ -export function validateNpmPackageName(packageName: string): void { +export function validateNpmPackageName(packageNameOrSpecifier: string): void { + const { packageName, version } = parseNpmPackageSpecifier(packageNameOrSpecifier); const npmPackageRegex = /^(?:@[a-z0-9_.-]+\/[a-z0-9_.-]+|[a-z0-9_.-]+)$/i; - if (!packageName || packageName.length > 214 || !npmPackageRegex.test(packageName)) { + if ( + !packageName || + packageName.length > 214 || + !npmPackageRegex.test(packageName) || + packageNameOrSpecifier.endsWith("@") || + (packageNameOrSpecifier.lastIndexOf("@") > 0 && !version) + ) { throw new FirebaseError( - `Invalid NPM package name '${packageName}'. Package names must be valid npm package specifiers (e.g. 'my-kit' or '@scope/my-kit').`, + `Invalid NPM package name '${packageNameOrSpecifier}'. Package names must be valid npm package specifiers (e.g. 'my-kit' or '@scope/my-kit').`, ); } } @@ -213,7 +221,8 @@ export function validateNpmPackageName(packageName: string): void { * Sanitizes an npm package name into a valid kit identifier. * e.g., "@firebase-function-kits/firestore-bigquery-export" -> "firestore-bigquery-export" */ -export function sanitizePackageNameToKitName(packageName: string): string { +export function sanitizePackageNameToKitName(packageNameOrSpecifier: string): string { + const { packageName } = parseNpmPackageSpecifier(packageNameOrSpecifier); const parts = packageName.split("/"); const nameWithoutScope = parts[parts.length - 1] || packageName; const sanitized = nameWithoutScope.toLowerCase().replace(/[^a-z0-9_-]/g, ""); @@ -223,7 +232,8 @@ export function sanitizePackageNameToKitName(packageName: string): string { /** * Checks if a package name is third-party (outside the @firebase-function-kits scope). */ -export function isThirdPartyPackage(packageName: string): boolean { +export function isThirdPartyPackage(packageNameOrSpecifier: string): boolean { + const { packageName } = parseNpmPackageSpecifier(packageNameOrSpecifier); return !packageName.startsWith("@firebase-function-kits/"); } From 67a295c2c426c039cf41516a1227b8b5884f6c39 Mon Sep 17 00:00:00 2001 From: Thomas Bouldin Date: Mon, 31 Aug 2026 16:03:00 -0700 Subject: [PATCH 05/15] fix(extensions): support optional source on extension instances and add ensureInstanceSpec ### Description - Updates `ExtensionConfig` interface to make `source?: ExtensionSource` optional, reflecting runtime API behavior for published extension instances. - Adds `ensureInstanceSpec` in `extensionsHelper.ts` to fetch an extension version's specification on demand when `instance.config.source?.spec` is absent. - Adds defensive guards when accessing `instance.config.source?.spec` in `secretsUtils.ts` and `export.ts`. ### Scenarios Tested - Added unit tests in `src/extensions/export.spec.ts` for `ensureInstanceSpec`: - Returns instance unchanged when spec is already present. - Fetches spec on demand from the Extension publisher API when missing. - Verified TypeScript compilation and ESLint checks. ### Sample Commands - `npm test` --- src/extensions/export.spec.ts | 95 +++++++++++++++++++++++++++++- src/extensions/export.ts | 11 ++-- src/extensions/extensionsHelper.ts | 54 ++++++++++++++++- src/extensions/secretsUtils.ts | 3 + src/extensions/types.ts | 2 +- 5 files changed, 156 insertions(+), 9 deletions(-) diff --git a/src/extensions/export.spec.ts b/src/extensions/export.spec.ts index bc23bfd5e27..6e77f358cef 100644 --- a/src/extensions/export.spec.ts +++ b/src/extensions/export.spec.ts @@ -1,9 +1,11 @@ import { expect } from "chai"; +import * as sinon from "sinon"; import { functionsEnvFromInstance, parameterizeProject, setSecretParamsToLatest } from "./export"; import { DeploymentInstanceSpec } from "../deploy/extensions/planner"; -import { ParamType } from "./types"; -import { ExtensionInstance } from "./types"; +import { ExtensionInstance, ParamType } from "./types"; +import { ensureInstanceSpec } from "./extensionsHelper"; +import * as publisherApi from "./publisherApi"; describe("ext:export helpers", () => { describe("parameterizeProject", () => { @@ -332,3 +334,92 @@ describe("functionsEnvFromInstance", () => { }); }); }); + +describe("ensureInstanceSpec", () => { + let sandbox: sinon.SinonSandbox; + + beforeEach(() => { + sandbox = sinon.createSandbox(); + }); + + afterEach(() => { + sandbox.restore(); + }); + + it("should return instance as is if spec already exists", async () => { + const instance: ExtensionInstance = { + name: "projects/123/instances/ext1", + createTime: "", + updateTime: "", + state: "ACTIVE", + serviceAccountEmail: "", + config: { + name: "", + createTime: "", + params: {}, + systemParams: {}, + source: { + name: "", + state: "ACTIVE", + packageUri: "", + hash: "", + spec: { + name: "my-ext", + version: "0.1.0", + resources: [], + params: [], + systemParams: [], + }, + }, + }, + }; + + const getExtensionVersionStub = sandbox.stub(publisherApi, "getExtensionVersion"); + const res = await ensureInstanceSpec(instance); + expect(res).to.equal(instance); + expect(getExtensionVersionStub).to.not.have.been.called; + }); + + it("should fetch spec on demand if missing", async () => { + const instance: ExtensionInstance = { + name: "projects/123/instances/ext1", + createTime: "", + updateTime: "", + state: "ACTIVE", + serviceAccountEmail: "", + config: { + name: "", + createTime: "", + params: {}, + systemParams: {}, + extensionRef: "firebase/firestore-send-email", + extensionVersion: "0.1.35", + }, + }; + + sandbox.stub(publisherApi, "getExtensionVersion").resolves({ + name: "publishers/firebase/extensions/firestore-send-email/versions/0.1.35", + ref: "firebase/firestore-send-email@0.1.35", + spec: { + name: "firestore-send-email", + version: "0.1.35", + resources: [], + params: [ + { + param: "LOCATION", + label: "Location", + type: ParamType.SELECT, + }, + ], + systemParams: [], + }, + state: "PUBLISHED", + hash: "hash123", + sourceDownloadUri: "https://example.com/download", + }); + + const res = await ensureInstanceSpec(instance); + expect(res.config?.source?.spec?.name).to.equal("firestore-send-email"); + expect(res.config?.source?.spec?.params).to.have.length(1); + }); +}); diff --git a/src/extensions/export.ts b/src/extensions/export.ts index 6c72c602669..6a8c21347e8 100644 --- a/src/extensions/export.ts +++ b/src/extensions/export.ts @@ -105,8 +105,8 @@ function displaySpecs(specs: DeploymentInstanceSpec[]): void { export function functionsEnvFromInstance(instance: ExtensionInstance): Record { const liveParams = instance.config?.params || {}; const liveSystemParams = instance.config?.systemParams || {}; - const specParams = instance.config?.source?.spec?.params || {}; - const specSystemParams = instance.config?.source?.spec?.systemParams || {}; + const specParams = instance.config?.source?.spec?.params || []; + const specSystemParams = instance.config?.source?.spec?.systemParams || []; const envs: Record = {}; @@ -132,14 +132,17 @@ export function functionsEnvFromInstance(instance: ExtensionInstance): Record { throw new FirebaseError("Unable to proceed until all issues are resolved."); } } + +/** + * Ensures that the extension instance has its spec loaded, fetching it on demand if missing. + */ +export async function ensureInstanceSpec(instance: ExtensionInstance): Promise { + if (instance.config?.source?.spec) { + return instance; + } + + const extensionRef = instance.config?.extensionRef; + const extensionVersion = instance.config?.extensionVersion; + + if (extensionRef) { + try { + const ref = refs.parse(extensionRef); + const version = extensionVersion ? extensionVersion : "latest"; + const extVersion = await getExtensionVersion( + `${ref.publisherId}/${ref.extensionId}@${version}`, + ); + if (extVersion?.spec) { + return { + ...instance, + config: { + ...instance.config, + source: { + ...(instance.config?.source ?? { + state: "ACTIVE", + name: "", + packageUri: "", + hash: "", + }), + spec: extVersion.spec, + }, + }, + }; + } + } catch (err: unknown) { + logger.debug(`Failed to fetch extension version for ${extensionRef}: ${getErrMsg(err)}`); + } + } + + return instance; +} diff --git a/src/extensions/secretsUtils.ts b/src/extensions/secretsUtils.ts index 0cbfcc75dc1..feda9fb68e6 100644 --- a/src/extensions/secretsUtils.ts +++ b/src/extensions/secretsUtils.ts @@ -33,6 +33,9 @@ export async function grantFirexServiceAgentSecretAdminRole( } export async function getManagedSecrets(instance: ExtensionInstance): Promise { + if (!instance.config.source?.spec) { + return []; + } return ( await Promise.all( getActiveSecrets(instance.config.source.spec, instance.config.params).map( diff --git a/src/extensions/types.ts b/src/extensions/types.ts index 007d1350f8e..7d79f6e234c 100644 --- a/src/extensions/types.ts +++ b/src/extensions/types.ts @@ -99,7 +99,7 @@ export const isExtensionInstance = (value: unknown): value is ExtensionInstance export interface ExtensionConfig { name: string; createTime: string; - source: ExtensionSource; + source?: ExtensionSource; params: Record; systemParams: Record; populatedPostinstallContent?: string; From 379b6b1e12e8f0db2718b66445a2dee520d0560e Mon Sep 17 00:00:00 2001 From: Thomas Bouldin Date: Mon, 31 Aug 2026 16:07:00 -0700 Subject: [PATCH 06/15] feat(functions): support seedEnv and skipReport in kit installation ### Description - Adds `skipReport` option to `InstallKitOrInstanceOptions` and `ExistingKitInstallOptions` to allow callers (such as migration tooling) to suppress the first-deploy report. - Supports `seedEnv` in `addKitInstanceOrConfigureProject` when configuring an existing kit instance for an environment/project. - Suppresses `printKitFirstDeployReport` when `skipReport: true` is provided. ### Scenarios Tested - Added unit test in `src/functions/kits/install.spec.ts` verifying `seedEnv` writes `.env.` during existing instance configuration. - Added unit test verifying `skipReport: true` suppresses `printKitFirstDeployReport`. - Ran full test suite in `src/functions/kits/install.spec.ts`. ### Sample Commands - `npm test` --- src/functions/kits/install.spec.ts | 78 ++++++++++++++++++++++++++++++ src/functions/kits/install.ts | 49 +++++++++++++------ 2 files changed, 111 insertions(+), 16 deletions(-) diff --git a/src/functions/kits/install.spec.ts b/src/functions/kits/install.spec.ts index 487a476bcff..e782f4258a6 100644 --- a/src/functions/kits/install.spec.ts +++ b/src/functions/kits/install.spec.ts @@ -1967,6 +1967,62 @@ describe("functions/kits/install", () => { }); }); + it("should seed env for existing instance when seedEnv is provided", async () => { + const existingKit: ValidatedKitSingle = { + kit: "firestore-bigquery-export", + sourcePackage: { name: "@firebase-function-kits/firestore-bigquery-export" }, + source: "function-kits/firestore-bigquery-export/source", + instances: { + inst1: "function-kits/firestore-bigquery-export/config-inst1", + }, + }; + const mockConfig = { + projectDir: "/mock/project", + src: { functions: [existingKit] }, + path: (p: string) => path.join("/mock/project", p), + } as unknown as Config; + + sinon.stub(prompt, "select").resolves("addEnv"); + + const res = await addKitInstanceOrConfigureProject( + { + config: mockConfig, + project: "my-project", + seedEnv: { + projectId: "my-project", + envs: { + FOO: "bar", + }, + }, + }, + existingKit, + { + existingFunctions: [existingKit], + existingKitIds: new Set(["firestore-bigquery-export"]), + existingCodebases: new Set(), + existingInstanceIds: new Set(["inst1"]), + }, + ); + + expect(seedKitInstanceEnvStub).to.have.been.calledOnceWith({ + configDir: path.join( + "/mock/project", + "function-kits/firestore-bigquery-export/config-inst1", + ), + functionsSource: path.join( + "/mock/project", + "function-kits/firestore-bigquery-export/source", + ), + projectDir: "/mock/project", + projectId: "my-project", + projectAlias: undefined, + envs: { + FOO: "bar", + }, + }); + expect(res.action).to.equal("configuredEnv"); + }); + it("should prompt and write params when configuring env for existing instance with params", async () => { const existingKit: ValidatedKitSingle = { kit: "firestore-bigquery-export", @@ -2301,6 +2357,28 @@ describe("functions/kits/install", () => { }); }); + it("should suppress first deploy report when skipReport is true", async () => { + const mockConfig = { + projectDir: "/mock/project", + src: { functions: [] }, + path: (p: string) => path.join("/mock/project", p), + writeProjectFile: sinon.stub(), + askWriteProjectFile: sinon.stub().resolves(), + } as unknown as Config; + + const getRuntimeDelegateStub = sinon.stub(runtimes, "getRuntimeDelegate"); + + await installKitOrInstance({ + config: mockConfig, + package: "@firebase-function-kits/firestore-bigquery-export@1.0.0", + nonInteractive: true, + configure: false, + skipReport: true, + }); + + expect(getRuntimeDelegateStub).to.not.have.been.called; + }); + it("should handle existing kit when package is already in firebase.json", async () => { const existingKit: ValidatedKitSingle = { kit: "firestore-bigquery-export", diff --git a/src/functions/kits/install.ts b/src/functions/kits/install.ts index ec3cd4ebbe1..232a20e6660 100644 --- a/src/functions/kits/install.ts +++ b/src/functions/kits/install.ts @@ -109,6 +109,7 @@ export interface InstallKitOrInstanceOptions { project?: string; projectId?: string; rc?: RC; + skipReport?: boolean; } export interface InstallKitOrInstanceResult { @@ -136,6 +137,7 @@ export interface ExistingKitInstallOptions { rc?: RC; instanceId?: string; seedEnv?: KitInstanceEnvSeed; + skipReport?: boolean; } export interface PromptAndWriteKitParamsOptions { @@ -1071,6 +1073,17 @@ export async function addKitInstanceOrConfigureProject( ); } absConfigDirPath = options.config.path(configDirPath); + if (options.seedEnv?.envs && Object.keys(options.seedEnv.envs).length > 0) { + await fs.ensureDir(absConfigDirPath); + seedKitInstanceEnv({ + configDir: absConfigDirPath, + functionsSource: options.config.path(existingKit.source), + projectDir: options.config.projectDir, + projectId: options.seedEnv.projectId, + projectAlias: options.seedEnv.projectAlias, + envs: options.seedEnv.envs, + }); + } } else { throw new FirebaseError(`Unexpected action '${String(action)}' for kit installation.`); } @@ -1118,14 +1131,16 @@ export async function addKitInstanceOrConfigureProject( ); } - await printKitFirstDeployReport({ - config: options.config, - project: options.project, - projectId: options.projectId, - instanceId, - absSourcePath, - preDiscoveredBuild: discoveredBuild, - }); + if (!options.skipReport) { + await printKitFirstDeployReport({ + config: options.config, + project: options.project, + projectId: options.projectId, + instanceId, + absSourcePath, + preDiscoveredBuild: discoveredBuild, + }); + } return { action: resultAction, @@ -1344,14 +1359,16 @@ export async function installKitOrInstance( }); logLabeledSuccess("functions", `Function kit ${clc.bold(kitId)} successfully installed.`); - await printKitFirstDeployReport({ - config: options.config, - project: options.project, - projectId: options.projectId, - instanceId, - absSourcePath, - preDiscoveredBuild: discoveredBuild, - }); + if (!options.skipReport) { + await printKitFirstDeployReport({ + config: options.config, + project: options.project, + projectId: options.projectId, + instanceId, + absSourcePath, + preDiscoveredBuild: discoveredBuild, + }); + } return { action: "installedKit", From 86d0378e4aa911eae0a9816aa2ff03f59f3ff965 Mon Sep 17 00:00:00 2001 From: Thomas Bouldin Date: Mon, 31 Aug 2026 16:14:05 -0700 Subject: [PATCH 07/15] Merge branch 'pr-11008' into feat_kit_seed_env_skip_report --- src/functions/kits/install.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/functions/kits/install.spec.ts b/src/functions/kits/install.spec.ts index fed4e55ef94..2e463058116 100644 --- a/src/functions/kits/install.spec.ts +++ b/src/functions/kits/install.spec.ts @@ -1986,9 +1986,9 @@ describe("functions/kits/install", () => { existingKit, { existingFunctions: [existingKit], - existingKitIds: new Set(["firestore-bigquery-export"]), - existingCodebases: new Set(), - existingInstanceIds: new Set(["inst1"]), + existingKitIds: ["firestore-bigquery-export"], + existingCodebases: [], + existingInstanceIds: ["inst1"], }, ); From 1bca47ebf78a3fbaa627fcf750b9eca3ceeceb0f Mon Sep 17 00:00:00 2001 From: Thomas Bouldin Date: Mon, 31 Aug 2026 16:16:31 -0700 Subject: [PATCH 08/15] fix(extensions): address review feedback on optional chaining and fallback extensionRef - Use optional chaining for instance.config?.source?.spec in getManagedSecrets. - Fall back to top-level instance.extensionRef and instance.extensionVersion in ensureInstanceSpec. - Add unit test verifying top-level instance.extensionRef spec fetch. --- src/extensions/export.spec.ts | 43 ++++++++++++++++++++++++++++++ src/extensions/extensionsHelper.ts | 4 +-- src/extensions/secretsUtils.ts | 2 +- 3 files changed, 46 insertions(+), 3 deletions(-) diff --git a/src/extensions/export.spec.ts b/src/extensions/export.spec.ts index 6e77f358cef..575896d6a72 100644 --- a/src/extensions/export.spec.ts +++ b/src/extensions/export.spec.ts @@ -422,4 +422,47 @@ describe("ensureInstanceSpec", () => { expect(res.config?.source?.spec?.name).to.equal("firestore-send-email"); expect(res.config?.source?.spec?.params).to.have.length(1); }); + + it("should fetch spec on demand if extensionRef is on instance directly", async () => { + const instance: ExtensionInstance = { + name: "projects/123/instances/ext1", + createTime: "", + updateTime: "", + state: "ACTIVE", + serviceAccountEmail: "", + extensionRef: "firebase/firestore-send-email", + extensionVersion: "0.1.35", + config: { + name: "", + createTime: "", + params: {}, + systemParams: {}, + }, + }; + + sandbox.stub(publisherApi, "getExtensionVersion").resolves({ + name: "publishers/firebase/extensions/firestore-send-email/versions/0.1.35", + ref: "firebase/firestore-send-email@0.1.35", + spec: { + name: "firestore-send-email", + version: "0.1.35", + resources: [], + params: [ + { + param: "LOCATION", + label: "Location", + type: ParamType.SELECT, + }, + ], + systemParams: [], + }, + state: "PUBLISHED", + hash: "hash123", + sourceDownloadUri: "https://example.com/download", + }); + + const res = await ensureInstanceSpec(instance); + expect(res.config?.source?.spec?.name).to.equal("firestore-send-email"); + expect(res.config?.source?.spec?.params).to.have.length(1); + }); }); diff --git a/src/extensions/extensionsHelper.ts b/src/extensions/extensionsHelper.ts index ffa1e650a40..f1fa6add23f 100644 --- a/src/extensions/extensionsHelper.ts +++ b/src/extensions/extensionsHelper.ts @@ -1271,8 +1271,8 @@ export async function ensureInstanceSpec(instance: ExtensionInstance): Promise { - if (!instance.config.source?.spec) { + if (!instance.config?.source?.spec) { return []; } return ( From 9113626d3a07811218db5a848b5ad521e3bf4d4c Mon Sep 17 00:00:00 2001 From: Thomas Bouldin Date: Mon, 31 Aug 2026 16:23:24 -0700 Subject: [PATCH 09/15] fix(functions): validate raw package specifier before parsing in resolvePackageSource - Pass rawPkgName to validateNpmPackageName before destructuring packageName. - Simplify validateNpmPackageName condition. - Add unit test for malformed specifiers in resolvePackageSource. --- src/functions/kits/install.spec.ts | 11 +++++++++++ src/functions/kits/install.ts | 3 +-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/functions/kits/install.spec.ts b/src/functions/kits/install.spec.ts index cfc833ca34f..81ee985856c 100644 --- a/src/functions/kits/install.spec.ts +++ b/src/functions/kits/install.spec.ts @@ -813,6 +813,17 @@ describe("functions/kits/install", () => { expect(source.sourcePackageName).to.equal("@firebase-function-kits/firestore-export"); expect(source.hasBuildScript).to.be.true; }); + + it("should reject malformed package specifier with trailing @", async () => { + await expect( + resolvePackageSource({ + config: { projectDir: "/mock/project" } as Config, + package: "my-kit@", + template: "installation", + nonInteractive: true, + }), + ).to.be.rejectedWith(FirebaseError, /Invalid NPM package name 'my-kit@'/); + }); }); describe("resolveDirectorySource", () => { diff --git a/src/functions/kits/install.ts b/src/functions/kits/install.ts index 9a0a0d02cdb..4bb59f9c866 100644 --- a/src/functions/kits/install.ts +++ b/src/functions/kits/install.ts @@ -208,7 +208,6 @@ export function validateNpmPackageName(packageNameOrSpecifier: string): void { !packageName || packageName.length > 214 || !npmPackageRegex.test(packageName) || - packageNameOrSpecifier.endsWith("@") || (packageNameOrSpecifier.lastIndexOf("@") > 0 && !version) ) { throw new FirebaseError( @@ -1192,8 +1191,8 @@ export async function resolvePackageSource( throw new FirebaseError("Set the --package option to a valid NPM package and try again."); } + validateNpmPackageName(rawPkgName); const { packageName } = parseNpmPackageSpecifier(rawPkgName); - validateNpmPackageName(packageName); const isThirdParty = await promptSecurityConfirmation( rawPkgName, From b39c1d901ea54c468d1fe67eee66b4809f6b0df3 Mon Sep 17 00:00:00 2001 From: Thomas Bouldin Date: Mon, 31 Aug 2026 16:24:44 -0700 Subject: [PATCH 10/15] fix(extensions): refactor ensureInstanceSpec with early returns and error bubbling - Remove silent error suppression; allow errors from getExtensionVersion to bubble up. - Flatten control flow with early returns. - Populate real source metadata from ExtensionVersion instead of placeholder fields. - Update unit tests. --- src/extensions/export.spec.ts | 32 +++++++++++++++++ src/extensions/extensionsHelper.ts | 57 ++++++++++++++---------------- 2 files changed, 58 insertions(+), 31 deletions(-) diff --git a/src/extensions/export.spec.ts b/src/extensions/export.spec.ts index 575896d6a72..289e812bf87 100644 --- a/src/extensions/export.spec.ts +++ b/src/extensions/export.spec.ts @@ -421,6 +421,12 @@ describe("ensureInstanceSpec", () => { const res = await ensureInstanceSpec(instance); expect(res.config?.source?.spec?.name).to.equal("firestore-send-email"); expect(res.config?.source?.spec?.params).to.have.length(1); + expect(res.config?.source?.name).to.equal( + "publishers/firebase/extensions/firestore-send-email/versions/0.1.35", + ); + expect(res.config?.source?.packageUri).to.equal("https://example.com/download"); + expect(res.config?.source?.hash).to.equal("hash123"); + expect(res.config?.source?.state).to.equal("ACTIVE"); }); it("should fetch spec on demand if extensionRef is on instance directly", async () => { @@ -464,5 +470,31 @@ describe("ensureInstanceSpec", () => { const res = await ensureInstanceSpec(instance); expect(res.config?.source?.spec?.name).to.equal("firestore-send-email"); expect(res.config?.source?.spec?.params).to.have.length(1); + expect(res.config?.source?.name).to.equal( + "publishers/firebase/extensions/firestore-send-email/versions/0.1.35", + ); + }); + + it("should let getExtensionVersion errors bubble up", async () => { + const instance: ExtensionInstance = { + name: "projects/123/instances/ext1", + createTime: "", + updateTime: "", + state: "ACTIVE", + serviceAccountEmail: "", + extensionRef: "firebase/firestore-send-email", + extensionVersion: "0.1.35", + config: { + name: "", + createTime: "", + params: {}, + systemParams: {}, + }, + }; + + const networkErr = new Error("Network failure"); + sandbox.stub(publisherApi, "getExtensionVersion").rejects(networkErr); + + await expect(ensureInstanceSpec(instance)).to.be.rejectedWith(networkErr); }); }); diff --git a/src/extensions/extensionsHelper.ts b/src/extensions/extensionsHelper.ts index f1fa6add23f..73831bd603f 100644 --- a/src/extensions/extensionsHelper.ts +++ b/src/extensions/extensionsHelper.ts @@ -15,7 +15,7 @@ import { extensionsOrigin, extensionsPublisherOrigin, storageOrigin } from "../a import { archiveDirectory } from "../archiveDirectory"; import { getFirebaseConfig } from "../functionsConfig"; import { getProjectAdminSdkConfigOrCached } from "../emulator/adminSdkConfig"; -import { getErrMsg, FirebaseError } from "../error"; +import { FirebaseError } from "../error"; import { diagnose } from "./diagnose"; import { checkResponse } from "./askUserForParam"; import { ensure, check } from "../ensureApiEnabled"; @@ -1272,36 +1272,31 @@ export async function ensureInstanceSpec(instance: ExtensionInstance): Promise Date: Mon, 31 Aug 2026 17:25:26 -0700 Subject: [PATCH 11/15] feat(ext:migrate): default suggested kit instance ID to extension instance ID - Add defaultInstanceId option to InstallKitOrInstanceOptions and ExistingKitInstallOptions. - Pass plan.instanceId as defaultInstanceId in ext:migrate command. - Add unit tests verifying defaultInstanceId suggestion and ext:migrate wiring. --- src/commands/ext-migrate.ts | 1 + src/extensions/migrate.spec.ts | 1 + src/functions/kits/install.spec.ts | 31 ++++++++++++++++++++++++++++++ src/functions/kits/install.ts | 6 ++++-- 4 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/commands/ext-migrate.ts b/src/commands/ext-migrate.ts index 1ccea1b6a93..d6cd10dabce 100644 --- a/src/commands/ext-migrate.ts +++ b/src/commands/ext-migrate.ts @@ -83,6 +83,7 @@ export const command = new Command("ext:migrate") config: options.config, package: plan.kitPackage, template: "migration", + defaultInstanceId: plan.instanceId, seedEnv: { projectId, envs: exportedEnvs, diff --git a/src/extensions/migrate.spec.ts b/src/extensions/migrate.spec.ts index e264a6f1093..bd7fc209cc1 100644 --- a/src/extensions/migrate.spec.ts +++ b/src/extensions/migrate.spec.ts @@ -659,6 +659,7 @@ describe("ext:migrate core logic (Unique Veneer)", () => { config: mockConfig, package: "@firebase-function-kits/firestore-send-email", template: "migration", + defaultInstanceId: "email-1", seedEnv: { projectId: "test-project", envs: { diff --git a/src/functions/kits/install.spec.ts b/src/functions/kits/install.spec.ts index 6d799bf3e95..2f003f0cc22 100644 --- a/src/functions/kits/install.spec.ts +++ b/src/functions/kits/install.spec.ts @@ -2414,6 +2414,37 @@ describe("functions/kits/install", () => { }); }); + it("should accept defaultInstanceId as suggested instance ID for package kit", async () => { + const writtenFiles: Record = {}; + const mockConfig = { + projectDir: "/mock/project", + src: { functions: [] }, + path: (p: string) => path.join("/mock/project", p), + writeProjectFile: (file: string, content: unknown) => { + writtenFiles[file] = content; + }, + askWriteProjectFile: (file: string, content: unknown) => { + writtenFiles[file] = content; + return Promise.resolve(); + }, + } as unknown as Config; + + const res = await installKitOrInstance({ + config: mockConfig, + package: "@firebase-function-kits/firestore-bigquery-export@1.0.0", + defaultInstanceId: "my-extension-inst", + nonInteractive: true, + }); + + expect(res).to.deep.equal({ + action: "installedKit", + kitId: "firestore-bigquery-export", + instanceId: "my-extension-inst", + sourcePath: "function-kits/firestore-bigquery-export/source", + configDirPath: "function-kits/firestore-bigquery-export/config-my-extension-inst", + }); + }); + it("should seed environment variables when seedEnv is provided for package kit", async () => { const writtenFiles: Record = {}; const mockConfig = { diff --git a/src/functions/kits/install.ts b/src/functions/kits/install.ts index 3831f56592c..47dc9585bf0 100644 --- a/src/functions/kits/install.ts +++ b/src/functions/kits/install.ts @@ -102,6 +102,7 @@ export interface InstallKitOrInstanceOptions { template?: TemplateType; kitId?: string; instanceId?: string; + defaultInstanceId?: string; seedEnv?: KitInstanceEnvSeed; nonInteractive?: boolean; force?: boolean; @@ -136,6 +137,7 @@ export interface ExistingKitInstallOptions { configure?: boolean; rc?: RC; instanceId?: string; + defaultInstanceId?: string; seedEnv?: KitInstanceEnvSeed; skipReport?: boolean; } @@ -1061,7 +1063,7 @@ export async function addKitInstanceOrConfigureProject( if (action === "addInstance") { resultAction = "addedInstance"; instanceId = await promptKitInstanceId( - existingKit.kit, + options.defaultInstanceId ?? existingKit.kit, existingFunctionsInfo.existingInstanceIds, existingFunctionsInfo.existingCodebases, options.nonInteractive, @@ -1304,7 +1306,7 @@ export async function installKitOrInstance( ); const instanceId = await promptKitInstanceId( - kitId, + options.defaultInstanceId ?? kitId, existingFunctionsInfo.existingInstanceIds, existingFunctionsInfo.existingCodebases, options.nonInteractive, From d26cd67535d8ee5d82e3b08b6e19889483bd583a Mon Sep 17 00:00:00 2001 From: Thomas Bouldin Date: Mon, 31 Aug 2026 17:31:28 -0700 Subject: [PATCH 12/15] feat(functions): remove skipReport option from kit install helpers --- src/functions/kits/install.spec.ts | 22 ----------------- src/functions/kits/install.ts | 38 +++++++++++++----------------- 2 files changed, 16 insertions(+), 44 deletions(-) diff --git a/src/functions/kits/install.spec.ts b/src/functions/kits/install.spec.ts index 2e463058116..b4464c463ad 100644 --- a/src/functions/kits/install.spec.ts +++ b/src/functions/kits/install.spec.ts @@ -2345,28 +2345,6 @@ describe("functions/kits/install", () => { }); }); - it("should suppress first deploy report when skipReport is true", async () => { - const mockConfig = { - projectDir: "/mock/project", - src: { functions: [] }, - path: (p: string) => path.join("/mock/project", p), - writeProjectFile: sinon.stub(), - askWriteProjectFile: sinon.stub().resolves(), - } as unknown as Config; - - const getRuntimeDelegateStub = sinon.stub(runtimes, "getRuntimeDelegate"); - - await installKitOrInstance({ - config: mockConfig, - package: "@firebase-function-kits/firestore-bigquery-export@1.0.0", - nonInteractive: true, - configure: false, - skipReport: true, - }); - - expect(getRuntimeDelegateStub).to.not.have.been.called; - }); - it("should handle existing kit when package is already in firebase.json", async () => { const existingKit: ValidatedKitSingle = { kit: "firestore-bigquery-export", diff --git a/src/functions/kits/install.ts b/src/functions/kits/install.ts index f1f7a32f2d2..dd6262c9b73 100644 --- a/src/functions/kits/install.ts +++ b/src/functions/kits/install.ts @@ -109,7 +109,6 @@ export interface InstallKitOrInstanceOptions { project?: string; projectId?: string; rc?: RC; - skipReport?: boolean; } export interface InstallKitOrInstanceResult { @@ -137,7 +136,6 @@ export interface ExistingKitInstallOptions { rc?: RC; instanceId?: string; seedEnv?: KitInstanceEnvSeed; - skipReport?: boolean; } export interface PromptAndWriteKitParamsOptions { @@ -1129,16 +1127,14 @@ export async function addKitInstanceOrConfigureProject( ); } - if (!options.skipReport) { - await printKitFirstDeployReport({ - config: options.config, - project: options.project, - projectId: options.projectId, - instanceId, - absSourcePath, - preDiscoveredBuild: discoveredBuild, - }); - } + await printKitFirstDeployReport({ + config: options.config, + project: options.project, + projectId: options.projectId, + instanceId, + absSourcePath, + preDiscoveredBuild: discoveredBuild, + }); return { action: resultAction, @@ -1357,16 +1353,14 @@ export async function installKitOrInstance( }); logLabeledSuccess("functions", `Function kit ${clc.bold(kitId)} successfully installed.`); - if (!options.skipReport) { - await printKitFirstDeployReport({ - config: options.config, - project: options.project, - projectId: options.projectId, - instanceId, - absSourcePath, - preDiscoveredBuild: discoveredBuild, - }); - } + await printKitFirstDeployReport({ + config: options.config, + project: options.project, + projectId: options.projectId, + instanceId, + absSourcePath, + preDiscoveredBuild: discoveredBuild, + }); return { action: "installedKit", From 4ec0a7db5a39179a4d6aa4ee96141e410aaefb8f Mon Sep 17 00:00:00 2001 From: Thomas Bouldin Date: Mon, 31 Aug 2026 17:33:38 -0700 Subject: [PATCH 13/15] feat(ext:migrate): remove skipReport option from installKitOrInstance call --- src/commands/ext-migrate.ts | 1 - src/extensions/migrate.spec.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/src/commands/ext-migrate.ts b/src/commands/ext-migrate.ts index d6cd10dabce..7658a83318f 100644 --- a/src/commands/ext-migrate.ts +++ b/src/commands/ext-migrate.ts @@ -88,7 +88,6 @@ export const command = new Command("ext:migrate") projectId, envs: exportedEnvs, }, - skipReport: true, }); logger.info("TODO: Draw the rest of the owl"); diff --git a/src/extensions/migrate.spec.ts b/src/extensions/migrate.spec.ts index bd7fc209cc1..17af830b271 100644 --- a/src/extensions/migrate.spec.ts +++ b/src/extensions/migrate.spec.ts @@ -666,7 +666,6 @@ describe("ext:migrate core logic (Unique Veneer)", () => { PARAM_A: "val_a", }, }, - skipReport: true, }), ); From d2c1fe89ab11b4def43cee09394956dafe0362f7 Mon Sep 17 00:00:00 2001 From: Thomas Bouldin Date: Mon, 31 Aug 2026 17:41:51 -0700 Subject: [PATCH 14/15] fix(ext): address PR review comments - Rename DEFAULT_FUNCTION_REGION to FUNCTION_DEFAULT_REGION in functionsEnvFromInstance. - Use ref.version from parsed extensionRef if top-level extensionVersion is unset in ensureInstanceSpec. - Co-locate ensureInstanceSpec unit tests in extensionsHelper.spec.ts. --- src/extensions/export.spec.ts | 173 +++----------------- src/extensions/export.ts | 4 +- src/extensions/extensionsHelper.spec.ts | 203 ++++++++++++++++++++++++ src/extensions/extensionsHelper.ts | 4 +- 4 files changed, 230 insertions(+), 154 deletions(-) diff --git a/src/extensions/export.spec.ts b/src/extensions/export.spec.ts index 289e812bf87..9cacbe90689 100644 --- a/src/extensions/export.spec.ts +++ b/src/extensions/export.spec.ts @@ -1,11 +1,8 @@ import { expect } from "chai"; -import * as sinon from "sinon"; import { functionsEnvFromInstance, parameterizeProject, setSecretParamsToLatest } from "./export"; import { DeploymentInstanceSpec } from "../deploy/extensions/planner"; import { ExtensionInstance, ParamType } from "./types"; -import { ensureInstanceSpec } from "./extensionsHelper"; -import * as publisherApi from "./publisherApi"; describe("ext:export helpers", () => { describe("parameterizeProject", () => { @@ -298,9 +295,9 @@ describe("functionsEnvFromInstance", () => { }); }); - it("eventarc special cases", () => { + it("system params location should map to FUNCTION_DEFAULT_REGION", () => { const instance: ExtensionInstance = { - name: "", + name: "projects/1234/instances/ext1", createTime: "", updateTime: "", state: "ACTIVE", @@ -310,45 +307,37 @@ describe("functionsEnvFromInstance", () => { createTime: "", params: {}, systemParams: {}, - allowedEventTypes: ["firebase.extensions.storage-resize-images.v1.complete"], - eventarcChannel: "projects/1234/locations/us-west1/channels/firebase", source: { name: "", state: "ACTIVE", packageUri: "", hash: "", spec: { - name: "", - version: "1", + name: "storage-resize-images", + version: "0.1.30", resources: [], params: [], - systemParams: [], + systemParams: [ + { + param: "firebaseextensions.v1beta.function/location", + label: "Location", + default: "us-central1", + }, + ], }, }, }, }; + const output = functionsEnvFromInstance(instance); expect(output).to.deep.equal({ - EXT_SELECTED_EVENTS: "firebase.extensions.storage-resize-images.v1.complete", - EVENTARC_CHANNEL: "projects/1234/locations/us-west1/channels/firebase", + FUNCTION_DEFAULT_REGION: "us-central1", }); }); -}); - -describe("ensureInstanceSpec", () => { - let sandbox: sinon.SinonSandbox; - - beforeEach(() => { - sandbox = sinon.createSandbox(); - }); - afterEach(() => { - sandbox.restore(); - }); - - it("should return instance as is if spec already exists", async () => { + it("eventarc special cases", () => { const instance: ExtensionInstance = { - name: "projects/123/instances/ext1", + name: "", createTime: "", updateTime: "", state: "ACTIVE", @@ -358,14 +347,16 @@ describe("ensureInstanceSpec", () => { createTime: "", params: {}, systemParams: {}, + allowedEventTypes: ["firebase.extensions.storage-resize-images.v1.complete"], + eventarcChannel: "projects/1234/locations/us-west1/channels/firebase", source: { name: "", state: "ACTIVE", packageUri: "", hash: "", spec: { - name: "my-ext", - version: "0.1.0", + name: "", + version: "1", resources: [], params: [], systemParams: [], @@ -373,128 +364,10 @@ describe("ensureInstanceSpec", () => { }, }, }; - - const getExtensionVersionStub = sandbox.stub(publisherApi, "getExtensionVersion"); - const res = await ensureInstanceSpec(instance); - expect(res).to.equal(instance); - expect(getExtensionVersionStub).to.not.have.been.called; - }); - - it("should fetch spec on demand if missing", async () => { - const instance: ExtensionInstance = { - name: "projects/123/instances/ext1", - createTime: "", - updateTime: "", - state: "ACTIVE", - serviceAccountEmail: "", - config: { - name: "", - createTime: "", - params: {}, - systemParams: {}, - extensionRef: "firebase/firestore-send-email", - extensionVersion: "0.1.35", - }, - }; - - sandbox.stub(publisherApi, "getExtensionVersion").resolves({ - name: "publishers/firebase/extensions/firestore-send-email/versions/0.1.35", - ref: "firebase/firestore-send-email@0.1.35", - spec: { - name: "firestore-send-email", - version: "0.1.35", - resources: [], - params: [ - { - param: "LOCATION", - label: "Location", - type: ParamType.SELECT, - }, - ], - systemParams: [], - }, - state: "PUBLISHED", - hash: "hash123", - sourceDownloadUri: "https://example.com/download", - }); - - const res = await ensureInstanceSpec(instance); - expect(res.config?.source?.spec?.name).to.equal("firestore-send-email"); - expect(res.config?.source?.spec?.params).to.have.length(1); - expect(res.config?.source?.name).to.equal( - "publishers/firebase/extensions/firestore-send-email/versions/0.1.35", - ); - expect(res.config?.source?.packageUri).to.equal("https://example.com/download"); - expect(res.config?.source?.hash).to.equal("hash123"); - expect(res.config?.source?.state).to.equal("ACTIVE"); - }); - - it("should fetch spec on demand if extensionRef is on instance directly", async () => { - const instance: ExtensionInstance = { - name: "projects/123/instances/ext1", - createTime: "", - updateTime: "", - state: "ACTIVE", - serviceAccountEmail: "", - extensionRef: "firebase/firestore-send-email", - extensionVersion: "0.1.35", - config: { - name: "", - createTime: "", - params: {}, - systemParams: {}, - }, - }; - - sandbox.stub(publisherApi, "getExtensionVersion").resolves({ - name: "publishers/firebase/extensions/firestore-send-email/versions/0.1.35", - ref: "firebase/firestore-send-email@0.1.35", - spec: { - name: "firestore-send-email", - version: "0.1.35", - resources: [], - params: [ - { - param: "LOCATION", - label: "Location", - type: ParamType.SELECT, - }, - ], - systemParams: [], - }, - state: "PUBLISHED", - hash: "hash123", - sourceDownloadUri: "https://example.com/download", + const output = functionsEnvFromInstance(instance); + expect(output).to.deep.equal({ + EXT_SELECTED_EVENTS: "firebase.extensions.storage-resize-images.v1.complete", + EVENTARC_CHANNEL: "projects/1234/locations/us-west1/channels/firebase", }); - - const res = await ensureInstanceSpec(instance); - expect(res.config?.source?.spec?.name).to.equal("firestore-send-email"); - expect(res.config?.source?.spec?.params).to.have.length(1); - expect(res.config?.source?.name).to.equal( - "publishers/firebase/extensions/firestore-send-email/versions/0.1.35", - ); - }); - - it("should let getExtensionVersion errors bubble up", async () => { - const instance: ExtensionInstance = { - name: "projects/123/instances/ext1", - createTime: "", - updateTime: "", - state: "ACTIVE", - serviceAccountEmail: "", - extensionRef: "firebase/firestore-send-email", - extensionVersion: "0.1.35", - config: { - name: "", - createTime: "", - params: {}, - systemParams: {}, - }, - }; - - const networkErr = new Error("Network failure"); - sandbox.stub(publisherApi, "getExtensionVersion").rejects(networkErr); - - await expect(ensureInstanceSpec(instance)).to.be.rejectedWith(networkErr); }); }); diff --git a/src/extensions/export.ts b/src/extensions/export.ts index ac05f92230b..3069bd8fd43 100644 --- a/src/extensions/export.ts +++ b/src/extensions/export.ts @@ -129,7 +129,7 @@ export function functionsEnvFromInstance(instance: ExtensionInstance): Record { ).to.eql("Prerelease"); }); }); + + describe("ensureInstanceSpec", () => { + let sandbox: sinon.SinonSandbox; + + beforeEach(() => { + sandbox = sinon.createSandbox(); + }); + + afterEach(() => { + sandbox.restore(); + }); + + it("should return instance as is if spec already exists", async () => { + const instance: ExtensionInstance = { + name: "projects/123/instances/ext1", + createTime: "", + updateTime: "", + state: "ACTIVE", + serviceAccountEmail: "", + config: { + name: "", + createTime: "", + params: {}, + systemParams: {}, + source: { + name: "", + state: "ACTIVE", + packageUri: "", + hash: "", + spec: { + name: "my-ext", + version: "0.1.0", + resources: [], + params: [], + systemParams: [], + }, + }, + }, + }; + + const getExtensionVersionStub = sandbox.stub(publisherApi, "getExtensionVersion"); + const res = await extensionsHelper.ensureInstanceSpec(instance); + expect(res).to.equal(instance); + expect(getExtensionVersionStub).to.not.have.been.called; + }); + + it("should fetch spec on demand if missing", async () => { + const instance: ExtensionInstance = { + name: "projects/123/instances/ext1", + createTime: "", + updateTime: "", + state: "ACTIVE", + serviceAccountEmail: "", + config: { + name: "", + createTime: "", + params: {}, + systemParams: {}, + extensionRef: "firebase/firestore-send-email", + extensionVersion: "0.1.35", + }, + }; + + sandbox.stub(publisherApi, "getExtensionVersion").resolves({ + name: "publishers/firebase/extensions/firestore-send-email/versions/0.1.35", + ref: "firebase/firestore-send-email@0.1.35", + spec: { + name: "firestore-send-email", + version: "0.1.35", + resources: [], + params: [ + { + param: "LOCATION", + label: "Location", + type: ParamType.SELECT, + }, + ], + systemParams: [], + }, + state: "PUBLISHED", + hash: "hash123", + sourceDownloadUri: "https://example.com/download", + }); + + const res = await extensionsHelper.ensureInstanceSpec(instance); + expect(res.config?.source?.spec?.name).to.equal("firestore-send-email"); + expect(res.config?.source?.spec?.params).to.have.length(1); + expect(res.config?.source?.name).to.equal( + "publishers/firebase/extensions/firestore-send-email/versions/0.1.35", + ); + expect(res.config?.source?.packageUri).to.equal("https://example.com/download"); + expect(res.config?.source?.hash).to.equal("hash123"); + expect(res.config?.source?.state).to.equal("ACTIVE"); + }); + + it("should fetch spec on demand if extensionRef is on instance directly", async () => { + const instance: ExtensionInstance = { + name: "projects/123/instances/ext1", + createTime: "", + updateTime: "", + state: "ACTIVE", + serviceAccountEmail: "", + extensionRef: "firebase/firestore-send-email", + extensionVersion: "0.1.35", + config: { + name: "", + createTime: "", + params: {}, + systemParams: {}, + }, + }; + + sandbox.stub(publisherApi, "getExtensionVersion").resolves({ + name: "publishers/firebase/extensions/firestore-send-email/versions/0.1.35", + ref: "firebase/firestore-send-email@0.1.35", + spec: { + name: "firestore-send-email", + version: "0.1.35", + resources: [], + params: [ + { + param: "LOCATION", + label: "Location", + type: ParamType.SELECT, + }, + ], + systemParams: [], + }, + state: "PUBLISHED", + hash: "hash123", + sourceDownloadUri: "https://example.com/download", + }); + + const res = await extensionsHelper.ensureInstanceSpec(instance); + expect(res.config?.source?.spec?.name).to.equal("firestore-send-email"); + expect(res.config?.source?.spec?.params).to.have.length(1); + expect(res.config?.source?.name).to.equal( + "publishers/firebase/extensions/firestore-send-email/versions/0.1.35", + ); + }); + + it("should preserve version from extensionRef when extensionVersion is unset", async () => { + const instance: ExtensionInstance = { + name: "projects/123/instances/ext1", + createTime: "", + updateTime: "", + state: "ACTIVE", + serviceAccountEmail: "", + extensionRef: "firebase/firestore-send-email@0.1.35", + config: { + name: "", + createTime: "", + params: {}, + systemParams: {}, + }, + }; + + const getExtensionVersionStub = sandbox.stub(publisherApi, "getExtensionVersion").resolves({ + name: "publishers/firebase/extensions/firestore-send-email/versions/0.1.35", + ref: "firebase/firestore-send-email@0.1.35", + spec: { + name: "firestore-send-email", + version: "0.1.35", + resources: [], + params: [], + systemParams: [], + }, + state: "PUBLISHED", + hash: "hash123", + sourceDownloadUri: "https://example.com/download", + }); + + const res = await extensionsHelper.ensureInstanceSpec(instance); + expect(getExtensionVersionStub).to.have.been.calledWith( + "firebase/firestore-send-email@0.1.35", + ); + expect(res.config?.source?.spec?.name).to.equal("firestore-send-email"); + }); + + it("should let getExtensionVersion errors bubble up", async () => { + const instance: ExtensionInstance = { + name: "projects/123/instances/ext1", + createTime: "", + updateTime: "", + state: "ACTIVE", + serviceAccountEmail: "", + extensionRef: "firebase/firestore-send-email", + extensionVersion: "0.1.35", + config: { + name: "", + createTime: "", + params: {}, + systemParams: {}, + }, + }; + + const networkErr = new Error("Network failure"); + sandbox.stub(publisherApi, "getExtensionVersion").rejects(networkErr); + + await expect(extensionsHelper.ensureInstanceSpec(instance)).to.be.rejectedWith(networkErr); + }); + }); }); diff --git a/src/extensions/extensionsHelper.ts b/src/extensions/extensionsHelper.ts index 73831bd603f..f46cbcf6150 100644 --- a/src/extensions/extensionsHelper.ts +++ b/src/extensions/extensionsHelper.ts @@ -1276,9 +1276,9 @@ export async function ensureInstanceSpec(instance: ExtensionInstance): Promise Date: Mon, 31 Aug 2026 18:23:05 -0700 Subject: [PATCH 15/15] style: restore empty comment lines in JSDocs --- src/extensions/extensionsHelper.ts | 15 +++++++++++---- .../functions/typescript/index-kit-migration.ts | 2 +- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/extensions/extensionsHelper.ts b/src/extensions/extensionsHelper.ts index 69bbecfbf69..f46cbcf6150 100644 --- a/src/extensions/extensionsHelper.ts +++ b/src/extensions/extensionsHelper.ts @@ -193,7 +193,7 @@ type SecretParam = ReturnType; * Substitutes any secret parameters with the correct format * @param projectNumber The project number we are installing into * @param params the full list of params to check for substitution. - * @return The substituted list of params + * @returns The substituted list of params */ export async function substituteSecretParams( projectNumber: string, @@ -202,9 +202,10 @@ export async function substituteSecretParams( const newParams: Record = {}; for await (const [key, value] of Object.entries(params)) { if (typeof value !== "string") { - newParams[key] = `projects/${projectNumber}/secrets/${value.name}/versions/latest`; + newParams[key] = + `projects/${projectNumber}/secrets/${(value as SecretParam).name}/versions/latest`; } else { - newParams[key] = value; + newParams[key] = value as string; } } return newParams; @@ -444,6 +445,7 @@ export async function promptForValidRepoURI(): Promise { /** * Prompts for an extension root. + * * @param defaultRoot the default extension root */ export async function promptForExtensionRoot(defaultRoot: string): Promise { @@ -456,6 +458,7 @@ export async function promptForExtensionRoot(defaultRoot: string): Promise