mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-14 04:09:09 +00:00
fix(proxy): don't let an all-null key router_settings shadow team settings
The admin UI persists the whole key/team router-settings form on save, blank fields included as explicit nulls. Resolution is first-wins (Key > Team, no per-field merge), so a key whose stored settings are only nulls carried no opinion yet still shadowed every team setting - the team's model_group_alias, routing strategy and friends were silently dropped for all requests on that key. Strip null entries when parsing stored key/team router_settings and resolve an all-null form dump to None so the hierarchical fallback applies. Falsy-but-set values ([] , 0, false) are preserved. The merge-site null guard for request kwargs shipped separately in #45227; this covers the source-side case it cannot see.
This commit is contained in:
parent
7c7efd4d55
commit
829802515f
2 changed files with 71 additions and 4 deletions
|
|
@ -7885,12 +7885,18 @@ class ProxyConfig:
|
|||
"""
|
||||
Parse a router_settings value that may be a dict or a JSON/YAML string.
|
||||
|
||||
Null entries are stripped: the admin UI persists the whole router-settings
|
||||
form on save, including fields left blank as explicit nulls. A null means
|
||||
"no opinion - keep the router default", never "override the default with
|
||||
nothing". An all-null form dump must also resolve to None (not an
|
||||
empty-looking dict) so the key->team hierarchical fallback still applies.
|
||||
|
||||
Returns a non-empty dict if valid, otherwise None.
|
||||
"""
|
||||
if value is None:
|
||||
return None
|
||||
|
||||
parsed: dict | None = None
|
||||
parsed: dict[str, object] | None = None
|
||||
if isinstance(value, dict):
|
||||
parsed = value
|
||||
elif isinstance(value, str):
|
||||
|
|
@ -7904,9 +7910,11 @@ class ProxyConfig:
|
|||
except json.JSONDecodeError:
|
||||
pass
|
||||
|
||||
if isinstance(parsed, dict) and parsed:
|
||||
return parsed
|
||||
return None
|
||||
if not isinstance(parsed, dict) or not parsed:
|
||||
return None
|
||||
|
||||
stripped: Final = {k: v for k, v in parsed.items() if v is not None}
|
||||
return stripped or None
|
||||
|
||||
async def get_hierarchical_router_settings(
|
||||
self,
|
||||
|
|
|
|||
|
|
@ -3747,6 +3747,39 @@ def test_ProxyConfig__parse_router_settings_value_invalid_returns_none():
|
|||
assert ProxyConfig._parse_router_settings_value({}) is None
|
||||
|
||||
|
||||
def test_ProxyConfig__parse_router_settings_value_strips_null_entries():
|
||||
# The admin UI persists the whole router-settings form, blank fields included.
|
||||
# Nulls must not survive parsing: as per-request kwargs they would shadow the
|
||||
# Router's own defaults (e.g. fallbacks=None disables the global fallback chain).
|
||||
ui_form_dump = {
|
||||
"routing_strategy": "simple-shuffle",
|
||||
"enable_tag_filtering": False,
|
||||
"fallbacks": None,
|
||||
"num_retries": None,
|
||||
"timeout": None,
|
||||
"retry_after": None,
|
||||
"retry_policy": None,
|
||||
"allowed_fails": None,
|
||||
"cooldown_time": None,
|
||||
"model_group_alias": None,
|
||||
"routing_strategy_args": None,
|
||||
"context_window_fallbacks": None,
|
||||
}
|
||||
assert ProxyConfig._parse_router_settings_value(ui_form_dump) == {
|
||||
"routing_strategy": "simple-shuffle",
|
||||
"enable_tag_filtering": False,
|
||||
}
|
||||
# Falsy-but-set values are NOT nulls and must be kept.
|
||||
assert ProxyConfig._parse_router_settings_value({"fallbacks": [], "timeout": 0}) == {
|
||||
"fallbacks": [],
|
||||
"timeout": 0,
|
||||
}
|
||||
# A settings dict that is *only* nulls carries no opinion -> None.
|
||||
assert ProxyConfig._parse_router_settings_value({"fallbacks": None, "timeout": None}) is None
|
||||
# Also for the JSON-string storage shape.
|
||||
assert ProxyConfig._parse_router_settings_value('{"num_retries": null, "timeout": 30}') == {"timeout": 30}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# ProxyConfig._get_hierarchical_router_settings
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
@ -3821,6 +3854,32 @@ async def test_ProxyConfig__get_hierarchical_router_settings_key_shadows_team_en
|
|||
team_lookup.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_ProxyConfig__get_hierarchical_router_settings_all_null_key_falls_back_to_team(monkeypatch):
|
||||
"""A key whose stored settings are only nulls (an admin UI form save with
|
||||
every field blank) carries no opinion and must not shadow the team's
|
||||
configured settings, otherwise the team's model_group_alias is silently
|
||||
dropped for every request on that key."""
|
||||
pc = ProxyConfig()
|
||||
fake_key = SimpleNamespace(
|
||||
router_settings={"fallbacks": None, "num_retries": None, "timeout": None},
|
||||
team_id="team-1",
|
||||
)
|
||||
team_settings = {"model_group_alias": {"group-a": "group-b"}, "routing_strategy": "simple-shuffle"}
|
||||
monkeypatch.setattr(
|
||||
"litellm.proxy.proxy_server.get_team_object",
|
||||
AsyncMock(return_value=SimpleNamespace(router_settings=team_settings)),
|
||||
)
|
||||
|
||||
out = await pc._get_hierarchical_router_settings(
|
||||
user_api_key_dict=fake_key,
|
||||
prisma_client=None,
|
||||
proxy_logging_obj=None,
|
||||
)
|
||||
|
||||
assert out == team_settings
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# ProxyConfig._add_router_settings_from_db_config
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue