mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-16 23:41:43 +00:00
fix(credentials): answer 409 on a name collision, let PATCH resolve values from model_id
POST /credentials let a duplicate name hit the unique index and handed back
Prisma's "Unique constraint failed" as a 500, so callers string-matched that
message to tell a caller mistake from a server fault. The unique violation now
maps to a 409 whose message names the PATCH route, two concurrent creates of
one name agree on it, and the detection lives in a repository helper the five
hand-rolled copies can move onto later
PATCH /credentials/{name} took a CredentialItem body, so the model_id the
Terraform adopt path sent was dropped. It now accepts UpdateCredentialItem and
shares the deployment lookup with create. Both handlers take the router as a
FastAPI dependency instead of reading the proxy global, which is what the
tests override
This commit is contained in:
parent
c788769129
commit
81806f33cf
5 changed files with 242 additions and 34 deletions
|
|
@ -5,6 +5,8 @@ These are the canonical credential types for the proxy. They live in the model
|
|||
layer; ``litellm.types.utils`` re-exports them for backwards compatibility.
|
||||
"""
|
||||
|
||||
from collections.abc import Mapping
|
||||
|
||||
from pydantic import BaseModel, model_validator
|
||||
|
||||
|
||||
|
|
@ -27,3 +29,10 @@ class CreateCredentialItem(CredentialBase):
|
|||
if not values.get("credential_values") and not values.get("model_id"):
|
||||
raise ValueError("Either credential_values or model_id must be set")
|
||||
return values
|
||||
|
||||
|
||||
class UpdateCredentialItem(BaseModel):
|
||||
credential_name: str
|
||||
credential_info: Mapping[str, object]
|
||||
credential_values: Mapping[str, object] | None = None
|
||||
model_id: str | None = None
|
||||
|
|
|
|||
|
|
@ -2,25 +2,31 @@
|
|||
CRUD endpoints for storing reusable credentials.
|
||||
"""
|
||||
|
||||
from collections.abc import Mapping
|
||||
from typing import (
|
||||
Annotated,
|
||||
Final,
|
||||
cast, # noqa: TID251 # jsonify_object in proxy/utils.py is annotated with a bare dict
|
||||
)
|
||||
|
||||
from fastapi import APIRouter, Depends, HTTPException, Path, Request, Response
|
||||
from pydantic import TypeAdapter
|
||||
|
||||
import litellm
|
||||
from litellm._logging import verbose_proxy_logger
|
||||
from litellm.litellm_core_utils.credential_accessor import CredentialAccessor
|
||||
from litellm.litellm_core_utils.litellm_logging import _get_masked_values
|
||||
from litellm.models.credentials import UpdateCredentialItem
|
||||
from litellm.proxy._types import CommonProxyErrors, UserAPIKeyAuth
|
||||
from litellm.proxy.auth.user_api_key_auth import user_api_key_auth
|
||||
from litellm.proxy.common_utils.encrypt_decrypt_utils import encrypt_value_helper
|
||||
from litellm.proxy.utils import handle_exception_on_proxy, jsonify_object
|
||||
from litellm.repositories.base_repository import is_unique_violation
|
||||
from litellm.repositories.credentials_repository import CredentialsRepository
|
||||
from litellm.types.utils import CreateCredentialItem, CredentialItem
|
||||
|
||||
router: Final = APIRouter()
|
||||
_CREDENTIAL_DICT_ADAPTER: Final = TypeAdapter(dict[str, object])
|
||||
|
||||
|
||||
class CredentialHelperUtils:
|
||||
|
|
@ -40,6 +46,33 @@ class CredentialHelperUtils:
|
|||
)
|
||||
|
||||
|
||||
def _credential_exists_detail(credential_name: str) -> str:
|
||||
return (
|
||||
f"Credential '{credential_name}' already exists. "
|
||||
f"Update it with PATCH /credentials/{credential_name}, or delete it first."
|
||||
)
|
||||
|
||||
|
||||
def get_llm_router() -> litellm.Router | None:
|
||||
from litellm.proxy.proxy_server import llm_router
|
||||
|
||||
return llm_router
|
||||
|
||||
|
||||
def _resolve_deployment_credentials(llm_router: litellm.Router | None, model_id: str) -> Mapping[str, object]:
|
||||
if llm_router is None:
|
||||
raise HTTPException(
|
||||
status_code=500,
|
||||
detail="LLM router not found. Please ensure you have a valid router instance.",
|
||||
)
|
||||
if llm_router.get_deployment(model_id) is None:
|
||||
raise HTTPException(status_code=404, detail="Model not found")
|
||||
credential_values: Final = llm_router.get_deployment_credentials(model_id)
|
||||
if credential_values is None:
|
||||
raise HTTPException(status_code=404, detail="Model not found")
|
||||
return _CREDENTIAL_DICT_ADAPTER.validate_python(credential_values)
|
||||
|
||||
|
||||
@router.post(
|
||||
"/credentials",
|
||||
dependencies=[Depends(user_api_key_auth)],
|
||||
|
|
@ -50,13 +83,14 @@ async def create_credential(
|
|||
fastapi_response: Response,
|
||||
credential: CreateCredentialItem,
|
||||
user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth),
|
||||
llm_router: Annotated[litellm.Router | None, Depends(get_llm_router)] = None,
|
||||
):
|
||||
"""
|
||||
[BETA] endpoint. This might change unexpectedly.
|
||||
Stores credential in DB.
|
||||
Reloads credentials in memory.
|
||||
"""
|
||||
from litellm.proxy.proxy_server import llm_router, prisma_client
|
||||
from litellm.proxy.proxy_server import prisma_client
|
||||
|
||||
try:
|
||||
if prisma_client is None:
|
||||
|
|
@ -64,29 +98,19 @@ async def create_credential(
|
|||
status_code=500,
|
||||
detail={"error": CommonProxyErrors.db_not_connected_error.value},
|
||||
)
|
||||
if credential.model_id:
|
||||
if llm_router is None:
|
||||
raise HTTPException(
|
||||
status_code=500,
|
||||
detail="LLM router not found. Please ensure you have a valid router instance.",
|
||||
)
|
||||
# get model from router
|
||||
model: Final = llm_router.get_deployment(credential.model_id)
|
||||
if model is None:
|
||||
raise HTTPException(status_code=404, detail="Model not found")
|
||||
credential_values: Final = llm_router.get_deployment_credentials(credential.model_id)
|
||||
if credential_values is None:
|
||||
raise HTTPException(status_code=404, detail="Model not found")
|
||||
credential.credential_values = credential_values
|
||||
|
||||
if credential.credential_values is None:
|
||||
credential_values: Final = (
|
||||
_resolve_deployment_credentials(llm_router, credential.model_id)
|
||||
if credential.model_id
|
||||
else credential.credential_values
|
||||
)
|
||||
if credential_values is None:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail="Credential values are required. Unable to infer credential values from model ID.",
|
||||
)
|
||||
processed_credential: Final = CredentialItem(
|
||||
credential_name=credential.credential_name,
|
||||
credential_values=credential.credential_values,
|
||||
credential_values=_CREDENTIAL_DICT_ADAPTER.validate_python(credential_values),
|
||||
credential_info=credential.credential_info,
|
||||
)
|
||||
encrypted_credential: Final = CredentialHelperUtils.encrypt_credential_values(processed_credential)
|
||||
|
|
@ -94,13 +118,18 @@ async def create_credential(
|
|||
credentials_dict_jsonified: Final = cast( # cast-ok: deep-copies a model_dump, so keys are str
|
||||
"dict[str, object]", jsonify_object(credentials_dict)
|
||||
)
|
||||
await CredentialsRepository(prisma_client).create(
|
||||
data={
|
||||
**credentials_dict_jsonified,
|
||||
"created_by": user_api_key_dict.user_id,
|
||||
"updated_by": user_api_key_dict.user_id,
|
||||
}
|
||||
)
|
||||
try:
|
||||
await CredentialsRepository(prisma_client).create(
|
||||
data={
|
||||
**credentials_dict_jsonified,
|
||||
"created_by": user_api_key_dict.user_id,
|
||||
"updated_by": user_api_key_dict.user_id,
|
||||
}
|
||||
)
|
||||
except Exception as e:
|
||||
if not is_unique_violation(e):
|
||||
raise
|
||||
raise HTTPException(status_code=409, detail=_credential_exists_detail(credential.credential_name))
|
||||
|
||||
## ADD TO LITELLM ##
|
||||
CredentialAccessor.upsert_credentials([processed_credential])
|
||||
|
|
@ -300,9 +329,10 @@ def update_db_credential(
|
|||
async def update_credential(
|
||||
request: Request,
|
||||
fastapi_response: Response,
|
||||
credential: CredentialItem,
|
||||
credential: UpdateCredentialItem,
|
||||
credential_name: str = Path(..., description="The credential name, percent-decoded; may contain slashes"),
|
||||
user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth),
|
||||
llm_router: Annotated[litellm.Router | None, Depends(get_llm_router)] = None,
|
||||
):
|
||||
"""
|
||||
[BETA] endpoint. This might change unexpectedly.
|
||||
|
|
@ -319,7 +349,16 @@ async def update_credential(
|
|||
db_credential: Final = await credentials_repository.find_by_name(credential_name)
|
||||
if db_credential is None:
|
||||
raise HTTPException(status_code=404, detail="Credential not found in DB.")
|
||||
merged_credential: Final = update_db_credential(db_credential, credential)
|
||||
patch: Final = CredentialItem(
|
||||
credential_name=credential.credential_name,
|
||||
credential_info=_CREDENTIAL_DICT_ADAPTER.validate_python(credential.credential_info),
|
||||
credential_values=_CREDENTIAL_DICT_ADAPTER.validate_python(
|
||||
_resolve_deployment_credentials(llm_router, credential.model_id)
|
||||
if credential.model_id
|
||||
else credential.credential_values or {}
|
||||
),
|
||||
)
|
||||
merged_credential: Final = update_db_credential(db_credential, patch)
|
||||
credential_object_jsonified: Final = cast( # cast-ok: deep-copies a model_dump, so keys are str
|
||||
"dict[str, object]", jsonify_object(merged_credential.model_dump())
|
||||
)
|
||||
|
|
@ -341,11 +380,11 @@ async def update_credential(
|
|||
|
||||
if existing_in_memory is not None:
|
||||
in_memory_values: Final = dict(existing_in_memory.credential_values or {})
|
||||
if credential.credential_values:
|
||||
in_memory_values.update(credential.credential_values)
|
||||
if patch.credential_values:
|
||||
in_memory_values.update(patch.credential_values)
|
||||
in_memory_info: Final = dict(existing_in_memory.credential_info or {})
|
||||
if credential.credential_info:
|
||||
in_memory_info.update(credential.credential_info)
|
||||
if patch.credential_info:
|
||||
in_memory_info.update(patch.credential_info)
|
||||
updated_in_memory: Final = CredentialItem(
|
||||
credential_name=new_name,
|
||||
credential_values=in_memory_values,
|
||||
|
|
|
|||
|
|
@ -117,3 +117,13 @@ class BaseRepository(ABC, Generic[T]):
|
|||
"""Check if a record exists."""
|
||||
record: Final = await self.table.find_unique(where={id_field: id_value})
|
||||
return record is not None
|
||||
|
||||
|
||||
def is_unique_violation(exc: BaseException) -> bool:
|
||||
try:
|
||||
from prisma.errors import UniqueViolationError
|
||||
except ImportError:
|
||||
return "P2002" in str(exc) or "unique constraint" in str(exc).lower()
|
||||
if isinstance(exc, UniqueViolationError):
|
||||
return True
|
||||
return getattr(exc, "code", None) == "P2002"
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
"""Tests for the credential management endpoints."""
|
||||
|
||||
import json
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
|
@ -9,6 +10,7 @@ from fastapi.testclient import TestClient
|
|||
import litellm
|
||||
from litellm.proxy._types import UserAPIKeyAuth
|
||||
from litellm.proxy.auth.user_api_key_auth import user_api_key_auth
|
||||
from litellm.proxy.credential_endpoints.endpoints import get_llm_router
|
||||
from litellm.proxy.proxy_server import app
|
||||
from litellm.types.utils import CredentialItem
|
||||
|
||||
|
|
@ -47,23 +49,27 @@ def _list_credentials():
|
|||
@pytest.fixture
|
||||
def credential_store():
|
||||
"""Stands the credential store up for one test: whether the database is reachable, what
|
||||
the proxy is already serving from memory, and what each repository call hands back."""
|
||||
the proxy is already serving from memory, which router deployments resolve against, and
|
||||
what each repository call hands back."""
|
||||
|
||||
def install(
|
||||
*,
|
||||
connected: bool = True,
|
||||
in_memory: tuple[object, ...] = (),
|
||||
llm_router: object | None = None,
|
||||
**repository_calls: AsyncMock,
|
||||
) -> None:
|
||||
patch("litellm.proxy.proxy_server.prisma_client", MagicMock() if connected else None).start()
|
||||
patch("litellm.proxy.proxy_server.master_key", "sk-test-master").start()
|
||||
patch.object(litellm, "credential_list", list(in_memory)).start()
|
||||
app.dependency_overrides[get_llm_router] = lambda: llm_router
|
||||
repository = patch("litellm.proxy.credential_endpoints.endpoints.CredentialsRepository").start()
|
||||
for call_name, result in repository_calls.items():
|
||||
setattr(repository.return_value, call_name, result)
|
||||
|
||||
yield install
|
||||
patch.stopall()
|
||||
app.dependency_overrides.pop(get_llm_router, None)
|
||||
|
||||
|
||||
def test_update_credential_answers_404_when_the_credential_does_not_exist(credential_store):
|
||||
|
|
@ -122,7 +128,9 @@ def test_delete_credential_answers_404_when_the_credential_does_not_exist(creden
|
|||
|
||||
response = _delete_credential("definitely-not-there")
|
||||
|
||||
assert response.status_code == 404, f"delete of a missing credential answered {response.status_code}: {response.text}"
|
||||
assert response.status_code == 404, (
|
||||
f"delete of a missing credential answered {response.status_code}: {response.text}"
|
||||
)
|
||||
assert "definitely-not-there" in response.text
|
||||
|
||||
|
||||
|
|
@ -195,3 +203,130 @@ def test_get_credentials_answers_an_error_status_when_the_listing_fails(credenti
|
|||
|
||||
assert response.status_code == 500, f"failed listing answered {response.status_code}: {response.text}"
|
||||
assert response.json().get("success") is not True
|
||||
|
||||
|
||||
def _create_credential(body: dict):
|
||||
return _call_as_admin("POST", "/credentials", body)
|
||||
|
||||
|
||||
class _UniqueViolation(Exception):
|
||||
code = "P2002"
|
||||
|
||||
|
||||
def test_create_credential_answers_409_when_the_name_is_already_taken(credential_store):
|
||||
"""Regression: the unique index used to surface as a Prisma 500 that callers string-matched."""
|
||||
credential_store(
|
||||
create=AsyncMock(side_effect=_UniqueViolation("Unique constraint failed on the fields: (`credential_name`)")),
|
||||
)
|
||||
|
||||
response = _create_credential(
|
||||
{"credential_name": "aws_bedrock", "credential_values": {"aws_access_key_id": "new"}, "credential_info": {}},
|
||||
)
|
||||
|
||||
assert response.status_code == 409, f"name collision answered {response.status_code}: {response.text}"
|
||||
message = response.json()["error"]["message"]
|
||||
assert message == (
|
||||
"Credential 'aws_bedrock' already exists. Update it with PATCH /credentials/aws_bedrock, or delete it first."
|
||||
), f"the operator reads this message verbatim: {message}"
|
||||
assert "Unique constraint" not in response.text, f"the Prisma internals must not leak: {response.text}"
|
||||
|
||||
|
||||
def test_create_credential_still_answers_500_when_the_write_fails_for_another_reason(credential_store):
|
||||
credential_store(create=AsyncMock(side_effect=Exception("connection reset by peer")))
|
||||
|
||||
response = _create_credential(
|
||||
{"credential_name": "aws_bedrock", "credential_values": {"aws_access_key_id": "new"}, "credential_info": {}},
|
||||
)
|
||||
|
||||
assert response.status_code == 500, f"database fault answered {response.status_code}: {response.text}"
|
||||
|
||||
|
||||
def test_create_credential_still_answers_200_for_a_name_that_is_free(credential_store):
|
||||
find_by_name = AsyncMock()
|
||||
credential_store(find_by_name=find_by_name, create=AsyncMock(return_value=None))
|
||||
|
||||
response = _create_credential(
|
||||
{"credential_name": "brand_new", "credential_values": {"aws_access_key_id": "new"}, "credential_info": {}},
|
||||
)
|
||||
|
||||
assert response.status_code == 200, response.text
|
||||
assert response.json()["success"] is True
|
||||
find_by_name.assert_not_awaited(), "the unique index is the guard; create must not add a lookup"
|
||||
|
||||
|
||||
def test_update_credential_resolves_credential_values_from_model_id_like_create(credential_store):
|
||||
"""Regression: PATCH dropped ``model_id`` from the body, so an update that named a
|
||||
deployment instead of raw values wrote whatever the caller sent, or nothing."""
|
||||
stored = CredentialItem(
|
||||
credential_name="from-deployment",
|
||||
credential_values={"api_key": "sk-old"},
|
||||
credential_info={},
|
||||
)
|
||||
update_by_name = AsyncMock(return_value=None)
|
||||
router = MagicMock()
|
||||
router.get_deployment.return_value = {"model_name": "gpt-5.2"}
|
||||
router.get_deployment_credentials.return_value = {"api_key": "sk-from-deployment"}
|
||||
credential_store(find_by_name=AsyncMock(return_value=stored), update_by_name=update_by_name, llm_router=router)
|
||||
|
||||
response = _patch_credential(
|
||||
"from-deployment",
|
||||
{"credential_name": "from-deployment", "model_id": "deployment-1", "credential_info": {}},
|
||||
)
|
||||
|
||||
assert response.status_code == 200, response.text
|
||||
router.get_deployment_credentials.assert_called_once_with("deployment-1")
|
||||
written = json.loads(update_by_name.await_args.kwargs["data"]["credential_values"])
|
||||
assert set(written) == {"api_key"}
|
||||
assert written["api_key"] != "sk-old", "the deployment's values must replace the stored ones"
|
||||
assert written["api_key"] != "sk-from-deployment", "values are encrypted before they reach the table"
|
||||
|
||||
|
||||
def test_update_credential_answers_404_when_model_id_names_no_deployment(credential_store):
|
||||
stored = CredentialItem(
|
||||
credential_name="from-deployment", credential_values={"api_key": "sk-old"}, credential_info={}
|
||||
)
|
||||
update_by_name = AsyncMock(return_value=None)
|
||||
router = MagicMock()
|
||||
router.get_deployment.return_value = None
|
||||
credential_store(find_by_name=AsyncMock(return_value=stored), update_by_name=update_by_name, llm_router=router)
|
||||
|
||||
response = _patch_credential(
|
||||
"from-deployment",
|
||||
{"credential_name": "from-deployment", "model_id": "no-such-deployment", "credential_info": {}},
|
||||
)
|
||||
|
||||
assert response.status_code == 404, response.text
|
||||
update_by_name.assert_not_awaited()
|
||||
|
||||
|
||||
def test_update_credential_answers_500_when_model_id_is_given_but_no_router_is_loaded(credential_store):
|
||||
stored = CredentialItem(
|
||||
credential_name="from-deployment", credential_values={"api_key": "sk-old"}, credential_info={}
|
||||
)
|
||||
update_by_name = AsyncMock(return_value=None)
|
||||
credential_store(find_by_name=AsyncMock(return_value=stored), update_by_name=update_by_name, llm_router=None)
|
||||
|
||||
response = _patch_credential(
|
||||
"from-deployment",
|
||||
{"credential_name": "from-deployment", "model_id": "deployment-1", "credential_info": {}},
|
||||
)
|
||||
|
||||
assert response.status_code == 500, response.text
|
||||
update_by_name.assert_not_awaited()
|
||||
|
||||
|
||||
def test_update_credential_still_accepts_a_body_without_credential_values(credential_store):
|
||||
"""Renaming or re-tagging a credential sends only ``credential_info``; that must not 422."""
|
||||
stored = CredentialItem(credential_name="existing", credential_values={"api_key": "sk-old"}, credential_info={})
|
||||
update_by_name = AsyncMock(return_value=None)
|
||||
credential_store(find_by_name=AsyncMock(return_value=stored), update_by_name=update_by_name)
|
||||
|
||||
response = _patch_credential(
|
||||
"existing",
|
||||
{"credential_name": "existing", "credential_info": {"custom_llm_provider": "openai"}},
|
||||
)
|
||||
|
||||
assert response.status_code == 200, response.text
|
||||
written = update_by_name.await_args.kwargs["data"]
|
||||
assert json.loads(written["credential_info"]) == {"custom_llm_provider": "openai"}
|
||||
assert set(json.loads(written["credential_values"])) == {"api_key"}, "stored values survive an info-only patch"
|
||||
|
|
|
|||
17
ui/litellm-dashboard/src/lib/http/schema.d.ts
generated
vendored
17
ui/litellm-dashboard/src/lib/http/schema.d.ts
generated
vendored
|
|
@ -37944,6 +37944,21 @@ export interface components {
|
|||
*/
|
||||
blocked_users: string[];
|
||||
};
|
||||
/** UpdateCredentialItem */
|
||||
UpdateCredentialItem: {
|
||||
/** Credential Info */
|
||||
credential_info: {
|
||||
[key: string]: unknown;
|
||||
};
|
||||
/** Credential Name */
|
||||
credential_name: string;
|
||||
/** Credential Values */
|
||||
credential_values?: {
|
||||
[key: string]: unknown;
|
||||
} | null;
|
||||
/** Model Id */
|
||||
model_id?: string | null;
|
||||
};
|
||||
/**
|
||||
* UpdateCustomerRequest
|
||||
* @description Update a Customer, use this to update customer budgets etc
|
||||
|
|
@ -45535,7 +45550,7 @@ export interface operations {
|
|||
};
|
||||
requestBody: {
|
||||
content: {
|
||||
"application/json": components["schemas"]["CredentialItem"];
|
||||
"application/json": components["schemas"]["UpdateCredentialItem"];
|
||||
};
|
||||
};
|
||||
responses: {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue