diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index 92a75bf953a..3a6f7085733 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -18467,6 +18467,21 @@ async def delete_config_general_settings( return response +_CALLBACK_LIST_KEYS: Final = ("success_callback", "failure_callback", "callbacks") + + +def _configured_callback_names(value: object) -> tuple[object, ...]: + if isinstance(value, str): + return (value,) + if isinstance(value, (list, tuple, dict)): + return tuple(value) + return () + + +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"], @@ -18507,19 +18522,27 @@ async def delete_callback( # Check if callback exists in current configuration litellm_settings: Final = config.get("litellm_settings", {}) - success_callbacks: Final = litellm_settings.get("success_callback", []) + 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 = {key: list(configured_lists[key]) for key in matching_keys} + after_callbacks: Final = { + key: [entry for entry in configured_lists[key] if not _is_callback_name(entry, callback_name)] + for key in matching_keys + } + config["litellm_settings"] = {**litellm_settings, **after_callbacks} # Save the updated configuration await proxy_config.save_config(new_config=config) @@ -18528,8 +18551,8 @@ async def delete_callback( create_config_audit_log( "litellm_settings", "deleted", - {"success_callback": before_success_callbacks}, - {"success_callback": success_callbacks}, + before_callbacks, + after_callbacks, user_api_key_dict, ) ) @@ -18537,10 +18560,13 @@ async def delete_callback( # Restart the proxy to apply changes await proxy_config.add_deployment(prisma_client=prisma_client, proxy_logging_obj=proxy_logging_obj) + updated_settings: Final = config["litellm_settings"] return { "message": f"Successfully deleted callback: {callback_name}", "removed_callback": callback_name, - "remaining_callbacks": success_callbacks, + "remaining_callbacks": [ + entry for key in _CALLBACK_LIST_KEYS for entry in _configured_callback_names(updated_settings.get(key)) + ], "deleted_at": datetime.now().isoformat(), } diff --git a/tests/e2e/logging/logging_client.py b/tests/e2e/logging/logging_client.py index c2f987ea33d..3aff01a176d 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,56 @@ class LoggingClient: def delete_model(self, model_id: str) -> None: self.proxy.delete_model(model_id) + # ---- global callback configuration (Admin UI Logging & Alerts surface) ---- + + def set_litellm_callbacks(self, settings: LitellmCallbackSettings) -> ConfigUpdateResponse: + """POST /config/update, the route the Admin UI's Logging page uses to add + a callback.""" + 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: + """Empty the given litellm_settings callback lists so teardown leaves the + stored row as it was. success_callback is absent on purpose: /config/update + unions that key instead of replacing it, so entries there can only leave + through /config/callback/delete.""" + _ = 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]: + """Every name GET /get/config/callbacks lists as an active callback.""" + 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 + } + + def delete_config_callback(self, callback_name: str) -> Result[CallbackDeleteResponse]: + """POST /config/callback/delete. Returns the raw Result so the caller can + unwrap for the success contract or ignore it for best-effort cleanup.""" + 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..71d3243b8c2 --- /dev/null +++ b/tests/e2e/logging/test_callback_delete_e2e.py @@ -0,0 +1,85 @@ +"""Live e2e: a callback the Admin UI lists must also be deletable. + +GET /get/config/callbacks enumerates litellm_settings.success_callback, +failure_callback and callbacks, plus runtime-only loggers, but +POST /config/callback/delete only looks inside success_callback. A callback +configured under `callbacks` or `failure_callback` is listed as active on the +Logging & Alerts -> Logging Callbacks page yet its delete answers 404 "Callback +not found in active configuration". The success_callback case is the control: +it proves the add/list/delete round trip works when the configured key is the +one the delete route actually reads. +""" + +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 = "datadog" + + +def _assert_listed_then_deleted( + 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)}" + ) + + +class TestCallbackDelete: + def test_callback_under_callbacks_key_is_listed_and_deletable( + self, client: LoggingClient, resources: ResourceManager + ) -> None: + _assert_listed_then_deleted( + client, + resources, + LitellmCallbackSettings(callbacks=[CALLBACK_NAME]), + lambda: client.clear_litellm_callbacks("callbacks"), + ) + + def test_callback_under_failure_callback_key_is_listed_and_deletable( + self, client: LoggingClient, resources: ResourceManager + ) -> None: + _assert_listed_then_deleted( + client, + resources, + LitellmCallbackSettings(failure_callback=[CALLBACK_NAME]), + lambda: client.clear_litellm_callbacks("failure_callback"), + ) + + def test_callback_under_success_callback_key_is_deletable( + self, client: LoggingClient, resources: ResourceManager + ) -> None: + _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 bd7f5171172..98476ea6d74 100644 --- a/tests/e2e/models.py +++ b/tests/e2e/models.py @@ -1147,6 +1147,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/test_litellm/proxy/proxy_server/test_routes_config.py b/tests/test_litellm/proxy/proxy_server/test_routes_config.py index 4234cdad23d..86b07440780 100644 --- a/tests/test_litellm/proxy/proxy_server/test_routes_config.py +++ b/tests/test_litellm/proxy/proxy_server/test_routes_config.py @@ -944,6 +944,102 @@ 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, auth_as, mock_prisma, monkeypatch, litellm_settings, callback_name): + """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, auth_as, mock_prisma, 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, auth_as, mock_prisma, 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, auth_as, mock_prisma, 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, auth_as, mock_prisma, 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 # ---------------------------------------------------------------------------