Metadata API: ValueError in add/remove key in Root

Modify the exception thrown in Root.add_key() and Root.remove_key() to
be "ValueError" instead of "KeyError".
The reason for that change is that in my opinion the correct exception
here should be ValueError as the problems are in the given values -
role doesn't exist or key is not used by a particular role.

Additionally, document the thrown exceptions in "Root.add_key" and
add a test which invokes that exception.

Signed-off-by: Martin Vrachev <mvrachev@vmware.com>

tests/test_api.py

@@ -465,6 +465,10 @@ def test_root_add_key_and_remove_key(self):
         # Add the same key to targets role as well
         root.signed.add_key('targets', key_metadata)
 
+        # Add the same key to a nonexistent role.
+        with self.assertRaises(ValueError):
+            root.signed.add_key("nosuchrole", key_metadata)
+
         # Remove the key from root role (targets role still uses it)
         root.signed.remove_key('root', keyid)
         self.assertNotIn(keyid, root.signed.roles['root'].keyids)
@@ -475,7 +479,7 @@ def test_root_add_key_and_remove_key(self):
         self.assertNotIn(keyid, root.signed.roles['targets'].keyids)
         self.assertNotIn(keyid, root.signed.keys)
 
-        with self.assertRaises(KeyError):
+        with self.assertRaises(ValueError):
             root.signed.remove_key('root', 'nosuchkey')
 
     def test_is_target_in_pathpattern(self):

tuf/api/metadata.py

@@ -741,16 +741,24 @@ def to_dict(self) -> Dict[str, Any]:
 
     # Update key for a role.
     def add_key(self, role: str, key: Key) -> None:
-        """Adds new signing key for delegated role 'role'."""
+        """Adds new signing key for delegated role 'role'.
+
+        Raises:
+            ValueError: If 'role' doesn't exist.
+        """
+        if role not in self.roles:
+            raise ValueError(f"Role {role} doesn't exist")
         self.roles[role].keyids.add(key.keyid)
         self.keys[key.keyid] = key
 
     def remove_key(self, role: str, keyid: str) -> None:
         """Removes key from 'role' and updates the key store.
 
         Raises:
-            KeyError: If 'role' does not include the key
+            ValueError: If 'role' does not include the key.
         """
+        if keyid not in self.roles[role].keyids:
+            raise ValueError(f"Key with id {keyid} is not used by {role}")
         self.roles[role].keyids.remove(keyid)
         for keyinfo in self.roles.values():
             if keyid in keyinfo.keyids: