From 6d51fabb037e8b9f14d5723fc6d1a95c6c4e2046 Mon Sep 17 00:00:00 2001 From: Mahmoud Hassanen <88080392+MahmoudHassanen99@users.noreply.github.com> Date: Sun, 26 Jul 2026 01:49:55 +0300 Subject: [PATCH 1/4] HDDS-15982. Fix UnsupportedOperationException and add idempotency to RangerClientMultiTenantAccessController Co-authored-by: sallmayasser --- ...ngerClientMultiTenantAccessController.java | 124 +++++++++++++++--- 1 file changed, 106 insertions(+), 18 deletions(-) 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..d8c8846a136d 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 @@ -1,5 +1,4 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more +/* Licensed to the Apache Software Foundation (ASF) under one or more * contributor license agreements. See the NOTICE file distributed with * this work for additional information regarding copyright ownership. * The ASF licenses this file to You under the Apache License, Version 2.0 @@ -49,10 +48,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -/** - * Implementation of {@link MultiTenantAccessController} using the - * {@link RangerClient} to communicate with Ranger. - */ + public class RangerClientMultiTenantAccessController implements MultiTenantAccessController { @@ -61,6 +57,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 +168,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 +209,19 @@ public Policy createPolicy(Policy policy) throws IOException { try { rangerPolicy = client.createPolicy(toRangerPolicy(policy)); } catch (RangerServiceException e) { + // Fix 2: 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 +289,22 @@ public void deletePolicy(String policyName) throws IOException { try { client.deletePolicy(rangerServiceName, policyName); } catch (RangerServiceException e) { + // Fix 4: 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 +316,22 @@ public Role createRole(Role role) throws IOException { rangerRole = client.createRole(rangerServiceName, toRangerRole(role, shortName)); } catch (RangerServiceException e) { + // Fix 1: 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 +381,14 @@ public void deleteRole(String roleName) throws IOException { try { client.deleteRole(roleName, shortName, rangerServiceName); } catch (RangerServiceException e) { + // Fix 3: 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 +409,10 @@ public long getRangerServicePolicyVersion() throws IOException { return policyVersion == null ? -1L : policyVersion; } + // ========================================================================= + // Private conversion helpers + // ========================================================================= + private static List toRangerRoleMembers( Map members) { return members.entrySet().stream() @@ -414,8 +494,8 @@ private Policy fromRangerPolicy(RangerPolicy rangerPolicy) { policyBuilder.addKeys(resourceNames); break; default: - LOG.warn("Pulled Ranger policy with unknown resource type '{}' with" + - " names '{}'", resourceType, String.join(",", resourceNames)); + LOG.warn("Pulled Ranger policy with unknown resource type '{}' with" + + " names '{}'", resourceType, String.join(",", resourceNames)); } } @@ -433,23 +513,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 +539,27 @@ private RangerPolicy toRangerPolicy(Policy policy) { rangerPolicy.setDescription(policy.getDescription().get()); } + // Fix 5: 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())); + // Fix 5 (continued): 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 +568,22 @@ private RangerPolicy toRangerPolicy(Policy policy) { RangerPolicy.RangerPolicyItem item = new RangerPolicy.RangerPolicyItem(); item.setRoles(Collections.singletonList(roleAcls.getKey())); + // Fix 5 (continued): 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; } } From 387080428858da28744db6f432fb118044edb753 Mon Sep 17 00:00:00 2001 From: Mahmoud Hassanen <88080392+MahmoudHassanen99@users.noreply.github.com> Date: Sun, 26 Jul 2026 12:25:48 +0300 Subject: [PATCH 2/4] HDDS-15982. Address code review feedback for Ranger client --- ...ngerClientMultiTenantAccessController.java | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) 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 d8c8846a136d..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 @@ -1,4 +1,5 @@ -/* Licensed to the Apache Software Foundation (ASF) under one or more +/* + * Licensed to the Apache Software Foundation (ASF) under one or more * contributor license agreements. See the NOTICE file distributed with * this work for additional information regarding copyright ownership. * The ASF licenses this file to You under the Apache License, Version 2.0 @@ -48,7 +49,10 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; - +/** + * Implementation of {@link MultiTenantAccessController} using the + * {@link RangerClient} to communicate with Ranger. + */ public class RangerClientMultiTenantAccessController implements MultiTenantAccessController { @@ -209,7 +213,7 @@ public Policy createPolicy(Policy policy) throws IOException { try { rangerPolicy = client.createPolicy(toRangerPolicy(policy)); } catch (RangerServiceException e) { - // Fix 2: if the policy already exists, fetch and return it + // 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.", @@ -289,11 +293,10 @@ public void deletePolicy(String policyName) throws IOException { try { client.deletePolicy(rangerServiceName, policyName); } catch (RangerServiceException e) { - // Fix 4: if the policy does not exist, silently return. + // 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); + LOG.warn("Policy {} not found in Ranger during delete - assuming already deleted.", policyName); return; } decodeRSEStatusCodes(e); @@ -316,7 +319,7 @@ public Role createRole(Role role) throws IOException { rangerRole = client.createRole(rangerServiceName, toRangerRole(role, shortName)); } catch (RangerServiceException e) { - // Fix 1: if the role already exists (HTTP 400 duplicate), fetch + // 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. @@ -381,12 +384,11 @@ public void deleteRole(String roleName) throws IOException { try { client.deleteRole(roleName, shortName, rangerServiceName); } catch (RangerServiceException e) { - // Fix 3: if the role does not exist, silently return. + // 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); + LOG.warn("Role {} not found in Ranger during delete - assuming already deleted.", roleName); return; } decodeRSEStatusCodes(e); @@ -494,8 +496,8 @@ private Policy fromRangerPolicy(RangerPolicy rangerPolicy) { policyBuilder.addKeys(resourceNames); break; default: - LOG.warn("Pulled Ranger policy with unknown resource type '{}' with" - + " names '{}'", resourceType, String.join(",", resourceNames)); + LOG.warn("Pulled Ranger policy with unknown resource type '{}' with" + + " names '{}'", resourceType, String.join(",", resourceNames)); } } @@ -539,7 +541,7 @@ private RangerPolicy toRangerPolicy(Policy policy) { rangerPolicy.setDescription(policy.getDescription().get()); } - // Fix 5: use an explicit mutable ArrayList for policy items to prevent + // Use an explicit mutable ArrayList for policy items to prevent // UnsupportedOperationException when Ranger model returns unmodifiable list. List policyItems = new ArrayList<>(); @@ -549,7 +551,7 @@ private RangerPolicy toRangerPolicy(Policy policy) { RangerPolicy.RangerPolicyItem item = new RangerPolicy.RangerPolicyItem(); item.setUsers(Collections.singletonList(userAcls.getKey())); - // Fix 5 (continued): use mutable ArrayList for accesses. + // Use mutable ArrayList for accesses. List accesses = new ArrayList<>(); for (Acl acl: userAcls.getValue()) { RangerPolicy.RangerPolicyItemAccess access = @@ -568,7 +570,7 @@ private RangerPolicy toRangerPolicy(Policy policy) { RangerPolicy.RangerPolicyItem item = new RangerPolicy.RangerPolicyItem(); item.setRoles(Collections.singletonList(roleAcls.getKey())); - // Fix 5 (continued): use mutable ArrayList for accesses. + // Use mutable ArrayList for accesses. List accesses = new ArrayList<>(); for (Acl acl: roleAcls.getValue()) { RangerPolicy.RangerPolicyItemAccess access = From b47008f94c82be148e2a791ddf6e2ae8de386dc3 Mon Sep 17 00:00:00 2001 From: Mahmoud Hassanen <88080392+MahmoudHassanen99@users.noreply.github.com> Date: Sun, 26 Jul 2026 14:20:19 +0300 Subject: [PATCH 3/4] Add co-author Co-authored-by: Amr ElBoridy --- .../om/multitenant/RangerClientMultiTenantAccessController.java | 1 - 1 file changed, 1 deletion(-) 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 8616f93cc0cc..5b59ca0e252a 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 @@ -16,7 +16,6 @@ */ package org.apache.hadoop.ozone.om.multitenant; - import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_KERBEROS_KEYTAB_FILE_KEY; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_KERBEROS_PRINCIPAL_KEY; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_RANGER_HTTPS_ADMIN_API_PASSWD; From c1c952941987534a8c84884c8d9c1b88e43010f2 Mon Sep 17 00:00:00 2001 From: Mahmoud Hassanen <88080392+MahmoudHassanen99@users.noreply.github.com> Date: Sun, 26 Jul 2026 17:29:14 +0300 Subject: [PATCH 4/4] Fix checkstyle import spacing --- .../om/multitenant/RangerClientMultiTenantAccessController.java | 1 + 1 file changed, 1 insertion(+) 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 5b59ca0e252a..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 @@ -16,6 +16,7 @@ */ package org.apache.hadoop.ozone.om.multitenant; + import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_KERBEROS_KEYTAB_FILE_KEY; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_KERBEROS_PRINCIPAL_KEY; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_RANGER_HTTPS_ADMIN_API_PASSWD;