From 464baf560ddd7ba33134ccd584461fb021344e1b Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Mar 05 2020 15:11:00 +0000 Subject: [PATCH 1/2] limit size of extra field in proton msgs Fixes: https://pagure.io/koji/issue/2017 --- diff --git a/plugins/hub/protonmsg.conf b/plugins/hub/protonmsg.conf index b664196..97c2679 100644 --- a/plugins/hub/protonmsg.conf +++ b/plugins/hub/protonmsg.conf @@ -5,3 +5,9 @@ cacert = /etc/koji-hub/plugins/ca.pem topic_prefix = koji connect_timeout = 10 send_timeout = 60 + +[message] +# limit build.extra size which is sent to bus +# if field is longer (json.dumps), ignore it +# default value is 0 - unlimited size +extra_limit = 0 diff --git a/plugins/hub/protonmsg.py b/plugins/hub/protonmsg.py index 2c1874a..725ae5b 100644 --- a/plugins/hub/protonmsg.py +++ b/plugins/hub/protonmsg.py @@ -135,6 +135,20 @@ class TimeoutHandler(MessagingHandler): self.timeout_task = None +def _strip_extra(buildinfo): + """If extra_limit is configured, compare extra's size and drop it, + if it is over""" + global CONFIG + if not CONFIG: + CONFIG = koji.read_config_files([(CONFIG_FILE, True)]) + if CONFIG.has_option('message', 'extra_limit'): + extra_limit = CONFIG.getint('message', 'extra_limit') + extra_size = len(json_serialize(buildinfo.get('extra', {}))) + if extra_limit and extra_size > extra_limit: + del buildinfo['extra'] + return buildinfo + + def json_serialize(o): """JSON helper to encode otherwise unserializable data types""" if isinstance(o, set): @@ -191,6 +205,7 @@ def prep_build_state_change(cbtype, *args, **kws): old = koji.BUILD_STATES[old] new = koji.BUILD_STATES[kws['new']] address = 'build.' + new.lower() + kws['info'] = _strip_extra(kws['info']) props = {'type': cbtype[4:], 'name': kws['info']['name'], 'version': kws['info']['version'], @@ -204,6 +219,7 @@ def prep_build_state_change(cbtype, *args, **kws): @convert_datetime @callback('postImport') def prep_import(cbtype, *args, **kws): + kws['build'] = _strip_extra(kws['build']) address = 'import.' + kws['type'] props = {'type': cbtype[4:], 'importType': kws['type'], @@ -218,6 +234,7 @@ def prep_import(cbtype, *args, **kws): def prep_rpm_sign(cbtype, *args, **kws): if not kws['sigkey']: return + kws['build'] = _strip_extra(kws['build']) address = 'sign.rpm' props = {'type': cbtype[4:], 'sigkey': kws['sigkey'], @@ -232,12 +249,12 @@ def prep_rpm_sign(cbtype, *args, **kws): def _prep_tag_msg(address, cbtype, kws): - build = kws['build'] + kws['build'] = _strip_extra(kws['build']) props = {'type': cbtype[4:], 'tag': kws['tag']['name'], - 'name': build['name'], - 'version': build['version'], - 'release': build['release'], + 'name': kws['build']['name'], + 'version': kws['build']['version'], + 'release': kws['build']['release'], 'user': kws['user']['name']} queue_msg(address, props, kws) diff --git a/tests/test_plugins/test_protonmsg.py b/tests/test_plugins/test_protonmsg.py index 04d63ab..1b5c90e 100644 --- a/tests/test_plugins/test_protonmsg.py +++ b/tests/test_plugins/test_protonmsg.py @@ -12,9 +12,27 @@ from koji.context import context from six.moves.configparser import ConfigParser, SafeConfigParser class TestProtonMsg(unittest.TestCase): + def setUp(self): + self.conf = tempfile.NamedTemporaryFile() + self.conf.write(six.b("""[broker] +urls = amqps://broker1.example.com:5671 amqps://broker2.example.com:5671 +cert = /etc/koji-hub/plugins/client.pem +cacert = /etc/koji-hub/plugins/ca.pem +topic_prefix = koji +connect_timeout = 10 +send_timeout = 60 + +[message] +extra_limit = 2048 +""")) + self.conf.flush() + protonmsg.CONFIG_FILE = self.conf.name + protonmsg.CONFIG = None + def tearDown(self): if hasattr(context, 'protonmsg_msgs'): del context.protonmsg_msgs + del self.conf def assertMsg(self, topic, body=None, **kws): self.assertTrue(hasattr(context, 'protonmsg_msgs')) @@ -191,18 +209,6 @@ class TestProtonMsg(unittest.TestCase): @patch('logging.getLogger') def test_send_queued_msgs_fail(self, getLogger, Container): context.protonmsg_msgs = [('test.topic', {'testheader': 1}, 'test body')] - conf = tempfile.NamedTemporaryFile() - conf.write(six.b("""[broker] -urls = amqps://broker1.example.com:5671 amqps://broker2.example.com:5671 -cert = /etc/koji-hub/plugins/client.pem -cacert = /etc/koji-hub/plugins/ca.pem -topic_prefix = koji -connect_timeout = 10 -send_timeout = 60 -""")) - conf.flush() - protonmsg.CONFIG_FILE = conf.name - protonmsg.CONFIG = None protonmsg.send_queued_msgs('postCommit') log = getLogger.return_value self.assertEqual(log.debug.call_count, 2) @@ -215,18 +221,6 @@ send_timeout = 60 @patch('logging.getLogger') def test_send_queued_msgs_success(self, getLogger, Container): context.protonmsg_msgs = [('test.topic', {'testheader': 1}, 'test body')] - conf = tempfile.NamedTemporaryFile() - conf.write(six.b("""[broker] -urls = amqps://broker1.example.com:5671 amqps://broker2.example.com:5671 -cert = /etc/koji-hub/plugins/client.pem -cacert = /etc/koji-hub/plugins/ca.pem -topic_prefix = koji -connect_timeout = 10 -send_timeout = 60 -""")) - conf.flush() - protonmsg.CONFIG_FILE = conf.name - protonmsg.CONFIG = None def clear_msgs(): del context.protonmsg_msgs[:] Container.return_value.run.side_effect = clear_msgs From eeeb153b1c374b82f378e0e781d3035654930ca0 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Mar 23 2020 13:57:23 +0000 Subject: [PATCH 2/2] use buildinfo copy --- diff --git a/plugins/hub/protonmsg.py b/plugins/hub/protonmsg.py index 725ae5b..68f5399 100644 --- a/plugins/hub/protonmsg.py +++ b/plugins/hub/protonmsg.py @@ -145,6 +145,7 @@ def _strip_extra(buildinfo): extra_limit = CONFIG.getint('message', 'extra_limit') extra_size = len(json_serialize(buildinfo.get('extra', {}))) if extra_limit and extra_size > extra_limit: + buildinfo = buildinfo.copy() del buildinfo['extra'] return buildinfo