> On May 10, 2018, 2:17 a.m., Sergio Pena wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java > > Lines 123 (patched) > > <https://reviews.apache.org/r/67046/diff/1/?file=2019242#file2019242line123> > > > > Are roles with INSERT privileges allowed to add partitions in a table? > > > > I think that in order to add new partitions on a table, the ALTER > > privilege is required; and to remove partitions the DROP privilege is > > required. Both privileges are supported in Sentry. Can you confirm which > > privilege is required? > > Na Li wrote: > I debug into the code, the following is what's filled by hive > > // input required privilege: > > // 1) select, scope: user, db, table, column 2) delete, scope: > user, db, table > > // output required privilege: > > // 1) insert, scope: user, db, table > > Sergio Pena wrote: > Which code did you debug? > > Sentry has this privilege for adding partitions: > > HiveAuthzPrivileges addPartitionPrivilege = new > HiveAuthzPrivileges.AuthzPrivilegeBuilder(). > addOutputObjectPriviledge(AuthorizableType.Table, > EnumSet.of(DBModelAction.ALTER)). > //TODO: Uncomment this if we want to make it more restrictive > //addInputObjectPriviledge(AuthorizableType.Table, > EnumSet.of(DBModelAction.CREATE)). > addInputObjectPriviledge(AuthorizableType.URI, > EnumSet.of(DBModelAction.SELECT)).//TODO: make it optional > addOutputObjectPriviledge(AuthorizableType.URI, > EnumSet.of(DBModelAction.ALL)). > setOperationScope(HiveOperationScope.TABLE). > setOperationType(HiveOperationType.DDL). > build(); > > And this for dropping partitions: > > HiveAuthzPrivileges dropPartitionPrivilege = new > HiveAuthzPrivileges.AuthzPrivilegeBuilder(). > addInputObjectPriviledge(AuthorizableType.Table, > EnumSet.of(DBModelAction.ALTER)). > addInputObjectPriviledge(AuthorizableType.Table, > EnumSet.of(DBModelAction.DROP)). > setOperationScope(HiveOperationScope.TABLE). > setOperationType(HiveOperationType.DDL). > build(); > > Isn't exchanging partitions the same as adding a partition in the dest > table and dropping a partition in the source table? > > Na Li wrote: > 1) That is why you think, but not what hive does. > > Can you take a look at input required privileges and output required > privileges in stmtOperation for command "ALTER TABLE EXCHANGE" in following > code of HiveAuthzBindingHook class. These are the privileges user must have > in order to execute the command. > > @Override > public void postAnalyze(HiveSemanticAnalyzerHookContext context, > List<Task<? extends Serializable>> rootTasks) throws > SemanticException { > HiveOperation stmtOperation = context.getHiveOperation(); > HiveAuthzPrivileges stmtAuthObject; > > stmtAuthObject = > HiveAuthzPrivilegesMap.getHiveAuthzPrivileges(stmtOperation); <-check > stmtOperation > > > 2) Hive actions > > public enum PrivilegeType { > > ALL(HiveParser.TOK_PRIV_ALL, "All"), > ALTER_DATA(HiveParser.TOK_PRIV_ALTER_DATA, "Update"), > ALTER_METADATA(HiveParser.TOK_PRIV_ALTER_METADATA, "Alter"), > CREATE(HiveParser.TOK_PRIV_CREATE, "Create"), > DROP(HiveParser.TOK_PRIV_DROP, "Drop"), > INDEX(HiveParser.TOK_PRIV_INDEX, "Index"), > LOCK(HiveParser.TOK_PRIV_LOCK, "Lock"), > SELECT(HiveParser.TOK_PRIV_SELECT, "Select"), > SHOW_DATABASE(HiveParser.TOK_PRIV_SHOW_DATABASE, "Show_Database"), > INSERT(HiveParser.TOK_PRIV_INSERT, "Insert"), > DELETE(HiveParser.TOK_PRIV_DELETE, "Delete"), > UNKNOWN(null, null); > > 3) Sentry supported actions > > public enum DBModelAction implements Action { > > // SENTRY-1292 > // Need to ensure the order of enum to have SELECT in front to avoid > performance > // regression. Since most real use case of permissions may be read > only(SELECT). > SELECT(AccessConstants.SELECT), > INSERT(AccessConstants.INSERT), > ALTER(AccessConstants.ALTER), > CREATE(AccessConstants.CREATE), > DROP(AccessConstants.DROP), > INDEX(AccessConstants.INDEX), > LOCK(AccessConstants.LOCK), > ALL(AccessConstants.ALL); > > 4) As you can see Hive has DELECT, but sentry does not have that. DELECT > is different from DROP. IN "ALTER TABLE EXCHANGE", the partition is not > dropped. It is deleted from source table and inserted into destination table. > 5) As shown in the added unit tests, you can see once granting user the > corresponding privileges, user can execute the command. Basically, the > privileges requested by hive have to be granted to user.
Hive has a DELETE command which requires DELETE privileges. Why do we need DELETE for an ALTER operation? That's the legacy authorization of Hive which it does not necessary match to Sentry. Btw, I think both tables (source and destination) require ALTER privileges instead of CREATE/DROP or INSERT/DELETE because this exchange happens with an ALTER command, which btw, I think we would need to fix the ALTER ADDPART and ALTER DROPPART to reflect the privilege if we do this (in another patch of course). Regarding the partition is not dropped comment, I tested it and the partition is dropped from the source table, and added in the destination table. But if we do allow the exchange with the ALTER privilege in both source and destination, then that would fix the privileges, right? - Sergio ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67046/#review202818 ----------------------------------------------------------- On May 10, 2018, 1:11 a.m., Na Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67046/ > ----------------------------------------------------------- > > (Updated May 10, 2018, 1:11 a.m.) > > > Review request for sentry. > > > Bugs: sentry-2226 > https://issues.apache.org/jira/browse/sentry-2226 > > > Repository: sentry > > > Description > ------- > > add support for "ALTER TABLE EXCHANGE" > > > Diffs > ----- > > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java > ffa193f > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbColumnLevelMetaDataOps.java > 3735179 > > > Diff: https://reviews.apache.org/r/67046/diff/1/ > > > Testing > ------- > > unit test for "ALTER TABLE EXCHANGE" succeeded > > > Thanks, > > Na Li > >
