Re: Review Request 69702: SENTRY-2483: Implement HMS PreReadEvent support in MetastoreAuthzBinding

2019-01-29 Thread Na Li via Review Board

---
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

2019-01-29 Thread Na Li via Review Board


> 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)
> > 
> >
> > 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)
> > 
> >
> > 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)
> > 
> >
> > 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)
> > 
> >
> > 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

2019-01-29 Thread Na Li via Review Board


> 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)
> > 
> >
> > 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

2019-01-29 Thread kalyan kumar kalvagadda via Review Board

---
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)


Why is thie removed?



sentry-binding/sentry-binding-hive-conf/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java
Lines 113 (patched)


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)


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)


Why is change needed?



sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java
Lines 212 (patched)


How about authorizeDatabaseRead?



sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java
Lines 217 (patched)


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

2019-01-29 Thread Na Li via Review Board

---
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

2019-01-28 Thread Na Li via Review Board

---
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

2019-01-28 Thread Na Li via Review Board

---
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

2019-01-24 Thread Na Li via Review Board

---
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

2019-01-12 Thread Na Li via Review Board

---
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)


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

2019-01-11 Thread Arjun Mishra via Review Board

---
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

2019-01-11 Thread Arjun Mishra via Review Board


> 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)
> > 
> >
> > 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

2019-01-10 Thread Na Li via Review Board

---
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)


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

2019-01-10 Thread Na Li via Review Board

---
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

2019-01-09 Thread Na Li via Review Board

---
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



Review Request 69702: SENTRY-2483: Implement HMS PreReadEvent support in MetastoreAuthzBinding

2019-01-09 Thread Na Li via Review Board

---
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