Conversation
| return str(value).strip() | ||
|
|
||
|
|
||
| def is_trusted_distserve_replica_request(raw_request, engine_config) -> bool: |
There was a problem hiding this comment.
Thanks for the fix. I verified that the original reproducer from #4965 now results in a request-level error rather than terminating the engine. Health checks and subsequent generation remained successful.
is_trusted_distserve_replica_request() checks the receiving server’s role and a fixed, client-controlled header. This identifies the intended request path, but does not authenticate the caller.
If P/D replicas are internal-only services protected by network access controls, this is a reasonable deployment model, consistent with vLLM’s documented network-isolation requirements.
There was a problem hiding this comment.
Thanks for verifying. Agreed — this is meant for DistServe P/D replicas behind network isolation (same model as vLLM’s inter-node guidance), not as a substitute for authenticating arbitrary public clients. Happy to adjust further if you want a stronger internal-auth check on top.
There was a problem hiding this comment.
I think this is enough. Note that there are conflicts with the latest main. After resolving it, the pr is ready to merge.
817259a to
810824f
Compare
| @@ -0,0 +1,211 @@ | |||
| # Copyright (c) OpenMMLab. All rights reserved. | |||
| """Reject untrusted DistServe fields on the public OpenAI path.""" | |||
There was a problem hiding this comment.
This test file adds little value. I suggest removing it.
| assert context.async_engine.call_count == 0 | ||
|
|
||
|
|
||
| def test_chat_completions_still_serves_after_untrusted_migration_request( |
There was a problem hiding this comment.
This test adds little value and can be removed.
Public /v1/completions and /v1/chat/completions accepted client migration_request after Pydantic shape checks only. A bad payload could raise in the Decode migration loop and tear down EngineLoop. Reject DistServe-only fields on the public path unless the DistServe proxy marks Prefill/Decode replica traffic. Isolate per-request migration failures so they cannot cancel the engine task group. Rebased onto latest main: use Scheduler.get_block_tables after the scheduler ownership refactor. Fixes InternLM#4965 Signed-off-by: GokayAI <60583610+gokay-ai@users.noreply.github.com>
810824f to
e4bcd8a
Compare
|
Removed the two test files per @lvhan028. |
Motivation
Decode
/v1/completionsand/v1/chat/completionsaccepted clientmigration_requestafter Pydantic shape checks only. A syntactically valid but semantically bad payload can raise in the Decode migration loop.EngineLoopwaits withasyncio.FIRST_EXCEPTION, so that one request can stop the Decode engine until restart (/healthunhealthy, lasting 503s).Fixes #4965
Modification
migration_request,with_cache,preserve_cache) unless the request is Prefill/Decode replica traffic marked by the DistServe proxy.x-lmdeploy-distserve-proxyon replica POSTs so legitimate PD traffic still works.BC-breaking
External clients can no longer send DistServe-only fields on
/v1/completionsor/v1/chat/completions. DistServe through the proxy is unchanged.Checklist