Fallback to netty client if AWS Crt client is not available on the target platform / architecture

[reta]

Description

The AWS Crt client is not available on all platforms / architectures and fails with:

Unable to determine platform for AWS CRT: software.amazon.awssdk.crt.CRT$UnknownPlatformException: AWS CRT: architecture not supported: ppc64le
  2> software.amazon.awssdk.crt.CRT$UnknownPlatformException: AWS CRT: architecture not supported: ppc64le
  2>    at software.amazon.awssdk.crt.CRT.getArchIdentifier(CRT.java:154)
  2>    at software.amazon.awssdk.crt.internal.ExtractLib.extractLibrary(ExtractLib.java:43)
  2>    at software.amazon.awssdk.crt.CRT.extractAndLoadLibrary(CRT.java:310)
  2>    at software.amazon.awssdk.crt.CRT.loadLibraryFromJar(CRT.java:330)
  2>    at software.amazon.awssdk.crt.CRT.<clinit>(CRT.java:50)
  2>    at software.amazon.awssdk.crt.CrtResource.<clinit>(CrtResource.java:104)
  2>    at software.amazon.awssdk.http.crt.AwsCrtHttpClientBase.<init>(AwsCrtHttpClientBase.java:78)
  2>    at software.amazon.awssdk.http.crt.AwsCrtAsyncHttpClient.<init>(AwsCrtAsyncHttpClient.java:52)
  2>    at software.amazon.awssdk.http.crt.AwsCrtAsyncHttpClient.<init>(AwsCrtAsyncHttpClient.java:49)
  2>    at software.amazon.awssdk.http.crt.AwsCrtAsyncHttpClient$DefaultAsyncBuilder.build(AwsCrtAsyncHttpClient.java:252)
  2>    at org.opensearch.repositories.s3.S3AsyncService.buildAsyncCrtHttpClient(S3AsyncService.java:388)
  2>    at org.opensearch.repositories.s3.S3AsyncService.buildHttpClient(S3AsyncService.java:338)
  2>    at org.opensearch.repositories.s3.S3AsyncService.buildClient(S3AsyncService.java:267)
  2>    at org.opensearch.repositories.s3.S3AsyncService.client(S3AsyncService.java:171)
  2>    at org.opensearch.repositories.s3.S3BlobStore.asyncClientReference(S3BlobStore.java:193)
  2>    at org.opensearch.secure_sm.AccessController.doPrivileged(AccessController.java:76)
  2>    at org.opensearch.repositories.s3.S3BlobContainer.asyncBlobUpload(S3BlobContainer.java:306)
  2>    at org.opensearch.repositories.s3.S3BlobContainerRetriesTests.testWriteBlobByStreamsWithRetries(S3BlobContainerRetriesTests.java:426)
  2>    at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
  2>    at java.base/java.lang.reflect.Method.invoke(Method.java:565)

In this case it would make sense to fallback to netty HTTP client type since users do not know which platforms they may be running on.

Related Issues

N/A

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

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.

[github-actions]

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit 78bd56e.

Path	Line	Severity	Description
plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/utils/AwsCrtUtils.java	36	low	Misleading JavaDoc comment claims to check for HTTP/3 and netty-codec-native-quic but actually checks AWS CRT availability - likely copy-paste error rather than malicious obfuscation

The table above displays the top 10 most important findings.

Total: 1 | Critical: 0 | High: 0 | Medium: 0 | Low: 1

---

Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.

---

⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit aee28f1)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Exception Handling The static initializer catches ExceptionInInitializerError which could mask initialization problems in the CRT library itself. Consider logging the exception details to help diagnose platform-specific issues during troubleshooting. } catch (ExceptionInInitializerError | UnknownPlatformException ex) { crtAvailable = false; } Redundant Comparison The condition AwsCrtUtils.isAwsCrtAvailable() == true uses explicit boolean comparison. The == true is redundant since the method already returns a boolean. if (AwsCrtUtils.isAwsCrtAvailable() == true) {

[github-actions]

Persistent review updated to latest commit 9c31e7d

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to aee28f1
Explore these optional code suggestions:

Category	Suggestion	Impact
General	Simplify boolean comparison Remove the explicit == true comparison as it's redundant in Java. The method already returns a boolean, so the condition can be simplified for better readability and idiomatic Java code. plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3AsyncService.java [339-341] -if (AwsCrtUtils.isAwsCrtAvailable() == true) { +if (AwsCrtUtils.isAwsCrtAvailable()) { return buildAsyncCrtHttpClient(clientSettings); } Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies that == true is redundant in Java boolean comparisons. While this is a valid style improvement that enhances code readability, it's a minor change that doesn't affect functionality or correctness.	Low
	Use assumeTrue for cleaner code The assumeThat with equalTo(true) is verbose. Use org.junit.Assume.assumeTrue() instead for a more concise and idiomatic way to skip tests when a condition is not met. plugins/repository-s3/src/test/java/org/opensearch/repositories/s3/S3AsyncServiceTests.java [179] -assumeThat("AWS CRT client is not available on the target platform", AwsCrtUtils.isAwsCrtAvailable(), equalTo(true)); +assumeTrue("AWS CRT client is not available on the target platform", AwsCrtUtils.isAwsCrtAvailable()); Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies that assumeTrue() is more concise than assumeThat(..., equalTo(true)). This is a valid style improvement for test code that makes it more idiomatic, though it requires adding the assumeTrue import.	Low
	Use isEmpty instead of isBlank The condition checks if archIdentifier is not blank, but isBlank() returns true for null strings in some implementations. Consider using isEmpty() instead or ensure the null check is sufficient, as the current logic may not behave as intended. plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/utils/AwsCrtUtils.java [24] -crtAvailable = (archIdentifier != null && archIdentifier.isBlank() == false); +crtAvailable = (archIdentifier != null && !archIdentifier.isEmpty()); Suggestion importance[1-10]: 3 __ Why: The suggestion misunderstands the intent. isBlank() checks for whitespace-only strings while isEmpty() only checks for zero length. The current code with isBlank() is more robust as it rejects whitespace-only identifiers, which is likely the intended behavior for archIdentifier.	Low

---

Previous suggestions

Suggestions up to commit 9c31e7d

Category	Suggestion	Impact
General	Simplify boolean comparison Remove the explicit comparison with true as it's redundant in Java. The method already returns a boolean, so the condition can be simplified for better readability and idiomatic code. plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3AsyncService.java [339-341] -if (AwsCrtUtils.isAwsCrtAvailable() == true) { +if (AwsCrtUtils.isAwsCrtAvailable()) { return buildAsyncCrtHttpClient(clientSettings); } Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies that == true is redundant in Java boolean comparisons. While this is a valid style improvement that enhances code readability, it's a minor change that doesn't affect functionality or correctness.	Low
	Simplify boolean negation The condition archIdentifier.isBlank() == false can be simplified to !archIdentifier.isBlank() for better readability. Additionally, consider using isEmpty() instead of isBlank() if you only need to check for empty strings, as isBlank() also checks for whitespace-only strings. plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/utils/AwsCrtUtils.java [23-24] final String archIdentifier = CRT.getArchIdentifier(); -crtAvailable = (archIdentifier != null && archIdentifier.isBlank() == false); +crtAvailable = (archIdentifier != null && !archIdentifier.isBlank()); Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies that == false can be simplified to !. This is a valid style improvement for readability. However, the recommendation to use isEmpty() instead of isBlank() is questionable without knowing the specific requirements, as isBlank() provides more thorough validation.	Low

[github-actions]

✅ Gradle check result for 9c31e7d: SUCCESS

[codecov]

Codecov Report

❌ Patch coverage is 50.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.21%. Comparing base (c856e62) to head (aee28f1).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
...org/opensearch/repositories/s3/S3AsyncService.java	25.00%	2 Missing and 1 partial ⚠️
.../opensearch/repositories/s3/utils/AwsCrtUtils.java	62.50%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #20698      +/-   ##
============================================
- Coverage     73.25%   73.21%   -0.04%     
- Complexity    71966    71982      +16     
============================================
  Files          5781     5782       +1     
  Lines        329414   329425      +11     
  Branches      47531    47533       +2     
============================================
- Hits         241307   241192     -115     
- Misses        68741    68926     +185     
+ Partials      19366    19307      -59     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

Persistent review updated to latest commit aee28f1

[github-actions]

❌ Gradle check result for aee28f1: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

❌ Gradle check result for aee28f1: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

✅ Gradle check result for aee28f1: SUCCESS