Fix traceback when upgrading old INI-style configuration with upgrade-borgmatic-config (#367).

Reviewed-on: https://projects.torsion.org/witten/borgmatic/pulls/371

NEWS

@@ -1,3 +1,13 @@
+1.5.11.dev0
+ * #341: Add "temporary_directory" option for changing Borg's temporary directory.
+ * #352: Lock down systemd security settings in sample systemd service file.
+ * #355: Fix traceback when a database hook value is null in a configuration file.
+ * #361: Merge override values when specifying the "--override" flag multiple times. The previous
+   behavior was to take the value of the last "--override" flag only.
+ * #367: Fix traceback when upgrading old INI-style configuration with upgrade-borgmatic-config.
+ * #368: Fix signal forwarding from borgmatic to Borg resulting in recursion traceback.
+ * #369: Document support for Borg placeholders in repository names.
+
 1.5.10
  * #347: Add hooks that run for the "extract" action: "before_extract" and "after_extract".
  * #350: Fix traceback when a configuration directory is non-readable due to directory permissions.

README.md

@@ -22,8 +22,9 @@ location:
 
     # Paths of local or remote repositories to backup to.
     repositories:
-        - k8pDxu32@k8pDxu32.repo.borgbase.com:repo
         - 1234@usw-s001.rsync.net:backups.borg
+        - k8pDxu32@k8pDxu32.repo.borgbase.com:repo
+        - user1@scp2.cdn.lima-labs.com:repo
         - /var/lib/backups/local.borg
 
 retention:
@@ -66,9 +67,9 @@ borgmatic is powered by [Borg Backup](https://www.borgbackup.org/).
 <a href="https://healthchecks.io/"><img src="docs/static/healthchecks.png" alt="Healthchecks" height="60px" style="margin-bottom:20px;"></a>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
 <a href="https://cronitor.io/"><img src="docs/static/cronitor.png" alt="Cronitor" height="60px" style="margin-bottom:20px;"></a>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
 <a href="https://cronhub.io/"><img src="docs/static/cronhub.png" alt="Cronhub" height="60px" style="margin-bottom:20px;"></a>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
-<a href="https://www.pagerduty.com/"><img src="docs/static/pagerduty.png" alt="PagerDuty" height="60px" style="margin-bottom:20px;"></a>
+<a href="https://www.pagerduty.com/"><img src="docs/static/pagerduty.png" alt="PagerDuty" height="60px" style="margin-bottom:20px;"></a>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
+<a href="https://www.rsync.net/cgi-bin/borg.cgi?campaign=borg&adgroup=borgmatic"><img src="docs/static/rsyncnet.png" alt="rsync.net" height="60px" style="margin-bottom:20px;"></a>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
 <a href="https://www.borgbase.com/?utm_source=borgmatic"><img src="docs/static/borgbase.png" alt="BorgBase" height="60px" style="margin-bottom:20px;"></a>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
-<img src="docs/static/rsyncnet.png" alt="rsync.net" height="60px" style="margin-bottom:20px;">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
 
 
 ## Getting started
@@ -89,8 +90,9 @@ services helps support borgmatic development and hosting. (These are referral
 links, but without any tracking scripts or cookies.)
 
 <ul>
+ <li class="referral"><a href="https://www.rsync.net/cgi-bin/borg.cgi?campaign=borg&adgroup=borgmatic">rsync.net</a>: Cloud Storage provider with full support for borg and any other SSH/SFTP tool</li>
  <li class="referral"><a href="https://www.borgbase.com/?utm_source=borgmatic">BorgBase</a>: Borg hosting service with support for monitoring, 2FA, and append-only repos</li>
- <li>rsync.net: Cloud Storage provider with full support for borg and any other SSH/SFTP tool</li>
+ <li class="referral"><a href="https://storage.lima-labs.com/special-pricing-offer-for-borgmatic-users/">Lima-Labs</a>: Affordable, reliable cloud data storage accessable via SSH/SCP/FTP for Borg backups or any other bulk storage needs</li>
 </ul>
 
 ## Support and contributing

borgmatic/borg/environment.py

@@ -9,6 +9,7 @@ OPTION_TO_ENVIRONMENT_VARIABLE = {
     'encryption_passcommand': 'BORG_PASSCOMMAND',
     'encryption_passphrase': 'BORG_PASSPHRASE',
     'ssh_command': 'BORG_RSH',
+    'temporary_directory': 'TMPDIR',
 }
 
 DEFAULT_BOOL_OPTION_TO_ENVIRONMENT_VARIABLE = {

borgmatic/commands/arguments.py

@@ -1,5 +1,5 @@
 import collections
-from argparse import ArgumentParser
+from argparse import Action, ArgumentParser
 
 from borgmatic.config import collect
 
@@ -102,6 +102,20 @@ def parse_global_arguments(unparsed_arguments, top_level_parser, subparsers):
     return top_level_parser.parse_args(remaining_arguments)
 
 
+class Extend_action(Action):
+    '''
+    An argparse action to support Python 3.8's "extend" action in older versions of Python.
+    '''
+
+    def __call__(self, parser, namespace, values, option_string=None):
+        items = getattr(namespace, self.dest, None)
+
+        if items:
+            items.extend(values)
+        else:
+            setattr(namespace, self.dest, list(values))
+
+
 def parse_arguments(*unparsed_arguments):
     '''
     Given command-line arguments with which this script was invoked, parse the arguments and return
@@ -111,6 +125,7 @@ def parse_arguments(*unparsed_arguments):
     unexpanded_config_paths = collect.get_default_config_paths(expand_home=False)
 
     global_parser = ArgumentParser(add_help=False)
+    global_parser.register('action', 'extend', Extend_action)
     global_group = global_parser.add_argument_group('global arguments')
 
     global_group.add_argument(
@@ -178,6 +193,7 @@ def parse_arguments(*unparsed_arguments):
         metavar='SECTION.OPTION=VALUE',
         nargs='+',
         dest='overrides',
+        action='extend',
         help='One or more configuration file options to override with specified values',
     )
     global_group.add_argument(
@@ -581,7 +597,7 @@ def parse_arguments(*unparsed_arguments):
         help='Show info for first N archives after other filters are applied',
     )
     info_group.add_argument(
-        '--last', metavar='N', help='Show info for first N archives after other filters are applied'
+        '--last', metavar='N', help='Show info for last N archives after other filters are applied'
     )
     info_group.add_argument('-h', '--help', action='help', help='Show this help message and exit')
 

borgmatic/commands/borgmatic.py

@@ -658,7 +658,9 @@ def collect_configuration_run_summary_logs(configs, arguments):
 
     if not configs:
         yield from make_error_log_records(
-            '{}: No configuration files found'.format(' '.join(arguments['global'].config_paths))
+            '{}: No valid configuration files found'.format(
+                ' '.join(arguments['global'].config_paths)
+            )
         )
         return
 

borgmatic/commands/convert_config.py

@@ -99,7 +99,9 @@ def main():  # pragma: no cover
         )
 
         generate.write_configuration(
-            args.destination_config_filename, destination_config, mode=source_config_file_mode
+            args.destination_config_filename,
+            generate.render_configuration(destination_config),
+            mode=source_config_file_mode,
         )
 
         display_result(args)

borgmatic/config/generate.py

@@ -99,7 +99,7 @@ def _comment_out_optional_configuration(rendered_config):
     return '\n'.join(lines)
 
 
-def _render_configuration(config):
+def render_configuration(config):
     '''
     Given a config data structure of nested OrderedDicts, render the config as YAML and return it.
     '''
@@ -284,5 +284,5 @@ def generate_sample_configuration(source_filename, destination_filename, schema_
 
     write_configuration(
         destination_filename,
-        _comment_out_optional_configuration(_render_configuration(destination_config)),
+        _comment_out_optional_configuration(render_configuration(destination_config)),
     )

borgmatic/config/schema.yaml

@@ -27,10 +27,14 @@ map:
                 desc: |
                     Paths to local or remote repositories (required). Tildes are
                     expanded. Multiple repositories are backed up to in
-                    sequence. See ssh_command for SSH options like identity file
-                    or port.
+                    sequence. Borg placeholders can be used. See the output of
+                    "borg help placeholders" for details. See ssh_command for
+                    SSH options like identity file or port. If systemd service
+                    is used, then add local repository paths in the systemd
+                    service file to the ReadWritePaths list.
                 example:
                     - user@backupserver:sourcehostname.borg
+                    - "user@backupserver:{fqdn}"
             one_file_system:
                 type: bool
                 desc: |
@@ -226,6 +230,12 @@ map:
                     Remote network upload rate limit in kiBytes/second. Defaults
                     to unlimited.
                 example: 100
+            temporary_directory:
+                type: str
+                desc: |
+                    Directory where temporary files are stored. Defaults to
+                    $TMPDIR
+                example: /path/to/tmpdir
             ssh_command:
                 type: str
                 desc: |

borgmatic/hooks/dispatch.py

@@ -58,5 +58,5 @@ def call_hooks(function_name, hooks, log_prefix, hook_names, *args, **kwargs):
     return {
         hook_name: call_hook(function_name, hooks, log_prefix, hook_name, *args, **kwargs)
         for hook_name in hook_names
-        if hook_name in hooks
+        if hooks.get(hook_name)
     }

borgmatic/signals.py

@@ -4,8 +4,13 @@ import signal
 
 def _handle_signal(signal_number, frame):  # pragma: no cover
     '''
-    Send the signal to all processes in borgmatic's process group, which includes child process.
+    Send the signal to all processes in borgmatic's process group, which includes child processes.
     '''
+    # Prevent infinite signal handler recursion. If the parent frame is this very same handler
+    # function, we know we're recursing.
+    if frame.f_back.f_code.co_name == _handle_signal.__name__:
+        return
+
     os.killpg(os.getpgrp(), signal_number)
 
 

docs/how-to/backup-your-databases.md

@@ -74,6 +74,17 @@ hooks:
 Note that you may need to use a `username` of the `postgres` superuser for
 this to work with PostgreSQL.
 
+If you would like to backup databases only and not source directories, you can
+specify an empty `source_directories` value because it is a mandatory field:
+
+```yaml
+location:
+    source_directories: []
+hooks:
+    mysql_databases:
+        - name: all
+```
+
 
 ### Configuration backups
 

docs/how-to/deal-with-very-large-backups.md

@@ -18,7 +18,7 @@ repositories.
 
 If you find yourself in this situation, you have some options. First, you can
 run borgmatic's pruning, creating, or checking actions separately. For
-instance, the the following optional actions are available:
+instance, the following optional actions are available:
 
 ```bash
 borgmatic prune

docs/how-to/make-backups-redundant.md

@@ -20,8 +20,9 @@ location:
 
     # Paths of local or remote repositories to backup to.
     repositories:
-        - k8pDxu32@k8pDxu32.repo.borgbase.com:repo
         - 1234@usw-s001.rsync.net:backups.borg
+        - k8pDxu32@k8pDxu32.repo.borgbase.com:repo
+        - user1@scp2.cdn.lima-labs.com:repo
         - /var/lib/backups/local.borg
 ```
 
@@ -32,9 +33,10 @@ your source directories.
 
 Here's a way of visualizing what borgmatic does with the above configuration:
 
-2. Backup `/home` and `/etc` to `k8pDxu32@k8pDxu32.repo.borgbase.com:repo`
 1. Backup `/home` and `/etc` to `1234@usw-s001.rsync.net:backups.borg`
-3. Backup `/home` and `/etc` to `/var/lib/backups/local.borg`
+2. Backup `/home` and `/etc` to `k8pDxu32@k8pDxu32.repo.borgbase.com:repo`
+3. Backup `/home` and `/etc` to `user1@scp2.cdn.lima-labs.com:repo`
+4. Backup `/home` and `/etc` to `/var/lib/backups/local.borg`
 
 This gives you redundancy of your data across repositories and even
 potentially across providers.

docs/how-to/set-up-backups.md

@@ -99,8 +99,9 @@ services helps support borgmatic development and hosting. (These are referral
 links, but without any tracking scripts or cookies.)
 
 <ul>
+ <li class="referral"><a href="https://www.rsync.net/cgi-bin/borg.cgi?campaign=borg&adgroup=borgmatic">rsync.net</a>: Cloud Storage provider with full support for borg and any other SSH/SFTP tool</li>
  <li class="referral"><a href="https://www.borgbase.com/?utm_source=borgmatic">BorgBase</a>: Borg hosting service with support for monitoring, 2FA, and append-only repos</li>
- <li>rsync.net: Cloud Storage provider with full support for borg and any other SSH/SFTP tool</li>
+ <li class="referral"><a href="https://storage.lima-labs.com/special-pricing-offer-for-borgmatic-users/">Lima-Labs</a>: Affordable, reliable cloud data storage accessable via SSH/SCP/FTP for Borg backups or any other bulk storage needs</li>
 </ul>
 
 ## Configuration
@@ -268,6 +269,10 @@ sudo mv borgmatic.service borgmatic.timer /etc/systemd/system/
 sudo systemctl enable --now borgmatic.timer
 ```
 
+Review the security settings in the service file and update them as needed.
+If `ProtectSystem=strict` is enabled and local repositories are used, then
+the repository path must be added to the `ReadWritePaths` list.
+
 Feel free to modify the timer file based on how frequently you'd like
 borgmatic to run.
 

sample/systemd/borgmatic.service

@@ -7,6 +7,38 @@ ConditionACPower=true
 [Service]
 Type=oneshot
 
+# Security settings for systemd running as root
+# For more details about this settings check the systemd manuals
+# https://www.freedesktop.org/software/systemd/man/systemd.exec.html
+LockPersonality=true
+# Certain borgmatic features like Healthchecks integration need MemoryDenyWriteExecute to be off.
+# But you can try setting it to "yes" for improved security if you don't use those features.
+MemoryDenyWriteExecute=no
+NoNewPrivileges=yes
+PrivateDevices=yes
+PrivateTmp=yes
+ProtectClock=yes
+ProtectControlGroups=yes
+ProtectHostname=yes
+ProtectKernelLogs=yes
+ProtectKernelModules=yes
+ProtectKernelTunables=yes
+RestrictAddressFamilies=AF_UNIX AF_INET AF_INET6 AF_NETLINK
+RestrictNamespaces=yes
+RestrictRealtime=yes
+RestrictSUIDSGID=yes
+SystemCallArchitectures=native
+SystemCallFilter=@system-service
+# Restrict write access
+# Change to 'ProtectSystem=strict' and uncomment 'ProtectHome' to make the whole file
+# system read-only be default and uncomment 'ReadWritePaths' for the required write access.
+# Add local repositroy paths to the list of 'ReadWritePaths' like '-/mnt/my_backup_drive'.
+ProtectSystem=full
+# ProtectHome=read-only
+# ReadWritePaths=-/root/.config/borg -/root/.cache/borg -/root/.borgmatic
+
+CapabilityBoundingSet=CAP_DAC_READ_SEARCH CAP_NET_RAW
+
 # Lower CPU and I/O priority.
 Nice=19
 CPUSchedulingPolicy=batch

setup.py

@@ -1,6 +1,6 @@
 from setuptools import find_packages, setup
 
-VERSION = '1.5.10'
+VERSION = '1.5.11.dev0'
 
 
 setup(

tests/end-to-end/test_borgmatic.py

@@ -19,6 +19,7 @@ def generate_configuration(config_path, repository_path):
         open(config_path)
         .read()
         .replace('user@backupserver:sourcehostname.borg', repository_path)
+        .replace('- user@backupserver:{fqdn}', '')
         .replace('- /home', '- {}'.format(config_path))
         .replace('- /etc', '')
         .replace('- /var/log/syslog*', '')

tests/integration/commands/test_arguments.py

@@ -71,6 +71,35 @@ def test_parse_arguments_with_log_file_verbosity_overrides_default():
     assert global_arguments.log_file_verbosity == -1
 
 
+def test_parse_arguments_with_single_override_parses():
+    flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default'])
+
+    arguments = module.parse_arguments('--override', 'foo.bar=baz')
+
+    global_arguments = arguments['global']
+    assert global_arguments.overrides == ['foo.bar=baz']
+
+
+def test_parse_arguments_with_multiple_overrides_parses():
+    flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default'])
+
+    arguments = module.parse_arguments('--override', 'foo.bar=baz', 'foo.quux=7')
+
+    global_arguments = arguments['global']
+    assert global_arguments.overrides == ['foo.bar=baz', 'foo.quux=7']
+
+
+def test_parse_arguments_with_multiple_overrides_and_flags_parses():
+    flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default'])
+
+    arguments = module.parse_arguments(
+        '--override', 'foo.bar=baz', '--override', 'foo.quux=7', 'this.that=8'
+    )
+
+    global_arguments = arguments['global']
+    assert global_arguments.overrides == ['foo.bar=baz', 'foo.quux=7', 'this.that=8']
+
+
 def test_parse_arguments_with_list_json_overrides_default():
     arguments = module.parse_arguments('list', '--json')
 

tests/integration/config/test_generate.py

@@ -87,8 +87,8 @@ location:
     assert module._comment_out_optional_configuration(config.strip()) == expected_config.strip()
 
 
-def test_render_configuration_converts_configuration_to_yaml_string():
-    yaml_string = module._render_configuration({'foo': 'bar'})
+def testrender_configuration_converts_configuration_to_yaml_string():
+    yaml_string = module.render_configuration({'foo': 'bar'})
 
     assert yaml_string == 'foo: bar\n'
 
@@ -194,7 +194,7 @@ def test_generate_sample_configuration_does_not_raise():
     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('render_configuration')
     flexmock(module).should_receive('_comment_out_optional_configuration')
     flexmock(module).should_receive('write_configuration')
 
@@ -208,7 +208,7 @@ def test_generate_sample_configuration_with_source_filename_does_not_raise():
     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('render_configuration')
     flexmock(module).should_receive('_comment_out_optional_configuration')
     flexmock(module).should_receive('write_configuration')
 

tests/unit/borg/test_create.py

@@ -1326,6 +1326,34 @@ def test_create_archive_with_archive_name_format_accepts_borg_placeholders():
     )
 
 
+def test_create_archive_with_repository_accepts_borg_placeholders():
+    flexmock(module).should_receive('borgmatic_source_directories').and_return([])
+    flexmock(module).should_receive('deduplicate_directories').and_return(('foo', 'bar'))
+    flexmock(module).should_receive('map_directories_to_devices').and_return({})
+    flexmock(module).should_receive('_expand_directories').and_return(())
+    flexmock(module).should_receive('_expand_home_directories').and_return(())
+    flexmock(module).should_receive('_write_pattern_file').and_return(None)
+    flexmock(module).should_receive('_make_pattern_flags').and_return(())
+    flexmock(module).should_receive('_make_exclude_flags').and_return(())
+    flexmock(module).should_receive('execute_command').with_args(
+        ('borg', 'create', '{fqdn}::Documents_{hostname}-{now}', 'foo', 'bar'),
+        output_log_level=logging.INFO,
+        output_file=None,
+        borg_local_path='borg',
+    )
+
+    module.create_archive(
+        dry_run=False,
+        repository='{fqdn}',
+        location_config={
+            'source_directories': ['foo', 'bar'],
+            'repositories': ['{fqdn}'],
+            'exclude_patterns': None,
+        },
+        storage_config={'archive_name_format': 'Documents_{hostname}-{now}'},
+    )
+
+
 def test_create_archive_with_extra_borg_options_calls_borg_with_extra_options():
     flexmock(module).should_receive('borgmatic_source_directories').and_return([])
     flexmock(module).should_receive('deduplicate_directories').and_return(('foo', 'bar'))

tests/unit/hooks/test_dispatch.py

@@ -58,7 +58,7 @@ def test_call_hooks_calls_each_hook_and_collects_return_values():
     assert return_values == expected_return_values
 
 
-def test_call_hooks_calls_skips_return_values_for_unconfigured_hooks():
+def test_call_hooks_calls_skips_return_values_for_missing_hooks():
     hooks = {'super_hook': flexmock()}
     expected_return_values = {'super_hook': flexmock()}
     flexmock(module).should_receive('call_hook').and_return(expected_return_values['super_hook'])
@@ -66,3 +66,13 @@ def test_call_hooks_calls_skips_return_values_for_unconfigured_hooks():
     return_values = module.call_hooks('do_stuff', hooks, 'prefix', ('super_hook', 'other_hook'), 55)
 
     assert return_values == expected_return_values
+
+
+def test_call_hooks_calls_skips_return_values_for_null_hooks():
+    hooks = {'super_hook': flexmock(), 'other_hook': None}
+    expected_return_values = {'super_hook': flexmock()}
+    flexmock(module).should_receive('call_hook').and_return(expected_return_values['super_hook'])
+
+    return_values = module.call_hooks('do_stuff', hooks, 'prefix', ('super_hook', 'other_hook'), 55)
+
+    assert return_values == expected_return_values