docker arbitrary user support via PUID and PGID#9633
docker arbitrary user support via PUID and PGID#9633kon-foo wants to merge 5 commits intopgadmin-org:masterfrom
PUID and PGID#9633Conversation
… su-exec to drop privileges
WalkthroughThe changes modify Docker container setup to add runtime dependencies (libcap, su-exec) and adjust user configuration. The entrypoint script now handles PUID/PGID environment variables for flexible user ID configuration, implements root-aware initialization with group/user reassignment, OpenShift passwd compatibility, and wraps Gunicorn invocations with su-exec wrapper for privilege management. Changes
Sequence Diagram(s)sequenceDiagram
participant Container as Container Runtime
participant Entrypoint as entrypoint.sh
participant System as System (groups/users)
participant Gunicorn
Container->>Entrypoint: Start (PUID/PGID env vars set)
Entrypoint->>Entrypoint: Check if running as root
alt Running as Root
Entrypoint->>System: Create group with PGID
Entrypoint->>System: Reassign pgadmin UID/GID
Entrypoint->>Entrypoint: Set SU_EXEC wrapper
else Not Root (OpenShift)
Entrypoint->>Entrypoint: Check /etc/passwd writable
alt Passwd Writable
Entrypoint->>System: Append pgadmin user entry
end
end
Entrypoint->>System: Adjust ownership of runtime paths
Entrypoint->>System: Adjust ownership of config files
Entrypoint->>Gunicorn: Invoke with SU_EXEC wrapper<br/>(preserving all flags)
Gunicorn-->>Container: Running
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@pkg/docker/entrypoint.sh`:
- Around line 199-201: The chown block should avoid errors for missing /certs,
avoid always-recursing /var/lib/pgadmin, and honor a custom config path; update
the block to: check existence of /certs before attempting chown, check current
owner of /var/lib/pgadmin (using stat) and only run chown -R if it differs from
"$PUID:$PGID" to avoid costly repeated recursion, and replace the hardcoded
/pgadmin4/config_distro.py with the PGADMIN_CUSTOM_CONFIG_DISTRO_FILE variable
(if set and the file exists) and chown that file non-recursively; ensure all
chown commands still run only when uid is 0.
🧹 Nitpick comments (3)
pkg/docker/entrypoint.sh (3)
5-19: Consider validatingPUID/PGIDare numeric before use.If a user sets
PUID=abc,usermodfails silently (line 12 swallows errors), thensu-exec abc:0will fail at the very end of the script with a cryptic error. Fail-fast with a clear message would improve the user experience.🛡️ Proposed validation
PUID=${PUID:-5050} PGID=${PGID:-0} + +# Validate PUID/PGID are numeric +if ! echo "$PUID" | grep -qE '^[0-9]+$'; then + echo "Error: PUID must be a numeric value, got '$PUID'" >&2 + exit 1 +fi +if ! echo "$PGID" | grep -qE '^[0-9]+$'; then + echo "Error: PGID must be a numeric value, got '$PGID'" >&2 + exit 1 +fi if [ "$(id -u)" = "0" ]; then
155-162: DB initialization runs as root before ownership is fixed.
run_pgadmin.py(line 157) andload-servers(line 113) execute as UID 0 when the container starts as root, creating files in/var/lib/pgadminowned by root. Thechown -Rat line 199 corrects this, but if the script fails between these points the volume is left root-owned.An alternative is to move the
chown+su-execwrapping earlier, so that the initialization itself runs as the target user. This would also eliminate the need for the expensive recursivechown.
12-12: Silentusermodfailure could mislead operators.
2>/dev/null || truesuppresses all diagnostics. Ifusermodfails for an unexpected reason (e.g., shadow/passwd locked), the script continues and line 16 still prints the intended UID/GID — but thepgadminuser was never actually reassigned. Gunicorn will still launch viasu-execwith the numeric UID (so it works), but any files created during init (lines 157–187) will be owned by the old pgadmin UID.Consider logging the failure:
Proposed change
- usermod -o -u "$PUID" -g "$PGID" pgadmin 2>/dev/null || true + if ! usermod -o -u "$PUID" -g "$PGID" pgadmin 2>/dev/null; then + echo "Warning: failed to reassign pgadmin user to UID=$PUID GID=$PGID" >&2 + fi
| if [ "$(id -u)" = "0" ]; then | ||
| chown -R "$PUID:$PGID" /run/pgadmin /var/lib/pgadmin /pgadmin4/config_distro.py /certs | ||
| fi |
There was a problem hiding this comment.
Multiple issues with the chown block.
-
/certsmay not exist when TLS is not enabled, producing a noisy error on every startup for non-TLS users. -
chown -Ron/var/lib/pgadminruns on every start and can be extremely slow for volumes with many files (e.g., large session/storage directories). Consider guarding with an ownership check or limiting recursion depth. -
Hardcoded
/pgadmin4/config_distro.py— Line 58 allows overriding the config distro path viaPGADMIN_CUSTOM_CONFIG_DISTRO_FILE. If a custom path is used, thischownwon't cover the actual config file.
🔧 Proposed fix addressing all three points
if [ "$(id -u)" = "0" ]; then
- chown -R "$PUID:$PGID" /run/pgadmin /var/lib/pgadmin /pgadmin4/config_distro.py /certs
+ chown -R "$PUID:$PGID" /run/pgadmin /var/lib/pgadmin
+ chown "$PUID:$PGID" "${CONFIG_DISTRO_FILE_PATH}"
+ if [ -d /certs ]; then
+ chown -R "$PUID:$PGID" /certs
+ fi
fiFor the /var/lib/pgadmin performance concern, a common pattern is to only recurse when ownership doesn't match:
if [ "$(stat -c '%u:%g' /var/lib/pgadmin)" != "$PUID:$PGID" ]; then
chown -R "$PUID:$PGID" /var/lib/pgadmin
fi🤖 Prompt for AI Agents
In `@pkg/docker/entrypoint.sh` around lines 199 - 201, The chown block should
avoid errors for missing /certs, avoid always-recursing /var/lib/pgadmin, and
honor a custom config path; update the block to: check existence of /certs
before attempting chown, check current owner of /var/lib/pgadmin (using stat)
and only run chown -R if it differs from "$PUID:$PGID" to avoid costly repeated
recursion, and replace the hardcoded /pgadmin4/config_distro.py with the
PGADMIN_CUSTOM_CONFIG_DISTRO_FILE variable (if set and the file exists) and
chown that file non-recursively; ensure all chown commands still run only when
uid is 0.
This PR adds support for
PUIDandPGIDenvironment variables as popularised by linuxserver.io images. This allows users to control the UID and GID of the process running gunicorn inside the container.Why
Currently, when running pgAdmin via Docker with bind-mounted volumes, the mounted directories must be owned by the hardcoded 5050 user on the host and inside the container. This creates two issues:
Permission Headaches:
Users are forced to manually chown directories on the host to 5050 before starting the container. This is cumbersome, especially if running docker as a non-root users and potentially lose access to the mounted dir.
Security Risks:
The hardcoded 5050 UID might already be in use on the host system. Running the container as this UID could unintentionally grant pgAdmin access to files or resources owned by that UID or vise versa.
Solution
UID0 by default.PUID5050 andGUID0 to keep current behaviour when no env vars are set."$(id -u)" = "0", aka no--userflag was used to run the image:PGIDexists and reassign the existing pgadmin user toPUIDandPGID.chownrequired files and dirs to said user.$PUID:$PGIDThe check for id=0 makes sure nothing changes for users, that use the
--userflag oruser:in compose to run the image.Compatability
Without PUID/PGID set, the behaviour is identical to the current image.
Tests
For each test, I cheked:
Further tests conducted:
Open Questions:
chowned? Currently/run/pgadmin,/var/lib/pgadmin,/certsandpgadmin4/config_distro.pyare covered.I have been using this for a couple of weeks now, but I am only using a very small subset of PgAdmin functionality.
Summary by CodeRabbit
Release Notes
New Features
Chores