mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-03 02:22:24 +00:00
fix(router): clean pattern_router state on upsert/delete
PatternMatchRouter.add_pattern was append-only, and neither Router.upsert_deployment nor Router.delete_deployment removed the existing entry. Rotated-out api_keys stayed in the routing rotation for wildcard deployments (model_name with `*`) until proxy restart, silently defeating key rotation as an admin operation. The same leak applied to provider_default_deployment_ids and per-team pattern routers, and the patterns list grew unboundedly on every edit
This commit is contained in:
parent
d45e9e4d56
commit
d89535f933
4 changed files with 310 additions and 0 deletions
|
|
@ -8686,6 +8686,9 @@ class Router:
|
|||
self._update_deployment_indices_after_removal(
|
||||
model_id=deployment_id, removal_idx=removal_idx
|
||||
)
|
||||
self._remove_deployment_from_wildcard_state(
|
||||
model_id=deployment_id
|
||||
)
|
||||
|
||||
# if the model_id is not in router
|
||||
self.add_deployment(deployment=deployment)
|
||||
|
|
@ -8721,6 +8724,7 @@ class Router:
|
|||
self._update_deployment_indices_after_removal(
|
||||
model_id=id, removal_idx=deployment_idx
|
||||
)
|
||||
self._remove_deployment_from_wildcard_state(model_id=id)
|
||||
_budget_limiter = self._get_router_deployment_budget_limiter()
|
||||
if _budget_limiter is not None:
|
||||
_budget_limiter.unregister_deployment_budget(model_id=id)
|
||||
|
|
@ -8730,6 +8734,28 @@ class Router:
|
|||
except Exception:
|
||||
return None
|
||||
|
||||
def _remove_deployment_from_wildcard_state(self, model_id: str) -> None:
|
||||
"""
|
||||
Drop every reference to model_id from the wildcard-routing data
|
||||
structures. Without this, upsert/delete leaves stale credentials in
|
||||
pattern_router which silently defeats key rotation for wildcard
|
||||
deployments.
|
||||
"""
|
||||
if not model_id:
|
||||
return
|
||||
self.pattern_router.remove_deployment(model_id)
|
||||
empty_team_ids: List[str] = []
|
||||
for team_id, team_router in self.team_pattern_routers.items():
|
||||
team_router.remove_deployment(model_id)
|
||||
if not team_router.patterns:
|
||||
empty_team_ids.append(team_id)
|
||||
for team_id in empty_team_ids:
|
||||
del self.team_pattern_routers[team_id]
|
||||
if model_id in self.provider_default_deployment_ids:
|
||||
self.provider_default_deployment_ids = [
|
||||
i for i in self.provider_default_deployment_ids if i != model_id
|
||||
]
|
||||
|
||||
def _get_router_deployment_budget_limiter(
|
||||
self,
|
||||
) -> Optional[RouterBudgetLimiting]:
|
||||
|
|
|
|||
|
|
@ -75,6 +75,34 @@ class PatternMatchRouter:
|
|||
self.patterns[regex] = []
|
||||
self.patterns[regex].append(llm_deployment)
|
||||
|
||||
def remove_deployment(self, model_id: str) -> int:
|
||||
"""
|
||||
Remove every stored entry whose model_info.id equals model_id.
|
||||
|
||||
Returns the number of entries removed. A regex whose deployment list
|
||||
becomes empty is dropped so the patterns dict does not grow unboundedly.
|
||||
|
||||
Empty / falsy model_id is a no-op so callers cannot accidentally wipe
|
||||
entries whose model_info.id is missing.
|
||||
"""
|
||||
if not model_id:
|
||||
return 0
|
||||
removed_total = 0
|
||||
for regex in list(self.patterns.keys()):
|
||||
original = self.patterns[regex]
|
||||
filtered = [
|
||||
d for d in original if (d.get("model_info") or {}).get("id") != model_id
|
||||
]
|
||||
removed_here = len(original) - len(filtered)
|
||||
if removed_here == 0:
|
||||
continue
|
||||
removed_total += removed_here
|
||||
if filtered:
|
||||
self.patterns[regex] = filtered
|
||||
else:
|
||||
del self.patterns[regex]
|
||||
return removed_total
|
||||
|
||||
def _pattern_to_regex(self, pattern: str) -> str:
|
||||
"""
|
||||
Convert a wildcard pattern to a regex pattern
|
||||
|
|
|
|||
|
|
@ -395,3 +395,90 @@ def test_wildcard_priority_over_deployment_names():
|
|||
assert (
|
||||
deployments[0]["litellm_params"]["api_base"] == "http://localhost:8081/openai"
|
||||
), f"Expected '*' wildcard deployment (8081), got {deployments[0]['litellm_params']['api_base']}"
|
||||
|
||||
|
||||
def _make_wildcard_entry(model_id: str, api_key: str) -> dict:
|
||||
"""Build a dict shaped like Deployment(...).to_json(exclude_none=True)."""
|
||||
return Deployment(
|
||||
model_name="openai/*",
|
||||
litellm_params=LiteLLM_Params(model="openai/*", api_key=api_key),
|
||||
model_info=ModelInfo(id=model_id),
|
||||
).to_json(exclude_none=True)
|
||||
|
||||
|
||||
def test_remove_deployment_drops_matching_id_only():
|
||||
router = PatternMatchRouter()
|
||||
router.add_pattern("openai/*", _make_wildcard_entry("id-A", "key-A"))
|
||||
router.add_pattern("openai/*", _make_wildcard_entry("id-B", "key-B"))
|
||||
|
||||
removed = router.remove_deployment("id-A")
|
||||
|
||||
assert removed == 1
|
||||
survivors = router.patterns["openai/(.*)"]
|
||||
assert len(survivors) == 1
|
||||
assert survivors[0]["model_info"]["id"] == "id-B"
|
||||
assert survivors[0]["litellm_params"]["api_key"] == "key-B"
|
||||
|
||||
|
||||
def test_remove_deployment_drops_regex_key_when_list_empties():
|
||||
router = PatternMatchRouter()
|
||||
router.add_pattern("openai/*", _make_wildcard_entry("id-A", "key-A"))
|
||||
|
||||
router.remove_deployment("id-A")
|
||||
|
||||
assert router.patterns == {}
|
||||
|
||||
|
||||
def test_remove_deployment_spans_multiple_regexes():
|
||||
router = PatternMatchRouter()
|
||||
router.add_pattern("openai/*", _make_wildcard_entry("id-A", "key-A"))
|
||||
router.add_pattern("anthropic/*", _make_wildcard_entry("id-A", "key-A"))
|
||||
router.add_pattern("openai/*", _make_wildcard_entry("id-B", "key-B"))
|
||||
|
||||
removed = router.remove_deployment("id-A")
|
||||
|
||||
assert removed == 2
|
||||
assert "anthropic/(.*)" not in router.patterns
|
||||
assert [d["model_info"]["id"] for d in router.patterns["openai/(.*)"]] == ["id-B"]
|
||||
|
||||
|
||||
def test_remove_deployment_noop_for_unknown_id():
|
||||
router = PatternMatchRouter()
|
||||
router.add_pattern("openai/*", _make_wildcard_entry("id-A", "key-A"))
|
||||
before = {k: list(v) for k, v in router.patterns.items()}
|
||||
|
||||
removed = router.remove_deployment("id-DOES-NOT-EXIST")
|
||||
|
||||
assert removed == 0
|
||||
assert router.patterns == before
|
||||
|
||||
|
||||
def test_remove_deployment_with_falsy_id_is_noop_even_when_entries_have_no_id():
|
||||
"""
|
||||
An entry without model_info.id has '' / None as its id. remove_deployment('')
|
||||
must NOT match those entries; otherwise a stray empty-string call would
|
||||
wipe every id-less wildcard deployment in the router.
|
||||
"""
|
||||
router = PatternMatchRouter()
|
||||
router.patterns["openai/(.*)"] = [
|
||||
{"model_name": "openai/*", "litellm_params": {"api_key": "key-X"}}
|
||||
]
|
||||
|
||||
for falsy in ("", None):
|
||||
removed = router.remove_deployment(falsy)
|
||||
assert removed == 0
|
||||
assert len(router.patterns["openai/(.*)"]) == 1
|
||||
|
||||
|
||||
def test_remove_deployment_tolerates_missing_model_info():
|
||||
router = PatternMatchRouter()
|
||||
router.patterns["openai/(.*)"] = [
|
||||
{"model_name": "openai/*", "litellm_params": {"api_key": "X"}},
|
||||
_make_wildcard_entry("id-A", "key-A"),
|
||||
]
|
||||
|
||||
router.remove_deployment("id-A")
|
||||
|
||||
survivors = router.patterns["openai/(.*)"]
|
||||
assert len(survivors) == 1
|
||||
assert "model_info" not in survivors[0]
|
||||
|
|
|
|||
|
|
@ -4231,3 +4231,172 @@ def test_is_deployment_blocked_static_helper_reflects_blocked_flag():
|
|||
)
|
||||
is True
|
||||
)
|
||||
|
||||
|
||||
def _build_wildcard_router(api_key: str, model_id: str = "wildcard-deployment-1"):
|
||||
from litellm.router import Deployment
|
||||
|
||||
router = litellm.Router(
|
||||
model_list=[
|
||||
{
|
||||
"model_name": "openai/*",
|
||||
"litellm_params": {"model": "openai/*", "api_key": api_key},
|
||||
"model_info": {"id": model_id},
|
||||
},
|
||||
],
|
||||
)
|
||||
return router, Deployment
|
||||
|
||||
|
||||
def test_upsert_wildcard_deployment_removes_stale_api_key():
|
||||
"""
|
||||
Regression: rotating the api_key on a wildcard deployment must NOT leave
|
||||
the previous credential in pattern_router. Before the fix, the old entry
|
||||
survived in pattern_router.patterns[regex] and was still selected on
|
||||
roughly half of wildcard traffic.
|
||||
"""
|
||||
router, Deployment = _build_wildcard_router(api_key="OLD-KEY")
|
||||
model_id = "wildcard-deployment-1"
|
||||
|
||||
router.upsert_deployment(
|
||||
Deployment(
|
||||
model_name="openai/*",
|
||||
litellm_params={"model": "openai/*", "api_key": "NEW-KEY"},
|
||||
model_info={"id": model_id},
|
||||
)
|
||||
)
|
||||
|
||||
entries = router.pattern_router.patterns["openai/(.*)"]
|
||||
assert (
|
||||
len(entries) == 1
|
||||
), f"pattern_router accumulated stale entries: {len(entries)} present"
|
||||
assert entries[0]["litellm_params"]["api_key"] == "NEW-KEY"
|
||||
assert all(
|
||||
e["litellm_params"]["api_key"] != "OLD-KEY" for e in entries
|
||||
), "Rotated-out api_key is still resident in pattern_router"
|
||||
|
||||
|
||||
def test_upsert_wildcard_deployment_is_idempotent_in_pattern_router():
|
||||
"""
|
||||
Repeated upserts of the same wildcard deployment must not grow the
|
||||
pattern_router unboundedly.
|
||||
"""
|
||||
router, Deployment = _build_wildcard_router(api_key="key-0")
|
||||
model_id = "wildcard-deployment-1"
|
||||
|
||||
for i in range(1, 6):
|
||||
router.upsert_deployment(
|
||||
Deployment(
|
||||
model_name="openai/*",
|
||||
litellm_params={"model": "openai/*", "api_key": f"key-{i}"},
|
||||
model_info={"id": model_id},
|
||||
)
|
||||
)
|
||||
|
||||
entries = router.pattern_router.patterns["openai/(.*)"]
|
||||
assert len(entries) == 1
|
||||
assert entries[0]["litellm_params"]["api_key"] == "key-5"
|
||||
assert router.provider_default_deployment_ids.count(model_id) == 1
|
||||
|
||||
|
||||
def test_delete_wildcard_deployment_clears_pattern_router():
|
||||
router, _ = _build_wildcard_router(api_key="OLD-KEY")
|
||||
model_id = "wildcard-deployment-1"
|
||||
|
||||
assert router.pattern_router.patterns # sanity
|
||||
|
||||
router.delete_deployment(id=model_id)
|
||||
|
||||
assert router.pattern_router.patterns == {}
|
||||
assert model_id not in router.provider_default_deployment_ids
|
||||
|
||||
|
||||
def test_upsert_wildcard_to_concrete_model_removes_old_wildcard_entry():
|
||||
"""
|
||||
Changing model_name from a wildcard to a concrete name must drop the
|
||||
old wildcard entry. Otherwise traffic to openai/<anything> still routes
|
||||
to a model the operator believes they renamed.
|
||||
"""
|
||||
router, Deployment = _build_wildcard_router(api_key="key-A")
|
||||
model_id = "wildcard-deployment-1"
|
||||
|
||||
router.upsert_deployment(
|
||||
Deployment(
|
||||
model_name="openai/gpt-4o",
|
||||
litellm_params={"model": "openai/gpt-4o", "api_key": "key-A"},
|
||||
model_info={"id": model_id},
|
||||
)
|
||||
)
|
||||
|
||||
assert router.pattern_router.patterns == {}
|
||||
assert model_id not in router.provider_default_deployment_ids
|
||||
|
||||
|
||||
def test_upsert_team_wildcard_deployment_does_not_leak_old_key():
|
||||
from litellm.router import Deployment
|
||||
|
||||
team_id = "team-acme"
|
||||
model_id = "team-wildcard-1"
|
||||
|
||||
router = litellm.Router(
|
||||
model_list=[
|
||||
{
|
||||
"model_name": "openai/team-foo-*",
|
||||
"litellm_params": {"model": "openai/*", "api_key": "OLD-KEY"},
|
||||
"model_info": {
|
||||
"id": model_id,
|
||||
"team_id": team_id,
|
||||
"team_public_model_name": "team-foo-*",
|
||||
},
|
||||
},
|
||||
],
|
||||
)
|
||||
|
||||
assert team_id in router.team_pattern_routers
|
||||
assert router.team_pattern_routers[team_id].patterns
|
||||
|
||||
router.upsert_deployment(
|
||||
Deployment(
|
||||
model_name="openai/team-foo-*",
|
||||
litellm_params={"model": "openai/*", "api_key": "NEW-KEY"},
|
||||
model_info={
|
||||
"id": model_id,
|
||||
"team_id": team_id,
|
||||
"team_public_model_name": "team-foo-*",
|
||||
},
|
||||
)
|
||||
)
|
||||
|
||||
team_router = router.team_pattern_routers[team_id]
|
||||
assert len(team_router.patterns) == 1
|
||||
team_entries = next(iter(team_router.patterns.values()))
|
||||
assert len(team_entries) == 1
|
||||
assert team_entries[0]["litellm_params"]["api_key"] == "NEW-KEY"
|
||||
|
||||
|
||||
def test_delete_team_wildcard_removes_empty_team_router():
|
||||
"""
|
||||
When the last wildcard deployment for a team is deleted, its dedicated
|
||||
team_pattern_router entry should be discarded too, not left as an empty
|
||||
PatternMatchRouter holding the team_id forever.
|
||||
"""
|
||||
team_id = "team-acme"
|
||||
model_id = "team-wildcard-1"
|
||||
|
||||
router = litellm.Router(
|
||||
model_list=[
|
||||
{
|
||||
"model_name": "openai/team-foo-*",
|
||||
"litellm_params": {"model": "openai/*", "api_key": "key-A"},
|
||||
"model_info": {
|
||||
"id": model_id,
|
||||
"team_id": team_id,
|
||||
"team_public_model_name": "team-foo-*",
|
||||
},
|
||||
},
|
||||
],
|
||||
)
|
||||
|
||||
router.delete_deployment(id=model_id)
|
||||
|
||||
assert team_id not in router.team_pattern_routers
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue