From d038c80bd411e404ebb32a8a3bcd74bf771df02e Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Apr 17 2018 12:15:09 +0000 Subject: Use any_() and all_() methods in whitelist to make it clear how it is evaluated. --- diff --git a/conf/config.py b/conf/config.py index 3bf505d..7558da7 100644 --- a/conf/config.py +++ b/conf/config.py @@ -3,6 +3,7 @@ import os import tempfile +from freshmaker.config import all_, any_ # noqa # FIXME: workaround for this moment till confdir, dbdir (installdir etc.) are # declared properly somewhere/somehow @@ -100,22 +101,39 @@ class BaseConfiguration(object): # In format of: # # { : - # { : } + # { : } # } # - # Here is an example of allowing MBSModuleStateChangeHandler to build - # any module that module name matches 'base-.*' or branch rawhide + # The `handler_name` is usually set to "global" to affect all + # the handlers. + # + # The `rule(s)` part of a whitelist are dictionaries with key named as + # some artifact attribute. The value can be str, bool or list of strings. + # If it is list of strings, the rule matches if any string from the list + # matches the artifact attribute. + # + # The rule(s) can be also grouped using the any_() or all_() functions: + # + # - The any_(rule_1, rule_2, ...) matches when any of the rules + # matches. + # - The all_(rule_1, rule_2, ...) matches when all the rules matches. + # + # For more information see . + # + # Here is an example of allowing container images to be build as soon as + # an RHSA advisory with critical/important severity or with hightouch bug + # moves to SHIPPED_LIVE: # # HANDLER_BUILD_WHITELIST = { - # "MBSModuleStateChangeHandler": { - # "module": [ - # { - # 'name': 'base-.*', - # }, - # { - # 'branch': 'rawhide', - # }, - # ], + # "global": { + # "image": all_( + # {'advisory_name': 'RHSA-.*' + # 'advisory_state: 'SHIPPED_LIVE'}, + # any_( + # {'has_hightouch_bugs': True}, + # {'severity': ['critical', 'important']} + # ) + # ) # }, # } @@ -274,18 +292,14 @@ class TestConfiguration(BaseConfiguration): HANDLER_BUILD_WHITELIST = { 'BrewSignRPMHandler': { - 'image': [ - { - 'advisory_state': 'REL_PREP|PUSH_READY|IN_PUSH|SHIPPED_LIVE', - }, - ], + 'image': { + 'advisory_state': 'REL_PREP|PUSH_READY|IN_PUSH|SHIPPED_LIVE', + }, }, 'ErrataAdvisoryStateChangedHandler': { - 'image': [ - { - 'advisory_state': 'REL_PREP|PUSH_READY|IN_PUSH|SHIPPED_LIVE', - }, - ], + 'image': { + 'advisory_state': 'REL_PREP|PUSH_READY|IN_PUSH|SHIPPED_LIVE', + }, }, } diff --git a/freshmaker/config.py b/freshmaker/config.py index c470973..92e3de1 100644 --- a/freshmaker/config.py +++ b/freshmaker/config.py @@ -31,6 +31,30 @@ from os import sys from freshmaker import logger +def any_(*rules): + """ + Returns list of rules which can be used in HANDLER_MODULE_WHITELIST + or HANDLER_MODULE_BLACKLIST and is later evaluated as "matched" if *any* + rule from the `rules` matches. + + :param rules: Each rule is a dict in the same format as other + dicts in the HANDLER_MODULE_WHITELIST or HANDLER_MODULE_BLACKLIST. + """ + return ["any", [rule for rule in rules]] + + +def all_(*rules): + """ + Returns list of rules which can be used in HANDLER_MODULE_WHITELIST + or HANDLER_MODULE_BLACKLIST and is later evaluated as "matched" if *all* + rules from the `rules` matches. + + :param rules: Each rule is a dict in the same format as other + dicts in the HANDLER_MODULE_WHITELIST or HANDLER_MODULE_BLACKLIST. + """ + return ["all", [rule for rule in rules]] + + def init_config(app): """ Configure Freshmaker diff --git a/freshmaker/handlers/__init__.py b/freshmaker/handlers/__init__.py index e5db5e8..6fa6655 100644 --- a/freshmaker/handlers/__init__.py +++ b/freshmaker/handlers/__init__.py @@ -24,7 +24,6 @@ import abc import json import re -import itertools import six from functools import wraps @@ -247,6 +246,82 @@ class BaseHandler(object): db.session.commit() return build + def _match_allow_build_rule(self, criteria, rule): + """ + Returns True if the build criteria matches the rule. + + :param dict criteria: key-val criteria defining all the attributes of + an artifact which is considered for rebuild. + :param dict or list of dicts rule: Rule from the Freshmaker + configuration. It can be list or dict: + + If it is dict, all the key-vals in the rule dict must match the + key-vals in the criteria dict. If the value is list for + particular key in rule dict, the relationship between this list's + items is OR. + + If it is list, it must have following format: + + ["operator_name", [{rules}, {to}, {evaluate}, ...]] + + Such list is constructed by freshmaker.config's any_() and all_() + methods. The operator name is either "any" or "all". + + If "any" is used, this method returns True if *any* dict in list + after the operator name matches the criteria. + + If "all" is used, this method returns True if *all* dicts in list + after the operator name matches the criteria. + :rtype: bool + :return: True if the crtieria matches the rule. + """ + # If rule is list, check each item (which should be a dict) separately + # and return True if any item matches. Support also tuples for + # convenience. + if isinstance(rule, list): + if not rule: + return False + + if not isinstance(rule[0], six.string_types): + raise TypeError( + "Rule does not have any operator, use any_() or all_() " + "methods to construct the rule: %r" % rule) + + if rule[0] == "any": + operator = any + elif rule[0] == "all": + operator = all + else: + raise ValueError( + "Invalid operator %s in rule: %r." % (rule[0], rule)) + + return operator([ + self._match_allow_build_rule(criteria, subrule) + for subrule in rule[1]]) + + if not isinstance(rule, dict): + raise TypeError( + "Rebuild rule must be dict or list, got %r." % rule) + + # If none of passed criteria matches configured rule, build is not allowed + if not (set(rule.keys()) & set(criteria.keys())): + return False + + # For each key-val of artifact to rebuild, check if it matches + # the key-val of rule. If the key-val is not in the rule, it means + # the configuration does not care about the value. + for key, value in criteria.items(): + value_patterns = rule.get(key, None) + if value_patterns is None: + continue + + if not isinstance(value_patterns, (tuple, list)): + value_patterns = [str(value_patterns)] + + if not any((re.match(regex, str(value)) for regex in value_patterns)): + return False + return True + def allow_build(self, artifact_type, **criteria): """ Check whether the artifact is allowed to be built by checking @@ -268,28 +343,11 @@ class BaseHandler(object): handler_name = self.name whitelist_rules.update(conf.handler_build_whitelist.get(handler_name, {})) - def match_rule(criteria, rule): - for name, value in criteria.items(): - value_patterns = rule.get(name, None) - if value_patterns is None: - continue - - if not isinstance(value_patterns, (tuple, list)): - value_patterns = [str(value_patterns)] - - if not any((re.match(regex, str(value)) for regex in value_patterns)): - return False - return True - try: whitelist = whitelist_rules.get(artifact_type.name.lower(), []) - # 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 whitelisted.', - criteria, artifact_type.name.lower()) + if self._match_allow_build_rule(criteria, whitelist): + self.log_debug('%r, type=%r is whitelisted.', + criteria, artifact_type.name.lower()) return True except re.error as exc: err_msg = ("Error while compiling whilelist rule " @@ -297,11 +355,11 @@ class BaseHandler(object): "Incorrect regular expression: %s\n" "Whitelist will not take effect" % (handler_name, artifact_type.name.lower(), str(exc))) - log.error(err_msg) + self.log_error(err_msg) raise UnprocessableEntity(err_msg) - log.debug('%r, type=%r is not whitelisted.', - criteria, artifact_type.name.lower()) + self.log_debug('%r, type=%r is not whitelisted.', + criteria, artifact_type.name.lower()) return False diff --git a/tests/test_bodhi_update_complete_stable_handler.py b/tests/test_bodhi_update_complete_stable_handler.py index bc52307..56cd866 100644 --- a/tests/test_bodhi_update_complete_stable_handler.py +++ b/tests/test_bodhi_update_complete_stable_handler.py @@ -100,7 +100,7 @@ class BodhiUpdateCompleteStableHandlerTest(helpers.ModelsTestCase): @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'}] + 'image': {'name': r'testimage\d', 'branch': 'f25'} } }) def test_trigger_rebuild_container_when_receives_bodhi_update_complete_stable_message(self, conf, utils, PDC): diff --git a/tests/test_brew_sign_rpm_handler.py b/tests/test_brew_sign_rpm_handler.py index d513b11..595a2ac 100644 --- a/tests/test_brew_sign_rpm_handler.py +++ b/tests/test_brew_sign_rpm_handler.py @@ -35,7 +35,7 @@ class TestBrewSignHandler(helpers.ModelsTestCase): @patch('freshmaker.errata.Errata.builds_signed') @patch("freshmaker.config.Config.handler_build_whitelist", new_callable=PropertyMock, return_value={ - "BrewSignRPMHandler": {"image": [{"advisory_name": "RHSA-.*"}]}}) + "BrewSignRPMHandler": {"image": {"advisory_name": "RHSA-.*"}}}) def test_return_value(self, handler_build_whitelist, builds_signed, advisories_from_event): """ @@ -58,7 +58,7 @@ class TestBrewSignHandler(helpers.ModelsTestCase): @patch('freshmaker.errata.Errata.builds_signed') @patch("freshmaker.config.Config.handler_build_whitelist", new_callable=PropertyMock, return_value={ - "global": {"image": [{"advisory_name": "RHSA-.*"}]}}) + "global": {"image": {"advisory_name": "RHSA-.*"}}}) def test_allow_build_false_global(self, handler_build_whitelist, builds_signed, advisories_from_event): """ @@ -79,7 +79,7 @@ class TestBrewSignHandler(helpers.ModelsTestCase): @patch('freshmaker.errata.Errata.builds_signed') @patch("freshmaker.config.Config.handler_build_whitelist", new_callable=PropertyMock, return_value={ - "global": {"image": [{"advisory_name": "RHSA-.*"}]}}) + "global": {"image": {"advisory_name": "RHSA-.*"}}}) def test_allow_build_true_global(self, handler_build_whitelist, builds_signed, advisories_from_event): """ @@ -100,7 +100,7 @@ class TestBrewSignHandler(helpers.ModelsTestCase): @patch('freshmaker.errata.Errata.builds_signed') @patch("freshmaker.config.Config.handler_build_whitelist", new_callable=PropertyMock, return_value={ - "BrewSignRPMHandler": {"image": [{"advisory_name": "RHSA-.*"}]}}) + "BrewSignRPMHandler": {"image": {"advisory_name": "RHSA-.*"}}}) def test_allow_build_false(self, handler_build_whitelist, builds_signed, advisories_from_event): """ @@ -121,7 +121,7 @@ class TestBrewSignHandler(helpers.ModelsTestCase): @patch('freshmaker.errata.Errata.builds_signed') @patch("freshmaker.config.Config.handler_build_whitelist", new_callable=PropertyMock, return_value={ - "BrewSignRPMHandler": {"image": [{"advisory_name": "RHSA-.*"}]}}) + "BrewSignRPMHandler": {"image": {"advisory_name": "RHSA-.*"}}}) def test_allow_build_true(self, handler_build_whitelist, builds_signed, advisories_from_event): """ @@ -145,11 +145,11 @@ class TestBrewSignHandler(helpers.ModelsTestCase): new_callable=PropertyMock, return_value={ "BrewSignRPMHandler": { - "image": [{ + "image": { "advisory_security_impact": [ "Normal", "Important" ] - }] + } } }) def test_allow_security_impact_important_true( @@ -176,11 +176,11 @@ class TestBrewSignHandler(helpers.ModelsTestCase): new_callable=PropertyMock, return_value={ "BrewSignRPMHandler": { - "image": [{ + "image": { "advisory_security_impact": [ "Normal", "Important" ] - }] + } } }) def test_allow_security_impact_important_false( @@ -204,7 +204,7 @@ class TestBrewSignHandler(helpers.ModelsTestCase): @patch('freshmaker.errata.Errata.builds_signed') @patch("freshmaker.config.Config.handler_build_whitelist", new_callable=PropertyMock, return_value={ - "BrewSignRPMHandler": {"image": [{"advisory_name": "RHSA-.*"}]}}) + "BrewSignRPMHandler": {"image": {"advisory_name": "RHSA-.*"}}}) def test_do_not_create_already_handled_event( self, handler_build_whitelist, builds_signed, advisories_from_event): diff --git a/tests/test_errata_advisory_rpms_signed_handler.py b/tests/test_errata_advisory_rpms_signed_handler.py index 332cbb6..90d24c1 100644 --- a/tests/test_errata_advisory_rpms_signed_handler.py +++ b/tests/test_errata_advisory_rpms_signed_handler.py @@ -32,6 +32,7 @@ from freshmaker.lightblue import ContainerImage from freshmaker.models import Event, Compose from freshmaker.types import EventState from freshmaker.errata import ErrataAdvisory +from freshmaker.config import any_ from tests import helpers @@ -212,7 +213,7 @@ class TestErrataAdvisoryRPMsSignedHandler(helpers.ModelsTestCase): @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ 'ErrataAdvisoryRPMsSignedHandler': { - 'image': [{'product_short_name': 'foo'}] + 'image': {'product_short_name': 'foo'} } }) @patch.object(freshmaker.conf, 'dry_run', new=True) @@ -230,9 +231,9 @@ class TestErrataAdvisoryRPMsSignedHandler(helpers.ModelsTestCase): @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ 'ErrataAdvisoryRPMsSignedHandler': { - 'image': [ - {'advisory_highest_cve_severity': ['critical', 'important']} - ] + 'image': { + 'advisory_highest_cve_severity': ['critical', 'important'] + } } }) @patch.object(freshmaker.conf, 'dry_run', new=True) @@ -259,7 +260,7 @@ class TestErrataAdvisoryRPMsSignedHandler(helpers.ModelsTestCase): @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ 'ErrataAdvisoryRPMsSignedHandler': { - 'image': [{'advisory_name': 'RHBA-2017'}] + 'image': {'advisory_name': 'RHBA-2017'} } }) @patch.object(freshmaker.conf, 'dry_run', new=True) @@ -276,7 +277,7 @@ class TestErrataAdvisoryRPMsSignedHandler(helpers.ModelsTestCase): @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ 'ErrataAdvisoryRPMsSignedHandler': { - 'image': [{'advisory_name': 'RHBA-2017'}] + 'image': {'advisory_name': 'RHBA-2017'} } }) def test_event_state_updated_when_no_images_to_rebuild(self): @@ -292,7 +293,7 @@ class TestErrataAdvisoryRPMsSignedHandler(helpers.ModelsTestCase): @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ 'ErrataAdvisoryRPMsSignedHandler': { - 'image': [{'advisory_name': 'RHBA-2017'}] + 'image': {'advisory_name': 'RHBA-2017'} } }) def test_event_state_updated_when_all_images_failed(self): @@ -588,7 +589,7 @@ class TestFindImagesToRebuild(helpers.FreshmakerTestCase): @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ 'ErrataAdvisoryRPMsSignedHandler': { - 'image': [{'advisory_name': 'RHBA-*'}] + 'image': {'advisory_name': 'RHBA-*'} } }) @patch('os.path.exists', return_value=True) @@ -603,7 +604,7 @@ class TestFindImagesToRebuild(helpers.FreshmakerTestCase): @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ 'ErrataAdvisoryRPMsSignedHandler': { - 'image': [{'advisory_name': 'RHBA-*'}] + 'image': {'advisory_name': 'RHBA-*'} } }) @patch('os.path.exists', return_value=True) @@ -619,10 +620,10 @@ class TestFindImagesToRebuild(helpers.FreshmakerTestCase): @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ 'ErrataAdvisoryRPMsSignedHandler': { - 'image': [{'advisory_name': 'RHBA-*', 'published': True, - 'advisory_product_short_name': 'foo'}, - {'advisory_name': 'RHBA-*', 'published': False, - 'advisory_product_short_name': 'product'}] + 'image': any_({'advisory_name': 'RHBA-*', 'published': True, + 'advisory_product_short_name': 'foo'}, + {'advisory_name': 'RHBA-*', 'published': False, + 'advisory_product_short_name': 'product'}) } }) @patch('os.path.exists', return_value=True) @@ -637,8 +638,8 @@ class TestFindImagesToRebuild(helpers.FreshmakerTestCase): @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ 'ErrataAdvisoryRPMsSignedHandler': { - 'image': [{'advisory_name': 'RHBA-*', - 'published': True}] + 'image': {'advisory_name': 'RHBA-*', + 'published': True} } }) @patch('os.path.exists', return_value=True) diff --git a/tests/test_errata_advisory_state_changed.py b/tests/test_errata_advisory_state_changed.py index 1167078..caf2f6a 100644 --- a/tests/test_errata_advisory_state_changed.py +++ b/tests/test_errata_advisory_state_changed.py @@ -72,7 +72,7 @@ class TestAllowBuild(helpers.ModelsTestCase): "_find_images_to_rebuild", return_value=[]) @patch("freshmaker.config.Config.handler_build_whitelist", new_callable=PropertyMock, return_value={ - "ErrataAdvisoryRPMsSignedHandler": {"image": [{"advisory_name": "RHSA-.*"}]}}) + "ErrataAdvisoryRPMsSignedHandler": {"image": {"advisory_name": "RHSA-.*"}}}) def test_allow_build_false(self, handler_build_whitelist, record_images): """ Tests that allow_build filters out advisories based on advisory_name. @@ -91,7 +91,7 @@ class TestAllowBuild(helpers.ModelsTestCase): "_find_images_to_rebuild", return_value=[]) @patch("freshmaker.config.Config.handler_build_whitelist", new_callable=PropertyMock, return_value={ - "ErrataAdvisoryRPMsSignedHandler": {"image": [{"advisory_name": "RHSA-.*"}]}}) + "ErrataAdvisoryRPMsSignedHandler": {"image": {"advisory_name": "RHSA-.*"}}}) def test_allow_build_true(self, handler_build_whitelist, record_images): """ Tests that allow_build does not filter out advisories based on @@ -115,12 +115,12 @@ class TestAllowBuild(helpers.ModelsTestCase): new_callable=PropertyMock, return_value={ "ErrataAdvisoryRPMsSignedHandler": { - "image": [{ + "image": { "advisory_security_impact": [ "Normal", "Important" ], "image_name": "foo", - }] + } } }) def test_allow_security_impact_important_true( @@ -146,11 +146,11 @@ class TestAllowBuild(helpers.ModelsTestCase): new_callable=PropertyMock, return_value={ "ErrataAdvisoryRPMsSignedHandler": { - "image": [{ + "image": { "advisory_security_impact": [ "Normal", "Important" ] - }] + } } }) def test_allow_security_impact_important_false( @@ -174,9 +174,9 @@ class TestAllowBuild(helpers.ModelsTestCase): new_callable=PropertyMock, return_value={ "ErrataAdvisoryRPMsSignedHandler": { - "image": [{ + "image": { "image_name": ["foo", "bar"] - }] + } } }) def test_filter_out_not_allowed_builds( @@ -213,10 +213,10 @@ class TestAllowBuild(helpers.ModelsTestCase): new_callable=PropertyMock, return_value={ "ErrataAdvisoryRPMsSignedHandler": { - "image": [{ + "image": { "image_name": ["foo", "bar"], "advisory_name": "RHSA-.*", - }] + } } }) def test_filter_out_image_name_and_advisory_name( @@ -618,11 +618,9 @@ class TestErrataAdvisoryStateChangedHandler(helpers.ModelsTestCase): @patch('freshmaker.errata.Errata.advisories_from_event') @patch.object(conf, 'handler_build_whitelist', new={ 'ErrataAdvisoryStateChangedHandler': { - 'image': [ - { - 'advisory_state': r'REL_PREP|SHIPPED_LIVE', - } - ] + 'image': { + 'advisory_state': r'REL_PREP|SHIPPED_LIVE', + } } }) def test_rebuild_if_not_exists_unknown_states( @@ -720,11 +718,9 @@ class TestErrataAdvisoryStateChangedHandler(helpers.ModelsTestCase): '.rebuild_if_not_exists') @patch.object(conf, 'handler_build_whitelist', new={ 'ErrataAdvisoryStateChangedHandler': { - 'image': [ - { - 'advisory_state': r'REL_PREP', - } - ] + 'image': { + 'advisory_state': r'REL_PREP', + } } }) def test_not_rebuild_if_errata_state_is_not_allowed( @@ -747,11 +743,9 @@ class TestErrataAdvisoryStateChangedHandler(helpers.ModelsTestCase): '.rebuild_if_not_exists') @patch.object(conf, 'handler_build_whitelist', new={ 'ErrataAdvisoryStateChangedHandler': { - 'image': [ - { - 'advisory_state': r'REL_PREP', - } - ] + 'image': { + 'advisory_state': r'REL_PREP', + } } }) def test_rebuild_if_errata_state_is_not_allowed_but_manual_is_true( diff --git a/tests/test_git_dockerfile_change_handler.py b/tests/test_git_dockerfile_change_handler.py index d13307b..e2e698b 100644 --- a/tests/test_git_dockerfile_change_handler.py +++ b/tests/test_git_dockerfile_change_handler.py @@ -30,6 +30,7 @@ import freshmaker from freshmaker import models from freshmaker.consumer import FreshmakerConsumer from freshmaker.types import ArtifactType +from freshmaker.config import any_ from tests import get_fedmsg, helpers @@ -56,7 +57,7 @@ class GitDockerfileChangeHandlerTest(BaseTestCase): new_callable=PropertyMock, return_value="user@example.com") @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ 'GitDockerfileChangeHandler': { - 'image': [{'name': 'testimage'}, {'branch': 'master'}] + 'image': any_({'name': 'testimage'}, {'branch': 'master'}) } }) def test_rebuild_if_dockerfile_changed( @@ -97,7 +98,7 @@ class GitDockerfileChangeHandlerTest(BaseTestCase): @patch('koji.ClientSession') @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ 'GitDockerfileChangeHandler': { - 'image': [{'name': 'testimage'}, {'branch': 'master'}] + 'image': any_({'name': 'testimage'}, {'branch': 'master'}) } }) def test_ensure_logout_in_whatever_case(self, ClientSession, read_config): diff --git a/tests/test_git_module_metadata_change_handler.py b/tests/test_git_module_metadata_change_handler.py index a916b13..ea1c7a9 100644 --- a/tests/test_git_module_metadata_change_handler.py +++ b/tests/test_git_module_metadata_change_handler.py @@ -32,6 +32,7 @@ from freshmaker import events, models from freshmaker.types import ArtifactType from freshmaker.handlers.git import GitModuleMetadataChangeHandler from freshmaker.parsers.git import GitReceiveParser +from freshmaker.config import any_ class GitModuleMetadataChangeHandlerTest(helpers.ModelsTestCase): @@ -54,7 +55,7 @@ class GitModuleMetadataChangeHandlerTest(helpers.ModelsTestCase): @mock.patch.object(freshmaker.conf, 'handler_build_whitelist', new={ 'GitModuleMetadataChangeHandler': { - 'module': [{'name': 'testmodule'}, {'branch': 'master'}] + 'module': any_({'name': 'testmodule'}, {'branch': 'master'}) } }) def test_can_rebuild_module_when_module_metadata_changed(self): diff --git a/tests/test_git_rpm_spec_change_handler.py b/tests/test_git_rpm_spec_change_handler.py index 568e21c..2d423d8 100644 --- a/tests/test_git_rpm_spec_change_handler.py +++ b/tests/test_git_rpm_spec_change_handler.py @@ -32,6 +32,7 @@ from freshmaker import events, models from freshmaker.types import ArtifactType from freshmaker.handlers.git import GitRPMSpecChangeHandler from freshmaker.parsers.git import GitReceiveParser +from freshmaker.config import any_ class GitRPMSpecChangeHandlerTest(helpers.ModelsTestCase): @@ -71,7 +72,7 @@ class GitRPMSpecChangeHandlerTest(helpers.ModelsTestCase): @mock.patch('freshmaker.handlers.git.rpm_spec_change.conf') @mock.patch.object(freshmaker.conf, 'handler_build_whitelist', new={ 'GitRPMSpecChangeHandler': { - 'module': [{'name': 'testmodule'}, {'branch': 'master'}] + 'module': any_({'name': 'testmodule'}, {'branch': 'master'}) } }) def test_can_rebuild_modules_has_rpm_included(self, conf, utils, PDC): diff --git a/tests/test_handler.py b/tests/test_handler.py index 0c9b3c8..19f6c56 100644 --- a/tests/test_handler.py +++ b/tests/test_handler.py @@ -35,6 +35,7 @@ from freshmaker.models import ( ) from freshmaker.errors import UnprocessableEntity, ProgrammingError from freshmaker.types import ArtifactType, EventState +from freshmaker.config import any_, all_ from tests import helpers @@ -224,7 +225,7 @@ class TestAllowBuildBasedOnWhitelist(helpers.FreshmakerTestCase): @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"}]} + whitelist_rules = {"image": any_({'name': "test"})} handler = MyHandler() conf.handler_build_whitelist.get.return_value = whitelist_rules container = {"name": "test", "branch": "branch"} @@ -237,7 +238,7 @@ class TestAllowBuildBasedOnWhitelist(helpers.FreshmakerTestCase): @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"}]} + whitelist_rules = {"image": any_({'name': "test1"})} handler = MyHandler() conf.handler_build_whitelist.get.return_value = whitelist_rules container = {"name": "test", "branch": "branch"} @@ -251,7 +252,7 @@ class TestAllowBuildBasedOnWhitelist(helpers.FreshmakerTestCase): 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"}]} + whitelist_rules = {"image": any_({'name': "te(st"})} handler = MyHandler() conf.handler_build_whitelist.get.return_value = whitelist_rules container = {"name": "test", "branch": "branch"} @@ -263,9 +264,9 @@ class TestAllowBuildBasedOnWhitelist(helpers.FreshmakerTestCase): @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ 'MyHandler': { - 'image': [ - {'advisory_state': ['REL_PREP', 'SHIPPED_LIVE']} - ] + 'image': { + 'advisory_state': ['REL_PREP', 'SHIPPED_LIVE'] + } } }) def test_rule_not_defined(self): @@ -280,10 +281,10 @@ class TestAllowBuildBasedOnWhitelist(helpers.FreshmakerTestCase): @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ 'MyHandler': { - 'image': [ - {'advisory_state': ['REL_PREP', 'SHIPPED_LIVE'], - 'published': False} - ] + 'image': { + 'advisory_state': ['REL_PREP', 'SHIPPED_LIVE'], + 'published': False + } } }) def test_boolean_rule(self): @@ -294,9 +295,9 @@ class TestAllowBuildBasedOnWhitelist(helpers.FreshmakerTestCase): @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ 'MyHandler': { - 'image': [ - {'advisory_state': ['REL_PREP', 'SHIPPED_LIVE']} - ] + 'image': { + 'advisory_state': ['REL_PREP', 'SHIPPED_LIVE'] + } } }) def test_not_allow_if_none_passed_rule_is_configured(self): @@ -312,9 +313,9 @@ class TestAllowBuildBasedOnWhitelist(helpers.FreshmakerTestCase): @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ 'MyHandler': { - 'image': [ - {'advisory_state': ['REL_PREP', 'SHIPPED_LIVE']} - ] + 'image': { + 'advisory_state': ['REL_PREP', 'SHIPPED_LIVE'] + } } }) def test_define_rule_values_as_list(self): @@ -325,9 +326,9 @@ class TestAllowBuildBasedOnWhitelist(helpers.FreshmakerTestCase): @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ 'MyHandler': { - 'image': [ - {'advisory_name': 'RHSA-\d+:\d+'} - ] + 'image': { + 'advisory_name': 'RHSA-\d+:\d+' + } } }) def test_define_rule_value_as_single_regex_string(self): @@ -342,10 +343,10 @@ class TestAllowBuildBasedOnWhitelist(helpers.FreshmakerTestCase): @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ 'MyHandler': { - 'image': [{ + 'image': { 'advisory_name': 'RHSA-\d+:\d+', 'advisory_state': 'REL_PREP' - }] + } } }) def test_AND_rule(self): @@ -362,10 +363,10 @@ class TestAllowBuildBasedOnWhitelist(helpers.FreshmakerTestCase): @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ 'MyHandler': { - 'image': [ + 'image': any_( {'advisory_name': 'RHSA-\d+:\d+'}, {'advisory_state': 'REL_PREP'}, - ] + ) } }) def test_OR_rule(self): @@ -379,3 +380,40 @@ class TestAllowBuildBasedOnWhitelist(helpers.FreshmakerTestCase): advisory_name='RHSA-2017', advisory_state='REL_PREP') self.assertTrue(allowed) + + @patch.object(freshmaker.conf, 'handler_build_whitelist', new={ + 'MyHandler': { + 'image': all_( + {'advisory_name': 'RHSA-\d+:\d+'}, + any_( + {'has_hightouch_bugs': True}, + {'severity': ['critical', 'important']} + ), + ) + } + }) + def test_OR_between_subrules(self): + handler = MyHandler() + allowed = handler.allow_build(ArtifactType.IMAGE, + advisory_name='RHSA-2017:1000', + has_hightouch_bugs=True, + severity="low") + self.assertTrue(allowed) + + allowed = handler.allow_build(ArtifactType.IMAGE, + advisory_name='RHSA-2017:1000', + has_hightouch_bugs=False, + severity="critical") + self.assertTrue(allowed) + + allowed = handler.allow_build(ArtifactType.IMAGE, + advisory_name='RHSA-2017:1000', + has_hightouch_bugs=False, + severity="low") + self.assertFalse(allowed) + + allowed = handler.allow_build(ArtifactType.IMAGE, + advisory_name='RHBA-2017:1000', + has_hightouch_bugs=False, + severity="critical") + self.assertFalse(allowed) diff --git a/tests/test_mbs_module_state_change_handler.py b/tests/test_mbs_module_state_change_handler.py index 6edd674..ccdde12 100644 --- a/tests/test_mbs_module_state_change_handler.py +++ b/tests/test_mbs_module_state_change_handler.py @@ -32,6 +32,7 @@ from freshmaker import events, db, models from freshmaker.types import ArtifactType from freshmaker.handlers.mbs import MBSModuleStateChangeHandler from freshmaker.parsers.mbs import MBSModuleStateChangeParser +from freshmaker.config import any_ class MBSModuleStateChangeHandlerTest(helpers.ModelsTestCase): @@ -55,7 +56,7 @@ class MBSModuleStateChangeHandlerTest(helpers.ModelsTestCase): @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'}], + 'module': any_({'name': r'testmodule\d*'}, {'branch': 'master'}), } }) def test_can_rebuild_depending_modules(self, conf, utils, PDC): @@ -112,11 +113,11 @@ class MBSModuleStateChangeHandlerTest(helpers.ModelsTestCase): def test_module_is_not_allowed_in_whitelist(self, conf, utils, PDC): conf.handler_build_whitelist = { "MBSModuleStateChangeHandler": { - "module": [ + "module": any_( { 'name': 'base.*', }, - ], + ), }, } @@ -144,7 +145,7 @@ class MBSModuleStateChangeHandlerTest(helpers.ModelsTestCase): @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'}] + 'module': any_({'name': r'module\d+'}, {'branch': 'master'}) } }) def test_handler_not_fall_into_cyclic_rebuild_loop(self, log, utils, PDC):