Skip to content

Commit

Permalink
Add Support for Environment Proxy Configurations (#7593)
Browse files Browse the repository at this point in the history 
* Implementation idea to support loading proxy environment variables

* Beginning to add Netty tests

* Added tests for ProxyOptions

* Adding tests, removing classes which are no longer being used

* Fix linting issue

* Fixed failing tests

* Updating tests to handle configuration that is setup in the environment

* Updates based on PR comments

* Suppress static logger check

* Updated log messages and internal code

* Added comment to explain the expected format for nonProxyHosts in ProxyOptions
  • Loading branch information
@alzimmermsft
alzimmermsft authored Jan 29, 2020
1 parent 34cbb46 commit 5a19707
Show file tree
Hide file tree
Showing 8 changed files with 595 additions and 102 deletions.
1 change: 1 addition & 0 deletions eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@
<!-- Use the logger in Utility static method. -->
<suppress checks="com.azure.tools.checkstyle.checks.GoodLoggingCheck" files="com.azure.storage.common.Utility.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.GoodLoggingCheck" files="com.azure.storage.common.implementation.StorageImplUtils.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.GoodLoggingCheck" files="com.azure.core.http.ProxyOptions.java"/>

<!-- Azure Core Test -->
<suppress checks="com.azure.tools.checkstyle.checks.GoodLoggingCheck" files=".*[/\\]core[/\\]test[/\\].*\.java"/>
Expand Down
1 change: 1 addition & 0 deletions pom.client.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -927,6 +927,7 @@
<!-- JUnit5 and Mockito use reflection to create objects. -->
--add-opens com.azure.core/com.azure.core=ALL-UNNAMED
--add-opens com.azure.core/com.azure.core.util.polling=ALL-UNNAMED
--add-opens com.azure.core/com.azure.core.http=ALL-UNNAMED
--add-opens com.azure.core/com.azure.core.http.policy=ALL-UNNAMED
--add-opens com.azure.core/com.azure.core.util=ALL-UNNAMED
--add-opens com.azure.ai.textanalytics/com.azure.ai.textanalytics=ALL-UNNAMED
Expand Down
104 changes: 66 additions & 38 deletions ...-core-http-netty/src/main/java/com/azure/core/http/netty/NettyAsyncHttpClientBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package com.azure.core.http.netty;

import com.azure.core.http.ProxyOptions;
import com.azure.core.util.Configuration;
import com.azure.core.util.logging.ClientLogger;
import io.netty.channel.nio.NioEventLoopGroup;
import reactor.netty.http.client.HttpClient;
Expand Down Expand Up @@ -31,18 +32,19 @@ public class NettyAsyncHttpClientBuilder {
private boolean enableWiretap;
private int port = 80;
private NioEventLoopGroup nioEventLoopGroup;
private Configuration configuration;

/**
* Creates a new builder instance, where a builder is capable of generating multiple instances of
* {@link NettyAsyncHttpClient}.
* Creates a new builder instance, where a builder is capable of generating multiple instances of {@link
* NettyAsyncHttpClient}.
*/
public NettyAsyncHttpClientBuilder() {
this.baseHttpClient = null;
}

/**
* Creates a new builder instance, where a builder is capable of generating multiple instances of
* {@link NettyAsyncHttpClient} based on the provided reactor netty HttpClient.
* Creates a new builder instance, where a builder is capable of generating multiple instances of {@link
* NettyAsyncHttpClient} based on the provided reactor netty HttpClient.
*
* {@codesnippet com.azure.core.http.netty.from-existing-http-client}
*
Expand All @@ -53,8 +55,8 @@ public NettyAsyncHttpClientBuilder(HttpClient nettyHttpClient) {
}

/**
* Creates a new Netty-backed {@link com.azure.core.http.HttpClient} instance on every call, using the
* configuration set in the builder at the time of the build method call.
* Creates a new Netty-backed {@link com.azure.core.http.HttpClient} instance on every call, using the configuration
* set in the builder at the time of the build method call.
*
* @return A new Netty-backed {@link com.azure.core.http.HttpClient} instance.
* @throws IllegalStateException If the builder is configured to use an unknown proxy type.
Expand All @@ -70,6 +72,11 @@ public com.azure.core.http.HttpClient build() {
} else {
nettyHttpClient = this.baseHttpClient == null ? HttpClient.create() : this.baseHttpClient;
}

Configuration buildConfiguration = (configuration == null)
? Configuration.getGlobalConfiguration()
: configuration;

nettyHttpClient = nettyHttpClient
.port(port)
.wiretap(enableWiretap)
Expand All @@ -78,50 +85,57 @@ public com.azure.core.http.HttpClient build() {
tcpConfig = tcpConfig.runOn(nioEventLoopGroup);
}

if (proxyOptions != null) {
ProxyProvider.Proxy nettyProxy;
switch (proxyOptions.getType()) {
case HTTP:
nettyProxy = ProxyProvider.Proxy.HTTP;
break;
case SOCKS4:
nettyProxy = ProxyProvider.Proxy.SOCKS4;
break;
case SOCKS5:
nettyProxy = ProxyProvider.Proxy.SOCKS5;
break;
default:
throw logger.logExceptionAsError(new IllegalStateException(
String.format("Unknown Proxy type '%s' in use. Not configuring Netty proxy.",
proxyOptions.getType())));
}
if (proxyOptions.getUsername() != null) {
// Netty supports only Basic proxy authentication and we default to it.
return tcpConfig.proxy(ts -> ts.type(nettyProxy)
.address(proxyOptions.getAddress())
.username(proxyOptions.getUsername())
.password(userName -> proxyOptions.getPassword())
.build());
} else {
return tcpConfig.proxy(ts -> ts.type(nettyProxy).address(proxyOptions.getAddress()));
}
ProxyOptions buildProxyOptions = (proxyOptions == null)
? ProxyOptions.fromConfiguration(buildConfiguration)
: proxyOptions;

if (buildProxyOptions != null) {
tcpConfig = tcpConfig.proxy(typeSpec ->
typeSpec.type(mapProxyType(buildProxyOptions.getType(), logger))
.address(proxyOptions.getAddress())
.username(proxyOptions.getUsername())
.password(user -> proxyOptions.getPassword())
.nonProxyHosts(proxyOptions.getNonProxyHosts()));
}

return tcpConfig;
});

return new NettyAsyncHttpClient(nettyHttpClient);
}

/*
* Maps a 'ProxyOptions.Type' to a 'ProxyProvider.Proxy', if the type is unknown or cannot be mapped an
* IllegalStateException will be thrown.
*/
private static ProxyProvider.Proxy mapProxyType(ProxyOptions.Type type, ClientLogger logger) {
Objects.requireNonNull(type, "'ProxyOptions.getType()' cannot be null.");

switch (type) {
case HTTP:
return ProxyProvider.Proxy.HTTP;
case SOCKS4:
return ProxyProvider.Proxy.SOCKS4;
case SOCKS5:
return ProxyProvider.Proxy.SOCKS5;
default:
throw logger.logExceptionAsError(new IllegalStateException(
String.format("Unknown proxy type '%s' in use. Use a proxy type from 'ProxyOptions.Type'.", type)));
}
}

/**
* Sets the connection provider.
*
* @param connectionProvider the connection provider
* @return the updated {@link NettyAsyncHttpClientBuilder} object
* @return the updated {@link NettyAsyncHttpClientBuilder} object.
*/
public NettyAsyncHttpClientBuilder connectionProvider(ConnectionProvider connectionProvider) {
// Enables overriding the default reactor-netty connection/channel pool.
this.connectionProvider = connectionProvider;
return this;
}

/**
* Sets the {@link ProxyOptions proxy options} that the client will use.
*
Expand All @@ -130,7 +144,7 @@ public NettyAsyncHttpClientBuilder connectionProvider(ConnectionProvider connect
* {@codesnippet com.azure.core.http.netty.NettyAsyncHttpClientBuilder#proxy}
*
* @param proxyOptions The proxy configuration to use.
* @return the updated NettyAsyncHttpClientBuilder object
* @return the updated NettyAsyncHttpClientBuilder object.
*/
public NettyAsyncHttpClientBuilder proxy(ProxyOptions proxyOptions) {
// proxyOptions can be null
Expand All @@ -142,7 +156,7 @@ public NettyAsyncHttpClientBuilder proxy(ProxyOptions proxyOptions) {
* Enables the Netty wiretap feature.
*
* @param enableWiretap Flag indicating wiretap status
* @return the updated NettyAsyncHttpClientBuilder object
* @return the updated NettyAsyncHttpClientBuilder object.
*/
public NettyAsyncHttpClientBuilder wiretap(boolean enableWiretap) {
this.enableWiretap = enableWiretap;
Expand All @@ -153,7 +167,7 @@ public NettyAsyncHttpClientBuilder wiretap(boolean enableWiretap) {
* Sets the port which this client should connect, which by default will be set to port 80.
*
* @param port The port to connect to.
* @return the updated NettyAsyncHttpClientBuilder object
* @return the updated NettyAsyncHttpClientBuilder object.
*/
public NettyAsyncHttpClientBuilder port(int port) {
this.port = port;
Expand All @@ -168,10 +182,24 @@ public NettyAsyncHttpClientBuilder port(int port) {
* {@codesnippet com.azure.core.http.netty.NettyAsyncHttpClientBuilder#nioEventLoopGroup}
*
* @param nioEventLoopGroup The {@link NioEventLoopGroup} that will run IO loops.
* @return the updated NettyAsyncHttpClientBuilder object
* @return the updated NettyAsyncHttpClientBuilder object.
*/
public NettyAsyncHttpClientBuilder nioEventLoopGroup(NioEventLoopGroup nioEventLoopGroup) {
this.nioEventLoopGroup = nioEventLoopGroup;
return this;
}

/**
* Sets the configuration store that is used during construction of the HTTP client.
* <p>
* The default configuration store is a clone of the {@link Configuration#getGlobalConfiguration() global
* configuration store}, use {@link Configuration#NONE} to bypass using configuration settings during construction.
*
* @param configuration The configuration store used to
* @return The updated NettyAsyncHttpClientBuilder object.
*/
public NettyAsyncHttpClientBuilder configuration(Configuration configuration) {
this.configuration = configuration;
return this;
}
}
38 changes: 16 additions & 22 deletions ...zure-core-http-netty/src/test/java/com/azure/core/http/netty/ReactorNettyClientTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@

import static com.azure.core.http.netty.NettyAsyncHttpClient.ReactorNettyHttpResponse;
import static java.time.Duration.ofMillis;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTimeout;

public class ReactorNettyClientTests {
Expand Down Expand Up @@ -179,8 +181,7 @@ public void testServerShutsDownSocketShouldPushErrorToContentFlowable() {
assertTimeout(ofMillis(5000), () -> {
CountDownLatch latch = new CountDownLatch(1);
AtomicReference<Socket> sock = new AtomicReference<>();
ServerSocket ss = new ServerSocket(0);
try {
try (ServerSocket ss = new ServerSocket(0)) {
Mono.fromCallable(() -> {
latch.countDown();
Socket socket = ss.accept();
Expand Down Expand Up @@ -210,15 +211,13 @@ public void testServerShutsDownSocketShouldPushErrorToContentFlowable() {
HttpClient client = HttpClient.createDefault();
HttpRequest request = new HttpRequest(HttpMethod.GET,
new URL("http://localhost:" + ss.getLocalPort() + "/get"));

HttpResponse response = client.send(request).block();
Assertions.assertEquals(200, response.getStatusCode());
System.out.println("reading body");
//
assertNotNull(response);
assertEquals(200, response.getStatusCode());

StepVerifier.create(response.getBodyAsByteArray())
// .awaitDone(20, TimeUnit.SECONDS)
.verifyError(IOException.class);
} finally {
ss.close();
}
});
}
Expand Down Expand Up @@ -291,10 +290,7 @@ private static MessageDigest md5Digest() {
}

private static byte[] digest(String s) throws NoSuchAlgorithmException {
MessageDigest md = MessageDigest.getInstance("MD5");
md.update(s.getBytes(StandardCharsets.UTF_8));
byte[] expectedDigest = md.digest();
return expectedDigest;
return MessageDigest.getInstance("MD5").digest(s.getBytes(StandardCharsets.UTF_8));
}

private static final class NumberedByteBuf {
Expand Down Expand Up @@ -326,24 +322,22 @@ private static URL url(WireMockServer server, String path) {
}

private static String createLongBody() {
StringBuilder s = new StringBuilder(10000000);
StringBuilder builder = new StringBuilder("abcdefghijk".length() * 1000000);
for (int i = 0; i < 1000000; i++) {
s.append("abcdefghijk");
builder.append("abcdefghijk");
}
return s.toString();

return builder.toString();
}

private void checkBodyReceived(String expectedBody, String path) {
NettyAsyncHttpClient client = new NettyAsyncHttpClient();
HttpResponse response = doRequest(client, path);
String s = new String(response.getBodyAsByteArray().block(),
StandardCharsets.UTF_8);
Assertions.assertEquals(expectedBody, s);
StepVerifier.create(doRequest(new NettyAsyncHttpClient(), path).getBodyAsByteArray())
.assertNext(bytes -> assertEquals(expectedBody, new String(bytes, StandardCharsets.UTF_8)))
.verifyComplete();
}

private ReactorNettyHttpResponse doRequest(NettyAsyncHttpClient client, String path) {
HttpRequest request = new HttpRequest(HttpMethod.GET, url(server, path));
ReactorNettyHttpResponse response = (ReactorNettyHttpResponse) client.send(request).block();
return response;
return (ReactorNettyHttpResponse) client.send(request).block();
}
}
Loading

0 comments on commit 5a19707

Please sign in to comment.