diff --git a/CHANGELOG.md b/CHANGELOG.md index e69de29bb2d..bf2e52b12d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -0,0 +1 @@ +Warn users who first try to deploy a Genkit function without a secret as this is likely a bug (#8138) diff --git a/src/deploy/functions/prepare.spec.ts b/src/deploy/functions/prepare.spec.ts index b63f5da6c55..8348ff7477b 100644 --- a/src/deploy/functions/prepare.spec.ts +++ b/src/deploy/functions/prepare.spec.ts @@ -3,6 +3,9 @@ import { expect } from "chai"; import * as backend from "./backend"; import * as prepare from "./prepare"; import { BEFORE_CREATE_EVENT, BEFORE_SIGN_IN_EVENT } from "../../functions/events/v1"; +import * as sinon from "sinon"; +import * as prompt from "../../prompt"; +import { FirebaseError } from "../../error"; describe("prepare", () => { const ENDPOINT_BASE: Omit = { @@ -291,4 +294,129 @@ describe("prepare", () => { expect(endpoint2InBackend2.targetedByOnly).to.be.false; }); }); + + describe("warnIfNewGenkitFunctionIsMissingSecrets", () => { + const nonGenkitEndpoint: backend.Endpoint = { + id: "nonGenkit", + platform: "gcfv2", + region: "us-central1", + project: "project", + entryPoint: "entry", + runtime: "nodejs16", + httpsTrigger: {}, + }; + + const genkitEndpointWithSecrets: backend.Endpoint = { + id: "genkitWithSecrets", + platform: "gcfv2", + region: "us-central1", + project: "project", + entryPoint: "entry", + runtime: "nodejs16", + callableTrigger: { + genkitAction: "action", + }, + secretEnvironmentVariables: [ + { + key: "SECRET", + secret: "secret", + projectId: "project", + }, + ], + }; + + const genkitEndpointWithoutSecrets: backend.Endpoint = { + id: "genkitWithoutSecrets", + platform: "gcfv2", + region: "us-central1", + project: "project", + entryPoint: "entry", + runtime: "nodejs16", + callableTrigger: { + genkitAction: "action", + }, + }; + + let confirm: sinon.SinonStub< + Parameters, + ReturnType + >; + + beforeEach(() => { + confirm = sinon.stub(prompt, "confirm"); + }); + + afterEach(() => { + sinon.verifyAndRestore(); + }); + + it("should not prompt if there are no genkit functions", async () => { + await prepare.warnIfNewGenkitFunctionIsMissingSecrets( + backend.empty(), + backend.of(nonGenkitEndpoint), + {} as any, + ); + expect(confirm).to.not.be.called; + }); + + it("should not prompt if all genkit functions have secrets", async () => { + await prepare.warnIfNewGenkitFunctionIsMissingSecrets( + backend.empty(), + backend.of(genkitEndpointWithSecrets), + {} as any, + ); + expect(confirm).to.not.be.called; + }); + + it("should not prompt if the function is already deployed", async () => { + await prepare.warnIfNewGenkitFunctionIsMissingSecrets( + backend.of(genkitEndpointWithoutSecrets), + backend.of(genkitEndpointWithoutSecrets), + {} as any, + ); + expect(confirm).to.not.be.called; + }); + + it("should not prompt if force is true", async () => { + await prepare.warnIfNewGenkitFunctionIsMissingSecrets( + backend.empty(), + backend.of(genkitEndpointWithoutSecrets), + { force: true } as any, + ); + expect(confirm).to.not.be.called; + }); + + it("should throw if missing secrets and noninteractive", async () => { + confirm.resolves(false); + await expect( + prepare.warnIfNewGenkitFunctionIsMissingSecrets( + backend.empty(), + backend.of(genkitEndpointWithoutSecrets), + { nonInteractive: true } as any, + ), + ).to.be.rejectedWith(FirebaseError); + expect(confirm).to.have.been.calledWithMatch({ nonInteractive: true }); + }); + + it("should prompt if missing secrets and interactive", async () => { + confirm.resolves(true); + await prepare.warnIfNewGenkitFunctionIsMissingSecrets( + backend.empty(), + backend.of(genkitEndpointWithoutSecrets), + {} as any, + ); + expect(confirm).to.be.calledOnce; + }); + + it("should throw if user declines to deploy", async () => { + confirm.resolves(false); + await expect( + prepare.warnIfNewGenkitFunctionIsMissingSecrets( + backend.empty(), + backend.of(genkitEndpointWithoutSecrets), + {} as any, + ), + ).to.be.rejectedWith(FirebaseError); + }); + }); }); diff --git a/src/deploy/functions/prepare.ts b/src/deploy/functions/prepare.ts index 7d5a7ee021e..0ec92a218fb 100644 --- a/src/deploy/functions/prepare.ts +++ b/src/deploy/functions/prepare.ts @@ -48,6 +48,7 @@ import { assertExhaustive } from "../../functional"; import { prepareDynamicExtensions } from "../extensions/prepare"; import { Context as ExtContext, Payload as ExtPayload } from "../extensions/args"; import { DeployOptions } from ".."; +import * as prompt from "../../prompt"; export const EVENTARC_SOURCE_ENV = "EVENTARC_CLOUD_EVENT_SOURCE"; @@ -240,6 +241,8 @@ export async function prepare( const wantBackend = backend.merge(...Object.values(wantBackends)); const haveBackend = backend.merge(...Object.values(haveBackends)); + await warnIfNewGenkitFunctionIsMissingSecrets(wantBackend, haveBackend, options); + // Enable required APIs. This may come implicitly from triggers (e.g. scheduled triggers // require cloudscheudler and, in v1, require pub/sub), or can eventually come from // explicit dependencies. @@ -495,3 +498,38 @@ export async function loadCodebases( } return wantBuilds; } + +// Genkit almost always requires an API key, so warn if the customer is about to deploy +// a function and doesn't have one. To avoid repetitive nagging, only warn on the first +// deploy of the function. +export async function warnIfNewGenkitFunctionIsMissingSecrets( + have: backend.Backend, + want: backend.Backend, + options: DeployOptions, +) { + if (options.force) { + return; + } + + const newAndMissingSecrets = backend.allEndpoints( + backend.matchingBackend(want, (e) => { + if (!backend.isCallableTriggered(e) || !e.callableTrigger.genkitAction) { + return false; + } + if (e.secretEnvironmentVariables?.length) { + return false; + } + return !backend.hasEndpoint(have)(e); + }), + ); + + if (newAndMissingSecrets.length) { + const message = + `The function(s) ${newAndMissingSecrets.map((e) => e.id).join(", ")} use Genkit but do not have access to a secret. ` + + "This may cause the function to fail if it depends on an API key. To learn more about granting a function access to " + + "secrets, see https://firebase.google.com/docs/functions/config-env?gen=2nd#secret_parameters. Continue?"; + if (!(await prompt.confirm({ message, nonInteractive: options.nonInteractive }))) { + throw new FirebaseError("Aborted"); + } + } +}