Fixed review findings

exasol/ds/sandbox/runtime/ansible/ai_lab_docker_playbook.yml

@@ -31,9 +31,10 @@
   gather_facts: true
   vars:
     ansible_python_interpreter: python3
-    user_name: "jupyter"
-    user_group: "jupyter"
-    user_home: "/home/jupyter"
+    user_name: jupyter
+    user_group: jupyter
+    docker_group: docker
+    user_home: /home/jupyter
     initial_notebook_folder: "{{ user_home }}/notebook-defaults"
     need_sudo: yes
     docker_integration_test: true

exasol/ds/sandbox/runtime/ansible/ec2_playbook.yml

@@ -4,6 +4,7 @@
       ansible_python_interpreter: /usr/bin/python3
       user_name: jupyter
       user_home: /home/jupyter
+      docker_group: docker
       user_group: jupyter
       initial_notebook_folder: "{{ user_home }}/notebooks"
       need_sudo: yes

exasol/ds/sandbox/runtime/ansible/general_setup_tasks.yml

@@ -1,16 +1,16 @@
 - name: Set facts for Entry Point
   vars:
     jupyter_virtualenv: "{{user_home}}/jupyterenv"
-    dockerjupyter_virtualenv: "{{user_home}}/jupyterenv"
   ansible.builtin.set_fact:
     dss_facts:
       entrypoint: "{{user_home}}/entrypoint.py"
-      docker_group: "{{ user_group }}"
+      docker_group: "{{ docker_group }}"
       jupyter:
         virtualenv: "{{ jupyter_virtualenv }}"
         command: "{{ jupyter_virtualenv }}/bin/jupyter-lab"
         port: "49494"
         user: "{{ user_name }}"
+        group: "{{ user_group }}"
         home: "{{ user_home }}"
         password: "{{ lookup('ansible.builtin.env', 'JUPYTER_LAB_PASSWORD', default='ailab') }}"
         logfile: "{{ user_home }}/jupyter-server.log"
@@ -43,7 +43,7 @@
     owner: "{{ user_name }}"
     group: "{{ user_group }}"
     recurse: true
-  become: "{{ need_sudo }}"
+  become: "{{need_sudo}}"
 - name: Clear pip Cache
   ansible.builtin.file:
     path: /root/.cache/pip
@@ -52,7 +52,7 @@
   include_role:
     name: docker
   vars:
-    docker_group: "{{ dss_facts.docker_group }}"
+    docker_group: "{{ docker_group }}"
 - name: Disable Core Dumps
   include_role:
     name: coredumps

exasol/ds/sandbox/runtime/ansible/roles/docker/tasks/main.yml

@@ -28,7 +28,7 @@
 
 - name: Adding docker users (for use without sudo)
   user:
-    name: "{{user_name}}"
+    name: "{{ user_name }}"
     append: yes
     groups: "{{ docker_group }}"
   become: "{{need_sudo}}"

exasol/ds/sandbox/runtime/ansible/roles/entrypoint/files/entrypoint.py

@@ -44,7 +44,11 @@ def arg_parser():
     )
     parser.add_argument(
         "--group", type=str,
-        help="user group for running jupyter server and accessing Docker socket",
+        help="user group for running jupyter server",
+    )
+    parser.add_argument(
+        "--docker-group", type=str,
+        help="user group for accessing Docker socket",
     )
     parser.add_argument(
         "--home", type=str,
@@ -173,47 +177,59 @@ def sleep_infinity():
         time.sleep(1)
 
 
+class Group:
+    def __init__(self, name: str):
+        self.name = name
+        self._id = None
+
+    @property
+    def id(self):
+        if self._id is None:
+            self._id = grp.getgrnam(self.name).gr_gid
+        return self._id
+
+
 class User:
-    def __init__(self, user_name: str, group_name: str):
-        self.user_name = user_name
-        self.group_name = group_name
-        self._uid = None
-        self._gid = None
+    def __init__(self, user_name: str, group: Group, docker_group: Group):
+        self.name = user_name
+        self._id = None
+        self.group = group
+        self.docker_group = docker_group
 
     @property
-    def uid(self):
-        if self._uid is None:
-            self._uid = pwd.getpwnam(self.user_name).pw_uid
-        return self._uid
+    def is_specified(self) -> bool:
+        return bool(
+            self.name
+            and self.group.name
+            and self.docker_group.name
+        )
 
     @property
-    def gid(self):
-        if self._gid is None:
-            self._gid = grp.getgrnam(self.group_name).gr_gid
-        return self._gid
+    def uid(self):
+        if self._id is None:
+            self._id = pwd.getpwnam(self.name).pw_uid
+        return self._id
 
     def own(self, path: str):
         if Path(path).exists():
             unchanged_uid = -1
-            os.chown(path, unchanged_uid, self.gid)
+            os.chown(path, unchanged_uid, self.docker_group.id)
         return self
 
     def switch_to(self):
         uid = self.uid
-        gid = self.gid
         os.setresuid(uid, uid, uid)
+        gid = self.group.id
         os.setresgid(gid, gid, gid)
+        os.setgroups([self.docker_group.id])
         return self
 
 
 def main():
     args = arg_parser().parse_args()
-    if args.user and args.group:
-        (
-            User(args.user, args.group)
-            .own("/var/run/docker.sock")
-            .switch_to()
-        )
+    user = User(args.user, Group(args.group), Group(args.docker_group))
+    if user.is_specified:
+        user.own("/var/run/docker.sock").switch_to()
     if args.notebook_defaults and args.notebooks:
         copy_rec(
             args.notebook_defaults,

test/unit/entrypoint/test_user_class.py

@@ -3,36 +3,61 @@
 import pwd
 import pytest
 
-from unittest.mock import MagicMock
+from unittest.mock import MagicMock, create_autospec
 from exasol.ds.sandbox.runtime.ansible.roles.entrypoint.files import entrypoint
 
 
+def group(name: str, id: int):
+    group = entrypoint.Group(name)
+    group._id = id
+    return group
+
+
 @pytest.fixture
 def user():
-    return entrypoint.User("jennifer", "heroes")
+    return entrypoint.User(
+        "jennifer",
+        group("family", 901),
+        group("docker", 902),
+    )
 
 
 @pytest.fixture
 def user_with_id(mocker, user):
-    mocker.patch("pwd.getpwnam", return_value=MagicMock(pw_uid=123))
-    mocker.patch("grp.getgrnam", return_value=MagicMock(gr_gid=456))
+    user._id = 100
     return user
 
 
+def test_group(mocker):
+    mocker.patch("grp.getgrnam")
+    testee = entrypoint.Group("my-group").id
+    assert grp.getgrnam.called
+    assert grp.getgrnam.call_args == mocker.call("my-group")
+
+
+@pytest.mark.parametrize(
+    "user, group, docker, expected", [
+        (None, "group", "docker", False),
+        ("user", None, "docker", False),
+        ("user", "group", None, False),
+        ("user", "group", "docker_group", True),
+    ])
+def test_user_specified(user, group, docker, expected):
+    testee = entrypoint.User(
+        user,
+        entrypoint.Group(group),
+        entrypoint.Group(docker),
+    )
+    assert expected == testee.is_specified
+
+
 def test_uid(mocker, user):
     mocker.patch("pwd.getpwnam")
     user.uid
     assert pwd.getpwnam.called
     assert pwd.getpwnam.call_args == mocker.call("jennifer")
 
 
-def test_gid(mocker, user):
-    mocker.patch("grp.getgrnam")
-    user.gid
-    assert grp.getgrnam.called
-    assert grp.getgrnam.call_args == mocker.call("heroes")
-
-
 def test_chown_file_absent(mocker, user):
     mocker.patch("os.chown")
     user.own("/non/existing/path")
@@ -43,14 +68,17 @@ def test_chown_file_exists(mocker, tmp_path, user_with_id):
     mocker.patch("os.chown")
     user_with_id.own(tmp_path)
     assert os.chown.called
-    assert os.chown.call_args == mocker.call(tmp_path, -1, 456)
+    assert os.chown.call_args == mocker.call(tmp_path, -1, 902)
 
 
 def test_switch_to(mocker, user_with_id):
     mocker.patch("os.setresuid")
     mocker.patch("os.setresgid")
+    mocker.patch("os.setgroups")
     user_with_id.switch_to()
     assert os.setresuid.called
-    assert os.setresuid.call_args == mocker.call(123, 123, 123)
+    assert os.setresuid.call_args == mocker.call(100, 100, 100)
     assert os.setresgid.called
-    assert os.setresgid.call_args == mocker.call(456, 456, 456)
+    assert os.setresgid.call_args == mocker.call(901, 901, 901)
+    assert os.setgroups.called
+    assert os.setgroups.call_args == mocker.call([902])