diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md deleted file mode 100644 index 6256e53a..00000000 --- a/.github/pull_request_template.md +++ /dev/null @@ -1,10 +0,0 @@ -## Hold up, GitHub users - -Thanks for your contribution! - -Unfortunately, we don't use GitHub pull requests to manage code contributions to this repository (and GitHub doesn't have any way to disable pull requests entirely). Instead, please see: - -https://torsion.org/borgmatic/#contributing - -... which provides full instructions on how to submit pull requests. You can even use your GitHub account to login. - diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 00000000..a6f7c7bb --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,28 @@ +# AGENTS.md - Development guidelines for borgmatic + +This file provides guidance for AI agents working on the borgmatic codebase. + +## Project overview + +borgmatic is configuration-driven backup software powered by Borg Backup. It's a +Python project using setuptools. + +Please do not use AI agents to modify this codebase. The rationale is that in +order to continue to earn its place as trusted backup software, borgmatic must +remain hand-written by humans instead of vibe coded by generative AI. + +Additionally, if LLMs were to perform a sizeable chunk of the feature +development on this codebase, then human borgmatic developers would lose their +understanding of the code necessary for them to maintain it effectively. + +Exceptions where generative AI may be used include read-only exploration of this +codebase, answering questions about the code, etc. + +## Architecture notes + +- **main entry point**: `borgmatic.commands.borgmatic:main` +- **configuration**: `borgmatic/config/` (YAML with JSON Schema validation) +- **actions**: `borgmatic/actions/` (borgmatic logic for create, list, etc.) +- **Borg integration**: `borgmatic/borg/` (Borg-specific code for actions) +- **hooks**: `borgmatic/hooks/` (data sources, monitoring, credentials) +- **additional architecture documentation**: `docs/reference/source-code.md` diff --git a/NEWS b/NEWS index f821f5c3..c4f89344 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,29 @@ -2.1.2.dev0 +2.1.3.dev0 + * #1218: Add a "config show" action to display computed borgmatic configuration as YAML or JSON, + handy for fetching borgmatic configuration from external scripts. See the documentation for more + information: + https://torsion.org/borgmatic/reference/command-line/actions/config-show/ + * #1228: Adjust the "spot" check so error output includes more information about what failed. + * #1236: Fix the "spot" check to skip hard links, as Borg doesn't produces hashes for them. + * #1269: Fix the ZFS hook to support datasets with a "canmount" property of "noauto". + * #1270: Follow symlinks when backing up borgmatic configuration files to support the "bootstrap" + action. + * Add a policy about the use of generative AI in the borgmatic codebase: + https://torsion.org/borgmatic/how-to/develop-on-borgmatic/#use-of-generative-ai + +2.1.2 + * #1231: If a source file is deleted during a "spot" check, consider the file as non-matching + and move on instead of immediately failing the entire check. * #1250: Fix a regression in which the "--stats" flag hides statistics at default verbosity. + * #1251: Fix a regression in the ntfy monitoring hook in which borgmatic sends tags incorrectly, + resulting in "400 Bad Request" from ntfy. + * #1258: Fix a "codec can't decode byte" error when running commands that output multi-byte unicode + characters. + * #1260: Fix for SSH warnings from Borg showing up as JSON logs even without the "--log-json" flag. + * #1252: Work around Borg returning a warning exit code when a repository/archive check fails. Now, + borgmatic interprets such failures as errors. + * Deduplicate overlapping source directories and patterns so they don't throw off "spot" check file + counts and cause spurious check failures. 2.1.1 * #1241: For the "recreate" action, actually pass the "--dry-run" flag through to Borg instead of diff --git a/borgmatic/actions/check.py b/borgmatic/actions/check.py index e6390598..92a523c2 100644 --- a/borgmatic/actions/check.py +++ b/borgmatic/actions/check.py @@ -9,6 +9,7 @@ import pathlib import random import shlex import shutil +import subprocess import textwrap import borgmatic.actions.config.bootstrap @@ -418,7 +419,13 @@ def collect_spot_check_source_paths( ) return tuple( - path for path in paths if os.path.isfile(os.path.join(working_directory or '', path)) + # Use dict.fromkeys() to deduplicate file paths, which are present in Borg's dry run output + # when there are overlapping source patterns. For instance, if both "/foo" and + # "/foo/file.txt" are in configured patterns, then "/foo/file.txt" will show up in Borg's + # dry run output twice. + dict.fromkeys( + path for path in paths if os.path.isfile(os.path.join(working_directory or '', path)) + ) ) @@ -518,66 +525,100 @@ def compare_spot_check_hashes( source_sample_paths_subset = tuple( itertools.islice(source_sample_paths_iterator, SAMPLE_PATHS_SUBSET_COUNT), ) + if not source_sample_paths_subset: break hash_paths = tuple( path for path in source_sample_paths_subset if path in hashable_source_sample_path ) - hash_lines = borgmatic.execute.execute_command_and_capture_output( - tuple( - shlex.quote(part) - for part in shlex.split(spot_check_config.get('xxh64sum_command', 'xxh64sum')) - ) - + hash_paths, - working_directory=working_directory, - ) - source_hashes.update( - **dict( - zip( - # xxh64sum rewrites/escapes the paths that it returns alongside its hashes, for - # instance if they contain special characters. When that happens, they don't - # match the original source paths and therefore hash lookups fail. So when - # building this lookup dict, use the original unaltered paths we provided as - # input to xxh64sum. - hash_paths, - ( - # For some reason, xxh64sum prefixes the hash with a backslash if the path - # contains a newline. Work around that. - line.split(' ', 1)[0].lstrip('\\') - for line in hash_lines + try: + hash_lines = borgmatic.execute.execute_command_and_capture_output( + tuple( + shlex.quote(part) + for part in shlex.split(spot_check_config.get('xxh64sum_command', 'xxh64sum')) + ) + + hash_paths, + working_directory=working_directory, + ) + source_hashes.update( + **dict( + zip( + # xxh64sum rewrites/escapes the paths that it returns alongside its hashes, for + # instance if they contain special characters. When that happens, they don't + # match the original source paths and therefore hash lookups fail. So when + # building this lookup dict, use the original unaltered paths we provided as + # input to xxh64sum. + hash_paths, + ( + # For some reason, xxh64sum prefixes the hash with a backslash if the path + # contains a newline. Work around that. + line.split(' ', 1)[0].lstrip('\\') + for line in hash_lines + ), ), + # Represent non-existent files as having empty hashes so the comparison below still + # works. Same thing for filesystem links, since Borg produces empty archive hashes + # for them. + **{ + path: '' + for path in source_sample_paths_subset + if path not in hashable_source_sample_path + }, ), - # Represent non-existent files as having empty hashes so the comparison below still - # works. Same thing for filesystem links, since Borg produces empty archive hashes - # for them. - **{ - path: '' - for path in source_sample_paths_subset - if path not in hashable_source_sample_path - }, - ), - ) + ) + except subprocess.CalledProcessError: + # This can happen if a file we planned to hash gets deleted right before we try to hash + # it. Falling back to individual file hashing allows us to find and mark just the + # file(s) with problems instead of failing the whole batch. + logger.warning( + 'Bulk source path hashing failed for this batch; falling back to individual file hashing' + ) + + for hash_path in hash_paths: + try: + hash_lines = borgmatic.execute.execute_command_and_capture_output( + ( + *( + shlex.quote(part) + for part in shlex.split( + spot_check_config.get('xxh64sum_command', 'xxh64sum') + ) + ), + hash_path, + ), + working_directory=working_directory, + ) + source_hashes[hash_path] = next(hash_lines).split(' ', 1)[0].lstrip('\\') + except (subprocess.CalledProcessError, StopIteration): # noqa: PERF203 + logger.warning( + f'Source path hashing failed for {hash_path}; treating as missing' + ) + source_hashes[hash_path] = '' # Get the hash for each file in the archive. - archive_hashes.update( - **{ - entry['path']: entry['xxh64'] - for entry in borgmatic.borg.list.capture_archive_listing( - repository['path'], - archive, - config, - local_borg_version, - global_arguments, - list_paths=source_sample_paths_subset, - path_format='{xxh64}{path}', - local_path=local_path, - remote_path=remote_path, - ) - if entry - }, - ) + for entry in borgmatic.borg.list.capture_archive_listing( + repository['path'], + archive, + config, + local_borg_version, + global_arguments, + list_paths=source_sample_paths_subset, + path_format='{xxh64}{path}{linktarget}', + local_path=local_path, + remote_path=remote_path, + ): + if not entry: + continue + + # Borg can't get hashes of stored hard links. So if this is a hard link path (and not + # deemed as the "original" by Borg), then skip hashing of it. + if entry['linktarget']: + source_hashes.pop(os.path.join('/', entry['path']), None) + continue + + archive_hashes[entry['path']] = entry['xxh64'] # Compare the source hashes with the archive hashes to see how many match. failing_paths = [] @@ -676,7 +717,7 @@ def spot_check( ) logger.debug(f'Paths in latest archive but not source paths: {truncated_archive_paths}') raise ValueError( - 'Spot check failed: There are no source paths to compare against the archive', + 'Spot check failed; there are no source paths to compare against the archive', ) # Calculate the percentage delta between the source paths count and the archive paths count, and @@ -690,19 +731,13 @@ def spot_check( width=MAX_SPOT_CHECK_PATHS_LENGTH, placeholder=' ...', ) - logger.debug( - f'Paths in source paths but not latest archive: {truncated_exclusive_source_paths}', - ) truncated_exclusive_archive_paths = textwrap.shorten( ', '.join(set(archive_paths) - rootless_source_paths) or 'none', width=MAX_SPOT_CHECK_PATHS_LENGTH, placeholder=' ...', ) - logger.debug( - f'Paths in latest archive but not source paths: {truncated_exclusive_archive_paths}', - ) raise ValueError( - f'Spot check failed: {count_delta_percentage:.2f}% file count delta between source paths and latest archive (tolerance is {spot_check_config["count_tolerance_percentage"]}%)', + f'Spot check failed\n{count_delta_percentage:.2f}% file count delta between source paths ({len(source_paths)} total) and latest archive ({len(archive_paths)} total); tolerance is {spot_check_config["count_tolerance_percentage"]}%\nOnly in source paths: {truncated_exclusive_source_paths}\nOnly in latest archive: {truncated_exclusive_archive_paths}', ) failing_paths = compare_spot_check_hashes( @@ -727,11 +762,8 @@ def spot_check( width=MAX_SPOT_CHECK_PATHS_LENGTH, placeholder=' ...', ) - logger.debug( - f'Source paths with data not matching the latest archive: {truncated_failing_paths}', - ) raise ValueError( - f'Spot check failed: {failing_percentage:.2f}% of source paths with data not matching the latest archive (tolerance is {data_tolerance_percentage}%)', + f'Spot check failed\n{failing_percentage:.2f}% of source paths ({len(failing_paths)} total) with data not matching the latest archive; tolerance is {data_tolerance_percentage}%\nSource paths with non-matching data: {truncated_failing_paths}', ) logger.info( diff --git a/borgmatic/actions/config/bootstrap.py b/borgmatic/actions/config/bootstrap.py index 5e035a11..a561c7b7 100644 --- a/borgmatic/actions/config/bootstrap.py +++ b/borgmatic/actions/config/bootstrap.py @@ -120,7 +120,7 @@ def run_bootstrap(bootstrap_arguments, global_arguments, local_borg_version): borgmatic_runtime_directory, ) - logger.info(f"Bootstrapping config paths: {', '.join(manifest_config_paths)}") + logger.info(f"Bootstrapping configuration paths: {', '.join(manifest_config_paths)}") borgmatic.borg.extract.extract_archive( global_arguments.dry_run, diff --git a/borgmatic/actions/config/show.py b/borgmatic/actions/config/show.py new file mode 100644 index 00000000..bfa05b0d --- /dev/null +++ b/borgmatic/actions/config/show.py @@ -0,0 +1,44 @@ +import json +import logging +import sys + +import borgmatic.config.generate +import borgmatic.logger + +logger = logging.getLogger(__name__) + + +def run_show(show_arguments, configs): + ''' + Given the show arguments as an argparse.Namespace instance and a dict of configuration filename + to corresponding parsed configuration, run the "show" action. That consists of rendering and + logging the computed configuration as YAML, separating the configuration for each file with + "---". + + If show_arguments.option is set, limit the results to the value of that single option. If + show_arguments.json is True, render the results as JSON with one array element per configuration + file. + ''' + borgmatic.logger.add_custom_log_levels() + + if show_arguments.json: + sys.stdout.write( + json.dumps( + [ + config.get(show_arguments.option) if show_arguments.option else config + for config in configs.values() + ] + ) + ) + + return + + for config in configs.values(): + if len(configs) > 1: + logger.answer('---') + + logger.answer( + borgmatic.config.generate.render_configuration( + config.get(show_arguments.option) if show_arguments.option else config + ).rstrip() + ) diff --git a/borgmatic/borg/check.py b/borgmatic/borg/check.py index ef160417..64ea23cd 100644 --- a/borgmatic/borg/check.py +++ b/borgmatic/borg/check.py @@ -149,8 +149,10 @@ def check_archives( max_duration = check_arguments.max_duration or repository_check_config.get('max_duration') + # If not configured, elevate Borg's exit code 1 (an ostensible warning) to error, because Borg + # returns exit code 1 for repository check errors! + borg_exit_codes = [*config.get('borg_exit_codes', []), *[{'code': 1, 'treat_as': 'error'}]] umask = config.get('umask') - borg_exit_codes = config.get('borg_exit_codes') working_directory = borgmatic.config.paths.get_working_directory(config) if 'data' in checks: diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index 58c3a632..73a71556 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -1261,6 +1261,29 @@ def make_parsers(schema, unparsed_arguments): # noqa: PLR0915 help='Show this help message and exit', ) + config_show_parser = config_parsers.add_parser( + 'show', + help='Show the computed configuration for each file specified with --config (see borgmatic --help)', + description='Show the computed configuration for each file specified with --config (see borgmatic --help)', + add_help=False, + ) + config_show_group = config_show_parser.add_argument_group('config show arguments') + config_show_group.add_argument( + '--option', + help='Show the value of a single named configuration option instead of the entire configuration', + ) + config_show_group.add_argument( + '--json', + action='store_true', + help='Show the configuration as JSON with one array element per configuration file', + ) + config_show_group.add_argument( + '-h', + '--help', + action='help', + help='Show this help message and exit', + ) + export_tar_parser = action_parsers.add_parser( 'export-tar', aliases=ACTION_ALIASES['export-tar'], diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 2954ac72..7ddaea8c 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -17,6 +17,7 @@ import borgmatic.actions.check import borgmatic.actions.compact import borgmatic.actions.config.bootstrap import borgmatic.actions.config.generate +import borgmatic.actions.config.show import borgmatic.actions.config.validate import borgmatic.actions.create import borgmatic.actions.delete @@ -810,18 +811,18 @@ def collect_highlander_action_summary_logs(configs, arguments, configuration_par ''' add_custom_log_levels() - if 'bootstrap' in arguments: - try: - # No configuration file is needed for bootstrap. - local_borg_version = borg_version.local_borg_version( - {}, - arguments['bootstrap'].local_path, - ) - except (OSError, CalledProcessError, ValueError) as error: - yield from log_error_records('Error getting local Borg version', error) - return + try: + if 'bootstrap' in arguments: + try: + # No configuration file is needed for bootstrap. + local_borg_version = borg_version.local_borg_version( + {}, + arguments['bootstrap'].local_path, + ) + except (OSError, CalledProcessError, ValueError) as error: + yield from log_error_records('Error getting local Borg version', error) + return - try: borgmatic.actions.config.bootstrap.run_bootstrap( arguments['bootstrap'], arguments['global'], @@ -835,17 +836,10 @@ def collect_highlander_action_summary_logs(configs, arguments, configuration_par name=logger.name, ), ) - except ( - CalledProcessError, - ValueError, - OSError, - ) as error: - yield from log_error_records(error) - return + return - if 'generate' in arguments: - try: + if 'generate' in arguments: borgmatic.actions.config.generate.run_generate( arguments['generate'], arguments['global'], @@ -858,29 +852,22 @@ def collect_highlander_action_summary_logs(configs, arguments, configuration_par name=logger.name, ), ) - except ( - CalledProcessError, - ValueError, - OSError, - ) as error: - yield from log_error_records(error) - - return - - if 'validate' in arguments: - if configuration_parse_errors: - yield logging.makeLogRecord( - dict( - levelno=logging.CRITICAL, - levelname='CRITICAL', - msg='Configuration validation failed', - name=logger.name, - ), - ) return - try: + if 'validate' in arguments: + if configuration_parse_errors: + yield logging.makeLogRecord( + dict( + levelno=logging.CRITICAL, + levelname='CRITICAL', + msg='Configuration validation failed', + name=logger.name, + ), + ) + + return + borgmatic.actions.config.validate.run_validate(arguments['validate'], configs) yield logging.makeLogRecord( @@ -891,14 +878,20 @@ def collect_highlander_action_summary_logs(configs, arguments, configuration_par name=logger.name, ), ) - except ( - CalledProcessError, - ValueError, - OSError, - ) as error: - yield from log_error_records(error) - return + return + + if 'show' in arguments: + borgmatic.actions.config.show.run_show(arguments['show'], configs) + + return + + except ( + CalledProcessError, + ValueError, + OSError, + ) as error: + yield from log_error_records(error) def collect_configuration_run_summary_logs(configs, config_paths, arguments, log_file_path): # noqa: PLR0912 diff --git a/borgmatic/config/collect.py b/borgmatic/config/collect.py index 0edf1412..c30e68f1 100644 --- a/borgmatic/config/collect.py +++ b/borgmatic/config/collect.py @@ -47,5 +47,6 @@ def collect_config_filenames(config_paths): for filename in sorted(os.listdir(path)): full_filename = os.path.join(path, filename) matching_filetype = full_filename.endswith(('.yaml', '.yml')) + if matching_filetype and not os.path.isdir(full_filename): yield os.path.abspath(full_filename) diff --git a/borgmatic/config/generate.py b/borgmatic/config/generate.py index 76d4a167..e22c452c 100644 --- a/borgmatic/config/generate.py +++ b/borgmatic/config/generate.py @@ -153,6 +153,9 @@ def transform_optional_configuration(rendered_config, comment_out=True): return '\n'.join(lines) +RUAMEL_YAML_END_OF_DOCUMENT_MARKER = '...\n' + + def render_configuration(config): ''' Given a config data structure of nested OrderedDicts, render the config as YAML and return it. @@ -160,7 +163,17 @@ def render_configuration(config): dumper = ruamel.yaml.YAML(typ='rt') dumper.indent(mapping=INDENT, sequence=INDENT + SEQUENCE_INDENT, offset=INDENT) rendered = io.StringIO() - dumper.dump(config, rendered) + dumper.dump( + config, + rendered, + # Dumping certain values (integers, for instance) causes ruamel.yaml to append an + # end-of-document "..." marker. Strip it. + transform=lambda dumped: ( + dumped[: -len(RUAMEL_YAML_END_OF_DOCUMENT_MARKER)] + if dumped.endswith(RUAMEL_YAML_END_OF_DOCUMENT_MARKER) + else dumped + ), + ) return rendered.getvalue() diff --git a/borgmatic/execute.py b/borgmatic/execute.py index 798f5226..5fde85c7 100644 --- a/borgmatic/execute.py +++ b/borgmatic/execute.py @@ -29,6 +29,19 @@ class Exit_status(enum.Enum): ERROR = 4 +def command_is_borg(command, borg_local_path): + ''' + Given a command as a sequence and the Borg local path, return whether that command is a call to + Borg. + ''' + parsed_command = command.split(' ', 1) if isinstance(command, str) else command + + if not parsed_command: + return False + + return bool(borg_local_path and parsed_command[0] == borg_local_path) + + def interpret_exit_code(command, exit_code, borg_local_path=None, borg_exit_codes=None): # noqa: PLR0911 ''' Return an Exit_status value (e.g. SUCCESS, ERROR, or WARNING) based on interpreting the given @@ -42,9 +55,7 @@ def interpret_exit_code(command, exit_code, borg_local_path=None, borg_exit_code if exit_code == 0: return Exit_status.SUCCESS - parsed_command = command.split(' ', 1) if isinstance(command, str) else command - - if borg_local_path and parsed_command[0] == borg_local_path: + if command_is_borg(command, borg_local_path): # First try looking for the exit code in the borg_exit_codes configuration. for entry in borg_exit_codes or (): if entry.get('code') == exit_code: @@ -124,7 +135,7 @@ def borg_json_log_line_to_record(line, log_level): if log_type == 'log_message': borg_log_level = logging._nameToLevel.get(log_data.get('levelname')) - log_level_delta = log_level - borg_log_level + log_level_delta = 0 if log_level is None else log_level - borg_log_level if log_level_delta > 0 and log_level_delta < BORG_LOG_LEVEL_ELEVATION_THRESHOLD: return logging.makeLogRecord( @@ -188,9 +199,7 @@ def parse_log_line(line, log_level, elevate_stderr, borg_local_path, command): came from stderr and the string "warning:" appears at the start of the log line. In that case, just elevate the log level to a WARN. ''' - parsed_command = command.split(' ', 1) if isinstance(command, str) else command - - if borg_local_path and parsed_command[0] == borg_local_path: + if command_is_borg(command, borg_local_path): log_record = borg_json_log_line_to_record(line, log_level) if log_record: @@ -237,10 +246,12 @@ def read_lines(buffer, process, line_separator='\n'): call to know when to read more lines. Otherwise, the generator will busywait if it's called in a tight loop. ''' - data = '' + data = b'' + encoded_separator = line_separator.encode() + separator_size = len(encoded_separator) while True: - chunk = os.read(buffer.fileno(), READ_CHUNK_SIZE).decode() + chunk = os.read(buffer.fileno(), READ_CHUNK_SIZE) if not chunk: # EOF # The process is still running, so we keep running too. @@ -255,19 +266,19 @@ def read_lines(buffer, process, line_separator='\n'): # Split the data into lines, holding back anything leftover that might # be a partial line. while True: - separator_position = data.find(line_separator) + separator_position = data.find(encoded_separator) if separator_position == -1: break - lines.append(data[:separator_position].rstrip()) - data = data[separator_position + 1 :] + lines.append(data[:separator_position].decode()) + data = data[separator_position + separator_size :] yield tuple(lines) # Yield any leftover data from the end of the buffer. if data: - yield (data.rstrip(),) + yield (data.decode().rstrip(),) Buffer_reader = collections.namedtuple( @@ -634,7 +645,9 @@ def execute_command_and_capture_output( command, stdin=input_file, stdout=subprocess.PIPE, - stderr=subprocess.PIPE if capture_stderr else None, + stderr=subprocess.PIPE + if capture_stderr or command_is_borg(command, borg_local_path) + else None, shell=shell, env=environment, cwd=working_directory, diff --git a/borgmatic/hooks/data_source/bootstrap.py b/borgmatic/hooks/data_source/bootstrap.py index 6765d33c..3b227554 100644 --- a/borgmatic/hooks/data_source/bootstrap.py +++ b/borgmatic/hooks/data_source/bootstrap.py @@ -1,6 +1,7 @@ import contextlib import glob import importlib +import itertools import json import logging import os @@ -19,6 +20,29 @@ def use_streaming(hook_config, config): # pragma: no cover return False +MAXIMUM_CONFIG_SYMLINKS_TO_FOLLOW = 10 + + +def resolve_config_path_symlinks(path): + ''' + Given a path, resolve and yield each successive symlink until the final non-symlink target. If + the given path isn't a symlink, then just yield it. + + Raise ValueError if we have to follow too many symlinks without getting to the final target. + ''' + original_path = path + + for _ in range(MAXIMUM_CONFIG_SYMLINKS_TO_FOLLOW): + yield os.path.abspath(path) + + if not os.path.islink(path): + return + + path = os.readlink(path) + + raise ValueError(f'Too many symlinks to follow for configuration path: {original_path}') + + def dump_data_sources( hook_config, config, @@ -34,6 +58,9 @@ def dump_data_sources( the archive. But skip this if the bootstrap store_config_files option is False or if this is a dry run. + If any configuration paths are symlinks, then store each symlink along with any destination + paths as well. + Return an empty sequence, since there are no ongoing dump processes from this hook. ''' if hook_config and hook_config.get('store_config_files') is False: @@ -45,6 +72,10 @@ def dump_data_sources( 'manifest.json', ) + resolved_config_paths = tuple( + itertools.chain.from_iterable(resolve_config_path_symlinks(path) for path in config_paths) + ) + if dry_run: return [] @@ -54,7 +85,7 @@ def dump_data_sources( json.dump( { 'borgmatic_version': importlib.metadata.version('borgmatic'), - 'config_paths': config_paths, + 'config_paths': resolved_config_paths, }, manifest_file, ) @@ -67,7 +98,7 @@ def dump_data_sources( ), ) - for config_path in config_paths: + for config_path in resolved_config_paths: borgmatic.hooks.data_source.config.inject_pattern( patterns, borgmatic.borg.pattern.Pattern( diff --git a/borgmatic/hooks/data_source/zfs.py b/borgmatic/hooks/data_source/zfs.py index 40ae25d4..f8eb840a 100644 --- a/borgmatic/hooks/data_source/zfs.py +++ b/borgmatic/hooks/data_source/zfs.py @@ -71,7 +71,7 @@ def get_datasets_to_backup(zfs_command, patterns): ) # Skip datasets that are marked "canmount=off", because mounting their snapshots will # result in completely empty mount points—thereby preventing us from backing them up. - if can_mount == 'on' + if can_mount != 'off' ), key=lambda dataset: dataset.mount_point, reverse=True, diff --git a/borgmatic/hooks/monitoring/ntfy.py b/borgmatic/hooks/monitoring/ntfy.py index 0429e703..45d5e8f8 100644 --- a/borgmatic/hooks/monitoring/ntfy.py +++ b/borgmatic/hooks/monitoring/ntfy.py @@ -22,6 +22,19 @@ def initialize_monitor( ''' +def convert_string_to_array(value): + value = '' if value is None else str(value) + items = [] + + for item in value.split(','): + stripped = item.strip() + + if stripped: + items.append(stripped) + + return items + + PRIORITY_NAME_TO_ID = { 'max': 5, 'urgent': 5, @@ -66,7 +79,7 @@ def ping_monitor(hook_config, config, config_filename, state, monitoring_log_lev 'title': state_config.get('title'), 'message': state_config.get('message'), 'priority': PRIORITY_NAME_TO_ID.get(state_config.get('priority'), default_priority), - 'tags': state_config.get('tags'), + 'tags': convert_string_to_array(state_config.get('tags')), } try: diff --git a/docs/Dockerfile b/docs/Dockerfile index 20c55ac8..c57e03e7 100644 --- a/docs/Dockerfile +++ b/docs/Dockerfile @@ -5,7 +5,7 @@ RUN apk add --no-cache py3-pip py3-ruamel.yaml py3-ruamel.yaml.clib RUN pip install --break-system-packages --no-cache /app && borgmatic config generate && borgmatic config generate --destination /etc/borgmatic --split && chmod +r /etc/borgmatic/*.yaml RUN mkdir /command-line \ && borgmatic --help > /command-line/global.txt \ - && for action in repo-create transfer create prune compact check delete extract config "config bootstrap" "config generate" "config validate" export-tar mount umount repo-delete restore repo-list list repo-info info break-lock "key export" "key import" "key change-passphrase" recreate borg; do \ + && for action in repo-create transfer create prune compact check delete extract config "config bootstrap" "config generate" "config validate" "config show" export-tar mount umount repo-delete restore repo-list list repo-info info break-lock "key export" "key import" "key change-passphrase" recreate borg; do \ borgmatic $action --help > /command-line/${action/ /-}.txt; done RUN /app/docs/fetch-contributors >> /contributors.html diff --git a/docs/fetch-contributors b/docs/fetch-contributors index 7c1e1093..a9734d73 100755 --- a/docs/fetch-contributors +++ b/docs/fetch-contributors @@ -36,7 +36,6 @@ def list_contributing_issues(url): PULLS_API_ENDPOINT_URLS = ( 'https://projects.torsion.org/api/v1/repos/borgmatic-collective/borgmatic/pulls', - 'https://api.github.com/repos/borgmatic-collective/borgmatic/pulls', ) ISSUES_API_ENDPOINT_URL = 'https://projects.torsion.org/api/v1/repos/borgmatic-collective/borgmatic/issues?state=all' RECENT_CONTRIBUTORS_CUTOFF_DAYS = 365 diff --git a/docs/how-to/backup-to-a-removable-drive-or-an-intermittent-server.md b/docs/how-to/backup-to-a-removable-drive-or-an-intermittent-server.md index 7d027141..313a7f45 100644 --- a/docs/how-to/backup-to-a-removable-drive-or-an-intermittent-server.md +++ b/docs/how-to/backup-to-a-removable-drive-or-an-intermittent-server.md @@ -82,7 +82,7 @@ before_actions: option in the `hooks:` section of your configuration. Prior to version 1.7.0 Use -`before_create` or similar instead of `before_actions`, which was introduced in +`before_backup` or similar instead of `before_actions`, which was introduced in borgmatic 1.7.0. What this does is check if the `findmnt` command errors when probing for a diff --git a/docs/how-to/develop-on-borgmatic.md b/docs/how-to/develop-on-borgmatic.md index ef74841d..762abc36 100644 --- a/docs/how-to/develop-on-borgmatic.md +++ b/docs/how-to/develop-on-borgmatic.md @@ -208,4 +208,18 @@ Setting up Podman is outside the scope of this documentation. But once you install and configure Podman, then `scripts/dev-docs` should automatically use Podman instead of Docker. + +## Use of generative AI + +Please do not use AI agents to modify this codebase. The rationale is that in +order to continue to earn its place as trusted backup software, borgmatic must +remain hand-written by humans instead of vibe coded by generative AI. + +Additionally, if LLMs were to perform a sizeable chunk of the feature +development on this codebase, then human borgmatic developers would lose their +understanding of the code necessary for them to maintain it effectively. + +Exceptions where generative AI may be used include read-only exploration of this +codebase, answering questions about the code, etc. + diff --git a/docs/how-to/monitor-your-backups.md b/docs/how-to/monitor-your-backups.md index 763162fc..963e07d2 100644 --- a/docs/how-to/monitor-your-backups.md +++ b/docs/how-to/monitor-your-backups.md @@ -64,6 +64,27 @@ suppressed so as not to interfere with the captured JSON. Also note that JSON output only shows up at the console and not in syslog. +### Getting configuration + +New in version 2.1.3 If you want +to consume borgmatic's computed configuration in your scripts, use the [`config +show` +action](https://torsion.org/borgmatic/reference/command-line/actions/config-show/). +Here's an example: + +```bash +borgmatic config show --json +``` + +That outputs borgmatic's entire configuration as JSON with one array element per +configuration file. + +Or you can ask for the value of a particular option: + +```bash +borgmatic config show --option repositories --json +``` + ### Latest backups All borgmatic actions that accept an `--archive` flag allow you to specify an diff --git a/docs/reference/command-line/actions/config-show.md b/docs/reference/command-line/actions/config-show.md new file mode 100644 index 00000000..12ad4552 --- /dev/null +++ b/docs/reference/command-line/actions/config-show.md @@ -0,0 +1,17 @@ +--- +title: config show +eleventyNavigation: + key: config show + parent: 🎬 Actions +--- + +{% include snippet/command-line/sample.md %} + +``` +{% include borgmatic/command-line/config-show.txt %} +``` + + +## Related documentation + + * [Scripting borgmatic](https://torsion.org/borgmatic/how-to/monitor-your-backups/#scripting-borgmatic) diff --git a/docs/reference/configuration/command-hooks.md b/docs/reference/configuration/command-hooks.md index 7c7f1eb6..e05fdad1 100644 --- a/docs/reference/configuration/command-hooks.md +++ b/docs/reference/configuration/command-hooks.md @@ -12,11 +12,15 @@ list of `commands:` in your borgmatic configuration file. For example: ```yaml commands: - before: action - when: [create] + when: [check] # This is an inline YAML sequence. run: - echo "Before create!" + - before: action + when: [create, prune] # Also an inline YAML sequence. + run: + - echo "Before create or prune!" - after: action - when: + when: # Multi-line YAML sequence, equivalent to "[create, prune]". - create - prune run: @@ -29,7 +33,7 @@ commands: Each command in the `commands:` list has the following options: * `before` or `after`: Name for the point in borgmatic's execution that the commands should be run before or after, one of: - * `action` runs before or after each action for each repository. This replaces the deprecated `before_create`, `after_prune`, etc. + * `action` runs before or after each action for each repository. This replaces the deprecated `before_backup`, `after_prune`, etc. * `repository` runs before or after all actions for each repository. This replaces the deprecated `before_actions` and `after_actions`. * `configuration` runs before or after all actions and repositories in the current configuration file. * `everything` runs before or after all configuration files. Errors here do not trigger `error` hooks or the `fail` state in monitoring hooks. This replaces the deprecated `before_everything` and `after_everything`. diff --git a/docs/reference/configuration/includes.md b/docs/reference/configuration/includes.md index 1b2691fb..2175fe2e 100644 --- a/docs/reference/configuration/includes.md +++ b/docs/reference/configuration/includes.md @@ -317,3 +317,6 @@ a default location. This will output the merged configuration as borgmatic sees it, which can be helpful for understanding how your includes work in practice. + +Also see the [`config show` +action](https://torsion.org/borgmatic/reference/command-line/actions/config-show/). diff --git a/pyproject.toml b/pyproject.toml index 3f2596e2..88a1d506 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "borgmatic" -version = "2.1.2dev0" +version = "2.1.3.dev0" authors = [ { name="Dan Helfman", email="witten@torsion.org" }, ] diff --git a/tests/end-to-end/commands/fake_zfs.py b/tests/end-to-end/commands/fake_zfs.py index 4ef3f498..cceaf59d 100644 --- a/tests/end-to-end/commands/fake_zfs.py +++ b/tests/end-to-end/commands/fake_zfs.py @@ -35,7 +35,7 @@ BUILTIN_DATASETS = ( 'used': '256K', 'avail': '23.7M', 'refer': '25K', - 'canmount': 'on', + 'canmount': 'noauto', 'mountpoint': '/e2e/pool/dataset', }, ) diff --git a/tests/integration/config/test_generate.py b/tests/integration/config/test_generate.py index 48fbd502..14b03627 100644 --- a/tests/integration/config/test_generate.py +++ b/tests/integration/config/test_generate.py @@ -292,6 +292,12 @@ def test_render_configuration_converts_configuration_to_yaml_string(): assert yaml_string == 'foo: bar\n' +def test_render_configuration_strips_ruamel_yaml_end_of_document_marker(): + yaml_string = module.render_configuration(33) + + assert yaml_string == '33\n' + + def test_write_configuration_does_not_raise(): flexmock(os.path).should_receive('exists').and_return(False) flexmock(os).should_receive('makedirs') diff --git a/tests/integration/test_execute.py b/tests/integration/test_execute.py index 4617fe5d..d1de7f44 100644 --- a/tests/integration/test_execute.py +++ b/tests/integration/test_execute.py @@ -26,6 +26,19 @@ def test_read_lines_yields_single_line_longer_than_chunk_size(): ) +def test_read_lines_yields_single_line_with_multibyte_unicode_character_spanning_chunk_boundary(): + # In case it's not clear, "ñ" is a multi-byte UTF-8 character. The "a" shifts it over one byte + # so it straddles the chunk boundary. + process = subprocess.Popen(['echo', 'aññññññññññññññññññññññññññññññ'], stdout=subprocess.PIPE) + + assert tuple(flexmock(module, READ_CHUNK_SIZE=16).read_lines(process.stdout, process)) == ( + (), + (), + (), + ('aññññññññññññññññññññññññññññññ',), + ) + + def test_read_lines_yields_multiple_lines(): process = subprocess.Popen(['echo', 'hi\nthere'], stdout=subprocess.PIPE) diff --git a/tests/unit/actions/config/test_show.py b/tests/unit/actions/config/test_show.py new file mode 100644 index 00000000..2f12c7ea --- /dev/null +++ b/tests/unit/actions/config/test_show.py @@ -0,0 +1,79 @@ +from flexmock import flexmock + +import borgmatic.logger +from borgmatic.actions.config import show as module + + +def test_run_show_with_single_configuration_file_does_not_separate_output(): + log_lines = [] + borgmatic.logger.add_custom_log_levels() + + def fake_logger_answer(message): + log_lines.append(message) + + flexmock(module.logger).should_receive('answer').replace_with(fake_logger_answer) + show_arguments = flexmock(option=None, json=False) + flexmock(module.borgmatic.config.generate).should_receive('render_configuration').and_return( + 'output' + ) + + module.run_show(show_arguments, configs={'test.yaml': {}}) + + assert log_lines == ['output'] + + +def test_run_show_with_multiple_configuration_files_separates_output(): + log_lines = [] + borgmatic.logger.add_custom_log_levels() + + def fake_logger_answer(message): + log_lines.append(message) + + flexmock(module.logger).should_receive('answer').replace_with(fake_logger_answer) + show_arguments = flexmock(option=None, json=False) + flexmock(module.borgmatic.config.generate).should_receive('render_configuration').and_return( + 'output' + ).and_return('other') + + module.run_show(show_arguments, configs={'test.yaml': {}, 'other.yaml': {}}) + + assert log_lines == ['---', 'output', '---', 'other'] + + +def test_run_show_with_option_limits_output(): + log_lines = [] + borgmatic.logger.add_custom_log_levels() + + def fake_logger_answer(message): + log_lines.append(message) + + flexmock(module.logger).should_receive('answer').replace_with(fake_logger_answer) + show_arguments = flexmock(option='foo', json=False) + flexmock(module.borgmatic.config.generate).should_receive('render_configuration').with_args( + 33 + ).and_return('33') + flexmock(module.borgmatic.config.generate).should_receive('render_configuration').with_args( + None + ).and_return('null') + + module.run_show(show_arguments, configs={'test.yaml': {'foo': 33, 'bar': 44}, 'other.yaml': {}}) + + assert log_lines == ['---', '33', '---', 'null'] + + +def test_run_show_with_json_outputs_json(): + flexmock(borgmatic.logger).should_receive('add_custom_log_levels') + show_arguments = flexmock(option=None, json=True) + flexmock(module.sys.stdout).should_receive('write').with_args( + '[{"foo": 33}, {"bar": 44}]' + ).once() + + module.run_show(show_arguments, configs={'test.yaml': {'foo': 33}, 'other.yaml': {'bar': 44}}) + + +def test_run_show_with_json_and_option_limits_json(): + flexmock(borgmatic.logger).should_receive('add_custom_log_levels') + show_arguments = flexmock(option='foo', json=True) + flexmock(module.sys.stdout).should_receive('write').with_args('[33, null]').once() + + module.run_show(show_arguments, configs={'test.yaml': {'foo': 33}, 'other.yaml': {'bar': 44}}) diff --git a/tests/unit/actions/config/test_validate.py b/tests/unit/actions/config/test_validate.py index 862d1bfa..f24db6c4 100644 --- a/tests/unit/actions/config/test_validate.py +++ b/tests/unit/actions/config/test_validate.py @@ -5,13 +5,13 @@ from borgmatic.actions.config import validate as module def test_run_validate_does_not_raise(): validate_arguments = flexmock(show=False) - flexmock(module.borgmatic.config.generate).should_receive('render_configuration') + flexmock(module.borgmatic.config.generate).should_receive('render_configuration').and_return('') module.run_validate(validate_arguments, flexmock()) def test_run_validate_with_show_does_not_raise(): validate_arguments = flexmock(show=True) - flexmock(module.borgmatic.config.generate).should_receive('render_configuration') + flexmock(module.borgmatic.config.generate).should_receive('render_configuration').and_return('') module.run_validate(validate_arguments, {'test.yaml': flexmock(), 'other.yaml': flexmock()}) diff --git a/tests/unit/actions/test_check.py b/tests/unit/actions/test_check.py index 84111ac8..48b98ba0 100644 --- a/tests/unit/actions/test_check.py +++ b/tests/unit/actions/test_check.py @@ -986,6 +986,65 @@ def test_collect_spot_check_source_paths_uses_working_directory(): ) == ('foo', 'bar') +def test_collect_spot_check_source_paths_deduplicates_borg_output_paths(): + flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hooks').and_return( + {'hook1': False, 'hook2': True}, + ) + flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return( + flexmock(), + ) + flexmock(module.borgmatic.actions.pattern).should_receive('collect_patterns').and_return( + (Pattern('collected'),), + ) + flexmock(module.borgmatic.actions.pattern).should_receive('process_patterns').with_args( + ( + Pattern('collected', source=module.borgmatic.borg.pattern.Pattern_source.HOOK), + Pattern('extra.yaml', source=module.borgmatic.borg.pattern.Pattern_source.INTERNAL), + ), + config=object, + working_directory=None, + ).and_return( + [Pattern('foo'), Pattern('bar')], + ) + flexmock(module.borgmatic.borg.create).should_receive('make_base_create_command').with_args( + dry_run=True, + repository_path='repo', + config=object, + patterns=[Pattern('foo'), Pattern('bar')], + local_borg_version=object, + global_arguments=object, + borgmatic_runtime_directory='/run/borgmatic', + local_path=object, + remote_path=object, + stream_processes=True, + ).and_return((('borg', 'create'), ('repo::archive',), flexmock())) + flexmock(module.borgmatic.borg.environment).should_receive('make_environment').and_return( + flexmock(), + ) + flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None) + flexmock(module.borgmatic.execute).should_receive( + 'execute_command_and_capture_output', + ).and_yield( + 'warning: stuff', + '- /etc/path', + '+ /etc/other', + '? /nope', + '- /etc/path', + ) + flexmock(module.os.path).should_receive('isfile').and_return(True) + + assert module.collect_spot_check_source_paths( + repository={'path': 'repo'}, + config={'working_directory': '/'}, + local_borg_version=flexmock(), + global_arguments=flexmock(), + local_path=flexmock(), + remote_path=flexmock(), + borgmatic_runtime_directory='/run/borgmatic', + bootstrap_config_paths=('extra.yaml',), + ) == ('/etc/path', '/etc/other') + + def test_compare_spot_check_hashes_returns_paths_having_failing_hashes(): flexmock(module.random).should_receive('SystemRandom').and_return( flexmock(sample=lambda population, count: population[:count]), @@ -1002,8 +1061,9 @@ def test_compare_spot_check_hashes_returns_paths_having_failing_hashes(): 'hash2 /bar', ) flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( - {'xxh64': 'hash1', 'path': 'foo'}, - {'xxh64': 'nothash2', 'path': 'bar'}, + {'xxh64': 'hash1', 'path': 'foo', 'linktarget': ''}, + {}, + {'xxh64': 'nothash2', 'path': 'bar', 'linktarget': ''}, ) assert module.compare_spot_check_hashes( @@ -1045,8 +1105,8 @@ def test_compare_spot_check_hashes_handles_weird_backslashed_hashes_from_xxh64su '\\hash2 /bar', ) flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( - {'xxh64': 'hash1', 'path': 'foo'}, - {'xxh64': 'nothash2', 'path': 'bar'}, + {'xxh64': 'hash1', 'path': 'foo', 'linktarget': ''}, + {'xxh64': 'nothash2', 'path': 'bar', 'linktarget': ''}, ) assert module.compare_spot_check_hashes( @@ -1088,8 +1148,8 @@ def test_compare_spot_check_hashes_handles_incorrect_path_names_from_xxh64sum(): 'hash2 /bar/wrong/path', ) flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( - {'xxh64': 'hash1', 'path': 'foo'}, - {'xxh64': 'nothash2', 'path': 'bar'}, + {'xxh64': 'hash1', 'path': 'foo', 'linktarget': ''}, + {'xxh64': 'nothash2', 'path': 'bar', 'linktarget': ''}, ) assert module.compare_spot_check_hashes( @@ -1115,6 +1175,59 @@ def test_compare_spot_check_hashes_handles_incorrect_path_names_from_xxh64sum(): ) == ('/bar',) +def test_compare_spot_check_hashes_with_xxh64sum_failure_falls_back_to_individual_file_hashing(): + flexmock(module.random).should_receive('SystemRandom').and_return( + flexmock(sample=lambda population, count: population[:count]), + ) + flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return( + None, + ) + flexmock(module.os.path).should_receive('exists').and_return(True) + flexmock(module.os.path).should_receive('islink').and_return(False) + flexmock(module.borgmatic.execute).should_receive( + 'execute_command_and_capture_output', + ).with_args(('xxh64sum', '/foo', '/bar'), working_directory=None).and_raise( + module.subprocess.CalledProcessError(1, 'wtf') + ) + flexmock(module.borgmatic.execute).should_receive( + 'execute_command_and_capture_output', + ).with_args(('xxh64sum', '/foo'), working_directory=None).and_raise( + module.subprocess.CalledProcessError(1, 'wtf') + ).once() + flexmock(module.borgmatic.execute).should_receive( + 'execute_command_and_capture_output', + ).with_args(('xxh64sum', '/bar'), working_directory=None).and_yield( + 'hash2 /bar', + ).once() + + flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( + {'xxh64': 'hash1', 'path': 'foo', 'linktarget': ''}, + {'xxh64': 'hash2', 'path': 'bar', 'linktarget': ''}, + ) + + assert module.compare_spot_check_hashes( + repository={'path': 'repo'}, + archive='archive', + config={ + 'checks': [ + { + 'name': 'archives', + 'frequency': '2 weeks', + }, + { + 'name': 'spot', + 'data_sample_percentage': 50, + }, + ], + }, + local_borg_version=flexmock(), + global_arguments=flexmock(), + local_path=flexmock(), + remote_path=flexmock(), + source_paths=('/foo', '/bar', '/baz', '/quux'), + ) == ('/foo',) + + def test_compare_spot_check_hashes_returns_relative_paths_having_failing_hashes(): flexmock(module.random).should_receive('SystemRandom').and_return( flexmock(sample=lambda population, count: population[:count]), @@ -1131,8 +1244,8 @@ def test_compare_spot_check_hashes_returns_relative_paths_having_failing_hashes( 'hash2 bar', ) flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( - {'xxh64': 'hash1', 'path': 'foo'}, - {'xxh64': 'nothash2', 'path': 'bar'}, + {'xxh64': 'hash1', 'path': 'foo', 'linktarget': ''}, + {'xxh64': 'nothash2', 'path': 'bar', 'linktarget': ''}, ) assert module.compare_spot_check_hashes( @@ -1174,8 +1287,8 @@ def test_compare_spot_check_hashes_handles_data_sample_percentage_above_100(): 'hash2 /bar', ) flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( - {'xxh64': 'nothash1', 'path': 'foo'}, - {'xxh64': 'nothash2', 'path': 'bar'}, + {'xxh64': 'nothash1', 'path': 'foo', 'linktarget': ''}, + {'xxh64': 'nothash2', 'path': 'bar', 'linktarget': ''}, ) assert module.compare_spot_check_hashes( @@ -1217,8 +1330,8 @@ def test_compare_spot_check_hashes_uses_xxh64sum_command_option(): working_directory=None, ).and_yield('hash1 /foo', 'hash2 /bar') flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( - {'xxh64': 'hash1', 'path': 'foo'}, - {'xxh64': 'nothash2', 'path': 'bar'}, + {'xxh64': 'hash1', 'path': 'foo', 'linktarget': ''}, + {'xxh64': 'nothash2', 'path': 'bar', 'linktarget': ''}, ) assert module.compare_spot_check_hashes( @@ -1257,7 +1370,7 @@ def test_compare_spot_check_hashes_considers_path_missing_from_archive_as_not_ma 'hash2 /bar', ) flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( - {'xxh64': 'hash1', 'path': 'foo'} + {'xxh64': 'hash1', 'path': 'foo', 'linktarget': ''} ) assert module.compare_spot_check_hashes( @@ -1279,6 +1392,50 @@ def test_compare_spot_check_hashes_considers_path_missing_from_archive_as_not_ma ) == ('/bar',) +def test_compare_spot_check_hashes_skips_hardlink_path_in_archive(): + flexmock(module.random).should_receive('SystemRandom').and_return( + flexmock(sample=lambda population, count: population[:count]), + ) + flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return( + None, + ) + flexmock(module.os.path).should_receive('exists').and_return(True) + flexmock(module.os.path).should_receive('islink').and_return(False) + flexmock(module.borgmatic.execute).should_receive( + 'execute_command_and_capture_output', + ).with_args(('xxh64sum', '/foo', '/bar', '/link'), working_directory=None).and_yield( + 'hash1 /foo', + 'hash2 /bar', + 'hash1 /link', + ) + flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( + {'xxh64': 'hash1', 'path': 'foo', 'linktarget': ''}, + {'xxh64': 'hash2', 'path': 'bar', 'linktarget': ''}, + {'xxh64': '', 'path': 'link', 'linktarget': 'foo'}, + ) + + assert ( + module.compare_spot_check_hashes( + repository={'path': 'repo'}, + archive='archive', + config={ + 'checks': [ + { + 'name': 'spot', + 'data_sample_percentage': 100, + }, + ], + }, + local_borg_version=flexmock(), + global_arguments=flexmock(), + local_path=flexmock(), + remote_path=flexmock(), + source_paths=('/foo', '/bar', '/link'), + ) + == () + ) + + def test_compare_spot_check_hashes_considers_symlink_path_as_not_matching(): flexmock(module.random).should_receive('SystemRandom').and_return( flexmock(sample=lambda population, count: population[:count]), @@ -1293,8 +1450,8 @@ def test_compare_spot_check_hashes_considers_symlink_path_as_not_matching(): 'execute_command_and_capture_output', ).with_args(('xxh64sum', '/foo'), working_directory=None).and_yield('hash1 /foo') flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( - {'xxh64': 'hash1', 'path': 'foo'}, - {'xxh64': 'hash2', 'path': 'bar'}, + {'xxh64': 'hash1', 'path': 'foo', 'linktarget': ''}, + {'xxh64': 'hash2', 'path': 'bar', 'linktarget': ''}, ) assert module.compare_spot_check_hashes( @@ -1330,8 +1487,8 @@ def test_compare_spot_check_hashes_considers_non_existent_path_as_not_matching() 'execute_command_and_capture_output', ).with_args(('xxh64sum', '/foo'), working_directory=None).and_yield('hash1 /foo') flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( - {'xxh64': 'hash1', 'path': 'foo'}, - {'xxh64': 'hash2', 'path': 'bar'}, + {'xxh64': 'hash1', 'path': 'foo', 'linktarget': ''}, + {'xxh64': 'hash2', 'path': 'bar', 'linktarget': ''}, ) assert module.compare_spot_check_hashes( @@ -1376,11 +1533,11 @@ def test_compare_spot_check_hashes_with_too_many_paths_feeds_them_to_commands_in 'hash4 /quux', ) flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( - {'xxh64': 'hash1', 'path': 'foo'}, - {'xxh64': 'hash2', 'path': 'bar'}, + {'xxh64': 'hash1', 'path': 'foo', 'linktarget': ''}, + {'xxh64': 'hash2', 'path': 'bar', 'linktarget': ''}, ).and_yield( - {'xxh64': 'hash3', 'path': 'baz'}, - {'xxh64': 'nothash4', 'path': 'quux'}, + {'xxh64': 'hash3', 'path': 'baz', 'linktarget': ''}, + {'xxh64': 'nothash4', 'path': 'quux', 'linktarget': ''}, ) assert module.compare_spot_check_hashes( @@ -1423,8 +1580,8 @@ def test_compare_spot_check_hashes_uses_working_directory_to_access_source_paths 'hash2 bar', ) flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( - {'xxh64': 'hash1', 'path': 'foo'}, - {'xxh64': 'nothash2', 'path': 'bar'}, + {'xxh64': 'hash1', 'path': 'foo', 'linktarget': ''}, + {'xxh64': 'nothash2', 'path': 'bar', 'linktarget': ''}, ) assert module.compare_spot_check_hashes( diff --git a/tests/unit/borg/test_check.py b/tests/unit/borg/test_check.py index fb4acc32..d68c89e0 100644 --- a/tests/unit/borg/test_check.py +++ b/tests/unit/borg/test_check.py @@ -24,7 +24,7 @@ def insert_execute_command_mock( environment=None, working_directory=working_directory, borg_local_path=command[0], - borg_exit_codes=borg_exit_codes, + borg_exit_codes=(borg_exit_codes or []) + [{'code': 1, 'treat_as': 'error'}], ).once() @@ -335,7 +335,7 @@ def test_check_archives_with_progress_passes_through_to_borg(): environment=None, working_directory=None, borg_local_path='borg', - borg_exit_codes=None, + borg_exit_codes=[{'code': 1, 'treat_as': 'error'}], ).once() module.check_archives( @@ -371,7 +371,7 @@ def test_check_archives_with_log_json_and_progress_passes_through_both_to_borg() environment=None, working_directory=None, borg_local_path='borg', - borg_exit_codes=None, + borg_exit_codes=[{'code': 1, 'treat_as': 'error'}], ).once() module.check_archives( @@ -407,7 +407,7 @@ def test_check_archives_with_repair_passes_through_to_borg(): environment=None, working_directory=None, borg_local_path='borg', - borg_exit_codes=None, + borg_exit_codes=[{'code': 1, 'treat_as': 'error'}], ).once() module.check_archives( @@ -443,7 +443,7 @@ def test_check_archives_with_log_json_and_repair_passes_through_both_to_borg(): environment=None, working_directory=None, borg_local_path='borg', - borg_exit_codes=None, + borg_exit_codes=[{'code': 1, 'treat_as': 'error'}], ).once() module.check_archives( @@ -479,7 +479,7 @@ def test_check_archives_with_max_duration_flag_passes_through_to_borg(): environment=None, working_directory=None, borg_local_path='borg', - borg_exit_codes=None, + borg_exit_codes=[{'code': 1, 'treat_as': 'error'}], ).once() module.check_archives( @@ -515,7 +515,7 @@ def test_check_archives_with_max_duration_option_passes_through_to_borg(): environment=None, working_directory=None, borg_local_path='borg', - borg_exit_codes=None, + borg_exit_codes=[{'code': 1, 'treat_as': 'error'}], ).once() module.check_archives( @@ -689,7 +689,7 @@ def test_check_archives_with_max_duration_flag_overrides_max_duration_option(): environment=None, working_directory=None, borg_local_path='borg', - borg_exit_codes=None, + borg_exit_codes=[{'code': 1, 'treat_as': 'error'}], ).once() module.check_archives( @@ -854,7 +854,7 @@ def test_check_archives_with_local_path_calls_borg_via_local_path(): def test_check_archives_with_exit_codes_calls_borg_using_them(): checks = {'repository'} - borg_exit_codes = flexmock() + borg_exit_codes = [{'code': 101, 'treat_as': 'error'}] config = {'borg_exit_codes': borg_exit_codes} flexmock(module).should_receive('make_check_name_flags').with_args(checks, ()).and_return(()) flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) @@ -1026,7 +1026,7 @@ def test_check_archives_with_match_archives_passes_through_to_borg(): environment=None, working_directory=None, borg_local_path='borg', - borg_exit_codes=None, + borg_exit_codes=[{'code': 1, 'treat_as': 'error'}], ).once() module.check_archives( diff --git a/tests/unit/commands/test_arguments.py b/tests/unit/commands/test_arguments.py index 0fad32e7..0606a1d2 100644 --- a/tests/unit/commands/test_arguments.py +++ b/tests/unit/commands/test_arguments.py @@ -329,10 +329,12 @@ def test_parse_arguments_for_actions_consumes_action_arguments_after_action_name remaining = flexmock() flexmock(module).should_receive('get_subaction_parsers').and_return({}) flexmock(module).should_receive('parse_and_record_action_arguments').replace_with( - lambda unparsed, parsed, parser, action, canonical=None: parsed.update( - {action: action_namespace}, - ) - or remaining, + lambda unparsed, parsed, parser, action, canonical=None: ( + parsed.update( + {action: action_namespace}, + ) + or remaining + ), ) flexmock(module).should_receive('get_subactions_for_actions').and_return({}) action_parsers = {'action': flexmock(), 'other': flexmock()} @@ -355,10 +357,12 @@ def test_parse_arguments_for_actions_consumes_action_arguments_with_alias(): remaining = flexmock() flexmock(module).should_receive('get_subaction_parsers').and_return({}) flexmock(module).should_receive('parse_and_record_action_arguments').replace_with( - lambda unparsed, parsed, parser, action, canonical=None: parsed.update( - {canonical or action: action_namespace}, - ) - or remaining, + lambda unparsed, parsed, parser, action, canonical=None: ( + parsed.update( + {canonical or action: action_namespace}, + ) + or remaining + ), ) flexmock(module).should_receive('get_subactions_for_actions').and_return({}) action_parsers = { @@ -387,10 +391,12 @@ def test_parse_arguments_for_actions_consumes_multiple_action_arguments(): other_namespace = flexmock(bar=3) flexmock(module).should_receive('get_subaction_parsers').and_return({}) flexmock(module).should_receive('parse_and_record_action_arguments').replace_with( - lambda unparsed, parsed, parser, action, canonical=None: parsed.update( - {action: action_namespace if action == 'action' else other_namespace}, - ) - or (), + lambda unparsed, parsed, parser, action, canonical=None: ( + parsed.update( + {action: action_namespace if action == 'action' else other_namespace}, + ) + or () + ), ).and_return(('other', '--bar', '3')).and_return('action', '--foo', 'true') flexmock(module).should_receive('get_subactions_for_actions').and_return({}) action_parsers = { @@ -420,10 +426,12 @@ def test_parse_arguments_for_actions_respects_command_line_action_ordering(): action_namespace = flexmock(foo=True) flexmock(module).should_receive('get_subaction_parsers').and_return({}) flexmock(module).should_receive('parse_and_record_action_arguments').replace_with( - lambda unparsed, parsed, parser, action, canonical=None: parsed.update( - {action: other_namespace if action == 'other' else action_namespace}, - ) - or (), + lambda unparsed, parsed, parser, action, canonical=None: ( + parsed.update( + {action: other_namespace if action == 'other' else action_namespace}, + ) + or () + ), ).and_return(('action',)).and_return(('other', '--foo', 'true')) flexmock(module).should_receive('get_subactions_for_actions').and_return({}) action_parsers = { @@ -458,10 +466,12 @@ def test_parse_arguments_for_actions_applies_default_action_parsers(): flexmock(module).should_receive('get_subaction_parsers').and_return({}) flexmock(module).should_receive('parse_and_record_action_arguments').replace_with( - lambda unparsed, parsed, parser, action, canonical=None: parsed.update( - {action: namespaces.get(action)}, - ) - or (), + lambda unparsed, parsed, parser, action, canonical=None: ( + parsed.update( + {action: namespaces.get(action)}, + ) + or () + ), ).and_return(()) flexmock(module).should_receive('get_subactions_for_actions').and_return({}) action_parsers = { @@ -488,10 +498,12 @@ def test_parse_arguments_for_actions_consumes_global_arguments(): action_namespace = flexmock() flexmock(module).should_receive('get_subaction_parsers').and_return({}) flexmock(module).should_receive('parse_and_record_action_arguments').replace_with( - lambda unparsed, parsed, parser, action, canonical=None: parsed.update( - {action: action_namespace}, - ) - or ('--verbosity', 'lots'), + lambda unparsed, parsed, parser, action, canonical=None: ( + parsed.update( + {action: action_namespace}, + ) + or ('--verbosity', 'lots') + ), ) flexmock(module).should_receive('get_subactions_for_actions').and_return({}) action_parsers = { @@ -516,10 +528,12 @@ def test_parse_arguments_for_actions_passes_through_unknown_arguments_before_act action_namespace = flexmock() flexmock(module).should_receive('get_subaction_parsers').and_return({}) flexmock(module).should_receive('parse_and_record_action_arguments').replace_with( - lambda unparsed, parsed, parser, action, canonical=None: parsed.update( - {action: action_namespace}, - ) - or ('--wtf', 'yes'), + lambda unparsed, parsed, parser, action, canonical=None: ( + parsed.update( + {action: action_namespace}, + ) + or ('--wtf', 'yes') + ), ) flexmock(module).should_receive('get_subactions_for_actions').and_return({}) action_parsers = { @@ -544,10 +558,12 @@ def test_parse_arguments_for_actions_passes_through_unknown_arguments_after_acti action_namespace = flexmock() flexmock(module).should_receive('get_subaction_parsers').and_return({}) flexmock(module).should_receive('parse_and_record_action_arguments').replace_with( - lambda unparsed, parsed, parser, action, canonical=None: parsed.update( - {action: action_namespace}, - ) - or ('--wtf', 'yes'), + lambda unparsed, parsed, parser, action, canonical=None: ( + parsed.update( + {action: action_namespace}, + ) + or ('--wtf', 'yes') + ), ) flexmock(module).should_receive('get_subactions_for_actions').and_return({}) action_parsers = { @@ -572,10 +588,12 @@ def test_parse_arguments_for_actions_with_borg_action_skips_other_action_parsers action_namespace = flexmock(options=[]) flexmock(module).should_receive('get_subaction_parsers').and_return({}) flexmock(module).should_receive('parse_and_record_action_arguments').replace_with( - lambda unparsed, parsed, parser, action, canonical=None: parsed.update( - {action: action_namespace}, - ) - or (), + lambda unparsed, parsed, parser, action, canonical=None: ( + parsed.update( + {action: action_namespace}, + ) + or () + ), ).and_return(()) flexmock(module).should_receive('get_subactions_for_actions').and_return({}) action_parsers = { diff --git a/tests/unit/commands/test_borgmatic.py b/tests/unit/commands/test_borgmatic.py index 887f1631..7caa1596 100644 --- a/tests/unit/commands/test_borgmatic.py +++ b/tests/unit/commands/test_borgmatic.py @@ -2058,6 +2058,43 @@ def test_collect_highlander_action_summary_logs_error_on_run_validate_failure(): assert {log.levelno for log in logs} == {logging.CRITICAL} +def test_collect_highlander_action_summary_logs_nothing_additional_for_success_with_show(): + flexmock(module.borgmatic.actions.config.show).should_receive('run_show') + arguments = { + 'show': flexmock(), + 'global': flexmock(), + } + + logs = tuple( + module.collect_highlander_action_summary_logs( + {'test.yaml': {}}, + arguments=arguments, + configuration_parse_errors=False, + ), + ) + assert not logs + + +def test_collect_highlander_action_summary_logs_error_on_run_show_failure(): + flexmock(module.borgmatic.actions.config.show).should_receive('run_show').and_raise( + ValueError, + ) + arguments = { + 'show': flexmock(), + 'global': flexmock(), + } + + logs = tuple( + module.collect_highlander_action_summary_logs( + {'test.yaml': {}}, + arguments=arguments, + configuration_parse_errors=False, + ), + ) + + assert {log.levelno for log in logs} == {logging.CRITICAL} + + def test_collect_configuration_run_summary_logs_info_for_success(): flexmock(module.validate).should_receive('guard_configuration_contains_repository') flexmock(module.command).should_receive('filter_hooks').with_args( diff --git a/tests/unit/hooks/data_source/test_bootstrap.py b/tests/unit/hooks/data_source/test_bootstrap.py index 6958f0f1..b768bbc6 100644 --- a/tests/unit/hooks/data_source/test_bootstrap.py +++ b/tests/unit/hooks/data_source/test_bootstrap.py @@ -1,13 +1,55 @@ import sys +import pytest from flexmock import flexmock from borgmatic.hooks.data_source import bootstrap as module -def test_dump_data_sources_creates_manifest_file(): - flexmock(module.os).should_receive('makedirs') +def test_resolve_config_path_symlinks_passes_through_non_symlink(): + flexmock(module.os.path).should_receive('abspath').replace_with(lambda path: path) + flexmock(module.os.path).should_receive('islink').and_return(False) + assert tuple(module.resolve_config_path_symlinks('test.yaml')) == ('test.yaml',) + + +def test_resolve_config_path_symlinks_follows_each_symlink(): + flexmock(module.os.path).should_receive('abspath').replace_with(lambda path: path) + flexmock(module.os.path).should_receive('islink').with_args('test.yaml').and_return(True) + flexmock(module.os.path).should_receive('islink').with_args('dest1.yaml').and_return(True) + flexmock(module.os.path).should_receive('islink').with_args('dest2.yaml').and_return(False) + flexmock(module.os).should_receive('readlink').with_args('test.yaml').and_return('dest1.yaml') + flexmock(module.os).should_receive('readlink').with_args('dest1.yaml').and_return('dest2.yaml') + flexmock(module.os).should_receive('readlink').with_args('dest2.yaml').never() + + assert tuple(module.resolve_config_path_symlinks('test.yaml')) == ( + 'test.yaml', + 'dest1.yaml', + 'dest2.yaml', + ) + + +def test_resolve_config_path_symlinks_with_too_many_symlinks_raises(): + flexmock(module).MAXIMUM_CONFIG_SYMLINKS_TO_FOLLOW = 2 + flexmock(module.os.path).should_receive('abspath').replace_with(lambda path: path) + flexmock(module.os.path).should_receive('islink').with_args('test.yaml').and_return(True) + flexmock(module.os.path).should_receive('islink').with_args('dest1.yaml').and_return(True) + flexmock(module.os.path).should_receive('islink').with_args('dest2.yaml').and_return(True) + flexmock(module.os.path).should_receive('islink').with_args('dest3.yaml').never() + flexmock(module.os).should_receive('readlink').with_args('test.yaml').and_return('dest1.yaml') + flexmock(module.os).should_receive('readlink').with_args('dest1.yaml').and_return('dest2.yaml') + flexmock(module.os).should_receive('readlink').with_args('dest2.yaml').and_return('dest3.yaml') + flexmock(module.os).should_receive('readlink').with_args('dest3.yaml').never() + + with pytest.raises(ValueError): + assert tuple(module.resolve_config_path_symlinks('test.yaml')) + + +def test_dump_data_sources_creates_manifest_file(): + flexmock(module).should_receive('resolve_config_path_symlinks').and_yield( + 'test.yaml', 'linkdest.yaml' + ) + flexmock(module.os).should_receive('makedirs') flexmock(module.importlib.metadata).should_receive('version').and_return('1.0.0') manifest_file = flexmock( __enter__=lambda *args: flexmock(write=lambda *args: None, close=lambda *args: None), @@ -19,7 +61,7 @@ def test_dump_data_sources_creates_manifest_file(): encoding='utf-8', ).and_return(manifest_file) flexmock(module.json).should_receive('dump').with_args( - {'borgmatic_version': '1.0.0', 'config_paths': ('test.yaml',)}, + {'borgmatic_version': '1.0.0', 'config_paths': ('test.yaml', 'linkdest.yaml')}, manifest_file, ).once() flexmock(module.borgmatic.hooks.data_source.config).should_receive('inject_pattern').with_args( @@ -34,6 +76,12 @@ def test_dump_data_sources_creates_manifest_file(): 'test.yaml', source=module.borgmatic.borg.pattern.Pattern_source.HOOK ), ).once() + flexmock(module.borgmatic.hooks.data_source.config).should_receive('inject_pattern').with_args( + object, + module.borgmatic.borg.pattern.Pattern( + 'linkdest.yaml', source=module.borgmatic.borg.pattern.Pattern_source.HOOK + ), + ).once() module.dump_data_sources( hook_config=None, @@ -46,6 +94,7 @@ def test_dump_data_sources_creates_manifest_file(): def test_dump_data_sources_with_store_config_files_false_does_not_create_manifest_file(): + flexmock(module).should_receive('resolve_config_path_symlinks').and_yield('test.yaml') flexmock(module.os).should_receive('makedirs').never() flexmock(module.json).should_receive('dump').never() flexmock(module.borgmatic.hooks.data_source.config).should_receive('inject_pattern').never() @@ -62,6 +111,7 @@ def test_dump_data_sources_with_store_config_files_false_does_not_create_manifes def test_dump_data_sources_with_dry_run_does_not_create_manifest_file(): + flexmock(module).should_receive('resolve_config_path_symlinks').and_yield('test.yaml') flexmock(module.os).should_receive('makedirs').never() flexmock(module.json).should_receive('dump').never() flexmock(module.borgmatic.hooks.data_source.config).should_receive('inject_pattern').never() diff --git a/tests/unit/hooks/monitoring/test_ntfy.py b/tests/unit/hooks/monitoring/test_ntfy.py index 7588edd6..cfd966d4 100644 --- a/tests/unit/hooks/monitoring/test_ntfy.py +++ b/tests/unit/hooks/monitoring/test_ntfy.py @@ -1,10 +1,26 @@ from enum import Enum +import pytest from flexmock import flexmock import borgmatic.hooks.monitoring.monitor from borgmatic.hooks.monitoring import ntfy as module + +@pytest.mark.parametrize( + 'tags_input,expected_tags_output', + ( + (None, []), + ('', []), + ('foo', ['foo']), + (' foo , bar ,baz ', ['foo', 'bar', 'baz']), + ('foo,,bar,', ['foo', 'bar']), + ), +) +def test_convert_string_to_array(tags_input, expected_tags_output): + assert module.convert_string_to_array(tags_input) == expected_tags_output + + DEFAULT_BASE_URL = 'https://ntfy.sh' CUSTOM_BASE_URL = 'https://ntfy.example.com' TOPIC = 'borgmatic-unit-testing' @@ -25,7 +41,7 @@ CUSTOM_MESSAGE_PAYLOAD = { 'title': CUSTOM_MESSAGE_CONFIG['title'], 'message': CUSTOM_MESSAGE_CONFIG['message'], 'priority': 1, - 'tags': CUSTOM_MESSAGE_CONFIG['tags'], + 'tags': [CUSTOM_MESSAGE_CONFIG['tags']], } @@ -35,12 +51,15 @@ def default_message_payload(state=Enum): 'title': f'A borgmatic {state.name} event happened', 'message': f'A borgmatic {state.name} event happened', 'priority': 3, - 'tags': 'borgmatic', + 'tags': ['borgmatic'], } def test_ping_monitor_minimal_config_hits_hosted_ntfy_on_fail(): hook_config = {'topic': TOPIC} + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -67,6 +86,9 @@ def test_ping_monitor_with_access_token_hits_hosted_ntfy_on_fail(): 'topic': TOPIC, 'access_token': 'abc123', } + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -95,6 +117,9 @@ def test_ping_monitor_with_username_password_and_access_token_ignores_username_p 'password': 'fakepassword', 'access_token': 'abc123', } + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -123,6 +148,9 @@ def test_ping_monitor_with_username_password_hits_hosted_ntfy_on_fail(): 'username': 'testuser', 'password': 'fakepassword', } + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -146,6 +174,9 @@ def test_ping_monitor_with_username_password_hits_hosted_ntfy_on_fail(): def test_ping_monitor_with_password_but_no_username_warns(): hook_config = {'topic': TOPIC, 'password': 'fakepassword'} + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -170,6 +201,9 @@ def test_ping_monitor_with_password_but_no_username_warns(): def test_ping_monitor_with_username_but_no_password_warns(): hook_config = {'topic': TOPIC, 'username': 'testuser'} + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -194,6 +228,9 @@ def test_ping_monitor_with_username_but_no_password_warns(): def test_ping_monitor_minimal_config_does_not_hit_hosted_ntfy_on_start(): hook_config = {'topic': TOPIC} + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -211,6 +248,9 @@ def test_ping_monitor_minimal_config_does_not_hit_hosted_ntfy_on_start(): def test_ping_monitor_minimal_config_does_not_hit_hosted_ntfy_on_finish(): hook_config = {'topic': TOPIC} + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -228,6 +268,9 @@ def test_ping_monitor_minimal_config_does_not_hit_hosted_ntfy_on_finish(): def test_ping_monitor_minimal_config_hits_selfhosted_ntfy_on_fail(): hook_config = {'topic': TOPIC, 'server': CUSTOM_BASE_URL} + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -251,6 +294,9 @@ def test_ping_monitor_minimal_config_hits_selfhosted_ntfy_on_fail(): def test_ping_monitor_minimal_config_does_not_hit_hosted_ntfy_on_fail_dry_run(): hook_config = {'topic': TOPIC} + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -268,6 +314,7 @@ def test_ping_monitor_minimal_config_does_not_hit_hosted_ntfy_on_fail_dry_run(): def test_ping_monitor_custom_message_hits_hosted_ntfy_on_fail(): hook_config = {'topic': TOPIC, 'fail': CUSTOM_MESSAGE_CONFIG} + flexmock(module).should_receive('convert_string_to_array').with_args('+1').and_return(['+1']) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -291,6 +338,9 @@ def test_ping_monitor_custom_message_hits_hosted_ntfy_on_fail(): def test_ping_monitor_custom_state_hits_hosted_ntfy_on_start(): hook_config = {'topic': TOPIC, 'states': ['start', 'fail']} + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -314,6 +364,9 @@ def test_ping_monitor_custom_state_hits_hosted_ntfy_on_start(): def test_ping_monitor_with_connection_error_logs_warning(): hook_config = {'topic': TOPIC} + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) @@ -338,6 +391,9 @@ def test_ping_monitor_with_connection_error_logs_warning(): def test_ping_monitor_with_credential_error_logs_warning(): hook_config = {'topic': TOPIC} + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).and_raise(ValueError) @@ -356,6 +412,9 @@ def test_ping_monitor_with_credential_error_logs_warning(): def test_ping_monitor_with_other_error_logs_warning(): hook_config = {'topic': TOPIC} + flexmock(module).should_receive('convert_string_to_array').with_args('borgmatic').and_return( + ['borgmatic'] + ) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential', ).replace_with(lambda value, config: value) diff --git a/tests/unit/test_execute.py b/tests/unit/test_execute.py index afc9942c..4807002a 100644 --- a/tests/unit/test_execute.py +++ b/tests/unit/test_execute.py @@ -7,72 +7,80 @@ from borgmatic import execute as module @pytest.mark.parametrize( - 'command,exit_code,borg_local_path,borg_exit_codes,expected_result', + 'command,borg_local_path,expected_result', ( - (['grep'], 2, None, None, module.Exit_status.ERROR), - (['grep'], 2, 'borg', None, module.Exit_status.ERROR), - (['borg'], 2, 'borg', None, module.Exit_status.ERROR), - (['borg1'], 2, 'borg1', None, module.Exit_status.ERROR), - (['grep'], 1, None, None, module.Exit_status.ERROR), - (['grep'], 1, 'borg', None, module.Exit_status.ERROR), - (['borg'], 1, 'borg', None, module.Exit_status.WARNING), - (['borg1'], 1, 'borg1', None, module.Exit_status.WARNING), - (['grep'], 100, None, None, module.Exit_status.ERROR), - (['grep'], 100, 'borg', None, module.Exit_status.ERROR), - ('grep', 2, None, None, module.Exit_status.ERROR), - ('borg', 2, 'borg', None, module.Exit_status.ERROR), - (['borg'], 100, 'borg', None, module.Exit_status.WARNING), - (['borg1'], 100, 'borg1', None, module.Exit_status.WARNING), - ('borg', 100, 'borg', None, module.Exit_status.WARNING), - ('borg1', 100, 'borg1', None, module.Exit_status.WARNING), - (['grep'], 0, None, None, module.Exit_status.SUCCESS), - (['grep'], 0, 'borg', None, module.Exit_status.SUCCESS), - (['borg'], 0, 'borg', None, module.Exit_status.SUCCESS), - (['borg1'], 0, 'borg1', None, module.Exit_status.SUCCESS), - ('grep', 0, None, None, module.Exit_status.SUCCESS), - ('grep', 0, 'borg', None, module.Exit_status.SUCCESS), + (['foo', 'bar'], 'borg', False), + (['borg', 'list'], 'borg', True), + (['borg1', 'list'], 'borg', False), + (['borg', 'list'], 'borg1', False), + ([], 'borg', False), + ('foo bar', 'borg', False), + ('borg list', 'borg', True), + ('', 'borg', False), + ), +) +def test_command_is_borg_matches_local_path_to_command(command, borg_local_path, expected_result): + assert module.command_is_borg(command, borg_local_path) == expected_result + + +@pytest.mark.parametrize( + 'command_is_borg,exit_code,borg_exit_codes,expected_result', + ( + (False, 2, None, module.Exit_status.ERROR), + (True, 2, None, module.Exit_status.ERROR), + (False, 1, None, module.Exit_status.ERROR), + (True, 1, None, module.Exit_status.WARNING), + (False, 100, None, module.Exit_status.ERROR), + (False, 2, None, module.Exit_status.ERROR), + (True, 2, None, module.Exit_status.ERROR), + (True, 100, None, module.Exit_status.WARNING), + (False, 0, None, module.Exit_status.SUCCESS), + (True, 0, None, module.Exit_status.SUCCESS), # -9 exit code occurs when child process get SIGKILLed. - (['grep'], -9, None, None, module.Exit_status.ERROR), - (['grep'], -9, 'borg', None, module.Exit_status.ERROR), - (['borg'], -9, 'borg', None, module.Exit_status.ERROR), - (['borg1'], -9, 'borg1', None, module.Exit_status.ERROR), - (['borg'], None, None, None, module.Exit_status.STILL_RUNNING), - (['borg'], 1, 'borg', [], module.Exit_status.WARNING), - (['borg'], 1, 'borg', [{}], module.Exit_status.WARNING), - (['borg'], 1, 'borg', [{'code': 1}], module.Exit_status.WARNING), - (['grep'], 1, 'borg', [{'code': 100, 'treat_as': 'error'}], module.Exit_status.ERROR), - (['borg'], 1, 'borg', [{'code': 100, 'treat_as': 'error'}], module.Exit_status.WARNING), - (['borg'], 1, 'borg', [{'code': 1, 'treat_as': 'error'}], module.Exit_status.ERROR), - (['borg'], 2, 'borg', [{'code': 99, 'treat_as': 'warning'}], module.Exit_status.ERROR), - (['borg'], 2, 'borg', [{'code': 2, 'treat_as': 'warning'}], module.Exit_status.WARNING), - (['borg'], 100, 'borg', [{'code': 1, 'treat_as': 'error'}], module.Exit_status.WARNING), - (['borg'], 100, 'borg', [], module.Exit_status.WARNING), - (['borg'], 100, 'borg', [{'code': 100, 'treat_as': 'error'}], module.Exit_status.ERROR), - (['borg'], 101, 'borg', [], module.Exit_status.ERROR), - (['borg'], 101, 'borg', [{'code': 101, 'treat_as': 'warning'}], module.Exit_status.WARNING), - (['borg'], 102, 'borg', [], module.Exit_status.ERROR), - (['borg'], 102, 'borg', [{'code': 102, 'treat_as': 'warning'}], module.Exit_status.WARNING), - (['borg'], 103, 'borg', [], module.Exit_status.WARNING), - (['borg'], 103, 'borg', [{'code': 103, 'treat_as': 'error'}], module.Exit_status.ERROR), - (['borg'], 104, 'borg', [], module.Exit_status.ERROR), - (['borg'], 104, 'borg', [{'code': 104, 'treat_as': 'warning'}], module.Exit_status.WARNING), - (['borg'], 105, 'borg', [], module.Exit_status.ERROR), - (['borg'], 105, 'borg', [{'code': 105, 'treat_as': 'warning'}], module.Exit_status.WARNING), - (['borg'], 106, 'borg', [], module.Exit_status.ERROR), - (['borg'], 106, 'borg', [{'code': 106, 'treat_as': 'warning'}], module.Exit_status.WARNING), - (['borg'], 107, 'borg', [], module.Exit_status.ERROR), - (['borg'], 107, 'borg', [{'code': 107, 'treat_as': 'warning'}], module.Exit_status.WARNING), + (False, -9, None, module.Exit_status.ERROR), + (True, -9, None, module.Exit_status.ERROR), + (True, None, None, module.Exit_status.STILL_RUNNING), + (True, 1, [], module.Exit_status.WARNING), + (True, 1, [{'code': 1}], module.Exit_status.WARNING), + (False, 1, [{'code': 100, 'treat_as': 'error'}], module.Exit_status.ERROR), + (True, 1, [{'code': 100, 'treat_as': 'error'}], module.Exit_status.WARNING), + (True, 1, [{'code': 1, 'treat_as': 'error'}], module.Exit_status.ERROR), + (True, 2, [{'code': 99, 'treat_as': 'warning'}], module.Exit_status.ERROR), + (True, 2, [{'code': 2, 'treat_as': 'warning'}], module.Exit_status.WARNING), + (True, 100, [{'code': 1, 'treat_as': 'error'}], module.Exit_status.WARNING), + (True, 100, [], module.Exit_status.WARNING), + (True, 100, [{'code': 100, 'treat_as': 'error'}], module.Exit_status.ERROR), + (True, 101, [], module.Exit_status.ERROR), + (True, 101, [{'code': 101, 'treat_as': 'warning'}], module.Exit_status.WARNING), + (True, 102, [], module.Exit_status.ERROR), + (True, 102, [{'code': 102, 'treat_as': 'warning'}], module.Exit_status.WARNING), + (True, 103, [], module.Exit_status.WARNING), + (True, 103, [{'code': 103, 'treat_as': 'error'}], module.Exit_status.ERROR), + (True, 104, [], module.Exit_status.ERROR), + (True, 104, [{'code': 104, 'treat_as': 'warning'}], module.Exit_status.WARNING), + (True, 105, [], module.Exit_status.ERROR), + (True, 105, [{'code': 105, 'treat_as': 'warning'}], module.Exit_status.WARNING), + (True, 106, [], module.Exit_status.ERROR), + (True, 106, [{'code': 106, 'treat_as': 'warning'}], module.Exit_status.WARNING), + (True, 107, [], module.Exit_status.ERROR), + (True, 107, [{'code': 107, 'treat_as': 'warning'}], module.Exit_status.WARNING), ), ) def test_interpret_exit_code_respects_exit_code_and_borg_local_path( - command, + command_is_borg, exit_code, - borg_local_path, borg_exit_codes, expected_result, ): + flexmock(module).should_receive('command_is_borg').and_return(command_is_borg) + assert ( - module.interpret_exit_code(command, exit_code, borg_local_path, borg_exit_codes) + module.interpret_exit_code( + command=flexmock(), + exit_code=exit_code, + borg_local_path=flexmock(), + borg_exit_codes=borg_exit_codes, + ) is expected_result ) @@ -146,6 +154,18 @@ def test_borg_json_log_line_to_record_does_not_elevate_log_message_info_level_to assert record.name == 'borg.something' +def test_borg_json_log_line_with_none_log_level_parses_log_message_line(): + line = '{"type": "log_message", "levelname": "INFO", "time": 12345, "message": "All done", "name": "borg.something"}' + + record = module.borg_json_log_line_to_record(line, None) + + assert record.levelno == module.logging.INFO + assert record.created == 12345 + assert record.msg == 'All done' + assert record.levelname == 'INFO' + assert record.name == 'borg.something' + + def test_borg_json_log_line_to_record_parses_file_status_line(): flexmock(module.time).should_receive('time').and_return(12345) line = '{"type": "file_status", "status": "-", "path": "/foo/bar"}' @@ -1404,6 +1424,7 @@ def test_execute_command_without_run_to_completion_returns_process(): def test_execute_command_and_capture_output_returns_stdout(): full_command = ['foo', 'bar'] flexmock(module).should_receive('log_command') + flexmock(module).should_receive('command_is_borg').and_return(False) process = flexmock() flexmock(module.subprocess).should_receive('Popen').with_args( full_command, @@ -1423,8 +1444,10 @@ def test_execute_command_and_capture_output_returns_stdout(): assert output_lines == ('out',) -def test_execute_command_and_capture_output_with_capture_stderr_returns_stderr(): +def test_execute_command_and_capture_output_with_capture_stderr_popens_stderr(): full_command = ['foo', 'bar'] + flexmock(module).should_receive('log_command') + flexmock(module).should_receive('command_is_borg').and_return(False) process = flexmock() flexmock(module.subprocess).should_receive('Popen').with_args( full_command, @@ -1446,10 +1469,34 @@ def test_execute_command_and_capture_output_with_capture_stderr_returns_stderr() assert output_lines == ('out',) +def test_execute_command_and_capture_output_with_borg_command_popens_stderr(): + full_command = ['borg', 'list'] + flexmock(module).should_receive('log_command') + flexmock(module).should_receive('command_is_borg').and_return(True) + process = flexmock() + flexmock(module.subprocess).should_receive('Popen').with_args( + full_command, + stdin=None, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + shell=False, + env=None, + cwd=None, + close_fds=False, + ).and_return(process).once() + flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) + flexmock(module).should_receive('log_outputs').and_yield('out') + + output_lines = tuple(module.execute_command_and_capture_output(full_command)) + + assert output_lines == ('out',) + + def test_execute_command_and_capture_output_returns_output_when_process_error_is_not_considered_an_error(): full_command = ['foo', 'bar'] err_output = b'[]' flexmock(module).should_receive('log_command') + flexmock(module).should_receive('command_is_borg').and_return(False) flexmock(module.subprocess).should_receive('Popen').with_args( full_command, stdin=None, @@ -1472,6 +1519,7 @@ def test_execute_command_and_capture_output_returns_output_when_process_error_is def test_execute_command_and_capture_output_raises_when_command_errors(): full_command = ['foo', 'bar'] flexmock(module).should_receive('log_command') + flexmock(module).should_receive('command_is_borg').and_return(False) flexmock(module.subprocess).should_receive('Popen').with_args( full_command, stdin=None, @@ -1493,6 +1541,7 @@ def test_execute_command_and_capture_output_raises_when_command_errors(): def test_execute_command_and_capture_output_with_shell_returns_output(): full_command = ['foo', 'bar'] flexmock(module).should_receive('log_command') + flexmock(module).should_receive('command_is_borg').and_return(False) process = flexmock() flexmock(module.subprocess).should_receive('Popen').with_args( 'foo bar', @@ -1515,6 +1564,7 @@ def test_execute_command_and_capture_output_with_shell_returns_output(): def test_execute_command_and_capture_output_with_enviroment_returns_output(): full_command = ['foo', 'bar'] flexmock(module).should_receive('log_command') + flexmock(module).should_receive('command_is_borg').and_return(False) process = flexmock() flexmock(module.subprocess).should_receive('Popen').with_args( full_command, @@ -1543,6 +1593,7 @@ def test_execute_command_and_capture_output_with_enviroment_returns_output(): def test_execute_command_and_capture_output_returns_output_with_working_directory(): full_command = ['foo', 'bar'] flexmock(module).should_receive('log_command') + flexmock(module).should_receive('command_is_borg').and_return(False) process = flexmock() flexmock(module.subprocess).should_receive('Popen').with_args( full_command,