feat: Add Eventarc deployment for Dart Cloud Run functions - #10851
feat: Add Eventarc deployment for Dart Cloud Run functions#10851Lyokone wants to merge 2 commits into
Conversation
![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 implements support for event-triggered Cloud Run functions (such as Dart functions) using Eventarc triggers. It introduces trigger lifecycle management (creation, updates, replacements, and deletion), IAM role provisioning for Eventarc delivery agents, API enablement checks, and deployment validations. Feedback on the changes suggests trimming trailing hyphens from the sliced prefix in eventarcTriggerIdForFunction to prevent consecutive hyphens, and refactoring the complex, nested try-catch blocks in upsertEventarcTrigger into helper functions to align with the repository's style guide on reducing nesting.
| const hash = crypto.createHash("sha256").update(fullId).digest("hex").slice(0, 8); | ||
| return `${fullId.slice(0, 54)}-${hash}`; |
There was a problem hiding this comment.
If fullId.slice(0, 54) ends with a hyphen, appending -${hash} will result in consecutive hyphens (e.g., --hash). While Eventarc trigger IDs can technically contain consecutive hyphens, it is cleaner and more robust to trim any trailing hyphens from the sliced prefix before appending the hash.
const hash = crypto.createHash(\"sha256\").update(fullId).digest(\"hex\").slice(0, 8);\n const prefix = fullId.slice(0, 54).replace(/-+$/, \"\");\n return prefix + \"-\" + hash;| } else { | ||
| let replacementStarted = false; | ||
| try { | ||
| await this.executor.run( | ||
| async () => { | ||
| const deleteOp = await eventarc.deleteTrigger(trigger.name); | ||
| replacementStarted = true; | ||
| await poller.pollOperation<void>({ | ||
| ...eventarcPollerOptions, | ||
| pollerName: `replace-eventarc-${endpoint.codebase}-${endpoint.region}-${endpoint.id}`, | ||
| operationResourceName: deleteOp.name, | ||
| }); | ||
| const op = await eventarc.createTrigger(reconciledTrigger); | ||
| await poller.pollOperation<eventarc.Trigger>({ | ||
| ...eventarcPollerOptions, | ||
| pollerName: `create-eventarc-${endpoint.codebase}-${endpoint.region}-${endpoint.id}`, | ||
| operationResourceName: op.name, | ||
| }); | ||
| }, | ||
| { retryCodes: [] }, | ||
| ); | ||
| } catch (err) { | ||
| if (replacementStarted) { | ||
| try { | ||
| const failedReplacement = await eventarc.getTrigger(trigger.name); | ||
| if (failedReplacement) { | ||
| const cleanupOp = await eventarc.deleteTrigger(trigger.name); | ||
| await poller.pollOperation<void>({ | ||
| ...eventarcPollerOptions, | ||
| pollerName: `cleanup-eventarc-${endpoint.codebase}-${endpoint.region}-${endpoint.id}`, | ||
| operationResourceName: cleanupOp.name, | ||
| }); | ||
| } | ||
| const rollbackOp = await eventarc.createTrigger(eventarc.triggerForCreate(existing)); | ||
| await poller.pollOperation<eventarc.Trigger>({ | ||
| ...eventarcPollerOptions, | ||
| pollerName: `rollback-eventarc-${endpoint.codebase}-${endpoint.region}-${endpoint.id}`, | ||
| operationResourceName: rollbackOp.name, | ||
| }); | ||
| previousTriggerRestored = true; | ||
| } catch (rollbackError) { | ||
| const rollbackMessage = | ||
| rollbackError instanceof Error ? rollbackError.message : String(rollbackError); | ||
| throw new FirebaseError( | ||
| `Failed to replace Eventarc trigger ${trigger.name} and failed to restore the previous trigger: ${rollbackMessage}`, | ||
| { original: err as Error }, | ||
| ); | ||
| } | ||
| } | ||
| throw err; | ||
| } | ||
| } |
There was a problem hiding this comment.
The nested try-catch blocks and conditional branches within upsertEventarcTrigger make the code quite complex and hard to follow, violating the repository style guide rule to 'Reduce nesting as much as possible'. Consider extracting the trigger replacement and rollback logic into a separate helper function (e.g., replaceEventarcTrigger) to improve readability and maintainability.
References
- Reduce nesting as much as possible. Code should avoid unnecessarily deep nesting or long periods of nesting. Handle edge cases early and exit or fold them into the general case. Consider helper functions that can completely encapsulate branching. (link)
c0d5bf2 to
37dff48
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #10851 +/- ##
=======================================
Coverage ? 58.65%
=======================================
Files ? 621
Lines ? 40812
Branches ? 8300
=======================================
Hits ? 23939
Misses ? 14866
Partials ? 2007 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
3 participants
Summary
Adds production deployment support for Eventarc-backed Dart functions running directly on Cloud Run.