Stabilize UID/GID handling: create users before groups

[boecks]

Summary

Currently the entrypoint creates groups before users. On Alpine, this can cause
user creation to fail if a GROUP_* and an ACCOUNT_* are configured with the
same numeric ID (e.g. UID=1000 and GID=1000). In this case adduser fails and
Samba does not export the shares.

By creating users first, the UID can always be allocated. If the same numeric
GID is later requested, addgroup fails harmlessly with "gid in use". The user
is still created successfully and the container continues normally.

Benefits

• Prevents confusing failures when UID and GID collide in env configuration.
• Still works for all existing setups (no regressions).
• Safer and more predictable behavior in environments where UID=GID mappings are
  common for volume ownership.

Notes

This does not claim that UID=GID was previously impossible in all cases. It was
always possible if no GROUP_* collided. The change simply hardens the
entrypoint against collisions when both are defined.

[MarvAmBass]

Hi thanks for your work.

But I like to argue that addgroup fails harmlessly with "gid in use". statement. sure it's working, but you might end up exposing data which was specifically granted for a group to a user.

as I clearly state in the readme, you should not try to create the usergroups yourself - it's the systems job - groups are for grouping only, take higher ids and try to avoid those collisions.

if you don't need to group users to groups, ignore groups compleletly.
but you shouldn't expect to have group ids change ownership.

do you see any flaw/problems I don't see right now?

[boecks]

Hi and thanks for the quick feedback! I understand your point about the risk, and it makes sense to pay careful attention to this.

My use case was simply to streamline UID/GID across my systems for consistency.

I had the patch from the PR in place and it worked, but later I realized that using the environment variables correctly was already enough — so the patch wasn’t actually necessary for my setup.

What confused me was that the container failed hard when I defined both a group and a user with ID 1000 (just for consistency as described above). I probably overlooked that part of the README, which added to my misunderstanding.

I completely understand if you decide not to merge this PR for the reasons you mentioned, if you don’t see a benefit for the image.

[MarvAmBass]

good to read, that you also see my point and that you don't see problems or other benefits in your contribution.

If I suspect correctly My use case was simply to streamline UID/GID across my systems for consistency. if you leave out the specific group id configuration environment variables in my docker file it is automatically consistent, right?
So your usecase was invalid, and your configuration was just not as my container expected it to be?
if you still have consistency issues, I might need to take a look into this issue - maybe by overwriting the behaviour of group creation if a usergroup is specified or something - but right now I think it would just add complexity without real world benefits.

I'll keep it open, maybe someone really needs this feature (for some active directory syncronisation stuff I'm currently not seeing)
but I think, that the solution needs to be a bit more secure - to not accedentaly break permission settings on data

[boecks]

Thanks for the clarification. That helps me see the distinction better.

Just to give a bit more background: I run the Samba containers as sidecars in unprivileged LXCs on a Proxmox cluster. The PVE host mounts the Samba shares and rebinds them into other unprivileged LXCs that cannot mount SMB themselves. Because of that, I need UID/GID 1000:1000 for consistency across host and containers. Since I only have a single user I don’t rely on group ACLs at all.

What caused my issue originally was simply that I also added a GROUP_* env var with GID 1000. That triggered the collision and the container failed. In my setup I don’t need that at all – defining only:

      ACCOUNT_user: password
      UID_user: 1000

is already enough. While the following lead to the crash:

      GROUP_mygroup: 1000 
      ACCOUNT_user: password 
      UID_user: 1000 
      GROUPS_user: mygroup

So I fully understand your concern about breaking group ACL semantics if both UID and GID are defined the same. In my case it was just a misconfiguration. I just wanted to share my use case and where the confusion came from, in case it helps others.