This is an automated email from the ASF dual-hosted git repository.
yuqi1129 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 9526d8cfbb [#13519] fix(core): surface NoSuchMetalakeException when
adding or removing users and groups (#13524)
9526d8cfbb is described below
commit 9526d8cfbb16e4761012f275bd0db9e795f47d4c
Author: YangJie <[email protected]>
AuthorDate: Mon Sep 28 05:42:44 2026 -0400
[#13519] fix(core): surface NoSuchMetalakeException when adding or removing
users and groups (#13524)
### What changes were proposed in this pull request?
`UserGroupManager.addUser`, `removeUser`, `addGroup`, and `removeGroup`
now call `MetalakeManager.checkMetalake` on entry, the same check the
sibling `countUsers`, paginated `listUsers`, and `countGroups` already
perform.
### Why are the changes needed?
These four methods declare `NoSuchMetalakeException`, but nothing on the
path enforced it: an add or remove against a missing metalake surfaced
the internal `NoSuchEntityException` from the entity-id lookup, so a
caller catching the declared type missed it and the 404 carried a
generic entity message. The check also rejects a not-in-use metalake
with `MetalakeNotInUseException`, consistent with the sibling methods.
Fix: #13519
### Does this PR introduce _any_ user-facing change?
Adding or removing a user or group in a missing or disabled metalake now
returns the documented metalake exception instead of a generic entity
error. No API or property changes.
### How was this patch tested?
Added `testAddRemoveUserGroupChecksMetalakeExists`, which asserts all
four methods throw `NoSuchMetalakeException` for a missing metalake; it
fails against the pre-fix code, which surfaced `NoSuchEntityException`.
---
.../gravitino/authorization/UserGroupManager.java | 4 ++
.../authorization/TestAccessControlManager.java | 45 ++++++++++++++++++++++
2 files changed, 49 insertions(+)
diff --git
a/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java
b/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java
index be0d56879e..6563ea264d 100644
---
a/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java
+++
b/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java
@@ -65,6 +65,7 @@ class UserGroupManager {
}
User addUser(String metalake, String name) throws UserAlreadyExistsException
{
+ MetalakeManager.checkMetalake(NameIdentifier.of(metalake), store);
try {
UserEntity userEntity =
UserEntity.builder()
@@ -92,6 +93,7 @@ class UserGroupManager {
}
boolean removeUser(String metalake, String user) {
+ MetalakeManager.checkMetalake(NameIdentifier.of(metalake), store);
try {
return store.delete(AuthorizationUtils.ofUser(metalake, user),
Entity.EntityType.USER);
} catch (IOException ioe) {
@@ -139,6 +141,7 @@ class UserGroupManager {
}
Group addGroup(String metalake, String group) throws
GroupAlreadyExistsException {
+ MetalakeManager.checkMetalake(NameIdentifier.of(metalake), store);
try {
GroupEntity groupEntity =
GroupEntity.builder()
@@ -166,6 +169,7 @@ class UserGroupManager {
}
boolean removeGroup(String metalake, String group) {
+ MetalakeManager.checkMetalake(NameIdentifier.of(metalake), store);
try {
return store.delete(AuthorizationUtils.ofGroup(metalake, group),
Entity.EntityType.GROUP);
} catch (IOException ioe) {
diff --git
a/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManager.java
b/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManager.java
index 33638b3057..ac0089fb80 100644
---
a/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManager.java
+++
b/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManager.java
@@ -62,6 +62,7 @@ import org.apache.gravitino.Configs;
import org.apache.gravitino.EntityStore;
import org.apache.gravitino.EntityStoreFactory;
import org.apache.gravitino.GravitinoEnv;
+import org.apache.gravitino.Metalake;
import org.apache.gravitino.Namespace;
import org.apache.gravitino.StringIdentifier;
import org.apache.gravitino.bulk.BulkItemResult;
@@ -73,6 +74,7 @@ import org.apache.gravitino.catalog.CatalogTestUtils;
import org.apache.gravitino.connector.BaseCatalog;
import org.apache.gravitino.connector.authorization.AuthorizationPlugin;
import org.apache.gravitino.exceptions.GroupAlreadyExistsException;
+import org.apache.gravitino.exceptions.MetalakeNotInUseException;
import org.apache.gravitino.exceptions.NoSuchGroupException;
import org.apache.gravitino.exceptions.NoSuchMetalakeException;
import org.apache.gravitino.exceptions.NoSuchRoleException;
@@ -126,6 +128,16 @@ public class TestAccessControlManager {
.withVersion(SchemaVersion.V_0_1)
.build();
+ private static BaseMetalake disabledMetalakeEntity =
+ BaseMetalake.builder()
+ .withId(3L)
+ .withName("metalake_disabled")
+ .withProperties(ImmutableMap.of(Metalake.PROPERTY_IN_USE, "false"))
+ .withAuditInfo(
+
AuditInfo.builder().withCreator("test").withCreateTime(Instant.now()).build())
+ .withVersion(SchemaVersion.V_0_1)
+ .build();
+
@BeforeAll
public static void setUp() throws Exception {
File dbDir = new File(DB_DIR);
@@ -165,6 +177,7 @@ public class TestAccessControlManager {
entityStore.put(metalakeEntity, true);
entityStore.put(listMetalakeEntity, true);
+ entityStore.put(disabledMetalakeEntity, true);
CatalogEntity catalogEntity =
CatalogEntity.builder()
@@ -348,6 +361,38 @@ public class TestAccessControlManager {
Assertions.assertTrue(results.get(2).error().get() instanceof
IllegalArgumentException);
}
+ @Test
+ public void testAddRemoveUserGroupChecksMetalakeExists() {
+ // add/remove user/group against a nonexistent metalake must surface the
+ // documented NoSuchMetalakeException, not a raw storage error.
+ Assertions.assertThrows(
+ NoSuchMetalakeException.class, () ->
accessControlManager.addUser("nope", "u1"));
+ Assertions.assertThrows(
+ NoSuchMetalakeException.class, () ->
accessControlManager.addGroup("nope", "g1"));
+ Assertions.assertThrows(
+ NoSuchMetalakeException.class, () ->
accessControlManager.removeUser("nope", "u1"));
+ Assertions.assertThrows(
+ NoSuchMetalakeException.class, () ->
accessControlManager.removeGroup("nope", "g1"));
+ }
+
+ @Test
+ public void testAddRemoveUserGroupRejectsDisabledMetalake() {
+ // add/remove user/group against a disabled (not-in-use) metalake must
surface
+ // MetalakeNotInUseException, consistent with the count/list siblings.
+ Assertions.assertThrows(
+ MetalakeNotInUseException.class,
+ () -> accessControlManager.addUser("metalake_disabled", "u1"));
+ Assertions.assertThrows(
+ MetalakeNotInUseException.class,
+ () -> accessControlManager.addGroup("metalake_disabled", "g1"));
+ Assertions.assertThrows(
+ MetalakeNotInUseException.class,
+ () -> accessControlManager.removeUser("metalake_disabled", "u1"));
+ Assertions.assertThrows(
+ MetalakeNotInUseException.class,
+ () -> accessControlManager.removeGroup("metalake_disabled", "g1"));
+ }
+
@Test
public void testListUsers() {
accessControlManager.addUser("metalake_list", "testList1");
