Skip to content

Commit a8a69c8

Browse files
pavelgjpavelgj
committed
Addressed review feedback
1 parent dd93757 commit a8a69c8

File tree

3 files changed

+94
-99
lines changed

3 files changed

+94
-99
lines changed

‎src/commands/ext-update.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { displayNode10UpdateBillingNotice } from "../extensions/billingMigration
1212
import { enableBilling } from "../extensions/checkProjectBilling";
1313
import { checkBillingEnabled } from "../gcp/cloudbilling";
1414
import * as extensionsApi from "../extensions/extensionsApi";
15+
import * as provisioningHelper from "../extensions/provisioningHelper";
1516
import {
1617
ensureExtensionsApiEnabled,
1718
logPrefix,
@@ -232,6 +233,9 @@ export default new Command("ext:update <extensionInstanceId> [updateSource]")
232233
newSourceOrigin === SourceOrigin.OFFICIAL_EXTENSION ||
233234
newSourceOrigin === SourceOrigin.OFFICIAL_EXTENSION_VERSION;
234235
await displayChanges(existingSpec, newSpec, isOfficial);
236+
237+
await provisioningHelper.checkProductsProvisioned(projectId, newSpec);
238+
235239
if (newSpec.billingRequired) {
236240
const enabled = await checkBillingEnabled(projectId);
237241
if (!enabled) {

‎src/extensions/provisioningHelper.ts‎

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1+
import * as marked from "marked";
2+
13
import * as extensionsApi from "./extensionsApi";
24
import * as api from "../api";
3-
import * as utils from "../utils";
4-
import * as marked from "marked";
55
import { FirebaseError } from "../error";
66

77
/** Product for which provisioning can be (or is) deferred */
@@ -97,26 +97,20 @@ async function isStorageProvisioned(projectId: string): Promise<boolean> {
9797
auth: true,
9898
origin: api.firebaseStorageOrigin,
9999
});
100-
return await Promise.resolve(
101-
!!resp.body?.buckets?.find((bucket: any) => {
102-
const bucketResourceName = bucket.name;
103-
// Bucket resource name looks like: projects/PROJECT_NUMBER/buckets/BUCKET_NAME
104-
// and we just need the BUCKET_NAME part.
105-
const bucketResourceNameTokens = bucketResourceName.split("/");
106-
const pattern = "^" + projectId + "(.[[a-z0-9]+)*.appspot.com$";
107-
return new RegExp(pattern).test(
108-
bucketResourceNameTokens[bucketResourceNameTokens.length - 1]
109-
);
110-
})
111-
);
100+
return !!resp.body?.buckets?.find((bucket: any) => {
101+
const bucketResourceName = bucket.name;
102+
// Bucket resource name looks like: projects/PROJECT_NUMBER/buckets/BUCKET_NAME
103+
// and we just need the BUCKET_NAME part.
104+
const bucketResourceNameTokens = bucketResourceName.split("/");
105+
const pattern = "^" + projectId + "(.[[a-z0-9]+)*.appspot.com$";
106+
return new RegExp(pattern).test(bucketResourceNameTokens[bucketResourceNameTokens.length - 1]);
107+
});
112108
}
113109

114110
async function isAuthProvisioned(projectId: string): Promise<boolean> {
115111
const resp = await api.request("GET", `/v1/projects/${projectId}/products`, {
116112
auth: true,
117113
origin: api.firedataOrigin,
118114
});
119-
return Promise.resolve(
120-
!!resp.body?.activation?.map((a: any) => a.service).includes("FIREBASE_AUTH")
121-
);
115+
return !!resp.body?.activation?.map((a: any) => a.service).includes("FIREBASE_AUTH");
122116
}

‎src/test/extensions/provisioningHelper.spec.ts‎

Lines changed: 79 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import * as nock from "nock";
2+
import { expect } from "chai";
3+
24
import * as api from "../../api";
35
import * as provisioningHelper from "../../extensions/provisioningHelper";
46
import * as extensionsApi from "../../extensions/extensionsApi";
5-
import { expect } from "chai";
67
import { FirebaseError } from "../../error";
78

89
const TEST_INSTANCES_RESPONSE = {};
@@ -35,105 +36,97 @@ const FIREBASE_STORAGE_DEFAULT_BUCKET_LINKED_RESPONSE = {
3536
],
3637
};
3738

38-
describe.only("provisioningHelper", () => {
39+
describe("provisioningHelper", () => {
3940
afterEach(() => {
4041
nock.cleanAll();
4142
});
4243

4344
describe("getUsedProducts", () => {
45+
let testSpec: extensionsApi.ExtensionSpec;
46+
47+
beforeEach(() => {
48+
testSpec = {
49+
apis: [
50+
{
51+
apiName: "unrelated.googleapis.com",
52+
},
53+
] as extensionsApi.Api[],
54+
roles: [
55+
{
56+
role: "unrelated.role",
57+
},
58+
] as extensionsApi.Role[],
59+
resources: [
60+
{
61+
propertiesYaml:
62+
"availableMemoryMb: 1024\neventTrigger:\n eventType: providers/unrelates.service/eventTypes/something.do\n resource: projects/_/buckets/${param:IMG_BUCKET}\nlocation: ${param:LOCATION}\nruntime: nodejs10\n",
63+
},
64+
] as extensionsApi.Resource[],
65+
} as extensionsApi.ExtensionSpec;
66+
});
67+
4468
it("returns empty array when nothing is used", () => {
45-
expect(
46-
provisioningHelper.getUsedProducts({
47-
apis: [
48-
{
49-
apiName: "unrelated.googleapis.com",
50-
},
51-
] as extensionsApi.Api[],
52-
roles: [
53-
{
54-
role: "unrelated.role",
55-
},
56-
] as extensionsApi.Role[],
57-
resources: [
58-
{
59-
propertiesYaml:
60-
"availableMemoryMb: 1024\neventTrigger:\n eventType: providers/unrelates.service/eventTypes/something.do\n resource: projects/_/buckets/${param:IMG_BUCKET}\nlocation: ${param:LOCATION}\nruntime: nodejs10\n",
61-
},
62-
] as extensionsApi.Resource[],
63-
} as extensionsApi.ExtensionSpec)
64-
).to.be.empty;
69+
expect(provisioningHelper.getUsedProducts(testSpec)).to.be.empty;
6570
});
71+
6672
it("returns STORAGE when Storage API is used", () => {
67-
expect(
68-
provisioningHelper.getUsedProducts({
69-
apis: [
70-
{
71-
apiName: "storage-component.googleapis.com",
72-
},
73-
] as extensionsApi.Api[],
74-
resources: [] as extensionsApi.Resource[],
75-
} as extensionsApi.ExtensionSpec)
76-
).to.be.deep.eq([provisioningHelper.DeferredProduct.STORAGE]);
73+
testSpec.apis?.push({
74+
apiName: "storage-component.googleapis.com",
75+
reason: "whatever",
76+
});
77+
expect(provisioningHelper.getUsedProducts(testSpec)).to.be.deep.eq([
78+
provisioningHelper.DeferredProduct.STORAGE,
79+
]);
7780
});
81+
7882
it("returns STORAGE when Storage Role is used", () => {
79-
expect(
80-
provisioningHelper.getUsedProducts({
81-
roles: [
82-
{
83-
role: "storage.object.admin",
84-
},
85-
] as extensionsApi.Role[],
86-
resources: [] as extensionsApi.Resource[],
87-
} as extensionsApi.ExtensionSpec)
88-
).to.be.deep.eq([provisioningHelper.DeferredProduct.STORAGE]);
83+
testSpec.roles?.push({
84+
role: "storage.object.admin",
85+
reason: "whatever",
86+
});
87+
expect(provisioningHelper.getUsedProducts(testSpec)).to.be.deep.eq([
88+
provisioningHelper.DeferredProduct.STORAGE,
89+
]);
8990
});
91+
9092
it("returns STORAGE when Storage trigger is used", () => {
91-
expect(
92-
provisioningHelper.getUsedProducts({
93-
resources: [
94-
{
95-
propertiesYaml:
96-
"availableMemoryMb: 1024\neventTrigger:\n eventType: google.storage.object.finalize\n resource: projects/_/buckets/${param:IMG_BUCKET}\nlocation: ${param:LOCATION}\nruntime: nodejs10\n",
97-
},
98-
] as extensionsApi.Resource[],
99-
} as extensionsApi.ExtensionSpec)
100-
).to.be.deep.eq([provisioningHelper.DeferredProduct.STORAGE]);
93+
testSpec.resources?.push({
94+
propertiesYaml:
95+
"availableMemoryMb: 1024\neventTrigger:\n eventType: google.storage.object.finalize\n resource: projects/_/buckets/${param:IMG_BUCKET}\nlocation: ${param:LOCATION}\nruntime: nodejs10\n",
96+
} as extensionsApi.Resource);
97+
expect(provisioningHelper.getUsedProducts(testSpec)).to.be.deep.eq([
98+
provisioningHelper.DeferredProduct.STORAGE,
99+
]);
101100
});
101+
102102
it("returns AUTH when Authentication API is used", () => {
103-
expect(
104-
provisioningHelper.getUsedProducts({
105-
apis: [
106-
{
107-
apiName: "identitytoolkit.googleapis.com",
108-
},
109-
] as extensionsApi.Api[],
110-
resources: [] as extensionsApi.Resource[],
111-
} as extensionsApi.ExtensionSpec)
112-
).to.be.deep.eq([provisioningHelper.DeferredProduct.AUTH]);
103+
testSpec.apis?.push({
104+
apiName: "identitytoolkit.googleapis.com",
105+
reason: "whatever",
106+
});
107+
expect(provisioningHelper.getUsedProducts(testSpec)).to.be.deep.eq([
108+
provisioningHelper.DeferredProduct.AUTH,
109+
]);
113110
});
111+
114112
it("returns AUTH when Authentication Role is used", () => {
115-
expect(
116-
provisioningHelper.getUsedProducts({
117-
roles: [
118-
{
119-
role: "firebaseauth.user.admin",
120-
},
121-
] as extensionsApi.Role[],
122-
resources: [] as extensionsApi.Resource[],
123-
} as extensionsApi.ExtensionSpec)
124-
).to.be.deep.eq([provisioningHelper.DeferredProduct.AUTH]);
113+
testSpec.roles?.push({
114+
role: "firebaseauth.user.admin",
115+
reason: "whatever",
116+
});
117+
expect(provisioningHelper.getUsedProducts(testSpec)).to.be.deep.eq([
118+
provisioningHelper.DeferredProduct.AUTH,
119+
]);
125120
});
121+
126122
it("returns AUTH when Auth trigger is used", () => {
127-
expect(
128-
provisioningHelper.getUsedProducts({
129-
resources: [
130-
{
131-
propertiesYaml:
132-
"availableMemoryMb: 1024\neventTrigger:\n eventType: providers/firebase.auth/eventTypes/user.create\n resource: projects/_/buckets/${param:IMG_BUCKET}\nlocation: ${param:LOCATION}\nruntime: nodejs10\n",
133-
},
134-
] as extensionsApi.Resource[],
135-
} as extensionsApi.ExtensionSpec)
136-
).to.be.deep.eq([provisioningHelper.DeferredProduct.AUTH]);
123+
testSpec.resources?.push({
124+
propertiesYaml:
125+
"availableMemoryMb: 1024\neventTrigger:\n eventType: providers/firebase.auth/eventTypes/user.create\n resource: projects/_/buckets/${param:IMG_BUCKET}\nlocation: ${param:LOCATION}\nruntime: nodejs10\n",
126+
} as extensionsApi.Resource);
127+
expect(provisioningHelper.getUsedProducts(testSpec)).to.be.deep.eq([
128+
provisioningHelper.DeferredProduct.AUTH,
129+
]);
137130
});
138131
});
139132

@@ -145,6 +138,7 @@ describe.only("provisioningHelper", () => {
145138
} as extensionsApi.ExtensionSpec)
146139
).to.be.fulfilled;
147140
});
141+
148142
it("passes provisioning check when all is provisioned", async () => {
149143
nock(api.firedataOrigin)
150144
.get(`/v1/projects/${PROJECT_ID}/products`)
@@ -159,6 +153,7 @@ describe.only("provisioningHelper", () => {
159153

160154
expect(nock.isDone()).to.be.true;
161155
});
156+
162157
it("fails provisioning check storage when default bucket is not linked", async () => {
163158
nock(api.firedataOrigin)
164159
.get(`/v1/projects/${PROJECT_ID}/products`)
@@ -179,6 +174,7 @@ describe.only("provisioningHelper", () => {
179174

180175
expect(nock.isDone()).to.be.true;
181176
});
177+
182178
it("fails provisioning check storage when no firebase storage buckets", async () => {
183179
nock(api.firedataOrigin)
184180
.get(`/v1/projects/${PROJECT_ID}/products`)
@@ -191,6 +187,7 @@ describe.only("provisioningHelper", () => {
191187

192188
expect(nock.isDone()).to.be.true;
193189
});
190+
194191
it("fails provisioning check storage when no auth is not provisioned", async () => {
195192
nock(api.firedataOrigin).get(`/v1/projects/${PROJECT_ID}/products`).reply(200, {});
196193
nock(api.firebaseStorageOrigin)

0 commit comments

Comments
 (0)