Modify "mount" and "extract" actions to require the "--repository" flag when multiple repositories are configured (#566).

NEWS

@@ -1,5 +1,7 @@
 1.6.7.dev0
  * #565: Fix handling of "repository" and "data" consistency checks to prevent invalid Borg flags.
+ * #566: Modify "mount" and "extract" actions to require the "--repository" flag when multiple
+   repositories are configured.
  * Add support for disabling TLS verification in Healthchecks monitoring hook with "verify_tls"
    option.
 

borgmatic/commands/borgmatic.py

@@ -768,21 +768,21 @@ def collect_configuration_run_summary_logs(configs, arguments):
     any, to stdout.
     '''
     # Run cross-file validation checks.
-    if 'extract' in arguments:
-        repository = arguments['extract'].repository
-    elif 'list' in arguments and arguments['list'].archive:
-        repository = arguments['list'].repository
-    elif 'mount' in arguments:
-        repository = arguments['mount'].repository
-    else:
-        repository = None
-
-    if repository:
-        try:
-            validate.guard_configuration_contains_repository(repository, configs)
-        except ValueError as error:
-            yield from log_error_records(str(error))
-            return
+    repository = None
+
+    for action_name, action_arguments in arguments.items():
+        if hasattr(action_arguments, 'repository'):
+            repository = getattr(action_arguments, 'repository')
+            break
+
+    try:
+        if 'extract' in arguments or 'mount' in arguments:
+            validate.guard_single_repository_selected(repository, configs)
+
+        validate.guard_configuration_contains_repository(repository, configs)
+    except ValueError as error:
+        yield from log_error_records(str(error))
+        return
 
     if not configs:
         yield from log_error_records(

borgmatic/config/validate.py

@@ -140,27 +140,13 @@ def repositories_match(first, second):
 def guard_configuration_contains_repository(repository, configurations):
     '''
     Given a repository path and a dict mapping from config filename to corresponding parsed config
-    dict, ensure that the repository is declared exactly once in all of the configurations.
-
-    If no repository is given, then error if there are multiple configured repositories.
+    dict, ensure that the repository is declared exactly once in all of the configurations. If no
+    repository is given, skip this check.
 
     Raise ValueError if the repository is not found in a configuration, or is declared multiple
     times.
     '''
     if not repository:
-        count = len(
-            tuple(
-                config_repository
-                for config in configurations.values()
-                for config_repository in config['location']['repositories']
-            )
-        )
-
-        if count > 1:
-            raise ValueError(
-                'Can\'t determine which repository to use. Use --repository option to disambiguate'
-            )
-
         return
 
     count = len(
@@ -176,3 +162,26 @@ def guard_configuration_contains_repository(repository, configurations):
         raise ValueError('Repository {} not found in configuration files'.format(repository))
     if count > 1:
         raise ValueError('Repository {} found in multiple configuration files'.format(repository))
+
+
+def guard_single_repository_selected(repository, configurations):
+    '''
+    Given a repository path and a dict mapping from config filename to corresponding parsed config
+    dict, ensure either a single repository exists across all configuration files or a repository
+    path was given.
+    '''
+    if repository:
+        return
+
+    count = len(
+        tuple(
+            config_repository
+            for config in configurations.values()
+            for config_repository in config['location']['repositories']
+        )
+    )
+
+    if count != 1:
+        raise ValueError(
+            'Can\'t determine which repository to use. Use --repository to disambiguate'
+        )

tests/unit/commands/test_borgmatic.py

@@ -746,6 +746,7 @@ def test_get_local_path_without_local_path_defaults_to_borg():
 
 def test_collect_configuration_run_summary_logs_info_for_success():
     flexmock(module.command).should_receive('execute_hook').never()
+    flexmock(module.validate).should_receive('guard_configuration_contains_repository')
     flexmock(module).should_receive('run_configuration').and_return([])
     arguments = {}
 
@@ -757,6 +758,7 @@ def test_collect_configuration_run_summary_logs_info_for_success():
 
 
 def test_collect_configuration_run_summary_executes_hooks_for_create():
+    flexmock(module.validate).should_receive('guard_configuration_contains_repository')
     flexmock(module).should_receive('run_configuration').and_return([])
     arguments = {'create': flexmock(), 'global': flexmock(monitoring_verbosity=1, dry_run=False)}
 
@@ -768,6 +770,7 @@ def test_collect_configuration_run_summary_executes_hooks_for_create():
 
 
 def test_collect_configuration_run_summary_logs_info_for_success_with_extract():
+    flexmock(module.validate).should_receive('guard_single_repository_selected')
     flexmock(module.validate).should_receive('guard_configuration_contains_repository')
     flexmock(module).should_receive('run_configuration').and_return([])
     arguments = {'extract': flexmock(repository='repo')}
@@ -795,6 +798,7 @@ def test_collect_configuration_run_summary_logs_extract_with_repository_error():
 
 
 def test_collect_configuration_run_summary_logs_info_for_success_with_mount():
+    flexmock(module.validate).should_receive('guard_single_repository_selected')
     flexmock(module.validate).should_receive('guard_configuration_contains_repository')
     flexmock(module).should_receive('run_configuration').and_return([])
     arguments = {'mount': flexmock(repository='repo')}
@@ -846,6 +850,7 @@ def test_collect_configuration_run_summary_logs_pre_hook_error():
 
 def test_collect_configuration_run_summary_logs_post_hook_error():
     flexmock(module.command).should_receive('execute_hook').and_return(None).and_raise(ValueError)
+    flexmock(module.validate).should_receive('guard_configuration_contains_repository')
     flexmock(module).should_receive('run_configuration').and_return([])
     expected_logs = (flexmock(),)
     flexmock(module).should_receive('log_error_records').and_return(expected_logs)
@@ -874,6 +879,7 @@ def test_collect_configuration_run_summary_logs_for_list_with_archive_and_reposi
 
 
 def test_collect_configuration_run_summary_logs_info_for_success_with_list():
+    flexmock(module.validate).should_receive('guard_configuration_contains_repository')
     flexmock(module).should_receive('run_configuration').and_return([])
     arguments = {'list': flexmock(repository='repo', archive=None)}
 
@@ -916,6 +922,7 @@ def test_collect_configuration_run_summary_logs_run_umount_error():
 
 
 def test_collect_configuration_run_summary_logs_outputs_merged_json_results():
+    flexmock(module.validate).should_receive('guard_configuration_contains_repository')
     flexmock(module).should_receive('run_configuration').and_return(['foo', 'bar']).and_return(
         ['baz']
     )

tests/unit/config/test_validate.py

@@ -120,14 +120,6 @@ def test_guard_configuration_contains_repository_does_not_raise_when_repository_
     )
 
 
-def test_guard_configuration_contains_repository_errors_when_repository_assumed_to_match_config_twice():
-    with pytest.raises(ValueError):
-        module.guard_configuration_contains_repository(
-            repository=None,
-            configurations={'config.yaml': {'location': {'repositories': ['repo', 'repo2']}}},
-        )
-
-
 def test_guard_configuration_contains_repository_errors_when_repository_missing_from_config():
     flexmock(module).should_receive('repositories_match').replace_with(
         lambda first, second: first == second
@@ -153,3 +145,30 @@ def test_guard_configuration_contains_repository_errors_when_repository_matches_
                 'other.yaml': {'location': {'repositories': ['repo']}},
             },
         )
+
+
+def test_guard_single_repository_selected_raises_when_multiple_repositories_configured_and_none_selected():
+    with pytest.raises(ValueError):
+        module.guard_single_repository_selected(
+            repository=None,
+            configurations={'config.yaml': {'location': {'repositories': ['repo', 'repo2']}}},
+        )
+
+
+def test_guard_single_repository_selected_does_not_raise_when_single_repository_configured_and_none_selected():
+    module.guard_single_repository_selected(
+        repository=None, configurations={'config.yaml': {'location': {'repositories': ['repo']}}},
+    )
+
+
+def test_guard_single_repository_selected_does_not_raise_when_no_repositories_configured_and_one_selected():
+    module.guard_single_repository_selected(
+        repository='repo', configurations={'config.yaml': {'location': {'repositories': []}}},
+    )
+
+
+def test_guard_single_repository_selected_does_not_raise_when_repositories_configured_and_one_selected():
+    module.guard_single_repository_selected(
+        repository='repo',
+        configurations={'config.yaml': {'location': {'repositories': ['repo', 'repo2']}}},
+    )