Deep merging when including common configuration (#381).

NEWS

@@ -1,13 +1,16 @@
 1.6.0.dev0
- * #473: Instead of executing "before" command hooks before all borgmatic actions run (and "after"
-   hooks after), execute these hooks right before/after the corresponding action. E.g.,
+ * #381: BREAKING: Greatly simplify configuration file reuse by deep merging when including common
+   configuration. See the documentation for more information:
+   https://torsion.org/borgmatic/docs/how-to/make-per-application-backups/#include-merging
+ * #473: BREAKING: Instead of executing "before" command hooks before all borgmatic actions run (and
+   "after" hooks after), execute these hooks right before/after the corresponding action. E.g.,
    "before_check" now runs immediately before the "check" action. This better supports running
    timing-sensitive tasks like pausing containers. Side effect: before/after command hooks now run
    once for each configured repository instead of once per configuration file. Additionally, the
    "repositories" interpolated variable has been changed to "repository", containing the path to the
    current repository for the hook.
  * #513: Add mention of sudo's "secure_path" option in borgmatic installation documentation.
- * #515: Fix "borgmatic borg key ..." to pass parameters to Borg in correct order.
+ * #515: Fix "borgmatic borg key ..." to pass parameters to Borg in the correct order.
  * #516: Fix handling of TERM signal to exit borgmatic, not just forward the signal to Borg.
  * #517: Fix borgmatic exit code (so it's zero) when initial Borg calls fail but later retries
    succeed.

borgmatic/config/load.py

@@ -28,6 +28,107 @@ def include_configuration(loader, filename_node):
     return load_configuration(os.path.expanduser(filename_node.value))
 
 
+DELETED_NODE = object()
+
+
+def deep_merge_nodes(nodes):
+    '''
+    Given a nested borgmatic configuration data structure as a list of tuples in the form of:
+
+        (
+            ruamel.yaml.nodes.ScalarNode as a key,
+            ruamel.yaml.nodes.MappingNode or other Node as a value,
+        ),
+
+    ... deep merge any node values corresponding to duplicate keys and return the result. If
+    there are colliding keys with non-MappingNode values (e.g., integers or strings), the last
+    of the values wins.
+
+    For instance, given node values of:
+
+        [
+            (
+                ScalarNode(tag='tag:yaml.org,2002:str', value='retention'),
+                MappingNode(tag='tag:yaml.org,2002:map', value=[
+                    (
+                        ScalarNode(tag='tag:yaml.org,2002:str', value='keep_hourly'),
+                        ScalarNode(tag='tag:yaml.org,2002:int', value='24')
+                    ),
+                    (
+                        ScalarNode(tag='tag:yaml.org,2002:str', value='keep_daily'),
+                        ScalarNode(tag='tag:yaml.org,2002:int', value='7')
+                    ),
+                ]),
+            ),
+            (
+                ScalarNode(tag='tag:yaml.org,2002:str', value='retention'),
+                MappingNode(tag='tag:yaml.org,2002:map', value=[
+                    (
+                        ScalarNode(tag='tag:yaml.org,2002:str', value='keep_daily'),
+                        ScalarNode(tag='tag:yaml.org,2002:int', value='5')
+                    ),
+                ]),
+            ),
+        ]
+
+    ... the returned result would be:
+
+        [
+            (
+                ScalarNode(tag='tag:yaml.org,2002:str', value='retention'),
+                MappingNode(tag='tag:yaml.org,2002:map', value=[
+                    (
+                        ScalarNode(tag='tag:yaml.org,2002:str', value='keep_hourly'),
+                        ScalarNode(tag='tag:yaml.org,2002:int', value='24')
+                    ),
+                    (
+                        ScalarNode(tag='tag:yaml.org,2002:str', value='keep_daily'),
+                        ScalarNode(tag='tag:yaml.org,2002:int', value='5')
+                    ),
+                ]),
+            ),
+        ]
+
+    The purpose of deep merging like this is to support, for instance, merging one borgmatic
+    configuration file into another for reuse, such that a configuration section ("retention",
+    etc.) does not completely replace the corresponding section in a merged file.
+    '''
+    # Map from original node key/value to the replacement merged node. DELETED_NODE as a replacement
+    # node indications deletion.
+    replaced_nodes = {}
+
+    # To find nodes that require merging, compare each node with each other node.
+    for a_key, a_value in nodes:
+        for b_key, b_value in nodes:
+            # If we've already considered one of the nodes for merging, skip it.
+            if (a_key, a_value) in replaced_nodes or (b_key, b_value) in replaced_nodes:
+                continue
+
+            # If the keys match and the values are different, we need to merge these two A and B nodes.
+            if a_key.tag == b_key.tag and a_key.value == b_key.value and a_value != b_value:
+                # Since we're merging into the B node, consider the A node a duplicate and remove it.
+                replaced_nodes[(a_key, a_value)] = DELETED_NODE
+
+                # If we're dealing with MappingNodes, recurse and merge its values as well.
+                if isinstance(b_value, ruamel.yaml.nodes.MappingNode):
+                    replaced_nodes[(b_key, b_value)] = (
+                        b_key,
+                        ruamel.yaml.nodes.MappingNode(
+                            tag=b_value.tag,
+                            value=deep_merge_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,
+                            comment=b_value.comment,
+                            anchor=b_value.anchor,
+                        ),
+                    )
+
+    return [
+        replaced_nodes.get(node, node) for node in nodes if replaced_nodes.get(node) != DELETED_NODE
+    ]
+
+
 class Include_constructor(ruamel.yaml.SafeConstructor):
     '''
     A YAML "constructor" (a ruamel.yaml concept) that supports a custom "!include" tag for including
@@ -40,14 +141,19 @@ class Include_constructor(ruamel.yaml.SafeConstructor):
 
     def flatten_mapping(self, node):
         '''
-        Support the special case of shallow merging included configuration into an existing mapping
+        Support the special case of deep merging included configuration into an existing mapping
         using the YAML '<<' merge key. Example syntax:
 
         ```
         retention:
             keep_daily: 1
-            <<: !include common.yaml
+
+        <<: !include common.yaml
         ```
+
+        These includes are deep merged into the current configuration file. For instance, in this
+        example, any "retention" options in common.yaml will get merged into the "retention" section
+        in the example configuration file.
         '''
         representer = ruamel.yaml.representer.SafeRepresenter()
 
@@ -57,3 +163,5 @@ class Include_constructor(ruamel.yaml.SafeConstructor):
                 node.value[index] = (key_node, included_value)
 
         super(Include_constructor, self).flatten_mapping(node)
+
+        node.value = deep_merge_nodes(node.value)

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

@@ -80,7 +80,7 @@ location:
 !include /etc/borgmatic/common_retention.yaml
 ```
 
-But if you do want to merge in a YAML key and its values, keep reading!
+But if you do want to merge in a YAML key *and* its values, keep reading!
 
 
 ## Include merging
@@ -89,31 +89,35 @@ If you need to get even fancier and pull in common configuration options while
 potentially overriding individual options, you can perform a YAML merge of
 included configuration using the YAML `<<` key. For instance, here's an
 example of a main configuration file that pulls in two retention options via
-an include, and then overrides one of them locally:
+an include and then overrides one of them locally:
 
 ```yaml
+<<: !include /etc/borgmatic/common.yaml
+
 location:
    ...
 
 retention:
     keep_daily: 5
-    <<: !include /etc/borgmatic/common_retention.yaml
 ```
 
-This is what `common_retention.yaml` might look like:
+This is what `common.yaml` might look like:
 
 ```yaml
-keep_hourly: 24
-keep_daily: 7
+retention:
+    keep_hourly: 24
+    keep_daily: 7
 ```
 
 Once this include gets merged in, the resulting configuration would have a
 `keep_hourly` value of `24` and an overridden `keep_daily` value of `5`.
 
-When there is a collision of an option between the local file and the merged
-include, the local file's option takes precedent. And note that this is a
-shallow merge rather than a deep merge, so the merging does not descend into
-nested values.
+When there's an option collision between the local file and the merged
+include, the local file's option takes precedent. And as of borgmatic 1.6.0,
+this feature performs a deep merge, meaning that values are merged at all
+levels in the two configuration files. This allows you to include common
+configuration—up to full borgmatic configuration files—while overriding only
+the parts you want to customize.
 
 Note that this `<<` include merging syntax is only for merging in mappings
 (keys/values). If you'd like to include other types like scalars or lists

tests/integration/config/test_load.py

@@ -60,3 +60,184 @@ def test_load_configuration_does_not_merge_include_list():
 
     with pytest.raises(ruamel.yaml.error.YAMLError):
         assert module.load_configuration('config.yaml')
+
+
+def test_deep_merge_nodes_replaces_colliding_scalar_values():
+    node_values = [
+        (
+            ruamel.yaml.nodes.ScalarNode(tag='tag:yaml.org,2002:str', value='retention'),
+            ruamel.yaml.nodes.MappingNode(
+                tag='tag:yaml.org,2002:map',
+                value=[
+                    (
+                        ruamel.yaml.nodes.ScalarNode(
+                            tag='tag:yaml.org,2002:str', value='keep_hourly'
+                        ),
+                        ruamel.yaml.nodes.ScalarNode(tag='tag:yaml.org,2002:int', value='24'),
+                    ),
+                    (
+                        ruamel.yaml.nodes.ScalarNode(
+                            tag='tag:yaml.org,2002:str', value='keep_daily'
+                        ),
+                        ruamel.yaml.nodes.ScalarNode(tag='tag:yaml.org,2002:int', value='7'),
+                    ),
+                ],
+            ),
+        ),
+        (
+            ruamel.yaml.nodes.ScalarNode(tag='tag:yaml.org,2002:str', value='retention'),
+            ruamel.yaml.nodes.MappingNode(
+                tag='tag:yaml.org,2002:map',
+                value=[
+                    (
+                        ruamel.yaml.nodes.ScalarNode(
+                            tag='tag:yaml.org,2002:str', value='keep_daily'
+                        ),
+                        ruamel.yaml.nodes.ScalarNode(tag='tag:yaml.org,2002:int', value='5'),
+                    ),
+                ],
+            ),
+        ),
+    ]
+
+    result = module.deep_merge_nodes(node_values)
+    assert len(result) == 1
+    (section_key, section_value) = result[0]
+    assert section_key.value == 'retention'
+    options = section_value.value
+    assert len(options) == 2
+    assert options[0][0].value == 'keep_hourly'
+    assert options[0][1].value == '24'
+    assert options[1][0].value == 'keep_daily'
+    assert options[1][1].value == '5'
+
+
+def test_deep_merge_nodes_keeps_non_colliding_scalar_values():
+    node_values = [
+        (
+            ruamel.yaml.nodes.ScalarNode(tag='tag:yaml.org,2002:str', value='retention'),
+            ruamel.yaml.nodes.MappingNode(
+                tag='tag:yaml.org,2002:map',
+                value=[
+                    (
+                        ruamel.yaml.nodes.ScalarNode(
+                            tag='tag:yaml.org,2002:str', value='keep_hourly'
+                        ),
+                        ruamel.yaml.nodes.ScalarNode(tag='tag:yaml.org,2002:int', value='24'),
+                    ),
+                    (
+                        ruamel.yaml.nodes.ScalarNode(
+                            tag='tag:yaml.org,2002:str', value='keep_daily'
+                        ),
+                        ruamel.yaml.nodes.ScalarNode(tag='tag:yaml.org,2002:int', value='7'),
+                    ),
+                ],
+            ),
+        ),
+        (
+            ruamel.yaml.nodes.ScalarNode(tag='tag:yaml.org,2002:str', value='retention'),
+            ruamel.yaml.nodes.MappingNode(
+                tag='tag:yaml.org,2002:map',
+                value=[
+                    (
+                        ruamel.yaml.nodes.ScalarNode(
+                            tag='tag:yaml.org,2002:str', value='keep_minutely'
+                        ),
+                        ruamel.yaml.nodes.ScalarNode(tag='tag:yaml.org,2002:int', value='10'),
+                    ),
+                ],
+            ),
+        ),
+    ]
+
+    result = module.deep_merge_nodes(node_values)
+    assert len(result) == 1
+    (section_key, section_value) = result[0]
+    assert section_key.value == 'retention'
+    options = section_value.value
+    assert len(options) == 3
+    assert options[0][0].value == 'keep_hourly'
+    assert options[0][1].value == '24'
+    assert options[1][0].value == 'keep_daily'
+    assert options[1][1].value == '7'
+    assert options[2][0].value == 'keep_minutely'
+    assert options[2][1].value == '10'
+
+
+def test_deep_merge_nodes_keeps_deeply_nested_values():
+    node_values = [
+        (
+            ruamel.yaml.nodes.ScalarNode(tag='tag:yaml.org,2002:str', value='storage'),
+            ruamel.yaml.nodes.MappingNode(
+                tag='tag:yaml.org,2002:map',
+                value=[
+                    (
+                        ruamel.yaml.nodes.ScalarNode(
+                            tag='tag:yaml.org,2002:str', value='lock_wait'
+                        ),
+                        ruamel.yaml.nodes.ScalarNode(tag='tag:yaml.org,2002:int', value='5'),
+                    ),
+                    (
+                        ruamel.yaml.nodes.ScalarNode(
+                            tag='tag:yaml.org,2002:str', value='extra_borg_options'
+                        ),
+                        ruamel.yaml.nodes.MappingNode(
+                            tag='tag:yaml.org,2002:map',
+                            value=[
+                                (
+                                    ruamel.yaml.nodes.ScalarNode(
+                                        tag='tag:yaml.org,2002:str', value='init'
+                                    ),
+                                    ruamel.yaml.nodes.ScalarNode(
+                                        tag='tag:yaml.org,2002:str', value='--init-option'
+                                    ),
+                                ),
+                            ],
+                        ),
+                    ),
+                ],
+            ),
+        ),
+        (
+            ruamel.yaml.nodes.ScalarNode(tag='tag:yaml.org,2002:str', value='storage'),
+            ruamel.yaml.nodes.MappingNode(
+                tag='tag:yaml.org,2002:map',
+                value=[
+                    (
+                        ruamel.yaml.nodes.ScalarNode(
+                            tag='tag:yaml.org,2002:str', value='extra_borg_options'
+                        ),
+                        ruamel.yaml.nodes.MappingNode(
+                            tag='tag:yaml.org,2002:map',
+                            value=[
+                                (
+                                    ruamel.yaml.nodes.ScalarNode(
+                                        tag='tag:yaml.org,2002:str', value='prune'
+                                    ),
+                                    ruamel.yaml.nodes.ScalarNode(
+                                        tag='tag:yaml.org,2002:str', value='--prune-option'
+                                    ),
+                                ),
+                            ],
+                        ),
+                    ),
+                ],
+            ),
+        ),
+    ]
+
+    result = module.deep_merge_nodes(node_values)
+    assert len(result) == 1
+    (section_key, section_value) = result[0]
+    assert section_key.value == 'storage'
+    options = section_value.value
+    assert len(options) == 2
+    assert options[0][0].value == 'lock_wait'
+    assert options[0][1].value == '5'
+    assert options[1][0].value == 'extra_borg_options'
+    nested_options = options[1][1].value
+    assert len(nested_options) == 2
+    assert nested_options[0][0].value == 'init'
+    assert nested_options[0][1].value == '--init-option'
+    assert nested_options[1][0].value == 'prune'
+    assert nested_options[1][1].value == '--prune-option'