From 037b7ea3499a2a7b4aed8ea6b248be977491027a Mon Sep 17 00:00:00 2001 From: Cole Rogers Date: Mon, 16 Aug 2021 15:08:40 -0400 Subject: [PATCH] HTTP Invoker for v2 functions (#3683) * adding invoker to replace allUsers * fixing build issues * adding tests * update function names, clean up comments * add invoker option when parsing function from SDK * fix service account format for IAM API * fixing ts issues and added check for full service account * cleaning up code for readability * merged iam policy creation and set policy into setInvoker * cleaning up old function and adding comments * added unit tests * fixing api version & removing import * adding setInvoker to cloud run * cleaning up * lint fix * fix reference * added v2 invoker functions * fixing pr comments --- src/deploy/functions/tasks.ts | 5 +- src/gcp/cloudfunctions.ts | 2 +- src/gcp/run.ts | 114 ++++++++++++-- src/test/gcp/run.spec.ts | 273 ++++++++++++++++++++++++++++++++++ 4 files changed, 376 insertions(+), 18 deletions(-) create mode 100644 src/test/gcp/run.spec.ts diff --git a/src/deploy/functions/tasks.ts b/src/deploy/functions/tasks.ts index 30f71243..5848aabe 100644 --- a/src/deploy/functions/tasks.ts +++ b/src/deploy/functions/tasks.ts @@ -128,7 +128,7 @@ export function createFunctionTask( await gcf.setInvokerCreate(params.projectId, fnName, invoker); } else { const serviceName = (cloudFunction as gcfV2.CloudFunction).serviceConfig.service!; - cloudrun.setIamPolicy(serviceName, cloudrun.DEFAULT_PUBLIC_POLICY); + cloudrun.setInvokerCreate(params.projectId, serviceName, invoker); } } catch (err) { params.errorHandler.record("error", fnName, "set invoker", err.message); @@ -197,7 +197,8 @@ export function updateFunctionTask( if (fn.platform === "gcfv1") { await gcf.setInvokerUpdate(params.projectId, fnName, fn.invoker); } else { - // TODO: gcfv2 + const serviceName = (cloudFunction as gcfV2.CloudFunction).serviceConfig.service!; + cloudrun.setInvokerUpdate(params.projectId, serviceName, fn.invoker); } } catch (err) { params.errorHandler.record("error", fnName, "set invoker", err.message); diff --git a/src/gcp/cloudfunctions.ts b/src/gcp/cloudfunctions.ts index e0919038..7e2c80d3 100644 --- a/src/gcp/cloudfunctions.ts +++ b/src/gcp/cloudfunctions.ts @@ -331,7 +331,7 @@ export async function setInvokerUpdate( ); if ( currentInvokerBinding && - _.isEqual(new Set(currentInvokerBinding.members), new Set(invokerMembers)) + JSON.stringify(currentInvokerBinding.members.sort()) === JSON.stringify(invokerMembers.sort()) ) { return; } diff --git a/src/gcp/run.ts b/src/gcp/run.ts index c3b2ac57..25a19f66 100644 --- a/src/gcp/run.ts +++ b/src/gcp/run.ts @@ -2,6 +2,8 @@ import { Client } from "../apiv2"; import { FirebaseError } from "../error"; import { runOrigin } from "../api"; import * as proto from "./proto"; +import * as iam from "./iam"; +import * as _ from "lodash"; const API_VERSION = "v1"; @@ -105,22 +107,12 @@ export interface TrafficTarget { } export interface IamPolicy { - version: number; - bindings: Record[]; + version?: number; + bindings?: iam.Binding[]; auditConfigs?: Record[]; etag?: string; } -export const DEFAULT_PUBLIC_POLICY = { - version: 3, - bindings: [ - { - role: "roles/run.invoker", - members: ["allUsers"], - }, - ], -}; - export async function getService(name: string): Promise { try { const response = await client.get(name); @@ -148,16 +140,20 @@ export async function replaceService(name: string, service: Service): Promise { +export async function setIamPolicy( + name: string, + policy: iam.Policy, + httpClient: Client = client +): Promise { // Cloud Run has an atypical REST binding for SetIamPolicy. Instead of making the body a policy and // the update mask a query parameter (e.g. Cloud Functions v1) the request body is the literal // proto. interface Request { - policy: IamPolicy; + policy: iam.Policy; updateMask: string; } try { - await client.post(`${name}:setIamPolicy`, { + await httpClient.post(`${name}:setIamPolicy`, { policy, updateMask: proto.fieldMasks(policy).join(","), }); @@ -167,3 +163,91 @@ export async function setIamPolicy(name: string, policy: IamPolicy): Promise { + try { + const response = await httpClient.get(`${serviceName}:getIamPolicy`); + return response.body; + } catch (err) { + throw new FirebaseError(`Failed to get the IAM Policy on the Service ${serviceName}`, { + original: err, + }); + } +} + +/** + * Gets the current IAM policy for the run service and overrides the invoker role with the supplied invoker members + * @param projectId id of the project + * @param serviceName cloud run service + * @param invoker an array of invoker strings + * + * @throws {@link FirebaseError} on an empty invoker, when the IAM Polciy fails to be grabbed or set + */ +export async function setInvokerCreate( + projectId: string, + serviceName: string, + invoker: string[], + httpClient: Client = client // for unit testing +) { + if (invoker.length == 0) { + throw new FirebaseError("Invoker cannot be an empty array"); + } + const invokerMembers = proto.getInvokerMembers(invoker, projectId); + const invokerRole = "roles/run.invoker"; + const bindings = [{ role: invokerRole, members: invokerMembers }]; + + const policy: iam.Policy = { + bindings: bindings, + etag: "", + version: 3, + }; + + await setIamPolicy(serviceName, policy, httpClient); +} + +/** + * Gets the current IAM policy for the run service and overrides the invoker role with the supplied invoker members + * @param projectId id of the project + * @param serviceName cloud run service + * @param invoker an array of invoker strings + * + * @throws {@link FirebaseError} on an empty invoker, when the IAM Polciy fails to be grabbed or set + */ +export async function setInvokerUpdate( + projectId: string, + serviceName: string, + invoker: string[], + httpClient: Client = client // for unit testing +) { + if (invoker.length == 0) { + throw new FirebaseError("Invoker cannot be an empty array"); + } + const invokerMembers = proto.getInvokerMembers(invoker, projectId); + const invokerRole = "roles/run.invoker"; + const currentPolicy = await getIamPolicy(serviceName, httpClient); + const currentInvokerBinding = currentPolicy.bindings?.find( + (binding) => binding.role === invokerRole + ); + if ( + currentInvokerBinding && + JSON.stringify(currentInvokerBinding.members.sort()) === JSON.stringify(invokerMembers.sort()) + ) { + return; + } + + const bindings = (currentPolicy.bindings || []).filter((binding) => binding.role !== invokerRole); + bindings.push({ + role: invokerRole, + members: invokerMembers, + }); + + const policy: iam.Policy = { + bindings: bindings, + etag: currentPolicy.etag || "", + version: 3, + }; + await setIamPolicy(serviceName, policy, httpClient); +} diff --git a/src/test/gcp/run.spec.ts b/src/test/gcp/run.spec.ts new file mode 100644 index 00000000..95f12e59 --- /dev/null +++ b/src/test/gcp/run.spec.ts @@ -0,0 +1,273 @@ +import { expect } from "chai"; +import * as sinon from "sinon"; +import * as run from "../../gcp/run"; +import { Client } from "../../apiv2"; + +describe("run", () => { + describe("setInvokerCreate", () => { + let sandbox: sinon.SinonSandbox; + let apiRequestStub: sinon.SinonStub; + let client: Client; + + beforeEach(() => { + client = new Client({ + urlPrefix: "origin", + auth: true, + apiVersion: "v1", + }); + sandbox = sinon.createSandbox(); + apiRequestStub = sandbox.stub(client, "post").throws("Unexpected API post call"); + }); + + afterEach(() => { + sandbox.restore(); + }); + + it("should reject on emtpy invoker array", async () => { + await expect(run.setInvokerCreate("project", "service", [], client)).to.be.rejected; + }); + + it("should reject if the setting the IAM policy fails", async () => { + apiRequestStub.onFirstCall().throws("Error calling set api."); + + await expect( + run.setInvokerCreate("project", "service", ["public"], client) + ).to.be.rejectedWith("Failed to set the IAM Policy on the Service service"); + expect(apiRequestStub).to.be.calledOnce; + }); + + it("should set a private policy on a function", async () => { + apiRequestStub.onFirstCall().callsFake((path: string, json: any) => { + expect(json.policy).to.deep.eq({ + bindings: [ + { + role: "roles/run.invoker", + members: [], + }, + ], + etag: "", + version: 3, + }); + + return Promise.resolve(); + }); + + await expect(run.setInvokerCreate("project", "service", ["private"], client)).to.not.be + .rejected; + expect(apiRequestStub).to.be.calledOnce; + }); + + it("should set a public policy on a function", async () => { + apiRequestStub.onFirstCall().callsFake((path: string, json: any) => { + expect(json.policy).to.deep.eq({ + bindings: [ + { + role: "roles/run.invoker", + members: ["allUsers"], + }, + ], + etag: "", + version: 3, + }); + + return Promise.resolve(); + }); + + await expect(run.setInvokerCreate("project", "service", ["public"], client)).to.not.be + .rejected; + expect(apiRequestStub).to.be.calledOnce; + }); + + it("should set the policy with a set of invokers with active policies", async () => { + apiRequestStub.onFirstCall().callsFake((path: string, json: any) => { + json.policy.bindings[0].members.sort(); + expect(json.policy.bindings[0].members).to.deep.eq([ + "serviceAccount:service-account1@project.iam.gserviceaccount.com", + "serviceAccount:service-account2@project.iam.gserviceaccount.com", + "serviceAccount:service-account3@project.iam.gserviceaccount.com", + ]); + + return Promise.resolve(); + }); + + await expect( + run.setInvokerCreate( + "project", + "service", + [ + "service-account1@", + "service-account2@project.iam.gserviceaccount.com", + "service-account3@", + ], + client + ) + ).to.not.be.rejected; + expect(apiRequestStub).to.be.calledOnce; + }); + }); + + describe("setInvokerUpdate", () => { + describe("setInvokerCreate", () => { + let sandbox: sinon.SinonSandbox; + let apiPostStub: sinon.SinonStub; + let apiGetStub: sinon.SinonStub; + let client: Client; + + beforeEach(() => { + client = new Client({ + urlPrefix: "origin", + auth: true, + apiVersion: "v1", + }); + sandbox = sinon.createSandbox(); + apiPostStub = sandbox.stub(client, "post").throws("Unexpected API post call"); + apiGetStub = sandbox.stub(client, "get").throws("Unexpected API get call"); + }); + + afterEach(() => { + sandbox.restore(); + }); + + it("should reject on emtpy invoker array", async () => { + await expect(run.setInvokerUpdate("project", "service", [])).to.be.rejected; + }); + + it("should reject if the getting the IAM policy fails", async () => { + apiGetStub.onFirstCall().throws("Error calling get api."); + + await expect( + run.setInvokerUpdate("project", "service", ["public"], client) + ).to.be.rejectedWith("Failed to get the IAM Policy on the Service service"); + + expect(apiGetStub).to.be.called; + }); + + it("should reject if the setting the IAM policy fails", async () => { + apiGetStub.resolves({ body: {} }); + apiPostStub.throws("Error calling set api."); + + await expect( + run.setInvokerUpdate("project", "service", ["public"], client) + ).to.be.rejectedWith("Failed to set the IAM Policy on the Service service"); + expect(apiGetStub).to.be.calledOnce; + expect(apiPostStub).to.be.calledOnce; + }); + + it("should set a basic policy on a function without any polices", async () => { + apiGetStub.onFirstCall().resolves({ body: {} }); + apiPostStub.onFirstCall().callsFake((path: string, json: any) => { + expect(json.policy).to.deep.eq({ + bindings: [ + { + role: "roles/run.invoker", + members: ["allUsers"], + }, + ], + etag: "", + version: 3, + }); + + return Promise.resolve(); + }); + + await expect(run.setInvokerUpdate("project", "service", ["public"], client)).to.not.be + .rejected; + expect(apiGetStub).to.be.calledOnce; + expect(apiPostStub).to.be.calledOnce; + }); + + it("should set the policy with private invoker with active policies", async () => { + apiGetStub.onFirstCall().resolves({ + body: { + bindings: [ + { role: "random-role", members: ["user:pineapple"] }, + { role: "roles/run.invoker", members: ["some-service-account"] }, + ], + etag: "1234", + version: 3, + }, + }); + apiPostStub.onFirstCall().callsFake((path: string, json: any) => { + expect(json.policy).to.deep.eq({ + bindings: [ + { role: "random-role", members: ["user:pineapple"] }, + { role: "roles/run.invoker", members: [] }, + ], + etag: "1234", + version: 3, + }); + + return Promise.resolve(); + }); + + await expect(run.setInvokerUpdate("project", "service", ["private"], client)).to.not.be + .rejected; + expect(apiGetStub).to.be.calledOnce; + expect(apiPostStub).to.be.calledOnce; + }); + + it("should set the policy with a set of invokers with active policies", async () => { + apiGetStub.onFirstCall().resolves({ body: {} }); + apiPostStub.onFirstCall().callsFake((path: string, json: any) => { + json.policy.bindings[0].members.sort(); + expect(json.policy.bindings[0].members).to.deep.eq([ + "serviceAccount:service-account1@project.iam.gserviceaccount.com", + "serviceAccount:service-account2@project.iam.gserviceaccount.com", + "serviceAccount:service-account3@project.iam.gserviceaccount.com", + ]); + + return Promise.resolve(); + }); + + await expect( + run.setInvokerUpdate( + "project", + "service", + [ + "service-account1@", + "service-account2@project.iam.gserviceaccount.com", + "service-account3@", + ], + client + ) + ).to.not.be.rejected; + expect(apiGetStub).to.be.calledOnce; + expect(apiPostStub).to.be.calledOnce; + }); + + it("should not set the policy if the set of invokers is the same as the current invokers", async () => { + apiGetStub.onFirstCall().resolves({ + body: { + bindings: [ + { + role: "roles/run.invoker", + members: [ + "serviceAccount:service-account1@project.iam.gserviceaccount.com", + "serviceAccount:service-account3@project.iam.gserviceaccount.com", + "serviceAccount:service-account2@project.iam.gserviceaccount.com", + ], + }, + ], + etag: "1234", + version: 3, + }, + }); + + await expect( + run.setInvokerUpdate( + "project", + "service", + [ + "service-account2@project.iam.gserviceaccount.com", + "service-account3@", + "service-account1@", + ], + client + ) + ).to.not.be.rejected; + expect(apiGetStub).to.be.calledOnce; + expect(apiPostStub).to.not.be.called; + }); + }); + }); +});