Skip to content

Commit

Permalink
Merge branch 'main' into samikshya-db/newmergedChanges
Browse files Browse the repository at this point in the history
  • Loading branch information
samikshya-db authored Aug 29, 2024
2 parents 3863ea0 + bea4755 commit 862c510
Show file tree
Hide file tree
Showing 11 changed files with 265 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ private void initHttp() {
return;
}
// eventually it'll get decoupled from config.
httpClient = new CommonsHttpClient(this);
httpClient = new CommonsHttpClient.Builder().withDatabricksConfig(this).build();
}

public synchronized Map<String, String> authenticate() throws DatabricksException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ public class ProxyConfig {
private String password;
private ProxyAuthType proxyAuthType;
private Boolean useSystemProperties;
// a list of hosts that should be reached directly, bypassing the proxy.
// This is a list of patterns separated by '|'. The patterns may start or end with a '*' for
// wildcards.
// Any host matching one of these patterns will be reached through a direct connection instead of
// through a proxy.
// More info here: https://docs.oracle.com/javase/8/docs/technotes/guides/net/proxies.html
private String nonProxyHosts;

public enum ProxyAuthType {
// Currently we only support BASIC and SPNEGO
Expand Down Expand Up @@ -80,4 +87,19 @@ public ProxyConfig setUseSystemProperties(Boolean useSystemProperties) {
this.useSystemProperties = useSystemProperties;
return this;
}

public String getNonProxyHosts() {
return nonProxyHosts;
}

/**
* @param nonProxyHosts a list of hosts that should be reached directly, bypassing the proxy. This
* is a list of patterns separated by '|'. The patterns may start or end with a '*' for
* wildcards.
* @return the current ProxyConfig object
*/
public ProxyConfig setNonProxyHosts(String nonProxyHosts) {
this.nonProxyHosts = nonProxyHosts;
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.apache.http.*;
import org.apache.http.client.config.RequestConfig;
import org.apache.http.client.methods.*;
import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
import org.apache.http.entity.InputStreamEntity;
import org.apache.http.entity.StringEntity;
import org.apache.http.impl.client.CloseableHttpClient;
Expand All @@ -36,18 +37,101 @@
import org.slf4j.LoggerFactory;

public class CommonsHttpClient implements HttpClient {
/**
* Builder for CommonsHttpClient. This class is used to construct instances of CommonsHttpClient
* with configurable parameters for the underlying Apache HttpClient.
*/
public static class Builder {
private DatabricksConfig databricksConfig;
private Integer timeoutSeconds;
private ProxyConfig proxyConfig;
private SSLConnectionSocketFactory sslSocketFactory;

/**
* @param databricksConfig The DatabricksConfig to use for the HttpClient. If the
* DatabricksConfig has an httpTimeoutSeconds set, it will be used as the default timeout
* for the HttpClient.
* @return This builder.
*/
public Builder withDatabricksConfig(DatabricksConfig databricksConfig) {
this.databricksConfig = databricksConfig;
return this;
}

/**
* @param timeoutSeconds The timeout in seconds to use for the HttpClient. This will override
* any timeout set in the DatabricksConfig.
* @return This builder.
*/
public Builder withTimeoutSeconds(int timeoutSeconds) {
this.timeoutSeconds = timeoutSeconds;
return this;
}

/**
* @param proxyConfig the proxy configuration to use for the HttpClient.
* @return This builder.
*/
public Builder withProxyConfig(ProxyConfig proxyConfig) {
this.proxyConfig = proxyConfig;
return this;
}

/**
* @param sslSocketFactory the SSLConnectionSocketFactory to use for the HttpClient.
* @return This builder.
*/
public Builder withSslSocketFactory(SSLConnectionSocketFactory sslSocketFactory) {
this.sslSocketFactory = sslSocketFactory;
return this;
}

/** Builds a new instance of CommonsHttpClient with the configured parameters. */
public CommonsHttpClient build() {
return new CommonsHttpClient(this);
}
}

private static final Logger LOG = LoggerFactory.getLogger(CommonsHttpClient.class);
private final PoolingHttpClientConnectionManager connectionManager =
new PoolingHttpClientConnectionManager();
private final CloseableHttpClient hc;
private int timeout;

private CommonsHttpClient(Builder builder) {
int timeoutSeconds = 300;
if (builder.databricksConfig != null
&& builder.databricksConfig.getHttpTimeoutSeconds() != null) {
timeoutSeconds = builder.databricksConfig.getHttpTimeoutSeconds();
}
if (builder.timeoutSeconds != null) {
timeoutSeconds = builder.timeoutSeconds;
}
timeout = timeoutSeconds * 1000;
connectionManager.setMaxTotal(100);
HttpClientBuilder httpClientBuilder =
HttpClientBuilder.create()
.setConnectionManager(connectionManager)
.setDefaultRequestConfig(makeRequestConfig());
if (builder.proxyConfig != null) {
ProxyUtils.setupProxy(builder.proxyConfig, httpClientBuilder);
}
if (builder.sslSocketFactory != null) {
httpClientBuilder.setSSLSocketFactory(builder.sslSocketFactory);
}
hc = httpClientBuilder.build();
}

// These constructors have been deprecate in favour of a builder pattern.
// They will be removed in a future release.
@Deprecated
public CommonsHttpClient(int timeoutSeconds) {
timeout = timeoutSeconds * 1000;
connectionManager.setMaxTotal(100);
hc = makeClosableHttpClient();
}

@Deprecated
public CommonsHttpClient(DatabricksConfig databricksConfig) {
this(
databricksConfig.getHttpTimeoutSeconds() == null
Expand All @@ -56,6 +140,7 @@ public CommonsHttpClient(DatabricksConfig databricksConfig) {
new ProxyConfig(databricksConfig));
}

@Deprecated
public CommonsHttpClient(int timeoutSeconds, ProxyConfig proxyConfig) {
timeout = timeoutSeconds * 1000;
connectionManager.setMaxTotal(100);
Expand All @@ -70,13 +155,15 @@ private RequestConfig makeRequestConfig() {
.build();
}

@Deprecated
private CloseableHttpClient makeClosableHttpClient() {
return HttpClientBuilder.create()
.setConnectionManager(connectionManager)
.setDefaultRequestConfig(makeRequestConfig())
.build();
}

@Deprecated
private CloseableHttpClient makeClosableHttpClient(ProxyConfig proxyConfig) {
HttpClientBuilder builder =
HttpClientBuilder.create()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public static class Builder {
private String clientId;
private String clientSecret;
private String tokenUrl;
private HttpClient hc = new CommonsHttpClient(30);
private HttpClient hc = new CommonsHttpClient.Builder().withTimeoutSeconds(30).build();
private Map<String, String> endpointParams = Collections.emptyMap();
private List<String> scopes = Collections.emptyList();
private AuthParameterPosition position = AuthParameterPosition.BODY;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public class Consent implements Serializable {
private final String clientSecret;

public static class Builder {
private HttpClient hc = new CommonsHttpClient(30);
private HttpClient hc = new CommonsHttpClient.Builder().withTimeoutSeconds(30).build();
private String authUrl;
private String verifier;
private String state;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package com.databricks.sdk.core.utils;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import org.apache.http.HttpException;
import org.apache.http.HttpHost;
import org.apache.http.HttpRequest;
import org.apache.http.conn.routing.HttpRoute;
import org.apache.http.conn.routing.HttpRoutePlanner;
import org.apache.http.impl.conn.DefaultProxyRoutePlanner;
import org.apache.http.protocol.HttpContext;

/**
* Custom route planner that routes requests via a proxy, except for hosts that match a list of
* non-proxy hosts.
*/
public class CustomRoutePlanner implements HttpRoutePlanner {
private final DefaultProxyRoutePlanner defaultRoutePlanner;
private final List<Pattern> nonProxyHostRegex;

public CustomRoutePlanner(HttpHost proxy, String nonProxyHosts) {
this.defaultRoutePlanner = new DefaultProxyRoutePlanner(proxy);
if (nonProxyHosts == null || nonProxyHosts.isEmpty()) {
this.nonProxyHostRegex = new ArrayList<>();
} else {
this.nonProxyHostRegex =
Arrays.stream(nonProxyHosts.split("\\|"))
.map(host -> host.replace(".", "\\.").replace("*", ".*"))
.map(Pattern::compile)
.collect(Collectors.toList());
}
}

@Override
public HttpRoute determineRoute(HttpHost target, HttpRequest request, HttpContext context)
throws HttpException {
String targetHostName = target.getHostName();
if (nonProxyHostRegex.stream().anyMatch(pattern -> pattern.matcher(targetHostName).matches())) {
return new HttpRoute(target); // Direct route, no proxy
}
return defaultRoutePlanner.determineRoute(target, request, context); // Route via proxy
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@ public static void setupProxy(ProxyConfig config, HttpClientBuilder builder) {
proxyAuthType = config.getProxyAuthType();
builder.setProxy(new HttpHost(proxyHost, proxyPort));
}
if (proxyHost == null) {
// No proxy is set in system properties or in the config
return;
}
if (config.getNonProxyHosts() != null) {
builder.setRoutePlanner(
new CustomRoutePlanner(new HttpHost(proxyHost, proxyPort), config.getNonProxyHosts()));
}
setupProxyAuth(proxyHost, proxyPort, proxyAuthType, proxyUser, proxyPassword, builder);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class CommonsHttpClientTest {
@Test
public void itWorks() throws IOException {
try (FixtureServer fixtures = new FixtureServer().with("GET", "/foo?x=y", "bar")) {
HttpClient httpClient = new CommonsHttpClient(30);
HttpClient httpClient = new CommonsHttpClient.Builder().withTimeoutSeconds(30).build();
Request in = new Request("GET", fixtures.getUrl() + "/foo").withQueryParam("x", "y");
Response out = httpClient.execute(in);
assertEquals("bar", out.getDebugBody().trim());
Expand All @@ -37,7 +37,7 @@ public void testStringBody() throws IOException {
.withResponse("quux")
.build();
try (FixtureServer fixtures = new FixtureServer().with(fixture)) {
HttpClient httpClient = new CommonsHttpClient(30);
HttpClient httpClient = new CommonsHttpClient.Builder().withTimeoutSeconds(30).build();
Request in = new Request("POST", fixtures.getUrl() + "/foo", "bar");
Response out = httpClient.execute(in);
assertEquals("quux", out.getDebugBody().trim());
Expand All @@ -58,7 +58,7 @@ public void testInputStreamBody() throws IOException {
.withResponse("quux")
.build();
try (FixtureServer fixtures = new FixtureServer().with(fixture)) {
HttpClient httpClient = new CommonsHttpClient(30);
HttpClient httpClient = new CommonsHttpClient.Builder().withTimeoutSeconds(30).build();
Request in =
new Request("POST", fixtures.getUrl() + "/foo", IOUtils.toInputStream("bar", "UTF-8"));
Response out = httpClient.execute(in);
Expand All @@ -76,7 +76,7 @@ public void testNoRedirection() throws IOException {
.build();

try (FixtureServer fixtures = new FixtureServer().with(fixture)) {
HttpClient httpClient = new CommonsHttpClient(30);
HttpClient httpClient = new CommonsHttpClient.Builder().withTimeoutSeconds(30).build();
Request in = new Request("GET", fixtures.getUrl() + "/redirect");
in.setRedirectionBehavior(
false); // If we don't set redirection behavior to false, we get 200 as it gets
Expand Down Expand Up @@ -106,7 +106,7 @@ public void testRedirection() throws IOException {
.build());

try (FixtureServer server = new FixtureServer().with(fixtures)) {
HttpClient httpClient = new CommonsHttpClient(30);
HttpClient httpClient = new CommonsHttpClient.Builder().withTimeoutSeconds(30).build();
Request in = new Request("GET", server.getUrl() + "/redirect");
Response out = httpClient.execute(in);
assertEquals(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ void clientAndConsentTest() throws IOException {
.setAuthType("external-browser")
.setHost(fixtures.getUrl())
.setClientId("test-client-id")
.setHttpClient(new CommonsHttpClient(30));
.setHttpClient(new CommonsHttpClient.Builder().withTimeoutSeconds(30).build());
config.resolve();

assertEquals("tokenEndPointFromServer", config.getOidcEndpoints().getTokenEndpoint());
Expand Down Expand Up @@ -70,7 +70,7 @@ void clientAndConsentTestWithCustomRedirectUrl() throws IOException {
.setAuthType("external-browser")
.setHost(fixtures.getUrl())
.setClientId("test-client-id")
.setHttpClient(new CommonsHttpClient(30))
.setHttpClient(new CommonsHttpClient.Builder().withTimeoutSeconds(30).build())
.setOAuthRedirectUrl("http://localhost:8010")
.setScopes(Arrays.asList("sql"));
config.resolve();
Expand Down Expand Up @@ -98,7 +98,7 @@ void openIDConnectEndPointsTestAccounts() throws IOException {
new DatabricksConfig()
.setAuthType("external-browser")
.setHost("https://accounts.cloud.databricks.com")
.setHttpClient(new CommonsHttpClient(30))
.setHttpClient(new CommonsHttpClient.Builder().withTimeoutSeconds(30).build())
.setAccountId("testAccountId");
config.resolve();

Expand Down
Loading

0 comments on commit 862c510

Please sign in to comment.