From be27995663849b03f467cfc99034191364db3766 Mon Sep 17 00:00:00 2001
From: Jeremy Stein <j.stein@ucl.ac.uk>
Date: Fri, 8 Mar 2024 12:34:09 +0000
Subject: [PATCH 01/16] Add non-root user "pixl" to all Dockerfiles, except
 postgres-derived ones, which use their own user. Run as this user from the
 docker-compose files for the services we control.

---
 docker-compose.yml                      | 3 +++
 docker/ehr-api/Dockerfile               | 2 ++
 docker/hasher-api/Dockerfile            | 2 ++
 docker/imaging-api/Dockerfile           | 2 ++
 docker/orthanc-anon/Dockerfile          | 2 ++
 docker/orthanc-raw/Dockerfile           | 3 +++
 test/docker-compose.yml                 | 1 +
 test/dummy-services/cogstack/Dockerfile | 1 +
 8 files changed, 16 insertions(+)

diff --git a/docker-compose.yml b/docker-compose.yml
index 54c8493e1..89d378e34 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -100,6 +100,7 @@ services:
       - *logs-volume
     networks:
       - pixl-net
+    user: pixl
     healthcheck:
       test: ["CMD", "curl", "-f", "http://hasher-api:8000/heart-beat"]
       interval: 10s
@@ -278,6 +279,7 @@ services:
       retries: 5
     networks:
       - pixl-net
+    user: pixl
     # needed for testing under GHA (linux), so this container
     # can reach the test FTP server running on the docker host
     extra_hosts:
@@ -304,6 +306,7 @@ services:
       retries: 5
     networks:
       - pixl-net
+    user: pixl
     environment:
       <<: [*pixl-rabbit-mq, *proxy-common]
       ORTHANC_RAW_USERNAME: ${ORTHANC_RAW_USERNAME}
diff --git a/docker/ehr-api/Dockerfile b/docker/ehr-api/Dockerfile
index b032e71ad..128884f74 100644
--- a/docker/ehr-api/Dockerfile
+++ b/docker/ehr-api/Dockerfile
@@ -44,6 +44,8 @@ RUN --mount=type=cache,target=/root/.cache \
     pip install core/ . && \
     if [ "$TEST" = "true" ]; then pip install core/[test] .[test]; fi
 
+RUN useradd pixl
+
 HEALTHCHECK CMD /usr/bin/curl -f http://0.0.0.0:8000/heart-beat || exit 1
 
 ENTRYPOINT ["uvicorn", "pixl_ehr.main:app", "--host", "0.0.0.0", "--port", "8000"]
diff --git a/docker/hasher-api/Dockerfile b/docker/hasher-api/Dockerfile
index af1305c68..b4fd683ad 100644
--- a/docker/hasher-api/Dockerfile
+++ b/docker/hasher-api/Dockerfile
@@ -24,6 +24,7 @@ RUN sed -i '/en_GB.UTF-8/s/^# //g' /etc/locale.gen && locale-gen
 # clean up
 RUN  apt-get autoremove && apt-get clean && rm -rf /var/lib/apt/lists/*
 
+RUN useradd pixl
 WORKDIR /app
 # Install requirements before copying modules
 COPY ./hasher/pyproject.toml .
@@ -34,4 +35,5 @@ COPY ./hasher/ .
 RUN --mount=type=cache,target=/root/.cache \
     pip install .
 
+RUN mkdir /logs && chown pixl:pixl /logs
 ENTRYPOINT ["uvicorn", "hasher.main:app", "--host", "0.0.0.0", "--port", "8000"]
diff --git a/docker/imaging-api/Dockerfile b/docker/imaging-api/Dockerfile
index 412284cd2..850a2dba1 100644
--- a/docker/imaging-api/Dockerfile
+++ b/docker/imaging-api/Dockerfile
@@ -44,4 +44,6 @@ RUN --mount=type=cache,target=/root/.cache \
     pip install core/ . && \
     if [ "$TEST" = "true" ]; then pip install core/ .[test]; fi
 
+RUN useradd pixl
+
 ENTRYPOINT ["/app/scripts/migrate_and_run.sh"]
diff --git a/docker/orthanc-anon/Dockerfile b/docker/orthanc-anon/Dockerfile
index d918337da..96786b56d 100644
--- a/docker/orthanc-anon/Dockerfile
+++ b/docker/orthanc-anon/Dockerfile
@@ -24,6 +24,8 @@ RUN python3 -m venv /.venv
 # Install curl, used in system tests
 RUN apt-get --assume-yes install curl
 
+RUN useradd pixl
+
 # Install requirements before copying modules
 COPY ./pixl_core/pyproject.toml /pixl_core/pyproject.toml
 COPY ./pixl_dcmd/pyproject.toml /pixl_dcmd/pyproject.toml
diff --git a/docker/orthanc-raw/Dockerfile b/docker/orthanc-raw/Dockerfile
index 801abb400..f71bb9d7e 100644
--- a/docker/orthanc-raw/Dockerfile
+++ b/docker/orthanc-raw/Dockerfile
@@ -21,6 +21,9 @@ ARG ORTHANC_RAW_MAXIMUM_STORAGE_SIZE
 RUN export DEBIAN_FRONTEND=noninteractive && \
     apt-get update && \
     apt-get install --yes --no-install-recommends python3-venv
+
+RUN useradd pixl
+
 RUN python3 -m venv /.venv
 
 # Install requirements before copying modules
diff --git a/test/docker-compose.yml b/test/docker-compose.yml
index 85b279280..46b81d568 100644
--- a/test/docker-compose.yml
+++ b/test/docker-compose.yml
@@ -80,3 +80,4 @@ services:
             retries: 5
         networks:
             pixl-net:
+        user: pixl
diff --git a/test/dummy-services/cogstack/Dockerfile b/test/dummy-services/cogstack/Dockerfile
index 2caf85e37..ff917a717 100644
--- a/test/dummy-services/cogstack/Dockerfile
+++ b/test/dummy-services/cogstack/Dockerfile
@@ -23,6 +23,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
         gnupg \
         locales
 
+RUN useradd pixl
 WORKDIR /app
 
 COPY cogstack.py .

From 7116500aaa8919e47467d9ab0689d23d1ba186fa Mon Sep 17 00:00:00 2001
From: Jeremy Stein <j.stein@ucl.ac.uk>
Date: Thu, 14 Mar 2024 18:51:23 +0000
Subject: [PATCH 02/16] TEMP - a bit of debugging to investigate the setup on
 GITHUB actions when it comes to permissions and how sudo works.

---
 test/conftest.py | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/test/conftest.py b/test/conftest.py
index a266d4055..7075e6a57 100644
--- a/test/conftest.py
+++ b/test/conftest.py
@@ -12,6 +12,8 @@
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
 """System/E2E test setup"""
+import os
+import subprocess
 from pathlib import Path
 
 import pytest
@@ -22,7 +24,7 @@
 pytest_plugins = "pytest_pixl"
 
 
-@pytest.fixture()
+@pytest.fixture(scope="session")
 def host_export_root_dir():
     """Intermediate export dir as seen from the host"""
     return Path(__file__).parents[1] / "projects" / "exports"
@@ -34,8 +36,28 @@ def host_export_root_dir():
 
 
 @pytest.fixture(scope="session")
-def _setup_pixl_cli(ftps_server) -> None:
+def _setup_pixl_cli(ftps_server, host_export_root_dir) -> None:
     """Run pixl populate/start. Cleanup intermediate export dir on exit."""
+    # see if it's already created
+    try:
+        print("JES: trying first ls -laR")
+        run_subprocess(["ls", "-laR", host_export_root_dir.parents[0]])
+    except subprocess.CalledProcessError:
+        try:
+            print("JES: trying second ls -laR")
+            run_subprocess(["ls", "-laR", host_export_root_dir.parents[1]])
+        except subprocess.CalledProcessError:
+            pass
+
+    # In production, it will be documented that the exports directory must be writable
+    # by the "pixl" user. For the system test we must do it here.
+    if os.getenv("GITHUB_ACTIONS") == "true":
+        run_subprocess(
+            ["sudo", "--non-interactive", "chown", "-R", "pixl:pixl", host_export_root_dir]
+        )
+
+    print("JES: after wards ls -laR")
+    run_subprocess(["ls", "-laR", host_export_root_dir.parents[0]])
     # CLI calls need to have CWD = test dir so they can find the pixl_config.yml file
     run_subprocess(["pixl", "populate", str(RESOURCES_OMOP_DIR.absolute())], TEST_DIR)
     # poll here for two minutes to check for imaging to be processed, printing progress

From b0b0a0904b5ff2e79f549dcd0ac5297377522258 Mon Sep 17 00:00:00 2001
From: Jeremy Stein <j.stein@ucl.ac.uk>
Date: Fri, 15 Mar 2024 11:51:53 +0000
Subject: [PATCH 03/16] More debugging

---
 test/conftest.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/test/conftest.py b/test/conftest.py
index 7075e6a57..793a46050 100644
--- a/test/conftest.py
+++ b/test/conftest.py
@@ -49,12 +49,18 @@ def _setup_pixl_cli(ftps_server, host_export_root_dir) -> None:
         except subprocess.CalledProcessError:
             pass
 
+    print("JES: running id and passwd")
+    run_subprocess(["id"])
+    run_subprocess(["cat", "/etc/passwd"])
     # In production, it will be documented that the exports directory must be writable
     # by the "pixl" user. For the system test we must do it here.
     if os.getenv("GITHUB_ACTIONS") == "true":
+        run_subprocess(["sudo", "--non-interactive", "adduser", "pixl"])
         run_subprocess(
             ["sudo", "--non-interactive", "chown", "-R", "pixl:pixl", host_export_root_dir]
         )
+    print("JES: after create user pixl")
+    run_subprocess(["cat", "/etc/passwd"])
 
     print("JES: after wards ls -laR")
     run_subprocess(["ls", "-laR", host_export_root_dir.parents[0]])

From 7122548ba51718abe520035870fc0d09b04d4cbb Mon Sep 17 00:00:00 2001
From: Jeremy Stein <j.stein@ucl.ac.uk>
Date: Fri, 15 Mar 2024 13:35:14 +0000
Subject: [PATCH 04/16] more debugging

---
 test/conftest.py | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/test/conftest.py b/test/conftest.py
index 793a46050..06286ae62 100644
--- a/test/conftest.py
+++ b/test/conftest.py
@@ -34,6 +34,9 @@ def host_export_root_dir():
 RESOURCES_DIR = TEST_DIR / "resources"
 RESOURCES_OMOP_DIR = RESOURCES_DIR / "omop"
 
+PIXL_USER_UID = 7001
+PIXL_USER_GID = 7001
+
 
 @pytest.fixture(scope="session")
 def _setup_pixl_cli(ftps_server, host_export_root_dir) -> None:
@@ -49,20 +52,29 @@ def _setup_pixl_cli(ftps_server, host_export_root_dir) -> None:
         except subprocess.CalledProcessError:
             pass
 
-    print("JES: running id and passwd")
-    run_subprocess(["id"])
-    run_subprocess(["cat", "/etc/passwd"])
     # In production, it will be documented that the exports directory must be writable
     # by the "pixl" user. For the system test we must do it here.
     if os.getenv("GITHUB_ACTIONS") == "true":
-        run_subprocess(["sudo", "--non-interactive", "adduser", "pixl"])
+        run_subprocess(
+            [
+                "sudo",
+                "--non-interactive",
+                "useradd",
+                "--uid",
+                PIXL_USER_UID,
+                "--gid",
+                PIXL_USER_GID,
+                "pixl",
+            ]
+        )
+        # this directory is part of the git repo so will already exist
         run_subprocess(
             ["sudo", "--non-interactive", "chown", "-R", "pixl:pixl", host_export_root_dir]
         )
     print("JES: after create user pixl")
     run_subprocess(["cat", "/etc/passwd"])
 
-    print("JES: after wards ls -laR")
+    print("JES: afterwards ls -laR")
     run_subprocess(["ls", "-laR", host_export_root_dir.parents[0]])
     # CLI calls need to have CWD = test dir so they can find the pixl_config.yml file
     run_subprocess(["pixl", "populate", str(RESOURCES_OMOP_DIR.absolute())], TEST_DIR)

From 6232898580f36264574f64b7a7d7e25d749d3cc5 Mon Sep 17 00:00:00 2001
From: Jeremy Stein <j.stein@ucl.ac.uk>
Date: Fri, 15 Mar 2024 13:53:02 +0000
Subject: [PATCH 05/16] fix silly error that I'd have thought mypy should catch

---
 test/conftest.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/conftest.py b/test/conftest.py
index 06286ae62..52eda03ac 100644
--- a/test/conftest.py
+++ b/test/conftest.py
@@ -61,9 +61,9 @@ def _setup_pixl_cli(ftps_server, host_export_root_dir) -> None:
                 "--non-interactive",
                 "useradd",
                 "--uid",
-                PIXL_USER_UID,
+                str(PIXL_USER_UID),
                 "--gid",
-                PIXL_USER_GID,
+                str(PIXL_USER_GID),
                 "pixl",
             ]
         )

From c1c055f44840e8f27f69d10667650e3b5021890e Mon Sep 17 00:00:00 2001
From: Jeremy Stein <j.stein@ucl.ac.uk>
Date: Fri, 15 Mar 2024 15:06:30 +0000
Subject: [PATCH 06/16] need to create group first

---
 test/conftest.py | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/test/conftest.py b/test/conftest.py
index 52eda03ac..b21e67394 100644
--- a/test/conftest.py
+++ b/test/conftest.py
@@ -55,6 +55,16 @@ def _setup_pixl_cli(ftps_server, host_export_root_dir) -> None:
     # In production, it will be documented that the exports directory must be writable
     # by the "pixl" user. For the system test we must do it here.
     if os.getenv("GITHUB_ACTIONS") == "true":
+        run_subprocess(
+            [
+                "sudo",
+                "--non-interactive",
+                "groupadd",
+                "--gid",
+                str(PIXL_USER_GID),
+                "pixl",
+            ]
+        )
         run_subprocess(
             [
                 "sudo",

From aea2c222f4c40b296840bac4e4a941b7c05cdd1a Mon Sep 17 00:00:00 2001
From: Jeremy Stein <j.stein@ucl.ac.uk>
Date: Fri, 15 Mar 2024 15:30:04 +0000
Subject: [PATCH 07/16] Create group and user with correct IDs in images

---
 docker/ehr-api/Dockerfile               | 4 +++-
 docker/hasher-api/Dockerfile            | 4 +++-
 docker/imaging-api/Dockerfile           | 4 +++-
 docker/orthanc-anon/Dockerfile          | 4 +++-
 docker/orthanc-raw/Dockerfile           | 4 +++-
 test/conftest.py                        | 4 ++--
 test/dummy-services/cogstack/Dockerfile | 4 +++-
 7 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/docker/ehr-api/Dockerfile b/docker/ehr-api/Dockerfile
index 128884f74..3cf70ebf7 100644
--- a/docker/ehr-api/Dockerfile
+++ b/docker/ehr-api/Dockerfile
@@ -44,7 +44,9 @@ RUN --mount=type=cache,target=/root/.cache \
     pip install core/ . && \
     if [ "$TEST" = "true" ]; then pip install core/[test] .[test]; fi
 
-RUN useradd pixl
+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
 
 HEALTHCHECK CMD /usr/bin/curl -f http://0.0.0.0:8000/heart-beat || exit 1
 
diff --git a/docker/hasher-api/Dockerfile b/docker/hasher-api/Dockerfile
index b4fd683ad..c958b5904 100644
--- a/docker/hasher-api/Dockerfile
+++ b/docker/hasher-api/Dockerfile
@@ -24,7 +24,9 @@ RUN sed -i '/en_GB.UTF-8/s/^# //g' /etc/locale.gen && locale-gen
 # clean up
 RUN  apt-get autoremove && apt-get clean && rm -rf /var/lib/apt/lists/*
 
-RUN useradd pixl
+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
 WORKDIR /app
 # Install requirements before copying modules
 COPY ./hasher/pyproject.toml .
diff --git a/docker/imaging-api/Dockerfile b/docker/imaging-api/Dockerfile
index 850a2dba1..ed8460194 100644
--- a/docker/imaging-api/Dockerfile
+++ b/docker/imaging-api/Dockerfile
@@ -44,6 +44,8 @@ RUN --mount=type=cache,target=/root/.cache \
     pip install core/ . && \
     if [ "$TEST" = "true" ]; then pip install core/ .[test]; fi
 
-RUN useradd pixl
+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
 
 ENTRYPOINT ["/app/scripts/migrate_and_run.sh"]
diff --git a/docker/orthanc-anon/Dockerfile b/docker/orthanc-anon/Dockerfile
index 96786b56d..6d7bc6793 100644
--- a/docker/orthanc-anon/Dockerfile
+++ b/docker/orthanc-anon/Dockerfile
@@ -24,7 +24,9 @@ RUN python3 -m venv /.venv
 # Install curl, used in system tests
 RUN apt-get --assume-yes install curl
 
-RUN useradd pixl
+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
 
 # Install requirements before copying modules
 COPY ./pixl_core/pyproject.toml /pixl_core/pyproject.toml
diff --git a/docker/orthanc-raw/Dockerfile b/docker/orthanc-raw/Dockerfile
index f71bb9d7e..9c136c5df 100644
--- a/docker/orthanc-raw/Dockerfile
+++ b/docker/orthanc-raw/Dockerfile
@@ -22,7 +22,9 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
     apt-get update && \
     apt-get install --yes --no-install-recommends python3-venv
 
-RUN useradd pixl
+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
 
 RUN python3 -m venv /.venv
 
diff --git a/test/conftest.py b/test/conftest.py
index b21e67394..d4024fba6 100644
--- a/test/conftest.py
+++ b/test/conftest.py
@@ -34,8 +34,8 @@ def host_export_root_dir():
 RESOURCES_DIR = TEST_DIR / "resources"
 RESOURCES_OMOP_DIR = RESOURCES_DIR / "omop"
 
-PIXL_USER_UID = 7001
-PIXL_USER_GID = 7001
+PIXL_USER_UID = os.getenv("PIXL_USER_UID")
+PIXL_USER_GID = os.getenv("PIXL_USER_GID")
 
 
 @pytest.fixture(scope="session")
diff --git a/test/dummy-services/cogstack/Dockerfile b/test/dummy-services/cogstack/Dockerfile
index ff917a717..d785574ec 100644
--- a/test/dummy-services/cogstack/Dockerfile
+++ b/test/dummy-services/cogstack/Dockerfile
@@ -23,7 +23,9 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
         gnupg \
         locales
 
-RUN useradd pixl
+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
 WORKDIR /app
 
 COPY cogstack.py .

From a3b8d2d9c989a73fb0fce0534eea1af2523a1642 Mon Sep 17 00:00:00 2001
From: Jeremy Stein <j.stein@ucl.ac.uk>
Date: Fri, 15 Mar 2024 15:30:48 +0000
Subject: [PATCH 08/16] Try to feed in the right variables to the build

---
 docker-compose.yml      | 5 ++++-
 test/docker-compose.yml | 6 ++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/docker-compose.yml b/docker-compose.yml
index 89d378e34..62c1a8639 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -26,9 +26,12 @@ x-proxy-common: &proxy-common
   https_proxy: *https-proxy
   NO_PROXY: *no-proxy
   no_proxy: *no-proxy
+x-pixl-uid-gid: &pixl-uid-gid
+  PIXL_USER_UID: 7001
+  PIXL_USER_GID: 7001
 
 x-build-args-common: &build-args-common
-  <<: [*proxy-common]
+  <<: [*proxy-common, *pixl-uid-gid]
 
 x-pixl-common-env: &pixl-common-env
   ENV: ${ENV}
diff --git a/test/docker-compose.yml b/test/docker-compose.yml
index 46b81d568..8b3033896 100644
--- a/test/docker-compose.yml
+++ b/test/docker-compose.yml
@@ -13,6 +13,10 @@
 #  limitations under the License.
 version: "3.8"
 
+x-pixl-uid-gid: &pixl-uid-gid
+    PIXL_USER_UID: 7001
+    PIXL_USER_GID: 7001
+
 volumes:
     vna-qr-data:
 
@@ -72,6 +76,8 @@ services:
     cogstack-api:
         container_name: system-test-cogstack
         build:
+            args:
+                <<: *pixl-uid-gid
             context: ./dummy-services/cogstack/
         healthcheck:
             test: ["CMD", "curl", "-f", "http://localhost:8000/heart-beat"]

From bf5705aabd5f33b2f1cddc2f54b7715fbb9a886a Mon Sep 17 00:00:00 2001
From: Jeremy Stein <j.stein@ucl.ac.uk>
Date: Fri, 15 Mar 2024 16:23:49 +0000
Subject: [PATCH 09/16] Move to vars file

---
 .env.sample             | 5 +++++
 docker-compose.yml      | 4 ++--
 test/.env               | 5 +++++
 test/docker-compose.yml | 4 ++--
 4 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/.env.sample b/.env.sample
index 049a7b02a..67777f5f6 100644
--- a/.env.sample
+++ b/.env.sample
@@ -98,3 +98,8 @@ PIXL_MAX_MESSAGES_IN_FLIGHT=100
 
 # Project configs directory
 PROJECT_CONFIGS_DIR=projects/configs
+
+# user and group IDs for pixl:pixl.
+# Only needed in production at build time
+PIXL_USER_UID=7001
+PIXL_USER_GID=7001
diff --git a/docker-compose.yml b/docker-compose.yml
index 62c1a8639..138a34c62 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -27,8 +27,8 @@ x-proxy-common: &proxy-common
   NO_PROXY: *no-proxy
   no_proxy: *no-proxy
 x-pixl-uid-gid: &pixl-uid-gid
-  PIXL_USER_UID: 7001
-  PIXL_USER_GID: 7001
+  PIXL_USER_UID: ${PIXL_USER_UID}
+  PIXL_USER_GID: ${PIXL_USER_GID}
 
 x-build-args-common: &build-args-common
   <<: [*proxy-common, *pixl-uid-gid]
diff --git a/test/.env b/test/.env
index 12d8484b5..79cfe6b83 100644
--- a/test/.env
+++ b/test/.env
@@ -82,3 +82,8 @@ FTP_USER_PASSWORD=longpassword
 
 # Project configs directory
 PROJECT_CONFIGS_DIR=projects/configs
+
+# user and group IDs for pixl:pixl that will be created during image
+# builds and on the GHA runner during the system test
+PIXL_USER_UID=7001
+PIXL_USER_GID=7001
diff --git a/test/docker-compose.yml b/test/docker-compose.yml
index 8b3033896..b244e6318 100644
--- a/test/docker-compose.yml
+++ b/test/docker-compose.yml
@@ -14,8 +14,8 @@
 version: "3.8"
 
 x-pixl-uid-gid: &pixl-uid-gid
-    PIXL_USER_UID: 7001
-    PIXL_USER_GID: 7001
+    PIXL_USER_UID: ${PIXL_USER_UID}
+    PIXL_USER_GID: ${PIXL_USER_GID}
 
 volumes:
     vna-qr-data:

From 0f62671b7b90611902c69b26cc6c25fc3fec91a9 Mon Sep 17 00:00:00 2001
From: Jeremy Stein <j.stein@ucl.ac.uk>
Date: Fri, 15 Mar 2024 16:43:04 +0000
Subject: [PATCH 10/16] Pass into the pytest so it can setup the user and group

---
 test/run-system-test.sh | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/test/run-system-test.sh b/test/run-system-test.sh
index f623e0d74..fa9ea79b9 100755
--- a/test/run-system-test.sh
+++ b/test/run-system-test.sh
@@ -48,7 +48,16 @@ elif [ "$subcmd" = "teardown" ]; then
     teardown
 else
     setup
+
+    # These two variables are needed for creating the pixl user and group on the GHA runner.
+    # Don't worry about passing them into pytest if you're running pytest manually or through your
+    # IDE - they're only used inside an "if GITHUB_ACTIONS" block anyway.
+    # We could do this in the setup function, but we don't at that point know the location of the
+    # exports dir (how the exports dir is determined is due to change anyway)
+    source .env
+    export PIXL_USER_UID PIXL_USER_GID
     pytest --verbose --log-cli-level INFO
+
     echo FINISHED PYTEST COMMAND
     teardown
     echo SYSTEM TEST SUCCESSFUL

From 5cbe3ffc197206204f5de89f92ffc61b23f987a4 Mon Sep 17 00:00:00 2001
From: Jeremy Stein <j.stein@ucl.ac.uk>
Date: Fri, 15 Mar 2024 17:30:32 +0000
Subject: [PATCH 11/16] Remove entire orthanc build caching from system test
 workflow because it seems to be doing it differently to the docker compose
 file, thus missing the build args. Yuck.

---
 .github/workflows/main.yml     | 24 ------------------------
 docker/orthanc-anon/Dockerfile |  1 +
 2 files changed, 1 insertion(+), 24 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 2b765b841..a663d1caf 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -186,30 +186,6 @@ jobs:
     timeout-minutes: 30
     steps:
       - uses: actions/checkout@v4
-      - uses: docker/setup-buildx-action@v3
-      # pre-build and cache the orthanc-anon image: installing python3 takes a while, doesn't push
-      - name: Cache Docker layers
-        uses: actions/cache@v4
-        with:
-          path: /tmp/.buildx-cache
-          key: ${{ runner.os }}-buildx-${{ github.ref }}-${{ github.sha }}
-          restore-keys: |
-            ${{ runner.os }}-buildx-${{ github.ref }}-
-            ${{ runner.os }}-buildx-
-          save-always: true
-      - uses: docker/build-push-action@v5
-        with:
-          context: .
-          file: docker/orthanc-anon/Dockerfile
-          cache-from: type=local,src=/tmp/.buildx-cache
-          cache-to: type=local,dest=/tmp/.buildx-cache-new,mode=max
-      - # Temp fix
-        # https://github.com/docker/build-push-action/issues/252
-        # https://github.com/moby/buildkit/issues/1896
-        name: Move cache
-        run: |
-          rm -rf /tmp/.buildx-cache
-          mv /tmp/.buildx-cache-new /tmp/.buildx-cache
 
       - name: Init Python
         uses: actions/setup-python@v5
diff --git a/docker/orthanc-anon/Dockerfile b/docker/orthanc-anon/Dockerfile
index 6d7bc6793..62d9308b7 100644
--- a/docker/orthanc-anon/Dockerfile
+++ b/docker/orthanc-anon/Dockerfile
@@ -26,6 +26,7 @@ RUN apt-get --assume-yes install curl
 
 ARG PIXL_USER_UID
 ARG PIXL_USER_GID
+RUN echo JES $PIXL_USER_GID $PIXL_USER_UID JES WTF
 RUN groupadd --gid $PIXL_USER_GID pixl && useradd --uid $PIXL_USER_UID --gid $PIXL_USER_GID pixl
 
 # Install requirements before copying modules

From cec5d7016453069a4f2a90c8e6063138e34a189c Mon Sep 17 00:00:00 2001
From: Jeremy Stein <j.stein@ucl.ac.uk>
Date: Fri, 15 Mar 2024 17:33:57 +0000
Subject: [PATCH 12/16] More copy paste :(

---
 pixl_imaging/tests/docker-compose.yml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/pixl_imaging/tests/docker-compose.yml b/pixl_imaging/tests/docker-compose.yml
index e5de17927..37798640d 100644
--- a/pixl_imaging/tests/docker-compose.yml
+++ b/pixl_imaging/tests/docker-compose.yml
@@ -13,6 +13,10 @@
 #  limitations under the License.
 version: "3.8"
 
+x-pixl-uid-gid: &pixl-uid-gid
+  PIXL_USER_UID: ${PIXL_USER_UID}
+  PIXL_USER_GID: ${PIXL_USER_GID}
+
 volumes:
   orthanc-raw-data:
   vna-qr-data:
@@ -26,6 +30,7 @@ services:
       context: ../../
       dockerfile: ./docker/imaging-api/Dockerfile
       args:
+        <<: *pixl-uid-gid
         TEST: true
     depends_on:
       queue:
@@ -76,6 +81,8 @@ services:
     build:
       context: ../../
       dockerfile: ./docker/orthanc-raw/Dockerfile
+      args:
+        <<: *pixl-uid-gid
     environment:
       ORTHANC_NAME: "PIXL: Raw"
       ORTHANC_USERNAME: "orthanc"

From bb4615cd7b361960b7c1c64edf911e9c26bb2ce9 Mon Sep 17 00:00:00 2001
From: Jeremy Stein <j.stein@ucl.ac.uk>
Date: Fri, 15 Mar 2024 17:37:25 +0000
Subject: [PATCH 13/16] The values from the env files are not being passed
 through. I think this all needs a rethink.

---
 pixl_imaging/tests/docker-compose.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pixl_imaging/tests/docker-compose.yml b/pixl_imaging/tests/docker-compose.yml
index 37798640d..dd8c7cf2e 100644
--- a/pixl_imaging/tests/docker-compose.yml
+++ b/pixl_imaging/tests/docker-compose.yml
@@ -14,8 +14,8 @@
 version: "3.8"
 
 x-pixl-uid-gid: &pixl-uid-gid
-  PIXL_USER_UID: ${PIXL_USER_UID}
-  PIXL_USER_GID: ${PIXL_USER_GID}
+  PIXL_USER_UID: 7001
+  PIXL_USER_GID: 7001
 
 volumes:
   orthanc-raw-data:

From 36489aad57d56126b5a2efebf88cd7c9bcf33c9c Mon Sep 17 00:00:00 2001
From: Jeremy Stein <j.stein@ucl.ac.uk>
Date: Fri, 15 Mar 2024 17:44:44 +0000
Subject: [PATCH 14/16] Why build before (with the wrong env probably) when the
 build script does this anyway?

---
 .github/workflows/main.yml | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index a663d1caf..87195067d 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -204,15 +204,6 @@ jobs:
       - name: Create .secrets.env
         run: touch .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:

From 76efd8eed3d5b50b82f31943593a071890f2a815 Mon Sep 17 00:00:00 2001
From: Jeremy Stein <j.stein@ucl.ac.uk>
Date: Mon, 18 Mar 2024 11:57:31 +0000
Subject: [PATCH 15/16] Just a thought - do the tests need to run as pixl now
 that we've changed the file ownership

---
 .github/workflows/main.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 87195067d..f8a4bda73 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -212,7 +212,7 @@ jobs:
           EXPORT_AZ_TENANT_ID: ${{ secrets.EXPORT_AZ_TENANT_ID }}
           EXPORT_AZ_KEY_VAULT_NAME: ${{ secrets.EXPORT_AZ_KEY_VAULT_NAME }}
         run: |
-          ./run-system-test.sh
+          sudo -u pixl -g pixl ./run-system-test.sh
           echo FINISHED SYSTEM TEST SCRIPT
 
       - name: Dump ehr-api docker logs for debugging

From 5ca72499a1ae04b11f008b3532727e57050ab77d Mon Sep 17 00:00:00 2001
From: Jeremy Stein <j.stein@ucl.ac.uk>
Date: Tue, 19 Mar 2024 17:57:38 +0000
Subject: [PATCH 16/16] Very WIP - easier to not pass this into the python at
 all, but we need to know where the export directory is - however this is
 supposed to be made configurable at some point so will actuall be in an env
 var, so we could do it this way?

---
 .github/workflows/main.yml | 13 +++++++++++
 test/conftest.py           | 44 +++++++++++++++++++-------------------
 test/run-system-test.sh    |  4 ++--
 3 files changed, 37 insertions(+), 24 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index f8a4bda73..2e3495b52 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -212,6 +212,19 @@ jobs:
           EXPORT_AZ_TENANT_ID: ${{ secrets.EXPORT_AZ_TENANT_ID }}
           EXPORT_AZ_KEY_VAULT_NAME: ${{ secrets.EXPORT_AZ_KEY_VAULT_NAME }}
         run: |
+          
+          # These two variables are needed for creating the pixl user and group on the GHA runner.
+          # Don't worry about passing them into pytest if you're running pytest manually or through your
+          # IDE - they're only used inside an "if GITHUB_ACTIONS" block anyway.
+          # We could do this in the setup function, but we don't at that point know the location of the
+          # exports dir (how the exports dir is determined is due to change anyway)
+          source .env
+          export PIXL_USER_UID PIXL_USER_GID
+          sudo --non-interactive groupadd --gid $PIXL_USER_GID pixl
+          sudo --non-interactive useradd --uid $PIXL_USER_UID --gid pixl --groups docker pixl
+          sudo --non-interactive chown -R pixl:docker host_export_root_dir
+          sudo --non-interactive chmod -R ug+rwx host_export_root_dir
+          
           sudo -u pixl -g pixl ./run-system-test.sh
           echo FINISHED SYSTEM TEST SCRIPT
 
diff --git a/test/conftest.py b/test/conftest.py
index d4024fba6..169e66065 100644
--- a/test/conftest.py
+++ b/test/conftest.py
@@ -55,28 +55,28 @@ def _setup_pixl_cli(ftps_server, host_export_root_dir) -> None:
     # In production, it will be documented that the exports directory must be writable
     # by the "pixl" user. For the system test we must do it here.
     if os.getenv("GITHUB_ACTIONS") == "true":
-        run_subprocess(
-            [
-                "sudo",
-                "--non-interactive",
-                "groupadd",
-                "--gid",
-                str(PIXL_USER_GID),
-                "pixl",
-            ]
-        )
-        run_subprocess(
-            [
-                "sudo",
-                "--non-interactive",
-                "useradd",
-                "--uid",
-                str(PIXL_USER_UID),
-                "--gid",
-                str(PIXL_USER_GID),
-                "pixl",
-            ]
-        )
+        # run_subprocess(
+        #     [
+        #         "sudo",
+        #         "--non-interactive",
+        #         "groupadd",
+        #         "--gid",
+        #         str(PIXL_USER_GID),
+        #         "pixl",
+        #     ]
+        # )
+        # run_subprocess(
+        #     [
+        #         "sudo",
+        #         "--non-interactive",
+        #         "useradd",
+        #         "--uid",
+        #         str(PIXL_USER_UID),
+        #         "--gid",
+        #         str(PIXL_USER_GID),
+        #         "pixl",
+        #     ]
+        # )
         # this directory is part of the git repo so will already exist
         run_subprocess(
             ["sudo", "--non-interactive", "chown", "-R", "pixl:pixl", host_export_root_dir]
diff --git a/test/run-system-test.sh b/test/run-system-test.sh
index fa9ea79b9..33c18d630 100755
--- a/test/run-system-test.sh
+++ b/test/run-system-test.sh
@@ -54,8 +54,8 @@ else
     # IDE - they're only used inside an "if GITHUB_ACTIONS" block anyway.
     # We could do this in the setup function, but we don't at that point know the location of the
     # exports dir (how the exports dir is determined is due to change anyway)
-    source .env
-    export PIXL_USER_UID PIXL_USER_GID
+#    source .env
+#    export PIXL_USER_UID PIXL_USER_GID
     pytest --verbose --log-cli-level INFO
 
     echo FINISHED PYTEST COMMAND