diff --git a/tests/test_updater_ng.py b/tests/test_updater_ng.py index cfdc178d94..69fbeef8cd 100644 --- a/tests/test_updater_ng.py +++ b/tests/test_updater_ng.py @@ -16,7 +16,7 @@ import tuf.unittest_toolbox as unittest_toolbox from tests import utils -from tuf.api.metadata import Metadata +from tuf.api.metadata import Metadata, TargetFile from tuf import exceptions, ngclient from securesystemslib.signer import SSlibSigner from securesystemslib.interface import import_rsa_privatekey_from_file @@ -111,11 +111,14 @@ def setUp(self): self.metadata_url = f"{url_prefix}/metadata/" self.targets_url = f"{url_prefix}/targets/" - self.destination_directory = self.make_temp_directory() + self.dl_dir = self.make_temp_directory() # Creating a repository instance. The test cases will use this client # updater to refresh metadata, fetch target files, etc. - self.repository_updater = ngclient.Updater( - self.client_directory, self.metadata_url, self.targets_url + self.updater = ngclient.Updater( + repository_dir=self.client_directory, + metadata_base_url=self.metadata_url, + target_dir=self.dl_dir, + target_base_url=self.targets_url ) def tearDown(self): @@ -187,53 +190,40 @@ def consistent_snapshot_modifier(root): self._modify_repository_root( consistent_snapshot_modifier, bump_version=True ) - self.repository_updater = ngclient.Updater( - self.client_directory, self.metadata_url, self.targets_url + updater = ngclient.Updater( + self.client_directory, self.metadata_url, self.dl_dir, self.targets_url ) # All metadata is in local directory already - self.repository_updater.refresh() + updater.refresh() # Make sure that consistent snapshot is enabled self.assertTrue( - self.repository_updater._trusted_set.root.signed.consistent_snapshot - ) - # 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 - targetinfo3 = self.repository_updater.get_one_valid_targetinfo( - "file3.txt" + updater._trusted_set.root.signed.consistent_snapshot ) + # Get targetinfos, assert cache does not contain the files + info1 = updater.get_targetinfo("file1.txt") + info3 = updater.get_targetinfo("file3.txt") + self.assertIsNone(updater.find_cached_target(info1)) + self.assertIsNone(updater.find_cached_target(info3)) + # Create consistent targets with file path HASH.FILENAME.EXT - target1_hash = list(targetinfo1.hashes.values())[0] - target3_hash = list(targetinfo3.hashes.values())[0] + target1_hash = list(info1.hashes.values())[0] + target3_hash = list(info3.hashes.values())[0] self._create_consistent_target("file1.txt", target1_hash) self._create_consistent_target("file3.txt", target3_hash) - updated_targets = self.repository_updater.updated_targets( - [targetinfo1, targetinfo3], self.destination_directory - ) - - self.assertListEqual(updated_targets, [targetinfo1, targetinfo3]) - self.repository_updater.download_target( - targetinfo1, self.destination_directory - ) - updated_targets = self.repository_updater.updated_targets( - updated_targets, self.destination_directory - ) - - self.assertListEqual(updated_targets, [targetinfo3]) - - self.repository_updater.download_target( - targetinfo3, self.destination_directory - ) - updated_targets = self.repository_updater.updated_targets( - updated_targets, self.destination_directory - ) + # Download files, assert that cache has correct files + updater.download_target(info1) + path = updater.find_cached_target(info1) + self.assertEqual(path, os.path.join(self.dl_dir, info1.path)) + self.assertIsNone(updater.find_cached_target(info3)) - self.assertListEqual(updated_targets, []) + updater.download_target(info3) + path = updater.find_cached_target(info1) + self.assertEqual(path, os.path.join(self.dl_dir, info1.path)) + path = updater.find_cached_target(info3) + self.assertEqual(path, os.path.join(self.dl_dir, info3.path)) def test_refresh_and_download(self): # Test refresh without consistent targets - targets without hash prefixes. @@ -244,50 +234,33 @@ def test_refresh_and_download(self): os.remove(os.path.join(self.client_directory, "1.root.json")) # top-level metadata is in local directory already - self.repository_updater.refresh() + self.updater.refresh() self._assert_files(["root", "snapshot", "targets", "timestamp"]) - # Get targetinfo for 'file1.txt' listed in targets - targetinfo1 = self.repository_updater.get_one_valid_targetinfo( - "file1.txt" - ) + # Get targetinfos, assert that cache does not contain files + info1 = self.updater.get_targetinfo("file1.txt") self._assert_files(["root", "snapshot", "targets", "timestamp"]) + # Get targetinfo for 'file3.txt' listed in the delegated role1 - targetinfo3 = self.repository_updater.get_one_valid_targetinfo( - "file3.txt" - ) + info3 = self.updater.get_targetinfo("file3.txt") expected_files = ["role1", "root", "snapshot", "targets", "timestamp"] self._assert_files(expected_files) + self.assertIsNone(self.updater.find_cached_target(info1)) + self.assertIsNone(self.updater.find_cached_target(info3)) - updated_targets = self.repository_updater.updated_targets( - [targetinfo1, targetinfo3], self.destination_directory - ) - - self.assertListEqual(updated_targets, [targetinfo1, targetinfo3]) - - self.repository_updater.download_target( - targetinfo1, self.destination_directory - ) - updated_targets = self.repository_updater.updated_targets( - updated_targets, self.destination_directory + # Download files, assert that cache has correct files + self.updater.download_target(info1) + path = self.updater.find_cached_target(info1) + self.assertEqual(path, os.path.join(self.dl_dir, info1.path)) + self.assertIsNone( + self.updater.find_cached_target(info3) ) - self.assertListEqual(updated_targets, [targetinfo3]) - - # Check that duplicates are excluded - updated_targets = self.repository_updater.updated_targets( - [targetinfo3, targetinfo3], self.destination_directory - ) - self.assertListEqual(updated_targets, [targetinfo3]) - - self.repository_updater.download_target( - targetinfo3, self.destination_directory - ) - updated_targets = self.repository_updater.updated_targets( - updated_targets, self.destination_directory - ) - - self.assertListEqual(updated_targets, []) + self.updater.download_target(info3) + path = self.updater.find_cached_target(info1) + self.assertEqual(path, os.path.join(self.dl_dir, info1.path)) + path = self.updater.find_cached_target(info3) + self.assertEqual(path, os.path.join(self.dl_dir, info3.path)) def test_refresh_with_only_local_root(self): os.remove(os.path.join(self.client_directory, "timestamp.json")) @@ -298,70 +271,61 @@ def test_refresh_with_only_local_root(self): os.remove(os.path.join(self.client_directory, "1.root.json")) self._assert_files(["root"]) - self.repository_updater.refresh() + self.updater.refresh() self._assert_files(["root", "snapshot", "targets", "timestamp"]) # Get targetinfo for 'file3.txt' listed in the delegated role1 - targetinfo3 = self.repository_updater.get_one_valid_targetinfo( - "file3.txt" - ) + targetinfo3 = self.updater.get_targetinfo("file3.txt") expected_files = ["role1", "root", "snapshot", "targets", "timestamp"] self._assert_files(expected_files) def test_both_target_urls_not_set(self): # target_base_url = None and Updater._target_base_url = None - self.repository_updater = ngclient.Updater( - self.client_directory, self.metadata_url - ) + updater = ngclient.Updater(self.client_directory, self.metadata_url, self.dl_dir) + info = TargetFile(1, {"sha256": ""}, "targetpath") with self.assertRaises(ValueError): - self.repository_updater.download_target( - [], self.destination_directory - ) + updater.download_target(info) + + def test_no_target_dir_no_filepath(self): + # filepath = None and Updater.target_dir = None + updater = ngclient.Updater(self.client_directory, self.metadata_url) + info = TargetFile(1, {"sha256": ""}, "targetpath") + with self.assertRaises(ValueError): + updater.find_cached_target(info) + with self.assertRaises(ValueError): + updater.download_target(info) def test_external_targets_url(self): - self.repository_updater.refresh() - targetinfo = self.repository_updater.get_one_valid_targetinfo( - "file1.txt" - ) - self.repository_updater.download_target( - targetinfo, self.destination_directory, self.targets_url - ) + self.updater.refresh() + info = self.updater.get_targetinfo("file1.txt") + + self.updater.download_target(info, target_base_url=self.targets_url) def test_length_hash_mismatch(self): - self.repository_updater.refresh() - targetinfo = self.repository_updater.get_one_valid_targetinfo( - "file1.txt" - ) + self.updater.refresh() + targetinfo = self.updater.get_targetinfo("file1.txt") + length = targetinfo.length with self.assertRaises(exceptions.RepositoryError): targetinfo.length = 44 - self.repository_updater.download_target( - targetinfo, self.destination_directory - ) + self.updater.download_target(targetinfo) with self.assertRaises(exceptions.RepositoryError): targetinfo.length = length targetinfo.hashes = {"sha256": "abcd"} - self.repository_updater.download_target( - targetinfo, self.destination_directory - ) + self.updater.download_target(targetinfo) def test_updating_root(self): # Bump root version, resign and refresh self._modify_repository_root(lambda root: None, bump_version=True) - self.repository_updater.refresh() - self.assertEqual( - self.repository_updater._trusted_set.root.signed.version, 2 - ) + self.updater.refresh() + self.assertEqual(self.updater._trusted_set.root.signed.version, 2) def test_missing_targetinfo(self): - self.repository_updater.refresh() + self.updater.refresh() # Get targetinfo for non-existing file - targetinfo = self.repository_updater.get_one_valid_targetinfo( - "file33.txt" - ) - self.assertIsNone(targetinfo) + self.assertIsNone(self.updater.get_targetinfo("file33.txt")) if __name__ == "__main__": diff --git a/tests/test_updater_with_simulator.py b/tests/test_updater_with_simulator.py index 5ea3fc5f20..8cfb9de72a 100644 --- a/tests/test_updater_with_simulator.py +++ b/tests/test_updater_with_simulator.py @@ -54,6 +54,7 @@ def _run_refresh(self) -> Updater: updater = Updater( self.metadata_dir, "https://example.com/metadata/", + self.targets_dir, "https://example.com/targets/", self.sim ) @@ -90,9 +91,13 @@ def test_refresh(self): @utils.run_sub_tests_with_dataset(targets) def test_targets(self, test_case_data: Tuple[str, bytes, str]): targetpath, content, encoded_path = test_case_data - # target does not exist yet + path = os.path.join(self.targets_dir, encoded_path) + updater = self._run_refresh() - self.assertIsNone(updater.get_one_valid_targetinfo(targetpath)) + # target does not exist yet, configuration is what we expect + self.assertIsNone(updater.get_targetinfo(targetpath)) + self.assertTrue(self.sim.root.consistent_snapshot) + self.assertTrue(updater.config.prefix_targets_with_hash) # Add targets to repository self.sim.targets.version += 1 @@ -101,30 +106,25 @@ def test_targets(self, test_case_data: Tuple[str, bytes, str]): updater = self._run_refresh() # target now exists, is not in cache yet - file_info = updater.get_one_valid_targetinfo(targetpath) - self.assertIsNotNone(file_info) - self.assertEqual( - updater.updated_targets([file_info], self.targets_dir), - [file_info] - ) + info = updater.get_targetinfo(targetpath) + self.assertIsNotNone(info) + # Test without and with explicit local filepath + self.assertIsNone(updater.find_cached_target(info)) + self.assertIsNone(updater.find_cached_target(info, path)) - # Assert consistent_snapshot is True and downloaded targets have prefix. - self.assertTrue(self.sim.root.consistent_snapshot) - self.assertTrue(updater.config.prefix_targets_with_hash) # download target, assert it is in cache and content is correct - local_path = updater.download_target(file_info, self.targets_dir) - self.assertEqual( - updater.updated_targets([file_info], self.targets_dir), [] - ) - self.assertTrue(local_path.startswith(self.targets_dir)) - with open(local_path, "rb") as f: - self.assertEqual(f.read(), content) - - # Assert that the targetpath was URL encoded as expected. - encoded_absolute_path = os.path.join(self.targets_dir, encoded_path) - self.assertEqual(local_path, encoded_absolute_path) + self.assertEqual(path, updater.download_target(info)) + self.assertEqual(path, updater.find_cached_target(info)) + self.assertEqual(path, updater.find_cached_target(info, path)) + with open(path, "rb") as f: + self.assertEqual(f.read(), content) + # download using explicit filepath as well + os.remove(path) + self.assertEqual(path, updater.download_target(info, path)) + self.assertEqual(path, updater.find_cached_target(info)) + self.assertEqual(path, updater.find_cached_target(info, path)) def test_fishy_rolenames(self): roles_to_filenames = { @@ -145,7 +145,7 @@ def test_fishy_rolenames(self): updater = self._run_refresh() # trigger updater to fetch the delegated metadata, check filenames - updater.get_one_valid_targetinfo("anything") + updater.get_targetinfo("anything") local_metadata = os.listdir(self.metadata_dir) for fname in roles_to_filenames.values(): self.assertTrue(fname in local_metadata) diff --git a/tuf/ngclient/updater.py b/tuf/ngclient/updater.py index fdbcec4ed3..5857be13d4 100644 --- a/tuf/ngclient/updater.py +++ b/tuf/ngclient/updater.py @@ -21,11 +21,11 @@ snapshot is consistent so multiple targets can be downloaded without fear of repository content changing. For each target: - * :func:`~tuf.ngclient.updater.Updater.get_one_valid_targetinfo()` is + * :func:`~tuf.ngclient.updater.Updater.get_targetinfo()` is used to find information about a specific target. This will load new targets metadata as needed (from local cache or remote repository). - * :func:`~tuf.ngclient.updater.Updater.updated_targets()` can be used to - check if target files are already locally cached. + * :func:`~tuf.ngclient.updater.Updater.find_cached_target()` can be used + to check if a target file is already locally cached. * :func:`~tuf.ngclient.updater.Updater.download_target()` downloads a target file and ensures it is verified correct by the metadata. @@ -43,27 +43,30 @@ from tuf.ngclient import Updater - # Load trusted local root metadata from client metadata cache. Define the - # remote repository metadata URL prefix and target URL prefix. + # Load trusted local root metadata from client metadata cache. Define + # where metadata and targets will be downloaded from. updater = Updater( repository_dir="~/tufclient/metadata/", metadata_base_url="http://localhost:8000/tuf-repo/", + target_dir="~/tufclient/downloads/", target_base_url="http://localhost:8000/targets/", ) # Update top-level metadata from remote updater.refresh() - # Securely download a target: - # Update target metadata, then download and verify target - targetinfo = updater.get_one_valid_targetinfo("file.txt") - updater.download_target(targetinfo, "~/tufclient/downloads/") + # Update metadata, then download target if needed + info = updater.get_targetinfo("file.txt") + path = updater.find_cached_target(info) + if path is None: + path = updater.download_target(info) + print(f"Local file {path} contains target {info.path}") """ import logging import os import tempfile -from typing import List, Optional, Set, Tuple +from typing import Optional, Set, Tuple from urllib import parse from securesystemslib import util as sslib_util @@ -84,6 +87,9 @@ class Updater: repository_dir: Local metadata directory. Directory must be writable and it must contain a trusted root.json file. metadata_base_url: Base URL for all remote metadata downloads + target_dir: Local targets directory. Directory must be writable. It + will be used as the default target download directory by + ``find_cached_target()`` and ``download_target()`` 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 @@ -98,12 +104,14 @@ def __init__( self, repository_dir: str, metadata_base_url: str, + target_dir: Optional[str] = None, target_base_url: Optional[str] = None, fetcher: Optional[FetcherInterface] = None, config: Optional[UpdaterConfig] = None, ): self._dir = repository_dir self._metadata_base_url = _ensure_trailing_slash(metadata_base_url) + self.target_dir = target_dir if target_base_url is None: self._target_base_url = None else: @@ -123,7 +131,7 @@ def refresh(self) -> None: all the checks required in the TUF client workflow. The metadata for delegated roles are not refreshed by this method as - that happens on demand during get_one_valid_targetinfo(). + that happens on demand during get_targetinfo(). The refresh() method should be called by the client before any other method calls. @@ -139,19 +147,24 @@ def refresh(self) -> None: self._load_snapshot() self._load_targets("targets", "root") - def get_one_valid_targetinfo( - self, target_path: str - ) -> Optional[TargetFile]: + def _generate_target_file_path(self, targetinfo: TargetFile) -> str: + if self.target_dir is None: + raise ValueError("target_dir must be set if filepath is not given") + + # Use URL encoded target path as filename + filename = parse.quote(targetinfo.path, "") + return os.path.join(self.target_dir, filename) + + def get_targetinfo(self, target_path: str) -> Optional[TargetFile]: """Returns TargetFile instance with information for 'target_path'. The return value can be used as an argument to - :func:`download_target()` and :func:`updated_targets()`. - + :func:`download_target()` and :func:`find_cached_target()`. :func:`refresh()` must be called before calling - `get_one_valid_targetinfo()`. Subsequent calls to - `get_one_valid_targetinfo()` will use the same consistent repository + `get_targetinfo()`. Subsequent calls to + `get_targetinfo()` will use the same consistent repository state: Changes that happen in the repository between calling - :func:`refresh()` and `get_one_valid_targetinfo()` will not be + :func:`refresh()` and `get_targetinfo()` will not be seen by the updater. As a side-effect this method downloads all the additional (delegated @@ -173,59 +186,51 @@ def get_one_valid_targetinfo( """ return self._preorder_depth_first_walk(target_path) - @staticmethod - def updated_targets( - targets: List[TargetFile], destination_directory: str - ) -> List[TargetFile]: - """Checks whether local cached target files are up to date - - After retrieving the target information for the targets that should be - updated, updated_targets() can be called to determine which targets - have changed compared to locally stored versions. + def find_cached_target( + self, + targetinfo: TargetFile, + filepath: Optional[str] = None, + ) -> Optional[str]: + """Checks whether a local file is an up to date target - All the targets that are not up-to-date in destination_directory are - returned in a list. The list items can be downloaded with - 'download_target()'. - """ - # Keep track of TargetFiles and local paths. Return 'updated_targets' - # and use 'local_paths' to avoid duplicates. - updated_targets: List[TargetFile] = [] - local_paths: List[str] = [] + Args: + targetinfo: TargetFile from ``get_targetinfo()``. + filepath: Local path to file. If None, a file path is generated + based on ``target_dir`` constructor argument. - for target in targets: - # URL encode to get local filename like download_target() does - filename = parse.quote(target.path, "") - local_path = os.path.join(destination_directory, filename) + Raises: + ValueError: Incorrect arguments - if local_path in local_paths: - continue + Returns: + Local file path if the file is an up to date target file. + None if file is not found or it is not up to date. + """ - try: - with open(local_path, "rb") as target_file: - target.verify_length_and_hashes(target_file) - # If the file does not exist locally or length and hashes - # do not match, append to updated targets. - except (OSError, exceptions.LengthOrHashMismatchError): - updated_targets.append(target) - local_paths.append(local_path) + if filepath is None: + filepath = self._generate_target_file_path(targetinfo) - return updated_targets + try: + with open(filepath, "rb") as target_file: + targetinfo.verify_length_and_hashes(target_file) + return filepath + except (OSError, exceptions.LengthOrHashMismatchError): + return None def download_target( self, targetinfo: TargetFile, - destination_directory: str, + filepath: Optional[str] = None, target_base_url: Optional[str] = None, ) -> str: - """Downloads the target file specified by 'targetinfo'. + """Downloads the target file specified by ``targetinfo``. Args: - targetinfo: TargetFile instance received from - get_one_valid_targetinfo() or updated_targets(). - destination_directory: existing local directory to download into. - Note that new directories may be created inside - destination_directory as required. - target_base_url: Optional; Base URL used to form the final target + targetinfo: TargetFile from ``get_targetinfo()``. + filepath: Local path to download into. If None, the file is + downloaded into directory defined by ``target_dir`` constructor + argument using a generated filename. If file already exists, + it is overwritten. + target_base_url: Base URL used to form the final target download URL. Default is the value provided in Updater() Raises: @@ -234,9 +239,12 @@ def download_target( TODO: file write errors Returns: - Path to downloaded file + Local path to downloaded file """ + if filepath is None: + filepath = self._generate_target_file_path(targetinfo) + if target_base_url is None: if self._target_base_url is None: raise ValueError( @@ -266,12 +274,10 @@ def download_target( f"{target_filepath} length or hashes do not match" ) from e - # Use a URL encoded targetpath as the local filename - filename = parse.quote(targetinfo.path, "") - local_filepath = os.path.join(destination_directory, filename) - sslib_util.persist_temp_file(target_file, local_filepath) + sslib_util.persist_temp_file(target_file, filepath) - return local_filepath + logger.info("Downloaded target %s", targetinfo.path) + return filepath def _download_metadata( self, rolename: str, length: int, version: Optional[int] = None