Merge pull request 'Do not use mounting for inplace restore' (#52) from mountchecks into master

Reviewed-on: https://git.selfprivacy.org/SelfPrivacy/selfprivacy-rest-api/pulls/52
Reviewed-by: Inex Code <inex.code@selfprivacy.org>

selfprivacy_api/backup/backuppers/restic_backupper.py

@@ -9,8 +9,8 @@ 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 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
@@ -32,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:
+        except Exception as error:
-            if "unable to create lock" in str(e):
+            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
@@ -126,32 +126,6 @@ class ResticBackupper(AbstractBackupper):
                     output,
                 )
 
-    def mount_repo(self, 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):
-        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():
-                return IOError("failed to unmount dir ", mount_directory, ": ", output)
-
-        if not listdir(mount_directory) == []:
-            return IOError("failed to unmount dir ", mount_directory)
-
     @staticmethod
     def __flatten_list(list_to_flatten):
         """string-aware list flattener"""
@@ -318,8 +292,8 @@ class ResticBackupper(AbstractBackupper):
                     break
                 if "unable" in line:
                     raise ValueError(line)
-        except Exception as e:
+        except Exception as error:
-            raise ValueError("could not lock repository") from e
+            raise ValueError("could not lock repository") from error
 
     @unlocked_repo
     def restored_size(self, snapshot_id: str) -> int:
@@ -362,20 +336,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
-                self.mount_repo(temp_dir)
-                snapshot_root = join(temp_dir, "ids", snapshot_id)
-
-            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}")
+                        raise ValueError(
+                            f"No such path: {src}. We tried to find {folder}"
+                        )
                     dst = folder
                     sync(src, dst)
 
-            if not verify:
+            else:  # attempting inplace restore
-                self.unmount_repo(temp_dir)
+                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"""

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"

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",

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
 
@@ -725,25 +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_move_blocks_backups(backups, dummy_service, restore_strategy):
     snap = Backups.back_up(dummy_service)
     job = Jobs.add(
@@ -806,3 +789,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)