Skip to content

HDDS-15982. Fix UnsupportedOperationException and add idempotency to RangerClientMultiTenantAccessController #10872

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MahmoudHassanen99 wants to merge 4 commits into
base: master
Choose a base branch
from
+100 −10
Open

HDDS-15982. Fix UnsupportedOperationException and add idempotency to RangerClientMultiTenantAccessController #10872

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6d51fab
HDDS-15982. Fix UnsupportedOperationException and add idempotency to …
MahmoudHassanen99 Jul 25, 2026
3870804
HDDS-15982. Address code review feedback for Ranger client
MahmoudHassanen99 Jul 26, 2026
b47008f
Add co-author
MahmoudHassanen99 Jul 26, 2026
c1c9529
Fix checkstyle import spacing
MahmoudHassanen99 Jul 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 100 additions & 10 deletions .../java/org/apache/hadoop/ozone/om/multitenant/RangerClientMultiTenantAccessController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ public class RangerClientMultiTenantAccessController implements

private static final int HTTP_STATUS_CODE_UNAUTHORIZED = 401;
private static final int HTTP_STATUS_CODE_BAD_REQUEST = 400;
private static final int HTTP_STATUS_CODE_NOT_FOUND = 404;

private final RangerClient client;
private final String rangerServiceName;
Expand Down Expand Up @@ -171,6 +172,37 @@ private void decodeRSEStatusCodes(RangerServiceException rse) {
}
}

/**
* Returns true if the RangerServiceException indicates a duplicate object
* (HTTP 400 with "already exists" in the message).
* Used to implement idempotent create operations.
*/
private static boolean isDuplicateException(RangerServiceException e) {
if (e.getStatus() == null) {
return false;
}
if (e.getStatus().getStatusCode() != HTTP_STATUS_CODE_BAD_REQUEST) {
return false;
}
String msg = e.getMessage();
return msg != null && (msg.contains("already exists")
|| msg.contains("duplicate")
|| msg.contains("DUPLICATE"));
}

/**
* Returns true if the RangerServiceException indicates a not-found condition
* (HTTP 404). Used to implement tolerant delete operations.
*/
private static boolean isNotFoundException(RangerServiceException e) {
return e.getStatus() != null
&& e.getStatus().getStatusCode() == HTTP_STATUS_CODE_NOT_FOUND;
}

// =========================================================================
// Policy operations
// =========================================================================

@Override
public Policy createPolicy(Policy policy) throws IOException {
if (LOG.isDebugEnabled()) {
Expand All @@ -181,6 +213,19 @@ public Policy createPolicy(Policy policy) throws IOException {
try {
rangerPolicy = client.createPolicy(toRangerPolicy(policy));
} catch (RangerServiceException e) {
// If the policy already exists, fetch and return it
// instead of failing. This makes tenant creation idempotent.
if (isDuplicateException(e)) {
LOG.warn("Policy {} already exists in Ranger, fetching existing policy.",
policy.getName());
try {
rangerPolicy = client.getPolicy(rangerServiceName, policy.getName());
return fromRangerPolicy(rangerPolicy);
} catch (RangerServiceException e2) {
decodeRSEStatusCodes(e2);
throw new IOException(e2);
}
}
decodeRSEStatusCodes(e);
throw new IOException(e);
}
Expand Down Expand Up @@ -248,11 +293,21 @@ public void deletePolicy(String policyName) throws IOException {
try {
client.deletePolicy(rangerServiceName, policyName);
} catch (RangerServiceException e) {
// If the policy does not exist, silently return.
// This makes tenant deletion tolerant of partial previous state.
if (isNotFoundException(e)) {
LOG.warn("Policy {} not found in Ranger during delete - assuming already deleted.", policyName);
return;
}
decodeRSEStatusCodes(e);
throw new IOException(e);
}
}

// =========================================================================
// Role operations
// =========================================================================

@Override
public Role createRole(Role role) throws IOException {
if (LOG.isDebugEnabled()) {
Expand All @@ -264,6 +319,22 @@ public Role createRole(Role role) throws IOException {
rangerRole = client.createRole(rangerServiceName,
toRangerRole(role, shortName));
} catch (RangerServiceException e) {
// If the role already exists (HTTP 400 duplicate), fetch
// and return the existing role instead of throwing IOException.
// This makes tenant creation idempotent — safe to retry after a
// partial failure from a previous ozone tenant create attempt.
if (isDuplicateException(e)) {
LOG.warn("Role {} already exists in Ranger, fetching existing role.",
role.getName());
try {
RangerRole existingRole = client.getRole(
role.getName(), shortName, rangerServiceName);
return fromRangerRole(existingRole);
} catch (RangerServiceException e2) {
decodeRSEStatusCodes(e2);
throw new IOException(e2);
}
}
decodeRSEStatusCodes(e);
throw new IOException(e);
}
Expand Down Expand Up @@ -313,6 +384,13 @@ public void deleteRole(String roleName) throws IOException {
try {
client.deleteRole(roleName, shortName, rangerServiceName);
} catch (RangerServiceException e) {
// If the role does not exist, silently return.
// This makes tenant deletion tolerant of partial previous state,
// e.g. when one role was deleted but another was not.
if (isNotFoundException(e)) {
LOG.warn("Role {} not found in Ranger during delete - assuming already deleted.", roleName);
return;
}
decodeRSEStatusCodes(e);
throw new IOException(e);
}
Expand All @@ -333,6 +411,10 @@ public long getRangerServicePolicyVersion() throws IOException {
return policyVersion == null ? -1L : policyVersion;
}

// =========================================================================
// Private conversion helpers
// =========================================================================

private static List<RangerRole.RoleMember> toRangerRoleMembers(
Map<String, Boolean> members) {
return members.entrySet().stream()
Expand Down Expand Up @@ -433,23 +515,20 @@ private RangerPolicy toRangerPolicy(Policy policy) {
rangerPolicy.setService(rangerServiceName);
rangerPolicy.setPolicyLabels(new ArrayList<>(policy.getLabels()));

// Add resources.
// Add resources — always use new ArrayList to ensure mutability.
Map<String, RangerPolicy.RangerPolicyResource> resource = new HashMap<>();
// Add volumes.
if (!policy.getVolumes().isEmpty()) {
RangerPolicy.RangerPolicyResource volumeResources =
new RangerPolicy.RangerPolicyResource();
volumeResources.setValues(new ArrayList<>(policy.getVolumes()));
resource.put("volume", volumeResources);
}
// Add buckets.
if (!policy.getBuckets().isEmpty()) {
RangerPolicy.RangerPolicyResource bucketResources =
new RangerPolicy.RangerPolicyResource();
bucketResources.setValues(new ArrayList<>(policy.getBuckets()));
resource.put("bucket", bucketResources);
}
// Add keys.
if (!policy.getKeys().isEmpty()) {
RangerPolicy.RangerPolicyResource keyResources =
new RangerPolicy.RangerPolicyResource();
Expand All @@ -462,21 +541,27 @@ private RangerPolicy toRangerPolicy(Policy policy) {
rangerPolicy.setDescription(policy.getDescription().get());
}

// Use an explicit mutable ArrayList for policy items to prevent
// UnsupportedOperationException when Ranger model returns unmodifiable list.
List<RangerPolicy.RangerPolicyItem> policyItems = new ArrayList<>();

// Add users to the policy.
for (Map.Entry<String, Collection<Acl>> userAcls:
policy.getUserAcls().entrySet()) {
RangerPolicy.RangerPolicyItem item = new RangerPolicy.RangerPolicyItem();
item.setUsers(Collections.singletonList(userAcls.getKey()));

// Use mutable ArrayList for accesses.
List<RangerPolicy.RangerPolicyItemAccess> accesses = new ArrayList<>();
for (Acl acl: userAcls.getValue()) {
RangerPolicy.RangerPolicyItemAccess access =
new RangerPolicy.RangerPolicyItemAccess();
access.setIsAllowed(acl.isAllowed());
access.setType(aclToString.get(acl.getAclType()));
item.getAccesses().add(access);
accesses.add(access);
}

rangerPolicy.getPolicyItems().add(item);
item.setAccesses(accesses);
policyItems.add(item);
}

// Add roles to the policy.
Expand All @@ -485,17 +570,22 @@ private RangerPolicy toRangerPolicy(Policy policy) {
RangerPolicy.RangerPolicyItem item = new RangerPolicy.RangerPolicyItem();
item.setRoles(Collections.singletonList(roleAcls.getKey()));

// Use mutable ArrayList for accesses.
List<RangerPolicy.RangerPolicyItemAccess> accesses = new ArrayList<>();
for (Acl acl: roleAcls.getValue()) {
RangerPolicy.RangerPolicyItemAccess access =
new RangerPolicy.RangerPolicyItemAccess();
access.setIsAllowed(acl.isAllowed());
access.setType(aclToString.get(acl.getAclType()));
item.getAccesses().add(access);
accesses.add(access);
}

rangerPolicy.getPolicyItems().add(item);
item.setAccesses(accesses);
policyItems.add(item);
}

// Set the fully populated mutable list on rangerPolicy.
rangerPolicy.setPolicyItems(policyItems);

return rangerPolicy;
}
}