Skip to content

Commit ce0173c

Browse files
rjernstsarog
authored andcommitted
Validate the highest transport id is used (elastic#133689) (elastic#133741)
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 bf2cdef commit ce0173c

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
@@ -85,6 +85,8 @@ public void validateTransportVersions() throws IOException {
8585
for (var upperBound : upperBounds.values()) {
8686
validateUpperBound(upperBound, allDefinitions, idsByBase);
8787
}
88+
89+
validateLargestIdIsUsed(upperBounds, allDefinitions);
8890
}
8991

9092
private Map<String, TransportVersionDefinition> collectAllDefinitions(
@@ -255,6 +257,27 @@ private void validateBase(int base, List<IdAndDefinition> ids) {
255257
}
256258
}
257259

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

0 commit comments

Comments
 (0)