Merge "Check Gerrit submit requirements"

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)