fix: customize container user permissions using PUID and PGID. #9657#9833
Open
fix: customize container user permissions using PUID and PGID. #9657#9833
Conversation
Add support for custom container user permissions via PUID and PGID environment variables. When the container is started as root (--user root), the pgadmin user is reassigned to the requested UID/GID and all initialization runs under that user via su-exec, ensuring files are created with correct ownership from the start. Key changes: - Dockerfile: add su-exec package, add chmod g=u for /run/pgadmin (fixes OpenShift random UID access) - entrypoint.sh: add PUID/PGID validation and privilege dropping before initialization (not after), preserving OpenShift compatibility Three modes supported: - Default (USER 5050): unchanged behavior - Custom UID (--user root -e PUID=N -e PGID=N): drops to target user before any init - OpenShift (random UID, GID 0): passwd fixup + group permissions
WalkthroughThe pull request modifies Docker configuration and the pgAdmin entrypoint script to support non-root container execution via PUID/PGID environment variables. It adds the Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pkg/docker/entrypoint.sh (2)
240-240: Consider using parameter expansion instead of sed.Per static analysis hint (SC2001), bash parameter expansion is more efficient and avoids spawning a subprocess:
♻️ Suggested refactor
- PGADMIN_USER_CONFIG_DIR=$(echo "${PGADMIN_DEFAULT_EMAIL}" | sed 's/@/_/g') + PGADMIN_USER_CONFIG_DIR="${PGADMIN_DEFAULT_EMAIL//@/_}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/docker/entrypoint.sh` at line 240, The assignment to PGADMIN_USER_CONFIG_DIR uses sed and spawns a subprocess; change it to use bash parameter expansion to replace all '@' characters in PGADMIN_DEFAULT_EMAIL with underscores (use the parameter expansion pattern that substitutes globally, e.g. the ${VAR//@/_} form) so PGADMIN_USER_CONFIG_DIR is set without invoking sed.
47-52: Consider documenting potential startup delay for large data volumes.The recursive
chown -Ron/var/lib/pgadminwill traverse all files in the volume. For containers with significant stored data (sessions, query history, server groups), this could noticeably delay startup when using custom PUID/PGID.Consider adding a note to the PR documentation or a log message indicating this is expected behavior:
# Fix ownership of runtime directories BEFORE any initialization for dir in /run/pgadmin /var/lib/pgadmin; do if [ -d "$dir" ]; then + echo "Fixing ownership of $dir (may take time for large volumes)..." chown -R "$PUID:$PGID" "$dir" fi done🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/docker/entrypoint.sh` around lines 47 - 52, Add a brief startup warning before the recursive ownership fix in entrypoint.sh: before the for-loop that runs chown -R on /run/pgadmin and /var/lib/pgadmin, emit a log message (e.g., printf or echo to stderr) that indicates recursive chown may take significant time on large volumes and includes the PUID/PGID values; optionally, in the same change consider guarding the chown with a quick ownership check (stat/ls -n) to skip chown if owner/group already match to avoid unnecessary delays.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/docker/entrypoint.sh`:
- Line 240: The assignment to PGADMIN_USER_CONFIG_DIR uses sed and spawns a
subprocess; change it to use bash parameter expansion to replace all '@'
characters in PGADMIN_DEFAULT_EMAIL with underscores (use the parameter
expansion pattern that substitutes globally, e.g. the ${VAR//@/_} form) so
PGADMIN_USER_CONFIG_DIR is set without invoking sed.
- Around line 47-52: Add a brief startup warning before the recursive ownership
fix in entrypoint.sh: before the for-loop that runs chown -R on /run/pgadmin and
/var/lib/pgadmin, emit a log message (e.g., printf or echo to stderr) that
indicates recursive chown may take significant time on large volumes and
includes the PUID/PGID values; optionally, in the same change consider guarding
the chown with a quick ownership check (stat/ls -n) to skip chown if owner/group
already match to avoid unnecessary delays.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1990768e-8eff-4a35-a7c5-4eeb2d7f4585
📒 Files selected for processing (2)
Dockerfilepkg/docker/entrypoint.sh
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Re-implements custom container user permissions via
PUIDandPGIDenvironment variables, fixing the issues that caused the original PR #9633 to be reverted (#9690).Fixes #9657
What changed
Dockerfile
su-execpackage for privilege droppingchmod g=u /run/pgadmin— was missing, required for OpenShift random UIDs to write to this directoryentrypoint.sh
PUID/PGIDenvironment variable support with input validation (must be numeric,PUID=0rejected)--user root): reassignspgadminuser to target UID/GID, fixes ownership of runtime directories before any initialization, runs all commands viasu-execchownwhen running as root$SU_EXECto prevent root-owned file creationWhy the original PR was reverted
The original implementation (#9633) had three critical issues:
USER 5050from Dockerfile — container ran as root by default (security regression)run_pgadmin.pyandload-serverscreated root-owned files in/var/lib/pgadmin. Thechown -Rhappened after init, so if anything failed between init and chown, the volume was left root-owned and inaccessibleusermod/chown, rejected by OpenShift'srestricted-v2SCCHow this fix addresses each issue
USER 5050is preserved — default behavior is completely unchangedrun_pgadmin.py,load-servers,set-prefs) run as the target user viasu-exec, so files are created with correct ownership from the start. No post-hocchownfixup needed for init files.Three supported modes
id -u5050SU_EXEC="", all commands run directly as UID 5050.docker run --user root -e PUID=1000 -e PGID=1000 ...0usermods pgadmin user,chowns runtime dirs, then drops to target user viasu-execfor all init + gunicorn.1001320000SU_EXEC="", passwd fixup adds entry for random UID, files accessible via group permissions (chmod g=u+ GID 0). Nochownneeded.OpenShift compatibility details
On OpenShift with
restricted-v2SCC:USER 5050with a random UID from the namespace range (e.g.,1001320000-1001329999)id -ureturns the random UID, so theelsebranch is takenchmod g=uin the Dockerfile, making them group-writable by GID 0/etc/passwdentry for the random UID (needed forwhoami, Python'sos.getlogin(), etc.)chownis needed — file access works through group permissionsrunAsUser: 5050orfsGroup: 5050in the pod spec — let the SCC assign the UIDThe reported error
.spec.securityContext.fsGroup: Invalid value: []int64{5050}: 5050 is not an allowed groupoccurs when the pod spec explicitly setsfsGroup: 5050, which the SCC rejects. The fix is to remove those fields from the pod spec.Usage examples
Test plan
--user 1001320000:0): container starts, pgAdmin accessible--user root -e PUID=1000 -e PGID=1000): container starts, files owned by 1000:1000Summary by CodeRabbit
Release Notes
Bug Fixes
Chores