From 43d0e597a2116af87b12862a882aa88e8b05fdd6 Mon Sep 17 00:00:00 2001 From: Dan Date: Sun, 29 Oct 2017 19:36:26 -0700 Subject: [PATCH] Require "prefix" in retention section when "archive_name_format" is set. --- .gitignore | 2 + NEWS | 6 ++- borgmatic/config/schema.yaml | 4 +- borgmatic/config/validate.py | 42 +++++++++++------- .../tests/integration/config/test_validate.py | 7 --- borgmatic/tests/unit/config/test_validate.py | 43 +++++++++++++++++++ 6 files changed, 79 insertions(+), 25 deletions(-) create mode 100644 borgmatic/tests/unit/config/test_validate.py diff --git a/.gitignore b/.gitignore index 2a53007..768a626 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,8 @@ *.egg-info *.pyc *.swp +.cache +.coverage .tox build dist diff --git a/NEWS b/NEWS index 71e7c8e..1b815db 100644 --- a/NEWS +++ b/NEWS @@ -2,8 +2,10 @@ * #16, #38: Support for user-defined hooks before/after backup, or on error. * #33: Improve clarity of logging spew at high verbosity levels. * #29: Support for using tilde in source directory path to reference home directory. - * Converted main source repository from Mercurial to Git. - * Updated dead links to Borg documentation. + * Require "prefix" in retention section when "archive_name_format" is set. This is to avoid + accidental pruning of archives with a different archive name format. + * Convert main source repository from Mercurial to Git. + * Update dead links to Borg documentation. 1.1.8 * #39: Fix to make /etc/borgmatic/config.yaml optional rather than required when using the default diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index b6d3c83..0c0a328 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -94,7 +94,9 @@ map: desc: | Name of the archive. Borg placeholders can be used. See the output of "borg help placeholders" for details. Default is - "{hostname}-{now:%Y-%m-%dT%H:%M:%S.%f}" + "{hostname}-{now:%Y-%m-%dT%H:%M:%S.%f}". If you specify this option, you must + also specify a prefix in the retention section to avoid accidental pruning of + archives with a different archive name format. example: "{hostname}-documents-{now}" retention: desc: | diff --git a/borgmatic/config/validate.py b/borgmatic/config/validate.py index b29f07b..1ce3924 100644 --- a/borgmatic/config/validate.py +++ b/borgmatic/config/validate.py @@ -25,6 +25,31 @@ class Validation_error(ValueError): self.config_filename = config_filename self.error_messages = error_messages + def __str__(self): + ''' + Render a validation error as a user-facing string. + ''' + return 'An error occurred while parsing a configuration file at {}:\n'.format( + self.config_filename + ) + '\n'.join(self.error_messages) + + +def apply_logical_validation(config_filename, parsed_configuration): + ''' + Given a parsed and schematically valid configuration as a data structure of nested dicts (see + below), run through any additional logical validation checks. If there are any such validation + problems, raise a Validation_error. + ''' + archive_name_format = parsed_configuration.get('storage', {}).get('archive_name_format') + prefix = parsed_configuration.get('retention', {}).get('prefix') + + if archive_name_format and not prefix: + raise Validation_error( + config_filename, ( + 'If you provide an archive_name_format, you must also specify a retention prefix.', + ) + ) + def parse_configuration(config_filename, schema_filename): ''' @@ -58,19 +83,6 @@ def parse_configuration(config_filename, schema_filename): if validator.validation_errors: raise Validation_error(config_filename, validator.validation_errors) + apply_logical_validation(config_filename, parsed_result) + return parsed_result - - -def display_validation_error(validation_error): - ''' - Given a Validation_error, display its error messages to stderr. - ''' - print( - 'An error occurred while parsing a configuration file at {}:'.format( - validation_error.config_filename - ), - file=sys.stderr, - ) - - for error in validation_error.error_messages: - print(error, file=sys.stderr) diff --git a/borgmatic/tests/integration/config/test_validate.py b/borgmatic/tests/integration/config/test_validate.py index eec282c..3892ca2 100644 --- a/borgmatic/tests/integration/config/test_validate.py +++ b/borgmatic/tests/integration/config/test_validate.py @@ -148,10 +148,3 @@ def test_parse_configuration_raises_for_validation_error(): with pytest.raises(module.Validation_error): module.parse_configuration('config.yaml', 'schema.yaml') - - -def test_display_validation_error_does_not_raise(): - flexmock(sys.modules['builtins']).should_receive('print') - error = module.Validation_error('config.yaml', ('oops', 'uh oh')) - - module.display_validation_error(error) diff --git a/borgmatic/tests/unit/config/test_validate.py b/borgmatic/tests/unit/config/test_validate.py new file mode 100644 index 0000000..9a943a0 --- /dev/null +++ b/borgmatic/tests/unit/config/test_validate.py @@ -0,0 +1,43 @@ +import pytest + +from borgmatic.config import validate as module + + +def test_validation_error_str_contains_error_messages_and_config_filename(): + error = module.Validation_error('config.yaml', ('oops', 'uh oh')) + + result = str(error) + + assert 'config.yaml' in result + assert 'oops' in result + assert 'uh oh' in result + + +def test_apply_logical_validation_raises_if_archive_name_format_present_without_prefix(): + with pytest.raises(module.Validation_error): + module.apply_logical_validation( + 'config.yaml', + { + 'storage': {'archive_name_format': '{hostname}-{now}'}, + 'retention': {'keep_daily': 7}, + }, + ) + + +def test_apply_logical_validation_does_not_raise_if_archive_name_format_and_prefix_present(): + module.apply_logical_validation( + 'config.yaml', + { + 'storage': {'archive_name_format': '{hostname}-{now}'}, + 'retention': {'prefix': '{hostname}-'}, + }, + ) + + +def test_apply_logical_validation_does_not_raise_otherwise(): + module.apply_logical_validation( + 'config.yaml', + { + 'retention': {'keep_secondly': 1000}, + }, + )