Skip to content

Commit e1aa2bf

Browse files
authored
Add support for deploying 2nd gen Firestore auth context triggers (#6734)
* track new firestore auth event types * clean up unit tests * changelog * run new formatter * address feedback * format and fix unit tests
1 parent c92c91a commit e1aa2bf

File tree

8 files changed

+257
-13
lines changed

8 files changed

+257
-13
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
- Add new 2nd gen Firestore triggered functions with auth context. (#1519)
12
- Adds (opt-out) experiment to disable cleaning up containers after a functions deploy (#6861)
23
- Fix Next.js image optimization check in app directory for Windows (#6930)
34
- Add support to next.config.mjs (#6933)

src/deploy/functions/prompts.ts

Lines changed: 65 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@ import * as clc from "colorette";
22

33
import { getFunctionLabel } from "./functionsDeployHelper";
44
import { FirebaseError } from "../../error";
5-
import { promptOnce } from "../../prompt";
5+
import { confirm, promptOnce } from "../../prompt";
66
import { logger } from "../../logger";
77
import * as backend from "./backend";
88
import * as pricing from "./pricing";
99
import * as utils from "../../utils";
1010
import { Options } from "../../options";
11+
import { EndpointUpdate } from "./release/planner";
1112

1213
/**
1314
* Checks if a deployment will create any functions with a failure policy
@@ -75,19 +76,18 @@ export async function promptForFailurePolicies(
7576
*/
7677
export async function promptForFunctionDeletion(
7778
functionsToDelete: (backend.TargetIds & { platform: backend.FunctionsPlatform })[],
78-
force: boolean,
79-
nonInteractive: boolean,
79+
options: Options,
8080
): Promise<boolean> {
8181
let shouldDeleteFns = true;
82-
if (functionsToDelete.length === 0 || force) {
82+
if (functionsToDelete.length === 0 || options.force) {
8383
return true;
8484
}
8585
const deleteList = functionsToDelete
8686
.sort(backend.compareFunctions)
8787
.map((fn) => "\t" + getFunctionLabel(fn))
8888
.join("\n");
8989

90-
if (nonInteractive) {
90+
if (options.nonInteractive) {
9191
const deleteCommands = functionsToDelete
9292
.map((func) => {
9393
return "\tfirebase functions:delete " + func.id + " --region " + func.region;
@@ -108,9 +108,7 @@ export async function promptForFunctionDeletion(
108108
"function first before deleting the old one to prevent event loss. For more info, visit " +
109109
clc.underline("https://firebase.google.com/docs/functions/manage-functions#modify" + "\n"),
110110
);
111-
shouldDeleteFns = await promptOnce({
112-
type: "confirm",
113-
name: "confirm",
111+
shouldDeleteFns = await confirm({
114112
default: false,
115113
message:
116114
"Would you like to proceed with deletion? Selecting no will continue the rest of the deployments.",
@@ -119,6 +117,65 @@ export async function promptForFunctionDeletion(
119117
return shouldDeleteFns;
120118
}
121119

120+
/**
121+
* Prompts users to confirm potentially unsafe function updates.
122+
* Cases include:
123+
* Migrating from 2nd gen Firestore triggers to Firestore triggers with auth context
124+
* @param fnsToUpdate An array of endpoint updates
125+
* @param options
126+
* @return An array of endpoints to proceed with updating
127+
*/
128+
export async function promptForUnsafeMigration(
129+
fnsToUpdate: EndpointUpdate[],
130+
options: Options,
131+
): Promise<EndpointUpdate[]> {
132+
const unsafeUpdates = fnsToUpdate.filter((eu) => eu.unsafe);
133+
134+
if (unsafeUpdates.length === 0 || options.force) {
135+
return fnsToUpdate;
136+
}
137+
138+
const warnMessage =
139+
"The following functions are unsafely changing event types: " +
140+
clc.bold(
141+
unsafeUpdates
142+
.map((eu) => eu.endpoint)
143+
.sort(backend.compareFunctions)
144+
.map(getFunctionLabel)
145+
.join(", "),
146+
) +
147+
". " +
148+
"While automatic migration is allowed for these functions, updating the underlying event type may result in data loss. " +
149+
"To avoid this, consider the best practices outlined in the migration guide: https://firebase.google.com/docs/functions/manage-functions?gen=2nd#modify-trigger";
150+
151+
utils.logLabeledWarning("functions", warnMessage);
152+
153+
const safeUpdates = fnsToUpdate.filter((eu) => !eu.unsafe);
154+
155+
if (options.nonInteractive) {
156+
utils.logLabeledWarning(
157+
"functions",
158+
"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.",
159+
);
160+
return safeUpdates;
161+
}
162+
163+
for (const eu of unsafeUpdates) {
164+
const shouldUpdate = await promptOnce({
165+
type: "confirm",
166+
name: "confirm",
167+
default: false,
168+
message: `[${getFunctionLabel(
169+
eu.endpoint,
170+
)}] Would you like to proceed with the unsafe migration?`,
171+
});
172+
if (shouldUpdate) {
173+
safeUpdates.push(eu);
174+
}
175+
}
176+
return safeUpdates;
177+
}
178+
122179
/**
123180
* Checks whether a deploy will increase the min instance idle time bill of
124181
* any function. Cases include:

src/deploy/functions/release/index.ts

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,17 +49,28 @@ export async function release(
4949
const fnsToDelete = Object.values(plan)
5050
.map((regionalChanges) => regionalChanges.endpointsToDelete)
5151
.reduce(reduceFlat, []);
52-
const shouldDelete = await prompts.promptForFunctionDeletion(
53-
fnsToDelete,
54-
options.force,
55-
options.nonInteractive,
56-
);
52+
const shouldDelete = await prompts.promptForFunctionDeletion(fnsToDelete, options);
5753
if (!shouldDelete) {
5854
for (const change of Object.values(plan)) {
5955
change.endpointsToDelete = [];
6056
}
6157
}
6258

59+
const fnsToUpdate = Object.values(plan)
60+
.map((regionalChanges) => regionalChanges.endpointsToUpdate)
61+
.reduce(reduceFlat, []);
62+
const fnsToUpdateSafe = await prompts.promptForUnsafeMigration(fnsToUpdate, options);
63+
// Replace endpointsToUpdate in deployment plan with endpoints that are either safe
64+
// to update or customers have confirmed they want to update unsafely
65+
for (const key of Object.keys(plan)) {
66+
plan[key].endpointsToUpdate = [];
67+
}
68+
for (const eu of fnsToUpdateSafe) {
69+
const e = eu.endpoint;
70+
const key = `${e.codebase || ""}-${e.region}-${e.availableMemoryMb || "default"}`;
71+
plan[key].endpointsToUpdate.push(eu);
72+
}
73+
6374
const throttlerOptions = {
6475
retries: 30,
6576
backoff: 20000,

src/deploy/functions/release/planner.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,15 @@ import { FirebaseError } from "../../../error";
88
import * as utils from "../../../utils";
99
import * as backend from "../backend";
1010
import * as v2events from "../../../functions/events/v2";
11+
import {
12+
FIRESTORE_EVENT_REGEX,
13+
FIRESTORE_EVENT_WITH_AUTH_CONTEXT_REGEX,
14+
} from "../../../functions/events/v2";
1115

1216
export interface EndpointUpdate {
1317
endpoint: backend.Endpoint;
1418
deleteAndRecreate?: backend.Endpoint;
19+
unsafe?: boolean;
1520
}
1621

1722
export interface Changeset {
@@ -112,6 +117,7 @@ export function calculateUpdate(want: backend.Endpoint, have: backend.Endpoint):
112117

113118
const update: EndpointUpdate = {
114119
endpoint: want,
120+
unsafe: checkForUnsafeUpdate(want, have),
115121
};
116122
const needsDelete =
117123
changedTriggerRegion(want, have) ||
@@ -251,6 +257,16 @@ export function upgradedScheduleFromV1ToV2(
251257
return true;
252258
}
253259

260+
/** Whether a function update is considered unsafe to perform automatically by the CLI */
261+
export function checkForUnsafeUpdate(want: backend.Endpoint, have: backend.Endpoint): boolean {
262+
return (
263+
backend.isEventTriggered(want) &&
264+
FIRESTORE_EVENT_WITH_AUTH_CONTEXT_REGEX.test(want.eventTrigger.eventType) &&
265+
backend.isEventTriggered(have) &&
266+
FIRESTORE_EVENT_REGEX.test(have.eventTrigger.eventType)
267+
);
268+
}
269+
254270
/** Throws if there is an illegal update to a function. */
255271
export function checkForIllegalUpdate(want: backend.Endpoint, have: backend.Endpoint): void {
256272
const triggerType = (e: backend.Endpoint): string => {

src/deploy/functions/services/index.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,10 @@ const EVENT_SERVICE_MAPPING: Record<events.Event, Service> = {
150150
"google.cloud.firestore.document.v1.created": firestoreService,
151151
"google.cloud.firestore.document.v1.updated": firestoreService,
152152
"google.cloud.firestore.document.v1.deleted": firestoreService,
153+
"google.cloud.firestore.document.v1.written.withAuthContext": firestoreService,
154+
"google.cloud.firestore.document.v1.created.withAuthContext": firestoreService,
155+
"google.cloud.firestore.document.v1.updated.withAuthContext": firestoreService,
156+
"google.cloud.firestore.document.v1.deleted.withAuthContext": firestoreService,
153157
};
154158

155159
/**

src/functions/events/v2.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,14 @@ export const FIRESTORE_EVENTS = [
2525
"google.cloud.firestore.document.v1.created",
2626
"google.cloud.firestore.document.v1.updated",
2727
"google.cloud.firestore.document.v1.deleted",
28+
"google.cloud.firestore.document.v1.written.withAuthContext",
29+
"google.cloud.firestore.document.v1.created.withAuthContext",
30+
"google.cloud.firestore.document.v1.updated.withAuthContext",
31+
"google.cloud.firestore.document.v1.deleted.withAuthContext",
2832
] as const;
33+
export const FIRESTORE_EVENT_REGEX = /^google\.cloud\.firestore\.document\.v1\.[^\.]*$/;
34+
export const FIRESTORE_EVENT_WITH_AUTH_CONTEXT_REGEX =
35+
/^google\.cloud\.firestore\.document\.v1\..*\.withAuthContext$/;
2936

3037
export type Event =
3138
| typeof PUBSUB_PUBLISH_EVENT

src/test/deploy/functions/prompts.spec.ts

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,3 +416,117 @@ describe("promptForMinInstances", () => {
416416
expect(logStub.firstCall.args[1]).to.match(/Cannot calculate the minimum monthly bill/);
417417
});
418418
});
419+
420+
describe("promptForUnsafeMigration", () => {
421+
let promptStub: sinon.SinonStub;
422+
423+
beforeEach(() => {
424+
promptStub = sinon.stub(prompt, "promptOnce");
425+
});
426+
427+
afterEach(() => {
428+
promptStub.restore();
429+
});
430+
431+
const firestoreEventTrigger: backend.EventTrigger = {
432+
eventType: "google.cloud.firestore.document.v1.written",
433+
eventFilters: { namespace: "(default)", document: "messages/{id}" },
434+
retry: false,
435+
};
436+
const v2Endpoint0: backend.Endpoint = {
437+
platform: "gcfv2",
438+
id: "0",
439+
region: "us-central1",
440+
project: "a",
441+
entryPoint: "function",
442+
labels: {},
443+
environmentVariables: {},
444+
runtime: "nodejs18",
445+
eventTrigger: firestoreEventTrigger,
446+
};
447+
const v2Endpoint1: backend.Endpoint = {
448+
platform: "gcfv2",
449+
id: "1",
450+
region: "us-central1",
451+
project: "a",
452+
entryPoint: "function",
453+
labels: {},
454+
environmentVariables: {},
455+
runtime: "nodejs18",
456+
eventTrigger: firestoreEventTrigger,
457+
};
458+
459+
it("should prompt if there are potentially unsafe function updates", async () => {
460+
promptStub.resolves(false);
461+
const epUpdates = [
462+
{
463+
endpoint: v2Endpoint0,
464+
},
465+
{
466+
endpoint: v2Endpoint1,
467+
unsafe: true,
468+
},
469+
];
470+
471+
await functionPrompts.promptForUnsafeMigration(epUpdates, SAMPLE_OPTIONS);
472+
473+
expect(promptStub).to.have.been.calledOnce;
474+
});
475+
476+
it("should only keep function updates that have been confirmed by user", async () => {
477+
promptStub.onFirstCall().resolves(true);
478+
promptStub.onSecondCall().resolves(false);
479+
const epUpdates = [
480+
{
481+
endpoint: v2Endpoint0,
482+
unsafe: true,
483+
},
484+
{
485+
endpoint: v2Endpoint1,
486+
unsafe: true,
487+
},
488+
];
489+
490+
await expect(
491+
functionPrompts.promptForUnsafeMigration(epUpdates, SAMPLE_OPTIONS),
492+
).to.eventually.deep.equal([{ endpoint: v2Endpoint0, unsafe: true }]);
493+
});
494+
495+
it("should force unsafe function updates when flag is set", async () => {
496+
const epUpdates = [
497+
{
498+
endpoint: v2Endpoint0,
499+
unsafe: true,
500+
},
501+
{
502+
endpoint: v2Endpoint1,
503+
unsafe: true,
504+
},
505+
];
506+
const options = { ...SAMPLE_OPTIONS, force: true };
507+
508+
await expect(functionPrompts.promptForUnsafeMigration(epUpdates, options)).to.eventually.equal(
509+
epUpdates,
510+
);
511+
expect(promptStub).to.have.not.been.called;
512+
});
513+
514+
it("should not proceed with unsafe function updates in non-interactive mode", async () => {
515+
const epUpdates = [
516+
{
517+
endpoint: v2Endpoint0,
518+
unsafe: true,
519+
},
520+
{
521+
endpoint: v2Endpoint1,
522+
unsafe: false,
523+
},
524+
];
525+
const options = { ...SAMPLE_OPTIONS, nonInteractive: true };
526+
527+
await expect(
528+
functionPrompts.promptForUnsafeMigration(epUpdates, options),
529+
).to.eventually.deep.equal([{ endpoint: v2Endpoint1, unsafe: false }]);
530+
expect(promptStub).to.have.not.been.called;
531+
});
532+
});

0 commit comments

Comments
 (0)