You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ranger.apache.org by pr...@apache.org on 2020/05/16 07:08:18 UTC

[ranger] branch master updated: RANGER-2817: Service configuration update does not cause policies in the service to get downloaded to plugin

This is an automated email from the ASF dual-hosted git repository.

pradeep pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ranger.git


The following commit(s) were added to refs/heads/master by this push:
     new acd7516  RANGER-2817: Service configuration update does not cause policies in the service to get downloaded to plugin
acd7516 is described below

commit acd7516eea2e11921a42478b717d58f2bead54e8
Author: maheshbandal <ma...@gmail.com>
AuthorDate: Fri May 15 13:42:31 2020 +0530

    RANGER-2817: Service configuration update does not cause policies in the service to get downloaded to plugin
    
    Signed-off-by: pradeep <pr...@apache.org>
---
 .../java/org/apache/ranger/biz/ServiceDBStore.java | 48 +++++++++++-------
 .../org/apache/ranger/biz/TestServiceDBStore.java  | 59 ++++++++++++++++++++++
 2 files changed, 88 insertions(+), 19 deletions(-)

diff --git a/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java b/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
index 857a597..ed69761 100644
--- a/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
+++ b/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
@@ -243,6 +243,8 @@ public class ServiceDBStore extends AbstractServiceStore {
 	public static Integer RETENTION_PERIOD_IN_DAYS = 7;
 	public static Integer TAG_RETENTION_PERIOD_IN_DAYS = 3;
 
+	private static final String RANGER_PLUGIN_CONFIG_PREFIX = "ranger.plugin.";
+
 	static {
 		try {
 			LOCAL_HOSTNAME = java.net.InetAddress.getLocalHost().getCanonicalHostName();
@@ -1657,7 +1659,7 @@ public class ServiceDBStore extends AbstractServiceStore {
 		boolean hasIsEnabledChanged = !existing.getIsenabled().equals(service.getIsEnabled());
 
 		List<XXServiceConfigMap> dbConfigMaps = daoMgr.getXXServiceConfigMap().findByServiceId(service.getId());
-		boolean hasExcludedUGRConfigChanged = hasExcludedUGRConfigChanged(dbConfigMaps, validConfigs);
+		boolean hasServiceConfigForPluginChanged = hasServiceConfigForPluginChanged(dbConfigMaps, validConfigs);
 
 		List<XXTrxLog> trxLogList = svcService.getTransactionLog(service, existing, RangerServiceService.OPERATION_UPDATE_CONTEXT);
 
@@ -1671,7 +1673,7 @@ public class ServiceDBStore extends AbstractServiceStore {
 			service.setVersion(existing.getVersion());
 			service = svcService.update(service);
 
-			if (hasTagServiceValueChanged || hasIsEnabledChanged || hasExcludedUGRConfigChanged) {
+			if (hasTagServiceValueChanged || hasIsEnabledChanged || hasServiceConfigForPluginChanged) {
 				updatePolicyVersion(service, RangerPolicyDelta.CHANGE_TYPE_SERVICE_CHANGE, null, false);
 			}
 		}
@@ -5375,7 +5377,7 @@ public class ServiceDBStore extends AbstractServiceStore {
 		List<XXServiceConfigMap> xxServiceConfigMaps = daoMgr.getXXServiceConfigMap().findByServiceId(serviceId);
 		if (CollectionUtils.isNotEmpty(xxServiceConfigMaps)) {
 			for (XXServiceConfigMap svcConfMap : xxServiceConfigMaps) {
-				if (StringUtils.startsWith(svcConfMap.getConfigkey(), "ranger.plugin.")) {
+				if (StringUtils.startsWith(svcConfMap.getConfigkey(), RANGER_PLUGIN_CONFIG_PREFIX)) {
 					configs.put(svcConfMap.getConfigkey(), svcConfMap.getConfigvalue());
 				}
 			}
@@ -5383,23 +5385,31 @@ public class ServiceDBStore extends AbstractServiceStore {
 		return configs;
 	}
 
-	private boolean hasExcludedUGRConfigChanged(List<XXServiceConfigMap> dbConfigMaps, Map<String, String> validConfigs) {
+	boolean hasServiceConfigForPluginChanged(List<XXServiceConfigMap> dbConfigMaps, Map<String, String> validConfigs) {
 		boolean ret = false;
-		String auditExcludeUsers = null;
-		String auditExcludeGroups = null;
-		String auditExcludeRoles = null;
-		for (XXServiceConfigMap dbConfigMap : dbConfigMaps) {
-			if (StringUtils.equalsIgnoreCase(dbConfigMap.getConfigkey(), RangerPolicyEngine.PLUGIN_AUDIT_EXCLUDE_USERS)) {
-				auditExcludeUsers = StringUtils.trimToEmpty(dbConfigMap.getConfigvalue());
-			} else if (StringUtils.equalsIgnoreCase(dbConfigMap.getConfigkey(), RangerPolicyEngine.PLUGIN_AUDIT_EXCLUDE_GROUPS)) {
-				auditExcludeGroups = StringUtils.trimToEmpty(dbConfigMap.getConfigvalue());
-			} else if (StringUtils.equalsIgnoreCase(dbConfigMap.getConfigkey(), RangerPolicyEngine.PLUGIN_AUDIT_EXCLUDE_ROLES)) {
-				auditExcludeRoles = StringUtils.trimToEmpty(dbConfigMap.getConfigvalue());
-			}
-		}
-		ret = !StringUtils.equals(auditExcludeUsers, validConfigs.get(RangerPolicyEngine.PLUGIN_AUDIT_EXCLUDE_USERS))
-				|| !StringUtils.equals(auditExcludeGroups, validConfigs.get(RangerPolicyEngine.PLUGIN_AUDIT_EXCLUDE_GROUPS))
-				|| !StringUtils.equals(auditExcludeRoles, validConfigs.get(RangerPolicyEngine.PLUGIN_AUDIT_EXCLUDE_ROLES));
+		Map<String, String> configs = new HashMap<String, String>();
+		if (CollectionUtils.isNotEmpty(dbConfigMaps)) {
+			for (XXServiceConfigMap dbConfigMap : dbConfigMaps) {
+				if (StringUtils.startsWith(dbConfigMap.getConfigkey(), RANGER_PLUGIN_CONFIG_PREFIX)) {
+					configs.put(dbConfigMap.getConfigkey(), dbConfigMap.getConfigvalue());
+				}
+			}
+		}
+		if (MapUtils.isNotEmpty(validConfigs)) {
+			for (String key : validConfigs.keySet()) {
+				if (StringUtils.startsWith(key, RANGER_PLUGIN_CONFIG_PREFIX)) {
+					if (!StringUtils.equals(configs.get(key), validConfigs.get(key))) {
+						return true;
+					} else {
+						configs.remove(key);
+					}
+				}
+			}
+		}
+		if (configs.size() > 0) {
+			return true;
+		}
+
 		return ret;
 	}
 
diff --git a/security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java b/security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java
index 69c8a4c..c219e6c 100644
--- a/security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java
+++ b/security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java
@@ -2503,4 +2503,63 @@ public class TestServiceDBStore {
 		Mockito.verify(daoManager, Mockito.atLeast(1)).getXXService();
 		Mockito.verify(daoManager).getXXServiceConfigMap();
 	}
+
+	@Test
+	public void test50hasServiceConfigForPluginChanged() throws Exception {
+		String pluginConfigKey = "ranger.plugin.testconfig";
+		String otherConfigKey = "ranger.other.testconfig";
+		Map<String, String> serviceConfigs = rangerService().getConfigs();
+		List<XXServiceConfigMap> xConfMapList = new ArrayList<XXServiceConfigMap>();
+		for (String key : serviceConfigs.keySet()) {
+			XXServiceConfigMap xConfMap = new XXServiceConfigMap();
+			xConfMap.setConfigkey(key);
+			xConfMap.setConfigvalue(serviceConfigs.get(key));
+			xConfMap.setServiceId(Id);
+			xConfMapList.add(xConfMap);
+		}
+
+		Map<String, String> validConfig = new HashMap<String, String>();
+		validConfig.putAll(serviceConfigs);
+		Assert.assertFalse(serviceDBStore.hasServiceConfigForPluginChanged(null, null));
+		Assert.assertFalse(serviceDBStore.hasServiceConfigForPluginChanged(xConfMapList, validConfig));
+
+		validConfig.put(pluginConfigKey, "test value added");
+		Assert.assertTrue(serviceDBStore.hasServiceConfigForPluginChanged(xConfMapList, validConfig));
+
+		XXServiceConfigMap xConfMap = new XXServiceConfigMap();
+		xConfMap.setConfigkey(pluginConfigKey);
+		xConfMap.setConfigvalue("test value added");
+		xConfMap.setServiceId(Id);
+		xConfMapList.add(xConfMap);
+		Assert.assertFalse(serviceDBStore.hasServiceConfigForPluginChanged(xConfMapList, validConfig));
+
+		validConfig.put(pluginConfigKey, "test value changed");
+		Assert.assertTrue(serviceDBStore.hasServiceConfigForPluginChanged(xConfMapList, validConfig));
+
+		validConfig.remove(pluginConfigKey);
+		Assert.assertTrue(serviceDBStore.hasServiceConfigForPluginChanged(xConfMapList, validConfig));
+		int index = xConfMapList.size();
+		xConfMap = xConfMapList.remove(index - 1);
+		Assert.assertFalse(serviceDBStore.hasServiceConfigForPluginChanged(xConfMapList, validConfig));
+
+		validConfig.put(otherConfigKey, "other test value added");
+		Assert.assertFalse(serviceDBStore.hasServiceConfigForPluginChanged(xConfMapList, validConfig));
+
+		xConfMap = new XXServiceConfigMap();
+		xConfMap.setConfigkey(otherConfigKey);
+		xConfMap.setConfigvalue("other test value added");
+		xConfMap.setServiceId(Id);
+		xConfMapList.add(xConfMap);
+		Assert.assertFalse(serviceDBStore.hasServiceConfigForPluginChanged(xConfMapList, validConfig));
+
+		validConfig.put(otherConfigKey, "other test value changed");
+		Assert.assertFalse(serviceDBStore.hasServiceConfigForPluginChanged(xConfMapList, validConfig));
+
+		validConfig.remove(otherConfigKey);
+		Assert.assertFalse(serviceDBStore.hasServiceConfigForPluginChanged(xConfMapList, validConfig));
+
+		index = xConfMapList.size();
+		xConfMap = xConfMapList.remove(index - 1);
+		Assert.assertFalse(serviceDBStore.hasServiceConfigForPluginChanged(xConfMapList, validConfig));
+	}
 }