Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions .github/workflows/docker.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@ jobs:
username: ${{ vars.DOCKERHUB_USERNAME }}
password: ${{ secrets.DOCKERHUB_TOKEN }}

- name: Set up QEMU
uses: docker/setup-qemu-action@v3

- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v3

Expand All @@ -38,6 +35,8 @@ jobs:
uses: docker/build-push-action@v6
with:
context: .
file: .github/Dockerfile
file: dockerfile
push: ${{ github.ref == 'refs/heads/main' }}
tags: ${{ steps.tags.outputs.tags }}
cache-from: type=gha
cache-to: type=gha,mode=max
37 changes: 37 additions & 0 deletions dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
FROM python:3.10-slim AS stage1

FROM mambaorg/micromamba:1.5.5 AS stage2

FROM debian:stable-slim AS final
Comment on lines +1 to +5
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

debian:stable-slim is a mutable/floating tag — pin to a specific version for reproducibility.

stable currently resolves to Debian 12 (bookworm) but will silently change with the next Debian stable release, breaking reproducible builds.

🔧 Suggested fix
-FROM debian:stable-slim AS final
+FROM debian:12-slim AS final
📝 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
FROM python:3.10-slim AS stage1
FROM mambaorg/micromamba:1.5.5 AS stage2
FROM debian:stable-slim AS final
FROM python:3.10-slim AS stage1
FROM mambaorg/micromamba:1.5.5 AS stage2
FROM debian:12-slim AS final
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@dockerfile` around lines 1 - 5, The Dockerfile uses the mutable tag
"debian:stable-slim" for the final stage (the line with FROM debian:stable-slim
AS final); replace that mutable tag with a pinned, reproducible reference
(either a specific Debian version tag such as debian:12-slim or a full image
digest) so builds remain deterministic—update the FROM debian:stable-slim AS
final line accordingly to the chosen pinned tag or digest.


RUN apt-get update && apt-get install -y ca-certificates
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

apt-get layer is missing --no-install-recommends and cache cleanup, bloating the image.

Without --no-install-recommends, extra recommended packages are pulled in. Without removing the apt lists, the package index is permanently baked into the layer.

🔧 Suggested fix
-RUN apt-get update && apt-get install -y ca-certificates
+RUN apt-get update && apt-get install -y --no-install-recommends ca-certificates \
+    && rm -rf /var/lib/apt/lists/*
📝 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
RUN apt-get update && apt-get install -y ca-certificates
RUN apt-get update && apt-get install -y --no-install-recommends ca-certificates \
&& rm -rf /var/lib/apt/lists/*
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@dockerfile` at line 7, Update the RUN instruction that currently executes
"apt-get update && apt-get install -y ca-certificates" to use
"--no-install-recommends" and remove apt caches in the same layer; change the
RUN line (the Dockerfile RUN that calls apt-get update/install ca-certificates)
to run apt-get install with --no-install-recommends and then delete
/var/lib/apt/lists/* (and optionally run apt-get clean) in the same command so
index files are not baked into the image.


COPY --from=stage1 /usr/local /usr/local
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

COPY --from=stage1 /usr/local /usr/local is redundant and bloats the image.

stage1 (python:3.10-slim) is brought in solely to copy its entire /usr/local tree (~100 MB+) into the final image. All subsequent Python/pip usage goes through the micromamba-managed mdx2-dev environment at its own prefix — the stage1 Python installation is never used. Remove stage1 entirely and drop this COPY.

🔧 Suggested fix
-FROM python:3.10-slim AS stage1
-
 FROM mambaorg/micromamba:1.5.5 AS stage2

 FROM debian:12-slim AS final
 ...
-COPY --from=stage1 /usr/local /usr/local
 COPY --from=stage2 /bin/micromamba /usr/local/bin/micromamba
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@dockerfile` at line 9, Remove the unused build stage and the COPY that
imports its /usr/local into the final image: delete the "stage1" FROM
(python:3.10-slim) stage and remove the line `COPY --from=stage1 /usr/local
/usr/local`; then verify there are no remaining references to "stage1" elsewhere
in the Dockerfile and that the final image relies solely on the
micromamba-managed mdx2-dev environment for Python/pip usage.

COPY --from=stage2 /bin/micromamba /usr/local/bin/micromamba

ENV MAMBA_ROOT_PREFIX="/root/micromamba" \
CONDA_PREFIX="/root/micromamba/envs/mdx2-dev" \
PATH="/root/micromamba/envs/mdx2-dev/bin:/usr/local/bin:/opt/conda/bin:$PATH"

# Ensure /opt/conda exists for micromamba to use
RUN mkdir -p /opt/conda

WORKDIR /home/dev

COPY env.yaml .

# Use micromamba to create the environment and install packages
RUN --mount=type=cache,target=/root/micromamba/pkgs \
/usr/local/bin/micromamba create -f env.yaml -n mdx2-dev && \
/usr/local/bin/micromamba install -y -n mdx2-dev nexpy jupyterlab jupyterlab-h5web dials xia2 wget tar -c conda-forge

COPY . .
Comment thread
coderabbitai[bot] marked this conversation as resolved.

RUN --mount=type=cache,target=/root/.cache/pip \
/usr/local/bin/micromamba run -n mdx2-dev pip install -e . && \
/usr/local/bin/micromamba run -n mdx2-dev pip install jupyterhub jupyter-vscode-proxy

EXPOSE 8888

CMD ["/usr/local/bin/micromamba", "run", "-n", "mdx2-dev", "jupyterhub-singleuser"]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Container runs as root and micromamba run is not a proper PID 1 — signals won't propagate correctly.

Two concerns:

  1. No USER directive — the jupyterhub-singleuser process runs as root, which is a security risk in a multi-user JupyterHub deployment.
  2. micromamba run is a wrapper process; it doesn't forward SIGTERM/SIGINT to its child. Use exec or a proper init (e.g., tini) so the Jupyter process receives container lifecycle signals correctly.
🔧 Suggested fix
+RUN useradd -m dev
+USER dev

 EXPOSE 8888

-CMD ["/usr/local/bin/micromamba", "run", "-n", "mdx2-dev", "jupyterhub-singleuser"]
+CMD ["/usr/local/bin/micromamba", "run", "-n", "mdx2-dev", "--", \
+     "python", "-m", "jupyterhub_singleuser"]

Alternatively, install tini and use it as the entrypoint:

RUN apt-get install -y --no-install-recommends tini
ENTRYPOINT ["/usr/bin/tini", "--"]
CMD ["/usr/local/bin/micromamba", "run", "-n", "mdx2-dev", "jupyterhub-singleuser"]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@dockerfile` at line 34, The Dockerfile currently runs CMD [".../micromamba",
"run", "-n", "mdx2-dev", "jupyterhub-singleuser"] as root and via a wrapper that
won't receive/forward signals; change the image to create/use a non-root user
(add a USER directive and ensure filesystem perms for that user) so
jupyterhub-singleuser does not run as root, and install/use a proper init as PID
1 (e.g., install tini and set ENTRYPOINT to tini -- so tini is PID 1 and
forwards signals) while leaving the micromamba run command as CMD so the
jupyterhub-singleuser process is started under the non-root user and receives
container signals correctly.


Loading