hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From te...@apache.org
Subject hbase git commit: HBASE-15769 Perform validation on cluster key for add_peer (Matt Warhaftig)
Date Tue, 17 May 2016 20:26:43 GMT
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 <yuzhihong@gmail.com>
Authored: Tue May 17 13:26:45 2016 -0700
Committer: tedyu <yuzhihong@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


Mime
View raw message