config: verify before saving a modification

Close #3192

dvc/config.py

@@ -252,7 +252,7 @@ class Config(object):  # pylint: disable=too-many-instance-attributes
 
     def __init__(self, dvc_dir=None, validate=True):
         self.dvc_dir = dvc_dir
-        self.validate = validate
+        self.should_validate = validate
 
         if not dvc_dir:
             try:
@@ -382,14 +382,10 @@ class Config(object):  # pylint: disable=too-many-instance-attributes
             c = self._lower(c)
             self.config.merge(c)
 
-        if not self.validate:
+        if not self.should_validate:
             return
 
-        d = self.config.dict()
-        try:
-            d = self.COMPILED_SCHEMA(d)
-        except Invalid as exc:
-            raise ConfigError(str(exc)) from exc
+        d = self.validate()
         self.config = configobj.ConfigObj(d, write_empty_values=True)
 
     def save(self, config=None):
@@ -427,6 +423,14 @@ class Config(object):  # pylint: disable=too-many-instance-attributes
                 raise
         config.write()
 
+    def validate(self, config=None):
+        d = (config or self.config).dict()
+
+        try:
+            return self.COMPILED_SCHEMA(d)
+        except Invalid as exc:
+            raise ConfigError(str(exc)) from exc
+
     def unset(self, section, opt=None, level=None, force=False):
         """Unsets specified option and/or section in the config.
 
@@ -491,6 +495,8 @@ class Config(object):  # pylint: disable=too-many-instance-attributes
             )
 
         config[section][opt] = value
+
+        self.validate(config)
         self.save(config)
 
     def get(self, section, opt=None, level=None):

tests/func/test_config.py

@@ -33,11 +33,11 @@ class TestConfigCLI(TestDvc):
         self.assertEqual(ret, 0)
 
     def _do_test(self, local=False):
-        section = "setsection"
-        field = "setfield"
+        section = "core"
+        field = "analytics"
         section_field = "{}.{}".format(section, field)
-        value = "setvalue"
-        newvalue = "setnewvalue"
+        value = "True"
+        newvalue = "False"
 
         base = ["config"]
         if local:

tests/func/test_remote.py

@@ -32,7 +32,7 @@ class TestRemote(TestDvc):
         self.assertEqual(main(["remote", "list"]), 0)
 
         self.assertEqual(
-            main(["remote", "modify", remotes[0], "option", "value"]), 0
+            main(["remote", "modify", remotes[0], "checksum_jobs", "1"]), 0
         )
         self.assertEqual(main(["remote", "remove", remotes[0]]), 0)
 
@@ -113,7 +113,7 @@ class TestRemoteRemoveDefault(TestDvc):
 
 class TestRemoteRemove(TestDvc):
     def test(self):
-        ret = main(["config", "core.jobs", "1"])
+        ret = main(["config", "core.checksum_jobs", "1"])
         self.assertEqual(ret, 0)
 
         remote = "mys3"
@@ -257,3 +257,11 @@ def test_modify_missing_remote(dvc):
 
     with pytest.raises(ConfigError, match=r"unable to find remote section"):
         remote_config.modify("myremote", "gdrive_client_id", "xxx")
+
+
+def test_modify_invalid_key(dvc):
+    remote_config = RemoteConfig(dvc.config)
+    remote_config.add("example", "https://example.com")
+
+    with pytest.raises(ConfigError, match=r"extra keys not allowed"):
+        remote_config.modify("example", "random_key", "value")