Skip to content

Commit b103ff1

Browse files
bryanatkinsonbryanatkinson
authored
Update function deploy to add necessary Genkit monitoring permissions when deploying Genkit function (#8636)
Genkit Monitoring requires that the service account running the Genkit code has permission to write metrics, traces and logs. This change adds those permissions to the default service account when deploying as a Firebase Function.
1 parent c977451 commit b103ff1

File tree

3 files changed

+441
-2
lines changed

3 files changed

+441
-2
lines changed

‎src/deploy/functions/checkIam.spec.ts

Lines changed: 358 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,364 @@ describe("checkIam", () => {
244244
});
245245
});
246246

247+
describe("ensureGenkitMonitoringRoles", () => {
248+
it("should return early if we do not have new endpoints", async () => {
249+
const fn1: backend.Endpoint = {
250+
id: "genkitFn1",
251+
platform: "gcfv2",
252+
entryPoint: "genkitFn1",
253+
callableTrigger: {
254+
genkitAction: "action",
255+
},
256+
...SPEC,
257+
};
258+
const fn2: backend.Endpoint = {
259+
id: "genkitFn2",
260+
platform: "gcfv2",
261+
entryPoint: "genkitFn2",
262+
callableTrigger: {
263+
genkitAction: "action",
264+
},
265+
...SPEC,
266+
};
267+
const wantFn: backend.Endpoint = {
268+
id: "wantGenkitFnFn",
269+
entryPoint: "wantGenkitFn",
270+
platform: "gcfv2",
271+
callableTrigger: {
272+
genkitAction: "action",
273+
},
274+
...SPEC,
275+
};
276+
277+
await checkIam.ensureGenkitMonitoringRoles(
278+
projectId,
279+
projectNumber,
280+
backend.of(wantFn),
281+
backend.of(fn1, fn2, wantFn),
282+
);
283+
284+
expect(getIamStub).to.not.have.been.called;
285+
expect(setIamStub).to.not.have.been.called;
286+
});
287+
288+
it("should return early if none of the new endpoints are genkit", async () => {
289+
const fn1: backend.Endpoint = {
290+
id: "genkitFn1",
291+
platform: "gcfv2",
292+
entryPoint: "genkitFn1",
293+
callableTrigger: {
294+
genkitAction: "action",
295+
},
296+
...SPEC,
297+
};
298+
const fn2: backend.Endpoint = {
299+
id: "genkitFn2",
300+
platform: "gcfv2",
301+
entryPoint: "genkitFn2",
302+
callableTrigger: {
303+
genkitAction: "action",
304+
},
305+
...SPEC,
306+
};
307+
const wantFn1: backend.Endpoint = {
308+
id: "wantFn1",
309+
entryPoint: "wantFn1",
310+
platform: "gcfv2",
311+
eventTrigger: {
312+
eventType: "google.cloud.storage.object.v1.finalized",
313+
eventFilters: { bucket: "my-bucket" },
314+
retry: false,
315+
},
316+
...SPEC,
317+
};
318+
const wantFn2: backend.Endpoint = {
319+
id: "wantFn2",
320+
entryPoint: "wantFn2",
321+
platform: "gcfv2",
322+
callableTrigger: {},
323+
...SPEC,
324+
};
325+
326+
await checkIam.ensureGenkitMonitoringRoles(
327+
projectId,
328+
projectNumber,
329+
backend.of(wantFn1, wantFn2),
330+
backend.of(fn1, fn2),
331+
);
332+
333+
expect(getIamStub).to.not.have.been.called;
334+
expect(setIamStub).to.not.have.been.called;
335+
});
336+
337+
it("should return early if we fail to get the IAM policy", async () => {
338+
getIamStub.rejects("Failed to get the IAM policy");
339+
const wantFn: backend.Endpoint = {
340+
id: "genkitFn1",
341+
platform: "gcfv2",
342+
entryPoint: "wantFn",
343+
callableTrigger: {
344+
genkitAction: "action",
345+
},
346+
...SPEC,
347+
};
348+
349+
await expect(
350+
checkIam.ensureGenkitMonitoringRoles(
351+
projectId,
352+
projectNumber,
353+
backend.of(wantFn),
354+
backend.empty(),
355+
),
356+
).to.not.be.rejected;
357+
expect(getIamStub).to.have.been.calledOnce;
358+
expect(getIamStub).to.have.been.calledWith(projectNumber);
359+
expect(setIamStub).to.not.have.been.called;
360+
});
361+
362+
it("should error if we fail to set the IAM policy", async () => {
363+
getIamStub.resolves({
364+
etag: "etag",
365+
version: 3,
366+
bindings: [BINDING],
367+
});
368+
const wantFn: backend.Endpoint = {
369+
id: "genkitFn1",
370+
platform: "gcfv2",
371+
entryPoint: "wantFn",
372+
callableTrigger: {
373+
genkitAction: "action",
374+
},
375+
...SPEC,
376+
};
377+
378+
await expect(
379+
checkIam.ensureGenkitMonitoringRoles(
380+
projectId,
381+
projectNumber,
382+
backend.of(wantFn),
383+
backend.empty(),
384+
),
385+
).to.be.rejectedWith(
386+
"We failed to modify the IAM policy for the project. The functions " +
387+
"deployment requires specific roles to be granted to service agents," +
388+
" otherwise the deployment will fail.",
389+
);
390+
expect(getIamStub).to.have.been.calledOnce;
391+
expect(getIamStub).to.have.been.calledWith(projectNumber);
392+
expect(setIamStub).to.have.been.calledOnce;
393+
});
394+
395+
it("should not update policy if it already has necessary bindings", async () => {
396+
const serviceAccount = `test-sa@${projectId}.iam.gserviceaccount.com`;
397+
const iamPolicy = {
398+
etag: "etag",
399+
version: 3,
400+
bindings: [
401+
BINDING,
402+
{
403+
role: "roles/monitoring.metricWriter",
404+
members: [`serviceAccount:${serviceAccount}`, "anotheruser"],
405+
},
406+
{
407+
role: "roles/cloudtrace.agent",
408+
members: [`serviceAccount:${serviceAccount}`, "anotheruser"],
409+
},
410+
{
411+
role: "roles/logging.logWriter",
412+
members: [`serviceAccount:${serviceAccount}`, "anotheruser"],
413+
},
414+
],
415+
};
416+
getIamStub.resolves(iamPolicy);
417+
const wantFn: backend.Endpoint = {
418+
id: "genkitFn1",
419+
platform: "gcfv2",
420+
entryPoint: "wantFn",
421+
serviceAccount: serviceAccount,
422+
callableTrigger: {
423+
genkitAction: "action",
424+
},
425+
...SPEC,
426+
};
427+
428+
await checkIam.ensureGenkitMonitoringRoles(
429+
projectId,
430+
projectNumber,
431+
backend.of(wantFn),
432+
backend.empty(),
433+
);
434+
435+
expect(getIamStub).to.have.been.calledOnce;
436+
expect(getIamStub).to.have.been.calledWith(projectNumber);
437+
expect(setIamStub).to.not.have.been.called;
438+
});
439+
440+
it("should update policy if any bindings are missing", async () => {
441+
const serviceAccount = `test-sa@${projectId}.iam.gserviceaccount.com`;
442+
const initialPolicy = {
443+
etag: "etag",
444+
version: 3,
445+
bindings: [
446+
BINDING,
447+
{
448+
role: "roles/monitoring.metricWriter",
449+
members: [`serviceAccount:${serviceAccount}`, "anotheruser"],
450+
},
451+
{
452+
role: "roles/logging.logWriter",
453+
members: [`serviceAccount:${serviceAccount}`, "anotheruser"],
454+
},
455+
],
456+
};
457+
getIamStub.resolves(initialPolicy);
458+
setIamStub.resolves({});
459+
const wantFn: backend.Endpoint = {
460+
id: "genkitFn1",
461+
platform: "gcfv2",
462+
entryPoint: "wantFn",
463+
serviceAccount: serviceAccount,
464+
callableTrigger: {
465+
genkitAction: "action",
466+
},
467+
...SPEC,
468+
};
469+
470+
await checkIam.ensureGenkitMonitoringRoles(
471+
projectId,
472+
projectNumber,
473+
backend.of(wantFn),
474+
backend.empty(),
475+
);
476+
477+
expect(getIamStub).to.have.been.calledOnce;
478+
expect(getIamStub).to.have.been.calledWith(projectNumber);
479+
expect(setIamStub).to.have.been.calledOnce;
480+
expect(setIamStub).to.have.been.calledWith(
481+
projectNumber,
482+
{
483+
etag: "etag",
484+
version: 3,
485+
bindings: [
486+
BINDING,
487+
{
488+
role: "roles/monitoring.metricWriter",
489+
members: [`serviceAccount:${serviceAccount}`, "anotheruser"],
490+
},
491+
{
492+
role: "roles/logging.logWriter",
493+
members: [`serviceAccount:${serviceAccount}`, "anotheruser"],
494+
},
495+
// Should include this missing binding
496+
{
497+
role: "roles/cloudtrace.agent",
498+
members: [`serviceAccount:${serviceAccount}`],
499+
},
500+
],
501+
},
502+
"bindings",
503+
);
504+
});
505+
506+
it("should update policy for all missing roles and service accounts", async () => {
507+
const serviceAccount1 = `test-sa-1@${projectId}.iam.gserviceaccount.com`;
508+
const serviceAccount2 = `test-sa-2@${projectId}.iam.gserviceaccount.com`;
509+
const defaultServiceAccount = `${projectNumber}[email protected]`;
510+
const initialPolicy = {
511+
etag: "etag",
512+
version: 3,
513+
bindings: [BINDING],
514+
};
515+
getIamStub.resolves(initialPolicy);
516+
setIamStub.resolves({});
517+
const fn1: backend.Endpoint = {
518+
id: "genkitFn1",
519+
platform: "gcfv2",
520+
entryPoint: "wantFn1",
521+
serviceAccount: serviceAccount1,
522+
callableTrigger: {
523+
genkitAction: "action",
524+
},
525+
...SPEC,
526+
};
527+
const fn2: backend.Endpoint = {
528+
id: "genkitFn2",
529+
platform: "gcfv2",
530+
entryPoint: "wantFn2",
531+
serviceAccount: serviceAccount1,
532+
callableTrigger: {
533+
genkitAction: "action",
534+
},
535+
...SPEC,
536+
};
537+
const fn3: backend.Endpoint = {
538+
id: "genkitFn3",
539+
platform: "gcfv2",
540+
entryPoint: "wantFn3",
541+
serviceAccount: serviceAccount2,
542+
callableTrigger: {
543+
genkitAction: "action",
544+
},
545+
...SPEC,
546+
};
547+
const fn4: backend.Endpoint = {
548+
id: "genkitFnWithDefaultServiceAccount",
549+
platform: "gcfv2",
550+
entryPoint: "wantFn",
551+
callableTrigger: {
552+
genkitAction: "action",
553+
},
554+
...SPEC,
555+
};
556+
557+
await checkIam.ensureGenkitMonitoringRoles(
558+
projectId,
559+
projectNumber,
560+
backend.of(fn1, fn2, fn3, fn4),
561+
backend.empty(),
562+
);
563+
564+
expect(getIamStub).to.have.been.calledOnce;
565+
expect(getIamStub).to.have.been.calledWith(projectNumber);
566+
expect(setIamStub).to.have.been.calledOnce;
567+
expect(setIamStub).to.have.been.calledWith(
568+
projectNumber,
569+
{
570+
etag: "etag",
571+
version: 3,
572+
bindings: [
573+
BINDING,
574+
{
575+
role: "roles/monitoring.metricWriter",
576+
members: [
577+
`serviceAccount:${serviceAccount1}`,
578+
`serviceAccount:${serviceAccount2}`,
579+
`serviceAccount:${defaultServiceAccount}`,
580+
],
581+
},
582+
{
583+
role: "roles/cloudtrace.agent",
584+
members: [
585+
`serviceAccount:${serviceAccount1}`,
586+
`serviceAccount:${serviceAccount2}`,
587+
`serviceAccount:${defaultServiceAccount}`,
588+
],
589+
},
590+
{
591+
role: "roles/logging.logWriter",
592+
members: [
593+
`serviceAccount:${serviceAccount1}`,
594+
`serviceAccount:${serviceAccount2}`,
595+
`serviceAccount:${defaultServiceAccount}`,
596+
],
597+
},
598+
],
599+
},
600+
"bindings",
601+
);
602+
});
603+
});
604+
247605
it("should add the pubsub publisher role for a new v2 storage function with v2 deployed functions", async () => {
248606
const newIamPolicy = {
249607
etag: "etag",

0 commit comments

Comments
 (0)