----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71023/#review216420 -----------------------------------------------------------
security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java Line 1563 (original), 1564 (patched) <https://reviews.apache.org/r/71023/#comment303644> Perhaps this block can be changed to for clarity. if(request != null && updateIfExists != null && updateIfExists.ignoreIgnoreCase("true")) { RangerPolicy existingPolicy = null; if (StringUtils.isNotEmpty(policy.getGuid())) { existingPolicy = getPolicyByGuid(policy.getGuid()); } if (existingPolicy == null) { String serviceName = request.getParameter(PARAM_SERVICE_NAME); String policyName = request.getParameter(PARAM_POLICY_NAME); if (serviceName == null) { serviceName = (String) request.getAttribute(PARAM_SERVICE_NAME); } if (StringUtils.isNotEmpty(serviceName)) { policy.setService(serviceName); } if (policyName == null) { policyName = (String) request.getAttribute(PARAM_POLICY_NAME); } if (StringUtils.isNotEmpty(policyName)) { policy.setName(StringUtils.trim(policyName)); } if (StringUtils.isNotEmpty(serviceName) && StringUtils.isNotEmpty(policyName)) { String zoneName = request.getParameter(PARAM_ZONE_NAME); if (StringUtils.isBlank(zoneName)) { zoneName = (String) request.getAttribute(PARAM_ZONE_NAME); } if (StringUtils.isNotBlank(zoneName)) { policy.setZoneName(StringUtils.trim(zoneName)); } try { if (StringUtils.isNotEmpty(zoneName)) { existingPolicy = getPolicyByNameAndZone(policy.getService(), policy.getName(), policy.getZoneName()); } else { existingPolicy = getPolicyByName(policy.getService(), policy.getName()); } if (existingPolicy != null) { policy.setId(existingPolicy.getId()); ret = updatePolicy(policy); } } catch(Exception excp) { LOG.error("updatePolicy(" + policy + ") failed", excp); throw restErrorUtil.createRESTException(excp.getMessage()); } } } } security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java Lines 3403 (patched) <https://reviews.apache.org/r/71023/#comment303645> Please consider checking if the size of the returned policy list is exactly one if policy list is not empty. - Abhay Kulkarni On July 8, 2019, 5:39 a.m., Pradeep Agrawal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71023/ > ----------------------------------------------------------- > > (Updated July 8, 2019, 5:39 a.m.) > > > Review request for ranger, Ankita Sinha, bhavik patel, Gautam Borad, Abhay > Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Nitin Galave, Ramesh > Mani, Sailaja Polavarapu, and Velmurugan Periasamy. > > > Bugs: RANGER-2500 > https://issues.apache.org/jira/browse/RANGER-2500 > > > Repository: ranger > > > Description > ------- > > **Problem Statement:** Zone Policies import may fail when > 'updateIfExists=true' is passed through curl. > After zone implementation its possible that same policy may exist in a zone > and unzone. if it exists unzone then the current implementation will bring > that existing db object in memory and try to update the same with new > request. since the request will try to update unzone policy to a zone policy; > the request will fail during the policy update validation as there is a check > that policy zone can't be updated. > > **Proposed Solution:** The proposed solution will bring policy from > respective zone only. This way policy update request will happen with in the > same zone policy and update validation shall pass. > > **Other notes:** > When updateIfExists true is passed then following flow will happen. > 1) First it will check if the new policy guid and the existing policy guid is > same or not. if found same then it will try to update that record. please > note that if existing object is in unzone and the new request is for the zone > then request may fail due to policy update validation check as mentioned > above. > 2) if first condition is not applied then it will try to get a policy by name > and zone. if record is found then it will try to update that record. > 3) if first and second condition is not applied then it will try to get a > policy by name. if record is found then it will try to update that record. > 4) if any of the above condition fail then policy update will fail but if all > three conditions are not met then it will try to create the policy. > > > Diffs > ----- > > security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java > 171d73bfa > > > Diff: https://reviews.apache.org/r/71023/diff/1/ > > > Testing > ------- > > Tested with 'updateIfExists=true' param and unable to reproduce the case. > > > Thanks, > > Pradeep Agrawal > >
