diff --git a/NEWS b/NEWS index 5e601436..cb1b43ec 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,8 @@ * #1114: Document systemd configuration changes for the ZFS filesystem hook. * #1118: Fix a bug in which Borg hangs during database backup when different filesystems are in use. + * #1122: To prevent the user from inadvertently excluding the "bootstrap" action's manifest, always + error and exit when the borgmatic runtime directory overlaps with the configured excludes. * #1125: Clarify documentation about ZFS, Btrfs, and LVM snapshotting when a separate filesystem is mounted in the source directory. (Spoiler: The separate filesystem doesn't get included in the snapshot.) diff --git a/borgmatic/borg/create.py b/borgmatic/borg/create.py index 2f0bf842..2d7d58d5 100644 --- a/borgmatic/borg/create.py +++ b/borgmatic/borg/create.py @@ -45,28 +45,26 @@ def any_parent_directories(path, candidate_parents): return False -def collect_special_file_paths( +def validate_planned_backup_paths( dry_run, create_command, config, + patterns, local_path, working_directory, borgmatic_runtime_directory, ): ''' Given a dry-run flag, a Borg create command as a tuple, a configuration dict, a local Borg path, - a working directory, and the borgmatic runtime directory, collect the paths for any special - files (character devices, block devices, and named pipes / FIFOs) that Borg would encounter - during a create. These are all paths that could cause Borg to hang if its --read-special flag is - used. + a working directory, and the borgmatic runtime directory, perform a "borg create --dry-run" to + determine whether Borg's planned paths to include in a backup look good. Specifically, if the + given runtime directory exists, validate that it will be included in a backup and hasn't been + excluded. - Skip looking for special files in the given borgmatic runtime directory, as borgmatic creates - its own special files there for database dumps and we don't want those omitted. - - Additionally, if the borgmatic runtime directory is not contained somewhere in the files Borg - plans to backup, that means the user must have excluded the runtime directory (e.g. via - "exclude_patterns" or similar). Therefore, raise, because this means Borg won't be able to - consume any database dumps and therefore borgmatic will hang when it tries to do so. + Raise ValueError if the runtime directory has been excluded via "exclude_patterns" or similar, + because any features that rely on the runtime directory getting backed up will break. For + instance, without the runtime directory, Borg can't consume any database dumps and borgmatic may + hang waiting for them to be consumed. ''' # Omit "--exclude-nodump" from the Borg dry run command, because that flag causes Borg to open # files including any named pipe we've created. And omit "--filter" because that can break the @@ -94,27 +92,33 @@ def collect_special_file_paths( if path_line and path_line.startswith(('- ', '+ ')) ) - # These are the subset of those files that contain the borgmatic runtime directory. - paths_containing_runtime_directory = {} + # These are the subset of output paths contained within the borgmatic runtime directory. + paths_inside_runtime_directory = { + path for path in paths if any_parent_directories(path, (borgmatic_runtime_directory,)) + } - if os.path.exists(borgmatic_runtime_directory): - paths_containing_runtime_directory = { - path for path in paths if any_parent_directories(path, (borgmatic_runtime_directory,)) - } - - # If no paths to backup contain the runtime directory, it must've been excluded. - if not paths_containing_runtime_directory and not dry_run: - raise ValueError( - f'The runtime directory {os.path.normpath(borgmatic_runtime_directory)} overlaps with the configured excludes or patterns with excludes. Please ensure the runtime directory is not excluded.', - ) - - return tuple( - path - for path in paths - if special_file(path, working_directory) - if path not in paths_containing_runtime_directory + # If the runtime directory isn't present in the source patterns, then we shouldn't expect it to + # be in the paths output from the Borg dry run. + runtime_directory_present_in_patterns = any( + pattern + for pattern in patterns + if any_parent_directories(pattern.path, (borgmatic_runtime_directory,)) + if pattern.type == borgmatic.borg.pattern.Pattern_type.ROOT ) + # If no paths to backup are inside the runtime directory, it must've been excluded. + if ( + not paths_inside_runtime_directory + and runtime_directory_present_in_patterns + and not dry_run + and os.path.exists(borgmatic_runtime_directory) + ): + raise ValueError( + f'The runtime directory {os.path.normpath(borgmatic_runtime_directory)} overlaps with the configured excludes or patterns with excludes. Please ensure the runtime directory is not excluded.', + ) + + return tuple(path for path in paths if path not in paths_inside_runtime_directory) + MAX_SPECIAL_FILE_PATHS_LENGTH = 1000 @@ -228,6 +232,18 @@ def make_base_create_command( archive_name_format, local_borg_version, ) + working_directory = borgmatic.config.paths.get_working_directory(config) + + logger.debug('Checking file paths Borg plans to include') + planned_backup_paths = validate_planned_backup_paths( + dry_run, + create_flags + create_positional_arguments, + config, + patterns, + local_path, + working_directory, + borgmatic_runtime_directory=borgmatic_runtime_directory, + ) # If database hooks are enabled (as indicated by streaming processes), exclude files that might # cause Borg to hang. But skip this if the user has explicitly set the "read_special" to True. @@ -235,16 +251,9 @@ def make_base_create_command( logger.warning( 'Ignoring configured "read_special" value of false, as true is needed for database hooks.', ) - working_directory = borgmatic.config.paths.get_working_directory(config) - logger.debug('Collecting special file paths') - special_file_paths = collect_special_file_paths( - dry_run, - create_flags + create_positional_arguments, - config, - local_path, - working_directory, - borgmatic_runtime_directory=borgmatic_runtime_directory, + special_file_paths = tuple( + path for path in planned_backup_paths if special_file(path, working_directory) ) if special_file_paths: diff --git a/tests/end-to-end/test_borgmatic.py b/tests/end-to-end/test_borgmatic.py index e16fdb09..766c39a9 100644 --- a/tests/end-to-end/test_borgmatic.py +++ b/tests/end-to-end/test_borgmatic.py @@ -85,7 +85,7 @@ def test_borgmatic_command(generate_configuration): ) # Run borgmatic to generate a backup archive, and then list it to make sure it exists. - subprocess.check_call(f'borgmatic --config {config_path}'.split(' ')) + subprocess.check_call(f'borgmatic -v 2 --config {config_path}'.split(' ')) output = subprocess.check_output( f'borgmatic --config {config_path} list --json'.split(' '), ).decode(sys.stdout.encoding) diff --git a/tests/unit/borg/test_create.py b/tests/unit/borg/test_create.py index 08532317..b200a1c8 100644 --- a/tests/unit/borg/test_create.py +++ b/tests/unit/borg/test_create.py @@ -59,7 +59,7 @@ def test_any_parent_directories_treats_unrelated_paths_as_non_match(): module.any_parent_directories('/foo/bar.txt', ('/usr', '/etc')) -def test_collect_special_file_paths_parses_special_files_from_borg_dry_run_file_list(): +def test_validate_planned_backup_paths_parses_borg_dry_run_file_list(): flexmock(module.flags).should_receive('omit_flag').replace_with( lambda arguments, flag: arguments, ) @@ -70,21 +70,25 @@ def test_collect_special_file_paths_parses_special_files_from_borg_dry_run_file_ flexmock(module).should_receive('execute_command_and_capture_output').and_return( 'Processing files ...\n- /foo\n+ /bar\n- /baz', ) - flexmock(module).should_receive('special_file').and_return(True) flexmock(module.os.path).should_receive('exists').and_return(False) - flexmock(module).should_receive('any_parent_directories').never() + flexmock(module).should_receive('any_parent_directories').and_return(False) - assert module.collect_special_file_paths( + assert module.validate_planned_backup_paths( dry_run=False, create_command=('borg', 'create'), config={}, + patterns=( + module.borgmatic.borg.pattern.Pattern('/foo'), + module.borgmatic.borg.pattern.Pattern('/bar'), + module.borgmatic.borg.pattern.Pattern('/baz'), + ), local_path=None, working_directory=None, borgmatic_runtime_directory='/run/borgmatic', ) == ('/foo', '/bar', '/baz') -def test_collect_special_file_paths_skips_borgmatic_runtime_directory(): +def test_validate_planned_backup_paths_skips_borgmatic_runtime_directory(): flexmock(module.flags).should_receive('omit_flag').replace_with( lambda arguments, flag: arguments, ) @@ -95,32 +99,29 @@ def test_collect_special_file_paths_skips_borgmatic_runtime_directory(): flexmock(module).should_receive('execute_command_and_capture_output').and_return( '+ /foo\n- /run/borgmatic/bar\n- /baz', ) - flexmock(module).should_receive('special_file').and_return(True) flexmock(module.os.path).should_receive('exists').and_return(True) - flexmock(module).should_receive('any_parent_directories').with_args( - '/foo', - ('/run/borgmatic',), - ).and_return(False) - flexmock(module).should_receive('any_parent_directories').with_args( - '/run/borgmatic/bar', - ('/run/borgmatic',), - ).and_return(True) - flexmock(module).should_receive('any_parent_directories').with_args( - '/baz', - ('/run/borgmatic',), - ).and_return(False) + flexmock(module).should_receive('any_parent_directories').replace_with( + lambda path, _: path == '/run/borgmatic/bar' + ) - assert module.collect_special_file_paths( + assert module.validate_planned_backup_paths( dry_run=False, create_command=('borg', 'create'), config={}, + patterns=( + module.borgmatic.borg.pattern.Pattern('/foo'), + module.borgmatic.borg.pattern.Pattern( + '/run/borgmatic/bar', module.borgmatic.borg.pattern.Pattern_type.ROOT + ), + module.borgmatic.borg.pattern.Pattern('/baz'), + ), local_path=None, working_directory=None, borgmatic_runtime_directory='/run/borgmatic', ) == ('/foo', '/baz') -def test_collect_special_file_paths_with_borgmatic_runtime_directory_missing_from_paths_output_errors(): +def test_validate_planned_backup_paths_with_borgmatic_runtime_directory_missing_from_paths_output_errors(): flexmock(module.flags).should_receive('omit_flag').replace_with( lambda arguments, flag: arguments, ) @@ -131,22 +132,30 @@ def test_collect_special_file_paths_with_borgmatic_runtime_directory_missing_fro flexmock(module).should_receive('execute_command_and_capture_output').and_return( '+ /foo\n- /bar\n- /baz', ) - flexmock(module).should_receive('special_file').and_return(True) flexmock(module.os.path).should_receive('exists').and_return(True) - flexmock(module).should_receive('any_parent_directories').and_return(False) + flexmock(module).should_receive('any_parent_directories').replace_with( + lambda path, _: path == '/run/borgmatic/bar' + ) with pytest.raises(ValueError): - module.collect_special_file_paths( + module.validate_planned_backup_paths( dry_run=False, create_command=('borg', 'create'), config={}, + patterns=( + module.borgmatic.borg.pattern.Pattern('/foo'), + module.borgmatic.borg.pattern.Pattern( + '/run/borgmatic/bar', module.borgmatic.borg.pattern.Pattern_type.ROOT + ), + module.borgmatic.borg.pattern.Pattern('/baz'), + ), local_path=None, working_directory=None, borgmatic_runtime_directory='/run/borgmatic', ) -def test_collect_special_file_paths_with_dry_run_and_borgmatic_runtime_directory_missing_from_paths_output_does_not_raise(): +def test_validate_planned_backup_paths_with_borgmatic_runtime_directory_missing_from_patterns_does_not_raise(): flexmock(module.flags).should_receive('omit_flag').replace_with( lambda arguments, flag: arguments, ) @@ -155,49 +164,58 @@ def test_collect_special_file_paths_with_dry_run_and_borgmatic_runtime_directory ) flexmock(module.environment).should_receive('make_environment').and_return(None) flexmock(module).should_receive('execute_command_and_capture_output').and_return( - '+ /foo\n- /bar\n- /baz', + '+ /foo\n- /run/borgmatic/bar\n- /baz', ) - flexmock(module).should_receive('special_file').and_return(True) flexmock(module.os.path).should_receive('exists').and_return(True) - flexmock(module).should_receive('any_parent_directories').and_return(False) - - assert module.collect_special_file_paths( - dry_run=True, - create_command=('borg', 'create'), - config={}, - local_path=None, - working_directory=None, - borgmatic_runtime_directory='/run/borgmatic', - ) == ('/foo', '/bar', '/baz') - - -def test_collect_special_file_paths_excludes_non_special_files(): - flexmock(module.flags).should_receive('omit_flag').replace_with( - lambda arguments, flag: arguments, + flexmock(module).should_receive('any_parent_directories').replace_with( + lambda path, _: path == '/run/borgmatic/bar' ) - flexmock(module.flags).should_receive('omit_flag_and_value').replace_with( - lambda arguments, flag: arguments, - ) - flexmock(module.environment).should_receive('make_environment').and_return(None) - flexmock(module).should_receive('execute_command_and_capture_output').and_return( - '+ /foo\n+ /bar\n+ /baz', - ) - flexmock(module).should_receive('special_file').and_return(True).and_return(False).and_return( - True, - ) - flexmock(module.os.path).should_receive('exists').and_return(False) - flexmock(module).should_receive('any_parent_directories').never() - assert module.collect_special_file_paths( + assert module.validate_planned_backup_paths( dry_run=False, create_command=('borg', 'create'), config={}, + patterns=( + module.borgmatic.borg.pattern.Pattern('/foo'), + module.borgmatic.borg.pattern.Pattern('/baz'), + ), local_path=None, working_directory=None, borgmatic_runtime_directory='/run/borgmatic', ) == ('/foo', '/baz') +def test_validate_planned_backup_paths_with_dry_run_and_borgmatic_runtime_directory_missing_from_paths_output_does_not_raise(): + flexmock(module.flags).should_receive('omit_flag').replace_with( + lambda arguments, flag: arguments, + ) + flexmock(module.flags).should_receive('omit_flag_and_value').replace_with( + lambda arguments, flag: arguments, + ) + flexmock(module.environment).should_receive('make_environment').and_return(None) + flexmock(module).should_receive('execute_command_and_capture_output').and_return( + '+ /foo\n- /run/borgmatic/bar\n- /baz', + ) + flexmock(module.os.path).should_receive('exists').and_return(True) + flexmock(module).should_receive('any_parent_directories').and_return(False) + + assert module.validate_planned_backup_paths( + dry_run=True, + create_command=('borg', 'create'), + config={}, + patterns=( + module.borgmatic.borg.pattern.Pattern('/foo'), + module.borgmatic.borg.pattern.Pattern( + '/run/borgmatic/bar', module.borgmatic.borg.pattern.Pattern_type.ROOT + ), + module.borgmatic.borg.pattern.Pattern('/baz'), + ), + local_path=None, + working_directory=None, + borgmatic_runtime_directory='/run/borgmatic', + ) == ('/foo', '/run/borgmatic/bar', '/baz') + + DEFAULT_ARCHIVE_NAME = '{hostname}-{now:%Y-%m-%dT%H:%M:%S.%f}' REPO_ARCHIVE = (f'repo::{DEFAULT_ARCHIVE_NAME}',) @@ -211,6 +229,7 @@ def test_make_base_create_produces_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -246,6 +265,7 @@ def test_make_base_create_command_includes_patterns_file_in_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -278,6 +298,7 @@ def test_make_base_create_command_with_store_config_false_omits_config_files(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -346,6 +367,7 @@ def test_make_base_create_command_includes_configuration_option_as_command_flag( flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -378,6 +400,7 @@ def test_make_base_create_command_includes_dry_run_in_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=True, @@ -410,6 +433,7 @@ def test_make_base_create_command_includes_comment_in_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -443,6 +467,7 @@ def test_make_base_create_command_includes_local_path_in_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -475,6 +500,7 @@ def test_make_base_create_command_includes_remote_path_in_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -507,6 +533,7 @@ def test_make_base_create_command_includes_log_json_in_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -539,6 +566,7 @@ def test_make_base_create_command_includes_list_flags_in_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -578,7 +606,18 @@ def test_make_base_create_command_with_stream_processes_ignores_read_special_fal ) flexmock(module.logger).should_receive('warning').twice() flexmock(module.environment).should_receive('make_environment') - flexmock(module).should_receive('collect_special_file_paths').and_return(('/dev/null',)).once() + flexmock(module).should_receive('validate_planned_backup_paths').and_return( + ( + '/non/special', + '/dev/null', + ) + ) + flexmock(module).should_receive('special_file').with_args( + '/non/special', working_directory=None + ).and_return(False) + flexmock(module).should_receive('special_file').with_args( + '/dev/null', working_directory=None + ).and_return(True) flexmock(module.borgmatic.borg.pattern).should_receive('write_patterns_file').with_args( ( Pattern( @@ -630,7 +669,18 @@ def test_make_base_create_command_without_patterns_and_with_stream_processes_ign ) flexmock(module.logger).should_receive('warning').twice() flexmock(module.environment).should_receive('make_environment') - flexmock(module).should_receive('collect_special_file_paths').and_return(('/dev/null',)).once() + flexmock(module).should_receive('validate_planned_backup_paths').and_return( + ( + '/non/special', + '/dev/null', + ) + ) + flexmock(module).should_receive('special_file').with_args( + '/non/special', working_directory=None + ).and_return(False) + flexmock(module).should_receive('special_file').with_args( + '/dev/null', working_directory=None + ).and_return(True) flexmock(module.borgmatic.borg.pattern).should_receive('write_patterns_file').with_args( ( Pattern( @@ -678,7 +728,7 @@ def test_make_base_create_command_with_stream_processes_and_read_special_true_sk (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) flexmock(module.logger).should_receive('warning').never() - flexmock(module).should_receive('collect_special_file_paths').never() + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -712,6 +762,7 @@ def test_make_base_create_command_includes_archive_name_format_in_borg_command() flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( ('repo::ARCHIVE_NAME',), ) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -744,6 +795,7 @@ def test_make_base_create_command_includes_default_archive_name_format_in_borg_c flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( ('repo::{hostname}',), ) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -775,6 +827,7 @@ def test_make_base_create_command_includes_archive_name_format_with_placeholders flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (repository_archive_pattern,), ) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -807,6 +860,7 @@ def test_make_base_create_command_includes_repository_and_archive_name_format_wi flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (repository_archive_pattern,), ) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -835,6 +889,7 @@ def test_make_base_create_command_includes_archive_suffix_in_borg_command(): DEFAULT_ARCHIVE_NAME, ) flexmock(module.borgmatic.borg.flags).should_receive('make_exclude_flags').and_return(()) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -867,6 +922,7 @@ def test_make_base_create_command_includes_extra_borg_options_in_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False,