validate root container in entity cache grant manager

apache · Dec 19, 2024 · 2ec5473 · 2ec5473
1 parent 8ef6313
commit 2ec5473
Show file tree

Hide file tree

Showing 2 changed files with 92 additions and 14 deletions.
diff --git a/...core/src/main/java/org/apache/polaris/core/persistence/cache/EntityCacheGrantManager.java b/...core/src/main/java/org/apache/polaris/core/persistence/cache/EntityCacheGrantManager.java
@@ -140,21 +140,17 @@ public EntityCacheGrantManager(
         .filter(lr -> lr != null && lr.getCacheEntry() != null)
         .map(lr -> lr.getCacheEntry().getEntity())
         .forEach(granteeList::add);
+    PolarisBaseEntity entity = lookupResult.getCacheEntry().getEntity();
     if (granteeList.size() != lookupResult.getCacheEntry().getGrantRecordsAsSecurable().size()) {
-      LOGGER.error(
-          "Failed to resolve all grantees for securable {}",
-          lookupResult.getCacheEntry().getEntity());
+      LOGGER.error("Failed to resolve all grantees for securable {}", entity);
       return new LoadGrantsResult(BaseResult.ReturnStatus.GRANT_NOT_FOUND, null);
     }
 
     // If the securable is the root container, then we need to add a grant record for the
     // service_admin PrincipalRole, which is the only role that has the SERVICE_MANAGE_ACCESS
     // privilege on the root
-    if (lookupResult
-        .getCacheEntry()
-        .getEntity()
-        .getName()
-        .equals(PolarisEntityConstants.getRootContainerName())) {
+    if (entity.getName().equals(PolarisEntityConstants.getRootContainerName())
+        && entity.getType().equals(PolarisEntityType.ROOT)) {
       if (serviceAdminEntity == null || serviceAdminRootContainerGrant == null) {
         EntityCacheLookupResult serviceAdminRole =
             entityCache.getOrLoadEntityByName(
@@ -177,13 +173,20 @@ public EntityCacheGrantManager(
                 PolarisPrivilege.SERVICE_MANAGE_ACCESS.getCode());
         serviceAdminEntity = serviceAdminRole.getCacheEntry().getEntity();
       }
-      grantRecords.add(serviceAdminRootContainerGrant);
-      granteeList.add(serviceAdminEntity);
+      grantRecords.stream()
+          .filter(gr -> gr.getGranteeId() == serviceAdminEntity.getId())
+          .findFirst()
+          .ifPresentOrElse(
+              gr -> {
+                LOGGER.trace("service_admin PrincipalRole already has a grant record on the root");
+              },
+              () -> {
+                LOGGER.trace("Adding service_admin PrincipalRole grant record on the root");
+                grantRecords.add(serviceAdminRootContainerGrant);
+                granteeList.add(serviceAdminEntity);
+              });
     }
-    return new LoadGrantsResult(
-        lookupResult.getCacheEntry().getEntity().getGrantRecordsVersion(),
-        grantRecords,
-        granteeList);
+    return new LoadGrantsResult(entity.getGrantRecordsVersion(), grantRecords, granteeList);
   }
 
   @Override

diff --git a/...core/src/test/java/org/apache/polaris/core/storage/cache/EntityCacheGrantManagerTest.java b/...core/src/test/java/org/apache/polaris/core/storage/cache/EntityCacheGrantManagerTest.java
@@ -38,7 +38,9 @@
 import org.apache.polaris.core.entity.NamespaceEntity;
 import org.apache.polaris.core.entity.PolarisBaseEntity;
 import org.apache.polaris.core.entity.PolarisEntity;
+import org.apache.polaris.core.entity.PolarisEntityConstants;
 import org.apache.polaris.core.entity.PolarisEntityCore;
+import org.apache.polaris.core.entity.PolarisEntitySubType;
 import org.apache.polaris.core.entity.PolarisEntityType;
 import org.apache.polaris.core.entity.PolarisGrantRecord;
 import org.apache.polaris.core.entity.PolarisPrivilege;
@@ -299,6 +301,79 @@ public void testReadFromResolverPopulatedCache() {
             });
   }
 
+  @Test
+  public void testImplicitRootContainerGrantForServiceAdmin() {
+    Namespace ns2 = Namespace.of(PolarisEntityConstants.getRootContainerName());
+    PolarisMetaStoreManager.EntityResult namespaceEntity =
+        metaStoreManager.createEntityIfNotExists(
+            polarisCallContext,
+            List.of(catalog),
+            new NamespaceEntity.Builder(ns2)
+                .setId(metaStoreManager.generateNewEntityId(polarisCallContext).getId())
+                .setCatalogId(catalog.getId())
+                .setParentId(catalog.getId())
+                .setCreateTimestamp(System.currentTimeMillis())
+                .setEntityVersion(1)
+                .build());
+    EntityCache entityCache = new EntityCache(metaStoreManager);
+    PolarisResolutionManifest manifest =
+        new PolarisResolutionManifest(
+            CallContext.of(realmContext, polarisCallContext),
+            new PolarisEntityManager(metaStoreManager, new StorageCredentialCache(), entityCache),
+            new AuthenticatedPolarisPrincipal(
+                principal, Set.of(principalRole1.getName(), principalRole2.getName())),
+            catalog.getName());
+    manifest.addPath(
+        new ResolverPath(
+            List.of(PolarisEntityConstants.getRootContainerName()), PolarisEntityType.NAMESPACE),
+        ns2);
+
+    ResolverStatus resolverStatus = manifest.resolveAll();
+    Assertions.assertThat(resolverStatus.getStatus()).isEqualTo(ResolverStatus.StatusEnum.SUCCESS);
+
+    // now create an EntityCacheGrantManager with a dummy grantManager implementation.
+    // everything should be returned from the cache
+    EntityCacheGrantManager grantManager = new EntityCacheGrantManager(Mockito.mock(), entityCache);
+
+    PolarisBaseEntity serviceAdmin =
+        metaStoreManager
+            .readEntityByName(
+                polarisCallContext,
+                null,
+                PolarisEntityType.PRINCIPAL_ROLE,
+                PolarisEntitySubType.NULL_SUBTYPE,
+                PolarisEntityConstants.getNameOfPrincipalServiceAdminRole())
+            .getEntity();
+    PolarisGrantManager.LoadGrantsResult grantsResult =
+        grantManager.loadGrantsOnSecurable(
+            polarisCallContext, 0L, PolarisEntityConstants.getRootEntityId());
+    Assertions.assertThat(grantsResult)
+        .returns(true, PolarisGrantManager.LoadGrantsResult::isSuccess)
+        .extracting(PolarisGrantManager.LoadGrantsResult::getGrantRecords)
+        .asInstanceOf(InstanceOfAssertFactories.list(PolarisGrantRecord.class))
+        .hasSize(1)
+        .satisfiesExactly(
+            grant -> {
+              Assertions.assertThat(grant)
+                  .returns(serviceAdmin.getId(), PolarisGrantRecord::getGranteeId)
+                  .returns(
+                      PolarisPrivilege.SERVICE_MANAGE_ACCESS.getCode(),
+                      PolarisGrantRecord::getPrivilegeCode);
+            });
+
+    PolarisResolvedPathWrapper resolvedTable = manifest.getResolvedPath(ns2, false);
+    PolarisGrantManager.LoadGrantsResult nsGrantsResult =
+        grantManager.loadGrantsOnSecurable(
+            polarisCallContext,
+            manifest.getResolvedReferenceCatalogEntity().getRawLeafEntity().getId(),
+            resolvedTable.getRawLeafEntity().getId());
+    Assertions.assertThat(nsGrantsResult)
+        .returns(true, PolarisGrantManager.LoadGrantsResult::isSuccess)
+        .extracting(PolarisGrantManager.LoadGrantsResult::getGrantRecords)
+        .asInstanceOf(InstanceOfAssertFactories.list(PolarisGrantRecord.class))
+        .isEmpty();
+  }
+
   @Test
   public void testRevokeGrantsClearsCache() {
     Namespace ns2 = Namespace.of(NS_1, NS_2);