RANGER-5678: Concurrent policy-engine rebuild on shared serviceDef ca…

agents-common/src/main/java/org/apache/ranger/plugin/policyengine/PolicyEngine.java

@@ -563,12 +563,21 @@ private void normalizeServiceDefs(ServicePolicies servicePolicies) {
         RangerServiceDef serviceDef = servicePolicies.getServiceDef();
 
         if (serviceDef != null) {
-            ServiceDefUtil.normalize(serviceDef);
+            RangerServiceDef normalizedServiceDef = ServiceDefUtil.normalizeCopiedServiceDef(serviceDef);
 
-            RangerServiceDef tagServiceDef = servicePolicies.getTagPolicies() != null ? servicePolicies.getTagPolicies().getServiceDef() : null;
+            servicePolicies.setServiceDef(normalizedServiceDef);
 
-            if (tagServiceDef != null) {
-                ServiceDefUtil.normalizeAccessTypeDefs(ServiceDefUtil.normalize(tagServiceDef), serviceDef.getName());
+            ServicePolicies.TagPolicies tagPolicies = servicePolicies.getTagPolicies();
+
+            if (tagPolicies != null) {
+                RangerServiceDef tagServiceDef = tagPolicies.getServiceDef();
+
+                if (tagServiceDef != null) {
+                    RangerServiceDef normalizedTagServiceDef = ServiceDefUtil.normalizeAccessTypeDefs(
+                            ServiceDefUtil.normalizeCopiedServiceDef(tagServiceDef), normalizedServiceDef.getName());
+
+                    tagPolicies.setServiceDef(normalizedTagServiceDef);
+                }
             }
         }
     }

agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyRepository.java

@@ -188,7 +188,7 @@ public RangerPolicyRepository(ServicePolicies servicePolicies, RangerPluginConte
         super();
 
         this.serviceName          = servicePolicies.getServiceName();
-        this.serviceDef           = ServiceDefUtil.normalize(servicePolicies.getServiceDef());
+        this.serviceDef           = ServiceDefUtil.normalizeCopiedServiceDef(servicePolicies.getServiceDef());
         this.componentServiceName = this.serviceName;
         this.componentServiceDef  = this.serviceDef;
         this.zoneName             = zoneName;
@@ -259,7 +259,7 @@ public RangerPolicyRepository(ServicePolicies servicePolicies, RangerPluginConte
         this.serviceName          = tagPolicies.getServiceName();
         this.componentServiceName = componentServiceName;
         this.zoneName             = null;
-        this.serviceDef           = ServiceDefUtil.normalizeAccessTypeDefs(ServiceDefUtil.normalize(tagPolicies.getServiceDef()), componentServiceDef.getName());
+        this.serviceDef           = ServiceDefUtil.normalizeAccessTypeDefs(ServiceDefUtil.normalizeCopiedServiceDef(tagPolicies.getServiceDef()), componentServiceDef.getName());
         this.componentServiceDef  = componentServiceDef;
         this.appId                = pluginContext.getConfig().getAppId();
         this.options              = new RangerPolicyEngineOptions(pluginContext.getConfig().getPolicyEngineOptions(), new RangerServiceDefHelper(serviceDef, false));
@@ -1530,4 +1530,5 @@ boolean getIsAudited() {
             return isAudited;
         }
     }
+
 }

agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceDefUtil.java

@@ -23,6 +23,7 @@
 import org.apache.commons.collections.MapUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.hadoop.conf.Configuration;
+import org.apache.ranger.authorization.utils.JsonUtils;
 import org.apache.ranger.plugin.conditionevaluator.RangerScriptConditionEvaluator;
 import org.apache.ranger.plugin.contextenricher.RangerGdsEnricher;
 import org.apache.ranger.plugin.contextenricher.RangerUserStoreEnricher;
@@ -132,6 +133,33 @@ public static RangerServiceDef normalize(RangerServiceDef serviceDef) {
         return serviceDef;
     }
 
+    /**
+     * Returns a normalized deep copy of the service definition so that shared cached
+     * instances are not mutated in place. Falls back to in-place normalize if copy fails.
+     */
+    public static RangerServiceDef normalizeCopiedServiceDef(RangerServiceDef serviceDef) {
+        if (serviceDef == null) {
+            return null;
+        }
+
+        RangerServiceDef serviceDefCopy = copyServiceDef(serviceDef);
+
+        return normalize(serviceDefCopy != null ? serviceDefCopy : serviceDef);
+    }
+
+    /**
+     * Returns a deep copy of the service definition, or null if copy fails.
+     */
+    public static RangerServiceDef copyServiceDef(RangerServiceDef serviceDef) {
+        RangerServiceDef ret = null;
+
+        if (serviceDef != null) {
+            ret = JsonUtils.jsonToObject(JsonUtils.objectToJson(serviceDef), RangerServiceDef.class);
+        }
+
+        return ret;
+    }
+
     public static RangerPolicyConditionDef getConditionDef(RangerServiceDef serviceDef, String conditionName) {
         RangerPolicyConditionDef ret = null;
 

agents-common/src/test/java/org/apache/ranger/plugin/util/ServiceDefUtilTest.java

@@ -57,6 +57,8 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNotSame;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 public class ServiceDefUtilTest {
@@ -530,4 +532,24 @@ private SecurityZoneInfo getSecurityZoneInfo(String zoneName) {
 
         return ret;
     }
+
+    @Test
+    public void testCopyServiceDefBeforeNormalizeDoesNotMutateSource() throws Exception {
+        RangerServiceDef source = EmbeddedServiceDefsUtil.instance().getEmbeddedServiceDef(
+                EmbeddedServiceDefsUtil.EMBEDDED_SERVICEDEF_HIVE_NAME);
+
+        assertNotNull(source, "hive embedded service-def should be available");
+        assertNotNull(source.getDataMaskDef(), "hive service-def should have dataMaskDef");
+
+        RangerServiceDef serviceDefCopy = ServiceDefUtil.copyServiceDef(source);
+
+        assertNotNull(serviceDefCopy);
+        assertNotSame(source, serviceDefCopy, "copyServiceDef must not return the cached instance");
+
+        RangerServiceDef normalized = ServiceDefUtil.normalize(serviceDefCopy);
+
+        assertNotNull(normalized);
+        assertNotSame(source, normalized, "normalize must operate on a copy, not the cached instance");
+        assertNotNull(normalized.getDataMaskDef());
+    }
 }