From afdf831c590d6578a6775b3ed9bb94a0a96aa205 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sun, 17 Nov 2024 11:55:10 -0800 Subject: [PATCH] Fix broken restore/bootstrap when using Borg 1.2 and a randomly named temporary directory (#934). --- borgmatic/actions/config/bootstrap.py | 6 ++-- borgmatic/actions/restore.py | 2 +- borgmatic/config/paths.py | 19 +++++++++++- tests/unit/actions/config/test_bootstrap.py | 32 ++++++++++++++++++--- tests/unit/actions/test_restore.py | 15 ++++++++++ tests/unit/config/test_paths.py | 13 +++++++++ 6 files changed, 79 insertions(+), 8 deletions(-) diff --git a/borgmatic/actions/config/bootstrap.py b/borgmatic/actions/config/bootstrap.py index d49ccd1f..17cf60f5 100644 --- a/borgmatic/actions/config/bootstrap.py +++ b/borgmatic/actions/config/bootstrap.py @@ -49,10 +49,12 @@ def get_config_paths(archive_name, bootstrap_arguments, global_arguments, local_ ) as borgmatic_runtime_directory: for base_directory in ( 'borgmatic', - borgmatic_runtime_directory, + borgmatic.config.paths.make_runtime_directory_glob(borgmatic_runtime_directory), borgmatic_source_directory, ): - borgmatic_manifest_path = os.path.join(base_directory, 'bootstrap', 'manifest.json') + borgmatic_manifest_path = 'sh:' + os.path.join( + base_directory, 'bootstrap', 'manifest.json' + ) extract_process = borgmatic.borg.extract.extract_archive( global_arguments.dry_run, diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index 7856e12b..0010b250 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -209,7 +209,7 @@ def collect_archive_data_source_names( + borgmatic.hooks.dump.make_data_source_dump_path(base_directory, '*_databases/*/*') for base_directory in ( 'borgmatic', - borgmatic_runtime_directory.lstrip('/'), + borgmatic.config.paths.make_runtime_directory_glob(borgmatic_runtime_directory), borgmatic_source_directory.lstrip('/'), ) ], diff --git a/borgmatic/config/paths.py b/borgmatic/config/paths.py index ec4f1db2..0b13e65d 100644 --- a/borgmatic/config/paths.py +++ b/borgmatic/config/paths.py @@ -30,6 +30,9 @@ def get_borgmatic_source_directory(config): return expand_user_in_path(config.get('borgmatic_source_directory') or '~/.borgmatic') +TEMPORARY_DIRECTORY_PREFIX = 'borgmatic-' + + class Runtime_directory: ''' A Python context manager for creating and cleaning up the borgmatic runtime directory used for @@ -72,7 +75,7 @@ class Runtime_directory: base_directory = os.environ.get('TMPDIR') or os.environ.get('TEMP') or '/tmp' os.makedirs(base_directory, mode=0o700, exist_ok=True) self.temporary_directory = tempfile.TemporaryDirectory( - prefix='borgmatic-', + prefix=TEMPORARY_DIRECTORY_PREFIX, dir=base_directory, ) runtime_directory = self.temporary_directory.name @@ -102,6 +105,20 @@ class Runtime_directory: self.temporary_directory.cleanup() +def make_runtime_directory_glob(borgmatic_runtime_directory): + ''' + Given a borgmatic runtime directory path, make a glob that would match that path, specifically + replacing any randomly generated temporary subdirectory with "*" since such a directory's name + changes on every borgmatic run. + ''' + return os.path.join( + *( + '*' if subdirectory.startswith(TEMPORARY_DIRECTORY_PREFIX) else subdirectory + for subdirectory in os.path.normpath(borgmatic_runtime_directory).split(os.path.sep) + ) + ) + + def get_borgmatic_state_directory(config): ''' Given a configuration dict, get the borgmatic state directory used for storing borgmatic state diff --git a/tests/unit/actions/config/test_bootstrap.py b/tests/unit/actions/config/test_bootstrap.py index 39e759f0..7a21e715 100644 --- a/tests/unit/actions/config/test_bootstrap.py +++ b/tests/unit/actions/config/test_bootstrap.py @@ -33,6 +33,9 @@ def test_get_config_paths_returns_list_of_config_paths(): flexmock(module.borgmatic.config.paths).should_receive('Runtime_directory').and_return( flexmock() ) + flexmock(module.borgmatic.config.paths).should_receive( + 'make_runtime_directory_glob' + ).replace_with(lambda path: path) extract_process = flexmock( stdout=flexmock( read=lambda: '{"config_paths": ["/borgmatic/config.yaml"]}', @@ -69,15 +72,18 @@ def test_get_config_paths_probes_for_manifest(): flexmock(module.borgmatic.config.paths).should_receive('Runtime_directory').and_return( borgmatic_runtime_directory, ) + flexmock(module.borgmatic.config.paths).should_receive( + 'make_runtime_directory_glob' + ).replace_with(lambda path: path) flexmock(module.os.path).should_receive('join').with_args( 'borgmatic', 'bootstrap', 'manifest.json' - ).and_return(flexmock()).once() + ).and_return('borgmatic/bootstrap/manifest.json').once() flexmock(module.os.path).should_receive('join').with_args( borgmatic_runtime_directory, 'bootstrap', 'manifest.json' - ).and_return(flexmock()).once() + ).and_return('run/borgmatic/bootstrap/manifest.json').once() flexmock(module.os.path).should_receive('join').with_args( '/source', 'bootstrap', 'manifest.json' - ).and_return(flexmock()).once() + ).and_return('/source/bootstrap/manifest.json').once() manifest_missing_extract_process = flexmock( stdout=flexmock(read=lambda: None), ) @@ -117,6 +123,9 @@ def test_get_config_paths_translates_ssh_command_argument_to_config(): flexmock(module.borgmatic.config.paths).should_receive('Runtime_directory').and_return( flexmock() ) + flexmock(module.borgmatic.config.paths).should_receive( + 'make_runtime_directory_glob' + ).replace_with(lambda path: path) extract_process = flexmock( stdout=flexmock( read=lambda: '{"config_paths": ["/borgmatic/config.yaml"]}', @@ -161,7 +170,10 @@ def test_get_config_paths_with_missing_manifest_raises_value_error(): flexmock(module.borgmatic.config.paths).should_receive('Runtime_directory').and_return( flexmock() ) - flexmock(module.os.path).should_receive('join').and_return(flexmock()) + flexmock(module.borgmatic.config.paths).should_receive( + 'make_runtime_directory_glob' + ).replace_with(lambda path: path) + flexmock(module.os.path).should_receive('join').and_return('run/borgmatic') extract_process = flexmock(stdout=flexmock(read=lambda: '')) flexmock(module.borgmatic.borg.extract).should_receive('extract_archive').and_return( extract_process @@ -194,6 +206,9 @@ def test_get_config_paths_with_broken_json_raises_value_error(): flexmock(module.borgmatic.config.paths).should_receive('Runtime_directory').and_return( flexmock() ) + flexmock(module.borgmatic.config.paths).should_receive( + 'make_runtime_directory_glob' + ).replace_with(lambda path: path) extract_process = flexmock( stdout=flexmock(read=lambda: '{"config_paths": ["/oops'), ) @@ -228,6 +243,9 @@ def test_get_config_paths_with_json_missing_key_raises_value_error(): flexmock(module.borgmatic.config.paths).should_receive('Runtime_directory').and_return( flexmock() ) + flexmock(module.borgmatic.config.paths).should_receive( + 'make_runtime_directory_glob' + ).replace_with(lambda path: path) extract_process = flexmock( stdout=flexmock(read=lambda: '{}'), ) @@ -262,6 +280,9 @@ def test_run_bootstrap_does_not_raise(): flexmock(module.borgmatic.config.paths).should_receive('Runtime_directory').and_return( flexmock() ) + flexmock(module.borgmatic.config.paths).should_receive( + 'make_runtime_directory_glob' + ).replace_with(lambda path: path) extract_process = flexmock( stdout=flexmock( read=lambda: '{"config_paths": ["borgmatic/config.yaml"]}', @@ -299,6 +320,9 @@ def test_run_bootstrap_translates_ssh_command_argument_to_config(): flexmock(module.borgmatic.config.paths).should_receive('Runtime_directory').and_return( flexmock() ) + flexmock(module.borgmatic.config.paths).should_receive( + 'make_runtime_directory_glob' + ).replace_with(lambda path: path) extract_process = flexmock( stdout=flexmock( read=lambda: '{"config_paths": ["borgmatic/config.yaml"]}', diff --git a/tests/unit/actions/test_restore.py b/tests/unit/actions/test_restore.py index a9d5cb88..e12c392a 100644 --- a/tests/unit/actions/test_restore.py +++ b/tests/unit/actions/test_restore.py @@ -471,6 +471,9 @@ def test_run_restore_restores_each_data_source(): flexmock(module.borgmatic.config.paths).should_receive('Runtime_directory').and_return( borgmatic_runtime_directory ) + flexmock(module.borgmatic.config.paths).should_receive( + 'make_runtime_directory_glob' + ).replace_with(lambda path: path) flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hooks_even_if_unconfigured') flexmock(module.borgmatic.borg.repo_list).should_receive('resolve_archive_name').and_return( flexmock() @@ -536,6 +539,9 @@ def test_run_restore_bails_for_non_matching_repository(): flexmock(module.borgmatic.config.paths).should_receive('Runtime_directory').and_return( flexmock() ) + flexmock(module.borgmatic.config.paths).should_receive( + 'make_runtime_directory_glob' + ).replace_with(lambda path: path) flexmock(module.borgmatic.hooks.dispatch).should_receive( 'call_hooks_even_if_unconfigured' ).never() @@ -562,6 +568,9 @@ def test_run_restore_restores_data_source_configured_with_all_name(): flexmock(module.borgmatic.config.paths).should_receive('Runtime_directory').and_return( borgmatic_runtime_directory ) + flexmock(module.borgmatic.config.paths).should_receive( + 'make_runtime_directory_glob' + ).replace_with(lambda path: path) flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hooks_even_if_unconfigured') flexmock(module.borgmatic.borg.repo_list).should_receive('resolve_archive_name').and_return( flexmock() @@ -646,6 +655,9 @@ def test_run_restore_skips_missing_data_source(): flexmock(module.borgmatic.config.paths).should_receive('Runtime_directory').and_return( borgmatic_runtime_directory ) + flexmock(module.borgmatic.config.paths).should_receive( + 'make_runtime_directory_glob' + ).replace_with(lambda path: path) flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hooks_even_if_unconfigured') flexmock(module.borgmatic.borg.repo_list).should_receive('resolve_archive_name').and_return( flexmock() @@ -731,6 +743,9 @@ def test_run_restore_restores_data_sources_from_different_hooks(): flexmock(module.borgmatic.config.paths).should_receive('Runtime_directory').and_return( borgmatic_runtime_directory ) + flexmock(module.borgmatic.config.paths).should_receive( + 'make_runtime_directory_glob' + ).replace_with(lambda path: path) flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hooks_even_if_unconfigured') flexmock(module.borgmatic.borg.repo_list).should_receive('resolve_archive_name').and_return( flexmock() diff --git a/tests/unit/config/test_paths.py b/tests/unit/config/test_paths.py index d121f952..36a8fb50 100644 --- a/tests/unit/config/test_paths.py +++ b/tests/unit/config/test_paths.py @@ -1,3 +1,4 @@ +import pytest from flexmock import flexmock from borgmatic.config import paths as module @@ -153,6 +154,18 @@ def test_runtime_directory_falls_back_to_hard_coded_tmp_path_and_adds_temporary_ assert borgmatic_runtime_directory == '/tmp/borgmatic-1234/./borgmatic' +@pytest.mark.parametrize( + 'borgmatic_runtime_directory,expected_glob', + ( + ('/foo/bar/baz/./borgmatic', 'foo/bar/baz/borgmatic'), + ('/foo/borgmatic/baz/./borgmatic', 'foo/borgmatic/baz/borgmatic'), + ('/foo/borgmatic-jti8idds/./borgmatic', 'foo/*/borgmatic'), + ), +) +def test_make_runtime_directory_glob(borgmatic_runtime_directory, expected_glob): + assert module.make_runtime_directory_glob(borgmatic_runtime_directory) == expected_glob + + def test_get_borgmatic_state_directory_uses_config_option(): flexmock(module).should_receive('expand_user_in_path').replace_with(lambda path: path) flexmock(module.os.environ).should_receive('get').never()