Merge pull request #10503 from vespa-engine/hmusum/remove-RestrictedServerCnxnFactory

Do not use RestrictedServerCnxnFactory

9 files changed, 7 insertions, 108 deletions

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 84ca6416269..675ec22dc2b 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,7 +4,6 @@ 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;
@@ -25,11 +24,9 @@ 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, DeployState deployState) {
+    public ClusterControllerCluster(AbstractConfigProducer parent, String subId) {
         super(parent, subId);
-        this.deployState = deployState;
     }
 
     @Override
@@ -41,7 +38,6 @@ 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 cd49e16dc56..1b0e04b50a8 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", deployState);
+            parent = new ClusterControllerCluster(parent, "standalone");
         }
         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 205489ca0fc..2eff081f42d 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,8 +78,6 @@ 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 f6cdf63232d..fc79a3f4bbf 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,7 @@ 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", context.getDeployState()),
+                clusterControllers = createClusterControllers(new ClusterControllerCluster(contentCluster, "standalone"),
                                                               hosts,
                                                               clusterName,
                                                               true,
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 35cbec015de..3817288d44b 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
@@ -212,7 +212,6 @@ public class ClusterControllerTestCase extends DomBuilderTest {
         assertThat(config.myid(), is(id));
         Collection<Integer> serverIds = Collections2.transform(config.server(), ZookeeperServerConfig.Server::id);
         assertTrue(serverIds.contains(id));
-        assertTrue(config.useRestrictedServerCnxnFactory());
     }
 
     @Test
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 d82859e4df2..b4d889a4598 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,7 +59,6 @@ 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
@@ -70,7 +69,6 @@ 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());
-        assertTrue(config.useRestrictedServerCnxnFactory());
     }
 
     @Test(expected = IllegalArgumentException.class)
@@ -124,13 +122,12 @@ public class ConfigserverClusterTest {
 
     @SuppressWarnings("varargs")
     private static <T> void assertZookeeperServerProperty(
-            List<ZookeeperServerConfig.Server> zkServers, Function<ZookeeperServerConfig.Server, T> properyMapper, T... expectedProperties) {
-        List<T> actualPropertyValues = zkServers.stream().map(properyMapper).collect(Collectors.toList());
+            List<ZookeeperServerConfig.Server> zkServers, Function<ZookeeperServerConfig.Server, T> propertyMapper, T... expectedProperties) {
+        List<T> actualPropertyValues = zkServers.stream().map(propertyMapper).collect(Collectors.toList());
         List<T> expectedPropertyValues = Arrays.asList(expectedProperties);
         assertEquals(expectedPropertyValues, actualPropertyValues);
     }
 
-
     private static TestOptions createTestOptions(List<String> configServerHostnames, List<Integer> configServerZkIds) {
         return createTestOptions(configServerHostnames, configServerZkIds, true);
 
diff --git a/configdefinitions/src/vespa/zookeeper-server.def b/configdefinitions/src/vespa/zookeeper-server.def
index 99a45f9ff66..723246daab0 100644
--- a/configdefinitions/src/vespa/zookeeper-server.def
+++ b/configdefinitions/src/vespa/zookeeper-server.def
@@ -6,7 +6,8 @@ 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
+# TODO: Remove when Vespa 7.103 is the oldest 7 version in use
+useRestrictedServerCnxnFactory bool default=false
 
 
 # For more info about the values below, see ZooKeeper documentation
diff --git a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java
deleted file mode 100644
index dab9ddb243b..00000000000
--- a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java
+++ /dev/null
@@ -1,90 +0,0 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.zookeeper;
-
-import com.google.common.collect.ImmutableSet;
-import com.yahoo.net.HostName;
-import com.yahoo.text.StringUtilities;
-import org.apache.zookeeper.server.NIOServerCnxn;
-import org.apache.zookeeper.server.NIOServerCnxnFactory;
-
-import java.io.IOException;
-import java.net.InetSocketAddress;
-import java.nio.channels.SelectionKey;
-import java.nio.channels.SocketChannel;
-import java.util.HashSet;
-import java.util.Set;
-import java.util.logging.Logger;
-
-/**
- * This class is created by zookeeper by reflection, see the ZooKeeperServer constructor. It will only work
- * when using ZooKeeper 3.4
- * 
- * @author bratseth
- */
-@SuppressWarnings("unused")
-public class RestrictedServerCnxnFactory extends NIOServerCnxnFactory {
-
-    private static final Logger log = Logger.getLogger(RestrictedServerCnxnFactory.class.getName());
-    
-    public RestrictedServerCnxnFactory() throws IOException {
-        super();
-    }
-    
-    @Override
-    protected NIOServerCnxn createConnection(SocketChannel socket, SelectionKey selection) throws IOException {
-        NIOServerCnxn ret = super.createConnection(socket, selection);
-        validateRemoteOrClose(socket);
-        return ret;
-    }
-
-    private void validateRemoteOrClose(SocketChannel socket) {
-        try {
-            String remoteHost = ((InetSocketAddress)socket.getRemoteAddress()).getHostName();
-
-            if (isLocalHost(remoteHost)) return; // always allow localhost
-
-            ImmutableSet<String> allowedZooKeeperClients = findAllowedZooKeeperClients();
-
-            if (allowedZooKeeperClients.isEmpty()) return; // inactive: allow all
-            if (allowedZooKeeperClients.contains(remoteHost)) return; // allowed
-
-            // Not allowed: Reject connection
-            String errorMessage = "Rejecting connection to ZooKeeper from " + remoteHost +
-                                  ": This cluster only allow connection from hosts in: " + allowedZooKeeperClients;
-            log.info(errorMessage);
-            socket.shutdownInput();
-            socket.shutdownOutput();
-        } catch (Exception e) {
-            log.warning("Unexpected exception: "+e);
-        }
-    }
-
-    /** Returns the allowed client host names. If the list is empty any host is allowed. */
-    private ImmutableSet<String> findAllowedZooKeeperClients() {
-        // Environment has precedence. Note that 
-        // - if this is set to "", client restriction is disabled
-        // - this environment variable is a public API - do not change
-        String environmentAllowedZooKeeperClients = System.getenv("vespa_zkfacade__restrict");
-        if (environmentAllowedZooKeeperClients != null) 
-            return ImmutableSet.copyOf(toHostnameSet(environmentAllowedZooKeeperClients));
-        else
-            return ImmutableSet.of();
-    }
-
-    private Set<String> toHostnameSet(String hostnamesString) {
-        Set<String> hostnames = new HashSet<>();
-        for (String hostname : StringUtilities.split(hostnamesString)) {
-            if ( ! hostname.trim().isEmpty())
-                hostnames.add(hostname.trim());
-        }
-        return hostnames;
-    }
-
-    private boolean isLocalHost(String remoteHost) {
-        if (remoteHost.equals("localhost")) return true;
-        if (remoteHost.equals("localhost.localdomain")) return true;
-        if (remoteHost.equals(HostName.getLocalhost())) return true;
-        return false;
-    }
-    
-}
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 5b78bc48a2d..8342dfb16df 100644
--- a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java
+++ b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java
@@ -31,8 +31,6 @@ 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());
-        if (zookeeperServerConfig.useRestrictedServerCnxnFactory())
-            System.setProperty("zookeeper.serverCnxnFactory", "com.yahoo.vespa.zookeeper.RestrictedServerCnxnFactory");
 
         writeConfigToDisk(zookeeperServerConfig);
         zkServerThread = new Thread(this, "zookeeper server");