fix: keep the whole Redis URL when handing out its databases

The three URLs were built by string surgery, so anything after the database
number was mangled rather than kept: rediss://host:6380/0?ssl_cert_reqs=required
came out as .../0?ssl_cert_reqs=required/0, and a URL carrying a query but no
database had /0 appended after the query. TLS and managed Redis endpoints
usually carry exactly those parameters.

The URL is split properly now, the three databases go in the path, and scheme,
credentials, host, query and fragment are preserved. A URL that cannot be
numbered this way — one that is not redis:// or rediss://, or that has
something other than a number where the database goes — is refused with a
message instead of being turned into something that merely looks like a URL.
This commit is contained in:
Alex committed 2026-09-16 23:22:07 +01:00
1 parent 76d81caaa4
commit 65ee2f4201
2 files changed
+48 -8

No files matched your search

+19 -8
View File
@@ -17,6 +17,7 @@ from dataclasses import dataclass
from datetime import datetime, timezone
from pathlib import Path
from typing import Any, Optional
from urllib.parse import urlsplit, urlunsplit
from docsgpt.deploy import backup as backup_format
from docsgpt.deploy import envfile, native, stack
@@ -205,16 +206,26 @@ def _redis_urls(base: str) -> dict[str, str]:
They start at database 0, or at the one the URL names: ``redis://host:6379/5`` puts them on 5, 6
and 7, which is how one Redis is shared with something that already uses the first databases.
Everything else in the URL is kept — TLS, credentials, and query parameters such as the
``ssl_cert_reqs`` that a rediss:// endpoint usually needs.
"""
trimmed = base.rstrip("/")
first = 0
head, _, tail = trimmed.rpartition("/")
if tail.isdigit():
trimmed, first = head, int(tail)
parts = urlsplit(base)
if parts.scheme not in ("redis", "rediss"):
raise DeployError(
f"the Redis URL {base!r} should start with redis:// or rediss://, with any options as "
"query parameters, so the broker, the result backend and the cache can be given a "
"database each."
)
path = parts.path.rstrip("/").lstrip("/")
if path and not path.isdigit():
raise DeployError(
f"the Redis URL {base!r} has {path!r} where a database number would go. Pass a URL like "
"redis://host:6379 or redis://host:6379/5."
)
first = int(path) if path else 0
return {
"CELERY_BROKER_URL": f"{trimmed}/{first}",
"CELERY_RESULT_BACKEND": f"{trimmed}/{first + 1}",
"CACHE_REDIS_URL": f"{trimmed}/{first + 2}",
key: urlunsplit((parts.scheme, parts.netloc, f"/{first + offset}", parts.query, parts.fragment))
for key, offset in (("CELERY_BROKER_URL", 0), ("CELERY_RESULT_BACKEND", 1), ("CACHE_REDIS_URL", 2))
}
+29
View File
@@ -180,6 +180,35 @@ class TestNativeUp:
"--expose", "local", "--no-docling"]
assert _run(argv, _native_context()) == 0
def test_a_redis_url_keeps_its_credentials_tls_and_query(self, tmp_path):
"""A rediss:// endpoint usually needs ssl_cert_reqs, and losing it breaks every connection."""
url = "rediss://user:pw@redis.example.com:6380/3?ssl_cert_reqs=required"
argv = ["up", "--native", "--dir", str(tmp_path), "--yes",
"--postgres-uri", "postgresql://localhost/d", "--redis-url", url]
assert _run(argv, _native_context()) == 0
env = envfile.read(tmp_path / ".env")
host = "rediss://user:pw@redis.example.com:6380"
assert env["CELERY_BROKER_URL"] == f"{host}/3?ssl_cert_reqs=required"
assert env["CELERY_RESULT_BACKEND"] == f"{host}/4?ssl_cert_reqs=required"
assert env["CACHE_REDIS_URL"] == f"{host}/5?ssl_cert_reqs=required"
def test_a_redis_url_with_a_query_and_no_database(self, tmp_path):
argv = ["up", "--native", "--dir", str(tmp_path), "--yes", "--postgres-uri", "postgresql://localhost/d",
"--redis-url", "redis://localhost:6379?health_check_interval=30"]
assert _run(argv, _native_context()) == 0
env = envfile.read(tmp_path / ".env")
assert env["CELERY_BROKER_URL"] == "redis://localhost:6379/0?health_check_interval=30"
assert env["CACHE_REDIS_URL"] == "redis://localhost:6379/2?health_check_interval=30"
@pytest.mark.parametrize("url", ["localhost:6379", "redis+socket:///var/run/redis.sock",
"redis://localhost:6379/queue"])
def test_a_redis_url_that_cannot_be_numbered_is_refused(self, tmp_path, url):
"""Silently turning it into something that looks like a URL is the worse failure."""
argv = ["up", "--native", "--dir", str(tmp_path), "--yes",
"--postgres-uri", "postgresql://localhost/d", "--redis-url", url]
with pytest.raises(DeployError, match="Redis URL"):
_run(argv, _native_context())
def test_a_database_url_is_required(self, tmp_path):
with pytest.raises(DeployError, match="--postgres-uri"):
_run(["up", "--native", "--dir", str(tmp_path), "--yes"], _native_context())