Merge "gerrit: trim messages to "human length""

releasenotes/notes/gerrit-long-message-truncate-320af6e43717651d.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Gerrit driver: Zuul error responses could overflow the default
+    message length with large configurations leading to Gerrit
+    rejecting the comment (and hence no notification from Zuul on the
+    change).  Such messages are now truncated to a safe length.

tests/base.py

@@ -1068,6 +1068,11 @@ class GerritWebServer(object):
                     return self._404()
 
                 message = data['message']
+                b_len = len(message.encode('utf-8'))
+                if b_len > gerritconnection.GERRIT_HUMAN_MESSAGE_LIMIT:
+                    self.send_response(400, message='Message length exceeded')
+                    self.end_headers()
+                    return
                 labels = data.get('labels', {})
                 comments = data.get('robot_comments', data.get('comments', {}))
                 tag = data.get('tag', None)

tests/unit/test_gerrit.py

@@ -184,6 +184,28 @@ class TestGerritWeb(ZuulTestCase):
                                        'username': 'jenkins'}}
         )
 
+    def test_message_too_long(self):
+        in_repo_conf = textwrap.dedent(
+            """
+            - job:
+                name: garbage-job
+                %s
+                garbage: True
+            """
+        ) % ('\n' * 16384)
+
+        file_dict = {'.zuul.yaml': in_repo_conf}
+        A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
+                                           files=file_dict)
+        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+
+        self.assertEqual(A.patchsets[0]['approvals'][0]['value'], "-1")
+        self.assertEqual(A.patchsets[0]['approvals'][0]['__tag'],
+                         "autogenerated:zuul:check")
+        self.assertIn('... (truncated)',
+                      A.messages[0])
+
     def test_dependent_dynamic_line_comment(self):
         in_repo_conf = textwrap.dedent(
             """

zuul/driver/gerrit/gerritconnection.py

@@ -52,6 +52,17 @@ from zuul.zk.event_queues import ConnectionEventQueue, EventReceiverElection
 # HTTP timeout in seconds
 TIMEOUT = 30
 
+# commentSizeLimit default set by Gerrit.  Gerrit is a bit
+# vague about what this means, it says
+#
+#  Comments which exceed this size will be rejected ... Size
+#  computation is approximate and may be off by roughly 1% ...
+#  Default is 16k
+#
+# This magic number is int((16 << 10) * 0.98).  Robot comments
+# are accounted for separately.
+GERRIT_HUMAN_MESSAGE_LIMIT = 16056
+
 
 class HTTPConflictException(Exception):
     message = "Received response 409"
@@ -1133,8 +1144,8 @@ class GerritConnection(ZKChangeCacheMixin, BaseConnection):
 
     def review_ssh(self, item, message, submit, labels, checks_api,
                    file_comments, zuul_event_id=None):
+        log = get_annotated_logger(self.log, zuul_event_id)
         if checks_api:
-            log = get_annotated_logger(self.log, zuul_event_id)
             log.error("Zuul is configured to report to the checks API, "
                       "but no HTTP password is present for the connection "
                       "in the configuration file.")
@@ -1142,6 +1153,12 @@ class GerritConnection(ZKChangeCacheMixin, BaseConnection):
         project = change.project.name
         cmd = 'gerrit review --project %s' % project
         if message:
+            b_len = len(message.encode('utf-8'))
+            if b_len >= GERRIT_HUMAN_MESSAGE_LIMIT:
+                log.info("Message truncated %d > %d" %
+                         (b_len, GERRIT_HUMAN_MESSAGE_LIMIT))
+                message = ("%s... (truncated)" %
+                           message[:GERRIT_HUMAN_MESSAGE_LIMIT - 20])
             cmd += ' --message %s' % shlex.quote(message)
         if submit:
             cmd += ' --submit'
@@ -1207,6 +1224,12 @@ class GerritConnection(ZKChangeCacheMixin, BaseConnection):
                     checks_api, file_comments, zuul_event_id=None):
         change = item.change
         log = get_annotated_logger(self.log, zuul_event_id)
+        b_len = len(message.encode('utf-8'))
+        if b_len >= GERRIT_HUMAN_MESSAGE_LIMIT:
+            log.info("Message truncated %d > %d" %
+                     (b_len, GERRIT_HUMAN_MESSAGE_LIMIT))
+            message = ("%s... (truncated)" %
+                       message[:GERRIT_HUMAN_MESSAGE_LIMIT - 20])
         data = dict(message=message,
                     strict_labels=False)
         if change.is_current_patchset: