diff options
author | Harald Musum <musum@oath.com> | 2018-04-03 11:09:03 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-04-03 11:09:03 +0200 |
commit | 6b084d6816955baaea656b062239c63c949bc9b8 (patch) | |
tree | e18cc35139aa9aee661f0c694ef6ce635ba5ccab /zkfacade/src | |
parent | 9a86a735b9c29054ac88cecf0cb38a1a8ac49d24 (diff) |
Revert "Revert "Only allow Zookeeper access for config servers in hosted Vespa""
Diffstat (limited to 'zkfacade/src')
3 files changed, 28 insertions, 27 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..352635ac920 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,40 +11,41 @@ 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 { /** * The set of hosts which can access the ZooKeeper server in this VM, or empty * to allow access from anywhere. - * This belongs logically to the server instance but must be static to make it accessible + * This belongs logically to the server instance and is final, but must be static to make it accessible * from RestrictedServerCnxnFactory, which is created by ZK through reflection. */ - private static volatile ImmutableSet<String> allowedClientHostnames = ImmutableSet.of(); + private static ImmutableSet<String> allowedClientHostnames = ImmutableSet.of(); private static final java.util.logging.Logger log = java.util.logging.Logger.getLogger(ZooKeeperServer.class.getName()); 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); + if (configserverConfig.hostedVespa()) // restrict access to config servers only + allowedClientHostnames = ImmutableSet.copyOf(zookeeperServerHostnames(zookeeperServerConfig)); + + writeConfigToDisk(zookeeperServerConfig); zkServerThread = new Thread(this, "zookeeper server"); if (startServer) { zkServerThread.start(); @@ -51,15 +53,10 @@ public class ZooKeeperServer extends AbstractComponent implements Runnable { } @Inject - public ZooKeeperServer(ZookeeperServerConfig config) { - this(config, true); - } - - /** Restrict access to this ZooKeeper server to the given client hosts */ - public static void setAllowedClientHostnames(Collection<String> hostnames) { - allowedClientHostnames = ImmutableSet.copyOf(hostnames); + public ZooKeeperServer(ZookeeperServerConfig zookeeperServerConfig, ConfigserverConfig configserverConfig) { + this(zookeeperServerConfig, configserverConfig, true); } - + /** Returns the hosts which are allowed to access this ZooKeeper server, or empty to allow access from anywhere */ public static ImmutableSet<String> getAllowedClientHostnames() { return allowedClientHostnames; } @@ -130,10 +127,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 +139,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)); |