----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25197/#review52502 -----------------------------------------------------------
Looks fine to me. A couple of minor comments below, mostly to verify things. I think we should add some more test cases, especially the negative tests to verify that fine grain DDLs don't give you access to data. Eg Create or Alter privileges on table shouldn't work for SELECT and INSERT statements. Please feel free to log followup tickets for testing. sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java <https://reviews.apache.org/r/25197/#comment91307> Is this causing a backward incompatibility for SQL client ? sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java <https://reviews.apache.org/r/25197/#comment91311> Can we remove these todos ? - Prasad Mujumdar On Aug. 31, 2014, 7:06 p.m., Sravya Tirukkovalur wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25197/ > ----------------------------------------------------------- > > (Updated Aug. 31, 2014, 7:06 p.m.) > > > Review request for sentry, Lenni Kuff and Prasad Mujumdar. > > > Bugs: sentry-331 > https://issues.apache.org/jira/browse/sentry-331 > > > Repository: sentry > > > Description > ------- > > Adding more granular privileges to the DBModel: Create, Drop, Alter, Index > and Lock. > Following broad rules apply: > - Creating a data object requires "create" privilege on the parent. That is, > create table requires create on db, create db requires create on server. > - Dropping a data object requires "drop" privilege on that object. > - All alter commands require "alter" privilege on that table with the > following exceptions: > -- Alter table drop also requires "drop" privilege on the table in addition > to "alter". > -- Alter table index rebuild only requires "index" privilege on the table > -- Alter table rename also requires "create" on db. > - Locking table requires "lock" on table. > > This patch also fixes SENTRY-413 and SENTRY-414 > > Note: I put comment "//TODO: Make sure" in the places where a second opinion > will help to make sure we are enforcing the right privileges for those > commands. > > > Diffs > ----- > > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/SentryHiveConstants.java > 49922f9f764579453a23bc4ccea9ce09f5fbcebd > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java > 0b268068b71b3602b95ddde9c87b21f4bfdc5754 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java > 2df741cab38aa6e7677d13da9677f8ea398149c6 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java > 9498a2806a997c441bf86bcc4b0f229d46cb1777 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java > 1c32946980d0b96edfb55a36e1b718d4248437ac > > sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/AccessConstants.java > 4e89f68dccbf92ab26d4150e929d363f2f098f27 > > sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/DBModelAction.java > a4f3a87862312794fe724f1cffc8926de8b3deba > > sentry-policy/sentry-policy-db/src/test/java/org/apache/sentry/policy/db/TestDBWildcardPrivilege.java > f4862e0a072c43b46f5470b988c8bc9419eb1fe1 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java > d4c58061779a551dda27bcb1dd505f5fb42045ba > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java > 30cbb0db13d190e4ec89bcd777021c2eaa756c40 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/SentryPolicyProviderForDb.java > c60d0d54cec4780c8a8c134885c7eef3c388f34f > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java > 55ae2f4aab3ea56050363d184eb435b605f3b4c5 > > Diff: https://reviews.apache.org/r/25197/diff/ > > > Testing > ------- > > Added test cases to the new updated privileges. > > > Thanks, > > Sravya Tirukkovalur > >
