From 6cdc92bd0c8b1f65fbd76b324970690088799ce5 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Wed, 13 Nov 2019 10:41:57 -0800 Subject: [PATCH] Fix for "borgmatic restore" showing success and incorrectly extracting archive files, even when no databases are configured to restore (#246). --- AUTHORS | 2 +- NEWS | 5 ++++- borgmatic/commands/borgmatic.py | 3 ++- borgmatic/hooks/dump.py | 25 +++++++++++++++++++++++++ setup.py | 2 +- tests/unit/hooks/test_dump.py | 33 +++++++++++++++++++++++++++++++++ 6 files changed, 66 insertions(+), 4 deletions(-) diff --git a/AUTHORS b/AUTHORS index a1bf861..4e28c57 100644 --- a/AUTHORS +++ b/AUTHORS @@ -11,4 +11,4 @@ Robin `ypid` Schneider: Support additional options of Borg and add validate-borg Scott Squires: Custom archive names Thomas LÉVEIL: Support for a keep_minutely prune option. Support for the --json option -Any many others! See the output of "git log". +And many others! See the output of "git log". diff --git a/NEWS b/NEWS index 2bbc2ef..2be93b1 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,7 @@ -1.4.10.dev0 +1.4.10 + * #246: Fix for "borgmatic restore" showing success and incorrectly extracting archive files, even + when no databases are configured to restore. As this can overwrite files from the archive and + lead to data loss, please upgrade to get the fix before using "borgmatic restore". * Reopen the file given by "--log-file" flag if an external program rotates the log file while borgmatic is running. diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 3cf6d9b..31dca27 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -266,12 +266,13 @@ def run_actions( dump.DATABASE_HOOK_NAMES, restore_names, ) + borg_extract.extract_archive( global_arguments.dry_run, repository, arguments['restore'].archive, dump.convert_glob_patterns_to_borg_patterns( - [pattern for patterns in dump_patterns.values() for pattern in patterns] + dump.flatten_dump_patterns(dump_patterns, restore_names) ), location, storage, diff --git a/borgmatic/hooks/dump.py b/borgmatic/hooks/dump.py index 11961b2..d0c32e9 100644 --- a/borgmatic/hooks/dump.py +++ b/borgmatic/hooks/dump.py @@ -20,6 +20,26 @@ def make_database_dump_filename(dump_path, name, hostname=None): return os.path.join(os.path.expanduser(dump_path), hostname or 'localhost', name) +def flatten_dump_patterns(dump_patterns, names): + ''' + Given a dict from a database hook name to glob patterns matching the dumps for the named + databases, flatten out all the glob patterns into a single sequence, and return it. + + Raise ValueError if there are no resulting glob patterns, which indicates that databases are not + configured in borgmatic's configuration. + ''' + flattened = [pattern for patterns in dump_patterns.values() for pattern in patterns] + + if not flattened: + raise ValueError( + 'Cannot restore database(s) {} missing from borgmatic\'s configuration'.format( + ', '.join(names) or '"all"' + ) + ) + + return flattened + + def remove_database_dumps(dump_path, databases, database_type_name, log_prefix, dry_run): ''' Remove the database dumps for the given databases in the dump directory path. The databases are @@ -121,6 +141,11 @@ def get_per_hook_database_configurations(hooks, names, dump_patterns): } if not names or 'all' in names: + if not any(hook_databases.values()): + raise ValueError( + 'Cannot restore database "all", as there are no database dumps in the archive' + ) + return hook_databases found_names = { diff --git a/setup.py b/setup.py index 8280afa..93a8a77 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,6 @@ from setuptools import find_packages, setup -VERSION = '1.4.10.dev0' +VERSION = '1.4.10' setup( diff --git a/tests/unit/hooks/test_dump.py b/tests/unit/hooks/test_dump.py index dd86ba0..1d52c44 100644 --- a/tests/unit/hooks/test_dump.py +++ b/tests/unit/hooks/test_dump.py @@ -26,6 +26,27 @@ def test_make_database_dump_filename_with_invalid_name_raises(): module.make_database_dump_filename('databases', 'invalid/name') +def test_flatten_dump_patterns_produces_list_of_all_patterns(): + dump_patterns = {'postgresql_databases': ['*/glob', 'glob/*'], 'mysql_databases': ['*/*/*']} + expected_patterns = dump_patterns['postgresql_databases'] + dump_patterns['mysql_databases'] + + assert module.flatten_dump_patterns(dump_patterns, ('bob',)) == expected_patterns + + +def test_flatten_dump_patterns_with_no_patterns_errors(): + dump_patterns = {'postgresql_databases': [], 'mysql_databases': []} + + with pytest.raises(ValueError): + assert module.flatten_dump_patterns(dump_patterns, ('bob',)) + + +def test_flatten_dump_patterns_with_no_hooks_errors(): + dump_patterns = {} + + with pytest.raises(ValueError): + assert module.flatten_dump_patterns(dump_patterns, ('bob',)) + + def test_remove_database_dumps_removes_dump_for_each_database(): databases = [{'name': 'foo'}, {'name': 'bar'}] flexmock(module).should_receive('make_database_dump_filename').and_return( @@ -134,3 +155,15 @@ def test_get_per_hook_database_configurations_with_unknown_database_name_raises( with pytest.raises(ValueError): module.get_per_hook_database_configurations(hooks, names, dump_patterns) + + +def test_get_per_hook_database_configurations_with_all_and_no_archive_dumps_raises(): + hooks = {'postgresql_databases': [flexmock()]} + names = ('foo', 'all') + dump_patterns = flexmock() + flexmock(module).should_receive('get_database_configurations').with_args( + hooks['postgresql_databases'], names + ).and_return([]) + + with pytest.raises(ValueError): + module.get_per_hook_database_configurations(hooks, names, dump_patterns)