Skip to content

Commit 3ee5150

Browse files
inlinedinlined
authored
BYO bucket for v2 functions uploads with runfunctions enabled (#8980)
* BYO bucket for v2 functions uploads with runfunctions enabled * update comments * Reconcile FAH & CF3 upsert bucket. Fix Map usage in FAH * PR feedback * PR feedback * PR feedback I failed to push earlier * Fix problematic merge
1 parent 99870a7 commit 3ee5150

File tree

13 files changed

+529
-196
lines changed

13 files changed

+529
-196
lines changed

‎src/apphosting/secrets/dialogs.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ describe("dialogs", () => {
9292
]);
9393
});
9494

95-
it("uses 'service accounts' header if any backend uses more than one service accont", async () => {
95+
it("uses 'service accounts' header if any backend uses more than one service account", async () => {
9696
const table = dialogs.tableForBackends(await dialogs.toMetadata("number", [legacy, modernA]));
9797
const legacyAccounts = await secrets.serviceAccountsForBackend("number", legacy);
9898
expect(table[0]).to.deep.equal(["location", "backend", "service accounts"]);

‎src/deploy/apphosting/args.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { AppHostingSingle } from "../../firebaseConfig";
22

33
export interface Context {
4-
backendConfigs: Map<string, AppHostingSingle>;
5-
backendLocations: Map<string, string>;
6-
backendStorageUris: Map<string, string>;
4+
backendConfigs: Record<string, AppHostingSingle>;
5+
backendLocations: Record<string, string>;
6+
backendStorageUris: Record<string, string>;
77
}

‎src/deploy/apphosting/deploy.spec.ts‎

Lines changed: 33 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
import { expect } from "chai";
22
import * as sinon from "sinon";
33
import { Config } from "../../config";
4-
import { FirebaseError } from "../../error";
5-
import { AppHostingSingle } from "../../firebaseConfig";
64
import * as gcs from "../../gcp/storage";
75
import { RC } from "../../rc";
86
import { Context } from "./args";
@@ -26,24 +24,20 @@ const BASE_OPTS = {
2624

2725
function initializeContext(): Context {
2826
return {
29-
backendConfigs: new Map<string, AppHostingSingle>([
30-
[
31-
"foo",
32-
{
33-
backendId: "foo",
34-
rootDir: "/",
35-
ignore: [],
36-
},
37-
],
38-
]),
39-
backendLocations: new Map<string, string>([["foo", "us-central1"]]),
40-
backendStorageUris: new Map<string, string>(),
27+
backendConfigs: {
28+
foo: {
29+
backendId: "foo",
30+
rootDir: "/",
31+
ignore: [],
32+
},
33+
},
34+
backendLocations: { foo: "us-central1" },
35+
backendStorageUris: {},
4136
};
4237
}
4338

4439
describe("apphosting", () => {
45-
let getBucketStub: sinon.SinonStub;
46-
let createBucketStub: sinon.SinonStub;
40+
let upsertBucketStub: sinon.SinonStub;
4741
let uploadObjectStub: sinon.SinonStub;
4842
let createArchiveStub: sinon.SinonStub;
4943
let createReadStreamStub: sinon.SinonStub;
@@ -53,8 +47,7 @@ describe("apphosting", () => {
5347
getProjectNumberStub = sinon
5448
.stub(getProjectNumber, "getProjectNumber")
5549
.throws("Unexpected getProjectNumber call");
56-
getBucketStub = sinon.stub(gcs, "getBucket").throws("Unexpected getBucket call");
57-
createBucketStub = sinon.stub(gcs, "createBucket").throws("Unexpected createBucket call");
50+
upsertBucketStub = sinon.stub(gcs, "upsertBucket").throws("Unexpected upsertBucket call");
5851
uploadObjectStub = sinon.stub(gcs, "uploadObject").throws("Unexpected uploadObject call");
5952
createArchiveStub = sinon.stub(util, "createArchive").throws("Unexpected createArchive call");
6053
createReadStreamStub = sinon
@@ -80,15 +73,10 @@ describe("apphosting", () => {
8073
}),
8174
};
8275

83-
it("creates regional GCS bucket if one doesn't exist yet", async () => {
76+
it("upserts regional GCS bucket", async () => {
8477
const context = initializeContext();
8578
getProjectNumberStub.resolves("000000000000");
86-
getBucketStub.onFirstCall().rejects(
87-
new FirebaseError("error", {
88-
original: new FirebaseError("original error", { status: 404 }),
89-
}),
90-
);
91-
createBucketStub.resolves();
79+
upsertBucketStub.resolves();
9280
createArchiveStub.resolves("path/to/foo-1234.zip");
9381
uploadObjectStub.resolves({
9482
bucket: "firebaseapphosting-sources-12345678-us-central1",
@@ -98,14 +86,30 @@ describe("apphosting", () => {
9886

9987
await deploy(context, opts);
10088

101-
expect(createBucketStub).to.be.calledOnce;
89+
expect(upsertBucketStub).to.be.calledWith({
90+
product: "apphosting",
91+
createMessage:
92+
"Creating Cloud Storage bucket in us-central1 to store App Hosting source code uploads at firebaseapphosting-sources-000000000000-us-central1...",
93+
projectId: "my-project",
94+
req: {
95+
name: "firebaseapphosting-sources-000000000000-us-central1",
96+
location: "us-central1",
97+
lifecycle: {
98+
rule: [
99+
{
100+
action: { type: "Delete" },
101+
condition: { age: 30 },
102+
},
103+
],
104+
},
105+
},
106+
});
102107
});
103108

104109
it("correctly creates and sets storage URIs", async () => {
105110
const context = initializeContext();
106111
getProjectNumberStub.resolves("000000000000");
107-
getBucketStub.resolves();
108-
createBucketStub.resolves();
112+
upsertBucketStub.resolves();
109113
createArchiveStub.resolves("path/to/foo-1234.zip");
110114
uploadObjectStub.resolves({
111115
bucket: "firebaseapphosting-sources-12345678-us-central1",
@@ -115,7 +119,7 @@ describe("apphosting", () => {
115119

116120
await deploy(context, opts);
117121

118-
expect(context.backendStorageUris.get("foo")).to.equal(
122+
expect(context.backendStorageUris["foo"]).to.equal(
119123
"gs://firebaseapphosting-sources-000000000000-us-central1/foo-1234.zip",
120124
);
121125
});

‎src/deploy/apphosting/deploy.ts‎

Lines changed: 54 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
import * as fs from "fs";
22
import * as path from "path";
3-
import { FirebaseError, getErrStatus } from "../../error";
3+
import { FirebaseError } from "../../error";
44
import * as gcs from "../../gcp/storage";
55
import { getProjectNumber } from "../../getProjectNumber";
66
import { Options } from "../../options";
77
import { needProjectId } from "../../projectUtils";
8-
import { logLabeledBullet, logLabeledWarning } from "../../utils";
8+
import { logLabeledBullet } from "../../utils";
99
import { Context } from "./args";
1010
import { createArchive } from "./util";
1111

@@ -14,7 +14,7 @@ import { createArchive } from "./util";
1414
* build and deployment. Creates storage buckets if necessary.
1515
*/
1616
export default async function (context: Context, options: Options): Promise<void> {
17-
if (context.backendConfigs.size === 0) {
17+
if (Object.entries(context.backendConfigs).length === 0) {
1818
return;
1919
}
2020
const projectId = needProjectId(options);
@@ -24,78 +24,58 @@ export default async function (context: Context, options: Options): Promise<void
2424
}
2525

2626
// Ensure that a bucket exists in each region that a backend is or will be deployed to
27-
for (const loc of context.backendLocations.values()) {
28-
const bucketName = `firebaseapphosting-sources-${options.projectNumber}-${loc.toLowerCase()}`;
29-
try {
30-
await gcs.getBucket(bucketName);
31-
} catch (err) {
32-
const errStatus = getErrStatus((err as FirebaseError).original);
33-
// Unfortunately, requests for a non-existent bucket from the GCS API sometimes return 403 responses as well as 404s.
34-
// We must attempt to create a new bucket on both 403s and 404s.
35-
if (errStatus === 403 || errStatus === 404) {
36-
logLabeledBullet(
37-
"apphosting",
38-
`Creating Cloud Storage bucket in ${loc} to store App Hosting source code uploads at ${bucketName}...`,
39-
);
40-
try {
41-
await gcs.createBucket(projectId, {
42-
name: bucketName,
43-
location: loc,
44-
lifecycle: {
45-
rule: [
46-
{
47-
action: {
48-
type: "Delete",
49-
},
50-
condition: {
51-
age: 30,
52-
},
27+
await Promise.all(
28+
Object.values(context.backendLocations).map(async (loc) => {
29+
const bucketName = `firebaseapphosting-sources-${options.projectNumber}-${loc.toLowerCase()}`;
30+
await gcs.upsertBucket({
31+
product: "apphosting",
32+
createMessage: `Creating Cloud Storage bucket in ${loc} to store App Hosting source code uploads at ${bucketName}...`,
33+
projectId,
34+
req: {
35+
name: bucketName,
36+
location: loc,
37+
lifecycle: {
38+
rule: [
39+
{
40+
action: {
41+
type: "Delete",
5342
},
54-
],
55-
},
56-
});
57-
} catch (err) {
58-
if (getErrStatus((err as FirebaseError).original) === 403) {
59-
logLabeledWarning(
60-
"apphosting",
61-
"Failed to create Cloud Storage bucket because user does not have sufficient permissions. " +
62-
"See https://cloud.google.com/storage/docs/access-control/iam-roles for more details on " +
63-
"IAM roles that are able to create a Cloud Storage bucket, and ask your project administrator " +
64-
"to grant you one of those roles.",
65-
);
66-
throw (err as FirebaseError).original;
67-
}
68-
}
69-
} else {
70-
throw err;
71-
}
72-
}
73-
}
43+
condition: {
44+
age: 30,
45+
},
46+
},
47+
],
48+
},
49+
},
50+
});
51+
}),
52+
);
7453

75-
for (const cfg of context.backendConfigs.values()) {
76-
const projectSourcePath = options.projectRoot ? options.projectRoot : process.cwd();
77-
const zippedSourcePath = await createArchive(cfg, projectSourcePath);
78-
const backendLocation = context.backendLocations.get(cfg.backendId);
79-
if (!backendLocation) {
80-
throw new FirebaseError(
81-
`Failed to find location for backend ${cfg.backendId}. Please contact support with the contents of your firebase-debug.log to report your issue.`,
54+
await Promise.all(
55+
Object.values(context.backendConfigs).map(async (cfg) => {
56+
const projectSourcePath = options.projectRoot ? options.projectRoot : process.cwd();
57+
const zippedSourcePath = await createArchive(cfg, projectSourcePath);
58+
const backendLocation = context.backendLocations[cfg.backendId];
59+
if (!backendLocation) {
60+
throw new FirebaseError(
61+
`Failed to find location for backend ${cfg.backendId}. Please contact support with the contents of your firebase-debug.log to report your issue.`,
62+
);
63+
}
64+
logLabeledBullet(
65+
"apphosting",
66+
`Uploading source code at ${projectSourcePath} for backend ${cfg.backendId}...`,
8267
);
83-
}
84-
logLabeledBullet(
85-
"apphosting",
86-
`Uploading source code at ${projectSourcePath} for backend ${cfg.backendId}...`,
87-
);
88-
const { bucket, object } = await gcs.uploadObject(
89-
{
90-
file: zippedSourcePath,
91-
stream: fs.createReadStream(zippedSourcePath),
92-
},
93-
`firebaseapphosting-sources-${options.projectNumber}-${backendLocation.toLowerCase()}`,
94-
);
95-
logLabeledBullet("apphosting", `Source code uploaded at gs://${bucket}/${object}`);
96-
context.backendStorageUris.set(
97-
cfg.backendId,
98-
`gs://firebaseapphosting-sources-${options.projectNumber}-${backendLocation.toLowerCase()}/${path.basename(zippedSourcePath)}`,
99-
);
100-
}
68+
const bucketName = `firebaseapphosting-sources-${options.projectNumber}-${backendLocation.toLowerCase()}`;
69+
const { bucket, object } = await gcs.uploadObject(
70+
{
71+
file: zippedSourcePath,
72+
stream: fs.createReadStream(zippedSourcePath),
73+
},
74+
bucketName,
75+
);
76+
logLabeledBullet("apphosting", `Source code uploaded at gs://${bucket}/${object}`);
77+
context.backendStorageUris[cfg.backendId] =
78+
`gs://${bucketName}/${path.basename(zippedSourcePath)}`;
79+
}),
80+
);
10181
}

‎src/deploy/apphosting/prepare.spec.ts‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import * as sinon from "sinon";
33
import * as backend from "../../apphosting/backend";
44
import { Config } from "../../config";
55
import * as apiEnabled from "../../ensureApiEnabled";
6-
import { AppHostingSingle } from "../../firebaseConfig";
76
import * as apphosting from "../../gcp/apphosting";
87
import * as devconnect from "../../gcp/devConnect";
98
import * as prompt from "../../prompt";
@@ -26,9 +25,9 @@ const BASE_OPTS = {
2625

2726
function initializeContext(): Context {
2827
return {
29-
backendConfigs: new Map<string, AppHostingSingle>(),
30-
backendLocations: new Map<string, string>(),
31-
backendStorageUris: new Map<string, string>(),
28+
backendConfigs: {},
29+
backendLocations: {},
30+
backendStorageUris: {},
3231
};
3332
}
3433

@@ -86,8 +85,8 @@ describe("apphosting", () => {
8685

8786
await prepare(context, opts);
8887

89-
expect(context.backendLocations.get("foo")).to.equal("us-central1");
90-
expect(context.backendConfigs.get("foo")).to.deep.equal({
88+
expect(context.backendLocations["foo"]).to.equal("us-central1");
89+
expect(context.backendConfigs["foo"]).to.deep.equal({
9190
backendId: "foo",
9291
rootDir: "/",
9392
ignore: [],
@@ -106,8 +105,8 @@ describe("apphosting", () => {
106105
await prepare(context, opts);
107106

108107
expect(doSetupSourceDeployStub).to.be.calledWith("my-project", "foo");
109-
expect(context.backendLocations.get("foo")).to.equal("us-central1");
110-
expect(context.backendConfigs.get("foo")).to.deep.equal({
108+
expect(context.backendLocations["foo"]).to.equal("us-central1");
109+
expect(context.backendConfigs["foo"]).to.deep.equal({
111110
backendId: "foo",
112111
rootDir: "/",
113112
ignore: [],
@@ -140,8 +139,8 @@ describe("apphosting", () => {
140139

141140
await prepare(context, optsWithAlwaysDeploy);
142141

143-
expect(context.backendLocations.get("foo")).to.equal(undefined);
144-
expect(context.backendConfigs.get("foo")).to.deep.equal(undefined);
142+
expect(context.backendLocations["foo"]).to.be.undefined;
143+
expect(context.backendConfigs["foo"]).to.be.undefined;
145144
});
146145

147146
it("prompts user if codebase is already connected and alwaysDeployFromSource is undefined", async () => {
@@ -164,8 +163,8 @@ describe("apphosting", () => {
164163

165164
await prepare(context, opts);
166165

167-
expect(context.backendLocations.get("foo")).to.equal("us-central1");
168-
expect(context.backendConfigs.get("foo")).to.deep.equal({
166+
expect(context.backendLocations["foo"]).to.equal("us-central1");
167+
expect(context.backendConfigs["foo"]).to.deep.equal({
169168
backendId: "foo",
170169
rootDir: "/",
171170
ignore: [],

‎src/deploy/apphosting/prepare.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ export default async function (context: Context, options: Options): Promise<void
2323
await ensureRequiredApisEnabled(projectId);
2424
await ensureAppHostingComputeServiceAccount(projectId, /* serviceAccount= */ "");
2525

26-
context.backendConfigs = new Map<string, AppHostingSingle>();
27-
context.backendLocations = new Map<string, string>();
28-
context.backendStorageUris = new Map<string, string>();
26+
context.backendConfigs = {};
27+
context.backendLocations = {};
28+
context.backendStorageUris = {};
2929

3030
const configs = getBackendConfigs(options);
3131
const { backends } = await listBackends(projectId, "-");
@@ -101,8 +101,8 @@ export default async function (context: Context, options: Options): Promise<void
101101
}
102102
}
103103
}
104-
context.backendConfigs.set(cfg.backendId, cfg);
105-
context.backendLocations.set(cfg.backendId, location);
104+
context.backendConfigs[cfg.backendId] = cfg;
105+
context.backendLocations[cfg.backendId] = location;
106106
}
107107

108108
if (notFoundBackends.length > 0) {
@@ -131,8 +131,8 @@ export default async function (context: Context, options: Options): Promise<void
131131
for (const cfg of selectedBackends) {
132132
logLabeledBullet("apphosting", `Creating a new backend ${cfg.backendId}...`);
133133
const { location } = await doSetupSourceDeploy(projectId, cfg.backendId);
134-
context.backendConfigs.set(cfg.backendId, cfg);
135-
context.backendLocations.set(cfg.backendId, location);
134+
context.backendConfigs[cfg.backendId] = cfg;
135+
context.backendLocations[cfg.backendId] = location;
136136
}
137137
} else {
138138
skippedBackends.push(...notFoundBackends);

0 commit comments

Comments
 (0)