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

5 files changed, 148 insertions, 6 deletions

diff --git a/releasenotes/notes/submit-requirements-1d61f88e54be1fde.yaml b/releasenotes/notes/submit-requirements-1d61f88e54be1fde.yaml
new file mode 100644
index 000000000..14f037156
--- /dev/null
+++ b/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.
diff --git a/tests/base.py b/tests/base.py
index 290d51934..1cfcecde5 100644
--- a/tests/base.py
+++ b/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):
diff --git a/tests/unit/test_gerrit.py b/tests/unit/test_gerrit.py
index 2a63d5ef8..ac3dccf3b 100644
--- a/tests/unit/test_gerrit.py
+++ b/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'
diff --git a/zuul/driver/gerrit/gerritconnection.py b/zuul/driver/gerrit/gerritconnection.py
index f871671aa..6efca17c5 100644
--- a/zuul/driver/gerrit/gerritconnection.py
+++ b/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):
-            return True
-        return False
+        if change.missing_labels > set(allow_needs):
+            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&'
-                        'o=CURRENT_COMMIT&o=CURRENT_FILES&o=LABELS&'
-                        'o=DETAILED_LABELS' % (number,))
+        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' % (
diff --git a/zuul/driver/gerrit/gerritmodel.py b/zuul/driver/gerrit/gerritmodel.py
index 0ac3e7f9d..4ac291f2b 100644
--- a/zuul/driver/gerrit/gerritmodel.py
+++ b/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)