mirror of
https://github.com/milvus-io/milvus.git
synced 2026-07-21 02:05:41 +00:00
fix: log intra-cluster no-identity requests at debug level in rootcoord (#50987)
## What
`getCurrentUserVisible{Databases,Collections}` in rootcoord logged an
expected situation as a misleading warning. With authorization enabled,
every intra-cluster ListDatabases/ShowCollections call (datacoord /
querycoord brokers, in-process calls inside mixcoord) carries no user
identity, hits the fallback branch, and emitted:
```
[WARN] ["get current user from context failed"]
[error="fail to get authorization from the md, authorization:[token]: invalid parameter"]
```
The wording suggests a request input error, but there is no failed user
request at all: the RPC deliberately falls back to full visibility and
succeeds (see the RCA in #50974).
## How
- `contextutil.IsIntraClusterRequest(ctx)`: classifies the request by
the shape of the incoming gRPC metadata (see table below). The
authorization key always wins: a request carrying user identity is never
treated as intra-cluster.
- rootcoord logs the intra-cluster case at debug level with an explicit
"fall back to full visibility" message; any other identity-resolution
failure keeps the warning.
| incoming metadata | classified as | log |
|---|---|---|
| carries `authorization` | user request | (unchanged RBAC path) |
| no metadata at all | in-process intra-cluster call | debug |
| `ServerID`/`Cluster` keys, no `authorization` | intra-cluster gRPC |
debug |
| metadata present, none of the keys | unknown source | warn (kept) |
No behavior change: the AnyWord visibility fallback, the error
construction in contextutil (shared with user-facing paths and asserted
on by e2e), and all merr codes are untouched.
## Verification
Live-verified on a 2.6 build of this patch (standalone,
authorizationEnabled=true):
- startup intra-cluster calls (no-metadata variant): debug line emitted,
zero occurrences of the old warning in the whole log;
- gRPC probe to rootcoord with Cluster metadata and no authorization
(the variant seen in the 2.6-nightly CI logs): debug;
- gRPC probe with unrelated metadata only: warning kept;
- authenticated REST request: succeeds, no new log lines;
- unauthenticated external request: still rejected at the proxy edge and
never reaches rootcoord.
On master: package compile + unit tests (`pkg/util/contextutil` incl.
new `TestIsIntraClusterRequest`, `internal/rootcoord` visibility task
tests).
issue: #50974
Signed-off-by: zhenshan.cao <zhenshan.cao@zilliz.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
3fce3171ef
commit
629c0a58d2
@@ -3255,6 +3255,19 @@ func (c *Core) getDefaultAndCustomPrivilegeGroups(ctx context.Context) ([]*milvu
|
||||
return allGroups, nil
|
||||
}
|
||||
|
||||
// logGetCurUserFailed logs a failure to resolve the current user from ctx.
|
||||
// Intra-cluster calls (e.g. datacoord/querycoord invoking ListDatabases or
|
||||
// ShowCollections) carry no user identity by design and deliberately fall back
|
||||
// to full visibility, so they are logged at debug level; a missing identity on
|
||||
// any other request is genuinely anomalous and keeps the warning.
|
||||
func logGetCurUserFailed(ctx context.Context, err error) {
|
||||
if contextutil.IsIntraClusterRequest(ctx) {
|
||||
mlog.Debug(ctx, "intra-cluster request without user identity, fall back to full visibility", mlog.Err(err))
|
||||
return
|
||||
}
|
||||
mlog.Warn(ctx, "get current user from context failed", mlog.Err(err))
|
||||
}
|
||||
|
||||
func (c *Core) getCurrentUserVisibleDatabases(ctx context.Context) (typeutil.Set[string], error) {
|
||||
enableAuth := Params.CommonCfg.AuthorizationEnabled.GetAsBool()
|
||||
privilegeDatabases := typeutil.NewSet[string]()
|
||||
@@ -3266,7 +3279,7 @@ func (c *Core) getCurrentUserVisibleDatabases(ctx context.Context) (typeutil.Set
|
||||
// it will fail if the inner node server use the list database API
|
||||
if err != nil || (curUser == util.UserRoot && !Params.CommonCfg.RootShouldBindRole.GetAsBool()) {
|
||||
if err != nil {
|
||||
mlog.Warn(ctx, "get current user from context failed", mlog.Err(err))
|
||||
logGetCurUserFailed(ctx, err)
|
||||
}
|
||||
privilegeDatabases.Insert(util.AnyWord)
|
||||
return privilegeDatabases, nil
|
||||
@@ -3332,7 +3345,7 @@ func (c *Core) getCurrentUserVisibleCollections(ctx context.Context, databaseNam
|
||||
curUser, err := contextutil.GetCurUserFromContext(ctx)
|
||||
if err != nil || (curUser == util.UserRoot && !Params.CommonCfg.RootShouldBindRole.GetAsBool()) {
|
||||
if err != nil {
|
||||
mlog.Warn(ctx, "get current user from context failed", mlog.Err(err))
|
||||
logGetCurUserFailed(ctx, err)
|
||||
}
|
||||
privilegeColls.Insert(util.AnyWord)
|
||||
return privilegeColls, nil
|
||||
|
||||
@@ -27,6 +27,7 @@ import (
|
||||
"github.com/milvus-io/milvus/pkg/v3/metrics"
|
||||
"github.com/milvus-io/milvus/pkg/v3/util"
|
||||
"github.com/milvus-io/milvus/pkg/v3/util/crypto"
|
||||
"github.com/milvus-io/milvus/pkg/v3/util/interceptor"
|
||||
"github.com/milvus-io/milvus/pkg/v3/util/merr"
|
||||
)
|
||||
|
||||
@@ -83,6 +84,26 @@ func SetToIncomingContext(ctx context.Context, kv ...string) context.Context {
|
||||
return metadata.NewIncomingContext(ctx, md)
|
||||
}
|
||||
|
||||
// IsIntraClusterRequest reports whether the request comes from another Milvus
|
||||
// component rather than a user request forwarded by proxy, judged by the shape
|
||||
// of the incoming metadata:
|
||||
// - no incoming metadata at all: an in-process call inside the same process;
|
||||
// - metadata carries the ServerID/Cluster keys (injected by the grpcclient
|
||||
// interceptors on every intra-cluster RPC) but no authorization key.
|
||||
//
|
||||
// A user request forwarded by proxy always carries the authorization key
|
||||
// (appended by proxy after authentication), so it never matches.
|
||||
func IsIntraClusterRequest(ctx context.Context) bool {
|
||||
md, ok := metadata.FromIncomingContext(ctx)
|
||||
if !ok {
|
||||
return true
|
||||
}
|
||||
if len(md.Get(util.HeaderAuthorize)) > 0 {
|
||||
return false
|
||||
}
|
||||
return len(md.Get(interceptor.ServerIDKey)) > 0 || len(md.Get(interceptor.ClusterKey)) > 0
|
||||
}
|
||||
|
||||
func GetCurUserFromContext(ctx context.Context) (string, error) {
|
||||
username, _, err := GetAuthInfoFromContext(ctx)
|
||||
return username, err
|
||||
|
||||
@@ -29,6 +29,7 @@ import (
|
||||
|
||||
"github.com/milvus-io/milvus/pkg/v3/util"
|
||||
"github.com/milvus-io/milvus/pkg/v3/util/crypto"
|
||||
"github.com/milvus-io/milvus/pkg/v3/util/interceptor"
|
||||
)
|
||||
|
||||
func TestAppendToIncomingContext(t *testing.T) {
|
||||
@@ -94,6 +95,43 @@ func TestGetCurUserFromContext(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsIntraClusterRequest(t *testing.T) {
|
||||
t.Run("no incoming metadata means in-process call", func(t *testing.T) {
|
||||
assert.True(t, IsIntraClusterRequest(context.Background()))
|
||||
})
|
||||
|
||||
t.Run("serverID without authorization", func(t *testing.T) {
|
||||
ctx := metadata.NewIncomingContext(context.Background(),
|
||||
metadata.New(map[string]string{interceptor.ServerIDKey: "1"}))
|
||||
assert.True(t, IsIntraClusterRequest(ctx))
|
||||
})
|
||||
|
||||
t.Run("cluster key without authorization", func(t *testing.T) {
|
||||
ctx := metadata.NewIncomingContext(context.Background(),
|
||||
metadata.New(map[string]string{interceptor.ClusterKey: "by-dev"}))
|
||||
assert.True(t, IsIntraClusterRequest(ctx))
|
||||
})
|
||||
|
||||
t.Run("authorization always wins", func(t *testing.T) {
|
||||
ctx := metadata.NewIncomingContext(context.Background(), metadata.New(map[string]string{
|
||||
interceptor.ServerIDKey: "1",
|
||||
util.HeaderAuthorize: crypto.Base64Encode("root:root"),
|
||||
}))
|
||||
assert.False(t, IsIntraClusterRequest(ctx))
|
||||
})
|
||||
|
||||
t.Run("authorization only", func(t *testing.T) {
|
||||
ctx := GetContext(context.Background(), "root:root")
|
||||
assert.False(t, IsIntraClusterRequest(ctx))
|
||||
})
|
||||
|
||||
t.Run("metadata without any known key is not intra-cluster", func(t *testing.T) {
|
||||
ctx := metadata.NewIncomingContext(context.Background(),
|
||||
metadata.New(map[string]string{"foo": "bar"}))
|
||||
assert.False(t, IsIntraClusterRequest(ctx))
|
||||
})
|
||||
}
|
||||
|
||||
func GetContext(ctx context.Context, originValue string) context.Context {
|
||||
authKey := strings.ToLower(util.HeaderAuthorize)
|
||||
authValue := crypto.Base64Encode(originValue)
|
||||
|
||||
Reference in New Issue
Block a user