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 a98f018253 [MINOR] fix(auth): Handle missing parent catalog during
metadata id resolution (#12214)
a98f018253 is described below
commit a98f018253d511824eecb4e408fc65f26cd07ed0
Author: jarred0214 <[email protected]>
AuthorDate: Mon Aug 10 20:07:14 2026 +0800
[MINOR] fix(auth): Handle missing parent catalog during metadata id
resolution (#12214)
### What changes were proposed in this pull request?
This PR updates `MetadataIdConverter` to gracefully handle missing
parent metadata during metadata id resolution.
When resolving metadata ids for objects such as filesets, schemas,
tables, or topics, `MetadataIdConverter` normalizes the object
identifier according to the parent catalog's case-sensitivity capability
before looking up the entity id. If that parent catalog no longer
exists, the target metadata cannot be resolved, so the converter now
returns `Optional.empty()` instead of propagating `NotFoundException`.
A unit test was added to cover the missing parent catalog case.
### Why are the changes needed?
Revoking privileges from a role for a metadata object whose parent
catalog no longer exists can fail during authorization expression
evaluation with an internal error.
For example, the failure can look like:
```text
System internal error during authorization - Operation:
revokePrivilegeFromRole
Caused by: org.apache.gravitino.exceptions.NoSuchCatalogException:
Catalog <metalake>.<catalog> does not exist
at org.apache.gravitino.catalog.CatalogManager.loadCatalogInternal
at org.apache.gravitino.catalog.CapabilityHelpers.getCapability
at
org.apache.gravitino.server.authorization.MetadataIdConverter.normalizeCaseSensitive
at org.apache.gravitino.server.authorization.MetadataIdConverter.getID
at
org.apache.gravitino.server.authorization.jcasbin.JcasbinAuthorizationLookups.loadMetadataId
```
This happens before the revoke operation reaches the business logic.
During authorization, the requested metadata object is converted to an
internal metadata id. For child metadata objects, this conversion first
loads the parent catalog to apply the correct case-sensitivity
normalization. If the parent catalog has already been dropped, that
lookup throws `NoSuchCatalogException`, which currently bubbles up
through the authorization expression evaluator as an internal error.
Returning `Optional.empty()` is consistent with the existing contract of
`MetadataIdConverter#getID`: if the metadata object cannot be resolved,
the converter should return empty. A missing parent catalog means the
requested child metadata object cannot exist or be resolved in
Gravitino, so treating it as missing metadata is safer and more accurate
than failing authorization evaluation with an internal exception.
Fix: #12216
### Does this PR introduce any user-facing change?
No API or configuration changes.
### How was this patch tested?
Ran:
```shell
./gradlew :server-common:test \
--tests org.apache.gravitino.server.authorization.TestMetadataIdConverter
\
--tests
org.apache.gravitino.server.authorization.jcasbin.TestJcasbinAuthorizationLookups
\
-PskipITs -PskipDockerTests=false
```
---
.../server/authorization/MetadataIdConverter.java | 10 ++++++--
.../authorization/TestMetadataIdConverter.java | 30 ++++++++++++++++++++++
2 files changed, 38 insertions(+), 2 deletions(-)
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataIdConverter.java
b/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataIdConverter.java
index 5f7a5695a2..220acba819 100644
---
a/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataIdConverter.java
+++
b/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataIdConverter.java
@@ -35,6 +35,7 @@ import org.apache.gravitino.catalog.CapabilityHelpers;
import org.apache.gravitino.catalog.CatalogManager;
import org.apache.gravitino.connector.capability.Capability;
import org.apache.gravitino.exceptions.NoSuchEntityException;
+import org.apache.gravitino.exceptions.NotFoundException;
import org.apache.gravitino.utils.EntityClassMapper;
import org.apache.gravitino.utils.MetadataObjectUtil;
@@ -68,8 +69,13 @@ public class MetadataIdConverter {
MetadataObject.Type metadataType = metadataObject.type();
NameIdentifier ident = MetadataObjectUtil.toEntityIdent(metalake,
metadataObject);
- NameIdentifier normalizedIdent =
- normalizeCaseSensitive(ident,
METADATA_SCOPE_MAPPING.get(metadataType), catalogManager);
+ NameIdentifier normalizedIdent;
+ try {
+ normalizedIdent =
+ normalizeCaseSensitive(ident,
METADATA_SCOPE_MAPPING.get(metadataType), catalogManager);
+ } catch (NotFoundException e) {
+ return Optional.empty();
+ }
Entity.EntityType entityType =
MetadataObjectUtil.toEntityType(metadataType);
diff --git
a/server-common/src/test/java/org/apache/gravitino/server/authorization/TestMetadataIdConverter.java
b/server-common/src/test/java/org/apache/gravitino/server/authorization/TestMetadataIdConverter.java
index 4a11a0a152..3adfa55f02 100644
---
a/server-common/src/test/java/org/apache/gravitino/server/authorization/TestMetadataIdConverter.java
+++
b/server-common/src/test/java/org/apache/gravitino/server/authorization/TestMetadataIdConverter.java
@@ -41,6 +41,7 @@ import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.Namespace;
import org.apache.gravitino.catalog.CatalogManager;
import org.apache.gravitino.connector.capability.Capability;
+import org.apache.gravitino.exceptions.NoSuchCatalogException;
import org.apache.gravitino.file.Fileset;
import org.apache.gravitino.meta.AuditInfo;
import org.apache.gravitino.meta.BaseMetalake;
@@ -200,6 +201,35 @@ public class TestMetadataIdConverter {
}
}
+ @Test
+ void testConvertReturnsEmptyWhenParentCatalogDoesNotExist() throws
IllegalAccessException {
+ CatalogManager mockCatalogManager = mock(CatalogManager.class);
+ Object originalCatalogManager =
+ FieldUtils.readDeclaredField(GravitinoEnv.getInstance(),
"catalogManager", true);
+ Object originalEntityStore =
+ FieldUtils.readDeclaredField(GravitinoEnv.getInstance(),
"entityStore", true);
+
+ FieldUtils.writeDeclaredField(
+ GravitinoEnv.getInstance(), "catalogManager", mockCatalogManager,
true);
+ FieldUtils.writeDeclaredField(GravitinoEnv.getInstance(), "entityStore",
mockStore, true);
+
+ MetadataObject fileset =
+ MetadataObjects.of(
+ ImmutableList.of("missing_catalog", "schema", "fileset"),
MetadataObject.Type.FILESET);
+ when(mockCatalogManager.loadCatalogAndWrap(NameIdentifier.of("metalake",
"missing_catalog")))
+ .thenThrow(
+ new NoSuchCatalogException("Catalog %s does not exist",
"metalake.missing_catalog"));
+
+ try {
+ Assertions.assertEquals(Optional.empty(),
MetadataIdConverter.getID(fileset, "metalake"));
+ } finally {
+ FieldUtils.writeDeclaredField(
+ GravitinoEnv.getInstance(), "catalogManager",
originalCatalogManager, true);
+ FieldUtils.writeDeclaredField(
+ GravitinoEnv.getInstance(), "entityStore", originalEntityStore,
true);
+ }
+ }
+
private void initTestNameIdentifier() {
ident1 = NameIdentifier.of("metalake");
ident2 = NameIdentifier.of("metalake", "catalog");
