From 239517c521219cff75401193d6e6e03192c2ee58 Mon Sep 17 00:00:00 2001 From: Wanda Mora Date: Tue, 8 Sep 2026 22:06:55 +0000 Subject: [PATCH 1/2] fix: resolve memory param value to valid MemoryOption --- src/extensions/export.spec.ts | 272 +++++++++++++++++++++++++++++++++- src/extensions/export.ts | 88 ++++++++++- 2 files changed, 356 insertions(+), 4 deletions(-) diff --git a/src/extensions/export.spec.ts b/src/extensions/export.spec.ts index ef23b5c0125..ac53636c9fa 100644 --- a/src/extensions/export.spec.ts +++ b/src/extensions/export.spec.ts @@ -3,7 +3,10 @@ import * as sinon from "sinon"; import { functionsEnvFromInstance, + memoryToMb, parameterizeProject, + parseMemory, + resolveMigratedMemory, setSecretParamsToLatest, ejectSecretsFromInstance, } from "./export"; @@ -11,6 +14,7 @@ import { DeploymentInstanceSpec } from "../deploy/extensions/planner"; import { ExtensionInstance, ParamType } from "./types"; import * as secretsModule from "../deploy/extensions/secrets"; import { FirebaseError } from "../error"; +import { MemoryOption } from "firebase-functions/v2/options"; describe("ext:export helpers", () => { describe("parameterizeProject", () => { @@ -125,6 +129,121 @@ describe("ext:export helpers", () => { }); } }); + + describe("memoryToMb", () => { + const testCases: { input: string; expected: number }[] = [ + { input: "256", expected: 256 }, + { input: "512", expected: 512 }, + { input: "1024", expected: 1024 }, + { input: "256Mi", expected: 256 }, + { input: "512Mi", expected: 512 }, + { input: "512MiB", expected: 512 }, + { input: "1Gi", expected: 1024 }, + { input: "1GiB", expected: 1024 }, + { input: "2Gi", expected: 2048 }, + { input: "2GiB", expected: 2048 }, + { input: "1G", expected: 1024 }, + { input: "1GB", expected: 1024 }, + { input: "0.5Gi", expected: 512 }, + { input: "-256", expected: 0 }, + { input: "invalid", expected: 0 }, + { input: "", expected: 0 }, + ]; + + for (const { input, expected } of testCases) { + it(`should parse "${input}" to ${expected} MB`, () => { + expect(memoryToMb(input)).to.equal(expected); + }); + } + }); + + describe("parseMemory", () => { + const testCases: { input?: string; expected?: MemoryOption }[] = [ + { input: "256", expected: "256MiB" }, + { input: "512", expected: "512MiB" }, + { input: "1024", expected: "1GiB" }, + { input: "2048", expected: "2GiB" }, + { input: "4096", expected: "4GiB" }, + { input: "8192", expected: "8GiB" }, + { input: "16384", expected: "16GiB" }, + { input: "32768", expected: "32GiB" }, + { input: "256Mi", expected: "256MiB" }, + { input: "512Mi", expected: "512MiB" }, + { input: "512MiB", expected: "512MiB" }, + { input: "1024Mi", expected: "1GiB" }, + { input: "2048Mi", expected: "2GiB" }, + { input: "1Gi", expected: "1GiB" }, + { input: "1GiB", expected: "1GiB" }, + { input: "2Gi", expected: "2GiB" }, + { input: "2GiB", expected: "2GiB" }, + { input: "1G", expected: "1GiB" }, + { input: "1GB", expected: "1GiB" }, + { input: "0.5Gi", expected: "512MiB" }, + { input: "300", expected: undefined }, + { input: "-256", expected: undefined }, + { input: "invalid", expected: undefined }, + { input: "", expected: undefined }, + { input: undefined, expected: undefined }, + ]; + + for (const { input, expected } of testCases) { + it(`should parse "${String(input)}" to ${String(expected)}`, () => { + expect(parseMemory(input)).to.equal(expected); + }); + } + }); + + describe("resolveMigratedMemory", () => { + it("should return undefined if no memory params are present", () => { + expect(resolveMigratedMemory({}, [])).to.be.undefined; + }); + + it("should return V1 memory if only V1 is present", () => { + expect( + resolveMigratedMemory({ "firebaseextensions.v1beta.function/memory": "256" }, []), + ).to.equal("256"); + }); + + it("should return V2 memory if only V2 is present", () => { + expect( + resolveMigratedMemory({ "firebaseextensions.v1beta.v2function/memory": "512Mi" }, []), + ).to.equal("512Mi"); + }); + + it("should select the highest value when both V1 and V2 are present", () => { + expect( + resolveMigratedMemory( + { + "firebaseextensions.v1beta.function/memory": "1024", + "firebaseextensions.v1beta.v2function/memory": "512Mi", + }, + [], + ), + ).to.equal("1024"); + + expect( + resolveMigratedMemory( + { + "firebaseextensions.v1beta.function/memory": "256", + "firebaseextensions.v1beta.v2function/memory": "512Mi", + }, + [], + ), + ).to.equal("512Mi"); + }); + + it("should fall back to spec defaults when live params are missing", () => { + expect( + resolveMigratedMemory({ "firebaseextensions.v1beta.function/memory": "1024" }, [ + { + param: "firebaseextensions.v1beta.v2function/memory", + label: "Memory", + default: "256Mi", + }, + ]), + ).to.equal("1024"); + }); + }); }); describe("functionsEnvFromInstance", () => { @@ -255,7 +374,7 @@ describe("functionsEnvFromInstance", () => { }; const output = functionsEnvFromInstance(instance); expect(output).to.deep.equal({ - EXT_MIGRATED_SYSTEM_MEMORY: "256", + EXT_MIGRATED_SYSTEM_MEMORY: "256MiB", EXT_MIGRATED_SYSTEM_MININSTANCES: "10", }); }); @@ -298,11 +417,160 @@ describe("functionsEnvFromInstance", () => { }; const output = functionsEnvFromInstance(instance); expect(output).to.deep.equal({ - EXT_MIGRATED_SYSTEM_MEMORY: "256", + EXT_MIGRATED_SYSTEM_MEMORY: "256MiB", EXT_MIGRATED_SYSTEM_MININSTANCES: "10", }); }); + it("system params (both v1 and v2 functions, v2 higher)", () => { + const instance: ExtensionInstance = { + name: "", + createTime: "", + updateTime: "", + state: "ACTIVE", + serviceAccountEmail: "", + config: { + name: "", + createTime: "", + params: {}, + systemParams: { + "firebaseextensions.v1beta.function/memory": "256", + "firebaseextensions.v1beta.v2function/memory": "512Mi", + }, + source: { + name: "", + state: "ACTIVE", + packageUri: "", + hash: "", + spec: { + name: "", + version: "1", + resources: [], + params: [], + systemParams: [], + }, + }, + }, + }; + const output = functionsEnvFromInstance(instance); + expect(output).to.deep.equal({ + EXT_MIGRATED_SYSTEM_MEMORY: "512MiB", + }); + }); + + it("system params (both v1 and v2 functions, v1 higher)", () => { + const instance: ExtensionInstance = { + name: "", + createTime: "", + updateTime: "", + state: "ACTIVE", + serviceAccountEmail: "", + config: { + name: "", + createTime: "", + params: {}, + systemParams: { + "firebaseextensions.v1beta.function/memory": "1024", + "firebaseextensions.v1beta.v2function/memory": "512Mi", + }, + source: { + name: "", + state: "ACTIVE", + packageUri: "", + hash: "", + spec: { + name: "", + version: "1", + resources: [], + params: [], + systemParams: [], + }, + }, + }, + }; + const output = functionsEnvFromInstance(instance); + expect(output).to.deep.equal({ + EXT_MIGRATED_SYSTEM_MEMORY: "1GiB", + }); + }); + + it("system params (both v1 and v2 functions, equal memory)", () => { + const instance: ExtensionInstance = { + name: "", + createTime: "", + updateTime: "", + state: "ACTIVE", + serviceAccountEmail: "", + config: { + name: "", + createTime: "", + params: {}, + systemParams: { + "firebaseextensions.v1beta.function/memory": "256", + "firebaseextensions.v1beta.v2function/memory": "256Mi", + }, + source: { + name: "", + state: "ACTIVE", + packageUri: "", + hash: "", + spec: { + name: "", + version: "1", + resources: [], + params: [], + systemParams: [], + }, + }, + }, + }; + const output = functionsEnvFromInstance(instance); + expect(output).to.deep.equal({ + EXT_MIGRATED_SYSTEM_MEMORY: "256MiB", + }); + }); + + it("system params (v1 in live, v2 in spec defaults, v1 higher)", () => { + const instance: ExtensionInstance = { + name: "", + createTime: "", + updateTime: "", + state: "ACTIVE", + serviceAccountEmail: "", + config: { + name: "", + createTime: "", + params: {}, + systemParams: { + "firebaseextensions.v1beta.function/memory": "1024", + }, + source: { + name: "", + state: "ACTIVE", + packageUri: "", + hash: "", + spec: { + name: "", + version: "1", + resources: [], + params: [], + systemParams: [ + { + param: "firebaseextensions.v1beta.v2function/memory", + label: "Memory", + default: "256Mi", + }, + ], + }, + }, + }, + }; + const output = functionsEnvFromInstance(instance); + expect(output).to.deep.equal({ + EXT_MIGRATED_SYSTEM_MEMORY: "1GiB", + }); + }); + it("system params location should map to FUNCTION_DEFAULT_REGION", () => { const instance: ExtensionInstance = { name: "projects/1234/instances/ext1", diff --git a/src/extensions/export.ts b/src/extensions/export.ts index f755eaaf9aa..6e58769b8aa 100644 --- a/src/extensions/export.ts +++ b/src/extensions/export.ts @@ -7,10 +7,11 @@ import { SECRET_VERSION_NAME_REGEX, } from "../gcp/secretManager"; import { getActiveSecrets } from "./secretsUtils"; -import { ExtensionInstance } from "./types"; +import { ExtensionInstance, Param } from "./types"; import { transferSecretToKits, secretHasExtensionsLabel } from "../deploy/extensions/secrets"; import { FirebaseError } from "../error"; import { logLabeledError } from "../utils"; +import { MemoryOption } from "firebase-functions/v2/options"; /** * parameterizeProject searchs spec.params for any param that include projectId or projectNumber, @@ -97,6 +98,81 @@ function displaySpecs(specs: DeploymentInstanceSpec[]): void { } } +/** + * Converts a memory string (e.g. "256", "512Mi", "1Gi", "1024") to megabytes (MB) for comparison. + */ +export function memoryToMb(memory: string): number { + const trimmed = memory.trim(); + if (/^\d+(?:\.\d+)?(?:Gi|GiB|G|GB)$/i.test(trimmed)) { + return parseFloat(trimmed) * 1024; + } + const parsed = parseFloat(trimmed); + return isNaN(parsed) || parsed < 0 ? 0 : parsed; +} + +const MB_TO_MEMORY_OPTION: Record = { + 128: "128MiB", + 256: "256MiB", + 512: "512MiB", + 1024: "1GiB", + 2048: "2GiB", + 4096: "4GiB", + 8192: "8GiB", + 16384: "16GiB", + 32768: "32GiB", +}; + +/** + * Normalizes a memory string (e.g. "512Mi", "1Gi", "256", "1024") into a valid MemoryOption (e.g. "512MiB", "1GiB"). + */ +export function parseMemory(raw?: string): MemoryOption | undefined { + if (!raw) { + return undefined; + } + const trimmed = raw.trim(); + let mb: number; + if (/^\d+(?:\.\d+)?(?:Gi|GiB|G|GB)$/i.test(trimmed)) { + mb = parseFloat(trimmed) * 1024; + } else if (/^\d+(?:\.\d+)?(?:Mi|MiB|M|MB)?$/i.test(trimmed)) { + mb = parseFloat(trimmed); + } else { + return undefined; + } + if (isNaN(mb) || mb <= 0) { + return undefined; + } + return MB_TO_MEMORY_OPTION[mb]; +} + +const V1_MEMORY_PARAM = "firebaseextensions.v1beta.function/memory"; +const V2_MEMORY_PARAM = "firebaseextensions.v1beta.v2function/memory"; +const MEMORY_PARAMS = new Set([V1_MEMORY_PARAM, V2_MEMORY_PARAM]); + +/** + * Resolves the memory configuration for a migrated Extension instance. + * If both V1 and V2 memory parameters are present, picks the one with the highest memory value. + */ +export function resolveMigratedMemory( + liveSystemParams: Record = {}, + specSystemParams: readonly Param[] = [], +): string | undefined { + const getParam = (paramName: string): string | undefined => { + if (paramName in liveSystemParams) { + return liveSystemParams[paramName]; + } + const defaultVal = specSystemParams.find((p) => p.param === paramName)?.default; + return defaultVal !== undefined ? String(defaultVal) : undefined; + }; + + const v1 = getParam(V1_MEMORY_PARAM); + const v2 = getParam(V2_MEMORY_PARAM); + + if (v1 && v2) { + return memoryToMb(v1) > memoryToMb(v2) ? v1 : v2; + } + return v2 ?? v1; +} + /** * Translates a currently deployed Extension instance into a Functions environment. * This includes setting any default params not set in the deployed instance to their @@ -125,6 +201,9 @@ export function functionsEnvFromInstance(instance: ExtensionInstance): Record Date: Wed, 9 Sep 2026 18:01:46 +0000 Subject: [PATCH 2/2] Address extra comments --- src/extensions/export.spec.ts | 7 +++++-- src/extensions/export.ts | 34 ++++++++++++++++------------------ 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/src/extensions/export.spec.ts b/src/extensions/export.spec.ts index ac53636c9fa..f7440ed52e1 100644 --- a/src/extensions/export.spec.ts +++ b/src/extensions/export.spec.ts @@ -131,7 +131,7 @@ describe("ext:export helpers", () => { }); describe("memoryToMb", () => { - const testCases: { input: string; expected: number }[] = [ + const testCases: { input?: string; expected: number }[] = [ { input: "256", expected: 256 }, { input: "512", expected: 512 }, { input: "1024", expected: 1024 }, @@ -148,10 +148,12 @@ describe("ext:export helpers", () => { { input: "-256", expected: 0 }, { input: "invalid", expected: 0 }, { input: "", expected: 0 }, + { input: " ", expected: 0 }, + { input: undefined, expected: 0 }, ]; for (const { input, expected } of testCases) { - it(`should parse "${input}" to ${expected} MB`, () => { + it(`should parse "${String(input)}" to ${expected} MB`, () => { expect(memoryToMb(input)).to.equal(expected); }); } @@ -183,6 +185,7 @@ describe("ext:export helpers", () => { { input: "-256", expected: undefined }, { input: "invalid", expected: undefined }, { input: "", expected: undefined }, + { input: " ", expected: undefined }, { input: undefined, expected: undefined }, ]; diff --git a/src/extensions/export.ts b/src/extensions/export.ts index 6e58769b8aa..bdc3117512e 100644 --- a/src/extensions/export.ts +++ b/src/extensions/export.ts @@ -98,19 +98,28 @@ function displaySpecs(specs: DeploymentInstanceSpec[]): void { } } +const GIB_REGEX = /^\d+(?:\.\d+)?(?:Gi|GiB|G|GB)$/i; +const MIB_REGEX = /^\d+(?:\.\d+)?(?:Mi|MiB|M|MB)?$/i; + /** * Converts a memory string (e.g. "256", "512Mi", "1Gi", "1024") to megabytes (MB) for comparison. */ -export function memoryToMb(memory: string): number { +export function memoryToMb(memory?: string): number { + if (!memory) { + return 0; + } const trimmed = memory.trim(); - if (/^\d+(?:\.\d+)?(?:Gi|GiB|G|GB)$/i.test(trimmed)) { - return parseFloat(trimmed) * 1024; + if (GIB_REGEX.test(trimmed)) { + return Math.round(parseFloat(trimmed) * 1024); } - const parsed = parseFloat(trimmed); - return isNaN(parsed) || parsed < 0 ? 0 : parsed; + if (MIB_REGEX.test(trimmed)) { + const parsed = parseFloat(trimmed); + return isNaN(parsed) || parsed < 0 ? 0 : parsed; + } + return 0; } -const MB_TO_MEMORY_OPTION: Record = { +const MB_TO_MEMORY_OPTION: Partial> = { 128: "128MiB", 256: "256MiB", 512: "512MiB", @@ -129,18 +138,7 @@ export function parseMemory(raw?: string): MemoryOption | undefined { if (!raw) { return undefined; } - const trimmed = raw.trim(); - let mb: number; - if (/^\d+(?:\.\d+)?(?:Gi|GiB|G|GB)$/i.test(trimmed)) { - mb = parseFloat(trimmed) * 1024; - } else if (/^\d+(?:\.\d+)?(?:Mi|MiB|M|MB)?$/i.test(trimmed)) { - mb = parseFloat(trimmed); - } else { - return undefined; - } - if (isNaN(mb) || mb <= 0) { - return undefined; - } + const mb = memoryToMb(raw); return MB_TO_MEMORY_OPTION[mb]; }