Adjustments to allow for read-only #39
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enhances Docker security and read-only filesystem compatibility by refactoring the container entrypoint and user management. The changes introduce multiple deployment methods to support both traditional PUID/GUID setups and security-hardened environments with read-only filesystems.
Key changes:
- Refactored entrypoint script with modular functions for user mapping, directory setup, and privilege management
- Added comprehensive security-hardened Docker Compose configuration with read-only filesystem support
- Updated environment variable naming from PGID to GUID for consistency
- Created extensive documentation for PUID/GUID configuration methods
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| docker-entrypoint.sh | Complete rewrite with modular functions supporting read-only filesystems and enhanced security |
| docker-compose.yml | Updated PGID to GUID and reorganized environment variables for clarity |
| docker-compose.security.yml | New security-hardened configuration with read-only filesystem, capability restrictions, and resource limits |
| PUID_GUID_GUIDE.md | Comprehensive documentation covering multiple deployment methods and troubleshooting |
| Dockerfile | Added build-time user creation and ownership settings for security hardening |
| .env.example | Updated PGID to GUID and added performance configuration examples |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| is_readonly_fs() { | ||
| # Try to create a test file in /tmp to check if filesystem is writable | ||
| touch /tmp/.write-test 2>/dev/null && rm -f /tmp/.write-test 2>/dev/null | ||
| return $? | ||
| } |
There was a problem hiding this comment.
The function name is_readonly_fs() suggests it returns true when the filesystem is read-only, but the logic returns true when the filesystem is writable. This creates confusion as the return value is opposite to what the name implies.
| postgres_data: | ||
| driver: local | ||
| mariadb_data: | ||
|
|
There was a problem hiding this comment.
There's an extra blank line between the volume name and its configuration. This should be removed for consistency with the other volume definitions.
| # These should be mounted as volumes in production | ||
| ensure_writable_dirs() { | ||
| # Only attempt directory creation if we can write | ||
| if is_readonly_fs; then |
There was a problem hiding this comment.
Due to the inverted logic in is_readonly_fs(), this condition will execute read-only filesystem handling when the filesystem is actually writable, and vice versa. The function should be renamed to is_writable_fs() or the logic should be inverted.
No description provided.