tests: Cleanup in new Updater tests

Add test for a refresh with just a local root.json.

Remove unused code. Add docstrings for raised exceptions, add TODOs for
the missing exception handling.

Signed-off-by: Jussi Kukkonen <[email protected]>

Author: Jussi Kukkonen

tests/test_updater_rework.py

@@ -122,38 +122,17 @@ def setUp(self):
                                               metadata_url,
                                               targets_url)
 
-    # Metadata role keys are needed by the test cases to make changes to the
-    # repository (e.g., adding a new target file to 'targets.json' and then
-    # requesting a refresh()).
-    self.role_keys = _load_role_keys(self.keystore_directory)
-
-
-
   def tearDown(self):
     # We are inheriting from custom class.
     unittest_toolbox.Modified_TestCase.tearDown(self)
 
     # Logs stdout and stderr from the sever subprocess.
     self.server_process_handler.flush_log()
 
-
-
-  # UNIT TESTS.
   def test_refresh(self):
 
     self.repository_updater.refresh()
 
-    for role in ['root', 'timestamp', 'snapshot', 'targets']:
-        metadata_obj = metadata.Metadata.from_file(os.path.join(
-            self.client_directory, role + '.json'))
-
-        metadata_obj_2 = metadata.Metadata.from_file(os.path.join(
-            self.repository_directory, 'metadata', role + '.json'))
-
-
-        self.assertDictEqual(metadata_obj.to_dict(),
-                             metadata_obj_2.to_dict())
-
     # Get targetinfo for 'file1.txt' listed in targets
     targetinfo1 = self.repository_updater.get_one_valid_targetinfo('file1.txt')
     # Get targetinfo for 'file3.txt' listed in the delegated role1
@@ -178,60 +157,16 @@ def test_refresh(self):
 
     self.assertListEqual(updated_targets, [])
 
+  def test_refresh_with_only_local_root(self):
+    os.remove(os.path.join(self.client_directory, "timestamp.json"))
+    os.remove(os.path.join(self.client_directory, "snapshot.json"))
+    os.remove(os.path.join(self.client_directory, "targets.json"))
+    os.remove(os.path.join(self.client_directory, "role1.json"))
+
+    self.repository_updater.refresh()
 
-def _load_role_keys(keystore_directory):
-
-  # Populating 'self.role_keys' by importing the required public and private
-  # keys of 'tuf/tests/repository_data/'.  The role keys are needed when
-  # modifying the remote repository used by the test cases in this unit test.
-
-  # The pre-generated key files in 'repository_data/keystore' are all encrypted with
-  # a 'password' passphrase.
-  EXPECTED_KEYFILE_PASSWORD = 'password'
-
-  # Store and return the cryptography keys of the top-level roles, including 1
-  # delegated role.
-  role_keys = {}
-
-  root_key_file = os.path.join(keystore_directory, 'root_key')
-  targets_key_file = os.path.join(keystore_directory, 'targets_key')
-  snapshot_key_file = os.path.join(keystore_directory, 'snapshot_key')
-  timestamp_key_file = os.path.join(keystore_directory, 'timestamp_key')
-  delegation_key_file = os.path.join(keystore_directory, 'delegation_key')
-
-  role_keys = {'root': {}, 'targets': {}, 'snapshot': {}, 'timestamp': {},
-               'role1': {}}
-
-  # Import the top-level and delegated role public keys.
-  role_keys['root']['public'] = \
-    repo_tool.import_rsa_publickey_from_file(root_key_file+'.pub')
-  role_keys['targets']['public'] = \
-    repo_tool.import_ed25519_publickey_from_file(targets_key_file+'.pub')
-  role_keys['snapshot']['public'] = \
-    repo_tool.import_ed25519_publickey_from_file(snapshot_key_file+'.pub')
-  role_keys['timestamp']['public'] = \
-      repo_tool.import_ed25519_publickey_from_file(timestamp_key_file+'.pub')
-  role_keys['role1']['public'] = \
-      repo_tool.import_ed25519_publickey_from_file(delegation_key_file+'.pub')
-
-  # Import the private keys of the top-level and delegated roles.
-  role_keys['root']['private'] = \
-    repo_tool.import_rsa_privatekey_from_file(root_key_file,
-                                              EXPECTED_KEYFILE_PASSWORD)
-  role_keys['targets']['private'] = \
-    repo_tool.import_ed25519_privatekey_from_file(targets_key_file,
-                                              EXPECTED_KEYFILE_PASSWORD)
-  role_keys['snapshot']['private'] = \
-    repo_tool.import_ed25519_privatekey_from_file(snapshot_key_file,
-                                              EXPECTED_KEYFILE_PASSWORD)
-  role_keys['timestamp']['private'] = \
-    repo_tool.import_ed25519_privatekey_from_file(timestamp_key_file,
-                                              EXPECTED_KEYFILE_PASSWORD)
-  role_keys['role1']['private'] = \
-    repo_tool.import_ed25519_privatekey_from_file(delegation_key_file,
-                                              EXPECTED_KEYFILE_PASSWORD)
-
-  return role_keys
+    # Get targetinfo for 'file3.txt' listed in the delegated role1
+    targetinfo3= self.repository_updater.get_one_valid_targetinfo('file3.txt')
 
 if __name__ == '__main__':
   utils.configure_test_logging(sys.argv)

tuf/client_rework/updater_rework.py

@@ -48,12 +48,17 @@ def __init__(
     ):
         """
         Args:
-            repository_dir: Local metadata directory. Must contain root.json
+            repository_dir: Local metadata directory. Directory must be
+                writable and it must contain at least a root.json file.
             metadata_base_url: Base URL for all remote metadata downloads
             target_base_url: Optional; Default base URL for all remote target
                 downloads. Can be individually set in download_target()
             fetcher: Optional; FetcherInterface implementation used to download
                 both metadata and targets. Default is RequestsFetcher
+
+        Raises:
+            OSError: Local root.json cannot be read
+            RepositoryError: Local root.json is invalid
         """
         self._dir = repository_dir
         self._metadata_base_url = _ensure_trailing_slash(metadata_base_url)
@@ -83,6 +88,11 @@ def refresh(self) -> None:
 
         The refresh() method should be called by the client before any target
         requests.
+
+        Raises:
+            OSError: New metadata could not be written to disk
+            RepositoryError: Metadata failed to verify in some way
+            TODO: download-related errors
         """
 
         self._load_root()
@@ -102,6 +112,11 @@ def get_one_valid_targetinfo(self, target_path: str) -> Dict:
                 (https://url.spec.whatwg.org/#path-relative-url-string).
                 Typically this is also the unix file path of the eventually
                 downloaded file.
+
+        Raises:
+            OSError: New metadata could not be written to disk
+            RepositoryError: Metadata failed to verify in some way
+            TODO: download-related errors
         """
         return self._preorder_depth_first_walk(target_path)
 
@@ -172,6 +187,10 @@ def download_target(
                 destination_directory as required.
             target_base_url: Optional; Base URL used to form the final target
                 download URL. Default is the value provided in Updater()
+
+        Raises:
+            TODO: download-related errors
+            TODO: file write errors
         """
         if target_base_url is None and self._target_base_url is None:
             raise ValueError(
@@ -269,6 +288,7 @@ def _load_snapshot(self) -> None:
         try:
             data = self._load_local_metadata("snapshot")
             self._bundle.update_snapshot(data)
+            logger.debug("Local snapshot is valid: not downloading new one")
         except (OSError, exceptions.RepositoryError) as e:
             # Local load failed: we must update from remote
             logger.debug("Failed to load local snapshot %s", e)
@@ -288,6 +308,7 @@ def _load_targets(self, role: str, parent_role: str) -> None:
         try:
             data = self._load_local_metadata(role)
             self._bundle.update_delegated_targets(data, role, parent_role)
+            logger.debug("Local %s is valid: not downloading new one", role)
         except (OSError, exceptions.RepositoryError) as e:
             # Local load failed: we must update from remote
             logger.debug("Failed to load local %s: %s", role, e)