This is an automated email from the ASF dual-hosted git repository. github-actions[bot] pushed a commit to branch cherry-pick-744a7591-to-branch-1.3 in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit bb0075c27eb27ec5b3e2a9f5c58413f6f6f2ff05 Author: roryqi <[email protected]> AuthorDate: Fri Sep 18 22:12:41 2026 +0800 [#13326] fix(server): Check object before listing roles (#13327) ### What changes were proposed in this pull request? This PR checks the metadata object in the list-roles-by-object REST API before querying role bindings. For table objects, the existing metadata object check goes through `tableExists()`, which can load/import externally managed tables into the entity store before role relations are queried. ### Why are the changes needed? The roles API currently queries role bindings directly through the entity-store relation path. For externally managed tables, such as JDBC tables, the table may exist in the underlying catalog but not yet have a local `table_meta` row until it is loaded. As a result, listing roles for an existing table can return `NoSuchMetadataObjectException` before the table is loaded, then return `200` after loading the same table. Fix: #13326 ### Does this PR introduce _any_ user-facing change? Yes. Listing roles for an existing metadata object now checks/resolves the object first. For externally managed tables that can be loaded from the catalog, this avoids returning a misleading 404 before the table has been imported into the entity store. ### How was this patch tested? Added a REST test covering table-object role listing and verifying that the table object is checked before roles are listed. Ran: `./gradlew spotlessApply` `./gradlew :server:test --tests org.apache.gravitino.server.web.rest.TestMetadataObjectRoleOperations -PskipITs -PskipDockerTests=false` --- .../web/rest/MetadataObjectRoleOperations.java | 2 + .../web/rest/TestMetadataObjectRoleOperations.java | 47 ++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/MetadataObjectRoleOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/MetadataObjectRoleOperations.java index 225a2e5e6b..fc6eebcdac 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/MetadataObjectRoleOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/MetadataObjectRoleOperations.java @@ -39,6 +39,7 @@ import org.apache.gravitino.metrics.MetricNames; import org.apache.gravitino.server.authorization.MetadataAuthzHelper; import org.apache.gravitino.server.authorization.NameBindings; import org.apache.gravitino.server.web.Utils; +import org.apache.gravitino.utils.MetadataObjectUtil; import org.apache.gravitino.utils.NameIdentifierUtil; @NameBindings.AccessControlInterfaces @@ -76,6 +77,7 @@ public class MetadataObjectRoleOperations { httpRequest, () -> { MetalakeManager.checkMetalakeInUse(metalake); + MetadataObjectUtil.checkMetadataObject(metalake, object); String[] names = accessControlDispatcher.listRoleNamesByObject(metalake, object); names = MetadataAuthzHelper.filterByExpression( diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestMetadataObjectRoleOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestMetadataObjectRoleOperations.java index 339953434a..17d33f3807 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestMetadataObjectRoleOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestMetadataObjectRoleOperations.java @@ -35,7 +35,9 @@ import org.apache.commons.lang3.reflect.FieldUtils; import org.apache.gravitino.Config; import org.apache.gravitino.EntityStore; import org.apache.gravitino.GravitinoEnv; +import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.authorization.AccessControlManager; +import org.apache.gravitino.catalog.TableDispatcher; import org.apache.gravitino.connector.PropertiesMetadata; import org.apache.gravitino.dto.responses.ErrorConstants; import org.apache.gravitino.dto.responses.ErrorResponse; @@ -44,6 +46,7 @@ import org.apache.gravitino.exceptions.NoSuchEntityException; import org.apache.gravitino.exceptions.NoSuchMetalakeException; import org.apache.gravitino.lock.LockManager; import org.apache.gravitino.meta.BaseMetalake; +import org.apache.gravitino.metalake.MetalakeDispatcher; import org.apache.gravitino.rest.RESTUtils; import org.glassfish.hk2.utilities.binding.AbstractBinder; import org.glassfish.jersey.server.ResourceConfig; @@ -51,6 +54,7 @@ import org.glassfish.jersey.test.JerseyTest; import org.glassfish.jersey.test.TestProperties; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mockito; @@ -58,6 +62,8 @@ public class TestMetadataObjectRoleOperations extends JerseyTest { private static final AccessControlManager manager = mock(AccessControlManager.class); private static final EntityStore entityStore = mock(EntityStore.class); + private static final MetalakeDispatcher metalakeDispatcher = mock(MetalakeDispatcher.class); + private static final TableDispatcher tableDispatcher = mock(TableDispatcher.class); private static class MockServletRequestFactory extends ServletRequestFactoryBase { @Override @@ -77,6 +83,26 @@ public class TestMetadataObjectRoleOperations extends JerseyTest { FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager", new LockManager(config), true); FieldUtils.writeField(GravitinoEnv.getInstance(), "accessControlDispatcher", manager, true); FieldUtils.writeField(GravitinoEnv.getInstance(), "entityStore", entityStore, true); + FieldUtils.writeField( + GravitinoEnv.getInstance(), "internalMetalakeDispatcher", metalakeDispatcher, true); + FieldUtils.writeField( + GravitinoEnv.getInstance(), "internalTableDispatcher", tableDispatcher, true); + } + + @BeforeEach + public void resetMocks() throws IOException { + Mockito.reset(manager, entityStore, metalakeDispatcher, tableDispatcher); + when(metalakeDispatcher.metalakeExists(any())).thenReturn(true); + when(tableDispatcher.tableExists(any())).thenReturn(true); + mockInUseMetalake(); + } + + private static void mockInUseMetalake() throws IOException { + BaseMetalake metalake = mock(BaseMetalake.class); + PropertiesMetadata propertiesMetadata = mock(PropertiesMetadata.class); + when(propertiesMetadata.getOrDefault(any(), any())).thenReturn(true); + when(metalake.propertiesMetadata()).thenReturn(propertiesMetadata); + when(entityStore.get(any(), any(), any())).thenReturn(metalake); } @Override @@ -160,4 +186,25 @@ public class TestMetadataObjectRoleOperations extends JerseyTest { Assertions.assertEquals(ErrorConstants.INTERNAL_ERROR_CODE, errorResponse2.getCode()); Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResponse2.getType()); } + + @Test + public void testListRoleNamesChecksTableObjectBeforeListing() throws IOException { + when(manager.listRoleNamesByObject(any(), any())).thenReturn(new String[0]); + + Response resp = + target("/metalakes/metalake1/objects/table/catalog1.schema1.table1/roles") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .get(); + + Assertions.assertEquals(Response.Status.OK.getStatusCode(), resp.getStatus()); + + NameListResponse listResponse = resp.readEntity(NameListResponse.class); + Assertions.assertEquals(0, listResponse.getCode()); + Assertions.assertEquals(0, listResponse.getNames().length); + + Mockito.verify(tableDispatcher) + .tableExists(NameIdentifier.of("metalake1", "catalog1", "schema1", "table1")); + Mockito.verify(manager).listRoleNamesByObject(any(), any()); + } }
