From 618d98fdc2134c28fa8013bbe61e5fcd5fa826fc Mon Sep 17 00:00:00 2001 From: Ian Wienand Date: Wed, 13 Oct 2021 12:52:31 +1100 Subject: [PATCH] gerrit: trim messages to "human length" The "message" field of a gerrit response (i.e. the bit that has "Zuul encountered a syntax error while parsing its configuration ...") has a limit in Gerrit of "change.commentSizeLimit" which is by default 16KiB [1]. If you have a syntax error in a very long section, say the middle of the project: section of OpenDev system-config, you can actually overflow this length. Gerrit rejects the update and as an end-user you don't get any clue why. This truncates the message in the gerrit driver if it's going to overflow. [1] Note change.robotCommentSizeLimit is larger, but that's not how syntax messages are reported. Change-Id: I2f14e734ef5f9f203b14556c7d2c8ed1ad052319 --- ...ong-message-truncate-320af6e43717651d.yaml | 7 ++++++ tests/base.py | 5 ++++ tests/unit/test_gerrit.py | 22 ++++++++++++++++ zuul/driver/gerrit/gerritconnection.py | 25 ++++++++++++++++++- 4 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/gerrit-long-message-truncate-320af6e43717651d.yaml diff --git a/releasenotes/notes/gerrit-long-message-truncate-320af6e43717651d.yaml b/releasenotes/notes/gerrit-long-message-truncate-320af6e43717651d.yaml new file mode 100644 index 0000000000..3a33dfce3d --- /dev/null +++ b/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. diff --git a/tests/base.py b/tests/base.py index c09cd99c97..eb6783c3ec 100644 --- a/tests/base.py +++ b/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) diff --git a/tests/unit/test_gerrit.py b/tests/unit/test_gerrit.py index 79b2c66e57..cd19d80576 100644 --- a/tests/unit/test_gerrit.py +++ b/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( """ diff --git a/zuul/driver/gerrit/gerritconnection.py b/zuul/driver/gerrit/gerritconnection.py index 1db8b4ec2a..b39ad0582a 100644 --- a/zuul/driver/gerrit/gerritconnection.py +++ b/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: