From 2beaee737aee879dd5e83c1b1d04932a13ca6875 Mon Sep 17 00:00:00 2001 From: Viraj Jasani Date: Mon, 25 Sep 2023 22:50:56 -0700 Subject: [PATCH 01/10] HBASE-28081 De-flake TestSnapshotScannerHDFSAclController#testModifyTable1 --- .../TestSnapshotScannerHDFSAclController.java | 63 ----- ...TestSnapshotScannerHDFSAclController3.java | 218 ++++++++++++++++++ 2 files changed, 218 insertions(+), 63 deletions(-) create mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController3.java 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 99d5a89ac2c9..63c19d49a944 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 @@ -40,7 +40,6 @@ import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.Admin; import org.apache.hadoop.hbase.client.Table; -import org.apache.hadoop.hbase.client.TableDescriptor; import org.apache.hadoop.hbase.client.TableDescriptorBuilder; import org.apache.hadoop.hbase.coprocessor.CoprocessorHost; import org.apache.hadoop.hbase.master.cleaner.HFileCleaner; @@ -751,68 +750,6 @@ public void testCleanArchiveTableDir() throws Exception { deleteTable(table); } - @Test - public void testModifyTable1() throws Exception { - String namespace = name.getMethodName(); - TableName table = TableName.valueOf(namespace, name.getMethodName()); - String snapshot = namespace + "t1"; - - String tableUserName = name.getMethodName(); - User tableUser = User.createUserForTesting(conf, tableUserName, new String[] {}); - String tableUserName2 = tableUserName + "2"; - User tableUser2 = User.createUserForTesting(conf, tableUserName2, new String[] {}); - String tableUserName3 = tableUserName + "3"; - User tableUser3 = User.createUserForTesting(conf, tableUserName3, new String[] {}); - String nsUserName = tableUserName + "-ns"; - User nsUser = User.createUserForTesting(conf, nsUserName, new String[] {}); - String globalUserName = tableUserName + "-global"; - User globalUser = User.createUserForTesting(conf, globalUserName, new String[] {}); - String globalUserName2 = tableUserName + "-global-2"; - User globalUser2 = User.createUserForTesting(conf, globalUserName2, new String[] {}); - - SecureTestUtil.grantGlobal(TEST_UTIL, globalUserName, READ); - TestHDFSAclHelper.createNamespace(TEST_UTIL, namespace); - SecureTestUtil.grantOnNamespace(TEST_UTIL, nsUserName, namespace, READ); - TableDescriptor td = TestHDFSAclHelper.createUserScanSnapshotDisabledTable(TEST_UTIL, table); - snapshotAndWait(snapshot, table); - SecureTestUtil.grantGlobal(TEST_UTIL, globalUserName2, READ); - TestHDFSAclHelper.grantOnTable(TEST_UTIL, tableUserName, table, READ); - SecureTestUtil.grantOnTable(TEST_UTIL, tableUserName2, table, TestHDFSAclHelper.COLUMN1, null, - READ); - TestHDFSAclHelper.grantOnTable(TEST_UTIL, tableUserName3, table, WRITE); - - TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, tableUser, snapshot, -1); - TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, tableUser2, snapshot, -1); - TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, tableUser3, snapshot, -1); - TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, nsUser, snapshot, -1); - // Global permission is set before table is created, the acl is inherited - TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, globalUser, snapshot, 6); - // Global permission is set after table is created, the table dir acl is skip - TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, globalUser2, snapshot, -1); - - // enable user scan snapshot - admin.modifyTable(TableDescriptorBuilder.newBuilder(td) - .setValue(SnapshotScannerHDFSAclHelper.ACL_SYNC_TO_HDFS_ENABLE, "true").build()); - // check scan snapshot - TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, tableUser, snapshot, 6); - TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, tableUser2, snapshot, -1); - TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, tableUser3, snapshot, -1); - TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, nsUser, snapshot, 6); - TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, globalUser, snapshot, 6); - // check acl table storage and ACLs in dirs - assertTrue(hasUserGlobalHdfsAcl(aclTable, globalUserName)); - checkUserAclEntry(FS, helper.getGlobalRootPaths(), globalUserName, true, true); - assertTrue(hasUserNamespaceHdfsAcl(aclTable, nsUserName, namespace)); - checkUserAclEntry(FS, helper.getNamespaceRootPaths(namespace), nsUserName, true, true); - assertTrue(hasUserTableHdfsAcl(aclTable, tableUserName, table)); - checkUserAclEntry(FS, helper.getTableRootPaths(table, false), tableUserName, true, true); - for (String user : new String[] { tableUserName2, tableUserName3 }) { - assertFalse(hasUserTableHdfsAcl(aclTable, user, table)); - checkUserAclEntry(FS, helper.getTableRootPaths(table, false), user, false, false); - } - deleteTable(table); - } - @Test public void testModifyTable2() throws Exception { String namespace = name.getMethodName(); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController3.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController3.java new file mode 100644 index 000000000000..42d8a4358874 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController3.java @@ -0,0 +1,218 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.security.access; + +import static org.apache.hadoop.hbase.security.access.Permission.Action.READ; +import static org.apache.hadoop.hbase.security.access.Permission.Action.WRITE; +import static org.apache.hadoop.hbase.security.access.SnapshotScannerHDFSAclController.SnapshotScannerHDFSAclStorage.hasUserGlobalHdfsAcl; +import static org.apache.hadoop.hbase.security.access.SnapshotScannerHDFSAclController.SnapshotScannerHDFSAclStorage.hasUserNamespaceHdfsAcl; +import static org.apache.hadoop.hbase.security.access.SnapshotScannerHDFSAclController.SnapshotScannerHDFSAclStorage.hasUserTableHdfsAcl; +import static org.apache.hadoop.hbase.security.access.TestSnapshotScannerHDFSAclController.checkUserAclEntry; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.io.IOException; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.FileSystem; +import org.apache.hadoop.fs.Path; +import org.apache.hadoop.fs.permission.FsPermission; +import org.apache.hadoop.hbase.HBaseClassTestRule; +import org.apache.hadoop.hbase.HBaseTestingUtil; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.Admin; +import org.apache.hadoop.hbase.client.Table; +import org.apache.hadoop.hbase.client.TableDescriptor; +import org.apache.hadoop.hbase.client.TableDescriptorBuilder; +import org.apache.hadoop.hbase.coprocessor.CoprocessorHost; +import org.apache.hadoop.hbase.security.User; +import org.apache.hadoop.hbase.testclassification.LargeTests; +import org.apache.hadoop.hbase.testclassification.SecurityTests; +import org.apache.hadoop.hbase.util.Threads; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.rules.TestName; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Separated from {@link TestSnapshotScannerHDFSAclController}. Uses facility from that class. + * @see TestSnapshotScannerHDFSAclController + */ +@Category({ SecurityTests.class, LargeTests.class }) +public class TestSnapshotScannerHDFSAclController3 { + + @ClassRule + public static final HBaseClassTestRule CLASS_RULE = + HBaseClassTestRule.forClass(TestSnapshotScannerHDFSAclController3.class); + + @Rule + public TestName name = new TestName(); + private static final Logger LOG = + LoggerFactory.getLogger(TestSnapshotScannerHDFSAclController3.class); + + private static final String UN_GRANT_USER = "un_grant_user"; + private static HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil(); + private static Configuration conf = TEST_UTIL.getConfiguration(); + private static Admin admin = null; + private static SnapshotScannerHDFSAclHelper helper; + private static Table aclTable; + private static FileSystem FS; + + @BeforeClass + public static void setupBeforeClass() throws Exception { + // enable hdfs acl and set umask to 027 + conf.setBoolean("dfs.namenode.acls.enabled", true); + conf.set("fs.permissions.umask-mode", "027"); + // enable hbase hdfs acl feature + conf.setBoolean(SnapshotScannerHDFSAclHelper.ACL_SYNC_TO_HDFS_ENABLE, true); + // enable secure + conf.set(User.HBASE_SECURITY_CONF_KEY, "simple"); + conf.set(SnapshotScannerHDFSAclHelper.SNAPSHOT_RESTORE_TMP_DIR, + SnapshotScannerHDFSAclHelper.SNAPSHOT_RESTORE_TMP_DIR_DEFAULT); + SecureTestUtil.enableSecurity(conf); + // add SnapshotScannerHDFSAclController coprocessor + conf.set(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY, + conf.get(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY) + "," + + SnapshotScannerHDFSAclController.class.getName()); + + TEST_UTIL.startMiniCluster(); + SnapshotScannerHDFSAclController coprocessor = TEST_UTIL.getHBaseCluster().getMaster() + .getMasterCoprocessorHost().findCoprocessor(SnapshotScannerHDFSAclController.class); + TEST_UTIL.waitFor(30000, () -> coprocessor.checkInitialized("check initialized")); + TEST_UTIL.waitTableAvailable(PermissionStorage.ACL_TABLE_NAME); + + admin = TEST_UTIL.getAdmin(); + Path rootDir = TEST_UTIL.getDefaultRootDirPath(); + FS = rootDir.getFileSystem(conf); + User unGrantUser = User.createUserForTesting(conf, UN_GRANT_USER, new String[] {}); + helper = new SnapshotScannerHDFSAclHelper(conf, admin.getConnection()); + + // set hbase directory permission + FsPermission commonDirectoryPermission = + new FsPermission(conf.get(SnapshotScannerHDFSAclHelper.COMMON_DIRECTORY_PERMISSION, + SnapshotScannerHDFSAclHelper.COMMON_DIRECTORY_PERMISSION_DEFAULT)); + Path path = rootDir; + while (path != null) { + FS.setPermission(path, commonDirectoryPermission); + path = path.getParent(); + } + // set restore directory permission + Path restoreDir = new Path(SnapshotScannerHDFSAclHelper.SNAPSHOT_RESTORE_TMP_DIR_DEFAULT); + if (!FS.exists(restoreDir)) { + FS.mkdirs(restoreDir); + FS.setPermission(restoreDir, + new FsPermission( + conf.get(SnapshotScannerHDFSAclHelper.SNAPSHOT_RESTORE_DIRECTORY_PERMISSION, + SnapshotScannerHDFSAclHelper.SNAPSHOT_RESTORE_DIRECTORY_PERMISSION_DEFAULT))); + } + path = restoreDir.getParent(); + while (path != null) { + FS.setPermission(path, commonDirectoryPermission); + path = path.getParent(); + } + aclTable = admin.getConnection().getTable(PermissionStorage.ACL_TABLE_NAME); + } + + @AfterClass + public static void tearDownAfterClass() throws Exception { + TEST_UTIL.shutdownMiniCluster(); + } + + @Test + public void testModifyTable() throws Exception { + String namespace = name.getMethodName(); + TableName table = TableName.valueOf(namespace, name.getMethodName()); + String snapshot = namespace + "t1"; + + String tableUserName = name.getMethodName(); + User tableUser = User.createUserForTesting(conf, tableUserName, new String[] {}); + String tableUserName2 = tableUserName + "2"; + User tableUser2 = User.createUserForTesting(conf, tableUserName2, new String[] {}); + String tableUserName3 = tableUserName + "3"; + User tableUser3 = User.createUserForTesting(conf, tableUserName3, new String[] {}); + String nsUserName = tableUserName + "-ns"; + User nsUser = User.createUserForTesting(conf, nsUserName, new String[] {}); + String globalUserName = tableUserName + "-global"; + User globalUser = User.createUserForTesting(conf, globalUserName, new String[] {}); + String globalUserName2 = tableUserName + "-global-2"; + User globalUser2 = User.createUserForTesting(conf, globalUserName2, new String[] {}); + + SecureTestUtil.grantGlobal(TEST_UTIL, globalUserName, READ); + TestHDFSAclHelper.createNamespace(TEST_UTIL, namespace); + SecureTestUtil.grantOnNamespace(TEST_UTIL, nsUserName, namespace, READ); + TableDescriptor td = TestHDFSAclHelper.createUserScanSnapshotDisabledTable(TEST_UTIL, table); + snapshotAndWait(snapshot, table); + SecureTestUtil.grantGlobal(TEST_UTIL, globalUserName2, READ); + TestHDFSAclHelper.grantOnTable(TEST_UTIL, tableUserName, table, READ); + SecureTestUtil.grantOnTable(TEST_UTIL, tableUserName2, table, TestHDFSAclHelper.COLUMN1, null, + READ); + TestHDFSAclHelper.grantOnTable(TEST_UTIL, tableUserName3, table, WRITE); + + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, tableUser, snapshot, -1); + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, tableUser2, snapshot, -1); + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, tableUser3, snapshot, -1); + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, nsUser, snapshot, -1); + // Global permission is set before table is created, the acl is inherited + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, globalUser, snapshot, 6); + // Global permission is set after table is created, the table dir acl is skip + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, globalUser2, snapshot, -1); + + // enable user scan snapshot + admin.modifyTable(TableDescriptorBuilder.newBuilder(td) + .setValue(SnapshotScannerHDFSAclHelper.ACL_SYNC_TO_HDFS_ENABLE, "true").build()); + // check scan snapshot + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, tableUser, snapshot, 6); + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, tableUser2, snapshot, -1); + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, tableUser3, snapshot, -1); + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, nsUser, snapshot, 6); + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, globalUser, snapshot, 6); + // check acl table storage and ACLs in dirs + assertTrue(hasUserGlobalHdfsAcl(aclTable, globalUserName)); + checkUserAclEntry(FS, helper.getGlobalRootPaths(), globalUserName, true, true); + assertTrue(hasUserNamespaceHdfsAcl(aclTable, nsUserName, namespace)); + checkUserAclEntry(FS, helper.getNamespaceRootPaths(namespace), nsUserName, true, true); + assertTrue(hasUserTableHdfsAcl(aclTable, tableUserName, table)); + checkUserAclEntry(FS, helper.getTableRootPaths(table, false), tableUserName, true, true); + for (String user : new String[] { tableUserName2, tableUserName3 }) { + assertFalse(hasUserTableHdfsAcl(aclTable, user, table)); + checkUserAclEntry(FS, helper.getTableRootPaths(table, false), user, false, false); + } + deleteTable(table); + } + + private static void deleteTable(TableName tableName) { + try { + admin.disableTable(tableName); + admin.deleteTable(tableName); + } catch (IOException e) { + LOG.warn("Failed to delete table: {}", tableName); + } + } + + private void snapshotAndWait(final String snapShotName, final TableName tableName) + throws Exception { + admin.snapshot(snapShotName, tableName); + LOG.info("Sleep for three seconds, waiting for HDFS Acl setup"); + Threads.sleep(3000); + } + +} From 6b156330fc59e37e78f27f3fd14575bfd0a6706b Mon Sep 17 00:00:00 2001 From: Viraj Jasani Date: Tue, 26 Sep 2023 19:30:19 -0700 Subject: [PATCH 02/10] Revert "HBASE-28081 De-flake TestSnapshotScannerHDFSAclController#testModifyTable1" This reverts commit 2beaee737aee879dd5e83c1b1d04932a13ca6875. --- .../TestSnapshotScannerHDFSAclController.java | 63 +++++ ...TestSnapshotScannerHDFSAclController3.java | 218 ------------------ 2 files changed, 63 insertions(+), 218 deletions(-) delete mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController3.java 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 63c19d49a944..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 @@ -40,6 +40,7 @@ import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.Admin; import org.apache.hadoop.hbase.client.Table; +import org.apache.hadoop.hbase.client.TableDescriptor; import org.apache.hadoop.hbase.client.TableDescriptorBuilder; import org.apache.hadoop.hbase.coprocessor.CoprocessorHost; import org.apache.hadoop.hbase.master.cleaner.HFileCleaner; @@ -750,6 +751,68 @@ public void testCleanArchiveTableDir() throws Exception { deleteTable(table); } + @Test + public void testModifyTable1() throws Exception { + String namespace = name.getMethodName(); + TableName table = TableName.valueOf(namespace, name.getMethodName()); + String snapshot = namespace + "t1"; + + String tableUserName = name.getMethodName(); + User tableUser = User.createUserForTesting(conf, tableUserName, new String[] {}); + String tableUserName2 = tableUserName + "2"; + User tableUser2 = User.createUserForTesting(conf, tableUserName2, new String[] {}); + String tableUserName3 = tableUserName + "3"; + User tableUser3 = User.createUserForTesting(conf, tableUserName3, new String[] {}); + String nsUserName = tableUserName + "-ns"; + User nsUser = User.createUserForTesting(conf, nsUserName, new String[] {}); + String globalUserName = tableUserName + "-global"; + User globalUser = User.createUserForTesting(conf, globalUserName, new String[] {}); + String globalUserName2 = tableUserName + "-global-2"; + User globalUser2 = User.createUserForTesting(conf, globalUserName2, new String[] {}); + + SecureTestUtil.grantGlobal(TEST_UTIL, globalUserName, READ); + TestHDFSAclHelper.createNamespace(TEST_UTIL, namespace); + SecureTestUtil.grantOnNamespace(TEST_UTIL, nsUserName, namespace, READ); + TableDescriptor td = TestHDFSAclHelper.createUserScanSnapshotDisabledTable(TEST_UTIL, table); + snapshotAndWait(snapshot, table); + SecureTestUtil.grantGlobal(TEST_UTIL, globalUserName2, READ); + TestHDFSAclHelper.grantOnTable(TEST_UTIL, tableUserName, table, READ); + SecureTestUtil.grantOnTable(TEST_UTIL, tableUserName2, table, TestHDFSAclHelper.COLUMN1, null, + READ); + TestHDFSAclHelper.grantOnTable(TEST_UTIL, tableUserName3, table, WRITE); + + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, tableUser, snapshot, -1); + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, tableUser2, snapshot, -1); + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, tableUser3, snapshot, -1); + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, nsUser, snapshot, -1); + // Global permission is set before table is created, the acl is inherited + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, globalUser, snapshot, 6); + // Global permission is set after table is created, the table dir acl is skip + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, globalUser2, snapshot, -1); + + // enable user scan snapshot + admin.modifyTable(TableDescriptorBuilder.newBuilder(td) + .setValue(SnapshotScannerHDFSAclHelper.ACL_SYNC_TO_HDFS_ENABLE, "true").build()); + // check scan snapshot + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, tableUser, snapshot, 6); + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, tableUser2, snapshot, -1); + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, tableUser3, snapshot, -1); + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, nsUser, snapshot, 6); + TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, globalUser, snapshot, 6); + // check acl table storage and ACLs in dirs + assertTrue(hasUserGlobalHdfsAcl(aclTable, globalUserName)); + checkUserAclEntry(FS, helper.getGlobalRootPaths(), globalUserName, true, true); + assertTrue(hasUserNamespaceHdfsAcl(aclTable, nsUserName, namespace)); + checkUserAclEntry(FS, helper.getNamespaceRootPaths(namespace), nsUserName, true, true); + assertTrue(hasUserTableHdfsAcl(aclTable, tableUserName, table)); + checkUserAclEntry(FS, helper.getTableRootPaths(table, false), tableUserName, true, true); + for (String user : new String[] { tableUserName2, tableUserName3 }) { + assertFalse(hasUserTableHdfsAcl(aclTable, user, table)); + checkUserAclEntry(FS, helper.getTableRootPaths(table, false), user, false, false); + } + deleteTable(table); + } + @Test public void testModifyTable2() throws Exception { String namespace = name.getMethodName(); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController3.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController3.java deleted file mode 100644 index 42d8a4358874..000000000000 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController3.java +++ /dev/null @@ -1,218 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.hadoop.hbase.security.access; - -import static org.apache.hadoop.hbase.security.access.Permission.Action.READ; -import static org.apache.hadoop.hbase.security.access.Permission.Action.WRITE; -import static org.apache.hadoop.hbase.security.access.SnapshotScannerHDFSAclController.SnapshotScannerHDFSAclStorage.hasUserGlobalHdfsAcl; -import static org.apache.hadoop.hbase.security.access.SnapshotScannerHDFSAclController.SnapshotScannerHDFSAclStorage.hasUserNamespaceHdfsAcl; -import static org.apache.hadoop.hbase.security.access.SnapshotScannerHDFSAclController.SnapshotScannerHDFSAclStorage.hasUserTableHdfsAcl; -import static org.apache.hadoop.hbase.security.access.TestSnapshotScannerHDFSAclController.checkUserAclEntry; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; - -import java.io.IOException; -import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.fs.FileSystem; -import org.apache.hadoop.fs.Path; -import org.apache.hadoop.fs.permission.FsPermission; -import org.apache.hadoop.hbase.HBaseClassTestRule; -import org.apache.hadoop.hbase.HBaseTestingUtil; -import org.apache.hadoop.hbase.TableName; -import org.apache.hadoop.hbase.client.Admin; -import org.apache.hadoop.hbase.client.Table; -import org.apache.hadoop.hbase.client.TableDescriptor; -import org.apache.hadoop.hbase.client.TableDescriptorBuilder; -import org.apache.hadoop.hbase.coprocessor.CoprocessorHost; -import org.apache.hadoop.hbase.security.User; -import org.apache.hadoop.hbase.testclassification.LargeTests; -import org.apache.hadoop.hbase.testclassification.SecurityTests; -import org.apache.hadoop.hbase.util.Threads; -import org.junit.AfterClass; -import org.junit.BeforeClass; -import org.junit.ClassRule; -import org.junit.Rule; -import org.junit.Test; -import org.junit.experimental.categories.Category; -import org.junit.rules.TestName; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * Separated from {@link TestSnapshotScannerHDFSAclController}. Uses facility from that class. - * @see TestSnapshotScannerHDFSAclController - */ -@Category({ SecurityTests.class, LargeTests.class }) -public class TestSnapshotScannerHDFSAclController3 { - - @ClassRule - public static final HBaseClassTestRule CLASS_RULE = - HBaseClassTestRule.forClass(TestSnapshotScannerHDFSAclController3.class); - - @Rule - public TestName name = new TestName(); - private static final Logger LOG = - LoggerFactory.getLogger(TestSnapshotScannerHDFSAclController3.class); - - private static final String UN_GRANT_USER = "un_grant_user"; - private static HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil(); - private static Configuration conf = TEST_UTIL.getConfiguration(); - private static Admin admin = null; - private static SnapshotScannerHDFSAclHelper helper; - private static Table aclTable; - private static FileSystem FS; - - @BeforeClass - public static void setupBeforeClass() throws Exception { - // enable hdfs acl and set umask to 027 - conf.setBoolean("dfs.namenode.acls.enabled", true); - conf.set("fs.permissions.umask-mode", "027"); - // enable hbase hdfs acl feature - conf.setBoolean(SnapshotScannerHDFSAclHelper.ACL_SYNC_TO_HDFS_ENABLE, true); - // enable secure - conf.set(User.HBASE_SECURITY_CONF_KEY, "simple"); - conf.set(SnapshotScannerHDFSAclHelper.SNAPSHOT_RESTORE_TMP_DIR, - SnapshotScannerHDFSAclHelper.SNAPSHOT_RESTORE_TMP_DIR_DEFAULT); - SecureTestUtil.enableSecurity(conf); - // add SnapshotScannerHDFSAclController coprocessor - conf.set(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY, - conf.get(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY) + "," - + SnapshotScannerHDFSAclController.class.getName()); - - TEST_UTIL.startMiniCluster(); - SnapshotScannerHDFSAclController coprocessor = TEST_UTIL.getHBaseCluster().getMaster() - .getMasterCoprocessorHost().findCoprocessor(SnapshotScannerHDFSAclController.class); - TEST_UTIL.waitFor(30000, () -> coprocessor.checkInitialized("check initialized")); - TEST_UTIL.waitTableAvailable(PermissionStorage.ACL_TABLE_NAME); - - admin = TEST_UTIL.getAdmin(); - Path rootDir = TEST_UTIL.getDefaultRootDirPath(); - FS = rootDir.getFileSystem(conf); - User unGrantUser = User.createUserForTesting(conf, UN_GRANT_USER, new String[] {}); - helper = new SnapshotScannerHDFSAclHelper(conf, admin.getConnection()); - - // set hbase directory permission - FsPermission commonDirectoryPermission = - new FsPermission(conf.get(SnapshotScannerHDFSAclHelper.COMMON_DIRECTORY_PERMISSION, - SnapshotScannerHDFSAclHelper.COMMON_DIRECTORY_PERMISSION_DEFAULT)); - Path path = rootDir; - while (path != null) { - FS.setPermission(path, commonDirectoryPermission); - path = path.getParent(); - } - // set restore directory permission - Path restoreDir = new Path(SnapshotScannerHDFSAclHelper.SNAPSHOT_RESTORE_TMP_DIR_DEFAULT); - if (!FS.exists(restoreDir)) { - FS.mkdirs(restoreDir); - FS.setPermission(restoreDir, - new FsPermission( - conf.get(SnapshotScannerHDFSAclHelper.SNAPSHOT_RESTORE_DIRECTORY_PERMISSION, - SnapshotScannerHDFSAclHelper.SNAPSHOT_RESTORE_DIRECTORY_PERMISSION_DEFAULT))); - } - path = restoreDir.getParent(); - while (path != null) { - FS.setPermission(path, commonDirectoryPermission); - path = path.getParent(); - } - aclTable = admin.getConnection().getTable(PermissionStorage.ACL_TABLE_NAME); - } - - @AfterClass - public static void tearDownAfterClass() throws Exception { - TEST_UTIL.shutdownMiniCluster(); - } - - @Test - public void testModifyTable() throws Exception { - String namespace = name.getMethodName(); - TableName table = TableName.valueOf(namespace, name.getMethodName()); - String snapshot = namespace + "t1"; - - String tableUserName = name.getMethodName(); - User tableUser = User.createUserForTesting(conf, tableUserName, new String[] {}); - String tableUserName2 = tableUserName + "2"; - User tableUser2 = User.createUserForTesting(conf, tableUserName2, new String[] {}); - String tableUserName3 = tableUserName + "3"; - User tableUser3 = User.createUserForTesting(conf, tableUserName3, new String[] {}); - String nsUserName = tableUserName + "-ns"; - User nsUser = User.createUserForTesting(conf, nsUserName, new String[] {}); - String globalUserName = tableUserName + "-global"; - User globalUser = User.createUserForTesting(conf, globalUserName, new String[] {}); - String globalUserName2 = tableUserName + "-global-2"; - User globalUser2 = User.createUserForTesting(conf, globalUserName2, new String[] {}); - - SecureTestUtil.grantGlobal(TEST_UTIL, globalUserName, READ); - TestHDFSAclHelper.createNamespace(TEST_UTIL, namespace); - SecureTestUtil.grantOnNamespace(TEST_UTIL, nsUserName, namespace, READ); - TableDescriptor td = TestHDFSAclHelper.createUserScanSnapshotDisabledTable(TEST_UTIL, table); - snapshotAndWait(snapshot, table); - SecureTestUtil.grantGlobal(TEST_UTIL, globalUserName2, READ); - TestHDFSAclHelper.grantOnTable(TEST_UTIL, tableUserName, table, READ); - SecureTestUtil.grantOnTable(TEST_UTIL, tableUserName2, table, TestHDFSAclHelper.COLUMN1, null, - READ); - TestHDFSAclHelper.grantOnTable(TEST_UTIL, tableUserName3, table, WRITE); - - TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, tableUser, snapshot, -1); - TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, tableUser2, snapshot, -1); - TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, tableUser3, snapshot, -1); - TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, nsUser, snapshot, -1); - // Global permission is set before table is created, the acl is inherited - TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, globalUser, snapshot, 6); - // Global permission is set after table is created, the table dir acl is skip - TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, globalUser2, snapshot, -1); - - // enable user scan snapshot - admin.modifyTable(TableDescriptorBuilder.newBuilder(td) - .setValue(SnapshotScannerHDFSAclHelper.ACL_SYNC_TO_HDFS_ENABLE, "true").build()); - // check scan snapshot - TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, tableUser, snapshot, 6); - TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, tableUser2, snapshot, -1); - TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, tableUser3, snapshot, -1); - TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, nsUser, snapshot, 6); - TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, globalUser, snapshot, 6); - // check acl table storage and ACLs in dirs - assertTrue(hasUserGlobalHdfsAcl(aclTable, globalUserName)); - checkUserAclEntry(FS, helper.getGlobalRootPaths(), globalUserName, true, true); - assertTrue(hasUserNamespaceHdfsAcl(aclTable, nsUserName, namespace)); - checkUserAclEntry(FS, helper.getNamespaceRootPaths(namespace), nsUserName, true, true); - assertTrue(hasUserTableHdfsAcl(aclTable, tableUserName, table)); - checkUserAclEntry(FS, helper.getTableRootPaths(table, false), tableUserName, true, true); - for (String user : new String[] { tableUserName2, tableUserName3 }) { - assertFalse(hasUserTableHdfsAcl(aclTable, user, table)); - checkUserAclEntry(FS, helper.getTableRootPaths(table, false), user, false, false); - } - deleteTable(table); - } - - private static void deleteTable(TableName tableName) { - try { - admin.disableTable(tableName); - admin.deleteTable(tableName); - } catch (IOException e) { - LOG.warn("Failed to delete table: {}", tableName); - } - } - - private void snapshotAndWait(final String snapShotName, final TableName tableName) - throws Exception { - admin.snapshot(snapShotName, tableName); - LOG.info("Sleep for three seconds, waiting for HDFS Acl setup"); - Threads.sleep(3000); - } - -} From 205fd62b8d3a9a200a0988384497c540cf17e875 Mon Sep 17 00:00:00 2001 From: Viraj Jasani Date: Tue, 26 Sep 2023 22:26:10 -0700 Subject: [PATCH 03/10] addendum --- .../master/snapshot/SnapshotManager.java | 26 +++++++++++++++++++ .../access/SnapshotScannerHDFSAclHelper.java | 14 +++++----- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java index 3c421dd8bd01..9af1cd39a8d7 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java @@ -42,6 +42,8 @@ import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.fs.permission.AclEntry; +import org.apache.hadoop.fs.permission.AclStatus; import org.apache.hadoop.hbase.HBaseInterfaceAudience; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.ServerName; @@ -564,6 +566,7 @@ public synchronized void prepareWorkingDirectory(SnapshotDescription snapshot) "Couldn't create working directory (" + workingDir + ") for snapshot", ProtobufUtil.createSnapshotDesc(snapshot)); } + updateWorkingDirAclsIfRequired(workingDir, workingDirFS); } catch (HBaseSnapshotException e) { throw e; } catch (IOException e) { @@ -573,6 +576,29 @@ public synchronized void prepareWorkingDirectory(SnapshotDescription snapshot) } } + /** + * If the parent dir of the snapshot working dir (e.g. /hbase/.hbase-snapshot) has non-empty + * ACLs, use them for the current working dir (e.g. /hbase/.hbase-snapshot/.tmp/{snapshot-name}) + * so that regardless of whether the snapshot commit phase performs atomic rename or non-atomic + * copy of the working dir to new snapshot dir, the ACLs are retained. + * + * @param workingDir working dir to build the snapshot. + * @param workingDirFS working dir file system. + * @throws IOException If ACL read/modify operation fails. + */ + private static void updateWorkingDirAclsIfRequired(Path workingDir, FileSystem workingDirFS) + throws IOException { + AclStatus snapshotWorkingParentDirStatus = + workingDirFS.getAclStatus(workingDir.getParent().getParent()); + List snapshotWorkingParentDirAclStatusEntries = + snapshotWorkingParentDirStatus.getEntries(); + if (snapshotWorkingParentDirAclStatusEntries != null + && snapshotWorkingParentDirAclStatusEntries.size() > 0) { + workingDirFS.modifyAclEntries(workingDir, + snapshotWorkingParentDirAclStatusEntries); + } + } + /** * Take a snapshot of a disabled table. * @param snapshot description of the snapshot to take. Modified to be {@link Type#DISABLED}. diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SnapshotScannerHDFSAclHelper.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SnapshotScannerHDFSAclHelper.java index 41f61a6efa33..0fd41e4748df 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SnapshotScannerHDFSAclHelper.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SnapshotScannerHDFSAclHelper.java @@ -152,11 +152,12 @@ public boolean grantAcl(UserPermission userPermission, Set skipNamespace long start = EnvironmentEdgeManager.currentTime(); handleGrantOrRevokeAcl(userPermission, HDFSAclOperation.OperationType.MODIFY, skipNamespaces, skipTables); - LOG.info("Set HDFS acl when grant {}, cost {} ms", userPermission, - EnvironmentEdgeManager.currentTime() - start); + LOG.info("Set HDFS acl when grant {}, skipNamespaces: {}, skipTables: {}, cost {} ms", + userPermission, skipNamespaces, skipTables, EnvironmentEdgeManager.currentTime() - start); return true; } catch (Exception e) { - LOG.error("Set HDFS acl error when grant: {}", userPermission, e); + LOG.error("Set HDFS acl error when grant: {}, skipNamespaces: {}, skipTables: {}", + userPermission, skipNamespaces, skipTables, e); return false; } } @@ -174,11 +175,12 @@ public boolean revokeAcl(UserPermission userPermission, Set skipNamespac long start = EnvironmentEdgeManager.currentTime(); handleGrantOrRevokeAcl(userPermission, HDFSAclOperation.OperationType.REMOVE, skipNamespaces, skipTables); - LOG.info("Set HDFS acl when revoke {}, cost {} ms", userPermission, - EnvironmentEdgeManager.currentTime() - start); + LOG.info("Set HDFS acl when revoke {}, skipNamespaces: {}, skipTables: {}, cost {} ms", + userPermission, skipNamespaces, skipTables, EnvironmentEdgeManager.currentTime() - start); return true; } catch (Exception e) { - LOG.error("Set HDFS acl error when revoke: {}", userPermission, e); + LOG.error("Set HDFS acl error when revoke: {}, skipNamespaces: {}, skipTables: {}", + userPermission, skipNamespaces, skipTables, e); return false; } } From 82b5023b8eb11279bcbb7d863ffaaa59015324c7 Mon Sep 17 00:00:00 2001 From: Viraj Jasani Date: Tue, 26 Sep 2023 23:01:02 -0700 Subject: [PATCH 04/10] addendum --- .../TestSnapshotScannerHDFSAclController.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) 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 99d5a89ac2c9..d79e3f308104 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,7 +158,6 @@ 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); @@ -175,6 +174,8 @@ 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); @@ -196,10 +197,10 @@ public void testGrantGlobal2() throws Exception { // create table in namespace1 and snapshot TestHDFSAclHelper.createTableAndPut(TEST_UTIL, table1); snapshotAndWait(snapshot1, table1); - // grant G(W) - SecureTestUtil.grantGlobal(TEST_UTIL, grantUserName, WRITE); admin.grant(new UserPermission(grantUserName, Permission.newBuilder(namespace1).withActions(READ).build()), false); + // grant G(W) + SecureTestUtil.grantGlobal(TEST_UTIL, grantUserName, WRITE); // create table in namespace2 and snapshot TestHDFSAclHelper.createTableAndPut(TEST_UTIL, table2); snapshotAndWait(snapshot2, table2); @@ -230,11 +231,11 @@ public void testGrantGlobal3() throws Exception { // grant table1(R) TestHDFSAclHelper.createTableAndPut(TEST_UTIL, table1); snapshotAndWait(snapshot1, table1); - TestHDFSAclHelper.createTableAndPut(TEST_UTIL, table2); - snapshotAndWait(snapshot2, table2); + TestHDFSAclHelper.grantOnTable(TEST_UTIL, grantUserName, table1, READ); // grant G(W) SecureTestUtil.grantGlobal(TEST_UTIL, grantUserName, WRITE); - TestHDFSAclHelper.grantOnTable(TEST_UTIL, grantUserName, table1, READ); + TestHDFSAclHelper.createTableAndPut(TEST_UTIL, table2); + snapshotAndWait(snapshot2, table2); // check scan snapshot TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot1, 6); TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot2, -1); From a142fc85bfe06cc81c02545028e46dc164994dbd Mon Sep 17 00:00:00 2001 From: Viraj Jasani Date: Tue, 26 Sep 2023 23:03:40 -0700 Subject: [PATCH 05/10] addendum - spotless --- .../master/snapshot/SnapshotManager.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java index 9af1cd39a8d7..88c90c910429 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java @@ -577,12 +577,11 @@ public synchronized void prepareWorkingDirectory(SnapshotDescription snapshot) } /** - * If the parent dir of the snapshot working dir (e.g. /hbase/.hbase-snapshot) has non-empty - * ACLs, use them for the current working dir (e.g. /hbase/.hbase-snapshot/.tmp/{snapshot-name}) - * so that regardless of whether the snapshot commit phase performs atomic rename or non-atomic - * copy of the working dir to new snapshot dir, the ACLs are retained. - * - * @param workingDir working dir to build the snapshot. + * If the parent dir of the snapshot working dir (e.g. /hbase/.hbase-snapshot) has non-empty ACLs, + * use them for the current working dir (e.g. /hbase/.hbase-snapshot/.tmp/{snapshot-name}) so that + * regardless of whether the snapshot commit phase performs atomic rename or non-atomic copy of + * the working dir to new snapshot dir, the ACLs are retained. + * @param workingDir working dir to build the snapshot. * @param workingDirFS working dir file system. * @throws IOException If ACL read/modify operation fails. */ @@ -592,10 +591,11 @@ private static void updateWorkingDirAclsIfRequired(Path workingDir, FileSystem w workingDirFS.getAclStatus(workingDir.getParent().getParent()); List snapshotWorkingParentDirAclStatusEntries = snapshotWorkingParentDirStatus.getEntries(); - if (snapshotWorkingParentDirAclStatusEntries != null - && snapshotWorkingParentDirAclStatusEntries.size() > 0) { - workingDirFS.modifyAclEntries(workingDir, - snapshotWorkingParentDirAclStatusEntries); + if ( + snapshotWorkingParentDirAclStatusEntries != null + && snapshotWorkingParentDirAclStatusEntries.size() > 0 + ) { + workingDirFS.modifyAclEntries(workingDir, snapshotWorkingParentDirAclStatusEntries); } } From 21e8029efc5a196d88a24f4a10d3db930639fa52 Mon Sep 17 00:00:00 2001 From: Viraj Jasani Date: Wed, 27 Sep 2023 09:16:38 -0700 Subject: [PATCH 06/10] addendum - handle disabled ACL support --- .../hadoop/hbase/master/snapshot/SnapshotManager.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java index 88c90c910429..d777c00b40ff 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java @@ -99,6 +99,7 @@ import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; import org.apache.hadoop.hbase.util.NonceKey; import org.apache.hadoop.hbase.util.TableDescriptorChecker; +import org.apache.hadoop.hdfs.protocol.AclException; import org.apache.yetus.audience.InterfaceAudience; import org.apache.yetus.audience.InterfaceStability; import org.apache.zookeeper.KeeperException; @@ -587,8 +588,14 @@ public synchronized void prepareWorkingDirectory(SnapshotDescription snapshot) */ private static void updateWorkingDirAclsIfRequired(Path workingDir, FileSystem workingDirFS) throws IOException { - AclStatus snapshotWorkingParentDirStatus = - workingDirFS.getAclStatus(workingDir.getParent().getParent()); + AclStatus snapshotWorkingParentDirStatus; + try { + snapshotWorkingParentDirStatus = + workingDirFS.getAclStatus(workingDir.getParent().getParent()); + } catch (AclException e) { + LOG.warn("Unable to retrieve ACL status for path: {}", workingDir.getParent().getParent(), e); + return; + } List snapshotWorkingParentDirAclStatusEntries = snapshotWorkingParentDirStatus.getEntries(); if ( From 82910fdf36b62d5c38036a3b1f707cb3192799cc Mon Sep 17 00:00:00 2001 From: Viraj Jasani Date: Wed, 27 Sep 2023 09:23:03 -0700 Subject: [PATCH 07/10] addendum --- .../hadoop/hbase/master/snapshot/SnapshotManager.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java index d777c00b40ff..94aa76162afc 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java @@ -99,7 +99,6 @@ import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; import org.apache.hadoop.hbase.util.NonceKey; import org.apache.hadoop.hbase.util.TableDescriptorChecker; -import org.apache.hadoop.hdfs.protocol.AclException; import org.apache.yetus.audience.InterfaceAudience; import org.apache.yetus.audience.InterfaceStability; import org.apache.zookeeper.KeeperException; @@ -592,8 +591,9 @@ private static void updateWorkingDirAclsIfRequired(Path workingDir, FileSystem w try { snapshotWorkingParentDirStatus = workingDirFS.getAclStatus(workingDir.getParent().getParent()); - } catch (AclException e) { - LOG.warn("Unable to retrieve ACL status for path: {}", workingDir.getParent().getParent(), e); + } catch (IOException e) { + LOG.warn("Unable to retrieve ACL status for path: {}, current working dir path: {}", + workingDir.getParent().getParent(), workingDir, e); return; } List snapshotWorkingParentDirAclStatusEntries = From f42260aba304e76f2ec1135b9a9cff23fbeb1738 Mon Sep 17 00:00:00 2001 From: Viraj Jasani Date: Wed, 27 Sep 2023 13:10:45 -0700 Subject: [PATCH 08/10] addendum --- .../apache/hadoop/hbase/master/snapshot/SnapshotManager.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java index 94aa76162afc..9d27a10e2169 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java @@ -587,6 +587,9 @@ public synchronized void prepareWorkingDirectory(SnapshotDescription snapshot) */ private static void updateWorkingDirAclsIfRequired(Path workingDir, FileSystem workingDirFS) throws IOException { + if (workingDir.getParent() == null || workingDir.getParent().getParent() == null) { + return; + } AclStatus snapshotWorkingParentDirStatus; try { snapshotWorkingParentDirStatus = From f968613e07edecf0d53386ca2d16ea893af3031c Mon Sep 17 00:00:00 2001 From: Viraj Jasani Date: Wed, 27 Sep 2023 13:29:15 -0700 Subject: [PATCH 09/10] addendum --- .../apache/hadoop/hbase/master/snapshot/SnapshotManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java index 9d27a10e2169..355dfba9ddcb 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java @@ -594,7 +594,7 @@ private static void updateWorkingDirAclsIfRequired(Path workingDir, FileSystem w try { snapshotWorkingParentDirStatus = workingDirFS.getAclStatus(workingDir.getParent().getParent()); - } catch (IOException e) { + } catch (IOException | UnsupportedOperationException e) { LOG.warn("Unable to retrieve ACL status for path: {}, current working dir path: {}", workingDir.getParent().getParent(), workingDir, e); return; From f6096ebd157aa4da3548a77b69f24278d84fc63c Mon Sep 17 00:00:00 2001 From: Viraj Jasani Date: Wed, 27 Sep 2023 15:49:30 -0700 Subject: [PATCH 10/10] addendum --- .../hadoop/hbase/master/snapshot/SnapshotManager.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java index 355dfba9ddcb..ed7ef583ec52 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java @@ -38,6 +38,7 @@ import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.stream.Collectors; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.CommonPathCapabilities; import org.apache.hadoop.fs.FSDataInputStream; import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; @@ -587,14 +588,17 @@ public synchronized void prepareWorkingDirectory(SnapshotDescription snapshot) */ private static void updateWorkingDirAclsIfRequired(Path workingDir, FileSystem workingDirFS) throws IOException { - if (workingDir.getParent() == null || workingDir.getParent().getParent() == null) { + if ( + !workingDirFS.hasPathCapability(workingDir, CommonPathCapabilities.FS_ACLS) + || workingDir.getParent() == null || workingDir.getParent().getParent() == null + ) { return; } AclStatus snapshotWorkingParentDirStatus; try { snapshotWorkingParentDirStatus = workingDirFS.getAclStatus(workingDir.getParent().getParent()); - } catch (IOException | UnsupportedOperationException e) { + } catch (IOException e) { LOG.warn("Unable to retrieve ACL status for path: {}, current working dir path: {}", workingDir.getParent().getParent(), workingDir, e); return;