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