From 164b1784c6991c3d113e12fc124e31d6548f3981 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Mon, 4 Dec 2023 14:07:14 -0800 Subject: [PATCH] Add gerrit hashtags support This adds support for the hashtags-changed trigger event as well as using hashtags as pipeline and trigger requirements. Change-Id: I1f6628d7c227d12355f651c3c822b06e2d5c5562 --- doc/source/drivers/gerrit.rst | 27 ++++++ .../gerrit-hashtags-6e28f1bdaa30f84e.yaml | 6 ++ tests/base.py | 28 ++++++ tests/fixtures/layouts/gerrit-hashtags.yaml | 40 +++++++++ tests/unit/test_gerrit.py | 29 ++++++ zuul/driver/gerrit/gerritconnection.py | 3 + zuul/driver/gerrit/gerritmodel.py | 88 ++++++++++++++++++- zuul/driver/gerrit/gerritsource.py | 7 +- zuul/driver/gerrit/gerrittrigger.py | 9 ++ 9 files changed, 235 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/gerrit-hashtags-6e28f1bdaa30f84e.yaml create mode 100644 tests/fixtures/layouts/gerrit-hashtags.yaml diff --git a/doc/source/drivers/gerrit.rst b/doc/source/drivers/gerrit.rst index 881bfa45e7..f74346785c 100644 --- a/doc/source/drivers/gerrit.rst +++ b/doc/source/drivers/gerrit.rst @@ -429,6 +429,22 @@ Trigger Configuration comments containing ``retrigger`` somewhere in the comment text are added to a change. + .. attr:: added + + This is only used for ``hashtags-changed`` events. It accepts a + regex or list of regexes that are searched for in the list of + hashtags added to the change in this event. If any of these + regexes match a portion of any of the added hashtags, the + trigger is matched. + + .. attr:: removed + + This is only used for ``hashtags-changed`` events. It accepts a + regex or list of regexes that are searched for in the list of + hashtags removed from the change in this event. If any of these + regexes match a portion of any of the removed hashtags, the + trigger is matched. + .. attr:: require-approval .. warning:: This is deprecated and will be removed in a future @@ -589,6 +605,12 @@ order to be enqueued into the pipeline. A string value that corresponds with the status of the change reported by Gerrit. + .. attr:: hashtags + + A regex or list of regexes. Each of these must match at least + one of the hashtags present on the change in order for the + change to be enqueued. + .. attr:: pipeline.reject. The `reject` attribute is the mirror of the `require` attribute. It @@ -661,6 +683,11 @@ order to be enqueued into the pipeline. A string value that corresponds with the status of the change reported by Gerrit. + .. attr:: hashtags + + A regex or list of regexes. If any of these match at least + one of the hashtags present on the change, it will be rejected. + Reference Pipelines Configuration --------------------------------- diff --git a/releasenotes/notes/gerrit-hashtags-6e28f1bdaa30f84e.yaml b/releasenotes/notes/gerrit-hashtags-6e28f1bdaa30f84e.yaml new file mode 100644 index 0000000000..1eb3dc47f2 --- /dev/null +++ b/releasenotes/notes/gerrit-hashtags-6e28f1bdaa30f84e.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + The Gerrit driver now supports the ``hashtags-changed`` event as a + trigger as well as using hashtags as trigger or pipeline + requirements. diff --git a/tests/base.py b/tests/base.py index c802752697..61b307af05 100644 --- a/tests/base.py +++ b/tests/base.py @@ -421,6 +421,7 @@ class FakeGerritChange(object): 'status': status, 'subject': subject, 'submitRecords': [], + 'hashtags': [], 'url': '%s/%s' % (self.gerrit.baseurl.rstrip('/'), number)} if topic: @@ -706,6 +707,33 @@ class FakeGerritChange(object): } return event + def getHashtagsChangedEvent(self, added=None, removed=None): + event = { + 'type': 'hashtags-changed', + 'change': {'branch': self.branch, + 'commitMessage': self.data['commitMessage'], + 'createdOn': 1689442009, + 'id': 'I254acfc54f9942394ff924a806cd87c70cec2f4d', + 'number': int(self.number), + 'owner': self.data['owner'], + 'project': self.project, + 'status': self.data['status'], + 'subject': self.subject, + 'url': 'https://hostname/3'}, + 'changeKey': {'id': 'I254acfc54f9942394ff924a806cd87c70cec2f4d'}, + 'editor': {'email': 'user@example.com', + 'name': 'User Name', + 'username': 'user'}, + 'eventCreatedOn': 1701711038, + 'project': self.project, + 'refName': self.branch, + } + if added: + event['added'] = added + if removed: + event['removed'] = removed + return event + def addApproval(self, category, value, username='reviewer_john', granted_on=None, message='', tag=None): if not granted_on: diff --git a/tests/fixtures/layouts/gerrit-hashtags.yaml b/tests/fixtures/layouts/gerrit-hashtags.yaml new file mode 100644 index 0000000000..37249efcf8 --- /dev/null +++ b/tests/fixtures/layouts/gerrit-hashtags.yaml @@ -0,0 +1,40 @@ +- pipeline: + name: check + manager: independent + trigger: + gerrit: + - event: hashtags-changed + added: check + - event: hashtags-changed + removed: nocheck + require: + gerrit: + hashtags: okay + reject: + gerrit: + hashtags: nope + success: + gerrit: + Verified: 1 + failure: + gerrit: + Verified: -1 + +- job: + name: base + parent: null + run: playbooks/base.yaml + nodeset: + nodes: + - label: ubuntu-xenial + name: controller + +- job: + name: check-job + run: playbooks/check.yaml + +- project: + name: org/project + check: + jobs: + - check-job diff --git a/tests/unit/test_gerrit.py b/tests/unit/test_gerrit.py index 5033859e9d..17e1890fa0 100644 --- a/tests/unit/test_gerrit.py +++ b/tests/unit/test_gerrit.py @@ -1223,3 +1223,32 @@ class TestGerritDriver(ZuulTestCase): self.assertEqual('tag-job', zuulvars['job']) self.assertEqual(tagsha, zuulvars['newrev']) self.assertEqual(tagsha, zuulvars['commit_id']) + + @simple_layout('layouts/gerrit-hashtags.yaml') + def test_hashtags_event(self): + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A') + A.data['hashtags'] = ['check'] + self.fake_gerrit.addEvent(A.getHashtagsChangedEvent(added=['check'])) + self.waitUntilSettled() + + # Does not meet pipeline requirement + self.assertHistory([]) + + A.data['hashtags'] = ['okay', 'check'] + self.fake_gerrit.addEvent(A.getHashtagsChangedEvent(added=['check'])) + self.waitUntilSettled() + + # This should work + self.assertHistory([ + dict(name='check-job', result='SUCCESS', changes='1,1'), + ]) + + # Matches reject + A.data['hashtags'] = ['okay', 'check', 'nope'] + self.fake_gerrit.addEvent(A.getHashtagsChangedEvent( + removed=['nocheck'])) + self.waitUntilSettled() + + self.assertHistory([ + dict(name='check-job', result='SUCCESS', changes='1,1'), + ]) diff --git a/zuul/driver/gerrit/gerritconnection.py b/zuul/driver/gerrit/gerritconnection.py index 6a447fe837..a0919f5d35 100644 --- a/zuul/driver/gerrit/gerritconnection.py +++ b/zuul/driver/gerrit/gerritconnection.py @@ -304,6 +304,8 @@ class GerritEventConnector(threading.Thread): for patchsetcomment in patchsetcomments: event.patchsetcomments.append( patchsetcomment.get('message')) + event.added = data.get('added') + event.removed = data.get('removed') refupdate = data.get('refUpdate') if refupdate: event.project_name = refupdate.get('project') @@ -338,6 +340,7 @@ class GerritEventConnector(threading.Thread): 'ref-updated': 'submitter', 'reviewer-added': 'reviewer', # Gerrit 2.5/2.6 'topic-changed': 'changer', + 'hashtags-changed': 'editor', 'vote-deleted': 'deleter', 'project-created': None, # Gerrit 2.14 'pending-check': None, # Gerrit 3.0+ diff --git a/zuul/driver/gerrit/gerritmodel.py b/zuul/driver/gerrit/gerritmodel.py index f390633cd2..42afe44861 100644 --- a/zuul/driver/gerrit/gerritmodel.py +++ b/zuul/driver/gerrit/gerritmodel.py @@ -38,6 +38,7 @@ class GerritChange(Change): self.missing_labels = set() self.submit_requirements = [] self.commit = None + self.hashtags = [] self.zuul_query_ltime = None def update(self, data, connection): @@ -60,6 +61,7 @@ class GerritChange(Change): "missing_labels": list(self.missing_labels), "submit_requirements": self.submit_requirements, "commit": self.commit, + "hashtags": self.hashtags, "zuul_query_ltime": self.zuul_query_ltime, }) return d @@ -73,6 +75,7 @@ class GerritChange(Change): self.missing_labels = set(data["missing_labels"]) self.submit_requirements = data.get("submit_requirements", []) self.commit = data.get("commit") + self.hashtags = data.get("hashtags", []) self.zuul_query_ltime = data.get("zuul_query_ltime") def updateFromSSH(self, data, connection): @@ -127,6 +130,7 @@ class GerritChange(Change): self.owner = data['owner'].get('username') self.message = data['commitMessage'] self.topic = data.get('topic') + self.hashtags = data.get('hashtags', []) self.missing_labels = set() for sr in data.get('submitRecords', []): @@ -204,6 +208,7 @@ class GerritChange(Change): self.owner = data['owner'].get('username') self.message = current_revision['commit']['message'] self.topic = data.get('topic') + self.hashtags = data.get('hashtags', []) class GerritTriggerEvent(TriggerEvent): @@ -214,6 +219,8 @@ class GerritTriggerEvent(TriggerEvent): self.uuid = None self.scheme = None self.patchsetcomments = None + self.added = None # Used by hashtags-changed event + self.removed = None # Used by hashtags-changed event self.default_branch_changed = None def toDict(self): @@ -222,6 +229,8 @@ class GerritTriggerEvent(TriggerEvent): d["uuid"] = self.uuid d["scheme"] = self.scheme d["patchsetcomments"] = self.patchsetcomments + d["added"] = self.added + d["removed"] = self.removed d["default_branch_changed"] = self.default_branch_changed return d @@ -231,6 +240,8 @@ class GerritTriggerEvent(TriggerEvent): self.uuid = d["uuid"] self.scheme = d["scheme"] self.patchsetcomments = d["patchsetcomments"] + self.added = d.get("added") + self.removed = d.get("removed") self.default_branch_changed = d.get("default_branch_changed") def __repr__(self): @@ -244,6 +255,10 @@ class GerritTriggerEvent(TriggerEvent): if self.approvals: ret += ' ' + ', '.join( ['%s:%s' % (a['type'], a['value']) for a in self.approvals]) + if self.added: + ret += f" added {self.added}" + if self.removed: + ret += f" removed {self.removed}" ret += '>' return ret @@ -262,6 +277,7 @@ class GerritEventFilter(EventFilter): def __init__(self, connection_name, trigger, types=[], branches=[], refs=[], event_approvals={}, comments=[], emails=[], usernames=[], required_approvals=[], reject_approvals=[], + added=[], removed=[], uuid=None, scheme=None, ignore_deletes=True, require=None, reject=None, parse_context=None): @@ -291,12 +307,16 @@ class GerritEventFilter(EventFilter): self._comments = [x.pattern for x in comments] self._emails = [x.pattern for x in emails] self._usernames = [x.pattern for x in usernames] + self._added = [x.pattern for x in added] + self._removed = [x.pattern for x in removed] self.types = types self.branches = branches self.refs = refs self.comments = comments self.emails = emails self.usernames = usernames + self.added = added + self.removed = removed self.event_approvals = event_approvals self.uuid = uuid self.scheme = scheme @@ -327,6 +347,10 @@ class GerritEventFilter(EventFilter): ret += ' emails: %s' % ', '.join(self._emails) if self._usernames: ret += ' usernames: %s' % ', '.join(self._usernames) + if self._added: + ret += ' added: %s' % ', '.join(self._added) + if self._removed: + ret += ' removed: %s' % ', '.join(self._removed) if self.require_filter: ret += ' require: %s' % repr(self.require_filter) if self.reject_filter: @@ -428,6 +452,34 @@ class GerritEventFilter(EventFilter): return FalseWithReason("Approvals %s do not match %s" % ( self.event_approvals, event.approvals)) + # hashtags are ORed + if self.added: + matches_token = False + event_added = event.added or [] + for action_re in self.added: + if matches_token: + break + for token in event_added: + if action_re.search(token): + matches_token = True + break + if not matches_token: + return FalseWithReason("Added %s does not match %s" % ( + self.added, event.added)) + if self.removed: + matches_token = False + event_removed = event.removed or [] + for action_re in self.removed: + if matches_token: + break + for token in event_removed: + if action_re.search(token): + matches_token = True + break + if not matches_token: + return FalseWithReason("Removed %s does not match %s" % ( + self.removed, event.removed)) + if self.require_filter: require_filter_result = self.require_filter.matches(change) if not require_filter_result: @@ -448,7 +500,8 @@ class GerritRefFilter(RefFilter): current_patchset=None, reject_current_patchset=None, wip=None, reject_wip=None, statuses=[], reject_statuses=[], - required_approvals=[], reject_approvals=[]): + required_approvals=[], reject_approvals=[], + required_hashtags=[], reject_hashtags=[]): RefFilter.__init__(self, connection_name) self._required_approvals = copy.deepcopy(required_approvals) @@ -459,6 +512,8 @@ class GerritRefFilter(RefFilter): self._reject_approvals, parse_context) self.statuses = statuses self.reject_statuses = reject_statuses + self.required_hashtags = required_hashtags + self.reject_hashtags = reject_hashtags if reject_open is not None: self.open = not reject_open @@ -475,6 +530,8 @@ class GerritRefFilter(RefFilter): @classmethod def requiresFromConfig(cls, connection_name, config, parse_context): + with parse_context.confAttr(config, 'hashtags') as attr: + hashtags = [make_regex(x, parse_context) for x in to_list(attr)] return cls( connection_name=connection_name, parse_context=parse_context, @@ -483,10 +540,13 @@ class GerritRefFilter(RefFilter): wip=config.get('wip'), statuses=to_list(config.get('status')), required_approvals=to_list(config.get('approval')), + required_hashtags=hashtags, ) @classmethod def rejectFromConfig(cls, connection_name, config, parse_context): + with parse_context.confAttr(config, 'hashtags') as attr: + hashtags = [make_regex(x, parse_context) for x in to_list(attr)] return cls( connection_name=connection_name, parse_context=parse_context, @@ -495,6 +555,7 @@ class GerritRefFilter(RefFilter): reject_wip=config.get('wip'), reject_statuses=to_list(config.get('status')), reject_approvals=to_list(config.get('approval')), + reject_hashtags=hashtags, ) def __repr__(self): @@ -517,6 +578,12 @@ class GerritRefFilter(RefFilter): if self.reject_approvals: ret += (' reject-approvals: %s' % str(self.reject_approvals)) + if self.required_hashtags: + ret += (' required-hashtags: %s' % + [x.pattern for x in self.required_hashtags]) + if self.reject_hashtags: + ret += (' reject-hashtags: %s' % + [x.pattern for x in self.reject_hashtags]) ret += '>' return ret @@ -563,6 +630,25 @@ class GerritRefFilter(RefFilter): "Reject statuses %s match %s" % ( self.reject_statuses, change.status)) + for hashtag_re in self.required_hashtags: + matches_hashtag = False + for token in change.hashtags: + if hashtag_re.search(token): + matches_hashtag = True + break + if not matches_hashtag: + return FalseWithReason( + "Required hashtags %s do not match %s" % ( + [x.pattern for x in self.required_hashtags], + change.hashtags)) + for hashtag_re in self.reject_hashtags: + for token in change.hashtags: + if hashtag_re.search(token): + return FalseWithReason( + "Reject hashtags %s match %s" % ( + [x.pattern for x in self.reject_hashtags], + change.hashtags)) + # required approvals are ANDed (reject approvals are ORed) matches_approvals_result = self.matchesApprovals(change) if not matches_approvals_result: diff --git a/zuul/driver/gerrit/gerritsource.py b/zuul/driver/gerrit/gerritsource.py index 0e3db1f5e7..562a8088f3 100644 --- a/zuul/driver/gerrit/gerritsource.py +++ b/zuul/driver/gerrit/gerritsource.py @@ -21,7 +21,10 @@ from urllib.parse import urlparse from zuul.source import BaseSource from zuul.model import Project from zuul.driver.gerrit.gerritmodel import GerritRefFilter -from zuul.driver.util import scalar_or_list +from zuul.driver.util import ( + scalar_or_list, + ZUUL_REGEX, +) from zuul.lib.dependson import find_dependency_headers from zuul.zk.change_cache import ChangeKey @@ -263,6 +266,7 @@ def getRequireSchema(): 'open': bool, 'current-patchset': bool, 'wip': bool, + 'hashtags': scalar_or_list(vs.Any(ZUUL_REGEX, str)), 'status': scalar_or_list(str)} return require @@ -272,5 +276,6 @@ def getRejectSchema(): 'open': bool, 'current-patchset': bool, 'wip': bool, + 'hashtags': scalar_or_list(vs.Any(ZUUL_REGEX, str)), 'status': scalar_or_list(str)} return reject diff --git a/zuul/driver/gerrit/gerrittrigger.py b/zuul/driver/gerrit/gerrittrigger.py index 74769e4c55..81f280542d 100644 --- a/zuul/driver/gerrit/gerrittrigger.py +++ b/zuul/driver/gerrit/gerrittrigger.py @@ -73,6 +73,10 @@ class GerritTrigger(BaseTrigger): branches = [make_regex(x, pcontext) for x in to_list(attr)] with pcontext.confAttr(trigger, 'ref') as attr: refs = [make_regex(x, pcontext) for x in to_list(attr)] + with pcontext.confAttr(trigger, 'added') as attr: + added = [make_regex(x, pcontext) for x in to_list(attr)] + with pcontext.confAttr(trigger, 'removed') as attr: + removed = [make_regex(x, pcontext) for x in to_list(attr)] ignore_deletes = trigger.get('ignore-deletes', True) @@ -101,6 +105,8 @@ class GerritTrigger(BaseTrigger): reject_approvals=to_list( trigger.get('reject-approval') ), + added=added, + removed=removed, uuid=trigger.get('uuid'), scheme=trigger.get('scheme'), ignore_deletes=ignore_deletes, @@ -133,6 +139,7 @@ def getSchema(): 'ref-updated', 'pending-check', 'vote-deleted', + 'hashtags-changed', 'wip-state-changed')), 'uuid': str, 'scheme': str, @@ -148,6 +155,8 @@ def getSchema(): 'approval': scalar_or_list(variable_dict), 'require-approval': scalar_or_list(approval), 'reject-approval': scalar_or_list(approval), + 'added': scalar_or_list(v.Any(ZUUL_REGEX, str)), + 'removed': scalar_or_list(v.Any(ZUUL_REGEX, str)), 'require': gerritsource.getRequireSchema(), 'reject': gerritsource.getRejectSchema(), }