From d35890e21973f3db968092e41e330c84bbab2646 Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Fri, 31 Aug 2018 10:33:38 +0100 Subject: [PATCH 1/2] Fixes SecurityIntegTestCase so it always adds at least one alias `SecurityIntegTestCase.createIndicesWithRandomAliases` could randomly fail because its not gauranteed that the randomness of which aliases to add to the `IndicesAliasesRequestBuilder` would always select at least one alias to add. This change fixes the problem by keeping track of whether we have added an alias to teh request and forcing the last alias to be added if no other aliases have been added so far. Closes #30098 Closes #33123e --- .../java/org/elasticsearch/test/SecurityIntegTestCase.java | 6 +++++- .../xpack/security/authz/ReadActionsTests.java | 1 - 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java index 9bb0e44eb664c..10b60c950ba9f 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java @@ -420,14 +420,18 @@ protected void createIndicesWithRandomAliases(String... indices) { createIndex(indices); if (frequently()) { + boolean noAliasAdded = true; IndicesAliasesRequestBuilder builder = client().admin().indices().prepareAliases(); for (String index : indices) { if (frequently()) { //one alias per index with prefix "alias-" builder.addAlias(index, "alias-" + index); + noAliasAdded = false; } } - if (randomBoolean()) { + // If we get to this point and we haven't added an alias to the request we need to add one + // or the request will fail so use noAliasAdded to force adding the alias in this case + if (noAliasAdded || randomBoolean()) { //one alias pointing to all indices for (String index : indices) { builder.addAlias(index, "alias"); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/ReadActionsTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/ReadActionsTests.java index a88dafece3251..76568d3d48b5a 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/ReadActionsTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/ReadActionsTests.java @@ -102,7 +102,6 @@ public void testEmptyAuthorizedIndicesSearchForAll() { assertNoSearchHits(client().prepareSearch().get()); } - @AwaitsFix(bugUrl="https://github.com/elastic/elasticsearch/issues/33123") public void testEmptyAuthorizedIndicesSearchForAllDisallowNoIndices() { createIndicesWithRandomAliases("index1", "index2"); IndexNotFoundException e = expectThrows(IndexNotFoundException.class, () -> client().prepareSearch() From 4c27d269625cd813879411d7b6c4233cfc69147c Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Fri, 31 Aug 2018 15:32:25 +0100 Subject: [PATCH 2/2] Addresses review comments --- .../org/elasticsearch/test/SecurityIntegTestCase.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java index 10b60c950ba9f..7143182c1621a 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java @@ -7,6 +7,7 @@ import io.netty.util.ThreadDeathWatcher; import io.netty.util.concurrent.GlobalEventExecutor; + import org.elasticsearch.Version; import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse; import org.elasticsearch.action.admin.cluster.node.info.NodeInfo; @@ -44,7 +45,6 @@ import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken; import org.elasticsearch.xpack.core.security.client.SecurityClient; import org.elasticsearch.xpack.security.LocalStateSecurity; - import org.elasticsearch.xpack.security.support.SecurityIndexManager; import org.junit.AfterClass; import org.junit.Before; @@ -420,18 +420,18 @@ protected void createIndicesWithRandomAliases(String... indices) { createIndex(indices); if (frequently()) { - boolean noAliasAdded = true; + boolean aliasAdded = false; IndicesAliasesRequestBuilder builder = client().admin().indices().prepareAliases(); for (String index : indices) { if (frequently()) { //one alias per index with prefix "alias-" builder.addAlias(index, "alias-" + index); - noAliasAdded = false; + aliasAdded = true; } } // If we get to this point and we haven't added an alias to the request we need to add one // or the request will fail so use noAliasAdded to force adding the alias in this case - if (noAliasAdded || randomBoolean()) { + if (aliasAdded == false || randomBoolean()) { //one alias pointing to all indices for (String index : indices) { builder.addAlias(index, "alias");