You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@polaris.apache.org by di...@apache.org on 2026/07/15 15:46:58 UTC
(polaris) branch main updated: Eliminate redundant metastore lookups when resolving principal roles (#5020)
This is an automated email from the ASF dual-hosted git repository.
dimas-b 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 ee8df5feb4 Eliminate redundant metastore lookups when resolving principal roles (#5020)
ee8df5feb4 is described below
commit ee8df5feb45d4c3f436e28e34a6175bfb562d6ef
Author: Ayush Saxena <ay...@apache.org>
AuthorDate: Wed Jul 15 21:16:54 2026 +0530
Eliminate redundant metastore lookups when resolving principal roles (#5020)
---
.../polaris/service/auth/DefaultAuthenticator.java | 39 +++++++++---
.../service/auth/DefaultAuthenticatorTest.java | 74 ++++++++++++++++++++++
2 files changed, 103 insertions(+), 10 deletions(-)
diff --git a/runtime/service/src/main/java/org/apache/polaris/service/auth/DefaultAuthenticator.java b/runtime/service/src/main/java/org/apache/polaris/service/auth/DefaultAuthenticator.java
index 74867054ff..5cd01cea62 100644
--- a/runtime/service/src/main/java/org/apache/polaris/service/auth/DefaultAuthenticator.java
+++ b/runtime/service/src/main/java/org/apache/polaris/service/auth/DefaultAuthenticator.java
@@ -22,6 +22,8 @@ import com.google.common.base.Throwables;
import io.smallrye.common.annotation.Identifier;
import jakarta.enterprise.context.RequestScoped;
import jakarta.inject.Inject;
+import java.util.Map;
+import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.function.Predicate;
@@ -32,12 +34,14 @@ import org.apache.polaris.core.PolarisCallContext;
import org.apache.polaris.core.PolarisDiagnostics;
import org.apache.polaris.core.auth.PolarisPrincipal;
import org.apache.polaris.core.context.CallContext;
+import org.apache.polaris.core.entity.PolarisBaseEntity;
import org.apache.polaris.core.entity.PolarisEntityType;
+import org.apache.polaris.core.entity.PolarisGrantRecord;
import org.apache.polaris.core.entity.PrincipalEntity;
import org.apache.polaris.core.entity.PrincipalRoleEntity;
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
-import org.apache.polaris.core.persistence.dao.entity.EntityResult;
import org.apache.polaris.core.persistence.dao.entity.LoadGrantsResult;
+import org.jspecify.annotations.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -167,17 +171,13 @@ public class DefaultAuthenticator implements Authenticator {
Predicate<String> includeRoleFilter =
requestedRoles == ALL_ROLES_REQUESTED ? role -> true : requestedRoles::contains;
+ Map<Long, PolarisBaseEntity> entitiesById = loadGrantsResult.getEntitiesAsMap();
+
Set<String> activeRoles =
loadGrantsResult.getGrantRecords().stream()
- .map(
- gr ->
- metaStoreManager.loadEntity(
- callContext.getPolarisCallContext(),
- gr.getSecurableCatalogId(),
- gr.getSecurableId(),
- PolarisEntityType.PRINCIPAL_ROLE))
- .filter(EntityResult::isSuccess)
- .map(EntityResult::getEntity)
+ .map(gr -> loadSecurableEntity(gr, entitiesById))
+ .filter(Objects::nonNull)
+ .filter(entity -> entity.getType() == PolarisEntityType.PRINCIPAL_ROLE)
.map(PrincipalRoleEntity::of)
.map(PrincipalRoleEntity::getName)
.filter(includeRoleFilter)
@@ -249,4 +249,23 @@ public class DefaultAuthenticator implements Authenticator {
}
return principalGrantResults;
}
+
+ /**
+ * Resolves the securable entity for a grant record, using preloaded entities when available and
+ * falling back to {@link PolarisMetaStoreManager#loadEntity} only when the metastore did not
+ * populate {@link LoadGrantsResult#getEntities()}.
+ */
+ private @Nullable PolarisBaseEntity loadSecurableEntity(
+ PolarisGrantRecord grant, @Nullable Map<Long, PolarisBaseEntity> entitiesById) {
+ if (entitiesById != null) {
+ return entitiesById.get(grant.getSecurableId());
+ }
+ return metaStoreManager
+ .loadEntity(
+ callContext.getPolarisCallContext(),
+ grant.getSecurableCatalogId(),
+ grant.getSecurableId(),
+ PolarisEntityType.PRINCIPAL_ROLE)
+ .getEntity();
+ }
}
diff --git a/runtime/service/src/test/java/org/apache/polaris/service/auth/DefaultAuthenticatorTest.java b/runtime/service/src/test/java/org/apache/polaris/service/auth/DefaultAuthenticatorTest.java
index 95fd04558b..20c00e58e1 100644
--- a/runtime/service/src/test/java/org/apache/polaris/service/auth/DefaultAuthenticatorTest.java
+++ b/runtime/service/src/test/java/org/apache/polaris/service/auth/DefaultAuthenticatorTest.java
@@ -21,6 +21,10 @@ package org.apache.polaris.service.auth;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import io.quarkus.security.identity.CurrentIdentityAssociation;
@@ -31,6 +35,7 @@ import jakarta.inject.Inject;
import java.util.Map;
import java.util.Set;
import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.polaris.core.PolarisDiagnostics;
import org.apache.polaris.core.admin.model.PrincipalWithCredentialsCredentials;
import org.apache.polaris.core.auth.PolarisAuthorizer;
import org.apache.polaris.core.auth.PolarisPrincipal;
@@ -41,6 +46,7 @@ import org.apache.polaris.core.entity.PrincipalEntity;
import org.apache.polaris.core.entity.PrincipalRoleEntity;
import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
+import org.apache.polaris.core.persistence.dao.entity.LoadGrantsResult;
import org.apache.polaris.core.persistence.resolver.ResolutionManifestFactory;
import org.apache.polaris.core.secrets.UserSecretsManager;
import org.apache.polaris.service.admin.PolarisAdminService;
@@ -72,6 +78,7 @@ public class DefaultAuthenticatorTest {
@Inject RealmContextHolder realmContextHolder;
@Inject PolarisMetaStoreManager metaStoreManager;
@Inject CallContext callContext;
+ @Inject PolarisDiagnostics diagnostics;
@Inject ResolutionManifestFactory resolutionManifestFactory;
@Inject UserSecretsManager userSecretsManager;
@Inject ServiceIdentityProvider serviceIdentityProvider;
@@ -299,6 +306,73 @@ public class DefaultAuthenticatorTest {
assertPrincipal(result, principalEntity);
}
+ @Test
+ void testResolvePrincipalRolesUsesEntitiesFromLoadGrantsResult() {
+ LoadGrantsResult grantsResult =
+ metaStoreManager.loadGrantsToGrantee(callContext.getPolarisCallContext(), principalEntity);
+ assertThat(grantsResult.getEntities())
+ .as("loadGrantsToGrantee must populate securable entities")
+ .isNotEmpty()
+ .allMatch(e -> e.getType() == PolarisEntityType.PRINCIPAL_ROLE);
+
+ // Build a standalone DefaultAuthenticator wired to a spied metaStoreManager so we
+ // can verify call counts. Going through the CDI-managed authenticator would give
+ // us a client proxy, and field writes on that proxy do not propagate to the
+ // per-request instance the authenticator actually uses.
+ PolarisMetaStoreManager metaStoreManagerSpy = Mockito.spy(metaStoreManager);
+ DefaultAuthenticator standaloneAuthenticator = new DefaultAuthenticator();
+ standaloneAuthenticator.metaStoreManager = metaStoreManagerSpy;
+ standaloneAuthenticator.callContext = callContext;
+ standaloneAuthenticator.diagnostics = diagnostics;
+
+ PolarisCredential credentials =
+ PolarisCredential.of(null, PRINCIPAL_NAME, Set.of(DefaultAuthenticator.PRINCIPAL_ROLE_ALL));
+
+ PolarisPrincipal result = standaloneAuthenticator.authenticate(credentials);
+
+ assertPrincipal(result, principalEntity, PRINCIPAL_ROLE1, PRINCIPAL_ROLE2);
+
+ // The role entities must have been served from LoadGrantsResult.getEntities(),
+ // not re-fetched via loadEntity(..., PRINCIPAL_ROLE) per grant record.
+ verify(metaStoreManagerSpy, never())
+ .loadEntity(any(), anyLong(), anyLong(), Mockito.eq(PolarisEntityType.PRINCIPAL_ROLE));
+ }
+
+ @Test
+ void testResolvePrincipalRolesFallsBackToLoadEntityWhenEntitiesNotPreloaded() {
+ // Backward-compatibility: older metastore backends may not serialize the entities
+ // list on LoadGrantsResult (see the JSON constructor comment in LoadGrantsResult).
+ // When getEntities() returns null, the authenticator must fall back to per-record
+ // loadEntity so those deployments continue to resolve roles correctly.
+ LoadGrantsResult grants =
+ metaStoreManager.loadGrantsToGrantee(callContext.getPolarisCallContext(), principalEntity);
+ LoadGrantsResult grantsWithoutEntities =
+ new LoadGrantsResult(grants.getGrantsVersion(), grants.getGrantRecords(), null);
+ assertThat(grantsWithoutEntities.getEntitiesAsMap()).isNull();
+
+ PolarisMetaStoreManager metaStoreManagerSpy = Mockito.spy(metaStoreManager);
+ Mockito.doReturn(grantsWithoutEntities)
+ .when(metaStoreManagerSpy)
+ .loadGrantsToGrantee(
+ any(), Mockito.argThat(p -> p != null && p.getId() == principalEntity.getId()));
+
+ DefaultAuthenticator standaloneAuthenticator = new DefaultAuthenticator();
+ standaloneAuthenticator.metaStoreManager = metaStoreManagerSpy;
+ standaloneAuthenticator.callContext = callContext;
+ standaloneAuthenticator.diagnostics = diagnostics;
+
+ PolarisCredential credentials =
+ PolarisCredential.of(null, PRINCIPAL_NAME, Set.of(DefaultAuthenticator.PRINCIPAL_ROLE_ALL));
+
+ PolarisPrincipal result = standaloneAuthenticator.authenticate(credentials);
+
+ // Roles should still resolve — the fallback path must produce the same result.
+ assertPrincipal(result, principalEntity, PRINCIPAL_ROLE1, PRINCIPAL_ROLE2);
+
+ verify(metaStoreManagerSpy, Mockito.times(2))
+ .loadEntity(any(), anyLong(), anyLong(), Mockito.eq(PolarisEntityType.PRINCIPAL_ROLE));
+ }
+
@Test
void testPrincipalIdTakesPrecedenceOverName() {
// Given: credentials with both principal ID and name (ID should take precedence)