This is an automated email from the ASF dual-hosted git repository.
madhan pushed a commit to branch ranger-2.6
in repository https://gitbox.apache.org/repos/asf/ranger.git
The following commit(s) were added to refs/heads/ranger-2.6 by this push:
new ea0f5dff42 RANGER-5115: fix for ConcurrentModificationException during
policy evaluation
ea0f5dff42 is described below
commit ea0f5dff426c51b642eab79e1d7a08723735728f
Author: Madhan Neethiraj <[email protected]>
AuthorDate: Sun Jan 26 02:12:58 2025 -0800
RANGER-5115: fix for ConcurrentModificationException during policy
evaluation
(cherry picked from commit aab28a56a21dce59416573b5f5da58650863579b))
---
.../RangerAbstractPolicyItemEvaluator.java | 64 ++++++++++++----------
.../RangerDefaultPolicyItemEvaluator.java | 8 +--
.../RangerOptimizedPolicyEvaluator.java | 2 +-
.../ranger/plugin/policyengine/TestPolicyACLs.java | 8 +--
.../plugin/policyengine/TestPolicyEngine.java | 9 +--
5 files changed, 43 insertions(+), 48 deletions(-)
diff --git
a/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerAbstractPolicyItemEvaluator.java
b/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerAbstractPolicyItemEvaluator.java
index 5370a27a39..38458c58c3 100644
---
a/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerAbstractPolicyItemEvaluator.java
+++
b/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerAbstractPolicyItemEvaluator.java
@@ -109,52 +109,56 @@ public RangerPolicyItem getWithImpliedGrants() {
}
protected RangerPolicyItem computeWithImpliedGrants() {
+ RangerPolicyItem ret = withImpliedGrants;
- final RangerPolicyItem ret;
+ if (ret == null) {
+ synchronized (this) {
+ ret = withImpliedGrants;
- if (withImpliedGrants == null) {
- if (CollectionUtils.isEmpty(policyItem.getAccesses())) {
- ret = policyItem;
- } else {
- // Compute implied-accesses
- Map<String, Collection<String>>
impliedAccessGrants = options.getServiceDefHelper().getImpliedAccessGrants();
+ if (ret == null) {
+ ret = policyItem;
- if (impliedAccessGrants != null &&
!impliedAccessGrants.isEmpty()) {
- ret = new RangerPolicyItem(policyItem);
+ if
(CollectionUtils.isNotEmpty(policyItem.getAccesses())) {
+ // Compute implied-accesses
+ Map<String, Collection<String>>
impliedAccessGrants = options.getServiceDefHelper().getImpliedAccessGrants();
- // Only one round of 'expansion' is
done; multi-level impliedGrants (like shown below) are not handled for now
- // multi-level impliedGrants: given
admin=>write; write=>read: must imply admin=>read,write
- for (Map.Entry<String,
Collection<String>> e : impliedAccessGrants.entrySet()) {
- String implyingAccessType =
e.getKey();
- Collection<String>
impliedGrants = e.getValue();
+ if (impliedAccessGrants != null
&& !impliedAccessGrants.isEmpty()) {
+ ret = new
RangerPolicyItem(policyItem);
-
RangerPolicy.RangerPolicyItemAccess access =
RangerDefaultPolicyEvaluator.getAccess(ret, implyingAccessType);
+ // Only one round of
'expansion' is done; multi-level impliedGrants (like shown below) are not
handled for now
+ // multi-level
impliedGrants: given admin=>write; write=>read: must imply admin=>read,write
+ for (Map.Entry<String,
Collection<String>> e : impliedAccessGrants.entrySet()) {
+ String
implyingAccessType = e.getKey();
+
Collection<String> impliedGrants = e.getValue();
- if (access == null) {
- continue;
- }
+
RangerPolicy.RangerPolicyItemAccess access =
RangerDefaultPolicyEvaluator.getAccess(ret, implyingAccessType);
+
+ if (access ==
null) {
+
continue;
+ }
- for (String impliedGrant :
impliedGrants) {
-
RangerPolicy.RangerPolicyItemAccess impliedAccess =
RangerDefaultPolicyEvaluator.getAccess(ret, impliedGrant);
+ for (String
impliedGrant : impliedGrants) {
+
RangerPolicy.RangerPolicyItemAccess impliedAccess =
RangerDefaultPolicyEvaluator.getAccess(ret, impliedGrant);
- if (impliedAccess ==
null) {
- impliedAccess =
new RangerPolicy.RangerPolicyItemAccess(impliedGrant, access.getIsAllowed());
+ if
(impliedAccess == null) {
+
impliedAccess = new RangerPolicy.RangerPolicyItemAccess(impliedGrant,
access.getIsAllowed());
-
ret.addAccess(impliedAccess);
- } else {
- if
(!impliedAccess.getIsAllowed()) {
-
impliedAccess.setIsAllowed(access.getIsAllowed());
+
ret.addAccess(impliedAccess);
+ } else {
+
if (!impliedAccess.getIsAllowed()) {
+
impliedAccess.setIsAllowed(access.getIsAllowed());
+
}
+ }
}
}
}
}
- } else {
- ret = policyItem;
+
+ withImpliedGrants = ret;
}
}
- } else {
- ret = withImpliedGrants;
}
+
return ret;
}
diff --git
a/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyItemEvaluator.java
b/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyItemEvaluator.java
index 9ed0249efe..7f9123e0fa 100644
---
a/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyItemEvaluator.java
+++
b/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyItemEvaluator.java
@@ -92,9 +92,7 @@ public boolean isMatch(RangerAccessRequest request) {
ret = true;
}
} else {
- if (withImpliedGrants == null) {
- withImpliedGrants =
computeWithImpliedGrants();
- }
+ RangerPolicyItem withImpliedGrants =
computeWithImpliedGrants();
if (withImpliedGrants != null &&
CollectionUtils.isNotEmpty(withImpliedGrants.getAccesses())) {
boolean isAccessTypeMatched =
false;
@@ -198,9 +196,7 @@ public boolean matchAccessType(String accessType) {
if (isAdminAccess) {
ret = policyItem.getDelegateAdmin();
} else {
- if (withImpliedGrants == null) {
- withImpliedGrants =
computeWithImpliedGrants();
- }
+ RangerPolicyItem withImpliedGrants =
computeWithImpliedGrants();
if
(CollectionUtils.isNotEmpty(withImpliedGrants.getAccesses())) {
boolean isAnyAccess =
StringUtils.equals(accessType, RangerPolicyEngine.ANY_ACCESS);
diff --git
a/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java
b/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java
index 665ee3cbec..2f8605f078 100644
---
a/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java
+++
b/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java
@@ -359,7 +359,7 @@ private void preprocessPolicyItems(List<? extends
RangerPolicy.RangerPolicyItem>
for(RangerPolicy.RangerPolicyItemAccess policyItemAccess :
policyItemAccesses) {
if (policyItemAccess.getIsAllowed()) {
- add(accessPerms, policyItemAccess.getType());
+ accessPerms = add(accessPerms,
policyItemAccess.getType());
}
}
diff --git
a/agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestPolicyACLs.java
b/agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestPolicyACLs.java
index 9a69efcbac..160e14cec1 100644
---
a/agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestPolicyACLs.java
+++
b/agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestPolicyACLs.java
@@ -26,6 +26,7 @@
import java.lang.reflect.Type;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Set;
import com.google.gson.JsonDeserializationContext;
@@ -121,10 +122,7 @@ private void runTests(InputStreamReader reader, String
testName) throws Exceptio
RangerPluginContext pluginContext = new
RangerPluginContext(new RangerPluginConfig(serviceType, null,
"test-policy-acls", "cl1", "on-prem", policyEngineOptions));
RangerPolicyEngine policyEngine = new
RangerPolicyEngineImpl(testCase.servicePolicies, pluginContext, null);
- for(PolicyACLsTests.TestCase.OneTest oneTest :
testCase.tests) {
- if(oneTest == null) {
- continue;
- }
+
testCase.tests.parallelStream().filter(Objects::nonNull).forEach(oneTest -> {
RangerAccessRequestImpl request = new
RangerAccessRequestImpl(oneTest.resource, RangerPolicyEngine.ANY_ACCESS, null,
null, null);
request.setResourceMatchingScope(oneTest.resourceMatchingScope);
@@ -288,7 +286,7 @@ private void runTests(InputStreamReader reader, String
testName) throws Exceptio
assertTrue("getResourceACLs() failed! " +
testCase.name + ":" + oneTest.name + " - roleACLsMatched", roleACLsMatched);
assertTrue("getResourceACLs() failed! " +
testCase.name + ":" + oneTest.name + " - rowFiltersMatched", rowFiltersMatched);
assertTrue("getResourceACLs() failed! " +
testCase.name + ":" + oneTest.name + " - dataMaskingMatched",
dataMaskingMatched);
- }
+ });
}
}
diff --git
a/agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestPolicyEngine.java
b/agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestPolicyEngine.java
index c892060221..e9fe36d5de 100644
---
a/agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestPolicyEngine.java
+++
b/agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestPolicyEngine.java
@@ -645,10 +645,8 @@ private void runTests(InputStreamReader reader, String
testName) {
}
private void runTestCaseTests(RangerPolicyEngine policyEngine,
RangerServiceDef serviceDef, String testName, List<TestData> tests) {
- RangerAccessRequest request = null;
-
- for(TestData test : tests) {
- request = test.request;
+ tests.parallelStream().forEach(test -> {
+ RangerAccessRequest request = test.request;
if
(request.getContext().containsKey(RangerAccessRequestUtil.KEY_CONTEXT_TAGS) ||
request.getContext().containsKey(RangerAccessRequestUtil.KEY_CONTEXT_REQUESTED_RESOURCES))
{
@@ -779,8 +777,7 @@ private void runTestCaseTests(RangerPolicyEngine
policyEngine, RangerServiceDef
assertEquals("deniedUsers mismatched! - " +
test.name, expected.getDeniedUsers(), result.getDeniedUsers());
assertEquals("deniedGroups mismatched! - " +
test.name, expected.getDeniedGroups(), result.getDeniedGroups());
}
- }
-
+ });
}
private void setPluginConfig(RangerPluginConfig conf, String suffix,
Set<String> value) {
