diff --git a/CHANGELOG.md b/CHANGELOG.md index f52a9a9a22f..3e23621e657 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,4 @@ +- Add new 2nd gen Firestore triggered functions with auth context. (#1519) - Adds (opt-out) experiment to disable cleaning up containers after a functions deploy (#6861) - Fix Next.js image optimization check in app directory for Windows (#6930) - Add support to next.config.mjs (#6933) diff --git a/src/deploy/functions/prompts.ts b/src/deploy/functions/prompts.ts index 15da7064a88..e4095bb6e66 100644 --- a/src/deploy/functions/prompts.ts +++ b/src/deploy/functions/prompts.ts @@ -2,12 +2,13 @@ import * as clc from "colorette"; import { getFunctionLabel } from "./functionsDeployHelper"; import { FirebaseError } from "../../error"; -import { promptOnce } from "../../prompt"; +import { confirm, promptOnce } from "../../prompt"; import { logger } from "../../logger"; import * as backend from "./backend"; import * as pricing from "./pricing"; import * as utils from "../../utils"; import { Options } from "../../options"; +import { EndpointUpdate } from "./release/planner"; /** * Checks if a deployment will create any functions with a failure policy @@ -75,11 +76,10 @@ export async function promptForFailurePolicies( */ export async function promptForFunctionDeletion( functionsToDelete: (backend.TargetIds & { platform: backend.FunctionsPlatform })[], - force: boolean, - nonInteractive: boolean, + options: Options, ): Promise { let shouldDeleteFns = true; - if (functionsToDelete.length === 0 || force) { + if (functionsToDelete.length === 0 || options.force) { return true; } const deleteList = functionsToDelete @@ -87,7 +87,7 @@ export async function promptForFunctionDeletion( .map((fn) => "\t" + getFunctionLabel(fn)) .join("\n"); - if (nonInteractive) { + if (options.nonInteractive) { const deleteCommands = functionsToDelete .map((func) => { return "\tfirebase functions:delete " + func.id + " --region " + func.region; @@ -108,9 +108,7 @@ export async function promptForFunctionDeletion( "function first before deleting the old one to prevent event loss. For more info, visit " + clc.underline("https://firebase.google.com/docs/functions/manage-functions#modify" + "\n"), ); - shouldDeleteFns = await promptOnce({ - type: "confirm", - name: "confirm", + shouldDeleteFns = await confirm({ default: false, message: "Would you like to proceed with deletion? Selecting no will continue the rest of the deployments.", @@ -119,6 +117,65 @@ export async function promptForFunctionDeletion( return shouldDeleteFns; } +/** + * Prompts users to confirm potentially unsafe function updates. + * Cases include: + * Migrating from 2nd gen Firestore triggers to Firestore triggers with auth context + * @param fnsToUpdate An array of endpoint updates + * @param options + * @return An array of endpoints to proceed with updating + */ +export async function promptForUnsafeMigration( + fnsToUpdate: EndpointUpdate[], + options: Options, +): Promise { + const unsafeUpdates = fnsToUpdate.filter((eu) => eu.unsafe); + + if (unsafeUpdates.length === 0 || options.force) { + return fnsToUpdate; + } + + const warnMessage = + "The following functions are unsafely changing event types: " + + clc.bold( + unsafeUpdates + .map((eu) => eu.endpoint) + .sort(backend.compareFunctions) + .map(getFunctionLabel) + .join(", "), + ) + + ". " + + "While automatic migration is allowed for these functions, updating the underlying event type may result in data loss. " + + "To avoid this, consider the best practices outlined in the migration guide: https://firebase.google.com/docs/functions/manage-functions?gen=2nd#modify-trigger"; + + utils.logLabeledWarning("functions", warnMessage); + + const safeUpdates = fnsToUpdate.filter((eu) => !eu.unsafe); + + if (options.nonInteractive) { + utils.logLabeledWarning( + "functions", + "Skipping updates for functions that may be unsafe to update. To update these functions anyway, deploy again in interactive mode or use the --force option.", + ); + return safeUpdates; + } + + for (const eu of unsafeUpdates) { + const shouldUpdate = await promptOnce({ + type: "confirm", + name: "confirm", + default: false, + message: `[${getFunctionLabel( + eu.endpoint, + )}] Would you like to proceed with the unsafe migration?`, + }); + if (shouldUpdate) { + safeUpdates.push(eu); + } + } + return safeUpdates; +} + /** * Checks whether a deploy will increase the min instance idle time bill of * any function. Cases include: diff --git a/src/deploy/functions/release/index.ts b/src/deploy/functions/release/index.ts index 5de4f3cc2b4..a8cb5e5ca01 100644 --- a/src/deploy/functions/release/index.ts +++ b/src/deploy/functions/release/index.ts @@ -49,17 +49,28 @@ export async function release( const fnsToDelete = Object.values(plan) .map((regionalChanges) => regionalChanges.endpointsToDelete) .reduce(reduceFlat, []); - const shouldDelete = await prompts.promptForFunctionDeletion( - fnsToDelete, - options.force, - options.nonInteractive, - ); + const shouldDelete = await prompts.promptForFunctionDeletion(fnsToDelete, options); if (!shouldDelete) { for (const change of Object.values(plan)) { change.endpointsToDelete = []; } } + const fnsToUpdate = Object.values(plan) + .map((regionalChanges) => regionalChanges.endpointsToUpdate) + .reduce(reduceFlat, []); + const fnsToUpdateSafe = await prompts.promptForUnsafeMigration(fnsToUpdate, options); + // Replace endpointsToUpdate in deployment plan with endpoints that are either safe + // to update or customers have confirmed they want to update unsafely + for (const key of Object.keys(plan)) { + plan[key].endpointsToUpdate = []; + } + for (const eu of fnsToUpdateSafe) { + const e = eu.endpoint; + const key = `${e.codebase || ""}-${e.region}-${e.availableMemoryMb || "default"}`; + plan[key].endpointsToUpdate.push(eu); + } + const throttlerOptions = { retries: 30, backoff: 20000, diff --git a/src/deploy/functions/release/planner.ts b/src/deploy/functions/release/planner.ts index 023fcc0ed30..142cb02e655 100644 --- a/src/deploy/functions/release/planner.ts +++ b/src/deploy/functions/release/planner.ts @@ -8,10 +8,15 @@ import { FirebaseError } from "../../../error"; import * as utils from "../../../utils"; import * as backend from "../backend"; import * as v2events from "../../../functions/events/v2"; +import { + FIRESTORE_EVENT_REGEX, + FIRESTORE_EVENT_WITH_AUTH_CONTEXT_REGEX, +} from "../../../functions/events/v2"; export interface EndpointUpdate { endpoint: backend.Endpoint; deleteAndRecreate?: backend.Endpoint; + unsafe?: boolean; } export interface Changeset { @@ -112,6 +117,7 @@ export function calculateUpdate(want: backend.Endpoint, have: backend.Endpoint): const update: EndpointUpdate = { endpoint: want, + unsafe: checkForUnsafeUpdate(want, have), }; const needsDelete = changedTriggerRegion(want, have) || @@ -251,6 +257,16 @@ export function upgradedScheduleFromV1ToV2( return true; } +/** Whether a function update is considered unsafe to perform automatically by the CLI */ +export function checkForUnsafeUpdate(want: backend.Endpoint, have: backend.Endpoint): boolean { + return ( + backend.isEventTriggered(want) && + FIRESTORE_EVENT_WITH_AUTH_CONTEXT_REGEX.test(want.eventTrigger.eventType) && + backend.isEventTriggered(have) && + FIRESTORE_EVENT_REGEX.test(have.eventTrigger.eventType) + ); +} + /** Throws if there is an illegal update to a function. */ export function checkForIllegalUpdate(want: backend.Endpoint, have: backend.Endpoint): void { const triggerType = (e: backend.Endpoint): string => { diff --git a/src/deploy/functions/services/index.ts b/src/deploy/functions/services/index.ts index af560ab938b..c580c6cde2e 100644 --- a/src/deploy/functions/services/index.ts +++ b/src/deploy/functions/services/index.ts @@ -150,6 +150,10 @@ const EVENT_SERVICE_MAPPING: Record = { "google.cloud.firestore.document.v1.created": firestoreService, "google.cloud.firestore.document.v1.updated": firestoreService, "google.cloud.firestore.document.v1.deleted": firestoreService, + "google.cloud.firestore.document.v1.written.withAuthContext": firestoreService, + "google.cloud.firestore.document.v1.created.withAuthContext": firestoreService, + "google.cloud.firestore.document.v1.updated.withAuthContext": firestoreService, + "google.cloud.firestore.document.v1.deleted.withAuthContext": firestoreService, }; /** diff --git a/src/functions/events/v2.ts b/src/functions/events/v2.ts index 98433c0a1e3..84fbd72085f 100644 --- a/src/functions/events/v2.ts +++ b/src/functions/events/v2.ts @@ -25,7 +25,14 @@ export const FIRESTORE_EVENTS = [ "google.cloud.firestore.document.v1.created", "google.cloud.firestore.document.v1.updated", "google.cloud.firestore.document.v1.deleted", + "google.cloud.firestore.document.v1.written.withAuthContext", + "google.cloud.firestore.document.v1.created.withAuthContext", + "google.cloud.firestore.document.v1.updated.withAuthContext", + "google.cloud.firestore.document.v1.deleted.withAuthContext", ] as const; +export const FIRESTORE_EVENT_REGEX = /^google\.cloud\.firestore\.document\.v1\.[^\.]*$/; +export const FIRESTORE_EVENT_WITH_AUTH_CONTEXT_REGEX = + /^google\.cloud\.firestore\.document\.v1\..*\.withAuthContext$/; export type Event = | typeof PUBSUB_PUBLISH_EVENT diff --git a/src/test/deploy/functions/prompts.spec.ts b/src/test/deploy/functions/prompts.spec.ts index eb29c627a9a..7dc6069d2a5 100644 --- a/src/test/deploy/functions/prompts.spec.ts +++ b/src/test/deploy/functions/prompts.spec.ts @@ -416,3 +416,117 @@ describe("promptForMinInstances", () => { expect(logStub.firstCall.args[1]).to.match(/Cannot calculate the minimum monthly bill/); }); }); + +describe("promptForUnsafeMigration", () => { + let promptStub: sinon.SinonStub; + + beforeEach(() => { + promptStub = sinon.stub(prompt, "promptOnce"); + }); + + afterEach(() => { + promptStub.restore(); + }); + + const firestoreEventTrigger: backend.EventTrigger = { + eventType: "google.cloud.firestore.document.v1.written", + eventFilters: { namespace: "(default)", document: "messages/{id}" }, + retry: false, + }; + const v2Endpoint0: backend.Endpoint = { + platform: "gcfv2", + id: "0", + region: "us-central1", + project: "a", + entryPoint: "function", + labels: {}, + environmentVariables: {}, + runtime: "nodejs18", + eventTrigger: firestoreEventTrigger, + }; + const v2Endpoint1: backend.Endpoint = { + platform: "gcfv2", + id: "1", + region: "us-central1", + project: "a", + entryPoint: "function", + labels: {}, + environmentVariables: {}, + runtime: "nodejs18", + eventTrigger: firestoreEventTrigger, + }; + + it("should prompt if there are potentially unsafe function updates", async () => { + promptStub.resolves(false); + const epUpdates = [ + { + endpoint: v2Endpoint0, + }, + { + endpoint: v2Endpoint1, + unsafe: true, + }, + ]; + + await functionPrompts.promptForUnsafeMigration(epUpdates, SAMPLE_OPTIONS); + + expect(promptStub).to.have.been.calledOnce; + }); + + it("should only keep function updates that have been confirmed by user", async () => { + promptStub.onFirstCall().resolves(true); + promptStub.onSecondCall().resolves(false); + const epUpdates = [ + { + endpoint: v2Endpoint0, + unsafe: true, + }, + { + endpoint: v2Endpoint1, + unsafe: true, + }, + ]; + + await expect( + functionPrompts.promptForUnsafeMigration(epUpdates, SAMPLE_OPTIONS), + ).to.eventually.deep.equal([{ endpoint: v2Endpoint0, unsafe: true }]); + }); + + it("should force unsafe function updates when flag is set", async () => { + const epUpdates = [ + { + endpoint: v2Endpoint0, + unsafe: true, + }, + { + endpoint: v2Endpoint1, + unsafe: true, + }, + ]; + const options = { ...SAMPLE_OPTIONS, force: true }; + + await expect(functionPrompts.promptForUnsafeMigration(epUpdates, options)).to.eventually.equal( + epUpdates, + ); + expect(promptStub).to.have.not.been.called; + }); + + it("should not proceed with unsafe function updates in non-interactive mode", async () => { + const epUpdates = [ + { + endpoint: v2Endpoint0, + unsafe: true, + }, + { + endpoint: v2Endpoint1, + unsafe: false, + }, + ]; + const options = { ...SAMPLE_OPTIONS, nonInteractive: true }; + + await expect( + functionPrompts.promptForUnsafeMigration(epUpdates, options), + ).to.eventually.deep.equal([{ endpoint: v2Endpoint1, unsafe: false }]); + expect(promptStub).to.have.not.been.called; + }); +}); diff --git a/src/test/deploy/functions/release/planner.spec.ts b/src/test/deploy/functions/release/planner.spec.ts index 2bc7e3e5160..3d1481685a9 100644 --- a/src/test/deploy/functions/release/planner.spec.ts +++ b/src/test/deploy/functions/release/planner.spec.ts @@ -63,6 +63,7 @@ describe("planner", () => { } expect(planner.calculateUpdate(changed, original)).to.deep.equal({ endpoint: changed, + unsafe: false, deleteAndRecreate: original, }); }); @@ -77,6 +78,7 @@ describe("planner", () => { allowV2Upgrades(); expect(planner.calculateUpdate(changed, original)).to.deep.equal({ endpoint: changed, + unsafe: false, deleteAndRecreate: original, }); }); @@ -103,6 +105,7 @@ describe("planner", () => { allowV2Upgrades(); expect(planner.calculateUpdate(changed, original)).to.deep.equal({ endpoint: changed, + unsafe: false, deleteAndRecreate: original, }); }); @@ -119,6 +122,7 @@ describe("planner", () => { }; expect(planner.calculateUpdate(v2Function, v1Function)).to.deep.equal({ endpoint: v2Function, + unsafe: false, }); }); }); @@ -145,6 +149,7 @@ describe("planner", () => { endpointsToUpdate: [ { endpoint: updated, + unsafe: false, }, ], endpointsToDelete: [deleted], @@ -189,6 +194,7 @@ describe("planner", () => { endpointsToUpdate: [ { endpoint: updatedWant, + unsafe: false, }, ], endpointsToDelete: [], @@ -214,6 +220,7 @@ describe("planner", () => { endpointsToUpdate: [ { endpoint: updatedWant, + unsafe: false, }, ], endpointsToDelete: [], @@ -240,6 +247,7 @@ describe("planner", () => { endpointsToUpdate: [ { endpoint: updatedWant, + unsafe: false, }, ], endpointsToDelete: [], @@ -265,6 +273,7 @@ describe("planner", () => { endpointsToUpdate: [ { endpoint: updated, + unsafe: false, }, ], endpointsToDelete: [deleted, pantheon], @@ -302,6 +311,7 @@ describe("planner", () => { endpointsToUpdate: [ { endpoint: region1mem1Updated, + unsafe: false, }, ], endpointsToDelete: [], @@ -318,6 +328,7 @@ describe("planner", () => { endpointsToUpdate: [ { endpoint: region2mem2Updated, + unsafe: false, }, ], endpointsToDelete: [region2mem2Deleted], @@ -354,6 +365,7 @@ describe("planner", () => { endpointsToUpdate: [ { endpoint: group1Updated, + unsafe: false, }, ], endpointsToDelete: [group1Deleted], @@ -415,6 +427,28 @@ describe("planner", () => { }); }); + describe("checkForUnsafeUpdate", () => { + it("returns true when upgrading from 2nd gen firestore to firestore auth context triggers", () => { + const have: backend.Endpoint = { + ...func("id", "region"), + platform: "gcfv2", + eventTrigger: { + eventType: "google.cloud.firestore.document.v1.written", + retry: false, + }, + }; + const want: backend.Endpoint = { + ...func("id", "region"), + platform: "gcfv2", + eventTrigger: { + eventType: "google.cloud.firestore.document.v1.written.withAuthContext", + retry: false, + }, + }; + expect(planner.checkForUnsafeUpdate(want, have)).to.be.true; + }); + }); + describe("checkForIllegalUpdate", () => { // TODO: delete this test once GCF supports upgrading from v1 to v2 it("prohibits upgrades from v1 to v2", () => {