Revert "Revert throwing on AwsSdk2 + ApacheHttpClient (#1333)" (#1338)

This reverts commit 4279d36.

Signed-off-by: Thomas Farr <[email protected]>

CHANGELOG.md

@@ -43,14 +43,12 @@ This section is for maintaining a changelog for all breaking changes for the cli
 ### Added
 
 ### Dependencies
-- 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 an issue where `FieldSort` was not implementing `SortOptionsVariant` ([#1323](https://github.com/opensearch-project/opensearch-java/pull/1323))

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();

java-client/build.gradle.kts

@@ -220,17 +220,17 @@ dependencies {
     testImplementation("com.fasterxml.jackson.datatype", "jackson-datatype-jakarta-jsonp", jacksonVersion)
 
     // 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)")

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 <RequestT> SignedRequest prepareRequest(
         Endpoint<RequestT, ?, ?> 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();

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();