Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add custom sslcontext support in fili #943

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ Current

### Added:

- [Add custom ssl context support to druid AsyncHttp requests](https://github.com/yahoo/fili/pull/943)
* Added new Constructors in `AsyncDruidWebServiceImpl` class that accept custom `SslContext` as additional argument.
This custom SslContext if not null, replaces the default ssl context while making the request to druid.
* Added `getSSLContext()` method in `AbstractBinderFactory` class that returns null as default. Custom Ssl Context is passed by overriding this method.

### Changed:

- [Refactored sample applications into distinct submodules](https://github.com/yahoo/fili/issues/977)
Expand Down Expand Up @@ -2338,3 +2343,5 @@ Jobs resource. Here are the highlights of what's in this release:

- [`DruidDimensionsLoader` doesn't set the dimension's lastUpdated date](https://github.com/yahoo/fili/pull/24)
* `DruidDimensionsLoader` now properly sets the `lastUpdated` field after it finished processing the Druid response

### Added:
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@
import com.codahale.metrics.health.HealthCheckRegistry;
import com.fasterxml.jackson.databind.ObjectMapper;

import io.netty.handler.ssl.SslContext;
import org.glassfish.hk2.api.TypeLiteral;
import org.glassfish.hk2.utilities.Binder;
import org.glassfish.hk2.utilities.binding.AbstractBinder;
Expand Down Expand Up @@ -1210,6 +1211,14 @@ protected Class<? extends PhysicalTableResolver> getPhysicalTableResolver() {
return new HashMap<>(0);
}

/**
* Get a custom configured ssl context. If null, the system default SSL Context will be applied
* @return SSL context
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add to comment:

If null, the system default SSL Context will be applied

*/
protected SslContext getSSLContext() {
return null;
}

/**
* Create a DruidWebService.
* <p>
Expand All @@ -1227,11 +1236,12 @@ protected DruidWebService buildDruidWebService(DruidServiceConfig druidServiceCo
druidServiceConfig,
mapper,
supplier,
getSSLContext(),
new HeaderNestingJsonBuilderStrategy(
AsyncDruidWebServiceImpl.DEFAULT_JSON_NODE_BUILDER_STRATEGY
)
)
: new AsyncDruidWebServiceImpl(druidServiceConfig, mapper, supplier);
: new AsyncDruidWebServiceImpl(druidServiceConfig, mapper, supplier, getSSLContext());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.ObjectWriter;

import io.netty.handler.ssl.SslContext;
import org.asynchttpclient.AsyncCompletionHandler;
import org.asynchttpclient.AsyncHttpClient;
import org.asynchttpclient.AsyncHttpClientConfig;
Expand Down Expand Up @@ -109,7 +110,7 @@ public AsyncDruidWebServiceImpl(
) {
this(
serviceConfig,
initializeWebClient(serviceConfig.getTimeout()),
initializeWebClient(serviceConfig.getTimeout(), null),
mapper,
HashMap::new,
DEFAULT_JSON_NODE_BUILDER_STRATEGY
Expand Down Expand Up @@ -150,30 +151,81 @@ public AsyncDruidWebServiceImpl(
) {
this(
serviceConfig,
initializeWebClient(serviceConfig.getTimeout()),
initializeWebClient(serviceConfig.getTimeout(), null),
mapper,
headersToAppend,
DEFAULT_JSON_NODE_BUILDER_STRATEGY
);
}

/**
* Friendly non-DI constructor useful for manual tests.
* <p>
* This constructor uses default JSON builder, which only uses response body to build the JSON response.
*
* @param serviceConfig Configuration for the Druid Service
* @param mapper A shared jackson object mapper resource
* @param headersToAppend Supplier for map of headers for Druid requests
* @param sslContext Custom Configured SslContext (null if default SSL context has to be used)
*/
public AsyncDruidWebServiceImpl(
DruidServiceConfig serviceConfig,
ObjectMapper mapper,
Supplier<Map<String, String>> headersToAppend,
SslContext sslContext
) {
this(
serviceConfig,
initializeWebClient(serviceConfig.getTimeout(), sslContext),
mapper,
headersToAppend,
DEFAULT_JSON_NODE_BUILDER_STRATEGY
);
}

/**
* Friendly non-DI constructor useful for manual tests.
*
* @param serviceConfig Configuration for the Druid Service
* @param mapper A shared jackson object mapper resource
* @param headersToAppend Supplier for map of headers for Druid requests
* @param jsonNodeBuilderStrategy A function to build JSON nodes from the response
*/
public AsyncDruidWebServiceImpl(
DruidServiceConfig serviceConfig,
ObjectMapper mapper,
Supplier<Map<String, String>> headersToAppend,
Function<Response, JsonNode> jsonNodeBuilderStrategy
) {
this(
serviceConfig,
initializeWebClient(serviceConfig.getTimeout(), null),
mapper,
headersToAppend,
jsonNodeBuilderStrategy
);
}


/**
* Friendly non-DI constructor useful for manual tests.
*
* @param serviceConfig Configuration for the Druid Service
* @param mapper A shared jackson object mapper resource
* @param headersToAppend Supplier for map of headers for Druid requests
* @param jsonNodeBuilderStrategy A function to build JSON nodes from the response
* @param sslContext Custom Configured SslContext (null if default SSL context has to be used)
*/
public AsyncDruidWebServiceImpl(
DruidServiceConfig serviceConfig,
ObjectMapper mapper,
Supplier<Map<String, String>> headersToAppend,
SslContext sslContext,
Function<Response, JsonNode> jsonNodeBuilderStrategy
) {
this(
serviceConfig,
initializeWebClient(serviceConfig.getTimeout()),
initializeWebClient(serviceConfig.getTimeout(), sslContext),
mapper,
headersToAppend,
jsonNodeBuilderStrategy
Expand Down Expand Up @@ -237,10 +289,11 @@ public AsyncDruidWebServiceImpl(
* Initialize the client config.
*
* @param requestTimeout Timeout to use for the client configuration.
* @param sslContext Custom Configured SslContext. (null if default ssl context has to be used)
*
* @return the set up client
*/
private static AsyncHttpClient initializeWebClient(int requestTimeout) {
private static AsyncHttpClient initializeWebClient(int requestTimeout, SslContext sslContext) {

LOG.debug("Druid request timeout: {}ms", requestTimeout);
List<String> cipherSuites = SYSTEM_CONFIG.getListProperty(SSL_ENABLED_CIPHER_KEY, null);
Expand All @@ -257,6 +310,7 @@ private static AsyncHttpClient initializeWebClient(int requestTimeout) {
.setPooledConnectionIdleTimeout(requestTimeout)
.setEnabledCipherSuites(enabledCipherSuites)
.setFollowRedirect(true)
.setSslContext(sslContext)
.build();

return new DefaultAsyncHttpClient(config);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import com.yahoo.bard.webservice.druid.model.query.WeightEvaluationQuery
import com.fasterxml.jackson.databind.ObjectMapper

import io.netty.handler.codec.http.HttpHeaders
import io.netty.handler.ssl.SslContext
import org.asynchttpclient.AsyncHttpClient;
import spock.lang.Specification

import java.util.function.Supplier
Expand Down Expand Up @@ -71,4 +73,16 @@ class AsyncDruidWebServiceImplSpec extends Specification {
assert actualHeaders.get(header.getKey()) == header.getValue()
}
}

def "Check whether given ssl context was set in HttpClient"() {
setup:
SslContext sslContext = Mock(SslContext);

when:
AsyncHttpClient httpClient = AsyncDruidWebServiceImpl.initializeWebClient(6000, sslContext);

then:
assert httpClient.getConfig().getSslContext() == sslContext;

}
}