From e451df2a80a982cfd1a7a248c28178af8c2ba5fe Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 23 2017 18:18:42 +0000 Subject: [PATCH 1/13] Adjust the API as discussed with Joseph and Kamil Signed-off-by: Pierre-Yves Chibon --- diff --git a/README.rst b/README.rst index 104169c..7e8f2c4 100644 --- a/README.rst +++ b/README.rst @@ -19,22 +19,26 @@ As a consumer, using rats is fairly simple, submit a ``POST`` or a ``GET`` request to the ``/submit`` endpoint with the following arguments * ``type``: - the type of element to test. Rats currently supports: - ``KojiBuild``, ``PagurePR``, ``BodhiUpdate``. + the type of element to test. They are the same types as defined in + `taskotron `_ + and ``PAGURE_PR``. -* ``target``: +* ``item`` (optional if result_id is specified): the identifier of the element tested, so it could look like this: - * KojiBuild: the NVR, for example: ``audit-2.8-1.fc26`` - * BodhiUpdate: the bodhi update ID, for example: ``FEDORA-2017-5e475c0b0d`` - * PagurePR: the path to the pull-request, for example: ``rpms/python-arrow/pull-request/13`` + * KOJI_BUILD : the NVR, for example: ``audit-2.8-1.fc26`` + * BODHI_UPDATE : the bodhi update ID, for example: ``FEDORA-2017-5e475c0b0d`` + * PAGURE_PR: the path to the pull-request, for example: ``rpms/python-arrow/pull-request/13`` - -* ``test`` +* ``test`` (optional if result_id is specified): the test to re-run, for example: ``simple-koji-ci``, ``AtomicCI``, ``dist.rpmdeplint`` or ``update.base_selinux``. +* ``result_id`` (optional if item and test are specified): + the unique identifier of a result in `resultsdb `_. + (The resultsdb url being defined in rats' configuration) + * ``extras`` (optional) any extras information you want to be present in the fedmsg messages sent at the end of the process. From 49d44ab05b1e4d4e6b608750c8f1a260ed7631d8 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 24 2017 12:59:57 +0000 Subject: [PATCH 2/13] Re-architecture the project - Use a per test system plugin system instead of per test subject, this just makes more sense. - Add the AtomicCI, Taskotron and simple-koji-ci backend - Update the tests for the new structure Signed-off-by: Pierre-Yves Chibon --- diff --git a/rats/default_config.py b/rats/default_config.py index d430f0e..5698ad0 100644 --- a/rats/default_config.py +++ b/rats/default_config.py @@ -22,6 +22,7 @@ DEBUG = True DIST_GIT_URL = 'https://src.fedoraproject.org' BODHI_URL = 'https://bodhi.fedoraproject.org' +RESULTSDB_API_URL = 'https://taskotron.fedoraproject.org/resultsdb_api/' # url to the database server: DB_URL = 'sqlite:////var/tmp/rats_dev.sqlite' @@ -30,7 +31,7 @@ DB_URL = 'sqlite:////var/tmp/rats_dev.sqlite' SECRET_KEY = '' -TEST_SYSTEMS = ["PagurePR", "KojiBuild", "BodhiUpdate"] +TEST_SYSTEMS = ["Taskotron", "AtomicCI", "simple-koji-ci"] # Worker configuration diff --git a/rats/lib/__init__.py b/rats/lib/__init__.py index 7359768..e0a2419 100644 --- a/rats/lib/__init__.py +++ b/rats/lib/__init__.py @@ -17,7 +17,7 @@ import fedmsg from flask_oidc import OpenIDConnect import rats - +import rats.lib.backends _log = logging.getLogger(__name__) oidc = OpenIDConnect() @@ -29,14 +29,15 @@ def get_test_systems(): """ configured_system = rats.config.get('TEST_SYSTEMS') backends = [] - for backend in Backend.__subclasses__(): + for backend in rats.lib.backends.Backend.__subclasses__(): + _log.debug('Found backend: %s' % backend) + print(backend, backend.name, configured_system) if backend.name in configured_system: try: - isinstance(backend(), rats.lib.Backend) - if not isinstance(backend().tests, list): # pragma: no cover - raise TypeError() + isinstance(backend(), rats.lib.backends.Backend) backends.append(backend) - except: # pragma: no cover + except Exception as err: # pragma: no cover + print err _log.debug( '%s is not an instance of rats.lib.Backend' % backend) @@ -57,227 +58,3 @@ def fedmsg_publish(*args, **kwargs): fedmsg.publish(*args, **kwargs) except Exception: _log.exception('Error sending fedmsg') - - -class Backend(object): # pragma: no cover - """ The class to inherit from when creating your own test system backend - to be re-triggered by rats. - """ - - __metaclass__ = abc.ABCMeta - - @abc.abstractproperty - def name(self): - """ Name the test system backend to be triggered. """ - return - - @abc.abstractproperty - def url(self): - """ URL the test system backend to be triggered. """ - return - - @abc.abstractproperty - def description(self): - """ A short description of the type of tested artifacts this backend - supports. """ - return - - @abc.abstractproperty - def tests(self): - """ The list of tests supported by this backend. """ - return - - @abc.abstractmethod - def trigger(self, test, identifier, user, extras=None): - """ This is the method that is called by rats when a test needs to - be re-triggered. - - """ - return - - -class KojiBuildBackend(Backend): - - name = 'KojiBuild' - url = 'https://koji.fedoraproject.org/koji' - description = 'Supports re-triggering tests ran against a koji build' - tests = [ - 'dist.rpmdeplint', - ] - - @classmethod - def trigger(self, test, identifier, user, extras=None): - """ Re-trigger a test against a koji build. """ - _log.info( - 'KojiBuildBackend: %s requests a new run of %s against %s' % ( - user, test, identifier)) - srpm_name = identifier.rsplit('-', 2)[0] - namespace = 'rpms' - # The goal is to support namespace when they arrive, but we don't - # know what their format will be like, so this is just temporary - if '/' in srpm_name: # pragma: no cover - namespace, srpm_name = srpm_name.split('/', 1) - _log.info('srpm name: %s/%s' % (namespace, srpm_name)) - - # Get the project info - project = _get_pagure_project(namespace, srpm_name) - - maintainers = _get_pagure_maintainers(project) - if user not in maintainers: - raise ValueError( - "You're not allowed to retrigger tests for this project") - - # Check if there was a request made recently to lower the DDOS risk - - title = test - # Make all the ``dist.`` tests be taskotron - if test.startswith('dist.'): - title = 'taskotron' - - # All clear, send a fedmsg asking for the test to be re-run - fedmsg_publish( - topic='test.%s' % title, - msg=dict( - test=test, - identifier=identifier, - agent=user, - extras=extras, - ) - ) - - -class BodhiUpdateBackend(Backend): - - name = 'BodhiUpdate' - url = 'https://bodhi.fedoraproject.org/' - description = 'Supports re-triggering tests ran against a bodhi update' - tests = [ - 'org.centos.prod.ci.pipeline.complete', - 'update.base_selinux', - ] - - @classmethod - def trigger(self, test, identifier, user, extras=None): - """ Re-trigger a test against a bodhi update. """ - _log.info( - 'BodhiUpdateBackend: %s requests a new run of %s against %s' % ( - user, test, identifier)) - - maintainers = _get_bodhi_maintainers(identifier) - if user not in maintainers: - raise ValueError( - "You're not allowed to retrigger tests for this project") - - # Check if there was a request made recently to lower the DDOS risk - - title = test - # Make all the ``update.`` tests be taskotron - if test.startswith('update.'): - title = 'taskotron' - - # All clear, send a fedmsg asking for the test to be re-run - fedmsg_publish( - topic='test.%s' % title, - msg=dict( - test=test, - identifier=identifier, - agent=user, - extras=extras, - ) - ) - - -class PagurePRBackend(Backend): - - name = 'PagurePR' - url = 'https://src.fedoraproject.org/' - description = 'Supports re-triggering tests ran against a pull-request '\ - 'on pagure' - tests = [ - 'simple-koji-ci', - 'AtomicCi', - ] - - @classmethod - def trigger(self, test, identifier, user, extras=None): - """ Re-trigger a test against a pull-request on pagure. """ - _log.info( - 'PagurePRBackend: %s requests a new run of %s against %s' % ( - user, test, identifier)) - - # Get the PR info - distgit_api = '%s/api/0/' % rats.config['DIST_GIT_URL'].rstrip('/') - url = '%s/%s' % (distgit_api.rstrip('/'), identifier) - _log.info('Querying %s' % url) - req = requests.get(url, timeout=30) - if not req.ok: - raise ValueError("Call to %s didn't return as expected" % url) - pull_request = req.json() - - # From the PR go to the project - project = _get_pagure_project( - pull_request['project']['namespace'], - pull_request['project']['name']) - - maintainers = _get_pagure_maintainers(project) - if user not in maintainers: - raise ValueError( - "You're not allowed to retrigger tests for this project") - - # Check if there was a request made recently to lower the DDOS risk - - # All clear, send a fedmsg asking for the test to be re-run - fedmsg_publish( - topic='test.%s' % test, - msg=dict( - pull_request=pull_request, - agent=user, - test=test, - identifier=identifier, - extras=extras, - ) - ) - - -def _get_pagure_project(namespace, name): - """ Retrieve the JSON blob about a pagure project given its name and - namespace. - """ - distgit_api = '%s/api/0/' % rats.config['DIST_GIT_URL'].rstrip('/') - url = '%s/%s/%s?expand_group=1' % ( - distgit_api.rstrip('/'), namespace, name) - _log.info('Querying %s' % url) - req = requests.get(url, timeout=30) - if not req.ok: # pragma: no cover - raise ValueError("Call to %s didn't return as expected" % url) - return req.json() - - -def _get_pagure_maintainers(data): - """ Provided a json blob of a project (with expanded groups), returns - the list of all the maintainers. - """ - maintainers = set() - maintainers.add(data['user']['name']) - for k in ['admin', 'commit']: - for user in data['access_users'][k]: - maintainers.add(user) - for grp in data['access_groups'][k]: - maintainers.update(set(data['group_details'][grp])) - _log.info('Maintainers found: %s' % maintainers) - return maintainers - - -def _get_bodhi_maintainers(identifier): - """ Provided an update identifier, returns the list of people allowed - to edit/interract with. - """ - url = '%s/updates/%s' % (rats.config['BODHI_URL'].rstrip('/'), identifier) - _log.info('Querying %s' % url) - req = requests.get(url, timeout=30) - if not req.ok: # pragma: no cover - raise ValueError("Call to %s didn't return as expected" % url) - data = req.json() - maintainers = set([data['update']['user']['name']]) - _log.info('Maintainers found: %s' % maintainers) - return maintainers diff --git a/rats/lib/backends/__init__.py b/rats/lib/backends/__init__.py new file mode 100644 index 0000000..d5a6ca3 --- /dev/null +++ b/rats/lib/backends/__init__.py @@ -0,0 +1,15 @@ +# -*- coding: utf-8 -*- + +""" + (c) 2017 - Copyright Red Hat Inc + + Authors: + Pierre-Yves Chibon + +""" + +from rats.lib.backends.base import Backend +from rats.lib.backends.atomic_ci import AtomicCiBackend +from rats.lib.backends.taskotron import TaskotronBackend +from rats.lib.backends.simple_koji_ci import SimpleKojiCiBackend + diff --git a/rats/lib/backends/atomic_ci.py b/rats/lib/backends/atomic_ci.py new file mode 100644 index 0000000..929b952 --- /dev/null +++ b/rats/lib/backends/atomic_ci.py @@ -0,0 +1,115 @@ +# -*- coding: utf-8 -*- + +""" + (c) 2017 - Copyright Red Hat Inc + + Authors: + Pierre-Yves Chibon + +""" + + +import abc +import logging + +import requests + +import rats.lib +from rats.lib.backends.base import ( + Backend, + get_pagure_maintainers, + get_pagure_project, + get_resultdb_result, +) + +_log = logging.getLogger(__name__) + + +class AtomicCiBackend(Backend): + """ The class to re-trigger tests ran in the AtomicCI pipeline. + """ + + name = 'AtomicCI' + url = 'https://fedoraproject.org/wiki/CI' + description = 'Supports re-triggering tests in Taskotron' + + @classmethod + def interested_by(self, info): + """ Returns whether this backend is interested in re-running this + test. + + """ + result_id = info.get('result_id') + extras = info.get('extras', {}) + + answer = False + if result_id is not None: + results = get_resultdb_result(result_id) + # This isn't part of resultsdb's results atm + if results.get('source', '').lower() == self.name.lower(): # pragma: no cover + answer = True + elif results.get('testcase', {}).get( + 'name', '').startswith('org.centos.prod.ci.pipeline'): + answer = True + else: + test = info['test'] + if test.lower() == self.name.lower(): + answer = True + + return answer + + @classmethod + def trigger(self, info, user): + """ This is the method that is called by rats when a test needs to + be re-triggered. + + """ + result_id = info.get('result_id') + extras = info.get('extras', {}) + identifier = info.get('item') + + _log.info( + 'AtomicCiBackend: %s requests a new run with: %s' % (user, info)) + + pull_request = None + old_result = None + if result_id is not None: + old_result = get_resultdb_result(result_id) + + elif identifier and '/pull-request/' in identifier: + test = info['type'] + # Get the PR info + distgit_api = '%s/api/0/' % ( + rats.config['DIST_GIT_URL'].rstrip('/')) + url = '%s/%s' % (distgit_api.rstrip('/'), identifier) + _log.info('Querying %s' % url) + req = requests.get(url, timeout=30) + if not req.ok: # pragma: no cover + raise ValueError("Call to %s didn't return as expected" % url) + pull_request = req.json() + + # From the PR go to the project + project = get_pagure_project( + pull_request['project']['namespace'], + pull_request['project']['name']) + + maintainers = get_pagure_maintainers(project) + if user not in maintainers: + raise ValueError( + "You're not allowed to retrigger tests for this project") + + # Check if there was a request made recently to lower the DDOS risk + + # All clear, send a fedmsg asking for the test to be re-run + rats.lib.fedmsg_publish( + topic='test.atomic-ci', + msg=dict( + pull_request=pull_request, + old_result=old_result, + agent=user, + test=info.get('test'), + identifier=info.get('identifier'), + extras=info.get('extras', {}), + result_id=info.get('result_id'), + ) + ) diff --git a/rats/lib/backends/base.py b/rats/lib/backends/base.py new file mode 100644 index 0000000..90babb0 --- /dev/null +++ b/rats/lib/backends/base.py @@ -0,0 +1,116 @@ +# -*- coding: utf-8 -*- + +""" + (c) 2017 - Copyright Red Hat Inc + + Authors: + Pierre-Yves Chibon + +""" + +import abc +import logging + +import requests + +import rats + +_log = logging.getLogger(__name__) + + +class Backend(object): # pragma: no cover + """ The class to inherit from when creating your own test system backend + to be re-triggered by rats. + """ + + __metaclass__ = abc.ABCMeta + + @abc.abstractproperty + def name(self): + """ Name the test system backend to be triggered. """ + return + + @abc.abstractproperty + def url(self): + """ URL the test system backend to be triggered. """ + return + + @abc.abstractproperty + def description(self): + """ A short description of the type of tested artifacts this backend + supports. """ + return + + @abc.abstractmethod + def interested_by( + self, test, identifier, user, extras=None, results_id=None): + """ Returns whether this backend is interested in re-running this + test. + + """ + return + + @abc.abstractmethod + def trigger(self, test, identifier, user, extras=None, results_id=None): + """ This is the method that is called by rats when a test needs to + be re-triggered. + + """ + return + + +def get_resultdb_result(result_id): + """ Retrieve the JSON blob about a specified result in resultsdb. + """ + distgit_api = '%s/' % rats.config['RESULTSDB_API_URL'].rstrip('/') + url = '%s/api/v2.0/results/%s' % (distgit_api.rstrip('/'), result_id) + _log.info('Querying %s' % url) + req = requests.get(url, timeout=30) + if not req.ok: # pragma: no cover + raise ValueError("Call to %s didn't return as expected" % url) + return req.json() + + +def get_pagure_project(namespace, name): + """ Retrieve the JSON blob about a pagure project given its name and + namespace. + """ + distgit_api = '%s/api/0/' % rats.config['DIST_GIT_URL'].rstrip('/') + url = '%s/%s/%s?expand_group=1' % ( + distgit_api.rstrip('/'), namespace, name) + _log.info('Querying %s' % url) + req = requests.get(url, timeout=30) + if not req.ok: # pragma: no cover + raise ValueError("Call to %s didn't return as expected" % url) + return req.json() + + +def get_pagure_maintainers(data): + """ Provided a json blob of a project (with expanded groups), returns + the list of all the maintainers. + """ + maintainers = set() + maintainers.add(data['user']['name']) + for k in ['admin', 'commit']: + for user in data['access_users'][k]: + maintainers.add(user) + for grp in data['access_groups'][k]: + maintainers.update(set(data['group_details'][grp])) + _log.info('Maintainers found: %s' % maintainers) + return maintainers + + +def get_bodhi_maintainers(identifier): + """ Provided an update identifier, returns the list of people allowed + to edit/interract with. + """ + url = '%s/updates/%s' % (rats.config['BODHI_URL'].rstrip('/'), identifier) + _log.info('Querying %s' % url) + req = requests.get(url, timeout=30) + if not req.ok: # pragma: no cover + raise ValueError("Call to %s didn't return as expected" % url) + data = req.json() + maintainers = set([data['update']['user']['name']]) + _log.info('Maintainers found: %s' % maintainers) + return maintainers + diff --git a/rats/lib/backends/simple_koji_ci.py b/rats/lib/backends/simple_koji_ci.py new file mode 100644 index 0000000..b36979e --- /dev/null +++ b/rats/lib/backends/simple_koji_ci.py @@ -0,0 +1,105 @@ +# -*- coding: utf-8 -*- + +""" + (c) 2017 - Copyright Red Hat Inc + + Authors: + Pierre-Yves Chibon + +""" + + +import abc +import logging + +import requests + +import rats.lib +from rats.lib.backends.base import ( + Backend, + get_pagure_maintainers, + get_pagure_project, + get_resultdb_result, +) + +_log = logging.getLogger(__name__) + + +class SimpleKojiCiBackend(Backend): + """ The class to re-trigger tests ran by simple-koji-ci. + """ + + name = 'simple-koji-ci' + url = 'https://pagure.io/simple-koji-ci' + description = 'Supports re-triggering tests by simple-koji-ci' + + @classmethod + def interested_by(self, info): + """ Returns whether this backend is interested in re-running this + test. + + """ + answer = None + + # This backend doesn't publish to resultsdb, so won't work with + # result_id + for key in ['test', 'item', 'type']: + if key not in info: + answer = False + + if answer is None: + test = info['test'] + type_ = info['type'].lower() + if test == self.name.lower() and type_ == 'pagure_pr': + answer = True + else: + answer = False + + return answer + + @classmethod + def trigger(self, info, user): + """ This is the method that is called by rats when a test needs to + be re-triggered. + + """ + identifier = info['item'] + test = info['test'] + + _log.info( + 'SimpleKojiCiBackend: %s requests a new run with: %s' % + (user, info)) + + # Get the PR info + distgit_api = '%s/api/0/' % rats.config['DIST_GIT_URL'].rstrip('/') + url = '%s/%s' % (distgit_api.rstrip('/'), identifier) + _log.info('Querying %s' % url) + req = requests.get(url, timeout=30) + if not req.ok: + raise ValueError("Call to %s didn't return as expected" % url) + pull_request = req.json() + + # From the PR go to the project + project = get_pagure_project( + pull_request['project']['namespace'], + pull_request['project']['name']) + + maintainers = get_pagure_maintainers(project) + if user not in maintainers: + raise ValueError( + "You're not allowed to retrigger tests for this project") + + # Check if there was a request made recently to lower the DDOS risk + + # All clear, send a fedmsg asking for the test to be re-run + rats.lib.fedmsg_publish( + topic='test.%s' % test, + msg=dict( + pull_request=pull_request, + agent=user, + test=info['test'], + identifier=info['item'], + extras=info.get('extras', {}), + result_id=info.get('result_id'), + ) + ) diff --git a/rats/lib/backends/taskotron.py b/rats/lib/backends/taskotron.py new file mode 100644 index 0000000..814d07a --- /dev/null +++ b/rats/lib/backends/taskotron.py @@ -0,0 +1,149 @@ +# -*- coding: utf-8 -*- + +""" + (c) 2017 - Copyright Red Hat Inc + + Authors: + Pierre-Yves Chibon + +""" + + +import abc +import logging + +import requests + +import rats.lib +from rats.lib.backends.base import ( + Backend, + get_bodhi_maintainers, + get_pagure_maintainers, + get_pagure_project, + get_resultdb_result, +) + +_log = logging.getLogger(__name__) + + +class TaskotronBackend(Backend): + """ The class to re-trigger tests ran by taskotron. + """ + + name = 'Taskotron' + url = 'https://fedoraproject.org/wiki/Taskotron' + description = 'Supports re-triggering tests in the Atomic CI pipeline' + + @classmethod + def interested_by(self, info): + """ Returns whether this backend is interested in re-running this + test. + + """ + result_id = info.get('result_id') + extras = info.get('extras', {}) + + answer = False + if result_id is not None: + results = get_resultdb_result(result_id) + if not 'groups' in results and not results['groups']: # pragma: no cover + # This should never happen in theory + answer = False + elif results.get('source', '').lower() == self.name.lower(): # pragma: no cover + # This isn't implemented in taskotron yet + answer = True + elif results.get('testcase', {}).get( + 'name', '').startswith(('dist.', 'update.')): + answer = True + else: + test = info['test'] + if not 'groups' in extras: + answer = False + elif test.startswith(('dist.', 'update.')): + answer = True + + return answer + + @classmethod + def trigger(self, info, user): + """ This is the method that is called by rats when a test needs to + be re-triggered. + + """ + result_id = info.get('result_id') + extras = info.get('extras', {}) + + _log.info( + 'TaskotronBackend: %s requests a new run with: %s' % (user, info)) + + groups = None + if result_id is not None: + results = get_resultdb_result(result_id) + if not 'groups' in results and not results['groups']: # pragma: no cover + # Should never happen in theory + raise ValueError( + 'Invalid results found for %s, no groups ' + 'defined' % result_id) + groups = results['groups'] + identifier = results['data']['item'][0] + type_ = results['data']['type'][0] + else: + if not 'groups' in extras: + raise ValueError( + 'Invalid input, no groups provided in extras') + groups = extras['groups'] + type_ = info['type'] + identifier = info['item'] + + # Check if the user is allowed to re-run the test + if type_.lower() == 'koji_build': + srpm_name = identifier.rsplit('-', 2)[0] + namespace = 'rpms' + _log.info('srpm name: %s/%s' % (namespace, srpm_name)) + + # Get the project info + project = get_pagure_project(namespace, srpm_name) + + maintainers = get_pagure_maintainers(project) + if user not in maintainers: + raise ValueError( + "You're not allowed to retrigger tests for " + "this project") + elif type_.lower() == 'module_build': + srpm_name = identifier.rsplit('-', 2)[0].split('#', 1)[0] + if '/' in srpm_name: + srpm_name = srpm_name.split('/', 1)[1] + namespace = 'modules' + _log.info('srpm name: %s/%s' % (namespace, srpm_name)) + + # Get the project info + project = get_pagure_project(namespace, srpm_name) + + maintainers = get_pagure_maintainers(project) + if user not in maintainers: + raise ValueError( + "You're not allowed to retrigger tests for " + "this project") + elif type_.lower() == 'bodhi_update': + maintainers = get_bodhi_maintainers(identifier) + if user not in maintainers: + raise ValueError( + "You're not allowed to retrigger tests for " + "this project") + + # Check if there was a request made recently to lower the DDOS risk + + # All clear, send a fedmsg asking for the test to be re-run + rats.lib.fedmsg_publish( + topic='test.%s' % self.name.lower(), + msg=dict( + test=info.get('test'), + identifier=info.get('item'), + agent=user, + extras=info.get('extras', {}), + result_id=info.get('result_id'), + groups=groups + ) + ) + + diff --git a/rats/lib/tasks.py b/rats/lib/tasks.py index 02d4a22..c7f0814 100644 --- a/rats/lib/tasks.py +++ b/rats/lib/tasks.py @@ -49,23 +49,22 @@ def process_request(info, user): """ Process the request to re-run a test. """ backends = rats.lib.get_test_systems() + print backends tested = False + names = [] for backend in backends: - if backend.name == info['type']: - if info['test'] in backend.tests: - tested = True - backend.trigger( - info['test'], - info['target'], - user, - info.get('extras') - ) + #print info + print backend.name, backend.interested_by(info) + if backend.interested_by(info): + tested = True + backend.trigger(info, user=user) + names.append(backend.name) if not tested: - msg = 'No backend found for %s on %s' % (info['test'], info['type']) + msg = 'No backend found for %s on %s' % (info.get('test'), info['type']) else: msg = '%s has been asked to be re-run on %s' % ( - info['test'], info['target']) + info.get('item', 'test'), ', '.join(names)) return msg diff --git a/rats/ui/ui_app.py b/rats/ui/ui_app.py index 1ad5c39..ceb525a 100644 --- a/rats/ui/ui_app.py +++ b/rats/ui/ui_app.py @@ -65,14 +65,21 @@ def index(): def submit_request(): """ Method to submit one or more tests to be re-run. """ print(flask.request.values) - for key in ['target', 'type', 'test']: - if key not in flask.request.values: - flask.flash( - 'Invalid request', 'error') - return flask.render_template( - 'results.html', - message='Invalid request, no "%s" field specified' % key, - ) + if not 'type' in flask.request.values: + return flask.render_template( + 'results.html', + message='Invalid request, no "type" field specified', + ) + + if not 'result_id' in flask.request.values: + for key in ['item', 'test']: + if key not in flask.request.values: + flask.flash( + 'Invalid request', 'error') + return flask.render_template( + 'results.html', + message='Invalid request, no "%s" field specified' % key, + ) next_url = flask.url_for('ui.process_request') return rats.lib.oidc.redirect_to_auth_server(next_url) @@ -86,19 +93,19 @@ def process_request(): # TODO: retrieve these information from oidc info = { - 'target': 'rpms/python-arrow/pull-request/13', + 'item': 'rpms/python-arrow/pull-request/13', 'type': 'PagurePR', 'test': 'simple-koji-ci', } #info = { - #'target': 'audit-2.8-1.fc26', + #'item': 'audit-2.8-1.fc26', #'type': 'KojiBuild', #'test': 'dist.rpmdeplint', #} #info = { - #'target': 'FEDORA-2017-5e475c0b0d', + #'item': 'FEDORA-2017-5e475c0b0d', #'type': 'BodhiUpdate', #'test': 'update.base_selinux', #} diff --git a/setup.cfg b/setup.cfg index a377bb3..eb06e8d 100644 --- a/setup.cfg +++ b/setup.cfg @@ -2,7 +2,7 @@ cover-erase=TRUE cover-html=TRUE cover-inclusive=TRUE -cover-min-percentage=88 +cover-min-percentage=87 cover-package=rats cover-xml=TRUE with-coverage=TRUE diff --git a/tests/lib/__init__.py b/tests/lib/__init__.py new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/tests/lib/__init__.py diff --git a/tests/lib/backends/__init__.py b/tests/lib/backends/__init__.py new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/tests/lib/backends/__init__.py diff --git a/tests/lib/backends/test_atomic_ci.py b/tests/lib/backends/test_atomic_ci.py new file mode 100644 index 0000000..f200ff0 --- /dev/null +++ b/tests/lib/backends/test_atomic_ci.py @@ -0,0 +1,129 @@ +# -*- coding: utf-8 -*- + +""" + (c) 2017 - Copyright Red Hat Inc + + Authors: + Pierre-Yves Chibon + + +Tests the flask application +""" + + +import logging +import os +import subprocess +import time +import unittest + +from mock import patch, MagicMock + +import rats +import rats.lib.backends + +from tests import BaseTests + +_log = logging.getLogger(__name__) +_log.setLevel(logging.DEBUG) + + +class AtomicCiTaskTests(BaseTests): + """ Tests the Atomic CI tasks. """ + + @patch.dict( + 'rats.config', + {'DIST_GIT_URL': 'https://src.stg.fedoraproject.org'}) + @patch('fedmsg.publish') + def test_process_pagurepr_request_atomic_ci(self, fedmsg_publish): + """ Test process_request for a pagure PR. + """ + + info = { + 'item': 'rpms/python-arrow/pull-request/13', + 'type': 'Pagure_PR', + 'test': 'AtomicCI', + 'extras': {'foo': 'bar', 'items': [1, 2, 3]} + } + + results = rats.lib.tasks.process_request(info, 'pingou') + self.assertEqual( + results, + 'rpms/python-arrow/pull-request/13 has been asked to be re-run ' + 'on AtomicCI') + + args = fedmsg_publish.call_args + self.assertEqual( + args[1].keys(), + ['topic', 'msg', 'modname', 'active', 'cert_prefix']) + self.assertEqual(args[1].get('topic'), 'test.atomic-ci') + self.assertEqual(args[1].get('active'), True) + self.assertEqual(args[1].get('cert_prefix'), 'rats') + self.assertEqual(args[1].get('modname'), 'rats') + self.assertEqual( + sorted(args[1].get('msg').keys()), + ['agent', 'extras', 'identifier', 'old_result', 'pull_request', + 'result_id', 'test'] + ) + self.assertEqual( + args[1]['msg']['extras'], + {'foo': 'bar', 'items': [1, 2, 3]} + ) + + @patch.dict( + 'rats.config', + {'DIST_GIT_URL': 'https://src.stg.fedoraproject.org'}) + @patch('fedmsg.publish') + def test_process_pagurepr_request_atomic_ci_result_id( + self, fedmsg_publish): + """ Test process_request for a pagure PR from a result_id. + """ + + info = { + 'type': 'Pagure_PR', + 'result_id': 17786885, + } + + results = rats.lib.tasks.process_request(info, 'pingou') + self.assertEqual( + results, + 'test has been asked to be re-run on AtomicCI') + + args = fedmsg_publish.call_args + self.assertEqual( + args[1].keys(), + ['topic', 'msg', 'modname', 'active', 'cert_prefix']) + self.assertEqual(args[1].get('topic'), 'test.atomic-ci') + self.assertEqual(args[1].get('active'), True) + self.assertEqual(args[1].get('cert_prefix'), 'rats') + self.assertEqual(args[1].get('modname'), 'rats') + self.assertEqual( + sorted(args[1].get('msg').keys()), + ['agent', 'extras', 'identifier', 'old_result', 'pull_request', + 'result_id', 'test'] + ) + self.assertEqual(args[1]['msg']['extras'], {}) + + @patch.dict( + 'rats.config', + {'DIST_GIT_URL': 'https://src.stg.fedoraproject.org'}) + def test_process_pagurepr_request_atomic_ci_invalid_user(self): + """ Test process_request for a pagure PR. + """ + + info = { + 'item': 'rpms/python-arrow/pull-request/13', + 'type': 'Pagure_PR', + 'test': 'AtomicCI', + 'extras': {'foo': 'bar', 'items': [1, 2, 3]} + } + + self.assertRaises( + ValueError, + rats.lib.tasks.process_request, + info, + 'foobar' + ) + +if __name__ == '__main__': + unittest.main(verbosity=2) diff --git a/tests/lib/backends/test_simple_koji_ci.py b/tests/lib/backends/test_simple_koji_ci.py new file mode 100644 index 0000000..f2173d5 --- /dev/null +++ b/tests/lib/backends/test_simple_koji_ci.py @@ -0,0 +1,109 @@ +# -*- coding: utf-8 -*- + +""" + (c) 2017 - Copyright Red Hat Inc + + Authors: + Pierre-Yves Chibon + + +Tests the flask application +""" + + +import logging +import os +import subprocess +import time +import unittest + +from mock import patch, MagicMock + +import rats +import rats.lib.backends + +from tests import BaseTests + +_log = logging.getLogger(__name__) +_log.setLevel(logging.DEBUG) + + +class SimpleKojiCiTaskTests(BaseTests): + """ Tests the simple-koji-ci tasks. """ + + def test_process_pr_invalid_user(self): + """ Test process_request for a PR when the requester is not allowed. + """ + + info = { + 'item': 'rpms/python-arrow/pull-request/13', + 'type': 'Pagure_PR', + 'test': 'simple-koji-ci', + } + + self.assertRaises( + ValueError, + rats.lib.tasks.process_request, + info, + 'trasher' + ) + + @patch.dict( + 'rats.config', + {'DIST_GIT_URL': 'https://src.stg.fedoraproject.org'}) + def test_process_pr_invalid_user_right_source(self): + """ Test process_request for a PR when the requester is not allowed. + """ + + info = { + 'item': 'rpms/python-arrow/pull-request/13', + 'type': 'Pagure_PR', + 'test': 'simple-koji-ci', + } + + self.assertRaises( + ValueError, + rats.lib.tasks.process_request, + info, + 'trasher' + ) + + @patch.dict( + 'rats.config', + {'DIST_GIT_URL': 'https://src.stg.fedoraproject.org'}) + @patch('fedmsg.publish') + def test_process_pr(self, fedmsg_publish): + """ Test process_request for a PR. """ + + info = { + 'item': 'rpms/python-arrow/pull-request/13', + 'type': 'Pagure_PR', + 'test': 'simple-koji-ci', + } + + results = rats.lib.tasks.process_request(info, 'pingou') + self.assertEqual( + results, + 'rpms/python-arrow/pull-request/13 has been asked to be re-run' + ' on simple-koji-ci') + + args = fedmsg_publish.call_args + self.assertEqual( + args[1].keys(), + ['topic', 'msg', 'modname', 'active', 'cert_prefix']) + self.assertEqual(args[1].get('topic'), 'test.simple-koji-ci') + self.assertEqual(args[1].get('active'), True) + self.assertEqual(args[1].get('cert_prefix'), 'rats') + self.assertEqual(args[1].get('modname'), 'rats') + self.assertEqual( + sorted(args[1].get('msg').keys()), + ['agent', 'extras', 'identifier', 'pull_request', 'result_id', + 'test'] + ) + self.assertEqual(args[1]['msg']['extras'], {}) + self.assertEqual(args[1]['msg']['test'], 'simple-koji-ci') + + + +if __name__ == '__main__': + unittest.main(verbosity=2) diff --git a/tests/lib/backends/test_taskotron.py b/tests/lib/backends/test_taskotron.py new file mode 100644 index 0000000..a5cb8d9 --- /dev/null +++ b/tests/lib/backends/test_taskotron.py @@ -0,0 +1,282 @@ +# -*- coding: utf-8 -*- + +""" + (c) 2017 - Copyright Red Hat Inc + + Authors: + Pierre-Yves Chibon + + +Tests the flask application +""" + + +import logging +import os +import subprocess +import time +import unittest + +from mock import patch, MagicMock + +import rats +import rats.lib.backends + +from tests import BaseTests + +_log = logging.getLogger(__name__) +_log.setLevel(logging.DEBUG) + + +class TaskotronTaskTests(BaseTests): + """ Tests the taskotron tasks. """ + + def test_process_invalid_backend(self): + """ Test process_request for a bodhi update. """ + + info = { + 'item': 'FEDORA-2017-5e475c0b0d', + 'type': 'foo', + 'test': 'update.base_selinux', + } + + results = rats.lib.tasks.process_request(info, 'sgrubb') + self.assertEqual( + results, 'No backend found for update.base_selinux on foo') + + def test_process_bodhi_request_invalid_user(self): + """ Test process_request for a bodhi update when the requester is + not allowed. + """ + + info = { + 'item': 'FEDORA-2017-5e475c0b0d', + 'type': 'Bodhi_Update', + 'test': 'update.base_selinux', + 'extras': {"groups": "foobar"}, + } + + self.assertRaises( + ValueError, + rats.lib.tasks.process_request, + info, + 'pingou' + ) + + @patch('fedmsg.publish') + def test_process_bodhi_request(self, fedmsg_publish): + """ Test process_request for a bodhi update. """ + + info = { + 'item': 'FEDORA-2017-5e475c0b0d', + 'type': 'Bodhi_Update', + 'test': 'update.base_selinux', + 'extras': {"groups": "foobar"}, + } + + results = rats.lib.tasks.process_request(info, 'sgrubb') + self.assertEqual( + results, + 'FEDORA-2017-5e475c0b0d has been asked to be re-run on ' + 'Taskotron') + + args = fedmsg_publish.call_args + self.assertEqual( + args[1].keys(), + ['topic', 'msg', 'modname', 'active', 'cert_prefix']) + self.assertEqual(args[1].get('topic'), 'test.taskotron') + self.assertEqual(args[1].get('active'), True) + self.assertEqual(args[1].get('cert_prefix'), 'rats') + self.assertEqual(args[1].get('modname'), 'rats') + self.assertEqual( + sorted(args[1].get('msg').keys()), + ['agent', 'extras', 'groups', 'identifier', 'result_id', 'test'] + ) + self.assertEqual(args[1]['msg']['extras'], {'groups': 'foobar'}) + self.assertEqual(args[1]['msg']['test'], 'update.base_selinux') + + @patch('fedmsg.publish') + def test_process_bodhi_request_result_id(self, fedmsg_publish): + """ Test process_request for a bodhi update. """ + + info = { + 'result_id': 17783947, + 'type': 'Bodhi_Update', + } + + results = rats.lib.tasks.process_request(info, 'kkeithle') + self.assertEqual( + results, + 'test has been asked to be re-run on Taskotron') + + args = fedmsg_publish.call_args + self.assertEqual( + args[1].keys(), + ['topic', 'msg', 'modname', 'active', 'cert_prefix']) + self.assertEqual(args[1].get('topic'), 'test.taskotron') + self.assertEqual(args[1].get('active'), True) + self.assertEqual(args[1].get('cert_prefix'), 'rats') + self.assertEqual(args[1].get('modname'), 'rats') + self.assertEqual( + sorted(args[1].get('msg').keys()), + ['agent', 'extras', 'groups', 'identifier', 'result_id', 'test'] + ) + self.assertEqual(args[1]['msg']['extras'], {}) + self.assertEqual(args[1]['msg']['test'], None) + + @patch('fedmsg.publish', MagicMock(side_effect=Exception)) + def test_process_bodhi_request_no_fedmsg(self): + """ Test process_request for a bodhi update. """ + + info = { + 'item': 'FEDORA-2017-5e475c0b0d', + 'type': 'Bodhi_Update', + 'test': 'update.base_selinux', + 'extras': {"groups": "foobar"}, + } + + results = rats.lib.tasks.process_request(info, 'sgrubb') + self.assertEqual( + results, + 'FEDORA-2017-5e475c0b0d has been asked to be re-run on ' + 'Taskotron') + + @patch('fedmsg.publish') + def test_process_module_build(self, fedmsg_publish): + """ Test process_request for a module build. """ + + info = { + 'item': 'modules/udisks2#817449ed6fade1ef5868008df2df697f717c4879', + 'type': 'Module_Build', + 'test': 'dist.modulemd', + 'extras': {"groups": "foobar"}, + } + + results = rats.lib.tasks.process_request(info, 'hhorak') + self.assertEqual( + results, + 'modules/udisks2#817449ed6fade1ef5868008df2df697f717c4879 has ' + 'been asked to be re-run on Taskotron') + + args = fedmsg_publish.call_args + self.assertEqual( + args[1].keys(), + ['topic', 'msg', 'modname', 'active', 'cert_prefix']) + self.assertEqual(args[1].get('topic'), 'test.taskotron') + self.assertEqual(args[1].get('active'), True) + self.assertEqual(args[1].get('cert_prefix'), 'rats') + self.assertEqual(args[1].get('modname'), 'rats') + self.assertEqual( + sorted(args[1].get('msg').keys()), + ['agent', 'extras', 'groups', 'identifier', 'result_id', 'test'] + ) + self.assertEqual(args[1]['msg']['extras'], {'groups': 'foobar'}) + self.assertEqual(args[1]['msg']['test'], 'dist.modulemd') + + def test_process_module_build_invalid_user(self): + """ Test process_request for a module build with an invalid user. """ + + info = { + 'item': 'modules/udisks2#817449ed6fade1ef5868008df2df697f717c4879', + 'type': 'Module_Build', + 'test': 'dist.modulemd', + 'extras': {"groups": "foobar"}, + } + + self.assertRaises( + ValueError, + rats.lib.tasks.process_request, + info, + 'pingou' + ) + + def test_process_koji_request_invalid_user(self): + """ Test process_request for a koji build when the requester is + not allowed. + """ + + info = { + 'item': 'audit-2.8-1.fc26', + 'type': 'Koji_Build', + 'test': 'dist.rpmdeplint', + 'extras': {"groups": "foobar"}, + } + + self.assertRaises( + ValueError, + rats.lib.tasks.process_request, + info, + 'pingou' + ) + + @patch('fedmsg.publish') + def test_process_koji_request(self, fedmsg_publish): + """ Test process_request for a koji build. """ + + info = { + 'item': 'audit-2.8-1.fc26', + 'type': 'Koji_Build', + 'test': 'dist.rpmdeplint', + 'extras': {"groups": "foobar"}, + } + + results = rats.lib.tasks.process_request(info, 'sgrubb') + self.assertEqual( + results, + 'audit-2.8-1.fc26 has been asked to be re-run on ' + 'Taskotron') + + args = fedmsg_publish.call_args + self.assertEqual( + args[1].keys(), + ['topic', 'msg', 'modname', 'active', 'cert_prefix']) + self.assertEqual(args[1].get('topic'), 'test.taskotron') + self.assertEqual(args[1].get('active'), True) + self.assertEqual(args[1].get('cert_prefix'), 'rats') + self.assertEqual(args[1].get('modname'), 'rats') + self.assertEqual( + sorted(args[1].get('msg').keys()), + ['agent', 'extras', 'groups', 'identifier', 'result_id', 'test'] + ) + self.assertEqual(args[1]['msg']['extras'], {'groups': 'foobar'}) + self.assertEqual(args[1]['msg']['test'], 'dist.rpmdeplint') + + def test_taskotron_trigger(self): + """ Test taskotron's trigger for a bodhi update. """ + + info = { + 'item': 'FEDORA-2017-5e475c0b0d', + 'type': 'foo', + 'test': 'update.base_selinux', + } + + backend = rats.lib.backends.TaskotronBackend + + self.assertRaises( + ValueError, + backend.trigger, + info, + 'sgrubb' + ) + + def test_taskotron_trigger_result_id(self): + """ Test taskotron's trigger for a bodhi update from a result_id. + """ + + info = { + 'result_id': 17783947, + 'type': 'Bodhi_Update', + } + + backend = rats.lib.backends.TaskotronBackend + + self.assertRaises( + ValueError, + backend.trigger, + info, + 'pingou' + ) + + +if __name__ == '__main__': + unittest.main(verbosity=2) diff --git a/tests/lib/test_tasks.py b/tests/lib/test_tasks.py index 9ad9f25..90e17ef 100644 --- a/tests/lib/test_tasks.py +++ b/tests/lib/test_tasks.py @@ -91,7 +91,7 @@ class TasksTests(BaseTests): """ Test process_request with an invalid backend and async. """ info = { - 'target': 'FEDORA-2017-5e475c0b0d', + 'item': 'FEDORA-2017-5e475c0b0d', 'type': 'foo', 'test': 'update.base_selinux', } @@ -108,213 +108,6 @@ class TasksTests(BaseTests): results, 'No backend found for update.base_selinux on foo') -class TasksTestsSimple(BaseTests): - """ Tasks tests simple, without broker. """ - - def test_process_invalid_backend(self): - """ Test process_request for a bodhi update. """ - - info = { - 'target': 'FEDORA-2017-5e475c0b0d', - 'type': 'foo', - 'test': 'update.base_selinux', - } - - results = rats.lib.tasks.process_request(info, 'sgrubb') - self.assertEqual( - results, 'No backend found for update.base_selinux on foo') - - def test_process_bodhi_request_invalid_user(self): - """ Test process_request for a bodhi update when the requester is - not allowed. - """ - - info = { - 'target': 'FEDORA-2017-5e475c0b0d', - 'type': 'BodhiUpdate', - 'test': 'update.base_selinux', - } - - self.assertRaises( - ValueError, - rats.lib.tasks.process_request, - info, - 'pingou' - ) - - @patch('fedmsg.publish') - def test_process_bodhi_request(self, fedmsg_publish): - """ Test process_request for a bodhi update. """ - - info = { - 'target': 'FEDORA-2017-5e475c0b0d', - 'type': 'BodhiUpdate', - 'test': 'update.base_selinux', - } - - results = rats.lib.tasks.process_request(info, 'sgrubb') - self.assertEqual( - results, - 'update.base_selinux has been asked to be re-run on ' - 'FEDORA-2017-5e475c0b0d') - - args = fedmsg_publish.call_args - self.assertEqual( - args[1].keys(), - ['topic', 'msg', 'modname', 'active', 'cert_prefix']) - self.assertEqual(args[1].get('topic'), 'test.taskotron') - self.assertEqual(args[1].get('active'), True) - self.assertEqual(args[1].get('cert_prefix'), 'rats') - self.assertEqual(args[1].get('modname'), 'rats') - self.assertEqual( - args[1].get('msg').keys(), - ['test', 'identifier', 'agent', 'extras'] - ) - self.assertIsNone(args[1]['msg']['extras']) - self.assertEqual(args[1]['msg']['test'], 'update.base_selinux') - - @patch('fedmsg.publish', MagicMock(side_effect=Exception)) - def test_process_bodhi_request_no_fedmsg(self): - """ Test process_request for a bodhi update. """ - - info = { - 'target': 'FEDORA-2017-5e475c0b0d', - 'type': 'BodhiUpdate', - 'test': 'update.base_selinux', - } - - results = rats.lib.tasks.process_request(info, 'sgrubb') - self.assertEqual( - results, - 'update.base_selinux has been asked to be re-run on ' - 'FEDORA-2017-5e475c0b0d') - - def test_process_koji_request_invalid_user(self): - """ Test process_request for a koji build when the requester is - not allowed. - """ - - info = { - 'target': 'audit-2.8-1.fc26', - 'type': 'KojiBuild', - 'test': 'dist.rpmdeplint', - } - - self.assertRaises( - ValueError, - rats.lib.tasks.process_request, - info, - 'pingou' - ) - - @patch('fedmsg.publish') - def test_process_koji_request(self, fedmsg_publish): - """ Test process_request for a koji build. """ - - info = { - 'target': 'audit-2.8-1.fc26', - 'type': 'KojiBuild', - 'test': 'dist.rpmdeplint', - } - - results = rats.lib.tasks.process_request(info, 'sgrubb') - self.assertEqual( - results, - 'dist.rpmdeplint has been asked to be re-run on ' - 'audit-2.8-1.fc26') - - args = fedmsg_publish.call_args - self.assertEqual( - args[1].keys(), - ['topic', 'msg', 'modname', 'active', 'cert_prefix']) - self.assertEqual(args[1].get('topic'), 'test.taskotron') - self.assertEqual(args[1].get('active'), True) - self.assertEqual(args[1].get('cert_prefix'), 'rats') - self.assertEqual(args[1].get('modname'), 'rats') - self.assertEqual( - args[1].get('msg').keys(), - ['test', 'identifier', 'agent', 'extras'] - ) - self.assertIsNone(args[1]['msg']['extras']) - self.assertEqual(args[1]['msg']['test'], 'dist.rpmdeplint') - - @patch.dict( - 'rats.config', - {'DIST_GIT_URL': 'https://src.stg.fedoraproject.org'}) - def test_process_pagurepr_request_invalid_user(self): - """ Test process_request for a pagure PR when the requester is not - allowed. - """ - - info = { - 'target': 'rpms/python-arrow/pull-request/13', - 'type': 'PagurePR', - 'test': 'simple-koji-ci', - } - - self.assertRaises( - ValueError, - rats.lib.tasks.process_request, - info, - 'remi' - ) - - def test_process_pagurepr_request_invalid_url(self): - """ Test process_request for a pagure PR when the requester is not - allowed. - """ - - info = { - 'target': 'rpms/python-arrow/pull-request/13', - 'type': 'PagurePR', - 'test': 'simple-koji-ci', - } - - self.assertRaises( - ValueError, - rats.lib.tasks.process_request, - info, - 'pingou' - ) - - @patch.dict( - 'rats.config', - {'DIST_GIT_URL': 'https://src.stg.fedoraproject.org'}) - @patch('fedmsg.publish') - def test_process_pagurepr_request(self, fedmsg_publish): - """ Test process_request for a pagure PR. - """ - - info = { - 'target': 'rpms/python-arrow/pull-request/13', - 'type': 'PagurePR', - 'test': 'simple-koji-ci', - 'extras': {'foo': 'bar', 'items': [1, 2, 3]} - } - - results = rats.lib.tasks.process_request(info, 'pingou') - self.assertEqual( - results, - 'simple-koji-ci has been asked to be re-run on ' - 'rpms/python-arrow/pull-request/13') - - args = fedmsg_publish.call_args - self.assertEqual( - args[1].keys(), - ['topic', 'msg', 'modname', 'active', 'cert_prefix']) - self.assertEqual(args[1].get('topic'), 'test.simple-koji-ci') - self.assertEqual(args[1].get('active'), True) - self.assertEqual(args[1].get('cert_prefix'), 'rats') - self.assertEqual(args[1].get('modname'), 'rats') - self.assertEqual( - args[1].get('msg').keys(), - ['test', 'identifier', 'extras', 'agent', 'pull_request'] - ) - self.assertEqual( - args[1]['msg']['extras'], - {'foo': 'bar', 'items': [1, 2, 3]} - ) - if __name__ == '__main__': unittest.main(verbosity=2) diff --git a/tests/ui/test_ui_app.py b/tests/ui/test_ui_app.py index d50c617..1fd4090 100644 --- a/tests/ui/test_ui_app.py +++ b/tests/ui/test_ui_app.py @@ -38,19 +38,19 @@ class AppTests(BaseTests): 'tests in', output.data.decode('utf-8') ) self.assertIn( - 'KojiBuild', + 'AtomicCI', output.data.decode('utf-8') ) self.assertIn( - 'a href="https://bodhi.fedoraproject.org/">BodhiUpdate', + 'a href="https://fedoraproject.org/wiki/Taskotron">Taskotron', output.data.decode('utf-8') ) self.assertIn( - 'PagurePR', + 'simple-koji-ci', output.data.decode('utf-8') ) - @patch.dict('rats.config', {'TEST_SYSTEMS': ['PagurePR']}) + @patch.dict('rats.config', {'TEST_SYSTEMS': ['simple-koji-ci']}) def test_index_other_backend(self): """ Test the index page. """ output = self.app.get('/') @@ -64,15 +64,15 @@ class AppTests(BaseTests): 'tests in', output.data.decode('utf-8') ) self.assertNotIn( - 'KojiBuild', + 'AtomicCI', output.data.decode('utf-8') ) self.assertNotIn( - 'a href="https://bodhi.fedoraproject.org/">BodhiUpdate', + 'a href="https://fedoraproject.org/wiki/Taskotron">Taskotron', output.data.decode('utf-8') ) self.assertIn( - 'PagurePR', + 'simple-koji-ci', output.data.decode('utf-8') ) @@ -81,7 +81,7 @@ class AppTests(BaseTests): output = self.app.get('/submit') self.assertEqual(output.status_code, 200) self.assertIn( - 'Invalid request, no "target" field specified', + 'Invalid request, no "type" field specified', output.data.decode('utf-8') ) @@ -95,14 +95,14 @@ class AppTests(BaseTests): def test_submit_no_type(self): """ Test the submit page when no type is provided. """ - output = self.app.get('/submit?target=audit-2.8-1.fc26') + output = self.app.get('/submit?item=audit-2.8-1.fc26') self.assertEqual(output.status_code, 200) self.assertIn( 'Invalid request, no "type" field specified', output.data.decode('utf-8') ) - data = {'target': 'audit-2.8-1.fc26'} + data = {'item': 'audit-2.8-1.fc26'} output2 = self.app.post('/submit', data=data) self.assertEqual(output.status_code, 200) self.assertEqual( @@ -113,14 +113,32 @@ class AppTests(BaseTests): def test_submit_no_test(self): """ Test the submit page when no test is provided. """ - output = self.app.get('/submit?target=audit-2.8-1.fc26&type=KojiBuild') + output = self.app.get('/submit?item=audit-2.8-1.fc26&type=KojiBuild') self.assertEqual(output.status_code, 200) self.assertIn( 'Invalid request, no "test" field specified', output.data.decode('utf-8') ) - data = {'target': 'audit-2.8-1.fc26', 'type': 'KojiBuild'} + data = {'item': 'audit-2.8-1.fc26', 'type': 'KojiBuild'} + output2 = self.app.post('/submit', data=data) + self.assertEqual(output.status_code, 200) + self.assertEqual( + output.data.decode('utf-8'), + output2.data.decode('utf-8') + ) + + def test_submit_no_item(self): + """ Test the submit page when no item is provided. """ + + output = self.app.get('/submit?type=KojiBuild') + self.assertEqual(output.status_code, 200) + self.assertIn( + 'Invalid request, no "item" field specified', + output.data.decode('utf-8') + ) + + data = {'type': 'KojiBuild'} output2 = self.app.post('/submit', data=data) self.assertEqual(output.status_code, 200) self.assertEqual( @@ -132,8 +150,8 @@ class AppTests(BaseTests): """ Test the submit page when all information are provided. """ output = self.app.get( - '/submit?target=audit-2.8-1.fc26' - '&type=KojiBuild&test=dist.rpmdeplint') + '/submit?item=audit-2.8-1.fc26' + '&type=KojiBuild&test=dist.rpmdeplint&extras={"groups": "foobar"}') self.assertEqual(output.status_code, 302) self.assertIn( '

You should be redirected automatically to target URL: ' @@ -142,9 +160,10 @@ class AppTests(BaseTests): ) data = { - 'target': 'audit-2.8-1.fc26', + 'item': 'audit-2.8-1.fc26', 'type': 'KojiBuild', 'test': 'dist.rpmdeplint', + 'extras': '{"groups": "foobar"}', } output = self.app.post('/submit', data=data) self.assertEqual(output.status_code, 302) From e7918faa3eb6cf4d2b48df7b0b7c4654d8a501a7 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 02 2017 15:53:31 +0000 Subject: [PATCH 3/13] Make all identifiers lower case to be consistent with taskotron Signed-off-by: Pierre-Yves Chibon --- diff --git a/README.rst b/README.rst index 7e8f2c4..acf0dd9 100644 --- a/README.rst +++ b/README.rst @@ -21,15 +21,15 @@ request to the ``/submit`` endpoint with the following arguments * ``type``: the type of element to test. They are the same types as defined in `taskotron `_ - and ``PAGURE_PR``. + and ``pagure_pr``. * ``item`` (optional if result_id is specified): the identifier of the element tested, so it could look like this: - * KOJI_BUILD : the NVR, for example: ``audit-2.8-1.fc26`` - * BODHI_UPDATE : the bodhi update ID, for example: ``FEDORA-2017-5e475c0b0d`` - * PAGURE_PR: the path to the pull-request, for example: ``rpms/python-arrow/pull-request/13`` + * koji_build : the NVR, for example: ``audit-2.8-1.fc26`` + * bodhi_update : the bodhi update ID, for example: ``FEDORA-2017-5e475c0b0d`` + * pagure_pr: the path to the pull-request, for example: ``rpms/python-arrow/pull-request/13`` * ``test`` (optional if result_id is specified): the test to re-run, for example: ``simple-koji-ci``, ``AtomicCI``, From c28ed4987befee78de4f1ee519e407a53fb54031 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 01 2017 10:25:41 +0000 Subject: [PATCH 4/13] Fix the comment that taskotron results should have a groups specified Signed-off-by: Pierre-Yves Chibon --- diff --git a/rats/lib/backends/taskotron.py b/rats/lib/backends/taskotron.py index 814d07a..249a87f 100644 --- a/rats/lib/backends/taskotron.py +++ b/rats/lib/backends/taskotron.py @@ -80,7 +80,9 @@ class TaskotronBackend(Backend): if result_id is not None: results = get_resultdb_result(result_id) if not 'groups' in results and not results['groups']: # pragma: no cover - # Should never happen in theory + # Should never happen in theory with results coming from + # taskotron (it does happen with results not generated by + # taskotron thus the quick first check) raise ValueError( 'Invalid results found for %s, no groups ' 'defined' % result_id) From b45aadda7ccb364564f209caddd4a34831c72f90 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 01 2017 10:26:02 +0000 Subject: [PATCH 5/13] Add caching using dogpile for results coming from resultsdb Signed-off-by: Pierre-Yves Chibon --- diff --git a/rats/default_config.py b/rats/default_config.py index 5698ad0..10edbb3 100644 --- a/rats/default_config.py +++ b/rats/default_config.py @@ -33,6 +33,16 @@ SECRET_KEY = '' TEST_SYSTEMS = ["Taskotron", "AtomicCI", "simple-koji-ci"] +# Cache config +# By default, don't cache anything +CACHE = {'backend': 'dogpile.cache.null'} +# Example config: +# CACHE = { +# "backend": "dogpile.cache.dbm", +# "expiration_time": 300, +# "arguments": {"filename": "/var/tmp/rats-cache.dbm"}, +# } + # Worker configuration CELERY_CONFIG = {} diff --git a/rats/lib/backends/base.py b/rats/lib/backends/base.py index 90babb0..c2bd90d 100644 --- a/rats/lib/backends/base.py +++ b/rats/lib/backends/base.py @@ -12,10 +12,14 @@ import abc import logging import requests +from dogpile.cache import make_region +from dogpile.cache.util import sha1_mangle_key import rats _log = logging.getLogger(__name__) +_cache = make_region(key_mangler=sha1_mangle_key) +_cache.configure(**rats.config['CACHE']) class Backend(object): # pragma: no cover @@ -59,6 +63,7 @@ class Backend(object): # pragma: no cover return +@_cache.cache_on_arguments() def get_resultdb_result(result_id): """ Retrieve the JSON blob about a specified result in resultsdb. """ From 21424da4b22f06959715c9894771b4fd5eea7360 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 20 2017 12:10:19 +0000 Subject: [PATCH 6/13] Make RATS use the state field of flask-oidc to store information This allows to send information to the oidc provider that will return it once the authentication succeeded. This way, we do not have to store it on the session scope where it would be overridden if the user makes two requests concurrently and we do not have to store it in a database either. Signed-off-by: Pierre-Yves Chibon --- diff --git a/rats/default_config.py b/rats/default_config.py index 10edbb3..d58e6ce 100644 --- a/rats/default_config.py +++ b/rats/default_config.py @@ -33,6 +33,8 @@ SECRET_KEY = '' TEST_SYSTEMS = ["Taskotron", "AtomicCI", "simple-koji-ci"] +OVERWRITE_REDIRECT_URI = 'http://localhost:5050/process' + # Cache config # By default, don't cache anything CACHE = {'backend': 'dogpile.cache.null'} diff --git a/rats/ui/ui_app.py b/rats/ui/ui_app.py index ceb525a..57f0a84 100644 --- a/rats/ui/ui_app.py +++ b/rats/ui/ui_app.py @@ -81,22 +81,26 @@ def submit_request(): message='Invalid request, no "%s" field specified' % key, ) - next_url = flask.url_for('ui.process_request') - return rats.lib.oidc.redirect_to_auth_server(next_url) + return rats.lib.oidc.redirect_to_auth_server( + None, customstate=flask.request.values.to_dict() + ) @ui_ns.route('/process') -def process_request(): +@rats.lib.oidc.custom_callback +def process_request(data): """ Method to submit one or more tests to be re-run. """ if flask.g.oidc_id_token is None: flask.abort(403, 'Authentication required') + set_session() + # TODO: retrieve these information from oidc - info = { - 'item': 'rpms/python-arrow/pull-request/13', - 'type': 'PagurePR', - 'test': 'simple-koji-ci', - } + #info = { + #'item': 'rpms/python-arrow/pull-request/13', + #'type': 'PagurePR', + #'test': 'simple-koji-ci', + #} #info = { #'item': 'audit-2.8-1.fc26', @@ -110,7 +114,7 @@ def process_request(): #'test': 'update.base_selinux', #} - task = rats.lib.tasks.process_request.delay(info, flask.g.user.username) + task = rats.lib.tasks.process_request.delay(data, flask.g.user.username) return flask.redirect(flask.url_for('ui.wait_task', taskid=task.id)) From 3f2e2475647c7f724ca72f6037c2712e5e4a5eee Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 20 2017 12:12:12 +0000 Subject: [PATCH 7/13] Let's not propagate the rats logging too much Signed-off-by: Pierre-Yves Chibon --- diff --git a/rats/default_config.py b/rats/default_config.py index d58e6ce..a10a12d 100644 --- a/rats/default_config.py +++ b/rats/default_config.py @@ -75,7 +75,7 @@ LOGGING = { 'rats': { 'handlers': ['console'], 'level': 'DEBUG', - 'propagate': True + 'propagate': False, }, 'flask': { 'handlers': ['console'], From a3b45e69acda8c80be9ef14ba72443c4098b0c3d Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 20 2017 12:12:32 +0000 Subject: [PATCH 8/13] Improve the logging when processing requests Use the regular logging mechanism instead of prints and store the boolean on whether the backend are interested in a specific request to avoid running the code twice. Signed-off-by: Pierre-Yves Chibon --- diff --git a/rats/lib/tasks.py b/rats/lib/tasks.py index c7f0814..b50c4d1 100644 --- a/rats/lib/tasks.py +++ b/rats/lib/tasks.py @@ -47,16 +47,17 @@ def get_result(uuid): @conn.task def process_request(info, user): """ Process the request to re-run a test. """ + _log.info('Processing request for: %s', info) backends = rats.lib.get_test_systems() - print backends + _log.info('found backends: %s', backends) tested = False names = [] for backend in backends: - #print info - print backend.name, backend.interested_by(info) - if backend.interested_by(info): + interested = backend.interested_by(info) + _log.info('%s: %s', backend.name, interested) + if interested: tested = True backend.trigger(info, user=user) names.append(backend.name) From fde9a828d5860e1c463a21c3345fd27f44c6e5ce Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jan 04 2018 15:21:15 +0000 Subject: [PATCH 9/13] Start working on preventing RATS to be abused to DDoS the test systems Signed-off-by: Pierre-Yves Chibon --- diff --git a/rats/default_config.py b/rats/default_config.py index a10a12d..686982e 100644 --- a/rats/default_config.py +++ b/rats/default_config.py @@ -30,6 +30,8 @@ DB_URL = 'sqlite:////var/tmp/rats_dev.sqlite' # secret key used to generate unique csrf token SECRET_KEY = '' +# The minimal time between allowing to re-submit a same re-run (in seconds) +GRACE_PERIOD = 60 * 5 TEST_SYSTEMS = ["Taskotron", "AtomicCI", "simple-koji-ci"] diff --git a/rats/lib/__init__.py b/rats/lib/__init__.py index e0a2419..61b0343 100644 --- a/rats/lib/__init__.py +++ b/rats/lib/__init__.py @@ -58,3 +58,38 @@ def fedmsg_publish(*args, **kwargs): fedmsg.publish(*args, **kwargs) except Exception: _log.exception('Error sending fedmsg') + + +def log_request( + session, username, type_, item, test, result_id=None, extras=None): + """ Log a requested re-run. """ + log = rats.lib.model.Log( + username=username, + type_=type_, + item=item, + test=test, + result_id=result_id, + extras=extras, + ) + session.add(log) + session.commit() + + +def log_request(session, type_, item, test, result_id=None): + """ Check if a request to re-run should be allowed time-wise. """ + query = session.query( + rats.lib.model.Log + ).filter( + rats.lib.model.Log.type_ == type_, + rats.lib.model.Logitem == item, + rats.lib.model.Log.test == test, + rats.lib.model.Log.result_id == result_id, + ).order_by( + rats.lib.model.Log.date_created.desc() + ) + + log = query.first() + + diff = datetime.datetime.utcnow() - log.date_created + + return diff.seconds < rats.config['GRACE_PERIOD'] diff --git a/rats/lib/model.py b/rats/lib/model.py index e1c5149..152407d 100644 --- a/rats/lib/model.py +++ b/rats/lib/model.py @@ -90,3 +90,23 @@ def db_session(db_url, create=True, alembic_ini=None, debug=False): scopedsession = scoped_session(sessionmaker(bind=engine)) BASE.metadata.bind = scopedsession return scopedsession + + +class Log(BASE): + """ Stores logs of the actions. + + Table -- logs + """ + + __tablename__ = 'logs' + + id = sa.Column(sa.Integer, primary_key=True) + username = sa.Column(sa.String(255), nullable=False, index=True) + type_ = sa.Column(sa.String(255), nullable=False, index=True) + item = sa.Column(sa.String(255), nullable=True, index=True) + test = sa.Column(sa.String(255), nullable=True, index=True) + result_id = sa.Column(sa.String(255), nullable=True, index=True) + extras = sa.Column(sa.Text(), nullable=True) + + date_created = sa.Column(sa.DateTime, nullable=False, + default=datetime.datetime.utcnow) From a21a1d77eb886667207c5eb240140ba4a1383c4f Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jan 04 2018 15:25:55 +0000 Subject: [PATCH 10/13] Remove some un-needed comments and debugging statements Signed-off-by: Pierre-Yves Chibon --- diff --git a/rats/lib/model.py b/rats/lib/model.py index 152407d..aa84dd0 100644 --- a/rats/lib/model.py +++ b/rats/lib/model.py @@ -36,12 +36,6 @@ BASE = declarative_base(metadata=MetaData(naming_convention=CONVENTION)) _log = logging.getLogger(__name__) -# hit w/ all the id field we use -# pylint: disable=invalid-name -# pylint: disable=too-few-public-methods -# pylint: disable=no-init -# pylint: disable=too-many-lines - def db_session(db_url, create=True, alembic_ini=None, debug=False): """ Create the tables in the database using the information from the diff --git a/rats/ui/ui_app.py b/rats/ui/ui_app.py index 57f0a84..ac77fc0 100644 --- a/rats/ui/ui_app.py +++ b/rats/ui/ui_app.py @@ -29,8 +29,6 @@ def set_session(): flask.session.permanent = True flask.g.db = rats.lib.model.db_session(rats.config['DB_URL']) - print('User logged in: %s' % rats.lib.oidc.user_loggedin) - if rats.lib.oidc.user_loggedin: if not hasattr(flask.session, 'user') or not flask.session.user: flask.session.user = munch.Munch({ @@ -64,7 +62,7 @@ def index(): @ui_ns.route('/submit', methods=('GET', 'POST')) def submit_request(): """ Method to submit one or more tests to be re-run. """ - print(flask.request.values) + if not 'type' in flask.request.values: return flask.render_template( 'results.html', From f0ade8be7d6e9af50e6f00f53838b384bff91853 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jan 04 2018 15:39:39 +0000 Subject: [PATCH 11/13] Flake8 fixes all around Signed-off-by: Pierre-Yves Chibon --- diff --git a/rats/lib/__init__.py b/rats/lib/__init__.py index 61b0343..26787ae 100644 --- a/rats/lib/__init__.py +++ b/rats/lib/__init__.py @@ -1,17 +1,16 @@ # -*- coding: utf-8 -*- """ - (c) 2017 - Copyright Red Hat Inc + (c) 2017-2018 - Copyright Red Hat Inc Authors: Pierre-Yves Chibon """ -import abc +import datetime import logging -import requests import fedmsg from flask_oidc import OpenIDConnect @@ -75,7 +74,7 @@ def log_request( session.commit() -def log_request(session, type_, item, test, result_id=None): +def check_request_grace(session, type_, item, test, result_id=None): """ Check if a request to re-run should be allowed time-wise. """ query = session.query( rats.lib.model.Log diff --git a/rats/lib/backends/__init__.py b/rats/lib/backends/__init__.py index d5a6ca3..9a6aa04 100644 --- a/rats/lib/backends/__init__.py +++ b/rats/lib/backends/__init__.py @@ -1,15 +1,14 @@ # -*- coding: utf-8 -*- """ - (c) 2017 - Copyright Red Hat Inc + (c) 2017-2018 - Copyright Red Hat Inc Authors: Pierre-Yves Chibon """ -from rats.lib.backends.base import Backend -from rats.lib.backends.atomic_ci import AtomicCiBackend -from rats.lib.backends.taskotron import TaskotronBackend -from rats.lib.backends.simple_koji_ci import SimpleKojiCiBackend - +from rats.lib.backends.base import Backend # noqa +from rats.lib.backends.atomic_ci import AtomicCiBackend # noqa +from rats.lib.backends.taskotron import TaskotronBackend # noqa +from rats.lib.backends.simple_koji_ci import SimpleKojiCiBackend # noqa diff --git a/rats/lib/backends/atomic_ci.py b/rats/lib/backends/atomic_ci.py index 929b952..21cd4e6 100644 --- a/rats/lib/backends/atomic_ci.py +++ b/rats/lib/backends/atomic_ci.py @@ -1,15 +1,13 @@ # -*- coding: utf-8 -*- """ - (c) 2017 - Copyright Red Hat Inc + (c) 2017-2018 - Copyright Red Hat Inc Authors: Pierre-Yves Chibon """ - -import abc import logging import requests @@ -40,13 +38,13 @@ class AtomicCiBackend(Backend): """ result_id = info.get('result_id') - extras = info.get('extras', {}) answer = False if result_id is not None: results = get_resultdb_result(result_id) # This isn't part of resultsdb's results atm - if results.get('source', '').lower() == self.name.lower(): # pragma: no cover + if results.get('source', '').lower() == self.name.lower( + ): # pragma: no cover answer = True elif results.get('testcase', {}).get( 'name', '').startswith('org.centos.prod.ci.pipeline'): @@ -65,7 +63,6 @@ class AtomicCiBackend(Backend): """ result_id = info.get('result_id') - extras = info.get('extras', {}) identifier = info.get('item') _log.info( @@ -77,7 +74,6 @@ class AtomicCiBackend(Backend): old_result = get_resultdb_result(result_id) elif identifier and '/pull-request/' in identifier: - test = info['type'] # Get the PR info distgit_api = '%s/api/0/' % ( rats.config['DIST_GIT_URL'].rstrip('/')) diff --git a/rats/lib/backends/base.py b/rats/lib/backends/base.py index c2bd90d..74a8a08 100644 --- a/rats/lib/backends/base.py +++ b/rats/lib/backends/base.py @@ -118,4 +118,3 @@ def get_bodhi_maintainers(identifier): maintainers = set([data['update']['user']['name']]) _log.info('Maintainers found: %s' % maintainers) return maintainers - diff --git a/rats/lib/backends/simple_koji_ci.py b/rats/lib/backends/simple_koji_ci.py index b36979e..6e1bcac 100644 --- a/rats/lib/backends/simple_koji_ci.py +++ b/rats/lib/backends/simple_koji_ci.py @@ -1,15 +1,13 @@ # -*- coding: utf-8 -*- """ - (c) 2017 - Copyright Red Hat Inc + (c) 2017-2018 - Copyright Red Hat Inc Authors: Pierre-Yves Chibon """ - -import abc import logging import requests @@ -19,7 +17,6 @@ from rats.lib.backends.base import ( Backend, get_pagure_maintainers, get_pagure_project, - get_resultdb_result, ) _log = logging.getLogger(__name__) diff --git a/rats/lib/backends/taskotron.py b/rats/lib/backends/taskotron.py index 249a87f..da3065e 100644 --- a/rats/lib/backends/taskotron.py +++ b/rats/lib/backends/taskotron.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- """ - (c) 2017 - Copyright Red Hat Inc + (c) 2017-2018 - Copyright Red Hat Inc Authors: Pierre-Yves Chibon @@ -9,11 +9,8 @@ """ -import abc import logging -import requests - import rats.lib from rats.lib.backends.base import ( Backend, @@ -46,10 +43,12 @@ class TaskotronBackend(Backend): answer = False if result_id is not None: results = get_resultdb_result(result_id) - if not 'groups' in results and not results['groups']: # pragma: no cover + if 'groups' not in results and \ + not results['groups']: # pragma: no cover # This should never happen in theory answer = False - elif results.get('source', '').lower() == self.name.lower(): # pragma: no cover + elif results.get('source', '').lower() == self.name.lower( + ): # pragma: no cover # This isn't implemented in taskotron yet answer = True elif results.get('testcase', {}).get( @@ -57,7 +56,7 @@ class TaskotronBackend(Backend): answer = True else: test = info['test'] - if not 'groups' in extras: + if 'groups' not in extras: answer = False elif test.startswith(('dist.', 'update.')): answer = True @@ -79,7 +78,8 @@ class TaskotronBackend(Backend): groups = None if result_id is not None: results = get_resultdb_result(result_id) - if not 'groups' in results and not results['groups']: # pragma: no cover + if 'groups' not in results and \ + not results['groups']: # pragma: no cover # Should never happen in theory with results coming from # taskotron (it does happen with results not generated by # taskotron thus the quick first check) @@ -90,7 +90,7 @@ class TaskotronBackend(Backend): identifier = results['data']['item'][0] type_ = results['data']['type'][0] else: - if not 'groups' in extras: + if 'groups' not in extras: raise ValueError( 'Invalid input, no groups provided in extras') groups = extras['groups'] @@ -147,5 +147,3 @@ class TaskotronBackend(Backend): groups=groups ) ) - - diff --git a/rats/lib/model.py b/rats/lib/model.py index aa84dd0..1f34f77 100644 --- a/rats/lib/model.py +++ b/rats/lib/model.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- """ - (c) 2017 - Copyright Red Hat Inc + (c) 2017-2018 - Copyright Red Hat Inc Authors: Pierre-Yves Chibon @@ -14,13 +14,9 @@ import logging import sqlalchemy as sa from sqlalchemy import create_engine, MetaData -from sqlalchemy.exc import SQLAlchemyError from sqlalchemy.ext.declarative import declarative_base -from sqlalchemy.orm import backref from sqlalchemy.orm import sessionmaker from sqlalchemy.orm import scoped_session -from sqlalchemy.orm import relation - CONVENTION = { diff --git a/rats/lib/tasks.py b/rats/lib/tasks.py index b50c4d1..34b3ad1 100644 --- a/rats/lib/tasks.py +++ b/rats/lib/tasks.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- """ - (c) 2017 - Copyright Red Hat Inc + (c) 2017-2018 - Copyright Red Hat Inc Authors: Pierre-Yves Chibon @@ -63,7 +63,8 @@ def process_request(info, user): names.append(backend.name) if not tested: - msg = 'No backend found for %s on %s' % (info.get('test'), info['type']) + msg = 'No backend found for %s on %s' % ( + info.get('test'), info['type']) else: msg = '%s has been asked to be re-run on %s' % ( info.get('item', 'test'), ', '.join(names)) diff --git a/rats/ui/ui_app.py b/rats/ui/ui_app.py index ac77fc0..6f55586 100644 --- a/rats/ui/ui_app.py +++ b/rats/ui/ui_app.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- """ - (c) 2017 - Copyright Red Hat Inc + (c) 2017-2018 - Copyright Red Hat Inc Authors: Pierre-Yves Chibon @@ -63,13 +63,13 @@ def index(): def submit_request(): """ Method to submit one or more tests to be re-run. """ - if not 'type' in flask.request.values: + if 'type' not in flask.request.values: return flask.render_template( 'results.html', message='Invalid request, no "type" field specified', ) - if not 'result_id' in flask.request.values: + if 'result_id' not in flask.request.values: for key in ['item', 'test']: if key not in flask.request.values: flask.flash( @@ -94,23 +94,23 @@ def process_request(data): set_session() # TODO: retrieve these information from oidc - #info = { - #'item': 'rpms/python-arrow/pull-request/13', - #'type': 'PagurePR', - #'test': 'simple-koji-ci', - #} - - #info = { - #'item': 'audit-2.8-1.fc26', - #'type': 'KojiBuild', - #'test': 'dist.rpmdeplint', - #} - - #info = { - #'item': 'FEDORA-2017-5e475c0b0d', - #'type': 'BodhiUpdate', - #'test': 'update.base_selinux', - #} + # info = { + # 'item': 'rpms/python-arrow/pull-request/13', + # 'type': 'PagurePR', + # 'test': 'simple-koji-ci', + # } + + # info = { + # 'item': 'audit-2.8-1.fc26', + # 'type': 'KojiBuild', + # 'test': 'dist.rpmdeplint', + # } + + # info = { + # 'item': 'FEDORA-2017-5e475c0b0d', + # 'type': 'BodhiUpdate', + # 'test': 'update.base_selinux', + # } task = rats.lib.tasks.process_request.delay(data, flask.g.user.username) From 3df7e04dab96adc78d5c66af60848450994d88eb Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jan 05 2018 11:30:00 +0000 Subject: [PATCH 12/13] Finish the grace feature in RATS This feature allows not making RATS a DDoS source for our test systems by preventing people to request too quickly the same re-run. This comes up with tests :) Signed-off-by: Pierre-Yves Chibon --- diff --git a/rats/lib/__init__.py b/rats/lib/__init__.py index 26787ae..9178274 100644 --- a/rats/lib/__init__.py +++ b/rats/lib/__init__.py @@ -9,6 +9,7 @@ """ import datetime +import json import logging import fedmsg @@ -17,6 +18,7 @@ from flask_oidc import OpenIDConnect import rats import rats.lib.backends +import rats.lib.model _log = logging.getLogger(__name__) oidc = OpenIDConnect() @@ -30,13 +32,11 @@ def get_test_systems(): backends = [] for backend in rats.lib.backends.Backend.__subclasses__(): _log.debug('Found backend: %s' % backend) - print(backend, backend.name, configured_system) if backend.name in configured_system: try: isinstance(backend(), rats.lib.backends.Backend) backends.append(backend) except Exception as err: # pragma: no cover - print err _log.debug( '%s is not an instance of rats.lib.Backend' % backend) @@ -60,8 +60,11 @@ def fedmsg_publish(*args, **kwargs): def log_request( - session, username, type_, item, test, result_id=None, extras=None): + session, username, + type_=None, item=None, test=None, result_id=None, extras=None): """ Log a requested re-run. """ + if extras is not None: + extras = json.dumps(extras) log = rats.lib.model.Log( username=username, type_=type_, @@ -74,21 +77,17 @@ def log_request( session.commit() -def check_request_grace(session, type_, item, test, result_id=None): +def get_last_request(session, type_, item, test, result_id=None): """ Check if a request to re-run should be allowed time-wise. """ query = session.query( rats.lib.model.Log ).filter( rats.lib.model.Log.type_ == type_, - rats.lib.model.Logitem == item, + rats.lib.model.Log.item == item, rats.lib.model.Log.test == test, rats.lib.model.Log.result_id == result_id, ).order_by( rats.lib.model.Log.date_created.desc() ) - log = query.first() - - diff = datetime.datetime.utcnow() - log.date_created - - return diff.seconds < rats.config['GRACE_PERIOD'] + return query.first() diff --git a/rats/lib/tasks.py b/rats/lib/tasks.py index 34b3ad1..59ab5a4 100644 --- a/rats/lib/tasks.py +++ b/rats/lib/tasks.py @@ -8,6 +8,7 @@ """ +import datetime import logging import logging.config import os @@ -52,6 +53,25 @@ def process_request(info, user): backends = rats.lib.get_test_systems() _log.info('found backends: %s', backends) + _log.info('Checking if re-run already got requested recently') + session = rats.lib.model.db_session(rats.config['DB_URL']) + last_request = rats.lib.get_last_request( + session, + type_=info.get('type'), + item=info.get('item'), + test=info.get('test'), + result_id=info.get('result_id') + ) + + recently_triggered = False + if last_request: + diff = datetime.datetime.utcnow() - last_request.date_created + recently_triggered = diff.seconds < rats.config['GRACE_PERIOD'] + + if recently_triggered: + return '%s has already been asked to be re-run at %s ' % ( + info.get('item', 'test'), last_request.date_created.isoformat()) + tested = False names = [] for backend in backends: @@ -64,9 +84,21 @@ def process_request(info, user): if not tested: msg = 'No backend found for %s on %s' % ( - info.get('test'), info['type']) + info.get('test'), info.get('type')) else: msg = '%s has been asked to be re-run on %s' % ( info.get('item', 'test'), ', '.join(names)) + rats.lib.log_request( + session, + user, + type_=info.get('type'), + item=info.get('item'), + test=info.get('test'), + result_id=info.get('result_id'), + extras=info.get('extras') + ) + _log.info('Task logged') + + session.close() return msg diff --git a/tests/__init__.py b/tests/__init__.py index 16f56e1..c952591 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- """ - (c) 2017 - Copyright Red Hat Inc + (c) 2017-2018 - Copyright Red Hat Inc Authors: Pierre-Yves Chibon @@ -49,13 +49,15 @@ class BaseTests(unittest.TestCase): """ Set the environment for the tests """ rats.config['TESTING'] = True rats.config['DEBUG'] = True - rats.config['DB_URL'] = DB_PATH + + self.path = tempfile.mkdtemp(prefix='rats-tests-') + + rats.config['DB_URL'] = 'sqlite:///%s' % ( + os.path.join(self.path, 'rats.sqlite')) _app = rats.app.create_app() self.app = _app.test_client() - self.path = tempfile.mkdtemp(prefix='rats-tests-') - def tearDown(self): """ Remove the test.db database if there is one. """ diff --git a/tests/lib/backends/test_atomic_ci.py b/tests/lib/backends/test_atomic_ci.py index f200ff0..8233954 100644 --- a/tests/lib/backends/test_atomic_ci.py +++ b/tests/lib/backends/test_atomic_ci.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- """ - (c) 2017 - Copyright Red Hat Inc + (c) 2017-2018 - Copyright Red Hat Inc Authors: Pierre-Yves Chibon @@ -74,6 +74,59 @@ class AtomicCiTaskTests(BaseTests): 'rats.config', {'DIST_GIT_URL': 'https://src.stg.fedoraproject.org'}) @patch('fedmsg.publish') + def test_process_pagurepr_request_atomic_ci_twice(self, fedmsg_publish): + """ Test process_request for a pagure PR. + """ + + info = { + 'item': 'rpms/python-arrow/pull-request/13', + 'type': 'Pagure_PR', + 'test': 'AtomicCI', + 'extras': {'foo': 'bar', 'items': [1, 2, 3]} + } + + results = rats.lib.tasks.process_request(info, 'pingou') + self.assertEqual( + results, + 'rpms/python-arrow/pull-request/13 has been asked to be re-run ' + 'on AtomicCI') + + self.assertEqual(fedmsg_publish.call_count, 1) + + args = fedmsg_publish.call_args + self.assertEqual( + args[1].keys(), + ['topic', 'msg', 'modname', 'active', 'cert_prefix']) + self.assertEqual(args[1].get('topic'), 'test.atomic-ci') + self.assertEqual(args[1].get('active'), True) + self.assertEqual(args[1].get('cert_prefix'), 'rats') + self.assertEqual(args[1].get('modname'), 'rats') + self.assertEqual( + sorted(args[1].get('msg').keys()), + ['agent', 'extras', 'identifier', 'old_result', 'pull_request', + 'result_id', 'test'] + ) + self.assertEqual( + args[1]['msg']['extras'], + {'foo': 'bar', 'items': [1, 2, 3]} + ) + + # Give it a little time before we try to trigger it again, from a + # different user + time.sleep(2) + + results = rats.lib.tasks.process_request(info, 'ralph') + self.assertTrue(results.startswith( + 'rpms/python-arrow/pull-request/13 has already been asked to ' + 'be re-run at')) + + # No more fedmsg.publish calls + self.assertEqual(fedmsg_publish.call_count, 1) + + @patch.dict( + 'rats.config', + {'DIST_GIT_URL': 'https://src.stg.fedoraproject.org'}) + @patch('fedmsg.publish') def test_process_pagurepr_request_atomic_ci_result_id( self, fedmsg_publish): """ Test process_request for a pagure PR from a result_id. @@ -107,6 +160,53 @@ class AtomicCiTaskTests(BaseTests): @patch.dict( 'rats.config', {'DIST_GIT_URL': 'https://src.stg.fedoraproject.org'}) + @patch('fedmsg.publish') + def test_process_pagurepr_request_atomic_ci_result_id_twice( + self, fedmsg_publish): + """ Test process_request for a pagure PR from a result_id. + """ + + info = { + 'type': 'Pagure_PR', + 'result_id': 17786885, + } + + results = rats.lib.tasks.process_request(info, 'pingou') + self.assertEqual( + results, + 'test has been asked to be re-run on AtomicCI') + + self.assertEqual(fedmsg_publish.call_count, 1) + + args = fedmsg_publish.call_args + self.assertEqual( + args[1].keys(), + ['topic', 'msg', 'modname', 'active', 'cert_prefix']) + self.assertEqual(args[1].get('topic'), 'test.atomic-ci') + self.assertEqual(args[1].get('active'), True) + self.assertEqual(args[1].get('cert_prefix'), 'rats') + self.assertEqual(args[1].get('modname'), 'rats') + self.assertEqual( + sorted(args[1].get('msg').keys()), + ['agent', 'extras', 'identifier', 'old_result', 'pull_request', + 'result_id', 'test'] + ) + self.assertEqual(args[1]['msg']['extras'], {}) + + # Give it a little time before we try to trigger it again, from a + # different user + time.sleep(2) + + results = rats.lib.tasks.process_request(info, 'ralph') + self.assertTrue(results.startswith( + 'test has already been asked to be re-run at ')) + + # No more fedmsg.publish calls + self.assertEqual(fedmsg_publish.call_count, 1) + + @patch.dict( + 'rats.config', + {'DIST_GIT_URL': 'https://src.stg.fedoraproject.org'}) def test_process_pagurepr_request_atomic_ci_invalid_user(self): """ Test process_request for a pagure PR. """ @@ -125,5 +225,6 @@ class AtomicCiTaskTests(BaseTests): 'foobar' ) + if __name__ == '__main__': unittest.main(verbosity=2) diff --git a/tests/lib/backends/test_simple_koji_ci.py b/tests/lib/backends/test_simple_koji_ci.py index f2173d5..3b057c3 100644 --- a/tests/lib/backends/test_simple_koji_ci.py +++ b/tests/lib/backends/test_simple_koji_ci.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- """ - (c) 2017 - Copyright Red Hat Inc + (c) 2017-2018 - Copyright Red Hat Inc Authors: Pierre-Yves Chibon @@ -71,6 +71,26 @@ class SimpleKojiCiTaskTests(BaseTests): @patch.dict( 'rats.config', {'DIST_GIT_URL': 'https://src.stg.fedoraproject.org'}) + def test_process_pr_invalid_pr(self): + """ Test process_request for a PR that does not exist. + """ + + info = { + 'item': 'rpms/python-arrow/pull-request/1335', + 'type': 'Pagure_PR', + 'test': 'simple-koji-ci', + } + + self.assertRaises( + ValueError, + rats.lib.tasks.process_request, + info, + 'trasher' + ) + + @patch.dict( + 'rats.config', + {'DIST_GIT_URL': 'https://src.stg.fedoraproject.org'}) @patch('fedmsg.publish') def test_process_pr(self, fedmsg_publish): """ Test process_request for a PR. """ @@ -103,6 +123,53 @@ class SimpleKojiCiTaskTests(BaseTests): self.assertEqual(args[1]['msg']['extras'], {}) self.assertEqual(args[1]['msg']['test'], 'simple-koji-ci') + @patch.dict( + 'rats.config', + {'DIST_GIT_URL': 'https://src.stg.fedoraproject.org'}) + @patch('fedmsg.publish') + def test_process_pr_twice(self, fedmsg_publish): + """ Test process_request for a PR twice in a short time. """ + + info = { + 'item': 'rpms/python-arrow/pull-request/13', + 'type': 'Pagure_PR', + 'test': 'simple-koji-ci', + } + + results = rats.lib.tasks.process_request(info, 'pingou') + self.assertEqual( + results, + 'rpms/python-arrow/pull-request/13 has been asked to be re-run' + ' on simple-koji-ci') + + self.assertEqual(fedmsg_publish.call_count, 1) + args = fedmsg_publish.call_args + self.assertEqual( + args[1].keys(), + ['topic', 'msg', 'modname', 'active', 'cert_prefix']) + self.assertEqual(args[1].get('topic'), 'test.simple-koji-ci') + self.assertEqual(args[1].get('active'), True) + self.assertEqual(args[1].get('cert_prefix'), 'rats') + self.assertEqual(args[1].get('modname'), 'rats') + self.assertEqual( + sorted(args[1].get('msg').keys()), + ['agent', 'extras', 'identifier', 'pull_request', 'result_id', + 'test'] + ) + self.assertEqual(args[1]['msg']['extras'], {}) + self.assertEqual(args[1]['msg']['test'], 'simple-koji-ci') + + # Give it a little time before we try to trigger it again, from a + # different user + time.sleep(2) + + results = rats.lib.tasks.process_request(info, 'ralph') + self.assertTrue(results.startswith( + 'rpms/python-arrow/pull-request/13 has already been asked to ' + 'be re-run at')) + + # No more fedmsg.publish calls + self.assertEqual(fedmsg_publish.call_count, 1) if __name__ == '__main__': diff --git a/tests/lib/backends/test_taskotron.py b/tests/lib/backends/test_taskotron.py index a5cb8d9..693de55 100644 --- a/tests/lib/backends/test_taskotron.py +++ b/tests/lib/backends/test_taskotron.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- """ - (c) 2017 - Copyright Red Hat Inc + (c) 2017-2018 - Copyright Red Hat Inc Authors: Pierre-Yves Chibon @@ -96,6 +96,51 @@ class TaskotronTaskTests(BaseTests): self.assertEqual(args[1]['msg']['test'], 'update.base_selinux') @patch('fedmsg.publish') + def test_process_bodhi_request_twice(self, fedmsg_publish): + """ Test process_request for a bodhi update. """ + + info = { + 'item': 'FEDORA-2017-5e475c0b0d', + 'type': 'Bodhi_Update', + 'test': 'update.base_selinux', + 'extras': {"groups": "foobar"}, + } + + results = rats.lib.tasks.process_request(info, 'sgrubb') + self.assertEqual( + results, + 'FEDORA-2017-5e475c0b0d has been asked to be re-run on ' + 'Taskotron') + + self.assertEqual(fedmsg_publish.call_count, 1) + + args = fedmsg_publish.call_args + self.assertEqual( + args[1].keys(), + ['topic', 'msg', 'modname', 'active', 'cert_prefix']) + self.assertEqual(args[1].get('topic'), 'test.taskotron') + self.assertEqual(args[1].get('active'), True) + self.assertEqual(args[1].get('cert_prefix'), 'rats') + self.assertEqual(args[1].get('modname'), 'rats') + self.assertEqual( + sorted(args[1].get('msg').keys()), + ['agent', 'extras', 'groups', 'identifier', 'result_id', 'test'] + ) + self.assertEqual(args[1]['msg']['extras'], {'groups': 'foobar'}) + self.assertEqual(args[1]['msg']['test'], 'update.base_selinux') + + # Give it a little time before we try to trigger it again, from a + # different user + time.sleep(2) + + results = rats.lib.tasks.process_request(info, 'ralph') + self.assertTrue(results.startswith( + 'FEDORA-2017-5e475c0b0d has already been asked to be re-run at ')) + + # No more fedmsg.publish calls + self.assertEqual(fedmsg_publish.call_count, 1) + + @patch('fedmsg.publish') def test_process_bodhi_request_result_id(self, fedmsg_publish): """ Test process_request for a bodhi update. """ @@ -109,6 +154,39 @@ class TaskotronTaskTests(BaseTests): results, 'test has been asked to be re-run on Taskotron') + self.assertEqual(fedmsg_publish.call_count, 1) + + args = fedmsg_publish.call_args + self.assertEqual( + args[1].keys(), + ['topic', 'msg', 'modname', 'active', 'cert_prefix']) + self.assertEqual(args[1].get('topic'), 'test.taskotron') + self.assertEqual(args[1].get('active'), True) + self.assertEqual(args[1].get('cert_prefix'), 'rats') + self.assertEqual(args[1].get('modname'), 'rats') + self.assertEqual( + sorted(args[1].get('msg').keys()), + ['agent', 'extras', 'groups', 'identifier', 'result_id', 'test'] + ) + self.assertEqual(args[1]['msg']['extras'], {}) + self.assertEqual(args[1]['msg']['test'], None) + + @patch('fedmsg.publish') + def test_process_bodhi_request_result_id_twice(self, fedmsg_publish): + """ Test process_request for a bodhi update. """ + + info = { + 'result_id': 17783947, + 'type': 'Bodhi_Update', + } + + results = rats.lib.tasks.process_request(info, 'kkeithle') + self.assertEqual( + results, + 'test has been asked to be re-run on Taskotron') + + self.assertEqual(fedmsg_publish.call_count, 1) + args = fedmsg_publish.call_args self.assertEqual( args[1].keys(), @@ -124,6 +202,17 @@ class TaskotronTaskTests(BaseTests): self.assertEqual(args[1]['msg']['extras'], {}) self.assertEqual(args[1]['msg']['test'], None) + # Give it a little time before we try to trigger it again, from a + # different user + time.sleep(2) + + results = rats.lib.tasks.process_request(info, 'ralph') + self.assertTrue(results.startswith( + 'test has already been asked to be re-run at ')) + + # No more fedmsg.publish calls + self.assertEqual(fedmsg_publish.call_count, 1) + @patch('fedmsg.publish', MagicMock(side_effect=Exception)) def test_process_bodhi_request_no_fedmsg(self): """ Test process_request for a bodhi update. """ @@ -146,16 +235,16 @@ class TaskotronTaskTests(BaseTests): """ Test process_request for a module build. """ info = { - 'item': 'modules/udisks2#817449ed6fade1ef5868008df2df697f717c4879', + 'item': 'modules/dhcp#82155518d9ca229fc14831cd1fe321f3a476f8e8', 'type': 'Module_Build', 'test': 'dist.modulemd', 'extras': {"groups": "foobar"}, } - results = rats.lib.tasks.process_request(info, 'hhorak') + results = rats.lib.tasks.process_request(info, 'karsten') self.assertEqual( results, - 'modules/udisks2#817449ed6fade1ef5868008df2df697f717c4879 has ' + 'modules/dhcp#82155518d9ca229fc14831cd1fe321f3a476f8e8 has ' 'been asked to be re-run on Taskotron') args = fedmsg_publish.call_args @@ -173,11 +262,57 @@ class TaskotronTaskTests(BaseTests): self.assertEqual(args[1]['msg']['extras'], {'groups': 'foobar'}) self.assertEqual(args[1]['msg']['test'], 'dist.modulemd') + @patch('fedmsg.publish') + def test_process_module_build_twice(self, fedmsg_publish): + """ Test process_request for a module build. """ + + info = { + 'item': 'modules/dhcp#82155518d9ca229fc14831cd1fe321f3a476f8e8', + 'type': 'Module_Build', + 'test': 'dist.modulemd', + 'extras': {"groups": "foobar"}, + } + + results = rats.lib.tasks.process_request(info, 'karsten') + self.assertEqual( + results, + 'modules/dhcp#82155518d9ca229fc14831cd1fe321f3a476f8e8 has ' + 'been asked to be re-run on Taskotron') + + self.assertEqual(fedmsg_publish.call_count, 1) + + args = fedmsg_publish.call_args + self.assertEqual( + args[1].keys(), + ['topic', 'msg', 'modname', 'active', 'cert_prefix']) + self.assertEqual(args[1].get('topic'), 'test.taskotron') + self.assertEqual(args[1].get('active'), True) + self.assertEqual(args[1].get('cert_prefix'), 'rats') + self.assertEqual(args[1].get('modname'), 'rats') + self.assertEqual( + sorted(args[1].get('msg').keys()), + ['agent', 'extras', 'groups', 'identifier', 'result_id', 'test'] + ) + self.assertEqual(args[1]['msg']['extras'], {'groups': 'foobar'}) + self.assertEqual(args[1]['msg']['test'], 'dist.modulemd') + + # Give it a little time before we try to trigger it again, from a + # different user + time.sleep(2) + + results = rats.lib.tasks.process_request(info, 'ralph') + self.assertTrue(results.startswith( + 'modules/dhcp#82155518d9ca229fc14831cd1fe321f3a476f8e8 has ' + 'already been asked to be re-run at ')) + + # No more fedmsg.publish calls + self.assertEqual(fedmsg_publish.call_count, 1) + def test_process_module_build_invalid_user(self): """ Test process_request for a module build with an invalid user. """ info = { - 'item': 'modules/udisks2#817449ed6fade1ef5868008df2df697f717c4879', + 'item': 'modules/dhcp#82155518d9ca229fc14831cd1fe321f3a476f8e8', 'type': 'Module_Build', 'test': 'dist.modulemd', 'extras': {"groups": "foobar"}, @@ -241,6 +376,51 @@ class TaskotronTaskTests(BaseTests): self.assertEqual(args[1]['msg']['extras'], {'groups': 'foobar'}) self.assertEqual(args[1]['msg']['test'], 'dist.rpmdeplint') + @patch('fedmsg.publish') + def test_process_koji_request_twice(self, fedmsg_publish): + """ Test process_request for a koji build. """ + + info = { + 'item': 'audit-2.8-1.fc26', + 'type': 'Koji_Build', + 'test': 'dist.rpmdeplint', + 'extras': {"groups": "foobar"}, + } + + results = rats.lib.tasks.process_request(info, 'sgrubb') + self.assertEqual( + results, + 'audit-2.8-1.fc26 has been asked to be re-run on ' + 'Taskotron') + + self.assertEqual(fedmsg_publish.call_count, 1) + + args = fedmsg_publish.call_args + self.assertEqual( + args[1].keys(), + ['topic', 'msg', 'modname', 'active', 'cert_prefix']) + self.assertEqual(args[1].get('topic'), 'test.taskotron') + self.assertEqual(args[1].get('active'), True) + self.assertEqual(args[1].get('cert_prefix'), 'rats') + self.assertEqual(args[1].get('modname'), 'rats') + self.assertEqual( + sorted(args[1].get('msg').keys()), + ['agent', 'extras', 'groups', 'identifier', 'result_id', 'test'] + ) + self.assertEqual(args[1]['msg']['extras'], {'groups': 'foobar'}) + self.assertEqual(args[1]['msg']['test'], 'dist.rpmdeplint') + + # Give it a little time before we try to trigger it again, from a + # different user + time.sleep(2) + + results = rats.lib.tasks.process_request(info, 'ralph') + self.assertTrue(results.startswith( + 'audit-2.8-1.fc26 has already been asked to be re-run at ')) + + # No more fedmsg.publish calls + self.assertEqual(fedmsg_publish.call_count, 1) + def test_taskotron_trigger(self): """ Test taskotron's trigger for a bodhi update. """ diff --git a/tests/test_app.py b/tests/test_app.py index 776e7d7..fd38cbc 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- """ - (c) 2017 - Copyright Red Hat Inc + (c) 2017-2018 - Copyright Red Hat Inc Authors: Pierre-Yves Chibon @@ -30,7 +30,9 @@ class AppTests(BaseTests): import rats conf = rats.config self.assertTrue(conf['DEBUG']) - self.assertEqual(conf['DB_URL'], 'sqlite:///:memory:') + self.assertTrue(conf['DB_URL'].startswith( + 'sqlite:////tmp/rats-tests-')) + self.assertTrue(conf['DB_URL'].endswith('/rats.sqlite')) self.assertEqual(conf['SECRET_KEY'], '') def test_config_from_env(self): diff --git a/tests/ui/test_ui_app.py b/tests/ui/test_ui_app.py index 1fd4090..89b6be7 100644 --- a/tests/ui/test_ui_app.py +++ b/tests/ui/test_ui_app.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- """ - (c) 2017 - Copyright Red Hat Inc + (c) 2017-2018 - Copyright Red Hat Inc Authors: Pierre-Yves Chibon @@ -12,9 +12,11 @@ Tests the flask application import logging +import time import unittest +from contextlib import contextmanager -from mock import patch +from mock import patch, MagicMock from tests import BaseTests @@ -22,6 +24,27 @@ _log = logging.getLogger(__name__) _log.setLevel(logging.DEBUG) +def basic_auth(sender, **kwargs): + from flask import g + g.oidc_id_token = {'sub': '123'} + + +@contextmanager +def custom_request(app, handler): + """ Set the provided user as fas_user in the provided application.""" + from flask import appcontext_pushed + + keep = [] + for meth in app.before_request_funcs[None]: + if 'OpenIDConnect._before_request' in str(meth): + continue + keep.append(meth) + app.before_request_funcs[None] = keep + + with appcontext_pushed.connected_to(handler, app): + yield + + class AppTests(BaseTests): """ Flask UI's tests. """ @@ -183,13 +206,102 @@ class AppTests(BaseTests): """ Test the process endpoint when user is logged out. """ output = self.app.get('/process') - self.assertEqual(output.status_code, 403) - self.assertIn( - '

Authentication required

', + self.assertEqual(output.status_code, 401) + self.assertEqual( + 'Not Authorized', output.data.decode('utf-8') ) + @patch('flask_oidc.OpenIDConnect._process_callback', MagicMock( + return_value=(False, {}))) + def test_process_auth_no_data(self): + """ Test the process endpoint when user is logged in but submits no + data. + """ + + with custom_request(self.app.application, basic_auth): + output = self.app.get('/process') + self.assertEqual(output.status_code, 200) + self.assertIn( + 'Invalid request, no "type" field specified', + output.data.decode('utf-8') + ) + + @patch('flask_oidc.OpenIDConnect._process_callback', MagicMock( + return_value=(False, {'item': 'audit-2.8-1.fc26'}))) + def test_process_auth_no_type(self): + """ Test the process endpoint when user is logged in but submits an + item without a type. + """ + + with custom_request(self.app.application, basic_auth): + output = self.app.get('/process') + self.assertEqual(output.status_code, 200) + self.assertIn( + 'Invalid request, no "type" field specified', + output.data.decode('utf-8') + ) + + @patch( + 'flask_oidc.OpenIDConnect._process_callback', MagicMock( + return_value=( + False, {'item': 'audit-2.8-1.fc26', 'type': 'KojiBuild'}))) + def test_process_auth_no_test(self): + """ Test the process endpoint when user is logged in but submits an + item without a test. + """ + + with custom_request(self.app.application, basic_auth): + output = self.app.get('/process') + self.assertEqual(output.status_code, 200) + self.assertIn( + 'Invalid request, no "test" field specified', + output.data.decode('utf-8') + ) + + @patch( + 'flask_oidc.OpenIDConnect._process_callback', MagicMock( + return_value=(False, {'type': 'KojiBuild'}))) + def test_process_auth_no_item(self): + """ Test the process endpoint when user is logged in but submits an + item without an item. + """ + + with custom_request(self.app.application, basic_auth): + output = self.app.get('/process') + self.assertEqual(output.status_code, 200) + self.assertIn( + 'Invalid request, no "item" field specified', + output.data.decode('utf-8') + ) + + @patch.dict('rats.config', {'BROKER_URL': None}) + @patch( + 'flask_oidc.OpenIDConnect._process_callback', MagicMock( + return_value=( + False, + { + 'item': 'audit-2.8-1.fc26', + 'type': 'KojiBuild', + 'test': 'dist.rpmdeplint', + 'extras': '{"groups": "foobar"}', + } + ))) + def test_process_auth_good(self): + """ Test the process endpoint when user is logged in and submits the + expected data. + """ + # the test_tasks messes things up a little when ran before these + # tests and it makes the tasks believe they can do something while + # in this file they can't. So reloading the tasks here to refresh + # their memory. + import rats + reload(rats.lib.tasks) + + with custom_request(self.app.application, basic_auth): + output = self.app.get('/process') + self.assertEqual(output.status_code, 302) -if __name__ == '__main__': +if __name__ == '__main__': unittest.main(verbosity=2) From bd522ba7c9363e91ade4ed04ea15b3181c9da6c0 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jan 05 2018 11:32:24 +0000 Subject: [PATCH 13/13] Make sure that we have the proper required data at the different time point Signed-off-by: Pierre-Yves Chibon --- diff --git a/rats/ui/ui_app.py b/rats/ui/ui_app.py index 6f55586..8e758e1 100644 --- a/rats/ui/ui_app.py +++ b/rats/ui/ui_app.py @@ -23,6 +23,27 @@ _log = logging.getLogger(__name__) ui_ns = flask.Blueprint('ui', __name__) +def validate_values(values): + """ Validate the values submitted by the user to ensure they meet our + needs. + """ + if 'type' not in values: + return flask.render_template( + 'results.html', + message='Invalid request, no "type" field specified', + ) + + if 'result_id' not in values: + for key in ['item', 'test']: + if key not in values: + flask.flash( + 'Invalid request', 'error') + return flask.render_template( + 'results.html', + message='Invalid request, no "%s" field specified' % key, + ) + + @ui_ns.before_request def set_session(): """ Set the flask session as permanent. """ @@ -63,25 +84,12 @@ def index(): def submit_request(): """ Method to submit one or more tests to be re-run. """ - if 'type' not in flask.request.values: - return flask.render_template( - 'results.html', - message='Invalid request, no "type" field specified', - ) - - if 'result_id' not in flask.request.values: - for key in ['item', 'test']: - if key not in flask.request.values: - flask.flash( - 'Invalid request', 'error') - return flask.render_template( - 'results.html', - message='Invalid request, no "%s" field specified' % key, - ) + values = flask.request.values.to_dict() + resp = validate_values(values) + if not resp: + resp = rats.lib.oidc.redirect_to_auth_server(None, customstate=values) - return rats.lib.oidc.redirect_to_auth_server( - None, customstate=flask.request.values.to_dict() - ) + return resp @ui_ns.route('/process') @@ -91,6 +99,10 @@ def process_request(data): if flask.g.oidc_id_token is None: flask.abort(403, 'Authentication required') + resp = validate_values(data) + if resp: + return resp + set_session() # TODO: retrieve these information from oidc