feat(kits): place event-triggered functions by resource location - #3173
Conversation
Adds DATABASE_REGION and deploys the function to the Cloud Run region derived from it. Firestore multi-region locations are not Cloud Run regions, so nam5 and nam7 map to us-central1 and eur3 to europe-west1; regional locations pass through. With the param unset the function declares no region and the CLI resolves one, as before.
Adds DATABASE_REGION and returns the derived Cloud Run region from envDeployOptions, so the function deploys next to the database. Multi-region locations map to a region inside them; unset still means no declared region.
Adds DATABASE_REGION and applies the derived region to the controller schedule and both Firestore triggers. All functions of an instance must share a region, so a new test asserts every one of the three is pinned.
…location Adds DATABASE_REGION and spreads the derived region through the four shared option objects, covering the task queues, both Firestore triggers and the callable. Task queues resolve from the enqueuing function's own region, so an unpinned function would enqueue against a queue that does not exist; a new test asserts all eight are pinned.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the DATABASE_REGION parameter across multiple Firestore kits (firestore-counter, firestore-genai-chatbot, firestore-translate-text, and firestore-vector-search) to deploy Cloud Run functions to the region closest to the Firestore database location. It maps multi-region locations to valid Cloud Run regions and passes regional locations through unchanged. The review feedback suggests clarifying the README documentation across all kits to explain that the 'second deploy' behavior only occurs when bypassing the standard firebase kit:install command and running firebase deploy directly.
Adds BUCKET_REGION and deploys the function to the Cloud Run region derived from it, which a 2nd gen storage trigger requires. Cloud Storage multi-regions are not Cloud Run regions, so us maps to us-east1, eu to europe-west1 and asia to asia-east1, matching what firebase-tools picks. Dual-regions are not mapped.
Adds BUCKET_REGION and returns the derived region from envDeployOptions, so the function deploys in a region its bucket trigger accepts. Same mapping as storage-resize-images.
Adds DATABASE_REGION and deploys the function to that region, which a 2nd gen database trigger requires. Database locations are Cloud Run regions already, so the helper only trims and lowercases.
…ctions The functions were already deployed to LOCATION, but the param carried no label or description, so nothing said it has to match the Firestore database's region or that multi-region locations are not valid values.
IzaakGough
changed the title
The READMEs said a value entered at the prompt only applies from the second deploy. That is only true when the prompt happens during firebase deploy, with the value missing from .env. functions:kits:install and ext:migrate both write .env before anything deploys, so one deploy is enough. Corrects the two kits that shipped the claim as well. Also fixes subject/verb agreement in the single-function kits.
|
Thanks, that is right and it matches what I took the substance but not the wording, for two reasons. The command is
Applied to all eight kits in this PR, plus |
Four kits only tested the helper that derives the region, so deleting the spread from index.ts left every suite green. Each now imports the entry with the location stubbed and asserts on __endpoint.region. Also notes that pinning firestore-vector-search to the database's location moves its Vertex AI calls there too, which is new: the embedding functions were previously unplaced and ran in us-central1.
…tabase-region # Conflicts: # kits/storage-resize-images/CHANGELOG.md
Three READMEs still said the extension's location setting was gone and the function lands in the codebase default. Adding the placement param made that false. They now point at the new param and say why it describes the resource's location rather than a free choice. firestore-translate-text also ties its existing Vertex AI note to DATABASE_REGION, since that setting now decides where the Vertex call goes.
Sequencing against the Vertex AI location workThis PR interacts with #3162 and #2943, which take opposite routes on the same question, and #3028 is still the open decision. Worth settling that order before this merges. The interaction.
If #2943 wins instead and those kits move to the Suggested order: settle #3028, land whichever of #3162 / #2943 wins, then I rebase this on top and correct the notes to match. File overlap with #3162, whichever goes second needs a merge:
|
GCLOUD_LOCATION cannot redirect the Vertex AI call. A deployed function always resolves a region and passes it explicitly, so the plugin never reads the env var. The notes now say there is no override. Only firestore-bigquery-export and firestore-send-email have a legacy DATABASE_REGION to copy, so the claim that a copied value is honored is dropped from the four kits whose extensions never declared it.
The other kits trim and lowercase before the multi-region lookup, so a hand-edited NAM5 resolved there but passed straight through here and failed the deploy.
Date the region.ts copies added on this branch to 2026, and drop a test alias that pointed at the function declared beside it.
…on' into fix/kits-firestore-database-region
…tabase-region # Conflicts: # kits/firestore-translate-text/tests/index.test.ts
…tabase-region # Conflicts: # kits/firestore-genai-chatbot/CHANGELOG.md
The READMEs and CHANGELOGs said an unset region parameter falls back to no declared region. A parameter absent from `.env` is never resolved from its default: `resolveParams` fails a non-interactive deploy before defaults are read, so the fallback needs an explicit empty value. The storage kits also told dual-region bucket owners to leave `BUCKET_REGION` empty, which lands the function in `us-central1` and fails the trigger's region check. Name the member regions instead; all six are already in the list.
Two sections still said the functions follow the codebase default region, which the parameter sections below them now contradict.
|
Self-review. I checked the region claims against live deploys of the vector-search and storage-resize-images kits in a test project whose default Firestore database is in 1. The mismatch guidance contradicts itself, and incremental-capture has it the wrong way round. 2. "Lands in 3. 4. Not a problem: the empty- |
A Firestore trigger is created in the database's own region and delivers across regions, so a function region that does not match the database is a latency cost, not a failed deploy or a dead trigger. The incremental-capture README, its LOCATION description and its changelog entry said the opposite. On a first deploy with no DATABASE_REGION the CLI resolves each function separately: the Firestore-triggered ones land next to the database and the task, callable and scheduled ones land in us-central1. The six Firestore kit READMEs said the whole instance lands in us-central1. The storage and speech kits keep that wording, where it is accurate. Vertex AI embeds in the region the functions run in, and it serves gemini-embedding-001 in most regions. Named the five regions offered by DATABASE_REGION where it does not.
Storage and database triggers cannot cross regions, but the mismatch is rejected when the function is created, not by a trigger that silently never fires. Quoted the two errors: "A function in region ... cannot listen to a bucket in region ..." and "cannot register cross-region trigger". The vector-search single-region comment described an enqueue failure this kit cannot reach, since only the task functions enqueue and they share a region with their queues. Stated the constraint instead.
cabljac
left a comment
cabljac
left a comment
There was a problem hiding this comment.
@IzaakGough this looks like the right general design, the mappings match firebase-tools' location.ts and the mutation checks on the new tests hold up. Requesting changes for a few things, mostly docs:
- The send-email and bigquery-export READMEs say an empty DATABASE_REGION lands the function next to the database on first deploy. For those two kits I don't think that holds, see inline.
- The "region does not accept a param expression" comment is repeated across the kits and isn't true of the SDK. For RTDB that changes the design, see inline.
- This is breaking for existing installs: a required select with no default makes
firebase deploy --non-interactivethrow until .env gains the new key, and the interactive prompt has no "keep no region" option, so any answer deletes and recreates the function. The CHANGELOGs describe it but nothing flags it as breaking. Please add a BREAKING marker or an upgrade note to each. - PR body: "any database or bucket outside us-central1 failed the deploy" only holds for storage and RTDB. The Firestore kits deployed fine cross-region (#11020 says it succeeds silently, and the READMEs say so too), the win there is latency. Worth rewording so the motivation is accurate.
Tests: ran vitest per kit on the branch, all green except storage-resize-images content-filter.test.ts, which fails on a missing @genkit-ai/core/schema import and is pre-existing, not yours. No live deploy on my side, so please treat the region claims below as source-read rather than deploy-verified and tell me if you have seen otherwise.
| ? params.nodePath.toCEL() | ||
| : params.nodePath.value(); | ||
|
|
||
| // The region option does not accept a param expression, so the value is read |
There was a problem hiding this comment.
"The region option does not accept a param expression" isn't true of firebase-functions v7: options.d.ts declares region?: SupportedRegion | string | Expression<string> | ResetValue, and firebase-tools build.ts:509-521 resolves a string-param region. I checked onValueCreated({ region: defineString("DATABASE_REGION") }).__endpoint.region and it holds the Expression.
For the Firestore and storage kits the hardcoding still stands, the multi-region mapping needs a nested ternary the CLI's CEL can't express, so please reword the comment there to that constraint. For RTDB there is no mapping, so region: params.databaseRegion should work directly and would remove both README caveats (first-deploy prompt value being ignored, and placement depending on .env being present at discovery).
There was a problem hiding this comment.
The comment was wrong, thanks. I checked options.d.ts and then the manifest: onValueCreated({ region: defineString("DATABASE_REGION") }).__endpoint.region holds the Expression, stackToWire emits {{ params.DATABASE_REGION }}, and build.ts resolves it through resolveList/resolveString.
2d7be58 rewords it everywhere it appeared (8 kits). For the Firestore and storage kits it now names the real constraint: the location to Cloud Run region mapping needs a nested ternary, and cel.ts only parses identity, one comparison, and one ternary.
I've left RTDB reading process.env for now, and the comment there says why rather than claiming the option won't take an expression. Two things the expression would cost: the explicit empty DATABASE_REGION= escape hatch the README documents would resolve to a region of "" rather than to no region at all, and normalizeRegion's trim/lowercase for a hand-edited .env goes away. Both look worth keeping, but I haven't deploy-tested the empty case either way, so happy to be talked round if you think the first-deploy win outweighs them.
There was a problem hiding this comment.
Tried it, and you were right: done in 045896f. Ran real discovery on the built kit and resolved the manifest through firebase-tools' own buildFromV1Alpha1 / toBackend: the wire manifest carries {{ params.DATABASE_REGION }} and resolves to the param value, with no .env needed at discovery.
On the rc question, that isn't it. stackToWire emits the same {{ params.DATABASE_REGION }} on 6.3.2, 6.6.0, 7.0.0, 7.3.2 and 7.3.3-rc.2, and the kits are CJS so there's no dual-package hazard around the instanceof Expression check. The version sensitivity is on the CLI side and it favours the expression: the CEL region block in build.ts is unchanged back to 13.0.0, whereas reading process.env needs 15.28.0 exactly, since 15.27.0 passes only firebaseEnvs to discoverBuild and 15.28.0 added ...userEnvs.
One thing I dropped rather than kept: an empty DATABASE_REGION= now resolves to a region of "" rather than to no region, and nothing in validate.ts catches it. So the value is required, and the README no longer offers the empty line as a way to let the CLI choose. Anyone who wanted us-central1 can just pick it. Still not deploy-verified, only resolved offline.
There was a problem hiding this comment.
Worth flagging here since it bears on the mapping: firebase/firebase-tools#11026 is open and fixes #11020. It moves param evaluation ahead of region resolution and substitutes the trigger filters just for the lookup, so database: "{{ params.DATABASE }}" in eur3 resolves to europe-west1 through the CLI's own FIRESTORE_DUAL_REGION_TO_REGION_MAPPING and STORAGE_MULTI_REGION_TO_REGION_MAPPING. Review-required, last touched 2026-09-03.
If it lands, the tables in each kit's region.ts duplicate the ones the CLI already has, and the #11020 reason for the param goes away for storage-resize-images, speech-to-text and rtdb-limit-child-nodes, plus the two already shipped. The scope argument that dropped DATABASE_REGION from translate-text and genai-chatbot would then apply to those too. It would also remove the second-deploy caveat and the firebase-tools 15.28.0 floor these READMEs document, since the prompt would then run before region resolution rather than after it.
Two things it does not cover: firestore-counter and firestore-vector-search still need the param, since their scheduled, task and callable functions have no event source to resolve a region from, and it does nothing about Cloud Scheduler availability.
Going ahead with the param for now. #11026 is expected to land at some point, so this is a workaround with a known exit rather than a permanent feature, but it places the functions correctly on every CLI version and those two kits need it either way. Once #11026 ships, dropping the param from the three kits that only needed it for #11020 is a small follow-up.
The region option does accept a param expression; the constraint is that the location to Cloud Run region mapping needs a nested ternary the CLI's CEL subset cannot parse. Reword the comment in every kit that carried the wrong reason. send-email and bigquery-export pass the database as a param expression, so the CLI's default-region lookup sees an unresolved expression and falls back to us-central1 rather than placing the function next to the database (firebase-tools#11020). Say so in both READMEs. Flag the new parameter as breaking in each CHANGELOG, since an existing install needs the key in .env before its next deploy.
Realtime Database locations are Cloud Run regions already, so the value needs no mapping and can go to the region option as a CEL expression. The CLI substitutes it after prompting, so the region applies on the deploy that sets it rather than the one after, and discovery no longer has to see .env, which only firebase-tools 15.28.0 and later provides. The value is now required rather than optional. An explicit empty line resolved to an empty region string, which nothing validates, so the README no longer offers it as a way to let the CLI pick.
…tabase-region # Conflicts: # kits/firestore-translate-text/CHANGELOG.md # kits/firestore-vector-search/README.md # kits/storage-resize-images/CHANGELOG.md
CorieW
left a comment
CorieW
left a comment
There was a problem hiding this comment.
The code is correct: mappings match firebase-tools location.ts, region accepts a string param, tests cover unset, mapped and all-functions cases, and CI is green. Two items need resolving before merge.
-
Scope. firebase/firebase-tools#11020 only affects triggers whose
databaseis a param expression.firestore-counter,firestore-translate-text,firestore-genai-chatbotandfirestore-vector-searchtrigger on(default), so the CLI already infers the region (your self-review showsembedOnWriteinus-east1). Fortranslate-textandgenai-chatbot,DATABASE_REGIONis a new required prompt that changes nothing. Drop it from those two kits, or reframe the PR as "explicit single-region placement" and record that decision. Either way, fix the PR body: "the function lands inus-central1away from its database" is false for these four kits. -
Ledger. #2974 "Valid differences" records
LOCATION params have been removed in kits, as kits builds this feature in.This PR adds one to seven kits. Update the ledger and confirm the firebase-tools agreement from #3101 covers this rollout.
CorieW
left a comment
CorieW
left a comment
There was a problem hiding this comment.
Inline non-blocking notes, following up on my earlier review.
Both kits have one Firestore trigger on the default database and pass no database option, so the CLI already places the function next to the database and the param was a required prompt that changed nothing. The kits that keep it have a reason the CLI cannot cover: storage-resize-images, speech-to-text and rtdb-limit-child-nodes pass a param expression the region lookup cannot resolve (firebase/firebase-tools#11020), and firestore-counter and firestore-vector-search have scheduled, task and callable functions that get no inference at all and otherwise split away from their trigger. The two README sections are restored with their placement claim corrected: with no region declared the CLI places the function next to the database rather than in us-central1, and the FUNCTION_DEFAULT_REGION written by ext:migrate is not read.
|
Thanks, the scope point holds for two of the four kits and the param is gone from those in 1385a9e.
On the agreement: #3101 and #3102 rest it on the trigger's On #2974: the bullet stays true if it is scoped to what it meant. The extension's |
|
Following up on the ledger point: I have updated the entry in #2974 rather than leaving it as an offer. The The firebase-tools half of that point is still open. |
… regions The FUNCTION_DEFAULT_REGION that ext:migrate writes is read by nothing, so every README that keeps a region param now says placement comes from that param alone and a migrated instance can move on its next deploy. firestore-counter offers twelve database locations whose Cloud Run region has no Cloud Scheduler, which the scheduled controllerCore needs, so those values fail the deploy. The README lists them and says to pick the nearest location that has it, and the parameter description points there. Checked against the published region list and gcloud scheduler locations list, not against a failed deploy. The dual-region guidance in storage-resize-images and speech-to-text is marked as not deploy-verified, in the README and in the region helper. The released DATABASE_REGION bullets in firestore-send-email and firestore-bigquery-export are restored, with the empty-line correction added as a new entry rather than edited into them.
Deployed storage-resize-images against a nam4 bucket with BUCKET_REGION set to us-central1, one half of the pair. Eventarc created the trigger in nam4 pointing at the us-central1 function, and an upload produced the resized image, so the member-region advice holds. Naming nam4 itself fails with `Location nam4 is not found or access is unauthorized`. speech-to-text carries the same note, attributed to the kit it was run on.
…tabase-region # Conflicts: # kits/rtdb-limit-child-nodes/README.md
The .firebaserc and firebase.json used for the nam4 dual-region test deploy were committed by mistake. The package has no files field, so npm pack included both in the published tarball.
The Vertex AI note put europe-west10 in the no-endpoint list; its regional endpoint answers, so it belongs with the regions that report the model as not found. The vector-search changelog now says that pinning the region moves the Vertex AI call. firestore-incremental-capture's LOCATION description claimed an unlisted database region was unsupported, contradicting its README: the trigger is created in the database's region and delivers across regions. rtdb-limit-child-nodes attributes the cross-region rejection to the CLI's trigger handling rather than a live deploy, and DATABASE_REGION joins REQUIRED_PARAMS so a blank entry fails at discovery instead of reaching Cloud Run as an empty region.
3 participants
2nd gen Eventarc triggers are tied to the region of the resource they listen to, and the CLI cannot always place the function next to it. Two things go wrong.
For
storage-resize-images,speech-to-textandrtdb-limit-child-nodes, the trigger's bucket or instance is a param expression, so the CLI's region lookup sees the literal{{ params.X }}and falls back tous-central1(firebase/firebase-tools#11020). Any bucket or database outside that region fails the deploy.For
firestore-counterandfirestore-vector-search, the CLI places the Firestore-triggered functions correctly but gets no signal for the scheduled, task and callable ones, so an instance splits across regions: a live deploy with no region set putembedOnWriteandqueryOnWriteinus-east1next to the database and the other six functions inus-central1.Changes
Each kit gains a location param and deploys its functions to the Cloud Run region derived from it, following #3101 and #3102. Locations that are not themselves Cloud Run regions map to one inside them, using the same targets firebase-tools picks.
firestore-counter,firestore-vector-searchDATABASE_REGIONnam5/nam7tous-central1,eur3toeurope-west1storage-resize-images,speech-to-textBUCKET_REGIONustous-east1,eutoeurope-west1,asiatoasia-east1rtdb-limit-child-nodesDATABASE_REGIONfirestore-incremental-captureLOCATION, already presentEvery function in an instance is pinned, not just the triggered one, because
firestore-vector-searchandfirestore-counterresolve task queues and schedules from the enqueuing function's own region.firestore-translate-textandfirestore-genai-chatbotget no param. Each has a single Firestore trigger on the default database with nodatabaseoption, so the CLI already places it correctly and the param only added a required prompt. Their READMEs are corrected instead, since they claimed a no-region function lands inus-central1.firestore-bigquery-exportgains no param here, but its region helper now trims and lowercases the location like the other kits do, so a hand-editedNAM5resolves instead of passing through and failing the deploy.Testing
Unit tests per kit, plus reading the manifest each built output emits: every pinned function takes the region, and unset emits none. Region claims were checked against live deploys of
firestore-vector-searchandstorage-resize-images; the mapping itself is not deploy-verified on a multi-region database.Decisions for the reviewer
nam4,eur4,asia1) are not offered as values. The READMEs point those users at a member region instead, since a trigger in either half of the pair fires for the bucket, and all six are already in the list. Not deploy-verified.databasebeing a param expression and onext:migrateexportingDATABASE_REGION. Only thefirestore-send-emailandfirestore-bigquery-exportextensions declare that param, so this rollout needs confirming: on the #11020 basis for the storage and RTDB kits, and on the split-placement basis forfirestore-counterandfirestore-vector-search.LOCATIONparams were removed in kits. The entry needs updating to separate the extension's function-location param from these resource-location params.firestore-incremental-capturealready pinned all five functions toLOCATION, so it only gained a label and description. Confirm that reusing it beats addingDATABASE_REGIONbeside it.