You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ranger.apache.org by ab...@apache.org on 2023/04/17 18:02:49 UTC

[ranger] branch master updated: RANGER-4136: Incorrect processing of tag-deltas by RangerTagEnricher - Part 2

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 fb63f21cf RANGER-4136: Incorrect processing of tag-deltas by RangerTagEnricher - Part 2
fb63f21cf is described below

commit fb63f21cf6f5007f178eef8f11f68cf2c9a57279
Author: Abhay Kulkarni <ab...@apache.org>
AuthorDate: Mon Apr 17 09:50:42 2023 -0700

    RANGER-4136: Incorrect processing of tag-deltas by RangerTagEnricher - Part 2
---
 .../plugin/contextenricher/RangerTagEnricher.java  | 64 +++++++++++++++-------
 .../org/apache/ranger/plugin/util/ServiceTags.java |  3 +
 2 files changed, 47 insertions(+), 20 deletions(-)

diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagEnricher.java b/agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagEnricher.java
index 198d24d97..e0a86c398 100644
--- a/agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagEnricher.java
+++ b/agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagEnricher.java
@@ -385,6 +385,9 @@ public class RangerTagEnricher extends RangerAbstractContextEnricher {
 		this.tagRefresher = null;
 
 		if (tagRefresher != null) {
+			if (LOG.isDebugEnabled()) {
+				LOG.debug("Trying to clean up RangerTagRefresher(" + tagRefresher.getName() + ")");
+			}
 			tagRefresher.cleanup();
 		}
 
@@ -473,20 +476,16 @@ public class RangerTagEnricher extends RangerAbstractContextEnricher {
 		List<RangerServiceResource> changedServiceResources = deltas.getServiceResources();
 
 		for (RangerServiceResource serviceResource : changedServiceResources) {
-
 			final boolean removedOldServiceResource = MapUtils.isEmpty(serviceResource.getResourceElements()) || removeOldServiceResource(serviceResource, resourceMatchers, serviceResourceTrie);
-			if (removedOldServiceResource) {
 
+			if (removedOldServiceResource) {
 				if (!StringUtils.isEmpty(serviceResource.getResourceSignature())) {
-
 					RangerServiceResourceMatcher resourceMatcher = createRangerServiceResourceMatcher(serviceResource, serviceDefHelper, hierarchies);
 
 					if (resourceMatcher != null) {
 						for (RangerServiceDef.RangerResourceDef resourceDef : serviceDef.getResources()) {
-
-							RangerPolicy.RangerPolicyResource policyResource = serviceResource.getResourceElements().get(resourceDef.getName());
-
-							RangerResourceTrie<RangerServiceResourceMatcher> trie = serviceResourceTrie.get(resourceDef.getName());
+							RangerPolicy.RangerPolicyResource                policyResource = serviceResource.getResourceElements().get(resourceDef.getName());
+							RangerResourceTrie<RangerServiceResourceMatcher> trie           = serviceResourceTrie.get(resourceDef.getName());
 
 							if (LOG.isDebugEnabled()) {
 								LOG.debug("Trying to add resource-matcher to " + (trie == null ? "new" : "existing") + " trie for " + resourceDef.getName());
@@ -495,6 +494,7 @@ public class RangerTagEnricher extends RangerAbstractContextEnricher {
 							if (trie != null) {
 								trie.add(policyResource, resourceMatcher);
 								trie.wrapUpUpdate();
+
 								if (LOG.isDebugEnabled()) {
 									LOG.debug("Added resource-matcher for policy-resource:[" + policyResource + "]");
 								}
@@ -521,6 +521,7 @@ public class RangerTagEnricher extends RangerAbstractContextEnricher {
 				break;
 			}
 		}
+
 		if (isInError) {
 			LOG.error("Error in processing tag-deltas. Will continue to use old tags");
 			deltas.setTagVersion(-1L);
@@ -530,44 +531,61 @@ public class RangerTagEnricher extends RangerAbstractContextEnricher {
 			}
 			enrichedServiceTags = new EnrichedServiceTags(allServiceTags, resourceMatchers, serviceResourceTrie);
 		}
-
 	}
 
 	private boolean removeOldServiceResource(RangerServiceResource serviceResource, List<RangerServiceResourceMatcher> resourceMatchers, Map<String, RangerResourceTrie<RangerServiceResourceMatcher>> resourceTries) {
 		boolean ret = true;
 
 		if (enrichedServiceTags != null) {
-
 			if (LOG.isDebugEnabled()) {
 				LOG.debug("Removing service-resource:[" + serviceResource + "] from trie-map");
 			}
 
-			// Remove existing serviceResource from the copy
-
 			RangerAccessResourceImpl accessResource = new RangerAccessResourceImpl();
 
 			for (Map.Entry<String, RangerPolicy.RangerPolicyResource> entry : serviceResource.getResourceElements().entrySet()) {
 				accessResource.setValue(entry.getKey(), entry.getValue().getValues());
 			}
+
 			if (LOG.isDebugEnabled()) {
 				LOG.debug("RangerAccessResource:[" + accessResource + "] created to represent service-resource[" + serviceResource + "] to find evaluators from trie-map");
 			}
 
-			RangerAccessRequestImpl  request = new RangerAccessRequestImpl();
+			RangerAccessRequestImpl request = new RangerAccessRequestImpl();
 			request.setResource(accessResource);
 
 			Collection<RangerServiceResourceMatcher> oldMatchers = getEvaluators(request, enrichedServiceTags);
 
 			if (LOG.isDebugEnabled()) {
-				LOG.debug("Found [" + oldMatchers.size() + "] matchers for service-resource[" + serviceResource + "]");
+				LOG.debug("Found [" + oldMatchers + "] matchers for service-resource[" + serviceResource + "]");
 			}
 
-			for (RangerServiceResourceMatcher matcher : oldMatchers) {
+			if (CollectionUtils.isNotEmpty(oldMatchers)) {
+				List<RangerServiceResourceMatcher> notMatched = new ArrayList<>();
 
-				for (RangerServiceDef.RangerResourceDef resourceDef : serviceDef.getResources()) {
-					String resourceDefName = resourceDef.getName();
+				for (RangerServiceResourceMatcher resourceMatcher : oldMatchers) {
+					final RangerPolicyResourceMatcher.MatchType matchType = resourceMatcher.getMatchType(accessResource, request.getContext());
+
+					if (LOG.isDebugEnabled()) {
+						LOG.debug("resource:[" + accessResource + ", MatchType:[" + matchType + "]");
+					}
+
+					if (matchType != RangerPolicyResourceMatcher.MatchType.SELF) {
+						notMatched.add(resourceMatcher);
+					}
+				}
 
-					RangerResourceTrie<RangerServiceResourceMatcher> trie = resourceTries.get(resourceDefName);
+				if (LOG.isDebugEnabled()) {
+					LOG.debug("oldMatchers : [" + notMatched + "] do not match resource:[" + accessResource + "] exactly and will be discarded");
+				}
+
+				oldMatchers.removeAll(notMatched);
+			}
+
+			for (RangerServiceResourceMatcher matcher : oldMatchers) {
+				for (RangerServiceDef.RangerResourceDef resourceDef : serviceDef.getResources()) {
+					String                                           resourceDefName = resourceDef.getName();
+					RangerResourceTrie<RangerServiceResourceMatcher> trie            = resourceTries.get(resourceDefName);
 
 					if (trie != null) {
 						trie.delete(serviceResource.getResourceElements().get(resourceDefName), matcher);
@@ -580,12 +598,11 @@ public class RangerTagEnricher extends RangerAbstractContextEnricher {
 				}
 			}
 
-			// Remove old resource matchers
 			if (ret) {
 				resourceMatchers.removeAll(oldMatchers);
 
 				if (LOG.isDebugEnabled()) {
-					LOG.debug("Found and removed [" + oldMatchers.size() + "] matchers for service-resource[" + serviceResource + "] from trie-map");
+					LOG.debug("Found and removed [" + oldMatchers + "] matchers for service-resource[" + serviceResource + "] from trie-map");
 				}
 			}
 		}
@@ -691,6 +708,10 @@ public class RangerTagEnricher extends RangerAbstractContextEnricher {
 
 					final RangerPolicyResourceMatcher.MatchType matchType = resourceMatcher.getMatchType(resource, request.getContext());
 
+					if (LOG.isDebugEnabled()) {
+						LOG.debug("resource:[" + resource + ", MatchType:[" + matchType + "]");
+					}
+
 					final boolean isMatched;
 
 					if (request.isAccessTypeAny()) {
@@ -759,7 +780,7 @@ public class RangerTagEnricher extends RangerAbstractContextEnricher {
 		}
 
 		if(LOG.isDebugEnabled()) {
-			LOG.debug("<== RangerTagEnricher.getEvaluators(request=" + request + "): evaluatorCount=" + ret.size());
+			LOG.debug("<== RangerTagEnricher.getEvaluators(request=" + request + "): evaluators=" + ret);
 		}
 
 		return ret;
@@ -1005,6 +1026,9 @@ public class RangerTagEnricher extends RangerAbstractContextEnricher {
 					try {
 						super.join();
 						isJoined = true;
+						if (LOG.isDebugEnabled()) {
+							LOG.debug("RangerTagRefresher(" + getName() + ") is stopped");
+						}
 					} catch (InterruptedException excp) {
 						LOG.warn("RangerTagRefresher(" + getName() + ").stopRefresher(): Error while waiting for thread to exit", excp);
 						LOG.warn("Retrying Thread.join(). Current thread will be marked as 'interrupted' after Thread.join() returns");
diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceTags.java b/agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceTags.java
index 9b902789a..96fd02873 100644
--- a/agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceTags.java
+++ b/agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceTags.java
@@ -228,6 +228,9 @@ public class ServiceTags implements java.io.Serializable {
 				.append("tagUpdateTime={").append(tagUpdateTime).append("}")
 				.append("isDelta={").append(isDelta).append("}")
 				.append("tagsChangeExtent={").append(tagsChangeExtent).append("}")
+				.append(", serviceResources={").append(serviceResources).append("}")
+				.append(", tags={").append(tags).append("}")
+				.append(", resourceToTagIds={").append(resourceToTagIds).append("}")
 				.append("}");
 
 		return sb;