From 4be1b0be768478e5438f91db51a7d0d0a0c2e141 Mon Sep 17 00:00:00 2001 From: Michael Pimmer Date: Tue, 27 Nov 2018 07:06:09 +0000 Subject: [PATCH] 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 --- actions/backups | 69 +++++++++++++---------- plinth/modules/backups/__init__.py | 89 ++++++++++++++++-------------- plinth/modules/backups/views.py | 9 +-- plinth/tests/test_backups.py | 41 +++++++++++--- 4 files changed, 127 insertions(+), 81 deletions(-) diff --git a/actions/backups b/actions/backups index 4ea00ab31..84e944fc0 100755 --- a/actions/backups +++ b/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') - create.add_argument('--name', help='Archive name', required=True) - create.add_argument('--paths', help='Paths to include in archive', - nargs='+') + list_repo = subparsers.add_parser('list-repo', + help='List repository contents') - delete = subparsers.add_parser('delete', help='Delete archive') - delete.add_argument('--name', help='Archive name', required=True) + create_archive = subparsers.add_parser('create-archive', + 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(cmd, env=env) + run(['borg', 'info', '--json', arguments.repository], 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([ - 'borg', - 'create', - '--json', - REPOSITORY + '::' + arguments.name, - ] + list(paths)) + run(['borg', 'create', '--json', arguments.path] + list(paths), env=env) -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) diff --git a/plinth/modules/backups/__init__.py b/plinth/modules/backups/__init__.py index 182752d1d..495ebbaf8 100644 --- a/plinth/modules/backups/__init__.py +++ b/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, - ssh_keyfile=None): - args = ['info', '--repository', repository] - 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) +def get_info(repository, access_params=None): + cmd = ['info', '--repository', repository] + output = run(cmd, access_params) return json.loads(output) -def list_archives(): - output = actions.superuser_run('backups', ['list']) +def list_archives(repository, access_params=None): + 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, - ssh_keyfile=None): +def test_connection(repository, access_params=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 - # 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 + return get_info(repository, access_params) 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): - api.backup_apps(_backup_handler, app_names, name) +def create_archive(name, app_names, access_params=None): + api.backup_apps(_backup_handler, app_names, name, + access_params=access_params) -def create_repository(repository, encryption, encryption_passphrase=None, - ssh_keyfile=None, ssh_password=None): +def create_repository(repository, encryption, access_params=None): cmd = ['init', '--repository', repository, '--encryption', encryption] - if ssh_keyfile: - 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 + run(cmd, access_params=access_params) -def delete_archive(name): - actions.superuser_run('backups', ['delete', '--name', name]) +def delete_archive(path): + 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) diff --git a/plinth/modules/backups/views.py b/plinth/modules/backups/views.py index 53c835e4d..c81605032 100644 --- a/plinth/modules/backups/views.py +++ b/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 - backups.create_repository(repository, encryption, **kwargs) + access_params['ssh_keyfile'] = ssh_keyfile + backups.create_repository(repository, encryption, + access_params=access_params) repositories = kvstore.get_default('backups_repositories', []) if repositories: diff --git a/plinth/tests/test_backups.py b/plinth/tests/test_backups.py index b2b18ff50..4da17b21f 100644 --- a/plinth/tests/test_backups.py +++ b/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' - # create_repository is supposed to create the folder automatically - # if it does not exist + # 'borg init' creates missing folders automatically + access_params = {'encryption_passphrase': '12345'} backups.create_repository(repo_path, 'repokey', - encryption_passphrase=passphrase) - assert backups.get_info(repo_path, encryption_passphrase=passphrase) - assert backups.test_connection(repo_path, - encryption_passphrase=passphrase) + access_params=access_params) + assert backups.get_info(repo_path, access_params) + assert backups.test_connection(repo_path, access_params) + + @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