Skip to content

Add support for deploying 2nd gen Firestore auth context triggers #6734

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Apr 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: In tests files, best practice is to use whitespace to split the test into steup, act, assert. For example (you can omit the comments when actually doing this):

  // Setup
   promptStub.resolves(false);
    const epUpdates = [
      {
        endpoint: v2Endpoint0,
      },
      {
        endpoint: v2Endpoint1,
        unsafe: true,
      },
    ];

    // Act
    await functionPrompts.promptForUnsafeMigration(epUpdates, SAMPLE_OPTIONS);

   // Assert
    expect(promptStub).to.have.been.calledOnce;

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;
});
});
Loading