From d3083e931c0baff51c3886f9b1512766b4e5b04e Mon Sep 17 00:00:00 2001 From: Yashvardhan Nanavati Date: Apr 23 2019 16:05:36 +0000 Subject: [PATCH 1/2] Add support for on-demand policies The change consists in enhancing the /decision endpoint API to allow a new parameter "rules" that will allow the user to pass some rules. These rules will be immediately processes by Greenwave that will, “on demand”, check the decision (as usually querying ResultsDB and WaiverDB) for those rules and return a response. --- diff --git a/functional-tests/test_api_v1.py b/functional-tests/test_api_v1.py index b33b8d4..9c5bda7 100644 --- a/functional-tests/test_api_v1.py +++ b/functional-tests/test_api_v1.py @@ -97,7 +97,8 @@ def test_cannot_make_decision_without_product_version(requests_session, greenwav @pytest.mark.smoke -def test_cannot_make_decision_without_decision_context(requests_session, greenwave_server): +def test_cannot_make_decision_without_decision_context_and_user_policies( + requests_session, greenwave_server): data = { 'product_version': 'fedora-26', 'subject_type': 'bodhi_update', @@ -107,7 +108,7 @@ def test_cannot_make_decision_without_decision_context(requests_session, greenwa headers={'Content-Type': 'application/json'}, data=json.dumps(data)) assert r.status_code == 400 - assert 'Missing required decision context' == r.json()['message'] + assert 'Either decision_context or rules is required.' == r.json()['message'] @pytest.mark.smoke @@ -1408,3 +1409,176 @@ def test_api_with_when(requests_session, greenwave_server, testdatabuilder): res_data = r.json() assert len(res_data['results']) == 2 + + +@pytest.mark.smoke +def test_cannot_make_decision_with_both_decision_context_and_user_policies( + requests_session, greenwave_server): + data = { + 'product_version': 'fedora-26', + 'subject_type': 'bodhi_update', + 'subject_identifier': 'FEDORA-2018-ec7cb4d5eb', + 'decision_context': 'koji_build_push_missing_results', + 'rules': [ + { + 'type': 'PassingTestCaseRule', + 'test_case_name': 'osci.brew-build.rpmdeplint.functional' + }, + ], + } + r = requests_session.post(greenwave_server + 'api/v1.0/decision', + headers={'Content-Type': 'application/json'}, + data=json.dumps(data)) + assert r.status_code == 400 + assert ('Cannot have both decision_context and rules') == r.json()['message'] + + +@pytest.mark.smoke +def test_cannot_make_decision_without_required_rule_type( + requests_session, greenwave_server): + data = { + 'product_version': 'fedora-26', + 'subject_type': 'bodhi_update', + 'subject_identifier': 'FEDORA-2018-ec7cb4d5eb', + 'rules': [ + { + 'typo': 'PassingTestCaseRule', + 'test_case_name': 'dist.abicheck' + }, + { + 'type': 'PassingTestCaseRule', + 'test_case_name': 'dist.rpmdeplint' + }, + ], + } + r = requests_session.post(greenwave_server + 'api/v1.0/decision', + headers={'Content-Type': 'application/json'}, + data=json.dumps(data)) + assert r.status_code == 400 + assert ('Key \'type\' is required for every rule') == r.json()['message'] + + +@pytest.mark.smoke +def test_cannot_make_decision_without_required_rule_testcase_name( + requests_session, greenwave_server): + data = { + 'product_version': 'fedora-26', + 'subject_type': 'bodhi_update', + 'subject_identifier': 'FEDORA-2018-ec7cb4d5eb', + 'rules': [ + { + 'type': 'PassingTestCaseRule', + 'test_case_name': 'dist.abicheck' + }, + { + 'type': 'PassingTestCaseRule' + }, + ], + } + r = requests_session.post(greenwave_server + 'api/v1.0/decision', + headers={'Content-Type': 'application/json'}, + data=json.dumps(data)) + assert r.status_code == 400 + assert ('Key \'test_case_name\' is required if not a RemoteRule') == r.json()['message'] + + +def test_make_a_decision_with_verbose_flag_on_demand_policy( + requests_session, greenwave_server, testdatabuilder): + nvr = testdatabuilder.unique_nvr() + results = [] + expected_waivers = [] + # First one failed but was waived + results.append(testdatabuilder.create_result(item=nvr, + testcase_name=TASKTRON_RELEASE_CRITICAL_TASKS[0], + outcome='FAILED')) + expected_waivers.append( + testdatabuilder.create_waiver(nvr=nvr, + product_version='fedora-31', + testcase_name=TASKTRON_RELEASE_CRITICAL_TASKS[0], + comment='This is fine')) + for testcase_name in TASKTRON_RELEASE_CRITICAL_TASKS[1:]: + results.append(testdatabuilder.create_result(item=nvr, + testcase_name=testcase_name, + outcome='PASSED')) + + data = { + 'product_version': 'fedora-31', + 'subject_type': 'koji_build', + 'subject_identifier': nvr, + 'verbose': True, + 'rules': [ + { + 'type': 'PassingTestCaseRule', + 'test_case_name': 'dist.abicheck' + }, + { + 'type': 'PassingTestCaseRule', + 'test_case_name': 'dist.rpmdeplint' + }, + { + 'type': 'PassingTestCaseRule', + 'test_case_name': 'dist.upgradepath' + }, + ], + } + r = requests_session.post(greenwave_server + 'api/v1.0/decision', + headers={'Content-Type': 'application/json'}, + data=json.dumps(data)) + assert r.status_code == 200 + res_data = r.json() + + assert len(res_data['results']) == len(results) + assert res_data['results'] == list(reversed(results)) + assert len(res_data['waivers']) == len(expected_waivers) + assert res_data['waivers'] == expected_waivers + assert len(res_data['satisfied_requirements']) == len(results) + assert len(res_data['unsatisfied_requirements']) == 0 + + +def test_make_a_decision_on_demand_policy( + requests_session, greenwave_server, testdatabuilder): + nvr = testdatabuilder.unique_nvr() + results = [] + expected_waivers = [] + # First one failed but was waived + results.append(testdatabuilder.create_result(item=nvr, + testcase_name=TASKTRON_RELEASE_CRITICAL_TASKS[0], + outcome='FAILED')) + expected_waivers.append( + testdatabuilder.create_waiver(nvr=nvr, + product_version='fedora-31', + testcase_name=TASKTRON_RELEASE_CRITICAL_TASKS[0], + comment='This is fine')) + for testcase_name in TASKTRON_RELEASE_CRITICAL_TASKS[1:]: + results.append(testdatabuilder.create_result(item=nvr, + testcase_name=testcase_name, + outcome='PASSED')) + + data = { + 'id': 'on_demand', + 'product_version': 'fedora-31', + 'subject_type': 'koji_build', + 'subject_identifier': nvr, + 'rules': [ + { + 'type': 'PassingTestCaseRule', + 'test_case_name': 'dist.abicheck' + }, + { + 'type': 'PassingTestCaseRule', + 'test_case_name': 'dist.rpmdeplint' + }, + { + 'type': 'PassingTestCaseRule', + 'test_case_name': 'dist.upgradepath' + }, + ], + } + r = requests_session.post(greenwave_server + 'api/v1.0/decision', + headers={'Content-Type': 'application/json'}, + data=json.dumps(data)) + assert r.status_code == 200 + res_data = r.json() + + assert len(res_data['satisfied_requirements']) == len(results) + assert len(res_data['unsatisfied_requirements']) == 0 diff --git a/greenwave/api_v1.py b/greenwave/api_v1.py index af1a7c0..5bd0b49 100644 --- a/greenwave/api_v1.py +++ b/greenwave/api_v1.py @@ -8,6 +8,7 @@ from prometheus_client import generate_latest from greenwave import __version__ from greenwave.policies import (summarize_answers, RemotePolicy, + OnDemandPolicy, _missing_decision_contexts_in_parent_policies) from greenwave.resources import ResultsRetriever, retrieve_waivers from greenwave.safe_yaml import SafeYAMLError @@ -298,23 +299,36 @@ def make_decision(): :statuscode 200: A decision was made. :statuscode 400: Invalid data was given. """ # noqa: E501 - if request.get_json(): - if ('product_version' not in request.get_json() or - not request.get_json()['product_version']): + data = request.get_json() + if data: + if not data.get('product_version'): log.error('Missing required product version') raise BadRequest('Missing required product version') - if ('decision_context' not in request.get_json() or - not request.get_json()['decision_context']): - log.error('Missing required decision context') - raise BadRequest('Missing required decision context') + if not data.get('decision_context') and not data.get('rules'): + log.error('Either decision_context or rules is required.') + raise BadRequest('Either decision_context or rules is required.') else: log.error('No JSON payload in request') raise UnsupportedMediaType('No JSON payload in request') - data = request.get_json() log.debug('New decision request for data: %s', data) product_version = data['product_version'] - decision_context = data['decision_context'] + + decision_context = data.get('decision_context', None) + rules = data.get('rules', []) + if decision_context and rules: + log.error('Cannot have both decision_context and rules') + raise BadRequest('Cannot have both decision_context and rules') + + on_demand_policies = [] + if rules: + request_data = {key: data[key] for key in data if key not in ('subject', 'subject_type')} + for subject_type, subject_identifier in _decision_subjects_for_request(data): + request_data['subject_type'] = subject_type + request_data['subject_identifier'] = subject_identifier + on_demand_policy = OnDemandPolicy.create_from_json(request_data) + on_demand_policies.append(on_demand_policy) + verbose = data.get('verbose', False) if not isinstance(verbose, bool): log.error('Invalid verbose flag, must be a bool') @@ -341,9 +355,10 @@ def make_decision(): verify=current_app.config['REQUESTS_VERIFY'], url=current_app.config['RESULTSDB_API_URL']) + policies = on_demand_policies or current_app.config['policies'] for subject_type, subject_identifier in _decision_subjects_for_request(data): subject_policies = [ - policy for policy in current_app.config['policies'] + policy for policy in policies if policy.matches( decision_context=decision_context, product_version=product_version, @@ -382,13 +397,16 @@ def make_decision(): response = { 'policies_satisfied': all(answer.is_satisfied for answer in answers), 'summary': summarize_answers(answers), - 'applicable_policies': [policy.id for policy in applicable_policies], '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], } + # Check if on-demand policy was specified + if not rules: + response.update({'applicable_policies': [policy.id for policy in applicable_policies]}) + if verbose: # removing duplicated elements... response.update({ diff --git a/greenwave/policies.py b/greenwave/policies.py index e528234..c8bf3b5 100644 --- a/greenwave/policies.py +++ b/greenwave/policies.py @@ -6,6 +6,7 @@ import logging import os import re import greenwave.resources +from werkzeug.exceptions import BadRequest from flask import current_app from greenwave.safe_yaml import ( @@ -305,6 +306,36 @@ class Rule(SafeYAMLObject): """ return True + @staticmethod + def process_on_demand_rules(rules): + """ + 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': + processed_rules.append(RemoteRule()) + elif rule['type'] == 'PassingTestCaseRule': + temp_rule = PassingTestCaseRule() + temp_rule.test_case_name = rule['test_case_name'] # pylint: disable=W0201 + temp_rule.scenario = rule.get('scenario') # pylint: disable=W0201 + processed_rules.append(temp_rule) + else: + raise BadRequest('Invalid rule type {}'.format(rule['type'])) + + return processed_rules + def waives_invalid_gating_yaml(waiver, subject_type, subject_identifier): return (waiver['testcase'] == 'invalid-gating-yaml' and @@ -334,6 +365,12 @@ class RemoteRule(Rule): return [] policies = RemotePolicy.safe_load_all(response) + if isinstance(policy, OnDemandPolicy): + return [ + sub_policy for sub_policy in policies + if set(sub_policy.product_versions) == set(policy.product_versions) + ] + return [ sub_policy for sub_policy in policies if sub_policy.decision_context == policy.decision_context @@ -592,6 +629,49 @@ class Policy(SafeYAMLObject): return 'Policy {!r}'.format(self.id or 'untitled') +class OnDemandPolicy(Policy): + root_yaml_tag = '!Policy' + safe_yaml_attributes = {} + + def __init__(self): + self.id = None + self.product_versions = None + self.subject_type = None + self.rules = None + self.blacklist = None + self.excluded_packages = None + self.packages = None + self.relevance_key = None + + @classmethod + def create_from_json(cls, data_dict): + policy = cls() + policy.id = data_dict.get('id') + policy.product_versions = [data_dict['product_version']] + policy.subject_type = data_dict['subject_type'] + policy.rules = Rule.process_on_demand_rules(data_dict['rules']) + policy.blacklist = data_dict.get('blacklist', []) + policy.excluded_packages = data_dict.get('excluded_packages', []) + policy.packages = data_dict.get('packages', []) + policy.relevance_key = data_dict.get('relevance_key') + + # Validate the data before processing. + policy.__validate_attributes() # pylint: disable=W0212 + return policy + + def __validate_attributes(self): + """ Validates types of the attributes. """ + list_attributes = ['product_versions', 'rules', 'excluded_packages', 'packages'] + for attribute in self.__dict__.keys(): + if attribute in list_attributes and not isinstance( + getattr(self, attribute, None), list): + raise TypeError('{} should be a list.'.format(attribute)) + elif attribute not in list_attributes: + if getattr(self, attribute, None) and not isinstance( + getattr(self, attribute, None), str): + raise TypeError('{} should be a string.'.format(attribute)) + + class RemotePolicy(Policy): root_yaml_tag = '!Policy' diff --git a/greenwave/tests/test_policies.py b/greenwave/tests/test_policies.py index cfbddbb..36a7a2b 100644 --- a/greenwave/tests/test_policies.py +++ b/greenwave/tests/test_policies.py @@ -16,7 +16,8 @@ from greenwave.policies import ( TestResultMissing, TestResultFailed, TestResultPassed, - InvalidGatingYaml + InvalidGatingYaml, + OnDemandPolicy ) from greenwave.resources import ResultsRetriever from greenwave.safe_yaml import SafeYAMLError @@ -889,3 +890,60 @@ def test_policy_with_subject_type_redhat_module(tmpdir): decision = policy.check('fedora-29', nsvc, results, waivers) assert len(decision) == 1 assert isinstance(decision[0], RuleSatisfied) + + +@pytest.mark.parametrize('namespace', ["rpms", ""]) +def test_remote_rule_policy_on_demand_policy(namespace): + """ Testing the RemoteRule with the koji interaction when on_demand policy is given. + In this case we are just mocking koji """ + + nvr = 'nethack-1.2.3-1.el9000' + + serverside_json = { + 'product_version': 'fedora-26', + 'id': 'taskotron_release_critical_tasks_with_remoterule', + 'subject_type': 'koji_build', + 'subject_identifier': nvr, + 'rules': [ + { + 'type': 'RemoteRule' + }, + ], + } + + remote_fragment = dedent(""" + --- !Policy + id: "some-policy-from-a-random-packager" + product_versions: + - fedora-26 + decision_context: bodhi_update_push_stable_with_remoterule + rules: + - !PassingTestCaseRule {test_case_name: dist.upgradepath} + """) + + app = create_app('greenwave.config.TestingConfig') + with app.app_context(): + with mock.patch('greenwave.resources.retrieve_scm_from_koji') as scm: + scm.return_value = (namespace, 'nethack', 'c3c47a08a66451cb9686c49f040776ed35a0d1bb') + with mock.patch('greenwave.resources.retrieve_yaml_remote_rule') as f: + f.return_value = remote_fragment + policy = OnDemandPolicy.create_from_json(serverside_json) # pylint: disable=W0212 + waivers = [] + + # Ensure that presence of a result is success. + results = DummyResultsRetriever(nvr, 'dist.upgradepath') + decision = policy.check('fedora-26', nvr, results, waivers) + assert len(decision) == 1 + assert isinstance(decision[0], RuleSatisfied) + + # Ensure that absence of a result is failure. + results = DummyResultsRetriever() + decision = policy.check('fedora-26', nvr, results, waivers) + assert len(decision) == 1 + assert isinstance(decision[0], TestResultMissing) + + # And that a result with a failure, is a failure. + results = DummyResultsRetriever(nvr, 'dist.upgradepath', 'FAILED') + decision = policy.check('fedora-26', nvr, results, waivers) + assert len(decision) == 1 + assert isinstance(decision[0], TestResultFailed) From f45935525304256ca8c0ed432dac39d14d2f2bc6 Mon Sep 17 00:00:00 2001 From: Yashvardhan Nanavati Date: Apr 24 2019 15:10:49 +0000 Subject: [PATCH 2/2] Add documentation for On-demand policy feature --- diff --git a/greenwave/api_v1.py b/greenwave/api_v1.py index 5bd0b49..636e4bf 100644 --- a/greenwave/api_v1.py +++ b/greenwave/api_v1.py @@ -272,10 +272,59 @@ def make_decision(): ], } + **Sample On-demand policy request**: + + Note: Greenwave would not publish a message on the message bus when an on-demand + policy request is received. + + .. sourcecode:: http + + POST /api/v1.0/decision HTTP/1.1 + Accept: application/json + Content-Type: application/json + + { + "subject_identifier": "cross-gcc-7.0.1-0.3.el8", + "verbose": false, + "subject_type": "koji_build", + "rules": [ + { + "type": "PassingTestCaseRule", + "test_case_name": "fake.testcase.tier0.validation" + } + ], + "product_version": ["rhel-8"], + "excluded_packages": ["python2-*"] + } + + + + **Sample On-demand policy response**: + + .. sourcecode:: none + + HTTP/1.0 200 + Content-Length: 228 + Content-Type: application/json + + { + "policies_satisfied": True, + "satisfied_requirements": [ + { + "result_id": 7403736, + "testcase": "fake.testcase.tier0.validation", + "type": "test-result-passed" + } + ], + "summary": "All required tests passed", + "unsatisfied_requirements": [] + } + :jsonparam string product_version: The product version string used for querying WaiverDB. :jsonparam string decision_context: The decision context string, identified by a free-form string label. It is to be named through coordination between policy author and calling application, for example ``bodhi_update_push_stable``. + Do not use this parameter with `rules`. :jsonparam string subject_type: The type of software artefact we are making a decision about, for example ``koji_build``. See :ref:`subject-types` for a list of possible subject types. @@ -296,6 +345,11 @@ def make_decision(): the decision. :jsonparam string when: A date (or datetime) in ISO8601 format. Greenwave will take a decision considering only results and waivers from that point in time. + :jsonparam list rules: A list of dictionaries containing the 'type' and 'test_case_name' + of an individual rule used to specify on-demand policy. + For example, [{"type":"PassingTestCaseRule", "test_case_name":"dist.abicheck"}, + {"type":"RemoteRule"}] + Do not use this parameter along with `decision_context`. :statuscode 200: A decision was made. :statuscode 400: Invalid data was given. """ # noqa: E501