You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Xiaomeng Huang <xi...@intel.com> on 2015/01/07 02:34:13 UTC
Review Request 29650: SENTRY-498: Sentry Hive authorization V2 via
Hive authorization framework
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29650/
-----------------------------------------------------------
Review request for sentry and Prasad Mujumdar.
Bugs: SENTRY-498
https://issues.apache.org/jira/browse/SENTRY-498
Repository: sentry
Description
-------
Currently Sentry grant/revoke privileges via hook DDLTask, and do authorization via HiveSemanticAnalyzerHook. Now hive has a pluggable authorization framework via exposing some interfaces HiveAccessController and HiveAuthorizationValidator. In this patch, SentryAccessController is used to grant/revoke roles and privileges, and SentryAuthorizationValidator is used to do fine-grained authorization.
Now it still blocked by some sentry jiras and a hive improvement(Prasad in working on this).
Diffs
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java ecbd664
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingSessionHookV2.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAccessController.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizationValidator.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizerFactory.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/DefaultSentryAccessController.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/DefaultSentryAuthorizationValidator.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/SentryAuthorizationTaskFactoryImplV2.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/SentryAuthorizerImpl.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/SimpleSemanticAnalyzer.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/util/SentryAccessControlException.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/util/SentryAuthorizerUtil.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/v2/TestDefaultSentryAuthorizationValidator.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/v2/TestSentryAuthorizerUtil.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/v2/TestSentryHiveAuthorizerFactory.java PRE-CREATION
sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/PrivilegeInfo.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 962228f
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java d21a401
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 574f23c
sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java fa5ab69
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java 04f50ed
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 742c74f
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 4a475ba
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbDDLAuditLog.java 2cecdfd
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java acb789f
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java a35cf21
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java 1af8baa
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java f8cc1d0
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMovingToProduction.java a6edf03
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java 2fbdfa6
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java 7c9a66d
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java bbac5c8
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestRuntimeMetadataRetrieval.java c47686b
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestServerConfiguration.java d8ebea6
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 934ceb8
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/InternalHiveServer.java a19cbd3
Diff: https://reviews.apache.org/r/29650/diff/
Testing
-------
Thanks,
Xiaomeng Huang
Re: Review Request 29650: SENTRY-498: Sentry Hive authorization V2
via Hive authorization framework
Posted by Xiaomeng Huang <xi...@intel.com>.
> On 一月 7, 2015, 3:29 a.m., Colin Ma wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java, line 468
> > <https://reviews.apache.org/r/29650/diff/1/?file=808311#file808311line468>
> >
> > The following code may be simple:
> > return sessionHookClassName.equalsIgnoreCase(HiveAuthzBindingSessionHook.class.getName()) || sessionHookClassName.equalsIgnoreCase(HiveAuthzBindingSessionHookV2.class.getName());
I will change it to "return HiveAuthzBindingSessionHookV2.class.getName().equalsIgnoreCase(readConfig(stmt, HiveConf.ConfVars.HIVE_SERVER2_SESSION_HOOK.varname));" In V2, HiveAuthzBindingSessionHook will be depracated.
> On 一月 7, 2015, 3:29 a.m., Colin Ma wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizerFactory.java, line 130
> > <https://reviews.apache.org/r/29650/diff/1/?file=808315#file808315line130>
> >
> > replace with @VisibleForTesting?
ok, add @VisibleForTesting and change public to protected
> On 一月 7, 2015, 3:29 a.m., Colin Ma wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/DefaultSentryAccessController.java, line 94
> > <https://reviews.apache.org/r/29650/diff/1/?file=808316#file808316line94>
> >
> > In every method, the code for getting client is the same, how about wrap it as:
> > private void checkAndSetClient() throws SentryAccessControlException {
> > try {
> > Preconditions.checkNotNull(authzConf, "HiveAuthConf cannot be null");
> > this.sentryClient = SentryServiceClientFactory.create(authzConf);
> > } catch (Exception e) {
> > String msg = "Error creating Sentry client V2: " + e.getMessage();
> > throw new SentryAccessControlException(msg, e);
> > }
> > }
good idea, will fix it.
- Xiaomeng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29650/#review66977
-----------------------------------------------------------
On 一月 7, 2015, 1:34 a.m., Xiaomeng Huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29650/
> -----------------------------------------------------------
>
> (Updated 一月 7, 2015, 1:34 a.m.)
>
>
> Review request for sentry and Prasad Mujumdar.
>
>
> Bugs: SENTRY-498
> https://issues.apache.org/jira/browse/SENTRY-498
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Currently Sentry grant/revoke privileges via hook DDLTask, and do authorization via HiveSemanticAnalyzerHook. Now hive has a pluggable authorization framework via exposing some interfaces HiveAccessController and HiveAuthorizationValidator. In this patch, SentryAccessController is used to grant/revoke roles and privileges, and SentryAuthorizationValidator is used to do fine-grained authorization.
> Now it still blocked by some sentry jiras and a hive improvement(Prasad in working on this).
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java ecbd664
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingSessionHookV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAccessController.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizationValidator.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizerFactory.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/DefaultSentryAccessController.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/DefaultSentryAuthorizationValidator.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/SentryAuthorizationTaskFactoryImplV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/SentryAuthorizerImpl.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/SimpleSemanticAnalyzer.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/util/SentryAccessControlException.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/util/SentryAuthorizerUtil.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/v2/TestDefaultSentryAuthorizationValidator.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/v2/TestSentryAuthorizerUtil.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/v2/TestSentryHiveAuthorizerFactory.java PRE-CREATION
> sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/PrivilegeInfo.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 962228f
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java d21a401
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 574f23c
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java fa5ab69
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java 04f50ed
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 742c74f
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 4a475ba
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbDDLAuditLog.java 2cecdfd
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java acb789f
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java a35cf21
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java 1af8baa
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java f8cc1d0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMovingToProduction.java a6edf03
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java 2fbdfa6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java 7c9a66d
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java bbac5c8
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestRuntimeMetadataRetrieval.java c47686b
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestServerConfiguration.java d8ebea6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 934ceb8
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/InternalHiveServer.java a19cbd3
>
> Diff: https://reviews.apache.org/r/29650/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Xiaomeng Huang
>
>
Re: Review Request 29650: SENTRY-498: Sentry Hive authorization V2
via Hive authorization framework
Posted by Colin Ma <ju...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29650/#review66977
-----------------------------------------------------------
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java
<https://reviews.apache.org/r/29650/#comment110775>
The following code may be simple:
return sessionHookClassName.equalsIgnoreCase(HiveAuthzBindingSessionHook.class.getName()) || sessionHookClassName.equalsIgnoreCase(HiveAuthzBindingSessionHookV2.class.getName());
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAccessController.java
<https://reviews.apache.org/r/29650/#comment110778>
why need the initilize method to wrap?
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizerFactory.java
<https://reviews.apache.org/r/29650/#comment110781>
replace with @VisibleForTesting?
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/DefaultSentryAccessController.java
<https://reviews.apache.org/r/29650/#comment110782>
In every method, the code for getting client is the same, how about wrap it as:
private void checkAndSetClient() throws SentryAccessControlException {
try {
Preconditions.checkNotNull(authzConf, "HiveAuthConf cannot be null");
this.sentryClient = SentryServiceClientFactory.create(authzConf);
} catch (Exception e) {
String msg = "Error creating Sentry client V2: " + e.getMessage();
throw new SentryAccessControlException(msg, e);
}
}
- Colin Ma
On Jan. 7, 2015, 1:34 a.m., Xiaomeng Huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29650/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2015, 1:34 a.m.)
>
>
> Review request for sentry and Prasad Mujumdar.
>
>
> Bugs: SENTRY-498
> https://issues.apache.org/jira/browse/SENTRY-498
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Currently Sentry grant/revoke privileges via hook DDLTask, and do authorization via HiveSemanticAnalyzerHook. Now hive has a pluggable authorization framework via exposing some interfaces HiveAccessController and HiveAuthorizationValidator. In this patch, SentryAccessController is used to grant/revoke roles and privileges, and SentryAuthorizationValidator is used to do fine-grained authorization.
> Now it still blocked by some sentry jiras and a hive improvement(Prasad in working on this).
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java ecbd664
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingSessionHookV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAccessController.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizationValidator.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizerFactory.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/DefaultSentryAccessController.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/DefaultSentryAuthorizationValidator.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/SentryAuthorizationTaskFactoryImplV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/SentryAuthorizerImpl.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/SimpleSemanticAnalyzer.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/util/SentryAccessControlException.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/util/SentryAuthorizerUtil.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/v2/TestDefaultSentryAuthorizationValidator.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/v2/TestSentryAuthorizerUtil.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/v2/TestSentryHiveAuthorizerFactory.java PRE-CREATION
> sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/PrivilegeInfo.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 962228f
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java d21a401
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 574f23c
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java fa5ab69
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java 04f50ed
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 742c74f
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 4a475ba
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbDDLAuditLog.java 2cecdfd
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java acb789f
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java a35cf21
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java 1af8baa
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java f8cc1d0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMovingToProduction.java a6edf03
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java 2fbdfa6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java 7c9a66d
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java bbac5c8
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestRuntimeMetadataRetrieval.java c47686b
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestServerConfiguration.java d8ebea6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 934ceb8
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/InternalHiveServer.java a19cbd3
>
> Diff: https://reviews.apache.org/r/29650/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Xiaomeng Huang
>
>
Re: Review Request 29650: SENTRY-498: Sentry Hive authorization V2
via Hive authorization framework
Posted by Xiaomeng Huang <xi...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29650/
-----------------------------------------------------------
(Updated March 11, 2015, 2:50 a.m.)
Review request for sentry and Prasad Mujumdar.
Changes
-------
fix some test cases failed and memory leak issue.
Bugs: SENTRY-498
https://issues.apache.org/jira/browse/SENTRY-498
Repository: sentry
Description
-------
Currently Sentry grant/revoke privileges via hook DDLTask, and do authorization via HiveSemanticAnalyzerHook. Now hive has a pluggable authorization framework via exposing some interfaces HiveAccessController and HiveAuthorizationValidator. In this patch, SentryAccessController is used to grant/revoke roles and privileges, and SentryAuthorizationValidator is used to do fine-grained authorization.
Now it still blocked by some sentry jiras and a hive improvement(Prasad in working on this).
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java ecbd664
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingSessionHookV2.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAccessController.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizationValidator.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizerFactory.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/DefaultSentryAccessController.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/DefaultSentryAuthorizationValidator.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/SentryAuthorizationTaskFactoryImplV2.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/SentryAuthorizerImpl.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/SimpleSemanticAnalyzer.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/util/SentryAccessControlException.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/util/SentryAuthorizerUtil.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/v2/TestDefaultSentryAuthorizationValidator.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/v2/TestSentryAuthorizerUtil.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/v2/TestSentryHiveAuthorizerFactory.java PRE-CREATION
sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/PrivilegeInfo.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 7a9f0df
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 44681ca
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 574f23c
sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java fa5ab69
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java 04f50ed
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 742c74f
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java f9e8f80
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbDDLAuditLog.java 2cecdfd
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java acb789f
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java a35cf21
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java 1af8baa
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestHaEnd2End.java 78894d1
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 3a8a6ef
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMovingToProduction.java a6edf03
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java 2fbdfa6
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java 7c9a66d
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java 7abc684
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestRuntimeMetadataRetrieval.java c47686b
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestServerConfiguration.java d8ebea6
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 1014361
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/InternalHiveServer.java 0e53d3d
Diff: https://reviews.apache.org/r/29650/diff/
Testing
-------
Thanks,
Xiaomeng Huang
Re: Review Request 29650: SENTRY-498: Sentry Hive authorization V2
via Hive authorization framework
Posted by Xiaomeng Huang <xi...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29650/
-----------------------------------------------------------
(Updated 一月 7, 2015, 11:37 a.m.)
Review request for sentry and Prasad Mujumdar.
Bugs: SENTRY-498
https://issues.apache.org/jira/browse/SENTRY-498
Repository: sentry
Description
-------
Currently Sentry grant/revoke privileges via hook DDLTask, and do authorization via HiveSemanticAnalyzerHook. Now hive has a pluggable authorization framework via exposing some interfaces HiveAccessController and HiveAuthorizationValidator. In this patch, SentryAccessController is used to grant/revoke roles and privileges, and SentryAuthorizationValidator is used to do fine-grained authorization.
Now it still blocked by some sentry jiras and a hive improvement(Prasad in working on this).
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java ecbd664
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingSessionHookV2.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAccessController.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizationValidator.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizerFactory.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/DefaultSentryAccessController.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/DefaultSentryAuthorizationValidator.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/SentryAuthorizationTaskFactoryImplV2.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/SentryAuthorizerImpl.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/SimpleSemanticAnalyzer.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/util/SentryAccessControlException.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/util/SentryAuthorizerUtil.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/v2/TestDefaultSentryAuthorizationValidator.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/v2/TestSentryAuthorizerUtil.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/v2/TestSentryHiveAuthorizerFactory.java PRE-CREATION
sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/PrivilegeInfo.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 962228f
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java d21a401
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 574f23c
sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java fa5ab69
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java 04f50ed
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 742c74f
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 4a475ba
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbDDLAuditLog.java 2cecdfd
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java acb789f
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java a35cf21
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java 1af8baa
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java f8cc1d0
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMovingToProduction.java a6edf03
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java 2fbdfa6
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java 7c9a66d
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java bbac5c8
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestRuntimeMetadataRetrieval.java c47686b
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestServerConfiguration.java d8ebea6
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 934ceb8
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/InternalHiveServer.java a19cbd3
Diff: https://reviews.apache.org/r/29650/diff/
Testing
-------
Thanks,
Xiaomeng Huang
Re: Review Request 29650: SENTRY-498: Sentry Hive authorization V2
via Hive authorization framework
Posted by Xiaomeng Huang <xi...@intel.com>.
> On 一月 7, 2015, 2:07 a.m., Dapeng Sun wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java, line 1582
> > <https://reviews.apache.org/r/29650/diff/1/?file=808333#file808333line1582>
> >
> > Why we add "[" and "]", on behavior of HIVE?
Yes, it is a behavior in Hive side, and I can't handle writeGrantInfo
> On 一月 7, 2015, 2:07 a.m., Dapeng Sun wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestRuntimeMetadataRetrieval.java, line 261
> > <https://reviews.apache.org/r/29650/diff/1/?file=808343#file808343line261>
> >
> > Is there any jira tracking this?
Yes, please see SENTRY-597. Now it is difficult to hand this operation because hive don't support this authorization now. Actually we can support it when we support it in hive first.
> On 一月 7, 2015, 2:07 a.m., Dapeng Sun wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java, line 216
> > <https://reviews.apache.org/r/29650/diff/1/?file=808345#file808345line216>
> >
> > It seems HiveAuthzBindingSessionHook will not be tested, yes?
In V2, HiveAuthzBindingSessionHookV2 will replace HiveAuthzBindingSessionHook, and HiveAuthzBindingSessionHook will be deprcated. HiveAuthzBindingSessionHookV2 will enable Hive authorization V2 to instead of semantic hook in HiveAuthzBindingSessionHook.
> On 一月 7, 2015, 2:07 a.m., Dapeng Sun wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java, line 87
> > <https://reviews.apache.org/r/29650/diff/1/?file=808331#file808331line87>
> >
> > It seems SentryHiveAuthorizationTaskFactoryImpl will not be tested, yes?
In V2, SentryAuthorizationTaskFactoryImplV2 will replace SentryHiveAuthorizationTaskFactoryImpl , and SentryHiveAuthorizationTaskFactoryImpl will be deprcated. SentryHiveAuthorizationTaskFactoryImplV2 has more advantages over SentryHiveAuthorizationTaskFactoryImpl.
> On 一月 7, 2015, 2:07 a.m., Dapeng Sun wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAccessController.java, line 46
> > <https://reviews.apache.org/r/29650/diff/1/?file=808313#file808313line46>
> >
> > Please use "slf4j.Logger"
yes, will fix it.
> On 一月 7, 2015, 2:07 a.m., Dapeng Sun wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingSessionHookV2.java, line 29
> > <https://reviews.apache.org/r/29650/diff/1/?file=808312#file808312line29>
> >
> > Why use the full classpath here?
same with HiveAuthzBindingSessionHook
> On 一月 7, 2015, 2:07 a.m., Dapeng Sun wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java, line 28
> > <https://reviews.apache.org/r/29650/diff/1/?file=808331#file808331line28>
> >
> > It not better to move the dependence
If the import is not used, and I am modifing this file, I will remove it.
On 一月 7, 2015, 2:07 a.m., Xiaomeng Huang wrote:
> > Hi Xiaomeng, the feature is great and significant! But the review request is so big, just finish review on some test related code, I will continue reviewing other part and give you my feedback.
Thanks dapeng for reviewing! You mentioned "don't remove some imports", I just remove some unused imports or sort the imports, because I set the format in my eclipse(organize imports), I think it is good for open source code format.
- Xiaomeng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29650/#review66968
-----------------------------------------------------------
On 一月 7, 2015, 1:34 a.m., Xiaomeng Huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29650/
> -----------------------------------------------------------
>
> (Updated 一月 7, 2015, 1:34 a.m.)
>
>
> Review request for sentry and Prasad Mujumdar.
>
>
> Bugs: SENTRY-498
> https://issues.apache.org/jira/browse/SENTRY-498
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Currently Sentry grant/revoke privileges via hook DDLTask, and do authorization via HiveSemanticAnalyzerHook. Now hive has a pluggable authorization framework via exposing some interfaces HiveAccessController and HiveAuthorizationValidator. In this patch, SentryAccessController is used to grant/revoke roles and privileges, and SentryAuthorizationValidator is used to do fine-grained authorization.
> Now it still blocked by some sentry jiras and a hive improvement(Prasad in working on this).
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java ecbd664
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingSessionHookV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAccessController.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizationValidator.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizerFactory.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/DefaultSentryAccessController.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/DefaultSentryAuthorizationValidator.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/SentryAuthorizationTaskFactoryImplV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/SentryAuthorizerImpl.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/SimpleSemanticAnalyzer.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/util/SentryAccessControlException.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/util/SentryAuthorizerUtil.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/v2/TestDefaultSentryAuthorizationValidator.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/v2/TestSentryAuthorizerUtil.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/v2/TestSentryHiveAuthorizerFactory.java PRE-CREATION
> sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/PrivilegeInfo.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 962228f
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java d21a401
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 574f23c
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java fa5ab69
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java 04f50ed
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 742c74f
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 4a475ba
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbDDLAuditLog.java 2cecdfd
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java acb789f
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java a35cf21
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java 1af8baa
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java f8cc1d0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMovingToProduction.java a6edf03
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java 2fbdfa6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java 7c9a66d
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java bbac5c8
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestRuntimeMetadataRetrieval.java c47686b
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestServerConfiguration.java d8ebea6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 934ceb8
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/InternalHiveServer.java a19cbd3
>
> Diff: https://reviews.apache.org/r/29650/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Xiaomeng Huang
>
>
Re: Review Request 29650: SENTRY-498: Sentry Hive authorization V2
via Hive authorization framework
Posted by Dapeng Sun <da...@intel.com>.
On 一月 7, 2015, 10:07 a.m., Xiaomeng Huang wrote:
> > Hi Xiaomeng, the feature is great and significant! But the review request is so big, just finish review on some test related code, I will continue reviewing other part and give you my feedback.
>
> Xiaomeng Huang wrote:
> Thanks dapeng for reviewing! You mentioned "don't remove some imports", I just remove some unused imports or sort the imports, because I set the format in my eclipse(organize imports), I think it is good for open source code format.
Hi Xiaomeng, about the dependences, you may misunderstand my comments, removing some unused is okay, but in your patch, I found many dependences just been changed theirs orders. In general, we would not move them without reason and eclipse may not change so many orders automatically. Have your imported "eclipse-java-google-style.xml", I'm afraid your eclipse format should be consistent with the standard.
- Dapeng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29650/#review66968
-----------------------------------------------------------
On 一月 7, 2015, 9:34 a.m., Xiaomeng Huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29650/
> -----------------------------------------------------------
>
> (Updated 一月 7, 2015, 9:34 a.m.)
>
>
> Review request for sentry and Prasad Mujumdar.
>
>
> Bugs: SENTRY-498
> https://issues.apache.org/jira/browse/SENTRY-498
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Currently Sentry grant/revoke privileges via hook DDLTask, and do authorization via HiveSemanticAnalyzerHook. Now hive has a pluggable authorization framework via exposing some interfaces HiveAccessController and HiveAuthorizationValidator. In this patch, SentryAccessController is used to grant/revoke roles and privileges, and SentryAuthorizationValidator is used to do fine-grained authorization.
> Now it still blocked by some sentry jiras and a hive improvement(Prasad in working on this).
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java ecbd664
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingSessionHookV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAccessController.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizationValidator.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizerFactory.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/DefaultSentryAccessController.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/DefaultSentryAuthorizationValidator.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/SentryAuthorizationTaskFactoryImplV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/SentryAuthorizerImpl.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/SimpleSemanticAnalyzer.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/util/SentryAccessControlException.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/util/SentryAuthorizerUtil.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/v2/TestDefaultSentryAuthorizationValidator.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/v2/TestSentryAuthorizerUtil.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/v2/TestSentryHiveAuthorizerFactory.java PRE-CREATION
> sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/PrivilegeInfo.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 962228f
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java d21a401
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 574f23c
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java fa5ab69
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java 04f50ed
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 742c74f
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 4a475ba
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbDDLAuditLog.java 2cecdfd
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java acb789f
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java a35cf21
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java 1af8baa
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java f8cc1d0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMovingToProduction.java a6edf03
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java 2fbdfa6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java 7c9a66d
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java bbac5c8
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestRuntimeMetadataRetrieval.java c47686b
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestServerConfiguration.java d8ebea6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 934ceb8
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/InternalHiveServer.java a19cbd3
>
> Diff: https://reviews.apache.org/r/29650/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Xiaomeng Huang
>
>
Re: Review Request 29650: SENTRY-498: Sentry Hive authorization V2
via Hive authorization framework
Posted by Dapeng Sun <da...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29650/#review66968
-----------------------------------------------------------
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java
<https://reviews.apache.org/r/29650/#comment110734>
Please not move the depenence if needed
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingSessionHookV2.java
<https://reviews.apache.org/r/29650/#comment110740>
Why use the full classpath here?
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAccessController.java
<https://reviews.apache.org/r/29650/#comment110737>
Please use "slf4j.Logger"
sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java
<https://reviews.apache.org/r/29650/#comment110752>
Please not move the depenence if needed
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java
<https://reviews.apache.org/r/29650/#comment110742>
It not better to move the dependence
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java
<https://reviews.apache.org/r/29650/#comment110743>
It seems SentryHiveAuthorizationTaskFactoryImpl will not be tested, yes?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
<https://reviews.apache.org/r/29650/#comment110745>
Why we add "[" and "]", on behavior of HIVE?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
<https://reviews.apache.org/r/29650/#comment110751>
Please not move the depenence if needed
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java
<https://reviews.apache.org/r/29650/#comment110749>
Please not move the depenence if needed
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java
<https://reviews.apache.org/r/29650/#comment110748>
Please not move the depenence if needed
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestRuntimeMetadataRetrieval.java
<https://reviews.apache.org/r/29650/#comment110750>
Is there any jira tracking this?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java
<https://reviews.apache.org/r/29650/#comment110747>
It seems HiveAuthzBindingSessionHook will not be tested, yes?
Hi Xiaomeng, the feature is great and significant! But the review request is so big, just finish review on some test related code, I will continue reviewing other part and give you my feedback.
- Dapeng Sun
On 一月 7, 2015, 9:34 a.m., Xiaomeng Huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29650/
> -----------------------------------------------------------
>
> (Updated 一月 7, 2015, 9:34 a.m.)
>
>
> Review request for sentry and Prasad Mujumdar.
>
>
> Bugs: SENTRY-498
> https://issues.apache.org/jira/browse/SENTRY-498
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Currently Sentry grant/revoke privileges via hook DDLTask, and do authorization via HiveSemanticAnalyzerHook. Now hive has a pluggable authorization framework via exposing some interfaces HiveAccessController and HiveAuthorizationValidator. In this patch, SentryAccessController is used to grant/revoke roles and privileges, and SentryAuthorizationValidator is used to do fine-grained authorization.
> Now it still blocked by some sentry jiras and a hive improvement(Prasad in working on this).
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java ecbd664
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingSessionHookV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAccessController.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizationValidator.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizerFactory.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/DefaultSentryAccessController.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/DefaultSentryAuthorizationValidator.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/SentryAuthorizationTaskFactoryImplV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/SentryAuthorizerImpl.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/impl/SimpleSemanticAnalyzer.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/util/SentryAccessControlException.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/v2/util/SentryAuthorizerUtil.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/v2/TestDefaultSentryAuthorizationValidator.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/v2/TestSentryAuthorizerUtil.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/v2/TestSentryHiveAuthorizerFactory.java PRE-CREATION
> sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/PrivilegeInfo.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 962228f
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java d21a401
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 574f23c
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java fa5ab69
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java 04f50ed
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java 742c74f
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 4a475ba
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbDDLAuditLog.java 2cecdfd
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java acb789f
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java a35cf21
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java 1af8baa
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java f8cc1d0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMovingToProduction.java a6edf03
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java 2fbdfa6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java 7c9a66d
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java bbac5c8
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestRuntimeMetadataRetrieval.java c47686b
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestServerConfiguration.java d8ebea6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 934ceb8
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/InternalHiveServer.java a19cbd3
>
> Diff: https://reviews.apache.org/r/29650/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Xiaomeng Huang
>
>