Node 22 and Azure Functions v4 migration#13
Node 22 and Azure Functions v4 migration#13MarcAstr0 wants to merge 10 commits intoboostercloud:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates the Azure Webhook Rocket to Node 22 and Azure Functions v4 compatibility (Booster Framework v4.x), while also updating dependency constraints and applying dependency overrides to address vulnerabilities.
Changes:
- Bump peer dependency compatibility across packages from Booster Framework v3.x to v4.x.
- Update Azure infrastructure generation for Functions v4 (including Node 22 runtime in the Function App stack and new V4 function mounting path).
- Update tooling/dependencies (Lerna 9, cdktf/provider versions, Node typings) and add root
overridesfor vulnerability mitigation.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/rocket-webhook-types/package.json | Update peer dependency minimums to Booster Framework v4. |
| packages/rocket-webhook-core/package.json | Update peer dependency minimums to Booster Framework v4. |
| packages/rocket-webhook-local-infrastructure/package.json | Update peer deps to v4, bump express, update local infra provider + Node typings for Node 22. |
| packages/rocket-webhook-azure-infrastructure/package.json | Update peer deps/dev deps to Booster v4 ecosystem, bump cdktf/provider + Node typings. |
| packages/rocket-webhook-azure-infrastructure/src/synth/terraform-function-app.ts | Set Azure Function App Node runtime to ~22 and Functions extension ~4. |
| packages/rocket-webhook-azure-infrastructure/src/synth/synth.ts | Minor refactor/formatting; continues wiring gateway overrides to the Function App. |
| packages/rocket-webhook-azure-infrastructure/src/index.ts | Switch infra rocket entrypoint to expose mountFunctionsV4. |
| packages/rocket-webhook-azure-infrastructure/src/functions/functions.ts | Implement mountFunctionsV4 returning v4 function app definitions and generated code. |
| packages/rocket-webhook-azure-infrastructure/src/functions/webhook-function.ts | Replace v3 function.json-style definition with generated @azure/functions v4 registration code. |
| package.json | Update peer deps to v4, bump lerna, add overrides to mitigate vulnerabilities. |
| lerna.json | Remove legacy command config (bootstrap/hoist settings). |
| .nvmrc | Update Node LTS to Node 22 (lts/jod). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/rocket-webhook-azure-infrastructure/src/functions/webhook-function.ts
Outdated
Show resolved
Hide resolved
packages/rocket-webhook-azure-infrastructure/src/functions/webhook-function.ts
Outdated
Show resolved
Hide resolved
packages/rocket-webhook-azure-infrastructure/src/functions/functions.ts
Outdated
Show resolved
Hide resolved
packages/rocket-webhook-azure-infrastructure/src/functions/webhook-function.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/rocket-webhook-azure-infrastructure/src/functions/webhook-function.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/rocket-webhook-azure-infrastructure/src/functions/webhook-function.ts
Show resolved
Hide resolved
packages/rocket-webhook-azure-infrastructure/src/functions/webhook-function.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/rocket-webhook-azure-infrastructure/src/functions/webhook-function.ts
Show resolved
Hide resolved
alvaroloes
left a comment
There was a problem hiding this comment.
Codex: I found one compatibility regression in the generated v4 Azure function names. The new normalization can merge distinct webhook routes into the same registration name, so I’d fix that before merging this migration.
|
|
||
| static generateFunctionsCode(endpoint: string): string { | ||
| return ` | ||
| app.http(${JSON.stringify(endpoint.replace(/[^a-zA-Z0-9_]/g, '_'))}, { |
There was a problem hiding this comment.
Codex: This normalization is broader than the previous v3 naming logic (/ -> _) and can collapse distinct routes into the same Azure function name. For example, foo-bar and foo/bar now both register as foo_bar, so two different webhooks end up sharing one function identity in functions.js. That is a compatibility regression for existing apps that already use both patterns.
alvaroloes
left a comment
There was a problem hiding this comment.
Claude: Exhaustive Code Review — Node 22 & Azure Functions v4 Migration
🟢 General Assessment
Solid migration PR. The move from Azure Functions v3 model (function.json + bindings) to the v4 programmatic model (app.http(...)) is well-structured. Dependency bumps and vulnerability overrides look reasonable. A few items worth addressing below.
🔴 Bug Fix (already included — good catch!)
packages/rocket-webhook-azure-infrastructure/src/index.ts
The bind() context was previously Synth for both Functions.mountFunctions and Functions.getFunctionAppName. This PR correctly fixes it to Functions:
// Before (bug):
mountFunctions: Functions.mountFunctions.bind(Synth, params),
// After (fixed):
mountFunctionsV4: Functions.mountFunctionsV4.bind(Functions, params),This was a latent bug — Synth doesn't have these static methods, so this inside them would have been wrong. Worth mentioning in the PR description as a fix.
🟡 Suggestions & Observations
1. webhook-function.ts — No input validation on endpoint
The function name is sanitized via endpoint.replace(...), and the route uses JSON.stringify(endpoint) (safe against injection), but there's no validation that endpoint is a reasonable value (non-empty string). Consider adding a guard:
if (!endpoint || typeof endpoint !== 'string') {
throw new Error(`Invalid webhook endpoint: "${endpoint}"`)
}2. webhook-function.ts — Multipart handling may lose binary data
const buffer = Buffer.from(await request.arrayBuffer())
rawBody = buffer.toString() // defaults to UTF-8buffer.toString() with UTF-8 encoding can corrupt binary multipart content (e.g., file uploads). If rawBody is only used for logging, this is acceptable. If downstream consumers rely on rawBody for multipart payloads, consider buffer.toString('latin1') or storing the raw buffer directly.
3. webhook-function.ts — Silent JSON parse error
The catch (e) block silently falls back to rawBody. Consider adding a trace for production troubleshooting:
} catch (e) {
context.warn('Body is not valid JSON, using raw text')
body = rawBody
}4. webhook-function.ts — Response body serialization edge case
const responseBody = ((typeof result.body === 'object' && result.body !== null) && !Buffer.isBuffer(result.body))
? JSON.stringify(result.body) : result.bodyDoesn't handle exotic objects (Date, Map, Set). If the contract guarantees only plain objects/strings/Buffers, a comment documenting this assumption would help.
5. Missing: Semver version bump
lerna.json version is still 1.0.1. Since peer deps are bumped from >=3.1.2 to >=4.0.0 (breaking change), this should be bumped to 2.0.0 following semver.
6. Missing: Tests for generated handler code
No test changes in this PR. The v4 handler is generated as a string, making it impossible to unit test directly. Consider adding integration tests or extracting handler logic into a testable module.
7. lerna.json — Verify CI replacement
The command.bootstrap block was removed (correct for Lerna 9). Verify that npm ci in CI correctly replaces the old lerna bootstrap --hoist --strict behavior.
8. synth.ts — Formatting mixed with logic
Changes are purely cosmetic (trailing commas, semicolons). Mixing formatting with functional changes makes diffs harder to review. Consider separating in future PRs.
📋 Summary
| Category | Status |
|---|---|
| Node 22 compatibility | ✅ |
| Azure Functions v4 migration | ✅ |
| Vulnerability overrides | ✅ |
bind() context bug fix |
✅ |
| Input validation | |
| Multipart binary handling | |
| Semver version bump | ❌ Missing |
| Tests | ❌ Missing |
Overall: Approve with suggestions. Core migration is correct and well-executed. Main gaps are the missing version bump and lack of tests for the generated handler code.
## Description
This PR adds Node 22 support to the Webhook Rocket. The changes in this PR are similar to those introduced in #1593 of the framework. This PR also addresses several vulnerabilities found in the dependencies.
This version of the rocket is compatible with 4.x versions of the main framework.
Changes
rocket-webhook-azure-infrastructurepackage to generate v4-compatible Azure Functionsrocket-webhook-azureto work with the v4@azure/functionsSDKpackage.jsonto address several vulnerabilities across dependencies