You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zookeeper.apache.org by ha...@apache.org on 2018/09/10 22:17:55 UTC

zookeeper git commit: ZOOKEEPER-2261: When only secureClientPort is configured connections, configuration, connection_stat_reset, and stats admin commands throw NullPointerException

Repository: zookeeper
Updated Branches:
  refs/heads/master 181de25c0 -> 4ad2341c1


ZOOKEEPER-2261: When only secureClientPort is configured connections, configuration, connection_stat_reset, and stats admin commands throw NullPointerException

Root cause of the issue is that property getter returns the non-secure ServerCnxnFactory instance always. When Quorum SSL is enabled, we set a separate field which is the secure instance.

Property getter should detect the scenario and return the proper instance.

First commit contains some refactoring: shuffling the existing ZooKeeperServer tests to relevant places.
Second commit is the actual fix + new unit tests.

Sorry about indentation changes, but `FileTxnLogTest.java` was indented by 2 spaces instead of 4.

Author: Andor Molnar <an...@apache.org>
Author: Andor Molnar <an...@cloudera.com>

Reviewers: breed, hanm, nkalmar

Closes #545 from anmolnar/ZOOKEEPER-2261


Project: http://git-wip-us.apache.org/repos/asf/zookeeper/repo
Commit: http://git-wip-us.apache.org/repos/asf/zookeeper/commit/4ad2341c
Tree: http://git-wip-us.apache.org/repos/asf/zookeeper/tree/4ad2341c
Diff: http://git-wip-us.apache.org/repos/asf/zookeeper/diff/4ad2341c

Branch: refs/heads/master
Commit: 4ad2341c18020ecb08e7e26b79cacc3569b9d31e
Parents: 181de25
Author: Andor Molnar <an...@apache.org>
Authored: Mon Sep 10 15:17:45 2018 -0700
Committer: Michael Han <ha...@apache.org>
Committed: Mon Sep 10 15:17:45 2018 -0700

----------------------------------------------------------------------
 .../zookeeper/server/ZooKeeperServer.java       |  4 ++++
 .../apache/zookeeper/server/admin/Commands.java | 16 ++++++++++++-
 .../zookeeper/server/admin/CommandsTest.java    | 25 ++++++++++++++++++--
 3 files changed, 42 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zookeeper/blob/4ad2341c/src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java b/src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
index 64f242f..2c9e5e7 100644
--- a/src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
+++ b/src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
@@ -869,6 +869,10 @@ public class ZooKeeperServer implements SessionExpirer, ServerStats.Provider {
         return serverCnxnFactory;
     }
 
+    public ServerCnxnFactory getSecureServerCnxnFactory() {
+        return secureServerCnxnFactory;
+    }
+
     public void setSecureServerCnxnFactory(ServerCnxnFactory factory) {
         secureServerCnxnFactory = factory;
     }

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/4ad2341c/src/java/main/org/apache/zookeeper/server/admin/Commands.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/server/admin/Commands.java b/src/java/main/org/apache/zookeeper/server/admin/Commands.java
index fefccf3..3f37246 100644
--- a/src/java/main/org/apache/zookeeper/server/admin/Commands.java
+++ b/src/java/main/org/apache/zookeeper/server/admin/Commands.java
@@ -18,7 +18,9 @@
 
 package org.apache.zookeeper.server.admin;
 
+import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -29,6 +31,7 @@ import org.apache.zookeeper.Environment;
 import org.apache.zookeeper.Environment.Entry;
 import org.apache.zookeeper.Version;
 import org.apache.zookeeper.server.DataTree;
+import org.apache.zookeeper.server.ServerCnxnFactory;
 import org.apache.zookeeper.server.ServerStats;
 import org.apache.zookeeper.server.ZKDatabase;
 import org.apache.zookeeper.server.ZooKeeperServer;
@@ -174,7 +177,18 @@ public class Commands {
         @Override
         public CommandResponse run(ZooKeeperServer zkServer, Map<String, String> kwargs) {
             CommandResponse response = initializeResponse();
-            response.put("connections", zkServer.getServerCnxnFactory().getAllConnectionInfo(false));
+            ServerCnxnFactory serverCnxnFactory = zkServer.getServerCnxnFactory();
+            if (serverCnxnFactory != null) {
+                response.put("connections", serverCnxnFactory.getAllConnectionInfo(false));
+            } else {
+                response.put("connections", Collections.emptyList());
+            }
+            ServerCnxnFactory secureServerCnxnFactory = zkServer.getSecureServerCnxnFactory();
+            if (secureServerCnxnFactory != null) {
+                response.put("secure_connections", secureServerCnxnFactory.getAllConnectionInfo(false));
+            } else {
+                response.put("secure_connections", Collections.emptyList());
+            }
             return response;
         }
     }

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/4ad2341c/src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java
----------------------------------------------------------------------
diff --git a/src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java b/src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java
index aae788f..762547f 100644
--- a/src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java
+++ b/src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java
@@ -18,15 +18,19 @@
 
 package org.apache.zookeeper.server.admin;
 
+import static org.hamcrest.core.Is.is;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 import java.io.IOException;
-import java.nio.Buffer;
 import java.util.HashMap;
 import java.util.Map;
 
+import org.apache.zookeeper.server.ServerCnxnFactory;
 import org.apache.zookeeper.server.ServerStats;
 import org.apache.zookeeper.server.ZooKeeperServer;
 import org.apache.zookeeper.server.quorum.BufferStats;
@@ -108,7 +112,9 @@ public class CommandsTest extends ClientBase {
     @Test
     public void testConnections() throws IOException, InterruptedException {
         testCommand("connections",
-                    new Field("connections", Iterable.class));
+                    new Field("connections", Iterable.class),
+                    new Field("secure_connections", Iterable.class)
+                );
     }
 
     @Test
@@ -240,4 +246,19 @@ public class CommandsTest extends ClientBase {
                     new Field("num_total_watches", Integer.class));
     }
 
+    @Test
+    public void testConsCommandSecureOnly() {
+        // Arrange
+        Commands.ConsCommand cmd = new Commands.ConsCommand();
+        ZooKeeperServer zkServer = mock(ZooKeeperServer.class);
+        ServerCnxnFactory cnxnFactory = mock(ServerCnxnFactory.class);
+        when(zkServer.getSecureServerCnxnFactory()).thenReturn(cnxnFactory);
+
+        // Act
+        CommandResponse response = cmd.run(zkServer, null);
+
+        // Assert
+        assertThat(response.toMap().containsKey("connections"), is(true));
+        assertThat(response.toMap().containsKey("secure_connections"), is(true));
+    }
 }