From 7c714b0035d5a0b4d69acde3f6bdc4764d8ef07d Mon Sep 17 00:00:00 2001 From: aoiasd <45024769+aoiasd@users.noreply.github.com> Date: Wed, 24 Dec 2025 17:23:19 +0800 Subject: [PATCH] enhance: disallow the file resource interface before release (#46362) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit relate: https://github.com/milvus-io/milvus/issues/43687 ## Summary by CodeRabbit * **Chores** * File resource operations (add, remove, list) are now unavailable and return a not-implemented response. * **Tests** * Tests updated to expect error responses for those file resource operations and removed some previous coordination-path assertions. ✏️ Tip: You can customize this high-level summary in your review settings. Signed-off-by: aoiasd --- internal/proxy/impl.go | 79 ++++++++++++++++++---------------- internal/proxy/impl_test.go | 84 ++++++++++++++++++------------------- 2 files changed, 83 insertions(+), 80 deletions(-) diff --git a/internal/proxy/impl.go b/internal/proxy/impl.go index ff11f210a3..bf02ab9391 100644 --- a/internal/proxy/impl.go +++ b/internal/proxy/impl.go @@ -6776,19 +6776,20 @@ func (node *Proxy) AddFileResource(ctx context.Context, req *milvuspb.AddFileRes } log.Info("receive AddFileResource request") + return merr.Status(errors.New("AddFileResource is not implemented")), nil - status, err := node.mixCoord.AddFileResource(ctx, req) - if err != nil { - log.Warn("AddFileResource fail", zap.Error(err)) - return merr.Status(err), nil - } - if err = merr.Error(status); err != nil { - log.Warn("AddFileResource fail", zap.Error(err)) - return merr.Status(err), nil - } + // status, err := node.mixCoord.AddFileResource(ctx, req) + // if err != nil { + // log.Warn("AddFileResource fail", zap.Error(err)) + // return merr.Status(err), nil + // } + // if err = merr.Error(status); err != nil { + // log.Warn("AddFileResource fail", zap.Error(err)) + // return merr.Status(err), nil + // } - log.Info("AddFileResource success") - return status, nil + // log.Info("AddFileResource success") + // return status, nil } // RemoveFileResource remove file resource from rootcoord @@ -6805,19 +6806,20 @@ func (node *Proxy) RemoveFileResource(ctx context.Context, req *milvuspb.RemoveF } log.Info("receive RemoveFileResource request") + return merr.Status(errors.New("RemoveFileResource is not implemented")), nil - status, err := node.mixCoord.RemoveFileResource(ctx, req) - if err != nil { - log.Warn("RemoveFileResource fail", zap.Error(err)) - return merr.Status(err), nil - } - if err = merr.Error(status); err != nil { - log.Warn("RemoveFileResource fail", zap.Error(err)) - return merr.Status(err), nil - } + // status, err := node.mixCoord.RemoveFileResource(ctx, req) + // if err != nil { + // log.Warn("RemoveFileResource fail", zap.Error(err)) + // return merr.Status(err), nil + // } + // if err = merr.Error(status); err != nil { + // log.Warn("RemoveFileResource fail", zap.Error(err)) + // return merr.Status(err), nil + // } - log.Info("RemoveFileResource success") - return status, nil + // log.Info("RemoveFileResource success") + // return status, nil } // ListFileResources list file resources from rootcoord @@ -6834,23 +6836,26 @@ func (node *Proxy) ListFileResources(ctx context.Context, req *milvuspb.ListFile } log.Info("receive ListFileResources request") + return &milvuspb.ListFileResourcesResponse{ + Status: merr.Status(errors.New("ListFileResources is not implemented")), + }, nil - resp, err := node.mixCoord.ListFileResources(ctx, req) - if err != nil { - log.Warn("ListFileResources fail", zap.Error(err)) - return &milvuspb.ListFileResourcesResponse{ - Status: merr.Status(err), - }, nil - } - if err = merr.Error(resp.GetStatus()); err != nil { - log.Warn("ListFileResources fail", zap.Error(err)) - return &milvuspb.ListFileResourcesResponse{ - Status: merr.Status(err), - }, nil - } + // resp, err := node.mixCoord.ListFileResources(ctx, req) + // if err != nil { + // log.Warn("ListFileResources fail", zap.Error(err)) + // return &milvuspb.ListFileResourcesResponse{ + // Status: merr.Status(err), + // }, nil + // } + // if err = merr.Error(resp.GetStatus()); err != nil { + // log.Warn("ListFileResources fail", zap.Error(err)) + // return &milvuspb.ListFileResourcesResponse{ + // Status: merr.Status(err), + // }, nil + // } - log.Info("ListFileResources success", zap.Int("count", len(resp.GetResources()))) - return resp, nil + // log.Info("ListFileResources success", zap.Int("count", len(resp.GetResources()))) + // return resp, nil } // UpdateReplicateConfiguration applies a full replacement of the current replication configuration across Milvus clusters. diff --git a/internal/proxy/impl_test.go b/internal/proxy/impl_test.go index c03958af44..d53a63c9a6 100644 --- a/internal/proxy/impl_test.go +++ b/internal/proxy/impl_test.go @@ -1833,7 +1833,7 @@ func TestProxy_AddFileResource(t *testing.T) { resp, err := proxy.AddFileResource(context.Background(), req) assert.NoError(t, err) - assert.NoError(t, merr.Error(resp)) + assert.Error(t, merr.Error(resp)) }) t.Run("proxy not healthy", func(t *testing.T) { @@ -1850,23 +1850,23 @@ func TestProxy_AddFileResource(t *testing.T) { assert.Error(t, merr.Error(resp)) }) - t.Run("mixCoord error", func(t *testing.T) { - proxy := &Proxy{} - proxy.UpdateStateCode(commonpb.StateCode_Healthy) + // t.Run("mixCoord error", func(t *testing.T) { + // proxy := &Proxy{} + // proxy.UpdateStateCode(commonpb.StateCode_Healthy) - mockMixCoord := mocks.NewMockMixCoordClient(t) - mockMixCoord.EXPECT().AddFileResource(mock.Anything, mock.Anything).Return(nil, fmt.Errorf("mock error")) - proxy.mixCoord = mockMixCoord + // mockMixCoord := mocks.NewMockMixCoordClient(t) + // mockMixCoord.EXPECT().AddFileResource(mock.Anything, mock.Anything).Return(nil, fmt.Errorf("mock error")) + // proxy.mixCoord = mockMixCoord - req := &milvuspb.AddFileResourceRequest{ - Name: "test_resource", - Path: "/path/to/resource", - } + // req := &milvuspb.AddFileResourceRequest{ + // Name: "test_resource", + // Path: "/path/to/resource", + // } - resp, err := proxy.AddFileResource(context.Background(), req) - assert.NoError(t, err) - assert.Error(t, merr.Error(resp)) - }) + // resp, err := proxy.AddFileResource(context.Background(), req) + // assert.NoError(t, err) + // assert.Error(t, merr.Error(resp)) + // }) } func TestProxy_RemoveFileResource(t *testing.T) { @@ -1882,7 +1882,7 @@ func TestProxy_RemoveFileResource(t *testing.T) { resp, err := proxy.RemoveFileResource(context.Background(), req) assert.NoError(t, err) - assert.NoError(t, merr.Error(resp)) + assert.Error(t, merr.Error(resp)) }) t.Run("proxy not healthy", func(t *testing.T) { @@ -1897,22 +1897,22 @@ func TestProxy_RemoveFileResource(t *testing.T) { assert.Error(t, merr.Error(resp)) }) - t.Run("mixCoord error", func(t *testing.T) { - proxy := &Proxy{} - proxy.UpdateStateCode(commonpb.StateCode_Healthy) + // t.Run("mixCoord error", func(t *testing.T) { + // proxy := &Proxy{} + // proxy.UpdateStateCode(commonpb.StateCode_Healthy) - mockMixCoord := mocks.NewMockMixCoordClient(t) - mockMixCoord.EXPECT().RemoveFileResource(mock.Anything, mock.Anything).Return(nil, fmt.Errorf("mock error")) - proxy.mixCoord = mockMixCoord + // mockMixCoord := mocks.NewMockMixCoordClient(t) + // mockMixCoord.EXPECT().RemoveFileResource(mock.Anything, mock.Anything).Return(nil, fmt.Errorf("mock error")) + // proxy.mixCoord = mockMixCoord - req := &milvuspb.RemoveFileResourceRequest{ - Name: "test_resource", - } + // req := &milvuspb.RemoveFileResourceRequest{ + // Name: "test_resource", + // } - resp, err := proxy.RemoveFileResource(context.Background(), req) - assert.NoError(t, err) - assert.Error(t, merr.Error(resp)) - }) + // resp, err := proxy.RemoveFileResource(context.Background(), req) + // assert.NoError(t, err) + // assert.Error(t, merr.Error(resp)) + // }) } func TestProxy_ListFileResources(t *testing.T) { @@ -1927,9 +1927,7 @@ func TestProxy_ListFileResources(t *testing.T) { resp, err := proxy.ListFileResources(context.Background(), req) assert.NoError(t, err) - assert.NoError(t, merr.Error(resp.GetStatus())) - assert.NotNil(t, resp.GetResources()) - assert.Equal(t, 0, len(resp.GetResources())) // Mock returns empty list + assert.Error(t, merr.Error(resp.GetStatus())) }) t.Run("proxy not healthy", func(t *testing.T) { @@ -1942,19 +1940,19 @@ func TestProxy_ListFileResources(t *testing.T) { assert.Error(t, merr.Error(resp.GetStatus())) }) - t.Run("mixCoord error", func(t *testing.T) { - proxy := &Proxy{} - proxy.UpdateStateCode(commonpb.StateCode_Healthy) + // t.Run("mixCoord error", func(t *testing.T) { + // proxy := &Proxy{} + // proxy.UpdateStateCode(commonpb.StateCode_Healthy) - mockMixCoord := mocks.NewMockMixCoordClient(t) - mockMixCoord.EXPECT().ListFileResources(mock.Anything, mock.Anything).Return(nil, fmt.Errorf("mock error")) - proxy.mixCoord = mockMixCoord + // mockMixCoord := mocks.NewMockMixCoordClient(t) + // mockMixCoord.EXPECT().ListFileResources(mock.Anything, mock.Anything).Return(nil, fmt.Errorf("mock error")) + // proxy.mixCoord = mockMixCoord - req := &milvuspb.ListFileResourcesRequest{} - resp, err := proxy.ListFileResources(context.Background(), req) - assert.NoError(t, err) - assert.Error(t, merr.Error(resp.GetStatus())) - }) + // req := &milvuspb.ListFileResourcesRequest{} + // resp, err := proxy.ListFileResources(context.Background(), req) + // assert.NoError(t, err) + // assert.Error(t, merr.Error(resp.GetStatus())) + // }) } func TestProxy_ComputePhraseMatchSlop(t *testing.T) {