From acafe1af4b828b4137b8449e10b5228fa5f30e28 Mon Sep 17 00:00:00 2001 From: Vyom Mani Tiwari Date: Thu, 6 Aug 2026 13:54:50 +0530 Subject: [PATCH] RANGER-5678: Concurrent policy-engine rebuild on shared serviceDef causes CME/NPE for delegated-admin User --- .../ranger/plugin/model/RangerServiceDef.java | 305 ++++++++++++++++++ .../plugin/policyengine/PolicyEngine.java | 2 +- .../policyengine/RangerPolicyRepository.java | 2 +- .../ranger/plugin/util/ServiceDefUtil.java | 264 +-------------- .../plugin/util/ServiceDefUtilTest.java | 186 +++++++++++ 5 files changed, 495 insertions(+), 264 deletions(-) diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/model/RangerServiceDef.java b/agents-common/src/main/java/org/apache/ranger/plugin/model/RangerServiceDef.java index 573c485ff1f..4cc01fb7753 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/model/RangerServiceDef.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/model/RangerServiceDef.java @@ -23,7 +23,11 @@ import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonInclude; +import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.collections.MapUtils; +import org.apache.commons.lang3.StringUtils; import org.apache.ranger.authorization.utils.StringUtil; +import org.apache.ranger.plugin.store.AbstractServiceStore; import java.util.ArrayList; import java.util.Collection; @@ -64,6 +68,9 @@ public class RangerServiceDef extends RangerBaseModelObject implements java.io.S private RangerRowFilterDef rowFilterDef; private List markerAccessTypes; // read-only + private transient volatile boolean normalized; + private transient volatile String accessTypesNormalizedForComponent; + public RangerServiceDef() { this(null, null, null, null, null, null, null, null, null, null, null, null, null); } @@ -120,6 +127,8 @@ public RangerServiceDef(String name, String displayName, String implClass, Strin public void updateFrom(RangerServiceDef other) { super.updateFrom(other); + clearNormalized(); + setName(other.getName()); setDisplayName(other.getDisplayName()); setImplClass(other.getImplClass()); @@ -139,6 +148,37 @@ public void updateFrom(RangerServiceDef other) { setMarkerAccessTypes(other.getMarkerAccessTypes()); } + public RangerServiceDef normalize() { + if (!normalized) { + synchronized (this) { + if (!normalized) { + normalizeInPlace(); + + normalized = true; + } + } + } + + return this; + } + + private void clearNormalized() { + normalized = false; + accessTypesNormalizedForComponent = null; + } + + public RangerServiceDef normalizeAccessTypeDefs(String componentType) { + if (StringUtils.isNotBlank(componentType) && !componentType.equals(accessTypesNormalizedForComponent)) { + synchronized (this) { + if (!componentType.equals(accessTypesNormalizedForComponent)) { + normalizeAccessTypeDefsInPlace(componentType); + accessTypesNormalizedForComponent = componentType; + } + } + } + return this; + } + /** * @return the name */ @@ -295,6 +335,8 @@ public void setResources(List resources) { if (resources != null) { this.resources.addAll(resources); } + + clearNormalized(); } /** @@ -321,6 +363,8 @@ public void setAccessTypes(List accessTypes) { if (accessTypes != null) { this.accessTypes.addAll(accessTypes); } + + clearNormalized(); } /** @@ -407,6 +451,8 @@ public RangerDataMaskDef getDataMaskDef() { public void setDataMaskDef(RangerDataMaskDef dataMaskDef) { this.dataMaskDef = dataMaskDef == null ? new RangerDataMaskDef() : dataMaskDef; + + clearNormalized(); } public RangerRowFilterDef getRowFilterDef() { @@ -415,6 +461,8 @@ public RangerRowFilterDef getRowFilterDef() { public void setRowFilterDef(RangerRowFilterDef rowFilterDef) { this.rowFilterDef = rowFilterDef == null ? new RangerRowFilterDef() : rowFilterDef; + + clearNormalized(); } public List getMarkerAccessTypes() { @@ -445,6 +493,263 @@ public void setDisplayName(String displayName) { this.displayName = displayName; } + private void normalizeInPlace() { + normalizeDataMaskDef(); + normalizeRowFilterDef(); + } + + private void normalizeDataMaskDef() { + if (dataMaskDef != null) { + List dataMaskResources = dataMaskDef.getResources(); + List dataMaskAccessTypes = dataMaskDef.getAccessTypes(); + + if (CollectionUtils.isNotEmpty(dataMaskResources)) { + List processedDefs = new ArrayList<>(dataMaskResources.size()); + + for (RangerResourceDef dataMaskResource : dataMaskResources) { + RangerResourceDef processedDef = dataMaskResource; + + for (RangerResourceDef resourceDef : resources) { + if (StringUtils.equals(resourceDef.getName(), dataMaskResource.getName())) { + processedDef = mergeResourceDef(resourceDef, dataMaskResource); + + break; + } + } + + processedDefs.add(processedDef); + } + + dataMaskDef.setResources(processedDefs); + } + + if (CollectionUtils.isNotEmpty(dataMaskAccessTypes)) { + List processedDefs = new ArrayList<>(accessTypes.size()); + + for (RangerAccessTypeDef dataMaskAccessType : dataMaskAccessTypes) { + RangerAccessTypeDef processedDef = dataMaskAccessType; + + for (RangerAccessTypeDef accessType : accessTypes) { + if (StringUtils.equals(accessType.getName(), dataMaskAccessType.getName())) { + processedDef = mergeAccessTypeDef(accessType, dataMaskAccessType); + + break; + } + } + + processedDefs.add(processedDef); + } + + dataMaskDef.setAccessTypes(processedDefs); + } + } + } + + private void normalizeRowFilterDef() { + if (rowFilterDef != null) { + List rowFilterResources = rowFilterDef.getResources(); + List rowFilterAccessTypes = rowFilterDef.getAccessTypes(); + + if (CollectionUtils.isNotEmpty(rowFilterResources)) { + List processedDefs = new ArrayList<>(rowFilterResources.size()); + + for (RangerResourceDef rowFilterResource : rowFilterResources) { + RangerResourceDef processedDef = rowFilterResource; + + for (RangerResourceDef resourceDef : resources) { + if (StringUtils.equals(resourceDef.getName(), rowFilterResource.getName())) { + processedDef = mergeResourceDef(resourceDef, rowFilterResource); + + break; + } + } + + processedDefs.add(processedDef); + } + + rowFilterDef.setResources(processedDefs); + } + + if (CollectionUtils.isNotEmpty(rowFilterAccessTypes)) { + List processedDefs = new ArrayList<>(accessTypes.size()); + + for (RangerAccessTypeDef rowFilterAccessType : rowFilterAccessTypes) { + RangerAccessTypeDef processedDef = rowFilterAccessType; + + for (RangerAccessTypeDef accessType : accessTypes) { + if (StringUtils.equals(accessType.getName(), rowFilterAccessType.getName())) { + processedDef = mergeAccessTypeDef(accessType, rowFilterAccessType); + + break; + } + } + + processedDefs.add(processedDef); + } + + rowFilterDef.setAccessTypes(processedDefs); + } + } + } + + private void normalizeAccessTypeDefsInPlace(String componentType) { + normalizeAccessTypeDefList(this.accessTypes, componentType); + normalizeAccessTypeDefList(this.markerAccessTypes, componentType); + + if (this.dataMaskDef != null) { + normalizeAccessTypeDefList(this.dataMaskDef.getAccessTypes(), componentType); + } + + if (this.rowFilterDef != null) { + normalizeAccessTypeDefList(this.rowFilterDef.getAccessTypes(), componentType); + } + } + + private static void normalizeAccessTypeDefList(List accessTypeDefs, String componentType) { + if (CollectionUtils.isNotEmpty(accessTypeDefs)) { + String prefix = componentType + AbstractServiceStore.COMPONENT_ACCESSTYPE_SEPARATOR; + List unneededAccessTypeDefs = null; + + for (RangerAccessTypeDef accessTypeDef : accessTypeDefs) { + String accessType = accessTypeDef.getName(); + + if (StringUtils.startsWith(accessType, prefix)) { + String newAccessType = StringUtils.removeStart(accessType, prefix); + + accessTypeDef.setName(newAccessType); + } else if (StringUtils.contains(accessType, AbstractServiceStore.COMPONENT_ACCESSTYPE_SEPARATOR)) { + if (unneededAccessTypeDefs == null) { + unneededAccessTypeDefs = new ArrayList<>(); + } + + unneededAccessTypeDefs.add(accessTypeDef); + + continue; + } + + Collection impliedGrants = accessTypeDef.getImpliedGrants(); + + if (CollectionUtils.isNotEmpty(impliedGrants)) { + Set newImpliedGrants = new HashSet<>(); + + for (String impliedGrant : impliedGrants) { + if (StringUtils.startsWith(impliedGrant, prefix)) { + String newImpliedGrant = StringUtils.removeStart(impliedGrant, prefix); + + newImpliedGrants.add(newImpliedGrant); + } else if (!StringUtils.contains(impliedGrant, AbstractServiceStore.COMPONENT_ACCESSTYPE_SEPARATOR)) { + newImpliedGrants.add(impliedGrant); + } + } + + accessTypeDef.setImpliedGrants(newImpliedGrants); + } + } + + if (unneededAccessTypeDefs != null) { + accessTypeDefs.removeAll(unneededAccessTypeDefs); + } + } + } + + private RangerResourceDef mergeResourceDef(RangerResourceDef base, RangerResourceDef delta) { + RangerResourceDef ret = new RangerResourceDef(base); + + // retain base values for: itemId, name, type, level, parent, lookupSupported + + if (Boolean.TRUE.equals(delta.getMandatory())) { + ret.setMandatory(delta.getMandatory()); + } + + if (delta.getRecursiveSupported() != null) { + ret.setRecursiveSupported(delta.getRecursiveSupported()); + } + + if (delta.getExcludesSupported() != null) { + ret.setExcludesSupported(delta.getExcludesSupported()); + } + + if (StringUtils.isNotEmpty(delta.getMatcher())) { + ret.setMatcher(delta.getMatcher()); + } + + if (MapUtils.isNotEmpty(delta.getMatcherOptions())) { + if (ret.getMatcherOptions() == null) { + ret.setMatcherOptions(new HashMap<>()); + } + + for (Map.Entry e : delta.getMatcherOptions().entrySet()) { + ret.getMatcherOptions().put(e.getKey(), e.getValue()); + } + } + + if (StringUtils.isNotEmpty(delta.getValidationRegEx())) { + ret.setValidationRegEx(delta.getValidationRegEx()); + } + + if (StringUtils.isNotEmpty(delta.getValidationMessage())) { + ret.setValidationMessage(delta.getValidationMessage()); + } + + ret.setUiHint(delta.getUiHint()); + + if (StringUtils.isNotEmpty(delta.getLabel())) { + ret.setLabel(delta.getLabel()); + } + + if (StringUtils.isNotEmpty(delta.getDescription())) { + ret.setDescription(delta.getDescription()); + } + + if (StringUtils.isNotEmpty(delta.getRbKeyLabel())) { + ret.setRbKeyLabel(delta.getRbKeyLabel()); + } + + if (StringUtils.isNotEmpty(delta.getRbKeyDescription())) { + ret.setRbKeyDescription(delta.getRbKeyDescription()); + } + + if (StringUtils.isNotEmpty(delta.getRbKeyValidationMessage())) { + ret.setRbKeyValidationMessage(delta.getRbKeyValidationMessage()); + } + + if (CollectionUtils.isNotEmpty(delta.getAccessTypeRestrictions())) { + ret.setAccessTypeRestrictions(delta.getAccessTypeRestrictions()); + } + + boolean copyLeafValue = false; + + if (ret.getIsValidLeaf() != null) { + if (!ret.getIsValidLeaf().equals(delta.getIsValidLeaf())) { + copyLeafValue = true; + } + } else if (delta.getIsValidLeaf() != null) { + copyLeafValue = true; + } + + if (copyLeafValue) { + ret.setIsValidLeaf(delta.getIsValidLeaf()); + } + + return ret; + } + + private RangerAccessTypeDef mergeAccessTypeDef(RangerAccessTypeDef base, RangerAccessTypeDef delta) { + RangerAccessTypeDef ret = new RangerAccessTypeDef(base); + + // retain base values for: itemId, name, impliedGrants + + if (StringUtils.isNotEmpty(delta.getLabel())) { + ret.setLabel(delta.getLabel()); + } + + if (StringUtils.isNotEmpty(delta.getRbKeyLabel())) { + ret.setRbKeyLabel(delta.getRbKeyLabel()); + } + + return ret; + } + public void dedupStrings(Map strTbl) { name = StringUtil.dedupString(name, strTbl); displayName = StringUtil.dedupString(displayName, strTbl); diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/PolicyEngine.java b/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/PolicyEngine.java index b8c99308b3e..f0336290fca 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/PolicyEngine.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/PolicyEngine.java @@ -568,7 +568,7 @@ private void normalizeServiceDefs(ServicePolicies servicePolicies) { RangerServiceDef tagServiceDef = servicePolicies.getTagPolicies() != null ? servicePolicies.getTagPolicies().getServiceDef() : null; if (tagServiceDef != null) { - ServiceDefUtil.normalizeAccessTypeDefs(ServiceDefUtil.normalize(tagServiceDef), serviceDef.getName()); + tagServiceDef.normalize().normalizeAccessTypeDefs(serviceDef.getName()); } } } diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyRepository.java b/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyRepository.java index 6eee0143c02..314941c1842 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyRepository.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyRepository.java @@ -259,7 +259,7 @@ public RangerPolicyRepository(ServicePolicies servicePolicies, RangerPluginConte this.serviceName = tagPolicies.getServiceName(); this.componentServiceName = componentServiceName; this.zoneName = null; - this.serviceDef = ServiceDefUtil.normalizeAccessTypeDefs(ServiceDefUtil.normalize(tagPolicies.getServiceDef()), componentServiceDef.getName()); + this.serviceDef = tagPolicies.getServiceDef().normalize().normalizeAccessTypeDefs(componentServiceDef.getName()); this.componentServiceDef = componentServiceDef; this.appId = pluginContext.getConfig().getAppId(); this.options = new RangerPolicyEngineOptions(pluginContext.getConfig().getPolicyEngineOptions(), new RangerServiceDefHelper(serviceDef, false)); diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceDefUtil.java b/agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceDefUtil.java index 4b5eabc0f5d..f677c3e5b64 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceDefUtil.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceDefUtil.java @@ -44,7 +44,6 @@ import org.apache.ranger.plugin.policyengine.RangerPluginContext; import org.apache.ranger.plugin.policyengine.RangerRequestScriptEvaluator; import org.apache.ranger.plugin.resourcematcher.RangerAbstractResourceMatcher; -import org.apache.ranger.plugin.store.AbstractServiceStore; import org.apache.ranger.plugin.store.EmbeddedServiceDefsUtil; import org.apache.ranger.plugin.util.ServicePolicies.SecurityZoneInfo; import org.slf4j.Logger; @@ -126,10 +125,7 @@ public static RangerDataMaskTypeDef getDataMaskType(RangerServiceDef serviceDef, } public static RangerServiceDef normalize(RangerServiceDef serviceDef) { - normalizeDataMaskDef(serviceDef); - normalizeRowFilterDef(serviceDef); - - return serviceDef; + return serviceDef != null ? serviceDef.normalize() : null; } public static RangerPolicyConditionDef getConditionDef(RangerServiceDef serviceDef, String conditionName) { @@ -260,20 +256,7 @@ public static char getCharOption(Map options, String name, char } public static RangerServiceDef normalizeAccessTypeDefs(RangerServiceDef serviceDef, final String componentType) { - if (serviceDef != null && StringUtils.isNotBlank(componentType)) { - normalizeAccessTypeDefs(serviceDef.getAccessTypes(), componentType); - normalizeAccessTypeDefs(serviceDef.getMarkerAccessTypes(), componentType); - - if (serviceDef.getDataMaskDef() != null) { - normalizeAccessTypeDefs(serviceDef.getDataMaskDef().getAccessTypes(), componentType); - } - - if (serviceDef.getRowFilterDef() != null) { - normalizeAccessTypeDefs(serviceDef.getRowFilterDef().getAccessTypes(), componentType); - } - } - - return serviceDef; + return serviceDef != null ? serviceDef.normalizeAccessTypeDefs(componentType) : null; } public static boolean getBooleanValue(Map map, String elementName, boolean defaultValue) { @@ -524,249 +507,6 @@ private static boolean hasWildcardValue(List values) { return ret; } - private static void normalizeAccessTypeDefs(List accessTypeDefs, String componentType) { - if (CollectionUtils.isNotEmpty(accessTypeDefs)) { - String prefix = componentType + AbstractServiceStore.COMPONENT_ACCESSTYPE_SEPARATOR; - List unneededAccessTypeDefs = null; - - for (RangerAccessTypeDef accessTypeDef : accessTypeDefs) { - String accessType = accessTypeDef.getName(); - - if (StringUtils.startsWith(accessType, prefix)) { - String newAccessType = StringUtils.removeStart(accessType, prefix); - - accessTypeDef.setName(newAccessType); - } else if (StringUtils.contains(accessType, AbstractServiceStore.COMPONENT_ACCESSTYPE_SEPARATOR)) { - if (unneededAccessTypeDefs == null) { - unneededAccessTypeDefs = new ArrayList<>(); - } - - unneededAccessTypeDefs.add(accessTypeDef); - - continue; - } - - Collection impliedGrants = accessTypeDef.getImpliedGrants(); - - if (CollectionUtils.isNotEmpty(impliedGrants)) { - Set newImpliedGrants = new HashSet<>(); - - for (String impliedGrant : impliedGrants) { - if (StringUtils.startsWith(impliedGrant, prefix)) { - String newImpliedGrant = StringUtils.removeStart(impliedGrant, prefix); - - newImpliedGrants.add(newImpliedGrant); - } else if (!StringUtils.contains(impliedGrant, AbstractServiceStore.COMPONENT_ACCESSTYPE_SEPARATOR)) { - newImpliedGrants.add(impliedGrant); - } - } - - accessTypeDef.setImpliedGrants(newImpliedGrants); - } - } - - if (unneededAccessTypeDefs != null) { - accessTypeDefs.removeAll(unneededAccessTypeDefs); - } - } - } - - private static void normalizeDataMaskDef(RangerServiceDef serviceDef) { - if (serviceDef != null && serviceDef.getDataMaskDef() != null) { - List dataMaskResources = serviceDef.getDataMaskDef().getResources(); - List dataMaskAccessTypes = serviceDef.getDataMaskDef().getAccessTypes(); - - if (CollectionUtils.isNotEmpty(dataMaskResources)) { - List resources = serviceDef.getResources(); - List processedDefs = new ArrayList<>(dataMaskResources.size()); - - for (RangerResourceDef dataMaskResource : dataMaskResources) { - RangerResourceDef processedDef = dataMaskResource; - - for (RangerResourceDef resourceDef : resources) { - if (StringUtils.equals(resourceDef.getName(), dataMaskResource.getName())) { - processedDef = ServiceDefUtil.mergeResourceDef(resourceDef, dataMaskResource); - - break; - } - } - - processedDefs.add(processedDef); - } - - serviceDef.getDataMaskDef().setResources(processedDefs); - } - - if (CollectionUtils.isNotEmpty(dataMaskAccessTypes)) { - List accessTypes = serviceDef.getAccessTypes(); - List processedDefs = new ArrayList<>(accessTypes.size()); - - for (RangerAccessTypeDef dataMaskAccessType : dataMaskAccessTypes) { - RangerAccessTypeDef processedDef = dataMaskAccessType; - - for (RangerAccessTypeDef accessType : accessTypes) { - if (StringUtils.equals(accessType.getName(), dataMaskAccessType.getName())) { - processedDef = ServiceDefUtil.mergeAccessTypeDef(accessType, dataMaskAccessType); - - break; - } - } - - processedDefs.add(processedDef); - } - - serviceDef.getDataMaskDef().setAccessTypes(processedDefs); - } - } - } - - private static void normalizeRowFilterDef(RangerServiceDef serviceDef) { - if (serviceDef != null && serviceDef.getRowFilterDef() != null) { - List rowFilterResources = serviceDef.getRowFilterDef().getResources(); - List rowFilterAccessTypes = serviceDef.getRowFilterDef().getAccessTypes(); - - if (CollectionUtils.isNotEmpty(rowFilterResources)) { - List resources = serviceDef.getResources(); - List processedDefs = new ArrayList<>(rowFilterResources.size()); - - for (RangerResourceDef rowFilterResource : rowFilterResources) { - RangerResourceDef processedDef = rowFilterResource; - - for (RangerResourceDef resourceDef : resources) { - if (StringUtils.equals(resourceDef.getName(), rowFilterResource.getName())) { - processedDef = ServiceDefUtil.mergeResourceDef(resourceDef, rowFilterResource); - - break; - } - } - - processedDefs.add(processedDef); - } - - serviceDef.getRowFilterDef().setResources(processedDefs); - } - - if (CollectionUtils.isNotEmpty(rowFilterAccessTypes)) { - List accessTypes = serviceDef.getAccessTypes(); - List processedDefs = new ArrayList<>(accessTypes.size()); - - for (RangerAccessTypeDef rowFilterAccessType : rowFilterAccessTypes) { - RangerAccessTypeDef processedDef = rowFilterAccessType; - - for (RangerAccessTypeDef accessType : accessTypes) { - if (StringUtils.equals(accessType.getName(), rowFilterAccessType.getName())) { - processedDef = ServiceDefUtil.mergeAccessTypeDef(accessType, rowFilterAccessType); - - break; - } - } - - processedDefs.add(processedDef); - } - - serviceDef.getRowFilterDef().setAccessTypes(processedDefs); - } - } - } - - private static RangerResourceDef mergeResourceDef(RangerResourceDef base, RangerResourceDef delta) { - RangerResourceDef ret = new RangerResourceDef(base); - - // retain base values for: itemId, name, type, level, parent, lookupSupported - - if (Boolean.TRUE.equals(delta.getMandatory())) { - ret.setMandatory(delta.getMandatory()); - } - - if (delta.getRecursiveSupported() != null) { - ret.setRecursiveSupported(delta.getRecursiveSupported()); - } - - if (delta.getExcludesSupported() != null) { - ret.setExcludesSupported(delta.getExcludesSupported()); - } - - if (StringUtils.isNotEmpty(delta.getMatcher())) { - ret.setMatcher(delta.getMatcher()); - } - - if (MapUtils.isNotEmpty(delta.getMatcherOptions())) { - if (ret.getMatcherOptions() == null) { - ret.setMatcherOptions(new HashMap<>()); - } - - for (Map.Entry e : delta.getMatcherOptions().entrySet()) { - ret.getMatcherOptions().put(e.getKey(), e.getValue()); - } - } - - if (StringUtils.isNotEmpty(delta.getValidationRegEx())) { - ret.setValidationRegEx(delta.getValidationRegEx()); - } - - if (StringUtils.isNotEmpty(delta.getValidationMessage())) { - ret.setValidationMessage(delta.getValidationMessage()); - } - - ret.setUiHint(delta.getUiHint()); - - if (StringUtils.isNotEmpty(delta.getLabel())) { - ret.setLabel(delta.getLabel()); - } - - if (StringUtils.isNotEmpty(delta.getDescription())) { - ret.setDescription(delta.getDescription()); - } - - if (StringUtils.isNotEmpty(delta.getRbKeyLabel())) { - ret.setRbKeyLabel(delta.getRbKeyLabel()); - } - - if (StringUtils.isNotEmpty(delta.getRbKeyDescription())) { - ret.setRbKeyDescription(delta.getRbKeyDescription()); - } - - if (StringUtils.isNotEmpty(delta.getRbKeyValidationMessage())) { - ret.setRbKeyValidationMessage(delta.getRbKeyValidationMessage()); - } - - if (CollectionUtils.isNotEmpty(delta.getAccessTypeRestrictions())) { - ret.setAccessTypeRestrictions(delta.getAccessTypeRestrictions()); - } - - boolean copyLeafValue = false; - if (ret.getIsValidLeaf() != null) { - if (!ret.getIsValidLeaf().equals(delta.getIsValidLeaf())) { - copyLeafValue = true; - } - } else { - if (delta.getIsValidLeaf() != null) { - copyLeafValue = true; - } - } - if (copyLeafValue) { - ret.setIsValidLeaf(delta.getIsValidLeaf()); - } - - return ret; - } - - private static RangerAccessTypeDef mergeAccessTypeDef(RangerAccessTypeDef base, RangerAccessTypeDef delta) { - RangerAccessTypeDef ret = new RangerAccessTypeDef(base); - - // retain base values for: itemId, name, impliedGrants - - if (StringUtils.isNotEmpty(delta.getLabel())) { - ret.setLabel(delta.getLabel()); - } - - if (StringUtils.isNotEmpty(delta.getRbKeyLabel())) { - ret.setRbKeyLabel(delta.getRbKeyLabel()); - } - - return ret; - } - private static Map> getMarkerAccessTypeGrants(List accessTypeDefs) { Map> ret = new HashMap<>(); diff --git a/agents-common/src/test/java/org/apache/ranger/plugin/util/ServiceDefUtilTest.java b/agents-common/src/test/java/org/apache/ranger/plugin/util/ServiceDefUtilTest.java index 9c0bb92022b..0c3c8518521 100644 --- a/agents-common/src/test/java/org/apache/ranger/plugin/util/ServiceDefUtilTest.java +++ b/agents-common/src/test/java/org/apache/ranger/plugin/util/ServiceDefUtilTest.java @@ -33,10 +33,13 @@ import org.apache.ranger.plugin.model.RangerServiceDef; import org.apache.ranger.plugin.model.RangerServiceDef.RangerAccessTypeDef; import org.apache.ranger.plugin.model.RangerServiceDef.RangerAccessTypeDef.AccessTypeCategory; +import org.apache.ranger.plugin.model.validation.RangerServiceDefHelper; import org.apache.ranger.plugin.store.EmbeddedServiceDefsUtil; import org.apache.ranger.plugin.util.ServicePolicies.SecurityZoneInfo; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.RepeatedTest; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; import java.io.InputStream; import java.io.InputStreamReader; @@ -47,6 +50,14 @@ import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; import static org.apache.ranger.plugin.util.ServiceDefUtil.ACCESS_TYPE_MARKER_ALL; import static org.apache.ranger.plugin.util.ServiceDefUtil.ACCESS_TYPE_MARKER_CREATE; @@ -57,6 +68,8 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; public class ServiceDefUtilTest { @@ -303,6 +316,179 @@ public void testNormalizeAccessTypeDefs() throws Exception { } } + @Test + public void testNormalizeIsThreadSafe() throws Throwable { + RangerServiceDef serviceDef = EmbeddedServiceDefsUtil.instance().getEmbeddedServiceDef("hive"); + + assertTrue(serviceDef != null && serviceDef.getDataMaskDef() != null, "hive serviceDef with dataMaskDef is required"); + + int threadCount = 8; + CountDownLatch startLatch = new CountDownLatch(1); + CountDownLatch doneLatch = new CountDownLatch(threadCount); + AtomicReference failure = new AtomicReference<>(); + + for (int i = 0; i < threadCount; i++) { + new Thread(() -> { + try { + startLatch.await(); + + RangerServiceDef result = ServiceDefUtil.normalize(serviceDef); + + assertSame(serviceDef, result, "normalize() must return the same instance"); + } catch (Throwable t) { + failure.compareAndSet(null, t); + } finally { + doneLatch.countDown(); + } + }).start(); + } + + startLatch.countDown(); + + assertTrue(doneLatch.await(30, TimeUnit.SECONDS), "normalize threads did not finish in time"); + + if (failure.get() != null) { + throw failure.get(); + } + } + + @Test + public void testConcurrentNormalizeAndServiceDefHelperInit() throws Throwable { + RangerServiceDef serviceDef = EmbeddedServiceDefsUtil.instance().getEmbeddedServiceDef("hive"); + + assertTrue(serviceDef != null && serviceDef.getDataMaskDef() != null, "hive serviceDef with dataMaskDef is required"); + + int threadCount = 8; + CountDownLatch startLatch = new CountDownLatch(1); + CountDownLatch doneLatch = new CountDownLatch(threadCount); + AtomicReference failure = new AtomicReference<>(); + + for (int i = 0; i < threadCount; i++) { + new Thread(() -> { + try { + startLatch.await(); + + ServiceDefUtil.normalize(serviceDef); + + RangerServiceDefHelper helper = new RangerServiceDefHelper(serviceDef, false); + + assertTrue(helper.isResourceGraphValid(), "resource graph must be valid after normalize"); + } catch (Throwable t) { + failure.compareAndSet(null, t); + } finally { + doneLatch.countDown(); + } + }).start(); + } + + startLatch.countDown(); + + assertTrue(doneLatch.await(30, TimeUnit.SECONDS), "normalize/helper threads did not finish in time"); + + if (failure.get() != null) { + throw failure.get(); + } + } + + @Test + public void testNormalizeSkipsWhenAlreadyNormalized() throws Exception { + RangerServiceDef serviceDef = EmbeddedServiceDefsUtil.instance().getEmbeddedServiceDef("hive"); + + assertTrue(serviceDef != null && serviceDef.getDataMaskDef() != null, "hive serviceDef with dataMaskDef is required"); + + ServiceDefUtil.normalize(serviceDef); + + List dataMaskResourcesAfterFirst = serviceDef.getDataMaskDef().getResources(); + List dataMaskAccessTypesAfterFirst = serviceDef.getDataMaskDef().getAccessTypes(); + + ServiceDefUtil.normalize(serviceDef); + + assertSame(dataMaskResourcesAfterFirst, serviceDef.getDataMaskDef().getResources(), "second normalize() must not replace dataMask resources"); + assertSame(dataMaskAccessTypesAfterFirst, serviceDef.getDataMaskDef().getAccessTypes(), "second normalize() must not replace dataMask accessTypes"); + } + + @Test + public void testSetResourcesRequiresRenormalize() throws Exception { + RangerServiceDef serviceDef = EmbeddedServiceDefsUtil.instance().getEmbeddedServiceDef("hive"); + + assertTrue(serviceDef != null && serviceDef.getDataMaskDef() != null, "hive serviceDef with dataMaskDef is required"); + + ServiceDefUtil.normalize(serviceDef); + + RangerServiceDef.RangerResourceDef dataMaskResourceAfterFirst = serviceDef.getDataMaskDef().getResources().get(0); + + serviceDef.setResources(new ArrayList<>(serviceDef.getResources())); + ServiceDefUtil.normalize(serviceDef); + + assertNotSame(dataMaskResourceAfterFirst, serviceDef.getDataMaskDef().getResources().get(0), "setResources() must require re-normalization"); + } + + @RepeatedTest(5) + @Timeout(value = 90, unit = TimeUnit.SECONDS) + public void testConcurrentNormalizeAccessTypeDefsOnSharedServiceDef() throws Exception { + RangerServiceDef sharedTagServiceDef = buildTagServiceDefWithPrefixedAccessTypes("hive"); + + int threads = 5; + int itersPerThread = 100; + ExecutorService pool = Executors.newFixedThreadPool(threads); + CyclicBarrier barrier = new CyclicBarrier(threads); + AtomicInteger failures = new AtomicInteger(); + List firstErrors = new ArrayList<>(); + List> futures = new ArrayList<>(); + + for (int t = 0; t < threads; t++) { + futures.add(pool.submit(() -> { + for (int i = 0; i < itersPerThread; i++) { + try { + barrier.await(); + } catch (Exception ignored) { + } + try { + // touch point #1 - mirrors PolicyEngine.normalizeServiceDefs() + ServiceDefUtil.normalizeAccessTypeDefs(sharedTagServiceDef, "hive"); + // intervening real work - mirrors the gap between the two real call sites + new RangerServiceDefHelper(sharedTagServiceDef, false); + // touch point #2 - mirrors RangerPolicyRepository's tag-policies constructor + ServiceDefUtil.normalizeAccessTypeDefs(sharedTagServiceDef, "hive"); + } catch (Throwable e) { + Throwable cause = e.getCause() != null ? e.getCause() : e; + failures.incrementAndGet(); + synchronized (firstErrors) { + if (firstErrors.size() < 5) { + firstErrors.add(cause); + } + } + } + } + })); + } + for (Future f : futures) { + f.get(); + } + pool.shutdown(); + if (!firstErrors.isEmpty()) { + StringBuilder sb = new StringBuilder(); + sb.append(failures.get()).append(" failures. First errors:\n"); + for (Throwable e : firstErrors) { + sb.append(" ").append(e).append('\n'); + } + //System.out.println(sb); + } + assertEquals(0, failures.get(), "Expected 0 failures - concurrent normalizeAccessTypeDefs() calls on the same shared " + + "serviceDef (same componentType, the realistic repeated-same-service scenario) must not race"); + } + + private static RangerServiceDef buildTagServiceDefWithPrefixedAccessTypes(String componentType) { + RangerServiceDef sd = new RangerServiceDef(); + sd.setName("tag"); + List accessTypes = new ArrayList<>(); + for (int i = 0; i < 5000; i++) { + accessTypes.add(new RangerAccessTypeDef((long) i, componentType + "#type" + i, "type" + i, null, null)); + } + sd.setAccessTypes(accessTypes); + return sd; + } + @Test public void testAccessTypeMarkers() { RangerAccessTypeDef create = new RangerAccessTypeDef(1L, "create", "create", null, null, AccessTypeCategory.CREATE);