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.

Sign up for GitHub

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

Migrate to Apache HttpClient / Core 5.x #2166

Merged
merged 38 commits into from
Oct 18, 2022
Merged

Migrate to Apache HttpClient / Core 5.x #2166

Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
b113a11
Migrate to Apache HttpClient / Core 5.x
cwperks Oct 12, 2022
98285f0
Update import in AbstractHTTPJwtAuthenticator
cwperks Oct 12, 2022
0ea54cb
Continue moving to Apache Client5 / Core5
cwperks Oct 13, 2022
621cfb9
Run spotlessApply
cwperks Oct 13, 2022
91a5188
Remove unused imports
cwperks Oct 13, 2022
6af3a37
Use apache client5 in AbstractSecurityUnitTests
cwperks Oct 13, 2022
8857f23
Keep SettingsBasedSSLConfigurator working with apache client4 which i…
cwperks Oct 13, 2022
fa3f62f
Refactor RestHelper for apache http client5
cwperks Oct 13, 2022
79fe129
Run spotlessApply, update IntegrationTests.java and rename SettingsBa…
cwperks Oct 13, 2022
669b4bc
Remove todos after finding v5 analog
cwperks Oct 13, 2022
4ebea9f
Update with new analogs in apache http v5
cwperks Oct 13, 2022
034492e
Run spotlessApply
cwperks Oct 13, 2022
f5eb1cd
Update Apache Client5 imports in tests
cwperks Oct 13, 2022
28d201c
Update imports for HttpStatus
cwperks Oct 13, 2022
74efa20
Update import in SecurityRestTestCase
cwperks Oct 13, 2022
1341878
Sync common utils and opensearch versions
cwperks Oct 13, 2022
0bbd92e
Run spotlessApply
cwperks Oct 13, 2022
39ba151
Remove now unused gradle dependencies
cwperks Oct 13, 2022
81eb84a
Changes tests to use Http 5.x client
DarshitChanpura Oct 13, 2022
8e64c2e
Merge Craig's branch
DarshitChanpura Oct 13, 2022
e83550c
Update AuthScope to be equivalent for AuthScope.ANY from v4
cwperks Oct 13, 2022
03c7519
Merge pull request #1 from DarshitChanpura/apache-client5
cwperks Oct 13, 2022
5ea3dcd
Remove more references to org.apache.http
cwperks Oct 13, 2022
c74f895
Fix various test failures
cwperks Oct 13, 2022
426f7a9
Strip extract / in request uri
cwperks Oct 14, 2022
1e8818c
Change placement of common-utils in build.gradle
cwperks Oct 17, 2022
4d83258
Specify common_utils_version
cwperks Oct 17, 2022
a4337a4
Remove unused import in RestHelper
cwperks Oct 17, 2022
b53fb74
Set connection manager
cwperks Oct 17, 2022
5901d16
Attempt to resolve Invalid protocol version test failure
cwperks Oct 17, 2022
e937d09
Run spotlessApply
cwperks Oct 17, 2022
6db8ffd
Fix issue passing credentials in RestHelper
cwperks Oct 18, 2022
2c23df4
Remove unused import
cwperks Oct 18, 2022
17344c3
Change expected count in BasicAuditlogTest from before doThenWaitForM…
cwperks Oct 18, 2022
adb1100
Make sure to setDefaultCredentialsProvider
cwperks Oct 18, 2022
7b0e3cd
Remove unused code, use variable for so timeout and set charset in We…
cwperks Oct 18, 2022
0d78ccf
Resolved org.apache.hc.core5.http.ParseException: Invalid protocol ve…
reta Oct 18, 2022
77fe540
Removing cipher suites which do not provide adequate security for HTTP/2
reta Oct 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions build.gradle
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
38 changes: 23 additions & 15 deletions src/integrationTest/java/org/opensearch/test/framework/cluster/OpenSearchClientProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,17 @@
import javax.net.ssl.SSLContext;
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.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.BasicClientTlsStrategy;
import org.apache.hc.core5.http.nio.ssl.TlsStrategy;
import org.apache.hc.core5.http2.HttpVersionPolicy;

import org.opensearch.client.RestClient;
import org.opensearch.client.RestClientBuilder;
Expand Down Expand Up @@ -94,17 +96,23 @@ 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 = new BasicClientTlsStrategy(getSSLContext());

final AsyncClientConnectionManager cm = PoolingAsyncClientConnectionManagerBuilder.create()
.setTlsStrategy(tlsStrategy)
.build();

httpClientBuilder.setDefaultCredentialsProvider(credentialsProvider);
httpClientBuilder.setConnectionManager(cm);
// Attempt to resolve org.apache.hc.core5.http.ParseException: Invalid protocol version
httpClientBuilder.setVersionPolicy(HttpVersionPolicy.FORCE_HTTP_1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@cwperks I will look at this one, we should good with HTTP/2 at this point

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
Open in desktop
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
Open in desktop
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
Open in desktop
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