[Router] Scope signal evaluation to authz-eligible decisions - #2648
[Router] Scope signal evaluation to authz-eligible decisions#2648duchangyong wants to merge 7 commits into
Conversation
✅ Deploy Preview for vllm-semantic-router ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
👥 vLLM Semantic Team NotificationThe following members have been identified for the changed files in this PR and have been automatically assigned when their GitHub accounts are assignable in this repository: 📁
|
✅ Supply Chain Security Report — All Clear
Scanned at |
FAUST-BENCHOU
left a comment
There was a problem hiding this comment.
The unit tests cover the scoped evaluation paths, but this is a behavior-visible routing change and the repository requires E2E coverage for it. The manual cluster smoke demonstrates the behavior once, but it won’t protect against future regressions. Could we extend the existing authz-rbac profile to verify that routing stays unchanged and a signal used only by an ineligible decision is not evaluated?
|
Hi @rootfs @Xunzhuo @FAUST-BENCHOU, the requested authz-rbac E2E coverage has been added: it verifies that an authz-ineligible decision does not evaluate its signal while fallback routing remains unchanged. All PR checks are now green (29 successful). Could you please take another look when you have a moment? Thank you! |
8d6bdcb to
be1971e
Compare
Signed-off-by: 0027020836 <du.changyong2@iwhalecloud.com>
Signed-off-by: 0027020836 <du.changyong2@iwhalecloud.com>
Signed-off-by: 0027020836 <du.changyong2@iwhalecloud.com>
Signed-off-by: 0027020836 <du.changyong2@iwhalecloud.com>
be1971e to
fcad011
Compare
| skipCompressionSignals, | ||
| convFacts, | ||
| imageURL, | ||
| c.Config.Decisions, |
There was a problem hiding this comment.
[P2] Preserve all recipe decisions for the compatibility wrapper. EvaluateAllSignalsWithContext has no request scope, and before this change getUsedSignals used Config.AllRoutingDecisions(). Passing only Config.Decisions here drops signals referenced exclusively by non-default recipes for callers such as ClassificationService.ClassifyIntent. Please keep AllRoutingDecisions() in unscoped wrappers and reserve the narrower list for the explicit request-scoped APIs.
There was a problem hiding this comment.
Thanks, addressed in e9ebd9b: EvaluateAllSignalsWithContext now uses Config.AllRoutingDecisions() again, while the explicit request-scoped APIs keep the narrower decision list. I also added regression coverage for a signal referenced only by a non-default recipe.
Validation:
- go test ./pkg/classification
- go test ./e2e/testcases
- full local Podman/Kind authz-rbac profile: 4/4 passed, including authz-signal-scope (admin_only_marker metric delta 0 and fallback model Qwen/Qwen2.5-7B-Instruct)
- all GitHub CI checks on 3ad3673 completed successfully, including both required checks and the Kubernetes integration test; conditional jobs were skipped as expected
Signed-off-by: 0027020836 <du.changyong2@iwhalecloud.com>
Signed-off-by: 0027020836 <du.changyong2@iwhalecloud.com>
Signed-off-by: 0027020836 <du.changyong2@iwhalecloud.com>
Closes #2647
Purpose
forceEvaluateAllis enabled and preserve the default-model fallback when authz removes every auto-routing candidate.Router.Test Plan
go test ./pkg/classification ./pkg/extproc -count=1make test-semantic-routermake agent-ci-gate ENV=cpu AGENT_BASE_REF=origin/main AGENT_SKIP_PRECOMMIT_BASELINE=1 ...git diff --check/app/router-candleSHAmaas-route-9120authz,jailbreak, andpiiclassifiersThe unit tests cover conservative AND/OR/NOT partial evaluation, malformed-rule retention, case-sensitive authz role matching, decision-scoped signal discovery, skipped irrelevant classifier work, force-all compatibility, extproc candidate propagation, and empty-candidate auto-model fallback.
Test Result
go test ./pkg/classification ./pkg/extproc -count=1: passed.make test-semantic-router: passed.git diff --check: passed.localhost/semantic-router-extproc:issue-2647-10826745(image IDsha256:1875b27085bc4e4565f5924aef48f707daceeb49b9a36a87d82a66fade9fc954) and verified/app/router-candleSHA7c5c8e1e8e375bd055d34963c2252e2d89c476ceb7b65098f3d75313e5d6f0fc.stream: true) for authz groupmaas-route-9120selected routemaas-route-9120, scenegeneral, and modelmaas-model-rel-91201.authz,jailbreak, andpiisignal extraction; unrelated configured signals such as keyword, embedding, domain, context, complexity, modality, and fact-check were not evaluated.kind-cluster, Deployment, and mounted model PVC without downloading models. Its cleanup restored the originallocalhost/semantic-router-extproc:route-scope-20260723image and Ready replica.Semantic Router PR Checklist
[Router],[CLI],[Dashboard],[Operator],[Fleet-Sim],[Bindings],[Training],[E2E],[Docs], or[CI/Build]git commit -s