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