diff --git a/rest-api-spec/src/yamlRestTest/java/org/elasticsearch/test/rest/ClientYamlTestSuiteIT.java b/rest-api-spec/src/yamlRestTest/java/org/elasticsearch/test/rest/ClientYamlTestSuiteIT.java index 465f17eca5532..2b3bab21e8ae6 100644 --- a/rest-api-spec/src/yamlRestTest/java/org/elasticsearch/test/rest/ClientYamlTestSuiteIT.java +++ b/rest-api-spec/src/yamlRestTest/java/org/elasticsearch/test/rest/ClientYamlTestSuiteIT.java @@ -13,6 +13,7 @@ import com.carrotsearch.randomizedtesting.annotations.TimeoutSuite; import org.apache.lucene.tests.util.TimeUnits; +import org.elasticsearch.core.UpdateForV9; import org.elasticsearch.test.cluster.ElasticsearchCluster; import org.elasticsearch.test.cluster.FeatureFlag; import org.elasticsearch.test.rest.yaml.ClientYamlTestCandidate; @@ -39,9 +40,15 @@ public ClientYamlTestSuiteIT(@Name("yaml") ClientYamlTestCandidate testCandidate super(testCandidate); } + @UpdateForV9 // remove restCompat check @ParametersFactory public static Iterable parameters() throws Exception { - return createParameters(); + String restCompatProperty = System.getProperty("tests.restCompat"); + if ("true".equals(restCompatProperty)) { + return createParametersWithLegacyNodeSelectorSupport(); + } else { + return createParameters(); + } } @Override diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.put_mapping/10_basic.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.put_mapping/10_basic.yml index fc747f401b11d..ef121411d8351 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.put_mapping/10_basic.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.put_mapping/10_basic.yml @@ -87,7 +87,7 @@ --- "Put mappings with explicit _doc type bwc": - skip: - version: "8.0.0 - " #TODO: add "mixed" to skip test for mixed cluster/upgrade tests + version: "8.0.0 - " reason: "old deprecation message for pre 8.0" features: "node_selector" - do: @@ -96,7 +96,7 @@ - do: node_selector: - version: " - 7.99.99" #TODO: OR replace with "non_current" here + version: "original" catch: bad_request indices.put_mapping: index: test_index diff --git a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java index 4be9481df58b1..7d8d1175385a1 100644 --- a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java +++ b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java @@ -28,6 +28,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.support.XContentMapValues; import org.elasticsearch.core.IOUtils; +import org.elasticsearch.core.UpdateForV9; import org.elasticsearch.test.ClasspathUtils; import org.elasticsearch.test.rest.ESRestTestCase; import org.elasticsearch.test.rest.TestFeatureService; @@ -35,8 +36,10 @@ import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestSpec; import org.elasticsearch.test.rest.yaml.section.ClientYamlTestSection; import org.elasticsearch.test.rest.yaml.section.ClientYamlTestSuite; +import org.elasticsearch.test.rest.yaml.section.DoSection; import org.elasticsearch.test.rest.yaml.section.ExecutableSection; import org.elasticsearch.xcontent.NamedXContentRegistry; +import org.elasticsearch.xcontent.ParseField; import org.elasticsearch.xcontent.ToXContent; import org.elasticsearch.xcontent.XContentBuilder; import org.junit.AfterClass; @@ -61,6 +64,7 @@ import java.util.SortedSet; import java.util.TreeSet; import java.util.stream.Collectors; +import java.util.stream.Stream; import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder; @@ -230,6 +234,28 @@ public static void closeClient() throws IOException { } } + /** + * Create parameters for this parameterized test. + * Enables support for parsing the legacy version-based node_selector format. + */ + @Deprecated + @UpdateForV9 + public static Iterable createParametersWithLegacyNodeSelectorSupport() throws Exception { + var executableSectionRegistry = new NamedXContentRegistry( + Stream.concat( + ExecutableSection.DEFAULT_EXECUTABLE_CONTEXTS.stream().filter(entry -> entry.name.getPreferredName().equals("do") == false), + Stream.of( + new NamedXContentRegistry.Entry( + ExecutableSection.class, + new ParseField("do"), + DoSection::parseWithLegacyNodeSelectorSupport + ) + ) + ).toList() + ); + return createParameters(executableSectionRegistry, null); + } + /** * Create parameters for this parameterized test. Uses the * {@link ExecutableSection#XCONTENT_REGISTRY list} of executable sections diff --git a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java index 4155472b42640..e850ade2bdf1d 100644 --- a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java +++ b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java @@ -10,6 +10,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.elasticsearch.Build; import org.elasticsearch.Version; import org.elasticsearch.client.HasAttributeNodeSelector; import org.elasticsearch.client.Node; @@ -86,6 +87,16 @@ */ public class DoSection implements ExecutableSection { public static DoSection parse(XContentParser parser) throws IOException { + return parse(parser, false); + } + + @UpdateForV9 + @Deprecated + public static DoSection parseWithLegacyNodeSelectorSupport(XContentParser parser) throws IOException { + return parse(parser, true); + } + + private static DoSection parse(XContentParser parser, boolean enableLegacyNodeSelectorSupport) throws IOException { String currentFieldName = null; XContentParser.Token token; @@ -175,7 +186,7 @@ public static DoSection parse(XContentParser parser) throws IOException { if (token == XContentParser.Token.FIELD_NAME) { selectorName = parser.currentName(); } else { - NodeSelector newSelector = buildNodeSelector(selectorName, parser); + NodeSelector newSelector = buildNodeSelector(selectorName, parser, enableLegacyNodeSelectorSupport); nodeSelector = nodeSelector == NodeSelector.ANY ? newSelector : new ComposeNodeSelector(nodeSelector, newSelector); @@ -610,10 +621,11 @@ private String formatStatusCodeMessage(ClientYamlTestResponse restTestResponse, ) ); - private static NodeSelector buildNodeSelector(String name, XContentParser parser) throws IOException { + private static NodeSelector buildNodeSelector(String name, XContentParser parser, boolean enableLegacyVersionSupport) + throws IOException { return switch (name) { case "attribute" -> parseAttributeValuesSelector(parser); - case "version" -> parseVersionSelector(parser); + case "version" -> parseVersionSelector(parser, enableLegacyVersionSupport); default -> throw new XContentParseException(parser.getTokenLocation(), "unknown node_selector [" + name + "]"); }; } @@ -678,14 +690,31 @@ private static boolean matchWithRange( } } - private static NodeSelector parseVersionSelector(XContentParser parser) throws IOException { + private static NodeSelector parseVersionSelector(XContentParser parser, boolean enableLegacyVersionSupport) throws IOException { if (false == parser.currentToken().isValue()) { throw new XContentParseException(parser.getTokenLocation(), "expected [version] to be a value"); } - var acceptedVersionRange = VersionRange.parseVersionRanges(parser.text()); - final Predicate nodeMatcher = nodeVersion -> matchWithRange(nodeVersion, acceptedVersionRange, parser.getTokenLocation()); - final String versionSelectorString = "version ranges " + acceptedVersionRange; + final Predicate nodeMatcher; + final String versionSelectorString; + if (parser.text().equals("current")) { + nodeMatcher = nodeVersion -> Build.current().version().equals(nodeVersion); + versionSelectorString = "version is " + Build.current().version() + " (current)"; + } else if (parser.text().equals("original")) { + nodeMatcher = nodeVersion -> Build.current().version().equals(nodeVersion) == false; + versionSelectorString = "version is not current (original)"; + } else { + if (enableLegacyVersionSupport) { + var acceptedVersionRange = VersionRange.parseVersionRanges(parser.text()); + nodeMatcher = nodeVersion -> matchWithRange(nodeVersion, acceptedVersionRange, parser.getTokenLocation()); + versionSelectorString = "version ranges " + acceptedVersionRange; + } else { + throw new XContentParseException( + parser.getTokenLocation(), + "unknown version selector [" + parser.text() + "]. Only [current] and [original] are allowed." + ); + } + } return new NodeSelector() { @Override diff --git a/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java index 0cb9a3e29e63f..7d9557d29e568 100644 --- a/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java +++ b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java @@ -10,12 +10,12 @@ import org.apache.http.HttpHost; import org.elasticsearch.Build; -import org.elasticsearch.Version; import org.elasticsearch.client.Node; import org.elasticsearch.client.NodeSelector; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.logging.HeaderWarning; import org.elasticsearch.core.Strings; +import org.elasticsearch.core.UpdateForV9; import org.elasticsearch.test.rest.yaml.ClientYamlTestExecutionContext; import org.elasticsearch.test.rest.yaml.ClientYamlTestResponse; import org.elasticsearch.xcontent.XContentLocation; @@ -579,14 +579,15 @@ public void testParseDoSectionAllowedWarnings() throws Exception { assertThat(e.getMessage(), equalTo("the warning [foo] was both allowed and expected")); } - public void testNodeSelectorByVersionRange() throws IOException { + @UpdateForV9 // remove + public void testLegacyNodeSelectorByVersionRange() throws IOException { parser = createParser(YamlXContent.yamlXContent, """ node_selector: version: 5.2.0-6.0.0 indices.get_field_mapping: index: test_index"""); - DoSection doSection = DoSection.parse(parser); + DoSection doSection = DoSection.parseWithLegacyNodeSelectorSupport(parser); assertNotSame(NodeSelector.ANY, doSection.getApiCallSection().getNodeSelector()); Node v170 = nodeWithVersion("1.7.0"); Node v521 = nodeWithVersion("5.2.1"); @@ -629,26 +630,21 @@ public void testNodeSelectorByVersionRange() throws IOException { } } - public void testNodeSelectorByVersionRangeFailsWithNonSemanticVersion() throws IOException { + public void testNodeSelectorByVersionRangeFails() throws IOException { parser = createParser(YamlXContent.yamlXContent, """ node_selector: version: 5.2.0-6.0.0 indices.get_field_mapping: index: test_index"""); - DoSection doSection = DoSection.parse(parser); - assertNotSame(NodeSelector.ANY, doSection.getApiCallSection().getNodeSelector()); - Node nonSemantic = nodeWithVersion("abddef"); - List nodes = new ArrayList<>(); + var exception = expectThrows(XContentParseException.class, () -> DoSection.parse(parser)); + assertThat(exception.getMessage(), endsWith("unknown version selector [5.2.0-6.0.0]. Only [current] and [original] are allowed.")); - var exception = expectThrows( - XContentParseException.class, - () -> doSection.getApiCallSection().getNodeSelector().select(List.of(nonSemantic)) - ); - assertThat( - exception.getMessage(), - endsWith("[version] range node selector expects a semantic version format (x.y.z), but found abddef") - ); + // We are throwing an early exception - this means the parser content is not fully consumed. This is OK as it would make + // the tests fail pointing to the correct syntax error location, preventing any further use of parser. + // Explicitly close the parser to avoid AbstractClientYamlTestFragmentParserTestCase checks. + parser.close(); + parser = null; } public void testNodeSelectorCurrentVersion() throws IOException { @@ -663,16 +659,36 @@ public void testNodeSelectorCurrentVersion() throws IOException { Node v170 = nodeWithVersion("1.7.0"); Node v521 = nodeWithVersion("5.2.1"); Node v550 = nodeWithVersion("5.5.0"); - Node oldCurrent = nodeWithVersion(Version.CURRENT.toString()); - Node newCurrent = nodeWithVersion(Build.current().version()); + Node current = nodeWithVersion(Build.current().version()); + List nodes = new ArrayList<>(); + nodes.add(v170); + nodes.add(v521); + nodes.add(v550); + nodes.add(current); + doSection.getApiCallSection().getNodeSelector().select(nodes); + assertEquals(List.of(current), nodes); + } + + public void testNodeSelectorNonCurrentVersion() throws IOException { + parser = createParser(YamlXContent.yamlXContent, """ + node_selector: + version: original + indices.get_field_mapping: + index: test_index"""); + + DoSection doSection = DoSection.parse(parser); + assertNotSame(NodeSelector.ANY, doSection.getApiCallSection().getNodeSelector()); + Node v170 = nodeWithVersion("1.7.0"); + Node v521 = nodeWithVersion("5.2.1"); + Node v550 = nodeWithVersion("5.5.0"); + Node current = nodeWithVersion(Build.current().version()); List nodes = new ArrayList<>(); nodes.add(v170); nodes.add(v521); nodes.add(v550); - nodes.add(oldCurrent); - nodes.add(newCurrent); + nodes.add(current); doSection.getApiCallSection().getNodeSelector().select(nodes); - assertEquals(List.of(oldCurrent, newCurrent), nodes); + assertEquals(List.of(v170, v521, v550), nodes); } private static Node nodeWithVersion(String version) { @@ -741,7 +757,7 @@ private static Node nodeWithAttributes(Map> attributes) { public void testNodeSelectorByTwoThings() throws IOException { parser = createParser(YamlXContent.yamlXContent, """ node_selector: - version: 5.2.0-6.0.0 + version: current attribute: attr: val indices.get_field_mapping: @@ -749,9 +765,9 @@ public void testNodeSelectorByTwoThings() throws IOException { DoSection doSection = DoSection.parse(parser); assertNotSame(NodeSelector.ANY, doSection.getApiCallSection().getNodeSelector()); - Node both = nodeWithVersionAndAttributes("5.2.1", singletonMap("attr", singletonList("val"))); + Node both = nodeWithVersionAndAttributes(Build.current().version(), singletonMap("attr", singletonList("val"))); Node badVersion = nodeWithVersionAndAttributes("5.1.1", singletonMap("attr", singletonList("val"))); - Node badAttr = nodeWithVersionAndAttributes("5.2.1", singletonMap("notattr", singletonList("val"))); + Node badAttr = nodeWithVersionAndAttributes(Build.current().version(), singletonMap("notattr", singletonList("val"))); List nodes = new ArrayList<>(); nodes.add(both); nodes.add(badVersion);