From f98ffe4a65326d9e1bdf534151b4c77067ece441 Mon Sep 17 00:00:00 2001 From: Thomas Bouldin Date: Wed, 11 Dec 2024 15:23:30 -0800 Subject: [PATCH] Allow a Callable to include a Genkit Action annotation (#8039) 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. --- CHANGELOG.md | 3 ++ src/deploy/functions/backend.ts | 10 +++-- src/deploy/functions/build.ts | 9 +++-- src/deploy/functions/release/planner.spec.ts | 10 +++++ src/deploy/functions/release/planner.ts | 15 +++---- .../runtimes/discovery/v1alpha1.spec.ts | 29 ++++++++++++++ .../functions/runtimes/discovery/v1alpha1.ts | 6 ++- src/functions/events/v2.ts | 18 +++++++-- src/gcp/cloudfunctionsv2.spec.ts | 40 +++++++++++++++++++ src/gcp/cloudfunctionsv2.ts | 6 +++ .../init/functions/typescript/tsconfig.json | 4 +- 11 files changed, 130 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f98ea1c0f7..b17de18fd0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/src/deploy/functions/backend.ts b/src/deploy/functions/backend.ts index 0b5e4cd041a..113d1ac09dd 100644 --- a/src/deploy/functions/backend.ts +++ b/src/deploy/functions/backend.ts @@ -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 { @@ -41,7 +41,9 @@ export interface HttpsTriggered { } /** API agnostic version of a Firebase callable function. */ -export type CallableTrigger = Record; +export type CallableTrigger = { + genkitAction?: string; +}; /** Something that has a callable trigger */ export interface CallableTriggered { @@ -135,6 +137,7 @@ export interface BlockingTrigger { eventType: string; options?: Record; } + export interface BlockingTriggered { blockingTrigger: BlockingTrigger; } @@ -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 diff --git a/src/deploy/functions/build.ts b/src/deploy/functions/build.ts index 6dc379c545d..4e5a0e5f6ec 100644 --- a/src/deploy/functions/build.ts +++ b/src/deploy/functions/build.ts @@ -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. @@ -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)) { diff --git a/src/deploy/functions/release/planner.spec.ts b/src/deploy/functions/release/planner.spec.ts index 0e8de3127a3..80be985ef9f 100644 --- a/src/deploy/functions/release/planner.spec.ts +++ b/src/deploy/functions/release/planner.spec.ts @@ -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", { diff --git a/src/deploy/functions/release/planner.ts b/src/deploy/functions/release/planner.ts index 74a137b24d0..9ae8872e215 100644 --- a/src/deploy/functions/release/planner.ts +++ b/src/deploy/functions/release/planner.ts @@ -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; @@ -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] ); } @@ -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, diff --git a/src/deploy/functions/runtimes/discovery/v1alpha1.spec.ts b/src/deploy/functions/runtimes/discovery/v1alpha1.spec.ts index bba3b9b0484..f66349b0699 100644 --- a/src/deploy/functions/runtimes/discovery/v1alpha1.spec.ts +++ b/src/deploy/functions/runtimes/discovery/v1alpha1.spec.ts @@ -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", diff --git a/src/deploy/functions/runtimes/discovery/v1alpha1.ts b/src/deploy/functions/runtimes/discovery/v1alpha1.ts index cc3a6e00c42..0436335d364 100644 --- a/src/deploy/functions/runtimes/discovery/v1alpha1.ts +++ b/src/deploy/functions/runtimes/discovery/v1alpha1.ts @@ -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", @@ -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", @@ -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 diff --git a/src/functions/events/v2.ts b/src/functions/events/v2.ts index 15132856a93..cd897d9a5ee 100644 --- a/src/functions/events/v2.ts +++ b/src/functions/events/v2.ts @@ -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] @@ -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> = { + "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", +}; diff --git a/src/gcp/cloudfunctionsv2.spec.ts b/src/gcp/cloudfunctionsv2.spec.ts index 4d3b89eab35..ef9486f9b9f 100644 --- a/src/gcp/cloudfunctionsv2.spec.ts +++ b/src/gcp/cloudfunctionsv2.spec.ts @@ -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", () => { @@ -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", diff --git a/src/gcp/cloudfunctionsv2.ts b/src/gcp/cloudfunctionsv2.ts index af1cb92984a..6d17c607bbc 100644 --- a/src/gcp/cloudfunctionsv2.ts +++ b/src/gcp/cloudfunctionsv2.ts @@ -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, @@ -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: { diff --git a/templates/init/functions/typescript/tsconfig.json b/templates/init/functions/typescript/tsconfig.json index 7ce05d039d6..57b915f3cc9 100644 --- a/templates/init/functions/typescript/tsconfig.json +++ b/templates/init/functions/typescript/tsconfig.json @@ -1,6 +1,8 @@ { "compilerOptions": { - "module": "commonjs", + "module": "NodeNext", + "esModuleInterop": true, + "moduleResolution": "nodenext", "noImplicitReturns": true, "noUnusedLocals": true, "outDir": "lib",