From 1c7910d3fb83bdf39dc6a790dc28a00573d48238 Mon Sep 17 00:00:00 2001 From: Dan Callaghan Date: May 24 2018 01:22:21 +0000 Subject: [PATCH 1/6] make Greenwave aware of specific subject types and identifiers Depends on: https://pagure.io/waiverdb/pull-request/175 Fixes #126. --- diff --git a/conf/policies/fedora.yaml b/conf/policies/fedora.yaml index 7761de3..a0d68fe 100644 --- a/conf/policies/fedora.yaml +++ b/conf/policies/fedora.yaml @@ -9,6 +9,7 @@ id: "taskotron_release_critical_tasks_with_blacklist" product_versions: - fedora-26 decision_context: bodhi_update_push_stable +subject_type: koji_build blacklist: # see the excluded list for dist.abicheck # https://infrastructure.fedoraproject.org/cgit/ansible.git/tree/roles/taskotron/taskotron-trigger/templates/trigger_rules.yml.j2#n17 @@ -27,6 +28,7 @@ id: "taskotron_release_critical_tasks_for_testing" product_versions: - fedora-* decision_context: bodhi_update_push_testing +subject_type: koji_build blacklist: [] rules: - !PassingTestCaseRule {test_case_name: dist.rpmdeplint} @@ -35,6 +37,7 @@ id: "taskotron_release_critical_tasks" product_versions: - fedora-26 decision_context: bodhi_update_push_stable +subject_type: koji_build blacklist: [] rules: - !PassingTestCaseRule {test_case_name: dist.rpmdeplint} @@ -44,6 +47,7 @@ id: "openqa_important_stuff_for_rawhide" product_versions: - fedora-rawhide decision_context: rawhide_compose_sync_to_mirrors +subject_type: compose blacklist: [] rules: - !PassingTestCaseRule {test_case_name: compose.install_no_user, scenario: scenario1} @@ -53,6 +57,7 @@ id: "taskotron_release_critical_tasks_with_remoterule" product_versions: - fedora-26 decision_context: bodhi_update_push_stable_with_remoterule +subject_type: bodhi_update blacklist: [] rules: - !RemoteOriginalSpecNvrRule {} @@ -64,5 +69,6 @@ id: "empty-policy" product_versions: - fedora-24 decision_context: bodhi_update_push_stable +subject_type: bodhi_update blacklist: [] rules: [] diff --git a/functional-tests/conftest.py b/functional-tests/conftest.py index 504ffe2..1e715da 100644 --- a/functional-tests/conftest.py +++ b/functional-tests/conftest.py @@ -134,6 +134,25 @@ def waiverdb_server(tmpdir_factory): @pytest.yield_fixture(scope='session') +def bodhi(): + if 'BODHI_TEST_URL' in os.environ: + yield os.environ['BODHI_TEST_URL'] + else: + # Start fake Bodhi as a subprocess + p = subprocess.Popen(['gunicorn', + '--bind=127.0.0.1:5677', + '--access-logfile=-', + '--pythonpath=' + os.path.dirname(__file__), + 'fake_bodhi:application']) + log.debug('Started fake Bodhi as pid %s', p.pid) + wait_for_listen(5677) + yield 'http://localhost:5677/' + log.debug('Terminating fake Bodhi pid %s', p.pid) + p.terminate() + p.wait() + + +@pytest.yield_fixture(scope='session') def distgit_server(tmpdir_factory): """ Creating a fake dist-git process. It is just a serving some files in a tmp dir """ tmp_dir = tmpdir_factory.mktemp('distgit') @@ -149,7 +168,7 @@ def distgit_server(tmpdir_factory): @pytest.yield_fixture(scope='session') -def greenwave_server(tmpdir_factory, resultsdb_server, waiverdb_server): +def greenwave_server(tmpdir_factory, resultsdb_server, waiverdb_server, bodhi): if 'GREENWAVE_TEST_URL' in os.environ: yield os.environ['GREENWAVE_TEST_URL'] else: @@ -164,7 +183,8 @@ def greenwave_server(tmpdir_factory, resultsdb_server, waiverdb_server): } RESULTSDB_API_URL = '%sapi/v2.0' WAIVERDB_API_URL = '%sapi/v1.0' - """ % (cache_file.strpath, resultsdb_server, waiverdb_server))) + BODHI_URL = %r + """ % (cache_file.strpath, resultsdb_server, waiverdb_server, bodhi))) # We also update the config file for *this* process, as well as the server subprocess, # because the fedmsg consumer tests actually invoke the handler code in-process. @@ -200,10 +220,11 @@ class TestDataBuilder(object): ResultsDB and WaiverDB. """ - def __init__(self, requests_session, resultsdb_url, waiverdb_url, distgit_url): + def __init__(self, requests_session, resultsdb_url, waiverdb_url, bodhi_url, distgit_url): self.requests_session = requests_session self.resultsdb_url = resultsdb_url self.waiverdb_url = waiverdb_url + self.bodhi_url = bodhi_url self.distgit_url = distgit_url self._counter = itertools.count(1) @@ -267,7 +288,18 @@ class TestDataBuilder(object): response.raise_for_status() return response.json() + def create_bodhi_update(self, build_nvrs): + data = {'builds': [{'nvr': nvr} for nvr in build_nvrs]} + response = self.requests_session.post( + self.bodhi_url + 'updates/', + headers={'Content-Type': 'application/json'}, + timeout=TEST_HTTP_TIMEOUT, + data=json.dumps(data)) + response.raise_for_status() + return response.json() + @pytest.fixture(scope='session') -def testdatabuilder(requests_session, resultsdb_server, waiverdb_server, distgit_server): - return TestDataBuilder(requests_session, resultsdb_server, waiverdb_server, distgit_server) +def testdatabuilder(requests_session, resultsdb_server, waiverdb_server, bodhi, distgit_server): + return TestDataBuilder(requests_session, resultsdb_server, waiverdb_server, bodhi, + distgit_server) diff --git a/functional-tests/consumers/test_resultsdb.py b/functional-tests/consumers/test_resultsdb.py index d028352..a613fb3 100644 --- a/functional-tests/consumers/test_resultsdb.py +++ b/functional-tests/consumers/test_resultsdb.py @@ -67,6 +67,8 @@ def test_consume_new_result( 'item': nvr, 'type': 'koji_build' }, + 'subject_type': 'koji_build', + 'subject_identifier': nvr, 'type': 'test-result-missing', 'scenario': None, }, @@ -76,6 +78,8 @@ def test_consume_new_result( 'item': nvr, 'type': 'koji_build' }, + 'subject_type': 'koji_build', + 'subject_identifier': nvr, 'type': 'test-result-missing', 'scenario': None, } @@ -87,6 +91,8 @@ def test_consume_new_result( 'type': 'koji_build' } ], + #'subject_type': 'koji_build', + #'subject_identifier': nvr, 'applicable_policies': ['taskotron_release_critical_tasks_with_blacklist', 'taskotron_release_critical_tasks'], 'previous': old_decision, @@ -116,6 +122,8 @@ def test_consume_new_result( 'type': 'koji_build' } ], + #'subject_type': 'koji_build', + #'subject_identifier': nvr, 'applicable_policies': ['taskotron_release_critical_tasks_for_testing'], 'previous': old_decision, } @@ -206,7 +214,7 @@ def test_invalidate_new_result_with_mocked_cache( #'topic_prefix.environment.waiver.new', ] handler.consume(message) - expected = ("greenwave.resources:retrieve_results|" + expected = ("greenwave.resources:retrieve_item_results|" "{u'item': u'%s', u'type': u'koji_build'}" % nvr) handler.cache.delete.assert_called_once_with(expected) @@ -386,10 +394,14 @@ def test_consume_compose_id_result( u'policies_satisfied': False, 'product_version': 'fedora-rawhide', 'subject': [{u'productmd.compose.id': compose_id}], + #'subject_type': 'compose', + #'subject_identifier': compose_id, u'summary': u'1 of 2 required test results missing', 'previous': old_decision, u'unsatisfied_requirements': [{ u'item': {u'productmd.compose.id': compose_id}, + 'subject_type': 'compose', + 'subject_identifier': compose_id, u'scenario': u'scenario2', u'testcase': u'compose.install_no_user', u'type': u'test-result-missing'} @@ -465,6 +477,8 @@ def test_consume_legacy_result( 'item': nvr, 'type': 'koji_build' }, + 'subject_type': 'koji_build', + 'subject_identifier': nvr, 'type': 'test-result-missing', 'scenario': None, }, @@ -474,6 +488,8 @@ def test_consume_legacy_result( 'item': nvr, 'type': 'koji_build' }, + 'subject_type': 'koji_build', + 'subject_identifier': nvr, 'type': 'test-result-missing', 'scenario': None, } @@ -485,6 +501,8 @@ def test_consume_legacy_result( 'type': 'koji_build' } ], + #'subject_type': 'koji_build', + #'subject_identifier': nvr, 'applicable_policies': ['taskotron_release_critical_tasks_with_blacklist', 'taskotron_release_critical_tasks'], 'previous': old_decision, @@ -514,6 +532,8 @@ def test_consume_legacy_result( 'type': 'koji_build' } ], + #'subject_type': 'koji_build', + #'subject_identifier': nvr, 'applicable_policies': ['taskotron_release_critical_tasks_for_testing'], 'previous': old_decision, } diff --git a/functional-tests/fake_bodhi.py b/functional-tests/fake_bodhi.py new file mode 100644 index 0000000..1838775 --- /dev/null +++ b/functional-tests/fake_bodhi.py @@ -0,0 +1,44 @@ + +import re +import json +import hashlib + + +updates = {} #: {id -> update info} + + +def application(environ, start_response): + path_info = environ['PATH_INFO'] + + m = re.match(r'/updates/(.+)$', path_info) + if m: + updateid = m.group(1) + if environ['REQUEST_METHOD'] == 'GET': + if updateid in updates: + start_response('200 OK', [('Content-Type', 'application/json')]) + return [json.dumps({'update': updates[updateid]})] + else: + start_response('404 Not Found', []) + return [] + else: + start_response('405 Method Not Allowed', []) + return [] + + m = re.match(r'/updates/$', path_info) + if m: + if environ['REQUEST_METHOD'] == 'POST': + body = environ['wsgi.input'].read(int(environ['CONTENT_LENGTH'])) + updateid = 'FEDORA-2000-{}'.format(hashlib.sha1(body).hexdigest()[-8:]) + assert updateid not in updates + update = json.loads(body) + update['updateid'] = updateid + updates[updateid] = update + print('Fake Bodhi created new update %r' % update) + start_response('201 Created', [('Content-Type', 'application/json')]) + return [json.dumps(update)] # XXX check what Bodhi really returns + else: + start_response('405 Method Not Allowed', []) + return [] + + start_response('404 Not Found', []) + return [] diff --git a/functional-tests/test_api_v1.py b/functional-tests/test_api_v1.py index e6aa784..fb1f194 100644 --- a/functional-tests/test_api_v1.py +++ b/functional-tests/test_api_v1.py @@ -59,7 +59,8 @@ def test_version_endpoint_jsonp(requests_session, greenwave_server): def test_cannot_make_decision_without_product_version(requests_session, greenwave_server): data = { 'decision_context': 'bodhi_update_push_stable', - 'subject': [{'item': 'foo-1.0.0-1.el7', 'type': 'koji_build'}] + 'subject_type': 'bodhi_update', + 'subject_identifier': 'FEDORA-2018-ec7cb4d5eb', } r = requests_session.post(greenwave_server + 'api/v1.0/decision', headers={'Content-Type': 'application/json'}, @@ -71,7 +72,8 @@ def test_cannot_make_decision_without_product_version(requests_session, greenwav def test_cannot_make_decision_without_decision_context(requests_session, greenwave_server): data = { 'product_version': 'fedora-26', - 'subject': [{'item': 'foo-1.0.0-1.el7', 'type': 'koji_build'}] + 'subject_type': 'bodhi_update', + 'subject_identifier': 'FEDORA-2018-ec7cb4d5eb', } r = requests_session.post(greenwave_server + 'api/v1.0/decision', headers={'Content-Type': 'application/json'}, @@ -80,16 +82,30 @@ def test_cannot_make_decision_without_decision_context(requests_session, greenwa assert u'Missing required decision context' == r.json()['message'] -def test_cannot_make_decision_without_subject(requests_session, greenwave_server): +def test_cannot_make_decision_without_subject_type(requests_session, greenwave_server): data = { 'decision_context': 'bodhi_update_push_stable', 'product_version': 'fedora-26', + 'subject_identifier': 'FEDORA-2018-ec7cb4d5eb', } 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 u'Missing required subject' == r.json()['message'] + assert u'Missing required "subject_type" parameter' == r.json()['message'] + + +def test_cannot_make_decision_without_subject_identifier(requests_session, greenwave_server): + data = { + 'decision_context': 'bodhi_update_push_stable', + 'product_version': 'fedora-26', + 'subject_type': 'bodhi_update', + } + 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 u'Missing required "subject_identifier" parameter' == r.json()['message'] def test_cannot_make_decision_with_invalid_subject(requests_session, greenwave_server): @@ -102,7 +118,7 @@ def test_cannot_make_decision_with_invalid_subject(requests_session, greenwave_s headers={'Content-Type': 'application/json'}, data=json.dumps(data)) assert r.status_code == 400 - assert 'Invalid subject, must be a list of items' == r.json()['message'] + assert 'Invalid subject, must be a list of dicts' == r.json()['message'] data = { 'decision_context': 'bodhi_update_push_stable', @@ -113,34 +129,40 @@ def test_cannot_make_decision_with_invalid_subject(requests_session, greenwave_s headers={'Content-Type': 'application/json'}, data=json.dumps(data)) assert r.status_code == 400 - assert u'Invalid subject, must be a list of dicts' in r.text + assert 'Invalid subject, must be a list of dicts' == r.json()['message'] -def test_404_for_invalid_product_version(requests_session, greenwave_server): +def test_404_for_invalid_product_version(requests_session, greenwave_server, testdatabuilder): + update = testdatabuilder.create_bodhi_update(build_nvrs=[testdatabuilder.unique_nvr()]) data = { 'decision_context': 'bodhi_push_update_stable', 'product_version': 'f26', # not a real product version - 'subject': [{'item': 'foo-1.0.0-1.el7', 'type': 'koji_build'}] + 'subject_type': 'bodhi_update', + 'subject_identifier': update['updateid'], } r = requests_session.post(greenwave_server + 'api/v1.0/decision', headers={'Content-Type': 'application/json'}, data=json.dumps(data)) assert r.status_code == 404 - expected = u'Cannot find any applicable policies for f26 and bodhi_push_update_stable' + expected = (u'Cannot find any applicable policies for bodhi_update subjects ' + u'at gating point bodhi_push_update_stable in f26') assert expected == r.json()['message'] -def test_404_for_invalid_decision_context(requests_session, greenwave_server): +def test_404_for_invalid_decision_context(requests_session, greenwave_server, testdatabuilder): + update = testdatabuilder.create_bodhi_update(build_nvrs=[testdatabuilder.unique_nvr()]) data = { 'decision_context': 'bodhi_push_update', # missing the _stable part! 'product_version': 'fedora-26', - 'subject': [{'item': 'foo-1.0.0-1.el7', 'type': 'koji_build'}] + 'subject_type': 'bodhi_update', + 'subject_identifier': update['updateid'], } r = requests_session.post(greenwave_server + 'api/v1.0/decision', headers={'Content-Type': 'application/json'}, data=json.dumps(data)) assert r.status_code == 404 - expected = u'Cannot find any applicable policies for fedora-26 and bodhi_push_update' + expected = (u'Cannot find any applicable policies for bodhi_update subjects ' + u'at gating point bodhi_push_update in fedora-26') assert expected == r.json()['message'] @@ -166,10 +188,12 @@ def test_make_a_decision_on_passed_result(requests_session, greenwave_server, te testdatabuilder.create_result(item=nvr, testcase_name=testcase_name, outcome='PASSED') + update = testdatabuilder.create_bodhi_update(build_nvrs=[nvr]) data = { 'decision_context': 'bodhi_update_push_stable', 'product_version': 'fedora-26', - 'subject': [{'item': nvr, 'type': 'koji_build'}] + 'subject_type': 'bodhi_update', + 'subject_identifier': update['updateid'], } r = requests_session.post(greenwave_server + 'api/v1.0/decision', @@ -193,10 +217,12 @@ def test_make_a_decision_with_verbose_flag(requests_session, greenwave_server, t results.append(testdatabuilder.create_result(item=nvr, testcase_name=testcase_name, outcome='PASSED')) + update = testdatabuilder.create_bodhi_update(build_nvrs=[nvr]) data = { 'decision_context': 'bodhi_update_push_stable', 'product_version': 'fedora-26', - 'subject': [{'item': nvr, 'type': 'koji_build'}], + 'subject_type': 'bodhi_update', + 'subject_identifier': update['updateid'], 'verbose': True, } @@ -228,10 +254,12 @@ def test_make_a_decision_on_failed_result_with_waiver( testdatabuilder.create_result(item=nvr, testcase_name=testcase_name, outcome='PASSED') + update = testdatabuilder.create_bodhi_update(build_nvrs=[nvr]) data = { 'decision_context': 'bodhi_update_push_stable', 'product_version': 'fedora-26', - 'subject': [{'item': nvr, 'type': 'koji_build'}] + 'subject_type': 'bodhi_update', + 'subject_identifier': update['updateid'], } r = requests_session.post(greenwave_server + 'api/v1.0/decision', headers={'Content-Type': 'application/json'}, @@ -250,10 +278,12 @@ def test_make_a_decision_on_failed_result(requests_session, greenwave_server, te result = testdatabuilder.create_result(item=nvr, testcase_name=TASKTRON_RELEASE_CRITICAL_TASKS[0], outcome='FAILED') + update = testdatabuilder.create_bodhi_update(build_nvrs=[nvr]) data = { 'decision_context': 'bodhi_update_push_stable', 'product_version': 'fedora-26', - 'subject': [{'item': nvr, 'type': 'koji_build'}] + 'subject_type': 'bodhi_update', + 'subject_identifier': update['updateid'], } r = requests_session.post(greenwave_server + 'api/v1.0/decision', headers={'Content-Type': 'application/json'}, @@ -276,6 +306,8 @@ def test_make_a_decision_on_failed_result(requests_session, greenwave_server, te ] + [ { 'item': {'item': nvr, 'type': 'koji_build'}, + 'subject_type': 'koji_build', + 'subject_identifier': nvr, 'testcase': name, 'type': 'test-result-missing', 'scenario': None, @@ -286,10 +318,12 @@ def test_make_a_decision_on_failed_result(requests_session, greenwave_server, te def test_make_a_decision_on_no_results(requests_session, greenwave_server, testdatabuilder): nvr = testdatabuilder.unique_nvr() + update = testdatabuilder.create_bodhi_update(build_nvrs=[nvr]) data = { 'decision_context': 'bodhi_update_push_stable', 'product_version': 'fedora-26', - 'subject': [{'item': nvr, 'type': 'koji_build'}] + 'subject_type': 'bodhi_update', + 'subject_identifier': update['updateid'], } r = requests_session.post(greenwave_server + 'api/v1.0/decision', headers={'Content-Type': 'application/json'}, @@ -304,6 +338,8 @@ def test_make_a_decision_on_no_results(requests_session, greenwave_server, testd expected_unsatisfied_requirements = [ { 'item': {'item': nvr, 'type': 'koji_build'}, + 'subject_type': 'koji_build', + 'subject_identifier': nvr, 'testcase': name, 'type': 'test-result-missing', 'scenario': None, @@ -315,10 +351,12 @@ def test_make_a_decision_on_no_results(requests_session, greenwave_server, testd def test_empty_policy_is_always_satisfied( requests_session, greenwave_server, testdatabuilder): nvr = testdatabuilder.unique_nvr() + update = testdatabuilder.create_bodhi_update(build_nvrs=[nvr]) data = { 'decision_context': 'bodhi_update_push_stable', 'product_version': 'fedora-24', - 'subject': [{'item': nvr, 'type': 'koji_build'}] + 'subject_type': 'bodhi_update', + 'subject_identifier': update['updateid'], } r = requests_session.post(greenwave_server + 'api/v1.0/decision', headers={'Content-Type': 'application/json'}, @@ -339,10 +377,12 @@ def test_bodhi_push_update_stable_policy( testdatabuilder.create_result(item=nvr, testcase_name=testcase_name, outcome='PASSED') + update = testdatabuilder.create_bodhi_update(build_nvrs=[nvr]) data = { 'decision_context': 'bodhi_update_push_stable', 'product_version': 'fedora-26', - 'subject': [{'item': nvr, 'type': 'koji_build'}] + 'subject_type': 'bodhi_update', + 'subject_identifier': update['updateid'], } r = requests_session.post(greenwave_server + 'api/v1.0/decision', headers={'Content-Type': 'application/json'}, @@ -376,10 +416,12 @@ def test_multiple_results_in_a_subject( testdatabuilder.create_result(item=nvr, testcase_name=testcase_name, outcome='PASSED') + update = testdatabuilder.create_bodhi_update(build_nvrs=[nvr]) data = { 'decision_context': 'bodhi_update_push_stable', 'product_version': 'fedora-26', - 'subject': [{'item': nvr, 'type': 'koji_build'}] + 'subject_type': 'bodhi_update', + 'subject_identifier': update['updateid'], } r = requests_session.post(greenwave_server + 'api/v1.0/decision', headers={'Content-Type': 'application/json'}, @@ -416,10 +458,12 @@ def test_ignore_result(requests_session, greenwave_server, testdatabuilder): testdatabuilder.create_result(item=nvr, testcase_name=testcase_name, outcome='PASSED') + update = testdatabuilder.create_bodhi_update(build_nvrs=[nvr]) data = { 'decision_context': 'bodhi_update_push_stable', 'product_version': 'fedora-26', - 'subject': [{'item': nvr, 'type': 'koji_build'}] + 'subject_type': 'bodhi_update', + 'subject_identifier': update['updateid'], } r = requests_session.post(greenwave_server + 'api/v1.0/decision', headers={'Content-Type': 'application/json'}, @@ -437,6 +481,8 @@ def test_ignore_result(requests_session, greenwave_server, testdatabuilder): expected_unsatisfied_requirements = [ { 'item': {'item': nvr, 'type': 'koji_build'}, + 'subject_type': 'koji_build', + 'subject_identifier': nvr, 'testcase': TASKTRON_RELEASE_CRITICAL_TASKS[0], 'type': 'test-result-missing', 'scenario': None, @@ -464,7 +510,8 @@ def test_make_a_decision_on_passed_result_with_scenario( data = { 'decision_context': 'rawhide_compose_sync_to_mirrors', 'product_version': 'fedora-rawhide', - 'subject': [{'productmd.compose.id': compose_id}], + 'subject_type': 'compose', + 'subject_identifier': compose_id, } r = requests_session.post(greenwave_server + 'api/v1.0/decision', headers={'Content-Type': 'application/json'}, @@ -500,7 +547,8 @@ def test_make_a_decision_on_failing_result_with_scenario( data = { 'decision_context': 'rawhide_compose_sync_to_mirrors', 'product_version': 'fedora-rawhide', - 'subject': [{'productmd.compose.id': compose_id}], + 'subject_type': 'compose', + 'subject_identifier': compose_id, } r = requests_session.post(greenwave_server + 'api/v1.0/decision', headers={'Content-Type': 'application/json'}, @@ -538,10 +586,12 @@ def test_ignore_waiver(requests_session, greenwave_server, testdatabuilder): testdatabuilder.create_result(item=nvr, testcase_name=testcase_name, outcome='PASSED') + update = testdatabuilder.create_bodhi_update(build_nvrs=[nvr]) data = { 'decision_context': 'bodhi_update_push_stable', 'product_version': 'fedora-26', - 'subject': [{'item': nvr, 'type': 'koji_build'}] + 'subject_type': 'bodhi_update', + 'subject_identifier': update['updateid'], } r_ = requests_session.post(greenwave_server + 'api/v1.0/decision', headers={'Content-Type': 'application/json'}, @@ -595,10 +645,12 @@ def test_cached_false_positive(requests_session, greenwave_server, testdatabuild testdatabuilder.create_result(item=nvr, testcase_name=testcase_name, outcome='PASSED') + update = testdatabuilder.create_bodhi_update(build_nvrs=[nvr]) data = { 'decision_context': 'bodhi_update_push_stable', 'product_version': 'fedora-26', - 'subject': [{'item': nvr, 'type': 'koji_build'}] + 'subject_type': 'bodhi_update', + 'subject_identifier': update['updateid'], } r = requests_session.post(greenwave_server + 'api/v1.0/decision', headers={'Content-Type': 'application/json'}, @@ -632,10 +684,12 @@ def test_blacklist(requests_session, greenwave_server, testdatabuilder): testdatabuilder.create_result(item=nvr, testcase_name=testcase_name, outcome='PASSED') + update = testdatabuilder.create_bodhi_update(build_nvrs=[nvr]) data = { 'decision_context': 'bodhi_update_push_stable', 'product_version': 'fedora-26', - 'subject': [{'item': nvr, 'type': 'koji_build'}] + 'subject_type': 'bodhi_update', + 'subject_identifier': update['updateid'], } r = requests_session.post(greenwave_server + 'api/v1.0/decision', headers={'Content-Type': 'application/json'}, diff --git a/greenwave/api_v1.py b/greenwave/api_v1.py index 69fa222..f3e2f7e 100644 --- a/greenwave/api_v1.py +++ b/greenwave/api_v1.py @@ -4,12 +4,43 @@ from flask import Blueprint, request, current_app, jsonify from werkzeug.exceptions import BadRequest, NotFound, UnsupportedMediaType, InternalServerError from greenwave import __version__ from greenwave.policies import summarize_answers, RemoteOriginalSpecNvrRule -from greenwave.resources import retrieve_results, retrieve_waivers +from greenwave.resources import retrieve_results, retrieve_waivers, retrieve_builds_in_update from greenwave.utils import insert_headers, jsonp api = (Blueprint('api_v1', __name__)) +def subject_list_to_type_identifier(subject): + """ + Greenwave < 0.8 accepted a list of arbitrary dicts for the 'subject'. + Now we expect a specific type and identifier. + This maps from the old style to the new, for backwards compatibility. + + Note that WaiverDB has a very similar helper function, for compatibility + with WaiverDB < 0.11, but it accepts a single subject dict. Here we accept + a list. + """ + if (not isinstance(subject, list) or not subject or + not all(isinstance(entry, dict) for entry in subject)): + raise BadRequest('Invalid subject, must be a list of dicts') + if any(entry.get('type') == 'bodhi_update' and 'item' in entry for entry in subject): + # Assume that all the other entries in the list are just for the + # builds which are in the Bodhi update. So really, the caller wants a + # decision about the Bodhi update. Ignore everything else. (Is this + # wise? Maybe not...) + identifier = [entry for entry in subject if entry.get('type') == 'bodhi_update'][0]['item'] + return ('bodhi_update', identifier) + if len(subject) == 1 and 'productmd.compose.id' in subject[0]: + return ('compose', subject[0]['productmd.compose.id']) + # We don't know of any callers who would ask about subjects like this, + # but it's easy enough to handle here anyway: + if len(subject) == 1 and subject[0].get('type') == 'koji_build' and 'item' in subject[0]: + return ('koji_build', subject[0]['item']) + if len(subject) == 1 and 'original_spec_nvr' in subject[0]: + return ('koji_build', subject[0]['original_spec_nvr']) + raise BadRequest('Unrecognised subject type: %r' % subject) + + @api.route('/version', methods=['GET']) @jsonp def version(): @@ -212,14 +243,22 @@ def make_decision(): if ('decision_context' not in request.get_json() or not request.get_json()['decision_context']): raise BadRequest('Missing required decision context') - if ('subject' not in request.get_json() or - not request.get_json()['subject']): - raise BadRequest('Missing required subject') else: raise UnsupportedMediaType('No JSON payload in request') data = request.get_json() - if not isinstance(data['subject'], list): - raise BadRequest('Invalid subject, must be a list of items') + + # Greenwave < 0.8 + if 'subject' in data: + data['subject_type'], data['subject_identifier'] = \ + subject_list_to_type_identifier(data['subject']) + + if 'subject_type' not in data: + raise BadRequest('Missing required "subject_type" parameter') + if 'subject_identifier' not in data: + raise BadRequest('Missing required "subject_identifier" parameter') + + subject_type = data['subject_type'] + subject_identifier = data['subject_identifier'] product_version = data['product_version'] decision_context = data['decision_context'] verbose = data.get('verbose', False) @@ -239,32 +278,36 @@ def make_decision(): "'DIST_GIT_URL_TEMPLATE' and KOJI_BASE_URL in " "your configuration.") - applicable_policies = [policy for policy in current_app.config['policies'] - if policy.applies_to(decision_context, product_version)] + subject_policies = [policy for policy in current_app.config['policies'] + if policy.applies_to(decision_context, product_version, subject_type)] + if subject_type == 'bodhi_update': + # Also need to apply policies for each build in the update. + build_policies = [policy for policy in current_app.config['policies'] + if policy.applies_to(decision_context, product_version, 'koji_build')] + else: + build_policies = [] + applicable_policies = subject_policies + build_policies if not applicable_policies: raise NotFound( - 'Cannot find any applicable policies for %s and %s' % ( - product_version, decision_context)) - subjects = [item for item in data['subject'] if isinstance(item, dict)] - if not subjects: - raise BadRequest('Invalid subject, must be a list of dicts') - - waivers = retrieve_waivers(product_version, subjects) - waivers = [w for w in waivers if w['id'] not in ignore_waivers] + 'Cannot find any applicable policies for %s subjects at gating point %s in %s' % ( + subject_type, decision_context, product_version)) - results = [] answers = [] - for item in subjects: - item_results = retrieve_results(item) - item_results = [r for r in item_results if r['id'] not in ignore_results] - results.extend(item_results) - - subject_subset = set(item.items()) - item_waivers = [w for w in waivers if subject_subset <= set(w['subject'].items())] - - for policy in applicable_policies: - if policy.is_relevant_to(item): - answers.extend(policy.check(item, item_results, item_waivers)) + results = retrieve_results(subject_type, subject_identifier) + results = [r for r in results if r['id'] not in ignore_results] + waivers = retrieve_waivers(product_version, subject_type, subject_identifier) + waivers = [w for w in waivers if w['id'] not in ignore_waivers] + for policy in subject_policies: + answers.extend(policy.check(subject_identifier, results, waivers)) + if build_policies: + build_nvrs = retrieve_builds_in_update(subject_identifier) + for nvr in build_nvrs: + results = retrieve_results('koji_build', nvr) + results = [r for r in results if r['id'] not in ignore_results] + waivers = retrieve_waivers(product_version, 'koji_build', nvr) + waivers = [w for w in waivers if w['id'] not in ignore_waivers] + for policy in build_policies: + answers.extend(policy.check(nvr, results, waivers)) res = { 'policies_satisfied': all(answer.is_satisfied for answer in answers), diff --git a/greenwave/config.py b/greenwave/config.py index e28dcce..13646dd 100644 --- a/greenwave/config.py +++ b/greenwave/config.py @@ -21,6 +21,7 @@ class Config(object): DIST_GIT_BASE_URL = 'https://src.fedoraproject.org' DIST_GIT_URL_TEMPLATE = '{DIST_GIT_BASE_URL}/{pkg_name}/{rev}/gating.yaml' KOJI_BASE_URL = 'https://koji.fedoraproject.org/kojihub' + BODHI_URL = 'https://bodhi.fedoraproject.org/' REQUESTS_TIMEOUT = (6.1, 15) REQUESTS_VERIFY = True diff --git a/greenwave/consumers/resultsdb.py b/greenwave/consumers/resultsdb.py index 5fb2610..77aa778 100644 --- a/greenwave/consumers/resultsdb.py +++ b/greenwave/consumers/resultsdb.py @@ -177,7 +177,7 @@ class ResultsDBHandler(fedmsg.consumers.FedmsgConsumer): subject (munch.Munch): A subject argument, used to query greenwave. """ namespace = None - fn = greenwave.resources.retrieve_results + fn = greenwave.resources.retrieve_item_results key = greenwave.cache.key_generator(namespace, fn)(subject) if not self.cache.get(key): log.debug("No cache value found for %r", key) diff --git a/greenwave/policies.py b/greenwave/policies.py index daaee8d..c67b346 100644 --- a/greenwave/policies.py +++ b/greenwave/policies.py @@ -11,6 +11,10 @@ def validate_policies(policies, disallowed_rules=None): if not isinstance(policy, Policy): raise RuntimeError('Policies are not configured properly as policy %s ' 'is not an instance of Policy' % policy) + for required_attribute in ['decision_context', 'product_versions', 'subject_type']: + if not hasattr(policy, required_attribute): + raise RuntimeError('Policies are not configured properly as policy %s ' + 'is missing attribute %s' % (policy.id, required_attribute)) for rule in policy.rules: if not isinstance(rule, Rule): raise RuntimeError('Policies are not configured properly as rule %s ' @@ -21,6 +25,21 @@ def validate_policies(policies, disallowed_rules=None): 'is an instance of %s' % (rule, disallowed_rule)) +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 == 'bodhi_update': + return {'type': 'bodhi_update', 'item': subject_identifier} + elif subject_type == 'koji_build': + return {'type': 'koji_build', '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 @@ -65,17 +84,21 @@ class TestResultMissing(RuleNotSatisfied): ResultsDB with a matching item and test case name). """ - def __init__(self, item, test_case_name, scenario): - self.item = item + def __init__(self, subject_type, subject_identifier, test_case_name, scenario): + self.subject_type = subject_type + self.subject_identifier = subject_identifier self.test_case_name = test_case_name self.scenario = scenario def to_json(self): return { 'type': 'test-result-missing', - 'item': self.item, 'testcase': self.test_case_name, + 'subject_type': self.subject_type, + 'subject_identifier': self.subject_identifier, 'scenario': self.scenario, + # For backwards compatibility only: + 'item': subject_type_identifier_to_item(self.subject_type, self.subject_identifier), } @@ -85,8 +108,9 @@ class TestResultFailed(RuleNotSatisfied): not ``PASSED`` or ``INFO``) and no corresponding waiver was found. """ - def __init__(self, item, test_case_name, scenario, result_id): - self.item = item + def __init__(self, subject_type, subject_identifier, test_case_name, scenario, result_id): + self.subject_type = subject_type + self.subject_identifier = subject_identifier self.test_case_name = test_case_name self.scenario = scenario self.result_id = result_id @@ -94,10 +118,13 @@ class TestResultFailed(RuleNotSatisfied): def to_json(self): return { 'type': 'test-result-failed', - 'item': self.item, 'testcase': self.test_case_name, - 'scenario': self.scenario, 'result_id': self.result_id, + # 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), + 'scenario': self.scenario, } @@ -135,14 +162,15 @@ class Rule(yaml.YAMLObject): This base class is not used directly. """ - def check(self, item, results, waivers): + def check(self, subject_type, subject_identifier, results, waivers): """ Evaluate this policy rule for the given item. Args: - item (dict): The item we are evaluating (one or more key-value pairs - of 'data' key in ResultsDB, for example {"type": "koji_build", - "item": "xscreensaver-5.37-3.fc27"}). + subject_type (str): Type of thing we are making a decision about + (for example, 'koji_build', 'bodhi_update', ...) + subject_identifier (str): Item we are making a decision about (for + example, Koji build NVR, Bodhi update id, ...) results (list): List of result objects looked up in ResultsDB for this item. waivers (list): List of waiver objects looked up in WaiverDB for the results. @@ -164,9 +192,12 @@ class RemoteOriginalSpecNvrRule(Rule): yaml_tag = u'!RemoteOriginalSpecNvrRule' yaml_loader = yaml.SafeLoader - def check(self, item, results, waivers): - pkg_name = item['original_spec_nvr'].rsplit('-', 2)[0] - rev = greenwave.resources.retrieve_rev_from_koji(item['original_spec_nvr']) + def check(self, subject_type, subject_identifier, results, waivers): + if subject_type != 'koji_build': + return RuleSatisfied() + + pkg_name = subject_identifier.rsplit('-', 2)[0] + rev = greenwave.resources.retrieve_rev_from_koji(subject_identifier) response = greenwave.resources.retrieve_yaml_remote_original_spec_nvr_rule(rev, pkg_name) if isinstance(response, RuleSatisfied): @@ -176,10 +207,13 @@ class RemoteOriginalSpecNvrRule(Rule): policies = yaml.safe_load_all(response) # policies is a generator, so listifying it policies = list(policies) + # policies in dist-git are always about a package + for policy in policies: + policy.subject_type = 'koji_build' validate_policies(policies, [RemoteOriginalSpecNvrRule]) answers = [] for policy in policies: - response = policy.check(item, results, waivers) + response = policy.check(subject_identifier, results, waivers) if isinstance(response, list): answers.extend(response) else: @@ -200,7 +234,7 @@ class PassingTestCaseRule(Rule): yaml_tag = u'!PassingTestCaseRule' yaml_loader = yaml.SafeLoader - def check(self, item, results, waivers): + def check(self, subject_type, subject_identifier, results, waivers): matching_results = [ r for r in results if r['testcase']['name'] == self.test_case_name] matching_waivers = [ @@ -214,7 +248,8 @@ class PassingTestCaseRule(Rule): # Investigate the absence of results first. if not matching_results: if not matching_waivers: - return TestResultMissing(item, self.test_case_name, self._scenario) + return TestResultMissing(subject_type, subject_identifier, self.test_case_name, + self._scenario) else: # The result is absent, but the absence is waived. return RuleSatisfied() @@ -232,7 +267,8 @@ class PassingTestCaseRule(Rule): w['testcase'] == matching_result['testcase']['name'] and w['waived'] for w in waivers): return RuleSatisfied() - return TestResultFailed(item, self.test_case_name, self._scenario, matching_result['id']) + return TestResultFailed(subject_type, subject_identifier, self.test_case_name, + self._scenario, matching_result['id']) @property def _scenario(self): @@ -262,31 +298,31 @@ class PackageSpecificRule(Rule): self.test_case_name = test_case_name self.repos = repos - def check(self, item, results, waivers): - """ Check that the item passes testcase for the given results, but - only if the item is an instance of a package name configured for + def check(self, subject_type, subject_identifier, results, waivers): + """ Check that the subject passes testcase for the given results, but + only if the subject is a build of a package name configured for this rule, specified by "repos". Any of the repos may be a glob. - Items which do not bear the "nvr_key" for this rule are considered - satisfied (ignored). + If this rule is used in a policy for some subject type other than + "koji_build" (which makes no sense), the rule is considered satisfied + (ignored). - Items whose package names (extracted from their NVR) do not match + Subjects whose package names (extracted from their NVR) do not match any of the globs in the "repos" list of this rule are considered satisfied (ignored). """ - if self.nvr_key not in item: + if subject_type != 'koji_build': return RuleSatisfied() - nvr = item[self.nvr_key] - pkg_name = nvr.rsplit('-', 2)[0] + pkg_name = subject_identifier.rsplit('-', 2)[0] if not any(fnmatch(pkg_name, repo) for repo in self.repos): return RuleSatisfied() rule = PassingTestCaseRule() # pylint: disable=attribute-defined-outside-init rule.test_case_name = self.test_case_name - return rule.check(item, results, waivers) + return rule.check(subject_type, subject_identifier, results, waivers) def __repr__(self): return "%s(test_case_name=%s, repos=%r)" % ( @@ -297,55 +333,37 @@ class PackageSpecificRule(Rule): 'rule': self.__class__.__name__, 'test_case_name': self.test_case_name, 'repos': self.repos, - 'nvr_key': self.nvr_key, } class FedoraAtomicCi(PackageSpecificRule): yaml_tag = u'!FedoraAtomicCi' yaml_loader = yaml.SafeLoader - nvr_key = 'original_spec_nvr' class PackageSpecificBuild(PackageSpecificRule): yaml_tag = u'!PackageSpecificBuild' yaml_loader = yaml.SafeLoader - nvr_key = 'item' class Policy(yaml.YAMLObject): yaml_tag = u'!Policy' yaml_loader = yaml.SafeLoader - def applies_to(self, decision_context, product_version): + def applies_to(self, decision_context, product_version, subject_type): return (decision_context == self.decision_context and - self._applies_to_product_version(product_version)) - - def is_relevant_to(self, item): - relevance_key = getattr(self, 'relevance_key', None) - relevance_value = getattr(self, 'relevance_value', None) - - if relevance_key and relevance_value: - return item.get(relevance_key) == relevance_value - - if relevance_key: - return relevance_key in item - - if relevance_value: - return relevance_value in item.values() - - return True + self._applies_to_product_version(product_version) and + subject_type == self.subject_type) - def check(self, item, results, waivers): + def check(self, subject_identifier, results, waivers): # If an item is about a package and it is in the blacklist, return RuleSatisfied() - for package in self.blacklist: - if (item.get('type') == 'koji_build' and - item.get('item') and - item['item'].rsplit('-', 2)[0] == package): + if self.subject_type == 'koji_build': + name = subject_identifier.rsplit('-', 2)[0] + if name in self.blacklist: return [RuleSatisfied() for rule in self.rules] answers = [] for rule in self.rules: - response = rule.check(item, results, waivers) + response = rule.check(self.subject_type, subject_identifier, results, waivers) if isinstance(response, list): answers.extend(response) else: @@ -353,15 +371,16 @@ class Policy(yaml.YAMLObject): return answers def __repr__(self): - return "%s(id=%r, product_versions=%r, decision_context=%r, rules=%r)" % ( + return "%s(id=%r, product_versions=%r, decision_context=%r, subject_type=%r, rules=%r)" % ( self.__class__.__name__, self.id, self.product_versions, self.decision_context, - self.rules) + self.subject_type, self.rules) def to_json(self): return { 'id': self.id, 'product_versions': self.product_versions, 'decision_context': self.decision_context, + 'subject_type': self.subject_type, 'rules': [rule.to_json() for rule in self.rules], 'blacklist': self.blacklist, 'relevance_key': getattr(self, 'relevance_key', None), diff --git a/greenwave/resources.py b/greenwave/resources.py index 9bc2731..c96e35c 100644 --- a/greenwave/resources.py +++ b/greenwave/resources.py @@ -67,11 +67,34 @@ def retrieve_yaml_remote_original_spec_nvr_rule(rev, pkg_name): return response.content +def retrieve_builds_in_update(update_id): + """ + Queries Bodhi to find the list of builds in the given update. + Returns a list of build NVRs. + """ + update_info_url = urlparse.urljoin(current_app.config['BODHI_URL'], + '/updates/{}'.format(update_id)) + timeout = current_app.config['REQUESTS_TIMEOUT'] + verify = current_app.config['REQUESTS_VERIFY'] + response = requests_session.get(update_info_url, + headers={'Accept': 'application/json'}, + timeout=timeout, verify=verify) + response.raise_for_status() + return [build['nvr'] for build in response.json()['update']['builds']] + + @cached -def retrieve_results(item): +def retrieve_item_results(item): """ Retrieve cached results from resultsdb for a given item. """ # XXX make this more efficient than just fetching everything + # Types matter here because of the cache decorator! + # While we are still on Python 2 we must ensure the args are unicode not str + assert isinstance(item, dict) + for k, v in item.iteritems(): + assert isinstance(k, unicode) + assert isinstance(v, unicode) + params = item.copy() params.update({'limit': '1000'}) timeout = current_app.config['REQUESTS_TIMEOUT'] @@ -83,21 +106,40 @@ def retrieve_results(item): return response.json()['data'] +def retrieve_results(subject_type, subject_identifier): + """ + Returns all results from ResultsDB which might be relevant for the given + decision subject, accounting for all the different possible ways in which + test results can be reported. + """ + results = [] + if subject_type == 'bodhi_update': + results.extend(retrieve_item_results( + {u'type': u'bodhi_update', u'item': subject_identifier})) + elif subject_type == 'koji_build': + results.extend(retrieve_item_results({u'type': u'koji_build', u'item': subject_identifier})) + results.extend(retrieve_item_results({u'original_spec_nvr': subject_identifier})) + elif subject_type == 'compose': + results.extend(retrieve_item_results({u'productmd.compose.id': subject_identifier})) + else: + raise RuntimeError('Unhandled subject type %r' % subject_type) + return results + + # NOTE - not cached, for now. @greenwave.utils.retry(wait_on=urllib3.exceptions.NewConnectionError) -def retrieve_waivers(product_version, items): +def retrieve_waivers(product_version, subject_type, subject_identifier): timeout = current_app.config['REQUESTS_TIMEOUT'] verify = current_app.config['REQUESTS_VERIFY'] - - data = { + filters = [{ 'product_version': product_version, - 'results': [{"subject": item} for item in items] - } - + 'subject_type': subject_type, + 'subject_identifier': subject_identifier, + }] response = requests_session.post( - current_app.config['WAIVERDB_API_URL'] + '/waivers/+by-subjects-and-testcases', + current_app.config['WAIVERDB_API_URL'] + '/waivers/+filtered', headers={'Content-Type': 'application/json'}, - data=json.dumps(data), + data=json.dumps({'filters': filters}), verify=verify, timeout=timeout) response.raise_for_status() diff --git a/greenwave/tests/test_policies.py b/greenwave/tests/test_policies.py index c4423b9..6d86cc3 100644 --- a/greenwave/tests/test_policies.py +++ b/greenwave/tests/test_policies.py @@ -17,18 +17,20 @@ from greenwave.utils import load_policies def test_summarize_answers(): assert summarize_answers([RuleSatisfied()]) == \ 'all required tests passed' - assert summarize_answers([TestResultFailed('item', 'test', None, 'id'), RuleSatisfied()]) == \ + assert summarize_answers([TestResultFailed('koji_build', 'nvr', 'test', None, 'id'), + RuleSatisfied()]) == \ '1 of 2 required tests failed' - assert summarize_answers([TestResultMissing('item', 'test', None)]) == \ + assert summarize_answers([TestResultMissing('koji_build', 'nvr', 'test', None)]) == \ '1 of 1 required test results missing' - assert summarize_answers([TestResultMissing('item', 'test', None), - TestResultFailed('item', 'test', None, 'id')]) == \ + assert summarize_answers([TestResultMissing('koji_build', 'nvr', 'test', None), + TestResultFailed('koji_build', 'nvr', 'test', None, 'id')]) == \ '1 of 2 required tests failed, 1 result missing' - assert summarize_answers([TestResultMissing('item', 'testa', None), - TestResultMissing('item', 'testb', None), - TestResultFailed('item', 'test', None, 'id')]) == \ + assert summarize_answers([TestResultMissing('koji_build', 'nvr', 'testa', None), + TestResultMissing('koji_build', 'nvr', 'testb', None), + TestResultFailed('koji_build', 'nvr', 'test', None, 'id')]) == \ '1 of 3 required tests failed, 2 results missing' - assert summarize_answers([TestResultMissing('item', 'test', None), RuleSatisfied()]) == \ + assert summarize_answers([TestResultMissing('koji_build', 'nvr', 'test', None), + RuleSatisfied()]) == \ '1 of 2 required test results missing' @@ -40,6 +42,7 @@ id: "rawhide_compose_sync_to_mirrors" product_versions: - fedora-rawhide decision_context: rawhide_compose_sync_to_mirrors +subject_type: compose blacklist: [] rules: - !PassingTestCaseRule {test_case_name: sometest} @@ -67,7 +70,8 @@ def test_package_specific_rule(tmpdir): id: "some_policy" product_versions: - rhel-9000 -decision_context: compose_gate +decision_context: bodhi_update_push_stable +subject_type: koji_build blacklist: [] rules: - !PackageSpecificBuild {test_case_name: sometest, repos: [nethack, python-*]} @@ -76,9 +80,8 @@ rules: policy = policies[0] # Ensure that we fail with no results - item = {'item': 'nethack-1.2.3-1.el9000', 'type': 'koji_build'} results, waivers = [], [] - decision = policy.check(item, results, waivers) + decision = policy.check('nethack-1.2.3-1.el9000', results, waivers) assert len(decision) == 1 assert isinstance(decision[0], TestResultMissing) @@ -89,7 +92,7 @@ rules: 'testcase': {'name': 'sometest'}, 'outcome': 'FAILED', }] - decision = policy.check(item, results, waivers) + decision = policy.check('nethack-1.2.3-1.el9000', results, waivers) assert len(decision) == 1 assert isinstance(decision[0], TestResultFailed) @@ -100,19 +103,18 @@ rules: 'testcase': {'name': 'sometest'}, 'outcome': 'PASSED', }] - decision = policy.check(item, results, waivers) + decision = policy.check('nethack-1.2.3-1.el9000', results, waivers) assert len(decision) == 1 assert isinstance(decision[0], RuleSatisfied) # That a non-matching passing result is ignored. - item = {'item': 'foobar-1.2.3-1.el9000', 'type': 'koji_build'} results = [{ 'id': 123, 'item': 'foobar-1.2.3-1.el9000', 'testcase': {'name': 'sometest'}, 'outcome': 'PASSED', }] - decision = policy.check(item, results, waivers) + decision = policy.check('foobar-1.2.3-1.el9000', results, waivers) assert len(decision) == 1 assert isinstance(decision[0], RuleSatisfied) @@ -123,14 +125,13 @@ rules: 'testcase': {'name': 'sometest'}, 'outcome': 'FAILED', }] - decision = policy.check(item, results, waivers) + decision = policy.check('foobar-1.2.3-1.el9000', results, waivers) assert len(decision) == 1 assert isinstance(decision[0], RuleSatisfied) # ooooh. # Ensure that fnmatch globs work in absence - item = {'item': 'python-foobar-1.2.3-1.el9000', 'type': 'koji_build'} results, waivers = [], [] - decision = policy.check(item, results, waivers) + decision = policy.check('python-foobar-1.2.3-1.el9000', results, waivers) assert len(decision) == 1 assert isinstance(decision[0], TestResultMissing) @@ -141,7 +142,7 @@ rules: 'testcase': {'name': 'sometest'}, 'outcome': 'FAILED', }] - decision = policy.check(item, results, waivers) + decision = policy.check('python-foobar-1.2.3-1.el9000', results, waivers) assert len(decision) == 1 assert isinstance(decision[0], TestResultFailed) @@ -152,7 +153,7 @@ rules: 'testcase': {'name': 'sometest'}, 'outcome': 'SUCCESS', }] - decision = policy.check(item, results, waivers) + decision = policy.check('python-foobar-1.2.3-1.el9000', results, waivers) assert len(decision) == 1 assert isinstance(decision[0], TestResultFailed) @@ -176,6 +177,7 @@ id: "taskotron_release_critical_tasks" product_versions: - fedora-26 decision_context: bodhi_update_push_stable +subject_type: bodhi_update rules: - !PassingTestCaseRule {test_case_name: dist.abicheck} """) @@ -192,6 +194,7 @@ id: "taskotron_release_critical_tasks" product_versions: - fedora-26 decision_context: bodhi_update_push_stable +subject_type: bodhi_update rules: - {test_case_name: dist.abicheck} """) @@ -208,6 +211,7 @@ id: "rawhide_compose_sync_to_mirrors" product_versions: - fedora-rawhide decision_context: rawhide_compose_sync_to_mirrors +subject_type: compose rules: - !PassingTestCaseRule {test_case_name: compose.install_default_upload, scenario: somescenario} """) @@ -222,15 +226,16 @@ id: dummy_policy product_versions: - fedora-* decision_context: dummy_context +subject_type: bodhi_update blacklist: [] rules: [] """) policies = load_policies(tmpdir.strpath) policy = policies[0] - assert policy.applies_to('dummy_context', 'fedora-27') - assert policy.applies_to('dummy_context', 'fedora-28') - assert not policy.applies_to('dummy_context', 'epel-7') + assert policy.applies_to('dummy_context', 'fedora-27', 'bodhi_update') + assert policy.applies_to('dummy_context', 'fedora-28', 'bodhi_update') + assert not policy.applies_to('dummy_context', 'epel-7', 'bodhi_update') def test_remote_original_spec_nvr_rule_policy(tmpdir): @@ -245,6 +250,7 @@ id: "taskotron_release_critical_tasks_with_remoterule" product_versions: - fedora-26 decision_context: bodhi_update_push_stable_with_remoterule +subject_type: koji_build blacklist: [] rules: - !RemoteOriginalSpecNvrRule {} @@ -271,7 +277,7 @@ rules: policies = load_policies(tmpdir.strpath) policy = policies[0] - item, waivers = {'original_spec_nvr': nvr}, [] + waivers = [] # Ensure that presence of a result is success. results = [{ @@ -280,13 +286,13 @@ rules: "testcase": {"name": "dist.upgradepath"}, "outcome": "PASSED" }] - decision = policy.check(item, results, waivers) + decision = policy.check(nvr, 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) + decision = policy.check(nvr, results, waivers) assert len(decision) == 1 assert isinstance(decision[0], TestResultMissing) @@ -297,6 +303,6 @@ rules: "testcase": {"name": "dist.upgradepath"}, "outcome": "FAILED" }] - decision = policy.check(item, results, waivers) + decision = policy.check(nvr, results, waivers) assert len(decision) == 1 assert isinstance(decision[0], TestResultFailed) From 87de1e025163c05c4daaf103e8e6d39ebc433e45 Mon Sep 17 00:00:00 2001 From: Dan Callaghan Date: May 24 2018 01:22:21 +0000 Subject: [PATCH 2/6] tests: use new 'subject_type' API in testdatabuilder.create_waiver() --- diff --git a/functional-tests/conftest.py b/functional-tests/conftest.py index 1e715da..e5db377 100644 --- a/functional-tests/conftest.py +++ b/functional-tests/conftest.py @@ -269,10 +269,11 @@ class TestDataBuilder(object): response.raise_for_status() return response.json() - def create_waiver(self, result, product_version, comment, waived=True): + def create_waiver(self, nvr, testcase_name, product_version, comment, waived=True): data = { - 'subject': result['subject'], - 'testcase': result['testcase'], + 'subject_type': 'koji_build', + 'subject_identifier': nvr, + 'testcase': testcase_name, 'product_version': product_version, 'waived': waived, 'comment': comment diff --git a/functional-tests/consumers/test_waiverdb.py b/functional-tests/consumers/test_waiverdb.py index 18feaff..a36a806 100644 --- a/functional-tests/consumers/test_waiverdb.py +++ b/functional-tests/consumers/test_waiverdb.py @@ -28,9 +28,10 @@ def test_consume_new_waiver( testcase_name=testcase_name, outcome='PASSED') testcase = str(result['testcase']['name']) - waiver = testdatabuilder.create_waiver(result={ - "subject": dict([(str(key), str(value[0])) for key, value in result['data'].items()]), - "testcase": testcase}, product_version='fedora-26', comment='Because I said so') + waiver = testdatabuilder.create_waiver(nvr=nvr, + testcase_name=testcase, + product_version='fedora-26', + comment='Because I said so') message = { 'body': { 'topic': 'waiver.new', diff --git a/functional-tests/test_api_v1.py b/functional-tests/test_api_v1.py index fb1f194..b9ad736 100644 --- a/functional-tests/test_api_v1.py +++ b/functional-tests/test_api_v1.py @@ -242,13 +242,13 @@ def test_make_a_decision_on_failed_result_with_waiver( requests_session, greenwave_server, testdatabuilder): nvr = testdatabuilder.unique_nvr() # First one failed but was waived - result = testdatabuilder.create_result(item=nvr, - testcase_name=TASKTRON_RELEASE_CRITICAL_TASKS[0], - outcome='FAILED') - waiver = testdatabuilder.create_waiver(result={ # noqa - "subject": dict([(key, value[0]) for key, value in result['data'].items()]), - "testcase": TASKTRON_RELEASE_CRITICAL_TASKS[0]}, product_version='fedora-26', - comment='This is fine') + testdatabuilder.create_result(item=nvr, + testcase_name=TASKTRON_RELEASE_CRITICAL_TASKS[0], + outcome='FAILED') + testdatabuilder.create_waiver(nvr=nvr, + product_version='fedora-26', + testcase_name=TASKTRON_RELEASE_CRITICAL_TASKS[0], + comment='This is fine') # The rest passed for testcase_name in TASKTRON_RELEASE_CRITICAL_TASKS[1:]: testdatabuilder.create_result(item=nvr, @@ -577,10 +577,10 @@ def test_ignore_waiver(requests_session, greenwave_server, testdatabuilder): result = testdatabuilder.create_result(item=nvr, testcase_name=TASKTRON_RELEASE_CRITICAL_TASKS[0], outcome='FAILED') - waiver = testdatabuilder.create_waiver(result={ - "subject": dict([(key, value[0]) for key, value in result['data'].items()]), - "testcase": TASKTRON_RELEASE_CRITICAL_TASKS[0]}, product_version='fedora-26', - comment='This is fine') + waiver = testdatabuilder.create_waiver(nvr=nvr, + testcase_name=TASKTRON_RELEASE_CRITICAL_TASKS[0], + product_version='fedora-26', + comment='This is fine') # The rest passed for testcase_name in TASKTRON_RELEASE_CRITICAL_TASKS[1:]: testdatabuilder.create_result(item=nvr, From 5c467ac6aaad6b126062cbfa24ea770219be251e Mon Sep 17 00:00:00 2001 From: Dan Callaghan Date: May 24 2018 01:22:21 +0000 Subject: [PATCH 3/6] make message consumers aware of specific subject types and identifiers Fixes #123. --- diff --git a/functional-tests/consumers/test_resultsdb.py b/functional-tests/consumers/test_resultsdb.py index a613fb3..1e39956 100644 --- a/functional-tests/consumers/test_resultsdb.py +++ b/functional-tests/consumers/test_resultsdb.py @@ -14,6 +14,8 @@ def test_consume_new_result( testdatabuilder): load_config.return_value = {'greenwave_api_url': greenwave_server + 'api/v1.0'} nvr = testdatabuilder.unique_nvr() + update = testdatabuilder.create_bodhi_update(build_nvrs=[nvr]) + updateid = update['updateid'] result = testdatabuilder.create_result(item=nvr, testcase_name='dist.rpmdeplint', outcome='PASSED') @@ -42,31 +44,18 @@ def test_consume_new_result( assert handler.topic == ['topic_prefix.environment.taskotron.result.new'] handler.consume(message) - # get old decision - data = { - 'decision_context': 'bodhi_update_push_stable', - 'product_version': 'fedora-26', - 'subject': [{'item': nvr, 'type': 'koji_build'}], - 'ignore_result': [result['id']] - } - r = requests_session.post(greenwave_server + 'api/v1.0/decision', - headers={'Content-Type': 'application/json'}, - data=json.dumps(data)) - assert r.status_code == 200 - old_decision = r.json() - # should have two messages published as we have two decision contexts applicable to - # this subject. - first_msg = { + # We expect 4 messages altogether. + assert len(mock_fedmsg.mock_calls) == 4 + assert all(call[2]['topic'] == 'decision.update' for call in mock_fedmsg.mock_calls) + actual_msgs_sent = [call[2]['msg'] for call in mock_fedmsg.mock_calls] + assert actual_msgs_sent[0] == { 'policies_satisfied': False, 'decision_context': 'bodhi_update_push_stable', 'product_version': 'fedora-26', 'unsatisfied_requirements': [ { 'testcase': 'dist.abicheck', - 'item': { - 'item': nvr, - 'type': 'koji_build' - }, + 'item': {'item': nvr, 'type': 'koji_build'}, 'subject_type': 'koji_build', 'subject_identifier': nvr, 'type': 'test-result-missing', @@ -74,10 +63,7 @@ def test_consume_new_result( }, { 'testcase': 'dist.upgradepath', - 'item': { - 'item': nvr, - 'type': 'koji_build' - }, + 'item': {'item': nvr, 'type': 'koji_build'}, 'subject_type': 'koji_build', 'subject_identifier': nvr, 'type': 'test-result-missing', @@ -86,48 +72,168 @@ def test_consume_new_result( ], 'summary': '2 of 3 required test results missing', 'subject': [ - { - 'item': nvr, - 'type': 'koji_build' - } + {'item': nvr, 'type': 'koji_build'}, ], - #'subject_type': 'koji_build', - #'subject_identifier': nvr, + 'subject_type': 'koji_build', + 'subject_identifier': nvr, 'applicable_policies': ['taskotron_release_critical_tasks_with_blacklist', 'taskotron_release_critical_tasks'], - 'previous': old_decision, + 'previous': { + 'applicable_policies': ['taskotron_release_critical_tasks_with_blacklist', + 'taskotron_release_critical_tasks'], + 'policies_satisfied': False, + 'summary': u'3 of 3 required test results missing', + 'unsatisfied_requirements': [ + { + 'testcase': 'dist.abicheck', + 'item': {'item': nvr, 'type': 'koji_build'}, + 'subject_type': 'koji_build', + 'subject_identifier': nvr, + 'type': 'test-result-missing', + 'scenario': None, + }, + { + 'testcase': 'dist.rpmdeplint', + 'item': {'item': nvr, 'type': 'koji_build'}, + 'subject_type': 'koji_build', + 'subject_identifier': nvr, + 'type': 'test-result-missing', + 'scenario': None, + }, + { + 'testcase': 'dist.upgradepath', + 'item': {'item': nvr, 'type': 'koji_build'}, + 'subject_type': 'koji_build', + 'subject_identifier': nvr, + 'type': 'test-result-missing', + 'scenario': None, + }, + ], + }, } - mock_fedmsg.assert_any_call(topic='decision.update', msg=first_msg) - # get the old decision for the second policy - data = { + assert actual_msgs_sent[1] == { + 'policies_satisfied': True, 'decision_context': 'bodhi_update_push_testing', + 'product_version': 'fedora-*', + 'unsatisfied_requirements': [], + 'summary': 'all required tests passed', + 'subject': [ + {'item': nvr, 'type': 'koji_build'}, + ], + 'subject_type': 'koji_build', + 'subject_identifier': nvr, + 'applicable_policies': ['taskotron_release_critical_tasks_for_testing'], + 'previous': { + 'applicable_policies': ['taskotron_release_critical_tasks_for_testing'], + 'policies_satisfied': False, + 'summary': '1 of 1 required test results missing', + 'unsatisfied_requirements': [ + { + 'testcase': 'dist.rpmdeplint', + 'item': {'item': nvr, 'type': 'koji_build'}, + 'subject_type': 'koji_build', + 'subject_identifier': nvr, + 'type': 'test-result-missing', + 'scenario': None, + }, + ], + }, + } + assert actual_msgs_sent[2] == { + 'policies_satisfied': False, + 'decision_context': 'bodhi_update_push_stable', 'product_version': 'fedora-26', - 'subject': [{'item': nvr, 'type': 'koji_build'}], - 'ignore_result': [result['id']] + 'unsatisfied_requirements': [ + { + 'testcase': 'dist.abicheck', + 'item': {'item': nvr, 'type': 'koji_build'}, + 'subject_type': 'koji_build', + 'subject_identifier': nvr, + 'type': 'test-result-missing', + 'scenario': None, + }, + { + 'testcase': 'dist.upgradepath', + 'item': {'item': nvr, 'type': 'koji_build'}, + 'subject_type': 'koji_build', + 'subject_identifier': nvr, + 'type': 'test-result-missing', + 'scenario': None, + } + ], + 'summary': '2 of 3 required test results missing', + 'subject': [ + {'item': updateid, 'type': 'bodhi_update'}, + {'item': nvr, 'type': 'koji_build'}, + {'original_spec_nvr': nvr}, + ], + 'subject_type': 'bodhi_update', + 'subject_identifier': updateid, + 'applicable_policies': ['taskotron_release_critical_tasks_with_blacklist', + 'taskotron_release_critical_tasks'], + 'previous': { + 'applicable_policies': ['taskotron_release_critical_tasks_with_blacklist', + 'taskotron_release_critical_tasks'], + 'policies_satisfied': False, + 'summary': u'3 of 3 required test results missing', + 'unsatisfied_requirements': [ + { + 'testcase': 'dist.abicheck', + 'item': {'item': nvr, 'type': 'koji_build'}, + 'subject_type': 'koji_build', + 'subject_identifier': nvr, + 'type': 'test-result-missing', + 'scenario': None, + }, + { + 'testcase': 'dist.rpmdeplint', + 'item': {'item': nvr, 'type': 'koji_build'}, + 'subject_type': 'koji_build', + 'subject_identifier': nvr, + 'type': 'test-result-missing', + 'scenario': None, + }, + { + 'testcase': 'dist.upgradepath', + 'item': {'item': nvr, 'type': 'koji_build'}, + 'subject_type': 'koji_build', + 'subject_identifier': nvr, + 'type': 'test-result-missing', + 'scenario': None, + }, + ], + }, } - r = requests_session.post(greenwave_server + 'api/v1.0/decision', - headers={'Content-Type': 'application/json'}, - data=json.dumps(data)) - assert r.status_code == 200 - old_decision = r.json() - second_msg = { + assert actual_msgs_sent[3] == { 'policies_satisfied': True, 'decision_context': 'bodhi_update_push_testing', 'product_version': 'fedora-*', 'unsatisfied_requirements': [], 'summary': 'all required tests passed', 'subject': [ - { - 'item': nvr, - 'type': 'koji_build' - } + {'item': updateid, 'type': 'bodhi_update'}, + {'item': nvr, 'type': 'koji_build'}, + {'original_spec_nvr': nvr}, ], - #'subject_type': 'koji_build', - #'subject_identifier': nvr, + 'subject_type': 'bodhi_update', + 'subject_identifier': updateid, 'applicable_policies': ['taskotron_release_critical_tasks_for_testing'], - 'previous': old_decision, + 'previous': { + 'applicable_policies': ['taskotron_release_critical_tasks_for_testing'], + 'policies_satisfied': False, + 'summary': '1 of 1 required test results missing', + 'unsatisfied_requirements': [ + { + 'testcase': 'dist.rpmdeplint', + 'item': {'item': nvr, 'type': 'koji_build'}, + 'subject_type': 'koji_build', + 'subject_identifier': nvr, + 'type': 'test-result-missing', + 'scenario': None, + }, + ], + }, } - mock_fedmsg.assert_any_call(topic='decision.update', msg=second_msg) @mock.patch('greenwave.consumers.resultsdb.fedmsg.config.load_config') @@ -214,8 +320,7 @@ def test_invalidate_new_result_with_mocked_cache( #'topic_prefix.environment.waiver.new', ] handler.consume(message) - expected = ("greenwave.resources:retrieve_item_results|" - "{u'item': u'%s', u'type': u'koji_build'}" % nvr) + expected = 'greenwave.resources:retrieve_results|koji_build %s' % nvr handler.cache.delete.assert_called_once_with(expected) @@ -394,8 +499,8 @@ def test_consume_compose_id_result( u'policies_satisfied': False, 'product_version': 'fedora-rawhide', 'subject': [{u'productmd.compose.id': compose_id}], - #'subject_type': 'compose', - #'subject_identifier': compose_id, + 'subject_type': 'compose', + 'subject_identifier': compose_id, u'summary': u'1 of 2 required test results missing', 'previous': old_decision, u'unsatisfied_requirements': [{ @@ -501,8 +606,8 @@ def test_consume_legacy_result( 'type': 'koji_build' } ], - #'subject_type': 'koji_build', - #'subject_identifier': nvr, + 'subject_type': 'koji_build', + 'subject_identifier': nvr, 'applicable_policies': ['taskotron_release_critical_tasks_with_blacklist', 'taskotron_release_critical_tasks'], 'previous': old_decision, @@ -532,8 +637,8 @@ def test_consume_legacy_result( 'type': 'koji_build' } ], - #'subject_type': 'koji_build', - #'subject_identifier': nvr, + 'subject_type': 'koji_build', + 'subject_identifier': nvr, 'applicable_policies': ['taskotron_release_critical_tasks_for_testing'], 'previous': old_decision, } diff --git a/functional-tests/consumers/test_waiverdb.py b/functional-tests/consumers/test_waiverdb.py index a36a806..7fe98bd 100644 --- a/functional-tests/consumers/test_waiverdb.py +++ b/functional-tests/consumers/test_waiverdb.py @@ -1,7 +1,6 @@ # SPDX-License-Identifier: GPL-2.0+ import mock -import json from greenwave.consumers import waiverdb @@ -19,6 +18,8 @@ def test_consume_new_waiver( mock_fedmsg, load_config, requests_session, greenwave_server, testdatabuilder): load_config.return_value = {'greenwave_api_url': greenwave_server + 'api/v1.0'} nvr = testdatabuilder.unique_nvr() + update = testdatabuilder.create_bodhi_update(build_nvrs=[nvr]) + updateid = update['updateid'] result = testdatabuilder.create_result(item=nvr, testcase_name='dist.abicheck', outcome='FAILED') @@ -35,16 +36,7 @@ def test_consume_new_waiver( message = { 'body': { 'topic': 'waiver.new', - "msg": { - "id": waiver['id'], - "comment": "Because I said so", - "username": "foo", - "waived": "true", - "timestamp": "2017-08-10T17:42:04.209638", - "product_version": "fedora-26", - "testcase": waiver['testcase'], - "subject": [waiver['subject']] - } + "msg": waiver, } } hub = mock.MagicMock() @@ -53,36 +45,69 @@ def test_consume_new_waiver( assert handler.topic == ['topic_prefix.environment.waiver.new'] handler.consume(message) - # get old decision - data = { + # We expect 2 messages altogether. + assert len(mock_fedmsg.mock_calls) == 2 + assert all(call[2]['topic'] == 'decision.update' for call in mock_fedmsg.mock_calls) + actual_msgs_sent = [call[2]['msg'] for call in mock_fedmsg.mock_calls] + assert actual_msgs_sent[0] == { + 'applicable_policies': ['taskotron_release_critical_tasks_with_blacklist', + 'taskotron_release_critical_tasks'], + 'policies_satisfied': True, 'decision_context': 'bodhi_update_push_stable', + 'previous': { + 'applicable_policies': ['taskotron_release_critical_tasks_with_blacklist', + 'taskotron_release_critical_tasks'], + 'policies_satisfied': False, + 'summary': u'1 of 3 required tests failed', + 'unsatisfied_requirements': [ + { + 'result_id': result['id'], + 'item': {'item': nvr, 'type': 'koji_build'}, + 'testcase': 'dist.abicheck', + 'type': 'test-result-failed', + 'scenario': None, + }, + ], + }, 'product_version': 'fedora-26', - 'subject': [{'item': nvr, 'type': 'koji_build'}], - 'ignore_waiver': [waiver['id']] + 'subject': [ + {'item': nvr, 'type': 'koji_build'}, + ], + 'subject_type': 'koji_build', + 'subject_identifier': nvr, + 'unsatisfied_requirements': [], + 'summary': 'all required tests passed', + 'testcase': testcase, } - r = requests_session.post(greenwave_server + 'api/v1.0/decision', - headers={'Content-Type': 'application/json'}, - data=json.dumps(data)) - assert r.status_code == 200 - old_decision = r.json() - assert old_decision['summary'] == '1 of 3 required tests failed' - - msg = { + assert actual_msgs_sent[1] == { 'applicable_policies': ['taskotron_release_critical_tasks_with_blacklist', 'taskotron_release_critical_tasks'], 'policies_satisfied': True, 'decision_context': 'bodhi_update_push_stable', - 'previous': old_decision, + 'previous': { + 'applicable_policies': ['taskotron_release_critical_tasks_with_blacklist', + 'taskotron_release_critical_tasks'], + 'policies_satisfied': False, + 'summary': u'1 of 3 required tests failed', + 'unsatisfied_requirements': [ + { + 'result_id': result['id'], + 'item': {'item': nvr, 'type': 'koji_build'}, + 'testcase': 'dist.abicheck', + 'type': 'test-result-failed', + 'scenario': None, + }, + ], + }, 'product_version': 'fedora-26', 'subject': [ - { - 'item': nvr, - 'type': 'koji_build' - } + {'item': updateid, 'type': 'bodhi_update'}, + {'item': nvr, 'type': 'koji_build'}, + {'original_spec_nvr': nvr}, ], + 'subject_type': 'bodhi_update', + 'subject_identifier': updateid, 'unsatisfied_requirements': [], 'summary': 'all required tests passed', 'testcase': testcase, } - mock_fedmsg.assert_called_once_with( - topic='decision.update', msg=msg) diff --git a/functional-tests/fake_bodhi.py b/functional-tests/fake_bodhi.py index 1838775..bf2ec29 100644 --- a/functional-tests/fake_bodhi.py +++ b/functional-tests/fake_bodhi.py @@ -2,6 +2,7 @@ import re import json import hashlib +from urlparse import parse_qs updates = {} #: {id -> update info} @@ -26,6 +27,23 @@ def application(environ, start_response): m = re.match(r'/updates/$', path_info) if m: + if environ['REQUEST_METHOD'] == 'GET': + params = parse_qs(environ['QUERY_STRING']) + if 'builds' in params: + response_updates = [u for u in updates.values() + if set(params['builds']).issubset(build['nvr'] + for build in u['builds'])] + else: + response_updates = updates.values() + response_data = { + 'page': 1, + 'pages': 1, + 'rows_per_page': len(updates.values()), + 'total': len(updates.values()), + 'updates': response_updates, + } + start_response('200 OK', [('Content-Type', 'application/json')]) + return [json.dumps(response_data)] if environ['REQUEST_METHOD'] == 'POST': body = environ['wsgi.input'].read(int(environ['CONTENT_LENGTH'])) updateid = 'FEDORA-2000-{}'.format(hashlib.sha1(body).hexdigest()[-8:]) diff --git a/greenwave/api_v1.py b/greenwave/api_v1.py index f3e2f7e..01ebf12 100644 --- a/greenwave/api_v1.py +++ b/greenwave/api_v1.py @@ -41,6 +41,25 @@ def subject_list_to_type_identifier(subject): raise BadRequest('Unrecognised subject type: %r' % subject) +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 == 'bodhi_update': + old_subject = [{'type': 'bodhi_update', 'item': subject_identifier}] + for nvr in retrieve_builds_in_update(subject_identifier): + old_subject.append({'type': 'koji_build', 'item': nvr}) + old_subject.append({'original_spec_nvr': nvr}) + return old_subject + elif subject_type == 'koji_build': + return [{'type': 'koji_build', 'item': subject_identifier}] + elif subject_type == 'compose': + return [{'productmd.compose.id': subject_identifier}] + else: + raise BadRequest('Unrecognised subject type: %s' % subject_type) + + @api.route('/version', methods=['GET']) @jsonp def version(): diff --git a/greenwave/config.py b/greenwave/config.py index 13646dd..12a2e3a 100644 --- a/greenwave/config.py +++ b/greenwave/config.py @@ -34,13 +34,6 @@ class Config(object): # By default, don't cache anything. CACHE = {'backend': 'dogpile.cache.null'} - # These are keys used to construct announcements about decision changes. - ANNOUNCEMENT_SUBJECT_KEYS = [ - ('item', 'type',), - ('original_spec_nvr',), - ('productmd.compose.id',), - ] - class ProductionConfig(Config): DEBUG = False diff --git a/greenwave/consumers/resultsdb.py b/greenwave/consumers/resultsdb.py index 77aa778..577929f 100644 --- a/greenwave/consumers/resultsdb.py +++ b/greenwave/consumers/resultsdb.py @@ -19,6 +19,7 @@ import requests import greenwave.app_factory import greenwave.cache import greenwave.resources +from greenwave.api_v1 import subject_type_identifier_to_list requests_session = requests.Session() @@ -60,10 +61,11 @@ class ResultsDBHandler(fedmsg.consumers.FedmsgConsumer): @staticmethod def announcement_subjects(message): - """ Yields subjects for announcement consideration from the message. + """ + Yields pairs of (subject type, subject identifier) for announcement + consideration from the message. Args: - config (dict): The greenwave configuration. message (munch.Munch): A fedmsg about a new result. """ @@ -72,19 +74,28 @@ class ResultsDBHandler(fedmsg.consumers.FedmsgConsumer): except KeyError: data = message['msg']['task'] # Old format - announcement_keys = [ - set(keys) for keys in current_app.config['ANNOUNCEMENT_SUBJECT_KEYS'] - ] - def _decode(value): """ Decode either a string or a list of strings. """ if len(value) == 1: value = value[0] return value.decode('utf-8') - for keys in announcement_keys: - if keys.issubset(data.keys()): - yield {key.decode('utf-8'): _decode(data[key]) for key in keys} + if data.get('type') == 'bodhi_update' and 'item' in data: + yield (u'bodhi_update', _decode(data['item'])) + if 'productmd.compose.id' in data: + yield (u'compose', _decode(data['productmd.compose.id'])) + if (data.get('type') == 'koji_build' and 'item' in data or + 'original_spec_nvr' in data): + if data.get('type') == 'koji_build': + nvr = _decode(data['item']) + else: + nvr = _decode(data['original_spec_nvr']) + yield (u'koji_build', nvr) + # If the result is for a build, it may also influence the decision + # about any update which the build is part of. + updateid = greenwave.resources.retrieve_update_for_build(nvr) + if updateid is not None: + yield (u'bodhi_update', updateid) def consume(self, message): """ @@ -107,12 +118,13 @@ class ResultsDBHandler(fedmsg.consumers.FedmsgConsumer): result_id = message['msg']['result']['id'] # Old format with self.flask_app.app_context(): - for subject in self.announcement_subjects(message): - log.debug('Considering subject "%s"', subject) - self._invalidate_cache(subject) - self._publish_decision_changes(subject, result_id, testcase) + for subject_type, subject_identifier in self.announcement_subjects(message): + log.debug('Considering subject %s: %r', subject_type, subject_identifier) + self._invalidate_cache(subject_type, subject_identifier) + self._publish_decision_changes(subject_type, subject_identifier, + result_id, testcase) - def _publish_decision_changes(self, subject, result_id, testcase): + def _publish_decision_changes(self, subject_type, subject_identifier, result_id, testcase): """ Process the given subject and publish a message if the decision is changed. @@ -148,7 +160,8 @@ class ResultsDBHandler(fedmsg.consumers.FedmsgConsumer): data = { 'decision_context': decision_context, 'product_version': product_version, - 'subject': [subject], + 'subject_type': subject_type, + 'subject_identifier': subject_identifier, } decision = greenwave.resources.retrieve_decision(greenwave_url, data) @@ -160,7 +173,11 @@ class ResultsDBHandler(fedmsg.consumers.FedmsgConsumer): if decision != old_decision: decision.update({ - 'subject': [subject], + 'subject_type': subject_type, + 'subject_identifier': subject_identifier, + # subject is for backwards compatibility only: + 'subject': subject_type_identifier_to_list(subject_type, + subject_identifier), 'decision_context': decision_context, 'product_version': product_version, 'previous': old_decision, @@ -169,16 +186,17 @@ class ResultsDBHandler(fedmsg.consumers.FedmsgConsumer): 'greenwave.decision.update') fedmsg.publish(topic='decision.update', msg=decision) - def _invalidate_cache(self, subject): + def _invalidate_cache(self, subject_type, subject_identifier): """ Process the given subject and delete cache keys as necessary. Args: - subject (munch.Munch): A subject argument, used to query greenwave. + subject_type (str): A subject type, used to query greenwave. + subject_identifier (str): A subject identifier, used to query greenwave. """ namespace = None - fn = greenwave.resources.retrieve_item_results - key = greenwave.cache.key_generator(namespace, fn)(subject) + fn = greenwave.resources.retrieve_results + key = greenwave.cache.key_generator(namespace, fn)(subject_type, subject_identifier) if not self.cache.get(key): log.debug("No cache value found for %r", key) else: diff --git a/greenwave/consumers/waiverdb.py b/greenwave/consumers/waiverdb.py index 783ca68..9c355f6 100644 --- a/greenwave/consumers/waiverdb.py +++ b/greenwave/consumers/waiverdb.py @@ -17,6 +17,7 @@ import fedmsg.consumers import requests import greenwave.app_factory +from greenwave.api_v1 import subject_type_identifier_to_list requests_session = requests.Session() @@ -67,13 +68,30 @@ class WaiverDBHandler(fedmsg.consumers.FedmsgConsumer): product_version = msg['product_version'] testcase = msg['testcase'] + subject_type = msg['subject_type'] + subject_identifier = msg['subject_identifier'] + + with self.flask_app.app_context(): + self._publish_decision_changes(subject_type, subject_identifier, msg['id'], + product_version, testcase) + if subject_type == 'koji_build': + # If the waiver is for a build, it may also influence the decision + # about any update which the build is part of. + updateid = greenwave.resources.retrieve_update_for_build(subject_identifier) + if updateid is not None: + self._publish_decision_changes('bodhi_update', updateid, msg['id'], + product_version, testcase) + + def _publish_decision_changes(self, subject_type, subject_identifier, waiver_id, + product_version, testcase): for policy in self.flask_app.config['policies']: for rule in policy.rules: if getattr(rule, 'test_case_name', None) == testcase: data = { 'decision_context': policy.decision_context, 'product_version': product_version, - 'subject': msg['subject'] + 'subject_type': subject_type, + 'subject_identifier': subject_identifier, } response = requests_session.post( self.fedmsg_config['greenwave_api_url'] + '/decision', @@ -83,7 +101,7 @@ class WaiverDBHandler(fedmsg.consumers.FedmsgConsumer): # get old decision data.update({ - 'ignore_waiver': [msg['id']], + 'ignore_waiver': [waiver_id], }) response = requests_session.post( self.fedmsg_config['greenwave_api_url'] + '/decision', @@ -93,11 +111,13 @@ class WaiverDBHandler(fedmsg.consumers.FedmsgConsumer): old_decision = response.json() if decision != old_decision: - subject = [dict((str(k), str(v)) for k, v in item.items()) - for item in msg['subject']] msg = decision decision.update({ - 'subject': subject, + 'subject_type': subject_type, + 'subject_identifier': subject_identifier, + # subject is for backwards compatibility only: + 'subject': subject_type_identifier_to_list(subject_type, + subject_identifier), 'testcase': testcase, 'decision_context': policy.decision_context, 'product_version': product_version, diff --git a/greenwave/resources.py b/greenwave/resources.py index c96e35c..6e25478 100644 --- a/greenwave/resources.py +++ b/greenwave/resources.py @@ -83,18 +83,30 @@ def retrieve_builds_in_update(update_id): return [build['nvr'] for build in response.json()['update']['builds']] -@cached +def retrieve_update_for_build(nvr): + """ + Queries Bodhi to find the update which the given build is in (if any). + Returns a Bodhi updateid, or None if the build is not in any update. + """ + updates_list_url = urlparse.urljoin(current_app.config['BODHI_URL'], '/updates/') + params = {'builds': nvr} + timeout = current_app.config['REQUESTS_TIMEOUT'] + verify = current_app.config['REQUESTS_VERIFY'] + response = requests_session.get(updates_list_url, + params=params, + headers={'Accept': 'application/json'}, + timeout=timeout, verify=verify) + response.raise_for_status() + matching_updates = response.json()['updates'] + if matching_updates: + return matching_updates[0]['updateid'] + return None + + def retrieve_item_results(item): """ Retrieve cached results from resultsdb for a given item. """ # XXX make this more efficient than just fetching everything - # Types matter here because of the cache decorator! - # While we are still on Python 2 we must ensure the args are unicode not str - assert isinstance(item, dict) - for k, v in item.iteritems(): - assert isinstance(k, unicode) - assert isinstance(v, unicode) - params = item.copy() params.update({'limit': '1000'}) timeout = current_app.config['REQUESTS_TIMEOUT'] @@ -106,12 +118,16 @@ def retrieve_item_results(item): return response.json()['data'] +@cached def retrieve_results(subject_type, subject_identifier): """ Returns all results from ResultsDB which might be relevant for the given decision subject, accounting for all the different possible ways in which test results can be reported. """ + # Note that the reverse of this logic also lives in the + # announcement_subjects() method of the Resultsdb consumer (it has to map + # from a newly received result back to the possible subjects it is for). results = [] if subject_type == 'bodhi_update': results.extend(retrieve_item_results( diff --git a/greenwave/tests/test_resultsdb_consumer.py b/greenwave/tests/test_resultsdb_consumer.py index ecfb51f..4b1fca4 100644 --- a/greenwave/tests/test_resultsdb_consumer.py +++ b/greenwave/tests/test_resultsdb_consumer.py @@ -1,5 +1,7 @@ # SPDX-License-Identifier: GPL-2.0+ +import mock + import greenwave.app_factory import greenwave.consumers.resultsdb @@ -7,12 +9,13 @@ import greenwave.consumers.resultsdb def test_announcement_keys_decode_with_list(): cls = greenwave.consumers.resultsdb.ResultsDBHandler app = greenwave.app_factory.create_app() - app.config['ANNOUNCEMENT_SUBJECT_KEYS'] = [('foo',)] message = {'msg': {'data': { - u'foo'.encode('utf-8'): [u'bar'.encode('utf-8')], + u'original_spec_nvr'.encode('utf-8'): [u'glibc-1.0-1.fc27'.encode('utf-8')], }}} with app.app_context(): - subjects = list(cls.announcement_subjects(message)) + with mock.patch('greenwave.resources.retrieve_update_for_build') as f: + f.return_value = None + subjects = list(cls.announcement_subjects(message)) - assert subjects == [{u'foo': u'bar'}] + assert subjects == [(u'koji_build', u'glibc-1.0-1.fc27')] From 7d87de09c0b7967ada85ac55d31c3bb3f679bbaf Mon Sep 17 00:00:00 2001 From: Dan Callaghan Date: May 24 2018 01:22:21 +0000 Subject: [PATCH 4/6] treat 'brew-build' as an alias for 'koji_build' --- diff --git a/conf/policies/redhat.yaml b/conf/policies/redhat.yaml new file mode 100644 index 0000000..a5200e8 --- /dev/null +++ b/conf/policies/redhat.yaml @@ -0,0 +1,21 @@ +# This is just a hypothetical policy which could be used for Red Hat products. +--- !Policy +id: "osci_compose" +product_versions: +- rhel-something +decision_context: osci_compose_gate +subject_type: koji_build +blacklist: [] +rules: +- !PackageSpecificBuild { + test_case_name: osci.brew-build.tier0.functional, + repos: [ + "avahi", + "cockpit", + "checkpolicy", + "libsemanage", + "libselinux", + "libsepol", + "policycoreutils", + ] + } diff --git a/functional-tests/conftest.py b/functional-tests/conftest.py index e5db377..d468da8 100644 --- a/functional-tests/conftest.py +++ b/functional-tests/conftest.py @@ -228,12 +228,21 @@ class TestDataBuilder(object): self.distgit_url = distgit_url self._counter = itertools.count(1) - def unique_nvr(self): - return 'glibc-1.0-{}.el7'.format(self._counter.next()) + def unique_nvr(self, name='glibc'): + return '{}-1.0-{}.el7'.format(name, self._counter.next()) def unique_compose_id(self): return 'Fedora-9000-19700101.n.{}'.format(self._counter.next()) + def _create_result(self, data): + response = self.requests_session.post( + self.resultsdb_url + 'api/v2.0/results', + headers={'Content-Type': 'application/json'}, + timeout=TEST_HTTP_TIMEOUT, + data=json.dumps(data)) + response.raise_for_status() + return response.json() + def create_compose_result(self, compose_id, testcase_name, outcome, scenario=None): data = { 'testcase': {'name': testcase_name}, @@ -242,13 +251,15 @@ class TestDataBuilder(object): } if scenario: data['data']['scenario'] = scenario - response = self.requests_session.post( - self.resultsdb_url + 'api/v2.0/results', - headers={'Content-Type': 'application/json'}, - timeout=TEST_HTTP_TIMEOUT, - data=json.dumps(data)) - response.raise_for_status() - return response.json() + return self._create_result(data) + + def create_koji_build_result(self, nvr, testcase_name, outcome, type_='koji_build'): + data = { + 'testcase': {'name': testcase_name}, + 'outcome': outcome, + 'data': {'item': nvr, 'type': type_}, + } + return self._create_result(data) def create_result(self, item, testcase_name, outcome, scenario=None, key=None): data = { @@ -261,13 +272,7 @@ class TestDataBuilder(object): data['data'] = {key: item} if scenario: data['data']['scenario'] = scenario - response = self.requests_session.post( - self.resultsdb_url + 'api/v2.0/results', - headers={'Content-Type': 'application/json'}, - timeout=TEST_HTTP_TIMEOUT, - data=json.dumps(data)) - response.raise_for_status() - return response.json() + return self._create_result(data) def create_waiver(self, nvr, testcase_name, product_version, comment, waived=True): data = { diff --git a/functional-tests/test_api_v1.py b/functional-tests/test_api_v1.py index b9ad736..647ac6f 100644 --- a/functional-tests/test_api_v1.py +++ b/functional-tests/test_api_v1.py @@ -23,7 +23,7 @@ def test_inspect_policies(requests_session, greenwave_server): assert r.status_code == 200 body = r.json() policies = body['policies'] - assert len(policies) == 6 + assert len(policies) == 7 assert any(p['id'] == 'taskotron_release_critical_tasks' for p in policies) assert any(p['decision_context'] == 'bodhi_update_push_stable' for p in policies) assert any(p['product_versions'] == ['fedora-26'] for p in policies) @@ -699,3 +699,26 @@ def test_blacklist(requests_session, greenwave_server, testdatabuilder): # the failed test result of dist.abicheck should be ignored and thus the policy # is satisfied. assert res_data['policies_satisfied'] is True + + +def test_make_a_decision_about_brew_build(requests_session, greenwave_server, testdatabuilder): + # The 'brew-build' type is used internally within Red Hat. We treat it as + # the 'koji_build' subject type. + nvr = testdatabuilder.unique_nvr(name='avahi') + testdatabuilder.create_koji_build_result( + nvr=nvr, testcase_name='osci.brew-build.tier0.functional', + outcome='PASSED', type_='brew-build') + data = { + 'decision_context': 'osci_compose_gate', + 'product_version': 'rhel-something', + 'subject': [{'type': 'brew-build', 'item': nvr}], + } + + 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 res_data['policies_satisfied'] is True + assert res_data['applicable_policies'] == ['osci_compose'] + assert res_data['summary'] == 'all required tests passed' diff --git a/greenwave/api_v1.py b/greenwave/api_v1.py index 01ebf12..89fdaea 100644 --- a/greenwave/api_v1.py +++ b/greenwave/api_v1.py @@ -34,6 +34,8 @@ def subject_list_to_type_identifier(subject): return ('compose', subject[0]['productmd.compose.id']) # We don't know of any callers who would ask about subjects like this, # but it's easy enough to handle here anyway: + if len(subject) == 1 and subject[0].get('type') == 'brew-build' and 'item' in subject[0]: + return ('koji_build', subject[0]['item']) if len(subject) == 1 and subject[0].get('type') == 'koji_build' and 'item' in subject[0]: return ('koji_build', subject[0]['item']) if len(subject) == 1 and 'original_spec_nvr' in subject[0]: diff --git a/greenwave/consumers/resultsdb.py b/greenwave/consumers/resultsdb.py index 577929f..659e9f1 100644 --- a/greenwave/consumers/resultsdb.py +++ b/greenwave/consumers/resultsdb.py @@ -85,8 +85,9 @@ class ResultsDBHandler(fedmsg.consumers.FedmsgConsumer): if 'productmd.compose.id' in data: yield (u'compose', _decode(data['productmd.compose.id'])) if (data.get('type') == 'koji_build' and 'item' in data or + data.get('type') == 'brew-build' and 'item' in data or 'original_spec_nvr' in data): - if data.get('type') == 'koji_build': + if data.get('type') in ['koji_build', 'brew-build']: nvr = _decode(data['item']) else: nvr = _decode(data['original_spec_nvr']) diff --git a/greenwave/resources.py b/greenwave/resources.py index 6e25478..6342630 100644 --- a/greenwave/resources.py +++ b/greenwave/resources.py @@ -134,6 +134,7 @@ def retrieve_results(subject_type, subject_identifier): {u'type': u'bodhi_update', u'item': subject_identifier})) elif subject_type == 'koji_build': results.extend(retrieve_item_results({u'type': u'koji_build', u'item': subject_identifier})) + results.extend(retrieve_item_results({u'type': u'brew-build', u'item': subject_identifier})) results.extend(retrieve_item_results({u'original_spec_nvr': subject_identifier})) elif subject_type == 'compose': results.extend(retrieve_item_results({u'productmd.compose.id': subject_identifier})) diff --git a/greenwave/tests/test_resultsdb_consumer.py b/greenwave/tests/test_resultsdb_consumer.py index 4b1fca4..968f339 100644 --- a/greenwave/tests/test_resultsdb_consumer.py +++ b/greenwave/tests/test_resultsdb_consumer.py @@ -19,3 +19,41 @@ def test_announcement_keys_decode_with_list(): subjects = list(cls.announcement_subjects(message)) assert subjects == [(u'koji_build', u'glibc-1.0-1.fc27')] + + +def test_announcement_subjects_include_bodhi_update(): + cls = greenwave.consumers.resultsdb.ResultsDBHandler + app = greenwave.app_factory.create_app() + message = {'msg': {'data': { + u'original_spec_nvr'.encode('utf-8'): [u'glibc-1.0-2.fc27'.encode('utf-8')], + }}} + + with app.app_context(): + with mock.patch('greenwave.resources.retrieve_update_for_build') as f: + f.return_value = 'FEDORA-27-12345678' + subjects = list(cls.announcement_subjects(message)) + + # Result was about a Koji build, but the build is in a Bodhi update. + # So we should announce decisions about both subjects. + assert subjects == [ + ('koji_build', 'glibc-1.0-2.fc27'), + ('bodhi_update', 'FEDORA-27-12345678'), + ] + + +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 + app = greenwave.app_factory.create_app() + message = {'msg': {'data': { + u'type'.encode('utf-8'): u'brew-build'.encode('utf-8'), + u'item'.encode('utf-8'): [u'glibc-1.0-3.fc27'.encode('utf-8')], + }}} + + with app.app_context(): + with mock.patch('greenwave.resources.retrieve_update_for_build') as f: + f.return_value = None + subjects = list(cls.announcement_subjects(message)) + + assert subjects == [('koji_build', 'glibc-1.0-3.fc27')] From 2970d19e133ec25723f2cb9edbe72e3735db5780 Mon Sep 17 00:00:00 2001 From: Dan Callaghan Date: May 24 2018 01:22:21 +0000 Subject: [PATCH 5/6] make Bodhi integration optional --- diff --git a/greenwave/config.py b/greenwave/config.py index 12a2e3a..8aa1613 100644 --- a/greenwave/config.py +++ b/greenwave/config.py @@ -16,12 +16,16 @@ class Config(object): RESULTSDB_API_URL = 'https://taskotron.fedoraproject.org/resultsdb_api/api/v2.0' WAIVERDB_API_URL = 'https://waiverdb.fedoraproject.org/api/v1.0' + # Greenwave queries Bodhi to map builds <-> updates, + # so that it can make decisions about updates based on results for builds. + # If you don't have Bodhi, set this to None + # (this effectively disables the 'bodhi_update' subject type). + BODHI_URL = 'https://bodhi.fedoraproject.org/' # Options for outbound HTTP requests made by python-requests DIST_GIT_BASE_URL = 'https://src.fedoraproject.org' DIST_GIT_URL_TEMPLATE = '{DIST_GIT_BASE_URL}/{pkg_name}/{rev}/gating.yaml' KOJI_BASE_URL = 'https://koji.fedoraproject.org/kojihub' - BODHI_URL = 'https://bodhi.fedoraproject.org/' REQUESTS_TIMEOUT = (6.1, 15) REQUESTS_VERIFY = True diff --git a/greenwave/consumers/resultsdb.py b/greenwave/consumers/resultsdb.py index 659e9f1..29e4767 100644 --- a/greenwave/consumers/resultsdb.py +++ b/greenwave/consumers/resultsdb.py @@ -94,9 +94,10 @@ class ResultsDBHandler(fedmsg.consumers.FedmsgConsumer): yield (u'koji_build', nvr) # If the result is for a build, it may also influence the decision # about any update which the build is part of. - updateid = greenwave.resources.retrieve_update_for_build(nvr) - if updateid is not None: - yield (u'bodhi_update', updateid) + if current_app.config['BODHI_URL']: + updateid = greenwave.resources.retrieve_update_for_build(nvr) + if updateid is not None: + yield (u'bodhi_update', updateid) def consume(self, message): """ diff --git a/greenwave/consumers/waiverdb.py b/greenwave/consumers/waiverdb.py index 9c355f6..fcafe8e 100644 --- a/greenwave/consumers/waiverdb.py +++ b/greenwave/consumers/waiverdb.py @@ -12,7 +12,7 @@ to the message bus about the newly satisfied/unsatisfied policy. import logging import json -# from flask import current_app +from flask import current_app import fedmsg.consumers import requests @@ -74,7 +74,7 @@ class WaiverDBHandler(fedmsg.consumers.FedmsgConsumer): with self.flask_app.app_context(): self._publish_decision_changes(subject_type, subject_identifier, msg['id'], product_version, testcase) - if subject_type == 'koji_build': + if subject_type == 'koji_build' and current_app.config['BODHI_URL']: # If the waiver is for a build, it may also influence the decision # about any update which the build is part of. updateid = greenwave.resources.retrieve_update_for_build(subject_identifier) diff --git a/greenwave/resources.py b/greenwave/resources.py index 6342630..77c1199 100644 --- a/greenwave/resources.py +++ b/greenwave/resources.py @@ -6,6 +6,7 @@ waiverdb, etc..). """ +import logging import json import requests import urllib3.exceptions @@ -19,6 +20,8 @@ from greenwave.cache import cached import greenwave.utils import greenwave.policies +log = logging.getLogger(__name__) + requests_session = requests.Session() @@ -72,6 +75,12 @@ def retrieve_builds_in_update(update_id): Queries Bodhi to find the list of builds in the given update. Returns a list of build NVRs. """ + if not current_app.config['BODHI_URL']: + log.warning('Making a decision about Bodhi update %s ' + 'but Bodhi integration is disabled! ' + 'Assuming no builds in update', + update_id) + return [] update_info_url = urlparse.urljoin(current_app.config['BODHI_URL'], '/updates/{}'.format(update_id)) timeout = current_app.config['REQUESTS_TIMEOUT'] From 74b66d94cbf1f371ee54eac5066110e0961c7d97 Mon Sep 17 00:00:00 2001 From: Dan Callaghan Date: May 24 2018 01:22:21 +0000 Subject: [PATCH 6/6] docs: update for subject_type/subject_identifier --- diff --git a/docs/policies.rst b/docs/policies.rst index 5dc5b96..0bd234c 100644 --- a/docs/policies.rst +++ b/docs/policies.rst @@ -22,6 +22,7 @@ Here is an example policy: --- !Policy id: taskotron_release_critical_tasks decision_context: bodhi_update_push_stable + subject_type: bodhi_update product_versions: - fedora-26 - fedora-27 @@ -59,6 +60,15 @@ The document is a map (dictionary) with the following keys: passes this value when it asks Greenwave to decide whether a Bodhi update is ready to be pushed to the stable repositories. +``subject_type`` + When you ask Greenwave for a decision, you ask it about a specific software + artefact (the "subject" of the decision). Each policy applies to some type + of software artefact -- in this example, the policy applies to Bodhi + updates. + + The subject type must be one of the fixed set of types known to Greenwave. + See the :ref:`subject-types` section below for a list of possible types. + ``product_versions`` A policy applies to one or more "product versions". When you ask Greenwave for a decision, you must tell it which product version you are working @@ -83,7 +93,7 @@ The document is a map (dictionary) with the following keys: map, tagged with the rule type. Currently there are a few rule types, ``PassingTestCaseRule`` being one of - them. See the section of Rule Types, below for a full list. + them. See the :ref:`rule-types` section below for a full list. ``blacklist`` A list of binary RPM package names which are exempted from this policy. @@ -91,11 +101,44 @@ The document is a map (dictionary) with the following keys: The blacklist only takes effect when Greenwave is making a decision about subjects with ``"item": "koji_build"``. +.. _Koji: https://pagure.io/koji .. _Bodhi: https://github.com/fedora-infra/bodhi .. _Product Definition Center: https://github.com/product-definition-center/product-definition-center -Rule Types +.. _subject-types: + +Subject types +============= + +Greenwave can make decisions about the following types of software artefacts: + +``koji_build`` + A build stored in the `Koji`_ build system. Builds are identified by their + Name-Version-Release (NVR) identifier, as in ``glibc-2.26-27.fc27``. + Note that Koji identifies builds by the NVR of their source RPM, + regardless which binary packages were produced in the build. + +``bodhi_update`` + A distribution update in `Bodhi`_. Updates are identified by their Bodhi + update id, as in ``FEDORA-2018-ec7cb4d5eb``. + + A Bodhi update contains one or more Koji builds. When Greenwave makes a + decision about a Bodhi update, it *also* considers any policies which apply + to Koji builds in that update. + +``compose`` + A distribution compose. The compose tool (typically Pungi) takes a snapshot + of the distribution at a point in time, and produces a directory hierarchy + containing packages, installer images, and other metadata. Composes are + identified by the compose id in their metadata, which is typically also + reflected in their directory name, for example + ``Fedora-Rawhide-20170508.n.0``. + + +.. _rule-types: + +Rule types ========== PassingTestCaseRule @@ -111,20 +154,15 @@ PackageSpecificBuild Just like the ``PassingTestCaseRule``, the ``PackageSpecificBuild`` rule requires that a given ``test_case_name`` is passing, but only for certain - packages (listed in the ``repos`` argument). The configured package names - in the ``repos`` list may contain wildcards to, for instance, write a rule - requiring a certain test must pass for all `python-*` packages. - - At query time, the package name is parsed from an assumed `nvr` in the - ``item`` of the subject. - + source package names (listed in the ``repos`` argument). The configured + package names in the ``repos`` list may contain wildcards to, for instance, + write a rule requiring a certain test must pass for all `python-*` + packages. -FedoraAtomicCi --------------- + This rule type can only be used if the policy's subject type is + ``koji_build``. - This rule is nearly identical to the ``PackageSpecificBuild`` rule, except - that at query time, the package name is parsed from an assumed `nvr` in the - ``original_spec_nvr`` of the subject. + ``FedoraAtomicCi`` is a backwards compatibility alias for this rule type. .. _remote-original-spec-nvr-rule: diff --git a/greenwave/api_v1.py b/greenwave/api_v1.py index 89fdaea..93141d2 100644 --- a/greenwave/api_v1.py +++ b/greenwave/api_v1.py @@ -160,18 +160,14 @@ def make_decision(): .. sourcecode:: http POST /api/v1.0/decision HTTP/1.1 - Host: localhost:5005 - Accept-Encoding: gzip, deflate Accept: application/json - Connection: keep-alive - User-Agent: HTTPie/0.9.4 Content-Type: application/json - Content-Length: 91 { "decision_context": "bodhi_update_push_stable", "product_version": "fedora-26", - "subject": [{"item": "glibc-1.0-1.f26", "type": "koji_build"}], + "subject_type": "koji_build", + "subject_identifier": "cross-gcc-7.0.1-0.3.fc26", "verbose": true } @@ -192,14 +188,13 @@ def make_decision(): "applicable_policies": ["1"], "unsatisfied_requirements": [ { - 'item': {"item": "glibc-1.0-1.f26", "type": "koji_build"}, 'result_id': "123", 'testcase': 'dist.depcheck', 'type': 'test-result-failed' }, { - 'item': {"item": "glibc-1.0-1.f26", "type": "koji_build"}, - 'result_id': "124", + "subject_type": "koji_build", + "subject_identifier": "cross-gcc-7.0.1-0.3.fc26", 'testcase': 'dist.rpmlint', 'type': 'test-result-missing' } @@ -234,7 +229,8 @@ def make_decision(): 'waived': true, 'timestamp': '2018-01-23T18:02:04.630122', 'proxied_by': null, - 'subject': [{'item': 'cross-gcc-7.0.1-0.3.fc26', 'type': 'koji_build'}], + "subject_type": "koji_build", + "subject_identifier": "cross-gcc-7.0.1-0.3.fc26", 'testcase': 'dist.rpmlint', 'id': 1 } @@ -245,9 +241,14 @@ def make_decision(): :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``. - :jsonparam list subject: A list of items about which the caller is requesting a decision - used for querying ResultsDB. Each item contains one or more key-value pairs of 'data' key - in ResultsDB API. For example, [{"type": "koji_build", "item": "xscreensaver-5.37-3.fc27"}]. + :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. + :jsonparam string subject_identifier: A string identifying the software + artefact we are making a decision about. The meaning of the identifier + depends on the subject type. + See :ref:`subject-types` for details of how each subject type is identified. + :jsonparam list subject: *Deprecated:* Pass 'subject_type' and 'subject_identifier' instead. :jsonparam bool verbose: A flag to return additional information. :jsonparam list ignore_result: A list of result ids that will be ignored when making the decision.