From 94d9f31bb19293c307be7c237e8753fb1a747fcd Mon Sep 17 00:00:00 2001 From: Lukas Holecek Date: Apr 19 2021 10:03:03 +0000 Subject: List remote rule file URLs in decision response Gets rid of duplicates early by verifying only unique rules. Fixes #605 JIRA: RHELWF-2263 --- diff --git a/functional-tests/consumers/test_resultsdb.py b/functional-tests/consumers/test_resultsdb.py index edab36d..b42688c 100644 --- a/functional-tests/consumers/test_resultsdb.py +++ b/functional-tests/consumers/test_resultsdb.py @@ -59,6 +59,7 @@ def test_consume_new_result( 'subject_identifier': nvr, 'result_id': result['id'], 'testcase': 'dist.rpmdeplint', + 'source': None, 'type': 'test-result-passed', }, ], @@ -70,6 +71,7 @@ def test_consume_new_result( 'subject_identifier': nvr, 'type': 'test-result-missing', 'scenario': None, + 'source': None, }, { 'testcase': 'dist.upgradepath', @@ -78,6 +80,7 @@ def test_consume_new_result( 'subject_identifier': nvr, 'type': 'test-result-missing', 'scenario': None, + 'source': None, } ], 'summary': '2 of 3 required test results missing', @@ -102,6 +105,7 @@ def test_consume_new_result( 'subject_identifier': nvr, 'type': 'test-result-missing', 'scenario': None, + 'source': None, }, { 'testcase': 'dist.rpmdeplint', @@ -110,6 +114,7 @@ def test_consume_new_result( 'subject_identifier': nvr, 'type': 'test-result-missing', 'scenario': None, + 'source': None, }, { 'testcase': 'dist.upgradepath', @@ -118,6 +123,7 @@ def test_consume_new_result( 'subject_identifier': nvr, 'type': 'test-result-missing', 'scenario': None, + 'source': None, }, ], }, @@ -133,6 +139,7 @@ def test_consume_new_result( 'result_id': result['id'], 'testcase': 'dist.rpmdeplint', 'type': 'test-result-passed', + 'source': None, }, ], 'unsatisfied_requirements': [], @@ -156,6 +163,7 @@ def test_consume_new_result( 'subject_identifier': nvr, 'type': 'test-result-missing', 'scenario': None, + 'source': None, }, ], }, @@ -250,6 +258,7 @@ def test_consume_compose_id_result( 'subject_identifier': compose_id, 'result_id': result['id'], 'testcase': 'compose.install_no_user', + 'source': None, 'type': 'test-result-passed' }], 'unsatisfied_requirements': [{ @@ -257,6 +266,7 @@ def test_consume_compose_id_result( 'subject_type': 'compose', 'subject_identifier': compose_id, 'scenario': 'scenario2', + 'source': None, 'testcase': 'compose.install_no_user', 'type': 'test-result-missing'} ] @@ -320,6 +330,7 @@ def test_consume_legacy_result( 'subject_identifier': nvr, 'result_id': result['id'], 'testcase': 'dist.rpmdeplint', + 'source': None, 'type': 'test-result-passed' }], 'unsatisfied_requirements': [ @@ -333,6 +344,7 @@ def test_consume_legacy_result( 'subject_identifier': nvr, 'type': 'test-result-missing', 'scenario': None, + 'source': None, }, { 'testcase': 'dist.upgradepath', @@ -344,6 +356,7 @@ def test_consume_legacy_result( 'subject_identifier': nvr, 'type': 'test-result-missing', 'scenario': None, + 'source': None, } ], 'summary': '2 of 3 required test results missing', @@ -379,6 +392,7 @@ def test_consume_legacy_result( 'subject_identifier': nvr, 'result_id': result['id'], 'testcase': 'dist.rpmdeplint', + 'source': None, 'type': 'test-result-passed' }], 'unsatisfied_requirements': [], @@ -583,6 +597,7 @@ def test_consume_new_result_container_image( 'subject_identifier': item_hash, 'result_id': result['id'], 'testcase': 'baseos-qe.baseos-ci.tier1.functional', + 'source': None, 'type': 'test-result-passed' }], 'unsatisfied_requirements': [] diff --git a/functional-tests/consumers/test_waiverdb.py b/functional-tests/consumers/test_waiverdb.py index d89fc46..f6be801 100644 --- a/functional-tests/consumers/test_waiverdb.py +++ b/functional-tests/consumers/test_waiverdb.py @@ -81,6 +81,7 @@ def test_consume_new_waiver( 'subject_identifier': nvr, 'result_id': results[0]['id'], 'testcase': passing_tests[0], + 'source': None, 'type': 'test-result-passed' }, { @@ -88,6 +89,7 @@ def test_consume_new_waiver( 'subject_identifier': nvr, 'result_id': results[1]['id'], 'testcase': passing_tests[1], + 'source': None, 'type': 'test-result-passed' } ], @@ -98,6 +100,7 @@ def test_consume_new_waiver( 'testcase': failing_test, 'type': 'test-result-failed', 'scenario': None, + 'source': None, }, ], }, @@ -114,13 +117,15 @@ def test_consume_new_waiver( 'result_id': result['id'], 'testcase': failing_test, 'type': 'test-result-failed-waived', - 'scenario': None + 'scenario': None, + 'source': None, }, { 'subject_type': 'koji_build', 'subject_identifier': nvr, 'result_id': results[0]['id'], 'testcase': passing_tests[0], + 'source': None, 'type': 'test-result-passed' }, { @@ -128,6 +133,7 @@ def test_consume_new_waiver( 'subject_identifier': nvr, 'result_id': results[1]['id'], 'testcase': passing_tests[1], + 'source': None, 'type': 'test-result-passed' } ], diff --git a/functional-tests/test_api_v1.py b/functional-tests/test_api_v1.py index 7e8f967..ed8addc 100644 --- a/functional-tests/test_api_v1.py +++ b/functional-tests/test_api_v1.py @@ -260,6 +260,7 @@ def test_make_a_decision_with_verbose_flag(requests_session, greenwave_server, t 'type': 'test-result-passed', 'subject_type': 'koji_build', 'subject_identifier': nvr, + 'source': None, } for result in results ] assert res_data['satisfied_requirements'] == expected_satisfied_requirements @@ -382,6 +383,7 @@ def test_make_a_decision_on_failed_result(requests_session, greenwave_server, te 'result_id': result['id'], 'testcase': TASKTRON_RELEASE_CRITICAL_TASKS[0], 'scenario': None, + 'source': None, 'type': 'test-result-failed' }, ] + [ @@ -392,6 +394,7 @@ def test_make_a_decision_on_failed_result(requests_session, greenwave_server, te 'testcase': name, 'type': 'test-result-missing', 'scenario': None, + 'source': None, } for name in TASKTRON_RELEASE_CRITICAL_TASKS[1:] ] assert res_data['unsatisfied_requirements'] == expected_unsatisfied_requirements @@ -423,6 +426,7 @@ def test_make_a_decision_on_queued_result(requests_session, greenwave_server, te 'subject_type': result['data']['type'][0], 'testcase': TASKTRON_RELEASE_CRITICAL_TASKS[0], 'scenario': None, + 'source': None, 'type': 'test-result-missing' }, ] + [ @@ -433,6 +437,7 @@ def test_make_a_decision_on_queued_result(requests_session, greenwave_server, te 'testcase': name, 'type': 'test-result-missing', 'scenario': None, + 'source': None, } for name in TASKTRON_RELEASE_CRITICAL_TASKS[1:] ] assert res_data['unsatisfied_requirements'] == expected_unsatisfied_requirements @@ -464,6 +469,7 @@ def test_make_a_decision_on_running_result(requests_session, greenwave_server, t 'subject_type': result['data']['type'][0], 'testcase': TASKTRON_RELEASE_CRITICAL_TASKS[0], 'scenario': None, + 'source': None, 'type': 'test-result-missing' }, ] + [ @@ -474,6 +480,7 @@ def test_make_a_decision_on_running_result(requests_session, greenwave_server, t 'testcase': name, 'type': 'test-result-missing', 'scenario': None, + 'source': None, } for name in TASKTRON_RELEASE_CRITICAL_TASKS[1:] ] assert res_data['unsatisfied_requirements'] == expected_unsatisfied_requirements @@ -503,6 +510,7 @@ def test_make_a_decision_on_no_results(requests_session, greenwave_server, testd 'testcase': name, 'type': 'test-result-missing', 'scenario': None, + 'source': None, } for name in TASKTRON_RELEASE_CRITICAL_TASKS ] assert res_data['unsatisfied_requirements'] == expected_unsatisfied_requirements @@ -545,6 +553,7 @@ def test_make_a_decision_on_redhat_cont_image(requests_session, greenwave_server 'result_id': result1['id'], 'testcase': 'test.testcase1', 'scenario': None, + 'source': None, 'type': 'test-result-failed' }, { @@ -553,6 +562,7 @@ def test_make_a_decision_on_redhat_cont_image(requests_session, greenwave_server 'testcase': 'test.testcase2', 'type': 'test-result-failed', 'scenario': None, + 'source': None, } ] assert res_data['unsatisfied_requirements'] == expected_unsatisfied_requirements @@ -687,6 +697,7 @@ def test_multiple_results_in_a_subject( 'testcase': 'dist.abicheck', 'type': 'test-result-failed', 'scenario': None, + 'source': None, }, ] assert res_data['unsatisfied_requirements'] == expected_unsatisfied_requirements @@ -728,6 +739,7 @@ def test_ignore_result(requests_session, greenwave_server, testdatabuilder): 'testcase': TASKTRON_RELEASE_CRITICAL_TASKS[0], 'type': 'test-result-missing', 'scenario': None, + 'source': None, }, ] assert r.status_code == 200 @@ -813,6 +825,7 @@ def test_make_a_decision_on_failing_result_with_scenario( 'testcase': testcase_name, 'type': 'test-result-failed', 'scenario': 'scenario2', + 'source': None, }] assert res_data['unsatisfied_requirements'] == expected_unsatisfied_requirements @@ -858,6 +871,7 @@ def test_ignore_waiver(requests_session, greenwave_server, testdatabuilder): 'testcase': TASKTRON_RELEASE_CRITICAL_TASKS[0], 'type': 'test-result-failed', 'scenario': None, + 'source': None, }, ] assert res_data['policies_satisfied'] is False @@ -1133,6 +1147,7 @@ def test_make_a_decision_about_compose_all_variants_architectures( 'item': {'productmd.compose.id': compose_id}, 'result_id': failed_results['id'], 'scenario': None, + 'source': None, 'testcase': 'rtt.acceptance.validation', 'type': 'test-result-failed' }] diff --git a/greenwave/decision.py b/greenwave/decision.py index c241483..03d2d0f 100644 --- a/greenwave/decision.py +++ b/greenwave/decision.py @@ -152,12 +152,14 @@ def make_decision(data, config): product_version=product_version, )) + visited_rules = set() for policy in subject_policies: answers.extend( policy.check( product_version, subject, - results_retriever)) + results_retriever, + visited_rules)) applicable_policies.extend(subject_policies) diff --git a/greenwave/policies.py b/greenwave/policies.py index f979a65..7037f1e 100644 --- a/greenwave/policies.py +++ b/greenwave/policies.py @@ -8,7 +8,6 @@ import re import greenwave.resources from werkzeug.exceptions import BadRequest, NotFound from flask import current_app -from greenwave.utils import remove_duplicates, to_hashable from greenwave.safe_yaml import ( SafeYAMLBool, SafeYAMLList, @@ -37,6 +36,54 @@ def load_policies(policies_dir): return policies +def _remote_urls(subject): + """ + Returns generator with possible remote rule URLs. + """ + rr_policies_conf = current_app.config.get('REMOTE_RULE_POLICIES', {}) + cur_subject_urls = ( + rr_policies_conf.get(subject.type) or + rr_policies_conf.get('*') or + current_app.config.get('DIST_GIT_URL_TEMPLATE') + ) + + if not cur_subject_urls: + raise RuntimeError(f'Cannot use a remote rule for {subject} subject ' + f'as it has not been configured') + + if not isinstance(cur_subject_urls, list): + cur_subject_urls = [cur_subject_urls] + + for current_url in cur_subject_urls: + url_params = {} + if '{pkg_name}' in current_url or '{pkg_namespace}' in current_url or \ + '{rev}' in current_url: + try: + pkg_namespace, pkg_name, rev = greenwave.resources.retrieve_scm_from_koji( + subject.identifier + ) + except greenwave.resources.NoSourceException as e: + log.error(e) + continue + + # if the element is actually a container and not a pkg there will be a "-container" + # string at the end of the "pkg_name" and it will not match with the one in the + # remote rule file URL + if pkg_namespace == 'containers': + pkg_name = re.sub('-container$', '', pkg_name) + if pkg_namespace: + pkg_namespace += '/' + url_params.update(rev=rev, pkg_name=pkg_name, pkg_namespace=pkg_namespace) + + if '{subject_id}' in current_url: + subj_id = subject.identifier + if subj_id.startswith('sha256:'): + subj_id = subj_id[7:] + url_params.update(subject_id=subj_id) + + yield current_url.format(**url_params) + + class DisallowedRuleError(RuntimeError): pass @@ -51,23 +98,14 @@ class Answer(object): a subclass, depending on what the answer was. """ + is_test_result = True + def to_json(self): """ Returns a machine-readable description of the problem for API responses. """ raise NotImplementedError() - def __hash__(self): - return hash(to_hashable(self.to_json())) - - def __eq__(self, other): - try: - json1 = self.to_json() - json2 = other.to_json() - except NotImplementedError: - return id(self) == id(other) - return json1 == json2 - class RuleSatisfied(Answer): """ @@ -106,10 +144,11 @@ class TestResultMissing(RuleNotSatisfied): ResultsDB with a matching item and test case name). """ - def __init__(self, subject, test_case_name, scenario): + def __init__(self, subject, test_case_name, scenario, source): self.subject = subject self.test_case_name = test_case_name self.scenario = scenario + self.source = source def to_json(self): return { @@ -118,6 +157,8 @@ class TestResultMissing(RuleNotSatisfied): 'subject_type': self.subject.type, 'subject_identifier': self.subject.identifier, 'scenario': self.scenario, + 'source': self.source, + # For backwards compatibility only: 'item': self.subject.to_dict() } @@ -157,16 +198,18 @@ class TestResultFailed(RuleNotSatisfied): not passing). """ - def __init__(self, subject, test_case_name, scenario, result_id): + def __init__(self, subject, test_case_name, scenario, source, result_id): self.subject = subject self.test_case_name = test_case_name self.scenario = scenario + self.source = source self.result_id = result_id def to_json(self): return { 'type': 'test-result-failed', 'testcase': self.test_case_name, + 'source': self.source, 'result_id': self.result_id, # These are for backwards compatibility only # (the values are already visible in the result data itself, the @@ -191,11 +234,13 @@ class TestResultErrored(RuleNotSatisfied): subject, test_case_name, scenario, + source, result_id, error_reason): self.subject = subject self.test_case_name = test_case_name self.scenario = scenario + self.source = source self.result_id = result_id self.error_reason = error_reason @@ -205,6 +250,8 @@ class TestResultErrored(RuleNotSatisfied): 'testcase': self.test_case_name, 'result_id': self.result_id, 'error_reason': self.error_reason, + 'source': self.source, + # These are for backwards compatibility only # (the values are already visible in the result data itself, the # caller shouldn't need them repeated here): @@ -221,11 +268,13 @@ class InvalidRemoteRuleYaml(RuleNotSatisfied): Remote policy parsing failed. """ - def __init__(self, subject, test_case_name, details, scenario=None): + scenario = None + + def __init__(self, subject, test_case_name, details, source): self.subject = subject self.test_case_name = test_case_name self.details = details - self.scenario = scenario + self.source = source def to_json(self): return { @@ -234,6 +283,7 @@ class InvalidRemoteRuleYaml(RuleNotSatisfied): 'subject_type': self.subject.type, 'subject_identifier': self.subject.identifier, 'scenario': self.scenario, + 'source': self.source, 'details': self.details } @@ -249,8 +299,9 @@ class MissingRemoteRuleYaml(RuleNotSatisfied): test_case_name = 'missing-gating-yaml' scenario = None - def __init__(self, subject): + def __init__(self, subject, sources): self.subject = subject + self.sources = sources def to_json(self): return { @@ -258,21 +309,72 @@ class MissingRemoteRuleYaml(RuleNotSatisfied): 'testcase': self.test_case_name, 'subject_type': self.subject.type, 'subject_identifier': self.subject.identifier, - 'scenario': self.scenario + 'scenario': self.scenario, + 'sources': self.sources, } def to_waived(self): return None +class FailedFetchRemoteRuleYaml(RuleNotSatisfied): + """ + Error while fetching remote policy. + """ + + scenario = None + + test_case_name = 'failed-fetch-gating-yaml' + + def __init__(self, subject, sources, error): + self.subject = subject + self.sources = sources + self.error = error + + def to_json(self): + return { + 'type': 'failed-fetch-gating-yaml', + 'testcase': self.test_case_name, + 'subject_type': self.subject.type, + 'subject_identifier': self.subject.identifier, + 'scenario': self.scenario, + 'sources': self.sources, + 'error': self.error, + } + + def to_waived(self): + return None + + +class FetchedRemoteRuleYaml(RuleSatisfied): + """ + Remote policy was found in remote repository. + """ + + is_test_result = False + + def __init__(self, subject, source): + self.subject = subject + self.source = source + + def to_json(self): + return { + 'type': 'fetched-gating-yaml', + 'subject_type': self.subject.type, + 'subject_identifier': self.subject.identifier, + 'source': self.source, + } + + class TestResultPassed(RuleSatisfied): """ A required test case passed (that is, its outcome in ResultsDB was passing) or a corresponding waiver was found. """ - def __init__(self, subject, test_case_name, result_id): + def __init__(self, subject, test_case_name, source, result_id): self.subject = subject self.test_case_name = test_case_name + self.source = source self.result_id = result_id def to_json(self): @@ -282,6 +384,7 @@ class TestResultPassed(RuleSatisfied): 'result_id': self.result_id, 'subject_type': self.subject.type, 'subject_identifier': self.subject.identifier, + 'source': self.source, } @@ -289,13 +392,16 @@ class BlacklistedInPolicy(RuleSatisfied): """ Package was blacklisted in policy. """ - def __init__(self, subject_identifier): + def __init__(self, subject_identifier, policy): self.subject_identifier = subject_identifier + self.policy = policy def to_json(self): return { 'type': 'blacklisted', 'subject_identifier': self.subject_identifier, + 'policy': self.policy.id, + 'source': self.policy.source, } @@ -303,13 +409,16 @@ class ExcludedInPolicy(RuleSatisfied): """ Package was excluded in policy. """ - def __init__(self, subject_identifier): + def __init__(self, subject_identifier, policy): self.subject_identifier = subject_identifier + self.policy = policy def to_json(self): return { 'type': 'excluded', 'subject_identifier': self.subject_identifier, + 'policy': self.policy.id, + 'source': self.policy.source, } @@ -348,12 +457,19 @@ def summarize_answers(answers): Returns: str: Human-readable summary. """ - if not answers: + test_answers = [ + answer for answer in answers + if answer.is_test_result + ] + if not test_answers: return 'no tests are required' - summary = _summarize_answers_without_errored(answers) + summary = _summarize_answers_without_errored(test_answers) - errored_count = len([answer for answer in answers if isinstance(answer, TestResultErrored)]) + errored_count = len([ + answer for answer in test_answers + if isinstance(answer, TestResultErrored) + ]) if errored_count: summary += f' ({errored_count} {"error" if errored_count == 1 else "errors"})' @@ -373,7 +489,8 @@ class Rule(SafeYAMLObject): policy, product_version, subject, - results_retriever): + results_retriever, + visited_rules=None): """ Evaluate this policy rule for the given item. @@ -403,38 +520,12 @@ class Rule(SafeYAMLObject): """ return True - @staticmethod - def process_on_demand_rules(rules): - #pylint: disable=attribute-defined-outside-init - """ - Validates rules and creates objects for them. - - Args: - rules (json): User specified rules - - Returns: - list: Returns a list of appropriate objects - """ - if not all([rule.get('type') for rule in rules]): - raise BadRequest('Key \'type\' is required for every rule') - if not all([rule.get('test_case_name') for rule in rules if rule['type'] != 'RemoteRule']): - raise BadRequest('Key \'test_case_name\' is required if not a RemoteRule') - - processed_rules = [] - for rule in rules: - if rule['type'] == 'RemoteRule': - temp_rule = RemoteRule() - temp_rule.required = rule.get('required', False) - processed_rules.append(temp_rule) - elif rule['type'] == 'PassingTestCaseRule': - temp_rule = PassingTestCaseRule() - temp_rule.test_case_name = rule['test_case_name'] - temp_rule.scenario = rule.get('scenario') - processed_rules.append(temp_rule) - else: - raise BadRequest('Invalid rule type {}'.format(rule['type'])) + def __hash__(self): + d = self.to_json() + return hash(tuple((k, d[k]) for k in sorted(d.keys()))) - return processed_rules + def __eq__(self, other): + return self.to_json() == other.to_json() class RemoteRule(Rule): @@ -443,126 +534,96 @@ class RemoteRule(Rule): 'required': SafeYAMLBool(optional=True, default=False), } - @staticmethod - def _get_sub_policies(policy, subject): + def _get_sub_policies(self, policy, subject): + #pylint: disable=broad-except + """ + Returns matching policies from the first available remote rule file, + and answers (including FetchedRemoteRuleYaml, MissingRemoteRuleYaml, + InvalidRemoteRuleYaml, FailedFetchRemoteRuleYaml). + """ if not subject.supports_remote_rule: - return [] + return [], [] - rr_policies_conf = current_app.config.get('REMOTE_RULE_POLICIES', {}) - cur_subject_urls = ( - rr_policies_conf.get(policy.subject_type) or - rr_policies_conf.get('*') or - current_app.config.get('DIST_GIT_URL_TEMPLATE') - ) - - if not cur_subject_urls: - raise RuntimeError(f'Cannot use a remote rule for {subject} subject ' - f'as it has not been configured') - - if not isinstance(cur_subject_urls, list): - cur_subject_urls = [cur_subject_urls] - - for current_url in cur_subject_urls: - response = None - url_params = {} - if '{pkg_name}' in current_url or '{pkg_namespace}' in current_url or \ - '{rev}' in current_url: - try: - pkg_namespace, pkg_name, rev = greenwave.resources.retrieve_scm_from_koji( - subject.identifier - ) - except greenwave.resources.NoSourceException as e: - log.error(e) - return None - - # if the element is actually a container and not a pkg there will be a "-container" - # string at the end of the "pkg_name" and it will not match with the one in the - # remote rule file URL - if pkg_namespace == 'containers': - pkg_name = re.sub('-container$', '', pkg_name) - if pkg_namespace: - pkg_namespace += '/' - url_params.update(rev=rev, pkg_name=pkg_name, pkg_namespace=pkg_namespace) - - if '{subject_id}' in current_url: - subj_id = subject.identifier - if subj_id.startswith('sha256:'): - subj_id = subj_id[7:] - url_params.update(subject_id=subj_id) - - response = greenwave.resources.retrieve_yaml_remote_rule( - current_url.format(**url_params) - ) + remote_policies_urls = [] + remote_policies_url = None + response = None + answers = [] - if response is not None: - break + try: + for remote_policies_url in _remote_urls(subject): + remote_policies_urls.append(remote_policies_url) + response = greenwave.resources.retrieve_yaml_remote_rule(remote_policies_url) + if response is not None: + break + except NotFound: + error = f'Koji build not found for {subject}' + return [], [FailedFetchRemoteRuleYaml(subject, remote_policies_urls, error)] + except Exception: + logging.exception('Failed to retrieve policies for %r', subject) + error = 'Unexpected error' + return [], [FailedFetchRemoteRuleYaml(subject, remote_policies_urls, error)] + # Remote rule file not found? if response is None: - # greenwave extension file not found - return None + if self.required: + answers.append(MissingRemoteRuleYaml(subject, remote_policies_urls)) + return [], answers - policies = RemotePolicy.safe_load_all(response) - if isinstance(policy, OnDemandPolicy): - return [ - sub_policy for sub_policy in policies - if any(sub_policy.matches_product_version(pv) for pv in policy.product_versions) - ] + answers.append(FetchedRemoteRuleYaml(subject, remote_policies_url)) - return [ + try: + policies = RemotePolicy.safe_load_all(response) + except SafeYAMLError as e: + answers.append( + InvalidRemoteRuleYaml( + subject, 'invalid-gating-yaml', str(e), remote_policies_url)) + policies = [] + + for sub_policy in policies: + sub_policy.source = remote_policies_url + + sub_policies = [ sub_policy for sub_policy in policies - if set(sub_policy.all_decision_contexts).intersection(policy.all_decision_contexts) + if policy.matches_sub_policy(sub_policy) ] + return sub_policies, answers - @remove_duplicates def check( self, policy, product_version, subject, - results_retriever): - try: - policies = self._get_sub_policies(policy, subject) - except SafeYAMLError as e: - return [ - InvalidRemoteRuleYaml(subject, 'invalid-gating-yaml', str(e)) - ] + results_retriever, + visited_rules=None): + policies, answers = self._get_sub_policies(policy, subject) - if policies is None: - if self.required: - return [MissingRemoteRuleYaml(subject)] - return [] - - answers = [] for remote_policy in policies: if remote_policy.matches_product_version(product_version): response = remote_policy.check( - product_version, subject, results_retriever) + product_version, subject, results_retriever, visited_rules) - if isinstance(response, list): - answers.extend(response) - else: - answers.append(response) + if not isinstance(response, list): + response = [response] + + answers.extend(response) return answers def matches(self, policy, **attributes): - #pylint: disable=broad-except subject = attributes.get('subject') if not subject: return True sub_policies = [] - try: - sub_policies = self._get_sub_policies(policy, subject) - except SafeYAMLError: - logging.exception('Failed to parse policies for %r', subject) - except NotFound: - logging.error('Koji build not found for %r', subject) - except Exception: - logging.exception('Failed to retrieve policies for %r', subject) + sub_policies, answers = self._get_sub_policies(policy, subject) + + # Include policy if remote rule file is missing. + if not answers: + return True - if sub_policies is None: - # RemoteRule matches if remote policy file is missing. + # Include any failure fetching/parsing remote rule file in the + # decision. + if any(not answer.is_satisfied for answer in answers): return True return any(sub_policy.matches(**attributes) for sub_policy in sub_policies) @@ -585,13 +646,13 @@ class PassingTestCaseRule(Rule): 'scenario': SafeYAMLString(optional=True), } - @remove_duplicates def check( self, policy, product_version, subject, - results_retriever): + results_retriever, + visited_rules=None): matching_results = results_retriever.retrieve(subject, self.test_case_name) if self.scenario is not None: @@ -601,13 +662,13 @@ class PassingTestCaseRule(Rule): # Investigate the absence of result first. if not matching_results: - return TestResultMissing(subject, self.test_case_name, self.scenario) + return TestResultMissing(subject, self.test_case_name, self.scenario, policy.source) # If we find multiple matching results, we always use the first one which # will be the latest chronologically, because ResultsDB always returns # results ordered by `submit_time` descending. return [ - self._answer_for_result(result, subject) + self._answer_for_result(result, subject, policy.source) for result in matching_results ] @@ -622,20 +683,20 @@ class PassingTestCaseRule(Rule): 'scenario': self.scenario, } - def _answer_for_result(self, result, subject): + def _answer_for_result(self, result, subject, source): outcome = result['outcome'] if outcome in current_app.config['OUTCOMES_PASSED']: log.debug('Test result passed for the result_id %s and testcase %s,' ' because the outcome is %s', result['id'], self.test_case_name, outcome) - return TestResultPassed(subject, self.test_case_name, result['id']) + return TestResultPassed(subject, self.test_case_name, source, result['id']) if outcome in current_app.config['OUTCOMES_INCOMPLETE']: log.debug('Test result MISSING for the %s and ' 'testcase %s, because the outcome is %s', subject, self.test_case_name, outcome) - return TestResultMissing(subject, self.test_case_name, self.scenario) + return TestResultMissing(subject, self.test_case_name, self.scenario, source) if outcome in current_app.config['OUTCOMES_ERROR']: error_reason = result.get('error_reason') @@ -643,13 +704,13 @@ class PassingTestCaseRule(Rule): 'testcase %s, because the outcome is %s; error reason: %s', subject, self.test_case_name, outcome, error_reason) return TestResultErrored( - subject, self.test_case_name, self.scenario, result['id'], + subject, self.test_case_name, self.scenario, source, result['id'], error_reason) log.debug('Test result failed for the %s and ' 'testcase %s, because the outcome is %s and it didn\'t match any of the ' 'previous cases', subject, self.test_case_name, outcome) - return TestResultFailed(subject, self.test_case_name, self.scenario, result['id']) + return TestResultFailed(subject, self.test_case_name, self.scenario, source, result['id']) class ObsoleteRule(Rule): @@ -669,7 +730,8 @@ class ObsoleteRule(Rule): policy, product_version, subject, - results_retriever): + results_retriever, + visited_rules=None): raise ValueError('This rule is obsolete and can\'t be checked') @@ -699,6 +761,8 @@ class Policy(SafeYAMLObject): 'relevance_value': SafeYAMLString(optional=True), } + source = None + def validate(self): if not self.decision_context and not self.decision_contexts: raise SafeYAMLError('No decision contexts provided') @@ -735,31 +799,43 @@ class Policy(SafeYAMLObject): subject = attributes.get('subject') return not subject or subject.type == self.subject_type - @remove_duplicates + def matches_sub_policy(self, sub_policy): + return set(sub_policy.all_decision_contexts).intersection(self.all_decision_contexts) + def check( self, product_version, subject, - results_retriever): + results_retriever, + visited_rules=None): + if visited_rules is None: + visited_rules = set() + # If an item is about a package and it is in the blacklist, return RuleSatisfied() name = subject.package_name if name: if name in self.blacklist: - return [BlacklistedInPolicy(subject.identifier) for rule in self.rules] + return [BlacklistedInPolicy(subject.identifier, self)] for exclude in self.excluded_packages: if fnmatch(name, exclude): - return [ExcludedInPolicy(subject.identifier) for rule in self.rules] + return [ExcludedInPolicy(subject.identifier, self)] if self.packages and not any(fnmatch(name, package) for package in self.packages): # If the `packages` whitelist is set and this package isn't in the # `packages` whitelist, then the policy doesn't apply to it return [] + answers = [] for rule in self.rules: + if rule in visited_rules: + continue + visited_rules.add(rule) + response = rule.check( self, product_version, subject, - results_retriever) + results_retriever, + visited_rules) if isinstance(response, list): answers.extend(response) else: @@ -803,6 +879,9 @@ class OnDemandPolicy(Policy): def matches_subject_type(self, **attributes): return True + def matches_sub_policy(self, sub_policy): + return any(sub_policy.matches_product_version(pv) for pv in self.product_versions) + class RemotePolicy(Policy): root_yaml_tag = '!Policy' diff --git a/greenwave/tests/test_policies.py b/greenwave/tests/test_policies.py index fd9ac1d..322fe03 100644 --- a/greenwave/tests/test_policies.py +++ b/greenwave/tests/test_policies.py @@ -11,6 +11,7 @@ from greenwave.app_factory import create_app from greenwave.policies import ( load_policies, summarize_answers, + FetchedRemoteRuleYaml, Policy, RemotePolicy, RemoteRule, @@ -75,23 +76,22 @@ class DummyResultsRetriever(ResultsRetriever): def test_summarize_answers(): - subject = create_subject('koji_build', 'nvr') - assert summarize_answers([RuleSatisfied()]) == \ + testSubject = create_subject('koji_build', 'nvr') + testResultPassed = RuleSatisfied() + testResultFailed = TestResultFailed(testSubject, 'test', None, None, 1) + testResultMissing = TestResultMissing(testSubject, 'test', None, None) + + assert summarize_answers([testResultPassed]) == \ 'All required tests passed' - assert summarize_answers([TestResultFailed(subject, 'test', None, 'id'), - RuleSatisfied()]) == \ + assert summarize_answers([testResultFailed, testResultPassed]) == \ '1 of 2 required tests failed' - assert summarize_answers([TestResultMissing(subject, 'test', None)]) == \ + assert summarize_answers([testResultMissing]) == \ '1 of 1 required test results missing' - assert summarize_answers([TestResultMissing(subject, 'test', None), - TestResultFailed(subject, 'test', None, 'id')]) == \ + assert summarize_answers([testResultMissing, testResultFailed]) == \ '1 of 2 required tests failed, 1 result missing' - assert summarize_answers([TestResultMissing(subject, 'testa', None), - TestResultMissing(subject, 'testb', None), - TestResultFailed(subject, 'test', None, 'id')]) == \ + assert summarize_answers([testResultMissing, testResultMissing, testResultFailed]) == \ '1 of 3 required tests failed, 2 results missing' - assert summarize_answers([TestResultMissing(subject, 'test', None), - RuleSatisfied()]) == \ + assert summarize_answers([testResultMissing, testResultPassed]) == \ '1 of 2 required test results missing' @@ -338,20 +338,23 @@ def test_remote_rule_policy(tmpdir, namespace): # Ensure that presence of a result is success. results = DummyResultsRetriever(subject, 'dist.upgradepath') decision = policy.check('fedora-26', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], RuleSatisfied) + assert len(decision) == 2 + assert isinstance(decision[0], FetchedRemoteRuleYaml) + assert isinstance(decision[1], RuleSatisfied) # Ensure that absence of a result is failure. results = DummyResultsRetriever() decision = policy.check('fedora-26', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], TestResultMissing) + assert len(decision) == 2 + assert isinstance(decision[0], FetchedRemoteRuleYaml) + assert isinstance(decision[1], TestResultMissing) # And that a result with a failure, is a failure. results = DummyResultsRetriever(subject, 'dist.upgradepath', 'FAILED') decision = policy.check('fedora-26', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], TestResultFailed) + assert len(decision) == 2 + assert isinstance(decision[0], FetchedRemoteRuleYaml) + assert isinstance(decision[1], TestResultFailed) f.assert_called_with( 'https://src.fedoraproject.org/{0}'.format( '' if not namespace else namespace + '/' @@ -413,8 +416,9 @@ def test_remote_rule_policy_old_config(tmpdir): # Ensure that presence of a result is success. results = DummyResultsRetriever(subject, 'dist.upgradepath') decision = policy.check('fedora-26', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], RuleSatisfied) + assert len(decision) == 2 + assert isinstance(decision[0], FetchedRemoteRuleYaml) + assert isinstance(decision[1], RuleSatisfied) f.assert_called_once_with( 'http://localhost.localdomain/nethack/' @@ -467,20 +471,23 @@ def test_remote_rule_policy_brew_build_group(tmpdir): # Ensure that presence of a result is success. results = DummyResultsRetriever(subject, 'dist.upgradepath') decision = policy.check('fedora-26', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], RuleSatisfied) + assert len(decision) == 2 + assert isinstance(decision[0], FetchedRemoteRuleYaml) + assert isinstance(decision[1], RuleSatisfied) # Ensure that absence of a result is failure. results = DummyResultsRetriever() decision = policy.check('fedora-26', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], TestResultMissing) + assert len(decision) == 2 + assert isinstance(decision[0], FetchedRemoteRuleYaml) + assert isinstance(decision[1], TestResultMissing) # And that a result with a failure, is a failure. results = DummyResultsRetriever(subject, 'dist.upgradepath', 'FAILED') decision = policy.check('fedora-26', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], TestResultFailed) + assert len(decision) == 2 + assert isinstance(decision[0], FetchedRemoteRuleYaml) + assert isinstance(decision[1], TestResultFailed) f.assert_called_with( 'https://git.example.com/devops/greenwave-policies/side-tags/raw/' 'master/0f41e56a1c32519e189ddbcb01d2551e861bd74e603d01769ef5f70d4b30a2dd.yaml' @@ -530,8 +537,9 @@ def test_remote_rule_policy_with_no_remote_rule_policies_param_defined(tmpdir): # Ensure that presence of a result is success. results = DummyResultsRetriever(subject, 'dist.upgradepath') decision = policy.check('fedora-26', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], RuleSatisfied) + assert len(decision) == 2 + assert isinstance(decision[0], FetchedRemoteRuleYaml) + assert isinstance(decision[1], RuleSatisfied) f.assert_called_with( 'https://src.fedoraproject.org/rpms/nethack/raw/' 'c3c47a08a66451cb9686c49f040776ed35a0d1bb/f/gating.yaml' @@ -580,21 +588,24 @@ def test_remote_rule_policy_redhat_module(tmpdir, namespace): # Ensure that presence of a result is success. results = DummyResultsRetriever(subject, 'baseos-ci.redhat-module.tier0.functional') decision = policy.check('rhel-8', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], RuleSatisfied) + assert len(decision) == 2 + assert isinstance(decision[0], FetchedRemoteRuleYaml) + assert isinstance(decision[1], RuleSatisfied) # Ensure that absence of a result is failure. results = DummyResultsRetriever(subject) decision = policy.check('rhel-8', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], TestResultMissing) + assert len(decision) == 2 + assert isinstance(decision[0], FetchedRemoteRuleYaml) + assert isinstance(decision[1], TestResultMissing) # And that a result with a failure, is a failure. results = DummyResultsRetriever( subject, 'baseos-ci.redhat-module.tier0.functional', 'FAILED') decision = policy.check('rhel-8', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], TestResultFailed) + assert len(decision) == 2 + assert isinstance(decision[0], FetchedRemoteRuleYaml) + assert isinstance(decision[1], TestResultFailed) def test_remote_rule_policy_redhat_container_image(tmpdir): @@ -639,21 +650,24 @@ def test_remote_rule_policy_redhat_container_image(tmpdir): results = DummyResultsRetriever( subject, 'baseos-ci.redhat-container-image.tier0.functional') decision = policy.check('rhel-8', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], RuleSatisfied) + assert len(decision) == 2 + assert isinstance(decision[0], FetchedRemoteRuleYaml) + assert isinstance(decision[1], RuleSatisfied) # Ensure that absence of a result is failure. results = DummyResultsRetriever(subject) decision = policy.check('rhel-8', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], TestResultMissing) + assert len(decision) == 2 + assert isinstance(decision[0], FetchedRemoteRuleYaml) + assert isinstance(decision[1], TestResultMissing) # And that a result with a failure, is a failure. results = DummyResultsRetriever( subject, 'baseos-ci.redhat-container-image.tier0.functional', 'FAILED') decision = policy.check('rhel-8', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], TestResultFailed) + assert len(decision) == 2 + assert isinstance(decision[0], FetchedRemoteRuleYaml) + assert isinstance(decision[1], TestResultFailed) def test_get_sub_policies_multiple_urls(tmpdir): @@ -775,9 +789,10 @@ def test_remote_rule_policy_optional_id(tmpdir): results = DummyResultsRetriever() decision = policy.check('fedora-26', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], TestResultMissing) - assert decision[0].is_satisfied is False + assert len(decision) == 2 + assert isinstance(decision[0], FetchedRemoteRuleYaml) + assert isinstance(decision[1], TestResultMissing) + assert decision[1].is_satisfied is False def test_remote_rule_malformed_yaml(tmpdir): @@ -827,9 +842,11 @@ def test_remote_rule_malformed_yaml(tmpdir): results = DummyResultsRetriever() decision = policy.check('fedora-26', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], InvalidRemoteRuleYaml) - assert decision[0].is_satisfied is False + assert len(decision) == 2 + assert isinstance(decision[0], FetchedRemoteRuleYaml) + assert isinstance(decision[1], InvalidRemoteRuleYaml) + assert decision[0].is_satisfied is True + assert decision[1].is_satisfied is False def test_remote_rule_malformed_yaml_with_waiver(tmpdir): @@ -891,7 +908,8 @@ def test_remote_rule_malformed_yaml_with_waiver(tmpdir): }] decision = policy.check('fedora-26', subject, results) decision = waive_answers(decision, waivers) - assert len(decision) == 0 + assert len(decision) == 1 + assert isinstance(decision[0], FetchedRemoteRuleYaml) def test_remote_rule_required(): @@ -1351,20 +1369,23 @@ def test_remote_rule_policy_on_demand_policy(namespace): # Ensure that presence of a result is success. results = DummyResultsRetriever(subject, 'dist.upgradepath') decision = policy.check('fedora-26', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], RuleSatisfied) + assert len(decision) == 2 + assert isinstance(decision[0], FetchedRemoteRuleYaml) + assert isinstance(decision[1], RuleSatisfied) # Ensure that absence of a result is failure. results = DummyResultsRetriever() decision = policy.check('fedora-26', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], TestResultMissing) + assert len(decision) == 2 + assert isinstance(decision[0], FetchedRemoteRuleYaml) + assert isinstance(decision[1], TestResultMissing) # And that a result with a failure, is a failure. results = DummyResultsRetriever(subject, 'dist.upgradepath', 'FAILED') decision = policy.check('fedora-26', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], TestResultFailed) + assert len(decision) == 2 + assert isinstance(decision[0], FetchedRemoteRuleYaml) + assert isinstance(decision[1], TestResultFailed) @pytest.mark.parametrize('two_rules', (True, False)) @@ -1484,20 +1505,23 @@ def test_two_rules_no_duplicate(tmpdir): # Ensure that presence of a result is success. results = DummyResultsRetriever(subject, 'dist.upgradepath') decision = policy.check('fedora-31', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], RuleSatisfied) + assert len(decision) == 2 + assert isinstance(decision[0], FetchedRemoteRuleYaml) + assert isinstance(decision[1], RuleSatisfied) # Ensure that absence of a result is failure. results = DummyResultsRetriever() decision = policy.check('fedora-31', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], TestResultMissing) + assert len(decision) == 2 + assert isinstance(decision[0], FetchedRemoteRuleYaml) + assert isinstance(decision[1], TestResultMissing) # And that a result with a failure, is a failure. results = DummyResultsRetriever(subject, 'dist.upgradepath', 'FAILED') decision = policy.check('fedora-31', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], TestResultFailed) + assert len(decision) == 2 + assert isinstance(decision[0], FetchedRemoteRuleYaml) + assert isinstance(decision[1], TestResultFailed) def test_cache_all_results_temporarily(): diff --git a/greenwave/tests/test_rules.py b/greenwave/tests/test_rules.py index a22952a..c6ca6ec 100644 --- a/greenwave/tests/test_rules.py +++ b/greenwave/tests/test_rules.py @@ -3,8 +3,11 @@ import pytest from textwrap import dedent +from werkzeug.exceptions import NotFound + from greenwave.app_factory import create_app from greenwave.policies import Policy, RemoteRule +from greenwave.resources import NoSourceException from greenwave.safe_yaml import SafeYAMLError from greenwave.subjects.factory import create_subject @@ -68,16 +71,83 @@ def test_match_remote_rule(mock_retrieve_scm_from_koji, mock_retrieve_yaml_remot assert rule.matches(policy, subject=subject, testcase='some_test_case') assert not rule.matches(policy, subject=subject, testcase='other_test_case') - # Simulate invalid gating.yaml file. - def raiseYamlError(*args): - #pylint: disable=unused-argument - raise SafeYAMLError() - mock_retrieve_yaml_remote_rule.side_effect = raiseYamlError - assert rule.matches(policy) - assert not rule.matches(policy, subject=subject) - assert not rule.matches(policy, subject=subject, testcase='some_test_case') - assert not rule.matches(policy, subject=subject, testcase='other_test_case') +@mock.patch('greenwave.resources.retrieve_yaml_remote_rule') +@mock.patch('greenwave.resources.retrieve_scm_from_koji') +def test_remote_rule_include_failures( + mock_retrieve_scm_from_koji, mock_retrieve_yaml_remote_rule): + policy_yaml = dedent(""" + --- !Policy + id: "some_policy" + product_versions: [rhel-9000] + decision_context: bodhi_update_push_stable + subject_type: koji_build + rules: + - !RemoteRule {} + """) + nvr = 'nethack-1.2.3-1.el9000' + mock_retrieve_scm_from_koji.return_value = ('rpms', nvr, '123') + + app = create_app('greenwave.config.TestingConfig') + with app.app_context(): + subject = create_subject('koji_build', nvr) + policies = Policy.safe_load_all(policy_yaml) + assert len(policies) == 1 + + policy = policies[0] + assert len(policy.rules) == 1 + + rule = policy.rules[0] + + # Include any failure fetching/parsing remote rule file in the + # decision. + mock_retrieve_yaml_remote_rule.return_value = "--- !Policy" + assert rule.matches(policy, subject=subject, testcase='other_test_case') + answers = rule.check( + policy, product_version='rhel-9000', subject=subject, results_retriever=None) + assert len(answers) == 2 + assert answers[1].test_case_name == 'invalid-gating-yaml' + + mock_retrieve_scm_from_koji.side_effect = NotFound + assert rule.matches(policy, subject=subject, testcase='other_test_case') + answers = rule.check( + policy, product_version='rhel-9000', subject=subject, results_retriever=None) + assert len(answers) == 1 + assert answers[0].error == f'Koji build not found for {subject}' + + +@mock.patch('greenwave.resources.retrieve_scm_from_koji') +def test_remote_rule_exclude_no_source(mock_retrieve_scm_from_koji): + policy_yaml = dedent(""" + --- !Policy + id: "some_policy" + product_versions: [rhel-9000] + decision_context: bodhi_update_push_stable + subject_type: koji_build + rules: + - !RemoteRule {} + """) + nvr = 'nethack-1.2.3-1.el9000' + + app = create_app('greenwave.config.TestingConfig') + with app.app_context(): + subject = create_subject('koji_build', nvr) + policies = Policy.safe_load_all(policy_yaml) + assert len(policies) == 1 + + policy = policies[0] + assert len(policy.rules) == 1 + + rule = policy.rules[0] + + mock_retrieve_scm_from_koji.side_effect = NoSourceException + assert rule.matches(policy, subject=subject) + assert rule.matches(policy, subject=subject, testcase='some_test_case') + assert rule.matches(policy, subject=subject, testcase='other_test_case') + + answers = rule.check( + policy, product_version='rhel-9000', subject=subject, results_retriever=None) + assert answers == [] @pytest.mark.parametrize(('required_flag', 'required_value'), ( diff --git a/greenwave/tests/test_summary.py b/greenwave/tests/test_summary.py index 79c20ac..6a9c903 100644 --- a/greenwave/tests/test_summary.py +++ b/greenwave/tests/test_summary.py @@ -16,13 +16,13 @@ from greenwave.subjects.subject_type import GenericSubjectType testSubject = Subject(GenericSubjectType('koji_build'), 'nethack-1.2.3-1.el9000') testResultPassed = RuleSatisfied() testResultErrored = TestResultErrored( - testSubject, 'test', None, 1, 'some error') + testSubject, 'test', None, None, 1, 'some error') testResultFailed = TestResultFailed( - testSubject, 'test', None, 1) + testSubject, 'test', None, None, 1) testResultMissing = TestResultMissing( - testSubject, 'test', None) + testSubject, 'test', None, None) testInvalidGatingYaml = InvalidRemoteRuleYaml( - testSubject, 'test', 'Missing !Policy tag') + testSubject, 'test', 'Missing !Policy tag', None) def test_summary_passed(): diff --git a/greenwave/tests/test_waive.py b/greenwave/tests/test_waive.py index a7f3961..670c922 100644 --- a/greenwave/tests/test_waive.py +++ b/greenwave/tests/test_waive.py @@ -1,10 +1,7 @@ # SPDX-License-Identifier: GPL-2.0+ -import mock - from greenwave.policies import ( InvalidRemoteRuleYaml, - TestResultPassed, TestResultMissing, TestResultFailed, ) @@ -23,6 +20,7 @@ def test_waive_failed_result(): subject=test_subject(), test_case_name='test1', scenario='scenario1', + source='https://greenwave_tests.example.com', result_id=99, ) ] @@ -46,6 +44,7 @@ def test_waive_failed_result(): subject_identifier='nethack-1.2.3-1.rawhide', result_id=99, scenario='scenario1', + source='https://greenwave_tests.example.com', ) assert 1 == len(waived) assert expected_json == waived[0].to_json() @@ -57,6 +56,7 @@ def test_waive_missing_result(): subject=test_subject(), test_case_name='test1', scenario='scenario1', + source='https://greenwave_tests.example.com', ) ] @@ -78,6 +78,7 @@ def test_waive_missing_result(): subject_type='koji_build', subject_identifier='nethack-1.2.3-1.rawhide', scenario='scenario1', + source='https://greenwave_tests.example.com', ) assert 1 == len(waived) assert expected_json == waived[0].to_json() @@ -88,6 +89,7 @@ def test_waive_invalid_gatin_yaml(): InvalidRemoteRuleYaml( subject=test_subject(), test_case_name='invalid-gating-yaml', + source='https://greenwave_tests.example.com', details='', ) ] @@ -107,75 +109,13 @@ def test_waive_invalid_gatin_yaml(): assert [] == waived -def test_waive_answers_duplicates(): - mock_subject = mock.Mock() - mock_subject.type = 'koji_build' - mock_subject.identifier = 'glibc-1.0-1588233006.954829.fedora-rawhide' - mock_subject.to_dict.return_value = {'item': mock_subject.identifier, 'type': mock_subject.type} - test_name1 = 'test1' - test_name2 = 'test2' - scenario = 'xyz' - result_id = 123456 - answers = [ - TestResultPassed(mock_subject, test_name1, result_id), - TestResultMissing(mock_subject, test_name2, scenario), - TestResultFailed(mock_subject, test_name1, scenario, result_id), - TestResultPassed(mock_subject, test_name1, result_id), - TestResultMissing(mock_subject, test_name2, scenario), - TestResultFailed(mock_subject, test_name2, scenario, result_id) - ] - waivers = [ - { - "subject_identifier": mock_subject.identifier, - "subject_type": mock_subject.type, - 'testcase': test_name2 - } - ] - answers_json = [ans.to_json() for ans in waive_answers(answers, waivers)] - answers_to_check = [ - { - "subject_identifier": mock_subject.identifier, - "subject_type": mock_subject.type, - "testcase": test_name1, - 'result_id': result_id, - "type": "test-result-passed" - }, - { - "scenario": scenario, - "subject_identifier": mock_subject.identifier, - "subject_type": mock_subject.type, - "testcase": test_name2, - "type": "test-result-missing-waived" - }, - { - "item": { - "item": mock_subject.identifier, - "type": mock_subject.type - }, - "scenario": scenario, - "testcase": test_name1, - 'result_id': result_id, - "type": "test-result-failed" - }, - { - "scenario": scenario, - "subject_identifier": mock_subject.identifier, - "subject_type": mock_subject.type, - "testcase": test_name2, - 'result_id': result_id, - "type": "test-result-failed-waived" - }, - ] - assert len(answers_json) == len(answers_to_check) - assert all(a in answers_json for a in answers_to_check) - - def test_waive_scenario(): answers = [ TestResultFailed( subject=test_subject(), test_case_name='test1', scenario='scenario1', + source='https://greenwave_tests.example.com', result_id=99, ) ] @@ -209,6 +149,7 @@ def test_waive_scenario(): subject_identifier='nethack-1.2.3-1.rawhide', result_id=99, scenario='scenario1', + source='https://greenwave_tests.example.com', ) assert 1 == len(waived) assert expected_json == waived[0].to_json() diff --git a/greenwave/utils.py b/greenwave/utils.py index bea6e57..cdbae21 100644 --- a/greenwave/utils.py +++ b/greenwave/utils.py @@ -160,22 +160,3 @@ def add_to_timestamp(timestamp, **kwargs): def right_before_this_time(timestamp): return add_to_timestamp(timestamp, microseconds=-1) - - -def remove_duplicates(func): - def wrapper(*args, **kwargs): - rv = func(*args, **kwargs) - if isinstance(rv, list) and len(rv): - rv = list(dict.fromkeys(rv)) - return rv - return wrapper - - -def to_hashable(val): - if isinstance(val, list) or isinstance(val, tuple): - return tuple([to_hashable(v) for v in val]) - if isinstance(val, dict): - return tuple([(k, to_hashable(val[k])) for k in sorted(val.keys())]) - if isinstance(val, set): - return tuple(sorted(val)) - return val diff --git a/greenwave/waivers.py b/greenwave/waivers.py index b47a5c6..bd62a0e 100644 --- a/greenwave/waivers.py +++ b/greenwave/waivers.py @@ -1,9 +1,6 @@ # SPDX-License-Identifier: GPL-2.0+ -from greenwave.utils import remove_duplicates - - def _is_waived(answer, waivers): """ Returns true only if there is a matching waiver for given answer. @@ -27,7 +24,6 @@ def _maybe_waive(answer, waivers): return answer -@remove_duplicates def waive_answers(answers, waivers): """ Returns answers with unsatisfied answers waived