From a45f70e9386432b3d1ee2b30bcaacfa9f1b76b89 Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Fri, 5 Apr 2024 10:12:11 +0900 Subject: [PATCH] redis: Add username Redis introduced ACL feature in 4.0.0, and this feature is supported by redis-py since 3.4.0[1]. When ACL is enabled, authentication requires username in addition to password. This also fixes how password is parsed from uri string. The parameter description has saied that password should be passed in the following format redis://[:]... but the actual format current code expects is redis://[]... which is not compliant with standard URL format. [1] https://github.com/redis/redis-py/commit/8df8cd54d135380ad8b3b8807a67a3e6915b0b49 Change-Id: I55f268eea13c7b45dceae85cfac86f3fb1562f1a --- .../redis-username-98a265f61fca6a1c.yaml | 9 ++ setup.cfg | 2 +- test-requirements.txt | 2 +- zaqar/conf/drivers_management_store_redis.py | 2 +- zaqar/conf/drivers_message_store_redis.py | 2 +- zaqar/storage/redis/driver.py | 34 +++--- zaqar/tests/unit/storage/test_impl_redis.py | 104 +++++++++++++++++- 7 files changed, 133 insertions(+), 22 deletions(-) create mode 100644 releasenotes/notes/redis-username-98a265f61fca6a1c.yaml diff --git a/releasenotes/notes/redis-username-98a265f61fca6a1c.yaml b/releasenotes/notes/redis-username-98a265f61fca6a1c.yaml new file mode 100644 index 000000000..581b32488 --- /dev/null +++ b/releasenotes/notes/redis-username-98a265f61fca6a1c.yaml @@ -0,0 +1,9 @@ +--- +features: + - | + Redis messaging store now supports authentication with username. + +deprecation: + - | + Password in redis uri will need to be prefixed by ':' in a future release. + Make sure all uri options are updated accordingly. diff --git a/setup.cfg b/setup.cfg index 741069e2c..70efd4b8e 100644 --- a/setup.cfg +++ b/setup.cfg @@ -96,7 +96,7 @@ oslo.policy.policies = mongodb = pymongo>=3.6.0 # Apache-2.0 redis = - redis>=3.0.0 # MIT + redis>=3.4.0 # MIT mysql = PyMySQL>=0.8.0 # MIT License diff --git a/test-requirements.txt b/test-requirements.txt index b82d4dcca..1409798c0 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,7 +1,7 @@ hacking>=6.1.0,<6.2.0 # Apache-2.0 # Backends -redis>=3.0.0 # MIT +redis>=3.4.0 # MIT pymongo>=3.6.0 # Apache-2.0 python-swiftclient>=3.10.1 # Apache-2.0 websocket-client>=0.44.0 # LGPLv2+ diff --git a/zaqar/conf/drivers_management_store_redis.py b/zaqar/conf/drivers_management_store_redis.py index 405b125a0..f929fea26 100644 --- a/zaqar/conf/drivers_management_store_redis.py +++ b/zaqar/conf/drivers_management_store_redis.py @@ -39,7 +39,7 @@ uri = cfg.StrOpt( 'string as "master=". Finally, to connect ' 'to a local instance of Redis over a unix socket, ' 'you may use the form ' - '"redis:[:password]@/path/to/redis.sock[?options]".' + '"redis://[:password]@/path/to/redis.sock[?options]".' ' In all forms, the "socket_timeout" option may be' 'specified in the query string. Its value is ' 'given in seconds. If not provided, ' diff --git a/zaqar/conf/drivers_message_store_redis.py b/zaqar/conf/drivers_message_store_redis.py index 689f4329d..3477fdf7d 100644 --- a/zaqar/conf/drivers_message_store_redis.py +++ b/zaqar/conf/drivers_message_store_redis.py @@ -40,7 +40,7 @@ uri = cfg.StrOpt( 'string as "master=". Finally, to connect ' 'to a local instance of Redis over a unix socket, ' 'you may use the form ' - '"redis:[:password]@/path/to/redis.sock[?options]".' + '"redis://[:password]@/path/to/redis.sock[?options]".' ' In all forms, the "socket_timeout" option may be' 'specified in the query string. Its value is ' 'given in seconds. If not provided, ' diff --git a/zaqar/storage/redis/driver.py b/zaqar/storage/redis/driver.py index 28c24724d..d3f09032c 100644 --- a/zaqar/storage/redis/driver.py +++ b/zaqar/storage/redis/driver.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +from oslo_log import log as logging from osprofiler import profiler import redis import redis.sentinel @@ -34,9 +35,11 @@ STRATEGY_TCP = 1 STRATEGY_UNIX = 2 STRATEGY_SENTINEL = 3 +LOG = logging.getLogger(__name__) + class ConnectionURI(object): - def __init__(self, uri): # noqa: C901 + def __init__(self, uri): # TODO(prashanthr_): Add SSL support try: parsed_url = urllib.parse.urlparse(uri) @@ -46,21 +49,23 @@ class ConnectionURI(object): if parsed_url.scheme != 'redis': raise errors.ConfigurationError(_('Invalid scheme in Redis URI')) - # NOTE(kgriffs): Python 2.6 has a bug that causes the - # query string to be appended to the path when given a - # hostless URL. path = parsed_url.path - if '?' in path: - path, sep, query = path.partition('?') - else: - query = parsed_url.query - # NOTE(gengchc2): Redis connection support password configure. - self.password = None - if '@' in path: - self.password, sep, path = path.partition('@') + query = parsed_url.query + # NOTE(tkajinam): Replace '' by None + self.password = parsed_url.password or None + self.username = parsed_url.username or None + netloc = parsed_url.netloc if '@' in netloc: - self.password, sep, netloc = netloc.partition('@') + cred, sep, netloc = netloc.partition('@') + + if self.username and not self.password: + # NOTE(tkajinam): This is kept for backword compatibility but + # should be removed after 2025.1 + LOG.warning('Credential in redis uri does not contain \':\'. ' + 'Make sure that \':\' is added before password.') + self.password = self.username + self.username = None query_params = dict(urllib.parse.parse_qsl(query)) @@ -315,6 +320,7 @@ def _get_redis_client(driver): sentinel = redis.sentinel.Sentinel( connection_uri.sentinels, db=connection_uri.dbid, + username=connection_uri.username, password=connection_uri.password, socket_timeout=connection_uri.socket_timeout) @@ -328,11 +334,13 @@ def _get_redis_client(driver): host=connection_uri.hostname, port=connection_uri.port, db=connection_uri.dbid, + username=connection_uri.username, password=connection_uri.password, socket_timeout=connection_uri.socket_timeout) else: return redis.Redis( unix_socket_path=connection_uri.unix_socket_path, db=connection_uri.dbid, + username=connection_uri.username, password=connection_uri.password, socket_timeout=connection_uri.socket_timeout) diff --git a/zaqar/tests/unit/storage/test_impl_redis.py b/zaqar/tests/unit/storage/test_impl_redis.py index e02c4188a..63fac9f5a 100644 --- a/zaqar/tests/unit/storage/test_impl_redis.py +++ b/zaqar/tests/unit/storage/test_impl_redis.py @@ -233,42 +233,99 @@ class RedisDriverTest(testing.TestBase): self.assertEqual(driver.STRATEGY_TCP, uri.strategy) self.assertEqual(6379, uri.port) self.assertEqual(0.1, uri.socket_timeout) + self.assertEqual(0, uri.dbid) + self.assertIsNone(uri.username) + self.assertIsNone(uri.password) uri = driver.ConnectionURI('redis://example.com:7777') self.assertEqual(driver.STRATEGY_TCP, uri.strategy) self.assertEqual(7777, uri.port) + self.assertEqual(0.1, uri.socket_timeout) + self.assertEqual(0, uri.dbid) + self.assertIsNone(uri.username) + self.assertIsNone(uri.password) uri = driver.ConnectionURI( 'redis://example.com:7777?socket_timeout=1') self.assertEqual(driver.STRATEGY_TCP, uri.strategy) self.assertEqual(7777, uri.port) self.assertEqual(1.0, uri.socket_timeout) + self.assertEqual(0, uri.dbid) + self.assertIsNone(uri.username) + self.assertIsNone(uri.password) uri = driver.ConnectionURI( - 'redis://test123@example.com:7777?socket_timeout=1&dbid=5') + 'redis://:test123@example.com:7777?socket_timeout=1&dbid=5') self.assertEqual(driver.STRATEGY_TCP, uri.strategy) self.assertEqual(7777, uri.port) self.assertEqual(1.0, uri.socket_timeout) self.assertEqual(5, uri.dbid) + self.assertIsNone(uri.username) + self.assertEqual('test123', uri.password) + + # NOTE(tkajinam): Test fallback for backword compatibility + uri = driver.ConnectionURI('redis://test123@example.com') + self.assertEqual(driver.STRATEGY_TCP, uri.strategy) + self.assertEqual(6379, uri.port) + self.assertEqual(0.1, uri.socket_timeout) + self.assertEqual(0, uri.dbid) + self.assertIsNone(uri.username) + self.assertEqual('test123', uri.password) + + uri = driver.ConnectionURI( + 'redis://default:test123@example.com') + self.assertEqual(driver.STRATEGY_TCP, uri.strategy) + self.assertEqual(6379, uri.port) + self.assertEqual(0.1, uri.socket_timeout) + self.assertEqual(0, uri.dbid) + self.assertEqual('default', uri.username) self.assertEqual('test123', uri.password) def test_connection_uri_unix_socket(self): - uri = driver.ConnectionURI('redis:/tmp/redis.sock') + uri = driver.ConnectionURI('redis:///tmp/redis.sock') self.assertEqual(driver.STRATEGY_UNIX, uri.strategy) self.assertEqual('/tmp/redis.sock', uri.unix_socket_path) self.assertEqual(0.1, uri.socket_timeout) + self.assertEqual(0, uri.dbid) + self.assertIsNone(uri.username) + self.assertIsNone(uri.password) - uri = driver.ConnectionURI('redis:/tmp/redis.sock?socket_timeout=1.5') + uri = driver.ConnectionURI( + 'redis:///tmp/redis.sock?socket_timeout=1.5') self.assertEqual(driver.STRATEGY_UNIX, uri.strategy) self.assertEqual('/tmp/redis.sock', uri.unix_socket_path) self.assertEqual(1.5, uri.socket_timeout) + self.assertEqual(0, uri.dbid) + self.assertIsNone(uri.username) + self.assertIsNone(uri.password) uri = driver.ConnectionURI( - 'redis:test123@/tmp/redis.sock?socket_timeout=1.5&dbid=5') + 'redis://:test123@/tmp/redis.sock?' + 'socket_timeout=1.5&dbid=5') self.assertEqual(driver.STRATEGY_UNIX, uri.strategy) self.assertEqual('/tmp/redis.sock', uri.unix_socket_path) self.assertEqual(1.5, uri.socket_timeout) self.assertEqual(5, uri.dbid) + self.assertIsNone(uri.username) + self.assertEqual('test123', uri.password) + + # NOTE(tkajinam): Test fallback for backword compatibility + uri = driver.ConnectionURI( + 'redis://test123@/tmp/redis.sock') + self.assertEqual(driver.STRATEGY_UNIX, uri.strategy) + self.assertEqual('/tmp/redis.sock', uri.unix_socket_path) + self.assertEqual(0.1, uri.socket_timeout) + self.assertEqual(0, uri.dbid) + self.assertIsNone(uri.username) + self.assertEqual('test123', uri.password) + + uri = driver.ConnectionURI( + 'redis://default:test123@/tmp/redis.sock') + self.assertEqual(driver.STRATEGY_UNIX, uri.strategy) + self.assertEqual('/tmp/redis.sock', uri.unix_socket_path) + self.assertEqual(0.1, uri.socket_timeout) + self.assertEqual(0, uri.dbid) + self.assertEqual('default', uri.username) self.assertEqual('test123', uri.password) def test_connection_uri_sentinel(self): @@ -277,18 +334,27 @@ class RedisDriverTest(testing.TestBase): self.assertEqual([('s1', 26379)], uri.sentinels) self.assertEqual('dumbledore', uri.master) self.assertEqual(0.1, uri.socket_timeout) + self.assertEqual(0, uri.dbid) + self.assertIsNone(uri.username) + self.assertIsNone(uri.password) uri = driver.ConnectionURI('redis://s1,s2?master=dumbledore') self.assertEqual(driver.STRATEGY_SENTINEL, uri.strategy) self.assertEqual([('s1', 26379), ('s2', 26379)], uri.sentinels) self.assertEqual('dumbledore', uri.master) self.assertEqual(0.1, uri.socket_timeout) + self.assertEqual(0, uri.dbid) + self.assertIsNone(uri.username) + self.assertIsNone(uri.password) uri = driver.ConnectionURI('redis://s1:26389,s1?master=dumbledore') self.assertEqual(driver.STRATEGY_SENTINEL, uri.strategy) self.assertEqual([('s1', 26389), ('s1', 26379)], uri.sentinels) self.assertEqual('dumbledore', uri.master) self.assertEqual(0.1, uri.socket_timeout) + self.assertEqual(0, uri.dbid) + self.assertIsNone(uri.username) + self.assertIsNone(uri.password) uri = driver.ConnectionURI( 'redis://[::1]:26389,[::2]?master=dumbledore') @@ -296,6 +362,9 @@ class RedisDriverTest(testing.TestBase): self.assertEqual([('::1', 26389), ('::2', 26379)], uri.sentinels) self.assertEqual('dumbledore', uri.master) self.assertEqual(0.1, uri.socket_timeout) + self.assertEqual(0, uri.dbid) + self.assertIsNone(uri.username) + self.assertIsNone(uri.password) uri = driver.ConnectionURI( 'redis://s1?master=dumbledore&socket_timeout=0.5') @@ -303,14 +372,39 @@ class RedisDriverTest(testing.TestBase): self.assertEqual([('s1', 26379)], uri.sentinels) self.assertEqual('dumbledore', uri.master) self.assertEqual(0.5, uri.socket_timeout) + self.assertEqual(0, uri.dbid) + self.assertIsNone(uri.username) + self.assertIsNone(uri.password) uri = driver.ConnectionURI( - 'redis://test123@s1?master=dumbledore&socket_timeout=0.5&dbid=5') + 'redis://:test123@s1?master=dumbledore&socket_timeout=0.5&dbid=5') self.assertEqual(driver.STRATEGY_SENTINEL, uri.strategy) self.assertEqual([('s1', 26379)], uri.sentinels) self.assertEqual('dumbledore', uri.master) self.assertEqual(0.5, uri.socket_timeout) self.assertEqual(5, uri.dbid) + self.assertIsNone(uri.username) + self.assertEqual('test123', uri.password) + + # NOTE(tkajinam): Test fallback for backword compatibility + uri = driver.ConnectionURI( + 'redis://test123@s1?master=dumbledore') + self.assertEqual(driver.STRATEGY_SENTINEL, uri.strategy) + self.assertEqual([('s1', 26379)], uri.sentinels) + self.assertEqual('dumbledore', uri.master) + self.assertEqual(0.1, uri.socket_timeout) + self.assertEqual(0, uri.dbid) + self.assertIsNone(uri.username) + self.assertEqual('test123', uri.password) + + uri = driver.ConnectionURI( + 'redis://default:test123@s1?master=dumbledore') + self.assertEqual(driver.STRATEGY_SENTINEL, uri.strategy) + self.assertEqual([('s1', 26379)], uri.sentinels) + self.assertEqual('dumbledore', uri.master) + self.assertEqual(0.1, uri.socket_timeout) + self.assertEqual(0, uri.dbid) + self.assertEqual('default', uri.username) self.assertEqual('test123', uri.password)