Conversation
- only run on modification (detect-changes) - relink workflow - openms-sync step Relink and open-ms step could be set to run only if the owner is bigbio
- see if token works
Updates (new images, instructions)
📝 WalkthroughWalkthroughCentralizes registry env, replaces hardcoded Changes
Sequence Diagram(s)sequenceDiagram
participant Actions as GitHub Actions
participant Docker as Docker Engine
participant Singularity as Singularity CLI
participant GHCR as GHCR / ORAS
Actions->>Docker: build image(s) (matrix)
Docker->>Docker: docker save -> ${matrix}.tar
Actions->>Singularity: singularity build ${matrix}.sif from ${matrix}.tar
Singularity->>GHCR: singularity remote login (GITHUB_TOKEN)
Singularity->>GHCR: oras push ${matrix}.sif to ghcr.io/${{ github.repository_owner }}
GHCR-->>Actions: push result
Actions->>Actions: run success()/failure() notification steps
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
.github/workflows/quantms-containers.yml (2)
254-260: Hardcodedbigbioin Singularity push URIs.While the job condition ensures this only runs for
bigbio(line 214), the hardcoded URIs at lines 256 and 259 are inconsistent with the parameterized approach used inbuild-diann(lines 192, 197). Consider using${{ github.repository_owner }}for consistency.Suggested change
- singularity push image.sif oras://ghcr.io/bigbio/${{ matrix.sif }} + singularity push image.sif oras://ghcr.io/${{ github.repository_owner }}/${{ matrix.sif }} if [[ "${{ github.event_name }}" == "release" && -n "${{ matrix.extra_tags }}" ]]; then SIF_LATEST=$(echo "${{ matrix.sif }}" | sed 's/:[^:]*$/:latest/') - singularity push image.sif oras://ghcr.io/bigbio/$SIF_LATEST + singularity push image.sif oras://ghcr.io/${{ github.repository_owner }}/$SIF_LATEST fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/quantms-containers.yml around lines 254 - 260, The Singularity push URIs are hardcoded to "bigbio" which breaks consistency with the parameterized owner used elsewhere; update the two singularity push targets that reference ghcr.io/bigbio/${{ matrix.sif }} and ghcr.io/bigbio/$SIF_LATEST to use the repository owner variable (e.g., ghcr.io/${{ github.repository_owner }}/${{ matrix.sif }} and ghcr.io/${{ github.repository_owner }}/$SIF_LATEST) so they match the parameterized pattern used by build-diann and respect different repo owners.
28-31:SINGULARITY_IMAGE_NAMEenvironment variable is defined but never used.The
SINGULARITY_IMAGE_NAMEenv var is defined at line 31, but the Singularity push steps (lines 192, 197) use${{ matrix.sif }}instead. Either use this env var consistently or remove it to avoid confusion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/quantms-containers.yml around lines 28 - 31, The SINGULARITY_IMAGE_NAME env var is defined but not used; update the workflow to either remove SINGULARITY_IMAGE_NAME or make the Singularity push steps use it consistently by replacing occurrences of `${{ matrix.sif }}` with the environment variable `${{ env.SINGULARITY_IMAGE_NAME }}` (or set `matrix.sif` from that env var); locate references to SINGULARITY_IMAGE_NAME and the push steps that reference `matrix.sif` to ensure a single source of truth and update the push step arguments and any matrix definitions accordingly.
🤖 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 138-149: The meta and date steps are producing outputs that are
never consumed; either remove the dead steps (ids meta and date) or wire their
outputs into the build step: expose docker/metadata-action outputs (e.g., use
steps.meta.outputs.tags or steps.meta.outputs.image) and the date step output
(steps.date.outputs.DATE_TAG) and replace matrix.tag / matrix.extra_tags in the
build-push-action invocation with the corresponding steps.meta.outputs.* and
steps.date.outputs.DATE_TAG references so the extracted tags are actually used.
- Around line 131-136: The Docker login in the build-diann job is using
github.actor while build-relink and sync-openms use github.repository_owner;
update the build-diann job's docker/login-action step (the "Log in to GitHub
Container Registry" step) to set username: ${{ github.repository_owner }} to
match the other jobs, or add a short comment in that step explaining why
github.actor must differ if there's an intentional reason—ensure consistency
across the jobs (build-diann, build-relink, sync-openms) by using
github.repository_owner unless a documented exception is required.
- Around line 121-123: RETRY_TIMES and RETRY_DELAY are defined but never used;
either remove these unused env vars or wire them into retry steps: update the
workflow to reference RETRY_TIMES and RETRY_DELAY when adding retry behavior
(for example by using the nick-fields/retry@v2 action like in the sync-openms
job) so that the retry action reads the environment variables (RETRY_TIMES,
RETRY_DELAY) to control attempts and delay, or simply delete RETRY_TIMES and
RETRY_DELAY from the env block if no retry logic is required.
- Line 155: Replace the current push condition so it only reads workflow inputs
for workflow_dispatch events and compares the input as a string; specifically,
change the push condition to check github.event_name == 'workflow_dispatch' &&
(github.event.inputs.push_images == 'true' || github.event.inputs.push_images ==
'') instead of the boolean comparison, and apply this same change to the other
push conditions referenced (the additional push: occurrences on the same
workflow). This ensures inputs are only evaluated for workflow_dispatch and uses
string comparisons for github.event.inputs.push_images.
---
Nitpick comments:
In @.github/workflows/quantms-containers.yml:
- Around line 254-260: The Singularity push URIs are hardcoded to "bigbio" which
breaks consistency with the parameterized owner used elsewhere; update the two
singularity push targets that reference ghcr.io/bigbio/${{ matrix.sif }} and
ghcr.io/bigbio/$SIF_LATEST to use the repository owner variable (e.g.,
ghcr.io/${{ github.repository_owner }}/${{ matrix.sif }} and ghcr.io/${{
github.repository_owner }}/$SIF_LATEST) so they match the parameterized pattern
used by build-diann and respect different repo owners.
- Around line 28-31: The SINGULARITY_IMAGE_NAME env var is defined but not used;
update the workflow to either remove SINGULARITY_IMAGE_NAME or make the
Singularity push steps use it consistently by replacing occurrences of `${{
matrix.sif }}` with the environment variable `${{ env.SINGULARITY_IMAGE_NAME }}`
(or set `matrix.sif` from that env var); locate references to
SINGULARITY_IMAGE_NAME and the push steps that reference `matrix.sif` to ensure
a single source of truth and update the push step arguments and any matrix
definitions accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f2177863-7053-466d-846e-386c8f0057a0
📒 Files selected for processing (1)
.github/workflows/quantms-containers.yml
Collaborator
|
So the login fails with your user as well. Before you used repository secrets, so maybe your GHCR setup is different? now: echo *** | singularity remote login -u ypriverol --password-stdin oras://ghcr.io
# Push with version tag
singularity push diann-2.3.2.sif oras://ghcr.io/bigbio/diann-sif:2.3.2before: export SINGULARITY_DOCKER_USERNAME=***
export SINGULARITY_DOCKER_PASSWORD=***
singularity push image.sif oras://ghcr.io/bigbio/diann-sif:2.3.2 |
Member
Author
|
What is the difference with the current main branch; the login and push works with the current approach; better to do not changed it; I have tried other approaches in the past that didn't work. |
- Fix critical bug: push condition used broken input comparison that would push images on pull_request events. Restored safe guard: github.event_name != 'pull_request' (build always, push only on merge) - Remove dead code: unused metadata-action step, date tag step, RETRY_TIMES/RETRY_DELAY env vars, and IMAGE_NAME/SINGULARITY_IMAGE_NAME - Standardize Docker login to use github.repository_owner across all jobs - Standardize Singularity install (eWaterCycle/setup-singularity@v7) and auth (singularity remote login --password-stdin) across all jobs, replacing fragile apt-get || true and env-var-based auth - Replace all hardcoded bigbio refs in push URIs with github.repository_owner - Remove redundant docker pull (image already loaded via load: true) https://claude.ai/code/session_01UFH8ge5154Z8q4DCEbQCTX
Collaborator
I think there is a way to use Personal Access Tokens which are set based on the user's repository permission. I had to do nothing to be able to push images to our ghcr. |
secrets.GITHUB_TOKEN is scoped to the workflow actor, so the login username must match. Using github.repository_owner causes 401 errors when the actor differs from the owner (e.g. collaborators, bots). Applies to all 3 Docker logins and all 3 Singularity logins. https://claude.ai/code/session_01UFH8ge5154Z8q4DCEbQCTX
Refactor container workflows to use dynamic registry and repository owner
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/quantms-containers.yml (1)
138-140: Emptyextra_tagsproduces blank line in tags input.When
matrix.extra_tagsis an empty string (e.g., for diann-2.2.0 through diann-1.8.1), the tags block will contain a trailing blank line. Whiledocker/build-push-actiontypically handles this gracefully, it's cleaner to conditionally include the extra tag.♻️ Proposed fix using conditional expression
tags: | ${{ matrix.tag }} - ${{ matrix.extra_tags }} + ${{ matrix.extra_tags != '' && matrix.extra_tags || '' }}Or use a multi-line conditional in the step:
tags: ${{ matrix.extra_tags != '' && format('{0}{1}{2}', matrix.tag, '\n', matrix.extra_tags) || matrix.tag }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/quantms-containers.yml around lines 138 - 140, The tags input may contain a trailing blank line when matrix.extra_tags is an empty string; update the workflow step that sets tags (the tags: input used with docker/build-push-action) to conditionally include matrix.extra_tags only when non-empty (e.g., replace the current multi-line tags block with a single expression that concatenates matrix.tag and matrix.extra_tags only if matrix.extra_tags != '' so you don't emit a blank line).
🤖 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 237-243: Replace the direct use of GitHub secrets in the
Singularity authentication step by reading credentials from the environment
variables SINGULARITY_DOCKER_USERNAME and SINGULARITY_DOCKER_PASSWORD;
specifically, change the singularity remote login invocation that currently uses
${{ secrets.GITHUB_TOKEN }} and ${{ github.actor }} to use these env vars
instead (keep the target URL oras://ghcr.io), so the echo/--password-stdin
pattern uses SINGULARITY_DOCKER_PASSWORD and the -u flag uses
SINGULARITY_DOCKER_USERNAME; leave the subsequent singularity push commands
(singularity push image.sif oras://ghcr.io/${{ github.repository_owner }}/${{
matrix.sif }} and the SIF_LATEST push logic using matrix.sif and SIF_LATEST)
unchanged.
- Around line 290-297: Replace the inline secrets usage in the "Sync OpenMS
Singularity" step with an environment variable like SINGULARITY_REMOTE_TOKEN
(set in the job's env from secrets.GITHUB_TOKEN), and change the login command
to echo "$SINGULARITY_REMOTE_TOKEN" | singularity remote login -u ${{
github.actor }} --password-stdin; keep the rest of the step (singularity
pull/push and conditional push on release) unchanged so authentication is
consistent with other jobs that use the environment variable approach.
- Around line 164-176: Replace the `singularity remote login` + `echo ${{
secrets.GITHUB_TOKEN }}` approach in the "Login and Deploy Container" step with
environment-based authentication by exporting SINGULARITY_DOCKER_USERNAME and
SINGULARITY_DOCKER_PASSWORD from secrets (e.g., set env:
SINGULARITY_DOCKER_USERNAME: ${{ secrets.SINGULARITY_DOCKER_USERNAME }} and
SINGULARITY_DOCKER_PASSWORD: ${{ secrets.SINGULARITY_DOCKER_PASSWORD }}) and
remove the persistent `singularity remote login` call; also stop using `echo` to
pass secrets (if you must pass a secret to a command, use printf '%s' or a
secure heredoc) and then call `singularity push ${{ matrix.context }}.sif
oras://${{ env.REGISTRY }}/${{ github.repository_owner }}/${{ matrix.sif }}` and
the existing latest-tag logic (SIF_LATEST/Singularity push) unchanged so pushes
continue to use the matrix.sif and matrix.context variables securely.
---
Nitpick comments:
In @.github/workflows/quantms-containers.yml:
- Around line 138-140: The tags input may contain a trailing blank line when
matrix.extra_tags is an empty string; update the workflow step that sets tags
(the tags: input used with docker/build-push-action) to conditionally include
matrix.extra_tags only when non-empty (e.g., replace the current multi-line tags
block with a single expression that concatenates matrix.tag and
matrix.extra_tags only if matrix.extra_tags != '' so you don't emit a blank
line).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 02b036a5-cf2f-416d-a13d-9beeeaf2baf3
📒 Files selected for processing (1)
.github/workflows/quantms-containers.yml
Comment on lines
+164
to
176
| - name: Login and Deploy Container | ||
| if: github.event_name != 'pull_request' | ||
| run: | | ||
| echo ${{ secrets.GITHUB_TOKEN }} | singularity remote login -u ${{ github.actor }} --password-stdin oras://ghcr.io | ||
|
|
||
| # Push with version tag | ||
| singularity push ${{ matrix.context }}.sif oras://${{ env.REGISTRY }}/${{ github.repository_owner }}/${{ matrix.sif }} | ||
|
|
||
| # If this is a release event, also tag as latest | ||
| if [[ "${{ github.event_name }}" == "release" && -n "${{ matrix.extra_tags }}" ]]; then | ||
| SIF_LATEST=$(echo "${{ matrix.sif }}" | sed 's/:[^:]*$/:latest/') | ||
| singularity push image.sif oras://ghcr.io/bigbio/$SIF_LATEST | ||
| singularity push ${{ matrix.context }}.sif oras://${{ env.REGISTRY }}/${{ github.repository_owner }}/$SIF_LATEST | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Singularity ORAS push authentication SINGULARITY_DOCKER_USERNAME vs remote login
💡 Result:
For singularity push ... oras://... (OCI/ORAS registry), Singularity can authenticate in two different ways:
1) SINGULARITY_DOCKER_USERNAME / SINGULARITY_DOCKER_PASSWORD (or flags)
- These are registry credentials used for
docker://andoras://operations. - They’re typically used for non-interactive/CI runs (or you can pass
--docker-username/--docker-passwordon the command). - In SingularityCE, these env vars take precedence over credentials loaded from an authfile / stored logins. [2]
2) singularity remote login (stored login)
- Despite the name,
remote logincan log in not only to a Sylabs “remote endpoint”, but also directly to an OCI/Docker registry, including anoras://registry URI. [1] - It stores the token/password in a docker-config-style file under your home directory (e.g.
~/.singularity/docker-config.json). [2]
Practical rule
- Use env vars / flags when you want ephemeral, job-scoped auth (CI, batch jobs, avoiding writing creds to disk). [2]
- Use
remote login(or, in SingularityCE 4.x, the newersingularity registry login) when you want a persistent local login that futureoras://pushes/pulls will reuse automatically. [1][2]
Sources: SingularityCE remote login docs (shows oras://... registry login) [1]; SingularityCE 4.1 user guide (credential storage + precedence of SINGULARITY_DOCKER_*) [2].
Use environment variables for Singularity authentication in CI—best practice and more secure.
This approach has two issues:
-
CI best practice: For GitHub Actions (ephemeral CI jobs),
SINGULARITY_DOCKER_USERNAMEandSINGULARITY_DOCKER_PASSWORDenvironment variables are the documented recommended approach. Thesingularity remote loginmethod stores credentials persistently to disk (~/.singularity/docker-config.json), which is unsuitable for transient CI environments. -
Security: Using
echo ${{ secrets.GITHUB_TOKEN }}can leak the secret in debug logs or error output. Useprintfwithout a trailing newline or a heredoc instead.
Recommended fix using environment variables
- name: Login and Deploy Container
if: github.event_name != 'pull_request'
+ env:
+ SINGULARITY_DOCKER_USERNAME: ${{ github.actor }}
+ SINGULARITY_DOCKER_PASSWORD: ${{ secrets.GITHUB_TOKEN }}
run: |
- echo ${{ secrets.GITHUB_TOKEN }} | singularity remote login -u ${{ github.actor }} --password-stdin oras://ghcr.io
-
# Push with version tag
singularity push ${{ matrix.context }}.sif oras://${{ env.REGISTRY }}/${{ github.repository_owner }}/${{ matrix.sif }}
# If this is a release event, also tag as latest
if [[ "${{ github.event_name }}" == "release" && -n "${{ matrix.extra_tags }}" ]]; then
SIF_LATEST=$(echo "${{ matrix.sif }}" | sed 's/:[^:]*$/:latest/')
singularity push ${{ matrix.context }}.sif oras://${{ env.REGISTRY }}/${{ github.repository_owner }}/$SIF_LATEST
fi📝 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
| - name: Login and Deploy Container | |
| if: github.event_name != 'pull_request' | |
| run: | | |
| echo ${{ secrets.GITHUB_TOKEN }} | singularity remote login -u ${{ github.actor }} --password-stdin oras://ghcr.io | |
| # Push with version tag | |
| singularity push ${{ matrix.context }}.sif oras://${{ env.REGISTRY }}/${{ github.repository_owner }}/${{ matrix.sif }} | |
| # If this is a release event, also tag as latest | |
| if [[ "${{ github.event_name }}" == "release" && -n "${{ matrix.extra_tags }}" ]]; then | |
| SIF_LATEST=$(echo "${{ matrix.sif }}" | sed 's/:[^:]*$/:latest/') | |
| singularity push image.sif oras://ghcr.io/bigbio/$SIF_LATEST | |
| singularity push ${{ matrix.context }}.sif oras://${{ env.REGISTRY }}/${{ github.repository_owner }}/$SIF_LATEST | |
| fi | |
| - name: Login and Deploy Container | |
| if: github.event_name != 'pull_request' | |
| env: | |
| SINGULARITY_DOCKER_USERNAME: ${{ github.actor }} | |
| SINGULARITY_DOCKER_PASSWORD: ${{ secrets.GITHUB_TOKEN }} | |
| run: | | |
| # Push with version tag | |
| singularity push ${{ matrix.context }}.sif oras://${{ env.REGISTRY }}/${{ github.repository_owner }}/${{ matrix.sif }} | |
| # If this is a release event, also tag as latest | |
| if [[ "${{ github.event_name }}" == "release" && -n "${{ matrix.extra_tags }}" ]]; then | |
| SIF_LATEST=$(echo "${{ matrix.sif }}" | sed 's/:[^:]*$/:latest/') | |
| singularity push ${{ matrix.context }}.sif oras://${{ env.REGISTRY }}/${{ github.repository_owner }}/$SIF_LATEST | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/quantms-containers.yml around lines 164 - 176, Replace the
`singularity remote login` + `echo ${{ secrets.GITHUB_TOKEN }}` approach in the
"Login and Deploy Container" step with environment-based authentication by
exporting SINGULARITY_DOCKER_USERNAME and SINGULARITY_DOCKER_PASSWORD from
secrets (e.g., set env: SINGULARITY_DOCKER_USERNAME: ${{
secrets.SINGULARITY_DOCKER_USERNAME }} and SINGULARITY_DOCKER_PASSWORD: ${{
secrets.SINGULARITY_DOCKER_PASSWORD }}) and remove the persistent `singularity
remote login` call; also stop using `echo` to pass secrets (if you must pass a
secret to a command, use printf '%s' or a secure heredoc) and then call
`singularity push ${{ matrix.context }}.sif oras://${{ env.REGISTRY }}/${{
github.repository_owner }}/${{ matrix.sif }}` and the existing latest-tag logic
(SIF_LATEST/Singularity push) unchanged so pushes continue to use the matrix.sif
and matrix.context variables securely.
Comment on lines
+237
to
+243
|
|
||
| echo ${{ secrets.GITHUB_TOKEN }} | singularity remote login -u ${{ github.actor }} --password-stdin oras://ghcr.io | ||
|
|
||
| singularity push image.sif oras://ghcr.io/${{ github.repository_owner }}/${{ matrix.sif }} | ||
| if [[ "${{ github.event_name }}" == "release" && -n "${{ matrix.extra_tags }}" ]]; then | ||
| SIF_LATEST=$(echo "${{ matrix.sif }}" | sed 's/:[^:]*$/:latest/') | ||
| singularity push image.sif oras://ghcr.io/bigbio/$SIF_LATEST | ||
| singularity push image.sif oras://ghcr.io/${{ github.repository_owner }}/$SIF_LATEST |
There was a problem hiding this comment.
Same Singularity authentication concerns apply here.
This step uses the same singularity remote login pattern that has reported issues. Apply the same fix using environment variables (SINGULARITY_DOCKER_USERNAME, SINGULARITY_DOCKER_PASSWORD) as suggested for the build-diann job.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/quantms-containers.yml around lines 237 - 243, Replace the
direct use of GitHub secrets in the Singularity authentication step by reading
credentials from the environment variables SINGULARITY_DOCKER_USERNAME and
SINGULARITY_DOCKER_PASSWORD; specifically, change the singularity remote login
invocation that currently uses ${{ secrets.GITHUB_TOKEN }} and ${{ github.actor
}} to use these env vars instead (keep the target URL oras://ghcr.io), so the
echo/--password-stdin pattern uses SINGULARITY_DOCKER_PASSWORD and the -u flag
uses SINGULARITY_DOCKER_USERNAME; leave the subsequent singularity push commands
(singularity push image.sif oras://ghcr.io/${{ github.repository_owner }}/${{
matrix.sif }} and the SIF_LATEST push logic using matrix.sif and SIF_LATEST)
unchanged.
Comment on lines
290
to
+297
| - name: Sync OpenMS Singularity | ||
| run: | | ||
| sudo apt-get update && sudo apt-get install -y singularity-container || true | ||
| export SINGULARITY_DOCKER_USERNAME=${{ secrets.GHCR_USERNAME }} | ||
| export SINGULARITY_DOCKER_PASSWORD=${{ secrets.GHCR_TOKEN }} | ||
| echo ${{ secrets.GITHUB_TOKEN }} | singularity remote login -u ${{ github.actor }} --password-stdin oras://ghcr.io | ||
|
|
||
| singularity pull --force openms.sif oras://ghcr.io/openms/openms-tools-thirdparty-sif:latest | ||
| singularity push openms.sif oras://ghcr.io/bigbio/openms-tools-thirdparty-sif:${{ env.OPENMS_VERSION }} | ||
| singularity push openms.sif oras://ghcr.io/${{ github.repository_owner }}/openms-tools-thirdparty-sif:${{ env.OPENMS_VERSION }} | ||
| if [[ "${{ github.event_name }}" == "release" ]]; then | ||
| singularity push openms.sif oras://ghcr.io/bigbio/openms-tools-thirdparty-sif:latest | ||
| singularity push openms.sif oras://ghcr.io/${{ github.repository_owner }}/openms-tools-thirdparty-sif:latest |
There was a problem hiding this comment.
Same Singularity authentication concerns apply here.
Apply the environment variable approach for consistency with the fix in other jobs.
🔒 Proposed fix
- name: Sync OpenMS Singularity
+ env:
+ SINGULARITY_DOCKER_USERNAME: ${{ github.actor }}
+ SINGULARITY_DOCKER_PASSWORD: ${{ secrets.GITHUB_TOKEN }}
run: |
- echo ${{ secrets.GITHUB_TOKEN }} | singularity remote login -u ${{ github.actor }} --password-stdin oras://ghcr.io
-
singularity pull --force openms.sif oras://ghcr.io/openms/openms-tools-thirdparty-sif:latest
singularity push openms.sif oras://ghcr.io/${{ github.repository_owner }}/openms-tools-thirdparty-sif:${{ env.OPENMS_VERSION }}
if [[ "${{ github.event_name }}" == "release" ]]; then
singularity push openms.sif oras://ghcr.io/${{ github.repository_owner }}/openms-tools-thirdparty-sif:latest
fi📝 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
| - name: Sync OpenMS Singularity | |
| run: | | |
| sudo apt-get update && sudo apt-get install -y singularity-container || true | |
| export SINGULARITY_DOCKER_USERNAME=${{ secrets.GHCR_USERNAME }} | |
| export SINGULARITY_DOCKER_PASSWORD=${{ secrets.GHCR_TOKEN }} | |
| echo ${{ secrets.GITHUB_TOKEN }} | singularity remote login -u ${{ github.actor }} --password-stdin oras://ghcr.io | |
| singularity pull --force openms.sif oras://ghcr.io/openms/openms-tools-thirdparty-sif:latest | |
| singularity push openms.sif oras://ghcr.io/bigbio/openms-tools-thirdparty-sif:${{ env.OPENMS_VERSION }} | |
| singularity push openms.sif oras://ghcr.io/${{ github.repository_owner }}/openms-tools-thirdparty-sif:${{ env.OPENMS_VERSION }} | |
| if [[ "${{ github.event_name }}" == "release" ]]; then | |
| singularity push openms.sif oras://ghcr.io/bigbio/openms-tools-thirdparty-sif:latest | |
| singularity push openms.sif oras://ghcr.io/${{ github.repository_owner }}/openms-tools-thirdparty-sif:latest | |
| - name: Sync OpenMS Singularity | |
| env: | |
| SINGULARITY_DOCKER_USERNAME: ${{ github.actor }} | |
| SINGULARITY_DOCKER_PASSWORD: ${{ secrets.GITHUB_TOKEN }} | |
| run: | | |
| singularity pull --force openms.sif oras://ghcr.io/openms/openms-tools-thirdparty-sif:latest | |
| singularity push openms.sif oras://ghcr.io/${{ github.repository_owner }}/openms-tools-thirdparty-sif:${{ env.OPENMS_VERSION }} | |
| if [[ "${{ github.event_name }}" == "release" ]]; then | |
| singularity push openms.sif oras://ghcr.io/${{ github.repository_owner }}/openms-tools-thirdparty-sif:latest |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/quantms-containers.yml around lines 290 - 297, Replace the
inline secrets usage in the "Sync OpenMS Singularity" step with an environment
variable like SINGULARITY_REMOTE_TOKEN (set in the job's env from
secrets.GITHUB_TOKEN), and change the login command to echo
"$SINGULARITY_REMOTE_TOKEN" | singularity remote login -u ${{ github.actor }}
--password-stdin; keep the rest of the step (singularity pull/push and
conditional push on release) unchanged so authentication is consistent with
other jobs that use the environment variable approach.
|
Here is probably why things didn't work as expected in this reppo: singularityhub/github-ci#6 |
Member
Author
|
@enryH @Skitionek everything looks OK now. Can you review the current PR for me to merge. |
enryH
reviewed
Apr 10, 2026
| with: | ||
| context: ./${{ matrix.context }} | ||
| push: ${{ (github.event.inputs.push_images == true || github.event.inputs.push_images == '') }} | ||
| push: ${{ github.event_name != 'pull_request' }} |
Collaborator
There was a problem hiding this comment.
this also means that we are not testing this atm with the PR, no?
Collaborator
it looks good because you changed the condition again. So we will only see if the steps work after merging: |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
.github/workflows/quantms-containers.yml (4)
237-243:⚠️ Potential issue | 🟠 MajorSame Singularity authentication concerns apply here.
Apply the environment variable approach (
SINGULARITY_DOCKER_USERNAME,SINGULARITY_DOCKER_PASSWORD) for consistency and security.🔒 Recommended fix
- name: Convert to Singularity and push if: github.event_name != 'pull_request' + env: + SINGULARITY_DOCKER_USERNAME: ${{ github.actor }} + SINGULARITY_DOCKER_PASSWORD: ${{ secrets.GITHUB_TOKEN }} run: | docker save ${{ matrix.tag }} -o image.tar singularity build image.sif docker-archive://image.tar - echo ${{ secrets.GITHUB_TOKEN }} | singularity remote login -u ${{ github.actor }} --password-stdin oras://ghcr.io - singularity push image.sif oras://ghcr.io/${{ github.repository_owner }}/${{ matrix.sif }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/quantms-containers.yml around lines 237 - 243, Replace the inline echo of the GitHub token and direct use of github.actor with environment-backed credentials: set env variables SINGULARITY_DOCKER_USERNAME and SINGULARITY_DOCKER_PASSWORD (mapped from repository secrets) at the job level, then call singularity remote login using -u $SINGULARITY_DOCKER_USERNAME and piping $SINGULARITY_DOCKER_PASSWORD (instead of echo ${{ secrets.GITHUB_TOKEN }} | singularity remote login -u ${{ github.actor }} --password-stdin); keep the subsequent singularity push lines intact so pushes to oras://ghcr.io/${{ github.repository_owner }}/${{ matrix.sif }} and the conditional latest push still run. Ensure the new env variable names are used consistently in the workflow and that secrets are referenced when defining those env vars.
290-297:⚠️ Potential issue | 🟠 MajorSame Singularity authentication concerns apply here.
Apply the environment variable approach for consistency.
🔒 Recommended fix
- name: Sync OpenMS Singularity + env: + SINGULARITY_DOCKER_USERNAME: ${{ github.actor }} + SINGULARITY_DOCKER_PASSWORD: ${{ secrets.GITHUB_TOKEN }} run: | - echo ${{ secrets.GITHUB_TOKEN }} | singularity remote login -u ${{ github.actor }} --password-stdin oras://ghcr.io - singularity pull --force openms.sif oras://ghcr.io/openms/openms-tools-thirdparty-sif:latest singularity push openms.sif oras://ghcr.io/${{ github.repository_owner }}/openms-tools-thirdparty-sif:${{ env.OPENMS_VERSION }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/quantms-containers.yml around lines 290 - 297, The Singularity authentication in the "Sync OpenMS Singularity" step currently echoes the GitHub token directly; change it to use the environment variable pattern used elsewhere by piping "${{ env.SINGULARITY_AUTH_TOKEN }}" into singularity remote login (keep the existing flags: -u ${{ github.actor }} --password-stdin) and ensure the job/workflow defines env.SINGULARITY_AUTH_TOKEN = ${{ secrets.SINGULARITY_AUTH_TOKEN }} (or map from the existing secret you use elsewhere) so authentication is consistent and secrets are not referenced inline; update both singularity login usages in this step and any duplicate blocks to use the same env variable.
164-176:⚠️ Potential issue | 🟠 MajorUse environment variables for Singularity authentication instead of
remote login.As previously flagged: (1)
echo ${{ secrets.GITHUB_TOKEN }}can leak secrets in debug logs; (2)singularity remote loginstores credentials to disk, which is unsuitable for ephemeral CI. UseSINGULARITY_DOCKER_USERNAMEandSINGULARITY_DOCKER_PASSWORDenvironment variables instead.🔒 Recommended fix
- name: Login and Deploy Container - if: ${{ (github.event.inputs.push_images == true || github.event.inputs.push_images == '') }} + if: ${{ github.event_name != 'pull_request' && github.event.inputs.push_images != 'false' }} + env: + SINGULARITY_DOCKER_USERNAME: ${{ github.actor }} + SINGULARITY_DOCKER_PASSWORD: ${{ secrets.GITHUB_TOKEN }} run: | - echo ${{ secrets.GITHUB_TOKEN }} | singularity remote login -u ${{ github.actor }} --password-stdin oras://ghcr.io - # Push with version tag singularity push ${{ matrix.context }}.sif oras://${{ env.REGISTRY }}/${{ github.repository_owner }}/${{ matrix.sif }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/quantms-containers.yml around lines 164 - 176, The current job uses echo ${{ secrets.GITHUB_TOKEN }} | singularity remote login which risks leaking secrets to logs and writes credentials to disk; replace that flow by exporting SINGULARITY_DOCKER_USERNAME and SINGULARITY_DOCKER_PASSWORD from GitHub secrets (do not echo them) and run singularity push directly (e.g., export SINGULARITY_DOCKER_USERNAME="${{ secrets.SINGULARITY_DOCKER_USERNAME }}" && export SINGULARITY_DOCKER_PASSWORD="${{ secrets.SINGULARITY_DOCKER_PASSWORD }}" && singularity push ${matrix.context}.sif oras://${{ env.REGISTRY }}/${{ github.repository_owner }}/${{ matrix.sif }}), and apply the same pattern for the release/tag-latest branch that pushes $SIF_LATEST; remove the singularity remote login call and ensure no secret is printed to stdout so credentials are only provided via environment variables.
136-136:⚠️ Potential issue | 🔴 CriticalPush condition will fail to push images on
pushandreleaseevents.For
push/releaseevents,github.event.inputsis undefined, causing both comparisons to fail. This means Docker images will never be pushed for these event types—only forworkflow_dispatch. Compare withbuild-relink(line 217) which correctly usesgithub.event_name != 'pull_request'.🐛 Proposed fix
- push: ${{ (github.event.inputs.push_images == true || github.event.inputs.push_images == '') }} + push: ${{ github.event_name != 'pull_request' && github.event.inputs.push_images != 'false' }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/quantms-containers.yml at line 136, The push condition uses github.event.inputs which is undefined for push/release events so images never get pushed; replace the current line starting with "push: ${{ (github.event.inputs.push_images..." with a condition that mirrors build-relink, e.g. use "push: ${{ github.event_name != 'pull_request' }}" (or otherwise check github.event_name != 'pull_request') so push/release events will allow image pushes.
🤖 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 145-162: Update the three Singularity workflow steps ("Set up
Singularity", "Save Docker image for Singularity conversion", and "Convert
Docker image to Singularity") to use the corrected push condition
github.event_name != 'pull_request' && github.event.inputs.push_images !=
'false' instead of the current broken expression; keep the same actions (using
eWaterCycle/setup-singularity@v7, docker save of matrix.tag to
matrix.context.tar, and singularity build docker-archive://matrix.context.tar to
produce matrix.context.sif) but change their if: expressions so Singularity
conversion runs on push/release and only skips when inputs.push_images is
explicitly 'false'.
---
Duplicate comments:
In @.github/workflows/quantms-containers.yml:
- Around line 237-243: Replace the inline echo of the GitHub token and direct
use of github.actor with environment-backed credentials: set env variables
SINGULARITY_DOCKER_USERNAME and SINGULARITY_DOCKER_PASSWORD (mapped from
repository secrets) at the job level, then call singularity remote login using
-u $SINGULARITY_DOCKER_USERNAME and piping $SINGULARITY_DOCKER_PASSWORD (instead
of echo ${{ secrets.GITHUB_TOKEN }} | singularity remote login -u ${{
github.actor }} --password-stdin); keep the subsequent singularity push lines
intact so pushes to oras://ghcr.io/${{ github.repository_owner }}/${{ matrix.sif
}} and the conditional latest push still run. Ensure the new env variable names
are used consistently in the workflow and that secrets are referenced when
defining those env vars.
- Around line 290-297: The Singularity authentication in the "Sync OpenMS
Singularity" step currently echoes the GitHub token directly; change it to use
the environment variable pattern used elsewhere by piping "${{
env.SINGULARITY_AUTH_TOKEN }}" into singularity remote login (keep the existing
flags: -u ${{ github.actor }} --password-stdin) and ensure the job/workflow
defines env.SINGULARITY_AUTH_TOKEN = ${{ secrets.SINGULARITY_AUTH_TOKEN }} (or
map from the existing secret you use elsewhere) so authentication is consistent
and secrets are not referenced inline; update both singularity login usages in
this step and any duplicate blocks to use the same env variable.
- Around line 164-176: The current job uses echo ${{ secrets.GITHUB_TOKEN }} |
singularity remote login which risks leaking secrets to logs and writes
credentials to disk; replace that flow by exporting SINGULARITY_DOCKER_USERNAME
and SINGULARITY_DOCKER_PASSWORD from GitHub secrets (do not echo them) and run
singularity push directly (e.g., export SINGULARITY_DOCKER_USERNAME="${{
secrets.SINGULARITY_DOCKER_USERNAME }}" && export
SINGULARITY_DOCKER_PASSWORD="${{ secrets.SINGULARITY_DOCKER_PASSWORD }}" &&
singularity push ${matrix.context}.sif oras://${{ env.REGISTRY }}/${{
github.repository_owner }}/${{ matrix.sif }}), and apply the same pattern for
the release/tag-latest branch that pushes $SIF_LATEST; remove the singularity
remote login call and ensure no secret is printed to stdout so credentials are
only provided via environment variables.
- Line 136: The push condition uses github.event.inputs which is undefined for
push/release events so images never get pushed; replace the current line
starting with "push: ${{ (github.event.inputs.push_images..." with a condition
that mirrors build-relink, e.g. use "push: ${{ github.event_name !=
'pull_request' }}" (or otherwise check github.event_name != 'pull_request') so
push/release events will allow image pushes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6dc6b58a-a311-4202-bd04-8a44c2094064
📒 Files selected for processing (1)
.github/workflows/quantms-containers.yml
Comment on lines
+145
to
+162
| - name: Set up Singularity | ||
| if: ${{ (github.event.inputs.push_images == true || github.event.inputs.push_images == '') }} | ||
| uses: eWaterCycle/setup-singularity@v7 | ||
| with: | ||
| singularity-version: 3.8.7 | ||
|
|
||
| - name: Save Docker image for Singularity conversion | ||
| if: ${{ (github.event.inputs.push_images == true || github.event.inputs.push_images == '') }} | ||
| run: | | ||
| sudo apt-get update && sudo apt-get install -y singularity-container || true | ||
| docker save ${{ matrix.tag }} -o image.tar | ||
| singularity build image.sif docker-archive://image.tar | ||
| export SINGULARITY_DOCKER_USERNAME=${{ secrets.GHCR_USERNAME }} | ||
| export SINGULARITY_DOCKER_PASSWORD=${{ secrets.GHCR_TOKEN }} | ||
| singularity push image.sif oras://ghcr.io/bigbio/${{ matrix.sif }} | ||
| docker save ${{ matrix.tag }} -o ${{ matrix.context }}.tar | ||
|
|
||
| - name: Convert Docker image to Singularity | ||
| if: ${{ (github.event.inputs.push_images == true || github.event.inputs.push_images == '') }} | ||
| run: | | ||
| # Convert Docker tar to Singularity SIF | ||
| singularity build ${{ matrix.context }}.sif docker-archive://${{ matrix.context }}.tar | ||
| # Verify the SIF file exists | ||
| ls -la ${{ matrix.context }}.sif |
There was a problem hiding this comment.
Singularity steps share the same broken push condition.
Lines 146, 152, and 157 use the same condition as line 136, which will prevent Singularity conversion on push and release events. Apply the same fix (github.event_name != 'pull_request' && github.event.inputs.push_images != 'false') to all these conditions.
The approach of using eWaterCycle/setup-singularity@v7 and the docker save → singularity build flow is sound.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/quantms-containers.yml around lines 145 - 162, Update the
three Singularity workflow steps ("Set up Singularity", "Save Docker image for
Singularity conversion", and "Convert Docker image to Singularity") to use the
corrected push condition github.event_name != 'pull_request' &&
github.event.inputs.push_images != 'false' instead of the current broken
expression; keep the same actions (using eWaterCycle/setup-singularity@v7,
docker save of matrix.tag to matrix.context.tar, and singularity build
docker-archive://matrix.context.tar to produce matrix.context.sif) but change
their if: expressions so Singularity conversion runs on push/release and only
skips when inputs.push_images is explicitly 'false'.
Collaborator
|
@ypriverol I guess it is as @Skitionek mentioned, that the setup might be changed, see singularityhub/github-ci#6 (comment) You will need to associate the ghcr packages with the repo. |
Collaborator
|
Once we know it works, I can revert the last commit then it should be ready to merge. |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
.github/workflows/quantms-containers.yml (2)
168-179:⚠️ Potential issue | 🟠 MajorUse env-based Singularity registry auth in CI instead of
remote login.The PR thread already reports failures with this exact
singularity remote login ... oras://ghcr.ioflow, and these three jobs still rely on it. SingularityCE explicitly documentsSINGULARITY_DOCKER_USERNAME/SINGULARITY_DOCKER_PASSWORDas the CI/CD-friendly path, while registry logins are stored for reuse rather than kept purely job-scoped. I’d switch these jobs back to env-based auth beforesingularity push. (docs.sylabs.io)Suggested fix pattern
- - name: Login and Deploy Container + - name: Login and Deploy Container + env: + SINGULARITY_DOCKER_USERNAME: ${{ github.actor }} + SINGULARITY_DOCKER_PASSWORD: ${{ secrets.GITHUB_TOKEN }} run: | - echo ${{ secrets.GITHUB_TOKEN }} | singularity remote login -u ${{ github.actor }} --password-stdin oras://ghcr.io - # Push with version tag singularity push ${{ matrix.context }}.sif oras://${{ env.REGISTRY }}/${{ github.repository_owner }}/${{ matrix.sif }}Also applies to: 236-247, 294-301
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/quantms-containers.yml around lines 168 - 179, Replace the ephemeral `singularity remote login ... oras://ghcr.io` step with environment-based auth by exporting SINGULARITY_DOCKER_USERNAME and SINGULARITY_DOCKER_PASSWORD from GitHub secrets before the `singularity push` calls (e.g., export SINGULARITY_DOCKER_USERNAME="${{ github.actor }}" and export SINGULARITY_DOCKER_PASSWORD="${{ secrets.GITHUB_TOKEN }}"), remove the `singularity remote login` invocation, and keep the existing `singularity push ${{ matrix.context }}.sif oras://${{ env.REGISTRY }}/${{ github.repository_owner }}/${{ matrix.sif }}` and the conditional push to the latest tag; apply the same replacement to the other two similar job blocks that use `singularity remote login` (the blocks that also run `singularity push` with `matrix.context`/`matrix.sif`).
140-169:⚠️ Potential issue | 🔴 CriticalThe publish guard is still using the wrong input context.
github.event.inputs.push_imagesis string-typed, so== truenever matches, and the== ''fallback is unsafe because GitHub coercesnullto''in expressions. Since this same condition gates both the Docker push and all Singularity publish steps here, non-workflow_dispatchevents can still satisfy the guard unexpectedly. Use the boolean-preservinginputscontext and gate it by event name instead. (docs.github.com)Suggested fix
- push: ${{ (github.event.inputs.push_images == true || github.event.inputs.push_images == '') }} + push: ${{ github.event_name != 'pull_request' && (github.event_name != 'workflow_dispatch' || inputs.push_images) }}- if: ${{ (github.event.inputs.push_images == true || github.event.inputs.push_images == '') }} + if: ${{ github.event_name != 'pull_request' && (github.event_name != 'workflow_dispatch' || inputs.push_images) }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/quantms-containers.yml around lines 140 - 169, Replace the current publish guard expressions that use github.event.inputs.push_images with a guard that first checks the event type and then uses the boolean-preserving inputs context; e.g. for the steps "Set up Singularity", "Save Docker image for Singularity conversion", "Convert Docker image to Singularity" and "Login and Deploy Container" change the if: to something like: if: ${{ github.event_name == 'workflow_dispatch' && (inputs.push_images == true || inputs.push_images == '') }} so the condition uses github.event_name and inputs.push_images (not github.event.inputs) to avoid string coercion and unexpected matches.
🤖 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 182-186: The success notice step ("Notify on success") currently
runs unconditionally on success and claims Docker and Singularity images were
pushed; update this step so it only claims pushes when the publish condition
actually ran (e.g., add the publish guard to the step's if: expression such as
combining success() with the same publish condition used for the image push
steps or a job output like needs.publish.result == 'success'), or change the
echo text to a neutral message (e.g., "Successfully built DiaNN ${{
matrix.context }} images (pushed when publish enabled)") so it doesn't assert
pushes when publish was skipped; modify the step named "Notify on success" and
its echo string accordingly.
- Around line 70-77: The DIANN_ALL matrix contains a hardcoded namespace for the
diann-2.5.0 entry: update the "tag" value in the diann-2.5.0 object (within
DIANN_ALL) to use the templated owner instead of ghcr.io/bigbio (i.e. replace
ghcr.io/bigbio with ghcr.io/${{ github.repository_owner }}), and ensure any
related fields (like "extra_tags" if intended) follow the same owner templating
so the workflow_dispatch/release runs publish under the repository_owner for
diann-2.5.0 as with the other diann entries.
- Line 196: The condition using always() allows downstream jobs to run even when
predecessors failed; change the if expressions for the build-relink job (which
currently checks always() && needs.detect-changes.outputs.has_relink == 'true'
&& github.repository_owner == 'bigbio') to instead require that the
predecessor(s) succeeded or were skipped, e.g. check needs.build-diann.result ==
'success' || needs.build-diann.result == 'skipped' (and/or
needs.detect-changes.result similarly) combined with
needs.detect-changes.outputs.has_relink == 'true' and github.repository_owner ==
'bigbio'; do the same for the sync-openms job so it only publishes when prior
jobs like build-diann and build-relink have result == 'success' or 'skipped'
rather than using always().
---
Duplicate comments:
In @.github/workflows/quantms-containers.yml:
- Around line 168-179: Replace the ephemeral `singularity remote login ...
oras://ghcr.io` step with environment-based auth by exporting
SINGULARITY_DOCKER_USERNAME and SINGULARITY_DOCKER_PASSWORD from GitHub secrets
before the `singularity push` calls (e.g., export
SINGULARITY_DOCKER_USERNAME="${{ github.actor }}" and export
SINGULARITY_DOCKER_PASSWORD="${{ secrets.GITHUB_TOKEN }}"), remove the
`singularity remote login` invocation, and keep the existing `singularity push
${{ matrix.context }}.sif oras://${{ env.REGISTRY }}/${{ github.repository_owner
}}/${{ matrix.sif }}` and the conditional push to the latest tag; apply the same
replacement to the other two similar job blocks that use `singularity remote
login` (the blocks that also run `singularity push` with
`matrix.context`/`matrix.sif`).
- Around line 140-169: Replace the current publish guard expressions that use
github.event.inputs.push_images with a guard that first checks the event type
and then uses the boolean-preserving inputs context; e.g. for the steps "Set up
Singularity", "Save Docker image for Singularity conversion", "Convert Docker
image to Singularity" and "Login and Deploy Container" change the if: to
something like: if: ${{ github.event_name == 'workflow_dispatch' &&
(inputs.push_images == true || inputs.push_images == '') }} so the condition
uses github.event_name and inputs.push_images (not github.event.inputs) to avoid
string coercion and unexpected matches.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ee43c3af-9f65-4dfe-811c-efe27f062bbc
📒 Files selected for processing (1)
.github/workflows/quantms-containers.yml
Comment on lines
70
to
+77
| DIANN_ALL='[ | ||
| {"context":"diann-2.5.0","tag":"ghcr.io/bigbio/diann:2.5.0","sif":"diann-sif:2.5.0","extra_tags":"ghcr.io/bigbio/diann:latest","chg":"CHG_250"}, | ||
| {"context":"diann-2.3.2","tag":"ghcr.io/bigbio/diann:2.3.2","sif":"diann-sif:2.3.2","extra_tags":"","chg":"CHG_232"}, | ||
| {"context":"diann-2.2.0","tag":"ghcr.io/bigbio/diann:2.2.0","sif":"diann-sif:2.2.0","extra_tags":"","chg":"CHG_220"}, | ||
| {"context":"diann-2.1.0","tag":"ghcr.io/bigbio/diann:2.1.0","sif":"diann-sif:2.1.0","extra_tags":"","chg":"CHG_210"}, | ||
| {"context":"diann-2.0.2","tag":"ghcr.io/bigbio/diann:2.0.2","sif":"diann-sif:2.0.2","extra_tags":"","chg":"CHG_20"}, | ||
| {"context":"diann-1.9.2","tag":"ghcr.io/bigbio/diann:1.9.2","sif":"diann-sif:1.9.2","extra_tags":"","chg":"CHG_192"}, | ||
| {"context":"diann-1.8.1","tag":"ghcr.io/bigbio/diann:1.8.1","sif":"diann-sif:1.8.1","extra_tags":"","chg":"CHG_181"} | ||
| {"context":"diann-2.3.2","tag":"ghcr.io/${{ github.repository_owner }}/diann:2.3.2","sif":"diann-sif:2.3.2","extra_tags":"","chg":"CHG_232"}, | ||
| {"context":"diann-2.2.0","tag":"ghcr.io/${{ github.repository_owner }}/diann:2.2.0","sif":"diann-sif:2.2.0","extra_tags":"","chg":"CHG_220"}, | ||
| {"context":"diann-2.1.0","tag":"ghcr.io/${{ github.repository_owner }}/diann:2.1.0","sif":"diann-sif:2.1.0","extra_tags":"","chg":"CHG_210"}, | ||
| {"context":"diann-2.0.2","tag":"ghcr.io/${{ github.repository_owner }}/diann:2.0.2","sif":"diann-sif:2.0.2","extra_tags":"","chg":"CHG_20"}, | ||
| {"context":"diann-1.9.2","tag":"ghcr.io/${{ github.repository_owner }}/diann:1.9.2","sif":"diann-sif:1.9.2","extra_tags":"","chg":"CHG_192"}, | ||
| {"context":"diann-1.8.1","tag":"ghcr.io/${{ github.repository_owner }}/diann:1.8.1","sif":"diann-sif:1.8.1","extra_tags":"","chg":"CHG_181"} |
There was a problem hiding this comment.
diann-2.5.0 is still pinned to ghcr.io/bigbio.
This block updates the later DIA-NN entries to ${{ github.repository_owner }}, but leaves the 2.5.0 matrix entry on the hardcoded namespace. release / workflow_dispatch will still publish that one image to bigbio, so forks and org mirrors remain broken for the newest image.
Suggested fix
- {"context":"diann-2.5.0","tag":"ghcr.io/bigbio/diann:2.5.0","sif":"diann-sif:2.5.0","extra_tags":"ghcr.io/bigbio/diann:latest","chg":"CHG_250"},
+ {"context":"diann-2.5.0","tag":"ghcr.io/${{ github.repository_owner }}/diann:2.5.0","sif":"diann-sif:2.5.0","extra_tags":"ghcr.io/${{ github.repository_owner }}/diann:latest","chg":"CHG_250"},📝 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
| DIANN_ALL='[ | |
| {"context":"diann-2.5.0","tag":"ghcr.io/bigbio/diann:2.5.0","sif":"diann-sif:2.5.0","extra_tags":"ghcr.io/bigbio/diann:latest","chg":"CHG_250"}, | |
| {"context":"diann-2.3.2","tag":"ghcr.io/bigbio/diann:2.3.2","sif":"diann-sif:2.3.2","extra_tags":"","chg":"CHG_232"}, | |
| {"context":"diann-2.2.0","tag":"ghcr.io/bigbio/diann:2.2.0","sif":"diann-sif:2.2.0","extra_tags":"","chg":"CHG_220"}, | |
| {"context":"diann-2.1.0","tag":"ghcr.io/bigbio/diann:2.1.0","sif":"diann-sif:2.1.0","extra_tags":"","chg":"CHG_210"}, | |
| {"context":"diann-2.0.2","tag":"ghcr.io/bigbio/diann:2.0.2","sif":"diann-sif:2.0.2","extra_tags":"","chg":"CHG_20"}, | |
| {"context":"diann-1.9.2","tag":"ghcr.io/bigbio/diann:1.9.2","sif":"diann-sif:1.9.2","extra_tags":"","chg":"CHG_192"}, | |
| {"context":"diann-1.8.1","tag":"ghcr.io/bigbio/diann:1.8.1","sif":"diann-sif:1.8.1","extra_tags":"","chg":"CHG_181"} | |
| {"context":"diann-2.3.2","tag":"ghcr.io/${{ github.repository_owner }}/diann:2.3.2","sif":"diann-sif:2.3.2","extra_tags":"","chg":"CHG_232"}, | |
| {"context":"diann-2.2.0","tag":"ghcr.io/${{ github.repository_owner }}/diann:2.2.0","sif":"diann-sif:2.2.0","extra_tags":"","chg":"CHG_220"}, | |
| {"context":"diann-2.1.0","tag":"ghcr.io/${{ github.repository_owner }}/diann:2.1.0","sif":"diann-sif:2.1.0","extra_tags":"","chg":"CHG_210"}, | |
| {"context":"diann-2.0.2","tag":"ghcr.io/${{ github.repository_owner }}/diann:2.0.2","sif":"diann-sif:2.0.2","extra_tags":"","chg":"CHG_20"}, | |
| {"context":"diann-1.9.2","tag":"ghcr.io/${{ github.repository_owner }}/diann:1.9.2","sif":"diann-sif:1.9.2","extra_tags":"","chg":"CHG_192"}, | |
| {"context":"diann-1.8.1","tag":"ghcr.io/${{ github.repository_owner }}/diann:1.8.1","sif":"diann-sif:1.8.1","extra_tags":"","chg":"CHG_181"} | |
| DIANN_ALL='[ | |
| {"context":"diann-2.5.0","tag":"ghcr.io/${{ github.repository_owner }}/diann:2.5.0","sif":"diann-sif:2.5.0","extra_tags":"ghcr.io/${{ github.repository_owner }}/diann:latest","chg":"CHG_250"}, | |
| {"context":"diann-2.3.2","tag":"ghcr.io/${{ github.repository_owner }}/diann:2.3.2","sif":"diann-sif:2.3.2","extra_tags":"","chg":"CHG_232"}, | |
| {"context":"diann-2.2.0","tag":"ghcr.io/${{ github.repository_owner }}/diann:2.2.0","sif":"diann-sif:2.2.0","extra_tags":"","chg":"CHG_220"}, | |
| {"context":"diann-2.1.0","tag":"ghcr.io/${{ github.repository_owner }}/diann:2.1.0","sif":"diann-sif:2.1.0","extra_tags":"","chg":"CHG_210"}, | |
| {"context":"diann-2.0.2","tag":"ghcr.io/${{ github.repository_owner }}/diann:2.0.2","sif":"diann-sif:2.0.2","extra_tags":"","chg":"CHG_20"}, | |
| {"context":"diann-1.9.2","tag":"ghcr.io/${{ github.repository_owner }}/diann:1.9.2","sif":"diann-sif:1.9.2","extra_tags":"","chg":"CHG_192"}, | |
| {"context":"diann-1.8.1","tag":"ghcr.io/${{ github.repository_owner }}/diann:1.8.1","sif":"diann-sif:1.8.1","extra_tags":"","chg":"CHG_181"} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/quantms-containers.yml around lines 70 - 77, The DIANN_ALL
matrix contains a hardcoded namespace for the diann-2.5.0 entry: update the
"tag" value in the diann-2.5.0 object (within DIANN_ALL) to use the templated
owner instead of ghcr.io/bigbio (i.e. replace ghcr.io/bigbio with ghcr.io/${{
github.repository_owner }}), and ensure any related fields (like "extra_tags" if
intended) follow the same owner templating so the workflow_dispatch/release runs
publish under the repository_owner for diann-2.5.0 as with the other diann
entries.
There was a problem hiding this comment.
@enryH, thanks for confirming! Here's the specific change needed for the diann-2.5.0 entry:
- {"context":"diann-2.5.0","tag":"ghcr.io/bigbio/diann:2.5.0","sif":"diann-sif:2.5.0","extra_tags":"ghcr.io/bigbio/diann:latest","chg":"CHG_250"},
+ {"context":"diann-2.5.0","tag":"ghcr.io/${{ github.repository_owner }}/diann:2.5.0","sif":"diann-sif:2.5.0","extra_tags":"ghcr.io/${{ github.repository_owner }}/diann:latest","chg":"CHG_250"},Both the tag and extra_tags fields need updating so the 2.5.0 (latest) image is consistent with all the other DIA-NN entries.
Comment on lines
+182
to
+186
| - name: Notify on success | ||
| if: success() | ||
| run: | | ||
| echo "::notice::Successfully built and pushed DiaNN ${{ matrix.context }} Docker and Singularity images to ghcr.io/${{ github.repository_owner }}" | ||
|
|
There was a problem hiding this comment.
Success notification is inaccurate on build-only runs.
On pull_request runs—or on workflow_dispatch when push_images is false—the publish steps can be skipped, but this notice still says both Docker and Singularity images were pushed. Gate the notice with the publish condition or change the wording to reflect build-only success.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/quantms-containers.yml around lines 182 - 186, The success
notice step ("Notify on success") currently runs unconditionally on success and
claims Docker and Singularity images were pushed; update this step so it only
claims pushes when the publish condition actually ran (e.g., add the publish
guard to the step's if: expression such as combining success() with the same
publish condition used for the image push steps or a job output like
needs.publish.result == 'success'), or change the echo text to a neutral message
(e.g., "Successfully built DiaNN ${{ matrix.context }} images (pushed when
publish enabled)") so it doesn't assert pushes when publish was skipped; modify
the step named "Notify on success" and its echo string accordingly.
| name: "Relink ${{ matrix.context }}" | ||
| needs: [detect-changes, build-diann] | ||
| if: always() && needs.detect-changes.outputs.has_relink == 'true' | ||
| if: always() && needs.detect-changes.outputs.has_relink == 'true' && github.repository_owner == 'bigbio' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd .github/workflows && wc -l quantms-containers.ymlRepository: bigbio/quantms-containers
Length of output: 95
🏁 Script executed:
cat -n .github/workflows/quantms-containers.yml | sed -n '180,210p'Repository: bigbio/quantms-containers
Length of output: 1361
🏁 Script executed:
cat -n .github/workflows/quantms-containers.yml | sed -n '240,270p'Repository: bigbio/quantms-containers
Length of output: 1576
🏁 Script executed:
cat -n .github/workflows/quantms-containers.yml | sed -n '1,50p'Repository: bigbio/quantms-containers
Length of output: 1865
🏁 Script executed:
rg -n "needs\." .github/workflows/quantms-containers.yml | head -20Repository: bigbio/quantms-containers
Length of output: 397
🏁 Script executed:
cat -n .github/workflows/quantms-containers.yml | sed -n '40,50p'Repository: bigbio/quantms-containers
Length of output: 573
🏁 Script executed:
cat -n .github/workflows/quantms-containers.yml | sed -n '110,130p'Repository: bigbio/quantms-containers
Length of output: 846
🏁 Script executed:
cat -n .github/workflows/quantms-containers.yml | sed -n '192,200p'Repository: bigbio/quantms-containers
Length of output: 501
🏁 Script executed:
cat -n .github/workflows/quantms-containers.yml | sed -n '250,260p'Repository: bigbio/quantms-containers
Length of output: 575
always() allows downstream jobs to publish even after upstream failures.
GitHub Actions treats always() as "run even if a needed job did not succeed." In this workflow:
build-relink(line 196) can push images even ifbuild-diannfailssync-openms(line 254) can publish even ifbuild-diannorbuild-relinkfails
If the intent is only to tolerate skipped predecessors, key the condition off needs.<job>.result being success or skipped instead.
Suggested fix
- if: always() && needs.detect-changes.outputs.has_relink == 'true' && github.repository_owner == 'bigbio'
+ if: needs.detect-changes.outputs.has_relink == 'true'
+ && github.repository_owner == 'bigbio'
+ && contains(fromJSON('["success","skipped"]'), needs.build-diann.result)- if: always() && (github.event_name == 'release' || github.event_name == 'workflow_dispatch') && github.repository_owner == 'bigbio'
+ if: (github.event_name == 'release' || github.event_name == 'workflow_dispatch')
+ && github.repository_owner == 'bigbio'
+ && contains(fromJSON('["success","skipped"]'), needs.build-diann.result)
+ && contains(fromJSON('["success","skipped"]'), needs.build-relink.result)Also applies to: 254-254
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/quantms-containers.yml at line 196, The condition using
always() allows downstream jobs to run even when predecessors failed; change the
if expressions for the build-relink job (which currently checks always() &&
needs.detect-changes.outputs.has_relink == 'true' && github.repository_owner ==
'bigbio') to instead require that the predecessor(s) succeeded or were skipped,
e.g. check needs.build-diann.result == 'success' || needs.build-diann.result ==
'skipped' (and/or needs.detect-changes.result similarly) combined with
needs.detect-changes.outputs.has_relink == 'true' and github.repository_owner ==
'bigbio'; do the same for the sync-openms job so it only publishes when prior
jobs like build-diann and build-relink have result == 'success' or 'skipped'
rather than using always().
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
4 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.
This pull request updates the GitHub Actions workflow for building and publishing Docker and Singularity images, focusing on improved maintainability, flexibility, and security. The main changes include parameterizing image names and registry details, updating authentication methods, refining job conditions, and enhancing the build and notification steps.
Container Image Parameterization and Flexibility:
REGISTRY,IMAGE_NAME,SINGULARITY_IMAGE_NAME) to make image naming and registry usage more flexible and reusable across jobs.${{ github.repository_owner }}instead of hardcoded values, enabling workflows to work for forks and other organizations.Build and Push Improvements:
build-diannjob with improved naming, retry logic, metadata extraction, and tagging strategies (including date-based tags). [1] [2]Authentication and Security Updates:
${{ github.repository_owner }}as the username and${{ secrets.GITHUB_TOKEN }}for password, replacing previous usage of${{ github.actor }}and custom tokens. This improves security and consistency across jobs. [1] [2] [3] [4]Conditional Execution and Permissions:
build-relink,sync-openms) to only run for thebigbioorganization, preventing accidental pushes from forks or other users. [1] [2]Workflow Usability and Clarity:
These changes collectively make the workflow more robust, secure, and easier to maintain.
Summary by CodeRabbit