remove need for manual ignore list

server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesNodeRequest.java

@@ -25,11 +25,12 @@
 
 import java.io.IOException;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 
-class FieldCapabilitiesNodeRequest extends ActionRequest implements IndicesRequest {
+class FieldCapabilitiesNodeRequest extends ActionRequest implements IndicesRequest.RemoteClusterShardRequest {
 
     private final List<ShardId> shardIds;
     private final String[] fields;
@@ -215,4 +216,9 @@ public int hashCode() {
         result = 31 * result + Arrays.hashCode(allowedTypes);
         return result;
     }
+
+    @Override
+    public Collection<ShardId> shards() {
+        return shardIds();
+    }
 }

server/src/main/java/org/elasticsearch/action/search/TransportOpenPointInTimeAction.java

@@ -47,6 +47,8 @@
 import org.elasticsearch.transport.TransportService;
 
 import java.io.IOException;
+import java.util.Collection;
+import java.util.List;
 import java.util.Map;
 import java.util.concurrent.Executor;
 import java.util.function.BiFunction;
@@ -280,7 +282,7 @@ boolean buildPointInTimeFromSearchResults() {
         }
     }
 
-    private static final class ShardOpenReaderRequest extends TransportRequest implements IndicesRequest {
+    private static final class ShardOpenReaderRequest extends TransportRequest implements IndicesRequest.RemoteClusterShardRequest {
         final ShardId shardId;
         final OriginalIndices originalIndices;
         final TimeValue keepAlive;
@@ -319,6 +321,11 @@ public String[] indices() {
         public IndicesOptions indicesOptions() {
             return originalIndices.indicesOptions();
         }
+
+        @Override
+        public Collection<ShardId> shards() {
+            return List.of(getShardId());
+        }
     }
 
     private static final class ShardOpenReaderResponse extends SearchPhaseResult {

server/src/main/java/org/elasticsearch/action/support/broadcast/unpromotable/BroadcastUnpromotableRequest.java

@@ -20,6 +20,8 @@
 import org.elasticsearch.index.shard.ShardId;
 
 import java.io.IOException;
+import java.util.Collection;
+import java.util.List;
 import java.util.Objects;
 
 import static org.elasticsearch.action.ValidateActions.addValidationError;
@@ -28,7 +30,7 @@
 /**
  * A request that is broadcast to the unpromotable assigned replicas of a primary.
  */
-public class BroadcastUnpromotableRequest extends ActionRequest implements IndicesRequest {
+public class BroadcastUnpromotableRequest extends ActionRequest implements IndicesRequest.RemoteClusterShardRequest {
 
     /**
      * Holds the index shard routing table that will be used by {@link TransportBroadcastUnpromotableAction} to broadcast the requests to
@@ -105,4 +107,9 @@ public boolean failShardOnError() {
     public IndicesOptions indicesOptions() {
         return strictSingleIndexNoExpandForbidClosed();
     }
+
+    @Override
+    public Collection<ShardId> shards() {
+        return List.of(shardId);
+    }
 }

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/termsenum/action/NodeTermsEnumRequest.java

@@ -19,14 +19,15 @@
 import org.elasticsearch.transport.TransportRequest;
 
 import java.io.IOException;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.Set;
 
 /**
  * Internal terms enum request executed directly against a specific node, querying potentially many
  * shards in one request
  */
-public class NodeTermsEnumRequest extends TransportRequest implements IndicesRequest {
+public class NodeTermsEnumRequest extends TransportRequest implements IndicesRequest.RemoteClusterShardRequest {
 
     private final String field;
     private final String string;
@@ -181,4 +182,9 @@ public IndicesOptions indicesOptions() {
     public boolean remove(ShardId shardId) {
         return shardIds.remove(shardId);
     }
+
+    @Override
+    public Collection<ShardId> shards() {
+        return shardIds();
+    }
 }

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/DataNodeRequest.java

@@ -32,11 +32,12 @@
 import org.elasticsearch.xpack.esql.session.EsqlConfiguration;
 
 import java.io.IOException;
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 
-final class DataNodeRequest extends TransportRequest implements IndicesRequest {
+final class DataNodeRequest extends TransportRequest implements IndicesRequest.RemoteClusterShardRequest {
     private static final PlanNameRegistry planNameRegistry = new PlanNameRegistry();
     private final String sessionId;
     private final EsqlConfiguration configuration;
@@ -140,6 +141,11 @@ List<ShardId> shardIds() {
         return shardIds;
     }
 
+    @Override
+    public Collection<ShardId> shards() {
+        return shardIds();
+    }
+
     /**
      * Returns a map from index UUID to alias filters
      */
@@ -179,4 +185,5 @@ public boolean equals(Object o) {
     public int hashCode() {
         return Objects.hash(sessionId, configuration, clusterAlias, shardIds, aliasFilters, plan);
     }
+
 }

x-pack/plugin/security/qa/consistency-checks/src/test/java/org/elasticsearch/xpack/security/CrossClusterShardTests.java

@@ -13,6 +13,7 @@
 import org.elasticsearch.common.inject.TypeLiteral;
 import org.elasticsearch.datastreams.DataStreamsPlugin;
 import org.elasticsearch.index.rankeval.RankEvalPlugin;
+import org.elasticsearch.index.shard.ShardId;
 import org.elasticsearch.ingest.IngestTestPlugin;
 import org.elasticsearch.ingest.common.IngestCommonPlugin;
 import org.elasticsearch.node.Node;
@@ -47,40 +48,24 @@
 import java.lang.reflect.Parameter;
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
+import java.lang.reflect.TypeVariable;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 /**
  * This test helps to ensure that RCS 2.0 transport actions and transport request handlers are correctly marked with the
  * IndicesRequest.RemoteClusterShardRequest interface. This interface is used to identify transport actions and request handlers
  * that operate on shards directly and can be used across clusters. This test will fail if a new transport action or request handler
  * is added that operates on shards directly and is not marked with the IndicesRequest.RemoteClusterShardRequest interface.
- * This is a best effort and no guarantee that all transport actions and request handlers are correctly marked.
+ * This is a best effort and not a guarantee that all transport actions and request handlers are correctly marked.
  */
 public class CrossClusterShardTests extends ESSingleNodeTestCase {
 
-    Set<String> IGNORED_ACTIONS = Set.of(
-        // TODO: go through this list and add comment or add interface
-        "org.elasticsearch.xpack.profiling.action.TransportGetStackTracesAction",
-        "org.elasticsearch.action.search.TransportSearchAction",
-        "org.elasticsearch.xpack.rollup.action.TransportRollupSearchAction.TransportHandler",
-        "org.elasticsearch.xpack.profiling.action.TransportGetFlamegraphAction",
-        "org.elasticsearch.action.fieldcaps.TransportFieldCapabilitiesAction.NodeTransportHandler",
-        "org.elasticsearch.xpack.rollup.action.TransportRollupSearchAction",
-        "org.elasticsearch.action.search.TransportOpenPointInTimeAction.ShardOpenReaderRequestHandler",
-        "org.elasticsearch.xpack.profiling.action.TransportGetTopNFunctionsAction",
-        "org.elasticsearch.action.support.broadcast.unpromotable.TransportBroadcastUnpromotableAction.UnpromotableTransportHandler",
-        "org.elasticsearch.xpack.esql.plugin.ComputeService.DataNodeRequestHandler",
-        "org.elasticsearch.action.search.TransportOpenPointInTimeAction",
-        "org.elasticsearch.xpack.core.termsenum.action.TransportTermsEnumAction.NodeTransportHandler",
-        "org.elasticsearch.action.admin.indices.validate.query.TransportValidateQueryAction",
-        "org.elasticsearch.action.support.broadcast.node.TransportBroadcastByNodeAction.BroadcastByNodeTransportRequestHandler"
-
-    );
 
     @Override
     protected Collection<Class<? extends Plugin>> getPlugins() {
@@ -133,10 +118,11 @@ public void testCheckForNewShardLevelTransportActions() throws Exception {
         for (Class<? extends IndicesRequest> clazz : indicesRequest) {
             for (Method method : clazz.getDeclaredMethods()) {
                 // not the most efficient way to check for shard related methods, but it's good enough for this test
-                // alternatively we could check methods that return some collection of ShardId or similar
-                // really we need a better way to inspect the request/action relationship
                 if (method.getName().toLowerCase(Locale.ROOT).contains("shard")) {
-                    candidateRequests.add(method.getDeclaringClass().getCanonicalName());
+                    // only care if the return type is a ShardId or a collection of ShardIds
+                    if(ShardId.class.getCanonicalName().equals(getTypeFromMaybeGeneric(method.getGenericReturnType()))) {
+                        candidateRequests.add(method.getDeclaringClass().getCanonicalName());
+                    }
                 }
             }
         }
@@ -157,7 +143,7 @@ public void testCheckForNewShardLevelTransportActions() throws Exception {
 
         // Find any transport actions that have methods related to shards, these are the candidate actions for the marker interface
         for (TransportAction transportAction : candidateActions) {
-            String actionRequestType = getRequestTypeName(transportAction.getClass().getGenericSuperclass());
+            String actionRequestType = getTypeFromMaybeGeneric(transportAction.getClass().getGenericSuperclass());
             if (candidateRequests.contains(actionRequestType)) {
                 actionsWithShardRequests.add(new FinalCandidate(transportAction.getClass().getCanonicalName(), actionRequestType));
             }
@@ -178,48 +164,36 @@ public void testCheckForNewShardLevelTransportActions() throws Exception {
             }
         }
 
-        // Sanity check that this test actually needs to ignore the actions it lists
-        for (String ignoredAction : IGNORED_ACTIONS) {
-            if (actionsWithShardRequests.stream().noneMatch(action -> action.actionClassName.equals(ignoredAction))) {
-                fail(
-                    String.format(
-                        "The action [%s] is in the IGNORED_ACTIONS list but is not needed. "
-                            + "Please remove [%s] from the IGNORED_ACTIONS.",
-                        ignoredAction, ignoredAction
-                    )
-                );
-            }
-        }
-
-        // filter out any ignored actions
-        actionsWithShardRequests.removeIf(action -> IGNORED_ACTIONS.contains(action.actionClassName));
-
         if (actionsWithShardRequests.isEmpty() == false) {
             fail(String.format("""
-                This test failed. You likely just added an index level transport action or transport request handler with an associated
-                transport request with `shard` in a method name. Transport actions or transport request handlers which
-                operate on shards directly and can be used across clusters must meet some additional requirements in order to be
-                handled correctly by all Elasticsearch infrastructure, so please make sure you have read the javadoc on the
-                IndicesRequest.RemoteClusterShardRequest interface and implemented. If this does apply to your change, please add the
-                canonical class name to the set of IGNORED_ACTIONS in this test. Failed action (request): %s
-                """, actionsWithShardRequests));
+                This test failed. You likely just added an index level transport action(s) or transport request handler(s)
+                [%s]
+                with an associated TransportRequest with `shard` in a method name. Transport actions or transport request handlers which
+                operate directly on shards and can be used across clusters must meet some additional requirements in order to be
+                handled correctly by the Elasticsearch security infrastructure. Please review the javadoc for
+                IndicesRequest.RemoteClusterShardRequest and implement the interface on the transport request(s)
+                [%s]
+                """, actionsWithShardRequests.stream().map(FinalCandidate::actionClassName).collect(Collectors.joining(", ")),
+                actionsWithShardRequests.stream().map(FinalCandidate::requestClassName).collect(Collectors.joining(", "))));
         }
     }
 
-    private static String getRequestTypeName(Type type) {
+    /**
+     * @return The canonical class name of the first parameter type of a generic type,
+     * or the canonical class name of the class if it's not a generic type
+     */
+    private static String getTypeFromMaybeGeneric(Type type) {
         if (type instanceof ParameterizedType parameterizedType) {
             Type[] typeArguments = parameterizedType.getActualTypeArguments();
-            return getRequestTypeName(typeArguments[0]);
+            return getTypeFromMaybeGeneric(typeArguments[0]);
+        } else if (type instanceof TypeVariable<?>) {
+            //too complex to handle this case, and is likely a CRTP pattern which we will catch the children of this class
+            return "";
         } else if (type instanceof Class) {
             return ((Class<?>) type).getCanonicalName();
         }
-        throw new RuntimeException("Unknown type: " + type);
+        throw new RuntimeException("Unknown type: " + type.getClass());
     }
 
-    private record FinalCandidate(String actionClassName, String requestClassName) {
-        @Override
-        public String toString() {
-            return actionClassName + " (" + requestClassName + ")";
-        }
-    }
+    private record FinalCandidate(String actionClassName, String requestClassName) {}
 }