From d4f48a3a9e1e42f9de41cfbcc399f9df4c5fb730 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 6 Mar 2025 11:23:24 -0800 Subject: [PATCH] Initial work on unified command hooks (#790). --- NEWS | 3 + borgmatic/actions/check.py | 19 -- borgmatic/actions/compact.py | 18 -- borgmatic/actions/create.py | 19 -- borgmatic/actions/extract.py | 17 -- borgmatic/actions/prune.py | 17 -- borgmatic/commands/borgmatic.py | 84 ++++--- borgmatic/config/generate.py | 66 +++--- borgmatic/config/normalize.py | 84 +++++++ borgmatic/config/schema.yaml | 254 +++++++++++++++++++--- borgmatic/hooks/command.py | 98 ++++++--- borgmatic/hooks/dispatch.py | 16 +- tests/integration/config/test_generate.py | 32 --- 13 files changed, 469 insertions(+), 258 deletions(-) diff --git a/NEWS b/NEWS index a41ebd1e..b8b9a7cb 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,9 @@ * #409: With the PagerDuty monitoring hook, send borgmatic logs to PagerDuty so they show up in the incident UI. See the documentation for more information: https://torsion.org/borgmatic/docs/how-to/monitor-your-backups/#pagerduty-hook + * #790: Deprecate all "before_*", "after_*" and "on_error" command hooks in favor of more flexible + "commands:". See the documentation for more information: + https://torsion.org/borgmatic/docs/how-to/add-preparation-and-cleanup-steps-to-backups/ * #936: Clarify Zabbix monitoring hook documentation about creating items: https://torsion.org/borgmatic/docs/how-to/monitor-your-backups/#zabbix-hook * #1017: Fix a regression in which some MariaDB/MySQL passwords were not escaped correctly. diff --git a/borgmatic/actions/check.py b/borgmatic/actions/check.py index 2fcca67d..b562d760 100644 --- a/borgmatic/actions/check.py +++ b/borgmatic/actions/check.py @@ -682,7 +682,6 @@ def run_check( config_filename, repository, config, - hook_context, local_borg_version, check_arguments, global_arguments, @@ -699,15 +698,6 @@ def run_check( ): return - borgmatic.hooks.command.execute_hook( - config.get('before_check'), - config.get('umask'), - config_filename, - 'pre-check', - global_arguments.dry_run, - **hook_context, - ) - logger.info('Running consistency checks') repository_id = borgmatic.borg.check.get_repository_id( @@ -772,12 +762,3 @@ def run_check( borgmatic_runtime_directory, ) write_check_time(make_check_time_path(config, repository_id, 'spot')) - - borgmatic.hooks.command.execute_hook( - config.get('after_check'), - config.get('umask'), - config_filename, - 'post-check', - global_arguments.dry_run, - **hook_context, - ) diff --git a/borgmatic/actions/compact.py b/borgmatic/actions/compact.py index b0b070bf..a8ab6a6f 100644 --- a/borgmatic/actions/compact.py +++ b/borgmatic/actions/compact.py @@ -12,7 +12,6 @@ def run_compact( config_filename, repository, config, - hook_context, local_borg_version, compact_arguments, global_arguments, @@ -28,14 +27,6 @@ def run_compact( ): return - borgmatic.hooks.command.execute_hook( - config.get('before_compact'), - config.get('umask'), - config_filename, - 'pre-compact', - global_arguments.dry_run, - **hook_context, - ) if borgmatic.borg.feature.available(borgmatic.borg.feature.Feature.COMPACT, local_borg_version): logger.info(f'Compacting segments{dry_run_label}') borgmatic.borg.compact.compact_segments( @@ -52,12 +43,3 @@ def run_compact( ) else: # pragma: nocover logger.info('Skipping compact (only available/needed in Borg 1.2+)') - - borgmatic.hooks.command.execute_hook( - config.get('after_compact'), - config.get('umask'), - config_filename, - 'post-compact', - global_arguments.dry_run, - **hook_context, - ) diff --git a/borgmatic/actions/create.py b/borgmatic/actions/create.py index dede448a..11928f62 100644 --- a/borgmatic/actions/create.py +++ b/borgmatic/actions/create.py @@ -261,7 +261,6 @@ def run_create( repository, config, config_paths, - hook_context, local_borg_version, create_arguments, global_arguments, @@ -279,15 +278,6 @@ def run_create( ): return - borgmatic.hooks.command.execute_hook( - config.get('before_backup'), - config.get('umask'), - config_filename, - 'pre-backup', - global_arguments.dry_run, - **hook_context, - ) - logger.info(f'Creating archive{dry_run_label}') working_directory = borgmatic.config.paths.get_working_directory(config) @@ -343,12 +333,3 @@ def run_create( borgmatic_runtime_directory, global_arguments.dry_run, ) - - borgmatic.hooks.command.execute_hook( - config.get('after_backup'), - config.get('umask'), - config_filename, - 'post-backup', - global_arguments.dry_run, - **hook_context, - ) diff --git a/borgmatic/actions/extract.py b/borgmatic/actions/extract.py index 0216f820..6e2e7900 100644 --- a/borgmatic/actions/extract.py +++ b/borgmatic/actions/extract.py @@ -12,7 +12,6 @@ def run_extract( config_filename, repository, config, - hook_context, local_borg_version, extract_arguments, global_arguments, @@ -22,14 +21,6 @@ def run_extract( ''' Run the "extract" action for the given repository. ''' - borgmatic.hooks.command.execute_hook( - config.get('before_extract'), - config.get('umask'), - config_filename, - 'pre-extract', - global_arguments.dry_run, - **hook_context, - ) if extract_arguments.repository is None or borgmatic.config.validate.repositories_match( repository, extract_arguments.repository ): @@ -56,11 +47,3 @@ def run_extract( strip_components=extract_arguments.strip_components, progress=extract_arguments.progress, ) - borgmatic.hooks.command.execute_hook( - config.get('after_extract'), - config.get('umask'), - config_filename, - 'post-extract', - global_arguments.dry_run, - **hook_context, - ) diff --git a/borgmatic/actions/prune.py b/borgmatic/actions/prune.py index 83dfe39f..e7caa158 100644 --- a/borgmatic/actions/prune.py +++ b/borgmatic/actions/prune.py @@ -11,7 +11,6 @@ def run_prune( config_filename, repository, config, - hook_context, local_borg_version, prune_arguments, global_arguments, @@ -27,14 +26,6 @@ def run_prune( ): return - borgmatic.hooks.command.execute_hook( - config.get('before_prune'), - config.get('umask'), - config_filename, - 'pre-prune', - global_arguments.dry_run, - **hook_context, - ) logger.info(f'Pruning archives{dry_run_label}') borgmatic.borg.prune.prune_archives( global_arguments.dry_run, @@ -46,11 +37,3 @@ def run_prune( local_path=local_path, remote_path=remote_path, ) - borgmatic.hooks.command.execute_hook( - config.get('after_prune'), - config.get('umask'), - config_filename, - 'post-prune', - global_arguments.dry_run, - **hook_context, - ) diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index a55f18ab..276f92e0 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -96,6 +96,13 @@ def run_configuration(config_filename, config, config_paths, arguments): f"Skipping {'/'.join(skip_actions)} action{'s' if len(skip_actions) > 1 else ''} due to configured skip_actions" ) + command.execute_hooks( + command.filter_hooks(config.get('commands'), before='configuration', action_names=arguments.keys()), + config.get('umask'), + global_arguments.dry_run, + configuration_filename=config_filename, + ) + try: local_borg_version = borg_version.local_borg_version(config, local_path) logger.debug(f'Borg {local_borg_version}') @@ -226,18 +233,26 @@ def run_configuration(config_filename, config, config_paths, arguments): encountered_error = error yield from log_error_records(f'{config_filename}: Error pinging monitor', error) + if encountered_error: + command.execute_hooks( + command.filter_hooks(config.get('commands'), after='error', action_names=arguments.keys()), + config.get('umask'), + global_arguments.dry_run, + configuration_filename=config_filename, + repository=error_repository, + error=encountered_error, + output=getattr(encountered_error, 'output', ''), + ) + + command.execute_hooks( + command.filter_hooks(config.get('commands'), after='configuration', action_names=arguments.keys()), + config.get('umask'), + global_arguments.dry_run, + configuration_filename=config_filename, + ) + if encountered_error and using_primary_action: try: - command.execute_hook( - config.get('on_error'), - config.get('umask'), - config_filename, - 'on-error', - global_arguments.dry_run, - repository=error_repository, - error=encountered_error, - output=getattr(encountered_error, 'output', ''), - ) dispatch.call_hooks( 'ping_monitor', config, @@ -289,6 +304,7 @@ def run_actions( global_arguments = arguments['global'] dry_run_label = ' (dry run; not making any changes)' if global_arguments.dry_run else '' hook_context = { + 'configuration_filename': config_filename, 'repository_label': repository.get('label', ''), 'log_file': global_arguments.log_file if global_arguments.log_file else '', # Deprecated: For backwards compatibility with borgmatic < 1.6.0. @@ -297,16 +313,24 @@ def run_actions( } skip_actions = set(get_skip_actions(config, arguments)) - command.execute_hook( - config.get('before_actions'), + command.execute_hooks( + command.filter_hooks(config.get('commands'), before='repository', action_names=arguments.keys()), config.get('umask'), - config_filename, - 'pre-actions', global_arguments.dry_run, **hook_context, ) for action_name, action_arguments in arguments.items(): + if action_name == 'global': + continue + + command.execute_hooks( + command.filter_hooks(config.get('commands'), before='action', action_names=arguments.keys()), + config.get('umask'), + global_arguments.dry_run, + **hook_context, + ) + if action_name == 'repo-create' and action_name not in skip_actions: borgmatic.actions.repo_create.run_repo_create( repository, @@ -333,7 +357,6 @@ def run_actions( repository, config, config_paths, - hook_context, local_borg_version, action_arguments, global_arguments, @@ -346,7 +369,6 @@ def run_actions( config_filename, repository, config, - hook_context, local_borg_version, action_arguments, global_arguments, @@ -359,7 +381,6 @@ def run_actions( config_filename, repository, config, - hook_context, local_borg_version, action_arguments, global_arguments, @@ -373,7 +394,6 @@ def run_actions( config_filename, repository, config, - hook_context, local_borg_version, action_arguments, global_arguments, @@ -385,7 +405,6 @@ def run_actions( config_filename, repository, config, - hook_context, local_borg_version, action_arguments, global_arguments, @@ -523,11 +542,16 @@ def run_actions( remote_path, ) - command.execute_hook( - config.get('after_actions'), + command.execute_hooks( + command.filter_hooks(config.get('commands'), after='action', action_names=arguments.keys()), + config.get('umask'), + global_arguments.dry_run, + **hook_context, + ) + + command.execute_hooks( + command.filter_hooks(config.get('commands'), after='repository', action_names=arguments.keys()), config.get('umask'), - config_filename, - 'post-actions', global_arguments.dry_run, **hook_context, ) @@ -813,12 +837,11 @@ def collect_configuration_run_summary_logs(configs, config_paths, arguments): if 'create' in arguments: try: for config_filename, config in configs.items(): - command.execute_hook( - config.get('before_everything'), + command.execute_hooks( + command.filter_hooks(config.get('commands'), before='everything', action_names=arguments.keys()), config.get('umask'), - config_filename, - 'pre-everything', arguments['global'].dry_run, + configuration_filename=config_filename, ) except (CalledProcessError, ValueError, OSError) as error: yield from log_error_records('Error running pre-everything hook', error) @@ -865,12 +888,11 @@ def collect_configuration_run_summary_logs(configs, config_paths, arguments): if 'create' in arguments: try: for config_filename, config in configs.items(): - command.execute_hook( - config.get('after_everything'), + command.execute_hooks( + command.filter_hooks(config.get('commands'), after='everything', action_names=arguments.keys()), config.get('umask'), - config_filename, - 'post-everything', arguments['global'].dry_run, + configuration_filename=config_filename, ) except (CalledProcessError, ValueError, OSError) as error: yield from log_error_records('Error running post-everything hook', error) diff --git a/borgmatic/config/generate.py b/borgmatic/config/generate.py index 6b375b76..11cf0964 100644 --- a/borgmatic/config/generate.py +++ b/borgmatic/config/generate.py @@ -1,5 +1,6 @@ import collections import io +import itertools import os import re @@ -24,20 +25,28 @@ def insert_newline_before_comment(config, field_name): def get_properties(schema): ''' Given a schema dict, return its properties. But if it's got sub-schemas with multiple different - potential properties, returned their merged properties instead. + potential properties, returned their merged properties instead (interleaved so the first + properties of each sub-schema come first). The idea is that the user should see all possible + options even if they're not all possible together. ''' if 'oneOf' in schema: return dict( - collections.ChainMap(*[sub_schema['properties'] for sub_schema in schema['oneOf']]) + item + for item in itertools.chain( + *itertools.zip_longest( + *[sub_schema['properties'].items() for sub_schema in schema['oneOf']] + ) + ) + if item is not None ) return schema['properties'] -def schema_to_sample_configuration(schema, level=0, parent_is_sequence=False): +def schema_to_sample_configuration(schema, source_config, level=0, parent_is_sequence=False): ''' - Given a loaded configuration schema, generate and return sample config for it. Include comments - for each option based on the schema "description". + Given a loaded configuration schema and a source configuration, generate and return sample + config for the schema. Include comments for each option based on the schema "description". ''' schema_type = schema.get('type') example = schema.get('example') @@ -47,19 +56,22 @@ def schema_to_sample_configuration(schema, level=0, parent_is_sequence=False): if schema_type == 'array' or (isinstance(schema_type, list) and 'array' in schema_type): config = ruamel.yaml.comments.CommentedSeq( - [schema_to_sample_configuration(schema['items'], level, parent_is_sequence=True)] + [schema_to_sample_configuration(schema['items'], source_config, level, parent_is_sequence=True)] ) add_comments_to_configuration_sequence(config, schema, indent=(level * INDENT)) elif schema_type == 'object' or (isinstance(schema_type, list) and 'object' in schema_type): + if source_config and isinstance(source_config, list) and isinstance(source_config[0], dict): + source_config = dict(collections.ChainMap(*source_config)) + config = ruamel.yaml.comments.CommentedMap( [ - (field_name, schema_to_sample_configuration(sub_schema, level + 1)) + (field_name, schema_to_sample_configuration(sub_schema, source_config.get(field_name, {}), level + 1)) for field_name, sub_schema in get_properties(schema).items() ] ) indent = (level * INDENT) + (SEQUENCE_INDENT if parent_is_sequence else 0) add_comments_to_configuration_object( - config, schema, indent=indent, skip_first=parent_is_sequence + config, schema, source_config, indent=indent, skip_first=parent_is_sequence ) else: raise ValueError(f'Schema at level {level} is unsupported: {schema}') @@ -179,14 +191,19 @@ def add_comments_to_configuration_sequence(config, schema, indent=0): return -REQUIRED_KEYS = {'source_directories', 'repositories', 'keep_daily'} +DEFAULT_KEYS = {'source_directories', 'repositories', 'keep_daily'} COMMENTED_OUT_SENTINEL = 'COMMENT_OUT' -def add_comments_to_configuration_object(config, schema, indent=0, skip_first=False): +def add_comments_to_configuration_object(config, schema, source_config, indent=0, skip_first=False): ''' Using descriptions from a schema as a source, add those descriptions as comments to the given - config mapping, before each field. Indent the comment the given number of characters. + configuration dict, putting them before each field. Indent the comment the given number of + characters. + + And a sentinel for commenting out options that are neither in DEFAULT_KEYS nor the the given + source configuration dict. The idea is that any options used in the source configuration should + stay active in the generated configuration. ''' for index, field_name in enumerate(config.keys()): if skip_first and index == 0: @@ -195,10 +212,10 @@ def add_comments_to_configuration_object(config, schema, indent=0, skip_first=Fa field_schema = get_properties(schema).get(field_name, {}) description = field_schema.get('description', '').strip() - # If this is an optional key, add an indicator to the comment flagging it to be commented + # If this isn't a default key, add an indicator to the comment flagging it to be commented # out from the sample configuration. This sentinel is consumed by downstream processing that # does the actual commenting out. - if field_name not in REQUIRED_KEYS: + if field_name not in DEFAULT_KEYS and field_name not in source_config: description = ( '\n'.join((description, COMMENTED_OUT_SENTINEL)) if description @@ -218,21 +235,6 @@ def add_comments_to_configuration_object(config, schema, indent=0, skip_first=Fa RUAMEL_YAML_COMMENTS_INDEX = 1 -def remove_commented_out_sentinel(config, field_name): - ''' - Given a configuration CommentedMap and a top-level field name in it, remove any "commented out" - sentinel found at the end of its YAML comments. This prevents the given field name from getting - commented out by downstream processing that consumes the sentinel. - ''' - try: - last_comment_value = config.ca.items[field_name][RUAMEL_YAML_COMMENTS_INDEX][-1].value - except KeyError: - return - - if last_comment_value == f'# {COMMENTED_OUT_SENTINEL}\n': - config.ca.items[field_name][RUAMEL_YAML_COMMENTS_INDEX].pop() - - def merge_source_configuration_into_destination(destination_config, source_config): ''' Deep merge the given source configuration dict into the destination configuration CommentedMap, @@ -247,12 +249,6 @@ def merge_source_configuration_into_destination(destination_config, source_confi return source_config for field_name, source_value in source_config.items(): - # Since this key/value is from the source configuration, leave it uncommented and remove any - # sentinel that would cause it to get commented out. - remove_commented_out_sentinel( - ruamel.yaml.comments.CommentedMap(destination_config), field_name - ) - # This is a mapping. Recurse for this key/value. if isinstance(source_value, collections.abc.Mapping): destination_config[field_name] = merge_source_configuration_into_destination( @@ -298,7 +294,7 @@ def generate_sample_configuration( normalize.normalize(source_filename, source_config) destination_config = merge_source_configuration_into_destination( - schema_to_sample_configuration(schema), source_config + schema_to_sample_configuration(schema, source_config), source_config ) if dry_run: diff --git a/borgmatic/config/normalize.py b/borgmatic/config/normalize.py index a869b24a..1f34c071 100644 --- a/borgmatic/config/normalize.py +++ b/borgmatic/config/normalize.py @@ -58,6 +58,89 @@ def normalize_sections(config_filename, config): return [] +def make_command_hook_deprecation_log(config_filename, option_name): + ''' + Given a configuration filename and the name of a configuration option, return a deprecation + warning log for it. + ''' + return logging.makeLogRecord( + dict( + levelno=logging.WARNING, + levelname='WARNING', + msg=f'{config_filename}: {option_name} is deprecated and support will be removed from a future release. Use commands: instead.', + ) + ) + + +def normalize_commands(config_filename, config): + ''' + Given a configuration filename and a configuration dict, transform any "before_*"- and + "after_*"-style command hooks into "commands:". + ''' + logs = [] + + # Normalize "before_actions" and "after_actions". + for preposition in ('before', 'after'): + option_name = f'{preposition}_actions' + commands = config.pop(option_name, None) + + if commands: + logs.append(make_command_hook_deprecation_log(config_filename, option_name)) + config.setdefault('commands', []).append( + { + preposition: 'repository', + 'run': commands, + } + ) + + # Normalize "before_backup", "before_prune", "after_backup", "after_prune", etc. + for action_name in ('create', 'prune', 'compact', 'check', 'extract'): + for preposition in ('before', 'after'): + option_name = f'{preposition}_{"backup" if action_name == "create" else action_name}' + commands = config.pop(option_name, None) + + if not commands: + continue + + logs.append(make_command_hook_deprecation_log(config_filename, option_name)) + config.setdefault('commands', []).append( + { + preposition: 'action', + 'when': [action_name], + 'run': commands, + } + ) + + # Normalize "on_error". + commands = config.pop('on_error', None) + + if commands: + logs.append(make_command_hook_deprecation_log(config_filename, 'on_error')) + config.setdefault('commands', []).append( + { + 'after': 'error', + 'when': ['create', 'prune', 'compact', 'check'], + 'run': commands, + } + ) + + # Normalize "before_everything" and "after_everything". + for preposition in ('before', 'after'): + option_name = f'{preposition}_everything' + commands = config.pop(option_name, None) + + if commands: + logs.append(make_command_hook_deprecation_log(config_filename, option_name)) + config.setdefault('commands', []).append( + { + preposition: 'everything', + 'run': commands, + } + ) + + return logs + + def normalize(config_filename, config): ''' Given a configuration filename and a configuration dict of its loaded contents, apply particular @@ -67,6 +150,7 @@ def normalize(config_filename, config): Raise ValueError the configuration cannot be normalized. ''' logs = normalize_sections(config_filename, config) + logs += normalize_commands(config_filename, config) if config.get('borgmatic_source_directory'): logs.append( diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index f6ec574e..4d62e5fe 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -796,8 +796,9 @@ properties: items: type: string description: | - List of one or more shell commands or scripts to execute before all - the actions for each repository. + Deprecated. Use "commands:" instead. List of one or more shell + commands or scripts to execute before all the actions for each + repository. example: - "echo Starting actions." before_backup: @@ -805,8 +806,9 @@ properties: items: type: string description: | - List of one or more shell commands or scripts to execute before - creating a backup, run once per repository. + Deprecated. Use "commands:" instead. List of one or more shell + commands or scripts to execute before creating a backup, run once + per repository. example: - "echo Starting a backup." before_prune: @@ -814,8 +816,9 @@ properties: items: type: string description: | - List of one or more shell commands or scripts to execute before - pruning, run once per repository. + Deprecated. Use "commands:" instead. List of one or more shell + commands or scripts to execute before pruning, run once per + repository. example: - "echo Starting pruning." before_compact: @@ -823,8 +826,9 @@ properties: items: type: string description: | - List of one or more shell commands or scripts to execute before - compaction, run once per repository. + Deprecated. Use "commands:" instead. List of one or more shell + commands or scripts to execute before compaction, run once per + repository. example: - "echo Starting compaction." before_check: @@ -832,8 +836,9 @@ properties: items: type: string description: | - List of one or more shell commands or scripts to execute before - consistency checks, run once per repository. + Deprecated. Use "commands:" instead. List of one or more shell + commands or scripts to execute before consistency checks, run once + per repository. example: - "echo Starting checks." before_extract: @@ -841,8 +846,9 @@ properties: items: type: string description: | - List of one or more shell commands or scripts to execute before - extracting a backup, run once per repository. + Deprecated. Use "commands:" instead. List of one or more shell + commands or scripts to execute before extracting a backup, run once + per repository. example: - "echo Starting extracting." after_backup: @@ -850,8 +856,9 @@ properties: items: type: string description: | - List of one or more shell commands or scripts to execute after - creating a backup, run once per repository. + Deprecated. Use "commands:" instead. List of one or more shell + commands or scripts to execute after creating a backup, run once per + repository. example: - "echo Finished a backup." after_compact: @@ -859,8 +866,9 @@ properties: items: type: string description: | - List of one or more shell commands or scripts to execute after - compaction, run once per repository. + Deprecated. Use "commands:" instead. List of one or more shell + commands or scripts to execute after compaction, run once per + repository. example: - "echo Finished compaction." after_prune: @@ -868,8 +876,9 @@ properties: items: type: string description: | - List of one or more shell commands or scripts to execute after - pruning, run once per repository. + Deprecated. Use "commands:" instead. List of one or more shell + commands or scripts to execute after pruning, run once per + repository. example: - "echo Finished pruning." after_check: @@ -877,8 +886,9 @@ properties: items: type: string description: | - List of one or more shell commands or scripts to execute after - consistency checks, run once per repository. + Deprecated. Use "commands:" instead. List of one or more shell + commands or scripts to execute after consistency checks, run once + per repository. example: - "echo Finished checks." after_extract: @@ -886,8 +896,9 @@ properties: items: type: string description: | - List of one or more shell commands or scripts to execute after - extracting a backup, run once per repository. + Deprecated. Use "commands:" instead. List of one or more shell + commands or scripts to execute after extracting a backup, run once + per repository. example: - "echo Finished extracting." after_actions: @@ -895,8 +906,9 @@ properties: items: type: string description: | - List of one or more shell commands or scripts to execute after all - actions for each repository. + Deprecated. Use "commands:" instead. List of one or more shell + commands or scripts to execute after all actions for each + repository. example: - "echo Finished actions." on_error: @@ -904,9 +916,10 @@ properties: items: type: string description: | - List of one or more shell commands or scripts to execute when an - exception occurs during a "create", "prune", "compact", or "check" - action or an associated before/after hook. + Deprecated. Use "commands:" instead. List of one or more shell + commands or scripts to execute when an exception occurs during a + "create", "prune", "compact", or "check" action or an associated + before/after hook. example: - "echo Error during create/prune/compact/check." before_everything: @@ -914,10 +927,10 @@ properties: items: type: string description: | - List of one or more shell commands or scripts to execute before - running all actions (if one of them is "create"). These are - collected from all configuration files and then run once before all - of them (prior to all actions). + Deprecated. Use "commands:" instead. List of one or more shell + commands or scripts to execute before running all actions (if one of + them is "create"). These are collected from all configuration files + and then run once before all of them (prior to all actions). example: - "echo Starting actions." after_everything: @@ -925,12 +938,183 @@ properties: items: type: string description: | - List of one or more shell commands or scripts to execute after - running all actions (if one of them is "create"). These are - collected from all configuration files and then run once after all - of them (after any action). + Deprecated. Use "commands:" instead. List of one or more shell + commands or scripts to execute after running all actions (if one of + them is "create"). These are collected from all configuration files + and then run once after all of them (after any action). example: - "echo Completed actions." + commands: + type: array + items: + type: object + oneOf: + - required: [before, run] + additionalProperties: false + properties: + before: + type: string + enum: + - action + - repository + - configuration + - everything + - dump_data_sources + - restore_data_source_dump + description: | + Name for the point in borgmatic's execution before + which the commands should be run (required if + "after" isn't set): + * "action" runs before each action for each + repository. + * "repository" runs before all actions for each + repository. + * "configuration" runs before all actions and + repositories in the current configuration file. + * "everything" runs before all configuration + files. + * "dump_data_sources" runs before each data + source is dumped. + * "restore_data_source_dumps" runs before each + data source is restored. + example: action + hooks: + type: array + items: + type: string + description: | + List of names of other hooks that this command + hook applies to. Defaults to all hooks of the + relevant type. Only supported for + "dump_data_sources" and + "restore_data_source_dumps" hooks. + example: postgresql + when: + type: array + items: + type: string + enum: + - repo-create + - transfer + - prune + - compact + - create + - check + - delete + - extract + - config + - export-tar + - mount + - umount + - repo-delete + - restore + - repo-list + - list + - repo-info + - info + - break-lock + - key + - borg + description: | + List of actions for which the commands will be run. Defaults to + running for all actions. Ignored for "dump_data_sources" and + "restore_data_source_dump", which by their nature only run for + particular actions. + example: [create, prune, compact, check] + run: + type: array + items: + type: string + description: | + List of one or more shell commands or scripts to + run when this command hook is triggered. Required. + example: + - "echo Doing stuff." + - required: [after, run] + additionalProperties: false + properties: + after: + type: string + enum: + - action + - repository + - configuration + - everything + - error + - dump_data_sources + - restore_data_source_dump + description: | + Name for the point in borgmatic's execution after + which the commands should be run (required if + "before" isn't set): + * "action" runs after each action for each + repository. + * "repository" runs after all actions for each + repository. + * "configuration" runs after all actions and + repositories in the current configuration file. + * "everything" runs after all configuration + files. + * "error" runs after an error occurs. + * "dump_data_sources" runs after each data + source is dumped. + * "restore_data_source_dumps" runs after each + data source is restored. + example: action + hooks: + type: array + items: + type: string + description: | + List of names of other hooks that this command + hook applies to. Defaults to all hooks of the + relevant type. Only supported for + "dump_data_sources" and + "restore_data_source_dumps" hooks. + example: postgresql + when: + type: array + items: + type: string + enum: + - repo-create + - transfer + - prune + - compact + - create + - check + - delete + - extract + - config + - export-tar + - mount + - umount + - repo-delete + - restore + - repo-list + - list + - repo-info + - info + - break-lock + - key + - borg + description: | + List of actions for which the commands will be run. + Defaults to running for all actions. + example: [create, prune, compact, check] + run: + type: array + items: + type: string + description: | + List of one or more shell commands or scripts to + run when this command hook is triggered. Required. + example: + - "echo Doing stuff." + description: | + List of one or more command hooks to execute, triggered at + particular points during borgmatic's execution. For each command + hook, specify one of "before" or "after", not both. bootstrap: type: object properties: diff --git a/borgmatic/hooks/command.py b/borgmatic/hooks/command.py index 9e605290..a423e0c3 100644 --- a/borgmatic/hooks/command.py +++ b/borgmatic/hooks/command.py @@ -5,6 +5,7 @@ import shlex import sys import borgmatic.execute +import borgmatic.logger logger = logging.getLogger(__name__) @@ -44,54 +45,83 @@ def make_environment(current_environment, sys_module=sys): return environment -def execute_hook(commands, umask, config_filename, description, dry_run, **context): +def filter_hooks(command_hooks, before=None, after=None, hook_name=None, action_names=None): ''' - Given a list of hook commands to execute, a umask to execute with (or None), a config filename, - a hook description, and whether this is a dry run, run the given commands. Or, don't run them - if this is a dry run. + Given a sequence of command hook dicts from configuration and one or more filters (before name, + after name, calling hook name, or a sequence of action names), filter down the command hooks to + just the ones that match the given filters. + ''' + return tuple( + hook_config + for hook_config in command_hooks + for config_hook_names in (hook_config.get('hooks'),) + for config_action_names in (hook_config.get('when'),) + if before is None or hook_config.get('before') == before + if after is None or hook_config.get('after') == after + if hook_name is None or config_hook_names is None or hook_name in config_hook_names + if action_names is None or config_action_names is None or set(config_action_names or ()).intersection(set(action_names)) + ) + + +def execute_hooks(command_hooks, umask, dry_run, **context): + ''' + Given a sequence of command hook dicts from configuration, a umask to execute with (or None), + and whether this is a dry run, run the commands for each hook. Or don't run them if this is a + dry run. The context contains optional values interpolated by name into the hook commands. - Raise ValueError if the umask cannot be parsed. + Raise ValueError if the umask cannot be parsed or a hook is invalid. Raise subprocesses.CalledProcessError if an error occurs in a hook. ''' - if not commands: - logger.debug(f'No commands to run for {description} hook') - return + borgmatic.logger.add_custom_log_levels() dry_run_label = ' (dry run; not actually running hooks)' if dry_run else '' - context['configuration_filename'] = config_filename - commands = [interpolate_context(description, command, context) for command in commands] + for hook_config in command_hooks: + commands = hook_config.get('run') - if len(commands) == 1: - logger.info(f'Running command for {description} hook{dry_run_label}') - else: - logger.info( - f'Running {len(commands)} commands for {description} hook{dry_run_label}', - ) + if 'before' in hook_config: + description = f'before {hook_config.get("before")}' + elif 'after' in hook_config: + description = f'after {hook_config.get("after")}' + else: + raise ValueError('Invalid hook configuration: {hook_config}') - if umask: - parsed_umask = int(str(umask), 8) - logger.debug(f'Set hook umask to {oct(parsed_umask)}') - original_umask = os.umask(parsed_umask) - else: - original_umask = None + if not commands: + logger.debug(f'No commands to run for {description} hook') + return - try: - for command in commands: - if dry_run: - continue + commands = [interpolate_context(description, command, context) for command in commands] - borgmatic.execute.execute_command( - [command], - output_log_level=(logging.ERROR if description == 'on-error' else logging.WARNING), - shell=True, - environment=make_environment(os.environ), + if len(commands) == 1: + logger.info(f'Running command for {description} hook{dry_run_label}') + else: + logger.info( + f'Running {len(commands)} commands for {description} hook{dry_run_label}', ) - finally: - if original_umask: - os.umask(original_umask) + + if umask: + parsed_umask = int(str(umask), 8) + logger.debug(f'Setting hook umask to {oct(parsed_umask)}') + original_umask = os.umask(parsed_umask) + else: + original_umask = None + + try: + for command in commands: + if dry_run: + continue + + borgmatic.execute.execute_command( + [command], + output_log_level=(logging.ERROR if hook_config.get('after') == 'error' else logging.ANSWER), + shell=True, + environment=make_environment(os.environ), + ) + finally: + if original_umask: + os.umask(original_umask) def considered_soft_failure(error): diff --git a/borgmatic/hooks/dispatch.py b/borgmatic/hooks/dispatch.py index 26b0832c..970d5d5d 100644 --- a/borgmatic/hooks/dispatch.py +++ b/borgmatic/hooks/dispatch.py @@ -3,6 +3,7 @@ import importlib import logging import pkgutil +import borgmatic.hooks.command import borgmatic.hooks.credential import borgmatic.hooks.data_source import borgmatic.hooks.monitoring @@ -62,7 +63,20 @@ def call_hook(function_name, config, hook_name, *args, **kwargs): logger.debug(f'Calling {hook_name} hook function {function_name}') - return getattr(module, function_name)(hook_config, config, *args, **kwargs) + borgmatic.hooks.command.execute_hooks( + borgmatic.hooks.command.filter_hooks(config.get('commands'), before=function_name, hook_name=hook_name), + config.get('umask'), + dry_run=False, # FIXME: Need to get this from somewhere. + ) + + try: + return getattr(module, function_name)(hook_config, config, *args, **kwargs) + finally: + borgmatic.hooks.command.execute_hooks( + borgmatic.hooks.command.filter_hooks(config.get('commands'), after=function_name, hook_name=hook_name), + config.get('umask'), + dry_run=False, # FIXME: Need to get this from somewhere. + ) def call_hooks(function_name, config, hook_type, *args, **kwargs): diff --git a/tests/integration/config/test_generate.py b/tests/integration/config/test_generate.py index 0781e8d1..2db4823a 100644 --- a/tests/integration/config/test_generate.py +++ b/tests/integration/config/test_generate.py @@ -170,38 +170,6 @@ def test_add_comments_to_configuration_object_with_skip_first_does_not_raise(): module.add_comments_to_configuration_object(config, schema, skip_first=True) -def test_remove_commented_out_sentinel_keeps_other_comments(): - field_name = 'foo' - config = module.ruamel.yaml.comments.CommentedMap([(field_name, 33)]) - config.yaml_set_comment_before_after_key(key=field_name, before='Actual comment.\nCOMMENT_OUT') - - module.remove_commented_out_sentinel(config, field_name) - - comments = config.ca.items[field_name][module.RUAMEL_YAML_COMMENTS_INDEX] - assert len(comments) == 1 - assert comments[0].value == '# Actual comment.\n' - - -def test_remove_commented_out_sentinel_without_sentinel_keeps_other_comments(): - field_name = 'foo' - config = module.ruamel.yaml.comments.CommentedMap([(field_name, 33)]) - config.yaml_set_comment_before_after_key(key=field_name, before='Actual comment.') - - module.remove_commented_out_sentinel(config, field_name) - - comments = config.ca.items[field_name][module.RUAMEL_YAML_COMMENTS_INDEX] - assert len(comments) == 1 - assert comments[0].value == '# Actual comment.\n' - - -def test_remove_commented_out_sentinel_on_unknown_field_does_not_raise(): - field_name = 'foo' - config = module.ruamel.yaml.comments.CommentedMap([(field_name, 33)]) - config.yaml_set_comment_before_after_key(key=field_name, before='Actual comment.') - - module.remove_commented_out_sentinel(config, 'unknown') - - def test_generate_sample_configuration_does_not_raise(): builtins = flexmock(sys.modules['builtins']) builtins.should_receive('open').with_args('schema.yaml').and_return('')