diff --git a/docsgpt/deploy/commands.py b/docsgpt/deploy/commands.py index 1495bb5b..8d00f7bb 100644 --- a/docsgpt/deploy/commands.py +++ b/docsgpt/deploy/commands.py @@ -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)) } diff --git a/tests/deploy/test_native.py b/tests/deploy/test_native.py index 31dc6729..cd7092ca 100644 --- a/tests/deploy/test_native.py +++ b/tests/deploy/test_native.py @@ -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())