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

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

@@ -16,7 +16,11 @@
 import org.elasticsearch.plugins.ExtensionLoader;
 
 import java.io.IOException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.NavigableMap;
 import java.util.ServiceLoader;
+import java.util.TreeMap;
 
 /**
  * Represents the version of the wire protocol used to communicate between a pair of ES nodes.
@@ -57,7 +61,7 @@ public static TransportVersion readVersion(StreamInput in) throws IOException {
     }
 
     public static TransportVersion fromId(int id) {
-        TransportVersion known = TransportVersions.VERSION_IDS.get(id);
+        TransportVersion known = VersionsHolder.ALL_VERSION_IDS.get(id);
         if (known != null) {
             return known;
         }
@@ -95,7 +99,14 @@ public static boolean isCompatible(TransportVersion version) {
      * This should be the transport version with the highest id.
      */
     public static TransportVersion current() {
-        return CurrentHolder.CURRENT;
+        return VersionsHolder.CURRENT;
+    }
+
+    /**
+     * Collection of all defined transport versions
+     */
+    public static Collection<TransportVersion> getAllVersions() {
+        return VersionsHolder.ALL_VERSION_IDS.values();
     }
 
     public static TransportVersion fromString(String str) {
@@ -139,16 +150,30 @@ public String toString() {
         return Integer.toString(id);
     }
 
-    private static class CurrentHolder {
+    private static class VersionsHolder {
+        private static final NavigableMap<Integer, TransportVersion> ALL_VERSION_IDS = getAllVersionIds();
         private static final TransportVersion CURRENT = findCurrent();
 
+        private static NavigableMap<Integer, TransportVersion> getAllVersionIds() {
+            Collection<TransportVersion> extendedVersions = ExtensionLoader.loadSingleton(ServiceLoader.load(VersionExtension.class))
+                .map(VersionExtension::getTransportVersions)
+                .orElse(Collections.emptyList());
+
+            if (extendedVersions.isEmpty()) {
+                return TransportVersions.VERSION_IDS;
+            }
+
+            NavigableMap<Integer, TransportVersion> result = new TreeMap<>(TransportVersions.VERSION_IDS);
+            for (TransportVersion extendedVersion : extendedVersions) {
+                result.put(extendedVersion.id(), extendedVersion);
+            }
+
+            return Collections.unmodifiableNavigableMap(result);
+        }
+
         // finds the pluggable current version
         private static TransportVersion findCurrent() {
-            var version = ExtensionLoader.loadSingleton(ServiceLoader.load(VersionExtension.class))
-                .map(e -> e.getCurrentTransportVersion(TransportVersions.LATEST_DEFINED))
-                .orElse(TransportVersions.LATEST_DEFINED);
-            assert version.onOrAfter(TransportVersions.LATEST_DEFINED);
-            return version;
+            return ALL_VERSION_IDS.lastEntry().getValue();
         }
     }
 }

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

@@ -13,7 +13,6 @@
 import org.elasticsearch.core.UpdateForV9;
 
 import java.lang.reflect.Field;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
@@ -210,9 +209,9 @@ static TransportVersion def(int id) {
      */
     public static final TransportVersion MINIMUM_CCS_VERSION = V_8_15_0;
 
-    static final NavigableMap<Integer, TransportVersion> VERSION_IDS = getAllVersionIds(TransportVersions.class);
+    static final NavigableMap<Integer, TransportVersion> VERSION_IDS = 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();
@@ -222,9 +221,9 @@ static TransportVersion def(int id) {
         IDS = null;
     }
 
-    public static NavigableMap<Integer, TransportVersion> getAllVersionIds(Class<?> cls) {
+    public static NavigableMap<Integer, TransportVersion> collectAllVersionIdsDefinedInClass(Class<?> cls) {
         Map<Integer, String> versionIdFields = new HashMap<>();
-        NavigableMap<Integer, TransportVersion> builder = new TreeMap<>();
+        NavigableMap<Integer, TransportVersion> definedTransportVersions = new TreeMap<>();
 
         Set<String> ignore = Set.of("ZERO", "CURRENT", "MINIMUM_COMPATIBLE", "MINIMUM_CCS_VERSION");
 
@@ -241,7 +240,7 @@ public static NavigableMap<Integer, TransportVersion> getAllVersionIds(Class<?>
                 } catch (IllegalAccessException e) {
                     throw new AssertionError(e);
                 }
-                builder.put(version.id(), version);
+                definedTransportVersions.put(version.id(), version);
 
                 if (Assertions.ENABLED) {
                     // check the version number is unique
@@ -258,11 +257,7 @@ public static NavigableMap<Integer, TransportVersion> getAllVersionIds(Class<?>
             }
         }
 
-        return Collections.unmodifiableNavigableMap(builder);
-    }
-
-    static Collection<TransportVersion> getAllVersions() {
-        return VERSION_IDS.values();
+        return Collections.unmodifiableNavigableMap(definedTransportVersions);
     }
 
     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 @@
 import org.elasticsearch.TransportVersion;
 import org.elasticsearch.index.IndexVersion;
 
+import java.util.Collection;
+
 /**
  * Allows plugging in current version elements.
  */
 public interface VersionExtension {
     /**
-     * Returns the {@link TransportVersion} that Elasticsearch should use.
-     * <p>
-     * This must be at least as high as the given fallback.
-     * @param fallback The latest transport version from server
+     * Returns collection of {@link TransportVersion} defined by extension
      */
-    TransportVersion getCurrentTransportVersion(TransportVersion fallback);
+    Collection<TransportVersion> getTransportVersions();
 
     /**
      * Returns the {@link IndexVersion} that Elasticsearch should use.

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

@@ -69,7 +69,7 @@ public static class DuplicatedIdFakeVersion {
 
     public void testStaticTransportVersionChecks() {
         assertThat(
-            TransportVersions.getAllVersionIds(CorrectFakeVersion.class),
+            TransportVersions.collectAllVersionIdsDefinedInClass(CorrectFakeVersion.class),
             equalTo(
                 Map.of(
                     199,
@@ -83,7 +83,10 @@ public void testStaticTransportVersionChecks() {
                 )
             )
         );
-        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 +189,7 @@ public void testVersionConstantPresent() {
     }
 
     public void testCURRENTIsLatest() {
-        assertThat(Collections.max(TransportVersions.getAllVersions()), is(TransportVersion.current()));
+        assertThat(Collections.max(TransportVersion.getAllVersions()), is(TransportVersion.current()));
     }
 
     public void testToReleaseVersion() {
@@ -210,7 +213,7 @@ public void testToString() {
     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

@@ -18,5 +18,5 @@ public class KnownTransportVersions {
     /**
      * A sorted list of all known transport versions
      */
-    public static final List<TransportVersion> ALL_VERSIONS = List.copyOf(TransportVersions.getAllVersions());
+    public static final List<TransportVersion> ALL_VERSIONS = List.copyOf(TransportVersion.getAllVersions());
 }