fix(kits): declare the Cloud Firestore API in every kit that uses it - #3119
fix(kits): declare the Cloud Firestore API in every kit that uses it#3119IzaakGough wants to merge 1 commit into
Conversation
None of the kits declared firestore.googleapis.com, so a deploy never enables it. Most have a Firestore trigger, and ensureTriggerRegions looks the database up at deploy time, which fails early with a clear message. Four kits use Firestore without a Firestore trigger, so nothing checked them and the failure landed at runtime as an HTTP 500. Enabling the API does not provision a database, so this is not a complete fix for an empty project. It makes the dependency visible at install and removes the silent runtime failure. Drops the narrow firebase-functions/v2/options import in firestore-incremental-capture. requiresAPI is only exported from the v2 barrel, and the failure the comment described no longer reproduces on firebase-functions 7.3.2.
There was a problem hiding this comment.
Code Review
This pull request updates multiple integration kits to explicitly declare a dependency on the Cloud Firestore API (firestore.googleapis.com) by adding it to their REQUIRED_APIS lists and invoking requiresAPI. It also adds a corresponding unit test to verify this requirement in the translation kit. I have no feedback to provide as there are no review comments.
CorieW
left a comment
There was a problem hiding this comment.
The Firestore API declarations look correct and address the reported no-trigger runtime failure. The changed-kit CI jobs are green.
Non-blocking follow-up: while touching kits/firestore-incremental-capture/src/index.ts, consider also declaring bigquery.googleapis.com and dataflow.googleapis.com. The predecessor extension manifest declares both, and the linked issue discussion identified them in the same audit. The required setup script currently enables them out of band, so I do not consider this relevant enough to block this focused Firestore change.
It would also be useful, but non-blocking, to add a direct regression assertion for firestore-bundle-builder, since that is the no-Firestore-trigger path reported in #3116.
Reported in #3116 against
firestore-bundle-builder, which returns an HTTP 500PERMISSION_DENIEDon a project wherefirestore.googleapis.comis disabled.What was broken. No kit declared
firestore.googleapis.com, so a functions deploy never enables it.wantBackend.requiredAPIsis populated only fromrequiresAPI()declarations, and the API is in neither the gen2 always-enabled set norSTANDARD_APIS. Most kits have a Firestore trigger, soensureTriggerRegionslooks the database up at deploy time and fails early with a usable message. Four kits use Firestore without a Firestore trigger (firestore-bundle-builder,bigquery-firestore-export,delete-user-data,speech-to-text), so nothing checked them and the failure landed at runtime.What changed. Adds
firestore.googleapis.comtoREQUIRED_APISin the 11 kits that use Firestore.rtdb-limit-child-nodesandstorage-resize-imagesdo not touch it and are unchanged.firestore-genai-chatbotis included: it never constructs a client, but it writes through the trigger eventDocumentReference.firestore-incremental-capturealso moves off the narrowfirebase-functions/v2/optionsimport, sincerequiresAPIis only exported from the v2 barrel. The comment there said the barrel fails to load without@firebase/app; that does not reproduce on firebase-functions 7.3.2, verified by loading the compiledlib/index.jswith@firebase/appunresolvable.Verification. All 11 kits build and their suites pass. Added an assertion in
firestore-translate-text, the one kit with an existingrequiresAPItest harness, and confirmed it fails when the declaration is removed. The firebase-tools behaviour above comes from reading the installed 15.29.0 source, not from a live deploy.For the reviewer. Enabling the API does not provision a Firestore database, so this does not by itself make a deploy succeed on a project that has none. It makes the dependency visible at install and turns a silent runtime 500 into an explicit declaration. Whether the CLI should also check for a database on non-triggered functions is a firebase-tools question, not addressed here.