Skip to content
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: Skip groupadd if gid exists in Docker setup #53

Merged
merged 3 commits into from
Dec 9, 2021

Conversation

vidbina
Copy link
Contributor

@vidbina vidbina commented Jan 1, 2021

Users who have a gid that matching an existing gid in the container will encounter an error when running groupadd. In order to allow the docker build process to continue, the addition of the -f flag will allow groupadd to exit successfully if the group already exists.

Users who have a gid that matching an existing gid in the container will
encounter an error when running `groupadd`. In order to allow the docker
build process to continue, the addition of the `-f` flag will allow the
`groupadd` to exit successfully if the group already exists.
@SafariMonkey
Copy link
Contributor

I'm not convinced that this is the best solution. I'd like to point out the following sentence from the documentation:

When used with -g, and the specified GID already exists, another (unique) GID is chosen (i.e. -g is turned off).

This means that it will create a group with that name but a different GID unnecessarily.

If the GID exists, the next line will run without an issue. If it doesn't, it'll presumably fail, based on the note:

A group number must refer to an already existing group.

Based on this, I suggest, as a possibility, just adding || true to the previous line. The command output will still show in the console if it fails, and if the next line succeeds, all is well.

-f should be fine, of course, as the extra group is only being created inside the container. This just seems like a cleaner solution to me.

@vidbina
Copy link
Contributor Author

vidbina commented Jan 2, 2021

Good spot, @SafariMonkey. Based it from the groupadd --help in the base image which mentioned the following for -f:

exit successfully if the group already exists, and cancel -g if the GID is already used

I understood this bit to mean that the use of -f with -g would be "safe enough".

└──> docker run --rm -it ubuntu:16.04 bash
root@8e4d4c8550cd:/# groupadd --help
Usage: groupadd [options] GROUP

Options:
  -f, --force                   exit successfully if the group already exists,
                                and cancel -g if the GID is already used
  -g, --gid GID                 use GID for the new group
  -h, --help                    display this help message and exit
  -K, --key KEY=VALUE           override /etc/login.defs defaults
  -o, --non-unique              allow to create groups with duplicate
                                (non-unique) GID
  -p, --password PASSWORD       use this encrypted password for the new group
  -r, --system                  create a system account
  -R, --root CHROOT_DIR         directory to chroot into
      --extrausers              Use the extra users database

Did look into the resulting image and notice that the builder group was created with a new gid (999 in my case), so I have updated the PR such that we don't have this rogue group in the the image. Thanks for your input 🙏

@canselcik let me know if this is worthwhile merging or if you'd like me to supply this PR squashed.

@fenollp fenollp merged commit eddd107 into canselcik:master Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants