Skip to content

Commit

Permalink
RANGER-4478: Incorrect trie updates when processing deltas
Browse files Browse the repository at this point in the history
  • Loading branch information
@kulkabhay
kulkabhay committed Oct 17, 2023
1 parent f1d9da4 commit 0e7d630
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 33 deletions.
71 changes: 39 additions & 32 deletions agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerResourceTrie.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,13 @@ public RangerResourceTrie(RangerResourceTrie<T> other) {

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()) {
Expand All @@ -109,7 +116,7 @@ public RangerResourceTrie(RangerResourceDef resourceDef, List<T> evaluators, boo
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 + ")");
}
Expand Down Expand Up @@ -158,9 +165,9 @@ public <T extends RangerResourceEvaluator, E> RangerResourceTrie(RangerResourceD
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;
Expand All @@ -179,7 +186,7 @@ public <T extends RangerResourceEvaluator, E> RangerResourceTrie(RangerResourceD
}

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);
}
}

Expand All @@ -191,16 +198,16 @@ public Set<T> getEvaluatorsForResource(Object resource) {
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);
}
}
Expand Down Expand Up @@ -457,6 +464,9 @@ private <E> TrieNode<T> buildTrie(RangerResourceDef resourceDef, List<E> evaluat
}
}
}
if (ret == null) {
break;
}
}

if (ret != null) {
Expand Down Expand Up @@ -543,6 +553,7 @@ private int insert(TrieNode<T> currentRoot, String resource, boolean isRecursive

builderThreads.get(index).add(resource, isRecursive, evaluator);
} else {
currentRoot.undoSetup();
currentRoot.addWildcardEvaluator(evaluator);
}

Expand All @@ -559,6 +570,7 @@ private void insert(TrieNode<T> currentRoot, String resource, boolean isRecursiv
}

if(isWildcard || isRecursive) {
curr.undoSetup();
curr.addWildcardEvaluator(evaluator);
} else {
curr.addEvaluator(evaluator);
Expand Down Expand Up @@ -648,19 +660,19 @@ private Set<T> getEvaluatorsForResource(String resource, ResourceElementMatching
}

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)
Expand All @@ -669,22 +681,22 @@ private Set<T> getEvaluatorsForResource(String resource, ResourceElementMatching

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);
Expand Down Expand Up @@ -889,8 +901,8 @@ class TrieNode<U extends T> {
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;

Expand Down Expand Up @@ -1049,19 +1061,15 @@ void addEvaluator(U evaluator) {
}

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)) {
Expand All @@ -1081,11 +1089,11 @@ void removeWildcardEvaluator(U evaluator) {
}

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;
Expand Down Expand Up @@ -1199,7 +1207,6 @@ void setup(Set<U> parentWildcardEvaluators) {
}
}
}

isSetup = true;
}
}
Expand Down Expand Up @@ -1312,15 +1319,15 @@ void toString(StringBuilder sb) {
sb.append("; evaluators=[");
if (evaluators != null) {
for (U evaluator : evaluators) {
sb.append(evaluator.getId()).append(",");
sb.append(evaluator.getId()).append("|,|");
}
}
sb.append("]");

sb.append("; wildcardEvaluators=[ ");
if (wildcardEvaluators != null) {
for (U evaluator : wildcardEvaluators) {
sb.append(evaluator.getId()).append(",");
sb.append(evaluator.getId()).append("|,|");
}
}
sb.append("]");
Expand Down
1 change: 0 additions & 1 deletion ...main/java/org/apache/ranger/plugin/policyresourcematcher/RangerPolicyResourceMatcher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
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 {
Expand Down

0 comments on commit 0e7d630

Please sign in to comment.