Detect Gerrit gate pipelines with the wrong connection

If multiple Gerrit connections are configured and the wrong one
is used in a gate pipeline, then the Gerrit reporter will correctly
detect that it should not act on a change from a different connection,
however, the isMerged check which is associated with the (correct)
source connection performs an extra check immediately after reporting.
Since the reporter didn't (couldn't) act, the ._ref_sha attribute
(pointing to the previous sha before reporting) won't be set, and
the isMerged check throws an exception.

To correct this, have isMerged detect this case, log an error, and
return False (which will fail the item in the pipeline).  Also,
have the reporter log when it's skipping reporting for a different
connection.  This is normal behavior when everything is configured
correctly, but could be a helpful clue when something is misconfigured,
so that is set at debug level.

Change-Id: Ic3b715273e08ed2df7096c79add1b319066c46ec
Story: 2007761

7 files changed, 91 insertions, 0 deletions

diff --git a/tests/fixtures/config/wrong-connection-in-pipeline/git/common-config/playbooks/test-job.yaml b/tests/fixtures/config/wrong-connection-in-pipeline/git/common-config/playbooks/test-job.yaml
new file mode 100644
index 000000000..f679dceae
--- /dev/null
+++ b/tests/fixtures/config/wrong-connection-in-pipeline/git/common-config/playbooks/test-job.yaml
@@ -0,0 +1,2 @@
+- hosts: all
+  tasks: []
diff --git a/tests/fixtures/config/wrong-connection-in-pipeline/git/common-config/zuul.yaml b/tests/fixtures/config/wrong-connection-in-pipeline/git/common-config/zuul.yaml
new file mode 100644
index 000000000..dcb90b7d4
--- /dev/null
+++ b/tests/fixtures/config/wrong-connection-in-pipeline/git/common-config/zuul.yaml
@@ -0,0 +1,33 @@
+- pipeline:
+    name: gate
+    manager: dependent
+    trigger:
+      review_gerrit:
+        - event: comment-added
+          approval:
+            - Approved: 1
+    success:
+      another_gerrit:
+        Verified: 2
+        submit: true
+    failure:
+      another_gerrit:
+        Verified: -2
+    start:
+      another_gerrit:
+        Verified: 0
+    precedence: high
+
+- job:
+    name: base
+    parent: null
+
+- job:
+    name: test-job
+    run: playbooks/test-job.yaml
+
+- project:
+    name: review.example.com/org/project
+    gate:
+      jobs:
+        - test-job
diff --git a/tests/fixtures/config/wrong-connection-in-pipeline/git/org_project/README b/tests/fixtures/config/wrong-connection-in-pipeline/git/org_project/README
new file mode 100644
index 000000000..9daeafb98
--- /dev/null
+++ b/tests/fixtures/config/wrong-connection-in-pipeline/git/org_project/README
@@ -0,0 +1 @@
+test
diff --git a/tests/fixtures/config/wrong-connection-in-pipeline/main.yaml b/tests/fixtures/config/wrong-connection-in-pipeline/main.yaml
new file mode 100644
index 000000000..bdcfc7c7c
--- /dev/null
+++ b/tests/fixtures/config/wrong-connection-in-pipeline/main.yaml
@@ -0,0 +1,11 @@
+- tenant:
+    name: tenant-one
+    source:
+      review_gerrit:
+        config-projects:
+          - common-config
+        untrusted-projects:
+          - org/project
+      another_gerrit:
+        untrusted-projects:
+          - org/project
diff --git a/tests/unit/test_gerrit.py b/tests/unit/test_gerrit.py
index 7b4d874ff..3524c022d 100644
--- a/tests/unit/test_gerrit.py
+++ b/tests/unit/test_gerrit.py
@@ -552,3 +552,35 @@ class TestPolling(ZuulTestCase):
         self.assertHistory([
             dict(name='tag-job', result='SUCCESS'),
         ])
+
+
+class TestWrongConnection(ZuulTestCase):
+    config_file = 'zuul-connections-multiple-gerrits.conf'
+    tenant_config_file = 'config/wrong-connection-in-pipeline/main.yaml'
+
+    def test_wrong_connection(self):
+        # Test if the wrong connection is configured in a gate pipeline
+
+        # Our system has two gerrits, and we have configured a gate
+        # pipeline to trigger on the "review_gerrit" connection, but
+        # report (and merge) via "another_gerrit".
+        A = self.fake_review_gerrit.addFakeChange('org/project', 'master', 'A')
+        A.addApproval('Code-Review', 2)
+        self.fake_review_gerrit.addEvent(A.addApproval('Approved', 1))
+
+        self.waitUntilSettled()
+
+        B = self.fake_review_gerrit.addFakeChange('org/project', 'master', 'B')
+        # Let's try this as if the change was merged (say, via another tenant).
+        B.setMerged()
+        B.addApproval('Code-Review', 2)
+        self.fake_review_gerrit.addEvent(B.addApproval('Approved', 1))
+
+        self.waitUntilSettled()
+
+        self.assertEqual(A.reported, 0)
+        self.assertEqual(B.reported, 0)
+        self.assertHistory([
+            dict(name='test-job', result='SUCCESS', changes='1,1'),
+            dict(name='test-job', result='SUCCESS', changes='2,1'),
+        ], ordered=False)
diff --git a/zuul/driver/gerrit/gerritconnection.py b/zuul/driver/gerrit/gerritconnection.py
index 0d0913789..85b628d90 100644
--- a/zuul/driver/gerrit/gerritconnection.py
+++ b/zuul/driver/gerrit/gerritconnection.py
@@ -917,6 +917,14 @@ class GerritConnection(BaseConnection):
 
         ref = 'refs/heads/' + change.branch
         self.log.debug("Waiting for %s to appear in git repo" % (change))
+        if not hasattr(change, '_ref_sha'):
+            self.log.error("Unable to confirm change %s in git repo: "
+                           "the change has not been reported; "
+                           "this pipeline may be misconfigured "
+                           "(check for multiple Gerrit connections)." %
+                           (change,))
+            return False
+
         if self._waitForRefSha(change.project, ref, change._ref_sha):
             self.log.debug("Change %s is in the git repo" %
                            (change))
diff --git a/zuul/driver/gerrit/gerritreporter.py b/zuul/driver/gerrit/gerritreporter.py
index 3f47d39b9..fbee0e0ec 100644
--- a/zuul/driver/gerrit/gerritreporter.py
+++ b/zuul/driver/gerrit/gerritreporter.py
@@ -51,6 +51,10 @@ class GerritReporter(BaseReporter):
         # the canonical hostname.
         if item.change.project.source.connection.canonical_hostname != \
                 self.connection.canonical_hostname:
+            log.debug("Not reporting %s as this Gerrit reporter "
+                      "is for %s and the change is from %s",
+                      item, self.connection.canonical_hostname,
+                      item.change.project.source.connection.canonical_hostname)
             return
 
         comments = self.getFileComments(item)