Skip to content

Commit

Permalink
SQL: relax version lock between server and clients (#56148)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
bpintea authored May 5, 2020
1 parent f26ee12 commit 108f907
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
public class VersionParityTests extends WebServerTestCase {

public void testExceptionThrownOnIncompatibleVersions() throws IOException, SQLException {
Version version = VersionUtils.randomVersionBetween(random(), null, VersionUtils.getPreviousVersion());
Version version = VersionUtils.randomVersionBetween(random(), null, VersionUtils.getPreviousVersion(Version.V_7_7_0));
logger.info("Checking exception is thrown for version {}", version);
prepareResponse(version);

Expand All @@ -38,11 +38,14 @@ public void testExceptionThrownOnIncompatibleVersions() throws IOException, SQLE
}

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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
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;
Expand Down Expand Up @@ -228,13 +229,13 @@ public ActionRequestValidationException validate() {
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -47,7 +49,8 @@ public void testConnection() throws Exception {

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);
Expand All @@ -61,18 +64,9 @@ public void testWrongServerVersion() throws Exception {

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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,4 +141,9 @@ public int compareToMajorMinor(SqlVersion o) {
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;
}
}

0 comments on commit 108f907

Please sign in to comment.