Skip to content

Commit ea3e5de

Browse files
committed
Validate the highest transport id is used (elastic#133689)
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".
1 parent f6c782c commit ea3e5de

File tree

2 files changed

+47
-1
lines changed

2 files changed

+47
-1
lines changed

build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionValidationFuncTest.groovy

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,10 +223,33 @@ class TransportVersionValidationFuncTest extends AbstractTransportVersionFuncTes
223223

224224
def "unreferable definitions can have primary ids that are patches"() {
225225
given:
226-
unreferableTransportVersion("initial_10.0.1", "10000001")
226+
unreferableTransportVersion("initial_7.0.1", "7000001")
227227
when:
228228
def result = gradleRunner(":myserver:validateTransportVersionResources").build()
229229
then:
230230
result.task(":myserver:validateTransportVersionResources").outcome == TaskOutcome.SUCCESS
231231
}
232+
233+
def "highest id in an referable definition should exist in an upper bounds file"() {
234+
given:
235+
referableAndReferencedTransportVersion("some_tv", "10000000")
236+
when:
237+
def result = validateResourcesFails()
238+
then:
239+
assertValidateResourcesFailure(result, "Transport version definition file " +
240+
"[myserver/src/main/resources/transport/definitions/referable/some_tv.csv] " +
241+
"has the highest transport version id [10000000] but is not present in any upper bounds files")
242+
}
243+
244+
def "highest id in an unreferable definition should exist in an upper bounds file"() {
245+
given:
246+
unreferableTransportVersion("initial_10.0.0", "10000000")
247+
when:
248+
def result = validateResourcesFails()
249+
then:
250+
// TODO: this should be _unreferable_ in the error message, but will require some rework
251+
assertValidateResourcesFailure(result, "Transport version definition file " +
252+
"[myserver/src/main/resources/transport/definitions/referable/initial_10.0.0.csv] " +
253+
"has the highest transport version id [10000000] but is not present in any upper bounds files")
254+
}
232255
}

build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionResourcesTask.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ public void validateTransportVersions() throws IOException {
8383
for (var upperBound : upperBounds.values()) {
8484
validateUpperBound(upperBound, allDefinitions, idsByBase);
8585
}
86+
87+
validateLargestIdIsUsed(upperBounds, allDefinitions);
8688
}
8789

8890
private Map<String, TransportVersionDefinition> collectAllDefinitions(
@@ -253,6 +255,27 @@ private void validateBase(int base, List<IdAndDefinition> ids) {
253255
}
254256
}
255257

258+
private void validateLargestIdIsUsed(
259+
Map<String, TransportVersionUpperBound> upperBounds,
260+
Map<String, TransportVersionDefinition> allDefinitions
261+
) {
262+
// first id is always the highest within a definition, and validated earlier
263+
// note we use min instead of max because the id comparator is in descending order
264+
var highestDefinition = allDefinitions.values().stream().min(Comparator.comparing(d -> d.ids().get(0))).get();
265+
var highestId = highestDefinition.ids().get(0);
266+
267+
for (var upperBound : upperBounds.values()) {
268+
if (upperBound.id().equals(highestId)) {
269+
return;
270+
}
271+
}
272+
273+
throwDefinitionFailure(
274+
highestDefinition,
275+
"has the highest transport version id [" + highestId + "] but is not present in any upper bounds files"
276+
);
277+
}
278+
256279
private void throwDefinitionFailure(TransportVersionDefinition definition, String message) {
257280
Path relativePath = getResources().get().getReferableDefinitionRepositoryPath(definition);
258281
throw new IllegalStateException("Transport version definition file [" + relativePath + "] " + message);

0 commit comments

Comments
 (0)