This is an automated email from the ASF dual-hosted git repository.
madhan 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 aab28a56a2 RANGER-5115: fix for ConcurrentModificationException during
policy evaluation (#517)
aab28a56a2 is described below
commit aab28a56a21dce59416573b5f5da58650863579b
Author: Madhan Neethiraj <[email protected]>
AuthorDate: Sun Jan 26 01:57:05 2025 -0800
RANGER-5115: fix for ConcurrentModificationException during policy
evaluation (#517)
---
.../RangerAbstractPolicyItemEvaluator.java | 65 ++++++++++++----------
.../RangerDefaultPolicyItemEvaluator.java | 8 +--
.../RangerOptimizedPolicyEvaluator.java | 2 +-
.../ranger/plugin/policyengine/TestPolicyACLs.java | 8 +--
.../plugin/policyengine/TestPolicyEngine.java | 8 +--
5 files changed, 44 insertions(+), 47 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 c2920ff3e4..19ba50f53d 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
@@ -106,51 +106,56 @@ protected boolean getConditionsDisabledOption() {
}
protected RangerPolicyItem computeWithImpliedGrants() {
- final RangerPolicyItem ret;
+ RangerPolicyItem 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) {
+ synchronized (this) {
+ ret = withImpliedGrants;
- if (impliedAccessGrants != null &&
!impliedAccessGrants.isEmpty()) {
- ret = new RangerPolicyItem(policyItem);
+ if (ret == null) {
+ ret = policyItem;
- // 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 (CollectionUtils.isNotEmpty(policyItem.getAccesses())) {
+ // Compute implied-accesses
+ Map<String, Collection<String>> impliedAccessGrants =
options.getServiceDefHelper().getImpliedAccessGrants();
- RangerPolicy.RangerPolicyItemAccess access =
RangerDefaultPolicyEvaluator.getAccess(ret, implyingAccessType);
+ if (impliedAccessGrants != null &&
!impliedAccessGrants.isEmpty()) {
+ ret = new RangerPolicyItem(policyItem);
- if (access == null) {
- continue;
- }
+ // 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();
- for (String impliedGrant : impliedGrants) {
- RangerPolicy.RangerPolicyItemAccess impliedAccess
= RangerDefaultPolicyEvaluator.getAccess(ret, impliedGrant);
+ RangerPolicy.RangerPolicyItemAccess access =
RangerDefaultPolicyEvaluator.getAccess(ret, implyingAccessType);
+
+ if (access == null) {
+ continue;
+ }
- if (impliedAccess == null) {
- impliedAccess = new
RangerPolicy.RangerPolicyItemAccess(impliedGrant, access.getIsAllowed());
+ for (String impliedGrant : impliedGrants) {
+ RangerPolicy.RangerPolicyItemAccess
impliedAccess = RangerDefaultPolicyEvaluator.getAccess(ret, impliedGrant);
- ret.addAccess(impliedAccess);
- } else {
- if (!impliedAccess.getIsAllowed()) {
-
impliedAccess.setIsAllowed(access.getIsAllowed());
+ if (impliedAccess == null) {
+ impliedAccess = new
RangerPolicy.RangerPolicyItemAccess(impliedGrant, 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 9f25db3752..311dfd6b2d 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
@@ -85,9 +85,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;
@@ -155,9 +153,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 7f01f5cd0d..7233ac2452 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
@@ -321,7 +321,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 40da6cf269..3faea812b7 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
@@ -43,6 +43,7 @@
import java.lang.reflect.Type;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Set;
import static org.junit.Assert.assertEquals;
@@ -119,10 +120,7 @@ private void runTests(InputStreamReader reader, String
testName) {
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);
@@ -287,7 +285,7 @@ private void runTests(InputStreamReader reader, String
testName) {
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 ee49520fd6..549bf6bd11 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
@@ -817,10 +817,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))
{
@@ -949,7 +947,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) {
