Copilot commented on code in PR #13597:
URL: https://github.com/apache/cloudstack/pull/13597#discussion_r3571010839
##########
tools/docker/supervisord.conf:
##########
@@ -16,7 +17,8 @@ redirect_stderr=true
user=root
[program:cloudstack-ui]
-command=/bin/bash -c "npm run serve"
+environment=NODE_OPTIONS="--openssl-legacy-provider"
+command=/bin/bash -c "NODE_OPTIONS=--openssl-legacy-provider npm run serve"
Review Comment:
With Node 24, `--openssl-legacy-provider` is rejected when provided via
`NODE_OPTIONS` (startup fails with “--openssl-legacy-provider is not allowed in
NODE_OPTIONS”). Since the Dockerfile now installs Node 24, this will prevent
the UI process from starting. Remove this flag (and fix the underlying
crypto/webpack issue via dependency upgrades) or pin Node to a version where
the workaround is actually supported.
##########
tools/docker/Dockerfile:
##########
@@ -61,12 +62,10 @@ RUN find /var/lib/mysql -type f -exec touch {} \; && \
mysql -e "ALTER USER 'root'@'localhost' IDENTIFIED WITH
caching_sha2_password by ''" --connect-expired-password; \
mvn -Pdeveloper -pl developer -Ddeploydb; \
mvn -Pdeveloper -pl developer -Ddeploydb-simulator; \
- MARVIN_FILE=`find /root/tools/marvin/dist/ -name "Marvin*.tar.gz"`; \
- rm -rf /usr/bin/x86_64-linux-gnu-gcc && \
- ln -s /usr/bin/gcc-10 /usr/bin/x86_64-linux-gnu-gcc; \
- pip3 install $MARVIN_FILE
+ MARVIN_FILE=`find /root/tools/marvin/dist/ -name "[mM]arvin*.tar.gz"`; \
+ pip3 install $MARVIN_FILE --break-system-packages
Review Comment:
`MARVIN_FILE` is assigned from `find ... -name "[mM]arvin*.tar.gz"` without
constraining to a single match and without checking for the empty case. If the
directory contains multiple tarballs, `pip3 install $MARVIN_FILE` may install
multiple unintended files; if none exist, the build fails with a less clear
error. Make the selection deterministic and fail fast with a clear message.
##########
.github/workflows/docker-cloudstack-simulator.yml:
##########
@@ -21,6 +21,7 @@ on:
push:
branches:
- main
+ - docker-image-build
Review Comment:
Triggering this workflow on an additional long-lived branch means pushes to
that branch in the upstream repo will also build *and push* images using
registry secrets. If the intent is to test image builds without publishing on
arbitrary branch pushes, prefer a manual trigger (`workflow_dispatch`) or a
PR-only build job without the push step, and keep the push-to-registry workflow
restricted to `main`/tags.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
