You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Colin Ma <ju...@intel.com> on 2014/09/21 04:10:26 UTC
Review Request 25879: SENTRY-355: Support metadata read privilege
enforcement for Metastore pluging
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25879/
-----------------------------------------------------------
Review request for sentry, Prasad Mujumdar and Sravya Tirukkovalur.
Repository: sentry
Description
-------
The Metastore plugin currently enforces Sentry policies for metadata modifications. This makes it inconsistent with Hive plugin that support privileges for both metadata read and write.
We should support the policy enforcement for metadata read as well.
Diffs
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreFilter.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 4c66ffe
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreFilter.java PRE-CREATION
Diff: https://reviews.apache.org/r/25879/diff/
Testing
-------
Thanks,
Colin Ma
Re: Review Request 25879: SENTRY-355: Support metadata read privilege
enforcement for Metastore pluging
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25879/#review55939
-----------------------------------------------------------
Ship it!
Please address Lenni's comments. Rest look fine to me.
- Prasad Mujumdar
On Oct. 8, 2014, 8 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25879/
> -----------------------------------------------------------
>
> (Updated Oct. 8, 2014, 8 a.m.)
>
>
> Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> The Metastore plugin currently enforces Sentry policies for metadata modifications. This makes it inconsistent with Hive plugin that support privileges for both metadata read and write.
> We should support the policy enforcement for metadata read as well.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStore.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 4c66ffe
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/25879/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 25879: SENTRY-355: Support metadata read privilege
enforcement for Metastore pluging
Posted by Prasad Mujumdar <pr...@cloudera.com>.
> On Oct. 8, 2014, 5 p.m., Lenni Kuff wrote:
> > Thanks for this change, lgtm + a few minor comments. Prasad, do you have anything additional?
no addional comments, looks fine to me.
- Prasad
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25879/#review55812
-----------------------------------------------------------
On Oct. 8, 2014, 8 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25879/
> -----------------------------------------------------------
>
> (Updated Oct. 8, 2014, 8 a.m.)
>
>
> Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> The Metastore plugin currently enforces Sentry policies for metadata modifications. This makes it inconsistent with Hive plugin that support privileges for both metadata read and write.
> We should support the policy enforcement for metadata read as well.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStore.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 4c66ffe
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/25879/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 25879: SENTRY-355: Support metadata read privilege
enforcement for Metastore pluging
Posted by Lenni Kuff <ls...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25879/#review55812
-----------------------------------------------------------
Ship it!
Thanks for this change, lgtm + a few minor comments. Prasad, do you have anything additional?
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStore.java
<https://reviews.apache.org/r/25879/#comment96188>
I think usernames should be treated as case-sensitive (that is, don't call "toLower()"). Consider adding a test case for username case-sensitivity.
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStore.java
<https://reviews.apache.org/r/25879/#comment96189>
userNames should be treated as case-sensitive.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java
<https://reviews.apache.org/r/25879/#comment96193>
Also add a comment here on what this "accessAllMetaUser" value is used for.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java
<https://reviews.apache.org/r/25879/#comment96195>
nit: rename "withoutAuthorize" to "withoutAccess" here and in the test method name.
- Lenni Kuff
On Oct. 8, 2014, 8 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25879/
> -----------------------------------------------------------
>
> (Updated Oct. 8, 2014, 8 a.m.)
>
>
> Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> The Metastore plugin currently enforces Sentry policies for metadata modifications. This makes it inconsistent with Hive plugin that support privileges for both metadata read and write.
> We should support the policy enforcement for metadata read as well.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStore.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 4c66ffe
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/25879/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 25879: SENTRY-355: Support metadata read privilege
enforcement for Metastore pluging
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25879/#review56323
-----------------------------------------------------------
Ship it!
Looks fine to me.
- Prasad Mujumdar
On Oct. 12, 2014, 12:42 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25879/
> -----------------------------------------------------------
>
> (Updated Oct. 12, 2014, 12:42 a.m.)
>
>
> Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> The Metastore plugin currently enforces Sentry policies for metadata modifications. This makes it inconsistent with Hive plugin that support privileges for both metadata read and write.
> We should support the policy enforcement for metadata read as well.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStore.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 4c66ffe
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/AbstractMetastoreTestWithStaticConfiguration.java 45d24f9
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetaStoreWithPigHCat.java 00d0492
>
> Diff: https://reviews.apache.org/r/25879/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 25879: SENTRY-355: Support metadata read privilege
enforcement for Metastore pluging
Posted by Colin Ma <ju...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25879/
-----------------------------------------------------------
(Updated Oct. 12, 2014, 12:42 a.m.)
Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
Repository: sentry
Description
-------
The Metastore plugin currently enforces Sentry policies for metadata modifications. This makes it inconsistent with Hive plugin that support privileges for both metadata read and write.
We should support the policy enforcement for metadata read as well.
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStore.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 4c66ffe
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/AbstractMetastoreTestWithStaticConfiguration.java 45d24f9
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetaStoreWithPigHCat.java 00d0492
Diff: https://reviews.apache.org/r/25879/diff/
Testing
-------
Thanks,
Colin Ma
Re: Review Request 25879: SENTRY-355: Support metadata read privilege
enforcement for Metastore pluging
Posted by Colin Ma <ju...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25879/
-----------------------------------------------------------
(Updated Oct. 8, 2014, 8 a.m.)
Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
Repository: sentry
Description
-------
The Metastore plugin currently enforces Sentry policies for metadata modifications. This makes it inconsistent with Hive plugin that support privileges for both metadata read and write.
We should support the policy enforcement for metadata read as well.
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStore.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 4c66ffe
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java PRE-CREATION
Diff: https://reviews.apache.org/r/25879/diff/
Testing
-------
Thanks,
Colin Ma
Re: Review Request 25879: SENTRY-355: Support metadata read privilege
enforcement for Metastore pluging
Posted by Colin Ma <ju...@intel.com>.
> On Oct. 6, 2014, 6:36 a.m., Lenni Kuff wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreFilter.java, line 53
> > <https://reviews.apache.org/r/25879/diff/1/?file=699160#file699160line53>
> >
> > Finish comment and fix grammar. Could you also add one or two more sentances on what this class does? For example:
> > "Callers will only receive objects back which they have privileges to access and all object lists (ex getAllTables()) will be filtered to exclude items the requetor does not have privileges to access."
Update the comments.
> On Oct. 6, 2014, 6:36 a.m., Lenni Kuff wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreFilter.java, line 56
> > <https://reviews.apache.org/r/25879/diff/1/?file=699160#file699160line56>
> >
> > since this is a subclass of ObjectStore consider a name such as "FilteringObjectStore" or "AuthorizingObjectStore". To me, MetastoreFilter sounds like it might be a standalone class used to filter results returned by the metastore.
Update the class name.
> On Oct. 6, 2014, 6:36 a.m., Lenni Kuff wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreFilter.java, line 102
> > <https://reviews.apache.org/r/25879/diff/1/?file=699160#file699160line102>
> >
> > Since you repeat this message in a bunch of places, consider making it a constant. Also consider re-phrasing to something like:
> > "Table does not exist or insufficient privileges to access: <dbName>.<tableName>"
> >
> > similar comment for the db error messages.
Update the error message and make part of massage as constant.
> On Oct. 6, 2014, 6:36 a.m., Lenni Kuff wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreFilter.java, line 109
> > <https://reviews.apache.org/r/25879/diff/1/?file=699160#file699160line109>
> >
> > Should this be implemented? If not, remove commented out code.
The code is removed.
> On Oct. 6, 2014, 6:36 a.m., Lenni Kuff wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreFilter.java, line 343
> > <https://reviews.apache.org/r/25879/diff/1/?file=699160#file699160line343>
> >
> > Instead of doing this initialization for every request or just cache the value and use that for future requests?
Make the hiveAuthzBinding, hiveConf and serviceUsers as static variable and will be initialized only once.
> On Oct. 6, 2014, 6:36 a.m., Lenni Kuff wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreFilter.java, line 399
> > <https://reviews.apache.org/r/25879/diff/1/?file=699160#file699160line399>
> >
> > Should this always return true?
Update this method, the configuration "sentry.metastore.service.users" is imported to define the user who has all access to the metadata.
The new test case is added for this configuration.
> On Oct. 6, 2014, 6:36 a.m., Lenni Kuff wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreFilter.java, line 123
> > <https://reviews.apache.org/r/25879/diff/1/?file=699162#file699162line123>
> >
> > It might be easier to debug test failures if you split this in to 3 separate test cases. something like:
> >
> > testFullAccess();
> > testPartialAccess();
> > testNoAccess();
Split these test cases and add new one to test the new configuration "sentry.metastore.service.users".
@Lenni, thanks for your great review.
- Colin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25879/#review55484
-----------------------------------------------------------
On Oct. 8, 2014, 8 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25879/
> -----------------------------------------------------------
>
> (Updated Oct. 8, 2014, 8 a.m.)
>
>
> Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> The Metastore plugin currently enforces Sentry policies for metadata modifications. This makes it inconsistent with Hive plugin that support privileges for both metadata read and write.
> We should support the policy enforcement for metadata read as well.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStore.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 4c66ffe
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/25879/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 25879: SENTRY-355: Support metadata read privilege
enforcement for Metastore pluging
Posted by Lenni Kuff <ls...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25879/#review55484
-----------------------------------------------------------
Thanks for the patch. Overall looks really good, left a few comments let me know if you have any questions.
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreFilter.java
<https://reviews.apache.org/r/25879/#comment95852>
Finish comment and fix grammar. Could you also add one or two more sentances on what this class does? For example:
"Callers will only receive objects back which they have privileges to access and all object lists (ex getAllTables()) will be filtered to exclude items the requetor does not have privileges to access."
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreFilter.java
<https://reviews.apache.org/r/25879/#comment95858>
since this is a subclass of ObjectStore consider a name such as "FilteringObjectStore" or "AuthorizingObjectStore". To me, MetastoreFilter sounds like it might be a standalone class used to filter results returned by the metastore.
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreFilter.java
<https://reviews.apache.org/r/25879/#comment95854>
Since you repeat this message in a bunch of places, consider making it a constant. Also consider re-phrasing to something like:
"Table does not exist or insufficient privileges to access: <dbName>.<tableName>"
similar comment for the db error messages.
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreFilter.java
<https://reviews.apache.org/r/25879/#comment95855>
Should this be implemented? If not, remove commented out code.
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreFilter.java
<https://reviews.apache.org/r/25879/#comment95859>
which user has so ...
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreFilter.java
<https://reviews.apache.org/r/25879/#comment95860>
Instead of doing this initialization for every request or just cache the value and use that for future requests?
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreFilter.java
<https://reviews.apache.org/r/25879/#comment95856>
Should this always return true?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreFilter.java
<https://reviews.apache.org/r/25879/#comment95857>
It might be easier to debug test failures if you split this in to 3 separate test cases. something like:
testFullAccess();
testPartialAccess();
testNoAccess();
- Lenni Kuff
On Oct. 6, 2014, 1:19 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25879/
> -----------------------------------------------------------
>
> (Updated Oct. 6, 2014, 1:19 a.m.)
>
>
> Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> The Metastore plugin currently enforces Sentry policies for metadata modifications. This makes it inconsistent with Hive plugin that support privileges for both metadata read and write.
> We should support the policy enforcement for metadata read as well.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreFilter.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 4c66ffe
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreFilter.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/25879/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 25879: SENTRY-355: Support metadata read privilege
enforcement for Metastore pluging
Posted by Colin Ma <ju...@intel.com>.
> On Oct. 7, 2014, 7:10 a.m., Prasad Mujumdar wrote:
> > Looks mostly fine. Thanks for adding an extensive test case.
> >
> > A couple of high level comments:
> > - With this patch, the metastore can directly handle filtering the metadata as per user's privileges. We can get rid of SentryHiveMetaStoreClient on HiveServer2.
> > - As a followup to this patch, we should look into moving the metastore write authorization check in this class. Let's create a separate ticket to track it.
> > - - Once the column level privileges are committed, we'll need to extend the filtering to columns. I have filed SENTRY-491 to track it
I'll implement these features in the new tickets.
> On Oct. 7, 2014, 7:10 a.m., Prasad Mujumdar wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreFilter.java, line 306
> > <https://reviews.apache.org/r/25879/diff/1/?file=699160#file699160line306>
> >
> > Can we move these filterShowXXX() methods from HiveAuthzBindingHook to this class ?
This'll be fixed when working on "get rid of SentryHiveMetaStoreClient on HiveServer2".
- Colin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25879/#review55631
-----------------------------------------------------------
On Oct. 8, 2014, 8 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25879/
> -----------------------------------------------------------
>
> (Updated Oct. 8, 2014, 8 a.m.)
>
>
> Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> The Metastore plugin currently enforces Sentry policies for metadata modifications. This makes it inconsistent with Hive plugin that support privileges for both metadata read and write.
> We should support the policy enforcement for metadata read as well.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStore.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 4c66ffe
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/25879/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 25879: SENTRY-355: Support metadata read privilege
enforcement for Metastore pluging
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25879/#review55631
-----------------------------------------------------------
Looks mostly fine. Thanks for adding an extensive test case.
A couple of high level comments:
- With this patch, the metastore can directly handle filtering the metadata as per user's privileges. We can get rid of SentryHiveMetaStoreClient on HiveServer2.
- As a followup to this patch, we should look into moving the metastore write authorization check in this class. Let's create a separate ticket to track it.
- - Once the column level privileges are committed, we'll need to extend the filtering to columns. I have filed SENTRY-491 to track it
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreFilter.java
<https://reviews.apache.org/r/25879/#comment96001>
Can we move these filterShowXXX() methods from HiveAuthzBindingHook to this class ?
- Prasad Mujumdar
On Oct. 6, 2014, 1:19 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25879/
> -----------------------------------------------------------
>
> (Updated Oct. 6, 2014, 1:19 a.m.)
>
>
> Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> The Metastore plugin currently enforces Sentry policies for metadata modifications. This makes it inconsistent with Hive plugin that support privileges for both metadata read and write.
> We should support the policy enforcement for metadata read as well.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreFilter.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 4c66ffe
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreFilter.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/25879/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 25879: SENTRY-355: Support metadata read privilege
enforcement for Metastore pluging
Posted by Colin Ma <ju...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25879/
-----------------------------------------------------------
(Updated Oct. 6, 2014, 1:19 a.m.)
Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
Repository: sentry
Description
-------
The Metastore plugin currently enforces Sentry policies for metadata modifications. This makes it inconsistent with Hive plugin that support privileges for both metadata read and write.
We should support the policy enforcement for metadata read as well.
Diffs
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreFilter.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 4c66ffe
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreFilter.java PRE-CREATION
Diff: https://reviews.apache.org/r/25879/diff/
Testing
-------
Thanks,
Colin Ma