Skip to content

Commit

Permalink
Add support for deploying 2nd gen Firestore auth context triggers (#6734
Browse files Browse the repository at this point in the history
)

* track new firestore auth event types

* clean up unit tests

* changelog

* run new formatter

* address feedback

* format and fix unit tests
  • Loading branch information
blidd-google committed Apr 4, 2024
1 parent c92c91a commit e1aa2bf
Show file tree
Hide file tree
Showing 8 changed files with 257 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
73 changes: 65 additions & 8 deletions src/deploy/functions/prompts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -75,19 +76,18 @@ export async function promptForFailurePolicies(
*/
export async function promptForFunctionDeletion(
functionsToDelete: (backend.TargetIds & { platform: backend.FunctionsPlatform })[],
force: boolean,
nonInteractive: boolean,
options: Options,
): Promise<boolean> {
let shouldDeleteFns = true;
if (functionsToDelete.length === 0 || force) {
if (functionsToDelete.length === 0 || options.force) {
return true;
}
const deleteList = functionsToDelete
.sort(backend.compareFunctions)
.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;
Expand All @@ -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.",
Expand All @@ -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<EndpointUpdate[]> {
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:
Expand Down
21 changes: 16 additions & 5 deletions src/deploy/functions/release/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
16 changes: 16 additions & 0 deletions src/deploy/functions/release/planner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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) ||
Expand Down Expand Up @@ -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 => {
Expand Down
4 changes: 4 additions & 0 deletions src/deploy/functions/services/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,10 @@ const EVENT_SERVICE_MAPPING: Record<events.Event, Service> = {
"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,
};

/**
Expand Down
7 changes: 7 additions & 0 deletions src/functions/events/v2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
114 changes: 114 additions & 0 deletions src/test/deploy/functions/prompts.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
});

0 comments on commit e1aa2bf

Please sign in to comment.