diff --git a/.github/workflows/docker_build_xnat_dcm2niix.yml b/.github/workflows/docker_build_xnat_dcm2niix.yml new file mode 100644 index 000000000..873bbe8e0 --- /dev/null +++ b/.github/workflows/docker_build_xnat_dcm2niix.yml @@ -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: + workflow_dispatch: + push: + branches: [main, develop] + # Narrower than the other xnat workflows' trust/xnat/** filter: this image + # 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 + 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- 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) + 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." diff --git a/.github/workflows/test_trust_xnat.yml b/.github/workflows/test_trust_xnat.yml index 670b11099..5410669c0 100644 --- a/.github/workflows/test_trust_xnat.yml +++ b/.github/workflows/test_trust_xnat.yml @@ -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] @@ -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] @@ -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: @@ -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 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 71c2132c2..f70241b5c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -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 `:` 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: diff --git a/deploy/README.md b/deploy/README.md index 273011624..a2d48549f 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -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 @@ -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. @@ -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` diff --git a/deploy/providers/kubernetes/templates/xnat-init-job.yaml b/deploy/providers/kubernetes/templates/xnat-init-job.yaml index adc7e7af9..c79e56ad1 100644 --- a/deploy/providers/kubernetes/templates/xnat-init-job.yaml +++ b/deploy/providers/kubernetes/templates/xnat-init-job.yaml @@ -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 \ @@ -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": [ diff --git a/docs/source/components/component-xnat.rst b/docs/source/components/component-xnat.rst index 1fa5f9842..75b4d164c 100644 --- a/docs/source/components/component-xnat.rst +++ b/docs/source/components/component-xnat.rst @@ -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 ================================ diff --git a/trust/imaging-api/imaging_api/config.py b/trust/imaging-api/imaging_api/config.py index 3c2d7726f..85cb1cb38 100644 --- a/trust/imaging-api/imaging_api/config.py +++ b/trust/imaging-api/imaging_api/config.py @@ -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 diff --git a/trust/imaging-api/imaging_api/routers/projects.py b/trust/imaging-api/imaging_api/routers/projects.py index e743ef99f..abb3e83a7 100644 --- a/trust/imaging-api/imaging_api/routers/projects.py +++ b/trust/imaging-api/imaging_api/routers/projects.py @@ -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, @@ -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 diff --git a/trust/imaging-api/imaging_api/services/projects.py b/trust/imaging-api/imaging_api/services/projects.py index e9fc82f81..c886cf8d6 100644 --- a/trust/imaging-api/imaging_api/services/projects.py +++ b/trust/imaging-api/imaging_api/services/projects.py @@ -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. Returns: @@ -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). diff --git a/trust/imaging-api/tests/routers/test_projects.py b/trust/imaging-api/tests/routers/test_projects.py index 9eafacb8f..7b7896d1a 100644 --- a/trust/imaging-api/tests/routers/test_projects.py +++ b/trust/imaging-api/tests/routers/test_projects.py @@ -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 @@ -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 ) diff --git a/trust/imaging-api/tests/services/test_projects.py b/trust/imaging-api/tests/services/test_projects.py index fc8bde817..014dabe0a 100644 --- a/trust/imaging-api/tests/services/test_projects.py +++ b/trust/imaging-api/tests/services/test_projects.py @@ -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" @@ -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) # =========================================================================== @@ -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] @@ -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"] @@ -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") @@ -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) # =========================================================================== diff --git a/trust/xnat/README.md b/trust/xnat/README.md index d1c54e0ff..f2a270d31 100644 --- a/trust/xnat/README.md +++ b/trust/xnat/README.md @@ -93,6 +93,44 @@ 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-:${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). +[`dcm2niix/check_image_pin_sync.sh`](dcm2niix/check_image_pin_sync.sh) enforces that the four stay in +sync — it runs as a pre-commit hook and as the `dcm2niix-pin-sync` CI job — because a half-bumped set +is otherwise silent: the old immutable tag simply keeps being pulled. + +#### Operator action: the GHCR package must be public + +**After the first publish, and again after any delete-and-recreate of the package, set +`ghcr.io/londonaicentre/xnat-dcm2niix` to public.** New org packages default to **private**, and +`GITHUB_TOKEN` cannot change a package's visibility, so the publish workflow cannot do this for you: + +> github.com/orgs/londonaicentre/packages/container/xnat-dcm2niix/settings → Danger Zone → +> Change visibility → Public + +This matters because the XNAT Container Service pulls the converter **anonymously** — it holds no +GHCR credentials, and the `ghcr.io` image host registered below is deliberately credential-less. A +private package therefore registers fine at deploy time and only fails when a scan is archived at a +trust, which is exactly the deferred, silent failure this pinning is meant to remove. + +The publish workflow's last step is the guard: it re-runs that anonymous manifest fetch against the +tag it just pushed and fails the run if it is not publicly readable. A red first-publish run is the +intended signal that the visibility flip is still outstanding — not a broken build. + +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: diff --git a/trust/xnat/dcm2niix/Dockerfile b/trust/xnat/dcm2niix/Dockerfile new file mode 100644 index 000000000..c6bdf9988 --- /dev/null +++ b/trust/xnat/dcm2niix/Dockerfile @@ -0,0 +1,68 @@ +# Copyright (c) 2026 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. +# +# Pinned dcm2niix image for the XNAT Container Service (FLIP#980). +# +# Replaces Docker Hub's `xnat/dcm2niix:latest`, which resolves to a 2021 build +# (v1.0.20211006) that upstream no longer updates and that silently drops slices +# from valid series ("Slices skipped: image position not sequential" mis-fires on +# clean, uniformly-spaced synthesized CT series — every affected spleen study then +# fails the tutorials' image/label shape QC). +# +# Referenced by exact image string in three places — keep them in sync when +# bumping DCM2NIIX_VERSION: +# - trust/xnat/xnat/config/dcm2niix_command.json (Compose deploy) +# - deploy/providers/kubernetes/templates/xnat-init-job.yaml (inline copy) +# - trust/imaging-api/imaging_api/config.py (Settings.DCM2NIIX_IMAGE) +# check_image_pin_sync.sh (pre-commit hook + a CI job in test_trust_xnat.yml) +# enforces that; a half-bumped set is otherwise silent, since the old immutable +# tag just keeps being pulled. +# The publish workflow (.github/workflows/docker_build_xnat_dcm2niix.yml) reads +# DCM2NIIX_VERSION below and pushes ghcr.io/londonaicentre/xnat-dcm2niix:. +# That package must be **public** — see trust/xnat/README.md. + +FROM debian:13-slim + +# The source label associates the GHCR package with this repo, so it is listed +# under github.com/londonaicentre/FLIP/packages alongside the other FLIP images +# instead of standing alone under the org. The siblings inherit that link from +# having been first published by a GITHUB_TOKEN push from this repo's workflows; +# the label states it explicitly, so the link holds however the image is pushed. +LABEL org.opencontainers.image.source="https://github.com/londonaicentre/FLIP" \ + org.opencontainers.image.description="dcm2niix for the FLIP trust XNAT Container Service, pinned to a \ +current upstream release (see trust/xnat/dcm2niix/)" \ + org.opencontainers.image.licenses="Apache-2.0" + +# Upstream release binary (glibc-linked), pinned by version AND checksum so a +# retagged upstream asset cannot slip through. +ARG DCM2NIIX_VERSION=v1.0.20260724 +ARG DCM2NIIX_SHA256=2916a7bc64511eb20fb976a994799183cc1bfdd03395bf5d8edef54a58dee06f + +RUN apt-get update \ + && apt-get install -y --no-install-recommends ca-certificates curl unzip \ + && curl -fsSL -o /tmp/dcm2niix_lnx.zip \ + "https://github.com/rordenlab/dcm2niix/releases/download/${DCM2NIIX_VERSION}/dcm2niix_lnx.zip" \ + && echo "${DCM2NIIX_SHA256} /tmp/dcm2niix_lnx.zip" | sha256sum -c - \ + && unzip -o /tmp/dcm2niix_lnx.zip -d /usr/local/bin dcm2niix \ + && chmod 0755 /usr/local/bin/dcm2niix \ + && rm /tmp/dcm2niix_lnx.zip \ + && apt-get purge -y curl unzip \ + && apt-get autoremove -y \ + && rm -rf /var/lib/apt/lists/* \ + # Build-time smoke check: the binary runs on this base (glibc) and reports the + # pinned version. Checked via the pipe because dcm2niix itself exits 3 on + # --version; the pipeline's status is grep's. + && dcm2niix --version | grep -qF "${DCM2NIIX_VERSION}" + +# The Container Service invokes `dcm2niix ... -o /output /input` directly (see +# dcm2niix_command.json) — no ENTRYPOINT, and the image keeps the root default +# user to match the previous image's output-file ownership on the build mount. +CMD ["dcm2niix", "--version"] diff --git a/trust/xnat/dcm2niix/check_image_pin_sync.sh b/trust/xnat/dcm2niix/check_image_pin_sync.sh new file mode 100755 index 000000000..e59151012 --- /dev/null +++ b/trust/xnat/dcm2niix/check_image_pin_sync.sh @@ -0,0 +1,85 @@ +#!/usr/bin/env bash +# Copyright (c) 2026 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. +# +# Guard: the dcm2niix Container Service image pin has one source of truth (FLIP#980). +# +# `ARG DCM2NIIX_VERSION` in trust/xnat/dcm2niix/Dockerfile decides which tag the publish workflow +# pushes; three deploy-time configs then carry the resulting +# `ghcr.io/londonaicentre/xnat-dcm2niix:` string as a literal. Nothing at runtime notices +# when a version bump leaves one of them behind — the old immutable tag simply keeps being pulled, +# and imaging-api's per-project event subscription looks the XNAT command up by that exact string, +# so a half-bumped set silently registers a command nothing ever triggers. This script fails on +# that drift. Wired as a pre-commit hook and as a CI step in .github/workflows/test_trust_xnat.yml. +# +# Usage: trust/xnat/dcm2niix/check_image_pin_sync.sh (runnable from any working directory) + +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../../.." && pwd)" +DOCKERFILE_REL="trust/xnat/dcm2niix/Dockerfile" +DOCKERFILE="${REPO_ROOT}/${DOCKERFILE_REL}" +IMAGE="ghcr.io/londonaicentre/xnat-dcm2niix" + +# Every file that pins the image by its full `:` string. +REFERENCES=( + "trust/xnat/xnat/config/dcm2niix_command.json" + "deploy/providers/kubernetes/templates/xnat-init-job.yaml" + "trust/imaging-api/imaging_api/config.py" +) + +if [[ ! -f "${DOCKERFILE}" ]]; then + echo "check_image_pin_sync: cannot find ${DOCKERFILE_REL}" >&2 + exit 1 +fi + +VERSION="$(sed -n 's/^ARG DCM2NIIX_VERSION=//p' "${DOCKERFILE}")" +if [[ -z "${VERSION}" ]]; then + echo "check_image_pin_sync: could not read 'ARG DCM2NIIX_VERSION=' from ${DOCKERFILE_REL}" >&2 + exit 1 +fi + +PINNED="${IMAGE}:${VERSION}" +status=0 + +for reference in "${REFERENCES[@]}"; do + path="${REPO_ROOT}/${reference}" + if [[ ! -f "${path}" ]]; then + echo "check_image_pin_sync: ${reference} is missing (the pin has nowhere to land)" >&2 + status=1 + continue + fi + + # Count lines carrying the image at any tag, then lines carrying it at the pinned tag. A file + # that dropped the reference entirely fails on the first count; a file left on an older tag + # fails on the mismatch. Counting (rather than `! grep -q`) keeps both failures distinct and + # keeps a partially-bumped file — some lines new, some old — from passing. + tagged="$(grep -c -F -- "${IMAGE}:" "${path}" || true)" + pinned_hits="$(grep -c -F -- "${PINNED}" "${path}" || true)" + + if [[ "${tagged}" -eq 0 ]]; then + echo "check_image_pin_sync: ${reference} no longer references ${IMAGE} at all" >&2 + status=1 + elif [[ "${tagged}" -ne "${pinned_hits}" ]]; then + echo "check_image_pin_sync: ${reference} does not match ${DOCKERFILE_REL} (${VERSION}):" >&2 + grep -n -F -- "${IMAGE}:" "${path}" | { grep -v -F -- "${PINNED}" || true; } >&2 + status=1 + fi +done + +if [[ "${status}" -ne 0 ]]; then + echo "" >&2 + echo "The dcm2niix image pin must read ${PINNED} in all of:" >&2 + printf ' %s\n' "${DOCKERFILE_REL}" "${REFERENCES[@]}" >&2 + exit 1 +fi + +echo "check_image_pin_sync: ${PINNED} is consistent across ${DOCKERFILE_REL} and ${#REFERENCES[@]} configs." diff --git a/trust/xnat/xnat/config/configure-dcm2niix.sh b/trust/xnat/xnat/config/configure-dcm2niix.sh index 9e3c7c607..86a5d912c 100644 --- a/trust/xnat/xnat/config/configure-dcm2niix.sh +++ b/trust/xnat/xnat/config/configure-dcm2niix.sh @@ -120,6 +120,38 @@ if [[ "$ping_status" != "200" ]]; then fi echo "Container Service -> Docker (via xnat-socket-proxy): OK" +# ---------------------------------------------------------------- +# IMAGE HOST (registry entry for the pinned dcm2niix image) +# ---------------------------------------------------------------- + +# In swarm mode the Container Service resolves registry credentials for `docker service create` by +# looking up an image-host entry whose `url` equals the registry hostname parsed from the image +# string — the BARE hostname, no scheme (container-service 3.8.1, DockerControlApi.authConfig → +# dockerHubService.getByUrl). With no matching entry the resolved AuthConfig is null, and the swarm +# launch path passes it to docker-java's withAuthConfig unguarded (unlike the pull path, which +# null-checks), so every launch dies with `NullPointerException: authConfig was not specified` +# before a service exists — even for a public image that needs no credentials. Register the +# registry as a credential-less image host so the lookup resolves. The hostname is derived from the +# image pinned in dcm2niix_command.json so it can never drift from the image actually launched; an +# image with no registry component is a Docker Hub image, covered by the built-in default entry. +DCM2NIIX_IMAGE=$(jq -r '.image // empty' dcm2niix_command.json) +DCM2NIIX_REGISTRY=${DCM2NIIX_IMAGE%%/*} +if [[ "$DCM2NIIX_REGISTRY" == *.* || "$DCM2NIIX_REGISTRY" == *:* || "$DCM2NIIX_REGISTRY" == localhost ]]; then + echo "Ensuring image host '$DCM2NIIX_REGISTRY' is registered..." + HUB_ID=$(xnat_curl "$XNAT_URL/xapi/docker/hubs" \ + | jq -r --arg url "$DCM2NIIX_REGISTRY" 'map(select(.url == $url)) | .[0].id // empty') + if [[ -z "$HUB_ID" ]]; then + xnat_curl -X POST "$XNAT_URL/xapi/docker/hubs" \ + -H "Content-Type: application/json" \ + -d "{\"name\": \"$DCM2NIIX_REGISTRY\", \"url\": \"$DCM2NIIX_REGISTRY\"}" >/dev/null + echo "Image host '$DCM2NIIX_REGISTRY' registered." + else + echo "Image host '$DCM2NIIX_REGISTRY' already registered (id=$HUB_ID)." + fi +else + echo "dcm2niix image has no registry component; the built-in Docker Hub image host applies." +fi + # ---------------------------------------------------------------- # CONTAINER SERVICE # ---------------------------------------------------------------- @@ -217,6 +249,16 @@ if [ "$WRAPPER_ENABLED" != "true" ]; then exit 1 fi +# Verify the image host for the pinned image's registry persisted (swarm launches NPE without it) +if [[ "$DCM2NIIX_REGISTRY" == *.* || "$DCM2NIIX_REGISTRY" == *:* || "$DCM2NIIX_REGISTRY" == localhost ]]; then + HUB_COUNT=$(xnat_curl "$XNAT_URL/xapi/docker/hubs" \ + | jq -r --arg url "$DCM2NIIX_REGISTRY" 'map(select(.url == $url)) | length') + if [ "$HUB_COUNT" -lt 1 ]; then + echo "ERROR: no image host registered with url '$DCM2NIIX_REGISTRY' (expected >= 1)" >&2 + exit 1 + fi +fi + # Verify event service is enabled EVENT_PREFS=$(xnat_curl "$XNAT_URL/xapi/events/prefs") EVENT_STATUS=$(echo "$EVENT_PREFS" | jq -r '.enabled // empty') diff --git a/trust/xnat/xnat/config/dcm2niix_command.json b/trust/xnat/xnat/config/dcm2niix_command.json index 22c0f78de..78e0cd895 100644 --- a/trust/xnat/xnat/config/dcm2niix_command.json +++ b/trust/xnat/xnat/config/dcm2niix_command.json @@ -2,10 +2,10 @@ "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": [