This is an automated email from the ASF dual-hosted git repository.
yuqi1129 pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new 25ae14d830 [Cherry-pick to branch-1.3] [#13519] fix(core): surface
NoSuchMetalakeException when adding or removing users and groups (#13524)
(#13564)
25ae14d830 is described below
commit 25ae14d830ddf2f5da8748886ffbfed740c4f173
Author: YangJie <[email protected]>
AuthorDate: Mon Sep 28 10:05:10 2026 -0400
[Cherry-pick to branch-1.3] [#13519] fix(core): surface
NoSuchMetalakeException when adding or removing users and groups (#13524)
(#13564)
### What changes were proposed in this pull request?
Cherry-pick of #13524 to branch-1.3. `UserGroupManager.addUser`,
`removeUser`, `addGroup`, and `removeGroup` now call
`MetalakeManager.checkMetalake` on entry, so a missing or not-in-use
metalake surfaces the documented `NoSuchMetalakeException` /
`MetalakeNotInUseException` instead of a raw entity error.
The automatic cherry-pick in #13559 conflicted because branch-1.3 does
not have the bulk user/group feature that #13524's test sat next to on
main. This PR applies the same fix adapted to branch-1.3: the two new
tests, plus the `NameIdentifier` and `MetalakeManager` imports that
branch-1.3's `UserGroupManager` did not yet have. It supersedes #13559.
### Why are the changes needed?
Backport the fix for #13519 to branch-1.3.
### 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?
`testAddRemoveUserGroupChecksMetalakeExists` asserts all four methods
throw `NoSuchMetalakeException` for a missing metalake;
`testAddRemoveUserGroupRejectsDisabledMetalake` asserts
`MetalakeNotInUseException` for a disabled one. Both fail against the
pre-fix code.
---
.../gravitino/authorization/UserGroupManager.java | 6 +++
.../authorization/TestAccessControlManager.java | 46 ++++++++++++++++++++++
2 files changed, 52 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 9a244212e1..8b4d327616 100644
---
a/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java
+++
b/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java
@@ -27,6 +27,7 @@ import org.apache.gravitino.Entity;
import org.apache.gravitino.Entity.EntityType;
import org.apache.gravitino.EntityAlreadyExistsException;
import org.apache.gravitino.EntityStore;
+import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.Namespace;
import org.apache.gravitino.exceptions.GroupAlreadyExistsException;
import org.apache.gravitino.exceptions.NoSuchEntityException;
@@ -37,6 +38,7 @@ import
org.apache.gravitino.exceptions.UserAlreadyExistsException;
import org.apache.gravitino.meta.AuditInfo;
import org.apache.gravitino.meta.GroupEntity;
import org.apache.gravitino.meta.UserEntity;
+import org.apache.gravitino.metalake.MetalakeManager;
import org.apache.gravitino.storage.IdGenerator;
import org.apache.gravitino.utils.PrincipalUtils;
import org.slf4j.Logger;
@@ -61,6 +63,7 @@ class UserGroupManager {
}
User addUser(String metalake, String name) throws UserAlreadyExistsException
{
+ MetalakeManager.checkMetalake(NameIdentifier.of(metalake), store);
try {
UserEntity userEntity =
UserEntity.builder()
@@ -88,6 +91,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) {
@@ -122,6 +126,7 @@ class UserGroupManager {
}
Group addGroup(String metalake, String group) throws
GroupAlreadyExistsException {
+ MetalakeManager.checkMetalake(NameIdentifier.of(metalake), store);
try {
GroupEntity groupEntity =
GroupEntity.builder()
@@ -149,6 +154,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 9bdfc167c9..dcf9ed3d3e 100644
---
a/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManager.java
+++
b/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManager.java
@@ -59,6 +59,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.catalog.CatalogManager;
@@ -66,7 +67,9 @@ 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;
import org.apache.gravitino.exceptions.NoSuchUserException;
import org.apache.gravitino.exceptions.RoleAlreadyExistsException;
@@ -117,6 +120,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);
@@ -156,6 +169,7 @@ public class TestAccessControlManager {
entityStore.put(metalakeEntity, true);
entityStore.put(listMetalakeEntity, true);
+ entityStore.put(disabledMetalakeEntity, true);
CatalogEntity catalogEntity =
CatalogEntity.builder()
@@ -319,6 +333,38 @@ public class TestAccessControlManager {
Assertions.assertFalse(removed1);
}
+ @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 testServiceAdmin() {
Assertions.assertTrue(accessControlManager.isServiceAdmin("admin1"));
