Refactor Dockerfile by crosspolar · Pull Request #712 · SuperCoopBerlin/tapir

crosspolar · 2025-11-20T09:45:42Z

I've added a new dockerfile for prod

without dev to remove those dependencies from the final docker image
non-root-user

This reverts commit 62ad5f8.

Theophile-Madet · 2025-11-26T14:48:15Z

Looks great, I don't have time to test properly today but I keep it on my list.

It should make the images smaller which will make deployment faster, that's great.

…r not

.github/workflows/docker-build.yml

Theophile-Madet

When I run docker compose build web (with the one backslash removed), then up -d web, I get the following error:

web-1  | The po files under /app/tapir/translations/locale/de/LC_MESSAGES are in a seemingly not writable location. mo files will not be updated/created.
web-1  | processing file django.po in /app/tapir/translations/locale/de/LC_MESSAGES
web-1  | CommandError: compilemessages generated one or more errors.

django.Dockerfile

crosspolar · 2025-12-16T21:34:04Z

@Theophile-Madet : I think the mounted volume in the compose-file overwrites the already copied files and their permissions?! I moved the compilemessage into the dockerfile, what do you think? Do we have other write-operations?

crosspolar · 2025-12-16T21:35:45Z

While I feel comfortable using docker, I still do mistakes. So I should be transparent, that I've used GPT5-mini for assistance (with which I mostly feel less comfortable, ironically)

Theophile-Madet · 2026-01-06T10:33:29Z

I think the mounted volume in the compose-file overwrites the already copied files and their permissions?!

It does, that's what we need: the files from the host machine being copied in the container when we develop, and the other way around if the container generates files.
The permission changes are sometimes annoying but I don't see a way around it?

I moved the compilemessage into the dockerfile, what do you think?

I think it's fine in the dockerfile 👍

Do we have other write-operations?

At what point? During development we write all the time.

I think we need Poetry to be installed in the dev version so that we can add or update packages from inside the container.

I'm not experienced with Docker either, we'll have to do what we can!

# Conflicts: # poetry.lock # pyproject.toml

techge

Okay I had a complete look at it. But being honest, I do not feel competent to really evaluate the code. Much of it has to do with the tapir setup and environment details that I am not familiar with. What I have seen and what I did understand, did not look insane though.

So... LGTM in a way 😬 I am sorry.

techge · 2026-02-15T18:19:35Z

.github/workflows/checks.yml

        run: docker compose run web make check-formatting
      - name: Check translations
-        run: docker compose run web make check-translations
+        run: docker compose run --user=$(id -u):$(id -g) web make check-translations


Why is the user defined here and not in the Dockerfile/Docker Compose? I am not at all familiar in how to do it the right way, so this is not complaint, I just do not understand, why using it here and not on other places as well/in the main definition.

In docker-compose it's luck I guess that user on docker and my machine is the same, but github was different. If your local machine would be different you would need an .env (I will add the documentation) and maybe better here would be UID=$(id -u) GID=$(id -g) docker compose run -v .:/app --rm web make test

I meant why don't we define the user/group in the docker-compose.yml instead of in the cli?

I haven't managed to make that work yet. It would need to take UID/GID from host environment during build right? btw, how does this work on windows?

I see. Well, as I said I don't know much about it, just wanted to check, if you thought about that. You clearly did, so it is what it is 🤷 🙃 Thank you again for your efforts!

So, $(id -u) is the ID of the user that is running the command, right?

So when I run the thing locally it's going to use my ID and on the github runner it's going to be the ID of the runner's user. Since the default user ID is 1000 we usually have the same ID for our local user and for nonroot but if my local user has another ID then the command is run inside the container as a non-existing user, which somehow works but is not what we want.

Wouldn't we want to use the nonroot user that is defined in the dockerfile anyway? That user is used by default if we don't specify --user. What kind of errors do you get when you don't specify --user?

My understanding is this: if you share files from the Docker volume, e.g., all Python files locally or in GitHub Actions, e.g., .coverage, nonroot needs the same UID as the host, otherwise there will be access problems. If this non-root user does not match the UID of the host user who owns the files in the volume, that non-root user will not have permissions to read or write to those files.

That is why for some occasions we have to either sync UID between Host and nonroot or override --user. I have to admit, currently is just a mess of both...

With your suggestion Theo, we would need to set UID/GID of nonroot to the host's user. This would have to be passed as an argument or via the environment.

we would need to set UID/GID of nonroot to the host's user

Is that not already done with ARG UID=1000 ARG GID=1000 in the Dockerfile and UID: ${UID:-1000} GID: ${GID:-1000} in the docker-compose? Then we're sure that we already have nonroot in sync with the host and we can skip all the --user?

I thought so too, but for some reason it didn't work

.github/workflows/tests.yml

django.Dockerfile

techge · 2026-02-15T18:34:27Z

The SonarCloud complaint about the tmp directory is not so bad in our environment. Would not hurt to still use a proper subdirectory as explained in the SonarCloud thingy. You never know what we'll do in the future.

crosspolar · 2026-02-22T10:38:30Z

@techge thanks for your feedback. I removed the "tmp"-directory now for members-current and used tempfile.TemporaryFile instead of tempfile.NamedTemporaryFile, so the filename should be more random (according to SonarCloud). For the tests, I haven't changed but don't see a problem here.

techge · 2026-02-22T11:34:43Z

I removed the "tmp"-directory now for members-current and used tempfile.TemporaryFile instead of tempfile.NamedTemporaryFile, so the filename should be more random (according to SonarCloud).

As I said I don't think this is thaaaat relevant for us, still, I think you missed the point. The /tmp folder is typically readable by every user. Using a subfolder like /tmp/tapir would prevent that, because this would be created only readable by the current user. This seems to be the reason SonarCloud is complaining about the last two bits.

Theophile-Madet

We need to clarify the --user-thing, I think it's not doing what's expected, see my comment on the corresponding thread.

Also, I think poetry should be installed in the runtime container so that we can update the packages from inside the container.

django.Dockerfile

tapir/accounts/tests/test_purchase_tracking_setting.py

Theophile-Madet · 2026-02-23T11:23:33Z

tapir/accounts/tests/test_purchase_tracking_setting.py

-    def test_updatePurchaseTrackingList_userHasTrackingDisabled_userIsNotInExportFile(
-        self,
+    @patch("tempfile.TemporaryFile")
+    def test_updatePurchaseTrackingList_userswithTrackingEnabledOrDisabled_includedOrExcludedInExportFile(


This should be two separate tests

why, it's a nice binary test case

Theophile-Madet · 2026-02-23T11:40:48Z

.github/workflows/checks.yml

        run: docker compose run web make check-formatting
      - name: Check translations
-        run: docker compose run web make check-translations
+        run: docker compose run --user=$(id -u):$(id -g) web make check-translations


So, $(id -u) is the ID of the user that is running the command, right?

So when I run the thing locally it's going to use my ID and on the github runner it's going to be the ID of the runner's user. Since the default user ID is 1000 we usually have the same ID for our local user and for nonroot but if my local user has another ID then the command is run inside the container as a non-existing user, which somehow works but is not what we want.

Wouldn't we want to use the nonroot user that is defined in the dockerfile anyway? That user is used by default if we don't specify --user. What kind of errors do you get when you don't specify --user?

django.Dockerfile

Co-authored-by: Théophile MADET <theo.madet@posteo.net>

# Conflicts: # django.Dockerfile # poetry.lock # pyproject.toml

crosspolar · 2026-03-29T15:46:36Z

Wouldn't we want to use the nonroot user that is defined in the dockerfile anyway? That user is used by default if we don't specify --user. What kind of errors do you get when you don't specify --user?

https://github.com/SuperCoopBerlin/tapir/actions/runs/23712692142/job/69074844974?pr=712

You can also specifiy UID and GID during build, like I do now. If you want to use volumes for developing or share files such as translation or coverage files, you have to specifiy UID and GID now with

export USER_UID=$(id -u)
export USER_GID=$(id -g)

crosspolar added 4 commits November 20, 2025 09:32

build: Create dockerfile without

62ad5f8

move drf-spectacular and icecream to prod

1b5f41b

add dockerfile for prod

3509e63

Revert "build: Create dockerfile without"

d5bf3b8

This reverts commit 62ad5f8.

crosspolar requested a review from Theophile-Madet November 20, 2025 09:45

crosspolar force-pushed the dockerfile_without_poetry_dev_dependencies branch from b7bf055 to d5bf3b8 Compare November 24, 2025 07:07

crosspolar added 4 commits November 24, 2025 10:48

remove poetry and initiate first-attempt of multi-stage and add user...

590468e

linux reduce installed packages

8b3d1c1

comments

a14ec3c

use python-slim with weasyprint-dependencies

5548afd

crosspolar added 4 commits November 26, 2025 18:12

set DEV in Dockerfile to decide whether to install dev-dependencies o…

cef345a

…r not

install poetry the curl-way (more recommended)

629e4f0

remove unneeded COPY

16b6f71

smaller apt-get in second layer

8f1735d

crosspolar changed the title ~~Dockerfile without poetry dev dependencies~~ Refactor Dockerfile Nov 28, 2025

crosspolar added the high priority label Dec 14, 2025

Theophile-Madet reviewed Dec 15, 2025

View reviewed changes

.github/workflows/docker-build.yml Outdated Show resolved Hide resolved

Theophile-Madet requested changes Dec 16, 2025

View reviewed changes

django.Dockerfile Outdated Show resolved Hide resolved

crosspolar added 2 commits December 16, 2025 21:57

remove \

6e36394

remove obsolete prod reference

3a823cb

compilemessages in dockerfile

c64a61e

crosspolar added 5 commits February 1, 2026 10:27

Merge branch 'master' into dockerfile_without_poetry_dev_dependencies

33b7099

# Conflicts: # poetry.lock # pyproject.toml

poetry lock

01fd77f

use uid 1000

97dc198

have git installed in container

ca31ccb

update translation

47184be

crosspolar requested review from Theophile-Madet and techge February 8, 2026 19:15

techge reviewed Feb 15, 2026

View reviewed changes

django.Dockerfile Outdated Show resolved Hide resolved

Merge branch 'master' into dockerfile_without_poetry_dev_dependencies

bbead6b

crosspolar force-pushed the dockerfile_without_poetry_dev_dependencies branch from 03685ab to bbead6b Compare February 21, 2026 10:27

crosspolar added 7 commits February 21, 2026 11:28

omit -L when installing poetry

d9171ed

merge tests

ce7efda

remove unlink

7be9980

remove tmp

e7fd7d5

make sure members-current is written to correct target file

a88b14f

remove unused EXPORT_FILE

9f9f62b

use tempfile.TemporaryFile

0d214f7

Theophile-Madet requested changes Feb 23, 2026

View reviewed changes

crosspolar and others added 10 commits February 27, 2026 09:42

Update django.Dockerfile

3389b9f

Co-authored-by: Théophile MADET <theo.madet@posteo.net>

Merge branch 'master' into dockerfile_without_poetry_dev_dependencies

afcf5e2

# Conflicts: # django.Dockerfile # poetry.lock # pyproject.toml

poetry lock

dd04093

use TapirUserFactory.create()

0ca6877

fix: duplicated args in compose

a26ee07

remove --user=$(id -u):$(id -g)

76a6dea

you need to export GID AND UID

30cf98f

you need to export GID AND UID to checks.yml too

a23c0d6

set default in compose not in dockerfile

a24d3e5

set USER_UID and USER_GID

9426724

crosspolar requested review from Theophile-Madet and techge March 29, 2026 15:56

Conversation

crosspolar commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Theophile-Madet commented Nov 26, 2025

Uh oh!

Uh oh!

Theophile-Madet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

crosspolar commented Dec 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

crosspolar commented Dec 16, 2025

Uh oh!

Theophile-Madet commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

techge left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crosspolar Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

techge commented Feb 15, 2026

Uh oh!

crosspolar commented Feb 22, 2026

Uh oh!

techge commented Feb 22, 2026

Uh oh!

Theophile-Madet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

crosspolar commented Mar 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

crosspolar commented Nov 20, 2025 •

edited

Loading

crosspolar commented Dec 16, 2025 •

edited

Loading

Theophile-Madet commented Jan 6, 2026 •

edited

Loading

crosspolar Feb 27, 2026 •

edited

Loading

crosspolar commented Mar 29, 2026 •

edited

Loading