From 7525299c07d5a78104dd47ff1fac255572b4eee7 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Dec 04 2017 14:04:59 +0000 Subject: [PATCH 1/3] Clean word blacklist since blacklist has been removed Signed-off-by: Chenxiong Qi --- diff --git a/freshmaker/config.py b/freshmaker/config.py index 0c93b85..dfb1b07 100644 --- a/freshmaker/config.py +++ b/freshmaker/config.py @@ -204,11 +204,6 @@ class Config(object): 'default': {}, 'desc': 'Whitelist for build targets of handlers', }, - 'handler_build_blacklist': { - 'type': dict, - 'default': {}, - 'desc': 'Blacklist for build targets of handlers', - }, 'lightblue_server_url': { 'type': str, 'default': '', diff --git a/freshmaker/handlers/__init__.py b/freshmaker/handlers/__init__.py index 53ceac6..ed945d4 100644 --- a/freshmaker/handlers/__init__.py +++ b/freshmaker/handlers/__init__.py @@ -220,7 +220,7 @@ 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. - # by default we assume the artifact is in whitelist and not in blacklist + # by default we assume the artifact is in whitelist in_whitelist = True # Global rules diff --git a/freshmaker/handlers/bodhi/update_complete_stable.py b/freshmaker/handlers/bodhi/update_complete_stable.py index ad75080..390b97b 100644 --- a/freshmaker/handlers/bodhi/update_complete_stable.py +++ b/freshmaker/handlers/bodhi/update_complete_stable.py @@ -51,7 +51,7 @@ class BodhiUpdateCompleteStableHandler(BaseHandler): for container in containers: 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", + log.info("Skip rebuild of image %s:%s as it's not allowed by configured whitelist", container['name'], container['branch']) continue try: diff --git a/freshmaker/handlers/git/dockerfile_change.py b/freshmaker/handlers/git/dockerfile_change.py index 7b572f4..183988d 100644 --- a/freshmaker/handlers/git/dockerfile_change.py +++ b/freshmaker/handlers/git/dockerfile_change.py @@ -40,7 +40,7 @@ class GitDockerfileChangeHandler(ContainerBuildHandler): log.info('Start to rebuild docker image %s.', event.container) 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", + log.info("Skip rebuild of %s:%s as it's not allowed by configured whitelist", event.container, event.branch) return [] diff --git a/freshmaker/handlers/git/module_metadata_change.py b/freshmaker/handlers/git/module_metadata_change.py index 4278e75..5575df0 100644 --- a/freshmaker/handlers/git/module_metadata_change.py +++ b/freshmaker/handlers/git/module_metadata_change.py @@ -41,7 +41,7 @@ class GitModuleMetadataChangeHandler(BaseHandler): log.info("Triggering rebuild of module %s:%s, metadata updated (%s).", event.module, event.branch, event.rev) 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", + log.info("Skip rebuild of %s:%s as it's not allowed by configured whitelist", event.module, event.branch) return [] diff --git a/freshmaker/handlers/git/rpm_spec_change.py b/freshmaker/handlers/git/rpm_spec_change.py index 179a8ba..c1ce5a5 100644 --- a/freshmaker/handlers/git/rpm_spec_change.py +++ b/freshmaker/handlers/git/rpm_spec_change.py @@ -55,7 +55,7 @@ class GitRPMSpecChangeHandler(BaseHandler): name = module['variant_name'] version = module['variant_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", + log.info("Skip rebuild of %s:%s as it's not allowed by configured whitelist", name, version) continue log.info("Going to rebuild module '%s:%s'.", name, version) diff --git a/freshmaker/handlers/mbs/module_state_change.py b/freshmaker/handlers/mbs/module_state_change.py index 5ec896b..b04f4b6 100644 --- a/freshmaker/handlers/mbs/module_state_change.py +++ b/freshmaker/handlers/mbs/module_state_change.py @@ -92,7 +92,7 @@ class MBSModuleStateChangeHandler(BaseHandler): name = mod['variant_name'] version = mod['variant_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", + log.info("Skip rebuild of %s:%s as it's not allowed by configured whitelist", name, version) continue # bump module repo first diff --git a/tests/test_mbs_module_state_change_handler.py b/tests/test_mbs_module_state_change_handler.py index 107dabb..2704807 100644 --- a/tests/test_mbs_module_state_change_handler.py +++ b/tests/test_mbs_module_state_change_handler.py @@ -121,7 +121,6 @@ class MBSModuleStateChangeHandlerTest(helpers.FreshmakerTestCase): ], }, } - conf.handler_build_blacklist = {} msg = helpers.ModuleStateChangeMessage('testmodule', 'master', state='ready').produce() event = self.get_event_from_msg(msg) From d7847796263137d2215a46fcbe19f17549c4379a Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Dec 04 2017 14:06:43 +0000 Subject: [PATCH 2/3] Do not allow build if configured whitelist and passed rule name does not match Signed-off-by: Chenxiong Qi --- diff --git a/freshmaker/handlers/__init__.py b/freshmaker/handlers/__init__.py index ed945d4..faa4ac4 100644 --- a/freshmaker/handlers/__init__.py +++ b/freshmaker/handlers/__init__.py @@ -24,6 +24,7 @@ import abc import json import re +import itertools from functools import wraps from freshmaker import conf, log, db, models @@ -208,21 +209,20 @@ class BaseHandler(object): db.session.commit() return build - def allow_build(self, artifact_type, **kwargs): + def allow_build(self, artifact_type, **criteria): """ Check whether the artifact is allowed to be built by checking HANDLER_BUILD_WHITELIST in config. :param artifact_type: an enum member of ArtifactType. - :param kwargs: dictionary of arguments to check against - :return: True or False. + :param criteria: keyword arguments listing criteria that will be + checked against whitelist to determine whether build is allowed. + There is not specific order or logical relationship to these + criteria. How they are checked depends on how whitelist is + configured. + :return: True if build is allowed, otherwise False is returned. + :rtype: bool """ - # If there is a whitelist specified for the (handler, artifact_type), - # the build target of (name, branch) need to be in that whitelist first. - - # by default we assume the artifact is in whitelist - in_whitelist = True - # Global rules whitelist_rules = conf.handler_build_whitelist.get("global", {}) @@ -230,26 +230,29 @@ class BaseHandler(object): handler_name = self.name whitelist_rules.update(conf.handler_build_whitelist.get(handler_name, {})) - def match_rule(kwargs, rule): - for key, value in kwargs.items(): - value_rule = rule.get(key, None) - if not value_rule: + def match_rule(criteria, rule): + for name, value in criteria.items(): + value_patterns = rule.get(name, None) + if not value_patterns: continue - if not isinstance(value_rule, list): - value_rule = [value_rule] + if not isinstance(value_patterns, (tuple, list)): + value_patterns = [value_patterns] - if not any((re.compile(r).match(value) for r in value_rule)): + if not any((re.match(regex, value) for regex in value_patterns)): return False return True try: whitelist = whitelist_rules.get(artifact_type.name.lower(), []) - if whitelist and not any([match_rule(kwargs, rule) for rule in whitelist]): + # If none of passed criteria matches configured rule, build is not allowed + if not (set(itertools.chain(*[rule.keys() for rule in whitelist])) & + set(criteria.keys())): + return False + if whitelist and any([match_rule(criteria, rule) for rule in whitelist]): log.debug('%r, type=%r is not whitelisted.', - kwargs, artifact_type.name.lower()) - in_whitelist = False - + criteria, artifact_type.name.lower()) + return True except re.error as exc: err_msg = ("Error while compiling whilelist rule " "for :\n" @@ -258,7 +261,8 @@ class BaseHandler(object): (handler_name, artifact_type.name.lower(), str(exc))) log.error(err_msg) raise UnprocessableEntity(err_msg) - return in_whitelist + + return False class ContainerBuildHandler(BaseHandler): diff --git a/tests/test_bodhi_update_complete_stable_handler.py b/tests/test_bodhi_update_complete_stable_handler.py index 63c617a..cb31cc8 100644 --- a/tests/test_bodhi_update_complete_stable_handler.py +++ b/tests/test_bodhi_update_complete_stable_handler.py @@ -28,6 +28,8 @@ sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) # noqa from tests import helpers from tests import get_fedmsg +import freshmaker + from freshmaker import events, db, models from freshmaker.types import ArtifactType from freshmaker.handlers.bodhi import BodhiUpdateCompleteStableHandler @@ -105,6 +107,11 @@ class BodhiUpdateCompleteStableHandlerTest(helpers.FreshmakerTestCase): @mock.patch('freshmaker.handlers.bodhi.update_complete_stable.PDC') @mock.patch('freshmaker.handlers.bodhi.update_complete_stable.utils') @mock.patch('freshmaker.handlers.bodhi.update_complete_stable.conf') + @mock.patch.object(freshmaker.conf, 'handler_build_whitelist', new={ + 'BodhiUpdateCompleteStableHandler': { + 'image': [{'name': r'testimage\d', 'branch': 'f25'}] + } + }) def test_trigger_rebuild_container_when_receives_bodhi_update_complete_stable_message(self, conf, utils, PDC): conf.git_base_url = 'git://pkgs.fedoraproject.org' diff --git a/tests/test_errata_advisory_rpms_signed_handler.py b/tests/test_errata_advisory_rpms_signed_handler.py index 426298f..b361300 100644 --- a/tests/test_errata_advisory_rpms_signed_handler.py +++ b/tests/test_errata_advisory_rpms_signed_handler.py @@ -23,10 +23,11 @@ import unittest from mock import patch -from freshmaker.handlers.errata import ErrataAdvisoryRPMsSignedHandler -from freshmaker.events import ErrataAdvisoryRPMsSignedEvent +import freshmaker from freshmaker import db +from freshmaker.events import ErrataAdvisoryRPMsSignedEvent +from freshmaker.handlers.errata import ErrataAdvisoryRPMsSignedHandler from freshmaker.models import Event from freshmaker.types import EventState @@ -152,6 +153,11 @@ class TestErrataAdvisoryRPMsSignedHandler(unittest.TestCase): db.drop_all() db.session.commit() + @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ + 'ErrataAdvisoryRPMsSignedHandler': { + 'image': [{'advisory_name': 'RHBA-2017'}] + } + }) def test_event_state_updated_when_no_images_to_rebuild(self): self.mock_find_images_to_rebuild.return_value = iter([[[]]]) event = ErrataAdvisoryRPMsSignedEvent( diff --git a/tests/test_git_dockerfile_change_handler.py b/tests/test_git_dockerfile_change_handler.py index 9c9a9ea..6b14d7a 100644 --- a/tests/test_git_dockerfile_change_handler.py +++ b/tests/test_git_dockerfile_change_handler.py @@ -27,6 +27,8 @@ import fedmsg.config from mock import patch from mock import MagicMock, PropertyMock +import freshmaker + from freshmaker import db, models from freshmaker.consumer import FreshmakerConsumer from freshmaker.types import ArtifactType @@ -65,6 +67,11 @@ class GitDockerfileChangeHandlerTest(BaseTestCase): @patch('freshmaker.utils.krbContext') @patch("freshmaker.config.Config.krb_auth_principal", new_callable=PropertyMock, return_value="user@example.com") + @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ + 'GitDockerfileChangeHandler': { + 'image': [{'name': 'testimage'}, {'branch': 'master'}] + } + }) def test_rebuild_if_dockerfile_changed( self, auth_principal, krbContext, ClientSession, read_config): read_config.return_value = { @@ -105,6 +112,11 @@ class GitDockerfileChangeHandlerTest(BaseTestCase): @patch('koji.read_config') @patch('koji.ClientSession') + @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ + 'GitDockerfileChangeHandler': { + 'image': [{'name': 'testimage'}, {'branch': 'master'}] + } + }) def test_ensure_logout_in_whatever_case(self, ClientSession, read_config): ClientSession.return_value.buildContainer.side_effect = RuntimeError read_config.return_value = { diff --git a/tests/test_git_module_metadata_change_handler.py b/tests/test_git_module_metadata_change_handler.py index 5d7482b..1b5fd38 100644 --- a/tests/test_git_module_metadata_change_handler.py +++ b/tests/test_git_module_metadata_change_handler.py @@ -26,6 +26,8 @@ import mock sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) # noqa from tests import helpers +import freshmaker + from freshmaker import events, db, models from freshmaker.types import ArtifactType from freshmaker.handlers.git import GitModuleMetadataChangeHandler @@ -59,6 +61,11 @@ class GitModuleMetadataChangeHandlerTest(helpers.FreshmakerTestCase): handler = GitModuleMetadataChangeHandler() self.assertTrue(handler.can_handle(event)) + @mock.patch.object(freshmaker.conf, 'handler_build_whitelist', new={ + 'GitModuleMetadataChangeHandler': { + 'module': [{'name': 'testmodule'}, {'branch': 'master'}] + } + }) def test_can_rebuild_module_when_module_metadata_changed(self): """ Tests handler can rebuild module when module metadata is changed in dist-git diff --git a/tests/test_git_rpm_spec_change_handler.py b/tests/test_git_rpm_spec_change_handler.py index d5e5a11..236debb 100644 --- a/tests/test_git_rpm_spec_change_handler.py +++ b/tests/test_git_rpm_spec_change_handler.py @@ -26,6 +26,8 @@ import mock sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) # noqa from tests import helpers +import freshmaker + from freshmaker import events, db, models from freshmaker.types import ArtifactType from freshmaker.handlers.git import GitRPMSpecChangeHandler @@ -76,6 +78,11 @@ class GitRPMSpecChangeHandlerTest(helpers.FreshmakerTestCase): @mock.patch('freshmaker.handlers.git.rpm_spec_change.PDC') @mock.patch('freshmaker.handlers.git.rpm_spec_change.utils') @mock.patch('freshmaker.handlers.git.rpm_spec_change.conf') + @mock.patch.object(freshmaker.conf, 'handler_build_whitelist', new={ + 'GitRPMSpecChangeHandler': { + 'module': [{'name': 'testmodule'}, {'branch': 'master'}] + } + }) def test_can_rebuild_modules_has_rpm_included(self, conf, utils, PDC): """ Test handler can rebuild modules which include the rpm. diff --git a/tests/test_handler.py b/tests/test_handler.py index 52815d3..fed44d6 100644 --- a/tests/test_handler.py +++ b/tests/test_handler.py @@ -27,6 +27,8 @@ import json from mock import patch, PropertyMock from unittest import TestCase +import freshmaker + from freshmaker import db from freshmaker.events import ErrataAdvisoryRPMsSignedEvent from freshmaker.handlers import ContainerBuildHandler @@ -138,6 +140,138 @@ class TestContext(TestCase): self.assertRaises(ProgrammingError, handler.set_context, "something") +class TestAllowBuildBasedOnWhitelist(TestCase): + """Test BaseHandler.allow_build""" + + @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"]) + + @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ + 'MyHandler': { + 'image': [ + {'advisory_state': ['REL_PREP', 'SHIPPED_LIVE']} + ] + } + }) + def test_not_allow_if_none_passed_rule_is_configured(self): + handler = MyHandler() + allowed = handler.allow_build(ArtifactType.IMAGE, state='SHIPPED_LIVE') + self.assertFalse(allowed) + + @patch.object(freshmaker.conf, 'handler_build_whitelist', new={}) + def test_not_allow_if_whitelist_is_not_configured(self): + handler = MyHandler() + allowed = handler.allow_build(ArtifactType.IMAGE, state='SHIPPED_LIVE') + self.assertFalse(allowed) + + @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ + 'MyHandler': { + 'image': [ + {'advisory_state': ['REL_PREP', 'SHIPPED_LIVE']} + ] + } + }) + def test_define_rule_values_as_list(self): + handler = MyHandler() + allowed = handler.allow_build(ArtifactType.IMAGE, + advisory_state='SHIPPED_LIVE') + self.assertTrue(allowed) + + @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ + 'MyHandler': { + 'image': [ + {'advisory_name': 'RHSA-\d+:\d+'} + ] + } + }) + def test_define_rule_value_as_single_regex_string(self): + handler = MyHandler() + allowed = handler.allow_build(ArtifactType.IMAGE, + advisory_name='RHSA-2017:31861') + self.assertTrue(allowed) + + allowed = handler.allow_build(ArtifactType.IMAGE, + advisory_name='RHBA-2017:31861') + self.assertFalse(allowed) + + @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ + 'MyHandler': { + 'image': [{ + 'advisory_name': 'RHSA-\d+:\d+', + 'advisory_state': 'REL_PREP' + }] + } + }) + def test_AND_rule(self): + handler = MyHandler() + allowed = handler.allow_build(ArtifactType.IMAGE, + advisory_name='RHSA-2017:1000', + advisory_state='REL_PREP') + self.assertTrue(allowed) + + allowed = handler.allow_build(ArtifactType.IMAGE, + advisory_name='RHSA-2017:1000', + advisory_state='SHIPPED_LIVE') + self.assertFalse(allowed) + + @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ + 'MyHandler': { + 'image': [ + {'advisory_name': 'RHSA-\d+:\d+'}, + {'advisory_state': 'REL_PREP'}, + ] + } + }) + def test_OR_rule(self): + handler = MyHandler() + allowed = handler.allow_build(ArtifactType.IMAGE, + advisory_name='RHSA-2017:1000', + advisory_state='SHIPPED_LIVE') + self.assertTrue(allowed) + + allowed = handler.allow_build(ArtifactType.IMAGE, + advisory_name='RHSA-2017', + advisory_state='REL_PREP') + self.assertTrue(allowed) + + class AnyStringWith(str): def __eq__(self, other): return self in other @@ -263,46 +397,6 @@ class TestBuildFirstBatch(TestCase): 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"]) - @patch('freshmaker.handlers.ODCS') @patch('koji.ClientSession') @patch('freshmaker.utils.krbContext') diff --git a/tests/test_mbs_module_state_change_handler.py b/tests/test_mbs_module_state_change_handler.py index 2704807..a3fa914 100644 --- a/tests/test_mbs_module_state_change_handler.py +++ b/tests/test_mbs_module_state_change_handler.py @@ -26,6 +26,8 @@ import mock sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) # noqa from tests import helpers +import freshmaker + from freshmaker import events, db, models from freshmaker.types import ArtifactType from freshmaker.handlers.mbs import MBSModuleStateChangeHandler @@ -60,6 +62,11 @@ 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.mbs.module_state_change.conf') + @mock.patch.object(freshmaker.conf, 'handler_build_whitelist', new={ + 'MBSModuleStateChangeHandler': { + 'module': [{'name': r'testmodule\d*'}, {'branch': 'master'}], + } + }) def test_can_rebuild_depending_modules(self, conf, utils, PDC): """ Tests handler can rebuild all modules which depend on the module @@ -144,6 +151,11 @@ 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.mbs.module_state_change.log') + @mock.patch.object(freshmaker.conf, 'handler_build_whitelist', new={ + 'MBSModuleStateChangeHandler': { + 'module': [{'name': r'module\d+'}, {'branch': 'master'}] + } + }) def test_handler_not_fall_into_cyclic_rebuild_loop(self, log, utils, PDC): """ Tests handler will not fall into cyclic rebuild loop when there is From 0512a9821ca207dee03843e10d6e87f29accacb4 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Dec 04 2017 14:06:43 +0000 Subject: [PATCH 3/3] Fix flake8 errors Signed-off-by: Chenxiong Qi --- diff --git a/freshmaker/config.py b/freshmaker/config.py index dfb1b07..2205eb6 100644 --- a/freshmaker/config.py +++ b/freshmaker/config.py @@ -41,11 +41,8 @@ def init_config(app): # automagically detect production environment: # - existing and readable config_file presets ProdConfiguration - try: - with open(config_file): - config_section = 'ProdConfiguration' - except: - pass + if os.path.exists(config_file) and os.access(config_file, os.O_RDONLY): + config_section = 'ProdConfiguration' # try getting config_file from os.environ if 'FRESHMAKER_CONFIG_FILE' in os.environ: @@ -81,7 +78,7 @@ def init_config(app): try: config_module = imp.load_source('freshmaker_runtime_config', config_file) - except: + except IOError: raise SystemError("Configuration file {} was not found." .format(config_file)) @@ -329,7 +326,7 @@ class Config(object): # Do no try to convert None... if value is not None: value = convert(value) - except: + except (TypeError, ValueError): raise TypeError("Configuration value conversion failed for name: %s" % key) # unknown type/unsupported conversion elif convert is not None: diff --git a/freshmaker/handlers/bodhi/update_complete_stable.py b/freshmaker/handlers/bodhi/update_complete_stable.py index 390b97b..0cac3f6 100644 --- a/freshmaker/handlers/bodhi/update_complete_stable.py +++ b/freshmaker/handlers/bodhi/update_complete_stable.py @@ -69,7 +69,7 @@ class BodhiUpdateCompleteStableHandler(BaseHandler): task_id = self.build_container(scm_url, branch, build_target) if task_id is not None: self.record_build(event, container['name'], ArtifactType.IMAGE, task_id) - except: + except Exception: log.exception('Error when rebuild %s', container) return [] diff --git a/freshmaker/handlers/git/dockerfile_change.py b/freshmaker/handlers/git/dockerfile_change.py index 183988d..6ce3766 100644 --- a/freshmaker/handlers/git/dockerfile_change.py +++ b/freshmaker/handlers/git/dockerfile_change.py @@ -60,7 +60,7 @@ class GitDockerfileChangeHandler(ContainerBuildHandler): except koji.krbV.Krb5Error as e: log.exception('Failed to login Koji via Kerberos using GSSAPI. %s', e.args[1]) - except: + except Exception: log.exception('Could not create task to build docker image %s', event.container) return [] diff --git a/freshmaker/logger.py b/freshmaker/logger.py index b144519..552f068 100644 --- a/freshmaker/logger.py +++ b/freshmaker/logger.py @@ -82,7 +82,7 @@ def init_logging(conf): logging.basicConfig(level=conf.log_level, format=log_format) try: from systemd import journal - except: + except ImportError: raise ValueError("systemd.journal module is not installed") log = logging.getLogger() diff --git a/freshmaker/manage.py b/freshmaker/manage.py index 42f6898..9c6d657 100644 --- a/freshmaker/manage.py +++ b/freshmaker/manage.py @@ -125,7 +125,7 @@ def generatelocalhostcert(): msg_cert_subject.C = 'US' msg_cert_subject.ST = 'MA' msg_cert_subject.L = 'Boston' - msg_cert_subject.O = 'Development' + msg_cert_subject.O = 'Development' # noqa msg_cert_subject.CN = 'localhost' cert.set_serial_number(2) cert.gmtime_adj_notBefore(0) diff --git a/freshmaker/models.py b/freshmaker/models.py index ff0b61f..af65806 100644 --- a/freshmaker/models.py +++ b/freshmaker/models.py @@ -83,7 +83,7 @@ def commit_on_success(func): def _decorator(*args, **kwargs): try: return func(*args, **kwargs) - except: + except Exception: db.session.rollback() raise finally: diff --git a/scripts/print_handlers_md.py b/scripts/print_handlers_md.py index 285fbd2..1c2f67e 100644 --- a/scripts/print_handlers_md.py +++ b/scripts/print_handlers_md.py @@ -60,7 +60,7 @@ for name in os.listdir(handlers_path): for submod_name in dir(mod): try: submod = getattr(mod, submod_name) - except: + except AttributeError: continue key = None deps = [] diff --git a/tox.ini b/tox.ini index 9a04d76..66a7182 100644 --- a/tox.ini +++ b/tox.ini @@ -39,4 +39,4 @@ ignore_outcome = True [flake8] ignore = E501,E731 -exclude = freshmaker/migrations/*,.tox/*,build/*,__pycache__ +exclude = freshmaker/migrations/*,.tox/*,build/*,__pycache__,scripts/print_handlers_md.py