This is an automated email from the ASF dual-hosted git repository.
jerryshao 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 19e3a555ca [#13504] fix(core): Report missing securable objects
correctly (#13505)
19e3a555ca is described below
commit 19e3a555caa85add47d54b473d54931ee4151700
Author: roryqi <[email protected]>
AuthorDate: Mon Sep 28 10:27:34 2026 +0800
[#13504] fix(core): Report missing securable objects correctly (#13505)
### What changes were proposed in this pull request?
Convert `NoSuchEntityException` raised while resolving a role's
securable object into `NoSuchMetadataObjectException`, while preserving
the original cause.
Add a regression test that verifies the correct exception and ensures
the role remains unchanged after the failed update.
### Why are the changes needed?
Role privilege updates currently report every missing entity as a
missing role. When the role exists but its securable object cannot be
resolved, clients incorrectly receive `NoSuchRoleException`, masking the
actual failure.
Fix: #13504
### Does this PR introduce _any_ user-facing change?
Yes. When a securable object is missing during a role privilege update,
clients now receive `NoSuchMetadataObjectException` instead of the
incorrect `NoSuchRoleException`. Missing roles retain their existing
behavior.
### How was this patch tested?
- `./gradlew :core:test --tests
org.apache.gravitino.storage.relational.service.TestRoleMetaService
-PskipDockerTests=false`
- `./gradlew :core:test --tests
org.apache.gravitino.authorization.TestAccessControlManagerForPermissions
-PskipDockerTests=false`
- `./gradlew :core:spotlessApply`
- `git diff --check`
---
.../relational/service/RoleMetaService.java | 8 +++-
.../relational/service/TestRoleMetaService.java | 44 ++++++++++++++++++++++
2 files changed, 51 insertions(+), 1 deletion(-)
diff --git
a/core/src/main/java/org/apache/gravitino/storage/relational/service/RoleMetaService.java
b/core/src/main/java/org/apache/gravitino/storage/relational/service/RoleMetaService.java
index 769474c192..318ead56b6 100644
---
a/core/src/main/java/org/apache/gravitino/storage/relational/service/RoleMetaService.java
+++
b/core/src/main/java/org/apache/gravitino/storage/relational/service/RoleMetaService.java
@@ -43,6 +43,7 @@ import org.apache.gravitino.Namespace;
import org.apache.gravitino.authorization.AuthorizationUtils;
import org.apache.gravitino.authorization.SecurableObject;
import org.apache.gravitino.exceptions.NoSuchEntityException;
+import org.apache.gravitino.exceptions.NoSuchMetadataObjectException;
import org.apache.gravitino.exceptions.NoSuchRoleException;
import org.apache.gravitino.meta.RoleEntity;
import org.apache.gravitino.meta.UserEntity;
@@ -335,7 +336,12 @@ public class RoleMetaService {
NameIdentifier nameIdentifier =
MetadataObjectUtil.toEntityIdent(metalake, object);
Entity.EntityType entityType =
MetadataObjectUtil.toEntityType(object.type());
-
objectBuilder.withMetadataObjectId(EntityIdService.getEntityId(nameIdentifier,
entityType));
+ try {
+
objectBuilder.withMetadataObjectId(EntityIdService.getEntityId(nameIdentifier,
entityType));
+ } catch (NoSuchEntityException nse) {
+ throw new NoSuchMetadataObjectException(
+ nse, "Metadata object %s type %s doesn't exist",
object.fullName(), object.type());
+ }
securableObjectPOs.add(objectBuilder.build());
}
return securableObjectPOs;
diff --git
a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestRoleMetaService.java
b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestRoleMetaService.java
index ecc034918c..f955bdd336 100644
---
a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestRoleMetaService.java
+++
b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestRoleMetaService.java
@@ -46,6 +46,7 @@ import org.apache.gravitino.authorization.Privileges;
import org.apache.gravitino.authorization.SecurableObject;
import org.apache.gravitino.authorization.SecurableObjects;
import org.apache.gravitino.exceptions.NoSuchEntityException;
+import org.apache.gravitino.exceptions.NoSuchMetadataObjectException;
import org.apache.gravitino.exceptions.OptimisticLockException;
import org.apache.gravitino.meta.AuditInfo;
import org.apache.gravitino.meta.BaseMetalake;
@@ -840,6 +841,49 @@ class TestRoleMetaService extends TestJDBCBackend {
Assertions.assertTrue(revokeMultipleRole.securableObjects().isEmpty());
}
+ @TestTemplate
+ void testUpdateRoleReportsMissingSecurableObject() throws IOException {
+ createAndInsertMakeLake(METALAKE_NAME);
+ String catalogName = "catalog";
+ createAndInsertCatalog(METALAKE_NAME, catalogName);
+
+ RoleMetaService roleMetaService = RoleMetaService.getInstance();
+ RoleEntity role =
+ createRoleEntity(
+ RandomIdGenerator.INSTANCE.nextId(),
+ AuthorizationUtils.ofRoleNamespace(METALAKE_NAME),
+ "role",
+ AUDIT_INFO,
+ catalogName);
+ roleMetaService.insertRole(role, false);
+
+ String missingCatalog = "missing_catalog";
+ NoSuchMetadataObjectException exception =
+ Assertions.assertThrows(
+ NoSuchMetadataObjectException.class,
+ () ->
+ roleMetaService.updateRole(
+ role.nameIdentifier(),
+ (RoleEntity current) ->
+ RoleEntity.builder()
+ .withId(current.id())
+ .withName(current.name())
+ .withNamespace(current.namespace())
+ .withProperties(current.properties())
+ .withSecurableObjects(
+ Lists.newArrayList(
+ SecurableObjects.ofCatalog(
+ missingCatalog,
+
Lists.newArrayList(Privileges.UseCatalog.allow()))))
+ .withAuditInfo(current.auditInfo())
+ .build()));
+
+ Assertions.assertEquals(
+ "Metadata object missing_catalog type CATALOG doesn't exist",
exception.getMessage());
+ Assertions.assertInstanceOf(NoSuchEntityException.class,
exception.getCause());
+ Assertions.assertEquals(role,
roleMetaService.getRoleByIdentifier(role.nameIdentifier()));
+ }
+
@TestTemplate
void testConcurrentUpdateDoesNotChangeSecurableObjectsOnConflict() throws
IOException {
createAndInsertMakeLake(METALAKE_NAME);
