Don't run docker containers as root

.env.sample

@@ -106,3 +106,7 @@ HASHER_API_AZ_CLIENT_ID=
 HASHER_API_AZ_CLIENT_PASSWORD=
 HASHER_API_AZ_TENANT_ID=
 HASHER_API_AZ_KEY_VAULT_NAME=
+
+# User and group IDs for pixl:pixl. Only needed at build time.
+PIXL_USER_UID=7001
+PIXL_USER_GID=7001

.github/workflows/main.yml

@@ -107,15 +107,6 @@ jobs:
             - name: Create .secrets.env
               run: touch test/.secrets.env
 
-            - name: Build test services
-              working-directory: test
-              run: |
-                  docker compose build
-
-            - name: Build services
-              run: |
-                  docker compose build
-
             - name: Run tests
               working-directory: test
               env:
@@ -128,7 +119,24 @@ jobs:
                   HASHER_API_AZ_TENANT_ID: ${{ secrets.EXPORT_AZ_TENANT_ID }}
                   HASHER_API_AZ_KEY_VAULT_NAME: ${{ secrets.EXPORT_AZ_KEY_VAULT_NAME }}
               run: |
-                  ./run-system-test.sh coverage
+                  # We need PIXL_USER_UID and PIXL_USER_GID from the test env file.
+                  source .env
+                  # Set up as per docker_perms.md
+                  # Most importantly, the pixl user is *not* in the docker group, and
+                  # the default "runner" user is added to the pixl group.
+                  sudo --non-interactive groupadd --gid $PIXL_USER_GID pixl
+                  sudo --non-interactive useradd --uid $PIXL_USER_UID --gid pixl pixl
+                  sudo --non-interactive usermod --append --groups pixl runner
+                  host_export_root_dir=../projects/exports
+                  sudo --non-interactive chown -R pixl:pixl "$host_export_root_dir"
+                  sudo --non-interactive chmod -R u=rwx,g=rwxs,o= "$host_export_root_dir"
+                  sudo --non-interactive setfacl -R -m d:g::rwX  "$host_export_root_dir"
+                  # The top-level commands are still run as the GHA "runner" user, which
+                  # is analogous to a human user on the GAE bringing up a PIXL instance.
+                  # The sudo command is needed to make it run in a new shell
+                  # so that the new group (see usermod above) is picked up.
+                  # PATH needs to be explicitly mentioned to be passed through.
+                  sudo --non-interactive --preserve-env --preserve-env=PATH -u runner ./run-system-test.sh coverage
                   echo FINISHED SYSTEM TEST SCRIPT
 
             - name: Dump export-api docker logs for debugging
@@ -146,6 +154,12 @@ jobs:
               run: |
                   docker logs -t system-test-imaging-api-1 2>&1
 
+            - name: View filesystem for debugging
+              if: ${{ failure() }}
+              run: |
+                  # just in case the error is caused by permissions, make sure all files are seen
+                  sudo --non-interactive  ls -laR
+
             - name: Upload coverage reports to Codecov
               uses: codecov/codecov-action@v4.2.0
               with:

README.md

@@ -277,10 +277,14 @@ These files will subsequently be uploaded to the `parquet` destination specified
 [project config](#3-configure-a-new-project).
 
 ```
-EXPORT_ROOT/PROJECT_SLUG/all_extracts/EXTRACT_DATETIME/radiology/radiology.parquet
+EXPORT_ROOT/PROJECT_SLUG/all_extracts/EXTRACT_DATETIME/radiology/IMAGE_LINKER.parquet
 ....................................................../omop/public/*.parquet
 ```
 
+This directory is current located in `projects/exports` relative to the source code root.
+
+[Further discussion about docker permissions and the exports directory is found here](docs/setup/docker_perms.md)
+
 ### Destination
 
 #### FTP server

docker-compose.yml

@@ -25,8 +25,12 @@ x-proxy-common: &proxy-common
     NO_PROXY: *no-proxy
     no_proxy: *no-proxy
 
+x-pixl-uid-gid: &pixl-uid-gid
+    PIXL_USER_UID: ${PIXL_USER_UID}
+    PIXL_USER_GID: ${PIXL_USER_GID}
+
 x-build-args-common: &build-args-common
-    <<: [*proxy-common]
+    <<: [*proxy-common, *pixl-uid-gid]
 
 x-pixl-common-env: &pixl-common-env
     DEBUG: ${DEBUG}
@@ -81,8 +85,10 @@ services:
     hasher-api:
         build:
             context: .
-            dockerfile: ./docker/hasher-api/Dockerfile
+            dockerfile: ./docker/pixl-python/Dockerfile
+            target: hasher_api
             args:
+                PIXL_PACKAGE_DIR: hasher
                 <<: *build-args-common
         environment:
             <<: [*proxy-common, *pixl-common-env]
@@ -100,7 +106,6 @@ services:
         networks:
             - pixl-net
         healthcheck:
-            test: ["CMD", "curl", "-f", "http://hasher-api:8000/heart-beat"]
             interval: 10s
             timeout: 30s
             retries: 5
@@ -253,8 +258,10 @@ services:
     export-api:
         build:
             context: .
-            dockerfile: ./docker/export-api/Dockerfile
+            dockerfile: ./docker/pixl-python/Dockerfile
+            target: export_api
             args:
+                PIXL_PACKAGE_DIR: pixl_export
                 <<: *build-args-common
         environment:
             <<:
@@ -293,22 +300,23 @@ services:
         extra_hosts:
             - "host.docker.internal:host-gateway"
         volumes:
-            - ${PWD}/projects/exports:/run/projects/exports
+            - ${PWD}/projects/exports:/run/projects/exports:ro
             - ${PWD}/projects/configs:/${PROJECT_CONFIGS_DIR:-/projects/configs}:ro
 
     imaging-api:
         build:
             context: .
-            dockerfile: ./docker/imaging-api/Dockerfile
+            dockerfile: ./docker/pixl-python/Dockerfile
+            target: imaging_api
             args:
+                PIXL_PACKAGE_DIR: pixl_imaging
                 <<: *build-args-common
         depends_on:
             queue:
                 condition: service_healthy
             orthanc-raw:
                 condition: service_healthy
         healthcheck:
-            test: curl -f http://0.0.0.0:8000/heart-beat
             interval: 10s
             timeout: 30s
             retries: 5

docker/export-api/Dockerfile → docker/pixl-python/Dockerfile

@@ -11,7 +11,7 @@
 #  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.
-FROM python:3.11.7-slim-bullseye
+FROM python:3.11.7-slim-bullseye AS pixl_python_base
 SHELL ["/bin/bash", "-o", "pipefail", "-e", "-u", "-x", "-c"]
 
 ARG TEST="false"
@@ -29,20 +29,36 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
 RUN sed -i '/en_GB.UTF-8/s/^# //g' /etc/locale.gen && locale-gen
 RUN  apt-get autoremove && apt-get clean && rm -rf /var/lib/apt/lists/*
 
+HEALTHCHECK CMD /usr/bin/curl -f http://0.0.0.0:8000/heart-beat || exit 1
+
 WORKDIR /app
+# specify which of our packages we're installing using build time arg
+ARG PIXL_PACKAGE_DIR
 
 # Install requirements before copying modules
 COPY ./pixl_core/pyproject.toml ./pixl_core/pyproject.toml
-COPY ./pixl_export/pyproject.toml ./pixl_export/pyproject.toml
+COPY ./$PIXL_PACKAGE_DIR/pyproject.toml ./$PIXL_PACKAGE_DIR/pyproject.toml
 RUN pip3 install --no-cache-dir pixl_core/ \
-    && pip3 install --no-cache-dir pixl_export/
+    && pip3 install --no-cache-dir $PIXL_PACKAGE_DIR/
 
+# Install our code
 COPY ./pixl_core/ pixl_core/
-COPY ./pixl_export/ .
+COPY ./$PIXL_PACKAGE_DIR/ .
 RUN pip install --no-cache-dir --force-reinstall --no-deps pixl_core/ \
     --no-cache-dir --force-reinstall --no-deps . && \
     if [ "$TEST" = "true" ]; then pip install --no-cache-dir pixl_core/[test] .[test]; fi
 
-HEALTHCHECK CMD /usr/bin/curl -f http://0.0.0.0:8000/heart-beat || exit 1
+ARG PIXL_USER_UID
+ARG PIXL_USER_GID
+RUN groupadd --gid $PIXL_USER_GID pixl && useradd --uid $PIXL_USER_UID --gid $PIXL_USER_GID pixl
+USER pixl
 
+# Each container should be run with a different entry point
+FROM pixl_python_base AS export_api
 ENTRYPOINT ["uvicorn", "pixl_export.main:app", "--host", "0.0.0.0", "--port", "8000"]
+
+FROM pixl_python_base AS hasher_api
+ENTRYPOINT ["uvicorn", "hasher.main:app", "--host", "0.0.0.0", "--port", "8000"]
+
+FROM pixl_python_base AS imaging_api
+ENTRYPOINT ["/app/scripts/migrate_and_run.sh"]

docs/setup/docker_perms.md

@@ -0,0 +1,36 @@
+# Docker setup
+
+Setup of user IDs, file permissions, ACLs, and other related things in docker.
+
+## Requirements and design considerations
+
+### The aim - Docker containers should not run as root
+
+Generally it's considered a bad idea to run anything as root that doesn't need it.
+See "Principle of Least Privilege".
+
+Bear in mind that if you're running as a user in the `docker` group, you might as well
+be running as root, because you can create docker containers that run as root.
+
+### How to run as non-root
+
+During the docker image build, a user called `pixl` with UID `PIXL_USER_UID` and
+a group called `pixl` with GID `PIXL_USER_GID` are created *on the container*.
+When the container is started, it runs as this user.
+
+The numerical IDs need to be configurable because on the GAEs,
+docker is set up so that there is no remapping between user IDs on containers and
+user IDs on the host.
+If you're UID 7001 on the container, then
+you're 7001 on the host!
+
+
+# System testing
+
+The build args are also used by the GHA system test workflow to set up this user and group on
+the GHA test runner, simulating how a user should set this up in production.
+
+If you're running the system test on linux, you will probably have better luck if Docker
+is installed in rootless mode.
+
+

docs/setup/uclh-infrastructure-setup.md

@@ -77,3 +77,26 @@ chmod -R g+rwxs /gae/pixl_dev  # inherit group when new directories or files are
 setfacl -R -m d:g::rwX /gae/pixl_dev
 # now clone the repository or copy an existing deployment
 ```
+
+Following clone, the exports directory needs to be permissioned appropriately:
+
+Change these values in your production `.env` file:
+```
+PIXL_USER_UID=???
+PIXL_USER_GID=???
+```
+so that they match the UID and GID of the pixl user/group on
+your host.
+
+Set the ownership of the "exports" directory as follows:
+
+```
+MY_EXPORT_DIR=/gae/pixl_dev/PIXL/projects/exports
+chgrp -R pixl "$MY_EXPORT_DIR"
+```
+
+See Issue https://github.com/UCLH-Foundry/PIXL/issues/401 where I suggest setting
+the whole PIXL repo as group `pixl` rather than `docker`, for simplicity.
+
+See [docker permissions setup](./docker_perms.md) for further discussion about
+why the docker export directory needs to be set up this way.

test/.env

@@ -83,3 +83,7 @@ FTP_USER_PASSWORD=longpassword
 
 # Project configs directory
 PROJECT_CONFIGS_DIR=projects/configs
+
+# User and group IDs for pixl:pixl. Only needed at build time.
+PIXL_USER_UID=7001
+PIXL_USER_GID=7001