From 037dee19ed9ab8023b0da0e2734bb54e88cb4cdb Mon Sep 17 00:00:00 2001
From: Mehdi Abaakouk <sileht@redhat.com>
Date: Thu, 23 Feb 2017 09:37:16 +0100
Subject: [PATCH] Validate the transport url query string
When driver load we allow to override option unrelated to the driver and
to set option useless for the driver.
This change validates the query string when the driver load to report as
soon as possible invalid options. And allow to override only option
of the driver option group (ie: [oslo_messaging_<driver_name>].
Related-bug: #1666903
Change-Id: Iaf23f773279c10bf37d545883ada7c2f6a9ffbbf
---
oslo_messaging/_drivers/common.py | 12 ++++++--
oslo_messaging/_drivers/impl_amqp1.py | 2 +-
oslo_messaging/_drivers/impl_pika.py | 2 +-
oslo_messaging/_drivers/impl_rabbit.py | 2 +-
oslo_messaging/_drivers/kafka_options.py | 2 +-
.../_drivers/zmq_driver/zmq_options.py | 2 +-
.../tests/test_config_opts_proxy.py | 30 ++++++++++++-------
7 files changed, 35 insertions(+), 17 deletions(-)
diff --git a/oslo_messaging/_drivers/common.py b/oslo_messaging/_drivers/common.py
index 7b0e4028b..9b35c5dd9 100644
--- a/oslo_messaging/_drivers/common.py
+++ b/oslo_messaging/_drivers/common.py
@@ -460,13 +460,21 @@ class ConfigOptsProxy(collections.Mapping):
and valid) override corresponding values from the configuration.
"""
- def __init__(self, conf, url):
+ def __init__(self, conf, url, group):
self._conf = conf
self._url = url
+ self._group = group
+ self._validate_query()
+
+ def _validate_query(self):
+ for name in self._url.query:
+ self.GroupAttrProxy(self._conf, self._group,
+ self._conf[self._group],
+ self._url)[name]
def __getattr__(self, name):
value = getattr(self._conf, name)
- if isinstance(value, self._conf.GroupAttr):
+ if isinstance(value, self._conf.GroupAttr) and name == self._group:
return self.GroupAttrProxy(self._conf, name, value, self._url)
return value
diff --git a/oslo_messaging/_drivers/impl_amqp1.py b/oslo_messaging/_drivers/impl_amqp1.py
index 8c17a9827..e5eb03669 100644
--- a/oslo_messaging/_drivers/impl_amqp1.py
+++ b/oslo_messaging/_drivers/impl_amqp1.py
@@ -200,7 +200,7 @@ class ProtonDriver(base.BaseDriver):
title='AMQP 1.0 driver options')
conf.register_group(opt_group)
conf.register_opts(opts.amqp1_opts, group=opt_group)
- conf = common.ConfigOptsProxy(conf, url)
+ conf = common.ConfigOptsProxy(conf, url, opt_group.name)
self._hosts = url.hosts
self._conf = conf
diff --git a/oslo_messaging/_drivers/impl_pika.py b/oslo_messaging/_drivers/impl_pika.py
index 2fe1b870a..0d413b41a 100644
--- a/oslo_messaging/_drivers/impl_pika.py
+++ b/oslo_messaging/_drivers/impl_pika.py
@@ -144,7 +144,7 @@ class PikaDriver(base.BaseDriver):
conf.register_opts(message_opts, group=opt_group)
conf.register_opts(rpc_opts, group=opt_group)
conf.register_opts(notification_opts, group=opt_group)
- conf = common.ConfigOptsProxy(conf, url)
+ conf = common.ConfigOptsProxy(conf, url, opt_group.name)
self._pika_engine = pika_drv_engine.PikaEngine(
conf, url, default_exchange, allowed_remote_exmods
diff --git a/oslo_messaging/_drivers/impl_rabbit.py b/oslo_messaging/_drivers/impl_rabbit.py
index 6f7afd529..a32c5ccac 100644
--- a/oslo_messaging/_drivers/impl_rabbit.py
+++ b/oslo_messaging/_drivers/impl_rabbit.py
@@ -1335,7 +1335,7 @@ class RabbitDriver(amqpdriver.AMQPDriverBase):
conf.register_opts(rabbit_opts, group=opt_group)
conf.register_opts(rpc_amqp.amqp_opts, group=opt_group)
conf.register_opts(base.base_opts, group=opt_group)
- conf = rpc_common.ConfigOptsProxy(conf, url)
+ conf = rpc_common.ConfigOptsProxy(conf, url, opt_group.name)
self.missing_destination_retry_timeout = (
conf.oslo_messaging_rabbit.kombu_missing_consumer_retry_timeout)
diff --git a/oslo_messaging/_drivers/kafka_options.py b/oslo_messaging/_drivers/kafka_options.py
index fa3b06aef..0bdcde532 100644
--- a/oslo_messaging/_drivers/kafka_options.py
+++ b/oslo_messaging/_drivers/kafka_options.py
@@ -59,4 +59,4 @@ def register_opts(conf, url):
title='Kafka driver options')
conf.register_group(opt_group)
conf.register_opts(KAFKA_OPTS, group=opt_group)
- return common.ConfigOptsProxy(conf, url)
+ return common.ConfigOptsProxy(conf, url, opt_group.name)
diff --git a/oslo_messaging/_drivers/zmq_driver/zmq_options.py b/oslo_messaging/_drivers/zmq_driver/zmq_options.py
index 529ec94d4..426570409 100644
--- a/oslo_messaging/_drivers/zmq_driver/zmq_options.py
+++ b/oslo_messaging/_drivers/zmq_driver/zmq_options.py
@@ -210,4 +210,4 @@ def register_opts(conf, url):
conf.register_opts(zmq_opts, group=opt_group)
conf.register_opts(server._pool_opts)
conf.register_opts(base.base_opts)
- return common.ConfigOptsProxy(conf, url)
+ return common.ConfigOptsProxy(conf, url, opt_group.name)
diff --git a/oslo_messaging/tests/test_config_opts_proxy.py b/oslo_messaging/tests/test_config_opts_proxy.py
index 0733c5363..0dee66970 100644
--- a/oslo_messaging/tests/test_config_opts_proxy.py
+++ b/oslo_messaging/tests/test_config_opts_proxy.py
@@ -27,7 +27,6 @@ class TestConfigOptsProxy(test_utils.BaseTestCase):
self.config(rabbit_retry_interval=1,
rabbit_qos_prefetch_count=0,
rabbit_max_retries=3,
- kombu_reconnect_delay=5.0,
group=group)
dummy_opts = [cfg.ListOpt('list_str', item_type=types.String(),
default=[]),
@@ -40,14 +39,12 @@ class TestConfigOptsProxy(test_utils.BaseTestCase):
url = transport.TransportURL.parse(
self.conf, "rabbit:///"
"?rabbit_qos_prefetch_count=2"
- "&unknown_opt=4"
- "&kombu_reconnect_delay=invalid_value"
"&list_str=1&list_str=2&list_str=3"
"&list_int=1&list_int=2&list_int=3"
"&dict=x:1&dict=y:2&dict=z:3"
"&bool=True"
)
- conf = drv_cmn.ConfigOptsProxy(self.conf, url)
+ conf = drv_cmn.ConfigOptsProxy(self.conf, url, group)
self.assertRaises(cfg.NoSuchOptError,
conf.__getattr__,
'unknown_group')
@@ -57,15 +54,28 @@ class TestConfigOptsProxy(test_utils.BaseTestCase):
self.assertEqual(2,
conf.oslo_messaging_rabbit.rabbit_qos_prefetch_count)
self.assertEqual(3, conf.oslo_messaging_rabbit.rabbit_max_retries)
- self.assertRaises(cfg.NoSuchOptError,
- conf.oslo_messaging_rabbit.__getattr__,
- 'unknown_opt')
- self.assertRaises(ValueError,
- conf.oslo_messaging_rabbit.__getattr__,
- 'kombu_reconnect_delay')
self.assertEqual(['1', '2', '3'], conf.oslo_messaging_rabbit.list_str)
self.assertEqual([1, 2, 3], conf.oslo_messaging_rabbit.list_int)
self.assertEqual({'x': '1', 'y': '2', 'z': '3'},
conf.oslo_messaging_rabbit.dict)
self.assertEqual(True, conf.oslo_messaging_rabbit.bool)
self.assertEqual('default', conf.oslo_messaging_rabbit.str)
+
+ def test_not_in_group(self):
+ group = 'oslo_messaging_rabbit'
+ url = transport.TransportURL.parse(
+ self.conf, "rabbit:///?unknown_opt=4"
+ )
+ self.assertRaises(cfg.NoSuchOptError,
+ drv_cmn.ConfigOptsProxy,
+ self.conf, url, group)
+
+ def test_invalid_value(self):
+ group = 'oslo_messaging_rabbit'
+ self.config(kombu_reconnect_delay=5.0,
+ group=group)
+ url = transport.TransportURL.parse(
+ self.conf, "rabbit:///?kombu_reconnect_delay=invalid_value"
+ )
+ self.assertRaises(ValueError, drv_cmn.ConfigOptsProxy, self.conf,
+ url, group)