Fixed review findings

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

@@ -211,23 +211,23 @@ def __init__(self, path: Path):
     @property
     def stat(self):
         if self._stat is None:
-            self._stat = os.stat(self.path)
+            self._stat = os.stat(self._path)
         return self._stat
 
     @property
     def group_id(self) -> int:
         return self.stat.st_gid
 
     def is_group_accessible(self) -> bool:
-        if not os.path.exists(self.path):
-            _logger.debug(f"File not found {self.path}")
+        if not os.path.exists(self._path):
+            _logger.debug(f"File not found {self._path}")
             return False
         permissions = stat.filemode(self.stat.st_mode)
         if permissions[4:6] == "rw":
             return True
         _logger.error(
             "No rw permissions for group in"
-            f" {permissions} {self.path}.")
+            f" {permissions} {self._path}.")
         return False
 
 
@@ -253,14 +253,14 @@ def _run(self, command: str) -> int:
         return subprocess.run(command.split()).returncode
 
     def enable(self) -> Group:
-        gid = self.group.id
-        existing = self._find_group(gid)
+        gid = self._group.id
+        existing = self._find_group_name(gid)
         if existing:
-            self._run(f"usermod --append --groups {existing} {self.user}")
+            self._run(f"usermod --append --groups {existing} {self._user}")
             return Group(existing, gid)
         else:
-            self._run(f"groupmod -g {gid} {self.group.name}")
-            return self.group
+            self._run(f"groupmod -g {gid} {self._group.name}")
+            return self._group
 
 
 class User:

test/docker/container.py

@@ -1,7 +1,13 @@
 import re
-from typing import Union, Iterator, Generator
 
 import docker
+from datetime import timedelta
+
+from re import Pattern
+from tenacity import Retrying
+from tenacity.wait import wait_fixed
+from tenacity.stop import stop_after_delay
+from typing import Generator, Union
 from docker.models.containers import Container
 from docker.models.images import Image
 
@@ -35,3 +41,34 @@ def container(request, base_name: str, image: Union[Image, str], start: bool = T
     finally:
         client.containers.get(container_name).remove(force=True)
         client.close()
+
+
+def wait_for(
+        container: Container,
+        log_message: Union[str, Pattern],
+        timeout: timedelta = timedelta(seconds=5),
+):
+    """
+    Wait until container log contains the specified string or regular
+    expression.
+    """
+    for attempt in Retrying(
+            wait=wait_fixed(timeout/10),
+            stop=stop_after_delay(timeout),
+    ):
+        with attempt:
+            logs = container.logs().decode("utf-8").strip()
+            if isinstance(log_message, Pattern):
+                matches = log_message.search(logs)
+            else:
+                matches = log_message in logs
+            if not matches:
+                raise Exception()
+
+DOCKER_SOCKET_CONTAINER = "/var/run/docker.sock"
+
+def wait_for_socket_access(container: Container):
+    wait_for(
+        container,
+        f"entrypoint.py: Enabled access to {DOCKER_SOCKET_CONTAINER}",
+    )

test/docker/dss_docker_image.py

@@ -1,5 +1,7 @@
 import docker
+import os
 import pytest
+import stat
 
 from exasol.ds.sandbox.lib.dss_docker import DssDockerImage
 
@@ -52,3 +54,27 @@ def dss_docker_image(request):
     finally:
         if not keep_image:
             docker.from_env().images.remove(testee.image_name, force=True)
+
+
+@pytest.fixture
+def fake_docker_socket_on_host(tmp_path):
+    socket = tmp_path / "socket.txt"
+    socket.touch()
+    os.chmod(socket, 0o660)
+    return socket
+
+
+@pytest.fixture
+def accessible_file(fake_docker_socket_on_host):
+    socket = fake_docker_socket_on_host
+    mode = os.stat(socket).st_mode | stat.S_IRGRP | stat.S_IWGRP
+    os.chmod(socket, mode)
+    return socket
+
+
+@pytest.fixture
+def non_accessible_file(fake_docker_socket_on_host):
+    socket = fake_docker_socket_on_host
+    mode = stat.S_IRUSR | stat.S_IWUSR
+    os.chmod(socket, mode)
+    return socket

test/integration/test_create_dss_docker_image.py

@@ -18,16 +18,20 @@
 from contextlib import contextmanager
 from tenacity.wait import wait_fixed
 from tenacity.stop import stop_after_delay
-from typing import Set, Tuple, Union
+from typing import Set, Tuple
 from datetime import datetime, timedelta
 from exasol.ds.sandbox.lib.dss_docker import DssDockerImage
 from exasol.ds.sandbox.lib.logging import set_log_level
 from exasol.ds.sandbox.lib import pretty_print
-from test.docker.container import container
+from test.docker.container import (
+    container,
+    DOCKER_SOCKET_CONTAINER,
+    wait_for,
+    wait_for_socket_access,
+)
 
 
 DOCKER_SOCKET_HOST = "/var/run/docker.sock"
-DOCKER_SOCKET_CONTAINER = "/var/run/docker.sock"
 
 
 @pytest.fixture
@@ -51,36 +55,6 @@ def dss_docker_container(dss_docker_image, jupyter_port):
         container.remove()
 
 
-def wait_for(
-        container,
-        log_message: Union[str, Pattern],
-        timeout: timedelta = timedelta(seconds=5),
-):
-    """
-    Wait until container log contains the specified string or regular
-    expression.
-    """
-    for attempt in Retrying(
-            wait=wait_fixed(timeout/10),
-            stop=stop_after_delay(timeout),
-    ):
-        with attempt:
-            logs = container.logs().decode("utf-8").strip()
-            if isinstance(log_message, Pattern):
-                matches = log_message.search(logs)
-            else:
-                matches = log_message in logs
-            if not matches:
-                raise Exception()
-
-
-def wait_for_socket_access(container):
-    wait_for(
-        container,
-        f"entrypoint.py: Enabled access to {DOCKER_SOCKET_CONTAINER}",
-    )
-
-
 def retry(exception: typing.Type[BaseException], timeout: timedelta):
     return tenacity.retry(
         retry=retry_if_exception_type(exception),
@@ -146,15 +120,7 @@ def test_docker_socket_access(dss_docker_container):
     assert exit_code == 0 and re.match(r"^CONTAINER ID +IMAGE .*", output)
 
 
-@pytest.fixture
-def fake_docker_socket_on_host(tmp_path):
-    socket = tmp_path / "socket.txt"
-    socket.touch()
-    socket.chmod(0o660)
-    return socket
-
-
-def test_docker_socket_on_host_touched(request, dss_docker_image, socket_on_host):
+def test_docker_socket_on_host_touched(request, dss_docker_image, fake_docker_socket_on_host):
     """
     Verify that when mounting the docker socket from the host's file
     system into the container, the permissions and owner of the original
@@ -173,11 +139,12 @@ def my_container(docker_socket_host):
                 'mode': 'rw', }, },
         )
 
-    stat_before = socket_on_host.stat()
-    with my_container(socket_on_host) as c:
+    socket = fake_docker_socket_on_host
+    stat_before = socket.stat()
+    with my_container(socket) as c:
         wait_for_socket_access(c)
         exit_code, output = c.exec_run(f"docker ps")
     output = output.decode("utf-8").strip()
     assert exit_code == 1 and \
         re.match(r"(Cannot connect|permission denied)", output) and \
-        stat_before == socket_on_host.stat()
+        stat_before == socket.stat()

test/notebook_test_runner/test_notebooks_in_dss_docker_image.py

@@ -9,7 +9,7 @@
 from test.docker.exec_run import exec_command
 from test.docker.image import image
 from test.docker.in_memory_build_context import InMemoryBuildContext
-from test.docker.container import container
+from test.docker.container import container, wait_for_socket_access, wait_for
 
 TEST_RESOURCE_PATH = Path(__file__).parent.parent / "notebooks"
 
@@ -57,12 +57,8 @@ def notebook_test_container(request, notebook_test_image):
 
 @pytest.fixture()
 def notebook_test_container_with_log(notebook_test_container):
-    time.sleep(10)  # wait that the entrypoint changed the permissions of the docker socket
-    print("Docker socket")
-    socket = Path("/var/run/docker.sock")
-    print(socket)
-    print(socket.stat())
-    print()
+    wait_for_socket_access(notebook_test_container)
+    # wait_for(notebook_test_container, "entrypoint.py: Copied notebooks")
     logs = notebook_test_container.logs().decode("utf-8").strip()
     print(f"Container Logs: {logs or '(empty)'}", flush=True)
     yield notebook_test_container

test/unit/entrypoint/test_file_permissions.py

@@ -31,7 +31,7 @@ def test_file_inspector_not_group_accessible(non_accessible_file, caplog):
 
 def test_group_access_unknown_group_id():
     testee = entrypoint.GroupAccess(None, None)
-    assert testee._find_group(9999999) is None
+    assert testee._find_group_name(9999999) is None
 
 
 def test_group_access_enable_existing_group(mocker, capsys):

test/unit/entrypoint/test_user_class.py

@@ -69,7 +69,7 @@ def test_enable_non_accessible_file(mocker, user, non_accessible_file):
 def group_access(group: entrypoint.Group, find_result: str) -> entrypoint.GroupAccess:
     group_access = entrypoint.GroupAccess("user_name", group)
     group_access._run = lambda x: 0
-    group_access._find_group = lambda x: find_result
+    group_access._find_group_name = lambda x: find_result
     return group_access