[GitHub] zookeeper pull request #96: ZOOKEEPER-2014: Only admin should be allowed to ...

[rgs1 <gi...@git.apache.org>]

Github user rgs1 commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/96#discussion_r86673398
  
    --- Diff: src/c/tests/TestReconfigServer.cc ---
    @@ -324,4 +336,109 @@ 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.
    +    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_[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);
    +
    +    // 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.
    +    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_[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);
    --- End diff --
    
    connect to follower seems repeated, can we move it to a private helper method?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---