Skip to content

Commit

Permalink
Migrate to Apache HttpClient / Core 5.x (#2166)
Browse files Browse the repository at this point in the history 
This PR contains changes related to migration to Apache HttpClient and Core 5.x from 4.x with changes that fix build failures. The security plugin needs to retain support for v4 http client for SAML because of the class SamlHTTPMetadataResolver which extends OpenSAML and OpenSAML has a dependency on v4.


Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Andriy Redko <[email protected]>
  • Loading branch information
@cwperks
cwperks authored Oct 18, 2022
1 parent 16b1676 commit 41d68cc
Show file tree
Hide file tree
Showing 112 changed files with 1,212 additions and 646 deletions.
12 changes: 7 additions & 5 deletions build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ buildscript {
version_tokens = opensearch_version.tokenize('-')
opensearch_build = version_tokens[0] + '.0'

common_utils_version = System.getProperty("common_utils.version", '2.1.0.0')
common_utils_version = System.getProperty("common_utils.version", '3.0.0.0-SNAPSHOT')
kafka_version = '3.0.2'

if (buildVersionQualifier) {
Expand Down Expand Up @@ -296,12 +296,15 @@ dependencies {
implementation 'jakarta.annotation:jakarta.annotation-api:1.3.5'
implementation "org.opensearch.plugin:transport-netty4-client:${opensearch_version}"
implementation "org.opensearch.client:opensearch-rest-high-level-client:${opensearch_version}"
implementation "org.apache.httpcomponents.client5:httpclient5-cache:${versions.httpclient5}"
implementation "org.apache.httpcomponents:httpclient:${versions.httpclient}"
implementation "org.apache.httpcomponents:httpcore:${versions.httpcore}"
implementation "org.apache.httpcomponents:httpasyncclient:${versions.httpasyncclient}"
implementation 'com.google.guava:guava:30.0-jre'
implementation 'org.greenrobot:eventbus:3.2.0'
implementation 'commons-cli:commons-cli:1.3.1'
implementation "org.bouncycastle:bcprov-jdk15on:${versions.bouncycastle}"
implementation 'org.ldaptive:ldaptive:1.2.3'
implementation 'org.apache.httpcomponents:httpclient-cache:4.5.13'
implementation 'io.jsonwebtoken:jjwt-api:0.10.8'
implementation('org.apache.cxf:cxf-rt-rs-security-jose:3.4.5') {
exclude(group: 'jakarta.activation', module: 'jakarta.activation-api')
Expand Down Expand Up @@ -348,8 +351,6 @@ dependencies {
implementation 'commons-lang:commons-lang:2.4'
implementation 'commons-collections:commons-collections:3.2.2'
implementation 'com.jayway.jsonpath:json-path:2.4.0'
implementation 'org.apache.httpcomponents:httpclient:4.5.13'
implementation 'org.apache.httpcomponents:httpclient:4.5.13'
implementation 'net.minidev:json-smart:2.4.7'
runtimeOnly 'io.jsonwebtoken:jjwt-impl:0.10.8'
runtimeOnly 'io.jsonwebtoken:jjwt-jackson:0.10.8'
Expand All @@ -373,6 +374,7 @@ dependencies {


implementation 'org.apache.commons:commons-lang3:3.4'
testImplementation "org.opensearch:common-utils:${common_utils_version}"
testImplementation "org.opensearch.plugin:reindex-client:${opensearch_version}"
testImplementation "org.opensearch:opensearch-ssl-config:${opensearch_version}"
testImplementation "org.opensearch.plugin:percolator-client:${opensearch_version}"
Expand All @@ -387,14 +389,14 @@ dependencies {
testImplementation 'com.unboundid:unboundid-ldapsdk:4.0.9'
testImplementation 'javax.servlet:servlet-api:2.5'
testImplementation 'org.apache.httpcomponents:fluent-hc:4.5.13'
testImplementation "org.apache.httpcomponents.client5:httpclient5-fluent:${versions.httpclient5}"
testImplementation "org.apache.kafka:kafka_2.13:${kafka_version}"
testImplementation "org.apache.kafka:kafka_2.13:${kafka_version}:test"
testImplementation "org.apache.kafka:kafka-clients:${kafka_version}:test"
testImplementation 'org.springframework.kafka:spring-kafka-test:2.8.6'
testImplementation 'org.springframework:spring-beans:5.3.20'
testImplementation 'org.junit.jupiter:junit-jupiter:5.8.2'
testImplementation 'org.junit.jupiter:junit-jupiter-api:5.8.2'
testImplementation "org.opensearch:common-utils:${common_utils_version}"
// JUnit build requirement
testCompileOnly 'org.apiguardian:apiguardian-api:1.0.0'
// Kafka test execution
Expand Down
2 changes: 1 addition & 1 deletion src/integrationTest/java/org/opensearch/security/SecurityRolesTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
package org.opensearch.security;

import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
import org.apache.http.HttpStatus;
import org.apache.hc.core5.http.HttpStatus;
import org.junit.ClassRule;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down
6 changes: 3 additions & 3 deletions src/integrationTest/java/org/opensearch/security/http/BasicAuthTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import java.util.List;

import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
import org.apache.http.HttpHeaders;
import org.apache.hc.core5.http.HttpHeaders;
import org.hamcrest.Matchers;
import org.junit.ClassRule;
import org.junit.Test;
Expand All @@ -25,8 +25,8 @@
import org.opensearch.test.framework.cluster.TestRestClient;
import org.opensearch.test.framework.cluster.TestRestClient.HttpResponse;

import static org.apache.http.HttpStatus.SC_OK;
import static org.apache.http.HttpStatus.SC_UNAUTHORIZED;
import static org.apache.hc.core5.http.HttpStatus.SC_OK;
import static org.apache.hc.core5.http.HttpStatus.SC_UNAUTHORIZED;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsStringIgnoringCase;
import static org.hamcrest.Matchers.equalTo;
Expand Down
2 changes: 1 addition & 1 deletion src/integrationTest/java/org/opensearch/security/http/BasicAuthWithoutChallengeTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
package org.opensearch.security.http;

import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
import org.apache.http.HttpHeaders;
import org.apache.hc.core5.http.HttpHeaders;
import org.junit.ClassRule;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down
2 changes: 1 addition & 1 deletion src/integrationTest/java/org/opensearch/security/http/DisabledBasicAuthTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import org.opensearch.test.framework.cluster.TestRestClient;
import org.opensearch.test.framework.cluster.TestRestClient.HttpResponse;

import static org.apache.http.HttpStatus.SC_UNAUTHORIZED;
import static org.apache.hc.core5.http.HttpStatus.SC_UNAUTHORIZED;
import static org.opensearch.security.http.BasicAuthTests.TEST_USER;
import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.DISABLED_AUTHC_HTTPBASIC_INTERNAL;
import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.JWT_AUTH_DOMAIN;
Expand Down
2 changes: 1 addition & 1 deletion src/integrationTest/java/org/opensearch/security/privileges/PrivilegesEvaluatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
package org.opensearch.security.privileges;

import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
import org.apache.http.HttpStatus;
import org.apache.hc.core5.http.HttpStatus;
import org.junit.ClassRule;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down
52 changes: 36 additions & 16 deletions src/integrationTest/java/org/opensearch/test/framework/cluster/OpenSearchClientProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,22 @@
import java.util.stream.Stream;

import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.TrustManagerFactory;

import org.apache.http.Header;
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.nio.conn.ssl.SSLIOSessionStrategy;
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.PoolingAsyncClientConnectionManagerBuilder;
import org.apache.hc.client5.http.nio.AsyncClientConnectionManager;
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.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.opensearch.client.RestClient;
import org.opensearch.client.RestClientBuilder;
Expand Down Expand Up @@ -94,17 +99,32 @@ default TestRestClient getRestClient(UserCredentialsHolder user, Header... heade

default RestHighLevelClient getRestHighLevelClient(UserCredentialsHolder user) {
InetSocketAddress httpAddress = getHttpAddress();
CredentialsProvider credentialsProvider = new BasicCredentialsProvider();
credentialsProvider.setCredentials(AuthScope.ANY, new UsernamePasswordCredentials(user.getName(), user.getPassword()));

BasicCredentialsProvider credentialsProvider = new BasicCredentialsProvider();
credentialsProvider.setCredentials(new AuthScope(null, -1), new UsernamePasswordCredentials(user.getName(), user.getPassword().toCharArray()));
RestClientBuilder.HttpClientConfigCallback configCallback = httpClientBuilder -> {
httpClientBuilder.setDefaultCredentialsProvider(credentialsProvider).setSSLStrategy(
new SSLIOSessionStrategy(getSSLContext(), null, null, NoopHostnameVerifier.INSTANCE));

TlsStrategy tlsStrategy = ClientTlsStrategyBuilder
.create()
.setSslContext(getSSLContext())
.setHostnameVerifier(NoopHostnameVerifier.INSTANCE)
// See please 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 AsyncClientConnectionManager cm = PoolingAsyncClientConnectionManagerBuilder.create()
.setTlsStrategy(tlsStrategy)
.build();

httpClientBuilder.setDefaultCredentialsProvider(credentialsProvider);
httpClientBuilder.setConnectionManager(cm);
return httpClientBuilder;
};

RestClientBuilder builder = RestClient.builder(new HttpHost(httpAddress.getHostString(), httpAddress.getPort(), "https"))
RestClientBuilder builder = RestClient.builder(new HttpHost("https", httpAddress.getHostString(), httpAddress.getPort()))
.setHttpClientConfigCallback(configCallback);


Expand Down
77 changes: 34 additions & 43 deletions src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@
package org.opensearch.test.framework.cluster;

import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.URI;
import java.net.URISyntaxException;
Expand All @@ -40,34 +38,37 @@
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.TimeUnit;

import javax.net.ssl.SSLContext;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonNode;
import org.apache.commons.io.IOUtils;
import org.apache.http.Header;
import org.apache.http.HttpEntity;
import org.apache.http.NameValuePair;
import org.apache.http.client.config.RequestConfig;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpDelete;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpHead;
import org.apache.http.client.methods.HttpOptions;
import org.apache.http.client.methods.HttpPatch;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.client.methods.HttpPut;
import org.apache.http.client.methods.HttpUriRequest;
import org.apache.http.client.utils.URIBuilder;
import org.apache.http.config.SocketConfig;
import org.apache.http.conn.ssl.NoopHostnameVerifier;
import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
import org.apache.http.entity.StringEntity;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.impl.client.HttpClients;
import org.apache.http.message.BasicHeader;
import org.apache.hc.client5.http.classic.methods.HttpDelete;
import org.apache.hc.client5.http.classic.methods.HttpGet;
import org.apache.hc.client5.http.classic.methods.HttpHead;
import org.apache.hc.client5.http.classic.methods.HttpOptions;
import org.apache.hc.client5.http.classic.methods.HttpPatch;
import org.apache.hc.client5.http.classic.methods.HttpPost;
import org.apache.hc.client5.http.classic.methods.HttpPut;
import org.apache.hc.client5.http.classic.methods.HttpUriRequest;
import org.apache.hc.client5.http.config.RequestConfig;
import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse;
import org.apache.hc.client5.http.impl.classic.HttpClientBuilder;
import org.apache.hc.client5.http.impl.classic.HttpClients;
import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder;
import org.apache.hc.client5.http.io.HttpClientConnectionManager;
import org.apache.hc.client5.http.ssl.NoopHostnameVerifier;
import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory;
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HttpEntity;
import org.apache.hc.core5.http.NameValuePair;
import org.apache.hc.core5.http.io.SocketConfig;
import org.apache.hc.core5.http.io.entity.StringEntity;
import org.apache.hc.core5.http.message.BasicHeader;
import org.apache.hc.core5.net.URIBuilder;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

Expand Down Expand Up @@ -137,11 +138,7 @@ public HttpResponse putJson(String path, String body, Header... headers) {
}

private StringEntity toStringEntity(String body) {
try {
return new StringEntity(body);
} catch (UnsupportedEncodingException e) {
throw new RestClientException("Cannot create string entity", e);
}
return new StringEntity(body);
}

public HttpResponse putJson(String path, ToXContentObject body) {
Expand Down Expand Up @@ -215,9 +212,11 @@ protected final CloseableHttpClient getHTTPClient() {
final SSLConnectionSocketFactory sslsf = new SSLConnectionSocketFactory(this.sslContext, protocols, null,
NoopHostnameVerifier.INSTANCE);

hcb.setSSLSocketFactory(sslsf);

hcb.setDefaultSocketConfig(SocketConfig.custom().setSoTimeout(60 * 1000).build());
final HttpClientConnectionManager cm = PoolingHttpClientConnectionManagerBuilder.create()
.setSSLSocketFactory(sslsf)
.setDefaultSocketConfig(SocketConfig.custom().setSoTimeout(60, TimeUnit.SECONDS).build())
.build();
hcb.setConnectionManager(cm);

if (requestConfig != null) {
hcb.setDefaultRequestConfig(requestConfig);
Expand Down Expand Up @@ -254,9 +253,9 @@ public HttpResponse(CloseableHttpResponse inner) throws IllegalStateException, I
} else {
this.body = IOUtils.toString(entity.getContent(), StandardCharsets.UTF_8);
}
this.header = inner.getAllHeaders();
this.statusCode = inner.getStatusLine().getStatusCode();
this.statusReason = inner.getStatusLine().getReasonPhrase();
this.header = inner.getHeaders();
this.statusCode = inner.getCode();
this.statusReason = inner.getReasonPhrase();
inner.close();
}

Expand Down Expand Up @@ -381,14 +380,6 @@ public void setRequestConfig(RequestConfig requestConfig) {
this.requestConfig = requestConfig;
}

public void setLocalAddress(InetAddress inetAddress) {
if (requestConfig == null) {
requestConfig = RequestConfig.custom().setLocalAddress(inetAddress).build();
} else {
requestConfig = RequestConfig.copy(requestConfig).setLocalAddress(inetAddress).build();
}
}

public boolean isSendHTTPClientCertificate() {
return sendHTTPClientCertificate;
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/amazon/dlic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import com.google.common.annotations.VisibleForTesting;
import org.apache.cxf.rs.security.jose.jwt.JwtClaims;
import org.apache.cxf.rs.security.jose.jwt.JwtToken;
import org.apache.http.HttpHeaders;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/amazon/dlic/auth/http/jwt/HTTPJwtAuthenticator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
import io.jsonwebtoken.Jwts;
import io.jsonwebtoken.io.Decoders;
import io.jsonwebtoken.security.WeakKeyException;
import org.apache.http.HttpHeaders;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

Expand Down
Loading

2 comments on commit 41d68cc

@warmachinesocial
Copy link

Choose a reason for hiding this comment

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

Hi this is very big pr, however I had issue in build the project, lot of dependices were not detect, tried out both java 11 and 17 resulting the same issue?

@warmachinesocial
Copy link

Choose a reason for hiding this comment

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

Sorry it's working i was one commit behind

Please sign in to comment.