diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestReplChangeManager.java b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestReplChangeManager.java index 392ebad1df..a313631c2b 100644 --- a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestReplChangeManager.java +++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestReplChangeManager.java @@ -399,13 +399,16 @@ public void testRecycleUsingImpersonation() throws Exception { "impala", hiveConf); //set owner of data path to impala fs.setOwner(dirTbl1, "impala", "default"); + fs.setOwner(part11, "impala", "default"); + fs.setOwner(part12, "impala", "default"); proxyUserUgi.doAs((PrivilegedExceptionAction) () -> { try { //impala doesn't have access. Should provide access control exception ReplChangeManager.getInstance(hiveConf).recycle(dirTbl1, RecycleType.MOVE, false); Assert.fail(); } catch (AccessControlException e) { - assertTrue(e.getMessage().contains("Permission denied: user=impala, access=WRITE")); + assertTrue(e.getMessage().contains("Permission denied: user=impala, access=EXECUTE")); + assertTrue(e.getMessage().contains("/cmroot")); } return null; }); @@ -434,6 +437,61 @@ public void testRecycleUsingImpersonation() throws Exception { } while (!cleared); } + @Test + public void tesRecyleImpersionationWithGroupPermissions() throws Exception { + FileSystem fs = warehouse.getWhRoot().getFileSystem(hiveConf); + Path dirDb = new Path(warehouse.getWhRoot(), "db3"); + long now = System.currentTimeMillis(); + fs.delete(dirDb, true); + fs.mkdirs(dirDb); + Path dirTbl2 = new Path(dirDb, "tbl2"); + fs.mkdirs(dirTbl2); + Path part21 = new Path(dirTbl2, "part1"); + createFile(part21, "testClearer21"); + String fileChksum21 = ReplChangeManager.checksumFor(part21, fs); + Path part22 = new Path(dirTbl2, "part2"); + createFile(part22, "testClearer22"); + String fileChksum22 = ReplChangeManager.checksumFor(part22, fs); + final UserGroupInformation proxyUserUgi = + UserGroupInformation.createUserForTesting("impala2", new String[] {"supergroup"}); + //set owner of data path to impala2 + fs.setOwner(dirTbl2, "impala2", "default"); + fs.setOwner(part21, "impala2", "default"); + fs.setOwner(part22, "impala2", "default"); + proxyUserUgi.doAs((PrivilegedExceptionAction) () -> { + try { + //impala2 doesn't have access but it belongs to a group which does. + ReplChangeManager.getInstance(hiveConf).recycle(dirTbl2, RecycleType.MOVE, false); + } catch (Exception e) { + Assert.fail(); + } + return null; + }); + ReplChangeManager.getInstance().recycle(dirTbl2, RecycleType.MOVE, false); + Assert.assertFalse(fs.exists(part21)); + Assert.assertFalse(fs.exists(part22)); + assertTrue(fs.exists(ReplChangeManager.getCMPath(hiveConf, part21.getName(), fileChksum21, cmroot))); + assertTrue(fs.exists(ReplChangeManager.getCMPath(hiveConf, part22.getName(), fileChksum22, cmroot))); + fs.setTimes(ReplChangeManager.getCMPath(hiveConf, part21.getName(), fileChksum21, cmroot), + now - 7 * 86400 * 1000 * 2, now - 7 * 86400 * 1000 * 2); + ReplChangeManager.scheduleCMClearer(hiveConf); + + long start = System.currentTimeMillis(); + long end; + boolean cleared = false; + do { + Thread.sleep(200); + end = System.currentTimeMillis(); + if (end - start > 5000) { + Assert.fail("timeout, cmroot has not been cleared"); + } + if (!fs.exists(ReplChangeManager.getCMPath(hiveConf, part21.getName(), fileChksum21, cmroot)) && + fs.exists(ReplChangeManager.getCMPath(hiveConf, part22.getName(), fileChksum22, cmroot))) { + cleared = true; + } + } while (!cleared); + } + @Test public void testRecycleUsingImpersonationWithAccess() throws Exception { try { diff --git a/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java b/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java index 5b76acec64..1d7c7bad8a 100644 --- a/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java +++ b/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java @@ -20,13 +20,16 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; import java.util.HashMap; import java.util.Map; +import java.util.List; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.Lists; import org.apache.commons.lang3.concurrent.BasicThreadFactory; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileChecksum; @@ -35,6 +38,12 @@ import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.PathFilter; import org.apache.hadoop.fs.Trash; +import org.apache.hadoop.fs.permission.AclEntry; +import static org.apache.hadoop.fs.permission.AclEntryScope.ACCESS; +import static org.apache.hadoop.fs.permission.AclEntryType.GROUP; +import static org.apache.hadoop.fs.permission.AclEntryType.USER; +import static org.apache.hadoop.fs.permission.AclEntryType.OTHER; +import org.apache.hadoop.fs.permission.FsAction; import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.hdfs.DFSConfigKeys; import org.apache.hadoop.hive.metastore.api.Database; @@ -595,6 +604,42 @@ public Void execute() throws IOException { if (!cmFs.exists(cmroot)) { cmFs.mkdirs(cmroot); cmFs.setPermission(cmroot, new FsPermission("700")); + //add additional acls to ensure that owner and group of warehouse have access. + try { + FileStatus warehouseStatus = cmFs.getFileStatus(new Path(MetastoreConf.get(conf, ConfVars.WAREHOUSE.getVarname()))); + if (warehouseStatus != null) { + List requiredACLs = new ArrayList<>(); + String warehouseOwner = warehouseStatus.getOwner(); + if (!warehouseOwner.equals(cmFs.getFileStatus(cmroot).getOwner())) { + FsAction whOwnerAction = warehouseStatus.getPermission().getUserAction(); + requiredACLs.add(new AclEntry.Builder().setScope(ACCESS).setType(USER).setName(warehouseOwner). + setPermission(whOwnerAction).build()); + } + + String warehouseGroup = warehouseStatus.getGroup(); + FsAction cmRootGroupAction = FsAction.NONE; + + if (warehouseGroup.equals(cmFs.getFileStatus(cmroot).getGroup())) { + cmRootGroupAction = warehouseStatus.getPermission().getGroupAction(); + } else { + FsAction whGroupAction = warehouseStatus.getPermission().getGroupAction(); + requiredACLs.add(new AclEntry.Builder().setScope(ACCESS).setType(GROUP).setName(warehouseGroup). + setPermission(whGroupAction).build()); + } + + //acl api requires ugo permisssion entries also + List aclList = Lists.newArrayList( + new AclEntry.Builder().setScope(ACCESS).setType(USER).setPermission(FsAction.ALL).build(), + new AclEntry.Builder().setScope(ACCESS).setType(GROUP).setPermission(cmRootGroupAction).build(), + new AclEntry.Builder().setScope(ACCESS).setType(OTHER).setPermission(FsAction.NONE).build()); + aclList.addAll(requiredACLs); + //if requiredACLs is empty, then the following call is a simple permission modification + cmFs.setAcl(cmroot, aclList); + } + } catch (Exception e) { + LOG.error("Unable to set ACLs corresponding to hive warehouse on CMRoot: ", e); + cmFs.setPermission(cmroot, new FsPermission("770")); + } } return null; }