Deprecate validate-borgmatic-config in favor of new "config validate" action (#529).

NEWS

@@ -4,7 +4,8 @@
  * #399: Add a documentation troubleshooting note for MySQL/MariaDB authentication errors.
  * #529: Remove upgrade-borgmatic-config command for upgrading borgmatic 1.1.0 INI-style
    configuration.
- * #529: Deprecate generate-borgmatic-config in favor if new "config generate" action.
+ * #529: Deprecate generate-borgmatic-config in favor of new "config generate" action.
+ * #529: Deprecate validate-borgmatic-config in favor of new "config validate" action.
  * #697, #712: Extract borgmatic configuration from backup via new "config bootstrap" action—even
    when borgmatic has no configuration yet!
  * #669: Add sample systemd user service for running borgmatic as a non-root user.

borgmatic/actions/config/generate.py

@@ -7,6 +7,13 @@ logger = logging.getLogger(__name__)
 
 
 def run_generate(generate_arguments, global_arguments):
+    '''
+    Given the generate arguments and the global arguments, each as an argparse.Namespace instance,
+    run the "generate" action.
+
+    Raise FileExistsError if a file already exists at the destination path and the generate
+    arguments do not have overwrite set.
+    '''
     dry_run_label = ' (dry run; not actually writing anything)' if global_arguments.dry_run else ''
 
     logger.answer(

borgmatic/actions/config/validate.py

@@ -0,0 +1,22 @@
+import logging
+
+import borgmatic.config.generate
+
+logger = logging.getLogger(__name__)
+
+
+def run_validate(validate_arguments, configs):
+    '''
+    Given the validate arguments as an argparse.Namespace instance and a dict of configuration
+    filename to corresponding parsed configuration, run the "validate" action.
+
+    Most of the validation is actually performed implicitly by the standard borgmatic configuration
+    loading machinery prior to here, so this function mainly exists to support additional validate
+    flags like "--show".
+    '''
+    if validate_arguments.show:
+        for config_path, config in configs.items():
+            if len(configs) > 1:
+                logger.answer('---')
+
+            logger.answer(borgmatic.config.generate.render_configuration(config))

borgmatic/commands/arguments.py

@@ -699,8 +699,8 @@ def make_parsers():
 
     config_bootstrap_parser = config_parsers.add_parser(
         'bootstrap',
-        help='Extract the borgmatic config files from a named archive',
-        description='Extract the borgmatic config files from a named archive',
+        help='Extract the borgmatic configuration files from a named archive',
+        description='Extract the borgmatic configuration files from a named archive',
         add_help=False,
     )
     config_bootstrap_group = config_bootstrap_parser.add_argument_group(
@@ -774,6 +774,23 @@ def make_parsers():
         '-h', '--help', action='help', help='Show this help message and exit'
     )
 
+    config_validate_parser = config_parsers.add_parser(
+        'validate',
+        help='Validate that borgmatic configuration files specified with --config (see borgmatic --help)',
+        description='Validate borgmatic configuration files specified with --config (see borgmatic --help)',
+        add_help=False,
+    )
+    config_validate_group = config_validate_parser.add_argument_group('config validate arguments')
+    config_validate_group.add_argument(
+        '-s',
+        '--show',
+        action='store_true',
+        help='Show the validated configuration after all include merging has occurred',
+    )
+    config_validate_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'],

borgmatic/commands/borgmatic.py

@@ -20,6 +20,7 @@ import borgmatic.actions.check
 import borgmatic.actions.compact
 import borgmatic.actions.config.bootstrap
 import borgmatic.actions.config.generate
+import borgmatic.actions.config.validate
 import borgmatic.actions.create
 import borgmatic.actions.export_tar
 import borgmatic.actions.extract
@@ -500,6 +501,9 @@ def load_configurations(config_filenames, overrides=None, resolve_env=True):
     Given a sequence of configuration filenames, load and validate each configuration file. Return
     the results as a tuple of: dict of configuration filename to corresponding parsed configuration,
     and sequence of logging.LogRecord instances containing any parse errors.
+
+    Log records are returned here instead of being logged directly because logging isn't yet
+    initialized at this point!
     '''
     # Dict mapping from config filename to corresponding parsed config dict.
     configs = collections.OrderedDict()
@@ -507,6 +511,17 @@ def load_configurations(config_filenames, overrides=None, resolve_env=True):
 
     # Parse and load each configuration file.
     for config_filename in config_filenames:
+        logs.extend(
+            [
+                logging.makeLogRecord(
+                    dict(
+                        levelno=logging.DEBUG,
+                        levelname='DEBUG',
+                        msg=f'{config_filename}: Loading configuration file',
+                    )
+                ),
+            ]
+        )
         try:
             configs[config_filename], parse_logs = validate.parse_configuration(
                 config_filename, validate.schema_filename(), overrides, resolve_env
@@ -603,12 +618,13 @@ def get_local_path(configs):
     return next(iter(configs.values())).get('location', {}).get('local_path', 'borg')
 
 
-def collect_highlander_action_summary_logs(configs, arguments):
+def collect_highlander_action_summary_logs(configs, arguments, configuration_parse_errors):
     '''
-    Given a dict of configuration filename to corresponding parsed configuration and parsed
-    command-line arguments as a dict from subparser name to a parsed namespace of arguments, run
-    a highlander action specified in the arguments, if any, and yield a series of logging.LogRecord
-    instances containing summary information.
+    Given a dict of configuration filename to corresponding parsed configuration, parsed
+    command-line arguments as a dict from subparser name to a parsed namespace of arguments, and
+    whether any configuration files encountered errors during parsing, run a highlander action
+    specified in the arguments, if any, and yield a series of logging.LogRecord instances containing
+    summary information.
 
     A highlander action is an action that cannot coexist with other actions on the borgmatic
     command-line, and borgmatic exits after processing such an action.
@@ -662,6 +678,37 @@ def collect_highlander_action_summary_logs(configs, arguments):
 
         return
 
+    if 'validate' in arguments:
+        if configuration_parse_errors:
+            yield logging.makeLogRecord(
+                dict(
+                    levelno=logging.CRITICAL,
+                    levelname='CRITICAL',
+                    msg='Configuration validation failed',
+                )
+            )
+
+            return
+
+        try:
+            borgmatic.actions.config.validate.run_validate(arguments['validate'], configs)
+
+            yield logging.makeLogRecord(
+                dict(
+                    levelno=logging.ANSWER,
+                    levelname='ANSWER',
+                    msg='All configuration files are valid',
+                )
+            )
+        except (
+            CalledProcessError,
+            ValueError,
+            OSError,
+        ) as error:
+            yield from log_error_records(error)
+
+        return
+
 
 def collect_configuration_run_summary_logs(configs, arguments):
     '''
@@ -800,6 +847,9 @@ def main(extra_summary_logs=[]):  # pragma: no cover
     configs, parse_logs = load_configurations(
         config_filenames, global_arguments.overrides, global_arguments.resolve_env
     )
+    configuration_parse_errors = (
+        (max(log.levelno for log in parse_logs) >= logging.CRITICAL) if parse_logs else False
+    )
 
     any_json_flags = any(
         getattr(sub_arguments, 'json', False) for sub_arguments in arguments.values()
@@ -822,15 +872,17 @@ def main(extra_summary_logs=[]):  # pragma: no cover
         logger.critical(f'Error configuring logging: {error}')
         exit_with_help_link()
 
-    logger.debug('Ensuring legacy configuration is upgraded')
-
     summary_logs = (
-        parse_logs
+        extra_summary_logs
+        + parse_logs
         + (
-            list(collect_highlander_action_summary_logs(configs, arguments))
+            list(
+                collect_highlander_action_summary_logs(
+                    configs, arguments, configuration_parse_errors
+                )
+            )
             or list(collect_configuration_run_summary_logs(configs, arguments))
         )
-        + extra_summary_logs
     )
     summary_logs_max_level = max(log.levelno for log in summary_logs)
 

borgmatic/commands/validate_config.py

@@ -1,68 +1,17 @@
 import logging
 import sys
-from argparse import ArgumentParser
 
-import borgmatic.config.generate
-from borgmatic.config import collect, validate
-
-logger = logging.getLogger(__name__)
+import borgmatic.commands.borgmatic
 
 
-def parse_arguments(*arguments):
-    '''
-    Given command-line arguments with which this script was invoked, parse the arguments and return
-    them as an ArgumentParser instance.
-    '''
-    config_paths = collect.get_default_config_paths()
-
-    parser = ArgumentParser(description='Validate borgmatic configuration file(s).')
-    parser.add_argument(
-        '-c',
-        '--config',
-        nargs='+',
-        dest='config_paths',
-        default=config_paths,
-        help=f'Configuration filenames or directories, defaults to: {config_paths}',
-    )
-    parser.add_argument(
-        '-s',
-        '--show',
-        action='store_true',
-        help='Show the validated configuration after all include merging has occurred',
+def main():
+    warning_log = logging.makeLogRecord(
+        dict(
+            levelno=logging.WARNING,
+            levelname='WARNING',
+            msg='validate-borgmatic-config is deprecated and will be removed from a future release. Please use "borgmatic config validate" instead.',
+        )
     )
 
-    return parser.parse_args(arguments)
-
-
-def main():  # pragma: no cover
-    arguments = parse_arguments(*sys.argv[1:])
-
-    logging.basicConfig(level=logging.INFO, format='%(message)s')
-
-    config_filenames = tuple(collect.collect_config_filenames(arguments.config_paths))
-    if len(config_filenames) == 0:
-        logger.critical('No files to validate found')
-        sys.exit(1)
-
-    found_issues = False
-    for config_filename in config_filenames:
-        try:
-            config, parse_logs = validate.parse_configuration(
-                config_filename, validate.schema_filename()
-            )
-        except (ValueError, OSError, validate.Validation_error) as error:
-            logging.critical(f'{config_filename}: Error parsing configuration file')
-            logging.critical(error)
-            found_issues = True
-        else:
-            for log in parse_logs:
-                logger.handle(log)
-
-            if arguments.show:
-                print('---')
-                print(borgmatic.config.generate.render_configuration(config))
-
-    if found_issues:
-        sys.exit(1)
-
-    logger.info(f"All given configuration files are valid: {', '.join(config_filenames)}")
+    sys.argv = ['borgmatic', 'config', 'validate'] + sys.argv[1:]
+    borgmatic.commands.borgmatic.main([warning_log])

docs/Dockerfile

@@ -4,7 +4,7 @@ COPY . /app
 RUN apk add --no-cache py3-pip py3-ruamel.yaml py3-ruamel.yaml.clib
 RUN pip install --no-cache /app && generate-borgmatic-config && chmod +r /etc/borgmatic/config.yaml
 RUN borgmatic --help > /command-line.txt \
-    && for action in rcreate transfer create prune compact check extract config "config bootstrap" "config generate" export-tar mount umount restore rlist list rinfo info break-lock borg; do \
+    && for action in rcreate transfer create prune compact check extract config "config bootstrap" "config generate" "config validate" export-tar mount umount restore rlist list rinfo info break-lock borg; do \
            echo -e "\n--------------------------------------------------------------------------------\n" >> /command-line.txt \
            && borgmatic $action --help >> /command-line.txt; done
 

docs/how-to/make-per-application-backups.md

@@ -399,9 +399,16 @@ includes.
 
 ## Debugging includes
 
-<span class="minilink minilink-addedin">New in version 1.7.12</span> If you'd
+<span class="minilink minilink-addedin">New in version 1.7.15</span> If you'd
 like to see what the loaded configuration looks like after includes get merged
-in, run `validate-borgmatic-config` on your configuration file:
+in, run the `validate` action on your configuration file:
+
+```bash
+sudo borgmatic config validate --show
+```
+
+<span class="minilink minilink-addedin">In version 1.7.12 through
+1.7.14</span> Use this command instead:
 
 ```bash
 sudo validate-borgmatic-config --show

docs/how-to/set-up-backups.md

@@ -185,6 +185,13 @@ redundant](https://torsion.org/borgmatic/docs/how-to/make-backups-redundant/).
 If you'd like to validate that your borgmatic configuration is valid, the
 following command is available for that:
 
+```bash
+sudo borgmatic config validate
+```
+
+<span class="minilink minilink-addedin">Prior to version 1.7.15</span>
+Validate a configuration file with this command instead:
+
 ```bash
 sudo validate-borgmatic-config
 ```

tests/integration/actions/config/test_validate.py

@@ -0,0 +1,37 @@
+import argparse
+
+from flexmock import flexmock
+
+import borgmatic.logger
+from borgmatic.actions.config import validate as module
+
+
+def test_run_validate_with_show_renders_configurations():
+    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)
+
+    module.run_validate(argparse.Namespace(show=True), {'test.yaml': {'foo': {'bar': 'baz'}}})
+
+    assert log_lines == ['''foo:\n    bar: baz\n''']
+
+
+def test_run_validate_with_show_and_multiple_configs_renders_each():
+    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)
+
+    module.run_validate(
+        argparse.Namespace(show=True),
+        {'test.yaml': {'foo': {'bar': 'baz'}}, 'other.yaml': {'quux': 'value'}},
+    )
+
+    assert log_lines == ['---', 'foo:\n    bar: baz\n', '---', 'quux: value\n']

tests/integration/commands/test_validate_config.py

@@ -3,27 +3,7 @@ from flexmock import flexmock
 from borgmatic.commands import validate_config as module
 
 
-def test_parse_arguments_with_no_arguments_uses_defaults():
-    config_paths = ['default']
-    flexmock(module.collect).should_receive('get_default_config_paths').and_return(config_paths)
+def test_main_does_not_raise():
+    flexmock(module.borgmatic.commands.borgmatic).should_receive('main')
 
-    parser = module.parse_arguments()
-
-    assert parser.config_paths == config_paths
-
-
-def test_parse_arguments_with_multiple_config_paths_parses_as_list():
-    flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default'])
-
-    parser = module.parse_arguments('--config', 'myconfig', 'otherconfig')
-
-    assert parser.config_paths == ['myconfig', 'otherconfig']
-
-
-def test_parse_arguments_supports_show_flag():
-    config_paths = ['default']
-    flexmock(module.collect).should_receive('get_default_config_paths').and_return(config_paths)
-
-    parser = module.parse_arguments('--config', 'myconfig', '--show')
-
-    assert parser.show
+    module.main()

tests/unit/actions/config/test_generate.py

@@ -3,7 +3,7 @@ from flexmock import flexmock
 from borgmatic.actions.config import generate as module
 
 
-def test_run_bootstrap_does_not_raise():
+def test_run_generate_does_not_raise():
     generate_arguments = flexmock(
         source_filename=None,
         destination_filename='destination.yaml',
@@ -15,7 +15,7 @@ def test_run_bootstrap_does_not_raise():
     module.run_generate(generate_arguments, global_arguments)
 
 
-def test_run_bootstrap_with_dry_run_does_not_raise():
+def test_run_generate_with_dry_run_does_not_raise():
     generate_arguments = flexmock(
         source_filename=None,
         destination_filename='destination.yaml',
@@ -27,7 +27,7 @@ def test_run_bootstrap_with_dry_run_does_not_raise():
     module.run_generate(generate_arguments, global_arguments)
 
 
-def test_run_bootstrap_with_source_filename_does_not_raise():
+def test_run_generate_with_source_filename_does_not_raise():
     generate_arguments = flexmock(
         source_filename='source.yaml',
         destination_filename='destination.yaml',

tests/unit/actions/config/test_validate.py

@@ -0,0 +1,17 @@
+from flexmock import flexmock
+
+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')
+
+    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')
+
+    module.run_validate(validate_arguments, {'test.yaml': flexmock(), 'other.yaml': flexmock()})

tests/unit/commands/test_borgmatic.py

@@ -877,7 +877,7 @@ def test_load_configurations_collects_parsed_configurations_and_logs():
     configs, logs = tuple(module.load_configurations(('test.yaml', 'other.yaml')))
 
     assert configs == {'test.yaml': configuration, 'other.yaml': other_configuration}
-    assert logs == test_expected_logs + other_expected_logs
+    assert set(logs) >= set(test_expected_logs + other_expected_logs)
 
 
 def test_load_configurations_logs_warning_for_permission_error():
@@ -886,7 +886,7 @@ def test_load_configurations_logs_warning_for_permission_error():
     configs, logs = tuple(module.load_configurations(('test.yaml',)))
 
     assert configs == {}
-    assert {log.levelno for log in logs} == {logging.WARNING}
+    assert max(log.levelno for log in logs) == logging.WARNING
 
 
 def test_load_configurations_logs_critical_for_parse_error():
@@ -895,7 +895,7 @@ def test_load_configurations_logs_critical_for_parse_error():
     configs, logs = tuple(module.load_configurations(('test.yaml',)))
 
     assert configs == {}
-    assert {log.levelno for log in logs} == {logging.CRITICAL}
+    assert max(log.levelno for log in logs) == logging.CRITICAL
 
 
 def test_log_record_does_not_raise():
@@ -971,7 +971,9 @@ def test_collect_highlander_action_summary_logs_info_for_success_with_bootstrap(
     }
 
     logs = tuple(
-        module.collect_highlander_action_summary_logs({'test.yaml': {}}, arguments=arguments)
+        module.collect_highlander_action_summary_logs(
+            {'test.yaml': {}}, arguments=arguments, configuration_parse_errors=False
+        )
     )
     assert {log.levelno for log in logs} == {logging.ANSWER}
 
@@ -987,7 +989,9 @@ def test_collect_highlander_action_summary_logs_error_on_bootstrap_failure():
     }
 
     logs = tuple(
-        module.collect_highlander_action_summary_logs({'test.yaml': {}}, arguments=arguments)
+        module.collect_highlander_action_summary_logs(
+            {'test.yaml': {}}, arguments=arguments, configuration_parse_errors=False
+        )
     )
 
     assert {log.levelno for log in logs} == {logging.CRITICAL}
@@ -1002,7 +1006,9 @@ def test_collect_highlander_action_summary_logs_error_on_bootstrap_local_borg_ve
     }
 
     logs = tuple(
-        module.collect_highlander_action_summary_logs({'test.yaml': {}}, arguments=arguments)
+        module.collect_highlander_action_summary_logs(
+            {'test.yaml': {}}, arguments=arguments, configuration_parse_errors=False
+        )
     )
 
     assert {log.levelno for log in logs} == {logging.CRITICAL}
@@ -1016,7 +1022,9 @@ def test_collect_highlander_action_summary_logs_info_for_success_with_generate()
     }
 
     logs = tuple(
-        module.collect_highlander_action_summary_logs({'test.yaml': {}}, arguments=arguments)
+        module.collect_highlander_action_summary_logs(
+            {'test.yaml': {}}, arguments=arguments, configuration_parse_errors=False
+        )
     )
     assert {log.levelno for log in logs} == {logging.ANSWER}
 
@@ -1031,7 +1039,58 @@ def test_collect_highlander_action_summary_logs_error_on_generate_failure():
     }
 
     logs = tuple(
-        module.collect_highlander_action_summary_logs({'test.yaml': {}}, arguments=arguments)
+        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_highlander_action_summary_logs_info_for_success_with_validate():
+    flexmock(module.borgmatic.actions.config.validate).should_receive('run_validate')
+    arguments = {
+        'validate': flexmock(),
+        'global': flexmock(dry_run=False),
+    }
+
+    logs = tuple(
+        module.collect_highlander_action_summary_logs(
+            {'test.yaml': {}}, arguments=arguments, configuration_parse_errors=False
+        )
+    )
+    assert {log.levelno for log in logs} == {logging.ANSWER}
+
+
+def test_collect_highlander_action_summary_logs_error_on_validate_parse_failure():
+    flexmock(module.borgmatic.actions.config.validate).should_receive('run_validate')
+    arguments = {
+        'validate': flexmock(),
+        'global': flexmock(dry_run=False),
+    }
+
+    logs = tuple(
+        module.collect_highlander_action_summary_logs(
+            {'test.yaml': {}}, arguments=arguments, configuration_parse_errors=True
+        )
+    )
+
+    assert {log.levelno for log in logs} == {logging.CRITICAL}
+
+
+def test_collect_highlander_action_summary_logs_error_on_run_validate_failure():
+    flexmock(module.borgmatic.actions.config.validate).should_receive('run_validate').and_raise(
+        ValueError
+    )
+    arguments = {
+        'validate': flexmock(),
+        'global': flexmock(dry_run=False),
+    }
+
+    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}