Add option --group-id to specify numerical group id
#1093
Conversation
Add test of gid to existing test, verify default behavior that gid = uid Add test of gid when specified, verify that provided gid is used The two tests are mostly duplicated code, could we refactor using a pytest parameterized test?
Group-id is an integer, like user-id. Its default value is the user-id, as that is the existing behavior. Note that gid=0 is a valid value.
Note that the default GID in the extracted tar image is still DEFAULT_NB_UID
|
Thanks for submitting your first pull request! You are awesome! 🤗 |
davidjsherman
changed the title
--group-id to specify numerical group id--group-id to specify numerical group id
There was a problem hiding this comment.
Thanks a lot for opening this, @davidjsherman. Apologies for the late review.
https://github.com/jupyterhub/repo2docker/search?q=chown shows there's a bunch of chown calls that currently rely on the assumption that the gid is the same as uid, and will need to be modified to take the gid into account properly. Once that's done, I think this is good to be merged.
|
I can do that. Do you by chance have a general idea of the tests that need to be made? I can verify that the uids and gids are as specified, but it would be better to check functional dependencies, if there are any. |
|
@davidjsherman not sure of the specifics, but perhaps look at the paths that are being chowned, and see what tests already cover them? Then, a gid variant can be added there. |
consideRatio
changed the title
--group-id to specify numerical group id--group-id to specify numerical group id
Add an option
--group-idto specify numerical group id separately from the numerical user id. If a group id is not specified, maintain the legacy behavior of using the numerical user id for the group.Images created with
--group-id=0follow Openshift best practices for container images, see discussion. Such images will work in the default unprivileged restricted security context constraint on Openshift and OKD Kubernetes platforms, where a non-root arbitrary user id is allocated on the fly to containers.The new test test_user_groups in
tests/unit/test_users.pyverifies the group id and checks file ownership. The existing test test_users now also tests for the legacy behavior. I admit that these tests are very similar and could perhaps be combined.