Skip to content

Commit

Permalink
Use alias name from rollover request to query indices stats (#40774)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bizybot committed Apr 16, 2019
1 parent 4fb156a commit 3c66cff
Show file tree
Hide file tree
Showing 3 changed files with 281 additions and 8 deletions.
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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());

Expand Down Expand Up @@ -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();
Expand Down
@@ -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"}

0 comments on commit 3c66cff

Please sign in to comment.