From 4a94c2c9bff7e7e17db595086a92647e18a23d23 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 13 Apr 2023 14:39:36 -0700 Subject: [PATCH] Selectively omit list values when including configuration files (#672). --- NEWS | 3 + borgmatic/config/load.py | 32 ++- docs/how-to/make-per-application-backups.md | 60 ++++- tests/integration/config/test_load.py | 285 +++++++++++++++++++- 4 files changed, 362 insertions(+), 18 deletions(-) diff --git a/NEWS b/NEWS index 6d4f544..a39e84d 100644 --- a/NEWS +++ b/NEWS @@ -10,6 +10,9 @@ * #672: Selectively shallow merge certain mappings or sequences when including configuration files. See the documentation for more information: https://torsion.org/borgmatic/docs/how-to/make-per-application-backups/#shallow-merge + * #672: Selectively omit list values when including configuration files. See the documentation for + more information: + https://torsion.org/borgmatic/docs/how-to/make-per-application-backups/#list-merge * #673: View the results of configuration file merging via "validate-borgmatic-config --show" flag. See the documentation for more information: https://torsion.org/borgmatic/docs/how-to/make-per-application-backups/#debugging-includes diff --git a/borgmatic/config/load.py b/borgmatic/config/load.py index e9caeb4..068e661 100644 --- a/borgmatic/config/load.py +++ b/borgmatic/config/load.py @@ -38,9 +38,9 @@ def include_configuration(loader, filename_node, include_directory): return load_configuration(include_filename) -def retain_node_error(loader, node): +def raise_retain_node_error(loader, node): ''' - Given a ruamel.yaml.loader.Loader and a YAML node, raise an error. + Given a ruamel.yaml.loader.Loader and a YAML node, raise an error about "!retain" usage. Raise ValueError if a mapping or sequence node is given, as that indicates that "!retain" was used in a configuration file without a merge. In configuration files with a merge, mapping and @@ -56,6 +56,19 @@ def retain_node_error(loader, node): raise ValueError('The !retain tag may only be used on a YAML mapping or sequence.') +def raise_omit_node_error(loader, node): + ''' + Given a ruamel.yaml.loader.Loader and a YAML node, raise an error about "!omit" usage. + + Raise ValueError unconditionally, as an "!omit" node here indicates it was used in a + configuration file without a merge. In configuration files with a merge, nodes with "!omit" + tags are handled by deep_merge_nodes() below. + ''' + raise ValueError( + 'The !omit tag may only be used on a scalar (e.g., string) list element within a configuration file containing a merged !include tag.' + ) + + class Include_constructor(ruamel.yaml.SafeConstructor): ''' A YAML "constructor" (a ruamel.yaml concept) that supports a custom "!include" tag for including @@ -68,7 +81,8 @@ class Include_constructor(ruamel.yaml.SafeConstructor): '!include', functools.partial(include_configuration, include_directory=include_directory), ) - self.add_constructor('!retain', retain_node_error) + self.add_constructor('!retain', raise_retain_node_error) + self.add_constructor('!omit', raise_omit_node_error) def flatten_mapping(self, node): ''' @@ -134,6 +148,16 @@ def load_configuration(filename): return config +def filter_omitted_nodes(nodes): + ''' + Given a list of nodes, return a filtered list omitting any nodes with an "!omit" tag or with a + value matching such nodes. + ''' + omitted_values = tuple(node.value for node in nodes if node.tag == '!omit') + + return [node for node in nodes if node.value not in omitted_values] + + DELETED_NODE = object() @@ -247,7 +271,7 @@ def deep_merge_nodes(nodes): b_key, ruamel.yaml.nodes.SequenceNode( tag=b_value.tag, - value=a_value.value + b_value.value, + value=filter_omitted_nodes(a_value.value + b_value.value), start_mark=b_value.start_mark, end_mark=b_value.end_mark, flow_style=b_value.flow_style, diff --git a/docs/how-to/make-per-application-backups.md b/docs/how-to/make-per-application-backups.md index 887bc43..9ee93f4 100644 --- a/docs/how-to/make-per-application-backups.md +++ b/docs/how-to/make-per-application-backups.md @@ -272,9 +272,65 @@ Once this include gets merged in, the resulting configuration would have a When there's an option collision between the local file and the merged include, the local file's option takes precedence. + +#### List merge + New in version 1.6.1 Colliding list values are appended together. +New in version 1.7.12 If there +is a list value from an include that you *don't* want in your local +configuration file, you can omit it with an `!omit` tag. For instance: + +```yaml +<<: !include /etc/borgmatic/common.yaml + +location: + source_directories: + - !omit /home + - /var +``` + +And `common.yaml` like this: + +```yaml +location: + source_directories: + - /home + - /etc +``` + +Once this include gets merged in, the resulting configuration will have a +`source_directories` value of `/etc` and `/var`—with `/home` omitted. + +This feature currently only works on scalar (e.g. string or number) list items +and will not work elsewhere in a configuration file. Be sure to put the +`!omit` tag *before* the list item (after the dash). Putting `!omit` after the +list item will not work, as it gets interpreted as part of the string. Here's +an example of some things not to do: + +```yaml +<<: !include /etc/borgmatic/common.yaml + +location: + source_directories: + # Do not do this! It will not work. "!omit" belongs before "/home". + - /home !omit + + # Do not do this either! "!omit" only works on scalar list items. + repositories: !omit + # Also do not do this for the same reason! This is a list item, but it's + # not a scalar. + - !omit path: repo.borg +``` + +Additionally, the `!omit` tag only works in a configuration file that also +performs a merge include with `<<: !include`. It doesn't make sense within, +for instance, an included configuration file itself (unless it in turn +performs its own merge include). That's because `!omit` only applies to the +file doing the include; it doesn't work in reverse or propagate through +includes. + ### Shallow merge @@ -296,7 +352,7 @@ on the `retention` mapping: location: repositories: - - repo.borg + - path: repo.borg retention: !retain keep_daily: 5 @@ -307,7 +363,7 @@ And `common.yaml` like this: ```yaml location: repositories: - - common.borg + - path: common.borg retention: keep_hourly: 24 diff --git a/tests/integration/config/test_load.py b/tests/integration/config/test_load.py index ef9be0f..028a652 100644 --- a/tests/integration/config/test_load.py +++ b/tests/integration/config/test_load.py @@ -211,7 +211,7 @@ def test_load_configuration_with_retain_tag_but_without_merge_include_raises(): builtins.should_receive('open').with_args('config.yaml').and_return(config_file) with pytest.raises(ValueError): - assert module.load_configuration('config.yaml') + module.load_configuration('config.yaml') def test_load_configuration_with_retain_tag_on_scalar_raises(): @@ -239,7 +239,156 @@ def test_load_configuration_with_retain_tag_on_scalar_raises(): builtins.should_receive('open').with_args('config.yaml').and_return(config_file) with pytest.raises(ValueError): - assert module.load_configuration('config.yaml') + module.load_configuration('config.yaml') + + +def test_load_configuration_with_omit_tag_merges_include_and_omits_requested_values(): + builtins = flexmock(sys.modules['builtins']) + flexmock(module.os).should_receive('getcwd').and_return('/tmp') + flexmock(module.os.path).should_receive('isabs').and_return(False) + flexmock(module.os.path).should_receive('exists').and_return(True) + include_file = io.StringIO( + ''' + stuff: + - a + - b + - c + ''' + ) + include_file.name = 'include.yaml' + builtins.should_receive('open').with_args('/tmp/include.yaml').and_return(include_file) + config_file = io.StringIO( + ''' + stuff: + - x + - !omit b + - y + <<: !include include.yaml + ''' + ) + config_file.name = 'config.yaml' + builtins.should_receive('open').with_args('config.yaml').and_return(config_file) + + assert module.load_configuration('config.yaml') == {'stuff': ['a', 'c', 'x', 'y']} + + +def test_load_configuration_with_omit_tag_on_unknown_value_merges_include_and_does_not_raise(): + builtins = flexmock(sys.modules['builtins']) + flexmock(module.os).should_receive('getcwd').and_return('/tmp') + flexmock(module.os.path).should_receive('isabs').and_return(False) + flexmock(module.os.path).should_receive('exists').and_return(True) + include_file = io.StringIO( + ''' + stuff: + - a + - b + - c + ''' + ) + include_file.name = 'include.yaml' + builtins.should_receive('open').with_args('/tmp/include.yaml').and_return(include_file) + config_file = io.StringIO( + ''' + stuff: + - x + - !omit q + - y + <<: !include include.yaml + ''' + ) + config_file.name = 'config.yaml' + builtins.should_receive('open').with_args('config.yaml').and_return(config_file) + + assert module.load_configuration('config.yaml') == {'stuff': ['a', 'b', 'c', 'x', 'y']} + + +def test_load_configuration_with_omit_tag_on_non_list_item_raises(): + builtins = flexmock(sys.modules['builtins']) + flexmock(module.os).should_receive('getcwd').and_return('/tmp') + flexmock(module.os.path).should_receive('isabs').and_return(False) + flexmock(module.os.path).should_receive('exists').and_return(True) + include_file = io.StringIO( + ''' + stuff: + - a + - b + - c + ''' + ) + include_file.name = 'include.yaml' + builtins.should_receive('open').with_args('/tmp/include.yaml').and_return(include_file) + config_file = io.StringIO( + ''' + stuff: !omit + - x + - y + <<: !include include.yaml + ''' + ) + config_file.name = 'config.yaml' + builtins.should_receive('open').with_args('config.yaml').and_return(config_file) + + with pytest.raises(ValueError): + module.load_configuration('config.yaml') + + +def test_load_configuration_with_omit_tag_on_non_scalar_list_item_raises(): + builtins = flexmock(sys.modules['builtins']) + flexmock(module.os).should_receive('getcwd').and_return('/tmp') + flexmock(module.os.path).should_receive('isabs').and_return(False) + flexmock(module.os.path).should_receive('exists').and_return(True) + include_file = io.StringIO( + ''' + stuff: + - foo: bar + baz: quux + ''' + ) + include_file.name = 'include.yaml' + builtins.should_receive('open').with_args('/tmp/include.yaml').and_return(include_file) + config_file = io.StringIO( + ''' + stuff: + - !omit foo: bar + baz: quux + <<: !include include.yaml + ''' + ) + config_file.name = 'config.yaml' + builtins.should_receive('open').with_args('config.yaml').and_return(config_file) + + with pytest.raises(ValueError): + module.load_configuration('config.yaml') + + +def test_load_configuration_with_omit_tag_but_without_merge_raises(): + builtins = flexmock(sys.modules['builtins']) + flexmock(module.os).should_receive('getcwd').and_return('/tmp') + flexmock(module.os.path).should_receive('isabs').and_return(False) + flexmock(module.os.path).should_receive('exists').and_return(True) + include_file = io.StringIO( + ''' + stuff: + - a + - !omit b + - c + ''' + ) + include_file.name = 'include.yaml' + builtins.should_receive('open').with_args('/tmp/include.yaml').and_return(include_file) + config_file = io.StringIO( + ''' + stuff: + - x + - y + <<: !include include.yaml + ''' + ) + config_file.name = 'config.yaml' + builtins.should_receive('open').with_args('config.yaml').and_return(config_file) + + with pytest.raises(ValueError): + module.load_configuration('config.yaml') def test_load_configuration_does_not_merge_include_list(): @@ -277,13 +426,33 @@ def test_load_configuration_does_not_merge_include_list(): module.ruamel.yaml.nodes.ScalarNode, ), ) -def test_retain_node_error_raises(node_class): +def test_raise_retain_node_error_raises(node_class): with pytest.raises(ValueError): - module.retain_node_error( + module.raise_retain_node_error( loader=flexmock(), node=node_class(tag=flexmock(), value=flexmock()) ) +def test_raise_omit_node_error_raises(): + with pytest.raises(ValueError): + module.raise_omit_node_error(loader=flexmock(), node=flexmock()) + + +def test_filter_omitted_nodes(): + nodes = [ + module.ruamel.yaml.nodes.ScalarNode(tag='tag:yaml.org,2002:str', value='a'), + module.ruamel.yaml.nodes.ScalarNode(tag='tag:yaml.org,2002:str', value='b'), + module.ruamel.yaml.nodes.ScalarNode(tag='tag:yaml.org,2002:str', value='c'), + module.ruamel.yaml.nodes.ScalarNode(tag='tag:yaml.org,2002:str', value='a'), + module.ruamel.yaml.nodes.ScalarNode(tag='!omit', value='b'), + module.ruamel.yaml.nodes.ScalarNode(tag='tag:yaml.org,2002:str', value='c'), + ] + + result = module.filter_omitted_nodes(nodes) + + assert [item.value for item in result] == ['a', 'c', 'a', 'c'] + + def test_deep_merge_nodes_replaces_colliding_scalar_values(): node_values = [ ( @@ -483,7 +652,15 @@ def test_deep_merge_nodes_appends_colliding_sequence_values(): tag='tag:yaml.org,2002:str', value='before_backup' ), module.ruamel.yaml.nodes.SequenceNode( - tag='tag:yaml.org,2002:seq', value=['echo 1', 'echo 2'] + tag='tag:yaml.org,2002:seq', + value=[ + module.ruamel.yaml.ScalarNode( + tag='tag:yaml.org,2002:str', value='echo 1' + ), + module.ruamel.yaml.ScalarNode( + tag='tag:yaml.org,2002:str', value='echo 2' + ), + ], ), ), ], @@ -499,7 +676,15 @@ def test_deep_merge_nodes_appends_colliding_sequence_values(): tag='tag:yaml.org,2002:str', value='before_backup' ), module.ruamel.yaml.nodes.SequenceNode( - tag='tag:yaml.org,2002:seq', value=['echo 3', 'echo 4'] + tag='tag:yaml.org,2002:seq', + value=[ + module.ruamel.yaml.ScalarNode( + tag='tag:yaml.org,2002:str', value='echo 3' + ), + module.ruamel.yaml.ScalarNode( + tag='tag:yaml.org,2002:str', value='echo 4' + ), + ], ), ), ], @@ -514,10 +699,10 @@ def test_deep_merge_nodes_appends_colliding_sequence_values(): options = section_value.value assert len(options) == 1 assert options[0][0].value == 'before_backup' - assert options[0][1].value == ['echo 1', 'echo 2', 'echo 3', 'echo 4'] + assert [item.value for item in options[0][1].value] == ['echo 1', 'echo 2', 'echo 3', 'echo 4'] -def test_deep_merge_nodes_keeps_mapping_values_tagged_with_retain(): +def test_deep_merge_nodes_only_keeps_mapping_values_tagged_with_retain(): node_values = [ ( module.ruamel.yaml.nodes.ScalarNode(tag='tag:yaml.org,2002:str', value='retention'), @@ -568,7 +753,7 @@ def test_deep_merge_nodes_keeps_mapping_values_tagged_with_retain(): assert options[0][1].value == '5' -def test_deep_merge_nodes_keeps_sequence_values_tagged_with_retain(): +def test_deep_merge_nodes_only_keeps_sequence_values_tagged_with_retain(): node_values = [ ( module.ruamel.yaml.nodes.ScalarNode(tag='tag:yaml.org,2002:str', value='hooks'), @@ -580,7 +765,15 @@ def test_deep_merge_nodes_keeps_sequence_values_tagged_with_retain(): tag='tag:yaml.org,2002:str', value='before_backup' ), module.ruamel.yaml.nodes.SequenceNode( - tag='tag:yaml.org,2002:seq', value=['echo 1', 'echo 2'] + tag='tag:yaml.org,2002:seq', + value=[ + module.ruamel.yaml.ScalarNode( + tag='tag:yaml.org,2002:str', value='echo 1' + ), + module.ruamel.yaml.ScalarNode( + tag='tag:yaml.org,2002:str', value='echo 2' + ), + ], ), ), ], @@ -596,7 +789,15 @@ def test_deep_merge_nodes_keeps_sequence_values_tagged_with_retain(): tag='tag:yaml.org,2002:str', value='before_backup' ), module.ruamel.yaml.nodes.SequenceNode( - tag='!retain', value=['echo 3', 'echo 4'] + tag='!retain', + value=[ + module.ruamel.yaml.ScalarNode( + tag='tag:yaml.org,2002:str', value='echo 3' + ), + module.ruamel.yaml.ScalarNode( + tag='tag:yaml.org,2002:str', value='echo 4' + ), + ], ), ), ], @@ -612,4 +813,64 @@ def test_deep_merge_nodes_keeps_sequence_values_tagged_with_retain(): assert len(options) == 1 assert options[0][0].value == 'before_backup' assert options[0][1].tag == 'tag:yaml.org,2002:seq' - assert options[0][1].value == ['echo 3', 'echo 4'] + assert [item.value for item in options[0][1].value] == ['echo 3', 'echo 4'] + + +def test_deep_merge_nodes_skips_sequence_values_tagged_with_omit(): + node_values = [ + ( + module.ruamel.yaml.nodes.ScalarNode(tag='tag:yaml.org,2002:str', value='hooks'), + module.ruamel.yaml.nodes.MappingNode( + tag='tag:yaml.org,2002:map', + value=[ + ( + module.ruamel.yaml.nodes.ScalarNode( + tag='tag:yaml.org,2002:str', value='before_backup' + ), + module.ruamel.yaml.nodes.SequenceNode( + tag='tag:yaml.org,2002:seq', + value=[ + module.ruamel.yaml.ScalarNode( + tag='tag:yaml.org,2002:str', value='echo 1' + ), + module.ruamel.yaml.ScalarNode( + tag='tag:yaml.org,2002:str', value='echo 2' + ), + ], + ), + ), + ], + ), + ), + ( + module.ruamel.yaml.nodes.ScalarNode(tag='tag:yaml.org,2002:str', value='hooks'), + module.ruamel.yaml.nodes.MappingNode( + tag='tag:yaml.org,2002:map', + value=[ + ( + module.ruamel.yaml.nodes.ScalarNode( + tag='tag:yaml.org,2002:str', value='before_backup' + ), + module.ruamel.yaml.nodes.SequenceNode( + tag='tag:yaml.org,2002:seq', + value=[ + module.ruamel.yaml.ScalarNode(tag='!omit', value='echo 2'), + module.ruamel.yaml.ScalarNode( + tag='tag:yaml.org,2002:str', value='echo 3' + ), + ], + ), + ), + ], + ), + ), + ] + + result = module.deep_merge_nodes(node_values) + assert len(result) == 1 + (section_key, section_value) = result[0] + assert section_key.value == 'hooks' + options = section_value.value + assert len(options) == 1 + assert options[0][0].value == 'before_backup' + assert [item.value for item in options[0][1].value] == ['echo 1', 'echo 3']