diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index 0e88ff9c13f..99b1fccfa95 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -22,6 +22,8 @@ from collections.abc import ( Awaitable, Callable, Collection, + Iterable, + Iterator, Mapping, MutableMapping, Sequence, @@ -19026,6 +19028,26 @@ async def delete_config_general_settings( return response +_CALLBACK_LIST_KEYS: Final = ("success_callback", "failure_callback", "callbacks") + + +def _configured_callback_names(value: object) -> tuple[JsonValue, ...]: + if isinstance(value, str): + return (value,) + if isinstance(value, (list, tuple, dict)): + return tuple(cast(Iterable[JsonValue], value)) # cast-ok: config.yaml values are untyped + return () + + +def _remaining_callback_names(settings: Mapping[str, object]) -> Iterator[JsonValue]: + for key in _CALLBACK_LIST_KEYS: + yield from _configured_callback_names(settings.get(key)) + + +def _is_callback_name(entry: object, callback_name: str) -> bool: + return isinstance(entry, str) and entry.lower() == callback_name + + @router.post( "/config/callback/delete", tags=["config.yaml"], @@ -19061,34 +19083,49 @@ async def delete_callback( try: # Get current configuration - config: Final = await proxy_config.get_config() + config: Final[dict[str, object]] = cast( # cast-ok: get_config returns an untyped config dict + dict[str, object], + await proxy_config.get_config(), + ) callback_name: Final = data.callback_name.lower() # Check if callback exists in current configuration - litellm_settings: Final = config.get("litellm_settings", {}) - success_callbacks: Final = litellm_settings.get("success_callback", []) + litellm_settings: Final[Mapping[str, object]] = cast( # cast-ok: the config dict's values are untyped + Mapping[str, object], + config.get("litellm_settings", {}), + ) + configured_lists: Final = { + key: _configured_callback_names(litellm_settings.get(key)) for key in _CALLBACK_LIST_KEYS + } + matching_keys: Final = tuple( + key + for key, names in configured_lists.items() + if any(_is_callback_name(entry, callback_name) for entry in names) + ) - if callback_name not in success_callbacks: + if not matching_keys: raise HTTPException( status_code=404, detail={"error": f"Callback '{callback_name}' not found in active configuration"}, ) - before_success_callbacks: Final = list(success_callbacks) - - # Remove callback from success_callback list - success_callbacks.remove(callback_name) - config.setdefault("litellm_settings", {})["success_callback"] = success_callbacks + before_callbacks: Final[dict[str, JsonValue]] = {key: list(configured_lists[key]) for key in matching_keys} + after_callbacks: Final[dict[str, JsonValue]] = { + key: [entry for entry in configured_lists[key] if not _is_callback_name(entry, callback_name)] + for key in matching_keys + } + updated_settings: Final = {**litellm_settings, **after_callbacks} + updated_config: Final = {**config, "litellm_settings": updated_settings} # Save the updated configuration - await proxy_config.save_config(new_config=config) + await proxy_config.save_config(new_config=updated_config) asyncio.create_task( create_config_audit_log( "litellm_settings", "deleted", - {"success_callback": before_success_callbacks}, - {"success_callback": success_callbacks}, + before_callbacks, + after_callbacks, user_api_key_dict, ) ) @@ -19099,7 +19136,7 @@ async def delete_callback( return { "message": f"Successfully deleted callback: {callback_name}", "removed_callback": callback_name, - "remaining_callbacks": success_callbacks, + "remaining_callbacks": list(_remaining_callback_names(updated_settings)), "deleted_at": datetime.now().isoformat(), } diff --git a/tests/e2e/coverage_registry/mgmt.yaml b/tests/e2e/coverage_registry/mgmt.yaml index bc728efacd4..f3ab8a2b3c1 100644 --- a/tests/e2e/coverage_registry/mgmt.yaml +++ b/tests/e2e/coverage_registry/mgmt.yaml @@ -71,6 +71,7 @@ - {id: mgmt.budget.list_v1.happy_path, module: mgmt, tier: P1, surface: api, assertions: [happy_path], source: "management_v1/budgets.py:129", rationale: "Budget enumeration the Budgets page can page, sort and filter"} - {id: mgmt.budget.list_v1.admin_only, module: mgmt, tier: P1, surface: api, assertions: [admin_only], source: "management_v1/budgets.py:129", rationale: "A caller without admin view is refused, not served an empty page"} - {id: mgmt.callback.list.happy_path, module: mgmt, tier: P2, surface: api, assertions: [happy_path], source: "callback_management_endpoints.py", rationale: "Callback config (smoke)"} +- {id: mgmt.callback.delete.removes_configured_callback, module: mgmt, tier: P2, surface: api, assertions: [removes_configured_callback], source: "proxy_server.py delete_callback", fail_before_fix: proven, rationale: "A callback listed by GET /get/config/callbacks from success_callback, failure_callback or callbacks must be removable via POST /config/callback/delete; the delete only searched success_callback, so the other two returned 404 (LIT-8444)"} - {id: mgmt.cost_tracking.estimate.happy_path, module: mgmt, tier: P2, surface: api, assertions: [happy_path], source: "cost_tracking_settings.py", rationale: "Cost estimate (smoke)"} - {id: mgmt.router_settings.update.happy_path, module: mgmt, tier: P2, surface: api, assertions: [happy_path], source: "router_settings_endpoints.py", rationale: "Router config (smoke)"} - {id: mgmt.jwt_key_mapping.new.happy_path, module: mgmt, tier: P2, surface: api, assertions: [happy_path], source: "jwt_key_mapping_endpoints.py", rationale: "JWT->key mapping (smoke)"} diff --git a/tests/e2e/logging/logging_client.py b/tests/e2e/logging/logging_client.py index c2f987ea33d..63f48def8f7 100644 --- a/tests/e2e/logging/logging_client.py +++ b/tests/e2e/logging/logging_client.py @@ -17,35 +17,40 @@ import base64 import json import os import time +from collections.abc import Callable from dataclasses import dataclass -from typing import Callable, Literal +from typing import Literal import pytest -from pydantic import BaseModel, ConfigDict, Field, JsonValue, TypeAdapter, ValidationError - from e2e_config import POLL_INTERVAL, POLL_TIMEOUT, settle_propagation -from proxy_client import ProxyClient from e2e_http import ( URL, AuthHeaders, - require_successful_call, NoBody, + Result, StreamingResponse, Success, get, + require_successful_call, unwrap, ) from models import ( AnthropicMessagesBody, + CallbackDeleteBody, + CallbackDeleteResponse, ChatBody, ChatMessage, ChatResponse, ChatTool, ChatToolFunction, + ConfigCallbacksResponse, + ConfigUpdateCallbacksBody, + ConfigUpdateResponse, KeyGenerateBody, KeyLoggingCallback, KeyLoggingCallbackVars, KeyMetadata, + LitellmCallbackSettings, LiteLLMParamsBody, OrgDeleteBody, OrgNewBody, @@ -58,6 +63,8 @@ from models import ( UserNewBody, UserNewResponse, ) +from proxy_client import ProxyClient +from pydantic import BaseModel, ConfigDict, Field, JsonValue, TypeAdapter, ValidationError # Deliberately invalid *upstream provider* key for failure-path tests. # Not a LiteLLM virtual key; OpenAI must reject it after the proxy accepts the call. @@ -488,6 +495,46 @@ class LoggingClient: def delete_model(self, model_id: str) -> None: self.proxy.delete_model(model_id) + def set_litellm_callbacks(self, settings: LitellmCallbackSettings) -> ConfigUpdateResponse: + return unwrap( + self.proxy.transport.post( + "/config/update", + headers=self.proxy.transport.master, + json=ConfigUpdateCallbacksBody(litellm_settings=settings), + response_type=ConfigUpdateResponse, + ) + ) + + def clear_litellm_callbacks(self, *keys: Literal["callbacks", "failure_callback"]) -> None: + _ = self.set_litellm_callbacks( + LitellmCallbackSettings( + callbacks=[] if "callbacks" in keys else None, + failure_callback=[] if "failure_callback" in keys else None, + ) + ) + + def config_callback_names(self) -> set[str]: + return { + entry.name + for entry in unwrap( + self.proxy.transport.get( + "/get/config/callbacks", + headers=self.proxy.transport.master, + params=NoBody(), + response_type=ConfigCallbacksResponse, + ) + ).callbacks + if not entry.read_only + } + + def delete_config_callback(self, callback_name: str) -> Result[CallbackDeleteResponse]: + return self.proxy.transport.post( + "/config/callback/delete", + headers=self.proxy.transport.master, + json=CallbackDeleteBody(callback_name=callback_name), + response_type=CallbackDeleteResponse, + ) + def chat(self, key: str, model: str, text: str) -> ChatResponse: return unwrap( self.proxy.chat( diff --git a/tests/e2e/logging/test_callback_delete_e2e.py b/tests/e2e/logging/test_callback_delete_e2e.py new file mode 100644 index 00000000000..9462edd43f2 --- /dev/null +++ b/tests/e2e/logging/test_callback_delete_e2e.py @@ -0,0 +1,78 @@ +"""A callback listed by GET /get/config/callbacks must be deletable with POST /config/callback/delete.""" + +from collections.abc import Callable + +import pytest +from e2e_http import unwrap +from lifecycle import ResourceManager +from logging_client import LoggingClient +from models import LitellmCallbackSettings + +pytestmark = pytest.mark.e2e + +CALLBACK_NAME = "langsmith" + + +class TestCallbackDelete: + def _assert_listed_then_deleted( + self, + client: LoggingClient, + resources: ResourceManager, + settings: LitellmCallbackSettings, + cleanup: Callable[[], object], + ) -> None: + response = client.set_litellm_callbacks(settings) + assert "success" in response.message.lower(), ( + f"/config/update reported {response.message!r}, expected a success message" + ) + resources.defer(cleanup) + + names = client.config_callback_names() + assert CALLBACK_NAME in names, ( + f"GET /get/config/callbacks does not list {CALLBACK_NAME!r} after it was " + f"configured via /config/update; listed {sorted(names)}" + ) + + deleted = unwrap(client.delete_config_callback(CALLBACK_NAME)) + assert deleted.removed_callback == CALLBACK_NAME, ( + f"/config/callback/delete removed {deleted.removed_callback!r}, expected {CALLBACK_NAME!r}" + ) + + names_after = client.config_callback_names() + assert CALLBACK_NAME not in names_after, ( + f"GET /get/config/callbacks still lists {CALLBACK_NAME!r} after a successful " + f"delete; listed {sorted(names_after)}" + ) + + @pytest.mark.covers("mgmt.callback.delete.removes_configured_callback", exercised_on=[]) + def test_callback_under_callbacks_key_is_listed_and_deletable( + self, client: LoggingClient, resources: ResourceManager + ) -> None: + self._assert_listed_then_deleted( + client, + resources, + LitellmCallbackSettings(callbacks=[CALLBACK_NAME]), + lambda: client.clear_litellm_callbacks("callbacks"), + ) + + @pytest.mark.covers("mgmt.callback.delete.removes_configured_callback", exercised_on=[]) + def test_callback_under_failure_callback_key_is_listed_and_deletable( + self, client: LoggingClient, resources: ResourceManager + ) -> None: + self._assert_listed_then_deleted( + client, + resources, + LitellmCallbackSettings(failure_callback=[CALLBACK_NAME]), + lambda: client.clear_litellm_callbacks("failure_callback"), + ) + + @pytest.mark.covers("mgmt.callback.delete.removes_configured_callback", exercised_on=[]) + def test_callback_under_success_callback_key_is_deletable( + self, client: LoggingClient, resources: ResourceManager + ) -> None: + self._assert_listed_then_deleted( + client, + resources, + LitellmCallbackSettings(success_callback=[CALLBACK_NAME]), + lambda: client.delete_config_callback(CALLBACK_NAME), + ) diff --git a/tests/e2e/models.py b/tests/e2e/models.py index e027c410e44..6bd8df2e1e9 100644 --- a/tests/e2e/models.py +++ b/tests/e2e/models.py @@ -1187,6 +1187,55 @@ class ConfigFieldList(RootModel[tuple[ConfigField, ...]]): """GET /config/list answers with a bare array of general_settings fields.""" +class LitellmCallbackSettings(BaseModel): + """The litellm_settings callback lists /config/update merges. Unset keys are + left out of the request so the proxy keeps whatever the stored row holds.""" + + model_config = ConfigDict(extra="ignore") + success_callback: list[str] | None = None + failure_callback: list[str] | None = None + callbacks: list[str] | None = None + + +class ConfigUpdateCallbacksBody(BaseModel): + """POST /config/update body: the same shape the Admin UI's Logging page + sends when an admin adds a callback (litellm_settings plus, in the UI's + case, environment_variables the test does not need).""" + + litellm_settings: LitellmCallbackSettings + + +class ConfigUpdateResponse(BaseModel): + model_config = ConfigDict(extra="ignore") + message: str + + +class ConfigCallbackEntry(BaseModel): + """One row of GET /get/config/callbacks `callbacks`. `type` is which + configured list the entry came from (success, failure, success_and_failure).""" + + model_config = ConfigDict(extra="ignore") + name: str + type: str | None = None + read_only: bool | None = None + + +class ConfigCallbacksResponse(BaseModel): + model_config = ConfigDict(extra="ignore") + callbacks: list[ConfigCallbackEntry] = [] + + +class CallbackDeleteBody(BaseModel): + callback_name: str + + +class CallbackDeleteResponse(BaseModel): + model_config = ConfigDict(extra="ignore") + message: str + removed_callback: str + remaining_callbacks: list[str] = [] + + class CostMapEntry(BaseModel): model_config = ConfigDict(extra="ignore") litellm_provider: str | None = None diff --git a/tests/unit/proxy/proxy_server/test_routes_config.py b/tests/unit/proxy/proxy_server/test_routes_config.py index 4234cdad23d..01dd877eb6d 100644 --- a/tests/unit/proxy/proxy_server/test_routes_config.py +++ b/tests/unit/proxy/proxy_server/test_routes_config.py @@ -14,10 +14,14 @@ Routes covered: from __future__ import annotations import asyncio +import contextlib import json +from collections.abc import Callable from unittest.mock import AsyncMock, MagicMock +import httpx import pytest +from fastapi.testclient import TestClient from .conftest import VOLATILE_KEYS, normalize @@ -944,6 +948,129 @@ def test_config_callback_delete_not_found(client, auth_as, mock_prisma, monkeypa assert "langfuse" in str(response.json()).lower() or "not found" in str(response.json()).lower() +def _delete_callback_roundtrip( + client: TestClient, + auth_as: Callable[..., contextlib.AbstractContextManager[None]], + mock_prisma: MagicMock, + monkeypatch: pytest.MonkeyPatch, + litellm_settings: dict[str, object], + callback_name: str, +) -> tuple[httpx.Response, dict[str, object]]: + """POST /config/callback/delete as admin against a stubbed config, and + return (response, saved litellm_settings) so each case pins the full body + plus the exact config persisted.""" + from litellm.proxy import proxy_server as ps + from litellm.proxy._types import LitellmUserRoles + + _install_litellm_config(mock_prisma) + monkeypatch.setattr(ps, "prisma_client", mock_prisma) + monkeypatch.setattr(ps, "store_model_in_db", True) + + fake_proxy_config = MagicMock() + fake_proxy_config.get_config = AsyncMock(return_value={"litellm_settings": litellm_settings}) + fake_proxy_config.save_config = AsyncMock() + fake_proxy_config.add_deployment = AsyncMock() + monkeypatch.setattr(ps, "proxy_config", fake_proxy_config) + + with auth_as(LitellmUserRoles.PROXY_ADMIN): + response = client.post("/config/callback/delete", json={"callback_name": callback_name}) + saved = fake_proxy_config.save_config.await_args.kwargs["new_config"]["litellm_settings"] + return response, saved + + +def test_config_callback_delete_from_failure_callback( + client: TestClient, + auth_as: Callable[..., contextlib.AbstractContextManager[None]], + mock_prisma: MagicMock, + monkeypatch: pytest.MonkeyPatch, +): + """A callback configured only under litellm_settings.failure_callback is + listed by GET /get/config/callbacks, so the delete route must remove it.""" + response, saved = _delete_callback_roundtrip( + client, + auth_as, + mock_prisma, + monkeypatch, + {"success_callback": ["slack"], "failure_callback": ["datadog", "sentry"]}, + "datadog", + ) + assert response.status_code == 200 + assert response.json()["removed_callback"] == "datadog" + assert saved == {"success_callback": ["slack"], "failure_callback": ["sentry"]} + assert response.json()["remaining_callbacks"] == ["slack", "sentry"] + + +def test_config_callback_delete_from_callbacks_list( + client: TestClient, + auth_as: Callable[..., contextlib.AbstractContextManager[None]], + mock_prisma: MagicMock, + monkeypatch: pytest.MonkeyPatch, +): + """A callback configured only under litellm_settings.callbacks is deleted + while unrelated lists pass through untouched.""" + response, saved = _delete_callback_roundtrip( + client, + auth_as, + mock_prisma, + monkeypatch, + {"success_callback": ["langfuse"], "callbacks": ["datadog"], "drop_params": True}, + "datadog", + ) + assert response.status_code == 200 + assert response.json()["removed_callback"] == "datadog" + assert saved == {"success_callback": ["langfuse"], "callbacks": [], "drop_params": True} + assert response.json()["remaining_callbacks"] == ["langfuse"] + + +def test_config_callback_delete_from_multiple_lists( + client: TestClient, + auth_as: Callable[..., contextlib.AbstractContextManager[None]], + mock_prisma: MagicMock, + monkeypatch: pytest.MonkeyPatch, +): + """A name present in both success_callback and callbacks is removed from + both, and remaining_callbacks reports the leftovers of every list.""" + response, saved = _delete_callback_roundtrip( + client, + auth_as, + mock_prisma, + monkeypatch, + { + "success_callback": ["datadog", "slack"], + "failure_callback": ["sentry"], + "callbacks": ["langfuse", "datadog"], + }, + "datadog", + ) + assert response.status_code == 200 + assert saved == { + "success_callback": ["slack"], + "failure_callback": ["sentry"], + "callbacks": ["langfuse"], + } + assert response.json()["remaining_callbacks"] == ["slack", "sentry", "langfuse"] + + +def test_config_callback_delete_case_insensitive( + client: TestClient, + auth_as: Callable[..., contextlib.AbstractContextManager[None]], + mock_prisma: MagicMock, + monkeypatch: pytest.MonkeyPatch, +): + """A mixed-case request name still matches the configured lowercase entry.""" + response, saved = _delete_callback_roundtrip( + client, + auth_as, + mock_prisma, + monkeypatch, + {"callbacks": ["datadog", "otel"]}, + "DataDog", + ) + assert response.status_code == 200 + assert response.json()["removed_callback"] == "datadog" + assert saved == {"callbacks": ["otel"]} + + # --------------------------------------------------------------------------- # GET /get/config/callbacks # ---------------------------------------------------------------------------