Only allow Zookeeper access for config servers in hosted Vespa

3 files changed, 28 insertions, 22 deletions

diff --git a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java
index a0c8b845aca..d7f42c7e6e9 100644
--- a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java
+++ b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java
@@ -71,9 +71,9 @@ public class RestrictedServerCnxnFactory extends NIOServerCnxnFactory {
         return ZooKeeperServer.getAllowedClientHostnames();
     }
 
-    private Set<String> toHostnameSet(String hosatnamesString) {
+    private Set<String> toHostnameSet(String hostnamesString) {
         Set<String> hostnames = new HashSet<>();
-        for (String hostname : StringUtilities.split(hosatnamesString)) {
+        for (String hostname : StringUtilities.split(hostnamesString)) {
             if ( ! hostname.trim().isEmpty())
                 hostnames.add(hostname.trim());
         }
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 74f9d01b833..aff798729a8 100644
--- a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java
+++ b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java
@@ -3,6 +3,7 @@ package com.yahoo.vespa.zookeeper;
 
 import com.google.common.collect.ImmutableSet;
 import com.google.inject.Inject;
+import com.yahoo.cloud.config.ConfigserverConfig;
 import com.yahoo.cloud.config.ZookeeperServerConfig;
 import com.yahoo.component.AbstractComponent;
 import com.yahoo.log.LogLevel;
@@ -10,16 +11,14 @@ import static com.yahoo.vespa.defaults.Defaults.getDefaults;
 
 import java.io.FileWriter;
 import java.io.IOException;
-import java.util.Collection;
 import java.util.List;
-import java.util.Optional;
+import java.util.Set;
 import java.util.stream.Collectors;
 
 /**
  * Writes zookeeper config and starts zookeeper server.
  *
- * @author lulf
- * @since 5.3
+ * @author Ulf Lilleengen
  */
 public class ZooKeeperServer extends AbstractComponent implements Runnable {
 
@@ -35,15 +34,16 @@ public class ZooKeeperServer extends AbstractComponent implements Runnable {
     private static final String ZOOKEEPER_JMX_LOG4J_DISABLE = "zookeeper.jmx.log4j.disable";
     static final String ZOOKEEPER_JUTE_MAX_BUFFER = "jute.maxbuffer";
     private final Thread zkServerThread;
-    private final ZookeeperServerConfig config;
+    private final ZookeeperServerConfig zookeeperServerConfig;
 
-    ZooKeeperServer(ZookeeperServerConfig config, boolean startServer) {
-        this.config = config;
+    ZooKeeperServer(ZookeeperServerConfig zookeeperServerConfig, ConfigserverConfig configserverConfig, boolean startServer) {
+        this.zookeeperServerConfig = zookeeperServerConfig;
         System.setProperty("zookeeper.jmx.log4j.disable", "true");
-        System.setProperty(ZOOKEEPER_JUTE_MAX_BUFFER, "" + config.juteMaxBuffer());
+        System.setProperty(ZOOKEEPER_JUTE_MAX_BUFFER, "" + zookeeperServerConfig.juteMaxBuffer());
         System.setProperty("zookeeper.serverCnxnFactory", "com.yahoo.vespa.zookeeper.RestrictedServerCnxnFactory");
 
-        writeConfigToDisk(config);
+        setAllowedClientHostnames(zookeeperServerConfig, configserverConfig);
+        writeConfigToDisk(zookeeperServerConfig);
         zkServerThread = new Thread(this, "zookeeper server");
         if (startServer) {
             zkServerThread.start();
@@ -51,13 +51,15 @@ public class ZooKeeperServer extends AbstractComponent implements Runnable {
     }
 
     @Inject
-    public ZooKeeperServer(ZookeeperServerConfig config) {
-        this(config, true);
+    public ZooKeeperServer(ZookeeperServerConfig zookeeperServerConfig, ConfigserverConfig configserverConfig) {
+        this(zookeeperServerConfig, configserverConfig, true);
     }
-    
+
     /** Restrict access to this ZooKeeper server to the given client hosts */
-    public static void setAllowedClientHostnames(Collection<String> hostnames) {
-        allowedClientHostnames = ImmutableSet.copyOf(hostnames);
+    private static void setAllowedClientHostnames(ZookeeperServerConfig zookeeperServerConfig, ConfigserverConfig configserverConfig) {
+        if (configserverConfig.hostedVespa())
+            allowedClientHostnames =  ImmutableSet.copyOf(zookeeperServerHostnames(zookeeperServerConfig));
+        // empty set if not hosted Vespa => allow all access
     }
     
     /** Returns the hosts which are allowed to access this ZooKeeper server, or empty to allow access from anywhere */
@@ -130,10 +132,9 @@ public class ZooKeeperServer extends AbstractComponent implements Runnable {
     @Override
     public void run() {
         System.setProperty(ZOOKEEPER_JMX_LOG4J_DISABLE, "true");
-        String[] args = new String[]{getDefaults().underVespaHome(config.zooKeeperConfigFile())};
+        String[] args = new String[]{getDefaults().underVespaHome(zookeeperServerConfig.zooKeeperConfigFile())};
         log.log(LogLevel.DEBUG, "Starting ZooKeeper server with config: " + args[0]);
-        log.log(LogLevel.INFO, "Trying to establish ZooKeeper quorum (from " +
-                config.server().stream().map(ZookeeperServerConfig.Server::hostname).collect(Collectors.toList()) + ")");
+        log.log(LogLevel.INFO, "Trying to establish ZooKeeper quorum (from " + zookeeperServerHostnames(zookeeperServerConfig) + ")");
         org.apache.zookeeper.server.quorum.QuorumPeerMain.main(args);
     }
 
@@ -143,6 +144,10 @@ public class ZooKeeperServer extends AbstractComponent implements Runnable {
         super.deconstruct();
     }
 
-    public ZookeeperServerConfig getConfig() { return config; }
+    public ZookeeperServerConfig getZookeeperServerConfig() { return zookeeperServerConfig; }
+
+    private static Set<String> zookeeperServerHostnames(ZookeeperServerConfig zookeeperServerConfig) {
+        return zookeeperServerConfig.server().stream().map(ZookeeperServerConfig.Server::hostname).collect(Collectors.toSet());
+    }
 
 }
diff --git a/zkfacade/src/test/java/com/yahoo/vespa/zookeeper/ZooKeeperServerTest.java b/zkfacade/src/test/java/com/yahoo/vespa/zookeeper/ZooKeeperServerTest.java
index 8dd33f3d744..626e5bf0627 100644
--- a/zkfacade/src/test/java/com/yahoo/vespa/zookeeper/ZooKeeperServerTest.java
+++ b/zkfacade/src/test/java/com/yahoo/vespa/zookeeper/ZooKeeperServerTest.java
@@ -1,6 +1,7 @@
 // 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.yahoo.cloud.config.ConfigserverConfig;
 import com.yahoo.cloud.config.ZookeeperServerConfig;
 import com.yahoo.io.IOUtils;
 import org.junit.Rule;
@@ -53,11 +54,11 @@ public class ZooKeeperServerTest {
     }
 
     private void createServer(ZookeeperServerConfig.Builder builder) {
-        new ZooKeeperServer(new ZookeeperServerConfig(builder), false);
+        new ZooKeeperServer(new ZookeeperServerConfig(builder), new ConfigserverConfig(new ConfigserverConfig.Builder()), false);
     }
 
     @Test(expected = RuntimeException.class)
-    public void require_that_this_id_must_be_present_amongst_servers() throws IOException {
+    public void require_that_this_id_must_be_present_amongst_servers() {
         ZookeeperServerConfig.Builder builder = new ZookeeperServerConfig.Builder();
         builder.server(newServer(2, "bar", 234, 432));
         builder.server(newServer(3, "baz", 345, 543));