dimas-b commented on code in PR #4526:
URL: https://github.com/apache/polaris/pull/4526#discussion_r3312911816
##########
persistence/nosql/persistence/metastore-maintenance/src/test/java/org/apache/polaris/persistence/nosql/metastore/maintenance/TestCatalogMaintenance.java:
##########
@@ -261,20 +268,428 @@ public void catalogMaintenance() {
checkEntities("real state after grace", entities);
}
+ @Test
+ public void maintenanceRemovesStalePolicyMappings() {
+ var testSetup = bootstrapRealm();
+ var manager = testSetup.manager();
+ var callCtx = testSetup.callCtx();
+ var persistence = testSetup.persistence();
+
+ var catalog = createCatalog(manager, callCtx, persistence);
+ mandatoryCatalogObjsForTestImpl(persistence, catalog.getId());
+ var namespace = createNamespace(manager, callCtx, catalog, persistence,
"policy-ns");
+ var policy =
+ createPolicy(
+ manager,
+ callCtx,
+ catalog,
+ namespace,
+ persistence,
+ "cleanup-policy",
+ PredefinedPolicyTypes.DATA_COMPACTION);
+ var tables =
+ createTables(
+ manager,
+ callCtx,
+ catalog,
+ namespace,
+ persistence,
+ CatalogRetainedIdentifier.STALE_CLEANUP_BATCH_SIZE + 1,
+ "policy-table-");
+
+ for (var table : tables) {
+ assertThat(
+ manager.attachPolicyToEntity(
+ callCtx,
+ List.of(catalog, namespace),
+ table,
+ List.of(catalog, namespace),
+ policy,
+ Map.of()))
+ .extracting(BaseResult::isSuccess)
+ .isEqualTo(true);
+ }
+
+ assertThat(countPolicyMappingsForPolicy(persistence,
policy.getCatalogId(), policy.getId()))
+ .isEqualTo(2L * tables.size());
+
+ assertThat(manager.dropEntityIfExists(callCtx, List.of(catalog,
namespace), policy, null, true))
+ .extracting(BaseResult::isSuccess)
+ .isEqualTo(true);
+
+ LoadPolicyMappingsResult staleMappings =
+ manager.loadPoliciesOnEntity(callCtx, tables.getFirst());
+ assertThat(staleMappings.isSuccess()).isTrue();
+ assertThat(staleMappings.getPolicyMappingRecords()).hasSize(1);
+ assertThat(staleMappings.getEntities()).isEmpty();
+
+ assertThat(runMaintenance().success()).isTrue();
+
+ purgeBackendCache("");
+
+ assertThat(countPolicyMappingsForPolicy(persistence,
policy.getCatalogId(), policy.getId()))
+ .isZero();
+ var cleanedMappings = manager.loadPoliciesOnEntity(callCtx,
tables.getFirst());
+ assertThat(cleanedMappings.isSuccess()).isTrue();
+ assertThat(cleanedMappings.getPolicyMappingRecords()).isEmpty();
+ assertThat(cleanedMappings.getEntities()).isEmpty();
+ }
+
+ @Test
+ public void maintenanceRemovesStalePolicyMappingsForDeletedTargets() {
+ var testSetup = bootstrapRealm();
+ var manager = testSetup.manager();
+ var callCtx = testSetup.callCtx();
+ var persistence = testSetup.persistence();
+
+ var catalog = createCatalog(manager, callCtx, persistence);
+ mandatoryCatalogObjsForTestImpl(persistence, catalog.getId());
+ var namespace = createNamespace(manager, callCtx, catalog, persistence,
"policy-target-ns");
+ var policy =
+ createPolicy(
+ manager,
+ callCtx,
+ catalog,
+ namespace,
+ persistence,
+ "cleanup-target-policy",
+ PredefinedPolicyTypes.DATA_COMPACTION);
+ var staleTables =
+ createTables(
+ manager,
+ callCtx,
+ catalog,
+ namespace,
+ persistence,
+ CatalogRetainedIdentifier.STALE_CLEANUP_BATCH_SIZE + 1,
+ "stale-policy-table-");
+ var liveTable =
+ createTable(manager, callCtx, catalog, namespace, persistence,
"live-policy-table");
+
+ for (var table : staleTables) {
+ assertThat(
+ manager.attachPolicyToEntity(
+ callCtx,
+ List.of(catalog, namespace),
+ table,
+ List.of(catalog, namespace),
+ policy,
+ Map.of()))
+ .extracting(BaseResult::isSuccess)
+ .isEqualTo(true);
+ }
+ assertThat(
+ manager.attachPolicyToEntity(
+ callCtx,
+ List.of(catalog, namespace),
+ liveTable,
+ List.of(catalog, namespace),
+ policy,
+ Map.of()))
+ .extracting(BaseResult::isSuccess)
+ .isEqualTo(true);
+
+ assertThat(countPolicyMappingsForPolicy(persistence,
policy.getCatalogId(), policy.getId()))
+ .isEqualTo(2L * (staleTables.size() + 1L));
+
+ for (var table : staleTables) {
+ assertThat(
+ manager.dropEntityIfExists(callCtx, List.of(catalog, namespace),
table, null, false))
+ .extracting(BaseResult::isSuccess)
+ .isEqualTo(true);
+ }
+
+ assertThat(countPolicyMappingsForPolicy(persistence,
policy.getCatalogId(), policy.getId()))
+ .isEqualTo(2L * (staleTables.size() + 1L));
+ var liveMappingsBeforeCleanup = manager.loadPoliciesOnEntity(callCtx,
liveTable);
+ assertThat(liveMappingsBeforeCleanup.isSuccess()).isTrue();
+ assertThat(liveMappingsBeforeCleanup.getPolicyMappingRecords())
+ .singleElement()
+ .satisfies(
+ mapping -> {
+
assertThat(mapping.getTargetCatalogId()).isEqualTo(liveTable.getCatalogId());
+ assertThat(mapping.getTargetId()).isEqualTo(liveTable.getId());
+ });
+
+ assertThat(runMaintenance().success()).isTrue();
+
+ purgeBackendCache("");
+
+ assertThat(countPolicyMappingsForPolicy(persistence,
policy.getCatalogId(), policy.getId()))
+ .isEqualTo(2L);
+ var liveMappingsAfterCleanup = manager.loadPoliciesOnEntity(callCtx,
liveTable);
+ assertThat(liveMappingsAfterCleanup.isSuccess()).isTrue();
+ assertThat(liveMappingsAfterCleanup.getPolicyMappingRecords())
+ .singleElement()
+ .satisfies(
+ mapping -> {
+
assertThat(mapping.getTargetCatalogId()).isEqualTo(liveTable.getCatalogId());
+ assertThat(mapping.getTargetId()).isEqualTo(liveTable.getId());
+ });
+ }
+
+ @Test
+ public void maintenanceRemovesStaleGrantRecords() {
+ var testSetup = bootstrapRealm();
+ var manager = testSetup.manager();
+ var callCtx = testSetup.callCtx();
+ var persistence = testSetup.persistence();
+
+ var catalog = createCatalog(manager, callCtx, persistence);
+ mandatoryCatalogObjsForTestImpl(persistence, catalog.getId());
+ var namespace = createNamespace(manager, callCtx, catalog, persistence,
"grant-ns");
+ var catalogRole = createCatalogRole(manager, callCtx, catalog,
persistence);
+ var tables =
+ createTables(
+ manager,
+ callCtx,
+ catalog,
+ namespace,
+ persistence,
+ CatalogRetainedIdentifier.STALE_CLEANUP_BATCH_SIZE + 1,
+ "grant-table-");
+
+ for (var table : tables) {
+ assertThat(
+ manager.grantPrivilegeOnSecurableToRole(
+ callCtx,
+ catalogRole,
+ List.of(catalog, namespace),
+ table,
+ PolarisPrivilege.TABLE_READ_DATA))
+ .extracting(BaseResult::isSuccess)
+ .isEqualTo(true);
+ }
+
+ var staleAclNames = new ArrayList<String>();
+ staleAclNames.add(grantAclName(catalogRole));
+ tables.forEach(table -> staleAclNames.add(grantAclName(table)));
+ assertThat(countGrantAclHeads(persistence,
staleAclNames)).isEqualTo(tables.size() + 1L);
+
+ assertThat(manager.dropEntityIfExists(callCtx, List.of(catalog),
catalogRole, null, false))
+ .extracting(BaseResult::isSuccess)
+ .isEqualTo(true);
+
+ assertThat(countGrantAclHeads(persistence,
staleAclNames)).isEqualTo(tables.size() + 1L);
+
+ var staleGrants = manager.loadGrantsOnSecurable(callCtx,
tables.getFirst());
+ assertThat(staleGrants.isSuccess()).isTrue();
+ assertThat(staleGrants.getGrantRecords()).isEmpty();
+ assertThat(staleGrants.getEntities()).isEmpty();
+
+ assertThat(runMaintenance().success()).isTrue();
+
+ purgeBackendCache("");
+
+ assertThat(countGrantAclHeads(persistence, staleAclNames)).isZero();
+
+ var cleanedGrants = manager.loadGrantsOnSecurable(callCtx,
tables.getFirst());
+ assertThat(cleanedGrants.isSuccess()).isTrue();
+ assertThat(cleanedGrants.getGrantRecords()).isEmpty();
+ assertThat(cleanedGrants.getEntities()).isEmpty();
+ }
+
+ @Test
+ public void maintenanceRewritesGrantAclWhenOnlySomeEntriesAreStale() {
+ var testSetup = bootstrapRealm();
+ var manager = testSetup.manager();
+ var callCtx = testSetup.callCtx();
+ var persistence = testSetup.persistence();
+
+ var catalog = createCatalog(manager, callCtx, persistence);
+ mandatoryCatalogObjsForTestImpl(persistence, catalog.getId());
+ var namespace = createNamespace(manager, callCtx, catalog, persistence,
"partial-grant-ns");
+ var table =
+ createTable(manager, callCtx, catalog, namespace, persistence,
"partial-grant-table");
+ var liveRole = createCatalogRole(manager, callCtx, catalog, persistence,
"live-role");
+ var staleRoles =
+ createCatalogRoles(
+ manager,
+ callCtx,
+ catalog,
+ persistence,
+ CatalogRetainedIdentifier.STALE_CLEANUP_BATCH_SIZE + 1,
+ "stale-role-");
+
+ assertThat(
+ manager.grantPrivilegeOnSecurableToRole(
+ callCtx,
+ liveRole,
+ List.of(catalog, namespace),
+ table,
+ PolarisPrivilege.TABLE_READ_DATA))
+ .extracting(BaseResult::isSuccess)
+ .isEqualTo(true);
+ for (var staleRole : staleRoles) {
+ assertThat(
+ manager.grantPrivilegeOnSecurableToRole(
+ callCtx,
+ staleRole,
+ List.of(catalog, namespace),
+ table,
+ PolarisPrivilege.TABLE_READ_DATA))
+ .extracting(BaseResult::isSuccess)
+ .isEqualTo(true);
+ }
+
+ var aclNames = new ArrayList<String>();
+ aclNames.add(grantAclName(table));
+ aclNames.add(grantAclName(liveRole));
+ staleRoles.forEach(role -> aclNames.add(grantAclName(role)));
+ assertThat(countGrantAclHeads(persistence,
aclNames)).isEqualTo(staleRoles.size() + 2L);
+ assertThat(grantAclRoleIds(persistence,
grantAclName(table))).hasSize(staleRoles.size() + 1);
+
+ for (var staleRole : staleRoles) {
+ assertThat(manager.dropEntityIfExists(callCtx, List.of(catalog),
staleRole, null, false))
+ .extracting(BaseResult::isSuccess)
+ .isEqualTo(true);
+ }
+
+ var staleGrants = manager.loadGrantsOnSecurable(callCtx, table);
+ assertThat(staleGrants.isSuccess()).isTrue();
+ assertThat(staleGrants.getGrantRecords()).hasSize(1);
+ assertThat(staleGrants.getEntities()).hasSize(1);
Review Comment:
nit: this data appears to be the same before/after `runMaintenance()` so I'm
not sure what makes these grants "stale" 🤔
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
