Check Gerrit submit requirements

With newer versions of Gerrit, we are increasingly likely to encounter
systems where the traditional label requirements are minimized in favor
of the new submit requirements rules.  If Gerrit is configured to use
a submit requirement instead of a traditional label blocking rule, that
is typically done by switching the label function to "NoBlock", which,
like the "NoOp" function, will still cause the label to appear in the
"submit_record" field, but with a value of "MAY" instead of "OK", "NEED",
or "REJECT".

Instead, the interesting information will be in the "submit_requirements"
field.  In this field we can see the individual submit requirement rules
and whether they are satisfied or not.

Since submit requirements do not have a 1:1 mapping with labels, determining
whether an "UNSATISFIED" submit requirement should be ignored (because it
pertains to a label that Zuul will alter, like "Verified") is not as
straightforward is it is for submit records.  To be conservative, this
change looks for any of the "allow needs" labels (typically "Verified") in
each unsatisfied submit record and if it finds one, it ignores that record.

With this change in place, we can avoid enqueing changes which we are certain
can not be merged into gate pipelines, and will continue to enqueue changes
about which we are uncertain.

Change-Id: I667181565684d97c1d036e2db6193dc606c76c57

releasenotes/notes/submit-requirements-1d61f88e54be1fde.yaml

@@ -0,0 +1,16 @@
+---
+fixes:
+  - |
+    Zuul will now attempt to honor Gerrit "submit requirements" when
+    determining whether to enqueue a change into a dependent (i.e.,
+    "gate") pipeline.  Zuul previously honored only Gerrit's older
+    "submit records" feature.  The new checks will avoid enqueing
+    changes in "gate" pipelines in the cases where Zuul can
+    unambiguously determine that there is no possibility of merging,
+    but some non-mergable changes may still be enqueued if Zuul can
+    not be certain whether a rule should apply or be disregarded (in
+    these cases, Gerrit will fail to merge the change and Zuul will
+    report the buildset as a MERGE_FAILURE).
+
+    This requires Gerrit version 3.5.0 or later, and Zuul to be
+    configured with HTTP access for Gerrit.

tests/base.py

@@ -403,6 +403,7 @@ class FakeGerritChange(object):
         self.comments = []
         self.checks = {}
         self.checks_history = []
+        self.submit_requirements = []
         self.data = {
             'branch': branch,
             'comments': self.comments,
@@ -788,6 +789,12 @@ class FakeGerritChange(object):
         return [{'status': 'NOT_READY',
                  'labels': labels}]
 
+    def getSubmitRequirements(self):
+        return self.submit_requirements
+
+    def setSubmitRequirements(self, reqs):
+        self.submit_requirements = reqs
+
     def setDependsOn(self, other, patchset):
         self.depends_on_change = other
         self.depends_on_patchset = patchset
@@ -894,6 +901,7 @@ class FakeGerritChange(object):
             data['parents'] = self.data['parents']
         if 'topic' in self.data:
             data['topic'] = self.data['topic']
+        data['submit_requirements'] = self.getSubmitRequirements()
         return json.loads(json.dumps(data))
 
     def queryRevisionHTTP(self, revision):

tests/unit/test_gerrit.py

@@ -958,6 +958,92 @@ class TestGerritConnection(ZuulTestCase):
         self.assertEqual(A.data['status'], 'MERGED')
         self.assertEqual(B.data['status'], 'MERGED')
 
+    def test_submit_requirements(self):
+        A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
+        A.addApproval('Code-Review', 2)
+        # Set an unsatisfied submit requirement
+        A.setSubmitRequirements([
+            {
+                "name": "Code-Review",
+                "description": "Disallow self-review",
+                "status": "UNSATISFIED",
+                "is_legacy": False,
+                "submittability_expression_result": {
+                    "expression": "label:Code-Review=MAX,user=non_uploader "
+                        "AND -label:Code-Review=MIN",
+                    "fulfilled": False,
+                    "passing_atoms": [],
+                    "failing_atoms": [
+                        "label:Code-Review=MAX,user=non_uploader",
+                        "label:Code-Review=MIN"
+                    ]
+                }
+            },
+            {
+                "name": "Verified",
+                "status": "UNSATISFIED",
+                "is_legacy": True,
+                "submittability_expression_result": {
+                    "expression": "label:Verified=MAX -label:Verified=MIN",
+                    "fulfilled": False,
+                    "passing_atoms": [],
+                    "failing_atoms": [
+                        "label:Verified=MAX",
+                        "-label:Verified=MIN"
+                    ]
+                }
+            },
+        ])
+        self.fake_gerrit.addEvent(A.addApproval('Approved', 1))
+        self.waitUntilSettled()
+        self.assertHistory([])
+        self.assertEqual(A.queried, 1)
+        self.assertEqual(A.data['status'], 'NEW')
+
+        # Mark the requirement satisfied
+        A.setSubmitRequirements([
+            {
+                "name": "Code-Review",
+                "description": "Disallow self-review",
+                "status": "SATISFIED",
+                "is_legacy": False,
+                "submittability_expression_result": {
+                    "expression": "label:Code-Review=MAX,user=non_uploader "
+                        "AND -label:Code-Review=MIN",
+                    "fulfilled": False,
+                    "passing_atoms": [
+                        "label:Code-Review=MAX,user=non_uploader",
+                    ],
+                    "failing_atoms": [
+                        "label:Code-Review=MIN"
+                    ]
+                }
+            },
+            {
+                "name": "Verified",
+                "status": "UNSATISFIED",
+                "is_legacy": True,
+                "submittability_expression_result": {
+                    "expression": "label:Verified=MAX -label:Verified=MIN",
+                    "fulfilled": False,
+                    "passing_atoms": [],
+                    "failing_atoms": [
+                        "label:Verified=MAX",
+                        "-label:Verified=MIN"
+                    ]
+                }
+            },
+        ])
+        self.fake_gerrit.addEvent(A.addApproval('Approved', 1))
+        self.waitUntilSettled()
+        self.assertHistory([
+            dict(name="project-merge", result="SUCCESS", changes="1,1"),
+            dict(name="project-test1", result="SUCCESS", changes="1,1"),
+            dict(name="project-test2", result="SUCCESS", changes="1,1"),
+        ], ordered=False)
+        self.assertEqual(A.queried, 3)
+        self.assertEqual(A.data['status'], 'MERGED')
+
 
 class TestGerritUnicodeRefs(ZuulTestCase):
     config_file = 'zuul-gerrit-web.conf'

zuul/driver/gerrit/gerritconnection.py

@@ -1182,9 +1182,34 @@ class GerritConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection):
             return True
         if change.wip:
             return False
-        if change.missing_labels <= set(allow_needs):
+        if change.missing_labels > set(allow_needs):
-            return True
+            self.log.debug("Unable to merge due to "
+                           "missing labels: %s", change.missing_labels)
             return False
+        for sr in change.submit_requirements:
+            if sr.get('status') == 'UNSATISFIED':
+                # Otherwise, we don't care and should skip.
+
+                # We're going to look at each unsatisfied submit
+                # requirement, and if one of the involved labels is an
+                # "allow_needs" label, we will assume that Zuul may be
+                # able to take an action which can cause the
+                # requirement to be satisfied, and we will ignore it.
+                # Otherwise, it is likely a requirement that Zuul can
+                # not alter in which case the requirement should stand
+                # and block merging.
+                result = sr.get("submittability_expression_result", {})
+                expression = result.get("expression", '')
+                expr_contains_allow = False
+                for allow in allow_needs:
+                    if f'label:{allow}' in expression:
+                        expr_contains_allow = True
+                        break
+                if not expr_contains_allow:
+                    self.log.debug("Unable to merge due to "
+                                   "submit requirement: %s", sr)
+                    return False
+        return True
 
     def getProjectOpenChanges(self, project: Project) -> List[GerritChange]:
         # This is a best-effort function in case Gerrit is unable to return
@@ -1443,9 +1468,12 @@ class GerritConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection):
         return data
 
     def queryChangeHTTP(self, number, event=None):
-        data = self.get('changes/%s?o=DETAILED_ACCOUNTS&o=CURRENT_REVISION&'
+        query = ('changes/%s?o=DETAILED_ACCOUNTS&o=CURRENT_REVISION&'
                  'o=CURRENT_COMMIT&o=CURRENT_FILES&o=LABELS&'
                  'o=DETAILED_LABELS' % (number,))
+        if self.version >= (3, 5, 0):
+            query += '&o=SUBMIT_REQUIREMENTS'
+        data = self.get(query)
         related = self.get('changes/%s/revisions/%s/related' % (
             number, data['current_revision']))
         files = self.get('changes/%s/revisions/%s/files?parent=1' % (

zuul/driver/gerrit/gerritmodel.py

@@ -34,6 +34,7 @@ class GerritChange(Change):
         self.wip = None
         self.approvals = []
         self.missing_labels = set()
+        self.submit_requirements = []
         self.commit = None
         self.zuul_query_ltime = None
 
@@ -52,6 +53,7 @@ class GerritChange(Change):
             "wip": self.wip,
             "approvals": self.approvals,
             "missing_labels": list(self.missing_labels),
+            "submit_requirements": self.submit_requirements,
             "commit": self.commit,
             "zuul_query_ltime": self.zuul_query_ltime,
         })
@@ -64,6 +66,7 @@ class GerritChange(Change):
         self.wip = data["wip"]
         self.approvals = data["approvals"]
         self.missing_labels = set(data["missing_labels"])
+        self.submit_requirements = data.get("submit_requirements", [])
         self.commit = data.get("commit")
         self.zuul_query_ltime = data.get("zuul_query_ltime")
 
@@ -189,6 +192,7 @@ class GerritChange(Change):
             if 'approved' in label_data:
                 continue
             self.missing_labels.add(label_name)
+        self.submit_requirements = data.get('submit_requirements', [])
         self.open = data['status'] == 'NEW'
         self.status = data['status']
         self.wip = data.get('work_in_progress', False)