From c81c2c6eec85038d56e734ff9ce762c3da3c168f Mon Sep 17 00:00:00 2001 From: Albin Vass Date: Mon, 2 Nov 2020 13:28:41 +0100 Subject: [PATCH] Filter events on event connection Currently if two triggers of the same connection type need to trigger on different events it's not possible to do so since the events are never filtered on which connection they came from. For example with the following setup where gerrit-org-1 only wants to trigger on changes to 'master' and gerrit-org-2 only wants to trigger on changes to 'develop' they will instead both trigger on 'master' and 'develop'since the events are never filtered on which connection they came from. - pipeline: name: check trigger: gerrit-org-1: - event: patchset-created branch: 'master' gerrit-org-2: - event: patchset-created branch: 'develop' Change-Id: Ia0476d71dee59c8b80db7630ac7a524bce87e6f9 --- ...ilter-on-triggername-2a80d7811a5f31b5.yaml | 5 +++ .../git/common-config/zuul.yaml | 2 ++ tests/unit/test_connection.py | 31 ++++++++++++++++++- zuul/configloader.py | 11 ++++--- zuul/driver/gerrit/gerritconnection.py | 1 + zuul/driver/gerrit/gerritmodel.py | 14 ++++++--- zuul/driver/gerrit/gerrittrigger.py | 3 +- zuul/driver/git/gitconnection.py | 1 + zuul/driver/git/gitmodel.py | 8 +++-- zuul/driver/git/gittrigger.py | 3 +- zuul/driver/github/githubconnection.py | 2 ++ zuul/driver/github/githubmodel.py | 11 ++++--- zuul/driver/github/githubtrigger.py | 3 +- zuul/driver/gitlab/gitlabconnection.py | 1 + zuul/driver/gitlab/gitlabmodel.py | 8 +++-- zuul/driver/gitlab/gitlabtrigger.py | 3 +- zuul/driver/pagure/pagureconnection.py | 1 + zuul/driver/pagure/paguremodel.py | 11 +++++-- zuul/driver/pagure/paguretrigger.py | 3 +- zuul/driver/timer/timermodel.py | 5 +-- zuul/driver/timer/timertrigger.py | 5 +-- zuul/driver/zuul/__init__.py | 2 ++ zuul/driver/zuul/zuulmodel.py | 8 +++-- zuul/driver/zuul/zuultrigger.py | 3 +- zuul/model.py | 11 ++++++- zuul/trigger/__init__.py | 2 +- 26 files changed, 122 insertions(+), 36 deletions(-) create mode 100644 releasenotes/notes/filter-on-triggername-2a80d7811a5f31b5.yaml diff --git a/releasenotes/notes/filter-on-triggername-2a80d7811a5f31b5.yaml b/releasenotes/notes/filter-on-triggername-2a80d7811a5f31b5.yaml new file mode 100644 index 0000000000..936f5bff19 --- /dev/null +++ b/releasenotes/notes/filter-on-triggername-2a80d7811a5f31b5.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixed a bug where multiple connections of the same type would not filter + trigger events coming from the wrong connection. diff --git a/tests/fixtures/config/zuul-connections-multiple-gerrits/git/common-config/zuul.yaml b/tests/fixtures/config/zuul-connections-multiple-gerrits/git/common-config/zuul.yaml index 6a0865e42a..3ca313710c 100644 --- a/tests/fixtures/config/zuul-connections-multiple-gerrits/git/common-config/zuul.yaml +++ b/tests/fixtures/config/zuul-connections-multiple-gerrits/git/common-config/zuul.yaml @@ -30,8 +30,10 @@ trigger: another_gerrit: - event: patchset-created + branch: 'master' review_gerrit: - event: patchset-created + branch: 'develop' success: review_gerrit: Verified: 1 diff --git a/tests/unit/test_connection.py b/tests/unit/test_connection.py index 570fd052d8..ae3d28d6a7 100644 --- a/tests/unit/test_connection.py +++ b/tests/unit/test_connection.py @@ -493,6 +493,16 @@ class TestMultipleGerrits(ZuulTestCase): def test_multiple_project_separate_gerrits_common_pipeline(self): self.executor_server.hold_jobs_in_build = True + self.create_branch('org/project2', 'develop') + self.fake_another_gerrit.addEvent( + self.fake_another_gerrit.getFakeBranchCreatedEvent( + 'org/project2', 'develop')) + + self.fake_another_gerrit.addEvent( + self.fake_review_gerrit.getFakeBranchCreatedEvent( + 'org/project2', 'develop')) + self.waitUntilSettled() + A = self.fake_another_gerrit.addFakeChange( 'org/project2', 'master', 'A') self.fake_another_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) @@ -512,7 +522,7 @@ class TestMultipleGerrits(ZuulTestCase): self.fake_review_gerrit.change_number = 50 B = self.fake_review_gerrit.addFakeChange( - 'org/project2', 'master', 'B') + 'org/project2', 'develop', 'B') self.fake_review_gerrit.addEvent(B.getPatchsetCreatedEvent(1)) self.waitUntilSettled() @@ -528,6 +538,25 @@ class TestMultipleGerrits(ZuulTestCase): pipeline='common_check'), ]) + # NOTE(avass): This last change should not trigger any pipelines since + # common_check is configured to only run on master for another_gerrit + C = self.fake_another_gerrit.addFakeChange( + 'org/project2', 'develop', 'C') + self.fake_another_gerrit.addEvent(C.getPatchsetCreatedEvent(1)) + + self.waitUntilSettled() + + self.assertBuilds([ + dict(name='project-test2', + changes='1,1', + project='org/project2', + pipeline='common_check'), + dict(name='project-test1', + changes='51,1', + project='org/project2', + pipeline='common_check'), + ]) + self.executor_server.hold_jobs_in_build = False self.executor_server.release() self.waitUntilSettled() diff --git a/zuul/configloader.py b/zuul/configloader.py index 2f6e5628ee..cfecf1b0a6 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -1366,15 +1366,16 @@ class PipelineParser(object): manager.ref_filters.extend( source.getRejectFilters(reject_config)) - for trigger_name, trigger_config in conf.get('trigger').items(): + for connection_name, trigger_config in conf.get('trigger').items(): if self.pcontext.tenant.allowed_triggers is not None and \ - trigger_name not in self.pcontext.tenant.allowed_triggers: - raise UnknownConnection(trigger_name) + connection_name not in self.pcontext.tenant.allowed_triggers: + raise UnknownConnection(connection_name) trigger = self.pcontext.connections.getTrigger( - trigger_name, trigger_config) + connection_name, trigger_config) pipeline.triggers.append(trigger) manager.event_filters.extend( - trigger.getEventFilters(conf['trigger'][trigger_name])) + trigger.getEventFilters(connection_name, + conf['trigger'][connection_name])) # Pipelines don't get frozen return pipeline diff --git a/zuul/driver/gerrit/gerritconnection.py b/zuul/driver/gerrit/gerritconnection.py index 2b7bf5ea72..f1bbcd9ade 100644 --- a/zuul/driver/gerrit/gerritconnection.py +++ b/zuul/driver/gerrit/gerritconnection.py @@ -179,6 +179,7 @@ class GerritEventConnector(threading.Thread): time.sleep(max((timestamp + self.delay) - now, 0.0)) event = GerritTriggerEvent() event.timestamp = timestamp + event.connection_name = self.connection.connection_name # Gerrit events don't have an event id that could be used to globally # identify this event in the system so we have to generate one. diff --git a/zuul/driver/gerrit/gerritmodel.py b/zuul/driver/gerrit/gerritmodel.py index 0957612225..095ba4e9e7 100644 --- a/zuul/driver/gerrit/gerritmodel.py +++ b/zuul/driver/gerrit/gerritmodel.py @@ -295,12 +295,12 @@ class GerritApprovalFilter(object): class GerritEventFilter(EventFilter, GerritApprovalFilter): - def __init__(self, trigger, types=[], branches=[], refs=[], - event_approvals={}, comments=[], emails=[], usernames=[], - required_approvals=[], reject_approvals=[], uuid=None, - scheme=None, ignore_deletes=True): + def __init__(self, connection_name, trigger, types=[], branches=[], + refs=[], event_approvals={}, comments=[], emails=[], + usernames=[], required_approvals=[], reject_approvals=[], + uuid=None, scheme=None, ignore_deletes=True): - EventFilter.__init__(self, trigger) + EventFilter.__init__(self, connection_name, trigger) GerritApprovalFilter.__init__(self, required_approvals=required_approvals, @@ -325,6 +325,7 @@ class GerritEventFilter(EventFilter, GerritApprovalFilter): def __repr__(self): ret = '