From 629c0a58d21f748979db74e8e8ddf248571410ea Mon Sep 17 00:00:00 2001 From: "zhenshan.cao" Date: Wed, 1 Jul 2026 20:08:29 -0700 Subject: [PATCH] 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 Co-authored-by: Claude Fable 5 --- internal/rootcoord/root_coord.go | 17 ++++++++-- pkg/util/contextutil/context_util.go | 21 +++++++++++++ pkg/util/contextutil/context_util_test.go | 38 +++++++++++++++++++++++ 3 files changed, 74 insertions(+), 2 deletions(-) diff --git a/internal/rootcoord/root_coord.go b/internal/rootcoord/root_coord.go index a1ff216b35..bb244803df 100644 --- a/internal/rootcoord/root_coord.go +++ b/internal/rootcoord/root_coord.go @@ -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 diff --git a/pkg/util/contextutil/context_util.go b/pkg/util/contextutil/context_util.go index 96758c6b10..68448d783c 100644 --- a/pkg/util/contextutil/context_util.go +++ b/pkg/util/contextutil/context_util.go @@ -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 diff --git a/pkg/util/contextutil/context_util_test.go b/pkg/util/contextutil/context_util_test.go index 0c3a4c7f1b..41278ea114 100644 --- a/pkg/util/contextutil/context_util_test.go +++ b/pkg/util/contextutil/context_util_test.go @@ -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)