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

Don't run docker containers as root #400

Open
wants to merge 81 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
81 commits
Select commit Hold shift + click to select a range
93e99d5
Make packages easier to read
jeremyestein May 8, 2024
4b1f8fa
Remove unneeded packages
jeremyestein May 8, 2024
d087520
Merge three pixl python docker images into one multi-stage dockerfile to
jeremyestein May 8, 2024
1e1c18e
Specify healthcheck command in only one place
jeremyestein May 8, 2024
63d6edb
De-dupe the pre-reqs installation code using a build arg
jeremyestein May 8, 2024
46a08e0
De-dupe the actual install as well
jeremyestein May 8, 2024
64fcbc5
Add (failing) test for the condition we are aiming for: containers to
jeremyestein May 8, 2024
0d5cb39
Run as user pixl, which we have to create inside the image with a
jeremyestein May 9, 2024
397cb68
The system test already builds the required docker images, and passes
jeremyestein May 9, 2024
d7fa46e
Semi-WIP: Create pixl user+group on the GHA test runner so that we ca…
jeremyestein May 9, 2024
d8525b1
Fix variable usage and exports dir location
jeremyestein May 9, 2024
6facef9
More debugging
jeremyestein May 9, 2024
55db93c
debug
jeremyestein May 9, 2024
6f7c8dd
debug
jeremyestein May 9, 2024
0ff9148
split build and up
jeremyestein May 9, 2024
cce26a5
Create user home dir in the GHA script to avoid error from failed
jeremyestein May 9, 2024
1e3ed88
Why is pytest not found? Some kind of env change caused by sudo?
jeremyestein May 9, 2024
8abcb5f
Preserve path when running sudo
jeremyestein May 9, 2024
de4e1e9
Run tests as normal runner user
jeremyestein May 9, 2024
5736f30
Don't see why this needs to have docker group
jeremyestein May 9, 2024
09f37ea
Debugging for perm failure on file deletes
jeremyestein May 9, 2024
0edcf17
Fix scope error
jeremyestein May 9, 2024
d0201d8
OK maybe this was needed after all
jeremyestein May 9, 2024
077a304
Since CLI creates everything in exports dir these days, it makes more
jeremyestein May 9, 2024
cd93adc
Try not creating home dir for pixl - what is even using this?
jeremyestein May 9, 2024
c0c0d96
and therefore I can't do this any more
jeremyestein May 9, 2024
9507816
Remove debugging
jeremyestein May 10, 2024
bd5cf84
Document the reason for the permissions setup better. Don't need to
jeremyestein May 10, 2024
de2ea9d
If Export API is believed to only read from the export dir, then let's
jeremyestein May 10, 2024
9aaf2c7
First attempt at documentation on docker permissions.
jeremyestein May 10, 2024
0bd75fb
CLI user needs to be able to write to export dir.
jeremyestein May 10, 2024
db201d9
Clarify docs
jeremyestein May 10, 2024
928a092
Go with the assumption that the host machine will have a "pixl" user and
jeremyestein May 15, 2024
e13fe01
oops left in empty arg
jeremyestein May 15, 2024
017a0a9
Set the setgid bit so that subdirs/files in exports will be owned by the
jeremyestein May 15, 2024
9eb831b
debugging
jeremyestein May 15, 2024
8e9771b
fix debugging
jeremyestein May 15, 2024
15b98c8
doesn't exist?
jeremyestein May 15, 2024
572144d
Make debugging more useful and permanent. More docs
jeremyestein May 15, 2024
2d7c902
more debugging
jeremyestein May 15, 2024
cf1900e
Set the FACL
jeremyestein May 15, 2024
eb5da97
usermod doesn't change groups of existing shells. Need to run the system
jeremyestein May 15, 2024
995b35b
Invoking a new shell with "bash" doesn't re-read groups, but using sudo
jeremyestein May 15, 2024
303374c
Need to preserve envs
jeremyestein May 15, 2024
edde878
more debugging
jeremyestein May 15, 2024
ef1b113
Unlikely to work because su will prompt
jeremyestein May 15, 2024
806449d
Will just passing PATH be enough?
jeremyestein May 15, 2024
629a5e4
Pass through all env as well as PATH
jeremyestein May 15, 2024
8bbe273
Remove debugging
jeremyestein May 15, 2024
b9edf54
Merge branch 'main' into jeremy/docker-uid-new
jeremyestein May 20, 2024
d167c13
Merge branch 'main' into jeremy/docker-uid-new
jeremyestein May 21, 2024
4c6f6d1
Unite orthanc Dockerfiles
jeremyestein May 21, 2024
1b46c83
Add orthanc containers to test
jeremyestein May 21, 2024
468d908
Make orthanc run as orthanc user instead of root
jeremyestein May 22, 2024
1004681
Can also use non-root for fakes in system test
jeremyestein May 22, 2024
668ebc0
Non-root orthanc works for core and imaging tests too
jeremyestein May 22, 2024
695f64f
Add fakes to system test container user check
jeremyestein May 22, 2024
b34abbc
Remove debugging
jeremyestein May 22, 2024
02e3b79
Use same orthanc version everywhere
jeremyestein May 22, 2024
420fb7f
Use existing YAML anchors
jeremyestein May 22, 2024
b4d2526
Don't think this is needed since we moved this code to the export-api
jeremyestein May 22, 2024
cca7b93
Try adding orthanc to PIXL group
stefpiatek Jun 14, 2024
5925520
Try adding orthanc to PIXL group
stefpiatek Jun 14, 2024
3437005
Try adding orthanc to PIXL group
stefpiatek Jun 14, 2024
905e741
Try adding orthanc to PIXL group
stefpiatek Jun 14, 2024
19be6c7
Change owner of pixl file
stefpiatek Jun 14, 2024
4c39072
Try setting pixl user before copying
stefpiatek Jun 14, 2024
32ca6bb
Change owner of python projects on copy
stefpiatek Jun 14, 2024
81eabb7
Change owner of python projects on copy
stefpiatek Jun 14, 2024
21ccb13
Change owner of python projects on copy
stefpiatek Jun 14, 2024
9ef77e7
Merge branch 'main' into jeremy/docker-uid-new
stefpiatek Jun 14, 2024
ee1b97c
Define wait for images to be exported in conftest
stefpiatek Jun 14, 2024
47822a9
Add group ID to test build args
stefpiatek Jun 14, 2024
242aacf
Merge branch 'main' into jeremy/docker-uid-new
milanmlft Jul 4, 2024
6e945c3
Update README.md
stefpiatek Jul 18, 2024
c7b2711
Merge branch 'main' into jeremy/docker-uid-new
milanmlft Jul 30, 2024
521b1cd
Consolidate `RUN` commands
milanmlft Jul 30, 2024
653a921
Merge branch 'main' into jeremy/docker-uid-new
stefpiatek Jul 30, 2024
5d1937d
Add @dram1964's instructions for setting up a PIXL user on GAE
milanmlft Jul 31, 2024
b8a6de5
Merge remote-tracking branch 'origin/main' into jeremy/docker-uid-new
stefpiatek Oct 7, 2024
75af526
Merge remote-tracking branch 'origin/main' into jeremy/docker-uid-new
stefpiatek Oct 7, 2024
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
4 changes: 4 additions & 0 deletions .env.sample
Original file line number Diff line number Diff line change
Expand Up @@ -103,3 +103,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
34 changes: 24 additions & 10 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -108,15 +108,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:
Expand All @@ -129,7 +120,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 queue docker logs for debugging
Expand Down Expand Up @@ -178,6 +186,12 @@ jobs:
docker logs -t system-test-dicomweb-server-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@e28ff129e5465c2c0dcc6f003fc735cb6ae0c673 # v4.5.0
with:
Expand Down
6 changes: 5 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -282,10 +282,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 currently 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
Expand Down
35 changes: 20 additions & 15 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -74,8 +78,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]
Expand All @@ -93,7 +99,6 @@ services:
networks:
- pixl-net
healthcheck:
test: ["CMD", "curl", "-f", "http://hasher-api:8000/heart-beat"]
interval: 10s
timeout: 30s
retries: 5
Expand All @@ -102,7 +107,8 @@ services:
orthanc-anon:
build:
context: .
dockerfile: ./docker/orthanc-anon/Dockerfile
dockerfile: ./docker/orthanc/Dockerfile
target: pixl_orthanc_anon
args:
<<: *build-args-common
ORTHANC_CONCURRENT_JOBS: ${ORTHANC_CONCURRENT_JOBS}
Expand Down Expand Up @@ -167,7 +173,8 @@ services:
orthanc-raw:
build:
context: .
dockerfile: ./docker/orthanc-raw/Dockerfile
dockerfile: ./docker/orthanc/Dockerfile
target: pixl_orthanc_raw
args:
<<: *build-args-common
ORTHANC_RAW_MAXIMUM_STORAGE_SIZE: ${ORTHANC_RAW_MAXIMUM_STORAGE_SIZE}
Expand Down Expand Up @@ -245,8 +252,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:
<<:
Expand Down Expand Up @@ -285,29 +294,30 @@ 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
networks:
- pixl-net
environment:
<<: [*pixl-rabbit-mq, *proxy-common, *pixl-common-env]
<<: [*pixl-rabbit-mq, *proxy-common, *pixl-common-env, *pixl-db]
ORTHANC_RAW_URL: ${ORTHANC_RAW_URL}
ORTHANC_RAW_USERNAME: ${ORTHANC_RAW_USERNAME}
ORTHANC_RAW_PASSWORD: ${ORTHANC_RAW_PASSWORD}
Expand All @@ -316,11 +326,6 @@ services:
PRIMARY_DICOM_SOURCE_MODALITY: ${PRIMARY_DICOM_SOURCE_MODALITY}
SECONDARY_DICOM_SOURCE_MODALITY: ${SECONDARY_DICOM_SOURCE_MODALITY}
SKIP_ALEMBIC: ${SKIP_ALEMBIC}
PIXL_DB_HOST: ${PIXL_DB_HOST}
PIXL_DB_PORT: ${PIXL_DB_PORT}
PIXL_DB_NAME: ${PIXL_DB_NAME}
PIXL_DB_USER: ${PIXL_DB_USER}
PIXL_DB_PASSWORD: ${PIXL_DB_PASSWORD}
PIXL_DICOM_TRANSFER_TIMEOUT: ${PIXL_DICOM_TRANSFER_TIMEOUT}
PIXL_QUERY_TIMEOUT: ${PIXL_QUERY_TIMEOUT}
PIXL_MAX_MESSAGES_IN_FLIGHT: ${PIXL_MAX_MESSAGES_IN_FLIGHT}
Expand Down
46 changes: 0 additions & 46 deletions docker/hasher-api/Dockerfile

This file was deleted.

50 changes: 0 additions & 50 deletions docker/imaging-api/Dockerfile

This file was deleted.

52 changes: 0 additions & 52 deletions docker/orthanc-anon/Dockerfile

This file was deleted.

Loading
Loading