Repository: incubator-ranger Updated Branches: refs/heads/master 01e1d0592 -> 220c13237
RANGER-1009: Add checks to ensure that ranger-admin returns correct service-policies if service-name is reused Signed-off-by: Madhan Neethiraj <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/incubator-ranger/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-ranger/commit/220c1323 Tree: http://git-wip-us.apache.org/repos/asf/incubator-ranger/tree/220c1323 Diff: http://git-wip-us.apache.org/repos/asf/incubator-ranger/diff/220c1323 Branch: refs/heads/master Commit: 220c13237589e4f3b624bf39074d800d895be909 Parents: 01e1d05 Author: Abhay Kulkarni <[email protected]> Authored: Tue May 31 13:01:23 2016 -0700 Committer: Madhan Neethiraj <[email protected]> Committed: Wed Jun 1 11:16:07 2016 -0700 ---------------------------------------------------------------------- .../org/apache/ranger/biz/ServiceDBStore.java | 4 +- .../java/org/apache/ranger/biz/TagDBStore.java | 2 +- .../common/RangerServicePoliciesCache.java | 54 ++++++++++---------- .../ranger/common/RangerServiceTagsCache.java | 51 +++++++++--------- 4 files changed, 57 insertions(+), 54 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/220c1323/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java ---------------------------------------------------------------------- diff --git a/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java b/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java index d2178f4..a659d45 100644 --- a/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java +++ b/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java @@ -2109,7 +2109,7 @@ public class ServiceDBStore extends AbstractServiceStore { List<RangerPolicy> ret = null; - ServicePolicies servicePolicies = RangerServicePoliciesCache.getInstance().getServicePolicies(service.getName(), this); + ServicePolicies servicePolicies = RangerServicePoliciesCache.getInstance().getServicePolicies(service.getName(), service.getId(), this); List<RangerPolicy> policies = servicePolicies != null ? servicePolicies.getPolicies() : null; if(policies != null && filter != null) { @@ -2184,7 +2184,7 @@ public class ServiceDBStore extends AbstractServiceStore { } if (lastKnownVersion == null || serviceVersionInfoDbObj == null || serviceVersionInfoDbObj.getPolicyVersion() == null || !lastKnownVersion.equals(serviceVersionInfoDbObj.getPolicyVersion())) { - ret = RangerServicePoliciesCache.getInstance().getServicePolicies(serviceName, this); + ret = RangerServicePoliciesCache.getInstance().getServicePolicies(serviceName, serviceDbObj.getId(), this); } if (ret != null && lastKnownVersion != null && lastKnownVersion.equals(ret.getPolicyVersion())) { http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/220c1323/security-admin/src/main/java/org/apache/ranger/biz/TagDBStore.java ---------------------------------------------------------------------- diff --git a/security-admin/src/main/java/org/apache/ranger/biz/TagDBStore.java b/security-admin/src/main/java/org/apache/ranger/biz/TagDBStore.java index 5f298d9..7fe0654 100644 --- a/security-admin/src/main/java/org/apache/ranger/biz/TagDBStore.java +++ b/security-admin/src/main/java/org/apache/ranger/biz/TagDBStore.java @@ -917,7 +917,7 @@ public class TagDBStore extends AbstractTagStore { } if (lastKnownVersion == null || serviceVersionInfoDbObj == null || serviceVersionInfoDbObj.getTagVersion() == null || !lastKnownVersion.equals(serviceVersionInfoDbObj.getTagVersion())) { - ret = RangerServiceTagsCache.getInstance().getServiceTags(serviceName, this); + ret = RangerServiceTagsCache.getInstance().getServiceTags(serviceName, xxService.getId(), this); } if (ret != null && lastKnownVersion != null && lastKnownVersion.equals(ret.getTagVersion())) { http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/220c1323/security-admin/src/main/java/org/apache/ranger/common/RangerServicePoliciesCache.java ---------------------------------------------------------------------- diff --git a/security-admin/src/main/java/org/apache/ranger/common/RangerServicePoliciesCache.java b/security-admin/src/main/java/org/apache/ranger/common/RangerServicePoliciesCache.java index b712f09..a68b215 100644 --- a/security-admin/src/main/java/org/apache/ranger/common/RangerServicePoliciesCache.java +++ b/security-admin/src/main/java/org/apache/ranger/common/RangerServicePoliciesCache.java @@ -29,7 +29,11 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.ranger.plugin.util.ServicePolicies; -import java.util.*; +import java.util.Date; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.ReentrantLock; @@ -82,32 +86,15 @@ public class RangerServicePoliciesCache { } } - public ServicePolicies getServicePolicies(String serviceName) { - - ServicePolicies ret = null; - - if (useServicePoliciesCache && StringUtils.isNotBlank(serviceName)) { - ServicePoliciesWrapper cachedServicePoliciesWrapper = null; - synchronized (this) { - cachedServicePoliciesWrapper = servicePoliciesMap.get(serviceName); - } - if (cachedServicePoliciesWrapper != null) { - ret = cachedServicePoliciesWrapper.getServicePolicies(); - } - } - - return ret; - } - - public ServicePolicies getServicePolicies(String serviceName, ServiceStore serviceStore) throws Exception { + public ServicePolicies getServicePolicies(String serviceName, Long serviceId, ServiceStore serviceStore) throws Exception { if (LOG.isDebugEnabled()) { - LOG.debug("==> RangerServicePoliciesCache.getServicePolicies(" + serviceName + ")"); + LOG.debug("==> RangerServicePoliciesCache.getServicePolicies(" + serviceName + ", " + serviceId + ")"); } ServicePolicies ret = null; - if (StringUtils.isNotBlank(serviceName)) { + if (StringUtils.isNotBlank(serviceName) && serviceId != null) { if (LOG.isDebugEnabled()) { LOG.debug("useServicePoliciesCache=" + useServicePoliciesCache); @@ -131,8 +118,20 @@ public class RangerServicePoliciesCache { synchronized (this) { servicePoliciesWrapper = servicePoliciesMap.get(serviceName); + if (servicePoliciesWrapper != null) { + if (!serviceId.equals(servicePoliciesWrapper.getServiceId())) { + if (LOG.isDebugEnabled()) { + LOG.debug("Service [" + serviceName + "] changed service-id from " + servicePoliciesWrapper.getServiceId() + + " to " + serviceId); + LOG.debug("Recreating servicePoliciesWrapper for serviceName [" + serviceName + "]"); + } + servicePoliciesMap.remove(serviceName); + servicePoliciesWrapper = null; + } + } + if (servicePoliciesWrapper == null) { - servicePoliciesWrapper = new ServicePoliciesWrapper(); + servicePoliciesWrapper = new ServicePoliciesWrapper(serviceId); servicePoliciesMap.put(serviceName, servicePoliciesWrapper); } } @@ -153,27 +152,31 @@ public class RangerServicePoliciesCache { ret = servicePolicies; } else { - LOG.error("getServicePolicies() failed to get policies as serviceName is null or blank!"); + LOG.error("getServicePolicies() failed to get policies as serviceName is null or blank and/or serviceId is null!"); } if (LOG.isDebugEnabled()) { - LOG.debug("<== RangerServicePoliciesCache.getServicePolicies(" + serviceName + "): count=" + ((ret == null || ret.getPolicies() == null) ? 0 : ret.getPolicies().size())); + LOG.debug("<== RangerServicePoliciesCache.getServicePolicies(" + serviceName + ", " + serviceId + "): count=" + ((ret == null || ret.getPolicies() == null) ? 0 : ret.getPolicies().size())); } return ret; } private class ServicePoliciesWrapper { + final Long serviceId; ServicePolicies servicePolicies; Date updateTime = null; long longestDbLoadTimeInMs = -1; ReentrantLock lock = new ReentrantLock(); - ServicePoliciesWrapper() { + ServicePoliciesWrapper(Long serviceId) { + this.serviceId = serviceId; servicePolicies = null; } + Long getServiceId() { return serviceId; } + ServicePolicies getServicePolicies() { return servicePolicies; } @@ -217,7 +220,6 @@ public class RangerServicePoliciesCache { Long servicePolicyVersionInDb = serviceStore.getServicePolicyVersion(serviceName); - if (servicePolicies == null || servicePolicyVersionInDb == null || !servicePolicyVersionInDb.equals(servicePolicies.getPolicyVersion())) { if (LOG.isDebugEnabled()) { LOG.debug("loading servicePolicies from db ... cachedServicePoliciesVersion=" + (servicePolicies != null ? servicePolicies.getPolicyVersion() : null) + ", servicePolicyVersionInDb=" + servicePolicyVersionInDb); http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/220c1323/security-admin/src/main/java/org/apache/ranger/common/RangerServiceTagsCache.java ---------------------------------------------------------------------- diff --git a/security-admin/src/main/java/org/apache/ranger/common/RangerServiceTagsCache.java b/security-admin/src/main/java/org/apache/ranger/common/RangerServiceTagsCache.java index 15af0f0..66c1562 100644 --- a/security-admin/src/main/java/org/apache/ranger/common/RangerServiceTagsCache.java +++ b/security-admin/src/main/java/org/apache/ranger/common/RangerServiceTagsCache.java @@ -30,7 +30,10 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.ranger.plugin.util.ServiceTags; -import java.util.*; +import java.util.Date; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.ReentrantLock; @@ -83,32 +86,15 @@ public class RangerServiceTagsCache { } } - public ServiceTags getServiceTags(String serviceName) { - - ServiceTags ret = null; - - if (useServiceTagsCache && StringUtils.isNotBlank(serviceName)) { - ServiceTagsWrapper cachedServiceTagsWrapper = null; - synchronized (this) { - cachedServiceTagsWrapper = serviceTagsMap.get(serviceName); - } - if (cachedServiceTagsWrapper != null) { - ret = cachedServiceTagsWrapper.getServiceTags(); - } - } - - return ret; - } - - public ServiceTags getServiceTags(String serviceName, TagStore tagStore) throws Exception { + public ServiceTags getServiceTags(String serviceName, Long serviceId, TagStore tagStore) throws Exception { if (LOG.isDebugEnabled()) { - LOG.debug("==> RangerServiceTagsCache.getServiceTags(" + serviceName + ")"); + LOG.debug("==> RangerServiceTagsCache.getServiceTags(" + serviceName + ", " + serviceId + ")"); } ServiceTags ret = null; - if (StringUtils.isNotBlank(serviceName)) { + if (StringUtils.isNotBlank(serviceName) && serviceId != null) { if (LOG.isDebugEnabled()) { LOG.debug("useServiceTagsCache=" + useServiceTagsCache); @@ -132,8 +118,19 @@ public class RangerServiceTagsCache { synchronized (this) { serviceTagsWrapper = serviceTagsMap.get(serviceName); + if (serviceTagsWrapper != null) { + if (!serviceId.equals(serviceTagsWrapper.getServiceId())) { + if (LOG.isDebugEnabled()) { + LOG.debug("Service [" + serviceName + "] changed service-id from " + serviceTagsWrapper.getServiceId() + + " to " + serviceId); + LOG.debug("Recreating serviceTagsWrapper for serviceName [" + serviceName + "]"); + } + serviceTagsMap.remove(serviceName); + serviceTagsWrapper = null; + } + } if (serviceTagsWrapper == null) { - serviceTagsWrapper = new ServiceTagsWrapper(); + serviceTagsWrapper = new ServiceTagsWrapper(serviceId); serviceTagsMap.put(serviceName, serviceTagsWrapper); } } @@ -154,27 +151,31 @@ public class RangerServiceTagsCache { ret = serviceTags; } else { - LOG.error("getServiceTags() failed to get tags as serviceName is null or blank!"); + LOG.error("getServiceTags() failed to get tags as serviceName is null or blank and/or serviceId is null!"); } if (LOG.isDebugEnabled()) { - LOG.debug("<== RangerServiceTagsCache.getServiceTags(" + serviceName + "): count=" + ((ret == null || ret.getTags() == null) ? 0 : ret.getTags().size())); + LOG.debug("<== RangerServiceTagsCache.getServiceTags(" + serviceName + ", " + serviceId + "): count=" + ((ret == null || ret.getTags() == null) ? 0 : ret.getTags().size())); } return ret; } private class ServiceTagsWrapper { + final Long serviceId; ServiceTags serviceTags; Date updateTime = null; long longestDbLoadTimeInMs = -1; ReentrantLock lock = new ReentrantLock(); - ServiceTagsWrapper() { + ServiceTagsWrapper(Long serviceId) { + this.serviceId = serviceId; serviceTags = null; } + Long getServiceId() { return serviceId; } + ServiceTags getServiceTags() { return serviceTags; }
