diff --git a/docs/user_guide/installation.md b/docs/user_guide/installation.md index aa0e3065..6548b81e 100644 --- a/docs/user_guide/installation.md +++ b/docs/user_guide/installation.md @@ -179,6 +179,7 @@ index = SearchIndex.from_yaml( The Sentinel URL format supports: - Multiple sentinel hosts (comma-separated) +- IPv6 literals in brackets, for example `redis+sentinel://[::1]:26380,[2001:db8::1]/mymaster`; omitted ports default to `26379` - Optional authentication (username:password) - Service name (defaults to `mymaster` if not specified) - Optional database number (defaults to 0) diff --git a/redisvl/redis/connection.py b/redisvl/redis/connection.py index fba7f194..aa57fbc5 100644 --- a/redisvl/redis/connection.py +++ b/redisvl/redis/connection.py @@ -1048,17 +1048,24 @@ def _parse_sentinel_url( ... ) ([('host1', 26379), ('host2', 26380)], 'mymaster', '0', 'user', 'pass') """ - parsed_url = urlparse(url) - hosts_part = parsed_url.netloc.split("@")[-1] - sentinel_hosts = hosts_part.split(",") + # A comma-separated authority is not a standard URL netloc, especially + # when it mixes hostnames and IPv6 literals. Parse each node separately. + scheme, _, remainder = url.partition("://") + authority = remainder.split("/", 1)[0].split("?", 1)[0].split("#", 1)[0] + userinfo, separator, hosts_part = authority.rpartition("@") + parsed_url = urlparse( + f"{scheme}://{userinfo}{separator}localhost{remainder[len(authority):]}" + ) sentinel_list = [] - for host in sentinel_hosts: - host_parts = host.split(":") - if len(host_parts) == 2: - sentinel_list.append((host_parts[0], int(host_parts[1]))) - else: - sentinel_list.append((host_parts[0], 26379)) + for host in hosts_part.split(","): + node = urlparse(f"//{host}") + hostname, port = node.hostname, node.port + if not hostname or host.endswith(":"): + raise ValueError("Sentinel nodes require a hostname and a valid port") + sentinel_list.append( + (unquote(hostname), port if port is not None else 26379) + ) service_name = "mymaster" db = None diff --git a/tests/unit/test_sentinel_url.py b/tests/unit/test_sentinel_url.py index 4576ec0a..746a8a32 100644 --- a/tests/unit/test_sentinel_url.py +++ b/tests/unit/test_sentinel_url.py @@ -14,7 +14,7 @@ the sync SentinelConnectionPool, causing runtime failures. """ -from unittest.mock import MagicMock, patch +from unittest.mock import AsyncMock, MagicMock, patch from urllib.parse import quote import pytest @@ -23,6 +23,91 @@ from redisvl.redis.connection import RedisConnectionFactory +@pytest.mark.parametrize( + "hosts, expected", + [ + ("localhost:26380", [("localhost", 26380)]), + ("127.0.0.1,host2:26380", [("127.0.0.1", 26379), ("host2", 26380)]), + ("[::1]:26380", [("::1", 26380)]), + ("[2001:db8::1]", [("2001:db8::1", 26379)]), + ( + "[2001:db8::1]:26380,[::1]:26381", + [("2001:db8::1", 26380), ("::1", 26381)], + ), + ("host1:26380,[::1]", [("host1", 26380), ("::1", 26379)]), + ("[::1],127.0.0.1:26380", [("::1", 26379), ("127.0.0.1", 26380)]), + ("host1,[::1]:0", [("host1", 26379), ("::1", 0)]), + ("[fe80::1%25lo0]", [("fe80::1%lo0", 26379)]), + ("[fe80::1%lo0]", [("fe80::1%lo0", 26379)]), + ], +) +def test_parse_sentinel_hosts(hosts, expected): + assert RedisConnectionFactory._parse_sentinel_url( + f"redis+sentinel://{hosts}/service/2" + ) == (expected, "service", "2", None, None) + + +def test_sentinel_query_and_fragment_do_not_become_hosts_or_path(): + assert RedisConnectionFactory._parse_sentinel_url( + "redis+sentinel://host1,[::1]?ignored=/wrong/9#x@other" + ) == ([("host1", 26379), ("::1", 26379)], "mymaster", None, None, None) + + +@pytest.mark.parametrize( + "hosts", + [ + "[::1", + "host1,[::1", + "[::1]suffix:26380", + "[::1]:not-a-port", + "[::1]:65536", + "[::1]:-1", + "[::1]:", + "::1", + "host1,", + ",host1", + ":26379", + "", + ], +) +def test_parse_invalid_sentinel_hosts(hosts): + with pytest.raises(ValueError): + RedisConnectionFactory._parse_sentinel_url(f"redis+sentinel://{hosts}/mymaster") + + +@pytest.mark.parametrize("factory", ["sync", "async", "legacy_async"]) +async def test_sentinel_ipv6_factory_preserves_url_components(factory): + username, password = "user@name", "pass:,/#?word" + sentinel_url = ( + f"redis+sentinel://{quote(username, safe='')}:" + f"{quote(password, safe='')}@host1:26380,[::1]/service/2" + ) + target = "Sentinel" if factory == "sync" else "AsyncSentinel" + client = MagicMock() if factory == "sync" else AsyncMock() + with patch(f"redisvl.redis.connection.{target}") as sentinel: + sentinel.return_value.master_for.return_value = client + if factory == "sync": + result = RedisConnectionFactory.get_redis_connection(sentinel_url) + elif factory == "async": + result = await RedisConnectionFactory._get_aredis_connection(sentinel_url) + else: + with pytest.warns(DeprecationWarning): + result = RedisConnectionFactory.get_async_redis_connection(sentinel_url) + + assert result is client + assert sentinel.call_args.args[0] == [("host1", 26380), ("::1", 26379)] + assert sentinel.call_args.kwargs["sentinel_kwargs"] == { + "username": username, + "password": password, + } + master_call = sentinel.return_value.master_for.call_args + assert master_call.args == ("service",) + assert master_call.kwargs["db"] == "2" + assert master_call.kwargs["username"] == username + assert master_call.kwargs["password"] == password + assert master_call.kwargs["protocol"] == 2 + + @pytest.mark.parametrize("use_async", [False, True]) def test_sentinel_url_connection(use_async): sentinel_url = (