> On June 21, 2014, 7:47 a.m., Prasad Mujumdar wrote: > > Firstly, thanks for taking the initiative to correct the privilege model > > and for all the work! > > Overall looks fine. A couple of suggestions high level comments - > > 1. The metastore plugin needs to be updated for this. Feel free to log a > > followup ticket and assign to me. I can build that patch on top of this > > change. > > 2. The hive binding hook is capturing the partition URI for DB scope > > operation. See > > https://github.com/apache/incubator-sentry/blob/master/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java#L386 > > Now that add partition is table scope, we need to move that to table > > scope code block. > > > > A few more comments/questions on specific changes below.
Created this for metastore side changes as a follow on: https://issues.apache.org/jira/browse/SENTRY-311 - Sravya ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22847/#review46359 ----------------------------------------------------------- On June 22, 2014, 11:14 p.m., Sravya Tirukkovalur wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22847/ > ----------------------------------------------------------- > > (Updated June 22, 2014, 11:14 p.m.) > > > Review request for sentry, Arun Suresh, Jarek Cecho, Lenni Kuff, and Prasad > Mujumdar. > > > Bugs: SENTRY-310 > https://issues.apache.org/jira/browse/SENTRY-310 > > > Repository: sentry > > > Description > ------- > > Required privileges for a given hive operation is too restrictive in some > cases. This patch cleans that up. The new model is documented as a pdf > attached to the ticket. > > In short: > - All DDL statements on an object require ALL on that object, except the > create database/table/view/partition which requires all on the parent, as we > should not allow granting privileges on non existing objects. > - Cleaned up some unwanted uri privileges, now we only support all on URI. > - Fixed some more non intuitive mappings > > > Diffs > ----- > > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java > 6c507b83419ab5e5e2797c62dc71bfa0fdf36776 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java > cedf368825a153be13d3a05d1519a581bc30082f > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java > 7d241d0ea7957e6b6c334c78c6bcf0934f1a36ab > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java > fd969a6cb221656d2dee65a068cdce77e1efc5cd > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java > e725eb06fc9915b0bcc2609e428a62feea80ec43 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/22847/diff/ > > > Testing > ------- > > Captured most of the Hive operations in TestOperations test class. All of > them pass. > > Added todos for the operations which need test cases. Now running the entire > suite. > > > Thanks, > > Sravya Tirukkovalur > >
