Stabilize resource test client load

[sharp-pixel]

Description

This test fix makes the Reactor Netty client used by resource-focused tests honor its configuration and bound its resource usage.

• Category: Test fix
• The client ignored the requested HTTP protocol, did not cap in-flight requests to the supplied parallelism, and left retained response buffers for callers to manage without exceptional-path cleanup.
• Previously, HTTP/3-focused tests could run over a different protocol and large request batches could be subscribed all at once. The new behavior honors and validates the requested protocol, limits concurrent subscriptions, applies a bounded wait, and releases reference-counted responses on success and exceptional termination.

Issues Resolved

Fixes #6465

This is not a backport.

No new permissions are introduced.

Testing

• ./gradlew :integrationTest --tests org.opensearch.test.framework.cluster.ReactorHttpClientTests -x :opensearch-sample-resource-plugin:integrationTest
• ./gradlew :integrationTest --tests org.opensearch.security.ResourceFocusedTests -x :opensearch-sample-resource-plugin:integrationTest
• ./gradlew spotlessJavaCheck
• ./gradlew :precommit -x :opensearch-sample-resource-plugin:precommit

The full aggregate precommit task was also attempted. It reaches pre-existing forbidden-API violations for URL.openStream() in the unchanged sample resource plugin; the root precommit suite above passes.

Check List

• New functionality includes testing
• New functionality has been documented (test behavior is covered by code and regression tests)
• New Roles/Permissions have a corresponding security dashboards plugin PR (not applicable; no roles or permissions changed)
• API changes companion pull request created (not applicable; no public API changed)
• 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.

[github-actions]

PR Reviewer Guide 🔍

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 Response leak on block timeout collectResponses uses block(Duration.ofMinutes(2)). If the timeout elapses, block throws and any FullHttpResponses already produced by the upstream Monos (which called b.retain()) will not go through doOnDiscard for items already collected into the list — those retained buffers leak. Consider using Flux.usingWhen/doOnNext release semantics or a cancellation path that iterates already-collected responses and releases them on timeout/error. static List<FullHttpResponse> collectResponses(Flux<Mono<FullHttpResponse>> responses, boolean ordered, int parallelism) { Flux<FullHttpResponse> responseFlux = ordered ? responses.concatMap(response -> response) : responses.flatMap(response -> response, parallelism); return responseFlux.collectList().doOnDiscard(FullHttpResponse.class, ReferenceCountUtil::release).block(Duration.ofMinutes(2)); } Validation misses unsupported protocols validateProtocol only rejects a hard-coded mismatch set. HTTP protocols like H2C with secure=false are accepted, but HTTP11 with secure=false combined with e.g. HTTP3 (secure only) is fine — however other values such as null-safe passes for future enum additions won't be caught. Minor: the error message says "not compatible with secure=" but the real cause for e.g. HTTP3 without TLS is that HTTP/3 requires QUIC/TLS; consider clearer messaging. private static void validateProtocol(HttpProtocol protocol, boolean secure) { if (protocol == null) { throw new IllegalArgumentException("protocol must not be null"); } boolean supported = secure ? protocol == HttpProtocol.HTTP11 || protocol == HttpProtocol.H2 || protocol == HttpProtocol.HTTP3 : protocol == HttpProtocol.HTTP11 || protocol == HttpProtocol.H2C; if (supported == false) { throw new IllegalArgumentException("Protocol " + protocol + " is not compatible with secure=" + secure); } }

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Guard HTTP/3 by availability and settings The validation allows HttpProtocol.HTTP3 when secure=true regardless of whether HTTP/3 is actually available or enabled in settings. This can cause runtime failures on systems where HTTP/3 is not supported. Consider gating HTTP3 on Http3Utils.isHttp3Available() and the SETTING_HTTP_HTTP3_ENABLED setting, as the previous randomProtocol did. src/integrationTest/java/org/opensearch/test/framework/cluster/ReactorHttpClient.java [152-154] +boolean http3Enabled = Http3Utils.isHttp3Available() && SETTING_HTTP_HTTP3_ENABLED.get(settings).booleanValue(); boolean supported = secure - ? protocol == HttpProtocol.HTTP11 || protocol == HttpProtocol.H2 || protocol == HttpProtocol.HTTP3 + ? protocol == HttpProtocol.HTTP11 || protocol == HttpProtocol.H2 || (protocol == HttpProtocol.HTTP3 && http3Enabled) : protocol == HttpProtocol.HTTP11 || protocol == HttpProtocol.H2C; Suggestion importance[1-10]: 6 __ Why: Valid concern - the new validation logic does not check Http3Utils.isHttp3Available() or SETTING_HTTP_HTTP3_ENABLED as the previous randomProtocol did, which could allow HTTP/3 to be selected on systems where it's not available. However, this may be intentional to let callers explicitly request HTTP/3.	Low
	Select address matching secure flag The secure parameter is passed to the client but getHttpAddress() is used unconditionally. If the caller requests secure=true, the address should correspond to the HTTPS endpoint. Otherwise callers can construct a client that will attempt TLS handshakes against a plaintext port (or vice versa), causing confusing failures. src/integrationTest/java/org/opensearch/test/framework/cluster/OpenSearchClientProvider.java [236-238] default ReactorHttpClient getGenericClient(HttpProtocol protocol, boolean secure, Settings settings) { - return new ReactorHttpClient(protocol, true, secure, settings, getHttpAddress()); + return new ReactorHttpClient(protocol, true, secure, settings, secure ? getHttpAddress() : getHttpAddress()); } Suggestion importance[1-10]: 2 __ Why: The improved_code is identical to existing_code (both use getHttpAddress() on both sides of the ternary), making the suggestion effectively a no-op that does not address the concern raised.	Low

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.75%. Comparing base (c33b831) to head (d56a406).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6467   +/-   ##
=======================================
  Coverage   75.75%   75.75%           
=======================================
  Files         457      457           
  Lines       30508    30556   +48     
  Branches     4615     4630   +15     
=======================================
+ Hits        23111    23149   +38     
- Misses       5276     5280    +4     
- Partials     2121     2127    +6     

see 8 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.