Skip to content

Commit

Permalink
Revert 2.x HttpClient to Apache HttpComponents / Core 4.x
Browse files Browse the repository at this point in the history
Signed-off-by: Daniel Widdis <[email protected]>
  • Loading branch information
dbwiddis committed Dec 20, 2023
1 parent 9c538b9 commit b1ffff7
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,16 @@

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import org.apache.hc.client5.http.auth.AuthScope;
import org.apache.hc.client5.http.auth.UsernamePasswordCredentials;
import org.apache.hc.client5.http.impl.auth.BasicCredentialsProvider;
import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManager;
import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManagerBuilder;
import org.apache.hc.client5.http.ssl.ClientTlsStrategyBuilder;
import org.apache.hc.client5.http.ssl.NoopHostnameVerifier;
import org.apache.hc.core5.function.Factory;
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.message.BasicHeader;
import org.apache.hc.core5.http.nio.ssl.TlsStrategy;
import org.apache.hc.core5.reactor.ssl.TlsDetails;
import org.apache.hc.core5.ssl.SSLContextBuilder;
import org.apache.hc.core5.util.Timeout;
import org.apache.http.Header;
import org.apache.http.HttpHeaders;
import org.apache.http.HttpHost;
import org.apache.http.auth.AuthScope;
import org.apache.http.auth.UsernamePasswordCredentials;
import org.apache.http.client.CredentialsProvider;
import org.apache.http.conn.ssl.NoopHostnameVerifier;
import org.apache.http.impl.client.BasicCredentialsProvider;
import org.apache.http.message.BasicHeader;
import org.apache.http.ssl.SSLContextBuilder;
import org.opensearch.action.search.SearchResponse;
import org.opensearch.client.Request;
import org.opensearch.client.Response;
Expand All @@ -51,8 +45,6 @@
import org.junit.AfterClass;
import org.junit.Before;

import javax.net.ssl.SSLEngine;

import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
Expand All @@ -66,8 +58,6 @@
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;

import static org.opensearch.client.RestClientBuilder.DEFAULT_MAX_CONN_PER_ROUTE;
import static org.opensearch.client.RestClientBuilder.DEFAULT_MAX_CONN_TOTAL;
import static org.opensearch.commons.ConfigConstants.OPENSEARCH_SECURITY_SSL_HTTP_ENABLED;
import static org.opensearch.commons.ConfigConstants.OPENSEARCH_SECURITY_SSL_HTTP_KEYSTORE_FILEPATH;
import static org.opensearch.commons.ConfigConstants.OPENSEARCH_SECURITY_SSL_HTTP_KEYSTORE_KEYPASSWORD;
Expand Down Expand Up @@ -121,6 +111,9 @@ public void setUpSettings() throws Exception {
);
assertEquals(200, response.getStatusLine().getStatusCode());

// Need a delay here on 2.x or next line consistently fails tests.
// TODO: figure out know why we need this and we should pursue a better option that doesn't require HTTP5
Thread.sleep(10000);
// Ensure .plugins-ml-config is created before proceeding with integration tests
assertBusy(() -> { assertTrue(indexExistsWithAdminClient(".plugins-ml-config")); });

Expand Down Expand Up @@ -173,7 +166,7 @@ protected static void deleteIndexWithAdminClient(String name) throws IOException

// Utility fn for checking if an index exists. Should only be used when not allowed in a regular context
// (e.g., checking existence of system indices)
protected static boolean indexExistsWithAdminClient(String indexName) throws IOException {
protected static boolean indexExistsWithAdminClient(String indexName) throws Exception {
Request request = new Request("HEAD", "/" + indexName);
Response response = adminClient().performRequest(request);
return RestStatus.OK.getStatus() == response.getStatusLine().getStatusCode();
Expand Down Expand Up @@ -213,7 +206,7 @@ protected RestClient buildClient(Settings settings, HttpHost[] hosts) throws IOE
@AfterClass
protected static void wipeAllSystemIndices() throws IOException {
Response response = adminClient().performRequest(new Request("GET", "/_cat/indices?format=json&expand_wildcards=all"));
MediaType xContentType = MediaType.fromMediaType(response.getEntity().getContentType());
MediaType xContentType = MediaType.fromMediaType(response.getEntity().getContentType().getValue());
try (
XContentParser parser = xContentType.xContent()
.createParser(
Expand Down Expand Up @@ -252,27 +245,13 @@ protected static void configureHttpsClient(RestClientBuilder builder, Settings s
.orElseThrow(() -> new RuntimeException("user name is missing"));
String password = Optional.ofNullable(System.getProperty("password"))
.orElseThrow(() -> new RuntimeException("password is missing"));
BasicCredentialsProvider credentialsProvider = new BasicCredentialsProvider();
final AuthScope anyScope = new AuthScope(null, -1);
credentialsProvider.setCredentials(anyScope, new UsernamePasswordCredentials(userName, password.toCharArray()));
final CredentialsProvider credentialsProvider = new BasicCredentialsProvider();
credentialsProvider.setCredentials(AuthScope.ANY, new UsernamePasswordCredentials(userName, password));
try {
final TlsStrategy tlsStrategy = ClientTlsStrategyBuilder.create()
.setHostnameVerifier(NoopHostnameVerifier.INSTANCE)
.setSslContext(SSLContextBuilder.create().loadTrustMaterial(null, (chains, authType) -> true).build())
// See https://issues.apache.org/jira/browse/HTTPCLIENT-2219
.setTlsDetailsFactory(new Factory<SSLEngine, TlsDetails>() {
@Override
public TlsDetails create(final SSLEngine sslEngine) {
return new TlsDetails(sslEngine.getSession(), sslEngine.getApplicationProtocol());
}
})
.build();
final PoolingAsyncClientConnectionManager connectionManager = PoolingAsyncClientConnectionManagerBuilder.create()
.setMaxConnPerRoute(DEFAULT_MAX_CONN_PER_ROUTE)
.setMaxConnTotal(DEFAULT_MAX_CONN_TOTAL)
.setTlsStrategy(tlsStrategy)
.build();
return httpClientBuilder.setDefaultCredentialsProvider(credentialsProvider).setConnectionManager(connectionManager);
return httpClientBuilder.setDefaultCredentialsProvider(credentialsProvider)
// disable the certificate since our testing cluster just uses the default security configuration
.setSSLHostnameVerifier(NoopHostnameVerifier.INSTANCE)
.setSSLContext(SSLContextBuilder.create().loadTrustMaterial(null, (chains, authType) -> true).build());
} catch (Exception e) {
throw new RuntimeException(e);
}
Expand All @@ -284,9 +263,9 @@ public TlsDetails create(final SSLEngine sslEngine) {
CLIENT_SOCKET_TIMEOUT
);
builder.setRequestConfigCallback(conf -> {
Timeout timeout = Timeout.ofMilliseconds(Math.toIntExact(socketTimeout.getMillis()));
int timeout = Math.toIntExact(socketTimeout.getMillis());
conf.setConnectTimeout(timeout);
conf.setResponseTimeout(timeout);
conf.setSocketTimeout(timeout);
return conf;
});
if (settings.hasValue(CLIENT_PATH_PREFIX)) {
Expand Down Expand Up @@ -404,7 +383,7 @@ protected SearchResponse searchWorkflows(String query) throws Exception {
assertEquals(RestStatus.OK, TestHelpers.restStatus(restSearchResponse));

// Parse entity content into SearchResponse
MediaType mediaType = MediaType.fromMediaType(restSearchResponse.getEntity().getContentType());
MediaType mediaType = MediaType.fromMediaType(restSearchResponse.getEntity().getContentType().getValue());
try (
XContentParser parser = mediaType.xContent()
.createParser(
Expand Down Expand Up @@ -454,7 +433,7 @@ protected List<ResourceCreated> getResourcesCreated(String workflowId, int timeo
Response response = getWorkflowStatus(workflowId, true);

// Parse workflow state from response and retreieve resources created
MediaType mediaType = MediaType.fromMediaType(response.getEntity().getContentType());
MediaType mediaType = MediaType.fromMediaType(response.getEntity().getContentType().getValue());
try (
XContentParser parser = mediaType.xContent()
.createParser(
Expand Down
14 changes: 7 additions & 7 deletions src/test/java/org/opensearch/flowframework/TestHelpers.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Sets;
import com.google.common.io.Resources;
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HttpEntity;
import org.apache.hc.core5.http.io.entity.StringEntity;
import org.apache.http.Header;
import org.apache.http.HttpEntity;
import org.apache.http.nio.entity.NStringEntity;
import org.apache.logging.log4j.util.Strings;
import org.opensearch.client.Request;
import org.opensearch.client.RequestOptions;
Expand Down Expand Up @@ -47,7 +47,7 @@
import java.util.stream.Stream;

import static org.opensearch.test.OpenSearchTestCase.randomAlphaOfLength;
import static org.apache.hc.core5.http.ContentType.APPLICATION_JSON;
import static org.apache.http.entity.ContentType.APPLICATION_JSON;

public class TestHelpers {

Expand All @@ -74,7 +74,7 @@ public static Response makeRequest(
String jsonEntity,
List<Header> headers
) throws IOException {
HttpEntity httpEntity = Strings.isBlank(jsonEntity) ? null : new StringEntity(jsonEntity, APPLICATION_JSON);
HttpEntity httpEntity = Strings.isBlank(jsonEntity) ? null : new NStringEntity(jsonEntity, APPLICATION_JSON);
return makeRequest(client, method, endpoint, params, httpEntity, headers);
}

Expand Down Expand Up @@ -117,11 +117,11 @@ public static Response makeRequest(
}

public static HttpEntity toHttpEntity(ToXContentObject object) throws IOException {
return new StringEntity(toJsonString(object), APPLICATION_JSON);
return new NStringEntity(toJsonString(object), APPLICATION_JSON);
}

public static HttpEntity toHttpEntity(String jsonString) throws IOException {
return new StringEntity(jsonString, APPLICATION_JSON);
return new NStringEntity(jsonString, APPLICATION_JSON);
}

public static RestStatus restStatus(Response response) {
Expand Down

0 comments on commit b1ffff7

Please sign in to comment.