diff --git a/scriptworker/artifacts.py b/scriptworker/artifacts.py index 55490848..dd328fd3 100644 --- a/scriptworker/artifacts.py +++ b/scriptworker/artifacts.py @@ -17,7 +17,7 @@ from scriptworker.client import validate_artifact_url from scriptworker.exceptions import ScriptWorkerRetryException, ScriptWorkerTaskException from scriptworker.task import get_task_id, get_run_id, get_decision_task_id -from scriptworker.utils import download_file, filepaths_in_dir, raise_future_exceptions, retry_async +from scriptworker.utils import download_file, filepaths_in_dir, raise_future_exceptions, retry_async, add_enumerable_item_to_dict log = logging.getLogger(__name__) @@ -325,24 +325,43 @@ def get_upstream_artifacts_full_paths_per_task_id(context): context (scriptworker.context.Context): the scriptworker context. Returns: - dict: lists of the paths to existing upstream artifacts, sorted by task_id + dict, dict: lists of the paths to upstream artifacts, sorted by task_id. + First dict represents the existing upstream artifacts. The second one + maps the optional artifacts that couldn't be downloaded Raises: scriptworker.exceptions.ScriptWorkerTaskException: when an artifact doesn't exist. """ + upstream_artifacts = context.task['payload']['upstreamArtifacts'] task_ids_and_relative_paths = [ (artifact_definition['taskId'], artifact_definition['paths']) - for artifact_definition in context.task['payload']['upstreamArtifacts'] + for artifact_definition in upstream_artifacts ] - return { - task_id: [ - get_and_check_single_upstream_artifact_full_path(context, task_id, path) - for path in paths - ] - for task_id, paths in task_ids_and_relative_paths - } + optional_artifacts_per_task_id = get_optional_artifacts_per_task_id(upstream_artifacts) + + upstream_artifacts_full_paths_per_task_id = {} + failed_paths_per_task_id = {} + for task_id, paths in task_ids_and_relative_paths: + for path in paths: + try: + path_to_add = get_and_check_single_upstream_artifact_full_path(context, task_id, path) + add_enumerable_item_to_dict( + dict_=upstream_artifacts_full_paths_per_task_id, + key=task_id, item=path_to_add + ) + except ScriptWorkerTaskException: + if path in optional_artifacts_per_task_id.get(task_id, []): + log.warn('Optional artifact "{}" of task "{}" not found'.format(path, task_id)) + add_enumerable_item_to_dict( + dict_=failed_paths_per_task_id, + key=task_id, item=path + ) + else: + raise + + return upstream_artifacts_full_paths_per_task_id, failed_paths_per_task_id def get_and_check_single_upstream_artifact_full_path(context, task_id, path): @@ -387,3 +406,30 @@ def get_single_upstream_artifact_full_path(context, task_id, path): """ return os.path.abspath(os.path.join(context.config['work_dir'], 'cot', task_id, path)) + + +def get_optional_artifacts_per_task_id(upstream_artifacts): + """Return every optional artifact defined in ``upstream_artifacts``, ordered by taskId. + + Args: + upstream_artifacts: the list of upstream artifact definitions + + Returns: + dict: list of paths to downloaded artifacts ordered by taskId + + """ + # A given taskId might be defined many times in upstreamArtifacts. Thus, we can't + # use a dict comprehension + optional_artifacts_per_task_id = {} + + for artifact_definition in upstream_artifacts: + if artifact_definition.get('optional', False) is True: + task_id = artifact_definition['taskId'] + artifacts_paths = artifact_definition['paths'] + + add_enumerable_item_to_dict( + dict_=optional_artifacts_per_task_id, + key=task_id, item=artifacts_paths + ) + + return optional_artifacts_per_task_id diff --git a/scriptworker/cot/verify.py b/scriptworker/cot/verify.py index 96b18bbd..3c8d2830 100644 --- a/scriptworker/cot/verify.py +++ b/scriptworker/cot/verify.py @@ -23,7 +23,8 @@ import sys import tempfile from urllib.parse import unquote, urlparse -from scriptworker.artifacts import download_artifacts, get_artifact_url, get_single_upstream_artifact_full_path +from scriptworker.artifacts import download_artifacts, get_artifact_url, get_single_upstream_artifact_full_path, \ + get_optional_artifacts_per_task_id from scriptworker.config import read_worker_creds from scriptworker.constants import DEFAULT_CONFIG from scriptworker.context import Context @@ -31,7 +32,8 @@ from scriptworker.gpg import get_body, GPG from scriptworker.log import contextual_log_handler from scriptworker.task import get_decision_task_id, get_parent_task_id, get_worker_type, get_task_id -from scriptworker.utils import format_json, get_hash, load_json, makedirs, match_url_regex, raise_future_exceptions, rm +from scriptworker.utils import format_json, get_hash, load_json, makedirs, match_url_regex, raise_future_exceptions, rm, \ + get_results_and_future_exceptions, add_enumerable_item_to_dict from taskcluster.exceptions import TaskclusterFailure log = logging.getLogger(__name__) @@ -504,6 +506,12 @@ def verify_docker_image_sha(chain, link): cot = link.cot task = link.task errors = [] + + if not cot: + log.warn('Chain of Trust for {} does not exist. See above log for more details. \ +Skipping docker image sha verification'.format(task['taskId'])) + return + if isinstance(task['payload'].get('image'), dict): # Using pre-built image from docker-image task docker_image_task_id = task['extra']['chainOfTrust']['inputs']['docker-image'] @@ -655,7 +663,8 @@ async def download_cot(chain): DownloadError: on failure. """ - async_tasks = [] + mandatory_artifact_tasks = [] + optional_artifact_tasks = [] # only deal with chain.links, which are previously finished tasks with # signed chain of trust artifacts. ``chain.task`` is the current running # task, and will not have a signed chain of trust artifact yet. @@ -663,15 +672,28 @@ async def download_cot(chain): task_id = link.task_id url = get_artifact_url(chain.context, task_id, 'public/chainOfTrust.json.asc') parent_dir = link.cot_dir - async_tasks.append( - asyncio.ensure_future( - download_artifacts( - chain.context, [url], parent_dir=parent_dir, - valid_artifact_task_ids=[task_id] - ) + coroutine = asyncio.ensure_future( + download_artifacts( + chain.context, [url], parent_dir=parent_dir, + valid_artifact_task_ids=[task_id] ) ) - paths = await raise_future_exceptions(async_tasks) + + if is_task_required_by_any_mandatory_artifact(chain, task_id): + mandatory_artifact_tasks.append(coroutine) + else: + optional_artifact_tasks.append(coroutine) + + mandatory_artifacts_paths = await raise_future_exceptions(mandatory_artifact_tasks) + succeeded_optional_artifacts_paths, failed_optional_artifacts = \ + await get_results_and_future_exceptions(optional_artifact_tasks) + + if failed_optional_artifacts: + error_messages = '\n'.join([' * {}'.format(failure) for failure in failed_optional_artifacts]) + log.warn('Could not download {} "chainOfTrust.json.asc". Although, they were not needed by \ +any mandatory artifact. Continuing CoT verifications. Errors gotten: {}'.format(len(failed_optional_artifacts), error_messages)) + + paths = mandatory_artifacts_paths + succeeded_optional_artifacts_paths for path in paths: sha = get_hash(path[0]) log.debug("{} downloaded; hash is {}".format(path[0], sha)) @@ -695,6 +717,11 @@ async def download_cot_artifact(chain, task_id, path): """ link = chain.get_link(task_id) log.debug("Verifying {} is in {} cot artifacts...".format(path, task_id)) + if not link.cot: + log.warn('Chain of Trust for "{}" in {} does not exist. See above log for more details. \ +Skipping download of this artifact'.format(path, task_id)) + return + if path not in link.cot['artifacts']: raise CoTError("path {} not in {} {} chain of trust artifacts!".format(path, link.name, link.task_id)) url = get_artifact_url(chain.context, task_id, path) @@ -714,64 +741,109 @@ async def download_cot_artifact(chain, task_id, path): # download_cot_artifacts {{{1 -async def download_cot_artifacts(chain, artifact_dict): - """Call ``download_cot_artifact`` in parallel for each key/value in ``artifact_dict``. +async def download_cot_artifacts(chain): + """Call ``download_cot_artifact`` in parallel for each "upstreamArtifacts". + + Optional artifacts are allowed to not be downloaded. Args: chain (ChainOfTrust): the chain of trust object - artifact_dict (dict): maps task_id to list of paths of artifacts to download Returns: - list: list of full paths to artifacts + list: list of full paths to downloaded artifacts. Failed optional artifacts + aren't returned Raises: - CoTError: on chain of trust sha validation error - DownloadError: on download error + CoTError: on chain of trust sha validation error, on a mandatory artifact + DownloadError: on download error on a mandatory artifact """ - tasks = [] - for task_id, paths in artifact_dict.items(): + upstream_artifacts = chain.task['payload'].get('upstreamArtifacts', []) + all_artifacts_per_task_id = get_all_artifacts_per_task_id(chain, upstream_artifacts) + + mandatory_artifact_tasks = [] + optional_artifact_tasks = [] + for task_id, paths in all_artifacts_per_task_id.items(): for path in paths: - tasks.append( - asyncio.ensure_future( - download_cot_artifact( - chain, task_id, path - ) - ) - ) - full_paths = await raise_future_exceptions(tasks) - return full_paths + coroutine = asyncio.ensure_future(download_cot_artifact(chain, task_id, path)) + + if is_artifact_optional(chain, task_id, path): + optional_artifact_tasks.append(coroutine) + else: + mandatory_artifact_tasks.append(coroutine) + + mandatory_artifacts_paths = await raise_future_exceptions(mandatory_artifact_tasks) + succeeded_optional_artifacts_paths, failed_optional_artifacts = \ + await get_results_and_future_exceptions(optional_artifact_tasks) + if failed_optional_artifacts: + log.warn('Could not download {} artifacts: {}'.format(len(failed_optional_artifacts), failed_optional_artifacts)) -# download_firefox_cot_artifacts {{{1 -async def download_firefox_cot_artifacts(chain): - """Download artifacts needed for firefox chain of trust verification. + return mandatory_artifacts_paths + succeeded_optional_artifacts_paths - This is only task-graph.json so far. + +def is_task_required_by_any_mandatory_artifact(chain, task_id): + """Tells whether a task has only optional artifacts or not. Args: chain (ChainOfTrust): the chain of trust object + task_id (str): the id of the aforementioned task Returns: - list: list of full paths to artifacts + bool: True if task has one or several mandatory artifacts + + """ + upstream_artifacts = chain.task['payload'].get('upstreamArtifacts', []) + all_artifacts_per_task_id = get_all_artifacts_per_task_id(chain, upstream_artifacts) + optional_artifacts_per_task_id = get_optional_artifacts_per_task_id(upstream_artifacts) + + all_artifacts = all_artifacts_per_task_id.get(task_id, []) + optional_artifacts = optional_artifacts_per_task_id.get(task_id, []) + mandatory_artifacts = set(all_artifacts) - set(optional_artifacts) + return bool(mandatory_artifacts) - Raises: - CoTError: on chain of trust sha validation error - DownloadError: on download error + +def is_artifact_optional(chain, task_id, path): + """Tells whether an artifact is flagged as optional or not. + + Args: + chain (ChainOfTrust): the chain of trust object + task_id (str): the id of the aforementioned task + + Returns: + bool: True if artifact is optional + + """ + upstream_artifacts = chain.task['payload'].get('upstreamArtifacts', []) + optional_artifacts_per_task_id = get_optional_artifacts_per_task_id(upstream_artifacts) + return path in optional_artifacts_per_task_id.get(task_id, []) + + +def get_all_artifacts_per_task_id(chain, upstream_artifacts): + """Return every artifact to download, including the Chain Of Trust Artifacts. + + Args: + chain (ChainOfTrust): the chain of trust object + upstream_artifacts: the list of upstream artifact definitions + + Returns: + dict: list of paths to downloaded artifacts ordered by taskId """ - artifact_dict = {} + all_artifacts_per_task_id = {} for link in chain.links: - task_type = link.task_type - if task_type in PARENT_TASK_TYPES: - artifact_dict.setdefault(link.task_id, []) - artifact_dict[link.task_id].append('public/task-graph.json') - if 'upstreamArtifacts' in chain.task['payload']: - for upstream_dict in chain.task['payload']['upstreamArtifacts']: - artifact_dict.setdefault(upstream_dict['taskId'], []) - for path in upstream_dict['paths']: - artifact_dict[upstream_dict['taskId']].append(path) - return await download_cot_artifacts(chain, artifact_dict) + if link.task_type in PARENT_TASK_TYPES: + add_enumerable_item_to_dict( + dict_=all_artifacts_per_task_id, key=link.task_id, item='public/task-graph.json' + ) + + if upstream_artifacts: + for upstream_dict in upstream_artifacts: + add_enumerable_item_to_dict( + dict_=all_artifacts_per_task_id, key=upstream_dict['taskId'], item=upstream_dict['paths'] + ) + + return all_artifacts_per_task_id # verify_cot_signatures {{{1 @@ -798,7 +870,11 @@ def verify_cot_signatures(chain): with open(path, "r") as fh: contents = fh.read() except OSError as exc: - raise CoTError("Can't read {}: {}!".format(path, str(exc))) + if is_task_required_by_any_mandatory_artifact(chain, link.task_id): + raise CoTError("Can't read mandatory {}: {}!".format(path, str(exc))) + else: + log.warn("Could not read optional {}. Continuing. Error gotten: {}!".format(path, str(exc))) + continue try: body = get_body( gpg, contents, @@ -1484,7 +1560,7 @@ async def verify_chain_of_trust(chain): # verify the signatures and populate the ``link.cot``s verify_cot_signatures(chain) # download all other artifacts needed to verify chain of trust - await download_firefox_cot_artifacts(chain) + await download_cot_artifacts(chain) # verify the task types, e.g. decision task_count = await verify_task_types(chain) check_num_tasks(chain, task_count) diff --git a/scriptworker/test/test_artifacts.py b/scriptworker/test/test_artifacts.py index 1a2ff1ac..63b0bd3f 100644 --- a/scriptworker/test/test_artifacts.py +++ b/scriptworker/test/test_artifacts.py @@ -11,7 +11,8 @@ from scriptworker.artifacts import get_expiration_arrow, guess_content_type_and_encoding, upload_artifacts, \ create_artifact, get_artifact_url, download_artifacts, compress_artifact_if_supported, \ _force_mimetypes_to_plain_text, _craft_artifact_put_headers, get_upstream_artifacts_full_paths_per_task_id, \ - get_and_check_single_upstream_artifact_full_path, get_single_upstream_artifact_full_path + get_and_check_single_upstream_artifact_full_path, get_single_upstream_artifact_full_path, \ + get_optional_artifacts_per_task_id from scriptworker.exceptions import ScriptWorkerRetryException, ScriptWorkerTaskException @@ -210,27 +211,58 @@ async def foo(_, url, path, **kwargs): def test_get_upstream_artifacts_full_paths_per_task_id(context): + artifacts_to_succeed = [{ + 'paths': ['public/file_a'], + 'taskId': 'dependency1', + 'taskType': 'signing', + }, { + 'paths': ['public/file_b'], + 'taskId': 'dependency2', + 'taskType': 'signing', + }] + context.task['payload'] = { 'upstreamArtifacts': [{ - 'paths': ['public/file_a'], - 'taskId': 'dependency1', + 'paths': ['public/failed_optional_file1'], + 'taskId': 'failedDependency1', 'taskType': 'signing', + 'optional': True }, { - 'paths': ['public/file_b'], - 'taskId': 'dependency2', + 'paths': ['public/failed_optional_file2', 'public/failed_optional_file3'], + 'taskId': 'failedDependency2', 'taskType': 'signing', + 'optional': True }], } - for artifact in context.task['payload']['upstreamArtifacts']: + context.task['payload']['upstreamArtifacts'].extend(artifacts_to_succeed) + for artifact in artifacts_to_succeed: folder = os.path.join(context.config['work_dir'], 'cot', artifact['taskId']) os.makedirs(os.path.join(folder, 'public')) touch(os.path.join(folder, artifact['paths'][0])) - assert get_upstream_artifacts_full_paths_per_task_id(context) == { + succeeded_artifacts, failed_artifacts = get_upstream_artifacts_full_paths_per_task_id(context) + + assert succeeded_artifacts == { 'dependency1': [os.path.join(context.config['work_dir'], 'cot', 'dependency1', 'public', 'file_a')], 'dependency2': [os.path.join(context.config['work_dir'], 'cot', 'dependency2', 'public', 'file_b')], } + assert failed_artifacts == { + 'failedDependency1': ['public/failed_optional_file1'], + 'failedDependency2': ['public/failed_optional_file2', 'public/failed_optional_file3'], + } + + +def test_fail_get_upstream_artifacts_full_paths_per_task_id(context): + context.task['payload'] = { + 'upstreamArtifacts': [{ + 'paths': ['public/failed_mandatory_file'], + 'taskId': 'failedDependency', + 'taskType': 'signing', + }] + } + with pytest.raises(ScriptWorkerTaskException): + get_upstream_artifacts_full_paths_per_task_id(context) def test_get_and_check_single_upstream_artifact_full_path(context): @@ -259,3 +291,32 @@ def test_get_single_upstream_artifact_full_path(context): assert get_single_upstream_artifact_full_path(context, 'non-existing-dep', 'public/file_a') == \ os.path.join(context.config['work_dir'], 'cot', 'non-existing-dep', 'public', 'file_a') + + +@pytest.mark.parametrize('upstream_artifacts, expected', (( + [{}], {}, +), ( + [{'taskId': 'someTaskId', 'paths': ['mandatory_artifact_1']}], {}, +), ( + [{'taskId': 'someTaskId', 'paths': ['optional_artifact_1'], 'optional': True}], + {'someTaskId': ['optional_artifact_1']}, +), ( + [{'taskId': 'someTaskId', 'paths': ['optional_artifact_1', 'optional_artifact_2'], 'optional': True}], + {'someTaskId': ['optional_artifact_1', 'optional_artifact_2']}, +), ( + [ + {'taskId': 'someTaskId', 'paths': ['optional_artifact_1'], 'optional': True}, + {'taskId': 'someOtherTaskId', 'paths': ['optional_artifact_2'], 'optional': True}, + {'taskId': 'anotherOtherTaskId', 'paths': ['mandatory_artifact_1']}, + ], + {'someTaskId': ['optional_artifact_1'], 'someOtherTaskId': ['optional_artifact_2']}, +), ( + [ + {'taskId': 'taskIdGivenThreeTimes', 'paths': ['optional_artifact_1'], 'optional': True}, + {'taskId': 'taskIdGivenThreeTimes', 'paths': ['mandatory_artifact_1']}, + {'taskId': 'taskIdGivenThreeTimes', 'paths': ['optional_artifact_2'], 'optional': True}, + ], + {'taskIdGivenThreeTimes': ['optional_artifact_1', 'optional_artifact_2']}, +))) +def test_get_optional_artifacts_per_task_id(upstream_artifacts, expected): + assert get_optional_artifacts_per_task_id(upstream_artifacts) == expected diff --git a/scriptworker/test/test_cot_verify.py b/scriptworker/test/test_cot_verify.py index e80ef9d5..ba7375c1 100644 --- a/scriptworker/test/test_cot_verify.py +++ b/scriptworker/test/test_cot_verify.py @@ -13,7 +13,7 @@ import tempfile from taskcluster.exceptions import TaskclusterFailure import scriptworker.cot.verify as cotverify -from scriptworker.exceptions import CoTError, ScriptWorkerGPGException +from scriptworker.exceptions import CoTError, ScriptWorkerGPGException, DownloadError from scriptworker.utils import makedirs from . import noop_async, noop_sync, rw_context, tmpdir, touch @@ -125,6 +125,7 @@ def decision_link(chain): 'taskGroupId': 'decision_task_id', 'schedulerId': 'scheduler_id', 'provisionerId': 'provisioner_id', + 'taskId': 'decision_task_id', 'workerType': 'workerType', 'scopes': [], 'metadata': { @@ -506,6 +507,13 @@ def test_verify_docker_image_sha_bad_allowlist(chain, build_link, decision_link, cotverify.verify_docker_image_sha(chain, decision_link) +def test_verify_docker_image_sha_no_downloaded_cot(chain, build_link, decision_link, docker_image_link): + decision_link._cot = None + chain.links = [build_link, decision_link, docker_image_link] + # Non-downloaded CoT may happen on non-exiting optional artifacts + cotverify.verify_docker_image_sha(chain, decision_link) + + # find_sorted_task_dependencies{{{1 @pytest.mark.parametrize("task,expected,task_type", (( # Make sure we don't follow other_task_id on a decision task @@ -627,26 +635,51 @@ def fake_find(task, name, _): # download_cot {{{1 -@pytest.mark.parametrize("raises", (True, False)) +@pytest.mark.parametrize('upstream_artifacts, raises, download_artifacts_mock', (( + [{'taskId': 'task_id', 'paths': ['path1', 'path2']}], + True, + die_async, +), ( + [{'taskId': 'task_id', 'paths': ['failed_path'], 'optional': True}], + False, + die_async, +), ( + [{'taskId': 'task_id', 'paths': ['path1', 'path2']}, + {'taskId': 'task_id', 'paths': ['failed_path'], 'optional': True}], + True, + die_async, +), ( + [{'taskId': 'task_id', 'paths': ['path1', 'path2']},], + False, + None, +), ( + [], + False, + None, +))) @pytest.mark.asyncio -async def test_download_cot(chain, mocker, raises, event_loop): +async def test_download_cot(chain, mocker, event_loop, upstream_artifacts, raises, download_artifacts_mock): async def down(*args, **kwargs): return ['x'] + if download_artifacts_mock is None: + download_artifacts_mock = down + def sha(*args, **kwargs): return "sha" m = mock.MagicMock() - m.task_id = "x" + m.task_id = "task_id" m.cot_dir = "y" chain.links = [m] + chain.task['payload']['upstreamArtifacts'] = upstream_artifacts mocker.patch.object(cotverify, 'get_artifact_url', new=noop_sync) if raises: - mocker.patch.object(cotverify, 'download_artifacts', new=die_async) + mocker.patch.object(cotverify, 'download_artifacts', new=download_artifacts_mock) with pytest.raises(CoTError): await cotverify.download_cot(chain) else: - mocker.patch.object(cotverify, 'download_artifacts', new=down) + mocker.patch.object(cotverify, 'download_artifacts', new=download_artifacts_mock) mocker.patch.object(cotverify, 'get_hash', new=sha) await cotverify.download_cot(chain) @@ -693,26 +726,130 @@ def fake_get_hash(*args, **kwargs): await cotverify.download_cot_artifact(chain, 'task_id', path) +@pytest.mark.asyncio +async def test_download_cot_artifact_no_downloaded_cot(chain, mocker, event_loop): + link = mock.MagicMock() + link.task_id = 'task_id' + link.cot = None + chain.links = [link] + await cotverify.download_cot_artifact(chain, 'task_id', 'path') + + # download_cot_artifacts {{{1 @pytest.mark.parametrize("raises", (True, False)) @pytest.mark.asyncio async def test_download_cot_artifacts(chain, raises, mocker, event_loop): async def fake_download(x, y, path): + if path == 'failed_path': + raise DownloadError('') return path - artifact_dict = {'task_id': ['path1', 'path2']} + chain.task['payload']['upstreamArtifacts'] = [ + {'taskId': 'task_id', 'paths': ['path1', 'path2']}, + {'taskId': 'task_id', 'paths': ['failed_path'], 'optional': True}, + ] + artifact_dict = { + 'task_id': ['path1', 'path2', 'failed_path'], + } if raises: mocker.patch.object(cotverify, 'download_cot_artifact', new=die_async) with pytest.raises(CoTError): - await cotverify.download_cot_artifacts(chain, artifact_dict) + await cotverify.download_cot_artifacts(chain) else: mocker.patch.object(cotverify, 'download_cot_artifact', new=fake_download) - result = await cotverify.download_cot_artifacts(chain, artifact_dict) + result = await cotverify.download_cot_artifacts(chain) assert sorted(result) == ['path1', 'path2'] -# download_firefox_cot_artifacts {{{1 +@pytest.mark.parametrize('upstream_artifacts, task_id, expected', (( + [{ + 'taskId': 'id1', + 'paths': ['id1_path1', 'id1_path2'], + }, { + 'taskId': 'id2', + 'paths': ['id2_path1', 'id2_path2'], + }], + 'id1', + True +), ( + [{ + 'taskId': 'id1', + 'paths': ['id1_path1', 'id1_path2'], + 'optional': True, + }, { + 'taskId': 'id2', + 'paths': ['id2_path1', 'id2_path2'], + }], + 'id1', + False +), ( + [{ + 'taskId': 'id1', + 'paths': ['id1_path1'], + 'optional': True, + }, { + 'taskId': 'id2', + 'paths': ['id2_path1', 'id2_path2'], + }, { + 'taskId': 'id1', + 'paths': ['id1_path2'], + }], + 'id1', + True, +))) +def test_is_task_required_by_any_mandatory_artifact(chain, upstream_artifacts, task_id, expected): + chain.task['payload']['upstreamArtifacts'] = upstream_artifacts + assert cotverify.is_task_required_by_any_mandatory_artifact(chain, task_id) == expected + + +@pytest.mark.parametrize('upstream_artifacts, task_id, path, expected', (( + [{ + 'taskId': 'id1', + 'paths': ['id1_path1', 'id1_path2'], + }], + 'id1', + 'id1_path1', + False, +), ( + [{ + 'taskId': 'id1', + 'paths': ['id1_path1', 'id1_path2'], + 'optional': True, + }], + 'id1', + 'id1_path1', + True +), ( + [{ + 'taskId': 'id1', + 'paths': ['id1_path1'], + 'optional': True, + }, { + 'taskId': 'id1', + 'paths': ['id1_path2'], + }], + 'id1', + 'id1_path1', + True, +), ( + [{ + 'taskId': 'id1', + 'paths': ['id1_path1'], + 'optional': True, + }, { + 'taskId': 'id1', + 'paths': ['id1_path2'], + }], + 'id1', + 'id1_path2', + False, +))) +def test_is_artifact_optional(chain, upstream_artifacts, task_id, path, expected): + chain.task['payload']['upstreamArtifacts'] = upstream_artifacts + assert cotverify.is_artifact_optional(chain, task_id, path) == expected + + @pytest.mark.parametrize("upstream_artifacts,expected", (( None, {'decision_task_id': ['public/task-graph.json']} ), ( @@ -730,25 +867,37 @@ async def fake_download(x, y, path): } ))) @pytest.mark.asyncio -async def test_download_firefox_cot_artifacts(chain, decision_link, build_link, - upstream_artifacts, expected, - docker_image_link, mocker, event_loop): - - async def fake_download(_, result): - return result +async def test_get_all_artifacts_per_task_id(chain, decision_link, build_link, + upstream_artifacts, expected, + docker_image_link, mocker, event_loop): chain.links = [decision_link, build_link, docker_image_link] - if upstream_artifacts is not None: - chain.task['payload']['upstreamArtifacts'] = upstream_artifacts - mocker.patch.object(cotverify, 'download_cot_artifacts', new=fake_download) - assert expected == await cotverify.download_firefox_cot_artifacts(chain) + assert expected == cotverify.get_all_artifacts_per_task_id(chain, upstream_artifacts) -# verify_cot_signatures {{{1 -def test_verify_cot_signatures_no_file(chain, build_link, mocker): +@pytest.mark.parametrize('upstream_artifacts, raises', (( + [{'taskId': 'build_task_id', 'paths': ['path1', 'path2']}], + True, +), ( + [{'taskId': 'build_task_id', 'paths': ['failed_path'], 'optional': True}], + False, +), ( + [{'taskId': 'build_task_id', 'paths': ['path1', 'path2']}, + {'taskId': 'build_task_id', 'paths': ['failed_path'], 'optional': True}], + True, +), ( + [], + False, +))) +def test_verify_cot_signatures_no_file(chain, build_link, mocker, upstream_artifacts, raises): chain.links = [build_link] mocker.patch.object(cotverify, 'GPG', new=noop_sync) - with pytest.raises(CoTError): + + chain.task['payload']['upstreamArtifacts'] = upstream_artifacts + if raises: + with pytest.raises(CoTError): + cotverify.verify_cot_signatures(chain) + else: cotverify.verify_cot_signatures(chain) @@ -1231,7 +1380,7 @@ async def maybe_die(*args): if exc is not None: raise exc("blah") - for func in ('build_task_dependencies', 'download_cot', 'download_firefox_cot_artifacts', + for func in ('build_task_dependencies', 'download_cot', 'download_cot_artifacts', 'verify_task_types', 'verify_worker_impls'): mocker.patch.object(cotverify, func, new=noop_async) for func in ('verify_cot_signatures', 'check_num_tasks'): diff --git a/scriptworker/test/test_utils.py b/scriptworker/test/test_utils.py index 6dca1a9e..2776cfb1 100644 --- a/scriptworker/test/test_utils.py +++ b/scriptworker/test/test_utils.py @@ -238,6 +238,29 @@ def test_raise_future_exceptions_noop(event_loop): ) +@pytest.mark.asyncio +@pytest.mark.parametrize("exc", (IOError, SyntaxError, None)) +async def test_get_results_and_future_exceptions(exc): + async def one(): + if exc is None: + return 'passed1' + else: + raise exc('failed') + + async def two(): + return 'passed2' + + tasks = [asyncio.ensure_future(one()), asyncio.ensure_future(two())] + + passed_results, error_results = await utils.get_results_and_future_exceptions(tasks) + if exc is None: + assert passed_results == ['passed1', 'passed2'] + assert error_results == [] + else: + assert passed_results == ['passed2'] + assert [str(error) for error in error_results] == [str(exc('failed'))] + + # filepaths_in_dir {{{1 def test_filepaths_in_dir(tmpdir): filepaths = sorted([ @@ -384,3 +407,21 @@ def cb(m): assert utils.match_url_regex(rules, "https://hg.mozilla.org/mozilla-central", cb) == "mozilla-central" assert utils.match_url_regex((), "https://hg.mozilla.org/mozilla-central", cb) is None + + +@pytest.mark.parametrize("dict_, key, item, expected", (( + {}, 'non_existing_key', 'an_item', {'non_existing_key': ['an_item']} +), ( + {}, 'non_existing_key', ['a', 'list'], {'non_existing_key': ['a', 'list']} +), ( + {}, 'non_existing_key', ('a', 'tuple', 'to', 'become', 'a', 'list'), {'non_existing_key': ['a', 'tuple', 'to', 'become', 'a', 'list']} +), ( + {'existing_key': []}, 'existing_key', 'an_item', {'existing_key': ['an_item']} +), ( + {'existing_key': ['an_item']}, 'existing_key', 'a_second_item', {'existing_key': ['an_item', 'a_second_item']} +), ( + {'existing_key': ['an_item']}, 'existing_key', ['some', 'new', 'items'], {'existing_key': ['an_item', 'some', 'new', 'items']} +))) +def test_add_enumerable_item_to_dict(dict_, key, item, expected): + utils.add_enumerable_item_to_dict(dict_, key, item) + assert dict_ == expected diff --git a/scriptworker/utils.py b/scriptworker/utils.py index fabf0f15..a86acbfc 100644 --- a/scriptworker/utils.py +++ b/scriptworker/utils.py @@ -309,23 +309,54 @@ async def raise_future_exceptions(tasks): tasks (list): the list of futures to await and check for exceptions. Returns: - list: the list of result()s from the futures. + list: the list of results from the futures. Raises: Exception: any exceptions in task.exception(), or CancelledError if the task was cancelled """ - if not tasks: - return - result = [] - await asyncio.wait(tasks) - for task in tasks: - exc = task.exception() - if exc is not None: - raise exc - result.append(task.result()) - return result + succeeded_results, _ = await _process_future_exceptions(tasks, raise_at_first_error=True) + return succeeded_results + + +async def get_results_and_future_exceptions(tasks): + """Given a list of futures, await them, then return results and exceptions. + + This is similar to raise_future_exceptions, except that it doesn't raise any + exception. They are returned instead. This allows some tasks to optionally fail. + Please consider that no exception will be raised when calling this function. + You must verify the content of the second item of the tuple. It contains all + exceptions raised by the futures. + + Args: + tasks (list): the list of futures to await and check for exceptions. + + Returns: + tuple: the list of results from the futures, then the list of exceptions. + + """ + return await _process_future_exceptions(tasks, raise_at_first_error=False) + + +async def _process_future_exceptions(tasks, raise_at_first_error): + succeeded_results = [] + error_results = [] + + if tasks: + await asyncio.wait(tasks) + for task in tasks: + exc = task.exception() + if exc is None: + succeeded_results.append(task.result()) + else: + if raise_at_first_error: + raise exc + else: + log.warn('Async task failed with error: {}'.format(exc)) + error_results.append(exc) + + return succeeded_results, error_results # filepaths_in_dir {{{1 @@ -490,3 +521,31 @@ def match_url_regex(rules, url, callback): result = callback(m) if result is not None: return result + + +def add_enumerable_item_to_dict(dict_, key, item): + """Add an item to a list contained in a dict. + + For example: If the dict is ``{'some_key': ['an_item']}``, then calling this function + will alter the dict to ``{'some_key': ['an_item', 'another_item']}``. + + If the key doesn't exist yet, the function initializes it with a list containing the + item. + + List-like items are allowed. In this case, the existing list will be extended. + + Args: + dict_ (dict): the dict to modify + key (str): the key to add the item to + item (whatever): The item to add to the list associated to the key + """ + if isinstance(item, (list, tuple)): + try: + dict_[key].extend(item) + except KeyError: + dict_[key] = list(item) + else: + try: + dict_[key].append(item) + except KeyError: + dict_[key] = [item]