Add diann 232#6
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThis pull request refactors the container build workflow and adds support for DIA-NN 2.3.2. The GitHub Actions workflow is simplified to use a fixed version matrix with standardized environment variables and metadata actions, while Singularity image handling transitions to a dedicated setup action. A new Dockerfile for DIA-NN 2.3.2 is introduced. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
only needed to change version -> could be even a more generic dockerfile.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
diann-2.3.2/Dockerfile (1)
43-52: Consider adding a non-root user.The container runs as root by default, which is flagged by static analysis (DS-0002). For data processing tools, consider adding a non-root user to follow security best practices.
🛡️ Proposed fix to add non-root user
# Create a symbolic link and add to PATH RUN ln -s /usr/diann-2.3.2/diann-linux /usr/diann-2.3.2/diann ENV PATH="$PATH:/usr/diann-2.3.2" +# Create non-root user +RUN useradd -m -s /bin/bash diann +USER diann + WORKDIR /data/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@diann-2.3.2/Dockerfile` around lines 43 - 52, The Dockerfile currently leaves the container running as root; create a non-root user and group (e.g., diann) in the Dockerfile, chown the installed binary directory (/usr/diann-2.3.2) and the WORKDIR (/data) to that user, set a HOME if needed, and switch to that user with the USER directive before finishing the image so the diann-linux binary and symlink remain executable by the non-root user and the container no longer runs as root.
🤖 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/quantms-containers.yml:
- Around line 90-110: The three workflow steps named "Set up Singularity", "Pull
Docker image for Singularity conversion", and "Convert Docker image to
Singularity" use an incorrect boolean check on github.event.inputs.push_images
(comparing to true), so update each step's if expression to treat the input as a
string (e.g., compare to 'true' or check for empty string) — replace the current
condition for these steps with a string-safe check like
github.event.inputs.push_images == 'true' || github.event.inputs.push_images ==
'' so the Singularity setup, docker pull/save, and singularity build steps run
when the input is actually set to 'true' or left empty.
- Around line 35-38: The CI matrix under the YAML key matrix -> version
currently lists "2.1.0" and "1.8.1" but is missing the newly added diann image
2.3.2; update the version list in the matrix (the version array) to include
"2.3.2" so the new diann-2.3.2/Dockerfile is built by the workflow.
- Around line 78-88: The build/push step uses the matrix to both push images and
apply a :latest tag which causes two problems: the push boolean reads
github.event.inputs (missing on non-dispatch events) and every matrix job tags
:latest causing a race. Fix by changing the push condition to detect dispatch vs
other events (e.g., use github.event_name == 'workflow_dispatch' ?
github.event.inputs.push_images : true) or explicitly check github.event_name
instead of accessing github.event.inputs directly, and remove :latest from the
matrix build: either stop tagging :latest in this matrix and add a separate job
that runs only for the chosen highest version (or uses an if: that checks
matrix.version == '<highest-version>') to push the :latest tag, or conditionally
add the :latest tag only when matrix.version equals the designated latest
version.
In `@diann-2.3.2/Dockerfile`:
- Around line 1-52: The CI matrix in .github/workflows/quantms-containers.yml
currently only lists versions 2.1.0 and 1.8.1 so the new diann-2.3.2/Dockerfile
will never be built; update the workflow by adding "2.3.2" to the
jobs.strategy.matrix entry that enumerates the diann versions (or replace the
hardcoded matrix with a dynamic detection/inclusion approach that globs
diann-*/Dockerfile directories and builds for each found folder) so that
diann-2.3.2 is included in the build/push job.
- Around line 34-37: The RUN step that downloads DIA-NN uses wget with the
insecure --no-check-certificate flag; remove that flag and instead ensure the
image has up-to-date CA certificates before fetching the file (install/upgrade
ca-certificates via the Dockerfile prior to the RUN that downloads DIA-NN), then
download the release URL (the wget command referencing
https://github.com/vdemichev/DiaNN/releases/download/2.0/DIA-NN-2.3.2-Academia-Linux.zip
and the local filename DIA-NN-2.3.2-Academia-Linux.zip) without bypassing
certificate validation; additionally, verify the release tag vs filename
mismatch with the project maintainers and update the URL/tag if needed.
---
Nitpick comments:
In `@diann-2.3.2/Dockerfile`:
- Around line 43-52: The Dockerfile currently leaves the container running as
root; create a non-root user and group (e.g., diann) in the Dockerfile, chown
the installed binary directory (/usr/diann-2.3.2) and the WORKDIR (/data) to
that user, set a HOME if needed, and switch to that user with the USER directive
before finishing the image so the diann-linux binary and symlink remain
executable by the non-root user and the container no longer runs as root.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e78e9762-a211-45c1-8b80-d169888b5fc9
📒 Files selected for processing (2)
.github/workflows/quantms-containers.ymldiann-2.3.2/Dockerfile
Comment on lines
+34
to
+37
| # Download and install DIA-NN | ||
| RUN wget --no-check-certificate https://github.com/vdemichev/DiaNN/releases/download/2.0/DIA-NN-2.3.2-Academia-Linux.zip && \ | ||
| unzip DIA-NN-2.3.2-Academia-Linux.zip -d /usr/ && \ | ||
| rm DIA-NN-2.3.2-Academia-Linux.zip |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the correct download URL for DIA-NN 2.3.2
echo "Checking if the URL with tag 2.0 exists:"
curl -sI "https://github.com/vdemichev/DiaNN/releases/download/2.0/DIA-NN-2.3.2-Academia-Linux.zip" | head -5
echo ""
echo "Checking if the URL with tag 2.3.2 exists:"
curl -sI "https://github.com/vdemichev/DiaNN/releases/download/2.3.2/DIA-NN-2.3.2-Academia-Linux.zip" | head -5
echo ""
echo "Listing available releases:"
curl -s "https://api.github.com/repos/vdemichev/DiaNN/releases" | jq '.[].tag_name' | head -10Repository: bigbio/quantms-containers
Length of output: 747
Security risk: Remove certificate validation bypass.
The --no-check-certificate flag disables SSL certificate validation, which exposes the download to MITM attacks. If there's a certificate issue with GitHub, the proper fix is to update CA certificates in the container rather than bypass validation.
The version mismatch between the release tag (2.0) and filename (DIA-NN-2.3.2-Academia-Linux.zip) is worth investigating, but the current URL does function (returns a valid redirect). Before changing the tag, verify with the DIA-NN project maintainers that the file under release 2.0 is indeed the correct Academia version.
🔒 Recommended fix
-RUN wget --no-check-certificate https://github.com/vdemichev/DiaNN/releases/download/2.0/DIA-NN-2.3.2-Academia-Linux.zip && \
+RUN wget https://github.com/vdemichev/DiaNN/releases/download/2.0/DIA-NN-2.3.2-Academia-Linux.zip && \
unzip DIA-NN-2.3.2-Academia-Linux.zip -d /usr/ && \
rm DIA-NN-2.3.2-Academia-Linux.zip📝 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
| # Download and install DIA-NN | |
| RUN wget --no-check-certificate https://github.com/vdemichev/DiaNN/releases/download/2.0/DIA-NN-2.3.2-Academia-Linux.zip && \ | |
| unzip DIA-NN-2.3.2-Academia-Linux.zip -d /usr/ && \ | |
| rm DIA-NN-2.3.2-Academia-Linux.zip | |
| # Download and install DIA-NN | |
| RUN wget https://github.com/vdemichev/DiaNN/releases/download/2.0/DIA-NN-2.3.2-Academia-Linux.zip && \ | |
| unzip DIA-NN-2.3.2-Academia-Linux.zip -d /usr/ && \ | |
| rm DIA-NN-2.3.2-Academia-Linux.zip |
🧰 Tools
🪛 Checkov (3.2.508)
[high] 35-37: Ensure that certificate validation isn't disabled with wget
(CKV2_DOCKER_3)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@diann-2.3.2/Dockerfile` around lines 34 - 37, The RUN step that downloads
DIA-NN uses wget with the insecure --no-check-certificate flag; remove that flag
and instead ensure the image has up-to-date CA certificates before fetching the
file (install/upgrade ca-certificates via the Dockerfile prior to the RUN that
downloads DIA-NN), then download the release URL (the wget command referencing
https://github.com/vdemichev/DiaNN/releases/download/2.0/DIA-NN-2.3.2-Academia-Linux.zip
and the local filename DIA-NN-2.3.2-Academia-Linux.zip) without bypassing
certificate validation; additionally, verify the release tag vs filename
mismatch with the project maintainers and update the URL/tag if needed.
Collaborator
Author
There was a problem hiding this comment.
@ypriverol Is there a reason you do not check the certificate?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
This was referenced Apr 3, 2026
5 tasks
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
2 participants
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.
Summary by CodeRabbit
Release Notes
New Features
Chores