Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run as non-root user "pixl" where possible [force-system-test] #345

Closed
wants to merge 16 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .env.sample
Original file line number Diff line number Diff line change
Expand Up @@ -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
48 changes: 14 additions & 34 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -228,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:
Expand All @@ -245,7 +212,20 @@ 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

# 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

- name: Dump ehr-api docker logs for debugging
Expand Down
8 changes: 7 additions & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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: ${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
ENV: ${ENV}
Expand Down Expand Up @@ -100,6 +103,7 @@ services:
- *logs-volume
networks:
- pixl-net
user: pixl
healthcheck:
test: ["CMD", "curl", "-f", "http://hasher-api:8000/heart-beat"]
interval: 10s
Expand Down Expand Up @@ -278,6 +282,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:
Expand All @@ -304,6 +309,7 @@ services:
retries: 5
networks:
- pixl-net
user: pixl
environment:
<<: [*pixl-rabbit-mq, *proxy-common]
ORTHANC_RAW_USERNAME: ${ORTHANC_RAW_USERNAME}
Expand Down
4 changes: 4 additions & 0 deletions docker/ehr-api/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ RUN --mount=type=cache,target=/root/.cache \
pip install core/ . && \
if [ "$TEST" = "true" ]; then pip install core/[test] .[test]; fi

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

ENTRYPOINT ["uvicorn", "pixl_ehr.main:app", "--host", "0.0.0.0", "--port", "8000"]
4 changes: 4 additions & 0 deletions docker/hasher-api/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +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/*

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 .
Expand All @@ -34,4 +37,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"]
4 changes: 4 additions & 0 deletions docker/imaging-api/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,8 @@ RUN --mount=type=cache,target=/root/.cache \
pip install core/ . && \
if [ "$TEST" = "true" ]; then pip install core/ .[test]; fi

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"]
5 changes: 5 additions & 0 deletions docker/orthanc-anon/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ RUN python3 -m venv /.venv
# Install curl, used in system tests
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
COPY ./pixl_core/pyproject.toml /pixl_core/pyproject.toml
COPY ./pixl_dcmd/pyproject.toml /pixl_dcmd/pyproject.toml
Expand Down
5 changes: 5 additions & 0 deletions docker/orthanc-raw/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ ARG ORTHANC_RAW_MAXIMUM_STORAGE_SIZE
RUN export DEBIAN_FRONTEND=noninteractive && \
apt-get update && \
apt-get install --yes --no-install-recommends python3-venv

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

# Install requirements before copying modules
Expand Down
7 changes: 7 additions & 0 deletions pixl_imaging/tests/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
orthanc-raw-data:
vna-qr-data:
Expand All @@ -26,6 +30,7 @@ services:
context: ../../
dockerfile: ./docker/imaging-api/Dockerfile
args:
<<: *pixl-uid-gid
TEST: true
depends_on:
queue:
Expand Down Expand Up @@ -76,6 +81,8 @@ services:
build:
context: ../../
dockerfile: ./docker/orthanc-raw/Dockerfile
args:
<<: *pixl-uid-gid
environment:
ORTHANC_NAME: "PIXL: Raw"
ORTHANC_USERNAME: "orthanc"
Expand Down
5 changes: 5 additions & 0 deletions test/.env
Original file line number Diff line number Diff line change
Expand Up @@ -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
54 changes: 52 additions & 2 deletions test/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"
Expand All @@ -32,10 +34,58 @@ def host_export_root_dir():
RESOURCES_DIR = TEST_DIR / "resources"
RESOURCES_OMOP_DIR = RESOURCES_DIR / "omop"

PIXL_USER_UID = os.getenv("PIXL_USER_UID")
PIXL_USER_GID = os.getenv("PIXL_USER_GID")


@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",
# "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]
)
print("JES: after create user pixl")
run_subprocess(["cat", "/etc/passwd"])

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)
# poll here for two minutes to check for imaging to be processed, printing progress
Expand Down
7 changes: 7 additions & 0 deletions test/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
vna-qr-data:

Expand Down Expand Up @@ -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"]
Expand All @@ -80,3 +86,4 @@ services:
retries: 5
networks:
pixl-net:
user: pixl
3 changes: 3 additions & 0 deletions test/dummy-services/cogstack/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
gnupg \
locales

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 .
Expand Down
9 changes: 9 additions & 0 deletions test/run-system-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down