Repository: milvus-io/milvus-sdk-go
Pull request #784 (2024-07-08 11:47:30)

fix broken Grant() call
The latest version of the SDK doesn't pass the actual privilege that is granted during a Grant() call. This causes the Grant call to fail in any situation. 

This PR fixes this issue by extending the method signature to take a `privilege` parameter, similarly to how the Python SDK does. This was an easy fix because the protobuf structs already had the field in place, so the SDK only needs to use that correct field in the API call.

sre-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/milvus-io/milvus-sdk-go/pull/784#" title="Author self-approved">nustiueudinastea</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **congqixia** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @congqixia` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=milvus-io%2Fmilvus-sdk-go).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/milvus-io/milvus-sdk-go/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["congqixia"]} -->
sre-ci-robot: Welcome @nustiueudinastea! It looks like this is your first PR to milvus-io/milvus-sdk-go 🎉
nustiueudinastea: /assign @congqixia
congqixia: @Mergifyio rebase
mergify[bot]: > rebase

#### ✅ Branch has been successfully rebased



<!---
DO NOT EDIT
-*- Mergify Payload -*-
{"command": "rebase", "conclusion": "success", "action_is_running": true}
-*- Mergify Payload End -*-
-->
sre-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/milvus-io/milvus-sdk-go/pull/784#" title="Author self-approved">nustiueudinastea</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please ask for approval from **congqixia** after the PR has been reviewed.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=milvus-io%2Fmilvus-sdk-go).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/milvus-io/milvus-sdk-go/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["congqixia"]} -->
congqixia: /kind bug
congqixia: /lgtm
/approve
#798 might have some conflict with this PR
@SimFG please fix after this patch merged
sre-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/milvus-io/milvus-sdk-go/pull/784#issuecomment-2264373422" title="Approved">congqixia</a>*, *<a href="https://github.com/milvus-io/milvus-sdk-go/pull/784#" title="Author self-approved">nustiueudinastea</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=milvus-io%2Fmilvus-sdk-go).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/milvus-io/milvus-sdk-go/blob/master/OWNERS)~~ [congqixia]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/client/client.go b/client/client.go
index ec3765d..1728926 100644
--- a/client/client.go
+++ b/client/client.go
@@ -202,7 +202,7 @@ type Client interface {
 	// ListGrants lists all assigned privileges and objects for the role.
 	ListGrants(ctx context.Context, role string, dbName string) ([]entity.RoleGrants, error)
 	// Grant adds privilege for role.
-	Grant(ctx context.Context, role string, objectType entity.PriviledgeObjectType, object string) error
+	Grant(ctx context.Context, role string, objectType entity.PriviledgeObjectType, object string, privilege string) error
 	// Revoke removes privilege from role.
 	Revoke(ctx context.Context, role string, objectType entity.PriviledgeObjectType, object string) error
 
diff --git a/client/rbac.go b/client/rbac.go
index 2e96bed..a94bfc0 100644
--- a/client/rbac.go
+++ b/client/rbac.go
@@ -320,7 +320,7 @@ func (c *GrpcClient) ListGrant(ctx context.Context, role string, object string,
 }
 
 // Grant adds object privileged for role.
-func (c *GrpcClient) Grant(ctx context.Context, role string, objectType entity.PriviledgeObjectType, object string) error {
+func (c *GrpcClient) Grant(ctx context.Context, role string, objectType entity.PriviledgeObjectType, object string, privilege string) error {
 	if c.Service == nil {
 		return ErrClientNotReady
 	}
@@ -333,6 +333,11 @@ func (c *GrpcClient) Grant(ctx context.Context, role string, objectType entity.P
 			Object: &milvuspb.ObjectEntity{
 				Name: commonpb.ObjectType_name[int32(objectType)],
 			},
+			Grantor: &milvuspb.GrantorEntity{
+				Privilege: &milvuspb.PrivilegeEntity{
+					Name: privilege,
+				},
+			},
 			ObjectName: object,
 		},
 		Type: milvuspb.OperatePrivilegeType_Grant,


Test patch:
diff --git a/client/rbac_test.go b/client/rbac_test.go
index 51f230d..2e6673d 100644
--- a/client/rbac_test.go
+++ b/client/rbac_test.go
@@ -629,6 +629,7 @@ func (s *RBACSuite) TestGrant() {
 	roleName := "testRole"
 	objectName := testCollectionName
 	objectType := entity.PriviledegeObjectTypeCollection
+	privilegeName := "testPrivilege"
 
 	s.Run("normal run", func() {
 		ctx, cancel := context.WithCancel(ctx)
@@ -641,7 +642,7 @@ func (s *RBACSuite) TestGrant() {
 			s.Equal(milvuspb.OperatePrivilegeType_Grant, req.GetType())
 		}).Return(&commonpb.Status{ErrorCode: commonpb.ErrorCode_Success}, nil)
 
-		err := s.client.Grant(ctx, roleName, objectType, objectName)
+		err := s.client.Grant(ctx, roleName, objectType, objectName, privilegeName)
 
 		s.NoError(err)
 	})
@@ -652,7 +653,7 @@ func (s *RBACSuite) TestGrant() {
 		defer s.resetMock()
 		s.mock.EXPECT().OperatePrivilege(mock.Anything, mock.Anything).Return(nil, errors.New("mock error"))
 
-		err := s.client.Grant(ctx, roleName, objectType, objectName)
+		err := s.client.Grant(ctx, roleName, objectType, objectName, privilegeName)
 		s.Error(err)
 	})
 
@@ -662,7 +663,7 @@ func (s *RBACSuite) TestGrant() {
 		defer s.resetMock()
 		s.mock.EXPECT().OperatePrivilege(mock.Anything, mock.Anything).Return(&commonpb.Status{ErrorCode: commonpb.ErrorCode_UnexpectedError}, nil)
 
-		err := s.client.Grant(ctx, roleName, objectType, objectName)
+		err := s.client.Grant(ctx, roleName, objectType, objectName, privilegeName)
 		s.Error(err)
 	})
 
@@ -671,7 +672,7 @@ func (s *RBACSuite) TestGrant() {
 		defer cancel()
 
 		c := &GrpcClient{}
-		err := c.Grant(ctx, roleName, objectType, objectName)
+		err := c.Grant(ctx, roleName, objectType, objectName, privilegeName)
 		s.Error(err)
 		s.ErrorIs(err, ErrClientNotReady)
 	})