fix(eventarc): set eventTrigger.region from channel to prevent unnecessary delete+recreate on deploy#1855
Conversation
…ssary delete+recreate on deploy
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request updates the onCustomEventPublished function in src/v2/providers/eventarc.ts to extract the region from the channel string and include it in the event trigger configuration. Feedback indicates that the regex used for region extraction is brittle and may lead to incorrect assignments if the channel format deviates from the expected pattern, suggesting a more robust parsing approach.
| const channelRegionMatch = channel.match(/locations\/([^/]+)\/channels\//); | ||
| const triggerRegion = channelRegionMatch ? channelRegionMatch[1] : "us-central1"; |
There was a problem hiding this comment.
The regex used to extract the region from the channel string is brittle. If the channel format changes or if the input does not match the expected pattern, it defaults to 'us-central1' silently. It would be safer to validate the channel format or use a more robust parsing method to avoid incorrect region assignment.
Problem
Functions defined with
onCustomEventPublishedare deleted and recreated on every deployment instead of being updated in-place. This causes:Root Cause
When
firebase-toolscompares the desired state ("want") against the deployed state ("have") during deployment planning, it callschangedTriggerRegion()insrc/deploy/functions/release/planner.ts:"have" side:
endpointFromFunction()infirebase-tools(cloudfunctionsv2.ts#L698-L703) correctly readstriggerRegionfrom the GCP API response and maps it toendpoint.eventTrigger.region(e.g.,"us-central1")."want" side: The
ManifestEndpointemitted byonCustomEventPublishedinfirebase-functionsnever setseventTrigger.region, so it remainsundefined.This means
undefined !== "us-central1"evaluates totrueon every deployment, forcingdeleteAndRecreate = true— even when nothing has changed.Fix
Derive the trigger region from the
channelstring (which already contains the region) and set it oneventTrigger.regionin the manifest endpoint. The channel follows the formatlocations/{region}/channels/{channel-id}, so the region is already available.This ensures
firebase-toolssees matching regions on both sides and performs an in-placePATCHupdate instead ofDELETE+POST.Reproduction
onCustomEventPublished:firebase deploy --only functions--debuglogs: the function isDELETEd thenPOSTed (created), notPATCHed (updated)Verification
After this fix,
--debugdeploy logs show:Instead of:
Impact
This affects all users of
onCustomEventPublished, including those using the Invertase Stripe extension's custom events. The fix is backward-compatible — it only adds a field that was already expected byfirebase-toolsbut never provided.