From 0e5632dc3a58d6d43b1cb9f92df0a7c5a489fb38 Mon Sep 17 00:00:00 2001 From: Bogdan Pintea Date: Tue, 5 May 2020 18:27:06 +0200 Subject: [PATCH] SQL: relax version lock between server and clients (#56148) (#56223) * Relax version lock between ES/SQL and clients Allow older-than-server clients to connect, if these are past or on a certain min release. (cherry picked from commit 108f907297542ce649aa7304060aaf0a504eb699) --- .../xpack/sql/jdbc/VersionParityTests.java | 12 +++++++---- .../sql/action/AbstractSqlQueryRequest.java | 5 +++-- .../xpack/sql/cli/CliSessionTests.java | 20 +++++++------------ .../xpack/sql/proto/SqlVersion.java | 5 +++++ 4 files changed, 23 insertions(+), 19 deletions(-) diff --git a/x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/VersionParityTests.java b/x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/VersionParityTests.java index 3ccdfa0d8f7..1291111cd5c 100644 --- a/x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/VersionParityTests.java +++ b/x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/VersionParityTests.java @@ -26,7 +26,8 @@ import java.sql.SQLException; public class VersionParityTests extends WebServerTestCase { public void testExceptionThrownOnIncompatibleVersions() throws IOException, SQLException { - Version version = VersionUtils.randomVersionBetween(random(), Version.V_6_0_0, VersionUtils.getPreviousVersion(Version.CURRENT)); + Version version = VersionUtils.randomVersionBetween(random(), null, VersionUtils.getPreviousVersion(Version.V_7_7_0)); + logger.info("Checking exception is thrown for version {}", version); prepareResponse(version); String url = JdbcConfiguration.URL_PREFIX + webServerAddress(); @@ -37,11 +38,14 @@ public class VersionParityTests extends WebServerTestCase { } public void testNoExceptionThrownForCompatibleVersions() throws IOException { - prepareResponse(null); - String url = JdbcConfiguration.URL_PREFIX + webServerAddress(); + Version version = Version.CURRENT; try { - new JdbcHttpClient(JdbcConfiguration.create(url, null, 0)); + do { + prepareResponse(version); + new JdbcHttpClient(JdbcConfiguration.create(url, null, 0)); + version = VersionUtils.getPreviousVersion(version); + } while (version.compareTo(Version.V_7_7_0) >= 0); } catch (SQLException sqle) { fail("JDBC driver version and Elasticsearch server version should be compatible. Error: " + sqle); } diff --git a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/AbstractSqlQueryRequest.java b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/AbstractSqlQueryRequest.java index d0260551c1c..b505576a574 100644 --- a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/AbstractSqlQueryRequest.java +++ b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/AbstractSqlQueryRequest.java @@ -27,6 +27,7 @@ import org.elasticsearch.xpack.sql.proto.Mode; import org.elasticsearch.xpack.sql.proto.Protocol; import org.elasticsearch.xpack.sql.proto.RequestInfo; import org.elasticsearch.xpack.sql.proto.SqlTypedParamValue; +import org.elasticsearch.xpack.sql.proto.SqlVersion; import java.io.IOException; import java.time.ZoneId; @@ -218,13 +219,13 @@ public abstract class AbstractSqlQueryRequest extends AbstractSqlRequest impleme ActionRequestValidationException validationException = null; // the version field is mandatory for drivers and CLI Mode mode = requestInfo().mode(); - if (mode != null && (Mode.isDedicatedClient(mode))) { + if (Mode.isDedicatedClient(mode)) { if (requestInfo().version() == null) { if (Strings.hasText(query())) { validationException = addValidationError("[version] is required for the [" + mode.toString() + "] client", validationException); } - } else if (requestInfo().version().equals(Version.CURRENT.toString()) == false) { + } else if (SqlVersion.isClientCompatible(requestInfo().version()) == false) { validationException = addValidationError("The [" + requestInfo().version() + "] version of the [" + mode.toString() + "] " + "client is not compatible with Elasticsearch version [" + Version.CURRENT + "]", validationException); diff --git a/x-pack/plugin/sql/sql-cli/src/test/java/org/elasticsearch/xpack/sql/cli/CliSessionTests.java b/x-pack/plugin/sql/sql-cli/src/test/java/org/elasticsearch/xpack/sql/cli/CliSessionTests.java index 438f67372ff..d889eb6901f 100644 --- a/x-pack/plugin/sql/sql-cli/src/test/java/org/elasticsearch/xpack/sql/cli/CliSessionTests.java +++ b/x-pack/plugin/sql/sql-cli/src/test/java/org/elasticsearch/xpack/sql/cli/CliSessionTests.java @@ -5,9 +5,11 @@ */ package org.elasticsearch.xpack.sql.cli; +import org.elasticsearch.Version; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.common.UUIDs; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.VersionUtils; import org.elasticsearch.xpack.sql.cli.command.CliSession; import org.elasticsearch.xpack.sql.client.ClientException; import org.elasticsearch.xpack.sql.client.ClientVersion; @@ -47,7 +49,8 @@ public class CliSessionTests extends ESTestCase { public void testWrongServerVersion() throws Exception { HttpClient httpClient = mock(HttpClient.class); - SqlVersion version = new SqlVersion((int)SqlVersion.V_7_7_0.major, SqlVersion.V_7_7_0.minor - 1, 0); + Version v = VersionUtils.randomVersionBetween(random(), null, VersionUtils.getPreviousVersion(Version.V_7_7_0)); + SqlVersion version = new SqlVersion(v.major, v.minor, v.revision); when(httpClient.serverInfo()).thenReturn(new MainResponse(randomAlphaOfLength(5), version.toString(), ClusterName.DEFAULT.value(), UUIDs.randomBase64UUID())); CliSession cliSession = new CliSession(httpClient); @@ -61,18 +64,9 @@ public class CliSessionTests extends ESTestCase { public void testHigherServerVersion() throws Exception { HttpClient httpClient = mock(HttpClient.class); - byte minor; - byte major; - if (randomBoolean()) { - minor = ClientVersion.CURRENT.minor; - major = (byte) (ClientVersion.CURRENT.major + 1); - } else { - minor = (byte) (ClientVersion.CURRENT.minor + 1); - major = ClientVersion.CURRENT.major; - - } - when(httpClient.serverInfo()).thenReturn(new MainResponse(randomAlphaOfLength(5), - SqlVersion.fromString(major + "." + minor + ".23").toString(), + Version v = VersionUtils.randomVersionBetween(random(), Version.V_7_7_0, null); + SqlVersion version = new SqlVersion(v.major, v.minor, v.revision); + when(httpClient.serverInfo()).thenReturn(new MainResponse(randomAlphaOfLength(5), version.toString(), ClusterName.DEFAULT.value(), UUIDs.randomBase64UUID())); CliSession cliSession = new CliSession(httpClient); cliSession.checkConnection(); diff --git a/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/SqlVersion.java b/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/SqlVersion.java index 4b311a373aa..ecbd693011e 100644 --- a/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/SqlVersion.java +++ b/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/SqlVersion.java @@ -141,4 +141,9 @@ public class SqlVersion implements Comparable{ public static boolean hasVersionCompatibility(SqlVersion version) { return version.compareTo(V_7_7_0) >= 0; } + + public static boolean isClientCompatible(SqlVersion version) { + /* only client's of version 7.7.0 and later are supported as backwards compatible */ + return V_7_7_0.compareTo(version) <= 0; + } }