From 3dde4bbc3de80695844f90aacd758c7b5d257791 Mon Sep 17 00:00:00 2001 From: Qixiang Wan Date: May 17 2017 07:34:30 +0000 Subject: Allow limiting handlers to handle some build targets on some events Add HANDLER_BUILD_WHITELIST and HANDLER_BUILD_BLACKLIST options to support whitelist and blacklist the build target by checking its name and branch. For example: HANDLER_BUILD_WHITELIST = { "MBS": { "RPMSpecUpdated": { "module": [ { 'name': 'base-.*', }, ], }, }, } HANDLER_BUILD_BLACKLIST = { "MBS": { "RPMSpecUpdated": { "module": [ { 'name': 'base-test-module', }, { 'branch': 'rawhide', }, ], }, }, } This will allow MBS handler to build any module on 'RPMSpecUpdated' event that name matches 'base-.*' but not: 1. name is not 'base-test-module', 2. branch is not 'rawhide'. so in this example: 1. "base-mymodule' from any branch can be built 2. "base-test-module" from any branch can not be built 3. any module from 'rawhide' branch can not be built The two options are empty dicts by default. --- diff --git a/conf/config.py b/conf/config.py index 45283cf..827bf7b 100644 --- a/conf/config.py +++ b/conf/config.py @@ -77,6 +77,49 @@ class BaseConfiguration(object): SSL_ENABLED = False + # whitelist and blacklist for handlers to decide whether an artifact + # can be built on some events. + # + # In format of: + # + # { : + # { : + # { : } + # } + # } + # + # Here is an example of allowing MBS handler to build any module on + # "RPMSpecUpdated" event that module name matches 'base-.*' but not: + # 1. module name matches 'base-test-module' + # or: + # 2. module from branch 'rawhide' + # + # HANDLER_BUILD_WHITELIST = { + # "MBS": { + # "RPMSpecUpdated": { + # "module": [ + # { + # 'name': 'base-.*', + # }, + # ], + # }, + # }, + # } + # HANDLER_BUILD_BLACKLIST = { + # "MBS": { + # "RPMSpecUpdated": { + # "module": [ + # { + # 'name': 'base-test-module', + # }, + # { + # 'branch': 'rawhide', + # }, + # ], + # }, + # }, + # } + class DevConfiguration(BaseConfiguration): DEBUG = True diff --git a/freshmaker/config.py b/freshmaker/config.py index b504477..1c835d3 100644 --- a/freshmaker/config.py +++ b/freshmaker/config.py @@ -176,6 +176,16 @@ class Config(object): 'type': str, 'default': '', 'desc': 'Build owner.'}, + 'handler_build_whitelist': { + 'type': dict, + 'default': {}, + 'desc': 'Whitelist for build targets of handlers', + }, + 'handler_build_blacklist': { + 'type': dict, + 'default': {}, + 'desc': 'Blacklist for build targets of handlers', + }, } def __init__(self, conf_section_obj): @@ -222,7 +232,7 @@ class Config(object): if key in self._defaults: # type conversion for configuration item convert = self._defaults[key]['type'] - if convert in [bool, int, list, str, set]: + if convert in [bool, int, list, str, set, dict]: try: # Do no try to convert None... if value is not None: diff --git a/freshmaker/handlers/__init__.py b/freshmaker/handlers/__init__.py index 9d352ac..d6a0577 100644 --- a/freshmaker/handlers/__init__.py +++ b/freshmaker/handlers/__init__.py @@ -22,9 +22,10 @@ # Written by Jan Kaluza import abc +import re import fedmsg.utils -from freshmaker import conf, db, models +from freshmaker import conf, log, db, models def load_handlers(): @@ -73,3 +74,55 @@ class BaseHandler(object): ev = models.Event.get_or_create(db.session, event.msg_id) models.ArtifactBuild.create(db.session, ev, name, type, build_id, dep_of) db.session.commit() + + def allow_build(self, event, artifact_type, name, branch): + """ + Check whether the artifact is allowed to be built by checking + HANDLER_BUILD_WHITELIST and HANDLER_BUILD_BLACKLIST in config. + + :param event: event instance. + :param artifact_type: 'module' or 'image'. + :param name: name of the artifact. + :param branch: branch name of the artifact. + :return: True or False. + """ + # If there is a whitelist specified for the (handler, event, artifact_type), + # the build target of (name, branch) need to be in that whitelist first. + # After that (if the build target is in whitelist), check the build target + # is not in the specified blacklist. + + # by default we assume the artifact is in whitelist and not in blacklist + in_whitelist = True + in_blacklist = False + + handler_name = self.name + event_name = type(event).__name__ + whitelist_rules = conf.handler_build_whitelist.get(handler_name, {}).get(event_name, {}) + blacklist_rules = conf.handler_build_blacklist.get(handler_name, {}).get(event_name, {}) + + def match_rule(name, branch, rule): + name_rule = rule.get('name', None) + branch_rule = rule.get('branch', None) + if name_rule and not re.compile(name_rule).match(name): + return False + if branch_rule and not re.compile(branch_rule).match(branch): + return False + return True + + try: + whitelist = whitelist_rules.get(artifact_type, []) + if whitelist and not any([match_rule(name, branch, rule) for rule in whitelist]): + in_whitelist = False + + # only need to check blacklist when it is in whitelist first + if in_whitelist: + blacklist = blacklist_rules.get(artifact_type, []) + if blacklist and any([match_rule(name, branch, rule) for rule in blacklist]): + in_blacklist = True + + except re.error as exc: + log.error("Error while compiling blacklist/whilelist rule for :\n" + "Incorrect regular expression: %s\nBlacklist and Whitelist will not take effect", + handler_name, event_name, artifact_type, str(exc)) + return True + return in_whitelist and not in_blacklist diff --git a/freshmaker/handlers/image_builder.py b/freshmaker/handlers/image_builder.py index 9aa2436..cd2b62a 100644 --- a/freshmaker/handlers/image_builder.py +++ b/freshmaker/handlers/image_builder.py @@ -38,6 +38,7 @@ from freshmaker.kojiservice import koji_service class DockerImageRebuildHandler(BaseHandler): + name = 'DockerImageRebuildHandler' def can_handle(self, event): return isinstance(event, DockerfileChanged) @@ -48,11 +49,16 @@ class DockerImageRebuildHandler(BaseHandler): log.info('Start to rebuild docker image %s', event.repo) + if not self.allow_build(event, 'image', event.repo, event.branch): + log.info("Skip rebuild of %s:%s as it's not allowed by configured whitelist/blacklist", + event.repo, event.branch) + return [] + try: task_id = self.build_image(repo_url=event.repo_url, - rev=event.rev, - branch=event.branch, - namespace=event.namespace) + rev=event.rev, + branch=event.branch, + namespace=event.namespace) self.record_build(event, event.repo, 'image', task_id) @@ -61,6 +67,8 @@ class DockerImageRebuildHandler(BaseHandler): except: log.exception('Could not create task to build docker image %s', event.repo) + return [] + def build_image(self, repo_url, rev, branch, namespace=None): with koji_service(profile=conf.koji_profile, logger=log) as service: log.debug('Logging into {0} with Kerberos authentication.'.format(service.server)) @@ -83,6 +91,7 @@ class DockerImageRebuildHandler(BaseHandler): class DockerImageRebuildHandlerForBodhi(DockerImageRebuildHandler): """Rebuild docker images when RPMs are synced by Bodhi""" + name = 'DockerImageRebuildForBodhiHandler' def __init__(self): self.pdc_session = pdc.get_client_session(conf) @@ -100,6 +109,10 @@ class DockerImageRebuildHandlerForBodhi(DockerImageRebuildHandler): log.info('Found docker images to rebuild: %s', containers) for container in containers: + if not self.allow_build(event, 'image', container['name'], container['branch']): + log.info("Skip rebuild of image %s:%s as it's not allowed by configured whitelist/blacklist", + container['name'], container['branch']) + continue try: task_id = self.handle_image_build(container) self.record_build(event, container['name'], 'image', task_id) @@ -107,23 +120,17 @@ class DockerImageRebuildHandlerForBodhi(DockerImageRebuildHandler): log.exception('Error when rebuild %s', container) def handle_image_build(self, container_info): - container_detail = pdc.get_release_component(self.pdc_session, - container_info['id']) - - branch = container_detail['dist_git_branch'] - image_name = container_detail['name'] - repo_url = '{}/{}/{}'.format(conf.git_base_url, - 'container', - image_name) + name = container_info['name'] + branch = container_info['branch'] + repo_url = '{}/{}/{}'.format(conf.git_base_url, 'container', name) - log.info('Start to rebuild docker image %s from branch %s', - image_name, branch) + log.info('Start to rebuild docker image %s from branch %s', name, branch) with temp_dir(suffix='-rebuild-docker-image') as working_dir: self.clone_repository(repo_url, branch, working_dir) last_commit_hash = get_commit_hash( - os.path.join(working_dir, image_name)) + os.path.join(working_dir, name)) return self.build_image(repo_url=repo_url, branch=branch, @@ -146,6 +153,8 @@ class DockerImageRebuildHandlerForBodhi(DockerImageRebuildHandler): for container in found: id = container['id'] if id not in containers: + container_detail = pdc.get_release_component(self.pdc_session, id) + container['branch'] = container_detail['dist_git_branch'] containers[id] = container return containers.values() diff --git a/freshmaker/handlers/mbs.py b/freshmaker/handlers/mbs.py index e7f3c35..3791e05 100644 --- a/freshmaker/handlers/mbs.py +++ b/freshmaker/handlers/mbs.py @@ -94,6 +94,10 @@ class MBS(BaseHandler): def handle_metadata_update(self, event): log.info("Triggering rebuild of %s, metadata updated", event.scm_url) + if not self.allow_build(event, 'module', event.name, event.branch): + log.info("Skip rebuild of %s:%s as it's not allowed by configured whitelist/blacklist", + event.name, event.branch) + return [] build_id = self.rebuild_module(event.scm_url, event.branch) if build_id is not None: self.record_build(event, event.name, 'module', build_id) @@ -139,11 +143,17 @@ class MBS(BaseHandler): for mod in modules: name = mod['variant_name'] version = mod['variant_version'] + if not self.allow_build(event, 'module', name, version): + log.info("Skip rebuild of %s:%s as it's not allowed by configured whitelist/blacklist", + name, version) + continue commit_msg = "Bump to rebuild because of %s update" % module_name build_id = self.bump_and_rebuild_module(name, version, commit_msg=commit_msg) if build_id is not None: self.record_build(event, name, 'module', build_id) + return [] + def handle_rpm_spec_updated(self, event): """ Rebuild module when spec file of rpm in module is updated. @@ -163,6 +173,10 @@ class MBS(BaseHandler): for mod in modules: module_name = mod['variant_name'] module_branch = mod['variant_version'] + if not self.allow_build(event, 'module', module_name, module_branch): + log.info("Skip rebuild of %s:%s as it's not allowed by configured whitelist/blacklist", + module_name, module_branch) + continue log.info("Going to rebuild module '%s:%s'.", module_name, module_branch) commit_msg = "Bump to rebuild because of %s rpm spec update (%s)." % (rpm, rev) build_id = self.bump_and_rebuild_module(module_name, module_branch, commit_msg=commit_msg) diff --git a/tests/handlers/test_image_builder.py b/tests/handlers/test_image_builder.py index 4f5296c..75f18bf 100644 --- a/tests/handlers/test_image_builder.py +++ b/tests/handlers/test_image_builder.py @@ -133,12 +133,14 @@ mock_found_containers = [ { 'release': 'fedora-25-updates', 'id': 5430, - 'name': 'testimage1' + 'name': 'testimage1', + 'branch': 'f25', }, { 'release': 'fedora-25-updates', 'id': 5431, - 'name': 'testimage2' + 'name': 'testimage2', + 'branch': 'f25', }, ] @@ -271,6 +273,7 @@ class TestRebuildWhenBodhiUpdateStable(BaseTestCase): class TestContainersIncludingRPMs(unittest.TestCase): + @patch('freshmaker.pdc.get_release_component', new=mock_get_release_component) @patch('freshmaker.handlers.image_builder.pdc.find_containers_by_rpm_name') def test_get_containers(self, find_containers_by_rpm_name): expected_found_containers = [ @@ -278,11 +281,13 @@ class TestContainersIncludingRPMs(unittest.TestCase): 'release': 'fedora-24-updates', 'id': 5430, 'name': 'testimage1', + 'branch': 'f25', }, { 'release': 'fedora-24-updates', - 'id': 5432, + 'id': 5431, 'name': 'testimage2', + 'branch': 'f25', }, ] find_containers_by_rpm_name.return_value = expected_found_containers @@ -305,6 +310,7 @@ class TestContainersIncludingRPMs(unittest.TestCase): 'release': '2.fc25', 'version': '5.7.18'}, ] + containers = handler.get_containers_including_rpms(rpms) self.assertEqual(3, find_containers_by_rpm_name.call_count) diff --git a/tests/test_mbs_handler.py b/tests/test_mbs_handler.py index ce3a697..2711cd6 100644 --- a/tests/test_mbs_handler.py +++ b/tests/test_mbs_handler.py @@ -307,5 +307,71 @@ class MBSHandlerTest(helpers.FreshmakerTestCase): # build state updated to 'done' self.assertEquals(builds[0].state, models.BUILD_STATES['done']) + @mock.patch('freshmaker.handlers.mbs.utils') + @mock.patch('freshmaker.handlers.mbs.pdc') + @mock.patch('freshmaker.handlers.conf') + def test_module_is_not_allowed_to_be_built_in_whitelist(self, conf, pdc, utils): + conf.handler_build_whitelist = { + "MBS": { + "RPMSpecUpdated": { + "module": [ + { + 'name': 'test-.*', + }, + ], + }, + }, + } + conf.handler_build_blacklist = {} + m = helpers.DistGitMessage('rpms', 'bash', 'master', '123') + m.add_changed_file('bash.spec', 1, 1) + msg = m.produce() + + event = self.get_event_from_msg(msg) + + mod_info = helpers.PDCModuleInfo('testmodule', 'master', '20170412010101') + mod_info.add_rpm("bash-1.2.3-4.f26.rpm") + mod = mod_info.produce() + pdc.get_latest_modules.return_value = [mod] + event = self.get_event_from_msg(msg) + handler = MBS() + handler.rebuild_module = mock.Mock() + handler.rebuild_module.return_value = None + handler.handle(event) + handler.rebuild_module.assert_not_called() + + @mock.patch('freshmaker.handlers.mbs.utils') + @mock.patch('freshmaker.handlers.mbs.pdc') + @mock.patch('freshmaker.handlers.conf') + def test_module_is_not_allowed_to_be_built_in_blacklist(self, conf, pdc, utils): + conf.handler_build_whitelist = {} + conf.handler_build_blacklist = { + "MBS": { + "RPMSpecUpdated": { + "module": [ + { + 'name': 'testmodule', + }, + ], + }, + }, + } + m = helpers.DistGitMessage('rpms', 'bash', 'master', '123') + m.add_changed_file('bash.spec', 1, 1) + msg = m.produce() + + event = self.get_event_from_msg(msg) + + mod_info = helpers.PDCModuleInfo('testmodule', 'master', '20170412010101') + mod_info.add_rpm("bash-1.2.3-4.f26.rpm") + mod = mod_info.produce() + pdc.get_latest_modules.return_value = [mod] + event = self.get_event_from_msg(msg) + handler = MBS() + handler.rebuild_module = mock.Mock() + handler.rebuild_module.return_value = None + handler.handle(event) + handler.rebuild_module.assert_not_called() + if __name__ == '__main__': unittest.main()