mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-09 22:31:41 +00:00
fix(proxy): thread config_file_path into pass-through and MCP-tool YAML loaders
The previous commit's gate broke two legitimate startup paths for operators using s3://gcs:// remote module loading from their config.yaml: - general_settings.pass_through_endpoints[].custom_handler - mcp_tools[].handler Both call sites called get_instance_fn without a config_file_path, so the new gate rejected them at startup. Thread config_file_path through: - create_pass_through_route accepts config_file_path and forwards it to get_instance_fn. add_exact_path_route, add_subpath_route, _register_pass_through_endpoint, and initialize_pass_through_endpoints accept and propagate it. - The YAML-load call site in proxy_server.load_config now passes config_file_path; the DB-overlay call site in _update_general_settings leaves it as the default None so the gate still fires on admin-written s3:// values. - MCPToolRegistry.load_tools_from_config accepts config_file_path and threads it into get_instance_fn; _init_non_llm_configs forwards it from load_config. Adds two regression tests verifying that the YAML-source callers thread the path through to get_instance_fn.
This commit is contained in:
parent
08f3c1aae8
commit
14a3083d45
4 changed files with 90 additions and 11 deletions
|
|
@ -92,13 +92,20 @@ class MCPToolRegistry:
|
|||
]
|
||||
|
||||
def load_tools_from_config(
|
||||
self, mcp_tools_config: Optional[Dict[str, Any]] = None
|
||||
self,
|
||||
mcp_tools_config: Optional[Dict[str, Any]] = None,
|
||||
config_file_path: Optional[str] = None,
|
||||
) -> None:
|
||||
"""
|
||||
Load and register tools from the proxy config
|
||||
|
||||
Args:
|
||||
mcp_tools_config: The mcp_tools config from the proxy config
|
||||
config_file_path: Path to the operator's config.yaml. Threaded
|
||||
through to ``get_instance_fn`` so an ``s3://``/``gcs://``
|
||||
``handler`` declared in the YAML resolves; callers from a
|
||||
non-YAML path must leave this ``None`` so the runtime gate
|
||||
fires.
|
||||
"""
|
||||
if mcp_tools_config is None:
|
||||
raise ValueError(
|
||||
|
|
@ -121,7 +128,7 @@ class MCPToolRegistry:
|
|||
# First check if it's a module path (e.g., "module.submodule.function")
|
||||
if handler_name is None:
|
||||
raise ValueError(f"handler is required for tool {name}")
|
||||
handler = get_instance_fn(handler_name)
|
||||
handler = get_instance_fn(handler_name, config_file_path)
|
||||
|
||||
if handler is None:
|
||||
verbose_logger.warning(
|
||||
|
|
|
|||
|
|
@ -1236,6 +1236,7 @@ def create_pass_through_route(
|
|||
query_params: Optional[dict] = None,
|
||||
default_query_params: Optional[dict] = None,
|
||||
guardrails: Optional[Dict[str, Any]] = None,
|
||||
config_file_path: Optional[str] = None,
|
||||
):
|
||||
# check if target is an adapter.py or a url
|
||||
from litellm._uuid import uuid
|
||||
|
|
@ -1245,7 +1246,7 @@ def create_pass_through_route(
|
|||
if isinstance(target, CustomLogger):
|
||||
adapter = target
|
||||
else:
|
||||
adapter = get_instance_fn(value=target)
|
||||
adapter = get_instance_fn(value=target, config_file_path=config_file_path)
|
||||
adapter_id = str(uuid.uuid4())
|
||||
litellm.adapters = [{"id": adapter_id, "adapter": adapter}]
|
||||
|
||||
|
|
@ -2019,6 +2020,7 @@ class InitPassThroughEndpointHelpers:
|
|||
guardrails: Optional[dict] = None,
|
||||
methods: Optional[List[str]] = None,
|
||||
default_query_params: Optional[dict] = None,
|
||||
config_file_path: Optional[str] = None,
|
||||
):
|
||||
"""Add exact path route for pass-through endpoint"""
|
||||
# Default to all methods if none specified (backward compatibility)
|
||||
|
|
@ -2058,6 +2060,7 @@ class InitPassThroughEndpointHelpers:
|
|||
cost_per_request=cost_per_request,
|
||||
default_query_params=default_query_params,
|
||||
guardrails=guardrails,
|
||||
config_file_path=config_file_path,
|
||||
),
|
||||
methods=methods,
|
||||
dependencies=dependencies,
|
||||
|
|
@ -2095,6 +2098,7 @@ class InitPassThroughEndpointHelpers:
|
|||
guardrails: Optional[dict] = None,
|
||||
methods: Optional[List[str]] = None,
|
||||
default_query_params: Optional[dict] = None,
|
||||
config_file_path: Optional[str] = None,
|
||||
):
|
||||
"""Add wildcard route for sub-paths"""
|
||||
# Default to all methods if none specified (backward compatibility)
|
||||
|
|
@ -2135,6 +2139,7 @@ class InitPassThroughEndpointHelpers:
|
|||
cost_per_request=cost_per_request,
|
||||
default_query_params=default_query_params,
|
||||
guardrails=guardrails,
|
||||
config_file_path=config_file_path,
|
||||
),
|
||||
methods=methods,
|
||||
dependencies=dependencies,
|
||||
|
|
@ -2298,6 +2303,7 @@ async def _register_pass_through_endpoint(
|
|||
app: FastAPI,
|
||||
premium_user: bool,
|
||||
visited_endpoints: set[str],
|
||||
config_file_path: Optional[str] = None,
|
||||
) -> None:
|
||||
endpoint_data: Dict[str, Any]
|
||||
if isinstance(endpoint, PassThroughGenericEndpoint):
|
||||
|
|
@ -2324,10 +2330,10 @@ async def _register_pass_through_endpoint(
|
|||
dependencies = None
|
||||
|
||||
if auth is not None and str(auth).lower() == "true":
|
||||
# Authentication on a pass-through endpoint used to be enterprise-only.
|
||||
# That left OSS with no safe configuration: auth=True raised at startup
|
||||
# unless the operator had a license. The safe option must always be free,
|
||||
# and unauthenticated forwarding should require explicit opt-in.
|
||||
# Authentication on a pass-through endpoint used to be enterprise-only.
|
||||
# That left OSS with no safe configuration: auth=True raised at startup
|
||||
# unless the operator had a license. The safe option must always be free,
|
||||
# and unauthenticated forwarding should require explicit opt-in.
|
||||
dependencies = [Depends(user_api_key_auth)]
|
||||
if path not in LiteLLMRoutes.openai_routes.value:
|
||||
LiteLLMRoutes.openai_routes.value.append(path)
|
||||
|
|
@ -2355,6 +2361,7 @@ async def _register_pass_through_endpoint(
|
|||
guardrails=guardrails,
|
||||
methods=methods,
|
||||
default_query_params=default_query_params,
|
||||
config_file_path=config_file_path,
|
||||
)
|
||||
|
||||
methods_for_key = methods if methods else ["GET", "POST", "PUT", "DELETE", "PATCH"]
|
||||
|
|
@ -2379,6 +2386,7 @@ async def _register_pass_through_endpoint(
|
|||
guardrails=guardrails,
|
||||
methods=methods,
|
||||
default_query_params=default_query_params,
|
||||
config_file_path=config_file_path,
|
||||
)
|
||||
visited_endpoints.add(f"{endpoint_id}:subpath:{path}:{methods_str}")
|
||||
|
||||
|
|
@ -2389,6 +2397,7 @@ async def _register_pass_through_endpoint(
|
|||
|
||||
async def initialize_pass_through_endpoints(
|
||||
pass_through_endpoints: Union[List[Dict], List[PassThroughGenericEndpoint]],
|
||||
config_file_path: Optional[str] = None,
|
||||
):
|
||||
"""
|
||||
1. Create a global list of pass-through endpoints (db + config)
|
||||
|
|
@ -2399,6 +2408,12 @@ async def initialize_pass_through_endpoints(
|
|||
|
||||
Args:
|
||||
pass_through_endpoints: List of pass-through endpoints to initialize
|
||||
config_file_path: Path to the operator's config.yaml when this call
|
||||
originates from a YAML-load. Threaded through to
|
||||
``create_pass_through_route`` so an operator using
|
||||
``s3://``/``gcs://`` ``custom_handler`` in their config still
|
||||
loads. Callers from the DB-overlay / runtime API path must leave
|
||||
this ``None`` so the runtime gate in ``get_instance_fn`` fires.
|
||||
|
||||
Returns:
|
||||
None
|
||||
|
|
@ -2438,6 +2453,7 @@ async def initialize_pass_through_endpoints(
|
|||
app=app,
|
||||
premium_user=premium_user,
|
||||
visited_endpoints=visited_endpoints,
|
||||
config_file_path=config_file_path,
|
||||
)
|
||||
|
||||
# remove the ones that are not visited from the list
|
||||
|
|
|
|||
|
|
@ -4095,7 +4095,8 @@ class ProxyConfig:
|
|||
"pass_through_endpoints"
|
||||
]
|
||||
await initialize_pass_through_endpoints(
|
||||
pass_through_endpoints=general_settings["pass_through_endpoints"]
|
||||
pass_through_endpoints=general_settings["pass_through_endpoints"],
|
||||
config_file_path=config_file_path,
|
||||
)
|
||||
|
||||
## ADMIN UI ACCESS ##
|
||||
|
|
@ -4316,11 +4317,15 @@ class ProxyConfig:
|
|||
litellm.credential_list = credential_list_dict
|
||||
|
||||
## NON-LLM CONFIGS eg. MCP tools, vector stores, etc.
|
||||
await self._init_non_llm_configs(config=config)
|
||||
await self._init_non_llm_configs(
|
||||
config=config, config_file_path=config_file_path
|
||||
)
|
||||
|
||||
return router, router.get_model_list(), general_settings
|
||||
|
||||
async def _init_non_llm_configs(self, config: dict):
|
||||
async def _init_non_llm_configs(
|
||||
self, config: dict, config_file_path: Optional[str] = None
|
||||
):
|
||||
"""
|
||||
Initialize non-LLM configs eg. MCP tools, vector stores, etc.
|
||||
"""
|
||||
|
|
@ -4331,7 +4336,9 @@ class ProxyConfig:
|
|||
global_mcp_tool_registry,
|
||||
)
|
||||
|
||||
global_mcp_tool_registry.load_tools_from_config(mcp_tools_config)
|
||||
global_mcp_tool_registry.load_tools_from_config(
|
||||
mcp_tools_config, config_file_path=config_file_path
|
||||
)
|
||||
|
||||
## AGENTS
|
||||
agent_config = config.get("agent_list", None)
|
||||
|
|
|
|||
|
|
@ -62,3 +62,52 @@ def test_dotted_module_path_is_unaffected_by_gate():
|
|||
result = get_instance_fn(value="my_module.my_instance")
|
||||
|
||||
assert result == "loaded"
|
||||
|
||||
|
||||
def test_pass_through_route_threads_config_file_path():
|
||||
# ``create_pass_through_route`` must forward ``config_file_path`` so
|
||||
# an operator with ``custom_handler: s3://...`` declared in
|
||||
# ``config.yaml`` still resolves at startup. Callers that omit it
|
||||
# (DB-overlay / runtime admin API) fall through to the gate.
|
||||
from litellm.proxy.pass_through_endpoints import pass_through_endpoints as pte
|
||||
|
||||
# ``get_instance_fn`` is imported lazily inside the function — patch
|
||||
# at the source so the deferred import resolves to the mock.
|
||||
with patch(
|
||||
"litellm.proxy.types_utils.utils.get_instance_fn", return_value=object()
|
||||
) as mock_get:
|
||||
pte.create_pass_through_route(
|
||||
endpoint="/x",
|
||||
target="s3://bucket/mod.inst",
|
||||
config_file_path="/etc/litellm/config.yaml",
|
||||
)
|
||||
|
||||
mock_get.assert_called_once_with(
|
||||
value="s3://bucket/mod.inst",
|
||||
config_file_path="/etc/litellm/config.yaml",
|
||||
)
|
||||
|
||||
|
||||
def test_mcp_tool_registry_threads_config_file_path():
|
||||
# MCP tool handlers declared in ``config.yaml`` mcp_tools[].handler
|
||||
# may legitimately be ``s3://...``; the YAML-load path must thread
|
||||
# ``config_file_path`` so they resolve.
|
||||
from litellm.proxy._experimental.mcp_server import tool_registry as tr
|
||||
|
||||
fake_handler = lambda **kwargs: None # noqa: E731 — registry requires callable
|
||||
with patch.object(tr, "get_instance_fn", return_value=fake_handler) as mock_get:
|
||||
registry = tr.MCPToolRegistry()
|
||||
registry.load_tools_from_config(
|
||||
mcp_tools_config=[
|
||||
{
|
||||
"name": "tool_a",
|
||||
"description": "d",
|
||||
"handler": "s3://bucket/mod.handler",
|
||||
}
|
||||
],
|
||||
config_file_path="/etc/litellm/config.yaml",
|
||||
)
|
||||
|
||||
mock_get.assert_called_once_with(
|
||||
"s3://bucket/mod.handler", "/etc/litellm/config.yaml"
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue