Minimum approval config

README.md

@@ -193,19 +193,24 @@ ssh-keygen -t ed25519 -C marge-bot@invalid -f marge-bot-ssh-key -P ''
 Add the public key (`marge-bot-ssh-key.pub`) to the user's `SSH Keys` in GitLab
 and keep the private one handy.
 
-### Per project configuration
+### Merge Approvals on Gitlab CE
 
 On GitLab enterprise the [merge request approvals](https://docs.gitlab.com/ee/user/project/merge_requests/merge_request_approvals.html)
 provide the information how many approvals from whom are needed for
-a merge request.  On GitlLab CE this is done via a configuration file
-called `.marge-bot.yml`.  Currently Marge uses the config file from master
-as config for all merge request.
+a merge request. 
 
-The `.marge-bot.yml` config currently only supports `approver_count`:
-```yaml
+On GitlLab CE this is done via the merge request's award API (thumbup action) and the 
+[CODEOWNERS](https://docs.gitlab.com/ee/user/project/code_owners.html) file.
+Appropriate approvers will be determined from that file and the changes in the MR and minimum approvers can be set 
+by adding a commented line anywhere in that file that contains:
 
-approver_count: 3  # number of "thumbs up" needed, defaults to 1
 ```
+# MARGEBOT_MINIMUM_APPROVERS = 2
+```
+
+Adjust for your needs. If no CODEOWNERS file exist or no matched owners the approval flow will be disabled. If no minimum
+approvers is set, all matched users from CODEOWNERS will be required to approve.
+
 
 ### Running marge-bot in docker (what we do)
 

marge/approvals.py

@@ -1,5 +1,6 @@
 import logging as log
 import fnmatch
+import re
 import shlex
 
 from . import gitlab
@@ -26,24 +27,29 @@ def refetch_info(self):
     def get_approvers_ce(self):
         """get approvers status using thumbs on merge request
         """
-        owner_globs = self.get_codeowners_ce()
-        if not owner_globs:
+        owner_file = self.get_codeowners_ce()
+        if not owner_file['owners']:
             log.info("No CODEOWNERS file in master, continuing without approvers flow")
             return dict(self._info, approvals_left=0, approved_by=[], codeowners=[])
 
-        codeowners = self.determine_responsible_owners(owner_globs, self.get_changes_ce())
+        code_owners = self.determine_responsible_owners(owner_file['owners'], self.get_changes_ce())
 
-        if not codeowners:
+        if not code_owners:
             log.info("No matched code owners, continuing without approvers flow")
             return dict(self._info, approvals_left=0, approved_by=[], codeowners=[])
 
         awards = self.get_awards_ce()
 
-        up_votes = [e for e in awards if e['name'] == 'thumbsup' and e['user']['username'] in codeowners]
-        approver_count = len(codeowners)
-        approvals_left = max(approver_count - len(up_votes), 0)
+        up_votes = [e for e in awards if e['name'] == 'thumbsup' and e['user']['username'] in code_owners]
 
-        return dict(self._info, approvals_left=approvals_left, approved_by=up_votes, codeowners=codeowners)
+        approvals_required = len(code_owners)
+
+        if owner_file['approvals_required'] > 0:
+            approvals_required = owner_file['approvals_required']
+
+        approvals_left = max(approvals_required - len(up_votes), 0)
+
+        return dict(self._info, approvals_left=approvals_left, approved_by=up_votes, codeowners=code_owners)
 
     def determine_responsible_owners(self, owners_glob, changes):
         owners = set([])
@@ -77,11 +83,18 @@ def get_awards_ce(self):
     def get_codeowners_ce(self):
         config_file = self._api.repo_file_get(self.project_id, "CODEOWNERS", "master")
         owner_globs = {}
+        required = 0
+        required_regex = re.compile('.*MARGEBOT_MINIMUM_APPROVERS *= *([0-9]+)')
 
         if config_file is None:
             return owner_globs
 
         for line in config_file['content'].splitlines():
+            if 'MARGEBOT_' in line:
+                match = required_regex.match(line)
+                if match:
+                    required = int(match.group(1))
+
             if line != "" and not line.startswith(' ') and not line.startswith('#'):
                 elements = shlex.split(line)
                 glob = elements.pop(0)
@@ -90,7 +103,7 @@ def get_codeowners_ce(self):
                 for user in elements:
                     owner_globs[glob].add(user.strip('@'))
 
-        return owner_globs
+        return {"approvals_required": required, "owners": owner_globs}
 
     @property
     def iid(self):

tests/test_approvals.py

@@ -11,6 +11,7 @@
 
 CODEOWNERS = {
     "content": """
+# MARGEBOT_MINIMUM_APPROVERS = 2
 # This is an example code owners file, lines starting with a `#` will
 # be ignored.
 
@@ -24,6 +25,7 @@
 # pylint: disable=anomalous-backslash-in-string
 CODEOWNERS_FULL = {
     "content": """
+# MARGEBOT_MINIMUM_APPROVERS=3
 # This is an example code owners file, lines starting with a `#` will
 # be ignored.
 
@@ -49,10 +51,6 @@
 # owner for the LICENSE file
 LICENSE @legal this_does_not_match [email protected]
 
-# Group names can be used to match groups and nested groups to specify
-# them as owners for a file
-README @group @group/with-nested/subgroup
-
 # Ending a path in a `/` will specify the code owners for every file
 # nested in that directory, on any level
 /docs/ @all-docs
@@ -222,7 +220,7 @@ def test_fetch_from_merge_request_ce_compat(self):
             'id': 74,
             'iid': 6,
             'project_id': 1234,
-            'approvals_left': 2,
+            'approvals_left': 1,
             'approved_by': [AWARDS[1]],
             'codeowners': {'default-codeowner', 'ebert', 'test-user1'},
         }
@@ -280,17 +278,20 @@ def test_approvals_ce_get_codeowners_full(self):
         approvals = Approvals(api, {'id': 74, 'iid': 6, 'project_id': 1234})
 
         assert approvals.get_codeowners_ce() == {
-            '#file_with_pound.rb': {'owner-file-with-pound'},
-            '*': {'default-codeowner'},
-            '*.rb': {'ruby-owner'},
-            '/config/': {'config-owner'},
-            '/docs/': {'all-docs'},
-            '/docs/*': {'root-docs'},
-            'CODEOWNERS': {'owners', 'multiple', 'code'},
-            'LICENSE': {'this_does_not_match', '[email protected]', 'legal'},
-            'README': {'group', 'group/with-nested/subgroup'},
-            'lib/': {'lib-owner'},
-            'path with spaces/': {'space-owner'}
+            'approvals_required': 3,
+            'owners': {
+                '#file_with_pound.rb': {'owner-file-with-pound'},
+                '*': {'default-codeowner'},
+                '*.rb': {'ruby-owner'},
+                '/config/': {'config-owner'},
+                '/docs/': {'all-docs'},
+                '/docs/*': {'root-docs'},
+                'CODEOWNERS': {'owners', 'multiple', 'code'},
+                'LICENSE': {'this_does_not_match', '[email protected]', 'legal'},
+                'README': {'group', 'group/with-nested/subgroup'},
+                'lib/': {'lib-owner'},
+                'path with spaces/': {'space-owner'}
+            }
         }
 
     def test_approvals_ce_get_codeowners_wildcard(self):
@@ -301,7 +302,8 @@ def test_approvals_ce_get_codeowners_wildcard(self):
         approvals = Approvals(api, {'id': 74, 'iid': 6, 'project_id': 1234})
 
         assert approvals.get_codeowners_ce() == {
-            '*': set(['default-codeowner', 'test-user1', 'ebert']), 'unmatched/*': {'test5'}
+            'approvals_required': 2,
+            'owners': {'*': {'default-codeowner', 'test-user1', 'ebert'}, 'unmatched/*': {'test5'}}
         }
 
     @patch('marge.approvals.Approvals.get_awards_ce', Mock(return_value=AWARDS))
@@ -316,5 +318,5 @@ def test_approvals_ce(self):
 
         result = approvals.get_approvers_ce()
 
-        assert result['approvals_left'] == 2
+        assert result['approvals_left'] == 1
         assert len(result['approved_by']) == 1