Browse Source

Add more transport version files validation (#132373) (#132773)

This commit adds additional validation cases to the new transport
version resource files. It also adds gradle tests for the validation
cases.

Co-authored-by: Jack Conradson <osjdconrad@gmail.com>
Ryan Ernst 2 months ago
parent
commit
de9839b019
15 changed files with 693 additions and 50 deletions
  1. 301 0
      build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionManagementPluginFuncTest.groovy
  2. 4 3
      build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/CollectTransportVersionReferencesTask.java
  3. 2 2
      build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionManifestTask.java
  4. 7 4
      build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GlobalTransportVersionManagementPlugin.java
  5. 4 1
      build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionManagementPlugin.java
  6. 93 23
      build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionUtils.java
  7. 275 10
      build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionDefinitionsTask.java
  8. 0 2
      server/src/main/java/org/elasticsearch/TransportVersions.java
  9. 1 0
      server/src/main/resources/transport/defined/initial_elasticsearch_8_18_5.csv
  10. 1 0
      server/src/main/resources/transport/defined/initial_elasticsearch_9_0_5.csv
  11. 1 1
      server/src/main/resources/transport/latest/8.18.csv
  12. 1 1
      server/src/main/resources/transport/latest/8.19.csv
  13. 1 1
      server/src/main/resources/transport/latest/9.0.csv
  14. 1 1
      server/src/main/resources/transport/latest/9.1.csv
  15. 1 1
      server/src/main/resources/transport/latest/9.2.csv

+ 301 - 0
build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionManagementPluginFuncTest.groovy

@@ -0,0 +1,301 @@
+/*
+ * 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.gradle.internal.transport
+
+
+import org.elasticsearch.gradle.fixtures.AbstractGradleFuncTest
+import org.gradle.testkit.runner.BuildResult
+import org.gradle.testkit.runner.TaskOutcome
+
+class TransportVersionManagementPluginFuncTest extends AbstractGradleFuncTest {
+
+    /**
+     *
+     * @param project
+     * @param path
+     * @param content
+     * @return
+     */
+    def javaResource(String project, String path, String content) {
+        file("${project}/src/main/resources/${path}").withWriter { writer ->
+            writer << content
+        }
+    }
+
+    def javaSource(String project, String packageName, String className, String imports, String content) {
+        String packageSlashes = packageName.replace('.', '/')
+        file("${project}/src/main/java/${packageSlashes}/${className}.java").withWriter { writer ->
+            writer << """
+                package ${packageName};
+                ${imports}
+                public class ${className} {
+                    ${content}
+                }
+            """
+        }
+    }
+
+    def definedTransportVersion(String name, String ids) {
+        javaResource("myserver", "transport/defined/" + name + ".csv", ids)
+    }
+
+    def definedAndUsedTransportVersion(String name, String ids) {
+        return definedAndUsedTransportVersion(name, ids, "Test${name.capitalize()}")
+    }
+
+    def definedAndUsedTransportVersion(String name, String ids, String classname) {
+        javaSource("myserver", "org.elasticsearch", classname, "", """
+            static final TransportVersion usage = TransportVersion.fromName("${name}");
+        """)
+        definedTransportVersion(name, ids)
+    }
+
+    def latestTransportVersion(String branch, String name, String id) {
+        javaResource("myserver", "transport/latest/" + branch + ".csv","${name},${id}")
+    }
+
+    def validateReferencesFails(String project) {
+        return gradleRunner(":${project}:validateTransportVersionReferences").buildAndFail()
+    }
+
+    def validateDefinitionsFails() {
+        return gradleRunner(":myserver:validateTransportVersionDefinitions").buildAndFail()
+    }
+
+    def assertReferencesFailure(BuildResult result, String project, String expectedOutput) {
+        result.task(":${project}:validateTransportVersionReferences").outcome == TaskOutcome.FAILED
+        assertOutputContains(result.output, expectedOutput)
+    }
+
+    def assertDefinitionsFailure(BuildResult result, String expectedOutput) {
+        result.task(":myserver:validateTransportVersionDefinitions").outcome == TaskOutcome.FAILED
+        assertOutputContains(result.output, expectedOutput)
+    }
+
+    def setup() {
+        configurationCacheCompatible = false
+        internalBuild()
+        settingsFile << """
+            include ':myserver'
+            include ':myplugin'
+        """
+        file("gradle.properties") << """
+            org.elasticsearch.transport.definitionsProject=:myserver
+        """
+
+        file("myserver/build.gradle") << """
+            apply plugin: 'java-library'
+            apply plugin: 'elasticsearch.transport-version-management'
+            apply plugin: 'elasticsearch.global-transport-version-management'
+        """
+        definedTransportVersion("existing_91", "8012000")
+        definedTransportVersion("existing_92", "8123000,8012001")
+        latestTransportVersion("9.2", "existing_92", "8123000")
+        latestTransportVersion("9.1", "existing_92", "8012001")
+        // a mock version of TransportVersion, just here so we can compile Dummy.java et al
+        javaSource("myserver", "org.elasticsearch", "TransportVersion", "", """
+            public static TransportVersion fromName(String name) {
+                return null;
+            }
+        """)
+        javaSource("myserver", "org.elasticsearch", "Dummy", "", """
+            static final TransportVersion existing91 = TransportVersion.fromName("existing_91");
+            static final TransportVersion existing92 = TransportVersion.fromName("existing_92");
+        """)
+
+        file("myplugin/build.gradle") << """
+            apply plugin: 'java-library'
+            apply plugin: 'elasticsearch.transport-version-management'
+
+            dependencies {
+                implementation project(":myserver")
+            }
+        """
+
+        setupLocalGitRepo()
+        execute("git checkout -b main")
+        execute("git checkout -b test")
+    }
+
+    def "test setup works"() {
+        when:
+        def result = gradleRunner("validateTransportVersionDefinitions", "validateTransportVersionReferences").build()
+        then:
+        result.task(":myserver:validateTransportVersionDefinitions").outcome == TaskOutcome.SUCCESS
+        result.task(":myserver:validateTransportVersionReferences").outcome == TaskOutcome.SUCCESS
+        result.task(":myplugin:validateTransportVersionReferences").outcome == TaskOutcome.SUCCESS
+    }
+
+    def "definitions must be referenced"() {
+        given:
+        javaSource("myplugin", "org.elasticsearch.plugin", "MyPlugin",
+            "import org.elasticsearch.TransportVersion;", """
+            static final TransportVersion dne = TransportVersion.fromName("dne");
+        """)
+        when:
+        def result = validateReferencesFails("myplugin")
+        then:
+        assertReferencesFailure(result, "myplugin", "TransportVersion.fromName(\"dne\") was used at " +
+                "org.elasticsearch.plugin.MyPlugin line 6, but lacks a transport version definition.")
+    }
+
+    def "references must be defined"() {
+        given:
+        definedTransportVersion("not_used", "1000000")
+        when:
+        def result = validateDefinitionsFails()
+        then:
+        assertDefinitionsFailure(result, "Transport version definition file " +
+            "[myserver/src/main/resources/transport/defined/not_used.csv] is not referenced")
+    }
+
+    def "names must be lowercase alphanum or underscore"() {
+        given:
+        definedAndUsedTransportVersion("${name}", "8100000", "TestNames")
+        when:
+        def result = validateDefinitionsFails()
+        then:
+        assertDefinitionsFailure(result, "Transport version definition file " +
+            "[myserver/src/main/resources/transport/defined/${name}.csv] does not have a valid name, " +
+            "must be lowercase alphanumeric and underscore")
+
+        where:
+        name << ["CapitalTV", "spaces tv", "trailing_spaces_tv ", "hyphen-tv", "period.tv"]
+    }
+
+    def "definitions contain at least one id"() {
+        given:
+        definedAndUsedTransportVersion("empty", "")
+        when:
+        def result = validateDefinitionsFails()
+        then:
+        assertDefinitionsFailure(result, "Transport version definition file " +
+            "[myserver/src/main/resources/transport/defined/empty.csv] does not contain any ids")
+    }
+
+    def "definitions have ids in descending order"() {
+        given:
+        definedAndUsedTransportVersion("out_of_order", "8100000,8200000")
+        when:
+        def result = validateDefinitionsFails()
+        then:
+        assertDefinitionsFailure(result, "Transport version definition file " +
+            "[myserver/src/main/resources/transport/defined/out_of_order.csv] does not have ordered ids")
+    }
+
+    def "definition ids are unique"() {
+        given:
+        definedAndUsedTransportVersion("duplicate", "8123000")
+        when:
+        def result = validateDefinitionsFails()
+        then:
+        assertDefinitionsFailure(result, "Transport version definition file " +
+            "[myserver/src/main/resources/transport/defined/existing_92.csv] contains id 8123000 already defined in " +
+            "[myserver/src/main/resources/transport/defined/duplicate.csv]")
+    }
+
+    def "definitions have bwc ids with non-zero patch part"() {
+        given:
+        definedAndUsedTransportVersion("patched", "8200000,8100000")
+        when:
+        def result = validateDefinitionsFails()
+        then:
+        assertDefinitionsFailure(result, "Transport version definition file " +
+            "[myserver/src/main/resources/transport/defined/patched.csv] contains bwc id [8100000] with a patch part of 0")
+    }
+
+    def "definitions have primary ids which cannot change"() {
+        given:
+        definedTransportVersion("existing_92", "8500000")
+        when:
+        def result = validateDefinitionsFails()
+        then:
+        assertDefinitionsFailure(result, "Transport version definition file " +
+            "[myserver/src/main/resources/transport/defined/existing_92.csv] has modified primary id from 8123000 to 8500000")
+    }
+
+    def "cannot change committed ids to a branch"() {
+        given:
+        definedTransportVersion("existing_92", "8123000,8012002")
+        when:
+        def result = validateDefinitionsFails()
+        then:
+        assertDefinitionsFailure(result, "Transport version definition file " +
+            "[myserver/src/main/resources/transport/defined/existing_92.csv] modifies existing patch id from 8012001 to 8012002")
+    }
+
+    def "latest files must reference defined name"() {
+        given:
+        latestTransportVersion("9.2", "dne", "8123000")
+        when:
+        def result = validateDefinitionsFails()
+        then:
+        assertDefinitionsFailure(result, "Latest transport version file " +
+            "[myserver/src/main/resources/transport/latest/9.2.csv] contains transport version name [dne] which is not defined")
+    }
+
+    def "latest files id must exist in definition"() {
+        given:
+        latestTransportVersion("9.2", "existing_92", "8124000")
+        when:
+        def result = validateDefinitionsFails()
+        then:
+        assertDefinitionsFailure(result, "Latest transport version file " +
+            "[myserver/src/main/resources/transport/latest/9.2.csv] has id 8124000 which is not in definition " +
+            "[myserver/src/main/resources/transport/defined/existing_92.csv]")
+    }
+
+    def "latest files have latest id within base"() {
+        given:
+        latestTransportVersion("9.0", "seemingly_latest", "8110001")
+        definedAndUsedTransportVersion("original", "8110000")
+        definedAndUsedTransportVersion("seemingly_latest", "8111000,8110001")
+        definedAndUsedTransportVersion("actual_latest", "8112000,8110002")
+        when:
+        def result = validateDefinitionsFails()
+        then:
+        assertDefinitionsFailure(result, "Latest transport version file " +
+            "[myserver/src/main/resources/transport/latest/9.0.csv] has id 8110001 from [seemingly_latest] with base 8110000 " +
+            "but another id 8110002 from [actual_latest] is later for that base")
+    }
+
+    def "latest files cannot change base id"() {
+        given:
+        definedAndUsedTransportVersion("original", "8013000")
+        definedAndUsedTransportVersion("patch", "8015000,8013001")
+        latestTransportVersion("9.1", "patch", "8013001")
+        when:
+        def result = validateDefinitionsFails()
+        then:
+        assertDefinitionsFailure(result, "Latest transport version file " +
+            "[myserver/src/main/resources/transport/latest/9.1.csv] modifies base id from 8012000 to 8013000")
+    }
+
+    def "ids must be dense"() {
+        given:
+        definedAndUsedTransportVersion("original", "8013000")
+        definedAndUsedTransportVersion("patch1", "8015000,8013002")
+        latestTransportVersion("9.0", "patch1", "8013002")
+        when:
+        def result = validateDefinitionsFails()
+        then:
+        assertDefinitionsFailure(result, "Transport version base id 8013000 is missing patch ids between 8013000 and 8013002")
+    }
+
+    def "primary id must not be patch version"() {
+        given:
+        definedAndUsedTransportVersion("patch", "8015001")
+        when:
+        def result = validateDefinitionsFails()
+        then:
+        assertDefinitionsFailure(result, "Transport version definition file " +
+            "[myserver/src/main/resources/transport/defined/patch.csv] has patch version 8015001 as primary id")
+    }
+}

+ 4 - 3
build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/CollectTransportVersionReferencesTask.java

@@ -74,14 +74,15 @@ public abstract class CollectTransportVersionReferencesTask extends DefaultTask
         Files.writeString(outputFile, String.join("\n", results.stream().map(Object::toString).sorted().toList()));
     }
 
-    private void addNamesFromClassesDirectory(Set<TransportVersionUtils.TransportVersionReference> results, Path file) throws IOException {
-        Files.walkFileTree(file, new SimpleFileVisitor<>() {
+    private void addNamesFromClassesDirectory(Set<TransportVersionUtils.TransportVersionReference> results, Path basePath)
+        throws IOException {
+        Files.walkFileTree(basePath, new SimpleFileVisitor<>() {
             @Override
             public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
                 String filename = file.getFileName().toString();
                 if (filename.endsWith(CLASS_EXTENSION) && filename.endsWith(MODULE_INFO) == false) {
                     try (var inputStream = Files.newInputStream(file)) {
-                        addNamesFromClass(results, inputStream, classname(file.toString()));
+                        addNamesFromClass(results, inputStream, classname(basePath.relativize(file).toString()));
                     }
                 }
                 return FileVisitResult.CONTINUE;

+ 2 - 2
build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionManifestTask.java

@@ -29,10 +29,10 @@ public abstract class GenerateTransportVersionManifestTask extends DefaultTask {
 
     @TaskAction
     public void generateTransportVersionManifest() throws IOException {
-        Path constantsDir = getDefinitionsDirectory().get().getAsFile().toPath();
+        Path definitionsDir = getDefinitionsDirectory().get().getAsFile().toPath();
         Path manifestFile = getManifestFile().get().getAsFile().toPath();
         try (var writer = Files.newBufferedWriter(manifestFile)) {
-            try (var stream = Files.list(constantsDir)) {
+            try (var stream = Files.list(definitionsDir)) {
                 for (String filename : stream.map(p -> p.getFileName().toString()).toList()) {
                     if (filename.equals(manifestFile.getFileName().toString())) {
                         // don't list self

+ 7 - 4
build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GlobalTransportVersionManagementPlugin.java

@@ -20,6 +20,9 @@ import org.gradle.language.base.plugins.LifecycleBasePlugin;
 
 import java.util.Map;
 
+import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.getDefinitionsDirectory;
+import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.getResourcesDirectory;
+
 public class GlobalTransportVersionManagementPlugin implements Plugin<Project> {
 
     @Override
@@ -43,9 +46,9 @@ public class GlobalTransportVersionManagementPlugin implements Plugin<Project> {
             .register("validateTransportVersionDefinitions", ValidateTransportVersionDefinitionsTask.class, t -> {
                 t.setGroup("Transport Versions");
                 t.setDescription("Validates that all defined TransportVersion constants are used in at least one project");
-                Directory definitionsDir = TransportVersionUtils.getDefinitionsDirectory(project);
-                if (definitionsDir.getAsFile().exists()) {
-                    t.getDefinitionsDirectory().set(definitionsDir);
+                Directory resourcesDir = getResourcesDirectory(project);
+                if (resourcesDir.getAsFile().exists()) {
+                    t.getResourcesDirectory().set(resourcesDir);
                 }
                 t.getReferencesFiles().setFrom(tvReferencesConfig);
             });
@@ -55,7 +58,7 @@ public class GlobalTransportVersionManagementPlugin implements Plugin<Project> {
             .register("generateTransportVersionManifest", GenerateTransportVersionManifestTask.class, t -> {
                 t.setGroup("Transport Versions");
                 t.setDescription("Generate a manifest resource for all the known transport version definitions");
-                t.getDefinitionsDirectory().set(TransportVersionUtils.getDefinitionsDirectory(project));
+                t.getDefinitionsDirectory().set(getDefinitionsDirectory(getResourcesDirectory(project)));
                 t.getManifestFile().set(project.getLayout().getBuildDirectory().file("generated-resources/manifest.txt"));
             });
         project.getTasks().named(JavaPlugin.PROCESS_RESOURCES_TASK_NAME, Copy.class).configure(t -> {

+ 4 - 1
build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionManagementPlugin.java

@@ -17,6 +17,9 @@ import org.gradle.api.file.Directory;
 import org.gradle.api.tasks.SourceSet;
 import org.gradle.language.base.plugins.LifecycleBasePlugin;
 
+import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.getDefinitionsDirectory;
+import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.getResourcesDirectory;
+
 public class TransportVersionManagementPlugin implements Plugin<Project> {
 
     @Override
@@ -43,7 +46,7 @@ public class TransportVersionManagementPlugin implements Plugin<Project> {
             .register("validateTransportVersionReferences", ValidateTransportVersionReferencesTask.class, t -> {
                 t.setGroup("Transport Versions");
                 t.setDescription("Validates that all TransportVersion references used in the project have an associated definition file");
-                Directory definitionsDir = TransportVersionUtils.getDefinitionsDirectory(project);
+                Directory definitionsDir = getDefinitionsDirectory(getResourcesDirectory(project));
                 if (definitionsDir.getAsFile().exists()) {
                     t.getDefinitionsDirectory().set(definitionsDir);
                 }

+ 93 - 23
build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionUtils.java

@@ -9,8 +9,6 @@
 
 package org.elasticsearch.gradle.internal.transport;
 
-import com.google.common.collect.Comparators;
-
 import org.gradle.api.Project;
 import org.gradle.api.attributes.Attribute;
 import org.gradle.api.attributes.AttributeContainer;
@@ -21,7 +19,6 @@ import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
-import java.util.Comparator;
 import java.util.List;
 
 import static org.gradle.api.artifacts.type.ArtifactTypeDefinition.ARTIFACT_TYPE_ATTRIBUTE;
@@ -30,40 +27,96 @@ class TransportVersionUtils {
 
     static final Attribute<Boolean> TRANSPORT_VERSION_REFERENCES_ATTRIBUTE = Attribute.of("transport-version-references", Boolean.class);
 
-    record TransportVersionConstant(String name, List<Integer> ids) {}
-
     record TransportVersionReference(String name, String location) {
         @Override
         public String toString() {
-            return name + " " + location;
+            return name + "," + location;
         }
     }
 
-    static TransportVersionConstant readDefinitionFile(Path file) throws IOException {
-        assert file.endsWith(".csv");
-        String rawName = file.getFileName().toString();
-        String name = rawName.substring(0, rawName.length() - 4);
-        List<Integer> ids = new ArrayList<>();
-
-        for (String rawId : Files.readString(file, StandardCharsets.UTF_8).split(",")) {
-            try {
-                ids.add(Integer.parseInt(rawId.strip()));
-            } catch (NumberFormatException e) {
-                throw new IOException("Failed to parse id " + rawId + " in " + file, e);
+    record TransportVersionDefinition(String name, List<TransportVersionId> ids) {
+        public static TransportVersionDefinition fromString(String filename, String contents) {
+            assert filename.endsWith(".csv");
+            String name = filename.substring(0, filename.length() - 4);
+            List<TransportVersionId> ids = new ArrayList<>();
+
+            if (contents.isEmpty() == false) {
+                for (String rawId : contents.split(",")) {
+                    try {
+                        ids.add(parseId(rawId));
+                    } catch (NumberFormatException e) {
+                        throw new IllegalStateException("Failed to parse id " + rawId + " in " + filename, e);
+                    }
+                }
             }
+
+            return new TransportVersionDefinition(name, ids);
         }
+    }
+
+    record TransportVersionLatest(String branch, String name, TransportVersionId id) {
+        public static TransportVersionLatest fromString(String filename, String contents) {
+            assert filename.endsWith(".csv");
+            String branch = filename.substring(0, filename.length() - 4);
 
-        if (Comparators.isInOrder(ids, Comparator.reverseOrder()) == false) {
-            throw new IOException("invalid transport version data file [" + file + "], ids are not in sorted");
+            String[] parts = contents.split(",");
+            if (parts.length != 2) {
+                throw new IllegalStateException("Invalid transport version latest file [" + filename + "]: " + contents);
+            }
+
+            return new TransportVersionLatest(branch, parts[0], parseId(parts[1]));
         }
-        return new TransportVersionConstant(name, ids);
+    }
+
+    record TransportVersionId(int complete, int major, int server, int subsidiary, int patch) implements Comparable<TransportVersionId> {
+
+        static TransportVersionId fromString(String s) {
+            int complete = Integer.parseInt(s);
+            int patch = complete % 100;
+            int subsidiary = (complete / 100) % 10;
+            int server = (complete / 1000) % 1000;
+            int major = complete / 1000000;
+            return new TransportVersionId(complete, major, server, subsidiary, patch);
+        }
+
+        @Override
+        public int compareTo(TransportVersionId o) {
+            return Integer.compare(complete, o.complete);
+        }
+
+        @Override
+        public String toString() {
+            return Integer.toString(complete);
+        }
+
+        public int base() {
+            return (complete / 1000) * 1000;
+        }
+    }
+
+    static Path definitionFilePath(Directory resourcesDirectory, String name) {
+        return getDefinitionsDirectory(resourcesDirectory).getAsFile().toPath().resolve(name + ".csv");
+    }
+
+    static Path latestFilePath(Directory resourcesDirectory, String name) {
+        return getLatestDirectory(resourcesDirectory).getAsFile().toPath().resolve(name + ".csv");
+    }
+
+    static TransportVersionDefinition readDefinitionFile(Path file) throws IOException {
+        String contents = Files.readString(file, StandardCharsets.UTF_8).strip();
+        return TransportVersionDefinition.fromString(file.getFileName().toString(), contents);
+    }
+
+    static TransportVersionLatest readLatestFile(Path file) throws IOException {
+        String contents = Files.readString(file, StandardCharsets.UTF_8).strip();
+        return TransportVersionLatest.fromString(file.getFileName().toString(), contents);
     }
 
     static List<TransportVersionReference> readReferencesFile(Path file) throws IOException {
         assert file.endsWith(".txt");
         List<TransportVersionReference> results = new ArrayList<>();
         for (String line : Files.readAllLines(file, StandardCharsets.UTF_8)) {
-            String[] parts = line.split(" ", 2);
+            String[] parts = line.split(",", 2);
             if (parts.length != 2) {
                 throw new IOException("Invalid transport version data file [" + file + "]: " + line);
             }
@@ -72,13 +125,30 @@ class TransportVersionUtils {
         return results;
     }
 
-    static Directory getDefinitionsDirectory(Project project) {
+    private static TransportVersionId parseId(String rawId) {
+        int complete = Integer.parseInt(rawId);
+        int patch = complete % 100;
+        int subsidiary = (complete / 100) % 10;
+        int server = (complete / 1000) % 1000;
+        int major = complete / 1000000;
+        return new TransportVersionId(complete, major, server, subsidiary, patch);
+    }
+
+    static Directory getDefinitionsDirectory(Directory resourcesDirectory) {
+        return resourcesDirectory.dir("defined");
+    }
+
+    static Directory getLatestDirectory(Directory resourcesDirectory) {
+        return resourcesDirectory.dir("latest");
+    }
+
+    static Directory getResourcesDirectory(Project project) {
         var projectName = project.findProperty("org.elasticsearch.transport.definitionsProject");
         if (projectName == null) {
             projectName = ":server";
         }
         Directory projectDir = project.project(projectName.toString()).getLayout().getProjectDirectory();
-        return projectDir.dir("src/main/resources/transport/defined");
+        return projectDir.dir("src/main/resources/transport");
     }
 
     static void addTransportVersionReferencesAttribute(AttributeContainer attributes) {

+ 275 - 10
build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionDefinitionsTask.java

@@ -9,6 +9,11 @@
 
 package org.elasticsearch.gradle.internal.transport;
 
+import com.google.common.collect.Comparators;
+
+import org.elasticsearch.gradle.internal.transport.TransportVersionUtils.TransportVersionDefinition;
+import org.elasticsearch.gradle.internal.transport.TransportVersionUtils.TransportVersionId;
+import org.elasticsearch.gradle.internal.transport.TransportVersionUtils.TransportVersionLatest;
 import org.elasticsearch.gradle.internal.transport.TransportVersionUtils.TransportVersionReference;
 import org.gradle.api.DefaultTask;
 import org.gradle.api.file.ConfigurableFileCollection;
@@ -20,14 +25,32 @@ import org.gradle.api.tasks.Optional;
 import org.gradle.api.tasks.PathSensitive;
 import org.gradle.api.tasks.PathSensitivity;
 import org.gradle.api.tasks.TaskAction;
+import org.gradle.process.ExecOperations;
+import org.gradle.process.ExecResult;
 
+import java.io.ByteArrayOutputStream;
 import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
 import java.util.Set;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+import java.util.regex.Pattern;
+
+import javax.inject.Inject;
 
+import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.definitionFilePath;
+import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.latestFilePath;
 import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.readDefinitionFile;
+import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.readLatestFile;
 import static org.elasticsearch.gradle.internal.transport.TransportVersionUtils.readReferencesFile;
 
 /**
@@ -39,28 +62,270 @@ public abstract class ValidateTransportVersionDefinitionsTask extends DefaultTas
     @InputDirectory
     @Optional
     @PathSensitive(PathSensitivity.RELATIVE)
-    public abstract DirectoryProperty getDefinitionsDirectory();
+    public abstract DirectoryProperty getResourcesDirectory();
 
     @InputFiles
     @PathSensitive(PathSensitivity.RELATIVE)
     public abstract ConfigurableFileCollection getReferencesFiles();
 
+    private record IdAndDefinition(TransportVersionId id, TransportVersionDefinition definition) {}
+
+    private static final Pattern NAME_FORMAT = Pattern.compile("[a-z0-9_]+");
+
+    private final Path rootPath;
+    private final ExecOperations execOperations;
+
+    // all transport version names referenced
+    private final Set<String> allNames = new HashSet<>();
+    // direct lookup of definition by name
+    private final Map<String, TransportVersionDefinition> definitions = new HashMap<>();
+    // which resource files already existed
+    private final Set<String> existingResources = new HashSet<>();
+    // reverse lookup of ids back to name
+    private final Map<Integer, String> definedIds = new HashMap<>();
+    // lookup of base ids back to definition
+    private final Map<Integer, List<IdAndDefinition>> idsByBase = new HashMap<>();
+    // direct lookup of latest for each branch
+    Map<String, TransportVersionLatest> latestByBranch = new HashMap<>();
+
+    @Inject
+    public ValidateTransportVersionDefinitionsTask(ExecOperations execOperations) {
+        this.execOperations = execOperations;
+        this.rootPath = getProject().getRootProject().getLayout().getProjectDirectory().getAsFile().toPath();
+    }
+
     @TaskAction
     public void validateTransportVersions() throws IOException {
-        Path constantsDir = getDefinitionsDirectory().getAsFile().get().toPath();
+        Path resourcesDir = getResourcesDirectory().getAsFile().get().toPath();
+        Path definitionsDir = resourcesDir.resolve("defined");
+        Path latestDir = resourcesDir.resolve("latest");
+
+        // first check which resource files already exist in main
+        recordExistingResources();
+
+        // then collect all names referenced in the codebase
+        for (var referencesFile : getReferencesFiles()) {
+            readReferencesFile(referencesFile.toPath()).stream().map(TransportVersionReference::name).forEach(allNames::add);
+        }
+
+        // now load all definitions, do some validation and record them by various keys for later quick lookup
+        // NOTE: this must run after loading referenced names and existing definitions
+        // NOTE: this is sorted so that the order of cross validation is deterministic
+        try (var definitionsStream = Files.list(definitionsDir).sorted()) {
+            for (var definitionFile : definitionsStream.toList()) {
+                recordAndValidateDefinition(readDefinitionFile(definitionFile));
+            }
+        }
+
+        // cleanup base lookup so we can check ids
+        // NOTE: this must run after definition recording
+        for (var entry : idsByBase.entrySet()) {
+            cleanupAndValidateBase(entry.getKey(), entry.getValue());
+        }
+
+        // now load all latest versions and do validation
+        // NOTE: this must run after definition recording and idsByBase cleanup
+        try (var latestStream = Files.list(latestDir)) {
+            for (var latestFile : latestStream.toList()) {
+                recordAndValidateLatest(readLatestFile(latestFile));
+            }
+        }
+    }
+
+    private String gitCommand(String... args) {
+        final ByteArrayOutputStream stdout = new ByteArrayOutputStream();
+
+        List<String> command = new ArrayList<>();
+        Collections.addAll(command, "git", "-C", rootPath.toAbsolutePath().toString());
+        Collections.addAll(command, args);
+
+        ExecResult result = execOperations.exec(spec -> {
+            spec.setCommandLine(command);
+            spec.setStandardOutput(stdout);
+            spec.setErrorOutput(stdout);
+            spec.setIgnoreExitValue(true);
+        });
+
+        if (result.getExitValue() != 0) {
+            throw new RuntimeException(
+                "git command failed with exit code "
+                    + result.getExitValue()
+                    + System.lineSeparator()
+                    + "command: "
+                    + String.join(" ", command)
+                    + System.lineSeparator()
+                    + "output:"
+                    + System.lineSeparator()
+                    + stdout.toString(StandardCharsets.UTF_8)
+            );
+        }
+
+        return stdout.toString(StandardCharsets.UTF_8);
+    }
+
+    private void recordExistingResources() {
+        String resourcesPath = relativePath(getResourcesDirectory().getAsFile().get().toPath());
+        String output = gitCommand("ls-tree", "--name-only", "-r", "main", resourcesPath);
+        Collections.addAll(existingResources, output.split(System.lineSeparator()));
+    }
 
-        Set<String> allTvNames = new HashSet<>();
-        for (var tvReferencesFile : getReferencesFiles()) {
-            readReferencesFile(tvReferencesFile.toPath()).stream().map(TransportVersionReference::name).forEach(allTvNames::add);
+    private void recordAndValidateDefinition(TransportVersionDefinition definition) {
+        definitions.put(definition.name(), definition);
+        // record the ids for each base id so we can ensure compactness later
+        for (TransportVersionId id : definition.ids()) {
+            idsByBase.computeIfAbsent(id.base(), k -> new ArrayList<>()).add(new IdAndDefinition(id, definition));
+        }
+
+        // validate any modifications
+        Map<Integer, TransportVersionId> existingIdsByBase = new HashMap<>();
+        TransportVersionDefinition originalDefinition = readExistingDefinition(definition.name());
+        if (originalDefinition != null) {
+
+            int primaryId = definition.ids().get(0).complete();
+            int originalPrimaryId = originalDefinition.ids().get(0).complete();
+            if (primaryId != originalPrimaryId) {
+                throwDefinitionFailure(definition.name(), "has modified primary id from " + originalPrimaryId + " to " + primaryId);
+            }
+
+            originalDefinition.ids().forEach(id -> existingIdsByBase.put(id.base(), id));
+        }
+
+        if (allNames.contains(definition.name()) == false && definition.name().startsWith("initial_") == false) {
+            throwDefinitionFailure(definition.name(), "is not referenced");
+        }
+        if (NAME_FORMAT.matcher(definition.name()).matches() == false) {
+            throwDefinitionFailure(definition.name(), "does not have a valid name, must be lowercase alphanumeric and underscore");
+        }
+        if (definition.ids().isEmpty()) {
+            throwDefinitionFailure(definition.name(), "does not contain any ids");
         }
+        if (Comparators.isInOrder(definition.ids(), Comparator.reverseOrder()) == false) {
+            throwDefinitionFailure(definition.name(), "does not have ordered ids");
+        }
+        for (int ndx = 0; ndx < definition.ids().size(); ++ndx) {
+            TransportVersionId id = definition.ids().get(ndx);
+
+            String existing = definedIds.put(id.complete(), definition.name());
+            if (existing != null) {
+                throwDefinitionFailure(
+                    definition.name(),
+                    "contains id " + id + " already defined in [" + definitionRelativePath(existing) + "]"
+                );
+            }
 
-        try (var constantsStream = Files.list(constantsDir)) {
-            for (var constantsFile : constantsStream.toList()) {
-                var tv = readDefinitionFile(constantsFile);
-                if (allTvNames.contains(tv.name()) == false) {
-                    throw new IllegalStateException("Transport version constant " + tv.name() + " is not referenced");
+            if (ndx == 0) {
+                // TODO: initial versions will only be applicable to a release branch, so they won't have an associated
+                // main version. They will also be loaded differently in the future, but until they are separate, we ignore them here.
+                if (id.patch() != 0 && definition.name().startsWith("initial_") == false) {
+                    throwDefinitionFailure(definition.name(), "has patch version " + id.complete() + " as primary id");
                 }
+            } else {
+                if (id.patch() == 0) {
+                    throwDefinitionFailure(definition.name(), "contains bwc id [" + id + "] with a patch part of 0");
+                }
+            }
+
+            // check modifications of ids on same branch, ie sharing same base
+            TransportVersionId maybeModifiedId = existingIdsByBase.get(id.base());
+            if (maybeModifiedId != null && maybeModifiedId.complete() != id.complete()) {
+                throwDefinitionFailure(definition.name(), "modifies existing patch id from " + maybeModifiedId + " to " + id);
+            }
+        }
+    }
+
+    private TransportVersionDefinition readExistingDefinition(String name) {
+        return readExistingFile(name, this::definitionRelativePath, TransportVersionDefinition::fromString);
+    }
+
+    private TransportVersionLatest readExistingLatest(String branch) {
+        return readExistingFile(branch, this::latestRelativePath, TransportVersionLatest::fromString);
+    }
+
+    private <T> T readExistingFile(String name, Function<String, String> pathFunction, BiFunction<String, String, T> parser) {
+        String relativePath = pathFunction.apply(name);
+        if (existingResources.contains(relativePath) == false) {
+            return null;
+        }
+        String content = gitCommand("show", "main:" + relativePath).strip();
+        return parser.apply(relativePath, content);
+    }
+
+    private void recordAndValidateLatest(TransportVersionLatest latest) {
+        latestByBranch.put(latest.branch(), latest);
+
+        TransportVersionDefinition latestDefinition = definitions.get(latest.name());
+        if (latestDefinition == null) {
+            throwLatestFailure(latest.branch(), "contains transport version name [" + latest.name() + "] which is not defined");
+        }
+        if (latestDefinition.ids().contains(latest.id()) == false) {
+            throwLatestFailure(
+                latest.branch(),
+                "has id " + latest.id() + " which is not in definition [" + definitionRelativePath(latest.name()) + "]"
+            );
+        }
+
+        List<IdAndDefinition> baseIds = idsByBase.get(latest.id().base());
+        IdAndDefinition lastId = baseIds.getLast();
+        if (lastId.id().complete() != latest.id().complete()) {
+            throwLatestFailure(
+                latest.branch(),
+                "has id "
+                    + latest.id()
+                    + " from ["
+                    + latest.name()
+                    + "] with base "
+                    + latest.id().base()
+                    + " but another id "
+                    + lastId.id().complete()
+                    + " from ["
+                    + lastId.definition().name()
+                    + "] is later for that base"
+            );
+        }
+
+        TransportVersionLatest existingLatest = readExistingLatest(latest.branch());
+        if (existingLatest != null) {
+            if (latest.id().patch() != 0 && latest.id().base() != existingLatest.id().base()) {
+                throwLatestFailure(latest.branch(), "modifies base id from " + existingLatest.id().base() + " to " + latest.id().base());
+            }
+        }
+    }
+
+    private void cleanupAndValidateBase(int base, List<IdAndDefinition> ids) {
+        // first sort the ids list so we can check compactness and quickly lookup the highest id later
+        ids.sort(Comparator.comparingInt(a -> a.id().complete()));
+
+        // TODO: switch this to a fully dense check once all existing transport versions have been migrated
+        IdAndDefinition previous = ids.getLast();
+        for (int ndx = ids.size() - 2; ndx >= 0; --ndx) {
+            IdAndDefinition next = ids.get(ndx);
+            // note that next and previous are reversed here because we are iterating in reverse order
+            if (previous.id().complete() - 1 != next.id().complete()) {
+                throw new IllegalStateException(
+                    "Transport version base id " + base + " is missing patch ids between " + next.id() + " and " + previous.id()
+                );
             }
+            previous = next;
         }
     }
+
+    private void throwDefinitionFailure(String name, String message) {
+        throw new IllegalStateException("Transport version definition file [" + definitionRelativePath(name) + "] " + message);
+    }
+
+    private void throwLatestFailure(String branch, String message) {
+        throw new IllegalStateException("Latest transport version file [" + latestRelativePath(branch) + "] " + message);
+    }
+
+    private String definitionRelativePath(String name) {
+        return relativePath(definitionFilePath(getResourcesDirectory().get(), name));
+    }
+
+    private String latestRelativePath(String branch) {
+        return relativePath(latestFilePath(getResourcesDirectory().get(), branch));
+    }
+
+    private String relativePath(Path file) {
+        return rootPath.relativize(file).toString();
+    }
 }

+ 0 - 2
server/src/main/java/org/elasticsearch/TransportVersions.java

@@ -149,7 +149,6 @@ public class TransportVersions {
     public static final TransportVersion INITIAL_ELASTICSEARCH_8_18_2 = def(8_840_0_04);
     public static final TransportVersion INITIAL_ELASTICSEARCH_8_18_3 = def(8_840_0_05);
     public static final TransportVersion INITIAL_ELASTICSEARCH_8_18_4 = def(8_840_0_06);
-    public static final TransportVersion INITIAL_ELASTICSEARCH_8_18_5 = def(8_840_0_07);
     public static final TransportVersion INITIAL_ELASTICSEARCH_8_18_6 = def(8_840_0_08);
     public static final TransportVersion INITIAL_ELASTICSEARCH_8_19 = def(8_841_0_00);
     public static final TransportVersion COHERE_BIT_EMBEDDING_TYPE_SUPPORT_ADDED_BACKPORT_8_X = def(8_841_0_01);
@@ -222,7 +221,6 @@ public class TransportVersions {
     public static final TransportVersion INITIAL_ELASTICSEARCH_9_0_2 = def(9_000_0_11);
     public static final TransportVersion INITIAL_ELASTICSEARCH_9_0_3 = def(9_000_0_12);
     public static final TransportVersion INITIAL_ELASTICSEARCH_9_0_4 = def(9_000_0_13);
-    public static final TransportVersion INITIAL_ELASTICSEARCH_9_0_5 = def(9_000_0_14);
     public static final TransportVersion INITIAL_ELASTICSEARCH_9_0_6 = def(9_000_0_15);
     public static final TransportVersion COHERE_BIT_EMBEDDING_TYPE_SUPPORT_ADDED = def(9_001_0_00);
     public static final TransportVersion REMOVE_SNAPSHOT_FAILURES = def(9_002_0_00);

+ 1 - 0
server/src/main/resources/transport/defined/initial_elasticsearch_8_18_5.csv

@@ -0,0 +1 @@
+8840007

+ 1 - 0
server/src/main/resources/transport/defined/initial_elasticsearch_9_0_5.csv

@@ -0,0 +1 @@
+9000014

+ 1 - 1
server/src/main/resources/transport/latest/8.18.csv

@@ -1 +1 @@
-placeholder,8840007
+initial_elasticsearch_8_18_5,8840007

+ 1 - 1
server/src/main/resources/transport/latest/8.19.csv

@@ -1 +1 @@
-placeholder,8841064
+esql_split_on_big_values,8841063

+ 1 - 1
server/src/main/resources/transport/latest/9.0.csv

@@ -1 +1 @@
-placeholder,9000014
+initial_elasticsearch_9_0_5,9000014

+ 1 - 1
server/src/main/resources/transport/latest/9.1.csv

@@ -1 +1 @@
-placeholder,9112003
+esql_split_on_big_values,9112001

+ 1 - 1
server/src/main/resources/transport/latest/9.2.csv

@@ -1 +1 @@
-placeholder,9130000
+esql_split_on_big_values,9116000