Constants support includes and command-line overrides (#745, #782)

NEWS

@@ -4,11 +4,15 @@
    https://torsion.org/borgmatic/docs/how-to/set-up-backups/#skipping-actions
  * #701: Deprecate the "disabled" value for the "checks" option in favor of the new "skip_actions"
    option.
+ * #745: Constants now apply to included configuration, not just the file doing the includes. As a
+   side effect of this change, constants no longer apply to option names and only substitute into
+   configuration values.
  * #779: Add a "--match-archives" flag to the "check" action for selecting the archives to check,
    overriding the existing "archive_name_format" and "match_archives" options in configuration.
  * #779: Only parse "--override" values as complex data types when they're for options of those
    types.
  * #782: Fix environment variable interpolation within configured repository paths.
+ * #782: Add configuration constant overriding via the existing "--override" flag.
  * #783: Upgrade ruamel.yaml dependency to support version 0.18.x.
  * #784: Drop support for Python 3.7, which has been end-of-lifed.
 

borgmatic/config/constants.py

@@ -0,0 +1,47 @@
+def coerce_scalar(value):
+    '''
+    Given a configuration value, coerce it to an integer or a boolean as appropriate and return the
+    result.
+    '''
+    try:
+        return int(value)
+    except (TypeError, ValueError):
+        pass
+
+    if value == 'true' or value == 'True':
+        return True
+    if value == 'false' or value == 'False':
+        return False
+
+    return value
+
+
+def apply_constants(value, constants):
+    '''
+    Given a configuration value (bool, dict, int, list, or string) and a dict of named constants,
+    replace any configuration string values of the form "{constant}" (or containing it) with the
+    value of the correspondingly named key from the constants. Recurse as necessary into nested
+    configuration to find values to replace.
+
+    For instance, if a configuration value contains "{foo}", replace it with the value of the "foo"
+    key found within the configuration's "constants".
+
+    Return the configuration value and modify the original.
+    '''
+    if not value or not constants:
+        return value
+
+    if isinstance(value, str):
+        for constant_name, constant_value in constants.items():
+            value = value.replace('{' + constant_name + '}', str(constant_value))
+
+        # Support constants within non-string scalars by coercing the value to its appropriate type.
+        value = coerce_scalar(value)
+    elif isinstance(value, list):
+        for index, list_value in enumerate(value):
+            value[index] = apply_constants(list_value, constants)
+    elif isinstance(value, dict):
+        for option_name, option_value in value.items():
+            value[option_name] = apply_constants(option_value, constants)
+
+    return value

borgmatic/config/load.py

@@ -1,6 +1,5 @@
 import functools
 import itertools
-import json
 import logging
 import operator
 import os
@@ -159,8 +158,7 @@ class Include_constructor(ruamel.yaml.SafeConstructor):
 def load_configuration(filename):
     '''
     Load the given configuration file and return its contents as a data structure of nested dicts
-    and lists. Also, replace any "{constant}" strings with the value of the "constant" key in the
+    and lists.
-    "constants" option of the configuration file.
 
     Raise ruamel.yaml.error.YAMLError if something goes wrong parsing the YAML, or RecursionError
     if there are too many recursive includes.
@@ -179,23 +177,7 @@ def load_configuration(filename):
     yaml.Constructor = Include_constructor_with_include_directory
 
     with open(filename) as file:
-        file_contents = file.read()
+        return yaml.load(file.read())
-        config = yaml.load(file_contents)
-
-        try:
-            has_constants = bool(config and 'constants' in config)
-        except TypeError:
-            has_constants = False
-
-        if has_constants:
-            for key, value in config['constants'].items():
-                value = json.dumps(value)
-                file_contents = file_contents.replace(f'{{{key}}}', value.strip('"'))
-
-            config = yaml.load(file_contents)
-            del config['constants']
-
-        return config
 
 
 def filter_omitted_nodes(nodes, values):

borgmatic/config/validate.py

@@ -4,7 +4,7 @@ import jsonschema
 import ruamel.yaml
 
 import borgmatic.config
-from borgmatic.config import environment, load, normalize, override
+from borgmatic.config import constants, environment, load, normalize, override
 
 
 def schema_filename():
@@ -110,6 +110,7 @@ def parse_configuration(config_filename, schema_filename, overrides=None, resolv
         raise Validation_error(config_filename, (str(error),))
 
     override.apply_overrides(config, schema, overrides)
+    constants.apply_constants(config, config.get('constants') if config else {})
 
     if resolve_env:
         environment.resolve_env_variables(config)

tests/integration/config/test_load.py

@@ -15,35 +15,6 @@ def test_load_configuration_parses_contents():
     assert module.load_configuration('config.yaml') == {'key': 'value'}
 
 
-def test_load_configuration_replaces_constants():
-    builtins = flexmock(sys.modules['builtins'])
-    config_file = io.StringIO(
-        '''
-        constants:
-            key: value
-        key: {key}
-        '''
-    )
-    config_file.name = 'config.yaml'
-    builtins.should_receive('open').with_args('config.yaml').and_return(config_file)
-    assert module.load_configuration('config.yaml') == {'key': 'value'}
-
-
-def test_load_configuration_replaces_complex_constants():
-    builtins = flexmock(sys.modules['builtins'])
-    config_file = io.StringIO(
-        '''
-        constants:
-            key:
-                subkey: value
-        key: {key}
-        '''
-    )
-    config_file.name = 'config.yaml'
-    builtins.should_receive('open').with_args('config.yaml').and_return(config_file)
-    assert module.load_configuration('config.yaml') == {'key': {'subkey': 'value'}}
-
-
 def test_load_configuration_with_only_integer_value_does_not_raise():
     builtins = flexmock(sys.modules['builtins'])
     config_file = io.StringIO('33')

tests/unit/config/test_constants.py

@@ -0,0 +1,58 @@
+import pytest
+from flexmock import flexmock
+
+from borgmatic.config import constants as module
+
+
+@pytest.mark.parametrize(
+    'value,expected_value',
+    (
+        ('3', 3),
+        ('0', 0),
+        ('-3', -3),
+        ('1234', 1234),
+        ('true', True),
+        ('True', True),
+        ('false', False),
+        ('False', False),
+        ('thing', 'thing'),
+        ({}, {}),
+        ({'foo': 'bar'}, {'foo': 'bar'}),
+        ([], []),
+        (['foo', 'bar'], ['foo', 'bar']),
+    ),
+)
+def test_coerce_scalar_converts_value(value, expected_value):
+    assert module.coerce_scalar(value) == expected_value
+
+
+def test_apply_constants_with_empty_constants_passes_through_value():
+    assert module.apply_constants(value='thing', constants={}) == 'thing'
+
+
+@pytest.mark.parametrize(
+    'value,expected_value',
+    (
+        (None, None),
+        ('thing', 'thing'),
+        ('{foo}', 'bar'),
+        ('abc{foo}', 'abcbar'),
+        ('{foo}xyz', 'barxyz'),
+        ('{foo}{baz}', 'barquux'),
+        ('{int}', '3'),
+        ('{bool}', 'True'),
+        (['thing', 'other'], ['thing', 'other']),
+        (['thing', '{foo}'], ['thing', 'bar']),
+        (['{foo}', '{baz}'], ['bar', 'quux']),
+        ({'key': 'value'}, {'key': 'value'}),
+        ({'key': '{foo}'}, {'key': 'bar'}),
+        (3, 3),
+        (True, True),
+        (False, False),
+    ),
+)
+def test_apply_constants_makes_string_substitutions(value, expected_value):
+    flexmock(module).should_receive('coerce_scalar').replace_with(lambda value: value)
+    constants = {'foo': 'bar', 'baz': 'quux', 'int': 3, 'bool': True}
+
+    assert module.apply_constants(value, constants) == expected_value