diff --git a/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/AlterPartitionEvent.java b/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/AlterPartitionEvent.java index a2ffb0ba9af0..561cf604e33a 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/AlterPartitionEvent.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/AlterPartitionEvent.java @@ -20,12 +20,15 @@ package org.apache.hadoop.hive.ql.security.authorization.plugin.metastore.events; import org.apache.commons.lang3.StringUtils; +import org.apache.hadoop.hive.metastore.Warehouse; import org.apache.hadoop.hive.metastore.api.Partition; import org.apache.hadoop.hive.metastore.events.PreAlterPartitionEvent; import org.apache.hadoop.hive.metastore.events.PreEventContext; +import org.apache.hadoop.hive.ql.metadata.Hive; +import org.apache.hadoop.hive.ql.metadata.HiveException; +import org.apache.hadoop.hive.ql.metadata.Table; import org.apache.hadoop.hive.ql.security.authorization.plugin.HiveOperationType; import org.apache.hadoop.hive.ql.security.authorization.plugin.HivePrivilegeObject; -import org.apache.hadoop.hive.ql.security.authorization.plugin.HivePrivilegeObject.HivePrivilegeObjectType; import org.apache.hadoop.hive.ql.security.authorization.plugin.metastore.HiveMetaStoreAuthorizableEvent; import org.apache.hadoop.hive.ql.security.authorization.plugin.metastore.HiveMetaStoreAuthzInfo; import org.slf4j.Logger; @@ -76,10 +79,17 @@ private List getOutputHObjs() { ret.add(getHivePrivilegeObject(event.getTable())); Partition newPartition = event.getNewPartition(); - String newUri = (newPartition != null) ? getSdLocation(newPartition.getSd()) : ""; + String oldUri = ""; - if (StringUtils.isNotEmpty(newUri)) { - ret.add(getHivePrivilegeObjectDfsUri(newUri)); + if (event.getOldPartVals() != null && event.getOldPartVals().size() > 0) { + oldUri = this.getOldPartSdLocation(event); + } + + String newUri = (newPartition != null) ? getSdLocation(newPartition.getSd()) : ""; + + // Skip DFS_URI auth if new partition loc is empty or old and new partition loc are same + if (StringUtils.isNotEmpty(newUri) && !StringUtils.equalsIgnoreCase(oldUri, newUri)) { + ret.add(getHivePrivilegeObjectDfsUri(newUri)); } COMMAND_STR = buildCommandString(COMMAND_STR, event.getTableName(), newPartition); @@ -89,6 +99,19 @@ private List getOutputHObjs() { return ret; } + private String getOldPartSdLocation(PreAlterPartitionEvent event) { + Table table = new Table(event.getTable()); + try { + org.apache.hadoop.hive.ql.metadata.Partition partition = Hive.get().getPartition(table, + Warehouse.makeSpecFromValues(event.getTable().getPartitionKeys(), event.getOldPartVals())); + + return partition.getLocation(); + + } catch (HiveException e) { + throw new RuntimeException(e); + } + } + private String buildCommandString(String cmdStr, String tbl, Partition partition ) { String ret = cmdStr; @@ -100,4 +123,4 @@ private String buildCommandString(String cmdStr, String tbl, Partition partition return ret; } -} +} \ No newline at end of file diff --git a/ql/src/test/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/TestHiveMetaStoreAuthorizer.java b/ql/src/test/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/TestHiveMetaStoreAuthorizer.java index 07e287baca03..52cc1d107262 100644 --- a/ql/src/test/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/TestHiveMetaStoreAuthorizer.java +++ b/ql/src/test/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/TestHiveMetaStoreAuthorizer.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hive.ql.security.authorization.plugin.metastore; +import com.google.common.collect.Lists; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.tuple.ImmutablePair; import org.apache.commons.lang3.tuple.Pair; @@ -45,12 +46,13 @@ import org.apache.hadoop.hive.ql.security.authorization.plugin.HivePrivilegeObject; import org.apache.hadoop.hive.ql.security.authorization.plugin.metastore.filtercontext.TableFilterContext; import org.apache.hadoop.security.UserGroupInformation; +import org.apache.thrift.TException; +import org.junit.Before; import org.junit.FixMethodOrder; +import org.junit.Test; import org.junit.runners.MethodSorters; -import org.junit.Before; import org.mockito.ArgumentCaptor; import org.mockito.Mockito; -import org.junit.Test; import java.util.ArrayList; import java.util.List; @@ -66,6 +68,7 @@ import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.eq; import static org.mockito.Mockito.verify; /* @@ -87,6 +90,7 @@ public class TestHiveMetaStoreAuthorizer { private static final String metaConfVal = ""; private static final String TEST_DATA_DIR = new File("file:///testdata").getPath(); + private static final List PARTCOL_SCHEMA = Lists.newArrayList("yyyy", "mm", "dd"); private RawStore rawStore; private Configuration conf; private HMSHandler hmsHandler; @@ -696,6 +700,106 @@ public void testU_DropDataConnector_authorizedUser() { } } + + /** + * Captures and returns the privilege objects for Alter Partition + */ + private Pair, List> getHivePrivilegeObjectsForAlterPartition() + throws HiveAuthzPluginException, HiveAccessControlException { + @SuppressWarnings("unchecked") + Class> class_listPrivObjects = (Class) List.class; + ArgumentCaptor> inputsCapturer = ArgumentCaptor + .forClass(class_listPrivObjects); + ArgumentCaptor> outputsCapturer = ArgumentCaptor + .forClass(class_listPrivObjects); + + verify(mockHiveAuthorizer).checkPrivileges(eq(HiveOperationType.ALTERPARTITION_FILEFORMAT), + inputsCapturer.capture(), outputsCapturer.capture(), + any(HiveAuthzContext.class)); + + return new ImmutablePair<>(inputsCapturer.getValue(), outputsCapturer.getValue()); + } + @Test + public void testV_AlterPartition_DFSUriPrivObject() { + UserGroupInformation.setLoginUser(UserGroupInformation.createRemoteUser(authorizedUser)); + try { + List> testValues = createTable4PartColsParts(); + List oldParts = hmsHandler.get_partitions(dbName, tblName, (short) -1); + Partition oldPart = oldParts.get(3); + Partition newPart = makeTestChangesOnPartition(oldPart); + + hmsHandler.rename_partition(dbName, tblName,oldPart.getValues(),newPart); + + Pair, List> io = getHivePrivilegeObjectsForAlterPartition(); + List outputs = io.getRight(); + + List tableOutputs = outputs.stream() + .filter(o -> o.getType() == HivePrivilegeObject.HivePrivilegeObjectType.DFS_URI) + .collect(Collectors.toList()); + + assertEquals("Should have one DFS_URI privilege object", 1, tableOutputs.size()); + HivePrivilegeObject DFSUriObj = tableOutputs.get(0); + + assertEquals("DFS_URI should be same as new partition location", + oldPart.getSd().getLocation()+ "/hh=01", DFSUriObj.getObjectName()); + } catch (Exception e) { + fail("testV_AlterPartition_DFSUriPrivObject() failed with " + e); + } + } + + protected Table createPartitionedTestTable(String dbName, String tableName, + List partCols, boolean setPartitionLevelPrivilages) + throws Exception { + + Database db = new DatabaseBuilder() + .setName(dbName) + .build(conf); + hmsHandler.create_database(db); + + TableBuilder builder = new TableBuilder() + .setDbName(dbName) + .setTableName(tableName) + .addCol("id", "int") + .addCol("name", "string"); + + partCols.forEach(col -> builder.addPartCol(col, "string")); + Table table = builder.build(conf); + + hmsHandler.create_table(table); + return table; + } + + protected List> createTable4PartColsParts() throws + Exception { + Table table = createPartitionedTestTable(dbName, tblName, PARTCOL_SCHEMA, false); + List> testValues = Lists.newArrayList( + Lists.newArrayList("1999", "01", "02"), + Lists.newArrayList("2009", "02", "10"), + Lists.newArrayList("2017", "10", "26"), + Lists.newArrayList("2017", "11", "27")); + + for (List vals : testValues) { + addPartition(table, vals); + } + + return testValues; + } + + protected void addPartition(Table table, List values) + throws TException { + PartitionBuilder partitionBuilder = new PartitionBuilder().inTable(table); + values.forEach(val -> partitionBuilder.addValue(val)); + hmsHandler.add_partition(partitionBuilder.build(conf)); + } + + protected static Partition makeTestChangesOnPartition(Partition partition) { + Partition newPart = new Partition(partition); + newPart.getParameters().put("hmsTestParam001", "testValue001"); + newPart.getSd().setLocation(partition.getSd().getLocation() + "/hh=01"); + newPart.setValues(Lists.newArrayList("2018", "11", "27")); + return newPart; + } + @Test public void testUnAuthorizedCause() { UserGroupInformation.setLoginUser(UserGroupInformation.createRemoteUser(unAuthorizedUser)); diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java index 75266aa6b1b3..46d6470ed0e3 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java @@ -6103,7 +6103,8 @@ private void alter_partitions_with_environment_context(String catName, String db if (tmpPart.getSd() != null && tmpPart.getSd().getCols() != null && tmpPart.getSd().getCols().isEmpty()) { tmpPart.getSd().setCols(table.getSd().getCols()); } - firePreEvent(new PreAlterPartitionEvent(db_name, tbl_name, table, null, tmpPart, this)); + // old part values are same as new partition values here + firePreEvent(new PreAlterPartitionEvent(db_name, tbl_name, table, tmpPart.getValues(), tmpPart, this)); } oldParts = alterHandler.alterPartitions(getMS(), wh, catName, db_name, tbl_name, new_parts, environmentContext, writeIdList, writeId, this);