diff --git a/NEWS b/NEWS index 72ef09b71..d8a3e8019 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,8 @@ +1.4.7 + * #239: Upgrade your borgmatic configuration to get new options and comments via + "generate-borgmatic-config --source". See the documentation for more information: + https://torsion.org/borgmatic/docs/how-to/upgrade/#upgrading-your-configuration + 1.4.6 * Verbosity level "-1" for even quieter output: Errors only (#236). diff --git a/borgmatic/commands/generate_config.py b/borgmatic/commands/generate_config.py index f338f3033..8ea3d5c13 100644 --- a/borgmatic/commands/generate_config.py +++ b/borgmatic/commands/generate_config.py @@ -12,12 +12,18 @@ def parse_arguments(*arguments): them as an ArgumentParser instance. ''' parser = ArgumentParser(description='Generate a sample borgmatic YAML configuration file.') + parser.add_argument( + '-s', + '--source', + dest='source_filename', + help='Optional YAML configuration file to merge into the generated configuration, useful for upgrading your configuration', + ) parser.add_argument( '-d', '--destination', dest='destination_filename', default=DEFAULT_DESTINATION_CONFIG_FILENAME, - help='Destination YAML configuration filename. Default: {}'.format( + help='Destination YAML configuration file. Default: {}'.format( DEFAULT_DESTINATION_CONFIG_FILENAME ), ) @@ -30,11 +36,21 @@ def main(): # pragma: no cover args = parse_arguments(*sys.argv[1:]) generate.generate_sample_configuration( - args.destination_filename, validate.schema_filename() + args.source_filename, args.destination_filename, validate.schema_filename() ) print('Generated a sample configuration file at {}.'.format(args.destination_filename)) print() + if args.source_filename: + print( + 'Merged in the contents of configuration file at {}.'.format(args.source_filename) + ) + print('To review the changes made, run:') + print() + print( + ' diff --unified {} {}'.format(args.source_filename, args.destination_filename) + ) + print() print('Please edit the file to suit your needs. The values are representative.') print('All fields are optional except where indicated.') print() diff --git a/borgmatic/config/generate.py b/borgmatic/config/generate.py index c0399ed2c..05bbd367c 100644 --- a/borgmatic/config/generate.py +++ b/borgmatic/config/generate.py @@ -1,9 +1,12 @@ +import collections import io import os import re from ruamel import yaml +from borgmatic.config import load + INDENT = 4 SEQUENCE_INDENT = 2 @@ -40,8 +43,8 @@ def _schema_to_sample_configuration(schema, level=0, parent_is_sequence=False): elif 'map' in schema: config = yaml.comments.CommentedMap( [ - (section_name, _schema_to_sample_configuration(section_schema, level + 1)) - for section_name, section_schema in schema['map'].items() + (field_name, _schema_to_sample_configuration(sub_schema, level + 1)) + for field_name, sub_schema in schema['map'].items() ] ) indent = (level * INDENT) + (SEQUENCE_INDENT if parent_is_sequence else 0) @@ -68,37 +71,32 @@ def _comment_out_line(line): return '# '.join((indent_spaces, line[count_indent_spaces:])) -REQUIRED_KEYS = {'source_directories', 'repositories', 'keep_daily'} -REQUIRED_SECTION_NAMES = {'location', 'retention'} - - def _comment_out_optional_configuration(rendered_config): ''' - Post-process a rendered configuration string to comment out optional key/values. The idea is - that this prevents the user from having to comment out a bunch of configuration they don't care - about to get to a minimal viable configuration file. + Post-process a rendered configuration string to comment out optional key/values, as determined + by a sentinel in the comment before each key. - Ideally ruamel.yaml would support this during configuration generation, but it's not terribly - easy to accomplish that way. + The idea is that the pre-commented configuration prevents the user from having to comment out a + bunch of configuration they don't care about to get to a minimal viable configuration file. + + Ideally ruamel.yaml would support commenting out keys during configuration generation, but it's + not terribly easy to accomplish that way. ''' lines = [] - required = False + optional = False for line in rendered_config.split('\n'): - key = line.strip().split(':')[0] - - if key in REQUIRED_SECTION_NAMES: - lines.append(line) + # Upon encountering an optional configuration option, commenting out lines until the next + # blank line. + if line.strip().startswith('# {}'.format(COMMENTED_OUT_SENTINEL)): + optional = True continue - # Upon encountering a required configuration option, skip commenting out lines until the - # next blank line. - if key in REQUIRED_KEYS: - required = True - elif not key: - required = False + # Hit a blank line, so reset commenting. + if not line.strip(): + optional = False - lines.append(_comment_out_line(line) if not required else line) + lines.append(_comment_out_line(line) if optional else line) return '\n'.join(lines) @@ -168,6 +166,11 @@ def add_comments_to_configuration_sequence(config, schema, indent=0): return +REQUIRED_SECTION_NAMES = {'location', 'retention'} +REQUIRED_KEYS = {'source_directories', 'repositories', 'keep_daily'} +COMMENTED_OUT_SENTINEL = 'COMMENT_OUT' + + def add_comments_to_configuration_map(config, schema, indent=0, skip_first=False): ''' Using descriptions from a schema as a source, add those descriptions as comments to the given @@ -178,10 +181,20 @@ def add_comments_to_configuration_map(config, schema, indent=0, skip_first=False continue field_schema = schema['map'].get(field_name, {}) - description = field_schema.get('desc') + description = field_schema.get('desc', '').strip() + + # If this is an optional key, add an indicator to the comment flagging it to be commented + # out from the sample configuration. This sentinel is consumed by downstream processing that + # does the actual commenting out. + if field_name not in REQUIRED_SECTION_NAMES and field_name not in REQUIRED_KEYS: + description = ( + '\n'.join((description, COMMENTED_OUT_SENTINEL)) + if description + else COMMENTED_OUT_SENTINEL + ) # No description to use? Skip it. - if not field_schema or not description: + if not field_schema or not description: # pragma: no cover continue config.yaml_set_comment_before_after_key(key=field_name, before=description, indent=indent) @@ -190,14 +203,86 @@ def add_comments_to_configuration_map(config, schema, indent=0, skip_first=False _insert_newline_before_comment(config, field_name) -def generate_sample_configuration(config_filename, schema_filename): +RUAMEL_YAML_COMMENTS_INDEX = 1 + + +def remove_commented_out_sentinel(config, field_name): ''' - Given a target config filename and the path to a schema filename in pykwalify YAML schema - format, write out a sample configuration file based on that schema. + Given a configuration CommentedMap and a top-level field name in it, remove any "commented out" + sentinel found at the end of its YAML comments. This prevents the given field name from getting + commented out by downstream processing that consumes the sentinel. + ''' + try: + last_comment_value = config.ca.items[field_name][RUAMEL_YAML_COMMENTS_INDEX][-1].value + except KeyError: + return + + if last_comment_value == '# {}\n'.format(COMMENTED_OUT_SENTINEL): + config.ca.items[field_name][RUAMEL_YAML_COMMENTS_INDEX].pop() + + +def merge_source_configuration_into_destination(destination_config, source_config): + ''' + Deep merge the given source configuration dict into the destination configuration CommentedMap, + favoring values from the source when there are collisions. + + The purpose of this is to upgrade configuration files from old versions of borgmatic by adding + new + configuration keys and comments. + ''' + if not destination_config or not isinstance(source_config, collections.abc.Mapping): + return source_config + + for field_name, source_value in source_config.items(): + # Since this key/value is from the source configuration, leave it uncommented and remove any + # sentinel that would cause it to get commented out. + remove_commented_out_sentinel(destination_config, field_name) + + # This is a mapping. Recurse for this key/value. + if isinstance(source_value, collections.abc.Mapping): + destination_config[field_name] = merge_source_configuration_into_destination( + destination_config[field_name], source_value + ) + continue + + # This is a sequence. Recurse for each item in it. + if isinstance(source_value, collections.abc.Sequence) and not isinstance(source_value, str): + destination_value = destination_config[field_name] + destination_config[field_name] = yaml.comments.CommentedSeq( + [ + merge_source_configuration_into_destination( + destination_value[index] if index < len(destination_value) else None, + source_item, + ) + for index, source_item in enumerate(source_value) + ] + ) + continue + + # This is some sort of scalar. Simply set it into the destination. + destination_config[field_name] = source_config[field_name] + + return destination_config + + +def generate_sample_configuration(source_filename, destination_filename, schema_filename): + ''' + Given an optional source configuration filename, and a required destination configuration + filename, and the path to a schema filename in pykwalify YAML schema format, write out a + sample configuration file based on that schema. If a source filename is provided, merge the + parsed contents of that configuration into the generated configuration. ''' schema = yaml.round_trip_load(open(schema_filename)) - config = _schema_to_sample_configuration(schema) + source_config = None + + if source_filename: + source_config = load.load_configuration(source_filename) + + destination_config = merge_source_configuration_into_destination( + _schema_to_sample_configuration(schema), source_config + ) write_configuration( - config_filename, _comment_out_optional_configuration(_render_configuration(config)) + destination_filename, + _comment_out_optional_configuration(_render_configuration(destination_config)), ) diff --git a/docs/how-to/set-up-backups.md b/docs/how-to/set-up-backups.md index f436041c2..bea044786 100644 --- a/docs/how-to/set-up-backups.md +++ b/docs/how-to/set-up-backups.md @@ -77,9 +77,12 @@ else borgmatic won't recognize the option. Also be sure to use spaces rather than tabs for indentation; YAML does not allow tabs. You can also get the same sample configuration file from the [configuration -reference](https://torsion.org/borgmatic/docs/reference/configuration/), the authoritative set of -all configuration options. This is handy if borgmatic has added new options -since you originally created your configuration file. +reference](https://torsion.org/borgmatic/docs/reference/configuration/), the +authoritative set of all configuration options. This is handy if borgmatic has +added new options +since you originally created your configuration file. Also check out how to +[upgrade your +configuration](https://torsion.org/borgmatic/docs/how-to/upgrade/#upgrading-your-configuration). ### Encryption @@ -248,5 +251,6 @@ it. * [Deal with very large backups](https://torsion.org/borgmatic/docs/how-to/deal-with-very-large-backups/) * [Inspect your backups](https://torsion.org/borgmatic/docs/how-to/inspect-your-backups/) * [Monitor your backups](https://torsion.org/borgmatic/docs/how-to/monitor-your-backups/) + * [Upgrade borgmatic](https://torsion.org/borgmatic/docs/how-to/upgrade/) * [borgmatic configuration reference](https://torsion.org/borgmatic/docs/reference/configuration/) * [borgmatic command-line reference](https://torsion.org/borgmatic/docs/reference/command-line/) diff --git a/docs/how-to/upgrade.md b/docs/how-to/upgrade.md index c66ebd47a..18455e8fb 100644 --- a/docs/how-to/upgrade.md +++ b/docs/how-to/upgrade.md @@ -10,7 +10,46 @@ following: sudo pip3 install --user --upgrade borgmatic ``` -See below about special cases. +See below about special cases with old versions of borgmatic. Additionally, if +you installed borgmatic [without using `pip3 install +--user`](https://torsion.org/borgmatic/docs/how-to/set-up-backups/#other-ways-to-install), +then your upgrade process may be different. + + +### Upgrading your configuration + +The borgmatic configuration file format is almost always backwards-compatible +from release to release without any changes, but you may still want to update +your configuration file when you upgrade to take advantage of new +configuration options. This is completely optional. If you prefer, you can add +new configuration options manually. + +If you do want to upgrade your configuration file to include new options, use +the `generate-borgmatic-config` script with its optional `--source` flag that +takes the path to your original configuration file. If provided with this +path, `generate-borgmatic-config` merges your original configuration into the +generated configuration file, so you get all the newest options and comments. + +Here's an example: + +```bash +generate-borgmatic-config --source config.yaml --destination config-new.yaml +``` + +New options start as commented out, so you can edit the file and decide +whether you want to use each one. + +There are a few caveats to this process, however. First, when generating the +new configuration file, `generate-borgmatic-config` replaces any comments +you've written in your original configuration file with the newest generated +comments. Second, the script adds back any options you had originally deleted, +although it does so with the options commented out. And finally, any YAML +includes you've used in the source configuration get flattened out into a +single generated file. + +As a safety measure, `generate-borgmatic-config` refuses to modify +configuration files in-place. So it's up to you to review the generated file +and, if desired, replace your original configuration file with it. ### Upgrading from borgmatic 1.0.x diff --git a/setup.py b/setup.py index db3144bfb..53f894fbe 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,6 @@ from setuptools import find_packages, setup -VERSION = '1.4.6' +VERSION = '1.4.7' setup( diff --git a/tests/integration/config/test_generate.py b/tests/integration/config/test_generate.py index 7065a6022..0ff610f5c 100644 --- a/tests/integration/config/test_generate.py +++ b/tests/integration/config/test_generate.py @@ -47,9 +47,13 @@ def test_comment_out_line_comments_twice_indented_option(): def test_comment_out_optional_configuration_comments_optional_config_only(): + # The "# COMMENT_OUT" comment is a sentinel used to express that the following key is optional. + # It's stripped out of the final output. flexmock(module)._comment_out_line = lambda line: '# ' + line config = ''' +# COMMENT_OUT foo: + # COMMENT_OUT bar: - baz - quux @@ -59,6 +63,8 @@ location: - one - two + # This comment should be kept. + # COMMENT_OUT other: thing ''' @@ -68,12 +74,13 @@ location: # bar: # - baz # - quux -# + location: repositories: - one - two -# + + # This comment should be kept. # other: thing ''' @@ -142,12 +149,67 @@ def test_add_comments_to_configuration_map_does_not_raise(): module.add_comments_to_configuration_map(config, schema) +def test_add_comments_to_configuration_map_with_skip_first_does_not_raise(): + config = module.yaml.comments.CommentedMap([('foo', 33)]) + schema = {'map': {'foo': {'desc': 'Foo'}}} + + module.add_comments_to_configuration_map(config, schema, skip_first=True) + + +def test_remove_commented_out_sentinel_keeps_other_comments(): + field_name = 'foo' + config = module.yaml.comments.CommentedMap([(field_name, 33)]) + config.yaml_set_comment_before_after_key(key=field_name, before='Actual comment.\nCOMMENT_OUT') + + module.remove_commented_out_sentinel(config, field_name) + + comments = config.ca.items[field_name][module.RUAMEL_YAML_COMMENTS_INDEX] + assert len(comments) == 1 + assert comments[0].value == '# Actual comment.\n' + + +def test_remove_commented_out_sentinel_without_sentinel_keeps_other_comments(): + field_name = 'foo' + config = module.yaml.comments.CommentedMap([(field_name, 33)]) + config.yaml_set_comment_before_after_key(key=field_name, before='Actual comment.') + + module.remove_commented_out_sentinel(config, field_name) + + comments = config.ca.items[field_name][module.RUAMEL_YAML_COMMENTS_INDEX] + assert len(comments) == 1 + assert comments[0].value == '# Actual comment.\n' + + +def test_remove_commented_out_sentinel_on_unknown_field_does_not_raise(): + field_name = 'foo' + config = module.yaml.comments.CommentedMap([(field_name, 33)]) + config.yaml_set_comment_before_after_key(key=field_name, before='Actual comment.') + + module.remove_commented_out_sentinel(config, 'unknown') + + def test_generate_sample_configuration_does_not_raise(): builtins = flexmock(sys.modules['builtins']) builtins.should_receive('open').with_args('schema.yaml').and_return('') + flexmock(module.yaml).should_receive('round_trip_load') flexmock(module).should_receive('_schema_to_sample_configuration') + flexmock(module).should_receive('merge_source_configuration_into_destination') flexmock(module).should_receive('_render_configuration') flexmock(module).should_receive('_comment_out_optional_configuration') flexmock(module).should_receive('write_configuration') - module.generate_sample_configuration('config.yaml', 'schema.yaml') + module.generate_sample_configuration(None, 'dest.yaml', 'schema.yaml') + + +def test_generate_sample_configuration_with_source_filename_does_not_raise(): + builtins = flexmock(sys.modules['builtins']) + builtins.should_receive('open').with_args('schema.yaml').and_return('') + flexmock(module.yaml).should_receive('round_trip_load') + flexmock(module.load).should_receive('load_configuration') + flexmock(module).should_receive('_schema_to_sample_configuration') + flexmock(module).should_receive('merge_source_configuration_into_destination') + flexmock(module).should_receive('_render_configuration') + flexmock(module).should_receive('_comment_out_optional_configuration') + flexmock(module).should_receive('write_configuration') + + module.generate_sample_configuration('source.yaml', 'dest.yaml', 'schema.yaml') diff --git a/tests/unit/config/test_convert.py b/tests/unit/config/test_convert.py index cdf1d444c..8ae5dc678 100644 --- a/tests/unit/config/test_convert.py +++ b/tests/unit/config/test_convert.py @@ -21,6 +21,7 @@ def test_convert_section_generates_integer_value_for_integer_type_in_schema(): def test_convert_legacy_parsed_config_transforms_source_config_to_mapping(): flexmock(module.yaml.comments).should_receive('CommentedMap').replace_with(OrderedDict) + flexmock(module.generate).should_receive('add_comments_to_configuration_map') source_config = Parsed_config( location=OrderedDict([('source_directories', '/home'), ('repository', 'hostname.borg')]), storage=OrderedDict([('encryption_passphrase', 'supersecret')]), @@ -53,6 +54,7 @@ def test_convert_legacy_parsed_config_transforms_source_config_to_mapping(): def test_convert_legacy_parsed_config_splits_space_separated_values(): flexmock(module.yaml.comments).should_receive('CommentedMap').replace_with(OrderedDict) + flexmock(module.generate).should_receive('add_comments_to_configuration_map') source_config = Parsed_config( location=OrderedDict( [('source_directories', '/home /etc'), ('repository', 'hostname.borg')] diff --git a/tests/unit/config/test_generate.py b/tests/unit/config/test_generate.py index aea5f1004..a0a79ba70 100644 --- a/tests/unit/config/test_generate.py +++ b/tests/unit/config/test_generate.py @@ -51,6 +51,7 @@ def test_schema_to_sample_configuration_generates_config_sequence_of_strings_wit def test_schema_to_sample_configuration_generates_config_sequence_of_maps_with_examples(): flexmock(module.yaml.comments).should_receive('CommentedSeq').replace_with(list) flexmock(module).should_receive('add_comments_to_configuration_sequence') + flexmock(module).should_receive('add_comments_to_configuration_map') schema = { 'seq': [ { @@ -71,3 +72,54 @@ def test_schema_to_sample_configuration_with_unsupported_schema_raises(): with pytest.raises(ValueError): module._schema_to_sample_configuration(schema) + + +def test_merge_source_configuration_into_destination_inserts_map_fields(): + destination_config = {'foo': 'dest1', 'bar': 'dest2'} + source_config = {'foo': 'source1', 'baz': 'source2'} + flexmock(module).should_receive('remove_commented_out_sentinel') + flexmock(module).should_receive('yaml.comments.CommentedSeq').replace_with(list) + + module.merge_source_configuration_into_destination(destination_config, source_config) + + assert destination_config == {'foo': 'source1', 'bar': 'dest2', 'baz': 'source2'} + + +def test_merge_source_configuration_into_destination_inserts_nested_map_fields(): + destination_config = {'foo': {'first': 'dest1', 'second': 'dest2'}, 'bar': 'dest3'} + source_config = {'foo': {'first': 'source1'}} + flexmock(module).should_receive('remove_commented_out_sentinel') + flexmock(module).should_receive('yaml.comments.CommentedSeq').replace_with(list) + + module.merge_source_configuration_into_destination(destination_config, source_config) + + assert destination_config == {'foo': {'first': 'source1', 'second': 'dest2'}, 'bar': 'dest3'} + + +def test_merge_source_configuration_into_destination_inserts_sequence_fields(): + destination_config = {'foo': ['dest1', 'dest2'], 'bar': ['dest3'], 'baz': ['dest4']} + source_config = {'foo': ['source1'], 'bar': ['source2', 'source3']} + flexmock(module).should_receive('remove_commented_out_sentinel') + flexmock(module).should_receive('yaml.comments.CommentedSeq').replace_with(list) + + module.merge_source_configuration_into_destination(destination_config, source_config) + + assert destination_config == { + 'foo': ['source1'], + 'bar': ['source2', 'source3'], + 'baz': ['dest4'], + } + + +def test_merge_source_configuration_into_destination_inserts_sequence_of_maps(): + destination_config = {'foo': [{'first': 'dest1', 'second': 'dest2'}], 'bar': 'dest3'} + source_config = {'foo': [{'first': 'source1'}, {'other': 'source2'}]} + flexmock(module).should_receive('remove_commented_out_sentinel') + flexmock(module).should_receive('yaml.comments.CommentedSeq').replace_with(list) + + module.merge_source_configuration_into_destination(destination_config, source_config) + + assert destination_config == { + 'foo': [{'first': 'source1', 'second': 'dest2'}, {'other': 'source2'}], + 'bar': 'dest3', + }