From 5627c2c465f2d36f97f056f0889a3d0ed122562a Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 27 Aug 2025 17:50:10 -0700 Subject: [PATCH 1/3] Validate the highest transport id is used Most upper bounds files for transport version are for an already branched version. For those, the upper bound must be within the base id for that branch, and validation exists to ensure the highest id within a branch is in the upper bound file for that branch. But for main the base is always different. This commit adds validation to ensure that the highest transport version id exists in _some_ upper bound file. Note that while we could try to identify which upper bound file belongs to main, this is tricky when validation runs in non-main branches. This check is just as good, just a little less specific than "it should exist in x.y upper bound file". --- .../TransportVersionValidationFuncTest.groovy | 23 +++++++++++++++++++ ...ValidateTransportVersionResourcesTask.java | 20 ++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionValidationFuncTest.groovy b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionValidationFuncTest.groovy index 3778ea3e62914..62e21d3aed2ca 100644 --- a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionValidationFuncTest.groovy +++ b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionValidationFuncTest.groovy @@ -229,4 +229,27 @@ class TransportVersionValidationFuncTest extends AbstractTransportVersionFuncTes then: result.task(":myserver:validateTransportVersionResources").outcome == TaskOutcome.SUCCESS } + + def "highest id in an referable definition should exist in an upper bounds file"() { + given: + referableAndReferencedTransportVersion("some_tv", "10000000") + when: + def result = validateResourcesFails() + then: + assertValidateResourcesFailure(result, "Transport version definition file " + + "[myserver/src/main/resources/transport/definitions/referable/some_tv.csv] " + + "has the highest transport version id [10000000] but is not present in any upper bounds files") + } + + def "highest id in an unreferable definition should exist in an upper bounds file"() { + given: + unreferableTransportVersion("initial_10.0.0", "10000000") + when: + def result = validateResourcesFails() + then: + // TODO: this should be _unreferable_ in the error message, but will require some rework + assertValidateResourcesFailure(result, "Transport version definition file " + + "[myserver/src/main/resources/transport/definitions/referable/initial_10.0.0.csv] " + + "has the highest transport version id [10000000] but is not present in any upper bounds files") + } } diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionResourcesTask.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionResourcesTask.java index 44e9bde8352f2..9e7af599f7447 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionResourcesTask.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionResourcesTask.java @@ -11,6 +11,7 @@ import com.google.common.collect.Comparators; +import org.elasticsearch.gradle.VersionProperties; import org.gradle.api.DefaultTask; import org.gradle.api.file.ConfigurableFileCollection; import org.gradle.api.provider.Property; @@ -83,6 +84,8 @@ public void validateTransportVersions() throws IOException { for (var upperBound : upperBounds.values()) { validateUpperBound(upperBound, allDefinitions, idsByBase); } + + validateLargestIdIsUsed(upperBounds, allDefinitions); } private Map collectAllDefinitions( @@ -253,6 +256,23 @@ private void validateBase(int base, List ids) { } } + + private void validateLargestIdIsUsed(Map upperBounds, Map allDefinitions) { + // first id is always the highest within a definition, and validated earlier + // note we use min instead of max because the id comparator is in descending order + var highestDefinition = allDefinitions.values().stream().min(Comparator.comparing(d -> d.ids().get(0))).get(); + var highestId = highestDefinition.ids().get(0); + + for (var upperBound : upperBounds.values()) { + if (upperBound.id().equals(highestId)) { + return; + } + } + + throwDefinitionFailure(highestDefinition, "has the highest transport version id [" + + highestId + "] but is not present in any upper bounds files"); + } + private void throwDefinitionFailure(TransportVersionDefinition definition, String message) { Path relativePath = getResources().get().getReferableDefinitionRepositoryPath(definition); throw new IllegalStateException("Transport version definition file [" + relativePath + "] " + message); From db7dc57bd323890bfd53b2e9ef5a3495bca89465 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Thu, 28 Aug 2025 01:04:21 +0000 Subject: [PATCH 2/3] [CI] Auto commit changes from spotless --- .../ValidateTransportVersionResourcesTask.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionResourcesTask.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionResourcesTask.java index 9e7af599f7447..222683d4514e5 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionResourcesTask.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionResourcesTask.java @@ -11,7 +11,6 @@ import com.google.common.collect.Comparators; -import org.elasticsearch.gradle.VersionProperties; import org.gradle.api.DefaultTask; import org.gradle.api.file.ConfigurableFileCollection; import org.gradle.api.provider.Property; @@ -256,8 +255,10 @@ private void validateBase(int base, List ids) { } } - - private void validateLargestIdIsUsed(Map upperBounds, Map allDefinitions) { + private void validateLargestIdIsUsed( + Map upperBounds, + Map allDefinitions + ) { // first id is always the highest within a definition, and validated earlier // note we use min instead of max because the id comparator is in descending order var highestDefinition = allDefinitions.values().stream().min(Comparator.comparing(d -> d.ids().get(0))).get(); @@ -269,8 +270,10 @@ private void validateLargestIdIsUsed(Map upp } } - throwDefinitionFailure(highestDefinition, "has the highest transport version id [" - + highestId + "] but is not present in any upper bounds files"); + throwDefinitionFailure( + highestDefinition, + "has the highest transport version id [" + highestId + "] but is not present in any upper bounds files" + ); } private void throwDefinitionFailure(TransportVersionDefinition definition, String message) { From 569f0b9819fcdba42500603eedc40aace4c7f590 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 27 Aug 2025 19:08:23 -0700 Subject: [PATCH 3/3] fix test --- .../transport/TransportVersionValidationFuncTest.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionValidationFuncTest.groovy b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionValidationFuncTest.groovy index 62e21d3aed2ca..36be4c4d94bff 100644 --- a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionValidationFuncTest.groovy +++ b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionValidationFuncTest.groovy @@ -223,7 +223,7 @@ class TransportVersionValidationFuncTest extends AbstractTransportVersionFuncTes def "unreferable definitions can have primary ids that are patches"() { given: - unreferableTransportVersion("initial_10.0.1", "10000001") + unreferableTransportVersion("initial_7.0.1", "7000001") when: def result = gradleRunner(":myserver:validateTransportVersionResources").build() then: