Implement reject filter in GitLab driver

The pipeline configuration in Zuul allows specifying
what is required in order to enqueue changes, as well
as what are conditions to reject an item from the queue.
However, the rejection filtering was not implemented
in the GitLab driver so far. This commit introduces
the missing mechanism, based on the existing implementation
of the GitLab ref filter.

The `reject.<gitlab source>.labels` follows the logic
of `hashtag` attribute in Gerrit driver – if any specified
label is present in the Merge Request, it will be rejected.

Change-Id: I36cc5e0c13b9564a43875c733b0589c81ca94f5d

doc/source/drivers/gitlab.rst

@@ -274,24 +274,33 @@ is taken from the pipeline.
 Requirements Configuration
 --------------------------
 
-As described in :attr:`pipeline.require` pipelines may specify that items meet
-certain conditions in order to be enqueued into the pipeline.  These conditions
-vary according to the source of the project in question.
+As described in :attr:`pipeline.require` and :attr:`pipeline.reject`,
+pipelines may specify that items meet certain conditions in order to be
+enqueued into the pipeline.  These conditions vary according to the source
+of the project in question.  To supply requirements for changes
+from a GitLab source named ``my-gitlab``, create a configuration
+such as the following::
 
 .. code-block:: yaml
 
    pipeline:
      require:
-       gitlab:
+       my-gitlab:
          open: true
+     reject:
+       my-gitlab:
+         labels:
+           - do-not-merge
 
-This indicates that changes originating from the GitLab connection must be
-in the *opened* state (not merged yet).
+This indicates that changes originating from the GitLab connection must be in
+the *opened* state (not merged yet) and must not contain `do-not-merge` label.
 
 .. attr:: pipeline.require.<gitlab source>
 
    The dictionary passed to the GitLab pipeline `require` attribute
-   supports the following attributes:
+   is used to specify Merge Requests which should be enqueued into
+   a pipeline.  If all of the defined conditions are met, the Merge
+   Request will be enqueued.  It supports the following attributes:
 
    .. attr:: open
 
@@ -312,6 +321,34 @@ in the *opened* state (not merged yet).
 
       A list of labels a Merge Request must have in order to be enqueued.
 
+.. attr:: pipeline.reject.<gitlab source>
+
+   The `reject` attribute is the mirror of the `require` attribute and
+   is used to specify Merge Requests which should not be enqueued into
+   a pipeline.  If any of the defined conditions is met, the Merge Request
+   will be rejected.  It accepts a dictionary under the connection name
+   and with the following attributes:
+
+   .. attr:: open
+
+      A boolean value (``true`` or ``false``) that indicates whether
+      the Merge Request must be open or not in order to be rejected.
+
+   .. attr:: merged
+
+      A boolean value (``true`` or ``false``) that indicates whether
+      the Merge Request must be merged or not in order to be rejected.
+
+   .. attr:: approved
+
+      A boolean value (``true`` or ``false``) that indicates whether
+      the Merge Request must be approved or not in order to be rejected.
+
+   .. attr:: labels
+
+      A list of labels a Merge Request must not have.
+      If any of these is present in the Merge Request, it will be rejected.
+
 
 Reference pipelines configuration
 ---------------------------------

doc/source/examples/pipelines/gitlab-reference-pipelines.yaml

@@ -30,6 +30,10 @@
         open: true
         labels:
           - gateit
+    reject:
+      gitlab.com:
+        labels:
+          - do-not-merge
     trigger:
       gitlab.com:
         - event: gl_merge_request

releasenotes/notes/gitlab-reject-pipeline-23e6b856b0efbae7.yaml

@@ -0,0 +1,9 @@
+---
+features:
+  - |
+    Implemented support for the `reject` attribute in the GitLab driver.
+    It allows specifying Merge Requests which should not be enqueued into
+    a pipeline. The supported conditions cover the same Merge Requests'
+    parameters as available in the existing `require` attribute – filtering
+    whether the Merge Request must (not) be open, merged, approved or contain
+    some labels.

tests/fixtures/layouts/requirements-gitlab.yaml

@@ -18,6 +18,26 @@
       gitlab:
         comment: true
 
+- pipeline:
+    name: reject-state-check
+    manager: independent
+    reject:
+      gitlab:
+        open: false
+        merged: true
+    trigger:
+      gitlab:
+        - event: gl_merge_request
+          action: comment
+          comment: (?i)^\s*recheck\s*$
+        - event: gl_merge_request
+          action:
+            - opened
+            - changed
+    success:
+      gitlab:
+        comment: true
+
 - pipeline:
     name: approval-check
     manager: independent
@@ -37,6 +57,29 @@
       gitlab:
         comment: true
 
+- pipeline:
+    name: reject-unapproved-check
+    manager: independent
+    require:
+      gitlab:
+        open: true
+        merged: false
+    reject:
+      gitlab:
+        approved: false
+    trigger:
+      gitlab:
+        - event: gl_merge_request
+          action: comment
+          comment: (?i)^\s*recheck\s*$
+        - event: gl_merge_request
+          action:
+            - opened
+            - changed
+    success:
+      gitlab:
+        comment: true
+
 - pipeline:
     name: label-check
     manager: independent
@@ -58,6 +101,59 @@
       gitlab:
         comment: true
 
+- pipeline:
+    name: reject-label-check
+    manager: independent
+    require:
+      gitlab:
+        labels:
+          - gateit
+          - verified
+    reject:
+      gitlab:
+        labels:
+          - do-not-merge
+          - do-not-test
+    trigger:
+      gitlab:
+        - event: gl_merge_request
+          action: comment
+          comment: (?i)^\s*recheck\s*$
+        - event: gl_merge_request
+          action:
+            - opened
+            - changed
+    success:
+      gitlab:
+        comment: true
+
+- pipeline:
+    name: require-reject-comprehensive-check
+    manager: independent
+    require:
+      gitlab:
+        open: true
+        merged: false
+        labels:
+          - gateit
+    reject:
+      gitlab:
+        open: false
+        labels:
+          - do-not-merge
+    trigger:
+      gitlab:
+        - event: gl_merge_request
+          action: comment
+          comment: (?i)^\s*recheck\s*$
+        - event: gl_merge_request
+          action:
+            - opened
+            - changed
+    success:
+      gitlab:
+        comment: true
+
 - job:
     name: base
     parent: null
@@ -92,3 +188,43 @@
     label-check:
       jobs:
         - project3-test
+
+- job:
+    name: project4-test
+    run: playbooks/project-test.yaml
+
+- project:
+    name: org/project4
+    reject-state-check:
+      jobs:
+        - project4-test
+
+- job:
+    name: project5-test
+    run: playbooks/project-test.yaml
+
+- project:
+    name: org/project5
+    reject-unapproved-check:
+      jobs:
+        - project5-test
+
+- job:
+    name: project6-test
+    run: playbooks/project-test.yaml
+
+- project:
+    name: org/project6
+    reject-label-check:
+      jobs:
+        - project6-test
+
+- job:
+    name: project7-test
+    run: playbooks/project-test.yaml
+
+- project:
+    name: org/project7
+    require-reject-comprehensive-check:
+      jobs:
+        - project7-test

tests/unit/test_gitlab_driver.py

@@ -706,6 +706,173 @@ class TestGitlabDriver(ZuulTestCase):
         self.waitUntilSettled()
         self.assertEqual(1, len(self.history))
 
+    @simple_layout('layouts/requirements-gitlab.yaml', driver='gitlab')
+    def test_state_reject(self):
+
+        fake_mr = self.fake_gitlab.openFakeMergeRequest(project='org/project4',
+                                                        branch='master',
+                                                        title='Example MR')
+
+        # The job should be triggered for new change
+        self.fake_gitlab.emitEvent(fake_mr.getMergeRequestOpenedEvent())
+        self.waitUntilSettled()
+        self.assertEqual(1, len(self.history))
+
+        # A recheck should not trigger the job for closed change
+        fake_mr.closeMergeRequest()
+        self.fake_gitlab.emitEvent(
+            fake_mr.getMergeRequestCommentedEvent('recheck')
+        )
+        self.waitUntilSettled()
+        self.assertEqual(1, len(self.history))
+
+        # The job should be triggered after change was reopened
+        fake_mr.reopenMergeRequest()
+        self.fake_gitlab.emitEvent(fake_mr.getMergeRequestOpenedEvent())
+        self.waitUntilSettled()
+        self.assertEqual(2, len(self.history))
+
+        # A recheck should not trigger the job after the change was merged
+        fake_mr.mergeMergeRequest()
+        self.fake_gitlab.emitEvent(
+            fake_mr.getMergeRequestCommentedEvent('recheck')
+        )
+        self.waitUntilSettled()
+        self.assertEqual(2, len(self.history))
+
+    @simple_layout('layouts/requirements-gitlab.yaml', driver='gitlab')
+    def test_reject_unapproved(self):
+
+        fake_mr = self.fake_gitlab.openFakeMergeRequest(project='org/project5',
+                                                        branch='master',
+                                                        title='Example MR')
+
+        # The job should not be triggered for new and unapproved change
+        self.fake_gitlab.emitEvent(fake_mr.getMergeRequestOpenedEvent())
+        self.waitUntilSettled()
+        self.assertEqual(0, len(self.history))
+
+        # The job still should not be triggered for unapproved change
+        fake_mr.approved = False
+        self.fake_gitlab.emitEvent(fake_mr.getMergeRequestUpdatedEvent())
+        self.waitUntilSettled()
+        self.assertEqual(0, len(self.history))
+
+        # The job now should be triggered for approved change
+        fake_mr.approved = True
+        self.fake_gitlab.emitEvent(fake_mr.getMergeRequestUpdatedEvent())
+        self.waitUntilSettled()
+        self.assertEqual(1, len(self.history))
+
+        # The job again should not be triggered for unapproved change
+        fake_mr.approved = False
+        self.fake_gitlab.emitEvent(fake_mr.getMergeRequestUpdatedEvent())
+        self.waitUntilSettled()
+        self.assertEqual(1, len(self.history))
+
+    @simple_layout('layouts/requirements-gitlab.yaml', driver='gitlab')
+    def test_label_reject(self):
+
+        fake_mr = self.fake_gitlab.openFakeMergeRequest(project='org/project6',
+                                                        branch='master',
+                                                        title='Example MR')
+
+        # The job should not be triggered when required labels are not set
+        self.fake_gitlab.emitEvent(fake_mr.getMergeRequestOpenedEvent())
+        self.waitUntilSettled()
+        self.assertEqual(0, len(self.history))
+
+        # The job still should not be triggered due to missing verified label
+        fake_mr.labels = ['gateit', 'ignored']
+        self.fake_gitlab.emitEvent(fake_mr.getMergeRequestUpdatedEvent())
+        self.waitUntilSettled()
+        self.assertEqual(0, len(self.history))
+
+        # The job still should not be triggered (now also due to do-not-merge)
+        fake_mr.labels = ['gateit', 'ignored', 'do-not-merge']
+        self.fake_gitlab.emitEvent(fake_mr.getMergeRequestUpdatedEvent())
+        self.waitUntilSettled()
+        self.assertEqual(0, len(self.history))
+
+        # The job still should not be triggered due to reject condition met
+        fake_mr.labels = ['gateit', 'ignored', 'do-not-merge', 'verified']
+        self.fake_gitlab.emitEvent(fake_mr.getMergeRequestUpdatedEvent())
+        self.waitUntilSettled()
+        self.assertEqual(0, len(self.history))
+
+        # The job still should not be triggered due to reject condition met
+        fake_mr.labels = ['gateit', 'ignored', 'do-not-merge',
+                          'do-not-test', 'verified']
+        self.fake_gitlab.emitEvent(fake_mr.getMergeRequestUpdatedEvent())
+        self.waitUntilSettled()
+        self.assertEqual(0, len(self.history))
+
+        # The job now should be triggered
+        fake_mr.labels = ['gateit', 'ignored', 'verified']
+        self.fake_gitlab.emitEvent(fake_mr.getMergeRequestUpdatedEvent())
+        self.waitUntilSettled()
+        self.assertEqual(1, len(self.history))
+
+    @simple_layout('layouts/requirements-gitlab.yaml', driver='gitlab')
+    def test_require_reject_comprehensive(self):
+
+        fake_mr = self.fake_gitlab.openFakeMergeRequest(project='org/project7',
+                                                        branch='master',
+                                                        title='Example MR')
+        fake_mr.approved = False
+
+        # The job should not be triggered for new change without gateit label
+        self.fake_gitlab.emitEvent(fake_mr.getMergeRequestOpenedEvent())
+        self.waitUntilSettled()
+        self.assertEqual(0, len(self.history))
+
+        # The job still should not be triggered even though it is approved now
+        fake_mr.approved = True
+        self.fake_gitlab.emitEvent(fake_mr.getMergeRequestUpdatedEvent())
+        self.waitUntilSettled()
+        self.assertEqual(0, len(self.history))
+
+        # The job now should be triggered with required label set
+        fake_mr.approved = False
+        fake_mr.labels = ['gateit']
+        self.fake_gitlab.emitEvent(fake_mr.getMergeRequestUpdatedEvent())
+        self.waitUntilSettled()
+        self.assertEqual(1, len(self.history))
+
+        # The job should not be triggered due to rejected label
+        fake_mr.labels = ['gateit', 'do-not-merge']
+        self.fake_gitlab.emitEvent(fake_mr.getMergeRequestUpdatedEvent())
+        self.waitUntilSettled()
+        self.assertEqual(1, len(self.history))
+
+        # The job again should be triggered when rejected label is gone
+        fake_mr.labels = ['gateit']
+        self.fake_gitlab.emitEvent(fake_mr.getMergeRequestUpdatedEvent())
+        self.waitUntilSettled()
+        self.assertEqual(2, len(self.history))
+
+        # The job should not be triggered for closed change
+        fake_mr.closeMergeRequest()
+        self.fake_gitlab.emitEvent(
+            fake_mr.getMergeRequestCommentedEvent('recheck')
+        )
+        self.waitUntilSettled()
+        self.assertEqual(2, len(self.history))
+
+        # The job again should be triggered after reopening the change
+        fake_mr.reopenMergeRequest()
+        self.fake_gitlab.emitEvent(fake_mr.getMergeRequestOpenedEvent())
+        self.waitUntilSettled()
+        self.assertEqual(3, len(self.history))
+
+        # The job no longer should be triggered after the change was merged
+        fake_mr.mergeMergeRequest()
+        self.fake_gitlab.emitEvent(
+            fake_mr.getMergeRequestCommentedEvent('recheck')
+        )
+        self.waitUntilSettled()
+        self.assertEqual(3, len(self.history))
+
     @simple_layout('layouts/gitlab-label-add-remove.yaml', driver='gitlab')
     def test_label_add_remove(self):
 

zuul/driver/gitlab/gitlabmodel.py

@@ -13,7 +13,11 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
-from zuul.model import Change, TriggerEvent, EventFilter, RefFilter
+from zuul.model import Change
+from zuul.model import EventFilter
+from zuul.model import FalseWithReason
+from zuul.model import RefFilter
+from zuul.model import TriggerEvent
 
 EMPTY_GIT_REF = '0' * 40  # git sha of all zeros, used during creates/deletes
 
@@ -293,3 +297,47 @@ class GitlabRefFilter(RefFilter):
                 return False
 
         return True
+
+
+# The RejectFilter is the negative version of RefFilter/RequireFilter
+# – if any of the defined conditions is met, the change will not trigger jobs
+class GitlabRejectFilter(RefFilter):
+    def __init__(self, connection_name, open=None, merged=None, approved=None,
+                 labels=None):
+        RefFilter.__init__(self, connection_name)
+        self.open = open
+        self.merged = merged
+        self.approved = approved
+        self.labels = labels or []
+
+    def __repr__(self):
+        ret = f'<GitlabRejectFilter connection_name: {self.connection_name}'
+        if self.open is not None:
+            ret += f' open: {self.open}'
+        if self.merged is not None:
+            ret += f' merged: {self.merged}'
+        if self.approved is not None:
+            ret += f' approved: {self.approved}'
+        if self.labels:
+            ret += f' labels: {",".join(self.labels)}'
+        ret += '>'
+        return ret
+
+    def matches(self, change):
+        if self.open is not None:
+            if change.open == self.open:
+                return FalseWithReason('Matched the open attribute')
+
+        if self.merged is not None:
+            if change.is_merged == self.merged:
+                return FalseWithReason('Matched the merged attribute')
+
+        if self.approved is not None:
+            if change.approved == self.approved:
+                return FalseWithReason('Matched the approved attribute')
+
+        if self.labels:
+            if set(self.labels).intersection(set(change.labels)):
+                return FalseWithReason('Matched one or more labels')
+
+        return True  # By default, do not reject the change

zuul/driver/gitlab/gitlabsource.py

@@ -19,6 +19,7 @@ import urllib
 from zuul.model import Project
 from zuul.source import BaseSource
 from zuul.driver.gitlab.gitlabmodel import GitlabRefFilter
+from zuul.driver.gitlab.gitlabmodel import GitlabRejectFilter
 from zuul.driver.util import scalar_or_list, to_list
 from zuul.zk.change_cache import ChangeKey
 
@@ -154,7 +155,14 @@ class GitlabSource(BaseSource):
         return [f]
 
     def getRejectFilters(self, config, parse_context):
-        raise NotImplementedError()
+        f = GitlabRejectFilter(
+            connection_name=self.connection.connection_name,
+            open=config.get('open'),
+            merged=config.get('merged'),
+            approved=config.get('approved'),
+            labels=to_list(config.get('labels')),
+        )
+        return [f]
 
     def getRefForChange(self, change):
         return "refs/merge-requests/%s/head" % change
@@ -175,5 +183,10 @@ def getRequireSchema():
 
 
 def getRejectSchema():
-    reject = {}
+    reject = {
+        'open': bool,
+        'merged': bool,
+        'approved': bool,
+        'labels': scalar_or_list(str)
+    }
     return reject