[#4903] feat(core,server): Support to grant or revoke privileges for a role #5021

jerqi · 2024-09-25T11:45:48Z

What changes were proposed in this pull request?

Supports to grant or revoke privileges for a role

Why are the changes needed?

Fix: #4903

Does this PR introduce any user-facing change?

I will add the document later.

How was this patch tested?

Add the UT.

jerqi · 2024-09-25T12:24:29Z

@lw-yang Could you help me review this pull request?

jerryshao · 2024-09-27T09:53:06Z

...rc/test/java/org/apache/gravitino/client/integration/test/authorization/AccessControlIT.java

+        NoSuchRoleException.class,
+        () ->
+            metalake.revokePrivilegesFromRole(
+                "not-exist", metadataObject, Lists.newArrayList(Privileges.CreateCatalog.allow())));


Should you add the tests about binding wrong privileges to the metadata object?

You'd better add more bad cases to test the robustness of your logic.

jerryshao · 2024-09-27T10:03:24Z

common/src/main/java/org/apache/gravitino/dto/authorization/RoleDTO.java

@@ -93,6 +94,10 @@ public Map<String, String> properties() {
   */
  @Override
  public List<SecurableObject> securableObjects() {
+    if (securableObjects == null) {
+      return Collections.emptyList();
+    }


Will we return a null securableObjects here?

Legacy code. Removed.

jerryshao · 2024-09-27T10:30:57Z

core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java

+            String.format("Doesn't support the type %s", object.type()));
+    }
+  }
+


I think we can create a helper method like this to simplify the code.

public static void check(final boolean expression, Supplier<? extends RuntimeException> exceptionToThrowSupplier) { if (!expression) { throw checkNotNull(exceptionToThrowSupplier).get(); } }

yuqi1129 · 2024-09-27T10:07:10Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

+
+            List<SecurableObject> updateSecurableObjects = Lists.newArrayList();
+            // Only update the first matched securable object privilege
+            for (SecurableObject securableObject : securableObjects) {


Is it possible that there are many securable objects that equals to the object in the List?

Yes, but it's enough that we add the first matched securable object like the comment.

Why not remove duplicated items in the roleEntity?

OK, I can add the limitation for them.

yuqi1129 · 2024-09-27T10:27:17Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

+            List<SecurableObject> updateSecurableObjects = Lists.newArrayList();
+            // Only update the first matched securable object privilege
+            for (SecurableObject securableObject : securableObjects) {
+              if (!objectExist
+                  && securableObject.type() == object.type()
+                  && securableObject.fullName().equals(object.fullName())) {
+                objectExist = true;
+                Set<Privilege> updatePrivileges = Sets.newHashSet();
+                updatePrivileges.addAll(securableObject.privileges());
+                // If privileges has granted, we won't generate the new securable object.
+                if (updatePrivileges.containsAll(privileges)) {
+                  updateSecurableObjects.add(securableObject);
+                } else {
+                  updatePrivileges.addAll(privileges);
+                  SecurableObject newSecurableObject =
+                      SecurableObjects.parse(
+                          securableObject.fullName(),
+                          securableObject.type(),
+                          Lists.newArrayList(updatePrivileges));
+
+                  updateSecurableObjects.add(newSecurableObject);
+                  AuthorizationUtils.callAuthorizationPluginForMetadataObject(
+                      metalake,
+                      object,
+                      authorizationPlugin -> {
+                        authorizationPlugin.onRoleUpdated(
+                            roleEntity,
+                            RoleChange.updateSecurableObject(
+                                role, securableObject, newSecurableObject));
+                      });
+                }
+
+              } else {
+                updateSecurableObjects.add(securableObject);
+              }
+            }
+
+            // If the role doesn't contain the securable object, the role appends a new securable
+            // object.
+            if (!objectExist) {
+              SecurableObject securableObject =
+                  SecurableObjects.parse(


Suggested change

List<SecurableObject> updateSecurableObjects = Lists.newArrayList();

// Only update the first matched securable object privilege

for (SecurableObject securableObject : securableObjects) {

if (!objectExist

&& securableObject.type() == object.type()

&& securableObject.fullName().equals(object.fullName())) {

objectExist = true;

Set<Privilege> updatePrivileges = Sets.newHashSet();

updatePrivileges.addAll(securableObject.privileges());

// If privileges has granted, we won't generate the new securable object.

if (updatePrivileges.containsAll(privileges)) {

updateSecurableObjects.add(securableObject);

} else {

updatePrivileges.addAll(privileges);

SecurableObject newSecurableObject =

SecurableObjects.parse(

securableObject.fullName(),

securableObject.type(),

Lists.newArrayList(updatePrivileges));

updateSecurableObjects.add(newSecurableObject);

AuthorizationUtils.callAuthorizationPluginForMetadataObject(

metalake,

object,

authorizationPlugin -> {

authorizationPlugin.onRoleUpdated(

roleEntity,

RoleChange.updateSecurableObject(

role, securableObject, newSecurableObject));

});

}

} else {

updateSecurableObjects.add(securableObject);

}

}

// If the role doesn't contain the securable object, the role appends a new securable

// object.

if (!objectExist) {

SecurableObject securableObject =

SecurableObjects.parse(

List<SecurableObject> updateSecurableObjects = Lists.newArrayList(securableObjects);

SecurableObject se =

securableObjects.stream().filter(securableObject -> securableObject.type() == object.type()

&& securableObject.fullName().equals(object.fullName()))

.findFirst().orElse(null);

if (se == null) {

SecurableObject securableObject =

SecurableObjects.parse(

object.fullName(), object.type(), Lists.newArrayList(privileges));

AuthorizationUtils.callAuthorizationPluginForMetadataObject(

metalake,

object,

authorizationPlugin -> {

authorizationPlugin.onRoleUpdated(

roleEntity, RoleChange.addSecurableObject(role, securableObject));

});

updateSecurableObjects.add(securableObject);

} else {

Set<Privilege> updatePrivileges = Sets.newHashSet();

updatePrivileges.addAll(se.privileges());

if (!updatePrivileges.containsAll(privileges)) {

updatePrivileges.addAll(privileges);

SecurableObject newSecurableObject =

SecurableObjects.parse(

se.fullName(),

se.type(),

Lists.newArrayList(updatePrivileges));

// Remove the old one and add the new one

updateSecurableObjects.remove(se);

updateSecurableObjects.add(newSecurableObject);

// Update the new newSecurableObject in the authorization plugin

AuthorizationUtils.callAuthorizationPluginForMetadataObject(

metalake,

object,

authorizationPlugin -> {

authorizationPlugin.onRoleUpdated(

roleEntity,

RoleChange.updateSecurableObject(

role, se, newSecurableObject));

});

}

}

I rewrite here.

jerryshao · 2024-09-27T10:44:52Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

-  private List<String> toRoleNames(List<RoleEntity> roleEntities) {
-    return roleEntities.stream().map(RoleEntity::name).collect(Collectors.toList());
+  Role grantPrivilegeToRole(
+      String metalake, String role, MetadataObject object, List<Privilege> privileges) {


I think you need to check if these privileges can be bound to this metadata object before you update the role.

Also I think you need to deduplicate the privileges if the user inputs the same privileges.

I deduplicate privileges in the PermissonManager.

What if two same privileges with one allow and one deny are both set, what will be happened and do you handle such scenario?

I add new check logic to avoid this case.

jerryshao · 2024-09-27T10:46:09Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

+            boolean objectExist = false;
+
+            List<SecurableObject> updateSecurableObjects = Lists.newArrayList();
+            // Only update the first matched securable object privilege


What's the meaning of this comment?

After discussing with Yuqi, I add more limitations. We won't have duplicated securable objects. So I remove this comment.

jerryshao · 2024-09-27T10:52:11Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

+                          Lists.newArrayList(updatePrivileges));
+
+                  updateSecurableObjects.add(newSecurableObject);
+                  AuthorizationUtils.callAuthorizationPluginForMetadataObject(


Should you do this in the SQL transaction, or after the update is done?

jerryshao · 2024-09-29T03:44:26Z

clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java

+   * @throws NoSuchMetadataObjectException If the metadata object with the given name does not
+   *     exist.
+   * @throws NoSuchMetalakeException If the Metalake with the given name does not exist.
+   * @throws RuntimeException If revoking roles from a group encounters storage issues.


“grant”, not "revoke".

jerryshao · 2024-09-29T03:45:49Z

clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java

+   * @throws RuntimeException If revoking roles from a group encounters storage issues.
+   */
+  public Role grantPrivilegeToRole(String role, MetadataObject object, List<Privilege> privileges)
+      throws NoSuchRoleException, NoSuchMetadataObjectException, NoSuchMetalakeException {


I think we should also add an exception about failed to grant/revoke privileges to role, because some privileges cannot be bound to some metadata objects.

I used IllegalArgumentException. WDYT?

On a second thought, maybe we should explicitly define an exception for illegal grant/revoke action, so that users will know better what issue it is, not implicitly thrown an exception here. Even for IllegalArgumentException I would suggest clearly define it.

OK, I will do it.

jerryshao · 2024-09-29T03:48:15Z

clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java

+   * @throws NoSuchMetadataObjectException If the metadata object with the given name does not
+   *     exist.
+   * @throws NoSuchMetalakeException If the Metalake with the given name does not exist.
+   * @throws RuntimeException If revoking roles from a group encounters storage issues.


Check all the codes to make sure no such error for "grant" or "revoke".

jerryshao · 2024-09-29T03:58:20Z

common/src/main/java/org/apache/gravitino/dto/requests/RoleCreateRequest.java

-
-    Preconditions.checkArgument(
-        securableObjects != null && securableObjects.length != 0,
-        "\"securableObjects\" can't null or empty");


Can you add a UT and IT about the changes here, to check to create a role with empty/null privilege?

jerryshao · 2024-09-29T03:59:01Z

core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java

+   * @return The role after granted.
+   * @throws NoSuchRoleException If the role with the given name does not exist.
+   * @throws NoSuchMetalakeException If the Metalake with the given name does not exist.
+   * @throws RuntimeException If revoking roles from a group encounters storage issues.


Also here...

jerryshao · 2024-09-29T04:07:48Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

+                .withAuditInfo(auditInfo)
+                .withSecurableObjects(updateSecurableObjects)
+                .build();
+          });


The update logic here is too long, you'd better split into several helper methods.

jerryshao · 2024-09-29T04:38:29Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

+            List<SecurableObject> securableObjects = roleEntity.securableObjects();
+
+            List<SecurableObject> updateSecurableObjects = Lists.newArrayList();
+            // Remove all the matching securable object privileges


What if all the securable object are not matched, or the privileges are not/partially matched, should you at least add some logs?

yuqi1129 · 2024-09-29T06:33:19Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

+                .build();
+          });
+    } catch (NoSuchEntityException nse) {
+      LOG.warn("Failed to revoke, role {} does not exist in the metalake {}", role, metalake, nse);


yuqi1129 · 2024-09-29T06:47:42Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

+
+            List<SecurableObject> updateSecurableObjects = Lists.newArrayList();
+            // Only update the first matched securable object privilege
+            for (SecurableObject securableObject : securableObjects) {


Why not remove duplicated items in the roleEntity?

jerryshao · 2024-10-08T03:24:43Z

clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java

+   * @throws RuntimeException If revoking roles from a group encounters storage issues.
+   */
+  public Role grantPrivilegeToRole(String role, MetadataObject object, List<Privilege> privileges)
+      throws NoSuchRoleException, NoSuchMetadataObjectException, NoSuchMetalakeException {


On a second thought, maybe we should explicitly define an exception for illegal grant/revoke action, so that users will know better what issue it is, not implicitly thrown an exception here. Even for IllegalArgumentException I would suggest clearly define it.

jerryshao · 2024-10-08T03:37:10Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

-  private List<String> toRoleNames(List<RoleEntity> roleEntities) {
-    return roleEntities.stream().map(RoleEntity::name).collect(Collectors.toList());
+  Role grantPrivilegeToRole(
+      String metalake, String role, MetadataObject object, List<Privilege> privileges) {


What if two same privileges with one allow and one deny are both set, what will be happened and do you handle such scenario?

jerryshao · 2024-10-08T03:39:03Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

+                                    object.type(),
+                                    Lists.newArrayList(privileges));
+
+                            authorizationPluginCallbackWrapper.setCallBack(


Can we do this out of SQL transaction, this will hurt the performance and lead to unexpected behavior if we put it here.

This place we only set the callback. After we execute SQL transaction, we execute the callbak.

jerryshao · 2024-10-08T03:43:32Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

+                            if (updatePrivileges.containsAll(privileges)) {
+                              return oldObject;
+                            } else {
+                              updatePrivileges.addAll(privileges);


Should you check the allow and deny of the privileges here?

Also, do you need to check if the privileges are inclusive or exclusive, so you don't have store some particular privileges to the storage?

If the privilege have the same Name , but it have different conditions. They are different privileges.

I don't know the meaning about inclusive or exclusive.
If you mean the case about use schema and select table, we don't check it in the granting or revoking operations.
If you mean the case about schema's select table and table's select table, we don't check it, too. grant/revoke operation is effect on a specific object, schema and table are different objects.

OK, I will check the privileges. We don't allow the same name privilege is allowed and denied at the same time.

jerryshao · 2024-10-08T03:44:35Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

+                    .withAuditInfo(auditInfo)
+                    .withSecurableObjects(updateSecurableObjects)
+                    .build();
+              });


I think the method is still too big, you need to split it into some methods.

Already splitted.

jerryshao · 2024-10-08T03:49:56Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

+                                object.fullName(),
+                                object.type(),
+                                role);
+                            return null;


What will be happened in the client side when we return a null object?

This is updater. If this return null, we don't include the the new object.

jerryshao · 2024-10-08T03:51:06Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

+                            // securable object. Remove duplicated privileges
+                            Set<Privilege> updatePrivileges = Sets.newHashSet();
+                            updatePrivileges.addAll(oldObject.privileges());
+                            privileges.forEach(updatePrivileges::remove);


The same question here, some privileges are not exactly the same, but they're inclusive, should you handle this problem?

jerryshao · 2024-10-08T03:53:12Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

+                                                RoleChange.removeSecurableObject(role, oldObject));
+                                          }));
+
+                              return null;


Also here, it would be better to 1) simplify the current method to split into several small methods; 2) move the callback out of transaction to avoid transaction block.

I will split it.

jerryshao · 2024-10-08T10:06:23Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

+
+    // We set authorization callback here, we won't execute this callback in this place.
+    // We will execute the callback after we execute the SQL transaction.
+    authorizationPluginCallbackWrapper.setCallBack(


Callback is a word, please change to Callback.

jerryshao · 2024-10-08T10:10:21Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

+
+      return newSecurableObject;
+    } else {
+      // If the object doesn't contain any privilege, we remove this object.


Do we need to remove this in the database?

We have removed the object in the database in the current logic.

jerryshao · 2024-10-08T10:12:24Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

+      Function<SecurableObject, SecurableObject> objectUpdater) {
+    boolean isExist = false;
+    List<SecurableObject> updateSecurableObjects = Lists.newArrayList();
+    for (SecurableObject securableObject : securableObjects) {


Do we support updating multiple securable objects in one time? I don't quite understand the code here, can you please explain more?

We don't support to update multiple securable objects. We find one target securable object and update the privileges of the obejct.

If we don't support, why don't we just search the matched securable object and update it, no need to do this in a loop add a flag?

For example:

securableObject = get matched securableObject updater.apply(securableOject)

The method name isn't good. I should generate all the securable objects for the role. I changed to

generateNewSecurableObjects

And I tune the logic of this method and I make it clear.

List<SecurableObject> updateSecurableObjects = Lists.newArrayList(securableObjects); SecurableObject matchedSecurableObject = securableObjects.stream().filter(targetObject::equals).findFirst().orElse(null); if (matchedSecurableObject != null) { updateSecurableObjects.remove(matchedSecurableObject); }

This may be more efficient that the above.

jerryshao · 2024-10-08T10:14:36Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

+              Entity.EntityType.ROLE,
+              roleEntity -> {
+                List<SecurableObject> updateSecurableObjects =
+                    updateSecurableObjects(


The variable here is the same as method name, can you change to a different name?

changed to grantedSecurableObjects.

jerryshao · 2024-10-08T10:16:00Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

+      MetadataObject targetObject,
+      Function<SecurableObject, SecurableObject> objectUpdater) {
+    boolean isExist = false;
+    List<SecurableObject> updateSecurableObjects = Lists.newArrayList();


Also here, can you use a different name?

Changed to RevokedSecurableObjects.

jerryshao · 2024-10-08T10:18:59Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

+
+    // If there not exists a securable object matching the target object.
+    if (!isExist) {
+      SecurableObject newSecurableObject = objectUpdater.apply(null);


From the code I saw that if the input is null, then the output of this function is always null, can you please confirm this?

In grant method, if apply null, we will return a new created object.

jerryshao · 2024-10-08T10:22:47Z

core/src/main/java/org/apache/gravitino/storage/relational/service/RoleMetaService.java

+            for (SecurableObjectPO objectPO : deleteSecurableObjectPOS) {
+              SessionUtils.doWithoutCommit(
+                  SecurableObjectMapper.class,
+                  mapper ->
+                      mapper.softDeleteSecurableObjectsByObjectIdAndPrivileges(
+                          objectPO.getRoleId(),
+                          objectPO.getMetadataObjectId(),
+                          objectPO.getPrivilegeConditions(),
+                          objectPO.getPrivilegeNames()));
+            }


Can we do this in a batch?

I tried. It's ok to use batch.

yuqi1129 · 2024-10-08T13:10:04Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

+      Function<SecurableObject, SecurableObject> objectUpdater) {
+    boolean isExist = false;
+    List<SecurableObject> updateSecurableObjects = Lists.newArrayList();
+    for (SecurableObject securableObject : securableObjects) {


List<SecurableObject> updateSecurableObjects = Lists.newArrayList(securableObjects); SecurableObject matchedSecurableObject = securableObjects.stream().filter(targetObject::equals).findFirst().orElse(null); if (matchedSecurableObject != null) { updateSecurableObjects.remove(matchedSecurableObject); }

This may be more efficient that the above.

yuqi1129 · 2024-10-08T13:20:54Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

+      List<Privilege> privileges,
+      RoleEntity roleEntity,
+      AuthorizationPluginCallbackWrapper authorizationPluginCallbackWrapper) {
+    // Add a new securable object if there not exists the object in the role


if there doesn't exist the...

jerryshao · 2024-10-08T15:02:46Z

core/src/main/java/org/apache/gravitino/storage/relational/service/RoleMetaService.java

+        return newRoleEntity;
+      }
+
+      List<SecurableObjectPO> deleteSecurableObjectPOS =


"POs", not "POS"

jerryshao · 2024-10-08T15:03:46Z

core/src/main/java/org/apache/gravitino/storage/relational/service/RoleMetaService.java

+    }
+  }
+
+  private List<SecurableObjectPO> toSecurablePOs(


toSecurableObjectPOs

jerryshao · 2024-10-08T15:06:27Z

Generally LGTM, @yuqi1129 can you please take another look at this PR?

yuqi1129

Some minor ones, others LGTM.

yuqi1129 · 2024-10-09T01:50:21Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

+      LOG.error("Failed to revoke, role {} does not exist in the metalake {}", role, metalake, nse);
+      throw new NoSuchRoleException(ROLE_DOES_NOT_EXIST_MSG, role, metalake);
+    } catch (IOException ioe) {
+      throw new RuntimeException(ioe);


I noticed that you have logged NoSuchEntityException. Why don't you also add logs here? Do you have any other considerations?

Added the log.

yuqi1129 · 2024-10-09T01:51:07Z

core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java

+    updatePrivileges.addAll(targetObject.privileges());
+    privileges.forEach(updatePrivileges::remove);
+
+    // If the object still contain privilege, we should update the object


contain -> contains

yuqi1129 · 2024-10-09T01:58:35Z

core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java

+    for (Privilege privilege : privileges) {
+      if (privilegeNameSet.contains(privilege.name())) {
+        throw new IllegalPrivilegeException(
+            "Doesn't support duplicated privilege name %s with different condition",


Can you output all the duplicated privilege names , not just one of them?

This should be unnecessary. It will be allow and deny the privilege name.

yuqi1129

Some minor ones, others LGTM.

jerryshao · 2024-10-09T02:00:51Z

core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java

-              "Catalog %s type %s don't support privilege %s",
-              catalogIdent, catalog.type(), privilege));
+      throw new IllegalPrivilegeException(
+          "Catalog %s type %s don't support privilege %s", catalogIdent, catalog.type(), privilege);


"doesn't support..."

yuqi1129

LGTM

jerryshao · 2024-10-09T03:28:44Z

@jerqi the cherry-pick is failed, please do it manually against branch-0.6.

…s for a role (apache#5021) Supports to grant or revoke privileges for a role Fix: apache#4903 I will add the document later. Add the UT.

jerqi force-pushed the ISSUE-4903 branch 2 times, most recently from 6b06613 to 32bb163 Compare September 25, 2024 12:04

jerqi self-assigned this Sep 25, 2024

jerqi added the branch-0.6 label Sep 25, 2024

jerqi requested review from yuqi1129, jerryshao and xunliu September 25, 2024 12:39

jerqi force-pushed the ISSUE-4903 branch 3 times, most recently from fc4ee73 to 9e62ca7 Compare September 26, 2024 02:23

jerryshao reviewed Sep 27, 2024

View reviewed changes

yuqi1129 reviewed Sep 27, 2024

View reviewed changes

jerryshao reviewed Sep 27, 2024

View reviewed changes

Support to grant or revoke securables

4aecca2

jerqi force-pushed the ISSUE-4903 branch from 9e62ca7 to 4aecca2 Compare September 27, 2024 14:05

jerryshao reviewed Sep 29, 2024

View reviewed changes

yuqi1129 reviewed Sep 29, 2024

View reviewed changes

jerqi added 2 commits September 29, 2024 14:52

add limitation

bd4d55e

Add comments

18a4932

Fix comment

f7c681a

jerryshao reviewed Oct 8, 2024

View reviewed changes

jerqi added 3 commits October 8, 2024 16:14

address comment

b6802de

remove unnecessary change

c03ef32

Add new uts

51a6880

jerryshao reviewed Oct 8, 2024

View reviewed changes

jerqi added 3 commits October 8, 2024 19:16

Address comment

c32d652

Fix SQL grammar

79bdf52

polish by myself

c7ec422

yuqi1129 reviewed Oct 8, 2024

View reviewed changes

address comment

45da052

jerryshao reviewed Oct 8, 2024

View reviewed changes

yuqi1129 reviewed Oct 9, 2024

View reviewed changes

jerryshao reviewed Oct 9, 2024

View reviewed changes

jerqi added 3 commits October 9, 2024 10:21

Add logs

6751c60

Change the code order

7c2163f

Modify the log information

69cc0dc

yuqi1129 approved these changes Oct 9, 2024

View reviewed changes

jerryshao merged commit 75ec8ae into apache:main Oct 9, 2024
26 checks passed

[#4903] feat(core,server): Support to grant or revoke privileges for a role #5021

[#4903] feat(core,server): Support to grant or revoke privileges for a role #5021

Uh oh!

Conversation

jerqi commented Sep 25, 2024

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

jerqi commented Sep 25, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jerryshao Sep 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jerryshao Sep 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jerryshao Sep 27, 2024 •

edited

Loading

jerryshao Sep 29, 2024 •

edited

Loading