Do not use RestrictedServerCnxnFactory in self-hosted installations

9 files changed, 48 insertions, 18 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/Service.java b/config-model/src/main/java/com/yahoo/vespa/model/Service.java
index 7958718af11..25128c5ab14 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/Service.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/Service.java
@@ -7,7 +7,7 @@ import java.util.HashMap;
 import java.util.Optional;
 
 /**
- * Representation of a markProcessed which runs a service
+ * Representation of a process which runs a service
  *
  * @author gjoranv
  */
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerCluster.java
index 8ba85047057..84ca6416269 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerCluster.java
@@ -4,6 +4,7 @@ package com.yahoo.vespa.model.admin.clustercontroller;
 import com.google.common.base.Joiner;
 import com.yahoo.cloud.config.ZookeeperServerConfig;
 import com.yahoo.cloud.config.ZookeepersConfig;
+import com.yahoo.config.model.deploy.DeployState;
 import com.yahoo.config.model.producer.AbstractConfigProducer;
 import com.yahoo.vespa.model.Service;
 import com.yahoo.vespa.model.admin.Configserver;
@@ -17,7 +18,6 @@ import java.util.Collection;
  * to provide common configs to container components.
  *
  * @author Ulf Lilleengen
- * @since 5.6
  */
 public class ClusterControllerCluster extends AbstractConfigProducer<ClusterControllerContainerCluster> implements
         ZookeeperServerConfig.Producer,
@@ -25,9 +25,11 @@ public class ClusterControllerCluster extends AbstractConfigProducer<ClusterCont
 
     private static final int ZK_CLIENT_PORT = 2181;
     private ClusterControllerContainerCluster containerCluster = null;
+    private final DeployState deployState;
 
-    public ClusterControllerCluster(AbstractConfigProducer parent, String subId) {
+    public ClusterControllerCluster(AbstractConfigProducer parent, String subId, DeployState deployState) {
         super(parent, subId);
+        this.deployState = deployState;
     }
 
     @Override
@@ -39,6 +41,7 @@ public class ClusterControllerCluster extends AbstractConfigProducer<ClusterCont
             serverBuilder.id(container.index());
             builder.server(serverBuilder);
         }
+        builder.useRestrictedServerCnxnFactory( ! deployState.isHosted());
     }
 
     @Override
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2Builder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2Builder.java
index 1b0e04b50a8..cd49e16dc56 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2Builder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2Builder.java
@@ -89,7 +89,7 @@ public class DomAdminV2Builder extends DomAdminBuilderBase {
 
         boolean standaloneZooKeeper = "true".equals(controllersElements.getAttribute(ATTRIBUTE_CLUSTER_CONTROLLER_STANDALONE_ZK)) || multitenant;
         if (standaloneZooKeeper) {
-            parent = new ClusterControllerCluster(parent, "standalone");
+            parent = new ClusterControllerCluster(parent, "standalone", deployState);
         }
         var cluster = new ClusterControllerContainerCluster(parent,
                                                             "cluster-controllers",
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/configserver/ConfigserverCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/configserver/ConfigserverCluster.java
index 2eff081f42d..205489ca0fc 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/configserver/ConfigserverCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/configserver/ConfigserverCluster.java
@@ -78,6 +78,8 @@ public class ConfigserverCluster extends AbstractConfigProducer
         if (options.zookeeperClientPort().isPresent()) {
             builder.clientPort(options.zookeeperClientPort().get());
         }
+        boolean hosted = options.hostedVespa().orElse(false);
+        builder.useRestrictedServerCnxnFactory( ! hosted);
     }
 
     @Override
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java
index 48779657162..87eff87592e 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java
@@ -311,7 +311,11 @@ public class ContentCluster extends AbstractConfigProducer implements
                 Collection<HostResource> hosts = nodesSpecification.isDedicated() ?
                                                  getControllerHosts(nodesSpecification, admin, clusterName, context) :
                                                  drawControllerHosts(nodesSpecification.count(), rootGroup, containers);
-                clusterControllers = createClusterControllers(new ClusterControllerCluster(contentCluster, "standalone"), hosts, clusterName, true, context.getDeployState());
+                clusterControllers = createClusterControllers(new ClusterControllerCluster(contentCluster, "standalone", context.getDeployState()),
+                                                              hosts,
+                                                              clusterName,
+                                                              true,
+                                                              context.getDeployState());
                 contentCluster.clusterControllers = clusterControllers;
             }
             else {
@@ -447,7 +451,8 @@ public class ContentCluster extends AbstractConfigProducer implements
 
         private ClusterControllerContainerCluster createClusterControllers(AbstractConfigProducer parent,
                                                                            Collection<HostResource> hosts,
-                                                                           String name, boolean multitenant,
+                                                                           String name,
+                                                                           boolean multitenant,
                                                                            DeployState deployState) {
             var clusterControllers = new ClusterControllerContainerCluster(parent, name, name, deployState);
             List<ClusterControllerContainer> containers = new ArrayList<>();
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/ClusterControllerTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/ClusterControllerTestCase.java
index c88d91a3ede..35cbec015de 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/admin/ClusterControllerTestCase.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/ClusterControllerTestCase.java
@@ -1,7 +1,6 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.model.admin;
 
-import com.google.common.base.Function;
 import com.google.common.collect.Collections2;
 import com.yahoo.cloud.config.ZookeeperServerConfig;
 import com.yahoo.cloud.config.ZookeepersConfig;
@@ -27,7 +26,10 @@ import java.util.Collection;
 import java.util.List;
 
 import static org.hamcrest.CoreMatchers.is;
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
 
 /**
  * Test for creating cluster controllers under the admin tag.
@@ -208,16 +210,11 @@ public class ClusterControllerTestCase extends DomBuilderTest {
         ZookeeperServerConfig config = new ZookeeperServerConfig(builder);
         assertThat(config.server().size(), is(3));
         assertThat(config.myid(), is(id));
-        Collection<Integer> serverIds = Collections2.transform(config.server(), new Function<ZookeeperServerConfig.Server, Integer>() {
-            @Override
-            public Integer apply(ZookeeperServerConfig.Server server) {
-                return server.id();
-            }
-        });
+        Collection<Integer> serverIds = Collections2.transform(config.server(), ZookeeperServerConfig.Server::id);
         assertTrue(serverIds.contains(id));
+        assertTrue(config.useRestrictedServerCnxnFactory());
     }
 
-
     @Test
     public void testUnconfigured() throws Exception {
         String xml = "<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n" +
@@ -435,7 +432,7 @@ public class ClusterControllerTestCase extends DomBuilderTest {
                                                   .withServices(servicesXml)
                                                   .withSearchDefinitions(sds)
                                                   .build());
-        SimpleApplicationValidator.checkServices(new StringReader(servicesXml), new Version(6));
+        SimpleApplicationValidator.checkServices(new StringReader(servicesXml), new Version(7));
         return model;
     }
 }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/configserver/ConfigserverClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/configserver/ConfigserverClusterTest.java
index 2a41f972eb3..d82859e4df2 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/configserver/ConfigserverClusterTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/configserver/ConfigserverClusterTest.java
@@ -59,6 +59,18 @@ public class ConfigserverClusterTest {
         assertZookeeperServerProperty(config.server(), ZookeeperServerConfig.Server::hostname, "cfg1", "localhost", "cfg3");
         assertZookeeperServerProperty(config.server(), ZookeeperServerConfig.Server::id, 4, 2, 3);
         assertEquals(2, config.myid());
+        assertFalse(config.useRestrictedServerCnxnFactory());
+    }
+
+    @Test
+    public void zookeeperConfig_self_hosted() {
+        final boolean hostedVespa = false;
+        TestOptions testOptions = createTestOptions(Arrays.asList("cfg1", "localhost", "cfg3"), Arrays.asList(4, 2, 3), hostedVespa);
+        ZookeeperServerConfig config = getConfig(ZookeeperServerConfig.class, testOptions);
+        assertZookeeperServerProperty(config.server(), ZookeeperServerConfig.Server::hostname, "cfg1", "localhost", "cfg3");
+        assertZookeeperServerProperty(config.server(), ZookeeperServerConfig.Server::id, 4, 2, 3);
+        assertEquals(2, config.myid());
+        assertTrue(config.useRestrictedServerCnxnFactory());
     }
 
     @Test(expected = IllegalArgumentException.class)
@@ -118,11 +130,16 @@ public class ConfigserverClusterTest {
         assertEquals(expectedPropertyValues, actualPropertyValues);
     }
 
+
     private static TestOptions createTestOptions(List<String> configServerHostnames, List<Integer> configServerZkIds) {
+        return createTestOptions(configServerHostnames, configServerZkIds, true);
+
+    }
+    private static TestOptions createTestOptions(List<String> configServerHostnames, List<Integer> configServerZkIds, boolean hostedVespa) {
         TestOptions testOptions = new TestOptions()
                 .rpcPort(12345)
                 .useVespaVersionInRequest(true)
-                .hostedVespa(true)
+                .hostedVespa(hostedVespa)
                 .environment("test")
                 .region("bar")
                 .numParallelTenantLoaders(99);
diff --git a/configdefinitions/src/vespa/zookeeper-server.def b/configdefinitions/src/vespa/zookeeper-server.def
index bd868504eba..99a45f9ff66 100644
--- a/configdefinitions/src/vespa/zookeeper-server.def
+++ b/configdefinitions/src/vespa/zookeeper-server.def
@@ -4,6 +4,11 @@ namespace=cloud.config
 # Vespa home is prepended if the file is relative
 zooKeeperConfigFile string default="conf/zookeeper/zookeeper.cfg"
 
+# Use a connection factory that is controlled by vespa_zkfacade__restrict environment variable
+# See RestrictedServerCnxnFactory for details
+useRestrictedServerCnxnFactory bool default=true
+
+
 # For more info about the values below, see ZooKeeper documentation
 
 # tick time in milliseconds
diff --git a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java
index 9c580b4f9ce..5b78bc48a2d 100644
--- a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java
+++ b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java
@@ -31,7 +31,8 @@ public class ZooKeeperServer extends AbstractComponent implements Runnable {
         this.zookeeperServerConfig = zookeeperServerConfig;
         System.setProperty("zookeeper.jmx.log4j.disable", "true");
         System.setProperty(ZOOKEEPER_JUTE_MAX_BUFFER, "" + zookeeperServerConfig.juteMaxBuffer());
-        System.setProperty("zookeeper.serverCnxnFactory", "com.yahoo.vespa.zookeeper.RestrictedServerCnxnFactory");
+        if (zookeeperServerConfig.useRestrictedServerCnxnFactory())
+            System.setProperty("zookeeper.serverCnxnFactory", "com.yahoo.vespa.zookeeper.RestrictedServerCnxnFactory");
 
         writeConfigToDisk(zookeeperServerConfig);
         zkServerThread = new Thread(this, "zookeeper server");