Repository: hbase Updated Branches: refs/heads/branch-1 c0fcce2bd -> ce43e3387
HBASE-15769 Perform validation on cluster key for add_peer (Matt Warhaftig) Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/ce43e338 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/ce43e338 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/ce43e338 Branch: refs/heads/branch-1 Commit: ce43e33876193fbd5f04708da1072186f7d9bf4b Parents: c0fcce2 Author: tedyu <yuzhih...@gmail.com> Authored: Tue May 17 13:26:45 2016 -0700 Committer: tedyu <yuzhih...@gmail.com> Committed: Tue May 17 13:26:45 2016 -0700 ---------------------------------------------------------------------- .../replication/ReplicationPeersZKImpl.java | 9 ++++++ .../apache/hadoop/hbase/zookeeper/ZKConfig.java | 11 +++++++ .../hadoop/hbase/zookeeper/TestZKConfig.java | 20 ++++++------ .../replication/TestReplicationStateBasic.java | 33 +++++++++++++++++++- .../src/main/ruby/shell/commands/add_peer.rb | 7 ++--- 5 files changed, 64 insertions(+), 16 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/ce43e338/hbase-client/src/main/java/org/apache/hadoop/hbase/replication/ReplicationPeersZKImpl.java ---------------------------------------------------------------------- diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/replication/ReplicationPeersZKImpl.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/replication/ReplicationPeersZKImpl.java index 7bf6c43..c0c3f7e 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/replication/ReplicationPeersZKImpl.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/replication/ReplicationPeersZKImpl.java @@ -40,6 +40,7 @@ import org.apache.hadoop.hbase.protobuf.generated.ZooKeeperProtos; import org.apache.hadoop.hbase.replication.ReplicationPeer.PeerState; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.Pair; +import org.apache.hadoop.hbase.zookeeper.ZKConfig; import org.apache.hadoop.hbase.zookeeper.ZKUtil; import org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher; import org.apache.hadoop.hbase.zookeeper.ZKUtil.ZKUtilOp; @@ -115,6 +116,14 @@ public class ReplicationPeersZKImpl extends ReplicationStateZKBase implements Re throw new IllegalArgumentException("Found invalid peer name:" + id); } + if (peerConfig.getClusterKey() != null) { + try { + ZKConfig.validateClusterKey(peerConfig.getClusterKey()); + } catch (IOException ioe) { + throw new IllegalArgumentException(ioe.getMessage()); + } + } + checkQueuesDeleted(id); ZKUtil.createWithParents(this.zookeeper, this.peersZNode); http://git-wip-us.apache.org/repos/asf/hbase/blob/ce43e338/hbase-common/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKConfig.java ---------------------------------------------------------------------- diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKConfig.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKConfig.java index 787b5cc..17bf300 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKConfig.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKConfig.java @@ -384,12 +384,23 @@ public final class ZKConfig { String[] parts = key.split(":"); if (parts.length == 3) { + if (!parts[2].matches("/.*[^/]")) { + throw new IOException("Cluster key passed " + key + " is invalid, the format should be:" + + HConstants.ZOOKEEPER_QUORUM + ":" + HConstants.ZOOKEEPER_CLIENT_PORT + ":" + + HConstants.ZOOKEEPER_ZNODE_PARENT); + } return new ZKClusterKey(parts [0], Integer.parseInt(parts [1]), parts [2]); } if (parts.length > 3) { // The quorum could contain client port in server:clientport format, try to transform more. String zNodeParent = parts [parts.length - 1]; + if (!zNodeParent.matches("/.*[^/]")) { + throw new IOException("Cluster key passed " + key + " is invalid, the format should be:" + + HConstants.ZOOKEEPER_QUORUM + ":" + HConstants.ZOOKEEPER_CLIENT_PORT + ":" + + HConstants.ZOOKEEPER_ZNODE_PARENT); + } + String clientPort = parts [parts.length - 2]; // The first part length is the total length minus the lengths of other parts and minus 2 ":" http://git-wip-us.apache.org/repos/asf/hbase/blob/ce43e338/hbase-common/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKConfig.java ---------------------------------------------------------------------- diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKConfig.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKConfig.java index 7879aea..945b291 100644 --- a/hbase-common/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKConfig.java +++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKConfig.java @@ -60,10 +60,10 @@ public class TestZKConfig { @Test public void testClusterKey() throws Exception { - testKey("server", 2181, "hbase"); - testKey("server1,server2,server3", 2181, "hbase"); + testKey("server", 2181, "/hbase"); + testKey("server1,server2,server3", 2181, "/hbase"); try { - ZKConfig.validateClusterKey("2181:hbase"); + ZKConfig.validateClusterKey("2181:/hbase"); } catch (IOException ex) { // OK } @@ -72,19 +72,19 @@ public class TestZKConfig { @Test public void testClusterKeyWithMultiplePorts() throws Exception { // server has different port than the default port - testKey("server1:2182", 2181, "hbase", true); + testKey("server1:2182", 2181, "/hbase", true); // multiple servers have their own port - testKey("server1:2182,server2:2183,server3:2184", 2181, "hbase", true); + testKey("server1:2182,server2:2183,server3:2184", 2181, "/hbase", true); // one server has no specified port, should use default port - testKey("server1:2182,server2,server3:2184", 2181, "hbase", true); + testKey("server1:2182,server2,server3:2184", 2181, "/hbase", true); // the last server has no specified port, should use default port - testKey("server1:2182,server2:2183,server3", 2181, "hbase", true); + testKey("server1:2182,server2:2183,server3", 2181, "/hbase", true); // multiple servers have no specified port, should use default port for those servers - testKey("server1:2182,server2,server3:2184,server4", 2181, "hbase", true); + testKey("server1:2182,server2,server3:2184,server4", 2181, "/hbase", true); // same server, different ports - testKey("server1:2182,server1:2183,server1", 2181, "hbase", true); + testKey("server1:2182,server1:2183,server1", 2181, "/hbase", true); // mix of same server/different port and different server - testKey("server1:2182,server2:2183,server1", 2181, "hbase", true); + testKey("server1:2182,server2:2183,server1", 2181, "/hbase", true); } private void testKey(String ensemble, int port, String znode) http://git-wip-us.apache.org/repos/asf/hbase/blob/ce43e338/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationStateBasic.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationStateBasic.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationStateBasic.java index 41c3240..d10d6de 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationStateBasic.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationStateBasic.java @@ -161,6 +161,37 @@ public abstract class TestReplicationStateBasic { } @Test + public void testInvalidClusterKeys() throws ReplicationException, KeeperException { + rp.init(); + + try { + rp.addPeer(ID_ONE, + new ReplicationPeerConfig().setClusterKey("hostname1.example.org:1234:hbase"), null); + fail("Should throw an IllegalArgumentException because " + + "zookeeper.znode.parent is missing leading '/'."); + } catch (IllegalArgumentException e) { + // Expected. + } + + try { + rp.addPeer(ID_ONE, + new ReplicationPeerConfig().setClusterKey("hostname1.example.org:1234:/"), null); + fail("Should throw an IllegalArgumentException because zookeeper.znode.parent is missing."); + } catch (IllegalArgumentException e) { + // Expected. + } + + try { + rp.addPeer(ID_ONE, + new ReplicationPeerConfig().setClusterKey("hostname1.example.org::/hbase"), null); + fail("Should throw an IllegalArgumentException because " + + "hbase.zookeeper.property.clientPort is missing."); + } catch (IllegalArgumentException e) { + // Expected. + } + } + + @Test public void testHfileRefsReplicationQueues() throws ReplicationException, KeeperException { rp.init(); rq1.init(server1); @@ -325,7 +356,7 @@ public abstract class TestReplicationStateBasic { rq3.addLog("qId" + i, "filename" + j); } //Add peers for the corresponding queues so they are not orphans - rp.addPeer("qId" + i, new ReplicationPeerConfig().setClusterKey("bogus" + i), null); + rp.addPeer("qId" + i, new ReplicationPeerConfig().setClusterKey("localhost:2818:/bogus" + i), null); } } } http://git-wip-us.apache.org/repos/asf/hbase/blob/ce43e338/hbase-shell/src/main/ruby/shell/commands/add_peer.rb ---------------------------------------------------------------------- diff --git a/hbase-shell/src/main/ruby/shell/commands/add_peer.rb b/hbase-shell/src/main/ruby/shell/commands/add_peer.rb index be01041..0fcdd3d 100644 --- a/hbase-shell/src/main/ruby/shell/commands/add_peer.rb +++ b/hbase-shell/src/main/ruby/shell/commands/add_peer.rb @@ -31,11 +31,8 @@ This gives a full path for HBase to connect to another HBase cluster. An optiona table column families identifies which column families will be replicated to the peer cluster. Examples: - hbase> add_peer '1', "server1.cie.com:2181:/hbase" - hbase> add_peer '2', "zk1,zk2,zk3:2182:/hbase-prod" - hbase> add_peer '3', "zk4,zk5,zk6:11000:/hbase-test", "table1; table2:cf1; table3:cf1,cf2" - hbase> add_peer '4', CLUSTER_KEY => "server1.cie.com:2181:/hbase" - hbase> add_peer '5', CLUSTER_KEY => "server1.cie.com:2181:/hbase", + hbase> add_peer '1', CLUSTER_KEY => "server1.cie.com:2181:/hbase" + hbase> add_peer '2', CLUSTER_KEY => "zk1,zk2,zk3:2182:/hbase-prod", TABLE_CFS => { "table1" => [], "table2" => ["cf1"], "table3" => ["cf1", "cf2"] } For a custom replication endpoint, the ENDPOINT_CLASSNAME can be provided. Two optional arguments
