TransportVersionUtils#randomVersionBetween does not work with version extensions (106119) (#116198)

Introduces a new extension method to VersionExtension enabling extensions to provide additional versions and creates method TransportVersion.getAllVersions returning all transport versions defined by Elasticsearch and the extension. This ensures that TransportVersion.current() always returns the correct current (latest) transport version even if it is defined by an extension.

Fixes #106119

server/src/main/java/org/elasticsearch/TransportVersion.java

@@ -16,7 +16,14 @@ import org.elasticsearch.internal.VersionExtension;
 import org.elasticsearch.plugins.ExtensionLoader;
 
 import java.io.IOException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
 import java.util.ServiceLoader;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 /**
  * Represents the version of the wire protocol used to communicate between a pair of ES nodes.
@@ -56,8 +63,14 @@ public record TransportVersion(int id) implements VersionId<TransportVersion> {
         return fromId(in.readVInt());
     }
 
+    /**
+     * Finds a {@code TransportVersion} by its id.
+     * If a transport version with the specified ID does not exist,
+     * this method creates and returns a new instance of {@code TransportVersion} with the specified ID.
+     * The new instance is not registered in {@code TransportVersion.getAllVersions}.
+     */
     public static TransportVersion fromId(int id) {
-        TransportVersion known = TransportVersions.VERSION_IDS.get(id);
+        TransportVersion known = VersionsHolder.ALL_VERSIONS_MAP.get(id);
         if (known != null) {
             return known;
         }
@@ -95,7 +108,14 @@ public record TransportVersion(int id) implements VersionId<TransportVersion> {
      * This should be the transport version with the highest id.
      */
     public static TransportVersion current() {
-        return CurrentHolder.CURRENT;
+        return VersionsHolder.CURRENT;
+    }
+
+    /**
+     * Sorted list of all defined transport versions
+     */
+    public static List<TransportVersion> getAllVersions() {
+        return VersionsHolder.ALL_VERSIONS;
     }
 
     public static TransportVersion fromString(String str) {
@@ -139,16 +159,25 @@ public record TransportVersion(int id) implements VersionId<TransportVersion> {
         return Integer.toString(id);
     }
 
-    private static class CurrentHolder {
+    private static class VersionsHolder {
-        private static final TransportVersion CURRENT = findCurrent();
+        private static final List<TransportVersion> ALL_VERSIONS;
+        private static final Map<Integer, TransportVersion> ALL_VERSIONS_MAP;
+        private static final TransportVersion CURRENT;
 
-        // finds the pluggable current version
+        static {
-        private static TransportVersion findCurrent() {
+            Collection<TransportVersion> extendedVersions = ExtensionLoader.loadSingleton(ServiceLoader.load(VersionExtension.class))
-            var version = ExtensionLoader.loadSingleton(ServiceLoader.load(VersionExtension.class))
+                .map(VersionExtension::getTransportVersions)
-                .map(e -> e.getCurrentTransportVersion(TransportVersions.LATEST_DEFINED))
+                .orElse(Collections.emptyList());
-                .orElse(TransportVersions.LATEST_DEFINED);
+
-            assert version.onOrAfter(TransportVersions.LATEST_DEFINED);
+            if (extendedVersions.isEmpty()) {
-            return version;
+                ALL_VERSIONS = TransportVersions.DEFINED_VERSIONS;
+            } else {
+                ALL_VERSIONS = Stream.concat(TransportVersions.DEFINED_VERSIONS.stream(), extendedVersions.stream()).sorted().toList();
+            }
+
+            ALL_VERSIONS_MAP = ALL_VERSIONS.stream().collect(Collectors.toUnmodifiableMap(TransportVersion::id, Function.identity()));
+
+            CURRENT = ALL_VERSIONS.getLast();
         }
     }
 }

server/src/main/java/org/elasticsearch/TransportVersions.java

@@ -13,13 +13,12 @@ import org.elasticsearch.core.Assertions;
 import org.elasticsearch.core.UpdateForV9;
 
 import java.lang.reflect.Field;
-import java.util.Collection;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
-import java.util.NavigableMap;
 import java.util.Set;
-import java.util.TreeMap;
 import java.util.TreeSet;
 import java.util.function.IntFunction;
 
@@ -209,21 +208,24 @@ public class TransportVersions {
      */
     public static final TransportVersion MINIMUM_CCS_VERSION = V_8_15_0;
 
-    static final NavigableMap<Integer, TransportVersion> VERSION_IDS = getAllVersionIds(TransportVersions.class);
+    /**
+     * Sorted list of all versions defined in this class
+     */
+    static final List<TransportVersion> DEFINED_VERSIONS = collectAllVersionIdsDefinedInClass(TransportVersions.class);
 
-    // the highest transport version constant defined in this file, used as a fallback for TransportVersion.current()
+    // the highest transport version constant defined
     static final TransportVersion LATEST_DEFINED;
     static {
-        LATEST_DEFINED = VERSION_IDS.lastEntry().getValue();
+        LATEST_DEFINED = DEFINED_VERSIONS.getLast();
 
         // see comment on IDS field
         // now we're registered all the transport versions, we can clear the map
         IDS = null;
     }
 
-    public static NavigableMap<Integer, TransportVersion> getAllVersionIds(Class<?> cls) {
+    public static List<TransportVersion> collectAllVersionIdsDefinedInClass(Class<?> cls) {
         Map<Integer, String> versionIdFields = new HashMap<>();
-        NavigableMap<Integer, TransportVersion> builder = new TreeMap<>();
+        List<TransportVersion> definedTransportVersions = new ArrayList<>();
 
         Set<String> ignore = Set.of("ZERO", "CURRENT", "MINIMUM_COMPATIBLE", "MINIMUM_CCS_VERSION");
 
@@ -240,7 +242,7 @@ public class TransportVersions {
                 } catch (IllegalAccessException e) {
                     throw new AssertionError(e);
                 }
-                builder.put(version.id(), version);
+                definedTransportVersions.add(version);
 
                 if (Assertions.ENABLED) {
                     // check the version number is unique
@@ -257,11 +259,9 @@ public class TransportVersions {
             }
         }
 
-        return Collections.unmodifiableNavigableMap(builder);
+        Collections.sort(definedTransportVersions);
-    }
 
-    static Collection<TransportVersion> getAllVersions() {
+        return List.copyOf(definedTransportVersions);
-        return VERSION_IDS.values();
     }
 
     static final IntFunction<String> VERSION_LOOKUP = ReleaseVersions.generateVersionsLookup(TransportVersions.class, LATEST_DEFINED.id());

server/src/main/java/org/elasticsearch/internal/VersionExtension.java

@@ -12,17 +12,16 @@ package org.elasticsearch.internal;
 import org.elasticsearch.TransportVersion;
 import org.elasticsearch.index.IndexVersion;
 
+import java.util.List;
+
 /**
  * Allows plugging in current version elements.
  */
 public interface VersionExtension {
     /**
-     * Returns the {@link TransportVersion} that Elasticsearch should use.
+     * Returns list of {@link TransportVersion} defined by extension
-     * <p>
-     * This must be at least as high as the given fallback.
-     * @param fallback The latest transport version from server
      */
-    TransportVersion getCurrentTransportVersion(TransportVersion fallback);
+    List<TransportVersion> getTransportVersions();
 
     /**
      * Returns the {@link IndexVersion} that Elasticsearch should use.

server/src/test/java/org/elasticsearch/TransportVersionTests.java

@@ -14,7 +14,7 @@ import org.elasticsearch.test.TransportVersionUtils;
 
 import java.lang.reflect.Modifier;
 import java.util.Collections;
-import java.util.Map;
+import java.util.List;
 import java.util.Set;
 import java.util.TreeSet;
 import java.util.regex.Matcher;
@@ -69,21 +69,20 @@ public class TransportVersionTests extends ESTestCase {
 
     public void testStaticTransportVersionChecks() {
         assertThat(
-            TransportVersions.getAllVersionIds(CorrectFakeVersion.class),
+            TransportVersions.collectAllVersionIdsDefinedInClass(CorrectFakeVersion.class),
             equalTo(
-                Map.of(
+                List.of(
-                    199,
-                    CorrectFakeVersion.V_0_00_01,
-                    2,
                     CorrectFakeVersion.V_0_000_002,
-                    3,
                     CorrectFakeVersion.V_0_000_003,
-                    4,
+                    CorrectFakeVersion.V_0_000_004,
-                    CorrectFakeVersion.V_0_000_004
+                    CorrectFakeVersion.V_0_00_01
                 )
             )
         );
-        AssertionError e = expectThrows(AssertionError.class, () -> TransportVersions.getAllVersionIds(DuplicatedIdFakeVersion.class));
+        AssertionError e = expectThrows(
+            AssertionError.class,
+            () -> TransportVersions.collectAllVersionIdsDefinedInClass(DuplicatedIdFakeVersion.class)
+        );
         assertThat(e.getMessage(), containsString("have the same version number"));
     }
 
@@ -186,7 +185,7 @@ public class TransportVersionTests extends ESTestCase {
     }
 
     public void testCURRENTIsLatest() {
-        assertThat(Collections.max(TransportVersions.getAllVersions()), is(TransportVersion.current()));
+        assertThat(Collections.max(TransportVersion.getAllVersions()), is(TransportVersion.current()));
     }
 
     public void testToReleaseVersion() {
@@ -210,7 +209,7 @@ public class TransportVersionTests extends ESTestCase {
     public void testDenseTransportVersions() {
         Set<Integer> missingVersions = new TreeSet<>();
         TransportVersion previous = null;
-        for (var tv : TransportVersions.getAllVersions()) {
+        for (var tv : TransportVersion.getAllVersions()) {
             if (tv.before(TransportVersions.V_8_16_0)) {
                 continue;
             }

test/framework/src/main/java/org/elasticsearch/KnownTransportVersions.java

@@ -1,22 +0,0 @@
-/*
- * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
- * or more contributor license agreements. Licensed under the "Elastic License
- * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
- * Public License v 1"; you may not use this file except in compliance with, at
- * your election, the "Elastic License 2.0", the "GNU Affero General Public
- * License v3.0 only", or the "Server Side Public License, v 1".
- */
-
-package org.elasticsearch;
-
-import java.util.List;
-
-/**
- * Provides access to all known transport versions.
- */
-public class KnownTransportVersions {
-    /**
-     * A sorted list of all known transport versions
-     */
-    public static final List<TransportVersion> ALL_VERSIONS = List.copyOf(TransportVersions.getAllVersions());
-}

test/framework/src/main/java/org/elasticsearch/test/BWCVersions.java

@@ -15,14 +15,13 @@ import org.elasticsearch.TransportVersions;
 import java.util.Collections;
 import java.util.List;
 
-import static org.elasticsearch.KnownTransportVersions.ALL_VERSIONS;
-
 public final class BWCVersions {
     private BWCVersions() {}
 
     public static List<TransportVersion> getAllBWCVersions() {
-        int minCompatVersion = Collections.binarySearch(ALL_VERSIONS, TransportVersions.MINIMUM_COMPATIBLE);
+        List<TransportVersion> allVersions = TransportVersion.getAllVersions();
-        return ALL_VERSIONS.subList(minCompatVersion, ALL_VERSIONS.size());
+        int minCompatVersion = Collections.binarySearch(allVersions, TransportVersions.MINIMUM_COMPATIBLE);
+        return allVersions.subList(minCompatVersion, allVersions.size());
     }
 
     public static final List<TransportVersion> DEFAULT_BWC_VERSIONS = getAllBWCVersions();

test/framework/src/main/java/org/elasticsearch/test/TransportVersionUtils.java

@@ -19,32 +19,30 @@ import java.util.Random;
 import java.util.Set;
 import java.util.stream.Collectors;
 
-import static org.elasticsearch.KnownTransportVersions.ALL_VERSIONS;
-
 public class TransportVersionUtils {
     /** Returns all released versions */
     public static List<TransportVersion> allReleasedVersions() {
-        return ALL_VERSIONS;
+        return TransportVersion.getAllVersions();
     }
 
     /** Returns the oldest known {@link TransportVersion} */
     public static TransportVersion getFirstVersion() {
-        return ALL_VERSIONS.get(0);
+        return allReleasedVersions().getFirst();
     }
 
     /** Returns a random {@link TransportVersion} from all available versions. */
     public static TransportVersion randomVersion() {
-        return ESTestCase.randomFrom(ALL_VERSIONS);
+        return ESTestCase.randomFrom(allReleasedVersions());
     }
 
     /** Returns a random {@link TransportVersion} from all available versions without the ignore set */
     public static TransportVersion randomVersion(Set<TransportVersion> ignore) {
-        return ESTestCase.randomFrom(ALL_VERSIONS.stream().filter(v -> ignore.contains(v) == false).collect(Collectors.toList()));
+        return ESTestCase.randomFrom(allReleasedVersions().stream().filter(v -> ignore.contains(v) == false).collect(Collectors.toList()));
     }
 
     /** Returns a random {@link TransportVersion} from all available versions. */
     public static TransportVersion randomVersion(Random random) {
-        return ALL_VERSIONS.get(random.nextInt(ALL_VERSIONS.size()));
+        return allReleasedVersions().get(random.nextInt(allReleasedVersions().size()));
     }
 
     /** Returns a random {@link TransportVersion} between <code>minVersion</code> and <code>maxVersion</code> (inclusive). */
@@ -58,12 +56,13 @@ public class TransportVersionUtils {
         }
 
         int minVersionIndex = 0;
+        List<TransportVersion> allReleasedVersions = allReleasedVersions();
         if (minVersion != null) {
-            minVersionIndex = Collections.binarySearch(ALL_VERSIONS, minVersion);
+            minVersionIndex = Collections.binarySearch(allReleasedVersions, minVersion);
         }
-        int maxVersionIndex = ALL_VERSIONS.size() - 1;
+        int maxVersionIndex = allReleasedVersions.size() - 1;
         if (maxVersion != null) {
-            maxVersionIndex = Collections.binarySearch(ALL_VERSIONS, maxVersion);
+            maxVersionIndex = Collections.binarySearch(allReleasedVersions, maxVersion);
         }
         if (minVersionIndex < 0) {
             throw new IllegalArgumentException("minVersion [" + minVersion + "] does not exist.");
@@ -72,7 +71,7 @@ public class TransportVersionUtils {
         } else {
             // minVersionIndex is inclusive so need to add 1 to this index
             int range = maxVersionIndex + 1 - minVersionIndex;
-            return ALL_VERSIONS.get(minVersionIndex + random.nextInt(range));
+            return allReleasedVersions.get(minVersionIndex + random.nextInt(range));
         }
     }
 
@@ -83,7 +82,7 @@ public class TransportVersionUtils {
     }
 
     public static TransportVersion getPreviousVersion(TransportVersion version) {
-        int place = Collections.binarySearch(ALL_VERSIONS, version);
+        int place = Collections.binarySearch(allReleasedVersions(), version);
         if (place < 0) {
             // version does not exist - need the item before the index this version should be inserted
             place = -(place + 1);
@@ -92,7 +91,7 @@ public class TransportVersionUtils {
         if (place < 1) {
             throw new IllegalArgumentException("couldn't find any released versions before [" + version + "]");
         }
-        return ALL_VERSIONS.get(place - 1);
+        return allReleasedVersions().get(place - 1);
     }
 
     public static TransportVersion getNextVersion(TransportVersion version) {
@@ -100,7 +99,8 @@ public class TransportVersionUtils {
     }
 
     public static TransportVersion getNextVersion(TransportVersion version, boolean createIfNecessary) {
-        int place = Collections.binarySearch(ALL_VERSIONS, version);
+        List<TransportVersion> allReleasedVersions = allReleasedVersions();
+        int place = Collections.binarySearch(allReleasedVersions, version);
         if (place < 0) {
             // version does not exist - need the item at the index this version should be inserted
             place = -(place + 1);
@@ -109,7 +109,7 @@ public class TransportVersionUtils {
             place++;
         }
 
-        if (place < 0 || place >= ALL_VERSIONS.size()) {
+        if (place < 0 || place >= allReleasedVersions.size()) {
             if (createIfNecessary) {
                 // create a new transport version one greater than specified
                 return new TransportVersion(version.id() + 1);
@@ -117,7 +117,7 @@ public class TransportVersionUtils {
                 throw new IllegalArgumentException("couldn't find any released versions after [" + version + "]");
             }
         }
-        return ALL_VERSIONS.get(place);
+        return allReleasedVersions.get(place);
     }
 
     /** Returns a random {@code TransportVersion} that is compatible with {@link TransportVersion#current()} */

x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/AbstractBWCSerializationTestCase.java

@@ -16,14 +16,14 @@ import java.io.IOException;
 import java.util.Collections;
 import java.util.List;
 
-import static org.elasticsearch.KnownTransportVersions.ALL_VERSIONS;
 import static org.hamcrest.Matchers.equalTo;
 
 public abstract class AbstractBWCSerializationTestCase<T extends Writeable & ToXContent> extends AbstractXContentSerializingTestCase<T> {
 
     private static List<TransportVersion> getAllBWCVersions() {
-        int minCompatVersion = Collections.binarySearch(ALL_VERSIONS, TransportVersions.MINIMUM_COMPATIBLE);
+        List<TransportVersion> allVersions = TransportVersion.getAllVersions();
-        return ALL_VERSIONS.subList(minCompatVersion, ALL_VERSIONS.size());
+        int minCompatVersion = Collections.binarySearch(allVersions, TransportVersions.MINIMUM_COMPATIBLE);
+        return allVersions.subList(minCompatVersion, allVersions.size());
     }
 
     private static final List<TransportVersion> DEFAULT_BWC_VERSIONS = getAllBWCVersions();

x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/AbstractBWCWireSerializingTestCase.java

@@ -15,14 +15,14 @@ import java.io.IOException;
 import java.util.Collections;
 import java.util.List;
 
-import static org.elasticsearch.KnownTransportVersions.ALL_VERSIONS;
 import static org.hamcrest.Matchers.equalTo;
 
 public abstract class AbstractBWCWireSerializingTestCase<T extends Writeable> extends AbstractWireSerializingTestCase<T> {
 
     private static List<TransportVersion> getAllBWCVersions() {
-        int minCompatVersion = Collections.binarySearch(ALL_VERSIONS, TransportVersions.MINIMUM_COMPATIBLE);
+        List<TransportVersion> allVersions = TransportVersion.getAllVersions();
-        return ALL_VERSIONS.subList(minCompatVersion, ALL_VERSIONS.size());
+        int minCompatVersion = Collections.binarySearch(allVersions, TransportVersions.MINIMUM_COMPATIBLE);
+        return allVersions.subList(minCompatVersion, allVersions.size());
     }
 
     private static final List<TransportVersion> DEFAULT_BWC_VERSIONS = getAllBWCVersions();