Warn and tranform on non-ssh://-style repositories (#557).

README.md

@@ -24,8 +24,8 @@ location:
 
     # Paths of local or remote repositories to backup to.
     repositories:
-        - ssh://1234@usw-s001.rsync.net:backups.borg
-        - ssh://k8pDxu32@k8pDxu32.repo.borgbase.com:repo
+        - ssh://1234@usw-s001.rsync.net/backups.borg
+        - ssh://k8pDxu32@k8pDxu32.repo.borgbase.com/repo
         - /var/lib/backups/local.borg
 
 retention:

borgmatic/commands/borgmatic.py

@@ -743,9 +743,10 @@ def load_configurations(config_filenames, overrides=None, resolve_env=True):
     # Parse and load each configuration file.
     for config_filename in config_filenames:
         try:
-            configs[config_filename] = validate.parse_configuration(
+            configs[config_filename], parse_logs = validate.parse_configuration(
                 config_filename, validate.schema_filename(), overrides, resolve_env
             )
+            logs.extend(parse_logs)
         except PermissionError:
             logs.extend(
                 [

borgmatic/config/normalize.py

@@ -1,8 +1,14 @@
-def normalize(config):
+import logging
+
+
+def normalize(config_filename, config):
     '''
-    Given a configuration dict, apply particular hard-coded rules to normalize its contents to
-    adhere to the configuration schema.
+    Given a configuration filename and a configuration dict of its loaded contents, apply particular
+    hard-coded rules to normalize the configuration to adhere to the current schema. Return any log
+    message warnings produced based on the normalization performed.
     '''
+    logs = []
+
     # Upgrade exclude_if_present from a string to a list.
     exclude_if_present = config.get('location', {}).get('exclude_if_present')
     if isinstance(exclude_if_present, str):
@@ -29,3 +35,38 @@ def normalize(config):
     checks = config.get('consistency', {}).get('checks')
     if isinstance(checks, list) and len(checks) and isinstance(checks[0], str):
         config['consistency']['checks'] = [{'name': check_type} for check_type in checks]
+
+    # Upgrade remote repositories to ssh:// syntax, required in Borg 2.
+    repositories = config.get('location', {}).get('repositories')
+    if repositories:
+        config['location']['repositories'] = []
+        for repository in repositories:
+            # TODO: Instead of logging directly here, return logs and bubble them up to be displayed *after* logging is initialized.
+            if '~' in repository:
+                logs.append(
+                    logging.makeLogRecord(
+                        dict(
+                            levelno=logging.WARNING,
+                            levelname='WARNING',
+                            msg=f'{config_filename}: Repository paths containing "~" are deprecated in borgmatic and no longer work in Borg 2.',
+                        )
+                    )
+                )
+            if ':' in repository and not repository.startswith('ssh://'):
+                rewritten_repository = (
+                    f"ssh://{repository.replace(':~', '/~').replace(':/', '/').replace(':', '/./')}"
+                )
+                logs.append(
+                    logging.makeLogRecord(
+                        dict(
+                            levelno=logging.WARNING,
+                            levelname='WARNING',
+                            msg=f'{config_filename}: Remote repository paths without ssh:// syntax are deprecated. Interpreting "{repository}" as "{rewritten_repository}"',
+                        )
+                    )
+                )
+                config['location']['repositories'].append(rewritten_repository)
+            else:
+                config['location']['repositories'].append(repository)
+
+    return logs

borgmatic/config/validate.py

@@ -89,6 +89,9 @@ def parse_configuration(config_filename, schema_filename, overrides=None, resolv
        {'location': {'source_directories': ['/home', '/etc'], 'repository': 'hostname.borg'},
        'retention': {'keep_daily': 7}, 'consistency': {'checks': ['repository', 'archives']}}
 
+    Also return a sequence of logging.LogRecord instances containing any warnings about the
+    configuration.
+
     Raise FileNotFoundError if the file does not exist, PermissionError if the user does not
     have permissions to read the file, or Validation_error if the config does not match the schema.
     '''
@@ -99,7 +102,7 @@ def parse_configuration(config_filename, schema_filename, overrides=None, resolv
         raise Validation_error(config_filename, (str(error),))
 
     override.apply_overrides(config, overrides)
-    normalize.normalize(config)
+    logs = normalize.normalize(config_filename, config)
     if resolve_env:
         environment.resolve_env_variables(config)
 
@@ -116,7 +119,7 @@ def parse_configuration(config_filename, schema_filename, overrides=None, resolv
 
     apply_logical_validation(config_filename, config)
 
-    return config
+    return config, logs
 
 
 def normalize_repository_path(repository):

docs/how-to/backup-to-a-removable-drive-or-an-intermittent-server.md

@@ -76,7 +76,7 @@ location:
         - /home
 
     repositories:
-        - ssh://me@buddys-server.org:backup.borg
+        - ssh://me@buddys-server.org/backup.borg
 
 hooks:
     before_backup:

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

@@ -20,8 +20,8 @@ location:
 
     # Paths of local or remote repositories to backup to.
     repositories:
-        - ssh://1234@usw-s001.rsync.net:backups.borg
-        - ssh://k8pDxu32@k8pDxu32.repo.borgbase.com:repo
+        - ssh://1234@usw-s001.rsync.net/backups.borg
+        - ssh://k8pDxu32@k8pDxu32.repo.borgbase.com/repo
         - /var/lib/backups/local.borg
 ```
 

tests/integration/config/test_validate.py

@@ -60,39 +60,39 @@ def test_parse_configuration_transforms_file_into_mapping():
         '''
     )
 
-    result = module.parse_configuration('/tmp/config.yaml', '/tmp/schema.yaml')
+    config, logs = module.parse_configuration('/tmp/config.yaml', '/tmp/schema.yaml')
 
-    assert result == {
+    assert config == {
         'location': {'source_directories': ['/home', '/etc'], 'repositories': ['hostname.borg']},
         'retention': {'keep_daily': 7, 'keep_hourly': 24, 'keep_minutely': 60},
         'consistency': {'checks': [{'name': 'repository'}, {'name': 'archives'}]},
     }
+    assert logs == []
 
 
 def test_parse_configuration_passes_through_quoted_punctuation():
     escaped_punctuation = string.punctuation.replace('\\', r'\\').replace('"', r'\"')
 
     mock_config_and_schema(
-        '''
+        f'''
         location:
             source_directories:
-                - /home
+                - "/home/{escaped_punctuation}"
 
             repositories:
-                - "{}.borg"
-        '''.format(
-            escaped_punctuation
-        )
+                - test.borg
+        '''
     )
 
-    result = module.parse_configuration('/tmp/config.yaml', '/tmp/schema.yaml')
+    config, logs = module.parse_configuration('/tmp/config.yaml', '/tmp/schema.yaml')
 
-    assert result == {
+    assert config == {
         'location': {
-            'source_directories': ['/home'],
-            'repositories': ['{}.borg'.format(string.punctuation)],
+            'source_directories': [f'/home/{string.punctuation}'],
+            'repositories': ['test.borg'],
         }
     }
+    assert logs == []
 
 
 def test_parse_configuration_with_schema_lacking_examples_does_not_raise():
@@ -148,12 +148,13 @@ def test_parse_configuration_inlines_include():
     include_file.name = 'include.yaml'
     builtins.should_receive('open').with_args('/tmp/include.yaml').and_return(include_file)
 
-    result = module.parse_configuration('/tmp/config.yaml', '/tmp/schema.yaml')
+    config, logs = module.parse_configuration('/tmp/config.yaml', '/tmp/schema.yaml')
 
-    assert result == {
+    assert config == {
         'location': {'source_directories': ['/home'], 'repositories': ['hostname.borg']},
         'retention': {'keep_daily': 7, 'keep_hourly': 24},
     }
+    assert logs == []
 
 
 def test_parse_configuration_merges_include():
@@ -181,12 +182,13 @@ def test_parse_configuration_merges_include():
     include_file.name = 'include.yaml'
     builtins.should_receive('open').with_args('/tmp/include.yaml').and_return(include_file)
 
-    result = module.parse_configuration('/tmp/config.yaml', '/tmp/schema.yaml')
+    config, logs = module.parse_configuration('/tmp/config.yaml', '/tmp/schema.yaml')
 
-    assert result == {
+    assert config == {
         'location': {'source_directories': ['/home'], 'repositories': ['hostname.borg']},
         'retention': {'keep_daily': 1, 'keep_hourly': 24},
     }
+    assert logs == []
 
 
 def test_parse_configuration_raises_for_missing_config_file():
@@ -238,17 +240,18 @@ def test_parse_configuration_applies_overrides():
         '''
     )
 
-    result = module.parse_configuration(
+    config, logs = module.parse_configuration(
         '/tmp/config.yaml', '/tmp/schema.yaml', overrides=['location.local_path=borg2']
     )
 
-    assert result == {
+    assert config == {
         'location': {
             'source_directories': ['/home'],
             'repositories': ['hostname.borg'],
             'local_path': 'borg2',
         }
     }
+    assert logs == []
 
 
 def test_parse_configuration_applies_normalization():
@@ -265,12 +268,13 @@ def test_parse_configuration_applies_normalization():
         '''
     )
 
-    result = module.parse_configuration('/tmp/config.yaml', '/tmp/schema.yaml')
+    config, logs = module.parse_configuration('/tmp/config.yaml', '/tmp/schema.yaml')
 
-    assert result == {
+    assert config == {
         'location': {
             'source_directories': ['/home'],
             'repositories': ['hostname.borg'],
             'exclude_if_present': ['.nobackup'],
         }
     }
+    assert logs == []

tests/unit/commands/test_borgmatic.py

@@ -699,17 +699,19 @@ def test_run_actions_does_not_raise_for_borg_action():
     )
 
 
-def test_load_configurations_collects_parsed_configurations():
+def test_load_configurations_collects_parsed_configurations_and_logs():
     configuration = flexmock()
     other_configuration = flexmock()
+    test_expected_logs = [flexmock(), flexmock()]
+    other_expected_logs = [flexmock(), flexmock()]
     flexmock(module.validate).should_receive('parse_configuration').and_return(
-        configuration
-    ).and_return(other_configuration)
+        configuration, test_expected_logs
+    ).and_return(other_configuration, other_expected_logs)
 
     configs, logs = tuple(module.load_configurations(('test.yaml', 'other.yaml')))
 
     assert configs == {'test.yaml': configuration, 'other.yaml': other_configuration}
-    assert logs == []
+    assert logs == test_expected_logs + other_expected_logs
 
 
 def test_load_configurations_logs_warning_for_permission_error():

tests/unit/config/test_normalize.py

@@ -4,44 +4,83 @@ from borgmatic.config import normalize as module
 
 
 @pytest.mark.parametrize(
-    'config,expected_config',
+    'config,expected_config,produces_logs',
     (
         (
             {'location': {'exclude_if_present': '.nobackup'}},
             {'location': {'exclude_if_present': ['.nobackup']}},
+            False,
         ),
         (
             {'location': {'exclude_if_present': ['.nobackup']}},
             {'location': {'exclude_if_present': ['.nobackup']}},
+            False,
         ),
         (
             {'location': {'source_directories': ['foo', 'bar']}},
             {'location': {'source_directories': ['foo', 'bar']}},
+            False,
+        ),
+        (
+            {'storage': {'compression': 'yes_please'}},
+            {'storage': {'compression': 'yes_please'}},
+            False,
         ),
-        ({'storage': {'compression': 'yes_please'}}, {'storage': {'compression': 'yes_please'}}),
         (
             {'hooks': {'healthchecks': 'https://example.com'}},
             {'hooks': {'healthchecks': {'ping_url': 'https://example.com'}}},
+            False,
         ),
         (
             {'hooks': {'cronitor': 'https://example.com'}},
             {'hooks': {'cronitor': {'ping_url': 'https://example.com'}}},
+            False,
         ),
         (
             {'hooks': {'pagerduty': 'https://example.com'}},
             {'hooks': {'pagerduty': {'integration_key': 'https://example.com'}}},
+            False,
         ),
         (
             {'hooks': {'cronhub': 'https://example.com'}},
             {'hooks': {'cronhub': {'ping_url': 'https://example.com'}}},
+            False,
         ),
         (
             {'consistency': {'checks': ['archives']}},
             {'consistency': {'checks': [{'name': 'archives'}]}},
+            False,
+        ),
+        (
+            {'location': {'repositories': ['foo@bar:/repo']}},
+            {'location': {'repositories': ['ssh://foo@bar/repo']}},
+            True,
+        ),
+        (
+            {'location': {'repositories': ['foo@bar:repo']}},
+            {'location': {'repositories': ['ssh://foo@bar/./repo']}},
+            True,
+        ),
+        (
+            {'location': {'repositories': ['foo@bar:~/repo']}},
+            {'location': {'repositories': ['ssh://foo@bar/~/repo']}},
+            True,
+        ),
+        (
+            {'location': {'repositories': ['ssh://foo@bar/repo']}},
+            {'location': {'repositories': ['ssh://foo@bar/repo']}},
+            False,
         ),
     ),
 )
-def test_normalize_applies_hard_coded_normalization_to_config(config, expected_config):
-    module.normalize(config)
+def test_normalize_applies_hard_coded_normalization_to_config(
+    config, expected_config, produces_logs
+):
+    logs = module.normalize('test.yaml', config)
 
     assert config == expected_config
+
+    if produces_logs:
+        assert logs
+    else:
+        assert logs == []