diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZNodePaths.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZNodePaths.java index 51272a60fae..9e7e51a3513 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZNodePaths.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZNodePaths.java @@ -26,7 +26,6 @@ import static org.apache.hadoop.hbase.client.RegionInfo.DEFAULT_REPLICA_ID; import java.util.Optional; import java.util.stream.IntStream; - import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.yetus.audience.InterfaceAudience; @@ -184,11 +183,23 @@ public class ZNodePaths { } /** - * Join the prefix znode name with the suffix znode name to generate a proper - * full znode name. - * + * Returns whether the znode is supposed to be readable by the client and DOES NOT contain + * sensitive information (world readable). + */ + public boolean isClientReadable(String node) { + // Developer notice: These znodes are world readable. DO NOT add more znodes here UNLESS + // all clients need to access this data to work. Using zk for sharing data to clients (other + // than service lookup case is not a recommended design pattern. + return node.equals(baseZNode) || isAnyMetaReplicaZNode(node) || + node.equals(masterAddressZNode) || node.equals(clusterIdZNode) || node.equals(rsZNode) || + // /hbase/table and /hbase/table/foo is allowed, /hbase/table-lock is not + node.equals(tableZNode) || node.startsWith(tableZNode + "/"); + } + + /** + * Join the prefix znode name with the suffix znode name to generate a proper full znode name. + *

* Assumes prefix does not end with slash and suffix does not begin with it. - * * @param prefix beginning of znode name * @param suffix ending of znode name * @return result of properly joining prefix with suffix diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZNodePaths.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZNodePaths.java new file mode 100644 index 00000000000..102dde19271 --- /dev/null +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZNodePaths.java @@ -0,0 +1,51 @@ +/** + * 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.zookeeper; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import org.apache.hadoop.hbase.HBaseConfiguration; +import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.apache.hadoop.hbase.testclassification.ZKTests; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +@Category({ ZKTests.class, SmallTests.class }) +public class TestZNodePaths { + + @Test + public void testIsClientReadable() { + ZNodePaths znodePaths = new ZNodePaths(HBaseConfiguration.create()); + assertTrue(znodePaths.isClientReadable(znodePaths.baseZNode)); + assertTrue(znodePaths.isClientReadable(znodePaths.getZNodeForReplica(0))); + assertTrue(znodePaths.isClientReadable(znodePaths.masterAddressZNode)); + assertTrue(znodePaths.isClientReadable(znodePaths.clusterIdZNode)); + assertTrue(znodePaths.isClientReadable(znodePaths.tableZNode)); + assertTrue(znodePaths.isClientReadable(ZNodePaths.joinZNode(znodePaths.tableZNode, "foo"))); + assertTrue(znodePaths.isClientReadable(znodePaths.rsZNode)); + + assertFalse(znodePaths.isClientReadable(znodePaths.tableLockZNode)); + assertFalse(znodePaths.isClientReadable(znodePaths.balancerZNode)); + assertFalse(znodePaths.isClientReadable(znodePaths.regionNormalizerZNode)); + assertFalse(znodePaths.isClientReadable(znodePaths.clusterStateZNode)); + assertFalse(znodePaths.isClientReadable(znodePaths.drainingZNode)); + assertFalse(znodePaths.isClientReadable(znodePaths.splitLogZNode)); + assertFalse(znodePaths.isClientReadable(znodePaths.backupMasterAddressesZNode)); + } +} diff --git a/hbase-it/src/test/java/org/apache/hadoop/hbase/test/IntegrationTestZKAndFSPermissions.java b/hbase-it/src/test/java/org/apache/hadoop/hbase/test/IntegrationTestZKAndFSPermissions.java index 6629f89ffc0..f785e9446da 100644 --- a/hbase-it/src/test/java/org/apache/hadoop/hbase/test/IntegrationTestZKAndFSPermissions.java +++ b/hbase-it/src/test/java/org/apache/hadoop/hbase/test/IntegrationTestZKAndFSPermissions.java @@ -159,7 +159,7 @@ public class IntegrationTestZKAndFSPermissions extends AbstractHBaseTool { private void checkZnodePermsRecursive(ZKWatcher watcher, RecoverableZooKeeper zk, String znode) throws KeeperException, InterruptedException { - boolean expectedWorldReadable = watcher.isClientReadable(znode); + boolean expectedWorldReadable = watcher.znodePaths.isClientReadable(znode); assertZnodePerms(zk, znode, expectedWorldReadable); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java index b41e39907e1..e03a9da51ec 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java @@ -20,23 +20,21 @@ package org.apache.hadoop.hbase; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; -import java.io.IOException; import java.util.List; import java.util.Map; - import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.client.Admin; +import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; import org.apache.hadoop.hbase.client.Put; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.Result; import org.apache.hadoop.hbase.client.ResultScanner; import org.apache.hadoop.hbase.client.Scan; 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.coordination.ZkSplitLogWorkerCoordination; import org.apache.hadoop.hbase.master.HMaster; import org.apache.hadoop.hbase.master.LoadBalancer; @@ -45,17 +43,9 @@ import org.apache.hadoop.hbase.testclassification.LargeTests; import org.apache.hadoop.hbase.testclassification.MiscTests; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.FSUtils; -import org.apache.hadoop.hbase.util.Threads; -import org.apache.hadoop.hbase.zookeeper.EmptyWatcher; -import org.apache.hadoop.hbase.zookeeper.ZKConfig; import org.apache.hadoop.hbase.zookeeper.ZKUtil; import org.apache.hadoop.hbase.zookeeper.ZKWatcher; -import org.apache.zookeeper.CreateMode; import org.apache.zookeeper.KeeperException; -import org.apache.zookeeper.ZooDefs; -import org.apache.zookeeper.ZooKeeper; -import org.apache.zookeeper.data.ACL; -import org.apache.zookeeper.data.Stat; import org.junit.After; import org.junit.AfterClass; import org.junit.Before; @@ -67,20 +57,15 @@ import org.junit.rules.TestName; import org.slf4j.Logger; import org.slf4j.LoggerFactory; - @Category({MiscTests.class, LargeTests.class}) public class TestZooKeeper { private static final Logger LOG = LoggerFactory.getLogger(TestZooKeeper.class); - private final static HBaseTestingUtility - TEST_UTIL = new HBaseTestingUtility(); + private final static HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility(); @Rule public TestName name = new TestName(); - /** - * @throws java.lang.Exception - */ @BeforeClass public static void setUpBeforeClass() throws Exception { // Test we can first start the ZK cluster by itself @@ -92,17 +77,11 @@ public class TestZooKeeper { LoadBalancer.class); } - /** - * @throws java.lang.Exception - */ @AfterClass public static void tearDownAfterClass() throws Exception { TEST_UTIL.shutdownMiniCluster(); } - /** - * @throws java.lang.Exception - */ @Before public void setUp() throws Exception { TEST_UTIL.startMiniHBaseCluster(2, 2); @@ -155,13 +134,11 @@ public class TestZooKeeper { /** * Make sure we can use the cluster - * @throws Exception */ - private void testSanity(final String testName) throws Exception{ + private void testSanity(final String testName) throws Exception { String tableName = testName + "_" + System.currentTimeMillis(); - HTableDescriptor desc = new HTableDescriptor(TableName.valueOf(tableName)); - HColumnDescriptor family = new HColumnDescriptor("fam"); - desc.addFamily(family); + TableDescriptor desc = TableDescriptorBuilder.newBuilder(TableName.valueOf(tableName)) + .addColumnFamily(ColumnFamilyDescriptorBuilder.of("fam")).build(); LOG.info("Creating table " + tableName); Admin admin = TEST_UTIL.getAdmin(); try { @@ -178,162 +155,6 @@ public class TestZooKeeper { table.close(); } - /** - * Create a znode with data - * @throws Exception - */ - @Test - public void testCreateWithParents() throws Exception { - ZKWatcher zkw = - new ZKWatcher(new Configuration(TEST_UTIL.getConfiguration()), - TestZooKeeper.class.getName(), null); - byte[] expectedData = new byte[] { 1, 2, 3 }; - ZKUtil.createWithParents(zkw, "/l1/l2/l3/l4/testCreateWithParents", expectedData); - byte[] data = ZKUtil.getData(zkw, "/l1/l2/l3/l4/testCreateWithParents"); - assertTrue(Bytes.equals(expectedData, data)); - ZKUtil.deleteNodeRecursively(zkw, "/l1"); - - ZKUtil.createWithParents(zkw, "/testCreateWithParents", expectedData); - data = ZKUtil.getData(zkw, "/testCreateWithParents"); - assertTrue(Bytes.equals(expectedData, data)); - ZKUtil.deleteNodeRecursively(zkw, "/testCreateWithParents"); - } - - /** - * Create a bunch of znodes in a hierarchy, try deleting one that has childs (it will fail), then - * delete it recursively, then delete the last znode - * @throws Exception - */ - @Test - public void testZNodeDeletes() throws Exception { - ZKWatcher zkw = new ZKWatcher( - new Configuration(TEST_UTIL.getConfiguration()), - TestZooKeeper.class.getName(), null); - ZKUtil.createWithParents(zkw, "/l1/l2/l3/l4"); - try { - ZKUtil.deleteNode(zkw, "/l1/l2"); - fail("We should not be able to delete if znode has childs"); - } catch (KeeperException ex) { - assertNotNull(ZKUtil.getDataNoWatch(zkw, "/l1/l2/l3/l4", null)); - } - ZKUtil.deleteNodeRecursively(zkw, "/l1/l2"); - // make sure it really is deleted - assertNull(ZKUtil.getDataNoWatch(zkw, "/l1/l2/l3/l4", null)); - - // do the same delete again and make sure it doesn't crash - ZKUtil.deleteNodeRecursively(zkw, "/l1/l2"); - - ZKUtil.deleteNode(zkw, "/l1"); - assertNull(ZKUtil.getDataNoWatch(zkw, "/l1/l2", null)); - } - - /** - * A test for HBASE-3238 - * @throws IOException A connection attempt to zk failed - * @throws InterruptedException One of the non ZKUtil actions was interrupted - * @throws KeeperException Any of the zookeeper connections had a - * KeeperException - */ - @Test - public void testCreateSilentIsReallySilent() throws InterruptedException, - KeeperException, IOException { - Configuration c = TEST_UTIL.getConfiguration(); - - String aclZnode = "/aclRoot"; - String quorumServers = ZKConfig.getZKQuorumServersString(c); - int sessionTimeout = 5 * 1000; // 5 seconds - ZooKeeper zk = new ZooKeeper(quorumServers, sessionTimeout, EmptyWatcher.instance); - zk.addAuthInfo("digest", "hbase:rox".getBytes()); - - // Assumes the root of the ZooKeeper space is writable as it creates a node - // wherever the cluster home is defined. - ZKWatcher zk2 = new ZKWatcher(TEST_UTIL.getConfiguration(), - "testCreateSilentIsReallySilent", null); - - // Save the previous ACL - Stat s = null; - List oldACL = null; - while (true) { - try { - s = new Stat(); - oldACL = zk.getACL("/", s); - break; - } catch (KeeperException e) { - switch (e.code()) { - case CONNECTIONLOSS: - case SESSIONEXPIRED: - case OPERATIONTIMEOUT: - LOG.warn("Possibly transient ZooKeeper exception", e); - Threads.sleep(100); - break; - default: - throw e; - } - } - } - - // I set this acl after the attempted creation of the cluster home node. - // Add retries in case of retryable zk exceptions. - while (true) { - try { - zk.setACL("/", ZooDefs.Ids.CREATOR_ALL_ACL, -1); - break; - } catch (KeeperException e) { - switch (e.code()) { - case CONNECTIONLOSS: - case SESSIONEXPIRED: - case OPERATIONTIMEOUT: - LOG.warn("Possibly transient ZooKeeper exception: " + e); - Threads.sleep(100); - break; - default: - throw e; - } - } - } - - while (true) { - try { - zk.create(aclZnode, null, ZooDefs.Ids.CREATOR_ALL_ACL, CreateMode.PERSISTENT); - break; - } catch (KeeperException e) { - switch (e.code()) { - case CONNECTIONLOSS: - case SESSIONEXPIRED: - case OPERATIONTIMEOUT: - LOG.warn("Possibly transient ZooKeeper exception: " + e); - Threads.sleep(100); - break; - default: - throw e; - } - } - } - zk.close(); - ZKUtil.createAndFailSilent(zk2, aclZnode); - - // Restore the ACL - ZooKeeper zk3 = new ZooKeeper(quorumServers, sessionTimeout, EmptyWatcher.instance); - zk3.addAuthInfo("digest", "hbase:rox".getBytes()); - try { - zk3.setACL("/", oldACL, -1); - } finally { - zk3.close(); - } - } - - /** - * Test should not fail with NPE when getChildDataAndWatchForNewChildren - * invoked with wrongNode - */ - @Test - @SuppressWarnings("deprecation") - public void testGetChildDataAndWatchForNewChildrenShouldNotThrowNPE() - throws Exception { - ZKWatcher zkw = new ZKWatcher(TEST_UTIL.getConfiguration(), name.getMethodName(), null); - ZKUtil.getChildDataAndWatchForNewChildren(zkw, "/wrongNode"); - } - /** * Tests that the master does not call retainAssignment after recovery from expired zookeeper * session. Without the HBASE-6046 fix master always tries to assign all the user regions by @@ -351,8 +172,9 @@ public class TestZooKeeper { byte[][] SPLIT_KEYS = new byte[][] { Bytes.toBytes("a"), Bytes.toBytes("b"), Bytes.toBytes("c"), Bytes.toBytes("d"), Bytes.toBytes("e"), Bytes.toBytes("f"), Bytes.toBytes("g"), Bytes.toBytes("h"), Bytes.toBytes("i"), Bytes.toBytes("j") }; - HTableDescriptor htd = new HTableDescriptor(TableName.valueOf(name.getMethodName())); - htd.addFamily(new HColumnDescriptor(HConstants.CATALOG_FAMILY)); + TableDescriptor htd = + TableDescriptorBuilder.newBuilder(TableName.valueOf(name.getMethodName())) + .addColumnFamily(ColumnFamilyDescriptorBuilder.of(HConstants.CATALOG_FAMILY)).build(); admin.createTable(htd, SPLIT_KEYS); TEST_UTIL.waitUntilNoRegionsInTransition(60000); m.getZooKeeper().close(); @@ -414,11 +236,10 @@ public class TestZooKeeper { Table table = null; try { byte[][] SPLIT_KEYS = new byte[][] { Bytes.toBytes("1"), Bytes.toBytes("2"), - Bytes.toBytes("3"), Bytes.toBytes("4"), Bytes.toBytes("5") }; - - HTableDescriptor htd = new HTableDescriptor(TableName.valueOf(name.getMethodName())); - HColumnDescriptor hcd = new HColumnDescriptor("col"); - htd.addFamily(hcd); + Bytes.toBytes("3"), Bytes.toBytes("4"), Bytes.toBytes("5") }; + TableDescriptor htd = + TableDescriptorBuilder.newBuilder(TableName.valueOf(name.getMethodName())) + .addColumnFamily(ColumnFamilyDescriptorBuilder.of("col")).build(); admin.createTable(htd, SPLIT_KEYS); TEST_UTIL.waitUntilNoRegionsInTransition(60000); table = TEST_UTIL.getConnection().getTable(htd.getTableName()); diff --git a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java index 952013da32d..7bb47527597 100644 --- a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java +++ b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java @@ -923,7 +923,7 @@ public final class ZKUtil { } // Certain znodes are accessed directly by the client, // so they must be readable by non-authenticated clients - if (zkw.isClientReadable(node)) { + if (zkw.znodePaths.isClientReadable(node)) { acls.addAll(Ids.CREATOR_ALL_ACL); acls.addAll(Ids.READ_ACL_UNSAFE); } else { diff --git a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKWatcher.java b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKWatcher.java index b0c6a6f0c3d..3aac946947b 100644 --- a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKWatcher.java +++ b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKWatcher.java @@ -159,23 +159,6 @@ public class ZKWatcher implements Watcher, Abortable, Closeable { } } - /** Returns whether the znode is supposed to be readable by the client - * and DOES NOT contain sensitive information (world readable).*/ - public boolean isClientReadable(String node) { - // Developer notice: These znodes are world readable. DO NOT add more znodes here UNLESS - // all clients need to access this data to work. Using zk for sharing data to clients (other - // than service lookup case is not a recommended design pattern. - return - node.equals(znodePaths.baseZNode) || - znodePaths.isAnyMetaReplicaZNode(node) || - node.equals(znodePaths.masterAddressZNode) || - node.equals(znodePaths.clusterIdZNode)|| - node.equals(znodePaths.rsZNode) || - // /hbase/table and /hbase/table/foo is allowed, /hbase/table-lock is not - node.equals(znodePaths.tableZNode) || - node.startsWith(znodePaths.tableZNode + "/"); - } - /** * On master start, we check the znode ACLs under the root directory and set the ACLs properly * if needed. If the cluster goes from an unsecure setup to a secure setup, this step is needed diff --git a/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKUtil.java b/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKUtil.java index 2ea70d62404..95dd270ff5c 100644 --- a/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKUtil.java +++ b/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKUtil.java @@ -7,105 +7,221 @@ * "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 + * 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. + * 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.zookeeper; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + import java.io.IOException; import java.util.List; - import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.hbase.HBaseConfiguration; -import org.apache.hadoop.hbase.ZooKeeperConnectionException; -import org.apache.hadoop.hbase.security.Superusers; -import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.apache.hadoop.hbase.Abortable; +import org.apache.hadoop.hbase.HBaseZKTestingUtility; +import org.apache.hadoop.hbase.testclassification.MediumTests; import org.apache.hadoop.hbase.testclassification.ZKTests; -import org.apache.hadoop.security.UserGroupInformation; +import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.hbase.util.Threads; +import org.apache.zookeeper.CreateMode; import org.apache.zookeeper.KeeperException; -import org.apache.zookeeper.ZooDefs.Ids; -import org.apache.zookeeper.ZooDefs.Perms; +import org.apache.zookeeper.ZooDefs; +import org.apache.zookeeper.ZooKeeper; import org.apache.zookeeper.data.ACL; -import org.apache.zookeeper.data.Id; -import org.junit.Assert; +import org.apache.zookeeper.data.Stat; +import org.junit.AfterClass; +import org.junit.BeforeClass; import org.junit.Test; import org.junit.experimental.categories.Category; -import org.mockito.Mockito; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; -@Category({ ZKTests.class, SmallTests.class }) +import org.apache.hbase.thirdparty.com.google.common.io.Closeables; + +@Category({ ZKTests.class, MediumTests.class }) public class TestZKUtil { - @Test - public void testUnsecure() throws ZooKeeperConnectionException, IOException { - Configuration conf = HBaseConfiguration.create(); - conf.set(Superusers.SUPERUSER_CONF_KEY, "user1"); - String node = "/hbase/testUnsecure"; - ZKWatcher watcher = new ZKWatcher(conf, node, null, false); - List aclList = ZKUtil.createACL(watcher, node, false); - Assert.assertEquals(1, aclList.size()); - Assert.assertTrue(aclList.contains(Ids.OPEN_ACL_UNSAFE.iterator().next())); + private static final Logger LOG = LoggerFactory.getLogger(TestZKUtil.class); + + private static HBaseZKTestingUtility UTIL = new HBaseZKTestingUtility(); + + private static ZKWatcher ZKW; + + @BeforeClass + public static void setUp() throws Exception { + UTIL.startMiniZKCluster().getClientPort(); + ZKW = new ZKWatcher(new Configuration(UTIL.getConfiguration()), TestZKUtil.class.getName(), + new WarnOnlyAbortable()); + } - @Test - public void testSecuritySingleSuperuser() throws ZooKeeperConnectionException, IOException { - Configuration conf = HBaseConfiguration.create(); - conf.set(Superusers.SUPERUSER_CONF_KEY, "user1"); - String node = "/hbase/testSecuritySingleSuperuser"; - ZKWatcher watcher = new ZKWatcher(conf, node, null, false); - List aclList = ZKUtil.createACL(watcher, node, true); - Assert.assertEquals(2, aclList.size()); // 1+1, since ACL will be set for the creator by default - Assert.assertTrue(aclList.contains(new ACL(Perms.ALL, new Id("sasl", "user1")))); - Assert.assertTrue(aclList.contains(Ids.CREATOR_ALL_ACL.iterator().next())); + @AfterClass + public static void tearDown() throws IOException { + Closeables.close(ZKW, true); + UTIL.shutdownMiniZKCluster(); + UTIL.cleanupTestDir(); } + /** + * Create a znode with data + */ @Test - public void testCreateACL() throws ZooKeeperConnectionException, IOException { - Configuration conf = HBaseConfiguration.create(); - conf.set(Superusers.SUPERUSER_CONF_KEY, "user1,@group1,user2,@group2,user3"); - String node = "/hbase/testCreateACL"; - ZKWatcher watcher = new ZKWatcher(conf, node, null, false); - List aclList = ZKUtil.createACL(watcher, node, true); - Assert.assertEquals(4, aclList.size()); // 3+1, since ACL will be set for the creator by default - Assert.assertFalse(aclList.contains(new ACL(Perms.ALL, new Id("sasl", "@group1")))); - Assert.assertFalse(aclList.contains(new ACL(Perms.ALL, new Id("sasl", "@group2")))); - Assert.assertTrue(aclList.contains(new ACL(Perms.ALL, new Id("sasl", "user1")))); - Assert.assertTrue(aclList.contains(new ACL(Perms.ALL, new Id("sasl", "user2")))); - Assert.assertTrue(aclList.contains(new ACL(Perms.ALL, new Id("sasl", "user3")))); + public void testCreateWithParents() throws KeeperException, InterruptedException { + byte[] expectedData = new byte[] { 1, 2, 3 }; + ZKUtil.createWithParents(ZKW, "/l1/l2/l3/l4/testCreateWithParents", expectedData); + byte[] data = ZKUtil.getData(ZKW, "/l1/l2/l3/l4/testCreateWithParents"); + assertTrue(Bytes.equals(expectedData, data)); + ZKUtil.deleteNodeRecursively(ZKW, "/l1"); + + ZKUtil.createWithParents(ZKW, "/testCreateWithParents", expectedData); + data = ZKUtil.getData(ZKW, "/testCreateWithParents"); + assertTrue(Bytes.equals(expectedData, data)); + ZKUtil.deleteNodeRecursively(ZKW, "/testCreateWithParents"); } + /** + * Create a bunch of znodes in a hierarchy, try deleting one that has childs (it will fail), then + * delete it recursively, then delete the last znode + */ @Test - public void testCreateACLWithSameUser() throws ZooKeeperConnectionException, IOException { - Configuration conf = HBaseConfiguration.create(); - conf.set(Superusers.SUPERUSER_CONF_KEY, "user4,@group1,user5,user6"); - UserGroupInformation.setLoginUser(UserGroupInformation.createRemoteUser("user4")); - String node = "/hbase/testCreateACL"; - ZKWatcher watcher = new ZKWatcher(conf, node, null, false); - List aclList = ZKUtil.createACL(watcher, node, true); - Assert.assertEquals(3, aclList.size()); // 3, since service user the same as one of superuser - Assert.assertFalse(aclList.contains(new ACL(Perms.ALL, new Id("sasl", "@group1")))); - Assert.assertTrue(aclList.contains(new ACL(Perms.ALL, new Id("auth", "")))); - Assert.assertTrue(aclList.contains(new ACL(Perms.ALL, new Id("sasl", "user5")))); - Assert.assertTrue(aclList.contains(new ACL(Perms.ALL, new Id("sasl", "user6")))); + public void testZNodeDeletes() throws Exception { + ZKUtil.createWithParents(ZKW, "/l1/l2/l3/l4"); + try { + ZKUtil.deleteNode(ZKW, "/l1/l2"); + fail("We should not be able to delete if znode has childs"); + } catch (KeeperException ex) { + assertNotNull(ZKUtil.getDataNoWatch(ZKW, "/l1/l2/l3/l4", null)); + } + ZKUtil.deleteNodeRecursively(ZKW, "/l1/l2"); + // make sure it really is deleted + assertNull(ZKUtil.getDataNoWatch(ZKW, "/l1/l2/l3/l4", null)); + + // do the same delete again and make sure it doesn't crash + ZKUtil.deleteNodeRecursively(ZKW, "/l1/l2"); + + ZKUtil.deleteNode(ZKW, "/l1"); + assertNull(ZKUtil.getDataNoWatch(ZKW, "/l1/l2", null)); } - @Test(expected = KeeperException.SystemErrorException.class) - public void testInterruptedDuringAction() - throws ZooKeeperConnectionException, IOException, KeeperException, InterruptedException { - final RecoverableZooKeeper recoverableZk = Mockito.mock(RecoverableZooKeeper.class); - ZKWatcher zkw = new ZKWatcher(HBaseConfiguration.create(), "unittest", null) { - @Override - public RecoverableZooKeeper getRecoverableZooKeeper() { - return recoverableZk; + /** + * A test for HBASE-3238 + * @throws IOException A connection attempt to zk failed + * @throws InterruptedException One of the non ZKUtil actions was interrupted + * @throws KeeperException Any of the zookeeper connections had a KeeperException + */ + @Test + public void testCreateSilentIsReallySilent() + throws InterruptedException, KeeperException, IOException { + Configuration c = UTIL.getConfiguration(); + + String aclZnode = "/aclRoot"; + String quorumServers = ZKConfig.getZKQuorumServersString(c); + int sessionTimeout = 5 * 1000; // 5 seconds + ZooKeeper zk = new ZooKeeper(quorumServers, sessionTimeout, EmptyWatcher.instance); + zk.addAuthInfo("digest", "hbase:rox".getBytes()); + + // Save the previous ACL + Stat s = null; + List oldACL = null; + while (true) { + try { + s = new Stat(); + oldACL = zk.getACL("/", s); + break; + } catch (KeeperException e) { + switch (e.code()) { + case CONNECTIONLOSS: + case SESSIONEXPIRED: + case OPERATIONTIMEOUT: + LOG.warn("Possibly transient ZooKeeper exception", e); + Threads.sleep(100); + break; + default: + throw e; + } } - }; - Mockito.doThrow(new InterruptedException()).when(recoverableZk) - .getChildren(zkw.znodePaths.baseZNode, null); - ZKUtil.listChildrenNoWatch(zkw, zkw.znodePaths.baseZNode); + } + + // I set this acl after the attempted creation of the cluster home node. + // Add retries in case of retryable zk exceptions. + while (true) { + try { + zk.setACL("/", ZooDefs.Ids.CREATOR_ALL_ACL, -1); + break; + } catch (KeeperException e) { + switch (e.code()) { + case CONNECTIONLOSS: + case SESSIONEXPIRED: + case OPERATIONTIMEOUT: + LOG.warn("Possibly transient ZooKeeper exception: " + e); + Threads.sleep(100); + break; + default: + throw e; + } + } + } + + while (true) { + try { + zk.create(aclZnode, null, ZooDefs.Ids.CREATOR_ALL_ACL, CreateMode.PERSISTENT); + break; + } catch (KeeperException e) { + switch (e.code()) { + case CONNECTIONLOSS: + case SESSIONEXPIRED: + case OPERATIONTIMEOUT: + LOG.warn("Possibly transient ZooKeeper exception: " + e); + Threads.sleep(100); + break; + default: + throw e; + } + } + } + zk.close(); + ZKUtil.createAndFailSilent(ZKW, aclZnode); + + // Restore the ACL + ZooKeeper zk3 = new ZooKeeper(quorumServers, sessionTimeout, EmptyWatcher.instance); + zk3.addAuthInfo("digest", "hbase:rox".getBytes()); + try { + zk3.setACL("/", oldACL, -1); + } finally { + zk3.close(); + } + } + + /** + * Test should not fail with NPE when getChildDataAndWatchForNewChildren invoked with wrongNode + */ + @Test + @SuppressWarnings("deprecation") + public void testGetChildDataAndWatchForNewChildrenShouldNotThrowNPE() throws Exception { + ZKUtil.getChildDataAndWatchForNewChildren(ZKW, "/wrongNode"); + } + + private static class WarnOnlyAbortable implements Abortable { + + @Override + public void abort(String why, Throwable e) { + LOG.warn("ZKWatcher received abort, ignoring. Reason: " + why); + if (LOG.isDebugEnabled()) { + LOG.debug(e.toString(), e); + } + } + + @Override + public boolean isAborted() { + return false; + } } } diff --git a/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKUtilNoServer.java b/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKUtilNoServer.java new file mode 100644 index 00000000000..cc2517b163f --- /dev/null +++ b/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKUtilNoServer.java @@ -0,0 +1,113 @@ +/** + * 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.zookeeper; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.io.IOException; +import java.util.List; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.HBaseConfiguration; +import org.apache.hadoop.hbase.ZooKeeperConnectionException; +import org.apache.hadoop.hbase.security.Superusers; +import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.apache.hadoop.hbase.testclassification.ZKTests; +import org.apache.hadoop.security.UserGroupInformation; +import org.apache.zookeeper.KeeperException; +import org.apache.zookeeper.ZooDefs.Ids; +import org.apache.zookeeper.ZooDefs.Perms; +import org.apache.zookeeper.data.ACL; +import org.apache.zookeeper.data.Id; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.mockito.Mockito; + +@Category({ ZKTests.class, SmallTests.class }) +public class TestZKUtilNoServer { + + @Test + public void testUnsecure() throws ZooKeeperConnectionException, IOException { + Configuration conf = HBaseConfiguration.create(); + conf.set(Superusers.SUPERUSER_CONF_KEY, "user1"); + String node = "/hbase/testUnsecure"; + ZKWatcher watcher = new ZKWatcher(conf, node, null, false); + List aclList = ZKUtil.createACL(watcher, node, false); + assertEquals(1, aclList.size()); + assertTrue(aclList.contains(Ids.OPEN_ACL_UNSAFE.iterator().next())); + } + + @Test + public void testSecuritySingleSuperuser() throws ZooKeeperConnectionException, IOException { + Configuration conf = HBaseConfiguration.create(); + conf.set(Superusers.SUPERUSER_CONF_KEY, "user1"); + String node = "/hbase/testSecuritySingleSuperuser"; + ZKWatcher watcher = new ZKWatcher(conf, node, null, false); + List aclList = ZKUtil.createACL(watcher, node, true); + assertEquals(2, aclList.size()); // 1+1, since ACL will be set for the creator by default + assertTrue(aclList.contains(new ACL(Perms.ALL, new Id("sasl", "user1")))); + assertTrue(aclList.contains(Ids.CREATOR_ALL_ACL.iterator().next())); + } + + @Test + public void testCreateACL() throws ZooKeeperConnectionException, IOException { + Configuration conf = HBaseConfiguration.create(); + conf.set(Superusers.SUPERUSER_CONF_KEY, "user1,@group1,user2,@group2,user3"); + String node = "/hbase/testCreateACL"; + ZKWatcher watcher = new ZKWatcher(conf, node, null, false); + List aclList = ZKUtil.createACL(watcher, node, true); + assertEquals(4, aclList.size()); // 3+1, since ACL will be set for the creator by default + assertFalse(aclList.contains(new ACL(Perms.ALL, new Id("sasl", "@group1")))); + assertFalse(aclList.contains(new ACL(Perms.ALL, new Id("sasl", "@group2")))); + assertTrue(aclList.contains(new ACL(Perms.ALL, new Id("sasl", "user1")))); + assertTrue(aclList.contains(new ACL(Perms.ALL, new Id("sasl", "user2")))); + assertTrue(aclList.contains(new ACL(Perms.ALL, new Id("sasl", "user3")))); + } + + @Test + public void testCreateACLWithSameUser() throws ZooKeeperConnectionException, IOException { + Configuration conf = HBaseConfiguration.create(); + conf.set(Superusers.SUPERUSER_CONF_KEY, "user4,@group1,user5,user6"); + UserGroupInformation.setLoginUser(UserGroupInformation.createRemoteUser("user4")); + String node = "/hbase/testCreateACL"; + ZKWatcher watcher = new ZKWatcher(conf, node, null, false); + List aclList = ZKUtil.createACL(watcher, node, true); + assertEquals(3, aclList.size()); // 3, since service user the same as one of superuser + assertFalse(aclList.contains(new ACL(Perms.ALL, new Id("sasl", "@group1")))); + assertTrue(aclList.contains(new ACL(Perms.ALL, new Id("auth", "")))); + assertTrue(aclList.contains(new ACL(Perms.ALL, new Id("sasl", "user5")))); + assertTrue(aclList.contains(new ACL(Perms.ALL, new Id("sasl", "user6")))); + } + + @Test(expected = KeeperException.SystemErrorException.class) + public void testInterruptedDuringAction() + throws ZooKeeperConnectionException, IOException, KeeperException, InterruptedException { + final RecoverableZooKeeper recoverableZk = Mockito.mock(RecoverableZooKeeper.class); + ZKWatcher zkw = new ZKWatcher(HBaseConfiguration.create(), "unittest", null) { + @Override + public RecoverableZooKeeper getRecoverableZooKeeper() { + return recoverableZk; + } + }; + Mockito.doThrow(new InterruptedException()).when(recoverableZk) + .getChildren(zkw.znodePaths.baseZNode, null); + ZKUtil.listChildrenNoWatch(zkw, zkw.znodePaths.baseZNode); + } +} diff --git a/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKWatcher.java b/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKWatcher.java deleted file mode 100644 index f3d0b035132..00000000000 --- a/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKWatcher.java +++ /dev/null @@ -1,58 +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.zookeeper; - -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; - -import java.io.IOException; - -import org.apache.hadoop.hbase.HBaseConfiguration; -import org.apache.hadoop.hbase.testclassification.SmallTests; -import org.apache.hadoop.hbase.testclassification.ZKTests; -import org.junit.Test; -import org.junit.experimental.categories.Category; - -@Category({ ZKTests.class, SmallTests.class }) -public class TestZKWatcher { - - @Test - public void testIsClientReadable() throws IOException { - ZKWatcher watcher = - new ZKWatcher(HBaseConfiguration.create(), "testIsClientReadable", null, false); - - assertTrue(watcher.isClientReadable(watcher.znodePaths.baseZNode)); - assertTrue(watcher.isClientReadable(watcher.znodePaths.getZNodeForReplica(0))); - assertTrue(watcher.isClientReadable(watcher.znodePaths.masterAddressZNode)); - assertTrue(watcher.isClientReadable(watcher.znodePaths.clusterIdZNode)); - assertTrue(watcher.isClientReadable(watcher.znodePaths.tableZNode)); - assertTrue( - watcher.isClientReadable(ZNodePaths.joinZNode(watcher.znodePaths.tableZNode, "foo"))); - assertTrue(watcher.isClientReadable(watcher.znodePaths.rsZNode)); - - assertFalse(watcher.isClientReadable(watcher.znodePaths.tableLockZNode)); - assertFalse(watcher.isClientReadable(watcher.znodePaths.balancerZNode)); - assertFalse(watcher.isClientReadable(watcher.znodePaths.regionNormalizerZNode)); - assertFalse(watcher.isClientReadable(watcher.znodePaths.clusterStateZNode)); - assertFalse(watcher.isClientReadable(watcher.znodePaths.drainingZNode)); - assertFalse(watcher.isClientReadable(watcher.znodePaths.splitLogZNode)); - assertFalse(watcher.isClientReadable(watcher.znodePaths.backupMasterAddressesZNode)); - - watcher.close(); - } -}