-
Notifications
You must be signed in to change notification settings - Fork 494
fix: make Docker container UID/GID configurable #448
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
fix: make Docker container UID/GID configurable #448
Conversation
📝 WalkthroughWalkthroughThe pull request adds configurable build-time UID and GID arguments (defaulting to 1001) across Dockerfiles and docker-compose configurations to enable mapping container user IDs to host user IDs for mounted volumes. User creation commands are updated to use these dynamic values. Documentation is added explaining how to override these arguments during rebuild to fix file permission issues. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Summary of ChangesHello @comzine, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a common pain point in Docker development where file ownership conflicts arise when host directories are mounted into containers. By making the container's user and group IDs configurable, users can now easily synchronize these IDs with their host system, eliminating permission errors and streamlining workflows that involve shared volumes. This enhancement significantly improves the developer experience by ensuring consistent file access and modification capabilities. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Add UID and GID build arguments to Dockerfiles to allow matching the container user to the host user. This fixes file permission issues when mounting host directories as volumes. Default remains 1001 for backward compatibility. To match host user: UID=$(id -u) GID=$(id -g) docker-compose build Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
4507599 to
aa8caea
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request is a great improvement for handling file permissions with mounted volumes by making the container's UID/GID configurable. The changes to the docker-compose files and the documentation are well-implemented and clear. My main feedback is to improve the robustness of the user creation logic in the Dockerfile and Dockerfile.dev. The current implementation can fail for common UIDs (like 1000) that conflict with existing users in the base image. I've provided suggestions to handle this gracefully, ensuring the build process is more reliable for developers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docker-compose.dev-server.yml (1)
20-24: ConsiderUIDshell variable conflict on some systems.On bash shells,
UIDis a read-only built-in variable. While this works when usingUID=$(id -u)inline (which creates a subshell assignment), users attemptingexport UID=...will encounter errors. The current inline example in the comment is correct, but this could cause confusion.This is a minor ergonomic concern—the documented usage pattern works fine.
Dockerfile (1)
87-94: Implementation handles ID collisions correctly.The use of
-oflag is appropriate for this use case where the container user needs to match the host user's UID/GID. This correctly handles potential collisions with existing IDs in the base image (e.g., thenodeuser/group at 1000).Minor note: the comment mentions GID 1000 collision, but UID 1000 could also collide with the
nodeuser innode:22-slim. Consider updating the comment to mention both:📝 Suggested comment improvement
# Uses UID/GID build args to match host user for mounted volume permissions -# Use -o flag to allow non-unique IDs (GID 1000 may already exist as 'node' group) +# Use -o flag to allow non-unique IDs (UID/GID 1000 may already exist as 'node' user/group)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
DockerfileDockerfile.devdocker-compose.dev-server.ymldocker-compose.dev.ymldocker-compose.ymldocs/docker-isolation.md
🔇 Additional comments (7)
docker-compose.dev.yml (2)
21-25: LGTM!Build args correctly propagate UID/GID with appropriate defaults and clear documentation comment.
102-104: LGTM!UI service correctly mirrors the server service's UID/GID configuration, ensuring consistent user mapping across both containers.
Dockerfile.dev (1)
30-42: LGTM! Good use of-oflag for ID flexibility.The implementation correctly:
- Places ARGs after expensive operations to optimize layer caching
- Uses
-oflag to handle existing IDs (e.g., node group at GID 1000)- Documents the rationale for non-unique IDs
One edge case to be aware of: if a user passes
UID=0, the container user would effectively become root. This is likely acceptable since it requires explicit action, but consider documenting this behavior or adding validation if stricter security is needed.docs/docker-isolation.md (2)
60-73: Clear and helpful documentation.The section effectively explains the permission mismatch problem and provides a straightforward solution. The commands are correct and the explanation is concise.
134-134: LGTM!Good addition to the troubleshooting table with a clear cross-reference to the detailed section.
docker-compose.yml (1)
31-35: LGTM! Intentional consistency difference noted.The server service correctly receives UID/GID build args to create the non-root
automakeruser for mounted volume permissions. Theuiservice doesn't include these args because it uses the standardnginx:alpineimage, which doesn't require user ID matching—it only serves static files and doesn't mount host volumes.This design is correct: only services that create custom non-root users for volume access need the UID/GID args.
Dockerfile (1)
62-65: LGTM!The build arguments are well-documented with sensible defaults (1001) that maintain backward compatibility. The inline comment clearly shows how to override at build time.
Summary
UIDandGIDbuild arguments toDockerfileandDockerfile.devProblem
When mounting host directories into the container, files created by the container are owned by UID 1001 (the hardcoded container user). This causes permission mismatches when the host user has a different UID (e.g., 1000), making it difficult to debug or manually edit files created by Automaker.
Solution
Make the container user's UID/GID configurable via build arguments, defaulting to 1001 for backward compatibility:
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.