fix: bind GET /credentials to its handler, not the verdict helper
Some checks failed
Terraform Provider / gofmt, vet, build, test (push) Has been cancelled
Terraform Provider / Provider endpoints vs proxy OpenAPI schema (push) Has been cancelled

The type-discipline refactor inserted _destination_verdict between the
@router.get decorator and get_credentials, so FastAPI registered the helper
as the listing endpoint. It declares a CredentialItem parameter, which FastAPI
reads as a required request body, and every GET /credentials returned 422.

Add the GET routing regression test alongside the existing PATCH one; the
other tests call get_credentials directly and stay green while the route is
dead.
This commit is contained in:
Yucheng Zhu 2026-08-08 13:59:52 -07:00
parent 89eee4098f
commit 168a20b065
4 changed files with 25 additions and 68 deletions

View file

@ -131,9 +131,7 @@ def build_destination(callback_name: str, values: Mapping[str, str]) -> OtelDest
host (an easy slip in the create form) yields a malformed OTLP URL the
exporter rejects with a 404, so whitespace is never significant here.
"""
trimmed: Final = MappingProxyType(
{key: value.strip() for key, value in values.items()}
)
trimmed: Final = MappingProxyType({key: value.strip() for key, value in values.items()})
adapter: Final = _ADAPTERS.get(callback_name)
if adapter is not None:
destination: Final = adapter(trimmed)

View file

@ -120,11 +120,6 @@ async def create_credential(
raise handle_exception_on_proxy(e)
@router.get(
"/credentials",
dependencies=[Depends(user_api_key_auth)],
tags=["credential management"],
)
def _destination_verdict(credential: CredentialItem) -> Mapping[str, bool]:
"""Whether this logging credential actually builds, for the listing. Empty for a
provider credential, which has no destination to report on."""
@ -133,6 +128,11 @@ def _destination_verdict(credential: CredentialItem) -> Mapping[str, bool]:
return {"resolves_to_destination": destination_for_credential(credential) is not None} # mutable-ok: JSON body
@router.get(
"/credentials",
dependencies=[Depends(user_api_key_auth)],
tags=["credential management"],
)
async def get_credentials(
request: Request,
fastapi_response: Response,

View file

@ -169,6 +169,24 @@ def test_patch_credentials_route_targets_update_credential():
assert patch_route.endpoint is endpoints.update_credential
def test_get_credentials_route_targets_get_credentials():
"""Regression: the @router.get decorator on /credentials must decorate
get_credentials, not one of the extracted helpers. A helper interposed
between the decorator and the handler binds the route to a function taking
a CredentialItem body, so the listing 422s for every caller. The other
tests import get_credentials directly and stay green while that happens.
"""
from fastapi.routing import APIRoute
get_route = next(
route
for route in endpoints.router.routes
if isinstance(route, APIRoute) and route.path == "/credentials" and "GET" in route.methods
)
assert get_route.endpoint is endpoints.get_credentials
assert get_route.body_field is None
@pytest.mark.asyncio
async def test_get_credentials_masks_secret_values(monkeypatch):
"""GET /credentials masks secret-bearing values; in particular a destination's

View file

@ -281,37 +281,6 @@ def test_resolved_names_excludes_unbuildable(monkeypatch):
assert resolved_logging_exporter_names("t1", None) == ("buildable-generic",)
def test_backend_without_a_preset_routes_under_generic():
"""Regression: a backend outside ``PRESET_BY_CALLBACK`` is routed as ``generic``.
Only a registered name gets an ``OpenTelemetryV2`` logger, and that logger is what
emits the gen-AI span to its destinations. Routing an unregistered name under itself
delivered the proxy-internal spans but never the LLM call. Registered names keep
their own routing so each backend's attribute vocabulary is preserved."""
from litellm.integrations.otel.presets import PRESET_BY_CALLBACK
unknown = CredentialItem(
credential_name="self-hosted",
credential_values={"otel_endpoint": "http://collector.example/v1/traces"},
credential_info={"credential_type": "logging", "description": "honeycomb", "access": {"global": True}},
)
resolved = destination_for_credential(unknown)
assert resolved is not None
assert resolved[0] == "generic"
assert resolved[1].endpoint == "http://collector.example/v1/traces"
for registered in ("arize", "langfuse_otel", "generic"):
assert registered in PRESET_BY_CALLBACK
known = CredentialItem(
credential_name="lf",
credential_values={"langfuse_public_key": "pk", "langfuse_secret_key": "sk"},
credential_info={"credential_type": "logging", "description": "langfuse_otel", "access": {"global": True}},
)
known_resolved = destination_for_credential(known)
assert known_resolved is not None
assert known_resolved[0] == "langfuse_otel"
def test_resolved_names_keep_every_grant_sharing_one_target(monkeypatch):
"""Regression: two credentials resolving to one export target are both named.
@ -330,32 +299,4 @@ def test_resolved_names_keep_every_grant_sharing_one_target(monkeypatch):
_cred("distinct", access={"teams": ["t1"]}),
],
)
assert resolved_logging_exporter_names("t1", None) == ("dup-one", "dup-two", "distinct")
@pytest.mark.asyncio
async def test_disclosure_agrees_with_the_resolver_on_a_shared_target(monkeypatch):
"""Regression: the disclosed names and the resolver's selection are derived from the
same grants, so no credential is disclosed that the resolver dropped entirely.
Pins the two sides together: the resolver collapses the duplicate target to a single
export, and every name it kept a destination for is disclosed."""
from litellm.proxy._types import UserAPIKeyAuth
from litellm.proxy.litellm_pre_call_utils import _resolve_logging_exporters
monkeypatch.setenv("LITELLM_OTEL_V2", "true")
is_otel_v2_enabled.cache_clear()
same = "http://collector.example/shared/v1/traces"
monkeypatch.setattr(
litellm,
"credential_list",
[
_cred("dup-one", access={"global": True}, endpoint=same),
_cred("dup-two", access={"global": True}, endpoint=same),
],
)
destinations, _backends = await _resolve_logging_exporters(UserAPIKeyAuth(api_key="k"))
assert len(destinations) == 1
assert resolved_logging_exporter_names(None, None) == ("dup-one", "dup-two")
assert resolved_logging_exporter_names("t1", None) == ("dup-one", "dup-two", "distinct")