----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32007/#review76342 -----------------------------------------------------------
agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyRepository.java <https://reviews.apache.org/r/32007/#comment123884> Instantiation of contextEnrichers seems not right here; looks like the class member should be used here. Please review. agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyRepository.java <https://reviews.apache.org/r/32007/#comment123885> This debug log seems to be a copy/paste error. Please review. agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyRepository.java <https://reviews.apache.org/r/32007/#comment123886> request.toString() would contain the entire request data - including resource, user, groups, access-type, etc. I guess request.getResource().toString() was intented here. Please review this and other such occurances. agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java <https://reviews.apache.org/r/32007/#comment123888> the condition should be: (matchResult || (isAnyAccess && headMatchResult)). policyItems should not be evaluated when its not a full resource match (matchResult = false) and access-type is _not_ "any" (isAny = false). agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java <https://reviews.apache.org/r/32007/#comment123891> allSupportedAccessTypes member may not be necessary, given hasAllPerms can be used instead. agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java <https://reviews.apache.org/r/32007/#comment123892> when "*" is the resourceName, no change to the evalOrderScore (priority level). In other cases, evalOrderScore will go up by RANGER_POLICY_EVAL_RESERVED_SLOTS_PER_LEVEL_NUMBER agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java <https://reviews.apache.org/r/32007/#comment123893> priorityLevel = RANGER_POLICY_EVAL_RESERVED_SLOTS_NUMBER + index * RANGER_POLICY_EVAL_RESERVED_SLOTS_PER_LEVEL_NUMBER; should be: priorityLevel += RANGER_POLICY_EVAL_RESERVED_SLOTS_PER_LEVEL_NUMBER; agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java <https://reviews.apache.org/r/32007/#comment123897> priorityLevel adjustment based on user/group/accessType should be done outside of this resource-level loop - only once per policy. agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java <https://reviews.apache.org/r/32007/#comment123898> I think a better scoring for accessType count would be something like: (25.0 * accessPerms.size()) / serviceDef.getAccessTypes().size(). i.e. set 25 as the max deduction for policies that contain all accessTypes. and pro-rate other policies depending upon the number of accessTypes in the policy. agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java <https://reviews.apache.org/r/32007/#comment123895> looks like this break should be removed. Else subsequent resource level's will not infulence the score. agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java <https://reviews.apache.org/r/32007/#comment123899> This method is not needed. Please see earlier comment on removing allSupportedAccessTypes member. - Madhan Neethiraj On March 12, 2015, 11:37 p.m., Madhan Neethiraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32007/ > ----------------------------------------------------------- > > (Updated March 12, 2015, 11:37 p.m.) > > > Review request for ranger. > > > Bugs: RANGER-307 > https://issues.apache.org/jira/browse/RANGER-307 > > > Repository: ranger > > > Description > ------- > > RANGER-307: Improvement to make policy execution more efficient by sorting > policies in a way that a policy more likely to allow access is evaluated > first. > > > Diffs > ----- > > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/CacheMap.java > PRE-CREATION > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerAccessData.java > PRE-CREATION > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerAccessResult.java > 2eaec16 > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java > 51787ac > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEvaluatorFacade.java > PRE-CREATION > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyRepository.java > PRE-CREATION > > agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java > 0ac5eed > > agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java > PRE-CREATION > > agents-common/src/test/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluatorTest.java > 9256995 > agents-common/src/test/resources/policyengine/test_policyengine_hdfs.json > 943fe80 > pom.xml 7df033d > src/main/assembly/ranger-src.xml cf6b1da > > Diff: https://reviews.apache.org/r/32007/diff/ > > > Testing > ------- > > Verified that the code builds cleanly and all existing unit testcases pass. > > > Thanks, > > Madhan Neethiraj > >
