Review Request 50058: SENTRY-1359 Implement SHOW ROLE GRANT USER user_name in V2

2016-07-14 Thread Ke Jia 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50058/
---

Review request for sentry, Colin Ma and Dapeng Sun.


Bugs: SENTRY-1359
https://issues.apache.org/jira/browse/SENTRY-1359


Repository: sentry


Description
---

Implement SHOW ROLE GRANT USER user_name in V2


Diffs
-

  
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/authorizer/DefaultSentryAccessController.java
 0d22cae 
  
sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithGrantOption.java
 284f54c 

Diff: https://reviews.apache.org/r/50058/diff/


Testing
---

test done


Thanks,

Ke Jia

Review Request 50057: SENTRY-1405 Add test for "show grant role on all " command in V2.

2016-07-14 Thread Ke Jia 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50057/
---

Review request for sentry, Colin Ma and Dapeng Sun.


Bugs: SENTRY-1405
https://issues.apache.org/jira/browse/SENTRY-1405


Repository: sentry


Description
---

Add test for "show grant role on all " command in V2


Diffs
-

  
sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
 8cfd0d0 

Diff: https://reviews.apache.org/r/50057/diff/


Testing
---

test done


Thanks,

Ke Jia

Re: Review Request 48055: SENTRY-1209: Sentry does not block Hive's cross-schema table renames

2016-07-14 Thread Sravya Tirukkovalur 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48055/#review142323
---


Ship it!




Thanks for the changes Colin! Do we have coverage for rename db1.tb1 to db1.tb2?

- Sravya Tirukkovalur


On July 15, 2016, 1:22 a.m., Colin Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48055/
> ---
> 
> (Updated July 15, 2016, 1:22 a.m.)
> 
> 
> Review request for sentry and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> User Pete 
> has read-write access to schema A
> has read-only access to schema B
> 
> User Pete nevertheless was able to rename/move Hive table 
> from schema A to schema B (where he has read-only access):
> 
> {quote}
> use A;
> alter table table_a rename to B.table_a;
> {quote}
> 
> Hive allows to use rename table syntax to move tables across schemas, not 
> just rename.
> 
> Sentry does not check security boundaries in this case.
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHookBase.java
>  dd16960 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
>  7242fde 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java
>  6c9f223 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java
>  767bcbe 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart1.java
>  a13aef5 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart2.java
>  8eb2851 
> 
> Diff: https://reviews.apache.org/r/48055/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Colin Ma
> 
>

Meeting minutes - July 2016

2016-07-14 Thread Sravya Tirukkovalur 
Hi all,

Thanks all for joining the call today! Here are the notes:

1. Discussed merging of Sentry-1205
Follow on work on refactoring module dependencies:
- authorization-provider and db-provider (Colin to file a jira)
- SENTRY-1395: Move the HDFS code which lives inside the sentry daemon into
sentry-provider
SENTRY-1321:
- Moving HiveAuthzConf to sentry-binding-hive-conf, so that
sentry-provider-db does not have to depend on sentry-binding-hive-common
which creates a circular dependency
- Moving messaging/json to sentry-binding-hive-follower to avoid circular
dependency between sentry-hive-binding and sentry-provider-db
SENTRY-1403: Move SentryHDFSServiceClient code from hdfs-common into
hdfs-service

2. Performance test: SENTRY-1347

3. SENTRY-1209: rename table privilege fix

4. Hive binding V2:
We should setup build for V2. Probably precommit not required until we
default to V2.
Might be better to update
https://cwiki.apache.org/confluence/display/SENTRY/%5BWIP%5DSentry+integration+with+Hive+Authorization+V2
with pending/planned work.

5. We briefly discussed about deprecating policy files and Hive V1 in the
next minor release. And possibly making a major release where we can clean
up deprecated code.

Regards,

Re: Review Request 48055: SENTRY-1209: Sentry does not block Hive's cross-schema table renames

2016-07-14 Thread Colin Ma 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48055/
---

(Updated July 15, 2016, 1:22 a.m.)


Review request for sentry and Sravya Tirukkovalur.


Repository: sentry


Description
---

User Pete 
has read-write access to schema A
has read-only access to schema B

User Pete nevertheless was able to rename/move Hive table 
from schema A to schema B (where he has read-only access):

{quote}
use A;
alter table table_a rename to B.table_a;
{quote}

Hive allows to use rename table syntax to move tables across schemas, not just 
rename.

Sentry does not check security boundaries in this case.


Diffs (updated)
-

  
sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHookBase.java
 dd16960 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
 7242fde 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java
 6c9f223 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java
 767bcbe 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart1.java
 a13aef5 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart2.java
 8eb2851 

Diff: https://reviews.apache.org/r/48055/diff/


Testing
---


Thanks,

Colin Ma

Re: Review Request 49996: SENTRY-1399: Integrate Fencer with SentryStore

2016-07-14 Thread Sravya Tirukkovalur 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49996/#review142306
---



Thanks for the patch Colin! Left minor comments


sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
 (line 76)


Not related to this patch. But, seems like get(String) can be made private 
in Activators?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java
 (line 250)


Annotate with @VisibleForTesting as seems like this is test only thing?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activator.java
 (line 121)


When should Activator.isActive() versus Activator.verifyActive() be used?

Seems like verifyActive() is the ultimate ground truth, is there a risk in 
usage of isActive around failover?



sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestFencer.java
 (line 52)


Nit: 
Does it make sense to do 
this.act = Activators.INSTANCE.createActivators(conf)

rather than the following in multiple places:

this.act = new Activator()
Activators.INSTANCE.put(act)

Also, shall we restrict the use of Activators purely for testing purposes?



sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestFencer.java
 (lines 101 - 102)


close in finally{} ?



sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestFencer.java
 (lines 130 - 133)


Nit: inside finally?


- Sravya Tirukkovalur


On July 13, 2016, 6:28 p.m., Colin McCabe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49996/
> ---
> 
> (Updated July 13, 2016, 6:28 p.m.)
> 
> 
> Review request for sentry, Hao Hao and Sravya Tirukkovalur.
> 
> 
> Bugs: SENTRY-1399
> https://issues.apache.org/jira/browse/SENTRY-1399
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1399: Integrate Fencer with SentryStore
> 
> 
> Diffs
> -
> 
>   
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryStandbyException.java
>  73c7e4e 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
>  e960dcd 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java
>  c003965 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java
>  14cdde3 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  6e367e5 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activator.java
>  0b7ddf5 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestFencer.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  6e00505 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStoreImportExport.java
>  fc39658 
> 
> Diff: https://reviews.apache.org/r/49996/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Colin McCabe
> 
>

Re: Review Request 49996: SENTRY-1399: Integrate Fencer with SentryStore

2016-07-14 Thread Hao Hao 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49996/#review142297
---


Fix it, then Ship it!




Thanks Colin! +1 with minor comments.


sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java
 (line 90)


Do we also need verify active for the following APIs? e.g. 
alterRoleGrantPrivilege, renamePrivilege, etc.?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java
 (line 233)


It looks like this API is to verify by using select query. Not quite match 
the comments here?


- Hao Hao


On July 13, 2016, 6:28 p.m., Colin McCabe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49996/
> ---
> 
> (Updated July 13, 2016, 6:28 p.m.)
> 
> 
> Review request for sentry, Hao Hao and Sravya Tirukkovalur.
> 
> 
> Bugs: SENTRY-1399
> https://issues.apache.org/jira/browse/SENTRY-1399
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1399: Integrate Fencer with SentryStore
> 
> 
> Diffs
> -
> 
>   
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryStandbyException.java
>  73c7e4e 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
>  e960dcd 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java
>  c003965 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java
>  14cdde3 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  6e367e5 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activator.java
>  0b7ddf5 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestFencer.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  6e00505 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStoreImportExport.java
>  fc39658 
> 
> Diff: https://reviews.apache.org/r/49996/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Colin McCabe
> 
>

Review Request 50055: SENTRY-1404 Use the new INodeAttributesProvider API in sentry-hdfs

2016-07-14 Thread Colin McCabe 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50055/
---

Review request for sentry, Anne Yu, Hao Hao, and Sravya Tirukkovalur.


Bugs: SENTRY-1404
https://issues.apache.org/jira/browse/SENTRY-1404


Repository: sentry


Description
---

SENTRY-1404: Use the new INodeAttributesProvider API in sentry-hdfs


Diffs
-

  pom.xml 2ea4735 
  
sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/hadoop/hdfs/server/namenode/AuthorizationProvider.java
 383d64d 
  
sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java
 f639f5f 
  
sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryINodeAttributesProvider.java
 PRE-CREATION 
  
sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/MockSentryAuthorizationProvider.java
 2085b52 
  
sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/MockSentryINodeAttributesProvider.java
 PRE-CREATION 
  
sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/TestSentryAuthorizationProvider.java
 5da0dc2 
  
sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/TestSentryINodeAttributesProvider.java
 PRE-CREATION 
  
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/NoAuthorizationProvider.java
 be0830d 
  
sentry-provider/sentry-provider-common/src/test/java/org/apache/sentry/provider/common/TestNoAuthorizationProvider.java
 fe01b06 
  
sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
 4d0c4b5 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
 f2d74bf 

Diff: https://reviews.apache.org/r/50055/diff/


Testing
---


Thanks,

Colin McCabe