Make ports required and move ContactPoints#extract to AddressUtils

Author: alexsa

core/src/main/java/com/datastax/oss/driver/internal/core/ContactPoints.java

@@ -19,14 +19,11 @@
 
 import com.datastax.oss.driver.api.core.metadata.EndPoint;
 import com.datastax.oss.driver.internal.core.metadata.DefaultEndPoint;
+import com.datastax.oss.driver.internal.core.util.AddressUtils;
 import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableSet;
 import com.datastax.oss.driver.shaded.guava.common.collect.Sets;
-import java.net.InetAddress;
 import java.net.InetSocketAddress;
-import java.net.UnknownHostException;
-import java.util.Arrays;
 import java.util.Collections;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 import org.slf4j.Logger;
@@ -41,7 +38,22 @@ public static Set<EndPoint> merge(
 
     Set<EndPoint> result = Sets.newHashSet(programmaticContactPoints);
     for (String spec : configContactPoints) {
-      for (InetSocketAddress address : extract(spec, resolve)) {
+
+      Set<InetSocketAddress> addresses = Collections.emptySet();
+      try {
+        addresses = AddressUtils.extract(spec, resolve);
+      } catch (RuntimeException e) {
+        LOG.warn("Ignoring invalid contact point {} ({})", spec, e.getMessage());
+      }
+
+      if (addresses.size() > 1) {
+        LOG.info(
+            "Contact point {} resolves to multiple addresses, will use them all ({})",
+            spec,
+            addresses);
+      }
+
+      for (InetSocketAddress address : addresses) {
         DefaultEndPoint endPoint = new DefaultEndPoint(address);
         boolean wasNew = result.add(endPoint);
         if (!wasNew) {
@@ -51,43 +63,4 @@ public static Set<EndPoint> merge(
     }
     return ImmutableSet.copyOf(result);
   }
-
-  private static Set<InetSocketAddress> extract(String spec, boolean resolve) {
-    int separator = spec.lastIndexOf(':');
-    if (separator < 0) {
-      LOG.warn("Ignoring invalid contact point {} (expecting host:port)", spec);
-      return Collections.emptySet();
-    }
-
-    String host = spec.substring(0, separator);
-    String portSpec = spec.substring(separator + 1);
-    int port;
-    try {
-      port = Integer.parseInt(portSpec);
-    } catch (NumberFormatException e) {
-      LOG.warn("Ignoring invalid contact point {} (expecting a number, got {})", spec, portSpec);
-      return Collections.emptySet();
-    }
-    if (!resolve) {
-      return ImmutableSet.of(InetSocketAddress.createUnresolved(host, port));
-    } else {
-      try {
-        InetAddress[] inetAddresses = InetAddress.getAllByName(host);
-        if (inetAddresses.length > 1) {
-          LOG.info(
-              "Contact point {} resolves to multiple addresses, will use them all ({})",
-              spec,
-              Arrays.deepToString(inetAddresses));
-        }
-        Set<InetSocketAddress> result = new HashSet<>();
-        for (InetAddress inetAddress : inetAddresses) {
-          result.add(new InetSocketAddress(inetAddress, port));
-        }
-        return result;
-      } catch (UnknownHostException e) {
-        LOG.warn("Ignoring invalid contact point {} (unknown host {})", spec, host);
-        return Collections.emptySet();
-      }
-    }
-  }
 }

core/src/main/java/com/datastax/oss/driver/internal/core/addresstranslation/SubnetAddressTranslator.java

@@ -17,16 +17,18 @@
  */
 package com.datastax.oss.driver.internal.core.addresstranslation;
 
+import static com.datastax.oss.driver.api.core.config.DefaultDriverOption.RESOLVE_CONTACT_POINTS;
+
 import com.datastax.oss.driver.api.core.addresstranslation.AddressTranslator;
 import com.datastax.oss.driver.api.core.config.DriverOption;
 import com.datastax.oss.driver.api.core.context.DriverContext;
-import com.google.common.base.Splitter;
+import com.datastax.oss.driver.internal.core.util.AddressUtils;
 import edu.umd.cs.findbugs.annotations.NonNull;
+import edu.umd.cs.findbugs.annotations.Nullable;
 import inet.ipaddr.IPAddress;
 import inet.ipaddr.IPAddressString;
 import java.net.InetSocketAddress;
 import java.util.List;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.stream.Collectors;
 import org.slf4j.Logger;
@@ -43,7 +45,6 @@
  * different Cassandra datacenters deployed to different Kubernetes clusters.
  */
 public class SubnetAddressTranslator implements AddressTranslator {
-
   private static final Logger LOG = LoggerFactory.getLogger(SubnetAddressTranslator.class);
 
   /**
@@ -53,23 +54,22 @@ public class SubnetAddressTranslator implements AddressTranslator {
    * <pre>
    * advanced.address-translator.subnet-addresses {
    *   "100.64.0.0/15" = "cassandra.datacenter1.com:9042"
-   *   "100.66.0.0/15" = "cassandra.datacenter2.com"
+   *   "100.66.0.0/15" = "cassandra.datacenter2.com:9042"
    *   # IPv6 example:
    *   # "::ffff:6440:0/111" = "cassandra.datacenter1.com:9042"
-   *   # "::ffff:6442:0/111" = "cassandra.datacenter2.com"
+   *   # "::ffff:6442:0/111" = "cassandra.datacenter2.com:9042"
    * }
    * </pre>
    *
-   * If configured without port, the default 9042 will be used. Also supports IPv6 subnets. Note:
-   * subnets must be represented as prefix blocks, see {@link inet.ipaddr.Address#isPrefixBlock()}.
+   * Note: subnets must be represented as prefix blocks, see {@link
+   * inet.ipaddr.Address#isPrefixBlock()}.
    */
   public static final String ADDRESS_TRANSLATOR_SUBNET_ADDRESSES =
       "advanced.address-translator.subnet-addresses";
 
   /**
    * A default address to fallback to if Cassandra node IP isn't contained in any of the configured
-   * subnets. If configured without port, the default 9042 will be used. Also supports IPv6
-   * addresses.
+   * subnets.
    */
   public static final String ADDRESS_TRANSLATOR_DEFAULT_ADDRESS =
       "advanced.address-translator.default-address";
@@ -92,11 +92,9 @@ public String getPath() {
         }
       };
 
-  private static final String DELIMITER = ":";
-  private static final int DEFAULT_PORT = 9042;
-
   private final List<SubnetAddress> subnetAddresses;
-  private final Optional<InetSocketAddress> defaultAddress;
+  private final Optional<String> defaultAddress;
+  private final boolean resolveAddresses;
   private final String logPrefix;
 
   public SubnetAddressTranslator(@NonNull DriverContext context) {
@@ -109,19 +107,22 @@ public SubnetAddressTranslator(@NonNull DriverContext context) {
                   // Quoted and/or containing forward slashes map keys in reference.conf are read to
                   // strings with additional quotes, eg. 100.64.0.0/15 -> '100.64.0."0/15"' or
                   // "100.64.0.0/15" -> '"100.64.0.0/15"'
-                  String subnet = e.getKey().replaceAll("\"", "");
+                  String subnetCIDR = e.getKey().replaceAll("\"", "");
                   String address = e.getValue();
-                  return new SubnetAddress(subnet, address);
+                  return new SubnetAddress(subnetCIDR, address);
                 })
             .collect(Collectors.toList());
     this.defaultAddress =
         Optional.ofNullable(
-                context
-                    .getConfig()
-                    .getDefaultProfile()
-                    .getString(ADDRESS_TRANSLATOR_DEFAULT_ADDRESS_OPTION, null))
-            .map(SubnetAddressTranslator::parseAddress);
-    SubnetAddressTranslator.validateSubnetsAreNotOverlapping(this.subnetAddresses);
+            context
+                .getConfig()
+                .getDefaultProfile()
+                .getString(ADDRESS_TRANSLATOR_DEFAULT_ADDRESS_OPTION, null));
+    this.resolveAddresses =
+        context.getConfig().getDefaultProfile().getBoolean(RESOLVE_CONTACT_POINTS, true);
+
+    validateSubnetsAreNotOverlapping(this.subnetAddresses);
+    this.defaultAddress.ifPresent(SubnetAddressTranslator::validateAddress);
   }
 
   @NonNull
@@ -130,11 +131,11 @@ public InetSocketAddress translate(@NonNull InetSocketAddress address) {
     InetSocketAddress translatedAddress = null;
     for (SubnetAddress subnetAddress : subnetAddresses) {
       if (subnetAddress.contains(address)) {
-        translatedAddress = subnetAddress.address;
+        translatedAddress = parseAddress(subnetAddress.address, resolveAddresses);
       }
     }
     if (translatedAddress == null && defaultAddress.isPresent()) {
-      translatedAddress = defaultAddress.get();
+      translatedAddress = parseAddress(defaultAddress.get(), resolveAddresses);
     }
     if (translatedAddress == null) {
       translatedAddress = address;
@@ -146,15 +147,17 @@ public InetSocketAddress translate(@NonNull InetSocketAddress address) {
   @Override
   public void close() {}
 
-  private static InetSocketAddress parseAddress(String address) {
-    List<String> addressTuple = Splitter.onPattern(DELIMITER).splitToList(address);
-    if (addressTuple.size() == 2) {
-      return new InetSocketAddress(addressTuple.get(0), Integer.parseInt(addressTuple.get(1)));
-    }
-    if (addressTuple.size() == 1) {
-      return new InetSocketAddress(addressTuple.get(0), DEFAULT_PORT);
+  @Nullable
+  private static InetSocketAddress parseAddress(String address, boolean resolve) {
+    return AddressUtils.extract(address, resolve).iterator().next();
+  }
+
+  private static void validateAddress(String address) {
+    try {
+      parseAddress(address, false);
+    } catch (RuntimeException e) {
+      throw new IllegalArgumentException("Invalid address: " + address, e);
     }
-    throw new IllegalArgumentException("Invalid default address: " + address);
   }
 
   private static void validateSubnetsAreNotOverlapping(List<SubnetAddress> subnetAddresses) {
@@ -174,21 +177,28 @@ private static void validateSubnetsAreNotOverlapping(List<SubnetAddress> subnetA
 
   private static class SubnetAddress {
     private final IPAddress subnet;
-    private final InetSocketAddress address;
+    private final String address;
+
+    private SubnetAddress(String subnetCIDR, String address) {
+      this.subnet = parseSubnet(subnetCIDR);
+      this.address = address;
 
-    private SubnetAddress(String subnet, String address) {
-      IPAddress subnetIpAddress = new IPAddressString(subnet).getAddress();
-      if (subnetIpAddress == null) {
-        throw new IllegalArgumentException("Invalid subnet: " + subnet);
+      validateAddress(this.address);
+    }
+
+    private static IPAddress parseSubnet(String subnetCIDR) {
+      IPAddress subnet = new IPAddressString(subnetCIDR).getAddress();
+      if (subnet == null) {
+        throw new IllegalArgumentException("Invalid subnet: " + subnetCIDR);
       }
-      if (!subnetIpAddress.isPrefixBlock()) {
+      if (!subnet.isPrefixBlock()) {
+
         throw new IllegalArgumentException(
             String.format(
                 "Subnet %s must be represented as a network prefix block %s",
-                subnetIpAddress, subnetIpAddress.toPrefixBlock()));
+                subnet, subnet.toPrefixBlock()));
       }
-      this.subnet = subnetIpAddress;
-      this.address = parseAddress(address);
+      return subnet;
     }
 
     private boolean isOverlapping(SubnetAddress other) {
@@ -209,18 +219,5 @@ private boolean contains(InetSocketAddress address) {
       }
       return subnet.contains(ipAddress);
     }
-
-    @Override
-    public boolean equals(Object other) {
-      if (this == other) return true;
-      if (!(other instanceof SubnetAddress)) return false;
-      SubnetAddress that = (SubnetAddress) other;
-      return Objects.equals(subnet, that.subnet) && Objects.equals(address, that.address);
-    }
-
-    @Override
-    public int hashCode() {
-      return Objects.hash(subnet, address);
-    }
   }
 }

core/src/main/java/com/datastax/oss/driver/internal/core/util/AddressUtils.java

@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.datastax.oss.driver.internal.core.util;
+
+import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableSet;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.net.UnknownHostException;
+import java.util.HashSet;
+import java.util.Set;
+
+public class AddressUtils {
+
+  public static Set<InetSocketAddress> extract(String address, boolean resolve) {
+    int separator = address.lastIndexOf(':');
+    if (separator < 0) {
+      throw new IllegalArgumentException("expecting format host:port");
+    }
+
+    String host = address.substring(0, separator);
+    String portString = address.substring(separator + 1);
+    int port;
+    try {
+      port = Integer.parseInt(portString);
+    } catch (NumberFormatException e) {
+      throw new IllegalArgumentException("expecting port to be a number, got " + portString, e);
+    }
+    if (!resolve) {
+      return ImmutableSet.of(InetSocketAddress.createUnresolved(host, port));
+    } else {
+      InetAddress[] inetAddresses;
+      try {
+        inetAddresses = InetAddress.getAllByName(host);
+      } catch (UnknownHostException e) {
+        throw new RuntimeException("unknown host " + host, e);
+      }
+      Set<InetSocketAddress> result = new HashSet<>();
+      for (InetAddress inetAddress : inetAddresses) {
+        result.add(new InetSocketAddress(inetAddress, port));
+      }
+      return result;
+    }
+  }
+}

core/src/main/resources/reference.conf

@@ -1040,16 +1040,18 @@ datastax-java-driver {
     # This property has to be set only in case you use FixedHostNameAddressTranslator.
     # advertised-hostname = mycustomhostname
     #
-    # Theses properties have to be set only in case you use SubnetAddressTranslator.
+    # These properties are only applicable in case you use SubnetAddressTranslator.
     # subnet-addresses {
     #   "100.64.0.0/15" = "cassandra.datacenter1.com:9042"
-    #   "100.66.0.0/15" = "cassandra.datacenter2.com"  # port defaults to 9042 if not specified
+    #   "100.66.0.0/15" = "cassandra.datacenter2.com:9042"
     #   # IPv6 example:
     #   # "::ffff:6440:0/111" = "cassandra.datacenter1.com:9042"
-    #   # "::ffff:6442:0/111" = "cassandra.datacenter2.com"  # port defaults to 9042 if not specified
+    #   # "::ffff:6442:0/111" = "cassandra.datacenter2.com:9042"
     # }
-    # Optional. When configured, addresses not matching the configured subnets are translated to it. Port defaults to 9042 if not specified.
+    # Optional. When configured, addresses not matching the configured subnets are translated this address.
     # default-address = "cassandra.datacenter1.com:9042"
+    # Note: `advanced.resolve-contact-points` (see below) is used to determine whether to resolve subnet
+    # and default address on translation. 
   }
 
   # Whether to resolve the addresses passed to `basic.contact-points`.

core/src/test/java/com/datastax/oss/driver/internal/core/ContactPointsTest.java

@@ -121,7 +121,7 @@ public void should_ignore_malformed_host_and_port_and_warn() {
         ContactPoints.merge(Collections.emptySet(), ImmutableList.of("foobar"), true);
 
     assertThat(endPoints).isEmpty();
-    assertLog(Level.WARN, "Ignoring invalid contact point foobar (expecting host:port)");
+    assertLog(Level.WARN, "Ignoring invalid contact point foobar (expecting format host:port)");
   }
 
   @Test
@@ -132,7 +132,7 @@ public void should_ignore_malformed_port_and_warn() {
     assertThat(endPoints).isEmpty();
     assertLog(
         Level.WARN,
-        "Ignoring invalid contact point 127.0.0.1:foobar (expecting a number, got foobar)");
+        "Ignoring invalid contact point 127.0.0.1:foobar (expecting port to be a number, got foobar)");
   }
 
   @Test