Re: Review Request 71502: Ranger Import of fails if policy type is not passed.

2019-09-18 Thread Mateen Mansoori 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71502/
---

(Updated Sept. 19, 2019, 5:32 a.m.)


Review request for ranger, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, 
Pradeep Agrawal, and Velmurugan Periasamy.


Bugs: RANGER-2574
https://issues.apache.org/jira/browse/RANGER-2574


Repository: ranger


Description
---

Ranger Import of fails(NPE) if policy type is'NULL' or it is not passed.


Diffs (updated)
-

  
security-admin/src/main/java/org/apache/ranger/service/RangerPolicyService.java 
78954af 


Diff: https://reviews.apache.org/r/71502/diff/2/

Changes: https://reviews.apache.org/r/71502/diff/1-2/


Testing
---

Import policy was failing(NPE) when policy type is 'NULL' or it is not passed, 
Issue(NPE) rsolved with this changes. This issue was related to RANGER-2526.


Thanks,

Mateen Mansoori

Re: Review Request 71502: Ranger Import of fails if policy type is not passed.

2019-09-18 Thread Mateen Mansoori 


> On Sept. 18, 2019, 4:01 p.m., Abhay Kulkarni wrote:
> >

Please review again.

Thanks,


> On Sept. 18, 2019, 4:01 p.m., Abhay Kulkarni wrote:
> > security-admin/src/main/java/org/apache/ranger/service/RangerPolicyService.java
> > Line 202 (original), 204 (patched)
> > 
> >
> > Consider using this code fragment:
> > 
> > Integer policyType = vObj.getPolicyType();
> > 
> > if (policyType == null || policyType == 
> > RangerPolicy.POLICY_TYPE_ACCESS) {
> >result = null;
> > } else if (StringUtils.isNotBlank(fieldName)) {
> >if (policyType == RangerPolicy.POLICY_TYPE_ROWFILETER || policyType 
> > == RangerPolicy.POLICY_TYPE_DATAMASK) {
> >   result = null;
> >}
> > }
> > 
> > - policyType being null does not require a warning; implicitly it is 
> > considered an ACCESS policy.

When policy type is null, implicitly it is considered as an ACCESS type policy, 
so at that time log needs to be generated as 'isDenyAllElse' is the part of 
'ACCESS' type policy so returning result as it is( i.e., result = ""; not null).

Thanks for review,


- Mateen


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71502/#review217820
---


On Sept. 18, 2019, 11:48 a.m., Mateen Mansoori wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71502/
> ---
> 
> (Updated Sept. 18, 2019, 11:48 a.m.)
> 
> 
> Review request for ranger, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, 
> Pradeep Agrawal, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-2574
> https://issues.apache.org/jira/browse/RANGER-2574
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> Ranger Import of fails(NPE) if policy type is'NULL' or it is not passed.
> 
> 
> Diffs
> -
> 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerPolicyService.java
>  78954af 
> 
> 
> Diff: https://reviews.apache.org/r/71502/diff/1/
> 
> 
> Testing
> ---
> 
> Import policy was failing(NPE) when policy type is 'NULL' or it is not 
> passed, Issue(NPE) rsolved with this changes. This issue was related to 
> RANGER-2526.
> 
> 
> Thanks,
> 
> Mateen Mansoori
> 
>

Re: Review Request 71502: Ranger Import of fails if policy type is not passed.

2019-09-18 Thread Abhay Kulkarni 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71502/#review217820
---




security-admin/src/main/java/org/apache/ranger/service/RangerPolicyService.java
Lines 200 (patched)


Please review log statement to cleanly separate two values being printed.



security-admin/src/main/java/org/apache/ranger/service/RangerPolicyService.java
Line 201 (original), 203 (patched)


Why is vObj not checked for null (as it used to be) before proceeding to 
de-reference it?



security-admin/src/main/java/org/apache/ranger/service/RangerPolicyService.java
Line 202 (original), 204 (patched)


Consider using this code fragment:

Integer policyType = vObj.getPolicyType();

if (policyType == null || policyType == RangerPolicy.POLICY_TYPE_ACCESS) {
   result = null;
} else if (StringUtils.isNotBlank(fieldName)) {
   if (policyType == RangerPolicy.POLICY_TYPE_ROWFILETER || policyType == 
RangerPolicy.POLICY_TYPE_DATAMASK) {
  result = null;
   }
}

- policyType being null does not require a warning; implicitly it is 
considered an ACCESS policy.



security-admin/src/main/java/org/apache/ranger/service/RangerPolicyService.java
Line 210 (original), 214 (patched)


Please add a log when exiting function to match log when entering it.


- Abhay Kulkarni


On Sept. 18, 2019, 11:48 a.m., Mateen Mansoori wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71502/
> ---
> 
> (Updated Sept. 18, 2019, 11:48 a.m.)
> 
> 
> Review request for ranger, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, 
> Pradeep Agrawal, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-2574
> https://issues.apache.org/jira/browse/RANGER-2574
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> Ranger Import of fails(NPE) if policy type is'NULL' or it is not passed.
> 
> 
> Diffs
> -
> 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerPolicyService.java
>  78954af 
> 
> 
> Diff: https://reviews.apache.org/r/71502/diff/1/
> 
> 
> Testing
> ---
> 
> Import policy was failing(NPE) when policy type is 'NULL' or it is not 
> passed, Issue(NPE) rsolved with this changes. This issue was related to 
> RANGER-2526.
> 
> 
> Thanks,
> 
> Mateen Mansoori
> 
>

Review Request 71502: Ranger Import of fails if policy type is not passed.

2019-09-18 Thread Mateen Mansoori 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71502/
---

Review request for ranger, Abhay Kulkarni, Madhan Neethiraj, Mehul Parikh, 
Pradeep Agrawal, and Velmurugan Periasamy.


Bugs: RANGER-2574
https://issues.apache.org/jira/browse/RANGER-2574


Repository: ranger


Description
---

Ranger Import of fails(NPE) if policy type is'NULL' or it is not passed.


Diffs
-

  
security-admin/src/main/java/org/apache/ranger/service/RangerPolicyService.java 
78954af 


Diff: https://reviews.apache.org/r/71502/diff/1/


Testing
---

Import policy was failing(NPE) when policy type is 'NULL' or it is not passed, 
Issue(NPE) rsolved with this changes. This issue was related to RANGER-2526.


Thanks,

Mateen Mansoori