diff --git a/src/deploy/functions/functionsDeployHelper.ts b/src/deploy/functions/functionsDeployHelper.ts index 8094bc131b6..bcc1cce5eac 100644 --- a/src/deploy/functions/functionsDeployHelper.ts +++ b/src/deploy/functions/functionsDeployHelper.ts @@ -171,6 +171,23 @@ export function getFunctionLabel(fn: backend.TargetIds & { codebase?: string }): return id; } +/** + * Explains why a gcfv1 endpoint may not take over a name that already exists as something + * else, or undefined if the update is legal. A gcfv1 function is a different resource to a + * gcfv2 function or a Cloud Run service, so the CLI cannot update one into the other. + * Shared so that prepare-time validation and the release planner cannot drift apart. + */ +export function generationDowngradeMessage( + want: backend.Endpoint, + have: backend.Endpoint, +): string | undefined { + if (want.platform !== "gcfv1" || have.platform === "gcfv1") { + return undefined; + } + const from = have.platform === "gcfv2" ? "GCFv2" : "Cloud Run"; + return `[${getFunctionLabel(want)}] Functions cannot be downgraded from ${from} to GCFv1`; +} + /** * Returns list of codebases specified in firebase.json filtered by --only filters if present. */ diff --git a/src/deploy/functions/prepare.spec.ts b/src/deploy/functions/prepare.spec.ts index 73839e2e49d..241b63724f7 100644 --- a/src/deploy/functions/prepare.spec.ts +++ b/src/deploy/functions/prepare.spec.ts @@ -1018,6 +1018,58 @@ describe("prepare", () => { expect(want.timeoutSeconds).to.equal(120); }); + it("does not inherit cpu onto a gcfv1 endpoint", () => { + // Redeploying an existing gcfv2 function as gcfv1 (e.g. as a v1 blocking auth + // trigger). Inheriting cpu here fails CPU validation and masks the real + // "cannot be downgraded" error. Memory and timeout exist on both generations. + const have: backend.Endpoint = { + ...ENDPOINT_BASE, + platform: "gcfv2", + httpsTrigger: {}, + cpu: 1, + availableMemoryMb: 512, + timeoutSeconds: 120, + }; + const want: backend.Endpoint = { + ...ENDPOINT_BASE, + platform: "gcfv1", + httpsTrigger: {}, + }; + + prepare.inferDetailsFromExisting(backend.of(want), backend.of(have), /* usedDotEnv= */ false); + + expect(want.cpu).to.be.undefined; + expect(want.availableMemoryMb).to.equal(512); + expect(want.timeoutSeconds).to.equal(120); + }); + + for (const [havePlatform, wantPlatform] of [ + ["run", "gcfv2"], + ["gcfv2", "run"], + ] as const) { + it(`inherits cpu from ${havePlatform} onto ${wantPlatform}`, () => { + const have: backend.Endpoint = { + ...ENDPOINT_BASE, + platform: havePlatform, + httpsTrigger: {}, + cpu: 2, + }; + const want: backend.Endpoint = { + ...ENDPOINT_BASE, + platform: wantPlatform, + httpsTrigger: {}, + }; + + prepare.inferDetailsFromExisting( + backend.of(want), + backend.of(have), + /* usedDotEnv= */ false, + ); + + expect(want.cpu).to.equal(2); + }); + } + it("downgrades concurrency if necessary (explicit)", () => { const have: backend.Endpoint = { ...ENDPOINT_BASE, diff --git a/src/deploy/functions/prepare.ts b/src/deploy/functions/prepare.ts index 1c2e9255a35..f7d8d220294 100644 --- a/src/deploy/functions/prepare.ts +++ b/src/deploy/functions/prepare.ts @@ -641,7 +641,10 @@ export function inferDetailsFromExisting( wantE.availableMemoryMb = haveE.availableMemoryMb; } - if (typeof wantE.cpu === "undefined" && haveE.cpu) { + // cpu does not exist on gcfv1. Inheriting it from an existing gcfv2 function onto + // a gcfv1 endpoint fails CPU validation and masks the accurate "cannot be + // downgraded" error. + if (typeof wantE.cpu === "undefined" && haveE.cpu && wantE.platform !== "gcfv1") { wantE.cpu = haveE.cpu; } diff --git a/src/deploy/functions/release/planner.ts b/src/deploy/functions/release/planner.ts index b6b005e5661..f87101b0e41 100644 --- a/src/deploy/functions/release/planner.ts +++ b/src/deploy/functions/release/planner.ts @@ -1,6 +1,7 @@ import { EndpointFilter, endpointMatchesAnyFilter, + generationDowngradeMessage, getFunctionLabel, } from "../functionsDeployHelper"; import { isFirebaseManaged } from "../../../deploymentTool"; @@ -382,10 +383,9 @@ export function checkForIllegalUpdate(want: backend.Endpoint, have: backend.Endp )}] Changing from ${haveType} function to ${wantType} function is not allowed. Please delete your function and create a new one instead.`, ); } - if (want.platform === "gcfv1" && have.platform === "gcfv2") { - throw new FirebaseError( - `[${getFunctionLabel(want)}] Functions cannot be downgraded from GCFv2 to GCFv1`, - ); + const downgrade = generationDowngradeMessage(want, have); + if (downgrade) { + throw new FirebaseError(downgrade); } // We need to call from module exports so tests can stub this behavior, but that diff --git a/src/deploy/functions/validate.spec.ts b/src/deploy/functions/validate.spec.ts index 165317a106b..5ad425a36c8 100644 --- a/src/deploy/functions/validate.spec.ts +++ b/src/deploy/functions/validate.spec.ts @@ -125,6 +125,30 @@ describe("validate", () => { httpsTrigger: {}, }; + it("rejects downgrading an existing gcfv2 function to gcfv1", () => { + const want = backend.of({ ...ENDPOINT_BASE, platform: "gcfv1" }); + const have = backend.of({ ...ENDPOINT_BASE, platform: "gcfv2", cpu: 1 }); + + expect(() => validate.endpointsAreValid(want, have)).to.throw( + /cannot be downgraded from GCFv2 to GCFv1/, + ); + }); + + it("rejects redeploying an existing Cloud Run service as gcfv1", () => { + const want = backend.of({ ...ENDPOINT_BASE, platform: "gcfv1" }); + const have = backend.of({ ...ENDPOINT_BASE, platform: "run", cpu: 1 }); + + expect(() => validate.endpointsAreValid(want, have)).to.throw( + /cannot be downgraded from Cloud Run to GCFv1/, + ); + }); + + it("allows a gcfv1 function that does not exist yet", () => { + const want = backend.of({ ...ENDPOINT_BASE, platform: "gcfv1" }); + + expect(() => validate.endpointsAreValid(want, backend.empty())).to.not.throw(); + }); + it("disallows concurrency for GCF gen 1", () => { const ep: backend.Endpoint = { ...ENDPOINT_BASE, diff --git a/src/deploy/functions/validate.ts b/src/deploy/functions/validate.ts index 14efb258f4a..f93d495c582 100644 --- a/src/deploy/functions/validate.ts +++ b/src/deploy/functions/validate.ts @@ -4,7 +4,12 @@ import * as clc from "colorette"; import { FirebaseError } from "../../error"; import { getSecretVersion, SecretVersion } from "../../gcp/secretManager"; import { logger } from "../../logger"; -import { EndpointFilter, endpointMatchesFilter, getFunctionLabel } from "./functionsDeployHelper"; +import { + EndpointFilter, + endpointMatchesFilter, + generationDowngradeMessage, + getFunctionLabel, +} from "./functionsDeployHelper"; import { serviceForEndpoint } from "./services"; import * as fsutils from "../../fsutils"; import * as backend from "./backend"; @@ -91,6 +96,9 @@ export function endpointsAreValid( validateLifecycleHooks(wantBackend, existingBackend); const endpoints = backend.allEndpoints(wantBackend); functionIdsAreValid(endpoints); + if (existingBackend) { + noGenerationDowngrades(wantBackend, existingBackend); + } validateTimeoutConfig(endpoints); for (const ep of endpoints) { validateScheduledTimeout(ep); @@ -132,6 +140,23 @@ export function endpointsAreValid( cpuConfigIsValid(endpoints); } +/** + * Rejects an existing gcfv2 function or Cloud Run service being redeployed as gcfv1. The + * release planner enforces this too, but only after the source has been uploaded. + */ +function noGenerationDowngrades( + wantBackend: backend.Backend, + existingBackend: backend.Backend, +): void { + for (const want of backend.allEndpoints(wantBackend)) { + const have = existingBackend.endpoints[want.region]?.[want.id]; + const msg = have && generationDowngradeMessage(want, have); + if (msg) { + throw new FirebaseError(msg); + } + } +} + /** * Validate that endpoints have valid CPU configuration. * Enforces https://cloud.google.com/run/docs/configuring/cpu.