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
This commit is contained in:
Ian Wienand 2021-10-13 12:52:31 +11:00
parent 4a52c8053a
commit 618d98fdc2
4 changed files with 58 additions and 1 deletions

View File

@ -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.

View File

@ -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)

View File

@ -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(
"""

View File

@ -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: