You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@polaris.apache.org by ad...@apache.org on 2026/04/22 13:46:55 UTC
(polaris) branch main updated: NoSQL: filter ACL entries by direction in loadGrants (#4222)
This is an automated email from the ASF dual-hosted git repository.
adutra pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/polaris.git
The following commit(s) were added to refs/heads/main by this push:
new 5478b68fa0 NoSQL: filter ACL entries by direction in loadGrants (#4222)
5478b68fa0 is described below
commit 5478b68fa0024533af56ae1b472d562f1a989a41
Author: Alexandre Dutra <ad...@apache.org>
AuthorDate: Wed Apr 22 15:46:50 2026 +0200
NoSQL: filter ACL entries by direction in loadGrants (#4222)
Fixes #4219.
The NoSQL persistence layer stores both directional entry flavors in a single ACL document: reverse ('r/') entries where the entity is the grantee, and directed ('d/') entries where it is the securable. When loading grants for a specific perspective (grantee or securable), both flavors were returned, causing entities like `catalog_admin` — which appear as both grantee and securable — to produce incorrect grant records and trigger "Unexpected entity type 'CATALOG_ROLE'" errors.
---
.../nosql/metastore/NoSqlMetaStore.java | 29 ++++--
.../BasePolarisMetaStoreManagerTest.java | 9 ++
.../persistence/PolarisTestMetaStoreManager.java | 102 +++++++++++++++++++++
3 files changed, 134 insertions(+), 6 deletions(-)
diff --git a/persistence/nosql/persistence/metastore/src/main/java/org/apache/polaris/persistence/nosql/metastore/NoSqlMetaStore.java b/persistence/nosql/persistence/metastore/src/main/java/org/apache/polaris/persistence/nosql/metastore/NoSqlMetaStore.java
index eafb82f05b..bacb404f2c 100644
--- a/persistence/nosql/persistence/metastore/src/main/java/org/apache/polaris/persistence/nosql/metastore/NoSqlMetaStore.java
+++ b/persistence/nosql/persistence/metastore/src/main/java/org/apache/polaris/persistence/nosql/metastore/NoSqlMetaStore.java
@@ -1162,11 +1162,17 @@ class NoSqlMetaStore extends NonFunctionalBasePersistence {
var collector = new GrantRecordsCollector();
var entities = new ArrayList<PolarisBaseEntity>();
var ids = new HashSet<Long>();
+ // ACLs for a given entity contain two directional entry flavors:
+ // - reverse ('r/...'): grants where that entity is the grantee
+ // - directed ('d/...'): grants where that entity is the securable
+ // Filter by direction so callers receive only the requested perspective.
+ Predicate<GrantTriplet> entryFilter = GrantTriplet::reverseOrKey;
+ entryFilter = onSecurable ? entryFilter.negate() : entryFilter;
collectGrantRecords(
catalogId,
aclName,
- ((securableAndGrantee, granted) -> {
+ (securableAndGrantee, granted) -> {
var targetCatalogId =
onSecurable
? securableAndGrantee.granteeCatalogId()
@@ -1210,7 +1216,8 @@ class NoSqlMetaStore extends NonFunctionalBasePersistence {
} else {
LOGGER.trace(" Not returning stale entity reference");
}
- }));
+ },
+ entryFilter);
LOGGER.trace(
"Returning {} grant records for loadGrants for catalog:{}, id:{}, entityType:{}({})",
@@ -1254,7 +1261,8 @@ class NoSqlMetaStore extends NonFunctionalBasePersistence {
collector.handle(securableAndGrantee, granted);
},
- securablesIndex);
+ securablesIndex,
+ triplet -> true);
return collector.grantRecords;
}
@@ -1264,12 +1272,16 @@ class NoSqlMetaStore extends NonFunctionalBasePersistence {
}
private void collectGrantRecords(
- long catalogStableId, String aclName, AclEntryHandler aclEntryConsumer) {
+ long catalogStableId,
+ String aclName,
+ AclEntryHandler aclEntryConsumer,
+ Predicate<GrantTriplet> tripletFilter) {
LOGGER.debug("Checking ACL '{}'", aclName);
var securablesIndex = memoizedIndexedAccess.grantsIndex();
if (securablesIndex.isPresent()) {
- collectGrantRecords(catalogStableId, aclName, aclEntryConsumer, securablesIndex.get());
+ collectGrantRecords(
+ catalogStableId, aclName, aclEntryConsumer, securablesIndex.get(), tripletFilter);
} else {
LOGGER.trace("ACL {} does not exist", aclName);
}
@@ -1279,7 +1291,8 @@ class NoSqlMetaStore extends NonFunctionalBasePersistence {
long catalogStableId,
String aclName,
AclEntryHandler aclEntryConsumer,
- Index<ObjRef> securablesIndex) {
+ Index<ObjRef> securablesIndex,
+ Predicate<GrantTriplet> tripletFilter) {
var securableKey = IndexKey.key(aclName);
LOGGER.trace("Processing existing ACL {}", aclName);
@@ -1292,6 +1305,10 @@ class NoSqlMetaStore extends NonFunctionalBasePersistence {
acl.forEach(
(role, entry) -> {
var triplet = GrantTriplet.fromRoleName(role);
+ if (!tripletFilter.test(triplet)) {
+ LOGGER.trace(" Skipping ACL entry role '{}' due to direction filter", role);
+ return;
+ }
LOGGER
.atTrace()
.setMessage(" ACL has securable {} ({}) with privileges {}")
diff --git a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java
index c40449cf19..e94168d04e 100644
--- a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java
+++ b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java
@@ -295,6 +295,15 @@ public abstract class BasePolarisMetaStoreManagerTest {
polarisTestMetaStoreManager.testLoadResolvedEntitySkipsDroppedGranteeReferences();
}
+ /**
+ * Test that loadGrantsToGrantee/loadGrantsOnSecurable return only grants where the entity plays
+ * the expected role — regression test for entities that are both grantee and securable.
+ */
+ @Test
+ protected void testLoadGrantsGranteeVsSecurableRecords() {
+ polarisTestMetaStoreManager.testLoadGrantsGranteeVsSecurableRecords();
+ }
+
/** Test the set of functions for the entity cache */
@Test
protected void testEntityCache() {
diff --git a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java
index 60ab96de2b..a9d7474074 100644
--- a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java
+++ b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java
@@ -2963,6 +2963,108 @@ public class PolarisTestMetaStoreManager {
Assertions.assertThat(grantRecord.getGranteeId()).isEqualTo(principalRoleId));
}
+ /**
+ * Verify that loadGrantsToGrantee and loadGrantsOnSecurable return only grants where the entity
+ * plays the expected role (grantee or securable).
+ */
+ public void testLoadGrantsGranteeVsSecurableRecords() {
+ // create a catalog — this also creates the catalog_admin role, which is automatically
+ // both a grantee (it has grants on the catalog) and a securable (it gets granted to the
+ // catalog owner).
+ PolarisBaseEntity catalog =
+ new PolarisBaseEntity(
+ PolarisEntityConstants.getNullId(),
+ polarisMetaStoreManager.generateNewEntityId(this.polarisCallContext).getId(),
+ PolarisEntityType.CATALOG,
+ PolarisEntitySubType.NULL_SUBTYPE,
+ PolarisEntityConstants.getRootEntityId(),
+ "load_grants_test");
+ CreateCatalogResult catalogCreated =
+ polarisMetaStoreManager.createCatalog(this.polarisCallContext, catalog, List.of());
+ Assertions.assertThat(catalogCreated).isNotNull();
+ catalog = catalogCreated.getCatalog();
+
+ EntityResult catalogAdminResult =
+ polarisMetaStoreManager.readEntityByName(
+ this.polarisCallContext,
+ List.of(catalog),
+ PolarisEntityType.CATALOG_ROLE,
+ PolarisEntitySubType.ANY_SUBTYPE,
+ PolarisEntityConstants.getNameOfCatalogAdminRole());
+ Assertions.assertThat(catalogAdminResult.isSuccess()).isTrue();
+ PolarisBaseEntity catalogAdmin = catalogAdminResult.getEntity();
+
+ // none of the returned entities should be a CATALOG_ROLE — the securables should be
+ // catalogs, namespaces, etc., not the role itself
+ LoadGrantsResult catalogAdminAsGrantee =
+ polarisMetaStoreManager.loadGrantsToGrantee(this.polarisCallContext, catalogAdmin);
+ Assertions.assertThat(catalogAdminAsGrantee.isSuccess()).isTrue();
+ Assertions.assertThat(catalogAdminAsGrantee.getGrantRecords())
+ .isNotEmpty()
+ .allSatisfy(
+ g -> {
+ Assertions.assertThat(g.getGranteeCatalogId()).isEqualTo(catalogAdmin.getCatalogId());
+ Assertions.assertThat(g.getGranteeId()).isEqualTo(catalogAdmin.getId());
+ });
+ Assertions.assertThat(catalogAdminAsGrantee.getEntities())
+ .allSatisfy(
+ entity ->
+ Assertions.assertThat(entity.getType())
+ .isNotEqualTo(PolarisEntityType.CATALOG_ROLE));
+
+ // now set up an explicit scenario to validate both directions thoroughly:
+ // create a namespace and a catalog role CR1 with grants in both directions
+ PolarisBaseEntity N1 = createEntity(List.of(catalog), PolarisEntityType.NAMESPACE, "N1");
+ PolarisBaseEntity CR1 = createEntity(List.of(catalog), PolarisEntityType.CATALOG_ROLE, "CR1");
+
+ // grant a privilege TO CR1 on N1 (CR1 is the grantee)
+ grantPrivilege(CR1, List.of(catalog, N1), N1, PolarisPrivilege.TABLE_READ_DATA);
+
+ // create a principal role and grant CR1 usage to it (CR1 is the securable)
+ PolarisBaseEntity PR1 = createEntity(null, PolarisEntityType.PRINCIPAL_ROLE, "PR1");
+ grantToGrantee(catalog, CR1, PR1, PolarisPrivilege.CATALOG_ROLE_USAGE);
+
+ // loadGrantsToGrantee(CR1): all records must have granteeId == CR1
+ LoadGrantsResult granteeResult =
+ polarisMetaStoreManager.loadGrantsToGrantee(this.polarisCallContext, CR1);
+ Assertions.assertThat(granteeResult.getGrantRecords())
+ .isNotEmpty()
+ .allSatisfy(
+ g -> {
+ Assertions.assertThat(g.getGranteeId()).isEqualTo(CR1.getId());
+ Assertions.assertThat(g.getSecurableId()).isNotEqualTo(CR1.getId());
+ });
+ Assertions.assertThat(granteeResult.getGrantRecords())
+ .anySatisfy(
+ g -> {
+ Assertions.assertThat(g.getSecurableId()).isEqualTo(N1.getId());
+ Assertions.assertThat(g.getPrivilegeCode())
+ .isEqualTo(PolarisPrivilege.TABLE_READ_DATA.getCode());
+ });
+
+ // loadGrantsOnSecurable(CR1): all records must have securableId == CR1
+ LoadGrantsResult securableResult =
+ polarisMetaStoreManager.loadGrantsOnSecurable(this.polarisCallContext, CR1);
+ Assertions.assertThat(securableResult.getGrantRecords())
+ .isNotEmpty()
+ .allSatisfy(
+ g -> {
+ Assertions.assertThat(g.getSecurableId()).isEqualTo(CR1.getId());
+ Assertions.assertThat(g.getGranteeId()).isNotEqualTo(CR1.getId());
+ });
+ Assertions.assertThat(securableResult.getGrantRecords())
+ .anySatisfy(
+ g -> {
+ Assertions.assertThat(g.getGranteeId()).isEqualTo(PR1.getId());
+ Assertions.assertThat(g.getPrivilegeCode())
+ .isEqualTo(PolarisPrivilege.CATALOG_ROLE_USAGE.getCode());
+ });
+
+ // the two result sets must not overlap
+ Assertions.assertThat(granteeResult.getGrantRecords())
+ .doesNotContainAnyElementsOf(securableResult.getGrantRecords());
+ }
+
private static PolarisEntityCore getEntityCore(PolarisBaseEntity entity) {
return new PolarisEntityCore.Builder<>(entity).build();
}