Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -39,9 +40,15 @@ public ClientYamlTestSuiteIT(@Name("yaml") ClientYamlTestCandidate testCandidate
super(testCandidate);
}

@UpdateForV9 // remove restCompat check
@ParametersFactory
public static Iterable<Object[]> parameters() throws Exception {
return createParameters();
String restCompatProperty = System.getProperty("tests.restCompat");
if ("true".equals(restCompatProperty)) {
return createParametersWithLegacyNodeSelectorSupport();
} else {
return createParameters();
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,18 @@
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;
import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestApi;
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;
Expand All @@ -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;

Expand Down Expand Up @@ -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<Object[]> 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 + "]");
};
}
Expand Down Expand Up @@ -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<String> nodeMatcher = nodeVersion -> matchWithRange(nodeVersion, acceptedVersionRange, parser.getTokenLocation());
final String versionSelectorString = "version ranges " + acceptedVersionRange;
final Predicate<String> 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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<Node> 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();
Comment thread
williamrandolph marked this conversation as resolved.
parser = null;
}

public void testNodeSelectorCurrentVersion() throws IOException {
Expand All @@ -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<Node> 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<Node> 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) {
Expand Down Expand Up @@ -741,17 +757,17 @@ private static Node nodeWithAttributes(Map<String, List<String>> 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:
index: test_index""");

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<Node> nodes = new ArrayList<>();
nodes.add(both);
nodes.add(badVersion);
Expand Down