mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-17 23:51:30 +00:00
fix(router): keep s3_output_bucket_name in the trusted credential snapshot
The output-bucket retrieval fix only worked via the AWS_S3_OUTPUT_BUCKET_NAME env var, never via per-model s3_output_bucket_name config. The proxy builds the trusted snapshot that retrieval validates against by round-tripping a deployment's litellm_params through CredentialLiteLLMParams in get_deployment_credentials_with_provider, and that strict allowlist did not declare s3_output_bucket_name, so the field was silently dropped before retrieval saw it (same trap as azure_ad_token in #30235). The snapshot branch of get_configured_s3_bucket_names was therefore dead in the model-routing path and output-bucket file ids were rejected as foreign. Declaring s3_output_bucket_name on CredentialLiteLLMParams lets it survive into the snapshot, so the existing multi-bucket validation works for per-model output buckets too. The PR's tests injected the field straight into the MappingProxyType, bypassing this filter, so they passed despite the live gap. _trusted now builds the snapshot through CredentialLiteLLMParams the way the proxy does, and a router-level regression test pins that get_deployment_credentials_with_provider preserves the output bucket. Both fail without this change.
This commit is contained in:
parent
1d407c2f26
commit
a9a322d63f
3 changed files with 90 additions and 3 deletions
|
|
@ -188,6 +188,12 @@ class CredentialLiteLLMParams(BaseModel):
|
|||
aws_bedrock_runtime_endpoint: Optional[str] = None
|
||||
aws_bedrock_project_id: Optional[str] = None
|
||||
s3_bucket_name: Optional[str] = None
|
||||
# Like the fields above, must be declared here or the strict dump in
|
||||
# ``get_deployment_credentials_with_provider`` drops it from the trusted
|
||||
# snapshot, so per-model output-bucket config never reaches Bedrock
|
||||
# file-content retrieval and output-bucket file ids are wrongly rejected
|
||||
# (#26335).
|
||||
s3_output_bucket_name: Optional[str] = None
|
||||
## IBM WATSONX ##
|
||||
watsonx_region_name: Optional[str] = None
|
||||
|
||||
|
|
|
|||
|
|
@ -1308,17 +1308,30 @@ class TestBedrockFileContentTransformation:
|
|||
litellm_params=self._litellm_params(),
|
||||
)
|
||||
|
||||
def _trusted(self, **creds) -> dict:
|
||||
def _trusted(self, **deployment_litellm_params) -> dict:
|
||||
"""Build the trusted snapshot the way the proxy does: deployment
|
||||
litellm_params funneled through ``CredentialLiteLLMParams`` (the strict
|
||||
allowlist ``get_deployment_credentials_with_provider`` applies) before
|
||||
retrieval ever sees them. Injecting a raw ``MappingProxyType`` would
|
||||
bypass that filter and hide whether a bucket field actually survives
|
||||
into the snapshot in production."""
|
||||
from types import MappingProxyType
|
||||
|
||||
from litellm.types.router import CredentialLiteLLMParams
|
||||
|
||||
snapshot = CredentialLiteLLMParams(**deployment_litellm_params).model_dump(
|
||||
exclude_none=True
|
||||
)
|
||||
params = self._litellm_params()
|
||||
params["_litellm_internal_model_credentials"] = MappingProxyType(dict(creds))
|
||||
params["_litellm_internal_model_credentials"] = MappingProxyType(snapshot)
|
||||
return params
|
||||
|
||||
def test_retrieves_from_distinct_output_bucket(self, monkeypatch):
|
||||
"""Batch outputs can land in a separate s3_output_bucket_name. Retrieval
|
||||
must validate the file id against the output bucket too, not just the
|
||||
input bucket, or the very outputs the feature serves are unreachable."""
|
||||
input bucket, or the very outputs the feature serves are unreachable.
|
||||
The snapshot is built through the production credential filter, so this
|
||||
fails if s3_output_bucket_name is dropped from that allowlist."""
|
||||
from litellm.llms.bedrock.files.transformation import BedrockFilesConfig
|
||||
|
||||
monkeypatch.delenv("AWS_S3_BUCKET_NAME", raising=False)
|
||||
|
|
|
|||
|
|
@ -143,3 +143,71 @@ class TestRouterCredentialResolution:
|
|||
assert credentials is not None
|
||||
assert credentials.get("api_key") == "sk-static-key"
|
||||
assert "azure_ad_token" not in credentials
|
||||
|
||||
|
||||
class TestRouterCredentialResolutionS3OutputBucket:
|
||||
"""Same strict-dump trap as azure_ad_token (#30235), for Bedrock batch
|
||||
file retrieval (#26335). Bedrock batch outputs land in a per-model
|
||||
``s3_output_bucket_name`` when it differs from the input bucket. The
|
||||
file-content retrieval path validates a file id against the buckets in the
|
||||
trusted credential snapshot, and that snapshot is built by round-tripping
|
||||
the deployment's ``litellm_params`` through ``CredentialLiteLLMParams``. If
|
||||
the field is undeclared it is dropped, so the output bucket never reaches
|
||||
retrieval and output-bucket file ids are rejected as foreign."""
|
||||
|
||||
def test_credentials_preserve_s3_output_bucket_name(self):
|
||||
from litellm import Router
|
||||
|
||||
deployment_id = "bedrock-batch-output-bucket-fixed-uuid"
|
||||
router = Router(
|
||||
model_list=[
|
||||
{
|
||||
"model_name": "bedrock-batch",
|
||||
"litellm_params": {
|
||||
"model": "bedrock/anthropic.claude-3-sonnet-20240229-v1:0",
|
||||
"s3_bucket_name": "in-bucket",
|
||||
"s3_output_bucket_name": "out-bucket",
|
||||
"aws_region_name": "us-west-2",
|
||||
},
|
||||
"model_info": {"id": deployment_id},
|
||||
}
|
||||
]
|
||||
)
|
||||
|
||||
credentials = router.get_deployment_credentials_with_provider(
|
||||
model_id=deployment_id
|
||||
)
|
||||
assert credentials is not None
|
||||
assert credentials.get("s3_output_bucket_name") == "out-bucket", (
|
||||
"Router credential resolution dropped s3_output_bucket_name; "
|
||||
"Bedrock batch file-content retrieval will reject output-bucket "
|
||||
"file ids as foreign for model-routed deployments (#26335)"
|
||||
)
|
||||
assert credentials.get("s3_bucket_name") == "in-bucket"
|
||||
|
||||
def test_credentials_without_output_bucket_unaffected(self):
|
||||
"""A deployment that configures only the input bucket keeps it and does
|
||||
not gain a phantom output bucket in the resolved credentials."""
|
||||
from litellm import Router
|
||||
|
||||
deployment_id = "bedrock-batch-input-only-fixed-uuid"
|
||||
router = Router(
|
||||
model_list=[
|
||||
{
|
||||
"model_name": "bedrock-batch-input-only",
|
||||
"litellm_params": {
|
||||
"model": "bedrock/anthropic.claude-3-sonnet-20240229-v1:0",
|
||||
"s3_bucket_name": "in-bucket",
|
||||
"aws_region_name": "us-west-2",
|
||||
},
|
||||
"model_info": {"id": deployment_id},
|
||||
}
|
||||
]
|
||||
)
|
||||
|
||||
credentials = router.get_deployment_credentials_with_provider(
|
||||
model_id=deployment_id
|
||||
)
|
||||
assert credentials is not None
|
||||
assert credentials.get("s3_bucket_name") == "in-bucket"
|
||||
assert "s3_output_bucket_name" not in credentials
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue