From 6228c0a4127416129bee01a10c3d140855271fba Mon Sep 17 00:00:00 2001 From: Ralph Bean Date: May 05 2018 03:21:53 +0000 Subject: Simplify remote rule. We were originally thinking about getting the `rev` value from the results that were found, but then switched at one point to get the `rev` from koji. Now that we're doing that, we don't need to loop over the results the way we were. --- diff --git a/greenwave/policies.py b/greenwave/policies.py index 3d1e70b..ba750cf 100644 --- a/greenwave/policies.py +++ b/greenwave/policies.py @@ -2,7 +2,6 @@ from fnmatch import fnmatch import yaml -from werkzeug.exceptions import InternalServerError import greenwave.resources @@ -166,37 +165,26 @@ class RemoteOriginalSpecNvrRule(Rule): yaml_loader = yaml.SafeLoader def check(self, item, results, waivers): - for result in results: - if 'original_spec_nvr' not in result['data']: # just go on to the next one - continue - if 'rev' not in result['data'] or len(result['data']['rev']) == 0: - # Trying to find the rev asking to koji - rev = greenwave.resources.retrieve_rev_from_koji( - result['data']['original_spec_nvr'][0]) - else: - rev = result['data']['rev'][0] - pkg_name = result['data']['original_spec_nvr'][0].rsplit('-', 2)[0] - response = greenwave.resources.retrieve_yaml_remote_original_spec_nvr_rule(rev, - pkg_name) - # greenwave file not found - if isinstance(response, RuleSatisfied): - return RuleSatisfied() - else: - policies = yaml.safe_load_all(response) - # policies is a generator, so listifying it - policies = list(policies) - validate_policies(policies, [RemoteOriginalSpecNvrRule]) - answers = [] - for policy in policies: - response = policy.check(item, results, waivers) - if isinstance(response, list): - answers.extend(response) - else: - answers.append(response) - return answers - - # if we arrived here it means that we don't have any result... - raise InternalServerError('Impossible to extend the policy.') + pkg_name = item['original_spec_nvr'].rsplit('-', 2)[0] + rev = greenwave.resources.retrieve_rev_from_koji(item['original_spec_nvr']) + response = greenwave.resources.retrieve_yaml_remote_original_spec_nvr_rule(rev, pkg_name) + + if isinstance(response, RuleSatisfied): + # greenwave extension file not found + return RuleSatisfied() + else: + policies = yaml.safe_load_all(response) + # policies is a generator, so listifying it + policies = list(policies) + validate_policies(policies, [RemoteOriginalSpecNvrRule]) + answers = [] + for policy in policies: + response = policy.check(item, results, waivers) + if isinstance(response, list): + answers.extend(response) + else: + answers.append(response) + return answers def to_json(self): return { diff --git a/greenwave/resources.py b/greenwave/resources.py index c401675..d632476 100644 --- a/greenwave/resources.py +++ b/greenwave/resources.py @@ -28,6 +28,11 @@ def retrieve_rev_from_koji(nvr): """ Retrieve cached rev from koji using the nrv """ proxy = xmlrpclib.ServerProxy(current_app.config['KOJI_BASE_URL']) build = proxy.getBuild(nvr) + + if not build: + raise BadGateway("Found %s when looking for %s at %s" % ( + build, nvr, current_app.config['KOJI_BASE_URL'])) + try: url = urlparse.urlparse(build['extra']['source']['original_url']) if not url.scheme.startswith('git'): diff --git a/greenwave/tests/test_policies.py b/greenwave/tests/test_policies.py index 01b0a94..73697ce 100644 --- a/greenwave/tests/test_policies.py +++ b/greenwave/tests/test_policies.py @@ -384,10 +384,10 @@ rules: [] def test_remote_original_spec_nvr_rule_policy(tmpdir): """ Testing the RemoteOriginalSpecNvrRule with the koji interaction. In this case we are just mocking koji """ - app = create_app('greenwave.config.TestingConfig') - with app.app_context(): - p = tmpdir.join('greenwave.yaml') - p.write(""" + + nvr = 'nethack-1.2.3-1.el9000' + + serverside_fragment = """ --- !Policy id: "taskotron_release_critical_tasks_with_remoterule" product_versions: @@ -395,21 +395,56 @@ product_versions: decision_context: bodhi_update_push_stable_with_remoterule blacklist: [] rules: - - !RemoteOriginalSpecNvrRule {test_case_name: dist.upgradepath} - """) + - !RemoteOriginalSpecNvrRule {} + """ + + remote_fragment = """ +--- !Policy +id: "some-policy-from-a-random-packager" +product_versions: + - fedora-26 +decision_context: bodhi_update_push_stable_with_remoterule +blacklist: [] +rules: + - !PassingTestCaseRule {test_case_name: dist.upgradepath} + """ + + p = tmpdir.join('greenwave.yaml') + p.write(serverside_fragment) + app = create_app('greenwave.config.TestingConfig') + with app.app_context(): with mock.patch('greenwave.resources.retrieve_rev_from_koji'): - policies = load_policies(tmpdir.strpath) - policy = policies[0] - - # Ensure that absence of a result is failure. - item, waivers = {}, [] - results = [{ - "data": { - "original_spec_nvr": ['nethack-1.2.3-1.el9000'] - }, - "testcase": {"name": "dist.upgradepath"}, - "outcome": "PASSED" - }] - decision = policy.check(item, results, waivers) - assert len(decision) == 1 - assert isinstance(decision[0], RuleSatisfied) + with mock.patch('greenwave.resources.retrieve_yaml_remote_original_spec_nvr_rule') as f: + f.return_value = remote_fragment + policies = load_policies(tmpdir.strpath) + policy = policies[0] + + item, waivers = {'original_spec_nvr': nvr}, [] + + # Ensure that presence of a result is success. + results = [{ + "id": 12345, + "data": {"original_spec_nvr": [nvr]}, + "testcase": {"name": "dist.upgradepath"}, + "outcome": "PASSED" + }] + decision = policy.check(item, results, waivers) + assert len(decision) == 1 + assert isinstance(decision[0], RuleSatisfied) + + # Ensure that absence of a result is failure. + results = [] + decision = policy.check(item, results, waivers) + assert len(decision) == 1 + assert isinstance(decision[0], TestResultMissing) + + # And that a result with a failure, is a failure. + results = [{ + "id": 12345, + "data": {"original_spec_nvr": [nvr]}, + "testcase": {"name": "dist.upgradepath"}, + "outcome": "FAILED" + }] + decision = policy.check(item, results, waivers) + assert len(decision) == 1 + assert isinstance(decision[0], TestResultFailed)