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 <ab...@apache.org> 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 {