Metadata API: Add id to Key

This simplifies life for API users as usually a key needs its
identifier: this is already visible in how update() becomes simpler
in the API.

The downside is that 'from_dict()' now has two arguments (so arguably
the name is not great anymore but it still does _mostly_ the same job
as other from_dicts).

This is an API change, if a minor one.

Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>

Author: Jussi Kukkonen

tests/test_api.py

@@ -373,18 +373,18 @@ def test_key_class(self):
             # Testing that the workflow of deserializing and serializing
             # a key dictionary doesn't change the content.
             test_key_dict = key_dict.copy()
-            key_obj = Key.from_dict(test_key_dict)
+            key_obj = Key.from_dict("id", test_key_dict)
             self.assertEqual(key_dict, key_obj.to_dict())
             # Test creating an instance without a required attribute.
             for key in key_dict.keys():
                 test_key_dict = key_dict.copy()
                 del test_key_dict[key]
                 with self.assertRaises(KeyError):
-                    Key.from_dict(test_key_dict)
+                    Key.from_dict("id", test_key_dict)
             # Test creating a Key instance with wrong keyval format.
             key_dict["keyval"] = {}
             with self.assertRaises(ValueError):
-                Key.from_dict(key_dict)
+                Key.from_dict("id", key_dict)
 
 
     def test_role_class(self):
@@ -413,7 +413,7 @@ def test_role_class(self):
                 test_role_dict = role_dict.copy()
                 del test_role_dict[role_attr]
                 with self.assertRaises(KeyError):
-                    Key.from_dict(test_role_dict)
+                    Key.from_dict("id", test_role_dict)
             # Test creating a Role instance with keyid dublicates.
             # for keyid in role_dict["keyids"]:
             role_dict["keyids"].append(role_dict["keyids"][0])
@@ -433,15 +433,15 @@ def test_metadata_root(self):
 
 
         keyid = root_key2['keyid']
-        key_metadata = Key(root_key2['keytype'], root_key2['scheme'],
+        key_metadata = Key(keyid, root_key2['keytype'], root_key2['scheme'],
             root_key2['keyval'])
 
         # Assert that root does not contain the new key
         self.assertNotIn(keyid, root.signed.roles['root'].keyids)
         self.assertNotIn(keyid, root.signed.keys)
 
         # Add new root key
-        root.signed.add_key('root', keyid, key_metadata)
+        root.signed.add_key('root', key_metadata)
 
         # Assert that key is added
         self.assertIn(keyid, root.signed.roles['root'].keyids)
@@ -453,7 +453,7 @@ def test_metadata_root(self):
 
         # Try adding the same key again and assert its ignored.
         pre_add_keyid = root.signed.roles['root'].keyids.copy()
-        root.signed.add_key('root', keyid, key_metadata)
+        root.signed.add_key('root', key_metadata)
         self.assertEqual(pre_add_keyid, root.signed.roles['root'].keyids)
 
         # Remove the key

tuf/api/metadata.py

@@ -417,6 +417,7 @@ class Key:
     """A container class representing the public portion of a Key.
 
     Attributes:
+        keyid: An identifier string
         keytype: A string denoting a public key signature system,
             such as "rsa", "ed25519", and "ecdsa-sha2-nistp256".
         scheme: A string denoting a corresponding signature scheme. For example:
@@ -428,26 +429,28 @@ class Key:
 
     def __init__(
         self,
+        keyid: str,
         keytype: str,
         scheme: str,
         keyval: Dict[str, str],
         unrecognized_fields: Optional[Mapping[str, Any]] = None,
     ) -> None:
         if not keyval.get("public"):
             raise ValueError("keyval doesn't follow the specification format!")
+        self.keyid = keyid
         self.keytype = keytype
         self.scheme = scheme
         self.keyval = keyval
         self.unrecognized_fields: Mapping[str, Any] = unrecognized_fields or {}
 
     @classmethod
-    def from_dict(cls, key_dict: Dict[str, Any]) -> "Key":
+    def from_dict(cls, keyid: str, key_dict: Dict[str, Any]) -> "Key":
         """Creates Key object from its dict representation."""
         keytype = key_dict.pop("keytype")
         scheme = key_dict.pop("scheme")
         keyval = key_dict.pop("keyval")
         # All fields left in the key_dict are unrecognized.
-        return cls(keytype, scheme, keyval, key_dict)
+        return cls(keyid, keytype, scheme, keyval, key_dict)
 
     def to_dict(self) -> Dict[str, Any]:
         """Returns the dictionary representation of self."""
@@ -558,7 +561,7 @@ def from_dict(cls, root_dict: Dict[str, Any]) -> "Root":
         roles = root_dict.pop("roles")
 
         for keyid, key_dict in keys.items():
-            keys[keyid] = Key.from_dict(key_dict)
+            keys[keyid] = Key.from_dict(keyid, key_dict)
         for role_name, role_dict in roles.items():
             roles[role_name] = Role.from_dict(role_dict)
 
@@ -584,10 +587,10 @@ def to_dict(self) -> Dict[str, Any]:
         return root_dict
 
     # Update key for a role.
-    def add_key(self, role: str, keyid: str, key_metadata: Key) -> None:
-        """Adds new key for 'role' and updates the key store."""
-        self.roles[role].keyids.add(keyid)
-        self.keys[keyid] = key_metadata
+    def add_key(self, role: str, key: Key) -> None:
+        """Adds new signing key for delegated role 'role'."""
+        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.
@@ -863,7 +866,7 @@ def from_dict(cls, delegations_dict: Dict[str, Any]) -> "Delegations":
         keys = delegations_dict.pop("keys")
         keys_res = {}
         for keyid, key_dict in keys.items():
-            keys_res[keyid] = Key.from_dict(key_dict)
+            keys_res[keyid] = Key.from_dict(keyid, key_dict)
         roles = delegations_dict.pop("roles")
         roles_res = []
         for role_dict in roles: