Skip to content

Commit

Permalink
Add prompt if user deploys a genkit function without a secret (#8138)
Browse files Browse the repository at this point in the history
* Add prompt if user deploys a genkit function without a secret

* Changelog
  • Loading branch information
inlined authored Jan 27, 2025
1 parent 61cb79a commit 0b8e6ad
Show file tree
Hide file tree
Showing 3 changed files with 167 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Warn users who first try to deploy a Genkit function without a secret as this is likely a bug (#8138)
128 changes: 128 additions & 0 deletions src/deploy/functions/prepare.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<backend.Endpoint, "httpsTrigger"> = {
Expand Down Expand Up @@ -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<typeof prompt.confirm>,
ReturnType<typeof prompt.confirm>
>;

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);
});
});
});
38 changes: 38 additions & 0 deletions src/deploy/functions/prepare.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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");
}
}
}

0 comments on commit 0b8e6ad

Please sign in to comment.