You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Sun Dapeng <da...@intel.com> on 2014/07/31 13:16:12 UTC

Review Request 24151: SENTRY-327 Support auth admin delegation via SQL construct 'with grant option'

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

Review request for sentry, Arun Suresh, Jarek Cecho, Prasad Mujumdar, and Sravya Tirukkovalur.


Repository: sentry


Description
-------

This review request contains all 8 subtasks(need apply the patch of SENTRY-339 first)
SENTRY-340 Database implement for "with grant option"
SENTRY-341 Extend Thrift API for SentryStore to support "with grant option"
SENTRY-343 Privileges query from database support for "With Grant Option"
SENTRY-370 Judgement of MSentryPrivilege implies child privileges
SENTRY-342 Grant check with grant option
SENTRY-345 Revoke check with grant option
SENTRY-349 Extend Hive Hook with Grant Option
SENTRY-377 Add Hive e2e test for grantOption


Diffs
-----

  sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/SentryHiveConstants.java 26eea91 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 27a10ee 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java 991d734 
  sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java ac0d170 
  sentry-core/sentry-core-common/pom.xml d1785b8 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PathUtils.java 962179f 
  sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DBWildcardPrivilege.java 896283c 
  sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryGrantOption.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryPrivilege.java 9e8ac4c 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SentryNoGrantOpitonException.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java f8491db 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java.orig PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 945227e 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java ff8acdc 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java.orig PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 5fd4f8f 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 3bb7285 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql f2a62d2 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql f2a62d2 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql 70f4dbb 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql 363590e 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql 5dfae03 
  sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift fdc7b9c 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryPrivilege.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 7637376 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java a4ae291 
  sentry-provider/sentry-provider-file/pom.xml b834c8e 
  sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithGrantOption.java PRE-CREATION 

Diff: https://reviews.apache.org/r/24151/diff/


Testing
-------

Unit tests in local


Thanks,

Sun Dapeng


Re: Review Request 24151: SENTRY-327 Support auth admin delegation via SQL construct 'with grant option'

Posted by Xiaomeng Huang <xi...@intel.com>.

> On July 31, 2014, 10:47 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SentryNoGrantOpitonException.java, line 20
> > <https://reviews.apache.org/r/24151/diff/1/?file=646824#file646824line20>
> >
> >     Spelling of the class: SentryNoGrantOpitonException => SentryNoGrantOptionException
> >     
> >     Also, SentryAccessDeniedException is mostly used when user does is not part of sentry admin group, but is trying to create role, grant role or grant privilege.  Although I see that it is also used else where, there is scope for some clean up. For example, SentryStore.getMSentryVersion() (Filed SENTRY-378 to clean up). But after this patch, looks like they both (accessdenied and nograntoption) mean more or less the same exception. We can follow up on this as a follow on.
> >
> 
> Sun Dapeng wrote:
>     Yes, this will be fixed.

In my opinion, SentryAccessDeniedException is meaning access data denied, and NoGrantOption is grant privilege denied.
I want to change the name of the exception to SentryGrantDeniedException.
And SentryGrantDeniedException will be parallel with SentryAccessDeniedException, both extends SentryUserException.
This is just my thoughts, if you think we can reuse the SentryAccessDeniedException to instead of grantoption exception. It's OK on my side, I also think it's simple and clearly.


> On July 31, 2014, 10:47 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java, lines 234-237
> > <https://reviews.apache.org/r/24151/diff/1/?file=646825#file646825line234>
> >
> >     URI is a child of server, so we need to check serverName equality even when uris are same.

fixed


> On July 31, 2014, 10:47 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java, line 239
> > <https://reviews.apache.org/r/24151/diff/1/?file=646825#file646825line239>
> >
> >     Minor - Feel free to ignore:
> >     In practice serverName can never be null. So may be it is better to return false even if both server names are null?

agreed. fixed


> On July 31, 2014, 10:47 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, line 468
> > <https://reviews.apache.org/r/24151/diff/1/?file=646828#file646828line468>
> >
> >     I am not exactly sure why need this? I might be missing some thing.

fixed.


> On July 31, 2014, 10:47 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, line 492
> > <https://reviews.apache.org/r/24151/diff/1/?file=646828#file646828line492>
> >
> >     would'nt the table name always __NULL__ here as we have a check earlier?

agreed and fixed. Refactor revoke with grantOption


> On July 31, 2014, 10:47 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, line 1366
> > <https://reviews.apache.org/r/24151/diff/1/?file=646828#file646828line1366>
> >
> >     Performance: getRoleNamesForGroups is building rolenames from msentryroles, and again we are building msentryrole from role name here. So there is chance for performance improvement if we create a new function which returns Set<MSentryRoles> instead?

It's a good comment. fixed. thx.


> On July 31, 2014, 10:47 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryPrivilege.java, line 56
> > <https://reviews.apache.org/r/24151/diff/1/?file=646838#file646838line56>
> >
> >     Minor: may be just use all? As we do not support select on uri?

fixed


> On July 31, 2014, 10:47 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java, line 367
> > <https://reviews.apache.org/r/24151/diff/1/?file=646839#file646839line367>
> >
> >     Nit: Change comment to "insert" ?

fixed


- Xiaomeng


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


On July 31, 2014, 11:16 a.m., Sun Dapeng wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24151/
> -----------------------------------------------------------
> 
> (Updated July 31, 2014, 11:16 a.m.)
> 
> 
> Review request for sentry, Arun Suresh, Jarek Cecho, Prasad Mujumdar, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This review request contains all 8 subtasks(need apply the patch of SENTRY-339 first)
> SENTRY-340 Database implement for "with grant option"
> SENTRY-341 Extend Thrift API for SentryStore to support "with grant option"
> SENTRY-343 Privileges query from database support for "With Grant Option"
> SENTRY-370 Judgement of MSentryPrivilege implies child privileges
> SENTRY-342 Grant check with grant option
> SENTRY-345 Revoke check with grant option
> SENTRY-349 Extend Hive Hook with Grant Option
> SENTRY-377 Add Hive e2e test for grantOption
> 
> 
> Diffs
> -----
> 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/SentryHiveConstants.java 26eea91 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 27a10ee 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java 991d734 
>   sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java ac0d170 
>   sentry-core/sentry-core-common/pom.xml d1785b8 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PathUtils.java 962179f 
>   sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DBWildcardPrivilege.java 896283c 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryGrantOption.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryPrivilege.java 9e8ac4c 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SentryNoGrantOpitonException.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java f8491db 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java.orig PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 945227e 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java ff8acdc 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java.orig PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 5fd4f8f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 3bb7285 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql 70f4dbb 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql 363590e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql 5dfae03 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift fdc7b9c 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryPrivilege.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 7637376 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java a4ae291 
>   sentry-provider/sentry-provider-file/pom.xml b834c8e 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithGrantOption.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/24151/diff/
> 
> 
> Testing
> -------
> 
> Unit tests in local
> 
> 
> Thanks,
> 
> Sun Dapeng
> 
>


Re: Review Request 24151: SENTRY-327 Support auth admin delegation via SQL construct 'with grant option'

Posted by Sun Dapeng <da...@intel.com>.

> On Aug. 1, 2014, 6:47 a.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java, line 254
> > <https://reviews.apache.org/r/24151/diff/1/?file=646839#file646839line254>
> >
> >     Not part of this patch, but might be worth to clean up some grantorPrincipal semantics
> >     
> >     - I do not think grantorPrincipal is required for createRole, we should instead pass it for alterSentryRoleGrantPrivilege and alterSentryRoleRevokePrivilege?
> >     - As we pass grantorPrincipal as the user invoking the thrift request(requestorUserName), we may keep it consistent and avoid possibility of passing different requestorUserName and grantorPrincipal by getting rid of grantorPrincipal field in the TSentryPrivilege and TSentryRole?
> >     
> >     We can do it as a follow on if you prefer.

Yes, I'm agree with you, <grantorPrincipal> shouldn't be added to TSentryPrivilege or TSentryRole, it should belong to Role_Privilege_Mapping, but SENTRY is using JDO auto generate the M-N Relation Mapping, we may need separate it to two One-to-Many Relation and add an entity, all the features about Role and Privilege Persistent may need to change, could you fired a jira after this feature?


> On Aug. 1, 2014, 6:47 a.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift, line 35
> > <https://reviews.apache.org/r/24151/diff/1/?file=646837#file646837line35>
> >
> >     Can we add a comment here on the purpose of UNSET? Like: UNSET should be used only for revoking this privilege irrespective of grant option.

Yes, Good suggestion.


> On Aug. 1, 2014, 6:47 a.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SentryNoGrantOpitonException.java, line 20
> > <https://reviews.apache.org/r/24151/diff/1/?file=646824#file646824line20>
> >
> >     Spelling of the class: SentryNoGrantOpitonException => SentryNoGrantOptionException
> >     
> >     Also, SentryAccessDeniedException is mostly used when user does is not part of sentry admin group, but is trying to create role, grant role or grant privilege.  Although I see that it is also used else where, there is scope for some clean up. For example, SentryStore.getMSentryVersion() (Filed SENTRY-378 to clean up). But after this patch, looks like they both (accessdenied and nograntoption) mean more or less the same exception. We can follow up on this as a follow on.
> >

Yes, this will be fixed.


> On Aug. 1, 2014, 6:47 a.m., Sravya Tirukkovalur wrote:
> > sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java, line 154
> > <https://reviews.apache.org/r/24151/diff/1/?file=646818#file646818line154>
> >
> >     Minor: May be add a positive test case for with grant option here?

Yes, this will be fixed.


> On Aug. 1, 2014, 6:47 a.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java.orig, line 1
> > <https://reviews.apache.org/r/24151/diff/1/?file=646826#file646826line1>
> >
> >     Looks like this file is in the patch my mistake?

Yes, this will be fixed.


> On Aug. 1, 2014, 6:47 a.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, line 1048
> > <https://reviews.apache.org/r/24151/diff/1/?file=646828#file646828line1048>
> >
> >     We may want to restrict the usage of unset? 
> >     
> >     We default grantOption in thrift and MSentryPrivilege to false. But looks like when we convert thrift privilege struct to MSentryPrivilege and vice versa, we handle defaults differently by setting them as unset/null?

The method is do converting between MSentryPrivilege and TSentryPrivilege, it will follow the original privilege.


> On Aug. 1, 2014, 6:47 a.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, line 1072
> > <https://reviews.apache.org/r/24151/diff/1/?file=646828#file646828line1072>
> >
> >     Same as above: 
> >     
> >     We may want to restrict the usage of unset? 
> >     
> >     We default grantOption in thrift and MSentryPrivilege to false. But looks like when we convert thrift privilege struct to MSentryPrivilege and vice versa, we handle defaults differently by setting them as unset/null?

Same as above: The method is do converting between MSentryPrivilege and TSentryPrivilege, it will follow the original privilege.


> On Aug. 1, 2014, 6:47 a.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java, line 484
> > <https://reviews.apache.org/r/24151/diff/1/?file=646830#file646830line484>
> >
> >     Same as above: 
> >     
> >     We may want to restrict the usage of unset? 
> >     
> >     We default grantOption in thrift and MSentryPrivilege to false. But looks like when we convert thrift privilege struct to MSentryPrivilege and vice versa, we handle defaults differently by setting them as unset/null?

Same as above: The method is do converting between MSentryPrivilege and TSentryPrivilege, it will follow the original privilege.
This will change, but there is no impact with result.


> On Aug. 1, 2014, 6:47 a.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithGrantOption.java, line 101
> > <https://reviews.apache.org/r/24151/diff/1/?file=646842#file646842line101>
> >
> >     In all of these verifyFailureHook calls, we may want to assert that the hiveoperation is set correctly in the hook?

Good suggestion. I will do it


> On Aug. 1, 2014, 6:47 a.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java.orig, line 1
> > <https://reviews.apache.org/r/24151/diff/1/?file=646829#file646829line1>
> >
> >     Looks like this file is part of the patch by mistake?

Thank you, I will fix it.


- Sun


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


On July 31, 2014, 7:16 p.m., Sun Dapeng wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24151/
> -----------------------------------------------------------
> 
> (Updated July 31, 2014, 7:16 p.m.)
> 
> 
> Review request for sentry, Arun Suresh, Jarek Cecho, Prasad Mujumdar, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This review request contains all 8 subtasks(need apply the patch of SENTRY-339 first)
> SENTRY-340 Database implement for "with grant option"
> SENTRY-341 Extend Thrift API for SentryStore to support "with grant option"
> SENTRY-343 Privileges query from database support for "With Grant Option"
> SENTRY-370 Judgement of MSentryPrivilege implies child privileges
> SENTRY-342 Grant check with grant option
> SENTRY-345 Revoke check with grant option
> SENTRY-349 Extend Hive Hook with Grant Option
> SENTRY-377 Add Hive e2e test for grantOption
> 
> 
> Diffs
> -----
> 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/SentryHiveConstants.java 26eea91 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 27a10ee 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java 991d734 
>   sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java ac0d170 
>   sentry-core/sentry-core-common/pom.xml d1785b8 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PathUtils.java 962179f 
>   sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DBWildcardPrivilege.java 896283c 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryGrantOption.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryPrivilege.java 9e8ac4c 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SentryNoGrantOpitonException.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java f8491db 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java.orig PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 945227e 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java ff8acdc 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java.orig PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 5fd4f8f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 3bb7285 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql 70f4dbb 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql 363590e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql 5dfae03 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift fdc7b9c 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryPrivilege.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 7637376 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java a4ae291 
>   sentry-provider/sentry-provider-file/pom.xml b834c8e 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithGrantOption.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/24151/diff/
> 
> 
> Testing
> -------
> 
> Unit tests in local
> 
> 
> Thanks,
> 
> Sun Dapeng
> 
>


Re: Review Request 24151: SENTRY-327 Support auth admin delegation via SQL construct 'with grant option'

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24151/#review49253
-----------------------------------------------------------


Nice work and very useful test cases! Some minor comments below.


sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java
<https://reviews.apache.org/r/24151/#comment86153>

    Minor: May be add a positive test case for with grant option here?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SentryNoGrantOpitonException.java
<https://reviews.apache.org/r/24151/#comment86158>

    Spelling of the class: SentryNoGrantOpitonException => SentryNoGrantOptionException
    
    Also, SentryAccessDeniedException is mostly used when user does is not part of sentry admin group, but is trying to create role, grant role or grant privilege.  Although I see that it is also used else where, there is scope for some clean up. For example, SentryStore.getMSentryVersion() (Filed SENTRY-378 to clean up). But after this patch, looks like they both (accessdenied and nograntoption) mean more or less the same exception. We can follow up on this as a follow on.
    



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java
<https://reviews.apache.org/r/24151/#comment86243>

    URI is a child of server, so we need to check serverName equality even when uris are same.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java
<https://reviews.apache.org/r/24151/#comment86244>

    Minor - Feel free to ignore:
    In practice serverName can never be null. So may be it is better to return false even if both server names are null?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java.orig
<https://reviews.apache.org/r/24151/#comment86161>

    Looks like this file is in the patch my mistake?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
<https://reviews.apache.org/r/24151/#comment86254>

    I am not exactly sure why need this? I might be missing some thing.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
<https://reviews.apache.org/r/24151/#comment86186>

    would'nt the table name always __NULL__ here as we have a check earlier?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
<https://reviews.apache.org/r/24151/#comment86192>

    We may want to restrict the usage of unset? 
    
    We default grantOption in thrift and MSentryPrivilege to false. But looks like when we convert thrift privilege struct to MSentryPrivilege and vice versa, we handle defaults differently by setting them as unset/null?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
<https://reviews.apache.org/r/24151/#comment86197>

    Same as above: 
    
    We may want to restrict the usage of unset? 
    
    We default grantOption in thrift and MSentryPrivilege to false. But looks like when we convert thrift privilege struct to MSentryPrivilege and vice versa, we handle defaults differently by setting them as unset/null?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
<https://reviews.apache.org/r/24151/#comment86206>

    Performance: getRoleNamesForGroups is building rolenames from msentryroles, and again we are building msentryrole from role name here. So there is chance for performance improvement if we create a new function which returns Set<MSentryRoles> instead?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java.orig
<https://reviews.apache.org/r/24151/#comment86209>

    Looks like this file is part of the patch by mistake?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java
<https://reviews.apache.org/r/24151/#comment86211>

    Same as above: 
    
    We may want to restrict the usage of unset? 
    
    We default grantOption in thrift and MSentryPrivilege to false. But looks like when we convert thrift privilege struct to MSentryPrivilege and vice versa, we handle defaults differently by setting them as unset/null?



sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift
<https://reviews.apache.org/r/24151/#comment86212>

    Can we add a comment here on the purpose of UNSET? Like: UNSET should be used only for revoking this privilege irrespective of grant option.



sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryPrivilege.java
<https://reviews.apache.org/r/24151/#comment86220>

    Minor: may be just use all? As we do not support select on uri?



sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
<https://reviews.apache.org/r/24151/#comment86229>

    Not part of this patch, but might be worth to clean up some grantorPrincipal semantics
    
    - I do not think grantorPrincipal is required for createRole, we should instead pass it for alterSentryRoleGrantPrivilege and alterSentryRoleRevokePrivilege?
    - As we pass grantorPrincipal as the user invoking the thrift request(requestorUserName), we may keep it consistent and avoid possibility of passing different requestorUserName and grantorPrincipal by getting rid of grantorPrincipal field in the TSentryPrivilege and TSentryRole?
    
    We can do it as a follow on if you prefer.



sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
<https://reviews.apache.org/r/24151/#comment86234>

    Nit: Change comment to "insert" ?



sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithGrantOption.java
<https://reviews.apache.org/r/24151/#comment86252>

    In all of these verifyFailureHook calls, we may want to assert that the hiveoperation is set correctly in the hook?


- Sravya Tirukkovalur


On July 31, 2014, 11:16 a.m., Sun Dapeng wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24151/
> -----------------------------------------------------------
> 
> (Updated July 31, 2014, 11:16 a.m.)
> 
> 
> Review request for sentry, Arun Suresh, Jarek Cecho, Prasad Mujumdar, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This review request contains all 8 subtasks(need apply the patch of SENTRY-339 first)
> SENTRY-340 Database implement for "with grant option"
> SENTRY-341 Extend Thrift API for SentryStore to support "with grant option"
> SENTRY-343 Privileges query from database support for "With Grant Option"
> SENTRY-370 Judgement of MSentryPrivilege implies child privileges
> SENTRY-342 Grant check with grant option
> SENTRY-345 Revoke check with grant option
> SENTRY-349 Extend Hive Hook with Grant Option
> SENTRY-377 Add Hive e2e test for grantOption
> 
> 
> Diffs
> -----
> 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/SentryHiveConstants.java 26eea91 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 27a10ee 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java 991d734 
>   sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java ac0d170 
>   sentry-core/sentry-core-common/pom.xml d1785b8 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PathUtils.java 962179f 
>   sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DBWildcardPrivilege.java 896283c 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryGrantOption.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryPrivilege.java 9e8ac4c 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SentryNoGrantOpitonException.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java f8491db 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java.orig PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 945227e 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java ff8acdc 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java.orig PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 5fd4f8f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 3bb7285 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql 70f4dbb 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql 363590e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql 5dfae03 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift fdc7b9c 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryPrivilege.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 7637376 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java a4ae291 
>   sentry-provider/sentry-provider-file/pom.xml b834c8e 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithGrantOption.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/24151/diff/
> 
> 
> Testing
> -------
> 
> Unit tests in local
> 
> 
> Thanks,
> 
> Sun Dapeng
> 
>


Re: Review Request 24151: SENTRY-327 Support auth admin delegation via SQL construct 'with grant option'

Posted by Sun Dapeng <da...@intel.com>.

> On Aug. 2, 2014, 2:57 a.m., Sravya Tirukkovalur wrote:
> > Thanks for the quick updates! Can you also update the patch on the jira when you get a chance? Thanks!

Okay, I will update the patch and upload it to JIRA. Thank you very much for your review.


> On Aug. 2, 2014, 2:57 a.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/StaticUserGroup.java, lines 19-59
> > <https://reviews.apache.org/r/24151/diff/2/?file=648035#file648035line19>
> >
> >     Looks like these changes have come in by mistake?

Yes, fixed it.


> On Aug. 2, 2014, 2:57 a.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithHiveServer.java, lines 42-55
> > <https://reviews.apache.org/r/24151/diff/2/?file=648033#file648033line42>
> >
> >     Looks like these changes have come in by mistake?

Yes, fixed it


> On Aug. 2, 2014, 2:57 a.m., Sravya Tirukkovalur wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryOnFailureHookContext.java, line 101
> > <https://reviews.apache.org/r/24151/diff/2/?file=648008#file648008line101>
> >
> >     Just curious, what is the motivation to add grantOption in the failure hook?

Hi Sravya, sorry for I misunderstand, I think you want me to check grantoption in FailureHook, I will revert it


- Sun


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


On Aug. 1, 2014, 5:34 p.m., Sun Dapeng wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24151/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2014, 5:34 p.m.)
> 
> 
> Review request for sentry, Arun Suresh, Jarek Cecho, Prasad Mujumdar, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This review request contains all 8 subtasks(need apply the patch of SENTRY-339 first)
> SENTRY-340 Database implement for "with grant option"
> SENTRY-341 Extend Thrift API for SentryStore to support "with grant option"
> SENTRY-343 Privileges query from database support for "With Grant Option"
> SENTRY-370 Judgement of MSentryPrivilege implies child privileges
> SENTRY-342 Grant check with grant option
> SENTRY-345 Revoke check with grant option
> SENTRY-349 Extend Hive Hook with Grant Option
> SENTRY-377 Add Hive e2e test for grantOption
> 
> 
> Diffs
> -----
> 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/SentryHiveConstants.java 26eea91 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 27a10ee 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java 991d734 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryOnFailureHookContext.java a380651 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryOnFailureHookContextImpl.java f97d7f3 
>   sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java ac0d170 
>   sentry-core/sentry-core-common/pom.xml d1785b8 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PathUtils.java 962179f 
>   sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DBWildcardPrivilege.java 896283c 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryGrantOption.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryPrivilege.java 9e8ac4c 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SentryGrantDeniedException.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java f8491db 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 945227e 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java ff8acdc 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 5fd4f8f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 3bb7285 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql 70f4dbb 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql 363590e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql 5dfae03 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift fdc7b9c 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryPrivilege.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 7637376 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java a4ae291 
>   sentry-provider/sentry-provider-file/pom.xml b834c8e 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithGrantOption.java PRE-CREATION 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithHiveServer.java 9b3c04a 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/DummySentryOnFailureHook.java 4838f76 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/StaticUserGroup.java 66f088f 
> 
> Diff: https://reviews.apache.org/r/24151/diff/
> 
> 
> Testing
> -------
> 
> Unit tests in local
> 
> 
> Thanks,
> 
> Sun Dapeng
> 
>


Re: Review Request 24151: SENTRY-327 Support auth admin delegation via SQL construct 'with grant option'

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24151/#review49367
-----------------------------------------------------------


Thanks for the quick updates! Can you also update the patch on the jira when you get a chance? Thanks!


sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SentryNoGrantOpitonException.java
<https://reviews.apache.org/r/24151/#comment86366>

    SentryGrantDeniedException sounds good, Thanks!



sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryOnFailureHookContext.java
<https://reviews.apache.org/r/24151/#comment86370>

    Just curious, what is the motivation to add grantOption in the failure hook?



sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryOnFailureHookContextImpl.java
<https://reviews.apache.org/r/24151/#comment86371>

    Just curious, what is the motivation to add grantOption in the failure hook?



sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithHiveServer.java
<https://reviews.apache.org/r/24151/#comment86372>

    Looks like these changes have come in by mistake?



sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/StaticUserGroup.java
<https://reviews.apache.org/r/24151/#comment86373>

    Looks like these changes have come in by mistake?


- Sravya Tirukkovalur


On Aug. 1, 2014, 9:34 a.m., Sun Dapeng wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24151/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2014, 9:34 a.m.)
> 
> 
> Review request for sentry, Arun Suresh, Jarek Cecho, Prasad Mujumdar, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This review request contains all 8 subtasks(need apply the patch of SENTRY-339 first)
> SENTRY-340 Database implement for "with grant option"
> SENTRY-341 Extend Thrift API for SentryStore to support "with grant option"
> SENTRY-343 Privileges query from database support for "With Grant Option"
> SENTRY-370 Judgement of MSentryPrivilege implies child privileges
> SENTRY-342 Grant check with grant option
> SENTRY-345 Revoke check with grant option
> SENTRY-349 Extend Hive Hook with Grant Option
> SENTRY-377 Add Hive e2e test for grantOption
> 
> 
> Diffs
> -----
> 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/SentryHiveConstants.java 26eea91 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 27a10ee 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java 991d734 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryOnFailureHookContext.java a380651 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryOnFailureHookContextImpl.java f97d7f3 
>   sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java ac0d170 
>   sentry-core/sentry-core-common/pom.xml d1785b8 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PathUtils.java 962179f 
>   sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DBWildcardPrivilege.java 896283c 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryGrantOption.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryPrivilege.java 9e8ac4c 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SentryGrantDeniedException.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java f8491db 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 945227e 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java ff8acdc 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 5fd4f8f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 3bb7285 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql 70f4dbb 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql 363590e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql 5dfae03 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift fdc7b9c 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryPrivilege.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 7637376 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java a4ae291 
>   sentry-provider/sentry-provider-file/pom.xml b834c8e 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithGrantOption.java PRE-CREATION 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithHiveServer.java 9b3c04a 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/DummySentryOnFailureHook.java 4838f76 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/StaticUserGroup.java 66f088f 
> 
> Diff: https://reviews.apache.org/r/24151/diff/
> 
> 
> Testing
> -------
> 
> Unit tests in local
> 
> 
> Thanks,
> 
> Sun Dapeng
> 
>


Re: Review Request 24151: SENTRY-327 Support auth admin delegation via SQL construct 'with grant option'

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24151/#review49479
-----------------------------------------------------------

Ship it!


Thanks Dapeng and Xiaomeng for the great work!

- Sravya Tirukkovalur


On Aug. 4, 2014, 6:33 a.m., Sun Dapeng wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24151/
> -----------------------------------------------------------
> 
> (Updated Aug. 4, 2014, 6:33 a.m.)
> 
> 
> Review request for sentry, Arun Suresh, Jarek Cecho, Prasad Mujumdar, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This review request contains all 8 subtasks(need apply the patch of SENTRY-339 first)
> SENTRY-340 Database implement for "with grant option"
> SENTRY-341 Extend Thrift API for SentryStore to support "with grant option"
> SENTRY-343 Privileges query from database support for "With Grant Option"
> SENTRY-370 Judgement of MSentryPrivilege implies child privileges
> SENTRY-342 Grant check with grant option
> SENTRY-345 Revoke check with grant option
> SENTRY-349 Extend Hive Hook with Grant Option
> SENTRY-377 Add Hive e2e test for grantOption
> 
> 
> Diffs
> -----
> 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/SentryHiveConstants.java 26eea91 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 27a10ee 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java 991d734 
>   sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java ac0d170 
>   sentry-core/sentry-core-common/pom.xml d1785b8 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PathUtils.java 962179f 
>   sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DBWildcardPrivilege.java 896283c 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryGrantOption.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryPrivilege.java 9e8ac4c 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SentryGrantDeniedException.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java f8491db 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 945227e 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java ff8acdc 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 5fd4f8f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 3bb7285 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql 70f4dbb 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql 363590e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql 5dfae03 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift fdc7b9c 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryPrivilege.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 7637376 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java a4ae291 
>   sentry-provider/sentry-provider-file/pom.xml b834c8e 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithGrantOption.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/24151/diff/
> 
> 
> Testing
> -------
> 
> Unit tests in local
> 
> 
> Thanks,
> 
> Sun Dapeng
> 
>


Re: Review Request 24151: SENTRY-327 Support auth admin delegation via SQL construct 'with grant option'

Posted by Sun Dapeng <da...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24151/
-----------------------------------------------------------

(Updated Aug. 4, 2014, 2:33 p.m.)


Review request for sentry, Arun Suresh, Jarek Cecho, Prasad Mujumdar, and Sravya Tirukkovalur.


Changes
-------

Updated the patch accroding Sravya's comments.


Repository: sentry


Description
-------

This review request contains all 8 subtasks(need apply the patch of SENTRY-339 first)
SENTRY-340 Database implement for "with grant option"
SENTRY-341 Extend Thrift API for SentryStore to support "with grant option"
SENTRY-343 Privileges query from database support for "With Grant Option"
SENTRY-370 Judgement of MSentryPrivilege implies child privileges
SENTRY-342 Grant check with grant option
SENTRY-345 Revoke check with grant option
SENTRY-349 Extend Hive Hook with Grant Option
SENTRY-377 Add Hive e2e test for grantOption


Diffs (updated)
-----

  sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/SentryHiveConstants.java 26eea91 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 27a10ee 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java 991d734 
  sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java ac0d170 
  sentry-core/sentry-core-common/pom.xml d1785b8 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PathUtils.java 962179f 
  sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DBWildcardPrivilege.java 896283c 
  sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryGrantOption.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryPrivilege.java 9e8ac4c 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SentryGrantDeniedException.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java f8491db 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 945227e 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java ff8acdc 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 5fd4f8f 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 3bb7285 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql f2a62d2 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql f2a62d2 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql 70f4dbb 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql 363590e 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql 5dfae03 
  sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift fdc7b9c 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryPrivilege.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 7637376 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java a4ae291 
  sentry-provider/sentry-provider-file/pom.xml b834c8e 
  sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithGrantOption.java PRE-CREATION 

Diff: https://reviews.apache.org/r/24151/diff/


Testing
-------

Unit tests in local


Thanks,

Sun Dapeng


Re: Review Request 24151: SENTRY-327 Support auth admin delegation via SQL construct 'with grant option'

Posted by Sun Dapeng <da...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24151/
-----------------------------------------------------------

(Updated Aug. 1, 2014, 5:34 p.m.)


Review request for sentry, Arun Suresh, Jarek Cecho, Prasad Mujumdar, and Sravya Tirukkovalur.


Changes
-------

Updated the patch according Sravya's comments.


Repository: sentry


Description
-------

This review request contains all 8 subtasks(need apply the patch of SENTRY-339 first)
SENTRY-340 Database implement for "with grant option"
SENTRY-341 Extend Thrift API for SentryStore to support "with grant option"
SENTRY-343 Privileges query from database support for "With Grant Option"
SENTRY-370 Judgement of MSentryPrivilege implies child privileges
SENTRY-342 Grant check with grant option
SENTRY-345 Revoke check with grant option
SENTRY-349 Extend Hive Hook with Grant Option
SENTRY-377 Add Hive e2e test for grantOption


Diffs (updated)
-----

  sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/SentryHiveConstants.java 26eea91 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 27a10ee 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java 991d734 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryOnFailureHookContext.java a380651 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryOnFailureHookContextImpl.java f97d7f3 
  sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java ac0d170 
  sentry-core/sentry-core-common/pom.xml d1785b8 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PathUtils.java 962179f 
  sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DBWildcardPrivilege.java 896283c 
  sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryGrantOption.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryPrivilege.java 9e8ac4c 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SentryGrantDeniedException.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java f8491db 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 945227e 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java ff8acdc 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 5fd4f8f 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 3bb7285 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql f2a62d2 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql f2a62d2 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql 70f4dbb 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql 363590e 
  sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql 5dfae03 
  sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift fdc7b9c 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryPrivilege.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 7637376 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java a4ae291 
  sentry-provider/sentry-provider-file/pom.xml b834c8e 
  sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithGrantOption.java PRE-CREATION 
  sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithHiveServer.java 9b3c04a 
  sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/DummySentryOnFailureHook.java 4838f76 
  sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/StaticUserGroup.java 66f088f 

Diff: https://reviews.apache.org/r/24151/diff/


Testing
-------

Unit tests in local


Thanks,

Sun Dapeng


Re: Review Request 24151: SENTRY-327 Support auth admin delegation via SQL construct 'with grant option'

Posted by Sun Dapeng <da...@intel.com>.

> On Aug. 1, 2014, 6:55 a.m., Sravya Tirukkovalur wrote:
> > Also, I ran the test suite with this patch and there are 5 test failures in TestDatabaseProvider, can you please take a look and fix the issue? Thanks!
> > testRevokeINSERTOnTable
> > testRevokeSELECTOnTable
> > testRevokeFailMultipleGrantsExist
> > testRevokeFailAnotherRoleExist
> > testGrantRevokePrivileges

Okay, thank Sravya, after we finished all the items, we will look into it.


- Sun


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


On July 31, 2014, 7:16 p.m., Sun Dapeng wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24151/
> -----------------------------------------------------------
> 
> (Updated July 31, 2014, 7:16 p.m.)
> 
> 
> Review request for sentry, Arun Suresh, Jarek Cecho, Prasad Mujumdar, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This review request contains all 8 subtasks(need apply the patch of SENTRY-339 first)
> SENTRY-340 Database implement for "with grant option"
> SENTRY-341 Extend Thrift API for SentryStore to support "with grant option"
> SENTRY-343 Privileges query from database support for "With Grant Option"
> SENTRY-370 Judgement of MSentryPrivilege implies child privileges
> SENTRY-342 Grant check with grant option
> SENTRY-345 Revoke check with grant option
> SENTRY-349 Extend Hive Hook with Grant Option
> SENTRY-377 Add Hive e2e test for grantOption
> 
> 
> Diffs
> -----
> 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/SentryHiveConstants.java 26eea91 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 27a10ee 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java 991d734 
>   sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java ac0d170 
>   sentry-core/sentry-core-common/pom.xml d1785b8 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PathUtils.java 962179f 
>   sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DBWildcardPrivilege.java 896283c 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryGrantOption.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryPrivilege.java 9e8ac4c 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SentryNoGrantOpitonException.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java f8491db 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java.orig PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 945227e 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java ff8acdc 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java.orig PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 5fd4f8f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 3bb7285 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql 70f4dbb 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql 363590e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql 5dfae03 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift fdc7b9c 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryPrivilege.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 7637376 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java a4ae291 
>   sentry-provider/sentry-provider-file/pom.xml b834c8e 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithGrantOption.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/24151/diff/
> 
> 
> Testing
> -------
> 
> Unit tests in local
> 
> 
> Thanks,
> 
> Sun Dapeng
> 
>


Re: Review Request 24151: SENTRY-327 Support auth admin delegation via SQL construct 'with grant option'

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24151/#review49298
-----------------------------------------------------------


Also, I ran the test suite with this patch and there are 5 test failures in TestDatabaseProvider, can you please take a look and fix the issue? Thanks!
testRevokeINSERTOnTable
testRevokeSELECTOnTable
testRevokeFailMultipleGrantsExist
testRevokeFailAnotherRoleExist
testGrantRevokePrivileges

- Sravya Tirukkovalur


On July 31, 2014, 11:16 a.m., Sun Dapeng wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24151/
> -----------------------------------------------------------
> 
> (Updated July 31, 2014, 11:16 a.m.)
> 
> 
> Review request for sentry, Arun Suresh, Jarek Cecho, Prasad Mujumdar, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This review request contains all 8 subtasks(need apply the patch of SENTRY-339 first)
> SENTRY-340 Database implement for "with grant option"
> SENTRY-341 Extend Thrift API for SentryStore to support "with grant option"
> SENTRY-343 Privileges query from database support for "With Grant Option"
> SENTRY-370 Judgement of MSentryPrivilege implies child privileges
> SENTRY-342 Grant check with grant option
> SENTRY-345 Revoke check with grant option
> SENTRY-349 Extend Hive Hook with Grant Option
> SENTRY-377 Add Hive e2e test for grantOption
> 
> 
> Diffs
> -----
> 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/SentryHiveConstants.java 26eea91 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 27a10ee 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java 991d734 
>   sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java ac0d170 
>   sentry-core/sentry-core-common/pom.xml d1785b8 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PathUtils.java 962179f 
>   sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DBWildcardPrivilege.java 896283c 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryGrantOption.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryPrivilege.java 9e8ac4c 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SentryNoGrantOpitonException.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java f8491db 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java.orig PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo 945227e 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java ff8acdc 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java.orig PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 5fd4f8f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 3bb7285 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql f2a62d2 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql 70f4dbb 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql 363590e 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql 5dfae03 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift fdc7b9c 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryPrivilege.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 7637376 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java a4ae291 
>   sentry-provider/sentry-provider-file/pom.xml b834c8e 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithGrantOption.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/24151/diff/
> 
> 
> Testing
> -------
> 
> Unit tests in local
> 
> 
> Thanks,
> 
> Sun Dapeng
> 
>