Skip to content

Commit df82021

Browse files
Brijeshthummar02Brijeshthummar02
authored
Merge branch 'main' into feature/cli-command-improvements
2 parents 253ae7d + 873c6af commit df82021

File tree

10 files changed

+207
-68
lines changed

10 files changed

+207
-68
lines changed

‎authorizations/authorization-chain/src/main/java/org/apache/gravitino/authorization/chain/ChainedAuthorizationPlugin.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,11 @@ public Boolean onRoleAcquired(Role role) throws AuthorizationPluginException {
111111

112112
@Override
113113
public Boolean onRoleDeleted(Role role) throws AuthorizationPluginException {
114+
onRoleUpdated(
115+
role,
116+
role.securableObjects().stream()
117+
.map(securableObject -> RoleChange.removeSecurableObject(role.name(), securableObject))
118+
.toArray(RoleChange[]::new));
114119
return chainedAction(plugin -> plugin.onRoleDeleted(role));
115120
}
116121

‎authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java

Lines changed: 71 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,8 @@ public abstract class RangerAuthorizationPlugin
7777

7878
protected String metalake;
7979
protected final String rangerServiceName;
80-
protected final RangerClientExtension rangerClient;
81-
protected final RangerHelper rangerHelper;
80+
protected RangerClientExtension rangerClient;
81+
protected RangerHelper rangerHelper;
8282
@VisibleForTesting public final String rangerAdminName;
8383

8484
protected RangerAuthorizationPlugin(String metalake, Map<String, String> config) {
@@ -108,6 +108,26 @@ public String getMetalake() {
108108
return metalake;
109109
}
110110

111+
@VisibleForTesting
112+
public RangerHelper getRangerHelper() {
113+
return rangerHelper;
114+
}
115+
116+
@VisibleForTesting
117+
public void setRangerHelper(RangerHelper rangerHelper) {
118+
this.rangerHelper = rangerHelper;
119+
}
120+
121+
@VisibleForTesting
122+
public RangerClientExtension getRangerClient() {
123+
return rangerClient;
124+
}
125+
126+
@VisibleForTesting
127+
public void setRangerClient(RangerClientExtension rangerClient) {
128+
this.rangerClient = rangerClient;
129+
}
130+
111131
/**
112132
* Set the Ranger policy resource defines rule.
113133
*
@@ -273,8 +293,13 @@ public Boolean onRoleDeleted(Role role) throws AuthorizationPluginException {
273293
rangerClient.deleteRole(
274294
rangerHelper.generateGravitinoRoleName(role.name()), rangerAdminName, rangerServiceName);
275295
} catch (RangerServiceException e) {
276-
// Ignore exception to support idempotent operation
277-
LOG.warn("Ranger delete role: {} failed!", role, e);
296+
if (rangerHelper.getRangerRole(role.name()) == null) {
297+
// Ignore exception to support idempotent operation
298+
LOG.info("Ranger delete role: {} failed!", role, e);
299+
} else {
300+
throw new AuthorizationPluginException(
301+
"Fail to delete role %s exception: %s", role, e.getMessage());
302+
}
278303
}
279304
return Boolean.TRUE;
280305
}
@@ -292,14 +317,13 @@ public Boolean onRoleUpdated(Role role, RoleChange... changes)
292317

293318
List<AuthorizationSecurableObject> authzSecurableObjects =
294319
translatePrivilege(securableObject);
295-
authzSecurableObjects.stream()
296-
.forEach(
297-
authzSecurableObject -> {
298-
if (!doAddSecurableObject(role.name(), authzSecurableObject)) {
299-
throw new AuthorizationPluginException(
300-
"Failed to add the securable object to the Ranger policy!");
301-
}
302-
});
320+
authzSecurableObjects.forEach(
321+
authzSecurableObject -> {
322+
if (!doAddSecurableObject(role.name(), authzSecurableObject)) {
323+
throw new AuthorizationPluginException(
324+
"Failed to add the securable object to the Ranger policy!");
325+
}
326+
});
303327
} else if (change instanceof RoleChange.RemoveSecurableObject) {
304328
SecurableObject securableObject =
305329
((RoleChange.RemoveSecurableObject) change).getSecurableObject();
@@ -337,16 +361,14 @@ public Boolean onRoleUpdated(Role role, RoleChange... changes)
337361
translatePrivilege(oldSecurableObject);
338362
List<AuthorizationSecurableObject> rangerNewSecurableObjects =
339363
translatePrivilege(newSecurableObject);
340-
rangerOldSecurableObjects.stream()
341-
.forEach(
342-
AuthorizationSecurableObject -> {
343-
removeSecurableObject(role.name(), AuthorizationSecurableObject);
344-
});
345-
rangerNewSecurableObjects.stream()
346-
.forEach(
347-
AuthorizationSecurableObject -> {
348-
doAddSecurableObject(role.name(), AuthorizationSecurableObject);
349-
});
364+
rangerOldSecurableObjects.forEach(
365+
AuthorizationSecurableObject -> {
366+
removeSecurableObject(role.name(), AuthorizationSecurableObject);
367+
});
368+
rangerNewSecurableObjects.forEach(
369+
AuthorizationSecurableObject -> {
370+
doAddSecurableObject(role.name(), AuthorizationSecurableObject);
371+
});
350372
} else {
351373
throw new IllegalArgumentException(
352374
"Unsupported role change type: "
@@ -499,23 +521,21 @@ public Boolean onOwnerSet(MetadataObject metadataObject, Owner preOwner, Owner n
499521
LOG.warn("Grant owner role: {} failed!", ownerRoleName, e);
500522
}
501523

502-
rangerSecurableObjects.stream()
503-
.forEach(
504-
rangerSecurableObject -> {
505-
RangerPolicy policy = findManagedPolicy(rangerSecurableObject);
506-
try {
507-
if (policy == null) {
508-
policy = addOwnerRoleToNewPolicy(rangerSecurableObject, ownerRoleName);
509-
rangerClient.createPolicy(policy);
510-
} else {
511-
rangerHelper.updatePolicyOwnerRole(policy, ownerRoleName);
512-
rangerClient.updatePolicy(policy.getId(), policy);
513-
}
514-
} catch (RangerServiceException e) {
515-
throw new AuthorizationPluginException(
516-
e, "Failed to add the owner to the Ranger!");
517-
}
518-
});
524+
rangerSecurableObjects.forEach(
525+
rangerSecurableObject -> {
526+
RangerPolicy policy = findManagedPolicy(rangerSecurableObject);
527+
try {
528+
if (policy == null) {
529+
policy = addOwnerRoleToNewPolicy(rangerSecurableObject, ownerRoleName);
530+
rangerClient.createPolicy(policy);
531+
} else {
532+
rangerHelper.updatePolicyOwnerRole(policy, ownerRoleName);
533+
rangerClient.updatePolicy(policy.getId(), policy);
534+
}
535+
} catch (RangerServiceException e) {
536+
throw new AuthorizationPluginException(e, "Failed to add the owner to the Ranger!");
537+
}
538+
});
519539
break;
520540
case SCHEMA:
521541
case TABLE:
@@ -576,8 +596,9 @@ public Boolean onGrantedRolesToUser(List<Role> roles, User user)
576596
try {
577597
rangerClient.grantRole(rangerServiceName, grantRevokeRoleRequest);
578598
} catch (RangerServiceException e) {
579-
// Ignore exception, support idempotent operation
580-
LOG.warn("Grant role: {} to user: {} failed!", role, user, e);
599+
throw new AuthorizationPluginException(
600+
"Fail to grant role %s to user %s, exception: %s",
601+
role.name(), user.name(), e.getMessage());
581602
}
582603
});
583604

@@ -611,8 +632,9 @@ public Boolean onRevokedRolesFromUser(List<Role> roles, User user)
611632
try {
612633
rangerClient.revokeRole(rangerServiceName, grantRevokeRoleRequest);
613634
} catch (RangerServiceException e) {
614-
// Ignore exception to support idempotent operation
615-
LOG.warn("Revoke role: {} from user: {} failed!", role, user, e);
635+
throw new AuthorizationPluginException(
636+
"Fail to revoke role %s from user %s, exception: %s",
637+
role.name(), user.name(), e.getMessage());
616638
}
617639
});
618640

@@ -646,8 +668,9 @@ public Boolean onGrantedRolesToGroup(List<Role> roles, Group group)
646668
try {
647669
rangerClient.grantRole(rangerServiceName, grantRevokeRoleRequest);
648670
} catch (RangerServiceException e) {
649-
// Ignore exception to support idempotent operation
650-
LOG.warn("Grant role: {} to group: {} failed!", role, group, e);
671+
throw new AuthorizationPluginException(
672+
"Fail to grant role: %s to group %s, exception: %s.",
673+
role, group, e.getMessage());
651674
}
652675
});
653676
return Boolean.TRUE;
@@ -678,8 +701,9 @@ public Boolean onRevokedRolesFromGroup(List<Role> roles, Group group)
678701
try {
679702
rangerClient.revokeRole(rangerServiceName, grantRevokeRoleRequest);
680703
} catch (RangerServiceException e) {
681-
// Ignore exception to support idempotent operation
682-
LOG.warn("Revoke role: {} from group: {} failed!", role, group, e);
704+
throw new AuthorizationPluginException(
705+
"Fail to revoke role %s from group %s, exception: %s",
706+
role.name(), group.name(), e.getMessage());
683707
}
684708
});
685709

‎authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ protected GrantRevokeRoleRequest createGrantRevokeRoleRequest(
236236
Set<String> groups =
237237
StringUtils.isEmpty(groupName) ? Sets.newHashSet() : Sets.newHashSet(groupName);
238238

239-
if (users.size() == 0 && groups.size() == 0) {
239+
if (users.isEmpty() && groups.isEmpty()) {
240240
throw new AuthorizationPluginException("The user and group cannot be empty!");
241241
}
242242

@@ -274,13 +274,8 @@ protected RangerRole createRangerRoleIfNotExists(String roleName, boolean isOwne
274274
GRAVITINO_METALAKE_OWNER_ROLE, GRAVITINO_CATALOG_OWNER_ROLE, GRAVITINO_OWNER_ROLE));
275275
}
276276

277-
RangerRole rangerRole = null;
278-
try {
279-
rangerRole = rangerClient.getRole(roleName, rangerAdminName, rangerServiceName);
280-
} catch (RangerServiceException e) {
281-
// ignore exception, If the role does not exist, then create it.
282-
LOG.warn("The role({}) does not exist in the Ranger!", roleName);
283-
}
277+
RangerRole rangerRole = getRangerRole(roleName);
278+
284279
try {
285280
if (rangerRole == null) {
286281
rangerRole = new RangerRole(roleName, RangerHelper.MANAGED_BY_GRAVITINO, null, null, null);
@@ -293,6 +288,26 @@ protected RangerRole createRangerRoleIfNotExists(String roleName, boolean isOwne
293288
return rangerRole;
294289
}
295290

291+
public RangerRole getRangerRole(String roleName) {
292+
RangerRole rangerRole = null;
293+
try {
294+
rangerRole = rangerClient.getRole(roleName, rangerAdminName, rangerServiceName);
295+
} catch (RangerServiceException e) {
296+
297+
// The client will return a error message contains `doesn't have permission` if the role does
298+
// not exist, then create it.
299+
if (e.getMessage() != null
300+
&& e.getMessage().contains("User doesn't have permissions to get details")) {
301+
LOG.warn("The role({}) does not exist in the Ranger!, e: {}", roleName, e);
302+
} else {
303+
throw new AuthorizationPluginException(
304+
"Failed to check role(%s) whether exists in the Ranger! e: %s",
305+
roleName, e.getMessage());
306+
}
307+
}
308+
return rangerRole;
309+
}
310+
296311
protected void updatePolicyOwner(RangerPolicy policy, Owner preOwner, Owner newOwner) {
297312
// Find matching policy items based on the owner's privileges
298313
List<RangerPolicy.RangerPolicyItem> matchPolicyItems =

0 commit comments

Comments
 (0)