Migrate to Apache HttpClient / Core 5.x #2166

cwperks · 2022-10-13T17:07:51Z

Signed-off-by: Craig Perkins cwperx@amazon.com

Description

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.

CI will fail until new artifacts for common-utils are available after this PR is merged - opensearch-project/common-utils#281

I updated build.gradle to keep opensearch and common-utils in sync, this can be changed back if needed but to get CI working for this PR it needs to reference the latest distribution build.

Category (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation)

Enhancement

Issues Resolved

Related to opensearch-project/OpenSearch#4256, security plugin needs to upgrade as well

Addresses: 2165

Testing

CI

Check List

New functionality includes testing
New functionality has been documented
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Craig Perkins <cwperx@amazon.com>

…s needed by OpenSAML Signed-off-by: Craig Perkins <cwperx@amazon.com>

Signed-off-by: Craig Perkins <cwperx@amazon.com>

…sedSSLConfiguratorTest to v4 Signed-off-by: Craig Perkins <cwperx@amazon.com>

Signed-off-by: Craig Perkins <cwperx@amazon.com>

peternied · 2022-10-13T17:39:40Z

src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java

-			requestConfig = RequestConfig.copy(requestConfig).setLocalAddress(inetAddress).build();
-		}
-	}
+	// TODO Can this be removed?


If this is unused, yes remove it.

And we can do this for any unused public members under src/integrationTest

peternied · 2022-10-13T17:42:24Z

build.gradle

@@ -394,7 +398,7 @@ dependencies {
    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}"
+    testImplementation "org.opensearch:common-utils:${opensearch_version}"


Could not resolve org.opensearch:common-utils:3.0.0-SNAPSHOT

We need this for the integration test dependenices? It seems like we might need to hold off on this change until we can update common-utils first :/

When new artifacts are available I will re-trigger the CI

org.opensearch.commons.rest.SecureRestClientBuilder fails as it is not compatible with v5. Common-utils has a open PR: opensearch-project/common-utils#281 out to address this

Signed-off-by: Craig Perkins <cwperx@amazon.com>

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

Signed-off-by: Craig Perkins <cwperx@amazon.com>

Update remaining tests to use http client 5

Signed-off-by: Craig Perkins <cwperx@amazon.com>

reta · 2022-10-18T12:28:28Z

src/integrationTest/java/org/opensearch/test/framework/cluster/OpenSearchClientProvider.java

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


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

src/main/java/com/amazon/dlic/util/SettingsBasedSSLConfigurator.java

src/main/java/org/opensearch/security/auditlog/sink/WebhookSink.java

cwperks · 2022-10-18T13:12:38Z

CI is now green! :)

@cwperks great one, I think I can close #2172 ?

Yes, unless this PR missed the mark on the migration somewhere. I was looking at your PR in core (opensearch-project/OpenSearch#4459) and the migration guide provided by apache to find the right analogs in v5 to migrate to.

Please push directly on this branch with any changes you think are appropriate. I was not able to resolve the org.apache.hc.core5.http.ParseException: Invalid protocol version errors with Http 1.1 / Http2

…bhookSink Signed-off-by: Craig Perkins <cwperx@amazon.com>

reta · 2022-10-18T13:27:29Z

Please push directly on this branch with any changes you think are appropriate. I was not able to resolve the org.apache.hc.core5.http.ParseException: Invalid protocol version errors with Http 1.1 / Http2

I will take a look at it, could you please add me as collaborator to your branch (I think I do not have permissions to push there), thank you.

cwperks · 2022-10-18T13:29:03Z

@reta I just added you as a collaborator - you should have received an invite.

peternied · 2022-10-18T14:35:10Z

There are some follow-ups but I'd rather do that with a building main - great work @cwperks

reta · 2022-10-18T15:07:48Z

There are some follow-ups but I'd rather do that with a building main - great work @cwperks

@peternied fix on the way, hold on please

…rsion Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

reta · 2022-10-18T17:32:43Z

@peternied all set. @cwperks great work!

DarshitChanpura

Ty @reta and @cwperks !!

cwperks · 2022-10-18T18:00:24Z

Thank you for the help @reta!

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 <cwperx@amazon.com> Signed-off-by: Darshit Chanpura <dchanp@amazon.com> Signed-off-by: Andriy Redko <andriy.redko@aiven.io> Signed-off-by: Stephen Crawford <steecraw@amazon.com>

cwperks added 17 commits October 12, 2022 18:19

Migrate to Apache HttpClient / Core 5.x

b113a11

Signed-off-by: Craig Perkins <cwperx@amazon.com>

Update import in AbstractHTTPJwtAuthenticator

98285f0

Signed-off-by: Craig Perkins <cwperx@amazon.com>

Continue moving to Apache Client5 / Core5

0ea54cb

Signed-off-by: Craig Perkins <cwperx@amazon.com>

Run spotlessApply

621cfb9

Signed-off-by: Craig Perkins <cwperx@amazon.com>

Remove unused imports

91a5188

Signed-off-by: Craig Perkins <cwperx@amazon.com>

Use apache client5 in AbstractSecurityUnitTests

6af3a37

Signed-off-by: Craig Perkins <cwperx@amazon.com>

Keep SettingsBasedSSLConfigurator working with apache client4 which i…

8857f23

…s needed by OpenSAML Signed-off-by: Craig Perkins <cwperx@amazon.com>

Refactor RestHelper for apache http client5

fa3f62f

Signed-off-by: Craig Perkins <cwperx@amazon.com>

Run spotlessApply, update IntegrationTests.java and rename SettingsBa…

79fe129

…sedSSLConfiguratorTest to v4 Signed-off-by: Craig Perkins <cwperx@amazon.com>

Remove todos after finding v5 analog

669b4bc

Signed-off-by: Craig Perkins <cwperx@amazon.com>

Update with new analogs in apache http v5

4ebea9f

Signed-off-by: Craig Perkins <cwperx@amazon.com>

Run spotlessApply

034492e

Signed-off-by: Craig Perkins <cwperx@amazon.com>

Update Apache Client5 imports in tests

f5eb1cd

Signed-off-by: Craig Perkins <cwperx@amazon.com>

Update imports for HttpStatus

28d201c

Signed-off-by: Craig Perkins <cwperx@amazon.com>

Update import in SecurityRestTestCase

74efa20

Signed-off-by: Craig Perkins <cwperx@amazon.com>

Sync common utils and opensearch versions

1341878

Signed-off-by: Craig Perkins <cwperx@amazon.com>

Run spotlessApply

0bbd92e

Signed-off-by: Craig Perkins <cwperx@amazon.com>

cwperks changed the title ~~Apache client5~~ Migrate to Apache HttpClient / Core 5.x Oct 13, 2022

peternied reviewed Oct 13, 2022

View reviewed changes

cwperks and others added 11 commits October 13, 2022 14:04

Remove now unused gradle dependencies

39ba151

Signed-off-by: Craig Perkins <cwperx@amazon.com>

Changes tests to use Http 5.x client

81eb84a

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

Merge Craig's branch

8e64c2e

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

Update AuthScope to be equivalent for AuthScope.ANY from v4

e83550c

Signed-off-by: Craig Perkins <cwperx@amazon.com>

Merge pull request #1 from DarshitChanpura/apache-client5

03c7519

Update remaining tests to use http client 5

Remove more references to org.apache.http

5ea3dcd

Signed-off-by: Craig Perkins <cwperx@amazon.com>

Fix various test failures

c74f895

Signed-off-by: Craig Perkins <cwperx@amazon.com>

Strip extract / in request uri

426f7a9

Signed-off-by: Craig Perkins <cwperx@amazon.com>

Change placement of common-utils in build.gradle

1e8818c

Signed-off-by: Craig Perkins <cwperx@amazon.com>

Specify common_utils_version

4d83258

Signed-off-by: Craig Perkins <cwperx@amazon.com>

Remove unused import in RestHelper

a4337a4

Signed-off-by: Craig Perkins <cwperx@amazon.com>

reta reviewed Oct 18, 2022

View reviewed changes

src/main/java/com/amazon/dlic/util/SettingsBasedSSLConfigurator.java Outdated Show resolved Hide resolved

reta reviewed Oct 18, 2022

View reviewed changes

src/main/java/org/opensearch/security/auditlog/sink/WebhookSink.java Outdated Show resolved Hide resolved

reta reviewed Oct 18, 2022

View reviewed changes

src/main/java/org/opensearch/security/auditlog/sink/WebhookSink.java Outdated Show resolved Hide resolved

reta reviewed Oct 18, 2022

View reviewed changes

src/main/java/org/opensearch/security/auditlog/sink/WebhookSink.java Outdated Show resolved Hide resolved

Remove unused code, use variable for so timeout and set charset in We…

7b0e3cd

…bhookSink Signed-off-by: Craig Perkins <cwperx@amazon.com>

peternied previously approved these changes Oct 18, 2022

View reviewed changes

reta mentioned this pull request Oct 18, 2022

Migrate client transports to Apache HttpClient / Core 5.x #2172

Closed

3 tasks

Resolved org.apache.hc.core5.http.ParseException: Invalid protocol ve…

0d78ccf

…rsion Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

reta dismissed peternied’s stale review via 0d78ccf October 18, 2022 15:38

Removing cipher suites which do not provide adequate security for HTTP/2

77fe540

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

reta force-pushed the apache-client5 branch from 25e161e to 77fe540 Compare October 18, 2022 16:59

bbarani mentioned this pull request Oct 18, 2022

[Bug]: 9200 is accessible without credentials in latest bundle OpenSearch 3.0.0 opensearch-project/opensearch-build#2743

Closed

peternied approved these changes Oct 18, 2022

View reviewed changes

reta approved these changes Oct 18, 2022

View reviewed changes

DarshitChanpura approved these changes Oct 18, 2022

View reviewed changes

DarshitChanpura merged commit 41d68cc into opensearch-project:main Oct 18, 2022

reta mentioned this pull request Oct 18, 2022

[FEATURE] Migrate client transports to Apache HttpClient / Core 5.x #2171

Closed

peternied mentioned this pull request Oct 24, 2022

[BUG] - Documentation guide not building the project #2174

Closed

cwperks mentioned this pull request Jul 3, 2023

Bump SAML libs #2927

Merged

3 tasks

cwperks mentioned this pull request Oct 20, 2023

[BUG] [Code Style] HttpStatus & Headers should not be imported from org.apache.hc.core5.http #3581

Open

3 tasks

cwperks mentioned this pull request Jan 22, 2025

[Release 3.0] Planned Breaking Changes for 3.0 in Plugin #5031

Open

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to Apache HttpClient / Core 5.x #2166

Migrate to Apache HttpClient / Core 5.x #2166

cwperks commented Oct 13, 2022 •

edited

Loading

peternied Oct 13, 2022

peternied Oct 13, 2022

peternied Oct 13, 2022

cwperks Oct 13, 2022

DarshitChanpura Oct 13, 2022

reta Oct 18, 2022

cwperks commented Oct 18, 2022

reta commented Oct 18, 2022 •

edited

Loading

cwperks commented Oct 18, 2022 •

edited

Loading

peternied commented Oct 18, 2022

reta commented Oct 18, 2022

reta commented Oct 18, 2022

DarshitChanpura left a comment

cwperks commented Oct 18, 2022

Migrate to Apache HttpClient / Core 5.x #2166

Migrate to Apache HttpClient / Core 5.x #2166

Conversation

cwperks commented Oct 13, 2022 • edited Loading

Description

Issues Resolved

Testing

Check List

peternied Oct 13, 2022

Choose a reason for hiding this comment

peternied Oct 13, 2022

Choose a reason for hiding this comment

peternied Oct 13, 2022

Choose a reason for hiding this comment

cwperks Oct 13, 2022

Choose a reason for hiding this comment

DarshitChanpura Oct 13, 2022

Choose a reason for hiding this comment

reta Oct 18, 2022

Choose a reason for hiding this comment

cwperks commented Oct 18, 2022

reta commented Oct 18, 2022 • edited Loading

cwperks commented Oct 18, 2022 • edited Loading

peternied commented Oct 18, 2022

reta commented Oct 18, 2022

reta commented Oct 18, 2022

DarshitChanpura left a comment

Choose a reason for hiding this comment

cwperks commented Oct 18, 2022

cwperks commented Oct 13, 2022 •

edited

Loading

reta commented Oct 18, 2022 •

edited

Loading

cwperks commented Oct 18, 2022 •

edited

Loading