Merge pull request #2601 from imobachgs/maintainers-check

Check for maintainers in the source project

check_source.py

@@ -11,6 +11,8 @@
 except ImportError:
     import cElementTree as ET
 
+from lxml import etree as ETL
+
 import osc.conf
 import osc.core
 from osc.util.helper import decode_list
@@ -21,6 +23,8 @@
 from osclib.core import package_kind
 from osclib.core import source_file_load
 from osclib.core import target_archs
+from osclib.core import create_add_role_request
+from osc.core import show_project_meta
 from urllib.error import HTTPError
 
 import ReviewBot
@@ -52,6 +56,7 @@ def target_project_config(self, project):
         self.mail_release_list = config.get('mail-release-list')
         self.staging_group = config.get('staging-group')
         self.repo_checker = config.get('repo-checker')
+        self.required_maintainer = config.get('required-source-maintainer', '')
         self.devel_whitelist = config.get('devel-whitelist', '').split()
         self.skip_add_reviews = False
         self.security_review_team = config.get('security-review-team', 'security-team')
@@ -159,6 +164,25 @@ def check_source_submission(self, source_project, source_package, source_revisio
                 if match:
                     inair_renamed = target_package != match.group(1)
 
+        if not self.source_has_correct_maintainers(source_project):
+            declined_msg = (
+                'This request cannot be accepted unless %s is a maintainer of %s.' %
+                (self.required_maintainer, source_project)
+            )
+
+            try:
+                add_role_msg = 'Created automatically from request %s' % self.request.reqid
+                add_role_reqid = create_add_role_request(self.apiurl, source_project, self.required_maintainer,
+                                                         'maintainer', message=add_role_msg)
+                declined_msg += ' Created the add_role request %s for addressing this problem.' % add_role_reqid
+            except HTTPError as e:
+                self.logger.error(
+                    'Cannot create the corresponding add_role request for %s: %s' % (self.request.reqid, e)
+                )
+
+            self.review_messages['declined'] = declined_msg
+            return False
+
         if not self.in_air_rename_allow and inair_renamed:
             self.review_messages['declined'] = 'Source and target package names must match'
             return False
@@ -313,6 +337,25 @@ def is_devel_project(self, source_project, target_project):
         result = osc.core.search(self.apiurl, **search)
         return result['package'].attrib['matches'] != '0'
 
+    def source_has_correct_maintainers(self, source_project):
+        """Checks whether the source project has the required maintainer
+
+        If a 'required-source-maintainer' is set, it checks whether it is a
+        maintainer for the source project.
+
+        source_project - source project name
+        """
+        self.logger.info(
+            'Checking required maintainer from the source project (%s)' % self.required_maintainer
+        )
+        if not self.required_maintainer: return True
+
+        meta = ETL.fromstringlist(show_project_meta(self.apiurl, source_project))
+        maintainers = meta.xpath('//person[@role="maintainer"]/@userid')
+        maintainers += ['group:' + g for g in meta.xpath('//group[@role="maintainer"]/@groupid')]
+
+        return self.required_maintainer in maintainers
+
     @staticmethod
     def checkout_package(*args, **kwargs):
         _stdout = sys.stdout

osclib/conf.py

@@ -47,6 +47,7 @@
         'mail-noreply': 'noreply@opensuse.org',
         'mail-release-list': 'opensuse-releaseteam@opensuse.org',
         'always_set_productversion_to': '',
+        'required-source-maintainer': 'group:factory-maintainers',
     },
     r'openSUSE:(?P<project>Factory):ARM$': {
         'product': 'openSUSE.product',

osclib/core.py

@@ -1098,6 +1098,21 @@ def create_change_devel_request(apiurl, source_project, source_package,
             tgt_project=target_project, tgt_package=target_package)
     return create_request(apiurl, action, message)
 
+def create_add_role_request(apiurl, target_project, user, role, target_package=None, message=None):
+    """Create an add_role request
+
+    user -- user or group name. If it is a group, it has to start with 'group:'.
+    """
+
+    if user.startswith('group:'):
+        group = user.replace('group:', '')
+        kargs = dict(group_name=group, group_role=role)
+    else:
+        kargs = dict(person_name=user, person_role=role)
+
+    action = Action('add_role', tgt_project=target_project, tgt_package=target_package, **kargs)
+    return create_request(apiurl, action, message)
+
 def create_request(apiurl, action, message=None):
     """Create a request for the given action
 

tests/check_source_tests.py

@@ -3,11 +3,14 @@
 from check_source import CheckSource
 import random
 import os
+from osclib.core import request_action_list
+from osc.core import get_request_list
 
 PROJECT = 'openSUSE:Factory'
 SRC_PROJECT = 'devel:Fishing'
-FIXTURES = os.path.join(os.getcwd(), 'tests/fixtures')
+FIXTURES = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'fixtures')
 REVIEW_TEAM = 'reviewers-team'
+FACTORY_MAINTAINERS = 'group:factory-maintainers'
 
 # NOTE: Since there is no documentation explaining the good practices for creating tests for a
 # review bot, this test is created by mimicking parts of other existing tests. Most decisions are
@@ -22,6 +25,14 @@ def setUp(self):
 
         # Using OBSLocal.StagingWorkflow makes it easier to setup testing scenarios
         self.wf = OBSLocal.StagingWorkflow(PROJECT)
+
+        # Set up the reviewers team
+        self.wf.create_group(REVIEW_TEAM)
+
+        self.wf.remote_config_set(
+            { 'required-source-maintainer': 'Admin', 'review-team': REVIEW_TEAM }
+        )
+
         self.bot_user = 'factory-auto'
         self.wf.create_user(self.bot_user)
         self.project = self.wf.create_project(PROJECT)
@@ -43,6 +54,7 @@ def tearDown(self):
         del self.wf
 
     def test_no_devel_project(self):
+        """Declines the request when it does not come from a devel project"""
         req_id = self.wf.create_submit_request(SRC_PROJECT, self.randomString('package')).reqid
 
         self.assertReview(req_id, by_user=(self.bot_user, 'new'))
@@ -54,11 +66,50 @@ def test_no_devel_project(self):
         self.assertIn('%s is not a devel project of %s' % (SRC_PROJECT, PROJECT), review.comment)
 
     def test_devel_project(self):
+        """Accepts a request coming from a devel project"""
         self._setup_devel_project()
 
-        # Set up the reviewers team
-        self.wf.create_group(REVIEW_TEAM)
-        self.wf.remote_config_set({ 'review-team': REVIEW_TEAM }, replace_all=False)
+        req_id = self.wf.create_submit_request(SRC_PROJECT, 'blowfish').reqid
+
+        self.assertReview(req_id, by_user=(self.bot_user, 'new'))
+
+        self.review_bot.set_request_ids([req_id])
+        self.review_bot.check_requests()
+
+        self.assertReview(req_id, by_user=(self.bot_user, 'accepted'))
+        self.assertReview(req_id, by_group=(REVIEW_TEAM, 'new'))
+
+    def test_no_source_maintainer(self):
+        """Declines the request when the 'required_maintainer' is not maintainer of the source project"""
+        self._setup_devel_project()
+
+        # Change the required maintainer
+        self.wf.create_group(FACTORY_MAINTAINERS.replace('group:', ''))
+        self.wf.remote_config_set({ 'required-source-maintainer': FACTORY_MAINTAINERS })
+
+        req_id = self.wf.create_submit_request(SRC_PROJECT, 'blowfish').reqid
+
+        self.assertReview(req_id, by_user=(self.bot_user, 'new'))
+
+        self.review_bot.set_request_ids([req_id])
+        self.review_bot.check_requests()
+
+        review = self.assertReview(req_id, by_user=(self.bot_user, 'declined'))
+        add_role_req = get_request_list(self.wf.apiurl, SRC_PROJECT, req_state=['new'], req_type='add_role')[0]
+
+        self.assertIn('unless %s is a maintainer of %s' % (FACTORY_MAINTAINERS, SRC_PROJECT), review.comment)
+        self.assertIn('Created the add_role request %s' % add_role_req.reqid, review.comment)
+
+        self.assertEqual(add_role_req.actions[0].tgt_project, SRC_PROJECT)
+        self.assertEqual('Created automatically from request %s' % req_id, add_role_req.description)
+
+    def test_source_maintainer(self):
+        """Accepts the request when the 'required_maintainer' is a group and is a maintainer for the project"""
+        group_name = FACTORY_MAINTAINERS.replace('group:', '')
+        self.wf.create_group(group_name)
+        self.wf.remote_config_set({ 'required-source-maintainer': FACTORY_MAINTAINERS })
+
+        self._setup_devel_project(maintainer={'groups': [group_name]})
 
         req_id = self.wf.create_submit_request(SRC_PROJECT, 'blowfish').reqid
 
@@ -70,8 +121,8 @@ def test_devel_project(self):
         self.assertReview(req_id, by_user=(self.bot_user, 'accepted'))
         self.assertReview(req_id, by_group=(REVIEW_TEAM, 'new'))
 
-    def _setup_devel_project(self):
-        devel_project = self.wf.create_project(SRC_PROJECT)
+    def _setup_devel_project(self, maintainer={}):
+        devel_project = self.wf.create_project(SRC_PROJECT, maintainer=maintainer)
         devel_package = OBSLocal.Package('blowfish', project=devel_project)
 
         blowfish_spec = os.path.join(FIXTURES, 'packages', 'blowfish', 'blowfish.spec')