feat(build-node): multi-package / multi-asset-path support via inputs - #44
feat(build-node): multi-package / multi-asset-path support via inputs#44maikschneider wants to merge 2 commits into
Conversation
|
Warning Review limit reached
Next review available in: 34 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. WalkthroughThe build-node.yml GitLab CI job template is refactored from a fixed, single-project job into a reusable, parameterized template driven by spec.inputs. New inputs configure the Docker image, stage, package directories, install/build commands, artifact paths and expiry, and cache paths/keys. The job now loops over one or more package directories to install and build, replacing the prior root-only node_modules cache and fixed artifact/cache configuration. The existing rules block is retained unchanged. Estimated code review effort: 3 (Moderate) | ~20 minutes Changes
Sequence Diagram(s)sequenceDiagram
participant CI as GitLab CI Pipeline
participant Job as build-node Job
participant Dir as Package Directory Loop
participant Cache as Cache Store
participant Artifacts as Artifact Store
CI->>Job: Trigger with spec.inputs (image, package_dirs, commands)
Job->>Cache: Restore cache (cache_key_prefix, cache_key_files)
loop for each dir in package_dirs
Job->>Dir: Run install_command
Job->>Dir: Run build_command
end
Job->>Cache: Save updated cache
Job->>Artifacts: Collect artifact_paths (expire_in)
Job-->>CI: Report job result via rules
Related issues: None specified. 🐰 A rabbit hops through YAML fields anew, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Rework build-node.yml with spec:inputs so multi-package projects (e.g. TYPO3 monorepos with several site packages) no longer need to override the whole job. - package_dirs: space-separated list of dirs; installs + builds each in order - artifact_paths / cache_paths / cache_key_files: configurable via inputs - install_command / build_command / stage / artifacts_expire_in configurable - image passed as required input (pin + Renovate-manage it in the project) Defaults keep the classic single-package behaviour for existing consumers.
d9708e9 to
4415ce9
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
build-node.yml (1)
65-70: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
NPM_CONFIG_CACHEis hardcoded independent ofcache_paths.
NPM_CONFIG_CACHEis fixed to"$CI_PROJECT_DIR/.npm", but the actual cached paths are fully driven by thecache_pathsinput. If a consumer overridescache_pathsand omits.npm/(e.g. for a multi-package setup that only listspackages/*/node_modules/), the npm cache silently stops being persisted between runs — no functional breakage, just quietly lost caching benefit. Consider documenting this coupling explicitly (e.g. a comment nearcache_pathsdescription) so consumers know to keep.npm/in their override if they want npm cache reuse.Also applies to: 84-93
🤖 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 `@build-node.yml` around lines 65 - 70, Document the coupling between cache_paths and the hardcoded NPM_CONFIG_CACHE in build-node.yml so consumers know npm cache reuse depends on including .npm/ in overrides. Update the cache_paths description (and the related cache setup block used later in the file) to explicitly mention that NPM_CONFIG_CACHE is fixed to $CI_PROJECT_DIR/.npm, so custom cache_paths should retain .npm/ if they want npm cache persistence.
🤖 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 `@build-node.yml`:
- Around line 49-52: The default install_command still includes the obsolete
--unsafe-perm flag, which can fail with the template’s npm version. Update the
install_command default to remove --unsafe-perm while keeping the rest of the
npm ci flags intact. Use the install_command field in build-node.yml as the
place to change this default.
---
Nitpick comments:
In `@build-node.yml`:
- Around line 65-70: Document the coupling between cache_paths and the hardcoded
NPM_CONFIG_CACHE in build-node.yml so consumers know npm cache reuse depends on
including .npm/ in overrides. Update the cache_paths description (and the
related cache setup block used later in the file) to explicitly mention that
NPM_CONFIG_CACHE is fixed to $CI_PROJECT_DIR/.npm, so custom cache_paths should
retain .npm/ if they want npm cache persistence.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| install_command: | ||
| type: string | ||
| default: "npm ci --unsafe-perm --prefer-offline" | ||
| description: "Command used to install dependencies inside each package directory." |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the workflow/template file and inspect the relevant section.
git ls-files | rg '(^|/)build-node\.yml$|(^|/)build-node\.yaml$|(^|/)README\.md$|(^|/)docs?/.*node|(^|/)node.*(yml|yaml)$' || true
echo '--- build-node.yml excerpt ---'
nl -ba build-node.yml | sed -n '1,140p'
echo '--- search for node/npm version references and install_command usage ---'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' \
'install_command|node:22-slim|npm ci --unsafe-perm|unsafe-perm|node:[0-9]+-slim|npm [0-9]' .
echo '--- show any local docs/examples around node version usage ---'
for f in $(git ls-files | rg 'README\.md$|\.ya?ml$|\.md$' | head -n 20); do
if rg -n 'node:22-slim|install_command|unsafe-perm|npm ci' "$f" >/dev/null 2>&1; then
echo "### $f"
nl -ba "$f" | sed -n '1,220p'
fi
doneRepository: xima-media/gitlab-templates
Length of output: 261
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- build-node.yml lines 1-140 ---'
sed -n '1,140p' build-node.yml | cat -n
echo '--- README.md lines mentioning node/npm/install_command ---'
rg -n -C 2 'node:22-slim|install_command|unsafe-perm|npm ci|npm [0-9]' README.md build-node.ymlRepository: xima-media/gitlab-templates
Length of output: 6613
🌐 Web query:
npm 10 unsafe-perm invalid npm option npm ci documentation
💡 Result:
The unsafe-perm option is obsolete and no longer supported in npm version 7 and later [1][2][3]. Because you are using npm 10, attempting to use the --unsafe-perm flag—either as a command-line argument or in a configuration file—will trigger an "invalid npm option" error [4][2]. Background and Behavior: - Purpose: In older versions of npm (v6 and earlier), unsafe-perm was used to control whether npm would suppress UID/GID switching when running as root [5][3]. Setting it to true allowed scripts to run with root privileges instead of attempting to switch to the owner of the working directory [3]. - Removal: This configuration was made obsolete in npm v7 (released in 2020) [1][6]. Modern versions of npm automatically handle permissions by running lifecycle scripts with the effective UID and GID of the working directory's owner when npm is executed as root [2][3]. - Documentation: Because it is no longer a valid configuration, it has been removed from official npm documentation [1][2][3]. If you are encountering this error in an automated environment (such as Docker, CI/CD, or build tools), you should remove the --unsafe-perm flag from your install commands or configuration files [2]. If your build process requires specific permissions, you should adjust the file system ownership of the working directory so that the user running the command has the appropriate access, rather than relying on the deprecated unsafe-perm flag [6][2][3].
Citations:
- 1: doc: remove doc on obsolete
unsafe-permflag npm/cli#2196 - 2: [build-utils] Remove --unsafe-perm npm flag vercel/vercel#14302
- 3: https://www.vinayraghu.com/blog/npm-unsafe-perm/
- 4: https://bugs.launchpad.net/bugs/2015640
- 5: https://docs.npmjs.com/cli/v6/using-npm/config/
- 6: Substitute for --unsafe-perm in NPM 7 ? npm/feedback#121
🌐 Web query:
node:22-slim npm version official docker image node 22 includes npm 10
💡 Result:
The official Node.js Docker images, including the node:22-slim variant, bundle the version of npm that is distributed with the specific Node.js release [1][2]. Node.js 22 (the "Jod" LTS release) primarily ships with npm 10 [3][4]. For example, as of recent Node.js 22 releases, npm 10.9.x is typically included [3][5]. While there have been discussions and community requests to upgrade the bundled npm version to npm 11 for Node.js 22, the official Node.js 22 release line has continued to maintain npm 10 [5][6]. To verify the specific version of npm included in a particular image, you can check the bundled version by running the following command in your terminal or Dockerfile: node -e "console.log(require('child_process').execSync('npm --version').toString.trim)" Alternatively, you can check the specific version of the node:22-slim image you are using by running: docker run --rm node:22-slim npm --version Because the npm version is tied to the Node.js distribution, the version of npm included does not change unless the Node.js team updates the bundled npm in a subsequent Node.js 22.x patch release [3][5]. Users requiring a different npm version may update it manually within their container or Dockerfile using the npm install -g npm@ command [5].
Citations:
- 1: https://github.com/nodejs/docker-node
- 2: https://github.com/nodejs/docker-node/blob/main/README.md
- 3: https://nodejs.org/en/download/archive/v22
- 4: https://help.readymage.com/project-development/node.js-version
- 5: Possible NodeJS 22.22.2 regression when installing latest NPM nodejs/node#62425
- 6: Update npm to v11 in Node.js v22 nodejs/node#58423
Drop --unsafe-perm from the default install command
build-node.yml:49-52 — node:22-slim ships npm 10, and --unsafe-perm is obsolete in npm 7+. This default npm ci --unsafe-perm ... can error on the template’s own example image. Remove the flag from install_command.
🤖 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 `@build-node.yml` around lines 49 - 52, The default install_command still
includes the obsolete --unsafe-perm flag, which can fail with the template’s npm
version. Update the install_command default to remove --unsafe-perm while
keeping the rest of the npm ci flags intact. Use the install_command field in
build-node.yml as the place to change this default.
GitLab requires the spec:inputs header to be its own YAML document, ended with ---, otherwise inputs are not recognised and passing any input fails with 'Given inputs not defined in the spec section'.
1 participant
Why
The current
build-node.ymlonly handles a singlepackage.jsonwith a singlenode_modulescache and an emptyartifacts:paths. Projects with multiplepackage.jsonfiles (e.g. TYPO3 monorepos with several site packages / extensions) have to override the wholebuild-nodejob in their.gitlab-ci.yml— install script, artifact paths, image and cache — every time. It also lacks flexibility for multiple asset paths.What
Rework
build-node.ymlusingspec:inputsso everything is configurable from theinclude, no job override needed:imagestagebuildpackage_dirs.package.json; installs + builds each in orderinstall_commandnpm ci --unsafe-perm --prefer-offlinebuild_commandnpm run buildartifact_paths[]artifacts_expire_in1 daycache_paths[node_modules/, .npm/]packages/*/node_modules/)cache_key_files[package-lock.json]cache_key_prefixnodeDefaults preserve the classic single-package behaviour.
Image + Renovate
The image is not pinned in the template — it's a required input, pinned per project so Renovate keeps the digest current. Renovate's
gitlabcimanager doesn't readinclude.inputs, so pin it via an inline annotation + a regex customManager in the shared Renovate preset:Example (DWI Symphonia, 3 packages)
Breaking changes
imageis now a required input (previously the image was set in each project's job override / inherited). Consumers must passimage:when bumping to the version containing this change.build-nodecan drop most of the override.Summary by CodeRabbit
New Features
Chores