Skip to content

Conversation

@theProf
Copy link

@theProf theProf commented Aug 8, 2025

Description

Refactor the Dockerfile for readability, while optimizing for a slimmer final image by reducing the layers and stripping the build dependencies from the production image

Background

I wanted to start contributing to issues, but got side tracked during setup. This is a work in progress as it will require more testing. The image itself relies on external dependencies not included in the Dockerfile (looking at the CI pipelines) and since I haven't got the dev environment up and running yet, unsure which dependencies are solely for building and which are required for production, but err-ed on the side of caution rather than optimization.

Discussion

Created an Issue for larger discussion that can be found here

Questions

I didn't see anywhere that was tagged with contribution / discussion around DevOps, CI/CD, building, etc, but if I missed it please feel free to point me in the correct direction!

Relevant Links

  • The original PR was against a branch that was merged and closed. Couldn't figure out how to re-open and re-target the base, so I've just opened a new PR.
  • I like the work being done over in another optimization PR. I'm happy to incorporate the work, or wait until it is merged to rebase this PR. One issue I'll want to test with this solution is the user permissions (if not running as root). I was happy with nginx not being incorporated into the image itself (i use traefik as a reverse proxy), but if people like it incorporated, it may be more flexible to run it as a service rather than part of a boot.sh. s6 overlay has been great for this kind of thing

@vabene1111
Copy link
Collaborator

thanks for the update. I will merge #3904 because it is kinda needed/a bugfix. After that I will review your PR. Please note that I made some changes to the build/container setup to allow for the integrated nginx. once I merged the other PR it would be great if you could check that everything is still working as expected.

Also with the change to non root user: we need to be 1000% sure that this does not break anything like media file access or what not. As far as I am aware it is not needed/not heavily recommended to run the container as a non root users, its just a nice to have and I simply don't have the capacity for any kind of headache for something that is not absolutely needed.

@vabene1111
Copy link
Collaborator

ok so the other changes to the docker file are done. Can you update your PR?

Also what about the non root user, are we sure that this works 100%?

@theProf
Copy link
Author

theProf commented Aug 28, 2025

ok so the other changes to the docker file are done. Can you update your PR?

Also what about the non root user, are we sure that this works 100%?

Thanks, I will update when I get the chance. The nginx changes aren't tested yet and I assume they're breaking changes to what's here. A multi-service image will be more complex permission wise than the previous single service. nginx will likely want to be root so just setting a user: 1000:1000 field would likely fail on startup.

@ignas2526
Copy link

ignas2526 commented Nov 8, 2025

Do not add things to the Dockerfile that should be done on the host. Things like addgroup and adduser are bad. Just let everyone set their own UID and GID in the compose. Also, hardcoded USER 1000:1000 is bad. I run every container with its own user and group since its more secure.

The point of running a container as non-root is to give it fewer privileges, not more. The container should do the least amount of things. I've been running Tandoor Recipes as a non-root, read-only, with all capabilities dropped for a couple of months now, after doing some minor adjustments to the Dockerfile and boot.sh.

Since FS is read-only, run folder cannot be modified, so in boot.sh nginx -> nginx -g "pid /tmp/nginx.pid;" and /run/tandoor.sock -> tmp/tandoor.sock.

I've created user and group on the host groupadd --system do-recipes-app && useradd --system --gid do-recipes-app --no-create-home --shell /sbin/nologin do-recipes-app. Next I set these on the host os export D_RECIPES_APP_UID=$(id -u do-recipes-app); export D_RECIPES_APP_GID=$(id -g do-recipes-app) I have a sh file in the profile.d so it gets set on the startup.

I removed envsubst '$MEDIA_ROOT $STATIC_ROOT $TANDOOR_PORT' < /opt/recipes/http.d/Recipes.conf.template > /opt/recipes/http.d/Recipes.conf and replaced it with a static /opt/recipes/http.d/Recipes.conf. Since entire FS is now read-only and I don't see the point for specifying different mounting paths inside the container.

Here's my docker compose...

services:
  app:
    image: vabene1111/recipes:2.3.3
    user: ${D_RECIPES_APP_UID}:${D_RECIPES_APP_GID}
    container_name: recipes_app
    restart: unless-stopped
    env_file: .env
    volumes:
      - recipes_mediafiles:/opt/recipes/mediafiles
      - recipes_staticfiles:/opt/recipes/staticfiles
    tmpfs:
      - /tmp:mode=770,size=64m,uid=${D_RECIPES_APP_UID},gid=${D_RECIPES_APP_GID}
      - /var/lib/nginx/logs:mode=770,size=64m,uid=${D_RECIPES_APP_UID},gid=${D_RECIPES_APP_GID}
    deploy:
      resources:
          limits:
            cpus: '0.5'
            memory: 1G
    cap_drop:
      - ALL
    read_only: true
    depends_on:
      - db

volumes:
  # DO NOT run docker volume create.
  # docker container run --rm -v recipes_mediafiles:/data debian:bookworm /bin/sh -c "touch /data/.initialized && chown -R ${D_RECIPES_APP_UID}:${D_RECIPES_APP_GID} /data"
  # docker container run --rm -v recipes_staticfiles:/data debian:bookworm /bin/sh -c "touch /data/.initialized && chown -R ${D_RECIPES_APP_UID}:${D_RECIPES_APP_GID} /data"
  recipes_mediafiles:
    external: true
  recipes_staticfiles:
    external: true

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants