Browse Source

Fix for "borgmatic restore" showing success and incorrectly extracting archive files, even when no databases are configured to restore (#246).

Dan Helfman 5 years ago
parent
commit
6cdc92bd0c
6 changed files with 66 additions and 4 deletions
  1. 1 1
      AUTHORS
  2. 4 1
      NEWS
  3. 2 1
      borgmatic/commands/borgmatic.py
  4. 25 0
      borgmatic/hooks/dump.py
  5. 1 1
      setup.py
  6. 33 0
      tests/unit/hooks/test_dump.py

+ 1 - 1
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".

+ 4 - 1
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.
 

+ 2 - 1
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,

+ 25 - 0
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 = {

+ 1 - 1
setup.py

@@ -1,6 +1,6 @@
 from setuptools import find_packages, setup
 
-VERSION = '1.4.10.dev0'
+VERSION = '1.4.10'
 
 
 setup(

+ 33 - 0
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)