Skip to content

Commit

Permalink
Move resolve<Transport>PublishPort() from TcpTransport -> Transport.
Browse files Browse the repository at this point in the history
Signed-off-by: Finn Carroll <[email protected]>
  • Loading branch information
finnegancarroll committed Dec 30, 2024
1 parent ecd998e commit 6fc1453
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 63 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
import static org.opensearch.common.settings.Setting.intSetting;
import static org.opensearch.common.settings.Setting.listSetting;
import static org.opensearch.common.util.concurrent.OpenSearchExecutors.daemonThreadFactory;
import static org.opensearch.transport.TcpTransport.resolveTransportPublishPort;
import static org.opensearch.transport.Transport.resolveTransportPublishPort;

/**
* Netty4 gRPC server implemented as a LifecycleComponent.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
import org.opensearch.telemetry.tracing.channels.TraceableRestChannel;
import org.opensearch.threadpool.ThreadPool;
import org.opensearch.transport.BindTransportException;
import org.opensearch.transport.Transport;

import java.io.IOException;
import java.net.InetAddress;
Expand All @@ -83,7 +84,6 @@
import static org.opensearch.http.HttpTransportSettings.SETTING_HTTP_PORT;
import static org.opensearch.http.HttpTransportSettings.SETTING_HTTP_PUBLISH_HOST;
import static org.opensearch.http.HttpTransportSettings.SETTING_HTTP_PUBLISH_PORT;
import static org.opensearch.transport.TcpTransport.resolveTransportPublishPort;

/**
* Base HttpServer class
Expand Down Expand Up @@ -192,7 +192,11 @@ protected void bindServer() {
throw new BindTransportException("Failed to resolve publish address", e);
}

final int publishPort = resolveTransportPublishPort(SETTING_HTTP_PUBLISH_PORT.get(settings), boundAddresses, publishInetAddress);
final int publishPort = Transport.resolveTransportPublishPort(
SETTING_HTTP_PUBLISH_PORT.get(settings),
boundAddresses,
publishInetAddress
);
if (publishPort < 0) {
throw new BindHttpException(

Check warning on line 201 in server/src/main/java/org/opensearch/http/AbstractHttpServerTransport.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/http/AbstractHttpServerTransport.java#L201

Added line #L201 was not covered by tests
"Failed to auto-resolve http publish port, multiple bound addresses "
Expand Down
47 changes: 1 addition & 46 deletions server/src/main/java/org/opensearch/transport/TcpTransport.java
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,7 @@ private BoundTransportAddress createBoundTransportAddress(ProfileSettings profil
throw new BindTransportException("Failed to resolve publish address", e);
}

final int publishPort = resolvePublishPort(profileSettings.publishPort, boundAddresses, publishInetAddress);
final int publishPort = Transport.resolvePublishPort(profileSettings.publishPort, boundAddresses, publishInetAddress);
if (publishPort == -1) {
String profileExplanation = profileSettings.isDefaultProfile ? "" : " for profile " + profileSettings.profileName;
throw new BindTransportException(
Expand All @@ -543,51 +543,6 @@ private BoundTransportAddress createBoundTransportAddress(ProfileSettings profil
return new BoundTransportAddress(transportBoundAddresses, publishAddress);
}

/**
* Resolve the publishPort for a server provided a list of boundAddresses and a publishInetAddress.
* Resolution strategy is as follows:
* If a configured port exists resolve to that port.
* If a bound address matches the publishInetAddress resolve to that port.
* If a bound address is a wildcard address resolve to that port.
* If all bound addresses share the same port resolve to that port.
*
* @param publishPort -1 if no configured publish port exists
* @param boundAddresses addresses bound by the server
* @param publishInetAddress address published for the server
* @return Resolved port. If publishPort is negative and no port can be resolved return publishPort.
*/
public static int resolvePublishPort(int publishPort, List<InetSocketAddress> boundAddresses, InetAddress publishInetAddress) {
if (publishPort < 0) {
for (InetSocketAddress boundAddress : boundAddresses) {
InetAddress boundInetAddress = boundAddress.getAddress();
if (boundInetAddress.isAnyLocalAddress() || boundInetAddress.equals(publishInetAddress)) {
publishPort = boundAddress.getPort();
break;
}
}
}

if (publishPort < 0) {
final Set<Integer> ports = new HashSet<>();
for (InetSocketAddress boundAddress : boundAddresses) {
ports.add(boundAddress.getPort());
}
if (ports.size() == 1) {
publishPort = ports.iterator().next();
}
}

return publishPort;
}

public static int resolveTransportPublishPort(int publishPort, List<TransportAddress> boundAddresses, InetAddress publishInetAddress) {
return resolvePublishPort(
publishPort,
boundAddresses.stream().map(TransportAddress::address).collect(Collectors.toList()),
publishInetAddress
);
}

@Override
public TransportAddress[] addressesFromString(String address) throws UnknownHostException {
return parse(address, defaultPortRange()[0]);
Expand Down
50 changes: 50 additions & 0 deletions server/src/main/java/org/opensearch/transport/Transport.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,18 @@

import java.io.Closeable;
import java.io.IOException;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.UnknownHostException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicLong;
import java.util.function.Predicate;
import java.util.stream.Collectors;

/**
* OpenSearch Transport Interface
Expand Down Expand Up @@ -111,6 +116,51 @@ default boolean isSecure() {

RequestHandlers getRequestHandlers();

/**
* Resolve the publishPort for a server provided a list of boundAddresses and a publishInetAddress.
* Resolution strategy is as follows:
* If a configured port exists resolve to that port.
* If a bound address matches the publishInetAddress resolve to that port.
* If a bound address is a wildcard address resolve to that port.
* If all bound addresses share the same port resolve to that port.
*
* @param publishPort -1 if no configured publish port exists
* @param boundAddresses addresses bound by the server
* @param publishInetAddress address published for the server
* @return Resolved port. If publishPort is negative and no port can be resolved return publishPort.
*/
static int resolvePublishPort(int publishPort, List<InetSocketAddress> boundAddresses, InetAddress publishInetAddress) {
if (publishPort < 0) {
for (InetSocketAddress boundAddress : boundAddresses) {
InetAddress boundInetAddress = boundAddress.getAddress();
if (boundInetAddress.isAnyLocalAddress() || boundInetAddress.equals(publishInetAddress)) {
publishPort = boundAddress.getPort();
break;
}
}
}

if (publishPort < 0) {
final Set<Integer> ports = new HashSet<>();
for (InetSocketAddress boundAddress : boundAddresses) {
ports.add(boundAddress.getPort());
}
if (ports.size() == 1) {
publishPort = ports.iterator().next();
}
}

return publishPort;
}

static int resolveTransportPublishPort(int publishPort, List<TransportAddress> boundAddresses, InetAddress publishInetAddress) {
return Transport.resolvePublishPort(
publishPort,
boundAddresses.stream().map(TransportAddress::address).collect(Collectors.toList()),
publishInetAddress
);
}

/**
* A unidirectional connection to a {@link DiscoveryNode}
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
import org.opensearch.test.rest.FakeRestRequest;
import org.opensearch.threadpool.TestThreadPool;
import org.opensearch.threadpool.ThreadPool;
import org.opensearch.transport.Transport;
import org.junit.After;
import org.junit.Before;

Expand All @@ -70,7 +71,6 @@

import static java.net.InetAddress.getByName;
import static java.util.Arrays.asList;
import static org.opensearch.transport.TcpTransport.resolveTransportPublishPort;
import static org.hamcrest.Matchers.equalTo;

public class AbstractHttpServerTransportTests extends OpenSearchTestCase {
Expand Down Expand Up @@ -100,39 +100,39 @@ public void testHttpPublishPort() throws Exception {
int boundPort = randomIntBetween(9000, 9100);
int otherBoundPort = randomIntBetween(9200, 9300);

int publishPort = resolveTransportPublishPort(9080, randomAddresses(), getByName("127.0.0.2"));
int publishPort = Transport.resolveTransportPublishPort(9080, randomAddresses(), getByName("127.0.0.2"));
assertThat("Publish port should be explicitly set to 9080", publishPort, equalTo(9080));

publishPort = resolveTransportPublishPort(
publishPort = Transport.resolveTransportPublishPort(
-1,
asList(address("127.0.0.1", boundPort), address("127.0.0.2", otherBoundPort)),
getByName("127.0.0.1")
);
assertThat("Publish port should be derived from matched address", publishPort, equalTo(boundPort));

publishPort = resolveTransportPublishPort(
publishPort = Transport.resolveTransportPublishPort(
-1,
asList(address("127.0.0.1", boundPort), address("127.0.0.2", boundPort)),
getByName("127.0.0.3")
);
assertThat("Publish port should be derived from unique port of bound addresses", publishPort, equalTo(boundPort));

publishPort = resolveTransportPublishPort(
publishPort = Transport.resolveTransportPublishPort(
-1,
asList(address("127.0.0.1", boundPort), address("127.0.0.2", otherBoundPort)),
getByName("127.0.0.3")
);
assertThat(publishPort, equalTo(-1));

publishPort = resolveTransportPublishPort(
publishPort = Transport.resolveTransportPublishPort(
-1,
asList(address("0.0.0.0", boundPort), address("127.0.0.2", otherBoundPort)),
getByName("127.0.0.1")
);
assertThat("Publish port should be derived from matching wildcard address", publishPort, equalTo(boundPort));

if (NetworkUtils.SUPPORTS_V6) {
publishPort = resolveTransportPublishPort(
publishPort = Transport.resolveTransportPublishPort(
-1,
asList(address("0.0.0.0", boundPort), address("127.0.0.2", otherBoundPort)),
getByName("::1")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@

import static java.net.InetAddress.getByName;
import static java.util.Arrays.asList;
import static org.opensearch.transport.TcpTransport.resolvePublishPort;
import static org.hamcrest.Matchers.equalTo;

public class PublishPortTests extends OpenSearchTestCase {
Expand Down Expand Up @@ -72,43 +71,43 @@ public void testPublishPort() throws Exception {

}

int publishPort = resolvePublishPort(
int publishPort = Transport.resolvePublishPort(
new TcpTransport.ProfileSettings(settings, profile).publishPort,
randomAddresses(),
getByName("127.0.0.2")
);
assertThat("Publish port should be explicitly set", publishPort, equalTo(useProfile ? 9080 : 9081));

publishPort = resolvePublishPort(
publishPort = Transport.resolvePublishPort(
new TcpTransport.ProfileSettings(baseSettings, profile).publishPort,
asList(address("127.0.0.1", boundPort), address("127.0.0.2", otherBoundPort)),
getByName("127.0.0.1")
);
assertThat("Publish port should be derived from matched address", publishPort, equalTo(boundPort));

publishPort = resolvePublishPort(
publishPort = Transport.resolvePublishPort(
new TcpTransport.ProfileSettings(baseSettings, profile).publishPort,
asList(address("127.0.0.1", boundPort), address("127.0.0.2", boundPort)),
getByName("127.0.0.3")
);
assertThat("Publish port should be derived from unique port of bound addresses", publishPort, equalTo(boundPort));

int resPort = resolvePublishPort(
int resPort = Transport.resolvePublishPort(
new TcpTransport.ProfileSettings(baseSettings, profile).publishPort,
asList(address("127.0.0.1", boundPort), address("127.0.0.2", otherBoundPort)),
getByName("127.0.0.3")
);
assertThat("as publish_port not specified and non-unique port of bound addresses", resPort, equalTo(-1));

publishPort = resolvePublishPort(
publishPort = Transport.resolvePublishPort(
new TcpTransport.ProfileSettings(baseSettings, profile).publishPort,
asList(address("0.0.0.0", boundPort), address("127.0.0.2", otherBoundPort)),
getByName("127.0.0.1")
);
assertThat("Publish port should be derived from matching wildcard address", publishPort, equalTo(boundPort));

if (NetworkUtils.SUPPORTS_V6) {
publishPort = resolvePublishPort(
publishPort = Transport.resolvePublishPort(
new TcpTransport.ProfileSettings(baseSettings, profile).publishPort,
asList(address("0.0.0.0", boundPort), address("127.0.0.2", otherBoundPort)),
getByName("::1")
Expand Down

0 comments on commit 6fc1453

Please sign in to comment.