Skip to content

Commit 31f0f4f

Browse files
committed
Set temporary branch for external MRs
If it's to be reliable, CI should be run on the target project, even when the MR comes from a fork. This commit introduces the temp_branch argument for specifying the temporary branch name that will be used for running the CI in the target project.
1 parent be6061e commit 31f0f4f

File tree

6 files changed

+120
-21
lines changed

6 files changed

+120
-21
lines changed

marge/app.py

+7
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,12 @@ def regexp(str_regex):
176176
default='.*',
177177
help='Only process MRs whose target branches match the given regular expression.\n',
178178
)
179+
parser.add_argument(
180+
'--temp-branch',
181+
type=str,
182+
default="",
183+
help='Temporary branch name for external merge requests (disabled if empty).\n',
184+
)
179185
parser.add_argument(
180186
'--debug',
181187
action='store_true',
@@ -253,6 +259,7 @@ def main(args=None):
253259
embargo=options.embargo,
254260
ci_timeout=options.ci_timeout,
255261
use_merge_strategy=options.use_merge_strategy,
262+
temp_branch=options.temp_branch,
256263
),
257264
batch=options.batch,
258265
)

marge/job.py

+54-9
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
from collections import namedtuple
55
from datetime import datetime, timedelta
66

7-
from . import git
7+
from . import git, gitlab
8+
from .commit import Commit
89
from .interval import IntervalUnion
910
from .project import Project
1011
from .user import User
@@ -114,19 +115,23 @@ def add_trailers(self, merge_request):
114115
return sha
115116

116117
def get_mr_ci_status(self, merge_request, commit_sha=None):
118+
temp_branch = self.opts.temp_branch
117119
if commit_sha is None:
118120
commit_sha = merge_request.sha
119-
pipelines = Pipeline.pipelines_by_branch(
120-
merge_request.source_project_id,
121-
merge_request.source_branch,
122-
self._api,
123-
)
121+
if temp_branch and merge_request.source_project_id != self._project.id:
122+
pid = self._project.id
123+
ref = temp_branch
124+
self.update_temp_branch(merge_request, commit_sha)
125+
else:
126+
pid = merge_request.source_project_id
127+
ref = merge_request.source_branch
128+
pipelines = Pipeline.pipelines_by_branch(pid, ref, self._api)
124129
current_pipeline = next(iter(pipelines), None)
125130

126131
if current_pipeline and current_pipeline.sha == commit_sha:
127132
ci_status = current_pipeline.status
128133
else:
129-
log.warning('No pipeline listed for %s on branch %s', commit_sha, merge_request.source_branch)
134+
log.warning('No pipeline listed for %s on branch %s', commit_sha, ref)
130135
ci_status = None
131136

132137
return ci_status
@@ -196,7 +201,7 @@ def fetch_source_project(self, merge_request):
196201
remote = 'source'
197202
remote_url = source_project.ssh_url_to_repo
198203
self._repo.fetch(
199-
remote=remote,
204+
remote_name=remote,
200205
remote_url=remote_url,
201206
)
202207
return source_project, remote_url, remote
@@ -281,6 +286,43 @@ def update_from_target_branch_and_push(
281286
else:
282287
assert source_repo_url is not None
283288

289+
def update_temp_branch(self, merge_request, commit_sha):
290+
api = self._api
291+
project_id = self._project.id
292+
temp_branch = self.opts.temp_branch
293+
waiting_time_in_secs = 30
294+
295+
try:
296+
sha_branch = Commit.last_on_branch(project_id, temp_branch, api).id
297+
except gitlab.NotFound:
298+
sha_branch = None
299+
if sha_branch != commit_sha:
300+
log.info('Setting up %s in target project', temp_branch)
301+
self.delete_temp_branch(merge_request.source_project_id)
302+
self._project.create_branch(temp_branch, commit_sha, api)
303+
self._project.protect_branch(temp_branch, api)
304+
merge_request.comment(
305+
('The temporary branch **{branch}** was updated to [{sha:.8}](../commit/{sha}) ' +
306+
'and local pipelines will be used.').format(
307+
branch=temp_branch, sha=commit_sha
308+
)
309+
)
310+
311+
time.sleep(waiting_time_in_secs)
312+
313+
def delete_temp_branch(self, source_project_id):
314+
temp_branch = self.opts.temp_branch
315+
316+
if temp_branch and source_project_id != self._project.id:
317+
try:
318+
self._project.unprotect_branch(temp_branch, self._api)
319+
except gitlab.ApiError:
320+
pass
321+
try:
322+
self._project.delete_branch(temp_branch, self._api)
323+
except gitlab.ApiError:
324+
pass
325+
284326

285327
def _get_reviewer_names_and_emails(approvals, api):
286328
"""Return a list ['A. Prover <a.prover@example.com', ...]` for `merge_request.`"""
@@ -298,6 +340,7 @@ def _get_reviewer_names_and_emails(approvals, api):
298340
'embargo',
299341
'ci_timeout',
300342
'use_merge_strategy',
343+
'temp_branch',
301344
]
302345

303346

@@ -312,7 +355,8 @@ def requests_commit_tagging(self):
312355
def default(
313356
cls, *,
314357
add_tested=False, add_part_of=False, add_reviewers=False, reapprove=False,
315-
approval_timeout=None, embargo=None, ci_timeout=None, use_merge_strategy=False
358+
approval_timeout=None, embargo=None, ci_timeout=None, use_merge_strategy=False,
359+
temp_branch=""
316360
):
317361
approval_timeout = approval_timeout or timedelta(seconds=0)
318362
embargo = embargo or IntervalUnion.empty()
@@ -326,6 +370,7 @@ def default(
326370
embargo=embargo,
327371
ci_timeout=ci_timeout,
328372
use_merge_strategy=use_merge_strategy,
373+
temp_branch=temp_branch,
329374
)
330375

331376

marge/merge_request.py

+20-9
Original file line numberDiff line numberDiff line change
@@ -116,15 +116,26 @@ def comment(self, message):
116116

117117
return self._api.call(POST(notes_url, {'body': message}))
118118

119-
def accept(self, remove_branch=False, sha=None):
120-
return self._api.call(PUT(
121-
'/projects/{0.project_id}/merge_requests/{0.iid}/merge'.format(self),
122-
dict(
123-
should_remove_source_branch=remove_branch,
124-
merge_when_pipeline_succeeds=True,
125-
sha=sha or self.sha, # if provided, ensures what is merged is what we want (or fails)
126-
),
127-
))
119+
def accept(self, remove_branch=False, sha=None, trust_pipeline=True, project=None):
120+
reset = False
121+
if not trust_pipeline:
122+
# Temporarily remove flag, because we want to merge based on another pipeline
123+
if project.only_allow_merge_if_pipeline_succeeds:
124+
reset = True
125+
project.set_only_allow_merge_if_pipeline_succeeds(False, self._api)
126+
try:
127+
result = self._api.call(PUT(
128+
'/projects/{0.project_id}/merge_requests/{0.iid}/merge'.format(self),
129+
dict(
130+
should_remove_source_branch=remove_branch,
131+
merge_when_pipeline_succeeds=trust_pipeline,
132+
sha=sha or self.sha, # if provided, ensures what is merged is what we want (or fails)
133+
),
134+
))
135+
finally:
136+
if reset:
137+
project.set_only_allow_merge_if_pipeline_succeeds(True, self._api)
138+
return result
128139

129140
def close(self):
130141
return self._api.call(PUT(

marge/project.py

+30-1
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
import logging as log
22
from enum import Enum, unique
33
from functools import partial
4+
from requests.utils import quote
45

56
from . import gitlab
67

78

8-
GET = gitlab.GET
9+
GET, PUT, POST, DELETE = gitlab.GET, gitlab.PUT, gitlab.POST, gitlab.DELETE
910

1011

1112
class Project(gitlab.Resource):
@@ -72,6 +73,34 @@ def access_level(self):
7273
assert effective_access is not None, "GitLab failed to provide user permissions on project"
7374
return AccessLevel(effective_access['access_level'])
7475

76+
def set_only_allow_merge_if_pipeline_succeeds(self, value, api):
77+
return api.call(PUT(
78+
'/projects/%s' % self.info['id'],
79+
{'only_allow_merge_if_pipeline_succeeds': value}
80+
))
81+
82+
def protect_branch(self, branch, api):
83+
return api.call(POST(
84+
'/projects/%s/protected_branches' % self.info['id'],
85+
{'name': branch}
86+
))
87+
88+
def unprotect_branch(self, branch, api):
89+
return api.call(DELETE('/projects/{project_id}/protected_branches/{name}'.format(
90+
project_id=self.info['id'], name=quote(branch, safe='')
91+
)))
92+
93+
def create_branch(self, branch, sha, api):
94+
return api.call(POST(
95+
'/projects/%s/repository/branches' % self.info['id'],
96+
{'branch': branch, 'ref': sha}
97+
))
98+
99+
def delete_branch(self, branch, api):
100+
return api.call(DELETE('/projects/{project_id}/repository/branches/{name}'.format(
101+
project_id=self.info['id'], name=quote(branch, safe='')
102+
)))
103+
75104

76105
@unique
77106
class AccessLevel(Enum):

marge/single_merge_job.py

+8-2
Original file line numberDiff line numberDiff line change
@@ -65,14 +65,18 @@ def update_merge_request_and_accept(self, approvals):
6565

6666
self.maybe_reapprove(merge_request, approvals)
6767

68-
if source_project.only_allow_merge_if_pipeline_succeeds:
68+
trust = True
69+
if self._project.only_allow_merge_if_pipeline_succeeds:
6970
self.wait_for_ci_to_pass(merge_request, actual_sha)
7071
time.sleep(2)
72+
if self.opts.temp_branch and source_project is not self._project:
73+
trust = False
7174

7275
self.ensure_mergeable_mr(merge_request)
7376

7477
try:
75-
merge_request.accept(remove_branch=True, sha=actual_sha)
78+
merge_request.accept(remove_branch=True, sha=actual_sha, trust_pipeline=trust,
79+
project=self._project)
7680
except gitlab.NotAcceptable as err:
7781
new_target_sha = Commit.last_on_branch(self._project.id, merge_request.target_branch, api).id
7882
# target_branch has moved under us since we updated, just try again
@@ -126,6 +130,8 @@ def update_merge_request_and_accept(self, approvals):
126130
log.exception('Unanticipated ApiError from GitLab on merge attempt')
127131
raise CannotMerge('had some issue with GitLab, check my logs...')
128132
else:
133+
# temp_branch can be removed before because it was only used for CI, not for merging
134+
self.delete_temp_branch(source_project.id)
129135
self.wait_for_branch_to_be_merged()
130136
updated_into_up_to_date_target_branch = True
131137

tests/test_job.py

+1
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@ def test_default(self):
176176
embargo=marge.interval.IntervalUnion.empty(),
177177
ci_timeout=timedelta(minutes=15),
178178
use_merge_strategy=False,
179+
temp_branch='',
179180
)
180181

181182
def test_default_ci_time(self):

0 commit comments

Comments
 (0)