From ae42bbe73576d696de710370701c3a1c138d981b Mon Sep 17 00:00:00 2001 From: Fabien Boucher Date: Mon, 9 Sep 2019 17:32:01 +0200 Subject: [PATCH] Pagure - handle Pull Request tags (labels) metadata This change implements event handling for pull-request.tags.added. Tags can be used as trigger event filter or required metadata. Change-Id: I128bbef34245932e3bbee1f848ad1c484d3ccae3 --- doc/source/admin/drivers/pagure.rst | 19 +++++++- .../pipelines/pagure-reference-pipelines.yaml | 4 ++ tests/base.py | 25 ++++++++-- .../fixtures/layouts/requirements-pagure.yaml | 40 ++++++++++++++++ tests/unit/test_pagure_driver.py | 46 +++++++++++++++++++ zuul/driver/pagure/pagureconnection.py | 34 +++++++++++--- zuul/driver/pagure/paguremodel.py | 24 +++++++++- zuul/driver/pagure/paguresource.py | 4 +- zuul/driver/pagure/paguretrigger.py | 2 + 9 files changed, 185 insertions(+), 13 deletions(-) diff --git a/doc/source/admin/drivers/pagure.rst b/doc/source/admin/drivers/pagure.rst index 540940c67b..889ba8d378 100644 --- a/doc/source/admin/drivers/pagure.rst +++ b/doc/source/admin/drivers/pagure.rst @@ -109,6 +109,10 @@ the following options. Status set on pull request. + .. value:: tagged + + Tag metadata set on pull request. + A :value:`pipeline.trigger..event.pg_pull_request_review` event will have associated action(s) to trigger from. The supported actions are: @@ -123,7 +127,7 @@ the following options. .. attr:: comment - This is only used for ``pg_pull_request`` ``comment`` actions. It + This is only used for ``pg_pull_request`` and ``comment`` actions. It accepts a list of regexes that are searched for in the comment string. If any of these regexes matches a portion of the comment string the trigger is matched. ``comment: retrigger`` will @@ -137,6 +141,12 @@ the following options. the status, the status context, and the status itself in the format of ``status``. For example, ``success`` or ``failure``. + .. attr:: tag + + This is used for ``pg_pull_request`` and ``tagged`` actions. It + accepts a list of strings and if one of them is part of the + event tags metadata then the trigger is matched. + .. attr:: ref This is only used for ``pg_push`` events. This field is treated as @@ -199,6 +209,8 @@ configuration such as the following: score: 1 merged: false status: success + tags: + - gateit This indicates that changes originating from the Pagure connection must have a score of *1*, a CI status *success* and not being already merged. @@ -226,6 +238,11 @@ must have a score of *1*, a CI status *success* and not being already merged. A boolean value (``true`` or ``false``) that indicates whether the Pull Request must be open or closed in order to be enqueued. + .. attr:: tags + + if present, the list of tags a Pull Request must have. + + Reference pipelines configuration --------------------------------- diff --git a/doc/source/admin/examples/pipelines/pagure-reference-pipelines.yaml b/doc/source/admin/examples/pipelines/pagure-reference-pipelines.yaml index 6d23e2b0ac..e55ebf20a3 100644 --- a/doc/source/admin/examples/pipelines/pagure-reference-pipelines.yaml +++ b/doc/source/admin/examples/pipelines/pagure-reference-pipelines.yaml @@ -35,6 +35,7 @@ pagure.io: score: 1 merged: False + tags: gateit status: success sqlreporter: trigger: @@ -44,6 +45,9 @@ status: success - event: pg_pull_request_review action: thumbsup + - event: pg_pull_request + action: tagged + tag: gateit start: pagure.io: status: 'pending' diff --git a/tests/base.py b/tests/base.py index 01b8da1acf..875bb82e60 100644 --- a/tests/base.py +++ b/tests/base.py @@ -827,6 +827,7 @@ class FakePagurePullRequest(object): self.comments = [] self.flags = [] self.files = {} + self.tags = [] self.cached_merge_status = '' self.threshold_reached = False self.commit_stop = None @@ -841,16 +842,17 @@ class FakePagurePullRequest(object): self._addCommitInPR(files=files) self._updateTimeStamp() - def _getPullRequestEvent(self, action): + def _getPullRequestEvent(self, action, pull_data_field='pullrequest'): name = 'pg_pull_request' data = { 'msg': { - 'pullrequest': { + pull_data_field: { 'branch': self.branch, 'comments': self.comments, 'commit_start': self.commit_start, 'commit_stop': self.commit_stop, 'date_created': '0', + 'tags': self.tags, 'initial_comment': self.initial_comment, 'id': self.number, 'project': { @@ -867,6 +869,8 @@ class FakePagurePullRequest(object): } if action == 'pull-request.flag.added': data['msg']['flag'] = self.flags[0] + if action == 'pull-request.tag.added': + data['msg']['tags'] = self.tags return (name, data) def getPullRequestOpenedEvent(self): @@ -888,6 +892,20 @@ class FakePagurePullRequest(object): self._updateTimeStamp() return self._getPullRequestEvent('pull-request.initial_comment.edited') + def getPullRequestTagAddedEvent(self, tags, reset=True): + if reset: + self.tags = [] + _tags = set(self.tags) + _tags.update(set(tags)) + self.tags = list(_tags) + self.addComment( + "**Metadata Update from @pingou**:\n- " + + "Pull-request tagged with: %s" % ', '.join(tags), + True) + self._updateTimeStamp() + return self._getPullRequestEvent( + 'pull-request.tag.added', pull_data_field='pull_request') + def getPullRequestStatusSetEvent(self, status): self.addFlag( status, "https://url", "Build %s" % status) @@ -1014,7 +1032,8 @@ class FakePagureAPIClient(pagureconnection.PagureAPIClient): 'comments': pr.comments, 'commit_stop': pr.commit_stop, 'threshold_reached': pr.threshold_reached, - 'cached_merge_status': pr.cached_merge_status + 'cached_merge_status': pr.cached_merge_status, + 'tags': pr.tags, }, 200, "", "GET" match = re.match(r'.+/api/0/(.+)/pull-request/(\d+)/flag$', url) diff --git a/tests/fixtures/layouts/requirements-pagure.yaml b/tests/fixtures/layouts/requirements-pagure.yaml index 47ca6c1355..36f785f30f 100644 --- a/tests/fixtures/layouts/requirements-pagure.yaml +++ b/tests/fixtures/layouts/requirements-pagure.yaml @@ -38,6 +38,34 @@ pagure: status: 'success' +- pipeline: + name: trigger-tag + manager: independent + trigger: + pagure: + - event: pg_pull_request + action: tagged + tag: + - gateit + - mergeit + success: + pagure: + status: 'success' + +- pipeline: + name: require-tag + manager: independent + require: + pagure: + tags: gateit + trigger: + pagure: + - event: pg_pull_request + action: changed + success: + pagure: + status: 'success' + - job: name: base parent: null @@ -64,3 +92,15 @@ trigger-flag: jobs: - project-test + +- project: + name: org/project4 + trigger-tag: + jobs: + - project-test + +- project: + name: org/project5 + require-tag: + jobs: + - project-test \ No newline at end of file diff --git a/tests/unit/test_pagure_driver.py b/tests/unit/test_pagure_driver.py index 4643b86205..3f023beea0 100644 --- a/tests/unit/test_pagure_driver.py +++ b/tests/unit/test_pagure_driver.py @@ -360,6 +360,52 @@ class TestPagureDriver(ZuulTestCase): self.waitUntilSettled() self.assertEqual(1, len(self.history)) + @simple_layout('layouts/requirements-pagure.yaml', driver='pagure') + def test_tag_trigger(self): + + A = self.fake_pagure.openFakePullRequest( + 'org/project4', 'master', 'A') + + self.fake_pagure.emitEvent( + A.getPullRequestTagAddedEvent(["lambda"])) + self.waitUntilSettled() + self.assertEqual(0, len(self.history)) + + self.fake_pagure.emitEvent( + A.getPullRequestTagAddedEvent(["gateit", "lambda"])) + self.waitUntilSettled() + self.assertEqual(1, len(self.history)) + + self.fake_pagure.emitEvent( + A.getPullRequestTagAddedEvent(["mergeit"])) + self.waitUntilSettled() + self.assertEqual(2, len(self.history)) + + @simple_layout('layouts/requirements-pagure.yaml', driver='pagure') + def test_tag_require(self): + + A = self.fake_pagure.openFakePullRequest( + 'org/project5', 'master', 'A') + + self.fake_pagure.emitEvent(A.getPullRequestUpdatedEvent()) + self.waitUntilSettled() + self.assertEqual(0, len(self.history)) + + A.tags = ["lambda"] + self.fake_pagure.emitEvent(A.getPullRequestUpdatedEvent()) + self.waitUntilSettled() + self.assertEqual(0, len(self.history)) + + A.tags = ["lambda", "gateit"] + self.fake_pagure.emitEvent(A.getPullRequestUpdatedEvent()) + self.waitUntilSettled() + self.assertEqual(1, len(self.history)) + + A.tags = [] + self.fake_pagure.emitEvent(A.getPullRequestUpdatedEvent()) + self.waitUntilSettled() + self.assertEqual(1, len(self.history)) + @simple_layout('layouts/merging-pagure.yaml', driver='pagure') def test_merge_action_in_independent(self): diff --git a/zuul/driver/pagure/pagureconnection.py b/zuul/driver/pagure/pagureconnection.py index 8d91df2235..313a900bf8 100644 --- a/zuul/driver/pagure/pagureconnection.py +++ b/zuul/driver/pagure/pagureconnection.py @@ -18,6 +18,7 @@ import hashlib import queue import threading import time +import re import json import requests import cherrypy @@ -194,13 +195,17 @@ class PagureEventConnector(threading.Thread): self.daemon = True self.connection = connection self._stopped = False + self.metadata_notif = re.compile( + r"^\*\*Metadata Update", re.MULTILINE) self.event_handler_mapping = { 'pull-request.comment.added': self._event_issue_comment, 'pull-request.new': self._event_pull_request, 'pull-request.flag.added': self._event_flag_added, 'git.receive': self._event_ref_updated, 'pull-request.initial_comment.edited': - self._event_issue_initial_comment + self._event_issue_initial_comment, + 'pull-request.tag.added': + self._event_pull_request_tags_changed } def stop(self): @@ -244,16 +249,19 @@ class PagureEventConnector(threading.Thread): self.connection.logEvent(event) self.connection.sched.addEvent(event) - def _event_base(self, body): + def _event_base(self, body, pull_data_field='pullrequest'): event = PagureTriggerEvent() - if 'pullrequest' in body['msg']: - data = body['msg']['pullrequest'] + + if pull_data_field in body['msg']: + data = body['msg'][pull_data_field] + data['tags'] = body['msg'].get('tags', []) data['flag'] = body['msg'].get('flag') event.title = data.get('title') event.project_name = data.get('project', {}).get('fullname') event.change_number = data.get('id') event.updated_at = data.get('date_created') event.branch = data.get('branch') + event.tags = data.get('tags', []) event.change_url = self.connection.getPullUrl(event.project_name, event.change_number) event.ref = "refs/pull/%s/head" % event.change_number @@ -271,12 +279,21 @@ class PagureEventConnector(threading.Thread): event.action = 'changed' return event + def _event_pull_request_tags_changed(self, body): + """ Handles pull request metadata change """ + # pull-request.tag.added/removed use pull_request in payload body + event, _ = self._event_base(body, pull_data_field='pull_request') + event.action = 'tagged' + return event + def _event_issue_comment(self, body): """ Handles pull request comments """ # https://fedora-fedmsg.readthedocs.io/en/latest/topics.html#pagure-pull-request-comment-added event, data = self._event_base(body) last_comment = data.get('comments', [])[-1] - if last_comment.get('notification') is True: + if (last_comment.get('notification') is True and + not self.metadata_notif.match( + last_comment.get('comment', ''))): # An updated PR (new commits) triggers the comment.added # event. A message is added by pagure on the PR but notification # is set to true. @@ -556,7 +573,8 @@ class PagureConnection(BaseConnection): self.connectors = {} self.source = driver.getSource(self) self.event_queue = queue.Queue() - + self.metadata_notif = re.compile( + r"^\*\*Metadata Update", re.MULTILINE) self.sched = None def onLoad(self): @@ -793,6 +811,9 @@ class PagureConnection(BaseConnection): for comment in pr.get('comments', []): # PR updated are reported as comment but with the notification flag if comment['notification']: + # Ignore metadata update such as assignee and tags + if self.metadata_notif.match(comment.get('comment', '')): + continue date = int(comment['date_created']) if date > last_pr_code_updated: last_pr_code_updated = date @@ -822,6 +843,7 @@ class PagureConnection(BaseConnection): change.patchset = change.pr.get('commit_stop') change.files = change.pr.get('files') change.title = change.pr.get('title') + change.tags = change.pr.get('tags') change.open = change.pr.get('status') == 'Open' change.is_merged = change.pr.get('status') == 'Merged' change.status = self.getStatus(change.project, change.number) diff --git a/zuul/driver/pagure/paguremodel.py b/zuul/driver/pagure/paguremodel.py index 5036fc8a74..f62e7f7874 100644 --- a/zuul/driver/pagure/paguremodel.py +++ b/zuul/driver/pagure/paguremodel.py @@ -27,6 +27,7 @@ class PullRequest(Change): self.title = None self.score = 0 self.files = [] + self.tags = [] def __repr__(self): r = ['