You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ap...@apache.org on 2016/06/07 23:06:51 UTC
[3/8] hbase git commit: HBASE-15769 Perform validation on cluster key
for add_peer (Matt Warhaftig)
HBASE-15769 Perform validation on cluster key for add_peer (Matt Warhaftig)
Conflicts:
hbase-client/src/main/java/org/apache/hadoop/hbase/replication/ReplicationPeersZKImpl.java
hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationStateBasic.java
Amending-Author: Andrew Purtell <ap...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/458378d7
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/458378d7
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/458378d7
Branch: refs/heads/0.98
Commit: 458378d7089a18b325a35c4c99ca15272fb033b4
Parents: 2d3ef97
Author: tedyu <yu...@gmail.com>
Authored: Tue May 17 13:26:45 2016 -0700
Committer: Andrew Purtell <ap...@apache.org>
Committed: Tue Jun 7 14:29:22 2016 -0700
----------------------------------------------------------------------
.../replication/ReplicationPeersZKImpl.java | 13 ++++++--
.../apache/hadoop/hbase/zookeeper/ZKConfig.java | 11 +++++++
.../hadoop/hbase/zookeeper/TestZKConfig.java | 20 ++++++------
.../replication/TestReplicationStateBasic.java | 34 +++++++++++++++++++-
.../src/main/ruby/shell/commands/add_peer.rb | 7 ++--
5 files changed, 67 insertions(+), 18 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hbase/blob/458378d7/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 632894c..0179574 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
@@ -39,6 +39,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.ZKUtil.ZKUtilOp;
import org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher;
@@ -107,10 +108,18 @@ public class ReplicationPeersZKImpl extends ReplicationStateZKBase implements Re
+ " because that id already exists.");
}
- if(id.contains("-")){
+ if (id.contains("-")) {
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());
+ }
+ }
+
ZKUtil.createWithParents(this.zookeeper, this.peersZNode);
List<ZKUtilOp> listOfOps = new ArrayList<ZKUtil.ZKUtilOp>();
ZKUtilOp op1 = ZKUtilOp.createAndFailSilent(ZKUtil.joinZNode(this.peersZNode, id),
http://git-wip-us.apache.org/repos/asf/hbase/blob/458378d7/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 d23a607..af6b98f 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
@@ -339,12 +339,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/458378d7/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/458378d7/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 696c130..da04f41 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
@@ -160,6 +160,38 @@ 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 testReplicationPeers() throws Exception {
rp.init();
@@ -268,7 +300,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/458378d7/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