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
200 changes: 200 additions & 0 deletions .github/workflows/docker_build_xnat_dcm2niix.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,200 @@
# Copyright (c) Guy's and St Thomas' NHS Foundation Trust & King's College London
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
# http://www.apache.org/licenses/LICENSE-2.0
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

name: Build and Push Docker Image for XNAT dcm2niix

on:
Comment thread
atriaybagur marked this conversation as resolved.
workflow_dispatch:
push:
branches: [main, develop]
# Narrower than the other xnat workflows' trust/xnat/** filter: this image
Comment thread
atriaybagur marked this conversation as resolved.
# shares nothing with the xnat-web/db/nginx build contexts, so only its own
# directory should trigger a republish.
paths:
- "trust/xnat/dcm2niix/**"
- ".github/workflows/docker_build_xnat_dcm2niix.yml"
# PR runs exercise the Dockerfile — the pinned dcm2niix download, its sha256
# check and the smoke test baked into the build — so a bad pin fails in the PR
# rather than after merge. The login/push/visibility steps are gated below, so
# nothing is published from a pull request (same pattern as docker_build_orthanc).
pull_request:
paths:
- "trust/xnat/dcm2niix/**"
- ".github/workflows/docker_build_xnat_dcm2niix.yml"

permissions:
contents: read

jobs:
build-and-push:
# Skip on forks: they cannot push to ghcr.io/londonaicentre, so this whole
# build-and-publish job would otherwise show a spurious CI failure on fork
# pushes. Upstream main/develop publishing is unchanged.
if: github.repository == 'londonaicentre/FLIP'
runs-on: ubuntu-latest
permissions: # override top-level read-only default to allow GHCR push
contents: read
packages: write
defaults:
run:
working-directory: ./trust/xnat/dcm2niix
env:
REGISTRY: ghcr.io
IMAGE_NAME: londonaicentre/xnat-dcm2niix
Comment thread
atriaybagur marked this conversation as resolved.
steps:
- name: Checkout code
uses: actions/checkout@v5

- name: Determine tags
id: tags
env:
GH_REF_NAME: ${{ github.ref_name }}
GH_EVENT_NAME: ${{ github.event_name }}
GH_REF: ${{ github.ref }}
GH_SHA: ${{ github.sha }}
run: |
TAGS="${REGISTRY}/${IMAGE_NAME}:${{ github.sha }}"

# An empty sha would silently publish a mutable literal `sha-` tag — refuse.
[[ -n "$GH_SHA" ]] || { echo "::error::empty commit SHA — cannot compute the sha-<short7> tag"; exit 1; }

# Immutable short-SHA tag (FLIP#751), pushed uniformly on every publish.
# Length 7 must match the tag resolution in deploy/providers/AWS/Makefile.
TAGS="${TAGS},${REGISTRY}/${IMAGE_NAME}:sha-${GH_SHA:0:7}"

# The tool-version tag is the one every deploy config references
# (dcm2niix_command.json, the K8s init-job copy, imaging-api's
# Settings.DCM2NIIX_IMAGE). Read it from the Dockerfile so the pin has a
# single source of truth; refuse to publish if it can't be parsed.
DCM2NIIX_VERSION=$(sed -n 's/^ARG DCM2NIIX_VERSION=//p' Dockerfile)
[[ -n "$DCM2NIIX_VERSION" ]] || { echo "::error::could not read ARG DCM2NIIX_VERSION from Dockerfile"; exit 1; }
TAGS="${TAGS},${REGISTRY}/${IMAGE_NAME}:${DCM2NIIX_VERSION}"

# Branch name sanitization
SAFE_REF_NAME=$(echo "$GH_REF_NAME" | sed 's/[^a-zA-Z0-9]/-/g')
TAGS="${TAGS},${REGISTRY}/${IMAGE_NAME}:${SAFE_REF_NAME}"

# Branch number (if starts with number)
if [[ "$GH_REF_NAME" =~ ^[0-9]+ ]]; then
BRANCH_NUM=$(echo "$GH_REF_NAME" | grep -oE '^[0-9]+')
TAGS="${TAGS},${REGISTRY}/${IMAGE_NAME}:${BRANCH_NUM}"
fi

# PR Number (if PR)
Comment thread
atriaybagur marked this conversation as resolved.
if [[ "$GH_EVENT_NAME" == "pull_request" ]]; then
PR_NUMBER=$(echo "$GH_REF" | awk -F / '{print $3}')
TAGS="${TAGS},${REGISTRY}/${IMAGE_NAME}:pr-${PR_NUMBER}"
fi

# Determine if this is a merge/push to main or develop. Unlike the
# workflow_run-gated application workflows, this one is triggered
# directly, so github.ref_name is the branch already.
BRANCH_NAME=""
if [[ "$GH_EVENT_NAME" == "push" ]]; then
BRANCH_NAME="$GH_REF_NAME"
fi

if [[ "$BRANCH_NAME" == "main" ]]; then
TAGS="${TAGS},${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:prod"
elif [[ "$BRANCH_NAME" == "develop" ]]; then
TAGS="${TAGS},${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:stag"
fi
echo "tags=${TAGS}" >> $GITHUB_OUTPUT
echo "version=${DCM2NIIX_VERSION}" >> $GITHUB_OUTPUT
echo "Generated tags: ${TAGS}"

- name: Log in to GitHub Container Registry
if: github.event_name != 'pull_request'
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GH_ACTOR: ${{ github.actor }}
run: echo "$GH_TOKEN" | docker login $REGISTRY -u "$GH_ACTOR" --password-stdin

- name: Build Docker image
env:
DOCKER_TAGS: ${{ steps.tags.outputs.tags }}
run: |
IFS=',' read -ra TAG_ARRAY <<< "$DOCKER_TAGS"
TAG_FLAGS=()
for tag in "${TAG_ARRAY[@]}"; do
TAG_FLAGS+=("-t" "$tag")
done
docker build "${TAG_FLAGS[@]}" .

- name: Push Docker image
if: github.event_name != 'pull_request'
env:
DOCKER_TAGS: ${{ steps.tags.outputs.tags }}
run: |
IFS=',' read -ra TAG_ARRAY <<< "$DOCKER_TAGS"
for tag in "${TAG_ARRAY[@]}"; do
docker push "$tag"
done

# The XNAT Container Service pulls this image with no credentials, so a
# private package is registrable but unpullable — conversion then fails
# only at runtime, at a trust. New org packages default to private and
# GITHUB_TOKEN cannot flip that, so the flip is an operator action
# (trust/xnat/README.md). This probe is what makes forgetting it loud:
# it repeats the anonymous pull the Container Service performs, and the
# run goes red until the package is public. Deliberately token-less —
# the docker login above is not reused, and the ghcr.io token endpoint
# is called with no credentials, so the check sees exactly what an
# unauthenticated trust sees.
- name: Verify the published image is anonymously pullable
if: github.event_name != 'pull_request'
env:
DCM2NIIX_VERSION: ${{ steps.tags.outputs.version }}
run: |
MANIFEST_URL="https://${REGISTRY}/v2/${IMAGE_NAME}/manifests/${DCM2NIIX_VERSION}"
TOKEN_URL="https://${REGISTRY}/token?scope=repository:${IMAGE_NAME}:pull&service=${REGISTRY}"
TOKEN_BODY=$(mktemp)
FAILURE=""

# GHCR wants a bearer token even for anonymous pulls, and it refuses to mint one at all
# (403) for a package that is not public — so both requests are failure points and
# neither may use `curl -f`, which would abort the step with a bare exit 22 instead of
# the operator instructions below. `-w %{http_code}` keeps a refusal a value, not a
# crash; a genuine network error still fails the step through curl's own exit status.
TOKEN_CODE=$(curl -sS -o "$TOKEN_BODY" -w '%{http_code}' "$TOKEN_URL")
if [[ "$TOKEN_CODE" != "200" ]]; then
FAILURE="anonymous pull token refused (HTTP ${TOKEN_CODE})"
else
ANON_TOKEN=$(jq -r '.token // empty' "$TOKEN_BODY")
if [[ -z "$ANON_TOKEN" ]]; then
FAILURE="the ${REGISTRY} token endpoint returned no token"
else
HTTP_CODE=$(curl -sS -o /dev/null -w '%{http_code}' \
-H "Authorization: Bearer ${ANON_TOKEN}" \
-H "Accept: application/vnd.oci.image.index.v1+json" \
-H "Accept: application/vnd.oci.image.manifest.v1+json" \
-H "Accept: application/vnd.docker.distribution.manifest.list.v2+json" \
-H "Accept: application/vnd.docker.distribution.manifest.v2+json" \
"$MANIFEST_URL")
if [[ "$HTTP_CODE" != "200" ]]; then
FAILURE="anonymous GET of the manifest returned HTTP ${HTTP_CODE}, expected 200"
fi
fi
fi
rm -f "$TOKEN_BODY"

if [[ -n "$FAILURE" ]]; then
# The push above just succeeded, so the package exists: a refusal here means private.
echo "Probing ${MANIFEST_URL} without credentials: ${FAILURE}."
echo "The XNAT Container Service pulls this image with no credentials, so while the"
echo "package is private every trust's DICOM->NIfTI conversion fails at runtime."
echo "Fix: github.com/orgs/londonaicentre/packages/container/xnat-dcm2niix/settings"
echo " -> Danger Zone -> Change visibility -> Public (see trust/xnat/README.md)."
echo "::error::${IMAGE_NAME}:${DCM2NIIX_VERSION} is not public: flip the package visibility"
exit 1
fi
echo "${IMAGE_NAME}:${DCM2NIIX_VERSION} is anonymously pullable."
21 changes: 21 additions & 0 deletions .github/workflows/test_trust_xnat.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ name: Trust - XNAT CI
# guard parity tests read their inputs from outside trust/xnat/tests/, so every
# file they assert on has to trigger this workflow — otherwise a change to a
# guard (or to the credential minter) lands with the test that pins it unrun.
# The dcm2niix-pin-sync job below likewise reads four files scattered across the
# repo, so each of those triggers the workflow too.
on:
push:
branches: [main, develop]
Expand All @@ -34,6 +36,9 @@ on:
- "trust/xnat/postgres/guard-xnat-db-passwords.sh"
- "trust/imaging-api/imaging_api/config.py"
- "flip-api/src/flip_api/scripts/generate_xnat_credentials.py"
- "trust/xnat/dcm2niix/**"
- "trust/xnat/xnat/config/dcm2niix_command.json"
- "deploy/providers/kubernetes/templates/xnat-init-job.yaml"
- ".github/workflows/test_trust_xnat.yml"
pull_request:
branches: [main, develop]
Expand All @@ -52,6 +57,9 @@ on:
- "trust/xnat/postgres/guard-xnat-db-passwords.sh"
- "trust/imaging-api/imaging_api/config.py"
- "flip-api/src/flip_api/scripts/generate_xnat_credentials.py"
- "trust/xnat/dcm2niix/**"
- "trust/xnat/xnat/config/dcm2niix_command.json"
- "deploy/providers/kubernetes/templates/xnat-init-job.yaml"
- ".github/workflows/test_trust_xnat.yml"

permissions:
Expand All @@ -78,3 +86,16 @@ jobs:

- name: Run XNAT tests
run: make unit_test

# Backstop for the xnat-dcm2niix-pin-sync pre-commit hook, for commits made without pre-commit
# installed. Kept a separate job (no Python, no uv) because it is a two-second grep and must stay
# cheap enough to run on any of the four pin sites changing.
dcm2niix-pin-sync:
runs-on: ubuntu-latest

steps:
- name: Checkout code
uses: actions/checkout@v5

- name: Check the dcm2niix image pin is in sync
run: ./trust/xnat/dcm2niix/check_image_pin_sync.sh
14 changes: 14 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,20 @@ repos:
files: '^fl-apps/(check_required_files\.sh|.*required_files\.json)$'
always_run: false

# The dcm2niix Container Service image is pinned by `ARG DCM2NIIX_VERSION` in its Dockerfile
# and referenced as a literal `<image>:<tag>` string by three deploy configs. A version bump
# that misses one of them is silent at runtime (the old immutable tag keeps being pulled), so
# verify them here. Unlike fl-apps-required-files this only reports — the four sites are
# hand-written, not generated. The dcm2niix-pin-sync job in test_trust_xnat.yml re-runs the
# same script as the backstop for commits made without pre-commit installed. (#980)
- id: xnat-dcm2niix-pin-sync
name: Check the dcm2niix image pin is in sync
entry: bash trust/xnat/dcm2niix/check_image_pin_sync.sh
language: system
pass_filenames: false
files: '^(trust/xnat/dcm2niix/(Dockerfile|check_image_pin_sync\.sh)|trust/xnat/xnat/config/dcm2niix_command\.json|deploy/providers/kubernetes/templates/xnat-init-job\.yaml|trust/imaging-api/imaging_api/config\.py)$'
always_run: false

- repo: https://github.com/trufflesecurity/trufflehog
rev: v3.93.3
hooks:
Expand Down
18 changes: 14 additions & 4 deletions deploy/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ Each Dockerfile explicitly drops root privileges by running the application as a
| xnat-nginx | `nginx` | Pre-existing in the base image (`nginx`) |
| xnat-db | `postgres` | Pre-existing in the base image (`postgres`) |
| xnat-socket-proxy | `root` | Upstream `tecnativa/docker-socket-proxy` image — HAProxy connects to the root-owned Docker socket as its owner. Runs under `cap_drop: ALL` with no capabilities added back. |
| xnat-dcm2niix | `root` | Deliberately keeps the base image's root default, matching the output-file ownership the previous `xnat/dcm2niix` image produced on the Container Service's build mount (XNAT reads the converted NIfTIs back off that mount). Not a compose service: a one-shot container the Container Service launches per scan and then reaps, so it sits outside the `cap_drop` regime below, which the compose files impose. |
| flip-db / omop-db | `postgres` | Pre-existing in the base image (`postgres`) |

**Bind-mount ownership.** Because XNAT (`xnat`, UID 1001) and Orthanc (`orthanc`, UID 999) no
Expand Down Expand Up @@ -339,9 +340,14 @@ container.

### Docker Socket Isolation (XNAT Container Service)

XNAT's Container Service plugin launches processing containers — currently `xnat/dcm2niix`,
which every project created with DICOM→NIfTI conversion enabled triggers automatically on scan
archive. It used to do this through `/var/run/docker.sock` mounted straight into `xnat-web`,
XNAT's Container Service plugin launches processing containers — currently
`ghcr.io/londonaicentre/xnat-dcm2niix` (a version-pinned build of dcm2niix; see
`trust/xnat/dcm2niix/`), which every project created with DICOM→NIfTI conversion enabled
triggers automatically on scan archive. That GHCR package must be **public** — the Container
Service pulls it with no credentials, and org packages default to private, so a first publish (or
any package recreate) needs a one-off operator visibility flip; see
[`trust/xnat/README.md`](../trust/xnat/README.md#operator-action-the-ghcr-package-must-be-public).
It used to do this through `/var/run/docker.sock` mounted straight into `xnat-web`,
which is a root-equivalent capability: anything that compromises XNAT can exec into any container
on the host, start privileged containers, or mount arbitrary host paths.

Expand All @@ -356,7 +362,11 @@ proxy at all. The Container Service is pointed at that endpoint by
`trust/xnat/xnat/config/container-service-backend-configuration.json`, which
`configure-dcm2niix.sh` POSTs to `/xapi/docker/server` — and the configure run now hard-fails if
`/xapi/docker/server/ping` cannot reach Docker through the proxy, instead of leaving a
registered-but-unlaunchable dcm2niix command behind.
registered-but-unlaunchable dcm2niix command behind. The same script registers `ghcr.io` as a
credential-less Container Service image host: swarm-mode launches resolve registry auth from the
image-host list, and with no entry matching the image's registry hostname they NPE before any
service is created — even though the public image needs no credentials (see
`trust/xnat/README.md`).

The proxy allowlists only what a swarm-mode Container Service launch needs — `SERVICES`
(+ `POST` for the mutating calls), `TASKS`, `NODES`, `IMAGES` (pull-on-init), `INFO`, `SWARM`
Expand Down
8 changes: 6 additions & 2 deletions deploy/providers/kubernetes/templates/xnat-init-job.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,10 @@ spec:
exit 1
fi

# No image-host (registry) registration here, unlike configure-dcm2niix.sh: the
# Kubernetes compute backend pulls images via kubelet (public ghcr.io images need no
# pull secret), not via the Container Service's docker client, so the swarm-only
# null-AuthConfig NPE that registration works around cannot occur on this path.
echo "Registering Kubernetes backend with the Container Service plugin..."
CS_HTTP=$(curl -s -o /tmp/cs_resp.txt -w "%{http_code}" \
--connect-timeout 10 --max-time 120 \
Expand Down Expand Up @@ -730,10 +734,10 @@ data:
"name": "dcm2niix",
"label": "dcm2niix",
"description": "Runs dcm2niix",
"version": "1.6",
"version": "1.7",
"schema-version": "1.0",
"info-url": "https://github.com/rordenlab/dcm2niix",
"image": "xnat/dcm2niix:latest",
"image": "ghcr.io/londonaicentre/xnat-dcm2niix:v1.0.20260724",
"type": "docker",
"command-line": "dcm2niix [BIDS] [GZIP] [OTHER_OPTIONS] -o /output /input",
"mounts": [
Expand Down
2 changes: 1 addition & 1 deletion docs/source/components/component-xnat.rst
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ The following XNAT anonymize-api endpoints are used by FLIP:
DICOM to NIfTI Conversion
****************************

XNAT can automatically convert DICOM images to NIfTI format using the ``dcm2niix`` tool via the Container Service plugin. FLIP controls this conversion on a per-project basis through two XNAT mechanisms:
XNAT can automatically convert DICOM images to NIfTI format using the ``dcm2niix`` tool via the Container Service plugin. The converter runs from FLIP's version-pinned image ``ghcr.io/londonaicentre/xnat-dcm2niix`` (built from ``trust/xnat/dcm2niix/``; never a mutable ``latest`` tag — the previously used Docker Hub ``xnat/dcm2niix:latest`` resolved to a stale 2021 build that silently dropped slices from valid series). On the Docker/swarm backend the setup script also registers ``ghcr.io`` as a credential-less Container Service *image host*: without an image-host entry matching the image's registry hostname, container-service 3.8.1's swarm launch path fails with a ``NullPointerException`` before any container exists, even for a public image (the Kubernetes backend pulls via kubelet and does not need the entry). FLIP controls this conversion on a per-project basis through two XNAT mechanisms:

Commands vs Event Subscriptions
================================
Expand Down
8 changes: 8 additions & 0 deletions trust/imaging-api/imaging_api/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,14 @@ def apply_xnat_datasource_password(self) -> Self:
#
DATA_ACCESS_API_URL: str = "http://data-access-api:8000"

# Container Service image for automatic DICOM→NIfTI conversion. The per-project
# event subscription looks the XNAT command up by this exact image string, so it
# must match what trust/xnat/xnat/config/dcm2niix_command.json (and the K8s
# init-job's inline copy) registers at deploy time. Pinned by version tag, never
# `latest`: Docker Hub's mutable `xnat/dcm2niix:latest` resolved to a 2021 build
# that silently dropped slices from valid series (FLIP#980).
DCM2NIIX_IMAGE: str = "ghcr.io/londonaicentre/xnat-dcm2niix:v1.0.20260724"

#
BASE_IMAGES_DOWNLOAD_DIR: str

Expand Down
3 changes: 2 additions & 1 deletion trust/imaging-api/imaging_api/routers/projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException

from imaging_api.config import get_settings
from imaging_api.routers.schemas import (
CentralHubProject,
CreatedProject,
Expand Down Expand Up @@ -150,7 +151,7 @@ async def create_project_from_central_hub_project(
# Create a project-scoped event subscription for automatic DICOM-to-NIfTI conversion
# Active when dicom_to_nifti=True, deactivated when False (can be toggled later via XNAT API)
create_project_event_subscription(
project.ID, "xnat/dcm2niix:latest", central_hub_project.dicom_to_nifti, headers
project.ID, get_settings().DCM2NIIX_IMAGE, central_hub_project.dicom_to_nifti, headers
)

# Add central hub users to imaging project
Expand Down
Loading
Loading