mirror of
https://github.com/tiennm99/litellm.git
synced 2026-08-05 08:23:17 +00:00
fix(proxy): /config/update writes only sent sections, drop store_model_in_db gate
The endpoint loaded the full merged YAML+DB config and re-saved every top-level section to LiteLLM_Config rows via save_config(), so a UI toggle of one field persisted unrelated YAML state to DB as a side effect. It also rejected every request when store_model_in_db was False — including the request that would flip the flag to True (chicken-and-egg). Replace save_config with targeted per-section upserts: read the existing litellm_config row, merge in the request, upsert just that row. Sections the caller did not send are not touched. Drop the blanket store_model_in_db guard — the endpoint already requires prisma_client, and the startup-side override at proxy_server.py:6491 picks up general_settings.store_model_in_db=True from the DB on next restart.
This commit is contained in:
Yuneng Jiang
@@ -12557,9 +12557,12 @@ async def update_config( # noqa: PLR0915
|
||||
user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth),
|
||||
):
|
||||
"""
|
||||
For Admin UI - allows admin to update config via UI
|
||||
For Admin UI - allows admin to update config via UI.
|
||||
|
||||
Currently supports modifying General Settings + LiteLLM settings
|
||||
Writes only the sections present in the request body to LiteLLM_Config rows
|
||||
(one row per top-level section). Sections the caller did not send are left
|
||||
untouched — this endpoint never persists pre-existing YAML values to DB as
|
||||
a side effect of an unrelated update.
|
||||
"""
|
||||
global llm_router, llm_model_list, general_settings, proxy_config, proxy_logging_obj, master_key, prisma_client
|
||||
try:
|
||||
@@ -12567,108 +12570,77 @@ async def update_config( # noqa: PLR0915
|
||||
raise HTTPException(
|
||||
status_code=403, detail="Only proxy admins can update config"
|
||||
)
|
||||
import base64
|
||||
|
||||
"""
|
||||
- Update the ConfigTable DB
|
||||
- Run 'add_deployment'
|
||||
"""
|
||||
if prisma_client is None:
|
||||
raise Exception("No DB Connected")
|
||||
|
||||
if store_model_in_db is not True:
|
||||
raise HTTPException(
|
||||
status_code=500,
|
||||
detail={
|
||||
"error": "Set `'STORE_MODEL_IN_DB='True'` in your env to enable this feature."
|
||||
async def _read_section(param_name: str) -> dict:
|
||||
row = await prisma_client.db.litellm_config.find_first(
|
||||
where={"param_name": param_name}
|
||||
)
|
||||
if row is None or row.param_value is None:
|
||||
return {}
|
||||
return dict(row.param_value)
|
||||
|
||||
async def _upsert_section(param_name: str, value: dict) -> None:
|
||||
serialized = json.dumps(value)
|
||||
await prisma_client.db.litellm_config.upsert(
|
||||
where={"param_name": param_name},
|
||||
data={
|
||||
"create": {"param_name": param_name, "param_value": serialized},
|
||||
"update": {"param_value": serialized},
|
||||
},
|
||||
)
|
||||
|
||||
updated_settings = config_info.json(exclude_none=True)
|
||||
updated_settings = prisma_client.jsonify_object(updated_settings)
|
||||
for k, v in updated_settings.items():
|
||||
if k == "router_settings":
|
||||
await prisma_client.db.litellm_config.upsert(
|
||||
where={"param_name": k},
|
||||
data={
|
||||
"create": {"param_name": k, "param_value": v},
|
||||
"update": {"param_value": v},
|
||||
},
|
||||
)
|
||||
|
||||
### OLD LOGIC [TODO] MOVE TO DB ###
|
||||
|
||||
# Load existing config
|
||||
config = await proxy_config.get_config()
|
||||
verbose_proxy_logger.debug("Loaded config: %s", config)
|
||||
|
||||
# update the general settings
|
||||
# general_settings: merge per-key, with the alert_to_webhook_url side
|
||||
# effect of auto-enabling slack alerting.
|
||||
if config_info.general_settings is not None:
|
||||
config.setdefault("general_settings", {})
|
||||
updated_general_settings = config_info.general_settings.dict(
|
||||
exclude_none=True
|
||||
)
|
||||
|
||||
_existing_settings = config["general_settings"]
|
||||
for k, v in updated_general_settings.items():
|
||||
# overwrite existing settings with updated values
|
||||
existing = await _read_section("general_settings")
|
||||
updates = config_info.general_settings.dict(exclude_none=True)
|
||||
for k, v in updates.items():
|
||||
if k == "alert_to_webhook_url":
|
||||
# check if slack is already enabled. if not, enable it
|
||||
if "alerting" not in _existing_settings:
|
||||
_existing_settings = {"alerting": ["slack"]}
|
||||
elif isinstance(_existing_settings["alerting"], list):
|
||||
if "slack" not in _existing_settings["alerting"]:
|
||||
_existing_settings["alerting"].append("slack")
|
||||
_existing_settings[k] = v
|
||||
config["general_settings"] = _existing_settings
|
||||
if "alerting" not in existing:
|
||||
existing["alerting"] = ["slack"]
|
||||
elif (
|
||||
isinstance(existing["alerting"], list)
|
||||
and "slack" not in existing["alerting"]
|
||||
):
|
||||
existing["alerting"].append("slack")
|
||||
existing[k] = v
|
||||
await _upsert_section("general_settings", existing)
|
||||
|
||||
# environment_variables: encrypt request values, then merge into existing.
|
||||
if config_info.environment_variables is not None:
|
||||
config.setdefault("environment_variables", {})
|
||||
_updated_environment_variables = config_info.environment_variables
|
||||
existing = await _read_section("environment_variables")
|
||||
for k, v in config_info.environment_variables.items():
|
||||
existing[k] = encrypt_value_helper(value=v)
|
||||
await _upsert_section("environment_variables", existing)
|
||||
|
||||
# encrypt updated_environment_variables #
|
||||
for k, v in _updated_environment_variables.items():
|
||||
encrypted_value = encrypt_value_helper(value=v)
|
||||
_updated_environment_variables[k] = encrypted_value
|
||||
|
||||
_existing_env_variables = config["environment_variables"]
|
||||
|
||||
for k, v in _updated_environment_variables.items():
|
||||
# overwrite existing env variables with updated values
|
||||
_existing_env_variables[k] = _updated_environment_variables[k]
|
||||
|
||||
# update the litellm settings
|
||||
# litellm_settings: existing-wins merge (preserving legacy behavior),
|
||||
# except success_callback is unioned with the request value.
|
||||
if config_info.litellm_settings is not None:
|
||||
config.setdefault("litellm_settings", {})
|
||||
existing = await _read_section("litellm_settings")
|
||||
updated_litellm_settings = config_info.litellm_settings
|
||||
config["litellm_settings"] = {
|
||||
**updated_litellm_settings,
|
||||
**config["litellm_settings"],
|
||||
}
|
||||
|
||||
# if litellm.success_callback in updated_litellm_settings and config["litellm_settings"]
|
||||
merged = {**updated_litellm_settings, **existing}
|
||||
if (
|
||||
"success_callback" in updated_litellm_settings
|
||||
and "success_callback" in config["litellm_settings"]
|
||||
and "success_callback" in existing
|
||||
and isinstance(existing["success_callback"], list)
|
||||
and isinstance(updated_litellm_settings["success_callback"], list)
|
||||
):
|
||||
# check both success callback are lists
|
||||
if isinstance(
|
||||
config["litellm_settings"]["success_callback"], list
|
||||
) and isinstance(updated_litellm_settings["success_callback"], list):
|
||||
updated_success_callbacks_normalized = normalize_callback_names(
|
||||
updated_litellm_settings["success_callback"]
|
||||
)
|
||||
combined_success_callback = (
|
||||
config["litellm_settings"]["success_callback"]
|
||||
+ updated_success_callbacks_normalized
|
||||
)
|
||||
combined_success_callback = list(set(combined_success_callback))
|
||||
config["litellm_settings"][
|
||||
"success_callback"
|
||||
] = combined_success_callback
|
||||
normalized = normalize_callback_names(
|
||||
updated_litellm_settings["success_callback"]
|
||||
)
|
||||
merged["success_callback"] = list(
|
||||
set(existing["success_callback"] + normalized)
|
||||
)
|
||||
await _upsert_section("litellm_settings", merged)
|
||||
|
||||
# Save the updated config
|
||||
await proxy_config.save_config(new_config=config)
|
||||
# router_settings: merge existing + request, request wins.
|
||||
if config_info.router_settings is not None:
|
||||
existing = await _read_section("router_settings")
|
||||
updates = config_info.router_settings.dict(exclude_none=True)
|
||||
await _upsert_section("router_settings", {**existing, **updates})
|
||||
|
||||
await proxy_config.add_deployment(
|
||||
prisma_client=prisma_client, proxy_logging_obj=proxy_logging_obj
|
||||
|
||||
@@ -2764,40 +2764,40 @@ async def test_update_config_success_callback_normalization():
|
||||
import litellm.proxy.proxy_server as proxy_server
|
||||
from litellm.proxy._types import ConfigYAML
|
||||
|
||||
# Ensure feature is enabled and prisma_client is set
|
||||
setattr(proxy_server, "store_model_in_db", True)
|
||||
setattr(proxy_server, "proxy_logging_obj", MagicMock())
|
||||
|
||||
existing_litellm_settings = {"success_callback": ["langfuse"]}
|
||||
|
||||
class FakeRow:
|
||||
def __init__(self, name, value):
|
||||
self.param_name = name
|
||||
self.param_value = value
|
||||
|
||||
upserted = {}
|
||||
|
||||
async def fake_find_first(where=None):
|
||||
if where and where.get("param_name") == "litellm_settings":
|
||||
return FakeRow("litellm_settings", existing_litellm_settings)
|
||||
return None
|
||||
|
||||
async def fake_upsert(where=None, data=None):
|
||||
upserted[where["param_name"]] = json.loads(data["update"]["param_value"])
|
||||
|
||||
class MockPrisma:
|
||||
def __init__(self):
|
||||
self.db = MagicMock()
|
||||
self.db.litellm_config = MagicMock()
|
||||
self.db.litellm_config.upsert = AsyncMock()
|
||||
|
||||
# proxy_server.update_config expects this to be sync returning a dict
|
||||
def jsonify_object(self, obj):
|
||||
return obj
|
||||
self.db.litellm_config.find_first = AsyncMock(side_effect=fake_find_first)
|
||||
self.db.litellm_config.upsert = AsyncMock(side_effect=fake_upsert)
|
||||
|
||||
setattr(proxy_server, "prisma_client", MockPrisma())
|
||||
|
||||
class MockProxyConfig:
|
||||
def __init__(self):
|
||||
self.saved_config = None
|
||||
|
||||
async def get_config(self):
|
||||
# Existing config has one lowercase callback already
|
||||
return {"litellm_settings": {"success_callback": ["langfuse"]}}
|
||||
|
||||
async def save_config(self, new_config: dict):
|
||||
self.saved_config = new_config
|
||||
|
||||
async def add_deployment(self, prisma_client=None, proxy_logging_obj=None):
|
||||
return None
|
||||
|
||||
mock_proxy_config = MockProxyConfig()
|
||||
setattr(proxy_server, "proxy_config", mock_proxy_config)
|
||||
setattr(proxy_server, "proxy_config", MockProxyConfig())
|
||||
|
||||
# Update config with mixed-case callbacks - expect normalization to lowercase
|
||||
config_update = ConfigYAML(litellm_settings={"success_callback": ["SQS", "sQs"]})
|
||||
from litellm.proxy._types import LitellmUserRoles, UserAPIKeyAuth
|
||||
|
||||
@@ -2806,9 +2806,10 @@ async def test_update_config_success_callback_normalization():
|
||||
)
|
||||
await proxy_server.update_config(config_update, user_api_key_dict=admin_user)
|
||||
|
||||
saved = mock_proxy_config.saved_config
|
||||
assert saved is not None, "save_config was not called"
|
||||
callbacks = saved["litellm_settings"]["success_callback"]
|
||||
assert (
|
||||
"litellm_settings" in upserted
|
||||
), "litellm_config.upsert was not called for litellm_settings"
|
||||
callbacks = upserted["litellm_settings"]["success_callback"]
|
||||
|
||||
# Deduped and normalized
|
||||
assert "sqs" in callbacks
|
||||
|
||||
@@ -0,0 +1,241 @@
|
||||
"""
|
||||
Tests for the /config/update endpoint (litellm.proxy.proxy_server.update_config).
|
||||
|
||||
These tests cover the targeted-per-section upsert behavior that replaced the
|
||||
legacy save_config-based implementation, plus the removal of the
|
||||
store_model_in_db blanket guard.
|
||||
"""
|
||||
|
||||
import json
|
||||
import os
|
||||
import sys
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
sys.path.insert(0, os.path.abspath("../../../.."))
|
||||
|
||||
from litellm.proxy._types import LitellmUserRoles, UserAPIKeyAuth
|
||||
from litellm.proxy.auth.user_api_key_auth import user_api_key_auth
|
||||
from litellm.proxy.proxy_server import app
|
||||
|
||||
|
||||
class FakeRow:
|
||||
def __init__(self, param_name, param_value):
|
||||
self.param_name = param_name
|
||||
self.param_value = param_value
|
||||
|
||||
|
||||
class FakeLitellmConfig:
|
||||
def __init__(self, initial_rows=None):
|
||||
self.rows = dict(initial_rows or {})
|
||||
self.upsert_calls = []
|
||||
self.find_first = AsyncMock(side_effect=self._find_first)
|
||||
self.upsert = AsyncMock(side_effect=self._upsert)
|
||||
|
||||
async def _find_first(self, where=None):
|
||||
if where and "param_name" in where:
|
||||
name = where["param_name"]
|
||||
if name in self.rows:
|
||||
return FakeRow(name, self.rows[name])
|
||||
return None
|
||||
|
||||
async def _upsert(self, where=None, data=None):
|
||||
name = where["param_name"]
|
||||
# The endpoint calls upsert with json.dumps'd payloads. Some paths in
|
||||
# the codebase pass dicts directly, so handle both.
|
||||
raw = data["update"]["param_value"]
|
||||
value = json.loads(raw) if isinstance(raw, str) else raw
|
||||
self.rows[name] = value
|
||||
self.upsert_calls.append((name, value))
|
||||
|
||||
|
||||
class FakeDB:
|
||||
def __init__(self, initial_rows=None):
|
||||
self.litellm_config = FakeLitellmConfig(initial_rows=initial_rows)
|
||||
|
||||
|
||||
class FakePrismaClient:
|
||||
def __init__(self, initial_rows=None):
|
||||
self.db = FakeDB(initial_rows=initial_rows)
|
||||
self.jsonify_object = lambda obj: obj
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def admin_auth():
|
||||
original = app.dependency_overrides.copy()
|
||||
app.dependency_overrides[user_api_key_auth] = lambda: UserAPIKeyAuth(
|
||||
user_id="test_admin",
|
||||
user_role=LitellmUserRoles.PROXY_ADMIN,
|
||||
api_key="sk-1234",
|
||||
)
|
||||
yield
|
||||
app.dependency_overrides = original
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def non_admin_auth():
|
||||
original = app.dependency_overrides.copy()
|
||||
app.dependency_overrides[user_api_key_auth] = lambda: UserAPIKeyAuth(
|
||||
user_id="test_user",
|
||||
user_role=LitellmUserRoles.INTERNAL_USER,
|
||||
api_key="sk-1234",
|
||||
)
|
||||
yield
|
||||
app.dependency_overrides = original
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def patched_proxy(monkeypatch):
|
||||
"""Returns a callable that installs a FakePrismaClient + no-op add_deployment."""
|
||||
|
||||
def _install(initial_rows=None):
|
||||
prisma = FakePrismaClient(initial_rows=initial_rows)
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", prisma)
|
||||
# add_deployment is a coroutine method on proxy_config; replace with no-op
|
||||
from litellm.proxy.proxy_server import proxy_config as real_proxy_config
|
||||
|
||||
monkeypatch.setattr(
|
||||
real_proxy_config, "add_deployment", AsyncMock(return_value=None)
|
||||
)
|
||||
# Stub encrypt to be identity so we can assert on plain text
|
||||
monkeypatch.setattr(
|
||||
"litellm.proxy.proxy_server.encrypt_value_helper",
|
||||
lambda value, **_: f"enc:{value}",
|
||||
)
|
||||
return prisma
|
||||
|
||||
return _install
|
||||
|
||||
|
||||
def test_no_db_returns_500_class_error(admin_auth, monkeypatch):
|
||||
"""When prisma_client is None, the endpoint surfaces a ProxyException."""
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", None)
|
||||
client = TestClient(app)
|
||||
resp = client.post(
|
||||
"/config/update", json={"general_settings": {"store_model_in_db": True}}
|
||||
)
|
||||
assert resp.status_code >= 400
|
||||
# 'No DB Connected' is the message raised inside the endpoint.
|
||||
assert "No DB" in resp.text or "DB Connected" in resp.text
|
||||
|
||||
|
||||
def test_non_admin_rejected(non_admin_auth, patched_proxy):
|
||||
patched_proxy()
|
||||
client = TestClient(app)
|
||||
resp = client.post(
|
||||
"/config/update", json={"general_settings": {"store_model_in_db": True}}
|
||||
)
|
||||
assert resp.status_code in (401, 403)
|
||||
|
||||
|
||||
def test_can_flip_store_model_in_db_when_currently_false(
|
||||
admin_auth, patched_proxy, monkeypatch
|
||||
):
|
||||
"""
|
||||
Regression: previously the endpoint refused all writes when the global
|
||||
store_model_in_db flag was False, blocking even the request that would
|
||||
flip it to True. After this fix, the request succeeds and the flag is
|
||||
persisted to the general_settings row.
|
||||
"""
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.store_model_in_db", False)
|
||||
prisma = patched_proxy()
|
||||
|
||||
client = TestClient(app)
|
||||
resp = client.post(
|
||||
"/config/update", json={"general_settings": {"store_model_in_db": True}}
|
||||
)
|
||||
|
||||
assert resp.status_code == 200
|
||||
assert (
|
||||
prisma.db.litellm_config.rows["general_settings"]["store_model_in_db"] is True
|
||||
)
|
||||
|
||||
|
||||
def test_only_sent_section_is_written(admin_auth, patched_proxy):
|
||||
"""
|
||||
A request that only touches general_settings must not write
|
||||
litellm_settings, environment_variables, or router_settings rows.
|
||||
"""
|
||||
prisma = patched_proxy(
|
||||
initial_rows={
|
||||
"litellm_settings": {"drop_params": True},
|
||||
"environment_variables": {"FOO": "enc:bar"},
|
||||
}
|
||||
)
|
||||
|
||||
client = TestClient(app)
|
||||
resp = client.post(
|
||||
"/config/update",
|
||||
json={"general_settings": {"store_prompts_in_spend_logs": True}},
|
||||
)
|
||||
|
||||
assert resp.status_code == 200
|
||||
written_param_names = {name for name, _ in prisma.db.litellm_config.upsert_calls}
|
||||
assert written_param_names == {"general_settings"}
|
||||
# Untouched rows preserved.
|
||||
assert prisma.db.litellm_config.rows["litellm_settings"] == {"drop_params": True}
|
||||
assert prisma.db.litellm_config.rows["environment_variables"] == {"FOO": "enc:bar"}
|
||||
|
||||
|
||||
def test_environment_variables_encrypted_before_write(admin_auth, patched_proxy):
|
||||
prisma = patched_proxy()
|
||||
client = TestClient(app)
|
||||
resp = client.post(
|
||||
"/config/update",
|
||||
json={"environment_variables": {"OPENAI_API_KEY": "sk-secret"}},
|
||||
)
|
||||
|
||||
assert resp.status_code == 200
|
||||
stored = prisma.db.litellm_config.rows["environment_variables"]
|
||||
assert stored == {"OPENAI_API_KEY": "enc:sk-secret"}
|
||||
|
||||
|
||||
def test_success_callback_unioned_with_existing(admin_auth, patched_proxy):
|
||||
prisma = patched_proxy(
|
||||
initial_rows={"litellm_settings": {"success_callback": ["langfuse"]}}
|
||||
)
|
||||
|
||||
client = TestClient(app)
|
||||
resp = client.post(
|
||||
"/config/update",
|
||||
json={"litellm_settings": {"success_callback": ["prometheus"]}},
|
||||
)
|
||||
|
||||
assert resp.status_code == 200
|
||||
stored = prisma.db.litellm_config.rows["litellm_settings"]["success_callback"]
|
||||
assert set(stored) == {"langfuse", "prometheus"}
|
||||
|
||||
|
||||
def test_alert_to_webhook_url_enables_slack_alerting(admin_auth, patched_proxy):
|
||||
prisma = patched_proxy()
|
||||
client = TestClient(app)
|
||||
resp = client.post(
|
||||
"/config/update",
|
||||
json={
|
||||
"general_settings": {
|
||||
"alert_to_webhook_url": {"spend_reports": "https://hooks/foo"}
|
||||
}
|
||||
},
|
||||
)
|
||||
|
||||
assert resp.status_code == 200
|
||||
stored = prisma.db.litellm_config.rows["general_settings"]
|
||||
assert stored["alerting"] == ["slack"]
|
||||
assert stored["alert_to_webhook_url"] == {"spend_reports": "https://hooks/foo"}
|
||||
|
||||
|
||||
def test_router_settings_merged_with_existing(admin_auth, patched_proxy):
|
||||
prisma = patched_proxy(
|
||||
initial_rows={"router_settings": {"num_retries": 3, "timeout": 10}}
|
||||
)
|
||||
|
||||
client = TestClient(app)
|
||||
resp = client.post("/config/update", json={"router_settings": {"num_retries": 5}})
|
||||
|
||||
assert resp.status_code == 200
|
||||
stored = prisma.db.litellm_config.rows["router_settings"]
|
||||
# New value wins, untouched key preserved.
|
||||
assert stored["num_retries"] == 5
|
||||
assert stored["timeout"] == 10
|
||||
Reference in New Issue
Block a user