_cat/indices with Security, hide names when wildcard (#38824)

This changes the output of the `_cat/indices` API with `Security` enabled.

It is possible to only display the index name (and possibly the index
health, depending on the request options) but not its stats (doc count, merges,
size, etc). This is the case for closed indices which have index metadata in the
cluster state but no associated shards, hence no shard stats.
However, when `Security` is enabled, and the request contains wildcards,
**open** indices without stats are a common occurrence. This is because the
index names in the response table are picked up directly from the cluster state
which is not filtered by `Security`'s _indexNameExpressionResolver_, unlike the
stats data which is populated by the indices stats API which does go through the
index name resolver.
This is a bug, because it is circumventing `Security`'s function to hide
unauthorized indices.

This has been fixed by displaying the index names as they are resolved by the indices
stats API. The outputs of these two APIs is now very similar: same index names,
similar data but different format.

Closes #37190

Author: albertzaharovits

server/src/main/java/org/elasticsearch/rest/action/cat/RestIndicesAction.java

@@ -35,9 +35,9 @@
 import org.elasticsearch.cluster.health.ClusterIndexHealth;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
-import org.elasticsearch.cluster.metadata.MetaData;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.Table;
+import org.elasticsearch.common.collect.ImmutableOpenMap;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.time.DateFormatter;
 import org.elasticsearch.index.Index;
@@ -95,13 +95,8 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli
         return channel -> client.admin().cluster().state(clusterStateRequest, new RestActionListener<ClusterStateResponse>(channel) {
             @Override
             public void processResponse(final ClusterStateResponse clusterStateResponse) {
-                final ClusterState state = clusterStateResponse.getState();
-                final Index[] concreteIndices = indexNameExpressionResolver.concreteIndices(state, strictExpandIndicesOptions, indices);
-                // concreteIndices should contain exactly the indices in state.metaData() that were selected by clusterStateRequest using
-                // IndicesOptions.strictExpand(). We select the indices again here so that they can be displayed in the resulting table
-                // in the requesting order.
-                assert concreteIndices.length == state.metaData().getIndices().size();
-
+                final ClusterState clusterState = clusterStateResponse.getState();
+                final IndexMetaData[] indicesMetaData = getOrderedIndexMetaData(indices, clusterState, strictExpandIndicesOptions);
                 // Indices that were successfully resolved during the cluster state request might be deleted when the subsequent cluster
                 // health and indices stats requests execute. We have to distinguish two cases:
                 // 1) the deleted index was explicitly passed as parameter to the /_cat/indices request. In this case we want the subsequent
@@ -111,24 +106,24 @@ public void processResponse(final ClusterStateResponse clusterStateResponse) {
                 // This behavior can be ensured by letting the cluster health and indices stats requests re-resolve the index names with the
                 // same indices options that we used for the initial cluster state request (strictExpand). Unfortunately cluster health
                 // requests hard-code their indices options and the best we can do is apply strictExpand to the indices stats request.
-                ClusterHealthRequest clusterHealthRequest = Requests.clusterHealthRequest(indices);
+                final ClusterHealthRequest clusterHealthRequest = Requests.clusterHealthRequest(indices);
                 clusterHealthRequest.local(request.paramAsBoolean("local", clusterHealthRequest.local()));
+
                 client.admin().cluster().health(clusterHealthRequest, new RestActionListener<ClusterHealthResponse>(channel) {
                     @Override
                     public void processResponse(final ClusterHealthResponse clusterHealthResponse) {
-                        IndicesStatsRequest indicesStatsRequest = new IndicesStatsRequest();
+                        final IndicesStatsRequest indicesStatsRequest = new IndicesStatsRequest();
                         indicesStatsRequest.indices(indices);
                         indicesStatsRequest.indicesOptions(strictExpandIndicesOptions);
                         indicesStatsRequest.all();
+
                         client.admin().indices().stats(indicesStatsRequest, new RestResponseListener<IndicesStatsResponse>(channel) {
                             @Override
                             public RestResponse buildResponse(IndicesStatsResponse indicesStatsResponse) throws Exception {
-                                Table tab = buildTable(request, concreteIndices, clusterHealthResponse,
-                                    indicesStatsResponse, state.metaData());
+                                final Table tab = buildTable(request, indicesMetaData, clusterHealthResponse, indicesStatsResponse);
                                 return RestTable.buildResponse(tab, channel);
                             }
                         });
-
                     }
                 });
             }
@@ -388,8 +383,7 @@ protected Table getTableWithHeader(final RestRequest request) {
     }
 
     // package private for testing
-    Table buildTable(RestRequest request, Index[] indices, ClusterHealthResponse response,
-                     IndicesStatsResponse stats, MetaData indexMetaDatas) {
+    Table buildTable(RestRequest request, IndexMetaData[] indicesMetaData, ClusterHealthResponse response, IndicesStatsResponse stats) {
         final String healthParam = request.param("health");
         final ClusterHealthStatus status;
         if (healthParam != null) {
@@ -400,31 +394,50 @@ Table buildTable(RestRequest request, Index[] indices, ClusterHealthResponse res
 
         Table table = getTableWithHeader(request);
 
-        for (final Index index : indices) {
-            final String indexName = index.getName();
+        for (IndexMetaData indexMetaData : indicesMetaData) {
+            final String indexName = indexMetaData.getIndex().getName();
             ClusterIndexHealth indexHealth = response.getIndices().get(indexName);
             IndexStats indexStats = stats.getIndices().get(indexName);
-            IndexMetaData indexMetaData = indexMetaDatas.getIndices().get(indexName);
             IndexMetaData.State state = indexMetaData.getState();
             boolean searchThrottled = IndexSettings.INDEX_SEARCH_THROTTLED.get(indexMetaData.getSettings());
 
             if (status != null) {
                 if (state == IndexMetaData.State.CLOSE ||
-                        (indexHealth == null && !ClusterHealthStatus.RED.equals(status)) ||
-                        !indexHealth.getStatus().equals(status)) {
+                        (indexHealth == null && false == ClusterHealthStatus.RED.equals(status)) ||
+                        false == indexHealth.getStatus().equals(status)) {
                     continue;
                 }
             }
 
-            final CommonStats primaryStats = indexStats == null ? new CommonStats() : indexStats.getPrimaries();
-            final CommonStats totalStats = indexStats == null ? new CommonStats() : indexStats.getTotal();
+            // the open index is present in the cluster state but is not returned in the indices stats API
+            if (indexStats == null && state != IndexMetaData.State.CLOSE) {
+                // the index stats API is called last, after cluster state and cluster health. If the index stats
+                // has not resolved the same open indices as the initial cluster state call, then the indices might
+                // have been removed in the meantime or, more likely, are unauthorized. This is because the cluster
+                // state exposes everything, even unauthorized indices, which are not exposed in APIs.
+                // We ignore such an index instead of displaying it with an empty stats.
+                continue;
+            }
+
+            final CommonStats primaryStats;
+            final CommonStats totalStats;
+
+            if (state == IndexMetaData.State.CLOSE) {
+                // empty stats for closed indices, but their names are displayed
+                assert indexStats == null;
+                primaryStats = new CommonStats();
+                totalStats = new CommonStats();
+            } else {
+                primaryStats = indexStats.getPrimaries();
+                totalStats = indexStats.getTotal();
+            }
 
             table.startRow();
             table.addCell(state == IndexMetaData.State.OPEN ?
                 (indexHealth == null ? "red*" : indexHealth.getStatus().toString().toLowerCase(Locale.ROOT)) : null);
             table.addCell(state.toString().toLowerCase(Locale.ROOT));
             table.addCell(indexName);
-            table.addCell(index.getUUID());
+            table.addCell(indexMetaData.getIndexUUID());
             table.addCell(indexHealth == null ? null : indexHealth.getNumberOfShards());
             table.addCell(indexHealth == null ? null : indexHealth.getNumberOfReplicas());
 
@@ -606,8 +619,8 @@ Table buildTable(RestRequest request, Index[] indices, ClusterHealthResponse res
             table.addCell(totalStats.getSearch() == null ? null : totalStats.getSearch().getTotal().getSuggestCount());
             table.addCell(primaryStats.getSearch() == null ? null : primaryStats.getSearch().getTotal().getSuggestCount());
 
-            table.addCell(indexStats == null ? null : indexStats.getTotal().getTotalMemory());
-            table.addCell(indexStats == null ? null : indexStats.getPrimaries().getTotalMemory());
+            table.addCell(totalStats.getTotalMemory());
+            table.addCell(primaryStats.getTotalMemory());
 
             table.addCell(searchThrottled);
 
@@ -616,4 +629,21 @@ Table buildTable(RestRequest request, Index[] indices, ClusterHealthResponse res
 
         return table;
     }
+
+    // package private for testing
+    IndexMetaData[] getOrderedIndexMetaData(String[] indicesExpression, ClusterState clusterState, IndicesOptions indicesOptions) {
+        final Index[] concreteIndices = indexNameExpressionResolver.concreteIndices(clusterState, indicesOptions, indicesExpression);
+        // concreteIndices should contain exactly the indices in state.metaData() that were selected by clusterStateRequest using the
+        // same indices option (IndicesOptions.strictExpand()). We select the indices again here so that they can be displayed in the
+        // resulting table in the requesting order.
+        assert concreteIndices.length == clusterState.metaData().getIndices().size();
+        final ImmutableOpenMap<String, IndexMetaData> indexMetaDataMap = clusterState.metaData().getIndices();
+        final IndexMetaData[] indicesMetaData = new IndexMetaData[concreteIndices.length];
+        // select the index metadata in the requested order, so that the response can display the indices in the resulting table
+        // in the requesting order.
+        for (int i = 0; i < concreteIndices.length; i++) {
+            indicesMetaData[i] = indexMetaDataMap.get(concreteIndices[i].getName());
+        }
+        return indicesMetaData;
+    }
 }

server/src/test/java/org/elasticsearch/rest/action/cat/RestIndicesActionTests.java

@@ -25,6 +25,7 @@
 import org.elasticsearch.action.admin.indices.stats.IndicesStatsResponse;
 import org.elasticsearch.action.admin.indices.stats.IndicesStatsTests;
 import org.elasticsearch.action.admin.indices.stats.ShardStats;
+import org.elasticsearch.action.support.IndicesOptions;
 import org.elasticsearch.cluster.ClusterName;
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
@@ -38,7 +39,6 @@
 import org.elasticsearch.common.UUIDs;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.TimeValue;
-import org.elasticsearch.index.Index;
 import org.elasticsearch.index.cache.query.QueryCacheStats;
 import org.elasticsearch.index.cache.request.RequestCacheStats;
 import org.elasticsearch.index.engine.SegmentsStats;
@@ -62,6 +62,7 @@
 
 import java.nio.file.Path;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
@@ -73,44 +74,61 @@
  */
 public class RestIndicesActionTests extends ESTestCase {
 
-    public void testBuildTable() {
-        final Settings settings = Settings.EMPTY;
-        UsageService usageService = new UsageService();
-        final RestController restController = new RestController(Collections.emptySet(), null, null, null, usageService);
-        final RestIndicesAction action = new RestIndicesAction(settings, restController, new IndexNameExpressionResolver());
-
+    private IndexMetaData[] buildRandomIndicesMetaData(int numIndices) {
         // build a (semi-)random table
-        final int numIndices = randomIntBetween(0, 5);
-        Index[] indices = new Index[numIndices];
+        final IndexMetaData[] indicesMetaData = new IndexMetaData[numIndices];
         for (int i = 0; i < numIndices; i++) {
-            indices[i] = new Index(randomAlphaOfLength(5), UUIDs.randomBase64UUID());
-        }
-
-        final MetaData.Builder metaDataBuilder = MetaData.builder();
-        for (final Index index : indices) {
-            metaDataBuilder.put(IndexMetaData.builder(index.getName())
+            indicesMetaData[i] = IndexMetaData.builder(randomAlphaOfLength(5) + i)
                                     .settings(Settings.builder()
-                                                  .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
-                                                  .put(IndexMetaData.SETTING_INDEX_UUID, index.getUUID()))
+                                        .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
+                                        .put(IndexMetaData.SETTING_INDEX_UUID, UUIDs.randomBase64UUID()))
                                     .creationDate(System.currentTimeMillis())
                                     .numberOfShards(1)
                                     .numberOfReplicas(1)
-                                    .state(IndexMetaData.State.OPEN));
+                                    .state(IndexMetaData.State.OPEN)
+                                    .build();
         }
-        final MetaData metaData = metaDataBuilder.build();
+        return indicesMetaData;
+    }
 
+    private ClusterState buildClusterState(IndexMetaData[] indicesMetaData) {
+        final MetaData.Builder metaDataBuilder = MetaData.builder();
+        for (IndexMetaData indexMetaData : indicesMetaData) {
+            metaDataBuilder.put(indexMetaData, false);
+        }
+        final MetaData metaData = metaDataBuilder.build();
         final ClusterState clusterState = ClusterState.builder(ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY))
                                               .metaData(metaData)
                                               .build();
-        final String[] indicesStr = new String[indices.length];
-        for (int i = 0; i < indices.length; i++) {
-            indicesStr[i] = indices[i].getName();
+        return clusterState;
+    }
+
+    private ClusterHealthResponse buildClusterHealthResponse(ClusterState clusterState, IndexMetaData[] indicesMetaData) {
+        final String[] indicesStr = new String[indicesMetaData.length];
+        for (int i = 0; i < indicesMetaData.length; i++) {
+            indicesStr[i] = indicesMetaData[i].getIndex().getName();
         }
-        final ClusterHealthResponse clusterHealth = new ClusterHealthResponse(
+        final ClusterHealthResponse clusterHealthResponse = new ClusterHealthResponse(
             clusterState.getClusterName().value(), indicesStr, clusterState, 0, 0, 0, TimeValue.timeValueMillis(1000L)
         );
+        return clusterHealthResponse;
+    }
 
-        final Table table = action.buildTable(new FakeRestRequest(), indices, clusterHealth, randomIndicesStatsResponse(indices), metaData);
+    public void testBuildTable() {
+        final Settings settings = Settings.EMPTY;
+        UsageService usageService = new UsageService();
+        final RestController restController = new RestController(Collections.emptySet(), null, null, null, usageService);
+        final RestIndicesAction action = new RestIndicesAction(settings, restController, new IndexNameExpressionResolver());
+
+        final IndexMetaData[] generatedIndicesMetaData = buildRandomIndicesMetaData(randomIntBetween(1, 5));
+        final ClusterState clusterState = buildClusterState(generatedIndicesMetaData);
+        final ClusterHealthResponse clusterHealthResponse = buildClusterHealthResponse(clusterState, generatedIndicesMetaData);
+
+        final IndexMetaData[] sortedIndicesMetaData = action.getOrderedIndexMetaData(new String[0], clusterState,
+            IndicesOptions.strictExpand());
+        final IndexMetaData[] smallerSortedIndicesMetaData = removeRandomElement(sortedIndicesMetaData);
+        final Table table = action.buildTable(new FakeRestRequest(), sortedIndicesMetaData, clusterHealthResponse,
+                randomIndicesStatsResponse(smallerSortedIndicesMetaData));
 
         // now, verify the table is correct
         int count = 0;
@@ -121,27 +139,27 @@ public void testBuildTable() {
         assertThat(headers.get(count++).value, equalTo("uuid"));
 
         List<List<Table.Cell>> rows = table.getRows();
-        assertThat(rows.size(), equalTo(indices.length));
+        assertThat(rows.size(), equalTo(smallerSortedIndicesMetaData.length));
         // TODO: more to verify (e.g. randomize cluster health, num primaries, num replicas, etc)
         for (int i = 0; i < rows.size(); i++) {
             count = 0;
             final List<Table.Cell> row = rows.get(i);
             assertThat(row.get(count++).value, equalTo("red*")); // all are red because cluster state doesn't have routing entries
             assertThat(row.get(count++).value, equalTo("open")); // all are OPEN for now
-            assertThat(row.get(count++).value, equalTo(indices[i].getName()));
-            assertThat(row.get(count++).value, equalTo(indices[i].getUUID()));
+            assertThat(row.get(count++).value, equalTo(smallerSortedIndicesMetaData[i].getIndex().getName()));
+            assertThat(row.get(count++).value, equalTo(smallerSortedIndicesMetaData[i].getIndexUUID()));
         }
     }
 
-    private IndicesStatsResponse randomIndicesStatsResponse(final Index[] indices) {
+    private IndicesStatsResponse randomIndicesStatsResponse(final IndexMetaData[] indices) {
         List<ShardStats> shardStats = new ArrayList<>();
-        for (final Index index : indices) {
-            int numShards = randomInt(5);
+        for (final IndexMetaData index : indices) {
+            int numShards = randomIntBetween(1, 3);
             int primaryIdx = randomIntBetween(-1, numShards - 1); // -1 means there is no primary shard.
             for (int i = 0; i < numShards; i++) {
-                ShardId shardId = new ShardId(index, i);
+                ShardId shardId = new ShardId(index.getIndex(), i);
                 boolean primary = (i == primaryIdx);
-                Path path = createTempDir().resolve("indices").resolve(index.getUUID()).resolve(String.valueOf(i));
+                Path path = createTempDir().resolve("indices").resolve(index.getIndexUUID()).resolve(String.valueOf(i));
                 ShardRouting shardRouting = ShardRouting.newUnassigned(shardId, primary,
                     primary ? RecoverySource.EmptyStoreRecoverySource.INSTANCE : PeerRecoverySource.INSTANCE,
                     new UnassignedInfo(UnassignedInfo.Reason.INDEX_CREATED, null)
@@ -170,4 +188,14 @@ private IndicesStatsResponse randomIndicesStatsResponse(final Index[] indices) {
             shardStats.toArray(new ShardStats[shardStats.size()]), shardStats.size(), shardStats.size(), 0, emptyList()
         );
     }
+
+    private IndexMetaData[] removeRandomElement(IndexMetaData[] array) {
+        assert array != null;
+        assert array.length > 0;
+        final List<IndexMetaData> collectionLessAnItem = new ArrayList<>();
+        collectionLessAnItem.addAll(Arrays.asList(array));
+        final int toRemoveIndex = randomIntBetween(0, array.length - 1);
+        collectionLessAnItem.remove(toRemoveIndex);
+        return collectionLessAnItem.toArray(new IndexMetaData[0]);
+    }
 }