From 23d4cfc54c53a897004adf7c7819a73fc20978f5 Mon Sep 17 00:00:00 2001 From: ckunki Date: Wed, 13 Mar 2024 16:23:56 +0100 Subject: [PATCH] Fixed review findings --- .../roles/entrypoint/files/entrypoint.py | 24 ++++++++----------- .../test_create_dss_docker_image.py | 14 +++++++---- .../test_notebooks_in_dss_docker_image.py | 1 - test/unit/entrypoint/test_file_permissions.py | 21 +++++++++------- test/unit/entrypoint/test_user_class.py | 3 ++- 5 files changed, 33 insertions(+), 30 deletions(-) diff --git a/exasol/ds/sandbox/runtime/ansible/roles/entrypoint/files/entrypoint.py b/exasol/ds/sandbox/runtime/ansible/roles/entrypoint/files/entrypoint.py index 69a497ea..238752b6 100644 --- a/exasol/ds/sandbox/runtime/ansible/roles/entrypoint/files/entrypoint.py +++ b/exasol/ds/sandbox/runtime/ansible/roles/entrypoint/files/entrypoint.py @@ -206,29 +206,25 @@ def __repr__(self): class FileInspector: def __init__(self, path: Path): self._path = path - self._stat = None - - @property - def stat(self): - if self._stat is None: - self._stat = os.stat(self._path) - return self._stat + self._stat = path.stat() if path.exists() else None @property def group_id(self) -> int: - return self.stat.st_gid + if self._stat is None: + raise FileNotFoundError(self._path) + return self._stat.st_gid def is_group_accessible(self) -> bool: - if not os.path.exists(self._path): + if self._stat is None: _logger.debug(f"File not found {self._path}") return False - permissions = stat.filemode(self.stat.st_mode) + permissions = stat.filemode(self._stat.st_mode) if permissions[4:6] == "rw": return True - _logger.error( + raise PermissionError( "No rw permissions for group in" - f" {permissions} {self._path}.") - return False + f" {permissions} {self._path}." + ) class GroupAccess: @@ -284,7 +280,7 @@ def id(self): self._id = pwd.getpwnam(self.name).pw_uid return self._id - def enable_group_access(self, path: str): + def enable_group_access(self, path: Path): file = FileInspector(path) if file.is_group_accessible(): group = GroupAccess( diff --git a/test/integration/test_create_dss_docker_image.py b/test/integration/test_create_dss_docker_image.py index 922f8e53..8abb261b 100644 --- a/test/integration/test_create_dss_docker_image.py +++ b/test/integration/test_create_dss_docker_image.py @@ -120,6 +120,13 @@ def test_docker_socket_access(dss_docker_container): assert exit_code == 0 and re.match(r"^CONTAINER ID +IMAGE .*", output) +# add tests for +# - error for insufficient group permissions on docker.sock +# - docker socket gid matches another group inside container +# - docker socket gid matches no group inside container +# tests require to inspect available groups inside docker container +# docker run -v /home/chku/tmp/a:/aa ubuntu chgrp 1200 /aa + 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 @@ -143,8 +150,5 @@ def my_container(docker_socket_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.stat() + + assert stat_before == socket.stat() diff --git a/test/notebook_test_runner/test_notebooks_in_dss_docker_image.py b/test/notebook_test_runner/test_notebooks_in_dss_docker_image.py index cd98611a..08106c91 100644 --- a/test/notebook_test_runner/test_notebooks_in_dss_docker_image.py +++ b/test/notebook_test_runner/test_notebooks_in_dss_docker_image.py @@ -58,7 +58,6 @@ def notebook_test_container(request, notebook_test_image): @pytest.fixture() def notebook_test_container_with_log(notebook_test_container): 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 diff --git a/test/unit/entrypoint/test_file_permissions.py b/test/unit/entrypoint/test_file_permissions.py index 9100dd04..23d3819b 100644 --- a/test/unit/entrypoint/test_file_permissions.py +++ b/test/unit/entrypoint/test_file_permissions.py @@ -4,6 +4,7 @@ import re import stat +from pathlib import Path from exasol.ds.sandbox.runtime.ansible.roles.entrypoint.files import entrypoint from unittest.mock import MagicMock @@ -12,26 +13,28 @@ def test_file_inspector_non_existing_file(mocker): mocker.patch("os.stat") # need to mock os.path.exists as os.path.exists seems to call os.stat :) mocker.patch("os.path.exists", return_value=False) - testee = entrypoint.FileInspector("/non/existing/file") + testee = entrypoint.FileInspector(Path("/non/existing/file")) actual = testee.is_group_accessible() assert actual == False assert not os.stat.called +def test_file_inspector_group_id_non_existing_file(): + testee = entrypoint.FileInspector(Path("/non/existing/file")) + with pytest.raises(FileNotFoundError): + testee.group_id + + def test_file_inspector_group_accessible(accessible_file): testee = entrypoint.FileInspector(accessible_file) assert testee.is_group_accessible() -def test_file_inspector_not_group_accessible(non_accessible_file, caplog): +def test_file_inspector_not_group_accessible(non_accessible_file): testee = entrypoint.FileInspector(non_accessible_file) - assert not testee.is_group_accessible() - assert re.match(r"ERROR .* No rw permissions for group", caplog.text) - - -def test_group_access_unknown_group_id(): - testee = entrypoint.GroupAccess(None, None) - assert testee._find_group_name(9999999) is None + with pytest.raises(PermissionError) as err: + testee.is_group_accessible() + assert re.match(r"No rw permissions for group", str(err.value)) def test_group_access_enable_existing_group(mocker, capsys): diff --git a/test/unit/entrypoint/test_user_class.py b/test/unit/entrypoint/test_user_class.py index edb2fec3..d9327253 100644 --- a/test/unit/entrypoint/test_user_class.py +++ b/test/unit/entrypoint/test_user_class.py @@ -4,6 +4,7 @@ import pytest import unittest +from pathlib import Path from unittest.mock import MagicMock, create_autospec from exasol.ds.sandbox.runtime.ansible.roles.entrypoint.files import entrypoint from test.unit.entrypoint.entrypoint_mock import entrypoint_method @@ -56,7 +57,7 @@ def test_uid(mocker, user): def test_enable_file_absent(mocker, user): mocker.patch(entrypoint_method("GroupAccess")) - user.enable_group_access("/non/existing/path") + user.enable_group_access(Path("/non/existing/path")) assert not entrypoint.GroupAccess.called