From commits-return-8045-archive-asf-public=cust-asf.ponee.io@zookeeper.apache.org Tue Dec 3 09:01:32 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id C1093180629 for ; Tue, 3 Dec 2019 10:01:31 +0100 (CET) Received: (qmail 62037 invoked by uid 500); 3 Dec 2019 09:01:30 -0000 Mailing-List: contact commits-help@zookeeper.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@zookeeper.apache.org Delivered-To: mailing list commits@zookeeper.apache.org Received: (qmail 62024 invoked by uid 99); 3 Dec 2019 09:01:30 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 03 Dec 2019 09:01:30 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id 8968E8D809; Tue, 3 Dec 2019 09:01:30 +0000 (UTC) Date: Tue, 03 Dec 2019 09:01:30 +0000 To: "commits@zookeeper.apache.org" Subject: [zookeeper] branch master updated: ZOOKEEPER-3633: AdminServer commands throw NPE when only secure client port is used MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Message-ID: <157536369039.19623.13387646776099049611@gitbox.apache.org> From: nkalmar@apache.org X-Git-Host: gitbox.apache.org X-Git-Repo: zookeeper X-Git-Refname: refs/heads/master X-Git-Reftype: branch X-Git-Oldrev: 815c8f2130b8b43e11abe52b226707f707a93581 X-Git-Newrev: 01e198aec9ceae52f160ddcf2e45cd234823e505 X-Git-Rev: 01e198aec9ceae52f160ddcf2e45cd234823e505 X-Git-NotificationType: ref_changed_plus_diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated This is an automated email from the ASF dual-hosted git repository. nkalmar pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/zookeeper.git The following commit(s) were added to refs/heads/master by this push: new 01e198a ZOOKEEPER-3633: AdminServer commands throw NPE when only secure client port is used 01e198a is described below commit 01e198aec9ceae52f160ddcf2e45cd234823e505 Author: Mate Szalay-Beko AuthorDate: Tue Dec 3 10:01:22 2019 +0100 ZOOKEEPER-3633: AdminServer commands throw NPE when only secure client port is used When only secureClientPort is defined in the config and there is no regular clientPort, then both the stat and the conf commands on the AdminServer result in 500 Server Error caused by NullPointerExceptions. The problem is that no serverCnxFactory is defined in the ZooKeeperServer in this case, we have only secureServerCnxnFactory. In the fix we return info about both the secure and unsecure connections. Example of the stat command output for secure-only configuration: ``` { "version" : "3.6.0-SNAPSHOT-8e8905069f4bff670c0492fe9e28ced0f86bca00, built on 11/29/2019 08:04 GMT", "read_only" : false, "server_stats" : { "packets_sent" : 1, "packets_received" : 1, "fsync_threshold_exceed_count" : 0, "client_response_stats" : { "last_buffer_size" : -1, "min_buffer_size" : -1, "max_buffer_size" : -1 }, "data_dir_size" : 671094270, "log_dir_size" : 671094270, "last_processed_zxid" : 20, "outstanding_requests" : 0, "server_state" : "standalone", "avg_latency" : 5.0, "max_latency" : 5, "min_latency" : 5, "num_alive_client_connections" : 1, "provider_null" : false, "uptime" : 15020 }, "client_response" : { "last_buffer_size" : -1, "min_buffer_size" : -1, "max_buffer_size" : -1 }, "node_count" : 6, "connections" : [ ], "secure_connections" : [ { "remote_socket_address" : "127.0.0.1:57276", "interest_ops" : 1, "outstanding_requests" : 0, "packets_received" : 1, "packets_sent" : 1 } ], "command" : "stats", "error" : null } ``` Author: Mate Szalay-Beko Reviewers: Andor Molnar , Enrico Olivelli , Norbert Kalmar Closes #1161 from symat/ZOOKEEPER-3633 --- .../apache/zookeeper/server/ZooKeeperServer.java | 2 +- .../apache/zookeeper/server/admin/Commands.java | 17 ++++++++++- .../zookeeper/server/admin/CommandsTest.java | 33 +++++++++++++++++++++- 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java index 65be888..1c9dda7 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java @@ -414,7 +414,7 @@ public class ZooKeeperServer implements SessionExpirer, ServerStats.Provider { zkDb.snapLog.getSnapDir().getAbsolutePath(), zkDb.snapLog.getDataDir().getAbsolutePath(), getTickTime(), - serverCnxnFactory.getMaxClientCnxnsPerHost(), + getMaxClientCnxnsPerHost(), getMinSessionTimeout(), getMaxSessionTimeout(), getServerId(), diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java index dbcdabf..7bb7c42 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java @@ -577,7 +577,22 @@ public class Commands { @Override public CommandResponse run(ZooKeeperServer zkServer, Map kwargs) { CommandResponse response = super.run(zkServer, kwargs); - response.put("connections", zkServer.getServerCnxnFactory().getAllConnectionInfo(true)); + + final Iterable> connections; + if (zkServer.getServerCnxnFactory() != null) { + connections = zkServer.getServerCnxnFactory().getAllConnectionInfo(true); + } else { + connections = Collections.emptyList(); + } + response.put("connections", connections); + + final Iterable> secureConnections; + if (zkServer.getSecureServerCnxnFactory() != null) { + secureConnections = zkServer.getSecureServerCnxnFactory().getAllConnectionInfo(true); + } else { + secureConnections = Collections.emptyList(); + } + response.put("secure_connections", secureConnections); return response; } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/admin/CommandsTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/admin/CommandsTest.java index aa7d113..5f6879e 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/admin/CommandsTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/admin/CommandsTest.java @@ -33,6 +33,7 @@ import java.util.Map; import org.apache.zookeeper.metrics.MetricsUtils; import org.apache.zookeeper.server.ServerCnxnFactory; import org.apache.zookeeper.server.ServerStats; +import org.apache.zookeeper.server.ZKDatabase; import org.apache.zookeeper.server.ZooKeeperServer; import org.apache.zookeeper.server.quorum.BufferStats; import org.apache.zookeeper.test.ClientBase; @@ -219,7 +220,14 @@ public class CommandsTest extends ClientBase { @Test public void testStat() throws IOException, InterruptedException { - testCommand("stats", new Field("version", String.class), new Field("read_only", Boolean.class), new Field("server_stats", ServerStats.class), new Field("node_count", Integer.class), new Field("connections", Iterable.class), new Field("client_response", BufferStats.class)); + testCommand("stats", + new Field("version", String.class), + new Field("read_only", Boolean.class), + new Field("server_stats", ServerStats.class), + new Field("node_count", Integer.class), + new Field("connections", Iterable.class), + new Field("secure_connections", Iterable.class), + new Field("client_response", BufferStats.class)); } @Test @@ -264,4 +272,27 @@ public class CommandsTest extends ClientBase { assertThat(response.toMap().containsKey("secure_connections"), is(true)); } + /** + * testing Stat command, when only SecureClientPort is defined by the user and there is no + * regular (non-SSL port) open. In this case zkServer.getServerCnxnFactory === null + * see: ZOOKEEPER-3633 + */ + @Test + public void testStatCommandSecureOnly() { + Commands.StatCommand cmd = new Commands.StatCommand(); + ZooKeeperServer zkServer = mock(ZooKeeperServer.class); + ServerCnxnFactory cnxnFactory = mock(ServerCnxnFactory.class); + ServerStats serverStats = mock(ServerStats.class); + ZKDatabase zkDatabase = mock(ZKDatabase.class); + when(zkServer.getSecureServerCnxnFactory()).thenReturn(cnxnFactory); + when(zkServer.serverStats()).thenReturn(serverStats); + when(zkServer.getZKDatabase()).thenReturn(zkDatabase); + when(zkDatabase.getNodeCount()).thenReturn(0); + + CommandResponse response = cmd.run(zkServer, null); + + assertThat(response.toMap().containsKey("connections"), is(true)); + assertThat(response.toMap().containsKey("secure_connections"), is(true)); + } + }