tolbertam commented on code in PR #2013:
URL:
https://github.com/apache/cassandra-java-driver/pull/2013#discussion_r1956786827
##########
core/src/main/resources/reference.conf:
##########
@@ -1020,17 +1020,33 @@ datastax-java-driver {
# the package com.datastax.oss.driver.internal.core.addresstranslation.
#
# The driver provides the following implementations out of the box:
- # - PassThroughAddressTranslator: returns all addresses unchanged
+ # - PassThroughAddressTranslator: returns all addresses unchanged.
# - FixedHostNameAddressTranslator: translates all addresses to a specific
hostname.
+ # - SubnetAddressTranslator: translates addresses to hostname based on the
subnet match.
# - Ec2MultiRegionAddressTranslator: suitable for an Amazon multi-region
EC2 deployment where
# clients are also deployed in EC2. It optimizes network costs by
favoring private IPs over
# public ones whenever possible.
#
# You can also specify a custom class that implements AddressTranslator
and has a public
# constructor with a DriverContext argument.
class = PassThroughAddressTranslator
+ #
# This property has to be set only in case you use
FixedHostNameAddressTranslator.
# advertised-hostname = mycustomhostname
+ #
+ # 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:9042"
+ # # IPv6 example:
+ # # "::ffff:6440:0/111" = "cassandra.datacenter1.com:9042"
+ # # "::ffff:6442:0/111" = "cassandra.datacenter2.com:9042"
+ # }
+ # Optional. When configured, addresses not matching the configured subnets
are translated to this address.
+ # default-address = "cassandra.datacenter1.com:9042"
+ # Whether to resolve the addresses once on initialization (if true) or on
each node (re-)connection (if false).
+ # If not configured, defaults to false.
+ # resolve-addresses = false
Review Comment:
:+1: while `resolve-contact-points` defaults to `true`, I was thinking about
whether we should do the same here, but I think you probably will generally use
a DNS name that might change its backing IPs from time to time, so makes sense
for this to be `false`. Was that what you were thinking as well?
##########
core/src/test/java/com/datastax/oss/driver/internal/core/addresstranslation/SubnetAddressTranslatorTest.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.addresstranslation;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static
org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import com.datastax.oss.driver.api.core.config.DriverExecutionProfile;
+import com.datastax.oss.driver.internal.core.context.DefaultDriverContext;
+import
com.datastax.oss.driver.internal.core.context.MockedDriverContextFactory;
+import com.google.common.collect.ImmutableMap;
+import java.net.InetSocketAddress;
+import java.util.Map;
+import java.util.Optional;
+import org.junit.Test;
+
+public class SubnetAddressTranslatorTest {
Review Comment:
Mind adding:
```suggestion
@SuppressWarnings("resource")
public class SubnetAddressTranslatorTest {
```
to avoid the warnings around using `new SubnetAddressTranslator` without a
try block? Since `AddressTranslator` is `AutoCloseable` it causes IntelliJ to
throw some warnings, but I think its fine to use it in this way for the test.
##########
core/src/main/java/com/datastax/oss/driver/internal/core/addresstranslation/SubnetAddressTranslator.java:
##########
@@ -0,0 +1,235 @@
+/*
+ * 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.addresstranslation;
+
+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.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.Optional;
+import java.util.stream.Collectors;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * This translator returns the proxy address of the private subnet containing
the Cassandra node IP,
+ * or default address if no matching subnets, or passes through the original
node address if no
+ * default configured.
+ *
+ * <p>The translator can be used for scenarios when all nodes are behind some
kind of proxy, and
+ * that proxy is different for nodes located in different subnets (eg. when
Cassandra is deployed in
+ * multiple datacenters/regions). One can use this, for example, for Cassandra
on Kubernetes with
+ * different Cassandra datacenters deployed to different Kubernetes clusters.
+ */
+public class SubnetAddressTranslator implements AddressTranslator {
+ private static final Logger LOG =
LoggerFactory.getLogger(SubnetAddressTranslator.class);
+
+ /**
+ * A map of Cassandra node subnets (CIDR notations) to target addresses, for
example (note quoted
+ * keys):
+ *
+ * <pre>
+ * advanced.address-translator.subnet-addresses {
+ * "100.64.0.0/15" = "cassandra.datacenter1.com:9042"
+ * "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:9042"
+ * }
+ * </pre>
+ *
+ * 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.
+ */
+ public static final String ADDRESS_TRANSLATOR_DEFAULT_ADDRESS =
+ "advanced.address-translator.default-address";
+
+ /**
+ * Whether to resolve the addresses on initialization (if true) or on each
node (re-)connection
+ * (if false). Defaults to false.
+ */
+ public static final String ADDRESS_TRANSLATOR_RESOLVE_ADDRESSES =
+ "advanced.address-translator.resolve-addresses";
+
+ public static DriverOption ADDRESS_TRANSLATOR_SUBNET_ADDRESSES_OPTION =
+ new DriverOption() {
+ @NonNull
+ @Override
+ public String getPath() {
+ return ADDRESS_TRANSLATOR_SUBNET_ADDRESSES;
+ }
+ };
+
+ public static DriverOption ADDRESS_TRANSLATOR_DEFAULT_ADDRESS_OPTION =
+ new DriverOption() {
+ @NonNull
+ @Override
+ public String getPath() {
+ return ADDRESS_TRANSLATOR_DEFAULT_ADDRESS;
+ }
+ };
+
+ public static DriverOption ADDRESS_TRANSLATOR_RESOLVE_ADDRESSES_OPTION =
+ new DriverOption() {
+ @NonNull
+ @Override
+ public String getPath() {
+ return ADDRESS_TRANSLATOR_RESOLVE_ADDRESSES;
+ }
+ };
+
+ private final List<SubnetAddress> subnetAddresses;
+ private final Optional<InetSocketAddress> defaultAddress;
Review Comment:
mind adding this?
```suggestion
@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
private final Optional<InetSocketAddress> defaultAddress;
```
It bothers me that IntelliJ doesn't look this as I think its completely
fine, from [the
docs](https://www.jetbrains.com/help/inspectopedia/OptionalUsedAsFieldOrParameterType.html)
it looks like the main concern is around `Optional` not being `Serializable`,
but that doesn't apply here and I like reasoning about `Optional` types than
checking for null.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
