From 7dcf8f6fe1d54c899789fa3a1444b42936e03b04 Mon Sep 17 00:00:00 2001 From: Giulia Naponiello Date: Dec 12 2018 12:54:16 +0000 Subject: WIP: (trying to) generalize the subject_types Trying to remove the hard-coded subject_types, and put the list of available subject_types inside the configuration, but the greenwave/policies.py file seems to have issues with imports/scope. --- diff --git a/greenwave/api_v1.py b/greenwave/api_v1.py index cfb8493..bb077df 100644 --- a/greenwave/api_v1.py +++ b/greenwave/api_v1.py @@ -23,11 +23,10 @@ def _decision_subject(subject): subject_identifier = subject.get('item') if subject_identifier: - if subject_type in ('bodhi_update', 'component-version', 'koji_build', 'redhat-module'): - return (subject_type, subject_identifier) - if subject_type == 'brew-build': - return ('koji_build', subject_identifier) + subject_type = 'koji_build' + if subject_type in current_app.config['SUBSET_SUBJECT_TYPES']: + return (subject_type, subject_identifier) if 'productmd.compose.id' in subject: return ('compose', subject['productmd.compose.id']) @@ -70,7 +69,7 @@ def subject_type_identifier_to_list(subject_type, subject_identifier): Inverse of the above function. This is for backwards compatibility in emitted messages. """ - if subject_type in ['bodhi_update', 'koji_build', 'component-version', 'redhat-module']: + if subject_type in current_app.config['SUBSET_SUBJECT_TYPES']: return [{'type': subject_type, 'item': subject_identifier}] if subject_type == 'compose': return [{'productmd.compose.id': subject_identifier}] diff --git a/greenwave/config.py b/greenwave/config.py index d3f0563..1e0a8a3 100644 --- a/greenwave/config.py +++ b/greenwave/config.py @@ -33,6 +33,10 @@ class Config(object): # By default, don't cache anything. CACHE = {'backend': 'dogpile.cache.null'} + # Subject types + SUBSET_SUBJECT_TYPES = {'bodhi_update', 'component-version', 'koji_build', 'redhat-module'} + ALL_SUBJECT_TYPES = SUBSET_SUBJECT_TYPES.union({'compose', 'brew-build'}) + class ProductionConfig(Config): DEBUG = False diff --git a/greenwave/consumers/resultsdb.py b/greenwave/consumers/resultsdb.py index 0d397c9..42ebeae 100644 --- a/greenwave/consumers/resultsdb.py +++ b/greenwave/consumers/resultsdb.py @@ -138,8 +138,7 @@ class ResultsDBHandler(fedmsg.consumers.FedmsgConsumer): log.info('Greenwave resultsdb handler listening on: %s', self.topic) - @staticmethod - def announcement_subjects(message): + def announcement_subjects(self, message): """ Yields pairs of (subject type, subject identifier) for announcement consideration from the message. @@ -160,7 +159,9 @@ class ResultsDBHandler(fedmsg.consumers.FedmsgConsumer): return value _type = _decode(data.get('type')) - if _type in ['bodhi_update', 'component-version', 'redhat-module'] and ( + if _type == 'brew-build': + _type = 'koji_build' + if _type in self.flask_app.config['SUBSET_SUBJECT_TYPES'] and ( 'item' in data): yield (_type, _decode(data['item'])) # note: it is *intentional* that we do not handle old format @@ -174,16 +175,11 @@ class ResultsDBHandler(fedmsg.consumers.FedmsgConsumer): # https://pagure.io/greenwave/pull-request/262#comment-70350 if 'productmd.compose.id' in data: yield ('compose', _decode(data['productmd.compose.id'])) - if (_type == 'koji_build' and 'item' in data or - _type == 'brew-build' and 'item' in data or - 'original_spec_nvr' in data): - if _type in ['koji_build', 'brew-build']: - nvr = _decode(data['item']) - else: - nvr = _decode(data['original_spec_nvr']) + if 'original_spec_nvr' in data: # when the pipeline ignores a package, which happens # *a lot*, we get a message with an 'original_spec_nvr' # key with an empty value; let's not try and handle this + nvr = _decode(data['original_spec_nvr']) if nvr: yield ('koji_build', nvr) diff --git a/greenwave/policies.py b/greenwave/policies.py index d4ab83b..f25cd70 100644 --- a/greenwave/policies.py +++ b/greenwave/policies.py @@ -7,6 +7,7 @@ import logging import os import re import greenwave.resources +import greenwave.utils from greenwave.safe_yaml import ( SafeYAMLChoice, @@ -40,19 +41,6 @@ class DisallowedRuleError(RuntimeError): pass -def subject_type_identifier_to_item(subject_type, subject_identifier): - """ - Greenwave < 0.8 included an "item" key in the "unsatisfied_requirements". - This returns a suitable value for that key, for backwards compatibility. - """ - if subject_type in ['bodhi_update', 'koji_build', 'component-version', 'redhat-module']: - return {'type': subject_type, 'item': subject_identifier} - elif subject_type == 'compose': - return {'productmd.compose.id': subject_identifier} - else: - raise RuntimeError('Unrecognised subject type: %s' % subject_type) - - class Answer(object): """ Represents the result of evaluating a policy rule against a particular @@ -115,7 +103,8 @@ class TestResultMissing(RuleNotSatisfied): 'subject_identifier': self.subject_identifier, 'scenario': self.scenario, # For backwards compatibility only: - 'item': subject_type_identifier_to_item(self.subject_type, self.subject_identifier), + 'item': greenwave.utils.subject_type_identifier_to_item(self.subject_type, + self.subject_identifier), } @@ -160,7 +149,8 @@ class TestResultFailed(RuleNotSatisfied): # These are for backwards compatibility only # (the values are already visible in the result data itself, the # caller shouldn't need them repeated here): - 'item': subject_type_identifier_to_item(self.subject_type, self.subject_identifier), + 'item': greenwave.utils.subject_type_identifier_to_item(self.subject_type, + self.subject_identifier), 'scenario': self.scenario, } @@ -303,9 +293,10 @@ def waives_invalid_gating_yaml(waiver, subject_type, subject_identifier): class RemoteRule(Rule): yaml_tag = '!RemoteRule' safe_yaml_attributes = {} + REMOTERULE_ENABLED_SUBJECT_TYPES = {'koji_build', 'redhat-module'} def _get_sub_policies(self, policy, subject_identifier): - if policy.subject_type not in ['koji_build', 'redhat-module']: + if policy.subject_type not in self.REMOTERULE_ENABLED_SUBJECT_TYPES: return [] pkg_namespace, pkg_name, rev = greenwave.resources.retrieve_scm_from_koji( @@ -548,10 +539,11 @@ class Policy(SafeYAMLObject): 'id': SafeYAMLString(), 'product_versions': SafeYAMLList(str), 'decision_context': SafeYAMLString(), - # TODO: Handle brew-build value better. 'subject_type': SafeYAMLChoice( - 'koji_build', 'bodhi_update', 'compose', 'brew-build', 'component-version', - 'redhat-module'), + 'bodhi_update', 'component-version', 'koji_build', 'redhat-module', 'compose', + 'brew-build'), + # TODO: this should be changed... + # *current_app.config['ALL_SUBJECT_TYPES']), 'rules': SafeYAMLList(Rule), 'blacklist': SafeYAMLList(str, optional=True), 'relevance_key': SafeYAMLString(optional=True), diff --git a/greenwave/resources.py b/greenwave/resources.py index eb2caf8..a52dd2b 100644 --- a/greenwave/resources.py +++ b/greenwave/resources.py @@ -100,11 +100,7 @@ class ResultsRetriever(object): params['testcases'] = testcase results = [] - if subject_type == 'bodhi_update': - params['type'] = subject_type - params['item'] = subject_identifier - results = self._make_request(params=params) - elif subject_type == 'koji_build': + if subject_type == 'koji_build': params['type'] = subject_type params['item'] = subject_identifier results = self._make_request(params=params) @@ -116,17 +112,15 @@ class ResultsRetriever(object): del params['item'] params['original_spec_nvr'] = subject_identifier results.extend(self._make_request(params=params)) - elif subject_type == 'compose': - params['productmd.compose.id'] = subject_identifier - results = self._make_request(params=params) - elif subject_type == 'component-version' or subject_type == 'redhat-module': + elif subject_type in current_app.config['SUBSET_SUBJECT_TYPES']: params['type'] = subject_type params['item'] = subject_identifier results = self._make_request(params=params) - + elif subject_type == 'compose': + params['productmd.compose.id'] = subject_identifier + results = self._make_request(params=params) else: raise RuntimeError('Unhandled subject type %r' % subject_type) - return results diff --git a/greenwave/tests/test_policies.py b/greenwave/tests/test_policies.py index 8528341..75e391c 100644 --- a/greenwave/tests/test_policies.py +++ b/greenwave/tests/test_policies.py @@ -73,8 +73,10 @@ def test_summarize_answers(): def test_waive_absence_of_result(tmpdir): - p = tmpdir.join('fedora.yaml') - p.write(""" + app = create_app('greenwave.config.TestingConfig') + with app.app_context(): + p = tmpdir.join('fedora.yaml') + p.write(""" --- !Policy id: "rawhide_compose_sync_to_mirrors" product_versions: @@ -83,21 +85,21 @@ decision_context: rawhide_compose_sync_to_mirrors subject_type: compose rules: - !PassingTestCaseRule {test_case_name: sometest} - """) - policies = load_policies(tmpdir.strpath) - policy = policies[0] + """) + policies = load_policies(tmpdir.strpath) + policy = policies[0] - # Ensure that absence of a result is failure. - item, results, waivers = {}, DummyResultsRetriever(), [] - decision = policy.check('fedora-rawhide', item, results, waivers) - assert len(decision) == 1 - assert isinstance(decision[0], TestResultMissing) + # Ensure that absence of a result is failure. + item, results, waivers = {}, DummyResultsRetriever(), [] + decision = policy.check('fedora-rawhide', item, results, waivers) + assert len(decision) == 1 + assert isinstance(decision[0], TestResultMissing) - # But also that waiving the absence works. - waivers = [{'testcase': 'sometest', 'waived': True}] - decision = policy.check('fedora-rawhide', item, results, waivers) - assert len(decision) == 1 - assert isinstance(decision[0], RuleSatisfied) + # But also that waiving the absence works. + waivers = [{'testcase': 'sometest', 'waived': True}] + decision = policy.check('fedora-rawhide', item, results, waivers) + assert len(decision) == 1 + assert isinstance(decision[0], RuleSatisfied) def test_waive_brew_koji_mismatch(tmpdir): @@ -152,8 +154,10 @@ def test_waive_bodhi_update(tmpdir): waiver. Even though these are different strings, this should work. """ - p = tmpdir.join('fedora.yaml') - p.write(""" + app = create_app('greenwave.config.TestingConfig') + with app.app_context(): + p = tmpdir.join('fedora.yaml') + p.write(""" --- !Policy id: some_id product_versions: @@ -162,32 +166,32 @@ decision_context: test subject_type: bodhi_update rules: - !PassingTestCaseRule {test_case_name: sometest} - """) - policies = load_policies(tmpdir.strpath) - policy = policies[0] - - item = 'some_bodhi_update' - results = DummyResultsRetriever(item, 'sometest', 'FAILED', 'bodhi_update') - waivers = [{ - u'subject_identifier': item, - u'subject_type': 'bodhi_update', - u'testcase': 'sometest', - u'waived': True, - }] - - decision = policy.check('fedora-rawhide', item, results, []) - assert len(decision) == 1 - assert isinstance(decision[0], TestResultFailed) - - decision = policy.check('fedora-rawhide', item, results, waivers) - assert len(decision) == 1 - assert isinstance(decision[0], RuleSatisfied) - - # Also, be sure that negative waivers work. - waivers[0]['waived'] = False - decision = policy.check('fedora-rawhide', item, results, waivers) - assert len(decision) == 1 - assert isinstance(decision[0], TestResultFailed) + """) + policies = load_policies(tmpdir.strpath) + policy = policies[0] + + item = 'some_bodhi_update' + results = DummyResultsRetriever(item, 'sometest', 'FAILED', 'bodhi_update') + waivers = [{ + u'subject_identifier': item, + u'subject_type': 'bodhi_update', + u'testcase': 'sometest', + u'waived': True, + }] + + decision = policy.check('fedora-rawhide', item, results, []) + assert len(decision) == 1 + assert isinstance(decision[0], TestResultFailed) + + decision = policy.check('fedora-rawhide', item, results, waivers) + assert len(decision) == 1 + assert isinstance(decision[0], RuleSatisfied) + + # Also, be sure that negative waivers work. + waivers[0]['waived'] = False + decision = policy.check('fedora-rawhide', item, results, waivers) + assert len(decision) == 1 + assert isinstance(decision[0], TestResultFailed) def test_package_specific_rule(tmpdir): @@ -867,9 +871,11 @@ def test_policies_to_json(): def test_policy_with_subject_type_component_version(tmpdir): - nv = '389-ds-base-1.4.0.10' - p = tmpdir.join('fedora.yaml') - p.write(""" + app = create_app('greenwave.config.TestingConfig') + with app.app_context(): + nv = '389-ds-base-1.4.0.10' + p = tmpdir.join('fedora.yaml') + p.write(""" --- !Policy id: "test-new-subject-type" product_versions: @@ -879,21 +885,23 @@ subject_type: component-version blacklist: [] rules: - !PassingTestCaseRule {test_case_name: test_for_new_type} - """) - policies = load_policies(tmpdir.strpath) - policy = policies[0] - results = DummyResultsRetriever(nv, 'test_for_new_type', 'PASSED', - 'component-version') - waivers = [] - decision = policy.check('fedora-29', nv, results, waivers) - assert len(decision) == 1 - assert isinstance(decision[0], RuleSatisfied) + """) + policies = load_policies(tmpdir.strpath) + policy = policies[0] + results = DummyResultsRetriever(nv, 'test_for_new_type', 'PASSED', + 'component-version') + waivers = [] + decision = policy.check('fedora-29', nv, results, waivers) + assert len(decision) == 1 + assert isinstance(decision[0], RuleSatisfied) def test_policy_with_subject_type_redhat_module(tmpdir): - nsvc = 'httpd:2.4:20181018085700:9edba152' - p = tmpdir.join('fedora.yaml') - p.write(""" + app = create_app('greenwave.config.TestingConfig') + with app.app_context(): + nsvc = 'httpd:2.4:20181018085700:9edba152' + p = tmpdir.join('fedora.yaml') + p.write(""" --- !Policy id: "test-new-subject-type" product_versions: @@ -903,12 +911,12 @@ subject_type: redhat-module blacklist: [] rules: - !PassingTestCaseRule {test_case_name: test_for_redhat_module_type} - """) - policies = load_policies(tmpdir.strpath) - policy = policies[0] - results = DummyResultsRetriever(nsvc, 'test_for_redhat_module_type', 'PASSED', - 'redhat-module') - waivers = [] - decision = policy.check('fedora-29', nsvc, results, waivers) - assert len(decision) == 1 - assert isinstance(decision[0], RuleSatisfied) + """) + policies = load_policies(tmpdir.strpath) + policy = policies[0] + results = DummyResultsRetriever(nsvc, 'test_for_redhat_module_type', 'PASSED', + 'redhat-module') + waivers = [] + decision = policy.check('fedora-29', nsvc, results, waivers) + assert len(decision) == 1 + assert isinstance(decision[0], RuleSatisfied) diff --git a/greenwave/tests/test_resultsdb_consumer.py b/greenwave/tests/test_resultsdb_consumer.py index f51458b..79e0b02 100644 --- a/greenwave/tests/test_resultsdb_consumer.py +++ b/greenwave/tests/test_resultsdb_consumer.py @@ -10,11 +10,18 @@ from greenwave.policies import Policy def test_announcement_keys_decode_with_list(): - cls = greenwave.consumers.resultsdb.ResultsDBHandler - message = {'msg': {'data': { - 'original_spec_nvr': ['glibc-1.0-1.fc27'], - }}} - subjects = list(cls.announcement_subjects(message)) + app = greenwave.app_factory.create_app('greenwave.config.TestingConfig') + with app.app_context(): + hub = mock.MagicMock() + hub.config = { + 'environment': 'environment', + 'topic_prefix': 'topic_prefix', + } + handler = greenwave.consumers.resultsdb.ResultsDBHandler(hub) + message = {'msg': {'data': { + 'original_spec_nvr': ['glibc-1.0-1.fc27'], + }}} + subjects = list(handler.announcement_subjects(message)) assert subjects == [('koji_build', 'glibc-1.0-1.fc27')] @@ -26,11 +33,16 @@ def test_no_announcement_subjects_for_empty_nvr(): empty string. To avoid unpredictable consequences, we should not return any announcement subjects for such a message. """ - cls = greenwave.consumers.resultsdb.ResultsDBHandler + hub = mock.MagicMock() + hub.config = { + 'environment': 'environment', + 'topic_prefix': 'topic_prefix', + } + handler = greenwave.consumers.resultsdb.ResultsDBHandler(hub) message = {'msg': {'data': { 'original_spec_nvr': [""], }}} - subjects = list(cls.announcement_subjects(message)) + subjects = list(handler.announcement_subjects(message)) assert subjects == [] @@ -38,14 +50,21 @@ def test_no_announcement_subjects_for_empty_nvr(): def test_announcement_subjects_for_brew_build(): # The 'brew-build' type appears internally within Red Hat. We treat it as an # alias of 'koji_build'. - cls = greenwave.consumers.resultsdb.ResultsDBHandler - message = {'msg': {'data': { - 'type': 'brew-build', - 'item': ['glibc-1.0-3.fc27'], - }}} - subjects = list(cls.announcement_subjects(message)) + app = greenwave.app_factory.create_app('greenwave.config.TestingConfig') + with app.app_context(): + hub = mock.MagicMock() + hub.config = { + 'environment': 'environment', + 'topic_prefix': 'topic_prefix', + } + handler = greenwave.consumers.resultsdb.ResultsDBHandler(hub) + message = {'msg': {'data': { + 'type': 'brew-build', + 'item': ['glibc-1.0-3.fc27'], + }}} + subjects = list(handler.announcement_subjects(message)) - assert subjects == [('koji_build', 'glibc-1.0-3.fc27')] + assert subjects == [('koji_build', 'glibc-1.0-3.fc27')] def test_announcement_subjects_for_new_compose_message(): @@ -55,7 +74,12 @@ def test_announcement_subjects_for_new_compose_message(): productmd.compose.id with value of the compose ID. This is only possible with new-style 'resultsdb' fedmsgs, like this one. """ - cls = greenwave.consumers.resultsdb.ResultsDBHandler + hub = mock.MagicMock() + hub.config = { + 'environment': 'environment', + 'topic_prefix': 'topic_prefix', + } + handler = greenwave.consumers.resultsdb.ResultsDBHandler(hub) message = { 'msg': { 'data': { @@ -76,7 +100,7 @@ def test_announcement_subjects_for_new_compose_message(): } } } - subjects = list(cls.announcement_subjects(message)) + subjects = list(handler.announcement_subjects(message)) assert subjects == [('compose', 'Fedora-Rawhide-20181205.n.0')] @@ -87,7 +111,12 @@ def test_no_announcement_subjects_for_old_compose_message(): https://pagure.io/greenwave/issue/122 etc. So we should NOT produce any subjects for this kind of message. """ - cls = greenwave.consumers.resultsdb.ResultsDBHandler + hub = mock.MagicMock() + hub.config = { + 'environment': 'environment', + 'topic_prefix': 'topic_prefix', + } + handler = greenwave.consumers.resultsdb.ResultsDBHandler(hub) message = { 'msg': { 'task': { @@ -104,7 +133,7 @@ def test_no_announcement_subjects_for_old_compose_message(): } } } - subjects = list(cls.announcement_subjects(message)) + subjects = list(handler.announcement_subjects(message)) assert subjects == [] diff --git a/greenwave/utils.py b/greenwave/utils.py index 05fb680..3b86341 100644 --- a/greenwave/utils.py +++ b/greenwave/utils.py @@ -147,3 +147,16 @@ def sha1_mangle_key(key): to hashlib.sha1()). """ return hashlib.sha1(key.encode('utf-8')).hexdigest() + + +def subject_type_identifier_to_item(subject_type, subject_identifier): + """ + Greenwave < 0.8 included an "item" key in the "unsatisfied_requirements". + This returns a suitable value for that key, for backwards compatibility. + """ + if subject_type in current_app.config['SUBSET_SUBJECT_TYPES']: + return {'type': subject_type, 'item': subject_identifier} + elif subject_type == 'compose': + return {'productmd.compose.id': subject_identifier} + else: + raise RuntimeError('Unrecognised subject type: %s' % subject_type)