Dvc remote default in list validation

dvc/command/remote.py

@@ -46,18 +46,17 @@ def run(self):
 
 class CmdRemoteRemove(CmdRemote):
     def run(self):
-        with self.config.edit(self.args.level) as conf:
-            self._check_exists(conf)
-            del conf["remote"][self.args.name]
+        conf = self.config.load_one(self.args.level)
+        self._check_exists(conf)
 
         # Remove core.remote refs to this remote in any shadowing configs
         for level in reversed(self.config.LEVELS):
             with self.config.edit(level) as conf:
                 if conf["core"].get("remote") == self.args.name:
                     del conf["core"]["remote"]
-
-            if level == self.args.level:
-                break
+                if level == self.args.level:
+                    del conf["remote"][self.args.name]
+                    break
 
         return 0
 

dvc/config.py

@@ -219,7 +219,8 @@ class Config(dict):
     CONFIG = "config"
     CONFIG_LOCAL = "config.local"
 
-    def __init__(self, dvc_dir=None, validate=True):
+    def __init__(self, dvc_dir=None, validate=True, **kwargs):
+        super().__init__(**kwargs)
         self.dvc_dir = dvc_dir
 
         if not dvc_dir:
@@ -238,12 +239,11 @@ def __init__(self, dvc_dir=None, validate=True):
     def get_dir(cls, level):
         from appdirs import user_config_dir, site_config_dir
 
-        assert level in ("global", "system")
-
         if level == "global":
             return user_config_dir(cls.APPNAME, cls.APPAUTHOR)
         if level == "system":
             return site_config_dir(cls.APPNAME, cls.APPAUTHOR)
+        raise ConfigError("This method only used in global or system level.")
 
     @cached_property
     def files(self):
@@ -359,6 +359,11 @@ def edit(self, level="repo"):
     @staticmethod
     def validate(data):
         try:
+            if (
+                "remote" in data["core"]
+                and data["core"]["remote"] not in data["remote"]
+            ):
+                raise ConfigError("Default remote not in the remote list.")
             return COMPILED_SCHEMA(data)
         except Invalid as exc:
             raise ConfigError(str(exc)) from None

tests/func/test_config.py

@@ -78,7 +78,7 @@ def test_non_existing(self):
         self.assertEqual(ret, 251)
 
         ret = main(["config", "core.remote", "myremote"])
-        self.assertEqual(ret, 0)
+        self.assertEqual(ret, 251)
 
         ret = main(["config", "core.non_existing_field", "-u"])
         self.assertEqual(ret, 251)
@@ -109,6 +109,36 @@ def test_merging_two_levels(dvc):
     }
 
 
+def test_remote_default_set(dvc):
+    local_level = "local_remote"
+    repo_level = "repo_remote"
+    with dvc.config.edit() as conf:
+        conf["remote"][repo_level] = {"url": "ssh://example.com"}
+    with dvc.config.edit() as conf:
+        conf["remote"][local_level] = {"url": "ssh://example.com"}
+
+    # repo-level remote repo cannot set default in global level
+    with pytest.raises(ConfigError):
+        with dvc.config.edit("global") as conf:
+            conf["core"]["remote"] = repo_level
+    assert "remote" not in dvc.config["core"]
+
+    # remote default must in remote list
+    with pytest.raises(ConfigError):
+        with dvc.config.edit("local") as conf:
+            conf["core"]["remote"] = "not_in_list"
+    assert "remote" not in dvc.config["core"]
+
+    with dvc.config.edit() as conf:
+        conf["core"]["remote"] = repo_level
+    assert dvc.config["core"]["remote"] == repo_level
+
+    # repo-level remote repo can set default in local level
+    with dvc.config.edit("local") as conf:
+        conf["core"]["remote"] = local_level
+    assert dvc.config["core"]["remote"] == local_level
+
+
 def test_config_loads_without_error_for_non_dvc_repo(tmp_dir):
     # regression testing for https://github.com/iterative/dvc/issues/3328
     Config(validate=True)

tests/func/test_remote.py

@@ -121,6 +121,7 @@ def test(self):
 
 
 def test_show_default(dvc, capsys):
+    assert main(["remote", "add", "foo", "s3://bucket/name"]) == 0
     assert main(["remote", "default", "foo"]) == 0
     assert main(["remote", "default"]) == 0
     out, _ = capsys.readouterr()
@@ -270,3 +271,21 @@ def test_remote_modify_validation(dvc):
     )
     config = configobj.ConfigObj(dvc.config.files["repo"])
     assert unsupported_config not in config['remote "{}"'.format(remote_name)]
+
+
+def test_remote_modify_default(dvc):
+    remote_repo = "repo_level"
+    remote_local = "local_level"
+    wrong_name = "anything"
+    assert main(["remote", "add", remote_repo, "s3://bucket/repo"]) == 0
+    assert main(["remote", "add", remote_local, "s3://bucket/local"]) == 0
+
+    assert main(["remote", "default", wrong_name]) == 251
+    assert main(["remote", "default", remote_repo]) == 0
+    assert main(["remote", "default", "--local", remote_local]) == 0
+
+    repo_config = configobj.ConfigObj(dvc.config.files["repo"])
+    local_config = configobj.ConfigObj(dvc.config.files["local"])
+
+    assert repo_config["core"]["remote"] == remote_repo
+    assert local_config["core"]["remote"] == remote_local