zuul
/
zuul
Code Issues Proposed changes

pagure: Improve CI status flag handling 

- Ensure to consider only CI status flags set by the related token
  account. This is to avoid being triggered by another CI system voting
  on the same Pull Request.
- Re-use the same status CI slot.
- Allow to set the display name of the CI system in from of
  the CI status flag.

Change-Id: I12fc761e34413f4efd7c2ad60ed04bf12d5932a3
This commit is contained in:
Fabien Boucher 2020-04-07 11:35:46 +02:00
parent 33fef37bc5
commit 5c04c782b9
5 changed files with 115 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
doc/source/reference/drivers/pagure.rst
View File

@ -81,6 +81,12 @@ The supported options in ``zuul.conf`` connections are:
Path to the Pagure Git repositories. Used to clone.
.. attr:: app_name
:default: Zuul
Display name that will appear as the application name in front
of each CI status flag.
.. attr:: source_whitelist
:default: ''

36
tests/base.py
View File

@ -1232,19 +1232,33 @@ class FakePagurePullRequest(object):
return self._getPullRequestEvent(
'pull-request.tag.added', pull_data_field='pull_request')
def getPullRequestStatusSetEvent(self, status):
def getPullRequestStatusSetEvent(self, status, username="zuul"):
self.addFlag(
status, "https://url", "Build %s" % status)
status, "https://url", "Build %s" % status, username)
return self._getPullRequestEvent('pull-request.flag.added')
def addFlag(self, status, url, comment, username="Pingou"):
def insertFlag(self, flag):
to_pop = None
for i, _flag in enumerate(self.flags):
if _flag['uid'] == flag['uid']:
to_pop = i
if to_pop is not None:
self.flags.pop(to_pop)
self.flags.insert(0, flag)
def addFlag(self, status, url, comment, username="zuul"):
flag_uid = "%s-%s-%s" % (username, self.number, self.project)
flag = {
"username": username,
"username": "Zuul CI",
"user": {
"name": username
},
"uid": flag_uid[:32],
"comment": comment,
"status": status,
"url": url
}
self.flags.insert(0, flag)
self.insertFlag(flag)
self._updateTimeStamp()
def editInitialComment(self, initial_comment):
@ -1402,13 +1416,18 @@ class FakePagureAPIClient(pagureconnection.PagureAPIClient):
pr.is_merged = True
return {}, 200, "", "POST"
match = re.match(r'.+/api/0/-/whoami$', url)
if match:
return {"username": "zuul"}, 200, "", "POST"
if not params:
return self.gen_error("POST")
match = re.match(r'.+/api/0/(.+)/pull-request/(\d+)/flag$', url)
if match:
pr = self._get_pr(match)
pr.flags.insert(0, params)
params['user'] = {"name": "zuul"}
pr.insertFlag(params)
match = re.match(r'.+/api/0/(.+)/pull-request/(\d+)/comment$', url)
if match:
@ -1435,9 +1454,12 @@ class FakePagureConnection(pagureconnection.PagureConnection):
self.cloneurl = self.upstream_root
def get_project_api_client(self, project):
return FakePagureAPIClient(
client = FakePagureAPIClient(
self.baseurl, None, project,
pull_requests_db=self.pull_requests)
if not self.username:
self.set_my_username(client)
return client
def get_project_webhook_token(self, project):
return 'fake_webhook_token-%s' % project

20
tests/fixtures/layouts/requirements-pagure.yaml vendored
View File

@ -66,6 +66,18 @@
pagure:
status: 'success'
- pipeline:
name: require-flag
manager: independent
require:
pagure:
status: success
trigger:
pagure:
- event: pg_pull_request
action: status
status: success
- pipeline:
name: require-trigger-pg-closed-merged
precedence: high
@ -125,4 +137,10 @@
name: org/project6
require-trigger-pg-closed-merged:
jobs:
- project-test
- project-test
- project:
name: org/project7
require-flag:
jobs:
- project-test

28
tests/unit/test_pagure_driver.py
View File

@ -61,9 +61,8 @@ class TestPagureDriver(ZuulTestCase):
self.assertThat(
A.comments[1]['comment'],
MatchesRegex(r'.*\[project-test2 \]\(.*\).*', re.DOTALL))
self.assertEqual(2, len(A.flags))
self.assertEqual(1, len(A.flags))
self.assertEqual('success', A.flags[0]['status'])
self.assertEqual('pending', A.flags[1]['status'])
@simple_layout('layouts/basic-pagure.yaml', driver='pagure')
def test_pull_request_updated(self):
@ -492,6 +491,31 @@ class TestPagureDriver(ZuulTestCase):
self.waitUntilSettled()
self.assertEqual(1, len(self.history))
@simple_layout('layouts/requirements-pagure.yaml', driver='pagure')
def test_flag_require(self):
A = self.fake_pagure.openFakePullRequest(
'org/project7', 'master', 'A')
# CI status from other CIs must not be handled
self.fake_pagure.emitEvent(
A.getPullRequestStatusSetEvent("success", username="notzuul"))
self.waitUntilSettled()
self.assertEqual(0, len(self.history))
self.assertEqual(1, len(A.flags))
self.fake_pagure.emitEvent(
A.getPullRequestStatusSetEvent("failure"))
self.waitUntilSettled()
self.assertEqual(0, len(self.history))
self.assertEqual(2, len(A.flags))
self.fake_pagure.emitEvent(
A.getPullRequestStatusSetEvent("success"))
self.waitUntilSettled()
self.assertEqual(1, len(self.history))
self.assertEqual(2, len(A.flags))
@simple_layout('layouts/requirements-pagure.yaml', driver='pagure')
def test_pull_request_closed(self):

46
zuul/driver/pagure/pagureconnection.py
View File

@ -206,6 +206,7 @@ class PagureEventConnector(threading.Thread):
'pull-request.closed': self._event_pull_request_closed,
'pull-request.new': self._event_pull_request,
'pull-request.flag.added': self._event_flag_added,
'pull-request.flag.updated': self._event_flag_added,
'git.receive': self._event_ref_updated,
'git.branch.creation': self._event_ref_created,
'git.branch.deletion': self._event_ref_deleted,
@ -438,6 +439,12 @@ class PagureAPIClient():
ret.status_code, ret.text))
return ret.json(), ret.status_code, ret.url, 'POST'
def whoami(self):
path = '-/whoami'
resp = self.post(self.base_url + path)
self._manage_error(*resp)
return resp[0]['username']
def get_project_branches(self):
path = '%s/git/branches' % self.project
resp = self.get(self.base_url + path)
@ -456,23 +463,29 @@ class PagureAPIClient():
self._manage_error(*resp)
return resp[0]
def get_pr_flags(self, number, last=False):
def get_pr_flags(self, number, owner, last=False):
path = '%s/pull-request/%s/flag' % (self.project, number)
resp = self.get(self.base_url + path)
self._manage_error(*resp)
data = resp[0]
owned_flags = [
flag for flag in data['flags']
if flag['user']['name'] == owner]
if last:
if data['flags']:
return data['flags'][0]
if owned_flags:
return owned_flags[0]
else:
return {}
else:
return data['flags']
return owned_flags
def set_pr_flag(self, number, status, url, description):
def set_pr_flag(
self, number, status, url, description, app_name, username):
flag_uid = "%s-%s-%s" % (username, number, self.project)
params = {
"username": "Zuul",
"username": app_name,
"comment": "Jobs result is %s" % status,
"uid": flag_uid[:32],
"status": status,
"url": url}
path = '%s/pull-request/%s/flag' % (self.project, number)
@ -505,7 +518,6 @@ class PagureAPIClient():
class PagureConnection(BaseConnection):
driver_name = 'pagure'
log = logging.getLogger("zuul.PagureConnection")
payload_path = 'payload'
def __init__(self, driver, connection_name, connection_config):
super(PagureConnection, self).__init__(
@ -518,6 +530,9 @@ class PagureConnection(BaseConnection):
'canonical_hostname', self.server)
self.git_ssh_key = self.connection_config.get('sshkey')
self.api_token = self.connection_config.get('api_token')
self.app_name = self.connection_config.get(
'app_name', 'Zuul')
self.username = None
self.baseurl = self.connection_config.get(
'baseurl', 'https://%s' % self.server).rstrip('/')
self.cloneurl = self.connection_config.get(
@ -562,9 +577,17 @@ class PagureConnection(BaseConnection):
def eventDone(self):
self.event_queue.task_done()
def set_my_username(self, client):
self.log.debug("Fetching my username ...")
self.username = client.whoami()
self.log.debug("My username is %s" % self.username)
def get_project_api_client(self, project):
self.log.debug("Building project %s api_client" % project)
return PagureAPIClient(self.baseurl, self.api_token, project)
client = PagureAPIClient(self.baseurl, self.api_token, project)
if not self.username:
self.set_my_username(client)
return client
def get_project_webhook_token(self, project, force_refresh=False):
token = self.webhook_tokens.get(project)
@ -696,7 +719,7 @@ class PagureConnection(BaseConnection):
def _hasRequiredStatusChecks(self, change):
pagure = self.get_project_api_client(change.project.name)
flag = pagure.get_pr_flags(change.number, last=True)
flag = pagure.get_pr_flags(change.number, self.username, last=True)
return True if flag.get('status', '') == 'success' else False
def canMerge(self, change, allow_needs, event=None):
@ -804,14 +827,15 @@ class PagureConnection(BaseConnection):
def setCommitStatus(self, project, number, state, url='',
description='', context=''):
pagure = self.get_project_api_client(project)
pagure.set_pr_flag(number, state, url, description)
pagure.set_pr_flag(
number, state, url, description, self.app_name, self.username)
self.log.info("Set pull-request CI flag status : %s" % description)
# Wait for 1 second as flag timestamp is by second
time.sleep(1)
def getCommitStatus(self, project, number):
pagure = self.get_project_api_client(project)
flag = pagure.get_pr_flags(number, last=True)
flag = pagure.get_pr_flags(number, self.username, last=True)
self.log.info(
"Got pull-request CI status for PR %s on %s status: %s" % (
number, project, flag.get('status')))