Only prompt for failurePolicies if they are new. (#3164)

* Only prompt for failurePolicies if they are new. Fixes firebase/firebase-functions#798 We can now conditionally list functions earlier in the deploy pipeline if there are any functions with a failure policy in the current set of functions to deploy. Silence the prompt if all functions with failure policies already had failure policies. Also modifies prepare.ts to avoid redundantly calling listallfunctions later.
firebase · Mar 3, 2021 · 9895790 · 9895790
1 parent 7a3e9eb
commit 9895790
Show file tree

Hide file tree

Showing 5 changed files with 125 additions and 29 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1 +1,2 @@
+- Deploying a function with a retry policy will no longer prompt if the function already had a retry policy
 - Fixes issue where the Firebase Hosting emulator would fail to start with `--only` filters using targets (#2820).
diff --git a/src/deploy/functions/deploy.ts b/src/deploy/functions/deploy.ts
@@ -26,7 +26,8 @@ async function uploadSource(context: any): Promise<void> {
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 export async function deploy(context: any, options: any, payload: any): Promise<void> {
  if (options.config.get("functions")) {
- context.existingFunctions = await gcp.cloudfunctions.listAllFunctions(context.projectId);
+ context.existingFunctions =
+ context.existingFunctions || (await gcp.cloudfunctions.listAllFunctions(context.projectId));
 
  await checkHttpIam(context, options, payload);
 

diff --git a/src/deploy/functions/prepare.ts b/src/deploy/functions/prepare.ts
@@ -87,5 +87,5 @@ export async function prepare(context: any, options: any, payload: any): Promise
  const localFnsInRelease = payload.functions.triggers.filter((fn: CloudFunctionTrigger) => {
  return functionMatchesAnyGroup(fn.name, context.filters);
  });
- await promptForFailurePolicies(options, localFnsInRelease);
+ await promptForFailurePolicies(context, options, localFnsInRelease);
 }
diff --git a/src/deploy/functions/prompts.ts b/src/deploy/functions/prompts.ts
@@ -4,6 +4,7 @@ import { getFunctionLabel, getFunctionId, getRegion } from "../../functionsDeplo
 import { CloudFunctionTrigger } from "./deploymentPlanner";
 import { FirebaseError } from "../../error";
 import { promptOnce } from "../../prompt";
+import * as gcp from "../../gcp";
 import * as utils from "../../utils";
 import * as logger from "../../logger";
 
@@ -14,6 +15,7 @@ import * as logger from "../../logger";
  * @param functions A list of all functions in the deployment
  */
 export async function promptForFailurePolicies(
+ context: any,
  options: any,
  functions: CloudFunctionTrigger[]
 ): Promise<void> {
@@ -25,21 +27,47 @@ export async function promptForFailurePolicies(
  if (failurePolicyFunctions.length === 0) {
  return;
  }
- const failurePolicyFunctionLabels = failurePolicyFunctions.map((fn: CloudFunctionTrigger) => {
- return getFunctionLabel(fn.name);
+
+ context.existingFunctions =
+ (context.existingFunctions as CloudFunctionTrigger[]) ||
+ (await gcp.cloudfunctions.listAllFunctions(context.projectId));
+ const existingFailurePolicyFunctions = context.existingFunctions.filter(
+ (fn: CloudFunctionTrigger) => {
+ return !!fn?.eventTrigger?.failurePolicy;
+ }
+ );
+ const newFailurePolicyFunctions = failurePolicyFunctions.filter((fn: CloudFunctionTrigger) => {
+ for (const existing of existingFailurePolicyFunctions) {
+ if (existing.name === fn.name) {
+ return false;
+ }
+ }
+ return true;
  });
- const retryMessage =
- "The following functions will be retried in case of failure: " +
- clc.bold(failurePolicyFunctionLabels.join(", ")) +
+
+ if (newFailurePolicyFunctions.length == 0) {
+ return;
+ }
+
+ const newFailurePolicyFunctionLabels = newFailurePolicyFunctions.map(
+ (fn: CloudFunctionTrigger) => {
+ return getFunctionLabel(fn.name);
+ }
+ );
+
+ const warnMessage =
+ "The following functions will newly be retried in case of failure: " +
+ clc.bold(newFailurePolicyFunctionLabels.join(", ")) +
  ". " +
  "Retried executions are billed as any other execution, and functions are retried repeatedly until they either successfully execute or the maximum retry period has elapsed, which can be up to 7 days. " +
  "For safety, you might want to ensure that your functions are idempotent; see https://firebase.google.com/docs/functions/retries to learn more.";
 
- utils.logLabeledWarning("functions", retryMessage);
+ utils.logLabeledWarning("functions", warnMessage);
 
  if (options.force) {
  return;
- } else if (options.nonInteractive) {
+ }
+ if (options.nonInteractive) {
  throw new FirebaseError("Pass the --force option to deploy functions with a failure policy", {
  exit: 1,
  });

diff --git a/src/test/deploy/functions/prompts.spec.ts b/src/test/deploy/functions/prompts.spec.ts
@@ -5,19 +5,27 @@ import * as prompt from "../../../prompt";
 import * as functionPrompts from "../../../deploy/functions/prompts";
 import { FirebaseError } from "../../../error";
 import { CloudFunctionTrigger } from "../../../deploy/functions/deploymentPlanner";
+import * as gcp from "../../../gcp";
 
 describe("promptForFailurePolicies", () => {
  let promptStub: sinon.SinonStub;
+ let listAllFunctionsStub: sinon.SinonStub;
+ let existingFunctions: CloudFunctionTrigger[] = [];
 
  beforeEach(() => {
  promptStub = sinon.stub(prompt, "promptOnce");
+ listAllFunctionsStub = sinon.stub(gcp.cloudfunctions, "listAllFunctions").callsFake(() => {
+ return Promise.resolve(existingFunctions);
+ });
  });
 
  afterEach(() => {
  promptStub.restore();
+ listAllFunctionsStub.restore();
+ existingFunctions = [];
  });
 
- it("should prompt if there are any functions with failure policies", () => {
+ it("should prompt if there are new functions with failure policies", async () => {
  const funcs: CloudFunctionTrigger[] = [
  {
  name: "projects/a/locations/b/functions/c",
@@ -28,11 +36,71 @@ describe("promptForFailurePolicies", () => {
  },
  ];
  const options = {};
+ const context = {};
  promptStub.resolves(true);
 
- expect(
- async () => await functionPrompts.promptForFailurePolicies(options, funcs)
- ).not.to.throw();
+ await expect(functionPrompts.promptForFailurePolicies(context, options, funcs)).not.to.be
+ .rejected;
+ expect(promptStub).to.have.been.calledOnce;
+ });
+
+ it("should not prompt if all functions with failure policies already had failure policies", async () => {
+ // Note: local definitions of function triggers use a top-level "failurePolicy" but
+ // the API returns eventTrigger.failurePolicy.
+ const func: any = {
+ name: "projects/a/locations/b/functions/c",
+ entryPoint: "",
+ labels: {},
+ environmentVariables: {},
+ failurePolicy: {},
+ eventTrigger: {
+ failurePolicy: {},
+ },
+ };
+ existingFunctions = [func];
+ const options = {};
+ const context = {};
+
+ await expect(functionPrompts.promptForFailurePolicies(context, options, [func])).to.eventually
+ .be.fulfilled;
+ expect(promptStub).to.not.have.been.called;
+ });
+
+ it("should throw if user declines the prompt", async () => {
+ const funcs: CloudFunctionTrigger[] = [
+ {
+ name: "projects/a/locations/b/functions/c",
+ entryPoint: "",
+ labels: {},
+ environmentVariables: {},
+ failurePolicy: {},
+ },
+ ];
+ const options = {};
+ const context = {};
+ promptStub.resolves(false);
+
+ await expect(
+ functionPrompts.promptForFailurePolicies(context, options, funcs)
+ ).to.eventually.be.rejectedWith(FirebaseError, /Deployment canceled/);
+ expect(promptStub).to.have.been.calledOnce;
+ });
+
+ it("should propmt if an existing function adds a failure policy", async () => {
+ const func: CloudFunctionTrigger = {
+ name: "projects/a/locations/b/functions/c",
+ entryPoint: "",
+ labels: {},
+ environmentVariables: {},
+ };
+ existingFunctions = [func];
+ const newFunc = Object.assign({}, func, { failurePolicy: {} });
+ const options = {};
+ const context = {};
+ promptStub.resolves(true);
+
+ await expect(functionPrompts.promptForFailurePolicies(context, options, [newFunc])).to
+ .eventually.be.fulfilled;
  expect(promptStub).to.have.been.calledOnce;
  });
 
@@ -47,16 +115,16 @@ describe("promptForFailurePolicies", () => {
  },
  ];
  const options = {};
+ const context = {};
  promptStub.resolves(false);
 
- await expect(functionPrompts.promptForFailurePolicies(options, funcs)).to.be.rejectedWith(
- FirebaseError,
- /Deployment canceled/
- );
+ await expect(
+ functionPrompts.promptForFailurePolicies(context, options, funcs)
+ ).to.eventually.be.rejectedWith(FirebaseError, /Deployment canceled/);
  expect(promptStub).to.have.been.calledOnce;
  });
 
- it("should not prompt if there are no functions with failure policies", () => {
+ it("should not prompt if there are no functions with failure policies", async () => {
  const funcs: CloudFunctionTrigger[] = [
  {
  name: "projects/a/locations/b/functions/c",
@@ -66,11 +134,11 @@ describe("promptForFailurePolicies", () => {
  },
  ];
  const options = {};
+ const context = {};
  promptStub.resolves();
 
- expect(
- async () => await functionPrompts.promptForFailurePolicies(options, funcs)
- ).not.to.throw();
+ await expect(functionPrompts.promptForFailurePolicies(context, options, funcs)).to.eventually.be
+ .fulfilled;
  expect(promptStub).not.to.have.been.called;
  });
 
@@ -86,14 +154,13 @@ describe("promptForFailurePolicies", () => {
  ];
  const options = { nonInteractive: true };
 
- await expect(functionPrompts.promptForFailurePolicies(options, funcs)).to.be.rejectedWith(
- FirebaseError,
- /--force option/
- );
+ await expect(
+ functionPrompts.promptForFailurePolicies(context, options, funcs)
+ ).to.be.rejectedWith(FirebaseError, /--force option/);
  expect(promptStub).not.to.have.been.called;
  });
 
- it("should not throw if there are any functions with failure policies, in noninteractive mode, with the force flag set", () => {
+ it("should not throw if there are any functions with failure policies, in noninteractive mode, with the force flag set", async () => {
  const funcs: CloudFunctionTrigger[] = [
  {
  name: "projects/a/locations/b/functions/c",
@@ -105,9 +172,8 @@ describe("promptForFailurePolicies", () => {
  ];
  const options = { nonInteractive: true, force: true };
 
- expect(
- async () => await functionPrompts.promptForFailurePolicies(options, funcs)
- ).not.to.throw();
+ await expect(functionPrompts.promptForFailurePolicies(context, options, funcs)).to.eventually.be
+ .fulfilled;
  expect(promptStub).not.to.have.been.called;
  });
 });