diff --git a/hadoop-ozone/multitenancy-ranger/src/main/java/org/apache/hadoop/ozone/om/multitenant/RangerClientMultiTenantAccessController.java b/hadoop-ozone/multitenancy-ranger/src/main/java/org/apache/hadoop/ozone/om/multitenant/RangerClientMultiTenantAccessController.java index 936259a2b948..8616f93cc0cc 100644 --- a/hadoop-ozone/multitenancy-ranger/src/main/java/org/apache/hadoop/ozone/om/multitenant/RangerClientMultiTenantAccessController.java +++ b/hadoop-ozone/multitenancy-ranger/src/main/java/org/apache/hadoop/ozone/om/multitenant/RangerClientMultiTenantAccessController.java @@ -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; @@ -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()) { @@ -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); } @@ -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()) { @@ -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); } @@ -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); } @@ -333,6 +411,10 @@ public long getRangerServicePolicyVersion() throws IOException { return policyVersion == null ? -1L : policyVersion; } + // ========================================================================= + // Private conversion helpers + // ========================================================================= + private static List toRangerRoleMembers( Map members) { return members.entrySet().stream() @@ -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 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(); @@ -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 policyItems = new ArrayList<>(); + // Add users to the policy. for (Map.Entry> userAcls: policy.getUserAcls().entrySet()) { RangerPolicy.RangerPolicyItem item = new RangerPolicy.RangerPolicyItem(); item.setUsers(Collections.singletonList(userAcls.getKey())); + // Use mutable ArrayList for accesses. + List 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. @@ -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 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; } }