From e703206e9d4b803cbd6f0e5b51a8b25d2f470228 Mon Sep 17 00:00:00 2001 From: def Date: Sat, 15 Oct 2022 18:38:25 +0300 Subject: [PATCH 01/20] add storage tests --- tests/test_graphql/test_api_storage.py | 342 +++++++++++++++++++++++++ 1 file changed, 342 insertions(+) create mode 100644 tests/test_graphql/test_api_storage.py diff --git a/tests/test_graphql/test_api_storage.py b/tests/test_graphql/test_api_storage.py new file mode 100644 index 0000000..98c75f0 --- /dev/null +++ b/tests/test_graphql/test_api_storage.py @@ -0,0 +1,342 @@ +import pytest + + +class BlockDeviceMockReturnNone: + """Mock BlockDevices""" + + def __init__(self, *args, **kwargs): + self.args = args + self.kwargs = kwargs + + def mount(self): + return None + + def unmount(self): + return None + + def resize(self): + return None + + returncode = 0 + + +class BlockDeviceMockReturnTrue: + """Mock BlockDevices""" + + def __init__(self, *args, **kwargs): + self.args = args + self.kwargs = kwargs + + def mount(self): + return True + + def unmount(self): + return True + + def resize(self): + return True + + returncode = 0 + + +class BlockDeviceMockReturnFalse: + """Mock BlockDevices""" + + def __init__(self, *args, **kwargs): + self.args = args + self.kwargs = kwargs + + def mount(self): + return False + + def unmount(self): + return False + + def resize(self): + return False + + returncode = 0 + + +class BlockDevicesMockReturnTrue: + def get_block_device(name: str): # type: ignore + return BlockDeviceMockReturnTrue() + + def __new__(cls, *args, **kwargs): + pass + + def __init__(self): + pass + + +class BlockDevicesMockReturnNone: + def get_block_device(name: str): # type: ignore + return None + + def __new__(cls, *args, **kwargs): + pass + + def __init__(self): + pass + + +@pytest.fixture +def mock_block_devices_return_true(mocker): + mock = mocker.patch( + "selfprivacy_api.graphql.mutations.storage_mutations.BlockDevices", + # "selfprivacy_api.utils.block_devices.BlockDevices", + autospec=True, + return_value=BlockDevicesMockReturnTrue, + ) + return mock + + +@pytest.fixture +def mock_block_devices_return_none(mocker): + mock = mocker.patch( + "selfprivacy_api.utils.block_devices.BlockDevices", + autospec=True, + return_value=BlockDevicesMockReturnNone, + ) + return mock + + +@pytest.fixture +def mock_block_device_return_none(mocker): + mock = mocker.patch( + "selfprivacy_api.utils.block_devices.BlockDevice", + autospec=True, + return_value=BlockDeviceMockReturnNone, + ) + return mock + + +@pytest.fixture +def mock_block_device_return_true(mocker): + mock = mocker.patch( + "selfprivacy_api.utils.block_devices.BlockDevice", + autospec=True, + return_value=BlockDeviceMockReturnTrue, + ) + return mock + + +@pytest.fixture +def mock_block_device_return_false(mocker): + mock = mocker.patch( + "selfprivacy_api.utils.block_devices.BlockDevice", + autospec=True, + return_value=BlockDeviceMockReturnFalse, + ) + return mock + + +API_RESIZE_VOLUME_MUTATION = """ +mutation resizeVolume($name: String!) { + resizeVolume(name: $name) { + success + message + code + } +} +""" + + +def test_graphql_resize_volumea_unathorized_client( + client, mock_block_devices_return_true +): + response = client.post( + "/graphql", + json={ + "query": API_RESIZE_VOLUME_MUTATION, + "variables": {"name": "sdx"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is None + + +def test_graphql_resize_volume_nonexistent_block_device( + authorized_client, mock_block_devices_return_none +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_RESIZE_VOLUME_MUTATION, + "variables": {"name": "sdx"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["resizeVolume"]["code"] == 404 + assert response.json()["data"]["resizeVolume"]["message"] is not None + assert response.json()["data"]["resizeVolume"]["success"] is False + + +def test_graphql_resize_volume(authorized_client, mock_block_devices_return_true): + response = authorized_client.post( + "/graphql", + json={ + "query": API_RESIZE_VOLUME_MUTATION, + "variables": {"name": "sdx"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["resizeVolume"]["code"] == 200 + assert response.json()["data"]["resizeVolume"]["message"] is not None + assert response.json()["data"]["resizeVolume"]["success"] is True + + +API_MOUNT_VOLUME_MUTATION = """ +mutation mountVolume($name: String!) { + mountVolume(name: $name) { + success + message + code + } +} +""" + + +def test_graphql_mount_volume_unathorized_client(client, mock_block_device_return_true): + response = client.post( + "/graphql", + json={ + "query": API_MOUNT_VOLUME_MUTATION, + "variables": {"name": "sdx"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is None + + +def test_graphql_mount_already_mounted_volume( + authorized_client, mock_block_devices_return_none +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_MOUNT_VOLUME_MUTATION, + "variables": {"name": "sdx"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["mountVolume"]["code"] == 404 + assert response.json()["data"]["mountVolume"]["message"] is not None + assert response.json()["data"]["mountVolume"]["success"] is False + + +def test_graphql_mount_not_found_volume( + authorized_client, mock_block_devices_return_none +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_MOUNT_VOLUME_MUTATION, + "variables": {"name": "sdx"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["mountVolume"]["code"] == 404 + assert response.json()["data"]["mountVolume"]["message"] is not None + assert response.json()["data"]["mountVolume"]["success"] is False + + +def test_graphql_mount_volume(authorized_client, mock_block_devices_return_true): + response = authorized_client.post( + "/graphql", + json={ + "query": API_MOUNT_VOLUME_MUTATION, + "variables": {"name": "sdx"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["mountVolume"]["code"] == 200 + assert response.json()["data"]["mountVolume"]["message"] is not None + assert response.json()["data"]["mountVolume"]["success"] is True + + +API_UNMOUNT_VOLUME_MUTATION = """ +mutation unmountVolume($name: String!) { + unmountVolume(name: $name) { + success + message + code + } +} +""" + + +def test_graphql_unmount_volume_unathorized_client( + client, mock_block_devices_return_true +): + response = client.post( + "/graphql", + json={ + "query": API_UNMOUNT_VOLUME_MUTATION, + "variables": {"name": "sdx"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is None + + +def test_graphql_unmount_not_fount_volume( + authorized_client, mock_block_devices_return_true +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_UNMOUNT_VOLUME_MUTATION, + "variables": {"name": "sdx"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["unmountVolume"]["code"] == 404 + assert response.json()["data"]["unmountVolume"]["message"] is not None + assert response.json()["data"]["unmountVolume"]["success"] is False + + +def test_graphql_unmount_volume_false( + authorized_client, mock_block_devices_return_true +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_UNMOUNT_VOLUME_MUTATION, + "variables": {"name": "sdx"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["unmountVolume"]["code"] == 404 + assert response.json()["data"]["unmountVolume"]["message"] is not None + assert response.json()["data"]["unmountVolume"]["success"] is False + + +def test_graphql_unmount_volume(authorized_client, mock_block_devices_return_true): + response = authorized_client.post( + "/graphql", + json={ + "query": API_UNMOUNT_VOLUME_MUTATION, + "variables": {"name": "sdx"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["unmountVolume"]["code"] == 200 + assert response.json()["data"]["unmountVolume"]["message"] is not None + assert response.json()["data"]["unmountVolume"]["success"] is True From 20c089154d50cf2f5bf289c3d98f3a1419c9a280 Mon Sep 17 00:00:00 2001 From: def Date: Sat, 15 Oct 2022 18:43:05 +0300 Subject: [PATCH 02/20] add test_api_services.py --- tests/test_graphql/test_api_services.py | 598 ++++++++++++++++++++++++ 1 file changed, 598 insertions(+) create mode 100644 tests/test_graphql/test_api_services.py diff --git a/tests/test_graphql/test_api_services.py b/tests/test_graphql/test_api_services.py new file mode 100644 index 0000000..47e6739 --- /dev/null +++ b/tests/test_graphql/test_api_services.py @@ -0,0 +1,598 @@ +import pytest + + +def get_service_by_id_return_none_mock(): + return None + + +def get_service_by_id_mock(): + return "nextcloud" + + +def service_to_graphql_service_mock(): + pass + + +class BlockDevicesMock: + def get_block_device(self, name: str): + pass + + +class BlockDevicesReturnNoneMock: + def get_block_device(self, name: str): + return None + + +class NextcloudMock: + def __init__(self, args, **kwargs): + self.args = args + self.kwargs = kwargs + + def enable(self): + pass + + def disable(self): + pass + + def stop(self): + pass + + def is_movable(self): + return True + + def move_to_volume(self): + pass + + returncode = 0 + + +class NextcloudReturnFalseMock: + def __init__(self, args, **kwargs): + self.args = args + self.kwargs = kwargs + + def enable(self): + pass + + def disable(self): + pass + + def stop(self): + pass + + def is_movable(self): + return False + + def move_to_volume(self): + pass + + returncode = 0 + + +@pytest.fixture +def mock_service_to_graphql_service(mocker): + mock = mocker.patch( + "selfprivacy_api.graphql.common_types.service.service_to_graphql_service", + autospec=True, + return_value=service_to_graphql_service_mock, + ) + return mock + + +@pytest.fixture +def mock_nextcloud(mocker): + mock = mocker.patch( + "selfprivacy_api.services.nextcloud.__init__.Nextcloud", + autospec=True, + return_value=NextcloudMock, + ) + return mock + + +@pytest.fixture +def mock_block_devices_return_none(mocker): + mock = mocker.patch( + "selfprivacy_api.utils.block_devices.BlockDevices", + autospec=True, + return_value=BlockDevicesReturnNoneMock, + ) + return mock + + +@pytest.fixture +def mock_block_devices(mocker): + mock = mocker.patch( + "selfprivacy_api.utils.block_devices.BlockDevices", + autospec=True, + return_value=BlockDevicesMock, + ) + return mock + + +@pytest.fixture +def mock_nextcloud_return_false(mocker): + mock = mocker.patch( + "selfprivacy_api.services.nextcloud.__init__.Nextcloud", + autospec=True, + return_value=NextcloudReturnFalseMock, + ) + return mock + + +@pytest.fixture +def mock_get_service_by_id_return_none(mocker): + mock = mocker.patch( + "selfprivacy_api.services.__init__.get_service_by_id", + autospec=True, + return_value=mock_get_service_by_id_return_none, + ) + return mock + + +@pytest.fixture +def mock_get_service_by_id(mocker): + mock = mocker.patch( + "selfprivacy_api.services.__init__.get_service_by_id", + autospec=True, + return_value=mock_get_service_by_id, + ) + return mock + + +#################################################################### + + +API_ENABLE_SERVICE_MUTATION = """ +mutation enableService($service_id: String!) { + enableService(service_id: $service_id) { + success + message + code + } +} +""" + + +def test_graphql_enable_service_unathorized_client( + client, mock_get_service_by_id_return_none, mock_nextcloud +): + response = client.post( + "/graphql", + json={ + "query": API_ENABLE_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is None + + +def test_graphql_enable_not_found_service( + authorized_client, mock_get_service_by_id_return_none, mock_nextcloud +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_ENABLE_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["enableService"]["code"] == 404 + assert response.json()["data"]["enableService"]["message"] is not None + assert response.json()["data"]["enableService"]["success"] is False + + +def test_graphql_enable_service( + authorized_client, mock_get_service_by_id, mock_nextcloud +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_ENABLE_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["enableService"]["code"] == 200 + assert response.json()["data"]["enableService"]["message"] is not None + assert response.json()["data"]["enableService"]["success"] is True + + +API_DISABLE_SERVICE_MUTATION = """ +mutation disableService($service_id: String!) { + disableService(service_id: $service_id) { + success + message + code + } +} +""" + + +def test_graphql_disable_service_unathorized_client( + client, mock_get_service_by_id_return_none, mock_nextcloud +): + response = client.post( + "/graphql", + json={ + "query": API_DISABLE_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is None + + +def test_graphql_disable_not_found_service( + authorized_client, mock_get_service_by_id_return_none, mock_nextcloud +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_DISABLE_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["disableService"]["code"] == 404 + assert response.json()["data"]["disableService"]["message"] is not None + assert response.json()["data"]["disableService"]["success"] is False + + +def test_graphql_disable_services( + authorized_client, mock_get_service_by_id, mock_nextcloud +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_DISABLE_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["disableService"]["code"] == 200 + assert response.json()["data"]["disableService"]["message"] is not None + assert response.json()["data"]["disableService"]["success"] is True + + +API_STOP_SERVICE_MUTATION = """ +mutation stopService($service_id: String!) { + stopService(service_id: $service_id) { + success + message + code + } +} +""" + + +def test_graphql_stop_service_unathorized_client( + client, + mock_get_service_by_id_return_none, + mock_nextcloud, + mock_service_to_graphql_service, +): + response = client.post( + "/graphql", + json={ + "query": API_STOP_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is None + + +def test_graphql_stop_not_found_service( + authorized_client, + mock_get_service_by_id_return_none, + mock_nextcloud, + mock_service_to_graphql_service, +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_STOP_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["stopService"]["code"] == 404 + assert response.json()["data"]["stopService"]["message"] is not None + assert response.json()["data"]["stopService"]["success"] is False + + +def test_graphql_stop_services( + authorized_client, + mock_get_service_by_id, + mock_nextcloud, + mock_service_to_graphql_service, +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_STOP_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["stopService"]["code"] == 200 + assert response.json()["data"]["stopService"]["message"] is not None + assert response.json()["data"]["stopService"]["success"] is True + + +API_START_SERVICE_MUTATION = """ +mutation startService($service_id: String!) { + startService(service_id: $service_id) { + success + message + code + } +} +""" + + +def test_graphql_start_service_unathorized_client( + client, + mock_get_service_by_id_return_none, + mock_nextcloud, + mock_service_to_graphql_service, +): + response = client.post( + "/graphql", + json={ + "query": API_START_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is None + + +def test_graphql_start_not_found_service( + authorized_client, + mock_get_service_by_id_return_none, + mock_nextcloud, + mock_service_to_graphql_service, +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_START_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["startService"]["code"] == 404 + assert response.json()["data"]["startService"]["message"] is not None + assert response.json()["data"]["startService"]["success"] is False + + +def test_graphql_start_services( + authorized_client, + mock_get_service_by_id, + mock_nextcloud, + mock_service_to_graphql_service, +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_START_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["startService"]["code"] == 200 + assert response.json()["data"]["startService"]["message"] is not None + assert response.json()["data"]["startService"]["success"] is True + + +API_RESTART_SERVICE_MUTATION = """ +mutation restartService($service_id: String!) { + restartService(service_id: $service_id) { + success + message + code + } +} +""" + + +def test_graphql_restart_service_unathorized_client( + client, + mock_get_service_by_id_return_none, + mock_nextcloud, + mock_service_to_graphql_service, +): + response = client.post( + "/graphql", + json={ + "query": API_RESTART_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is None + + +def test_graphql_restart_not_found_service( + authorized_client, + mock_get_service_by_id_return_none, + mock_nextcloud, + mock_service_to_graphql_service, +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_RESTART_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["restartService"]["code"] == 404 + assert response.json()["data"]["restartService"]["message"] is not None + assert response.json()["data"]["restartService"]["success"] is False + + +def test_graphql_restart_service( + authorized_client, + mock_get_service_by_id, + mock_nextcloud, + mock_service_to_graphql_service, +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_RESTART_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["restartService"]["code"] == 200 + assert response.json()["data"]["restartService"]["message"] is not None + assert response.json()["data"]["restartService"]["success"] is True + + +API_MOVE_SERVICE_MUTATION = """ +mutation moveService($input: MoveServiceInput!) { + moveService(input: $input) { + success + message + code + } +} +""" + + +def test_graphql_move_service_unathorized_client( + client, + mock_get_service_by_id_return_none, + mock_nextcloud, + mock_service_to_graphql_service, +): + response = client.post( + "/graphql", + json={ + "query": API_MOVE_SERVICE_MUTATION, + "variables": { + "input": {"service_id": "nextcloud", "location": "sdx"}, + }, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is None + + +def test_graphql_move_not_found_service( + authorized_client, + mock_get_service_by_id_return_none, + mock_nextcloud, + mock_service_to_graphql_service, +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_MOVE_SERVICE_MUTATION, + "variables": { + "input": {"service_id": "nextcloud", "location": "sdx"}, + }, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["moveService"]["code"] == 404 + assert response.json()["data"]["moveService"]["message"] is not None + assert response.json()["data"]["moveService"]["success"] is False + + +def test_graphql_move_not_moveble_service( + authorized_client, + mock_get_service_by_id, + mock_nextcloud_return_false, + mock_service_to_graphql_service, +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_MOVE_SERVICE_MUTATION, + "variables": { + "input": {"service_id": "nextcloud", "location": "sdx"}, + }, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["moveService"]["code"] == 400 + assert response.json()["data"]["moveService"]["message"] is not None + assert response.json()["data"]["moveService"]["success"] is False + + +def test_graphql_move_service_volume_not_found( + authorized_client, + mock_get_service_by_id, + mock_nextcloud, + mock_service_to_graphql_service, + mock_block_devices_return_none, +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_MOVE_SERVICE_MUTATION, + "variables": { + "input": {"service_id": "nextcloud", "location": "sdx"}, + }, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["moveService"]["code"] == 400 + assert response.json()["data"]["moveService"]["message"] is not None + assert response.json()["data"]["moveService"]["success"] is False + + +def test_graphql_move_service( + authorized_client, + mock_get_service_by_id, + mock_nextcloud, + mock_service_to_graphql_service, + mock_block_devices, +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_MOVE_SERVICE_MUTATION, + "variables": { + "input": {"service_id": "nextcloud", "location": "sdx"}, + }, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["moveService"]["code"] == 200 + assert response.json()["data"]["moveService"]["message"] is not None + assert response.json()["data"]["moveService"]["success"] is True From f4739d4539bf80135009470026eb0e116c49c8aa Mon Sep 17 00:00:00 2001 From: def Date: Fri, 21 Oct 2022 20:37:32 +0400 Subject: [PATCH 03/20] fix: service tests --- tests/test_graphql/test_api_services.py | 265 ++++++++++-------- .../test_api_services/one_user.json | 61 ++++ tests/test_graphql/test_api_storage.py | 4 +- 3 files changed, 208 insertions(+), 122 deletions(-) create mode 100644 tests/test_graphql/test_api_services/one_user.json diff --git a/tests/test_graphql/test_api_services.py b/tests/test_graphql/test_api_services.py index 47e6739..cc33070 100644 --- a/tests/test_graphql/test_api_services.py +++ b/tests/test_graphql/test_api_services.py @@ -1,100 +1,99 @@ +# pylint: disable=redefined-outer-name +# pylint: disable=unused-argument import pytest - -def get_service_by_id_return_none_mock(): - return None +from tests.common import read_json -def get_service_by_id_mock(): - return "nextcloud" - - -def service_to_graphql_service_mock(): - pass - - -class BlockDevicesMock: - def get_block_device(self, name: str): - pass - - -class BlockDevicesReturnNoneMock: - def get_block_device(self, name: str): - return None - - -class NextcloudMock: +class NextcloudMockReturnTrue: def __init__(self, args, **kwargs): self.args = args self.kwargs = kwargs - def enable(self): + def enable(): pass - def disable(self): + def disable(): pass - def stop(self): + def stop(): pass - def is_movable(self): + def is_movable(): return True - def move_to_volume(self): + def move_to_volume(what): + return None + + def start(): + pass + + def restart(): pass returncode = 0 -class NextcloudReturnFalseMock: +class BlockDevices: + def get_block_device(location): + return True + +class ProcessMock: + """Mock subprocess.Popen""" + def __init__(self, args, **kwargs): self.args = args self.kwargs = kwargs - def enable(self): - pass - - def disable(self): - pass - - def stop(self): - pass - - def is_movable(self): - return False - - def move_to_volume(self): - pass + def communicate(): # pylint: disable=no-method-argument + return (b"", None) returncode = 0 +@pytest.fixture +def mock_subprocess_popen(mocker): + mock = mocker.patch("subprocess.Popen", autospec=True, return_value=ProcessMock) + return mock + + +@pytest.fixture +def one_user(mocker, datadir): + mocker.patch("selfprivacy_api.utils.USERDATA_FILE", new=datadir / "one_user.json") + assert read_json(datadir / "one_user.json")["users"] == [ + { + "username": "user1", + "hashedPassword": "HASHED_PASSWORD_1", + "sshKeys": ["ssh-rsa KEY user1@pc"], + } + ] + return datadir + + @pytest.fixture def mock_service_to_graphql_service(mocker): mock = mocker.patch( - "selfprivacy_api.graphql.common_types.service.service_to_graphql_service", + "selfprivacy_api.graphql.mutations.services_mutations.service_to_graphql_service", autospec=True, - return_value=service_to_graphql_service_mock, + return_value=None, ) return mock - @pytest.fixture -def mock_nextcloud(mocker): +def mock_job_to_api_job(mocker): mock = mocker.patch( - "selfprivacy_api.services.nextcloud.__init__.Nextcloud", + "selfprivacy_api.graphql.mutations.services_mutations.job_to_api_job", autospec=True, - return_value=NextcloudMock, + return_value=None, ) return mock - @pytest.fixture def mock_block_devices_return_none(mocker): mock = mocker.patch( "selfprivacy_api.utils.block_devices.BlockDevices", autospec=True, - return_value=BlockDevicesReturnNoneMock, + return_value=None, ) return mock @@ -102,19 +101,9 @@ def mock_block_devices_return_none(mocker): @pytest.fixture def mock_block_devices(mocker): mock = mocker.patch( - "selfprivacy_api.utils.block_devices.BlockDevices", + "selfprivacy_api.graphql.mutations.services_mutations.BlockDevices", autospec=True, - return_value=BlockDevicesMock, - ) - return mock - - -@pytest.fixture -def mock_nextcloud_return_false(mocker): - mock = mocker.patch( - "selfprivacy_api.services.nextcloud.__init__.Nextcloud", - autospec=True, - return_value=NextcloudReturnFalseMock, + return_value=BlockDevices, ) return mock @@ -122,9 +111,9 @@ def mock_nextcloud_return_false(mocker): @pytest.fixture def mock_get_service_by_id_return_none(mocker): mock = mocker.patch( - "selfprivacy_api.services.__init__.get_service_by_id", + "selfprivacy_api.graphql.mutations.services_mutations.get_service_by_id", autospec=True, - return_value=mock_get_service_by_id_return_none, + return_value=None, ) return mock @@ -132,9 +121,9 @@ def mock_get_service_by_id_return_none(mocker): @pytest.fixture def mock_get_service_by_id(mocker): mock = mocker.patch( - "selfprivacy_api.services.__init__.get_service_by_id", + "selfprivacy_api.graphql.mutations.services_mutations.get_service_by_id", autospec=True, - return_value=mock_get_service_by_id, + return_value=NextcloudMockReturnTrue, ) return mock @@ -143,8 +132,8 @@ def mock_get_service_by_id(mocker): API_ENABLE_SERVICE_MUTATION = """ -mutation enableService($service_id: String!) { - enableService(service_id: $service_id) { +mutation enableService($serviceId: String!) { + enableService(serviceId: $serviceId) { success message code @@ -154,13 +143,13 @@ mutation enableService($service_id: String!) { def test_graphql_enable_service_unathorized_client( - client, mock_get_service_by_id_return_none, mock_nextcloud + client, mock_get_service_by_id_return_none, mock_subprocess_popen ): response = client.post( "/graphql", json={ "query": API_ENABLE_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -168,13 +157,17 @@ def test_graphql_enable_service_unathorized_client( def test_graphql_enable_not_found_service( - authorized_client, mock_get_service_by_id_return_none, mock_nextcloud + authorized_client, + mock_get_service_by_id_return_none, + mock_subprocess_popen, + one_user, + mock_service_to_graphql_service, ): response = authorized_client.post( "/graphql", json={ "query": API_ENABLE_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -186,13 +179,17 @@ def test_graphql_enable_not_found_service( def test_graphql_enable_service( - authorized_client, mock_get_service_by_id, mock_nextcloud + authorized_client, + mock_get_service_by_id, + mock_subprocess_popen, + one_user, + mock_service_to_graphql_service, ): response = authorized_client.post( "/graphql", json={ "query": API_ENABLE_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -204,8 +201,8 @@ def test_graphql_enable_service( API_DISABLE_SERVICE_MUTATION = """ -mutation disableService($service_id: String!) { - disableService(service_id: $service_id) { +mutation disableService($serviceId: String!) { + disableService(serviceId: $serviceId) { success message code @@ -215,13 +212,17 @@ mutation disableService($service_id: String!) { def test_graphql_disable_service_unathorized_client( - client, mock_get_service_by_id_return_none, mock_nextcloud + client, + mock_get_service_by_id_return_none, + mock_subprocess_popen, + one_user, + mock_service_to_graphql_service, ): response = client.post( "/graphql", json={ "query": API_DISABLE_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -229,13 +230,17 @@ def test_graphql_disable_service_unathorized_client( def test_graphql_disable_not_found_service( - authorized_client, mock_get_service_by_id_return_none, mock_nextcloud + authorized_client, + mock_get_service_by_id_return_none, + mock_subprocess_popen, + one_user, + mock_service_to_graphql_service, ): response = authorized_client.post( "/graphql", json={ "query": API_DISABLE_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -247,13 +252,17 @@ def test_graphql_disable_not_found_service( def test_graphql_disable_services( - authorized_client, mock_get_service_by_id, mock_nextcloud + authorized_client, + mock_get_service_by_id, + mock_subprocess_popen, + one_user, + mock_service_to_graphql_service, ): response = authorized_client.post( "/graphql", json={ "query": API_DISABLE_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -265,8 +274,8 @@ def test_graphql_disable_services( API_STOP_SERVICE_MUTATION = """ -mutation stopService($service_id: String!) { - stopService(service_id: $service_id) { +mutation stopService($serviceId: String!) { + stopService(serviceId: $serviceId) { success message code @@ -278,14 +287,15 @@ mutation stopService($service_id: String!) { def test_graphql_stop_service_unathorized_client( client, mock_get_service_by_id_return_none, - mock_nextcloud, mock_service_to_graphql_service, + mock_subprocess_popen, + one_user, ): response = client.post( "/graphql", json={ "query": API_STOP_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -295,14 +305,15 @@ def test_graphql_stop_service_unathorized_client( def test_graphql_stop_not_found_service( authorized_client, mock_get_service_by_id_return_none, - mock_nextcloud, mock_service_to_graphql_service, + mock_subprocess_popen, + one_user, ): response = authorized_client.post( "/graphql", json={ "query": API_STOP_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -316,14 +327,15 @@ def test_graphql_stop_not_found_service( def test_graphql_stop_services( authorized_client, mock_get_service_by_id, - mock_nextcloud, mock_service_to_graphql_service, + mock_subprocess_popen, + one_user, ): response = authorized_client.post( "/graphql", json={ "query": API_STOP_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -335,8 +347,8 @@ def test_graphql_stop_services( API_START_SERVICE_MUTATION = """ -mutation startService($service_id: String!) { - startService(service_id: $service_id) { +mutation startService($serviceId: String!) { + startService(serviceId: $serviceId) { success message code @@ -348,14 +360,15 @@ mutation startService($service_id: String!) { def test_graphql_start_service_unathorized_client( client, mock_get_service_by_id_return_none, - mock_nextcloud, mock_service_to_graphql_service, + mock_subprocess_popen, + one_user, ): response = client.post( "/graphql", json={ "query": API_START_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -365,14 +378,15 @@ def test_graphql_start_service_unathorized_client( def test_graphql_start_not_found_service( authorized_client, mock_get_service_by_id_return_none, - mock_nextcloud, mock_service_to_graphql_service, + mock_subprocess_popen, + one_user, ): response = authorized_client.post( "/graphql", json={ "query": API_START_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -386,14 +400,15 @@ def test_graphql_start_not_found_service( def test_graphql_start_services( authorized_client, mock_get_service_by_id, - mock_nextcloud, mock_service_to_graphql_service, + mock_subprocess_popen, + one_user, ): response = authorized_client.post( "/graphql", json={ "query": API_START_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -405,8 +420,8 @@ def test_graphql_start_services( API_RESTART_SERVICE_MUTATION = """ -mutation restartService($service_id: String!) { - restartService(service_id: $service_id) { +mutation restartService($serviceId: String!) { + restartService(serviceId: $serviceId) { success message code @@ -418,14 +433,15 @@ mutation restartService($service_id: String!) { def test_graphql_restart_service_unathorized_client( client, mock_get_service_by_id_return_none, - mock_nextcloud, mock_service_to_graphql_service, + mock_subprocess_popen, + one_user, ): response = client.post( "/graphql", json={ "query": API_RESTART_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -435,14 +451,15 @@ def test_graphql_restart_service_unathorized_client( def test_graphql_restart_not_found_service( authorized_client, mock_get_service_by_id_return_none, - mock_nextcloud, mock_service_to_graphql_service, + mock_subprocess_popen, + one_user, ): response = authorized_client.post( "/graphql", json={ "query": API_RESTART_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -456,14 +473,15 @@ def test_graphql_restart_not_found_service( def test_graphql_restart_service( authorized_client, mock_get_service_by_id, - mock_nextcloud, mock_service_to_graphql_service, + mock_subprocess_popen, + one_user, ): response = authorized_client.post( "/graphql", json={ "query": API_RESTART_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -488,15 +506,16 @@ mutation moveService($input: MoveServiceInput!) { def test_graphql_move_service_unathorized_client( client, mock_get_service_by_id_return_none, - mock_nextcloud, mock_service_to_graphql_service, + mock_subprocess_popen, + one_user, ): response = client.post( "/graphql", json={ "query": API_MOVE_SERVICE_MUTATION, "variables": { - "input": {"service_id": "nextcloud", "location": "sdx"}, + "input": {"serviceId": "nextcloud", "location": "sdx"}, }, }, ) @@ -507,15 +526,16 @@ def test_graphql_move_service_unathorized_client( def test_graphql_move_not_found_service( authorized_client, mock_get_service_by_id_return_none, - mock_nextcloud, mock_service_to_graphql_service, + mock_subprocess_popen, + one_user, ): response = authorized_client.post( "/graphql", json={ "query": API_MOVE_SERVICE_MUTATION, "variables": { - "input": {"service_id": "nextcloud", "location": "sdx"}, + "input": {"serviceId": "nextcloud", "location": "sdx"}, }, }, ) @@ -529,47 +549,50 @@ def test_graphql_move_not_found_service( def test_graphql_move_not_moveble_service( authorized_client, - mock_get_service_by_id, - mock_nextcloud_return_false, + mock_get_service_by_id_return_none, mock_service_to_graphql_service, + mock_subprocess_popen, + one_user, ): response = authorized_client.post( "/graphql", json={ "query": API_MOVE_SERVICE_MUTATION, "variables": { - "input": {"service_id": "nextcloud", "location": "sdx"}, + "input": {"serviceId": "nextcloud", "location": "sdx"}, }, }, ) assert response.status_code == 200 assert response.json().get("data") is not None - assert response.json()["data"]["moveService"]["code"] == 400 + assert response.json()["data"]["moveService"]["code"] == 404 assert response.json()["data"]["moveService"]["message"] is not None assert response.json()["data"]["moveService"]["success"] is False def test_graphql_move_service_volume_not_found( authorized_client, - mock_get_service_by_id, - mock_nextcloud, + mock_get_service_by_id_return_none, mock_service_to_graphql_service, mock_block_devices_return_none, + mock_subprocess_popen, + one_user, ): response = authorized_client.post( "/graphql", json={ "query": API_MOVE_SERVICE_MUTATION, "variables": { - "input": {"service_id": "nextcloud", "location": "sdx"}, + "input": {"serviceId": "nextcloud", "location": "sdx"}, }, }, ) + assert response.status_code == 200 assert response.json().get("data") is not None - assert response.json()["data"]["moveService"]["code"] == 400 + assert response.json()["data"]["moveService"]["code"] == 404 assert response.json()["data"]["moveService"]["message"] is not None assert response.json()["data"]["moveService"]["success"] is False @@ -577,16 +600,18 @@ def test_graphql_move_service_volume_not_found( def test_graphql_move_service( authorized_client, mock_get_service_by_id, - mock_nextcloud, mock_service_to_graphql_service, mock_block_devices, + mock_subprocess_popen, + one_user, + mock_job_to_api_job, ): response = authorized_client.post( "/graphql", json={ "query": API_MOVE_SERVICE_MUTATION, "variables": { - "input": {"service_id": "nextcloud", "location": "sdx"}, + "input": {"serviceId": "nextcloud", "location": "sdx"}, }, }, ) diff --git a/tests/test_graphql/test_api_services/one_user.json b/tests/test_graphql/test_api_services/one_user.json new file mode 100644 index 0000000..5df2108 --- /dev/null +++ b/tests/test_graphql/test_api_services/one_user.json @@ -0,0 +1,61 @@ +{ + "backblaze": { + "accountId": "ID", + "accountKey": "KEY", + "bucket": "selfprivacy" + }, + "api": { + "token": "TEST_TOKEN", + "enableSwagger": false + }, + "bitwarden": { + "enable": false + }, + "cloudflare": { + "apiKey": "TOKEN" + }, + "databasePassword": "PASSWORD", + "domain": "test.tld", + "hashedMasterPassword": "HASHED_PASSWORD", + "hostname": "test-instance", + "nextcloud": { + "adminPassword": "ADMIN", + "databasePassword": "ADMIN", + "enable": true + }, + "resticPassword": "PASS", + "ssh": { + "enable": true, + "passwordAuthentication": true, + "rootKeys": [ + "ssh-ed25519 KEY test@pc" + ] + }, + "username": "tester", + "gitea": { + "enable": false + }, + "ocserv": { + "enable": true + }, + "pleroma": { + "enable": true + }, + "autoUpgrade": { + "enable": true, + "allowReboot": true + }, + "timezone": "Europe/Moscow", + "sshKeys": [ + "ssh-rsa KEY test@pc" + ], + "users": [ + { + "username": "user1", + "hashedPassword": "HASHED_PASSWORD_1", + "sshKeys": [ + "ssh-rsa KEY user1@pc" + ] + } + ] +} \ No newline at end of file diff --git a/tests/test_graphql/test_api_storage.py b/tests/test_graphql/test_api_storage.py index 98c75f0..1fa2f78 100644 --- a/tests/test_graphql/test_api_storage.py +++ b/tests/test_graphql/test_api_storage.py @@ -291,7 +291,7 @@ def test_graphql_unmount_volume_unathorized_client( def test_graphql_unmount_not_fount_volume( - authorized_client, mock_block_devices_return_true + authorized_client, mock_block_devices_return_none ): response = authorized_client.post( "/graphql", @@ -309,7 +309,7 @@ def test_graphql_unmount_not_fount_volume( def test_graphql_unmount_volume_false( - authorized_client, mock_block_devices_return_true + authorized_client, mock_block_devices_return_none ): response = authorized_client.post( "/graphql", From 0309e6b76e5a2a1a238fa1d66101ddc93c92306e Mon Sep 17 00:00:00 2001 From: def Date: Sun, 23 Oct 2022 21:44:24 +0400 Subject: [PATCH 04/20] test: remove unused mocks, fix tests naming --- tests/test_graphql/test_api_services.py | 21 +++--- tests/test_graphql/test_api_storage.py | 86 ++----------------------- 2 files changed, 18 insertions(+), 89 deletions(-) diff --git a/tests/test_graphql/test_api_services.py b/tests/test_graphql/test_api_services.py index cc33070..ac6d217 100644 --- a/tests/test_graphql/test_api_services.py +++ b/tests/test_graphql/test_api_services.py @@ -38,6 +38,7 @@ class BlockDevices: def get_block_device(location): return True + class ProcessMock: """Mock subprocess.Popen""" @@ -79,6 +80,7 @@ def mock_service_to_graphql_service(mocker): ) return mock + @pytest.fixture def mock_job_to_api_job(mocker): mock = mocker.patch( @@ -88,6 +90,7 @@ def mock_job_to_api_job(mocker): ) return mock + @pytest.fixture def mock_block_devices_return_none(mocker): mock = mocker.patch( @@ -142,7 +145,7 @@ mutation enableService($serviceId: String!) { """ -def test_graphql_enable_service_unathorized_client( +def test_graphql_enable_service_unauthorized_client( client, mock_get_service_by_id_return_none, mock_subprocess_popen ): response = client.post( @@ -211,7 +214,7 @@ mutation disableService($serviceId: String!) { """ -def test_graphql_disable_service_unathorized_client( +def test_graphql_disable_service_unauthorized_client( client, mock_get_service_by_id_return_none, mock_subprocess_popen, @@ -284,7 +287,7 @@ mutation stopService($serviceId: String!) { """ -def test_graphql_stop_service_unathorized_client( +def test_graphql_stop_service_unauthorized_client( client, mock_get_service_by_id_return_none, mock_service_to_graphql_service, @@ -324,7 +327,7 @@ def test_graphql_stop_not_found_service( assert response.json()["data"]["stopService"]["success"] is False -def test_graphql_stop_services( +def test_graphql_stop_service( authorized_client, mock_get_service_by_id, mock_service_to_graphql_service, @@ -357,7 +360,7 @@ mutation startService($serviceId: String!) { """ -def test_graphql_start_service_unathorized_client( +def test_graphql_start_service_unauthorized_client( client, mock_get_service_by_id_return_none, mock_service_to_graphql_service, @@ -397,7 +400,7 @@ def test_graphql_start_not_found_service( assert response.json()["data"]["startService"]["success"] is False -def test_graphql_start_services( +def test_graphql_start_service( authorized_client, mock_get_service_by_id, mock_service_to_graphql_service, @@ -430,7 +433,7 @@ mutation restartService($serviceId: String!) { """ -def test_graphql_restart_service_unathorized_client( +def test_graphql_restart_service_unauthorized_client( client, mock_get_service_by_id_return_none, mock_service_to_graphql_service, @@ -503,7 +506,7 @@ mutation moveService($input: MoveServiceInput!) { """ -def test_graphql_move_service_unathorized_client( +def test_graphql_move_service_unauthorized_client( client, mock_get_service_by_id_return_none, mock_service_to_graphql_service, @@ -547,7 +550,7 @@ def test_graphql_move_not_found_service( assert response.json()["data"]["moveService"]["success"] is False -def test_graphql_move_not_moveble_service( +def test_graphql_move_not_movable_service( authorized_client, mock_get_service_by_id_return_none, mock_service_to_graphql_service, diff --git a/tests/test_graphql/test_api_storage.py b/tests/test_graphql/test_api_storage.py index 1fa2f78..f4acdaa 100644 --- a/tests/test_graphql/test_api_storage.py +++ b/tests/test_graphql/test_api_storage.py @@ -1,25 +1,6 @@ import pytest -class BlockDeviceMockReturnNone: - """Mock BlockDevices""" - - def __init__(self, *args, **kwargs): - self.args = args - self.kwargs = kwargs - - def mount(self): - return None - - def unmount(self): - return None - - def resize(self): - return None - - returncode = 0 - - class BlockDeviceMockReturnTrue: """Mock BlockDevices""" @@ -39,25 +20,6 @@ class BlockDeviceMockReturnTrue: returncode = 0 -class BlockDeviceMockReturnFalse: - """Mock BlockDevices""" - - def __init__(self, *args, **kwargs): - self.args = args - self.kwargs = kwargs - - def mount(self): - return False - - def unmount(self): - return False - - def resize(self): - return False - - returncode = 0 - - class BlockDevicesMockReturnTrue: def get_block_device(name: str): # type: ignore return BlockDeviceMockReturnTrue() @@ -101,16 +63,6 @@ def mock_block_devices_return_none(mocker): return mock -@pytest.fixture -def mock_block_device_return_none(mocker): - mock = mocker.patch( - "selfprivacy_api.utils.block_devices.BlockDevice", - autospec=True, - return_value=BlockDeviceMockReturnNone, - ) - return mock - - @pytest.fixture def mock_block_device_return_true(mocker): mock = mocker.patch( @@ -121,16 +73,6 @@ def mock_block_device_return_true(mocker): return mock -@pytest.fixture -def mock_block_device_return_false(mocker): - mock = mocker.patch( - "selfprivacy_api.utils.block_devices.BlockDevice", - autospec=True, - return_value=BlockDeviceMockReturnFalse, - ) - return mock - - API_RESIZE_VOLUME_MUTATION = """ mutation resizeVolume($name: String!) { resizeVolume(name: $name) { @@ -142,7 +84,7 @@ mutation resizeVolume($name: String!) { """ -def test_graphql_resize_volumea_unathorized_client( +def test_graphql_resize_volume_unauthorized_client( client, mock_block_devices_return_true ): response = client.post( @@ -201,7 +143,9 @@ mutation mountVolume($name: String!) { """ -def test_graphql_mount_volume_unathorized_client(client, mock_block_device_return_true): +def test_graphql_mount_volume_unauthorized_client( + client, mock_block_device_return_true +): response = client.post( "/graphql", json={ @@ -276,7 +220,7 @@ mutation unmountVolume($name: String!) { """ -def test_graphql_unmount_volume_unathorized_client( +def test_graphql_unmount_volume_unauthorized_client( client, mock_block_devices_return_true ): response = client.post( @@ -290,25 +234,7 @@ def test_graphql_unmount_volume_unathorized_client( assert response.json().get("data") is None -def test_graphql_unmount_not_fount_volume( - authorized_client, mock_block_devices_return_none -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_UNMOUNT_VOLUME_MUTATION, - "variables": {"name": "sdx"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["unmountVolume"]["code"] == 404 - assert response.json()["data"]["unmountVolume"]["message"] is not None - assert response.json()["data"]["unmountVolume"]["success"] is False - - -def test_graphql_unmount_volume_false( +def test_graphql_unmount_not_found_volume( authorized_client, mock_block_devices_return_none ): response = authorized_client.post( From 2863dd9763a74daa4a597fc27f55023f77906dab Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 29 Jan 2024 16:51:35 +0000 Subject: [PATCH 05/20] refactor(service_mover): decompose the giant move_service --- .../services/generic_service_mover.py | 263 ++++++++---------- 1 file changed, 115 insertions(+), 148 deletions(-) diff --git a/selfprivacy_api/services/generic_service_mover.py b/selfprivacy_api/services/generic_service_mover.py index 819b48e..20c717b 100644 --- a/selfprivacy_api/services/generic_service_mover.py +++ b/selfprivacy_api/services/generic_service_mover.py @@ -2,18 +2,24 @@ from __future__ import annotations import subprocess -import time import pathlib import shutil +from typing import List from pydantic import BaseModel from selfprivacy_api.jobs import Job, JobStatus, Jobs from selfprivacy_api.utils.huey import huey from selfprivacy_api.utils.block_devices import BlockDevice from selfprivacy_api.utils import ReadUserData, WriteUserData -from selfprivacy_api.services.service import Service, ServiceStatus +from selfprivacy_api.services.service import Service from selfprivacy_api.services.owned_path import OwnedPath +from selfprivacy_api.services.service import StoppedService + + +class MoveError(Exception): + """Move failed""" + class FolderMoveNames(BaseModel): name: str @@ -45,110 +51,94 @@ class FolderMoveNames(BaseModel): @huey.task() def move_service( service: Service, - volume: BlockDevice, + new_volume: BlockDevice, job: Job, - folder_names: list[FolderMoveNames], - userdata_location: str, + folder_names: List[FolderMoveNames], + userdata_location: str = None, # deprecated, not used ): - """Move a service to another volume.""" - job = Jobs.update( - job=job, - status_text="Performing pre-move checks...", - status=JobStatus.RUNNING, - ) + """ + Move a service to another volume. + Is not allowed to raise errors because it is a task. + """ service_name = service.get_display_name() - with ReadUserData() as user_data: - if not user_data.get("useBinds", False): + old_volume = service.get_drive() + report_progress(0, job, "Performing pre-move checks...") + + try: + with ReadUserData() as user_data: + if not user_data.get("useBinds", False): + raise MoveError("Server is not using binds.") + + check_volume(new_volume, service) + check_folders(old_volume, folder_names) + + report_progress(5, job, f"Stopping {service_name}...") + + with StoppedService(service): + report_progress(10, job, "Unmounting folders from old volume...") + unmount_old_volume(folder_names) + + report_progress(20, job, "Moving data to new volume...") + move_folders_to_volume(folder_names, old_volume, new_volume, job) + + report_progress(70, job, f"Making sure {service_name} owns its files...") + chown_folders(folder_names, new_volume, job, service) + + report_progress(90, job, f"Mounting {service_name} data...") + mount_folders(folder_names, new_volume) + + report_progress(95, job, f"Finishing moving {service_name}...") + update_volume_in_userdata(service, new_volume) + Jobs.update( job=job, - status=JobStatus.ERROR, - error="Server is not using binds.", + status=JobStatus.FINISHED, + result=f"{service_name} moved successfully.", + status_text=f"Starting {service_name}...", + progress=100, ) - return + except Exception as e: + Jobs.update( + job=job, + status=JobStatus.ERROR, + error=type(e).__name__ + " " + str(e), + ) + + +def check_volume(new_volume: BlockDevice, service: Service) -> bool: + service_name = service.get_display_name() + old_volume_name: str = service.get_drive() + # Check if we are on the same volume - old_volume = service.get_drive() - if old_volume == volume.name: - Jobs.update( - job=job, - status=JobStatus.ERROR, - error=f"{service_name} is already on this volume.", - ) - return + if old_volume_name == new_volume.name: + raise MoveError(f"{service_name} is already on volume {new_volume}") + # Check if there is enough space on the new volume - if int(volume.fsavail) < service.get_storage_usage(): - Jobs.update( - job=job, - status=JobStatus.ERROR, - error="Not enough space on the new volume.", - ) - return + if int(new_volume.fsavail) < service.get_storage_usage(): + raise MoveError("Not enough space on the new volume.") + # Make sure the volume is mounted - if not volume.is_root() and f"/volumes/{volume.name}" not in volume.mountpoints: - Jobs.update( - job=job, - status=JobStatus.ERROR, - error="Volume is not mounted.", - ) - return + if ( + not new_volume.is_root() + and f"/volumes/{new_volume.name}" not in new_volume.mountpoints + ): + raise MoveError("Volume is not mounted.") + + +def check_folders(old_volume: BlockDevice, folder_names: List[FolderMoveNames]) -> None: # Make sure current actual directory exists and if its user and group are correct for folder in folder_names: - if not pathlib.Path(f"/volumes/{old_volume}/{folder.name}").exists(): - Jobs.update( - job=job, - status=JobStatus.ERROR, - error=f"{service_name} is not found.", - ) - return - if not pathlib.Path(f"/volumes/{old_volume}/{folder.name}").is_dir(): - Jobs.update( - job=job, - status=JobStatus.ERROR, - error=f"{service_name} is not a directory.", - ) - return - if ( - not pathlib.Path(f"/volumes/{old_volume}/{folder.name}").owner() - == folder.owner - ): - Jobs.update( - job=job, - status=JobStatus.ERROR, - error=f"{service_name} owner is not {folder.owner}.", - ) - return + path = pathlib.Path(f"/volumes/{old_volume}/{folder.name}") - # Stop service - Jobs.update( - job=job, - status=JobStatus.RUNNING, - status_text=f"Stopping {service_name}...", - progress=5, - ) - service.stop() - # Wait for the service to stop, check every second - # If it does not stop in 30 seconds, abort - for _ in range(30): - if service.get_status() not in ( - ServiceStatus.ACTIVATING, - ServiceStatus.DEACTIVATING, - ): - break - time.sleep(1) - else: - Jobs.update( - job=job, - status=JobStatus.ERROR, - error=f"{service_name} did not stop in 30 seconds.", - ) - return + if not path.exists(): + raise MoveError(f"{path} is not found.") + if not path.is_dir(): + raise MoveError(f"{path} is not a directory.") + if path.owner() != folder.owner: + raise MoveError(f"{path} owner is not {folder.owner}.") - # Unmount old volume - Jobs.update( - job=job, - status_text="Unmounting old folder...", - status=JobStatus.RUNNING, - progress=10, - ) + +def unmount_old_volume(folder_names: List[FolderMoveNames]) -> None: for folder in folder_names: try: subprocess.run( @@ -156,39 +146,31 @@ def move_service( check=True, ) except subprocess.CalledProcessError: - Jobs.update( - job=job, - status=JobStatus.ERROR, - error="Unable to unmount old volume.", - ) - return + raise MoveError("Unable to unmount old volume.") + + +def move_folders_to_volume( + folder_names: List[FolderMoveNames], + old_volume: BlockDevice, + new_volume: BlockDevice, + job: Job, +) -> None: # Move data to new volume and set correct permissions - Jobs.update( - job=job, - status_text="Moving data to new volume...", - status=JobStatus.RUNNING, - progress=20, - ) - current_progress = 20 + current_progress = job.progress folder_percentage = 50 // len(folder_names) for folder in folder_names: shutil.move( f"/volumes/{old_volume}/{folder.name}", - f"/volumes/{volume.name}/{folder.name}", - ) - Jobs.update( - job=job, - status_text="Moving data to new volume...", - status=JobStatus.RUNNING, - progress=current_progress + folder_percentage, + f"/volumes/{new_volume.name}/{folder.name}", ) + progress = current_progress + folder_percentage + report_progress(progress, job, "Moving data to new volume...") - Jobs.update( - job=job, - status_text=f"Making sure {service_name} owns its files...", - status=JobStatus.RUNNING, - progress=70, - ) + +def chown_folders( + folder_names: List[FolderMoveNames], volume: BlockDevice, job: Job, service: Service +) -> None: + service_name = service.get_display_name() for folder in folder_names: try: subprocess.run( @@ -208,14 +190,8 @@ def move_service( error=f"Unable to set ownership of new volume. {service_name} may not be able to access its files. Continuing anyway.", ) - # Mount new volume - Jobs.update( - job=job, - status_text=f"Mounting {service_name} data...", - status=JobStatus.RUNNING, - progress=90, - ) +def mount_folders(folder_names: List[FolderMoveNames], volume: BlockDevice) -> None: for folder in folder_names: try: subprocess.run( @@ -229,32 +205,23 @@ def move_service( ) except subprocess.CalledProcessError as error: print(error.output) - Jobs.update( - job=job, - status=JobStatus.ERROR, - error="Unable to mount new volume.", - ) - return + raise MoveError(f"Unable to mount new volume:{error.output}") - # Update userdata - Jobs.update( - job=job, - status_text="Finishing move...", - status=JobStatus.RUNNING, - progress=95, - ) + +def update_volume_in_userdata(service: Service, volume: BlockDevice): with WriteUserData() as user_data: + service_id = service.get_id() if "modules" not in user_data: user_data["modules"] = {} - if userdata_location not in user_data["modules"]: - user_data["modules"][userdata_location] = {} - user_data["modules"][userdata_location]["location"] = volume.name - # Start service - service.start() + if service_id not in user_data["modules"]: + user_data["modules"][service_id] = {} + user_data["modules"][service_id]["location"] = volume.name + + +def report_progress(progress: int, job: Job, status_text: str) -> None: Jobs.update( job=job, - status=JobStatus.FINISHED, - result=f"{service_name} moved successfully.", - status_text=f"Starting {service_name}...", - progress=100, + status=JobStatus.RUNNING, + status_text=status_text, + progress=progress, ) From e42da357fb861563f4498cacd44b9b346fe74851 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 29 Jan 2024 16:53:32 +0000 Subject: [PATCH 06/20] fix(services): report moving errors fully --- selfprivacy_api/graphql/mutations/services_mutations.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/selfprivacy_api/graphql/mutations/services_mutations.py b/selfprivacy_api/graphql/mutations/services_mutations.py index 9bacf66..e2aea34 100644 --- a/selfprivacy_api/graphql/mutations/services_mutations.py +++ b/selfprivacy_api/graphql/mutations/services_mutations.py @@ -157,7 +157,7 @@ class ServicesMutations: if service is None: return ServiceJobMutationReturn( success=False, - message="Service not found.", + message=f"Service not found: {input.service_id}", code=404, ) # TODO: make serviceImmovable and BlockdeviceNotFound exceptions @@ -165,7 +165,7 @@ class ServicesMutations: if not service.is_movable(): return ServiceJobMutationReturn( success=False, - message="Service is not movable.", + message=f"Service is not movable: {service.get_display_name()}", code=400, service=service_to_graphql_service(service), ) @@ -173,7 +173,7 @@ class ServicesMutations: if volume is None: return ServiceJobMutationReturn( success=False, - message="Volume not found.", + message=f"Volume not found: {input.location}", code=404, service=service_to_graphql_service(service), ) @@ -197,7 +197,7 @@ class ServicesMutations: else: return ServiceJobMutationReturn( success=False, - message=f"Service move failure: {job.status_text}", + message=f"Service move failure: {job.status_text}: {job.error}", code=400, service=service_to_graphql_service(service), job=job_to_api_job(job), From 6cd1d27902dd604ea8e761693546eaadef3fc308 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 29 Jan 2024 16:54:31 +0000 Subject: [PATCH 07/20] test(services): merge def and current service tests --- tests/test_graphql/test_api_services.py | 626 ------------------------ tests/test_graphql/test_services.py | 81 +++ 2 files changed, 81 insertions(+), 626 deletions(-) delete mode 100644 tests/test_graphql/test_api_services.py diff --git a/tests/test_graphql/test_api_services.py b/tests/test_graphql/test_api_services.py deleted file mode 100644 index ac6d217..0000000 --- a/tests/test_graphql/test_api_services.py +++ /dev/null @@ -1,626 +0,0 @@ -# pylint: disable=redefined-outer-name -# pylint: disable=unused-argument -import pytest - -from tests.common import read_json - - -class NextcloudMockReturnTrue: - def __init__(self, args, **kwargs): - self.args = args - self.kwargs = kwargs - - def enable(): - pass - - def disable(): - pass - - def stop(): - pass - - def is_movable(): - return True - - def move_to_volume(what): - return None - - def start(): - pass - - def restart(): - pass - - returncode = 0 - - -class BlockDevices: - def get_block_device(location): - return True - - -class ProcessMock: - """Mock subprocess.Popen""" - - def __init__(self, args, **kwargs): - self.args = args - self.kwargs = kwargs - - def communicate(): # pylint: disable=no-method-argument - return (b"", None) - - returncode = 0 - - -@pytest.fixture -def mock_subprocess_popen(mocker): - mock = mocker.patch("subprocess.Popen", autospec=True, return_value=ProcessMock) - return mock - - -@pytest.fixture -def one_user(mocker, datadir): - mocker.patch("selfprivacy_api.utils.USERDATA_FILE", new=datadir / "one_user.json") - assert read_json(datadir / "one_user.json")["users"] == [ - { - "username": "user1", - "hashedPassword": "HASHED_PASSWORD_1", - "sshKeys": ["ssh-rsa KEY user1@pc"], - } - ] - return datadir - - -@pytest.fixture -def mock_service_to_graphql_service(mocker): - mock = mocker.patch( - "selfprivacy_api.graphql.mutations.services_mutations.service_to_graphql_service", - autospec=True, - return_value=None, - ) - return mock - - -@pytest.fixture -def mock_job_to_api_job(mocker): - mock = mocker.patch( - "selfprivacy_api.graphql.mutations.services_mutations.job_to_api_job", - autospec=True, - return_value=None, - ) - return mock - - -@pytest.fixture -def mock_block_devices_return_none(mocker): - mock = mocker.patch( - "selfprivacy_api.utils.block_devices.BlockDevices", - autospec=True, - return_value=None, - ) - return mock - - -@pytest.fixture -def mock_block_devices(mocker): - mock = mocker.patch( - "selfprivacy_api.graphql.mutations.services_mutations.BlockDevices", - autospec=True, - return_value=BlockDevices, - ) - return mock - - -@pytest.fixture -def mock_get_service_by_id_return_none(mocker): - mock = mocker.patch( - "selfprivacy_api.graphql.mutations.services_mutations.get_service_by_id", - autospec=True, - return_value=None, - ) - return mock - - -@pytest.fixture -def mock_get_service_by_id(mocker): - mock = mocker.patch( - "selfprivacy_api.graphql.mutations.services_mutations.get_service_by_id", - autospec=True, - return_value=NextcloudMockReturnTrue, - ) - return mock - - -#################################################################### - - -API_ENABLE_SERVICE_MUTATION = """ -mutation enableService($serviceId: String!) { - enableService(serviceId: $serviceId) { - success - message - code - } -} -""" - - -def test_graphql_enable_service_unauthorized_client( - client, mock_get_service_by_id_return_none, mock_subprocess_popen -): - response = client.post( - "/graphql", - json={ - "query": API_ENABLE_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is None - - -def test_graphql_enable_not_found_service( - authorized_client, - mock_get_service_by_id_return_none, - mock_subprocess_popen, - one_user, - mock_service_to_graphql_service, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_ENABLE_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["enableService"]["code"] == 404 - assert response.json()["data"]["enableService"]["message"] is not None - assert response.json()["data"]["enableService"]["success"] is False - - -def test_graphql_enable_service( - authorized_client, - mock_get_service_by_id, - mock_subprocess_popen, - one_user, - mock_service_to_graphql_service, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_ENABLE_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["enableService"]["code"] == 200 - assert response.json()["data"]["enableService"]["message"] is not None - assert response.json()["data"]["enableService"]["success"] is True - - -API_DISABLE_SERVICE_MUTATION = """ -mutation disableService($serviceId: String!) { - disableService(serviceId: $serviceId) { - success - message - code - } -} -""" - - -def test_graphql_disable_service_unauthorized_client( - client, - mock_get_service_by_id_return_none, - mock_subprocess_popen, - one_user, - mock_service_to_graphql_service, -): - response = client.post( - "/graphql", - json={ - "query": API_DISABLE_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is None - - -def test_graphql_disable_not_found_service( - authorized_client, - mock_get_service_by_id_return_none, - mock_subprocess_popen, - one_user, - mock_service_to_graphql_service, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_DISABLE_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["disableService"]["code"] == 404 - assert response.json()["data"]["disableService"]["message"] is not None - assert response.json()["data"]["disableService"]["success"] is False - - -def test_graphql_disable_services( - authorized_client, - mock_get_service_by_id, - mock_subprocess_popen, - one_user, - mock_service_to_graphql_service, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_DISABLE_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["disableService"]["code"] == 200 - assert response.json()["data"]["disableService"]["message"] is not None - assert response.json()["data"]["disableService"]["success"] is True - - -API_STOP_SERVICE_MUTATION = """ -mutation stopService($serviceId: String!) { - stopService(serviceId: $serviceId) { - success - message - code - } -} -""" - - -def test_graphql_stop_service_unauthorized_client( - client, - mock_get_service_by_id_return_none, - mock_service_to_graphql_service, - mock_subprocess_popen, - one_user, -): - response = client.post( - "/graphql", - json={ - "query": API_STOP_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is None - - -def test_graphql_stop_not_found_service( - authorized_client, - mock_get_service_by_id_return_none, - mock_service_to_graphql_service, - mock_subprocess_popen, - one_user, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_STOP_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["stopService"]["code"] == 404 - assert response.json()["data"]["stopService"]["message"] is not None - assert response.json()["data"]["stopService"]["success"] is False - - -def test_graphql_stop_service( - authorized_client, - mock_get_service_by_id, - mock_service_to_graphql_service, - mock_subprocess_popen, - one_user, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_STOP_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["stopService"]["code"] == 200 - assert response.json()["data"]["stopService"]["message"] is not None - assert response.json()["data"]["stopService"]["success"] is True - - -API_START_SERVICE_MUTATION = """ -mutation startService($serviceId: String!) { - startService(serviceId: $serviceId) { - success - message - code - } -} -""" - - -def test_graphql_start_service_unauthorized_client( - client, - mock_get_service_by_id_return_none, - mock_service_to_graphql_service, - mock_subprocess_popen, - one_user, -): - response = client.post( - "/graphql", - json={ - "query": API_START_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is None - - -def test_graphql_start_not_found_service( - authorized_client, - mock_get_service_by_id_return_none, - mock_service_to_graphql_service, - mock_subprocess_popen, - one_user, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_START_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["startService"]["code"] == 404 - assert response.json()["data"]["startService"]["message"] is not None - assert response.json()["data"]["startService"]["success"] is False - - -def test_graphql_start_service( - authorized_client, - mock_get_service_by_id, - mock_service_to_graphql_service, - mock_subprocess_popen, - one_user, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_START_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["startService"]["code"] == 200 - assert response.json()["data"]["startService"]["message"] is not None - assert response.json()["data"]["startService"]["success"] is True - - -API_RESTART_SERVICE_MUTATION = """ -mutation restartService($serviceId: String!) { - restartService(serviceId: $serviceId) { - success - message - code - } -} -""" - - -def test_graphql_restart_service_unauthorized_client( - client, - mock_get_service_by_id_return_none, - mock_service_to_graphql_service, - mock_subprocess_popen, - one_user, -): - response = client.post( - "/graphql", - json={ - "query": API_RESTART_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is None - - -def test_graphql_restart_not_found_service( - authorized_client, - mock_get_service_by_id_return_none, - mock_service_to_graphql_service, - mock_subprocess_popen, - one_user, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_RESTART_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["restartService"]["code"] == 404 - assert response.json()["data"]["restartService"]["message"] is not None - assert response.json()["data"]["restartService"]["success"] is False - - -def test_graphql_restart_service( - authorized_client, - mock_get_service_by_id, - mock_service_to_graphql_service, - mock_subprocess_popen, - one_user, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_RESTART_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["restartService"]["code"] == 200 - assert response.json()["data"]["restartService"]["message"] is not None - assert response.json()["data"]["restartService"]["success"] is True - - -API_MOVE_SERVICE_MUTATION = """ -mutation moveService($input: MoveServiceInput!) { - moveService(input: $input) { - success - message - code - } -} -""" - - -def test_graphql_move_service_unauthorized_client( - client, - mock_get_service_by_id_return_none, - mock_service_to_graphql_service, - mock_subprocess_popen, - one_user, -): - response = client.post( - "/graphql", - json={ - "query": API_MOVE_SERVICE_MUTATION, - "variables": { - "input": {"serviceId": "nextcloud", "location": "sdx"}, - }, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is None - - -def test_graphql_move_not_found_service( - authorized_client, - mock_get_service_by_id_return_none, - mock_service_to_graphql_service, - mock_subprocess_popen, - one_user, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_MOVE_SERVICE_MUTATION, - "variables": { - "input": {"serviceId": "nextcloud", "location": "sdx"}, - }, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["moveService"]["code"] == 404 - assert response.json()["data"]["moveService"]["message"] is not None - assert response.json()["data"]["moveService"]["success"] is False - - -def test_graphql_move_not_movable_service( - authorized_client, - mock_get_service_by_id_return_none, - mock_service_to_graphql_service, - mock_subprocess_popen, - one_user, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_MOVE_SERVICE_MUTATION, - "variables": { - "input": {"serviceId": "nextcloud", "location": "sdx"}, - }, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["moveService"]["code"] == 404 - assert response.json()["data"]["moveService"]["message"] is not None - assert response.json()["data"]["moveService"]["success"] is False - - -def test_graphql_move_service_volume_not_found( - authorized_client, - mock_get_service_by_id_return_none, - mock_service_to_graphql_service, - mock_block_devices_return_none, - mock_subprocess_popen, - one_user, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_MOVE_SERVICE_MUTATION, - "variables": { - "input": {"serviceId": "nextcloud", "location": "sdx"}, - }, - }, - ) - - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["moveService"]["code"] == 404 - assert response.json()["data"]["moveService"]["message"] is not None - assert response.json()["data"]["moveService"]["success"] is False - - -def test_graphql_move_service( - authorized_client, - mock_get_service_by_id, - mock_service_to_graphql_service, - mock_block_devices, - mock_subprocess_popen, - one_user, - mock_job_to_api_job, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_MOVE_SERVICE_MUTATION, - "variables": { - "input": {"serviceId": "nextcloud", "location": "sdx"}, - }, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["moveService"]["code"] == 200 - assert response.json()["data"]["moveService"]["message"] is not None - assert response.json()["data"]["moveService"]["success"] is True diff --git a/tests/test_graphql/test_services.py b/tests/test_graphql/test_services.py index 3983b56..c6784ee 100644 --- a/tests/test_graphql/test_services.py +++ b/tests/test_graphql/test_services.py @@ -10,6 +10,9 @@ from selfprivacy_api.services.test_service import DummyService 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 @pytest.fixture() @@ -23,6 +26,33 @@ 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_service_mover_folders(mocker): + mock = mocker.patch( + "selfprivacy_api.services.generic_service_mover.check_folders", + autospec=True, + return_value=None, + ) + 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 { @@ -474,6 +504,15 @@ def test_move_immovable(authorized_client, only_dummy_service): assert data["job"] is None +def test_move_no_such_service(authorized_client, only_dummy_service): + mutation_response = api_move_by_name(authorized_client, "bogus_service", "sda1") + data = get_data(mutation_response)["services"]["moveService"] + assert_errorcode(data, 404) + + assert data["service"] is None + assert data["job"] is None + + def test_move_no_such_volume(authorized_client, only_dummy_service): dummy_service = only_dummy_service mutation_response = api_move(authorized_client, dummy_service, "bogus_volume") @@ -502,6 +541,48 @@ def test_move_same_volume(authorized_client, dummy_service): assert data["job"] is not None +def test_graphql_move_service_without_folders_on_old_volume( + authorized_client, + generic_userdata, + lsblk_singular_mock, + dummy_service: DummyService, +): + target = "sda1" + BlockDevices().update() + assert BlockDevices().get_block_device(target) is not None + + 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"] + assert_errorcode(data, 400) + + +def test_graphql_move_service( + authorized_client, + generic_userdata, + mock_check_service_mover_folders, + lsblk_singular_mock, + dummy_service: DummyService, + mock_subprocess_run, + mock_shutil_move, +): + # Does not check real moving, + # but tests the finished job propagation through API + + target = "sda1" + BlockDevices().update() + assert BlockDevices().get_block_device(target) is not None + + 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"] + assert_ok(data) + + def test_mailservice_cannot_enable_disable(authorized_client): mailservice = get_service_by_id("simple-nixos-mailserver") From b3c7e2fa9e63ddf8bde143f9a435b116165f93fd Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 29 Jan 2024 17:06:29 +0000 Subject: [PATCH 08/20] test(services): remove unused json --- .../test_api_services/one_user.json | 61 ------------------- 1 file changed, 61 deletions(-) delete mode 100644 tests/test_graphql/test_api_services/one_user.json diff --git a/tests/test_graphql/test_api_services/one_user.json b/tests/test_graphql/test_api_services/one_user.json deleted file mode 100644 index 5df2108..0000000 --- a/tests/test_graphql/test_api_services/one_user.json +++ /dev/null @@ -1,61 +0,0 @@ -{ - "backblaze": { - "accountId": "ID", - "accountKey": "KEY", - "bucket": "selfprivacy" - }, - "api": { - "token": "TEST_TOKEN", - "enableSwagger": false - }, - "bitwarden": { - "enable": false - }, - "cloudflare": { - "apiKey": "TOKEN" - }, - "databasePassword": "PASSWORD", - "domain": "test.tld", - "hashedMasterPassword": "HASHED_PASSWORD", - "hostname": "test-instance", - "nextcloud": { - "adminPassword": "ADMIN", - "databasePassword": "ADMIN", - "enable": true - }, - "resticPassword": "PASS", - "ssh": { - "enable": true, - "passwordAuthentication": true, - "rootKeys": [ - "ssh-ed25519 KEY test@pc" - ] - }, - "username": "tester", - "gitea": { - "enable": false - }, - "ocserv": { - "enable": true - }, - "pleroma": { - "enable": true - }, - "autoUpgrade": { - "enable": true, - "allowReboot": true - }, - "timezone": "Europe/Moscow", - "sshKeys": [ - "ssh-rsa KEY test@pc" - ], - "users": [ - { - "username": "user1", - "hashedPassword": "HASHED_PASSWORD_1", - "sshKeys": [ - "ssh-rsa KEY user1@pc" - ] - } - ] -} \ No newline at end of file From b22dfc04691c0d2863099d8619def94485e0821b Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Wed, 31 Jan 2024 10:51:01 +0000 Subject: [PATCH 09/20] fix(services): a better error message --- selfprivacy_api/graphql/mutations/services_mutations.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/selfprivacy_api/graphql/mutations/services_mutations.py b/selfprivacy_api/graphql/mutations/services_mutations.py index e2aea34..e8edbcf 100644 --- a/selfprivacy_api/graphql/mutations/services_mutations.py +++ b/selfprivacy_api/graphql/mutations/services_mutations.py @@ -197,7 +197,7 @@ class ServicesMutations: else: return ServiceJobMutationReturn( success=False, - message=f"Service move failure: {job.status_text}: {job.error}", + message=f"While moving service and performing the step '{job.status_text}', error occured: {job.error}", code=400, service=service_to_graphql_service(service), job=job_to_api_job(job), From c947922a5d1ff5d6ceb7142633a42636dda9a69f Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 12 Feb 2024 14:41:15 +0000 Subject: [PATCH 10/20] refactor(services): make moving a part of generic service functionality --- selfprivacy_api/jobs/__init__.py | 12 + .../services/generic_service_mover.py | 227 ------------------ selfprivacy_api/services/moving.py | 114 +++++++++ selfprivacy_api/services/service.py | 97 +++++++- selfprivacy_api/services/tasks.py | 11 + 5 files changed, 233 insertions(+), 228 deletions(-) delete mode 100644 selfprivacy_api/services/generic_service_mover.py create mode 100644 selfprivacy_api/services/moving.py create mode 100644 selfprivacy_api/services/tasks.py diff --git a/selfprivacy_api/jobs/__init__.py b/selfprivacy_api/jobs/__init__.py index 7310016..7f46e9d 100644 --- a/selfprivacy_api/jobs/__init__.py +++ b/selfprivacy_api/jobs/__init__.py @@ -268,6 +268,18 @@ class Jobs: return False +# A terse way to call a common operation, for readability +# job.report_progress() would be even better +# but it would go against how this file is written +def report_progress(progress: int, job: Job, status_text: str) -> None: + Jobs.update( + job=job, + status=JobStatus.RUNNING, + status_text=status_text, + progress=progress, + ) + + def _redis_key_from_uuid(uuid_string) -> str: return "jobs:" + str(uuid_string) diff --git a/selfprivacy_api/services/generic_service_mover.py b/selfprivacy_api/services/generic_service_mover.py deleted file mode 100644 index 20c717b..0000000 --- a/selfprivacy_api/services/generic_service_mover.py +++ /dev/null @@ -1,227 +0,0 @@ -"""Generic handler for moving services""" - -from __future__ import annotations -import subprocess -import pathlib -import shutil -from typing import List - -from pydantic import BaseModel -from selfprivacy_api.jobs import Job, JobStatus, Jobs -from selfprivacy_api.utils.huey import huey -from selfprivacy_api.utils.block_devices import BlockDevice -from selfprivacy_api.utils import ReadUserData, WriteUserData -from selfprivacy_api.services.service import Service -from selfprivacy_api.services.owned_path import OwnedPath - -from selfprivacy_api.services.service import StoppedService - - -class MoveError(Exception): - """Move failed""" - - -class FolderMoveNames(BaseModel): - name: str - bind_location: str - owner: str - group: str - - @staticmethod - def from_owned_path(path: OwnedPath) -> FolderMoveNames: - return FolderMoveNames( - name=FolderMoveNames.get_foldername(path.path), - bind_location=path.path, - owner=path.owner, - group=path.group, - ) - - @staticmethod - def get_foldername(path: str) -> str: - return path.split("/")[-1] - - @staticmethod - def default_foldermoves(service: Service) -> list[FolderMoveNames]: - return [ - FolderMoveNames.from_owned_path(folder) - for folder in service.get_owned_folders() - ] - - -@huey.task() -def move_service( - service: Service, - new_volume: BlockDevice, - job: Job, - folder_names: List[FolderMoveNames], - userdata_location: str = None, # deprecated, not used -): - """ - Move a service to another volume. - Is not allowed to raise errors because it is a task. - """ - service_name = service.get_display_name() - old_volume = service.get_drive() - report_progress(0, job, "Performing pre-move checks...") - - try: - with ReadUserData() as user_data: - if not user_data.get("useBinds", False): - raise MoveError("Server is not using binds.") - - check_volume(new_volume, service) - check_folders(old_volume, folder_names) - - report_progress(5, job, f"Stopping {service_name}...") - - with StoppedService(service): - report_progress(10, job, "Unmounting folders from old volume...") - unmount_old_volume(folder_names) - - report_progress(20, job, "Moving data to new volume...") - move_folders_to_volume(folder_names, old_volume, new_volume, job) - - report_progress(70, job, f"Making sure {service_name} owns its files...") - chown_folders(folder_names, new_volume, job, service) - - report_progress(90, job, f"Mounting {service_name} data...") - mount_folders(folder_names, new_volume) - - report_progress(95, job, f"Finishing moving {service_name}...") - update_volume_in_userdata(service, new_volume) - - Jobs.update( - job=job, - status=JobStatus.FINISHED, - result=f"{service_name} moved successfully.", - status_text=f"Starting {service_name}...", - progress=100, - ) - except Exception as e: - Jobs.update( - job=job, - status=JobStatus.ERROR, - error=type(e).__name__ + " " + str(e), - ) - - -def check_volume(new_volume: BlockDevice, service: Service) -> bool: - service_name = service.get_display_name() - old_volume_name: str = service.get_drive() - - # Check if we are on the same volume - if old_volume_name == new_volume.name: - raise MoveError(f"{service_name} is already on volume {new_volume}") - - # Check if there is enough space on the new volume - if int(new_volume.fsavail) < service.get_storage_usage(): - raise MoveError("Not enough space on the new volume.") - - # Make sure the volume is mounted - if ( - not new_volume.is_root() - and f"/volumes/{new_volume.name}" not in new_volume.mountpoints - ): - raise MoveError("Volume is not mounted.") - - -def check_folders(old_volume: BlockDevice, folder_names: List[FolderMoveNames]) -> None: - # Make sure current actual directory exists and if its user and group are correct - for folder in folder_names: - path = pathlib.Path(f"/volumes/{old_volume}/{folder.name}") - - if not path.exists(): - raise MoveError(f"{path} is not found.") - if not path.is_dir(): - raise MoveError(f"{path} is not a directory.") - if path.owner() != folder.owner: - raise MoveError(f"{path} owner is not {folder.owner}.") - - -def unmount_old_volume(folder_names: List[FolderMoveNames]) -> None: - for folder in folder_names: - try: - subprocess.run( - ["umount", folder.bind_location], - check=True, - ) - except subprocess.CalledProcessError: - raise MoveError("Unable to unmount old volume.") - - -def move_folders_to_volume( - folder_names: List[FolderMoveNames], - old_volume: BlockDevice, - new_volume: BlockDevice, - job: Job, -) -> None: - # Move data to new volume and set correct permissions - current_progress = job.progress - folder_percentage = 50 // len(folder_names) - for folder in folder_names: - shutil.move( - f"/volumes/{old_volume}/{folder.name}", - f"/volumes/{new_volume.name}/{folder.name}", - ) - progress = current_progress + folder_percentage - report_progress(progress, job, "Moving data to new volume...") - - -def chown_folders( - folder_names: List[FolderMoveNames], volume: BlockDevice, job: Job, service: Service -) -> None: - service_name = service.get_display_name() - for folder in folder_names: - try: - subprocess.run( - [ - "chown", - "-R", - f"{folder.owner}:{folder.group}", - f"/volumes/{volume.name}/{folder.name}", - ], - check=True, - ) - except subprocess.CalledProcessError as error: - print(error.output) - Jobs.update( - job=job, - status=JobStatus.RUNNING, - error=f"Unable to set ownership of new volume. {service_name} may not be able to access its files. Continuing anyway.", - ) - - -def mount_folders(folder_names: List[FolderMoveNames], volume: BlockDevice) -> None: - for folder in folder_names: - try: - subprocess.run( - [ - "mount", - "--bind", - f"/volumes/{volume.name}/{folder.name}", - folder.bind_location, - ], - check=True, - ) - except subprocess.CalledProcessError as error: - print(error.output) - raise MoveError(f"Unable to mount new volume:{error.output}") - - -def update_volume_in_userdata(service: Service, volume: BlockDevice): - with WriteUserData() as user_data: - service_id = service.get_id() - if "modules" not in user_data: - user_data["modules"] = {} - if service_id not in user_data["modules"]: - user_data["modules"][service_id] = {} - user_data["modules"][service_id]["location"] = volume.name - - -def report_progress(progress: int, job: Job, status_text: str) -> None: - Jobs.update( - job=job, - status=JobStatus.RUNNING, - status_text=status_text, - progress=progress, - ) diff --git a/selfprivacy_api/services/moving.py b/selfprivacy_api/services/moving.py new file mode 100644 index 0000000..d667935 --- /dev/null +++ b/selfprivacy_api/services/moving.py @@ -0,0 +1,114 @@ +"""Generic handler for moving services""" + +from __future__ import annotations +import subprocess +import pathlib +import shutil +from typing import List + +from selfprivacy_api.jobs import Job, report_progress +from selfprivacy_api.utils.block_devices import BlockDevice +from selfprivacy_api.services.owned_path import OwnedPath + + +class MoveError(Exception): + """Move failed""" + +def get_foldername(path: str) -> str: + return path.split("/")[-1] + + + + +def check_volume(volume: BlockDevice, space_needed: int) -> bool: + # Check if there is enough space on the new volume + if int(volume.fsavail) < space_needed: + raise MoveError("Not enough space on the new volume.") + + # Make sure the volume is mounted + if ( + not volume.is_root() + and f"/volumes/{volume.name}" not in volume.mountpoints + ): + raise MoveError("Volume is not mounted.") + + +def check_folders(current_volume: BlockDevice, folders: List[OwnedPath]) -> None: + # Make sure current actual directory exists and if its user and group are correct + for folder in folders: + path = pathlib.Path(f"/volumes/{current_volume}/{get_foldername(folder)}") + + if not path.exists(): + raise MoveError(f"{path} is not found.") + if not path.is_dir(): + raise MoveError(f"{path} is not a directory.") + if path.owner() != folder.owner: + raise MoveError(f"{path} owner is not {folder.owner}.") + + +def unbind_folders(owned_folders: List[OwnedPath]) -> None: + for folder in owned_folders: + try: + subprocess.run( + ["umount", folder.path], + check=True, + ) + except subprocess.CalledProcessError: + raise MoveError(f"Unable to unmount folder {folder.path}.") + + +def move_folders_to_volume( + folders: List[OwnedPath], + old_volume: BlockDevice, + new_volume: BlockDevice, + job: Job, +) -> None: + current_progress = job.progress + folder_percentage = 50 // len(folders) + for folder in folders: + folder_name = get_foldername(folder.path) + shutil.move( + f"/volumes/{old_volume}/{folder_name}", + f"/volumes/{new_volume.name}/{folder_name}", + ) + progress = current_progress + folder_percentage + report_progress(progress, job, "Moving data to new volume...") + + +def ensure_folder_ownership( + folders: List[OwnedPath], volume: BlockDevice +) -> None: + for folder in folders: + true_location = f"/volumes/{volume.name}/{get_foldername(folder.path)}" + try: + subprocess.run( + [ + "chown", + "-R", + f"{folder.owner}:{folder.group}", + # Could we just chown the binded location instead? + true_location + ], + check=True, + ) + except subprocess.CalledProcessError as error: + error_message = f"Unable to set ownership of {true_location} :{error.output}" + print(error.output) + raise MoveError(error_message) + + +def bind_folders(folders: List[OwnedPath], volume: BlockDevice) -> None: + for folder in folders: + try: + subprocess.run( + [ + "mount", + "--bind", + f"/volumes/{volume.name}/{get_foldername(folder.path)}", + folder.path, + ], + check=True, + ) + except subprocess.CalledProcessError as error: + print(error.output) + raise MoveError(f"Unable to mount new volume:{error.output}") diff --git a/selfprivacy_api/services/service.py b/selfprivacy_api/services/service.py index 0cca38a..6255f20 100644 --- a/selfprivacy_api/services/service.py +++ b/selfprivacy_api/services/service.py @@ -4,12 +4,14 @@ from enum import Enum from typing import List, Optional from pydantic import BaseModel -from selfprivacy_api.jobs import Job +from selfprivacy_api.jobs import Job, Jobs, JobStatus, report_progress from selfprivacy_api.utils.block_devices import BlockDevice, BlockDevices from selfprivacy_api.services.generic_size_counter import get_storage_usage from selfprivacy_api.services.owned_path import OwnedPath +from selfprivacy_api.services.moving import check_folders, check_volume, unbind_folders, bind_folders, ensure_folder_ownership, MoveError, move_folders_to_volume + from selfprivacy_api import utils from selfprivacy_api.utils.waitloop import wait_until_true from selfprivacy_api.utils import ReadUserData, WriteUserData @@ -294,6 +296,99 @@ class Service(ABC): def get_foldername(path: str) -> str: return path.split("/")[-1] + # TODO: with better json utils, it can be one line, and not a separate function + @classmethod + def set_location(cls, volume: BlockDevice): + """ + Only changes userdata + """ + + with WriteUserData() as user_data: + service_id = cls.get_id() + if "modules" not in user_data: + user_data["modules"] = {} + if service_id not in user_data["modules"]: + user_data["modules"][service_id] = {} + user_data["modules"][service_id]["location"] = volume.name + + def assert_can_move(self, new_volume): + """ + Checks if the service can be moved to new volume + Raises errors if it cannot + """ + with ReadUserData() as user_data: + if not user_data.get("useBinds", False): + raise MoveError("Server is not using binds.") + + current_volume_name = self.get_drive() + service_name = self.get_display_name() + if current_volume_name == new_volume.name: + raise MoveError(f"{service_name} is already on volume {new_volume}") + + check_volume(new_volume, space_needed=self.get_storage_usage()) + + owned_folders = self.get_owned_folders() + if owned_folders == []: + raise MoveError("nothing to move") + + check_folders(current_volume_name, owned_folders) + + + def do_move_to_volume( + self, + new_volume: BlockDevice, + job: Job, + ): + """ + Move a service to another volume. + Is not allowed to raise errors because it is a task. + """ + service_name = self.get_display_name() + old_volume_name = self.get_drive() + owned_folders = self.get_owned_folders() + + report_progress(0, job, "Performing pre-move checks...") + + # TODO: move trying to the task + try: + report_progress(5, job, f"Stopping {service_name}...") + + with StoppedService(self): + report_progress(10, job, "Unmounting folders from old volume...") + unbind_folders(owned_folders) + + report_progress(20, job, "Moving data to new volume...") + move_folders_to_volume(owned_folders, old_volume_name, new_volume, job) + + report_progress(70, job, f"Making sure {service_name} owns its files...") + try: + ensure_folder_ownership(owned_folders, new_volume, job, self) + except Exception as error: + # We have logged it via print and we additionally log it here in the error field + # We are continuing anyway but Job has no warning field + Jobs.update(job, JobStatus.RUNNING, error=f"Service {service_name} will not be able to write files: " + str(error)) + + report_progress(90, job, f"Mounting {service_name} data...") + bind_folders(owned_folders, new_volume) + + report_progress(95, job, f"Finishing moving {service_name}...") + self.set_location(self, new_volume) + + Jobs.update( + job=job, + status=JobStatus.FINISHED, + result=f"{service_name} moved successfully.", + status_text=f"Starting {service_name}...", + progress=100, + ) + except Exception as e: + Jobs.update( + job=job, + status=JobStatus.ERROR, + error=type(e).__name__ + " " + str(e), + ) + + @abstractmethod def move_to_volume(self, volume: BlockDevice) -> Job: """Cannot raise errors. diff --git a/selfprivacy_api/services/tasks.py b/selfprivacy_api/services/tasks.py new file mode 100644 index 0000000..2cc52ad --- /dev/null +++ b/selfprivacy_api/services/tasks.py @@ -0,0 +1,11 @@ +from selfprivacy_api.services import Service +from selfprivacy_api.utils.block_devices import BlockDevice +from selfprivacy_api.utils.huey import huey + + +@huey.task() +def move_service( + service: Service, + new_volume: BlockDevice, +): + service.move_to_volume(new_volume) From fb41c092f1b565be5deec0b7f6e6a87b2878550e Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Sun, 18 Feb 2024 23:58:00 +0000 Subject: [PATCH 11/20] feature(services): check before moving task and before move itself --- selfprivacy_api/actions/services.py | 36 ++++++ .../graphql/mutations/services_mutations.py | 51 +++++---- .../services/bitwarden/__init__.py | 23 +--- selfprivacy_api/services/gitea/__init__.py | 23 +--- .../services/mailserver/__init__.py | 20 ---- selfprivacy_api/services/moving.py | 30 +++-- .../services/nextcloud/__init__.py | 22 +--- selfprivacy_api/services/pleroma/__init__.py | 27 +---- selfprivacy_api/services/service.py | 104 ++++++++++-------- selfprivacy_api/services/tasks.py | 21 +++- .../services/test_service/__init__.py | 22 +--- tests/test_graphql/test_services.py | 17 ++- tests/test_services.py | 25 ++--- 13 files changed, 192 insertions(+), 229 deletions(-) create mode 100644 selfprivacy_api/actions/services.py diff --git a/selfprivacy_api/actions/services.py b/selfprivacy_api/actions/services.py new file mode 100644 index 0000000..56d35e9 --- /dev/null +++ b/selfprivacy_api/actions/services.py @@ -0,0 +1,36 @@ +from selfprivacy_api.utils.block_devices import BlockDevices +from selfprivacy_api.jobs import Jobs, Job + +from selfprivacy_api.services import get_service_by_id +from selfprivacy_api.services.tasks import move_service as move_service_task + + +class ServiceNotFoundError(Exception): + pass + + +class VolumeNotFoundError(Exception): + pass + + +def move_service(service_id: str, volume_name: str) -> Job: + service = get_service_by_id(service_id) + if service is None: + raise ServiceNotFoundError(f"No such service:{service_id}") + + volume = BlockDevices().get_block_device(volume_name) + if volume is None: + raise VolumeNotFoundError(f"No such volume:{volume_name}") + + service.assert_can_move(volume) + + job = Jobs.add( + type_id=f"services.{service.get_id()}.move", + name=f"Move {service.get_display_name()}", + description=f"Moving {service.get_display_name()} data to {volume.name}", + ) + + handle = move_service_task(service, volume, job) + # Nonblocking + handle() + return job diff --git a/selfprivacy_api/graphql/mutations/services_mutations.py b/selfprivacy_api/graphql/mutations/services_mutations.py index e8edbcf..911ad26 100644 --- a/selfprivacy_api/graphql/mutations/services_mutations.py +++ b/selfprivacy_api/graphql/mutations/services_mutations.py @@ -5,18 +5,25 @@ import strawberry from selfprivacy_api.graphql import IsAuthenticated from selfprivacy_api.graphql.common_types.jobs import job_to_api_job from selfprivacy_api.jobs import JobStatus +from selfprivacy_api.utils.block_devices import BlockDevices -from selfprivacy_api.graphql.common_types.service import ( - Service, - service_to_graphql_service, -) from selfprivacy_api.graphql.mutations.mutation_interface import ( GenericJobMutationReturn, GenericMutationReturn, ) +from selfprivacy_api.graphql.common_types.service import ( + Service, + service_to_graphql_service, +) + +from selfprivacy_api.actions.services import ( + move_service, + ServiceNotFoundError, + VolumeNotFoundError, +) +from selfprivacy_api.services.moving import MoveError from selfprivacy_api.services import get_service_by_id -from selfprivacy_api.utils.block_devices import BlockDevices @strawberry.type @@ -60,7 +67,7 @@ class ServicesMutations: except Exception as e: return ServiceMutationReturn( success=False, - message=format_error(e), + message=pretty_error(e), code=400, ) @@ -86,7 +93,7 @@ class ServicesMutations: except Exception as e: return ServiceMutationReturn( success=False, - message=format_error(e), + message=pretty_error(e), code=400, ) return ServiceMutationReturn( @@ -153,31 +160,31 @@ class ServicesMutations: @strawberry.mutation(permission_classes=[IsAuthenticated]) def move_service(self, input: MoveServiceInput) -> ServiceJobMutationReturn: """Move service.""" + # We need a service instance for a reply later service = get_service_by_id(input.service_id) if service is None: return ServiceJobMutationReturn( success=False, - message=f"Service not found: {input.service_id}", + message=f"Service does not exist: {input.service_id}", code=404, ) - # TODO: make serviceImmovable and BlockdeviceNotFound exceptions - # in the move_to_volume() function and handle them here - if not service.is_movable(): + + try: + job = move_service(input.service_id, input.location) + except (ServiceNotFoundError, VolumeNotFoundError) as e: return ServiceJobMutationReturn( success=False, - message=f"Service is not movable: {service.get_display_name()}", + message=pretty_error(e), + code=404, + ) + except Exception as e: + return ServiceJobMutationReturn( + success=False, + message=pretty_error(e), code=400, service=service_to_graphql_service(service), ) - volume = BlockDevices().get_block_device(input.location) - if volume is None: - return ServiceJobMutationReturn( - success=False, - message=f"Volume not found: {input.location}", - code=404, - service=service_to_graphql_service(service), - ) - job = service.move_to_volume(volume) + if job.status in [JobStatus.CREATED, JobStatus.RUNNING]: return ServiceJobMutationReturn( success=True, @@ -204,5 +211,5 @@ class ServicesMutations: ) -def format_error(e: Exception) -> str: +def pretty_error(e: Exception) -> str: return type(e).__name__ + ": " + str(e) diff --git a/selfprivacy_api/services/bitwarden/__init__.py b/selfprivacy_api/services/bitwarden/__init__.py index f04381d..0734115 100644 --- a/selfprivacy_api/services/bitwarden/__init__.py +++ b/selfprivacy_api/services/bitwarden/__init__.py @@ -3,12 +3,10 @@ import base64 import subprocess from typing import Optional, List -from selfprivacy_api.jobs import Job, Jobs -from selfprivacy_api.services.generic_service_mover import FolderMoveNames, move_service +from selfprivacy_api.utils import get_domain + from selfprivacy_api.services.generic_status_getter import get_service_status from selfprivacy_api.services.service import Service, ServiceStatus -from selfprivacy_api.utils import get_domain -from selfprivacy_api.utils.block_devices import BlockDevice from selfprivacy_api.services.bitwarden.icon import BITWARDEN_ICON @@ -101,20 +99,3 @@ class Bitwarden(Service): @staticmethod def get_folders() -> List[str]: return ["/var/lib/bitwarden", "/var/lib/bitwarden_rs"] - - def move_to_volume(self, volume: BlockDevice) -> Job: - job = Jobs.add( - type_id="services.bitwarden.move", - name="Move Bitwarden", - description=f"Moving Bitwarden data to {volume.name}", - ) - - move_service( - self, - volume, - job, - FolderMoveNames.default_foldermoves(self), - "bitwarden", - ) - - return job diff --git a/selfprivacy_api/services/gitea/__init__.py b/selfprivacy_api/services/gitea/__init__.py index bf3f5d2..26a0fd9 100644 --- a/selfprivacy_api/services/gitea/__init__.py +++ b/selfprivacy_api/services/gitea/__init__.py @@ -3,12 +3,10 @@ import base64 import subprocess from typing import Optional, List -from selfprivacy_api.jobs import Job, Jobs -from selfprivacy_api.services.generic_service_mover import FolderMoveNames, move_service +from selfprivacy_api.utils import get_domain + from selfprivacy_api.services.generic_status_getter import get_service_status from selfprivacy_api.services.service import Service, ServiceStatus -from selfprivacy_api.utils import get_domain -from selfprivacy_api.utils.block_devices import BlockDevice from selfprivacy_api.services.gitea.icon import GITEA_ICON @@ -96,20 +94,3 @@ class Gitea(Service): @staticmethod def get_folders() -> List[str]: return ["/var/lib/gitea"] - - def move_to_volume(self, volume: BlockDevice) -> Job: - job = Jobs.add( - type_id="services.gitea.move", - name="Move Gitea", - description=f"Moving Gitea data to {volume.name}", - ) - - move_service( - self, - volume, - job, - FolderMoveNames.default_foldermoves(self), - "gitea", - ) - - return job diff --git a/selfprivacy_api/services/mailserver/__init__.py b/selfprivacy_api/services/mailserver/__init__.py index b82a793..492cc55 100644 --- a/selfprivacy_api/services/mailserver/__init__.py +++ b/selfprivacy_api/services/mailserver/__init__.py @@ -4,14 +4,11 @@ import base64 import subprocess from typing import Optional, List -from selfprivacy_api.jobs import Job, Jobs -from selfprivacy_api.services.generic_service_mover import FolderMoveNames, move_service from selfprivacy_api.services.generic_status_getter import ( get_service_status_from_several_units, ) from selfprivacy_api.services.service import Service, ServiceDnsRecord, ServiceStatus from selfprivacy_api import utils -from selfprivacy_api.utils.block_devices import BlockDevice from selfprivacy_api.services.mailserver.icon import MAILSERVER_ICON @@ -166,20 +163,3 @@ class MailServer(Service): ), ) return dns_records - - def move_to_volume(self, volume: BlockDevice) -> Job: - job = Jobs.add( - type_id="services.email.move", - name="Move Mail Server", - description=f"Moving mailserver data to {volume.name}", - ) - - move_service( - self, - volume, - job, - FolderMoveNames.default_foldermoves(self), - "simple-nixos-mailserver", - ) - - return job diff --git a/selfprivacy_api/services/moving.py b/selfprivacy_api/services/moving.py index d667935..ecc505b 100644 --- a/selfprivacy_api/services/moving.py +++ b/selfprivacy_api/services/moving.py @@ -14,10 +14,9 @@ from selfprivacy_api.services.owned_path import OwnedPath class MoveError(Exception): """Move failed""" -def get_foldername(path: str) -> str: - return path.split("/")[-1] - +def get_foldername(p: OwnedPath) -> str: + return p.path.split("/")[-1] def check_volume(volume: BlockDevice, space_needed: int) -> bool: @@ -26,10 +25,7 @@ def check_volume(volume: BlockDevice, space_needed: int) -> bool: raise MoveError("Not enough space on the new volume.") # Make sure the volume is mounted - if ( - not volume.is_root() - and f"/volumes/{volume.name}" not in volume.mountpoints - ): + if not volume.is_root() and f"/volumes/{volume.name}" not in volume.mountpoints: raise MoveError("Volume is not mounted.") @@ -39,11 +35,11 @@ def check_folders(current_volume: BlockDevice, folders: List[OwnedPath]) -> None path = pathlib.Path(f"/volumes/{current_volume}/{get_foldername(folder)}") if not path.exists(): - raise MoveError(f"{path} is not found.") + raise MoveError(f"directory {path} is not found.") if not path.is_dir(): raise MoveError(f"{path} is not a directory.") if path.owner() != folder.owner: - raise MoveError(f"{path} owner is not {folder.owner}.") + raise MoveError(f"{path} is not owned by {folder.owner}.") def unbind_folders(owned_folders: List[OwnedPath]) -> None: @@ -66,7 +62,7 @@ def move_folders_to_volume( current_progress = job.progress folder_percentage = 50 // len(folders) for folder in folders: - folder_name = get_foldername(folder.path) + folder_name = get_foldername(folder) shutil.move( f"/volumes/{old_volume}/{folder_name}", f"/volumes/{new_volume.name}/{folder_name}", @@ -75,11 +71,9 @@ def move_folders_to_volume( report_progress(progress, job, "Moving data to new volume...") -def ensure_folder_ownership( - folders: List[OwnedPath], volume: BlockDevice -) -> None: +def ensure_folder_ownership(folders: List[OwnedPath], volume: BlockDevice) -> None: for folder in folders: - true_location = f"/volumes/{volume.name}/{get_foldername(folder.path)}" + true_location = f"/volumes/{volume.name}/{get_foldername(folder)}" try: subprocess.run( [ @@ -87,12 +81,14 @@ def ensure_folder_ownership( "-R", f"{folder.owner}:{folder.group}", # Could we just chown the binded location instead? - true_location + true_location, ], check=True, ) except subprocess.CalledProcessError as error: - error_message = f"Unable to set ownership of {true_location} :{error.output}" + error_message = ( + f"Unable to set ownership of {true_location} :{error.output}" + ) print(error.output) raise MoveError(error_message) @@ -104,7 +100,7 @@ def bind_folders(folders: List[OwnedPath], volume: BlockDevice) -> None: [ "mount", "--bind", - f"/volumes/{volume.name}/{get_foldername(folder.path)}", + f"/volumes/{volume.name}/{get_foldername(folder)}", folder.path, ], check=True, diff --git a/selfprivacy_api/services/nextcloud/__init__.py b/selfprivacy_api/services/nextcloud/__init__.py index 17e72d7..9a7aaec 100644 --- a/selfprivacy_api/services/nextcloud/__init__.py +++ b/selfprivacy_api/services/nextcloud/__init__.py @@ -2,12 +2,13 @@ import base64 import subprocess from typing import Optional, List + +from selfprivacy_api.utils import get_domain from selfprivacy_api.jobs import Job, Jobs -from selfprivacy_api.services.generic_service_mover import FolderMoveNames, move_service + from selfprivacy_api.services.generic_status_getter import get_service_status from selfprivacy_api.services.service import Service, ServiceStatus -from selfprivacy_api.utils import get_domain -from selfprivacy_api.utils.block_devices import BlockDevice + from selfprivacy_api.services.nextcloud.icon import NEXTCLOUD_ICON @@ -101,18 +102,3 @@ class Nextcloud(Service): @staticmethod def get_folders() -> List[str]: return ["/var/lib/nextcloud"] - - def move_to_volume(self, volume: BlockDevice) -> Job: - job = Jobs.add( - type_id="services.nextcloud.move", - name="Move Nextcloud", - description=f"Moving Nextcloud to volume {volume.name}", - ) - move_service( - self, - volume, - job, - FolderMoveNames.default_foldermoves(self), - "nextcloud", - ) - return job diff --git a/selfprivacy_api/services/pleroma/__init__.py b/selfprivacy_api/services/pleroma/__init__.py index cd21178..84eca59 100644 --- a/selfprivacy_api/services/pleroma/__init__.py +++ b/selfprivacy_api/services/pleroma/__init__.py @@ -2,13 +2,13 @@ import base64 import subprocess from typing import Optional, List -from selfprivacy_api.jobs import Job, Jobs -from selfprivacy_api.services.generic_service_mover import FolderMoveNames, move_service + +from selfprivacy_api.utils import get_domain + +from selfprivacy_api.services.owned_path import OwnedPath from selfprivacy_api.services.generic_status_getter import get_service_status from selfprivacy_api.services.service import Service, ServiceStatus -from selfprivacy_api.services.owned_path import OwnedPath -from selfprivacy_api.utils import get_domain -from selfprivacy_api.utils.block_devices import BlockDevice + from selfprivacy_api.services.pleroma.icon import PLEROMA_ICON @@ -88,7 +88,7 @@ class Pleroma(Service): def get_owned_folders() -> List[OwnedPath]: """ Get a list of occupied directories with ownership info - pleroma has folders that are owned by different users + Pleroma has folders that are owned by different users """ return [ OwnedPath( @@ -102,18 +102,3 @@ class Pleroma(Service): group="postgres", ), ] - - def move_to_volume(self, volume: BlockDevice) -> Job: - job = Jobs.add( - type_id="services.pleroma.move", - name="Move Pleroma", - description=f"Moving Pleroma to volume {volume.name}", - ) - move_service( - self, - volume, - job, - FolderMoveNames.default_foldermoves(self), - "pleroma", - ) - return job diff --git a/selfprivacy_api/services/service.py b/selfprivacy_api/services/service.py index 6255f20..224fde6 100644 --- a/selfprivacy_api/services/service.py +++ b/selfprivacy_api/services/service.py @@ -10,7 +10,15 @@ from selfprivacy_api.utils.block_devices import BlockDevice, BlockDevices from selfprivacy_api.services.generic_size_counter import get_storage_usage from selfprivacy_api.services.owned_path import OwnedPath -from selfprivacy_api.services.moving import check_folders, check_volume, unbind_folders, bind_folders, ensure_folder_ownership, MoveError, move_folders_to_volume +from selfprivacy_api.services.moving import ( + check_folders, + check_volume, + unbind_folders, + bind_folders, + ensure_folder_ownership, + MoveError, + move_folders_to_volume, +) from selfprivacy_api import utils from selfprivacy_api.utils.waitloop import wait_until_true @@ -300,7 +308,7 @@ class Service(ABC): @classmethod def set_location(cls, volume: BlockDevice): """ - Only changes userdata + Only changes userdata """ with WriteUserData() as user_data: @@ -313,15 +321,18 @@ class Service(ABC): def assert_can_move(self, new_volume): """ - Checks if the service can be moved to new volume - Raises errors if it cannot + Checks if the service can be moved to new volume + Raises errors if it cannot """ + service_name = self.get_display_name() + if not self.is_movable(): + raise MoveError(f"{service_name} is not movable") + with ReadUserData() as user_data: if not user_data.get("useBinds", False): raise MoveError("Server is not using binds.") current_volume_name = self.get_drive() - service_name = self.get_display_name() if current_volume_name == new_volume.name: raise MoveError(f"{service_name} is already on volume {new_volume}") @@ -333,7 +344,6 @@ class Service(ABC): check_folders(current_volume_name, owned_folders) - def do_move_to_volume( self, new_volume: BlockDevice, @@ -341,59 +351,57 @@ class Service(ABC): ): """ Move a service to another volume. - Is not allowed to raise errors because it is a task. """ service_name = self.get_display_name() old_volume_name = self.get_drive() owned_folders = self.get_owned_folders() - report_progress(0, job, "Performing pre-move checks...") + report_progress(10, job, "Unmounting folders from old volume...") + unbind_folders(owned_folders) - # TODO: move trying to the task + report_progress(20, job, "Moving data to new volume...") + move_folders_to_volume(owned_folders, old_volume_name, new_volume, job) + + report_progress(70, job, f"Making sure {service_name} owns its files...") try: - report_progress(5, job, f"Stopping {service_name}...") - - with StoppedService(self): - report_progress(10, job, "Unmounting folders from old volume...") - unbind_folders(owned_folders) - - report_progress(20, job, "Moving data to new volume...") - move_folders_to_volume(owned_folders, old_volume_name, new_volume, job) - - report_progress(70, job, f"Making sure {service_name} owns its files...") - try: - ensure_folder_ownership(owned_folders, new_volume, job, self) - except Exception as error: - # We have logged it via print and we additionally log it here in the error field - # We are continuing anyway but Job has no warning field - Jobs.update(job, JobStatus.RUNNING, error=f"Service {service_name} will not be able to write files: " + str(error)) - - report_progress(90, job, f"Mounting {service_name} data...") - bind_folders(owned_folders, new_volume) - - report_progress(95, job, f"Finishing moving {service_name}...") - self.set_location(self, new_volume) - - Jobs.update( - job=job, - status=JobStatus.FINISHED, - result=f"{service_name} moved successfully.", - status_text=f"Starting {service_name}...", - progress=100, - ) - except Exception as e: + ensure_folder_ownership(owned_folders, new_volume, job, self) + except Exception as error: + # We have logged it via print and we additionally log it here in the error field + # We are continuing anyway but Job has no warning field Jobs.update( - job=job, - status=JobStatus.ERROR, - error=type(e).__name__ + " " + str(e), + job, + JobStatus.RUNNING, + error=f"Service {service_name} will not be able to write files: " + + str(error), ) + report_progress(90, job, f"Mounting {service_name} data...") + bind_folders(owned_folders, new_volume) - @abstractmethod - def move_to_volume(self, volume: BlockDevice) -> Job: - """Cannot raise errors. - Returns errors as an errored out Job instead.""" - pass + report_progress(95, job, f"Finishing moving {service_name}...") + self.set_location(new_volume) + + Jobs.update( + job=job, + status=JobStatus.FINISHED, + result=f"{service_name} moved successfully.", + status_text=f"Starting {service_name}...", + progress=100, + ) + + def move_to_volume(self, volume: BlockDevice, job: Job) -> Job: + service_name = self.get_display_name() + + report_progress(0, job, "Performing pre-move checks...") + self.assert_can_move(volume) + + report_progress(5, job, f"Stopping {service_name}...") + assert self is not None + with StoppedService(self): + report_progress(9, job, f"Stopped server, starting the move...") + self.do_move_to_volume(volume, job) + + return job @classmethod def owned_path(cls, path: str): diff --git a/selfprivacy_api/services/tasks.py b/selfprivacy_api/services/tasks.py index 2cc52ad..ec44e37 100644 --- a/selfprivacy_api/services/tasks.py +++ b/selfprivacy_api/services/tasks.py @@ -1,11 +1,22 @@ from selfprivacy_api.services import Service from selfprivacy_api.utils.block_devices import BlockDevice from selfprivacy_api.utils.huey import huey +from selfprivacy_api.jobs import Job, Jobs, JobStatus @huey.task() -def move_service( - service: Service, - new_volume: BlockDevice, -): - service.move_to_volume(new_volume) +def move_service(service: Service, new_volume: BlockDevice, job: Job) -> bool: + """ + Move service's folders to new physical volume + Does not raise exceptions (we cannot handle exceptions from tasks). + Reports all errors via job. + """ + try: + service.move_to_volume(new_volume, job) + except Exception as e: + Jobs.update( + job=job, + status=JobStatus.ERROR, + error=type(e).__name__ + " " + str(e), + ) + return True diff --git a/selfprivacy_api/services/test_service/__init__.py b/selfprivacy_api/services/test_service/__init__.py index 803896b..f869bb3 100644 --- a/selfprivacy_api/services/test_service/__init__.py +++ b/selfprivacy_api/services/test_service/__init__.py @@ -11,7 +11,6 @@ from os import path from selfprivacy_api.jobs import Job, Jobs, JobStatus from selfprivacy_api.services.service import Service, ServiceDnsRecord, ServiceStatus from selfprivacy_api.utils.block_devices import BlockDevice -from selfprivacy_api.services.generic_service_mover import move_service, FolderMoveNames import selfprivacy_api.utils.network as network_utils from selfprivacy_api.services.test_service.icon import BITWARDEN_ICON @@ -189,23 +188,10 @@ class DummyService(Service): def get_folders(cls) -> List[str]: return cls.folders - def move_to_volume(self, volume: BlockDevice) -> Job: - job = Jobs.add( - type_id=f"services.{self.get_id()}.move", - name=f"Move {self.get_display_name()}", - description=f"Moving {self.get_display_name()} data to {volume.name}", - ) + def do_move_to_volume(self, volume: BlockDevice, job: Job) -> Job: if self.simulate_moving is False: - # completely generic code, TODO: make it the default impl. - move_service( - self, - volume, - job, - FolderMoveNames.default_foldermoves(self), - self.get_id(), - ) + return super(DummyService, self).do_move_to_volume(volume, job) else: Jobs.update(job, status=JobStatus.FINISHED) - - self.set_drive(volume.name) - return job + self.set_drive(volume.name) + return job diff --git a/tests/test_graphql/test_services.py b/tests/test_graphql/test_services.py index c6784ee..9208371 100644 --- a/tests/test_graphql/test_services.py +++ b/tests/test_graphql/test_services.py @@ -8,6 +8,8 @@ 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 @@ -32,7 +34,7 @@ MOVER_MOCK_PROCESS = CompletedProcess(["ls"], returncode=0) @pytest.fixture() def mock_check_service_mover_folders(mocker): mock = mocker.patch( - "selfprivacy_api.services.generic_service_mover.check_folders", + "selfprivacy_api.services.service.check_folders", autospec=True, return_value=None, ) @@ -495,9 +497,14 @@ def test_disable_enable(authorized_client, only_dummy_service): def test_move_immovable(authorized_client, only_dummy_service): dummy_service = only_dummy_service dummy_service.set_movable(False) - mutation_response = api_move(authorized_client, dummy_service, "sda1") + root = BlockDevices().get_root_block_device() + mutation_response = api_move(authorized_client, dummy_service, root.name) data = get_data(mutation_response)["services"]["moveService"] assert_errorcode(data, 400) + try: + assert "not movable" in data["message"] + except AssertionError: + raise ValueError("wrong type of error?: ", data["message"]) # is there a meaning in returning the service in this? assert data["service"] is not None @@ -519,8 +526,7 @@ def test_move_no_such_volume(authorized_client, only_dummy_service): data = get_data(mutation_response)["services"]["moveService"] assert_notfound(data) - # is there a meaning in returning the service in this? - assert data["service"] is not None + assert data["service"] is None assert data["job"] is None @@ -538,7 +544,8 @@ def test_move_same_volume(authorized_client, dummy_service): # is there a meaning in returning the service in this? assert data["service"] is not None - assert data["job"] is not None + # We do not create a job if task is not created + assert data["job"] is None def test_graphql_move_service_without_folders_on_old_volume( diff --git a/tests/test_services.py b/tests/test_services.py index de3665a..98cfa4e 100644 --- a/tests/test_services.py +++ b/tests/test_services.py @@ -13,7 +13,6 @@ from selfprivacy_api.services.bitwarden import Bitwarden from selfprivacy_api.services.pleroma import Pleroma from selfprivacy_api.services.mailserver import MailServer from selfprivacy_api.services.owned_path import OwnedPath -from selfprivacy_api.services.generic_service_mover import FolderMoveNames from selfprivacy_api.services.test_service import DummyService from selfprivacy_api.services.service import Service, ServiceStatus, StoppedService @@ -81,19 +80,19 @@ def test_paths_from_owned_paths(): ] -def test_foldermoves_from_ownedpaths(): - owned = OwnedPath( - path="var/lib/bitwarden", - group="vaultwarden", - owner="vaultwarden", - ) +# def test_foldermoves_from_ownedpaths(): +# owned = OwnedPath( +# path="var/lib/bitwarden", +# group="vaultwarden", +# owner="vaultwarden", +# ) - assert FolderMoveNames.from_owned_path(owned) == FolderMoveNames( - name="bitwarden", - bind_location="var/lib/bitwarden", - group="vaultwarden", - owner="vaultwarden", - ) +# assert FolderMoveNames.from_owned_path(owned) == FolderMoveNames( +# name="bitwarden", +# bind_location="var/lib/bitwarden", +# group="vaultwarden", +# owner="vaultwarden", +# ) def test_enabling_disabling_reads_json(dummy_service: DummyService): From f059c83b57d2a806db31bcc226198a1950ef739c Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 19 Feb 2024 00:10:13 +0000 Subject: [PATCH 12/20] fix(services): check for possible None progress when moving folders --- selfprivacy_api/services/moving.py | 3 +++ selfprivacy_api/services/service.py | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/selfprivacy_api/services/moving.py b/selfprivacy_api/services/moving.py index ecc505b..ce13b30 100644 --- a/selfprivacy_api/services/moving.py +++ b/selfprivacy_api/services/moving.py @@ -60,6 +60,9 @@ def move_folders_to_volume( job: Job, ) -> None: current_progress = job.progress + if current_progress is None: + current_progress = 0 + folder_percentage = 50 // len(folders) for folder in folders: folder_name = get_foldername(folder) diff --git a/selfprivacy_api/services/service.py b/selfprivacy_api/services/service.py index 224fde6..233e6e5 100644 --- a/selfprivacy_api/services/service.py +++ b/selfprivacy_api/services/service.py @@ -311,8 +311,8 @@ class Service(ABC): Only changes userdata """ + service_id = cls.get_id() with WriteUserData() as user_data: - service_id = cls.get_id() if "modules" not in user_data: user_data["modules"] = {} if service_id not in user_data["modules"]: From baaf3299cea973fec0558ac6f443751e23be79ec Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 19 Feb 2024 00:24:32 +0000 Subject: [PATCH 13/20] refactor(services): fix type annotation --- selfprivacy_api/services/moving.py | 6 +++--- selfprivacy_api/services/service.py | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/selfprivacy_api/services/moving.py b/selfprivacy_api/services/moving.py index ce13b30..e311c4f 100644 --- a/selfprivacy_api/services/moving.py +++ b/selfprivacy_api/services/moving.py @@ -19,7 +19,7 @@ def get_foldername(p: OwnedPath) -> str: return p.path.split("/")[-1] -def check_volume(volume: BlockDevice, space_needed: int) -> bool: +def check_volume(volume: BlockDevice, space_needed: int) -> None: # Check if there is enough space on the new volume if int(volume.fsavail) < space_needed: raise MoveError("Not enough space on the new volume.") @@ -55,7 +55,7 @@ def unbind_folders(owned_folders: List[OwnedPath]) -> None: def move_folders_to_volume( folders: List[OwnedPath], - old_volume: BlockDevice, + old_volume_name: str, # TODO: pass an actual validated block device new_volume: BlockDevice, job: Job, ) -> None: @@ -67,7 +67,7 @@ def move_folders_to_volume( for folder in folders: folder_name = get_foldername(folder) shutil.move( - f"/volumes/{old_volume}/{folder_name}", + f"/volumes/{old_volume_name}/{folder_name}", f"/volumes/{new_volume.name}/{folder_name}", ) progress = current_progress + folder_percentage diff --git a/selfprivacy_api/services/service.py b/selfprivacy_api/services/service.py index 233e6e5..da3f5ca 100644 --- a/selfprivacy_api/services/service.py +++ b/selfprivacy_api/services/service.py @@ -353,6 +353,7 @@ class Service(ABC): Move a service to another volume. """ service_name = self.get_display_name() + # TODO: validate that this volume exists old_volume_name = self.get_drive() owned_folders = self.get_owned_folders() From 18934a53e636cc883ea349402aed118c43eb8dbf Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 19 Feb 2024 18:37:00 +0000 Subject: [PATCH 14/20] refactor(services): break out location construction when moving --- selfprivacy_api/services/moving.py | 25 ++++++++++++++----------- selfprivacy_api/services/service.py | 6 +++--- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/selfprivacy_api/services/moving.py b/selfprivacy_api/services/moving.py index e311c4f..ba9b2c9 100644 --- a/selfprivacy_api/services/moving.py +++ b/selfprivacy_api/services/moving.py @@ -19,6 +19,10 @@ def get_foldername(p: OwnedPath) -> str: return p.path.split("/")[-1] +def location_at_volume(binding_path: OwnedPath, volume_name: str): + return f"/volumes/{volume_name}/{get_foldername(binding_path)}" + + def check_volume(volume: BlockDevice, space_needed: int) -> None: # Check if there is enough space on the new volume if int(volume.fsavail) < space_needed: @@ -29,10 +33,10 @@ def check_volume(volume: BlockDevice, space_needed: int) -> None: raise MoveError("Volume is not mounted.") -def check_folders(current_volume: BlockDevice, folders: List[OwnedPath]) -> None: +def check_folders(volume_name: str, folders: List[OwnedPath]) -> None: # Make sure current actual directory exists and if its user and group are correct for folder in folders: - path = pathlib.Path(f"/volumes/{current_volume}/{get_foldername(folder)}") + path = pathlib.Path(location_at_volume(folder, volume_name)) if not path.exists(): raise MoveError(f"directory {path} is not found.") @@ -55,7 +59,7 @@ def unbind_folders(owned_folders: List[OwnedPath]) -> None: def move_folders_to_volume( folders: List[OwnedPath], - old_volume_name: str, # TODO: pass an actual validated block device + old_volume_name: str, # TODO: pass an actual validated block device new_volume: BlockDevice, job: Job, ) -> None: @@ -63,20 +67,19 @@ def move_folders_to_volume( if current_progress is None: current_progress = 0 - folder_percentage = 50 // len(folders) + progress_per_folder = 50 // len(folders) for folder in folders: - folder_name = get_foldername(folder) shutil.move( - f"/volumes/{old_volume_name}/{folder_name}", - f"/volumes/{new_volume.name}/{folder_name}", + location_at_volume(folder, old_volume_name), + location_at_volume(folder, new_volume.name), ) - progress = current_progress + folder_percentage + progress = current_progress + progress_per_folder report_progress(progress, job, "Moving data to new volume...") def ensure_folder_ownership(folders: List[OwnedPath], volume: BlockDevice) -> None: for folder in folders: - true_location = f"/volumes/{volume.name}/{get_foldername(folder)}" + true_location = location_at_volume(folder, volume.name) try: subprocess.run( [ @@ -89,10 +92,10 @@ def ensure_folder_ownership(folders: List[OwnedPath], volume: BlockDevice) -> No check=True, ) except subprocess.CalledProcessError as error: + print(error.output) error_message = ( f"Unable to set ownership of {true_location} :{error.output}" ) - print(error.output) raise MoveError(error_message) @@ -103,7 +106,7 @@ def bind_folders(folders: List[OwnedPath], volume: BlockDevice) -> None: [ "mount", "--bind", - f"/volumes/{volume.name}/{get_foldername(folder)}", + location_at_volume(folder, volume.name), folder.path, ], check=True, diff --git a/selfprivacy_api/services/service.py b/selfprivacy_api/services/service.py index da3f5ca..e60cf8a 100644 --- a/selfprivacy_api/services/service.py +++ b/selfprivacy_api/services/service.py @@ -353,7 +353,7 @@ class Service(ABC): Move a service to another volume. """ service_name = self.get_display_name() - # TODO: validate that this volume exists + # TODO : Make sure device exists old_volume_name = self.get_drive() owned_folders = self.get_owned_folders() @@ -365,7 +365,7 @@ class Service(ABC): report_progress(70, job, f"Making sure {service_name} owns its files...") try: - ensure_folder_ownership(owned_folders, new_volume, job, self) + ensure_folder_ownership(owned_folders, new_volume) except Exception as error: # We have logged it via print and we additionally log it here in the error field # We are continuing anyway but Job has no warning field @@ -399,7 +399,7 @@ class Service(ABC): report_progress(5, job, f"Stopping {service_name}...") assert self is not None with StoppedService(self): - report_progress(9, job, f"Stopped server, starting the move...") + report_progress(9, job, "Stopped service, starting the move...") self.do_move_to_volume(volume, job) return job From 00682723828e04d563e4f6090aebf2df8c234b97 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 26 Feb 2024 15:01:07 +0000 Subject: [PATCH 15/20] feature(backups): intermittent commit for binds, to be replaced --- .../graphql/mutations/services_mutations.py | 6 +- selfprivacy_api/services/moving.py | 102 +++++------------- selfprivacy_api/services/owned_path.py | 96 +++++++++++++++++ selfprivacy_api/services/service.py | 33 +++--- selfprivacy_api/utils/block_devices.py | 2 + tests/test_graphql/test_services.py | 6 +- 6 files changed, 154 insertions(+), 91 deletions(-) diff --git a/selfprivacy_api/graphql/mutations/services_mutations.py b/selfprivacy_api/graphql/mutations/services_mutations.py index 911ad26..97eb4d9 100644 --- a/selfprivacy_api/graphql/mutations/services_mutations.py +++ b/selfprivacy_api/graphql/mutations/services_mutations.py @@ -7,6 +7,8 @@ from selfprivacy_api.graphql.common_types.jobs import job_to_api_job from selfprivacy_api.jobs import JobStatus from selfprivacy_api.utils.block_devices import BlockDevices +from traceback import format_tb as format_traceback + from selfprivacy_api.graphql.mutations.mutation_interface import ( GenericJobMutationReturn, GenericMutationReturn, @@ -171,6 +173,7 @@ class ServicesMutations: try: job = move_service(input.service_id, input.location) + except (ServiceNotFoundError, VolumeNotFoundError) as e: return ServiceJobMutationReturn( success=False, @@ -212,4 +215,5 @@ class ServicesMutations: def pretty_error(e: Exception) -> str: - return type(e).__name__ + ": " + str(e) + traceback = "/r".join(format_traceback(e.__traceback__)) + return type(e).__name__ + ": " + str(e) + ": " + traceback diff --git a/selfprivacy_api/services/moving.py b/selfprivacy_api/services/moving.py index ba9b2c9..09af765 100644 --- a/selfprivacy_api/services/moving.py +++ b/selfprivacy_api/services/moving.py @@ -1,26 +1,16 @@ """Generic handler for moving services""" from __future__ import annotations -import subprocess -import pathlib import shutil from typing import List from selfprivacy_api.jobs import Job, report_progress from selfprivacy_api.utils.block_devices import BlockDevice -from selfprivacy_api.services.owned_path import OwnedPath +from selfprivacy_api.services.owned_path import Bind class MoveError(Exception): - """Move failed""" - - -def get_foldername(p: OwnedPath) -> str: - return p.path.split("/")[-1] - - -def location_at_volume(binding_path: OwnedPath, volume_name: str): - return f"/volumes/{volume_name}/{get_foldername(binding_path)}" + """Move of the data has failed""" def check_volume(volume: BlockDevice, space_needed: int) -> None: @@ -33,84 +23,50 @@ def check_volume(volume: BlockDevice, space_needed: int) -> None: raise MoveError("Volume is not mounted.") -def check_folders(volume_name: str, folders: List[OwnedPath]) -> None: +def check_binds(volume_name: str, binds: List[Bind]) -> None: # Make sure current actual directory exists and if its user and group are correct - for folder in folders: - path = pathlib.Path(location_at_volume(folder, volume_name)) - - if not path.exists(): - raise MoveError(f"directory {path} is not found.") - if not path.is_dir(): - raise MoveError(f"{path} is not a directory.") - if path.owner() != folder.owner: - raise MoveError(f"{path} is not owned by {folder.owner}.") + for bind in binds: + bind.validate() -def unbind_folders(owned_folders: List[OwnedPath]) -> None: +def unbind_folders(owned_folders: List[Bind]) -> None: for folder in owned_folders: - try: - subprocess.run( - ["umount", folder.path], - check=True, - ) - except subprocess.CalledProcessError: - raise MoveError(f"Unable to unmount folder {folder.path}.") + folder.unbind() -def move_folders_to_volume( - folders: List[OwnedPath], - old_volume_name: str, # TODO: pass an actual validated block device +# May be moved into Bind +def move_data_to_volume( + binds: List[Bind], new_volume: BlockDevice, job: Job, -) -> None: +) -> List[Bind]: current_progress = job.progress if current_progress is None: current_progress = 0 - progress_per_folder = 50 // len(folders) - for folder in folders: - shutil.move( - location_at_volume(folder, old_volume_name), - location_at_volume(folder, new_volume.name), - ) + progress_per_folder = 50 // len(binds) + for bind in binds: + old_location = bind.location_at_volume() + bind.drive = new_volume + new_location = bind.location_at_volume() + + try: + shutil.move(old_location, new_location) + except Exception as error: + raise MoveError( + f"could not move {old_location} to {new_location} : {str(error)}" + ) from error + progress = current_progress + progress_per_folder report_progress(progress, job, "Moving data to new volume...") + return binds -def ensure_folder_ownership(folders: List[OwnedPath], volume: BlockDevice) -> None: +def ensure_folder_ownership(folders: List[Bind]) -> None: for folder in folders: - true_location = location_at_volume(folder, volume.name) - try: - subprocess.run( - [ - "chown", - "-R", - f"{folder.owner}:{folder.group}", - # Could we just chown the binded location instead? - true_location, - ], - check=True, - ) - except subprocess.CalledProcessError as error: - print(error.output) - error_message = ( - f"Unable to set ownership of {true_location} :{error.output}" - ) - raise MoveError(error_message) + folder.ensure_ownership() -def bind_folders(folders: List[OwnedPath], volume: BlockDevice) -> None: +def bind_folders(folders: List[Bind], volume: BlockDevice): for folder in folders: - try: - subprocess.run( - [ - "mount", - "--bind", - location_at_volume(folder, volume.name), - folder.path, - ], - check=True, - ) - except subprocess.CalledProcessError as error: - print(error.output) - raise MoveError(f"Unable to mount new volume:{error.output}") + folder.bind() diff --git a/selfprivacy_api/services/owned_path.py b/selfprivacy_api/services/owned_path.py index 23542dc..da40510 100644 --- a/selfprivacy_api/services/owned_path.py +++ b/selfprivacy_api/services/owned_path.py @@ -1,7 +1,103 @@ +from __future__ import annotations +import subprocess +import pathlib from pydantic import BaseModel +from selfprivacy_api.utils.block_devices import BlockDevice, BlockDevices + +class BindError(Exception): + pass + + +# May be deprecated because of Binds class OwnedPath(BaseModel): path: str owner: str group: str + + +class Bind: + """ + A directory that resides on some volume but we mount it into fs + where we need it. + Used for service data. + """ + + def __init__(self, binding_path: str, owner: str, group: str, drive: BlockDevice): + self.binding_path = binding_path + self.owner = owner + self.group = group + self.drive = drive + + # TODO: make Service return a list of binds instead of owned paths + @staticmethod + def from_owned_path(path: OwnedPath, drive_name: str) -> Bind: + drive = BlockDevices().get_block_device(drive_name) + if drive is None: + raise BindError(f"No such drive: {drive_name}") + + return Bind( + binding_path=path.path, owner=path.owner, group=path.group, drive=drive + ) + + def bind_foldername(self) -> str: + return self.binding_path.split("/")[-1] + + def location_at_volume(self) -> str: + return f"/volumes/{self.drive.name}/{self.bind_foldername()}" + + def validate(self) -> str: + path = pathlib.Path(self.location_at_volume()) + + if not path.exists(): + raise BindError(f"directory {path} is not found.") + if not path.is_dir(): + raise BindError(f"{path} is not a directory.") + if path.owner() != self.owner: + raise BindError(f"{path} is not owned by {self.owner}.") + + def bind(self) -> None: + try: + subprocess.run( + [ + "mount", + "--bind", + self.location_at_volume(), + self.binding_path, + ], + check=True, + ) + except subprocess.CalledProcessError as error: + print(error.output) + raise BindError(f"Unable to mount new volume:{error.output}") + + def unbind(self) -> None: + try: + subprocess.run( + ["umount", self.binding_path], + check=True, + ) + except subprocess.CalledProcessError: + raise BindError(f"Unable to unmount folder {self.binding_path}.") + pass + + def ensure_ownership(self) -> None: + true_location = self.location_at_volume() + try: + subprocess.run( + [ + "chown", + "-R", + f"{self.owner}:{self.group}", + # Could we just chown the binded location instead? + true_location, + ], + check=True, + ) + except subprocess.CalledProcessError as error: + print(error.output) + error_message = ( + f"Unable to set ownership of {true_location} :{error.output}" + ) + raise BindError(error_message) diff --git a/selfprivacy_api/services/service.py b/selfprivacy_api/services/service.py index e60cf8a..19e395e 100644 --- a/selfprivacy_api/services/service.py +++ b/selfprivacy_api/services/service.py @@ -9,15 +9,15 @@ from selfprivacy_api.jobs import Job, Jobs, JobStatus, report_progress from selfprivacy_api.utils.block_devices import BlockDevice, BlockDevices from selfprivacy_api.services.generic_size_counter import get_storage_usage -from selfprivacy_api.services.owned_path import OwnedPath +from selfprivacy_api.services.owned_path import OwnedPath, Bind from selfprivacy_api.services.moving import ( - check_folders, + check_binds, check_volume, unbind_folders, bind_folders, ensure_folder_ownership, MoveError, - move_folders_to_volume, + move_data_to_volume, ) from selfprivacy_api import utils @@ -319,6 +319,13 @@ class Service(ABC): user_data["modules"][service_id] = {} user_data["modules"][service_id]["location"] = volume.name + def binds(self) -> typing.List[Bind]: + owned_folders = self.get_owned_folders() + + return [ + Bind.from_owned_path(folder, self.get_drive()) for folder in owned_folders + ] + def assert_can_move(self, new_volume): """ Checks if the service can be moved to new volume @@ -338,11 +345,10 @@ class Service(ABC): check_volume(new_volume, space_needed=self.get_storage_usage()) - owned_folders = self.get_owned_folders() - if owned_folders == []: + binds = self.binds() + if binds == []: raise MoveError("nothing to move") - - check_folders(current_volume_name, owned_folders) + check_binds(current_volume_name, binds) def do_move_to_volume( self, @@ -351,21 +357,20 @@ class Service(ABC): ): """ Move a service to another volume. + Note: It may be much simpler to write it per bind, but a bit less safe? """ service_name = self.get_display_name() - # TODO : Make sure device exists - old_volume_name = self.get_drive() - owned_folders = self.get_owned_folders() + binds = self.binds() report_progress(10, job, "Unmounting folders from old volume...") - unbind_folders(owned_folders) + unbind_folders(binds) report_progress(20, job, "Moving data to new volume...") - move_folders_to_volume(owned_folders, old_volume_name, new_volume, job) + binds = move_data_to_volume(binds, new_volume, job) report_progress(70, job, f"Making sure {service_name} owns its files...") try: - ensure_folder_ownership(owned_folders, new_volume) + ensure_folder_ownership(binds) except Exception as error: # We have logged it via print and we additionally log it here in the error field # We are continuing anyway but Job has no warning field @@ -377,7 +382,7 @@ class Service(ABC): ) report_progress(90, job, f"Mounting {service_name} data...") - bind_folders(owned_folders, new_volume) + bind_folders(binds) report_progress(95, job, f"Finishing moving {service_name}...") self.set_location(new_volume) diff --git a/selfprivacy_api/utils/block_devices.py b/selfprivacy_api/utils/block_devices.py index ab3794d..f1a4149 100644 --- a/selfprivacy_api/utils/block_devices.py +++ b/selfprivacy_api/utils/block_devices.py @@ -4,6 +4,8 @@ import subprocess import json import typing +from pydantic import BaseModel + from selfprivacy_api.utils import WriteUserData from selfprivacy_api.utils.singleton_metaclass import SingletonMetaclass diff --git a/tests/test_graphql/test_services.py b/tests/test_graphql/test_services.py index 9208371..d509a6f 100644 --- a/tests/test_graphql/test_services.py +++ b/tests/test_graphql/test_services.py @@ -32,9 +32,9 @@ MOVER_MOCK_PROCESS = CompletedProcess(["ls"], returncode=0) @pytest.fixture() -def mock_check_service_mover_folders(mocker): +def mock_check_service_mover_binds(mocker): mock = mocker.patch( - "selfprivacy_api.services.service.check_folders", + "selfprivacy_api.services.service.check_binds", autospec=True, return_value=None, ) @@ -569,7 +569,7 @@ def test_graphql_move_service_without_folders_on_old_volume( def test_graphql_move_service( authorized_client, generic_userdata, - mock_check_service_mover_folders, + mock_check_service_mover_binds, lsblk_singular_mock, dummy_service: DummyService, mock_subprocess_run, From 1599f601a26fd37abdf8a7df42892fcb63e8f405 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Thu, 29 Feb 2024 00:54:39 +0000 Subject: [PATCH 16/20] 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"] From 8402f66a33f30a293b0bfc7aca0bd02e9120e52c Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Fri, 1 Mar 2024 14:30:54 +0000 Subject: [PATCH 17/20] refactor(services): remove too many imports and cleanup --- selfprivacy_api/actions/services.py | 4 +--- selfprivacy_api/graphql/mutations/services_mutations.py | 2 -- selfprivacy_api/jobs/__init__.py | 8 +++++--- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/selfprivacy_api/actions/services.py b/selfprivacy_api/actions/services.py index 56d35e9..ebb0917 100644 --- a/selfprivacy_api/actions/services.py +++ b/selfprivacy_api/actions/services.py @@ -30,7 +30,5 @@ def move_service(service_id: str, volume_name: str) -> Job: description=f"Moving {service.get_display_name()} data to {volume.name}", ) - handle = move_service_task(service, volume, job) - # Nonblocking - handle() + move_service_task(service, volume, job) return job diff --git a/selfprivacy_api/graphql/mutations/services_mutations.py b/selfprivacy_api/graphql/mutations/services_mutations.py index 97eb4d9..be0cb77 100644 --- a/selfprivacy_api/graphql/mutations/services_mutations.py +++ b/selfprivacy_api/graphql/mutations/services_mutations.py @@ -5,7 +5,6 @@ import strawberry from selfprivacy_api.graphql import IsAuthenticated from selfprivacy_api.graphql.common_types.jobs import job_to_api_job from selfprivacy_api.jobs import JobStatus -from selfprivacy_api.utils.block_devices import BlockDevices from traceback import format_tb as format_traceback @@ -23,7 +22,6 @@ from selfprivacy_api.actions.services import ( ServiceNotFoundError, VolumeNotFoundError, ) -from selfprivacy_api.services.moving import MoveError from selfprivacy_api.services import get_service_by_id diff --git a/selfprivacy_api/jobs/__init__.py b/selfprivacy_api/jobs/__init__.py index 7f46e9d..4649bb0 100644 --- a/selfprivacy_api/jobs/__init__.py +++ b/selfprivacy_api/jobs/__init__.py @@ -268,10 +268,12 @@ class Jobs: return False -# A terse way to call a common operation, for readability -# job.report_progress() would be even better -# but it would go against how this file is written def report_progress(progress: int, job: Job, status_text: str) -> None: + """ + A terse way to call a common operation, for readability + job.report_progress() would be even better + but it would go against how this file is written + """ Jobs.update( job=job, status=JobStatus.RUNNING, From 2c1c783b5e7deafe048350524843b22bc149fd1f Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 4 Mar 2024 14:26:26 +0000 Subject: [PATCH 18/20] fix(services): fix merge bug --- selfprivacy_api/services/service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/selfprivacy_api/services/service.py b/selfprivacy_api/services/service.py index 19e395e..576877c 100644 --- a/selfprivacy_api/services/service.py +++ b/selfprivacy_api/services/service.py @@ -319,7 +319,7 @@ class Service(ABC): user_data["modules"][service_id] = {} user_data["modules"][service_id]["location"] = volume.name - def binds(self) -> typing.List[Bind]: + def binds(self) -> List[Bind]: owned_folders = self.get_owned_folders() return [ From 1bed9d87ca455305f0bd5c12a3a7cc5b9b7315af Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 4 Mar 2024 17:16:08 +0000 Subject: [PATCH 19/20] doc(services): explain the Owned Path reason d'etre after trying to remove it --- selfprivacy_api/services/owned_path.py | 15 ++++++++++++++- selfprivacy_api/services/service.py | 22 +++++++++++++++++++--- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/selfprivacy_api/services/owned_path.py b/selfprivacy_api/services/owned_path.py index 6e85fb0..aa6e92e 100644 --- a/selfprivacy_api/services/owned_path.py +++ b/selfprivacy_api/services/owned_path.py @@ -14,8 +14,21 @@ class BindError(Exception): pass -# May be deprecated because of Binds class OwnedPath(BaseModel): + """ + A convenient interface for explicitly defining ownership of service folders. + One overrides Service.get_owned_paths() for this. + + Why this exists?: + One could use Bind to define ownership but then one would need to handle drive which + is unnecessary and produces code duplication. + + It is also somewhat semantically wrong to include Owned Path into Bind + instead of user and group. Because owner and group in Bind are applied to + the original folder on the drive, not to the binding path. But maybe it is + ok since they are technically both owned. Idk yet. + """ + path: str owner: str group: str diff --git a/selfprivacy_api/services/service.py b/selfprivacy_api/services/service.py index 576877c..9add2dc 100644 --- a/selfprivacy_api/services/service.py +++ b/selfprivacy_api/services/service.py @@ -411,11 +411,27 @@ class Service(ABC): @classmethod def owned_path(cls, path: str): - """A default guess on folder ownership""" + """Default folder ownership""" + service_name = cls.get_display_name() + + try: + owner = cls.get_user() + if owner is None: + # TODO: assume root? + # (if we do not want to do assumptions, maybe not declare user optional?) + raise LookupError(f"no user for service: {service_name}") + group = cls.get_group() + if group is None: + raise LookupError(f"no group for service: {service_name}") + except Exception as error: + raise LookupError( + f"when deciding a bind for folder {path} of service {service_name}, error: {str(error)}" + ) + return OwnedPath( path=path, - owner=cls.get_user(), - group=cls.get_group(), + owner=owner, + group=group, ) def pre_backup(self): From ee7c41e0c20d2ae6fc3a5363125649ac60cf7668 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 4 Mar 2024 17:37:26 +0000 Subject: [PATCH 20/20] test(services): clean up tests --- tests/conftest.py | 2 +- tests/test_graphql/test_services.py | 13 ------------- tests/test_services.py | 15 --------------- 3 files changed, 1 insertion(+), 29 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 0cd1493..dceac72 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -149,7 +149,7 @@ def volume_folders(tmpdir, mocker): @pytest.fixture() -def raw_dummy_service(tmpdir): +def raw_dummy_service(tmpdir) -> DummyService: dirnames = ["test_service", "also_test_service"] service_dirs = [] for d in dirnames: diff --git a/tests/test_graphql/test_services.py b/tests/test_graphql/test_services.py index 6d841d6..06ef3a1 100644 --- a/tests/test_graphql/test_services.py +++ b/tests/test_graphql/test_services.py @@ -105,17 +105,6 @@ def mock_check_volume(mocker): return mock -# TODO: remove -@pytest.fixture() -def mock_check_service_mover_binds(mocker): - mock = mocker.patch( - "selfprivacy_api.services.service.check_binds", - autospec=True, - return_value=None, - ) - return mock - - API_START_MUTATION = """ mutation TestStartService($service_id: String!) { services { @@ -631,8 +620,6 @@ def test_graphql_move_service_without_folders_on_old_volume( def test_graphql_move_service( authorized_client, generic_userdata, - # TODO: substitute with a weaker mock or delete altogether - mock_check_service_mover_binds, mock_check_volume, dummy_service_with_binds, ): diff --git a/tests/test_services.py b/tests/test_services.py index 98cfa4e..de828d8 100644 --- a/tests/test_services.py +++ b/tests/test_services.py @@ -80,21 +80,6 @@ def test_paths_from_owned_paths(): ] -# def test_foldermoves_from_ownedpaths(): -# owned = OwnedPath( -# path="var/lib/bitwarden", -# group="vaultwarden", -# owner="vaultwarden", -# ) - -# assert FolderMoveNames.from_owned_path(owned) == FolderMoveNames( -# name="bitwarden", -# bind_location="var/lib/bitwarden", -# group="vaultwarden", -# owner="vaultwarden", -# ) - - def test_enabling_disabling_reads_json(dummy_service: DummyService): with WriteUserData() as data: data["modules"][dummy_service.get_id()]["enable"] = False