Skip to content

Commit

Permalink
[#5105] improvement(server,client): Error code optimization about acc…
Browse files Browse the repository at this point in the history
…ess control API (#5144)

### What changes were proposed in this pull request?

Error code optimization about access control API

### Why are the changes needed?

Fix: #5105 

### Does this PR introduce _any_ user-facing change?
Yes.

### How was this patch tested?

Modify some UTs
  • Loading branch information
jerqi authored Oct 18, 2024
1 parent 10e2943 commit 6272303
Show file tree
Hide file tree
Showing 16 changed files with 350 additions and 74 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.gravitino.exceptions;

import com.google.errorprone.annotations.FormatMethod;
import com.google.errorprone.annotations.FormatString;

/** An exception thrown when a metadata object is invalid. */
public class IllegalMetadataObjectException extends IllegalArgumentException {
/**
* Constructs a new exception with the specified detail message.
*
* @param message the detail message.
* @param args the arguments to the message.
*/
@FormatMethod
public IllegalMetadataObjectException(@FormatString String message, Object... args) {
super(String.format(message, args));
}

/**
* Constructs a new exception with the specified detail message and cause.
*
* @param cause the cause.
* @param message the detail message.
* @param args the arguments to the message.
*/
@FormatMethod
public IllegalMetadataObjectException(
Throwable cause, @FormatString String message, Object... args) {
super(String.format(message, args), cause);
}

/**
* Constructs a new exception with the specified cause.
*
* @param cause the cause.
*/
public IllegalMetadataObjectException(Throwable cause) {
super(cause);
}

/** Constructs a new exception with the specified detail message and cause. */
public IllegalMetadataObjectException() {
super();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.gravitino.exceptions;

import com.google.errorprone.annotations.FormatMethod;
import com.google.errorprone.annotations.FormatString;

/** An exception thrown when a role is invalid. */
public class IllegalRoleException extends IllegalArgumentException {
/**
* Constructs a new exception with the specified detail message.
*
* @param message the detail message.
* @param args the arguments to the message.
*/
@FormatMethod
public IllegalRoleException(@FormatString String message, Object... args) {
super(String.format(message, args));
}

/**
* Constructs a new exception with the specified detail message and cause.
*
* @param cause the cause.
* @param message the detail message.
* @param args the arguments to the message.
*/
@FormatMethod
public IllegalRoleException(Throwable cause, @FormatString String message, Object... args) {
super(String.format(message, args), cause);
}

/**
* Constructs a new exception with the specified cause.
*
* @param cause the cause.
*/
public IllegalRoleException(Throwable cause) {
super(cause);
}

/** Constructs a new exception with the specified detail message and cause. */
public IllegalRoleException() {
super();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@
import org.apache.gravitino.exceptions.FilesetAlreadyExistsException;
import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.exceptions.GroupAlreadyExistsException;
import org.apache.gravitino.exceptions.IllegalMetadataObjectException;
import org.apache.gravitino.exceptions.IllegalPrivilegeException;
import org.apache.gravitino.exceptions.IllegalRoleException;
import org.apache.gravitino.exceptions.InUseException;
import org.apache.gravitino.exceptions.MetalakeAlreadyExistsException;
import org.apache.gravitino.exceptions.NoSuchCatalogException;
Expand Down Expand Up @@ -706,6 +708,10 @@ public void accept(ErrorResponse errorResponse) {
case ErrorConstants.ILLEGAL_ARGUMENTS_CODE:
if (errorResponse.getType().equals(IllegalPrivilegeException.class.getSimpleName())) {
throw new IllegalPrivilegeException(errorMessage);
} else if (errorResponse
.getType()
.equals(IllegalMetadataObjectException.class.getSimpleName())) {
throw new IllegalMetadataObjectException(errorMessage);
} else {
throw new IllegalArgumentException(errorMessage);
}
Expand Down Expand Up @@ -756,6 +762,8 @@ public void accept(ErrorResponse errorResponse) {
case ErrorConstants.ILLEGAL_ARGUMENTS_CODE:
if (errorResponse.getType().equals(IllegalPrivilegeException.class.getSimpleName())) {
throw new IllegalPrivilegeException(errorMessage);
} else if (errorResponse.getType().equals(IllegalRoleException.class.getSimpleName())) {
throw new IllegalRoleException(errorMessage);
} else {
throw new IllegalArgumentException(errorMessage);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@
import org.apache.gravitino.exceptions.CatalogAlreadyExistsException;
import org.apache.gravitino.exceptions.CatalogInUseException;
import org.apache.gravitino.exceptions.GroupAlreadyExistsException;
import org.apache.gravitino.exceptions.IllegalMetadataObjectException;
import org.apache.gravitino.exceptions.IllegalPrivilegeException;
import org.apache.gravitino.exceptions.IllegalRoleException;
import org.apache.gravitino.exceptions.NoSuchCatalogException;
import org.apache.gravitino.exceptions.NoSuchGroupException;
import org.apache.gravitino.exceptions.NoSuchMetadataObjectException;
Expand Down Expand Up @@ -297,12 +299,12 @@ public boolean deleteRole(String role) throws NoSuchMetalakeException {
* @return The created Role instance.
* @throws RoleAlreadyExistsException If a Role with the same name already exists.
* @throws NoSuchMetalakeException If the Metalake with the given name does not exist.
* @throws NoSuchMetadataObjectException If securable object doesn't exist
* @throws IllegalMetadataObjectException If securable object is invalid
* @throws RuntimeException If creating the Role encounters storage issues.
*/
public Role createRole(
String role, Map<String, String> properties, List<SecurableObject> securableObjects)
throws RoleAlreadyExistsException, NoSuchMetalakeException, NoSuchMetadataObjectException {
throws RoleAlreadyExistsException, NoSuchMetalakeException, IllegalMetadataObjectException {
return getMetalake().createRole(role, properties, securableObjects);
}
/**
Expand All @@ -312,12 +314,12 @@ public Role createRole(
* @param roles The names of the Role.
* @return The Group after granted.
* @throws NoSuchUserException If the User with the given name does not exist.
* @throws NoSuchRoleException If the Role with the given name does not exist.
* @throws IllegalRoleException If the Role with the given name is invalid.
* @throws NoSuchMetalakeException If the Metalake with the given name does not exist.
* @throws RuntimeException If granting roles to a user encounters storage issues.
*/
public User grantRolesToUser(List<String> roles, String user)
throws NoSuchUserException, NoSuchRoleException, NoSuchMetalakeException {
throws NoSuchUserException, IllegalRoleException, NoSuchMetalakeException {
return getMetalake().grantRolesToUser(roles, user);
}

Expand All @@ -328,12 +330,12 @@ public User grantRolesToUser(List<String> roles, String user)
* @param roles The names of the Role.
* @return The Group after granted.
* @throws NoSuchGroupException If the Group with the given name does not exist.
* @throws NoSuchRoleException If the Role with the given name does not exist.
* @throws IllegalRoleException If the Role with the given name is invalid.
* @throws NoSuchMetalakeException If the Metalake with the given name does not exist.
* @throws RuntimeException If granting roles to a group encounters storage issues.
*/
public Group grantRolesToGroup(List<String> roles, String group)
throws NoSuchGroupException, NoSuchRoleException, NoSuchMetalakeException {
throws NoSuchGroupException, IllegalRoleException, NoSuchMetalakeException {
return getMetalake().grantRolesToGroup(roles, group);
}

Expand All @@ -344,12 +346,12 @@ public Group grantRolesToGroup(List<String> roles, String group)
* @param roles The names of the Role.
* @return The User after revoked.
* @throws NoSuchUserException If the User with the given name does not exist.
* @throws NoSuchRoleException If the Role with the given name does not exist.
* @throws IllegalRoleException If the Role with the given name is invalid.
* @throws NoSuchMetalakeException If the Metalake with the given name does not exist.
* @throws RuntimeException If revoking roles from a user encounters storage issues.
*/
public User revokeRolesFromUser(List<String> roles, String user)
throws NoSuchUserException, NoSuchRoleException, NoSuchMetalakeException {
throws NoSuchUserException, IllegalRoleException, NoSuchMetalakeException {
return getMetalake().revokeRolesFromUser(roles, user);
}

Expand All @@ -360,12 +362,12 @@ public User revokeRolesFromUser(List<String> roles, String user)
* @param roles The names of the Role.
* @return The Group after revoked.
* @throws NoSuchGroupException If the Group with the given name does not exist.
* @throws NoSuchRoleException If the Role with the given name does not exist.
* @throws IllegalRoleException If the Role with the given name is invalid.
* @throws NoSuchMetalakeException If the Metalake with the given name does not exist.
* @throws RuntimeException If revoking roles from a group encounters storage issues.
*/
public Group revokeRolesFromGroup(List<String> roles, String group)
throws NoSuchGroupException, NoSuchRoleException, NoSuchMetalakeException {
throws NoSuchGroupException, IllegalRoleException, NoSuchMetalakeException {
return getMetalake().revokeRolesFromGroup(roles, group);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,9 @@
import org.apache.gravitino.exceptions.CatalogAlreadyExistsException;
import org.apache.gravitino.exceptions.CatalogInUseException;
import org.apache.gravitino.exceptions.GroupAlreadyExistsException;
import org.apache.gravitino.exceptions.IllegalMetadataObjectException;
import org.apache.gravitino.exceptions.IllegalPrivilegeException;
import org.apache.gravitino.exceptions.IllegalRoleException;
import org.apache.gravitino.exceptions.NoSuchCatalogException;
import org.apache.gravitino.exceptions.NoSuchGroupException;
import org.apache.gravitino.exceptions.NoSuchMetadataObjectException;
Expand Down Expand Up @@ -785,12 +787,12 @@ public boolean deleteRole(String role) throws NoSuchMetalakeException {
* @return The created Role instance.
* @throws RoleAlreadyExistsException If a Role with the same name already exists.
* @throws NoSuchMetalakeException If the Metalake with the given name does not exist.
* @throws NoSuchMetadataObjectException If the securable object doesn't exist
* @throws IllegalMetadataObjectException If the securable object is invalid
* @throws RuntimeException If creating the Role encounters storage issues.
*/
public Role createRole(
String role, Map<String, String> properties, List<SecurableObject> securableObjects)
throws RoleAlreadyExistsException, NoSuchMetalakeException, NoSuchMetadataObjectException {
throws RoleAlreadyExistsException, NoSuchMetalakeException, IllegalMetadataObjectException {
RoleCreateRequest req =
new RoleCreateRequest(
role,
Expand Down Expand Up @@ -837,12 +839,12 @@ public String[] listRoleNames() {
* @param roles The names of the Role.
* @return The Group after granted.
* @throws NoSuchUserException If the User with the given name does not exist.
* @throws NoSuchRoleException If the Role with the given name does not exist.
* @throws IllegalRoleException If the Role with the given name is invalid.
* @throws NoSuchMetalakeException If the Metalake with the given name does not exist.
* @throws RuntimeException If granting roles to a user encounters storage issues.
*/
public User grantRolesToUser(List<String> roles, String user)
throws NoSuchUserException, NoSuchRoleException, NoSuchMetalakeException {
throws NoSuchUserException, IllegalRoleException, NoSuchMetalakeException {
RoleGrantRequest request = new RoleGrantRequest(roles);
request.validate();

Expand All @@ -868,7 +870,7 @@ public User grantRolesToUser(List<String> roles, String user)
* @param roles The names of the Role.
* @return The Group after granted.
* @throws NoSuchGroupException If the Group with the given name does not exist.
* @throws NoSuchRoleException If the Role with the given name does not exist.
* @throws IllegalRoleException If the Role with the given name is invalid.
* @throws NoSuchMetalakeException If the Metalake with the given name does not exist.
* @throws RuntimeException If granting roles to a group encounters storage issues.
*/
Expand Down Expand Up @@ -899,7 +901,7 @@ public Group grantRolesToGroup(List<String> roles, String group)
* @param roles The names of the Role.
* @return The User after revoked.
* @throws NoSuchUserException If the User with the given name does not exist.
* @throws NoSuchRoleException If the Role with the given name does not exist.
* @throws IllegalRoleException If the Role with the given name is invalid.
* @throws NoSuchMetalakeException If the Metalake with the given name does not exist.
* @throws RuntimeException If revoking roles from a user encounters storage issues.
*/
Expand Down Expand Up @@ -930,12 +932,12 @@ public User revokeRolesFromUser(List<String> roles, String user)
* @param roles The names of the Role.
* @return The Group after revoked.
* @throws NoSuchGroupException If the Group with the given name does not exist.
* @throws NoSuchRoleException If the Role with the given name does not exist.
* @throws IllegalRoleException If the Role with the given name is invalid.
* @throws NoSuchMetalakeException If the Metalake with the given name does not exist.
* @throws RuntimeException If revoking roles from a group encounters storage issues.
*/
public Group revokeRolesFromGroup(List<String> roles, String group)
throws NoSuchGroupException, NoSuchRoleException, NoSuchMetalakeException {
throws NoSuchGroupException, IllegalRoleException, NoSuchMetalakeException {
RoleRevokeRequest request = new RoleRevokeRequest(roles);
request.validate();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,10 @@
import org.apache.gravitino.authorization.User;
import org.apache.gravitino.client.GravitinoMetalake;
import org.apache.gravitino.exceptions.GroupAlreadyExistsException;
import org.apache.gravitino.exceptions.IllegalMetadataObjectException;
import org.apache.gravitino.exceptions.IllegalPrivilegeException;
import org.apache.gravitino.exceptions.IllegalRoleException;
import org.apache.gravitino.exceptions.NoSuchGroupException;
import org.apache.gravitino.exceptions.NoSuchMetadataObjectException;
import org.apache.gravitino.exceptions.NoSuchRoleException;
import org.apache.gravitino.exceptions.NoSuchUserException;
import org.apache.gravitino.exceptions.UserAlreadyExistsException;
Expand Down Expand Up @@ -214,7 +215,7 @@ void testManageRoles() {
"not-existed", Lists.newArrayList(Privileges.UseCatalog.allow()));

Assertions.assertThrows(
NoSuchMetadataObjectException.class,
IllegalMetadataObjectException.class,
() -> metalake.createRole("not-existed", properties, Lists.newArrayList(catalogObject)));

// Create a role with duplicated securable objects
Expand Down Expand Up @@ -359,12 +360,12 @@ void testManageUserPermissions() {

// Grant a not-existed role
Assertions.assertThrows(
NoSuchRoleException.class,
IllegalRoleException.class,
() -> metalake.grantRolesToUser(Lists.newArrayList("not-existed"), username));

// Revoke a not-existed role
Assertions.assertThrows(
NoSuchRoleException.class,
IllegalRoleException.class,
() -> metalake.revokeRolesFromUser(Lists.newArrayList("not-existed"), username));

// Grant to a not-existed user
Expand Down Expand Up @@ -414,12 +415,12 @@ void testManageGroupPermissions() {

// Grant a not-existed role
Assertions.assertThrows(
NoSuchRoleException.class,
IllegalRoleException.class,
() -> metalake.grantRolesToGroup(Lists.newArrayList("not-existed"), groupName));

// Revoke a not-existed role
Assertions.assertThrows(
NoSuchRoleException.class,
IllegalRoleException.class,
() -> metalake.revokeRolesFromGroup(Lists.newArrayList("not-existed"), groupName));

// Grant to a not-existed group
Expand Down
Loading

0 comments on commit 6272303

Please sign in to comment.