Merge remote-tracking branch 'origin/litellm_internal_staging' into litellm_fix_pdf_file_id_test

litellm/proxy/proxy_server.py

@@ -12891,9 +12891,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:
@@ -12901,109 +12904,96 @@ 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},
                 },
             )
+            # invalidate the DualCache entry so the next reader (this process
+            # or any other proxy in the cluster) goes to DB.
+            await invalidate_config_param(param_name)
 
-        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},
-                    },
-                )
-                await invalidate_config_param(k)
-
-        ### 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: merge existing + request, request wins (matching
+        # router_settings semantics — the caller's value for any given key is
+        # what gets persisted). success_callback is special-cased: it is
+        # always normalized + deduped, and unioned with any existing list,
+        # because callbacks are additive (callers send the new entry, not
+        # the full set). Normalizing on every write — not only when an
+        # existing entry is present — keeps the DB free of mixed-case
+        # entries that delete_callback (lowercase lookup) cannot find.
         if config_info.litellm_settings is not None:
-            config.setdefault("litellm_settings", {})
-            updated_litellm_settings = config_info.litellm_settings
-            config["litellm_settings"] = {
-                **updated_litellm_settings,
-                **config["litellm_settings"],
-            }
+            existing = await _read_section("litellm_settings")
+            updated_litellm_settings = dict(config_info.litellm_settings)
 
-            # if litellm.success_callback in updated_litellm_settings and config["litellm_settings"]
-            if (
-                "success_callback" in updated_litellm_settings
-                and "success_callback" in config["litellm_settings"]
-            ):
-                # 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
+            incoming_cb = updated_litellm_settings.get("success_callback")
+            if isinstance(incoming_cb, list):
+                updated_litellm_settings["success_callback"] = normalize_callback_names(
+                    incoming_cb
+                )
 
-        # Save the updated config
-        await proxy_config.save_config(new_config=config)
+            merged = {**existing, **updated_litellm_settings}
+
+            incoming_cb = updated_litellm_settings.get("success_callback")
+            existing_cb = existing.get("success_callback")
+            if isinstance(incoming_cb, list):
+                if isinstance(existing_cb, list):
+                    # Normalize the existing list too — a row written by a
+                    # different code path may still hold mixed-case names,
+                    # which would otherwise dedup-miss against the lowercase
+                    # incoming entries.
+                    merged["success_callback"] = list(
+                        set(normalize_callback_names(existing_cb) + incoming_cb)
+                    )
+                else:
+                    merged["success_callback"] = list(set(incoming_cb))
+
+            await _upsert_section("litellm_settings", merged)
+
+        # 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

tests/proxy_unit_tests/test_proxy_server.py

@@ -2768,40 +2768,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
 
@@ -2810,9 +2810,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

tests/test_litellm/proxy/test_proxy_server.py

@@ -5505,6 +5505,202 @@ async def test_reseed_warms_cache_even_on_zero_db_spend():
         ps.prisma_client = orig_prisma
 
 
+# -----------------------------------------------------------------------------
+# /config/update — critical paths only.
+#
+# These exercise the four behaviors that broke or changed in the rewrite of
+# update_config (litellm/proxy/proxy_server.py): targeted per-section writes,
+# the removal of the store_model_in_db gate, env var encryption, and the
+# success_callback / litellm_settings merge semantics. All other branches
+# (auth, missing-DB, slack auto-enable, router_settings merge) are covered
+# implicitly or by upstream tests.
+# -----------------------------------------------------------------------------
+
+
+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: list = []
+        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"]
+        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 _FakePrismaClient:
+    def __init__(self, initial_rows=None):
+        self.db = mock.MagicMock()
+        self.db.litellm_config = _FakeLitellmConfig(initial_rows=initial_rows)
+        self.jsonify_object = lambda obj: obj
+
+
+@pytest.fixture
+def _update_config_setup(monkeypatch):
+    """Install fakes for the /config/update endpoint and return (client, prisma)."""
+    from litellm.proxy._types import LitellmUserRoles, UserAPIKeyAuth
+    from litellm.proxy.auth.user_api_key_auth import user_api_key_auth as auth_dep
+
+    def _install(initial_rows=None, store_model_in_db=True):
+        prisma = _FakePrismaClient(initial_rows=initial_rows)
+        monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", prisma)
+        monkeypatch.setattr(
+            "litellm.proxy.proxy_server.store_model_in_db", store_model_in_db
+        )
+        monkeypatch.setattr(
+            "litellm.proxy.proxy_server.encrypt_value_helper",
+            lambda value, **_: f"enc:{value}",
+        )
+        monkeypatch.setattr(
+            "litellm.proxy.proxy_server.invalidate_config_param",
+            AsyncMock(return_value=None),
+        )
+        from litellm.proxy.proxy_server import proxy_config as real_proxy_config
+
+        monkeypatch.setattr(
+            real_proxy_config, "add_deployment", AsyncMock(return_value=None)
+        )
+
+        original_overrides = app.dependency_overrides.copy()
+        app.dependency_overrides[auth_dep] = lambda: UserAPIKeyAuth(
+            user_id="test_admin",
+            user_role=LitellmUserRoles.PROXY_ADMIN,
+            api_key="sk-1234",
+        )
+        client = TestClient(app)
+
+        def _restore():
+            app.dependency_overrides = original_overrides
+
+        return client, prisma, _restore
+
+    return _install
+
+
+def test_update_config_writes_only_sent_section(_update_config_setup):
+    """A request that only touches general_settings must not write any other
+    section row, and must leave previously-written rows byte-identical."""
+    client, prisma, restore = _update_config_setup(
+        initial_rows={
+            "litellm_settings": {"drop_params": True},
+            "environment_variables": {"FOO": "enc:bar"},
+        }
+    )
+    try:
+        resp = client.post(
+            "/config/update",
+            json={"general_settings": {"store_prompts_in_spend_logs": True}},
+        )
+        assert resp.status_code == 200
+        written = {name for name, _ in prisma.db.litellm_config.upsert_calls}
+        assert written == {"general_settings"}
+        assert prisma.db.litellm_config.rows["litellm_settings"] == {
+            "drop_params": True
+        }
+        assert prisma.db.litellm_config.rows["environment_variables"] == {
+            "FOO": "enc:bar"
+        }
+    finally:
+        restore()
+
+
+def test_update_config_can_flip_store_model_in_db_when_currently_false(
+    _update_config_setup,
+):
+    """The endpoint used to refuse all writes when store_model_in_db was
+    False, blocking the very request that would flip it to True."""
+    client, prisma, restore = _update_config_setup(store_model_in_db=False)
+    try:
+        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
+        )
+    finally:
+        restore()
+
+
+def test_update_config_environment_variables_encrypted_before_write(
+    _update_config_setup,
+):
+    """env var values must be encrypted before they hit the DB row."""
+    client, prisma, restore = _update_config_setup()
+    try:
+        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"}
+    finally:
+        restore()
+
+
+def test_update_config_litellm_settings_request_wins_for_non_callback_keys(
+    _update_config_setup,
+):
+    """Sending {"drop_params": False} when the row holds drop_params: True
+    must persist False (request wins). Untouched keys preserved."""
+    client, prisma, restore = _update_config_setup(
+        initial_rows={
+            "litellm_settings": {"drop_params": True, "set_verbose": True},
+        }
+    )
+    try:
+        resp = client.post(
+            "/config/update", json={"litellm_settings": {"drop_params": False}}
+        )
+        assert resp.status_code == 200
+        stored = prisma.db.litellm_config.rows["litellm_settings"]
+        assert stored["drop_params"] is False
+        assert stored["set_verbose"] is True
+    finally:
+        restore()
+
+
+def test_update_config_success_callback_normalizes_existing_mixed_case(
+    _update_config_setup,
+):
+    """Existing mixed-case callback names (written elsewhere) must be
+    normalized to lowercase before union, otherwise the union dedup misses
+    against the lowercase incoming entry and delete_callback (lowercase
+    lookup) cannot find the original."""
+    client, prisma, restore = _update_config_setup(
+        initial_rows={"litellm_settings": {"success_callback": ["Langfuse", "SQS"]}}
+    )
+    try:
+        resp = client.post(
+            "/config/update",
+            json={"litellm_settings": {"success_callback": ["langfuse"]}},
+        )
+        assert resp.status_code == 200
+        stored = prisma.db.litellm_config.rows["litellm_settings"]["success_callback"]
+        assert set(stored) == {"langfuse", "sqs"}
+    finally:
+        restore()
+
+
 # ---------------------------------------------------------------------------
 # Lazy feature loading (LazyFeatureMiddleware) — verifies that optional
 # routers are NOT imported at module load and ARE imported on first request
@@ -5513,9 +5709,6 @@ async def test_reseed_warms_cache_even_on_zero_db_spend():
 # ---------------------------------------------------------------------------
 
 
-import sys
-
-
 class TestLazyFeatureRegistry:
     """Sanity checks on the registry shape — guards against accidental edits."""