smengcl commented on code in PR #10872:
URL: https://github.com/apache/ozone/pull/10872#discussion_r3718504383
##########
hadoop-ozone/multitenancy-ranger/src/test/java/org/apache/hadoop/ozone/om/multitenant/TestRangerClientMultiTenantAccessController.java:
##########
@@ -21,66 +21,159 @@
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.assertInstanceOf;
-
+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 org.apache.hadoop.hdds.conf.InMemoryConfigurationForTesting;
import org.apache.hadoop.hdds.conf.MutableConfigurationSource;
+import
org.apache.hadoop.ozone.om.multitenant.MultiTenantAccessController.Policy;
+import org.apache.hadoop.ozone.om.multitenant.MultiTenantAccessController.Role;
import org.apache.hadoop.security.authentication.util.KerberosName;
import org.apache.ozone.test.GenericTestUtils;
import org.apache.ozone.test.tag.Unhealthy;
import org.apache.ranger.RangerClient;
+import org.apache.ranger.RangerServiceException;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import org.slf4j.LoggerFactory;
import org.slf4j.event.Level;
@Unhealthy("Requires a Ranger endpoint")
class TestRangerClientMultiTenantAccessController extends
MultiTenantAccessControllerTests {
+ 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");
+
+ accessController = new RangerClientMultiTenantAccessController(conf);
+
+ Field clientField =
RangerClientMultiTenantAccessController.class.getDeclaredField("client");
+ clientField.setAccessible(true);
+ clientField.set(accessController, rangerClient);
+ }
+
@Override
protected MultiTenantAccessController createSubject() {
MutableConfigurationSource conf = new InMemoryConfigurationForTesting();
-
// Set up truststore
System.setProperty("javax.net.ssl.trustStore",
"/path/to/cm-auto-global_truststore.jks");
-
// Specify Kerberos client config (krb5.conf) path
System.setProperty("java.security.krb5.conf", "/etc/krb5.conf");
-
// Enable Kerberos debugging
System.setProperty("sun.security.krb5.debug", "true");
-
// DEFAULT rule uses the default realm configured in krb5.conf
KerberosName.setRules("DEFAULT");
-
// These config keys must be properly set when the test is run:
//
// OZONE_RANGER_HTTPS_ADDRESS_KEY
// OZONE_RANGER_SERVICE
// OZONE_OM_KERBEROS_PRINCIPAL_KEY
// OZONE_OM_KERBEROS_KEYTAB_FILE_KEY
-
// Same as OM ranger-ozone-security.xml ranger.plugin.ozone.policy.rest.url
conf.set(OZONE_RANGER_HTTPS_ADDRESS_KEY,
"https://localhost:6182/";);
-
// Same as OM ranger-ozone-security.xml ranger.plugin.ozone.service.name
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");
-
// TODO: Test with clear text username and password as well.
// conf.set(OZONE_OM_RANGER_HTTPS_ADMIN_API_USER, "rangeruser");
// conf.set(OZONE_OM_RANGER_HTTPS_ADMIN_API_PASSWD, "passwd");
-
// (Optional) Enable RangerClient debug log
GenericTestUtils.setLogLevel(
LoggerFactory.getLogger(RangerClient.class), Level.DEBUG);
-
return assertInstanceOf(RangerClientMultiTenantAccessController.class,
MultiTenantAccessController.create(conf));
}
+ @Test
Review Comment:
The inherited tests require a real Ranger endpoint, so the `@Unhealthy` tag
is expected for them. However, these new tests use a mock Ranger client and do
not require that endpoint. The class-level tag excludes them from the normal
build.
Could you move the mock tests to a separate class that has no `@Unhealthy`
tag and does not extend `MultiTenantAccessControllerTests`?
##########
hadoop-ozone/multitenancy-ranger/src/test/java/org/apache/hadoop/ozone/om/multitenant/TestRangerClientMultiTenantAccessController.java:
##########
@@ -21,66 +21,159 @@
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.assertInstanceOf;
-
+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 org.apache.hadoop.hdds.conf.InMemoryConfigurationForTesting;
import org.apache.hadoop.hdds.conf.MutableConfigurationSource;
+import
org.apache.hadoop.ozone.om.multitenant.MultiTenantAccessController.Policy;
+import org.apache.hadoop.ozone.om.multitenant.MultiTenantAccessController.Role;
import org.apache.hadoop.security.authentication.util.KerberosName;
import org.apache.ozone.test.GenericTestUtils;
import org.apache.ozone.test.tag.Unhealthy;
import org.apache.ranger.RangerClient;
+import org.apache.ranger.RangerServiceException;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import org.slf4j.LoggerFactory;
import org.slf4j.event.Level;
@Unhealthy("Requires a Ranger endpoint")
class TestRangerClientMultiTenantAccessController extends
MultiTenantAccessControllerTests {
+ 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");
+
+ accessController = new RangerClientMultiTenantAccessController(conf);
+
+ Field clientField =
RangerClientMultiTenantAccessController.class.getDeclaredField("client");
+ clientField.setAccessible(true);
+ clientField.set(accessController, rangerClient);
+ }
+
@Override
protected MultiTenantAccessController createSubject() {
MutableConfigurationSource conf = new InMemoryConfigurationForTesting();
-
// Set up truststore
System.setProperty("javax.net.ssl.trustStore",
"/path/to/cm-auto-global_truststore.jks");
-
// Specify Kerberos client config (krb5.conf) path
System.setProperty("java.security.krb5.conf", "/etc/krb5.conf");
-
// Enable Kerberos debugging
System.setProperty("sun.security.krb5.debug", "true");
-
// DEFAULT rule uses the default realm configured in krb5.conf
KerberosName.setRules("DEFAULT");
-
// These config keys must be properly set when the test is run:
//
// OZONE_RANGER_HTTPS_ADDRESS_KEY
// OZONE_RANGER_SERVICE
// OZONE_OM_KERBEROS_PRINCIPAL_KEY
// OZONE_OM_KERBEROS_KEYTAB_FILE_KEY
-
// Same as OM ranger-ozone-security.xml ranger.plugin.ozone.policy.rest.url
conf.set(OZONE_RANGER_HTTPS_ADDRESS_KEY,
"https://localhost:6182/";);
-
// Same as OM ranger-ozone-security.xml ranger.plugin.ozone.service.name
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");
-
// TODO: Test with clear text username and password as well.
// conf.set(OZONE_OM_RANGER_HTTPS_ADMIN_API_USER, "rangeruser");
// conf.set(OZONE_OM_RANGER_HTTPS_ADMIN_API_PASSWD, "passwd");
-
// (Optional) Enable RangerClient debug log
GenericTestUtils.setLogLevel(
LoggerFactory.getLogger(RangerClient.class), Level.DEBUG);
-
return assertInstanceOf(RangerClientMultiTenantAccessController.class,
MultiTenantAccessController.create(conf));
}
+ @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").build();
Review Comment:
This policy has no user ACLs or role ACLs. Therefore, the test does not
enter the loops that contain the mutable-list fix.
Could you add one user ACL and one role ACL to this policy? The existing
`IOException` assertion will then fail if policy conversion throws
`UnsupportedOperationException`.
```diff
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;
- Policy policy = new Policy.Builder().setName("tenant-policy").build();
+ 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();
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
