Backups, remote repositories: uniform parameter handling

- introduce env_vars in backups script and access_params for more
  uniform handling of access parameters
- added tests for creating and deleting an archive

Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

actions/backups

@@ -39,28 +39,40 @@ def parse_arguments():
 
     subparsers.add_parser(
         'setup', help='Create repository if it does not already exist')
+
     info = subparsers.add_parser('info', help='Show repository information')
     info.add_argument('--repository', help='Repository path', required=True)
+
     init = subparsers.add_parser('init', help='Initialize a repository')
     init.add_argument('--repository', help='Repository path', required=True)
     init.add_argument('--encryption', help='Enryption of the repository',
                       required=True)
-    subparsers.add_parser('list', help='List repository contents')
 
-    create = subparsers.add_parser('create', help='Create archive')
+    list_repo = subparsers.add_parser('list-repo',
-    create.add_argument('--name', help='Archive name', required=True)
+                                      help='List repository contents')
-    create.add_argument('--paths', help='Paths to include in archive',
-                        nargs='+')
 
-    delete = subparsers.add_parser('delete', help='Delete archive')
+    create_archive = subparsers.add_parser('create-archive',
-    delete.add_argument('--name', help='Archive name', required=True)
+                                           help='Create archive')
+    create_archive.add_argument('--paths', help='Paths to include in archive',
+                                nargs='+')
+
+    delete_archive = subparsers.add_parser('delete-archive',
+                                           help='Delete archive')
 
     export_help = 'Export archive contents as tar on stdout'
     export_tar = subparsers.add_parser('export-tar', help=export_help)
     export_tar.add_argument('--name', help='Archive name)',
                             required=True)
-    # TODO: add parameters to missing commands (list, create, delete, export)
+
-    for cmd in [info, init]:
+    get_archive_apps = subparsers.add_parser(
+        'get-archive-apps',
+        help='Get list of apps included in archive')
+
+    # TODO: add parameters to missing commands (export etc)
+    for cmd in [info, init, list_repo, create_archive, delete_archive,
+                get_archive_apps]:
+        cmd.add_argument('--path', help='Repository or Archive path',
+                         required=False)  # TODO: set required to True!
         cmd.add_argument('--ssh-keyfile', help='Path of private ssh key',
                          default=None)
         cmd.add_argument('--encryption-passphrase',
@@ -73,12 +85,6 @@ def parse_arguments():
     get_exported_archive_apps.add_argument(
         '--path', help='Tarball file path', required=True)
 
-    get_archive_apps = subparsers.add_parser(
-        'get-archive-apps',
-        help='Get list of apps included in archive')
-    get_archive_apps.add_argument(
-        '--path', help='Path of the archive', required=True)
-
     restore_exported_archive = subparsers.add_parser(
         'restore-exported-archive',
         help='Restore files from an exported archive')
@@ -97,6 +103,7 @@ def parse_arguments():
 
 def subcommand_setup(_):
     """Create repository if it does not already exist."""
+    # TODO: use init()
     try:
         subprocess.run(['borg', 'info', REPOSITORY], check=True)
     except:
@@ -140,29 +147,26 @@ def subcommand_init(arguments):
 def subcommand_info(arguments):
     """Show repository information."""
     env = get_env(arguments)
-    cmd = ['borg', 'info', '--json', arguments.repository]
+    run(['borg', 'info', '--json', arguments.repository], env=env)
-    run(cmd, env=env)
 
 
-def subcommand_list(_):
+def subcommand_list_repo(arguments):
     """List repository contents."""
-    run(['borg', 'list', '--json', REPOSITORY])
+    env = get_env(arguments)
+    run(['borg', 'list', '--json', arguments.path], env=env)
 
 
-def subcommand_create(arguments):
+def subcommand_create_archive(arguments):
     """Create archive."""
+    env = get_env(arguments)
     paths = filter(os.path.exists, arguments.paths)
-    run([
+    run(['borg', 'create', '--json', arguments.path] + list(paths), env=env)
-        'borg',
-        'create',
-        '--json',
-        REPOSITORY + '::' + arguments.name,
-    ] + list(paths))
 
 
-def subcommand_delete(arguments):
+def subcommand_delete_archive(arguments):
     """Delete archive."""
-    run(['borg', 'delete', REPOSITORY + '::' + arguments.name])
+    env = get_env(arguments)
+    run(['borg', 'delete', arguments.path], env=env)
 
 
 def _extract(archive_path, destination, locations=None):
@@ -203,11 +207,18 @@ def _read_archive_file(archive, filepath):
 
 def subcommand_get_archive_apps(arguments):
     """Get list of apps included in archive."""
+    import ipdb; ipdb.set_trace()
+    env = get_env(arguments)
     manifest_folder = os.path.relpath(MANIFESTS_FOLDER, '/')
     borg_call = ['borg', 'list', arguments.path, manifest_folder,
                  '--format', '{path}{NEWLINE}']
+    timeout = None
+    if 'BORG_RSH' in env and 'SSHPASS' not in env:
+        timeout = TIMEOUT
     try:
-        manifest_path = subprocess.check_output(borg_call).decode().strip()
+        manifest_path = subprocess.check_output(borg_call, env=env,
+                                                timeout=timeout).decode()\
+                                                .strip()
     except subprocess.CalledProcessError:
         sys.exit(1)
 

plinth/modules/backups/__init__.py

@@ -49,6 +49,7 @@ REPOSITORY = '/var/lib/freedombox/borgbackup'
 SESSION_PATH_VARIABLE = 'fbx-backups-upload-path'
 # known errors that come up when remotely accessing a borg repository
 # 'errors' are error strings to look for in the stacktrace.
+ACCESS_PARAMS = ['ssh_keyfile', 'ssh_password', 'encryption_passphrase']
 KNOWN_ERRORS = [{
         "errors": ["subprocess.TimeoutExpired"],
         "message": _("Server not reachable - try providing a password."),
@@ -78,22 +79,14 @@ def setup(helper, old_version=None):
     helper.call('post', actions.superuser_run, 'backups', ['setup'])
 
 
-def get_info(repository, encryption_passphrase=None, ssh_password=None,
+def get_info(repository, access_params=None):
-             ssh_keyfile=None):
+    cmd = ['info', '--repository', repository]
-    args = ['info', '--repository', repository]
+    output = run(cmd, access_params)
-    kwargs = {}
-    if ssh_password is not None:
-        kwargs['input'] = ssh_password.encode()
-    if ssh_keyfile is not None:
-        args += ['--ssh-keyfile', ssh_keyfile]
-    if encryption_passphrase is not None:
-        args += ['--encryption-passphrase', encryption_passphrase]
-    output = actions.superuser_run('backups', args, **kwargs)
     return json.loads(output)
 
 
-def list_archives():
+def list_archives(repository, access_params=None):
-    output = actions.superuser_run('backups', ['list'])
+    output = run(['list-repo', '--path', repository], access_params)
     return json.loads(output)['archives']
 
 
@@ -105,20 +98,14 @@ def get_archive(name):
     return None
 
 
-def test_connection(repository, encryption_passphrase=None, ssh_password=None,
+def test_connection(repository, access_params=None):
-                    ssh_keyfile=None):
     """
     Test connecting to a local or remote borg repository.
     Tries to detect (and throw) some known ssh or borg errors.
     Returns 'borg info' information otherwise.
     """
     try:
-        # TODO: instead of passing encryption_passphrase, ssh_password and
+        return get_info(repository, access_params)
-        # ssh_keyfile around all the time, try using an 'options' dict.
-        message = get_info(repository,
-                           encryption_passphrase=encryption_passphrase,
-                           ssh_password=ssh_password, ssh_keyfile=ssh_keyfile)
-        return message
     except ActionError as err:
         caught_error = str(err)
         for known_error in KNOWN_ERRORS:
@@ -149,33 +136,22 @@ def _backup_handler(packet):
     paths = packet.directories + packet.files
     paths.append(manifest_path)
     actions.superuser_run(
-        'backups', ['create', '--name', packet.label, '--paths'] + paths)
+        'backups', ['create-archive', '--name', packet.label, '--paths'] +
+        paths)
 
 
-def create_archive(name, app_names):
+def create_archive(name, app_names, access_params=None):
-    api.backup_apps(_backup_handler, app_names, name)
+    api.backup_apps(_backup_handler, app_names, name,
+                    access_params=access_params)
 
 
-def create_repository(repository, encryption, encryption_passphrase=None,
+def create_repository(repository, encryption, access_params=None):
-                      ssh_keyfile=None, ssh_password=None):
     cmd = ['init', '--repository', repository, '--encryption', encryption]
-    if ssh_keyfile:
+    run(cmd, access_params=access_params)
-        cmd += ['--ssh-keyfile', ssh_keyfile]
-    if encryption_passphrase:
-        cmd += ['--encryption-passphrase', encryption_passphrase]
-
-    kwargs = {}
-    if ssh_password:
-        kwargs['input'] = ssh_password.encode()
-
-    output = actions.superuser_run('backups', cmd, **kwargs)
-    if output:
-        output = json.loads(output)
-    return output
 
 
-def delete_archive(name):
+def delete_archive(path):
-    actions.superuser_run('backups', ['delete', '--name', name])
+    actions.superuser_run('backups', ['delete-archive', '--path', path])
 
 
 def get_archive_path(archive_name):
@@ -223,3 +199,34 @@ def restore(archive_path, apps=None):
     """Restore files from a backup archive."""
     api.restore_apps(_restore_archive_handler, app_names=apps,
                      create_subvolume=False, backup_file=archive_path)
+
+
+def get_args(params):
+    args = []
+    kwargs = {}
+    if 'ssh_password' in params and params['ssh_password'] is not None:
+        kwargs['input'] = params['ssh_password'].encode()
+    if 'ssh_keyfile' in params and params['ssh_keyfile'] is not None:
+        args += ['--ssh-keyfile', params['ssh_keyfile']]
+    if 'encryption_passphrase' in params and \
+       params['encryption_passphrase'] is not None:
+        args += ['--encryption-passphrase', params['encryption_passphrase']]
+    return (args, kwargs)
+
+
+def run(arguments, access_params=None, superuser=True):
+    """Run a given command, appending (kw)args as necessary"""
+    command = 'backups'
+    kwargs = {}
+    if access_params:
+        for key in access_params.keys():
+            if key not in ACCESS_PARAMS:
+                raise ValueError('Unknown access parameter: %s' % key)
+
+        append_arguments, kwargs = get_args(access_params)
+        arguments += append_arguments
+
+    if superuser:
+        return actions.superuser_run(command, arguments, **kwargs)
+    else:
+        return actions.run(command, arguments, **kwargs)

plinth/modules/backups/views.py

@@ -334,12 +334,13 @@ class CreateRepositoryView(SuccessMessageMixin, FormView):
         try:
             backups.test_connection(repository, ssh_password)
         except BorgRepositoryDoesNotExistError:
-            kwargs = {}
+            access_params = {}
             if encryption_passphrase:
-                kwargs['encryption_passphrase'] = encryption_passphrase
+                access_params['encryption_passphrase'] = encryption_passphrase
             if ssh_keyfile:
-                kwargs['ssh_keyfile'] = ssh_keyfile
+                access_params['ssh_keyfile'] = ssh_keyfile
-            backups.create_repository(repository, encryption, **kwargs)
+            backups.create_repository(repository, encryption,
+                                      access_params=access_params)
 
         repositories = kvstore.get_default('backups_repositories', [])
         if repositories:

plinth/tests/test_backups.py

@@ -25,6 +25,8 @@ import unittest
 
 from plinth import cfg
 from plinth.modules import backups
+from plinth import actions
+
 
 euid = os.geteuid()
 
@@ -37,6 +39,8 @@ class TestBackups(unittest.TestCase):
         """Initial setup for all the classes."""
         cls.action_directory = tempfile.TemporaryDirectory()
         cls.backup_directory = tempfile.TemporaryDirectory()
+        cls.data_directory = os.path.join(os.path.dirname(
+            os.path.realpath(__file__)), 'data')
         cfg.actions_dir = cls.action_directory.name
         actions_dir = os.path.join(os.path.dirname(__file__), '..', '..',
                                    'actions')
@@ -73,11 +77,34 @@ class TestBackups(unittest.TestCase):
     def test_create_encrypted_repository(self):
         repo_path = os.path.join(self.backup_directory.name,
                                  'borgbackup_encrypted')
-        passphrase = '12345'
+        # 'borg init' creates missing folders automatically
-        # create_repository is supposed to create the folder automatically
+        access_params = {'encryption_passphrase': '12345'}
-        # if it does not exist
         backups.create_repository(repo_path, 'repokey',
-                                  encryption_passphrase=passphrase)
+                                  access_params=access_params)
-        assert backups.get_info(repo_path, encryption_passphrase=passphrase)
+        assert backups.get_info(repo_path, access_params)
-        assert backups.test_connection(repo_path,
+        assert backups.test_connection(repo_path, access_params)
-                                       encryption_passphrase=passphrase)
+
+    @unittest.skipUnless(euid == 0, 'Needs to be root')
+    def test_create_and_delete_archive(self):
+        """
+        - Create a repo
+        - Create an archive
+        - Verify archive content
+        - Delete archive
+        """
+        repo_name = 'test_create_and_delete'
+        archive_name = 'first_archive'
+        repo_path = os.path.join(self.backup_directory.name, repo_name)
+
+        backups.create_repository(repo_path, 'none')
+        archive_path = "::".join([repo_path, archive_name])
+        actions.superuser_run(
+            'backups', ['create-archive', '--path', archive_path, '--paths',
+                        self.data_directory])
+
+        archive = backups.list_archives(repo_path)[0]
+        assert archive['name'] == archive_name
+
+        backups.delete_archive(archive_path)
+        content = backups.list_archives(repo_path)
+        assert len(content) == 0