From 305e5cc2c3c02d9fced918c87e6259cc6e42b9ef Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Thu, 29 Feb 2024 00:54:39 +0000 Subject: [PATCH] refactor(services): introduce Bind class and test moving deeper --- selfprivacy_api/services/moving.py | 2 +- selfprivacy_api/services/owned_path.py | 42 +++++---- selfprivacy_api/utils/block_devices.py | 3 + tests/conftest.py | 40 +++++++++ tests/test_binds.py | 92 +++++++++++++++++++ tests/test_block_device_utils.py | 1 + tests/test_graphql/test_services.py | 118 +++++++++++++++++++------ 7 files changed, 253 insertions(+), 45 deletions(-) create mode 100644 tests/test_binds.py diff --git a/selfprivacy_api/services/moving.py b/selfprivacy_api/services/moving.py index 09af765..8b6d3b1 100644 --- a/selfprivacy_api/services/moving.py +++ b/selfprivacy_api/services/moving.py @@ -67,6 +67,6 @@ def ensure_folder_ownership(folders: List[Bind]) -> None: folder.ensure_ownership() -def bind_folders(folders: List[Bind], volume: BlockDevice): +def bind_folders(folders: List[Bind]): for folder in folders: folder.bind() diff --git a/selfprivacy_api/services/owned_path.py b/selfprivacy_api/services/owned_path.py index da40510..6e85fb0 100644 --- a/selfprivacy_api/services/owned_path.py +++ b/selfprivacy_api/services/owned_path.py @@ -2,9 +2,13 @@ from __future__ import annotations import subprocess import pathlib from pydantic import BaseModel +from os.path import exists from selfprivacy_api.utils.block_devices import BlockDevice, BlockDevices +# tests override it to a tmpdir +VOLUMES_PATH = "/volumes" + class BindError(Exception): pass @@ -19,9 +23,8 @@ class OwnedPath(BaseModel): class Bind: """ - A directory that resides on some volume but we mount it into fs - where we need it. - Used for service data. + A directory that resides on some volume but we mount it into fs where we need it. + Used for storing service data. """ def __init__(self, binding_path: str, owner: str, group: str, drive: BlockDevice): @@ -30,7 +33,7 @@ class Bind: self.group = group self.drive = drive - # TODO: make Service return a list of binds instead of owned paths + # TODO: delete owned path interface from Service @staticmethod def from_owned_path(path: OwnedPath, drive_name: str) -> Bind: drive = BlockDevices().get_block_device(drive_name) @@ -45,9 +48,9 @@ class Bind: return self.binding_path.split("/")[-1] def location_at_volume(self) -> str: - return f"/volumes/{self.drive.name}/{self.bind_foldername()}" + return f"{VOLUMES_PATH}/{self.drive.name}/{self.bind_foldername()}" - def validate(self) -> str: + def validate(self) -> None: path = pathlib.Path(self.location_at_volume()) if not path.exists(): @@ -58,23 +61,29 @@ class Bind: raise BindError(f"{path} is not owned by {self.owner}.") def bind(self) -> None: + if not exists(self.binding_path): + raise BindError(f"cannot bind to a non-existing path: {self.binding_path}") + + source = self.location_at_volume() + target = self.binding_path + try: subprocess.run( - [ - "mount", - "--bind", - self.location_at_volume(), - self.binding_path, - ], + ["mount", "--bind", source, target], + stderr=subprocess.PIPE, check=True, ) except subprocess.CalledProcessError as error: - print(error.output) - raise BindError(f"Unable to mount new volume:{error.output}") + print(error.stderr) + raise BindError(f"Unable to bind {source} to {target} :{error.stderr}") def unbind(self) -> None: + if not exists(self.binding_path): + raise BindError(f"cannot unbind a non-existing path: {self.binding_path}") + try: subprocess.run( + # umount -l ? ["umount", self.binding_path], check=True, ) @@ -94,10 +103,11 @@ class Bind: true_location, ], check=True, + stderr=subprocess.PIPE, ) except subprocess.CalledProcessError as error: - print(error.output) + print(error.stderr) error_message = ( - f"Unable to set ownership of {true_location} :{error.output}" + f"Unable to set ownership of {true_location} :{error.stderr}" ) raise BindError(error_message) diff --git a/selfprivacy_api/utils/block_devices.py b/selfprivacy_api/utils/block_devices.py index f1a4149..4de5b75 100644 --- a/selfprivacy_api/utils/block_devices.py +++ b/selfprivacy_api/utils/block_devices.py @@ -171,6 +171,9 @@ class BlockDevice: return False +# TODO: SingletonMetaclass messes with tests and is able to persist state +# between them. If you have very weird test crosstalk that's probably why +# I am not sure it NEEDS to be SingletonMetaclass class BlockDevices(metaclass=SingletonMetaclass): """Singleton holding all Block devices""" diff --git a/tests/conftest.py b/tests/conftest.py index e651c08..0cd1493 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -4,6 +4,7 @@ import os import pytest import datetime +import subprocess from os import path from os import makedirs @@ -135,6 +136,18 @@ def wrong_auth_client(huey_database, redis_repo_with_tokens): return client +@pytest.fixture() +def volume_folders(tmpdir, mocker): + volumes_dir = path.join(tmpdir, "volumes") + + makedirs(volumes_dir) + volumenames = ["sda1", "sda2"] + for d in volumenames: + service_dir = path.join(volumes_dir, d) + makedirs(service_dir) + mock = mocker.patch("selfprivacy_api.services.owned_path.VOLUMES_PATH", volumes_dir) + + @pytest.fixture() def raw_dummy_service(tmpdir): dirnames = ["test_service", "also_test_service"] @@ -161,11 +174,38 @@ def raw_dummy_service(tmpdir): return service +def ensure_user_exists(user: str): + try: + output = subprocess.check_output( + ["useradd", "-U", user], stderr=subprocess.PIPE, shell=False + ) + except subprocess.CalledProcessError as error: + if b"already exists" not in error.stderr: + raise error + + try: + output = subprocess.check_output( + ["useradd", user], stderr=subprocess.PIPE, shell=False + ) + except subprocess.CalledProcessError as error: + assert b"already exists" in error.stderr + return + + raise ValueError("could not create user", user) + + @pytest.fixture() def dummy_service( tmpdir, raw_dummy_service, generic_userdata ) -> Generator[Service, None, None]: service = raw_dummy_service + user = service.get_user() + + # TODO: use create_user from users actions. But it will need NIXOS to be there + # and react to our changes to files. + # from selfprivacy_api.actions.users import create_user + # create_user(user, "yay, it is me") + ensure_user_exists(user) # register our service services.services.append(service) diff --git a/tests/test_binds.py b/tests/test_binds.py new file mode 100644 index 0000000..ef9a0d5 --- /dev/null +++ b/tests/test_binds.py @@ -0,0 +1,92 @@ +import pytest +from os import mkdir, rmdir +from os.path import join, exists + + +from tests.conftest import ensure_user_exists +from tests.test_graphql.test_services import mock_lsblk_devices + +from selfprivacy_api.services.owned_path import Bind, BindError +from selfprivacy_api.utils.block_devices import BlockDevices +from selfprivacy_api.utils.waitloop import wait_until_true + + +BINDTESTS_USER = "binduser" +TESTFILE_CONTENTS = "testissimo" +TESTFILE_NAME = "testfile" + + +@pytest.fixture() +def bind_user(): + ensure_user_exists(BINDTESTS_USER) + return BINDTESTS_USER + + +def prepare_test_bind(tmpdir, bind_user) -> Bind: + test_binding_name = "bindy_dir" + binding_path = join(tmpdir, test_binding_name) + drive = BlockDevices().get_block_device("sda2") + assert drive is not None + + bind = Bind( + binding_path=binding_path, owner=bind_user, group=bind_user, drive=drive + ) + + source_dir = bind.location_at_volume() + mkdir(source_dir) + mkdir(binding_path) + + testfile_path = join(source_dir, TESTFILE_NAME) + with open(testfile_path, "w") as file: + file.write(TESTFILE_CONTENTS) + + return bind + + +def test_bind_unbind(volume_folders, tmpdir, bind_user, mock_lsblk_devices): + bind = prepare_test_bind(tmpdir, bind_user) + bind.ensure_ownership() + bind.validate() + + testfile_path = join(bind.location_at_volume(), TESTFILE_NAME) + assert exists(testfile_path) + with open(testfile_path, "r") as file: + assert file.read() == TESTFILE_CONTENTS + + bind.bind() + + testfile_binding_path = join(bind.binding_path, TESTFILE_NAME) + assert exists(testfile_path) + with open(testfile_path, "r") as file: + assert file.read() == TESTFILE_CONTENTS + + bind.unbind() + # wait_until_true(lambda : not exists(testfile_binding_path), timeout_sec=2) + assert not exists(testfile_binding_path) + assert exists(bind.binding_path) + + +def test_bind_nonexistent_target(volume_folders, tmpdir, bind_user, mock_lsblk_devices): + bind = prepare_test_bind(tmpdir, bind_user) + + bind.ensure_ownership() + bind.validate() + rmdir(bind.binding_path) + + with pytest.raises(BindError): + bind.bind() + + +def test_unbind_nonexistent_target( + volume_folders, tmpdir, bind_user, mock_lsblk_devices +): + bind = prepare_test_bind(tmpdir, bind_user) + + bind.ensure_ownership() + bind.validate() + bind.bind() + + bind.binding_path = "/bogus" + + with pytest.raises(BindError): + bind.unbind() diff --git a/tests/test_block_device_utils.py b/tests/test_block_device_utils.py index 41c30c8..2162c4d 100644 --- a/tests/test_block_device_utils.py +++ b/tests/test_block_device_utils.py @@ -410,6 +410,7 @@ def lsblk_full_mock(mocker): mock = mocker.patch( "subprocess.check_output", autospec=True, return_value=FULL_LSBLK_OUTPUT ) + BlockDevices().update() return mock diff --git a/tests/test_graphql/test_services.py b/tests/test_graphql/test_services.py index d509a6f..6d841d6 100644 --- a/tests/test_graphql/test_services.py +++ b/tests/test_graphql/test_services.py @@ -1,5 +1,8 @@ import pytest +import shutil + from typing import Generator +from os import mkdir from selfprivacy_api.utils.block_devices import BlockDevices @@ -8,13 +11,77 @@ from selfprivacy_api.services import get_service_by_id from selfprivacy_api.services.service import Service, ServiceStatus from selfprivacy_api.services.test_service import DummyService -# from selfprivacy_api.services.moving import check_folders - from tests.common import generate_service_query from tests.test_graphql.common import assert_empty, assert_ok, get_data from tests.test_block_device_utils import lsblk_singular_mock -from subprocess import CompletedProcess + +LSBLK_BLOCKDEVICES_DICTS = [ + { + "name": "sda1", + "path": "/dev/sda1", + "fsavail": "4614107136", + "fssize": "19814920192", + "fstype": "ext4", + "fsused": "14345314304", + "mountpoints": ["/nix/store", "/"], + "label": None, + "uuid": "ec80c004-baec-4a2c-851d-0e1807135511", + "size": 20210236928, + "model": None, + "serial": None, + "type": "part", + }, + { + "name": "sda2", + "path": "/dev/sda2", + "fsavail": "4614107136", + "fssize": "19814920192", + "fstype": "ext4", + "fsused": "14345314304", + "mountpoints": ["/home"], + "label": None, + "uuid": "deadbeef-baec-4a2c-851d-0e1807135511", + "size": 20210236928, + "model": None, + "serial": None, + "type": "part", + }, +] + + +@pytest.fixture() +def mock_lsblk_devices(mocker): + mock = mocker.patch( + "selfprivacy_api.utils.block_devices.BlockDevices.lsblk_device_dicts", + autospec=True, + return_value=LSBLK_BLOCKDEVICES_DICTS, + ) + BlockDevices().update() + assert BlockDevices().lsblk_device_dicts() == LSBLK_BLOCKDEVICES_DICTS + devices = BlockDevices().get_block_devices() + + assert len(devices) == 2 + + names = [device.name for device in devices] + assert "sda1" in names + assert "sda2" in names + return mock + + +@pytest.fixture() +def dummy_service_with_binds(dummy_service, mock_lsblk_devices, volume_folders): + binds = dummy_service.binds() + for bind in binds: + path = bind.binding_path + shutil.move(bind.binding_path, bind.location_at_volume()) + mkdir(bind.binding_path) + + bind.ensure_ownership() + bind.validate() + + bind.bind() + return dummy_service @pytest.fixture() @@ -28,9 +95,17 @@ def only_dummy_service(dummy_service) -> Generator[DummyService, None, None]: service_module.services.extend(back_copy) -MOVER_MOCK_PROCESS = CompletedProcess(["ls"], returncode=0) +@pytest.fixture() +def mock_check_volume(mocker): + mock = mocker.patch( + "selfprivacy_api.services.service.check_volume", + autospec=True, + return_value=None, + ) + return mock +# TODO: remove @pytest.fixture() def mock_check_service_mover_binds(mocker): mock = mocker.patch( @@ -41,20 +116,6 @@ def mock_check_service_mover_binds(mocker): return mock -@pytest.fixture() -def mock_subprocess_run(mocker): - mock = mocker.patch( - "subprocess.run", autospec=True, return_value=MOVER_MOCK_PROCESS - ) - return mock - - -@pytest.fixture() -def mock_shutil_move(mocker): - mock = mocker.patch("shutil.move", autospec=True, return_value=None) - return mock - - API_START_MUTATION = """ mutation TestStartService($service_id: String!) { services { @@ -551,7 +612,7 @@ def test_move_same_volume(authorized_client, dummy_service): def test_graphql_move_service_without_folders_on_old_volume( authorized_client, generic_userdata, - lsblk_singular_mock, + mock_lsblk_devices, dummy_service: DummyService, ): target = "sda1" @@ -564,26 +625,27 @@ def test_graphql_move_service_without_folders_on_old_volume( data = get_data(mutation_response)["services"]["moveService"] assert_errorcode(data, 400) + assert "sda2/test_service is not found" in data["message"] def test_graphql_move_service( authorized_client, generic_userdata, + # TODO: substitute with a weaker mock or delete altogether mock_check_service_mover_binds, - lsblk_singular_mock, - dummy_service: DummyService, - mock_subprocess_run, - mock_shutil_move, + mock_check_volume, + dummy_service_with_binds, ): - # Does not check real moving, - # but tests the finished job propagation through API + dummy_service = dummy_service_with_binds - target = "sda1" - BlockDevices().update() + origin = "sda1" + target = "sda2" assert BlockDevices().get_block_device(target) is not None + assert BlockDevices().get_block_device(origin) is not None + dummy_service.set_drive(origin) dummy_service.set_simulated_moves(False) - dummy_service.set_drive("sda2") + mutation_response = api_move(authorized_client, dummy_service, target) data = get_data(mutation_response)["services"]["moveService"]