From 7a45ab3925e7bf8114b35e3cf4146c7364ae6afa Mon Sep 17 00:00:00 2001 From: Giulia Naponiello Date: Dec 17 2018 13:21:36 +0000 Subject: [PATCH 1/2] Merge branch 'master' of ssh://pagure.io/greenwave --- diff --git a/docs/package-specific-policies.rst b/docs/package-specific-policies.rst index 0aba46c..c77a32a 100644 --- a/docs/package-specific-policies.rst +++ b/docs/package-specific-policies.rst @@ -23,14 +23,23 @@ Here is an example :file:`gating.yaml` file: product_versions: - fedora-* decision_context: bodhi_update_push_testing + subject_type: koji_build rules: - !PassingTestCaseRule {test_case_name: dist.depcheck} The structure of the file is the same as the policies in Greenwave's configuration, with the following differences: -* the "id" key is optional -* the "subject_type" shouldn't be defined - the value is always ``koji_build``. +* the "id" key is optional. + +``product_versions``, ``decision_context`` and ``subject_type`` in the +gating.yaml file should match with the defined values in the global +policy defined in the Greenwave conf that contains the ``RemoteRule`` +that will enable this check. + +The "subject_type" should always be defined and the permitted values are +``koji_build`` and ``redhat-module``. If no ``subject_type`` will be +specified the default value is ``koji_build``. Refer to :doc:`policies` for details about each of the keys in the YAML file. @@ -73,6 +82,7 @@ be this one: product_versions: - fedora-28 decision_context: bodhi_update_push_stable + subject_type: koji_build rules: - !PassingTestCaseRule {test_case_name: dist.depcheck} From 998cadaa44624a6f0fb690a179a5272fdc78e46e Mon Sep 17 00:00:00 2001 From: Giulia Naponiello Date: Dec 19 2018 23:49:54 +0000 Subject: [PATCH 2/2] Remove hard-coded subject_types Remove the hard-coded subject_types allowing to use any kind of subject_type. Only some special cases are "hard-coded" just to be handled correctly. --- diff --git a/docs/package-specific-policies.rst b/docs/package-specific-policies.rst index c77a32a..63ab8f4 100644 --- a/docs/package-specific-policies.rst +++ b/docs/package-specific-policies.rst @@ -28,16 +28,14 @@ Here is an example :file:`gating.yaml` file: - !PassingTestCaseRule {test_case_name: dist.depcheck} The structure of the file is the same as the policies in Greenwave's -configuration, with the following differences: - -* the "id" key is optional. +configuration, with the only difference that the "id" key is optional. ``product_versions``, ``decision_context`` and ``subject_type`` in the gating.yaml file should match with the defined values in the global policy defined in the Greenwave conf that contains the ``RemoteRule`` that will enable this check. -The "subject_type" should always be defined and the permitted values are +The ``subject_type`` should always be defined and the permitted values are ``koji_build`` and ``redhat-module``. If no ``subject_type`` will be specified the default value is ``koji_build``. diff --git a/greenwave/api_v1.py b/greenwave/api_v1.py index cfb8493..2418b25 100644 --- a/greenwave/api_v1.py +++ b/greenwave/api_v1.py @@ -22,20 +22,18 @@ def _decision_subject(subject): subject_type = subject.get('type') 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) - if 'productmd.compose.id' in subject: return ('compose', subject['productmd.compose.id']) if 'original_spec_nvr' in subject: return ('koji_build', subject['original_spec_nvr']) - raise BadRequest('Unrecognised subject type: %r' % subject) + if subject_identifier: + if subject_type == 'brew-build': + return ('koji_build', subject_identifier) + return (subject_type, subject_identifier) + + raise BadRequest('Couldn\'t detect subject_identifier.') def _decision_subjects_for_request(data): @@ -70,12 +68,10 @@ 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']: - return [{'type': subject_type, 'item': subject_identifier}] if subject_type == 'compose': return [{'productmd.compose.id': subject_identifier}] - - raise BadRequest('Unrecognised subject type: %s' % subject_type) + else: + return [{'type': subject_type, 'item': subject_identifier}] @api.route('/version', methods=['GET']) diff --git a/greenwave/consumers/resultsdb.py b/greenwave/consumers/resultsdb.py index 0d397c9..7a58834 100644 --- a/greenwave/consumers/resultsdb.py +++ b/greenwave/consumers/resultsdb.py @@ -160,9 +160,6 @@ class ResultsDBHandler(fedmsg.consumers.FedmsgConsumer): return value _type = _decode(data.get('type')) - if _type in ['bodhi_update', 'component-version', 'redhat-module'] and ( - 'item' in data): - yield (_type, _decode(data['item'])) # note: it is *intentional* that we do not handle old format # compose-type messages, because it is impossible to reliably # produce a decision from these. compose decisions can only be @@ -174,18 +171,19 @@ 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']) + elif _type == 'compose': + pass + elif 'original_spec_nvr' in data: + nvr = _decode(data['original_spec_nvr']) # 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 if nvr: yield ('koji_build', nvr) + elif _type == 'brew-build': + yield ('koji_build', _decode(data['item'])) + elif 'item' in data and _type: + yield (_type, _decode(data['item'])) def consume(self, message): """ diff --git a/greenwave/policies.py b/greenwave/policies.py index d4ab83b..1ca9253 100644 --- a/greenwave/policies.py +++ b/greenwave/policies.py @@ -45,12 +45,10 @@ 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': + if subject_type == 'compose': return {'productmd.compose.id': subject_identifier} else: - raise RuntimeError('Unrecognised subject type: %s' % subject_type) + return {'type': subject_type, 'item': subject_identifier} class Answer(object): @@ -549,9 +547,7 @@ class Policy(SafeYAMLObject): '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'), + 'subject_type': SafeYAMLString(), '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..48cebf5 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) @@ -119,14 +115,11 @@ class ResultsRetriever(object): 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': + else: params['type'] = subject_type params['item'] = 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..659145c 100644 --- a/greenwave/tests/test_policies.py +++ b/greenwave/tests/test_policies.py @@ -15,6 +15,7 @@ from greenwave.policies import ( RuleSatisfied, TestResultMissing, TestResultFailed, + TestResultPassed, InvalidGatingYaml ) from greenwave.resources import ResultsRetriever @@ -679,23 +680,26 @@ def test_parse_policies_missing_decision_context(): """)) -def test_parse_policies_invalid_subject_type(): - expected_error = ( - r"Policy 'test': Attribute 'subject_type': " - "Value must be one of:.*" - ) - with pytest.raises(SafeYAMLError, match=expected_error): - Policy.safe_load_all(dedent(""" - --- !Policy - id: test - product_versions: [fedora-rawhide] - decision_context: test - subject_type: bad_subject - blacklist: [] - rules: - - !PassingTestCaseRule {test_case_name: compose.cloud.all} - - 0 +def test_policy_with_arbitrary_subject_type(tmpdir): + p = tmpdir.join('fedora.yaml') + p.write(dedent(""" + --- !Policy + id: "some_policy" + product_versions: + - rhel-9000 + decision_context: bodhi_update_push_stable + subject_type: kind-of-magic + rules: + - !PassingTestCaseRule {test_case_name: sometest} """)) + policies = load_policies(tmpdir.strpath) + policy = policies[0] + + waivers = [] + results = DummyResultsRetriever('nethack-1.2.3-1.el9000', 'sometest', 'PASSED', 'kind-of-magic') + decision = policy.check('rhel-9000', 'nethack-1.2.3-1.el9000', results, waivers) + assert len(decision) == 1 + assert isinstance(decision[0], TestResultPassed) def test_parse_policies_invalid_rule():