From 9f74059a23ec6f82783109e1ca8f2ffb17d2a996 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bonicoli Date: Mon, 7 Sep 2020 13:16:57 +0200 Subject: [PATCH] [gitlab] approvals: fix error with community edition 'approvals_left' key is not present when 'Required Merge Request Approvals' feature isn't available. With enterprise edition, 'approved' key can not be used since the related value is always true when 'approvals_required' is equal to zero. The exception was: Traceback (most recent call last): File "zuul/driver/gitlab/gitlabconnection.py", line 240, in run self._handleEvent() File "zuul/driver/gitlab/gitlabconnection.py", line 230, in _handleEvent event=event) File "driver/gitlab/gitlabconnection.py", line 510, in _getChange self._updateChange(change, event) File "driver/gitlab/gitlabconnection.py", line 521, in _updateChange change.project.name, change.number, event=event) File "zuul/driver/gitlab/gitlabconnection.py", line 560, in getMR mr['approved'] = mr_approval_status['approvals_left'] == 0 KeyError: 'approvals_left' Change-Id: I07303ae3309937046b69afe18e9e183853fd448f --- tests/base.py | 18 +++++++++++++++--- tests/unit/test_gitlab_driver.py | 23 +++++++++++++++++++++++ zuul/driver/gitlab/gitlabconnection.py | 7 ++++++- 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/tests/base.py b/tests/base.py index 8cbe7a6c6c..38f1b1d4a0 100644 --- a/tests/base.py +++ b/tests/base.py @@ -1628,6 +1628,12 @@ class FakeGitlabConnection(gitlabconnection.GitlabConnection): } return (name, data) + @contextmanager + def enable_community_edition(self): + self.gl_client.community_edition = True + yield + self.gl_client.community_edition = False + class FakeGitlabAPIClient(gitlabconnection.GitlabAPIClient): log = logging.getLogger("zuul.test.FakeGitlabAPIClient") @@ -1635,6 +1641,7 @@ class FakeGitlabAPIClient(gitlabconnection.GitlabAPIClient): def __init__(self, baseurl, api_token, merge_requests_db={}): super(FakeGitlabAPIClient, self).__init__(baseurl, api_token) self.merge_requests = merge_requests_db + self.community_edition = False def gen_error(self, verb): return { @@ -1676,9 +1683,14 @@ class FakeGitlabAPIClient(gitlabconnection.GitlabAPIClient): r'.+/projects/(.+)/merge_requests/(\d+)/approvals$', url) if match: mr = self._get_mr(match) - return { - 'approvals_left': 0 if mr.approved else 1, - }, 200, "", "GET" + if not self.community_edition: + return { + 'approvals_left': 0 if mr.approved else 1, + }, 200, "", "GET" + else: + return { + 'approved': mr.approved, + }, 200, "", "GET" def post(self, url, params=None, zuul_event_id=None): diff --git a/tests/unit/test_gitlab_driver.py b/tests/unit/test_gitlab_driver.py index 11edaf0c43..009df0a7f1 100644 --- a/tests/unit/test_gitlab_driver.py +++ b/tests/unit/test_gitlab_driver.py @@ -559,6 +559,29 @@ class TestGitlabDriver(ZuulTestCase): self.waitUntilSettled() self.assertEqual(1, len(self.history)) + @simple_layout('layouts/requirements-gitlab.yaml', driver='gitlab') + def test_approval_require_community_edition(self): + + with self.fake_gitlab.enable_community_edition(): + A = self.fake_gitlab.openFakeMergeRequest( + 'org/project2', 'master', 'A') + + self.fake_gitlab.emitEvent(A.getMergeRequestOpenedEvent()) + self.waitUntilSettled() + self.assertEqual(0, len(self.history)) + + A.approved = True + + self.fake_gitlab.emitEvent(A.getMergeRequestUpdatedEvent()) + self.waitUntilSettled() + self.assertEqual(1, len(self.history)) + + A.approved = False + + self.fake_gitlab.emitEvent(A.getMergeRequestUpdatedEvent()) + self.waitUntilSettled() + self.assertEqual(1, len(self.history)) + @simple_layout('layouts/requirements-gitlab.yaml', driver='gitlab') def test_label_require(self): diff --git a/zuul/driver/gitlab/gitlabconnection.py b/zuul/driver/gitlab/gitlabconnection.py index 2798d84eea..4a1cecafb8 100644 --- a/zuul/driver/gitlab/gitlabconnection.py +++ b/zuul/driver/gitlab/gitlabconnection.py @@ -557,7 +557,12 @@ class GitlabConnection(BaseConnection): mr_approval_status = self.gl_client.get_mr_approvals_status( project_name, number, zuul_event_id=event) log.info('Got MR approval status %s#%s', project_name, number) - mr['approved'] = mr_approval_status['approvals_left'] == 0 + if 'approvals_left' in mr_approval_status: + # 'approvals_left' is not present when 'Required Merge Request + # Approvals' feature isn't available + mr['approved'] = mr_approval_status['approvals_left'] == 0 + else: + mr['approved'] = mr_approval_status['approved'] return mr def commentMR(self, project_name, number, message, event=None):