Re: Review Request 68249: SENTRY-2255: alter table set owner command can be executed only by user with proper privilege

2018-08-08 Thread kalyan kumar kalvagadda via Review Board

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


Ship it!




Ship It!

- kalyan kumar kalvagadda


On Aug. 7, 2018, 2:14 a.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68249/
> ---
> 
> (Updated Aug. 7, 2018, 2:14 a.m.)
> 
> 
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio 
> Pena.
> 
> 
> Bugs: sentry-2255
> https://issues.apache.org/jira/browse/sentry-2255
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> 1) support grant option for DDL option. This is a new feature. Many places 
> have to be changed at sentry server and client to make it happen
> 2) Add privilege map in HiveAuthzPrivilegesMap, which is commentted out as 
> hive version integrated with Sentry does not support "ALTER TABLE SET OWNER"
> 3) Add more testing cases and verify owner privilege, which is disabled
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
>  f1cbbb6 
>   
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java
>  f164b30 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java
>  9350af0 
>   
> sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java
>  ab55609 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java
>  73fcda8 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/NoAuthorizationProvider.java
>  be0830d 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java
>  a9b98f3 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  621ce89 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java
>  8a10214 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
>  4588963 
> 
> 
> Diff: https://reviews.apache.org/r/68249/diff/1/
> 
> 
> Testing
> ---
> 
> existing test cases in TestOwnerPrivileges passed
> 
> 
> Thanks,
> 
> Na Li
> 
>



Re: Review Request 68249: SENTRY-2255: alter table set owner command can be executed only by user with proper privilege

2018-08-07 Thread Sergio Pena via Review Board

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



Looks good.

- Sergio Pena


On Aug. 7, 2018, 2:14 a.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68249/
> ---
> 
> (Updated Aug. 7, 2018, 2:14 a.m.)
> 
> 
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio 
> Pena.
> 
> 
> Bugs: sentry-2255
> https://issues.apache.org/jira/browse/sentry-2255
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> 1) support grant option for DDL option. This is a new feature. Many places 
> have to be changed at sentry server and client to make it happen
> 2) Add privilege map in HiveAuthzPrivilegesMap, which is commentted out as 
> hive version integrated with Sentry does not support "ALTER TABLE SET OWNER"
> 3) Add more testing cases and verify owner privilege, which is disabled
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
>  f1cbbb6 
>   
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java
>  f164b30 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java
>  9350af0 
>   
> sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java
>  ab55609 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java
>  73fcda8 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/NoAuthorizationProvider.java
>  be0830d 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java
>  a9b98f3 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  621ce89 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java
>  8a10214 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
>  4588963 
> 
> 
> Diff: https://reviews.apache.org/r/68249/diff/1/
> 
> 
> Testing
> ---
> 
> existing test cases in TestOwnerPrivileges passed
> 
> 
> Thanks,
> 
> Na Li
> 
>



Re: Review Request 68249: SENTRY-2255: alter table set owner command can be executed only by user with proper privilege

2018-08-07 Thread kalyan kumar kalvagadda via Review Board

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



Lina,

Hive 2.3.3 which sentry integrates with, does not allow "alter table set owner" 
command. we should not commit this patch as this functionality can neither be 
tested or used.

We should either bump the hive version to one where it supports "alter table 
set owner" before commtting this patch.

- kalyan kumar kalvagadda


On Aug. 7, 2018, 2:14 a.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68249/
> ---
> 
> (Updated Aug. 7, 2018, 2:14 a.m.)
> 
> 
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio 
> Pena.
> 
> 
> Bugs: sentry-2255
> https://issues.apache.org/jira/browse/sentry-2255
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> 1) support grant option for DDL option. This is a new feature. Many places 
> have to be changed at sentry server and client to make it happen
> 2) Add privilege map in HiveAuthzPrivilegesMap, which is commentted out as 
> hive version integrated with Sentry does not support "ALTER TABLE SET OWNER"
> 3) Add more testing cases and verify owner privilege, which is disabled
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
>  f1cbbb6 
>   
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java
>  f164b30 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java
>  9350af0 
>   
> sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java
>  ab55609 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java
>  73fcda8 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/NoAuthorizationProvider.java
>  be0830d 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java
>  a9b98f3 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  621ce89 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java
>  8a10214 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
>  4588963 
> 
> 
> Diff: https://reviews.apache.org/r/68249/diff/1/
> 
> 
> Testing
> ---
> 
> existing test cases in TestOwnerPrivileges passed
> 
> 
> Thanks,
> 
> Na Li
> 
>