Filter out only the closed indices

Signed-off-by: Harsh Garg <[email protected]>

server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java

@@ -170,42 +170,38 @@ public void testListShardsWithClosedAndHiddenIndices() throws InterruptedExcepti
         // close index "test-closed-idx"
         client().admin().indices().close(Requests.closeIndexRequest("test-closed-idx")).get();
 
-        // Verifying responses for default queries: /_cat/shards and /_list/shards
-        CatShardsRequest shardsRequest = new CatShardsRequest();
-        shardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT);
-        shardsRequest.setIndices(Strings.EMPTY_ARRAY);
-        ActionFuture<CatShardsResponse> catShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest);
-        assertTrue(catShardsResponse.get().getResponseShards().stream().anyMatch(shard -> shard.getIndexName().equals("test-closed-idx")));
-        assertTrue(catShardsResponse.get().getResponseShards().stream().anyMatch(shard -> shard.getIndexName().equals("test-hidden-idx")));
-
-        shardsRequest.setPageParams(new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize));
-        ActionFuture<CatShardsResponse> listShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest);
+        // Verifying response for default queries: /_list/shards
+        // all the shards should be part of response, however stats should not be displayed for closed index
+        CatShardsRequest listShardsRequest = new CatShardsRequest();
+        listShardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT);
+        listShardsRequest.setIndices(Strings.EMPTY_ARRAY);
+        listShardsRequest.setPageParams(new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize));
+        ActionFuture<CatShardsResponse> listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest);
         assertTrue(listShardsResponse.get().getResponseShards().stream().anyMatch(shard -> shard.getIndexName().equals("test-closed-idx")));
         assertTrue(listShardsResponse.get().getResponseShards().stream().anyMatch(shard -> shard.getIndexName().equals("test-hidden-idx")));
-        assertEquals(catShardsResponse.get().getResponseShards().size(), listShardsResponse.get().getResponseShards().size());
-        assertEquals(
-            catShardsResponse.get().getIndicesStatsResponse().getShards().length,
-            listShardsResponse.get().getIndicesStatsResponse().getShards().length
+        assertFalse(
+            Arrays.stream(listShardsResponse.get().getIndicesStatsResponse().getShards())
+                .anyMatch(shardStats -> shardStats.getShardRouting().getIndexName().equals("test-closed-idx"))
         );
 
-        // Verifying responses when hidden indices are explicitly queried: /_cat/shards/test-hidden-idx and /_list/shards/test-hidden-idx
+        // Verifying responses when hidden indices are explicitly queried: /_list/shards/test-hidden-idx
         // Shards for hidden index should appear in response along with stats
-        shardsRequest = new CatShardsRequest();
-        shardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT);
-        shardsRequest.setIndices(List.of("test-hidden-idx").toArray(new String[0]));
-        catShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest);
-        assertTrue(catShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-hidden-idx")));
+        listShardsRequest.setIndices(List.of("test-hidden-idx").toArray(new String[0]));
+        listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest);
+        assertTrue(listShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-hidden-idx")));
         assertTrue(
-            Arrays.stream(catShardsResponse.get().getIndicesStatsResponse().getShards())
+            Arrays.stream(listShardsResponse.get().getIndicesStatsResponse().getShards())
                 .allMatch(shardStats -> shardStats.getShardRouting().getIndexName().equals("test-hidden-idx"))
         );
         assertEquals(
-            catShardsResponse.get().getResponseShards().size(),
-            catShardsResponse.get().getIndicesStatsResponse().getShards().length
+            listShardsResponse.get().getResponseShards().size(),
+            listShardsResponse.get().getIndicesStatsResponse().getShards().length
         );
 
-        shardsRequest.setPageParams(new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize));
-        listShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest);
+        // Verifying responses when hidden indices are queried with wildcards: /_list/shards/test-hidden-idx*
+        // Shards for hidden index should appear in response without stats
+        listShardsRequest.setIndices(List.of("test-hidden-idx*").toArray(new String[0]));
+        listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest);
         assertTrue(listShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-hidden-idx")));
         assertTrue(
             Arrays.stream(listShardsResponse.get().getIndicesStatsResponse().getShards())
@@ -216,52 +212,18 @@ public void testListShardsWithClosedAndHiddenIndices() throws InterruptedExcepti
             listShardsResponse.get().getIndicesStatsResponse().getShards().length
         );
 
-        // Verifying responses when hidden indices are queried with wildcards: /_cat/shards/test-hidden-idx* and
-        // /_list/shards/test-hidden-idx*
-        // Shards for hidden index should appear in response without stats
-        shardsRequest = new CatShardsRequest();
-        shardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT);
-        shardsRequest.setIndices(List.of("test-hidden-idx*").toArray(new String[0]));
-        catShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest);
-        assertTrue(catShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-hidden-idx")));
-        assertEquals(0, catShardsResponse.get().getIndicesStatsResponse().getShards().length);
-
-        shardsRequest.setPageParams(new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize));
-        listShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest);
-        assertTrue(listShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-hidden-idx")));
-        assertEquals(0, listShardsResponse.get().getIndicesStatsResponse().getShards().length);
-
-        // Explicitly querying for closed index: /_cat/shards/test-closed-idx and /_list/shards/test-closed-idx
-        // /_cat/shards/test-closed-idx should result in IndexClosedException
-        // while /_list/shards/test-closed-idx should output closed shards without stats.
-        shardsRequest = new CatShardsRequest();
-        shardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT);
-        shardsRequest.setIndices(List.of("test-closed-idx").toArray(new String[0]));
-        try {
-            catShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest);
-            catShardsResponse.get();
-            fail("Expected IndexClosedException");
-        } catch (Exception exception) {
-            assertTrue(exception.getMessage().contains("IndexClosedException"));
-        }
-
-        shardsRequest.setPageParams(new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize));
-        listShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest);
+        // Explicitly querying for closed index: /_list/shards/test-closed-idx
+        // should output closed shards without stats.
+        listShardsRequest.setIndices(List.of("test-closed-idx").toArray(new String[0]));
+        listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest);
         assertTrue(listShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-closed-idx")));
         assertEquals(0, listShardsResponse.get().getIndicesStatsResponse().getShards().length);
 
-        // Querying for closed index with wildcards: /_cat/shards/test-closed-idx and /_list/shards/test-closed-idx
-        // Both the queries should return zero entries
-        shardsRequest = new CatShardsRequest();
-        shardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT);
-        shardsRequest.setIndices(List.of("test-closed-idx*").toArray(new String[0]));
-        catShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest);
-        assertEquals(0, catShardsResponse.get().getResponseShards().size());
-        assertEquals(0, catShardsResponse.get().getIndicesStatsResponse().getShards().length);
-
-        shardsRequest.setPageParams(new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize));
-        listShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest);
-        assertEquals(0, listShardsResponse.get().getResponseShards().size());
+        // Querying for closed index with wildcards: /_list/shards/test-closed-idx*
+        // should output closed shards without stats.
+        listShardsRequest.setIndices(List.of("test-closed-idx*").toArray(new String[0]));
+        listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest);
+        assertTrue(listShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-closed-idx")));
         assertEquals(0, listShardsResponse.get().getIndicesStatsResponse().getShards().length);
     }
 

server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java

@@ -16,19 +16,21 @@
 import org.opensearch.action.pagination.ShardPaginationStrategy;
 import org.opensearch.action.support.ActionFilters;
 import org.opensearch.action.support.HandledTransportAction;
+import org.opensearch.action.support.IndicesOptions;
 import org.opensearch.action.support.TimeoutTaskCancellationUtility;
 import org.opensearch.client.node.NodeClient;
+import org.opensearch.cluster.ClusterState;
 import org.opensearch.cluster.metadata.IndexMetadata;
 import org.opensearch.common.breaker.ResponseLimitBreachedException;
 import org.opensearch.common.breaker.ResponseLimitSettings;
 import org.opensearch.common.inject.Inject;
 import org.opensearch.core.action.ActionListener;
 import org.opensearch.core.action.NotifyOnceListener;
+import org.opensearch.core.common.Strings;
 import org.opensearch.tasks.CancellableTask;
 import org.opensearch.tasks.Task;
 import org.opensearch.transport.TransportService;
 
-import java.util.Arrays;
 import java.util.List;
 import java.util.Objects;
 import java.util.stream.Collectors;
@@ -67,6 +69,7 @@ public void doExecute(Task parentTask, CatShardsRequest shardsRequest, ActionLis
             clusterStateRequest.clear().nodes(true).routingTable(true).indices(shardsRequest.getIndices());
         } else {
             clusterStateRequest.clear().nodes(true).routingTable(true).indices(shardsRequest.getIndices()).metadata(true);
+            clusterStateRequest.indicesOptions(IndicesOptions.lenientExpandHidden());
         }
         assert parentTask instanceof CancellableTask;
         clusterStateRequest.setParentTask(client.getLocalNodeId(), parentTask.getId());
@@ -112,11 +115,9 @@ public void onResponse(ClusterStateResponse clusterStateResponse) {
 
                         String[] indices = Objects.isNull(paginationStrategy)
                             ? shardsRequest.getIndices()
-                            : filterClosedAndHiddenIndices(
-                                clusterStateResponse,
-                                paginationStrategy.getRequestedIndices(),
-                                Arrays.asList(shardsRequest.getIndices())
-                            ).toArray(new String[0]);
+                            : filterPaginationResponse(clusterStateResponse.getState(), paginationStrategy.getRequestedIndices()).toArray(
+                                Strings.EMPTY_ARRAY
+                            );
                         // For paginated queries, if strategy outputs no shards to be returned, avoid fetching IndicesStats.
                         if (shouldSkipIndicesStatsRequest(paginationStrategy, indices)) {
                             catShardsResponse.setIndicesStatsResponse(IndicesStatsResponse.getEmptyResponse());
@@ -127,6 +128,11 @@ public void onResponse(ClusterStateResponse clusterStateResponse) {
                         indicesStatsRequest.setShouldCancelOnTimeout(true);
                         indicesStatsRequest.all();
                         indicesStatsRequest.indices(indices);
+                        // Since the indices for paginated query are already concrete and have been filtered to
+                        // only consider OPEN indices, invoking IndexNameExpressionResolver should be avoided.
+                        if (paginationStrategy != null) {
+                            indicesStatsRequest.skipIndexNameResolver(true);
+                        }
                         indicesStatsRequest.setParentTask(client.getLocalNodeId(), parentTask.getId());
                         client.admin().indices().stats(indicesStatsRequest, new ActionListener<IndicesStatsResponse>() {
                             @Override
@@ -176,24 +182,18 @@ private void validateRequestLimit(
     }
 
     private boolean shouldSkipIndicesStatsRequest(ShardPaginationStrategy paginationStrategy, String[] indices) {
-        return Objects.nonNull(paginationStrategy) && Objects.nonNull(indices) && indices.length == 0;
+        return Objects.nonNull(paginationStrategy) && (indices == null || indices.length == 0);
     }
 
     /**
-     * Will be used by paginated query (_list/shards) to filter out closed and hidden indices before fetching
+     * Will be used by paginated query (_list/shards) to filter out closed indices (only consider OPEN) before fetching
      * IndicesStats. Since pagination strategy always passes concrete indices to TransportIndicesStatsAction,
-     * the default behaviour of StrictExpandOpenAndForbidClosed leads to errors if closed indices are encountered and
-     * stats being fetched for hidden indices, making it deviate from default non-paginated queries.
+     * the default behaviour of StrictExpandOpenAndForbidClosed leads to errors if closed indices are encountered.
      */
-    private List<String> filterClosedAndHiddenIndices(
-        ClusterStateResponse clusterStateResponse,
-        List<String> indices,
-        List<String> requestedIndices
-    ) {
-        return indices.stream().filter(index -> {
-            IndexMetadata metadata = clusterStateResponse.getState().getMetadata().indices().get(index);
-            return metadata.getState().equals(IndexMetadata.State.OPEN)
-                && (requestedIndices.contains(index) || !IndexMetadata.INDEX_HIDDEN_SETTING.get(metadata.getSettings()));
+    private List<String> filterPaginationResponse(ClusterState clusterState, List<String> strategyIndices) {
+        return strategyIndices.stream().filter(index -> {
+            IndexMetadata metadata = clusterState.metadata().indices().get(index);
+            return metadata != null && metadata.getState().equals(IndexMetadata.State.OPEN);
         }).collect(Collectors.toList());
     }
 }

server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsRequest.java

@@ -57,6 +57,7 @@
 public class IndicesStatsRequest extends BroadcastRequest<IndicesStatsRequest> {
 
     private CommonStatsFlags flags = new CommonStatsFlags();
+    private boolean skipIndexNameResolver = false;
 
     public IndicesStatsRequest() {
         super((String[]) null);
@@ -307,4 +308,13 @@ public void writeTo(StreamOutput out) throws IOException {
     public boolean includeDataStreams() {
         return true;
     }
+
+    public boolean skipIndexNameResolver() {
+        return skipIndexNameResolver;
+    }
+
+    public IndicesStatsRequest skipIndexNameResolver(boolean skipIndexNameResolver) {
+        this.skipIndexNameResolver = skipIndexNameResolver;
+        return this;
+    }
 }

server/src/main/java/org/opensearch/action/admin/indices/stats/TransportIndicesStatsAction.java

@@ -153,4 +153,12 @@ protected ShardStats shardOperation(IndicesStatsRequest request, ShardRouting sh
         }
         return new ShardStats(indexShard.routingEntry(), indexShard.shardPath(), commonStats, commitStats, seqNoStats, retentionLeaseStats);
     }
+
+    @Override
+    protected String[] resolveConcreteIndexNames(ClusterState clusterState, IndicesStatsRequest request) {
+        if (request.skipIndexNameResolver()) {
+            return request.indices();
+        }
+        return super.resolveConcreteIndexNames(clusterState, request);
+    }
 }