From db7cf3da3e579fe9f481c2bcbaf64857bae67782 Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Jul 19 2017 08:36:40 +0000 Subject: Make allow_build(...) method more dynamic and use it in sign_rpm handler. --- diff --git a/freshmaker/handlers/__init__.py b/freshmaker/handlers/__init__.py index 5caaaf7..fee9918 100644 --- a/freshmaker/handlers/__init__.py +++ b/freshmaker/handlers/__init__.py @@ -113,14 +113,13 @@ class BaseHandler(object): models.ArtifactBuild.create(db.session, ev, name, artifact_type.name.lower(), build_id, dep_on) db.session.commit() - def allow_build(self, artifact_type, name, branch): + def allow_build(self, artifact_type, **kwargs): """ Check whether the artifact is allowed to be built by checking HANDLER_BUILD_WHITELIST and HANDLER_BUILD_BLACKLIST in config. :param artifact_type: an enum member of ArtifactType. - :param name: name of the artifact. - :param branch: branch name of the artifact. + :param kwargs: dictionary of arguments to check against :return: True or False. """ # If there is a whitelist specified for the (handler, artifact_type), @@ -136,28 +135,26 @@ class BaseHandler(object): whitelist_rules = conf.handler_build_whitelist.get(handler_name, {}) blacklist_rules = conf.handler_build_blacklist.get(handler_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): + def match_rule(kwargs, rule): + for key, value in kwargs.items(): + value_rule = rule.get(key, None) + if value_rule and not re.compile(value_rule).match(value): return False return True try: whitelist = whitelist_rules.get(artifact_type.name.lower(), []) - if whitelist and not any([match_rule(name, branch, rule) for rule in whitelist]): - log.debug('name=%r, branch=%r, type=%r is not whitelisted.', - name, branch, artifact_type.name.lower()) + if whitelist and not any([match_rule(kwargs, rule) for rule in whitelist]): + log.debug('%r, type=%r is not whitelisted.', + kwargs, artifact_type.name.lower()) in_whitelist = False # only need to check blacklist when it is in whitelist first if in_whitelist: blacklist = blacklist_rules.get(artifact_type.name.lower(), []) - if blacklist and any([match_rule(name, branch, rule) for rule in blacklist]): - log.debug('name=%r, branch=%r, type=%r is blacklisted.', - name, branch, artifact_type.name.lower()) + if blacklist and any([match_rule(kwargs, rule) for rule in blacklist]): + log.debug('%r, type=%r is blacklisted.', + kwargs, artifact_type.name.lower()) in_blacklist = True except re.error as exc: diff --git a/freshmaker/handlers/bodhi/update_complete_stable.py b/freshmaker/handlers/bodhi/update_complete_stable.py index f12ef73..f34f4f0 100644 --- a/freshmaker/handlers/bodhi/update_complete_stable.py +++ b/freshmaker/handlers/bodhi/update_complete_stable.py @@ -50,7 +50,7 @@ class BodhiUpdateCompleteStableHandler(BaseHandler): log.info('Found docker images to rebuild: %s', containers) for container in containers: - if not self.allow_build(ArtifactType.IMAGE, container['name'], container['branch']): + if not self.allow_build(ArtifactType.IMAGE, name=container['name'], branch=container['branch']): log.info("Skip rebuild of image %s:%s as it's not allowed by configured whitelist/blacklist", container['name'], container['branch']) continue diff --git a/freshmaker/handlers/brew/sign_rpm.py b/freshmaker/handlers/brew/sign_rpm.py index dbd0c44..d2b1c24 100644 --- a/freshmaker/handlers/brew/sign_rpm.py +++ b/freshmaker/handlers/brew/sign_rpm.py @@ -31,6 +31,7 @@ from freshmaker.kojiservice import koji_service from freshmaker.lightblue import LightBlue from freshmaker.pulp import Pulp from freshmaker.errata import Errata +from freshmaker.types import ArtifactType class BrewSignRPMHanlder(BaseHandler): @@ -77,6 +78,15 @@ class BrewSignRPMHanlder(BaseHandler): errata = Errata(conf.errata_tool_server_url) advisories = errata.advisories_from_event(event) + # Filter out advisories which are not allow by configuration + advisories = [advisory for advisory in advisories + if self.allow_build(ArtifactType.IMAGE, + advisory_name=advisory.name)] + if not advisories: + log.info("No advisories found suitable for rebuilding Docker " + "images") + return [] + if not all((errata.builds_signed(advisory.errata_id) for advisory in advisories)): log.info('Not all builds in %s are signed. Do not rebuild any ' diff --git a/freshmaker/handlers/git/dockerfile_change.py b/freshmaker/handlers/git/dockerfile_change.py index 964ed18..45f7e88 100644 --- a/freshmaker/handlers/git/dockerfile_change.py +++ b/freshmaker/handlers/git/dockerfile_change.py @@ -39,7 +39,7 @@ class GitDockerfileChangeHandler(BaseHandler): log.info('Start to rebuild docker image %s.', event.container) - if not self.allow_build(ArtifactType.IMAGE, event.container, event.branch): + if not self.allow_build(ArtifactType.IMAGE, name=event.container, branch=event.branch): log.info("Skip rebuild of %s:%s as it's not allowed by configured whitelist/blacklist", event.container, event.branch) return [] diff --git a/freshmaker/handlers/git/module_metadata_change.py b/freshmaker/handlers/git/module_metadata_change.py index 209153e..4278e75 100644 --- a/freshmaker/handlers/git/module_metadata_change.py +++ b/freshmaker/handlers/git/module_metadata_change.py @@ -40,7 +40,7 @@ class GitModuleMetadataChangeHandler(BaseHandler): def handle(self, event): log.info("Triggering rebuild of module %s:%s, metadata updated (%s).", event.module, event.branch, event.rev) - if not self.allow_build(ArtifactType.MODULE, event.module, event.branch): + if not self.allow_build(ArtifactType.MODULE, name=event.module, branch=event.branch): log.info("Skip rebuild of %s:%s as it's not allowed by configured whitelist/blacklist", event.module, event.branch) return [] diff --git a/freshmaker/handlers/git/rpm_spec_change.py b/freshmaker/handlers/git/rpm_spec_change.py index e6aeaca..179a8ba 100644 --- a/freshmaker/handlers/git/rpm_spec_change.py +++ b/freshmaker/handlers/git/rpm_spec_change.py @@ -54,7 +54,7 @@ class GitRPMSpecChangeHandler(BaseHandler): for module in modules: name = module['variant_name'] version = module['variant_version'] - if not self.allow_build(ArtifactType.MODULE, name, version): + if not self.allow_build(ArtifactType.MODULE, name=name, branch=version): log.info("Skip rebuild of %s:%s as it's not allowed by configured whitelist/blacklist", name, version) continue diff --git a/freshmaker/handlers/mbs/module_state_change.py b/freshmaker/handlers/mbs/module_state_change.py index 6d2678e..d3a181d 100644 --- a/freshmaker/handlers/mbs/module_state_change.py +++ b/freshmaker/handlers/mbs/module_state_change.py @@ -89,7 +89,7 @@ class MBSModuleStateChangeHandler(BaseHandler): for mod in modules: name = mod['variant_name'] version = mod['variant_version'] - if not self.allow_build(ArtifactType.MODULE, name, version): + if not self.allow_build(ArtifactType.MODULE, name=name, branch=version): log.info("Skip rebuild of %s:%s as it's not allowed by configured whitelist/blacklist", name, version) continue diff --git a/tests/test_brew_sign_rpm_handler.py b/tests/test_brew_sign_rpm_handler.py index 7dfd728..dc9688a 100644 --- a/tests/test_brew_sign_rpm_handler.py +++ b/tests/test_brew_sign_rpm_handler.py @@ -25,9 +25,10 @@ import six import pytest import unittest -from mock import patch +from mock import patch, MagicMock, PropertyMock from freshmaker.handlers.brew.sign_rpm import BrewSignRPMHanlder +from freshmaker.errata import ErrataAdvisory @pytest.mark.skipif(six.PY3, reason='koji does not work in Python 3') @@ -78,3 +79,48 @@ class TestFindBuildSrpmName(unittest.TestCase): session.getBuild.assert_called_once_with('bind-dyndb-ldap-2.3-8.el6') session.listRPMs.assert_called_once_with(buildID=439408, arches='src') + + +class TestAllowBuild(unittest.TestCase): + """Test BrewSignRPMHanlder.allow_build""" + + @patch('freshmaker.errata.Errata.advisories_from_event') + @patch('freshmaker.errata.Errata.builds_signed') + @patch("freshmaker.config.Config.handler_build_whitelist", + new_callable=PropertyMock, return_value={ + "BrewSignRPMHandler": {"image": [{"advisory_name": "RHSA-.*"}]}}) + def test_allow_build_false(self, handler_build_whitelist, builds_signed, + advisories_from_event): + """ + Tests that allow_build filters out advisories based on advisory_name. + """ + advisories_from_event.return_value = [ + ErrataAdvisory(123, "RHBA-2017", "REL_PREP")] + builds_signed.return_value = False + + event = MagicMock() + handler = BrewSignRPMHanlder() + handler.handle(event) + + builds_signed.assert_not_called() + + @patch('freshmaker.errata.Errata.advisories_from_event') + @patch('freshmaker.errata.Errata.builds_signed') + @patch("freshmaker.config.Config.handler_build_whitelist", + new_callable=PropertyMock, return_value={ + "BrewSignRPMHandler": {"image": [{"advisory_name": "RHSA-.*"}]}}) + def test_allow_build_true(self, handler_build_whitelist, builds_signed, + advisories_from_event): + """ + Tests that allow_build does not filter out advisories based on + advisory_name. + """ + advisories_from_event.return_value = [ + ErrataAdvisory(123, "RHSA-2017", "REL_PREP")] + builds_signed.return_value = False + + event = MagicMock() + handler = BrewSignRPMHanlder() + handler.handle(event) + + builds_signed.assert_called_once()