This is an automated email from the ASF dual-hosted git repository.
smengcl pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new 09a7d615dec HDDS-15982. Fix UnsupportedOperationException and add
idempotent deletion in RangerClientMultiTenantAccessController (#10872)
09a7d615dec is described below
commit 09a7d615dec6d17ecb7c62e01a5b42e8fe04b43b
Author: Mahmoud Hassanen <[email protected]>
AuthorDate: Tue Aug 11 22:22:50 2026 +0300
HDDS-15982. Fix UnsupportedOperationException and add idempotent deletion
in RangerClientMultiTenantAccessController (#10872)
Co-authored-by: Amr ElBoridy <[email protected]>
Co-authored-by: salma yasser
<[email protected]>
---
.../RangerClientMultiTenantAccessController.java | 98 +++++++++----
...lientMultiTenantAccessControllerMockClient.java | 153 +++++++++++++++++++++
2 files changed, 228 insertions(+), 23 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 936259a2b94..eb326c67e47 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
@@ -32,6 +32,7 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;
@@ -61,6 +62,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;
@@ -137,13 +139,6 @@ public
RangerClientMultiTenantAccessController(ConfigurationSource conf)
// set back the expected login user
UserGroupInformation.setLoginUser(loginUser);
}
-
- // Whether or not the Ranger credentials are valid is unknown right after
- // RangerClient initialization here. Because RangerClient does not perform
- // any authentication at this point just yet.
- //
- // If the credentials are invalid, RangerClient later throws 401 in every
- // single request to Ranger.
}
/**
@@ -171,6 +166,40 @@ private void decodeRSEStatusCodes(RangerServiceException
rse) {
}
}
+ /**
+ * 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;
+ }
+
+ /**
+ * Returns true if the RangerServiceException indicates the role did not
+ * exist at delete time, handling the Ranger 2.8 compatibility quirk.
+ */
+ private static boolean isRoleNotFoundException(RangerServiceException e) {
+ if (isNotFoundException(e)) {
+ return true;
+ }
+ if (e.getStatus() == null
+ || e.getStatus().getStatusCode() != HTTP_STATUS_CODE_BAD_REQUEST) {
+ return false;
+ }
+ String message = e.getMessage();
+ if (message == null) {
+ return false;
+ }
+ String lowerMessage = message.toLowerCase(Locale.ROOT);
+ return lowerMessage.contains("role with name")
+ && lowerMessage.contains("does not exist");
+ }
+
+ // =========================================================================
+ // Policy operations
+ // =========================================================================
+
@Override
public Policy createPolicy(Policy policy) throws IOException {
if (LOG.isDebugEnabled()) {
@@ -248,11 +277,20 @@ public void deletePolicy(String policyName) throws
IOException {
try {
client.deletePolicy(rangerServiceName, policyName);
} catch (RangerServiceException e) {
+ 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()) {
@@ -292,8 +330,6 @@ public Role updateRole(long roleId, Role role) throws
IOException {
LOG.debug("Sending update request for role ID {} to Ranger.",
roleId);
}
- // TODO: Check if createdByUser is even needed for updateRole request.
- // If not, remove the createdByUser param and set it after.
final RangerRole rangerRole;
try {
rangerRole = client.updateRole(roleId, toRangerRole(role, shortName));
@@ -313,6 +349,11 @@ public void deleteRole(String roleName) throws IOException
{
try {
client.deleteRole(roleName, shortName, rangerServiceName);
} catch (RangerServiceException e) {
+ if (isRoleNotFoundException(e)) {
+ LOG.warn("Role {} not found in Ranger during delete - assuming already
deleted.",
+ roleName);
+ return;
+ }
decodeRSEStatusCodes(e);
throw new IOException(e);
}
@@ -327,12 +368,15 @@ public long getRangerServicePolicyVersion() throws
IOException {
decodeRSEStatusCodes(e);
throw new IOException(e);
}
- // If the login user doesn't have sufficient privilege, policyVersion
- // field could be null in RangerService.
+ // If the login user doesn't have sufficient privilege, policyVersion
field could be null in RangerService.
final Long policyVersion = rangerOzoneService.getPolicyVersion();
return policyVersion == null ? -1L : policyVersion;
}
+ // =========================================================================
+ // Private conversion helpers
+ // =========================================================================
+
private static List<RangerRole.RoleMember> toRangerRoleMembers(
Map<String, Boolean> members) {
return members.entrySet().stream()
@@ -433,23 +477,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();
@@ -462,40 +503,51 @@ 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()));
+ item.setUsers(new
ArrayList<>(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.
for (Map.Entry<String, Collection<Acl>> roleAcls:
policy.getRoleAcls().entrySet()) {
RangerPolicy.RangerPolicyItem item = new RangerPolicy.RangerPolicyItem();
- item.setRoles(Collections.singletonList(roleAcls.getKey()));
+ item.setRoles(new
ArrayList<>(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;
}
}
diff --git
a/hadoop-ozone/multitenancy-ranger/src/test/java/org/apache/hadoop/ozone/om/multitenant/TestRangerClientMultiTenantAccessControllerMockClient.java
b/hadoop-ozone/multitenancy-ranger/src/test/java/org/apache/hadoop/ozone/om/multitenant/TestRangerClientMultiTenantAccessControllerMockClient.java
new file mode 100644
index 00000000000..8b96c2d863c
--- /dev/null
+++
b/hadoop-ozone/multitenancy-ranger/src/test/java/org/apache/hadoop/ozone/om/multitenant/TestRangerClientMultiTenantAccessControllerMockClient.java
@@ -0,0 +1,153 @@
+/*
+ * 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
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+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_RANGER_HTTPS_ADDRESS_KEY;
+import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_RANGER_SERVICE;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import com.sun.jersey.api.client.ClientResponse;
+import java.io.IOException;
+import java.lang.reflect.Field;
+import java.util.Collections;
+import org.apache.hadoop.hdds.conf.InMemoryConfigurationForTesting;
+import org.apache.hadoop.hdds.conf.MutableConfigurationSource;
+import org.apache.hadoop.ozone.om.multitenant.MultiTenantAccessController.Acl;
+import
org.apache.hadoop.ozone.om.multitenant.MultiTenantAccessController.Policy;
+import org.apache.hadoop.ozone.om.multitenant.MultiTenantAccessController.Role;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType;
+import org.apache.hadoop.security.authentication.util.KerberosName;
+import org.apache.ranger.RangerClient;
+import org.apache.ranger.RangerServiceException;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Unit tests for {@link RangerClientMultiTenantAccessController} that use a
+ * mock {@link RangerClient} instead of a live Ranger endpoint.
+ */
+class TestRangerClientMultiTenantAccessControllerMockClient {
+
+ private RangerClient rangerClient;
+ private RangerClientMultiTenantAccessController accessController;
+
+ @BeforeEach
+ public void setUpMocks() throws Exception {
+ rangerClient = mock(RangerClient.class);
+ MutableConfigurationSource conf = new InMemoryConfigurationForTesting();
+ conf.set(OZONE_RANGER_HTTPS_ADDRESS_KEY, "https://localhost:6182/";);
+ conf.set(OZONE_RANGER_SERVICE, "cm_ozone");
+ conf.set(OZONE_OM_KERBEROS_PRINCIPAL_KEY, "om/[email protected]");
+ conf.set(OZONE_OM_KERBEROS_KEYTAB_FILE_KEY, "/path/to/ozone.keytab");
+
+ // Initialize Kerberos name rules before creating the controller
+ KerberosName.setRules(
+ "RULE:[2:$1@$0](.*@EXAMPLE.COM)s/@.*//\n" +
+ "DEFAULT");
+ accessController = new RangerClientMultiTenantAccessController(conf);
+
+ Field clientField =
RangerClientMultiTenantAccessController.class.getDeclaredField("client");
+ clientField.setAccessible(true);
+ clientField.set(accessController, rangerClient);
+ }
+
+ @Test
+ public void testDeleteRoleAbsentRoleRanger28Workaround() throws Exception {
+ // Ranger 2.8 returns HTTP 400 with "does not exist" message when role is
missing.
+ RangerServiceException rse = mock(RangerServiceException.class);
+ when(rse.getStatus()).thenReturn(ClientResponse.Status.BAD_REQUEST);
+ when(rse.getMessage()).thenReturn("Role with name 'tenant-role' does not
exist");
+
+ doThrow(rse).when(rangerClient)
+ .deleteRole(anyString(), anyString(), anyString());
+
+ assertDoesNotThrow(() -> accessController.deleteRole("tenant-role"));
+ }
+
+ @Test
+ public void testDeleteRoleAbsentRoleCaseInsensitive() throws Exception {
+ // Verify case-normalization handles uppercase/mixed-case responses.
+ RangerServiceException rse = mock(RangerServiceException.class);
+ when(rse.getStatus()).thenReturn(ClientResponse.Status.BAD_REQUEST);
+ when(rse.getMessage()).thenReturn("ROLE WITH NAME 'tenant-role' DOES NOT
EXIST");
+
+ doThrow(rse).when(rangerClient)
+ .deleteRole(anyString(), anyString(), anyString());
+
+ assertDoesNotThrow(() -> accessController.deleteRole("tenant-role"));
+ }
+
+ @Test
+ public void testDeleteRoleUnrelated400Propagates() throws Exception {
+ // Unrelated HTTP 400 (e.g. role referenced by policy) MUST propagate.
+ RangerServiceException rse = mock(RangerServiceException.class);
+ when(rse.getStatus()).thenReturn(ClientResponse.Status.BAD_REQUEST);
+ when(rse.getMessage()).thenReturn("Role 'tenant-role' is currently in use
by policy 'p1'");
+
+ doThrow(rse).when(rangerClient)
+ .deleteRole(anyString(), anyString(), anyString());
+
+ assertThrows(IOException.class, () ->
accessController.deleteRole("tenant-role"));
+ }
+
+ @Test
+ public void testDeleteRoleGenuine404TreatedAsIdempotent() throws Exception {
+ // Standard HTTP 404 for missing role should pass silently.
+ RangerServiceException rse = mock(RangerServiceException.class);
+ when(rse.getStatus()).thenReturn(ClientResponse.Status.NOT_FOUND);
+
+ doThrow(rse).when(rangerClient)
+ .deleteRole(anyString(), anyString(), anyString());
+
+ assertDoesNotThrow(() -> accessController.deleteRole("tenant-role"));
+ }
+
+ @Test
+ public void testCreatePolicyFailFastOnDuplicate() throws Exception {
+ // Verify createPolicy fails fast on exception without attempting
reconciliation.
+ RangerServiceException rse = mock(RangerServiceException.class);
+ when(rangerClient.createPolicy(any())).thenThrow(rse);
+
+ Policy policy = new Policy.Builder()
+ .setName("tenant-policy")
+ .addUserAcl("user",
+ Collections.singletonList(Acl.allow(ACLType.READ)))
+ .addRoleAcl("role",
+ Collections.singletonList(Acl.allow(ACLType.READ)))
+ .build();
+ assertThrows(IOException.class, () ->
accessController.createPolicy(policy));
+ }
+
+ @Test
+ public void testCreateRoleFailFastOnDuplicate() throws Exception {
+ // Verify createRole fails fast on exception without attempting
reconciliation.
+ RangerServiceException rse = mock(RangerServiceException.class);
+ when(rangerClient.createRole(anyString(), any())).thenThrow(rse);
+
+ Role role = new Role.Builder().setName("tenant-role").build();
+ assertThrows(IOException.class, () -> accessController.createRole(role));
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
