From 788281cfb9be6fc4e5b019c2f2ea83d0f8f628f2 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 19 May 2022 17:15:05 -0700 Subject: [PATCH] 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). --- NEWS | 3 + borgmatic/config/load.py | 42 +++++++- docs/how-to/make-per-application-backups.md | 4 + tests/integration/config/test_load.py | 103 ++++++++++++++++++-- tests/integration/config/test_validate.py | 42 +++++--- 5 files changed, 165 insertions(+), 29 deletions(-) diff --git a/NEWS b/NEWS index bfdbb498e..dc7f60304 100644 --- a/NEWS +++ b/NEWS @@ -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 diff --git a/borgmatic/config/load.py b/borgmatic/config/load.py index 13adb06ac..9eb0da9f7 100644 --- 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() diff --git a/docs/how-to/make-per-application-backups.md b/docs/how-to/make-per-application-backups.md index dde3476c8..c4fe8cb6d 100644 --- 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: diff --git a/tests/integration/config/test_load.py b/tests/integration/config/test_load.py index c0d598f92..400c30d5e 100644 --- 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') diff --git a/tests/integration/config/test_validate.py b/tests/integration/config/test_validate.py index fbd2098c1..d28190f9a 100644 --- 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': {