feat(cache): back the Redis URL and Database Index UI fields end-to-end - #32075
Conversation
The typed cache-settings form already renders a Redis URL and a Database Index field, but the backend never defined them, so GET /cache/settings could not round-trip a saved value into the form and the "URL takes precedence over Host/Port/Password/Database Index" help text the UI shows was not actually enforced anywhere. Add the url and db entries to CACHE_SETTINGS_FIELDS so the endpoint knows about them, and add _resolve_cache_url_precedence: when a non-empty url is present it wins and the discrete host/port/db/password fields are dropped before the settings are tested or persisted, matching how litellm._redis resolves the connection at runtime (redis.Redis.from_url ignores them). Cluster mode is exempt because it authenticates via the discrete fields rather than a url. Both test and save paths go through the resolver so the stored config is unambiguous. This finishes LIT-3996: operators can now isolate the cache into a logical database (e.g. redis://host:6379/1) entirely from the Admin UI instead of hardcoding REDIS_URL in the environment.
Greptile SummaryThis PR wires up the Redis URL and Database Index fields end-to-end: both are added to
Confidence Score: 5/5Safe to merge. The change is additive, well-tested, and the two credential-exposure issues flagged in the prior review are both resolved in this version. The resolver logic is correct and handles every edge case tested (blank URL, cluster exemption, username stripping). The No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/management_endpoints/cache_settings_endpoints.py | Adds url to _CACHE_SENSITIVE_FIELDS, introduces _resolve_cache_url_precedence (with correct cluster-mode exemption), and calls it in both test and save paths. Both P1s from the previous review (username omitted from _URL_OVERRIDDEN_CONNECTION_FIELDS, url not masked on GET) are addressed. |
| litellm/types/management_endpoints/cache_settings_endpoints.py | Adds url (String) and db (Integer) entries to CACHE_SETTINGS_FIELDS with correct types, descriptions, and redis_type=None (common fields). Changes are purely additive and backward-compatible. |
| tests/test_litellm/proxy/management_endpoints/test_cache_settings_endpoints.py | Adds six new unit tests covering field-definition assertions, resolver logic (URL wins, blank URL, cluster exemption), both endpoint flows under URL precedence, and GET masking. All tests are properly mocked with no real network calls. Existing tests changed for formatting only — no assertions weakened. |
Reviews (2): Last reviewed commit: "fix(cache): use builtin dict annotation ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
…der url precedence Two follow-ups from review of the url/db work. A Redis/Valkey url can embed a password (redis://:secret@host:6379/1), but _CACHE_SENSITIVE_FIELDS only masked the discrete password and sentinel_password, so a stored password-bearing url came back in plaintext from every GET /cache/settings. Add url to the masked set so it gets the same masked-on-read treatment as password. The url-precedence resolver dropped host/port/db/password but not username, even though a url can encode a username too (redis://user:pass@host). Left in, the discrete username rode along and could contradict the url. Add username to the overridden set and update the Redis URL help text to list it among the fields url takes precedence over. Tests: GET masks a password-bearing url (secret never returned verbatim) while a non-credential field is untouched, and the resolver drops a discrete username when a url is present.
|
@veria-ai review |
2 participants
Relevant issues
Linear ticket
LIT-3996 (UI Deficiencies: Database Selection & Cache Config)
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewScreenshots / Proof of Fix
Backend behavior against a proxy with
STORE_MODEL_IN_DB='True'and a database connected. The url-precedence resolver runs on both the test and save paths, so the discrete host/port/db/username/password fields are dropped when a full URL is supplied, and a stored password-bearing URL is masked on read/1(e.g.redis://:pw@host:6379/1) and confirm the value persists on reload and the cache connects to logical DB 1Type
🆕 New Feature
Changes
The typed cache-settings form (shipped in #31939) already renders a "Redis URL" and a "Database Index" field, but the backend never defined them. Two consequences:
GET /cache/settingsbuilds its field list fromCACHE_SETTINGS_FIELDS, so a savedurl/dbcould not round-trip back into the form on reload; and the "When set, it takes precedence over Host, Port, Password, and Database Index" help text the UI shows for the URL field was not enforced anywhere on the backendThis adds the
urlanddbentries toCACHE_SETTINGS_FIELDSso the endpoint knows about them, and introduces_resolve_cache_url_precedence. When a non-emptyurlis present it wins and the discretehost/port/db/username/passwordfields are dropped before the settings are tested or persisted, which matches howlitellm._redisresolves the connection at runtime (redis.Redis.from_urlignores those fields when a URL is given). Cluster mode (redis_startup_nodes) is exempt because it authenticates via the discrete fields rather than a URL. Both the test path (POST /cache/settings/test) and the save path (POST /cache/settings) go through the resolver, so the persisted config is unambiguousBecause a Redis/Valkey URL can carry an inline password (
redis://:secret@host:6379/1),urlis added to the set of credential fields that are masked on read, so a stored password-bearing URL is never returned in plaintext byGET /cache/settings; it gets the same masked-on-read treatment as the discretepasswordfield.usernameis likewise encodable in a URL, so it is dropped under URL precedence alongsidepassword, and the URL field help text now lists Username among the fields the URL overridesThis completes LIT-3996: operators can now isolate the cache into a specific logical database (e.g.
redis://host:6379/1) entirely from the Admin UI instead of falling back to hardcodingREDIS_URLvia the environment. TTL and namespace were already configurable via existing fields; the missing piece was full-URI and logical-database support, which the UI advertised but the backend did not yet honorTests cover the field definitions exposing
url/db, the resolver dropping discrete fields (includingusername) only when a non-empty URL is present, blank-URL and cluster-mode exemptions, both endpoints persisting the resolved settings, andGET /cache/settingsmasking a password-bearing URL while leaving non-credential fields untouched