chore: upgrade Node.js version to 24 across Dockerfiles and workflows#9180
chore: upgrade Node.js version to 24 across Dockerfiles and workflows#9180mikeallisonJS wants to merge 11 commits into
Conversation
mikeallisonJS
requested review from
kiran-redhat,
sharon-elsa-mathew1995 and
tataihono
as code owners
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR updates Node.js versions to 24 across devcontainer images, GitHub Actions workflows (build/test/lint/e2e/deploy), Playwright/cache key namespaces, and the Arclight production Docker base image. ChangesNode.js 24 runtime upgrade
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit f6a474d
☁️ Nx Cloud last updated this comment at |
|
The latest updates on your projects.
|
|
Heads up — the matrix bump on the If the branch protection rule was already updated to require the Did you update the ruleset alongside this? Two nits while you're in there:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/e2e-tests.yml (1)
84-86:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate Playwright cache keys to Node 24.
These keys are still hardcoded to
node-22, which leaves this job using an outdated cache namespace after the runtime bump.Suggested patch
- key: ${{ runner.os }}-node-22-playwright-${{ hashFiles('pnpm-lock.yaml') }} + key: ${{ runner.os }}-node-24-playwright-${{ hashFiles('pnpm-lock.yaml') }} restore-keys: | - ${{ runner.os }}-node-22-playwright- + ${{ runner.os }}-node-24-playwright-🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/e2e-tests.yml around lines 84 - 86, The Playwright cache key strings in the e2e-tests workflow are still using the old node runtime namespace; update the cache key and restore-keys values that currently contain "node-22" to "node-24" (i.e., replace the string "node-22-playwright-" in the key and restore-keys entries), and scan the same workflow for any other "node-22" occurrences to update them consistently so the cache namespace matches the Node 24 runtime.
🧹 Nitpick comments (2)
.github/workflows/visual-test.yml (1)
4-4: Note: Workflow is currently disabled.The Node version update on line 17 is correct, but this workflow won't run until the trigger is changed from
branches: [never]to an active branch pattern. The upgrade will take effect when the workflow is re-enabled.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/visual-test.yml at line 4, The workflow is disabled by the trigger "branches: [never]"; replace that line with an active branch pattern (for example change branches: [never] to branches: [main] or branches: [main, release/*]) or configure triggers under "on:" (e.g., on: push and/or pull_request with branches) so the workflow actually runs; keep the Node version change you already made on line 17 unchanged.apps/arclight/Dockerfile (1)
1-34: ⚡ Quick winConsider adding a non-root user for production security.
The Dockerfile runs as root, which is flagged by Trivy (DS-0002). While this is a pre-existing issue and outside the scope of this Node upgrade, production containers should run as a non-root user to limit the impact of potential container escapes or compromised processes.
🔒 Suggested enhancement to add non-root user
FROM node:24-bullseye-slim EXPOSE 3000 ARG SERVICE_VERSION=0.0.1 ENV OTEL_RESOURCE_ATTRIBUTES="service.version=$SERVICE_VERSION" ENV VERCEL=true ENV PNPM_HOME="/usr/local/share/pnpm" ENV PATH="$PNPM_HOME:$PATH" RUN apt-get update && \ apt-get install -y --no-install-recommends \ build-essential \ python3 \ python3-pip \ libcurl4-openssl-dev \ postgresql-client && \ rm -rf /var/lib/apt/lists/* WORKDIR /app COPY ./dist/apps/arclight . COPY ./libs/prisma/media/db ./prisma-media/db COPY ./libs/prisma/media/prisma.config.ts ./prisma-media/prisma.config.ts COPY ./libs/prisma/languages/db ./prisma-languages/db COPY ./libs/prisma/languages/prisma.config.ts ./prisma-languages/prisma.config.ts COPY ./apps/arclight/docker-entrypoint.sh ./docker-entrypoint.sh # dependencies RUN mkdir -p $PNPM_HOME && corepack enable && corepack prepare pnpm@10.33.2 --activate RUN pnpm add -g next RUN pnpm install --prod --silent RUN pnpm add sharp pino dd-trace next-logger `@prisma/client` `@prisma/adapter-pg` dotenv prisma --silent + +# Create non-root user and switch to it +RUN groupadd -r nodeapp && useradd -r -g nodeapp nodeapp && \ + chown -R nodeapp:nodeapp /app +USER nodeapp + ENTRYPOINT ["./docker-entrypoint.sh"]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/arclight/Dockerfile` around lines 1 - 34, The Dockerfile currently runs as root; add a non-root user and switch to it before ENTRYPOINT to satisfy Trivy DS-0002. In the Dockerfile (near the end after creating directories and installing deps but before ENTRYPOINT), create a group/user (e.g., groupadd/app useradd or addgroup/adduser), chown the runtime directories (WORKDIR /app, $PNPM_HOME and any copied prisma dirs) to that user, and add a USER <username> line so the container runs unprivileged; keep ENTRYPOINT unchanged. Ensure the created user is non-login (nologin) and has ownership of /app and PNPM_HOME so pnpm and the app can run.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.devcontainer/Dockerfile:
- Line 2: The Dockerfile currently uses the deprecated namespace in the FROM
line; update the base image reference to the official namespace by changing the
FROM instruction that uses ARG VARIANT (the line starting with "FROM
mcr.microsoft.com/vscode/devcontainers/typescript-node:${VARIANT} AS base") to
use "mcr.microsoft.com/devcontainers/typescript-node:${VARIANT}" instead so the
build pulls the maintained Microsoft devcontainer image for Node 24.
In @.github/workflows/autofix.ci.yml:
- Around line 94-97: Update the inline comment that currently reads "lint (22)"
to match the matrix change to Node 24 so it reads "lint (24)"; specifically,
edit the comment block that mentions "Matrix `node-version: [24]`" and replace
the `lint (22)` reference with `lint (24)` so the comment accurately reflects
the new node-version/check name.
---
Outside diff comments:
In @.github/workflows/e2e-tests.yml:
- Around line 84-86: The Playwright cache key strings in the e2e-tests workflow
are still using the old node runtime namespace; update the cache key and
restore-keys values that currently contain "node-22" to "node-24" (i.e., replace
the string "node-22-playwright-" in the key and restore-keys entries), and scan
the same workflow for any other "node-22" occurrences to update them
consistently so the cache namespace matches the Node 24 runtime.
---
Nitpick comments:
In @.github/workflows/visual-test.yml:
- Line 4: The workflow is disabled by the trigger "branches: [never]"; replace
that line with an active branch pattern (for example change branches: [never] to
branches: [main] or branches: [main, release/*]) or configure triggers under
"on:" (e.g., on: push and/or pull_request with branches) so the workflow
actually runs; keep the Node version change you already made on line 17
unchanged.
In `@apps/arclight/Dockerfile`:
- Around line 1-34: The Dockerfile currently runs as root; add a non-root user
and switch to it before ENTRYPOINT to satisfy Trivy DS-0002. In the Dockerfile
(near the end after creating directories and installing deps but before
ENTRYPOINT), create a group/user (e.g., groupadd/app useradd or
addgroup/adduser), chown the runtime directories (WORKDIR /app, $PNPM_HOME and
any copied prisma dirs) to that user, and add a USER <username> line so the
container runs unprivileged; keep ENTRYPOINT unchanged. Ensure the created user
is non-login (nologin) and has ownership of /app and PNPM_HOME so pnpm and the
app can run.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5d819484-199c-4eb7-b42c-4cba2333a791
📒 Files selected for processing (18)
.devcontainer/Dockerfile.devcontainer/docker-compose.yml.github/workflows/ai-build-spike.yml.github/workflows/api-deploy-prod.yml.github/workflows/api-deploy-stage.yml.github/workflows/api-deploy-worker.yml.github/workflows/app-deploy.yml.github/workflows/autofix.ci.yml.github/workflows/danger.yml.github/workflows/e2e-tests.yml.github/workflows/ecs-frontend-deploy-prod-worker.yml.github/workflows/ecs-frontend-deploy-prod.yml.github/workflows/ecs-frontend-deploy-stage-worker.yml.github/workflows/ecs-frontend-deploy-stage.yml.github/workflows/main.yml.github/workflows/visual-test.yml.github/workflows/worker-deploy.ymlapps/arclight/Dockerfile
Switch from the deprecated mcr.microsoft.com/vscode/devcontainers/ namespace to mcr.microsoft.com/devcontainers/. The legacy namespace does not publish a typescript-node:24 tag; the maintained namespace is the only one with Node 24 support. Addresses PR review feedback.
Review feedback addressed (402debc)Fixed:
Skipped (already resolved):
|
|
I see you added the "on stage" label, I'll get this merged to the stage branch! |
|
Merge conflict attempting to merge this into stage. Please fix manually. |
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…ctly setting node version to 24
2 participants
Summary by CodeRabbit