From 6ddae20fa1f2f8c8207d18f74595c96dc6f55569 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sat, 23 Jul 2022 21:02:21 -0700 Subject: [PATCH] Fix handling of "repository" and "data" consistency checks to prevent invalid Borg flags (#565). --- NEWS | 3 +++ borgmatic/borg/check.py | 15 ++++++++------- setup.py | 2 +- tests/unit/borg/test_check.py | 28 ++++++++-------------------- 4 files changed, 20 insertions(+), 28 deletions(-) diff --git a/NEWS b/NEWS index 0b37437b3..55c4fca7a 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +1.6.7.dev0 + * #565: Fix handling of "repository" and "data" consistency checks to prevent invalid Borg flags. + 1.6.6 * #559: Update documentation about configuring multiple consistency checks or multiple databases. * #560: Fix all database hooks to error when the requested database to restore isn't present in the diff --git a/borgmatic/borg/check.py b/borgmatic/borg/check.py index 83c3eea2d..665256fdc 100644 --- a/borgmatic/borg/check.py +++ b/borgmatic/borg/check.py @@ -33,8 +33,6 @@ def parse_checks(consistency_config, only_checks=None): If no "checks" option is present in the config, return the DEFAULT_CHECKS. If a checks value has a name of "disabled", return an empty tuple, meaning that no checks should be run. - - If the "data" check is present, then make sure the "archives" check is included as well. ''' checks = only_checks or tuple( check_config['name'] @@ -48,9 +46,6 @@ def parse_checks(consistency_config, only_checks=None): ) return () - if 'data' in checks and 'archives' not in checks: - return checks + ('archives',) - return checks @@ -164,7 +159,7 @@ def make_check_flags(checks, check_last=None, prefix=None): ('--repository-only',) However, if both "repository" and "archives" are in checks, then omit them from the returned - flags because Borg does both checks by default. + flags because Borg does both checks by default. If "data" is in checks, that implies "archives". Additionally, if a check_last value is given and "archives" is in checks, then include a "--last" flag. And if a prefix value is given and "archives" is in checks, then include a @@ -183,7 +178,13 @@ def make_check_flags(checks, check_last=None, prefix=None): 'Ignoring consistency prefix option, as "archives" is not in consistency checks' ) - common_flags = last_flags + prefix_flags + (('--verify-data',) if 'data' in checks else ()) + if 'data' in checks: + data_flags = ('--verify-data',) + checks += ('archives',) + else: + data_flags = () + + common_flags = last_flags + prefix_flags + data_flags if {'repository', 'archives'}.issubset(set(checks)): return common_flags diff --git a/setup.py b/setup.py index e084f4438..4b2c6ee9a 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,6 @@ from setuptools import find_packages, setup -VERSION = '1.6.6' +VERSION = '1.6.7.dev0' setup( diff --git a/tests/unit/borg/test_check.py b/tests/unit/borg/test_check.py index 0b4c0cef5..94a913369 100644 --- a/tests/unit/borg/test_check.py +++ b/tests/unit/borg/test_check.py @@ -49,18 +49,6 @@ def test_parse_checks_with_disabled_returns_no_checks(): assert checks == () -def test_parse_checks_with_data_check_also_injects_archives(): - checks = module.parse_checks({'checks': [{'name': 'data'}]}) - - assert checks == ('data', 'archives') - - -def test_parse_checks_with_data_check_passes_through_archives(): - checks = module.parse_checks({'checks': [{'name': 'data'}, {'name': 'archives'}]}) - - assert checks == ('data', 'archives') - - def test_parse_checks_prefers_override_checks_to_configured_checks(): checks = module.parse_checks( {'checks': [{'name': 'archives'}]}, only_checks=['repository', 'extract'] @@ -69,12 +57,6 @@ def test_parse_checks_prefers_override_checks_to_configured_checks(): assert checks == ('repository', 'extract') -def test_parse_checks_with_override_data_check_also_injects_archives(): - checks = module.parse_checks({'checks': [{'name': 'extract'}]}, only_checks=['data']) - - assert checks == ('data', 'archives') - - @pytest.mark.parametrize( 'frequency,expected_result', ( @@ -217,10 +199,10 @@ def test_make_check_flags_with_archives_check_returns_flag(): assert flags == ('--archives-only',) -def test_make_check_flags_with_data_check_returns_flag(): +def test_make_check_flags_with_data_check_returns_flag_and_implies_archives(): flags = module.make_check_flags(('data',)) - assert flags == ('--verify-data',) + assert flags == ('--archives-only', '--verify-data',) def test_make_check_flags_with_extract_omits_extract_flag(): @@ -229,6 +211,12 @@ def test_make_check_flags_with_extract_omits_extract_flag(): assert flags == () +def test_make_check_flags_with_repository_and_data_checks_does_not_return_repository_only(): + flags = module.make_check_flags(('repository', 'data',)) + + assert flags == ('--verify-data',) + + def test_make_check_flags_with_default_checks_and_default_prefix_returns_default_flags(): flags = module.make_check_flags(('repository', 'archives'), prefix=module.DEFAULT_PREFIX)