You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Na Li via Review Board <no...@reviews.apache.org> on 2019/01/09 17:21:37 UTC
Review Request 69702: SENTRY-2483: Implement HMS PreReadEvent support
in MetastoreAuthzBinding
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69702/
-----------------------------------------------------------
Review request for sentry, Arjun Mishra and kalyan kumar kalvagadda.
Bugs: sentry-2483
https://issues.apache.org/jira/browse/sentry-2483
Repository: sentry
Description
-------
Add READ_DATABASE and READ_TABLE events support to provide read authorization to HMS.
This is based on changed made by Sergio at https://reviews.apache.org/r/69620/, and add code to fix unstable e2e tests
Diffs
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java 328d2b5
sentry-tests/sentry-tests-hive/pom.xml 74777bb
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationTogglingConf.java e504a8a
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 8bf486e
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 7d41348
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java f8f304f
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 9fa42f2
Diff: https://reviews.apache.org/r/69702/diff/1/
Testing
-------
add new e2e tests for READ_DATABASE and READ_TABLE at HMS
Thanks,
Na Li
Re: Review Request 69702: SENTRY-2483: Implement HMS PreReadEvent
support in MetastoreAuthzBinding
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
> On Jan. 11, 2019, 4:59 a.m., Na Li wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/HiveSimpleConnectionFactory.java
> > Line 112 (original), 117 (patched)
> > <https://reviews.apache.org/r/69702/diff/3/?file=2119162#file2119162line117>
> >
> > If I don't make this change, in insecured mode, the user will be current login user. It will be hard to configure super user at HMS server to bypass authoriozation check at readiong metadata.
> >
> > The value of realm is not important as only short user name is checked.
Why would it be the current login user? If it is insecure we wou;dn't have initialized the KerberonContext. Do we want to allow insecure connection between Sentry and HMS? I don't think we should be forcing this change.
Instead of this you could remove the if (insecure) code block from the init method. That way the connection is always secure
- Arjun
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69702/#review211854
-----------------------------------------------------------
On Jan. 11, 2019, 4:55 a.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69702/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2019, 4:55 a.m.)
>
>
> Review request for sentry, Arjun Mishra and kalyan kumar kalvagadda.
>
>
> Bugs: sentry-2483
> https://issues.apache.org/jira/browse/sentry-2483
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add READ_DATABASE and READ_TABLE events support to provide read authorization to HMS.
>
> This is based on changed made by Sergio at https://reviews.apache.org/r/69620/, and add code to fix unstable e2e tests
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java 328d2b5
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/HiveSimpleConnectionFactory.java 31e58fd
> sentry-tests/sentry-tests-hive/pom.xml 74777bb
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java 47f7466
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationTogglingConf.java e504a8a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 8bf486e
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 7d41348
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java 3c28fd0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java f8f304f
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 9fa42f2
>
>
> Diff: https://reviews.apache.org/r/69702/diff/3/
>
>
> Testing
> -------
>
> add new e2e tests for READ_DATABASE and READ_TABLE at HMS
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 69702: SENTRY-2483: Implement HMS PreReadEvent
support in MetastoreAuthzBinding
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69702/#review211854
-----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/HiveSimpleConnectionFactory.java
Line 112 (original), 117 (patched)
<https://reviews.apache.org/r/69702/#comment297443>
If I don't make this change, in insecured mode, the user will be current login user. It will be hard to configure super user at HMS server to bypass authoriozation check at readiong metadata.
The value of realm is not important as only short user name is checked.
- Na Li
On Jan. 11, 2019, 4:55 a.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69702/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2019, 4:55 a.m.)
>
>
> Review request for sentry, Arjun Mishra and kalyan kumar kalvagadda.
>
>
> Bugs: sentry-2483
> https://issues.apache.org/jira/browse/sentry-2483
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add READ_DATABASE and READ_TABLE events support to provide read authorization to HMS.
>
> This is based on changed made by Sergio at https://reviews.apache.org/r/69620/, and add code to fix unstable e2e tests
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java 328d2b5
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/HiveSimpleConnectionFactory.java 31e58fd
> sentry-tests/sentry-tests-hive/pom.xml 74777bb
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java 47f7466
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationTogglingConf.java e504a8a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 8bf486e
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 7d41348
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java 3c28fd0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java f8f304f
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 9fa42f2
>
>
> Diff: https://reviews.apache.org/r/69702/diff/3/
>
>
> Testing
> -------
>
> add new e2e tests for READ_DATABASE and READ_TABLE at HMS
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 69702: SENTRY-2483: Implement HMS PreReadEvent
support in MetastoreAuthzBinding
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69702/#review211938
-----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/HiveSimpleConnectionFactory.java
Line 112 (original), 117 (patched)
<https://reviews.apache.org/r/69702/#comment297502>
1) In UserGroupInformation, if the context does not have subject, the getLoginUser()
@Public
@Evolving
public static UserGroupInformation getCurrentUser() throws IOException {
AccessControlContext context = AccessController.getContext();
Subject subject = Subject.getSubject(context);
return subject != null && !subject.getPrincipals(User.class).isEmpty() ? new UserGroupInformation(subject) : getLoginUser();
}
2) removing the if (insecure) code block from the init does not work.
3) Insecure mode is usually done only for testing. In production, it is always secure mode.
Fixing this bug is not the focus of this jira. I change this code only to get tests pass. I have created SENTRY-2486 to fix this bug correctly
- Na Li
On Jan. 11, 2019, 4:55 a.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69702/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2019, 4:55 a.m.)
>
>
> Review request for sentry, Arjun Mishra and kalyan kumar kalvagadda.
>
>
> Bugs: sentry-2483
> https://issues.apache.org/jira/browse/sentry-2483
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add READ_DATABASE and READ_TABLE events support to provide read authorization to HMS.
>
> This is based on changed made by Sergio at https://reviews.apache.org/r/69620/, and add code to fix unstable e2e tests
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java 328d2b5
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/HiveSimpleConnectionFactory.java 31e58fd
> sentry-tests/sentry-tests-hive/pom.xml 74777bb
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java 47f7466
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationTogglingConf.java e504a8a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 8bf486e
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 7d41348
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java 3c28fd0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java f8f304f
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 9fa42f2
>
>
> Diff: https://reviews.apache.org/r/69702/diff/3/
>
>
> Testing
> -------
>
> add new e2e tests for READ_DATABASE and READ_TABLE at HMS
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 69702: SENTRY-2483: Implement HMS PreReadEvent
support in MetastoreAuthzBinding
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69702/#review211887
-----------------------------------------------------------
- Arjun Mishra
On Jan. 11, 2019, 4:55 a.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69702/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2019, 4:55 a.m.)
>
>
> Review request for sentry, Arjun Mishra and kalyan kumar kalvagadda.
>
>
> Bugs: sentry-2483
> https://issues.apache.org/jira/browse/sentry-2483
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add READ_DATABASE and READ_TABLE events support to provide read authorization to HMS.
>
> This is based on changed made by Sergio at https://reviews.apache.org/r/69620/, and add code to fix unstable e2e tests
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java 328d2b5
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/HiveSimpleConnectionFactory.java 31e58fd
> sentry-tests/sentry-tests-hive/pom.xml 74777bb
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java 47f7466
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationTogglingConf.java e504a8a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 8bf486e
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 7d41348
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java 3c28fd0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java f8f304f
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 9fa42f2
>
>
> Diff: https://reviews.apache.org/r/69702/diff/3/
>
>
> Testing
> -------
>
> add new e2e tests for READ_DATABASE and READ_TABLE at HMS
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 69702: SENTRY-2483: Implement HMS PreReadEvent
support in MetastoreAuthzBinding
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69702/
-----------------------------------------------------------
(Updated Jan. 30, 2019, 5:02 a.m.)
Review request for sentry, Arjun Mishra and kalyan kumar kalvagadda.
Bugs: sentry-2483
https://issues.apache.org/jira/browse/sentry-2483
Repository: sentry
Description
-------
Add READ_DATABASE and READ_TABLE events support to provide read authorization to HMS.
This is based on changed made by Sergio at https://reviews.apache.org/r/69620/, and add code to fix unstable e2e tests
Diffs (updated)
-----
sentry-binding/sentry-binding-hive-conf/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java 9efd612
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java 328d2b5
sentry-tests/sentry-tests-hive/pom.xml 74777bb
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 8bf486e
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 7d41348
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/AbstractMetastoreTestWithStaticConfiguration.java f14cbb6
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java 3c28fd0
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java f8f304f
Diff: https://reviews.apache.org/r/69702/diff/8/
Changes: https://reviews.apache.org/r/69702/diff/7-8/
Testing
-------
add new e2e tests for READ_DATABASE and READ_TABLE at HMS
Thanks,
Na Li
Re: Review Request 69702: SENTRY-2483: Implement HMS PreReadEvent
support in MetastoreAuthzBinding
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> On Jan. 30, 2019, 12:35 a.m., kalyan kumar kalvagadda wrote:
> > sentry-binding/sentry-binding-hive-conf/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java
> > Lines 113 (patched)
> > <https://reviews.apache.org/r/69702/diff/7/?file=2123178#file2123178line113>
> >
> > Don't you think,
> > senry.metastore.read.authorization.enabled is better?
AGREE.
> On Jan. 30, 2019, 12:35 a.m., kalyan kumar kalvagadda wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java
> > Line 158 (original), 163 (patched)
> > <https://reviews.apache.org/r/69702/diff/7/?file=2123179#file2123179line163>
> >
> > Why is change needed?
user name should be case sensitive
> On Jan. 30, 2019, 12:35 a.m., kalyan kumar kalvagadda wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java
> > Lines 212 (patched)
> > <https://reviews.apache.org/r/69702/diff/7/?file=2123179#file2123179line212>
> >
> > How about authorizeDatabaseRead?
the event type is read_database. Using authorizeReadDatabase seems clearer for me
> On Jan. 30, 2019, 12:35 a.m., kalyan kumar kalvagadda wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java
> > Lines 217 (patched)
> > <https://reviews.apache.org/r/69702/diff/7/?file=2123179#file2123179line217>
> >
> > How about authorizeTableRead?
the event type is read_table, that is why authorizeReadTable is used
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69702/#review212291
-----------------------------------------------------------
On Jan. 29, 2019, 4:15 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69702/
> -----------------------------------------------------------
>
> (Updated Jan. 29, 2019, 4:15 p.m.)
>
>
> Review request for sentry, Arjun Mishra and kalyan kumar kalvagadda.
>
>
> Bugs: sentry-2483
> https://issues.apache.org/jira/browse/sentry-2483
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add READ_DATABASE and READ_TABLE events support to provide read authorization to HMS.
>
> This is based on changed made by Sergio at https://reviews.apache.org/r/69620/, and add code to fix unstable e2e tests
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive-conf/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java 9efd612
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java 328d2b5
> sentry-tests/sentry-tests-hive/pom.xml 74777bb
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 8bf486e
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 7d41348
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/AbstractMetastoreTestWithStaticConfiguration.java f14cbb6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java 3c28fd0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java f8f304f
>
>
> Diff: https://reviews.apache.org/r/69702/diff/7/
>
>
> Testing
> -------
>
> add new e2e tests for READ_DATABASE and READ_TABLE at HMS
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 69702: SENTRY-2483: Implement HMS PreReadEvent
support in MetastoreAuthzBinding
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> On Jan. 30, 2019, 12:35 a.m., kalyan kumar kalvagadda wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java
> > Line 158 (original), 162 (patched)
> > <https://reviews.apache.org/r/69702/diff/4/?file=2122038#file2122038line162>
> >
> > Why is thie removed?
User name should be case sensitive. See details in https://issues.apache.org/jira/browse/SENTRY-2432
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69702/#review212291
-----------------------------------------------------------
On Jan. 29, 2019, 4:15 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69702/
> -----------------------------------------------------------
>
> (Updated Jan. 29, 2019, 4:15 p.m.)
>
>
> Review request for sentry, Arjun Mishra and kalyan kumar kalvagadda.
>
>
> Bugs: sentry-2483
> https://issues.apache.org/jira/browse/sentry-2483
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add READ_DATABASE and READ_TABLE events support to provide read authorization to HMS.
>
> This is based on changed made by Sergio at https://reviews.apache.org/r/69620/, and add code to fix unstable e2e tests
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive-conf/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java 9efd612
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java 328d2b5
> sentry-tests/sentry-tests-hive/pom.xml 74777bb
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 8bf486e
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 7d41348
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/AbstractMetastoreTestWithStaticConfiguration.java f14cbb6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java 3c28fd0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java f8f304f
>
>
> Diff: https://reviews.apache.org/r/69702/diff/7/
>
>
> Testing
> -------
>
> add new e2e tests for READ_DATABASE and READ_TABLE at HMS
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 69702: SENTRY-2483: Implement HMS PreReadEvent
support in MetastoreAuthzBinding
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69702/#review212291
-----------------------------------------------------------
Fix it, then Ship it!
Please consider the cosmetic comments added. You can address them if you agree and commit the change.
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java
Line 158 (original), 162 (patched)
<https://reviews.apache.org/r/69702/#comment298042>
Why is thie removed?
sentry-binding/sentry-binding-hive-conf/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java
Lines 113 (patched)
<https://reviews.apache.org/r/69702/#comment298195>
Don't you think,
senry.metastore.read.authorization.enabled is better?
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java
Lines 142 (patched)
<https://reviews.apache.org/r/69702/#comment298197>
variable name, readAuthorizationEnabled would be clear,
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java
Line 158 (original), 163 (patched)
<https://reviews.apache.org/r/69702/#comment298196>
Why is change needed?
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java
Lines 212 (patched)
<https://reviews.apache.org/r/69702/#comment298198>
How about authorizeDatabaseRead?
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java
Lines 217 (patched)
<https://reviews.apache.org/r/69702/#comment298199>
How about authorizeTableRead?
- kalyan kumar kalvagadda
On Jan. 29, 2019, 4:15 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69702/
> -----------------------------------------------------------
>
> (Updated Jan. 29, 2019, 4:15 p.m.)
>
>
> Review request for sentry, Arjun Mishra and kalyan kumar kalvagadda.
>
>
> Bugs: sentry-2483
> https://issues.apache.org/jira/browse/sentry-2483
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add READ_DATABASE and READ_TABLE events support to provide read authorization to HMS.
>
> This is based on changed made by Sergio at https://reviews.apache.org/r/69620/, and add code to fix unstable e2e tests
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive-conf/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java 9efd612
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java 328d2b5
> sentry-tests/sentry-tests-hive/pom.xml 74777bb
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 8bf486e
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 7d41348
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/AbstractMetastoreTestWithStaticConfiguration.java f14cbb6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java 3c28fd0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java f8f304f
>
>
> Diff: https://reviews.apache.org/r/69702/diff/7/
>
>
> Testing
> -------
>
> add new e2e tests for READ_DATABASE and READ_TABLE at HMS
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 69702: SENTRY-2483: Implement HMS PreReadEvent
support in MetastoreAuthzBinding
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69702/
-----------------------------------------------------------
(Updated Jan. 29, 2019, 4:15 p.m.)
Review request for sentry, Arjun Mishra and kalyan kumar kalvagadda.
Bugs: sentry-2483
https://issues.apache.org/jira/browse/sentry-2483
Repository: sentry
Description
-------
Add READ_DATABASE and READ_TABLE events support to provide read authorization to HMS.
This is based on changed made by Sergio at https://reviews.apache.org/r/69620/, and add code to fix unstable e2e tests
Diffs (updated)
-----
sentry-binding/sentry-binding-hive-conf/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java 9efd612
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java 328d2b5
sentry-tests/sentry-tests-hive/pom.xml 74777bb
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 8bf486e
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 7d41348
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/AbstractMetastoreTestWithStaticConfiguration.java f14cbb6
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java 3c28fd0
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java f8f304f
Diff: https://reviews.apache.org/r/69702/diff/7/
Changes: https://reviews.apache.org/r/69702/diff/6-7/
Testing
-------
add new e2e tests for READ_DATABASE and READ_TABLE at HMS
Thanks,
Na Li
Re: Review Request 69702: SENTRY-2483: Implement HMS PreReadEvent
support in MetastoreAuthzBinding
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69702/
-----------------------------------------------------------
(Updated Jan. 28, 2019, 10:14 p.m.)
Review request for sentry, Arjun Mishra and kalyan kumar kalvagadda.
Bugs: sentry-2483
https://issues.apache.org/jira/browse/sentry-2483
Repository: sentry
Description
-------
Add READ_DATABASE and READ_TABLE events support to provide read authorization to HMS.
This is based on changed made by Sergio at https://reviews.apache.org/r/69620/, and add code to fix unstable e2e tests
Diffs (updated)
-----
sentry-binding/sentry-binding-hive-conf/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java 9efd612
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java 328d2b5
sentry-tests/sentry-tests-hive/pom.xml 74777bb
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 8bf486e
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 7d41348
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/AbstractMetastoreTestWithStaticConfiguration.java f14cbb6
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java 3c28fd0
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java f8f304f
Diff: https://reviews.apache.org/r/69702/diff/6/
Changes: https://reviews.apache.org/r/69702/diff/5-6/
Testing
-------
add new e2e tests for READ_DATABASE and READ_TABLE at HMS
Thanks,
Na Li
Re: Review Request 69702: SENTRY-2483: Implement HMS PreReadEvent
support in MetastoreAuthzBinding
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69702/
-----------------------------------------------------------
(Updated Jan. 28, 2019, 9:03 p.m.)
Review request for sentry, Arjun Mishra and kalyan kumar kalvagadda.
Bugs: sentry-2483
https://issues.apache.org/jira/browse/sentry-2483
Repository: sentry
Description
-------
Add READ_DATABASE and READ_TABLE events support to provide read authorization to HMS.
This is based on changed made by Sergio at https://reviews.apache.org/r/69620/, and add code to fix unstable e2e tests
Diffs (updated)
-----
sentry-binding/sentry-binding-hive-conf/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java 9efd612
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java 328d2b5
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/HiveSimpleConnectionFactory.java 31e58fd
sentry-tests/sentry-tests-hive/pom.xml 74777bb
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java 47f7466
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationTogglingConf.java e504a8a
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 8bf486e
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 7d41348
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java 3c28fd0
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java f8f304f
Diff: https://reviews.apache.org/r/69702/diff/5/
Changes: https://reviews.apache.org/r/69702/diff/4-5/
Testing
-------
add new e2e tests for READ_DATABASE and READ_TABLE at HMS
Thanks,
Na Li
Re: Review Request 69702: SENTRY-2483: Implement HMS PreReadEvent
support in MetastoreAuthzBinding
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69702/
-----------------------------------------------------------
(Updated Jan. 24, 2019, 4:15 p.m.)
Review request for sentry, Arjun Mishra and kalyan kumar kalvagadda.
Bugs: sentry-2483
https://issues.apache.org/jira/browse/sentry-2483
Repository: sentry
Description
-------
Add READ_DATABASE and READ_TABLE events support to provide read authorization to HMS.
This is based on changed made by Sergio at https://reviews.apache.org/r/69620/, and add code to fix unstable e2e tests
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java 328d2b5
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/HiveSimpleConnectionFactory.java 31e58fd
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java 0d62941
sentry-tests/sentry-tests-hive/pom.xml 74777bb
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java 47f7466
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationTogglingConf.java e504a8a
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 8bf486e
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 7d41348
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java 3c28fd0
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java f8f304f
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 9fa42f2
Diff: https://reviews.apache.org/r/69702/diff/4/
Changes: https://reviews.apache.org/r/69702/diff/3-4/
Testing
-------
add new e2e tests for READ_DATABASE and READ_TABLE at HMS
Thanks,
Na Li
Re: Review Request 69702: SENTRY-2483: Implement HMS PreReadEvent
support in MetastoreAuthzBinding
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69702/
-----------------------------------------------------------
(Updated Jan. 11, 2019, 4:55 a.m.)
Review request for sentry, Arjun Mishra and kalyan kumar kalvagadda.
Bugs: sentry-2483
https://issues.apache.org/jira/browse/sentry-2483
Repository: sentry
Description
-------
Add READ_DATABASE and READ_TABLE events support to provide read authorization to HMS.
This is based on changed made by Sergio at https://reviews.apache.org/r/69620/, and add code to fix unstable e2e tests
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java 328d2b5
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/HiveSimpleConnectionFactory.java 31e58fd
sentry-tests/sentry-tests-hive/pom.xml 74777bb
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java 47f7466
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationTogglingConf.java e504a8a
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 8bf486e
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 7d41348
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java 3c28fd0
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java f8f304f
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 9fa42f2
Diff: https://reviews.apache.org/r/69702/diff/3/
Changes: https://reviews.apache.org/r/69702/diff/2-3/
Testing
-------
add new e2e tests for READ_DATABASE and READ_TABLE at HMS
Thanks,
Na Li
Re: Review Request 69702: SENTRY-2483: Implement HMS PreReadEvent
support in MetastoreAuthzBinding
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69702/
-----------------------------------------------------------
(Updated Jan. 9, 2019, 10:39 p.m.)
Review request for sentry, Arjun Mishra and kalyan kumar kalvagadda.
Bugs: sentry-2483
https://issues.apache.org/jira/browse/sentry-2483
Repository: sentry
Description
-------
Add READ_DATABASE and READ_TABLE events support to provide read authorization to HMS.
This is based on changed made by Sergio at https://reviews.apache.org/r/69620/, and add code to fix unstable e2e tests
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java 328d2b5
sentry-tests/sentry-tests-hive/pom.xml 74777bb
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java 47f7466
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationTogglingConf.java e504a8a
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 8bf486e
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 7d41348
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestAuthorizingObjectStore.java 3c28fd0
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java f8f304f
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 9fa42f2
Diff: https://reviews.apache.org/r/69702/diff/2/
Changes: https://reviews.apache.org/r/69702/diff/1-2/
Testing
-------
add new e2e tests for READ_DATABASE and READ_TABLE at HMS
Thanks,
Na Li