Add EXTRA_GROUPS env var for beetle user group membership#239
Add EXTRA_GROUPS env var for beetle user group membership#239pSpitzner merged 10 commits intopSpitzner:mainfrom
Conversation
Co-authored-by: djdubd <2773193+djdubd@users.noreply.github.com>
Co-authored-by: djdubd <2773193+djdubd@users.noreply.github.com>
Co-authored-by: djdubd <2773193+djdubd@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds support for adding the beetle user to additional groups at container startup, addressing permission issues when managing files owned by groups other than beetle's primary group (GID 1000).
Key Changes:
- Adds
EXTRA_GROUPSenvironment variable for specifying additional group memberships - Implements parsing and validation logic for comma-separated
group_name:gidpairs - Documents the new feature with usage examples and common use cases
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
docker/entrypoints/entrypoint_fix_permissions.sh |
Implements parsing of EXTRA_GROUPS env var with validation, group creation, and user membership management |
docker/docker-compose.yaml |
Adds commented example showing how to configure EXTRA_GROUPS |
docs/configuration.md |
Documents the new EXTRA_GROUPS variable with format, examples, and use cases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add group name validation Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Warn user if named group exists with different gid Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Add explicit check for user having existing group membership. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Remove misleading variable example Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Remove incorrect and redundant positive integer check. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
semohr
left a comment
There was a problem hiding this comment.
Thank you for the PR! Highly appreciated.
I left some comments which hopeful allow us to make this more maintainable going forward.
I didn’t follow the initial issue too closely, but I was wondering how this approach compares to docker compose’s built-in group_add option. Are there specific reasons we’re handling it manually here instead of trying to use the buildin feature? I know we are currently also not using the user option properly but maybe now is the time to reevaluate this and maybe also simplify?
+1 :)
I agree, using as much of dockers built-ins as possible would be good. |
Why would you need both per entry? Maybe I'm missing something 🤔 E.g.: services:
ubuntu:
image: ubuntu:latest
command: tail -f /dev/null
stdin_open: true
tty: true
user: 1000:998
group_add:
- 24
- dialoutThe group id and names are resolved properly and the same as on the host: # in container
> id
uid=1000(ubuntu) gid=998 groups=998,20(dialout),24(cdrom)
# host machine
> id
...,20(dialout),24(cdrom),... |
|
I don't disagree that it would be better handled using My first proposed solution was to honor the settings chosen in the compose file related to @pSpitzner proposed adding extra groups to the I think this discussion is good, and appreciate you all paying attention to this. I look forward to knowing what the consensus is. |
|
The main question for me is whether we should drop the custom user-creation steps in the Dockerfile and instead rely on the built-in I understand why the PR is structured the way it is now, but I’m wondering if simplifying things and removing the extra layers might give us a more streamlined solution overall. Do we think there are any downsides we might run into by switching entirely to the built-in user/group functionality. If not, I’d vote to drop the extra steps. We’re planning a 2.0.0 release in the near future anyway, so this seems like a reasonable change to include. |
- Separated group_add into its own entrypoint script (called by fix_permissions one) - Split group_add logic into two functions - Removed old Testing parts from Dockerfile and entrypoint - Improved chown performance on macos
|
We just took another spin at it. @djdubd |
djdubd
left a comment
There was a problem hiding this comment.
Looks good to me, I believe it will add an additional level of flexibility for those with complex requirements due to nested virtualization scenarios, permissions set on other systems, and also permissions on network shares. In my testing it did solve the issue I encountered. The ability to add additional groups allows other users to work around the issue without having to modify permissions on their host, which I think is always preferable. Thanks for the feedback, I appreciate you taking the time to work on this!
…ROUPS env var (#239) - Add EXTRA_GROUPS support for beetle user permissions - Update docker/entrypoints/entrypoint_fix_permissions.sh - Add group name validation - Warn user if named group exists with different gid - Add explicit check for user having existing group membership. - Added changelog entry - Separated group_add into its own entrypoint script (called by fix_permissions one) - Split group_add logic into two functions - Removed old Testing parts from Dockerfile and entrypoint - Improved chown performance on macos closes #234 --------- Co-authored-by: djdubd <2773193+djdubd@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: pSpitzner <github@makeitso.one>
…ROUPS env var (#239) - Add EXTRA_GROUPS support for beetle user permissions - Update docker/entrypoints/entrypoint_fix_permissions.sh - Add group name validation - Warn user if named group exists with different gid - Add explicit check for user having existing group membership. - Added changelog entry - Separated group_add into its own entrypoint script (called by fix_permissions one) - Split group_add logic into two functions - Removed old Testing parts from Dockerfile and entrypoint - Improved chown performance on macos closes #234 --------- Co-authored-by: djdubd <2773193+djdubd@users.noreply.github.com> Co-authored-by: pSpitzner <github@makeitso.one>
Fixes #234
The web UI cannot delete inbox files owned by groups other than beetle's primary group (gid=1000). This affects setups where download clients create files as root or with different group ownership.
Changes
docker/entrypoints/entrypoint_fix_permissions.sh: ParseEXTRA_GROUPSenv var and add beetle to specified groups at container startupdocker/docker-compose.yaml: Add commented exampledocs/configuration.md: Document new env var under "Docker Environment Variables"Usage
Groups are created if they don't exist, then beetle is added. Invalid specs (missing colon, non-numeric/negative GID) are logged and skipped.
Testing
Tested by building the dev container with
EXTRA_GROUPS: "nas_shares:2000":Verified group membership:
Positive test - beetle can delete files owned by
root:nas_shares(775 permissions):Negative test - beetle cannot delete files owned by
root:root(as expected):This provides users with a container-side solution for the permissions issue, eliminating the need for host-level ACL modifications.