Only parse "--override" values as complex data types when they're for options of those types (#779).

NEWS

@@ -1,6 +1,8 @@
 1.8.5.dev0
  * #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.
 
 1.8.4
  * #715: Add a monitoring hook for sending backup status to a variety of monitoring services via the

borgmatic/config/override.py

@@ -22,13 +22,19 @@ def set_values(config, keys, value):
     set_values(config[first_key], keys[1:], value)
 
 
-def convert_value_type(value):
+def convert_value_type(value, option_type):
     '''
-    Given a string value, determine its logical type (string, boolean, integer, etc.), and return it
-    converted to that type.
+    Given a string value and its schema type as a string, determine its logical type (string,
+    boolean, integer, etc.), and return it converted to that type.
+
+    If the option type is a string, leave the value as a string so that special characters in it
+    don't get interpreted as YAML during conversion.
 
     Raise ruamel.yaml.error.YAMLError if there's a parse issue with the YAML.
     '''
+    if option_type == 'string':
+        return value
+
     return ruamel.yaml.YAML(typ='safe').load(io.StringIO(value))
 
 
@@ -46,11 +52,32 @@ def strip_section_names(parsed_override_key):
     return parsed_override_key
 
 
-def parse_overrides(raw_overrides):
+def type_for_option(schema, option_keys):
     '''
-    Given a sequence of configuration file override strings in the form of "option.suboption=value",
-    parse and return a sequence of tuples (keys, values), where keys is a sequence of strings. For
-    instance, given the following raw overrides:
+    Given a configuration schema and a sequence of keys identifying an option, e.g.
+    ('extra_borg_options', 'init'), return the schema type of that option as a string.
+
+    Return None if the option or its type cannot be found in the schema.
+    '''
+    option_schema = schema
+
+    for key in option_keys:
+        try:
+            option_schema = option_schema['properties'][key]
+        except KeyError:
+            return None
+
+    try:
+        return option_schema['type']
+    except KeyError:
+        return None
+
+
+def parse_overrides(raw_overrides, schema):
+    '''
+    Given a sequence of configuration file override strings in the form of "option.suboption=value"
+    and a configuration schema dict, parse and return a sequence of tuples (keys, values), where
+    keys is a sequence of strings. For instance, given the following raw overrides:
 
         ['my_option.suboption=value1', 'other_option=value2']
 
@@ -71,10 +98,13 @@ def parse_overrides(raw_overrides):
     for raw_override in raw_overrides:
         try:
             raw_keys, value = raw_override.split('=', 1)
+            keys = strip_section_names(tuple(raw_keys.split('.')))
+            option_type = type_for_option(schema, keys)
+
             parsed_overrides.append(
                 (
-                    strip_section_names(tuple(raw_keys.split('.'))),
-                    convert_value_type(value),
+                    keys,
+                    convert_value_type(value, option_type),
                 )
             )
         except ValueError:
@@ -87,12 +117,13 @@ def parse_overrides(raw_overrides):
     return tuple(parsed_overrides)
 
 
-def apply_overrides(config, raw_overrides):
+def apply_overrides(config, schema, raw_overrides):
     '''
-    Given a configuration dict and a sequence of configuration file override strings in the form of
-    "option.suboption=value", parse each override and set it the configuration dict.
+    Given a configuration dict, a corresponding configuration schema dict, and a sequence of
+    configuration file override strings in the form of "option.suboption=value", parse each override
+    and set it into the configuration dict.
     '''
-    overrides = parse_overrides(raw_overrides)
+    overrides = parse_overrides(raw_overrides, schema)
 
     for keys, value in overrides:
         set_values(config, keys, value)

borgmatic/config/validate.py

@@ -109,7 +109,7 @@ def parse_configuration(config_filename, schema_filename, overrides=None, resolv
     except (ruamel.yaml.error.YAMLError, RecursionError) as error:
         raise Validation_error(config_filename, (str(error),))
 
-    override.apply_overrides(config, overrides)
+    override.apply_overrides(config, schema, overrides)
     logs = normalize.normalize(config_filename, config)
     if resolve_env:
         environment.resolve_env_variables(config)

tests/integration/config/test_override.py

@@ -4,19 +4,24 @@ from borgmatic.config import override as module
 
 
 @pytest.mark.parametrize(
-    'value,expected_result',
+    'value,expected_result,option_type',
     (
-        ('thing', 'thing'),
-        ('33', 33),
-        ('33b', '33b'),
-        ('true', True),
-        ('false', False),
-        ('[foo]', ['foo']),
-        ('[foo, bar]', ['foo', 'bar']),
+        ('thing', 'thing', 'string'),
+        ('33', 33, 'integer'),
+        ('33', '33', 'string'),
+        ('33b', '33b', 'integer'),
+        ('33b', '33b', 'string'),
+        ('true', True, 'boolean'),
+        ('false', False, 'boolean'),
+        ('true', 'true', 'string'),
+        ('[foo]', ['foo'], 'array'),
+        ('[foo]', '[foo]', 'string'),
+        ('[foo, bar]', ['foo', 'bar'], 'array'),
+        ('[foo, bar]', '[foo, bar]', 'string'),
     ),
 )
-def test_convert_value_type_coerces_values(value, expected_result):
-    assert module.convert_value_type(value) == expected_result
+def test_convert_value_type_coerces_values(value, expected_result, option_type):
+    assert module.convert_value_type(value, option_type) == expected_result
 
 
 def test_apply_overrides_updates_config():
@@ -25,16 +30,23 @@ def test_apply_overrides_updates_config():
         'other_section.thing=value2',
         'section.nested.key=value3',
         'new.foo=bar',
+        'new.mylist=[baz]',
+        'new.nonlist=[quux]',
     ]
     config = {
         'section': {'key': 'value', 'other': 'other_value'},
         'other_section': {'thing': 'thing_value'},
     }
+    schema = {
+        'properties': {
+            'new': {'properties': {'mylist': {'type': 'array'}, 'nonlist': {'type': 'string'}}}
+        }
+    }
 
-    module.apply_overrides(config, raw_overrides)
+    module.apply_overrides(config, schema, raw_overrides)
 
     assert config == {
         'section': {'key': 'value1', 'other': 'other_value', 'nested': {'key': 'value3'}},
         'other_section': {'thing': 'value2'},
-        'new': {'foo': 'bar'},
+        'new': {'foo': 'bar', 'mylist': ['baz'], 'nonlist': '[quux]'},
     }

tests/unit/config/test_override.py

@@ -44,6 +44,24 @@ def test_set_values_with_multiple_keys_updates_hierarchy():
     assert config == {'option': {'key': 'value', 'other': 'other_value'}}
 
 
+@pytest.mark.parametrize(
+    'schema,option_keys,expected_type',
+    (
+        ({'properties': {'foo': {'type': 'array'}}}, ('foo',), 'array'),
+        (
+            {'properties': {'foo': {'properties': {'bar': {'type': 'array'}}}}},
+            ('foo', 'bar'),
+            'array',
+        ),
+        ({'properties': {'foo': {'type': 'array'}}}, ('other',), None),
+        ({'properties': {'foo': {'description': 'stuff'}}}, ('foo',), None),
+        ({}, ('foo',), None),
+    ),
+)
+def test_type_for_option_grabs_type_if_found_in_schema(schema, option_keys, expected_type):
+    assert module.type_for_option(schema, option_keys) == expected_type
+
+
 @pytest.mark.parametrize(
     'key,expected_key',
     (
@@ -63,51 +81,64 @@ def test_strip_section_names_passes_through_key_without_section_name(key, expect
 
 def test_parse_overrides_splits_keys_and_values():
     flexmock(module).should_receive('strip_section_names').replace_with(lambda value: value)
-    flexmock(module).should_receive('convert_value_type').replace_with(lambda value: value)
+    flexmock(module).should_receive('type_for_option').and_return('string')
+    flexmock(module).should_receive('convert_value_type').replace_with(
+        lambda value, option_type: value
+    )
     raw_overrides = ['option.my_option=value1', 'other_option=value2']
     expected_result = (
         (('option', 'my_option'), 'value1'),
         (('other_option'), 'value2'),
     )
 
-    module.parse_overrides(raw_overrides) == expected_result
+    module.parse_overrides(raw_overrides, schema={}) == expected_result
 
 
 def test_parse_overrides_allows_value_with_equal_sign():
     flexmock(module).should_receive('strip_section_names').replace_with(lambda value: value)
-    flexmock(module).should_receive('convert_value_type').replace_with(lambda value: value)
+    flexmock(module).should_receive('type_for_option').and_return('string')
+    flexmock(module).should_receive('convert_value_type').replace_with(
+        lambda value, option_type: value
+    )
     raw_overrides = ['option=this===value']
     expected_result = ((('option',), 'this===value'),)
 
-    module.parse_overrides(raw_overrides) == expected_result
+    module.parse_overrides(raw_overrides, schema={}) == expected_result
 
 
 def test_parse_overrides_raises_on_missing_equal_sign():
     flexmock(module).should_receive('strip_section_names').replace_with(lambda value: value)
-    flexmock(module).should_receive('convert_value_type').replace_with(lambda value: value)
+    flexmock(module).should_receive('type_for_option').and_return('string')
+    flexmock(module).should_receive('convert_value_type').replace_with(
+        lambda value, option_type: value
+    )
     raw_overrides = ['option']
 
     with pytest.raises(ValueError):
-        module.parse_overrides(raw_overrides)
+        module.parse_overrides(raw_overrides, schema={})
 
 
 def test_parse_overrides_raises_on_invalid_override_value():
     flexmock(module).should_receive('strip_section_names').replace_with(lambda value: value)
+    flexmock(module).should_receive('type_for_option').and_return('string')
     flexmock(module).should_receive('convert_value_type').and_raise(ruamel.yaml.parser.ParserError)
     raw_overrides = ['option=[in valid]']
 
     with pytest.raises(ValueError):
-        module.parse_overrides(raw_overrides)
+        module.parse_overrides(raw_overrides, schema={})
 
 
 def test_parse_overrides_allows_value_with_single_key():
     flexmock(module).should_receive('strip_section_names').replace_with(lambda value: value)
-    flexmock(module).should_receive('convert_value_type').replace_with(lambda value: value)
+    flexmock(module).should_receive('type_for_option').and_return('string')
+    flexmock(module).should_receive('convert_value_type').replace_with(
+        lambda value, option_type: value
+    )
     raw_overrides = ['option=value']
     expected_result = ((('option',), 'value'),)
 
-    module.parse_overrides(raw_overrides) == expected_result
+    module.parse_overrides(raw_overrides, schema={}) == expected_result
 
 
 def test_parse_overrides_handles_empty_overrides():
-    module.parse_overrides(raw_overrides=None) == ()
+    module.parse_overrides(raw_overrides=None, schema={}) == ()