Skip to content

Commit

Permalink
Allow a Callable to include a Genkit Action annotation (#8039)
Browse files Browse the repository at this point in the history
In preparation for Genkit 1.0, we are going to have a `onCallGenkit` function declaration in `firebase-functions/https`. This will be a "subclass" of callable functions.

To represent this, "Genkit callables" are represented in memory as a callableTriggered with a new "genkitAction" property. Actual Cloud Functions will be created with the label deployed-callable: true still, but will also include a 'genkit-action' label as well, which will eventually be used to annotate the Firebase Console.

To allow users to migrate from `@genkit-ai/firebase/functions:onFlow` to `firebase-functions/https:onCallGenkit` we need to relax the restrictions around converting function types in function updates. This change allows an HTTPS function to become a Callable function. I also noticed a bug where you could convert between any type of non-auth-context Firestore function to any type of auth-context Firestore function and fixed it.
  • Loading branch information
inlined authored Dec 11, 2024
1 parent bc1a390 commit f98ffe4
Show file tree
Hide file tree
Showing 11 changed files with 130 additions and 20 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
- Changes default CF3 runtime to nodejs22 (#8037)
- Fixed an issue where `--import` would error for the Data Connect emulator if `dataDir` was also set.
- Fixed an issue where `firebase init dataconnect` errored when importing a schema with no GQL files.
- CF3 callables can now be annotate with a genkit action they are serving (#8039)
- HTTPS functions can now be upgraded to HTTPS Callable functions (#8039)
- Update default tsconfig to support more modern defaults (#8039)
10 changes: 6 additions & 4 deletions src/deploy/functions/backend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import * as utils from "../../utils";
import { Runtime } from "./runtimes/supported";
import { FirebaseError } from "../../error";
import { Context } from "./args";
import { flattenArray } from "../../functional";
import { assertExhaustive, flattenArray } from "../../functional";

/** Retry settings for a ScheduleSpec. */
export interface ScheduleRetryConfig {
Expand Down Expand Up @@ -41,7 +41,9 @@ export interface HttpsTriggered {
}

/** API agnostic version of a Firebase callable function. */
export type CallableTrigger = Record<string, never>;
export type CallableTrigger = {
genkitAction?: string;
};

/** Something that has a callable trigger */
export interface CallableTriggered {
Expand Down Expand Up @@ -135,6 +137,7 @@ export interface BlockingTrigger {
eventType: string;
options?: Record<string, unknown>;
}

export interface BlockingTriggered {
blockingTrigger: BlockingTrigger;
}
Expand All @@ -153,9 +156,8 @@ export function endpointTriggerType(endpoint: Endpoint): string {
return "taskQueue";
} else if (isBlockingTriggered(endpoint)) {
return endpoint.blockingTrigger.eventType;
} else {
throw new Error("Unexpected trigger type for endpoint " + JSON.stringify(endpoint));
}
assertExhaustive(endpoint);
}

// TODO(inlined): Enum types should be singularly named
Expand Down
9 changes: 6 additions & 3 deletions src/deploy/functions/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,9 @@ export interface HttpsTrigger {

// Trigger definitions for RPCs servers using the HTTP protocol defined at
// https://firebase.google.com/docs/functions/callable-reference
// eslint-disable-next-line
interface CallableTrigger {}
interface CallableTrigger {
genkitAction?: string;
}

// Trigger definitions for endpoints that should be called as a delegate for other operations.
// For example, before user login.
Expand Down Expand Up @@ -568,7 +569,9 @@ function discoverTrigger(endpoint: Endpoint, region: string, r: Resolver): backe
}
return { httpsTrigger };
} else if (isCallableTriggered(endpoint)) {
return { callableTrigger: {} };
const trigger: CallableTriggered = { callableTrigger: {} };
proto.copyIfPresent(trigger.callableTrigger, endpoint.callableTrigger, "genkitAction");
return trigger;
} else if (isBlockingTriggered(endpoint)) {
return { blockingTrigger: endpoint.blockingTrigger };
} else if (isEventTriggered(endpoint)) {
Expand Down
10 changes: 10 additions & 0 deletions src/deploy/functions/release/planner.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,16 @@ describe("planner", () => {
expect(() => planner.calculateUpdate(httpsFunc, scheduleFunc)).to.throw();
});

it("allows upgrades of genkit functions from the genkit plugin to firebase-functions SDK", () => {
const httpsFunc = func("a", "b", { httpsTrigger: {} });
const genkitFunc = func("a", "b", { callableTrigger: { genkitAction: "flows/flow" } });
expect(planner.calculateUpdate(genkitFunc, httpsFunc)).to.deep.equal({
// Missing: deleteAndRecreate
endpoint: genkitFunc,
unsafe: false,
});
});

it("knows to delete & recreate for v2 topic changes", () => {
const original: backend.Endpoint = {
...func("a", "b", {
Expand Down
15 changes: 8 additions & 7 deletions src/deploy/functions/release/planner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@ import { FirebaseError } from "../../../error";
import * as utils from "../../../utils";
import * as backend from "../backend";
import * as v2events from "../../../functions/events/v2";
import {
FIRESTORE_EVENT_REGEX,
FIRESTORE_EVENT_WITH_AUTH_CONTEXT_REGEX,
} from "../../../functions/events/v2";

export interface EndpointUpdate {
endpoint: backend.Endpoint;
Expand Down Expand Up @@ -261,9 +257,9 @@ export function upgradedScheduleFromV1ToV2(
export function checkForUnsafeUpdate(want: backend.Endpoint, have: backend.Endpoint): boolean {
return (
backend.isEventTriggered(want) &&
FIRESTORE_EVENT_WITH_AUTH_CONTEXT_REGEX.test(want.eventTrigger.eventType) &&
backend.isEventTriggered(have) &&
FIRESTORE_EVENT_REGEX.test(have.eventTrigger.eventType)
want.eventTrigger.eventType ===
v2events.CONVERTABLE_EVENTS[have.eventTrigger.eventType as v2events.Event]
);
}

Expand All @@ -289,7 +285,12 @@ export function checkForIllegalUpdate(want: backend.Endpoint, have: backend.Endp
};
const wantType = triggerType(want);
const haveType = triggerType(have);
if (wantType !== haveType) {

// Originally, @genkit-ai/firebase/functions defined onFlow which created an HTTPS trigger that implemented the streaming callable protocol for the Flow.
// The new version is firebase-functions/https which defines onCallFlow
const upgradingHttpsFunction =
backend.isHttpsTriggered(have) && backend.isCallableTriggered(want);
if (wantType !== haveType && !upgradingHttpsFunction) {
throw new FirebaseError(
`[${getFunctionLabel(
want,
Expand Down
29 changes: 29 additions & 0 deletions src/deploy/functions/runtimes/discovery/v1alpha1.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,35 @@ describe("buildFromV1Alpha", () => {
});
});

describe("genkitTriggers", () => {
it("fails with invalid fields", () => {
assertParserError({
endpoints: {
func: {
...MIN_ENDPOINT,
genkitTrigger: {
tool: "tools are not supported",
},
},
},
});
});

it("cannot be used with 1st gen", () => {
assertParserError({
endpoints: {
func: {
...MIN_ENDPOINT,
platform: "gcfv1",
genkitTrigger: {
flow: "agent",
},
},
},
});
});
});

describe("scheduleTriggers", () => {
const validTrigger: build.ScheduleTrigger = {
schedule: "every 5 minutes",
Expand Down
6 changes: 5 additions & 1 deletion src/deploy/functions/runtimes/discovery/v1alpha1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,9 @@ function assertBuildEndpoint(ep: WireEndpoint, id: string): void {
invoker: "array?",
});
} else if (build.isCallableTriggered(ep)) {
// no-op
assertKeyTypes(prefix + ".callableTrigger", ep.callableTrigger, {
genkitAction: "string?",
});
} else if (build.isScheduleTriggered(ep)) {
assertKeyTypes(prefix + ".scheduleTrigger", ep.scheduleTrigger, {
schedule: "Field<string>",
Expand Down Expand Up @@ -263,6 +265,7 @@ function assertBuildEndpoint(ep: WireEndpoint, id: string): void {
options: "object",
});
} else {
// TODO: Replace with assertExhaustive, which needs some type magic here because we have an any
throw new FirebaseError(
`Do not recognize trigger type for endpoint ${id}. Try upgrading ` +
"firebase-tools with npm install -g firebase-tools@latest",
Expand Down Expand Up @@ -310,6 +313,7 @@ function parseEndpointForBuild(
copyIfPresent(triggered.httpsTrigger, ep.httpsTrigger, "invoker");
} else if (build.isCallableTriggered(ep)) {
triggered = { callableTrigger: {} };
copyIfPresent(triggered.callableTrigger, ep.callableTrigger, "genkitAction");
} else if (build.isScheduleTriggered(ep)) {
const st: build.ScheduleTrigger = {
// TODO: consider adding validation for fields like this that reject
Expand Down
18 changes: 14 additions & 4 deletions src/functions/events/v2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,6 @@ export const FIRESTORE_EVENTS = [

export const FIREALERTS_EVENT = "google.firebase.firebasealerts.alerts.v1.published";

export const FIRESTORE_EVENT_REGEX = /^google\.cloud\.firestore\.document\.v1\.[^\.]*$/;
export const FIRESTORE_EVENT_WITH_AUTH_CONTEXT_REGEX =
/^google\.cloud\.firestore\.document\.v1\..*\.withAuthContext$/;

export type Event =
| typeof PUBSUB_PUBLISH_EVENT
| (typeof STORAGE_EVENTS)[number]
Expand All @@ -46,3 +42,17 @@ export type Event =
| typeof TEST_LAB_EVENT
| (typeof FIRESTORE_EVENTS)[number]
| typeof FIREALERTS_EVENT;

// Why can't auth context be removed? This is map was added to correct a bug where a regex
// allowed any non-auth type to be converted to any auth type, but we should follow up for why
// a functon can't opt into reducing PII.
export const CONVERTABLE_EVENTS: Partial<Record<Event, Event>> = {
"google.cloud.firestore.document.v1.created":
"google.cloud.firestore.document.v1.created.withAuthContext",
"google.cloud.firestore.document.v1.updated":
"google.cloud.firestore.document.v1.updated.withAuthContext",
"google.cloud.firestore.document.v1.deleted":
"google.cloud.firestore.document.v1.deleted.withAuthContext",
"google.cloud.firestore.document.v1.written":
"google.cloud.firestore.document.v1.written.withAuthContext",
};
40 changes: 40 additions & 0 deletions src/gcp/cloudfunctionsv2.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,23 @@ describe("cloudfunctionsv2", () => {
[BLOCKING_LABEL]: "before-sign-in",
},
});

expect(
cloudfunctionsv2.functionFromEndpoint({
...ENDPOINT,
platform: "gcfv2",
callableTrigger: {
genkitAction: "flows/flow",
},
}),
).to.deep.equal({
...CLOUD_FUNCTION_V2,
labels: {
...CLOUD_FUNCTION_V2.labels,
"deployment-callable": "true",
"genkit-action": "flows/flow",
},
});
});

it("should copy trival fields", () => {
Expand Down Expand Up @@ -637,6 +654,29 @@ describe("cloudfunctionsv2", () => {
});
});

it("should translate genkit callables", () => {
expect(
cloudfunctionsv2.endpointFromFunction({
...HAVE_CLOUD_FUNCTION_V2,
labels: {
"deployment-callable": "true",
"genkit-action": "flows/flow",
},
}),
).to.deep.equal({
...ENDPOINT,
callableTrigger: {
genkitAction: "flows/flow",
},
platform: "gcfv2",
uri: GCF_URL,
labels: {
"deployment-callable": "true",
"genkit-action": "flows/flow",
},
});
});

it("should copy optional fields", () => {
const extraFields: backend.ServiceConfiguration = {
ingressSettings: "ALLOW_ALL",
Expand Down
6 changes: 6 additions & 0 deletions src/gcp/cloudfunctionsv2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,9 @@ export function functionFromEndpoint(endpoint: backend.Endpoint): InputCloudFunc
gcfFunction.labels = { ...gcfFunction.labels, "deployment-taskqueue": "true" };
} else if (backend.isCallableTriggered(endpoint)) {
gcfFunction.labels = { ...gcfFunction.labels, "deployment-callable": "true" };
if (endpoint.callableTrigger.genkitAction) {
gcfFunction.labels["genkit-action"] = endpoint.callableTrigger.genkitAction;
}
} else if (backend.isBlockingTriggered(endpoint)) {
gcfFunction.labels = {
...gcfFunction.labels,
Expand Down Expand Up @@ -654,6 +657,9 @@ export function endpointFromFunction(gcfFunction: OutputCloudFunction): backend.
trigger = {
callableTrigger: {},
};
if (gcfFunction.labels["genkit-action"]) {
trigger.callableTrigger.genkitAction = gcfFunction.labels["genkit-action"];
}
} else if (gcfFunction.labels?.[BLOCKING_LABEL]) {
trigger = {
blockingTrigger: {
Expand Down
4 changes: 3 additions & 1 deletion templates/init/functions/typescript/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
{
"compilerOptions": {
"module": "commonjs",
"module": "NodeNext",
"esModuleInterop": true,
"moduleResolution": "nodenext",
"noImplicitReturns": true,
"noUnusedLocals": true,
"outDir": "lib",
Expand Down

0 comments on commit f98ffe4

Please sign in to comment.