This is an automated email from the ASF dual-hosted git repository.
abhay 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 0e7d63022 RANGER-4478: Incorrect trie updates when processing deltas
0e7d63022 is described below
commit 0e7d63022252dc3c74478aa32bffac3ea755fee9
Author: Abhay Kulkarni <[email protected]>
AuthorDate: Tue Oct 17 13:00:22 2023 -0700
RANGER-4478: Incorrect trie updates when processing deltas
---
.../plugin/policyengine/RangerResourceTrie.java | 71 ++++++++++++----------
.../RangerPolicyResourceMatcher.java | 1 -
2 files changed, 39 insertions(+), 33 deletions(-)
diff --git
a/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerResourceTrie.java
b/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerResourceTrie.java
index 2f725036d..61b6a4357 100644
---
a/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerResourceTrie.java
+++
b/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerResourceTrie.java
@@ -94,6 +94,13 @@ public class RangerResourceTrie<T extends
RangerResourceEvaluator> {
wrapUpUpdate();
+ if (!isOptimizedForRetrieval) {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Trie for " + this.resourceDef.getName() + " is not
optimized for retrieval. Resetting isSetup flag by calling undoSetup() on the
root");
+ }
+ root.undoSetup();
+ }
+
RangerPerfTracer.logAlways(perf);
if (PERF_TRIE_INIT_LOG.isDebugEnabled()) {
@@ -109,7 +116,7 @@ public class RangerResourceTrie<T extends
RangerResourceEvaluator> {
this(resourceDef, evaluators, isOptimizedForRetrieval, false,
pluginContext);
}
- public <T extends RangerResourceEvaluator, E>
RangerResourceTrie(RangerResourceDef resourceDef, List<E> evaluators, boolean
isOptimizedForRetrieval, boolean isOptimizedForSpace, RangerPluginContext
pluginContext) {
+ public <E> RangerResourceTrie(RangerResourceDef resourceDef, List<E>
evaluators, boolean isOptimizedForRetrieval, boolean isOptimizedForSpace,
RangerPluginContext pluginContext) {
if(LOG.isDebugEnabled()) {
LOG.debug("==> RangerResourceTrie(" + resourceDef.getName() + ",
evaluatorCount=" + evaluators.size() + ", isOptimizedForRetrieval=" +
isOptimizedForRetrieval + ", isOptimizedForSpace=" + isOptimizedForSpace + ")");
}
@@ -158,9 +165,9 @@ public class RangerResourceTrie<T extends
RangerResourceEvaluator> {
this.isOptimizedForRetrieval = !isOptimizedForSpace &&
isOptimizedForRetrieval; // isOptimizedForSpace takes precedence
this.separatorChar =
ServiceDefUtil.getCharOption(matcherOptions, OPTION_PATH_SEPARATOR,
DEFAULT_PATH_SEPARATOR_CHAR);
- final TrieNode tmpRoot = buildTrie(resourceDef, evaluators,
builderThreadCount);
+ final TrieNode<T> tmpRoot = buildTrie(resourceDef, evaluators,
builderThreadCount);
- if (builderThreadCount > 1 && tmpRoot == null) { // if multi-threaded
trie-creation failed, build using a single thread
+ if (builderThreadCount > 1 && tmpRoot == null) { // if multithreaded
trie-creation failed, build using a single thread
this.root = buildTrie(resourceDef, evaluators, 1);
} else {
this.root = tmpRoot;
@@ -179,7 +186,7 @@ public class RangerResourceTrie<T extends
RangerResourceEvaluator> {
}
if(LOG.isDebugEnabled()) {
- LOG.debug("<== RangerResourceTrie(" + resourceDef.getName() + ",
evaluatorCount=" + evaluators.size() + ", isOptimizedForRetrieval=" +
this.isOptimizedForRetrieval + ", isOptimizedForSpace=" +
this.isOptimizedForSpace + "): " + toString());
+ LOG.debug("<== RangerResourceTrie(" + resourceDef.getName() + ",
evaluatorCount=" + evaluators.size() + ", isOptimizedForRetrieval=" +
this.isOptimizedForRetrieval + ", isOptimizedForSpace=" +
this.isOptimizedForSpace + "): " + this);
}
}
@@ -191,16 +198,16 @@ public class RangerResourceTrie<T extends
RangerResourceEvaluator> {
return getEvaluatorsForResource(resource,
ResourceElementMatchingScope.SELF);
}
+ @SuppressWarnings("unchecked")
public Set<T> getEvaluatorsForResource(Object resource,
ResourceElementMatchingScope scope) {
if (resource instanceof String) {
return getEvaluatorsForResource((String) resource, scope);
} else if (resource instanceof Collection) {
- if (CollectionUtils.isEmpty((Collection) resource)) { // treat
empty collection same as empty-string
+ Collection<String> resources = (Collection<String>) resource;
+
+ if (CollectionUtils.isEmpty(resources)) { // treat empty
collection same as empty-string
return getEvaluatorsForResource("", scope);
} else {
- @SuppressWarnings("unchecked")
- Collection<String> resources = (Collection<String>) resource;
-
return getEvaluatorsForResources(resources, scope);
}
}
@@ -457,6 +464,9 @@ public class RangerResourceTrie<T extends
RangerResourceEvaluator> {
}
}
}
+ if (ret == null) {
+ break;
+ }
}
if (ret != null) {
@@ -543,6 +553,7 @@ public class RangerResourceTrie<T extends
RangerResourceEvaluator> {
builderThreads.get(index).add(resource, isRecursive, evaluator);
} else {
+ currentRoot.undoSetup();
currentRoot.addWildcardEvaluator(evaluator);
}
@@ -559,6 +570,7 @@ public class RangerResourceTrie<T extends
RangerResourceEvaluator> {
}
if(isWildcard || isRecursive) {
+ curr.undoSetup();
curr.addWildcardEvaluator(evaluator);
} else {
curr.addEvaluator(evaluator);
@@ -648,19 +660,19 @@ public class RangerResourceTrie<T extends
RangerResourceEvaluator> {
}
final boolean includeChildEvaluators = scope ==
ResourceElementMatchingScope.SELF_OR_CHILD || scope ==
ResourceElementMatchingScope.SELF_OR_PREFIX;
- final Set<T> childEvalautors = includeChildEvaluators ? new
HashSet<>() : null;
+ final Set<T> childEvaluators = includeChildEvaluators ? new
HashSet<>() : null;
if (scope == ResourceElementMatchingScope.SELF_OR_CHILD) {
final boolean resourceEndsWithSep =
resource.charAt(resource.length() - 1) == separatorChar;
if (isSelfMatch) { // resource == path(curr)
if (resourceEndsWithSep) { // ex: resource=/tmp/
- curr.getChildren().values().stream().forEach(c ->
c.collectChildEvaluators(separatorChar, 0, childEvalautors));
+ curr.getChildren().values().stream().forEach(c ->
c.collectChildEvaluators(separatorChar, 0, childEvaluators));
} else { // ex: resource=/tmp
curr = curr.getChild(separatorChar);
if (curr != null) {
- curr.collectChildEvaluators(separatorChar, 1,
childEvalautors);
+ curr.collectChildEvaluators(separatorChar, 1,
childEvaluators);
}
}
} else if (child != null) { // resource != path(child) ex:
(resource=/tmp, path(child)=/tmp/test.txt or path(child)=/tmpdir)
@@ -669,22 +681,22 @@ public class RangerResourceTrie<T extends
RangerResourceEvaluator> {
if (isPrefixMatch) {
if (resourceEndsWithSep) { // ex: resource=/tmp/
- child.collectChildEvaluators(separatorChar,
remainingLen, childEvalautors);
+ child.collectChildEvaluators(separatorChar,
remainingLen, childEvaluators);
} else if (child.getStr().charAt(remainingLen) ==
separatorChar) { // ex: resource=/tmp
- child.collectChildEvaluators(separatorChar,
remainingLen + 1, childEvalautors);
+ child.collectChildEvaluators(separatorChar,
remainingLen + 1, childEvaluators);
}
}
}
} else if (scope == ResourceElementMatchingScope.SELF_OR_PREFIX) {
- curr.collectChildEvaluators(resource, i, childEvalautors);
+ curr.collectChildEvaluators(resource, i, childEvaluators);
}
- if (CollectionUtils.isNotEmpty(childEvalautors)) {
+ if (CollectionUtils.isNotEmpty(childEvaluators)) {
if (CollectionUtils.isNotEmpty(ret)) {
- childEvalautors.addAll(ret);
+ childEvaluators.addAll(ret);
}
- ret = childEvalautors;
+ ret = childEvaluators;
}
RangerPerfTracer.logAlways(perf);
@@ -889,8 +901,8 @@ public class RangerResourceTrie<T extends
RangerResourceEvaluator> {
private String str;
private TrieNode<U> parent;
private final Map<Character, TrieNode<U>> children = new
HashMap<>();
- private Set<U> evaluators;
- private Set<U> wildcardEvaluators;
+ private volatile Set<U> evaluators;
+ private volatile Set<U> wildcardEvaluators;
private boolean
isSharingParentWildcardEvaluators;
private volatile boolean isSetup = false;
@@ -1049,19 +1061,15 @@ public class RangerResourceTrie<T extends
RangerResourceEvaluator> {
}
void addWildcardEvaluator(U evaluator) {
- undoSetup();
-
if (wildcardEvaluators == null) {
wildcardEvaluators = new HashSet<>();
}
- if (!wildcardEvaluators.contains(evaluator)) {
- wildcardEvaluators.add(evaluator);
- }
+ wildcardEvaluators.add(evaluator);
}
void removeEvaluator(U evaluator) {
- if (CollectionUtils.isNotEmpty(evaluators) &&
evaluators.contains(evaluator)) {
+ if (CollectionUtils.isNotEmpty(evaluators)) {
evaluators.remove(evaluator);
if (CollectionUtils.isEmpty(evaluators)) {
@@ -1081,11 +1089,11 @@ public class RangerResourceTrie<T extends
RangerResourceEvaluator> {
}
void undoSetup() {
- if (isSetup) {
- for (TrieNode<U> child : children.values()) {
- child.undoSetup();
- }
+ for (TrieNode<U> child : children.values()) {
+ child.undoSetup();
+ }
+ if (isSetup) {
if (evaluators != null) {
if (evaluators == wildcardEvaluators) {
evaluators = null;
@@ -1199,7 +1207,6 @@ public class RangerResourceTrie<T extends
RangerResourceEvaluator> {
}
}
}
-
isSetup = true;
}
}
@@ -1312,7 +1319,7 @@ public class RangerResourceTrie<T extends
RangerResourceEvaluator> {
sb.append("; evaluators=[");
if (evaluators != null) {
for (U evaluator : evaluators) {
- sb.append(evaluator.getId()).append(",");
+ sb.append(evaluator.getId()).append("|,|");
}
}
sb.append("]");
@@ -1320,7 +1327,7 @@ public class RangerResourceTrie<T extends
RangerResourceEvaluator> {
sb.append("; wildcardEvaluators=[ ");
if (wildcardEvaluators != null) {
for (U evaluator : wildcardEvaluators) {
- sb.append(evaluator.getId()).append(",");
+ sb.append(evaluator.getId()).append("|,|");
}
}
sb.append("]");
diff --git
a/agents-common/src/main/java/org/apache/ranger/plugin/policyresourcematcher/RangerPolicyResourceMatcher.java
b/agents-common/src/main/java/org/apache/ranger/plugin/policyresourcematcher/RangerPolicyResourceMatcher.java
index 3ad870b1d..e1cd89b70 100644
---
a/agents-common/src/main/java/org/apache/ranger/plugin/policyresourcematcher/RangerPolicyResourceMatcher.java
+++
b/agents-common/src/main/java/org/apache/ranger/plugin/policyresourcematcher/RangerPolicyResourceMatcher.java
@@ -28,7 +28,6 @@ import org.apache.ranger.plugin.model.RangerServiceDef;
import org.apache.ranger.plugin.model.validation.RangerServiceDefHelper;
import
org.apache.ranger.plugin.policyengine.RangerAccessRequest.ResourceElementMatchingScope;
import org.apache.ranger.plugin.policyengine.RangerAccessResource;
-import org.apache.ranger.plugin.policyevaluator.RangerPolicyEvaluator;
import org.apache.ranger.plugin.resourcematcher.RangerResourceMatcher;
public interface RangerPolicyResourceMatcher {
