Merge pull request #492 from mesos/bug/hardcoded-zk-mesos-endpoint

Allow arbitrary path in ZK url instead of appending /mesos

commons/src/main/java/org/apache/mesos/elasticsearch/common/zookeeper/formatter/MesosZKFormatter.java

@@ -1,40 +1,33 @@
 package org.apache.mesos.elasticsearch.common.zookeeper.formatter;
 
 import org.apache.mesos.elasticsearch.common.zookeeper.exception.ZKAddressException;
-import org.apache.mesos.elasticsearch.common.zookeeper.model.ZKAddress;
 import org.apache.mesos.elasticsearch.common.zookeeper.parser.ZKAddressParser;
 
-import java.util.List;
-
 /**
  * Provides the ZooKeeper address(es) in a format required by Mesos.
  *
- * The format consists of a list of ZK servers and appends a /mesos
+ * The format consists of a list of ZK servers and a path
  *
  * Example: zk://host1:port1,host2:port2/mesos
  */
 public class MesosZKFormatter extends AbstractZKFormatter {
 
-    public static final String MESOS_PATH = "/mesos";
-
     public MesosZKFormatter(ZKAddressParser parser) {
         super(parser);
     }
 
     /**
-     * Get the ZooKeeper address for the mesos master, correctly formatted.
+     * Validates the ZooKeeper address and returns it
+     *
      * @param zkUrl The raw ZK address string in the format "zk://host:port/zkNode,..."
+     *
      * @throws ZKAddressException if the raw zkURL is invalid.
-     * @return the zookeeper addresses in the format host:port[,host:port,...]/mesos
+     *
+     * @return the zookeeper addresses in the format zk://host:port[,host:port,...]/mesos
      */
     public String format(String zkUrl) {
-        List<ZKAddress> addressList = parser.validateZkUrl(zkUrl);
-        StringBuilder builder = new StringBuilder();
-        addressList.forEach(add -> builder.append(",").append(add.getAddress()).append(":").append(add.getPort()));
-        builder.deleteCharAt(0); // Delete first ','
-        builder.append(MESOS_PATH); // Append "/mesos"
-        builder.insert(0, ZKAddress.ZK_PREFIX); // Prepend "zk://"
-        return builder.toString();
+        parser.validateZkUrl(zkUrl);
+        return zkUrl;
     }
 
 }

commons/src/main/java/org/apache/mesos/elasticsearch/common/zookeeper/parser/ZKAddressParser.java

@@ -1,5 +1,6 @@
 package org.apache.mesos.elasticsearch.common.zookeeper.parser;
 
+import org.apache.commons.lang3.StringUtils;
 import org.apache.mesos.elasticsearch.common.zookeeper.exception.ZKAddressException;
 import org.apache.mesos.elasticsearch.common.zookeeper.model.ZKAddress;
 
@@ -9,7 +10,7 @@
 import java.util.regex.Pattern;
 
 /**
- * Ensures ZooKeeper addresses are formatted properly
+ * Validates ZK url and parses ZK addresses.
  *
  * IMPORTANT: Different components in the framework require different ZK address strings.
  *
@@ -20,11 +21,6 @@
  * 2) The MesosSchedulerDriver requires a full ZK url
  *
  * zk://host1:port1,host2:port2/mesos
- *
- * 3) The Elasticsearch ZK plugin requires ZK servers PLUS path WITHOUT the zk:// prefix
- *
- * host1:port1,host2:port2/mesos
- *
  */
 public class ZKAddressParser {
     public static final String ZK_PREFIX_REGEX = "^" + ZKAddress.ZK_PREFIX + ".*";
@@ -38,6 +34,10 @@ public List<ZKAddress> validateZkUrl(final String zkUrl) {
             throw new ZKAddressException(zkUrl);
         }
 
+        if (StringUtils.countMatches(zkUrl, '/') < 3) {
+            throw new ZKAddressException(zkUrl);
+        }
+
         // Strip zk prefix and spaces
         String zkStripped = zkUrl.replace(ZKAddress.ZK_PREFIX, "").replace(" ", "");
 

commons/src/test/java/org/apache/mesos/elasticsearch/common/zookeeper/formatter/MesosZKFormatterTest.java

@@ -2,58 +2,70 @@
 
 import org.apache.mesos.elasticsearch.common.zookeeper.model.ZKAddress;
 import org.apache.mesos.elasticsearch.common.zookeeper.parser.ZKAddressParser;
+import org.junit.Before;
 import org.junit.Test;
 import org.mockito.Mockito;
 
-import java.util.ArrayList;
-import java.util.List;
+import java.util.Collections;
 
+import static java.util.Arrays.asList;
 import static org.junit.Assert.*;
-import static org.mockito.Matchers.anyString;
+import static org.mockito.Mockito.when;
 
 /**
- * Tests for Mesos ZK formatting. Basically the same as MesosStateZKFormmater, but with /mesos
+ * Tests for Mesos ZK formatting. Basically the same as MesosStateZKFormatter, but with /mesos
  */
 public class MesosZKFormatterTest {
+
+    private ZKAddressParser parser;
+
+    private MesosZKFormatter formatter;
+
+    @Before
+    public void before() {
+        parser = Mockito.mock(ZKAddressParser.class);
+        formatter = new MesosZKFormatter(parser);
+    }
+
     @Test
-    public void shouldReturnSameAddressIfValidated() {
-        ZKAddressParser mock = Mockito.mock(ZKAddressParser.class);
-        String add = "192.168.0.1:2182";
-        List<ZKAddress> dummyReturn = new ArrayList<>(1);
-        dummyReturn.add(new ZKAddress(add));
-        Mockito.when(mock.validateZkUrl(anyString())).thenReturn(dummyReturn);
-        ZKFormatter formatter = new MesosZKFormatter(mock);
-        String address = formatter.format(""); // Doesn't matter. We're returning a dummy.
-        assertEquals(ZKAddress.ZK_PREFIX + add + MesosZKFormatter.MESOS_PATH, address);
+    public void testFormat() {
+        String zkUrl = "zk://zookeeper:2181/mesos";
+
+        when(parser.validateZkUrl(zkUrl)).thenReturn(Collections.singletonList(
+                new ZKAddress("zookeeper:2181")
+        ));
+
+        String format = formatter.format(zkUrl);
+
+        assertEquals("zk://zookeeper:2181/mesos", format);
     }
 
     @Test
-    public void shouldReturnMultiAddress() {
-        ZKAddressParser mock = Mockito.mock(ZKAddressParser.class);
-        String add1 = "192.168.0.1:2182";
-        String add2 = "192.168.0.2:2183";
-        String concat = add1 + "," + add2;
-        List<ZKAddress> dummyReturn = new ArrayList<>(1);
-        dummyReturn.add(new ZKAddress(add1));
-        dummyReturn.add(new ZKAddress(add2));
-        Mockito.when(mock.validateZkUrl(anyString())).thenReturn(dummyReturn);
-        ZKFormatter formatter = new MesosZKFormatter(mock);
-        String address = formatter.format(""); // Doesn't matter. We're returning a dummy.
-        assertEquals(ZKAddress.ZK_PREFIX + concat + MesosZKFormatter.MESOS_PATH, address);
+    public void testFormat_nestedPath() {
+        String zkUrl = "zk://zookeeper:2181/dev/mesos";
+
+        when(parser.validateZkUrl(zkUrl)).thenReturn(Collections.singletonList(
+                new ZKAddress("zookeeper:2181")
+        ));
+
+        String format = formatter.format(zkUrl);
+
+        assertEquals("zk://zookeeper:2181/dev/mesos", format);
     }
 
     @Test
-    public void shouldReturnArrayWhenUsersOrPath() {
-        ZKAddressParser mock = Mockito.mock(ZKAddressParser.class);
-        String add1 = "192.168.0.1:2182";
-        String add2 = "192.168.0.2:2183";
-        String concat = add1 + "," + add2;
-        List<ZKAddress> dummyReturn = new ArrayList<>(1);
-        dummyReturn.add(new ZKAddress(add1 + "/mesos"));
-        dummyReturn.add(new ZKAddress("bob:pass@" + add2));
-        Mockito.when(mock.validateZkUrl(anyString())).thenReturn(dummyReturn);
-        ZKFormatter formatter = new MesosZKFormatter(mock);
-        String address = formatter.format(""); // Doesn't matter. We're returning a dummy.
-        assertEquals(ZKAddress.ZK_PREFIX + concat + MesosZKFormatter.MESOS_PATH, address);
+    public void testFormat_threeNodes() {
+        String zkUrl = "zk://zookeeper1:2181,zookeeper2:2181,zookeeper3:2181/mesos";
+
+        when(parser.validateZkUrl(zkUrl)).thenReturn(asList(
+                new ZKAddress("zookeeper1:2181"),
+                new ZKAddress("zookeeper2:2181"),
+                new ZKAddress("zookeeper3:2181")
+        ));
+
+        String format = formatter.format(zkUrl);
+
+        assertEquals("zk://zookeeper1:2181,zookeeper2:2181,zookeeper3:2181/mesos", format);
     }
+
 }

commons/src/test/java/org/apache/mesos/elasticsearch/common/zookeeper/parser/ZKAddressParserTest.java

@@ -46,15 +46,13 @@ public void shouldExceptionWithMalformedPort() {
     }
 
     @Test(expected = ZKAddressException.class)
-    public void shouldExceptionWithMalformedPortOnSecond() {
-        new ZKAddressParser().validateZkUrl("zk://master:2130,slave:in");
+    public void shouldExceptionWithoutPath() {
+        new ZKAddressParser().validateZkUrl("zk://master:2181");
     }
 
-    @Test
-    public void shouldAcceptIfSingleZKAddress() {
-        String add = "zk://192.168.0.1:2182";
-        List<ZKAddress> zk = new ZKAddressParser().validateZkUrl(add);
-        assertZKEquals(zk.get(0), "", "", "192.168.0.1", "2182", "");
+    @Test(expected = ZKAddressException.class)
+    public void shouldExceptionWithMalformedPortOnSecond() {
+        new ZKAddressParser().validateZkUrl("zk://master:2130,slave:in");
     }
 
     @Test
@@ -80,13 +78,6 @@ public void shouldAcceptIfSpacesInPath() {
         assertZKEquals(zk.get(1), "", "", "10.4.52.3", "1234", "mesos");
     }
 
-    @Test
-    public void shouldAcceptIfSpacesAtEnd() {
-        String add = "zk://192.168.0.1:2182 ";
-        List<ZKAddress> zk = new ZKAddressParser().validateZkUrl(add);
-        assertZKEquals(zk.get(0), "", "", "192.168.0.1", "2182", "");
-    }
-
     @Test
     public void shouldAcceptIfMultiZKAddressWithMultiPath() {
         String add = "zk://192.168.0.1:2182/somePath,10.4.52.3:1234/mesos";
@@ -102,7 +93,6 @@ public void shouldAcceptIfUserPass() {
         assertZKEquals(zk.get(0), "bob", "pass", "192.168.0.1", "2182", "mesos");
     }
 
-
     @Test
     public void shouldAcceptIfMultiUserPassSpaces() {
         String add = "zk://bob:pass@192.168.0.1:2182, team:dev@10.4.52.3:1234/mesos";

scheduler/src/test/java/org/apache/mesos/elasticsearch/scheduler/controllers/TestConfiguration.java

@@ -31,7 +31,7 @@ public ElasticsearchScheduler getMockScheduler() {
 
     @Bean
     public org.apache.mesos.elasticsearch.scheduler.Configuration getConfig() {
-        return new org.apache.mesos.elasticsearch.scheduler.Configuration(ZookeeperCLIParameter.ZOOKEEPER_MESOS_URL, "zk://dummy.mesos.master:2181") {
+        return new org.apache.mesos.elasticsearch.scheduler.Configuration(ZookeeperCLIParameter.ZOOKEEPER_MESOS_URL, "zk://dummy.mesos.master:2181/mesos") {
             @SuppressFBWarnings("UMAC_UNCALLABLE_METHOD_OF_ANONYMOUS_CLASS")
             public String getFakePassword() {
                 return "Sh0uld n0t be v1s1bl3";