From 25848c326aedc5fc958d90dfc172f1f32596978e Mon Sep 17 00:00:00 2001 From: David James Sherman Date: Mon, 18 Oct 2021 22:45:18 +0200 Subject: [PATCH 1/4] Unit test of specifying Unix group id 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? --- tests/unit/test_users.py | 50 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_users.py b/tests/unit/test_users.py index 2bceb5ff9..d714933f2 100644 --- a/tests/unit/test_users.py +++ b/tests/unit/test_users.py @@ -44,7 +44,7 @@ def test_user(): "--", "/bin/bash", "-c", - "id -u > id && pwd > pwd && whoami > name && echo -n $USER > env_user".format( + "id -u > id && id -g > grp && pwd > pwd && whoami > name && echo -n $USER > env_user".format( ts ), ] @@ -58,3 +58,51 @@ def test_user(): assert f.read().strip() == username with open(os.path.join(tmpdir, "name")) as f: assert f.read().strip() == username + # When group-id NOT specified, group id in container is user id + with open(os.path.join(tmpdir, "grp")) as f: + assert f.read().strip() == userid + + +def test_user_groups(): + """ + Validate user id and name setting + """ + ts = str(time.time()) + # FIXME: Use arbitrary login here, We need it now since we wanna put things to volume. + username = getuser() + userid = str(os.geteuid()) + groupid = str(os.geteuid() + 1) + with tempfile.TemporaryDirectory() as tmpdir: + tmpdir = os.path.realpath(tmpdir) + subprocess.check_call( + [ + "repo2docker", + "-v", + "{}:/home/{}".format(tmpdir, username), + "--user-id", + userid, + "--user-name", + username, + "--group-id", + groupid, + tmpdir, + "--", + "/bin/bash", + "-c", + "id -u > id && id -g > grp && pwd > pwd && whoami > name && echo -n $USER > env_user".format( + ts + ), + ] + ) + + with open(os.path.join(tmpdir, "id")) as f: + assert f.read().strip() == userid + with open(os.path.join(tmpdir, "pwd")) as f: + assert f.read().strip() == "/home/{}".format(username) + with open(os.path.join(tmpdir, "name")) as f: + assert f.read().strip() == username + with open(os.path.join(tmpdir, "name")) as f: + assert f.read().strip() == username + # When group-id specified, group id in container is same as specified + with open(os.path.join(tmpdir, "grp")) as f: + assert f.read().strip() == groupid From 5cbc8730af2cc62ca215cae48261eab0b447ce51 Mon Sep 17 00:00:00 2001 From: David James Sherman Date: Mon, 18 Oct 2021 22:51:20 +0200 Subject: [PATCH 2/4] Add command-line option to specify group-id 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. --- repo2docker/__main__.py | 9 +++++++++ repo2docker/app.py | 20 ++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/repo2docker/__main__.py b/repo2docker/__main__.py index 1dd49ebe9..f4ce4a9f9 100644 --- a/repo2docker/__main__.py +++ b/repo2docker/__main__.py @@ -180,6 +180,12 @@ def get_argparser(): "--user-name", help="Username of the primary user in the image" ) + argparser.add_argument( + "--group-id", + help="Group ID of the primary user in the image, defaults to same as user ID", + type=int, + ) + # Process the environment options the same way that docker does, as # they are passed directly to docker as the environment to use. This # requires a custom action for argparse. @@ -334,6 +340,9 @@ def make_r2d(argv=None): "Please see repo2docker --help for more details.\n" ) sys.exit(1) + # Check group-id not None because gid=0 is a valid value + if args.group_id is not None: + r2d.group_id = args.group_id if args.build_memory_limit: # if the string only contains numerals we assume it should be an int diff --git a/repo2docker/app.py b/repo2docker/app.py index cab2079c4..5e2b5e463 100755 --- a/repo2docker/app.py +++ b/repo2docker/app.py @@ -227,6 +227,25 @@ def _user_name_default(self): """ return getpass.getuser() + group_id = Int( + help=""" + GID of the user to create inside the built image. + + Defaults to gid of currently running user, since that is the most + common case when running r2d manually. + + Might not affect Dockerfile builds. + """, + config=True, + ) + + @default("group_id") + def _group_id_default(self): + """ + Default group_id to same as user id. + """ + return self._user_id_default() + appendix = Unicode( config=True, help=""" @@ -761,6 +780,7 @@ def build(self): build_args = { "NB_USER": self.user_name, "NB_UID": str(self.user_id), + "NB_GID": str(self.group_id), } if self.target_repo_dir: build_args["REPO_DIR"] = self.target_repo_dir From e805a216e38fb0362ff08bd1d754a53794959b2d Mon Sep 17 00:00:00 2001 From: David James Sherman Date: Mon, 18 Oct 2021 22:52:33 +0200 Subject: [PATCH 3/4] Add Unix group with NB_GID if supplied, default NB_GID = NB_UID. Note that the default GID in the extracted tar image is still DEFAULT_NB_UID --- repo2docker/buildpacks/base.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/repo2docker/buildpacks/base.py b/repo2docker/buildpacks/base.py index 46a407ad3..a69d9359e 100644 --- a/repo2docker/buildpacks/base.py +++ b/repo2docker/buildpacks/base.py @@ -37,16 +37,18 @@ # Set up user ARG NB_USER ARG NB_UID +ARG NB_GID=${NB_UID} ENV USER ${NB_USER} ENV HOME /home/${NB_USER} RUN groupadd \ - --gid ${NB_UID} \ + --gid ${NB_GID} \ + --non-unique \ ${NB_USER} && \ useradd \ --comment "Default user" \ --create-home \ - --gid ${NB_UID} \ + --gid ${NB_GID} \ --no-log-init \ --shell /bin/bash \ --uid ${NB_UID} \ @@ -572,7 +574,7 @@ def _filter_tar(tar): tar.uname = "" tar.gname = "" tar.uid = int(build_args.get("NB_UID", DEFAULT_NB_UID)) - tar.gid = int(build_args.get("NB_UID", DEFAULT_NB_UID)) + tar.gid = int(build_args.get("NB_GID", DEFAULT_NB_UID)) return tar for src in sorted(self.get_build_script_files()): From de80ca5e03670d1349a3c4389a2934aa0094d277 Mon Sep 17 00:00:00 2001 From: David James Sherman Date: Tue, 19 Oct 2021 07:39:57 +0200 Subject: [PATCH 4/4] Unit test of group id also stats uid, gid of created files --- tests/unit/test_users.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_users.py b/tests/unit/test_users.py index d714933f2..9fa7dba67 100644 --- a/tests/unit/test_users.py +++ b/tests/unit/test_users.py @@ -89,7 +89,7 @@ def test_user_groups(): "--", "/bin/bash", "-c", - "id -u > id && id -g > grp && pwd > pwd && whoami > name && echo -n $USER > env_user".format( + "id -u > id && id -g > grp && stat --format %u:%g grp > id_grp && pwd > pwd && whoami > name && echo -n $USER > env_user".format( ts ), ] @@ -106,3 +106,5 @@ def test_user_groups(): # When group-id specified, group id in container is same as specified with open(os.path.join(tmpdir, "grp")) as f: assert f.read().strip() == groupid + with open(os.path.join(tmpdir, "id_grp")) as f: + assert f.read().strip() == userid + ":" + groupid