Use valid REST version when determining capabilities (#123864)

If the rest api version is not specified, infer the correct one to use from the major versions present on the cluster, determined using features

Author: thecoop

server/src/main/java/module-info.java

@@ -429,7 +429,7 @@
     provides org.elasticsearch.features.FeatureSpecification
         with
             org.elasticsearch.action.bulk.BulkFeatures,
-            org.elasticsearch.features.FeatureInfrastructureFeatures,
+            org.elasticsearch.features.InfrastructureFeatures,
             org.elasticsearch.rest.action.admin.cluster.ClusterRerouteFeatures,
             org.elasticsearch.index.mapper.MapperFeatures,
             org.elasticsearch.index.IndexFeatures,

server/src/main/java/org/elasticsearch/action/admin/cluster/node/capabilities/NodesCapabilitiesRequest.java

@@ -11,9 +11,11 @@
 
 import org.elasticsearch.action.support.nodes.BaseNodesRequest;
 import org.elasticsearch.common.Strings;
+import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.RestApiVersion;
 import org.elasticsearch.rest.RestRequest;
 
+import java.util.Optional;
 import java.util.Set;
 
 public class NodesCapabilitiesRequest extends BaseNodesRequest {
@@ -22,7 +24,7 @@ public class NodesCapabilitiesRequest extends BaseNodesRequest {
     private String path = "/";
     private Set<String> parameters = Set.of();
     private Set<String> capabilities = Set.of();
-    private RestApiVersion restApiVersion = RestApiVersion.current();
+    private @Nullable RestApiVersion restApiVersion;
 
     public NodesCapabilitiesRequest() {
         // send to all nodes
@@ -75,7 +77,7 @@ public NodesCapabilitiesRequest restApiVersion(RestApiVersion restApiVersion) {
         return this;
     }
 
-    public RestApiVersion restApiVersion() {
-        return restApiVersion;
+    public Optional<RestApiVersion> restApiVersion() {
+        return Optional.ofNullable(restApiVersion);
     }
 }

server/src/main/java/org/elasticsearch/action/admin/cluster/node/capabilities/TransportNodesCapabilitiesAction.java

@@ -18,7 +18,8 @@
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.core.RestApiVersion;
-import org.elasticsearch.core.UpdateForV9;
+import org.elasticsearch.features.FeatureService;
+import org.elasticsearch.features.InfrastructureFeatures;
 import org.elasticsearch.injection.guice.Inject;
 import org.elasticsearch.rest.RestController;
 import org.elasticsearch.rest.RestRequest;
@@ -41,14 +42,16 @@ public class TransportNodesCapabilitiesAction extends TransportNodesAction<
     public static final ActionType<NodesCapabilitiesResponse> TYPE = new ActionType<>("cluster:monitor/nodes/capabilities");
 
     private final RestController restController;
+    private final FeatureService featureService;
 
     @Inject
     public TransportNodesCapabilitiesAction(
         ThreadPool threadPool,
         ClusterService clusterService,
         TransportService transportService,
         ActionFilters actionFilters,
-        RestController restController
+        RestController restController,
+        FeatureService featureService
     ) {
         super(
             TYPE.name(),
@@ -59,6 +62,7 @@ public TransportNodesCapabilitiesAction(
             threadPool.executor(ThreadPool.Names.MANAGEMENT)
         );
         this.restController = restController;
+        this.featureService = featureService;
     }
 
     @Override
@@ -72,13 +76,21 @@ protected NodesCapabilitiesResponse newResponse(
 
     @Override
     protected NodeCapabilitiesRequest newNodeRequest(NodesCapabilitiesRequest request) {
-        return new NodeCapabilitiesRequest(
-            request.method(),
-            request.path(),
-            request.parameters(),
-            request.capabilities(),
-            request.restApiVersion()
-        );
+        RestApiVersion restVersion;
+        if (request.restApiVersion().isPresent()) {
+            // explicit version - just use it, and see what happens
+            restVersion = request.restApiVersion().get();
+        } else if (featureService.clusterHasFeature(clusterService.state(), InfrastructureFeatures.CURRENT_VERSION)) {
+            restVersion = RestApiVersion.current(); // every node is at least this major version, so use that
+        } else {
+            // not all nodes are the current version. previous major version nodes do not understand
+            // the new REST API version, so query using the previous version.
+            // Capabilities can come and go, so it's ok for the response to change
+            // when the nodes change
+            restVersion = RestApiVersion.previous();
+        }
+
+        return new NodeCapabilitiesRequest(request.method(), request.path(), request.parameters(), request.capabilities(), restVersion);
     }
 
     @Override
@@ -129,10 +141,6 @@ public NodeCapabilitiesRequest(
             this.restApiVersion = restApiVersion;
         }
 
-        @UpdateForV9(owner = UpdateForV9.Owner.CORE_INFRA) // 8.x blows up in a mixed cluster when trying to read RestApiVersion.forMajor(9)
-        // ./gradlew ":qa:mixed-cluster:v8.16.0#mixedClusterTest"
-        // -Dtests.class="org.elasticsearch.backwards.MixedClusterClientYamlTestSuiteIT"
-        // -Dtests.method="test {p0=capabilities/10_basic/Capabilities API}"
         @Override
         public void writeTo(StreamOutput out) throws IOException {
             super.writeTo(out);
@@ -141,9 +149,7 @@ public void writeTo(StreamOutput out) throws IOException {
             out.writeString(path);
             out.writeCollection(parameters, StreamOutput::writeString);
             out.writeCollection(capabilities, StreamOutput::writeString);
-            // Fixme: lies! all lies!
-            out.writeVInt(8);
-            // out.writeVInt(restApiVersion.major);
+            out.writeVInt(restApiVersion.major);
         }
     }
 }

server/src/main/java/org/elasticsearch/features/FeatureInfrastructureFeatures.java

@@ -0,0 +1,55 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the "Elastic License
+ * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+package org.elasticsearch.features;
+
+import org.elasticsearch.core.RestApiVersion;
+
+import java.util.Set;
+
+/**
+ * This class specifies features for Elasticsearch infrastructure.
+ */
+public class InfrastructureFeatures implements FeatureSpecification {
+
+    /*
+     * These features are auto-generated from the constants in RestApiVersion.
+     *
+     * When there's a new major version, CURRENT becomes N+1 and PREVIOUS becomes N.
+     * Because PREVIOUS is marked as assumed, this doesn't stop N+1 nodes from joining the cluster.
+     * A little table helps:
+     *
+     * Major    |  9  |  10 |  11
+     * ---------|-----|---- |-----
+     * CURRENT  |  9  |  10 |  11
+     * PREVIOUS |  8  |  9  |  10
+     *
+     * v9 knows about REST API 9 and 8. v10 knows about REST API 10 and 9.
+     * A v10 node can join a v9 cluster, as the ES_V_8 feature known by v9 is assumed.
+     * But the v9 nodes don't know about ES_V_10, so that feature isn't active
+     * on the v10 nodes until the cluster is fully upgraded,
+     * at which point the ES_V_8 feature also disappears from the cluster.
+     *
+     * One thing you must not do is check the PREVIOUS_VERSION feature existence on the cluster,
+     * as the answer will be wrong (v9 nodes will assume that v10 nodes have the v8 feature) - hence why it is private.
+     * That feature only exists here so that upgrades work to remove the feature from the cluster.
+     */
+    public static final NodeFeature CURRENT_VERSION = new NodeFeature("ES_" + RestApiVersion.current());
+    private static final NodeFeature PREVIOUS_VERSION = new NodeFeature("ES_" + RestApiVersion.previous(), true);
+
+    @Override
+    public Set<NodeFeature> getFeatures() {
+        return Set.of(CURRENT_VERSION, PREVIOUS_VERSION);
+    }
+
+    @Override
+    public Set<NodeFeature> getTestFeatures() {
+        return Set.of(FeatureService.TEST_FEATURES_ENABLED);
+    }
+}

server/src/main/java/org/elasticsearch/features/InfrastructureFeatures.java

@@ -66,8 +66,10 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
         NodesCapabilitiesRequest r = requestNodes.method(RestRequest.Method.valueOf(request.param("method", "GET")))
             .path(path)
             .parameters(request.paramAsStringArray("parameters", Strings.EMPTY_ARRAY))
-            .capabilities(request.paramAsStringArray("capabilities", Strings.EMPTY_ARRAY))
-            .restApiVersion(request.getRestApiVersion());
+            .capabilities(request.paramAsStringArray("capabilities", Strings.EMPTY_ARRAY));
+        if (request.hasExplicitRestApiVersion()) {
+            r.restApiVersion(request.getRestApiVersion());
+        }
 
         return channel -> client.admin().cluster().nodesCapabilities(r, new NodesResponseRestListener<>(channel));
     }

server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestNodesCapabilitiesAction.java

@@ -8,7 +8,7 @@
 #
 
 org.elasticsearch.action.bulk.BulkFeatures
-org.elasticsearch.features.FeatureInfrastructureFeatures
+org.elasticsearch.features.InfrastructureFeatures
 org.elasticsearch.rest.action.admin.cluster.ClusterRerouteFeatures
 org.elasticsearch.index.IndexFeatures
 org.elasticsearch.index.mapper.MapperFeatures