From b5a62d94ec5ee2b80e3f5bfb940c661e1e56a16a Mon Sep 17 00:00:00 2001
From: Bernedotcom2312 <88426124+Bernedotcom2312@users.noreply.github.com>
Date: Sun, 20 Sep 2026 16:13:00 +0200
Subject: [PATCH] test(proxy): assert the assembled database URL round-trips,
 not how it is spelled
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 .../tests/migrations-job_tests.yaml           |  4 ++
 .../proxy/utils/helpers/test_misc_helpers.py  | 41 +++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/helm/litellm-helm/tests/migrations-job_tests.yaml b/helm/litellm-helm/tests/migrations-job_tests.yaml
index 8a454440744..36200936e48 100644
--- a/helm/litellm-helm/tests/migrations-job_tests.yaml
+++ b/helm/litellm-helm/tests/migrations-job_tests.yaml
@@ -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:
diff --git a/tests/unit/proxy/utils/helpers/test_misc_helpers.py b/tests/unit/proxy/utils/helpers/test_misc_helpers.py
index 6267428e02e..7b858ee16ab 100644
--- a/tests/unit/proxy/utils/helpers/test_misc_helpers.py
+++ b/tests/unit/proxy/utils/helpers/test_misc_helpers.py
@@ -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")