Re: Review Request 74705: RANGER-4486: zone-v2 PUT API Partial update #2

2023-10-31 Thread Madhan Neethiraj 

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


Ship it!




Ship It!

- Madhan Neethiraj


On Oct. 31, 2023, 3:49 a.m., Subhrat Chaudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74705/
> ---
> 
> (Updated Oct. 31, 2023, 3:49 a.m.)
> 
> 
> Review request for ranger, Anand Nadar, Ankita Sinha, Madhan Neethiraj, 
> Monika Kachhadiya, and Prashant Satam.
> 
> 
> Bugs: RANGER-4486
> https://issues.apache.org/jira/browse/RANGER-4486
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> Following issues are noticed in the zone-v2 PUT API - 
> /service/public/v2/api/zones-v2/{zone-id}/partial:
> 1. If adminsToRemove or auditorsToRemove have some principal that doesn't 
> exist, response is true (updated to throw exception in this case).
> 2. If tagServicesToRemove have some tag service name that doesn't exist, 
> response is true (updated to throw exception in this case).
> 3. If resourcesToRemove have some resource that doesn't exist, response is 
> true (updated to throw exception in this case).
> 4. If the resource, is updated, the audit data i.e. createdBy and createTime 
> is overwritten,
> 
> 
> Diffs
> -
> 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/util/RangerSecurityZoneHelper.java
>  fbdacd4a6 
>   security-admin/src/main/java/org/apache/ranger/rest/SecurityZoneREST.java 
> f45cdd396 
> 
> 
> Diff: https://reviews.apache.org/r/74705/diff/2/
> 
> 
> Testing
> ---
> 
> Validations done:
> 1.Tried to remove resources (one valid and one invalid) from a zone using 
> partial PUT API - error thrown.
> 2.Tried to remove tag services (one valid and one invalid) from a zone using 
> partial PUT API - error thrown.
> 3.Tried to remove user (one valid and one invalid) from a zone using partial 
> PUT API - error thrown.
> 4.Updated resource using zone-v2 PUT API - createdBy/createTime available in 
> updated resource in the zone.
> 
> 
> Thanks,
> 
> Subhrat Chaudhary
> 
>

Re: Review Request 74705: RANGER-4486: zone-v2 PUT API Partial update #2

2023-10-30 Thread Subhrat Chaudhary via Review Board 


> On Oct. 31, 2023, 4:01 a.m., Madhan Neethiraj wrote:
> > agents-common/src/main/java/org/apache/ranger/plugin/util/RangerSecurityZoneHelper.java
> > Lines 147 (patched)
> > 
> >
> > Is a HashSet<>() needed here? Why not 
> > zone.getTagServices().containsAll(changeData.getTagServicesToRemove())?

Added to improve performance, but in case of small list should not make 
difference. Updated.


- Subhrat


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


On Oct. 31, 2023, 9:19 a.m., Subhrat Chaudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74705/
> ---
> 
> (Updated Oct. 31, 2023, 9:19 a.m.)
> 
> 
> Review request for ranger, Anand Nadar, Ankita Sinha, Madhan Neethiraj, 
> Monika Kachhadiya, and Prashant Satam.
> 
> 
> Bugs: RANGER-4486
> https://issues.apache.org/jira/browse/RANGER-4486
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> Following issues are noticed in the zone-v2 PUT API - 
> /service/public/v2/api/zones-v2/{zone-id}/partial:
> 1. If adminsToRemove or auditorsToRemove have some principal that doesn't 
> exist, response is true (updated to throw exception in this case).
> 2. If tagServicesToRemove have some tag service name that doesn't exist, 
> response is true (updated to throw exception in this case).
> 3. If resourcesToRemove have some resource that doesn't exist, response is 
> true (updated to throw exception in this case).
> 4. If the resource, is updated, the audit data i.e. createdBy and createTime 
> is overwritten,
> 
> 
> Diffs
> -
> 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/util/RangerSecurityZoneHelper.java
>  fbdacd4a6 
>   security-admin/src/main/java/org/apache/ranger/rest/SecurityZoneREST.java 
> f45cdd396 
> 
> 
> Diff: https://reviews.apache.org/r/74705/diff/2/
> 
> 
> Testing
> ---
> 
> Validations done:
> 1.Tried to remove resources (one valid and one invalid) from a zone using 
> partial PUT API - error thrown.
> 2.Tried to remove tag services (one valid and one invalid) from a zone using 
> partial PUT API - error thrown.
> 3.Tried to remove user (one valid and one invalid) from a zone using partial 
> PUT API - error thrown.
> 4.Updated resource using zone-v2 PUT API - createdBy/createTime available in 
> updated resource in the zone.
> 
> 
> Thanks,
> 
> Subhrat Chaudhary
> 
>

Re: Review Request 74705: RANGER-4486: zone-v2 PUT API Partial update #2

2023-10-30 Thread Subhrat Chaudhary via Review Board 

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

(Updated Oct. 31, 2023, 9:19 a.m.)


Review request for ranger, Anand Nadar, Ankita Sinha, Madhan Neethiraj, Monika 
Kachhadiya, and Prashant Satam.


Changes
---

Addressed review comments


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


Repository: ranger


Description
---

Following issues are noticed in the zone-v2 PUT API - 
/service/public/v2/api/zones-v2/{zone-id}/partial:
1. If adminsToRemove or auditorsToRemove have some principal that doesn't 
exist, response is true (updated to throw exception in this case).
2. If tagServicesToRemove have some tag service name that doesn't exist, 
response is true (updated to throw exception in this case).
3. If resourcesToRemove have some resource that doesn't exist, response is true 
(updated to throw exception in this case).
4. If the resource, is updated, the audit data i.e. createdBy and createTime is 
overwritten,


Diffs (updated)
-

  
agents-common/src/main/java/org/apache/ranger/plugin/util/RangerSecurityZoneHelper.java
 fbdacd4a6 
  security-admin/src/main/java/org/apache/ranger/rest/SecurityZoneREST.java 
f45cdd396 


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

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


Testing
---

Validations done:
1.Tried to remove resources (one valid and one invalid) from a zone using 
partial PUT API - error thrown.
2.Tried to remove tag services (one valid and one invalid) from a zone using 
partial PUT API - error thrown.
3.Tried to remove user (one valid and one invalid) from a zone using partial 
PUT API - error thrown.
4.Updated resource using zone-v2 PUT API - createdBy/createTime available in 
updated resource in the zone.


Thanks,

Subhrat Chaudhary

Re: Review Request 74705: RANGER-4486: zone-v2 PUT API Partial update #2

2023-10-30 Thread Madhan Neethiraj 

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




agents-common/src/main/java/org/apache/ranger/plugin/util/RangerSecurityZoneHelper.java
Lines 147 (patched)


Is a HashSet<>() needed here? Why not 
zone.getTagServices().containsAll(changeData.getTagServicesToRemove())?



agents-common/src/main/java/org/apache/ranger/plugin/util/RangerSecurityZoneHelper.java
Lines 187 (patched)


isPrincipalAvailable => isRemoved



agents-common/src/main/java/org/apache/ranger/plugin/util/RangerSecurityZoneHelper.java
Lines 198 (patched)


principal + ": principal not in an admin or auditor zone"


- Madhan Neethiraj


On Oct. 30, 2023, 5:05 p.m., Subhrat Chaudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74705/
> ---
> 
> (Updated Oct. 30, 2023, 5:05 p.m.)
> 
> 
> Review request for ranger, Anand Nadar, Ankita Sinha, Madhan Neethiraj, 
> Monika Kachhadiya, and Prashant Satam.
> 
> 
> Bugs: RANGER-4486
> https://issues.apache.org/jira/browse/RANGER-4486
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> Following issues are noticed in the zone-v2 PUT API - 
> /service/public/v2/api/zones-v2/{zone-id}/partial:
> 1. If adminsToRemove or auditorsToRemove have some principal that doesn't 
> exist, response is true (updated to throw exception in this case).
> 2. If tagServicesToRemove have some tag service name that doesn't exist, 
> response is true (updated to throw exception in this case).
> 3. If resourcesToRemove have some resource that doesn't exist, response is 
> true (updated to throw exception in this case).
> 4. If the resource, is updated, the audit data i.e. createdBy and createTime 
> is overwritten,
> 
> 
> Diffs
> -
> 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/util/RangerSecurityZoneHelper.java
>  fbdacd4a6 
>   security-admin/src/main/java/org/apache/ranger/rest/SecurityZoneREST.java 
> f45cdd396 
> 
> 
> Diff: https://reviews.apache.org/r/74705/diff/1/
> 
> 
> Testing
> ---
> 
> Validations done:
> 1.Tried to remove resources (one valid and one invalid) from a zone using 
> partial PUT API - error thrown.
> 2.Tried to remove tag services (one valid and one invalid) from a zone using 
> partial PUT API - error thrown.
> 3.Tried to remove user (one valid and one invalid) from a zone using partial 
> PUT API - error thrown.
> 4.Updated resource using zone-v2 PUT API - createdBy/createTime available in 
> updated resource in the zone.
> 
> 
> Thanks,
> 
> Subhrat Chaudhary
> 
>

Re: Review Request 74705: RANGER-4486: zone-v2 PUT API Partial update #2

2023-10-30 Thread Subhrat Chaudhary via Review Board 

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

(Updated Oct. 30, 2023, 10:35 p.m.)


Review request for ranger, Anand Nadar, Ankita Sinha, Madhan Neethiraj, Monika 
Kachhadiya, and Prashant Satam.


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


Repository: ranger


Description
---

Following issues are noticed in the zone-v2 PUT API - 
/service/public/v2/api/zones-v2/{zone-id}/partial:
1. If adminsToRemove or auditorsToRemove have some principal that doesn't 
exist, response is true (updated to throw exception in this case).
2. If tagServicesToRemove have some tag service name that doesn't exist, 
response is true (updated to throw exception in this case).
3. If resourcesToRemove have some resource that doesn't exist, response is true 
(updated to throw exception in this case).
4. If the resource, is updated, the audit data i.e. createdBy and createTime is 
overwritten,


Diffs
-

  
agents-common/src/main/java/org/apache/ranger/plugin/util/RangerSecurityZoneHelper.java
 fbdacd4a6 
  security-admin/src/main/java/org/apache/ranger/rest/SecurityZoneREST.java 
f45cdd396 


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


Testing
---

Validations done:
1.Tried to remove resources (one valid and one invalid) from a zone using 
partial PUT API - error thrown.
2.Tried to remove tag services (one valid and one invalid) from a zone using 
partial PUT API - error thrown.
3.Tried to remove user (one valid and one invalid) from a zone using partial 
PUT API - error thrown.
4.Updated resource using zone-v2 PUT API - createdBy/createTime available in 
updated resource in the zone.


Thanks,

Subhrat Chaudhary

Review Request 74705: RANGER-4486: zone-v2 PUT API Partial update #2

2023-10-30 Thread Subhrat Chaudhary via Review Board 

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

Review request for ranger, Anand Nadar, Ankita Sinha, Madhan Neethiraj, Monika 
Kachhadiya, and Prashant Satam.


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


Repository: ranger


Description
---

Following issues are noticed in the zone-v2 PUT API - 
/service/public/v2/api/zones-v2/{zone-id}/partial:
1. If adminsToRemove or auditorsToRemove have some principal that doesn't 
exist, response is true (updated to throw exception in this case).
2. If tagServicesToRemove have some tag service name that doesn't exist, 
response is true (updated to throw exception in this case).
3. If resourcesToRemove have some resource that doesn't exist, response is true 
(updated to throw exception in this case).
4. If the resource, is updated, the audit data i.e. createdBy and createTime is 
overwritten,


Diffs
-

  
agents-common/src/main/java/org/apache/ranger/plugin/util/RangerSecurityZoneHelper.java
 fbdacd4a6 
  security-admin/src/main/java/org/apache/ranger/rest/SecurityZoneREST.java 
f45cdd396 


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


Testing
---

Validations done:
1.Tried to remove resources (one valid and one invalid) from a zone using 
partial PUT API - error thrown.
2.Tried to remove tag services (one valid and one invalid) from a zone using 
partial PUT API - error thrown.
3.Tried to remove user (one valid and one invalid) from a zone using partial 
PUT API - error thrown.
4.Updated resource using zone-v2 PUT API - createdBy/createTime available in 
updated resource in the zone.


Thanks,

Subhrat Chaudhary