-
Notifications
You must be signed in to change notification settings - Fork 5
FEAT: Add PUID and GUID Environment Variable Support #79
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
Conversation
- Remove: UserAdd function from Dockerfile - Remove: extra run directive - Add: shadow packge to runtime for adduser, addgroup, and su
- Accepts PUID and PGID Environment Variables. - Default PUID and PGID to 1001 to maintain backwards compatability and make variables optional - Checks for existing group and ID, then creates/recreates if it does not exist or match PGID - Checks for existing user, id, and default group id, then creates/recreates as necessary - Changes ownership of /opt/imapfilter/config and /home/imapfilter
**FEAT: Add PUID and GUID Environment Variable Support** * **Renames original `entrypoint.sh` to `run-imapfilter.sh`** * **Remove UserAdd from Dockerfile, and remove extra run** * **Remove**: UserAdd function from Dockerfile * **Remove**: extra run directive * **Add**: shadow package to runtime for adduser, addgroup, and su * **New entrypoint handles PUID and PGID Variables** * Accepts PUID and PGID Environment Variables. * Default PUID and PGID to 1001 to maintain backwards compatibility and make variables optional * Checks for existing group and ID, then creates/recreates if it does not exist or match PGID * Checks for existing user, id, and default group id, then creates/recreates as necessary * Changes ownership of /opt/imapfilter/config and /home/imapfilter
Updated readme to include PUID and PGID environment variables. Signed-off-by: Vylyne <[email protected]>
added PUID and PGID to docker-compose.yaml example. Signed-off-by: Vylyne <[email protected]>
okay this one should stay. be able to stay as is sorry about all the noise. |
updated copy to match new docker-entrypoint.sh name Signed-off-by: Vylyne <[email protected]>
Added ignore to ending dockerfile as root user since user is not yet created and switch happens after Signed-off-by: Vylyne <[email protected]>
Thank you for the PR, however that would break any convention surrounding modern user management. Why do you want this change? If you want to run the image as root you can tell docker ( PUID/GUID are only present in very very old images from when docker didn't have any scaffolding for user management and every container process was always root in its cgroup or in images that require a very specific setup, like starting multiple processes in on container as different users - something especially mail distributions seems to like to do. You can also look into these two pages for the modern best practices on docker images: |
Sorry, long story short, I've not yet found a better what that works, to allow the service inside the docker container to work with files that are not inside the docker container. When I tried to run this under a different user with the user directive it could not read or write or write anything. Thanks for your reply, we can close this, and I can keep this for my own use, if I actually need it, but I have after making a bunch of changes to your entrypoint.sh script been able to make github work for my config at least so I don't need the config file mounts myself anymore. |
Here's what I was trying to fix. with this compose: services:
imapfilter:
user: 568:568
container_name: imapfilter
restart: always
image: ntnn/imapfilter
environment:
IMAPFILTER_CONFIG: imapfilter.lua
IMAPFILTER_CONFIG_BASE: /config
IMAPFILTER_DAEMON: yes
volumes:
- ./config:/config and a config that is owned 568:568 with read and write permissions on user and group: /entrypoint.sh: cd: line 81: can't cd to /config: Permission denied
The file 'imapfilter.lua' does not exist relative to '/config', exiting
Please validate IMAPFILTER_CONFIG |
Mhm, that should work. Do you have AppArmor or SELinux enabled? Those would interfere with that, but there are ways around that. |
Actually to my understanding it really shouldn't work. Basically the user directive overrides what uid/gid is running the entrypoint. It doesn't actually do anything inside the image to make it work either. So if that user id that I randomly pick to run your container as won't likely exist in the container. So it doesn't work for anything that "Other" doesn't have access It can't even read the secret files as they're just bind mounted. Annoyingly enough. So I do have a new dockerfile/PR I was going to offer you, but I found another bug which I wasn't sure if this introduced it or not until I understood why that bug was happening so I didn't give you that PR yesterday... I've created a Draft PR of that as it is now. It is still a security compromise to an extent but. It's a much different one. We're only opening up the files we need to be able to write to be writable by any userid in the container. But with the other path you come in through exec you have root, this does not grant that to any process. Its the best I could come up with that would allow use to build the image to support the user directive and allow the user to control what uid:gid is reading and writing the files short of forcing them to build the image themselves. Which this build file does support defining the uid and gid at build time too. I thought about putting a check in the docker file to see if they were arguments were specified or not, and only preform the chmods if we're using an image that doesn't define them. That's a relatively simple change. hell could probably expand on the build action to add an image that is locked down to 1001 and the user directive will break it if you wanted. And one that's user "compliant" for lack of a better word. The bug is unrelated to the user directive. So You can take that if you want. The bug I'm working on is when the config is updated via GIT the script fails to kill the imapfilter process. Basically the process the script is tracking is the subshell running the application not the application. The subshell should close when the application is closed so in my dev I'm moving it to just poll the services rather then remembering the service as it's in a docker container and I'm thinking it's safe to just assume that any imapfilter services running in the container are controlled by this entrypoint script. I've currently got both in https://github.com/Vylyne/docker-imapfilter/tree/dev/user But I've got a bit more testing and running to do on that before I'd offer that as a new PR if you want it. The there's another way where you can change the kill command to kill the group. But I didn't think it necessary to go down that path, feeling that a catchall solution is a little more robust given the constraints of a docker container there's no need to protect against killing a separate imapfilter process. |
Linux doesn't care if a uid/gid exist in shadow - if a process is run with uid 1234 and gid 5678 then the process will run with that, regardless of the uid/gid is recorded in shadow.
That doesn't matter - when the shell is terminated the child processes are terminated as well. What you are seeing is likely that your imapfilter config refuses to stop for some reason or another. What could also be is that you are running the script in daemon mode but your config is not in daemon mode. I'd generally not recommend mounting a directory in a container unless it's for testing or development. It's a whole host of issues (parts of which you see^^) and rarely secure. E.g. I run my imapfilter in kube without any volume and just let it clone the config. |
I appreciate the distinction regarding how the Linux kernel handles UIDs/GIDs at the process level, especially in environments utilizing external identity providers like Active Directory. That's a crucial point for understanding the kernel's flexibility. However, in the context of persistent filesystem access, particularly with bind mounts, the absence of a local UID/GID mapping can still lead to permission issues. Specifically, if a container runs as an unmapped UID (e.g., 1234), the filesystem will treat it as a numeric ID, and the process will be unable to read or write to files/folders whose permissions require a different UID or GID. This is the practical constraint that often necessitates adjusting permissions or ensuring the container runs as a mapped user, particularly when interacting with sensitive mounted secrets/directories/files. Or accessing files and process inside the container when the user directive is used.
I understand the standard behaviour where process trees are typically terminated upon shell exit. In my environment, however, I consistently observed imapfilter sessions persisting after the subshell has been killed, suggesting the process wasn't receiving or handling the termination signal correctly, and not being perhaps being forcibly closed either. While I will investigate potential misconfiguration on my end, I suggest that implementing a specific termination mechanism within the script for uncooperative child processes would enhance the script's robustness against this class of error.
I agree with your general sentiment regarding the best practice of minimizing bind mounts for production environments, and I'm actively exploring alternatives to docker like Kubernetes and Podman, which each present their own trade-offs (secrets management, volume access, management, etc.) to see if there's a better way to get around these common issues. When I mention bind mounts, it's typically for specific containers that require optimized I/O latency on my single-host environment, a common pragmatic choice when dealing with local resources not exposed to the public internet. Regarding the image's current design, the core challenge I encountered is its interaction with docker secrets and permissions. To make secrets available to the process running as UID 1001 (or any unmapped user):
This experience led me to confirm a few practical issues that might affect other users employing non-standard secrets management:
The Dockerfile changes addressing the configuration cloning and user permission challenges are available in a draft Pull Request(#82). This PR provides a concrete demonstration of the fixes I've discussed on the filesystem and access side of things. I'm perfectly content running my own fork and image with these necessary adjustments, but I offer the PR as a resource; feel free to adopt the changes in full, select specific parts, or simply use it for reference. I respect your ultimate decision on your project's image/code, and thank you for your contributions. |
FEAT: Add PUID and GUID Environment Variable Support
Renames original
entrypoint.sh
torun-imapfilter.sh
Remove UserAdd from Dockerfile, and remove extra run
New entrypoint handles PUID and PGID Variables
superseeds Feature: Add PUID and GUID Environment Variable Support #78