[Backport 2.x] [Bug] Fix InboundDecoder version compat check (#2574)

Change InboundDecoder ensureVersionCompatibility check for onOrAfter V_2_0_0 instead of explicit version check. This way bug fix and minor versions will correctly handshake in a mixed 1.x Cluster. Also fixes version filtering and warning handling for bwc testing. Signed-off-by: Nicholas Walter Knize <nknize@apache.org> (cherry picked from commit 511ac88)
opensearch-project · Mar 24, 2022 · 388b02b · 388b02b
1 parent 11dd394
commit 388b02b
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 5 deletions.
diff --git a/qa/mixed-cluster/src/test/java/org/opensearch/backwards/ExceptionIT.java b/qa/mixed-cluster/src/test/java/org/opensearch/backwards/ExceptionIT.java
@@ -47,7 +47,13 @@ public void testOpensearchException() throws Exception {
     private void logClusterNodes() throws IOException {
         ObjectPath objectPath = ObjectPath.createFromResponse(client().performRequest(new Request("GET", "_nodes")));
         Map<String, ?> nodes = objectPath.evaluate("nodes");
-        String master = EntityUtils.toString(client().performRequest(new Request("GET", "_cat/master?h=id")).getEntity()).trim();
+        Request request = new Request("GET", "_cat/master?h=id");
+        String inclusiveWarning = "[GET /_cat/master] is deprecated! Use [GET /_cat/cluster_manager] instead.";
+        request.setOptions(expectVersionSpecificWarnings(v -> {
+            v.current(inclusiveWarning);
+            v.compatible(inclusiveWarning);
+        }));
+        String master = EntityUtils.toString(client().performRequest(request).getEntity()).trim();
         logger.info("cluster discovered: master id='{}'", master);
         for (String id : nodes.keySet()) {
             logger.info("{}: id='{}', name='{}', version={}",

diff --git a/server/src/main/java/org/opensearch/transport/InboundDecoder.java b/server/src/main/java/org/opensearch/transport/InboundDecoder.java
@@ -217,7 +217,7 @@ static IllegalStateException ensureVersionCompatibility(Version remoteVersion, V
         // handshake. This looks odd but it's required to establish the connection correctly we check for real compatibility
         // once the connection is established
         final Version compatibilityVersion = isHandshake ? currentVersion.minimumCompatibilityVersion() : currentVersion;
-        if ((currentVersion.equals(Version.V_2_0_0) && remoteVersion.equals(Version.fromId(6079999))) == false
+        if ((currentVersion.onOrAfter(Version.V_2_0_0) && remoteVersion.equals(Version.fromId(6079999))) == false
             && remoteVersion.isCompatible(compatibilityVersion) == false) {
             final Version minCompatibilityVersion = isHandshake ? compatibilityVersion : compatibilityVersion.minimumCompatibilityVersion();
             String msg = "Received " + (isHandshake ? "handshake " : "") + "message from unsupported version: [";

diff --git a/test/framework/src/main/java/org/opensearch/test/VersionUtils.java b/test/framework/src/main/java/org/opensearch/test/VersionUtils.java
@@ -89,7 +89,7 @@ static Tuple<List<Version>, List<Version>> resolveReleasedVersions(Version curre
             stableVersions = previousMajor;
             // remove current
             moveLastToUnreleased(currentMajor, unreleasedVersions);
-        } else if (current.major != 1 && current.major != 2) {
+        } else if (current.major != 1) {
             // on a stable or release branch, ie N.x
             stableVersions = currentMajor;
             // remove the next maintenance bugfix

diff --git a/test/framework/src/test/java/org/opensearch/test/VersionUtilsTests.java b/test/framework/src/test/java/org/opensearch/test/VersionUtilsTests.java
@@ -284,15 +284,17 @@ public void testResolveReleasedVersionsAtNewMinorBranchIn2x() {
                 Arrays.asList(
                     TestNewMinorBranchIn6x.V_1_6_0,
                     TestNewMinorBranchIn6x.V_1_6_1,
-                    TestNewMinorBranchIn6x.V_1_6_2,
                     TestNewMinorBranchIn6x.V_2_0_0,
                     TestNewMinorBranchIn6x.V_2_0_1,
                     TestNewMinorBranchIn6x.V_2_1_0,
                     TestNewMinorBranchIn6x.V_2_1_1
                 )
             )
         );
-        assertThat(unreleased, equalTo(Arrays.asList(TestNewMinorBranchIn6x.V_2_1_2, TestNewMinorBranchIn6x.V_2_2_0)));
+        assertThat(
+            unreleased,
+            equalTo(Arrays.asList(TestNewMinorBranchIn6x.V_1_6_2, TestNewMinorBranchIn6x.V_2_1_2, TestNewMinorBranchIn6x.V_2_2_0))
+        );
     }
 
     /**