----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/74232/#review224965 -----------------------------------------------------------
agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java Line 704 (original), 700 (patched) <https://reviews.apache.org/r/74232/#comment313802> "all" seems to be an accessType in Hive. Is this special handling necessary in policy engine? agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java Lines 838 (patched) <https://reviews.apache.org/r/74232/#comment313801> if request is not RangerAccessRequestImpl, #838 would result in cast error. Please consider handling this case - perhaps by a request-wrapper class like: public class RangerAccessRequestWrapper implements RangerAccessRequest { private final RangerAccessRequest request; private final String accessType; private final boolean isAccessTypeAny; private final boolean isAccessTypeDelegatedAdmin; public RangerAccessRequestWrapper(RangerAccessRequest request, String accessType) { this.request = request; this.accessType = accessType; this.isAccessTypeAny = StringUtils.equals(accessType, RangerPolicyEngine.ANY_ACCESS) this.isAccessTypeDelegatedAdmin = StringUtils.equals(accessType, RangerPolicyEngine.ADMIN_ACCESS); } @Override public String getAccessType() { return accessType; } @Override public boolean isAccessTypeAny() { return isAccessTypeAny; } @Override public boolean isAccessTypeDelegatedAdmin() { return isAccessTypeDelegatedAdmin; } // other methods simply call corresponding method on request, like: @Override public RangerAccessResource getResource() { return request.getResource(); } ... } agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java Lines 262 (patched) <https://reviews.apache.org/r/74232/#comment313803> Consider adding following method to have all type casting in one place: class RangerAccessRequestUtil { ... public static Set<String> getAllRequestedAccessTypes(Map<String, Object> context) { return (Set<String>) context.get(KEY_CONTEXT_ACCESSTYPES); } } - Madhan Neethiraj On Dec. 6, 2022, 5:58 p.m., Abhay Kulkarni wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/74232/ > ----------------------------------------------------------- > > (Updated Dec. 6, 2022, 5:58 p.m.) > > > Review request for ranger, madhan, Madhan Neethiraj, Ramesh Mani, Sailaja > Polavarapu, and Velmurugan Periasamy. > > > Bugs: RANGER-3999 > https://issues.apache.org/jira/browse/RANGER-3999 > > > Repository: ranger > > > Description > ------- > > If a user-initiated operation requires checking if more than one permission > is granted, then currently, each permission requires a call to internal > Policy Engine API for the same accessed resource. This leads to many > repetitive computations which may be avoided if the policy engine API > supports multiple permissions. In that case, optimization may be achieved by > pushing authorization for multiple permissions down to the lowest possible > level. > > > Diffs > ----- > > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java > 23db18f3a > > agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java > 520ddf865 > > agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java > d3fc27d7d > > agents-common/src/main/java/org/apache/ranger/plugin/util/RangerAccessRequestUtil.java > df03ed1c4 > > > Diff: https://reviews.apache.org/r/74232/diff/2/ > > > Testing > ------- > > Passed all existing unit tests. > > > Thanks, > > Abhay Kulkarni > >
