Use CODEOWNERS and changes to determine who can approve

At the moment all matches from codeowners has to be available to approve

marge/approvals.py

@@ -1,5 +1,7 @@
 import yaml
 import logging as log
+import shlex
+import fnmatch
 
 from . import gitlab
 
@@ -20,28 +22,65 @@ def refetch_info(self):
         if gitlab_version.is_ee:
             self._info = self._api.call(GET(approver_url))
         else:
-            self.get_approvers_ce()
+            self._info = self.get_approvers_ce()
 
     def get_approvers_ce(self):
         """get approvers status using thumbs on merge request
         """
 
-        config_file = self._api.repo_file_get(self.project_id, ".marge-bot.yml", "master")
-        if config_file is None:
-            log.info('Project id %s missing .marge-bot.yaml', self.project_id)
-            config = {}
-        else:
-            config = yaml.load(config_file["content"])
+        codeowners = self.determine_responsible_owners(self.get_codeowners_ce(), self.get_changes_ce())
+        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)
+        return dict(self._info, approvals_left=approvals_left, approved_by=up_votes)
+
+    def determine_responsible_owners(self, owners_glob, changes):
+        owners = set([])
+
+        # Always add global users
+        if '*' in owners_glob:
+            owners.update(owners_glob['*'])
+
+        if 'changes' not in changes:
+            return owners
+
+        for change in changes['changes']:
+            for glob, users in owners_glob.items():
+                if 'new_path' in change and fnmatch.fnmatch(change['new_path'], glob):
+                    owners.update(users)
 
+        return owners
 
+    def get_changes_ce(self):
+        changes_url = '/projects/{0.project_id}/merge_requests/{0.iid}/changes'
+        changes_url = changes_url.format(self)
+
+        return self._api.call(GET(changes_url))
+
+    def get_awards_ce(self):
         emoji_url = '/projects/{0.project_id}/merge_requests/{0.iid}/award_emoji'
         emoji_url = emoji_url.format(self)
-        emoji = self._api.call(GET(emoji_url))
+        return self._api.call(GET(emoji_url))
 
-        up_votes = [e  for e in emoji if e['name'] == 'thumbsup']
-        approver_count = config.get('approver_count', 1)
-        approvals_left = max(approver_count - len(up_votes), 0)
-        self._info = dict(self._info, approvals_left=approvals_left, approved_by=up_votes)
+    def get_codeowners_ce(self):
+        config_file = self._api.repo_file_get(self.project_id, "CODEOWNERS", "master")
+        owner_globs = {}
+
+        if config_file is None:
+            return owner_globs
+
+        for line in config_file.splitlines():
+            if line != "" and not line.startswith(' ') and not line.startswith('#'):
+                elements = shlex.split(line) # line.split()
+                glob = elements.pop(0)
+                owner_globs.setdefault(glob, set([]))
+
+                for user in elements:
+                    owner_globs[glob].add(user.strip('@'))
+
+        return owner_globs
 
     @property
     def iid(self):

tests/test_approvals.py

@@ -9,6 +9,124 @@
 # testing this here is more convenient
 from marge.job import CannotMerge, _get_reviewer_names_and_emails
 
+CODEOWNERS = """
+# This is an example code owners file, lines starting with a `#` will
+# be ignored.
+
+* @default-codeowner @test-user1
+* @test-user1 @ebert
+
+unmatched/* @test5
+"""
+
+CODEOWNERS_FULL = """
+# This is an example code owners file, lines starting with a `#` will
+# be ignored.
+
+# app/ @commented-rule
+
+# We can specify a default match using wildcards:
+* @default-codeowner
+
+# Rules defined later in the file take precedence over the rules
+# defined before.
+# This will match all files for which the file name ends in `.rb`
+*.rb @ruby-owner
+
+# Files with a `#` can still be accesssed by escaping the pound sign
+\#file_with_pound.rb @owner-file-with-pound
+
+# Multiple codeowners can be specified, separated by spaces or tabs
+CODEOWNERS @multiple @code @owners
+
+# Both usernames or email addresses can be used to match
+# users. Everything else will be ignored. For example this will
+# specify `@legal` and a user with email `[email protected]` as the
+# 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
+
+# Ending a path in `/*` will specify code owners for every file in
+# that directory, but not nested deeper. This will match
+# `docs/index.md` but not `docs/projects/index.md`
+/docs/* @root-docs
+
+# This will make a `lib` directory nested anywhere in the repository
+# match
+lib/ @lib-owner
+
+# This will only match a `config` directory in the root of the
+# repository
+/config/ @config-owner
+
+# If the path contains spaces, these need to be escaped like this:
+path\ with\ spaces/ @space-owner
+"""
+
+AWARDS = [
+    {
+        "id": 1,
+        "name": "thumbsdown",
+        "user": {
+            "name": "Test User 1",
+            "username": "test-user1",
+            "id": 3,
+            "state": "active",
+        },
+        "created_at": "2020-04-24T13:16:23.614Z",
+        "updated_at": "2020-04-24T13:16:23.614Z",
+        "awardable_id": 11,
+        "awardable_type": "MergeRequest"
+    },
+    {
+        "id": 2,
+        "name": "thumbsup",
+        "user": {
+            "name": "Roger Ebert",
+            "username": "ebert",
+            "id": 2,
+            "state": "active",
+        },
+        "created_at": "2020-04-24T13:16:23.614Z",
+        "updated_at": "2020-04-24T13:16:23.614Z",
+        "awardable_id": 12,
+        "awardable_type": "MergeRequest"
+    }
+]
+
+CHANGES = {
+    "changes": [
+        {
+            "old_path": "README",
+            "new_path": "README",
+            "a_mode": "100644",
+            "b_mode": "100644",
+            "new_file": False,
+            "renamed_file": False,
+            "deleted_file": False,
+            "diff": "",
+        },
+        {
+            "old_path": "main.go",
+            "new_path": "main.go",
+            "a_mode": "100644",
+            "b_mode": "100644",
+            "new_file": False,
+            "renamed_file": False,
+            "deleted_file": False,
+            "diff": ""
+        }
+    ]
+}
+
+
 INFO = {
     "id": 5,
     "iid": 6,
@@ -83,17 +201,20 @@ def test_fetch_from_merge_request(self):
         ))
         assert approvals.info == INFO
 
+    @patch('marge.approvals.Approvals.get_awards_ce', Mock(return_value=AWARDS))
+    @patch('marge.approvals.Approvals.get_changes_ce', Mock(return_value=CHANGES))
     def test_fetch_from_merge_request_ce_compat(self):
         api = self.api
         api.version = Mock(return_value=Version.parse('9.2.3'))
         api.call = Mock()
+        api.repo_file_get = Mock(return_value=CODEOWNERS)
 
         merge_request = MergeRequest(api, {'id': 74, 'iid': 6, 'project_id': 1234})
         approvals = merge_request.fetch_approvals()
 
         api.call.assert_not_called()
         assert approvals.info == {
-            'id': 74, 'iid': 6, 'project_id': 1234, 'approvals_left': 0, 'approved_by': [],
+            'id': 74, 'iid': 6, 'project_id': 1234, 'approvals_left': 2, 'approved_by': [AWARDS[1]],
         }
 
     def test_properties(self):
@@ -140,3 +261,48 @@ def test_approvals_succeeds_with_independent_author(self, user_fetch_by_id):
             'Administrator <root@localhost>',
             'Roger Ebert <[email protected]>',
         ]
+
+    def test_approvals_ce_get_codeowners_full(self):
+        api = self.api
+        api.version = Mock(return_value=Version.parse('9.2.3'))
+        api.repo_file_get = Mock(return_value=CODEOWNERS_FULL)
+
+        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'}
+        }
+
+    def test_approvals_ce_get_codeowners_wildcard(self):
+        api = self.api
+        api.version = Mock(return_value=Version.parse('9.2.3'))
+        api.repo_file_get = Mock(return_value=CODEOWNERS)
+
+        approvals = Approvals(api, {'id': 74, 'iid': 6, 'project_id': 1234})
+
+        assert approvals.get_codeowners_ce() == {'*': set(['default-codeowner', 'test-user1', 'ebert']), 'unmatched/*': {'test5'}}
+
+    @patch('marge.approvals.Approvals.get_awards_ce', Mock(return_value=AWARDS))
+    @patch('marge.approvals.Approvals.get_changes_ce', Mock(return_value=CHANGES))
+    def test_approvals_ce(self):
+        api = self.api
+        api.version = Mock(return_value=Version.parse('9.2.3'))
+        api.repo_file_get = Mock(return_value=CODEOWNERS)
+
+        merge_request = MergeRequest(api, {'id': 74, 'iid': 6, 'project_id': 1234})
+        approvals = merge_request.fetch_approvals()
+
+        result = approvals.get_approvers_ce()
+
+        assert result['approvals_left'] == 2
+        assert len(result['approved_by']) == 1