Use alias name from rollover request to query indices stats (#40774) (#41284)

bizybot · web-flow · commit 6a552c05fef4 · 2019-04-17T14:15:05.000+10:00
In `TransportRolloverAction` before doing rollover we resolve source index name (write index) from the alias in the rollover request. Before evaluating the conditions and executing rollover action, we retrieve stats, but to do so we used the source index name resolved from the alias instead of alias from the index. This fails when the user is assigned a role with index privilege on the alias instead of the concrete index. This commit fixes this by using the alias from the request. After this change, verified that when we retrieve all the stats (including write + read indexes) we are considering only source index. Closes #40771
diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java
@@ -120,7 +120,7 @@ protected void masterOperation(final RolloverRequest rolloverRequest, final Clus
         final String rolloverIndexName = indexNameExpressionResolver.resolveDateMathExpression(unresolvedName);
         MetaDataCreateIndexService.validateIndexName(rolloverIndexName, state); // will fail if the index already exists
         checkNoDuplicatedAliasInIndexTemplate(metaData, rolloverIndexName, rolloverRequest.getAlias());
-        client.admin().indices().prepareStats(sourceIndexName).clear().setDocs(true).execute(
+        client.admin().indices().prepareStats(rolloverRequest.getAlias()).clear().setDocs(true).execute(
             new ActionListener<IndicesStatsResponse>() {
                 @Override
                 public void onResponse(IndicesStatsResponse statsResponse) {
@@ -249,7 +249,7 @@ static Map<String, Boolean> evaluateConditions(final Collection<Condition<?>> co
 
     static Map<String, Boolean> evaluateConditions(final Collection<Condition<?>> conditions, final IndexMetaData metaData,
                                                     final IndicesStatsResponse statsResponse) {
-        return evaluateConditions(conditions, statsResponse.getPrimaries().getDocs(), metaData);
+        return evaluateConditions(conditions, statsResponse.getIndex(metaData.getIndex().getName()).getPrimaries().getDocs(), metaData);
     }
 
     static void validate(MetaData metaData, RolloverRequest request) {
diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverActionTests.java
@@ -20,17 +20,30 @@
 package org.elasticsearch.action.admin.indices.rollover;
 
 import org.elasticsearch.Version;
+import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.admin.indices.alias.IndicesAliasesClusterStateUpdateRequest;
 import org.elasticsearch.action.admin.indices.create.CreateIndexClusterStateUpdateRequest;
 import org.elasticsearch.action.admin.indices.stats.CommonStats;
+import org.elasticsearch.action.admin.indices.stats.IndexStats;
+import org.elasticsearch.action.admin.indices.stats.IndicesStatsRequestBuilder;
 import org.elasticsearch.action.admin.indices.stats.IndicesStatsResponse;
+import org.elasticsearch.action.support.ActionFilters;
 import org.elasticsearch.action.support.ActiveShardCount;
+import org.elasticsearch.action.support.PlainActionFuture;
+import org.elasticsearch.client.AdminClient;
+import org.elasticsearch.client.Client;
+import org.elasticsearch.client.IndicesAdminClient;
+import org.elasticsearch.cluster.ClusterName;
+import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.metadata.AliasAction;
 import org.elasticsearch.cluster.metadata.AliasMetaData;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
 import org.elasticsearch.cluster.metadata.IndexTemplateMetaData;
 import org.elasticsearch.cluster.metadata.MetaData;
+import org.elasticsearch.cluster.metadata.MetaDataCreateIndexService;
+import org.elasticsearch.cluster.metadata.MetaDataIndexAliasesService;
+import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.UUIDs;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.ByteSizeUnit;
@@ -39,9 +52,12 @@
 import org.elasticsearch.common.util.set.Sets;
 import org.elasticsearch.index.shard.DocsStats;
 import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.threadpool.ThreadPool;
+import org.elasticsearch.transport.TransportService;
 import org.mockito.ArgumentCaptor;
 
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -51,7 +67,9 @@
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.is;
 import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -64,7 +82,9 @@ public void testDocStatsSelectionFromPrimariesOnly() {
         long docsInShards = 200;
 
         final Condition<?> condition = createTestCondition();
-        evaluateConditions(Sets.newHashSet(condition), createMetaData(), createIndicesStatResponse(docsInShards, docsInPrimaryShards));
+        String indexName = randomAlphaOfLengthBetween(5, 7);
+        evaluateConditions(Sets.newHashSet(condition), createMetaData(indexName),
+                createIndicesStatResponse(indexName, docsInShards, docsInPrimaryShards));
         final ArgumentCaptor<Condition.Stats> argument = ArgumentCaptor.forClass(Condition.Stats.class);
         verify(condition).evaluate(argument.capture());
 
@@ -286,36 +306,150 @@ public void testRejectDuplicateAlias() {
             .patterns(Arrays.asList("foo-*", "bar-*"))
             .putAlias(AliasMetaData.builder("foo-write")).putAlias(AliasMetaData.builder("bar-write").writeIndex(randomBoolean()))
             .build();
-        final MetaData metaData = MetaData.builder().put(createMetaData(), false).put(template).build();
+        final MetaData metaData = MetaData.builder().put(createMetaData(randomAlphaOfLengthBetween(5, 7)), false).put(template).build();
         String indexName = randomFrom("foo-123", "bar-xyz");
         String aliasName = randomFrom("foo-write", "bar-write");
         final IllegalArgumentException ex = expectThrows(IllegalArgumentException.class,
             () -> TransportRolloverAction.checkNoDuplicatedAliasInIndexTemplate(metaData, indexName, aliasName));
         assertThat(ex.getMessage(), containsString("index template [test-template]"));
     }
 
-    private IndicesStatsResponse createIndicesStatResponse(long totalDocs, long primaryDocs) {
+    public void testConditionEvaluationWhenAliasToWriteAndReadIndicesConsidersOnlyPrimariesFromWriteIndex() {
+        final TransportService mockTransportService = mock(TransportService.class);
+        final ClusterService mockClusterService = mock(ClusterService.class);
+        final ThreadPool mockThreadPool = mock(ThreadPool.class);
+        final MetaDataCreateIndexService mockCreateIndexService = mock(MetaDataCreateIndexService.class);
+        final IndexNameExpressionResolver mockIndexNameExpressionResolver = mock(IndexNameExpressionResolver.class);
+        when(mockIndexNameExpressionResolver.resolveDateMathExpression(any())).thenReturn("logs-index-000003");
+        final ActionFilters mockActionFilters = mock(ActionFilters.class);
+        final MetaDataIndexAliasesService mdIndexAliasesService = mock(MetaDataIndexAliasesService.class);
+
+        final Client mockClient = mock(Client.class);
+        final AdminClient mockAdminClient = mock(AdminClient.class);
+        final IndicesAdminClient mockIndicesAdminClient = mock(IndicesAdminClient.class);
+        when(mockClient.admin()).thenReturn(mockAdminClient);
+        when(mockAdminClient.indices()).thenReturn(mockIndicesAdminClient);
+
+        final IndicesStatsRequestBuilder mockIndicesStatsBuilder = mock(IndicesStatsRequestBuilder.class);
+        when(mockIndicesAdminClient.prepareStats(any())).thenReturn(mockIndicesStatsBuilder);
+        final Map<String, IndexStats> indexStats = new HashMap<>();
+        int total = randomIntBetween(500, 1000);
+        indexStats.put("logs-index-000001", createIndexStats(200L, total));
+        indexStats.put("logs-index-000002", createIndexStats(300L, total));
+        final IndicesStatsResponse statsResponse = createAliasToMultipleIndicesStatsResponse(indexStats);
+        when(mockIndicesStatsBuilder.clear()).thenReturn(mockIndicesStatsBuilder);
+        when(mockIndicesStatsBuilder.setDocs(true)).thenReturn(mockIndicesStatsBuilder);
+
+        assert statsResponse.getPrimaries().getDocs().getCount() == 500L;
+        assert statsResponse.getTotal().getDocs().getCount() == (total + total);
+
+        doAnswer(invocation -> {
+            Object[] args = invocation.getArguments();
+            assert args.length == 1;
+            ActionListener<IndicesStatsResponse> listener = (ActionListener<IndicesStatsResponse>) args[0];
+            listener.onResponse(statsResponse);
+            return null;
+        }).when(mockIndicesStatsBuilder).execute(any(ActionListener.class));
+
+        final IndexMetaData.Builder indexMetaData = IndexMetaData.builder("logs-index-000001")
+                .putAlias(AliasMetaData.builder("logs-alias").writeIndex(false).build()).settings(settings(Version.CURRENT))
+                .numberOfShards(1).numberOfReplicas(1);
+        final IndexMetaData.Builder indexMetaData2 = IndexMetaData.builder("logs-index-000002")
+                .putAlias(AliasMetaData.builder("logs-alias").writeIndex(true).build()).settings(settings(Version.CURRENT))
+                .numberOfShards(1).numberOfReplicas(1);
+        final ClusterState stateBefore = ClusterState.builder(ClusterName.DEFAULT)
+                .metaData(MetaData.builder().put(indexMetaData).put(indexMetaData2)).build();
+
+        final TransportRolloverAction transportRolloverAction = new TransportRolloverAction(mockTransportService, mockClusterService,
+                mockThreadPool, mockCreateIndexService, mockActionFilters, mockIndexNameExpressionResolver, mdIndexAliasesService,
+                mockClient);
+
+        // For given alias, verify that condition evaluation fails when the condition doc count is greater than the primaries doc count
+        // (primaries from only write index is considered)
+        PlainActionFuture<RolloverResponse> future = new PlainActionFuture<>();
+        RolloverRequest rolloverRequest = new RolloverRequest("logs-alias", "logs-index-000003");
+        rolloverRequest.addMaxIndexDocsCondition(500L);
+        rolloverRequest.dryRun(true);
+        transportRolloverAction.masterOperation(rolloverRequest, stateBefore, future);
+
+        RolloverResponse response = future.actionGet();
+        assertThat(response.getOldIndex(), equalTo("logs-index-000002"));
+        assertThat(response.getNewIndex(), equalTo("logs-index-000003"));
+        assertThat(response.isDryRun(), equalTo(true));
+        assertThat(response.isRolledOver(), equalTo(false));
+        assertThat(response.getConditionStatus().size(), equalTo(1));
+        assertThat(response.getConditionStatus().get("[max_docs: 500]"), is(false));
+
+        // For given alias, verify that the condition evaluation is successful when condition doc count is less than the primaries doc count
+        // (primaries from only write index is considered)
+        future = new PlainActionFuture<>();
+        rolloverRequest = new RolloverRequest("logs-alias", "logs-index-000003");
+        rolloverRequest.addMaxIndexDocsCondition(300L);
+        rolloverRequest.dryRun(true);
+        transportRolloverAction.masterOperation(rolloverRequest, stateBefore, future);
+
+        response = future.actionGet();
+        assertThat(response.getOldIndex(), equalTo("logs-index-000002"));
+        assertThat(response.getNewIndex(), equalTo("logs-index-000003"));
+        assertThat(response.isDryRun(), equalTo(true));
+        assertThat(response.isRolledOver(), equalTo(false));
+        assertThat(response.getConditionStatus().size(), equalTo(1));
+        assertThat(response.getConditionStatus().get("[max_docs: 300]"), is(true));
+    }
+
+    private IndicesStatsResponse createIndicesStatResponse(String indexName, long totalDocs, long primariesDocs) {
         final CommonStats primaryStats = mock(CommonStats.class);
-        when(primaryStats.getDocs()).thenReturn(new DocsStats(primaryDocs, 0, between(1, 10000)));
+        when(primaryStats.getDocs()).thenReturn(new DocsStats(primariesDocs, 0, between(1, 10000)));
 
         final CommonStats totalStats = mock(CommonStats.class);
         when(totalStats.getDocs()).thenReturn(new DocsStats(totalDocs, 0, between(1, 10000)));
 
         final IndicesStatsResponse response = mock(IndicesStatsResponse.class);
         when(response.getPrimaries()).thenReturn(primaryStats);
         when(response.getTotal()).thenReturn(totalStats);
+        final IndexStats indexStats = mock(IndexStats.class);
+        when(response.getIndex(indexName)).thenReturn(indexStats);
+        when(indexStats.getPrimaries()).thenReturn(primaryStats);
+        when(indexStats.getTotal()).thenReturn(totalStats);
+        return response;
+    }
+
+    private IndicesStatsResponse createAliasToMultipleIndicesStatsResponse(Map<String, IndexStats> indexStats) {
+        final IndicesStatsResponse response = mock(IndicesStatsResponse.class);
+        final CommonStats primariesStats = new CommonStats();
+        final CommonStats totalStats = new CommonStats();
+        for (String indexName : indexStats.keySet()) {
+            when(response.getIndex(indexName)).thenReturn(indexStats.get(indexName));
+            primariesStats.add(indexStats.get(indexName).getPrimaries());
+            totalStats.add(indexStats.get(indexName).getTotal());
+        }
 
+        when(response.getPrimaries()).thenReturn(primariesStats);
+        when(response.getTotal()).thenReturn(totalStats);
         return response;
     }
 
-    private static IndexMetaData createMetaData() {
+    private IndexStats createIndexStats(long primaries, long total) {
+        final CommonStats primariesCommonStats = mock(CommonStats.class);
+        when(primariesCommonStats.getDocs()).thenReturn(new DocsStats(primaries, 0, between(1, 10000)));
+
+        final CommonStats totalCommonStats = mock(CommonStats.class);
+        when(totalCommonStats.getDocs()).thenReturn(new DocsStats(total, 0, between(1, 10000)));
+
+        IndexStats indexStats = mock(IndexStats.class);
+        when(indexStats.getPrimaries()).thenReturn(primariesCommonStats);
+        when(indexStats.getTotal()).thenReturn(totalCommonStats);
+        return indexStats;
+    }
+
+    private static IndexMetaData createMetaData(String indexName) {
         final Settings settings = Settings.builder()
             .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
             .put(IndexMetaData.SETTING_INDEX_UUID, UUIDs.randomBase64UUID())
             .put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1)
             .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0)
             .build();
-        return IndexMetaData.builder(randomAlphaOfLength(10))
+        return IndexMetaData.builder(indexName)
             .creationDate(System.currentTimeMillis() - TimeValue.timeValueHours(3).getMillis())
             .settings(settings)
             .build();
diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/security/authz/31_rollover_using_alias.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/security/authz/31_rollover_using_alias.yml
@@ -0,0 +1,139 @@
+---
+
+setup:
+  - skip:
+      features: headers
+
+  - do:
+      cluster.health:
+          wait_for_status: yellow
+
+  - do:
+      security.put_role:
+        name: "alias_write_manage_role"
+        body:  >
+            {
+              "indices": [
+                { "names": ["write_manage_alias"], "privileges": ["write", "manage"] }
+              ]
+            }
+
+  - do:
+      security.put_user:
+        username: "test_user"
+        body:  >
+          {
+            "password" : "x-pack-test-password",
+            "roles" : [ "alias_write_manage_role" ],
+            "full_name" : "user with privileges to write, manage via alias"
+          }
+
+  - do:
+      indices.create:
+        index: logs-000001
+        body:
+          settings:
+            index:
+              number_of_shards:   1
+              number_of_replicas: 0
+
+  - do:
+      indices.put_alias:
+        index: logs-000001
+        name: write_manage_alias
+
+---
+teardown:
+  - do:
+      security.delete_user:
+        username: "test_user"
+        ignore: 404
+
+  - do:
+      security.delete_role:
+        name: "alias_write_role"
+        ignore: 404
+
+  - do:
+      indices.delete_alias:
+        index: "logs-000001"
+        name: [ "write_manage_alias" ]
+        ignore: 404
+
+  - do:
+      indices.delete:
+        index: [ "logs-000001" ]
+        ignore: 404
+
+---
+"Test rollover, index via write alias of index":
+
+  # index using alias
+  - do:
+      headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
+      create:
+        id: 1
+        index: write_manage_alias
+        body: >
+          {
+            "name" : "doc1"
+          }
+
+  - do:
+      headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
+      create:
+        id: 2
+        index: write_manage_alias
+        body: >
+          {
+            "name" : "doc2"
+          }
+
+  - do:
+      indices.refresh: {}
+
+  # rollover using alias
+  - do:
+      headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
+      indices.rollover:
+        alias: "write_manage_alias"
+        wait_for_active_shards: 1
+        body:
+          conditions:
+            max_docs: 1
+
+  - match: { old_index: logs-000001 }
+  - match: { new_index: logs-000002 }
+  - match: { rolled_over: true }
+  - match: { dry_run: false }
+  - match: { conditions: { "[max_docs: 1]": true } }
+
+  # ensure new index is created
+  - do:
+      indices.exists:
+        index: logs-000002
+
+  - is_true: ''
+
+  # index using alias
+  - do:
+      headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
+      create:
+        id: 3
+        index: write_manage_alias
+        body: >
+          {
+            "name" : "doc3"
+          }
+
+  - do:
+      indices.refresh: {}
+
+  # check alias points to the new index and the doc was indexed
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: write_manage_alias
+
+  - match: { hits.total: 1 }
+  - match: { hits.hits.0._index: "logs-000002"}
