From adff229db54f520d81d1ea5c73d8f240ac36f3f8 Mon Sep 17 00:00:00 2001 From: Andriy Redko Date: Tue, 10 Dec 2024 08:20:09 -0500 Subject: [PATCH] Use the Upgrade flow by default Signed-off-by: Andriy Redko --- client/rest/licenses/httpclient5-5.4.1.jar.sha1 | 2 +- client/rest/licenses/httpcore5-5.3.1.jar.sha1 | 2 +- client/rest/licenses/httpcore5-h2-5.3.1.jar.sha1 | 2 +- client/rest/licenses/httpcore5-reactive-5.3.1.jar.sha1 | 2 +- .../java/org/opensearch/client/RestClientBuilder.java | 9 ++------- .../client/RestClientSingleHostIntegTests.java | 10 +++++++++- client/sniffer/licenses/httpclient5-5.4.1.jar.sha1 | 2 +- client/sniffer/licenses/httpcore5-5.3.1.jar.sha1 | 2 +- .../org/opensearch/rest/ReactorNetty4BadRequestIT.java | 9 +++++++++ .../rest/ReactorNetty4HeadBodyIsEmptyIT.java | 9 +++++++++ .../org/opensearch/rest/ReactorNetty4StreamingIT.java | 10 ++++++++++ .../rest/ReactorNetty4StreamingStressIT.java | 10 ++++++++++ .../org/opensearch/bootstrap/BootstrapForTesting.java | 8 ++------ .../opensearch/test/rest/OpenSearchRestTestCase.java | 3 +++ 14 files changed, 60 insertions(+), 20 deletions(-) diff --git a/client/rest/licenses/httpclient5-5.4.1.jar.sha1 b/client/rest/licenses/httpclient5-5.4.1.jar.sha1 index 3a4fec6e8afc7..40156e9a42620 100644 --- a/client/rest/licenses/httpclient5-5.4.1.jar.sha1 +++ b/client/rest/licenses/httpclient5-5.4.1.jar.sha1 @@ -1 +1 @@ -ce913081e592ee8eeee35c4e577d7dce13cba7a4 +ce913081e592ee8eeee35c4e577d7dce13cba7a4 \ No newline at end of file diff --git a/client/rest/licenses/httpcore5-5.3.1.jar.sha1 b/client/rest/licenses/httpcore5-5.3.1.jar.sha1 index 90f194e770368..3b7536d6e8998 100644 --- a/client/rest/licenses/httpcore5-5.3.1.jar.sha1 +++ b/client/rest/licenses/httpcore5-5.3.1.jar.sha1 @@ -1 +1 @@ -eaf64237945d7d0f301d48420e8bdb7f565a7b0e +eaf64237945d7d0f301d48420e8bdb7f565a7b0e \ No newline at end of file diff --git a/client/rest/licenses/httpcore5-h2-5.3.1.jar.sha1 b/client/rest/licenses/httpcore5-h2-5.3.1.jar.sha1 index bc122991af736..6c3e4ffa5c6da 100644 --- a/client/rest/licenses/httpcore5-h2-5.3.1.jar.sha1 +++ b/client/rest/licenses/httpcore5-h2-5.3.1.jar.sha1 @@ -1 +1 @@ -760c34db3ba41b0ffa07e956bc308d3a12356915 +760c34db3ba41b0ffa07e956bc308d3a12356915 \ No newline at end of file diff --git a/client/rest/licenses/httpcore5-reactive-5.3.1.jar.sha1 b/client/rest/licenses/httpcore5-reactive-5.3.1.jar.sha1 index cb71833aad877..ca9828394f40e 100644 --- a/client/rest/licenses/httpcore5-reactive-5.3.1.jar.sha1 +++ b/client/rest/licenses/httpcore5-reactive-5.3.1.jar.sha1 @@ -1 +1 @@ -c4c0c3c7bbcb0db54aa7ddd39e34a835428c99c0 +c4c0c3c7bbcb0db54aa7ddd39e34a835428c99c0 \ No newline at end of file diff --git a/client/rest/src/main/java/org/opensearch/client/RestClientBuilder.java b/client/rest/src/main/java/org/opensearch/client/RestClientBuilder.java index 78a1d3e3fcf06..3e38f9ae95dec 100644 --- a/client/rest/src/main/java/org/opensearch/client/RestClientBuilder.java +++ b/client/rest/src/main/java/org/opensearch/client/RestClientBuilder.java @@ -308,15 +308,10 @@ private CloseableHttpAsyncClient createHttpClient() { // default timeouts are all infinite RequestConfig.Builder requestConfigBuilder = RequestConfig.custom() .setConnectTimeout(Timeout.ofMilliseconds(DEFAULT_CONNECT_TIMEOUT_MILLIS)) - .setResponseTimeout(Timeout.ofMilliseconds(DEFAULT_RESPONSE_TIMEOUT_MILLIS)) - .setProtocolUpgradeEnabled(false); + .setResponseTimeout(Timeout.ofMilliseconds(DEFAULT_RESPONSE_TIMEOUT_MILLIS)); if (requestConfigCallback != null) { requestConfigBuilder = requestConfigCallback.customizeRequestConfig(requestConfigBuilder); } - RequestConfig requestConfig = requestConfigBuilder.build(); - if (requestConfig.isProtocolUpgradeEnabled()) { - throw new IllegalArgumentException("protocol upgrade is not supported"); - } try { final TlsStrategy tlsStrategy = ClientTlsStrategyBuilder.create() @@ -337,7 +332,7 @@ public TlsDetails create(final SSLEngine sslEngine) { .build(); HttpAsyncClientBuilder httpClientBuilder = HttpAsyncClientBuilder.create() - .setDefaultRequestConfig(requestConfig) + .setDefaultRequestConfig(requestConfigBuilder.build()) .setConnectionManager(connectionManager) .setTargetAuthenticationStrategy(DefaultAuthenticationStrategy.INSTANCE) .disableAutomaticRetries(); diff --git a/client/rest/src/test/java/org/opensearch/client/RestClientSingleHostIntegTests.java b/client/rest/src/test/java/org/opensearch/client/RestClientSingleHostIntegTests.java index de04dd843b2db..84f6e7c8beb2e 100644 --- a/client/rest/src/test/java/org/opensearch/client/RestClientSingleHostIntegTests.java +++ b/client/rest/src/test/java/org/opensearch/client/RestClientSingleHostIntegTests.java @@ -382,6 +382,10 @@ public void testHeaders() throws Exception { if (method.equals("HEAD") == false) { standardHeaders.add("Content-length"); } + if (method.equals("HEAD") == true || method.equals("GET") == true || method.equals("OPTIONS") == true) { + standardHeaders.add("Upgrade"); + } + final Header[] requestHeaders = RestClientTestUtil.randomHeaders(getRandom(), "Header"); final int statusCode = randomStatusCode(getRandom()); Request request = new Request(method, "/" + statusCode); @@ -400,11 +404,15 @@ public void testHeaders() throws Exception { assertEquals(method, esResponse.getRequestLine().getMethod()); assertEquals(statusCode, esResponse.getStatusLine().getStatusCode()); assertEquals(pathPrefix + "/" + statusCode, esResponse.getRequestLine().getUri()); + assertHeaders(defaultHeaders, requestHeaders, esResponse.getHeaders(), standardHeaders); + final Set removedHeaders = new HashSet<>(); for (final Header responseHeader : esResponse.getHeaders()) { String name = responseHeader.getName(); - if (name.startsWith("Header") == false) { + // Some headers could be returned multiple times in response, like Connection fe. + if (name.startsWith("Header") == false && removedHeaders.contains(name) == false) { assertTrue("unknown header was returned " + name, standardHeaders.remove(name)); + removedHeaders.add(name); } } assertTrue("some expected standard headers weren't returned: " + standardHeaders, standardHeaders.isEmpty()); diff --git a/client/sniffer/licenses/httpclient5-5.4.1.jar.sha1 b/client/sniffer/licenses/httpclient5-5.4.1.jar.sha1 index 3a4fec6e8afc7..40156e9a42620 100644 --- a/client/sniffer/licenses/httpclient5-5.4.1.jar.sha1 +++ b/client/sniffer/licenses/httpclient5-5.4.1.jar.sha1 @@ -1 +1 @@ -ce913081e592ee8eeee35c4e577d7dce13cba7a4 +ce913081e592ee8eeee35c4e577d7dce13cba7a4 \ No newline at end of file diff --git a/client/sniffer/licenses/httpcore5-5.3.1.jar.sha1 b/client/sniffer/licenses/httpcore5-5.3.1.jar.sha1 index 90f194e770368..3b7536d6e8998 100644 --- a/client/sniffer/licenses/httpcore5-5.3.1.jar.sha1 +++ b/client/sniffer/licenses/httpcore5-5.3.1.jar.sha1 @@ -1 +1 @@ -eaf64237945d7d0f301d48420e8bdb7f565a7b0e +eaf64237945d7d0f301d48420e8bdb7f565a7b0e \ No newline at end of file diff --git a/plugins/transport-reactor-netty4/src/javaRestTest/java/org/opensearch/rest/ReactorNetty4BadRequestIT.java b/plugins/transport-reactor-netty4/src/javaRestTest/java/org/opensearch/rest/ReactorNetty4BadRequestIT.java index b1d21fe4eee09..29ad714716475 100644 --- a/plugins/transport-reactor-netty4/src/javaRestTest/java/org/opensearch/rest/ReactorNetty4BadRequestIT.java +++ b/plugins/transport-reactor-netty4/src/javaRestTest/java/org/opensearch/rest/ReactorNetty4BadRequestIT.java @@ -124,4 +124,13 @@ public void testUnsupportedContentType() throws IOException { final String error = objectPath.evaluate("error"); assertThat(error, equalTo("Content-Type header [] is not supported")); } + + @Override + protected final Settings restClientSettings() { + return Settings.builder() + .put(super.restClientSettings()) + // See please https://github.com/reactor/reactor-netty/issues/3538 + .put(OpenSearchRestTestCase.CLIENT_PROTOCOL_UPGRADE_ENABLED, false) + .build(); + } } diff --git a/plugins/transport-reactor-netty4/src/javaRestTest/java/org/opensearch/rest/ReactorNetty4HeadBodyIsEmptyIT.java b/plugins/transport-reactor-netty4/src/javaRestTest/java/org/opensearch/rest/ReactorNetty4HeadBodyIsEmptyIT.java index 663eb9ef6e946..c74c63712e13f 100644 --- a/plugins/transport-reactor-netty4/src/javaRestTest/java/org/opensearch/rest/ReactorNetty4HeadBodyIsEmptyIT.java +++ b/plugins/transport-reactor-netty4/src/javaRestTest/java/org/opensearch/rest/ReactorNetty4HeadBodyIsEmptyIT.java @@ -34,6 +34,7 @@ import org.opensearch.client.Request; import org.opensearch.client.Response; +import org.opensearch.common.settings.Settings; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.test.rest.OpenSearchRestTestCase; import org.hamcrest.Matcher; @@ -201,4 +202,12 @@ private void headTestCase( assertNull("HEAD requests shouldn't have a response body but " + url + " did", response.getEntity()); } + @Override + protected final Settings restClientSettings() { + return Settings.builder() + .put(super.restClientSettings()) + // See please https://github.com/reactor/reactor-netty/issues/3538 + .put(OpenSearchRestTestCase.CLIENT_PROTOCOL_UPGRADE_ENABLED, false) + .build(); + } } diff --git a/plugins/transport-reactor-netty4/src/javaRestTest/java/org/opensearch/rest/ReactorNetty4StreamingIT.java b/plugins/transport-reactor-netty4/src/javaRestTest/java/org/opensearch/rest/ReactorNetty4StreamingIT.java index e022dfedb788d..4c56a7909b4a5 100644 --- a/plugins/transport-reactor-netty4/src/javaRestTest/java/org/opensearch/rest/ReactorNetty4StreamingIT.java +++ b/plugins/transport-reactor-netty4/src/javaRestTest/java/org/opensearch/rest/ReactorNetty4StreamingIT.java @@ -13,6 +13,7 @@ import org.opensearch.client.ResponseException; import org.opensearch.client.StreamingRequest; import org.opensearch.client.StreamingResponse; +import org.opensearch.common.settings.Settings; import org.opensearch.test.rest.OpenSearchRestTestCase; import org.opensearch.test.rest.yaml.ObjectPath; import org.junit.After; @@ -350,4 +351,13 @@ public void testStreamingLargeDocumentThatExceedsChunkSize() throws IOException assertThat(streamingResponse.getStatusLine().getStatusCode(), equalTo(200)); assertThat(streamingResponse.getWarnings(), empty()); } + + @Override + protected final Settings restClientSettings() { + return Settings.builder() + .put(super.restClientSettings()) + // See please https://github.com/reactor/reactor-netty/issues/3538 + .put(OpenSearchRestTestCase.CLIENT_PROTOCOL_UPGRADE_ENABLED, false) + .build(); + } } diff --git a/plugins/transport-reactor-netty4/src/javaRestTest/java/org/opensearch/rest/ReactorNetty4StreamingStressIT.java b/plugins/transport-reactor-netty4/src/javaRestTest/java/org/opensearch/rest/ReactorNetty4StreamingStressIT.java index 9da456f618ffc..8842d4e5356a3 100644 --- a/plugins/transport-reactor-netty4/src/javaRestTest/java/org/opensearch/rest/ReactorNetty4StreamingStressIT.java +++ b/plugins/transport-reactor-netty4/src/javaRestTest/java/org/opensearch/rest/ReactorNetty4StreamingStressIT.java @@ -13,6 +13,7 @@ import org.opensearch.client.Response; import org.opensearch.client.StreamingRequest; import org.opensearch.client.StreamingResponse; +import org.opensearch.common.settings.Settings; import org.opensearch.test.rest.OpenSearchRestTestCase; import org.junit.After; @@ -79,4 +80,13 @@ public void testCloseClientStreamingRequest() throws Exception { .expectErrorMatches(t -> t instanceof InterruptedIOException || t instanceof ConnectionClosedException) .verify(); } + + @Override + protected final Settings restClientSettings() { + return Settings.builder() + .put(super.restClientSettings()) + // See please https://github.com/reactor/reactor-netty/issues/3538 + .put(OpenSearchRestTestCase.CLIENT_PROTOCOL_UPGRADE_ENABLED, false) + .build(); + } } diff --git a/test/framework/src/main/java/org/opensearch/bootstrap/BootstrapForTesting.java b/test/framework/src/main/java/org/opensearch/bootstrap/BootstrapForTesting.java index 79cead80fff3e..933385dedcf49 100644 --- a/test/framework/src/main/java/org/opensearch/bootstrap/BootstrapForTesting.java +++ b/test/framework/src/main/java/org/opensearch/bootstrap/BootstrapForTesting.java @@ -301,15 +301,11 @@ static Set parseClassPathWithSymlinks() throws Exception { private static Set getTrustedHosts() { // try { - List hosts = Collections.list(NetworkInterface.getNetworkInterfaces()) + return Collections.list(NetworkInterface.getNetworkInterfaces()) .stream() .flatMap(iface -> Collections.list(iface.getInetAddresses()).stream()) .map(address -> NetworkAddress.format(address)) - .collect(Collectors.toList()); - // 0:0:0:0:0:0:0:1 is simplified to ::1, in it test, the incoming address can be 0:0:0:0:0:0:0:1, - // so we should add it to trusted hosts. - hosts.add("0:0:0:0:0:0:0:1"); - return Collections.unmodifiableSet(new HashSet<>(hosts)); + .collect(Collectors.toSet()); } catch (final SocketException e) { return Collections.emptySet(); } diff --git a/test/framework/src/main/java/org/opensearch/test/rest/OpenSearchRestTestCase.java b/test/framework/src/main/java/org/opensearch/test/rest/OpenSearchRestTestCase.java index 8c612d258f183..b5ae0b4c3778a 100644 --- a/test/framework/src/main/java/org/opensearch/test/rest/OpenSearchRestTestCase.java +++ b/test/framework/src/main/java/org/opensearch/test/rest/OpenSearchRestTestCase.java @@ -135,6 +135,7 @@ public abstract class OpenSearchRestTestCase extends OpenSearchTestCase { public static final String TRUSTSTORE_PASSWORD = "truststore.password"; public static final String CLIENT_SOCKET_TIMEOUT = "client.socket.timeout"; public static final String CLIENT_PATH_PREFIX = "client.path.prefix"; + public static final String CLIENT_PROTOCOL_UPGRADE_ENABLED = "client.protocol.upgrade.enabled"; // This set will contain the warnings already asserted since we are eliminating logging duplicate warnings. // This ensures that no matter in what order the tests run, the warning is asserted once. @@ -881,8 +882,10 @@ public TlsDetails create(final SSLEngine sslEngine) { socketTimeoutString == null ? "60s" : socketTimeoutString, CLIENT_SOCKET_TIMEOUT ); + final boolean protocolUpgradeEnabled = settings.getAsBoolean(CLIENT_PROTOCOL_UPGRADE_ENABLED, true); builder.setRequestConfigCallback( conf -> conf.setResponseTimeout(Timeout.ofMilliseconds(Math.toIntExact(socketTimeout.getMillis()))) + .setProtocolUpgradeEnabled(protocolUpgradeEnabled) ); if (settings.hasValue(CLIENT_PATH_PREFIX)) { builder.setPathPrefix(settings.get(CLIENT_PATH_PREFIX));