Copilot commented on code in PR #12337: URL: https://github.com/apache/cloudstack/pull/12337#discussion_r2645739062
########## tools/docker/Dockerfile.s390x: ########## @@ -0,0 +1,90 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# +# CloudStack-simulator build + +FROM ubuntu:22.04 + +LABEL Vendor="Apache.org" License="ApacheV2" Version="4.23.0.0-SNAPSHOT" Author="Apache CloudStack <[email protected]>" + +ARG DEBIAN_FRONTEND=noninteractive + +RUN apt-get -y update && apt-get install -y \ + genisoimage \ + libffi-dev \ + libssl-dev \ + curl \ + gcc-10 \ + git \ + sudo \ + ipmitool \ + iproute2 \ + maven \ + openjdk-11-jdk \ + python3-dev \ + python-is-python3 \ + python3-setuptools \ + python3-pip \ + python3-mysql.connector \ + python3-bcrypt \ + python3-cryptography \ + python3-cffi \ + supervisor + +RUN apt-get install -qqy mysql-server && \ + apt-get clean all && \ + mkdir -p /var/run/mysqld; \ + chown mysql /var/run/mysqld + +RUN echo '''sql_mode = "STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION"''' >> /etc/mysql/mysql.conf.d/mysqld.cnf + +COPY tools/docker/supervisord.conf /etc/supervisor/conf.d/supervisord.conf +COPY . ./root +WORKDIR /root + +RUN mvn -Pdeveloper -Dsimulator -DskipTests clean install + +RUN find /var/lib/mysql -type f -exec touch {} \; && \ + (/usr/bin/mysqld_safe &) && \ + sleep 5; \ + mysql -e "ALTER USER 'root'@'localhost' IDENTIFIED WITH mysql_native_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/s390x-linux-gnu-gcc && \ + ln -s /usr/bin/gcc-10 /usr/bin/s390x-linux-gnu-gcc; \ + pip3 install $MARVIN_FILE + +RUN apt-get install -y nodejs npm build-essential python3 g++ make && \ + bash + +RUN curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.39.4/install.sh | bash && \ + . /root/.nvm/nvm.sh && \ + nvm install 10 && \ + nvm use 10 && \ + NVM_BIN="$(dirname "$(nvm which node)")" && \ + ln -sf "$NVM_BIN/node" /usr/local/bin/node && \ + ln -sf "$NVM_BIN/npm" /usr/local/bin/npm && \ + cd ui && npm install && npm rebuild node-sass Review Comment: The nvm installation approach differs from the main Dockerfile which uses the nodesource repository. While nvm provides flexibility, this creates maintenance inconsistency between architectures. If Node.js 14 from nodesource is not available for s390x, consider documenting this architectural difference with a comment explaining why nvm is required for s390x. ########## tools/docker/Dockerfile.s390x: ########## @@ -0,0 +1,90 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# +# CloudStack-simulator build + +FROM ubuntu:22.04 + +LABEL Vendor="Apache.org" License="ApacheV2" Version="4.23.0.0-SNAPSHOT" Author="Apache CloudStack <[email protected]>" + +ARG DEBIAN_FRONTEND=noninteractive + +RUN apt-get -y update && apt-get install -y \ + genisoimage \ + libffi-dev \ + libssl-dev \ + curl \ + gcc-10 \ + git \ + sudo \ + ipmitool \ + iproute2 \ + maven \ + openjdk-11-jdk \ + python3-dev \ + python-is-python3 \ + python3-setuptools \ + python3-pip \ + python3-mysql.connector \ + python3-bcrypt \ + python3-cryptography \ + python3-cffi \ + supervisor + +RUN apt-get install -qqy mysql-server && \ + apt-get clean all && \ + mkdir -p /var/run/mysqld; \ + chown mysql /var/run/mysqld + +RUN echo '''sql_mode = "STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION"''' >> /etc/mysql/mysql.conf.d/mysqld.cnf + +COPY tools/docker/supervisord.conf /etc/supervisor/conf.d/supervisord.conf +COPY . ./root +WORKDIR /root + +RUN mvn -Pdeveloper -Dsimulator -DskipTests clean install + +RUN find /var/lib/mysql -type f -exec touch {} \; && \ + (/usr/bin/mysqld_safe &) && \ + sleep 5; \ + mysql -e "ALTER USER 'root'@'localhost' IDENTIFIED WITH mysql_native_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/s390x-linux-gnu-gcc && \ + ln -s /usr/bin/gcc-10 /usr/bin/s390x-linux-gnu-gcc; \ + pip3 install $MARVIN_FILE + +RUN apt-get install -y nodejs npm build-essential python3 g++ make && \ + bash + +RUN curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.39.4/install.sh | bash && \ + . /root/.nvm/nvm.sh && \ + nvm install 10 && \ + nvm use 10 && \ Review Comment: The s390x Dockerfile installs Node.js version 10 using nvm, while the main Dockerfile (for x86_64) uses Node.js version 14 from nodesource. Node.js 10 reached end-of-life in April 2021 and is no longer supported or receiving security updates. Consider using Node.js 14 or later to match the main Dockerfile and ensure security and compatibility. ```suggestion nvm install 14 && \ nvm use 14 && \ ``` ########## tools/docker/Dockerfile.s390x: ########## @@ -0,0 +1,90 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# +# CloudStack-simulator build + +FROM ubuntu:22.04 + +LABEL Vendor="Apache.org" License="ApacheV2" Version="4.23.0.0-SNAPSHOT" Author="Apache CloudStack <[email protected]>" + +ARG DEBIAN_FRONTEND=noninteractive + +RUN apt-get -y update && apt-get install -y \ + genisoimage \ + libffi-dev \ + libssl-dev \ + curl \ + gcc-10 \ + git \ + sudo \ + ipmitool \ + iproute2 \ + maven \ + openjdk-11-jdk \ + python3-dev \ + python-is-python3 \ + python3-setuptools \ + python3-pip \ + python3-mysql.connector \ + python3-bcrypt \ + python3-cryptography \ + python3-cffi \ + supervisor Review Comment: The s390x Dockerfile includes additional Python cryptography packages (python3-bcrypt, python3-cryptography, python3-cffi) that are not present in the main Dockerfile. If these packages are required for s390x-specific functionality or to address build issues on this architecture, consider adding a comment explaining why they are needed. Alternatively, if they should be in both Dockerfiles, the main Dockerfile may need updating as well. ########## tools/docker/Dockerfile.s390x: ########## @@ -0,0 +1,90 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# +# CloudStack-simulator build + +FROM ubuntu:22.04 + +LABEL Vendor="Apache.org" License="ApacheV2" Version="4.23.0.0-SNAPSHOT" Author="Apache CloudStack <[email protected]>" + +ARG DEBIAN_FRONTEND=noninteractive + +RUN apt-get -y update && apt-get install -y \ + genisoimage \ + libffi-dev \ + libssl-dev \ + curl \ + gcc-10 \ + git \ + sudo \ + ipmitool \ + iproute2 \ + maven \ + openjdk-11-jdk \ + python3-dev \ + python-is-python3 \ + python3-setuptools \ + python3-pip \ + python3-mysql.connector \ + python3-bcrypt \ + python3-cryptography \ + python3-cffi \ + supervisor + +RUN apt-get install -qqy mysql-server && \ + apt-get clean all && \ + mkdir -p /var/run/mysqld; \ + chown mysql /var/run/mysqld + +RUN echo '''sql_mode = "STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION"''' >> /etc/mysql/mysql.conf.d/mysqld.cnf + +COPY tools/docker/supervisord.conf /etc/supervisor/conf.d/supervisord.conf +COPY . ./root +WORKDIR /root + +RUN mvn -Pdeveloper -Dsimulator -DskipTests clean install + +RUN find /var/lib/mysql -type f -exec touch {} \; && \ + (/usr/bin/mysqld_safe &) && \ + sleep 5; \ + mysql -e "ALTER USER 'root'@'localhost' IDENTIFIED WITH mysql_native_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/s390x-linux-gnu-gcc && \ + ln -s /usr/bin/gcc-10 /usr/bin/s390x-linux-gnu-gcc; \ + pip3 install $MARVIN_FILE + +RUN apt-get install -y nodejs npm build-essential python3 g++ make && \ + bash + +RUN curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.39.4/install.sh | bash && \ Review Comment: The `curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.39.4/install.sh | bash` pattern downloads and executes remote shell code during the image build without any integrity or authenticity verification. If the `nvm-sh` repository or the delivery path is compromised, an attacker can serve a malicious script that will execute with full privileges inside the build, allowing them to backdoor the resulting simulator image. Prefer installing Node.js and related tooling from distribution packages or a vendored, checksummed artifact, or at minimum verify a cryptographic checksum or signature of the script before execution. ########## tools/docker/Dockerfile.s390x: ########## @@ -0,0 +1,90 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# +# CloudStack-simulator build + +FROM ubuntu:22.04 + +LABEL Vendor="Apache.org" License="ApacheV2" Version="4.23.0.0-SNAPSHOT" Author="Apache CloudStack <[email protected]>" + +ARG DEBIAN_FRONTEND=noninteractive + +RUN apt-get -y update && apt-get install -y \ + genisoimage \ + libffi-dev \ + libssl-dev \ + curl \ + gcc-10 \ + git \ + sudo \ + ipmitool \ + iproute2 \ + maven \ + openjdk-11-jdk \ + python3-dev \ + python-is-python3 \ + python3-setuptools \ + python3-pip \ + python3-mysql.connector \ + python3-bcrypt \ + python3-cryptography \ + python3-cffi \ + supervisor + +RUN apt-get install -qqy mysql-server && \ + apt-get clean all && \ + mkdir -p /var/run/mysqld; \ + chown mysql /var/run/mysqld + +RUN echo '''sql_mode = "STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION"''' >> /etc/mysql/mysql.conf.d/mysqld.cnf + +COPY tools/docker/supervisord.conf /etc/supervisor/conf.d/supervisord.conf +COPY . ./root +WORKDIR /root + +RUN mvn -Pdeveloper -Dsimulator -DskipTests clean install + +RUN find /var/lib/mysql -type f -exec touch {} \; && \ + (/usr/bin/mysqld_safe &) && \ + sleep 5; \ + mysql -e "ALTER USER 'root'@'localhost' IDENTIFIED WITH mysql_native_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/s390x-linux-gnu-gcc && \ + ln -s /usr/bin/gcc-10 /usr/bin/s390x-linux-gnu-gcc; \ + pip3 install $MARVIN_FILE + +RUN apt-get install -y nodejs npm build-essential python3 g++ make && \ + bash + +RUN curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.39.4/install.sh | bash && \ + . /root/.nvm/nvm.sh && \ + nvm install 10 && \ + nvm use 10 && \ Review Comment: The RUN command on line 72 installs build tools (nodejs, npm, build-essential, python3, g++, make) followed by a standalone bash command on line 73. The bash command appears incomplete and will cause the build to fail or hang. If these build tools are prerequisites for the subsequent nvm installation, they should be installed in a separate RUN layer before line 75, and line 73 should be removed. ```suggestion RUN apt-get install -y nodejs npm build-essential python3 g++ make RUN curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.39.4/install.sh | bash && \ . /root/.nvm/nvm.sh && \ nvm install 10 && \ nvm use 10 && \ nvm use 10 && \ ``` ########## tools/docker/Dockerfile.s390x: ########## @@ -0,0 +1,90 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# +# CloudStack-simulator build + +FROM ubuntu:22.04 + +LABEL Vendor="Apache.org" License="ApacheV2" Version="4.23.0.0-SNAPSHOT" Author="Apache CloudStack <[email protected]>" + +ARG DEBIAN_FRONTEND=noninteractive + +RUN apt-get -y update && apt-get install -y \ + genisoimage \ + libffi-dev \ + libssl-dev \ + curl \ + gcc-10 \ + git \ + sudo \ + ipmitool \ + iproute2 \ + maven \ + openjdk-11-jdk \ + python3-dev \ + python-is-python3 \ + python3-setuptools \ + python3-pip \ + python3-mysql.connector \ + python3-bcrypt \ + python3-cryptography \ + python3-cffi \ + supervisor + +RUN apt-get install -qqy mysql-server && \ + apt-get clean all && \ + mkdir -p /var/run/mysqld; \ + chown mysql /var/run/mysqld + +RUN echo '''sql_mode = "STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION"''' >> /etc/mysql/mysql.conf.d/mysqld.cnf + +COPY tools/docker/supervisord.conf /etc/supervisor/conf.d/supervisord.conf +COPY . ./root +WORKDIR /root + +RUN mvn -Pdeveloper -Dsimulator -DskipTests clean install + +RUN find /var/lib/mysql -type f -exec touch {} \; && \ + (/usr/bin/mysqld_safe &) && \ + sleep 5; \ + mysql -e "ALTER USER 'root'@'localhost' IDENTIFIED WITH mysql_native_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/s390x-linux-gnu-gcc && \ + ln -s /usr/bin/gcc-10 /usr/bin/s390x-linux-gnu-gcc; \ + pip3 install $MARVIN_FILE + +RUN apt-get install -y nodejs npm build-essential python3 g++ make && \ + bash Review Comment: This block installs Node.js dependencies before the nvm installation in lines 75-82. Consider combining these installations or reordering them to improve Docker layer caching and reduce image size. The nodejs and npm packages installed here may conflict with or be redundant with the nvm-installed versions. ########## tools/docker/Dockerfile.s390x: ########## @@ -0,0 +1,90 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# +# CloudStack-simulator build + +FROM ubuntu:22.04 + +LABEL Vendor="Apache.org" License="ApacheV2" Version="4.23.0.0-SNAPSHOT" Author="Apache CloudStack <[email protected]>" + +ARG DEBIAN_FRONTEND=noninteractive + +RUN apt-get -y update && apt-get install -y \ + genisoimage \ + libffi-dev \ + libssl-dev \ + curl \ + gcc-10 \ + git \ + sudo \ + ipmitool \ + iproute2 \ + maven \ + openjdk-11-jdk \ + python3-dev \ + python-is-python3 \ + python3-setuptools \ + python3-pip \ + python3-mysql.connector \ + python3-bcrypt \ + python3-cryptography \ + python3-cffi \ + supervisor + +RUN apt-get install -qqy mysql-server && \ + apt-get clean all && \ + mkdir -p /var/run/mysqld; \ + chown mysql /var/run/mysqld + +RUN echo '''sql_mode = "STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION"''' >> /etc/mysql/mysql.conf.d/mysqld.cnf + +COPY tools/docker/supervisord.conf /etc/supervisor/conf.d/supervisord.conf +COPY . ./root +WORKDIR /root + +RUN mvn -Pdeveloper -Dsimulator -DskipTests clean install + +RUN find /var/lib/mysql -type f -exec touch {} \; && \ + (/usr/bin/mysqld_safe &) && \ + sleep 5; \ + mysql -e "ALTER USER 'root'@'localhost' IDENTIFIED WITH mysql_native_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/s390x-linux-gnu-gcc && \ + ln -s /usr/bin/gcc-10 /usr/bin/s390x-linux-gnu-gcc; \ + pip3 install $MARVIN_FILE + +RUN apt-get install -y nodejs npm build-essential python3 g++ make && \ + bash + +RUN curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.39.4/install.sh | bash && \ + . /root/.nvm/nvm.sh && \ + nvm install 10 && \ + nvm use 10 && \ + NVM_BIN="$(dirname "$(nvm which node)")" && \ + ln -sf "$NVM_BIN/node" /usr/local/bin/node && \ + ln -sf "$NVM_BIN/npm" /usr/local/bin/npm && \ + cd ui && npm install && npm rebuild node-sass Review Comment: The order of npm operations differs from the main Dockerfile. This Dockerfile runs "npm install && npm rebuild node-sass" while the main Dockerfile runs "npm rebuild node-sass && npm install". The main Dockerfile's order is more correct as node-sass should be rebuilt after dependencies are already installed. Consider changing the order to match the main Dockerfile for consistency. ```suggestion cd ui && npm rebuild node-sass && npm install ``` -- 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]
