From 9c77a8cdfa0ae4874e254c85feec7965524b51d3 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Sun, 4 Sep 2016 16:00:49 +0200 Subject: Revert "Revert "Revert "Revert "Revert "Bratseth/lockdown zk""""" --- .../main/java/com/yahoo/vespa/curator/Curator.java | 14 ++---- .../com/yahoo/vespa/curator/mock/MockCurator.java | 7 --- .../zookeeper/RestrictedServerCnxnFactory.java | 56 ---------------------- .../com/yahoo/vespa/zookeeper/ZooKeeperServer.java | 6 +-- .../yahoo/vespa/zookeeper/ZooKeeperServerTest.java | 3 +- 5 files changed, 8 insertions(+), 78 deletions(-) delete mode 100644 zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java (limited to 'zkfacade') diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java index 135154bba92..4c08924f8de 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java @@ -25,7 +25,6 @@ import java.time.Duration; import java.util.Arrays; import java.util.Collections; import java.util.List; -import java.util.Objects; import java.util.Optional; import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; @@ -55,7 +54,7 @@ public class Curator { private final String connectionSpec; private final int serverCount; - /** Creates a curator instance from a comma-separated string of ZooKeeper host:port strings */ + /** Creates a curator instance from a comma-separated string of ZooKeeper host names */ public static Curator create(String connectionSpec) { return new Curator(connectionSpec); } @@ -65,7 +64,7 @@ public class Curator { public Curator(ConfigserverConfig configserverConfig, ZooKeeperServer server) { this(createConnectionSpec(configserverConfig)); } - + private static String createConnectionSpec(ConfigserverConfig config) { StringBuilder sb = new StringBuilder(); for (int i = 0; i < config.zookeeperserver().size(); i++) { @@ -81,7 +80,6 @@ public class Curator { } private Curator(String connectionSpec) { - Objects.requireNonNull(connectionSpec, "The curator connection spec cannot be null"); this.connectionSpec = connectionSpec; this.serverCount = connectionSpec.split(",").length; validateConnectionSpec(connectionSpec); @@ -105,17 +103,15 @@ public class Curator { } private static void validateConnectionSpec(String connectionSpec) { - if (connectionSpec == null || connectionSpec.isEmpty()) + if (connectionSpec == null || connectionSpec.isEmpty()) { throw new IllegalArgumentException(String.format("Connections spec '%s' is not valid", connectionSpec)); + } } /** Returns the number of zooKeeper servers in this cluster */ public int serverCount() { return serverCount; } - /** - * Returns the servers in this cluster as a comma-separated list of host:port strings. - * This may be empty but never null - */ + /** Returns a comma-separated list of the zookeeper servers in this cluster */ public String connectionSpec() { return connectionSpec; } /** For internal use; prefer creating a {@link CuratorCounter} */ diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCurator.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCurator.java index 4615eb4e34a..92db255a67f 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCurator.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCurator.java @@ -101,7 +101,6 @@ public class MockCurator extends Curator { private boolean shouldTimeoutOnEnter = false; private int monotonicallyIncreasingNumber = 0; private final boolean stableOrdering; - private String connectionSpec = ""; /** The file system used by this mock to store zookeeper files and directories */ private final MemoryFileSystem fileSystem = new MemoryFileSystem(); @@ -144,12 +143,6 @@ public class MockCurator extends Curator { public Optional counter(String path) { return Optional.ofNullable(atomicCounters.get(path)); } - - /** Assigns the connection string, which must be on the form host1:port,host2:port ... */ - public void setConnectionSpec(String connectionSpec) { this.connectionSpec = connectionSpec; } - - @Override - public String connectionSpec() { return connectionSpec; } // ----- Start of adaptor methods from Curator to the mock file system ----- 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 d8561c67767..00000000000 --- a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java +++ /dev/null @@ -1,56 +0,0 @@ -package com.yahoo.vespa.zookeeper; - -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. - * - * @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 { - String remoteHost = ((InetSocketAddress)socket.getRemoteAddress()).getHostName(); // TODO: Move this line down - - String zookeeperClients = System.getProperty(ZooKeeperServer.ZOOKEEPER_VESPA_CLIENTS_PROPERTY); - if (zookeeperClients == null || zookeeperClients.isEmpty()) { - log.info("Allowing connection to ZooKeeper from " + remoteHost + ", as " + ZooKeeperServer.ZOOKEEPER_VESPA_CLIENTS_PROPERTY + " is not set"); // TODO: Remove this line - return super.createConnection(socket, selection); // client checking is not activated - } - - Set zooKeeperClients = toHostnameSet(zookeeperClients); - if ( ! remoteHost.equals("localhost") && ! zooKeeperClients.contains(remoteHost)) { - String errorMessage = "Rejecting connection to ZooKeeper from " + remoteHost + - ": This cluster only allow connection from hosts in: " + zooKeeperClients; - log.warning(errorMessage); - throw new IllegalArgumentException(errorMessage); - } - log.info("Allowing connection to ZooKeeper from " + remoteHost + ", as it is in " + zookeeperClients); // TODO: Remove this line - return super.createConnection(socket, selection); - } - - private Set toHostnameSet(String commaSeparatedString) { - Set hostnames = new HashSet<>(); - for (String hostname : commaSeparatedString.split(",")) - hostnames.add(hostname.trim()); - return hostnames; - } - -} 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 f037e3c9265..ffae797561c 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java @@ -19,8 +19,6 @@ import java.util.List; */ public class ZooKeeperServer extends AbstractComponent implements Runnable { - public static final String ZOOKEEPER_VESPA_CLIENTS_PROPERTY = "zookeeper.vespa.clients"; - 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"; @@ -31,8 +29,6 @@ public class ZooKeeperServer extends AbstractComponent implements Runnable { this.config = config; System.setProperty("zookeeper.jmx.log4j.disable", "true"); System.setProperty(ZOOKEEPER_JUTE_MAX_BUFFER, "" + config.juteMaxBuffer()); - System.setProperty("zookeeper.serverCnxnFactory", "com.yahoo.vespa.zookeeper.RestrictedServerCnxnFactory"); - writeConfigToDisk(config); zkServerThread = new Thread(this, "zookeeper server"); if (startServer) { @@ -44,7 +40,7 @@ public class ZooKeeperServer extends AbstractComponent implements Runnable { public ZooKeeperServer(ZookeeperServerConfig config) { this(config, true); } - + private void writeConfigToDisk(ZookeeperServerConfig config) { String cfg = transformConfigToString(config); try (FileWriter writer = new FileWriter(Defaults.getDefaults().underVespaHome(config.zooKeeperConfigFile()))) { 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 8c4db5dc8e4..4e231631f28 100644 --- a/zkfacade/src/test/java/com/yahoo/vespa/zookeeper/ZooKeeperServerTest.java +++ b/zkfacade/src/test/java/com/yahoo/vespa/zookeeper/ZooKeeperServerTest.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.zookeeper; import com.yahoo.cloud.config.ZookeeperServerConfig; import com.yahoo.io.IOUtils; +import com.yahoo.vespa.curator.Curator; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -11,6 +12,7 @@ import java.io.File; import java.io.IOException; import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; import com.yahoo.vespa.defaults.Defaults; @@ -72,7 +74,6 @@ public class ZooKeeperServerTest { File idFile = folder.newFile(); File cfgFile = folder.newFile(); - builder.server(new ZookeeperServerConfig.Server.Builder().id(0).hostname("testhost")); builder.zooKeeperConfigFile(cfgFile.getAbsolutePath()); builder.myidFile(idFile.getAbsolutePath()); -- cgit v1.2.3