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 d484e2b RANGER-2510: Support for Incremental tag updates to improve performance - part 2 d484e2b is described below commit d484e2bad26cc024e36908691138f8c4ac133f47 Author: Abhay Kulkarni <ab...@apache.org> AuthorDate: Tue Oct 1 08:19:54 2019 -0700 RANGER-2510: Support for Incremental tag updates to improve performance - part 2 --- .../plugin/contextenricher/RangerTagEnricher.java | 427 +++++++++++---------- .../org/apache/ranger/db/XXPolicyChangeLogDao.java | 28 +- .../apache/ranger/db/XXServiceVersionInfoDao.java | 43 ++- .../org/apache/ranger/db/XXTagChangeLogDao.java | 20 +- .../apache/ranger/entity/XXPolicyChangeLog.java | 42 +- .../org/apache/ranger/entity/XXTagChangeLog.java | 36 +- .../java/org/apache/ranger/rest/PublicAPIsv2.java | 24 +- .../apache/ranger/service/RangerTagDefService.java | 9 - .../main/resources/META-INF/jpa_named_queries.xml | 5 - .../ranger/service/TestRangerTagDefService.java | 8 - 10 files changed, 318 insertions(+), 324 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 3063885..7434ec9 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 @@ -263,9 +263,6 @@ public class RangerTagEnricher extends RangerAbstractContextEnricher { LOG.info("ServiceTags is null for service " + serviceName); enrichedServiceTags = null; } else { - RangerServiceDefHelper serviceDefHelper = new RangerServiceDefHelper(serviceDef, false); - ResourceHierarchies hierarchies = new ResourceHierarchies(); - RangerPerfTracer perf = null; if(RangerPerfTracer.isPerfTraceEnabled(PERF_SET_SERVICETAGS_LOG)) { @@ -273,50 +270,14 @@ public class RangerTagEnricher extends RangerAbstractContextEnricher { } if (!serviceTags.getIsDelta()) { - if (LOG.isDebugEnabled()) { - LOG.debug("Received all service-tags"); - } - if (CollectionUtils.isEmpty(serviceTags.getServiceResources())) { - LOG.info("There are no tagged resources for service " + serviceName); - enrichedServiceTags = null; - } else { - - List<RangerServiceResourceMatcher> resourceMatchers = new ArrayList<>(); - List<RangerServiceResource> serviceResources = serviceTags.getServiceResources(); - - for (RangerServiceResource serviceResource : serviceResources) { - RangerServiceResourceMatcher serviceResourceMatcher = createRangerServiceResourceMatcher(serviceResource, serviceDefHelper, hierarchies); - if (serviceResourceMatcher != null) { - resourceMatchers.add(serviceResourceMatcher); - } else { - LOG.error("Could not create service-resource-matcher for service-resource:[" + serviceResource + "]"); - } - } - - Map<String, RangerResourceTrie<RangerServiceResourceMatcher>> serviceResourceTrie = null; - - if (!disableTrieLookupPrefilter) { - serviceResourceTrie = new HashMap<>(); - - for (RangerServiceDef.RangerResourceDef resourceDef : serviceDef.getResources()) { - serviceResourceTrie.put(resourceDef.getName(), new RangerResourceTrie<>(resourceDef, resourceMatchers)); - } - } - - enrichedServiceTags = new EnrichedServiceTags(serviceTags, resourceMatchers, serviceResourceTrie); - } - + processServiceTags(serviceTags); } else { if (LOG.isDebugEnabled()) { LOG.debug("Received service-tag deltas:" + serviceTags); } - ServiceTags delta = serviceTags; ServiceTags oldServiceTags = enrichedServiceTags != null ? enrichedServiceTags.getServiceTags() : new ServiceTags(); - - ServiceTags newServiceTags = rebuildOnlyIndex ? oldServiceTags : RangerServiceTagsDeltaUtil.applyDelta(oldServiceTags, delta); - List<RangerServiceResourceMatcher> newResourceMatchers = enrichedServiceTags != null ? enrichedServiceTags.getServiceResourceMatchers() : new ArrayList<>(); - Map<String, RangerResourceTrie<RangerServiceResourceMatcher>> newServiceResourceTrie = enrichedServiceTags != null ? enrichedServiceTags.getServiceResourceTrie() : new HashMap<>(); + ServiceTags allServiceTags = rebuildOnlyIndex ? oldServiceTags : RangerServiceTagsDeltaUtil.applyDelta(oldServiceTags, serviceTags); if (serviceTags.getTagsChangeExtent() == ServiceTags.TagsChangeExtent.NONE) { if (LOG.isDebugEnabled()) { @@ -324,128 +285,15 @@ public class RangerTagEnricher extends RangerAbstractContextEnricher { } } else { if (serviceTags.getTagsChangeExtent() != ServiceTags.TagsChangeExtent.TAGS) { - if (LOG.isDebugEnabled()) { - LOG.debug("Delta contains changes other than tag attribute changes, [" + serviceTags.getTagsChangeExtent() + "]"); - } - - newServiceResourceTrie = new HashMap<>(); - newResourceMatchers = new ArrayList<>(); - - if (enrichedServiceTags != null) { - newResourceMatchers.addAll(enrichedServiceTags.getServiceResourceMatchers()); - } - - Map<String, RangerResourceTrie<RangerServiceResourceMatcher>> serviceResourceTrie = enrichedServiceTags != null ? enrichedServiceTags.getServiceResourceTrie() : new HashMap<>(); - - for (Map.Entry<String, RangerResourceTrie<RangerServiceResourceMatcher>> entry : serviceResourceTrie.entrySet()) { - RangerResourceTrie<RangerServiceResourceMatcher> resourceTrie = new RangerResourceTrie<>(entry.getValue()); - newServiceResourceTrie.put(entry.getKey(), resourceTrie); - } - - List<RangerServiceResource> changedServiceResources = delta.getServiceResources(); - - for (RangerServiceResource serviceResource : changedServiceResources) { - - 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()); - } - if (LOG.isDebugEnabled()) { - LOG.debug("RangerAccessResource:[" + accessResource + "] created to represent service-resource[" + serviceResource + "] to find evaluators from trie-map"); - } - - List<RangerServiceResourceMatcher> oldMatchers = getEvaluators(accessResource, enrichedServiceTags); - - if (LOG.isDebugEnabled()) { - LOG.debug("Found [" + oldMatchers.size() + "] matchers for service-resource[" + serviceResource + "]"); - } - - for (RangerServiceResourceMatcher matcher : oldMatchers) { - - for (String resourceDefName : serviceResource.getResourceElements().keySet()) { - RangerResourceTrie<RangerServiceResourceMatcher> trie = newServiceResourceTrie.get(resourceDefName); - if (trie != null) { - trie.delete(serviceResource.getResourceElements().get(resourceDefName), matcher); - } else { - LOG.error("Cannot find resourceDef with name:[" + resourceDefName + "]. Should NOT happen!!"); - LOG.error("Setting tagVersion to -1 to ensure that in the next download all tags are downloaded"); - newServiceTags.setTagVersion(-1L); - } - } - } - - // Remove old resource matchers - newResourceMatchers.removeAll(oldMatchers); - - if (LOG.isDebugEnabled()) { - LOG.debug("Found and removed [" + oldMatchers.size() + "] matchers for service-resource[" + serviceResource + "] from trie-map"); - } - } - - if (!StringUtils.isEmpty(serviceResource.getResourceSignature())) { - - RangerServiceResourceMatcher resourceMatcher = createRangerServiceResourceMatcher(serviceResource, serviceDefHelper, hierarchies); - - if (resourceMatcher != null) { - for (String resourceDefName : serviceResource.getResourceElements().keySet()) { - - RangerResourceTrie<RangerServiceResourceMatcher> trie = newServiceResourceTrie.get(resourceDefName); - - if (trie == null) { - List<RangerServiceDef.RangerResourceDef> resourceDefs = serviceDef.getResources(); - RangerServiceDef.RangerResourceDef found = null; - for (RangerServiceDef.RangerResourceDef resourceDef : resourceDefs) { - if (StringUtils.equals(resourceDef.getName(), resourceDefName)) { - found = resourceDef; - break; - } - } - if (found != null) { - List<RangerServiceResourceMatcher> resourceMatchers = new ArrayList<>(); - trie = new RangerResourceTrie<>(found, resourceMatchers); - newServiceResourceTrie.put(resourceDefName, trie); - } - } - - if (trie != null) { - trie.add(serviceResource.getResourceElements().get(resourceDefName), resourceMatcher); - if (LOG.isDebugEnabled()) { - LOG.debug("Added resource-matcher for service-resource:[" + serviceResource + "]"); - } - } else { - LOG.error("Could not create resource-matcher for resource: [" + serviceResource + "]. Should NOT happen!!"); - LOG.error("Setting tagVersion to -1 to ensure that in the next download all tags are downloaded"); - newServiceTags.setTagVersion(-1L); - } - } - newResourceMatchers.add(resourceMatcher); - } else { - LOG.error("Could not create resource-matcher for resource: [" + serviceResource + "]. Should NOT happen!!"); - LOG.error("Setting tagVersion to -1 to ensure that in the next download all tags are downloaded"); - newServiceTags.setTagVersion(-1L); - } - } else { - if (LOG.isDebugEnabled()) { - LOG.debug("Service-resource:[id=" + serviceResource.getId() + "] is deleted as its resource-signature is empty. No need to create it!"); - } - } - } + processServiceTagDeltas(serviceTags, allServiceTags); } else { if (LOG.isDebugEnabled()) { LOG.debug("Delta contains only tag attribute changes"); } + List<RangerServiceResourceMatcher> resourceMatchers = enrichedServiceTags != null ? enrichedServiceTags.getServiceResourceMatchers() : new ArrayList<>(); + Map<String, RangerResourceTrie<RangerServiceResourceMatcher>> serviceResourceTrie = enrichedServiceTags != null ? enrichedServiceTags.getServiceResourceTrie() : new HashMap<>(); + enrichedServiceTags = new EnrichedServiceTags(allServiceTags, resourceMatchers, serviceResourceTrie); } - - enrichedServiceTags = new EnrichedServiceTags(newServiceTags, newResourceMatchers, newServiceResourceTrie); } } RangerPerfTracer.logAlways(perf); @@ -499,6 +347,10 @@ public class RangerTagEnricher extends RangerAbstractContextEnricher { public boolean compare(RangerTagEnricher other) { boolean ret; + if (enrichedServiceTags == null || other == null || other.enrichedServiceTags == null) { + return false; + } + if (enrichedServiceTags.getServiceResourceTrie() != null && other.enrichedServiceTags.getServiceResourceTrie() != null) { ret = enrichedServiceTags.getServiceResourceTrie().size() == other.enrichedServiceTags.getServiceResourceTrie().size(); @@ -543,65 +395,192 @@ public class RangerTagEnricher extends RangerAbstractContextEnricher { return ret; } - private Set<RangerTagForEval> findMatchingTags(final RangerAccessRequest request, EnrichedServiceTags dataStore) { + private void processServiceTags(ServiceTags serviceTags) { if (LOG.isDebugEnabled()) { - LOG.debug("==> RangerTagEnricher.findMatchingTags(" + request + ")"); + LOG.debug("Processing all service-tags"); } - // To minimize chance for race condition between Tag-Refresher thread and access-evaluation thread - final EnrichedServiceTags enrichedServiceTags = dataStore != null ? dataStore : this.enrichedServiceTags; - - Set<RangerTagForEval> ret = null; + boolean isInError = false; - RangerAccessResource resource = request.getResource(); - - if ((resource == null || resource.getKeys() == null || resource.getKeys().isEmpty()) && request.isAccessTypeAny()) { - ret = enrichedServiceTags.getTagsForEmptyResourceAndAnyAccess(); + if (CollectionUtils.isEmpty(serviceTags.getServiceResources())) { + LOG.info("There are no tagged resources for service " + serviceName); + enrichedServiceTags = null; } else { - final List<RangerServiceResourceMatcher> serviceResourceMatchers = getEvaluators(resource, enrichedServiceTags); + RangerServiceDefHelper serviceDefHelper = new RangerServiceDefHelper(serviceDef, false); + ResourceHierarchies hierarchies = new ResourceHierarchies(); - if (CollectionUtils.isNotEmpty(serviceResourceMatchers)) { + List<RangerServiceResourceMatcher> resourceMatchers = new ArrayList<>(); + List<RangerServiceResource> serviceResources = serviceTags.getServiceResources(); - for (RangerServiceResourceMatcher resourceMatcher : serviceResourceMatchers) { + for (RangerServiceResource serviceResource : serviceResources) { + RangerServiceResourceMatcher serviceResourceMatcher = createRangerServiceResourceMatcher(serviceResource, serviceDefHelper, hierarchies); + if (serviceResourceMatcher != null) { + resourceMatchers.add(serviceResourceMatcher); + } else { + LOG.error("Could not create service-resource-matcher for service-resource:[" + serviceResource + "]"); + isInError = true; + break; + } + } - final RangerPolicyResourceMatcher.MatchType matchType = resourceMatcher.getMatchType(resource, request.getContext()); + if (isInError) { + serviceTags.setTagVersion(-1L); + LOG.error("Error in processing tag-deltas. Will continue to use old tags"); + } else { + Map<String, RangerResourceTrie<RangerServiceResourceMatcher>> serviceResourceTrie = null; - final boolean isMatched; + if (!disableTrieLookupPrefilter) { + serviceResourceTrie = new HashMap<>(); - if (request.isAccessTypeAny()) { - isMatched = matchType != RangerPolicyResourceMatcher.MatchType.NONE; - } else if (request.getResourceMatchingScope() == RangerAccessRequest.ResourceMatchingScope.SELF_OR_DESCENDANTS) { - isMatched = matchType != RangerPolicyResourceMatcher.MatchType.NONE; - } else { - isMatched = matchType == RangerPolicyResourceMatcher.MatchType.SELF || matchType == RangerPolicyResourceMatcher.MatchType.ANCESTOR; + for (RangerServiceDef.RangerResourceDef resourceDef : serviceDef.getResources()) { + serviceResourceTrie.put(resourceDef.getName(), new RangerResourceTrie<>(resourceDef, resourceMatchers)); } + } + enrichedServiceTags = new EnrichedServiceTags(serviceTags, resourceMatchers, serviceResourceTrie); + } + } + } - if (isMatched) { - if (ret == null) { - ret = new HashSet<>(); + private void processServiceTagDeltas(ServiceTags deltas, ServiceTags allServiceTags) { + if (LOG.isDebugEnabled()) { + LOG.debug("Delta contains changes other than tag attribute changes, [" + deltas.getTagsChangeExtent() + "]"); + } + + boolean isInError = false; + + RangerServiceDefHelper serviceDefHelper = new RangerServiceDefHelper(serviceDef, false); + ResourceHierarchies hierarchies = new ResourceHierarchies(); + + List<RangerServiceResourceMatcher> resourceMatchers = new ArrayList<>(); + Map<String, RangerResourceTrie<RangerServiceResourceMatcher>> serviceResourceTrie = new HashMap<>(); + + if (enrichedServiceTags != null) { + resourceMatchers.addAll(enrichedServiceTags.getServiceResourceMatchers()); + for (Map.Entry<String, RangerResourceTrie<RangerServiceResourceMatcher>> entry : enrichedServiceTags.getServiceResourceTrie().entrySet()) { + RangerResourceTrie<RangerServiceResourceMatcher> resourceTrie = new RangerResourceTrie<>(entry.getValue()); + serviceResourceTrie.put(entry.getKey(), resourceTrie); + } + } + + List<RangerServiceResource> changedServiceResources = deltas.getServiceResources(); + + for (RangerServiceResource serviceResource : changedServiceResources) { + + if (removeOldServiceResource(serviceResource, resourceMatchers, serviceResourceTrie)) { + + if (!StringUtils.isEmpty(serviceResource.getResourceSignature())) { + + RangerServiceResourceMatcher resourceMatcher = createRangerServiceResourceMatcher(serviceResource, serviceDefHelper, hierarchies); + + if (resourceMatcher != null) { + for (String resourceDefName : serviceResource.getResourceElements().keySet()) { + + RangerResourceTrie<RangerServiceResourceMatcher> trie = serviceResourceTrie.get(resourceDefName); + + if (trie == null) { + List<RangerServiceDef.RangerResourceDef> resourceDefs = serviceDef.getResources(); + RangerServiceDef.RangerResourceDef found = null; + for (RangerServiceDef.RangerResourceDef resourceDef : resourceDefs) { + if (StringUtils.equals(resourceDef.getName(), resourceDefName)) { + found = resourceDef; + break; + } + } + if (found != null) { + trie = new RangerResourceTrie<>(found, new ArrayList<>()); + serviceResourceTrie.put(resourceDefName, trie); + } + } + + if (trie != null) { + trie.add(serviceResource.getResourceElements().get(resourceDefName), resourceMatcher); + if (LOG.isDebugEnabled()) { + LOG.debug("Added resource-matcher for service-resource:[" + serviceResource + "]"); + } + } else { + LOG.error("Could not create resource-matcher for resource: [" + serviceResource + "]. Should NOT happen!!"); + LOG.error("Setting tagVersion to -1 to ensure that in the next download all tags are downloaded"); + isInError = true; + } } - ret.addAll(getTagsForServiceResource(enrichedServiceTags.getServiceTags(), resourceMatcher.getServiceResource(), matchType)); + resourceMatchers.add(resourceMatcher); + } else { + LOG.error("Could not create resource-matcher for resource: [" + serviceResource + "]. Should NOT happen!!"); + LOG.error("Setting tagVersion to -1 to ensure that in the next download all tags are downloaded"); + isInError = true; + } + } else { + if (LOG.isDebugEnabled()) { + LOG.debug("Service-resource:[id=" + serviceResource.getId() + "] is deleted as its resource-signature is empty. No need to create it!"); } - } + } else { + isInError = true; + } + if (isInError) { + break; } } + if (isInError) { + LOG.error("Error in processing tag-deltas. Will continue to use old tags"); + deltas.setTagVersion(-1L); + } else { + 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 (CollectionUtils.isEmpty(ret)) { if (LOG.isDebugEnabled()) { - LOG.debug("RangerTagEnricher.findMatchingTags(" + resource + ") - No tags Found "); + 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()); } - } else { if (LOG.isDebugEnabled()) { - LOG.debug("RangerTagEnricher.findMatchingTags(" + resource + ") - " + ret.size() + " tags Found "); + LOG.debug("RangerAccessResource:[" + accessResource + "] created to represent service-resource[" + serviceResource + "] to find evaluators from trie-map"); } - } - if (LOG.isDebugEnabled()) { - LOG.debug("<== RangerTagEnricher.findMatchingTags(" + request + ")"); - } + List<RangerServiceResourceMatcher> oldMatchers = getEvaluators(accessResource, enrichedServiceTags); + if (LOG.isDebugEnabled()) { + LOG.debug("Found [" + oldMatchers.size() + "] matchers for service-resource[" + serviceResource + "]"); + } + + for (RangerServiceResourceMatcher matcher : oldMatchers) { + + for (String resourceDefName : serviceResource.getResourceElements().keySet()) { + RangerResourceTrie<RangerServiceResourceMatcher> trie = resourceTries.get(resourceDefName); + if (trie != null) { + trie.delete(serviceResource.getResourceElements().get(resourceDefName), matcher); + } else { + LOG.error("Cannot find resourceDef with name:[" + resourceDefName + "]. Should NOT happen!!"); + LOG.error("Setting tagVersion to -1 to ensure that in the next download all tags are downloaded"); + ret = false; + break; + } + } + } + + // 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"); + } + } + } return ret; } @@ -677,6 +656,68 @@ public class RangerTagEnricher extends RangerAbstractContextEnricher { } } + private Set<RangerTagForEval> findMatchingTags(final RangerAccessRequest request, EnrichedServiceTags dataStore) { + if (LOG.isDebugEnabled()) { + LOG.debug("==> RangerTagEnricher.findMatchingTags(" + request + ")"); + } + + // To minimize chance for race condition between Tag-Refresher thread and access-evaluation thread + final EnrichedServiceTags enrichedServiceTags = dataStore != null ? dataStore : this.enrichedServiceTags; + + Set<RangerTagForEval> ret = null; + + RangerAccessResource resource = request.getResource(); + + if ((resource == null || resource.getKeys() == null || resource.getKeys().isEmpty()) && request.isAccessTypeAny()) { + ret = enrichedServiceTags.getTagsForEmptyResourceAndAnyAccess(); + } else { + + final List<RangerServiceResourceMatcher> serviceResourceMatchers = getEvaluators(resource, enrichedServiceTags); + + if (CollectionUtils.isNotEmpty(serviceResourceMatchers)) { + + for (RangerServiceResourceMatcher resourceMatcher : serviceResourceMatchers) { + + final RangerPolicyResourceMatcher.MatchType matchType = resourceMatcher.getMatchType(resource, request.getContext()); + + final boolean isMatched; + + if (request.isAccessTypeAny()) { + isMatched = matchType != RangerPolicyResourceMatcher.MatchType.NONE; + } else if (request.getResourceMatchingScope() == RangerAccessRequest.ResourceMatchingScope.SELF_OR_DESCENDANTS) { + isMatched = matchType != RangerPolicyResourceMatcher.MatchType.NONE; + } else { + isMatched = matchType == RangerPolicyResourceMatcher.MatchType.SELF || matchType == RangerPolicyResourceMatcher.MatchType.ANCESTOR; + } + + if (isMatched) { + if (ret == null) { + ret = new HashSet<>(); + } + ret.addAll(getTagsForServiceResource(enrichedServiceTags.getServiceTags(), resourceMatcher.getServiceResource(), matchType)); + } + + } + } + } + + if (CollectionUtils.isEmpty(ret)) { + if (LOG.isDebugEnabled()) { + LOG.debug("RangerTagEnricher.findMatchingTags(" + resource + ") - No tags Found "); + } + } else { + if (LOG.isDebugEnabled()) { + LOG.debug("RangerTagEnricher.findMatchingTags(" + resource + ") - " + ret.size() + " tags Found "); + } + } + + if (LOG.isDebugEnabled()) { + LOG.debug("<== RangerTagEnricher.findMatchingTags(" + request + ")"); + } + + return ret; + } + private List<RangerServiceResourceMatcher> getEvaluators(RangerAccessResource resource, EnrichedServiceTags enrichedServiceTags) { if(LOG.isDebugEnabled()) { LOG.debug("==> RangerTagEnricher.getEvaluators(" + (resource != null ? resource.getAsString() : null) + ")"); @@ -901,7 +942,7 @@ public class RangerTagEnricher extends RangerAbstractContextEnricher { if (serviceTags != null) { tagEnricher.setServiceTags(serviceTags); - if (serviceTags.getIsDelta()) { + if (serviceTags.getIsDelta() && serviceTags.getTagVersion() != -1L) { saveToCache(tagEnricher.enrichedServiceTags.serviceTags); } LOG.info("RangerTagRefresher.populateTags() - Updated tags-cache to new version of tags, lastKnownVersion=" + lastKnownVersion + "; newVersion=" diff --git a/security-admin/src/main/java/org/apache/ranger/db/XXPolicyChangeLogDao.java b/security-admin/src/main/java/org/apache/ranger/db/XXPolicyChangeLogDao.java index 7055391..0a1d1c1 100644 --- a/security-admin/src/main/java/org/apache/ranger/db/XXPolicyChangeLogDao.java +++ b/security-admin/src/main/java/org/apache/ranger/db/XXPolicyChangeLogDao.java @@ -41,6 +41,14 @@ public class XXPolicyChangeLogDao extends BaseDao<XXPolicyChangeLog> { private static final Log LOG = LogFactory.getLog(XXPolicyChangeLogDao.class); + private static final int POLICY_CHANGE_LOG_RECORD_ID_COLUMN_NUMBER = 0; + private static final int POLICY_CHANGE_LOG_RECORD_CHANGE_TYPE_COLUMN_NUMBER = 1; + private static final int POLICY_CHANGE_LOG_RECORD_POLICY_VERSION_COLUMN_NUMBER = 2; + private static final int POLICY_CHANGE_LOG_RECORD_SERVICE_TYPE_COLUMN_NUMBER = 3; + private static final int POLICY_CHANGE_LOG_RECORD_POLICY_TYPE_COLUMN_NUMBER = 4; + private static final int POLICY_CHANGE_LOG_RECORD_POLICY_ID_COLUMN_NUMBER = 5; + private static final int POLICY_CHANGE_LOG_RECORD_ZONE_NAME_COLUMN_NUMBER = 6; + /** * Default Constructor */ @@ -57,14 +65,14 @@ public class XXPolicyChangeLogDao extends BaseDao<XXPolicyChangeLog> { .setParameter("serviceId", serviceId) .getResultList(); - // Ensure that some record has the same version as the base-version from where the records are fetched + // Ensure that first record has the same version as the base-version from where the records are fetched if (CollectionUtils.isNotEmpty(logs)) { Iterator<Object[]> iter = logs.iterator(); boolean foundAndRemoved = false; while (iter.hasNext()) { Object[] record = iter.next(); - Long recordVersion = (Long) record[2]; + Long recordVersion = (Long) record[POLICY_CHANGE_LOG_RECORD_POLICY_VERSION_COLUMN_NUMBER]; if (version.equals(recordVersion)) { iter.remove(); foundAndRemoved = true; @@ -124,10 +132,10 @@ public class XXPolicyChangeLogDao extends BaseDao<XXPolicyChangeLog> { RangerPolicy policy; - Long logRecordId = (Long) log[0]; - Integer policyChangeType = (Integer) log[1]; - String serviceType = (String) log[3]; - Long policyId = (Long) log[5]; + Long logRecordId = (Long) log[POLICY_CHANGE_LOG_RECORD_ID_COLUMN_NUMBER]; + Integer policyChangeType = (Integer) log[POLICY_CHANGE_LOG_RECORD_CHANGE_TYPE_COLUMN_NUMBER]; + String serviceType = (String) log[POLICY_CHANGE_LOG_RECORD_SERVICE_TYPE_COLUMN_NUMBER]; + Long policyId = (Long) log[POLICY_CHANGE_LOG_RECORD_POLICY_ID_COLUMN_NUMBER]; if (policyId != null) { XXPolicy xxPolicy = daoManager.getXXPolicy().getById(policyId); @@ -148,16 +156,16 @@ public class XXPolicyChangeLogDao extends BaseDao<XXPolicyChangeLog> { // Create a dummy policy as the policy cannot be found - probably already deleted policy = new RangerPolicy(); policy.setId(policyId); - policy.setVersion((Long) log[2]); - policy.setPolicyType((Integer) log[4]); - policy.setZoneName((String) log[6]); + policy.setVersion((Long) log[POLICY_CHANGE_LOG_RECORD_POLICY_VERSION_COLUMN_NUMBER]); + policy.setPolicyType((Integer) log[POLICY_CHANGE_LOG_RECORD_POLICY_TYPE_COLUMN_NUMBER]); + policy.setZoneName((String) log[POLICY_CHANGE_LOG_RECORD_ZONE_NAME_COLUMN_NUMBER]); } policy.setServiceType(serviceType); ret.add(new RangerPolicyDelta(logRecordId, policyChangeType, policy)); } else { if (LOG.isDebugEnabled()) { - LOG.debug("policyId is null! log-record-id:[" + logRecordId + ", service-type:[" + log[3] + "], policy-change-type:[" + log[1] + "]"); + LOG.debug("policyId is null! log-record-id:[" + logRecordId + ", service-type:[" + log[POLICY_CHANGE_LOG_RECORD_SERVICE_TYPE_COLUMN_NUMBER] + "], policy-change-type:[" + log[POLICY_CHANGE_LOG_RECORD_CHANGE_TYPE_COLUMN_NUMBER] + "]"); } ret.clear(); ret.add(new RangerPolicyDelta(logRecordId, policyChangeType, null)); diff --git a/security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java b/security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java index 9f6f024..b18f8f2 100644 --- a/security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java +++ b/security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java @@ -22,6 +22,8 @@ import java.util.List; import javax.persistence.NoResultException; import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.apache.ranger.biz.ServiceDBStore; import org.apache.ranger.common.db.BaseDao; import org.apache.ranger.entity.XXServiceVersionInfo; @@ -33,6 +35,8 @@ import org.springframework.stereotype.Service; @Service public class XXServiceVersionInfoDao extends BaseDao<XXServiceVersionInfo> { + private static final Log LOG = LogFactory.getLog(XXServiceVersionInfoDao.class); + /** * Default Constructor */ @@ -74,6 +78,7 @@ public class XXServiceVersionInfoDao extends BaseDao<XXServiceVersionInfo> { public void updateServiceVersionInfoForTagResourceMapCreate(Long resourceId, Long tagId) { if (resourceId == null || tagId == null) { + LOG.warn("Unexpected null value for resourceId and/or tagId"); return; } @@ -82,12 +87,12 @@ public class XXServiceVersionInfoDao extends BaseDao<XXServiceVersionInfo> { updateTagVersionAndTagUpdateTime(serviceVersionInfos, resourceId, tagId); } catch (NoResultException e) { - return; } } public void updateServiceVersionInfoForTagResourceMapDelete(Long resourceId, Long tagId) { if (resourceId == null || tagId == null) { + LOG.warn("Unexpected null value for resourceId and/or tagId"); return; } @@ -96,11 +101,11 @@ public class XXServiceVersionInfoDao extends BaseDao<XXServiceVersionInfo> { updateTagVersionAndTagUpdateTime(serviceVersionInfos, resourceId, tagId); } catch (NoResultException e) { - return; } } public void updateServiceVersionInfoForServiceResourceUpdate(Long resourceId) { if (resourceId == null) { + LOG.warn("Unexpected null value for resourceId"); return; } @@ -111,12 +116,12 @@ public class XXServiceVersionInfoDao extends BaseDao<XXServiceVersionInfo> { updateTagVersionAndTagUpdateTime(serviceVersionInfos, resourceId, tagId); } catch (NoResultException e) { - return; } } public void updateServiceVersionInfoForTagUpdate(Long tagId) { if (tagId == null) { + LOG.warn("Unexpected null value for tagId"); return; } @@ -126,13 +131,6 @@ public class XXServiceVersionInfoDao extends BaseDao<XXServiceVersionInfo> { updateTagVersionAndTagUpdateTime(serviceVersionInfos, resourceId, tagId); } catch (NoResultException e) { - return; - } - } - - public void updateServiceVersionInfoForTagDefUpdate(Long tagDefId) { - if (tagDefId != null) { - return; } } @@ -140,23 +138,26 @@ public class XXServiceVersionInfoDao extends BaseDao<XXServiceVersionInfo> { if(CollectionUtils.isNotEmpty(serviceVersionInfos) || (resourceId == null && tagId == null)) { - for (XXServiceVersionInfo serviceVersionInfo : serviceVersionInfos) { + final ServiceDBStore.VERSION_TYPE versionType = ServiceDBStore.VERSION_TYPE.TAG_VERSION; + final ServiceTags.TagsChangeType tagChangeType; - final Long serviceId = serviceVersionInfo.getServiceId(); - final ServiceDBStore.VERSION_TYPE versionType = ServiceDBStore.VERSION_TYPE.TAG_VERSION; - final ServiceTags.TagsChangeType tagChangeType; + if (tagId == null) { + tagChangeType = ServiceTags.TagsChangeType.SERVICE_RESOURCE_UPDATE; + } else if (resourceId == null) { + tagChangeType = ServiceTags.TagsChangeType.TAG_UPDATE; + } else { + tagChangeType = ServiceTags.TagsChangeType.TAG_RESOURCE_MAP_UPDATE; + } - if (tagId == null) { - tagChangeType = ServiceTags.TagsChangeType.SERVICE_RESOURCE_UPDATE; - } else if (resourceId == null) { - tagChangeType = ServiceTags.TagsChangeType.TAG_UPDATE; - } else { - tagChangeType = ServiceTags.TagsChangeType.TAG_RESOURCE_MAP_UPDATE; - } + for (XXServiceVersionInfo serviceVersionInfo : serviceVersionInfos) { + final Long serviceId = serviceVersionInfo.getServiceId(); final Runnable serviceVersionUpdater = new ServiceDBStore.ServiceVersionUpdater(daoManager, serviceId, versionType, tagChangeType, resourceId, tagId); + daoManager.getRangerTransactionSynchronizationAdapter().executeOnTransactionCommit(serviceVersionUpdater); } + } else { + LOG.warn("Unexpected empty list of serviceVersionInfos and/or null value for resourceId and tagId"); } } diff --git a/security-admin/src/main/java/org/apache/ranger/db/XXTagChangeLogDao.java b/security-admin/src/main/java/org/apache/ranger/db/XXTagChangeLogDao.java index ef65e9a..2ca2200 100644 --- a/security-admin/src/main/java/org/apache/ranger/db/XXTagChangeLogDao.java +++ b/security-admin/src/main/java/org/apache/ranger/db/XXTagChangeLogDao.java @@ -37,6 +37,12 @@ public class XXTagChangeLogDao extends BaseDao<XXTagChangeLog> { private static final Log LOG = LogFactory.getLog(XXTagChangeLogDao.class); + private static final int TAG_CHANGE_LOG_RECORD_ID_COLUMN_NUMBER = 0; + private static final int TAG_CHANGE_LOG_RECORD_CHANGE_TYPE_COLUMN_NUMBER = 1; + private static final int TAG_CHANGE_LOG_RECORD_VERSION_ID_COLUMN_NUMBER = 2; + private static final int TAG_CHANGE_LOG_RECORD_SERVICE_RESOURCE_ID_COLUMN_NUMBER = 3; + private static final int TAG_CHANGE_LOG_RECORD_TAG_ID_COLUMN_NUMBER = 4; + /** * Default Constructor */ @@ -52,14 +58,14 @@ public class XXTagChangeLogDao extends BaseDao<XXTagChangeLog> { .setParameter("version", version) .setParameter("serviceId", serviceId) .getResultList(); - // Ensure that some record has the same version as the base-version from where the records are fetched + // Ensure that first record has the same version as the base-version from where the records are fetched if (CollectionUtils.isNotEmpty(logs)) { Iterator<Object[]> iter = logs.iterator(); boolean foundAndRemoved = false; while (iter.hasNext()) { Object[] record = iter.next(); - Long recordVersion = (Long) record[2]; + Long recordVersion = (Long) record[TAG_CHANGE_LOG_RECORD_VERSION_ID_COLUMN_NUMBER]; if (version.equals(recordVersion)) { iter.remove(); foundAndRemoved = true; @@ -103,11 +109,11 @@ public class XXTagChangeLogDao extends BaseDao<XXTagChangeLog> { for (Object[] log : queryResult) { - Long logRecordId = (Long) log[0]; - Integer tagChangeType = (Integer) log[1]; - Long serviceTagsVersion = (Long) log[2]; - Long serviceResourceId = (Long) log[3]; - Long tagId = (Long) log[4]; + Long logRecordId = (Long) log[TAG_CHANGE_LOG_RECORD_ID_COLUMN_NUMBER]; + Integer tagChangeType = (Integer) log[TAG_CHANGE_LOG_RECORD_CHANGE_TYPE_COLUMN_NUMBER]; + Long serviceTagsVersion = (Long) log[TAG_CHANGE_LOG_RECORD_VERSION_ID_COLUMN_NUMBER]; + Long serviceResourceId = (Long) log[TAG_CHANGE_LOG_RECORD_SERVICE_RESOURCE_ID_COLUMN_NUMBER]; + Long tagId = (Long) log[TAG_CHANGE_LOG_RECORD_TAG_ID_COLUMN_NUMBER]; ret.add(new XXTagChangeLog(logRecordId, tagChangeType, serviceTagsVersion, serviceResourceId, tagId)); } diff --git a/security-admin/src/main/java/org/apache/ranger/entity/XXPolicyChangeLog.java b/security-admin/src/main/java/org/apache/ranger/entity/XXPolicyChangeLog.java index df87f70..bc48d4a 100644 --- a/security-admin/src/main/java/org/apache/ranger/entity/XXPolicyChangeLog.java +++ b/security-admin/src/main/java/org/apache/ranger/entity/XXPolicyChangeLog.java @@ -20,6 +20,7 @@ package org.apache.ranger.entity; import java.util.Date; +import java.util.Objects; import javax.persistence.Cacheable; import javax.persistence.Entity; @@ -188,45 +189,12 @@ public class XXPolicyChangeLog implements java.io.Serializable { return false; if (getClass() != obj.getClass()) return false; + XXPolicyChangeLog other = (XXPolicyChangeLog) obj; - if ((this.id == null && other.id != null) || (this.id != null && !this.id.equals(other.id))) { - return false; - } - if ((this.serviceId == null && other.serviceId != null) || (this.serviceId != null && !this.serviceId.equals(other.serviceId))) { - return false; - } - if ((this.policyVersion == null && other.policyVersion != null) || (this.policyVersion != null && !this.policyVersion.equals(other.policyVersion))) { - return false; - } - if ((this.createTime == null && other.createTime != null) || (this.createTime != null && !this.createTime.equals(other.createTime))) { - return false; - } - if ((this.changeType == null && other.changeType != null) || (this.changeType != null && !this.changeType.equals(other.changeType))) { - return false; - } - if ((this.serviceType == null && other.serviceType != null) || (this.serviceType != null && !this.serviceType.equals(other.serviceType))) { - return false; - } - if ((this.policyType == null && other.policyType != null) || (this.policyType != null && !this.policyType.equals(other.policyType))) { - return false; - } - if ((this.zoneName == null && other.zoneName != null) || (this.zoneName != null && !this.zoneName.equals(other.zoneName))) { - return false; - } - if ((this.policyId == null && other.policyId != null) || (this.policyId != null && !this.policyId.equals(other.policyId))) { - return false; - } - return true; - } - public static boolean equals(Object object1, Object object2) { - if (object1 == object2) { - return true; - } - if ((object1 == null) || (object2 == null)) { - return false; - } - return object1.equals(object2); + return Objects.equals(this.id, other.id) && Objects.equals(this.serviceId, other.serviceId) && Objects.equals(this.policyVersion, other.policyVersion) + && Objects.equals(this.createTime, other.createTime) && Objects.equals(this.changeType, other.changeType) && Objects.equals(this.serviceType, other.serviceType) + && Objects.equals(this.policyType, other.policyType) && Objects.equals(this.zoneName, other.zoneName) && Objects.equals(this.policyId, other.policyId); } } diff --git a/security-admin/src/main/java/org/apache/ranger/entity/XXTagChangeLog.java b/security-admin/src/main/java/org/apache/ranger/entity/XXTagChangeLog.java index c3eb144..9f1002c 100644 --- a/security-admin/src/main/java/org/apache/ranger/entity/XXTagChangeLog.java +++ b/security-admin/src/main/java/org/apache/ranger/entity/XXTagChangeLog.java @@ -20,6 +20,7 @@ package org.apache.ranger.entity; import java.util.Date; +import java.util.Objects; import javax.persistence.Cacheable; import javax.persistence.Entity; @@ -179,39 +180,12 @@ public class XXTagChangeLog implements java.io.Serializable { return false; if (getClass() != obj.getClass()) return false; + XXTagChangeLog other = (XXTagChangeLog) obj; - if ((this.id == null && other.id != null) || (this.id != null && !this.id.equals(other.id))) { - return false; - } - if ((this.createTime == null && other.createTime != null) || (this.createTime != null && !this.createTime.equals(other.createTime))) { - return false; - } - if ((this.serviceId == null && other.serviceId != null) || (this.serviceId != null && !this.serviceId.equals(other.serviceId))) { - return false; - } - if ((this.changeType == null && other.changeType != null) || (this.changeType != null && !this.changeType.equals(other.changeType))) { - return false; - } - if ((this.serviceTagsVersion == null && other.serviceTagsVersion != null) || (this.serviceTagsVersion != null && !this.serviceTagsVersion.equals(other.serviceTagsVersion))) { - return false; - } - if ((this.serviceResourceId == null && other.serviceResourceId != null) || (this.serviceResourceId != null && !this.serviceResourceId.equals(other.serviceResourceId))) { - return false; - } - if ((this.tagId == null && other.tagId != null) || (this.tagId != null && !this.tagId.equals(other.tagId))) { - return false; - } - return true; - } - public static boolean equals(Object object1, Object object2) { - if (object1 == object2) { - return true; - } - if ((object1 == null) || (object2 == null)) { - return false; - } - return object1.equals(object2); + return Objects.equals(this.id, other.id) && Objects.equals(this.createTime, other.createTime) && Objects.equals(this.serviceId, other.serviceId) + && Objects.equals(this.changeType, other.changeType) && Objects.equals(this.serviceTagsVersion, other.serviceTagsVersion) + && Objects.equals(this.serviceResourceId, other.serviceResourceId) && Objects.equals(this.tagId, other.tagId); } } diff --git a/security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java b/security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java index c33841d..f329d17 100644 --- a/security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java +++ b/security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java @@ -62,6 +62,9 @@ public class PublicAPIsv2 { ServiceREST serviceREST; @Autowired + TagREST tagREST; + + @Autowired SecurityZoneREST securityZoneRest; @Autowired @@ -527,15 +530,30 @@ public class PublicAPIsv2 { @DELETE @Path("/api/server/policydeltas") @PreAuthorize("hasRole('ROLE_SYS_ADMIN')") - public void deletePolicyDeltas(@DefaultValue("7") @QueryParam("days") Integer olderThan, @DefaultValue("false") @QueryParam("reloadServicePoliciesCache") Boolean reloadServicePoliciesCache, @Context HttpServletRequest request) { + public void deletePolicyDeltas(@DefaultValue("7") @QueryParam("days") Integer olderThan, @Context HttpServletRequest request) { if (logger.isDebugEnabled()) { - logger.debug("==> PublicAPIsv2.deletePolicyDeltas(" + olderThan + ", " + reloadServicePoliciesCache + ")"); + logger.debug("==> PublicAPIsv2.deletePolicyDeltas(" + olderThan + ")"); } serviceREST.deletePolicyDeltas(olderThan, request); if (logger.isDebugEnabled()) { - logger.debug("<== PublicAPIsv2.deletePolicyDeltas(" + olderThan + ", " + reloadServicePoliciesCache + ")"); + logger.debug("<== PublicAPIsv2.deletePolicyDeltas(" + olderThan + ")"); + } + } + + @DELETE + @Path("/api/server/tagdeltas") + @PreAuthorize("hasRole('ROLE_SYS_ADMIN')") + public void deleteTagDeltas(@DefaultValue("7") @QueryParam("days") Integer olderThan, @Context HttpServletRequest request) { + if (logger.isDebugEnabled()) { + logger.debug("==> PublicAPIsv2.deleteTagDeltas(" + olderThan + ")"); + } + + tagREST.deleteTagDeltas(olderThan, request); + + if (logger.isDebugEnabled()) { + logger.debug("<== PublicAPIsv2.deleteTagDeltas(" + olderThan + ")"); } } diff --git a/security-admin/src/main/java/org/apache/ranger/service/RangerTagDefService.java b/security-admin/src/main/java/org/apache/ranger/service/RangerTagDefService.java index d4350b6..8f67799 100644 --- a/security-admin/src/main/java/org/apache/ranger/service/RangerTagDefService.java +++ b/security-admin/src/main/java/org/apache/ranger/service/RangerTagDefService.java @@ -52,15 +52,6 @@ public class RangerTagDefService extends RangerTagDefServiceBase<XXTagDef, Range } - @Override - public RangerTagDef postUpdate(XXTagDef tagDef) { - RangerTagDef ret = super.postUpdate(tagDef); - - daoMgr.getXXServiceVersionInfo().updateServiceVersionInfoForTagDefUpdate(tagDef.getId()); - - return ret; - } - public RangerTagDef getPopulatedViewObject(XXTagDef xObj) { return populateViewBean(xObj); } diff --git a/security-admin/src/main/resources/META-INF/jpa_named_queries.xml b/security-admin/src/main/resources/META-INF/jpa_named_queries.xml index 9714fa9..ab8e675 100755 --- a/security-admin/src/main/resources/META-INF/jpa_named_queries.xml +++ b/security-admin/src/main/resources/META-INF/jpa_named_queries.xml @@ -1563,11 +1563,6 @@ <!-- XXTagChangeLog --> - <named-query name="XXTagChangeLog.findByServiceId"> - <query> - select obj from XXTagChangeLog obj where obj.serviceId = :serviceId - </query> - </named-query> <named-query name="XXTagChangeLog.findSinceVersion"> <query> select obj.id, obj.changeType, obj.serviceTagsVersion, obj.serviceResourceId, obj.tagId from diff --git a/security-admin/src/test/java/org/apache/ranger/service/TestRangerTagDefService.java b/security-admin/src/test/java/org/apache/ranger/service/TestRangerTagDefService.java index 160a53c..c031c94 100644 --- a/security-admin/src/test/java/org/apache/ranger/service/TestRangerTagDefService.java +++ b/security-admin/src/test/java/org/apache/ranger/service/TestRangerTagDefService.java @@ -21,7 +21,6 @@ import java.util.Date; import java.util.List; import org.apache.ranger.db.RangerDaoManager; -import org.apache.ranger.db.XXServiceVersionInfoDao; import org.apache.ranger.db.XXTagDefDao; import org.apache.ranger.entity.XXTagAttributeDef; import org.apache.ranger.entity.XXTagDef; @@ -79,18 +78,11 @@ public class TestRangerTagDefService { xxTagAttributeDef.setId(id); xxTagAttributeDef.setName(name); tagAttrDefList.add(xxTagAttributeDef); - - XXServiceVersionInfoDao xxServiceVersionInfoDao = Mockito.mock(XXServiceVersionInfoDao.class); - Mockito.when(daoMgr.getXXServiceVersionInfo()).thenReturn(xxServiceVersionInfoDao); - Mockito.doNothing().when(xxServiceVersionInfoDao).updateServiceVersionInfoForTagDefUpdate(tagDef.getId()); - RangerTagDef result = rangerTagDefService.postUpdate(tagDef); Assert.assertEquals(result.getId(), tagAttrDefList.get(0).getId()); Assert.assertEquals(result.getName(), tagAttrDefList.get(0).getName()); - Mockito.verify(daoMgr).getXXServiceVersionInfo(); - Mockito.verify(xxServiceVersionInfoDao).updateServiceVersionInfoForTagDefUpdate(tagDef.getId()); } @Test