diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotDescriptionUtils.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotDescriptionUtils.java index 1e2bbb68c41d..689cd89259ba 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotDescriptionUtils.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotDescriptionUtils.java @@ -419,11 +419,9 @@ public static void completeSnapshot(Path snapshotDir, Path workingDir, FileSyste // if this fails URI workingURI = workingDirFs.getUri(); URI rootURI = fs.getUri(); + if ( - (!workingURI.getScheme().equals(rootURI.getScheme()) || workingURI.getAuthority() == null - || !workingURI.getAuthority().equals(rootURI.getAuthority()) - || workingURI.getUserInfo() == null - || !workingURI.getUserInfo().equals(rootURI.getUserInfo()) + (shouldSkipRenameSnapshotDirectories(workingURI, rootURI) || !fs.rename(workingDir, snapshotDir)) && !FileUtil.copy(workingDirFs, workingDir, fs, snapshotDir, true, true, conf) ) { @@ -432,6 +430,37 @@ public static void completeSnapshot(Path snapshotDir, Path workingDir, FileSyste } } + static boolean shouldSkipRenameSnapshotDirectories(URI workingURI, URI rootURI) { + // check scheme, e.g. file, hdfs + if (workingURI.getScheme() == null && rootURI.getScheme() != null) { + return true; + } + if (workingURI.getScheme() != null && !workingURI.getScheme().equals(rootURI.getScheme())) { + return true; + } + + // check Authority, e.g. localhost:port + if (workingURI.getAuthority() == null && rootURI.getAuthority() != null) { + return true; + } + if ( + workingURI.getAuthority() != null && !workingURI.getAuthority().equals(rootURI.getAuthority()) + ) { + return true; + } + + // check UGI/userInfo + if (workingURI.getUserInfo() == null && rootURI.getUserInfo() != null) { + return true; + } + if ( + workingURI.getUserInfo() != null && !workingURI.getUserInfo().equals(rootURI.getUserInfo()) + ) { + return true; + } + return false; + } + /** * Check if the user is this table snapshot's owner * @param snapshot the table snapshot description diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java index d79e3f308104..99d5a89ac2c9 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java @@ -158,6 +158,7 @@ public void testGrantGlobal1() throws Exception { TestHDFSAclHelper.createTableAndPut(TEST_UTIL, table); snapshotAndWait(snapshot1, table); + snapshotAndWait(snapshot2, table); // grant G(R) SecureTestUtil.grantGlobal(TEST_UTIL, grantUserName, READ); TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot1, 6); @@ -174,8 +175,6 @@ public void testGrantGlobal1() throws Exception { // grant G(R) SecureTestUtil.grantGlobal(TEST_UTIL, grantUserName, READ); TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot1, 6); - // take a snapshot and ACLs are inherited automatically - snapshotAndWait(snapshot2, table); TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot2, 6); assertTrue(hasUserGlobalHdfsAcl(aclTable, grantUserName)); deleteTable(table); @@ -197,10 +196,10 @@ public void testGrantGlobal2() throws Exception { // create table in namespace1 and snapshot TestHDFSAclHelper.createTableAndPut(TEST_UTIL, table1); snapshotAndWait(snapshot1, table1); - admin.grant(new UserPermission(grantUserName, - Permission.newBuilder(namespace1).withActions(READ).build()), false); // grant G(W) SecureTestUtil.grantGlobal(TEST_UTIL, grantUserName, WRITE); + admin.grant(new UserPermission(grantUserName, + Permission.newBuilder(namespace1).withActions(READ).build()), false); // create table in namespace2 and snapshot TestHDFSAclHelper.createTableAndPut(TEST_UTIL, table2); snapshotAndWait(snapshot2, table2); @@ -231,11 +230,11 @@ public void testGrantGlobal3() throws Exception { // grant table1(R) TestHDFSAclHelper.createTableAndPut(TEST_UTIL, table1); snapshotAndWait(snapshot1, table1); - TestHDFSAclHelper.grantOnTable(TEST_UTIL, grantUserName, table1, READ); - // grant G(W) - SecureTestUtil.grantGlobal(TEST_UTIL, grantUserName, WRITE); TestHDFSAclHelper.createTableAndPut(TEST_UTIL, table2); snapshotAndWait(snapshot2, table2); + // grant G(W) + SecureTestUtil.grantGlobal(TEST_UTIL, grantUserName, WRITE); + TestHDFSAclHelper.grantOnTable(TEST_UTIL, grantUserName, table1, READ); // check scan snapshot TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot1, 6); TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot2, -1); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestSnapshotDescriptionUtils.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestSnapshotDescriptionUtils.java index 2093baf36075..8e62ef16bbfe 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestSnapshotDescriptionUtils.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestSnapshotDescriptionUtils.java @@ -22,6 +22,7 @@ import static org.junit.Assert.fail; import java.io.IOException; +import java.net.URI; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; @@ -191,4 +192,52 @@ public void testIsWithinWorkingDir() throws IOException { assertTrue(SnapshotDescriptionUtils.isWithinDefaultWorkingDir( new Path("file:" + hbsaeDir + "/.hbase-snapshot/.tmp/snapshot"), conf)); } + + @Test + public void testShouldSkipRenameSnapshotDirectories() { + URI workingDirURI = URI.create("/User/test1"); + URI rootDirURI = URI.create("hdfs:///User/test2"); + + // should skip rename if it's not the same scheme; + assertTrue( + SnapshotDescriptionUtils.shouldSkipRenameSnapshotDirectories(workingDirURI, rootDirURI)); + + workingDirURI = URI.create("/User/test1"); + rootDirURI = URI.create("file:///User/test2"); + assertTrue( + SnapshotDescriptionUtils.shouldSkipRenameSnapshotDirectories(workingDirURI, rootDirURI)); + + // skip rename when either scheme or authority are the not same + workingDirURI = URI.create("hdfs://localhost:8020/User/test1"); + rootDirURI = URI.create("hdfs://otherhost:8020/User/test2"); + assertTrue( + SnapshotDescriptionUtils.shouldSkipRenameSnapshotDirectories(workingDirURI, rootDirURI)); + + workingDirURI = URI.create("file:///User/test1"); + rootDirURI = URI.create("hdfs://localhost:8020/User/test2"); + assertTrue( + SnapshotDescriptionUtils.shouldSkipRenameSnapshotDirectories(workingDirURI, rootDirURI)); + + workingDirURI = URI.create("hdfs:///User/test1"); + rootDirURI = URI.create("hdfs:///User/test2"); + assertFalse( + SnapshotDescriptionUtils.shouldSkipRenameSnapshotDirectories(workingDirURI, rootDirURI)); + + workingDirURI = URI.create("hdfs://localhost:8020/User/test1"); + rootDirURI = URI.create("hdfs://localhost:8020/User/test2"); + assertFalse( + SnapshotDescriptionUtils.shouldSkipRenameSnapshotDirectories(workingDirURI, rootDirURI)); + + workingDirURI = URI.create("hdfs://user:password@localhost:8020/User/test1"); + rootDirURI = URI.create("hdfs://user:password@localhost:8020/User/test2"); + assertFalse( + SnapshotDescriptionUtils.shouldSkipRenameSnapshotDirectories(workingDirURI, rootDirURI)); + + // skip rename when user information is not the same + workingDirURI = URI.create("hdfs://user:password@localhost:8020/User/test1"); + rootDirURI = URI.create("hdfs://user2:password2@localhost:8020/User/test2"); + assertTrue( + SnapshotDescriptionUtils.shouldSkipRenameSnapshotDirectories(workingDirURI, rootDirURI)); + } + }