Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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
135 changes: 135 additions & 0 deletions .github/workflows/docker_build_xnat_dcm2niix.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
# 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"

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
with:
repository: ${{ github.event.workflow_run.head_repository.full_name }}
Comment thread
atriaybagur marked this conversation as resolved.
Outdated
ref: ${{ github.event.workflow_run.head_sha }}

- 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 }}
GH_WR_BRANCH: ${{ github.event.workflow_run.head_branch }}
GH_WR_EVENT: ${{ github.event.workflow_run.event }}
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
BRANCH_NAME=""
if [[ "$GH_EVENT_NAME" == "workflow_run" && "$GH_WR_EVENT" == "push" ]]; then
BRANCH_NAME="$GH_WR_BRANCH"
elif [[ "$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 "Generated tags: ${TAGS}"

- name: Log in to GitHub Container Registry
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
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
13 changes: 9 additions & 4 deletions deploy/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -339,9 +339,10 @@ 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. 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 +357,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
4 changes: 2 additions & 2 deletions trust/imaging-api/imaging_api/services/projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ def get_command_info(container: str, headers: dict[str, str]) -> tuple[int, str]
Fetches the XNAT command ID and wrapper name for a given container image.

Args:
container (str): Container image name, e.g. "xnat/dcm2niix:latest".
container (str): Container image name (typically the configured ``Settings.DCM2NIIX_IMAGE``).
headers (dict[str, str]): XNAT authentication headers.
Comment thread
atriaybagur marked this conversation as resolved.

Returns:
Expand Down Expand Up @@ -301,7 +301,7 @@ def create_project_event_subscription(project_id: str, container: str, active: b

Args:
project_id (str): XNAT project ID to scope the subscription to.
container (str): Container image name, e.g. "xnat/dcm2niix:latest".
container (str): Container image name (typically the configured ``Settings.DCM2NIIX_IMAGE``).
headers (dict[str, str]): XNAT authentication headers.
active (bool): If True, the subscription is active immediately. If False, it is created
but deactivated (can be toggled later via the XNAT API).
Expand Down
3 changes: 2 additions & 1 deletion trust/imaging-api/tests/routers/test_projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import pytest

from imaging_api.config import get_settings
from imaging_api.routers.schemas import CentralHubProject, Experiment, Project, Subject
from imaging_api.utils.exceptions import AlreadyExistsError, NotFoundError

Expand Down Expand Up @@ -89,7 +90,7 @@ async def test_create_project_event_subscription_active_matches_dicom_to_nifti(
await create_project_from_central_hub_project(central_hub_project, headers, background_tasks)

mock_event_sub.assert_called_once_with(
TEST_XNAT_PROJECT_ID, "xnat/dcm2niix:latest", dicom_to_nifti, headers
TEST_XNAT_PROJECT_ID, get_settings().DCM2NIIX_IMAGE, dicom_to_nifti, headers
)


Expand Down
12 changes: 6 additions & 6 deletions trust/imaging-api/tests/services/test_projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ def test_get_command_info_success(mock_get, headers):
json=MagicMock(return_value=[{"id": 1, "xnat": [{"name": "dcm2niix-scan"}]}]),
)

command_id, wrapper_name = get_command_info("xnat/dcm2niix:latest", headers)
command_id, wrapper_name = get_command_info("ghcr.io/londonaicentre/xnat-dcm2niix:v1.0.20260724", headers)

assert command_id == 1
assert wrapper_name == "dcm2niix-scan"
Expand All @@ -297,7 +297,7 @@ def test_get_command_info_fetch_failure(mock_get, headers):
mock_get.return_value = MagicMock(status_code=500, text="Internal Server Error")

with pytest.raises(Exception, match="XNAT command fetch failed"):
get_command_info("xnat/dcm2niix:latest", headers)
get_command_info("ghcr.io/londonaicentre/xnat-dcm2niix:v1.0.20260724", headers)


# ===========================================================================
Expand All @@ -311,7 +311,7 @@ def test_create_project_event_subscription_active(mock_cmd_info, mock_put, mock_
mock_put.return_value = MagicMock(status_code=200)
mock_post.return_value = MagicMock(status_code=200)

create_project_event_subscription("TEST", "xnat/dcm2niix:latest", True, headers)
create_project_event_subscription("TEST", "ghcr.io/londonaicentre/xnat-dcm2niix:v1.0.20260724", True, headers)

mock_put.assert_called_once()
assert "/commands/1/wrappers/dcm2niix-scan/enabled" in mock_put.call_args[0][0]
Expand All @@ -331,7 +331,7 @@ def test_create_project_event_subscription_inactive(mock_cmd_info, mock_put, moc
mock_put.return_value = MagicMock(status_code=200)
mock_post.return_value = MagicMock(status_code=200)

create_project_event_subscription("TEST", "xnat/dcm2niix:latest", False, headers)
create_project_event_subscription("TEST", "ghcr.io/londonaicentre/xnat-dcm2niix:v1.0.20260724", False, headers)

mock_post.assert_called_once()
call_payload = mock_post.call_args[1]["json"]
Expand All @@ -345,7 +345,7 @@ def test_create_project_event_subscription_enable_command_failure(mock_cmd_info,
mock_put.return_value = MagicMock(status_code=500, text="Internal Server Error")

with pytest.raises(Exception, match="Enabling command"):
create_project_event_subscription("TEST", "xnat/dcm2niix:latest", True, headers)
create_project_event_subscription("TEST", "ghcr.io/londonaicentre/xnat-dcm2niix:v1.0.20260724", True, headers)


@patch("imaging_api.services.projects.requests.post")
Expand All @@ -357,7 +357,7 @@ def test_create_project_event_subscription_failure(mock_cmd_info, mock_put, mock
mock_post.return_value = MagicMock(status_code=500, text="Internal Server Error")

with pytest.raises(Exception, match="Creating event subscription"):
create_project_event_subscription("TEST", "xnat/dcm2niix:latest", True, headers)
create_project_event_subscription("TEST", "ghcr.io/londonaicentre/xnat-dcm2niix:v1.0.20260724", True, headers)


# ===========================================================================
Expand Down
17 changes: 17 additions & 0 deletions trust/xnat/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,23 @@ make build

This downloads the XNAT WAR and plugins from S3, then builds all three images (`xnat-web`, `xnat-db`, `xnat-nginx`) tagged as `${DOCKER_REGISTRY}xnat-<service>:${DOCKER_TAG}`.

The DICOM→NIfTI converter the Container Service launches is a fourth, standalone image —
`ghcr.io/londonaicentre/xnat-dcm2niix` (built from [`dcm2niix/`](dcm2niix/), published by its own
GitHub workflow). It is referenced by an immutable version tag from
[`xnat/config/dcm2niix_command.json`](xnat/config/dcm2niix_command.json), the K8s init-job's inline
copy of that command, and imaging-api's `Settings.DCM2NIIX_IMAGE` — bump all three together with the
Dockerfile's `DCM2NIIX_VERSION` (see FLIP#980: the Docker Hub `xnat/dcm2niix:latest` it replaces was
a stale 2021 build that silently dropped slices from valid series).

Because the image now lives on a registry other than Docker Hub, `configure-dcm2niix.sh` also
registers `ghcr.io` as a credential-less Container Service **image host**. This is load-bearing on
the swarm backend: container-service 3.8.1 resolves the launch's registry auth by matching an
image-host `url` against the bare registry hostname parsed from the image string, and with no match
the null result reaches docker-java's `withAuthConfig` unguarded — every conversion then fails with
`NullPointerException: authConfig was not specified` even though the public image needs no
credentials at all. (Found live: a full platform image pull converted nothing until the host entry
existed. The Kubernetes backend is unaffected — kubelet does the pulling there.)

### Run XNAT

Run both configured development XNAT instances with:
Expand Down
Loading
Loading