-
Notifications
You must be signed in to change notification settings - Fork 0
fix(docker): Resolve bind mount permission issues for containers #64
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
Code Review SummaryThis pull request introduces a crucial feature: automated and intelligent handling of bind mount permissions for Docker Compose deployments, especially when using templates. This significantly enhances security, robustness, and compatibility with non-root containers. The changes involve modifying template metadata to include ownership information, updating the Docker discovery and manager to parse and apply these permissions, and adjusting existing templates to leverage this new capability. 🚀 Key Improvements
💡 Minor Suggestions
🚨 Critical Issues
|
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.
Review complete. See the overview comment for a summary.
- Add User and Subdirectories fields to TemplateMount for declaring container ownership requirements - Add ApplyMountOwnership to set correct UID:GID on bind mount dirs - Inspect container user after start and apply ownership automatically - Convert template volumes from named to bind mounts (wordpress, ghost, nextcloud) - Add user field to template metadata (www-data 33:33, bitnami 1000:1000) - Add Laravel storage subdirectories (framework/cache, sessions, views) Closes #50 Signed-off-by: nfebe <fenn25.fn@gmail.com>
4cff2cb to
1c76c91
Compare
🔍 Code Review💡 1. **internal/docker/discovery.go** (Lines 281-289) - SECURITYThe Suggested Code: Current Code: if err := os.MkdirAll(fullPath, 0777); err != nil {
return err
}
if err := os.Chmod(fullPath, 0777); err != nil {
return err
}💡 2. **internal/api/server.go** (Lines 1795-1807) - REFACTORThe Suggested Code: Current Code: type TemplateMount struct {
ID string `json:"id" yaml:"id"`
Name string `json:"name" yaml:"name"`
ContainerPath string `json:"container_path" yaml:"container_path"`
Description string `json:"description" yaml:"description"`
Type string `json:"type" yaml:"type"`
Required bool `json:"required" yaml:"required"`
}💡 3. **templates/ghost/metadata.yml** (Line 1) - BUGThe Suggested Code: Current Code: name: WordPress💡 4. **templates/nextcloud/metadata.yml** (Line 1) - BUGThe Suggested Code: Current Code: name: WordPressVerdict: APPROVE Posted as a comment because posting a review failed. |
Closes #50