roryqi commented on code in PR #12517:
URL: https://github.com/apache/gravitino/pull/12517#discussion_r3820395040
##########
authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java:
##########
@@ -523,9 +535,9 @@ public Boolean onOwnerSet(MetadataObject metadataObject,
Owner preOwner, Owner n
case CATALOG:
// The metalake and catalog use role to manage the owner
if (metadataObject.type() == MetadataObject.Type.METALAKE) {
- ownerRoleName = RangerHelper.GRAVITINO_METALAKE_OWNER_ROLE;
+ ownerRoleName =
RangerHelper.generateMetalakeOwnerRoleName(metalakeId);
} else {
- ownerRoleName = RangerHelper.GRAVITINO_CATALOG_OWNER_ROLE;
+ ownerRoleName = RangerHelper.generateCatalogOwnerRoleName(catalogId);
Review Comment:
Fixed in 603075d33. `updatePolicyOwnerRole` now removes the exact legacy
shared owner role from each matching owner policy item before adding the
entity-specific role. Reapplying the same owner therefore migrates an existing
policy idempotently instead of leaving the shared role active.
##########
core/src/main/java/org/apache/gravitino/connector/BaseCatalog.java:
##########
@@ -280,7 +280,13 @@ public AuthorizationPlugin getAuthorizationPlugin() {
return authorizationPlugin;
}
- public void initAuthorizationPluginInstance(IsolatedClassLoader classLoader)
{
+ /**
+ * Initializes the authorization plugin for this catalog.
+ *
+ * @param classLoader the catalog isolated class loader
+ * @param metalakeId the stable entity ID of the metalake containing this
catalog
+ */
+ public void initAuthorizationPluginInstance(IsolatedClassLoader classLoader,
long metalakeId) {
Review Comment:
This change is for Gravitino 2.0, where this lifecycle signature can change.
A repository-wide search found only `CatalogManager` and internal tests calling
`initAuthorizationPluginInstance`; catalog connector modules do not call it. I
kept the two-argument method so initialization always supplies the stable
metalake ID required by the Ranger role isolation. Custom code compiled against
the old internal lifecycle method will need to rebuild/update for 2.0.
##########
core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java:
##########
@@ -1301,14 +1302,26 @@ private Map<String, String>
getResolvedProperties(CatalogEntity entity) {
}
private BaseCatalog<?> createBaseCatalog(IsolatedClassLoader classLoader,
CatalogEntity entity) {
+ BaseMetalake metalakeEntity;
+ try {
+ metalakeEntity =
+ store.get(
+ NameIdentifier.of(entity.namespace().levels()),
+ EntityType.METALAKE,
+ BaseMetalake.class);
+ } catch (IOException e) {
+ throw new RuntimeException(
+ String.format("Failed to load metalake for catalog %s",
entity.nameIdentifier()), e);
+ }
+
// Load Catalog class instance
BaseCatalog<?> catalog = createCatalogInstance(classLoader,
entity.getProvider());
// Resolve secret URNs to plaintext for connector init only; entity
storage keeps URNs.
// Fileset FS merge assumes catalog conf is already plaintext at this
boundary.
catalog
.withCatalogConf(secretManager.toPlaintextProperties(entity.getProperties()))
.withCatalogEntity(entity);
- catalog.initAuthorizationPluginInstance(classLoader);
+ catalog.initAuthorizationPluginInstance(classLoader, metalakeEntity.id());
Review Comment:
This targets the 2.0 upgrade, so I added an explicit idempotent
reconciliation procedure instead of an automatic startup migration. The
authorization pushdown documentation now instructs administrators to GET each
current metalake/catalog owner and PUT the same owner again. That creates the
ID-specific role, grants the current member, and replaces the legacy role in
matching policies. I also separated revoke and grant error handling so a revoke
failure on a newly created role cannot skip the grant. After every object is
reconciled, administrators verify that no policy references the exact legacy
role names and then delete those roles.
##########
authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java:
##########
@@ -1492,8 +1493,8 @@ public void testOnOwnerSetCatalog() {
null,
null,
Lists.newArrayList(
- RangerHelper.GRAVITINO_METALAKE_OWNER_ROLE,
- RangerHelper.GRAVITINO_CATALOG_OWNER_ROLE));
+
RangerHelper.generateMetalakeOwnerRoleName(RangerITEnv.METALAKE_ID),
+
RangerHelper.generateCatalogOwnerRoleName(RangerITEnv.CATALOG_ID)));
Review Comment:
Added `testMigrateLegacyOwnerRolesForCatalogsSharingRangerService` in
603075d33. It uses two catalog IDs and two different owners against the same
Ranger service, prepares legacy shared role membership and policies, removes
the new roles, and then runs same-owner reconciliation. The test verifies
isolated role membership, replacement of the legacy policy role, and
idempotency. The targeted Ranger Docker test passes.
--
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]
