pithecuse527 commented on code in PR #13834:
URL: https://github.com/apache/gravitino/pull/13834#discussion_r4430222438
##########
server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinAuthorizer.java:
##########
@@ -3814,6 +3820,108 @@ public CapabilityResult caseSensitiveOnName(Scope
scope) {
}
}
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ public void
testMetalakeDropEvictsEveryMetadataTypeAndKeepsOtherMetalake(boolean peerReplay)
+ throws Exception {
+ // Use a dedicated metalake so the shared MetadataIdConverter stubs do not
leak into other
+ // tests. The other metalake name extends the dropped one to check the
name boundary.
+ String metalakeName = peerReplay ? "peer_drop_ml" : "local_drop_ml";
+ String otherMetalake = metalakeName + "_other";
+ GravitinoCache<String, Long> metadataIdCache =
getMetadataIdCache(jcasbinAuthorizer);
+ JcasbinAuthorizationLookups lookups =
+ new JcasbinAuthorizationLookups(metadataIdCache,
getOwnerRelCache(jcasbinAuthorizer));
+ MetadataObject.Type[] types = MetadataObject.Type.values();
+ for (int i = 0; i < types.length; i++) {
+ MetadataObject object = metadataObjectOfType(metalakeName, types[i]);
+ MetadataObject otherObject = metadataObjectOfType(otherMetalake,
types[i]);
+ metadataIdConverterMockedStatic
+ .when(() -> MetadataIdConverter.getID(object, metalakeName))
+ .thenReturn(Optional.of(1000L + i), Optional.of(2000L + i));
+ metadataIdConverterMockedStatic
+ .when(() -> MetadataIdConverter.getID(otherObject, otherMetalake))
+ .thenReturn(Optional.of(3000L + i));
+
+ assertEquals(Optional.of(1000L + i), resolve(lookups, object,
metalakeName));
+ assertEquals(Optional.of(3000L + i), resolve(lookups, otherObject,
otherMetalake));
+ assertEquals(
+ Optional.of(1000L + i),
+ metadataIdCache.getIfPresent(
+ JcasbinAuthorizationCacheKeys.metadataIdCacheKey(metalakeName,
object)),
+ types[i].name());
+ }
+
+ metalakeDropInvalidation(metalakeName, peerReplay).run();
+
+ for (int i = 0; i < types.length; i++) {
+ MetadataObject object = metadataObjectOfType(metalakeName, types[i]);
+ MetadataObject otherObject = metadataObjectOfType(otherMetalake,
types[i]);
+ assertEquals(
+ Optional.empty(),
+ metadataIdCache.getIfPresent(
+ JcasbinAuthorizationCacheKeys.metadataIdCacheKey(metalakeName,
object)),
+ types[i].name());
+ assertEquals(
+ Optional.of(3000L + i),
+ metadataIdCache.getIfPresent(
+ JcasbinAuthorizationCacheKeys.metadataIdCacheKey(otherMetalake,
otherObject)),
+ types[i].name());
+ // The recreated metalake resolves same-named objects to their new IDs.
+ assertEquals(Optional.of(2000L + i), resolve(lookups, object,
metalakeName));
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ public void
testRecreatedMetalakeDoesNotInheritOldOwnershipOrPermissions(boolean peerReplay)
+ throws Exception {
+ // Use a dedicated metalake so the shared mapper and MetadataIdConverter
stubs do not leak into
+ // other tests.
+ String metalakeName = peerReplay ? "peer_recreate_ml" :
"local_recreate_ml";
+ String roleName = "recreated_role";
+ long oldCatalogId = 7001L;
+ long oldUserId = 7002L;
+ long oldRoleId = 7003L;
+ long newCatalogId = 8001L;
+ long newUserId = 8002L;
+ long newRoleId = 8003L;
+ long otherCatalogId = 8004L;
+ long otherOwnerId = 9999L;
+ Principal principal = PrincipalUtils.getCurrentPrincipal();
+ MetadataObject catalog = MetadataObjects.parse("cat",
MetadataObject.Type.CATALOG);
+ MetadataObject otherCatalog = MetadataObjects.parse("other_cat",
MetadataObject.Type.CATALOG);
+ metadataIdConverterMockedStatic
+ .when(() -> MetadataIdConverter.getID(catalog, metalakeName))
+ .thenReturn(Optional.of(oldCatalogId), Optional.of(newCatalogId));
+ metadataIdConverterMockedStatic
+ .when(() -> MetadataIdConverter.getID(otherCatalog, metalakeName))
+ .thenReturn(Optional.of(otherCatalogId));
+
when(ownerMetaMapper.selectOwnerByMetadataObjectIdAndType(eq(oldCatalogId),
eq("CATALOG")))
+ .thenReturn(new OwnerInfo(oldUserId, "USER"));
+
when(ownerMetaMapper.selectOwnerByMetadataObjectIdAndType(eq(newCatalogId),
eq("CATALOG")))
+ .thenReturn(new OwnerInfo(otherOwnerId, "USER"));
+
when(ownerMetaMapper.selectOwnerByMetadataObjectIdAndType(eq(otherCatalogId),
eq("CATALOG")))
+ .thenReturn(new OwnerInfo(newUserId, "USER"));
+
+ // Old metalake: the user owns "cat" and holds a role that grants
USE_CATALOG on it.
+ mockMetalakeUserWithRole(metalakeName, oldUserId, oldRoleId, roleName,
"cat");
+ assertTrue(authorizeUseCatalog(principal, metalakeName, catalog));
+ assertTrue(isOwner(principal, metalakeName, catalog));
+
+ // Drop the metalake, then recreate the same user, role and catalog names
with new IDs. The
+ // new role grants USE_CATALOG on "other_cat" instead, and someone else
owns "cat".
+ metalakeDropInvalidation(metalakeName, peerReplay).run();
+ mockedRoleVersions.remove(oldRoleId);
+ mockMetalakeUserWithRole(metalakeName, newUserId, newRoleId, roleName,
"other_cat");
Review Comment:
resolved this with a group variant of the recreate test
now, the role is bound to a group, and the group is recreated with a new
groupId, so the loadGroupRoles guard is covered as well
--
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]
