Support merged as requirement in github driver
Currently when creating a post pipeline with the github driver we can only use the push event. This has the drawback that it ignores protected branches and thus also works speculatively on any branch if you want to have a generic post pipeline. This imposes problems with secrets. So instead using the push event we want to trigger on pull_request closed events. However this currently doesn't work as we cannot distinguish between merged and abandoned pull requests. Adding a merged requirement nicely solves that problem. Change-Id: I46670c6aa036976c430a6034a6b1da0e23fa9f92
This commit is contained in:
parent
1b747ca65f
commit
735190f2ec
@ -409,6 +409,11 @@ enqueued into the pipeline.
|
||||
A boolean value (``true`` or ``false``) that indicates whether
|
||||
the change must be open or closed in order to be enqueued.
|
||||
|
||||
.. attr:: merged
|
||||
|
||||
A boolean value (``true`` or ``false``) that indicates whether
|
||||
the change must be merged or not in order to be enqueued.
|
||||
|
||||
.. attr:: current-patchset
|
||||
|
||||
A boolean value (``true`` or ``false``) that indicates whether
|
||||
|
5
releasenotes/notes/require-merged-70784e1e45cac08e.yaml
Normal file
5
releasenotes/notes/require-merged-70784e1e45cac08e.yaml
Normal file
@ -0,0 +1,5 @@
|
||||
---
|
||||
features:
|
||||
- |
|
||||
The GitHub driver now supports the :attr:`pipeline.require.<github source>.merged`
|
||||
requirement.
|
23
tests/fixtures/layouts/basic-github.yaml
vendored
23
tests/fixtures/layouts/basic-github.yaml
vendored
@ -17,6 +17,22 @@
|
||||
failure:
|
||||
github: {}
|
||||
|
||||
- pipeline:
|
||||
name: post
|
||||
manager: independent
|
||||
require:
|
||||
github:
|
||||
merged: true
|
||||
trigger:
|
||||
github:
|
||||
- event: pull_request
|
||||
action:
|
||||
- closed
|
||||
success:
|
||||
github: {}
|
||||
failure:
|
||||
github: {}
|
||||
|
||||
- job:
|
||||
name: base
|
||||
parent: null
|
||||
@ -30,9 +46,16 @@
|
||||
name: project-test2
|
||||
run: playbooks/project-test2.yaml
|
||||
|
||||
- job:
|
||||
name: project-post
|
||||
run: playbooks/project-post.yaml
|
||||
|
||||
- project:
|
||||
name: org/project
|
||||
check:
|
||||
jobs:
|
||||
- project-test1
|
||||
- project-test2
|
||||
post:
|
||||
jobs:
|
||||
- project-post
|
||||
|
@ -72,6 +72,22 @@ class TestGithubDriver(ZuulTestCase):
|
||||
|
||||
self.assertEqual(2, len(self.history))
|
||||
|
||||
# now emit closed event without merging
|
||||
self.fake_github.emitEvent(A.getPullRequestClosedEvent())
|
||||
self.waitUntilSettled()
|
||||
|
||||
# nothing should have happened due to the merged requirement
|
||||
self.assertEqual(2, len(self.history))
|
||||
|
||||
# now merge the PR and emit the event again
|
||||
A.setMerged('merged')
|
||||
|
||||
self.fake_github.emitEvent(A.getPullRequestClosedEvent())
|
||||
self.waitUntilSettled()
|
||||
|
||||
# post job must be run
|
||||
self.assertEqual(3, len(self.history))
|
||||
|
||||
@simple_layout('layouts/files-github.yaml', driver='github')
|
||||
def test_pull_matched_file_event(self):
|
||||
A = self.fake_github.openFakePullRequest(
|
||||
|
@ -321,9 +321,10 @@ class GithubEventFilter(EventFilter, GithubCommonFilter):
|
||||
|
||||
class GithubRefFilter(RefFilter, GithubCommonFilter):
|
||||
def __init__(self, connection_name, statuses=[], required_reviews=[],
|
||||
reject_reviews=[], open=None, current_patchset=None,
|
||||
reject_open=None, reject_current_patchset=None,
|
||||
labels=[], reject_labels=[], reject_statuses=[]):
|
||||
reject_reviews=[], open=None, merged=None,
|
||||
current_patchset=None, reject_open=None, reject_merged=None,
|
||||
reject_current_patchset=None, labels=[], reject_labels=[],
|
||||
reject_statuses=[]):
|
||||
RefFilter.__init__(self, connection_name)
|
||||
|
||||
GithubCommonFilter.__init__(self, required_reviews=required_reviews,
|
||||
@ -335,6 +336,10 @@ class GithubRefFilter(RefFilter, GithubCommonFilter):
|
||||
self.open = not reject_open
|
||||
else:
|
||||
self.open = open
|
||||
if reject_merged is not None:
|
||||
self.merged = not reject_merged
|
||||
else:
|
||||
self.merged = merged
|
||||
if reject_current_patchset is not None:
|
||||
self.current_patchset = not reject_current_patchset
|
||||
else:
|
||||
@ -358,6 +363,8 @@ class GithubRefFilter(RefFilter, GithubCommonFilter):
|
||||
str(self.reject_reviews))
|
||||
if self.open:
|
||||
ret += ' open: %s' % self.open
|
||||
if self.merged:
|
||||
ret += ' merged: %s' % self.merged
|
||||
if self.current_patchset:
|
||||
ret += ' current-patchset: %s' % self.current_patchset
|
||||
if self.labels:
|
||||
@ -382,6 +389,15 @@ class GithubRefFilter(RefFilter, GithubCommonFilter):
|
||||
else:
|
||||
return False
|
||||
|
||||
if self.merged is not None:
|
||||
# if a "change" has no number, it's not a change, but a push
|
||||
# and cannot possibly pass this test.
|
||||
if hasattr(change, 'number'):
|
||||
if self.merged != change.is_merged:
|
||||
return False
|
||||
else:
|
||||
return False
|
||||
|
||||
if self.current_patchset is not None:
|
||||
# if a "change" has no number, it's not a change, but a push
|
||||
# and cannot possibly pass this test.
|
||||
|
@ -132,6 +132,7 @@ class GithubSource(BaseSource):
|
||||
statuses=to_list(config.get('status')),
|
||||
required_reviews=to_list(config.get('review')),
|
||||
open=config.get('open'),
|
||||
merged=config.get('merged'),
|
||||
current_patchset=config.get('current-patchset'),
|
||||
labels=to_list(config.get('label')),
|
||||
)
|
||||
@ -144,6 +145,7 @@ class GithubSource(BaseSource):
|
||||
reject_labels=to_list(config.get('label')),
|
||||
reject_statuses=to_list(config.get('status')),
|
||||
reject_open=config.get('open'),
|
||||
reject_merged=config.get('merged'),
|
||||
reject_current_patchset=config.get('current-patchset'),
|
||||
)
|
||||
return [f]
|
||||
@ -165,6 +167,7 @@ def getRequireSchema():
|
||||
require = {'status': scalar_or_list(str),
|
||||
'review': scalar_or_list(review),
|
||||
'open': bool,
|
||||
'merged': bool,
|
||||
'current-patchset': bool,
|
||||
'label': scalar_or_list(str)}
|
||||
return require
|
||||
@ -174,6 +177,7 @@ def getRejectSchema():
|
||||
reject = {'status': scalar_or_list(str),
|
||||
'review': scalar_or_list(review),
|
||||
'open': bool,
|
||||
'merged': bool,
|
||||
'current-patchset': bool,
|
||||
'label': scalar_or_list(str)}
|
||||
return reject
|
||||
|
Loading…
x
Reference in New Issue
Block a user