When a configuration include is a relative path, load it from either the current working directory or from the directory containing the file doing the including (#532).

2022-05-19 17:15:05 -07:00 · 2022-05-19 17:15:05 -07:00 · 788281cfb9
commit 788281cfb9
parent cd234b689d
5 changed files with 165 additions and 29 deletions
--- a/3
+++ b/3
@ -1,4 +1,7 @@
 1.6.1.dev0
+ * #532: When a configuration include is a relative path, load it from either the current working
+   directory or from the directory containing the file doing the including. (Previously, only the
+   working directory was used.)
 * Add a randomized delay to the sample systemd timer to spread out the load on a server.

 1.6.0
--- a/borgmatic/config/load.py
+++ b/borgmatic/config/load.py
@ -6,6 +6,19 @@ import ruamel.yaml
 logger = logging.getLogger(__name__)


+class Yaml_with_loader_stream(ruamel.yaml.YAML):
+    '''
+    A derived class of ruamel.yaml.YAML that simply tacks the loaded stream (file object) onto the
+    loader class so that it's available anywhere that's passed a loader (in this case,
+    include_configuration() below).
+    '''
+
+    def get_constructor_parser(self, stream):
+        constructor, parser = super(Yaml_with_loader_stream, self).get_constructor_parser(stream)
+        constructor.loader.stream = stream
+        return constructor, parser
+
+
 def load_configuration(filename):
    '''
    Load the given configuration file and return its contents as a data structure of nested dicts
@ -14,7 +27,7 @@ def load_configuration(filename):
    Raise ruamel.yaml.error.YAMLError if something goes wrong parsing the YAML, or RecursionError
    if there are too many recursive includes.
    '''
-    yaml = ruamel.yaml.YAML(typ='safe')
+    yaml = Yaml_with_loader_stream(typ='safe')
    yaml.Constructor = Include_constructor

    return yaml.load(open(filename))
@ -22,10 +35,31 @@ def load_configuration(filename):

 def include_configuration(loader, filename_node):
    '''
-    Load the given YAML filename (ignoring the given loader so we can use our own), and return its
-    contents as a data structure of nested dicts and lists.
+    Load the given YAML filename (ignoring the given loader so we can use our own) and return its
+    contents as a data structure of nested dicts and lists. If the filename is relative, probe for
+    it within 1. the current working directory and 2. the directory containing the YAML file doing
+    the including.
+
+    Raise FileNotFoundError if an included file was not found.
    '''
-    return load_configuration(os.path.expanduser(filename_node.value))
+    include_directories = [os.getcwd(), os.path.abspath(os.path.dirname(loader.stream.name))]
+    include_filename = os.path.expanduser(filename_node.value)
+
+    if not os.path.isabs(include_filename):
+        candidate_filenames = [
+            os.path.join(directory, include_filename) for directory in include_directories
+        ]
+
+        for candidate_filename in candidate_filenames:
+            if os.path.exists(candidate_filename):
+                include_filename = candidate_filename
+                break
+        else:
+            raise FileNotFoundError(
+                f'Could not find include {filename_node.value} at {" or ".join(candidate_filenames)}'
+            )
+
+    return load_configuration(include_filename)


 DELETED_NODE = object()
--- a/docs/how-to/make-per-application-backups.md
+++ b/docs/how-to/make-per-application-backups.md
@ -75,6 +75,10 @@ themselves and complaining that they are not valid configuration files, you
 should put them in a directory other than `/etc/borgmatic.d/`. (A subdirectory
 is fine.)

+When a configuration include is a relative path, borgmatic loads it from either
+the current working directory or from the directory containing the file doing
+the including.
+
 Note that this form of include must be a YAML value rather than a key. For
 example, this will not work:

--- a/tests/integration/config/test_load.py
+++ b/tests/integration/config/test_load.py
@ -1,3 +1,4 @@
+import io
 import sys

 import pytest
@ -14,49 +15,133 @@ def test_load_configuration_parses_contents():
    assert module.load_configuration('config.yaml') == {'key': 'value'}


-def test_load_configuration_inlines_include():
+def test_load_configuration_inlines_include_relative_to_current_directory():
    builtins = flexmock(sys.modules['builtins'])
-    builtins.should_receive('open').with_args('include.yaml').and_return('value')
-    builtins.should_receive('open').with_args('config.yaml').and_return(
-        'key: !include include.yaml'
-    )
+    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('value')
+    include_file.name = 'include.yaml'
+    builtins.should_receive('open').with_args('/tmp/include.yaml').and_return(include_file)
+    config_file = io.StringIO('key: !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') == {'key': 'value'}


+def test_load_configuration_inlines_include_relative_to_config_parent_directory():
+    builtins = flexmock(sys.modules['builtins'])
+    flexmock(module.os).should_receive('getcwd').and_return('/tmp')
+    flexmock(module.os.path).should_receive('isabs').with_args('/etc').and_return(True)
+    flexmock(module.os.path).should_receive('isabs').with_args('/etc/config.yaml').and_return(True)
+    flexmock(module.os.path).should_receive('isabs').with_args('include.yaml').and_return(False)
+    flexmock(module.os.path).should_receive('exists').with_args('/tmp/include.yaml').and_return(
+        False
+    )
+    flexmock(module.os.path).should_receive('exists').with_args('/etc/include.yaml').and_return(
+        True
+    )
+    include_file = io.StringIO('value')
+    include_file.name = 'include.yaml'
+    builtins.should_receive('open').with_args('/etc/include.yaml').and_return(include_file)
+    config_file = io.StringIO('key: !include include.yaml')
+    config_file.name = '/etc/config.yaml'
+    builtins.should_receive('open').with_args('/etc/config.yaml').and_return(config_file)
+
+    assert module.load_configuration('/etc/config.yaml') == {'key': 'value'}
+
+
+def test_load_configuration_raises_if_relative_include_does_not_exist():
+    builtins = flexmock(sys.modules['builtins'])
+    flexmock(module.os).should_receive('getcwd').and_return('/tmp')
+    flexmock(module.os.path).should_receive('isabs').with_args('/etc').and_return(True)
+    flexmock(module.os.path).should_receive('isabs').with_args('/etc/config.yaml').and_return(True)
+    flexmock(module.os.path).should_receive('isabs').with_args('include.yaml').and_return(False)
+    flexmock(module.os.path).should_receive('exists').and_return(False)
+    config_file = io.StringIO('key: !include include.yaml')
+    config_file.name = '/etc/config.yaml'
+    builtins.should_receive('open').with_args('/etc/config.yaml').and_return(config_file)
+
+    with pytest.raises(FileNotFoundError):
+        module.load_configuration('/etc/config.yaml')
+
+
+def test_load_configuration_inlines_absolute_include():
+    builtins = flexmock(sys.modules['builtins'])
+    flexmock(module.os).should_receive('getcwd').and_return('/tmp')
+    flexmock(module.os.path).should_receive('isabs').and_return(True)
+    flexmock(module.os.path).should_receive('exists').never()
+    include_file = io.StringIO('value')
+    include_file.name = '/root/include.yaml'
+    builtins.should_receive('open').with_args('/root/include.yaml').and_return(include_file)
+    config_file = io.StringIO('key: !include /root/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') == {'key': 'value'}
+
+
+def test_load_configuration_raises_if_absolute_include_does_not_exist():
+    builtins = flexmock(sys.modules['builtins'])
+    flexmock(module.os).should_receive('getcwd').and_return('/tmp')
+    flexmock(module.os.path).should_receive('isabs').and_return(True)
+    builtins.should_receive('open').with_args('/root/include.yaml').and_raise(FileNotFoundError)
+    config_file = io.StringIO('key: !include /root/include.yaml')
+    config_file.name = 'config.yaml'
+    builtins.should_receive('open').with_args('config.yaml').and_return(config_file)
+
+    with pytest.raises(FileNotFoundError):
+        assert module.load_configuration('config.yaml')
+
+
 def test_load_configuration_merges_include():
    builtins = flexmock(sys.modules['builtins'])
-    builtins.should_receive('open').with_args('include.yaml').and_return(
+    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(
        '''
        foo: bar
        baz: quux
        '''
    )
-    builtins.should_receive('open').with_args('config.yaml').and_return(
+    include_file.name = 'include.yaml'
+    builtins.should_receive('open').with_args('/tmp/include.yaml').and_return(include_file)
+    config_file = io.StringIO(
        '''
        foo: override
        <<: !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') == {'foo': 'override', 'baz': 'quux'}


 def test_load_configuration_does_not_merge_include_list():
    builtins = flexmock(sys.modules['builtins'])
-    builtins.should_receive('open').with_args('include.yaml').and_return(
+    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(
        '''
          - one
          - two
        '''
    )
-    builtins.should_receive('open').with_args('config.yaml').and_return(
+    include_file.name = 'include.yaml'
+    builtins.should_receive('open').with_args('/tmp/include.yaml').and_return(include_file)
+    config_file = io.StringIO(
        '''
        foo: bar
        repositories:
          <<: !include include.yaml
        '''
    )
+    config_file.name = 'config.yaml'
+    builtins.should_receive('open').with_args('config.yaml').and_return(config_file)

    with pytest.raises(ruamel.yaml.error.YAMLError):
        assert module.load_configuration('config.yaml')
--- a/tests/integration/config/test_validate.py
+++ b/tests/integration/config/test_validate.py
@ -21,14 +21,20 @@ def mock_config_and_schema(config_yaml, schema_yaml=None):
    when parsing the configuration.
    '''
    config_stream = io.StringIO(config_yaml)
+    config_stream.name = 'config.yaml'
+
    if schema_yaml is None:
        schema_stream = open(module.schema_filename())
    else:
        schema_stream = io.StringIO(schema_yaml)
+        schema_stream.name = 'schema.yaml'

    builtins = flexmock(sys.modules['builtins'])
-    builtins.should_receive('open').with_args('config.yaml').and_return(config_stream)
-    builtins.should_receive('open').with_args('schema.yaml').and_return(schema_stream)
+    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)
+    builtins.should_receive('open').with_args('/tmp/config.yaml').and_return(config_stream)
+    builtins.should_receive('open').with_args('/tmp/schema.yaml').and_return(schema_stream)


 def test_parse_configuration_transforms_file_into_mapping():
@ -54,7 +60,7 @@ def test_parse_configuration_transforms_file_into_mapping():
        '''
    )

-    result = module.parse_configuration('config.yaml', 'schema.yaml')
+    result = module.parse_configuration('/tmp/config.yaml', '/tmp/schema.yaml')

    assert result == {
        'location': {'source_directories': ['/home', '/etc'], 'repositories': ['hostname.borg']},
@ -79,7 +85,7 @@ def test_parse_configuration_passes_through_quoted_punctuation():
        )
    )

-    result = module.parse_configuration('config.yaml', 'schema.yaml')
+    result = module.parse_configuration('/tmp/config.yaml', '/tmp/schema.yaml')

    assert result == {
        'location': {
@ -115,7 +121,7 @@ def test_parse_configuration_with_schema_lacking_examples_does_not_raise():
        ''',
    )

-    module.parse_configuration('config.yaml', 'schema.yaml')
+    module.parse_configuration('/tmp/config.yaml', '/tmp/schema.yaml')


 def test_parse_configuration_inlines_include():
@ -133,14 +139,16 @@ def test_parse_configuration_inlines_include():
        '''
    )
    builtins = flexmock(sys.modules['builtins'])
-    builtins.should_receive('open').with_args('include.yaml').and_return(
+    include_file = io.StringIO(
        '''
        keep_daily: 7
        keep_hourly: 24
        '''
    )
+    include_file.name = 'include.yaml'
+    builtins.should_receive('open').with_args('/tmp/include.yaml').and_return(include_file)

-    result = module.parse_configuration('config.yaml', 'schema.yaml')
+    result = module.parse_configuration('/tmp/config.yaml', '/tmp/schema.yaml')

    assert result == {
        'location': {'source_directories': ['/home'], 'repositories': ['hostname.borg']},
@ -164,14 +172,16 @@ def test_parse_configuration_merges_include():
        '''
    )
    builtins = flexmock(sys.modules['builtins'])
-    builtins.should_receive('open').with_args('include.yaml').and_return(
+    include_file = io.StringIO(
        '''
        keep_daily: 7
        keep_hourly: 24
        '''
    )
+    include_file.name = 'include.yaml'
+    builtins.should_receive('open').with_args('/tmp/include.yaml').and_return(include_file)

-    result = module.parse_configuration('config.yaml', 'schema.yaml')
+    result = module.parse_configuration('/tmp/config.yaml', '/tmp/schema.yaml')

    assert result == {
        'location': {'source_directories': ['/home'], 'repositories': ['hostname.borg']},
@ -181,23 +191,23 @@ def test_parse_configuration_merges_include():

 def test_parse_configuration_raises_for_missing_config_file():
    with pytest.raises(FileNotFoundError):
-        module.parse_configuration('config.yaml', 'schema.yaml')
+        module.parse_configuration('/tmp/config.yaml', '/tmp/schema.yaml')


 def test_parse_configuration_raises_for_missing_schema_file():
    mock_config_and_schema('')
    builtins = flexmock(sys.modules['builtins'])
-    builtins.should_receive('open').with_args('schema.yaml').and_raise(FileNotFoundError)
+    builtins.should_receive('open').with_args('/tmp/schema.yaml').and_raise(FileNotFoundError)

    with pytest.raises(FileNotFoundError):
-        module.parse_configuration('config.yaml', 'schema.yaml')
+        module.parse_configuration('/tmp/config.yaml', '/tmp/schema.yaml')


 def test_parse_configuration_raises_for_syntax_error():
    mock_config_and_schema('foo:\nbar')

    with pytest.raises(ValueError):
-        module.parse_configuration('config.yaml', 'schema.yaml')
+        module.parse_configuration('/tmp/config.yaml', '/tmp/schema.yaml')


 def test_parse_configuration_raises_for_validation_error():
@ -211,7 +221,7 @@ def test_parse_configuration_raises_for_validation_error():
    )

    with pytest.raises(module.Validation_error):
-        module.parse_configuration('config.yaml', 'schema.yaml')
+        module.parse_configuration('/tmp/config.yaml', '/tmp/schema.yaml')


 def test_parse_configuration_applies_overrides():
@ -229,7 +239,7 @@ def test_parse_configuration_applies_overrides():
    )

    result = module.parse_configuration(
-        'config.yaml', 'schema.yaml', overrides=['location.local_path=borg2']
+        '/tmp/config.yaml', '/tmp/schema.yaml', overrides=['location.local_path=borg2']
    )

    assert result == {
@ -255,7 +265,7 @@ def test_parse_configuration_applies_normalization():
        '''
    )

-    result = module.parse_configuration('config.yaml', 'schema.yaml')
+    result = module.parse_configuration('/tmp/config.yaml', '/tmp/schema.yaml')

    assert result == {
        'location': {