Merged
Conversation
Add Dockerfile.arm for linux/aarch64 builds (macOS Apple Silicon) with aarch64 Miniforge, conditional THIRDPARTY copy, and trimmed thirdparty PATH. Add parallel ARM64 build job to CI workflow using ubuntu-22.04-arm runner.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ARM64 support: a new Changes
Sequence Diagram(s)sequenceDiagram
participant Trigger as Trigger (manual or workflow_run)
participant Resolve as resolve-tag
participant BuildA as build-amd64
participant BuildB as build-arm64
participant GHCR as GHCR Registry
participant Manifest as create-manifest
Trigger->>Resolve: provide event (tag or workflow_run)
Resolve-->>BuildA: output version & sha
Resolve-->>BuildB: output version & sha
BuildA->>GHCR: login, build (Dockerfile), push <image>:<version>-amd64
BuildB->>GHCR: login, build (Dockerfile.arm), push <image>:<version>-arm64
GHCR-->>Manifest: images available
Manifest->>GHCR: create/push <image>:<version> (amd64+arm64)
Manifest->>GHCR: create/push <image>:latest (amd64+arm64)
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
Dockerfile.arm (5)
59-65: Add--no-install-recommendsto apt-get install commands.The GitHub CLI installation script is missing
--no-install-recommendsfor theapt-get installcommands, which increases image size with unnecessary packages.♻️ Proposed fix
RUN (type -p wget >/dev/null || (apt-get update && apt-get install wget -y)) \ +RUN (type -p wget >/dev/null || (apt-get update && apt-get install --no-install-recommends wget -y)) \ && mkdir -p -m 755 /etc/apt/keyrings \ && wget -qO- https://cli.github.com/packages/githubcli-archive-keyring.gpg | tee /etc/apt/keyrings/githubcli-archive-keyring.gpg > /dev/null \ && chmod go+r /etc/apt/keyrings/githubcli-archive-keyring.gpg \ && echo "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/githubcli-archive-keyring.gpg] https://cli.github.com/packages stable main" | tee /etc/apt/sources.list.d/github-cli.list > /dev/null \ && apt-get update \ - && apt-get install gh -y + && apt-get install --no-install-recommends gh -y🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile.arm` around lines 59 - 65, The RUN block that installs wget and gh should add the --no-install-recommends flag to apt-get install invocations to avoid pulling unnecessary packages; update the two apt-get install calls in the RUN sequence (the one that installs wget and the final apt-get install gh -y) to include --no-install-recommends (and keep existing -y), leaving apt-get update and the wget/gh steps otherwise unchanged.
253-258: Consider adding error handling for GitHub release download.If
GH_TOKENis set but the release or asset doesn't exist,gh release downloadwill fail and stop the build due toset -ein the entrypoint (though this RUN is separate). Consider adding explicit error handling or using|| trueif the download is optional.♻️ Optional: Add error handling
RUN if [ -n "$GH_TOKEN" ]; then \ echo "GH_TOKEN is set, proceeding to download the release asset..."; \ - gh release download -R ${GITHUB_USER}/${GITHUB_REPO} -p "OpenMS-App.zip" -D /app; \ + gh release download -R ${GITHUB_USER}/${GITHUB_REPO} -p "OpenMS-App.zip" -D /app || \ + echo "WARNING: Failed to download release asset, continuing anyway"; \ else \ echo "GH_TOKEN is not set, skipping the release asset download."; \ fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile.arm` around lines 253 - 258, The RUN step that uses GH_TOKEN to call "gh release download -R ${GITHUB_USER}/${GITHUB_REPO} -p \"OpenMS-App.zip\" -D /app" can fail the build if the release or asset is missing; update the Dockerfile RUN block to handle download failures gracefully by checking the release/asset existence (e.g., use "gh release view" first) or by making the download non-fatal (e.g., append "|| true" or catch errors and log a warning) so that a missing OpenMS-App.zip does not abort the image build when GH_TOKEN is present.
86-86: Uselesscd /OpenMSat end of RUN command.Each RUN command starts in a fresh shell, so
cd /OpenMSat the end has no effect on subsequent commands. The WORKDIR on line 89 already handles this correctly.♻️ Proposed fix
-RUN git clone --recursive --depth=1 -b ${OPENMS_BRANCH} --single-branch ${OPENMS_REPO} && cd /OpenMS +RUN git clone --recursive --depth=1 -b ${OPENMS_BRANCH} --single-branch ${OPENMS_REPO}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile.arm` at line 86, The RUN that executes "git clone --recursive --depth=1 -b ${OPENMS_BRANCH} --single-branch ${OPENMS_REPO} && cd /OpenMS" contains a useless "cd /OpenMS" because each RUN uses a fresh shell and the Dockerfile later sets WORKDIR; remove the trailing "&& cd /OpenMS" from the RUN command (the git clone invocation is fine) so the clone happens without the ineffective cd.
184-242: Complex entrypoint script is hard to maintain.The inline entrypoint script with embedded nginx config is difficult to read, test, and maintain. Consider extracting it to a separate
entrypoint.shfile and COPYing it into the image. This would also make debugging easier and allow proper shell syntax highlighting.Additionally, the Streamlit instances are started without
--server.headless true, which may cause issues in headless environments.♻️ Suggested improvement for Streamlit commands
- streamlit run app.py --server.port $PORT --server.address 0.0.0.0 &\n\ + streamlit run app.py --server.port $PORT --server.address 0.0.0.0 --server.headless true &\n\ ... - exec streamlit run app.py --server.address 0.0.0.0\n\ + exec streamlit run app.py --server.address 0.0.0.0 --server.headless true\n\🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile.arm` around lines 184 - 242, Extract the large inline script into a standalone entrypoint script file (e.g., entrypoint.sh) and COPY it into the image instead of echoing it in Dockerfile; ensure the file contains the same logic around starting cron, Redis, RQ workers (RQ_WORKER_COUNT, openms-workflows, REDIS_URL), generating nginx upstream using STREAMLIT_SERVER_COUNT/BASE_PORT, starting Streamlit instances and nginx, and make the script executable (chmod +x) and used via ENTRYPOINT or CMD. While extracting, update all Streamlit invocations (both in single-instance and multi-instance loops) to include --server.headless true (keep --server.port and --server.address flags as present) so Streamlit runs correctly in headless/container environments. Ensure the nginx config generation and exec /usr/sbin/nginx -g "daemon off;" and exec streamlit run app.py lines remain identical in behavior after the move.
90-97: Consider adding explicit logging when ARM64 thirdparty binaries are present or missing.The conditional check for
THIRDPARTY/Linux/aarch64is appropriate, and the directory does exist with binaries (Comet, Percolator, Sage). However, to improve build visibility and aid debugging, consider adding explicit output when the directory is present or missing:Suggested improvement for build visibility
RUN mkdir /thirdparty && \ git submodule update --init THIRDPARTY && \ cp -r THIRDPARTY/All/* /thirdparty && \ if [ -d "THIRDPARTY/Linux/aarch64" ]; then \ cp -r THIRDPARTY/Linux/aarch64/* /thirdparty; \ + echo "INFO: Copied ARM64-specific binaries from THIRDPARTY/Linux/aarch64"; \ + else \ + echo "WARNING: THIRDPARTY/Linux/aarch64 not found - using generic binaries only"; \ fi && \ chmod -R +x /thirdparty🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile.arm` around lines 90 - 97, The build step that copies ARM64 third-party binaries (the RUN block that creates /thirdparty, updates the THIRDPARTY submodule and conditionally copies from THIRDPARTY/Linux/aarch64) lacks visible output about whether the aarch64 directory was found; update that RUN sequence to emit explicit messages (e.g., using echo) for both the present and missing cases around the if [ -d "THIRDPARTY/Linux/aarch64" ] check so the build log shows “Found ARM64 thirdparty: copying...” or “No ARM64 thirdparty directory; skipping” before or after the cp commands; keep the existing chmod and ENV PATH update unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-docker-images.yml:
- Around line 20-27: Replace the deprecated actions/checkout@v3 usage with
actions/checkout@v4 in the workflow job that builds the ARM64 image (job name
build-full-app-arm64); find the step that currently reads "uses:
actions/checkout@v3" and update it to "uses: actions/checkout@v4" so the runner
uses Node 20-compatible checkout action and avoids Node 16 deprecation failures.
In `@Dockerfile.arm`:
- Line 1: Update the top-line comment in the Dockerfile.arm header to correct
the typo "thidparty" to "thirdparty" so the comment reads "...pyOpenMS and
thirdparty tools."; locate the string in the file (the first-line comment) and
replace the misspelled word.
- Around line 78-79: The SHELL directive using "~/.bashrc" won't expand in
Docker's JSON-array form; replace the tilde with an absolute path or an
environment variable (e.g., use "/root/.bashrc" or "$HOME/.bashrc") in the SHELL
invocation so the rcfile is actually sourced; update the two SHELL lines (the
first SHELL ["/bin/bash", "--rcfile", "~/.bashrc"] and the following SHELL
["mamba", "run", "-n", "streamlit-env", "/bin/bash", "-c"]) to reference the
expanded path or $HOME consistently (or remove the first SHELL and adjust the
mamba-run line to source the absolute rcfile) so bash will source the intended
file.
- Around line 44-56: The Dockerfile currently sets USER root and runs multiple
separated apt-get update/install RUNs; create a dedicated non-root user and
group (e.g., appuser) during the image build, chown any necessary app files and
switch to that user before the final CMD/ENTRYPOINT (replace or move the
existing USER root), and consolidate package installation by combining apt-get
-y update && apt-get install -y --no-install-recommends ... into single RUN
statements (replace the separate RUN apt-get -y update and subsequent RUN
apt-get install lines) to prevent stale package lists and reduce image layers;
update any file ownership or startup steps to use the new user.
- Around line 36-38: Fix the typos in the Dockerfile comments by changing
"Gihub" to "GitHub" where the ARG is declared (e.g., the comment above ARG
GITHUB_USER) and in the adjacent repository-name comment; update the two comment
strings so they read "GitHub" instead of "Gihub" while leaving ARG GITHUB_USER
and the repository ARG unchanged.
---
Nitpick comments:
In `@Dockerfile.arm`:
- Around line 59-65: The RUN block that installs wget and gh should add the
--no-install-recommends flag to apt-get install invocations to avoid pulling
unnecessary packages; update the two apt-get install calls in the RUN sequence
(the one that installs wget and the final apt-get install gh -y) to include
--no-install-recommends (and keep existing -y), leaving apt-get update and the
wget/gh steps otherwise unchanged.
- Around line 253-258: The RUN step that uses GH_TOKEN to call "gh release
download -R ${GITHUB_USER}/${GITHUB_REPO} -p \"OpenMS-App.zip\" -D /app" can
fail the build if the release or asset is missing; update the Dockerfile RUN
block to handle download failures gracefully by checking the release/asset
existence (e.g., use "gh release view" first) or by making the download
non-fatal (e.g., append "|| true" or catch errors and log a warning) so that a
missing OpenMS-App.zip does not abort the image build when GH_TOKEN is present.
- Line 86: The RUN that executes "git clone --recursive --depth=1 -b
${OPENMS_BRANCH} --single-branch ${OPENMS_REPO} && cd /OpenMS" contains a
useless "cd /OpenMS" because each RUN uses a fresh shell and the Dockerfile
later sets WORKDIR; remove the trailing "&& cd /OpenMS" from the RUN command
(the git clone invocation is fine) so the clone happens without the ineffective
cd.
- Around line 184-242: Extract the large inline script into a standalone
entrypoint script file (e.g., entrypoint.sh) and COPY it into the image instead
of echoing it in Dockerfile; ensure the file contains the same logic around
starting cron, Redis, RQ workers (RQ_WORKER_COUNT, openms-workflows, REDIS_URL),
generating nginx upstream using STREAMLIT_SERVER_COUNT/BASE_PORT, starting
Streamlit instances and nginx, and make the script executable (chmod +x) and
used via ENTRYPOINT or CMD. While extracting, update all Streamlit invocations
(both in single-instance and multi-instance loops) to include --server.headless
true (keep --server.port and --server.address flags as present) so Streamlit
runs correctly in headless/container environments. Ensure the nginx config
generation and exec /usr/sbin/nginx -g "daemon off;" and exec streamlit run
app.py lines remain identical in behavior after the move.
- Around line 90-97: The build step that copies ARM64 third-party binaries (the
RUN block that creates /thirdparty, updates the THIRDPARTY submodule and
conditionally copies from THIRDPARTY/Linux/aarch64) lacks visible output about
whether the aarch64 directory was found; update that RUN sequence to emit
explicit messages (e.g., using echo) for both the present and missing cases
around the if [ -d "THIRDPARTY/Linux/aarch64" ] check so the build log shows
“Found ARM64 thirdparty: copying...” or “No ARM64 thirdparty directory;
skipping” before or after the cp commands; keep the existing chmod and ENV PATH
update unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 251ca5b0-9e83-4973-b9a0-a442be04b513
📒 Files selected for processing (2)
.github/workflows/build-docker-images.ymlDockerfile.arm
Comment on lines
+20
to
+27
| build-full-app-arm64: | ||
|
|
||
| runs-on: ubuntu-22.04-arm | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v3 | ||
| - name: Build the ARM64 Docker image | ||
| run: docker build --file Dockerfile.arm --tag streamlitapp:latest-arm64 --output type=tar,dest=/dev/null . No newline at end of file |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
GitHub Actions checkout v3 ARM64 runner compatibility
💡 Result:
actions/checkout@v3 is architecture-agnostic (it’s a JavaScript action), so it can run on ARM64 runners as long as the runner provides a compatible Node runtime. checkout@v3 declares runs.using: node16. [1]
However, GitHub Actions moved JavaScript actions to run on Node 20 by default (enforced starting June 3, 2024). [2] And Node 16 was removed from GitHub-hosted runners on Nov 12, 2024, meaning workflows relying on Node16-only actions are expected to break on runners that no longer include Node16. [3]
So, for ARM64 runners (including GitHub-hosted ARM64 runners, which are GA), [4] the practical guidance is:
- Use
actions/checkout@v4(it declaresruns.using: node20). [5] - If you’re on self-hosted ARM64, ensure the OS/glibc toolchain can run the runner’s embedded Node 20 (otherwise you may see runtime/library errors). [6]
Sources
[1] https://raw.githubusercontent.com/actions/checkout/v3/action.yml
[2] https://github.blog/changelog/2024-03-06-github-actions-all-actions-will-run-on-node20-instead-of-node16-by-default/
[3] https://github.blog/changelog/2024-09-25-end-of-life-for-actions-node16/
[4] https://github.blog/changelog/2024-09-03-github-actions-arm64-linux-and-windows-runners-are-now-generally-available
[5] https://raw.githubusercontent.com/actions/checkout/v4/action.yml
[6] actions/checkout#1809
🏁 Script executed:
cat -n .github/workflows/build-docker-images.yml | head -40Repository: OpenMS/FLASHApp
Length of output: 896
Update actions/checkout to v4 due to Node 16 deprecation.
actions/checkout@v3 uses Node 16, which was removed from GitHub-hosted runners on November 12, 2024. Both jobs must be updated to v4, which uses Node 20. This affects all runners, including the ARM64 runner, and will cause workflow failures if not addressed.
🐛 Proposed fix
build-full-app:
runs-on: ubuntu-latest
steps:
- - uses: actions/checkout@v3
+ - uses: actions/checkout@v4
- name: Build the full Docker image
run: docker build --file Dockerfile --tag streamlitapp:latest --output type=tar,dest=/dev/null .
build-full-app-arm64:
runs-on: ubuntu-22.04-arm
steps:
- - uses: actions/checkout@v3
+ - uses: actions/checkout@v4
- name: Build the ARM64 Docker image
- run: docker build --file Dockerfile.arm --tag streamlitapp:latest-arm64 --output type=tar,dest=/dev/null .
+ run: docker build --file Dockerfile.arm --tag streamlitapp:latest-arm64 --output type=tar,dest=/dev/null .🧰 Tools
🪛 actionlint (1.7.11)
[error] 25-25: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build-docker-images.yml around lines 20 - 27, Replace the
deprecated actions/checkout@v3 usage with actions/checkout@v4 in the workflow
job that builds the ARM64 image (job name build-full-app-arm64); find the step
that currently reads "uses: actions/checkout@v3" and update it to "uses:
actions/checkout@v4" so the runner uses Node 20-compatible checkout action and
avoids Node 16 deprecation failures.
| @@ -0,0 +1,263 @@ | |||
| # This Dockerfile builds OpenMS, the TOPP tools, pyOpenMS and thidparty tools. | |||
Contributor
There was a problem hiding this comment.
Fix typos in documentation comments.
Minor typos: "thidparty" should be "thirdparty".
📝 Proposed fix
-# This Dockerfile builds OpenMS, the TOPP tools, pyOpenMS and thidparty tools.
+# This Dockerfile builds OpenMS, the TOPP tools, pyOpenMS and thirdparty tools.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # This Dockerfile builds OpenMS, the TOPP tools, pyOpenMS and thidparty tools. | |
| # This Dockerfile builds OpenMS, the TOPP tools, pyOpenMS and thirdparty tools. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Dockerfile.arm` at line 1, Update the top-line comment in the Dockerfile.arm
header to correct the typo "thidparty" to "thirdparty" so the comment reads
"...pyOpenMS and thirdparty tools."; locate the string in the file (the
first-line comment) and replace the misspelled word.
Comment on lines
+36
to
+38
| # Streamlit app Gihub user name (to download artifact from). | ||
| ARG GITHUB_USER=OpenMS | ||
| # Streamlit app Gihub repository name (to download artifact from). |
Contributor
There was a problem hiding this comment.
Fix typos: "Gihub" → "GitHub".
📝 Proposed fix
-# Streamlit app Gihub user name (to download artifact from).
+# Streamlit app GitHub user name (to download artifact from).
ARG GITHUB_USER=OpenMS
-# Streamlit app Gihub repository name (to download artifact from).
+# Streamlit app GitHub repository name (to download artifact from).
ARG GITHUB_REPO=FLASHApp📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Streamlit app Gihub user name (to download artifact from). | |
| ARG GITHUB_USER=OpenMS | |
| # Streamlit app Gihub repository name (to download artifact from). | |
| # Streamlit app GitHub user name (to download artifact from). | |
| ARG GITHUB_USER=OpenMS | |
| # Streamlit app GitHub repository name (to download artifact from). | |
| ARG GITHUB_REPO=FLASHApp |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Dockerfile.arm` around lines 36 - 38, Fix the typos in the Dockerfile
comments by changing "Gihub" to "GitHub" where the ARG is declared (e.g., the
comment above ARG GITHUB_USER) and in the adjacent repository-name comment;
update the two comment strings so they read "GitHub" instead of "Gihub" while
leaving ARG GITHUB_USER and the repository ARG unchanged.
Comment on lines
+44
to
+56
| USER root | ||
|
|
||
| # Install required Ubuntu packages. | ||
| RUN apt-get -y update | ||
| RUN apt-get install -y --no-install-recommends --no-install-suggests g++ autoconf automake patch libtool make git gpg wget ca-certificates curl jq libgtk2.0-dev openjdk-8-jdk cron | ||
| RUN update-ca-certificates | ||
| RUN apt-get install -y --no-install-recommends --no-install-suggests libsvm-dev libeigen3-dev coinor-libcbc-dev libglpk-dev libzip-dev zlib1g-dev libxerces-c-dev libbz2-dev libomp-dev libhdf5-dev | ||
| RUN apt-get install -y --no-install-recommends --no-install-suggests libboost-date-time1.74-dev \ | ||
| libboost-iostreams1.74-dev \ | ||
| libboost-regex1.74-dev \ | ||
| libboost-math1.74-dev \ | ||
| libboost-random1.74-dev | ||
| RUN apt-get install -y --no-install-recommends --no-install-suggests qt6-base-dev libqt6svg6-dev libqt6opengl6-dev libqt6openglwidgets6 libgl-dev |
Contributor
There was a problem hiding this comment.
Security concern: Container runs as root.
The container runs all processes as root, which is a security risk in production deployments. Consider creating a non-root user for running the application. Additionally, as flagged by static analysis, apt-get update on line 47 should be combined with apt-get install in the same RUN statement to avoid stale package lists.
🔒 Proposed fix for apt-get consolidation
-RUN apt-get -y update
-RUN apt-get install -y --no-install-recommends --no-install-suggests g++ autoconf automake patch libtool make git gpg wget ca-certificates curl jq libgtk2.0-dev openjdk-8-jdk cron
+RUN apt-get -y update && \
+ apt-get install -y --no-install-recommends --no-install-suggests g++ autoconf automake patch libtool make git gpg wget ca-certificates curl jq libgtk2.0-dev openjdk-8-jdk cronNote: Running as root may be acceptable for this build if it's only used in controlled CI/test environments, but should be addressed for any production deployment.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| USER root | |
| # Install required Ubuntu packages. | |
| RUN apt-get -y update | |
| RUN apt-get install -y --no-install-recommends --no-install-suggests g++ autoconf automake patch libtool make git gpg wget ca-certificates curl jq libgtk2.0-dev openjdk-8-jdk cron | |
| RUN update-ca-certificates | |
| RUN apt-get install -y --no-install-recommends --no-install-suggests libsvm-dev libeigen3-dev coinor-libcbc-dev libglpk-dev libzip-dev zlib1g-dev libxerces-c-dev libbz2-dev libomp-dev libhdf5-dev | |
| RUN apt-get install -y --no-install-recommends --no-install-suggests libboost-date-time1.74-dev \ | |
| libboost-iostreams1.74-dev \ | |
| libboost-regex1.74-dev \ | |
| libboost-math1.74-dev \ | |
| libboost-random1.74-dev | |
| RUN apt-get install -y --no-install-recommends --no-install-suggests qt6-base-dev libqt6svg6-dev libqt6opengl6-dev libqt6openglwidgets6 libgl-dev | |
| USER root | |
| # Install required Ubuntu packages. | |
| RUN apt-get -y update && \ | |
| apt-get install -y --no-install-recommends --no-install-suggests g++ autoconf automake patch libtool make git gpg wget ca-certificates curl jq libgtk2.0-dev openjdk-8-jdk cron | |
| RUN update-ca-certificates | |
| RUN apt-get install -y --no-install-recommends --no-install-suggests libsvm-dev libeigen3-dev coinor-libcbc-dev libglpk-dev libzip-dev zlib1g-dev libxerces-c-dev libbz2-dev libomp-dev libhdf5-dev | |
| RUN apt-get install -y --no-install-recommends --no-install-suggests libboost-date-time1.74-dev \ | |
| libboost-iostreams1.74-dev \ | |
| libboost-regex1.74-dev \ | |
| libboost-math1.74-dev \ | |
| libboost-random1.74-dev | |
| RUN apt-get install -y --no-install-recommends --no-install-suggests qt6-base-dev libqt6svg6-dev libqt6opengl6-dev libqt6openglwidgets6 libgl-dev |
🧰 Tools
🪛 Trivy (0.69.3)
[error] 44-44: Image user should not be 'root'
Last USER command in Dockerfile should not be 'root'
Rule: DS-0002
(IaC/Dockerfile)
[error] 47-47: 'RUN update' instruction alone
The instruction 'RUN update' should always be followed by ' install' in the same RUN statement.
Rule: DS-0017
(IaC/Dockerfile)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Dockerfile.arm` around lines 44 - 56, The Dockerfile currently sets USER root
and runs multiple separated apt-get update/install RUNs; create a dedicated
non-root user and group (e.g., appuser) during the image build, chown any
necessary app files and switch to that user before the final CMD/ENTRYPOINT
(replace or move the existing USER root), and consolidate package installation
by combining apt-get -y update && apt-get install -y --no-install-recommends ...
into single RUN statements (replace the separate RUN apt-get -y update and
subsequent RUN apt-get install lines) to prevent stale package lists and reduce
image layers; update any file ownership or startup steps to use the new user.
Avoids conda libyaml-cpp transitive dependency that causes GLIBCXX linker conflicts when building TOPP tools under QEMU emulation.
Builds and pushes multi-arch (amd64 + arm64) Docker images to ghcr.io/openms/flashapp when a release is published. Chains on the Windows build workflow to ensure OpenMS-App.zip is available. Uses native runners (no QEMU) and registry-based caching.
Use system cmake from apt instead of pip and split the cmake configure into two passes — first without miniforge to discover system C++ libs, then with miniforge to enable pyopenms. This prevents miniforge's libyaml-cpp from ending up in TOPP link flags.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/publish-docker-images.yml:
- Around line 85-86: Replace the build-args usage in the publish workflow with
BuildKit secrets: remove the GITHUB_TOKEN under build-args in the
docker/build-push-action@v6 steps and add a secrets: entry that provides id:
gh_token and value: ${{ secrets.GITHUB_TOKEN }} so the token is mounted as a
secret instead of baked into the image; then update both Dockerfiles to remove
ARG GITHUB_TOKEN and ENV GH_TOKEN=${GITHUB_TOKEN} and change the gh release
download invocation to use BuildKit secret mounts (use RUN
--mount=type=secret,id=gh_token ...) where gh release download is called.
- Around line 158-163: The docker manifest create/push steps currently always
repoint IMAGE:latest to VERSION; add a guard so those steps only run when
VERSION is actually the repository's latest release (or when not a manual
workflow_dispatch backfill). Implement a short preparatory step (e.g.,
"get_latest") that queries the GitHub Releases API to output the current latest
tag, then add an if condition on the docker manifest create and docker manifest
push steps that checks env.VERSION equals that get_latest output (or that
github.event_name != 'workflow_dispatch' OR VERSION ==
steps.get_latest.outputs.tag) so manual backfills of older tags do not overwrite
IMAGE:latest.
In `@Dockerfile.arm`:
- Around line 152-175: The Dockerfile adds a cron entry that runs
clean-up-workspaces.py, but the image never creates the hardcoded
Path("/workspaces-flashapp") that script expects; add a step before the crontab
line to create the directory (e.g., RUN mkdir -p /workspaces-flashapp and set
ownership/permissions appropriate for the user that will run
clean-up-workspaces.py) so the first cron job won't fail; reference the
Dockerfile lines around WORKDIR /app, the COPY of clean-up-workspaces.py, and
the crontab RUN echo ... | crontab - when adding this directory creation step.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 944c2c3c-4ca6-4d01-a1c8-029f802507a8
📒 Files selected for processing (3)
.github/workflows/publish-docker-images.ymlDockerfileDockerfile.arm
Comment on lines
+85
to
+86
| build-args: | | ||
| GITHUB_TOKEN=${{ secrets.GITHUB_TOKEN }} |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'ARG GITHUB_TOKEN|ENV GH_TOKEN=|gh release download' Dockerfile Dockerfile.arm
sed -n '78,88p' .github/workflows/publish-docker-images.yml
sed -n '120,130p' .github/workflows/publish-docker-images.ymlRepository: OpenMS/FLASHApp
Length of output: 1392
Replace build-args with secrets: to prevent baking the GitHub token into the image.
The GITHUB_TOKEN is currently passed via build-args (lines 85–86 and 127–128), which causes it to be converted into an ENV variable in the Dockerfile (ENV GH_TOKEN=${GITHUB_TOKEN} at line 35 in both Dockerfiles). Docker ENV values persist in the final image and can be exposed through image metadata/inspection.
Use docker/build-push-action@v6's secrets: input with BuildKit secret mounts instead:
Workflow changes
- build-args: |
- GITHUB_TOKEN=${{ secrets.GITHUB_TOKEN }}
+ secrets: |
+ "gh_token=${{ secrets.GITHUB_TOKEN }}"Then in both Dockerfiles, replace the ARG GITHUB_TOKEN / ENV GH_TOKEN=${GITHUB_TOKEN} pattern with RUN --mount=type=secret,id=gh_token when invoking gh release download.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| build-args: | | |
| GITHUB_TOKEN=${{ secrets.GITHUB_TOKEN }} | |
| secrets: | | |
| "gh_token=${{ secrets.GITHUB_TOKEN }}" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/publish-docker-images.yml around lines 85 - 86, Replace
the build-args usage in the publish workflow with BuildKit secrets: remove the
GITHUB_TOKEN under build-args in the docker/build-push-action@v6 steps and add a
secrets: entry that provides id: gh_token and value: ${{ secrets.GITHUB_TOKEN }}
so the token is mounted as a secret instead of baked into the image; then update
both Dockerfiles to remove ARG GITHUB_TOKEN and ENV GH_TOKEN=${GITHUB_TOKEN} and
change the gh release download invocation to use BuildKit secret mounts (use RUN
--mount=type=secret,id=gh_token ...) where gh release download is called.
Comment on lines
+158
to
+163
| # Create/update latest manifest | ||
| docker manifest create ${{ env.IMAGE }}:latest \ | ||
| ${{ env.IMAGE }}:${VERSION}-amd64 \ | ||
| ${{ env.IMAGE }}:${VERSION}-arm64 | ||
|
|
||
| docker manifest push ${{ env.IMAGE }}:latest |
Contributor
There was a problem hiding this comment.
Guard latest on manual backfills.
workflow_dispatch accepts any release tag, but this step always repoints ghcr.io/openms/flashapp:latest to whatever version was requested. Rebuilding an older release would silently roll latest backward.
🛠️ One possible guard
on:
workflow_dispatch:
inputs:
tag:
description: 'Release tag to build (e.g., v0.9.15)'
required: true
+ update_latest:
+ description: 'Also move the latest tag'
+ type: boolean
+ default: false- # Create/update latest manifest
- docker manifest create ${{ env.IMAGE }}:latest \
- ${{ env.IMAGE }}:${VERSION}-amd64 \
- ${{ env.IMAGE }}:${VERSION}-arm64
-
- docker manifest push ${{ env.IMAGE }}:latest
+ if [ "${{ github.event_name }}" = "workflow_run" ] || [ "${{ github.event.inputs.update_latest }}" = "true" ]; then
+ docker manifest create ${{ env.IMAGE }}:latest \
+ ${{ env.IMAGE }}:${VERSION}-amd64 \
+ ${{ env.IMAGE }}:${VERSION}-arm64
+ docker manifest push ${{ env.IMAGE }}:latest
+ fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/publish-docker-images.yml around lines 158 - 163, The
docker manifest create/push steps currently always repoint IMAGE:latest to
VERSION; add a guard so those steps only run when VERSION is actually the
repository's latest release (or when not a manual workflow_dispatch backfill).
Implement a short preparatory step (e.g., "get_latest") that queries the GitHub
Releases API to output the current latest tag, then add an if condition on the
docker manifest create and docker manifest push steps that checks env.VERSION
equals that get_latest output (or that github.event_name != 'workflow_dispatch'
OR VERSION == steps.get_latest.outputs.tag) so manual backfills of older tags do
not overwrite IMAGE:latest.
Comment on lines
+152
to
+175
| # Create Redis data directory | ||
| RUN mkdir -p /var/lib/redis && chown redis:redis /var/lib/redis | ||
|
|
||
| # Create workdir and copy over all streamlit related files/folders. | ||
|
|
||
| # note: specifying folder with slash as suffix and repeating the folder name seems important to preserve directory structure | ||
| WORKDIR /app | ||
|
|
||
| COPY .streamlit/ /app/.streamlit | ||
| COPY assets/ /app/assets | ||
| COPY static/ /app/static | ||
| COPY clean-up-workspaces.py /app/clean-up-workspaces.py | ||
| COPY content/ /app/content | ||
| COPY example-data/ /app/example-data | ||
| COPY gdpr_consent/ /app/gdpr_consent | ||
| COPY hooks/ /app/hooks | ||
| COPY src/ /app/src | ||
| COPY app.py /app/app.py | ||
| COPY settings.json /app/settings.json | ||
| COPY default-parameters.json /app/default-parameters.json | ||
| COPY presets.json /app/presets.json | ||
|
|
||
| # add cron job to the crontab | ||
| RUN echo "0 3 * * * /root/miniforge3/envs/streamlit-env/bin/python /app/clean-up-workspaces.py >> /app/clean-up-workspaces.log 2>&1" | crontab - |
Contributor
There was a problem hiding this comment.
Create /workspaces-flashapp before installing the cleanup cron.
clean-up-workspaces.py hardcodes Path("/workspaces-flashapp"). This image never creates that directory, so the first cron run will fail instead of deleting stale workspaces.
🛠️ Proposed fix
RUN mkdir -p /var/lib/redis && chown redis:redis /var/lib/redis
+RUN mkdir -p /workspaces-flashapp🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Dockerfile.arm` around lines 152 - 175, The Dockerfile adds a cron entry that
runs clean-up-workspaces.py, but the image never creates the hardcoded
Path("/workspaces-flashapp") that script expects; add a step before the crontab
line to create the directory (e.g., RUN mkdir -p /workspaces-flashapp and set
ownership/permissions appropriate for the user that will run
clean-up-workspaces.py) so the first cron job won't fail; reference the
Dockerfile lines around WORKDIR /app, the COPY of clean-up-workspaces.py, and
the crontab RUN echo ... | crontab - when adding this directory creation step.
Ubuntu 22.04 apt cmake is only 3.22.1 but OpenMS externals require >= 3.24. Replace apt cmake with pip3-installed cmake available at /usr/local/bin without mamba activation.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Dockerfile (1)
50-51: Pincmaketo a constrained version for deterministic rebuilds.Line 51 installs whatever latest
cmakeis on PyPI at build time, which can cause non-reproducible images and surprise breakages. The Dockerfile comment itself documents that >= 3.24 is required for OpenMS externals.Proposed patch
-RUN pip3 install cmake +RUN pip3 install --no-cache-dir "cmake>=3.24,<4"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 50 - 51, The Dockerfile currently runs pip3 install cmake which pulls whatever latest cmake from PyPI at build time; change the RUN instruction that installs cmake to pin a specific, constrained version to ensure deterministic builds (for example replace pip3 install cmake with a pinned requirement such as pip3 install "cmake==<specific-version>" or a constrained range like "cmake>=3.24,<4.0"); update the comment to note the chosen pinned version and why (OpenMS requires >=3.24) and ensure the RUN line uses that exact version string instead of the unpinned package name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Dockerfile.arm`:
- Around line 34-35: The Dockerfile currently persists the GitHub token into
image layers by setting ENV GH_TOKEN from ARG GITHUB_TOKEN; remove the ENV
GH_TOKEN assignments (e.g., the ENV GH_TOKEN lines) so the secret is not baked
into the image, and instead keep GITHUB_TOKEN only as ARG and pass it inline at
the time you invoke gh (for example prefix the gh invocation with the
environment variable from ARG or pipe the ARG into gh auth login --with-token)
so the token is only used during build/run and not stored in image metadata;
apply the same change to the other occurrences noted (the similar ENV lines
around the other blocks).
---
Nitpick comments:
In `@Dockerfile`:
- Around line 50-51: The Dockerfile currently runs pip3 install cmake which
pulls whatever latest cmake from PyPI at build time; change the RUN instruction
that installs cmake to pin a specific, constrained version to ensure
deterministic builds (for example replace pip3 install cmake with a pinned
requirement such as pip3 install "cmake==<specific-version>" or a constrained
range like "cmake>=3.24,<4.0"); update the comment to note the chosen pinned
version and why (OpenMS requires >=3.24) and ensure the RUN line uses that exact
version string instead of the unpinned package name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a976b5e8-88e3-4562-8569-1fe108d0f3d0
📒 Files selected for processing (2)
DockerfileDockerfile.arm
Comment on lines
+34
to
+35
| ARG GITHUB_TOKEN | ||
| ENV GH_TOKEN=${GITHUB_TOKEN} |
Contributor
There was a problem hiding this comment.
Do not persist GitHub token in image environment.
Line 35 bakes the token into image env metadata/layers. Use ARG only at download time and pass it inline to gh instead.
Proposed patch
-ENV GH_TOKEN=${GITHUB_TOKEN}
@@
-RUN if [ -n "$GH_TOKEN" ]; then \
+ARG GITHUB_TOKEN
+RUN if [ -n "$GITHUB_TOKEN" ]; then \
echo "GH_TOKEN is set, proceeding to download the release asset..."; \
- gh release download -R ${GITHUB_USER}/${GITHUB_REPO} -p "OpenMS-App.zip" -D /app; \
+ GH_TOKEN="$GITHUB_TOKEN" gh release download -R ${GITHUB_USER}/${GITHUB_REPO} -p "${ASSET_NAME}" -D /app; \
else \
echo "GH_TOKEN is not set, skipping the release asset download."; \
fiAlso applies to: 40-41, 257-260
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Dockerfile.arm` around lines 34 - 35, The Dockerfile currently persists the
GitHub token into image layers by setting ENV GH_TOKEN from ARG GITHUB_TOKEN;
remove the ENV GH_TOKEN assignments (e.g., the ENV GH_TOKEN lines) so the secret
is not baked into the image, and instead keep GITHUB_TOKEN only as ARG and pass
it inline at the time you invoke gh (for example prefix the gh invocation with
the environment variable from ARG or pipe the ARG into gh auth login
--with-token) so the token is only used during build/run and not stored in image
metadata; apply the same change to the other occurrences noted (the similar ENV
lines around the other blocks).
Add CMAKE_IGNORE_PREFIX_PATH=/root/miniforge3 to both cmake passes to prevent cmake from discovering miniforge's yaml-cpp, which requires GLIBCXX_3.4.32 not available in Ubuntu 22.04's system libstdc++ on ARM64.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
Dockerfile.arm (2)
34-35:⚠️ Potential issue | 🟠 MajorDo not bake the GitHub token into image metadata.
Line 35 persists the token as image environment data. Also, Line 259 bypasses
ARG ASSET_NAMEby hardcoding the filename.🔐 Proposed patch
ARG GITHUB_TOKEN -ENV GH_TOKEN=${GITHUB_TOKEN} @@ -RUN if [ -n "$GH_TOKEN" ]; then \ +RUN if [ -n "$GITHUB_TOKEN" ]; then \ echo "GH_TOKEN is set, proceeding to download the release asset..."; \ - gh release download -R ${GITHUB_USER}/${GITHUB_REPO} -p "OpenMS-App.zip" -D /app; \ + GH_TOKEN="$GITHUB_TOKEN" gh release download -R ${GITHUB_USER}/${GITHUB_REPO} -p "${ASSET_NAME}" -D /app; \ else \ echo "GH_TOKEN is not set, skipping the release asset download."; \ fiAlso applies to: 257-260
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile.arm` around lines 34 - 35, Remove the instruction that persists the GitHub token into image metadata (do not set ENV GH_TOKEN from ARG GITHUB_TOKEN); instead rely on ARG GITHUB_TOKEN only at build-time or use Docker build secrets so the token isn't baked into the image, and ensure any runtime code reads tokens from secure runtime mechanisms (not an ENV in the Dockerfile). Also replace the hardcoded asset filename that bypasses ARG ASSET_NAME with usage of ARG ASSET_NAME so the build uses the provided argument (fix references around ASSET_NAME and the hardcoded filename at the spot noted), and verify both ARG GITHUB_TOKEN and ARG ASSET_NAME are only used in build stages that do not persist them to final image layers.
44-44:⚠️ Potential issue | 🟠 MajorContainer runtime remains root.
Line 44 keeps all runtime processes privileged. For production deployment, switch final app processes to a non-root user (even if setup/bootstrap steps remain root).
Also applies to: 188-245
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile.arm` at line 44, The Dockerfile currently leaves runtime processes running as root (the USER root instruction); create a dedicated non-root user (e.g., appuser) and group during image build, chown the app directories/files used at runtime to that user, and switch to that user with USER appuser before the final runtime stage so the final app process does not run as root; ensure any bootstrap/setup steps that require root remain before the USER appuser switch and reference the existing USER root instruction, created user name (appuser), and relevant app directories when applying chown.
🧹 Nitpick comments (1)
Dockerfile.arm (1)
61-67: Add--no-install-recommendsin the GitHub CLI install step too.Line 61-Line 67 install
wget/ghwithout the same package-minimization policy used elsewhere.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile.arm` around lines 61 - 67, The RUN block installs wget and gh without using the project's package-minimization flag; update the apt-get install invocations in that RUN line so both "apt-get install wget -y" and "apt-get install gh -y" include "--no-install-recommends" (preserve the existing chaining, mkdir/wget/tee/chmod/echo/apt-get update steps and only add the flag to the install commands) to match the minimal install policy used elsewhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Dockerfile.arm`:
- Around line 34-35: Remove the instruction that persists the GitHub token into
image metadata (do not set ENV GH_TOKEN from ARG GITHUB_TOKEN); instead rely on
ARG GITHUB_TOKEN only at build-time or use Docker build secrets so the token
isn't baked into the image, and ensure any runtime code reads tokens from secure
runtime mechanisms (not an ENV in the Dockerfile). Also replace the hardcoded
asset filename that bypasses ARG ASSET_NAME with usage of ARG ASSET_NAME so the
build uses the provided argument (fix references around ASSET_NAME and the
hardcoded filename at the spot noted), and verify both ARG GITHUB_TOKEN and ARG
ASSET_NAME are only used in build stages that do not persist them to final image
layers.
- Line 44: The Dockerfile currently leaves runtime processes running as root
(the USER root instruction); create a dedicated non-root user (e.g., appuser)
and group during image build, chown the app directories/files used at runtime to
that user, and switch to that user with USER appuser before the final runtime
stage so the final app process does not run as root; ensure any bootstrap/setup
steps that require root remain before the USER appuser switch and reference the
existing USER root instruction, created user name (appuser), and relevant app
directories when applying chown.
---
Nitpick comments:
In `@Dockerfile.arm`:
- Around line 61-67: The RUN block installs wget and gh without using the
project's package-minimization flag; update the apt-get install invocations in
that RUN line so both "apt-get install wget -y" and "apt-get install gh -y"
include "--no-install-recommends" (preserve the existing chaining,
mkdir/wget/tee/chmod/echo/apt-get update steps and only add the flag to the
install commands) to match the minimal install policy used elsewhere.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: be90a62d-bae1-481c-91d1-ebf481cfde45
📒 Files selected for processing (2)
DockerfileDockerfile.arm
Ubuntu 24.04 ships GCC 13 / GLIBCXX_3.4.32 natively, eliminating the linker conflict with miniforge's libyaml-cpp.so. This also lets us install cmake from apt (>= 3.28) instead of pip, use unversioned boost packages, and stop deleting CMakeFiles before pyopenms build.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Dockerfile.arm (1)
34-35:⚠️ Potential issue | 🟠 MajorDo not persist
GITHUB_TOKENin the image environment.Lines 34-35 copy the build arg into image metadata, and Lines 255-257 then rely on that persisted env var in the final stage. Any build that supplies
GITHUB_TOKENwill bake the secret into the resulting image config. Keep it as a stage-localARGnear the download step and pass it inline toghinstead.🔒 Proposed fix
-ARG GITHUB_TOKEN -ENV GH_TOKEN=${GITHUB_TOKEN} @@ # Download latest OpenMS App executable as a ZIP file -RUN if [ -n "$GH_TOKEN" ]; then \ - echo "GH_TOKEN is set, proceeding to download the release asset..."; \ - gh release download -R ${GITHUB_USER}/${GITHUB_REPO} -p "OpenMS-App.zip" -D /app; \ +ARG GITHUB_TOKEN +RUN if [ -n "$GITHUB_TOKEN" ]; then \ + echo "GITHUB_TOKEN is set, proceeding to download the release asset..."; \ + GH_TOKEN="$GITHUB_TOKEN" gh release download -R ${GITHUB_USER}/${GITHUB_REPO} -p "${ASSET_NAME}" -D /app; \ else \ - echo "GH_TOKEN is not set, skipping the release asset download."; \ + echo "GITHUB_TOKEN is not set, skipping the release asset download."; \ fiAlso applies to: 255-257
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile.arm` around lines 34 - 35, Remove the ENV GH_TOKEN persist and avoid baking GITHUB_TOKEN into the image by deleting the ENV GH_TOKEN line and keeping GITHUB_TOKEN only as a stage-local ARG near the step that downloads using gh; instead of relying on GH_TOKEN in the final stage (references: ARG GITHUB_TOKEN, ENV GH_TOKEN and the later step that uses GH_TOKEN around lines 255-257), pass the token inline to the gh command (e.g., gh ... --hostname ... --cli-arg 'GITHUB_TOKEN=...' or via echoing into gh's stdin) or use build-stage ARG forwarding/secret mechanisms so the token is never written to the image metadata.
🧹 Nitpick comments (1)
Dockerfile.arm (1)
12-12: Use a supported LTS Node image for the JS build stage.Line 12 pins the build stage to
node:21, but Node 21 reached end-of-life on June 1, 2024, and odd-numbered Node releases are intentionally short-lived. Please move this stage to a supported even-numbered LTS tag before the ARM image becomes part of the regular release path. (github.com)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile.arm` at line 12, Change the JS build base image from the EOL node:21 to a supported even-numbered LTS tag by updating the Dockerfile's build stage reference ("FROM node:21 AS js-build") to a current LTS (e.g., node:20 or node:22) so the ARM image uses a supported Node release; pick the active LTS for your release schedule and update the tag accordingly, then rerun the build to verify the image resolves and the [request_verification] passes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Dockerfile.arm`:
- Around line 34-35: Remove the ENV GH_TOKEN persist and avoid baking
GITHUB_TOKEN into the image by deleting the ENV GH_TOKEN line and keeping
GITHUB_TOKEN only as a stage-local ARG near the step that downloads using gh;
instead of relying on GH_TOKEN in the final stage (references: ARG GITHUB_TOKEN,
ENV GH_TOKEN and the later step that uses GH_TOKEN around lines 255-257), pass
the token inline to the gh command (e.g., gh ... --hostname ... --cli-arg
'GITHUB_TOKEN=...' or via echoing into gh's stdin) or use build-stage ARG
forwarding/secret mechanisms so the token is never written to the image
metadata.
---
Nitpick comments:
In `@Dockerfile.arm`:
- Line 12: Change the JS build base image from the EOL node:21 to a supported
even-numbered LTS tag by updating the Dockerfile's build stage reference ("FROM
node:21 AS js-build") to a current LTS (e.g., node:20 or node:22) so the ARM
image uses a supported Node release; pick the active LTS for your release
schedule and update the tag accordingly, then rerun the build to verify the
image resolves and the [request_verification] passes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ea2bb08b-a888-4adf-b977-45aafcae7c94
📒 Files selected for processing (4)
.github/workflows/build-docker-images.yml.github/workflows/publish-docker-images.ymlDockerfileDockerfile.arm
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/publish-docker-images.yml
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add Dockerfile.arm for linux/aarch64 builds (macOS Apple Silicon) with aarch64 Miniforge, conditional THIRDPARTY copy, and trimmed thirdparty PATH. Add parallel ARM64 build job to CI workflow using ubuntu-22.04-arm runner.
Summary by CodeRabbit
New Features
Chores