From 61eeef68ba8347237b042d87e0f77db995743b37 Mon Sep 17 00:00:00 2001 From: Lukas Holecek Date: May 06 2021 05:33:41 +0000 Subject: Add specialized classes gather requirements --- diff --git a/greenwave/decision.py b/greenwave/decision.py index 03d2d0f..57366ca 100644 --- a/greenwave/decision.py +++ b/greenwave/decision.py @@ -23,6 +23,111 @@ from greenwave.waivers import waive_answers log = logging.getLogger(__name__) +class RuleContext: + """ + Environment for verifying rules from multiple policies for a single + decision subject. + """ + def __init__(self, product_version, subject, results_retriever): + self.product_version = product_version + self.subject = subject + self.results_retriever = results_retriever + self.verified_rules = set() + + def get_results(self, test_case_name): + return self.results_retriever.retrieve(self.subject, test_case_name) + + def verify(self, policy, rule): + if rule in self.verified_rules: + return [] + + self.verified_rules.add(rule) + + return rule.check(policy, self) + + +class Decision: + """ + Collects answers from rules from policies. + """ + def __init__(self, decision_context, product_version, verbose=False): + self.decision_context = decision_context + self.product_version = product_version + self.verbose = verbose + + self.verbose_results = [] + self.waivers = [] + self.waiver_filters = [] + self.answers = [] + self.applicable_policies = [] + + def check(self, subject, policies, results_retriever): + subject_policies = [ + policy for policy in policies + if policy.matches( + decision_context=self.decision_context, + product_version=self.product_version, + subject=subject) + ] + + if not subject_policies: + if subject.ignore_missing_policy: + return + + log.error( + 'Cannot find any applicable policies for %s subjects at gating point %s in %s', + subject.type, self.decision_context, self.product_version) + raise NotFound( + 'Cannot find any applicable policies for %s subjects at gating point %s in %s' % ( + subject.type, self.decision_context, self.product_version)) + + if self.verbose: + # Retrieve test results and waivers for all items when verbose output is requested. + self.verbose_results.extend(results_retriever.retrieve(subject)) + self.waiver_filters.append(dict( + subject_type=subject.type, + subject_identifier=subject.identifier, + product_version=self.product_version, + )) + + rule_context = RuleContext(self.product_version, subject, results_retriever) + for policy in subject_policies: + self.answers.extend(policy.check(rule_context)) + + self.applicable_policies.extend(subject_policies) + + def waive_answers(self, waivers_retriever): + if not self.verbose: + for answer in self.answers: + if not answer.is_satisfied: + self.waiver_filters.append(dict( + subject_type=answer.subject.type, + subject_identifier=answer.subject.identifier, + product_version=self.product_version, + testcase=answer.test_case_name, + scenario=answer.scenario + )) + + if self.waiver_filters: + self.waivers = waivers_retriever.retrieve(self.waiver_filters) + else: + self.waivers = [] + + self.answers = waive_answers(self.answers, self.waivers) + + def policies_satisfied(self): + return all(answer.is_satisfied for answer in self.answers) + + def summary(self): + return summarize_answers(self.answers) + + def satisfied_requirements(self): + return [answer.to_json() for answer in self.answers if answer.is_satisfied] + + def unsatisfied_requirements(self): + return [answer.to_json() for answer in self.answers if not answer.is_satisfied] + + def _decision_subject(data): try: subject = create_subject_from_data(data) @@ -108,9 +213,6 @@ def make_decision(data, config): except ValueError: raise BadRequest('Invalid "when" parameter, must be in ISO8601 format') - answers = [] - verbose_results = [] - applicable_policies = [] retriever_args = {'when': when} results_retriever = ResultsRetriever( ignore_ids=ignore_results, @@ -120,83 +222,30 @@ def make_decision(data, config): ignore_ids=ignore_waivers, url=config['WAIVERDB_API_URL'], **retriever_args) - waiver_filters = [] policies = on_demand_policies or config['policies'] + decision = Decision(decision_context, product_version, verbose) for subject in _decision_subjects_for_request(data): - subject_policies = [ - policy for policy in policies - if policy.matches( - decision_context=decision_context, - product_version=product_version, - subject=subject) - ] + decision.check(subject, policies, results_retriever) - if not subject_policies: - if subject.ignore_missing_policy: - continue - - log.error( - 'Cannot find any applicable policies for %s subjects at gating point %s in %s', - subject.type, decision_context, product_version) - raise NotFound( - 'Cannot find any applicable policies for %s subjects at gating point %s in %s' % ( - subject.type, decision_context, product_version)) - - if verbose: - # Retrieve test results and waivers for all items when verbose output is requested. - verbose_results.extend(results_retriever.retrieve(subject)) - waiver_filters.append(dict( - subject_type=subject.type, - subject_identifier=subject.identifier, - product_version=product_version, - )) - - visited_rules = set() - for policy in subject_policies: - answers.extend( - policy.check( - product_version, - subject, - results_retriever, - visited_rules)) - - applicable_policies.extend(subject_policies) - - if not verbose: - for answer in answers: - if not answer.is_satisfied: - waiver_filters.append(dict( - subject_type=answer.subject.type, - subject_identifier=answer.subject.identifier, - product_version=product_version, - testcase=answer.test_case_name, - scenario=answer.scenario - )) - - if waiver_filters: - waivers = waivers_retriever.retrieve(waiver_filters) - else: - waivers = [] - answers = waive_answers(answers, waivers) + decision.waive_answers(waivers_retriever) response = { - 'policies_satisfied': all(answer.is_satisfied for answer in answers), - 'summary': summarize_answers(answers), - 'satisfied_requirements': - [answer.to_json() for answer in answers if answer.is_satisfied], - 'unsatisfied_requirements': - [answer.to_json() for answer in answers if not answer.is_satisfied] + 'policies_satisfied': decision.policies_satisfied(), + 'summary': decision.summary(), + 'satisfied_requirements': decision.satisfied_requirements(), + 'unsatisfied_requirements': decision.unsatisfied_requirements(), } - # Check if on-demand policy was specified + # Include applicable_policies if on-demand policy was not specified. if not rules: - response.update({'applicable_policies': [policy.id for policy in applicable_policies]}) + response.update({'applicable_policies': [ + policy.id for policy in decision.applicable_policies]}) if verbose: response.update({ - 'results': list({result['id']: result for result in verbose_results}.values()), - 'waivers': list({waiver['id']: waiver for waiver in waivers}.values()), + 'results': list({result['id']: result for result in decision.verbose_results}.values()), + 'waivers': list({waiver['id']: waiver for waiver in decision.waivers}.values()), }) return response diff --git a/greenwave/policies.py b/greenwave/policies.py index 7037f1e..e0346bf 100644 --- a/greenwave/policies.py +++ b/greenwave/policies.py @@ -106,6 +106,10 @@ class Answer(object): """ raise NotImplementedError() + def __repr__(self): + attributes = ' '.join(f'{k}={v}' for k, v in self.to_json().items()) + return f'<{self.__class__.__name__} {attributes}>' + class RuleSatisfied(Answer): """ @@ -484,23 +488,13 @@ class Rule(SafeYAMLObject): This base class is not used directly. """ - def check( - self, - policy, - product_version, - subject, - results_retriever, - visited_rules=None): + def check(self, policy, rule_context): """ Evaluate this policy rule for the given item. Args: policy (Policy): Parent policy of the rule - product_version (str): Product version we are making a decision about - subject (Subject): Item we are making a decision about (for - example, Koji build NVR, Bodhi update id, ...) - results_retriever (ResultsRetriever): Object for retrieving data - from ResultsDB. + rule_context (RuleContext): rule context Returns: Answer: An instance of a subclass of :py:class:`Answer` describing the result. @@ -588,19 +582,15 @@ class RemoteRule(Rule): ] return sub_policies, answers - def check( - self, - policy, - product_version, - subject, - results_retriever, - visited_rules=None): - policies, answers = self._get_sub_policies(policy, subject) + def check(self, policy, rule_context): + policies, answers = self._get_sub_policies(policy, rule_context.subject) + + # Copy cached value. + answers = list(answers) for remote_policy in policies: - if remote_policy.matches_product_version(product_version): - response = remote_policy.check( - product_version, subject, results_retriever, visited_rules) + if remote_policy.matches_product_version(rule_context.product_version): + response = remote_policy.check(rule_context) if not isinstance(response, list): response = [response] @@ -646,14 +636,8 @@ class PassingTestCaseRule(Rule): 'scenario': SafeYAMLString(optional=True), } - def check( - self, - policy, - product_version, - subject, - results_retriever, - visited_rules=None): - matching_results = results_retriever.retrieve(subject, self.test_case_name) + def check(self, policy, rule_context): + matching_results = rule_context.get_results(self.test_case_name) if self.scenario is not None: matching_results = [ @@ -662,13 +646,16 @@ class PassingTestCaseRule(Rule): # Investigate the absence of result first. if not matching_results: - return TestResultMissing(subject, self.test_case_name, self.scenario, policy.source) + return [ + TestResultMissing( + rule_context.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, policy.source) + self._answer_for_result(result, rule_context.subject, policy.source) for result in matching_results ] @@ -725,13 +712,7 @@ class ObsoleteRule(Rule): tag = self.yaml_tag or '!' + type(self).__name__ raise SafeYAMLError('{} is obsolete. {}'.format(tag, self.advice)) - def check( - self, - policy, - product_version, - subject, - results_retriever, - visited_rules=None): + def check(self, policy, rule_context): raise ValueError('This rule is obsolete and can\'t be checked') @@ -802,23 +783,15 @@ class Policy(SafeYAMLObject): 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, - visited_rules=None): - if visited_rules is None: - visited_rules = set() - + def check(self, rule_context): # If an item is about a package and it is in the blacklist, return RuleSatisfied() - name = subject.package_name + name = rule_context.subject.package_name if name: if name in self.blacklist: - return [BlacklistedInPolicy(subject.identifier, self)] + return [BlacklistedInPolicy(rule_context.subject.identifier, self)] for exclude in self.excluded_packages: if fnmatch(name, exclude): - return [ExcludedInPolicy(subject.identifier, self)] + return [ExcludedInPolicy(rule_context.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 @@ -826,20 +799,7 @@ class Policy(SafeYAMLObject): 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, - visited_rules) - if isinstance(response, list): - answers.extend(response) - else: - answers.append(response) + answers.extend(rule_context.verify(self, rule)) return answers def matches_product_version(self, product_version): diff --git a/greenwave/resources.py b/greenwave/resources.py index 646af8e..e4b4f50 100644 --- a/greenwave/resources.py +++ b/greenwave/resources.py @@ -60,10 +60,10 @@ class ResultsRetriever(BaseRetriever): super().__init__(**args) self.cache = {} - def _retrieve_all(self, subject, testcase=None, scenarios=None): + def _retrieve_all(self, subject, testcase=None): # Get test case result from cache if all test case results were already # retrieved for given Subject. - cache_key = (subject.type, subject.identifier, scenarios) + cache_key = (subject.type, subject.identifier) if testcase and cache_key in self.cache: return [res for res in self.cache[cache_key] if res['testcase']['name'] == testcase] @@ -72,7 +72,7 @@ class ResultsRetriever(BaseRetriever): if testcase: external_cache_key = ( "greenwave.resources:ResultsRetriever|" - f"{subject.type} {subject.identifier} {testcase} {scenarios}") + f"{subject.type} {subject.identifier} {testcase}") results = self.get_external_cache(external_cache_key) if results and self._results_match_time(results): return results @@ -84,8 +84,6 @@ class ResultsRetriever(BaseRetriever): params.update({'since': self.since}) if testcase: params.update({'testcases': testcase}) - if scenarios: - params.update({'scenario': ','.join(scenarios)}) results = [] for query in subject.result_queries(): diff --git a/greenwave/tests/test_policies.py b/greenwave/tests/test_policies.py index 322fe03..52d1287 100644 --- a/greenwave/tests/test_policies.py +++ b/greenwave/tests/test_policies.py @@ -8,19 +8,16 @@ import time from textwrap import dedent from greenwave.app_factory import create_app +from greenwave.decision import Decision from greenwave.policies import ( load_policies, summarize_answers, - FetchedRemoteRuleYaml, Policy, RemotePolicy, RemoteRule, RuleSatisfied, TestResultMissing, TestResultFailed, - TestResultPassed, - InvalidRemoteRuleYaml, - MissingRemoteRuleYaml, OnDemandPolicy ) from greenwave.resources import ResultsRetriever @@ -38,6 +35,10 @@ def app(): yield +def answer_types(answers): + return [x.to_json()['type'] for x in answers] + + class DummyResultsRetriever(ResultsRetriever): def __init__(self, subject=None, testcase=None, outcome='PASSED', when=''): super(DummyResultsRetriever, self).__init__( @@ -108,15 +109,14 @@ def test_decision_with_missing_result(tmpdir): - !PassingTestCaseRule {test_case_name: sometest} """)) policies = load_policies(tmpdir.strpath) - policy = policies[0] + subject = create_subject('compose', 'some_nevr') results = DummyResultsRetriever() - subject = create_subject('koji_build', 'some_nevr') + decision = Decision('rawhide_compose_sync_to_mirrors', 'fedora-rawhide') # Ensure that absence of a result is failure. - decision = policy.check('fedora-rawhide', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], TestResultMissing) + decision.check(subject, policies, results) + assert answer_types(decision.answers) == ['test-result-missing'] def test_waive_brew_koji_mismatch(tmpdir): @@ -138,28 +138,28 @@ def test_waive_brew_koji_mismatch(tmpdir): - !PassingTestCaseRule {test_case_name: sometest} """)) policies = load_policies(tmpdir.strpath) - policy = policies[0] - item = 'some_nevr' - subject = create_subject('koji_build', item) + subject = create_subject('koji_build', 'some_nevr') results = DummyResultsRetriever(subject, 'sometest', 'FAILED') - decision = policy.check('fedora-rawhide', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], TestResultFailed) + # Ensure that absence of a result is failure. + decision = Decision('test', 'fedora-rawhide') + decision.check(subject, policies, results) + answers = waive_answers(decision.answers, []) + assert answer_types(answers) == ['test-result-failed'] waivers = [{ 'id': 1, - 'subject_identifier': item, - 'subject_type': 'koji_build', + 'subject_identifier': subject.identifier, + 'subject_type': subject.type, 'testcase': 'sometest', 'product_version': 'fedora-rawhide', 'waived': True, }] - decision = policy.check('fedora-rawhide', subject, results) - decision = waive_answers(decision, waivers) - assert len(decision) == 1 - assert isinstance(decision[0], RuleSatisfied) + decision = Decision('test', 'fedora-rawhide') + decision.check(subject, policies, results) + answers = waive_answers(decision.answers, waivers) + assert answer_types(answers) == ['test-result-failed-waived'] def test_waive_bodhi_update(tmpdir): @@ -181,28 +181,26 @@ def test_waive_bodhi_update(tmpdir): - !PassingTestCaseRule {test_case_name: sometest} """)) policies = load_policies(tmpdir.strpath) - policy = policies[0] - item = 'some_bodhi_update' - subject = create_subject('bodhi_update', item) + subject = create_subject('bodhi_update', 'some_bodhi_update') results = DummyResultsRetriever(subject, 'sometest', 'FAILED') - decision = policy.check('fedora-rawhide', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], TestResultFailed) + decision = Decision('test', 'fedora-rawhide') + decision.check(subject, policies, results) + assert answer_types(decision.answers) == ['test-result-failed'] waivers = [{ 'id': 1, - 'subject_identifier': item, - 'subject_type': 'bodhi_update', + 'subject_identifier': subject.identifier, + 'subject_type': subject.type, 'testcase': 'sometest', 'product_version': 'fedora-rawhide', 'waived': True, }] - decision = policy.check('fedora-rawhide', subject, results) - decision = waive_answers(decision, waivers) - assert len(decision) == 1 - assert isinstance(decision[0], RuleSatisfied) + decision = Decision('test', 'fedora-rawhide') + decision.check(subject, policies, results) + answers = waive_answers(decision.answers, waivers) + assert answer_types(answers) == ['test-result-failed-waived'] def test_load_policies(): @@ -333,28 +331,24 @@ def test_remote_rule_policy(tmpdir, namespace): with mock.patch('greenwave.resources.retrieve_yaml_remote_rule') as f: f.return_value = remote_fragment policies = load_policies(tmpdir.strpath) - policy = policies[0] # Ensure that presence of a result is success. results = DummyResultsRetriever(subject, 'dist.upgradepath') - decision = policy.check('fedora-26', subject, results) - assert len(decision) == 2 - assert isinstance(decision[0], FetchedRemoteRuleYaml) - assert isinstance(decision[1], RuleSatisfied) + decision = Decision('bodhi_update_push_stable_with_remoterule', 'fedora-26') + decision.check(subject, policies, results) + assert answer_types(decision.answers) == ['fetched-gating-yaml', 'test-result-passed'] # Ensure that absence of a result is failure. results = DummyResultsRetriever() - decision = policy.check('fedora-26', subject, results) - assert len(decision) == 2 - assert isinstance(decision[0], FetchedRemoteRuleYaml) - assert isinstance(decision[1], TestResultMissing) + decision = Decision('bodhi_update_push_stable_with_remoterule', 'fedora-26') + decision.check(subject, policies, results) + assert answer_types(decision.answers) == ['fetched-gating-yaml', 'test-result-missing'] # 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) == 2 - assert isinstance(decision[0], FetchedRemoteRuleYaml) - assert isinstance(decision[1], TestResultFailed) + decision = Decision('bodhi_update_push_stable_with_remoterule', 'fedora-26') + decision.check(subject, policies, results) + assert answer_types(decision.answers) == ['fetched-gating-yaml', 'test-result-failed'] f.assert_called_with( 'https://src.fedoraproject.org/{0}'.format( '' if not namespace else namespace + '/' @@ -411,19 +405,19 @@ def test_remote_rule_policy_old_config(tmpdir): with mock.patch('greenwave.resources.retrieve_yaml_remote_rule') as f: f.return_value = remote_fragment policies = load_policies(tmpdir.strpath) - policy = policies[0] # Ensure that presence of a result is success. results = DummyResultsRetriever(subject, 'dist.upgradepath') - decision = policy.check('fedora-26', subject, results) - assert len(decision) == 2 - assert isinstance(decision[0], FetchedRemoteRuleYaml) - assert isinstance(decision[1], RuleSatisfied) + decision = Decision('bodhi_update_push_stable_with_remoterule', 'fedora-26') + decision.check(subject, policies, results) + assert answer_types(decision.answers) == [ + 'fetched-gating-yaml', 'test-result-passed'] - f.assert_called_once_with( + call = mock.call( 'http://localhost.localdomain/nethack/' 'c3c47a08a66451cb9686c49f040776ed35a0d1bb/gating.yaml' ) + assert f.mock_calls == [call, call] finally: Config.REMOTE_RULE_POLICIES = config_remote_rules_backup @@ -454,6 +448,7 @@ def test_remote_rule_policy_brew_build_group(tmpdir): id: "some-policy-from-a-random-packager" product_versions: - fedora-26 + subject_type: brew-build-group decision_context: bodhi_update_push_stable_with_remoterule rules: - !PassingTestCaseRule {test_case_name: dist.upgradepath} @@ -466,28 +461,24 @@ def test_remote_rule_policy_brew_build_group(tmpdir): with mock.patch('greenwave.resources.retrieve_yaml_remote_rule') as f: f.return_value = remote_fragment policies = load_policies(tmpdir.strpath) - policy = policies[0] # Ensure that presence of a result is success. results = DummyResultsRetriever(subject, 'dist.upgradepath') - decision = policy.check('fedora-26', subject, results) - assert len(decision) == 2 - assert isinstance(decision[0], FetchedRemoteRuleYaml) - assert isinstance(decision[1], RuleSatisfied) + decision = Decision('bodhi_update_push_stable_with_remoterule', 'fedora-26') + decision.check(subject, policies, results) + assert answer_types(decision.answers) == ['fetched-gating-yaml', 'test-result-passed'] # Ensure that absence of a result is failure. results = DummyResultsRetriever() - decision = policy.check('fedora-26', subject, results) - assert len(decision) == 2 - assert isinstance(decision[0], FetchedRemoteRuleYaml) - assert isinstance(decision[1], TestResultMissing) + decision = Decision('bodhi_update_push_stable_with_remoterule', 'fedora-26') + decision.check(subject, policies, results) + assert answer_types(decision.answers) == ['fetched-gating-yaml', 'test-result-missing'] # 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) == 2 - assert isinstance(decision[0], FetchedRemoteRuleYaml) - assert isinstance(decision[1], TestResultFailed) + decision = Decision('bodhi_update_push_stable_with_remoterule', 'fedora-26') + decision.check(subject, policies, results) + assert answer_types(decision.answers) == ['fetched-gating-yaml', 'test-result-failed'] f.assert_called_with( 'https://git.example.com/devops/greenwave-policies/side-tags/raw/' 'master/0f41e56a1c32519e189ddbcb01d2551e861bd74e603d01769ef5f70d4b30a2dd.yaml' @@ -532,14 +523,13 @@ def test_remote_rule_policy_with_no_remote_rule_policies_param_defined(tmpdir): with mock.patch('greenwave.resources.retrieve_yaml_remote_rule') as f: f.return_value = remote_fragment policies = load_policies(tmpdir.strpath) - policy = policies[0] # Ensure that presence of a result is success. results = DummyResultsRetriever(subject, 'dist.upgradepath') - decision = policy.check('fedora-26', subject, results) - assert len(decision) == 2 - assert isinstance(decision[0], FetchedRemoteRuleYaml) - assert isinstance(decision[1], RuleSatisfied) + decision = Decision('bodhi_update_push_stable_with_remoterule', 'fedora-26') + decision.check(subject, policies, results) + assert answer_types(decision.answers) == [ + 'fetched-gating-yaml', 'test-result-passed'] f.assert_called_with( 'https://src.fedoraproject.org/rpms/nethack/raw/' 'c3c47a08a66451cb9686c49f040776ed35a0d1bb/f/gating.yaml' @@ -583,29 +573,25 @@ def test_remote_rule_policy_redhat_module(tmpdir, namespace): with mock.patch('greenwave.resources.retrieve_yaml_remote_rule') as f: f.return_value = remote_fragment policies = load_policies(tmpdir.strpath) - policy = policies[0] # 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) == 2 - assert isinstance(decision[0], FetchedRemoteRuleYaml) - assert isinstance(decision[1], RuleSatisfied) + decision = Decision('osci_compose_gate', 'rhel-8') + decision.check(subject, policies, results) + assert answer_types(decision.answers) == ['fetched-gating-yaml', 'test-result-passed'] # Ensure that absence of a result is failure. results = DummyResultsRetriever(subject) - decision = policy.check('rhel-8', subject, results) - assert len(decision) == 2 - assert isinstance(decision[0], FetchedRemoteRuleYaml) - assert isinstance(decision[1], TestResultMissing) + decision = Decision('osci_compose_gate', 'rhel-8') + decision.check(subject, policies, results) + assert answer_types(decision.answers) == ['fetched-gating-yaml', 'test-result-missing'] # 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) == 2 - assert isinstance(decision[0], FetchedRemoteRuleYaml) - assert isinstance(decision[1], TestResultFailed) + decision = Decision('osci_compose_gate', 'rhel-8') + decision.check(subject, policies, results) + assert answer_types(decision.answers) == ['fetched-gating-yaml', 'test-result-failed'] def test_remote_rule_policy_redhat_container_image(tmpdir): @@ -644,30 +630,26 @@ def test_remote_rule_policy_redhat_container_image(tmpdir): with mock.patch('greenwave.resources.retrieve_yaml_remote_rule') as f: f.return_value = remote_fragment policies = load_policies(tmpdir.strpath) - policy = policies[0] # Ensure that presence of a result is success. results = DummyResultsRetriever( subject, 'baseos-ci.redhat-container-image.tier0.functional') - decision = policy.check('rhel-8', subject, results) - assert len(decision) == 2 - assert isinstance(decision[0], FetchedRemoteRuleYaml) - assert isinstance(decision[1], RuleSatisfied) + decision = Decision('osci_compose_gate', 'rhel-8') + decision.check(subject, policies, results) + assert answer_types(decision.answers) == ['fetched-gating-yaml', 'test-result-passed'] # Ensure that absence of a result is failure. results = DummyResultsRetriever(subject) - decision = policy.check('rhel-8', subject, results) - assert len(decision) == 2 - assert isinstance(decision[0], FetchedRemoteRuleYaml) - assert isinstance(decision[1], TestResultMissing) + decision = Decision('osci_compose_gate', 'rhel-8') + decision.check(subject, policies, results) + assert answer_types(decision.answers) == ['fetched-gating-yaml', 'test-result-missing'] # 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) == 2 - assert isinstance(decision[0], FetchedRemoteRuleYaml) - assert isinstance(decision[1], TestResultFailed) + decision = Decision('osci_compose_gate', 'rhel-8') + decision.check(subject, policies, results) + assert answer_types(decision.answers) == ['fetched-gating-yaml', 'test-result-failed'] def test_get_sub_policies_multiple_urls(tmpdir): @@ -703,14 +685,15 @@ def test_get_sub_policies_multiple_urls(tmpdir): with mock.patch('greenwave.resources.requests_session') as session: response = mock.MagicMock() response.status_code = 404 - session.request.side_effect = [response, response] + session.request.return_value = response policy = OnDemandPolicy.create_from_json(serverside_json) assert isinstance(policy.rules[0], RemoteRule) assert policy.rules[0].required results = DummyResultsRetriever() - decision = policy.check('fedora-26', subject, results) + decision = Decision(None, 'fedora-26') + decision.check(subject, [policy], results) expected_call1 = mock.call( 'HEAD', 'https://src1.fp.org/{0}/{1}/raw/{2}/f/gating.yaml'.format( *scm.return_value @@ -721,11 +704,12 @@ def test_get_sub_policies_multiple_urls(tmpdir): *scm.return_value ) ) - assert session.request.mock_calls == [expected_call1, expected_call2] - assert len(decision) == 1 - assert isinstance(decision[0], MissingRemoteRuleYaml) - assert not decision[0].is_satisfied - assert decision[0].subject.identifier == subject.identifier + assert session.request.mock_calls == [ + expected_call1, expected_call2, + expected_call1, expected_call2] + assert answer_types(decision.answers) == ['missing-gating-yaml'] + assert not decision.answers[0].is_satisfied + assert decision.answers[0].subject.identifier == subject.identifier def test_redhat_container_image_subject_type(): @@ -785,14 +769,12 @@ def test_remote_rule_policy_optional_id(tmpdir): with mock.patch('greenwave.resources.retrieve_yaml_remote_rule') as f: f.return_value = remote_fragment policies = load_policies(tmpdir.strpath) - policy = policies[0] results = DummyResultsRetriever() - decision = policy.check('fedora-26', subject, results) - assert len(decision) == 2 - assert isinstance(decision[0], FetchedRemoteRuleYaml) - assert isinstance(decision[1], TestResultMissing) - assert decision[1].is_satisfied is False + decision = Decision('bodhi_update_push_stable_with_remoterule', 'fedora-26') + decision.check(subject, policies, results) + assert answer_types(decision.answers) == ['fetched-gating-yaml', 'test-result-missing'] + assert decision.answers[1].is_satisfied is False def test_remote_rule_malformed_yaml(tmpdir): @@ -838,15 +820,14 @@ def test_remote_rule_malformed_yaml(tmpdir): with mock.patch('greenwave.resources.retrieve_yaml_remote_rule') as f: f.return_value = remote_fragment policies = load_policies(tmpdir.strpath) - policy = policies[0] results = DummyResultsRetriever() - decision = policy.check('fedora-26', subject, results) - 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 + decision = Decision('bodhi_update_push_stable_with_remoterule', 'fedora-26') + decision.check(subject, policies, results) + assert answer_types(decision.answers) == [ + 'fetched-gating-yaml', 'invalid-gating-yaml'] + assert decision.answers[0].is_satisfied is True + assert decision.answers[1].is_satisfied is False def test_remote_rule_malformed_yaml_with_waiver(tmpdir): @@ -893,7 +874,6 @@ def test_remote_rule_malformed_yaml_with_waiver(tmpdir): with mock.patch('greenwave.resources.retrieve_yaml_remote_rule') as f: f.return_value = remote_fragment policies = load_policies(tmpdir.strpath) - policy = policies[0] results = DummyResultsRetriever() waivers = [{ @@ -906,10 +886,13 @@ def test_remote_rule_malformed_yaml_with_waiver(tmpdir): 'comment': 'Waiving the invalid gating.yaml file', 'waived': True, }] - decision = policy.check('fedora-26', subject, results) - decision = waive_answers(decision, waivers) - assert len(decision) == 1 - assert isinstance(decision[0], FetchedRemoteRuleYaml) + + decision = Decision('bodhi_update_push_stable_with_remoterule', 'fedora-26') + decision.check(subject, policies, results) + answers = decision.answers + assert answer_types(answers) == ['fetched-gating-yaml', 'invalid-gating-yaml'] + answers = waive_answers(answers, waivers) + assert answer_types(answers) == ['fetched-gating-yaml'] def test_remote_rule_required(): @@ -928,13 +911,12 @@ def test_remote_rule_required(): rules: - !RemoteRule {required: true} """)) - policy = policies[0] results = DummyResultsRetriever() - decision = policy.check('fedora-rawhide', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], MissingRemoteRuleYaml) - assert not decision[0].is_satisfied - assert decision[0].subject.identifier == subject.identifier + decision = Decision('test', 'fedora-rawhide') + decision.check(subject, policies, results) + assert answer_types(decision.answers) == ['missing-gating-yaml'] + assert not decision.answers[0].is_satisfied + assert decision.answers[0].subject.identifier == subject.identifier def test_parse_policies_missing_tag(): @@ -1028,13 +1010,11 @@ def test_policy_with_arbitrary_subject_type(tmpdir): - !PassingTestCaseRule {test_case_name: sometest} """)) policies = load_policies(tmpdir.strpath) - policy = policies[0] - subject = create_subject('kind-of-magic', 'nethack-1.2.3-1.el9000') results = DummyResultsRetriever(subject, 'sometest', 'PASSED') - decision = policy.check('rhel-9000', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], TestResultPassed) + decision = Decision('bodhi_update_push_stable', 'rhel-9000') + decision.check(subject, policies, results) + assert answer_types(decision.answers) == ['test-result-passed'] def test_policy_all_decision_contexts(tmpdir): @@ -1070,12 +1050,12 @@ def test_policy_all_decision_contexts(tmpdir): assert policy.all_decision_contexts == ['test4'] -@pytest.mark.parametrize(('package', 'num_decisions'), [ - ('nethack', 1), - ('net*', 1), - ('python-requests', 0), +@pytest.mark.parametrize(('package', 'expected_answers'), [ + ('nethack', ['test-result-passed']), + ('net*', ['test-result-passed']), + ('python-requests', []), ]) -def test_policy_with_packages_whitelist(tmpdir, package, num_decisions): +def test_policy_with_packages_whitelist(tmpdir, package, expected_answers): p = tmpdir.join('temp.yaml') p.write(dedent(""" --- !Policy @@ -1090,14 +1070,11 @@ def test_policy_with_packages_whitelist(tmpdir, package, num_decisions): - !PassingTestCaseRule {{test_case_name: sometest}} """.format(package))) policies = load_policies(tmpdir.strpath) - policy = policies[0] - subject = create_subject('koji_build', 'nethack-1.2.3-1.el9000') results = DummyResultsRetriever(subject, 'sometest', 'PASSED') - decision = policy.check('rhel-9000', subject, results) - assert len(decision) == num_decisions - if num_decisions: - assert isinstance(decision[0], TestResultPassed) + decision = Decision('test', 'rhel-9000') + decision.check(subject, policies, results) + assert answer_types(decision.answers) == expected_answers def test_parse_policies_invalid_rule(): @@ -1301,11 +1278,10 @@ def test_policy_with_subject_type_component_version(tmpdir): - !PassingTestCaseRule {test_case_name: test_for_new_type} """)) policies = load_policies(tmpdir.strpath) - policy = policies[0] results = DummyResultsRetriever(subject, 'test_for_new_type', 'PASSED') - decision = policy.check('fedora-29', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], RuleSatisfied) + decision = Decision('decision_context_test_component_version', 'fedora-29') + decision.check(subject, policies, results) + assert answer_types(decision.answers) == ['test-result-passed'] @pytest.mark.parametrize('subject_type', ["redhat-module", "redhat-container-image"]) @@ -1325,11 +1301,10 @@ def test_policy_with_subject_type_redhat_module(tmpdir, subject_type): - !PassingTestCaseRule {test_case_name: test_for_redhat_module_type} """ % subject_type)) policies = load_policies(tmpdir.strpath) - policy = policies[0] results = DummyResultsRetriever(subject, 'test_for_redhat_module_type', 'PASSED') - decision = policy.check('fedora-29', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], RuleSatisfied) + decision = Decision('decision_context_test_redhat_module', 'fedora-29') + decision.check(subject, policies, results) + assert answer_types(decision.answers) == ['test-result-passed'] @pytest.mark.parametrize('namespace', ["rpms", ""]) @@ -1368,24 +1343,21 @@ 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) == 2 - assert isinstance(decision[0], FetchedRemoteRuleYaml) - assert isinstance(decision[1], RuleSatisfied) + decision = Decision(None, 'fedora-26') + decision.check(subject, [policy], results) + assert answer_types(decision.answers) == ['fetched-gating-yaml', 'test-result-passed'] # Ensure that absence of a result is failure. results = DummyResultsRetriever() - decision = policy.check('fedora-26', subject, results) - assert len(decision) == 2 - assert isinstance(decision[0], FetchedRemoteRuleYaml) - assert isinstance(decision[1], TestResultMissing) + decision = Decision(None, 'fedora-26') + decision.check(subject, [policy], results) + assert answer_types(decision.answers) == ['fetched-gating-yaml', 'test-result-missing'] # 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) == 2 - assert isinstance(decision[0], FetchedRemoteRuleYaml) - assert isinstance(decision[1], TestResultFailed) + decision = Decision(None, 'fedora-26') + decision.check(subject, [policy], results) + assert answer_types(decision.answers) == ['fetched-gating-yaml', 'test-result-failed'] @pytest.mark.parametrize('two_rules', (True, False)) @@ -1424,10 +1396,10 @@ def test_on_demand_policy_match(two_rules, koji_proxy): results = DummyResultsRetriever( subject, 'fake.testcase.tier0.validation', 'PASSED' ) - decision = policy.check('fedora-30', subject, results) + decision = Decision(None, 'fedora-30') + decision.check(subject, [policy], results) if two_rules: - assert len(decision) == 1 - assert isinstance(decision[0], RuleSatisfied) + assert answer_types(decision.answers) == ['test-result-passed'] def test_remote_rule_policy_on_demand_policy_required(): @@ -1460,11 +1432,11 @@ def test_remote_rule_policy_on_demand_policy_required(): assert policy.rules[0].required results = DummyResultsRetriever() - decision = policy.check('fedora-26', subject, results) - assert len(decision) == 1 - assert isinstance(decision[0], MissingRemoteRuleYaml) - assert not decision[0].is_satisfied - assert decision[0].subject.identifier == subject.identifier + decision = Decision(None, 'fedora-26') + decision.check(subject, [policy], results) + assert answer_types(decision.answers) == ['missing-gating-yaml'] + assert not decision.answers[0].is_satisfied + assert decision.answers[0].subject.identifier == subject.identifier def test_two_rules_no_duplicate(tmpdir): @@ -1500,28 +1472,24 @@ def test_two_rules_no_duplicate(tmpdir): with mock.patch('greenwave.resources.retrieve_yaml_remote_rule') as f: f.return_value = remote_fragment policies = load_policies(tmpdir.strpath) - policy = policies[0] # Ensure that presence of a result is success. results = DummyResultsRetriever(subject, 'dist.upgradepath') - decision = policy.check('fedora-31', subject, results) - assert len(decision) == 2 - assert isinstance(decision[0], FetchedRemoteRuleYaml) - assert isinstance(decision[1], RuleSatisfied) + decision = Decision('bodhi_update_push_stable_with_remoterule', 'fedora-31') + decision.check(subject, policies, results) + assert answer_types(decision.answers) == ['fetched-gating-yaml', 'test-result-passed'] # Ensure that absence of a result is failure. results = DummyResultsRetriever() - decision = policy.check('fedora-31', subject, results) - assert len(decision) == 2 - assert isinstance(decision[0], FetchedRemoteRuleYaml) - assert isinstance(decision[1], TestResultMissing) + decision = Decision('bodhi_update_push_stable_with_remoterule', 'fedora-31') + decision.check(subject, policies, results) + assert answer_types(decision.answers) == ['fetched-gating-yaml', 'test-result-missing'] # 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) == 2 - assert isinstance(decision[0], FetchedRemoteRuleYaml) - assert isinstance(decision[1], TestResultFailed) + decision = Decision('bodhi_update_push_stable_with_remoterule', 'fedora-31') + decision.check(subject, policies, results) + assert answer_types(decision.answers) == ['fetched-gating-yaml', 'test-result-failed'] def test_cache_all_results_temporarily(): diff --git a/greenwave/tests/test_rules.py b/greenwave/tests/test_rules.py index c6ca6ec..8dec652 100644 --- a/greenwave/tests/test_rules.py +++ b/greenwave/tests/test_rules.py @@ -6,6 +6,7 @@ from textwrap import dedent from werkzeug.exceptions import NotFound from greenwave.app_factory import create_app +from greenwave.decision import Decision from greenwave.policies import Policy, RemoteRule from greenwave.resources import NoSourceException from greenwave.safe_yaml import SafeYAMLError @@ -103,17 +104,19 @@ def test_remote_rule_include_failures( # 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' + decision = Decision('bodhi_update_push_stable', 'rhel-9000') + decision.check(subject, policies, results_retriever=None) + assert len(decision.answers) == 2 + assert decision.answers[1].test_case_name == 'invalid-gating-yaml' + # Reload rules to clear cache. + policies = Policy.safe_load_all(policy_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}' + decision = Decision('bodhi_update_push_stable', 'rhel-9000') + decision.check(subject, policies, results_retriever=None) + assert [x.to_json()['type'] for x in decision.answers] == ['failed-fetch-gating-yaml'] + assert decision.answers[0].error == f'Koji build not found for {subject}' @mock.patch('greenwave.resources.retrieve_scm_from_koji') @@ -145,9 +148,9 @@ def test_remote_rule_exclude_no_source(mock_retrieve_scm_from_koji): 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 == [] + decision = Decision('bodhi_update_push_stable', 'rhel-9000') + decision.check(subject, policies, results_retriever=None) + assert decision.answers == [] @pytest.mark.parametrize(('required_flag', 'required_value'), (