mirror of
https://github.com/milvus-io/milvus.git
synced 2026-10-12 02:19:10 +00:00
f5eef7da8a
Removes the `/expr` runtime-introspection endpoint, everything that exists only to serve it, and the forked expression library it depended on. ### Why `/expr` evaluated a caller-supplied expression against live in-process objects (`param`, `proxy`, `cache`, and the component servers) and returned the result. It was useful for reading an effective config value or unexported state without a debugger or a rebuild, but the cost is hard to justify for what it does: - It required a fork of `expr-lang/expr` whose purpose is to remove the library's sandbox. The first commit on that fork is "Support the private field and auto change the struct to the point when exec the method". Upstream deliberately refuses unexported-field access; the fork exists to allow it. - That fork sits on a personal account, is 81 commits behind upstream, and was last pushed in May 2025. `go.mod` pinned it through `replace`, making it build-critical. Every other `replace` in that block points at `milvus-io/*`. - Nothing else imports the library. `pkg/util/expr/expr.go` was the only importer, using four symbols: `expr.Compile`, `expr.Env`, `expr.WithContext` and `vm.VM.Run`. The fork's ~550 lines of product code add no API; they change the behaviour of those four calls. Swapping back to upstream would therefore still compile and still pass CI, and break only at runtime — no test in the tree exercises unexported-field access, pointer-method calls, `mapValue`, the `view` builtin, or automatic type conversion. Removing the endpoint removes the only reason the fork was there. ### What is removed - the `pkg/util/expr` package - nine `expr.Init` / `expr.Register` call sites across roles, datacoord, datanode, proxy, meta cache, querycoord, querynode and rootcoord - the `/static/` Expr Executor page (`internal/http/static/index.html`, whose only function was to call `/expr`), `static_view.go`, `StaticPath` and its route registration - the `/expr` handler in `internal/http/server.go`, `ExprPath` in `router.go`, and `CheckExprAuth` / `checkExprRootAuth` / the two auth-mode constants in `internal/http/rbac.go` - `common.security.exprEnabled` and `common.security.exprAuthMode`, and their entries in `configs/milvus.yaml` - `github.com/expr-lang/expr` and its `replace` to `github.com/SimFG/expr`, from both `go.mod` and `pkg/go.mod` ### The Expr privilege v3.0.0 shipped `Expr` as a grantable RBAC privilege for this endpoint. The name cannot simply be dropped: grant and revoke share one validation path (`ddl_callbacks_rbac_privilege.go` dispatches on request version, not on grant-vs-revoke), so an undefined name would leave grants made on v3.0.0 both unusable and unrevocable, clearable only by editing etcd by hand. The second commit rejects the grant explicitly instead, with a message that says why, and leaves every other path untouched. The name stays recognized and inert so existing grants remain listable and revocable, and can be dropped entirely once no supported release still has the endpoint. ### What is deliberately kept `CheckPrivilege`, `RegisterGetUserRoleFunc`, `RegisterPasswordVerifyFunc` and `parseHTTPAuth` in `internal/http/rbac.go`. These are general HTTP RBAC helpers rather than expr-specific, and #52580 builds on that surface. After this change only tests call `CheckPrivilege`; that is intentional, not an oversight. ### Verification Unit tests pass for `internal/http`, `internal/rootcoord` and `pkg/util`, including `TestExprPrivilegeDefinition` and a new `TestIsDeprecatedExprPrivilege` that covers both the API and metastore spellings plus negative cases. Behaviour was also checked against a running instance rather than inferred: ``` route removal /expr 404 "404 page not found" /nonexistent-route-xyz 404 "404 page not found" identical response /healthz, /metrics 200 server is healthy /expr on a build that still registers it 500 from expr.Exec privilege guard grant Expr (v2) rejected: privilege "Expr" is deprecated: the /expr grant Expr (v1) rejected endpoint it granted access to has been removed revoke Expr (v2) accepted revoke Expr (v1) accepted grant/revoke Load accepted (control) a pre-existing v3.0.0 grant wrote grantee-id/<id>/PrivilegeExpr directly into etcd, restarted the cluster describe_role shows Global/Expr alongside Global/CreateDatabase revoke Expr succeeds describe_role only CreateDatabase remains etcd the PrivilegeExpr key is gone, no orphan left behind ``` ### Scope note This does not close the unauthenticated `:9091` surface. `/metrics` and `/debug/pprof/` remain unauthenticated; that is #52580's subject. This change removes one route from that port and the supply-chain exposure behind it. --------- Signed-off-by: zhenshan.cao <[email protected]> Signed-off-by: Zack <[email protected]> Co-authored-by: Zack <[email protected]> Co-authored-by: Claude Fable 5 <[email protected]>