Skip to content

Commit 3d84501

Browse files
bmaizelsbmaizels
committed
Refactoring plus unit tests to assert no additional DNS caching for asynchronous requests using the Netty client is taking place
1 parent 18d68e8 commit 3d84501

File tree

4 files changed

+207
-27
lines changed

4 files changed

+207
-27
lines changed

‎http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/AwaitCloseChannelPoolMap.java

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919

2020
import io.netty.bootstrap.Bootstrap;
2121
import io.netty.channel.Channel;
22-
import io.netty.channel.ChannelOption;
2322
import io.netty.channel.pool.ChannelPool;
2423
import io.netty.channel.pool.ChannelPoolHandler;
2524
import io.netty.handler.codec.http2.Http2SecurityUtil;
@@ -28,7 +27,6 @@
2827
import io.netty.handler.ssl.SslProvider;
2928
import io.netty.handler.ssl.SupportedCipherSuiteFilter;
3029
import io.netty.handler.ssl.util.InsecureTrustManagerFactory;
31-
import java.net.InetSocketAddress;
3230
import java.net.URI;
3331
import java.net.URISyntaxException;
3432
import java.time.Duration;
@@ -40,6 +38,7 @@
4038
import java.util.concurrent.TimeUnit;
4139
import java.util.concurrent.TimeoutException;
4240
import java.util.concurrent.atomic.AtomicReference;
41+
import java.util.function.Function;
4342
import javax.net.ssl.KeyManager;
4443
import javax.net.ssl.KeyManagerFactory;
4544
import javax.net.ssl.SSLException;
@@ -75,35 +74,47 @@ public void channelCreated(Channel ch) throws Exception {
7574
}
7675
};
7776

77+
// IMPORTANT: If the default bootstrap provider is changed, ensure that the new implementation is compliant with
78+
// DNS resolver testing in BootstrapProviderTest, specifically that no caching of hostname lookups is taking place.
79+
private static final Function<Builder, BootstrapProvider> DEFAULT_BOOTSTRAP_PROVIDER =
80+
b -> new BootstrapProvider(b.sdkEventLoopGroup, b.configuration, b.sdkChannelOptions);
81+
7882
private final Map<URI, Boolean> shouldProxyForHostCache = new ConcurrentHashMap<>();
7983

8084

81-
private final SdkChannelOptions sdkChannelOptions;
82-
private final SdkEventLoopGroup sdkEventLoopGroup;
8385
private final NettyConfiguration configuration;
8486
private final Protocol protocol;
8587
private final long maxStreams;
8688
private final Duration healthCheckPingPeriod;
8789
private final int initialWindowSize;
8890
private final SslProvider sslProvider;
8991
private final ProxyConfiguration proxyConfiguration;
92+
private final BootstrapProvider bootstrapProvider;
9093

91-
private AwaitCloseChannelPoolMap(Builder builder) {
92-
this.sdkChannelOptions = builder.sdkChannelOptions;
93-
this.sdkEventLoopGroup = builder.sdkEventLoopGroup;
94+
private AwaitCloseChannelPoolMap(Builder builder, Function<Builder, BootstrapProvider> createBootStrapProvider) {
9495
this.configuration = builder.configuration;
9596
this.protocol = builder.protocol;
9697
this.maxStreams = builder.maxStreams;
9798
this.healthCheckPingPeriod = builder.healthCheckPingPeriod;
9899
this.initialWindowSize = builder.initialWindowSize;
99100
this.sslProvider = builder.sslProvider;
100101
this.proxyConfiguration = builder.proxyConfiguration;
102+
this.bootstrapProvider = createBootStrapProvider.apply(builder);
103+
}
104+
105+
private AwaitCloseChannelPoolMap(Builder builder) {
106+
this(builder, DEFAULT_BOOTSTRAP_PROVIDER);
101107
}
102108

103109
@SdkTestInternalApi
104-
AwaitCloseChannelPoolMap(Builder builder, Map<URI, Boolean> shouldProxyForHostCache) {
105-
this(builder);
106-
this.shouldProxyForHostCache.putAll(shouldProxyForHostCache);
110+
AwaitCloseChannelPoolMap(Builder builder,
111+
Map<URI, Boolean> shouldProxyForHostCache,
112+
BootstrapProvider bootstrapProvider) {
113+
this(builder, bootstrapProvider == null ? DEFAULT_BOOTSTRAP_PROVIDER : b -> bootstrapProvider);
114+
115+
if (shouldProxyForHostCache != null) {
116+
this.shouldProxyForHostCache.putAll(shouldProxyForHostCache);
117+
}
107118
}
108119

109120
public static Builder builder() {
@@ -172,17 +183,7 @@ public void close() {
172183
private Bootstrap createBootstrap(URI poolKey) {
173184
String host = bootstrapHost(poolKey);
174185
int port = bootstrapPort(poolKey);
175-
176-
Bootstrap bootstrap =
177-
new Bootstrap()
178-
.group(sdkEventLoopGroup.eventLoopGroup())
179-
.channelFactory(sdkEventLoopGroup.channelFactory())
180-
.option(ChannelOption.CONNECT_TIMEOUT_MILLIS, configuration.connectTimeoutMillis())
181-
// TODO run some performance tests with and without this.
182-
.remoteAddress(InetSocketAddress.createUnresolved(host, port));
183-
sdkChannelOptions.channelOptions().forEach(bootstrap::option);
184-
185-
return bootstrap;
186+
return bootstrapProvider.createBootstrap(host, port);
186187
}
187188

188189

‎http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/BootstrapProvider.java

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/*
2+
* Copyright 2010-2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.http.nio.netty.internal;
17+
18+
import io.netty.bootstrap.Bootstrap;
19+
import io.netty.channel.ChannelOption;
20+
import java.net.InetSocketAddress;
21+
import software.amazon.awssdk.annotations.SdkInternalApi;
22+
import software.amazon.awssdk.http.nio.netty.SdkEventLoopGroup;
23+
24+
/**
25+
* The primary purpose of this Bootstrap provider is to ensure that all Bootstraps created by it are 'unresolved'
26+
* InetSocketAddress. This is to prevent Netty from caching the resolved address of a host and then re-using it in
27+
* subsequent connection attempts, and instead deferring to the JVM to handle address resolution and caching.
28+
*/
29+
@SdkInternalApi
30+
public class BootstrapProvider {
31+
private final SdkEventLoopGroup sdkEventLoopGroup;
32+
private final NettyConfiguration nettyConfiguration;
33+
private final SdkChannelOptions sdkChannelOptions;
34+
35+
36+
BootstrapProvider(SdkEventLoopGroup sdkEventLoopGroup,
37+
NettyConfiguration nettyConfiguration,
38+
SdkChannelOptions sdkChannelOptions) {
39+
this.sdkEventLoopGroup = sdkEventLoopGroup;
40+
this.nettyConfiguration = nettyConfiguration;
41+
this.sdkChannelOptions = sdkChannelOptions;
42+
}
43+
44+
/**
45+
* Creates a Bootstrap for a specific host and port with an unresolved InetSocketAddress as the remoteAddress.
46+
* @param host The unresolved remote hostname
47+
* @param port The remote port
48+
* @return A newly created Bootstrap using the configuration this provider was initialized with, and having an
49+
* unresolved remote address.
50+
*/
51+
public Bootstrap createBootstrap(String host, int port) {
52+
Bootstrap bootstrap =
53+
new Bootstrap()
54+
.group(sdkEventLoopGroup.eventLoopGroup())
55+
.channelFactory(sdkEventLoopGroup.channelFactory())
56+
.option(ChannelOption.CONNECT_TIMEOUT_MILLIS, nettyConfiguration.connectTimeoutMillis())
57+
.remoteAddress(InetSocketAddress.createUnresolved(host, port));
58+
sdkChannelOptions.channelOptions().forEach(bootstrap::option);
59+
60+
return bootstrap;
61+
}
62+
}

‎http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/internal/AwaitCloseChannelPoolMapTest.java

Lines changed: 67 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,22 +23,26 @@
2323
import static software.amazon.awssdk.http.SdkHttpConfigurationOption.GLOBAL_HTTP_DEFAULTS;
2424
import static software.amazon.awssdk.http.SdkHttpConfigurationOption.TLS_KEY_MANAGERS_PROVIDER;
2525

26-
import com.github.tomakehurst.wiremock.junit.WireMockRule;
27-
import io.netty.channel.Channel;
28-
import io.netty.channel.pool.ChannelPool;
29-
import io.netty.handler.ssl.SslProvider;
30-
import io.netty.util.concurrent.Future;
3126
import java.net.URI;
3227
import java.util.ArrayList;
3328
import java.util.HashMap;
3429
import java.util.List;
3530
import java.util.Map;
3631
import java.util.stream.Collectors;
3732
import java.util.stream.Stream;
33+
34+
import com.github.tomakehurst.wiremock.junit.WireMockRule;
35+
3836
import org.apache.commons.lang3.RandomStringUtils;
3937
import org.junit.After;
4038
import org.junit.Rule;
4139
import org.junit.Test;
40+
import org.mockito.Mockito;
41+
42+
import io.netty.channel.Channel;
43+
import io.netty.channel.pool.ChannelPool;
44+
import io.netty.handler.ssl.SslProvider;
45+
import io.netty.util.concurrent.Future;
4246
import software.amazon.awssdk.http.Protocol;
4347
import software.amazon.awssdk.http.TlsKeyManagersProvider;
4448
import software.amazon.awssdk.http.nio.netty.ProxyConfiguration;
@@ -94,6 +98,63 @@ public void close_underlyingPoolsShouldBeClosed() {
9498
});
9599
}
96100

101+
@Test
102+
public void get_callsInjectedBootstrapProviderCorrectly() {
103+
BootstrapProvider bootstrapProvider = Mockito.spy(
104+
new BootstrapProvider(SdkEventLoopGroup.builder().build(),
105+
new NettyConfiguration(GLOBAL_HTTP_DEFAULTS),
106+
new SdkChannelOptions()));
107+
108+
URI targetUri = URI.create("https://some-awesome-service-1234.amazonaws.com:8080");
109+
110+
AwaitCloseChannelPoolMap.Builder builder =
111+
AwaitCloseChannelPoolMap.builder()
112+
.sdkChannelOptions(new SdkChannelOptions())
113+
.sdkEventLoopGroup(SdkEventLoopGroup.builder().build())
114+
.configuration(new NettyConfiguration(GLOBAL_HTTP_DEFAULTS))
115+
.protocol(Protocol.HTTP1_1)
116+
.maxStreams(100)
117+
.sslProvider(SslProvider.OPENSSL);
118+
119+
channelPoolMap = new AwaitCloseChannelPoolMap(builder, null, bootstrapProvider);
120+
channelPoolMap.get(targetUri);
121+
122+
verify(bootstrapProvider).createBootstrap("some-awesome-service-1234.amazonaws.com", 8080);
123+
}
124+
125+
@Test
126+
public void get_usingProxy_callsInjectedBootstrapProviderCorrectly() {
127+
BootstrapProvider bootstrapProvider = Mockito.spy(
128+
new BootstrapProvider(SdkEventLoopGroup.builder().build(),
129+
new NettyConfiguration(GLOBAL_HTTP_DEFAULTS),
130+
new SdkChannelOptions()));
131+
132+
URI targetUri = URI.create("https://some-awesome-service-1234.amazonaws.com:8080");
133+
Map<URI, Boolean> shouldProxyCache = new HashMap<>();
134+
shouldProxyCache.put(targetUri, true);
135+
136+
ProxyConfiguration proxyConfiguration =
137+
ProxyConfiguration.builder()
138+
.host("localhost")
139+
.port(mockProxy.port())
140+
.build();
141+
142+
AwaitCloseChannelPoolMap.Builder builder =
143+
AwaitCloseChannelPoolMap.builder()
144+
.proxyConfiguration(proxyConfiguration)
145+
.sdkChannelOptions(new SdkChannelOptions())
146+
.sdkEventLoopGroup(SdkEventLoopGroup.builder().build())
147+
.configuration(new NettyConfiguration(GLOBAL_HTTP_DEFAULTS))
148+
.protocol(Protocol.HTTP1_1)
149+
.maxStreams(100)
150+
.sslProvider(SslProvider.OPENSSL);
151+
152+
channelPoolMap = new AwaitCloseChannelPoolMap(builder, shouldProxyCache, bootstrapProvider);
153+
channelPoolMap.get(targetUri);
154+
155+
verify(bootstrapProvider).createBootstrap("localhost", mockProxy.port());
156+
}
157+
97158
@Test
98159
public void usingProxy_usesCachedValueWhenPresent() {
99160
URI targetUri = URI.create("https://some-awesome-service-1234.amazonaws.com");
@@ -117,7 +178,7 @@ public void usingProxy_usesCachedValueWhenPresent() {
117178
.maxStreams(100)
118179
.sslProvider(SslProvider.OPENSSL);
119180

120-
channelPoolMap = new AwaitCloseChannelPoolMap(builder, shouldProxyCache);
181+
channelPoolMap = new AwaitCloseChannelPoolMap(builder, shouldProxyCache, null);
121182

122183
// The target host does not exist so acquiring a channel should fail unless we're configured to connect to
123184
// the mock proxy host for this URI.

‎http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/internal/BootstrapProviderTest.java

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/*
2+
* Copyright 2010-2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.http.nio.netty.internal;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
import static software.amazon.awssdk.http.SdkHttpConfigurationOption.GLOBAL_HTTP_DEFAULTS;
20+
21+
import java.net.InetSocketAddress;
22+
import java.net.SocketAddress;
23+
24+
import org.junit.Test;
25+
import org.junit.runner.RunWith;
26+
import org.mockito.Mock;
27+
import org.mockito.runners.MockitoJUnitRunner;
28+
29+
import io.netty.bootstrap.Bootstrap;
30+
import io.netty.resolver.AddressResolver;
31+
import io.netty.resolver.AddressResolverGroup;
32+
import software.amazon.awssdk.http.nio.netty.SdkEventLoopGroup;
33+
34+
@RunWith(MockitoJUnitRunner.class)
35+
public class BootstrapProviderTest {
36+
private final BootstrapProvider bootstrapProvider =
37+
new BootstrapProvider(SdkEventLoopGroup.builder().build(),
38+
new NettyConfiguration(GLOBAL_HTTP_DEFAULTS),
39+
new SdkChannelOptions());
40+
41+
// IMPORTANT: This unit test asserts that the bootstrap provider creates bootstraps using 'unresolved
42+
// InetSocketAddress'. If this test is replaced or removed, perhaps due to a different implementation of
43+
// SocketAddress, a different test must be created that ensures that the hostname will be resolved on every
44+
// connection attempt and not cached between connection attempts.
45+
@Test
46+
public void createBootstrap_usesUnresolvedInetSocketAddress() {
47+
Bootstrap bootstrap = bootstrapProvider.createBootstrap("some-awesome-service-1234.amazonaws.com", 443);
48+
49+
SocketAddress socketAddress = bootstrap.config().remoteAddress();
50+
51+
assertThat(socketAddress).isInstanceOf(InetSocketAddress.class);
52+
InetSocketAddress inetSocketAddress = (InetSocketAddress)socketAddress;
53+
54+
assertThat(inetSocketAddress.isUnresolved()).isTrue();
55+
}
56+
}

0 commit comments

Comments
 (0)