From ca61d6bb299d025ace68c14edb61dc024e8f616c Mon Sep 17 00:00:00 2001 From: Ralph Bean Date: Tue, 28 Feb 2017 17:15:55 -0500 Subject: [PATCH] Allow passing in multiple tag names to the validation decorator. We have problems when we try to wrap one decorator around another. There are ways to do that with the [decorator](https://pypi.python.org/pypi/decorator) module nicely, however.. they are ugly. Take a look at this: https://github.com/micheles/decorator/blob/master/src/decorator.py#L217-L218 And this: https://github.com/micheles/decorator/blob/master/src/decorator.py#L185-L188 The approach in this PR is.. simpler. --- module_build_service/builder.py | 6 +-- module_build_service/utils.py | 69 ++++++++++++++++++++++----------- tests/test_utils/test_utils.py | 22 +++++++++++ 3 files changed, 70 insertions(+), 27 deletions(-) diff --git a/module_build_service/builder.py b/module_build_service/builder.py index 8e992384..e3742b42 100644 --- a/module_build_service/builder.py +++ b/module_build_service/builder.py @@ -691,8 +691,7 @@ chmod 644 %buildroot/%_rpmconfigdir/macros.d/macros.modules raise SystemError("Unknown tag: %s" % tag) return taginfo - @module_build_service.utils.validate_koji_tag('tag_name') - @module_build_service.utils.validate_koji_tag('parent_tags') + @module_build_service.utils.validate_koji_tag(['tag_name', 'parent_tags']) def _koji_add_many_tag_inheritance(self, tag_name, parent_tags): tag = self._get_tag(tag_name) # highest priority num is at the end @@ -826,8 +825,7 @@ chmod 644 %buildroot/%_rpmconfigdir/macros.d/macros.modules self.koji_session.packageListAdd(self.module_tag['name'], package, owner) - @module_build_service.utils.validate_koji_tag('build_tag') - @module_build_service.utils.validate_koji_tag('dest_tag') + @module_build_service.utils.validate_koji_tag(['build_tag', 'dest_tag']) def _koji_add_target(self, name, build_tag, dest_tag): """ :param name: target name diff --git a/module_build_service/utils.py b/module_build_service/utils.py index 5f157b9a..64e052ad 100644 --- a/module_build_service/utils.py +++ b/module_build_service/utils.py @@ -784,7 +784,7 @@ def get_reusable_component(session, module, component_name): return None -def validate_koji_tag(tag_arg_name, pre='', post='-', dict_key='name'): +def validate_koji_tag(tag_arg_names, pre='', post='-', dict_key='name'): """ Used as a decorator validates koji tag arg value (which may be str or list) against configurable list of koji tag prefixes. @@ -794,33 +794,56 @@ def validate_koji_tag(tag_arg_name, pre='', post='-', dict_key='name'): tag prefix. :param dict_key: In case of a dict arg, inspect this key ('name' by default). """ + + if not isinstance(tag_arg_names, list): + tag_arg_names = [tag_arg_names] + def validation_decorator(function): def wrapper(*args, **kwargs): call_args = inspect.getcallargs(function, *args, **kwargs) - if tag_arg_name not in call_args: - raise ProgrammingError( - 'Koji tag validation: Inspected argument {} is not within function args.' - ' The function was: {}.' - .format(tag_arg_name, function.__name__)) - if isinstance(call_args[tag_arg_name], dict): - if dict_key not in call_args[tag_arg_name]: + + for tag_arg_name in tag_arg_names: + + # If any of them don't appear in the function, then fail. + if tag_arg_name not in call_args: raise ProgrammingError( - 'Koji tag validation: Inspected dict arg {} does not contain {} key.' + 'Koji tag validation: Inspected argument {} is not within function args.' ' The function was: {}.' - .format(call_args[tag_arg_name], dict_key, function.__name__)) - tag_list = [call_args[tag_arg_name][dict_key]] - elif isinstance(call_args[tag_arg_name], list): - tag_list = call_args[tag_arg_name] - else: - tag_list = [call_args[tag_arg_name]] - for tag_prefix in conf.koji_tag_prefixes: - if all([t.startswith(pre + tag_prefix + post) for t in tag_list]): - break - else: - raise ValidationError( - 'Koji tag validation: {} does not satisfy any of allowed prefixes: {}' - .format(tag_list, - [pre + p + post for p in conf.koji_tag_prefixes])) + .format(tag_arg_name, function.__name__)) + + # If any of them are a dict, then use the provided dict_key + if isinstance(call_args[tag_arg_name], dict): + if dict_key not in call_args[tag_arg_name]: + raise ProgrammingError( + 'Koji tag validation: Inspected dict arg {} does not contain {} key.' + ' The function was: {}.' + .format(call_args[tag_arg_name], dict_key, function.__name__)) + tag_list = [call_args[tag_arg_name][dict_key]] + elif isinstance(call_args[tag_arg_name], list): + tag_list = call_args[tag_arg_name] + else: + tag_list = [call_args[tag_arg_name]] + + # Check to make sure the provided values match our whitelist. + for allowed_prefix in conf.koji_tag_prefixes: + if all([t.startswith(pre + allowed_prefix + post) for t in tag_list]): + break + else: + # Only raise this error if the given tags don't start with + # *any* of our allowed prefixes. + raise ValidationError( + 'Koji tag validation: {} does not satisfy any of allowed prefixes: {}' + .format(tag_list, + [pre + p + post for p in conf.koji_tag_prefixes])) + + # Finally.. after all that validation, call the original function + # and return its value. return function(*args, **kwargs) + + # We're replacing the original function with our synthetic wrapper, + # but dress it up to make it look more like the original function. + wrapper.__name__ = function.__name__ + wrapper.__doc__ = function.__doc__ return wrapper + return validation_decorator diff --git a/tests/test_utils/test_utils.py b/tests/test_utils/test_utils.py index a357a879..4795ae91 100644 --- a/tests/test_utils/test_utils.py +++ b/tests/test_utils/test_utils.py @@ -233,3 +233,25 @@ class TestUtils(unittest.TestCase): self.assertEquals( validate_koji_tag_good_tag_value_in_dict_nondefault_key( {'nondefault': 'module-foo'}), True) + + def test_validate_koji_tag_double_trouble_good(self): + """ Test that we pass on a list of tags that are good. """ + + expected = 'foo' + + @module_build_service.utils.validate_koji_tag(['tag_arg1', 'tag_arg2']) + def validate_koji_tag_double_trouble(tag_arg1, tag_arg2): + return expected + + actual = validate_koji_tag_double_trouble('module-1', 'module-2') + self.assertEquals(actual, expected) + + def test_validate_koji_tag_double_trouble_bad(self): + """ Test that we fail on a list of tags that are bad. """ + + @module_build_service.utils.validate_koji_tag(['tag_arg1', 'tag_arg2']) + def validate_koji_tag_double_trouble(tag_arg1, tag_arg2): + pass + + with self.assertRaises(ValidationError): + validate_koji_tag_double_trouble('module-1', 'BADNEWS-2')