Add support for negative requirements

This change adds support for false matching of requirements. To make
this useful you can now require all approvals match a requirement or
only one (ie any).

Therefore depreciate require-approvals, replacing with
require-approvals-any and a new require-approvals-all.

Change-Id: I458e677315ccb90d64cd0c0e734951141324a9c3

15 files changed, 434 insertions, 82 deletions

diff --git a/doc/source/zuul.rst b/doc/source/zuul.rst
index 6c77477ac..493c18d4d 100644
--- a/doc/source/zuul.rst
+++ b/doc/source/zuul.rst
@@ -462,13 +462,21 @@ explanation of each of the parameters::
     A deprecated alternate spelling of *comment*.  Only one of *comment* or
     *comment_filter* should be used.
 
-    *require-approval*
+    *require-any-approval*
     This may be used for any event.  It requires that a certain kind
     of approval be present for the current patchset of the change (the
     approval could be added by the event in question).  It follows the
     same syntax as the :ref:`"approval" pipeline requirement below
     <pipeline-require-approval>`.
 
+    *require-all-approvals*
+    This takes a list of approvals in the same format as
+    *require-any-approval* but requires all approvals match the rules.
+
+    **require-approval** (depreciated)
+    A deprecated alternate spelling of *require-any-approval*. This will
+    be joined with *require-any-approval* if both are present.
+
   **timer**
     This trigger will run based on a cron-style time specification.
     It will enqueue an event into its pipeline for every project
@@ -515,7 +523,7 @@ explanation of each of the parameters::
 
 .. _pipeline-require-approval:
 
-  **approval**
+  **any-approval**
   This requires that a certain kind of approval be present for the
   current patchset of the change (the approval could be added by the
   event in question).  It takes several sub-parameters, all of which
@@ -549,6 +557,24 @@ explanation of each of the parameters::
     be a single value or a list: ``verified: [1, 2]`` would match
     either a +1 or +2 vote.
 
+    You can also match negative conditions by starting with an
+    exclamation mark (!). This requires the value to be a string.
+    Example: ``verified: '![-1, -2]'``
+
+  This takes a list of approvals in the same format as above. It
+  requires that any approval on a change can meet the specified
+  criteria.
+
+  **all-approvals**
+  This takes a list of approvals in the same format as *any-approval* but
+  requires all approvals match the rules. For example, you can stop any
+  new changes from queueing when there is a negative vote by requiring
+  all approves to not have a -1.
+
+  **approval** (depreciated)
+  A deprecated alternate spelling of *any-approval*. This will be
+  joined with *any-approval* if both are present.
+
   **open**
   A boolean value (``true`` or ``false``) that indicates whether the change
   must be open or closed in order to be enqueued.
diff --git a/tests/fixtures/layout-requirement-all.yaml b/tests/fixtures/layout-requirement-all.yaml
new file mode 100644
index 000000000..968739daf
--- /dev/null
+++ b/tests/fixtures/layout-requirement-all.yaml
@@ -0,0 +1,41 @@
+pipelines:
+  - name: pipeline
+    manager: IndependentPipelineManager
+    require:
+      all-approvals:
+        - username: jenkins
+          verified: [1, 2]
+        - verified: "![-1, -2]"
+    trigger:
+      gerrit:
+        - event: comment-added
+    success:
+      gerrit:
+        verified: 1
+    failure:
+      gerrit:
+        verified: -1
+
+  - name: trigger
+    manager: IndependentPipelineManager
+    trigger:
+      gerrit:
+        - event: comment-added
+          require-all-approvals:
+            - username: jenkins
+              verified: [1, 2]
+            - verified: "![-1, -2]"
+    success:
+      gerrit:
+        verified: 1
+    failure:
+      gerrit:
+        verified: -1
+
+projects:
+  - name: org/project1
+    pipeline:
+      - project1-pipeline
+  - name: org/project2
+    trigger:
+      - project2-trigger
diff --git a/tests/fixtures/layout-requirement-any.yaml b/tests/fixtures/layout-requirement-any.yaml
new file mode 100644
index 000000000..6275d8dd9
--- /dev/null
+++ b/tests/fixtures/layout-requirement-any.yaml
@@ -0,0 +1,43 @@
+pipelines:
+  - name: pipeline
+    manager: IndependentPipelineManager
+    require:
+      any-approval:
+        - username: jenkins
+          verified: [1, 2]
+        - username: core-reviewer
+          code-review: "![-1, -2]"
+    trigger:
+      gerrit:
+        - event: comment-added
+    success:
+      gerrit:
+        verified: 1
+    failure:
+      gerrit:
+        verified: -1
+
+  - name: trigger
+    manager: IndependentPipelineManager
+    trigger:
+      gerrit:
+        - event: comment-added
+          require-any-approval:
+            - username: jenkins
+              verified: [1, 2]
+            - username: core-reviewer
+              code-review: "![-1, -2]"
+    success:
+      gerrit:
+        verified: 1
+    failure:
+      gerrit:
+        verified: -1
+
+projects:
+  - name: org/project1
+    pipeline:
+      - project1-pipeline
+  - name: org/project2
+    trigger:
+      - project2-trigger
diff --git a/tests/fixtures/layout-requirement-email.yaml b/tests/fixtures/layout-requirement-email.yaml
index 4bfb733f9..dadcd6c6f 100644
--- a/tests/fixtures/layout-requirement-email.yaml
+++ b/tests/fixtures/layout-requirement-email.yaml
@@ -2,7 +2,7 @@ pipelines:
   - name: pipeline
     manager: IndependentPipelineManager
     require:
-      approval:
+      any-approval:
         - email: jenkins@example.com
     trigger:
       gerrit:
@@ -19,7 +19,7 @@ pipelines:
     trigger:
       gerrit:
         - event: comment-added
-          require-approval:
+          require-any-approval:
             - email: jenkins@example.com
     success:
       gerrit:
diff --git a/tests/fixtures/layout-requirement-negative-username.yaml b/tests/fixtures/layout-requirement-negative-username.yaml
new file mode 100644
index 000000000..f542b86a9
--- /dev/null
+++ b/tests/fixtures/layout-requirement-negative-username.yaml
@@ -0,0 +1,37 @@
+pipelines:
+  - name: pipeline
+    manager: IndependentPipelineManager
+    require:
+      all-approvals:
+        - username: '!jenkins'
+    trigger:
+      gerrit:
+        - event: comment-added
+    success:
+      gerrit:
+        verified: 1
+    failure:
+      gerrit:
+        verified: -1
+
+  - name: trigger
+    manager: IndependentPipelineManager
+    trigger:
+      gerrit:
+        - event: comment-added
+          require-all-approvals:
+            - username: '!jenkins'
+    success:
+      gerrit:
+        verified: 1
+    failure:
+      gerrit:
+        verified: -1
+
+projects:
+  - name: org/project1
+    pipeline:
+      - project1-pipeline
+  - name: org/project2
+    trigger:
+      - project2-trigger
\ No newline at end of file
diff --git a/tests/fixtures/layout-requirement-newer-than.yaml b/tests/fixtures/layout-requirement-newer-than.yaml
index b6beb35be..f723c7929 100644
--- a/tests/fixtures/layout-requirement-newer-than.yaml
+++ b/tests/fixtures/layout-requirement-newer-than.yaml
@@ -2,7 +2,7 @@ pipelines:
   - name: pipeline
     manager: IndependentPipelineManager
     require:
-      approval:
+      any-approval:
         - username: jenkins
           newer-than: 48h
     trigger:
@@ -20,7 +20,7 @@ pipelines:
     trigger:
       gerrit:
         - event: comment-added
-          require-approval:
+          require-any-approval:
             - username: jenkins
               newer-than: 48h
     success:
diff --git a/tests/fixtures/layout-requirement-older-than.yaml b/tests/fixtures/layout-requirement-older-than.yaml
index 2edf9df1a..0e011ccd3 100644
--- a/tests/fixtures/layout-requirement-older-than.yaml
+++ b/tests/fixtures/layout-requirement-older-than.yaml
@@ -2,7 +2,7 @@ pipelines:
   - name: pipeline
     manager: IndependentPipelineManager
     require:
-      approval:
+      any-approval:
         - username: jenkins
           older-than: 48h
     trigger:
@@ -20,7 +20,7 @@ pipelines:
     trigger:
       gerrit:
         - event: comment-added
-          require-approval:
+          require-any-approval:
             - username: jenkins
               older-than: 48h
     success:
diff --git a/tests/fixtures/layout-requirement-username.yaml b/tests/fixtures/layout-requirement-username.yaml
index 7a549f04b..852017996 100644
--- a/tests/fixtures/layout-requirement-username.yaml
+++ b/tests/fixtures/layout-requirement-username.yaml
@@ -2,7 +2,7 @@ pipelines:
   - name: pipeline
     manager: IndependentPipelineManager
     require:
-      approval:
+      any-approval:
         - username: jenkins
     trigger:
       gerrit:
@@ -19,7 +19,7 @@ pipelines:
     trigger:
       gerrit:
         - event: comment-added
-          require-approval:
+          require-any-approval:
             - username: jenkins
     success:
       gerrit:
diff --git a/tests/fixtures/layout-requirement-vote.yaml b/tests/fixtures/layout-requirement-vote.yaml
index 7ccadffa6..6736e98b5 100644
--- a/tests/fixtures/layout-requirement-vote.yaml
+++ b/tests/fixtures/layout-requirement-vote.yaml
@@ -2,7 +2,7 @@ pipelines:
   - name: pipeline
     manager: IndependentPipelineManager
     require:
-      approval:
+      any-approval:
         - username: jenkins
           verified: 1
     trigger:
@@ -20,7 +20,7 @@ pipelines:
     trigger:
       gerrit:
         - event: comment-added
-          require-approval:
+          require-any-approval:
             - username: jenkins
               verified: 1
     success:
diff --git a/tests/fixtures/layout-requirement-vote1.yaml b/tests/fixtures/layout-requirement-vote1.yaml
index 7ccadffa6..6736e98b5 100644
--- a/tests/fixtures/layout-requirement-vote1.yaml
+++ b/tests/fixtures/layout-requirement-vote1.yaml
@@ -2,7 +2,7 @@ pipelines:
   - name: pipeline
     manager: IndependentPipelineManager
     require:
-      approval:
+      any-approval:
         - username: jenkins
           verified: 1
     trigger:
@@ -20,7 +20,7 @@ pipelines:
     trigger:
       gerrit:
         - event: comment-added
-          require-approval:
+          require-any-approval:
             - username: jenkins
               verified: 1
     success:
diff --git a/tests/fixtures/layout-requirement-vote2.yaml b/tests/fixtures/layout-requirement-vote2.yaml
index 33d84d1d7..a6cd6a342 100644
--- a/tests/fixtures/layout-requirement-vote2.yaml
+++ b/tests/fixtures/layout-requirement-vote2.yaml
@@ -2,7 +2,7 @@ pipelines:
   - name: pipeline
     manager: IndependentPipelineManager
     require:
-      approval:
+      any-approval:
         - username: jenkins
           verified: [1, 2]
     trigger:
@@ -20,7 +20,7 @@ pipelines:
     trigger:
       gerrit:
         - event: comment-added
-          require-approval:
+          require-any-approval:
             - username: jenkins
               verified: [1, 2]
     success:
diff --git a/tests/test_requirements.py b/tests/test_requirements.py
index 431692539..52e397324 100644
--- a/tests/test_requirements.py
+++ b/tests/test_requirements.py
@@ -323,3 +323,131 @@ class TestRequirements(ZuulTestCase):
         self.fake_gerrit.addEvent(B.addApproval('CRVW', 2))
         self.waitUntilSettled()
         self.assertEqual(len(self.history), 1)
+
+    def test_pipeline_require_negative_username(self):
+        "Test negative pipeline requirement: no comment from jenkins"
+        return self._test_require_negative_username('org/project1',
+                                                    'project1-pipeline')
+
+    def test_trigger_require_negative_username(self):
+        "Test negative trigger requirement: no comment from jenkins"
+        return self._test_require_negative_username('org/project2',
+                                                    'project2-trigger')
+
+    def _test_require_negative_username(self, project, job):
+        "Test negative username's match"
+        # Should only trigger if Jenkins hasn't voted.
+        self.config.set(
+            'zuul', 'layout_config',
+            'tests/fixtures/layout-requirement-negative-username.yaml')
+        self.sched.reconfigure(self.config)
+        self.registerJobs()
+
+        # add in a change with no comments
+        A = self.fake_gerrit.addFakeChange(project, 'master', 'A')
+        self.waitUntilSettled()
+        self.assertEqual(len(self.history), 0)
+
+        # add in a comment that will trigger
+        self.fake_gerrit.addEvent(A.addApproval('CRVW', 1,
+                                                username='reviewer'))
+        self.waitUntilSettled()
+        self.assertEqual(len(self.history), 1)
+        self.assertEqual(self.history[0].name, job)
+
+        # add in a comment from jenkins user which shouldn't trigger
+        self.fake_gerrit.addEvent(A.addApproval('VRFY', 1, username='jenkins'))
+        self.waitUntilSettled()
+        self.assertEqual(len(self.history), 1)
+
+        # Check future reviews also won't trigger as a 'jenkins' user has
+        # commented previously
+        self.fake_gerrit.addEvent(A.addApproval('CRVW', 1,
+                                                username='reviewer'))
+        self.waitUntilSettled()
+        self.assertEqual(len(self.history), 1)
+
+    def test_pipeline_require_any(self):
+        "Test pipeline requirement: any requirement passes"
+        return self._test_require_any('org/project1', 'project1-pipeline')
+
+    def test_trigger_require_any(self):
+        "Test trigger requirement: any requirement passes"
+        return self._test_require_any('org/project2', 'project2-trigger')
+
+    def _test_require_any(self, project, job):
+        "Test any of the given requirements are matched"
+        self.config.set(
+            'zuul', 'layout_config',
+            'tests/fixtures/layout-requirement-any.yaml')
+        self.sched.reconfigure(self.config)
+        self.registerJobs()
+
+        A = self.fake_gerrit.addFakeChange(project, 'master', 'A')
+        # A comment event that we will keep submitting to trigger
+        comment = A.addApproval('CRVW', 1, username='nobody')
+        self.fake_gerrit.addEvent(comment)
+        self.waitUntilSettled()
+        # No approval from Jenkins so should not be enqueued
+        self.assertEqual(len(self.history), 0)
+
+        # A +1 from jenkins should allow it to be enqueued
+        A.addApproval('VRFY', 1, username='jenkins')
+        self.fake_gerrit.addEvent(comment)
+        self.waitUntilSettled()
+        self.assertEqual(len(self.history), 1)
+        self.assertEqual(self.history[0].name, job)
+
+        # A non-negative from a non-core should not queue
+        B = self.fake_gerrit.addFakeChange(project, 'master', 'B')
+        # A comment event that we will keep submitting to trigger
+        comment = B.addApproval('CRVW', 1, username='nobody')
+        self.fake_gerrit.addEvent(comment)
+        self.waitUntilSettled()
+        self.assertEqual(len(self.history), 1)
+
+        # A non-negative from a core member should queue
+        B.addApproval('CRVW', 2, username='core-reviewer')
+        self.fake_gerrit.addEvent(comment)
+        self.waitUntilSettled()
+        self.assertEqual(len(self.history), 2)
+        self.assertEqual(self.history[1].name, job)
+
+    def test_pipeline_require_all(self):
+        "Test pipeline requirement: all requirements pass"
+        return self._test_require_all('org/project1', 'project1-pipeline')
+
+    def test_trigger_require_all(self):
+        "Test trigger requirement: all requirements pass"
+        return self._test_require_all('org/project2', 'project2-trigger')
+
+    def _test_require_all(self, project, job):
+        "Test all of the given requirements are matched"
+        self.config.set(
+            'zuul', 'layout_config',
+            'tests/fixtures/layout-requirement-all.yaml')
+        self.sched.reconfigure(self.config)
+        self.registerJobs()
+
+        A = self.fake_gerrit.addFakeChange(project, 'master', 'A')
+        self.waitUntilSettled()
+        self.assertEqual(len(self.history), 0)
+
+        # A +2 from 'nobody' only satisfies the non-negative requirement,
+        # not the requirement to be from 'jenkins'
+        comment = A.addApproval('VRFY', 1, username='nobody')
+        self.fake_gerrit.addEvent(comment)
+        self.waitUntilSettled()
+        self.assertEqual(len(self.history), 0)
+
+        B = self.fake_gerrit.addFakeChange(project, 'master', 'A')
+        self.waitUntilSettled()
+        self.assertEqual(len(self.history), 0)
+
+        # A +2 from Jenkins satisfies both the user condition and the
+        # non-negative condition
+        comment = B.addApproval('VRFY', 2, username='jenkins')
+        self.fake_gerrit.addEvent(comment)
+        self.waitUntilSettled()
+        self.assertEqual(len(self.history), 1)
+        self.assertEqual(self.history[0].name, job)
diff --git a/zuul/layoutvalidator.py b/zuul/layoutvalidator.py
index 88d10e25b..1569fa908 100644
--- a/zuul/layoutvalidator.py
+++ b/zuul/layoutvalidator.py
@@ -62,6 +62,8 @@ class LayoutSchema(object):
                       'branch': toList(str),
                       'ref': toList(str),
                       'approval': toList(variable_dict),
+                      'require-all-approvals': toList(require_approval),
+                      'require-any-approval': toList(require_approval),
                       'require-approval': toList(require_approval),
                       }
 
@@ -85,7 +87,9 @@ class LayoutSchema(object):
                                },
                       }
 
-    require = {'approval': toList(require_approval),
+    require = {'all-approvals': toList(require_approval),
+               'any-approval': toList(require_approval),
+               'approval': toList(require_approval),
                'open': bool,
                'current-patchset': bool,
                'status': toList(str)}
diff --git a/zuul/model.py b/zuul/model.py
index 4d402ff9c..64c6c783e 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -12,8 +12,10 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import ast
 import copy
 import re
+import six
 import time
 from uuid import uuid4
 import extras
@@ -1019,68 +1021,127 @@ class TriggerEvent(object):
 
 
 class BaseFilter(object):
-    def __init__(self, required_approvals=[]):
-        self._required_approvals = copy.deepcopy(required_approvals)
-        self.required_approvals = required_approvals
-
-        for a in self.required_approvals:
+    def __init__(self, required_any_approval=[], required_all_approvals=[]):
+        self._required_any_approval = copy.deepcopy(required_any_approval)
+        self.required_any_approval = self._tidy_approvals(
+            required_any_approval)
+        self._required_all_approvals = copy.deepcopy(required_all_approvals)
+        self.required_all_approvals = self._tidy_approvals(
+            required_all_approvals)
+
+    def _tidy_approvals(self, approvals):
+        for a in approvals:
             for k, v in a.items():
                 if k == 'username':
                     pass
                 elif k in ['email', 'email-filter']:
-                    a['email'] = re.compile(v)
+                    a['email'] = v
                 elif k == 'newer-than':
-                    a[k] = time_to_seconds(v)
+                    a[k] = v
                 elif k == 'older-than':
-                    a[k] = time_to_seconds(v)
-                else:
-                    if not isinstance(v, list):
-                        a[k] = [v]
+                    a[k] = v
             if 'email-filter' in a:
                 del a['email-filter']
+        return approvals
 
-    def matchesRequiredApprovals(self, change):
+    def _match_approval_required_approval(self, rapproval, approval):
+        # Check if the required approval and approval match
+        if 'description' not in approval:
+            return False
         now = time.time()
-        for rapproval in self.required_approvals:
+        found_approval = True
+        by = approval.get('by', {})
+        for k, v in rapproval.items():
+            negative_match = False
+            item_match = True
+            if isinstance(v, six.string_types) and v[0] == '!':
+                v = v[1:].strip()
+                item_match = False
+                negative_match = True
+
+            if k == 'username':
+                if (by.get('username', '') != v):
+                        item_match = negative_match
+            elif k == 'email':
+                v = re.compile(v)
+                if (not v.search(by.get('email', ''))):
+                        item_match = negative_match
+            elif k == 'newer-than':
+                t = now - time_to_seconds(v)
+                if (approval['grantedOn'] < t):
+                        item_match = negative_match
+            elif k == 'older-than':
+                t = now - time_to_seconds(v)
+                if (approval['grantedOn'] >= t):
+                    item_match = negative_match
+            else:
+                if isinstance(v, six.string_types):
+                    v = ast.literal_eval(v)
+                if not isinstance(v, list):
+                    v = [v]
+                if (normalizeCategory(approval['description']) != k or
+                        int(approval['value']) not in v):
+                    item_match = negative_match
+            if not item_match:
+                found_approval = False
+        return found_approval
+
+    def matchesRequiredApprovals(self, change):
+        if (self.required_any_approval and not change.approvals
+                or self.required_all_approvals and not change.approvals):
+            # A change with no approvals can not match
+            return False
+
+        # TODO(jhesketh): If we wanted to optimise this slightly we could
+        # analyse both the ANY and ALL filters by looping over the approvals
+        # on the change and keeping track of what we have checked rather than
+        # needing to loop on the change approvals twice
+        return (self.matchesRequiredAnyApproval(change) and
+                self.matchesRequiredAllApprovals(change))
+
+    def matchesRequiredAnyApproval(self, change):
+        # Check if any approvals match the any requirements
+        if not self.required_any_approval:
+            # No approval required, so we must match
+            return True
+
+        for rapproval in self.required_any_approval:
             matches_approval = False
             for approval in change.approvals:
-                if 'description' not in approval:
-                    continue
-                found_approval = True
-                by = approval.get('by', {})
-                for k, v in rapproval.items():
-                    if k == 'username':
-                        if (by.get('username', '') != v):
-                            found_approval = False
-                    elif k == 'email':
-                        if (not v.search(by.get('email', ''))):
-                            found_approval = False
-                    elif k == 'newer-than':
-                        t = now - v
-                        if (approval['grantedOn'] < t):
-                            found_approval = False
-                    elif k == 'older-than':
-                        t = now - v
-                        if (approval['grantedOn'] >= t):
-                            found_approval = False
-                    else:
-                        if (normalizeCategory(approval['description']) != k or
-                            int(approval['value']) not in v):
-                            found_approval = False
-                if found_approval:
-                    matches_approval = True
-                    break
-            if not matches_approval:
-                return False
+                matches_approval = self._match_approval_required_approval(
+                    rapproval, approval)
+                if matches_approval:
+                    # We have a matching approval so this requirement is
+                    # fulfilled
+                    return True
+        return False
+
+    def matchesRequiredAllApprovals(self, change):
+        # Check that /all/ of the approvals match the requirements
+        if not self.required_all_approvals:
+            # No approvals required, so we must match
+            return True
+
+        for rapproval in self.required_all_approvals:
+            for approval in change.approvals:
+                matches_approval = self._match_approval_required_approval(
+                    rapproval, approval)
+                if not matches_approval:
+                    # We have an approval that doesn't match so this
+                    # requirement can't be fulfilled
+                    return False
+        # We must have matched everything
         return True
 
 
 class EventFilter(BaseFilter):
     def __init__(self, trigger, types=[], branches=[], refs=[],
                  event_approvals={}, comments=[], emails=[], usernames=[],
-                 timespecs=[], required_approvals=[], pipelines=[]):
+                 timespecs=[], required_any_approval=[],
+                 required_all_approvals=[], pipelines=[]):
         super(EventFilter, self).__init__(
-            required_approvals=required_approvals)
+            required_any_approval=required_any_approval,
+            required_all_approvals=required_all_approvals)
         self.trigger = trigger
         self._types = types
         self._branches = branches
@@ -1113,9 +1174,12 @@ class EventFilter(BaseFilter):
         if self.event_approvals:
             ret += ' event_approvals: %s' % ', '.join(
                 ['%s:%s' % a for a in self.event_approvals.items()])
-        if self.required_approvals:
-            ret += ' required_approvals: %s' % ', '.join(
-                ['%s' % a for a in self._required_approvals])
+        if self.required_any_approval:
+            ret += ' required_any_approval: %s' % ', '.join(
+                ['%s' % a for a in self._required_any_approval])
+        if self.required_all_approvals:
+            ret += ' required_all_approvals: %s' % ', '.join(
+                ['%s' % a for a in self._required_all_approvals])
         if self._comments:
             ret += ' comments: %s' % ', '.join(self._comments)
         if self._emails:
@@ -1204,10 +1268,6 @@ class EventFilter(BaseFilter):
             if not matches_approval:
                 return False
 
-        if self.required_approvals and not change.approvals:
-            # A change with no approvals can not match
-            return False
-
         # required approvals are ANDed
         if not self.matchesRequiredApprovals(change):
             return False
@@ -1225,9 +1285,11 @@ class EventFilter(BaseFilter):
 
 class ChangeishFilter(BaseFilter):
     def __init__(self, open=None, current_patchset=None,
-                 statuses=[], required_approvals=[]):
+                 statuses=[], required_any_approval=[],
+                 required_all_approvals=[]):
         super(ChangeishFilter, self).__init__(
-            required_approvals=required_approvals)
+            required_any_approval=required_any_approval,
+            required_all_approvals=required_all_approvals)
         self.open = open
         self.current_patchset = current_patchset
         self.statuses = statuses
@@ -1241,8 +1303,12 @@ class ChangeishFilter(BaseFilter):
             ret += ' current-patchset: %s' % self.current_patchset
         if self.statuses:
             ret += ' statuses: %s' % ', '.join(self.statuses)
-        if self.required_approvals:
-            ret += ' required_approvals: %s' % str(self.required_approvals)
+        if self.required_any_approval:
+            ret += (' required_any_approval: %s' %
+                    str(self.required_any_approval))
+        if self.required_all_approvals:
+            ret += (' required_all_approvals: %s' %
+                    str(self.required_all_approvals))
         ret += '>'
 
         return ret
@@ -1260,10 +1326,6 @@ class ChangeishFilter(BaseFilter):
             if change.status not in self.statuses:
                 return False
 
-        if self.required_approvals and not change.approvals:
-            # A change with no approvals can not match
-            return False
-
         # required approvals are ANDed
         if not self.matchesRequiredApprovals(change):
             return False
diff --git a/zuul/scheduler.py b/zuul/scheduler.py
index 131ad62c3..fed5c1704 100644
--- a/zuul/scheduler.py
+++ b/zuul/scheduler.py
@@ -326,7 +326,10 @@ class Scheduler(threading.Thread):
                     open=require.get('open'),
                     current_patchset=require.get('current-patchset'),
                     statuses=toList(require.get('status')),
-                    required_approvals=toList(require.get('approval')))
+                    required_any_approval=(toList(require.get('any-approval'))
+                                           + toList(require.get('approval'))),
+                    required_all_approvals=toList(require.get('all-approvals'))
+                )
                 manager.changeish_filters.append(f)
 
             # TODO: move this into triggers (may require pluggable
@@ -356,9 +359,13 @@ class Scheduler(threading.Thread):
                         comments=comments,
                         emails=emails,
                         usernames=usernames,
-                        required_approvals=toList(
-                            trigger.get('require-approval')
-                        )
+                        required_any_approval=(
+                            toList(trigger.get('require-any-approval'))
+                            + toList(trigger.get('require-approval'))
+                        ),
+                        required_all_approvals=toList(
+                            trigger.get('require-all-approvals')
+                        ),
                     )
                     manager.event_filters.append(f)
             if 'timer' in conf_pipeline['trigger']:
@@ -373,9 +380,13 @@ class Scheduler(threading.Thread):
                         trigger=self.triggers['zuul'],
                         types=toList(trigger['event']),
                         pipelines=toList(trigger.get('pipeline')),
-                        required_approvals=toList(
-                            trigger.get('require-approval')
-                        )
+                        required_any_approval=(
+                            toList(trigger.get('require-any-approval'))
+                            + toList(trigger.get('require-approval'))
+                        ),
+                        required_all_approvals=toList(
+                            trigger.get('require-all-approvals')
+                        ),
                     )
                     manager.event_filters.append(f)