From 508815e30ac868483ee82a152cbf186e8147cf1c Mon Sep 17 00:00:00 2001 From: Thomas Farr Date: Thu, 5 Dec 2024 14:20:08 +1300 Subject: [PATCH] Revert "Revert throwing on AwsSdk2 + ApacheHttpClient (#1333)" (#1338) (#1339) This reverts commit 4279d36165cf5fcf2d8be3a689df0c5dbfa47831. Signed-off-by: Thomas Farr (cherry picked from commit f7831962471bfd475aa4e98bf61bda4c543e0c79) --- CHANGELOG.md | 2 -- guides/auth.md | 4 +++- java-client/build.gradle.kts | 22 ++++++++--------- .../transport/aws/AwsSdk2Transport.java | 21 +++++++++++++++- .../transport/aws/AwsSdk2TransportTests.java | 24 ++++++++++++++++++- 5 files changed, 57 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d98cb9cad0..046a471cee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,14 +7,12 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### Dependencies - Bumps `org.junit:junit-bom` from 5.10.2 to 5.11.3 -- Upgraded aws-sdk-java dependencies to at least `2.19.22` ([#1333](https://github.com/opensearch-project/opensearch-java/pull/1333)) ### Changed ### Deprecated ### Removed -- Removed throwing of exception when using `AwsSdk2Transport` with AWS SDK's `ApacheHttpClient` to make an DELETE/GET request with a body ([#1333](https://github.com/opensearch-project/opensearch-java/pull/1333)) ### Fixed - Fixed `IcuCollationDecomposition`'s variants to align with those supported by OpenSearch ([#]()) diff --git a/guides/auth.md b/guides/auth.md index 7f76049045..060c3d6708 100644 --- a/guides/auth.md +++ b/guides/auth.md @@ -8,7 +8,9 @@ Requests to [OpenSearch Service and OpenSearch Serverless](https://docs.aws.amazon.com/opensearch-service/index.html) must be signed using the AWS signing protocol. Use `AwsSdk2Transport` to send signed requests. > ⚠️ **Warning** ⚠️ -> If you are using `software.amazon.awssdk.http.apache.ApacheHttpClient` please ensure you use at least version `2.29.22` as earlier versions do not support request bodies on GET or DELETE requests, which leads to incorrect handling of requests such as `OpenSearchClient.clearScroll()` and `OpenSearchClient.deletePit()`. +> Using `software.amazon.awssdk.http.apache.ApacheHttpClient` is discouraged as it does not support request bodies on GET or DELETE requests. +> This leads to incorrect handling of requests such as `OpenSearchClient.clearScroll()` and `OpenSearchClient.deletePit()`. +> As such `AwsSdk2Transport` will throw a `TransportException` if an unsupported request is encountered while using `ApacheHttpClient`. ```java SdkHttpClient httpClient = AwsCrtHttpClient.builder().build(); diff --git a/java-client/build.gradle.kts b/java-client/build.gradle.kts index 637ea78765..f2f2265cb7 100644 --- a/java-client/build.gradle.kts +++ b/java-client/build.gradle.kts @@ -217,17 +217,17 @@ dependencies { implementation("org.apache.httpcomponents.core5", "httpcore5-h2", "5.3.1") // For AwsSdk2Transport - "awsSdk2SupportCompileOnly"("software.amazon.awssdk", "sdk-core", "[2.29.22,3.0)") - "awsSdk2SupportCompileOnly"("software.amazon.awssdk", "auth", "[2.29.22,3.0)") - "awsSdk2SupportCompileOnly"("software.amazon.awssdk", "http-auth-aws", "[2.29.22,3.0)") - testImplementation("software.amazon.awssdk", "sdk-core", "[2.29.22,3.0)") - testImplementation("software.amazon.awssdk", "auth", "[2.29.22,3.0)") - testImplementation("software.amazon.awssdk", "http-auth-aws", "[2.29.22,3.0)") - testImplementation("software.amazon.awssdk", "aws-crt-client", "[2.29.22,3.0)") - testImplementation("software.amazon.awssdk", "apache-client", "[2.29.22,3.0)") - testImplementation("software.amazon.awssdk", "netty-nio-client", "[2.29.22,3.0)") - testImplementation("software.amazon.awssdk", "url-connection-client", "[2.29.22,3.0)") - testImplementation("software.amazon.awssdk", "sts", "[2.29.22,3.0)") + "awsSdk2SupportCompileOnly"("software.amazon.awssdk", "sdk-core", "[2.21,3.0)") + "awsSdk2SupportCompileOnly"("software.amazon.awssdk", "auth", "[2.21,3.0)") + "awsSdk2SupportCompileOnly"("software.amazon.awssdk", "http-auth-aws", "[2.21,3.0)") + testImplementation("software.amazon.awssdk", "sdk-core", "[2.21,3.0)") + testImplementation("software.amazon.awssdk", "auth", "[2.21,3.0)") + testImplementation("software.amazon.awssdk", "http-auth-aws", "[2.21,3.0)") + testImplementation("software.amazon.awssdk", "aws-crt-client", "[2.21,3.0)") + testImplementation("software.amazon.awssdk", "apache-client", "[2.21,3.0)") + testImplementation("software.amazon.awssdk", "netty-nio-client", "[2.21,3.0)") + testImplementation("software.amazon.awssdk", "url-connection-client", "[2.21,3.0)") + testImplementation("software.amazon.awssdk", "sts", "[2.21,3.0)") testImplementation("org.apache.logging.log4j", "log4j-api","[2.17.1,3.0)") testImplementation("org.apache.logging.log4j", "log4j-core","[2.17.1,3.0)") diff --git a/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java b/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java index 6237ec9eff..6ac694332f 100644 --- a/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java +++ b/java-client/src/main/java/org/opensearch/client/transport/aws/AwsSdk2Transport.java @@ -85,6 +85,7 @@ public class AwsSdk2Transport implements OpenSearchTransport { private static final byte[] NO_BYTES = new byte[0]; private final SdkAutoCloseable httpClient; + private final boolean isApacheHttpClient; private final String host; private final String signingServiceName; private final Region signingRegion; @@ -194,6 +195,8 @@ private AwsSdk2Transport( ) { Objects.requireNonNull(host, "Target OpenSearch service host must not be null"); this.httpClient = httpClient; + this.isApacheHttpClient = httpClient instanceof SdkHttpClient + && httpClient.getClass().getName().equals("software.amazon.awssdk.http.apache.ApacheHttpClient"); this.host = host; this.signingServiceName = signingServiceName; this.signingRegion = signingRegion; @@ -296,9 +299,25 @@ private SignedRequest prepareRequest( Endpoint endpoint, @CheckForNull TransportOptions options, @CheckForNull OpenSearchRequestBodyBuffer body - ) throws UnsupportedEncodingException { + ) throws UnsupportedEncodingException, TransportException { SdkHttpMethod method = SdkHttpMethod.fromValue(endpoint.method(request)); + // AWS Apache Http Client only supports request bodies on PATCH, POST & PUT requests. + // See: + // https://github.com/aws/aws-sdk-java-v2/blob/master/http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/internal/impl/ApacheHttpRequestFactory.java#L118-L137 + if (isApacheHttpClient + && method != SdkHttpMethod.PATCH + && method != SdkHttpMethod.POST + && method != SdkHttpMethod.PUT + && body != null + && body.getContentLength() > 0) { + throw new TransportException( + "AWS SDK's ApacheHttpClient does not support request bodies for HTTP method `" + + method + + "`. Please use a different SdkHttpClient implementation." + ); + } + SdkHttpFullRequest.Builder req = SdkHttpFullRequest.builder().method(method); StringBuilder url = new StringBuilder(); diff --git a/java-client/src/test/java/org/opensearch/client/transport/aws/AwsSdk2TransportTests.java b/java-client/src/test/java/org/opensearch/client/transport/aws/AwsSdk2TransportTests.java index 227b5a3515..c5a6aca793 100644 --- a/java-client/src/test/java/org/opensearch/client/transport/aws/AwsSdk2TransportTests.java +++ b/java-client/src/test/java/org/opensearch/client/transport/aws/AwsSdk2TransportTests.java @@ -13,6 +13,7 @@ import static org.apache.hc.core5.http.HttpHeaders.CONTENT_TYPE; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -56,6 +57,7 @@ import org.junit.runners.Parameterized; import org.opensearch.client.opensearch.OpenSearchClient; import org.opensearch.client.opensearch.generic.Requests; +import org.opensearch.client.transport.TransportException; import org.opensearch.client.transport.util.FunnellingHttpsProxy; import org.opensearch.client.transport.util.SelfSignedCertificateAuthority; import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; @@ -408,7 +410,27 @@ private void assertSigV4Request( String contentSha256, String expectedSignature ) throws Exception { - request.invoke(getTestClient()); + OpenSearchClient client = getTestClient(); + + if (sdkHttpClientType != SdkHttpClientType.APACHE + || contentLength == 0 + || "PATCH".equals(method) + || "POST".equals(method) + || "PUT".equals(method)) { + request.invoke(client); + } else { + // AWS Apache Http Client only supports content on PATCH, POST & PUT requests. + // See: + // https://github.com/aws/aws-sdk-java-v2/blob/master/http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/internal/impl/ApacheHttpRequestFactory.java#L118-L137 + assertThrows( + "AWS SDK's ApacheHttpClient does not support request bodies for HTTP method `" + + method + + "`. Please use a different SdkHttpClient implementation.", + TransportException.class, + () -> request.invoke(client) + ); + return; + } assertEquals(1, receivedRequests.size()); ReceivedRequest req = receivedRequests.poll();