Skip to content

Node 22 and Azure Functions v4 migration#21

Open
MarcAstr0 wants to merge 23 commits intoboostercloud:mainfrom
Optum:node_22
Open

Node 22 and Azure Functions v4 migration#21
MarcAstr0 wants to merge 23 commits intoboostercloud:mainfrom
Optum:node_22

Conversation

@MarcAstr0
Copy link
Collaborator

@MarcAstr0 MarcAstr0 commented Mar 6, 2026

Description

This PR adds Node 22 support to the File Uploads 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

  • Updated rocket-file-uploads-azure-infrastructure package to generate v4-compatible Azure Functions
  • Updated rocket-file-uploads-azure to work with the v4 @azure/functions SDK
  • Added overrides to package.json to address several vulnerabilities across dependencies
  • Added .nvmrc file
  • Modified lerna.json since Lerna 9 uses configuration in package.json instead
  • Added test configs compatible with Node 22
  • Updated dependencies across all packages to support Node 22

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR migrates the Booster File Uploads Rocket from Node 20 / Azure Functions v3 programming model to Node 22 / Azure Functions v4 programming model, aligning it with the Booster framework v4.x release. The infrastructure code now generates v4-compatible function definitions as JavaScript code strings, and the runtime code uses InvocationContext.triggerMetadata instead of the v3 Context.bindingData.

Changes:

  • Updated all packages to use @boostercloud/framework-* v4.0.0 peer/dev dependencies, along with Node 22 type definitions and test tooling updates
  • Migrated rocket-file-uploads-azure to use the @azure/functions v4 SDK (InvocationContext instead of Context) and rocket-file-uploads-azure-infrastructure to generate v4 function code strings via mountFunctionsV4
  • Added vulnerability overrides in root package.json, added .nvmrc, cleaned up lerna.json, and removed gitHead entries

Reviewed changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
.nvmrc Added Node version file pointing to lts/jod (Node 22)
package.json Updated lerna, metadata-booster versions; added vulnerability overrides; updated clean-compile script
lerna.json Removed legacy command.add and command.bootstrap configuration
packages/rocket-file-uploads-azure/src/file-uploaded.ts Migrated from v3 Context.bindingData to v4 InvocationContext.triggerMetadata
packages/rocket-file-uploads-azure/test/file-uploaded.test.ts Updated tests to use v4 triggerMetadata format with { blob, context } request shape
packages/rocket-file-uploads-azure/package.json Updated @azure/functions to v4, Node 22 types, framework v4 deps
packages/rocket-file-uploads-azure/.mocharc.yml Added no-experimental-strip-types node option for Node 22
packages/rocket-file-uploads-azure-infrastructure/src/index.ts Replaced mountFunctions with mountFunctionsV4; fixed bind target from Synth to Functions
packages/rocket-file-uploads-azure-infrastructure/src/functions/functions.ts Rewrote to generate v4 function code strings via FunctionAppV4Definitions
packages/rocket-file-uploads-azure-infrastructure/src/functions/rocket-files-file-uploaded-function.ts Replaced JSON-based function definition with generated JavaScript code template
packages/rocket-file-uploads-azure-infrastructure/src/synth/terraform-function-app.ts Updated node version to ~22, removed outdated v3 comment
packages/rocket-file-uploads-azure-infrastructure/package.json Updated cdktf, azurerm provider, Node 22 types, framework v4 deps
packages/rocket-file-uploads-types/package.json Updated framework v4 deps, minimatch, mocha, Node 22 types
packages/rocket-file-uploads-types/.mocharc.yml Added no-experimental-strip-types node option for Node 22
packages/rocket-file-uploads-core/package.json Updated framework v4 deps
packages/rocket-file-uploads-local/package.json Updated framework v4 deps
packages/rocket-file-uploads-local-infrastructure/package.json Updated framework v4 deps, azurerm provider

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 148 to 163
describe('getMetadataFromRequest', () => {
it('extracts bindingData from Azure Functions context', () => {
const mockContext = {
bindingData: {
blobTrigger: 'test-container/test-path/file.txt',
invocationId: 'test-id',
},
it('extracts triggerMetadata from Azure Functions v4 InvocationContext', () => {
const mockRequest = {
blob: Buffer.from('test content'),
context: {
triggerMetadata: {
blobTrigger: 'test-container/test-path/file.txt',
invocationId: 'test-id',
}
}
}

const result = getMetadataFromRequest(mockContext)
const result = getMetadataFromRequest(mockRequest)

expect(result).to.deep.equal(mockContext.bindingData)
expect(result).to.deep.equal(mockRequest.context.triggerMetadata)
})
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The getMetadataFromRequest function now has error handling for missing context or triggerMetadata (lines 9-11 in file-uploaded.ts), but the test suite only covers the happy path. Consider adding tests for error cases, such as when context is missing or when triggerMetadata is null/undefined, to verify the descriptive error is thrown.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 18 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.

Copy link

@alvaroloes alvaroloes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex: I found one blocking issue in the generated v4 functions.js for multi-storage configurations. Right now the concatenated output redeclares the shared imports and fails to load, so I’d fix that before merging.

Comment on lines +21 to +28
const functionsCode = configuration.userConfiguration
.map((userConfiguration) =>
RocketFilesFileUploadedFunction.generateFunctionsCode(
userConfiguration.containerName,
userConfiguration.storageName,
),
)
)
.join('\n')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex: generateFunctionsCode() returns a full top-level snippet, including const { app } = require('@azure/functions') and const { boosterRocketDispatcher } = require('./dist/index'). Because we concatenate one snippet per userConfiguration, any app with 2+ storage configs generates a functions.js that redeclares those constants and fails to load with SyntaxError: Identifier 'app' has already been declared. A shared-import prelude (like the webhook rocket uses) is needed here.

Copy link

@alvaroloes alvaroloes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude: Exhaustive Code Review — Node 22 & Azure Functions v4 Migration (File Uploads)


🟢 General Assessment

Well-structured migration, very consistent with the sibling PR on rocket-webhook. The move to Azure Functions v4 programmatic model for blob triggers is clean. Good additions: input validation on generated code, updated tests, mocha config for Node 22 compatibility.


🔴 Bug Fix (already included — good catch!)

packages/rocket-file-uploads-azure-infrastructure/src/index.ts

Same bind() context fix as in the webhook rocket:

// Before (bug):
mountFunctions: Functions.mountFunctions.bind(Synth, configuration),
// After (fixed):
mountFunctionsV4: Functions.mountFunctionsV4.bind(Functions, configuration),

Worth calling out explicitly in the PR description.


🟢 Good Practices

Input validation in rocket-files-file-uploaded-function.ts

const VALID_NAME_PATTERN = /^[a-z0-9_-]+$/
function validateName(value: string, label: string): void { ... }

Excellent addition. Since containerName and storageName are interpolated into template literals that become executable code, this validation prevents injection attacks. This is better than the webhook PR which uses JSON.stringify() but doesn't validate.

.mocharc.ymlno-experimental-strip-types

Good catch for Node 22 compatibility. Node 22 enables experimental type stripping by default which can interfere with TypeScript test execution.


🟡 Suggestions & Observations

1. rocket-files-file-uploaded-function.ts — Duplicated require per function

Each generated function block includes its own require statements:

const { app } = require('@azure/functions')
const { boosterRocketDispatcher } = require('./dist/index')

If there are multiple storage containers configured, these require calls are repeated. Unlike the webhook PR which uses a shared sharedImports() method called once at the top, this generates redundant imports. Node.js caches require calls so there's no runtime cost, but it's unnecessary code bloat and inconsistent with the webhook rocket's approach.

Suggestion: Extract shared imports to the top, similar to the webhook rocket pattern:

static sharedImports(): string {
  return `const { app } = require('@azure/functions')\nconst { boosterRocketDispatcher } = require('./dist/index')\n`
}

Then in Functions.mountFunctionsV4, prepend once:

const functionsCode = RocketFilesFileUploadedFunction.sharedImports() + registrations

2. file-uploaded.ts — Loose type assertion

const { context } = request as { blob: Buffer; context: InvocationContext }

The destructuring assumes the exact shape { blob, context }. If the v4 SDK changes the handler signature or additional properties are passed, this could silently break. Consider a more defensive approach:

const req = request as Record<string, unknown>
const context = req.context as InvocationContext | undefined
if (!context?.triggerMetadata) {
  throw new Error('InvocationContext.triggerMetadata is missing...')
}

3. file-uploaded.ts — Return type is too loose

return context.triggerMetadata as Record<string, unknown>

The return type Record<string, unknown> loses all type information. Downstream code accesses metadata.blobTrigger as string directly. Consider defining a minimal interface:

interface BlobTriggerMetadata {
  blobTrigger: string
  [key: string]: unknown
}

4. rocket-files-file-uploaded-function.ts — Inconsistency with webhook PR

The webhook PR uses JSON.stringify() for interpolation safety:

route: ${JSON.stringify(endpoint)},

This PR uses template literals after validation:

path: '${containerName}/{name}',

Both approaches are safe (this one validates input, the other escapes output), but the inconsistency between sibling rockets could confuse future maintainers. Consider aligning on one approach across both PRs.

5. Missing: Semver version bump

lerna.json version is still 2.0.3. Since peer deps are bumped from >=3.1.2 to >=4.0.0 (breaking change), this should be bumped to 3.0.0 following semver.

6. package.jsonclean-compile script updated correctly

"clean-compile": "lerna run clean && npm ci && lerna run compile"

Good update replacing lerna clean --yes && lerna bootstrap with npm ci. Consistent with Lerna 9 migration.

7. terraform-function-app.ts — Stale comment removed

-  functionsExtensionVersion: '~4', // keep it on version 3. Version 4 needs a migration process
+  functionsExtensionVersion: '~4',

Good cleanup — the comment was outdated since this PR is the migration to v4.

8. Test changes — Well done

The test updates in file-uploaded.test.ts are thorough:

  • Updated mock objects from v3 Context.bindingData to v4 { blob, context } format
  • Renamed test data from business-specific names to generic ones (good for OSS readability)
  • All existing test scenarios preserved

9. minimatch version bump

"minimatch": "3.1.5"  // was "3.0.5"

minimatch@3.0.5 had a known ReDoS vulnerability (CVE-2022-3517). The bump to 3.1.5 patches it. However, note that minimatch@3.x is deprecated — the latest is 10.x. Consider upgrading if there are no compatibility concerns with the glob matching behavior.


📋 Summary

Category Status
Node 22 compatibility
Azure Functions v4 migration
Vulnerability overrides
bind() context bug fix
Input validation (code gen) ✅ Excellent
Test updates ✅ Thorough
Mocha Node 22 compat
Duplicated requires ⚠️ Minor code bloat
Type safety in file-uploaded.ts ⚠️ Could be tighter
Cross-repo consistency ⚠️ Different patterns
Semver version bump ❌ Missing

Overall: Approve with suggestions. This PR is slightly more polished than the webhook sibling (validation, tests, stale comment cleanup). Main gap is the missing semver version bump and the require duplication pattern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants