You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Arun Suresh <ar...@gmail.com> on 2014/06/16 17:59:03 UTC
Review Request 22635: Allow users to grant/revoke SELECT/INSERT to ALL
tables in a Database/Server
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22635/
-----------------------------------------------------------
Review request for sentry, Jarek Cecho, Prasad Mujumdar, and Sravya Tirukkovalur.
Repository: sentry
Description
-------
Fixes for :
- https://issues.apache.org/jira/browse/SENTRY-303
- https://issues.apache.org/jira/browse/SENTRY-285
This patch depends on patch for SENTRY-281 (https://issues.apache.org/jira/browse/SENTRY-281)
Diffs
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 122d137
sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DBWildcardPrivilege.java d599de9
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 91669d6
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 9e2c200
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerWithoutKerberos.java 9f89302
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java f0bf127
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java 46d9332
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java ff73c0a
Diff: https://reviews.apache.org/r/22635/diff/
Testing
-------
This patch passes all unit and e2e tests
Thanks,
Arun Suresh
Re: Review Request 22635: Allow users to grant/revoke SELECT/INSERT to ALL
tables in a Database/Server
Posted by Arun Suresh <ar...@gmail.com>.
> On June 17, 2014, 3:30 p.m., Sravya Tirukkovalur wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java, line 591
> > <https://reviews.apache.org/r/22635/diff/1/?file=610690#file610690line591>
> >
> > Yes, we should
Actually I was thinking we shouldn't.
Before this patch.. we were translating all actions/operations from the binding layer to our layer as ALL..
Now we just selectively handle SELECT and INSERT.. everything else I guess should be passed along as ALL, we don't need to let the user know via exceptions that we don't handle the case specially
- Arun
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22635/#review45943
-----------------------------------------------------------
On June 19, 2014, 1:04 a.m., Arun Suresh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22635/
> -----------------------------------------------------------
>
> (Updated June 19, 2014, 1:04 a.m.)
>
>
> Review request for sentry, Jarek Cecho, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Fixes for :
> - https://issues.apache.org/jira/browse/SENTRY-303
> - https://issues.apache.org/jira/browse/SENTRY-285
>
> This patch depends on patch for SENTRY-281 (https://issues.apache.org/jira/browse/SENTRY-281)
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java f29078a
> sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DBWildcardPrivilege.java d599de9
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 56fc3df
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 9e2c200
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerWithoutKerberos.java 9f89302
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java f0bf127
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 27ef9ce
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java ff73c0a
>
> Diff: https://reviews.apache.org/r/22635/diff/
>
>
> Testing
> -------
>
> This patch passes all unit and e2e tests
>
>
> Thanks,
>
> Arun Suresh
>
>
Re: Review Request 22635: Allow users to grant/revoke SELECT/INSERT to ALL
tables in a Database/Server
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22635/#review45943
-----------------------------------------------------------
Mostly looks good, will review the test cases once this is rebased on 281.
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java
<https://reviews.apache.org/r/22635/#comment81063>
Yes, we should
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java
<https://reviews.apache.org/r/22635/#comment81064>
Can clean up same as 281 review.
- Sravya Tirukkovalur
On June 16, 2014, 3:59 p.m., Arun Suresh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22635/
> -----------------------------------------------------------
>
> (Updated June 16, 2014, 3:59 p.m.)
>
>
> Review request for sentry, Jarek Cecho, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Fixes for :
> - https://issues.apache.org/jira/browse/SENTRY-303
> - https://issues.apache.org/jira/browse/SENTRY-285
>
> This patch depends on patch for SENTRY-281 (https://issues.apache.org/jira/browse/SENTRY-281)
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 122d137
> sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DBWildcardPrivilege.java d599de9
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 91669d6
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 9e2c200
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerWithoutKerberos.java 9f89302
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java f0bf127
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java 46d9332
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java ff73c0a
>
> Diff: https://reviews.apache.org/r/22635/diff/
>
>
> Testing
> -------
>
> This patch passes all unit and e2e tests
>
>
> Thanks,
>
> Arun Suresh
>
>
Re: Review Request 22635: Allow users to grant/revoke SELECT/INSERT to ALL
tables in a Database/Server
Posted by Arun Suresh <ar...@gmail.com>.
> On June 19, 2014, 1:47 a.m., Sravya Tirukkovalur wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java, line 572
> > <https://reviews.apache.org/r/22635/diff/2/?file=612817#file612817line572>
> >
> > Curious, isn't this same as toSentryAction?
So toSentryAction() transforms privilegeType.getName() to a Sentry access constant.
There might be certain PrivilegeTypes applicable to Db only.. (for eg. SHOW_DATABASE, ALTER_METADATA.. ) which might not be applicable for Tables and we might want a different mapping.. hence the new method..
- Arun
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22635/#review46167
-----------------------------------------------------------
On June 19, 2014, 1:04 a.m., Arun Suresh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22635/
> -----------------------------------------------------------
>
> (Updated June 19, 2014, 1:04 a.m.)
>
>
> Review request for sentry, Jarek Cecho, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Fixes for :
> - https://issues.apache.org/jira/browse/SENTRY-303
> - https://issues.apache.org/jira/browse/SENTRY-285
>
> This patch depends on patch for SENTRY-281 (https://issues.apache.org/jira/browse/SENTRY-281)
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java f29078a
> sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DBWildcardPrivilege.java d599de9
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 56fc3df
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 9e2c200
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerWithoutKerberos.java 9f89302
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java f0bf127
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 27ef9ce
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java ff73c0a
>
> Diff: https://reviews.apache.org/r/22635/diff/
>
>
> Testing
> -------
>
> This patch passes all unit and e2e tests
>
>
> Thanks,
>
> Arun Suresh
>
>
Re: Review Request 22635: Allow users to grant/revoke SELECT/INSERT to ALL
tables in a Database/Server
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22635/#review46167
-----------------------------------------------------------
Ship it!
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java
<https://reviews.apache.org/r/22635/#comment81428>
Curious, isn't this same as toSentryAction?
- Sravya Tirukkovalur
On June 19, 2014, 1:04 a.m., Arun Suresh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22635/
> -----------------------------------------------------------
>
> (Updated June 19, 2014, 1:04 a.m.)
>
>
> Review request for sentry, Jarek Cecho, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Fixes for :
> - https://issues.apache.org/jira/browse/SENTRY-303
> - https://issues.apache.org/jira/browse/SENTRY-285
>
> This patch depends on patch for SENTRY-281 (https://issues.apache.org/jira/browse/SENTRY-281)
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java f29078a
> sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DBWildcardPrivilege.java d599de9
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 56fc3df
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 9e2c200
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerWithoutKerberos.java 9f89302
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java f0bf127
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 27ef9ce
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java ff73c0a
>
> Diff: https://reviews.apache.org/r/22635/diff/
>
>
> Testing
> -------
>
> This patch passes all unit and e2e tests
>
>
> Thanks,
>
> Arun Suresh
>
>
Re: Review Request 22635: Allow users to grant/revoke SELECT/INSERT to ALL
tables in a Database/Server
Posted by Arun Suresh <ar...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22635/#review46170
-----------------------------------------------------------
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java
<https://reviews.apache.org/r/22635/#comment81433>
So toSentryAction() transforms privilegeType.getName() to a Sentry access constant.
There might be certain PrivilegeTypes applicable to Db only.. (for eg. SHOW_DATABASE, ALTER_METADATA.. ) which might not be applicable for Tables and we might want a different mapping.. hence the new method..
- Arun Suresh
On June 19, 2014, 1:04 a.m., Arun Suresh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22635/
> -----------------------------------------------------------
>
> (Updated June 19, 2014, 1:04 a.m.)
>
>
> Review request for sentry, Jarek Cecho, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Fixes for :
> - https://issues.apache.org/jira/browse/SENTRY-303
> - https://issues.apache.org/jira/browse/SENTRY-285
>
> This patch depends on patch for SENTRY-281 (https://issues.apache.org/jira/browse/SENTRY-281)
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java f29078a
> sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DBWildcardPrivilege.java d599de9
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 56fc3df
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 9e2c200
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerWithoutKerberos.java 9f89302
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java f0bf127
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 27ef9ce
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java ff73c0a
>
> Diff: https://reviews.apache.org/r/22635/diff/
>
>
> Testing
> -------
>
> This patch passes all unit and e2e tests
>
>
> Thanks,
>
> Arun Suresh
>
>
Re: Review Request 22635: Allow users to grant/revoke SELECT/INSERT to ALL
tables in a Database/Server
Posted by Arun Suresh <ar...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22635/
-----------------------------------------------------------
(Updated June 19, 2014, 1:04 a.m.)
Review request for sentry, Jarek Cecho, Prasad Mujumdar, and Sravya Tirukkovalur.
Changes
-------
Rebased patch with Head
Repository: sentry
Description
-------
Fixes for :
- https://issues.apache.org/jira/browse/SENTRY-303
- https://issues.apache.org/jira/browse/SENTRY-285
This patch depends on patch for SENTRY-281 (https://issues.apache.org/jira/browse/SENTRY-281)
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java f29078a
sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DBWildcardPrivilege.java d599de9
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 56fc3df
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 9e2c200
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerWithoutKerberos.java 9f89302
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java f0bf127
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 27ef9ce
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java ff73c0a
Diff: https://reviews.apache.org/r/22635/diff/
Testing
-------
This patch passes all unit and e2e tests
Thanks,
Arun Suresh