mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-09 03:18:44 +00:00
test(proxy): assert the assembled database URL round-trips, not how it is spelled
Review feedback on #42142: the chart tests asserted the shape of the rendered env block and nothing exercised what the resulting URL does. AGENTS.md asks for function over structure. The existing special-character test checks that certain percent-encoded substrings appear in the output, which still describes the spelling rather than the result. Adds a parametrized test that parses the assembled URL the way a client does and asserts the host, database, username and password come back out as they went in — the property that actually says the Job reaches the right database. Also pins the bug class in the Helm suite independently of URL spelling: the standalone migrations Job must render no connection string at all. Mutation results. Dropping quote_plus from the password in construct_database_url_from_env_vars fails 3 of the 4 parametrized cases; 'a:b@c' survives because urlparse splits userinfo on the last '@', so raw interpolation happens to parse back the same. Restoring the old hand-built URL in the chart fails 3 of the Helm assertions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
3db5ba01a1
commit
b5a62d94ec
2 changed files with 45 additions and 0 deletions
|
|
@ -144,6 +144,10 @@ tests:
|
|||
# connected to the wrong host, or to none. The entrypoint builds the URL
|
||||
# from these four instead, percent-encoding as it goes, which is what the
|
||||
# proxy Deployment already relies on for this same case.
|
||||
# The bug class, pinned independently of how a URL would be spelled: the
|
||||
# Job must carry no connection string at all for this case.
|
||||
- notMatchRegexRaw:
|
||||
pattern: "postgresql://"
|
||||
- notContains:
|
||||
path: spec.template.spec.containers[0].env
|
||||
content:
|
||||
|
|
|
|||
|
|
@ -1,3 +1,5 @@
|
|||
from urllib.parse import unquote, urlparse
|
||||
|
||||
import pytest
|
||||
from fastapi import HTTPException
|
||||
|
||||
|
|
@ -168,6 +170,45 @@ def test_construct_database_url_from_env_vars_special_chars_encoded(monkeypatch)
|
|||
}
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"password",
|
||||
[
|
||||
"p@ss/w+rd=",
|
||||
"s3cr#t?x",
|
||||
"a:b@c",
|
||||
"br[ack]ets",
|
||||
],
|
||||
)
|
||||
def test_construct_database_url_from_env_vars_round_trips_special_char_password(monkeypatch, password):
|
||||
"""A client parsing the assembled URL gets back the host, database and password we put in.
|
||||
|
||||
The Helm chart's bundled-postgres path hands these four variables to the migrations Job
|
||||
instead of building a URL itself, because interpolating a password holding any of
|
||||
: / ? # [ ] @ + into a connection string silently moves the host and truncates the
|
||||
password. Asserting the parse, rather than the encoded spelling, is what says the
|
||||
resulting URL still addresses the right database.
|
||||
"""
|
||||
monkeypatch.setenv("DATABASE_HOST", "release-postgresql")
|
||||
monkeypatch.setenv("DATABASE_USERNAME", "litellm")
|
||||
monkeypatch.setenv("DATABASE_PASSWORD", password)
|
||||
monkeypatch.setenv("DATABASE_NAME", "litellm")
|
||||
monkeypatch.delenv("DATABASE_SCHEMA", raising=False)
|
||||
|
||||
parsed = urlparse(construct_database_url_from_env_vars())
|
||||
|
||||
assert {
|
||||
"hostname": parsed.hostname,
|
||||
"database": parsed.path.lstrip("/"),
|
||||
"username": unquote(parsed.username or ""),
|
||||
"password": unquote(parsed.password or ""),
|
||||
} == {
|
||||
"hostname": "release-postgresql",
|
||||
"database": "litellm",
|
||||
"username": "litellm",
|
||||
"password": password,
|
||||
}
|
||||
|
||||
|
||||
def test_construct_database_url_from_env_vars_with_schema(monkeypatch):
|
||||
monkeypatch.setenv("DATABASE_HOST", "db.example.com")
|
||||
monkeypatch.setenv("DATABASE_USERNAME", "user")
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue