> On June 8, 2014, 6:57 p.m., Sravya Tirukkovalur wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBinding.java, > > lines 191-262 > > <https://reviews.apache.org/r/22340/diff/2/?file=605386#file605386line191> > > > > Would be good to have same code paths for resolving (operation) => > > (required input/output privileges) as SemanticAnalyzerhook unless there is > > a reason not to? > > > > Here is how we resolve it based on operation scope of the operation in > > SemanticAnalyzerHook: > > https://github.com/apache/incubator-sentry/blob/master/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java#L357 > > Prasad Mujumdar wrote: > The entry point of Sentry for Hive is the compiler callback. This is a > common method for each query that gets the syntax tree and input/out entities > (ie. list of objects referred by the statement). Hence we have a common code > that examines the operation and generate the input and output hierarchies for > Sentry authorization. > In case of metastore, the entry point of Sentry is the pre-envent > callback. This gets a different context objects for each operation which > makes us easy to identify the operation type. Hence we have a different > handler method for each operation which hand constructs the input/out > hierarchies for that specific operation. > Thus we don't need the equivalent of hive binding hook for metastore > operations. > > > Sravya Tirukkovalur wrote: > Thanks for the info Prasad! I agree that contexts are different for these > two code paths and operation type is easily available in the pre event > listener context. But I am still wondering this implementation means we have > two different sources of truth on which operation translates to which > required privilege scope. For example: Alter table - add part requires all on > DB, which is deducible from here: > https://github.com/apache/incubator-sentry/blob/master/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java#L126 > > But also from the above authorizeAddPartition. So there is scope for > divergence in future.
The required privileges are not different, they are picked up from the same hiveAuthzStmtPrivMap by calling HiveAuthzPrivilegesMap.getHiveAuthzPrivileges(hiveOp). It's only the requested privileges which are extracted differently. Does that answers the question or I misunderstood the comment. - Prasad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22340/#review45037 ----------------------------------------------------------- On June 8, 2014, 2:30 a.m., Prasad Mujumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22340/ > ----------------------------------------------------------- > > (Updated June 8, 2014, 2:30 a.m.) > > > Review request for sentry, Brock Noland, Jarek Cecho, and Sravya Tirukkovalur. > > > Bugs: SENTRY-259 > https://issues.apache.org/jira/browse/SENTRY-259 > > > Repository: sentry > > > Description > ------- > > - Basic metastore binding via pre-even listener hooks. Uses the same > privilege model as hive. Required authorizable lists are created and the hive > binding is invoked with the corresponding hive operation type. The rest of > the auth is then handled by the Sentry. > - Test framework support for starting a metastore server > > > Diffs > ----- > > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java > 63484a8 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java > 7b7bf8e > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBinding.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/PolicyProviderForTest.java > 79ca387 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java > 8beedd7 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java > b6bb09c > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java > 39d411e > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestSentryOnFailureHookLoading.java > cae270b > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/MiniDFS.java > 184c066 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java > 19ff6cf > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/InternalMetastoreServer.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java > PRE-CREATION > sentry-tests/sentry-tests-hive/src/test/resources/core-site.xml > PRE-CREATION > > Diff: https://reviews.apache.org/r/22340/diff/ > > > Testing > ------- > > Added new test with various DB and Table opeartions. Additional testing > effort is tracked by a different jira. > > > Thanks, > > Prasad Mujumdar > >
