BYO bucket for v2 functions uploads with runfunctions enabled

src/deploy/apphosting/args.ts

@@ -1,7 +1,7 @@
 import { AppHostingSingle } from "../../firebaseConfig";
 
 export interface Context {
-  backendConfigs: Map<string, AppHostingSingle>;
-  backendLocations: Map<string, string>;
-  backendStorageUris: Map<string, string>;
+  backendConfigs: Record<string, AppHostingSingle>;
+  backendLocations: Record<string, string>;
+  backendStorageUris: Record<string, string>;
 }

src/deploy/apphosting/deploy.spec.ts

@@ -1,8 +1,6 @@
 import { expect } from "chai";
 import * as sinon from "sinon";
 import { Config } from "../../config";
-import { FirebaseError } from "../../error";
-import { AppHostingSingle } from "../../firebaseConfig";
 import * as gcs from "../../gcp/storage";
 import { RC } from "../../rc";
 import { Context } from "./args";
@@ -26,24 +24,20 @@ const BASE_OPTS = {
 
 function initializeContext(): Context {
   return {
-    backendConfigs: new Map<string, AppHostingSingle>([
-      [
-        "foo",
-        {
-          backendId: "foo",
-          rootDir: "/",
-          ignore: [],
-        },
-      ],
-    ]),
-    backendLocations: new Map<string, string>([["foo", "us-central1"]]),
-    backendStorageUris: new Map<string, string>(),
+    backendConfigs: {
+      foo: {
+        backendId: "foo",
+        rootDir: "/",
+        ignore: [],
+      },
+    },
+    backendLocations: { foo: "us-central1" },
+    backendStorageUris: {},
   };
 }
 
 describe("apphosting", () => {
-  let getBucketStub: sinon.SinonStub;
-  let createBucketStub: sinon.SinonStub;
+  let upsertBucketStub: sinon.SinonStub;
   let uploadObjectStub: sinon.SinonStub;
   let createArchiveStub: sinon.SinonStub;
   let createReadStreamStub: sinon.SinonStub;
@@ -53,8 +47,7 @@ describe("apphosting", () => {
     getProjectNumberStub = sinon
       .stub(getProjectNumber, "getProjectNumber")
       .throws("Unexpected getProjectNumber call");
-    getBucketStub = sinon.stub(gcs, "getBucket").throws("Unexpected getBucket call");
-    createBucketStub = sinon.stub(gcs, "createBucket").throws("Unexpected createBucket call");
+    upsertBucketStub = sinon.stub(gcs, "upsertBucket").throws("Unexpected upsertBucket call");
     uploadObjectStub = sinon.stub(gcs, "uploadObject").throws("Unexpected uploadObject call");
     createArchiveStub = sinon.stub(util, "createArchive").throws("Unexpected createArchive call");
     createReadStreamStub = sinon
@@ -80,15 +73,10 @@ describe("apphosting", () => {
       }),
     };
 
-    it("creates regional GCS bucket if one doesn't exist yet", async () => {
+    it("upserts regional GCS bucket", async () => {
       const context = initializeContext();
       getProjectNumberStub.resolves("000000000000");
-      getBucketStub.onFirstCall().rejects(
-        new FirebaseError("error", {
-          original: new FirebaseError("original error", { status: 404 }),
-        }),
-      );
-      createBucketStub.resolves();
+      upsertBucketStub.resolves();
       createArchiveStub.resolves({
         projectSourcePath: "my-project/",
         zippedSourcePath: "path/to/foo-1234.zip",
@@ -101,14 +89,30 @@ describe("apphosting", () => {
 
       await deploy(context, opts);
 
-      expect(createBucketStub).to.be.calledOnce;
+      expect(upsertBucketStub).to.be.calledWith({
+        product: "apphosting",
+        createMessage:
+          "Creating Cloud Storage bucket in us-central1 to store App Hosting source code uploads at firebaseapphosting-sources-000000000000-us-central1...",
+        projectId: "my-project",
+        req: {
+          name: "firebaseapphosting-sources-000000000000-us-central1",
+          location: "us-central1",
+          lifecycle: {
+            rule: [
+              {
+                action: { type: "Delete" },
+                condition: { age: 30 },
+              },
+            ],
+          },
+        },
+      });
     });
 
     it("correctly creates and sets storage URIs", async () => {
       const context = initializeContext();
       getProjectNumberStub.resolves("000000000000");
-      getBucketStub.resolves();
-      createBucketStub.resolves();
+      upsertBucketStub.resolves();
       createArchiveStub.resolves({
         projectSourcePath: "my-project/",
         zippedSourcePath: "path/to/foo-1234.zip",
@@ -121,7 +125,7 @@ describe("apphosting", () => {
 
       await deploy(context, opts);
 
-      expect(context.backendStorageUris.get("foo")).to.equal(
+      expect(context.backendStorageUris["foo"]).to.equal(
         "gs://firebaseapphosting-sources-000000000000-us-central1/foo-1234.zip",
       );
     });

src/deploy/apphosting/deploy.ts

@@ -1,11 +1,11 @@
 import * as fs from "fs";
 import * as path from "path";
-import { FirebaseError, getErrStatus } from "../../error";
+import { FirebaseError } from "../../error";
 import * as gcs from "../../gcp/storage";
 import { getProjectNumber } from "../../getProjectNumber";
 import { Options } from "../../options";
 import { needProjectId } from "../../projectUtils";
-import { logLabeledBullet, logLabeledWarning } from "../../utils";
+import { logLabeledBullet } from "../../utils";
 import { Context } from "./args";
 import { createArchive } from "./util";
 
@@ -14,7 +14,7 @@ import { createArchive } from "./util";
  * build and deployment. Creates storage buckets if necessary.
  */
 export default async function (context: Context, options: Options): Promise<void> {
-  if (context.backendConfigs.size === 0) {
+  if (Object.entries(context.backendConfigs).length === 0) {
     return;
   }
   const projectId = needProjectId(options);
@@ -24,77 +24,57 @@ export default async function (context: Context, options: Options): Promise<void
   }
 
   // Ensure that a bucket exists in each region that a backend is or will be deployed to
-  for (const loc of context.backendLocations.values()) {
-    const bucketName = `firebaseapphosting-sources-${options.projectNumber}-${loc.toLowerCase()}`;
-    try {
-      await gcs.getBucket(bucketName);
-    } catch (err) {
-      const errStatus = getErrStatus((err as FirebaseError).original);
-      // Unfortunately, requests for a non-existent bucket from the GCS API sometimes return 403 responses as well as 404s.
-      // We must attempt to create a new bucket on both 403s and 404s.
-      if (errStatus === 403 || errStatus === 404) {
-        logLabeledBullet(
-          "apphosting",
-          `Creating Cloud Storage bucket in ${loc} to store App Hosting source code uploads at ${bucketName}...`,
-        );
-        try {
-          await gcs.createBucket(projectId, {
-            name: bucketName,
-            location: loc,
-            lifecycle: {
-              rule: [
-                {
-                  action: {
-                    type: "Delete",
-                  },
-                  condition: {
-                    age: 30,
-                  },
+  await Promise.all(
+    Object.values(context.backendLocations).map(async (loc) => {
+      const bucketName = `firebaseapphosting-sources-${options.projectNumber}-${loc.toLowerCase()}`;
+      await gcs.upsertBucket({
+        product: "apphosting",
+        createMessage: `Creating Cloud Storage bucket in ${loc} to store App Hosting source code uploads at ${bucketName}...`,
+        projectId,
+        req: {
+          name: bucketName,
+          location: loc,
+          lifecycle: {
+            rule: [
+              {
+                action: {
+                  type: "Delete",
                 },
-              ],
-            },
-          });
-        } catch (err) {
-          if (getErrStatus((err as FirebaseError).original) === 403) {
-            logLabeledWarning(
-              "apphosting",
-              "Failed to create Cloud Storage bucket because user does not have sufficient permissions. " +
-                "See https://cloud.google.com/storage/docs/access-control/iam-roles for more details on " +
-                "IAM roles that are able to create a Cloud Storage bucket, and ask your project administrator " +
-                "to grant you one of those roles.",
-            );
-            throw (err as FirebaseError).original;
-          }
-        }
-      } else {
-        throw err;
-      }
-    }
-  }
+                condition: {
+                  age: 30,
+                },
+              },
+            ],
+          },
+        },
+      });
+    }),
+  );
 
-  for (const cfg of context.backendConfigs.values()) {
-    const { projectSourcePath, zippedSourcePath } = await createArchive(cfg, options.projectRoot);
-    const backendLocation = context.backendLocations.get(cfg.backendId);
-    if (!backendLocation) {
-      throw new FirebaseError(
-        `Failed to find location for backend ${cfg.backendId}. Please contact support with the contents of your firebase-debug.log to report your issue.`,
+  await Promise.all(
+    Object.values(context.backendConfigs).map(async (cfg) => {
+      const { projectSourcePath, zippedSourcePath } = await createArchive(cfg, options.projectRoot);
+      const backendLocation = context.backendLocations[cfg.backendId];
+      if (!backendLocation) {
+        throw new FirebaseError(
+          `Failed to find location for backend ${cfg.backendId}. Please contact support with the contents of your firebase-debug.log to report your issue.`,
+        );
+      }
+      logLabeledBullet(
+        "apphosting",
+        `Uploading source code at ${projectSourcePath} for backend ${cfg.backendId}...`,
       );
-    }
-    logLabeledBullet(
-      "apphosting",
-      `Uploading source code at ${projectSourcePath} for backend ${cfg.backendId}...`,
-    );
-    const { bucket, object } = await gcs.uploadObject(
-      {
-        file: zippedSourcePath,
-        stream: fs.createReadStream(zippedSourcePath),
-      },
-      `firebaseapphosting-sources-${options.projectNumber}-${backendLocation.toLowerCase()}`,
-    );
-    logLabeledBullet("apphosting", `Source code uploaded at gs://${bucket}/${object}`);
-    context.backendStorageUris.set(
-      cfg.backendId,
-      `gs://firebaseapphosting-sources-${options.projectNumber}-${backendLocation.toLowerCase()}/${path.basename(zippedSourcePath)}`,
-    );
-  }
+      const bucketName = `firebaseapphosting-sources-${options.projectNumber}-${backendLocation.toLowerCase()}`;
+      const { bucket, object } = await gcs.uploadObject(
+        {
+          file: zippedSourcePath,
+          stream: fs.createReadStream(zippedSourcePath),
+        },
+        bucketName,
+      );
+      logLabeledBullet("apphosting", `Source code uploaded at gs://${bucket}/${object}`);
+      context.backendStorageUris[cfg.backendId] =
+        `gs://${bucketName}/${path.basename(zippedSourcePath)}`;
+    }),
+  );
 }

src/deploy/apphosting/prepare.spec.ts

@@ -3,7 +3,6 @@ import * as sinon from "sinon";
 import * as backend from "../../apphosting/backend";
 import { Config } from "../../config";
 import * as apiEnabled from "../../ensureApiEnabled";
-import { AppHostingSingle } from "../../firebaseConfig";
 import * as apphosting from "../../gcp/apphosting";
 import * as devconnect from "../../gcp/devConnect";
 import * as prompt from "../../prompt";
@@ -26,9 +25,9 @@ const BASE_OPTS = {
 
 function initializeContext(): Context {
   return {
-    backendConfigs: new Map<string, AppHostingSingle>(),
-    backendLocations: new Map<string, string>(),
-    backendStorageUris: new Map<string, string>(),
+    backendConfigs: {},
+    backendLocations: {},
+    backendStorageUris: {},
   };
 }
 
@@ -86,8 +85,8 @@ describe("apphosting", () => {
 
       await prepare(context, opts);
 
-      expect(context.backendLocations.get("foo")).to.equal("us-central1");
-      expect(context.backendConfigs.get("foo")).to.deep.equal({
+      expect(context.backendLocations["foo"]).to.equal("us-central1");
+      expect(context.backendConfigs["foo"]).to.deep.equal({
         backendId: "foo",
         rootDir: "/",
         ignore: [],
@@ -106,8 +105,8 @@ describe("apphosting", () => {
       await prepare(context, opts);
 
       expect(doSetupSourceDeployStub).to.be.calledWith("my-project", "foo");
-      expect(context.backendLocations.get("foo")).to.equal("us-central1");
-      expect(context.backendConfigs.get("foo")).to.deep.equal({
+      expect(context.backendLocations["foo"]).to.equal("us-central1");
+      expect(context.backendConfigs["foo"]).to.deep.equal({
         backendId: "foo",
         rootDir: "/",
         ignore: [],
@@ -140,8 +139,8 @@ describe("apphosting", () => {
 
       await prepare(context, optsWithAlwaysDeploy);
 
-      expect(context.backendLocations.get("foo")).to.equal(undefined);
-      expect(context.backendConfigs.get("foo")).to.deep.equal(undefined);
+      expect(context.backendLocations["foo"]).to.be.undefined;
+      expect(context.backendConfigs["foo"]).to.be.undefined;
     });
 
     it("prompts user if codebase is already connected and alwaysDeployFromSource is undefined", async () => {
@@ -164,8 +163,8 @@ describe("apphosting", () => {
 
       await prepare(context, opts);
 
-      expect(context.backendLocations.get("foo")).to.equal("us-central1");
-      expect(context.backendConfigs.get("foo")).to.deep.equal({
+      expect(context.backendLocations["foo"]).to.equal("us-central1");
+      expect(context.backendConfigs["foo"]).to.deep.equal({
         backendId: "foo",
         rootDir: "/",
         ignore: [],

src/deploy/apphosting/prepare.ts

@@ -23,9 +23,9 @@ export default async function (context: Context, options: Options): Promise<void
   await ensureRequiredApisEnabled(projectId);
   await ensureAppHostingComputeServiceAccount(projectId, /* serviceAccount= */ "");
 
-  context.backendConfigs = new Map<string, AppHostingSingle>();
-  context.backendLocations = new Map<string, string>();
-  context.backendStorageUris = new Map<string, string>();
+  context.backendConfigs = {};
+  context.backendLocations = {};
+  context.backendStorageUris = {};
 
   const configs = getBackendConfigs(options);
   const { backends } = await listBackends(projectId, "-");
@@ -101,8 +101,8 @@ export default async function (context: Context, options: Options): Promise<void
         }
       }
     }
-    context.backendConfigs.set(cfg.backendId, cfg);
-    context.backendLocations.set(cfg.backendId, location);
+    context.backendConfigs[cfg.backendId] = cfg;
+    context.backendLocations[cfg.backendId] = location;
   }
 
   if (notFoundBackends.length > 0) {
@@ -131,8 +131,8 @@ export default async function (context: Context, options: Options): Promise<void
       for (const cfg of selectedBackends) {
         logLabeledBullet("apphosting", `Creating a new backend ${cfg.backendId}...`);
         const { location } = await doSetupSourceDeploy(projectId, cfg.backendId);
-        context.backendConfigs.set(cfg.backendId, cfg);
-        context.backendLocations.set(cfg.backendId, location);
+        context.backendConfigs[cfg.backendId] = cfg;
+        context.backendLocations[cfg.backendId] = location;
       }
     } else {
       skippedBackends.push(...notFoundBackends);