diff --git a/litellm/proxy/management_endpoints/model_management_endpoints.py b/litellm/proxy/management_endpoints/model_management_endpoints.py index 11f616a5e18..3caec435b7b 100644 --- a/litellm/proxy/management_endpoints/model_management_endpoints.py +++ b/litellm/proxy/management_endpoints/model_management_endpoints.py @@ -141,6 +141,7 @@ from litellm.router_utils.auto_router_model_naming import ( validate_strategy_router_model_write, ) from litellm.router_utils.auto_router_tuning_baseline import is_mutable_tuned_candidate, tuning_quota_violation +from litellm.router_utils.common_utils import with_admin_api_key_secret_reference, without_model_capability_flags from litellm.types.llms.base import LiteLLMBaseModel from litellm.types.llms.bedrock import AwsSessionTag from litellm.types.proxy.management_endpoints.model_management_endpoints import ( @@ -151,6 +152,7 @@ from litellm.types.router import ( SPECIAL_MODEL_INFO_PARAMS, Deployment, GenericLiteLLMParams, + LiteLLM_Params, ModelInfo, updateDeployment, updateLiteLLMParams, @@ -393,6 +395,52 @@ def _raise_on_strategy_router_write_violation( ) +_PROVIDER_ROUTING_PARAMS: Final = ("model", "custom_llm_provider", "api_base") + + +def _deployment_as_reloaded(db_model: Deployment, patch_data: updateDeployment) -> Deployment | None: + """The deployment the router reload will build from the row this patch writes, or None when it + cannot be predicted here, which leaves the write to proceed as it always has""" + from litellm.proxy.proxy_server import proxy_config + + try: + stored_params: Final = json.loads( + update_db_model(db_model=db_model, updated_patch=patch_data).get("litellm_params", "{}") + ) + reloaded_params: Final = LiteLLM_Params.model_validate( + {key: proxy_config.resolve_db_litellm_param(key=key, value=value) for key, value in stored_params.items()} + ) + return with_admin_api_key_secret_reference( + without_model_capability_flags( + Deployment( + model_name=db_model.model_name, litellm_params=reloaded_params, model_info=db_model.model_info + ) + ) + ) + except Exception: # noqa: BLE001 # an unpredictable reload keeps the pre-existing write behavior + return None + + +def _raise_on_unloadable_patch(db_model: Deployment, patch_data: updateDeployment) -> None: + """Reject a patch whose params the router reload would drop, which used to return 200 while the old + deployment kept serving and the model vanished on the next restart (#47964)""" + incoming: Final = patch_data.litellm_params + if incoming is None or all(getattr(incoming, field) is None for field in _PROVIDER_ROUTING_PARAMS): + return + reloaded: Final = _deployment_as_reloaded(db_model, patch_data) + if reloaded is None: + return + try: + Router.resolve_deployment_provider(reloaded.litellm_params) + except Exception as e: # noqa: BLE001 # the router drops the deployment on any exception from this check + raise ProxyException( + message=f"The router cannot load this deployment, so the update was not saved: {e}", + type=ProxyErrorTypes.validation_error.value, + code=status.HTTP_400_BAD_REQUEST, + param="litellm_params.model", + ) from e + + def _reject_non_admin_blocked_flag_on_create( blocked: bool | None, user_api_key_dict: UserAPIKeyAuth, @@ -1384,6 +1432,7 @@ async def patch_model( incoming_params=patch_data.litellm_params, existing_params=db_model.litellm_params, ) + _raise_on_unloadable_patch(db_model, effective_patch) effective_params: Final = _effective_complexity_router_params( patch_data.litellm_params, db_model.litellm_params diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index df948b69a67..f39db3f5e1d 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -7692,7 +7692,7 @@ class ProxyConfig: llm_router.delete_deployment(id=model_id) return frozenset(combined_id_list) | kept_config_ids - def _resolve_db_litellm_param(self, key: str, value: object) -> object: + def resolve_db_litellm_param(self, key: str, value: object) -> object: if not isinstance(value, str): return value @@ -7725,7 +7725,7 @@ class ProxyConfig: if isinstance(_litellm_params, dict): # decrypt values for k, v in _litellm_params.items(): - _litellm_params[k] = self._resolve_db_litellm_param(key=k, value=v) + _litellm_params[k] = self.resolve_db_litellm_param(key=k, value=v) _litellm_params = LiteLLM_Params.model_validate(_litellm_params) else: @@ -7756,7 +7756,7 @@ class ProxyConfig: if isinstance(_litellm_params, dict): # decrypt values for k, v in _litellm_params.items(): - _litellm_params[k] = self._resolve_db_litellm_param(key=k, value=v) + _litellm_params[k] = self.resolve_db_litellm_param(key=k, value=v) _litellm_params = LiteLLM_Params.model_validate(_litellm_params) else: verbose_proxy_logger.error( diff --git a/litellm/router.py b/litellm/router.py index 7993b5b921c..08fcf0426ae 100644 --- a/litellm/router.py +++ b/litellm/router.py @@ -11021,53 +11021,42 @@ class Router: # deployments have been registered. self._finalize_adaptive_router_if_configured() + @staticmethod + def resolve_deployment_provider( + litellm_params: LiteLLM_Params, + ) -> tuple[str, str | None, str | None, str | None]: + if "/" in litellm_params.model and ( + litellm_params.model.split("/")[0] in litellm._known_custom_logger_compatible_callbacks + ): + return litellm_params.model, None, None, None + resolved: Final = litellm.get_llm_provider( + model=litellm_params.model, + custom_llm_provider=litellm_params.get("custom_llm_provider", None), + api_base=litellm_params.api_base, + litellm_params=litellm_params, + ) + custom_llm_provider: Final = resolved[1] + if ( + custom_llm_provider not in litellm.provider_list + and not JSONProviderRegistry.exists(custom_llm_provider) + and not is_registered_custom_provider(custom_llm_provider) + ): + raise Exception(f"Unsupported provider - {custom_llm_provider}") + return resolved + def _add_deployment(self, deployment: Deployment) -> Deployment: import os #### VALIDATE MODEL ######## - # Check if this is a prompt management model before validating as LLM provider - litellm_model: Final = deployment.litellm_params.model if isinstance(deployment.litellm_params.drop_params, str): verbose_router_logger.warning( "model=%s drop_params=%r is not a flag value, treating it as unset", deployment.model_name, deployment.litellm_params.drop_params, ) - is_prompt_management_model = False - - if "/" in litellm_model: - split_litellm_model: Final = litellm_model.split("/")[0] - if split_litellm_model in litellm._known_custom_logger_compatible_callbacks: - is_prompt_management_model = True - - if is_prompt_management_model: - # For prompt management models, skip LLM provider validation - # The actual model will be resolved at runtime from the prompt file - _model = litellm_model - custom_llm_provider = None - dynamic_api_key = None - api_base = None - else: - # check if model provider in supported providers - ( - _model, - custom_llm_provider, - dynamic_api_key, - api_base, - ) = litellm.get_llm_provider( - model=deployment.litellm_params.model, - custom_llm_provider=deployment.litellm_params.get("custom_llm_provider", None), - api_base=deployment.litellm_params.api_base, - litellm_params=deployment.litellm_params, - ) - # done reading model["litellm_params"] - # Check if provider is supported: either in enum or JSON-configured - if ( - custom_llm_provider not in litellm.provider_list - and not JSONProviderRegistry.exists(custom_llm_provider) - and not is_registered_custom_provider(custom_llm_provider) - ): - raise Exception(f"Unsupported provider - {custom_llm_provider}") + _model, custom_llm_provider, dynamic_api_key, api_base = Router.resolve_deployment_provider( + deployment.litellm_params + ) #### DEPLOYMENT NAMES INIT ######## self.deployment_names.append(deployment.litellm_params.model) diff --git a/tests/unit/proxy/management_endpoints/test_model_management_endpoints.py b/tests/unit/proxy/management_endpoints/test_model_management_endpoints.py index 303b8e060ed..301c8b1229b 100644 --- a/tests/unit/proxy/management_endpoints/test_model_management_endpoints.py +++ b/tests/unit/proxy/management_endpoints/test_model_management_endpoints.py @@ -10137,3 +10137,90 @@ class TestNonAdminCannotSetPerUserOauthOnModel: user_api_key_dict=admin, ) assert result is created_row + + +class TestPatchModelRejectsParamsTheRouterDrops: + """A patch the router reload would drop used to answer 200 while the old deployment kept serving + and the model vanished on the next restart (#47964)""" + + @staticmethod + def _stored(**params: str) -> Deployment: + return Deployment( + model_name="clef", + litellm_params=LiteLLM_Params(**{key: encrypt_value_helper(value) for key, value in params.items()}), + model_info=ModelInfo(id="clef-1"), + ) + + @staticmethod + def _router_loads(params: Mapping[str, str]) -> bool: + router: Final = Router( + model_list=[{"model_name": "clef", "litellm_params": dict(params), "model_info": {"id": "clef-1"}}], + ignore_invalid_deployments=True, + ) + return router.get_deployment(model_id="clef-1") is not None + + @classmethod + def _rejection_code(cls, stored: Mapping[str, str], patched: Mapping[str, str]) -> str | None: + from litellm.proxy._types import ProxyException + from litellm.proxy.management_endpoints.model_management_endpoints import _raise_on_unloadable_patch + + try: + _raise_on_unloadable_patch( + cls._stored(**stored), updateDeployment(litellm_params=updateLiteLLMParams(**patched)) + ) + except ProxyException as e: + return e.code + return None + + @pytest.mark.parametrize( + "patched", + [ + {"model": "@cf/cloudflare/clef-flash"}, + {"model": "cloudflare/@cf/cloudflare/clef-flash"}, + {"model": "gpt-4o"}, + {"model": "not-a-provider/some-model"}, + {"model": "my-model", "custom_llm_provider": "openai"}, + {"model": "my-model", "custom_llm_provider": "not-a-provider"}, + {"model": "hosted_vllm/llama", "api_base": "http://vllm.internal/v1"}, + {"model": "langfuse/prompt-id"}, + {"model": "dotprompt/clef"}, + {"model": "azure/gpt-4o", "api_base": "os.environ/CLEF_TEST_AZURE_BASE"}, + {"api_base": "http://127.0.0.1:9911/ai/run"}, + ], + ) + def test_patch_is_rejected_exactly_when_the_router_would_drop_it( + self, monkeypatch: pytest.MonkeyPatch, patched: Mapping[str, str] + ) -> None: + from litellm.proxy._types import ProxyException + from litellm.proxy.management_endpoints.model_management_endpoints import _raise_on_unloadable_patch + + monkeypatch.setenv("LITELLM_SALT_KEY", "sk-clef-test-salt") + monkeypatch.setenv("CLEF_TEST_AZURE_BASE", "https://clef.openai.azure.com") + stored: Final = {"model": "cloudflare/clef-flash", "api_base": "https://api.cloudflare.com/client/v4/ai/run"} + router_loads: Final = self._router_loads({**stored, **patched}) + rejection_code: Final = self._rejection_code(stored, patched) + assert rejection_code == (None if router_loads else "400"), f"{router_loads=} {rejection_code=}" + + def test_reported_workers_ai_id_without_provider_prefix_is_rejected(self, monkeypatch: pytest.MonkeyPatch) -> None: + from litellm.proxy._types import ProxyException + from litellm.proxy.management_endpoints.model_management_endpoints import _raise_on_unloadable_patch + + monkeypatch.setenv("LITELLM_SALT_KEY", "sk-clef-test-salt") + with pytest.raises(ProxyException) as exc_info: + _raise_on_unloadable_patch( + self._stored(model="cloudflare/clef-flash"), + updateDeployment(litellm_params=updateLiteLLMParams(model="@cf/cloudflare/clef-flash")), + ) + assert exc_info.value.code == "400" + assert exc_info.value.param == "litellm_params.model" + assert "@cf/cloudflare/clef-flash" in exc_info.value.message + + def test_patch_not_touching_routing_params_is_never_judged(self, monkeypatch: pytest.MonkeyPatch) -> None: + from litellm.proxy.management_endpoints.model_management_endpoints import _raise_on_unloadable_patch + + monkeypatch.setenv("LITELLM_SALT_KEY", "sk-clef-test-salt") + verdict: Final = _raise_on_unloadable_patch( + self._stored(model="@cf/cloudflare/clef-flash"), + updateDeployment(litellm_params=updateLiteLLMParams(api_key="sk-rotated")), + ) + assert verdict is None diff --git a/tests/unit/proxy/proxy_server/test_proxy_config.py b/tests/unit/proxy/proxy_server/test_proxy_config.py index e59dd19271c..386cf515f88 100644 --- a/tests/unit/proxy/proxy_server/test_proxy_config.py +++ b/tests/unit/proxy/proxy_server/test_proxy_config.py @@ -3380,7 +3380,7 @@ def test_ProxyConfig__add_deployment_resolves_team_env_refs(monkeypatch): assert deployment.litellm_params.api_base == "https://team.example" -def test_ProxyConfig__resolve_db_litellm_param_skips_non_string_values(monkeypatch): +def test_ProxyConfig_resolve_db_litellm_param_skips_non_string_values(monkeypatch): def fail_on_call(value, key, return_original_value): raise AssertionError("decrypt_value_helper should only receive strings") @@ -3390,7 +3390,7 @@ def test_ProxyConfig__resolve_db_litellm_param_skips_non_string_values(monkeypat ) pc = ProxyConfig() - assert pc._resolve_db_litellm_param(key="tpm", value=100) == 100 + assert pc.resolve_db_litellm_param(key="tpm", value=100) == 100 def test_ProxyConfig__add_deployment_resolves_env_refs_for_aws_bedrock_auth_params( diff --git a/tests/unit/proxy/test_proxy_server_endpoints_and_startup.py b/tests/unit/proxy/test_proxy_server_endpoints_and_startup.py index 14196d4a1ac..04fd69d83fa 100644 --- a/tests/unit/proxy/test_proxy_server_endpoints_and_startup.py +++ b/tests/unit/proxy/test_proxy_server_endpoints_and_startup.py @@ -14908,10 +14908,10 @@ def test_resolve_db_litellm_param_keeps_wif_secret_pointers(monkeypatch): monkeypatch.setenv("WIF_TEST_KC_SECRET", "kc-secret") proxy_config = ProxyConfig() - pointer = proxy_config._resolve_db_litellm_param( + pointer = proxy_config.resolve_db_litellm_param( "anthropic_keycloak_client_secret_ref", "os.environ/WIF_TEST_KC_SECRET" ) - dereferenced = proxy_config._resolve_db_litellm_param("api_key", "os.environ/WIF_TEST_KC_SECRET") + dereferenced = proxy_config.resolve_db_litellm_param("api_key", "os.environ/WIF_TEST_KC_SECRET") assert pointer == "os.environ/WIF_TEST_KC_SECRET" assert dereferenced == "kc-secret"