feat: support Task lifecycle hooks in functions#10728
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 introduces support for GCF Gen 2 lifecycle hooks (afterInstall and afterUpdate) in Firebase Functions deployments, including manifest parsing, backend validation, and execution of task queue hooks via Cloud Tasks. The review feedback highlights several critical issues: a type mismatch when calling getProjectNumber, a logic bug in codebase filtering that causes hooks to be skipped for the default codebase, validation failures during partial deployments, a potential runtime TypeError when parsing null lifecycle hooks, and the silent skipping of unsupported callable and http hooks during execution.
wandamora
force-pushed
the
morawand-cf3-lifecycle-hooks
branch
from
f688cf8 to
f1e0350
Compare
wandamora
changed the title
inlined
left a comment
inlined
left a comment
There was a problem hiding this comment.
Done with a pass. Nothing foundational needs to be changed and I'm off today, so I'm LGTMing
| const wantBackend = backend.merge(...Object.values(payload.functions).map((p) => p.wantBackend)); | ||
| printTriggerUrls(wantBackend, projectNumber); | ||
|
|
||
| for (const [codebase, { wantBackend: w, haveBackend: h }] of Object.entries(payload.functions)) { |
There was a problem hiding this comment.
Printing URLs should probably be at the end users don't have to dig through the output to get them
There was a problem hiding this comment.
AFAICT this already does? At least it was for when I was deploying functions in a test project.
| backendSpec: backend.Backend, | ||
| targetId: string, | ||
| ): backend.Endpoint | undefined { | ||
| for (const endpoint of backend.allEndpoints(backendSpec)) { |
There was a problem hiding this comment.
Was just looking up the backend utility method (findEndpoint) and realized that there's an edge case we're not handling here where there may be multiple functions with the same name in different regions. Though I guess that shouldn't be a problem for task queue functions (HTTP functions can trigger this by specifying multiple regions)
There was a problem hiding this comment.
Should we address that in a separate CL? This seems like a non-blocking issue for this CL.
1. Adds validation of each trigger 2. Ensures they are prefixed properly 3. Resolves SA for OIDC token 4. Outputs a Cloud COnsole log
…ors during validation
wandamora
force-pushed
the
morawand-cf3-lifecycle-hooks
branch
2 times, most recently
from
1da9c0e to
b2c6ec6
Compare
- Add explanatory comments - Update logging - Change LifecycleHook to union type - Add assertExhaustive checks - Update project number detection
wandamora
force-pushed
the
morawand-cf3-lifecycle-hooks
branch
from
b2c6ec6 to
dbe17dd
Compare
3 participants
Description
This PR introduces support for Cloud Functions (CF3) lifecycle hooks, allowing developers to run actions after deployment events. This only supports Task Queue hooks. Execution of Callable and HTTP hooks will be implemented in a future CL.
The implementation spans:
lifecycleHooksconfig from the generated functions build manifest/metadata. Support is added to definetask(Task Queue functions),call(Callable functions), andhttp(HTTP functions or custom URLs) hooks.afterInstallhook target) or a subsequent update (afterUpdatehook target).cloudtasks.ts) to enqueue tasks targeting hook functions. Resolves the service account to attach anoidcTokenso that invocations are securely authenticated.taskhook targeting a task queue function), and are GCF Gen 2 functions.Scenarios Tested