You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by "Ramachandran (Jira)" <ji...@apache.org> on 2022/09/16 13:50:00 UTC
[jira] [Comment Edited] (RANGER-3903) Improvements in RangerPolicyDeltaUtil--> applyDeltas method
[ https://issues.apache.org/jira/browse/RANGER-3903?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17603469#comment-17603469 ]
Ramachandran edited comment on RANGER-3903 at 9/16/22 1:49 PM:
---------------------------------------------------------------
The review is available here -[https://reviews.apache.org/r/74112/]
cc >> [~madhan@apache.org] [~abhayk]
was (Author: JIRAUSER295265):
The review is available here -https://reviews.apache.org/r/74112/
> Improvements in RangerPolicyDeltaUtil--> applyDeltas method
> -----------------------------------------------------------
>
> Key: RANGER-3903
> URL: https://issues.apache.org/jira/browse/RANGER-3903
> Project: Ranger
> Issue Type: Improvement
> Components: Ranger
> Affects Versions: 3.0.0
> Reporter: Ramachandran
> Assignee: Ramachandran
> Priority: Major
> Attachments: 0001-RANGER-3897-Improvements-in-RangerPolicyDeltaUtil-ap.patch
>
>
>
> {code:java}
> while (iter.hasNext()) {
> RangerPolicy policy = iter.next();
> if (policyId.equals(policy.getId()) && (changeType == RangerPolicyDelta.CHANGE_TYPE_POLICY_DELETE || changeType == RangerPolicyDelta.CHANGE_TYPE_POLICY_UPDATE)) {
> deletedPolicies.add(policy);
> iter.remove();
> }
> }
> switch (changeType) {
> case RangerPolicyDelta.CHANGE_TYPE_POLICY_CREATE:
> {
> if (CollectionUtils.isNotEmpty(deletedPolicies)) {
> LOG.warn("Unexpected: found existing policy for CHANGE_TYPE_POLICY_CREATE: " + Arrays.toString(deletedPolicies.toArray()));
> }
> break;
> }
> case RangerPolicyDelta.CHANGE_TYPE_POLICY_UPDATE:
> {
> if (CollectionUtils.isEmpty(deletedPolicies) || deletedPolicies.size() > 1) {
> LOG.warn("Unexpected: found no policy or multiple policies for CHANGE_TYPE_POLICY_UPDATE: " + Arrays.toString(deletedPolicies.toArray()));
> }
> break;
> }
> case RangerPolicyDelta.CHANGE_TYPE_POLICY_DELETE:
> {
> if (CollectionUtils.isEmpty(deletedPolicies) || deletedPolicies.size() > 1) {
> LOG.warn("Unexpected: found no policy or multiple policies for CHANGE_TYPE_POLICY_DELETE: " + Arrays.toString(deletedPolicies.toArray()));
> }
> break;
> }
> default:
> break;
> }{code}
> From the above code, we iterate delta policies and check if this policy exists in the existing policy, we add that to deletePolicies list.
> If a delta change type is created, we expect that it should not be in the existing old policy which should not have happened. So the below block code is not needed
> {code:java}
> case RangerPolicyDelta.CHANGE_TYPE_POLICY_CREATE: {
> if (CollectionUtils.isNotEmpty(deletedPolicies)) {
> LOG.warn("Unexpected: found existing policy for CHANGE_TYPE_POLICY_CREATE: " + Arrays.toString(deletedPolicies.toArray()));
> }
> break;
> }{code}
> Proposal :
> The same id can be for multiple policies here.?
> If policy Id is uniqiue and found a match, why are still we iterating here instead of adding a break
>
> {code:java}
> while (iter.hasNext()) {
> RangerPolicy policy = iter.next();
> if (policyId.equals(policy.getId()) && (changeType == RangerPolicyDelta.CHANGE_TYPE_POLICY_DELETE || changeType == RangerPolicyDelta.CHANGE_TYPE_POLICY_UPDATE)) {
> deletedPolicies.add(policy);
> iter.remove();
> }
> } {code}
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)