RHAIENG-5608: Add patch to fix vsce-sign package for CodeServer on PPC64LE builds#2403
Conversation
|
Skipping CI for Draft Pull Request. |
📝 WalkthroughWalkthroughA Changesppc64le vsce-sign workaround
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 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 |
|
@daniellutz — This PR is from a fork. Recommended: Push your branch to the main repo for full CI: Then open a new PR from that branch. No push access? A maintainer will cherry-pick and test your changes. See CONTRIBUTING.md for details. |
❌ 3 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
There was a problem hiding this comment.
@coderabbitai is this the same fix already present on main, or something new?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the fix exists on main branch
gh api repos/red-hat-data-services/notebooks/contents/codeserver/ubi9-python-3.12/get_code_server_rpm.sh?ref=main \
--jq '.content' | base64 -d | grep -n "vsce-sign\|ppc64le\|postinstall" || echo "No matches found on main"Length of output: 394
🏁 Script executed:
#!/bin/bash
# Check what exists in the codeserver directory on main
gh api "repos/red-hat-data-services/notebooks/contents/codeserver?ref=main" --jq '.[].name' 2>/dev/null || echo "codeserver dir not found on main"
# Check if the file exists at a different path on main
gh api "repos/red-hat-data-services/notebooks/git/trees/main?recursive=1" --jq '.tree[].path' 2>/dev/null | grep "get_code_server" || echo "No get_code_server file found on main"Length of output: 563
🏁 Script executed:
#!/bin/bash
# Check the PR details
gh pr view 2403 --repo red-hat-data-services/notebooks --json title,baseRefName,headRefName,files
# Check what files exist in the codeserver/ubi9-python-3.12 directory on main
gh api "repos/red-hat-data-services/notebooks/contents/codeserver/ubi9-python-3.12?ref=main" --jq '.[].name' 2>/dev/null
# Check what files are changed in this PR
gh pr diff 2403 --repo red-hat-data-services/notebooks --name-only 2>/dev/null | head -30Length of output: 1048
🏁 Script executed:
#!/bin/bash
# Check the rhoai-2.25 branch (base branch) version of the file
gh api "repos/red-hat-data-services/notebooks/contents/codeserver/ubi9-python-3.12/get_code_server_rpm.sh?ref=rhoai-2.25" \
--jq '.content' | base64 -d | grep -n "vsce-sign\|postinstall\|ppc64" || echo "No matches on rhoai-2.25"
# Also check if main branch has the fix in any related file
gh api "repos/red-hat-data-services/notebooks/git/trees/main?recursive=1" --jq '.tree[].path' | grep -i "codeserver\|code.server\|vsce" || echo "No codeserver files on main"
# Check if there's a similar fix on main by searching recent PRs
gh pr list --repo red-hat-data-services/notebooks --state merged --search "vsce-sign ppc64" --json number,title,baseRefName 2>/dev/null | head -20Length of output: 8972
@jiridanek This is a new fix — it does not exist anywhere else in the repository in this form:
- The PR targets the
rhoai-2.25release branch, and therhoai-2.25base version ofget_code_server_rpm.shdoes not contain this workaround (no matches forvsce-signorpostinstallon the base branch). - The
mainbranch does not haveget_code_server_rpm.shat all — it uses a completely different offline/prefetch build approach (prefetch-input/patches/code-server-v4.106.3/) rather than building code-server from source at runtime.
So this fix is being introduced fresh into the rhoai-2.25 branch's online build script. There's no equivalent fix on main to compare against.
🧠 Learnings used
Learnt from: daniellutz
Repo: red-hat-data-services/notebooks PR: 2340
File: codeserver/ubi9-python-3.12/Dockerfile.cpu:24-26
Timestamp: 2026-06-08T15:07:27.675Z
Learning: In red-hat-data-services/notebooks under `codeserver/ubi9-python-3.12/`, treat CODESERVER_VERSION and NODE_VERSION bumps (e.g., in `Dockerfile.cpu`, `Dockerfile.konflux.cpu`, and `get_code_server_rpm.sh`) as following an intentional incremental upgrade strategy. During review, do not flag these changes as requiring an immediate jump to the latest available versions; only verify that the bump is part of the planned controlled stepwise upgrade sequence (e.g., moving to the next agreed version rather than skipping ahead).
There was a problem hiding this comment.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Fetch apply-patch.sh from main to compare
gh api "repos/red-hat-data-services/notebooks/contents/codeserver/ubi9-python-3.12/prefetch-input/patches/apply-patch.sh?ref=main" \
--jq '.content' | base64 -dLength of output: 8060
@jiridanek You're right — they solve the exact same problem with the same general concept: prevent @vscode/vsce-sign's postinstall script from running on ppc64le/s390x. The difference is the build context:
main (apply-patch.sh) |
This PR (get_code_server_rpm.sh) |
|
|---|---|---|
| Build type | Hermetic/offline (Cachi2 prefetched tarballs) | Online (downloads from npm registry) |
| Mechanism | Patches the cached .tgz directly (removes postinstall from its package.json), sets hasInstallScript=false + strips integrity in lockfile |
npm packs from registry, wraps postinstall.js to early-exit, repacks, injects via npm overrides |
The main branch fix can't npm pack from the internet (hermetic build), so it modifies the pre-fetched tarball in-place. This PR's rhoai-2.25 online build can reach the registry, so it uses the repack+override approach instead.
The main approach is arguably cleaner (removing postinstall entirely vs. wrapping it) — if desired, the PR could adopt the same strategy by patching the packed tarball's package.json to del(.scripts.postinstall) rather than wrapping the script. But both approaches are functionally equivalent for the goal of skipping the unsupported binary download on ppc64le.
🧠 Learnings used
Learnt from: daniellutz
Repo: red-hat-data-services/notebooks PR: 2340
File: codeserver/ubi9-python-3.12/Dockerfile.cpu:24-26
Timestamp: 2026-06-08T15:07:27.675Z
Learning: In red-hat-data-services/notebooks under `codeserver/ubi9-python-3.12/`, treat CODESERVER_VERSION and NODE_VERSION bumps (e.g., in `Dockerfile.cpu`, `Dockerfile.konflux.cpu`, and `get_code_server_rpm.sh`) as following an intentional incremental upgrade strategy. During review, do not flag these changes as requiring an immediate jump to the latest available versions; only verify that the bump is part of the planned controlled stepwise upgrade sequence (e.g., moving to the next agreed version rather than skipping ahead).
![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
🤖 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 `@codeserver/ubi9-python-3.12/get_code_server_rpm.sh`:
- Around line 71-76: The vsceSignVersion variable extraction using jq can return
null if the package-lock.json structure changes, leading to unclear failures in
the subsequent npm pack and tar commands. Add an explicit validation check
immediately after the jq command extracts the vsceSignVersion to verify it is
not empty or null. If the version is empty or null, exit the script with a clear
error message indicating that the vsce-sign package version lookup failed. This
will provide a faster, more informative failure rather than letting the script
continue and fail later on the npm pack or tar extraction steps.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 1099a4dc-ad64-44e9-97cd-156ca9e6a3c0
📒 Files selected for processing (1)
codeserver/ubi9-python-3.12/get_code_server_rpm.sh
| vsceSignVersion=$(jq -r '.packages["node_modules/@vscode/vsce-sign"].version' lib/vscode/build/package-lock.json) | ||
| patchdir=$(mktemp -d) | ||
| extracted="$patchdir/extracted" | ||
| mkdir -p "$extracted" | ||
| npm pack "@vscode/vsce-sign@${vsceSignVersion}" --pack-destination="$patchdir" | ||
| tar -xzf "$patchdir/vscode-vsce-sign-${vsceSignVersion}.tgz" -C "$extracted" --strip-components=1 |
There was a problem hiding this comment.
Fail fast when @vscode/vsce-sign version lookup returns empty/null.
At Line 71, jq can return null if lockfile structure shifts, which then causes a later npm pack/tar failure with a less clear error. Add an explicit guard right after extraction.
Suggested patch
vsceSignVersion=$(jq -r '.packages["node_modules/@vscode/vsce-sign"].version' lib/vscode/build/package-lock.json)
+ if [[ -z "${vsceSignVersion}" || "${vsceSignVersion}" == "null" ]]; then
+ echo "Failed to resolve `@vscode/vsce-sign` version from lib/vscode/build/package-lock.json" >&2
+ exit 1
+ fi
patchdir=$(mktemp -d)
extracted="$patchdir/extracted"
mkdir -p "$extracted"
npm pack "`@vscode/vsce-sign`@${vsceSignVersion}" --pack-destination="$patchdir"🤖 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 `@codeserver/ubi9-python-3.12/get_code_server_rpm.sh` around lines 71 - 76, The
vsceSignVersion variable extraction using jq can return null if the
package-lock.json structure changes, leading to unclear failures in the
subsequent npm pack and tar commands. Add an explicit validation check
immediately after the jq command extracts the vsceSignVersion to verify it is
not empty or null. If the version is empty or null, exit the script with a clear
error message indicating that the vsce-sign package version lookup failed. This
will provide a faster, more informative failure rather than letting the script
continue and fail later on the npm pack or tar extraction steps.
|
/build-konflux |
jiridanek
left a comment
jiridanek
left a comment
There was a problem hiding this comment.
if this is the version from main, then I'm ok with that
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jiridanek The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
3 participants
With the CodeServer upgrade introduced in #2340 and #2350, a new failures started to happen regarding vsce-sign packages on ppc64le builds.
More information can be found on https://redhat.atlassian.net/browse/RHAIENG-5608
How Has This Been Tested?
Local builds have been done using Podman with PPC64LE emulation
Building
linux/ppc64leimages locallyFirst step is to install qemu on your podman machine VM and then restart it:
After the installation and reboot, you can run the following quick test to ensure the emulation is working:
Building the CodeServer image locally
To build the CodeServer image for PPC64LE, just send the BUILD_ARCH parameter to the make command:
$ make codeserver-ubi9-python-3.12 \ -e BUILD_ARCH=linux/ppc64le \ -e IMAGE_REGISTRY=quay.io/dlutz/workbench-images \ -e RELEASE=2025b \ -e RELEASE_PYTHON_VERSION="3.12" \ -e PUSH_IMAGES="yes" \ -e CONTAINER_BUILD_CACHE_ARGS=""Self checklist (all need to be checked):
make test(gmakeon macOS) before asking for reviewDockerfile.konfluxfiles should be done inodh/notebooksand automatically synced torhds/notebooks. For Konflux-specific changes, modifyDockerfile.konfluxfiles directly inrhds/notebooksas these require special attention in the downstream repository and flow to the upcoming RHOAI release.Merge criteria:
Summary by CodeRabbit