ZOOKEEPER-2014: Only admin should be allowed to reconfig a cluster.

This PR implements ZOOKEEPER-2014. For details, please refer to JIRA: https://issues.apache.org/jira/browse/ZOOKEEPER-2014 Review board: https://reviews.apache.org/r/51546/ Author: Michael Han <[email protected]> Reviewers: fpj <[email protected]>, breed <[email protected]>, rgs <[email protected]> Closes apache#96 from hanm/ZOOKEEPER-2014 (cherry picked from commit 73e102a) Signed-off-by: fpj <[email protected]>
zuai · Nov 13, 2016 · 65d2572 · 65d2572
1 parent 6bd38e3
commit 65d2572
Show file tree

Hide file tree

Showing 36 changed files with 1,273 additions and 244 deletions.
diff --git a/build.xml b/build.xml
@@ -527,6 +527,7 @@ xmlns:cs="antlib:com.puppycrawl.tools.checkstyle.ant">
           <include name="org/apache/zookeeper/WatchedEvent.java"/>
           <include name="org/apache/zookeeper/ZooDefs.java"/>
           <include name="org/apache/zookeeper/ZooKeeper.java"/>
+          <include name="org/apache/zookeeper/admin/ZooKeeperAdmin.java"/>
           <include name="org/apache/zookeeper/server/LogFormatter.java"/>
           <include name="org/apache/zookeeper/server/SnapshotFormatter.java"/>
           <include name="org/apache/zookeeper/server/PurgeTxnLog.java"/>

diff --git a/src/c/include/zookeeper.h b/src/c/include/zookeeper.h
@@ -124,7 +124,8 @@ enum ZOO_ERRORS {
   ZNOTREADONLY = -119, /*!< state-changing request is passed to read-only server */
   ZEPHEMERALONLOCALSESSION = -120, /*!< Attempt to create ephemeral node on a local session */
   ZNOWATCHER = -121, /*!< The watcher couldn't be found */
-  ZRWSERVERFOUND = -122 /*!< r/w server found while in r/o mode */
+  ZRWSERVERFOUND = -122, /*!< r/w server found while in r/o mode */
+  ZRECONFIGDISABLED = -123 /*!< Attempts to perform a reconfiguration operation when reconfiguration feature is disabled */
 };
 
 #ifdef __cplusplus

diff --git a/src/c/tests/TestReconfigServer.cc b/src/c/tests/TestReconfigServer.cc
@@ -15,6 +15,9 @@
  * the License.
  */
 #include <algorithm>
+#include <sstream>
+#include <vector>
+#include <utility>
 #include <cppunit/extensions/HelperMacros.h>
 #include <unistd.h>
 #include "zookeeper.h"
@@ -28,6 +31,8 @@ class TestReconfigServer : public CPPUNIT_NS::TestFixture {
     CPPUNIT_TEST(testNonIncremental);
     CPPUNIT_TEST(testRemoveConnectedFollower);
     CPPUNIT_TEST(testRemoveFollower);
+    CPPUNIT_TEST(testReconfigFailureWithoutAuth);
+    CPPUNIT_TEST(testReconfigFailureWithoutServerSuperuserPasswordConfigured);
 #endif
     CPPUNIT_TEST_SUITE_END();
 
@@ -39,7 +44,8 @@ class TestReconfigServer : public CPPUNIT_NS::TestFixture {
     void testNonIncremental();
     void testRemoveConnectedFollower();
     void testRemoveFollower();
-
+    void testReconfigFailureWithoutAuth();
+    void testReconfigFailureWithoutServerSuperuserPasswordConfigured();
   private:
     static const uint32_t NUM_SERVERS;
     FILE* logfile_;
@@ -49,6 +55,7 @@ class TestReconfigServer : public CPPUNIT_NS::TestFixture {
     void parseConfig(char* buf, int len, std::vector<std::string>& servers,
                      std::string& version);
     bool waitForConnected(zhandle_t* zh, uint32_t timeout_sec);
+    zhandle_t* connectFollowers(std::vector<int32_t> &followers);
 };
 
 const uint32_t TestReconfigServer::NUM_SERVERS = 3;
@@ -70,7 +77,10 @@ TestReconfigServer::
 
 void TestReconfigServer::
 setUp() {
-    cluster_ = ZooKeeperQuorumServer::getCluster(NUM_SERVERS);
+    ZooKeeperQuorumServer::tConfigPairs configs;
+    configs.push_back(std::make_pair("reconfigEnabled", "true"));
+    cluster_ = ZooKeeperQuorumServer::getCluster(NUM_SERVERS, configs,
+        "SERVER_JVMFLAGS=-Dzookeeper.DigestAuthenticationProvider.superDigest=super:D/InIHSb7yEEbrWz8b9l71RjZJU="/* password is test */);
 }
 
 void TestReconfigServer::
@@ -151,7 +161,7 @@ testRemoveFollower() {
     zhandle_t* zk = zookeeper_init(host.c_str(), NULL, 10000, NULL, NULL, 0);
     CPPUNIT_ASSERT_EQUAL(true, waitForConnected(zk, 10));
     CPPUNIT_ASSERT_EQUAL((int)ZOK, zoo_getconfig(zk, 0, buf, &len, &stat));
-
+    CPPUNIT_ASSERT_EQUAL((int)ZOK, zoo_add_auth(zk, "digest", "super:test", 10, NULL,(void*)ZOK));
     // check if all the servers are listed in the config.
     parseConfig(buf, len, servers, version);
     // initially should be 1<<32, which is 0x100000000. This is the zxid
@@ -219,6 +229,7 @@ testNonIncremental() {
     zhandle_t* zk = zookeeper_init(host.c_str(), NULL, 10000, NULL, NULL, 0);
     CPPUNIT_ASSERT_EQUAL(true, waitForConnected(zk, 10));
     CPPUNIT_ASSERT_EQUAL((int)ZOK, zoo_getconfig(zk, 0, buf, &len, &stat));
+    CPPUNIT_ASSERT_EQUAL((int)ZOK, zoo_add_auth(zk, "digest", "super:test", 10, NULL,(void*)ZOK));
 
     // check if all the servers are listed in the config.
     parseConfig(buf, len, servers, version);
@@ -274,37 +285,46 @@ testNonIncremental() {
     zookeeper_close(zk);
 }
 
-/**
- * 1. Connect to a follower.
- * 2. Remove the follower the client is connected to.
- */
-void TestReconfigServer::
-testRemoveConnectedFollower() {
-    std::vector<std::string> servers;
-    std::string version;
-    struct Stat stat;
-    int len = 1024;
-    char buf[len];
-
-    // connect to a follower.
+zhandle_t* TestReconfigServer::
+connectFollowers(std::vector<int32_t> &followers) {
+    std::stringstream ss;
     int32_t leader = getLeader();
-    std::vector<int32_t> followers = getFollowers();
     CPPUNIT_ASSERT(leader >= 0);
     CPPUNIT_ASSERT_EQUAL(NUM_SERVERS - 1, (uint32_t)(followers.size()));
-    std::stringstream ss;
     for (int i = 0; i < followers.size(); i++) {
-      ss << cluster_[followers[i]]->getHostPort() << ",";
+        ss << cluster_[followers[i]]->getHostPort() << ",";
     }
     ss << cluster_[leader]->getHostPort();
     std::string hosts = ss.str().c_str();
     zoo_deterministic_conn_order(true);
     zhandle_t* zk = zookeeper_init(hosts.c_str(), NULL, 10000, NULL, NULL, 0);
     CPPUNIT_ASSERT_EQUAL(true, waitForConnected(zk, 10));
+
     std::string connectedHost(zoo_get_current_server(zk));
     std::string portString = connectedHost.substr(connectedHost.find(":") + 1);
     uint32_t port;
     std::istringstream (portString) >> port;
     CPPUNIT_ASSERT_EQUAL(cluster_[followers[0]]->getClientPort(), port);
+    return zk;
+}
+
+/**
+ * 1. Connect to a follower.
+ * 2. Remove the follower the client is connected to.
+ */
+void TestReconfigServer::
+testRemoveConnectedFollower() {
+    std::vector<std::string> servers;
+    std::string version;
+    struct Stat stat;
+    int len = 1024;
+    char buf[len];
+
+    // connect to a follower.
+    std::stringstream ss;
+    std::vector<int32_t> followers = getFollowers();
+    zhandle_t* zk = connectFollowers(followers);
+    CPPUNIT_ASSERT_EQUAL((int)ZOK, zoo_add_auth(zk, "digest", "super:test", 10, NULL,(void*)ZOK));
 
     // remove the follower.
     len = 1024;
@@ -324,4 +344,77 @@ testRemoveConnectedFollower() {
     zookeeper_close(zk);
 }
 
+/**
+ * ZOOKEEPER-2014: only admin or users who are explicitly granted permission can do reconfig.
+ */
+void TestReconfigServer::
+testReconfigFailureWithoutAuth() {
+    std::vector<std::string> servers;
+    std::string version;
+    struct Stat stat;
+    int len = 1024;
+    char buf[len];
+
+    // connect to a follower.
+    std::stringstream ss;
+    std::vector<int32_t> followers = getFollowers();
+    zhandle_t* zk = connectFollowers(followers);
+
+    // remove the follower.
+    len = 1024;
+    ss.str("");
+    ss << followers[0];
+    // No auth, should fail.
+    CPPUNIT_ASSERT_EQUAL((int)ZNOAUTH, zoo_reconfig(zk, NULL, ss.str().c_str(), NULL, -1, buf, &len, &stat));
+    // Wrong auth, should fail.
+    CPPUNIT_ASSERT_EQUAL((int)ZOK, zoo_add_auth(zk, "digest", "super:wrong", 11, NULL,(void*)ZOK));
+    CPPUNIT_ASSERT_EQUAL((int)ZNOAUTH, zoo_reconfig(zk, NULL, ss.str().c_str(), NULL, -1, buf, &len, &stat));
+    // Right auth, should pass.
+    CPPUNIT_ASSERT_EQUAL((int)ZOK, zoo_add_auth(zk, "digest", "super:test", 10, NULL,(void*)ZOK));
+    CPPUNIT_ASSERT_EQUAL((int)ZOK, zoo_reconfig(zk, NULL, ss.str().c_str(), NULL, -1, buf, &len, &stat));
+    CPPUNIT_ASSERT_EQUAL((int)ZOK, zoo_getconfig(zk, 0, buf, &len, &stat));
+    parseConfig(buf, len, servers, version);
+    CPPUNIT_ASSERT_EQUAL(NUM_SERVERS - 1, (uint32_t)(servers.size()));
+    for (int i = 0; i < cluster_.size(); i++) {
+        if (i == followers[0]) {
+            continue;
+        }
+        CPPUNIT_ASSERT(std::find(servers.begin(), servers.end(),
+                       cluster_[i]->getServerString()) != servers.end());
+    }
+    zookeeper_close(zk);
+}
+
+void TestReconfigServer::
+testReconfigFailureWithoutServerSuperuserPasswordConfigured() {
+    std::vector<std::string> servers;
+    std::string version;
+    struct Stat stat;
+    int len = 1024;
+    char buf[len];
+
+    // Create a new quorum with the super user's password not configured.
+    tearDown();
+    ZooKeeperQuorumServer::tConfigPairs configs;
+    configs.push_back(std::make_pair("reconfigEnabled", "true"));
+    cluster_ = ZooKeeperQuorumServer::getCluster(NUM_SERVERS, configs, "");
+
+    // connect to a follower.
+    std::stringstream ss;
+    std::vector<int32_t> followers = getFollowers();
+    zhandle_t* zk = connectFollowers(followers);
+
+    // remove the follower.
+    len = 1024;
+    ss.str("");
+    ss << followers[0];
+    // All cases should fail as server ensemble was not configured with the super user's password.
+    CPPUNIT_ASSERT_EQUAL((int)ZNOAUTH, zoo_reconfig(zk, NULL, ss.str().c_str(), NULL, -1, buf, &len, &stat));
+    CPPUNIT_ASSERT_EQUAL((int)ZOK, zoo_add_auth(zk, "digest", "super:", 11, NULL,(void*)ZOK));
+    CPPUNIT_ASSERT_EQUAL((int)ZNOAUTH, zoo_reconfig(zk, NULL, ss.str().c_str(), NULL, -1, buf, &len, &stat));
+    CPPUNIT_ASSERT_EQUAL((int)ZOK, zoo_add_auth(zk, "digest", "super:test", 10, NULL,(void*)ZOK));
+    CPPUNIT_ASSERT_EQUAL((int)ZNOAUTH, zoo_reconfig(zk, NULL, ss.str().c_str(), NULL, -1, buf, &len, &stat));
+    zookeeper_close(zk);
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(TestReconfigServer);
diff --git a/src/c/tests/ZooKeeperQuorumServer.cc b/src/c/tests/ZooKeeperQuorumServer.cc
@@ -21,18 +21,21 @@
 #include <cstdlib>
 #include <fstream>
 #include <sstream>
+#include <vector>
+#include <utility>
 #include <unistd.h>
 
 ZooKeeperQuorumServer::
-ZooKeeperQuorumServer(uint32_t id, uint32_t numServers) :
+ZooKeeperQuorumServer(uint32_t id, uint32_t numServers, std::string config, std::string env) :
     id_(id),
+    env_(env),
     numServers_(numServers) {
     const char* root = getenv("ZKROOT");
     if (root == NULL) {
         assert(!"Environment variable 'ZKROOT' is not set");
     }
     root_ = root;
-    createConfigFile();
+    createConfigFile(config);
     createDataDirectory();
     start();
 }
@@ -58,6 +61,9 @@ void ZooKeeperQuorumServer::
 start() {
     std::string command = root_ + "/bin/zkServer.sh start " +
                           getConfigFileName();
+    if (!env_.empty()) {
+        command = env_ + " " + command;
+    }
     assert(system(command.c_str()) == 0);
 }
 
@@ -102,7 +108,7 @@ isFollower() {
 }
 
 void ZooKeeperQuorumServer::
-createConfigFile() {
+createConfigFile(std::string config) {
     std::string command = "mkdir -p " + root_ + "/build/test/test-cppunit/conf";
     assert(system(command.c_str()) == 0);
     std::ofstream confFile;
@@ -118,6 +124,10 @@ createConfigFile() {
     for (int i = 0; i < numServers_; i++) {
         confFile << getServerString(i) << "\n";
     }
+    // Append additional config, if any.
+    if (!config.empty()) {
+      confFile << config << std::endl;
+    }
     confFile.close();
 }
 
@@ -188,3 +198,33 @@ getCluster(uint32_t numServers) {
     }
     assert(!"The cluster didn't start for 10 seconds");
 }
+
+std::vector<ZooKeeperQuorumServer*> ZooKeeperQuorumServer::
+getCluster(uint32_t numServers, ZooKeeperQuorumServer::tConfigPairs configs, std::string env) {
+    std::vector<ZooKeeperQuorumServer*> cluster;
+    std::string config;
+    for (ZooKeeperQuorumServer::tConfigPairs::const_iterator iter = configs.begin(); iter != configs.end(); ++iter) {
+        std::pair<std::string, std::string> pair = *iter;
+        config += (pair.first + "=" + pair.second + "\n");
+    }
+    for (int i = 0; i < numServers; i++) {
+        cluster.push_back(new ZooKeeperQuorumServer(i, numServers, config, env));
+    }
+
+    // Wait until all the servers start, and fail if they don't start within 10
+    // seconds.
+    for (int i = 0; i < 10; i++) {
+        int j = 0;
+        for (; j < cluster.size(); j++) {
+            if (cluster[j]->getMode() == "") {
+                // The server hasn't started.
+                sleep(1);
+                break;
+            }
+        }
+        if (j == cluster.size()) {
+            return cluster;
+        }
+    }
+    assert(!"The cluster didn't start for 10 seconds");
+}
diff --git a/src/c/tests/ZooKeeperQuorumServer.h b/src/c/tests/ZooKeeperQuorumServer.h
@@ -20,11 +20,16 @@
 #include <stdint.h>
 #include <string>
 #include <vector>
+#include <utility>
 
 class ZooKeeperQuorumServer {
   public:
     ~ZooKeeperQuorumServer();
+    typedef std::vector<std::pair<std::string, std::string> > tConfigPairs;
     static std::vector<ZooKeeperQuorumServer*> getCluster(uint32_t numServers);
+    static std::vector<ZooKeeperQuorumServer*> getCluster(uint32_t numServers,
+        tConfigPairs configs, /* Additional config options as a list of key/value pairs. */
+        std::string env       /* Additional environment variables when starting zkServer.sh. */);
     std::string getHostPort();
     uint32_t getClientPort();
     void start();
@@ -35,10 +40,11 @@ class ZooKeeperQuorumServer {
 
   private:
     ZooKeeperQuorumServer();
-    ZooKeeperQuorumServer(uint32_t id, uint32_t numServers);
+    ZooKeeperQuorumServer(uint32_t id, uint32_t numServers, std::string config = "",
+                          std::string env = "");
     ZooKeeperQuorumServer(const ZooKeeperQuorumServer& that);
     const ZooKeeperQuorumServer& operator=(const ZooKeeperQuorumServer& that);
-    void createConfigFile();
+    void createConfigFile(std::string config = "");
     std::string getConfigFileName();
     void createDataDirectory();
     std::string getDataDirectory();
@@ -52,6 +58,7 @@ class ZooKeeperQuorumServer {
     uint32_t numServers_;
     uint32_t id_;
     std::string root_;
+    std::string env_;
 };
 
 #endif  // ZOOKEEPER_QUORUM_SERVER_H