From 9e2264c8eb66b3e1695e28f46da8222a3ebceea5 Mon Sep 17 00:00:00 2001 From: Martin Curlej Date: Oct 16 2017 15:24:42 +0000 Subject: Removed blacklist from freshmaker, added error for regex failure Signed-off-by: Martin Curlej Removed a blacklist test --- diff --git a/conf/config.py b/conf/config.py index d17181d..5220288 100644 --- a/conf/config.py +++ b/conf/config.py @@ -90,7 +90,7 @@ class BaseConfiguration(object): SSL_ENABLED = False - # whitelist and blacklist for handlers to decide whether an artifact + # whitelist for handlers to decide whether an artifact # can be built. # # In format of: @@ -100,10 +100,7 @@ class BaseConfiguration(object): # } # # Here is an example of allowing MBSModuleStateChangeHandler to build - # any module that module name matches 'base-.*' but not: - # 1. module name matches 'base-test-module' - # or: - # 2. module from branch 'rawhide' + # any module that module name matches 'base-.*' or branch rawhide # # HANDLER_BUILD_WHITELIST = { # "MBSModuleStateChangeHandler": { @@ -111,15 +108,6 @@ class BaseConfiguration(object): # { # 'name': 'base-.*', # }, - # ], - # }, - # } - # HANDLER_BUILD_BLACKLIST = { - # "MBSModuleStateChangeHandler": { - # "module": [ - # { - # 'name': 'base-test-module', - # }, # { # 'branch': 'rawhide', # }, diff --git a/freshmaker/handlers/__init__.py b/freshmaker/handlers/__init__.py index 3bcab33..5e08df6 100644 --- a/freshmaker/handlers/__init__.py +++ b/freshmaker/handlers/__init__.py @@ -32,6 +32,7 @@ from freshmaker.models import ArtifactBuildState from freshmaker.types import ArtifactType from freshmaker.models import ArtifactBuild, Event from freshmaker.utils import krb_context, get_rebuilt_nvr +from freshmaker.errors import UnprocessableEntity from freshmaker.odcsclient import ODCS from freshmaker.odcsclient import AuthMech @@ -108,7 +109,7 @@ class BaseHandler(object): 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. + HANDLER_BUILD_WHITELIST in config. :param artifact_type: an enum member of ArtifactType. :param kwargs: dictionary of arguments to check against @@ -116,21 +117,16 @@ class BaseHandler(object): """ # If there is a whitelist specified for the (handler, 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 # Global rules whitelist_rules = conf.handler_build_whitelist.get("global", {}) - blacklist_rules = conf.handler_build_blacklist.get("global", {}) # This handler rules handler_name = self.name whitelist_rules.update(conf.handler_build_whitelist.get(handler_name, {})) - blacklist_rules.update(conf.handler_build_blacklist.get(handler_name, {})) def match_rule(kwargs, rule): for key, value in kwargs.items(): @@ -152,20 +148,15 @@ class BaseHandler(object): 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(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: - log.error("Error while compiling blacklist/whilelist rule for :\n" - "Incorrect regular expression: %s\nBlacklist and Whitelist will not take effect", - handler_name, artifact_type.name.lower(), str(exc)) - return True - return in_whitelist and not in_blacklist + err_msg = ("Error while compiling whilelist rule " + "for :\n" + "Incorrect regular expression: %s\n" + "Whitelist will not take effect" % + (handler_name, artifact_type.name.lower(), str(exc))) + log.error(err_msg) + raise UnprocessableEntity(err_msg) + return in_whitelist class ContainerBuildHandler(BaseHandler): diff --git a/tests/test_handler.py b/tests/test_handler.py index 96bd083..05b4cf6 100644 --- a/tests/test_handler.py +++ b/tests/test_handler.py @@ -33,11 +33,15 @@ from freshmaker.handlers import ContainerBuildHandler from freshmaker.models import ArtifactBuild from freshmaker.models import ArtifactBuildState from freshmaker.models import Event +from freshmaker.errors import UnprocessableEntity +from freshmaker.types import ArtifactType class MyHandler(ContainerBuildHandler): """Handler for running tests to test things defined in parents""" + name = "MyHandler" + def can_handle(self, event): """Implement BaseHandler method""" @@ -214,3 +218,43 @@ class TestBuildFirstBatch(TestCase): else: self.assertEqual(build.build_id, None) self.assertEqual(build.state, ArtifactBuildState.PLANNED.value) + + @patch('freshmaker.handlers.conf') + def test_allow_build_in_whitelist(self, conf): + """ Test if artifact is in the handlers whitelist """ + whitelist_rules = {"image": [{'name': "test"}]} + handler = MyHandler() + conf.handler_build_whitelist.get.return_value = whitelist_rules + container = {"name": "test", "branch": "branch"} + + allow = handler.allow_build(ArtifactType.IMAGE, + name=container["name"], + branch=container["branch"]) + assert allow + + @patch('freshmaker.handlers.conf') + def test_allow_build_not_in_whitelist(self, conf): + """ Test if artifact is not in the handlers whitelist """ + whitelist_rules = {"image": [{'name': "test1"}]} + handler = MyHandler() + conf.handler_build_whitelist.get.return_value = whitelist_rules + container = {"name": "test", "branch": "branch"} + + allow = handler.allow_build(ArtifactType.IMAGE, + name=container["name"], + branch=container["branch"]) + assert not allow + + @patch('freshmaker.handlers.conf') + def test_allow_build_regex_exception(self, conf): + """ If there is a regex error, method will raise UnprocessableEntity error """ + + whitelist_rules = {"image": [{'name': "te(st"}]} + handler = MyHandler() + conf.handler_build_whitelist.get.return_value = whitelist_rules + container = {"name": "test", "branch": "branch"} + + with self.assertRaises(UnprocessableEntity): + handler.allow_build(ArtifactType.IMAGE, + name=container["name"], + branch=container["branch"]) diff --git a/tests/test_mbs_module_state_change_handler.py b/tests/test_mbs_module_state_change_handler.py index dcf32f1..107dabb 100644 --- a/tests/test_mbs_module_state_change_handler.py +++ b/tests/test_mbs_module_state_change_handler.py @@ -144,39 +144,6 @@ class MBSModuleStateChangeHandlerTest(helpers.FreshmakerTestCase): @mock.patch('freshmaker.handlers.mbs.module_state_change.PDC') @mock.patch('freshmaker.handlers.mbs.module_state_change.utils') - @mock.patch('freshmaker.handlers.conf') - def test_module_is_not_allowed_in_blacklist(self, conf, utils, PDC): - conf.handler_build_whitelist = {} - conf.handler_build_blacklist = { - "MBSModuleStateChangeHandler": { - "module": [ - { - 'name': 'test.*', - }, - ], - }, - } - msg = helpers.ModuleStateChangeMessage('testmodule', 'master', state='ready').produce() - event = self.get_event_from_msg(msg) - - mod2_info = helpers.PDCModuleInfo('testmodule2', 'master', '20170412010101') - mod2_info.add_build_dep('testmodule', 'master') - mod2 = mod2_info.produce() - - pdc = PDC.return_value - pdc.get_latest_modules.return_value = [mod2] - - handler = MBSModuleStateChangeHandler() - handler.build_module = mock.Mock() - handler.record_build = mock.Mock() - - self.assertTrue(handler.can_handle(event)) - handler.handle(event) - - handler.build_module.assert_not_called() - - @mock.patch('freshmaker.handlers.mbs.module_state_change.PDC') - @mock.patch('freshmaker.handlers.mbs.module_state_change.utils') @mock.patch('freshmaker.handlers.mbs.module_state_change.log') def test_handler_not_fall_into_cyclic_rebuild_loop(self, log, utils, PDC): """