vyommani commented on code in PR #646:
URL: https://github.com/apache/ranger/pull/646#discussion_r2306110218


##########
agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java:
##########
@@ -1072,13 +1072,36 @@ private void updateFromGdsResult(RangerAccessResult 
result) {
 
         if (gdsResult != null) {
             if (result.getPolicyType() == RangerPolicy.POLICY_TYPE_ACCESS) {
+                // pick access result from GDS policies only if there is no 
decision yet
                 if (!result.getIsAccessDetermined() && 
gdsResult.getIsAllowed()) {
                     result.setIsAllowed(true);
                     result.setIsAccessDetermined(true);
                     result.setPolicyId(gdsResult.getPolicyId());
                     result.setPolicyVersion(gdsResult.getPolicyVersion());
                     
result.setPolicyPriority(RangerPolicy.POLICY_PRIORITY_NORMAL);
                 }
+            } else if (result.getPolicyType() == 
RangerPolicy.POLICY_TYPE_ROWFILTER) {

Review Comment:
   updateFromGdsResult contains lots of repetitive code , particularly handling 
the different policy type. Can we simplify the logic  by extracting the common 
code in a helper function and call the method instead. 
   
    private void updateCommonFields(RangerAccessResult result, GdsAccessResult 
gdsResult) {
           result.setIsAllowed(true);
           result.setIsAccessDetermined(true);
           result.setPolicyId(gdsResult.getPolicyId());
           result.setPolicyVersion(gdsResult.getPolicyVersion());
           result.setPolicyPriority(RangerPolicy.POLICY_PRIORITY_NORMAL);
       }
   
       private void    updateFromGdsResult(RangerAccessResult result) {
           LOG.debug("==> updateFromGdsResult(result={})", result);
   
           RangerAccessRequest request   = result.getAccessRequest();
           GdsAccessResult     gdsResult = request != null ? 
RangerAccessRequestUtil.getGdsResultFromContext(request.getContext()) : null;
   
           if (gdsResult != null) {
               if (result.getPolicyType() == RangerPolicy.POLICY_TYPE_ACCESS) {
                   // pick access result from GDS policies only if there is no 
decision yet
                   if (!result.getIsAccessDetermined() && 
gdsResult.getIsAllowed()) {
                      updateCommonFields(result, gdsResult);
                   }
               }else if (result.getPolicyType() == 
RangerPolicy.POLICY_TYPE_ROWFILTER) {
                   // pick row-filter from GDS policies only if there is no 
decision yet
                   if (result.getPolicyId() == -1 && 
CollectionUtils.isNotEmpty(gdsResult.getRowFilters())) {
                       updateCommonFields(result, gdsResult);
                       result.setFilterExpr(gdsResult.getRowFilters().get(0));
                   }
               } else if (result.getPolicyType() == 
RangerPolicy.POLICY_TYPE_DATAMASK) {
                   // pick data-mask from GDS policies only if there is no 
decision yet
                   if (result.getPolicyId() == -1 && 
StringUtils.isNotEmpty(gdsResult.getMaskType())) {
                       updateCommonFields(result, gdsResult);
                       result.setMaskedValue(gdsResult.getMaskedValue());
                       result.setMaskCondition(gdsResult.getMaskCondition());
                   }
               }



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to