From e9bb6d9973a8c1b948619e62dc49d658845c8b64 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Wed, 26 Jul 2023 10:09:27 +0000 Subject: [PATCH 01/27] test(backups):check that snapshot cache invalidation invalidates both ways. --- tests/test_graphql/test_backup.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/test_graphql/test_backup.py b/tests/test_graphql/test_backup.py index 6d12a5e..dcb4739 100644 --- a/tests/test_graphql/test_backup.py +++ b/tests/test_graphql/test_backup.py @@ -552,6 +552,34 @@ def test_snapshots_caching(backups, dummy_service): assert len(cached_snapshots) == 1 +def lowlevel_forget(snapshot_id): + Backups.provider().backupper.forget_snapshot(snapshot_id) + + +# Storage +def test_snapshots_cache_invalidation(backups, dummy_service): + Backups.back_up(dummy_service) + cached_snapshots = Storage.get_cached_snapshots() + assert len(cached_snapshots) == 1 + + Storage.invalidate_snapshot_storage() + cached_snapshots = Storage.get_cached_snapshots() + assert len(cached_snapshots) == 0 + + Backups.force_snapshot_cache_reload() + cached_snapshots = Storage.get_cached_snapshots() + assert len(cached_snapshots) == 1 + snap = cached_snapshots[0] + + lowlevel_forget(snap.id) + cached_snapshots = Storage.get_cached_snapshots() + assert len(cached_snapshots) == 1 + + Backups.force_snapshot_cache_reload() + cached_snapshots = Storage.get_cached_snapshots() + assert len(cached_snapshots) == 0 + + # Storage def test_init_tracking_caching(backups, raw_dummy_service): assert Storage.has_init_mark() is False From aa7cc7155756791a2353635fd0f8dd9ae37d107f Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Wed, 26 Jul 2023 11:54:17 +0000 Subject: [PATCH 02/27] feature(backups):add a function to set provider from env --- selfprivacy_api/backup/__init__.py | 26 ++++++++++++++++++++ tests/test_graphql/test_backup.py | 38 +++++++++++++++++++++++++++++- 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/selfprivacy_api/backup/__init__.py b/selfprivacy_api/backup/__init__.py index 9722b71..94d1abb 100644 --- a/selfprivacy_api/backup/__init__.py +++ b/selfprivacy_api/backup/__init__.py @@ -2,6 +2,7 @@ This module contains the controller class for backups. """ from datetime import datetime, timedelta +import os from os import statvfs from typing import List, Optional @@ -43,6 +44,13 @@ DEFAULT_JSON_PROVIDER = { "bucket": "", } +BACKUP_PROVIDER_ENVS = { + "kind": "BACKUP_KIND", + "login": "BACKUP_LOGIN", + "key": "BACKUP_KEY", + "location": "BACKUP_LOCATION", +} + class NotDeadError(AssertionError): """ @@ -132,6 +140,24 @@ class Backups: Storage.store_provider(none_provider) return none_provider + @staticmethod + def set_provider_from_envs(): + for env in BACKUP_PROVIDER_ENVS.values(): + if env not in os.environ.keys(): + raise ValueError( + f"Cannot set backup provider from envs, there is no {env} set" + ) + + kind_str = os.environ[BACKUP_PROVIDER_ENVS["kind"]] + kind_enum = BackupProviderEnum[kind_str] + provider = Backups._construct_provider( + kind=kind_enum, + login=os.environ[BACKUP_PROVIDER_ENVS["login"]], + key=os.environ[BACKUP_PROVIDER_ENVS["key"]], + location=os.environ[BACKUP_PROVIDER_ENVS["location"]], + ) + Storage.store_provider(provider) + @staticmethod def _construct_provider( kind: BackupProviderEnum, diff --git a/tests/test_graphql/test_backup.py b/tests/test_graphql/test_backup.py index dcb4739..fcb437e 100644 --- a/tests/test_graphql/test_backup.py +++ b/tests/test_graphql/test_backup.py @@ -1,4 +1,5 @@ import pytest +import os import os.path as path from os import makedirs from os import remove @@ -18,10 +19,11 @@ from selfprivacy_api.jobs import Jobs, JobStatus from selfprivacy_api.models.backup.snapshot import Snapshot -from selfprivacy_api.backup import Backups +from selfprivacy_api.backup import Backups, BACKUP_PROVIDER_ENVS import selfprivacy_api.backup.providers as providers from selfprivacy_api.backup.providers import AbstractBackupProvider from selfprivacy_api.backup.providers.backblaze import Backblaze +from selfprivacy_api.backup.providers.none import NoBackups from selfprivacy_api.backup.util import sync from selfprivacy_api.backup.backuppers.restic_backupper import ResticBackupper from selfprivacy_api.backup.jobs import add_backup_job, add_restore_job @@ -129,6 +131,40 @@ def test_config_load(generic_userdata): assert provider.backupper.key == "KEY" +def test_reset_sets_to_none1(): + Backups.reset() + provider = Backups.provider() + assert provider is not None + assert isinstance(provider, NoBackups) + + +def test_reset_sets_to_none2(backups): + # now with something set up first^^^ + Backups.reset() + provider = Backups.provider() + assert provider is not None + assert isinstance(provider, NoBackups) + + +def test_setting_from_envs(tmpdir): + Backups.reset() + os.environ[BACKUP_PROVIDER_ENVS["kind"]] = "BACKBLAZE" + os.environ[BACKUP_PROVIDER_ENVS["login"]] = "ID" + os.environ[BACKUP_PROVIDER_ENVS["key"]] = "KEY" + os.environ[BACKUP_PROVIDER_ENVS["location"]] = "selfprivacy" + Backups.set_provider_from_envs() + provider = Backups.provider() + + assert provider is not None + assert isinstance(provider, Backblaze) + assert provider.login == "ID" + assert provider.key == "KEY" + assert provider.location == "selfprivacy" + + assert provider.backupper.account == "ID" + assert provider.backupper.key == "KEY" + + def test_json_reset(generic_userdata): Backups.reset(reset_json=False) provider = Backups.provider() From ffec344ba81b9b7355930c8f65e8d7bae5aa0ab8 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Wed, 26 Jul 2023 14:26:04 +0000 Subject: [PATCH 03/27] test(backups): make the test repo overridable by envs --- selfprivacy_api/backup/__init__.py | 10 ++++- tests/test_graphql/test_backup.py | 64 ++++++++++++++++++++++++------ 2 files changed, 60 insertions(+), 14 deletions(-) diff --git a/selfprivacy_api/backup/__init__.py b/selfprivacy_api/backup/__init__.py index 94d1abb..725904e 100644 --- a/selfprivacy_api/backup/__init__.py +++ b/selfprivacy_api/backup/__init__.py @@ -431,10 +431,18 @@ class Backups: @staticmethod def forget_snapshot(snapshot: Snapshot) -> None: - """Deletes a snapshot from the storage""" + """Deletes a snapshot from the repo and from cache""" Backups.provider().backupper.forget_snapshot(snapshot.id) Storage.delete_cached_snapshot(snapshot) + @staticmethod + def forget_all_snapshots(): + """deliberately erase all snapshots we made""" + # there is no dedicated optimized command for this, + # but maybe we can have a multi-erase + for snapshot in Backups.get_all_snapshots(): + Backups.forget_snapshot(snapshot) + @staticmethod def force_snapshot_cache_reload() -> None: """ diff --git a/tests/test_graphql/test_backup.py b/tests/test_graphql/test_backup.py index fcb437e..fc42ca2 100644 --- a/tests/test_graphql/test_backup.py +++ b/tests/test_graphql/test_backup.py @@ -39,14 +39,34 @@ TESTFILE_2_BODY = "testissimo!" REPO_NAME = "test_backup" -@pytest.fixture(scope="function") -def backups(tmpdir): - Backups.reset() - - test_repo_path = path.join(tmpdir, "totallyunrelated") +def prepare_localfile_backups(temp_dir): + test_repo_path = path.join(temp_dir, "totallyunrelated") + assert not path.exists(test_repo_path) Backups.set_localfile_repo(test_repo_path) + +@pytest.fixture(scope="function") +def backups_local(tmpdir): + Backups.reset() + prepare_localfile_backups(tmpdir) Jobs.reset() + Backups.init_repo() + + +@pytest.fixture(scope="function") +def backups(tmpdir): + # for those tests that are supposed to pass with any repo + Backups.reset() + if BACKUP_PROVIDER_ENVS["kind"] in os.environ.keys(): + Backups.set_provider_from_envs() + else: + prepare_localfile_backups(tmpdir) + Jobs.reset() + # assert not repo_path + + Backups.init_repo() + yield + Backups.forget_all_snapshots() @pytest.fixture() @@ -82,11 +102,6 @@ def raw_dummy_service(tmpdir): @pytest.fixture() def dummy_service(tmpdir, backups, raw_dummy_service) -> Service: service = raw_dummy_service - repo_path = path.join(tmpdir, "test_repo") - assert not path.exists(repo_path) - # assert not repo_path - - Backups.init_repo() # register our service services.services.append(service) @@ -148,6 +163,12 @@ def test_reset_sets_to_none2(backups): def test_setting_from_envs(tmpdir): Backups.reset() + environment_stash = {} + if BACKUP_PROVIDER_ENVS["kind"] in os.environ.keys(): + # we are running under special envs, stash them before rewriting them + for key in BACKUP_PROVIDER_ENVS.values(): + environment_stash[key] = os.environ[key] + os.environ[BACKUP_PROVIDER_ENVS["kind"]] = "BACKBLAZE" os.environ[BACKUP_PROVIDER_ENVS["login"]] = "ID" os.environ[BACKUP_PROVIDER_ENVS["key"]] = "KEY" @@ -164,6 +185,13 @@ def test_setting_from_envs(tmpdir): assert provider.backupper.account == "ID" assert provider.backupper.key == "KEY" + if environment_stash != {}: + for key in BACKUP_PROVIDER_ENVS.values(): + os.environ[key] = environment_stash[key] + else: + for key in BACKUP_PROVIDER_ENVS.values(): + del os.environ[key] + def test_json_reset(generic_userdata): Backups.reset(reset_json=False) @@ -294,9 +322,12 @@ def test_sizing(backups, dummy_service): assert size > 0 -def test_init_tracking(backups, raw_dummy_service): +def test_init_tracking(backups, tmpdir): + assert Backups.is_initted() is True + Backups.reset() assert Backups.is_initted() is False - + separate_dir = tmpdir / "out_of_the_way" + prepare_localfile_backups(separate_dir) Backups.init_repo() assert Backups.is_initted() is True @@ -618,6 +649,8 @@ def test_snapshots_cache_invalidation(backups, dummy_service): # Storage def test_init_tracking_caching(backups, raw_dummy_service): + assert Storage.has_init_mark() is True + Backups.reset() assert Storage.has_init_mark() is False Storage.mark_as_init() @@ -627,7 +660,12 @@ def test_init_tracking_caching(backups, raw_dummy_service): # Storage -def test_init_tracking_caching2(backups, raw_dummy_service): +def test_init_tracking_caching2(backups, tmpdir): + assert Storage.has_init_mark() is True + Backups.reset() + assert Storage.has_init_mark() is False + separate_dir = tmpdir / "out_of_the_way" + prepare_localfile_backups(separate_dir) assert Storage.has_init_mark() is False Backups.init_repo() From cfa7f4ae59b23a279efa597d05453fcf11ac67bf Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Wed, 26 Jul 2023 16:45:08 +0000 Subject: [PATCH 04/27] feature(backups): add full repo erasure capability --- selfprivacy_api/backup/__init__.py | 8 ++++ selfprivacy_api/backup/backuppers/__init__.py | 5 +++ .../backup/backuppers/none_backupper.py | 4 ++ .../backup/backuppers/restic_backupper.py | 44 +++++++++++++++---- selfprivacy_api/backup/storage.py | 13 ++++-- tests/test_graphql/test_backup.py | 13 ++++++ 6 files changed, 74 insertions(+), 13 deletions(-) diff --git a/selfprivacy_api/backup/__init__.py b/selfprivacy_api/backup/__init__.py index 725904e..c28c01f 100644 --- a/selfprivacy_api/backup/__init__.py +++ b/selfprivacy_api/backup/__init__.py @@ -236,6 +236,14 @@ class Backups: Backups.provider().backupper.init() Storage.mark_as_init() + @staticmethod + def erase_repo() -> None: + """ + Completely empties the remote + """ + Backups.provider().backupper.erase_repo() + Storage.mark_as_uninitted() + @staticmethod def is_initted() -> bool: """ diff --git a/selfprivacy_api/backup/backuppers/__init__.py b/selfprivacy_api/backup/backuppers/__init__.py index ea2350b..ccf78b9 100644 --- a/selfprivacy_api/backup/backuppers/__init__.py +++ b/selfprivacy_api/backup/backuppers/__init__.py @@ -36,6 +36,11 @@ class AbstractBackupper(ABC): """Initialize the repository""" raise NotImplementedError + @abstractmethod + def erase_repo(self) -> None: + """Completely empties the remote""" + raise NotImplementedError + @abstractmethod def restore_from_backup( self, diff --git a/selfprivacy_api/backup/backuppers/none_backupper.py b/selfprivacy_api/backup/backuppers/none_backupper.py index d9edaeb..87e43c5 100644 --- a/selfprivacy_api/backup/backuppers/none_backupper.py +++ b/selfprivacy_api/backup/backuppers/none_backupper.py @@ -23,6 +23,10 @@ class NoneBackupper(AbstractBackupper): def init(self): raise NotImplementedError + def erase_repo(self) -> None: + """Completely empties the remote""" + raise NotImplementedError + def restore_from_backup(self, snapshot_id: str, folders: List[str], verify=True): """Restore a target folder using a snapshot""" raise NotImplementedError diff --git a/selfprivacy_api/backup/backuppers/restic_backupper.py b/selfprivacy_api/backup/backuppers/restic_backupper.py index e98c4c3..816bebf 100644 --- a/selfprivacy_api/backup/backuppers/restic_backupper.py +++ b/selfprivacy_api/backup/backuppers/restic_backupper.py @@ -40,20 +40,25 @@ class ResticBackupper(AbstractBackupper): def restic_repo(self) -> str: # https://restic.readthedocs.io/en/latest/030_preparing_a_new_repo.html#other-services-via-rclone # https://forum.rclone.org/t/can-rclone-be-run-solely-with-command-line-options-no-config-no-env-vars/6314/5 - return f"rclone:{self.storage_type}{self.repo}" + return f"rclone:{self.rclone_repo()}" + + def rclone_repo(self) -> str: + return f"{self.storage_type}{self.repo}" def rclone_args(self): - return "rclone.args=serve restic --stdio " + self.backend_rclone_args() + return "rclone.args=serve restic --stdio " + " ".join( + self.backend_rclone_args() + ) - def backend_rclone_args(self) -> str: - acc_arg = "" - key_arg = "" + def backend_rclone_args(self) -> list[str]: + args = [] if self.account != "": - acc_arg = f"{self.login_flag} {self.account}" + acc_args = [self.login_flag, self.account] + args.extend(acc_args) if self.key != "": - key_arg = f"{self.key_flag} {self.key}" - - return f"{acc_arg} {key_arg}" + key_args = [self.key_flag, self.key] + args.extend(key_args) + return args def _password_command(self): return f"echo {LocalBackupSecret.get()}" @@ -79,6 +84,27 @@ class ResticBackupper(AbstractBackupper): command.extend(ResticBackupper.__flatten_list(args)) return command + def erase_repo(self) -> None: + """Fully erases repo on remote, can be reinitted again""" + command = [ + "rclone", + "purge", + self.rclone_repo(), + ] + backend_args = self.backend_rclone_args() + if backend_args: + command.extend(backend_args) + + with subprocess.Popen(command, stdout=subprocess.PIPE, shell=False) as handle: + output = handle.communicate()[0].decode("utf-8") + if handle.returncode != 0: + raise ValueError( + "purge exited with errorcode", + handle.returncode, + ":", + output, + ) + def mount_repo(self, mount_directory): mount_command = self.restic_command("mount", mount_directory) mount_command.insert(0, "nohup") diff --git a/selfprivacy_api/backup/storage.py b/selfprivacy_api/backup/storage.py index f7384a0..d46f584 100644 --- a/selfprivacy_api/backup/storage.py +++ b/selfprivacy_api/backup/storage.py @@ -21,7 +21,7 @@ REDIS_SNAPSHOT_CACHE_EXPIRE_SECONDS = 24 * 60 * 60 # one day REDIS_SNAPSHOTS_PREFIX = "backups:snapshots:" REDIS_LAST_BACKUP_PREFIX = "backups:last-backed-up:" -REDIS_INITTED_CACHE_PREFIX = "backups:initted_services:" +REDIS_INITTED_CACHE = "backups:repo_initted" REDIS_PROVIDER_KEY = "backups:provider" REDIS_AUTOBACKUP_PERIOD_KEY = "backups:autobackup_period" @@ -38,9 +38,9 @@ class Storage: """Deletes all backup related data from redis""" redis.delete(REDIS_PROVIDER_KEY) redis.delete(REDIS_AUTOBACKUP_PERIOD_KEY) + redis.delete(REDIS_INITTED_CACHE) prefixes_to_clean = [ - REDIS_INITTED_CACHE_PREFIX, REDIS_SNAPSHOTS_PREFIX, REDIS_LAST_BACKUP_PREFIX, ] @@ -162,11 +162,16 @@ class Storage: @staticmethod def has_init_mark() -> bool: """Returns True if the repository was initialized""" - if redis.exists(REDIS_INITTED_CACHE_PREFIX): + if redis.exists(REDIS_INITTED_CACHE): return True return False @staticmethod def mark_as_init(): """Marks the repository as initialized""" - redis.set(REDIS_INITTED_CACHE_PREFIX, 1) + redis.set(REDIS_INITTED_CACHE, 1) + + @staticmethod + def mark_as_uninitted(): + """Marks the repository as initialized""" + redis.delete(REDIS_INITTED_CACHE) diff --git a/tests/test_graphql/test_backup.py b/tests/test_graphql/test_backup.py index fc42ca2..e85d1de 100644 --- a/tests/test_graphql/test_backup.py +++ b/tests/test_graphql/test_backup.py @@ -222,6 +222,19 @@ def test_file_backend_init(file_backup): file_backup.backupper.init() +def test_reinit_after_purge(backups): + assert Backups.is_initted() is True + + Backups.erase_repo() + assert Backups.is_initted() is False + with pytest.raises(ValueError): + Backups.get_all_snapshots() + + Backups.init_repo() + assert Backups.is_initted() is True + assert len(Backups.get_all_snapshots()) == 0 + + def test_backup_simple_file(raw_dummy_service, file_backup): # temporarily incomplete service = raw_dummy_service From 00317cc7e4f0fcab6f385ad1be8bf3830211f30b Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Wed, 26 Jul 2023 16:52:58 +0000 Subject: [PATCH 05/27] test(backups): erase repos between tests --- selfprivacy_api/backup/backuppers/none_backupper.py | 3 ++- tests/test_graphql/test_backup.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/selfprivacy_api/backup/backuppers/none_backupper.py b/selfprivacy_api/backup/backuppers/none_backupper.py index 87e43c5..3f9f7fd 100644 --- a/selfprivacy_api/backup/backuppers/none_backupper.py +++ b/selfprivacy_api/backup/backuppers/none_backupper.py @@ -25,7 +25,8 @@ class NoneBackupper(AbstractBackupper): def erase_repo(self) -> None: """Completely empties the remote""" - raise NotImplementedError + # this one is already empty + pass def restore_from_backup(self, snapshot_id: str, folders: List[str], verify=True): """Restore a target folder using a snapshot""" diff --git a/tests/test_graphql/test_backup.py b/tests/test_graphql/test_backup.py index e85d1de..da81c60 100644 --- a/tests/test_graphql/test_backup.py +++ b/tests/test_graphql/test_backup.py @@ -66,7 +66,7 @@ def backups(tmpdir): Backups.init_repo() yield - Backups.forget_all_snapshots() + Backups.erase_repo() @pytest.fixture() From bba837530afe2d776f4620b84473ea1d67c9b2ce Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Fri, 28 Jul 2023 10:40:40 +0000 Subject: [PATCH 06/27] feature(backups): expose forget to API --- .../graphql/mutations/backup_mutations.py | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/selfprivacy_api/graphql/mutations/backup_mutations.py b/selfprivacy_api/graphql/mutations/backup_mutations.py index b92af4a..f6dc282 100644 --- a/selfprivacy_api/graphql/mutations/backup_mutations.py +++ b/selfprivacy_api/graphql/mutations/backup_mutations.py @@ -157,6 +157,35 @@ class BackupMutations: job=job_to_api_job(job), ) + @strawberry.mutation(permission_classes=[IsAuthenticated]) + def forget_snapshot(self, snapshot_id: str) -> GenericMutationReturn: + """Forget a snapshot. + Makes it inaccessible from the server. + After some time, the data (encrypted) will not be recoverable + from the backup server too, but not immediately""" + + snap = Backups.get_snapshot_by_id(snapshot_id) + if snap is None: + return GenericMutationReturn( + success=False, + code=404, + message=f"snapshot {snapshot_id} not found", + ) + + try: + Backups.forget_snapshot(snap) + return GenericMutationReturn( + success=True, + code=200, + message="", + ) + except Exception as error: + return GenericMutationReturn( + success=False, + code=400, + message=str(error), + ) + @strawberry.mutation(permission_classes=[IsAuthenticated]) def force_snapshots_reload(self) -> GenericMutationReturn: """Force snapshots reload""" From 2934e2becac0d7f86ad79736e6d002ecc7f62941 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Fri, 28 Jul 2023 11:32:48 +0000 Subject: [PATCH 07/27] test(backups): test forgetting via API --- tests/test_graphql/test_api_backup.py | 50 +++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tests/test_graphql/test_api_backup.py b/tests/test_graphql/test_api_backup.py index bfa315b..e53ce2a 100644 --- a/tests/test_graphql/test_api_backup.py +++ b/tests/test_graphql/test_api_backup.py @@ -94,6 +94,18 @@ mutation TestRestoreService($snapshot_id: String!) { } """ +API_FORGET_MUTATION = """ +mutation TestForgetSnapshot($snapshot_id: String!) { + backup { + forgetSnapshot(snapshotId: $snapshot_id) { + success + message + code + } + } +} +""" + API_SNAPSHOTS_QUERY = """ allSnapshots { id @@ -143,6 +155,17 @@ def api_backup(authorized_client, service): return response +def api_forget(authorized_client, snapshot_id): + response = authorized_client.post( + "/graphql", + json={ + "query": API_FORGET_MUTATION, + "variables": {"snapshot_id": snapshot_id}, + }, + ) + return response + + def api_set_period(authorized_client, period): response = authorized_client.post( "/graphql", @@ -370,3 +393,30 @@ def test_reload_snapshots(authorized_client, dummy_service): snaps = api_snapshots(authorized_client) assert len(snaps) == 1 + + +def test_forget_snapshot(authorized_client, dummy_service): + response = api_backup(authorized_client, dummy_service) + data = get_data(response)["backup"]["startBackup"] + + snaps = api_snapshots(authorized_client) + assert len(snaps) == 1 + + response = api_forget(authorized_client, snaps[0]["id"]) + data = get_data(response)["backup"]["forgetSnapshot"] + assert_ok(data) + + snaps = api_snapshots(authorized_client) + assert len(snaps) == 0 + + +def test_forget_nonexistent_snapshot(authorized_client, dummy_service): + snaps = api_snapshots(authorized_client) + assert len(snaps) == 0 + response = api_forget(authorized_client, "898798uekiodpjoiweoiwuoeirueor") + data = get_data(response)["backup"]["forgetSnapshot"] + assert data["code"] == 404 + assert data["success"] is False + + snaps = api_snapshots(authorized_client) + assert len(snaps) == 0 From ff70a3588e6a8b447d87341eb576851e1f213043 Mon Sep 17 00:00:00 2001 From: Inex Code Date: Fri, 4 Aug 2023 12:57:31 +0300 Subject: [PATCH 08/27] chore: bump version --- selfprivacy_api/dependencies.py | 2 +- selfprivacy_api/graphql/mutations/backup_mutations.py | 6 +++--- setup.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/selfprivacy_api/dependencies.py b/selfprivacy_api/dependencies.py index 6f6f5a5..fb974e8 100644 --- a/selfprivacy_api/dependencies.py +++ b/selfprivacy_api/dependencies.py @@ -27,4 +27,4 @@ async def get_token_header( def get_api_version() -> str: """Get API version""" - return "2.2.1" + return "2.3.0" diff --git a/selfprivacy_api/graphql/mutations/backup_mutations.py b/selfprivacy_api/graphql/mutations/backup_mutations.py index f6dc282..c022d57 100644 --- a/selfprivacy_api/graphql/mutations/backup_mutations.py +++ b/selfprivacy_api/graphql/mutations/backup_mutations.py @@ -159,8 +159,8 @@ class BackupMutations: @strawberry.mutation(permission_classes=[IsAuthenticated]) def forget_snapshot(self, snapshot_id: str) -> GenericMutationReturn: - """Forget a snapshot. - Makes it inaccessible from the server. + """Forget a snapshot. + Makes it inaccessible from the server. After some time, the data (encrypted) will not be recoverable from the backup server too, but not immediately""" @@ -171,7 +171,7 @@ class BackupMutations: code=404, message=f"snapshot {snapshot_id} not found", ) - + try: Backups.forget_snapshot(snap) return GenericMutationReturn( diff --git a/setup.py b/setup.py index dea4568..684f54f 100755 --- a/setup.py +++ b/setup.py @@ -2,7 +2,7 @@ from setuptools import setup, find_packages setup( name="selfprivacy_api", - version="2.2.1", + version="2.3.0", packages=find_packages(), scripts=[ "selfprivacy_api/app.py", From 52336b885dfc804e93d4df4e3ad4fa0498d045d7 Mon Sep 17 00:00:00 2001 From: Inex Code Date: Fri, 4 Aug 2023 14:08:23 +0300 Subject: [PATCH 09/27] fix: check if repo is initted by checking retcode --- selfprivacy_api/backup/backuppers/restic_backupper.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/selfprivacy_api/backup/backuppers/restic_backupper.py b/selfprivacy_api/backup/backuppers/restic_backupper.py index 816bebf..37ae06b 100644 --- a/selfprivacy_api/backup/backuppers/restic_backupper.py +++ b/selfprivacy_api/backup/backuppers/restic_backupper.py @@ -228,8 +228,7 @@ class ResticBackupper(AbstractBackupper): def is_initted(self) -> bool: command = self.restic_command( - "check", - "--json", + "unlock", ) with subprocess.Popen( @@ -237,10 +236,8 @@ class ResticBackupper(AbstractBackupper): stdout=subprocess.PIPE, shell=False, ) as handle: - output = handle.communicate()[0].decode("utf-8") - if not ResticBackupper.has_json(output): + if handle.returncode != 0: return False - # raise NotImplementedError("error(big): " + output) return True def restored_size(self, snapshot_id: str) -> int: From 752a0b807e7cc0133f4818f6c11ff4df7761a855 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 7 Aug 2023 13:33:10 +0000 Subject: [PATCH 10/27] feature(backups): lock and unlock at will --- .../backup/backuppers/restic_backupper.py | 53 ++++++++++++++++++- selfprivacy_api/backup/util.py | 16 ++++-- tests/test_graphql/test_backup.py | 15 ++++++ 3 files changed, 78 insertions(+), 6 deletions(-) diff --git a/selfprivacy_api/backup/backuppers/restic_backupper.py b/selfprivacy_api/backup/backuppers/restic_backupper.py index 37ae06b..a359f98 100644 --- a/selfprivacy_api/backup/backuppers/restic_backupper.py +++ b/selfprivacy_api/backup/backuppers/restic_backupper.py @@ -227,6 +227,24 @@ class ResticBackupper(AbstractBackupper): raise ValueError("cannot init a repo: " + output) def is_initted(self) -> bool: + command = self.restic_command( + "check", + ) + + with subprocess.Popen( + command, + stdout=subprocess.PIPE, + shell=False, + stderr=subprocess.STDOUT, + ) as handle: + # communication forces to complete and for returncode to get defined + output = handle.communicate()[0].decode("utf-8") + if handle.returncode != 0: + return False + return True + + def unlock(self) -> None: + """Remove stale locks.""" command = self.restic_command( "unlock", ) @@ -235,10 +253,41 @@ class ResticBackupper(AbstractBackupper): command, stdout=subprocess.PIPE, shell=False, + stderr=subprocess.STDOUT, ) as handle: + # communication forces to complete and for returncode to get defined + output = handle.communicate()[0].decode("utf-8") if handle.returncode != 0: - return False - return True + raise ValueError("cannot unlock the backup repository: ", output) + + def lock(self) -> None: + """ + Introduce a stale lock. + Mainly for testing purposes. + Double lock is supposed to fail + """ + command = self.restic_command( + "check", + ) + + # using temporary cache in /run/user/1000/restic-check-cache-817079729 + # repository 9639c714 opened (repository version 2) successfully, password is correct + # created new cache in /run/user/1000/restic-check-cache-817079729 + # create exclusive lock for repository + # load indexes + # check all packs + # check snapshots, trees and blobs + # [0:00] 100.00% 1 / 1 snapshots + # no errors were found + + try: + for line in output_yielder(command): + if "indexes" in line: + break + if "unable" in line: + raise ValueError(line) + except Exception as e: + raise ValueError("could not lock repository") from e def restored_size(self, snapshot_id: str) -> int: """ diff --git a/selfprivacy_api/backup/util.py b/selfprivacy_api/backup/util.py index bda421e..41d926c 100644 --- a/selfprivacy_api/backup/util.py +++ b/selfprivacy_api/backup/util.py @@ -1,8 +1,10 @@ import subprocess from os.path import exists +from typing import Generator -def output_yielder(command): +def output_yielder(command) -> Generator[str, None, None]: + """Note: If you break during iteration, it kills the process""" with subprocess.Popen( command, shell=False, @@ -10,9 +12,15 @@ def output_yielder(command): stderr=subprocess.STDOUT, universal_newlines=True, ) as handle: - for line in iter(handle.stdout.readline, ""): - if "NOTICE:" not in line: - yield line + if handle is None or handle.stdout is None: + raise ValueError("could not run command: ", command) + + try: + for line in iter(handle.stdout.readline, ""): + if "NOTICE:" not in line: + yield line + except GeneratorExit: + handle.kill() def sync(src_path: str, dest_path: str): diff --git a/tests/test_graphql/test_backup.py b/tests/test_graphql/test_backup.py index da81c60..9743567 100644 --- a/tests/test_graphql/test_backup.py +++ b/tests/test_graphql/test_backup.py @@ -758,3 +758,18 @@ def test_move_blocks_backups(backups, dummy_service, restore_strategy): with pytest.raises(ValueError): Backups.restore_snapshot(snap, restore_strategy) + + +def test_double_lock_unlock(backups, dummy_service): + # notice that introducing stale locks is only safe for other tests if we erase repo in between + # which we do at the time of writing this test + + Backups.provider().backupper.lock() + with pytest.raises(ValueError): + Backups.provider().backupper.lock() + + Backups.provider().backupper.unlock() + Backups.provider().backupper.lock() + + Backups.provider().backupper.unlock() + Backups.provider().backupper.unlock() From eca4b26a3171c018cb5a658f6eb996772726b5be Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Wed, 9 Aug 2023 13:47:18 +0000 Subject: [PATCH 11/27] fix(backups): robustness against stale locks: backing up --- .../backup/backuppers/restic_backupper.py | 34 +++++++++++++++++-- tests/test_graphql/test_backup.py | 6 ++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/selfprivacy_api/backup/backuppers/restic_backupper.py b/selfprivacy_api/backup/backuppers/restic_backupper.py index a359f98..6c3dbcc 100644 --- a/selfprivacy_api/backup/backuppers/restic_backupper.py +++ b/selfprivacy_api/backup/backuppers/restic_backupper.py @@ -1,9 +1,11 @@ +from __future__ import annotations + import subprocess import json import datetime import tempfile -from typing import List +from typing import List, TypeVar, Callable from collections.abc import Iterable from json.decoder import JSONDecodeError from os.path import exists, join @@ -21,6 +23,25 @@ from selfprivacy_api.backup.local_secret import LocalBackupSecret SHORT_ID_LEN = 8 +T = TypeVar("T", bound=Callable) + + +def unlocked_repo(func: T) -> T: + """unlock repo and retry if it appears to be locked""" + + def inner(self: ResticBackupper, *args, **kwargs): + try: + return func(self, *args, **kwargs) + except Exception as e: + if "unable to create lock" in str(e): + self.unlock() + return func(self, *args, **kwargs) + else: + raise e + + # Above, we manually guarantee that the type returned is compatible. + return inner # type: ignore + class ResticBackupper(AbstractBackupper): def __init__(self, login_flag: str, key_flag: str, storage_type: str) -> None: @@ -142,6 +163,7 @@ class ResticBackupper(AbstractBackupper): result.append(item) return result + @unlocked_repo def start_backup(self, folders: List[str], tag: str) -> Snapshot: """ Start backup with restic @@ -165,8 +187,10 @@ class ResticBackupper(AbstractBackupper): raise ValueError("No service with id ", tag) job = get_backup_job(service) + output = [] try: for raw_message in output_yielder(backup_command): + output.append(raw_message) message = self.parse_message( raw_message, job, @@ -177,7 +201,13 @@ class ResticBackupper(AbstractBackupper): tag, ) except ValueError as error: - raise ValueError("Could not create a snapshot: ", messages) from error + raise ValueError( + "Could not create a snapshot: ", + str(error), + output, + "parsed messages:", + messages, + ) from error @staticmethod def _snapshot_from_backup_messages(messages, repo_name) -> Snapshot: diff --git a/tests/test_graphql/test_backup.py b/tests/test_graphql/test_backup.py index 9743567..b575b5b 100644 --- a/tests/test_graphql/test_backup.py +++ b/tests/test_graphql/test_backup.py @@ -773,3 +773,9 @@ def test_double_lock_unlock(backups, dummy_service): Backups.provider().backupper.unlock() Backups.provider().backupper.unlock() + + +def test_operations_while_locked(backups, dummy_service): + Backups.provider().backupper.lock() + snap = Backups.back_up(dummy_service) + assert snap is not None From 26ab7b4d7b9b9f7670a5bb9d085044e664160200 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Wed, 9 Aug 2023 13:58:53 +0000 Subject: [PATCH 12/27] fix(backups): robustness against stale locks: is_initted --- .../backup/backuppers/restic_backupper.py | 5 ++++- tests/test_graphql/test_backup.py | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/selfprivacy_api/backup/backuppers/restic_backupper.py b/selfprivacy_api/backup/backuppers/restic_backupper.py index 6c3dbcc..022bda7 100644 --- a/selfprivacy_api/backup/backuppers/restic_backupper.py +++ b/selfprivacy_api/backup/backuppers/restic_backupper.py @@ -256,6 +256,7 @@ class ResticBackupper(AbstractBackupper): if "created restic repository" not in output: raise ValueError("cannot init a repo: " + output) + @unlocked_repo def is_initted(self) -> bool: command = self.restic_command( "check", @@ -267,9 +268,10 @@ class ResticBackupper(AbstractBackupper): shell=False, stderr=subprocess.STDOUT, ) as handle: - # communication forces to complete and for returncode to get defined output = handle.communicate()[0].decode("utf-8") if handle.returncode != 0: + if "unable to create lock" in output: + raise ValueError("Stale lock detected: ", output) return False return True @@ -319,6 +321,7 @@ class ResticBackupper(AbstractBackupper): except Exception as e: raise ValueError("could not lock repository") from e + @unlocked_repo def restored_size(self, snapshot_id: str) -> int: """ Size of a snapshot diff --git a/tests/test_graphql/test_backup.py b/tests/test_graphql/test_backup.py index b575b5b..da4da7a 100644 --- a/tests/test_graphql/test_backup.py +++ b/tests/test_graphql/test_backup.py @@ -776,6 +776,21 @@ def test_double_lock_unlock(backups, dummy_service): def test_operations_while_locked(backups, dummy_service): + # Stale lock prevention test + + # consider making it fully at the level of backupper? + # because this is where prevention lives? + # Backups singleton is here only so that we can run this against B2, S3 and whatever + # But maybe it is not necessary (if restic treats them uniformly enough) + Backups.provider().backupper.lock() snap = Backups.back_up(dummy_service) assert snap is not None + + Backups.provider().backupper.lock() + # using lowlevel to make sure no caching interferes + assert Backups.provider().backupper.is_initted() is True + + # check that no locks were left + Backups.provider().backupper.lock() + Backups.provider().backupper.unlock() From 0eb70e1551f530853ea9a99b41a9d37efa033768 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Wed, 9 Aug 2023 14:46:27 +0000 Subject: [PATCH 13/27] fix(backups): robustness against stale locks: snapshot sizing --- selfprivacy_api/backup/backuppers/restic_backupper.py | 6 +++++- tests/test_graphql/test_backup.py | 3 +++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/selfprivacy_api/backup/backuppers/restic_backupper.py b/selfprivacy_api/backup/backuppers/restic_backupper.py index 022bda7..5db9f11 100644 --- a/selfprivacy_api/backup/backuppers/restic_backupper.py +++ b/selfprivacy_api/backup/backuppers/restic_backupper.py @@ -335,6 +335,7 @@ class ResticBackupper(AbstractBackupper): with subprocess.Popen( command, stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, shell=False, ) as handle: output = handle.communicate()[0].decode("utf-8") @@ -382,7 +383,10 @@ class ResticBackupper(AbstractBackupper): ) with subprocess.Popen( - restore_command, stdout=subprocess.PIPE, shell=False + restore_command, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + shell=False, ) as handle: # for some reason restore does not support diff --git a/tests/test_graphql/test_backup.py b/tests/test_graphql/test_backup.py index da4da7a..556b72b 100644 --- a/tests/test_graphql/test_backup.py +++ b/tests/test_graphql/test_backup.py @@ -791,6 +791,9 @@ def test_operations_while_locked(backups, dummy_service): # using lowlevel to make sure no caching interferes assert Backups.provider().backupper.is_initted() is True + Backups.provider().backupper.lock() + assert Backups.snapshot_restored_size(snap.id) > 0 + # check that no locks were left Backups.provider().backupper.lock() Backups.provider().backupper.unlock() From 2c9011cc87f5fb3c337627133d9cb6ac00bce56e Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Wed, 9 Aug 2023 15:18:20 +0000 Subject: [PATCH 14/27] fix(backups): robustness against stale locks: everything else --- selfprivacy_api/backup/backuppers/restic_backupper.py | 10 +++++----- tests/test_graphql/test_backup.py | 9 +++++++++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/selfprivacy_api/backup/backuppers/restic_backupper.py b/selfprivacy_api/backup/backuppers/restic_backupper.py index 5db9f11..3a5fc49 100644 --- a/selfprivacy_api/backup/backuppers/restic_backupper.py +++ b/selfprivacy_api/backup/backuppers/restic_backupper.py @@ -345,6 +345,7 @@ class ResticBackupper(AbstractBackupper): except ValueError as error: raise ValueError("cannot restore a snapshot: " + output) from error + @unlocked_repo def restore_from_backup( self, snapshot_id, @@ -406,6 +407,7 @@ class ResticBackupper(AbstractBackupper): output, ) + @unlocked_repo def forget_snapshot(self, snapshot_id) -> None: """ Either removes snapshot or marks it for deletion later, @@ -441,10 +443,7 @@ class ResticBackupper(AbstractBackupper): ) # none should be impossible after communicate if handle.returncode != 0: raise ValueError( - "forget exited with errorcode", - handle.returncode, - ":", - output, + "forget exited with errorcode", handle.returncode, ":", output, err ) def _load_snapshots(self) -> object: @@ -470,8 +469,9 @@ class ResticBackupper(AbstractBackupper): try: return ResticBackupper.parse_json_output(output) except ValueError as error: - raise ValueError("Cannot load snapshots: ") from error + raise ValueError("Cannot load snapshots: ", output) from error + @unlocked_repo def get_snapshots(self) -> List[Snapshot]: """Get all snapshots from the repo""" snapshots = [] diff --git a/tests/test_graphql/test_backup.py b/tests/test_graphql/test_backup.py index 556b72b..1990ef7 100644 --- a/tests/test_graphql/test_backup.py +++ b/tests/test_graphql/test_backup.py @@ -794,6 +794,15 @@ def test_operations_while_locked(backups, dummy_service): Backups.provider().backupper.lock() assert Backups.snapshot_restored_size(snap.id) > 0 + Backups.provider().backupper.lock() + Backups.restore_snapshot(snap) + + Backups.provider().backupper.lock() + Backups.forget_snapshot(snap) + + Backups.provider().backupper.lock() + assert Backups.provider().backupper.get_snapshots() == [] + # check that no locks were left Backups.provider().backupper.lock() Backups.provider().backupper.unlock() From 69f6e62877dcaafed3dec109767cbcd52c1d1ca7 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 14 Aug 2023 11:50:59 +0000 Subject: [PATCH 15/27] test(backups): more checks regarding tmpdirs and mounting --- .../backup/backuppers/restic_backupper.py | 13 +++++++++---- tests/test_graphql/test_backup.py | 9 +++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/selfprivacy_api/backup/backuppers/restic_backupper.py b/selfprivacy_api/backup/backuppers/restic_backupper.py index 3a5fc49..418aa5b 100644 --- a/selfprivacy_api/backup/backuppers/restic_backupper.py +++ b/selfprivacy_api/backup/backuppers/restic_backupper.py @@ -126,7 +126,9 @@ class ResticBackupper(AbstractBackupper): output, ) - def mount_repo(self, mount_directory): + def mount_repo(self, mount_directory: str) -> subprocess.Popen: + if not exists(mount_directory): + raise FileNotFoundError("no such directory to mount at: ", mount_directory) mount_command = self.restic_command("mount", mount_directory) mount_command.insert(0, "nohup") handle = subprocess.Popen( @@ -139,7 +141,7 @@ class ResticBackupper(AbstractBackupper): raise IOError("failed to mount dir ", mount_directory) return handle - def unmount_repo(self, mount_directory): + def unmount_repo(self, mount_directory: str) -> None: mount_command = ["umount", "-l", mount_directory] with subprocess.Popen( mount_command, stdout=subprocess.PIPE, shell=False @@ -147,10 +149,10 @@ class ResticBackupper(AbstractBackupper): output = handle.communicate()[0].decode("utf-8") # TODO: check for exit code? if "error" in output.lower(): - return IOError("failed to unmount dir ", mount_directory, ": ", output) + raise IOError("failed to unmount dir ", mount_directory, ": ", output) if not listdir(mount_directory) == []: - return IOError("failed to unmount dir ", mount_directory) + raise IOError("failed to unmount dir ", mount_directory) @staticmethod def __flatten_list(list_to_flatten): @@ -363,6 +365,9 @@ class ResticBackupper(AbstractBackupper): self._raw_verified_restore(snapshot_id, target=temp_dir) snapshot_root = temp_dir else: # attempting inplace restore via mount + sync + assert exists( + temp_dir + ) # paranoid check, TemporaryDirectory is supposedly created self.mount_repo(temp_dir) snapshot_root = join(temp_dir, "ids", snapshot_id) diff --git a/tests/test_graphql/test_backup.py b/tests/test_graphql/test_backup.py index 1990ef7..9736f91 100644 --- a/tests/test_graphql/test_backup.py +++ b/tests/test_graphql/test_backup.py @@ -8,6 +8,8 @@ from os import urandom from datetime import datetime, timedelta, timezone from subprocess import Popen +import tempfile + import selfprivacy_api.services as services from selfprivacy_api.services import Service, get_all_services @@ -806,3 +808,10 @@ def test_operations_while_locked(backups, dummy_service): # check that no locks were left Backups.provider().backupper.lock() Backups.provider().backupper.unlock() + + +# a paranoid check to weed out problems with tempdirs that are not dependent on us +def test_tempfile(): + with tempfile.TemporaryDirectory() as temp: + assert path.exists(temp) + assert not path.exists(temp) From c89f9cf89d06aeee6997e80b0376dd50a86e373b Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 14 Aug 2023 12:43:44 +0000 Subject: [PATCH 16/27] feature(backups): do not rely on mounting --- .../backup/backuppers/restic_backupper.py | 33 +++++++++---------- tests/test_graphql/test_backup.py | 11 +++++++ 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/selfprivacy_api/backup/backuppers/restic_backupper.py b/selfprivacy_api/backup/backuppers/restic_backupper.py index 418aa5b..a935520 100644 --- a/selfprivacy_api/backup/backuppers/restic_backupper.py +++ b/selfprivacy_api/backup/backuppers/restic_backupper.py @@ -9,8 +9,9 @@ from typing import List, TypeVar, Callable from collections.abc import Iterable from json.decoder import JSONDecodeError from os.path import exists, join -from os import listdir +from os import listdir, mkdir from time import sleep +from shutil import rmtree from selfprivacy_api.backup.util import output_yielder, sync from selfprivacy_api.backup.backuppers import AbstractBackupper @@ -364,23 +365,21 @@ class ResticBackupper(AbstractBackupper): if verify: self._raw_verified_restore(snapshot_id, target=temp_dir) snapshot_root = temp_dir - else: # attempting inplace restore via mount + sync - assert exists( - temp_dir - ) # paranoid check, TemporaryDirectory is supposedly created - self.mount_repo(temp_dir) - snapshot_root = join(temp_dir, "ids", snapshot_id) + for folder in folders: + src = join(snapshot_root, folder.strip("/")) + if not exists(src): + raise ValueError( + f"No such path: {src}. We tried to find {folder}" + ) + dst = folder + sync(src, dst) - assert snapshot_root is not None - for folder in folders: - src = join(snapshot_root, folder.strip("/")) - if not exists(src): - raise ValueError(f"No such path: {src}. We tried to find {folder}") - dst = folder - sync(src, dst) - - if not verify: - self.unmount_repo(temp_dir) + else: # attempting inplace restore + for folder in folders: + rmtree(folder) + mkdir(folder) + self._raw_verified_restore(snapshot_id, target="/") + return def _raw_verified_restore(self, snapshot_id, target="/"): """barebones restic restore""" diff --git a/tests/test_graphql/test_backup.py b/tests/test_graphql/test_backup.py index 9736f91..25eaaf4 100644 --- a/tests/test_graphql/test_backup.py +++ b/tests/test_graphql/test_backup.py @@ -746,6 +746,17 @@ def test_mount_umount(backups, dummy_service, tmpdir): assert len(listdir(mountpoint)) == 0 +def test_mount_nonexistent(backups, dummy_service, tmpdir): + backupper = Backups.provider().backupper + assert isinstance(backupper, ResticBackupper) + + mountpoint = tmpdir / "nonexistent" + assert not path.exists(mountpoint) + + with pytest.raises(FileNotFoundError): + handle = backupper.mount_repo(mountpoint) + + def test_move_blocks_backups(backups, dummy_service, restore_strategy): snap = Backups.back_up(dummy_service) job = Jobs.add( From d621ca64497a67b72b194a1d5002b0a764e56655 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 14 Aug 2023 12:50:45 +0000 Subject: [PATCH 17/27] refactor(backups): clean up unused mounting tools --- .../backup/backuppers/restic_backupper.py | 28 ----------------- tests/test_graphql/test_backup.py | 30 ------------------- 2 files changed, 58 deletions(-) diff --git a/selfprivacy_api/backup/backuppers/restic_backupper.py b/selfprivacy_api/backup/backuppers/restic_backupper.py index a935520..e954b65 100644 --- a/selfprivacy_api/backup/backuppers/restic_backupper.py +++ b/selfprivacy_api/backup/backuppers/restic_backupper.py @@ -127,34 +127,6 @@ class ResticBackupper(AbstractBackupper): output, ) - def mount_repo(self, mount_directory: str) -> subprocess.Popen: - if not exists(mount_directory): - raise FileNotFoundError("no such directory to mount at: ", mount_directory) - mount_command = self.restic_command("mount", mount_directory) - mount_command.insert(0, "nohup") - handle = subprocess.Popen( - mount_command, - stdout=subprocess.DEVNULL, - shell=False, - ) - sleep(2) - if "ids" not in listdir(mount_directory): - raise IOError("failed to mount dir ", mount_directory) - return handle - - def unmount_repo(self, mount_directory: str) -> None: - mount_command = ["umount", "-l", mount_directory] - with subprocess.Popen( - mount_command, stdout=subprocess.PIPE, shell=False - ) as handle: - output = handle.communicate()[0].decode("utf-8") - # TODO: check for exit code? - if "error" in output.lower(): - raise IOError("failed to unmount dir ", mount_directory, ": ", output) - - if not listdir(mount_directory) == []: - raise IOError("failed to unmount dir ", mount_directory) - @staticmethod def __flatten_list(list_to_flatten): """string-aware list flattener""" diff --git a/tests/test_graphql/test_backup.py b/tests/test_graphql/test_backup.py index 25eaaf4..6878de1 100644 --- a/tests/test_graphql/test_backup.py +++ b/tests/test_graphql/test_backup.py @@ -727,36 +727,6 @@ def test_sync_nonexistent_src(dummy_service): sync(src, dst) -# Restic lowlevel -def test_mount_umount(backups, dummy_service, tmpdir): - Backups.back_up(dummy_service) - backupper = Backups.provider().backupper - assert isinstance(backupper, ResticBackupper) - - mountpoint = tmpdir / "mount" - makedirs(mountpoint) - assert path.exists(mountpoint) - assert len(listdir(mountpoint)) == 0 - - handle = backupper.mount_repo(mountpoint) - assert len(listdir(mountpoint)) != 0 - - backupper.unmount_repo(mountpoint) - # handle.terminate() - assert len(listdir(mountpoint)) == 0 - - -def test_mount_nonexistent(backups, dummy_service, tmpdir): - backupper = Backups.provider().backupper - assert isinstance(backupper, ResticBackupper) - - mountpoint = tmpdir / "nonexistent" - assert not path.exists(mountpoint) - - with pytest.raises(FileNotFoundError): - handle = backupper.mount_repo(mountpoint) - - def test_move_blocks_backups(backups, dummy_service, restore_strategy): snap = Backups.back_up(dummy_service) job = Jobs.add( From d6cf2abdc23444e737bde58dd8b47f331c5f739b Mon Sep 17 00:00:00 2001 From: Inex Code Date: Wed, 23 Aug 2023 14:51:01 +0300 Subject: [PATCH 18/27] style: remove unused imports --- .../backup/backuppers/restic_backupper.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/selfprivacy_api/backup/backuppers/restic_backupper.py b/selfprivacy_api/backup/backuppers/restic_backupper.py index e954b65..f508368 100644 --- a/selfprivacy_api/backup/backuppers/restic_backupper.py +++ b/selfprivacy_api/backup/backuppers/restic_backupper.py @@ -9,8 +9,7 @@ from typing import List, TypeVar, Callable from collections.abc import Iterable from json.decoder import JSONDecodeError from os.path import exists, join -from os import listdir, mkdir -from time import sleep +from os import mkdir from shutil import rmtree from selfprivacy_api.backup.util import output_yielder, sync @@ -33,12 +32,12 @@ def unlocked_repo(func: T) -> T: def inner(self: ResticBackupper, *args, **kwargs): try: return func(self, *args, **kwargs) - except Exception as e: - if "unable to create lock" in str(e): + except Exception as error: + if "unable to create lock" in str(error): self.unlock() return func(self, *args, **kwargs) else: - raise e + raise error # Above, we manually guarantee that the type returned is compatible. return inner # type: ignore @@ -293,8 +292,8 @@ class ResticBackupper(AbstractBackupper): break if "unable" in line: raise ValueError(line) - except Exception as e: - raise ValueError("could not lock repository") from e + except Exception as error: + raise ValueError("could not lock repository") from error @unlocked_repo def restored_size(self, snapshot_id: str) -> int: From f2c972ed5f0790d9a3eb4271c3114bdbf9c3acd3 Mon Sep 17 00:00:00 2001 From: Inex Code Date: Wed, 23 Aug 2023 14:51:15 +0300 Subject: [PATCH 19/27] chore: bump version --- selfprivacy_api/dependencies.py | 2 +- setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/selfprivacy_api/dependencies.py b/selfprivacy_api/dependencies.py index fb974e8..095d087 100644 --- a/selfprivacy_api/dependencies.py +++ b/selfprivacy_api/dependencies.py @@ -27,4 +27,4 @@ async def get_token_header( def get_api_version() -> str: """Get API version""" - return "2.3.0" + return "2.3.1" diff --git a/setup.py b/setup.py index 684f54f..99f0679 100755 --- a/setup.py +++ b/setup.py @@ -2,7 +2,7 @@ from setuptools import setup, find_packages setup( name="selfprivacy_api", - version="2.3.0", + version="2.3.1", packages=find_packages(), scripts=[ "selfprivacy_api/app.py", From 0a852d8b509cd95edaad498a85efa4f7191795db Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Wed, 23 Aug 2023 13:39:12 +0000 Subject: [PATCH 20/27] fix(backups): consider failing services MORE and not try to stop them --- selfprivacy_api/services/service.py | 2 +- selfprivacy_api/services/test_service/__init__.py | 8 ++++++-- tests/test_graphql/test_backup.py | 12 +++++++++++- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/selfprivacy_api/services/service.py b/selfprivacy_api/services/service.py index 30e810f..e6f51d3 100644 --- a/selfprivacy_api/services/service.py +++ b/selfprivacy_api/services/service.py @@ -283,7 +283,7 @@ class StoppedService: def __enter__(self) -> Service: self.original_status = self.service.get_status() - if self.original_status != ServiceStatus.INACTIVE: + if self.original_status not in [ServiceStatus.INACTIVE, ServiceStatus.FAILED]: self.service.stop() wait_until_true( lambda: self.service.get_status() == ServiceStatus.INACTIVE, diff --git a/selfprivacy_api/services/test_service/__init__.py b/selfprivacy_api/services/test_service/__init__.py index 967b32e..6ae33ef 100644 --- a/selfprivacy_api/services/test_service/__init__.py +++ b/selfprivacy_api/services/test_service/__init__.py @@ -135,8 +135,12 @@ class DummyService(Service): @classmethod def stop(cls): - cls.set_status(ServiceStatus.DEACTIVATING) - cls.change_status_with_async_delay(ServiceStatus.INACTIVE, cls.startstop_delay) + # simulate a failing service unable to stop + if not cls.get_status() == ServiceStatus.FAILED: + cls.set_status(ServiceStatus.DEACTIVATING) + cls.change_status_with_async_delay( + ServiceStatus.INACTIVE, cls.startstop_delay + ) @classmethod def start(cls): diff --git a/tests/test_graphql/test_backup.py b/tests/test_graphql/test_backup.py index 1990ef7..f66398d 100644 --- a/tests/test_graphql/test_backup.py +++ b/tests/test_graphql/test_backup.py @@ -10,6 +10,7 @@ from subprocess import Popen import selfprivacy_api.services as services from selfprivacy_api.services import Service, get_all_services +from selfprivacy_api.services.service import ServiceStatus from selfprivacy_api.services import get_service_by_id from selfprivacy_api.services.test_service import DummyService @@ -462,10 +463,19 @@ def restore_strategy(request) -> RestoreStrategy: return RestoreStrategy.INPLACE +@pytest.fixture(params=["failed", "healthy"]) +def failed(request) -> bool: + if request.param == "failed": + return True + return False + + def test_restore_snapshot_task( - backups, dummy_service, restore_strategy, simulated_service_stopping_delay + backups, dummy_service, restore_strategy, simulated_service_stopping_delay, failed ): dummy_service.set_delay(simulated_service_stopping_delay) + if failed: + dummy_service.set_status(ServiceStatus.FAILED) Backups.back_up(dummy_service) snaps = Backups.get_snapshots(dummy_service) From 72535f86558ff90e7ad9ea964a282ec9a0044099 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Wed, 23 Aug 2023 13:40:04 +0000 Subject: [PATCH 21/27] fix(backups): default timeout to 5 min for service starting and stopping in backup operations --- 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 e6f51d3..e01501b 100644 --- a/selfprivacy_api/services/service.py +++ b/selfprivacy_api/services/service.py @@ -13,7 +13,7 @@ from selfprivacy_api.services.owned_path import OwnedPath from selfprivacy_api import utils from selfprivacy_api.utils.waitloop import wait_until_true -DEFAULT_START_STOP_TIMEOUT = 10 * 60 +DEFAULT_START_STOP_TIMEOUT = 5 * 60 class ServiceStatus(Enum): From de52dffddadab70dc09cf5fcd853324f215c9ad7 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Wed, 23 Aug 2023 13:55:23 +0000 Subject: [PATCH 22/27] refactor(backups): a better backup-related service timeout error --- selfprivacy_api/services/service.py | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/selfprivacy_api/services/service.py b/selfprivacy_api/services/service.py index e01501b..0f018b6 100644 --- a/selfprivacy_api/services/service.py +++ b/selfprivacy_api/services/service.py @@ -284,17 +284,23 @@ class StoppedService: def __enter__(self) -> Service: self.original_status = self.service.get_status() if self.original_status not in [ServiceStatus.INACTIVE, ServiceStatus.FAILED]: - self.service.stop() - wait_until_true( - lambda: self.service.get_status() == ServiceStatus.INACTIVE, - timeout_sec=DEFAULT_START_STOP_TIMEOUT, - ) + try: + self.service.stop() + wait_until_true( + lambda: self.service.get_status() == ServiceStatus.INACTIVE, + timeout_sec=DEFAULT_START_STOP_TIMEOUT, + ) + except TimeoutError as e: + raise TimeoutError(f"timed out waiting for {self.service.get_display_name()} to stop") return self.service def __exit__(self, type, value, traceback): if self.original_status in [ServiceStatus.ACTIVATING, ServiceStatus.ACTIVE]: - self.service.start() - wait_until_true( - lambda: self.service.get_status() == ServiceStatus.ACTIVE, - timeout_sec=DEFAULT_START_STOP_TIMEOUT, - ) + try: + self.service.start() + wait_until_true( + lambda: self.service.get_status() == ServiceStatus.ACTIVE, + timeout_sec=DEFAULT_START_STOP_TIMEOUT, + ) + except TimeoutError as e: + raise TimeoutError(f"timed out waiting for {self.service.get_display_name()} to start") From 02b03cf401c88e348cb6c2c9582b4d1f9556bf76 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Wed, 23 Aug 2023 14:02:07 +0000 Subject: [PATCH 23/27] feature(backups): report the error text in a job --- selfprivacy_api/backup/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/selfprivacy_api/backup/__init__.py b/selfprivacy_api/backup/__init__.py index c28c01f..167ff33 100644 --- a/selfprivacy_api/backup/__init__.py +++ b/selfprivacy_api/backup/__init__.py @@ -283,7 +283,7 @@ class Backups: Backups._store_last_snapshot(tag, snapshot) service.post_restore() except Exception as error: - Jobs.update(job, status=JobStatus.ERROR) + Jobs.update(job, status=JobStatus.ERROR, status_text=str(error)) raise error Jobs.update(job, status=JobStatus.FINISHED) @@ -348,7 +348,7 @@ class Backups: service.post_restore() except Exception as error: - Jobs.update(job, status=JobStatus.ERROR) + Jobs.update(job, status=JobStatus.ERROR, status_text=str(error)) raise error Jobs.update(job, status=JobStatus.FINISHED) From c68239044f8c5983a68ae0606ebb9f5ccd07d56c Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Wed, 23 Aug 2023 14:18:33 +0000 Subject: [PATCH 24/27] feature(backups): report status text for restore jobs --- selfprivacy_api/backup/__init__.py | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/selfprivacy_api/backup/__init__.py b/selfprivacy_api/backup/__init__.py index 167ff33..0f445a8 100644 --- a/selfprivacy_api/backup/__init__.py +++ b/selfprivacy_api/backup/__init__.py @@ -308,7 +308,9 @@ class Backups: ) -> None: failsafe_snapshot = Backups.back_up(service) - Jobs.update(job, status=JobStatus.RUNNING) + Jobs.update( + job, status=JobStatus.RUNNING, status_text=f"Restoring from {snapshot.id}" + ) try: Backups._restore_service_from_snapshot( service, @@ -316,9 +318,19 @@ class Backups: verify=False, ) except Exception as error: + Jobs.update( + job, + status=JobStatus.ERROR, + status_text=f" restore failed with {str(error)}, reverting to {failsafe_snapshot.id}", + ) Backups._restore_service_from_snapshot( service, failsafe_snapshot.id, verify=False ) + Jobs.update( + job, + status=JobStatus.ERROR, + status_text=f" restore failed with {str(error)}, reverted to {failsafe_snapshot.id}", + ) raise error @staticmethod @@ -335,17 +347,30 @@ class Backups: try: Backups._assert_restorable(snapshot) + Jobs.update( + job, status=JobStatus.CREATED, status_text="stopping the service" + ) with StoppedService(service): Backups.assert_dead(service) if strategy == RestoreStrategy.INPLACE: Backups._inplace_restore(service, snapshot, job) else: # verify_before_download is our default - Jobs.update(job, status=JobStatus.RUNNING) + Jobs.update( + job, + status=JobStatus.RUNNING, + status_text=f"Restoring from {snapshot.id}", + ) Backups._restore_service_from_snapshot( service, snapshot.id, verify=True ) service.post_restore() + Jobs.update( + job, + status=JobStatus.RUNNING, + progress=90, + status_text="restarting the service", + ) except Exception as error: Jobs.update(job, status=JobStatus.ERROR, status_text=str(error)) From 1333aad57dac56dea8f0d2b990bc0a66467c9a4b Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Wed, 23 Aug 2023 14:35:03 +0000 Subject: [PATCH 25/27] feature(backups): temporarily revert restore job status to created for inplace restore to run backup --- selfprivacy_api/backup/__init__.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/selfprivacy_api/backup/__init__.py b/selfprivacy_api/backup/__init__.py index 0f445a8..7b013f4 100644 --- a/selfprivacy_api/backup/__init__.py +++ b/selfprivacy_api/backup/__init__.py @@ -306,6 +306,9 @@ class Backups: snapshot: Snapshot, job: Job, ) -> None: + Jobs.update( + job, status=JobStatus.CREATED, status_text=f"Waiting for pre-restore backup" + ) failsafe_snapshot = Backups.back_up(service) Jobs.update( @@ -321,7 +324,7 @@ class Backups: Jobs.update( job, status=JobStatus.ERROR, - status_text=f" restore failed with {str(error)}, reverting to {failsafe_snapshot.id}", + status_text=f"Restore failed with {str(error)}, reverting to {failsafe_snapshot.id}", ) Backups._restore_service_from_snapshot( service, failsafe_snapshot.id, verify=False @@ -329,7 +332,7 @@ class Backups: Jobs.update( job, status=JobStatus.ERROR, - status_text=f" restore failed with {str(error)}, reverted to {failsafe_snapshot.id}", + status_text=f"Restore failed with {str(error)}, reverted to {failsafe_snapshot.id}", ) raise error @@ -348,7 +351,7 @@ class Backups: try: Backups._assert_restorable(snapshot) Jobs.update( - job, status=JobStatus.CREATED, status_text="stopping the service" + job, status=JobStatus.RUNNING, status_text="Stopping the service" ) with StoppedService(service): Backups.assert_dead(service) @@ -369,7 +372,7 @@ class Backups: job, status=JobStatus.RUNNING, progress=90, - status_text="restarting the service", + status_text="Restarting the service", ) except Exception as error: From 9db717c774ff2ddb22d81aad6930caebf53c4c83 Mon Sep 17 00:00:00 2001 From: Inex Code Date: Fri, 25 Aug 2023 19:28:37 +0300 Subject: [PATCH 26/27] style: linting --- selfprivacy_api/services/service.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/selfprivacy_api/services/service.py b/selfprivacy_api/services/service.py index 0f018b6..b66bd19 100644 --- a/selfprivacy_api/services/service.py +++ b/selfprivacy_api/services/service.py @@ -290,8 +290,10 @@ class StoppedService: lambda: self.service.get_status() == ServiceStatus.INACTIVE, timeout_sec=DEFAULT_START_STOP_TIMEOUT, ) - except TimeoutError as e: - raise TimeoutError(f"timed out waiting for {self.service.get_display_name()} to stop") + except TimeoutError as error: + raise TimeoutError( + f"timed out waiting for {self.service.get_display_name()} to stop" + ) from error return self.service def __exit__(self, type, value, traceback): @@ -302,5 +304,7 @@ class StoppedService: lambda: self.service.get_status() == ServiceStatus.ACTIVE, timeout_sec=DEFAULT_START_STOP_TIMEOUT, ) - except TimeoutError as e: - raise TimeoutError(f"timed out waiting for {self.service.get_display_name()} to start") + except TimeoutError as error: + raise TimeoutError( + f"timed out waiting for {self.service.get_display_name()} to start" + ) from error From 0dfb41a689249ff28d1316cd77de8e96f31295b4 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Fri, 1 Sep 2023 10:41:27 +0000 Subject: [PATCH 27/27] feature(backups): a task to autorefresh cache. Redis expiry abolished --- selfprivacy_api/backup/storage.py | 5 ----- selfprivacy_api/backup/tasks.py | 11 ++++++++++- tests/test_graphql/test_backup.py | 19 ++++++++++++++++++- 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/selfprivacy_api/backup/storage.py b/selfprivacy_api/backup/storage.py index d46f584..4d1d415 100644 --- a/selfprivacy_api/backup/storage.py +++ b/selfprivacy_api/backup/storage.py @@ -16,9 +16,6 @@ from selfprivacy_api.utils.redis_model_storage import ( from selfprivacy_api.backup.providers.provider import AbstractBackupProvider from selfprivacy_api.backup.providers import get_kind -# a hack to store file path. -REDIS_SNAPSHOT_CACHE_EXPIRE_SECONDS = 24 * 60 * 60 # one day - REDIS_SNAPSHOTS_PREFIX = "backups:snapshots:" REDIS_LAST_BACKUP_PREFIX = "backups:last-backed-up:" REDIS_INITTED_CACHE = "backups:repo_initted" @@ -26,7 +23,6 @@ REDIS_INITTED_CACHE = "backups:repo_initted" REDIS_PROVIDER_KEY = "backups:provider" REDIS_AUTOBACKUP_PERIOD_KEY = "backups:autobackup_period" - redis = RedisPool().get_connection() @@ -89,7 +85,6 @@ class Storage: """Stores snapshot metadata in redis for caching purposes""" snapshot_key = Storage.__snapshot_key(snapshot) store_model_as_hash(redis, snapshot_key, snapshot) - redis.expire(snapshot_key, REDIS_SNAPSHOT_CACHE_EXPIRE_SECONDS) @staticmethod def delete_cached_snapshot(snapshot: Snapshot) -> None: diff --git a/selfprivacy_api/backup/tasks.py b/selfprivacy_api/backup/tasks.py index db350d4..2b6b79c 100644 --- a/selfprivacy_api/backup/tasks.py +++ b/selfprivacy_api/backup/tasks.py @@ -7,13 +7,17 @@ from selfprivacy_api.graphql.common_types.backup import RestoreStrategy from selfprivacy_api.models.backup.snapshot import Snapshot from selfprivacy_api.utils.huey import huey +from huey import crontab from selfprivacy_api.services.service import Service from selfprivacy_api.backup import Backups +SNAPSHOT_CACHE_TTL_HOURS = 6 + def validate_datetime(dt: datetime) -> bool: """ - Validates that the datetime passed in is timezone-aware. + Validates that it is time to back up. + Also ensures that the timezone-aware time is used. """ if dt.tzinfo is None: return Backups.is_time_to_backup(dt.replace(tzinfo=timezone.utc)) @@ -50,3 +54,8 @@ def automatic_backup(): time = datetime.utcnow().replace(tzinfo=timezone.utc) for service in Backups.services_to_back_up(time): start_backup(service) + + +@huey.periodic_task(crontab(hour=SNAPSHOT_CACHE_TTL_HOURS)) +def reload_snapshot_cache(): + Backups.force_snapshot_cache_reload() diff --git a/tests/test_graphql/test_backup.py b/tests/test_graphql/test_backup.py index dc491c4..d54af7b 100644 --- a/tests/test_graphql/test_backup.py +++ b/tests/test_graphql/test_backup.py @@ -32,7 +32,11 @@ from selfprivacy_api.backup.backuppers.restic_backupper import ResticBackupper from selfprivacy_api.backup.jobs import add_backup_job, add_restore_job -from selfprivacy_api.backup.tasks import start_backup, restore_snapshot +from selfprivacy_api.backup.tasks import ( + start_backup, + restore_snapshot, + reload_snapshot_cache, +) from selfprivacy_api.backup.storage import Storage from selfprivacy_api.backup.jobs import get_backup_job @@ -806,3 +810,16 @@ def test_tempfile(): with tempfile.TemporaryDirectory() as temp: assert path.exists(temp) assert not path.exists(temp) + + +# Storage +def test_cache_invalidaton_task(backups, dummy_service): + Backups.back_up(dummy_service) + assert len(Storage.get_cached_snapshots()) == 1 + + # Does not trigger resync + Storage.invalidate_snapshot_storage() + assert Storage.get_cached_snapshots() == [] + + reload_snapshot_cache() + assert len(Storage.get_cached_snapshots()) == 1