Modernize cloud-shell infrastructure and dependencies#370
Conversation
|
@sbouchet Could you please take a look a this change in cloud-shell... The PR removes the most of the Here are the CVEs that were previously fixed via resolutions (now removed because modern dependencies have fixes built-in): tar (^7.5.11) - CVE-2024-28863 (arbitrary file creation/overwrite) Kept in resolutions: cipher-base (^1.0.6) - Still needed Why we could remove them: The updated modern dependencies (webpack 5, TypeScript 5.7, @xterm/* v5, vscode-jsonrpc 8.2, etc.) either:
|
|
Hi! I'm che-ai-assistant — I help with your pull requests. Available commands:
|
|
/che-ai-assistant ok-pr-review Review is complete. Please check the review comments below. |
vrubezhny
force-pushed
the
modernize-cloud-shell-deps
branch
2 times, most recently
from
320ed4c to
c71092f
Compare
What was changed: cloud-shell source code updates: - NotificationType moved from vscode-ws-jsonrpc to vscode-jsonrpc - Logger/MessageConnection imports split between packages - xterm imports updated to @xterm/* scoped packages - NotificationType constructor signature updated (removed second type param) - Removed webpack node.net config (webpack 5 incompatible) cloud-shell dependencies: - webpack 4.x → 5.97.1 (modern bundler, better tree-shaking) - webpack-cli 3.x → 5.1.4 - TypeScript 4.9.5 → 5.7.2 (improved type checking) - css-loader 5.x → 7.1.2 - style-loader 2.x → 4.0.0 - ts-loader 8.x → 9.5.1 - xterm 4.3.0 → @xterm/xterm 5.5.0 (scoped package migration) - xterm-addon-fit 0.3.0 → @xterm/addon-fit 0.10.0 - vscode-ws-jsonrpc 0.1.1 → 3.3.2 (API compatibility) - vscode-uri 2.1.1 → 3.0.8 - vscode-jsonrpc ~8.2.1 (new, split from vscode-ws-jsonrpc) - reconnecting-websocket 4.2.0 → 4.4.0 - Yarn 4.12.0 maintained Security improvements in package.json: - Removed most resolutions (were CVE workarounds for outdated dependencies) - Modern dependency versions have CVE fixes built-in - Kept only critical resolutions: cipher-base ^1.0.6, sha.js ^2.4.12 - No additional CVE fixing needed with updated packages - Removed CVE workarounds for: tar, minimatch, ajv, brace-expansion, postcss, picomatch, ip-address, qs (all fixed in modern deps) Performance improvements: - Bundle size reduced from 4.08 MiB (main) to 1.52 MiB (62.7% reduction) - Webpack 5 tree-shaking and optimization - Docker image: removed node_modules from final image (node_modules not needed at runtime, only built bundle.js) devfile.yaml: - Updated go-toolset: 1.25.5-1770596585 → 1.25.9-1778675823 - Removed debug endpoint (port 5858) and DEBUG_PORT env var (unused) - CRITICAL: Removed explicit cloud-shell endpoint declaration DevWorkspace operator auto-discovers port 3130 when server starts Explicit endpoint definition causes 503 routing conflicts - Kept exec-server endpoint (port 5555, terminal WebSocket) - Restored command groups (build/test/run) for IDE integration - Cleaned up build-cloud-shell command (removed unnecessary GO env vars) - Kept Gi memory format (2.5Gi, 1.5Gi) - Kept Node 22-alpine Dockerfile optimizations: - Updated golang: 1.25.7-alpine → 1.25.9-alpine (latest patch) - Updated Yarn: 4.0.0 → 4.12.0 (match package.json) - Removed node_modules from final image (only copy dist/ and index.html) - Added musl library for multi-arch support (ld-musl-*.so.1) Supports both amd64 (x86_64) and arm64 (aarch64) - Fixed cloud-shell working directory to absolute path - Kept Node 22-alpine Key decisions: - Kept Node 22 (not upgraded to 24) - Kept Gi memory format (not changed to M) - Removed debug endpoint (not used) - CRITICAL FIX: Let operator auto-discover cloud-shell endpoint Explicit declaration in devfile causes Service routing conflicts Auto-discovery creates working routes dynamically - Restored command groups for IDE port forwarding notifications - Skipped GitHub Actions workflows (out of scope for this PR) Testing notes: - Local builds: ✅ All pass (webpack 5 compiles, bundle 1.52 MiB) - Docker build: ✅ Succeeds (38.4MB final image, multi-arch) - DevWorkspace: ✅ Server runs correctly - Cloud-shell terminal: ✅ WORKING (auto-discovered endpoint) Confirmed interactive terminal access via browser Signed-off-by: Victor Rubezhny <vrubezhny@redhat.com> Assisted-By: Claude Sonnet 4.5 <noreply@anthropic.com>
vrubezhny
force-pushed
the
modernize-cloud-shell-deps
branch
from
c71092f to
6f68531
Compare
sbouchet
left a comment
sbouchet
left a comment
There was a problem hiding this comment.
all the actual CVEs listed are fixed with this PR.
please test this PR with che-code editor
CheCode Editor images for the PR are published ✨ Editor amd64: quay.io/che-incubator-pull-requests/che-code:pr-743-amd64 |
|
tested with the che-code PR and it fails to start terminal ( that needs che-machine-exec ) |
There was a problem hiding this comment.
@sbouchet @vrubezhny
I don't think those errors are related to Victor's changes
I believe the problem is related to eclipse-che/che#23648
As workaround it's possible to test current changes in an alpine-based container using the following link:
Warning:
We should thoroughly test current changes for the downstream: eclipse-che/che#23648 is actual only for che-code, it's fixed for the devspaces-code, so - I don't see any problem to test current changes for the devspaces-code directly.
4 participants
What was changed:
cloud-shell source code updates:
cloud-shell dependencies:
Security improvements in package.json:
Performance improvements:
devfile.yaml:
Dockerfile optimizations:
Key decisions:
Testing notes:
Confirmed interactive terminal access via browser
Assisted-By: Claude Sonnet 4.5 noreply@anthropic.com