Repository: ranger
Updated Branches:
  refs/heads/master a4e69aab2 -> 847e121f5


RANGER-1781: Policy model update to support restricted access-types based on 
selected resource (contd.)


Project: http://git-wip-us.apache.org/repos/asf/ranger/repo
Commit: http://git-wip-us.apache.org/repos/asf/ranger/commit/847e121f
Tree: http://git-wip-us.apache.org/repos/asf/ranger/tree/847e121f
Diff: http://git-wip-us.apache.org/repos/asf/ranger/diff/847e121f

Branch: refs/heads/master
Commit: 847e121f5e210638d292eee0cb9beef34a037927
Parents: a4e69aa
Author: Abhay Kulkarni <[email protected]>
Authored: Mon Oct 23 16:33:49 2017 -0700
Committer: Abhay Kulkarni <[email protected]>
Committed: Mon Oct 23 16:38:17 2017 -0700

----------------------------------------------------------------------
 .../plugin/errors/ValidationErrorCode.java      |   1 +
 .../validation/RangerServiceDefHelper.java      |  65 ++++++++++--
 .../validation/RangerServiceDefValidator.java   |  24 ++++-
 .../RangerDefaultPolicyResourceMatcher.java     | 100 ++++++++-----------
 .../ranger/plugin/util/RangerObjectFactory.java |   2 +-
 .../validation/TestRangerServiceDefHelper.java  |   2 +-
 .../TestRangerServiceDefValidator.java          |  36 ++++++-
 ...ltpolicyresourcematcher_for_hdfs_policy.json |   4 +-
 ...rcematcher_for_resource_specific_policy.json |   8 +-
 9 files changed, 162 insertions(+), 80 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ranger/blob/847e121f/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java
----------------------------------------------------------------------
diff --git 
a/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java
 
b/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java
index d0f015d..a7f7c39 100644
--- 
a/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java
+++ 
b/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java
@@ -60,6 +60,7 @@ public enum ValidationErrorCode {
     SERVICE_DEF_VALIDATION_ERR_ENUM_DEF_NO_VALUES(2018, "enum [{0}] does not 
have any elements"),
     SERVICE_DEF_VALIDATION_ERR_ENUM_DEF_INVALID_DEFAULT_INDEX(2019, "default 
index[{0}] for enum [{1}] is invalid"),
     SERVICE_DEF_VALIDATION_ERR_ENUM_DEF_NULL_ENUM_ELEMENT(2020, "An enum 
element in enum element collection of enum [{0}] is null"),
+    SERVICE_DEF_VALIDATION_ERR_INVALID_SERVICE_RESOURCE_LEVELS(2021, 
"Resource-def levels are not in increasing order in an hierarchy"),
 
     // POLICY VALIDATION
     POLICY_VALIDATION_ERR_UNSUPPORTED_ACTION(3001, "Internal error: method 
signature isValid(Long) is only supported for DELETE"),

http://git-wip-us.apache.org/repos/asf/ranger/blob/847e121f/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefHelper.java
----------------------------------------------------------------------
diff --git 
a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefHelper.java
 
b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefHelper.java
index 7a719ab..6103e8f 100644
--- 
a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefHelper.java
+++ 
b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefHelper.java
@@ -31,6 +31,7 @@ import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 
+import com.google.common.collect.Sets;
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.logging.Log;
@@ -119,15 +120,19 @@ public class RangerServiceDefHelper {
        }
 
        public RangerServiceDefHelper(RangerServiceDef serviceDef) {
-               this(serviceDef, true);
+               this(serviceDef, true, false);
        }
-       
+
+       public RangerServiceDefHelper(RangerServiceDef serviceDef, boolean 
useCache) {
+               this(serviceDef, useCache, false);
+       }
+
        /**
         * Intended for use when serviceDef object is not-trusted, e.g. when 
service-def is being created or updated.
         * @param serviceDef
         * @param useCache
         */
-       public RangerServiceDefHelper(RangerServiceDef serviceDef, boolean 
useCache) {
+       public RangerServiceDefHelper(RangerServiceDef serviceDef, boolean 
useCache, boolean checkForCycles) {
                // NOTE: we assume serviceDef, its name and update time are can 
never by null.
                
                if(LOG.isDebugEnabled()) {
@@ -149,7 +154,7 @@ public class RangerServiceDefHelper {
                        }
                }
                if (delegate == null) { // either not found in cache or date 
didn't match
-                       delegate = new Delegate(serviceDef);
+                       delegate = new Delegate(serviceDef, checkForCycles);
                        if (useCache) {
                                LOG.debug("RangerServiceDefHelper(): Created 
new delegate and put in delegate cache!");
                                _Cache.put(serviceName, delegate);
@@ -256,14 +261,16 @@ public class RangerServiceDefHelper {
                final Map<Integer, Set<List<RangerResourceDef>>> _hierarchies = 
new HashMap<>();
                final Date _serviceDefFreshnessDate;
                final String _serviceName;
+               final boolean _checkForCycles;
                final boolean _valid;
                final static Set<List<RangerResourceDef>> 
EMPTY_RESOURCE_HIERARCHY = Collections.unmodifiableSet(new 
HashSet<List<RangerResourceDef>>());
 
 
-               public Delegate(RangerServiceDef serviceDef) {
+               public Delegate(RangerServiceDef serviceDef, boolean 
checkForCycles) {
                        // NOTE: we assume serviceDef, its name and update time 
are can never by null.
                        _serviceName = serviceDef.getName();
                        _serviceDefFreshnessDate = serviceDef.getUpdateTime();
+                       _checkForCycles = checkForCycles;
 
                        boolean isValid = true;
                        for(Integer policyType : RangerPolicy.POLICY_TYPES) {
@@ -289,9 +296,6 @@ public class RangerServiceDefHelper {
                                                _serviceDefFreshnessDate == 
null ? null : _serviceDefFreshnessDate.toString(), _hierarchies);
                                LOG.debug(message);
                        }
-                       if (!_valid) {
-                               LOG.error("ERROR: Service resource-definitions 
do not form valid DAG!!");
-                       }
                }
                
                public Set<List<RangerResourceDef>> 
getResourceHierarchies(Integer policyType) {
@@ -390,6 +394,12 @@ public class RangerServiceDefHelper {
 
             if (CollectionUtils.isEmpty(sources) || 
CollectionUtils.isEmpty(sinks)) {
                 ret = false;
+            } else if (_checkForCycles) {
+                List<String> cycle = graph.getACycle(sources, sinks);
+                if (cycle != null) {
+                    LOG.error("Graph contains cycle! - " + cycle);
+                    ret = false;
+                }
             } else {
                 for (String sink : sinks) {
                     RangerResourceDef sinkResourceDef = 
resourceDefMap.get(sink);
@@ -574,6 +584,45 @@ public class RangerServiceDefHelper {
                        return sinks;
                }
 
+               List<String> getACycle(Set<String> sources, Set<String> sinks) {
+                       List<String> ret = null;
+                       Set<String> nonSourceOrSinkNodes = 
Sets.difference(_nodes.keySet(), Sets.union(sources, sinks));
+                       for (String node : nonSourceOrSinkNodes) {
+                               List<String> seenNodes = new ArrayList<>();
+                               seenNodes.add(node);
+                               ret = findCycle(node, seenNodes);
+                               if (ret != null) {
+                                       break;
+                               }
+                       }
+                       return ret;
+               }
+
+               /**
+                * Does a depth first traversal of a graph starting from given 
node. Returns a sequence of nodes that form first cycle or null if no cycle is 
found.
+                *
+                * @param node Start node
+                * @param seenNodes List of nodes seen thus far
+                * @return list of nodes comprising first cycle in graph; null 
if no cycle was found
+                */
+               List<String> findCycle(String node, List<String> seenNodes) {
+                       List<String> ret = null;
+                       Set<String> nbrs = _nodes.get(node);
+                       for (String nbr : nbrs) {
+                               boolean foundCycle = seenNodes.contains(nbr);
+                               seenNodes.add(nbr);
+                               if (foundCycle) {
+                                       ret = seenNodes;
+                                       break;
+                               } else {
+                                       ret = findCycle(nbr, seenNodes);
+                                       if (ret != null) {
+                                               break;
+                                       }
+                               }
+                       }
+                       return ret;
+               }
                /**
                 * Attempts to do a depth first traversal of a graph and 
returns the resulting path. Note that there could be several paths that connect 
node "from" to node "to".
                 *

http://git-wip-us.apache.org/repos/asf/ranger/blob/847e121f/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefValidator.java
----------------------------------------------------------------------
diff --git 
a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefValidator.java
 
b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefValidator.java
index 3f2cc2a..608d258 100644
--- 
a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefValidator.java
+++ 
b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefValidator.java
@@ -478,7 +478,7 @@ public class RangerServiceDefValidator extends 
RangerValidator {
                }
                return valid;
        }
-       
+
        boolean isValidResourceGraph(RangerServiceDef serviceDef, 
List<ValidationFailureDetails> failures) {
                if(LOG.isDebugEnabled()) {
                        LOG.debug(String.format("==> 
RangerServiceDefValidator.isValidResourceGraph(%s, %s)", serviceDef, failures));
@@ -504,6 +504,26 @@ public class RangerServiceDefValidator extends 
RangerValidator {
                                for (RangerResourceDef resourceDef : 
aHierarchy) {
                                        valid = 
isUnique(resourceDef.getLevel(), levels, "resource level", "resources", 
failures) && valid;
                                }
+
+                               // Ensure that aHierarchy contains 
resource-defs with increasing level values
+                               int lastResourceLevel = Integer.MIN_VALUE;
+                               for (RangerResourceDef resourceDef : 
aHierarchy) {
+                                       Integer resourceDefLevel = 
resourceDef.getLevel();
+                                       if (resourceDefLevel == null || 
resourceDefLevel < lastResourceLevel) {
+                                               ValidationErrorCode error = 
ValidationErrorCode.SERVICE_DEF_VALIDATION_ERR_INVALID_SERVICE_RESOURCE_LEVELS;
+                                               failures.add(new 
ValidationFailureDetailsBuilder()
+                                                               
.field("resource level")
+                                                               
.subField(String.valueOf(resourceDefLevel))
+                                                               
.isSemanticallyIncorrect()
+                                                               
.errorCode(error.getErrorCode())
+                                                               
.becauseOf(error.getMessage())
+                                                               .build());
+                                               valid = false;
+                                               break;
+                                       } else {
+                                               lastResourceLevel = 
resourceDef.getLevel();
+                                       }
+                               }
                        }
                }
 
@@ -512,7 +532,7 @@ public class RangerServiceDefValidator extends 
RangerValidator {
                }
                return valid;
        }
-       
+
        boolean isValidEnums(List<RangerEnumDef> enumDefs, 
List<ValidationFailureDetails> failures) {
                if(LOG.isDebugEnabled()) {
                        LOG.debug(String.format("==> 
RangerServiceDefValidator.isValidEnums(%s, %s)", enumDefs, failures));

http://git-wip-us.apache.org/repos/asf/ranger/blob/847e121f/agents-common/src/main/java/org/apache/ranger/plugin/policyresourcematcher/RangerDefaultPolicyResourceMatcher.java
----------------------------------------------------------------------
diff --git 
a/agents-common/src/main/java/org/apache/ranger/plugin/policyresourcematcher/RangerDefaultPolicyResourceMatcher.java
 
b/agents-common/src/main/java/org/apache/ranger/plugin/policyresourcematcher/RangerDefaultPolicyResourceMatcher.java
index e8d85c5..ae608c1 100644
--- 
a/agents-common/src/main/java/org/apache/ranger/plugin/policyresourcematcher/RangerDefaultPolicyResourceMatcher.java
+++ 
b/agents-common/src/main/java/org/apache/ranger/plugin/policyresourcematcher/RangerDefaultPolicyResourceMatcher.java
@@ -119,23 +119,8 @@ public class RangerDefaultPolicyResourceMatcher implements 
RangerPolicyResourceM
             int                          validHierarchiesCount = 0;
 
             for (List<RangerResourceDef> resourceHierarchy : 
resourceHierarchies) {
-                boolean foundGapsInResourceSpecs = false;
-                boolean skipped                  = false;
 
-                for (RangerResourceDef resourceDef : resourceHierarchy) {
-                    RangerPolicyResource policyResource = 
policyResources.get(resourceDef.getName());
-
-                    if (policyResource == null) {
-                        skipped = true;
-                    } else if (skipped) {
-                        foundGapsInResourceSpecs = true;
-                        break;
-                    }
-                }
-
-                if (foundGapsInResourceSpecs) {
-                    LOG.warn("RangerDefaultPolicyResourceMatcher.init(): gaps 
found in policyResources, skipping hierarchy:[" + resourceHierarchies + "]");
-                } else {
+                if (isHierarchyValidForResources(resourceHierarchy, 
policyResources)) {
                     validHierarchiesCount++;
 
                     if (validHierarchiesCount == 1) {
@@ -143,6 +128,8 @@ public class RangerDefaultPolicyResourceMatcher implements 
RangerPolicyResourceM
                     } else {
                         validResourceHierarchy = null;
                     }
+                } else {
+                    LOG.warn("RangerDefaultPolicyResourceMatcher.init(): gaps 
found in policyResources, skipping hierarchy:[" + resourceHierarchies + "]");
                 }
             }
 
@@ -153,7 +140,7 @@ public class RangerDefaultPolicyResourceMatcher implements 
RangerPolicyResourceM
                     for (RangerResourceDef resourceDef : resourceHierarchy) {
                         String resourceName = resourceDef.getName();
 
-                        if (allMatchers.get(resourceName) != null) {
+                        if (allMatchers.containsKey(resourceName)) {
                             continue;
                         }
 
@@ -260,8 +247,8 @@ public class RangerDefaultPolicyResourceMatcher implements 
RangerPolicyResourceM
         if (keysMatch) {
             for (RangerResourceDef resourceDef : serviceDef.getResources()) {
                 String                resourceName  = resourceDef.getName();
-                String                resourceValue = resource == null ? null 
: resource.getValue(resourceName);
-                RangerResourceMatcher matcher       = allMatchers == null ? 
null : allMatchers.get(resourceName);
+                String                resourceValue = 
resource.getValue(resourceName);
+                RangerResourceMatcher matcher       = 
getResourceMatcher(resourceName);
 
                 if (StringUtils.isEmpty(resourceValue)) {
                     ret = matcher == null || 
matcher.isCompleteMatch(resourceValue, evalContext);
@@ -300,7 +287,7 @@ public class RangerDefaultPolicyResourceMatcher implements 
RangerPolicyResourceM
         if (keysMatch) {
             for (RangerResourceDef resourceDef : serviceDef.getResources()) {
                 String               resourceName   = resourceDef.getName();
-                RangerPolicyResource resourceValues = resources == null ? null 
: resources.get(resourceName);
+                RangerPolicyResource resourceValues = 
resources.get(resourceName);
                 RangerPolicyResource policyValues   = policyResources == null 
? null : policyResources.get(resourceName);
 
                 if (resourceValues == null || 
CollectionUtils.isEmpty(resourceValues.getValues())) {
@@ -333,14 +320,10 @@ public class RangerDefaultPolicyResourceMatcher 
implements RangerPolicyResourceM
 
     @Override
     public boolean isMatch(RangerPolicy policy, MatchScope scope, Map<String, 
Object> evalContext) {
-        if (policy.getPolicyType() == policyType) {
-            return isMatch(policy.getResources(), scope, false, evalContext);
-        } else {
-            return false;
-        }
+        return policy.getPolicyType() == policyType && 
isMatch(policy.getResources(), scope, false, evalContext);
     }
 
-    boolean isMatch(Map<String, RangerPolicyResource> resources, MatchScope 
scope, boolean mustMatchAllPolicyValues, Map<String, Object> evalContext) {
+    private boolean isMatch(Map<String, RangerPolicyResource> resources, 
MatchScope scope, boolean mustMatchAllPolicyValues, Map<String, Object> 
evalContext) {
         boolean ret = false;
 
         if (MapUtils.isNotEmpty(resources)) {
@@ -413,9 +396,7 @@ public class RangerDefaultPolicyResourceMatcher implements 
RangerPolicyResourceM
     @Override
     public boolean isMatch(RangerAccessResource resource, MatchScope scope, 
Map<String, Object> evalContext) {
         MatchType matchType = getMatchType(resource, evalContext);
-        boolean   ret       = isMatch(scope, matchType);
-
-        return ret;
+        return isMatch(scope, matchType);
     }
 
     @Override
@@ -445,7 +426,7 @@ public class RangerDefaultPolicyResourceMatcher implements 
RangerPolicyResourceM
                 int matchersSize = 0;
 
                 for (RangerResourceDef resourceDef : hierarchy) {
-                    RangerResourceMatcher matcher = 
allMatchers.get(resourceDef.getName());
+                    RangerResourceMatcher matcher = 
getResourceMatcher(resourceDef.getName());
                     if (matcher != null) {
                         matchersSize++;
                         if (!matcher.isMatchAny()) {
@@ -458,12 +439,14 @@ public class RangerDefaultPolicyResourceMatcher 
implements RangerPolicyResourceM
                     ret = MatchType.SELF;
                 } else if (lastNonAnyMatcherIndex == 0) {
                     ret = MatchType.ANCESTOR;
+                } else if (resourceKeysSize == 0) {
+                    ret = MatchType.DESCENDANT;
                 } else {
                     int index = 0;
                     for (RangerResourceDef resourceDef : hierarchy) {
 
                         String resourceName = resourceDef.getName();
-                        RangerResourceMatcher matcher = 
allMatchers.get(resourceName);
+                        RangerResourceMatcher matcher = 
getResourceMatcher(resourceName);
                         String resourceValue = resource.getValue(resourceName);
 
                         if (resourceValue != null) {
@@ -513,6 +496,31 @@ public class RangerDefaultPolicyResourceMatcher implements 
RangerPolicyResourceM
         return ret;
     }
 
+    private static boolean 
isHierarchyValidForResources(List<RangerResourceDef> hierarchy, Map<String, ?> 
resources) {
+        boolean ret = true;
+
+        if (hierarchy != null) {
+            boolean skipped = false;
+
+            for (RangerResourceDef resourceDef : hierarchy) {
+                String resourceName = resourceDef.getName();
+                Object resourceValue = resources.get(resourceName);
+
+                if (resourceValue == null) {
+                    if (!skipped) {
+                        skipped = true;
+                    }
+                } else {
+                    if (skipped) {
+                        ret = false;
+                        break;
+                    }
+                }
+            }
+        }
+
+        return ret;
+    }
     private List<RangerResourceDef> getMatchingHierarchy(Set<String> 
resourceKeys) {
         List<RangerResourceDef> ret = null;
 
@@ -560,33 +568,7 @@ public class RangerDefaultPolicyResourceMatcher implements 
RangerPolicyResourceM
                     aValidHierarchy = getMatchingHierarchy(resource.getKeys());
                 }
             }
-
-            if (aValidHierarchy != null) {
-                boolean isValid = true;
-                boolean skipped = false;
-
-                for (RangerResourceDef resourceDef : aValidHierarchy) {
-                    String resourceName = resourceDef.getName();
-                    String resourceValue = resource.getValue(resourceName);
-
-                    if (resourceValue == null) {
-                        if (!skipped) {
-                            skipped = true;
-                        }
-                    } else {
-                        if (skipped) {
-                            isValid = false;
-                            break;
-                        }
-                    }
-                }
-
-                if (!isValid) {
-                    aValidHierarchy = null;
-                }
-            }
-
-            ret = aValidHierarchy;
+            ret = isHierarchyValidForResources(aValidHierarchy, 
resource.getAsMap()) ? aValidHierarchy : null;
         } else {
             ret = getMatchingHierarchy(policyResourcesKeySet);
         }
@@ -660,7 +642,7 @@ public class RangerDefaultPolicyResourceMatcher implements 
RangerPolicyResourceM
         return sb;
     }
 
-    protected static RangerResourceMatcher 
createResourceMatcher(RangerResourceDef resourceDef, RangerPolicyResource 
resource) {
+    private static RangerResourceMatcher 
createResourceMatcher(RangerResourceDef resourceDef, RangerPolicyResource 
resource) {
         if(LOG.isDebugEnabled()) {
             LOG.debug("==> 
RangerDefaultPolicyResourceMatcher.createResourceMatcher(" + resourceDef + ", " 
+ resource + ")");
         }

http://git-wip-us.apache.org/repos/asf/ranger/blob/847e121f/agents-common/src/main/java/org/apache/ranger/plugin/util/RangerObjectFactory.java
----------------------------------------------------------------------
diff --git 
a/agents-common/src/main/java/org/apache/ranger/plugin/util/RangerObjectFactory.java
 
b/agents-common/src/main/java/org/apache/ranger/plugin/util/RangerObjectFactory.java
index 1a48151..759fd19 100644
--- 
a/agents-common/src/main/java/org/apache/ranger/plugin/util/RangerObjectFactory.java
+++ 
b/agents-common/src/main/java/org/apache/ranger/plugin/util/RangerObjectFactory.java
@@ -34,6 +34,6 @@ public class RangerObjectFactory {
        }
 
        public RangerServiceDefHelper createServiceDefHelper(RangerServiceDef 
serviceDef, boolean useCache) {
-               return new RangerServiceDefHelper(serviceDef, useCache);
+               return new RangerServiceDefHelper(serviceDef, useCache, true);
        }
 }

http://git-wip-us.apache.org/repos/asf/ranger/blob/847e121f/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceDefHelper.java
----------------------------------------------------------------------
diff --git 
a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceDefHelper.java
 
b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceDefHelper.java
index 274028e..584e88e 100644
--- 
a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceDefHelper.java
+++ 
b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceDefHelper.java
@@ -109,7 +109,7 @@ public class TestRangerServiceDefHelper {
                _helper = new RangerServiceDefHelper(_serviceDef);
                assertFalse("Graph was valid!", _helper.isResourceGraphValid());
        }
-       
+
        @Test
        public final void test_isResourceGraphValid_forest() {
                /*

http://git-wip-us.apache.org/repos/asf/ranger/blob/847e121f/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceDefValidator.java
----------------------------------------------------------------------
diff --git 
a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceDefValidator.java
 
b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceDefValidator.java
index ca055ff..72c4520 100644
--- 
a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceDefValidator.java
+++ 
b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceDefValidator.java
@@ -432,8 +432,24 @@ public class TestRangerServiceDefValidator {
                _utils.checkFailureForMissingValue(_failures, "resource level");
                _utils.checkFailureForSemanticError(_failures, "resource 
level", "20"); // level 20 is duplicate for 1 hierarchy
                _utils.checkFailureForSemanticError(_failures, "resource 
level", "10"); // level 10 is duplicate for another hierarchy
-               
-               Object[][] data_good = new Object[][] {
+
+        data_bad = new Object[][] {
+                //  { name,  excludesSupported, recursiveSupported, mandatory, 
reg-exp, parent-level, level }
+                { "db",            null, null, null, null, "" ,             10 
},
+                { "table",         null, null, null, null, "db",            20 
},
+                { "column-family", null, null, null, null, "table",         15 
}, // level is smaller than table!
+                { "column",        null, null, null, null, "column-family", 30 
},
+                { "udf",           null, null, null, null, "db",            15 
},
+        };
+        resourceDefs = _utils.createResourceDefs(data_bad);
+        when(_serviceDef.getResources()).thenReturn(resourceDefs);
+        when(_serviceDef.getName()).thenReturn("service-name");
+        when(_serviceDef.getUpdateTime()).thenReturn(new Date());
+
+        _failures.clear(); 
assertFalse(_validator.isValidResourceGraph(_serviceDef, _failures));
+        _utils.checkFailureForSemanticError(_failures, "resource level", 
"15"); // level 20 is duplicate for 1 hierarchy
+
+        Object[][] data_good = new Object[][] {
                        //  { name,  excludesSupported, recursiveSupported, 
mandatory, reg-exp, parent-level, level }
                                { "db",     null, null, null, null, "" ,     
-10 }, // -ve level is ok
                                { "table",  null, null, null, null, "db",    0 
},   // 0 level is ok
@@ -444,7 +460,21 @@ public class TestRangerServiceDefValidator {
                when(_serviceDef.getResources()).thenReturn(resourceDefs);
                _failures.clear(); 
assertTrue(_validator.isValidResourceGraph(_serviceDef, _failures));
                assertTrue(_failures.isEmpty());
-       }
+
+        Object[][] data_cycles = new Object[][] {
+                //  { name,  excludesSupported, recursiveSupported, mandatory, 
reg-exp, parent-level, level }
+                { "db",     null, null, null, null, "column" ,     -10 }, // 
-ve level is ok
+                { "table",  null, null, null, null, "db",    0 },   // 0 level 
is ok
+                { "column", null, null, null, null, "table", 10 },  // level 
is null!
+                { "udf",    null, null, null, null, "db",    -5 },   // should 
not conflict as it belong to a different hierarchy
+        };
+
+        resourceDefs = _utils.createResourceDefs(data_cycles);
+        when(_serviceDef.getResources()).thenReturn(resourceDefs);
+        _failures.clear(); assertFalse("Graph was valid!", 
_validator.isValidResourceGraph(_serviceDef, _failures));
+        assertFalse(_failures.isEmpty());
+        _utils.checkFailureForSemanticError(_failures, "resource graph");
+    }
        
        @Test
        public final void test_isValidResources_happyPath() {

http://git-wip-us.apache.org/repos/asf/ranger/blob/847e121f/agents-common/src/test/resources/resourcematcher/test_defaultpolicyresourcematcher_for_hdfs_policy.json
----------------------------------------------------------------------
diff --git 
a/agents-common/src/test/resources/resourcematcher/test_defaultpolicyresourcematcher_for_hdfs_policy.json
 
b/agents-common/src/test/resources/resourcematcher/test_defaultpolicyresourcematcher_for_hdfs_policy.json
index b779090..3aa519b 100644
--- 
a/agents-common/src/test/resources/resourcematcher/test_defaultpolicyresourcematcher_for_hdfs_policy.json
+++ 
b/agents-common/src/test/resources/resourcematcher/test_defaultpolicyresourcematcher_for_hdfs_policy.json
@@ -132,8 +132,8 @@
       },
       "tests": [
         {
-          "name": "No match for 'path=/finance/tax/refund' policy",
-          "type": "anyMatch",
+          "name": "Exact match for 'path=/finance/tax/refund' policy",
+          "type": "exactMatch",
           "policy" : {
             "service" : "any",
             "name" : "test",

http://git-wip-us.apache.org/repos/asf/ranger/blob/847e121f/agents-common/src/test/resources/resourcematcher/test_defaultpolicyresourcematcher_for_resource_specific_policy.json
----------------------------------------------------------------------
diff --git 
a/agents-common/src/test/resources/resourcematcher/test_defaultpolicyresourcematcher_for_resource_specific_policy.json
 
b/agents-common/src/test/resources/resourcematcher/test_defaultpolicyresourcematcher_for_resource_specific_policy.json
index 6b774f8..4373647 100644
--- 
a/agents-common/src/test/resources/resourcematcher/test_defaultpolicyresourcematcher_for_resource_specific_policy.json
+++ 
b/agents-common/src/test/resources/resourcematcher/test_defaultpolicyresourcematcher_for_resource_specific_policy.json
@@ -139,7 +139,7 @@
       },
       "tests": [
         {
-          "name": "Ancestor match for 'finance,hr,tmp*:tax,employee,tmp*' 
policy",
+          "name": "Descendant match for 'finance,hr,tmp*:tax,employee,tmp*:' 
policy",
           "type": "descendantMatch",
           "policy" : {
             "service" : "any",
@@ -259,7 +259,7 @@
       },
       "tests": [
         {
-          "name": "Ancestor match for 
'finance,hr,tmp*:tax,employee,tmp*:refund,salary,tmp*' policy",
+          "name": "Ancestor match for 'finance,hr,tmp*:tax,employee,tmp*:' 
policy",
           "type": "ancestorMatch",
           "policy" : {
             "service" : "any",
@@ -283,7 +283,7 @@
           "result" : true
         },
         {
-          "name": "Ancestor match for 'finance,hr,tmp*' policy",
+          "name": "No match for 'finance,hr,tmp*::refund,salary,tmp*' policy",
           "type": "anyMatch",
           "policy" : {
             "service" : "any",
@@ -307,7 +307,7 @@
           "result" : false
         },
         {
-          "name": "No match for 'finance,hr,tmp*::*,salary,tmp*' policy",
+          "name": "Ancestor match for 'finance,hr,tmp*::' policy",
           "type": "ancestorMatch",
           "policy" : {
             "service" : "any",

Reply via email to