Koji tag validation

- Decorator supporting str/list args
- ProgrammingError exception which may uncover typos in the names of args to validate
- Configurable whitelist of koji tag prefixes
- Add basic validation to test_build.TestModuleBuilder

conf/config.py

@@ -31,6 +31,7 @@ class BaseConfiguration(object):
     KOJI_ARCHES = ['i686', 'armv7hl', 'x86_64']
     KOJI_PROXYUSER = True
     KOJI_REPOSITORY_URL = 'https://kojipkgs.stg.fedoraproject.org/repos'
+    KOJI_TAG_PREFIXES = ['module']
     COPR_CONFIG = '/etc/module-build-service/copr.conf'
     PDC_URL = 'http://modularity.fedorainfracloud.org:8080/rest_api/v1'
     PDC_INSECURE = True

module_build_service/builder.py

@@ -318,6 +318,7 @@ class KojiModuleBuilder(GenericBuilder):
     backend = "koji"
     _build_lock = threading.Lock()
 
+    @module_build_service.utils.validate_koji_tag('tag_name')
     def __init__(self, owner, module, config, tag_name):
         """
         :param owner: a string representing who kicked off the builds
@@ -378,6 +379,7 @@ class KojiModuleBuilder(GenericBuilder):
 
 
     @staticmethod
+    @module_build_service.utils.validate_koji_tag('disttag', pre='.', post='_')
     def get_disttag_srpm(disttag):
 
         #Taken from Karsten's create-distmacro-pkg.sh
@@ -667,6 +669,7 @@ chmod 644 %buildroot/%_rpmconfigdir/macros.d/macros.modules
         self.koji_session.cancelTask(task_id)
 
     @classmethod
+    @module_build_service.utils.validate_koji_tag('tag_name')
     def repo_from_tag(cls, config, tag_name, arch):
         """
         :param config: instance of rida.config.Config
@@ -678,6 +681,7 @@ chmod 644 %buildroot/%_rpmconfigdir/macros.d/macros.modules
         """
         return "%s/%s/latest/%s" % (config.koji_repository_url, tag_name, arch)
 
+    @module_build_service.utils.validate_koji_tag('tag')
     def _get_tag(self, tag, strict=True):
         if isinstance(tag, dict):
             tag = tag['name']
@@ -687,6 +691,8 @@ 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')
     def _koji_add_many_tag_inheritance(self, tag_name, parent_tags):
         tag = self._get_tag(tag_name)
         # highest priority num is at the end
@@ -719,6 +725,7 @@ chmod 644 %buildroot/%_rpmconfigdir/macros.d/macros.modules
         if inheritance_data:
             self.koji_session.setInheritanceData(tag['id'], inheritance_data)
 
+    @module_build_service.utils.validate_koji_tag('dest_tag')
     def _koji_add_groups_to_tag(self, dest_tag, groups=None):
         """
         :param build_tag_name
@@ -748,6 +755,7 @@ chmod 644 %buildroot/%_rpmconfigdir/macros.d/macros.modules
                 self.koji_session.groupPackageListAdd(dest_tag, group, pkg)
 
 
+    @module_build_service.utils.validate_koji_tag('tag_name')
     def _koji_create_tag(self, tag_name, arches=None, perm=None):
         """
         :param tag_name: name of koji tag
@@ -818,6 +826,8 @@ 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')
     def _koji_add_target(self, name, build_tag, dest_tag):
         """
         :param name: target name
@@ -896,6 +906,7 @@ class CoprModuleBuilder(GenericBuilder):
     backend = "copr"
     _build_lock = threading.Lock()
 
+    @module_build_service.utils.validate_koji_tag('tag_name')
     def __init__(self, owner, module, config, tag_name):
         self.owner = owner
         self.config = config
@@ -1070,6 +1081,7 @@ class CoprModuleBuilder(GenericBuilder):
         log.info(result.data["modulemd"])
 
     @staticmethod
+    @module_build_service.utils.validate_koji_tag('disttag', pre='.', post='_')
     def get_disttag_srpm(disttag):
         # @FIXME
         return KojiModuleBuilder.get_disttag_srpm(disttag)
@@ -1080,6 +1092,7 @@ class CoprModuleBuilder(GenericBuilder):
         return {"name": self.tag_name}
 
     @classmethod
+    @module_build_service.utils.validate_koji_tag('tag_name')
     def repo_from_tag(cls, config, tag_name, arch):
         """
         :param backend: a string representing the backend e.g. 'koji'.
@@ -1110,6 +1123,7 @@ class CoprModuleBuilder(GenericBuilder):
         pass
 
     @classmethod
+    @module_build_service.utils.validate_koji_tag('koji_tag')
     def _tag_to_copr_name(cls, koji_tag):
         return koji_tag.replace("+", "-")
 
@@ -1162,6 +1176,7 @@ mdpolicy=group:primary
 
 """
 
+    @module_build_service.utils.validate_koji_tag('tag_name')
     def __init__(self, owner, module, config, tag_name):
         self.module_str = module
         self.tag_name = tag_name
@@ -1478,6 +1493,7 @@ mdpolicy=group:primary
             return self.build_srpm(artifact_name, source, build_id)
 
     @staticmethod
+    @module_build_service.utils.validate_koji_tag('disttag', pre='.', post='_')
     def get_disttag_srpm(disttag):
         # @FIXME
         return KojiModuleBuilder.get_disttag_srpm(disttag)

module_build_service/config.py

@@ -162,8 +162,11 @@ class Config(object):
         'koji_build_macros_target': {
             'type': str,
             'default': '',
-            'desc': 'Target to build "module-build-macros" RPM in.'
-            },
+            'desc': 'Target to build "module-build-macros" RPM in.'},
+        'koji_tag_prefixes': {
+            'type': list,
+            'default': ['module'],
+            'desc': 'List of allowed koji tag prefixes.'},
         'rpms_default_repository': {
             'type': str,
             'default': 'git://pkgs.fedoraproject.org/rpms/',

module_build_service/errors.py

@@ -47,6 +47,10 @@ class NotFound(ValueError):
     pass
 
 
+class ProgrammingError(ValueError):
+    pass
+
+
 def json_error(status, error, message):
     response = jsonify(
         {'status': status,

module_build_service/utils.py

@@ -31,6 +31,7 @@ import os
 import logging
 import copy
 import kobo.rpmlib
+import inspect
 from six import iteritems
 
 import modulemd
@@ -39,7 +40,8 @@ from flask import request, url_for
 from datetime import datetime
 
 from module_build_service import log, models
-from module_build_service.errors import ValidationError, UnprocessableEntity
+from module_build_service.errors import (ValidationError, UnprocessableEntity,
+                                         ProgrammingError)
 from module_build_service import conf, db
 from module_build_service.errors import (Unauthorized, Conflict)
 import module_build_service.messaging
@@ -767,3 +769,36 @@ def get_reusable_component(session, module, component_name):
         return reusable_component
 
     return None
+
+
+def validate_koji_tag(tag_arg_name, pre='', post='-'):
+    """
+    Used as a decorator validates koji tag arg value (which may be str or list)
+    against configurable list of koji tag prefixes.
+    :param pre: Prepend this optional string (e.g. '.' in case of disttag
+    validation) to each koji tag prefix.
+    :param post: Append this string/delimiter ('-' by default) to each koji
+    tag prefix.
+    """
+    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.'
+                    .format(tag_arg_name))
+            if 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]))
+            return function(*args, **kwargs)
+        return wrapper
+    return validation_decorator

tests/test_build/test_build.py

@@ -90,6 +90,7 @@ class TestModuleBuilder(GenericBuilder):
     on_buildroot_add_artifacts_cb = None
     on_tag_artifacts_cb = None
 
+    @module_build_service.utils.validate_koji_tag('tag_name')
     def __init__(self, owner, module, config, tag_name):
         self.module_str = module
         self.tag_name = tag_name
@@ -189,6 +190,7 @@ class TestModuleBuilder(GenericBuilder):
         return TestModuleBuilder._build_id, state, reason, None
 
     @staticmethod
+    @module_build_service.utils.validate_koji_tag('disttag', pre='.', post='_')
     def get_disttag_srpm(disttag):
         # @FIXME
         return KojiModuleBuilder.get_disttag_srpm(disttag)