You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Sravya Tirukkovalur <sr...@cloudera.com> on 2014/06/21 00:51:10 UTC
Review Request 22847: SENTRY-310: Make Hive operation to required privileges
more granular
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22847/
-----------------------------------------------------------
Review request for sentry, Arun Suresh, Jarek Cecho, Lenni Kuff, and Prasad Mujumdar.
Bugs: SENTRY-310
https://issues.apache.org/jira/browse/SENTRY-310
Repository: sentry
Description
-------
Required privileges for a given hive operation is too restrictive in some cases. This patch cleans that up. The new model is documented as a pdf attached to the ticket.
In short:
- All DDL statements on an object require ALL on that object, except the create database/table/view/partition which requires all on the parent, as we should not allow granting privileges on non existing objects.
- Cleaned up some unwanted uri privileges, now we only support all on URI.
- Fixed some more non intuitive mappings
Diffs
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 7d241d0ea7957e6b6c334c78c6bcf0934f1a36ab
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java PRE-CREATION
Diff: https://reviews.apache.org/r/22847/diff/
Testing
-------
Captured most of the Hive operations in TestOperations test class. All of them pass.
Added todos for the operations which need test cases. Now running the entire suite.
Thanks,
Sravya Tirukkovalur
Re: Review Request 22847: SENTRY-310: Make Hive operation to required
privileges more granular
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22847/#review46350
-----------------------------------------------------------
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java
<https://reviews.apache.org/r/22847/#comment81681>
This file is missing licence.
- Jarek Cecho
On June 20, 2014, 10:51 p.m., Sravya Tirukkovalur wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22847/
> -----------------------------------------------------------
>
> (Updated June 20, 2014, 10:51 p.m.)
>
>
> Review request for sentry, Arun Suresh, Jarek Cecho, Lenni Kuff, and Prasad Mujumdar.
>
>
> Bugs: SENTRY-310
> https://issues.apache.org/jira/browse/SENTRY-310
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Required privileges for a given hive operation is too restrictive in some cases. This patch cleans that up. The new model is documented as a pdf attached to the ticket.
>
> In short:
> - All DDL statements on an object require ALL on that object, except the create database/table/view/partition which requires all on the parent, as we should not allow granting privileges on non existing objects.
> - Cleaned up some unwanted uri privileges, now we only support all on URI.
> - Fixed some more non intuitive mappings
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 7d241d0ea7957e6b6c334c78c6bcf0934f1a36ab
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/22847/diff/
>
>
> Testing
> -------
>
> Captured most of the Hive operations in TestOperations test class. All of them pass.
>
> Added todos for the operations which need test cases. Now running the entire suite.
>
>
> Thanks,
>
> Sravya Tirukkovalur
>
>
Re: Review Request 22847: SENTRY-310: Make Hive operation to required
privileges more granular
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
> On June 21, 2014, 2:18 a.m., Arun Suresh wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java, line 212
> > <https://reviews.apache.org/r/22847/diff/1/?file=614430#file614430line212>
> >
> > I guess we should have a couple of negative test cases..
> >
> > For eg : here we could have some statements that should fail if table has only INSERT perms.
Agree, we should add more negative test cases. I added some with the new updated patch. Extensive negative cases testing can be added as a follow on.
- Sravya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22847/#review46356
-----------------------------------------------------------
On June 21, 2014, 7:17 a.m., Sravya Tirukkovalur wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22847/
> -----------------------------------------------------------
>
> (Updated June 21, 2014, 7:17 a.m.)
>
>
> Review request for sentry, Arun Suresh, Jarek Cecho, Lenni Kuff, and Prasad Mujumdar.
>
>
> Bugs: SENTRY-310
> https://issues.apache.org/jira/browse/SENTRY-310
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Required privileges for a given hive operation is too restrictive in some cases. This patch cleans that up. The new model is documented as a pdf attached to the ticket.
>
> In short:
> - All DDL statements on an object require ALL on that object, except the create database/table/view/partition which requires all on the parent, as we should not allow granting privileges on non existing objects.
> - Cleaned up some unwanted uri privileges, now we only support all on URI.
> - Fixed some more non intuitive mappings
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 7d241d0ea7957e6b6c334c78c6bcf0934f1a36ab
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java e725eb06fc9915b0bcc2609e428a62feea80ec43
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/22847/diff/
>
>
> Testing
> -------
>
> Captured most of the Hive operations in TestOperations test class. All of them pass.
>
> Added todos for the operations which need test cases. Now running the entire suite.
>
>
> Thanks,
>
> Sravya Tirukkovalur
>
>
Re: Review Request 22847: SENTRY-310: Make Hive operation to required
privileges more granular
Posted by Arun Suresh <ar...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22847/#review46356
-----------------------------------------------------------
Looks good Sravya !!.. Just a few nits from my side..
I guess all long as all our tests pass.. this should be fine..
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java
<https://reviews.apache.org/r/22847/#comment81691>
I guess we should have a couple of negative test cases..
For eg : here we could have some statements that should fail if table has only INSERT perms.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java
<https://reviews.apache.org/r/22847/#comment81692>
Refer to prev. comments.. possibly one negative test case here where user has INSERT on table and ALL on db
- Arun Suresh
On June 20, 2014, 10:51 p.m., Sravya Tirukkovalur wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22847/
> -----------------------------------------------------------
>
> (Updated June 20, 2014, 10:51 p.m.)
>
>
> Review request for sentry, Arun Suresh, Jarek Cecho, Lenni Kuff, and Prasad Mujumdar.
>
>
> Bugs: SENTRY-310
> https://issues.apache.org/jira/browse/SENTRY-310
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Required privileges for a given hive operation is too restrictive in some cases. This patch cleans that up. The new model is documented as a pdf attached to the ticket.
>
> In short:
> - All DDL statements on an object require ALL on that object, except the create database/table/view/partition which requires all on the parent, as we should not allow granting privileges on non existing objects.
> - Cleaned up some unwanted uri privileges, now we only support all on URI.
> - Fixed some more non intuitive mappings
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 7d241d0ea7957e6b6c334c78c6bcf0934f1a36ab
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/22847/diff/
>
>
> Testing
> -------
>
> Captured most of the Hive operations in TestOperations test class. All of them pass.
>
> Added todos for the operations which need test cases. Now running the entire suite.
>
>
> Thanks,
>
> Sravya Tirukkovalur
>
>
Re: Review Request 22847: SENTRY-310: Make Hive operation to required
privileges more granular
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
> On June 21, 2014, 7:47 a.m., Prasad Mujumdar wrote:
> > Firstly, thanks for taking the initiative to correct the privilege model and for all the work!
> > Overall looks fine. A couple of suggestions high level comments -
> > 1. The metastore plugin needs to be updated for this. Feel free to log a followup ticket and assign to me. I can build that patch on top of this change.
> > 2. The hive binding hook is capturing the partition URI for DB scope operation. See https://github.com/apache/incubator-sentry/blob/master/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java#L386 Now that add partition is table scope, we need to move that to table scope code block.
> >
> > A few more comments/questions on specific changes below.
Created this for metastore side changes as a follow on: https://issues.apache.org/jira/browse/SENTRY-311
- Sravya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22847/#review46359
-----------------------------------------------------------
On June 22, 2014, 11:14 p.m., Sravya Tirukkovalur wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22847/
> -----------------------------------------------------------
>
> (Updated June 22, 2014, 11:14 p.m.)
>
>
> Review request for sentry, Arun Suresh, Jarek Cecho, Lenni Kuff, and Prasad Mujumdar.
>
>
> Bugs: SENTRY-310
> https://issues.apache.org/jira/browse/SENTRY-310
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Required privileges for a given hive operation is too restrictive in some cases. This patch cleans that up. The new model is documented as a pdf attached to the ticket.
>
> In short:
> - All DDL statements on an object require ALL on that object, except the create database/table/view/partition which requires all on the parent, as we should not allow granting privileges on non existing objects.
> - Cleaned up some unwanted uri privileges, now we only support all on URI.
> - Fixed some more non intuitive mappings
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 6c507b83419ab5e5e2797c62dc71bfa0fdf36776
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java cedf368825a153be13d3a05d1519a581bc30082f
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 7d241d0ea7957e6b6c334c78c6bcf0934f1a36ab
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java fd969a6cb221656d2dee65a068cdce77e1efc5cd
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java e725eb06fc9915b0bcc2609e428a62feea80ec43
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/22847/diff/
>
>
> Testing
> -------
>
> Captured most of the Hive operations in TestOperations test class. All of them pass.
>
> Added todos for the operations which need test cases. Now running the entire suite.
>
>
> Thanks,
>
> Sravya Tirukkovalur
>
>
Re: Review Request 22847: SENTRY-310: Make Hive operation to required
privileges more granular
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22847/#review46359
-----------------------------------------------------------
Firstly, thanks for taking the initiative to correct the privilege model and for all the work!
Overall looks fine. A couple of suggestions high level comments -
1. The metastore plugin needs to be updated for this. Feel free to log a followup ticket and assign to me. I can build that patch on top of this change.
2. The hive binding hook is capturing the partition URI for DB scope operation. See https://github.com/apache/incubator-sentry/blob/master/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java#L386 Now that add partition is table scope, we need to move that to table scope code block.
A few more comments/questions on specific changes below.
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java
<https://reviews.apache.org/r/22847/#comment81694>
Should the URI privilege be ALL ?
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java
<https://reviews.apache.org/r/22847/#comment81696>
If this is removed, how are we handling insert ?
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java
<https://reviews.apache.org/r/22847/#comment81695>
Does the URI need to be be removed ? You could still have 'create table .. location ..' which is covered by URI. Will it be able to handle that ?
- Prasad Mujumdar
On June 21, 2014, 7:17 a.m., Sravya Tirukkovalur wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22847/
> -----------------------------------------------------------
>
> (Updated June 21, 2014, 7:17 a.m.)
>
>
> Review request for sentry, Arun Suresh, Jarek Cecho, Lenni Kuff, and Prasad Mujumdar.
>
>
> Bugs: SENTRY-310
> https://issues.apache.org/jira/browse/SENTRY-310
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Required privileges for a given hive operation is too restrictive in some cases. This patch cleans that up. The new model is documented as a pdf attached to the ticket.
>
> In short:
> - All DDL statements on an object require ALL on that object, except the create database/table/view/partition which requires all on the parent, as we should not allow granting privileges on non existing objects.
> - Cleaned up some unwanted uri privileges, now we only support all on URI.
> - Fixed some more non intuitive mappings
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 7d241d0ea7957e6b6c334c78c6bcf0934f1a36ab
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java e725eb06fc9915b0bcc2609e428a62feea80ec43
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/22847/diff/
>
>
> Testing
> -------
>
> Captured most of the Hive operations in TestOperations test class. All of them pass.
>
> Added todos for the operations which need test cases. Now running the entire suite.
>
>
> Thanks,
>
> Sravya Tirukkovalur
>
>
Re: Review Request 22847: SENTRY-310: Make Hive operation to required
privileges more granular
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22847/#review46372
-----------------------------------------------------------
Thanks for the review Prasad!
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java
<https://reviews.apache.org/r/22847/#comment81710>
Ah, thanks for catching this. Updated in the newer patch.
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java
<https://reviews.apache.org/r/22847/#comment81711>
Handled it in the newer patch.
- Sravya Tirukkovalur
On June 21, 2014, 7:17 a.m., Sravya Tirukkovalur wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22847/
> -----------------------------------------------------------
>
> (Updated June 21, 2014, 7:17 a.m.)
>
>
> Review request for sentry, Arun Suresh, Jarek Cecho, Lenni Kuff, and Prasad Mujumdar.
>
>
> Bugs: SENTRY-310
> https://issues.apache.org/jira/browse/SENTRY-310
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Required privileges for a given hive operation is too restrictive in some cases. This patch cleans that up. The new model is documented as a pdf attached to the ticket.
>
> In short:
> - All DDL statements on an object require ALL on that object, except the create database/table/view/partition which requires all on the parent, as we should not allow granting privileges on non existing objects.
> - Cleaned up some unwanted uri privileges, now we only support all on URI.
> - Fixed some more non intuitive mappings
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 7d241d0ea7957e6b6c334c78c6bcf0934f1a36ab
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java e725eb06fc9915b0bcc2609e428a62feea80ec43
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/22847/diff/
>
>
> Testing
> -------
>
> Captured most of the Hive operations in TestOperations test class. All of them pass.
>
> Added todos for the operations which need test cases. Now running the entire suite.
>
>
> Thanks,
>
> Sravya Tirukkovalur
>
>
Re: Review Request 22847: SENTRY-310: Make Hive operation to required
privileges more granular
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22847/#review46447
-----------------------------------------------------------
Ship it!
Looks fine to me.
I guess we should log a followup ticket to cleanup the exception logic for select/insert and URI privileges in the privilege model.
- Prasad Mujumdar
On June 23, 2014, 8:39 p.m., Sravya Tirukkovalur wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22847/
> -----------------------------------------------------------
>
> (Updated June 23, 2014, 8:39 p.m.)
>
>
> Review request for sentry, Arun Suresh, Jarek Cecho, Lenni Kuff, and Prasad Mujumdar.
>
>
> Bugs: SENTRY-310
> https://issues.apache.org/jira/browse/SENTRY-310
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Required privileges for a given hive operation is too restrictive in some cases. This patch cleans that up. The new model is documented as a pdf attached to the ticket.
>
> In short:
> - All DDL statements on an object require ALL on that object, except the create database/table/view/partition which requires all on the parent, as we should not allow granting privileges on non existing objects.
> - Cleaned up some unwanted uri privileges, now we only support all on URI.
> - Fixed some more non intuitive mappings
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 6c507b83419ab5e5e2797c62dc71bfa0fdf36776
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingPreExecHook.java 7859521b2c56372280d73934293d9cd419119be4
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java cedf368825a153be13d3a05d1519a581bc30082f
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 7d241d0ea7957e6b6c334c78c6bcf0934f1a36ab
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 1f9d1eccceb45a8f4d600a36e72e3a2ad4dbc5fa
> sentry-tests/sentry-tests-hive/pom.xml d66627f8e91a8dcdbfdbd1d32457495fe9d2016e
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbOperations.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java fd969a6cb221656d2dee65a068cdce77e1efc5cd
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java e725eb06fc9915b0bcc2609e428a62feea80ec43
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestCrossDbOps.java 8552cc062fc7ebf6f093ef044321b13b860aaebc
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegeAtTransform.java 1267e6bfc035371fb48346cbcd00c15c327a2c42
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java c9658abafc7ad77ed18ce5bb9b33397dccab625c
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestSandboxOps.java 0d6e0b656ea0af48869c28d7d4938586f34084e7
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUriPermissions.java 5a620ba23a74e4ae85d019681d595172b3a86540
>
> Diff: https://reviews.apache.org/r/22847/diff/
>
>
> Testing
> -------
>
> Captured most of the Hive operations in TestOperations test class. All of them pass.
>
> Added todos for the operations which need test cases. Now running the entire suite.
>
>
> Thanks,
>
> Sravya Tirukkovalur
>
>
Re: Review Request 22847: SENTRY-310: Make Hive operation to required
privileges more granular
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22847/
-----------------------------------------------------------
(Updated June 23, 2014, 8:39 p.m.)
Review request for sentry, Arun Suresh, Jarek Cecho, Lenni Kuff, and Prasad Mujumdar.
Changes
-------
Adding the test to cluster profile.
Bugs: SENTRY-310
https://issues.apache.org/jira/browse/SENTRY-310
Repository: sentry
Description
-------
Required privileges for a given hive operation is too restrictive in some cases. This patch cleans that up. The new model is documented as a pdf attached to the ticket.
In short:
- All DDL statements on an object require ALL on that object, except the create database/table/view/partition which requires all on the parent, as we should not allow granting privileges on non existing objects.
- Cleaned up some unwanted uri privileges, now we only support all on URI.
- Fixed some more non intuitive mappings
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 6c507b83419ab5e5e2797c62dc71bfa0fdf36776
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingPreExecHook.java 7859521b2c56372280d73934293d9cd419119be4
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java cedf368825a153be13d3a05d1519a581bc30082f
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 7d241d0ea7957e6b6c334c78c6bcf0934f1a36ab
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 1f9d1eccceb45a8f4d600a36e72e3a2ad4dbc5fa
sentry-tests/sentry-tests-hive/pom.xml d66627f8e91a8dcdbfdbd1d32457495fe9d2016e
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbOperations.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java fd969a6cb221656d2dee65a068cdce77e1efc5cd
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java e725eb06fc9915b0bcc2609e428a62feea80ec43
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestCrossDbOps.java 8552cc062fc7ebf6f093ef044321b13b860aaebc
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegeAtTransform.java 1267e6bfc035371fb48346cbcd00c15c327a2c42
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java c9658abafc7ad77ed18ce5bb9b33397dccab625c
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestSandboxOps.java 0d6e0b656ea0af48869c28d7d4938586f34084e7
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUriPermissions.java 5a620ba23a74e4ae85d019681d595172b3a86540
Diff: https://reviews.apache.org/r/22847/diff/
Testing
-------
Captured most of the Hive operations in TestOperations test class. All of them pass.
Added todos for the operations which need test cases. Now running the entire suite.
Thanks,
Sravya Tirukkovalur
Re: Review Request 22847: SENTRY-310: Make Hive operation to required
privileges more granular
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22847/
-----------------------------------------------------------
(Updated June 23, 2014, 7:06 p.m.)
Review request for sentry, Arun Suresh, Jarek Cecho, Lenni Kuff, and Prasad Mujumdar.
Changes
-------
Updated the patch with review comments. We now basically not do strict check of all required privileges for QUERY operation. As hive Query can mean select/insert/analyze where all of them have different required privileges.
Bugs: SENTRY-310
https://issues.apache.org/jira/browse/SENTRY-310
Repository: sentry
Description
-------
Required privileges for a given hive operation is too restrictive in some cases. This patch cleans that up. The new model is documented as a pdf attached to the ticket.
In short:
- All DDL statements on an object require ALL on that object, except the create database/table/view/partition which requires all on the parent, as we should not allow granting privileges on non existing objects.
- Cleaned up some unwanted uri privileges, now we only support all on URI.
- Fixed some more non intuitive mappings
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 6c507b83419ab5e5e2797c62dc71bfa0fdf36776
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingPreExecHook.java 7859521b2c56372280d73934293d9cd419119be4
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java cedf368825a153be13d3a05d1519a581bc30082f
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 7d241d0ea7957e6b6c334c78c6bcf0934f1a36ab
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 1f9d1eccceb45a8f4d600a36e72e3a2ad4dbc5fa
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java fd969a6cb221656d2dee65a068cdce77e1efc5cd
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java e725eb06fc9915b0bcc2609e428a62feea80ec43
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestCrossDbOps.java 8552cc062fc7ebf6f093ef044321b13b860aaebc
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegeAtTransform.java 1267e6bfc035371fb48346cbcd00c15c327a2c42
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java c9658abafc7ad77ed18ce5bb9b33397dccab625c
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestSandboxOps.java 0d6e0b656ea0af48869c28d7d4938586f34084e7
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUriPermissions.java 5a620ba23a74e4ae85d019681d595172b3a86540
Diff: https://reviews.apache.org/r/22847/diff/
Testing
-------
Captured most of the Hive operations in TestOperations test class. All of them pass.
Added todos for the operations which need test cases. Now running the entire suite.
Thanks,
Sravya Tirukkovalur
Re: Review Request 22847: SENTRY-310: Make Hive operation to required
privileges more granular
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
> On June 23, 2014, 3:11 a.m., Prasad Mujumdar wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java, lines 313-314
> > <https://reviews.apache.org/r/22847/diff/5/?file=614860#file614860line313>
> >
> > I think this is problematic. We are looking string 'insert' in the statement which could match with any token that contains this string. For example,
> > "SELECT insert_code from table1"
> > "INSERT OVERWRITE TABLE directory_table SELECT .."
> >
> >
Right, we need a more robust way of handling this. For now, reverting the privilege map for QUERY and put a condition to not have strict check for all required privileges when hiveOp == QUERY
> On June 23, 2014, 3:11 a.m., Prasad Mujumdar wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingPreExecHook.java, line 60
> > <https://reviews.apache.org/r/22847/diff/5/?file=614861#file614861line60>
> >
> > Does this need to be input hierarchy ?
There was some inconsistency, some operations were setting it to output hierarchy and some to input, which were surfaced by making the check for required privileges stricter. Made it consistent now.
> On June 23, 2014, 3:11 a.m., Prasad Mujumdar wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java, line 361
> > <https://reviews.apache.org/r/22847/diff/5/?file=614862#file614862line361>
> >
> > Is this method used anymore ?
Removed it.
> On June 23, 2014, 3:11 a.m., Prasad Mujumdar wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java, line 123
> > <https://reviews.apache.org/r/22847/diff/5/?file=614863#file614863line123>
> >
> > I can't think of a reason to keep URI privilege in there. But just to be on safer side, can we leave it in there? Given that URI is optional, it shouldn't cause a problem. If it does require any additional code changes, then it's okay.
Added it back for now with a comment.
- Sravya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22847/#review46377
-----------------------------------------------------------
On June 23, 2014, 1:54 a.m., Sravya Tirukkovalur wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22847/
> -----------------------------------------------------------
>
> (Updated June 23, 2014, 1:54 a.m.)
>
>
> Review request for sentry, Arun Suresh, Jarek Cecho, Lenni Kuff, and Prasad Mujumdar.
>
>
> Bugs: SENTRY-310
> https://issues.apache.org/jira/browse/SENTRY-310
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Required privileges for a given hive operation is too restrictive in some cases. This patch cleans that up. The new model is documented as a pdf attached to the ticket.
>
> In short:
> - All DDL statements on an object require ALL on that object, except the create database/table/view/partition which requires all on the parent, as we should not allow granting privileges on non existing objects.
> - Cleaned up some unwanted uri privileges, now we only support all on URI.
> - Fixed some more non intuitive mappings
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 6c507b83419ab5e5e2797c62dc71bfa0fdf36776
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingPreExecHook.java 7859521b2c56372280d73934293d9cd419119be4
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java cedf368825a153be13d3a05d1519a581bc30082f
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 7d241d0ea7957e6b6c334c78c6bcf0934f1a36ab
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 1f9d1eccceb45a8f4d600a36e72e3a2ad4dbc5fa
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java fd969a6cb221656d2dee65a068cdce77e1efc5cd
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java e725eb06fc9915b0bcc2609e428a62feea80ec43
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestCrossDbOps.java 8552cc062fc7ebf6f093ef044321b13b860aaebc
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegeAtTransform.java 1267e6bfc035371fb48346cbcd00c15c327a2c42
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java c9658abafc7ad77ed18ce5bb9b33397dccab625c
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestSandboxOps.java 0d6e0b656ea0af48869c28d7d4938586f34084e7
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUriPermissions.java 5a620ba23a74e4ae85d019681d595172b3a86540
>
> Diff: https://reviews.apache.org/r/22847/diff/
>
>
> Testing
> -------
>
> Captured most of the Hive operations in TestOperations test class. All of them pass.
>
> Added todos for the operations which need test cases. Now running the entire suite.
>
>
> Thanks,
>
> Sravya Tirukkovalur
>
>
Re: Review Request 22847: SENTRY-310: Make Hive operation to required
privileges more granular
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22847/#review46377
-----------------------------------------------------------
Looks mostly fine, except the one issue about handling insert. Feel free to skip other.
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
<https://reviews.apache.org/r/22847/#comment81715>
I think this is problematic. We are looking string 'insert' in the statement which could match with any token that contains this string. For example,
"SELECT insert_code from table1"
"INSERT OVERWRITE TABLE directory_table SELECT .."
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingPreExecHook.java
<https://reviews.apache.org/r/22847/#comment81716>
Does this need to be input hierarchy ?
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
<https://reviews.apache.org/r/22847/#comment81718>
Is this method used anymore ?
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java
<https://reviews.apache.org/r/22847/#comment81719>
I can't think of a reason to keep URI privilege in there. But just to be on safer side, can we leave it in there? Given that URI is optional, it shouldn't cause a problem. If it does require any additional code changes, then it's okay.
- Prasad Mujumdar
On June 23, 2014, 1:54 a.m., Sravya Tirukkovalur wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22847/
> -----------------------------------------------------------
>
> (Updated June 23, 2014, 1:54 a.m.)
>
>
> Review request for sentry, Arun Suresh, Jarek Cecho, Lenni Kuff, and Prasad Mujumdar.
>
>
> Bugs: SENTRY-310
> https://issues.apache.org/jira/browse/SENTRY-310
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Required privileges for a given hive operation is too restrictive in some cases. This patch cleans that up. The new model is documented as a pdf attached to the ticket.
>
> In short:
> - All DDL statements on an object require ALL on that object, except the create database/table/view/partition which requires all on the parent, as we should not allow granting privileges on non existing objects.
> - Cleaned up some unwanted uri privileges, now we only support all on URI.
> - Fixed some more non intuitive mappings
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 6c507b83419ab5e5e2797c62dc71bfa0fdf36776
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingPreExecHook.java 7859521b2c56372280d73934293d9cd419119be4
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java cedf368825a153be13d3a05d1519a581bc30082f
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 7d241d0ea7957e6b6c334c78c6bcf0934f1a36ab
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 1f9d1eccceb45a8f4d600a36e72e3a2ad4dbc5fa
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java fd969a6cb221656d2dee65a068cdce77e1efc5cd
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java e725eb06fc9915b0bcc2609e428a62feea80ec43
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestCrossDbOps.java 8552cc062fc7ebf6f093ef044321b13b860aaebc
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegeAtTransform.java 1267e6bfc035371fb48346cbcd00c15c327a2c42
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java c9658abafc7ad77ed18ce5bb9b33397dccab625c
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestSandboxOps.java 0d6e0b656ea0af48869c28d7d4938586f34084e7
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUriPermissions.java 5a620ba23a74e4ae85d019681d595172b3a86540
>
> Diff: https://reviews.apache.org/r/22847/diff/
>
>
> Testing
> -------
>
> Captured most of the Hive operations in TestOperations test class. All of them pass.
>
> Added todos for the operations which need test cases. Now running the entire suite.
>
>
> Thanks,
>
> Sravya Tirukkovalur
>
>
Re: Review Request 22847: SENTRY-310: Make Hive operation to required
privileges more granular
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22847/
-----------------------------------------------------------
(Updated June 23, 2014, 1:54 a.m.)
Review request for sentry, Arun Suresh, Jarek Cecho, Lenni Kuff, and Prasad Mujumdar.
Bugs: SENTRY-310
https://issues.apache.org/jira/browse/SENTRY-310
Repository: sentry
Description
-------
Required privileges for a given hive operation is too restrictive in some cases. This patch cleans that up. The new model is documented as a pdf attached to the ticket.
In short:
- All DDL statements on an object require ALL on that object, except the create database/table/view/partition which requires all on the parent, as we should not allow granting privileges on non existing objects.
- Cleaned up some unwanted uri privileges, now we only support all on URI.
- Fixed some more non intuitive mappings
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 6c507b83419ab5e5e2797c62dc71bfa0fdf36776
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingPreExecHook.java 7859521b2c56372280d73934293d9cd419119be4
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java cedf368825a153be13d3a05d1519a581bc30082f
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 7d241d0ea7957e6b6c334c78c6bcf0934f1a36ab
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 1f9d1eccceb45a8f4d600a36e72e3a2ad4dbc5fa
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java fd969a6cb221656d2dee65a068cdce77e1efc5cd
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java e725eb06fc9915b0bcc2609e428a62feea80ec43
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestCrossDbOps.java 8552cc062fc7ebf6f093ef044321b13b860aaebc
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegeAtTransform.java 1267e6bfc035371fb48346cbcd00c15c327a2c42
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java c9658abafc7ad77ed18ce5bb9b33397dccab625c
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestSandboxOps.java 0d6e0b656ea0af48869c28d7d4938586f34084e7
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUriPermissions.java 5a620ba23a74e4ae85d019681d595172b3a86540
Diff: https://reviews.apache.org/r/22847/diff/
Testing
-------
Captured most of the Hive operations in TestOperations test class. All of them pass.
Added todos for the operations which need test cases. Now running the entire suite.
Thanks,
Sravya Tirukkovalur
Re: Review Request 22847: SENTRY-310: Make Hive operation to required
privileges more granular
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
> On June 23, 2014, 12:47 a.m., Prasad Mujumdar wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java, lines 49-68
> > <https://reviews.apache.org/r/22847/diff/4/?file=614850#file614850line49>
> >
> > If we just keep the SELECT and INSERT together (and level the URI privilege in there), then I guess we don't need all these changes. The proposed approach requires introspecting more into compiler structures to figure out select vs insert vs insert into dir etc
> >
Yes, but in which case we cannot enforce strict AND for these required privileges. As some of them will be optional in some cases.
- Sravya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22847/#review46375
-----------------------------------------------------------
On June 23, 2014, 12:03 a.m., Sravya Tirukkovalur wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22847/
> -----------------------------------------------------------
>
> (Updated June 23, 2014, 12:03 a.m.)
>
>
> Review request for sentry, Arun Suresh, Jarek Cecho, Lenni Kuff, and Prasad Mujumdar.
>
>
> Bugs: SENTRY-310
> https://issues.apache.org/jira/browse/SENTRY-310
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Required privileges for a given hive operation is too restrictive in some cases. This patch cleans that up. The new model is documented as a pdf attached to the ticket.
>
> In short:
> - All DDL statements on an object require ALL on that object, except the create database/table/view/partition which requires all on the parent, as we should not allow granting privileges on non existing objects.
> - Cleaned up some unwanted uri privileges, now we only support all on URI.
> - Fixed some more non intuitive mappings
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 6c507b83419ab5e5e2797c62dc71bfa0fdf36776
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java cedf368825a153be13d3a05d1519a581bc30082f
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 7d241d0ea7957e6b6c334c78c6bcf0934f1a36ab
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 1f9d1eccceb45a8f4d600a36e72e3a2ad4dbc5fa
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java fd969a6cb221656d2dee65a068cdce77e1efc5cd
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java e725eb06fc9915b0bcc2609e428a62feea80ec43
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestCrossDbOps.java 8552cc062fc7ebf6f093ef044321b13b860aaebc
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegeAtTransform.java 1267e6bfc035371fb48346cbcd00c15c327a2c42
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java c9658abafc7ad77ed18ce5bb9b33397dccab625c
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestSandboxOps.java 0d6e0b656ea0af48869c28d7d4938586f34084e7
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUriPermissions.java 5a620ba23a74e4ae85d019681d595172b3a86540
>
> Diff: https://reviews.apache.org/r/22847/diff/
>
>
> Testing
> -------
>
> Captured most of the Hive operations in TestOperations test class. All of them pass.
>
> Added todos for the operations which need test cases. Now running the entire suite.
>
>
> Thanks,
>
> Sravya Tirukkovalur
>
>
Re: Review Request 22847: SENTRY-310: Make Hive operation to required
privileges more granular
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22847/#review46375
-----------------------------------------------------------
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java
<https://reviews.apache.org/r/22847/#comment81712>
If we just keep the SELECT and INSERT together (and level the URI privilege in there), then I guess we don't need all these changes. The proposed approach requires introspecting more into compiler structures to figure out select vs insert vs insert into dir etc
- Prasad Mujumdar
On June 23, 2014, 12:03 a.m., Sravya Tirukkovalur wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22847/
> -----------------------------------------------------------
>
> (Updated June 23, 2014, 12:03 a.m.)
>
>
> Review request for sentry, Arun Suresh, Jarek Cecho, Lenni Kuff, and Prasad Mujumdar.
>
>
> Bugs: SENTRY-310
> https://issues.apache.org/jira/browse/SENTRY-310
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Required privileges for a given hive operation is too restrictive in some cases. This patch cleans that up. The new model is documented as a pdf attached to the ticket.
>
> In short:
> - All DDL statements on an object require ALL on that object, except the create database/table/view/partition which requires all on the parent, as we should not allow granting privileges on non existing objects.
> - Cleaned up some unwanted uri privileges, now we only support all on URI.
> - Fixed some more non intuitive mappings
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 6c507b83419ab5e5e2797c62dc71bfa0fdf36776
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java cedf368825a153be13d3a05d1519a581bc30082f
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 7d241d0ea7957e6b6c334c78c6bcf0934f1a36ab
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 1f9d1eccceb45a8f4d600a36e72e3a2ad4dbc5fa
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java fd969a6cb221656d2dee65a068cdce77e1efc5cd
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java e725eb06fc9915b0bcc2609e428a62feea80ec43
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestCrossDbOps.java 8552cc062fc7ebf6f093ef044321b13b860aaebc
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegeAtTransform.java 1267e6bfc035371fb48346cbcd00c15c327a2c42
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java c9658abafc7ad77ed18ce5bb9b33397dccab625c
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestSandboxOps.java 0d6e0b656ea0af48869c28d7d4938586f34084e7
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUriPermissions.java 5a620ba23a74e4ae85d019681d595172b3a86540
>
> Diff: https://reviews.apache.org/r/22847/diff/
>
>
> Testing
> -------
>
> Captured most of the Hive operations in TestOperations test class. All of them pass.
>
> Added todos for the operations which need test cases. Now running the entire suite.
>
>
> Thanks,
>
> Sravya Tirukkovalur
>
>
Re: Review Request 22847: SENTRY-310: Make Hive operation to required
privileges more granular
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22847/
-----------------------------------------------------------
(Updated June 23, 2014, 12:03 a.m.)
Review request for sentry, Arun Suresh, Jarek Cecho, Lenni Kuff, and Prasad Mujumdar.
Bugs: SENTRY-310
https://issues.apache.org/jira/browse/SENTRY-310
Repository: sentry
Description
-------
Required privileges for a given hive operation is too restrictive in some cases. This patch cleans that up. The new model is documented as a pdf attached to the ticket.
In short:
- All DDL statements on an object require ALL on that object, except the create database/table/view/partition which requires all on the parent, as we should not allow granting privileges on non existing objects.
- Cleaned up some unwanted uri privileges, now we only support all on URI.
- Fixed some more non intuitive mappings
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 6c507b83419ab5e5e2797c62dc71bfa0fdf36776
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java cedf368825a153be13d3a05d1519a581bc30082f
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 7d241d0ea7957e6b6c334c78c6bcf0934f1a36ab
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 1f9d1eccceb45a8f4d600a36e72e3a2ad4dbc5fa
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java fd969a6cb221656d2dee65a068cdce77e1efc5cd
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java e725eb06fc9915b0bcc2609e428a62feea80ec43
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestCrossDbOps.java 8552cc062fc7ebf6f093ef044321b13b860aaebc
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegeAtTransform.java 1267e6bfc035371fb48346cbcd00c15c327a2c42
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java c9658abafc7ad77ed18ce5bb9b33397dccab625c
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestSandboxOps.java 0d6e0b656ea0af48869c28d7d4938586f34084e7
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUriPermissions.java 5a620ba23a74e4ae85d019681d595172b3a86540
Diff: https://reviews.apache.org/r/22847/diff/
Testing
-------
Captured most of the Hive operations in TestOperations test class. All of them pass.
Added todos for the operations which need test cases. Now running the entire suite.
Thanks,
Sravya Tirukkovalur
Re: Review Request 22847: SENTRY-310: Make Hive operation to required
privileges more granular
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22847/
-----------------------------------------------------------
(Updated June 22, 2014, 11:14 p.m.)
Review request for sentry, Arun Suresh, Jarek Cecho, Lenni Kuff, and Prasad Mujumdar.
Changes
-------
Updated based on Prasad's review feedback.
- As part of it, cleaned up the way we verify privileges based on input/output hierarchies. Which can lead to semantic check success if the input/output privileges are not built properly with sufficient information to do a auth check.
- Now, the required privilege list for each operation is a strict AND (earlier it was OR, it was skipped if the input/output privilege doesn't have it) with the only exception of URIs.
- Added more test cases.
Bugs: SENTRY-310
https://issues.apache.org/jira/browse/SENTRY-310
Repository: sentry
Description
-------
Required privileges for a given hive operation is too restrictive in some cases. This patch cleans that up. The new model is documented as a pdf attached to the ticket.
In short:
- All DDL statements on an object require ALL on that object, except the create database/table/view/partition which requires all on the parent, as we should not allow granting privileges on non existing objects.
- Cleaned up some unwanted uri privileges, now we only support all on URI.
- Fixed some more non intuitive mappings
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 6c507b83419ab5e5e2797c62dc71bfa0fdf36776
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java cedf368825a153be13d3a05d1519a581bc30082f
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 7d241d0ea7957e6b6c334c78c6bcf0934f1a36ab
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java fd969a6cb221656d2dee65a068cdce77e1efc5cd
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java e725eb06fc9915b0bcc2609e428a62feea80ec43
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java PRE-CREATION
Diff: https://reviews.apache.org/r/22847/diff/
Testing
-------
Captured most of the Hive operations in TestOperations test class. All of them pass.
Added todos for the operations which need test cases. Now running the entire suite.
Thanks,
Sravya Tirukkovalur
Re: Review Request 22847: SENTRY-310: Make Hive operation to required
privileges more granular
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22847/
-----------------------------------------------------------
(Updated June 21, 2014, 7:17 a.m.)
Review request for sentry, Arun Suresh, Jarek Cecho, Lenni Kuff, and Prasad Mujumdar.
Changes
-------
Addressed review comments
Bugs: SENTRY-310
https://issues.apache.org/jira/browse/SENTRY-310
Repository: sentry
Description
-------
Required privileges for a given hive operation is too restrictive in some cases. This patch cleans that up. The new model is documented as a pdf attached to the ticket.
In short:
- All DDL statements on an object require ALL on that object, except the create database/table/view/partition which requires all on the parent, as we should not allow granting privileges on non existing objects.
- Cleaned up some unwanted uri privileges, now we only support all on URI.
- Fixed some more non intuitive mappings
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 7d241d0ea7957e6b6c334c78c6bcf0934f1a36ab
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java e725eb06fc9915b0bcc2609e428a62feea80ec43
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java PRE-CREATION
Diff: https://reviews.apache.org/r/22847/diff/
Testing
-------
Captured most of the Hive operations in TestOperations test class. All of them pass.
Added todos for the operations which need test cases. Now running the entire suite.
Thanks,
Sravya Tirukkovalur