Re: Review Request 69175: SENTRY-2433: Dropping object privileges does not include update of dropping user privileges

2018-10-29 Thread Na Li via Review Board 

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

(Updated Oct. 29, 2018, 5:07 p.m.)


Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio 
Pena.


Bugs: sentry-2433
https://issues.apache.org/jira/browse/sentry-2433


Repository: sentry


Description
---

1) use PermissionsUpdate.ALL_PRIVS in "1.1) NotificationProcessor.java static 
Update getPermUpdatableOnDrop(TSentryAuthorizable authorizable)" and 
"SentryPlugin.java public Update onDropSentryPrivilege(TDropPrivilegesRequest 
request)" instead of "PermissionsUpdate.ALL_ROLES"
2) check PermissionsUpdate.ALL_PRIVS instead of PermissionsUpdate.ALL_ROLES in 
UpdateableAuthzPermissions.applyPrivilegeUpdates() in 
"pUpdate.getDelPrivileges()" processing.


Diffs (updated)
-

  
sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java
 c87d205 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
 0f3c162 
  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java
 7b7d0e1 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java
 29d2256 


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

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


Testing
---

owner privilege tests pass and add new test for external table


Thanks,

Na Li

Re: Review Request 69175: SENTRY-2433: Dropping object privileges does not include update of dropping user privileges

2018-10-29 Thread kalyan kumar kalvagadda via Review Board 

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


Fix it, then Ship it!




Fix it and ship it.


sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java
Lines 931 (patched)


As you are verifying owner privileges in this test it is good to verify the 
presence of owner privileges after the external table is created.

I know that is indirectly tested when the user_1 is allowed to drop the 
table but it is good to have a explcit check.


- kalyan kumar kalvagadda


On Oct. 25, 2018, 9:56 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69175/
> ---
> 
> (Updated Oct. 25, 2018, 9:56 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio 
> Pena.
> 
> 
> Bugs: sentry-2433
> https://issues.apache.org/jira/browse/sentry-2433
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> 1) use PermissionsUpdate.ALL_PRIVS in "1.1) NotificationProcessor.java static 
> Update getPermUpdatableOnDrop(TSentryAuthorizable authorizable)" and 
> "SentryPlugin.java public Update onDropSentryPrivilege(TDropPrivilegesRequest 
> request)" instead of "PermissionsUpdate.ALL_ROLES"
> 2) check PermissionsUpdate.ALL_PRIVS instead of PermissionsUpdate.ALL_ROLES 
> in UpdateableAuthzPermissions.applyPrivilegeUpdates() in 
> "pUpdate.getDelPrivileges()" processing.
> 
> 
> Diffs
> -
> 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java
>  c87d205 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
>  b8f5ce7 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java
>  7b7d0e1 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java
>  29d2256 
> 
> 
> Diff: https://reviews.apache.org/r/69175/diff/1/
> 
> 
> Testing
> ---
> 
> owner privilege tests pass and add new test for external table
> 
> 
> Thanks,
> 
> Na Li
> 
>

Review Request 69175: SENTRY-2433: Dropping object privileges does not include update of dropping user privileges

2018-10-25 Thread Na Li via Review Board 

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

Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio 
Pena.


Bugs: sentry-2433
https://issues.apache.org/jira/browse/sentry-2433


Repository: sentry


Description
---

1) use PermissionsUpdate.ALL_PRIVS in "1.1) NotificationProcessor.java static 
Update getPermUpdatableOnDrop(TSentryAuthorizable authorizable)" and 
"SentryPlugin.java public Update onDropSentryPrivilege(TDropPrivilegesRequest 
request)" instead of "PermissionsUpdate.ALL_ROLES"
2) check PermissionsUpdate.ALL_PRIVS instead of PermissionsUpdate.ALL_ROLES in 
UpdateableAuthzPermissions.applyPrivilegeUpdates() in 
"pUpdate.getDelPrivileges()" processing.


Diffs
-

  
sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java
 c87d205 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
 b8f5ce7 
  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java
 7b7d0e1 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java
 29d2256 


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


Testing
---

owner privilege tests pass and add new test for external table


Thanks,

Na Li