Handle client port that is configured to TLS only

The client port can no longer be distributed through the ZK dynamic
reconfiguration as the protocol does not support SSL client port.
The port must be configured through the static config section instead.

2 files changed, 35 insertions, 21 deletions

diff --git a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Configurator.java b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Configurator.java
index aff4bb950f6..3c56741adfb 100644
--- a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Configurator.java
+++ b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Configurator.java
@@ -76,7 +76,7 @@ public class Configurator {
         sb.append("skipACL=yes").append("\n");
         sb.append("metricsProvider.className=org.apache.zookeeper.metrics.impl.NullMetricsProvider\n");
         ensureThisServerIsRepresented(config.myid(), config.server());
-        config.server().forEach(server -> addServerToCfg(sb, server, config.clientPort()));
+        config.server().forEach(server -> addServerToCfg(sb, server));
         sb.append(new TlsQuorumConfig().createConfig(config, tlsContext));
         sb.append(new TlsClientServerConfig().createConfig(config, tlsContext));
         return sb.toString();
@@ -101,7 +101,7 @@ public class Configurator {
         }
     }
 
-    private void addServerToCfg(StringBuilder sb, ZookeeperServerConfig.Server server, int clientPort) {
+    private void addServerToCfg(StringBuilder sb, ZookeeperServerConfig.Server server) {
         sb.append("server.")
           .append(server.id())
           .append("=")
@@ -119,9 +119,7 @@ public class Configurator {
             sb.append(":")
               .append("observer");
         }
-        sb.append(";")
-          .append(clientPort)
-          .append("\n");
+        sb.append("\n");
     }
 
     static List<String> zookeeperServerHostnames(ZookeeperServerConfig zookeeperServerConfig) {
@@ -176,21 +174,28 @@ public class Configurator {
 
             StringBuilder sb = new StringBuilder();
             boolean portUnification;
+            boolean secureClientPort;
             switch (tlsSetting) {
                 case "OFF":
+                    secureClientPort = false; portUnification = false;
+                    break;
                 case "TLS_ONLY":
-                    portUnification = false;
+                    secureClientPort = true; portUnification = false;
                     break;
                 case "PORT_UNIFICATION":
                 case "TLS_WITH_PORT_UNIFICATION":
-                    portUnification = true;
+                    secureClientPort = false; portUnification = true;
                     break;
                 default:
                     throw new IllegalArgumentException("Unknown value of config setting tlsForClientServerCommunication: " + tlsSetting);
             }
-            sb.append("client.portUnification=").append(portUnification).append("\n");
-            // TODO This should override "clientPort" if TLS enabled without port unification);
-            tlsContext.ifPresent(ctx -> sb.append("secureClientPort=").append(config.secureClientPort()).append("\n"));
+            // ZooKeeper Dynamic Reconfiguration does not support SSL/secure client port
+            // The secure client port must be configured in the static configuration section instead
+            // https://issues.apache.org/jira/browse/ZOOKEEPER-3577
+            sb.append("client.portUnification=").append(portUnification).append("\n")
+                    .append("clientPort=").append(secureClientPort ? 0 : config.clientPort()).append("\n")
+                    .append("secureClientPort=").append(secureClientPort ? config.clientPort() : 0).append("\n");
+
             appendTlsConfig(sb, config, tlsContext);
 
             return sb.toString();
diff --git a/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ConfiguratorTest.java b/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ConfiguratorTest.java
index aee44c90cbf..147b61a804c 100644
--- a/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ConfiguratorTest.java
+++ b/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ConfiguratorTest.java
@@ -182,10 +182,12 @@ public class ConfiguratorTest {
     private void validateConfigFileSingleHost(File cfgFile) {
         String expected =
                 commonConfig() +
-                "server.0=foo:321:123;2181\n" +
+                "server.0=foo:321:123\n" +
                 "sslQuorum=false\n" +
                 "portUnification=false\n" +
-                "client.portUnification=false\n";
+                "client.portUnification=false\n" +
+                "clientPort=2181\n" +
+                "secureClientPort=0\n";
         validateConfigFile(cfgFile, expected);
     }
 
@@ -198,8 +200,7 @@ public class ConfiguratorTest {
     }
 
     private String tlsClientServerConfig() {
-        return "secureClientPort=2184\n" +
-                "ssl.context.supplier.class=com.yahoo.vespa.zookeeper.VespaSslContextProvider\n" +
+        return "ssl.context.supplier.class=com.yahoo.vespa.zookeeper.VespaSslContextProvider\n" +
                 "ssl.ciphersuites=TLS_AES_128_GCM_SHA256,TLS_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256," +
                 "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384\n" +
                 "ssl.enabledProtocols=TLSv1.2\n" +
@@ -209,23 +210,27 @@ public class ConfiguratorTest {
     private void validateConfigFileMultipleHosts(File cfgFile) {
         String expected =
                 commonConfig() +
-                "server.0=foo:321:123;2181\n" +
-                "server.1=bar:432:234;2181\n" +
-                "server.2=baz:543:345:observer;2181\n" +
+                "server.0=foo:321:123\n" +
+                "server.1=bar:432:234\n" +
+                "server.2=baz:543:345:observer\n" +
                 "sslQuorum=false\n" +
                 "portUnification=false\n" +
-                "client.portUnification=false\n";
+                "client.portUnification=false\n" +
+                "clientPort=2181\n" +
+                "secureClientPort=0\n";
         validateConfigFile(cfgFile, expected);
     }
 
     private void validateConfigFilePortUnification(File cfgFile) {
         String expected =
                 commonConfig() +
-                "server.0=foo:321:123;2181\n" +
+                "server.0=foo:321:123\n" +
                 "sslQuorum=false\n" +
                 "portUnification=true\n" +
                 tlsQuorumConfig() +
                 "client.portUnification=true\n" +
+                "clientPort=2181\n" +
+                "secureClientPort=0\n" +
                 tlsClientServerConfig();
         validateConfigFile(cfgFile, expected);
     }
@@ -233,11 +238,13 @@ public class ConfiguratorTest {
     private void validateConfigFileTlsWithPortUnification(File cfgFile) {
         String expected =
                 commonConfig() +
-                "server.0=foo:321:123;2181\n" +
+                "server.0=foo:321:123\n" +
                 "sslQuorum=true\n" +
                 "portUnification=true\n" +
                 tlsQuorumConfig() +
                 "client.portUnification=true\n" +
+                "clientPort=2181\n" +
+                "secureClientPort=0\n" +
                 tlsClientServerConfig();
         validateConfigFile(cfgFile, expected);
     }
@@ -245,11 +252,13 @@ public class ConfiguratorTest {
     private void validateConfigFileTlsOnly(File cfgFile) {
         String expected =
                 commonConfig() +
-                "server.0=foo:321:123;2181\n" +
+                "server.0=foo:321:123\n" +
                 "sslQuorum=true\n" +
                 "portUnification=false\n" +
                 tlsQuorumConfig() +
                 "client.portUnification=false\n" +
+                "clientPort=0\n" +
+                "secureClientPort=2181\n" +
                 tlsClientServerConfig();
         validateConfigFile(cfgFile, expected);
     }